On Mon, Nov 25, 2013 at 01:22:41PM -0800, Keith Packard wrote:
> If the application sends us a file descriptor pointing at a prime
> buffer that we've already got, we have to re-use the same bo_gem
> structure or chaos will result.
>
> Track the set of all known prime objects and look to see if the kernel
> has returned one of those for a new file descriptor.
>
> Also checks for prime buffers in the flink case.
>
> Signed-off-by: Keith Packard <keithp@xxxxxxxxxx>
> ---
> intel/intel_bufmgr_gem.c | 50 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index df6fcec..2b7fe07 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -149,6 +149,8 @@ struct _drm_intel_bo_gem {
>
> /**
> * Kenel-assigned global name for this object
> + *
> + * List contains both flink named and prime fd'd objects
> */
> unsigned int global_name;
> drmMMListHead name_list;
> @@ -862,10 +864,6 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr,
> }
> }
>
> - bo_gem = calloc(1, sizeof(*bo_gem));
> - if (!bo_gem)
> - return NULL;
> -
> VG_CLEAR(open_arg);
> open_arg.name = handle;
> ret = drmIoctl(bufmgr_gem->fd,
> @@ -874,9 +872,26 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr,
> if (ret != 0) {
> DBG("Couldn't reference %s handle 0x%08x: %s\n",
> name, handle, strerror(errno));
> - free(bo_gem);
> return NULL;
> }
> + /* Now see if someone has used a prime handle to get this
> + * object from the kernel before by looking through the list
> + * again for a matching gem_handle
> + */
> + for (list = bufmgr_gem->named.next;
> + list != &bufmgr_gem->named;
> + list = list->next) {
> + bo_gem = DRMLISTENTRY(drm_intel_bo_gem, list, name_list);
> + if (bo_gem->gem_handle == open_arg.handle) {
> + drm_intel_gem_bo_reference(&bo_gem->bo);
> + return &bo_gem->bo;
> + }
> + }
The kernel actually doesn't bother with this, i.e. an open on an flink
name will always create a new handle. Given that it was a major pita to
get the prime reimporting going (due to a pile of funny lifetime issues
around reference loops and some assorted locking fun) I'm not volunteering
to fix this ;-) And I also think that a piece of userspace which both
flink-opens and prime imports on the same buffer gets both pieces.
Otoh this can't hurt either, so if you want to stick with this hunk maybe
add a small comment saying that the kernel lies. Or just remove it. Either
way:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Aside: I think drm is the only subsystem that goes out of it's way to
ensure a unique relationship between dmabuf and other handles and
underlying objects. If you throw v4l into the mix (e.g. by building a
gstreamer pipe that feeds into an egl image or so) I expect some fun to
happen. Otoh no open-source v4l driver for intel socs, so lalala ;-)
> +
> + bo_gem = calloc(1, sizeof(*bo_gem));
> + if (!bo_gem)
> + return NULL;
> +
> bo_gem->bo.size = open_arg.size;
> bo_gem->bo.offset = 0;
> bo_gem->bo.virtual = NULL;
> @@ -2451,8 +2466,25 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s
> uint32_t handle;
> drm_intel_bo_gem *bo_gem;
> struct drm_i915_gem_get_tiling get_tiling;
> + drmMMListHead *list;
>
> ret = drmPrimeFDToHandle(bufmgr_gem->fd, prime_fd, &handle);
> +
> + /*
> + * See if the kernel has already returned this buffer to us. Just as
> + * for named buffers, we must not create two bo's pointing at the same
> + * kernel object
> + */
> + for (list = bufmgr_gem->named.next;
> + list != &bufmgr_gem->named;
> + list = list->next) {
> + bo_gem = DRMLISTENTRY(drm_intel_bo_gem, list, name_list);
> + if (bo_gem->gem_handle == handle) {
> + drm_intel_gem_bo_reference(&bo_gem->bo);
> + return &bo_gem->bo;
> + }
> + }
> +
> if (ret) {
> fprintf(stderr,"ret is %d %d\n", ret, errno);
> return NULL;
> @@ -2487,8 +2519,8 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s
> bo_gem->has_error = false;
> bo_gem->reusable = false;
>
> - DRMINITLISTHEAD(&bo_gem->name_list);
> DRMINITLISTHEAD(&bo_gem->vma_list);
> + DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
>
> VG_CLEAR(get_tiling);
> get_tiling.handle = bo_gem->gem_handle;
> @@ -2513,6 +2545,9 @@ drm_intel_bo_gem_export_to_prime(drm_intel_bo *bo, int *prime_fd)
> drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>
> + if (DRMLISTEMPTY(&bo_gem->name_list))
> + DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
> +
> if (drmPrimeHandleToFD(bufmgr_gem->fd, bo_gem->gem_handle,
> DRM_CLOEXEC, prime_fd) != 0)
> return -errno;
> @@ -2542,7 +2577,8 @@ drm_intel_gem_bo_flink(drm_intel_bo *bo, uint32_t * name)
> bo_gem->global_name = flink.name;
> bo_gem->reusable = false;
>
> - DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
> + if (DRMLISTEMPTY(&bo_gem->name_list))
> + DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
> }
>
> *name = bo_gem->global_name;
> --
> 1.8.4.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
