Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-08-11 Thread Neil Roberts
Thanks for the review. I've pushed the Mesa patch and I'll have a look
at adding more formats to the Piglit test.

Regards,
- Neil

Kenneth Graunke kenn...@whitecape.org writes:

 On Tuesday, July 01, 2014 04:04:56 PM Neil Roberts wrote:
  FWIW, I relaxed the format restrictions in
  brw_blorp_copytexsubimage, so it can handle general format
  conversions as well (i.e. RGBA_FLOAT16 - RGBA_UNORM). There's
  no reason we couldn't do that for BlitFramebuffer as well, I just
  forgot to do it (and then we decided to make it a newbie task, and
  then the newbie didn't do said task, and...oops.)
 
 Ok, I think that is a good idea. Here is a patch to do it. I'm also
 sending a test to the piglit mailing list that tries blitting between
 different formats.
 
 Regards,
 - Neil

 Oh, sorry I missed this!  This looks good to me, and it'll be good to have 
 this for 10.3...

 Reviewed-by: Kenneth Graunke kenn...@whitecape.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-08-08 Thread Kenneth Graunke
On Tuesday, July 01, 2014 04:04:56 PM Neil Roberts wrote:
  FWIW, I relaxed the format restrictions in
  brw_blorp_copytexsubimage, so it can handle general format
  conversions as well (i.e. RGBA_FLOAT16 - RGBA_UNORM). There's
  no reason we couldn't do that for BlitFramebuffer as well, I just
  forgot to do it (and then we decided to make it a newbie task, and
  then the newbie didn't do said task, and...oops.)
 
 Ok, I think that is a good idea. Here is a patch to do it. I'm also
 sending a test to the piglit mailing list that tries blitting between
 different formats.
 
 Regards,
 - Neil

Oh, sorry I missed this!  This looks good to me, and it'll be good to have this 
for 10.3...

Reviewed-by: Kenneth Graunke kenn...@whitecape.org

 --- 8 --- (use git am --scissors to automatically chop here)
 Subject: i965: Don't check for format differences when using the blorp blitter
 
 Previously the blorp blitter wouldn't be used if the source and destination
 buffer had a different format other than swizzling between RGB and BGR and
 adding or removing a dummy alpha channel. However there's no reason why the
 blorp code path can't be used to do almost all format conversions so this
 patch just removes the checks. However it does explicitly disable converting
 to/from MESA_FORMAT_Z24_UNORM_X8_UINT because there is a similar check
 brw_blorp_copytexsubimage.
 
 This doesn't cause any Piglit test regressions at least on Ivybridge.
 ---
  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 66 
 +---
  1 file changed, 12 insertions(+), 54 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp 
 b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 index d7f5f7d..6a4918e 100644
 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 @@ -120,52 +120,6 @@ do_blorp_blit(struct brw_context *brw, GLbitfield 
 buffer_bit,
  }
  
  static bool
 -format_is_rgba_or_rgbx_byte(mesa_format format)
 -{
 -   switch (format) {
 -   case MESA_FORMAT_B8G8R8X8_UNORM:
 -   case MESA_FORMAT_B8G8R8A8_UNORM:
 -   case MESA_FORMAT_R8G8B8X8_UNORM:
 -   case MESA_FORMAT_R8G8B8A8_UNORM:
 -  return true;
 -   default:
 -  return false;
 -   }
 -}
 -
 -static bool
 -color_formats_match(mesa_format src_format, mesa_format dst_format)
 -{
 -   mesa_format linear_src_format = _mesa_get_srgb_format_linear(src_format);
 -   mesa_format linear_dst_format = _mesa_get_srgb_format_linear(dst_format);
 -
 -   /* Normally, we require the formats to be equal. However, we also support
 -* blitting from ARGB to XRGB (discarding alpha), and from XRGB to ARGB
 -* (overriding alpha to 1.0 via blending) as well as swizzling between BGR
 -* and RGB.
 -*/
 -
 -   return (linear_src_format == linear_dst_format ||
 -   (format_is_rgba_or_rgbx_byte(linear_src_format) 
 -format_is_rgba_or_rgbx_byte(linear_dst_format)));
 -}
 -
 -static bool
 -formats_match(GLbitfield buffer_bit, struct intel_renderbuffer *src_irb,
 -  struct intel_renderbuffer *dst_irb)
 -{
 -   /* Note: don't just check gl_renderbuffer::Format, because in some cases
 -* multiple gl_formats resolve to the same native type in the miptree (for
 -* example MESA_FORMAT_Z24_UNORM_X8_UINT and 
 MESA_FORMAT_Z24_UNORM_S8_UINT), and we can blit
 -* between those formats.
 -*/
 -   mesa_format src_format = find_miptree(buffer_bit, src_irb)-format;
 -   mesa_format dst_format = find_miptree(buffer_bit, dst_irb)-format;
 -
 -   return color_formats_match(src_format, dst_format);
 -}
 -
 -static bool
  try_blorp_blit(struct brw_context *brw,
 GLfloat srcX0, GLfloat srcY0, GLfloat srcX1, GLfloat srcY1,
 GLfloat dstX0, GLfloat dstY0, GLfloat dstX1, GLfloat dstY1,
 @@ -191,16 +145,13 @@ try_blorp_blit(struct brw_context *brw,
 /* Find buffers */
 struct intel_renderbuffer *src_irb;
 struct intel_renderbuffer *dst_irb;
 +   struct intel_mipmap_tree *src_mt;
 +   struct intel_mipmap_tree *dst_mt;
 switch (buffer_bit) {
 case GL_COLOR_BUFFER_BIT:
src_irb = intel_renderbuffer(read_fb-_ColorReadBuffer);
for (unsigned i = 0; i  ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
   dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
 - if (dst_irb  !formats_match(buffer_bit, src_irb, dst_irb))
 -return false;
 -  }
 -  for (unsigned i = 0; i  ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
 - dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
if (dst_irb)
  do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
 @@ -212,8 +163,17 @@ try_blorp_blit(struct brw_context *brw,
   intel_renderbuffer(read_fb-Attachment[BUFFER_DEPTH].Renderbuffer);
dst_irb =
   

Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-07-01 Thread Neil Roberts
 FWIW, I relaxed the format restrictions in
 brw_blorp_copytexsubimage, so it can handle general format
 conversions as well (i.e. RGBA_FLOAT16 - RGBA_UNORM). There's
 no reason we couldn't do that for BlitFramebuffer as well, I just
 forgot to do it (and then we decided to make it a newbie task, and
 then the newbie didn't do said task, and...oops.)

Ok, I think that is a good idea. Here is a patch to do it. I'm also
sending a test to the piglit mailing list that tries blitting between
different formats.

Regards,
- Neil

--- 8 --- (use git am --scissors to automatically chop here)
Subject: i965: Don't check for format differences when using the blorp blitter

Previously the blorp blitter wouldn't be used if the source and destination
buffer had a different format other than swizzling between RGB and BGR and
adding or removing a dummy alpha channel. However there's no reason why the
blorp code path can't be used to do almost all format conversions so this
patch just removes the checks. However it does explicitly disable converting
to/from MESA_FORMAT_Z24_UNORM_X8_UINT because there is a similar check
brw_blorp_copytexsubimage.

This doesn't cause any Piglit test regressions at least on Ivybridge.
---
 src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 66 +---
 1 file changed, 12 insertions(+), 54 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index d7f5f7d..6a4918e 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -120,52 +120,6 @@ do_blorp_blit(struct brw_context *brw, GLbitfield 
buffer_bit,
 }
 
 static bool
-format_is_rgba_or_rgbx_byte(mesa_format format)
-{
-   switch (format) {
-   case MESA_FORMAT_B8G8R8X8_UNORM:
-   case MESA_FORMAT_B8G8R8A8_UNORM:
-   case MESA_FORMAT_R8G8B8X8_UNORM:
-   case MESA_FORMAT_R8G8B8A8_UNORM:
-  return true;
-   default:
-  return false;
-   }
-}
-
-static bool
-color_formats_match(mesa_format src_format, mesa_format dst_format)
-{
-   mesa_format linear_src_format = _mesa_get_srgb_format_linear(src_format);
-   mesa_format linear_dst_format = _mesa_get_srgb_format_linear(dst_format);
-
-   /* Normally, we require the formats to be equal. However, we also support
-* blitting from ARGB to XRGB (discarding alpha), and from XRGB to ARGB
-* (overriding alpha to 1.0 via blending) as well as swizzling between BGR
-* and RGB.
-*/
-
-   return (linear_src_format == linear_dst_format ||
-   (format_is_rgba_or_rgbx_byte(linear_src_format) 
-format_is_rgba_or_rgbx_byte(linear_dst_format)));
-}
-
-static bool
-formats_match(GLbitfield buffer_bit, struct intel_renderbuffer *src_irb,
-  struct intel_renderbuffer *dst_irb)
-{
-   /* Note: don't just check gl_renderbuffer::Format, because in some cases
-* multiple gl_formats resolve to the same native type in the miptree (for
-* example MESA_FORMAT_Z24_UNORM_X8_UINT and 
MESA_FORMAT_Z24_UNORM_S8_UINT), and we can blit
-* between those formats.
-*/
-   mesa_format src_format = find_miptree(buffer_bit, src_irb)-format;
-   mesa_format dst_format = find_miptree(buffer_bit, dst_irb)-format;
-
-   return color_formats_match(src_format, dst_format);
-}
-
-static bool
 try_blorp_blit(struct brw_context *brw,
GLfloat srcX0, GLfloat srcY0, GLfloat srcX1, GLfloat srcY1,
GLfloat dstX0, GLfloat dstY0, GLfloat dstX1, GLfloat dstY1,
@@ -191,16 +145,13 @@ try_blorp_blit(struct brw_context *brw,
/* Find buffers */
struct intel_renderbuffer *src_irb;
struct intel_renderbuffer *dst_irb;
+   struct intel_mipmap_tree *src_mt;
+   struct intel_mipmap_tree *dst_mt;
switch (buffer_bit) {
case GL_COLOR_BUFFER_BIT:
   src_irb = intel_renderbuffer(read_fb-_ColorReadBuffer);
   for (unsigned i = 0; i  ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
  dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
- if (dst_irb  !formats_match(buffer_bit, src_irb, dst_irb))
-return false;
-  }
-  for (unsigned i = 0; i  ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
- dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
 if (dst_irb)
 do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
   srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
@@ -212,8 +163,17 @@ try_blorp_blit(struct brw_context *brw,
  intel_renderbuffer(read_fb-Attachment[BUFFER_DEPTH].Renderbuffer);
   dst_irb =
  intel_renderbuffer(draw_fb-Attachment[BUFFER_DEPTH].Renderbuffer);
-  if (!formats_match(buffer_bit, src_irb, dst_irb))
+  src_mt = find_miptree(buffer_bit, src_irb);
+  dst_mt = find_miptree(buffer_bit, dst_irb);
+
+  /* We can't handle format conversions between Z24 and other formats
+   * since we have to lie about the surface format. See the 

[Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-06-23 Thread Neil Roberts
Previously the blorp blitter would only be used if the format is identical or
there is only a difference between whether there is an alpha component or not.
This patch makes it also allow the blorp blitter if the only difference is the
ordering of the RGB components (ie, RGB or BGR).

This is particularly useful since commit 61e264f4fcdba3623 because Mesa now
prefers RGB ordering for textures but the window system buffers are still
created as BGR. That means that the blorp blitter won't be used for the
(probably) common case of blitting from a texture to the window system buffer.

This doesn't cause any regressions in the FBO piglit tests on Haswell. On
Sandybridge it causes the fbo-blit-stretch test to fail but that is only
because it was failing anyway before the above commit and that commit hid the
problem.

https://bugs.freedesktop.org/show_bug.cgi?id=68365
---
 src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 28 +---
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index 118af27..d7f5f7d 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -120,20 +120,34 @@ do_blorp_blit(struct brw_context *brw, GLbitfield 
buffer_bit,
 }
 
 static bool
+format_is_rgba_or_rgbx_byte(mesa_format format)
+{
+   switch (format) {
+   case MESA_FORMAT_B8G8R8X8_UNORM:
+   case MESA_FORMAT_B8G8R8A8_UNORM:
+   case MESA_FORMAT_R8G8B8X8_UNORM:
+   case MESA_FORMAT_R8G8B8A8_UNORM:
+  return true;
+   default:
+  return false;
+   }
+}
+
+static bool
 color_formats_match(mesa_format src_format, mesa_format dst_format)
 {
mesa_format linear_src_format = _mesa_get_srgb_format_linear(src_format);
mesa_format linear_dst_format = _mesa_get_srgb_format_linear(dst_format);
 
-   /* Normally, we require the formats to be equal.  However, we also support
+   /* Normally, we require the formats to be equal. However, we also support
 * blitting from ARGB to XRGB (discarding alpha), and from XRGB to ARGB
-* (overriding alpha to 1.0 via blending).
+* (overriding alpha to 1.0 via blending) as well as swizzling between BGR
+* and RGB.
 */
-   return linear_src_format == linear_dst_format ||
-  (linear_src_format == MESA_FORMAT_B8G8R8X8_UNORM 
-   linear_dst_format == MESA_FORMAT_B8G8R8A8_UNORM) ||
-  (linear_src_format == MESA_FORMAT_B8G8R8A8_UNORM 
-   linear_dst_format == MESA_FORMAT_B8G8R8X8_UNORM);
+
+   return (linear_src_format == linear_dst_format ||
+   (format_is_rgba_or_rgbx_byte(linear_src_format) 
+format_is_rgba_or_rgbx_byte(linear_dst_format)));
 }
 
 static bool
-- 
1.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-06-23 Thread Matt Turner
On Mon, Jun 23, 2014 at 11:02 AM, Neil Roberts n...@linux.intel.com wrote:
 Previously the blorp blitter would only be used if the format is identical or
 there is only a difference between whether there is an alpha component or not.
 This patch makes it also allow the blorp blitter if the only difference is the
 ordering of the RGB components (ie, RGB or BGR).

 This is particularly useful since commit 61e264f4fcdba3623 because Mesa now
 prefers RGB ordering for textures but the window system buffers are still
 created as BGR. That means that the blorp blitter won't be used for the
 (probably) common case of blitting from a texture to the window system buffer.

 This doesn't cause any regressions in the FBO piglit tests on Haswell. On
 Sandybridge it causes the fbo-blit-stretch test to fail but that is only
 because it was failing anyway before the above commit and that commit hid the
 problem.

 https://bugs.freedesktop.org/show_bug.cgi?id=68365

We typically write

Bugzilla: https:///

Reviewed-by: Matt Turner matts...@gmail.com

Do you have commit access?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-06-23 Thread Neil Roberts
Matt Turner matts...@gmail.com writes:

 We typically write

 Bugzilla: https:///

 Reviewed-by: Matt Turner matts...@gmail.com

 Do you have commit access?

Thanks for the review. I do have commit access so I've pushed the patch
with the suggested change to the commit message.

I've also started to try and look at why that bug is happening on
SandyBridge.

Regards,
- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-06-23 Thread Matt Turner
On Mon, Jun 23, 2014 at 12:06 PM, Neil Roberts n...@linux.intel.com wrote:
 Matt Turner matts...@gmail.com writes:

 We typically write

 Bugzilla: https:///

 Reviewed-by: Matt Turner matts...@gmail.com

 Do you have commit access?

 Thanks for the review. I do have commit access so I've pushed the patch
 with the suggested change to the commit message.

Excellent. Thanks Neil!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB

2014-06-23 Thread Kenneth Graunke
On Monday, June 23, 2014 07:02:59 PM Neil Roberts wrote:
 Previously the blorp blitter would only be used if the format is identical 
or
 there is only a difference between whether there is an alpha component or 
not.
 This patch makes it also allow the blorp blitter if the only difference is 
the
 ordering of the RGB components (ie, RGB or BGR).
 
 This is particularly useful since commit 61e264f4fcdba3623 because Mesa now
 prefers RGB ordering for textures but the window system buffers are still
 created as BGR. That means that the blorp blitter won't be used for the
 (probably) common case of blitting from a texture to the window system 
buffer.
 
 This doesn't cause any regressions in the FBO piglit tests on Haswell. On
 Sandybridge it causes the fbo-blit-stretch test to fail but that is only
 because it was failing anyway before the above commit and that commit hid 
the
 problem.
 
 https://bugs.freedesktop.org/show_bug.cgi?id=68365

FWIW, I relaxed the format restrictions in brw_blorp_copytexsubimage, so it 
can handle general format conversions as well (i.e. RGBA_FLOAT16 - 
RGBA_UNORM).  There's no reason we couldn't do that for BlitFramebuffer as 
well, I just forgot to do it (and then we decided to make it a newbie task, 
and then the newbie didn't do said task, and...oops.)

A couple of games do BlitFramebuffer with format conversions, IIRC.  I think 
DOTA 2 does 1010102 -  blits.

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev