From 0b89e2730c41466e8d9c04c469679ba23d052ec9 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Wed, 15 May 2013 13:18:02 -0400 Subject: freedreno: add handle and name tracking Due to the evil userspace buffer tracking we have to do, and hacks for creating GEM buffer from fbdev/scanout, "evil-twin" fd_bo objects are problematic. So introduce hashtable tracking of bo's and dev's, to avoid getting duplicate fd_bo ptrs for the same underlying gem object, in particular when importing via flink name. Signed-off-by: Rob Clark --- freedreno/freedreno_bo.c | 65 ++++++++++++++++++++++++++++++++++++++++---- freedreno/freedreno_device.c | 61 +++++++++++++++++++++++++++++++++++++++-- freedreno/freedreno_drmif.h | 1 + freedreno/freedreno_priv.h | 13 +++++++++ 4 files changed, 133 insertions(+), 7 deletions(-) (limited to 'freedreno') diff --git a/freedreno/freedreno_bo.c b/freedreno/freedreno_bo.c index be386b47..f52ce5ea 100644 --- a/freedreno/freedreno_bo.c +++ b/freedreno/freedreno_bo.c @@ -31,17 +31,46 @@ #include +static pthread_mutex_t table_lock = PTHREAD_MUTEX_INITIALIZER; + +/* set buffer name, and add to table, call w/ table_lock held: */ +static void set_name(struct fd_bo *bo, uint32_t name) +{ + bo->name = name; + /* add ourself into the handle table: */ + drmHashInsert(bo->dev->name_table, name, bo); +} + +/* lookup a buffer, call w/ table_lock held: */ +static struct fd_bo * lookup_bo(void *tbl, uint32_t key) +{ + struct fd_bo *bo = NULL; + if (!drmHashLookup(tbl, key, (void **)&bo)) { + /* found, incr refcnt and return: */ + bo = fd_bo_ref(bo); + } + return bo; +} + +/* allocate a new buffer object, call w/ table_lock held */ static struct fd_bo * bo_from_handle(struct fd_device *dev, uint32_t size, uint32_t handle) { unsigned i; struct fd_bo *bo = calloc(1, sizeof(*bo)); - if (!bo) + if (!bo) { + struct drm_gem_close req = { + .handle = handle, + }; + drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &req); return NULL; - bo->dev = dev; + } + bo->dev = fd_device_ref(dev); bo->size = size; bo->handle = handle; atomic_set(&bo->refcnt, 1); + /* add ourself into the handle table: */ + drmHashInsert(dev->handle_table, handle, bo); for (i = 0; i < ARRAY_SIZE(bo->list); i++) list_inithead(&bo->list[i]); return bo; @@ -95,7 +124,9 @@ struct fd_bo * fd_bo_new(struct fd_device *dev, return NULL; } + pthread_mutex_lock(&table_lock); bo = bo_from_handle(dev, size, req.handle); + pthread_mutex_unlock(&table_lock); if (!bo) { goto fail; } @@ -127,6 +158,7 @@ struct fd_bo * fd_bo_from_fbdev(struct fd_pipe *pipe, return NULL; } + pthread_mutex_lock(&table_lock); bo = bo_from_handle(pipe->dev, size, req.handle); /* this is fugly, but works around a bug in the kernel.. @@ -152,9 +184,11 @@ struct fd_bo * fd_bo_from_fbdev(struct fd_pipe *pipe, bo->gpuaddr = req.gpuaddr; bo->map = fbmem; } + pthread_mutex_unlock(&table_lock); return bo; fail: + pthread_mutex_unlock(&table_lock); if (bo) fd_bo_del(bo); return NULL; @@ -167,13 +201,28 @@ struct fd_bo * fd_bo_from_name(struct fd_device *dev, uint32_t name) }; struct fd_bo *bo; + pthread_mutex_lock(&table_lock); + + /* check name table first, to see if bo is already open: */ + bo = lookup_bo(dev->name_table, name); + if (bo) + goto out_unlock; + if (drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req)) { - return NULL; + ERROR_MSG("gem-open failed: %s", strerror(errno)); + goto out_unlock; } + bo = lookup_bo(dev->handle_table, req.handle); + if (bo) + goto out_unlock; + bo = bo_from_handle(dev, req.size, req.handle); if (bo) - bo->name = name; + set_name(bo, name); + +out_unlock: + pthread_mutex_unlock(&table_lock); return bo; } @@ -196,9 +245,13 @@ void fd_bo_del(struct fd_bo *bo) struct drm_gem_close req = { .handle = bo->handle, }; + pthread_mutex_lock(&table_lock); + drmHashDelete(bo->dev->handle_table, bo->handle); drmIoctl(bo->dev->fd, DRM_IOCTL_GEM_CLOSE, &req); + pthread_mutex_unlock(&table_lock); } + fd_device_del(bo->dev); free(bo); } @@ -215,7 +268,9 @@ int fd_bo_get_name(struct fd_bo *bo, uint32_t *name) return ret; } - bo->name = req.name; + pthread_mutex_lock(&table_lock); + set_name(bo, req.name); + pthread_mutex_unlock(&table_lock); } *name = bo->name; diff --git a/freedreno/freedreno_device.c b/freedreno/freedreno_device.c index 0a9ef9f2..901d0667 100644 --- a/freedreno/freedreno_device.c +++ b/freedreno/freedreno_device.c @@ -26,20 +26,77 @@ * Rob Clark */ +#include +#include +#include + #include "freedreno_drmif.h" #include "freedreno_priv.h" -struct fd_device * fd_device_new(int fd) +static pthread_mutex_t table_lock = PTHREAD_MUTEX_INITIALIZER; +static void * dev_table; + +static struct fd_device * fd_device_new_impl(int fd) { struct fd_device *dev = calloc(1, sizeof(*dev)); if (!dev) return NULL; + atomic_set(&dev->refcnt, 1); dev->fd = fd; + dev->handle_table = drmHashCreate(); + dev->name_table = drmHashCreate(); + return dev; +} + +/* use inode for key into dev_table, rather than fd, to avoid getting + * confused by multiple-opens: + */ +static int devkey(int fd) +{ + struct stat s; + if (fstat(fd, &s)) { + ERROR_MSG("stat failed: %s", strerror(errno)); + return -1; + } + return s.st_ino; +} + +struct fd_device * fd_device_new(int fd) +{ + struct fd_device *dev = NULL; + int key = devkey(fd); + + pthread_mutex_lock(&table_lock); + + if (!dev_table) + dev_table = drmHashCreate(); + + if (drmHashLookup(dev_table, key, (void **)&dev)) { + dev = fd_device_new_impl(fd); + drmHashInsert(dev_table, key, dev); + } else { + dev = fd_device_ref(dev); + } + + pthread_mutex_unlock(&table_lock); + + return dev; +} + +struct fd_device * fd_device_ref(struct fd_device *dev) +{ + atomic_inc(&dev->refcnt); return dev; } void fd_device_del(struct fd_device *dev) { + if (!atomic_dec_and_test(&dev->refcnt)) + return; + pthread_mutex_lock(&table_lock); + drmHashDestroy(dev->handle_table); + drmHashDestroy(dev->name_table); + drmHashDelete(dev_table, devkey(dev->fd)); + pthread_mutex_unlock(&table_lock); free(dev); } - diff --git a/freedreno/freedreno_drmif.h b/freedreno/freedreno_drmif.h index ba99afde..54b900e7 100644 --- a/freedreno/freedreno_drmif.h +++ b/freedreno/freedreno_drmif.h @@ -68,6 +68,7 @@ enum fd_param_id { */ struct fd_device * fd_device_new(int fd); +struct fd_device * fd_device_ref(struct fd_device *dev); void fd_device_del(struct fd_device *dev); diff --git a/freedreno/freedreno_priv.h b/freedreno/freedreno_priv.h index 0edca1d9..433bd300 100644 --- a/freedreno/freedreno_priv.h +++ b/freedreno/freedreno_priv.h @@ -37,6 +37,7 @@ #include #include #include +#include #include "xf86drm.h" #include "xf86atomic.h" @@ -49,6 +50,18 @@ struct fd_device { int fd; + atomic_t refcnt; + + /* tables to keep track of bo's, to avoid "evil-twin" fd_bo objects: + * + * handle_table: maps handle to fd_bo + * name_table: maps flink name to fd_bo + * + * We end up needing two tables, because DRM_IOCTL_GEM_OPEN always + * returns a new handle. So we need to figure out if the bo is already + * open in the process first, before calling gem-open. + */ + void *handle_table, *name_table; }; struct fd_pipe { -- cgit v1.2.3