[Mesa-dev] [PATCH 6/8] st/va: remove fence handling

Christian König deathsimple at vodafone.de
Tue Dec 15 02:02:09 PST 2015


Are you sure the flush after calling the compositor is really necessary?

That clearly looks odd, but if it works I'm fine with keeping that for now.

Regards,
Christian.

On 15.12.2015 10:06, Julien Isorce wrote:
> And the attachment :)
>
> On 15 December 2015 at 09:06, Julien Isorce <julien.isorce at gmail.com 
> <mailto:julien.isorce at gmail.com>> wrote:
>
>     Hi Christian,
>
>     I tried your v2.
>
>     I had to apply attached change on top of your patch. (the one in
>     buffer.c to avoid crashing, the one postproc.c otherwise same
>     behavior as the v1 of this patch). Note that I export the RGB-like
>     surface (the one that vpp output), not the NV12 one that come from
>     the decoder directly.
>
>     Cheers
>     Julien
>
>     On 14 December 2015 at 10:11, Christian König
>     <deathsimple at vodafone.de <mailto:deathsimple at vodafone.de>> wrote:
>
>>         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
>>         <deathsimple at vodafone.de <mailto:deathsimple at vodafone.de>> wrote:
>>
>>             From: Christian König <christian.koenig at amd.com
>>             <mailto:christian.koenig at amd.com>>
>>
>>             It's nonsense to drain the pipeline like this.
>>
>>             Signed-off-by: Christian König <christian.koenig at amd.com
>>             <mailto:christian.koenig at 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 at lists.freedesktop.org
>>             <mailto:mesa-dev at lists.freedesktop.org>
>>             http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151215/7149fb55/attachment-0001.html>


More information about the mesa-dev mailing list