CELL: fix several stencil problems

This small set of changes repairs several different stenciling problems;
now redbook/stencil also runs correctly (and maybe others - I haven't
checked everything yet).

- The number of instructions that had been allocated for fragment ops
  used to be 64 (in cell/common.h).  With complicated stencil use, we
  managed to get up to 93, which caused a segfault before we noticed
  we'd overran our memory buffer.  It's now been bumped to 128,
  which should be enough for even complicated stencil and fragment op
  usage.

- The status of cell surfaces never changed beyond the initial
  PIPE_SURFACE_STATUS_UNDEFINED.  When a user called glClear()
  to clear just the Z buffer (but not the stencil buffer), this caused
  the check_clear_depth_with_quad() function to return false (because
  the surface status was believed to be undefined), and so the device
  was instructed to clear the whole buffer (including the stencil buffer),
  instead of correctly using a quad to clear just the depth, leaving the
  stencil alone.

  This has been fixed similarly to the way the i915 driver handles
  the surface status: during cell_clear_surface(), the status is
  set to PIPE_SURFACE_STATUS_DEFINED.  Then a partial buffer clear is
  handled with a quad, as expected.  Note that we are *not* using
  PIPE_SURFACE_STATUS_CLEAR (also similar to the i915); technically,
  we should be setting the surface status to CLEAR on a clear, and
  to DEFINED when we actually draw something (say on cell_vbuf_draw()),
  but it's difficult to figure out exactly which surfaces are affected
  by a cell_vbuf_draw(), so for now we're doing the easy thing.

- The fragment ops handling was very clever about only pulling out the
  parts of the Z/stencil buffer that it needed for calculations;
  but this failed when only part of the buffer was written, because
  the part that was never pulled out was inadvertently cleared.

  Now all the data from the combined Z/stencil buffer is pulled out,
  just so the proper values can be recombined later and written back
  to the buffer correctly.  As a bonus, the fragment op code generation
  is simplified.

diff --git a/src/gallium/drivers/cell/common.h b/src/gallium/drivers/cell/common.h
index 23fb0b0831d5f21425938feafd72b401aebcf529..87488ea2d70904dad6e6ef5765dceaa554b74267 100644 (file)
--- a/src/gallium/drivers/cell/common.h
+++ b/src/gallium/drivers/cell/common.h
@@ -122,7 +122,7 @@
 #define CELL_DEBUG_CACHE                (1 << 6)
 
 /** Max instructions for doing per-fragment operations */
-#define SPU_MAX_FRAGMENT_OPS_INSTS 64
+#define SPU_MAX_FRAGMENT_OPS_INSTS 128
 
 
 

diff --git a/src/gallium/drivers/cell/ppu/cell_clear.c b/src/gallium/drivers/cell/ppu/cell_clear.c
index c9c0c721bbe5d812591791b9f319c11f219ddb3d..037635e4660b8f7ef85c589ecd96b841ea3aa207 100644 (file)
--- a/src/gallium/drivers/cell/ppu/cell_clear.c
+++ b/src/gallium/drivers/cell/ppu/cell_clear.c
@@ -106,4 +106,17 @@ cell_clear_surface(struct pipe_context *pipe, struct pipe_surface *ps,
       clr->surface = surfIndex;
       clr->value = clearValue;
    }
+
+   /* Technically, the surface's contents are now known and cleared,
+    * so we could set the status to PIPE_SURFACE_STATUS_CLEAR.  But
+    * it turns out it's quite painful to recognize when any particular
+    * surface goes from PIPE_SURFACE_STATUS_CLEAR to 
+    * PIPE_SURFACE_STATUS_DEFINED (i.e. with known contents), because
+    * the drawing commands could be operating on numerous draw buffers,
+    * which we'd have to iterate through to set all their stati...
+    * For now, we cheat a bit and set the surface's status to DEFINED
+    * right here.  Later we should revisit this and set the status to
+    * CLEAR here, and find a better place to set the status to DEFINED.
+    */
+   ps->status = PIPE_SURFACE_STATUS_DEFINED;
 }

