All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm: Update GEM refcounting docs
@ 2015-10-22 17:11 Daniel Vetter
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-10-22 17:11 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

I just realized that I've forgotten to update all the gem refcounting
docs. For pennance also add pretty docs for the overall drm_gem_object
structure, with a few links thrown in fore good.

As usually we need to make sure the kerneldoc reference is at most a
sect2 for otherwise it won't be listed.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl |  15 +++---
 include/drm/drm_gem.h          | 106 +++++++++++++++++++++++++++++++++++------
 2 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 90c2aab31269..6c5865bb5ee8 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -635,10 +635,10 @@ char *date;</synopsis>
           acquired and release by <function>calling drm_gem_object_reference</function>
           and <function>drm_gem_object_unreference</function> respectively. The
           caller must hold the <structname>drm_device</structname>
-          <structfield>struct_mutex</structfield> lock. As a convenience, GEM
-          provides the <function>drm_gem_object_reference_unlocked</function> and
-          <function>drm_gem_object_unreference_unlocked</function> functions that
-          can be called without holding the lock.
+	  <structfield>struct_mutex</structfield> lock when calling
+	  <function>drm_gem_object_reference</function>. As a convenience, GEM
+	  provides <function>drm_gem_object_unreference_unlocked</function>
+	  functions that can be called without holding the lock.
         </para>
         <para>
           When the last reference to a GEM object is released the GEM core calls
@@ -836,10 +836,11 @@ char *date;</synopsis>
           abstracted from the client in libdrm.
         </para>
       </sect3>
-      <sect3>
-        <title>GEM Function Reference</title>
+    </sect2>
+    <sect2>
+      <title>GEM Function Reference</title>
 !Edrivers/gpu/drm/drm_gem.c
-      </sect3>
+!Iinclude/drm/drm_gem.h
     </sect2>
     <sect2>
       <title>VMA Offset Manager</title>
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 15e7f007380f..0b3e11ab8757 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -35,76 +35,129 @@
  */
 
 /**
- * This structure defines the drm_mm memory object, which will be used by the
- * DRM for its buffer objects.
+ * struct drm_gem_object - GEM buffer object
+ *
+ * This structure defines the generic parts for GEM buffer objects, which are
+ * mostly around handling mmap and userspace handles.
+ *
+ * Buffer objects are often abbreviated to BO.
  */
 struct drm_gem_object {
-	/** Reference count of this object */
+	/**
+	 * @refcount:
+	 *
+	 * Reference count of this object
+	 *
+	 * Please use drm_gem_object_reference() to acquire and
+	 * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked()
+	 * to release a reference to a GEM buffer object.
+	 */
 	struct kref refcount;
 
 	/**
-	 * handle_count - gem file_priv handle count of this object
+	 * @handle_count:
+	 *
+	 * This is the GEM file_priv handle count of this object.
 	 *
 	 * Each handle also holds a reference. Note that when the handle_count
 	 * drops to 0 any global names (e.g. the id in the flink namespace) will
 	 * be cleared.
 	 *
 	 * Protected by dev->object_name_lock.
-	 * */
+	 */
 	unsigned handle_count;
 
-	/** Related drm device */
+	/**
+	 * @dev: DRM dev this object belongs to.
+	 */
 	struct drm_device *dev;
 
-	/** File representing the shmem storage */
+	/**
+	 * @filp:
+	 *
+	 * SHMEM file node used as backing storage for swappable buffer objects.
+	 * GEM also supports driver private objects with driver-specific backing
+	 * storage (contiguous CMA memory, special reserved blocks). In this
+	 * case @filp is NULL.
+	 */
 	struct file *filp;
 
-	/* Mapping info for this object */
+	/**
+	 * @vma_node:
+	 *
+	 * Mapping info for this object to support mmap. Drivers are supposed to
+	 * allocate the mmap offset using drm_gem_create_mmap_offset(). The
+	 * offset itself can be retrieved using drm_vma_node_offset_addr().
+	 *
+	 * Memory mapping itself is handled by drm_gem_mmap(), which also checks
+	 * that userspace is allowed to access the object.
+	 */
 	struct drm_vma_offset_node vma_node;
 
 	/**
+	 * @size:
+	 *
 	 * Size of the object, in bytes.  Immutable over the object's
 	 * lifetime.
 	 */
 	size_t size;
 
 	/**
+	 * @name:
+	 *
 	 * Global name for this object, starts at 1. 0 means unnamed.
-	 * Access is covered by the object_name_lock in the related drm_device
+	 * Access is covered by dev->object_name_lock. This is used by the GEM_FLINK
+	 * and GEM_OPEN ioctls.
 	 */
 	int name;
 
 	/**
-	 * Memory domains. These monitor which caches contain read/write data
+	 * @read_domains:
+	 *
+	 * Read memory domains. These monitor which caches contain read/write data
 	 * related to the object. When transitioning from one set of domains
 	 * to another, the driver is called to ensure that caches are suitably
-	 * flushed and invalidated
+	 * flushed and invalidated.
 	 */
 	uint32_t read_domains;
+
+	/**
+	 * @write_domain: Corresponding unique write memory domain.
+	 */
 	uint32_t write_domain;
 
 	/**
+	 * @pending_read_domains:
+	 *
 	 * While validating an exec operation, the
 	 * new read/write domain values are computed here.
 	 * They will be transferred to the above values
 	 * at the point that any cache flushing occurs
 	 */
 	uint32_t pending_read_domains;
+
+	/**
+	 * @pending_write_domain: Write domain similar to @pending_read_domains.
+	 */
 	uint32_t pending_write_domain;
 
 	/**
-	 * dma_buf - dma buf associated with this GEM object
+	 * @dma_buf:
+	 *
+	 * dma-buf associated with this GEM object.
 	 *
 	 * Pointer to the dma-buf associated with this gem object (either
 	 * through importing or exporting). We break the resulting reference
 	 * loop when the last gem handle for this object is released.
 	 *
-	 * Protected by obj->object_name_lock
+	 * Protected by obj->object_name_lock.
 	 */
 	struct dma_buf *dma_buf;
 
 	/**
-	 * import_attach - dma buf attachment backing this object
+	 * @import_attach:
+	 *
+	 * dma-buf attachment backing this object.
 	 *
 	 * Any foreign dma_buf imported as a gem object has this set to the
 	 * attachment point for the device. This is invariant over the lifetime
@@ -133,12 +186,30 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 		     struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+/**
+ * drm_gem_object_reference - acquire a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This acquires additional reference to @obj. It is illegal to call this
+ * without already holding a reference. No locks required.
+ */
 static inline void
 drm_gem_object_reference(struct drm_gem_object *obj)
 {
 	kref_get(&obj->refcount);
 }
 
+/**
+ * drm_gem_object_unreference - release a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This releases a reference to @obj. Callers must hold the dev->struct_mutex
+ * lock when calling this function, even when the driver doesn't use
+ * dev->struct_mutex for anything.
+ *
+ * For drivers not encumbered with legacy locking use
+ * drm_gem_object_unreference_unlocked() instead.
+ */
 static inline void
 drm_gem_object_unreference(struct drm_gem_object *obj)
 {
@@ -149,6 +220,13 @@ drm_gem_object_unreference(struct drm_gem_object *obj)
 	}
 }
 
+/**
+ * drm_gem_object_unreference_unlocked - release a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This releases a reference to @obj. Callers must not hold the
+ * dev->struct_mutex lock when calling this function.
+ */
 static inline void
 drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
 {
-- 
2.5.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] drm/vma_manage: Drop has_offset
  2015-10-22 17:11 [PATCH 1/3] drm: Update GEM refcounting docs Daniel Vetter
