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

[Leo Liu]

On 09/29/2017 11:37 AM, Leo Liu wrote:
>
>
> On 09/29/2017 10:45 AM, 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
>
> With Andy's information that this happens on newer MPV, and with 
> Marek's fixes, we know MPV is changed to
> interop decode buffer instead of previous output buffer, which means 
> the CSC is done within MPV by most likely calling GL directly instead of
> previously done by VL shader. This might increase CPU usage for MPV in 
> general.

I had a quick test between older and newer MPV on a system, there is no 
obvious difference on CPU usage, which is good.

And we may expect to get more flexibilities with the change.

Regards,
Leo

>
>>> 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;
>>>>>
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev