[Mesa-dev] [PATCH 17/26] gallium/radeon: disable CMASK on handle export if sharing doesn't allow it

[Marek Olšák]

On Wed, Mar 9, 2016 at 7:18 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 08.03.2016 14:35, Marek Olšák wrote:
>>
>> On Tue, Mar 8, 2016 at 4:41 AM, Michel Dänzer <michel at daenzer.net> wrote:
>>>
>>> On 03.03.2016 01:36, Marek Olšák wrote:
>>>>
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> The disabling of CMASK is simple, but notifying all contexts about it is
>>>> not:
>>>> - The screen must have a list of all contexts.
>>>> - Each context must have a monotonic counter that is incremented only
>>>> when
>>>>    the screen wants to re-emit framebuffer states.
>>>> - Each context must check in draw_vbo if the counter has been changed
>>>> and
>>>>    re-emit the framebuffer state accordingly.
>>>
>>>
>>> The list seems a bit overkill. How about having dirty_fb_counter in the
>>> screen and last_dirty_fb_counter in the context, incrementing the former
>>> in r600_dirty_all_framebuffer_states and emitting the framebuffer state
>>> if the two counters don't match?
>>
>>
>> Thanks. The updated patch is attached. Please review.
>
>
> There is an unneeded empty line in this hunk:
>
> @@ -260,6 +265,31 @@ static void r600_eliminate_fast_color_clear(struct
> r600_common_screen *rscreen,
>         pipe_mutex_unlock(rscreen->aux_context_lock);
>  }
>
> +static void r600_texture_disable_cmask(struct r600_common_screen *rscreen,
> +                                      struct r600_texture *rtex)
> +{
> +
> +       if (!rtex->cmask.size)
> +               return;
>
> Slightly further down, I believe the pipe_resource_reference should be
> unconditional:
>
> +       if (rtex->cmask_buffer != &rtex->resource)
> +           pipe_resource_reference((struct
> pipe_resource**)&rtex->cmask_buffer, NULL);

It's how cmask_buffer is unreferenced everywhere. If the texture had
multiple samples, cmask_buffer would be part of the resource and so
the code would release the resource, which would be undesirable. This
is not a possible scenario yet, because texture_get_handle doesn't
support MSAA surfaces, but it matches the unref code elsewhere.

Marek