[Mesa-dev] [PATCH 3/3] Revert "gallium/radeon: consolidate PIPE_BIND_SHARED/SCANOUT handling"

[Mark Thompson]

On 29/09/17 15:45, Andy Furniss wrote:
> Marek Olšák wrote:
>> Can you test this?
> 
> My mpv test case is fixed by
> 
> radeonsi/uvd: fix planar formats broken since f70f6baaa3bb0f8b280ac2eaea69bb

Also good for my VAAPI case; dropped the revert from my series.

Thanks,

- Mark

>>
>> Thanks,
>> Marek
>>
>> On Fri, Sep 29, 2017 at 1:51 AM, Andy Furniss <adf.lists at gmail.com> wrote:
>>> Mark Thompson wrote:
>>>>
>>>> This reverts commit f70f6baaa3bb0f8b280ac2eaea69bbffaf7de840.
>>>
>>>
>>> I just bisected to this as it also breaks
>>>
>>> mpv --hwdec=vdpau --vo=opengl
>>>
>>> amdgpu: The CS has been rejected, see dmesg for more information (-22).
>>> amdgpu: The CS has been cancelled because the context is lost.
>>>
>>> [drm:amdgpu_uvd_cs_pass2 [amdgpu]] *ERROR* buffer (2) to small (8355840 /
>>> 12441600)!
>>>
>>>
>>>
>>>> ---
>>>> This commit broke VAAPI surface export (found by bisection).  I think the
>>>> observed behaviour with playback is consistent with surfaces not being
>>>> updated some of the time, so something to do with sharing?  I tried setting
>>>> PIPE_BIND_SHARED on the surfaces explicitly, but that didn't help so I'm
>>>> somewhat unclear what's going on exactly.
>>>>
>>>> I've included this patch in the series as a revert because it makes it
>>>> testable for other people, but it would be better if someone who understands
>>>> how these interact could have a look and decide how to fix it properly.
>>>>
>>>> Thanks,
>>>>
>>>> - Mark
>>>>
>>>>
>>>>    src/gallium/drivers/radeon/r600_buffer_common.c | 13 +++++++++----
>>>>    src/gallium/drivers/radeon/r600_texture.c       |  4 ++++
>>>>    2 files changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c
>>>> b/src/gallium/drivers/radeon/r600_buffer_common.c
>>>> index b3e60a46e4..706c7485c3 100644
>>>> --- a/src/gallium/drivers/radeon/r600_buffer_common.c
>>>> +++ b/src/gallium/drivers/radeon/r600_buffer_common.c
>>>> @@ -167,10 +167,12 @@ void si_init_resource_fields(struct
>>>> r600_common_screen *rscreen,
>>>>                           RADEON_FLAG_GTT_WC;
>>>>          }
>>>>    -     /* Displayable and shareable surfaces are not suballocated. */
>>>> -       if (res->b.b.bind & (PIPE_BIND_SHARED | PIPE_BIND_SCANOUT))
>>>> -               res->flags |= RADEON_FLAG_NO_SUBALLOC; /* shareable */
>>>> -       else
>>>> +       /* Only displayable single-sample textures can be shared between
>>>> +        * processes. */
>>>> +       if (!(res->b.b.bind & (PIPE_BIND_SHARED | PIPE_BIND_SCANOUT)) &&
>>>> +           (res->b.b.target == PIPE_BUFFER ||
>>>> +            res->b.b.nr_samples >= 2 ||
>>>> +            rtex->surface.micro_tile_mode != RADEON_MICRO_MODE_DISPLAY))
>>>>                  res->flags |= RADEON_FLAG_NO_INTERPROCESS_SHARING;
>>>>          /* If VRAM is just stolen system memory, allow both VRAM and
>>>> @@ -190,6 +192,9 @@ void si_init_resource_fields(struct r600_common_screen
>>>> *rscreen,
>>>>          if (rscreen->debug_flags & DBG_NO_WC)
>>>>                  res->flags &= ~RADEON_FLAG_GTT_WC;
>>>>    +     if (res->b.b.bind & PIPE_BIND_SHARED)
>>>> +               res->flags |= RADEON_FLAG_NO_SUBALLOC;
>>>> +
>>>>          /* Set expected VRAM and GART usage for the buffer. */
>>>>          res->vram_usage = 0;
>>>>          res->gart_usage = 0;
>>>> diff --git a/src/gallium/drivers/radeon/r600_texture.c
>>>> b/src/gallium/drivers/radeon/r600_texture.c
>>>> index a9a1b2627e..829d105827 100644
>>>> --- a/src/gallium/drivers/radeon/r600_texture.c
>>>> +++ b/src/gallium/drivers/radeon/r600_texture.c
>>>> @@ -1219,6 +1219,10 @@ r600_texture_create_object(struct pipe_screen
>>>> *screen,
>>>>                  si_init_resource_fields(rscreen, resource, rtex->size,
>>>>                                            rtex->surface.surf_alignment);
>>>>    +             /* Displayable surfaces are not suballocated. */
>>>> +               if (resource->b.b.bind & PIPE_BIND_SCANOUT)
>>>> +                       resource->flags |= RADEON_FLAG_NO_SUBALLOC;
>>>> +
>>>>                  if (!si_alloc_resource(rscreen, resource)) {
>>>>                          FREE(rtex);
>>>>                          return NULL;
>>>>