* [PATCH v4 1/4] drm: add unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 0/4] Unified VMA Offset Manager David Herrmann
@ 2013-07-23 12:47 ` David Herrmann
2013-07-24 15:35 ` Daniel Vetter
2013-07-24 19:06 ` [PATCH v5 " David Herrmann
2013-07-23 12:47 ` [PATCH v4 2/4] drm/gem: convert to new unified vma manager David Herrmann
` (2 subsequent siblings)
3 siblings, 2 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-23 12:47 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, Daniel Vetter
If we want to map GPU memory into user-space, we need to linearize the
addresses to not confuse mm-core. Currently, GEM and TTM both implement
their own offset-managers to assign a pgoff to each object for user-space
CPU access. GEM uses a hash-table, TTM uses an rbtree.
This patch provides a unified implementation that can be used to replace
both. TTM allows partial mmaps with a given offset, so we cannot use
hashtables as the start address may not be known at mmap time. Hence, we
use the rbtree-implementation of TTM.
We could easily update drm_mm to use an rbtree instead of a linked list
for it's object list and thus drop the rbtree from the vma-manager.
However, this would slow down drm_mm object allocation for all other
use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
Hence, use the separate tree but allow for later migration.
This is a rewrite of the 2012-proposal by David Airlie <airlied@linux.ie>
v2:
- fix Docbook integration
- drop drm_mm_node_linked() and use drm_mm_node_allocated()
- remove unjustified likely/unlikely usage (but keep for rbtree paths)
- remove BUG_ON() as drm_mm already does that
- clarify page-based vs. byte-based addresses
- use drm_vma_node_reset() for initialization, too
v4:
- allow external locking via drm_vma_offset_un/lock_lookup()
- add locked lookup helper drm_vma_offset_lookup_locked()
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
Documentation/DocBook/drm.tmpl | 6 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_vma_manager.c | 283 ++++++++++++++++++++++++++++++++++++++
include/drm/drm_vma_manager.h | 180 ++++++++++++++++++++++++
4 files changed, 470 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_vma_manager.c
create mode 100644 include/drm/drm_vma_manager.h
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7d1278e..87e22ec 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2212,6 +2212,12 @@ void intel_crt_init(struct drm_device *dev)
!Iinclude/drm/drm_rect.h
!Edrivers/gpu/drm/drm_rect.c
</sect2>
+ <sect2>
+ <title>VMA Offset Manager</title>
+!Pdrivers/gpu/drm/drm_vma_manager.c vma offset manager
+!Edrivers/gpu/drm/drm_vma_manager.c
+!Iinclude/drm/drm_vma_manager.h
+ </sect2>
</sect1>
<!-- Internals: kms properties -->
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 801bcaf..d943b94 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -13,7 +13,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
drm_crtc.o drm_modes.o drm_edid.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
- drm_rect.o
+ drm_rect.o drm_vma_manager.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o
drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
new file mode 100644
index 0000000..27760bd
--- /dev/null
+++ b/drivers/gpu/drm/drm_vma_manager.c
@@ -0,0 +1,283 @@
+/*
+ * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA
+ * Copyright (c) 2012 David Airlie <airlied@linux.ie>
+ * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_mm.h>
+#include <drm/drm_vma_manager.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/**
+ * DOC: vma offset manager
+ *
+ * The vma-manager is responsible to map arbitrary driver-dependent memory
+ * regions into the linear user address-space. It provides offsets to the
+ * caller which can then be used on the address_space of the drm-device. It
+ * takes care to not overlap regions, size them appropriately and to not
+ * confuse mm-core by inconsistent fake vm_pgoff fields.
+ * Drivers shouldn't use this for object placement in VMEM. This manager should
+ * only be used to manage mappings into linear user-space VMs.
+ *
+ * We use drm_mm as backend to manage object allocations. But it is highly
+ * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
+ * speed up offset lookups.
+ *
+ * You must not use multiple offset managers on a single address_space.
+ * Otherwise, mm-core will be unable to tear down memory mappings as the VM will
+ * no longer be linear. Please use VM_NONLINEAR in that case and implement your
+ * own offset managers.
+ *
+ * This offset manager works on page-based addresses. That is, every argument
+ * and return code (with the exception of drm_vma_node_offset_addr()) is given
+ * in number of pages, not number of bytes. That means, object sizes and offsets
+ * must always be page-aligned (as usual).
+ * If you want to get a valid byte-based user-space address for a given offset,
+ * please see drm_vma_node_offset_addr().
+ */
+
+/**
+ * drm_vma_offset_manager_init - Initialize new offset-manager
+ * @mgr: Manager object
+ * @page_offset: Offset of available memory area (page-based)
+ * @size: Size of available memory area (page-based)
+ *
+ * Initialize a new offset-manager. The offset and area size available for the
+ * manager are given as @page_offset and @size. Both are interpreted as
+ * page-numbers, not bytes.
+ *
+ * Adding/removing nodes from the manager is locked internally and protected
+ * against concurrent access. However, node allocation and destruction is left
+ * for the caller. While calling into the vma-manager, a given node must
+ * always be guaranteed to be referenced.
+ */
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+ unsigned long page_offset, unsigned long size)
+{
+ rwlock_init(&mgr->vm_lock);
+ mgr->vm_addr_space_rb = RB_ROOT;
+ drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_init);
+
+/**
+ * drm_vma_offset_manager_destroy() - Destroy offset manager
+ * @mgr: Manager object
+ *
+ * Destroy an object manager which was previously created via
+ * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
+ * before destroying the manager. Otherwise, drm_mm will refuse to free the
+ * requested resources.
+ *
+ * The manager must not be accessed after this function is called.
+ */
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
+{
+ /* take the lock to protect against buggy drivers */
+ write_lock(&mgr->vm_lock);
+ drm_mm_takedown(&mgr->vm_addr_space_mm);
+ write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
+
+/**
+ * drm_vma_offset_lookup() - Find node in offset space
+ * @mgr: Manager object
+ * @start: Start address for object (page-based)
+ * @pages: Size of object (page-based)
+ *
+ * Find a node given a start address and object size. This returns the _best_
+ * match for the given node. That is, @start may point somewhere into a valid
+ * region and the given node will be returned, as long as the node spans the
+ * whole requested area (given the size in number of pages as @pages).
+ *
+ * RETURNS:
+ * Returns NULL if no suitable node can be found. Otherwise, the best match
+ * is returned. It's the caller's responsibility to make sure the node doesn't
+ * get destroyed before the caller can access it.
+ */
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node;
+
+ read_lock(&mgr->vm_lock);
+ node = drm_vma_offset_lookup_locked(mgr, start, pages);
+ read_unlock(&mgr->vm_lock);
+
+ return node;
+}
+EXPORT_SYMBOL(drm_vma_offset_lookup);
+
+/**
+ * drm_vma_offset_lookup_locked() - Find node in offset space
+ * @mgr: Manager object
+ * @start: Start address for object (page-based)
+ * @pages: Size of object (page-based)
+ *
+ * Same as drm_vma_offset_lookup() but requires the caller to lock offset lookup
+ * manually. See drm_vma_offset_lock_lookup() for an example.
+ *
+ * RETURNS:
+ * Returns NULL if no suitable node can be found. Otherwise, the best match
+ * is returned.
+ */
+struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node, *best;
+ struct rb_node *iter;
+ unsigned long offset;
+
+ iter = mgr->vm_addr_space_rb.rb_node;
+ best = NULL;
+
+ while (likely(iter)) {
+ node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
+ offset = node->vm_node.start;
+ if (start >= offset) {
+ iter = iter->rb_right;
+ best = node;
+ if (start == offset)
+ break;
+ } else {
+ iter = iter->rb_left;
+ }
+ }
+
+ /* verify that the node spans the requested area */
+ if (best) {
+ offset = best->vm_node.start + best->vm_pages;
+ if (offset > start + pages)
+ best = NULL;
+ }
+
+ return best;
+}
+EXPORT_SYMBOL(drm_vma_offset_lookup_locked);
+
+/* internal helper to link @node into the rb-tree */
+static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node)
+{
+ struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
+ struct rb_node *parent = NULL;
+ struct drm_vma_offset_node *iter_node;
+
+ while (likely(*iter)) {
+ parent = *iter;
+ iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
+
+ if (node->vm_node.start < iter_node->vm_node.start)
+ iter = &(*iter)->rb_left;
+ else if (node->vm_node.start > iter_node->vm_node.start)
+ iter = &(*iter)->rb_right;
+ else
+ BUG();
+ }
+
+ rb_link_node(&node->vm_rb, parent, iter);
+ rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
+}
+
+/**
+ * drm_vma_offset_add() - Add offset node to manager
+ * @mgr: Manager object
+ * @node: Node to be added
+ * @pages: Allocation size visible to user-space (in number of pages)
+ *
+ * Add a node to the offset-manager. If the node was already added, this does
+ * nothing and return 0. @pages is the size of the object given in number of
+ * pages.
+ * After this call succeeds, you can access the offset of the node until it
+ * is removed again.
+ *
+ * If this call fails, it is safe to retry the operation or call
+ * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
+ * case.
+ *
+ * @pages is not required to be the same size as the underlying memory object
+ * that you want to map. It only limits the size that user-space can map into
+ * their address space.
+ *
+ * RETURNS:
+ * 0 on success, negative error code on failure.
+ */
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node, unsigned long pages)
+{
+ int ret;
+
+ write_lock(&mgr->vm_lock);
+
+ if (drm_mm_node_allocated(&node->vm_node)) {
+ ret = 0;
+ goto out_unlock;
+ }
+
+ ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
+ &node->vm_node, pages, 0, 0);
+ if (ret)
+ goto out_unlock;
+
+ node->vm_pages = pages;
+ _drm_vma_offset_add_rb(mgr, node);
+ goto out_unlock;
+
+out_unlock:
+ write_unlock(&mgr->vm_lock);
+ return ret;
+}
+EXPORT_SYMBOL(drm_vma_offset_add);
+
+/**
+ * drm_vma_offset_remove() - Remove offset node from manager
+ * @mgr: Manager object
+ * @node: Node to be removed
+ *
+ * Remove a node from the offset manager. If the node wasn't added before, this
+ * does nothing. After this call returns, the offset of the node must be not
+ * accessed, anymore.
+ * It is safe to add the node via drm_vma_offset_add() again.
+ */
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node)
+{
+ write_lock(&mgr->vm_lock);
+
+ if (drm_mm_node_allocated(&node->vm_node)) {
+ rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
+ drm_mm_remove_node(&node->vm_node);
+ memset(&node->vm_node, 0, sizeof(node->vm_node));
+ node->vm_pages = 0;
+ }
+
+ write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_remove);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
new file mode 100644
index 0000000..ace2925
--- /dev/null
+++ b/include/drm/drm_vma_manager.h
@@ -0,0 +1,180 @@
+#ifndef __DRM_VMA_MANAGER_H__
+#define __DRM_VMA_MANAGER_H__
+
+/*
+ * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drm_mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+struct drm_vma_offset_node {
+ struct drm_mm_node vm_node;
+ struct rb_node vm_rb;
+ unsigned long vm_pages;
+};
+
+struct drm_vma_offset_manager {
+ rwlock_t vm_lock;
+ struct rb_root vm_addr_space_rb;
+ struct drm_mm vm_addr_space_mm;
+};
+
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+ unsigned long page_offset, unsigned long size);
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
+
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages);
+struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages);
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node, unsigned long pages);
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node);
+
+/**
+ * drm_vma_offset_exact_lookup() - Look up node by exact address
+ * @mgr: Manager object
+ * @start: Start address (page-based, not byte-based)
+ *
+ * Same as drm_vma_offset_lookup() but does not allow any offset into the node.
+ * It only returns the exact object with the given start address.
+ *
+ * RETURNS:
+ * Node at exact start address @start.
+ */
+static inline struct drm_vma_offset_node *
+drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start)
+{
+ return drm_vma_offset_lookup(mgr, start, 1);
+}
+
+/**
+ * drm_vma_offset_lock_lookup() - Lock lookup for extended private use
+ * @mgr: Manager object
+ *
+ * Lock VMA manager for extended lookups. Only *_locked() VMA function calls
+ * are allowed while holding this lock. All other contexts are blocked from VMA
+ * until the lock is released via drm_vma_offset_unlock_lookup().
+ *
+ * Use this if you need to take a reference to the objects returned by
+ * drm_vma_offset_lookup_locked() before releasing this lock again.
+ *
+ * This lock must not be used for anything else than extended lookups. You must
+ * not call any other VMA helpers while holding this lock.
+ *
+ * Note: You're in atomic-context while holding this lock!
+ *
+ * Example:
+ * drm_vma_offset_lock_lookup(mgr);
+ * node = drm_vma_offset_lookup_locked(mgr);
+ * if (node)
+ * kref_get_unless_zero(container_of(node, sth, entr));
+ * drm_vma_offset_unlock_lookup(mgr);
+ */
+static inline void drm_vma_offset_lock_lookup(struct drm_vma_offset_manager *mgr)
+{
+ read_lock(&mgr->vm_lock);
+}
+
+/**
+ * drm_vma_offset_unlock_lookup() - Unlock lookup for extended private use
+ * @mgr: Manager object
+ *
+ * Release lookup-lock. See drm_vma_offset_lock_lookup() for more information.
+ */
+static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *mgr)
+{
+ read_unlock(&mgr->vm_lock);
+}
+
+/**
+ * drm_vma_node_reset() - Initialize or reset node object
+ * @node: Node to initialize or reset
+ *
+ * Reset a node to its initial state. You must call this before using @node with
+ * any vma offset manager.
+ *
+ * This must not be called on an already allocated node, or you will leak
+ * memory.
+ */
+static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
+{
+ memset(node, 0, sizeof(*node));
+}
+
+/**
+ * drm_vma_node_start() - Return start address for page-based addressing
+ * @node: Node to inspect
+ *
+ * Return the start address of the given node. This can be used as offset into
+ * the linear VM space that is provided by the VMA offset manager. Note that
+ * this can only be used for page-based addressing. If you need a proper offset
+ * for user-space mappings, you must apply "<< PAGE_SHIFT" or use the
+ * drm_vma_node_offset_addr() helper instead.
+ *
+ * RETURNS:
+ * Start address of @node for page-based addressing. 0 if the node does not
+ * have an offset allocated.
+ */
+static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
+{
+ return node->vm_node.start;
+}
+
+/**
+ * drm_vma_node_has_offset() - Check whether node is added to offset manager
+ * @node: Node to be checked
+ *
+ * RETURNS:
+ * true iff the node was previously allocated an offset and added to
+ * an vma offset manager.
+ */
+static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
+{
+ return drm_mm_node_allocated(&node->vm_node);
+}
+
+/**
+ * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps
+ * @node: Linked offset node
+ *
+ * Same as drm_vma_node_start() but returns the address as a valid offset that
+ * can be used for user-space mappings during mmap().
+ * This must not be called on unlinked nodes.
+ *
+ * RETURNS:
+ * Offset of @node for byte-based addressing. 0 if the node does not have an
+ * object allocated.
+ */
+static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
+{
+ return ((__u64)node->vm_node.start) << PAGE_SHIFT;
+}
+
+#endif /* __DRM_VMA_MANAGER_H__ */
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v4 1/4] drm: add unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 1/4] drm: add unified vma offset manager David Herrmann
@ 2013-07-24 15:35 ` Daniel Vetter
2013-07-24 16:10 ` David Herrmann
2013-07-24 19:06 ` [PATCH v5 " David Herrmann
1 sibling, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2013-07-24 15:35 UTC (permalink / raw)
To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter
On Tue, Jul 23, 2013 at 02:47:13PM +0200, David Herrmann wrote:
> If we want to map GPU memory into user-space, we need to linearize the
> addresses to not confuse mm-core. Currently, GEM and TTM both implement
> their own offset-managers to assign a pgoff to each object for user-space
> CPU access. GEM uses a hash-table, TTM uses an rbtree.
>
> This patch provides a unified implementation that can be used to replace
> both. TTM allows partial mmaps with a given offset, so we cannot use
> hashtables as the start address may not be known at mmap time. Hence, we
> use the rbtree-implementation of TTM.
>
> We could easily update drm_mm to use an rbtree instead of a linked list
> for it's object list and thus drop the rbtree from the vma-manager.
> However, this would slow down drm_mm object allocation for all other
> use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
> Hence, use the separate tree but allow for later migration.
>
> This is a rewrite of the 2012-proposal by David Airlie <airlied@linux.ie>
>
> v2:
> - fix Docbook integration
> - drop drm_mm_node_linked() and use drm_mm_node_allocated()
> - remove unjustified likely/unlikely usage (but keep for rbtree paths)
> - remove BUG_ON() as drm_mm already does that
> - clarify page-based vs. byte-based addresses
> - use drm_vma_node_reset() for initialization, too
> v4:
> - allow external locking via drm_vma_offset_un/lock_lookup()
> - add locked lookup helper drm_vma_offset_lookup_locked()
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Documentation/DocBook/drm.tmpl | 6 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_vma_manager.c | 283 ++++++++++++++++++++++++++++++++++++++
> include/drm/drm_vma_manager.h | 180 ++++++++++++++++++++++++
> 4 files changed, 470 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_vma_manager.c
> create mode 100644 include/drm/drm_vma_manager.h
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 7d1278e..87e22ec 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2212,6 +2212,12 @@ void intel_crt_init(struct drm_device *dev)
> !Iinclude/drm/drm_rect.h
> !Edrivers/gpu/drm/drm_rect.c
> </sect2>
> + <sect2>
> + <title>VMA Offset Manager</title>
> +!Pdrivers/gpu/drm/drm_vma_manager.c vma offset manager
> +!Edrivers/gpu/drm/drm_vma_manager.c
> +!Iinclude/drm/drm_vma_manager.h
> + </sect2>
> </sect1>
>
> <!-- Internals: kms properties -->
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 801bcaf..d943b94 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -13,7 +13,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
> drm_crtc.o drm_modes.o drm_edid.o \
> drm_info.o drm_debugfs.o drm_encoder_slave.o \
> drm_trace_points.o drm_global.o drm_prime.o \
> - drm_rect.o
> + drm_rect.o drm_vma_manager.o
>
> drm-$(CONFIG_COMPAT) += drm_ioc32.o
> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
> new file mode 100644
> index 0000000..27760bd
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_vma_manager.c
> @@ -0,0 +1,283 @@
> +/*
> + * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA
> + * Copyright (c) 2012 David Airlie <airlied@linux.ie>
> + * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mm.h>
> +#include <drm/drm_vma_manager.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/rbtree.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +/**
> + * DOC: vma offset manager
> + *
> + * The vma-manager is responsible to map arbitrary driver-dependent memory
> + * regions into the linear user address-space. It provides offsets to the
> + * caller which can then be used on the address_space of the drm-device. It
> + * takes care to not overlap regions, size them appropriately and to not
> + * confuse mm-core by inconsistent fake vm_pgoff fields.
> + * Drivers shouldn't use this for object placement in VMEM. This manager should
> + * only be used to manage mappings into linear user-space VMs.
> + *
> + * We use drm_mm as backend to manage object allocations. But it is highly
> + * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
> + * speed up offset lookups.
> + *
> + * You must not use multiple offset managers on a single address_space.
> + * Otherwise, mm-core will be unable to tear down memory mappings as the VM will
> + * no longer be linear. Please use VM_NONLINEAR in that case and implement your
> + * own offset managers.
> + *
> + * This offset manager works on page-based addresses. That is, every argument
> + * and return code (with the exception of drm_vma_node_offset_addr()) is given
> + * in number of pages, not number of bytes. That means, object sizes and offsets
> + * must always be page-aligned (as usual).
> + * If you want to get a valid byte-based user-space address for a given offset,
> + * please see drm_vma_node_offset_addr().
> + */
> +
> +/**
> + * drm_vma_offset_manager_init - Initialize new offset-manager
> + * @mgr: Manager object
> + * @page_offset: Offset of available memory area (page-based)
> + * @size: Size of available memory area (page-based)
Bikeshed on naming: s/memory area/address space range/. Feel free to
ignore.
> + *
> + * Initialize a new offset-manager. The offset and area size available for the
> + * manager are given as @page_offset and @size. Both are interpreted as
> + * page-numbers, not bytes.
> + *
> + * Adding/removing nodes from the manager is locked internally and protected
> + * against concurrent access. However, node allocation and destruction is left
> + * for the caller. While calling into the vma-manager, a given node must
> + * always be guaranteed to be referenced.
> + */
> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
> + unsigned long page_offset, unsigned long size)
> +{
> + rwlock_init(&mgr->vm_lock);
> + mgr->vm_addr_space_rb = RB_ROOT;
> + drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_manager_init);
> +
> +/**
> + * drm_vma_offset_manager_destroy() - Destroy offset manager
> + * @mgr: Manager object
> + *
> + * Destroy an object manager which was previously created via
> + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
> + * before destroying the manager. Otherwise, drm_mm will refuse to free the
> + * requested resources.
> + *
> + * The manager must not be accessed after this function is called.
> + */
> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
> +{
> + /* take the lock to protect against buggy drivers */
> + write_lock(&mgr->vm_lock);
> + drm_mm_takedown(&mgr->vm_addr_space_mm);
> + write_unlock(&mgr->vm_lock);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
> +
> +/**
> + * drm_vma_offset_lookup() - Find node in offset space
> + * @mgr: Manager object
> + * @start: Start address for object (page-based)
> + * @pages: Size of object (page-based)
> + *
> + * Find a node given a start address and object size. This returns the _best_
> + * match for the given node. That is, @start may point somewhere into a valid
> + * region and the given node will be returned, as long as the node spans the
> + * whole requested area (given the size in number of pages as @pages).
> + *
> + * RETURNS:
> + * Returns NULL if no suitable node can be found. Otherwise, the best match
> + * is returned. It's the caller's responsibility to make sure the node doesn't
> + * get destroyed before the caller can access it.
> + */
> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
> + unsigned long start,
> + unsigned long pages)
> +{
> + struct drm_vma_offset_node *node;
> +
> + read_lock(&mgr->vm_lock);
> + node = drm_vma_offset_lookup_locked(mgr, start, pages);
> + read_unlock(&mgr->vm_lock);
> +
> + return node;
> +}
> +EXPORT_SYMBOL(drm_vma_offset_lookup);
> +
> +/**
> + * drm_vma_offset_lookup_locked() - Find node in offset space
> + * @mgr: Manager object
> + * @start: Start address for object (page-based)
> + * @pages: Size of object (page-based)
> + *
> + * Same as drm_vma_offset_lookup() but requires the caller to lock offset lookup
> + * manually. See drm_vma_offset_lock_lookup() for an example.
> + *
> + * RETURNS:
> + * Returns NULL if no suitable node can be found. Otherwise, the best match
> + * is returned.
> + */
> +struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
> + unsigned long start,
> + unsigned long pages)
> +{
> + struct drm_vma_offset_node *node, *best;
> + struct rb_node *iter;
> + unsigned long offset;
> +
> + iter = mgr->vm_addr_space_rb.rb_node;
> + best = NULL;
> +
> + while (likely(iter)) {
> + node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
> + offset = node->vm_node.start;
> + if (start >= offset) {
> + iter = iter->rb_right;
> + best = node;
> + if (start == offset)
> + break;
> + } else {
> + iter = iter->rb_left;
> + }
> + }
> +
> + /* verify that the node spans the requested area */
> + if (best) {
> + offset = best->vm_node.start + best->vm_pages;
> + if (offset > start + pages)
> + best = NULL;
> + }
> +
> + return best;
> +}
> +EXPORT_SYMBOL(drm_vma_offset_lookup_locked);
> +
> +/* internal helper to link @node into the rb-tree */
> +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
> + struct drm_vma_offset_node *node)
> +{
> + struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
> + struct rb_node *parent = NULL;
> + struct drm_vma_offset_node *iter_node;
> +
> + while (likely(*iter)) {
> + parent = *iter;
> + iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
> +
> + if (node->vm_node.start < iter_node->vm_node.start)
> + iter = &(*iter)->rb_left;
> + else if (node->vm_node.start > iter_node->vm_node.start)
> + iter = &(*iter)->rb_right;
> + else
> + BUG();
> + }
> +
> + rb_link_node(&node->vm_rb, parent, iter);
> + rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
> +}
> +
> +/**
> + * drm_vma_offset_add() - Add offset node to manager
> + * @mgr: Manager object
> + * @node: Node to be added
> + * @pages: Allocation size visible to user-space (in number of pages)
> + *
> + * Add a node to the offset-manager. If the node was already added, this does
> + * nothing and return 0. @pages is the size of the object given in number of
> + * pages.
> + * After this call succeeds, you can access the offset of the node until it
> + * is removed again.
> + *
> + * If this call fails, it is safe to retry the operation or call
> + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
> + * case.
> + *
> + * @pages is not required to be the same size as the underlying memory object
> + * that you want to map. It only limits the size that user-space can map into
> + * their address space.
> + *
> + * RETURNS:
> + * 0 on success, negative error code on failure.
> + */
> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
> + struct drm_vma_offset_node *node, unsigned long pages)
> +{
> + int ret;
> +
> + write_lock(&mgr->vm_lock);
> +
> + if (drm_mm_node_allocated(&node->vm_node)) {
> + ret = 0;
> + goto out_unlock;
> + }
> +
> + ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
> + &node->vm_node, pages, 0, 0);
> + if (ret)
> + goto out_unlock;
> +
> + node->vm_pages = pages;
Why not just use node->vm_node.size and remove vm_pages?
> + _drm_vma_offset_add_rb(mgr, node);
> + goto out_unlock;
Unnecessary goto.
> +
> +out_unlock:
> + write_unlock(&mgr->vm_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_vma_offset_add);
> +
> +/**
> + * drm_vma_offset_remove() - Remove offset node from manager
> + * @mgr: Manager object
> + * @node: Node to be removed
> + *
> + * Remove a node from the offset manager. If the node wasn't added before, this
> + * does nothing. After this call returns, the offset of the node must be not
> + * accessed, anymore.
I think we could clarify this a little "After this call returns the offset
node must not be used until a new offset is allocatd with
drm_vma_offset_add() again. Helper functions like
drm_vma_node_offset_addr() will return invalid data"
> + * It is safe to add the node via drm_vma_offset_add() again.
> + */
> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
> + struct drm_vma_offset_node *node)
> +{
> + write_lock(&mgr->vm_lock);
> +
> + if (drm_mm_node_allocated(&node->vm_node)) {
> + rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
> + drm_mm_remove_node(&node->vm_node);
> + memset(&node->vm_node, 0, sizeof(node->vm_node));
> + node->vm_pages = 0;
> + }
> +
> + write_unlock(&mgr->vm_lock);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_remove);
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> new file mode 100644
> index 0000000..ace2925
> --- /dev/null
> +++ b/include/drm/drm_vma_manager.h
> @@ -0,0 +1,180 @@
> +#ifndef __DRM_VMA_MANAGER_H__
> +#define __DRM_VMA_MANAGER_H__
> +
> +/*
> + * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <drm/drm_mm.h>
> +#include <linux/module.h>
> +#include <linux/rbtree.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +struct drm_vma_offset_node {
> + struct drm_mm_node vm_node;
> + struct rb_node vm_rb;
> + unsigned long vm_pages;
> +};
> +
> +struct drm_vma_offset_manager {
> + rwlock_t vm_lock;
> + struct rb_root vm_addr_space_rb;
> + struct drm_mm vm_addr_space_mm;
> +};
> +
> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
> + unsigned long page_offset, unsigned long size);
> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
> +
> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
> + unsigned long start,
> + unsigned long pages);
> +struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
> + unsigned long start,
> + unsigned long pages);
> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
> + struct drm_vma_offset_node *node, unsigned long pages);
> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
> + struct drm_vma_offset_node *node);
> +
> +/**
> + * drm_vma_offset_exact_lookup() - Look up node by exact address
> + * @mgr: Manager object
> + * @start: Start address (page-based, not byte-based)
> + *
> + * Same as drm_vma_offset_lookup() but does not allow any offset into the node.
> + * It only returns the exact object with the given start address.
> + *
> + * RETURNS:
> + * Node at exact start address @start.
> + */
> +static inline struct drm_vma_offset_node *
> +drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
> + unsigned long start)
> +{
> + return drm_vma_offset_lookup(mgr, start, 1);
> +}
> +
> +/**
> + * drm_vma_offset_lock_lookup() - Lock lookup for extended private use
> + * @mgr: Manager object
> + *
> + * Lock VMA manager for extended lookups. Only *_locked() VMA function calls
> + * are allowed while holding this lock. All other contexts are blocked from VMA
> + * until the lock is released via drm_vma_offset_unlock_lookup().
> + *
> + * Use this if you need to take a reference to the objects returned by
> + * drm_vma_offset_lookup_locked() before releasing this lock again.
> + *
> + * This lock must not be used for anything else than extended lookups. You must
> + * not call any other VMA helpers while holding this lock.
> + *
> + * Note: You're in atomic-context while holding this lock!
> + *
> + * Example:
> + * drm_vma_offset_lock_lookup(mgr);
> + * node = drm_vma_offset_lookup_locked(mgr);
> + * if (node)
> + * kref_get_unless_zero(container_of(node, sth, entr));
> + * drm_vma_offset_unlock_lookup(mgr);
> + */
> +static inline void drm_vma_offset_lock_lookup(struct drm_vma_offset_manager *mgr)
> +{
> + read_lock(&mgr->vm_lock);
> +}
> +
> +/**
> + * drm_vma_offset_unlock_lookup() - Unlock lookup for extended private use
> + * @mgr: Manager object
> + *
> + * Release lookup-lock. See drm_vma_offset_lock_lookup() for more information.
> + */
> +static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *mgr)
> +{
> + read_unlock(&mgr->vm_lock);
> +}
> +
> +/**
> + * drm_vma_node_reset() - Initialize or reset node object
> + * @node: Node to initialize or reset
> + *
> + * Reset a node to its initial state. You must call this before using @node with
> + * any vma offset manager.
The must here feels a bit strong since this is just a memset and pretty
much everyone allocates with kzalloc. So calling this is redundant. Maybe
"This must be called if @node isn't already cleared (e.g. by allocating
with kzalloc) before it using it with any vma offset manager."
> + *
> + * This must not be called on an already allocated node, or you will leak
> + * memory.
> + */
> +static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
> +{
> + memset(node, 0, sizeof(*node));
> +}
> +
> +/**
> + * drm_vma_node_start() - Return start address for page-based addressing
> + * @node: Node to inspect
> + *
> + * Return the start address of the given node. This can be used as offset into
> + * the linear VM space that is provided by the VMA offset manager. Note that
> + * this can only be used for page-based addressing. If you need a proper offset
> + * for user-space mappings, you must apply "<< PAGE_SHIFT" or use the
> + * drm_vma_node_offset_addr() helper instead.
> + *
> + * RETURNS:
> + * Start address of @node for page-based addressing. 0 if the node does not
> + * have an offset allocated.
> + */
> +static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
> +{
> + return node->vm_node.start;
> +}
> +
> +/**
> + * drm_vma_node_has_offset() - Check whether node is added to offset manager
> + * @node: Node to be checked
> + *
> + * RETURNS:
> + * true iff the node was previously allocated an offset and added to
> + * an vma offset manager.
> + */
> +static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
> +{
> + return drm_mm_node_allocated(&node->vm_node);
> +}
> +
> +/**
> + * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps
> + * @node: Linked offset node
> + *
> + * Same as drm_vma_node_start() but returns the address as a valid offset that
> + * can be used for user-space mappings during mmap().
> + * This must not be called on unlinked nodes.
> + *
> + * RETURNS:
> + * Offset of @node for byte-based addressing. 0 if the node does not have an
> + * object allocated.
> + */
> +static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
> +{
> + return ((__u64)node->vm_node.start) << PAGE_SHIFT;
> +}
> +
> +#endif /* __DRM_VMA_MANAGER_H__ */
With the above tiny bikesheds addressed this is:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v4 1/4] drm: add unified vma offset manager
2013-07-24 15:35 ` Daniel Vetter
@ 2013-07-24 16:10 ` David Herrmann
0 siblings, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 16:10 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Dave Airlie, dri-devel, Daniel Vetter
Hi
On Wed, Jul 24, 2013 at 5:35 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jul 23, 2013 at 02:47:13PM +0200, David Herrmann wrote:
>> If we want to map GPU memory into user-space, we need to linearize the
>> addresses to not confuse mm-core. Currently, GEM and TTM both implement
>> their own offset-managers to assign a pgoff to each object for user-space
>> CPU access. GEM uses a hash-table, TTM uses an rbtree.
>>
>> This patch provides a unified implementation that can be used to replace
>> both. TTM allows partial mmaps with a given offset, so we cannot use
>> hashtables as the start address may not be known at mmap time. Hence, we
>> use the rbtree-implementation of TTM.
>>
>> We could easily update drm_mm to use an rbtree instead of a linked list
>> for it's object list and thus drop the rbtree from the vma-manager.
>> However, this would slow down drm_mm object allocation for all other
>> use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
>> Hence, use the separate tree but allow for later migration.
>>
>> This is a rewrite of the 2012-proposal by David Airlie <airlied@linux.ie>
>>
>> v2:
>> - fix Docbook integration
>> - drop drm_mm_node_linked() and use drm_mm_node_allocated()
>> - remove unjustified likely/unlikely usage (but keep for rbtree paths)
>> - remove BUG_ON() as drm_mm already does that
>> - clarify page-based vs. byte-based addresses
>> - use drm_vma_node_reset() for initialization, too
>> v4:
>> - allow external locking via drm_vma_offset_un/lock_lookup()
>> - add locked lookup helper drm_vma_offset_lookup_locked()
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>> Documentation/DocBook/drm.tmpl | 6 +
>> drivers/gpu/drm/Makefile | 2 +-
>> drivers/gpu/drm/drm_vma_manager.c | 283 ++++++++++++++++++++++++++++++++++++++
>> include/drm/drm_vma_manager.h | 180 ++++++++++++++++++++++++
>> 4 files changed, 470 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/drm_vma_manager.c
>> create mode 100644 include/drm/drm_vma_manager.h
>>
>> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
>> index 7d1278e..87e22ec 100644
>> --- a/Documentation/DocBook/drm.tmpl
>> +++ b/Documentation/DocBook/drm.tmpl
>> @@ -2212,6 +2212,12 @@ void intel_crt_init(struct drm_device *dev)
>> !Iinclude/drm/drm_rect.h
>> !Edrivers/gpu/drm/drm_rect.c
>> </sect2>
>> + <sect2>
>> + <title>VMA Offset Manager</title>
>> +!Pdrivers/gpu/drm/drm_vma_manager.c vma offset manager
>> +!Edrivers/gpu/drm/drm_vma_manager.c
>> +!Iinclude/drm/drm_vma_manager.h
>> + </sect2>
>> </sect1>
>>
>> <!-- Internals: kms properties -->
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 801bcaf..d943b94 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -13,7 +13,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>> drm_crtc.o drm_modes.o drm_edid.o \
>> drm_info.o drm_debugfs.o drm_encoder_slave.o \
>> drm_trace_points.o drm_global.o drm_prime.o \
>> - drm_rect.o
>> + drm_rect.o drm_vma_manager.o
>>
>> drm-$(CONFIG_COMPAT) += drm_ioc32.o
>> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
>> new file mode 100644
>> index 0000000..27760bd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_vma_manager.c
>> @@ -0,0 +1,283 @@
>> +/*
>> + * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA
>> + * Copyright (c) 2012 David Airlie <airlied@linux.ie>
>> + * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_mm.h>
>> +#include <drm/drm_vma_manager.h>
>> +#include <linux/mm.h>
>> +#include <linux/module.h>
>> +#include <linux/rbtree.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +
>> +/**
>> + * DOC: vma offset manager
>> + *
>> + * The vma-manager is responsible to map arbitrary driver-dependent memory
>> + * regions into the linear user address-space. It provides offsets to the
>> + * caller which can then be used on the address_space of the drm-device. It
>> + * takes care to not overlap regions, size them appropriately and to not
>> + * confuse mm-core by inconsistent fake vm_pgoff fields.
>> + * Drivers shouldn't use this for object placement in VMEM. This manager should
>> + * only be used to manage mappings into linear user-space VMs.
>> + *
>> + * We use drm_mm as backend to manage object allocations. But it is highly
>> + * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
>> + * speed up offset lookups.
>> + *
>> + * You must not use multiple offset managers on a single address_space.
>> + * Otherwise, mm-core will be unable to tear down memory mappings as the VM will
>> + * no longer be linear. Please use VM_NONLINEAR in that case and implement your
>> + * own offset managers.
>> + *
>> + * This offset manager works on page-based addresses. That is, every argument
>> + * and return code (with the exception of drm_vma_node_offset_addr()) is given
>> + * in number of pages, not number of bytes. That means, object sizes and offsets
>> + * must always be page-aligned (as usual).
>> + * If you want to get a valid byte-based user-space address for a given offset,
>> + * please see drm_vma_node_offset_addr().
>> + */
>> +
>> +/**
>> + * drm_vma_offset_manager_init - Initialize new offset-manager
>> + * @mgr: Manager object
>> + * @page_offset: Offset of available memory area (page-based)
>> + * @size: Size of available memory area (page-based)
>
> Bikeshed on naming: s/memory area/address space range/. Feel free to
> ignore.
Makes sense, yepp.
>> + *
>> + * Initialize a new offset-manager. The offset and area size available for the
>> + * manager are given as @page_offset and @size. Both are interpreted as
>> + * page-numbers, not bytes.
>> + *
>> + * Adding/removing nodes from the manager is locked internally and protected
>> + * against concurrent access. However, node allocation and destruction is left
>> + * for the caller. While calling into the vma-manager, a given node must
>> + * always be guaranteed to be referenced.
>> + */
>> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
>> + unsigned long page_offset, unsigned long size)
>> +{
>> + rwlock_init(&mgr->vm_lock);
>> + mgr->vm_addr_space_rb = RB_ROOT;
>> + drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_manager_init);
>> +
>> +/**
>> + * drm_vma_offset_manager_destroy() - Destroy offset manager
>> + * @mgr: Manager object
>> + *
>> + * Destroy an object manager which was previously created via
>> + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
>> + * before destroying the manager. Otherwise, drm_mm will refuse to free the
>> + * requested resources.
>> + *
>> + * The manager must not be accessed after this function is called.
>> + */
>> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
>> +{
>> + /* take the lock to protect against buggy drivers */
>> + write_lock(&mgr->vm_lock);
>> + drm_mm_takedown(&mgr->vm_addr_space_mm);
>> + write_unlock(&mgr->vm_lock);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
>> +
>> +/**
>> + * drm_vma_offset_lookup() - Find node in offset space
>> + * @mgr: Manager object
>> + * @start: Start address for object (page-based)
>> + * @pages: Size of object (page-based)
>> + *
>> + * Find a node given a start address and object size. This returns the _best_
>> + * match for the given node. That is, @start may point somewhere into a valid
>> + * region and the given node will be returned, as long as the node spans the
>> + * whole requested area (given the size in number of pages as @pages).
>> + *
>> + * RETURNS:
>> + * Returns NULL if no suitable node can be found. Otherwise, the best match
>> + * is returned. It's the caller's responsibility to make sure the node doesn't
>> + * get destroyed before the caller can access it.
>> + */
>> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
>> + unsigned long start,
>> + unsigned long pages)
>> +{
>> + struct drm_vma_offset_node *node;
>> +
>> + read_lock(&mgr->vm_lock);
>> + node = drm_vma_offset_lookup_locked(mgr, start, pages);
>> + read_unlock(&mgr->vm_lock);
>> +
>> + return node;
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_lookup);
>> +
>> +/**
>> + * drm_vma_offset_lookup_locked() - Find node in offset space
>> + * @mgr: Manager object
>> + * @start: Start address for object (page-based)
>> + * @pages: Size of object (page-based)
>> + *
>> + * Same as drm_vma_offset_lookup() but requires the caller to lock offset lookup
>> + * manually. See drm_vma_offset_lock_lookup() for an example.
>> + *
>> + * RETURNS:
>> + * Returns NULL if no suitable node can be found. Otherwise, the best match
>> + * is returned.
>> + */
>> +struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
>> + unsigned long start,
>> + unsigned long pages)
>> +{
>> + struct drm_vma_offset_node *node, *best;
>> + struct rb_node *iter;
>> + unsigned long offset;
>> +
>> + iter = mgr->vm_addr_space_rb.rb_node;
>> + best = NULL;
>> +
>> + while (likely(iter)) {
>> + node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
>> + offset = node->vm_node.start;
>> + if (start >= offset) {
>> + iter = iter->rb_right;
>> + best = node;
>> + if (start == offset)
>> + break;
>> + } else {
>> + iter = iter->rb_left;
>> + }
>> + }
>> +
>> + /* verify that the node spans the requested area */
>> + if (best) {
>> + offset = best->vm_node.start + best->vm_pages;
>> + if (offset > start + pages)
>> + best = NULL;
>> + }
>> +
>> + return best;
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_lookup_locked);
>> +
>> +/* internal helper to link @node into the rb-tree */
>> +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
>> + struct drm_vma_offset_node *node)
>> +{
>> + struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
>> + struct rb_node *parent = NULL;
>> + struct drm_vma_offset_node *iter_node;
>> +
>> + while (likely(*iter)) {
>> + parent = *iter;
>> + iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
>> +
>> + if (node->vm_node.start < iter_node->vm_node.start)
>> + iter = &(*iter)->rb_left;
>> + else if (node->vm_node.start > iter_node->vm_node.start)
>> + iter = &(*iter)->rb_right;
>> + else
>> + BUG();
>> + }
>> +
>> + rb_link_node(&node->vm_rb, parent, iter);
>> + rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
>> +}
>> +
>> +/**
>> + * drm_vma_offset_add() - Add offset node to manager
>> + * @mgr: Manager object
>> + * @node: Node to be added
>> + * @pages: Allocation size visible to user-space (in number of pages)
>> + *
>> + * Add a node to the offset-manager. If the node was already added, this does
>> + * nothing and return 0. @pages is the size of the object given in number of
>> + * pages.
>> + * After this call succeeds, you can access the offset of the node until it
>> + * is removed again.
>> + *
>> + * If this call fails, it is safe to retry the operation or call
>> + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
>> + * case.
>> + *
>> + * @pages is not required to be the same size as the underlying memory object
>> + * that you want to map. It only limits the size that user-space can map into
>> + * their address space.
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error code on failure.
>> + */
>> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
>> + struct drm_vma_offset_node *node, unsigned long pages)
>> +{
>> + int ret;
>> +
>> + write_lock(&mgr->vm_lock);
>> +
>> + if (drm_mm_node_allocated(&node->vm_node)) {
>> + ret = 0;
>> + goto out_unlock;
>> + }
>> +
>> + ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
>> + &node->vm_node, pages, 0, 0);
>> + if (ret)
>> + goto out_unlock;
>> +
>> + node->vm_pages = pages;
>
> Why not just use node->vm_node.size and remove vm_pages?
In the first draft I initialized the size directly. I added it to
drm_vma_offset_add() later, so that's a left-over. I will remove the
duplicate and use node->vm_node.size directly.
>> + _drm_vma_offset_add_rb(mgr, node);
>> + goto out_unlock;
>
> Unnecessary goto.
Fixed.
>> +
>> +out_unlock:
>> + write_unlock(&mgr->vm_lock);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_add);
>> +
>> +/**
>> + * drm_vma_offset_remove() - Remove offset node from manager
>> + * @mgr: Manager object
>> + * @node: Node to be removed
>> + *
>> + * Remove a node from the offset manager. If the node wasn't added before, this
>> + * does nothing. After this call returns, the offset of the node must be not
>> + * accessed, anymore.
>
> I think we could clarify this a little "After this call returns the offset
> node must not be used until a new offset is allocatd with
> drm_vma_offset_add() again. Helper functions like
> drm_vma_node_offset_addr() will return invalid data"
They guarantee to return 0. I use that in the debugfs paths so we
don't have to check drm_vma_node_has_offset() all the time. So I guess
I will change it to "return 0 (invalid)" or alike instead of just
"invalid".
>> + * It is safe to add the node via drm_vma_offset_add() again.
>> + */
>> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
>> + struct drm_vma_offset_node *node)
>> +{
>> + write_lock(&mgr->vm_lock);
>> +
>> + if (drm_mm_node_allocated(&node->vm_node)) {
>> + rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
>> + drm_mm_remove_node(&node->vm_node);
>> + memset(&node->vm_node, 0, sizeof(node->vm_node));
>> + node->vm_pages = 0;
>> + }
>> +
>> + write_unlock(&mgr->vm_lock);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_remove);
>> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
>> new file mode 100644
>> index 0000000..ace2925
>> --- /dev/null
>> +++ b/include/drm/drm_vma_manager.h
>> @@ -0,0 +1,180 @@
>> +#ifndef __DRM_VMA_MANAGER_H__
>> +#define __DRM_VMA_MANAGER_H__
>> +
>> +/*
>> + * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include <drm/drm_mm.h>
>> +#include <linux/module.h>
>> +#include <linux/rbtree.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +
>> +struct drm_vma_offset_node {
>> + struct drm_mm_node vm_node;
>> + struct rb_node vm_rb;
>> + unsigned long vm_pages;
>> +};
>> +
>> +struct drm_vma_offset_manager {
>> + rwlock_t vm_lock;
>> + struct rb_root vm_addr_space_rb;
>> + struct drm_mm vm_addr_space_mm;
>> +};
>> +
>> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
>> + unsigned long page_offset, unsigned long size);
>> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
>> +
>> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
>> + unsigned long start,
>> + unsigned long pages);
>> +struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
>> + unsigned long start,
>> + unsigned long pages);
>> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
>> + struct drm_vma_offset_node *node, unsigned long pages);
>> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
>> + struct drm_vma_offset_node *node);
>> +
>> +/**
>> + * drm_vma_offset_exact_lookup() - Look up node by exact address
>> + * @mgr: Manager object
>> + * @start: Start address (page-based, not byte-based)
>> + *
>> + * Same as drm_vma_offset_lookup() but does not allow any offset into the node.
>> + * It only returns the exact object with the given start address.
>> + *
>> + * RETURNS:
>> + * Node at exact start address @start.
>> + */
>> +static inline struct drm_vma_offset_node *
>> +drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
>> + unsigned long start)
>> +{
>> + return drm_vma_offset_lookup(mgr, start, 1);
>> +}
>> +
>> +/**
>> + * drm_vma_offset_lock_lookup() - Lock lookup for extended private use
>> + * @mgr: Manager object
>> + *
>> + * Lock VMA manager for extended lookups. Only *_locked() VMA function calls
>> + * are allowed while holding this lock. All other contexts are blocked from VMA
>> + * until the lock is released via drm_vma_offset_unlock_lookup().
>> + *
>> + * Use this if you need to take a reference to the objects returned by
>> + * drm_vma_offset_lookup_locked() before releasing this lock again.
>> + *
>> + * This lock must not be used for anything else than extended lookups. You must
>> + * not call any other VMA helpers while holding this lock.
>> + *
>> + * Note: You're in atomic-context while holding this lock!
>> + *
>> + * Example:
>> + * drm_vma_offset_lock_lookup(mgr);
>> + * node = drm_vma_offset_lookup_locked(mgr);
>> + * if (node)
>> + * kref_get_unless_zero(container_of(node, sth, entr));
>> + * drm_vma_offset_unlock_lookup(mgr);
>> + */
>> +static inline void drm_vma_offset_lock_lookup(struct drm_vma_offset_manager *mgr)
>> +{
>> + read_lock(&mgr->vm_lock);
>> +}
>> +
>> +/**
>> + * drm_vma_offset_unlock_lookup() - Unlock lookup for extended private use
>> + * @mgr: Manager object
>> + *
>> + * Release lookup-lock. See drm_vma_offset_lock_lookup() for more information.
>> + */
>> +static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *mgr)
>> +{
>> + read_unlock(&mgr->vm_lock);
>> +}
>> +
>> +/**
>> + * drm_vma_node_reset() - Initialize or reset node object
>> + * @node: Node to initialize or reset
>> + *
>> + * Reset a node to its initial state. You must call this before using @node with
>> + * any vma offset manager.
>
> The must here feels a bit strong since this is just a memset and pretty
> much everyone allocates with kzalloc. So calling this is redundant. Maybe
>
> "This must be called if @node isn't already cleared (e.g. by allocating
> with kzalloc) before it using it with any vma offset manager."
Makes sense. drm_mm has the same semantics, anyway.
>> + *
>> + * This must not be called on an already allocated node, or you will leak
>> + * memory.
>> + */
>> +static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
>> +{
>> + memset(node, 0, sizeof(*node));
>> +}
>> +
>> +/**
>> + * drm_vma_node_start() - Return start address for page-based addressing
>> + * @node: Node to inspect
>> + *
>> + * Return the start address of the given node. This can be used as offset into
>> + * the linear VM space that is provided by the VMA offset manager. Note that
>> + * this can only be used for page-based addressing. If you need a proper offset
>> + * for user-space mappings, you must apply "<< PAGE_SHIFT" or use the
>> + * drm_vma_node_offset_addr() helper instead.
>> + *
>> + * RETURNS:
>> + * Start address of @node for page-based addressing. 0 if the node does not
>> + * have an offset allocated.
>> + */
>> +static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
>> +{
>> + return node->vm_node.start;
>> +}
>> +
>> +/**
>> + * drm_vma_node_has_offset() - Check whether node is added to offset manager
>> + * @node: Node to be checked
>> + *
>> + * RETURNS:
>> + * true iff the node was previously allocated an offset and added to
>> + * an vma offset manager.
>> + */
>> +static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
>> +{
>> + return drm_mm_node_allocated(&node->vm_node);
>> +}
>> +
>> +/**
>> + * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps
>> + * @node: Linked offset node
>> + *
>> + * Same as drm_vma_node_start() but returns the address as a valid offset that
>> + * can be used for user-space mappings during mmap().
>> + * This must not be called on unlinked nodes.
>> + *
>> + * RETURNS:
>> + * Offset of @node for byte-based addressing. 0 if the node does not have an
>> + * object allocated.
>> + */
>> +static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
>> +{
>> + return ((__u64)node->vm_node.start) << PAGE_SHIFT;
>> +}
>> +
>> +#endif /* __DRM_VMA_MANAGER_H__ */
>
> With the above tiny bikesheds addressed this is:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks
David
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 1/4] drm: add unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 1/4] drm: add unified vma offset manager David Herrmann
2013-07-24 15:35 ` Daniel Vetter
@ 2013-07-24 19:06 ` David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 19:06 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie
If we want to map GPU memory into user-space, we need to linearize the
addresses to not confuse mm-core. Currently, GEM and TTM both implement
their own offset-managers to assign a pgoff to each object for user-space
CPU access. GEM uses a hash-table, TTM uses an rbtree.
This patch provides a unified implementation that can be used to replace
both. TTM allows partial mmaps with a given offset, so we cannot use
hashtables as the start address may not be known at mmap time. Hence, we
use the rbtree-implementation of TTM.
We could easily update drm_mm to use an rbtree instead of a linked list
for it's object list and thus drop the rbtree from the vma-manager.
However, this would slow down drm_mm object allocation for all other
use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
Hence, use the separate tree but allow for later migration.
This is a rewrite of the 2012-proposal by David Airlie <airlied@linux.ie>
v2:
- fix Docbook integration
- drop drm_mm_node_linked() and use drm_mm_node_allocated()
- remove unjustified likely/unlikely usage (but keep for rbtree paths)
- remove BUG_ON() as drm_mm already does that
- clarify page-based vs. byte-based addresses
- use drm_vma_node_reset() for initialization, too
v4:
- allow external locking via drm_vma_offset_un/lock_lookup()
- add locked lookup helper drm_vma_offset_lookup_locked()
v5:
- fix drm_vma_offset_lookup() to correctly validate range-mismatches
(fix (offset > start + pages))
- fix drm_vma_offset_exact_lookup() to actually do what it says
- remove redundant vm_pages member (add drm_vma_node_size() helper)
- remove unneeded goto
- fix documentation
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Documentation/DocBook/drm.tmpl | 6 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_vma_manager.c | 281 ++++++++++++++++++++++++++++++++++++++
include/drm/drm_vma_manager.h | 202 +++++++++++++++++++++++++++
4 files changed, 490 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_vma_manager.c
create mode 100644 include/drm/drm_vma_manager.h
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7d1278e..87e22ec 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2212,6 +2212,12 @@ void intel_crt_init(struct drm_device *dev)
!Iinclude/drm/drm_rect.h
!Edrivers/gpu/drm/drm_rect.c
</sect2>
+ <sect2>
+ <title>VMA Offset Manager</title>
+!Pdrivers/gpu/drm/drm_vma_manager.c vma offset manager
+!Edrivers/gpu/drm/drm_vma_manager.c
+!Iinclude/drm/drm_vma_manager.h
+ </sect2>
</sect1>
<!-- Internals: kms properties -->
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 801bcaf..d943b94 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -13,7 +13,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
drm_crtc.o drm_modes.o drm_edid.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
- drm_rect.o
+ drm_rect.o drm_vma_manager.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o
drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
new file mode 100644
index 0000000..b966cea
--- /dev/null
+++ b/drivers/gpu/drm/drm_vma_manager.c
@@ -0,0 +1,281 @@
+/*
+ * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA
+ * Copyright (c) 2012 David Airlie <airlied@linux.ie>
+ * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_mm.h>
+#include <drm/drm_vma_manager.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/**
+ * DOC: vma offset manager
+ *
+ * The vma-manager is responsible to map arbitrary driver-dependent memory
+ * regions into the linear user address-space. It provides offsets to the
+ * caller which can then be used on the address_space of the drm-device. It
+ * takes care to not overlap regions, size them appropriately and to not
+ * confuse mm-core by inconsistent fake vm_pgoff fields.
+ * Drivers shouldn't use this for object placement in VMEM. This manager should
+ * only be used to manage mappings into linear user-space VMs.
+ *
+ * We use drm_mm as backend to manage object allocations. But it is highly
+ * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
+ * speed up offset lookups.
+ *
+ * You must not use multiple offset managers on a single address_space.
+ * Otherwise, mm-core will be unable to tear down memory mappings as the VM will
+ * no longer be linear. Please use VM_NONLINEAR in that case and implement your
+ * own offset managers.
+ *
+ * This offset manager works on page-based addresses. That is, every argument
+ * and return code (with the exception of drm_vma_node_offset_addr()) is given
+ * in number of pages, not number of bytes. That means, object sizes and offsets
+ * must always be page-aligned (as usual).
+ * If you want to get a valid byte-based user-space address for a given offset,
+ * please see drm_vma_node_offset_addr().
+ */
+
+/**
+ * drm_vma_offset_manager_init - Initialize new offset-manager
+ * @mgr: Manager object
+ * @page_offset: Offset of available memory area (page-based)
+ * @size: Size of available address space range (page-based)
+ *
+ * Initialize a new offset-manager. The offset and area size available for the
+ * manager are given as @page_offset and @size. Both are interpreted as
+ * page-numbers, not bytes.
+ *
+ * Adding/removing nodes from the manager is locked internally and protected
+ * against concurrent access. However, node allocation and destruction is left
+ * for the caller. While calling into the vma-manager, a given node must
+ * always be guaranteed to be referenced.
+ */
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+ unsigned long page_offset, unsigned long size)
+{
+ rwlock_init(&mgr->vm_lock);
+ mgr->vm_addr_space_rb = RB_ROOT;
+ drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_init);
+
+/**
+ * drm_vma_offset_manager_destroy() - Destroy offset manager
+ * @mgr: Manager object
+ *
+ * Destroy an object manager which was previously created via
+ * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
+ * before destroying the manager. Otherwise, drm_mm will refuse to free the
+ * requested resources.
+ *
+ * The manager must not be accessed after this function is called.
+ */
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
+{
+ /* take the lock to protect against buggy drivers */
+ write_lock(&mgr->vm_lock);
+ drm_mm_takedown(&mgr->vm_addr_space_mm);
+ write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
+
+/**
+ * drm_vma_offset_lookup() - Find node in offset space
+ * @mgr: Manager object
+ * @start: Start address for object (page-based)
+ * @pages: Size of object (page-based)
+ *
+ * Find a node given a start address and object size. This returns the _best_
+ * match for the given node. That is, @start may point somewhere into a valid
+ * region and the given node will be returned, as long as the node spans the
+ * whole requested area (given the size in number of pages as @pages).
+ *
+ * RETURNS:
+ * Returns NULL if no suitable node can be found. Otherwise, the best match
+ * is returned. It's the caller's responsibility to make sure the node doesn't
+ * get destroyed before the caller can access it.
+ */
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node;
+
+ read_lock(&mgr->vm_lock);
+ node = drm_vma_offset_lookup_locked(mgr, start, pages);
+ read_unlock(&mgr->vm_lock);
+
+ return node;
+}
+EXPORT_SYMBOL(drm_vma_offset_lookup);
+
+/**
+ * drm_vma_offset_lookup_locked() - Find node in offset space
+ * @mgr: Manager object
+ * @start: Start address for object (page-based)
+ * @pages: Size of object (page-based)
+ *
+ * Same as drm_vma_offset_lookup() but requires the caller to lock offset lookup
+ * manually. See drm_vma_offset_lock_lookup() for an example.
+ *
+ * RETURNS:
+ * Returns NULL if no suitable node can be found. Otherwise, the best match
+ * is returned.
+ */
+struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node, *best;
+ struct rb_node *iter;
+ unsigned long offset;
+
+ iter = mgr->vm_addr_space_rb.rb_node;
+ best = NULL;
+
+ while (likely(iter)) {
+ node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
+ offset = node->vm_node.start;
+ if (start >= offset) {
+ iter = iter->rb_right;
+ best = node;
+ if (start == offset)
+ break;
+ } else {
+ iter = iter->rb_left;
+ }
+ }
+
+ /* verify that the node spans the requested area */
+ if (best) {
+ offset = best->vm_node.start + best->vm_node.size;
+ if (offset < start + pages)
+ best = NULL;
+ }
+
+ return best;
+}
+EXPORT_SYMBOL(drm_vma_offset_lookup_locked);
+
+/* internal helper to link @node into the rb-tree */
+static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node)
+{
+ struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
+ struct rb_node *parent = NULL;
+ struct drm_vma_offset_node *iter_node;
+
+ while (likely(*iter)) {
+ parent = *iter;
+ iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
+
+ if (node->vm_node.start < iter_node->vm_node.start)
+ iter = &(*iter)->rb_left;
+ else if (node->vm_node.start > iter_node->vm_node.start)
+ iter = &(*iter)->rb_right;
+ else
+ BUG();
+ }
+
+ rb_link_node(&node->vm_rb, parent, iter);
+ rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
+}
+
+/**
+ * drm_vma_offset_add() - Add offset node to manager
+ * @mgr: Manager object
+ * @node: Node to be added
+ * @pages: Allocation size visible to user-space (in number of pages)
+ *
+ * Add a node to the offset-manager. If the node was already added, this does
+ * nothing and return 0. @pages is the size of the object given in number of
+ * pages.
+ * After this call succeeds, you can access the offset of the node until it
+ * is removed again.
+ *
+ * If this call fails, it is safe to retry the operation or call
+ * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
+ * case.
+ *
+ * @pages is not required to be the same size as the underlying memory object
+ * that you want to map. It only limits the size that user-space can map into
+ * their address space.
+ *
+ * RETURNS:
+ * 0 on success, negative error code on failure.
+ */
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node, unsigned long pages)
+{
+ int ret;
+
+ write_lock(&mgr->vm_lock);
+
+ if (drm_mm_node_allocated(&node->vm_node)) {
+ ret = 0;
+ goto out_unlock;
+ }
+
+ ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
+ &node->vm_node, pages, 0, 0);
+ if (ret)
+ goto out_unlock;
+
+ _drm_vma_offset_add_rb(mgr, node);
+
+out_unlock:
+ write_unlock(&mgr->vm_lock);
+ return ret;
+}
+EXPORT_SYMBOL(drm_vma_offset_add);
+
+/**
+ * drm_vma_offset_remove() - Remove offset node from manager
+ * @mgr: Manager object
+ * @node: Node to be removed
+ *
+ * Remove a node from the offset manager. If the node wasn't added before, this
+ * does nothing. After this call returns, the offset and size will be 0 until a
+ * new offset is allocated via drm_vma_offset_add() again. Helper functions like
+ * drm_vma_node_start() and drm_vma_node_offset_addr() will return 0 if no
+ * offset is allocated.
+ */
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node)
+{
+ write_lock(&mgr->vm_lock);
+
+ if (drm_mm_node_allocated(&node->vm_node)) {
+ rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
+ drm_mm_remove_node(&node->vm_node);
+ memset(&node->vm_node, 0, sizeof(node->vm_node));
+ }
+
+ write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_remove);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
new file mode 100644
index 0000000..7ee8c4b
--- /dev/null
+++ b/include/drm/drm_vma_manager.h
@@ -0,0 +1,202 @@
+#ifndef __DRM_VMA_MANAGER_H__
+#define __DRM_VMA_MANAGER_H__
+
+/*
+ * Copyright (c) 2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drm_mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+struct drm_vma_offset_node {
+ struct drm_mm_node vm_node;
+ struct rb_node vm_rb;
+};
+
+struct drm_vma_offset_manager {
+ rwlock_t vm_lock;
+ struct rb_root vm_addr_space_rb;
+ struct drm_mm vm_addr_space_mm;
+};
+
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+ unsigned long page_offset, unsigned long size);
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
+
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages);
+struct drm_vma_offset_node *drm_vma_offset_lookup_locked(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages);
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node, unsigned long pages);
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+ struct drm_vma_offset_node *node);
+
+/**
+ * drm_vma_offset_exact_lookup() - Look up node by exact address
+ * @mgr: Manager object
+ * @start: Start address (page-based, not byte-based)
+ * @pages: Size of object (page-based)
+ *
+ * Same as drm_vma_offset_lookup() but does not allow any offset into the node.
+ * It only returns the exact object with the given start address.
+ *
+ * RETURNS:
+ * Node at exact start address @start.
+ */
+static inline struct drm_vma_offset_node *
+drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
+ unsigned long start,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node;
+
+ node = drm_vma_offset_lookup(mgr, start, pages);
+ return (node && node->vm_node.start == start) ? node : NULL;
+}
+
+/**
+ * drm_vma_offset_lock_lookup() - Lock lookup for extended private use
+ * @mgr: Manager object
+ *
+ * Lock VMA manager for extended lookups. Only *_locked() VMA function calls
+ * are allowed while holding this lock. All other contexts are blocked from VMA
+ * until the lock is released via drm_vma_offset_unlock_lookup().
+ *
+ * Use this if you need to take a reference to the objects returned by
+ * drm_vma_offset_lookup_locked() before releasing this lock again.
+ *
+ * This lock must not be used for anything else than extended lookups. You must
+ * not call any other VMA helpers while holding this lock.
+ *
+ * Note: You're in atomic-context while holding this lock!
+ *
+ * Example:
+ * drm_vma_offset_lock_lookup(mgr);
+ * node = drm_vma_offset_lookup_locked(mgr);
+ * if (node)
+ * kref_get_unless_zero(container_of(node, sth, entr));
+ * drm_vma_offset_unlock_lookup(mgr);
+ */
+static inline void drm_vma_offset_lock_lookup(struct drm_vma_offset_manager *mgr)
+{
+ read_lock(&mgr->vm_lock);
+}
+
+/**
+ * drm_vma_offset_unlock_lookup() - Unlock lookup for extended private use
+ * @mgr: Manager object
+ *
+ * Release lookup-lock. See drm_vma_offset_lock_lookup() for more information.
+ */
+static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *mgr)
+{
+ read_unlock(&mgr->vm_lock);
+}
+
+/**
+ * drm_vma_node_reset() - Initialize or reset node object
+ * @node: Node to initialize or reset
+ *
+ * Reset a node to its initial state. This must be called if @node isn't
+ * already cleared (eg., via kzalloc) before using it with any VMA offset
+ * manager.
+ *
+ * This must not be called on an already allocated node, or you will leak
+ * memory.
+ */
+static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
+{
+ memset(node, 0, sizeof(*node));
+}
+
+/**
+ * drm_vma_node_start() - Return start address for page-based addressing
+ * @node: Node to inspect
+ *
+ * Return the start address of the given node. This can be used as offset into
+ * the linear VM space that is provided by the VMA offset manager. Note that
+ * this can only be used for page-based addressing. If you need a proper offset
+ * for user-space mappings, you must apply "<< PAGE_SHIFT" or use the
+ * drm_vma_node_offset_addr() helper instead.
+ *
+ * RETURNS:
+ * Start address of @node for page-based addressing. 0 if the node does not
+ * have an offset allocated.
+ */
+static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
+{
+ return node->vm_node.start;
+}
+
+/**
+ * drm_vma_node_size() - Return size (page-based)
+ * @node: Node to inspect
+ *
+ * Return the size as number of pages for the given node. This is the same size
+ * that was passed to drm_vma_offset_add(). If no offset is allocated for the
+ * node, this is 0.
+ *
+ * RETURNS:
+ * Size of @node as number of pages. 0 if the node does not have an offset
+ * allocated.
+ */
+static inline unsigned long drm_vma_node_size(struct drm_vma_offset_node *node)
+{
+ return node->vm_node.size;
+}
+
+/**
+ * drm_vma_node_has_offset() - Check whether node is added to offset manager
+ * @node: Node to be checked
+ *
+ * RETURNS:
+ * true iff the node was previously allocated an offset and added to
+ * an vma offset manager.
+ */
+static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
+{
+ return drm_mm_node_allocated(&node->vm_node);
+}
+
+/**
+ * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps
+ * @node: Linked offset node
+ *
+ * Same as drm_vma_node_start() but returns the address as a valid offset that
+ * can be used for user-space mappings during mmap().
+ * This must not be called on unlinked nodes.
+ *
+ * RETURNS:
+ * Offset of @node for byte-based addressing. 0 if the node does not have an
+ * object allocated.
+ */
+static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
+{
+ return ((__u64)node->vm_node.start) << PAGE_SHIFT;
+}
+
+#endif /* __DRM_VMA_MANAGER_H__ */
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 2/4] drm/gem: convert to new unified vma manager
2013-07-23 12:47 ` [PATCH v4 0/4] Unified VMA Offset Manager David Herrmann
2013-07-23 12:47 ` [PATCH v4 1/4] drm: add unified vma offset manager David Herrmann
@ 2013-07-23 12:47 ` David Herrmann
2013-07-24 15:52 ` Daniel Vetter
2013-07-24 19:07 ` [PATCH v5 " David Herrmann
2013-07-23 12:47 ` [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
2013-07-23 12:47 ` [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
3 siblings, 2 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-23 12:47 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, Dave Airlie
Use the new vma manager instead of the old hashtable. Also convert all
drivers to use the new convenience helpers. This drops all the
(map_list.hash.key << PAGE_SHIFT) non-sense.
Locking and access-management is exactly the same as before with an
additional lock inside of the vma-manager, which strictly wouldn't be
needed for gem.
v2:
- rebase on drm-next
- init nodes via drm_vma_node_reset() in drm_gem.c
v3:
- fix tegra
v4:
- remove duplicate if (drm_vma_node_has_offset()) checks
- inline now trivial drm_vma_node_offset_addr() calls
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
drivers/gpu/drm/drm_gem.c | 89 +++++-------------------------
drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++----
drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++---
drivers/gpu/drm/gma500/gem.c | 15 ++---
drivers/gpu/drm/i915/i915_gem.c | 10 ++--
drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++-----
drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---------------
drivers/gpu/drm/udl/udl_gem.c | 13 ++---
drivers/gpu/host1x/drm/gem.c | 5 +-
include/drm/drmP.h | 7 +--
include/uapi/drm/drm.h | 2 +-
11 files changed, 62 insertions(+), 186 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1ad9e7e..8d3cdf3 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -37,6 +37,7 @@
#include <linux/shmem_fs.h>
#include <linux/dma-buf.h>
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
/** @file drm_gem.c
*
@@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
}
dev->mm_private = mm;
-
- if (drm_ht_create(&mm->offset_hash, 12)) {
- kfree(mm);
- return -ENOMEM;
- }
-
- drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,
- DRM_FILE_PAGE_OFFSET_SIZE);
+ drm_vma_offset_manager_init(&mm->vma_manager,
+ DRM_FILE_PAGE_OFFSET_START,
+ DRM_FILE_PAGE_OFFSET_SIZE);
return 0;
}
@@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
{
struct drm_gem_mm *mm = dev->mm_private;
- drm_mm_takedown(&mm->offset_manager);
- drm_ht_remove(&mm->offset_hash);
+ drm_vma_offset_manager_destroy(&mm->vma_manager);
kfree(mm);
dev->mm_private = NULL;
}
@@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
kref_init(&obj->refcount);
atomic_set(&obj->handle_count, 0);
+ drm_vma_node_reset(&obj->vma_node);
obj->size = size;
}
EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list = &obj->map_list;
- drm_ht_remove_item(&mm->offset_hash, &list->hash);
- drm_mm_put_block(list->file_offset_node);
- kfree(list->map);
- list->map = NULL;
+ drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);
}
EXPORT_SYMBOL(drm_gem_free_mmap_offset);
@@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list;
- struct drm_local_map *map;
- int ret;
-
- /* Set the object up for mmap'ing */
- list = &obj->map_list;
- list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
- if (!list->map)
- return -ENOMEM;
-
- map = list->map;
- map->type = _DRM_GEM;
- map->size = obj->size;
- map->handle = obj;
-
- /* Get a DRM GEM mmap offset allocated... */
- list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
- obj->size / PAGE_SIZE, 0, false);
-
- if (!list->file_offset_node) {
- DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
- ret = -ENOSPC;
- goto out_free_list;
- }
- list->file_offset_node = drm_mm_get_block(list->file_offset_node,
- obj->size / PAGE_SIZE, 0);
- if (!list->file_offset_node) {
- ret = -ENOMEM;
- goto out_free_list;
- }
-
- list->hash.key = list->file_offset_node->start;
- ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
- if (ret) {
- DRM_ERROR("failed to add to map hash\n");
- goto out_free_mm;
- }
-
- return 0;
-
-out_free_mm:
- drm_mm_put_block(list->file_offset_node);
-out_free_list:
- kfree(list->map);
- list->map = NULL;
-
- return ret;
+ return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
+ obj->size / PAGE_SIZE);
}
EXPORT_SYMBOL(drm_gem_create_mmap_offset);
@@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
struct drm_file *priv = filp->private_data;
struct drm_device *dev = priv->minor->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_local_map *map = NULL;
- struct drm_hash_item *hash;
+ struct drm_gem_object *obj;
+ struct drm_vma_offset_node *node;
int ret = 0;
if (drm_device_is_unplugged(dev))
@@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
mutex_lock(&dev->struct_mutex);
- if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
+ node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff);
+ if (!node) {
mutex_unlock(&dev->struct_mutex);
return drm_mmap(filp, vma);
}
- map = drm_hash_entry(hash, struct drm_map_list, hash)->map;
- if (!map ||
- ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- ret = drm_gem_mmap_obj(map->handle, map->size, vma);
+ obj = container_of(node, struct drm_gem_object, vma_node);
+ ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);
-out_unlock:
mutex_unlock(&dev->struct_mutex);
return ret;
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index ece72a8..bf09a6dc 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -27,11 +27,7 @@
#include <drm/drmP.h>
#include <drm/drm.h>
#include <drm/drm_gem_cma_helper.h>
-
-static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj)
-{
- return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
-}
+#include <drm/drm_vma_manager.h>
/*
* __drm_gem_cma_create - Create a GEM CMA object without allocating memory
@@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
{
struct drm_gem_cma_object *cma_obj;
- if (gem_obj->map_list.map)
- drm_gem_free_mmap_offset(gem_obj);
+ drm_gem_free_mmap_offset(gem_obj);
cma_obj = to_drm_gem_cma_obj(gem_obj);
@@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
return -EINVAL;
}
- *offset = get_gem_mmap_offset(gem_obj);
+ *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node);
drm_gem_object_unreference(gem_obj);
@@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
{
struct drm_gem_object *obj = &cma_obj->base;
struct drm_device *dev = obj->dev;
- uint64_t off = 0;
+ uint64_t off;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map)
- off = (uint64_t)obj->map_list.hash.key;
+ off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
obj->name, obj->refcount.refcount.counter,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 24c22a8..be32db1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -10,6 +10,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
#include <linux/shmem_fs.h>
#include <drm/exynos_drm.h>
@@ -152,8 +153,7 @@ out:
exynos_drm_fini_buf(obj->dev, buf);
exynos_gem_obj->buffer = NULL;
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
/* release file pointer to gem object. */
drm_gem_object_release(obj);
@@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
goto unlock;
}
- if (!obj->map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
- *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&obj->vma_node);
DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
out:
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index fe1d332..2f77bea 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -26,6 +26,7 @@
#include <drm/drmP.h>
#include <drm/drm.h>
#include <drm/gma_drm.h>
+#include <drm/drm_vma_manager.h>
#include "psb_drv.h"
int psb_gem_init_object(struct drm_gem_object *obj)
@@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj)
struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
/* Remove the list map if one is present */
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
drm_gem_object_release(obj);
/* This must occur last as it frees up the memory of the GEM object */
@@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev,
/* What validation is needed here ? */
/* Make it mmapable */
- if (!obj->map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
- /* GEM should really work out the hash offsets for us */
- *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
+ *offset = drm_vma_node_offset_addr(&obj->vma_node);
out:
drm_gem_object_unreference(obj);
unlock:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 46bf7e3..53f81b3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -26,6 +26,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
#include <drm/i915_drm.h>
#include "i915_drv.h"
#include "i915_trace.h"
@@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (obj->base.dev->dev_mapping)
unmap_mapping_range(obj->base.dev->dev_mapping,
- (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
+ (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
obj->base.size, 1);
obj->fault_mappable = false;
@@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
int ret;
- if (obj->base.map_list.map)
+ if (drm_vma_node_has_offset(&obj->base.vma_node))
return 0;
dev_priv->mm.shrinker_no_lock_stealing = true;
@@ -1517,9 +1518,6 @@ out:
static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
{
- if (!obj->base.map_list.map)
- return;
-
drm_gem_free_mmap_offset(&obj->base);
}
@@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
if (ret)
goto out;
- *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
out:
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index cbcd71e..f90531fc00 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <linux/shmem_fs.h>
+#include <drm/drm_vma_manager.h>
#include "omap_drv.h"
#include "omap_dmm_tiler.h"
@@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj)
static uint64_t mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
+ int ret;
+ size_t size;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (!obj->map_list.map) {
- /* Make it mmapable */
- size_t size = omap_gem_mmap_size(obj);
- int ret = _drm_gem_create_mmap_offset_size(obj, size);
-
- if (ret) {
- dev_err(dev->dev, "could not allocate mmap offset\n");
- return 0;
- }
+ /* Make it mmapable */
+ size = omap_gem_mmap_size(obj);
+ ret = _drm_gem_create_mmap_offset_size(obj, size);
+ if (ret) {
+ dev_err(dev->dev, "could not allocate mmap offset\n");
+ return 0;
}
- return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
+ return drm_vma_node_offset_addr(&obj->vma_node);
}
uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj)
@@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
{
struct drm_device *dev = obj->dev;
struct omap_gem_object *omap_obj = to_omap_bo(obj);
- uint64_t off = 0;
+ uint64_t off;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map)
- off = (uint64_t)obj->map_list.hash.key;
+ off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d",
omap_obj->flags, obj->name, obj->refcount.refcount.counter,
@@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
list_del(&omap_obj->mm_list);
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
/* this means the object is still pinned.. which really should
* not happen. I think..
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
index f9eb679..dbb1575 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
@@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list;
- struct drm_local_map *map;
- int ret = 0;
-
- /* Set the object up for mmap'ing */
- list = &obj->map_list;
- list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
- if (!list->map)
- return -ENOMEM;
-
- map = list->map;
- map->type = _DRM_GEM;
- map->size = size;
- map->handle = obj;
-
- /* Get a DRM GEM mmap offset allocated... */
- list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
- size / PAGE_SIZE, 0, 0);
-
- if (!list->file_offset_node) {
- DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
- ret = -ENOSPC;
- goto out_free_list;
- }
-
- list->file_offset_node = drm_mm_get_block(list->file_offset_node,
- size / PAGE_SIZE, 0);
- if (!list->file_offset_node) {
- ret = -ENOMEM;
- goto out_free_list;
- }
-
- list->hash.key = list->file_offset_node->start;
- ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
- if (ret) {
- DRM_ERROR("failed to add to map hash\n");
- goto out_free_mm;
- }
-
- return 0;
-
-out_free_mm:
- drm_mm_put_block(list->file_offset_node);
-out_free_list:
- kfree(list->map);
- list->map = NULL;
- return ret;
+ return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
+ size / PAGE_SIZE);
}
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index ef034fa..2a4cb2f 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
if (obj->pages)
udl_gem_put_pages(obj);
- if (gem_obj->map_list.map)
- drm_gem_free_mmap_offset(gem_obj);
+ drm_gem_free_mmap_offset(gem_obj);
}
/* the dumb interface doesn't work with the GEM straight MMAP
@@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
ret = udl_gem_get_pages(gobj, GFP_KERNEL);
if (ret)
goto out;
- if (!gobj->base.map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
- *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
out:
drm_gem_object_unreference(&gobj->base);
diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c
index c5e9a9b..bc323b3 100644
--- a/drivers/gpu/host1x/drm/gem.c
+++ b/drivers/gpu/host1x/drm/gem.c
@@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo)
{
- return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT;
+ return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node);
}
struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size)
@@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
{
struct tegra_bo *bo = to_tegra_bo(gem);
- if (gem->map_list.map)
- drm_gem_free_mmap_offset(gem);
+ drm_gem_free_mmap_offset(gem);
drm_gem_object_release(gem);
tegra_bo_destroy(gem->dev, bo);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0ab6a09..4b518e0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -71,6 +71,7 @@
#include <asm/pgalloc.h>
#include <drm/drm.h>
#include <drm/drm_sarea.h>
+#include <drm/drm_vma_manager.h>
#include <linux/idr.h>
@@ -587,7 +588,6 @@ struct drm_map_list {
struct drm_local_map *map; /**< mapping */
uint64_t user_token;
struct drm_master *master;
- struct drm_mm_node *file_offset_node; /**< fake offset */
};
/**
@@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
* GEM specific mm private for tracking GEM objects
*/
struct drm_gem_mm {
- struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */
- struct drm_open_hash offset_hash; /**< User token hash table for maps */
+ struct drm_vma_offset_manager vma_manager;
};
/**
@@ -644,7 +643,7 @@ struct drm_gem_object {
struct file *filp;
/* Mapping info for this object */
- struct drm_map_list map_list;
+ struct drm_vma_offset_node vma_node;
/**
* Size of the object, in bytes. Immutable over the object's
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 238a166..272580c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -181,7 +181,7 @@ enum drm_map_type {
_DRM_AGP = 3, /**< AGP/GART */
_DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */
_DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
- _DRM_GEM = 6, /**< GEM object */
+ _DRM_GEM = 6, /**< GEM object (obsolete) */
};
/**
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v4 2/4] drm/gem: convert to new unified vma manager
2013-07-23 12:47 ` [PATCH v4 2/4] drm/gem: convert to new unified vma manager David Herrmann
@ 2013-07-24 15:52 ` Daniel Vetter
2013-07-24 16:27 ` David Herrmann
2013-07-24 19:07 ` [PATCH v5 " David Herrmann
1 sibling, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2013-07-24 15:52 UTC (permalink / raw)
To: David Herrmann; +Cc: Daniel Vetter, dri-devel, Dave Airlie
On Tue, Jul 23, 2013 at 02:47:14PM +0200, David Herrmann wrote:
> Use the new vma manager instead of the old hashtable. Also convert all
> drivers to use the new convenience helpers. This drops all the
> (map_list.hash.key << PAGE_SHIFT) non-sense.
>
> Locking and access-management is exactly the same as before with an
> additional lock inside of the vma-manager, which strictly wouldn't be
> needed for gem.
>
> v2:
> - rebase on drm-next
> - init nodes via drm_vma_node_reset() in drm_gem.c
> v3:
> - fix tegra
> v4:
> - remove duplicate if (drm_vma_node_has_offset()) checks
> - inline now trivial drm_vma_node_offset_addr() calls
>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
> drivers/gpu/drm/drm_gem.c | 89 +++++-------------------------
> drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++----
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++---
> drivers/gpu/drm/gma500/gem.c | 15 ++---
> drivers/gpu/drm/i915/i915_gem.c | 10 ++--
> drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++-----
> drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---------------
> drivers/gpu/drm/udl/udl_gem.c | 13 ++---
> drivers/gpu/host1x/drm/gem.c | 5 +-
> include/drm/drmP.h | 7 +--
> include/uapi/drm/drm.h | 2 +-
> 11 files changed, 62 insertions(+), 186 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 1ad9e7e..8d3cdf3 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -37,6 +37,7 @@
> #include <linux/shmem_fs.h>
> #include <linux/dma-buf.h>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
>
> /** @file drm_gem.c
> *
> @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
> }
>
> dev->mm_private = mm;
> -
> - if (drm_ht_create(&mm->offset_hash, 12)) {
> - kfree(mm);
> - return -ENOMEM;
> - }
> -
> - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,
> - DRM_FILE_PAGE_OFFSET_SIZE);
> + drm_vma_offset_manager_init(&mm->vma_manager,
> + DRM_FILE_PAGE_OFFSET_START,
> + DRM_FILE_PAGE_OFFSET_SIZE);
>
> return 0;
> }
> @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
> {
> struct drm_gem_mm *mm = dev->mm_private;
>
> - drm_mm_takedown(&mm->offset_manager);
> - drm_ht_remove(&mm->offset_hash);
> + drm_vma_offset_manager_destroy(&mm->vma_manager);
> kfree(mm);
> dev->mm_private = NULL;
> }
> @@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
>
> kref_init(&obj->refcount);
> atomic_set(&obj->handle_count, 0);
> + drm_vma_node_reset(&obj->vma_node);
We already presume (but it's not really documented) that a gem object is
cleared already (or allocated with kzalloc) so feels a bit like overhead.
> obj->size = size;
> }
> EXPORT_SYMBOL(drm_gem_private_object_init);
> @@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list = &obj->map_list;
>
> - drm_ht_remove_item(&mm->offset_hash, &list->hash);
> - drm_mm_put_block(list->file_offset_node);
> - kfree(list->map);
> - list->map = NULL;
> + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);
> }
> EXPORT_SYMBOL(drm_gem_free_mmap_offset);
>
> @@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list;
> - struct drm_local_map *map;
> - int ret;
> -
> - /* Set the object up for mmap'ing */
> - list = &obj->map_list;
> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> - if (!list->map)
> - return -ENOMEM;
> -
> - map = list->map;
> - map->type = _DRM_GEM;
> - map->size = obj->size;
> - map->handle = obj;
> -
> - /* Get a DRM GEM mmap offset allocated... */
> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
> - obj->size / PAGE_SIZE, 0, false);
> -
> - if (!list->file_offset_node) {
> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
> - ret = -ENOSPC;
> - goto out_free_list;
> - }
>
> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
> - obj->size / PAGE_SIZE, 0);
> - if (!list->file_offset_node) {
> - ret = -ENOMEM;
> - goto out_free_list;
> - }
> -
> - list->hash.key = list->file_offset_node->start;
> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
> - if (ret) {
> - DRM_ERROR("failed to add to map hash\n");
> - goto out_free_mm;
> - }
> -
> - return 0;
> -
> -out_free_mm:
> - drm_mm_put_block(list->file_offset_node);
> -out_free_list:
> - kfree(list->map);
> - list->map = NULL;
> -
> - return ret;
> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
> + obj->size / PAGE_SIZE);
> }
> EXPORT_SYMBOL(drm_gem_create_mmap_offset);
>
> @@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> struct drm_file *priv = filp->private_data;
> struct drm_device *dev = priv->minor->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_local_map *map = NULL;
> - struct drm_hash_item *hash;
> + struct drm_gem_object *obj;
> + struct drm_vma_offset_node *node;
> int ret = 0;
>
> if (drm_device_is_unplugged(dev))
> @@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>
> mutex_lock(&dev->struct_mutex);
>
> - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
> + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff);
> + if (!node) {
> mutex_unlock(&dev->struct_mutex);
> return drm_mmap(filp, vma);
> }
>
> - map = drm_hash_entry(hash, struct drm_map_list, hash)->map;
> - if (!map ||
> - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
> - ret = -EPERM;
> - goto out_unlock;
> - }
> -
> - ret = drm_gem_mmap_obj(map->handle, map->size, vma);
> + obj = container_of(node, struct drm_gem_object, vma_node);
> + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);
Iirc the old code only resulted in a ht hit if the start of the mmap
regioni exactly matched the offset. The new code (I think at least, maybe
I've misread it a bit) will return an object even if the start isn't
aligned. drm_gem_mmap_obj checks already that the object is big enough but
we don't check the start any more. I think this should be readded.
It's a bit funny that gem wants an exact match but is ok with just mapping
the beginning of the object, but I guess that abi is now enshrined ;-)
>
> -out_unlock:
> mutex_unlock(&dev->struct_mutex);
>
> return ret;
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index ece72a8..bf09a6dc 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -27,11 +27,7 @@
> #include <drm/drmP.h>
> #include <drm/drm.h>
> #include <drm/drm_gem_cma_helper.h>
> -
> -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj)
> -{
> - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
> -}
> +#include <drm/drm_vma_manager.h>
>
> /*
> * __drm_gem_cma_create - Create a GEM CMA object without allocating memory
> @@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
> {
> struct drm_gem_cma_object *cma_obj;
>
> - if (gem_obj->map_list.map)
> - drm_gem_free_mmap_offset(gem_obj);
> + drm_gem_free_mmap_offset(gem_obj);
>
> cma_obj = to_drm_gem_cma_obj(gem_obj);
>
> @@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
> return -EINVAL;
> }
>
> - *offset = get_gem_mmap_offset(gem_obj);
> + *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node);
*offset is an u64, same as drm_vma_node_offset_addr. Why do we need a cast
here?
>
> drm_gem_object_unreference(gem_obj);
>
> @@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
> {
> struct drm_gem_object *obj = &cma_obj->base;
> struct drm_device *dev = obj->dev;
> - uint64_t off = 0;
> + uint64_t off;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->map_list.map)
> - off = (uint64_t)obj->map_list.hash.key;
> + off = drm_vma_node_start(&obj->vma_node);
>
> seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
> obj->name, obj->refcount.refcount.counter,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 24c22a8..be32db1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -10,6 +10,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
>
> #include <linux/shmem_fs.h>
> #include <drm/exynos_drm.h>
> @@ -152,8 +153,7 @@ out:
> exynos_drm_fini_buf(obj->dev, buf);
> exynos_gem_obj->buffer = NULL;
>
> - if (obj->map_list.map)
> - drm_gem_free_mmap_offset(obj);
> + drm_gem_free_mmap_offset(obj);
>
> /* release file pointer to gem object. */
> drm_gem_object_release(obj);
> @@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
> goto unlock;
> }
>
> - if (!obj->map_list.map) {
> - ret = drm_gem_create_mmap_offset(obj);
> - if (ret)
> - goto out;
> - }
> + ret = drm_gem_create_mmap_offset(obj);
> + if (ret)
> + goto out;
>
> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>
> out:
> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
> index fe1d332..2f77bea 100644
> --- a/drivers/gpu/drm/gma500/gem.c
> +++ b/drivers/gpu/drm/gma500/gem.c
> @@ -26,6 +26,7 @@
> #include <drm/drmP.h>
> #include <drm/drm.h>
> #include <drm/gma_drm.h>
> +#include <drm/drm_vma_manager.h>
> #include "psb_drv.h"
>
> int psb_gem_init_object(struct drm_gem_object *obj)
> @@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj)
> struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
>
> /* Remove the list map if one is present */
> - if (obj->map_list.map)
> - drm_gem_free_mmap_offset(obj);
> + drm_gem_free_mmap_offset(obj);
> drm_gem_object_release(obj);
>
> /* This must occur last as it frees up the memory of the GEM object */
> @@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev,
> /* What validation is needed here ? */
>
> /* Make it mmapable */
> - if (!obj->map_list.map) {
> - ret = drm_gem_create_mmap_offset(obj);
> - if (ret)
> - goto out;
> - }
> - /* GEM should really work out the hash offsets for us */
> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
> + ret = drm_gem_create_mmap_offset(obj);
> + if (ret)
> + goto out;
> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
> out:
> drm_gem_object_unreference(obj);
> unlock:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 46bf7e3..53f81b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -26,6 +26,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
> #include "i915_trace.h"
> @@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
>
> if (obj->base.dev->dev_mapping)
> unmap_mapping_range(obj->base.dev->dev_mapping,
> - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
> + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
> obj->base.size, 1);
>
> obj->fault_mappable = false;
> @@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
> struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> int ret;
>
> - if (obj->base.map_list.map)
> + if (drm_vma_node_has_offset(&obj->base.vma_node))
> return 0;
>
> dev_priv->mm.shrinker_no_lock_stealing = true;
> @@ -1517,9 +1518,6 @@ out:
>
> static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
> {
> - if (!obj->base.map_list.map)
> - return;
> -
> drm_gem_free_mmap_offset(&obj->base);
> }
>
> @@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
> if (ret)
> goto out;
>
> - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
>
> out:
> drm_gem_object_unreference(&obj->base);
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index cbcd71e..f90531fc00 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -20,6 +20,7 @@
>
> #include <linux/spinlock.h>
> #include <linux/shmem_fs.h>
> +#include <drm/drm_vma_manager.h>
>
> #include "omap_drv.h"
> #include "omap_dmm_tiler.h"
> @@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj)
> static uint64_t mmap_offset(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> + int ret;
> + size_t size;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (!obj->map_list.map) {
> - /* Make it mmapable */
> - size_t size = omap_gem_mmap_size(obj);
> - int ret = _drm_gem_create_mmap_offset_size(obj, size);
> -
> - if (ret) {
> - dev_err(dev->dev, "could not allocate mmap offset\n");
> - return 0;
> - }
> + /* Make it mmapable */
> + size = omap_gem_mmap_size(obj);
> + ret = _drm_gem_create_mmap_offset_size(obj, size);
> + if (ret) {
> + dev_err(dev->dev, "could not allocate mmap offset\n");
> + return 0;
> }
>
> - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
> + return drm_vma_node_offset_addr(&obj->vma_node);
> }
>
> uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj)
> @@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
> {
> struct drm_device *dev = obj->dev;
> struct omap_gem_object *omap_obj = to_omap_bo(obj);
> - uint64_t off = 0;
> + uint64_t off;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->map_list.map)
> - off = (uint64_t)obj->map_list.hash.key;
> + off = drm_vma_node_start(&obj->vma_node);
>
> seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d",
> omap_obj->flags, obj->name, obj->refcount.refcount.counter,
> @@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
>
> list_del(&omap_obj->mm_list);
>
> - if (obj->map_list.map)
> - drm_gem_free_mmap_offset(obj);
> + drm_gem_free_mmap_offset(obj);
>
> /* this means the object is still pinned.. which really should
> * not happen. I think..
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> index f9eb679..dbb1575 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list;
> - struct drm_local_map *map;
> - int ret = 0;
> -
> - /* Set the object up for mmap'ing */
> - list = &obj->map_list;
> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> - if (!list->map)
> - return -ENOMEM;
> -
> - map = list->map;
> - map->type = _DRM_GEM;
> - map->size = size;
> - map->handle = obj;
> -
> - /* Get a DRM GEM mmap offset allocated... */
> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
> - size / PAGE_SIZE, 0, 0);
> -
> - if (!list->file_offset_node) {
> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
> - ret = -ENOSPC;
> - goto out_free_list;
> - }
> -
> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
> - size / PAGE_SIZE, 0);
> - if (!list->file_offset_node) {
> - ret = -ENOMEM;
> - goto out_free_list;
> - }
> -
> - list->hash.key = list->file_offset_node->start;
> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
> - if (ret) {
> - DRM_ERROR("failed to add to map hash\n");
> - goto out_free_mm;
> - }
> -
> - return 0;
> -
> -out_free_mm:
> - drm_mm_put_block(list->file_offset_node);
> -out_free_list:
> - kfree(list->map);
> - list->map = NULL;
>
> - return ret;
> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
> + size / PAGE_SIZE);
> }
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index ef034fa..2a4cb2f 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
> if (obj->pages)
> udl_gem_put_pages(obj);
>
> - if (gem_obj->map_list.map)
> - drm_gem_free_mmap_offset(gem_obj);
> + drm_gem_free_mmap_offset(gem_obj);
> }
>
> /* the dumb interface doesn't work with the GEM straight MMAP
> @@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
> ret = udl_gem_get_pages(gobj, GFP_KERNEL);
> if (ret)
> goto out;
> - if (!gobj->base.map_list.map) {
> - ret = drm_gem_create_mmap_offset(obj);
> - if (ret)
> - goto out;
> - }
> + ret = drm_gem_create_mmap_offset(obj);
> + if (ret)
> + goto out;
>
> - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
>
> out:
> drm_gem_object_unreference(&gobj->base);
> diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c
> index c5e9a9b..bc323b3 100644
> --- a/drivers/gpu/host1x/drm/gem.c
> +++ b/drivers/gpu/host1x/drm/gem.c
> @@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
>
> unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo)
> {
> - return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT;
> + return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node);
Ok, tegra is pretty funny here about mmap offsets, using an int.
Whatever, pre-existing fanciness so not our problem.
> }
>
> struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size)
> @@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
> {
> struct tegra_bo *bo = to_tegra_bo(gem);
>
> - if (gem->map_list.map)
> - drm_gem_free_mmap_offset(gem);
> + drm_gem_free_mmap_offset(gem);
>
> drm_gem_object_release(gem);
> tegra_bo_destroy(gem->dev, bo);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0ab6a09..4b518e0 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -71,6 +71,7 @@
> #include <asm/pgalloc.h>
> #include <drm/drm.h>
> #include <drm/drm_sarea.h>
> +#include <drm/drm_vma_manager.h>
>
> #include <linux/idr.h>
>
> @@ -587,7 +588,6 @@ struct drm_map_list {
> struct drm_local_map *map; /**< mapping */
> uint64_t user_token;
> struct drm_master *master;
> - struct drm_mm_node *file_offset_node; /**< fake offset */
> };
>
> /**
> @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
> * GEM specific mm private for tracking GEM objects
> */
> struct drm_gem_mm {
> - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */
> - struct drm_open_hash offset_hash; /**< User token hash table for maps */
> + struct drm_vma_offset_manager vma_manager;
> };
>
> /**
> @@ -644,7 +643,7 @@ struct drm_gem_object {
> struct file *filp;
>
> /* Mapping info for this object */
> - struct drm_map_list map_list;
> + struct drm_vma_offset_node vma_node;
>
> /**
> * Size of the object, in bytes. Immutable over the object's
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 238a166..272580c 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -181,7 +181,7 @@ enum drm_map_type {
> _DRM_AGP = 3, /**< AGP/GART */
> _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */
> _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
> - _DRM_GEM = 6, /**< GEM object */
> + _DRM_GEM = 6, /**< GEM object (obsolete) */
> };
>
> /**
> --
> 1.8.3.3
A few small things, with those addressed this is:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I guess with the common infrastructure we'll have a bit of room for
follow-up cleanups, but that can be done on especially rainy autumn days
;-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v4 2/4] drm/gem: convert to new unified vma manager
2013-07-24 15:52 ` Daniel Vetter
@ 2013-07-24 16:27 ` David Herrmann
0 siblings, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 16:27 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel, Dave Airlie
Hi
On Wed, Jul 24, 2013 at 5:52 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jul 23, 2013 at 02:47:14PM +0200, David Herrmann wrote:
>> Use the new vma manager instead of the old hashtable. Also convert all
>> drivers to use the new convenience helpers. This drops all the
>> (map_list.hash.key << PAGE_SHIFT) non-sense.
>>
>> Locking and access-management is exactly the same as before with an
>> additional lock inside of the vma-manager, which strictly wouldn't be
>> needed for gem.
>>
>> v2:
>> - rebase on drm-next
>> - init nodes via drm_vma_node_reset() in drm_gem.c
>> v3:
>> - fix tegra
>> v4:
>> - remove duplicate if (drm_vma_node_has_offset()) checks
>> - inline now trivial drm_vma_node_offset_addr() calls
>>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> ---
>> drivers/gpu/drm/drm_gem.c | 89 +++++-------------------------
>> drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++----
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++---
>> drivers/gpu/drm/gma500/gem.c | 15 ++---
>> drivers/gpu/drm/i915/i915_gem.c | 10 ++--
>> drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++-----
>> drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---------------
>> drivers/gpu/drm/udl/udl_gem.c | 13 ++---
>> drivers/gpu/host1x/drm/gem.c | 5 +-
>> include/drm/drmP.h | 7 +--
>> include/uapi/drm/drm.h | 2 +-
>> 11 files changed, 62 insertions(+), 186 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 1ad9e7e..8d3cdf3 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -37,6 +37,7 @@
>> #include <linux/shmem_fs.h>
>> #include <linux/dma-buf.h>
>> #include <drm/drmP.h>
>> +#include <drm/drm_vma_manager.h>
>>
>> /** @file drm_gem.c
>> *
>> @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
>> }
>>
>> dev->mm_private = mm;
>> -
>> - if (drm_ht_create(&mm->offset_hash, 12)) {
>> - kfree(mm);
>> - return -ENOMEM;
>> - }
>> -
>> - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,
>> - DRM_FILE_PAGE_OFFSET_SIZE);
>> + drm_vma_offset_manager_init(&mm->vma_manager,
>> + DRM_FILE_PAGE_OFFSET_START,
>> + DRM_FILE_PAGE_OFFSET_SIZE);
>>
>> return 0;
>> }
>> @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
>> {
>> struct drm_gem_mm *mm = dev->mm_private;
>>
>> - drm_mm_takedown(&mm->offset_manager);
>> - drm_ht_remove(&mm->offset_hash);
>> + drm_vma_offset_manager_destroy(&mm->vma_manager);
>> kfree(mm);
>> dev->mm_private = NULL;
>> }
>> @@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
>>
>> kref_init(&obj->refcount);
>> atomic_set(&obj->handle_count, 0);
>> + drm_vma_node_reset(&obj->vma_node);
>
> We already presume (but it's not really documented) that a gem object is
> cleared already (or allocated with kzalloc) so feels a bit like overhead.
I am fine with dropping it, if that's what gem already requires from drivers.
>> obj->size = size;
>> }
>> EXPORT_SYMBOL(drm_gem_private_object_init);
>> @@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
>> {
>> struct drm_device *dev = obj->dev;
>> struct drm_gem_mm *mm = dev->mm_private;
>> - struct drm_map_list *list = &obj->map_list;
>>
>> - drm_ht_remove_item(&mm->offset_hash, &list->hash);
>> - drm_mm_put_block(list->file_offset_node);
>> - kfree(list->map);
>> - list->map = NULL;
>> + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);
>> }
>> EXPORT_SYMBOL(drm_gem_free_mmap_offset);
>>
>> @@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
>> {
>> struct drm_device *dev = obj->dev;
>> struct drm_gem_mm *mm = dev->mm_private;
>> - struct drm_map_list *list;
>> - struct drm_local_map *map;
>> - int ret;
>> -
>> - /* Set the object up for mmap'ing */
>> - list = &obj->map_list;
>> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
>> - if (!list->map)
>> - return -ENOMEM;
>> -
>> - map = list->map;
>> - map->type = _DRM_GEM;
>> - map->size = obj->size;
>> - map->handle = obj;
>> -
>> - /* Get a DRM GEM mmap offset allocated... */
>> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
>> - obj->size / PAGE_SIZE, 0, false);
>> -
>> - if (!list->file_offset_node) {
>> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
>> - ret = -ENOSPC;
>> - goto out_free_list;
>> - }
>>
>> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
>> - obj->size / PAGE_SIZE, 0);
>> - if (!list->file_offset_node) {
>> - ret = -ENOMEM;
>> - goto out_free_list;
>> - }
>> -
>> - list->hash.key = list->file_offset_node->start;
>> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
>> - if (ret) {
>> - DRM_ERROR("failed to add to map hash\n");
>> - goto out_free_mm;
>> - }
>> -
>> - return 0;
>> -
>> -out_free_mm:
>> - drm_mm_put_block(list->file_offset_node);
>> -out_free_list:
>> - kfree(list->map);
>> - list->map = NULL;
>> -
>> - return ret;
>> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
>> + obj->size / PAGE_SIZE);
>> }
>> EXPORT_SYMBOL(drm_gem_create_mmap_offset);
>>
>> @@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> struct drm_file *priv = filp->private_data;
>> struct drm_device *dev = priv->minor->dev;
>> struct drm_gem_mm *mm = dev->mm_private;
>> - struct drm_local_map *map = NULL;
>> - struct drm_hash_item *hash;
>> + struct drm_gem_object *obj;
>> + struct drm_vma_offset_node *node;
>> int ret = 0;
>>
>> if (drm_device_is_unplugged(dev))
>> @@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>
>> mutex_lock(&dev->struct_mutex);
>>
>> - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
>> + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff);
>> + if (!node) {
>> mutex_unlock(&dev->struct_mutex);
>> return drm_mmap(filp, vma);
>> }
>>
>> - map = drm_hash_entry(hash, struct drm_map_list, hash)->map;
>> - if (!map ||
>> - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
>> - ret = -EPERM;
>> - goto out_unlock;
>> - }
>> -
>> - ret = drm_gem_mmap_obj(map->handle, map->size, vma);
>> + obj = container_of(node, struct drm_gem_object, vma_node);
>> + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);
>
> Iirc the old code only resulted in a ht hit if the start of the mmap
> regioni exactly matched the offset. The new code (I think at least, maybe
> I've misread it a bit) will return an object even if the start isn't
> aligned. drm_gem_mmap_obj checks already that the object is big enough but
> we don't check the start any more. I think this should be readded.
>
> It's a bit funny that gem wants an exact match but is ok with just mapping
> the beginning of the object, but I guess that abi is now enshrined ;-)
drm_vma_offset_exact_lookup() was meant for exactly that but it fails
to actually verify it. I can add a "drm_vma_node_start() == start"
check and return NULL if it doesn't match. The name is currently a bit
misleading.
I will fix that.
>>
>> -out_unlock:
>> mutex_unlock(&dev->struct_mutex);
>>
>> return ret;
>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
>> index ece72a8..bf09a6dc 100644
>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>> @@ -27,11 +27,7 @@
>> #include <drm/drmP.h>
>> #include <drm/drm.h>
>> #include <drm/drm_gem_cma_helper.h>
>> -
>> -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj)
>> -{
>> - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
>> -}
>> +#include <drm/drm_vma_manager.h>
>>
>> /*
>> * __drm_gem_cma_create - Create a GEM CMA object without allocating memory
>> @@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
>> {
>> struct drm_gem_cma_object *cma_obj;
>>
>> - if (gem_obj->map_list.map)
>> - drm_gem_free_mmap_offset(gem_obj);
>> + drm_gem_free_mmap_offset(gem_obj);
>>
>> cma_obj = to_drm_gem_cma_obj(gem_obj);
>>
>> @@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
>> return -EINVAL;
>> }
>>
>> - *offset = get_gem_mmap_offset(gem_obj);
>> + *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node);
>
> *offset is an u64, same as drm_vma_node_offset_addr. Why do we need a cast
> here?
Copied from the helper above. I can fix it.
>>
>> drm_gem_object_unreference(gem_obj);
>>
>> @@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
>> {
>> struct drm_gem_object *obj = &cma_obj->base;
>> struct drm_device *dev = obj->dev;
>> - uint64_t off = 0;
>> + uint64_t off;
>>
>> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>>
>> - if (obj->map_list.map)
>> - off = (uint64_t)obj->map_list.hash.key;
>> + off = drm_vma_node_start(&obj->vma_node);
>>
>> seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
>> obj->name, obj->refcount.refcount.counter,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 24c22a8..be32db1 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -10,6 +10,7 @@
>> */
>>
>> #include <drm/drmP.h>
>> +#include <drm/drm_vma_manager.h>
>>
>> #include <linux/shmem_fs.h>
>> #include <drm/exynos_drm.h>
>> @@ -152,8 +153,7 @@ out:
>> exynos_drm_fini_buf(obj->dev, buf);
>> exynos_gem_obj->buffer = NULL;
>>
>> - if (obj->map_list.map)
>> - drm_gem_free_mmap_offset(obj);
>> + drm_gem_free_mmap_offset(obj);
>>
>> /* release file pointer to gem object. */
>> drm_gem_object_release(obj);
>> @@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>> goto unlock;
>> }
>>
>> - if (!obj->map_list.map) {
>> - ret = drm_gem_create_mmap_offset(obj);
>> - if (ret)
>> - goto out;
>> - }
>> + ret = drm_gem_create_mmap_offset(obj);
>> + if (ret)
>> + goto out;
>>
>> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
>> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
>> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>
>> out:
>> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
>> index fe1d332..2f77bea 100644
>> --- a/drivers/gpu/drm/gma500/gem.c
>> +++ b/drivers/gpu/drm/gma500/gem.c
>> @@ -26,6 +26,7 @@
>> #include <drm/drmP.h>
>> #include <drm/drm.h>
>> #include <drm/gma_drm.h>
>> +#include <drm/drm_vma_manager.h>
>> #include "psb_drv.h"
>>
>> int psb_gem_init_object(struct drm_gem_object *obj)
>> @@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj)
>> struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
>>
>> /* Remove the list map if one is present */
>> - if (obj->map_list.map)
>> - drm_gem_free_mmap_offset(obj);
>> + drm_gem_free_mmap_offset(obj);
>> drm_gem_object_release(obj);
>>
>> /* This must occur last as it frees up the memory of the GEM object */
>> @@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev,
>> /* What validation is needed here ? */
>>
>> /* Make it mmapable */
>> - if (!obj->map_list.map) {
>> - ret = drm_gem_create_mmap_offset(obj);
>> - if (ret)
>> - goto out;
>> - }
>> - /* GEM should really work out the hash offsets for us */
>> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
>> + ret = drm_gem_create_mmap_offset(obj);
>> + if (ret)
>> + goto out;
>> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
>> out:
>> drm_gem_object_unreference(obj);
>> unlock:
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 46bf7e3..53f81b3 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -26,6 +26,7 @@
>> */
>>
>> #include <drm/drmP.h>
>> +#include <drm/drm_vma_manager.h>
>> #include <drm/i915_drm.h>
>> #include "i915_drv.h"
>> #include "i915_trace.h"
>> @@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
>>
>> if (obj->base.dev->dev_mapping)
>> unmap_mapping_range(obj->base.dev->dev_mapping,
>> - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
>> + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
>> obj->base.size, 1);
>>
>> obj->fault_mappable = false;
>> @@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
>> struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>> int ret;
>>
>> - if (obj->base.map_list.map)
>> + if (drm_vma_node_has_offset(&obj->base.vma_node))
>> return 0;
>>
>> dev_priv->mm.shrinker_no_lock_stealing = true;
>> @@ -1517,9 +1518,6 @@ out:
>>
>> static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
>> {
>> - if (!obj->base.map_list.map)
>> - return;
>> -
>> drm_gem_free_mmap_offset(&obj->base);
>> }
>>
>> @@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
>> if (ret)
>> goto out;
>>
>> - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
>> + *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
>>
>> out:
>> drm_gem_object_unreference(&obj->base);
>> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
>> index cbcd71e..f90531fc00 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
>> @@ -20,6 +20,7 @@
>>
>> #include <linux/spinlock.h>
>> #include <linux/shmem_fs.h>
>> +#include <drm/drm_vma_manager.h>
>>
>> #include "omap_drv.h"
>> #include "omap_dmm_tiler.h"
>> @@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj)
>> static uint64_t mmap_offset(struct drm_gem_object *obj)
>> {
>> struct drm_device *dev = obj->dev;
>> + int ret;
>> + size_t size;
>>
>> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>>
>> - if (!obj->map_list.map) {
>> - /* Make it mmapable */
>> - size_t size = omap_gem_mmap_size(obj);
>> - int ret = _drm_gem_create_mmap_offset_size(obj, size);
>> -
>> - if (ret) {
>> - dev_err(dev->dev, "could not allocate mmap offset\n");
>> - return 0;
>> - }
>> + /* Make it mmapable */
>> + size = omap_gem_mmap_size(obj);
>> + ret = _drm_gem_create_mmap_offset_size(obj, size);
>> + if (ret) {
>> + dev_err(dev->dev, "could not allocate mmap offset\n");
>> + return 0;
>> }
>>
>> - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
>> + return drm_vma_node_offset_addr(&obj->vma_node);
>> }
>>
>> uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj)
>> @@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
>> {
>> struct drm_device *dev = obj->dev;
>> struct omap_gem_object *omap_obj = to_omap_bo(obj);
>> - uint64_t off = 0;
>> + uint64_t off;
>>
>> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>>
>> - if (obj->map_list.map)
>> - off = (uint64_t)obj->map_list.hash.key;
>> + off = drm_vma_node_start(&obj->vma_node);
>>
>> seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d",
>> omap_obj->flags, obj->name, obj->refcount.refcount.counter,
>> @@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
>>
>> list_del(&omap_obj->mm_list);
>>
>> - if (obj->map_list.map)
>> - drm_gem_free_mmap_offset(obj);
>> + drm_gem_free_mmap_offset(obj);
>>
>> /* this means the object is still pinned.. which really should
>> * not happen. I think..
>> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
>> index f9eb679..dbb1575 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
>> @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
>> {
>> struct drm_device *dev = obj->dev;
>> struct drm_gem_mm *mm = dev->mm_private;
>> - struct drm_map_list *list;
>> - struct drm_local_map *map;
>> - int ret = 0;
>> -
>> - /* Set the object up for mmap'ing */
>> - list = &obj->map_list;
>> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
>> - if (!list->map)
>> - return -ENOMEM;
>> -
>> - map = list->map;
>> - map->type = _DRM_GEM;
>> - map->size = size;
>> - map->handle = obj;
>> -
>> - /* Get a DRM GEM mmap offset allocated... */
>> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
>> - size / PAGE_SIZE, 0, 0);
>> -
>> - if (!list->file_offset_node) {
>> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
>> - ret = -ENOSPC;
>> - goto out_free_list;
>> - }
>> -
>> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
>> - size / PAGE_SIZE, 0);
>> - if (!list->file_offset_node) {
>> - ret = -ENOMEM;
>> - goto out_free_list;
>> - }
>> -
>> - list->hash.key = list->file_offset_node->start;
>> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
>> - if (ret) {
>> - DRM_ERROR("failed to add to map hash\n");
>> - goto out_free_mm;
>> - }
>> -
>> - return 0;
>> -
>> -out_free_mm:
>> - drm_mm_put_block(list->file_offset_node);
>> -out_free_list:
>> - kfree(list->map);
>> - list->map = NULL;
>>
>> - return ret;
>> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
>> + size / PAGE_SIZE);
>> }
>> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
>> index ef034fa..2a4cb2f 100644
>> --- a/drivers/gpu/drm/udl/udl_gem.c
>> +++ b/drivers/gpu/drm/udl/udl_gem.c
>> @@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
>> if (obj->pages)
>> udl_gem_put_pages(obj);
>>
>> - if (gem_obj->map_list.map)
>> - drm_gem_free_mmap_offset(gem_obj);
>> + drm_gem_free_mmap_offset(gem_obj);
>> }
>>
>> /* the dumb interface doesn't work with the GEM straight MMAP
>> @@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
>> ret = udl_gem_get_pages(gobj, GFP_KERNEL);
>> if (ret)
>> goto out;
>> - if (!gobj->base.map_list.map) {
>> - ret = drm_gem_create_mmap_offset(obj);
>> - if (ret)
>> - goto out;
>> - }
>> + ret = drm_gem_create_mmap_offset(obj);
>> + if (ret)
>> + goto out;
>>
>> - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
>> + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
>>
>> out:
>> drm_gem_object_unreference(&gobj->base);
>> diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c
>> index c5e9a9b..bc323b3 100644
>> --- a/drivers/gpu/host1x/drm/gem.c
>> +++ b/drivers/gpu/host1x/drm/gem.c
>> @@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
>>
>> unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo)
>> {
>> - return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT;
>> + return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node);
>
> Ok, tegra is pretty funny here about mmap offsets, using an int.
> Whatever, pre-existing fanciness so not our problem.
All TTM helpers use "unsigned int", too. We can fix that in a followup patch.
>> }
>>
>> struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size)
>> @@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
>> {
>> struct tegra_bo *bo = to_tegra_bo(gem);
>>
>> - if (gem->map_list.map)
>> - drm_gem_free_mmap_offset(gem);
>> + drm_gem_free_mmap_offset(gem);
>>
>> drm_gem_object_release(gem);
>> tegra_bo_destroy(gem->dev, bo);
>> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> index 0ab6a09..4b518e0 100644
>> --- a/include/drm/drmP.h
>> +++ b/include/drm/drmP.h
>> @@ -71,6 +71,7 @@
>> #include <asm/pgalloc.h>
>> #include <drm/drm.h>
>> #include <drm/drm_sarea.h>
>> +#include <drm/drm_vma_manager.h>
>>
>> #include <linux/idr.h>
>>
>> @@ -587,7 +588,6 @@ struct drm_map_list {
>> struct drm_local_map *map; /**< mapping */
>> uint64_t user_token;
>> struct drm_master *master;
>> - struct drm_mm_node *file_offset_node; /**< fake offset */
>> };
>>
>> /**
>> @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
>> * GEM specific mm private for tracking GEM objects
>> */
>> struct drm_gem_mm {
>> - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */
>> - struct drm_open_hash offset_hash; /**< User token hash table for maps */
>> + struct drm_vma_offset_manager vma_manager;
>> };
>>
>> /**
>> @@ -644,7 +643,7 @@ struct drm_gem_object {
>> struct file *filp;
>>
>> /* Mapping info for this object */
>> - struct drm_map_list map_list;
>> + struct drm_vma_offset_node vma_node;
>>
>> /**
>> * Size of the object, in bytes. Immutable over the object's
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 238a166..272580c 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -181,7 +181,7 @@ enum drm_map_type {
>> _DRM_AGP = 3, /**< AGP/GART */
>> _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */
>> _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
>> - _DRM_GEM = 6, /**< GEM object */
>> + _DRM_GEM = 6, /**< GEM object (obsolete) */
>> };
>>
>> /**
>> --
>> 1.8.3.3
>
> A few small things, with those addressed this is:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> I guess with the common infrastructure we'll have a bit of room for
> follow-up cleanups, but that can be done on especially rainy autumn days
> ;-)
Thanks!
David
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 2/4] drm/gem: convert to new unified vma manager
2013-07-23 12:47 ` [PATCH v4 2/4] drm/gem: convert to new unified vma manager David Herrmann
2013-07-24 15:52 ` Daniel Vetter
@ 2013-07-24 19:07 ` David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 19:07 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie
Use the new vma manager instead of the old hashtable. Also convert all
drivers to use the new convenience helpers. This drops all the
(map_list.hash.key << PAGE_SHIFT) non-sense.
Locking and access-management is exactly the same as before with an
additional lock inside of the vma-manager, which strictly wouldn't be
needed for gem.
v2:
- rebase on drm-next
- init nodes via drm_vma_node_reset() in drm_gem.c
v3:
- fix tegra
v4:
- remove duplicate if (drm_vma_node_has_offset()) checks
- inline now trivial drm_vma_node_offset_addr() calls
v5:
- skip node-reset on gem-init due to kzalloc()
- do not allow mapping gem-objects with offsets (backwards compat)
- remove unneccessary casts
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_gem.c | 89 +++++-------------------------
drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++----
drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++---
drivers/gpu/drm/gma500/gem.c | 15 ++---
drivers/gpu/drm/i915/i915_gem.c | 10 ++--
drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++-----
drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---------------
drivers/gpu/drm/udl/udl_gem.c | 13 ++---
drivers/gpu/host1x/drm/gem.c | 5 +-
include/drm/drmP.h | 7 +--
include/uapi/drm/drm.h | 2 +-
11 files changed, 62 insertions(+), 186 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1ad9e7e..3613b50 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -37,6 +37,7 @@
#include <linux/shmem_fs.h>
#include <linux/dma-buf.h>
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
/** @file drm_gem.c
*
@@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
}
dev->mm_private = mm;
-
- if (drm_ht_create(&mm->offset_hash, 12)) {
- kfree(mm);
- return -ENOMEM;
- }
-
- drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,
- DRM_FILE_PAGE_OFFSET_SIZE);
+ drm_vma_offset_manager_init(&mm->vma_manager,
+ DRM_FILE_PAGE_OFFSET_START,
+ DRM_FILE_PAGE_OFFSET_SIZE);
return 0;
}
@@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
{
struct drm_gem_mm *mm = dev->mm_private;
- drm_mm_takedown(&mm->offset_manager);
- drm_ht_remove(&mm->offset_hash);
+ drm_vma_offset_manager_destroy(&mm->vma_manager);
kfree(mm);
dev->mm_private = NULL;
}
@@ -302,12 +297,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list = &obj->map_list;
- drm_ht_remove_item(&mm->offset_hash, &list->hash);
- drm_mm_put_block(list->file_offset_node);
- kfree(list->map);
- list->map = NULL;
+ drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);
}
EXPORT_SYMBOL(drm_gem_free_mmap_offset);
@@ -327,54 +318,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list;
- struct drm_local_map *map;
- int ret;
-
- /* Set the object up for mmap'ing */
- list = &obj->map_list;
- list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
- if (!list->map)
- return -ENOMEM;
-
- map = list->map;
- map->type = _DRM_GEM;
- map->size = obj->size;
- map->handle = obj;
-
- /* Get a DRM GEM mmap offset allocated... */
- list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
- obj->size / PAGE_SIZE, 0, false);
-
- if (!list->file_offset_node) {
- DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
- ret = -ENOSPC;
- goto out_free_list;
- }
- list->file_offset_node = drm_mm_get_block(list->file_offset_node,
- obj->size / PAGE_SIZE, 0);
- if (!list->file_offset_node) {
- ret = -ENOMEM;
- goto out_free_list;
- }
-
- list->hash.key = list->file_offset_node->start;
- ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
- if (ret) {
- DRM_ERROR("failed to add to map hash\n");
- goto out_free_mm;
- }
-
- return 0;
-
-out_free_mm:
- drm_mm_put_block(list->file_offset_node);
-out_free_list:
- kfree(list->map);
- list->map = NULL;
-
- return ret;
+ return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
+ obj->size / PAGE_SIZE);
}
EXPORT_SYMBOL(drm_gem_create_mmap_offset);
@@ -703,8 +649,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
struct drm_file *priv = filp->private_data;
struct drm_device *dev = priv->minor->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_local_map *map = NULL;
- struct drm_hash_item *hash;
+ struct drm_gem_object *obj;
+ struct drm_vma_offset_node *node;
int ret = 0;
if (drm_device_is_unplugged(dev))
@@ -712,21 +658,16 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
mutex_lock(&dev->struct_mutex);
- if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
+ node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff,
+ vma_pages(vma));
+ if (!node) {
mutex_unlock(&dev->struct_mutex);
return drm_mmap(filp, vma);
}
- map = drm_hash_entry(hash, struct drm_map_list, hash)->map;
- if (!map ||
- ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- ret = drm_gem_mmap_obj(map->handle, map->size, vma);
+ obj = container_of(node, struct drm_gem_object, vma_node);
+ ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma);
-out_unlock:
mutex_unlock(&dev->struct_mutex);
return ret;
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index ece72a8..847f091 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -27,11 +27,7 @@
#include <drm/drmP.h>
#include <drm/drm.h>
#include <drm/drm_gem_cma_helper.h>
-
-static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj)
-{
- return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
-}
+#include <drm/drm_vma_manager.h>
/*
* __drm_gem_cma_create - Create a GEM CMA object without allocating memory
@@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
{
struct drm_gem_cma_object *cma_obj;
- if (gem_obj->map_list.map)
- drm_gem_free_mmap_offset(gem_obj);
+ drm_gem_free_mmap_offset(gem_obj);
cma_obj = to_drm_gem_cma_obj(gem_obj);
@@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
return -EINVAL;
}
- *offset = get_gem_mmap_offset(gem_obj);
+ *offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
drm_gem_object_unreference(gem_obj);
@@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
{
struct drm_gem_object *obj = &cma_obj->base;
struct drm_device *dev = obj->dev;
- uint64_t off = 0;
+ uint64_t off;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map)
- off = (uint64_t)obj->map_list.hash.key;
+ off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
obj->name, obj->refcount.refcount.counter,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 24c22a8..be32db1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -10,6 +10,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
#include <linux/shmem_fs.h>
#include <drm/exynos_drm.h>
@@ -152,8 +153,7 @@ out:
exynos_drm_fini_buf(obj->dev, buf);
exynos_gem_obj->buffer = NULL;
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
/* release file pointer to gem object. */
drm_gem_object_release(obj);
@@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
goto unlock;
}
- if (!obj->map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
- *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&obj->vma_node);
DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
out:
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index fe1d332..2f77bea 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -26,6 +26,7 @@
#include <drm/drmP.h>
#include <drm/drm.h>
#include <drm/gma_drm.h>
+#include <drm/drm_vma_manager.h>
#include "psb_drv.h"
int psb_gem_init_object(struct drm_gem_object *obj)
@@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj)
struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
/* Remove the list map if one is present */
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
drm_gem_object_release(obj);
/* This must occur last as it frees up the memory of the GEM object */
@@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev,
/* What validation is needed here ? */
/* Make it mmapable */
- if (!obj->map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
- /* GEM should really work out the hash offsets for us */
- *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
+ *offset = drm_vma_node_offset_addr(&obj->vma_node);
out:
drm_gem_object_unreference(obj);
unlock:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 46bf7e3..53f81b3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -26,6 +26,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_vma_manager.h>
#include <drm/i915_drm.h>
#include "i915_drv.h"
#include "i915_trace.h"
@@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (obj->base.dev->dev_mapping)
unmap_mapping_range(obj->base.dev->dev_mapping,
- (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
+ (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
obj->base.size, 1);
obj->fault_mappable = false;
@@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
int ret;
- if (obj->base.map_list.map)
+ if (drm_vma_node_has_offset(&obj->base.vma_node))
return 0;
dev_priv->mm.shrinker_no_lock_stealing = true;
@@ -1517,9 +1518,6 @@ out:
static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
{
- if (!obj->base.map_list.map)
- return;
-
drm_gem_free_mmap_offset(&obj->base);
}
@@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
if (ret)
goto out;
- *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
out:
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index cbcd71e..f90531fc00 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <linux/shmem_fs.h>
+#include <drm/drm_vma_manager.h>
#include "omap_drv.h"
#include "omap_dmm_tiler.h"
@@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj)
static uint64_t mmap_offset(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
+ int ret;
+ size_t size;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (!obj->map_list.map) {
- /* Make it mmapable */
- size_t size = omap_gem_mmap_size(obj);
- int ret = _drm_gem_create_mmap_offset_size(obj, size);
-
- if (ret) {
- dev_err(dev->dev, "could not allocate mmap offset\n");
- return 0;
- }
+ /* Make it mmapable */
+ size = omap_gem_mmap_size(obj);
+ ret = _drm_gem_create_mmap_offset_size(obj, size);
+ if (ret) {
+ dev_err(dev->dev, "could not allocate mmap offset\n");
+ return 0;
}
- return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
+ return drm_vma_node_offset_addr(&obj->vma_node);
}
uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj)
@@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
{
struct drm_device *dev = obj->dev;
struct omap_gem_object *omap_obj = to_omap_bo(obj);
- uint64_t off = 0;
+ uint64_t off;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map)
- off = (uint64_t)obj->map_list.hash.key;
+ off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d",
omap_obj->flags, obj->name, obj->refcount.refcount.counter,
@@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
list_del(&omap_obj->mm_list);
- if (obj->map_list.map)
- drm_gem_free_mmap_offset(obj);
+ drm_gem_free_mmap_offset(obj);
/* this means the object is still pinned.. which really should
* not happen. I think..
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
index f9eb679..dbb1575 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
@@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
{
struct drm_device *dev = obj->dev;
struct drm_gem_mm *mm = dev->mm_private;
- struct drm_map_list *list;
- struct drm_local_map *map;
- int ret = 0;
-
- /* Set the object up for mmap'ing */
- list = &obj->map_list;
- list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
- if (!list->map)
- return -ENOMEM;
-
- map = list->map;
- map->type = _DRM_GEM;
- map->size = size;
- map->handle = obj;
-
- /* Get a DRM GEM mmap offset allocated... */
- list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
- size / PAGE_SIZE, 0, 0);
-
- if (!list->file_offset_node) {
- DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
- ret = -ENOSPC;
- goto out_free_list;
- }
-
- list->file_offset_node = drm_mm_get_block(list->file_offset_node,
- size / PAGE_SIZE, 0);
- if (!list->file_offset_node) {
- ret = -ENOMEM;
- goto out_free_list;
- }
-
- list->hash.key = list->file_offset_node->start;
- ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
- if (ret) {
- DRM_ERROR("failed to add to map hash\n");
- goto out_free_mm;
- }
-
- return 0;
-
-out_free_mm:
- drm_mm_put_block(list->file_offset_node);
-out_free_list:
- kfree(list->map);
- list->map = NULL;
- return ret;
+ return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
+ size / PAGE_SIZE);
}
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index ef034fa..2a4cb2f 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
if (obj->pages)
udl_gem_put_pages(obj);
- if (gem_obj->map_list.map)
- drm_gem_free_mmap_offset(gem_obj);
+ drm_gem_free_mmap_offset(gem_obj);
}
/* the dumb interface doesn't work with the GEM straight MMAP
@@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
ret = udl_gem_get_pages(gobj, GFP_KERNEL);
if (ret)
goto out;
- if (!gobj->base.map_list.map) {
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
- }
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret)
+ goto out;
- *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
+ *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
out:
drm_gem_object_unreference(&gobj->base);
diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c
index c5e9a9b..bc323b3 100644
--- a/drivers/gpu/host1x/drm/gem.c
+++ b/drivers/gpu/host1x/drm/gem.c
@@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo)
{
- return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT;
+ return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node);
}
struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size)
@@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
{
struct tegra_bo *bo = to_tegra_bo(gem);
- if (gem->map_list.map)
- drm_gem_free_mmap_offset(gem);
+ drm_gem_free_mmap_offset(gem);
drm_gem_object_release(gem);
tegra_bo_destroy(gem->dev, bo);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0ab6a09..4b518e0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -71,6 +71,7 @@
#include <asm/pgalloc.h>
#include <drm/drm.h>
#include <drm/drm_sarea.h>
+#include <drm/drm_vma_manager.h>
#include <linux/idr.h>
@@ -587,7 +588,6 @@ struct drm_map_list {
struct drm_local_map *map; /**< mapping */
uint64_t user_token;
struct drm_master *master;
- struct drm_mm_node *file_offset_node; /**< fake offset */
};
/**
@@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
* GEM specific mm private for tracking GEM objects
*/
struct drm_gem_mm {
- struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */
- struct drm_open_hash offset_hash; /**< User token hash table for maps */
+ struct drm_vma_offset_manager vma_manager;
};
/**
@@ -644,7 +643,7 @@ struct drm_gem_object {
struct file *filp;
/* Mapping info for this object */
- struct drm_map_list map_list;
+ struct drm_vma_offset_node vma_node;
/**
* Size of the object, in bytes. Immutable over the object's
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 238a166..272580c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -181,7 +181,7 @@ enum drm_map_type {
_DRM_AGP = 3, /**< AGP/GART */
_DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */
_DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
- _DRM_GEM = 6, /**< GEM object */
+ _DRM_GEM = 6, /**< GEM object (obsolete) */
};
/**
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 0/4] Unified VMA Offset Manager David Herrmann
2013-07-23 12:47 ` [PATCH v4 1/4] drm: add unified vma offset manager David Herrmann
2013-07-23 12:47 ` [PATCH v4 2/4] drm/gem: convert to new unified vma manager David Herrmann
@ 2013-07-23 12:47 ` David Herrmann
2013-07-24 15:57 ` Daniel Vetter
2013-07-24 19:08 ` [PATCH v5 " David Herrmann
2013-07-23 12:47 ` [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
3 siblings, 2 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-23 12:47 UTC (permalink / raw)
To: dri-devel
Cc: Thomas Hellstrom, Martin Peres, Daniel Vetter, Alex Deucher,
Ben Skeggs, Dave Airlie
Use the new vma-manager infrastructure. This doesn't change any
implementation details as the vma-offset-manager is nearly copied 1-to-1
from TTM.
The vm_lock is moved into the offset manager so we can drop it from TTM.
During lookup, we use the vma locking helpers to take a reference to the
found object.
In all other scenarios, locking stays the same as before. We always
guarantee that drm_vma_offset_remove() is called only during destruction.
Hence, helpers like drm_vma_node_offset_addr() are always safe as long as
the node has a valid offset.
This also drops the addr_space_offset member as it is a copy of vm_start
in vma_node objects. Use the accessor functions instead.
v4:
- remove vm_lock
- use drm_vma_offset_lock_lookup() to protect lookup (instead of vm_lock)
Cc: Dave Airlie <airlied@redhat.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Martin Peres <martin.peres@labri.fr>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/ast/ast_main.c | 2 +-
drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.h | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.h | 5 +-
drivers/gpu/drm/ttm/ttm_bo.c | 89 ++++++-------------------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +-
drivers/gpu/drm/ttm/ttm_bo_vm.c | 81 +++++++++++-----------------
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +-
include/drm/ttm/ttm_bo_api.h | 15 ++----
include/drm/ttm/ttm_bo_driver.h | 10 ++--
14 files changed, 66 insertions(+), 155 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f60fd7b..c195dc2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -487,7 +487,7 @@ void ast_gem_free_object(struct drm_gem_object *obj)
static inline u64 ast_bo_mmap_offset(struct ast_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
ast_dumb_mmap_offset(struct drm_file *file,
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 35cbae8..3a7a0ef 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -294,7 +294,7 @@ void cirrus_gem_free_object(struct drm_gem_object *obj)
static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 9fa5685..1a75ea3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -349,7 +349,7 @@ void mgag200_gem_free_object(struct drm_gem_object *obj)
static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 708b2d1..7a8caa1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -697,7 +697,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv,
gem = drm_gem_object_lookup(dev, file_priv, handle);
if (gem) {
struct nouveau_bo *bo = gem->driver_private;
- *poffset = bo->bo.addr_space_offset;
+ *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node);
drm_gem_object_unreference_unlocked(gem);
return 0;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index e72d09c..86597eb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -226,7 +226,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem,
}
rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT;
- rep->map_handle = nvbo->bo.addr_space_offset;
+ rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node);
rep->tile_mode = nvbo->tile_mode;
rep->tile_flags = nvbo->tile_flags;
return 0;
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index ee7ad79..af10165 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -59,7 +59,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
{
- return bo->tbo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->tbo.vma_node);
}
static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index b443d67..1a648e1 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -87,7 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
for (i = 0 ; i < release->bo_count; ++i) {
QXL_INFO(qdev, "release %llx\n",
- release->bos[i]->tbo.addr_space_offset
+ drm_vma_node_offset_addr(&release->bos[i]->tbo.vma_node)
- DRM_FILE_OFFSET);
qxl_fence_remove_release(&release->bos[i]->fence, release->id);
qxl_bo_unref(&release->bos[i]);
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 49c82c4..209b111 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -113,13 +113,10 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
* @bo: radeon object for which we query the offset
*
* Returns mmap offset of the object.
- *
- * Note: addr_space_offset is constant after ttm bo init thus isn't protected
- * by any lock.
*/
static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo)
{
- return bo->tbo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->tbo.vma_node);
}
extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb9dd67..3dc08b6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -615,13 +615,7 @@ static void ttm_bo_release(struct kref *kref)
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
- write_lock(&bdev->vm_lock);
- if (likely(bo->vm_node != NULL)) {
- rb_erase(&bo->vm_rb, &bdev->addr_space_rb);
- drm_mm_put_block(bo->vm_node);
- bo->vm_node = NULL;
- }
- write_unlock(&bdev->vm_lock);
+ drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node);
ttm_mem_io_lock(man, false);
ttm_mem_io_free_vm(bo);
ttm_mem_io_unlock(man);
@@ -1129,6 +1123,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
bo->resv = &bo->ttm_resv;
reservation_object_init(bo->resv);
atomic_inc(&bo->glob->bo_count);
+ drm_vma_node_reset(&bo->vma_node);
ret = ttm_bo_check_placement(bo, placement);
@@ -1424,10 +1419,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
TTM_DEBUG("Swap list was clean\n");
spin_unlock(&glob->lru_lock);
- BUG_ON(!drm_mm_clean(&bdev->addr_space_mm));
- write_lock(&bdev->vm_lock);
- drm_mm_takedown(&bdev->addr_space_mm);
- write_unlock(&bdev->vm_lock);
+ drm_vma_offset_manager_destroy(&bdev->vma_manager);
return ret;
}
@@ -1441,7 +1433,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
{
int ret = -EINVAL;
- rwlock_init(&bdev->vm_lock);
bdev->driver = driver;
memset(bdev->man, 0, sizeof(bdev->man));
@@ -1454,9 +1445,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
if (unlikely(ret != 0))
goto out_no_sys;
- bdev->addr_space_rb = RB_ROOT;
- drm_mm_init(&bdev->addr_space_mm, file_page_offset, 0x10000000);
-
+ drm_vma_offset_manager_init(&bdev->vma_manager, file_page_offset,
+ 0x10000000);
INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
INIT_LIST_HEAD(&bdev->ddestroy);
bdev->dev_mapping = NULL;
@@ -1498,12 +1488,17 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- loff_t offset = (loff_t) bo->addr_space_offset;
- loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
+ loff_t offset, holelen;
if (!bdev->dev_mapping)
return;
- unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
+
+ if (drm_vma_node_has_offset(&bo->vma_node)) {
+ offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
+ holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
+
+ unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
+ }
ttm_mem_io_free_vm(bo);
}
@@ -1520,31 +1515,6 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
EXPORT_SYMBOL(ttm_bo_unmap_virtual);
-static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
-{
- struct ttm_bo_device *bdev = bo->bdev;
- struct rb_node **cur = &bdev->addr_space_rb.rb_node;
- struct rb_node *parent = NULL;
- struct ttm_buffer_object *cur_bo;
- unsigned long offset = bo->vm_node->start;
- unsigned long cur_offset;
-
- while (*cur) {
- parent = *cur;
- cur_bo = rb_entry(parent, struct ttm_buffer_object, vm_rb);
- cur_offset = cur_bo->vm_node->start;
- if (offset < cur_offset)
- cur = &parent->rb_left;
- else if (offset > cur_offset)
- cur = &parent->rb_right;
- else
- BUG();
- }
-
- rb_link_node(&bo->vm_rb, parent, cur);
- rb_insert_color(&bo->vm_rb, &bdev->addr_space_rb);
-}
-
/**
* ttm_bo_setup_vm:
*
@@ -1559,38 +1529,9 @@ static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
static int ttm_bo_setup_vm(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- int ret;
-
-retry_pre_get:
- ret = drm_mm_pre_get(&bdev->addr_space_mm);
- if (unlikely(ret != 0))
- return ret;
-
- write_lock(&bdev->vm_lock);
- bo->vm_node = drm_mm_search_free(&bdev->addr_space_mm,
- bo->mem.num_pages, 0, 0);
-
- if (unlikely(bo->vm_node == NULL)) {
- ret = -ENOMEM;
- goto out_unlock;
- }
- bo->vm_node = drm_mm_get_block_atomic(bo->vm_node,
- bo->mem.num_pages, 0);
-
- if (unlikely(bo->vm_node == NULL)) {
- write_unlock(&bdev->vm_lock);
- goto retry_pre_get;
- }
-
- ttm_bo_vm_insert_rb(bo);
- write_unlock(&bdev->vm_lock);
- bo->addr_space_offset = ((uint64_t) bo->vm_node->start) << PAGE_SHIFT;
-
- return 0;
-out_unlock:
- write_unlock(&bdev->vm_lock);
- return ret;
+ return drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
+ bo->mem.num_pages);
}
int ttm_bo_wait(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 319cf41..7cc904d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -30,6 +30,7 @@
#include <drm/ttm/ttm_bo_driver.h>
#include <drm/ttm/ttm_placement.h>
+#include <drm/drm_vma_manager.h>
#include <linux/io.h>
#include <linux/highmem.h>
#include <linux/wait.h>
@@ -450,7 +451,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
INIT_LIST_HEAD(&fbo->lru);
INIT_LIST_HEAD(&fbo->swap);
INIT_LIST_HEAD(&fbo->io_reserve_lru);
- fbo->vm_node = NULL;
+ drm_vma_node_reset(&fbo->vma_node);
atomic_set(&fbo->cpu_writers, 0);
spin_lock(&bdev->fence_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 3df9f16..8c0e2c0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -33,6 +33,7 @@
#include <ttm/ttm_module.h>
#include <ttm/ttm_bo_driver.h>
#include <ttm/ttm_placement.h>
+#include <drm/drm_vma_manager.h>
#include <linux/mm.h>
#include <linux/rbtree.h>
#include <linux/module.h>
@@ -40,37 +41,6 @@
#define TTM_BO_VM_NUM_PREFAULT 16
-static struct ttm_buffer_object *ttm_bo_vm_lookup_rb(struct ttm_bo_device *bdev,
- unsigned long page_start,
- unsigned long num_pages)
-{
- struct rb_node *cur = bdev->addr_space_rb.rb_node;
- unsigned long cur_offset;
- struct ttm_buffer_object *bo;
- struct ttm_buffer_object *best_bo = NULL;
-
- while (likely(cur != NULL)) {
- bo = rb_entry(cur, struct ttm_buffer_object, vm_rb);
- cur_offset = bo->vm_node->start;
- if (page_start >= cur_offset) {
- cur = cur->rb_right;
- best_bo = bo;
- if (page_start == cur_offset)
- break;
- } else
- cur = cur->rb_left;
- }
-
- if (unlikely(best_bo == NULL))
- return NULL;
-
- if (unlikely((best_bo->vm_node->start + best_bo->num_pages) <
- (page_start + num_pages)))
- return NULL;
-
- return best_bo;
-}
-
static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
@@ -146,9 +116,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
}
page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
- bo->vm_node->start - vma->vm_pgoff;
+ drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
page_last = vma_pages(vma) +
- bo->vm_node->start - vma->vm_pgoff;
+ drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
if (unlikely(page_offset >= bo->num_pages)) {
retval = VM_FAULT_SIGBUS;
@@ -249,6 +219,30 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.close = ttm_bo_vm_close
};
+static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
+ unsigned long offset,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node;
+ struct ttm_buffer_object *bo = NULL;
+
+ drm_vma_offset_lock_lookup(&bdev->vma_manager);
+
+ node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages);
+ if (likely(node)) {
+ bo = container_of(node, struct ttm_buffer_object, vma_node);
+ if (!kref_get_unless_zero(&bo->kref))
+ bo = NULL;
+ }
+
+ drm_vma_offset_unlock_lookup(&bdev->vma_manager);
+
+ if (!bo)
+ pr_err("Could not find buffer object to map\n");
+
+ return bo;
+}
+
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
{
@@ -256,17 +250,9 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_buffer_object *bo;
int ret;
- read_lock(&bdev->vm_lock);
- bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff,
- vma_pages(vma));
- if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref))
- bo = NULL;
- read_unlock(&bdev->vm_lock);
-
- if (unlikely(bo == NULL)) {
- pr_err("Could not find buffer object to map\n");
+ bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+ if (unlikely(!bo))
return -EINVAL;
- }
driver = bo->bdev->driver;
if (unlikely(!driver->verify_access)) {
@@ -324,12 +310,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
bool no_wait = false;
bool dummy;
- read_lock(&bdev->vm_lock);
- bo = ttm_bo_vm_lookup_rb(bdev, dev_offset, 1);
- if (likely(bo != NULL))
- ttm_bo_reference(bo);
- read_unlock(&bdev->vm_lock);
-
+ bo = ttm_bo_vm_lookup(bdev, dev_offset, 1);
if (unlikely(bo == NULL))
return -EFAULT;
@@ -343,7 +324,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
if (unlikely(ret != 0))
goto out_unref;
- kmap_offset = dev_offset - bo->vm_node->start;
+ kmap_offset = dev_offset - drm_vma_node_start(&bo->vma_node);
if (unlikely(kmap_offset >= bo->num_pages)) {
ret = -EFBIG;
goto out_unref;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 7953d1f..0e67cf4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -500,7 +500,7 @@ int vmw_dmabuf_alloc_ioctl(struct drm_device *dev, void *data,
goto out_no_dmabuf;
rep->handle = handle;
- rep->map_handle = dma_buf->base.addr_space_offset;
+ rep->map_handle = drm_vma_node_offset_addr(&dma_buf->base.vma_node);
rep->cur_gmr_id = handle;
rep->cur_gmr_offset = 0;
@@ -834,7 +834,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv,
if (ret != 0)
return -EINVAL;
- *offset = out_buf->base.addr_space_offset;
+ *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node);
vmw_dmabuf_unreference(&out_buf);
return 0;
}
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 8a6aa56..751eaff 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -32,12 +32,12 @@
#define _TTM_BO_API_H_
#include <drm/drm_hashtab.h>
+#include <drm/drm_vma_manager.h>
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/wait.h>
#include <linux/mutex.h>
#include <linux/mm.h>
-#include <linux/rbtree.h>
#include <linux/bitmap.h>
#include <linux/reservation.h>
@@ -145,7 +145,6 @@ struct ttm_tt;
* @type: The bo type.
* @destroy: Destruction function. If NULL, kfree is used.
* @num_pages: Actual number of pages.
- * @addr_space_offset: Address space offset.
* @acc_size: Accounted size for this object.
* @kref: Reference count of this buffer object. When this refcount reaches
* zero, the object is put on the delayed delete list.
@@ -166,8 +165,7 @@ struct ttm_tt;
* @swap: List head for swap LRU list.
* @sync_obj: Pointer to a synchronization object.
* @priv_flags: Flags describing buffer object internal state.
- * @vm_rb: Rb node for the vm rb tree.
- * @vm_node: Address space manager node.
+ * @vma_node: Address space manager node.
* @offset: The current GPU offset, which can have different meanings
* depending on the memory type. For SYSTEM type memory, it should be 0.
* @cur_placement: Hint of current placement.
@@ -194,7 +192,6 @@ struct ttm_buffer_object {
enum ttm_bo_type type;
void (*destroy) (struct ttm_buffer_object *);
unsigned long num_pages;
- uint64_t addr_space_offset;
size_t acc_size;
/**
@@ -238,13 +235,7 @@ struct ttm_buffer_object {
void *sync_obj;
unsigned long priv_flags;
- /**
- * Members protected by the bdev::vm_lock
- */
-
- struct rb_node vm_rb;
- struct drm_mm_node *vm_node;
-
+ struct drm_vma_offset_node vma_node;
/**
* Special members that are protected by the reserve lock
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 984fc2d..8639c85 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -36,6 +36,7 @@
#include <ttm/ttm_placement.h>
#include <drm/drm_mm.h>
#include <drm/drm_global.h>
+#include <drm/drm_vma_manager.h>
#include <linux/workqueue.h>
#include <linux/fs.h>
#include <linux/spinlock.h>
@@ -519,7 +520,7 @@ struct ttm_bo_global {
* @man: An array of mem_type_managers.
* @fence_lock: Protects the synchronizing members on *all* bos belonging
* to this device.
- * @addr_space_mm: Range manager for the device address space.
+ * @vma_manager: Address space manager
* lru_lock: Spinlock that protects the buffer+device lru lists and
* ddestroy lists.
* @val_seq: Current validation sequence.
@@ -537,14 +538,13 @@ struct ttm_bo_device {
struct list_head device_list;
struct ttm_bo_global *glob;
struct ttm_bo_driver *driver;
- rwlock_t vm_lock;
struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
spinlock_t fence_lock;
+
/*
- * Protected by the vm lock.
+ * Protected by internal locks.
*/
- struct rb_root addr_space_rb;
- struct drm_mm addr_space_mm;
+ struct drm_vma_offset_manager vma_manager;
/*
* Protected by the global:lru lock.
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
@ 2013-07-24 15:57 ` Daniel Vetter
2013-07-24 19:08 ` [PATCH v5 " David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-07-24 15:57 UTC (permalink / raw)
To: David Herrmann
Cc: Thomas Hellstrom, Martin Peres, Daniel Vetter, dri-devel,
Alex Deucher, Ben Skeggs, Dave Airlie
On Tue, Jul 23, 2013 at 02:47:15PM +0200, David Herrmann wrote:
> Use the new vma-manager infrastructure. This doesn't change any
> implementation details as the vma-offset-manager is nearly copied 1-to-1
> from TTM.
>
> The vm_lock is moved into the offset manager so we can drop it from TTM.
> During lookup, we use the vma locking helpers to take a reference to the
> found object.
> In all other scenarios, locking stays the same as before. We always
> guarantee that drm_vma_offset_remove() is called only during destruction.
> Hence, helpers like drm_vma_node_offset_addr() are always safe as long as
> the node has a valid offset.
>
> This also drops the addr_space_offset member as it is a copy of vm_start
> in vma_node objects. Use the accessor functions instead.
>
> v4:
> - remove vm_lock
> - use drm_vma_offset_lock_lookup() to protect lookup (instead of vm_lock)
>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> Cc: Martin Peres <martin.peres@labri.fr>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> drivers/gpu/drm/ast/ast_main.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
> drivers/gpu/drm/qxl/qxl_object.h | 2 +-
> drivers/gpu/drm/qxl/qxl_release.c | 2 +-
> drivers/gpu/drm/radeon/radeon_object.h | 5 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 89 ++++++-------------------------
> drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +-
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 81 +++++++++++-----------------
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +-
> include/drm/ttm/ttm_bo_api.h | 15 ++----
> include/drm/ttm/ttm_bo_driver.h | 10 ++--
> 14 files changed, 66 insertions(+), 155 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index f60fd7b..c195dc2 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -487,7 +487,7 @@ void ast_gem_free_object(struct drm_gem_object *obj)
>
> static inline u64 ast_bo_mmap_offset(struct ast_bo *bo)
> {
> - return bo->bo.addr_space_offset;
> + return drm_vma_node_offset_addr(&bo->bo.vma_node);
> }
> int
> ast_dumb_mmap_offset(struct drm_file *file,
> diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
> index 35cbae8..3a7a0ef 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_main.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_main.c
> @@ -294,7 +294,7 @@ void cirrus_gem_free_object(struct drm_gem_object *obj)
>
> static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo)
> {
> - return bo->bo.addr_space_offset;
> + return drm_vma_node_offset_addr(&bo->bo.vma_node);
> }
>
> int
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
> index 9fa5685..1a75ea3 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -349,7 +349,7 @@ void mgag200_gem_free_object(struct drm_gem_object *obj)
>
> static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo)
> {
> - return bo->bo.addr_space_offset;
> + return drm_vma_node_offset_addr(&bo->bo.vma_node);
> }
>
> int
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 708b2d1..7a8caa1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -697,7 +697,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv,
> gem = drm_gem_object_lookup(dev, file_priv, handle);
> if (gem) {
> struct nouveau_bo *bo = gem->driver_private;
> - *poffset = bo->bo.addr_space_offset;
> + *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node);
> drm_gem_object_unreference_unlocked(gem);
> return 0;
> }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index e72d09c..86597eb 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -226,7 +226,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem,
> }
>
> rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT;
> - rep->map_handle = nvbo->bo.addr_space_offset;
> + rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node);
> rep->tile_mode = nvbo->tile_mode;
> rep->tile_flags = nvbo->tile_flags;
> return 0;
> diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
> index ee7ad79..af10165 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.h
> +++ b/drivers/gpu/drm/qxl/qxl_object.h
> @@ -59,7 +59,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
>
> static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
> {
> - return bo->tbo.addr_space_offset;
> + return drm_vma_node_offset_addr(&bo->tbo.vma_node);
> }
>
> static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
> index b443d67..1a648e1 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -87,7 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
>
> for (i = 0 ; i < release->bo_count; ++i) {
> QXL_INFO(qdev, "release %llx\n",
> - release->bos[i]->tbo.addr_space_offset
> + drm_vma_node_offset_addr(&release->bos[i]->tbo.vma_node)
> - DRM_FILE_OFFSET);
> qxl_fence_remove_release(&release->bos[i]->fence, release->id);
> qxl_bo_unref(&release->bos[i]);
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> index 49c82c4..209b111 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -113,13 +113,10 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
> * @bo: radeon object for which we query the offset
> *
> * Returns mmap offset of the object.
> - *
> - * Note: addr_space_offset is constant after ttm bo init thus isn't protected
> - * by any lock.
> */
> static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo)
> {
> - return bo->tbo.addr_space_offset;
> + return drm_vma_node_offset_addr(&bo->tbo.vma_node);
> }
>
> extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index cb9dd67..3dc08b6 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -615,13 +615,7 @@ static void ttm_bo_release(struct kref *kref)
> struct ttm_bo_device *bdev = bo->bdev;
> struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
>
> - write_lock(&bdev->vm_lock);
> - if (likely(bo->vm_node != NULL)) {
> - rb_erase(&bo->vm_rb, &bdev->addr_space_rb);
> - drm_mm_put_block(bo->vm_node);
> - bo->vm_node = NULL;
> - }
> - write_unlock(&bdev->vm_lock);
> + drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node);
> ttm_mem_io_lock(man, false);
> ttm_mem_io_free_vm(bo);
> ttm_mem_io_unlock(man);
> @@ -1129,6 +1123,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
> bo->resv = &bo->ttm_resv;
> reservation_object_init(bo->resv);
> atomic_inc(&bo->glob->bo_count);
> + drm_vma_node_reset(&bo->vma_node);
>
> ret = ttm_bo_check_placement(bo, placement);
>
> @@ -1424,10 +1419,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
> TTM_DEBUG("Swap list was clean\n");
> spin_unlock(&glob->lru_lock);
>
> - BUG_ON(!drm_mm_clean(&bdev->addr_space_mm));
> - write_lock(&bdev->vm_lock);
> - drm_mm_takedown(&bdev->addr_space_mm);
> - write_unlock(&bdev->vm_lock);
> + drm_vma_offset_manager_destroy(&bdev->vma_manager);
>
> return ret;
> }
> @@ -1441,7 +1433,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
> {
> int ret = -EINVAL;
>
> - rwlock_init(&bdev->vm_lock);
> bdev->driver = driver;
>
> memset(bdev->man, 0, sizeof(bdev->man));
> @@ -1454,9 +1445,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
> if (unlikely(ret != 0))
> goto out_no_sys;
>
> - bdev->addr_space_rb = RB_ROOT;
> - drm_mm_init(&bdev->addr_space_mm, file_page_offset, 0x10000000);
> -
> + drm_vma_offset_manager_init(&bdev->vma_manager, file_page_offset,
> + 0x10000000);
> INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
> INIT_LIST_HEAD(&bdev->ddestroy);
> bdev->dev_mapping = NULL;
> @@ -1498,12 +1488,17 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
> {
> struct ttm_bo_device *bdev = bo->bdev;
> - loff_t offset = (loff_t) bo->addr_space_offset;
> - loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
> + loff_t offset, holelen;
>
> if (!bdev->dev_mapping)
> return;
> - unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
> +
> + if (drm_vma_node_has_offset(&bo->vma_node)) {
> + offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
> + holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
> +
> + unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
> + }
> ttm_mem_io_free_vm(bo);
> }
>
> @@ -1520,31 +1515,6 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
>
> EXPORT_SYMBOL(ttm_bo_unmap_virtual);
>
> -static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
> -{
> - struct ttm_bo_device *bdev = bo->bdev;
> - struct rb_node **cur = &bdev->addr_space_rb.rb_node;
> - struct rb_node *parent = NULL;
> - struct ttm_buffer_object *cur_bo;
> - unsigned long offset = bo->vm_node->start;
> - unsigned long cur_offset;
> -
> - while (*cur) {
> - parent = *cur;
> - cur_bo = rb_entry(parent, struct ttm_buffer_object, vm_rb);
> - cur_offset = cur_bo->vm_node->start;
> - if (offset < cur_offset)
> - cur = &parent->rb_left;
> - else if (offset > cur_offset)
> - cur = &parent->rb_right;
> - else
> - BUG();
> - }
> -
> - rb_link_node(&bo->vm_rb, parent, cur);
> - rb_insert_color(&bo->vm_rb, &bdev->addr_space_rb);
> -}
> -
> /**
> * ttm_bo_setup_vm:
> *
> @@ -1559,38 +1529,9 @@ static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
> static int ttm_bo_setup_vm(struct ttm_buffer_object *bo)
> {
> struct ttm_bo_device *bdev = bo->bdev;
> - int ret;
> -
> -retry_pre_get:
> - ret = drm_mm_pre_get(&bdev->addr_space_mm);
> - if (unlikely(ret != 0))
> - return ret;
> -
> - write_lock(&bdev->vm_lock);
> - bo->vm_node = drm_mm_search_free(&bdev->addr_space_mm,
> - bo->mem.num_pages, 0, 0);
> -
> - if (unlikely(bo->vm_node == NULL)) {
> - ret = -ENOMEM;
> - goto out_unlock;
> - }
>
> - bo->vm_node = drm_mm_get_block_atomic(bo->vm_node,
> - bo->mem.num_pages, 0);
> -
> - if (unlikely(bo->vm_node == NULL)) {
> - write_unlock(&bdev->vm_lock);
> - goto retry_pre_get;
> - }
> -
> - ttm_bo_vm_insert_rb(bo);
> - write_unlock(&bdev->vm_lock);
> - bo->addr_space_offset = ((uint64_t) bo->vm_node->start) << PAGE_SHIFT;
> -
> - return 0;
> -out_unlock:
> - write_unlock(&bdev->vm_lock);
> - return ret;
> + return drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
> + bo->mem.num_pages);
> }
>
> int ttm_bo_wait(struct ttm_buffer_object *bo,
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 319cf41..7cc904d 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -30,6 +30,7 @@
>
> #include <drm/ttm/ttm_bo_driver.h>
> #include <drm/ttm/ttm_placement.h>
> +#include <drm/drm_vma_manager.h>
> #include <linux/io.h>
> #include <linux/highmem.h>
> #include <linux/wait.h>
> @@ -450,7 +451,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
> INIT_LIST_HEAD(&fbo->lru);
> INIT_LIST_HEAD(&fbo->swap);
> INIT_LIST_HEAD(&fbo->io_reserve_lru);
> - fbo->vm_node = NULL;
> + drm_vma_node_reset(&fbo->vma_node);
> atomic_set(&fbo->cpu_writers, 0);
>
> spin_lock(&bdev->fence_lock);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 3df9f16..8c0e2c0 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -33,6 +33,7 @@
> #include <ttm/ttm_module.h>
> #include <ttm/ttm_bo_driver.h>
> #include <ttm/ttm_placement.h>
> +#include <drm/drm_vma_manager.h>
> #include <linux/mm.h>
> #include <linux/rbtree.h>
> #include <linux/module.h>
> @@ -40,37 +41,6 @@
>
> #define TTM_BO_VM_NUM_PREFAULT 16
>
> -static struct ttm_buffer_object *ttm_bo_vm_lookup_rb(struct ttm_bo_device *bdev,
> - unsigned long page_start,
> - unsigned long num_pages)
> -{
> - struct rb_node *cur = bdev->addr_space_rb.rb_node;
> - unsigned long cur_offset;
> - struct ttm_buffer_object *bo;
> - struct ttm_buffer_object *best_bo = NULL;
> -
> - while (likely(cur != NULL)) {
> - bo = rb_entry(cur, struct ttm_buffer_object, vm_rb);
> - cur_offset = bo->vm_node->start;
> - if (page_start >= cur_offset) {
> - cur = cur->rb_right;
> - best_bo = bo;
> - if (page_start == cur_offset)
> - break;
> - } else
> - cur = cur->rb_left;
> - }
> -
> - if (unlikely(best_bo == NULL))
> - return NULL;
> -
> - if (unlikely((best_bo->vm_node->start + best_bo->num_pages) <
> - (page_start + num_pages)))
> - return NULL;
> -
> - return best_bo;
> -}
> -
> static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> {
> struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
> @@ -146,9 +116,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> }
>
> page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
> - bo->vm_node->start - vma->vm_pgoff;
> + drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
> page_last = vma_pages(vma) +
> - bo->vm_node->start - vma->vm_pgoff;
> + drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
>
> if (unlikely(page_offset >= bo->num_pages)) {
> retval = VM_FAULT_SIGBUS;
> @@ -249,6 +219,30 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
> .close = ttm_bo_vm_close
> };
>
> +static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
> + unsigned long offset,
> + unsigned long pages)
> +{
> + struct drm_vma_offset_node *node;
> + struct ttm_buffer_object *bo = NULL;
> +
> + drm_vma_offset_lock_lookup(&bdev->vma_manager);
> +
> + node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages);
> + if (likely(node)) {
> + bo = container_of(node, struct ttm_buffer_object, vma_node);
> + if (!kref_get_unless_zero(&bo->kref))
> + bo = NULL;
> + }
> +
> + drm_vma_offset_unlock_lookup(&bdev->vma_manager);
> +
> + if (!bo)
> + pr_err("Could not find buffer object to map\n");
> +
> + return bo;
> +}
Bikeshed for next time around: If you keep the function at the same spot
as the old one the diff looks a bit cuter&is nicer to read.
> +
> int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
> struct ttm_bo_device *bdev)
> {
> @@ -256,17 +250,9 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
> struct ttm_buffer_object *bo;
> int ret;
>
> - read_lock(&bdev->vm_lock);
> - bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff,
> - vma_pages(vma));
> - if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref))
> - bo = NULL;
> - read_unlock(&bdev->vm_lock);
> -
> - if (unlikely(bo == NULL)) {
> - pr_err("Could not find buffer object to map\n");
> + bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
> + if (unlikely(!bo))
> return -EINVAL;
> - }
>
> driver = bo->bdev->driver;
> if (unlikely(!driver->verify_access)) {
> @@ -324,12 +310,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
> bool no_wait = false;
> bool dummy;
>
> - read_lock(&bdev->vm_lock);
> - bo = ttm_bo_vm_lookup_rb(bdev, dev_offset, 1);
> - if (likely(bo != NULL))
> - ttm_bo_reference(bo);
> - read_unlock(&bdev->vm_lock);
> -
> + bo = ttm_bo_vm_lookup(bdev, dev_offset, 1);
> if (unlikely(bo == NULL))
> return -EFAULT;
>
> @@ -343,7 +324,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
> if (unlikely(ret != 0))
> goto out_unref;
>
> - kmap_offset = dev_offset - bo->vm_node->start;
> + kmap_offset = dev_offset - drm_vma_node_start(&bo->vma_node);
> if (unlikely(kmap_offset >= bo->num_pages)) {
> ret = -EFBIG;
> goto out_unref;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 7953d1f..0e67cf4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -500,7 +500,7 @@ int vmw_dmabuf_alloc_ioctl(struct drm_device *dev, void *data,
> goto out_no_dmabuf;
>
> rep->handle = handle;
> - rep->map_handle = dma_buf->base.addr_space_offset;
> + rep->map_handle = drm_vma_node_offset_addr(&dma_buf->base.vma_node);
> rep->cur_gmr_id = handle;
> rep->cur_gmr_offset = 0;
>
> @@ -834,7 +834,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv,
> if (ret != 0)
> return -EINVAL;
>
> - *offset = out_buf->base.addr_space_offset;
> + *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node);
> vmw_dmabuf_unreference(&out_buf);
> return 0;
> }
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 8a6aa56..751eaff 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -32,12 +32,12 @@
> #define _TTM_BO_API_H_
>
> #include <drm/drm_hashtab.h>
> +#include <drm/drm_vma_manager.h>
> #include <linux/kref.h>
> #include <linux/list.h>
> #include <linux/wait.h>
> #include <linux/mutex.h>
> #include <linux/mm.h>
> -#include <linux/rbtree.h>
> #include <linux/bitmap.h>
> #include <linux/reservation.h>
>
> @@ -145,7 +145,6 @@ struct ttm_tt;
> * @type: The bo type.
> * @destroy: Destruction function. If NULL, kfree is used.
> * @num_pages: Actual number of pages.
> - * @addr_space_offset: Address space offset.
> * @acc_size: Accounted size for this object.
> * @kref: Reference count of this buffer object. When this refcount reaches
> * zero, the object is put on the delayed delete list.
> @@ -166,8 +165,7 @@ struct ttm_tt;
> * @swap: List head for swap LRU list.
> * @sync_obj: Pointer to a synchronization object.
> * @priv_flags: Flags describing buffer object internal state.
> - * @vm_rb: Rb node for the vm rb tree.
> - * @vm_node: Address space manager node.
> + * @vma_node: Address space manager node.
> * @offset: The current GPU offset, which can have different meanings
> * depending on the memory type. For SYSTEM type memory, it should be 0.
> * @cur_placement: Hint of current placement.
> @@ -194,7 +192,6 @@ struct ttm_buffer_object {
> enum ttm_bo_type type;
> void (*destroy) (struct ttm_buffer_object *);
> unsigned long num_pages;
> - uint64_t addr_space_offset;
> size_t acc_size;
>
> /**
> @@ -238,13 +235,7 @@ struct ttm_buffer_object {
> void *sync_obj;
> unsigned long priv_flags;
>
> - /**
> - * Members protected by the bdev::vm_lock
> - */
> -
> - struct rb_node vm_rb;
> - struct drm_mm_node *vm_node;
> -
> + struct drm_vma_offset_node vma_node;
>
> /**
> * Special members that are protected by the reserve lock
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 984fc2d..8639c85 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -36,6 +36,7 @@
> #include <ttm/ttm_placement.h>
> #include <drm/drm_mm.h>
> #include <drm/drm_global.h>
> +#include <drm/drm_vma_manager.h>
> #include <linux/workqueue.h>
> #include <linux/fs.h>
> #include <linux/spinlock.h>
> @@ -519,7 +520,7 @@ struct ttm_bo_global {
> * @man: An array of mem_type_managers.
> * @fence_lock: Protects the synchronizing members on *all* bos belonging
> * to this device.
> - * @addr_space_mm: Range manager for the device address space.
> + * @vma_manager: Address space manager
> * lru_lock: Spinlock that protects the buffer+device lru lists and
> * ddestroy lists.
> * @val_seq: Current validation sequence.
> @@ -537,14 +538,13 @@ struct ttm_bo_device {
> struct list_head device_list;
> struct ttm_bo_global *glob;
> struct ttm_bo_driver *driver;
> - rwlock_t vm_lock;
> struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
> spinlock_t fence_lock;
> +
> /*
> - * Protected by the vm lock.
> + * Protected by internal locks.
> */
> - struct rb_root addr_space_rb;
> - struct drm_mm addr_space_mm;
> + struct drm_vma_offset_manager vma_manager;
>
> /*
> * Protected by the global:lru lock.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v5 3/4] drm/ttm: convert to unified vma offset manager
2013-07-23 12:47 ` [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
2013-07-24 15:57 ` Daniel Vetter
@ 2013-07-24 19:08 ` David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 19:08 UTC (permalink / raw)
To: dri-devel
Cc: Thomas Hellstrom, Martin Peres, Alex Deucher, Ben Skeggs, Dave Airlie
Use the new vma-manager infrastructure. This doesn't change any
implementation details as the vma-offset-manager is nearly copied 1-to-1
from TTM.
The vm_lock is moved into the offset manager so we can drop it from TTM.
During lookup, we use the vma locking helpers to take a reference to the
found object.
In all other scenarios, locking stays the same as before. We always
guarantee that drm_vma_offset_remove() is called only during destruction.
Hence, helpers like drm_vma_node_offset_addr() are always safe as long as
the node has a valid offset.
This also drops the addr_space_offset member as it is a copy of vm_start
in vma_node objects. Use the accessor functions instead.
v4:
- remove vm_lock
- use drm_vma_offset_lock_lookup() to protect lookup (instead of vm_lock)
Cc: Dave Airlie <airlied@redhat.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Martin Peres <martin.peres@labri.fr>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/ast/ast_main.c | 2 +-
drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.h | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.h | 5 +-
drivers/gpu/drm/ttm/ttm_bo.c | 89 ++++++-------------------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +-
drivers/gpu/drm/ttm/ttm_bo_vm.c | 81 +++++++++++-----------------
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +-
include/drm/ttm/ttm_bo_api.h | 15 ++----
include/drm/ttm/ttm_bo_driver.h | 10 ++--
14 files changed, 66 insertions(+), 155 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f60fd7b..c195dc2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -487,7 +487,7 @@ void ast_gem_free_object(struct drm_gem_object *obj)
static inline u64 ast_bo_mmap_offset(struct ast_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
ast_dumb_mmap_offset(struct drm_file *file,
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 35cbae8..3a7a0ef 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -294,7 +294,7 @@ void cirrus_gem_free_object(struct drm_gem_object *obj)
static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 9fa5685..1a75ea3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -349,7 +349,7 @@ void mgag200_gem_free_object(struct drm_gem_object *obj)
static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo)
{
- return bo->bo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->bo.vma_node);
}
int
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 708b2d1..7a8caa1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -697,7 +697,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv,
gem = drm_gem_object_lookup(dev, file_priv, handle);
if (gem) {
struct nouveau_bo *bo = gem->driver_private;
- *poffset = bo->bo.addr_space_offset;
+ *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node);
drm_gem_object_unreference_unlocked(gem);
return 0;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index e72d09c..86597eb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -226,7 +226,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem,
}
rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT;
- rep->map_handle = nvbo->bo.addr_space_offset;
+ rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node);
rep->tile_mode = nvbo->tile_mode;
rep->tile_flags = nvbo->tile_flags;
return 0;
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index ee7ad79..af10165 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -59,7 +59,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
{
- return bo->tbo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->tbo.vma_node);
}
static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index b443d67..1a648e1 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -87,7 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
for (i = 0 ; i < release->bo_count; ++i) {
QXL_INFO(qdev, "release %llx\n",
- release->bos[i]->tbo.addr_space_offset
+ drm_vma_node_offset_addr(&release->bos[i]->tbo.vma_node)
- DRM_FILE_OFFSET);
qxl_fence_remove_release(&release->bos[i]->fence, release->id);
qxl_bo_unref(&release->bos[i]);
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 49c82c4..209b111 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -113,13 +113,10 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
* @bo: radeon object for which we query the offset
*
* Returns mmap offset of the object.
- *
- * Note: addr_space_offset is constant after ttm bo init thus isn't protected
- * by any lock.
*/
static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo)
{
- return bo->tbo.addr_space_offset;
+ return drm_vma_node_offset_addr(&bo->tbo.vma_node);
}
extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb9dd67..3dc08b6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -615,13 +615,7 @@ static void ttm_bo_release(struct kref *kref)
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
- write_lock(&bdev->vm_lock);
- if (likely(bo->vm_node != NULL)) {
- rb_erase(&bo->vm_rb, &bdev->addr_space_rb);
- drm_mm_put_block(bo->vm_node);
- bo->vm_node = NULL;
- }
- write_unlock(&bdev->vm_lock);
+ drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node);
ttm_mem_io_lock(man, false);
ttm_mem_io_free_vm(bo);
ttm_mem_io_unlock(man);
@@ -1129,6 +1123,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
bo->resv = &bo->ttm_resv;
reservation_object_init(bo->resv);
atomic_inc(&bo->glob->bo_count);
+ drm_vma_node_reset(&bo->vma_node);
ret = ttm_bo_check_placement(bo, placement);
@@ -1424,10 +1419,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
TTM_DEBUG("Swap list was clean\n");
spin_unlock(&glob->lru_lock);
- BUG_ON(!drm_mm_clean(&bdev->addr_space_mm));
- write_lock(&bdev->vm_lock);
- drm_mm_takedown(&bdev->addr_space_mm);
- write_unlock(&bdev->vm_lock);
+ drm_vma_offset_manager_destroy(&bdev->vma_manager);
return ret;
}
@@ -1441,7 +1433,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
{
int ret = -EINVAL;
- rwlock_init(&bdev->vm_lock);
bdev->driver = driver;
memset(bdev->man, 0, sizeof(bdev->man));
@@ -1454,9 +1445,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
if (unlikely(ret != 0))
goto out_no_sys;
- bdev->addr_space_rb = RB_ROOT;
- drm_mm_init(&bdev->addr_space_mm, file_page_offset, 0x10000000);
-
+ drm_vma_offset_manager_init(&bdev->vma_manager, file_page_offset,
+ 0x10000000);
INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
INIT_LIST_HEAD(&bdev->ddestroy);
bdev->dev_mapping = NULL;
@@ -1498,12 +1488,17 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- loff_t offset = (loff_t) bo->addr_space_offset;
- loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
+ loff_t offset, holelen;
if (!bdev->dev_mapping)
return;
- unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
+
+ if (drm_vma_node_has_offset(&bo->vma_node)) {
+ offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
+ holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
+
+ unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
+ }
ttm_mem_io_free_vm(bo);
}
@@ -1520,31 +1515,6 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
EXPORT_SYMBOL(ttm_bo_unmap_virtual);
-static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
-{
- struct ttm_bo_device *bdev = bo->bdev;
- struct rb_node **cur = &bdev->addr_space_rb.rb_node;
- struct rb_node *parent = NULL;
- struct ttm_buffer_object *cur_bo;
- unsigned long offset = bo->vm_node->start;
- unsigned long cur_offset;
-
- while (*cur) {
- parent = *cur;
- cur_bo = rb_entry(parent, struct ttm_buffer_object, vm_rb);
- cur_offset = cur_bo->vm_node->start;
- if (offset < cur_offset)
- cur = &parent->rb_left;
- else if (offset > cur_offset)
- cur = &parent->rb_right;
- else
- BUG();
- }
-
- rb_link_node(&bo->vm_rb, parent, cur);
- rb_insert_color(&bo->vm_rb, &bdev->addr_space_rb);
-}
-
/**
* ttm_bo_setup_vm:
*
@@ -1559,38 +1529,9 @@ static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo)
static int ttm_bo_setup_vm(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- int ret;
-
-retry_pre_get:
- ret = drm_mm_pre_get(&bdev->addr_space_mm);
- if (unlikely(ret != 0))
- return ret;
-
- write_lock(&bdev->vm_lock);
- bo->vm_node = drm_mm_search_free(&bdev->addr_space_mm,
- bo->mem.num_pages, 0, 0);
-
- if (unlikely(bo->vm_node == NULL)) {
- ret = -ENOMEM;
- goto out_unlock;
- }
- bo->vm_node = drm_mm_get_block_atomic(bo->vm_node,
- bo->mem.num_pages, 0);
-
- if (unlikely(bo->vm_node == NULL)) {
- write_unlock(&bdev->vm_lock);
- goto retry_pre_get;
- }
-
- ttm_bo_vm_insert_rb(bo);
- write_unlock(&bdev->vm_lock);
- bo->addr_space_offset = ((uint64_t) bo->vm_node->start) << PAGE_SHIFT;
-
- return 0;
-out_unlock:
- write_unlock(&bdev->vm_lock);
- return ret;
+ return drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
+ bo->mem.num_pages);
}
int ttm_bo_wait(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 319cf41..7cc904d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -30,6 +30,7 @@
#include <drm/ttm/ttm_bo_driver.h>
#include <drm/ttm/ttm_placement.h>
+#include <drm/drm_vma_manager.h>
#include <linux/io.h>
#include <linux/highmem.h>
#include <linux/wait.h>
@@ -450,7 +451,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
INIT_LIST_HEAD(&fbo->lru);
INIT_LIST_HEAD(&fbo->swap);
INIT_LIST_HEAD(&fbo->io_reserve_lru);
- fbo->vm_node = NULL;
+ drm_vma_node_reset(&fbo->vma_node);
atomic_set(&fbo->cpu_writers, 0);
spin_lock(&bdev->fence_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 3df9f16..8c0e2c0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -33,6 +33,7 @@
#include <ttm/ttm_module.h>
#include <ttm/ttm_bo_driver.h>
#include <ttm/ttm_placement.h>
+#include <drm/drm_vma_manager.h>
#include <linux/mm.h>
#include <linux/rbtree.h>
#include <linux/module.h>
@@ -40,37 +41,6 @@
#define TTM_BO_VM_NUM_PREFAULT 16
-static struct ttm_buffer_object *ttm_bo_vm_lookup_rb(struct ttm_bo_device *bdev,
- unsigned long page_start,
- unsigned long num_pages)
-{
- struct rb_node *cur = bdev->addr_space_rb.rb_node;
- unsigned long cur_offset;
- struct ttm_buffer_object *bo;
- struct ttm_buffer_object *best_bo = NULL;
-
- while (likely(cur != NULL)) {
- bo = rb_entry(cur, struct ttm_buffer_object, vm_rb);
- cur_offset = bo->vm_node->start;
- if (page_start >= cur_offset) {
- cur = cur->rb_right;
- best_bo = bo;
- if (page_start == cur_offset)
- break;
- } else
- cur = cur->rb_left;
- }
-
- if (unlikely(best_bo == NULL))
- return NULL;
-
- if (unlikely((best_bo->vm_node->start + best_bo->num_pages) <
- (page_start + num_pages)))
- return NULL;
-
- return best_bo;
-}
-
static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
@@ -146,9 +116,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
}
page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
- bo->vm_node->start - vma->vm_pgoff;
+ drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
page_last = vma_pages(vma) +
- bo->vm_node->start - vma->vm_pgoff;
+ drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
if (unlikely(page_offset >= bo->num_pages)) {
retval = VM_FAULT_SIGBUS;
@@ -249,6 +219,30 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.close = ttm_bo_vm_close
};
+static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
+ unsigned long offset,
+ unsigned long pages)
+{
+ struct drm_vma_offset_node *node;
+ struct ttm_buffer_object *bo = NULL;
+
+ drm_vma_offset_lock_lookup(&bdev->vma_manager);
+
+ node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages);
+ if (likely(node)) {
+ bo = container_of(node, struct ttm_buffer_object, vma_node);
+ if (!kref_get_unless_zero(&bo->kref))
+ bo = NULL;
+ }
+
+ drm_vma_offset_unlock_lookup(&bdev->vma_manager);
+
+ if (!bo)
+ pr_err("Could not find buffer object to map\n");
+
+ return bo;
+}
+
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
{
@@ -256,17 +250,9 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_buffer_object *bo;
int ret;
- read_lock(&bdev->vm_lock);
- bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff,
- vma_pages(vma));
- if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref))
- bo = NULL;
- read_unlock(&bdev->vm_lock);
-
- if (unlikely(bo == NULL)) {
- pr_err("Could not find buffer object to map\n");
+ bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+ if (unlikely(!bo))
return -EINVAL;
- }
driver = bo->bdev->driver;
if (unlikely(!driver->verify_access)) {
@@ -324,12 +310,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
bool no_wait = false;
bool dummy;
- read_lock(&bdev->vm_lock);
- bo = ttm_bo_vm_lookup_rb(bdev, dev_offset, 1);
- if (likely(bo != NULL))
- ttm_bo_reference(bo);
- read_unlock(&bdev->vm_lock);
-
+ bo = ttm_bo_vm_lookup(bdev, dev_offset, 1);
if (unlikely(bo == NULL))
return -EFAULT;
@@ -343,7 +324,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
if (unlikely(ret != 0))
goto out_unref;
- kmap_offset = dev_offset - bo->vm_node->start;
+ kmap_offset = dev_offset - drm_vma_node_start(&bo->vma_node);
if (unlikely(kmap_offset >= bo->num_pages)) {
ret = -EFBIG;
goto out_unref;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 7953d1f..0e67cf4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -500,7 +500,7 @@ int vmw_dmabuf_alloc_ioctl(struct drm_device *dev, void *data,
goto out_no_dmabuf;
rep->handle = handle;
- rep->map_handle = dma_buf->base.addr_space_offset;
+ rep->map_handle = drm_vma_node_offset_addr(&dma_buf->base.vma_node);
rep->cur_gmr_id = handle;
rep->cur_gmr_offset = 0;
@@ -834,7 +834,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv,
if (ret != 0)
return -EINVAL;
- *offset = out_buf->base.addr_space_offset;
+ *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node);
vmw_dmabuf_unreference(&out_buf);
return 0;
}
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 8a6aa56..751eaff 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -32,12 +32,12 @@
#define _TTM_BO_API_H_
#include <drm/drm_hashtab.h>
+#include <drm/drm_vma_manager.h>
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/wait.h>
#include <linux/mutex.h>
#include <linux/mm.h>
-#include <linux/rbtree.h>
#include <linux/bitmap.h>
#include <linux/reservation.h>
@@ -145,7 +145,6 @@ struct ttm_tt;
* @type: The bo type.
* @destroy: Destruction function. If NULL, kfree is used.
* @num_pages: Actual number of pages.
- * @addr_space_offset: Address space offset.
* @acc_size: Accounted size for this object.
* @kref: Reference count of this buffer object. When this refcount reaches
* zero, the object is put on the delayed delete list.
@@ -166,8 +165,7 @@ struct ttm_tt;
* @swap: List head for swap LRU list.
* @sync_obj: Pointer to a synchronization object.
* @priv_flags: Flags describing buffer object internal state.
- * @vm_rb: Rb node for the vm rb tree.
- * @vm_node: Address space manager node.
+ * @vma_node: Address space manager node.
* @offset: The current GPU offset, which can have different meanings
* depending on the memory type. For SYSTEM type memory, it should be 0.
* @cur_placement: Hint of current placement.
@@ -194,7 +192,6 @@ struct ttm_buffer_object {
enum ttm_bo_type type;
void (*destroy) (struct ttm_buffer_object *);
unsigned long num_pages;
- uint64_t addr_space_offset;
size_t acc_size;
/**
@@ -238,13 +235,7 @@ struct ttm_buffer_object {
void *sync_obj;
unsigned long priv_flags;
- /**
- * Members protected by the bdev::vm_lock
- */
-
- struct rb_node vm_rb;
- struct drm_mm_node *vm_node;
-
+ struct drm_vma_offset_node vma_node;
/**
* Special members that are protected by the reserve lock
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 984fc2d..8639c85 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -36,6 +36,7 @@
#include <ttm/ttm_placement.h>
#include <drm/drm_mm.h>
#include <drm/drm_global.h>
+#include <drm/drm_vma_manager.h>
#include <linux/workqueue.h>
#include <linux/fs.h>
#include <linux/spinlock.h>
@@ -519,7 +520,7 @@ struct ttm_bo_global {
* @man: An array of mem_type_managers.
* @fence_lock: Protects the synchronizing members on *all* bos belonging
* to this device.
- * @addr_space_mm: Range manager for the device address space.
+ * @vma_manager: Address space manager
* lru_lock: Spinlock that protects the buffer+device lru lists and
* ddestroy lists.
* @val_seq: Current validation sequence.
@@ -537,14 +538,13 @@ struct ttm_bo_device {
struct list_head device_list;
struct ttm_bo_global *glob;
struct ttm_bo_driver *driver;
- rwlock_t vm_lock;
struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
spinlock_t fence_lock;
+
/*
- * Protected by the vm lock.
+ * Protected by internal locks.
*/
- struct rb_root addr_space_rb;
- struct drm_mm addr_space_mm;
+ struct drm_vma_offset_manager vma_manager;
/*
* Protected by the global:lru lock.
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper
2013-07-23 12:47 ` [PATCH v4 0/4] Unified VMA Offset Manager David Herrmann
` (2 preceding siblings ...)
2013-07-23 12:47 ` [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
@ 2013-07-23 12:47 ` David Herrmann
2013-07-24 15:58 ` Daniel Vetter
2013-07-24 19:10 ` [PATCH v5 " David Herrmann
3 siblings, 2 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-23 12:47 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, Thomas Hellstrom, Daniel Vetter
Instead of unmapping the nodes in TTM and GEM users manually, we provide
a generic wrapper which does the correct thing for all vma-nodes.
v2: remove bdev->dev_mapping test in ttm_bo_unmap_virtual_unlocked() as
ttm_mem_io_free_vm() does nothing in that case (io_reserved_vm is 0).
v4: Fix docbook comments
Cc: Dave Airlie <airlied@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 6 +-----
drivers/gpu/drm/ttm/ttm_bo.c | 11 +----------
include/drm/drm_vma_manager.h | 22 ++++++++++++++++++++++
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 53f81b3..8673a00 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1427,11 +1427,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->fault_mappable)
return;
- if (obj->base.dev->dev_mapping)
- unmap_mapping_range(obj->base.dev->dev_mapping,
- (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
- obj->base.size, 1);
-
+ drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->dev_mapping);
obj->fault_mappable = false;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3dc08b6..050edfa 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1488,17 +1488,8 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- loff_t offset, holelen;
- if (!bdev->dev_mapping)
- return;
-
- if (drm_vma_node_has_offset(&bo->vma_node)) {
- offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
- holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
-
- unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
- }
+ drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping);
ttm_mem_io_free_vm(bo);
}
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index ace2925..2772192 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -24,6 +24,7 @@
*/
#include <drm/drm_mm.h>
+#include <linux/mm.h>
#include <linux/module.h>
#include <linux/rbtree.h>
#include <linux/spinlock.h>
@@ -177,4 +178,25 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
return ((__u64)node->vm_node.start) << PAGE_SHIFT;
}
+/**
+ * drm_vma_node_unmap() - Unmap offset node
+ * @node: Offset node
+ * @file_mapping: Address space to unmap @node from
+ *
+ * Unmap all userspace mappings for a given offset node. The mappings must be
+ * associated with the @file_mapping address-space. If no offset exists or
+ * the address-space is invalid, nothing is done.
+ *
+ * This call is unlocked. The caller must guarantee that drm_vma_offset_remove()
+ * is not called on this node concurrently.
+ */
+static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
+ struct address_space *file_mapping)
+{
+ if (file_mapping && drm_vma_node_has_offset(node))
+ unmap_mapping_range(file_mapping,
+ drm_vma_node_offset_addr(node),
+ node->vm_pages << PAGE_SHIFT, 1);
+}
+
#endif /* __DRM_VMA_MANAGER_H__ */
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper
2013-07-23 12:47 ` [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
@ 2013-07-24 15:58 ` Daniel Vetter
2013-07-24 19:10 ` [PATCH v5 " David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-07-24 15:58 UTC (permalink / raw)
To: David Herrmann; +Cc: Dave Airlie, Thomas Hellstrom, dri-devel, Daniel Vetter
On Tue, Jul 23, 2013 at 02:47:16PM +0200, David Herrmann wrote:
> Instead of unmapping the nodes in TTM and GEM users manually, we provide
> a generic wrapper which does the correct thing for all vma-nodes.
>
> v2: remove bdev->dev_mapping test in ttm_bo_unmap_virtual_unlocked() as
> ttm_mem_io_free_vm() does nothing in that case (io_reserved_vm is 0).
> v4: Fix docbook comments
>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 6 +-----
> drivers/gpu/drm/ttm/ttm_bo.c | 11 +----------
> include/drm/drm_vma_manager.h | 22 ++++++++++++++++++++++
> 3 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 53f81b3..8673a00 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1427,11 +1427,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
> if (!obj->fault_mappable)
> return;
>
> - if (obj->base.dev->dev_mapping)
> - unmap_mapping_range(obj->base.dev->dev_mapping,
> - (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
> - obj->base.size, 1);
> -
> + drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->dev_mapping);
> obj->fault_mappable = false;
> }
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3dc08b6..050edfa 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1488,17 +1488,8 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
> {
> struct ttm_bo_device *bdev = bo->bdev;
> - loff_t offset, holelen;
>
> - if (!bdev->dev_mapping)
> - return;
> -
> - if (drm_vma_node_has_offset(&bo->vma_node)) {
> - offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
> - holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
> -
> - unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
> - }
> + drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping);
> ttm_mem_io_free_vm(bo);
> }
>
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> index ace2925..2772192 100644
> --- a/include/drm/drm_vma_manager.h
> +++ b/include/drm/drm_vma_manager.h
> @@ -24,6 +24,7 @@
> */
>
> #include <drm/drm_mm.h>
> +#include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/rbtree.h>
> #include <linux/spinlock.h>
> @@ -177,4 +178,25 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
> return ((__u64)node->vm_node.start) << PAGE_SHIFT;
> }
>
> +/**
> + * drm_vma_node_unmap() - Unmap offset node
> + * @node: Offset node
> + * @file_mapping: Address space to unmap @node from
> + *
> + * Unmap all userspace mappings for a given offset node. The mappings must be
> + * associated with the @file_mapping address-space. If no offset exists or
> + * the address-space is invalid, nothing is done.
> + *
> + * This call is unlocked. The caller must guarantee that drm_vma_offset_remove()
> + * is not called on this node concurrently.
> + */
> +static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
> + struct address_space *file_mapping)
> +{
> + if (file_mapping && drm_vma_node_has_offset(node))
> + unmap_mapping_range(file_mapping,
> + drm_vma_node_offset_addr(node),
> + node->vm_pages << PAGE_SHIFT, 1);
> +}
> +
> #endif /* __DRM_VMA_MANAGER_H__ */
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v5 4/4] drm/vma: provide drm_vma_node_unmap() helper
2013-07-23 12:47 ` [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
2013-07-24 15:58 ` Daniel Vetter
@ 2013-07-24 19:10 ` David Herrmann
1 sibling, 0 replies; 31+ messages in thread
From: David Herrmann @ 2013-07-24 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, Thomas Hellstrom
Instead of unmapping the nodes in TTM and GEM users manually, we provide
a generic wrapper which does the correct thing for all vma-nodes.
v2: remove bdev->dev_mapping test in ttm_bo_unmap_virtual_unlocked() as
ttm_mem_io_free_vm() does nothing in that case (io_reserved_vm is 0).
v4: Fix docbook comments
v5: use drm_vma_node_size()
Cc: Dave Airlie <airlied@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 6 +-----
drivers/gpu/drm/ttm/ttm_bo.c | 11 +----------
include/drm/drm_vma_manager.h | 22 ++++++++++++++++++++++
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 53f81b3..8673a00 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1427,11 +1427,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->fault_mappable)
return;
- if (obj->base.dev->dev_mapping)
- unmap_mapping_range(obj->base.dev->dev_mapping,
- (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
- obj->base.size, 1);
-
+ drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->dev_mapping);
obj->fault_mappable = false;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3dc08b6..050edfa 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1488,17 +1488,8 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
{
struct ttm_bo_device *bdev = bo->bdev;
- loff_t offset, holelen;
- if (!bdev->dev_mapping)
- return;
-
- if (drm_vma_node_has_offset(&bo->vma_node)) {
- offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node);
- holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
-
- unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
- }
+ drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping);
ttm_mem_io_free_vm(bo);
}
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 7ee8c4b..22eedac 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -24,6 +24,7 @@
*/
#include <drm/drm_mm.h>
+#include <linux/mm.h>
#include <linux/module.h>
#include <linux/rbtree.h>
#include <linux/spinlock.h>
@@ -199,4 +200,25 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
return ((__u64)node->vm_node.start) << PAGE_SHIFT;
}
+/**
+ * drm_vma_node_unmap() - Unmap offset node
+ * @node: Offset node
+ * @file_mapping: Address space to unmap @node from
+ *
+ * Unmap all userspace mappings for a given offset node. The mappings must be
+ * associated with the @file_mapping address-space. If no offset exists or
+ * the address-space is invalid, nothing is done.
+ *
+ * This call is unlocked. The caller must guarantee that drm_vma_offset_remove()
+ * is not called on this node concurrently.
+ */
+static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
+ struct address_space *file_mapping)
+{
+ if (file_mapping && drm_vma_node_has_offset(node))
+ unmap_mapping_range(file_mapping,
+ drm_vma_node_offset_addr(node),
+ drm_vma_node_size(node) << PAGE_SHIFT, 1);
+}
+
#endif /* __DRM_VMA_MANAGER_H__ */
--
1.8.3.3
^ permalink raw reply related [flat|nested] 31+ messages in thread