[Mesa-dev] [PATCH] i965: Don't use BRW_DEPTHFORMAT_D24_UNORM_X8_UINT on Gen4.

[Kenneth Graunke]

On 12/21/2011 09:56 AM, Chad Versace wrote:
> On 12/21/2011 05:52 AM, Eric Anholt wrote:
>> On Tue, 20 Dec 2011 23:30:29 -0800, Kenneth Graunke <kenneth at whitecape.org> wrote:
>>> X8 depth formats weren't supported until Ironlake (Gen 5).
>>>
>>> Fixes GPU hangs introduced in d84a180417d1eabd680554970f1eaaa93abcd41e.
>>> One example test case was "fbo-missing-attachment-blit from".
>>>
>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_misc_state.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> I tested this by comparing full piglit runs on GM45 between d84a180417d~1
>>> and d84a180417d with this patch applied on top of it.  No changes, no hangs.
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
>>> index 8a6ee70..21885a6 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>>> @@ -218,7 +218,9 @@ brw_depthbuffer_format(struct brw_context *brw)
>>>     case MESA_FORMAT_Z32_FLOAT:
>>>        return BRW_DEPTHFORMAT_D32_FLOAT;
>>>     case MESA_FORMAT_X8_Z24:
>>> -      return BRW_DEPTHFORMAT_D24_UNORM_X8_UINT;
>>> +      if (intel->gen >= 5)
>>> +	 return BRW_DEPTHFORMAT_D24_UNORM_X8_UINT;
>>> +      /* Gen4 doesn't support X8; use S8 instead. */
>>>     case MESA_FORMAT_S8_Z24:
>>>        return BRW_DEPTHFORMAT_D24_UNORM_S8_UINT;
>>>     default:
> 
>> Oh, thanks!  I think an explicit else case would be nicer to read than
>> an implicit fallthrough, though.
> 
> Ditto. Fallthroughs like this are jarring.

Sure, I'm okay with that.  I'll respin.

> While you're digging around in this function, wouldn't this commit
> and the one below be candidates for 7.11?
> 
>     commit d464a21e2d796e396e7e4c0cfe971b41003e2d2a
>     Author: Kenneth Graunke <kenneth at whitecape.org>
>     Date:   Wed Dec 7 02:21:37 2011 -0800
> 
>         i965: Return BRW_DEPTHBUFFER_D32_FLOAT as the null-depthbuffer format.
>         
>         Fixes many crashes on Ivybridge due to upload_sf_state calling
>         brw_depthbuffer_format without an actual depth buffer.  This was a
>         recent regression on master.
>         
>         +3992 piglits on Ivybridge.
>         
>         Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>         Reviewed-by: Eric Anholt <eric at anholt.net>

No, both were fixing regressions caused by refactoring on master.  7.11
shouldn't have been affected.