@ 2015-10-22 17:11 ` Daniel Vetter
  2015-10-22 17:41   ` kbuild test robot
                     ` (2 more replies)
  2015-10-22 17:11 ` [PATCH 3/3] drm/gem: Update/Polish docs Daniel Vetter
  2015-10-22 17:54 ` [PATCH 1/3] drm: Update GEM refcounting docs Alex Deucher
  2 siblings, 3 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-10-22 17:11 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development, David Herrmann

It's racy, creating mmap offsets is a slowpath, so better to remove it
to avoid drivers doing broken things.

The only user is i915, and it's ok there because everything (well
almost) is protected by dev->struct_mutex in i915-gem.

While at it add a note in the create_mmap_offset kerneldoc that
drivers must release it again. And then I also noticed that
drm_gem_object_release entirely lacks kerneldoc.

Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_gem.c       | 17 +++++++++++++++++
 drivers/gpu/drm/i915/i915_gem.c |  3 ---
 include/drm/drm_vma_manager.h   | 15 +--------------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 64353d40db53..38680380c6b3 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -387,6 +387,10 @@ EXPORT_SYMBOL(drm_gem_handle_create);
  * @obj: obj in question
  *
  * This routine frees fake offsets allocated by drm_gem_create_mmap_offset().
+ *
+ * Note that drm_gem_object_release() already calls this function, so drivers
+ * don't have to take care of releasing the mmap offset themselves when freeing
+ * the GEM object.
  */
 void
 drm_gem_free_mmap_offset(struct drm_gem_object *obj)
@@ -410,6 +414,9 @@ EXPORT_SYMBOL(drm_gem_free_mmap_offset);
  * This routine allocates and attaches a fake offset for @obj, in cases where
  * the virtual size differs from the physical size (ie. obj->size).  Otherwise
  * just use drm_gem_create_mmap_offset().
+ *
+ * This function is idempotent and handles an already allocated mmap offset
+ * transparently. Drivers do not need to check for this case.
  */
 int
 drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
@@ -431,6 +438,9 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset_size);
  * structures.
  *
  * This routine allocates and attaches a fake offset for @obj.
+ *
+ * Drivers can call drm_gem_free_mmap_offset() before freeing @obj to release
+ * the fake offset again.
  */
 int drm_gem_create_mmap_offset(struct drm_gem_object *obj)
 {
@@ -739,6 +749,13 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private)
 	idr_destroy(&file_private->object_idr);
 }
 
+/**
+ * drm_gem_object_release - release GEM buffer object resources
+ * @obj: GEM buffer object
+ *
+ * This releases any structures and resources used by @obj and is the invers of
+ * drm_gem_object_init().
+ */
 void
 drm_gem_object_release(struct drm_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d0fa5481543c..01fef54ecb2d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1972,9 +1972,6 @@ 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 (drm_vma_node_has_offset(&obj->base.vma_node))
-		return 0;
-
 	dev_priv->mm.shrinker_no_lock_stealing = true;
 
 	ret = drm_gem_create_mmap_offset(&obj->base);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 2f63dd5e05eb..06ea8e077ec2 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -176,19 +176,6 @@ static inline unsigned long drm_vma_node_size(struct drm_vma_offset_node *node)
 }
 
 /**
- * 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
  *
@@ -220,7 +207,7 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
 static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
 				      struct address_space *file_mapping)
 {
-	if (drm_vma_node_has_offset(node))
+	if (drm_mm_node_allocated(&node->vm_node))
 		unmap_mapping_range(file_mapping,
 				    drm_vma_node_offset_addr(node),
 				    drm_vma_node_size(node) << PAGE_SHIFT, 1);
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] drm/gem: Update/Polish docs
  2015-10-22 17:11 [PATCH 1/3] drm: Update GEM refcounting docs Daniel Vetter
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
@ 2015-10-22 17:11 ` Daniel Vetter
  2015-10-22 17:54 ` [PATCH 1/3] drm: Update GEM refcounting docs Alex Deucher
  2 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-10-22 17:11 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

A bunch of things have been removed meanwhile and docs not fully
brought up to speed. And a few gaps closed where I noticed missing
kerneldoc while reading through the overview sections.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl | 33 ++++-----------------------------
 drivers/gpu/drm/drm_gem.c      | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 6c5865bb5ee8..e6735d2b6ae8 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -615,18 +615,6 @@ char *date;</synopsis>
           <function>drm_gem_object_init</function>. Storage for private GEM
           objects must be managed by drivers.
         </para>
-        <para>
-          Drivers that do not need to extend GEM objects with private information
-          can call the <function>drm_gem_object_alloc</function> function to
-          allocate and initialize a struct <structname>drm_gem_object</structname>
-          instance. The GEM core will call the optional driver
-          <methodname>gem_init_object</methodname> operation after initializing
-          the GEM object with <function>drm_gem_object_init</function>.
-          <synopsis>int (*gem_init_object) (struct drm_gem_object *obj);</synopsis>
-        </para>
-        <para>
-          No alloc-and-init function exists for private GEM objects.
-        </para>
       </sect3>
       <sect3>
         <title>GEM Objects Lifetime</title>
@@ -649,15 +637,9 @@ char *date;</synopsis>
         </para>
         <para>
           <synopsis>void (*gem_free_object) (struct drm_gem_object *obj);</synopsis>
-          Drivers are responsible for freeing all GEM object resources, including
-          the resources created by the GEM core. If an mmap offset has been
-          created for the object (in which case
-          <structname>drm_gem_object</structname>::<structfield>map_list</structfield>::<structfield>map</structfield>
-          is not NULL) it must be freed by a call to
-          <function>drm_gem_free_mmap_offset</function>. The shmfs backing store
-          must be released by calling <function>drm_gem_object_release</function>
-          (that function can safely be called if no shmfs backing store has been
-          created).
+          Drivers are responsible for freeing all GEM object resources. This includes
+          the resources created by the GEM core, which need to be released with
+          <function>drm_gem_object_release</function>.
         </para>
       </sect3>
       <sect3>
@@ -740,17 +722,10 @@ char *date;</synopsis>
           DRM identifies the GEM object to be mapped by a fake offset passed
           through the mmap offset argument. Prior to being mapped, a GEM object
           must thus be associated with a fake offset. To do so, drivers must call
-          <function>drm_gem_create_mmap_offset</function> on the object. The
-          function allocates a fake offset range from a pool and stores the
-          offset divided by PAGE_SIZE in
-          <literal>obj-&gt;map_list.hash.key</literal>. Care must be taken not to
-          call <function>drm_gem_create_mmap_offset</function> if a fake offset
-          has already been allocated for the object. This can be tested by
-          <literal>obj-&gt;map_list.map</literal> being non-NULL.
+          <function>drm_gem_create_mmap_offset</function> on the object.
         </para>
         <para>
           Once allocated, the fake offset value
-          (<literal>obj-&gt;map_list.hash.key &lt;&lt; PAGE_SHIFT</literal>)
           must be passed to the application in a driver-specific way and can then
           be used as the mmap offset argument.
         </para>
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 38680380c6b3..c55446a789c7 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -244,8 +244,9 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
  * @filp: drm file-private structure to use for the handle look up
  * @handle: userspace handle to delete
  *
- * Removes the GEM handle from the @filp lookup table and if this is the last
- * handle also cleans up linked resources like GEM names.
+ * Removes the GEM handle from the @filp lookup table which has been added with
+ * drm_gem_handle_create(). If this is the last handle also cleans up linked
+ * resources like GEM names.
  */
 int
 drm_gem_handle_delete(struct drm_file *filp, u32 handle)
@@ -314,6 +315,10 @@ EXPORT_SYMBOL(drm_gem_dumb_destroy);
  * This expects the dev->object_name_lock to be held already and will drop it
  * before returning. Used to avoid races in establishing new handles when
  * importing an object from either an flink name or a dma-buf.
+ *
+ * Handles must be release again through drm_gem_handle_delete(). This is done
+ * when userspace closes @file_priv for all attached handles, or through the
+ * GEM_CLOSE ioctl for individual handles.
  */
 int
 drm_gem_handle_create_tail(struct drm_file *file_priv,
@@ -551,7 +556,17 @@ void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages,
 }
 EXPORT_SYMBOL(drm_gem_put_pages);
 
-/** Returns a reference to the object named by the handle. */
+/**
+ * drm_gem_object_lookup - look up a GEM object from it's handle
+ * @dev: DRM device
+ * @filp: DRM file private date
+ * @handle: userspace handle
+ *
+ * Returns:
+ *
+ * A reference to the object named by the handle if such exists on @filp, NULL
+ * otherwise.
+ */
 struct drm_gem_object *
 drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp,
 		      u32 handle)
@@ -791,6 +806,13 @@ drm_gem_object_free(struct kref *kref)
 }
 EXPORT_SYMBOL(drm_gem_object_free);
 
+/**
+ * drm_gem_vm_open - vma->ops->open implementation for GEM
+ * @vma: VM area structure
+ *
+ * This function implements the #vm_operations_struct open() callback for GEM
+ * drivers. This must be used together with drm_gem_vm_close().
+ */
 void drm_gem_vm_open(struct vm_area_struct *vma)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
@@ -799,6 +821,13 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
 }
 EXPORT_SYMBOL(drm_gem_vm_open);
 
+/**
+ * drm_gem_vm_close - vma->ops->close implementation for GEM
+ * @vma: VM area structure
+ *
+ * This function implements the #vm_operations_struct close() callback for GEM
+ * drivers. This must be used together with drm_gem_vm_open().
+ */
 void drm_gem_vm_close(struct vm_area_struct *vma)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] drm/vma_manage: Drop has_offset
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
@ 2015-10-22 17:41   ` kbuild test robot
  2015-10-22 18:47   ` [Intel-gfx] " kbuild test robot
  2015-10-23  9:33   ` David Herrmann
  2 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2015-10-22 17:41 UTC (permalink / raw)
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	kbuild-all, David Herrmann, Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

Hi Daniel,

