[Mesa-dev] [PATCH 6/8] st/va: remove fence handling
Julien Isorce
julien.isorce at gmail.com
Wed Dec 16 03:24:26 PST 2015
Hi Christian,
Sorry for the delay, I confirm the flush is required with that config
described before. Also in order to only affects that particular I wanted to
do this:
+if (buf->export_refcount > 0)
drv->pipe->flush(drv->pipe, NULL, 0);
But this is not the same kind of vlVaBuffer, so not possible to do that.
But in any case I think you are right we should not need thsi flush at all
so I am ok that you remove this flush. The problem should be elsewhere.
So with your v2 + my change in vlVaAcquireBufferHandle,
This patch is:
Reviewed-by: Julien Isorce <j.isorce at samsung.com>
Tested-by: Julien Isorce <j.isorce at samsung.com>
Cheers
Julien
On 15 December 2015 at 10:02, Christian König <deathsimple at vodafone.de>
wrote:
> 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>
> 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>
>> 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>deathsimple at vodafone.de> wrote:
>>>
>>>> From: Christian König < <christian.koenig at amd.com>
>>>> 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>
>>>> 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
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>
>>>
>>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151216/1472745a/attachment-0001.html>
More information about the mesa-dev
mailing list