Kenneth Graunke <kenn...@whitecape.org> writes:
> On 02/18/2014 01:38 PM, Eric Anholt wrote:
>> Kenneth Graunke <kenn...@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?
pgpFhkUCbiV6h.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
