[Mesa-dev] [PATCH] i965: Fix regression in depth texture rendering on pre-SNB

[Ian Romanick]

On 10/20/2012 10:02 PM, Kenneth Graunke wrote:
> On 10/19/2012 02:11 PM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> This was introduced by commit 24db6d6 (cherry-picked from a683012).  The
>> original patch fixed potential GPU hangs on SNB, and it caused some
>> rendering regressions there.  The benefits outweigh the costs.
>>
>> However, the work-around is not necessary for pre-SNB chipsets.
>> Applying the work-around there gives rendering regressions with no
>> benefit.  This patch disables the work-around on pre-SNB chipsets.
>>
>> NOTE: This is a candidate for the 8.0 and 9.0 branches
>
> Well.  The docs have always said:
>
> "The 3 LSBs of both offsets must be zero to ensure correct alignment"
> (G45 Vol 2 Page 280).
>
> So not following that is always wrong, even on pre-Gen6.  But somehow,
> it always seemed to work out...until we turned on HiZ.
>
> I am OK with applying this to the 8.0 branch, as it restores the old
> behavior that we shipped for ages, and shouldn't cause GPU hangs.  You
> can add my Acked-by to that.

I can go along with that.

> However, I am opposed to applying it to the 9.0 branch.  Eric has an
> actual fix for depth rendering to small miplevels, and I'd rather see
> that backported.  I don't know if it's landed yet though.

...which still needs some work.  All of the ILK cases that fail before 
his series crash with it. :(

>> ---
>>   src/mesa/drivers/dri/i965/brw_misc_state.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> index 6dfa08e..4799af2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> @@ -413,8 +413,10 @@ static void emit_depthbuffer(struct brw_context
>> *brw)
>>          * tile_x and tile_y to 0.  This is a temporary workaround
>> until we come
>>          * up with a better solution.
>>          */
>> -      tile_x &= ~7;
>> -      tile_y &= ~7;
>> +      if (intel->gen >= 6) {
>> +         tile_x &= ~7;
>> +         tile_y &= ~7;
>> +      }
>>
>>         BEGIN_BATCH(len);
>>         OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
>