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

[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