Also note that in this pipeline, HW decoding is done with nouveau driver and rendering is done with intel. dmabuf in between.
Yeah, I already thought that somebody is using it like this. I'm not sure if this is actually supposed to work because we don't have proper synchronization between kernel drivers with DMA-buf jet.

Maybe the idea of the patch is good but something is still wrong.
While it is not the proper solution I would say let's keep the pipeline draining during exporting the handle for now if that's really necessary for your use case. Please test the attached patch.

Coding the patch I've just noticed that there wasn't a pipe->flush() before exporting the handle. Does it work as well if you just flush the pipeline without waiting for the commands to be finished?

Regards,
Christian.

On 14.12.2015 10:14, Julien Isorce wrote:
Hi Christian,

I have tested this patch but then the displayed video is garbage (mostly white and sometimes just garbage). It also stall the nouveau driver which requires to reboot but I guess this is another issue.
I tested with:
GST_GL_WINDOW=x11 GST_GL_PLATFORM=egl GST_GL_API=gles2 GST_DEBUG=2 LIBVA_DRIVER_NAME=gallium gst-launch-1.0 filesrc location=simpson.mp4 ! qtdemux ! vaapidecodebin ! glimagesink

(to test that you need my gstreamer-vaapi and gstgl branches on my github but I would not waste time to try them since they should be merged upstream at some point)

Also note that in this pipeline, HW decoding is done with nouveau driver and rendering is done with intel. dmabuf in between.

Maybe the idea of the patch is good but something is still wrong.
I can test any update if it helps.

Cheers
Julien




