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

[Alex Deucher]

On Sat, Jul 11, 2015 at 8:48 AM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 11.07.2015 um 11:25 schrieb Marek Olšák:
>> On Fri, Jul 10, 2015 at 11:02 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> 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).
>>
>> Not really, there is just a packet to clear the on-chip tile bits and
>> we always clear the whole surface.
> Yes but for r100/r200 I don't know if such a packet exists. The kernel
> did all the effort for figuring out the actual on-chip addresses to
> clear (of course, with the shared z buffer of late, that way actually
> multiple apps ran with hyperz enabled in parallel correctly, as long as
> they were placed "far enough apart" on the screen (so the tiles wouldn't
> intersect).
> It is possible though such a packet exists on r100/r200 too, and we just
> never figured it out :-).

If there wasn't a special packet, I think you could just write to some
registers to get the same effect.

Alex

>
> Roland
>
>>
>> For reading back zbuffers, a full-screen rectangle must be drawn with
>> read compression enabled and write compression disabled for
>> decompressing the tiles and writing pixels. Then, the buffer can be
>> accessed by the CPU. This is usually done when switching zbuffers.
> Yes, that would presumably work the same for r100/r200, but the driver
> never made an effort to do it (though in theory, for just switching z
> buffers, at least on r100/r200 you could theoretically just read/write
> the on-chip compression bits and store/load them somewhere - they are
> all accessible).
>
> Ahh that was fun stuff back then ;-).
>
> Roland
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev