Re: [Mesa-dev] [PATCH 7/8] i965/fs: Add support for TXD with shadow comparisons on gen4-6.

2011-06-15 Thread Kenneth Graunke

On 06/15/2011 08:27 AM, Eric Anholt wrote:

On Wed, 15 Jun 2011 01:24:54 -0700, Kenneth Graunkekenn...@whitecape.org  
wrote:

Gen4-6 don't have a sample_d_c message, so we have to do a regular
sample_d and emit instructions to manually perform the comparison.

This requires a state dependent recompile whenever ctx-Depth.Func
changes.  do_wm_prog looks for a compiled program in the cache based off
of brw_wm_prog_key, and if it doesn't find one, recompiles.  So we
simply need to add the depth comparison function to the key; the
_NEW_DEPTH dirty bit was already in-place.



diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 03687ce..67344bd 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -744,7 +744,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, 
fs_reg coordinate,
 }
 mlen += ir-coordinate-type-vector_elements * reg_width;

-   if (ir-shadow_comparitor) {
+   if (ir-shadow_comparitor  ir-op != ir_txd) {
mlen = MAX2(mlen, header_present + 4 * reg_width);

this-result = reg_undef;
@@ -934,6 +934,21 @@ fs_visitor::visit(ir_texture *ir)
 int sampler = _mesa_get_sampler_uniform_value(ir-sampler, prog,fp-Base);
 sampler = fp-Base.SamplerUnits[sampler];

+   /* Pre-Ivybridge doesn't have a sample_d_c message, so shadow compares
+* for textureGrad/TXD need to be emulated with instructions.
+*/
+   bool hw_compare_supported = ir-op != ir_txd || intel-gen  7;
+   if (ir-shadow_comparitor  !hw_compare_supported) {
+  /* Mark that this program is only valid for the current glDepthFunc */
+  c-key.depth_compare_func = ctx-Depth.Func;


The compiler is only called if the key wasn't found in the cache, so you
can't set the key inside the compiler :)


You're right, of course.  Unfortunately, I really want to!  Only shaders 
using shadow2DGradARB need to be recompiled when glDepthFunc changes, so 
I really want to leave it as 0 for most shaders.  Otherwise the common 
case will have completely pointless recompiles.


Unfortunately, I only know whether or not I need it at compile time...

This -almost- works...it just means we'll recompile shaders using 
shadow2DGradARB every time, since they'll never be found in the cache. 
Stupid.  I'll reply with a patch which works around this.



+
+  /* No need to even sample for GL_ALWAYS or GL_NEVER...bail early */
+  if (ctx-Depth.Func == GL_ALWAYS)
+return swizzle_shadow_result(ir, fs_reg(1.0f), sampler);
+  else if (ctx-Depth.Func == GL_NEVER)
+return swizzle_shadow_result(ir, fs_reg(0.0f), sampler);
+   }


Generally I use c-key. values to make it obvious that state dependent
recompiles are in place, instead of ctx-.


Good idea.  Updated.


diff --git a/src/mesa/drivers/dri/i965/brw_wm.h 
b/src/mesa/drivers/dri/i965/brw_wm.h
index e244b55..0b284b6 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.h
+++ b/src/mesa/drivers/dri/i965/brw_wm.h
@@ -67,6 +67,7 @@ struct brw_wm_prog_key {
 GLuint alpha_test:1;
 GLuint clamp_fragment_color:1;
 GLuint line_aa:2;
+   uint8_t depth_compare_func; /* 0 if irrelevant, GL_LESS etc. otherwise */


Wow, those do happen to fit in 8 bits.  Might comment that explicitly --
leaving out bits when storing GLenums has been a common bug and seeing
things like this make me paranoid and go check :)


:) Makes sense; commented.  I figured we really want to pack the key.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7/8] i965/fs: Add support for TXD with shadow comparisons on gen4-6.