On 11 December 2015 at 12:33, Christian König <deathsim...@vodafone.de <mailto:deathsim...@vodafone.de>> wrote:

    From: Christian König <christian.koe...@amd.com
    <mailto:christian.koe...@amd.com>>

    It's nonsense to drain the pipeline like this.

    Signed-off-by: Christian König <christian.koe...@amd.com
    <mailto:christian.koe...@amd.com>>
    ---
     src/gallium/state_trackers/va/buffer.c     |  5 -----
     src/gallium/state_trackers/va/image.c      |  1 -
     src/gallium/state_trackers/va/postproc.c   |  6 ------
     src/gallium/state_trackers/va/surface.c    | 10 +---------
     src/gallium/state_trackers/va/va_private.h |  2 --
     5 files changed, 1 insertion(+), 23 deletions(-)

    diff --git a/src/gallium/state_trackers/va/buffer.c
    b/src/gallium/state_trackers/va/buffer.c
    index 769305e..2ec187c 100644
    --- a/src/gallium/state_trackers/va/buffer.c
    +++ b/src/gallium/state_trackers/va/buffer.c
    @@ -257,11 +257,6 @@ vlVaAcquireBufferHandle(VADriverContextP ctx,
    VABufferID buf_id,

        screen = VL_VA_PSCREEN(ctx);

    -   if (buf->derived_surface.fence) {
    -      screen->fence_finish(screen, buf->derived_surface.fence,
    PIPE_TIMEOUT_INFINITE);
    -      screen->fence_reference(screen,
    &buf->derived_surface.fence, NULL);
    -   }
    -
        if (buf->export_refcount > 0) {
           if (buf->export_state.mem_type != mem_type)
              return VA_STATUS_ERROR_INVALID_PARAMETER;
    diff --git a/src/gallium/state_trackers/va/image.c
    b/src/gallium/state_trackers/va/image.c
    index ae07da8..58c9ff7 100644
    --- a/src/gallium/state_trackers/va/image.c
    +++ b/src/gallium/state_trackers/va/image.c
    @@ -266,7 +266,6 @@ vlVaDeriveImage(VADriverContextP ctx,
    VASurfaceID surface, VAImage *image)
        img_buf->type = VAImageBufferType;
        img_buf->size = image->data_size;
        img_buf->num_elements = 1;
    -   img_buf->derived_surface.fence = surf->fence;

    pipe_resource_reference(&img_buf->derived_surface.resource,
    surfaces[0]->texture);

    diff --git a/src/gallium/state_trackers/va/postproc.c
    b/src/gallium/state_trackers/va/postproc.c
    index 105f251..1ee3587 100644
    --- a/src/gallium/state_trackers/va/postproc.c
    +++ b/src/gallium/state_trackers/va/postproc.c
    @@ -54,7 +54,6 @@
    vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
    vlVaContext *contex
        vlVaSurface *src_surface;
        VAProcPipelineParameterBuffer *pipeline_param;
        struct pipe_surface **surfaces;
    -   struct pipe_screen *screen;
        struct pipe_surface *psurf;

        if (!drv || !context)
    @@ -77,8 +76,6 @@
    vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
    vlVaContext *contex
        if (!surfaces || !surfaces[0])
           return VA_STATUS_ERROR_INVALID_SURFACE;

    -   screen = drv->pipe->screen;
    -
        psurf = surfaces[0];

        src_region = vlVaRegionDefault(pipeline_param->surface_region,
    src_surface->buffer, &def_src_region);
    @@ -99,9 +96,6 @@
    vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
    vlVaContext *contex
        vl_compositor_set_layer_dst_area(&drv->cstate, 0, &dst_rect);
        vl_compositor_render(&drv->cstate, &drv->compositor, psurf,
    NULL, false);

    -   screen->fence_reference(screen, &src_surface->fence, NULL);
    -   drv->pipe->flush(drv->pipe, &src_surface->fence, 0);
    -
        return VA_STATUS_SUCCESS;
     }

    diff --git a/src/gallium/state_trackers/va/surface.c
    b/src/gallium/state_trackers/va/surface.c
    index 4a18a6f..5ddaf04 100644
    --- a/src/gallium/state_trackers/va/surface.c
    +++ b/src/gallium/state_trackers/va/surface.c
    @@ -72,8 +72,6 @@ vlVaDestroySurfaces(VADriverContextP ctx,
    VASurfaceID *surface_list, int num_sur
           vlVaSurface *surf = handle_table_get(drv->htab,
    surface_list[i]);
           if (surf->buffer)
              surf->buffer->destroy(surf->buffer);
    -      if(surf->fence)
    -  drv->pipe->screen->fence_reference(drv->pipe->screen,
    &surf->fence, NULL);
           util_dynarray_fini(&surf->subpics);
           FREE(surf);
           handle_table_remove(drv->htab, surface_list[i]);
    @@ -245,11 +243,6 @@ vlVaPutSurface(VADriverContextP ctx,
    VASurfaceID surface_id, void* draw, short s
        screen = drv->pipe->screen;
        vscreen = drv->vscreen;

    -   if(surf->fence) {
    -      screen->fence_finish(screen, surf->fence,
    PIPE_TIMEOUT_INFINITE);
    -      screen->fence_reference(screen, &surf->fence, NULL);
    -   }
    -
        tex = vscreen->texture_from_drawable(vscreen, draw);
        if (!tex)
           return VA_STATUS_ERROR_INVALID_DISPLAY;
    @@ -281,8 +274,7 @@ vlVaPutSurface(VADriverContextP ctx,
    VASurfaceID surface_id, void* draw, short s
        screen->flush_frontbuffer(screen, tex, 0, 0,
    vscreen->get_private(vscreen), NULL);

    -   screen->fence_reference(screen, &surf->fence, NULL);
    -   drv->pipe->flush(drv->pipe, &surf->fence, 0);
    +   drv->pipe->flush(drv->pipe, NULL, 0);

        pipe_resource_reference(&tex, NULL);
        pipe_surface_reference(&surf_draw, NULL);
    diff --git a/src/gallium/state_trackers/va/va_private.h
    b/src/gallium/state_trackers/va/va_private.h
    index 6739efc..fa6e0fb 100644
    --- a/src/gallium/state_trackers/va/va_private.h
    +++ b/src/gallium/state_trackers/va/va_private.h
    @@ -244,7 +244,6 @@ typedef struct {
        struct {
           struct pipe_resource *resource;
           struct pipe_transfer *transfer;
    -      struct pipe_fence_handle *fence;
        } derived_surface;
        unsigned int export_refcount;
        VABufferInfo export_state;
    @@ -252,7 +251,6 @@ typedef struct {

     typedef struct {
        struct pipe_video_buffer templat, *buffer;
    -   struct pipe_fence_handle *fence;
        struct util_dynarray subpics; /* vlVaSubpicture */
     } vlVaSurface;

    --
    2.5.0

    _______________________________________________
    mesa-dev mailing list
    mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
    http://lists.freedesktop.org/mailman/listinfo/mesa-dev



>From 06b6fa286ac4c1b40bf9a4d60427241eebd9422e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20K=C3=B6nig?= <christian.koe...@amd.com>
Date: Tue, 8 Dec 2015 14:28:13 +0100
Subject: [PATCH] st/va: remove fence handling v2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's nonsense to drain the pipeline like this.

v2: keep the drain for DMA-buf exports.

Signed-off-by: Christian König <christian.koe...@amd.com>
---
 src/gallium/state_trackers/va/buffer.c     | 12 +++++++-----
 src/gallium/state_trackers/va/image.c      |  1 -
 src/gallium/state_trackers/va/postproc.c   |  6 ------
 src/gallium/state_trackers/va/surface.c    | 10 +---------
 src/gallium/state_trackers/va/va_private.h |  2 --
 5 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/src/gallium/state_trackers/va/buffer.c b/src/gallium/state_trackers/va/buffer.c
index 769305e..71de657 100644
--- a/src/gallium/state_trackers/va/buffer.c
+++ b/src/gallium/state_trackers/va/buffer.c
@@ -212,6 +212,7 @@ VAStatus
 vlVaAcquireBufferHandle(VADriverContextP ctx, VABufferID buf_id,
                         VABufferInfo *out_buf_info)
 {
+   vlVaDriver *drv;
    uint32_t i;
    uint32_t mem_type;
    vlVaBuffer *buf ;
@@ -255,13 +256,9 @@ vlVaAcquireBufferHandle(VADriverContextP ctx, VABufferID buf_id,
    if (!buf->derived_surface.resource)
       return VA_STATUS_ERROR_INVALID_BUFFER;
 
+   drv = VL_VA_DRIVER(ctx);
    screen = VL_VA_PSCREEN(ctx);
 
-   if (buf->derived_surface.fence) {
-      screen->fence_finish(screen, buf->derived_surface.fence, PIPE_TIMEOUT_INFINITE);
-      screen->fence_reference(screen, &buf->derived_surface.fence, NULL);
-   }
-
    if (buf->export_refcount > 0) {
       if (buf->export_state.mem_type != mem_type)
          return VA_STATUS_ERROR_INVALID_PARAMETER;
@@ -270,8 +267,13 @@ vlVaAcquireBufferHandle(VADriverContextP ctx, VABufferID buf_id,
 
       switch (mem_type) {
       case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME: {
+         struct pipe_fence_handle *fence;
          struct winsys_handle whandle;
 
+         drv->pipe->flush(drv->pipe, &fence, 0);
+         screen->fence_finish(screen, fence, PIPE_TIMEOUT_INFINITE);
+         screen->fence_reference(screen, &fence, NULL);
+
          memset(&whandle, 0, sizeof(whandle));
          whandle.type = DRM_API_HANDLE_TYPE_FD;
 
diff --git a/src/gallium/state_trackers/va/image.c b/src/gallium/state_trackers/va/image.c
index ae07da8..58c9ff7 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -266,7 +266,6 @@ vlVaDeriveImage(VADriverContextP ctx, VASurfaceID surface, VAImage *image)
    img_buf->type = VAImageBufferType;
    img_buf->size = image->data_size;
    img_buf->num_elements = 1;
-   img_buf->derived_surface.fence = surf->fence;
 
    pipe_resource_reference(&img_buf->derived_surface.resource, surfaces[0]->texture);
 
diff --git a/src/gallium/state_trackers/va/postproc.c b/src/gallium/state_trackers/va/postproc.c
index 105f251..1ee3587 100644
--- a/src/gallium/state_trackers/va/postproc.c
+++ b/src/gallium/state_trackers/va/postproc.c
@@ -54,7 +54,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *contex
    vlVaSurface *src_surface;
    VAProcPipelineParameterBuffer *pipeline_param;
    struct pipe_surface **surfaces;
-   struct pipe_screen *screen;
    struct pipe_surface *psurf;
 
    if (!drv || !context)
@@ -77,8 +76,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *contex
    if (!surfaces || !surfaces[0])
       return VA_STATUS_ERROR_INVALID_SURFACE;
 
-   screen = drv->pipe->screen;
-
    psurf = surfaces[0];
 
    src_region = vlVaRegionDefault(pipeline_param->surface_region, src_surface->buffer, &def_src_region);
@@ -99,9 +96,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *contex
    vl_compositor_set_layer_dst_area(&drv->cstate, 0, &dst_rect);
    vl_compositor_render(&drv->cstate, &drv->compositor, psurf, NULL, false);
 
-   screen->fence_reference(screen, &src_surface->fence, NULL);
-   drv->pipe->flush(drv->pipe, &src_surface->fence, 0);
-
    return VA_STATUS_SUCCESS;
 }
 
diff --git a/src/gallium/state_trackers/va/surface.c b/src/gallium/state_trackers/va/surface.c
index 4a18a6f..5ddaf04 100644
--- a/src/gallium/state_trackers/va/surface.c
+++ b/src/gallium/state_trackers/va/surface.c
@@ -72,8 +72,6 @@ vlVaDestroySurfaces(VADriverContextP ctx, VASurfaceID *surface_list, int num_sur
       vlVaSurface *surf = handle_table_get(drv->htab, surface_list[i]);
       if (surf->buffer)
          surf->buffer->destroy(surf->buffer);
-      if(surf->fence)
-         drv->pipe->screen->fence_reference(drv->pipe->screen, &surf->fence, NULL);
       util_dynarray_fini(&surf->subpics);
       FREE(surf);
       handle_table_remove(drv->htab, surface_list[i]);
@@ -245,11 +243,6 @@ vlVaPutSurface(VADriverContextP ctx, VASurfaceID surface_id, void* draw, short s
    screen = drv->pipe->screen;
    vscreen = drv->vscreen;
 
-   if(surf->fence) {
-      screen->fence_finish(screen, surf->fence, PIPE_TIMEOUT_INFINITE);
-      screen->fence_reference(screen, &surf->fence, NULL);
-   }
-
    tex = vscreen->texture_from_drawable(vscreen, draw);
    if (!tex)
       return VA_STATUS_ERROR_INVALID_DISPLAY;
@@ -281,8 +274,7 @@ vlVaPutSurface(VADriverContextP ctx, VASurfaceID surface_id, void* draw, short s
    screen->flush_frontbuffer(screen, tex, 0, 0,
                              vscreen->get_private(vscreen), NULL);
 
-   screen->fence_reference(screen, &surf->fence, NULL);
-   drv->pipe->flush(drv->pipe, &surf->fence, 0);
+   drv->pipe->flush(drv->pipe, NULL, 0);
 
    pipe_resource_reference(&tex, NULL);
    pipe_surface_reference(&surf_draw, NULL);
diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
index 6739efc..fa6e0fb 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -244,7 +244,6 @@ typedef struct {
    struct {
       struct pipe_resource *resource;
       struct pipe_transfer *transfer;
-      struct pipe_fence_handle *fence;
    } derived_surface;
    unsigned int export_refcount;
    VABufferInfo export_state;
@@ -252,7 +251,6 @@ typedef struct {
 
 typedef struct {
    struct pipe_video_buffer templat, *buffer;
-   struct pipe_fence_handle *fence;
    struct util_dynarray subpics; /* vlVaSubpicture */
 } vlVaSurface;
 
-- 
2.5.0

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to