drm/i915: Document that mmap forwarding is discouraged
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 16 Oct 2014 10:28:18 +0000 (12:28 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 24 Oct 2014 14:34:04 +0000 (16:34 +0200)
Too many new drm driver writers seem to look at i915 for inspiration.
But we have two ways to do mmap, so discourage readers from the old,
ugly version. In a new driver we'd just expose two mmap offsets per
object, one for the gtt map and the other for the cpu map.

v2: Make it clear that i915 does cpu mmaps this way for past
cluelessness^W^W historical reasons. Asked for by Jani.

Cc: "Cheng, Yao" <yao.cheng@intel.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_gem.c

index e9c783d5561239f7689ad4e3e690d27f104ae1b6..895f9881f0aa899b4850888606dbd1e3d2879564 100644 (file)
@@ -1466,6 +1466,16 @@ unlock:
  *
  * While the mapping holds a reference on the contents of the object, it doesn't
  * imply a ref on the object itself.
+ *
+ * IMPORTANT:
+ *
+ * DRM driver writers who look a this function as an example for how to do GEM
+ * mmap support, please don't implement mmap support like here. The modern way
+ * to implement DRM mmap support is with an mmap offset ioctl (like
+ * i915_gem_mmap_gtt) and then using the mmap syscall on the DRM fd directly.
+ * That way debug tooling like valgrind will understand what's going on, hiding
+ * the mmap call in a driver private ioctl will break that. The i915 driver only
+ * does cpu mmaps this way because we didn't know better.
  */
 int
 i915_gem_mmap_ioctl(struct drm_device *dev, void *data,