2011-06-15 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 06/15/2011 11:00 AM, Kenneth Graunke wrote:
 On 06/15/2011 08:27 AM, Eric Anholt wrote:
 On Wed, 15 Jun 2011 01:24:54 -0700, Kenneth
 Graunkekenn...@whitecape.org  wrote:
 Gen4-6 don't have a sample_d_c message, so we have to do a regular
 sample_d and emit instructions to manually perform the comparison.

 This requires a state dependent recompile whenever ctx-Depth.Func
 changes.  do_wm_prog looks for a compiled program in the cache based off
 of brw_wm_prog_key, and if it doesn't find one, recompiles.  So we
 simply need to add the depth comparison function to the key; the
 _NEW_DEPTH dirty bit was already in-place.

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index 03687ce..67344bd 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -744,7 +744,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir,
 fs_reg dst, fs_reg coordinate,
  }
  mlen += ir-coordinate-type-vector_elements * reg_width;

 -   if (ir-shadow_comparitor) {
 +   if (ir-shadow_comparitor  ir-op != ir_txd) {
 mlen = MAX2(mlen, header_present + 4 * reg_width);

 this-result = reg_undef;
 @@ -934,6 +934,21 @@ fs_visitor::visit(ir_texture *ir)
  int sampler = _mesa_get_sampler_uniform_value(ir-sampler,
 prog,fp-Base);
  sampler = fp-Base.SamplerUnits[sampler];

 +   /* Pre-Ivybridge doesn't have a sample_d_c message, so shadow
 compares
 +* for textureGrad/TXD need to be emulated with instructions.
 +*/
 +   bool hw_compare_supported = ir-op != ir_txd || intel-gen  7;
 +   if (ir-shadow_comparitor  !hw_compare_supported) {
 +  /* Mark that this program is only valid for the current
 glDepthFunc */
 +  c-key.depth_compare_func = ctx-Depth.Func;

 The compiler is only called if the key wasn't found in the cache, so you
 can't set the key inside the compiler :)
 
 You're right, of course.  Unfortunately, I really want to!  Only shaders
 using shadow2DGradARB need to be recompiled when glDepthFunc changes, so
 I really want to leave it as 0 for most shaders.  Otherwise the common
 case will have completely pointless recompiles.
 
 Unfortunately, I only know whether or not I need it at compile time...
 
 This -almost- works...it just means we'll recompile shaders using
 shadow2DGradARB every time, since they'll never be found in the cache.
 Stupid.  I'll reply with a patch which works around this.

We could always add a flag in the gl_shader_program that the program
uses TXD.  You could force c-key.depth_compare_func to 0 whenever that
flag is zero.  Or we could put a similar flag in the brw_fs structure
and do a preprocessing step to set it.  Right?

 +
 +  /* No need to even sample for GL_ALWAYS or GL_NEVER...bail
 early */
 +  if (ctx-Depth.Func == GL_ALWAYS)
 + return swizzle_shadow_result(ir, fs_reg(1.0f), sampler);
 +  else if (ctx-Depth.Func == GL_NEVER)
 + return swizzle_shadow_result(ir, fs_reg(0.0f), sampler);
 +   }

 Generally I use c-key. values to make it obvious that state dependent
 recompiles are in place, instead of ctx-.
 
 Good idea.  Updated.
 
 diff --git a/src/mesa/drivers/dri/i965/brw_wm.h
 b/src/mesa/drivers/dri/i965/brw_wm.h
 index e244b55..0b284b6 100644
 --- a/src/mesa/drivers/dri/i965/brw_wm.h
 +++ b/src/mesa/drivers/dri/i965/brw_wm.h
 @@ -67,6 +67,7 @@ struct brw_wm_prog_key {
  GLuint alpha_test:1;
  GLuint clamp_fragment_color:1;
  GLuint line_aa:2;
 +   uint8_t depth_compare_func; /* 0 if irrelevant, GL_LESS etc.
 otherwise */

 Wow, those do happen to fit in 8 bits.  Might comment that explicitly --
 leaving out bits when storing GLenums has been a common bug and seeing
 things like this make me paranoid and go check :)
 
 :) Makes sense; commented.  I figured we really want to pack the key.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk34/DQACgkQX1gOwKyEAw9KsACgk/L9OPEOdduhPeaYTyFyEK4U
L04Ani+R/agfr/dxurd0+poca4EP1Uru
=b63p
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev