Bug 90839

*** Bug 90791 has been marked as a duplicate of this bug. ***

This sounds like something Jason would know about.

Yeah, this is gnarly.  Unfortunately, I'm not able to reproduce at the moment probably due to getting different visuals.  However, I do see what the problem is and I think we can probably start just using the meta code and that might fix it.

I can't reproduce it either; actually, I don't see RGBX configs in glxinfo output, should they manifest there?

Anyhow, can the check in do_blit_drawpixels be relaxed to allow RGBX<->RGBA blits?   intel_miptree_blit is able to handle it per this comment: http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/intel_blit.c?id=536003c11e4cb1172c540932ce3cce06f03bf44e#n170

About using Meta for Drawpixels, it needs work to avoid regressing performance, please see discussion in comments 2, 3, 4 at https://bugs.freedesktop.org/show_bug.cgi?id=77412

I've pushed my drive-by fix to enable the blit path for glDrawPixels onto a RGBX framebuffer, 

commit 922c0c9fd526ce19b87bc74a3159dec7705c1de1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jun 5 14:45:18 2015 +0100

    i965: Export format comparison for blitting between miptrees

hopefully this fixes the immediate regression of falling all the way back to pulling and pushing pixels. Presumably this is only a stop gap until meta replaces this intermediate fallback?