[auto build test ERROR on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317
config: i386-randconfig-s1-201542 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map':
>> drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration]
     if (!drm_vma_node_has_offset(&obj->vma_node)) {
          ^
   cc1: some warnings being treated as errors

vim +/drm_vma_node_has_offset +211 drivers/gpu/drm/vgem/vgem_drv.c

502e95c6 Zach Reizner 2015-03-04  205  	obj = drm_gem_object_lookup(dev, file, handle);
502e95c6 Zach Reizner 2015-03-04  206  	if (!obj) {
502e95c6 Zach Reizner 2015-03-04  207  		ret = -ENOENT;
502e95c6 Zach Reizner 2015-03-04  208  		goto unlock;
502e95c6 Zach Reizner 2015-03-04  209  	}
502e95c6 Zach Reizner 2015-03-04  210  
502e95c6 Zach Reizner 2015-03-04 @211  	if (!drm_vma_node_has_offset(&obj->vma_node)) {
502e95c6 Zach Reizner 2015-03-04  212  		ret = drm_gem_create_mmap_offset(obj);
502e95c6 Zach Reizner 2015-03-04  213  		if (ret)
502e95c6 Zach Reizner 2015-03-04  214  			goto unref;

:::::: The code at line 211 was first introduced by commit
:::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM

:::::: TO: Zach Reizner <zachr@google.com>
:::::: CC: Dave Airlie <airlied@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23181 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm: Update GEM refcounting docs
  2015-10-22 17:11 [PATCH 1/3] drm: Update GEM refcounting docs Daniel Vetter
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
  2015-10-22 17:11 ` [PATCH 3/3] drm/gem: Update/Polish docs Daniel Vetter
@ 2015-10-22 17:54 ` Alex Deucher
  2015-10-22 19:46   ` Daniel Vetter
  2 siblings, 1 reply; 9+ messages in thread
From: Alex Deucher @ 2015-10-22 17:54 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development

On Thu, Oct 22, 2015 at 1:11 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> I just realized that I've forgotten to update all the gem refcounting
> docs. For pennance also add pretty docs for the overall drm_gem_object
> structure, with a few links thrown in fore good.
>
> As usually we need to make sure the kerneldoc reference is at most a
> sect2 for otherwise it won't be listed.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Patches 1 and 3 are:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/DocBook/gpu.tmpl |  15 +++---
>  include/drm/drm_gem.h          | 106 +++++++++++++++++++++++++++++++++++------
>  2 files changed, 100 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 90c2aab31269..6c5865bb5ee8 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -635,10 +635,10 @@ char *date;</synopsis>
>            acquired and release by <function>calling drm_gem_object_reference</function>
>            and <function>drm_gem_object_unreference</function> respectively. The
>            caller must hold the <structname>drm_device</structname>
> -          <structfield>struct_mutex</structfield> lock. As a convenience, GEM
> -          provides the <function>drm_gem_object_reference_unlocked</function> and
> -          <function>drm_gem_object_unreference_unlocked</function> functions that
> -          can be called without holding the lock.
> +         <structfield>struct_mutex</structfield> lock when calling
> +         <function>drm_gem_object_reference</function>. As a convenience, GEM
> +         provides <function>drm_gem_object_unreference_unlocked</function>
> +         functions that can be called without holding the lock.
>          </para>
>          <para>
>            When the last reference to a GEM object is released the GEM core calls
> @@ -836,10 +836,11 @@ char *date;</synopsis>
>            abstracted from the client in libdrm.
>          </para>
>        </sect3>
> -      <sect3>
> -        <title>GEM Function Reference</title>
> +    </sect2>
> +    <sect2>
> +      <title>GEM Function Reference</title>
>  !Edrivers/gpu/drm/drm_gem.c
> -      </sect3>
> +!Iinclude/drm/drm_gem.h
>      </sect2>
>      <sect2>
>        <title>VMA Offset Manager</title>
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 15e7f007380f..0b3e11ab8757 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -35,76 +35,129 @@
>   */
>
>  /**
> - * This structure defines the drm_mm memory object, which will be used by the
> - * DRM for its buffer objects.
> + * struct drm_gem_object - GEM buffer object
> + *
> + * This structure defines the generic parts for GEM buffer objects, which are
> + * mostly around handling mmap and userspace handles.
> + *
> + * Buffer objects are often abbreviated to BO.
>   */
>  struct drm_gem_object {
> -       /** Reference count of this object */
> +       /**
> +        * @refcount:
> +        *
> +        * Reference count of this object
> +        *
> +        * Please use drm_gem_object_reference() to acquire and
> +        * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked()
> +        * to release a reference to a GEM buffer object.
> +        */
>         struct kref refcount;
>
>         /**
> -        * handle_count - gem file_priv handle count of this object
> +        * @handle_count:
> +        *
> +        * This is the GEM file_priv handle count of this object.
>          *
>          * Each handle also holds a reference. Note that when the handle_count
>          * drops to 0 any global names (e.g. the id in the flink namespace) will
>          * be cleared.
>          *
>          * Protected by dev->object_name_lock.
> -        * */
> +        */
>         unsigned handle_count;
>
> -       /** Related drm device */
> +       /**
> +        * @dev: DRM dev this object belongs to.
> +        */
>         struct drm_device *dev;
>
> -       /** File representing the shmem storage */
> +       /**
> +        * @filp:
> +        *
> +        * SHMEM file node used as backing storage for swappable buffer objects.
> +        * GEM also supports driver private objects with driver-specific backing
> +        * storage (contiguous CMA memory, special reserved blocks). In this
> +        * case @filp is NULL.
> +        */
>         struct file *filp;
>
> -       /* Mapping info for this object */
> +       /**
> +        * @vma_node:
> +        *
> +        * Mapping info for this object to support mmap. Drivers are supposed to
> +        * allocate the mmap offset using drm_gem_create_mmap_offset(). The
> +        * offset itself can be retrieved using drm_vma_node_offset_addr().
> +        *
> +        * Memory mapping itself is handled by drm_gem_mmap(), which also checks
> +        * that userspace is allowed to access the object.
> +        */
>         struct drm_vma_offset_node vma_node;
>
>         /**
> +        * @size:
> +        *
>          * Size of the object, in bytes.  Immutable over the object's
>          * lifetime.
>          */
>         size_t size;
>
>         /**
> +        * @name:
> +        *
>          * Global name for this object, starts at 1. 0 means unnamed.
> -        * Access is covered by the object_name_lock in the related drm_device
> +        * Access is covered by dev->object_name_lock. This is used by the GEM_FLINK
> +        * and GEM_OPEN ioctls.
>          */
>         int name;
>
>         /**
> -        * Memory domains. These monitor which caches contain read/write data
> +        * @read_domains:
> +        *
> +        * Read memory domains. These monitor which caches contain read/write data
>          * related to the object. When transitioning from one set of domains
>          * to another, the driver is called to ensure that caches are suitably
> -        * flushed and invalidated
> +        * flushed and invalidated.
>          */
>         uint32_t read_domains;
> +
> +       /**
> +        * @write_domain: Corresponding unique write memory domain.
> +        */
>         uint32_t write_domain;
>
>         /**
> +        * @pending_read_domains:
> +        *
>          * While validating an exec operation, the
>          * new read/write domain values are computed here.
>          * They will be transferred to the above values
>          * at the point that any cache flushing occurs
>          */
>         uint32_t pending_read_domains;
> +
> +       /**
> +        * @pending_write_domain: Write domain similar to @pending_read_domains.
> +        */
>         uint32_t pending_write_domain;
>
>         /**
> -        * dma_buf - dma buf associated with this GEM object
> +        * @dma_buf:
> +        *
> +        * dma-buf associated with this GEM object.
>          *
>          * Pointer to the dma-buf associated with this gem object (either
>          * through importing or exporting). We break the resulting reference
>          * loop when the last gem handle for this object is released.
>          *
> -        * Protected by obj->object_name_lock
> +        * Protected by obj->object_name_lock.
>          */
>         struct dma_buf *dma_buf;
>
>         /**
> -        * import_attach - dma buf attachment backing this object
> +        * @import_attach:
> +        *
> +        * dma-buf attachment backing this object.
>          *
>          * Any foreign dma_buf imported as a gem object has this set to the
>          * attachment point for the device. This is invariant over the lifetime
> @@ -133,12 +186,30 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>                      struct vm_area_struct *vma);
>  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>
> +/**
> + * drm_gem_object_reference - acquire a GEM BO reference
> + * @obj: GEM buffer object
> + *
> + * This acquires additional reference to @obj. It is illegal to call this
> + * without already holding a reference. No locks required.
> + */
>  static inline void
>  drm_gem_object_reference(struct drm_gem_object *obj)
>  {
>         kref_get(&obj->refcount);
>  }
>
> +/**
> + * drm_gem_object_unreference - release a GEM BO reference
> + * @obj: GEM buffer object
> + *
> + * This releases a reference to @obj. Callers must hold the dev->struct_mutex
> + * lock when calling this function, even when the driver doesn't use
> + * dev->struct_mutex for anything.
> + *
> + * For drivers not encumbered with legacy locking use
> + * drm_gem_object_unreference_unlocked() instead.
> + */
>  static inline void
>  drm_gem_object_unreference(struct drm_gem_object *obj)
>  {
> @@ -149,6 +220,13 @@ drm_gem_object_unreference(struct drm_gem_object *obj)
>         }
>  }
>
> +/**
> + * drm_gem_object_unreference_unlocked - release a GEM BO reference
> + * @obj: GEM buffer object
> + *
> + * This releases a reference to @obj. Callers must not hold the
> + * dev->struct_mutex lock when calling this function.
> + */
>  static inline void
>  drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
>  {
> --
> 2.5.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/vma_manage: Drop has_offset
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
  2015-10-22 17:41   ` kbuild test robot
@ 2015-10-22 18:47   ` kbuild test robot
  2015-10-22 19:44     ` Daniel Vetter
  2015-10-23  9:33   ` David Herrmann
  2 siblings, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2015-10-22 18:47 UTC (permalink / raw)
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	kbuild-all, Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 14411 bytes --]

Hi Daniel,

[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317
config: x86_64-randconfig-s1-10230205 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/linux/list.h:4,
                    from include/linux/module.h:9,
                    from drivers/gpu/drm/vgem/vgem_drv.c:33:
   drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map':
   drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration]
     if (!drm_vma_node_has_offset(&obj->vma_node)) {
          ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/gpu/drm/vgem/vgem_drv.c:211:2: note: in expansion of macro 'if'
     if (!drm_vma_node_has_offset(&obj->vma_node)) {
     ^
   cc1: some warnings being treated as errors

vim +/if +211 drivers/gpu/drm/vgem/vgem_drv.c

502e95c6 Zach Reizner       2015-03-04   27  
502e95c6 Zach Reizner       2015-03-04   28  /**
502e95c6 Zach Reizner       2015-03-04   29   * This is vgem, a (non-hardware-backed) GEM service.  This is used by Mesa's
502e95c6 Zach Reizner       2015-03-04   30   * software renderer and the X server for efficient buffer sharing.
502e95c6 Zach Reizner       2015-03-04   31   */
502e95c6 Zach Reizner       2015-03-04   32  
502e95c6 Zach Reizner       2015-03-04  @33  #include <linux/module.h>
502e95c6 Zach Reizner       2015-03-04   34  #include <linux/ramfs.h>
502e95c6 Zach Reizner       2015-03-04   35  #include <linux/shmem_fs.h>
502e95c6 Zach Reizner       2015-03-04   36  #include <linux/dma-buf.h>
502e95c6 Zach Reizner       2015-03-04   37  #include "vgem_drv.h"
502e95c6 Zach Reizner       2015-03-04   38  
502e95c6 Zach Reizner       2015-03-04   39  #define DRIVER_NAME	"vgem"
502e95c6 Zach Reizner       2015-03-04   40  #define DRIVER_DESC	"Virtual GEM provider"
502e95c6 Zach Reizner       2015-03-04   41  #define DRIVER_DATE	"20120112"
502e95c6 Zach Reizner       2015-03-04   42  #define DRIVER_MAJOR	1
502e95c6 Zach Reizner       2015-03-04   43  #define DRIVER_MINOR	0
502e95c6 Zach Reizner       2015-03-04   44  
502e95c6 Zach Reizner       2015-03-04   45  void vgem_gem_put_pages(struct drm_vgem_gem_object *obj)
502e95c6 Zach Reizner       2015-03-04   46  {
502e95c6 Zach Reizner       2015-03-04   47  	drm_gem_put_pages(&obj->base, obj->pages, false, false);
502e95c6 Zach Reizner       2015-03-04   48  	obj->pages = NULL;
502e95c6 Zach Reizner       2015-03-04   49  }
502e95c6 Zach Reizner       2015-03-04   50  
502e95c6 Zach Reizner       2015-03-04   51  static void vgem_gem_free_object(struct drm_gem_object *obj)
502e95c6 Zach Reizner       2015-03-04   52  {
502e95c6 Zach Reizner       2015-03-04   53  	struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
502e95c6 Zach Reizner       2015-03-04   54  
502e95c6 Zach Reizner       2015-03-04   55  	drm_gem_free_mmap_offset(obj);
502e95c6 Zach Reizner       2015-03-04   56  
502e95c6 Zach Reizner       2015-03-04   57  	if (vgem_obj->use_dma_buf && obj->dma_buf) {
502e95c6 Zach Reizner       2015-03-04   58  		dma_buf_put(obj->dma_buf);
502e95c6 Zach Reizner       2015-03-04   59  		obj->dma_buf = NULL;
502e95c6 Zach Reizner       2015-03-04   60  	}
502e95c6 Zach Reizner       2015-03-04   61  
502e95c6 Zach Reizner       2015-03-04   62  	drm_gem_object_release(obj);
502e95c6 Zach Reizner       2015-03-04   63  
502e95c6 Zach Reizner       2015-03-04   64  	if (vgem_obj->pages)
502e95c6 Zach Reizner       2015-03-04   65  		vgem_gem_put_pages(vgem_obj);
502e95c6 Zach Reizner       2015-03-04   66  
502e95c6 Zach Reizner       2015-03-04   67  	vgem_obj->pages = NULL;
502e95c6 Zach Reizner       2015-03-04   68  
502e95c6 Zach Reizner       2015-03-04   69  	kfree(vgem_obj);
502e95c6 Zach Reizner       2015-03-04   70  }
502e95c6 Zach Reizner       2015-03-04   71  
502e95c6 Zach Reizner       2015-03-04   72  int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
502e95c6 Zach Reizner       2015-03-04   73  {
502e95c6 Zach Reizner       2015-03-04   74  	struct page **pages;
502e95c6 Zach Reizner       2015-03-04   75  
502e95c6 Zach Reizner       2015-03-04   76  	if (obj->pages || obj->use_dma_buf)
502e95c6 Zach Reizner       2015-03-04   77  		return 0;
502e95c6 Zach Reizner       2015-03-04   78  
502e95c6 Zach Reizner       2015-03-04   79  	pages = drm_gem_get_pages(&obj->base);
502e95c6 Zach Reizner       2015-03-04   80  	if (IS_ERR(pages)) {
502e95c6 Zach Reizner       2015-03-04   81  		return PTR_ERR(pages);
502e95c6 Zach Reizner       2015-03-04   82  	}
502e95c6 Zach Reizner       2015-03-04   83  
502e95c6 Zach Reizner       2015-03-04   84  	obj->pages = pages;
502e95c6 Zach Reizner       2015-03-04   85  
502e95c6 Zach Reizner       2015-03-04   86  	return 0;
502e95c6 Zach Reizner       2015-03-04   87  }
502e95c6 Zach Reizner       2015-03-04   88  
502e95c6 Zach Reizner       2015-03-04   89  static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
502e95c6 Zach Reizner       2015-03-04   90  {
502e95c6 Zach Reizner       2015-03-04   91  	struct drm_vgem_gem_object *obj = vma->vm_private_data;
502e95c6 Zach Reizner       2015-03-04   92  	struct drm_device *dev = obj->base.dev;
502e95c6 Zach Reizner       2015-03-04   93  	loff_t num_pages;
502e95c6 Zach Reizner       2015-03-04   94  	pgoff_t page_offset;
502e95c6 Zach Reizner       2015-03-04   95  	int ret;
502e95c6 Zach Reizner       2015-03-04   96  
502e95c6 Zach Reizner       2015-03-04   97  	/* We don't use vmf->pgoff since that has the fake offset */
502e95c6 Zach Reizner       2015-03-04   98  	page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
502e95c6 Zach Reizner       2015-03-04   99  		PAGE_SHIFT;
502e95c6 Zach Reizner       2015-03-04  100  
502e95c6 Zach Reizner       2015-03-04  101  	num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
502e95c6 Zach Reizner       2015-03-04  102  
502e95c6 Zach Reizner       2015-03-04  103  	if (page_offset > num_pages)
502e95c6 Zach Reizner       2015-03-04  104  		return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner       2015-03-04  105  
502e95c6 Zach Reizner       2015-03-04  106  	mutex_lock(&dev->struct_mutex);
502e95c6 Zach Reizner       2015-03-04  107  
502e95c6 Zach Reizner       2015-03-04  108  	ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
502e95c6 Zach Reizner       2015-03-04  109  			     obj->pages[page_offset]);
502e95c6 Zach Reizner       2015-03-04  110  
502e95c6 Zach Reizner       2015-03-04  111  	mutex_unlock(&dev->struct_mutex);
502e95c6 Zach Reizner       2015-03-04  112  	switch (ret) {
502e95c6 Zach Reizner       2015-03-04  113  	case 0:
502e95c6 Zach Reizner       2015-03-04  114  		return VM_FAULT_NOPAGE;
502e95c6 Zach Reizner       2015-03-04  115  	case -ENOMEM:
502e95c6 Zach Reizner       2015-03-04  116  		return VM_FAULT_OOM;
502e95c6 Zach Reizner       2015-03-04  117  	case -EBUSY:
502e95c6 Zach Reizner       2015-03-04  118  		return VM_FAULT_RETRY;
502e95c6 Zach Reizner       2015-03-04  119  	case -EFAULT:
502e95c6 Zach Reizner       2015-03-04  120  	case -EINVAL:
502e95c6 Zach Reizner       2015-03-04  121  		return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner       2015-03-04  122  	default:
502e95c6 Zach Reizner       2015-03-04  123  		WARN_ON(1);
502e95c6 Zach Reizner       2015-03-04  124  		return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner       2015-03-04  125  	}
502e95c6 Zach Reizner       2015-03-04  126  }
502e95c6 Zach Reizner       2015-03-04  127  
7cbea8dc Kirill A. Shutemov 2015-09-09  128  static const struct vm_operations_struct vgem_gem_vm_ops = {
502e95c6 Zach Reizner       2015-03-04  129  	.fault = vgem_gem_fault,
502e95c6 Zach Reizner       2015-03-04  130  	.open = drm_gem_vm_open,
502e95c6 Zach Reizner       2015-03-04  131  	.close = drm_gem_vm_close,
502e95c6 Zach Reizner       2015-03-04  132  };
502e95c6 Zach Reizner       2015-03-04  133  
502e95c6 Zach Reizner       2015-03-04  134  /* ioctls */
502e95c6 Zach Reizner       2015-03-04  135  
502e95c6 Zach Reizner       2015-03-04  136  static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
502e95c6 Zach Reizner       2015-03-04  137  					      struct drm_file *file,
502e95c6 Zach Reizner       2015-03-04  138  					      unsigned int *handle,
502e95c6 Zach Reizner       2015-03-04  139  					      unsigned long size)
502e95c6 Zach Reizner       2015-03-04  140  {
502e95c6 Zach Reizner       2015-03-04  141  	struct drm_vgem_gem_object *obj;
502e95c6 Zach Reizner       2015-03-04  142  	struct drm_gem_object *gem_object;
502e95c6 Zach Reizner       2015-03-04  143  	int err;
502e95c6 Zach Reizner       2015-03-04  144  
502e95c6 Zach Reizner       2015-03-04  145  	size = roundup(size, PAGE_SIZE);
502e95c6 Zach Reizner       2015-03-04  146  
502e95c6 Zach Reizner       2015-03-04  147  	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
502e95c6 Zach Reizner       2015-03-04  148  	if (!obj)
502e95c6 Zach Reizner       2015-03-04  149  		return ERR_PTR(-ENOMEM);
502e95c6 Zach Reizner       2015-03-04  150  
502e95c6 Zach Reizner       2015-03-04  151  	gem_object = &obj->base;
502e95c6 Zach Reizner       2015-03-04  152  
502e95c6 Zach Reizner       2015-03-04  153  	err = drm_gem_object_init(dev, gem_object, size);
502e95c6 Zach Reizner       2015-03-04  154  	if (err)
502e95c6 Zach Reizner       2015-03-04  155  		goto out;
502e95c6 Zach Reizner       2015-03-04  156  
502e95c6 Zach Reizner       2015-03-04  157  	err = drm_gem_handle_create(file, gem_object, handle);
502e95c6 Zach Reizner       2015-03-04  158  	if (err)
502e95c6 Zach Reizner       2015-03-04  159  		goto handle_out;
502e95c6 Zach Reizner       2015-03-04  160  
502e95c6 Zach Reizner       2015-03-04  161  	drm_gem_object_unreference_unlocked(gem_object);
502e95c6 Zach Reizner       2015-03-04  162  
502e95c6 Zach Reizner       2015-03-04  163  	return gem_object;
502e95c6 Zach Reizner       2015-03-04  164  
502e95c6 Zach Reizner       2015-03-04  165  handle_out:
502e95c6 Zach Reizner       2015-03-04  166  	drm_gem_object_release(gem_object);
502e95c6 Zach Reizner       2015-03-04  167  out:
502e95c6 Zach Reizner       2015-03-04  168  	kfree(obj);
502e95c6 Zach Reizner       2015-03-04  169  	return ERR_PTR(err);
502e95c6 Zach Reizner       2015-03-04  170  }
502e95c6 Zach Reizner       2015-03-04  171  
502e95c6 Zach Reizner       2015-03-04  172  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
502e95c6 Zach Reizner       2015-03-04  173  				struct drm_mode_create_dumb *args)
502e95c6 Zach Reizner       2015-03-04  174  {
502e95c6 Zach Reizner       2015-03-04  175  	struct drm_gem_object *gem_object;
502e95c6 Zach Reizner       2015-03-04  176  	uint64_t size;
502e95c6 Zach Reizner       2015-03-04  177  	uint64_t pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
502e95c6 Zach Reizner       2015-03-04  178  
502e95c6 Zach Reizner       2015-03-04  179  	size = args->height * pitch;
502e95c6 Zach Reizner       2015-03-04  180  	if (size == 0)
502e95c6 Zach Reizner       2015-03-04  181  		return -EINVAL;
502e95c6 Zach Reizner       2015-03-04  182  
502e95c6 Zach Reizner       2015-03-04  183  	gem_object = vgem_gem_create(dev, file, &args->handle, size);
502e95c6 Zach Reizner       2015-03-04  184  
502e95c6 Zach Reizner       2015-03-04  185  	if (IS_ERR(gem_object)) {
502e95c6 Zach Reizner       2015-03-04  186  		DRM_DEBUG_DRIVER("object creation failed\n");
502e95c6 Zach Reizner       2015-03-04  187  		return PTR_ERR(gem_object);
502e95c6 Zach Reizner       2015-03-04  188  	}
502e95c6 Zach Reizner       2015-03-04  189  
502e95c6 Zach Reizner       2015-03-04  190  	args->size = gem_object->size;
502e95c6 Zach Reizner       2015-03-04  191  	args->pitch = pitch;
502e95c6 Zach Reizner       2015-03-04  192  
502e95c6 Zach Reizner       2015-03-04  193  	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
502e95c6 Zach Reizner       2015-03-04  194  
502e95c6 Zach Reizner       2015-03-04  195  	return 0;
502e95c6 Zach Reizner       2015-03-04  196  }
502e95c6 Zach Reizner       2015-03-04  197  
502e95c6 Zach Reizner       2015-03-04  198  int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev,
502e95c6 Zach Reizner       2015-03-04  199  		      uint32_t handle, uint64_t *offset)
502e95c6 Zach Reizner       2015-03-04  200  {
502e95c6 Zach Reizner       2015-03-04  201  	int ret = 0;
502e95c6 Zach Reizner       2015-03-04  202  	struct drm_gem_object *obj;
502e95c6 Zach Reizner       2015-03-04  203  
502e95c6 Zach Reizner       2015-03-04  204  	mutex_lock(&dev->struct_mutex);
502e95c6 Zach Reizner       2015-03-04  205  	obj = drm_gem_object_lookup(dev, file, handle);
502e95c6 Zach Reizner       2015-03-04  206  	if (!obj) {
502e95c6 Zach Reizner       2015-03-04  207  		ret = -ENOENT;
502e95c6 Zach Reizner       2015-03-04  208  		goto unlock;
502e95c6 Zach Reizner       2015-03-04  209  	}
502e95c6 Zach Reizner       2015-03-04  210  
502e95c6 Zach Reizner       2015-03-04 @211  	if (!drm_vma_node_has_offset(&obj->vma_node)) {
502e95c6 Zach Reizner       2015-03-04  212  		ret = drm_gem_create_mmap_offset(obj);
502e95c6 Zach Reizner       2015-03-04  213  		if (ret)
502e95c6 Zach Reizner       2015-03-04  214  			goto unref;

:::::: The code at line 211 was first introduced by commit
:::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM

:::::: TO: Zach Reizner <zachr@google.com>
:::::: CC: Dave Airlie <airlied@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 19274 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/vma_manage: Drop has_offset
  2015-10-22 18:47   ` [Intel-gfx] " kbuild test robot
@ 2015-10-22 19:44     ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-10-22 19:44 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	kbuild-all, Daniel Vetter

On Fri, Oct 23, 2015 at 02:47:49AM +0800, kbuild test robot wrote:
> Hi Daniel,
> 
> [auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
> 
> url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317
> config: x86_64-randconfig-s1-10230205 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from include/uapi/linux/stddef.h:1:0,
>                     from include/linux/stddef.h:4,
>                     from include/uapi/linux/posix_types.h:4,
>                     from include/uapi/linux/types.h:13,
>                     from include/linux/types.h:5,
>                     from include/linux/list.h:4,
>                     from include/linux/module.h:9,
>                     from drivers/gpu/drm/vgem/vgem_drv.c:33:
>    drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map':
>    drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration]
>      if (!drm_vma_node_has_offset(&obj->vma_node)) {
>           ^
>    include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
>      if (__builtin_constant_p((cond)) ? !!(cond) :   \
>                                ^
> >> drivers/gpu/drm/vgem/vgem_drv.c:211:2: note: in expansion of macro 'if'
>      if (!drm_vma_node_has_offset(&obj->vma_node)) {
>      ^
>    cc1: some warnings being treated as errors

Argh, I've forgotten that the vgem patch in my tree isn't merged yet. So
that one needs to go in (to remove another dubious use of has_offset())
before we can pull in this one.
-Daniel

> 
> vim +/if +211 drivers/gpu/drm/vgem/vgem_drv.c
> 
> 502e95c6 Zach Reizner       2015-03-04   27  
> 502e95c6 Zach Reizner       2015-03-04   28  /**
> 502e95c6 Zach Reizner       2015-03-04   29   * This is vgem, a (non-hardware-backed) GEM service.  This is used by Mesa's
> 502e95c6 Zach Reizner       2015-03-04   30   * software renderer and the X server for efficient buffer sharing.
> 502e95c6 Zach Reizner       2015-03-04   31   */
> 502e95c6 Zach Reizner       2015-03-04   32  
> 502e95c6 Zach Reizner       2015-03-04  @33  #include <linux/module.h>
> 502e95c6 Zach Reizner       2015-03-04   34  #include <linux/ramfs.h>
> 502e95c6 Zach Reizner       2015-03-04   35  #include <linux/shmem_fs.h>
> 502e95c6 Zach Reizner       2015-03-04   36  #include <linux/dma-buf.h>
> 502e95c6 Zach Reizner       2015-03-04   37  #include "vgem_drv.h"
> 502e95c6 Zach Reizner       2015-03-04   38  
> 502e95c6 Zach Reizner       2015-03-04   39  #define DRIVER_NAME	"vgem"
> 502e95c6 Zach Reizner       2015-03-04   40  #define DRIVER_DESC	"Virtual GEM provider"
> 502e95c6 Zach Reizner       2015-03-04   41  #define DRIVER_DATE	"20120112"
> 502e95c6 Zach Reizner       2015-03-04   42  #define DRIVER_MAJOR	1
> 502e95c6 Zach Reizner       2015-03-04   43  #define DRIVER_MINOR	0
> 502e95c6 Zach Reizner       2015-03-04   44  
> 502e95c6 Zach Reizner       2015-03-04   45  void vgem_gem_put_pages(struct drm_vgem_gem_object *obj)
> 502e95c6 Zach Reizner       2015-03-04   46  {
> 502e95c6 Zach Reizner       2015-03-04   47  	drm_gem_put_pages(&obj->base, obj->pages, false, false);
> 502e95c6 Zach Reizner       2015-03-04   48  	obj->pages = NULL;
> 502e95c6 Zach Reizner       2015-03-04   49  }
> 502e95c6 Zach Reizner       2015-03-04   50  
> 502e95c6 Zach Reizner       2015-03-04   51  static void vgem_gem_free_object(struct drm_gem_object *obj)
> 502e95c6 Zach Reizner       2015-03-04   52  {
> 502e95c6 Zach Reizner       2015-03-04   53  	struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
> 502e95c6 Zach Reizner       2015-03-04   54  
> 502e95c6 Zach Reizner       2015-03-04   55  	drm_gem_free_mmap_offset(obj);
> 502e95c6 Zach Reizner       2015-03-04   56  
> 502e95c6 Zach Reizner       2015-03-04   57  	if (vgem_obj->use_dma_buf && obj->dma_buf) {
> 502e95c6 Zach Reizner       2015-03-04   58  		dma_buf_put(obj->dma_buf);
> 502e95c6 Zach Reizner       2015-03-04   59  		obj->dma_buf = NULL;
> 502e95c6 Zach Reizner       2015-03-04   60  	}
> 502e95c6 Zach Reizner       2015-03-04   61  
> 502e95c6 Zach Reizner       2015-03-04   62  	drm_gem_object_release(obj);
> 502e95c6 Zach Reizner       2015-03-04   63  
> 502e95c6 Zach Reizner       2015-03-04   64  	if (vgem_obj->pages)
> 502e95c6 Zach Reizner       2015-03-04   65  		vgem_gem_put_pages(vgem_obj);
> 502e95c6 Zach Reizner       2015-03-04   66  
> 502e95c6 Zach Reizner       2015-03-04   67  	vgem_obj->pages = NULL;
> 502e95c6 Zach Reizner       2015-03-04   68  
> 502e95c6 Zach Reizner       2015-03-04   69  	kfree(vgem_obj);
> 502e95c6 Zach Reizner       2015-03-04   70  }
> 502e95c6 Zach Reizner       2015-03-04   71  
> 502e95c6 Zach Reizner       2015-03-04   72  int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
> 502e95c6 Zach Reizner       2015-03-04   73  {
> 502e95c6 Zach Reizner       2015-03-04   74  	struct page **pages;
> 502e95c6 Zach Reizner       2015-03-04   75  
> 502e95c6 Zach Reizner       2015-03-04   76  	if (obj->pages || obj->use_dma_buf)
> 502e95c6 Zach Reizner       2015-03-04   77  		return 0;
> 502e95c6 Zach Reizner       2015-03-04   78  
> 502e95c6 Zach Reizner       2015-03-04   79  	pages = drm_gem_get_pages(&obj->base);
> 502e95c6 Zach Reizner       2015-03-04   80  	if (IS_ERR(pages)) {
> 502e95c6 Zach Reizner       2015-03-04   81  		return PTR_ERR(pages);
> 502e95c6 Zach Reizner       2015-03-04   82  	}
> 502e95c6 Zach Reizner       2015-03-04   83  
> 502e95c6 Zach Reizner       2015-03-04   84  	obj->pages = pages;
> 502e95c6 Zach Reizner       2015-03-04   85  
> 502e95c6 Zach Reizner       2015-03-04   86  	return 0;
> 502e95c6 Zach Reizner       2015-03-04   87  }
> 502e95c6 Zach Reizner       2015-03-04   88  
> 502e95c6 Zach Reizner       2015-03-04   89  static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> 502e95c6 Zach Reizner       2015-03-04   90  {
> 502e95c6 Zach Reizner       2015-03-04   91  	struct drm_vgem_gem_object *obj = vma->vm_private_data;
> 502e95c6 Zach Reizner       2015-03-04   92  	struct drm_device *dev = obj->base.dev;
> 502e95c6 Zach Reizner       2015-03-04   93  	loff_t num_pages;
> 502e95c6 Zach Reizner       2015-03-04   94  	pgoff_t page_offset;
> 502e95c6 Zach Reizner       2015-03-04   95  	int ret;
> 502e95c6 Zach Reizner       2015-03-04   96  
> 502e95c6 Zach Reizner       2015-03-04   97  	/* We don't use vmf->pgoff since that has the fake offset */
> 502e95c6 Zach Reizner       2015-03-04   98  	page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
> 502e95c6 Zach Reizner       2015-03-04   99  		PAGE_SHIFT;
> 502e95c6 Zach Reizner       2015-03-04  100  
> 502e95c6 Zach Reizner       2015-03-04  101  	num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
> 502e95c6 Zach Reizner       2015-03-04  102  
> 502e95c6 Zach Reizner       2015-03-04  103  	if (page_offset > num_pages)
> 502e95c6 Zach Reizner       2015-03-04  104  		return VM_FAULT_SIGBUS;
> 502e95c6 Zach Reizner       2015-03-04  105  
> 502e95c6 Zach Reizner       2015-03-04  106  	mutex_lock(&dev->struct_mutex);
> 502e95c6 Zach Reizner       2015-03-04  107  
> 502e95c6 Zach Reizner       2015-03-04  108  	ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
> 502e95c6 Zach Reizner       2015-03-04  109  			     obj->pages[page_offset]);
> 502e95c6 Zach Reizner       2015-03-04  110  
> 502e95c6 Zach Reizner       2015-03-04  111  	mutex_unlock(&dev->struct_mutex);
> 502e95c6 Zach Reizner       2015-03-04  112  	switch (ret) {
> 502e95c6 Zach Reizner       2015-03-04  113  	case 0:
> 502e95c6 Zach Reizner       2015-03-04  114  		return VM_FAULT_NOPAGE;
> 502e95c6 Zach Reizner       2015-03-04  115  	case -ENOMEM:
> 502e95c6 Zach Reizner       2015-03-04  116  		return VM_FAULT_OOM;
> 502e95c6 Zach Reizner       2015-03-04  117  	case -EBUSY:
> 502e95c6 Zach Reizner       2015-03-04  118  		return VM_FAULT_RETRY;
> 502e95c6 Zach Reizner       2015-03-04  119  	case -EFAULT:
> 502e95c6 Zach Reizner       2015-03-04  120  	case -EINVAL:
> 502e95c6 Zach Reizner       2015-03-04  121  		return VM_FAULT_SIGBUS;
> 502e95c6 Zach Reizner       2015-03-04  122  	default:
> 502e95c6 Zach Reizner       2015-03-04  123  		WARN_ON(1);
> 502e95c6 Zach Reizner       2015-03-04  124  		return VM_FAULT_SIGBUS;
> 502e95c6 Zach Reizner       2015-03-04  125  	}
> 502e95c6 Zach Reizner       2015-03-04  126  }
> 502e95c6 Zach Reizner       2015-03-04  127  
> 7cbea8dc Kirill A. Shutemov 2015-09-09  128  static const struct vm_operations_struct vgem_gem_vm_ops = {
> 502e95c6 Zach Reizner       2015-03-04  129  	.fault = vgem_gem_fault,
> 502e95c6 Zach Reizner       2015-03-04  130  	.open = drm_gem_vm_open,
> 502e95c6 Zach Reizner       2015-03-04  131  	.close = drm_gem_vm_close,
> 502e95c6 Zach Reizner       2015-03-04  132  };
> 502e95c6 Zach Reizner       2015-03-04  133  
> 502e95c6 Zach Reizner       2015-03-04  134  /* ioctls */
> 502e95c6 Zach Reizner       2015-03-04  135  
> 502e95c6 Zach Reizner       2015-03-04  136  static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> 502e95c6 Zach Reizner       2015-03-04  137  					      struct drm_file *file,
> 502e95c6 Zach Reizner       2015-03-04  138  					      unsigned int *handle,
> 502e95c6 Zach Reizner       2015-03-04  139  					      unsigned long size)
> 502e95c6 Zach Reizner       2015-03-04  140  {
> 502e95c6 Zach Reizner       2015-03-04  141  	struct drm_vgem_gem_object *obj;
> 502e95c6 Zach Reizner       2015-03-04  142  	struct drm_gem_object *gem_object;
> 502e95c6 Zach Reizner       2015-03-04  143  	int err;
> 502e95c6 Zach Reizner       2015-03-04  144  
> 502e95c6 Zach Reizner       2015-03-04  145  	size = roundup(size, PAGE_SIZE);
> 502e95c6 Zach Reizner       2015-03-04  146  
> 502e95c6 Zach Reizner       2015-03-04  147  	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> 502e95c6 Zach Reizner       2015-03-04  148  	if (!obj)
> 502e95c6 Zach Reizner       2015-03-04  149  		return ERR_PTR(-ENOMEM);
> 502e95c6 Zach Reizner       2015-03-04  150  
> 502e95c6 Zach Reizner       2015-03-04  151  	gem_object = &obj->base;
> 502e95c6 Zach Reizner       2015-03-04  152  
> 502e95c6 Zach Reizner       2015-03-04  153  	err = drm_gem_object_init(dev, gem_object, size);
> 502e95c6 Zach Reizner       2015-03-04  154  	if (err)
> 502e95c6 Zach Reizner       2015-03-04  155  		goto out;
> 502e95c6 Zach Reizner       2015-03-04  156  
> 502e95c6 Zach Reizner       2015-03-04  157  	err = drm_gem_handle_create(file, gem_object, handle);
> 502e95c6 Zach Reizner       2015-03-04  158  	if (err)
> 502e95c6 Zach Reizner       2015-03-04  159  		goto handle_out;
> 502e95c6 Zach Reizner       2015-03-04  160  
> 502e95c6 Zach Reizner       2015-03-04  161  	drm_gem_object_unreference_unlocked(gem_object);
> 502e95c6 Zach Reizner       2015-03-04  162  
> 502e95c6 Zach Reizner       2015-03-04  163  	return gem_object;
> 502e95c6 Zach Reizner       2015-03-04  164  
> 502e95c6 Zach Reizner       2015-03-04  165  handle_out:
> 502e95c6 Zach Reizner       2015-03-04  166  	drm_gem_object_release(gem_object);
> 502e95c6 Zach Reizner       2015-03-04  167  out:
> 502e95c6 Zach Reizner       2015-03-04  168  	kfree(obj);
> 502e95c6 Zach Reizner       2015-03-04  169  	return ERR_PTR(err);
> 502e95c6 Zach Reizner       2015-03-04  170  }
> 502e95c6 Zach Reizner       2015-03-04  171  
> 502e95c6 Zach Reizner       2015-03-04  172  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> 502e95c6 Zach Reizner       2015-03-04  173  				struct drm_mode_create_dumb *args)
> 502e95c6 Zach Reizner       2015-03-04  174  {
> 502e95c6 Zach Reizner       2015-03-04  175  	struct drm_gem_object *gem_object;
> 502e95c6 Zach Reizner       2015-03-04  176  	uint64_t size;
> 502e95c6 Zach Reizner       2015-03-04  177  	uint64_t pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> 502e95c6 Zach Reizner       2015-03-04  178  
> 502e95c6 Zach Reizner       2015-03-04  179  	size = args->height * pitch;
> 502e95c6 Zach Reizner       2015-03-04  180  	if (size == 0)
> 502e95c6 Zach Reizner       2015-03-04  181  		return -EINVAL;
> 502e95c6 Zach Reizner       2015-03-04  182  
> 502e95c6 Zach Reizner       2015-03-04  183  	gem_object = vgem_gem_create(dev, file, &args->handle, size);
> 502e95c6 Zach Reizner       2015-03-04  184  
> 502e95c6 Zach Reizner       2015-03-04  185  	if (IS_ERR(gem_object)) {
> 502e95c6 Zach Reizner       2015-03-04  186  		DRM_DEBUG_DRIVER("object creation failed\n");
> 502e95c6 Zach Reizner       2015-03-04  187  		return PTR_ERR(gem_object);
> 502e95c6 Zach Reizner       2015-03-04  188  	}
> 502e95c6 Zach Reizner       2015-03-04  189  
> 502e95c6 Zach Reizner       2015-03-04  190  	args->size = gem_object->size;
> 502e95c6 Zach Reizner       2015-03-04  191  	args->pitch = pitch;
> 502e95c6 Zach Reizner       2015-03-04  192  
> 502e95c6 Zach Reizner       2015-03-04  193  	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
> 502e95c6 Zach Reizner       2015-03-04  194  
> 502e95c6 Zach Reizner       2015-03-04  195  	return 0;
> 502e95c6 Zach Reizner       2015-03-04  196  }
> 502e95c6 Zach Reizner       2015-03-04  197  
> 502e95c6 Zach Reizner       2015-03-04  198  int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev,
> 502e95c6 Zach Reizner       2015-03-04  199  		      uint32_t handle, uint64_t *offset)
> 502e95c6 Zach Reizner       2015-03-04  200  {
> 502e95c6 Zach Reizner       2015-03-04  201  	int ret = 0;
> 502e95c6 Zach Reizner       2015-03-04  202  	struct drm_gem_object *obj;
> 502e95c6 Zach Reizner       2015-03-04  203  
> 502e95c6 Zach Reizner       2015-03-04  204  	mutex_lock(&dev->struct_mutex);
> 502e95c6 Zach Reizner       2015-03-04  205  	obj = drm_gem_object_lookup(dev, file, handle);
> 502e95c6 Zach Reizner       2015-03-04  206  	if (!obj) {
> 502e95c6 Zach Reizner       2015-03-04  207  		ret = -ENOENT;
> 502e95c6 Zach Reizner       2015-03-04  208  		goto unlock;
> 502e95c6 Zach Reizner       2015-03-04  209  	}
> 502e95c6 Zach Reizner       2015-03-04  210  
> 502e95c6 Zach Reizner       2015-03-04 @211  	if (!drm_vma_node_has_offset(&obj->vma_node)) {
> 502e95c6 Zach Reizner       2015-03-04  212  		ret = drm_gem_create_mmap_offset(obj);
> 502e95c6 Zach Reizner       2015-03-04  213  		if (ret)
> 502e95c6 Zach Reizner       2015-03-04  214  			goto unref;
> 
> :::::: The code at line 211 was first introduced by commit
> :::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM
> 
> :::::: TO: Zach Reizner <zachr@google.com>
> :::::: CC: Dave Airlie <airlied@redhat.com>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm: Update GEM refcounting docs
  2015-10-22 17:54 ` [PATCH 1/3] drm: Update GEM refcounting docs Alex Deucher
@ 2015-10-22 19:46   ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-10-22 19:46 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter

On Thu, Oct 22, 2015 at 01:54:17PM -0400, Alex Deucher wrote:
> On Thu, Oct 22, 2015 at 1:11 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > I just realized that I've forgotten to update all the gem refcounting
> > docs. For pennance also add pretty docs for the overall drm_gem_object
> > structure, with a few links thrown in fore good.
> >
> > As usually we need to make sure the kerneldoc reference is at most a
> > sect2 for otherwise it won't be listed.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Patches 1 and 3 are:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks, applied to drm-misc.
-Daniel

> 
> > ---
> >  Documentation/DocBook/gpu.tmpl |  15 +++---
> >  include/drm/drm_gem.h          | 106 +++++++++++++++++++++++++++++++++++------
> >  2 files changed, 100 insertions(+), 21 deletions(-)
> >
> > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> > index 90c2aab31269..6c5865bb5ee8 100644
> > --- a/Documentation/DocBook/gpu.tmpl
> > +++ b/Documentation/DocBook/gpu.tmpl
> > @@ -635,10 +635,10 @@ char *date;</synopsis>
> >            acquired and release by <function>calling drm_gem_object_reference</function>
> >            and <function>drm_gem_object_unreference</function> respectively. The
> >            caller must hold the <structname>drm_device</structname>
> > -          <structfield>struct_mutex</structfield> lock. As a convenience, GEM
> > -          provides the <function>drm_gem_object_reference_unlocked</function> and
> > -          <function>drm_gem_object_unreference_unlocked</function> functions that
> > -          can be called without holding the lock.
> > +         <structfield>struct_mutex</structfield> lock when calling
> > +         <function>drm_gem_object_reference</function>. As a convenience, GEM
> > +         provides <function>drm_gem_object_unreference_unlocked</function>
> > +         functions that can be called without holding the lock.
> >          </para>
> >          <para>
> >            When the last reference to a GEM object is released the GEM core calls
> > @@ -836,10 +836,11 @@ char *date;</synopsis>
> >            abstracted from the client in libdrm.
> >          </para>
> >        </sect3>
> > -      <sect3>
> > -        <title>GEM Function Reference</title>
> > +    </sect2>
> > +    <sect2>
> > +      <title>GEM Function Reference</title>
> >  !Edrivers/gpu/drm/drm_gem.c
> > -      </sect3>
> > +!Iinclude/drm/drm_gem.h
> >      </sect2>
> >      <sect2>
> >        <title>VMA Offset Manager</title>
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index 15e7f007380f..0b3e11ab8757 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -35,76 +35,129 @@
> >   */
> >
> >  /**
> > - * This structure defines the drm_mm memory object, which will be used by the
> > - * DRM for its buffer objects.
> > + * struct drm_gem_object - GEM buffer object
> > + *
> > + * This structure defines the generic parts for GEM buffer objects, which are
> > + * mostly around handling mmap and userspace handles.
> > + *
> > + * Buffer objects are often abbreviated to BO.
> >   */
> >  struct drm_gem_object {
> > -       /** Reference count of this object */
> > +       /**
> > +        * @refcount:
> > +        *
> > +        * Reference count of this object
> > +        *
> > +        * Please use drm_gem_object_reference() to acquire and
> > +        * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked()
> > +        * to release a reference to a GEM buffer object.
> > +        */
> >         struct kref refcount;
> >
> >         /**
> > -        * handle_count - gem file_priv handle count of this object
> > +        * @handle_count:
> > +        *
> > +        * This is the GEM file_priv handle count of this object.
> >          *
> >          * Each handle also holds a reference. Note that when the handle_count
> >          * drops to 0 any global names (e.g. the id in the flink namespace) will
> >          * be cleared.
> >          *
> >          * Protected by dev->object_name_lock.
> > -        * */
> > +        */
> >         unsigned handle_count;
> >
> > -       /** Related drm device */
> > +       /**
> > +        * @dev: DRM dev this object belongs to.
> > +        */
> >         struct drm_device *dev;
> >
> > -       /** File representing the shmem storage */
> > +       /**
> > +        * @filp:
> > +        *
> > +        * SHMEM file node used as backing storage for swappable buffer objects.
> > +        * GEM also supports driver private objects with driver-specific backing
> > +        * storage (contiguous CMA memory, special reserved blocks). In this
> > +        * case @filp is NULL.
> > +        */
> >         struct file *filp;
> >
> > -       /* Mapping info for this object */
> > +       /**
> > +        * @vma_node:
> > +        *
> > +        * Mapping info for this object to support mmap. Drivers are supposed to
> > +        * allocate the mmap offset using drm_gem_create_mmap_offset(). The
> > +        * offset itself can be retrieved using drm_vma_node_offset_addr().
> > +        *
> > +        * Memory mapping itself is handled by drm_gem_mmap(), which also checks
> > +        * that userspace is allowed to access the object.
> > +        */
> >         struct drm_vma_offset_node vma_node;
> >
> >         /**
> > +        * @size:
> > +        *
> >          * Size of the object, in bytes.  Immutable over the object's
> >          * lifetime.
> >          */
> >         size_t size;
> >
> >         /**
> > +        * @name:
> > +        *
> >          * Global name for this object, starts at 1. 0 means unnamed.
> > -        * Access is covered by the object_name_lock in the related drm_device
> > +        * Access is covered by dev->object_name_lock. This is used by the GEM_FLINK
> > +        * and GEM_OPEN ioctls.
> >          */
> >         int name;
> >
> >         /**
> > -        * Memory domains. These monitor which caches contain read/write data
> > +        * @read_domains:
> > +        *
> > +        * Read memory domains. These monitor which caches contain read/write data
> >          * related to the object. When transitioning from one set of domains
> >          * to another, the driver is called to ensure that caches are suitably
> > -        * flushed and invalidated
> > +        * flushed and invalidated.
> >          */
> >         uint32_t read_domains;
> > +
> > +       /**
> > +        * @write_domain: Corresponding unique write memory domain.
> > +        */
> >         uint32_t write_domain;
> >
> >         /**
> > +        * @pending_read_domains:
> > +        *
> >          * While validating an exec operation, the
> >          * new read/write domain values are computed here.
> >          * They will be transferred to the above values
> >          * at the point that any cache flushing occurs
> >          */
> >         uint32_t pending_read_domains;
> > +
> > +       /**
> > +        * @pending_write_domain: Write domain similar to @pending_read_domains.
> > +        */
> >         uint32_t pending_write_domain;
> >
> >         /**
> > -        * dma_buf - dma buf associated with this GEM object
> > +        * @dma_buf:
> > +        *
> > +        * dma-buf associated with this GEM object.
> >          *
> >          * Pointer to the dma-buf associated with this gem object (either
> >          * through importing or exporting). We break the resulting reference
> >          * loop when the last gem handle for this object is released.
> >          *
> > -        * Protected by obj->object_name_lock
> > +        * Protected by obj->object_name_lock.
> >          */
> >         struct dma_buf *dma_buf;
> >
> >         /**
> > -        * import_attach - dma buf attachment backing this object
> > +        * @import_attach:
> > +        *
> > +        * dma-buf attachment backing this object.
> >          *
> >          * Any foreign dma_buf imported as a gem object has this set to the
> >          * attachment point for the device. This is invariant over the lifetime
> > @@ -133,12 +186,30 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >                      struct vm_area_struct *vma);
> >  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> >
> > +/**
> > + * drm_gem_object_reference - acquire a GEM BO reference
> > + * @obj: GEM buffer object
> > + *
> > + * This acquires additional reference to @obj. It is illegal to call this
> > + * without already holding a reference. No locks required.
> > + */
> >  static inline void
> >  drm_gem_object_reference(struct drm_gem_object *obj)
> >  {
> >         kref_get(&obj->refcount);
> >  }
> >
> > +/**
> > + * drm_gem_object_unreference - release a GEM BO reference
> > + * @obj: GEM buffer object
> > + *
> > + * This releases a reference to @obj. Callers must hold the dev->struct_mutex
> > + * lock when calling this function, even when the driver doesn't use
> > + * dev->struct_mutex for anything.
> > + *
> > + * For drivers not encumbered with legacy locking use
> > + * drm_gem_object_unreference_unlocked() instead.
> > + */
> >  static inline void
> >  drm_gem_object_unreference(struct drm_gem_object *obj)
> >  {
> > @@ -149,6 +220,13 @@ drm_gem_object_unreference(struct drm_gem_object *obj)
> >         }
> >  }
> >
> > +/**
> > + * drm_gem_object_unreference_unlocked - release a GEM BO reference
> > + * @obj: GEM buffer object
> > + *
> > + * This releases a reference to @obj. Callers must not hold the
> > + * dev->struct_mutex lock when calling this function.
> > + */
> >  static inline void
> >  drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
> >  {
> > --
> > 2.5.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] drm/vma_manage: Drop has_offset
  2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
  2015-10-22 17:41   ` kbuild test robot
  2015-10-22 18:47   ` [Intel-gfx] " kbuild test robot
@ 2015-10-23  9:33   ` David Herrmann
  2 siblings, 0 replies; 9+ messages in thread
From: David Herrmann @ 2015-10-23  9:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development

Hi

On Thu, Oct 22, 2015 at 7:11 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> It's racy, creating mmap offsets is a slowpath, so better to remove it
> to avoid drivers doing broken things.
>
> The only user is i915, and it's ok there because everything (well
> almost) is protected by dev->struct_mutex in i915-gem.
>
> While at it add a note in the create_mmap_offset kerneldoc that
> drivers must release it again. And then I also noticed that
> drm_gem_object_release entirely lacks kerneldoc.
>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I'd even argue that no driver should ever call into
drm_vma_offset_add() nor drm_vma_offset_remove(). For TTM this is
already the case, for plain old gem drivers still call
drm_vma_offset_add() (which is fine to me, but could be turned into a
gem helper).

Anyway, if TTM wasn't a module, we should drop the export of
drm_vma_offset_remove(), but that'll never happen, I guess.

Long story short: If anyone calls drm_vma_offset_remove() somewhere
but in the destructor, it's probably racy, so I fully support this
patch. The vma helpers are also made fail-safe on purpose, it has
never been a fast-path.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> ---
>  drivers/gpu/drm/drm_gem.c       | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem.c |  3 ---
>  include/drm/drm_vma_manager.h   | 15 +--------------
>  3 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 64353d40db53..38680380c6b3 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -387,6 +387,10 @@ EXPORT_SYMBOL(drm_gem_handle_create);
>   * @obj: obj in question
>   *
>   * This routine frees fake offsets allocated by drm_gem_create_mmap_offset().
> + *
> + * Note that drm_gem_object_release() already calls this function, so drivers
> + * don't have to take care of releasing the mmap offset themselves when freeing
> + * the GEM object.
>   */
>  void
>  drm_gem_free_mmap_offset(struct drm_gem_object *obj)
> @@ -410,6 +414,9 @@ EXPORT_SYMBOL(drm_gem_free_mmap_offset);
>   * This routine allocates and attaches a fake offset for @obj, in cases where
>   * the virtual size differs from the physical size (ie. obj->size).  Otherwise
>   * just use drm_gem_create_mmap_offset().
> + *
> + * This function is idempotent and handles an already allocated mmap offset
> + * transparently. Drivers do not need to check for this case.
>   */
>  int
>  drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
> @@ -431,6 +438,9 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset_size);
>   * structures.
>   *
>   * This routine allocates and attaches a fake offset for @obj.
> + *
> + * Drivers can call drm_gem_free_mmap_offset() before freeing @obj to release
> + * the fake offset again.
>   */
>  int drm_gem_create_mmap_offset(struct drm_gem_object *obj)
>  {
> @@ -739,6 +749,13 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private)
>         idr_destroy(&file_private->object_idr);
>  }
>
> +/**
> + * drm_gem_object_release - release GEM buffer object resources
> + * @obj: GEM buffer object
> + *
> + * This releases any structures and resources used by @obj and is the invers of
> + * drm_gem_object_init().
> + */
>  void
>  drm_gem_object_release(struct drm_gem_object *obj)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d0fa5481543c..01fef54ecb2d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1972,9 +1972,6 @@ 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 (drm_vma_node_has_offset(&obj->base.vma_node))
> -               return 0;
> -
>         dev_priv->mm.shrinker_no_lock_stealing = true;
>
>         ret = drm_gem_create_mmap_offset(&obj->base);
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> index 2f63dd5e05eb..06ea8e077ec2 100644
> --- a/include/drm/drm_vma_manager.h
> +++ b/include/drm/drm_vma_manager.h
> @@ -176,19 +176,6 @@ static inline unsigned long drm_vma_node_size(struct drm_vma_offset_node *node)
>  }
>
>  /**
> - * 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
>   *
> @@ -220,7 +207,7 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
>  static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
>                                       struct address_space *file_mapping)
>  {
> -       if (drm_vma_node_has_offset(node))
> +       if (drm_mm_node_allocated(&node->vm_node))
>                 unmap_mapping_range(file_mapping,
>                                     drm_vma_node_offset_addr(node),
>                                     drm_vma_node_size(node) << PAGE_SHIFT, 1);
> --
> 2.5.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-10-23  9:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-22 17:11 [PATCH 1/3] drm: Update GEM refcounting docs Daniel Vetter
2015-10-22 17:11 ` [PATCH 2/3] drm/vma_manage: Drop has_offset Daniel Vetter
2015-10-22 17:41   ` kbuild test robot
2015-10-22 18:47   ` [Intel-gfx] " kbuild test robot
2015-10-22 19:44     ` Daniel Vetter
2015-10-23  9:33   ` David Herrmann
2015-10-22 17:11 ` [PATCH 3/3] drm/gem: Update/Polish docs Daniel Vetter
2015-10-22 17:54 ` [PATCH 1/3] drm: Update GEM refcounting docs Alex Deucher
2015-10-22 19:46   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.