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

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> 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...

Yeah, I'm not clear on how this ought to work for resolves.  For clears,
the strategy explained for the previous gens made sense: Round down your
coords to get 8x4 alignment, then do slow clears on the remaining strips
(if any).  But for a resolve, what else do you do?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140219/25748fc9/attachment.pgp>