[Mesa-dev] [PATCH] st/va: it is valid to release the VABuffer of an exported resource

[Christian König]

I'm clearly not deep enough into VA-API to judge if that is correct or 
not, but it sounds sane to me.

So feel free to add an Acked-by: Christian König 
<christian.koenig at amd.com> on the patch.

Cheers,
Christian.

Am 08.06.2016 um 13:13 schrieb Julien Isorce:
> Hi Christian,
>
> Thx for the review.
>
> pipe_resource_reference(&res, NULL)  will decrement reference counting 
> (p_atomic_dec  res->count). But the va surface still has the initial 
> reference since it created that resource.
> So calling vaDestroyImage on a derived image will call VaDestroyBuffer 
> but the decrementation wont't reach 0.
>
> It is just wrong that vlVaDestroyBuffer relies on the export_refcount 
> flag. I also compared with vaapi intel driver and they have same flag 
> and it is not present in their vaDestroyBuffer.
>
> Cheers
> Julien
>
>
> On 8 June 2016 at 09:22, Christian König <deathsimple at vodafone.de 
> <mailto:deathsimple at vodafone.de>> wrote:
>
>     Am 02.06.2016 um 16:03 schrieb Julien Isorce:
>
>         Signed-off-by: Julien Isorce <j.isorce at samsung.com
>         <mailto:j.isorce at samsung.com>>
>
>
>     Actually I'm not sure if that is correct.
>
>     If you release the VABuffer of an exported resource you won't be
>     able to properly close the handle with vlVaReleaseBufferHandle().
>
>     On the other hand the semantic VA requires for
>     vlVaAcquireBufferHandle() and vlVaReleaseBufferHandle() is
>     complete nonsense for DMA-buf handles.
>
>     Christian.
>
>         ---
>           src/gallium/state_trackers/va/buffer.c | 8 +-------
>           1 file changed, 1 insertion(+), 7 deletions(-)
>
>         diff --git a/src/gallium/state_trackers/va/buffer.c
>         b/src/gallium/state_trackers/va/buffer.c
>         index 2fd8661..7d3167b 100644
>         --- a/src/gallium/state_trackers/va/buffer.c
>         +++ b/src/gallium/state_trackers/va/buffer.c
>         @@ -192,14 +192,8 @@ vlVaDestroyBuffer(VADriverContextP ctx,
>         VABufferID buf_id)
>                 return VA_STATUS_ERROR_INVALID_BUFFER;
>              }
>           -   if (buf->derived_surface.resource) {
>         -      if (buf->export_refcount > 0) {
>         -         pipe_mutex_unlock(drv->mutex);
>         -         return VA_STATUS_ERROR_INVALID_BUFFER;
>         -      }
>         -
>         +   if (buf->derived_surface.resource)
>         pipe_resource_reference(&buf->derived_surface.resource, NULL);
>         -   }
>                FREE(buf->data);
>              FREE(buf);
>
>
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160608/19de933a/attachment.html>