Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2

2014-11-20 Thread Ian Romanick
On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 Check that the target is GL_TEXTURE_CUBE_MAP before emitting
 TEXCOORDTYPE_VECTOR texture coordinates.
 
 I'm not sure if the hardware would like CARTESIAN coordinates
 with cube maps, and as I'm too lazy to find out just emit the
 VECTOR coordinates for cube maps always. For other targets use
 CARTESIAN or HOMOGENOUS depending on the number of texture
 coordinates provided.
 
 Fixes rendering of the electric background texture in chromium-bsu
 main menu. We appear to be provided with three texture coordinates
 there (I'm guessing due to the funky texture matrix rotation it does).
 So the code would decide to use TEXCOORDTYPE_VECTOR instead of
 TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
 The results weren't what one might expect.
 
 demos/cubemap still works, which hopefully indicates that this doesn't
 break things.

I won't dare ask about a full piglit run on that hardware, but how about

bin/glean -o -v -v -v -t +texCube --quick

and

bin/cubemap -auto

from piglit?

These changes seem reasonable, and assuming those tests aren't made worse,

Reviewed-by: Ian Romanick ian.d.roman...@intel.com

If you're excited about GEN2 bugs, wanna take a look at 79841? :)

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  src/mesa/drivers/dri/i915/i830_vtbl.c | 37 
 ++-
  1 file changed, 19 insertions(+), 18 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c 
 b/src/mesa/drivers/dri/i915/i830_vtbl.c
 index 53d408b..0f22d86 100644
 --- a/src/mesa/drivers/dri/i915/i830_vtbl.c
 +++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
 @@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
  GLuint mcs = (i830-state.Tex[i][I830_TEXREG_MCS] 
~TEXCOORDTYPE_MASK);
  
 -switch (sz) {
 -case 1:
 -case 2:
 -   emit = EMIT_2F;
 -   sz = 2;
 -   mcs |= TEXCOORDTYPE_CARTESIAN;
 -   break;
 -case 3:
 +if (intel-ctx.Texture.Unit[i]._Current-Target == 
 GL_TEXTURE_CUBE_MAP) {
 emit = EMIT_3F;
 sz = 3;
 mcs |= TEXCOORDTYPE_VECTOR;
 -   break;
 -case 4:
 -   emit = EMIT_3F_XYW;
 -   sz = 3;
 -   mcs |= TEXCOORDTYPE_HOMOGENEOUS;
 -   break;
 -default:
 -   continue;
 -};
 -
 +} else {
 +   switch (sz) {
 +   case 1:
 +   case 2:
 +   case 3:
 +  emit = EMIT_2F;
 +  sz = 2;
 +  mcs |= TEXCOORDTYPE_CARTESIAN;
 +  break;
 +   case 4:
 +  emit = EMIT_3F_XYW;
 +  sz = 3;
 +  mcs |= TEXCOORDTYPE_HOMOGENEOUS;
 +  break;
 +   default:
 +  continue;
 +   }
 +}
  
  EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
  v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
 

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


Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2

2014-11-20 Thread Ville Syrjälä
On Thu, Nov 20, 2014 at 09:59:00AM -0800, Ian Romanick wrote:
 On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Check that the target is GL_TEXTURE_CUBE_MAP before emitting
  TEXCOORDTYPE_VECTOR texture coordinates.
  
  I'm not sure if the hardware would like CARTESIAN coordinates
  with cube maps, and as I'm too lazy to find out just emit the
  VECTOR coordinates for cube maps always. For other targets use
  CARTESIAN or HOMOGENOUS depending on the number of texture
  coordinates provided.
  
  Fixes rendering of the electric background texture in chromium-bsu
  main menu. We appear to be provided with three texture coordinates
  there (I'm guessing due to the funky texture matrix rotation it does).
  So the code would decide to use TEXCOORDTYPE_VECTOR instead of
  TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
  The results weren't what one might expect.
  
  demos/cubemap still works, which hopefully indicates that this doesn't
  break things.
 
 I won't dare ask about a full piglit run on that hardware,

I did actually try to run piglit on the 830, but it always
seemed to die in some X blit batch after a while :(

There have been some recent blt TLB workaround fixes in the
kernel though, so perhaps things are better now. And if not,
I do have a 855 that ought to be a bit less fragile. So
maybe I'll give it another go just for kicks :P

 but how about
 
 bin/glean -o -v -v -v -t +texCube --quick
 
 and
 
 bin/cubemap -auto
 
 from piglit?

pass-pass on both counts.

 
 These changes seem reasonable, and assuming those tests aren't made worse,
 
 Reviewed-by: Ian Romanick ian.d.roman...@intel.com
 
 If you're excited about GEN2 bugs, wanna take a look at 79841? :)

I'm thinking it should be fixed by:

commit dafae910d4fc791ba49f20e937cb918669f42944
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date:   Thu Jul 3 15:38:07 2014 +0300

i915: Accept GL_DEPTH_STENCIL GL_DEPTH_COMPONENT formats for
renderbuffers

I'll note it in the bug.

 
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   src/mesa/drivers/dri/i915/i830_vtbl.c | 37 
  ++-
   1 file changed, 19 insertions(+), 18 deletions(-)
  
  diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c 
  b/src/mesa/drivers/dri/i915/i830_vtbl.c
  index 53d408b..0f22d86 100644
  --- a/src/mesa/drivers/dri/i915/i830_vtbl.c
  +++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
  @@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
   GLuint mcs = (i830-state.Tex[i][I830_TEXREG_MCS] 
 ~TEXCOORDTYPE_MASK);
   
  -switch (sz) {
  -case 1:
  -case 2:
  -   emit = EMIT_2F;
  -   sz = 2;
  -   mcs |= TEXCOORDTYPE_CARTESIAN;
  -   break;
  -case 3:
  +if (intel-ctx.Texture.Unit[i]._Current-Target == 
  GL_TEXTURE_CUBE_MAP) {
  emit = EMIT_3F;
  sz = 3;
  mcs |= TEXCOORDTYPE_VECTOR;
  -   break;
  -case 4:
  -   emit = EMIT_3F_XYW;
  -   sz = 3;
  -   mcs |= TEXCOORDTYPE_HOMOGENEOUS;
  -   break;
  -default:
  -   continue;
  -};
  -
  +} else {
  +   switch (sz) {
  +   case 1:
  +   case 2:
  +   case 3:
  +  emit = EMIT_2F;
  +  sz = 2;
  +  mcs |= TEXCOORDTYPE_CARTESIAN;
  +  break;
  +   case 4:
  +  emit = EMIT_3F_XYW;
  +  sz = 3;
  +  mcs |= TEXCOORDTYPE_HOMOGENEOUS;
  +  break;
  +   default:
  +  continue;
  +   }
  +}
   
   EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
   v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
  

-- 
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2

2014-11-20 Thread Ian Romanick
On 11/20/2014 11:13 AM, Ville Syrjälä wrote:
 On Thu, Nov 20, 2014 at 09:59:00AM -0800, Ian Romanick wrote:
 On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Check that the target is GL_TEXTURE_CUBE_MAP before emitting
 TEXCOORDTYPE_VECTOR texture coordinates.

 I'm not sure if the hardware would like CARTESIAN coordinates
 with cube maps, and as I'm too lazy to find out just emit the
 VECTOR coordinates for cube maps always. For other targets use
 CARTESIAN or HOMOGENOUS depending on the number of texture
 coordinates provided.

 Fixes rendering of the electric background texture in chromium-bsu
 main menu. We appear to be provided with three texture coordinates
 there (I'm guessing due to the funky texture matrix rotation it does).
 So the code would decide to use TEXCOORDTYPE_VECTOR instead of
 TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
 The results weren't what one might expect.

 demos/cubemap still works, which hopefully indicates that this doesn't
 break things.

 I won't dare ask about a full piglit run on that hardware,
 
 I did actually try to run piglit on the 830, but it always
 seemed to die in some X blit batch after a while :(
 
 There have been some recent blt TLB workaround fixes in the
 kernel though, so perhaps things are better now. And if not,
 I do have a 855 that ought to be a bit less fragile. So
 maybe I'll give it another go just for kicks :P
 
 but how about

 bin/glean -o -v -v -v -t +texCube --quick

 and

 bin/cubemap -auto

 from piglit?
 
 pass-pass on both counts.
 

 These changes seem reasonable, and assuming those tests aren't made worse,

 Reviewed-by: Ian Romanick ian.d.roman...@intel.com

 If you're excited about GEN2 bugs, wanna take a look at 79841? :)
 
 I'm thinking it should be fixed by:
 
 commit dafae910d4fc791ba49f20e937cb918669f42944
 Author: Ville Syrjälä ville.syrj...@linux.intel.com
 Date:   Thu Jul 3 15:38:07 2014 +0300
 
 i915: Accept GL_DEPTH_STENCIL GL_DEPTH_COMPONENT formats for
 renderbuffers
 
 I'll note it in the bug.

I only brought it up because there's another report (bug #86480) of what
appears to be the same issue.  The reporter says it's on 10.4.x.

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


[Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2

2014-08-06 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Check that the target is GL_TEXTURE_CUBE_MAP before emitting
TEXCOORDTYPE_VECTOR texture coordinates.

I'm not sure if the hardware would like CARTESIAN coordinates
with cube maps, and as I'm too lazy to find out just emit the
VECTOR coordinates for cube maps always. For other targets use
CARTESIAN or HOMOGENOUS depending on the number of texture
coordinates provided.

Fixes rendering of the electric background texture in chromium-bsu
main menu. We appear to be provided with three texture coordinates
there (I'm guessing due to the funky texture matrix rotation it does).
So the code would decide to use TEXCOORDTYPE_VECTOR instead of
TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
The results weren't what one might expect.

demos/cubemap still works, which hopefully indicates that this doesn't
break things.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 src/mesa/drivers/dri/i915/i830_vtbl.c | 37 ++-
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c 
b/src/mesa/drivers/dri/i915/i830_vtbl.c
index 53d408b..0f22d86 100644
--- a/src/mesa/drivers/dri/i915/i830_vtbl.c
+++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
@@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
 GLuint mcs = (i830-state.Tex[i][I830_TEXREG_MCS] 
   ~TEXCOORDTYPE_MASK);
 
-switch (sz) {
-case 1:
-case 2:
-   emit = EMIT_2F;
-   sz = 2;
-   mcs |= TEXCOORDTYPE_CARTESIAN;
-   break;
-case 3:
+if (intel-ctx.Texture.Unit[i]._Current-Target == 
GL_TEXTURE_CUBE_MAP) {
emit = EMIT_3F;
sz = 3;
mcs |= TEXCOORDTYPE_VECTOR;
-   break;
-case 4:
-   emit = EMIT_3F_XYW;
-   sz = 3;
-   mcs |= TEXCOORDTYPE_HOMOGENEOUS;
-   break;
-default:
-   continue;
-};
-
+} else {
+   switch (sz) {
+   case 1:
+   case 2:
+   case 3:
+  emit = EMIT_2F;
+  sz = 2;
+  mcs |= TEXCOORDTYPE_CARTESIAN;
+  break;
+   case 4:
+  emit = EMIT_3F_XYW;
+  sz = 3;
+  mcs |= TEXCOORDTYPE_HOMOGENEOUS;
+  break;
+   default:
+  continue;
+   }
+}
 
 EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
 v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
-- 
1.8.5.5

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