[Mesa-dev] [PATCH 6/8] i965: Implement HiZ resolves on Broadwell.

[Kenneth Graunke]

On 02/18/2014 01:38 PM, Eric Anholt wrote:
> Kenneth Graunke <kenneth at whitecape.org> writes:
[snip]
>> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> index f30ff28..3fa20c8 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> @@ -203,3 +203,108 @@ gen8_emit_depth_stencil_hiz(struct brw_context *brw,
>>                        brw->depthstencil.stencil_offset,
>>                        hiz, width, height, depth, lod, min_array_element);
>>  }
>> +
>> +/**
>> + * Emit packets to perform a depth/HiZ resolve or fast depth/stencil clear.
>> + *
>> + * See the "Optimized Depth Buffer Clear and/or Stencil Buffer Clear" section
>> + * of the hardware documentation for details.
>> + */
>> +void
>> +gen8_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
>> +              unsigned int level, unsigned int layer, enum gen6_hiz_op op)
>> +{
>> +   if (op == GEN6_HIZ_OP_NONE)
>> +      return;
>> +
>> +   assert(mt->first_level == 0);
>> +
>> +   struct intel_mipmap_level *miplevel = &mt->level[level];
>> +
>> +   /* The basic algorithm is:
>> +    * - If needed, emit 3DSTATE_{DEPTH,HIER_DEPTH,STENCIL}_BUFFER and
>> +    *   3DSTATE_CLEAR_PARAMS packets to set up the relevant buffers.
>> +    * - If needed, emit 3DSTATE_DRAWING_RECTANGLE.
>> +    * - Emit 3DSTATE_WM_HZ_OP with a bit set for the particular operation.
>> +    * - Do a special PIPE_CONTROL to trigger an implicit rectangle primitive.
>> +    * - Emit 3DSTATE_WM_HZ_OP with no bits set to return to normal rendering.
>> +    */
>> +   emit_depth_packets(brw, mt,
>> +                      brw_depth_format(brw, mt->format),
>> +                      BRW_SURFACE_2D,
>> +                      true, /* depth writes */
>> +                      NULL, false, 0, /* no stencil for now */
>> +                      true, /* hiz */
>> +                      mt->logical_width0,
>> +                      mt->logical_height0,
>> +                      MAX2(mt->logical_depth0, 1),
> 
> Is logical_depth0 ever 0?  That seems like a bug.

No, I guess it isn't.  It looks like I copy and pasted this from BLORP,
or was being overly cautious for some reason.  Dropped.

>> +                      level,
>> +                      layer); /* min_array_element */
>> +
>> +   BEGIN_BATCH(4);
>> +   OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE << 16 | (4 - 2));
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(((mt->logical_width0 - 1) & 0xffff) |
>> +             ((mt->logical_height0 - 1) << 16));
>> +   OUT_BATCH(0);
>> +   ADVANCE_BATCH();
> 
> The drawing rectangle should be using the level's size, not the level 0
> size.

Yes, this makes sense...we bind a specific miplevel of the depth buffer,
so presumably the (0, 0) origin is the start of that miplevel, not the
start of the whole tree.  I'll change that.

Since the drawing rectangle is just the bounds of where you can draw,
and not actually the clear/resolve rectangle, I think specifying one
that's too large shouldn't be harmful.  But specifying the right value
is trivial, so I agree we should do it.

>> +   uint32_t sample_mask = 0xFFFF;
>> +   if (mt->num_samples > 0) {
>> +      dw1 |= SET_FIELD(ffs(mt->num_samples) - 1, GEN8_WM_HZ_NUM_SAMPLES);
>> +      sample_mask = gen6_determine_sample_mask(brw);
>> +   }
> 
> I don't think we want the user-set sample mask stuff to change the
> samples affected by our hiz/depth resolves.  I think you can just drop
> the if block.

Good point, whatever the user specified is probably unrelated to our
values.  I've dropped the sample_mask variable and just stuffed 0xFFFF
in the packet.

I kept the if-block for the "dw1 |= ...num_samples..." line.

>> +
>> +   BEGIN_BATCH(5);
>> +   OUT_BATCH(_3DSTATE_WM_HZ_OP << 16 | (5 - 2));
>> +   OUT_BATCH(dw1);
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(SET_FIELD(miplevel->width, GEN8_WM_HZ_CLEAR_RECTANGLE_X_MAX) |
>> +             SET_FIELD(miplevel->height, GEN8_WM_HZ_CLEAR_RECTANGLE_Y_MAX));
>> +   OUT_BATCH(SET_FIELD(sample_mask, GEN8_WM_HZ_SAMPLE_MASK));
>> +   ADVANCE_BATCH();
> 
> I think now the miplevel->width should be minify(mt->logical_width0,
> level).  Hope that helped

Yes, that's much nicer - and correct for MSAA buffers!  I'm unclear
whether we need to do:

   ALIGN(minify(mt->logical_width0,  level), 8)
   ALIGN(minify(mt->logical_height0, level), 4)

(both here and in the drawing rectangle)

I've read seemingly contradictory information...it sounds like it might
be necessary for depth resolves, but not otherwise...but I could be
misinterpreting it.  It seems to be working...

>> +
>> +   /* Emit a PIPE_CONTROL with "Post-Sync Operation" set to "Write Immediate
>> +    * Data", and no other bits set.  This causes 3DSTATE_WM_HZ_OP's state to
>> +    * take effect, and spawns a rectangle primitive.
>> +    */
>> +   brw_emit_pipe_control_write(brw,
>> +                               PIPE_CONTROL_WRITE_IMMEDIATE,
>> +                               brw->batch.workaround_bo, 0, 0, 0);
>> +
>> +   /* Emit 3DSTATE_WM_HZ_OP again to disable the state overrides. */
>> +   BEGIN_BATCH(5);
>> +   OUT_BATCH(_3DSTATE_WM_HZ_OP << 16 | (5 - 2));
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(0);
>> +   ADVANCE_BATCH();
>> +
>> +   /* We've clobbered all of the depth packets, and the drawing rectangle,
>> +    * so we need to ensure those packets are re-emitted before the next
>> +    * primitive.
>> +    *
>> +    * Setting _NEW_DEPTH and _NEW_BUFFERS covers it, but is rather overkill.
>> +    */
>> +   brw->state.dirty.mesa |= _NEW_DEPTH | _NEW_BUFFERS;
> 
> I'd rather we just made a new BRW_ flag for this that was watched by the
> two appropriate packets.
> 
> Oh, but we're running dangerously low on BRW_ flags.  I guess I don't
> care that much.

Yep, that would be much nicer...but we only have one or two bits left,
so I didn't (at least for now).  There are a bunch of different
solutions to that problem, but I didn't want to chase bunnies just yet.

> 
> With changes to drawing rectangle, mip->width/height, and sample mask,
> this series is:
> 
> Reviewed-by: Eric Anholt <eric at anholt.net>

Thank you!  I'll go ahead and push this, and if there are further
changes needed (or Chad has comments), I can send new patches to update it.

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140219/3ed2c82c/attachment.pgp>