From 4f7758ffa1d015ead0507487eb00919c8dac839e Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sun, 2 Oct 2022 17:14:06 +0300 Subject: [PATCH] minigbm: Use dmabuf inode as unique buffer id instead of handle Handle has some limits and can't be used as unique buffer ID on systems where display controller can scanout from CMA but GPU can work with both CMA and VRAM. Such systems have DRM/KMS and DRM/GPU drivers separated. GBM frontend is always expecting handle for DRM/KMS driver. In such system any attempt of importing the buffer with more than 1 contiguous chunk into DRM/KMS driver will fail. Using dma-buf inode as unique buffer ID is a common practice for a last several years starting from [this kernel patch][1]. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ed63bb1d1f8469586006a9ca63c42344401aa2ab Change-Id: Ic3a69010d5da2f866a2252fc7e9eb29d67f8e1ed Signed-off-by: Roman Stratiienko --- drv.c | 29 ++++++++++++++++++----------- drv.h | 2 +- drv_helpers.c | 13 +++++++++++++ drv_helpers.h | 2 ++ drv_priv.h | 2 ++ virtgpu_virgl.c | 8 ++++---- 6 files changed, 40 insertions(+), 16 deletions(-) diff --git a/drv.c b/drv.c index 0826bf4..dad95eb 100644 --- a/drv.c +++ b/drv.c @@ -276,7 +276,7 @@ static void drv_bo_mapping_destroy(struct bo *bo) while (idx < drv_array_size(drv->mappings)) { struct mapping *mapping = (struct mapping *)drv_array_at_idx(drv->mappings, idx); - if (mapping->vma->handle != bo->handle.u32) { + if (mapping->vma->inode != bo->inode) { idx++; continue; } @@ -310,11 +310,16 @@ static void drv_bo_acquire(struct bo *bo) pthread_mutex_lock(&drv->buffer_table_lock); for (size_t plane = 0; plane < bo->meta.num_planes; plane++) { uintptr_t num = 0; + if (!bo->inode) { + int fd = drv_bo_get_plane_fd(bo, plane); + bo->inode = drv_get_inode(fd); + close(fd); + } - if (!drmHashLookup(drv->buffer_table, bo->handle.u32, (void **)&num)) - drmHashDelete(drv->buffer_table, bo->handle.u32); + if (!drmHashLookup(drv->buffer_table, bo->inode, (void **)&num)) + drmHashDelete(drv->buffer_table, bo->inode); - drmHashInsert(drv->buffer_table, bo->handle.u32, (void *)(num + 1)); + drmHashInsert(drv->buffer_table, bo->inode, (void *)(num + 1)); } pthread_mutex_unlock(&drv->buffer_table_lock); } @@ -333,18 +338,18 @@ static bool drv_bo_release(struct bo *bo) pthread_mutex_lock(&drv->buffer_table_lock); for (size_t plane = 0; plane < bo->meta.num_planes; plane++) { - if (!drmHashLookup(drv->buffer_table, bo->handle.u32, (void **)&num)) { - drmHashDelete(drv->buffer_table, bo->handle.u32); + if (!drmHashLookup(drv->buffer_table, bo->inode, (void **)&num)) { + drmHashDelete(drv->buffer_table, bo->inode); if (num > 1) { - drmHashInsert(drv->buffer_table, bo->handle.u32, (void *)(num - 1)); + drmHashInsert(drv->buffer_table, bo->inode, (void *)(num - 1)); } } } /* The same buffer can back multiple planes with different offsets. */ for (size_t plane = 0; plane < bo->meta.num_planes; plane++) { - if (!drmHashLookup(drv->buffer_table, bo->handle.u32, (void **)&num)) { + if (!drmHashLookup(drv->buffer_table, bo->inode, (void **)&num)) { /* num is positive if found in the hashmap. */ pthread_mutex_unlock(&drv->buffer_table_lock); return false; @@ -462,6 +467,8 @@ struct bo *drv_bo_import(struct driver *drv, struct drv_import_fd_data *data) return NULL; } + bo->inode = drv_get_inode(data->fds[0]); + drv_bo_acquire(bo); bo->meta.format_modifier = data->format_modifier; @@ -525,7 +532,7 @@ void *drv_bo_map(struct bo *bo, const struct rectangle *rect, uint32_t map_flags for (i = 0; i < drv_array_size(drv->mappings); i++) { struct mapping *prior = (struct mapping *)drv_array_at_idx(drv->mappings, i); - if (prior->vma->handle != bo->handle.u32 || prior->vma->map_flags != map_flags) + if (prior->vma->inode != bo->inode || prior->vma->map_flags != map_flags) continue; if (rect->x != prior->rect.x || rect->y != prior->rect.y || @@ -539,7 +546,7 @@ void *drv_bo_map(struct bo *bo, const struct rectangle *rect, uint32_t map_flags for (i = 0; i < drv_array_size(drv->mappings); i++) { struct mapping *prior = (struct mapping *)drv_array_at_idx(drv->mappings, i); - if (prior->vma->handle != bo->handle.u32 || prior->vma->map_flags != map_flags) + if (prior->vma->inode != bo->inode || prior->vma->map_flags != map_flags) continue; prior->vma->refcount++; @@ -565,7 +572,7 @@ void *drv_bo_map(struct bo *bo, const struct rectangle *rect, uint32_t map_flags mapping.vma->refcount = 1; mapping.vma->addr = addr; - mapping.vma->handle = bo->handle.u32; + mapping.vma->inode = bo->inode; mapping.vma->map_flags = map_flags; success: diff --git a/drv.h b/drv.h index 51ed861..1fae5c0 100644 --- a/drv.h +++ b/drv.h @@ -134,7 +134,7 @@ struct drv_import_fd_data { struct vma { void *addr; size_t length; - uint32_t handle; + uint32_t inode; uint32_t map_flags; int32_t refcount; uint32_t map_strides[DRV_MAX_PLANES]; diff --git a/drv_helpers.c b/drv_helpers.c index b0979d1..17c9b64 100644 --- a/drv_helpers.c +++ b/drv_helpers.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -623,6 +624,18 @@ void drv_resolve_format_and_use_flags_helper(struct driver *drv, uint32_t format } } +uint32_t drv_get_inode(int dmabuf_fd) +{ + struct stat sb = { 0 }; + int ret = 0; + + ret = fstat(dmabuf_fd, &sb); + if (ret) + drv_loge("Failed to fstat dmabuf %d: %s\n", dmabuf_fd, strerror(errno)); + + return sb.st_ino; +} + const char *drv_get_os_option(const char *name) { const char *ret = getenv(name); diff --git a/drv_helpers.h b/drv_helpers.h index 1c347f8..360e91e 100644 --- a/drv_helpers.h +++ b/drv_helpers.h @@ -55,6 +55,8 @@ void drv_resolve_format_and_use_flags_helper(struct driver *drv, uint32_t format uint64_t use_flags, uint32_t *out_format, uint64_t *out_use_flags); +uint32_t drv_get_inode(int dmabuf_fd); + /* * Get an option. Should return NULL if specified option is not set. */ diff --git a/drv_priv.h b/drv_priv.h index 2f78f8e..29e8302 100644 --- a/drv_priv.h +++ b/drv_priv.h @@ -45,7 +45,9 @@ struct bo { struct driver *drv; struct bo_metadata meta; bool is_test_buffer; + /* handle are mandatory only for SCANOUT buffers */ union bo_handle handle; + uint32_t inode; void *priv; }; diff --git a/virtgpu_virgl.c b/virtgpu_virgl.c index ac8fbc2..d316b4e 100644 --- a/virtgpu_virgl.c +++ b/virtgpu_virgl.c @@ -1097,7 +1097,7 @@ static int virgl_bo_invalidate(struct bo *bo, struct mapping *mapping) if (params[param_resource_blob].value && (bo->meta.tiling & VIRTGPU_BLOB_FLAG_USE_MAPPABLE)) return 0; - xfer.bo_handle = mapping->vma->handle; + xfer.bo_handle = bo->handle.u32; if (mapping->rect.x || mapping->rect.y) { /* @@ -1151,7 +1151,7 @@ static int virgl_bo_invalidate(struct bo *bo, struct mapping *mapping) // The transfer needs to complete before invalidate returns so that any host changes // are visible and to ensure the host doesn't overwrite subsequent guest changes. // TODO(b/136733358): Support returning fences from transfers - waitcmd.handle = mapping->vma->handle; + waitcmd.handle = bo->handle.u32; ret = drmIoctl(bo->drv->fd, DRM_IOCTL_VIRTGPU_WAIT, &waitcmd); if (ret) { drv_loge("DRM_IOCTL_VIRTGPU_WAIT failed with %s\n", strerror(errno)); @@ -1179,7 +1179,7 @@ static int virgl_bo_flush(struct bo *bo, struct mapping *mapping) if (params[param_resource_blob].value && (bo->meta.tiling & VIRTGPU_BLOB_FLAG_USE_MAPPABLE)) return 0; - xfer.bo_handle = mapping->vma->handle; + xfer.bo_handle = bo->handle.u32; if (mapping->rect.x || mapping->rect.y) { /* @@ -1229,7 +1229,7 @@ static int virgl_bo_flush(struct bo *bo, struct mapping *mapping) // buffer, we need to wait for the transfer to complete for consistency. // TODO(b/136733358): Support returning fences from transfers if (bo->meta.use_flags & BO_USE_NON_GPU_HW) { - waitcmd.handle = mapping->vma->handle; + waitcmd.handle = bo->handle.u32; ret = drmIoctl(bo->drv->fd, DRM_IOCTL_VIRTGPU_WAIT, &waitcmd); if (ret) {