[Mesa-dev] [PATCH 3/6] radeonsi: clear PIPE_IMAGE_ACCESS_WRITE when it's invalid to be on the safe side

[Nicolai Hähnle]

On 29.11.2017 15:29, Marek Olšák wrote:
> On Wed, Nov 29, 2017 at 10:59 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 28.11.2017 22:17, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> ---
>>>    src/gallium/drivers/radeonsi/si_descriptors.c | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
>>> b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> index 69371ea..471c93a 100644
>>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> @@ -681,20 +681,28 @@ static void si_set_shader_image_desc(struct
>>> si_context *ctx,
>>>                                            view->format,
>>>                                            view->u.buf.offset,
>>>                                            view->u.buf.size, desc);
>>>                  si_set_buf_desc_address(res, view->u.buf.offset, desc +
>>> 4);
>>>          } else {
>>>                  static const unsigned char swizzle[4] = { 0, 1, 2, 3 };
>>>                  struct r600_texture *tex = (struct r600_texture *)res;
>>>                  unsigned level = view->u.tex.level;
>>>                  unsigned width, height, depth, hw_level;
>>>                  bool uses_dcc = vi_dcc_enabled(tex, level);
>>> +               unsigned access = view->access;
>>> +
>>> +               /* Clear the write flag when writes can't occur.
>>> +                * Note that DCC_DECOMPRESS for MSAA doesn't work in some
>>> cases,
>>> +                * so we don't wanna trigger it.
>>> +                */
>>> +               if (tex->is_depth || tex->resource.b.b.nr_samples >= 2)
>>> +                       access &= ~PIPE_IMAGE_ACCESS_WRITE;
>>
>>
>> Shouldn't this rather be an assert()? Just removing the bit here won't stop
>> the application from attempting to write to it from a shader anyway.
>>
>> We shouldn't be hitting that assert anyway, since we don't currently support
>> MSAA images.
> 
> Better safe than sorry. I think it's better to recover from a bad
> scenario than to fail an assertion, considering that assertions aren't
> even enabled in release builds. I'll add an assertion into the
> conditional block.

Sounds good, thanks :)


> 
> Marek
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.