drm: Manage drm_local_map structures with linux list functions
authorFrançois Tigeot <ftigeot@wolfpond.org>
Thu, 3 Oct 2013 18:22:43 +0000 (20:22 +0200)
committerFrançois Tigeot <ftigeot@wolfpond.org>
Thu, 3 Oct 2013 18:24:32 +0000 (20:24 +0200)
Keep things as close to Linux 3.5 as possible

sys/dev/drm/drm_bufs.c
sys/dev/drm/drm_context.c
sys/dev/drm/drm_drv.c
sys/dev/drm/drm_ioctl.c
sys/dev/drm/drm_sysctl.c
sys/dev/drm/drm_vm.c
sys/dev/drm/include/drm/drmP.h

index ea3aa53..a4358f2 100644 (file)
@@ -98,7 +98,8 @@ int drm_addmap(struct drm_device * dev, unsigned long offset,
               unsigned long size,
     enum drm_map_type type, enum drm_map_flags flags, drm_local_map_t **map_ptr)
 {
-       drm_local_map_t *map;
+       struct drm_local_map *map;
+       struct drm_map_list *entry;
        int align;
        /*drm_agp_mem_t *entry;
        int valid;*/
@@ -130,11 +131,11 @@ int drm_addmap(struct drm_device * dev, unsigned long offset,
         */
        if (type == _DRM_REGISTERS || type == _DRM_FRAME_BUFFER ||
            type == _DRM_SHM) {
-               TAILQ_FOREACH(map, &dev->maplist, link) {
-                       if (map->type == type && (map->offset == offset ||
-                           (map->type == _DRM_SHM &&
-                           map->flags == _DRM_CONTAINS_LOCK))) {
-                               map->size = size;
+               list_for_each_entry(entry, &dev->maplist, head) {
+                       if (entry->map->type == type && (entry->map->offset == offset ||
+                           (entry->map->type == _DRM_SHM &&
+                           entry->map->flags == _DRM_CONTAINS_LOCK))) {
+                               entry->map->size = size;
                                DRM_DEBUG("Found kernel map %d\n", type);
                                goto done;
                        }
@@ -255,7 +256,7 @@ int drm_addmap(struct drm_device * dev, unsigned long offset,
        }
 
        DRM_LOCK(dev);
-       TAILQ_INSERT_TAIL(&dev->maplist, map, link);
+       list_add(&entry->head, &dev->maplist);
 
 done:
        /* Jumped to, with lock held, when a kernel map is found. */
@@ -298,14 +299,28 @@ int drm_addmap_ioctl(struct drm_device *dev, void *data,
        return 0;
 }
 
-void drm_rmmap(struct drm_device *dev, drm_local_map_t *map)
+void drm_rmmap(struct drm_device *dev, struct drm_local_map *map)
 {
+       struct drm_map_list *r_list = NULL, *list_t;
+       int found = 0;
+
        DRM_LOCK_ASSERT(dev);
 
        if (map == NULL)
                return;
 
-       TAILQ_REMOVE(&dev->maplist, map, link);
+       /* Find the list entry for the map and remove it */
+       list_for_each_entry_safe(r_list, list_t, &dev->maplist, head) {
+               if (r_list->map == map) {
+                       list_del(&r_list->head);
+                       drm_free(r_list, DRM_MEM_DRIVER);
+                       found = 1;
+                       break;
+               }
+       }
+
+       if (!found)
+               return;
 
        switch (map->type) {
        case _DRM_REGISTERS:
@@ -349,27 +364,50 @@ void drm_rmmap(struct drm_device *dev, drm_local_map_t *map)
        drm_free(map, DRM_MEM_MAPS);
 }
 
-/* Remove a map private from list and deallocate resources if the mapping
- * isn't in use.
+/* The rmmap ioctl appears to be unnecessary.  All mappings are torn down on
+ * the last close of the device, and this is necessary for cleanup when things
+ * exit uncleanly.  Therefore, having userland manually remove mappings seems
+ * like a pointless exercise since they're going away anyway.
+ *
+ * One use case might be after addmap is allowed for normal users for SHM and
+ * gets used by drivers that the server doesn't need to care about.  This seems
+ * unlikely.
+ *
+ * \param inode device inode.
+ * \param file_priv DRM file private.
+ * \param cmd command.
+ * \param arg pointer to a struct drm_map structure.
+ * \return zero on success or a negative value on error.
  */
-
 int drm_rmmap_ioctl(struct drm_device *dev, void *data,
                    struct drm_file *file_priv)
 {
-       drm_local_map_t *map;
        struct drm_map *request = data;
+       struct drm_local_map *map = NULL;
+       struct drm_map_list *r_list;
 
        DRM_LOCK(dev);
-       TAILQ_FOREACH(map, &dev->maplist, link) {
-               if (map->handle == request->handle &&
-                   map->flags & _DRM_REMOVABLE)
+       list_for_each_entry(r_list, &dev->maplist, head) {
+               if (r_list->map &&
+                   r_list->user_token == (unsigned long)request->handle &&
+                   r_list->map->flags & _DRM_REMOVABLE) {
+                       map = r_list->map;
                        break;
+               }
        }
 
-       /* No match found. */
-       if (map == NULL) {
+       /* List has wrapped around to the head pointer, or its empty we didn't
+        * find anything.
+        */
+       if (list_empty(&dev->maplist) || !map) {
                DRM_UNLOCK(dev);
-               return EINVAL;
+               return -EINVAL;
+       }
+
+       /* Register and framebuffer maps are permanent */
+       if ((map->type == _DRM_REGISTERS) || (map->type == _DRM_FRAME_BUFFER)) {
+               DRM_UNLOCK(dev);
+               return 0;
        }
 
        drm_rmmap(dev, map);
index 8ef6d1e..f2ec2c5 100644 (file)
@@ -150,19 +150,34 @@ int drm_getsareactx(struct drm_device *dev, void *data,
        return 0;
 }
 
+/**
+ * Set per-context SAREA.
+ *
+ * \param inode device inode.
+ * \param file_priv DRM file private.
+ * \param cmd command.
+ * \param arg user argument pointing to a drm_ctx_priv_map structure.
+ * \return zero on success or a negative number on failure.
+ *
+ * Searches the mapping specified in \p arg and update the entry in
+ * drm_device::ctx_idr with it.
+ */
 int drm_setsareactx(struct drm_device *dev, void *data,
                    struct drm_file *file_priv)
 {
        struct drm_ctx_priv_map *request = data;
-       drm_local_map_t *map = NULL;
+       struct drm_local_map *map = NULL;
+       struct drm_map_list *r_list = NULL;
 
        DRM_LOCK(dev);
-       TAILQ_FOREACH(map, &dev->maplist, link) {
-               if (map->handle == request->handle) {
+       list_for_each_entry(r_list, &dev->maplist, head) {
+               if (r_list->map
+                   && r_list->map->handle == request->handle) {
                        if (dev->max_context < 0)
                                goto bad;
                        if (request->ctx_id >= (unsigned) dev->max_context)
                                goto bad;
+                       map = r_list->map;
                        dev->context_sareas[request->ctx_id] = map;
                        DRM_UNLOCK(dev);
                        return 0;
index d3ca6be..2d02609 100644 (file)
@@ -428,10 +428,18 @@ static int drm_firstopen(struct drm_device *dev)
        return 0;
 }
 
+/**
+ * Take down the DRM device.
+ *
+ * \param dev DRM device structure.
+ *
+ * Frees every resource in \p dev.
+ *
+ * \sa drm_device
+ */
 static int drm_lastclose(struct drm_device *dev)
 {
        drm_magic_entry_t *pt, *next;
-       drm_local_map_t *map, *mapsave;
        int i;
 
        DRM_LOCK_ASSERT(dev);
@@ -490,11 +498,6 @@ static int drm_lastclose(struct drm_device *dev)
                dev->sg = NULL;
        }
 
-       TAILQ_FOREACH_MUTABLE(map, &dev->maplist, link, mapsave) {
-               if (!(map->flags & _DRM_DRIVER))
-                       drm_rmmap(dev, map);
-       }
-
        drm_dma_takedown(dev);
        if (dev->lock.hw_lock) {
                dev->lock.hw_lock = NULL; /* SHM removed */
@@ -511,7 +514,7 @@ static int drm_load(struct drm_device *dev)
 
        DRM_DEBUG("\n");
 
-       TAILQ_INIT(&dev->maplist);
+       INIT_LIST_HEAD(&dev->maplist);
        dev->map_unrhdr = new_unrhdr(1, ((1 << DRM_MAP_HANDLE_BITS) - 1), NULL);
        if (dev->map_unrhdr == NULL) {
                DRM_ERROR("Couldn't allocate map number allocator\n");
@@ -932,12 +935,13 @@ int drm_ioctl(struct dev_ioctl_args *ap)
 
 drm_local_map_t *drm_getsarea(struct drm_device *dev)
 {
-       drm_local_map_t *map;
+       struct drm_map_list *entry;
 
-       DRM_LOCK_ASSERT(dev);
-       TAILQ_FOREACH(map, &dev->maplist, link) {
-               if (map->type == _DRM_SHM && (map->flags & _DRM_CONTAINS_LOCK))
-                       return map;
+       list_for_each_entry(entry, &dev->maplist, head) {
+               if (entry->map && entry->map->type == _DRM_SHM &&
+                   (entry->map->flags & _DRM_CONTAINS_LOCK)) {
+                       return entry->map;
+               }
        }
 
        return NULL;
index 7bc1418..1efe808 100644 (file)
@@ -141,39 +141,55 @@ drm_set_busid(struct drm_device *dev)
        return 0;
 }
 
-int drm_getmap(struct drm_device *dev, void *data, struct drm_file *file_priv)
+/**
+ * Get a mapping information.
+ *
+ * \param inode device inode.
+ * \param file_priv DRM file private.
+ * \param cmd command.
+ * \param arg user argument, pointing to a drm_map structure.
+ *
+ * \return zero on success or a negative number on failure.
+ *
+ * Searches for the mapping with the specified offset and copies its information
+ * into userspace
+ */
+int drm_getmap(struct drm_device *dev, void *data,
+              struct drm_file *file_priv)
 {
-       struct drm_map     *map = data;
-       drm_local_map_t    *mapinlist;
-       int          idx;
-       int          i = 0;
+       struct drm_map *map = data;
+       struct drm_map_list *r_list = NULL;
+       struct list_head *list;
+       int idx;
+       int i;
 
        idx = map->offset;
-
-       DRM_LOCK(dev);
        if (idx < 0) {
-               DRM_UNLOCK(dev);
                return EINVAL;
        }
 
-       TAILQ_FOREACH(mapinlist, &dev->maplist, link) {
+       i = 0;
+       DRM_LOCK(dev);
+       list_for_each(list, &dev->maplist) {
                if (i == idx) {
-                       map->offset = mapinlist->offset;
-                       map->size   = mapinlist->size;
-                       map->type   = mapinlist->type;
-                       map->flags  = mapinlist->flags;
-                       map->handle = mapinlist->handle;
-                       map->mtrr   = mapinlist->mtrr;
+                       r_list = list_entry(list, struct drm_map_list, head);
                        break;
                }
                i++;
        }
+       if (!r_list || !r_list->map) {
+               DRM_UNLOCK(dev);
+               return -EINVAL;
+       }
 
+       map->offset = r_list->map->offset;
+       map->size = r_list->map->size;
+       map->type = r_list->map->type;
+       map->flags = r_list->map->flags;
+       map->handle = r_list->map->handle;
+       map->mtrr   = r_list->map->mtrr;
        DRM_UNLOCK(dev);
 
-       if (mapinlist == NULL)
-               return EINVAL;
-
        return 0;
 }
 
index fc4d7c5..376ad1b 100644 (file)
@@ -175,84 +175,56 @@ done:
        return retcode;
 }
 
+/**
+ * Called when "/proc/dri/.../vm" is read.
+ *
+ * Prints information about all mappings in drm_device::maplist.
+ */
 static int drm_vm_info DRM_SYSCTL_HANDLER_ARGS
 {
-       struct drm_device *dev = arg1;
-       drm_local_map_t *map, *tempmaps;
-       const char *types[] = {
-               [_DRM_FRAME_BUFFER] = "FB",
-               [_DRM_REGISTERS] = "REG",
-               [_DRM_SHM] = "SHM",
-               [_DRM_AGP] = "AGP",
-               [_DRM_SCATTER_GATHER] = "SG",
-               [_DRM_CONSISTENT] = "CONS",
-               [_DRM_GEM] = "GEM"
-       };
-       const char *type, *yesno;
-       int i, mapcount;
        char buf[128];
        int retcode;
+       struct drm_device *dev = arg1;
+       struct drm_local_map *map;
+       struct drm_map_list *r_list;
+
+       /* Hardcoded from _DRM_FRAME_BUFFER,
+          _DRM_REGISTERS, _DRM_SHM, _DRM_AGP, and
+          _DRM_SCATTER_GATHER and _DRM_CONSISTENT */
+       const char *types[] = { "FB", "REG", "SHM", "AGP", "SG", "PCI" };
+       const char *type;
+       int i;
 
-       /* We can't hold the lock while doing SYSCTL_OUTs, so allocate a
-        * temporary copy of all the map entries and then SYSCTL_OUT that.
-        */
        DRM_LOCK(dev);
-
-       mapcount = 0;
-       TAILQ_FOREACH(map, &dev->maplist, link)
-               mapcount++;
-
-       tempmaps = kmalloc(sizeof(drm_local_map_t) * mapcount, DRM_MEM_DRIVER,
-           M_NOWAIT);
-       if (tempmaps == NULL) {
-               DRM_UNLOCK(dev);
-               return ENOMEM;
-       }
-
-       i = 0;
-       TAILQ_FOREACH(map, &dev->maplist, link)
-               tempmaps[i++] = *map;
-
-       DRM_UNLOCK(dev);
-
        DRM_SYSCTL_PRINT("\nslot offset         size       "
            "type flags address            handle mtrr\n");
-
-       for (i = 0; i < mapcount; i++) {
-               map = &tempmaps[i];
-
-               switch(map->type) {
-               default:
+       i = 0;
+       list_for_each_entry(r_list, &dev->maplist, head) {
+               map = r_list->map;
+               if (!map)
+                       continue;
+               if (map->type < 0 || map->type > 5)
                        type = "??";
-                       break;
-               case _DRM_FRAME_BUFFER:
-               case _DRM_REGISTERS:
-               case _DRM_SHM:
-               case _DRM_AGP:
-               case _DRM_SCATTER_GATHER:
-               case _DRM_CONSISTENT:
-               case _DRM_GEM:
+               else
                        type = types[map->type];
-                       break;
-               }
 
-               if (!map->mtrr)
-                       yesno = "no";
+               DRM_SYSCTL_PRINT("%4d 0x%016llx 0x%08lx %4.4s  0x%02x 0x%08lx ",
+                          i,
+                          (unsigned long long)map->offset,
+                          map->size, type, map->flags,
+                          (unsigned long) r_list->user_token);
+               if (map->mtrr < 0)
+                       DRM_SYSCTL_PRINT("none\n");
                else
-                       yesno = "yes";
-
-               DRM_SYSCTL_PRINT(
-                   "%4d 0x%016lx 0x%08lx %4.4s  0x%02x 0x%016lx %6d %s\n",
-                   i, map->offset, map->size, type, map->flags,
-                   (unsigned long)map->virtual,
-                   (unsigned int)((unsigned long)map->handle >>
-                   DRM_MAP_HANDLE_SHIFT), yesno);
+                       DRM_SYSCTL_PRINT("%4d\n", map->mtrr);
+               i++;
+
        }
        SYSCTL_OUT(req, "", 1);
+       DRM_UNLOCK(dev);
 
 done:
-       drm_free(tempmaps, DRM_MEM_DRIVER);
-       return retcode;
+       return 0;
 }
 
 static int drm_bufs_info DRM_SYSCTL_HANDLER_ARGS
index d62628e..e109c4e 100644 (file)
@@ -40,7 +40,9 @@ int drm_mmap(struct dev_mmap_args *ap)
        vm_offset_t offset = ap->a_offset;
        struct drm_device *dev = drm_get_device_from_kdev(kdev);
        struct drm_file *file_priv = NULL;
-       drm_local_map_t *map;
+       struct drm_local_map *map = NULL;
+       struct drm_map_list *r_list;
+
        enum drm_map_type type;
        vm_paddr_t phys;
 
@@ -89,18 +91,19 @@ int drm_mmap(struct dev_mmap_args *ap)
           bit longer.
        */
        DRM_LOCK(dev);
-       TAILQ_FOREACH(map, &dev->maplist, link) {
-               if (offset >> DRM_MAP_HANDLE_SHIFT ==
-                   (unsigned long)map->handle >> DRM_MAP_HANDLE_SHIFT)
+       list_for_each_entry(r_list, &dev->maplist, head) {
+               if (r_list->map && r_list->map->offset >> DRM_MAP_HANDLE_SHIFT ==
+                   (unsigned long)r_list->map->handle >> DRM_MAP_HANDLE_SHIFT)
+                       map = r_list->map;
                        break;
        }
 
        if (map == NULL) {
                DRM_DEBUG("Can't find map, request offset = %016jx\n",
                    (uintmax_t)offset);
-               TAILQ_FOREACH(map, &dev->maplist, link) {
+               list_for_each_entry(r_list, &dev->maplist, head) {
                        DRM_DEBUG("map offset = %016lx, handle = %016lx\n",
-                           map->offset, (unsigned long)map->handle);
+                           r_list->map->offset, (unsigned long)r_list->map->handle);
                }
                DRM_UNLOCK(dev);
                return -1;
index 78ac1f1..059477e 100644 (file)
@@ -574,9 +574,11 @@ typedef struct drm_sg_mem {
 
 #define DRM_MAP_HANDLE_BITS    (sizeof(void *) == 4 ? 4 : 24)
 #define DRM_MAP_HANDLE_SHIFT   (sizeof(void *) * 8 - DRM_MAP_HANDLE_BITS)
-typedef TAILQ_HEAD(drm_map_list, drm_local_map) drm_map_list_t;
 
-typedef struct drm_local_map {
+/**
+ * Kernel side of a mapping
+ */
+struct drm_local_map {
        unsigned long offset;     /* Physical address (0 for SAREA)       */
        unsigned long size;       /* Physical size (bytes)                */
        enum drm_map_type type;   /* Type of memory mapped                */
@@ -592,7 +594,21 @@ typedef struct drm_local_map {
        bus_space_handle_t bsh;
        drm_dma_handle_t *dmah;
        TAILQ_ENTRY(drm_local_map) link;
-} drm_local_map_t;
+};
+
+typedef struct drm_local_map drm_local_map_t;
+
+/**
+ * Mappings list
+ */
+struct drm_map_list {
+       struct list_head head;          /**< list head */
+       struct drm_hash_item hash;
+       struct drm_local_map *map;      /**< mapping */
+       uint64_t user_token;
+       struct drm_master *master;
+       struct drm_mm_node *file_offset_node;   /**< fake offset */
+};
 
 struct drm_vblank_info {
        wait_queue_head_t queue;        /* vblank wait queue */
@@ -913,8 +929,9 @@ struct drm_device {
        drm_file_list_t   files;
        drm_magic_head_t  magiclist[DRM_HASH_SIZE];
 
-       /* Linked list of mappable regions. Protected by dev_lock */
-       drm_map_list_t    maplist;
+       /** \name Memory management */
+       /*@{ */
+       struct list_head maplist;       /**< Linked list of regions */
        struct unrhdr     *map_unrhdr;
 
        drm_local_map_t   **context_sareas;
@@ -1495,12 +1512,11 @@ drm_core_ioremapfree(struct drm_local_map *map, struct drm_device *dev)
 static __inline__ struct drm_local_map *
 drm_core_findmap(struct drm_device *dev, unsigned long offset)
 {
-       drm_local_map_t *map;
+       struct drm_map_list *_entry;
 
-       DRM_LOCK_ASSERT(dev);
-       TAILQ_FOREACH(map, &dev->maplist, link) {
-               if (offset == (unsigned long)map->handle)
-                       return map;
+       list_for_each_entry(_entry, &dev->maplist, head) {
+               if (offset == (unsigned long)_entry->map->handle)
+                       return _entry->map;
        }
        return NULL;
 }