On 02/19/2014 11:12 AM, Eric Anholt wrote:
> 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?
We only enable HiZ for miplevels that align to 8x4 boundaries.
--Ken
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
