[Mesa-dev] [PATCH 17/19] i965: After emitting a HiZ buffer, mark that it needs a HiZ resolve

[Chad Versace]

On 09/26/2011 02:38 PM, Eric Anholt wrote:
> On Fri, 23 Sep 2011 17:37:47 -0700, Chad Versace<chad at chad-versace.us>  wrote:
>> Signed-off-by: Chad Versace<chad at chad-versace.us>
>> ---
>>   src/mesa/drivers/dri/i965/brw_misc_state.c |    6 ++++++
>>   1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> index 5adf2d7..1121f61 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> @@ -377,6 +377,8 @@ static void emit_depthbuffer(struct brw_context *brw)
>>         /* Emit hiz buffer. */
>>         if (has_hiz) {
>>   	 struct intel_hiz_control *hiz =&depth_irb->region->hiz;
>> +	 assert(hiz->need_resolve != INTEL_HIZ_NEED_HIZ_RESOLVE);
>> +
>>   	 BEGIN_BATCH(3);
>>   	 OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER<<  16) | (3 - 2));
>>   	 OUT_BATCH(hiz->region->pitch * hiz->region->cpp - 1);
>> @@ -384,6 +386,10 @@ static void emit_depthbuffer(struct brw_context *brw)
>>   		   I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>>   		   0);
>>   	 ADVANCE_BATCH();
>> +
>> +	 if (!brw->hiz.op) {
>> +	    hiz->need_resolve = INTEL_HIZ_NEED_DEPTH_RESOLVE;
>> +	 }
>
> This one makes me uncomfortable, because emit_depthbuffer isn't called
> every single time drawing using the depthbuffer occurs.  However, I
> can't come up with a way to fail:
>
> If you had a swrast-driven depth access between two Render engine draws,
> then there was a batch flush, so the second Render draw will
> emit_depthbuffer() too.
>
> If you had texturing from it, then for there to be defined behavior
> (without whatever that GL extension is), you must have unbound the depth
> buffer iirc, so that will result in an emit_depthbuffer() when
> re-bound.
>
> We don't have anything using the blitter in between draws, because lol Y
> tiling.
>
> So I think this is OK, but I'm still nervous.

Your nervousness is contagious. Now I'm prompted to find a better place
to set `hiz->need_resolve = DEPTH_RESOLVE`.

I think the proper location for the update is brw_try_draw_prims(). If the
DepthMask is enabled and the depthbuffer has hiz, then at the point of
emitting a primitive we know that a depth resolve must be needed.

-- 
Chad Versace
chad at chad-versace.us