(In reply to Chris Wilson from comment #5)
> I've pushed my drive-by fix to enable the blit path for glDrawPixels onto a
> RGBX framebuffer, 
> 
> commit 922c0c9fd526ce19b87bc74a3159dec7705c1de1
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jun 5 14:45:18 2015 +0100
> 
>     i965: Export format comparison for blitting between miptrees
> 
> hopefully this fixes the immediate regression of falling all the way back to
> pulling and pushing pixels. Presumably this is only a stop gap until meta
> replaces this intermediate fallback?

Thanks for getting that fixed quicly!  Yes, using meta was my plan and it really shouldn't take much code.  I just need to find time to get to it.

I've switch to mesa-git and this doesn't fix the issue :/

Still the same:
do_blit_drawpixels
do_blit_drawpixels: bad format for blit
intelDrawPixels: fallback to generic code in PBO case

Are you using lib32-mesa-git for 32-bit libraries?

Yes, both mesa-git and lib32-mesa-git.

Do you get the same INTEL_DEBUG=perf,pix diagnostics on the small repro referenced in the opening comment?

Can you apply the following patch and collect INTEL_DEBUG=perf,pix output again?  At least we'll know which exact formats are rejected.

From 981de62fb518cae8a2278123597131f4d22e930d Mon Sep 17 00:00:00 2001
From: Alexander Monakov <amonakov@ispras.ru>
Date: Tue, 9 Jun 2015 10:58:29 +0300
Subject: [PATCH] i965: spell out incompatible formats in do_blit_drawpixels

---
 src/mesa/drivers/dri/i965/intel_pixel_draw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_pixel_draw.c b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
index d68cbb6..d39c698 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
@@ -85,7 +85,9 @@ do_blit_drawpixels(struct gl_context * ctx,
    dst_format = _mesa_get_srgb_format_linear(dst_format);
 
    if (!intel_miptree_blit_compatible_formats(src_format, dst_format)) {
-      DBG("%s: bad format for blit\n", __func__);
+      DBG("%s: bad format for blit: %s to %s\n", __func__,
+	  _mesa_get_format_name(src_format),
+	  _mesa_get_format_name(dst_format));
       return false;
    }

Yes, I get the same on the small app.

With the debug patch the game just crash when I set INTEL_DEBUG=perf,pix.

Does it also crash the small repro with the patch?  If not, can you show INTEL_DEBUG=perf,pix output on the repro?  If yes, can you show crash backtraces?

For the small repro, it crashes without primusrun, and with primusrun this is the output:

...
do_blit_drawpixels: bad format for blit: (null) to MESA_FORMAT_B8G8R8X8_UNORM
intelDrawPixels: fallback to generic code in PBO case
do_blit_drawpixels
...

Sigh.  Please apply this:

From dae0d4b5716b9a5d9227539bb6312f1f389bfcd3 Mon Sep 17 00:00:00 2001
From: Alexander Monakov <amonakov@ispras.ru>
Date: Tue, 9 Jun 2015 20:58:22 +0300
Subject: [PATCH] i965: do_blit_drawpixels: decode array formats

---
 src/mesa/drivers/dri/i965/intel_pixel_draw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_pixel_draw.c b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
index d68cbb6..189a592 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
@@ -78,6 +78,8 @@ do_blit_drawpixels(struct gl_context * ctx,
    struct intel_renderbuffer *irb = intel_renderbuffer(rb);
 
    mesa_format src_format = _mesa_format_from_format_and_type(format, type);
+   if (_mesa_format_is_mesa_array_format(src_format))
+      src_format = _mesa_format_from_array_format(src_format);
    mesa_format dst_format = irb->mt->format;
 
    /* We can safely discard sRGB encode/decode for the DrawPixels interface */

(In reply to Alexander Monakov from comment #14)
> Sigh.  Please apply this:
> 
> From dae0d4b5716b9a5d9227539bb6312f1f389bfcd3 Mon Sep 17 00:00:00 2001
> From: Alexander Monakov <amonakov@ispras.ru>
> Date: Tue, 9 Jun 2015 20:58:22 +0300
> Subject: [PATCH] i965: do_blit_drawpixels: decode array formats
> 
> ---
>  src/mesa/drivers/dri/i965/intel_pixel_draw.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> index d68cbb6..189a592 100644
> --- a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> +++ b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> @@ -78,6 +78,8 @@ do_blit_drawpixels(struct gl_context * ctx,
>     struct intel_renderbuffer *irb = intel_renderbuffer(rb);
>  
>     mesa_format src_format = _mesa_format_from_format_and_type(format, type);
> +   if (_mesa_format_is_mesa_array_format(src_format))
> +      src_format = _mesa_format_from_array_format(src_format);

Assuming this fixes the problem, I'd rather see it as:

   const mesa_format src_format = _mesa_format_is_mesa_array_format(src_format)
      ? _mesa_format_from_array_format(src_format)
      : _mesa_format_from_format_and_type(format, type);

>     mesa_format dst_format = irb->mt->format;
>  
>     /* We can safely discard sRGB encode/decode for the DrawPixels interface
> */

> >     struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> >  
> >     mesa_format src_format = _mesa_format_from_format_and_type(format, type);
> > +   if (_mesa_format_is_mesa_array_format(src_format))
> > +      src_format = _mesa_format_from_array_format(src_format);
> 
> Assuming this fixes the problem, I'd rather see it as:
> 
>    const mesa_format src_format =
> _mesa_format_is_mesa_array_format(src_format)
                                    ^^^^^^^^^^

Huh?  You can't define src_format based on some predicate computed from yet-uninitialized src_format.

>       ? _mesa_format_from_array_format(src_format)
>       : _mesa_format_from_format_and_type(format, type);

(In reply to Alexander Monakov from comment #14)
> Sigh.  Please apply this:
> 
> From dae0d4b5716b9a5d9227539bb6312f1f389bfcd3 Mon Sep 17 00:00:00 2001
> From: Alexander Monakov <amonakov@ispras.ru>
> Date: Tue, 9 Jun 2015 20:58:22 +0300
> Subject: [PATCH] i965: do_blit_drawpixels: decode array formats
> 
> ---
>  src/mesa/drivers/dri/i965/intel_pixel_draw.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> index d68cbb6..189a592 100644
> --- a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> +++ b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> @@ -78,6 +78,8 @@ do_blit_drawpixels(struct gl_context * ctx,
>     struct intel_renderbuffer *irb = intel_renderbuffer(rb);
>  
>     mesa_format src_format = _mesa_format_from_format_and_type(format, type);
> +   if (_mesa_format_is_mesa_array_format(src_format))
> +      src_format = _mesa_format_from_array_format(src_format);
>     mesa_format dst_format = irb->mt->format;
>  
>     /* We can safely discard sRGB encode/decode for the DrawPixels interface
> */

I can confirm this fix the issue, thanks!

(In reply to Alexander Monakov from comment #14)
> Sigh.  Please apply this:
> 
> From dae0d4b5716b9a5d9227539bb6312f1f389bfcd3 Mon Sep 17 00:00:00 2001
> From: Alexander Monakov <amonakov@ispras.ru>
> Date: Tue, 9 Jun 2015 20:58:22 +0300
> Subject: [PATCH] i965: do_blit_drawpixels: decode array formats
> 
> ---
>  src/mesa/drivers/dri/i965/intel_pixel_draw.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> index d68cbb6..189a592 100644
> --- a/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> +++ b/src/mesa/drivers/dri/i965/intel_pixel_draw.c
> @@ -78,6 +78,8 @@ do_blit_drawpixels(struct gl_context * ctx,
>     struct intel_renderbuffer *irb = intel_renderbuffer(rb);
>  
>     mesa_format src_format = _mesa_format_from_format_and_type(format, type);
> +   if (_mesa_format_is_mesa_array_format(src_format))
> +      src_format = _mesa_format_from_array_format(src_format);
>     mesa_format dst_format = irb->mt->format;
>  
>     /* We can safely discard sRGB encode/decode for the DrawPixels interface
> */

Please send this to the mesa mailing list.

Done, http://lists.freedesktop.org/archives/mesa-dev/2015-June/086113.html

Everything works fine with mesa-git now, I guess this can be marked as fixed.