-----Original Message----- From: Christian König [mailto:deathsim...@vodafone.de] Sent: 29 October 2015 12:29 To: Julien Isorce; mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [PATCH 2/2] st/va: add support to export a surface as dmabuf
> @@ -108,6 +109,9 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, > void **pbuff) > if (!buf) > return VA_STATUS_ERROR_INVALID_BUFFER; > > + if (buf->export_refcount > 0) > + return VA_STATUS_ERROR_INVALID_BUFFER; >>Why it is illegal to CPU map a buffer which is exported? Hi, I think it is to prevent someone to write into the buffer while someone else is using the exported handle. Either the user call VaMapBuffer or VaAcquireBufferHandle. It seems to be exclusive. Also I took example here: http://cgit.freedesktop.org/vaapi/intel-driver/tree/src/i965_drv_video.c#n2447 > + switch (buf_info->mem_type) { > + case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME: > + close((intptr_t)buf_info->handle); > + break; >>Seriously? Are you sure this is correct? The VA-API driver is supposed to >>close the prime handle? Not the application? Good point actually. I think they assume the application can still call "dup" if it wants extra owner ship of the handle. And this is what gstreamer-vaapi does in fact: https://github.com/01org/gstreamer-vaapi/blob/master/gst/vaapi/gstvaapivideomemory.c#L839 But again I started this impl from intel vaapi driver: http://cgit.freedesktop.org/vaapi/intel-driver/tree/src/i965_drv_video.c#n5731 And for me it is fine since "importers" do not call close. For example when importing the dmabuf into an EGLIMage, eglCreateImage does not take ownership of the handle. Thx Julien >>Regards, >>Christian. On 29.10.2015 12:47, Julien Isorce wrote: > I.e. implements: > VaAcquireBufferHandle > VaReleaseBufferHandle > for memory of type VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME > > And apply relatives change to: > vlVaMapBuffer > vlVaUnMapBuffer > vlVaDestroyBuffer > > Implementation inspired from cgit.freedesktop.org/vaapi/intel-driver > > Tested with gstreamer-vaapi with nouveau driver. > > Signed-off-by: Julien Isorce <j.iso...@samsung.com> > --- > src/gallium/state_trackers/va/buffer.c | 136 > ++++++++++++++++++++++++++++- > src/gallium/state_trackers/va/context.c | 4 +- > src/gallium/state_trackers/va/va_private.h | 6 ++ > 3 files changed, 144 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/state_trackers/va/buffer.c > b/src/gallium/state_trackers/va/buffer.c > index b619d0b..4ad20d2 100644 > --- a/src/gallium/state_trackers/va/buffer.c > +++ b/src/gallium/state_trackers/va/buffer.c > @@ -27,6 +27,7 @@ > > ********************************************************************** > ****/ > > #include "pipe/p_screen.h" > +#include "state_tracker/drm_driver.h" > #include "util/u_memory.h" > #include "util/u_handle_table.h" > #include "util/u_transfer.h" > @@ -108,6 +109,9 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, > void **pbuff) > if (!buf) > return VA_STATUS_ERROR_INVALID_BUFFER; > > + if (buf->export_refcount > 0) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > if (buf->derived_surface.resource) { > *pbuff = pipe_buffer_map(drv->pipe, buf->derived_surface.resource, > PIPE_TRANSFER_WRITE, @@ -138,6 +142,9 > @@ vlVaUnmapBuffer(VADriverContextP ctx, VABufferID buf_id) > if (!buf) > return VA_STATUS_ERROR_INVALID_BUFFER; > > + if (buf->export_refcount > 0) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > if (buf->derived_surface.resource) { > if (!buf->derived_surface.transfer) > return VA_STATUS_ERROR_INVALID_BUFFER; @@ -164,8 +171,12 @@ > vlVaDestroyBuffer(VADriverContextP ctx, VABufferID buf_id) > if (buf->data) > FREE(buf->data); > > - if (buf->derived_surface.resource) > + if (buf->derived_surface.resource) { > + if (buf->export_refcount > 0) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > pipe_resource_reference(&buf->derived_surface.resource, NULL); > + } > > FREE(buf); > handle_table_remove(VL_VA_DRIVER(ctx)->htab, buf_id); @@ -192,3 > +203,126 @@ vlVaBufferInfo(VADriverContextP ctx, VABufferID buf_id, > VABufferType *type, > > return VA_STATUS_SUCCESS; > } > + > +VAStatus > +vlVaAcquireBufferHandle(VADriverContextP ctx, VABufferID buf_id, > + VABufferInfo *out_buf_info) { > + uint32_t i; > + uint32_t mem_type; > + vlVaBuffer *buf ; > + struct pipe_screen *screen; > + > + /* List of supported memory types, in preferred order. */ > + static const uint32_t mem_types[] = { > + VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME, > + 0 > + }; > + > + if (!ctx) > + return VA_STATUS_ERROR_INVALID_CONTEXT; > + > + buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, buf_id); > + > + if (!buf) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > + /* Only VA surface|image like buffers are supported for now .*/ > + if (buf->type != VAImageBufferType) > + return VA_STATUS_ERROR_UNSUPPORTED_BUFFERTYPE; > + > + if (!out_buf_info) > + return VA_STATUS_ERROR_INVALID_PARAMETER; > + > + if (!out_buf_info->mem_type) > + mem_type = mem_types[0]; > + else { > + mem_type = 0; > + for (i = 0; mem_types[i] != 0; i++) { > + if (out_buf_info->mem_type & mem_types[i]) { > + mem_type = out_buf_info->mem_type; > + break; > + } > + } > + if (!mem_type) > + return VA_STATUS_ERROR_UNSUPPORTED_MEMORY_TYPE; > + } > + > + if (!buf->derived_surface.resource) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > + 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; > + } else { > + VABufferInfo * const buf_info = &buf->export_state; > + > + switch (mem_type) { > + case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME: { > + struct winsys_handle whandle; > + > + memset(&whandle, 0, sizeof(whandle)); > + whandle.type = DRM_API_HANDLE_TYPE_FD; > + > + if (!screen->resource_get_handle(screen, > buf->derived_surface.resource, &whandle)) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > + buf_info->handle = (intptr_t)whandle.handle; > + break; > + default: > + return VA_STATUS_ERROR_UNSUPPORTED_MEMORY_TYPE; > + } > + } > + > + buf_info->type = buf->type; > + buf_info->mem_type = mem_type; > + buf_info->mem_size = buf->num_elements * buf->size; > + > + } > + > + buf->export_refcount++; > + > + *out_buf_info = buf->export_state; > + > + return VA_STATUS_SUCCESS; > +} > + > +VAStatus > +vlVaReleaseBufferHandle(VADriverContextP ctx, VABufferID buf_id) { > + vlVaBuffer *buf; > + > + if (!ctx) > + return VA_STATUS_ERROR_INVALID_CONTEXT; > + > + buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, buf_id); > + > + if (!buf) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > + if (buf->export_refcount == 0) > + return VA_STATUS_ERROR_INVALID_BUFFER; > + > + if (--buf->export_refcount == 0) { > + VABufferInfo * const buf_info = &buf->export_state; > + > + switch (buf_info->mem_type) { > + case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME: > + close((intptr_t)buf_info->handle); > + break; > + default: > + return VA_STATUS_ERROR_INVALID_BUFFER; > + } > + > + buf_info->mem_type = 0; > + } > + > + return VA_STATUS_SUCCESS; > +} > diff --git a/src/gallium/state_trackers/va/context.c > b/src/gallium/state_trackers/va/context.c > index 18dcfaa..dc2f179 100644 > --- a/src/gallium/state_trackers/va/context.c > +++ b/src/gallium/state_trackers/va/context.c > @@ -87,7 +87,9 @@ static struct VADriverVTable vtable = > &vlVaUnlockSurface, > NULL, /* DEPRECATED VaGetSurfaceAttributes */ > &vlVaCreateSurfaces2, > - &vlVaQuerySurfaceAttributes > + &vlVaQuerySurfaceAttributes, > + &vlVaAcquireBufferHandle, > + &vlVaReleaseBufferHandle > }; > > static struct VADriverVTableVPP vtable_vpp = diff --git > a/src/gallium/state_trackers/va/va_private.h > b/src/gallium/state_trackers/va/va_private.h > index 40363b3..4634a1e 100644 > --- a/src/gallium/state_trackers/va/va_private.h > +++ b/src/gallium/state_trackers/va/va_private.h > @@ -34,6 +34,7 @@ > #include <va/va.h> > #include <va/va_backend.h> > #include <va/va_backend_vpp.h> > +#include <va/va_drmcommon.h> > > #include "pipe/p_video_enums.h" > #include "pipe/p_video_codec.h" > @@ -241,6 +242,8 @@ typedef struct { > struct pipe_transfer *transfer; > struct pipe_fence_handle *fence; > } derived_surface; > + unsigned int export_refcount; > + VABufferInfo export_state; > } vlVaBuffer; > > typedef struct { > @@ -331,6 +334,9 @@ VAStatus vlVaCreateSurfaces2(VADriverContextP ctx, > unsigned int format, unsigned > VAStatus vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID > config, VASurfaceAttrib *attrib_list, > unsigned int *num_attribs); > > +VAStatus vlVaAcquireBufferHandle(VADriverContextP ctx, VABufferID > +buf_id, VABufferInfo *out_buf_info); VAStatus > +vlVaReleaseBufferHandle(VADriverContextP ctx, VABufferID buf_id); > + > VAStatus vlVaQueryVideoProcFilters(VADriverContextP ctx, VAContextID > context, VAProcFilterType *filters, > unsigned int *num_filters); > VAStatus vlVaQueryVideoProcFilterCaps(VADriverContextP ctx, > VAContextID context, VAProcFilterType type, _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
