[Mesa-dev] [PATCH v3 5/7] radeonsi: Implement DCC fast clear.

[Marek Olšák]

On Fri, Oct 23, 2015 at 1:30 PM, Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
> On Fri, Oct 23, 2015 at 12:50 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Fri, Oct 23, 2015 at 12:17 PM, Bas Nieuwenhuizen
>> <bas at basnieuwenhuizen.nl> wrote:
>>> On Thu, Oct 22, 2015 at 12:12 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
>>>>> index 5548cba3..a277fa5 100644
>>>>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>>>>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>>>>> @@ -234,7 +234,7 @@ static void si_set_sampler_views(struct pipe_context *ctx,
>>>>>                         } else {
>>>>>                                 samplers->depth_texture_mask &= ~(1 << slot);
>>>>>                         }
>>>>> -                       if (rtex->cmask.size || rtex->fmask.size) {
>>>>> +                       if (rtex->cmask.size || rtex->fmask.size || rtex->surface.dcc_enabled) {
>>>>>                                 samplers->compressed_colortex_mask |= 1 << slot;
>>>>
>>>> I'd like this flag to be set only when dirty_level_mask is non-zero.
>>>> Setting this for all textures that have DCC is quite expensive in draw
>>>> calls.
>>>
>>> I think this code is incorrect even without considering DCC. If we do
>>> a fast clear on a surface which allocates a cmask and then use that
>>> surface as a texture without calling set_sampler_views in between
>>> (because it was bound before) we get a stale compressed_colortex_mask.
>>>
>>> Some testing shows that this can be triggered using OpenGL, although
>>> the GL_ARB_texture_barrier extension may be needed to make the result
>>> not undefined per the specification.
>>
>> In that case, we should decompress in texture_barrier and not in draw calls.
>>
>> Marek
>
>
> texture_barrier does not need to be called though, the language
> changes might be needed.
>
> Basically the test is
>
> fbo1, fbo2 framebuffers with 1 color buffer each:
>
> bind fbo2 as texture
> clear fbo1 using shader
> bind fbo1 as texture
> clear fbo2 using shader
> clear fbo1 using clear (which results in cmask being allocated for fbo1)
> bind fbo2 as texture
> copy fbo2 to fbo1 using copy shader (which wrongly does not decompress fbo1)
>
> If the clear shader has an unused sampler2D, then we bind fbo1 as a
> texture before the clear and do not rebind it afterwards.
>
> Note that the normal clear is the only one for which we have a
> rendering feedback loop, and it should not fetch from any texture at
> all. I don't think we need a texture barrier for that.
>
> I would think this would be most easily fixed by recomputing
> compressed_colortex_mask after a fast clear.

I'm afraid I don't understand the given example. Copying to fbo1
doesn't need decompression, because fbo1 is a color buffer.

I'm assuming there are no image stores.

Marek