diff --git a/src/gallium/drivers/cell/ppu/cell_gen_fragment.c b/src/gallium/drivers/cell/ppu/cell_gen_fragment.c
index 6e2a5d2980413aa83ddb0208ba9cb7fb0698ddb6..d9c3ff3f4d0df635f5d7faa7909542cb5cf2feb9 100644 (file)
--- a/src/gallium/drivers/cell/ppu/cell_gen_fragment.c
+++ b/src/gallium/drivers/cell/ppu/cell_gen_fragment.c
@@ -1997,81 +1997,79 @@ cell_gen_fragment_function(struct cell_context *cell, struct spe_function *f)
        * Z and/or stencil.  We'll also convert the incoming fragment Z
        * value in fragZ_reg from a floating point value in [0.0..1.0] to
        * an unsigned integer value with the appropriate resolution.
+       * Note that even if depth or stencil is *not* enabled, if it's
+       * present in the buffer, we pull it out and put it back later;
+       * otherwise, we can inadvertently destroy the contents of
+       * buffers we're not supposed to touch (e.g., if the user is
+       * clearing the depth buffer but not the stencil buffer, a
+       * quad of constant depth is drawn over the surface; the stencil
+       * buffer must be maintained).
        */
       switch(zs_format) {
 
          case PIPE_FORMAT_S8Z24_UNORM: /* fall through */
          case PIPE_FORMAT_X8Z24_UNORM:
-            if (dsa->depth.enabled) {
-               /* We need the Z part at least */
-               setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
-               /* four 24-bit Z values in the low-order bits */
-               spe_and_uint(f, fbZ_reg, fbZS_reg, 0x00ffffff);
-
-               /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
-                * to a 24-bit unsigned integer
-                */
-               spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
-               spe_rotmi(f, fragZ_reg, fragZ_reg, -8);
-            }
-            if (dsa->stencil[0].enabled) {
-               setup_optional_register(f, &fbS_reg_set, &fbS_reg);
-               /* four 8-bit Z values in the high-order bits */
-               spe_rotmi(f, fbS_reg, fbZS_reg, -24);
-            }
-            break;
+            /* Pull out both Z and stencil */
+            setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
+            setup_optional_register(f, &fbS_reg_set, &fbS_reg);
+
+            /* four 24-bit Z values in the low-order bits */
+            spe_and_uint(f, fbZ_reg, fbZS_reg, 0x00ffffff);
+
+            /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
+             * to a 24-bit unsigned integer
+             */
+            spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
+            spe_rotmi(f, fragZ_reg, fragZ_reg, -8);
+
+            /* four 8-bit stencil values in the high-order bits */
+            spe_rotmi(f, fbS_reg, fbZS_reg, -24);
+         break;
 
          case PIPE_FORMAT_Z24S8_UNORM: /* fall through */
          case PIPE_FORMAT_Z24X8_UNORM:
-            if (dsa->depth.enabled) {
-               setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
-               /* shift by 8 to get the upper 24-bit values */
-               spe_rotmi(f, fbS_reg, fbZS_reg, -8);
-
-               /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
-                * to a 24-bit unsigned integer
-                */
-               spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
-               spe_rotmi(f, fragZ_reg, fragZ_reg, -8);
-            }
-            if (dsa->stencil[0].enabled) {
-               setup_optional_register(f, &fbS_reg_set, &fbS_reg);
-               /* 8-bit stencil in the low-order bits - mask them out */
-               spe_and_uint(f, fbS_reg, fbZS_reg, 0x000000ff);
-            }
-            break;
+            setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
+            setup_optional_register(f, &fbS_reg_set, &fbS_reg);
+
+            /* shift by 8 to get the upper 24-bit values */
+            spe_rotmi(f, fbS_reg, fbZS_reg, -8);
+
+            /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
+             * to a 24-bit unsigned integer
+             */
+            spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
+            spe_rotmi(f, fragZ_reg, fragZ_reg, -8);
+
+            /* 8-bit stencil in the low-order bits - mask them out */
+            spe_and_uint(f, fbS_reg, fbZS_reg, 0x000000ff);
+         break;
 
          case PIPE_FORMAT_Z32_UNORM:
-            if (dsa->depth.enabled) {
-               setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
-               /* Copy over 4 32-bit values */
-               spe_move(f, fbZ_reg, fbZS_reg);
-
-               /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
-                * to a 32-bit unsigned integer
-                */
-               spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
-            }
+            setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
+            /* Copy over 4 32-bit values */
+            spe_move(f, fbZ_reg, fbZS_reg);
+
+            /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
+             * to a 32-bit unsigned integer
+             */
+            spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
             /* No stencil, so can't do anything there */
-            break;
+         break;
 
          case PIPE_FORMAT_Z16_UNORM:
-            if (dsa->depth.enabled) {
-               /* XXX Not sure this is correct, but it was here before, so we're
-                * going with it for now
-                */
-               setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
-               /* Copy over 4 32-bit values */
-               spe_move(f, fbZ_reg, fbZS_reg);
-
-               /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
-                * to a 16-bit unsigned integer
-                */
-               spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
-               spe_rotmi(f, fragZ_reg, fragZ_reg, -16);
-            }
+            /* XXX Not sure this is correct, but it was here before, so we're
+             * going with it for now
+             */
+            setup_optional_register(f, &fbZ_reg_set, &fbZ_reg);
+            /* Copy over 4 32-bit values */
+            spe_move(f, fbZ_reg, fbZS_reg);
+
+            /* Incoming fragZ_reg value is a float in 0.0...1.0; convert
+             * to a 16-bit unsigned integer
+             */
+            spe_cfltu(f, fragZ_reg, fragZ_reg, 32);
+            spe_rotmi(f, fragZ_reg, fragZ_reg, -16);
             /* No stencil */
-            break;
 
          default:
             ASSERT(0); /* invalid format */
@@ -2118,39 +2116,19 @@ cell_gen_fragment_function(struct cell_context *cell, struct spe_function *f)
          spe_comment(f, 0, "Store quad's depth/stencil values in tile");
          if (zs_format == PIPE_FORMAT_S8Z24_UNORM ||
              zs_format == PIPE_FORMAT_X8Z24_UNORM) {
-            if (fbS_reg_set && fbZ_reg_set) {
-               spe_shli(f, fbS_reg, fbS_reg, 24); /* fbS = fbS << 24 */
-               spe_or(f, fbZS_reg, fbS_reg, fbZ_reg); /* fbZS = fbS | fbZ */
-            }
-            else if (fbS_reg_set) {
-               spe_shli(f, fbZS_reg, fbS_reg, 24); /* fbS = fbS << 24 */
-            }
-            else {
-               spe_move(f, fbZS_reg, fbZ_reg);
-            }
+            spe_shli(f, fbS_reg, fbS_reg, 24); /* fbS = fbS << 24 */
+            spe_or(f, fbZS_reg, fbS_reg, fbZ_reg); /* fbZS = fbS | fbZ */
          }
          else if (zs_format == PIPE_FORMAT_Z24S8_UNORM ||
                   zs_format == PIPE_FORMAT_Z24X8_UNORM) {
-            if (fbS_reg_set && fbZ_reg_set) {
-               spe_shli(f, fbZ_reg, fbZ_reg, 8); /* fbZ = fbZ << 8 */
-               spe_or(f, fbZS_reg, fbS_reg, fbZ_reg); /* fbZS = fbS | fbZ */
-            }
-            else if (fbS_reg_set) {
-               spe_move(f, fbZS_reg, fbS_reg);
-            }
-            else {
-               spe_shli(f, fbZ_reg, fbZ_reg, 8); /* fbZ = fbZ << 8 */
-            }
+            spe_shli(f, fbZ_reg, fbZ_reg, 8); /* fbZ = fbZ << 8 */
+            spe_or(f, fbZS_reg, fbS_reg, fbZ_reg); /* fbZS = fbS | fbZ */
          }
          else if (zs_format == PIPE_FORMAT_Z32_UNORM) {
-            if (fbZ_reg_set) {
-               spe_move(f, fbZS_reg, fbZ_reg); /* fbZS = fbZ */
-            }
+            spe_move(f, fbZS_reg, fbZ_reg); /* fbZS = fbZ */
          }
          else if (zs_format == PIPE_FORMAT_Z16_UNORM) {
-            if (fbZ_reg_set) {
-               spe_move(f, fbZS_reg, fbZ_reg); /* fbZS = fbZ */
-            }
+            spe_move(f, fbZS_reg, fbZ_reg); /* fbZS = fbZ */
          }
          else if (zs_format == PIPE_FORMAT_S8_UNORM) {
             ASSERT(0);   /* XXX to do */
@@ -2163,6 +2141,7 @@ cell_gen_fragment_function(struct cell_context *cell, struct spe_function *f)
          spe_stqx(f, fbZS_reg, depth_tile_reg, quad_offset_reg);
       }
 
+      /* Don't need these any more */
       release_optional_register(f, &fbZ_reg_set, fbZ_reg);
       release_optional_register(f, &fbS_reg_set, fbS_reg);
    }