[Mesa-dev] [PATCH 3/4] radeon, r200: allow hyperz for radeon DRM module v2

[Roland Scheidegger]

Am 10.07.2015 um 19:41 schrieb Emil Velikov:
> On 10 July 2015 at 13:18, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 10.07.2015 um 05:44 schrieb Michel Dänzer:
>>> On 10.07.2015 05:13, Emil Velikov wrote:
>>>> The original code only half considered hyperz as an option. As per
>>>> previous commit "major != 2 cannot occur" we can simply things, and
>>>> allow users to set the option if they choose to do so.
>>>>
>>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>> ---
>>>>  src/mesa/drivers/dri/r200/r200_context.c     | 10 ++--------
>>>>  src/mesa/drivers/dri/radeon/radeon_context.c |  9 ++-------
>>>>  2 files changed, 4 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/r200/r200_context.c b/src/mesa/drivers/dri/r200/r200_context.c
>>>> index 40cc50a..2a42ab3 100644
>>>> --- a/src/mesa/drivers/dri/r200/r200_context.c
>>>> +++ b/src/mesa/drivers/dri/r200/r200_context.c
>>>> @@ -225,14 +225,8 @@ GLboolean r200CreateContext( gl_api api,
>>>>     rmesa->radeon.initialMaxAnisotropy = driQueryOptionf(&rmesa->radeon.optionCache,
>>>>                                                      "def_max_anisotropy");
>>>>
>>>> -   if ( sPriv->drm_version.major == 1
>>>> -       && driQueryOptionb( &rmesa->radeon.optionCache, "hyperz" ) ) {
>>>> -      if ( sPriv->drm_version.minor < 13 )
>>>> -     fprintf( stderr, "DRM version 1.%d too old to support HyperZ, "
>>>> -                      "disabling.\n", sPriv->drm_version.minor );
>>>> -      else
>>>> -     rmesa->using_hyperz = GL_TRUE;
>>>> -   }
>>>
>>> This code only set rmesa->using_hyperz = GL_TRUE if
>>> sPriv->drm_version.major == 1. It was disabled for KMS in commit
>>> e541845959761e9f47d14ade6b58a32db04ef7e4 ("r200: Fix piglit paths test.").
>>>
>>>
>>>> +   if (driQueryOptionb( &rmesa->radeon.optionCache, "hyperz"))
>>>> +      rmesa->using_hyperz = GL_TRUE;
>>>
>>> This enables it again for KMS. Maybe that's okay though, especially if
>>> the driconf option is disabled by default.
>>
>>
>> Oh you're right. The reason given though why it was disabled looks bogus
>> to me ("Piglit doesn't like HyperZ warning so disable it for kms." ???),
>> and I can't see why that would have only applied to r200, not r100. So
>> it should be fine. (Of course, you will get more failures with that
>> enabled with piglit, some things just plain won't work, but that was
>> just the case with UMS too, and the reason why it never was enabled by
>> default.)
>>
> Yes without Roland's knowledge if hyperz is supposed to work for KMS
> the current code is quite ambiguous. If you guys prefer I can simply
> rip out the whole thing, then again hyperz is disabled by default so
> no harm should follow with this patch.
> 
> I don't mind either way.
> Emil
> 

I'd say keep the option (for both drivers) for now.
I've got some r200 in a dust bin actually, haven't touched it in years...
I think it should work in the same way as it does on r100.
The depth buffer metadata (i.e. the bits saying if a tile is compressed
or not etc.) is really fixed onchip cache, and there's no attempt to
grab or restore that data when the depth buffer is changed, which
obviously isn't quite right... The other limitation is that you cannot
read or write the depth buffer directly (well you can but you get back
garbage - not being able to do a glReadPixel on the depth buffer alone
will cause lots of piglit failures). UMS could also do fast z clear,
this is pretty simple as the command would just set the on-chip tile
bits to the "cleared" state, but this never made it to KMS - the rest of
hyperz should work without this, however, though I'm not entirely sure.
IIRC the r300 has actually pretty much the same hw limitations, however
the driver there fixes these issues (though I think that chip had some
nicer way of fast z clear).

Roland