[RFC] Revive the work on render-nodes branch

[RFC] Revive the work on render-nodes branch

[Ilija Hadzic]

The following set of patches will revive the drm-render-nodes [1]
branch that has been dormant in Dave Airlie's repository for some
time. I rebased this branch to the latest drm-core-next and did some
(hopefully useful) follow-up work.

I fixed a few bugs, did a substantial cleanup, separated the
experimental hard-coded stuff from general stuff and implemented
all kinds of checks and protections from any ugly stuff that
user space can send. I also have libdrm patches as well as a small
test-utility program that can be used to create and remove render
nodes from user space. I will send these in the next patch series.

There is still more work to be done, and a few weird behaviors to
track down, but with these patches, I am able to create and remove
render nodes and bring up independent seats on each.

The following are the details of the patch series:

* The first three patches (0001-0003) are direct derivative of
  original drm-render-nodes work. Originally, there was one patch
  that included multiple logical changes, as well as the test-only
  temporary code. I cleaned that up and broke up the original
  patch into three separate ones, that appeared logical units to me.
  Acknowledgment: Because of the rework (i.e. split into three), these
  three patches carry my name in the author field, but they are really
  Dave's work.

  * Patch 0004 adds ioctls for manipulating render nodes and is almost
    a verbatim copy of original Dave's patch. I tried to keep it as
    intact as possible.

  * Patches 0005 (prep) and 0006 (real thing) are my follow-up to
    ioctl work. They fix bugs I found in 0004 and provide a general
    cleanup.

  * Remaining patches are the the follow-up work on render nodes
    in general. Some are addressing TODO items listed in [1],
    and some are from my own TODO.

At this time, I'd like to solicit comments and feedback and I'll
be glad to rework the patches based on the feedback I receive.
Note that although the patches are meant to work on any GPU,
I have only tested this with Radeon hardware. If someone runs
this with other hardware, I would be very interested in hearing
about the result. Also, I have only tested this for multiseat-X
use case. If someone tries this for GPGPU use case, I'd appreciate
the feedback.

regards,
-- Ilija

----
[1] http://airlied.livejournal.com/72187.html

[PATCH 01/16] drm: do not push inode down into drm_open_helper

[Ilija Hadzic]

Push minor number instead. This is a preparatory
patch for introducing render nodes. It has been derived
from 7c5cc4f63556e351e9e5980ed22accad410e3fdc originally
created by Dave Airlie.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_fops.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 6263b01..98cb064 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -43,8 +43,8 @@
 DEFINE_MUTEX(drm_global_mutex);
 EXPORT_SYMBOL(drm_global_mutex);
 
-static int drm_open_helper(struct inode *inode, struct file *filp,
-			   struct drm_device * dev);
+static int drm_open_helper(struct file *filp,
+			   struct drm_minor *minor);
 
 static int drm_setup(struct drm_device * dev)
 {
@@ -133,7 +133,7 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (!(dev = minor->dev))
 		return -ENODEV;
 
-	retcode = drm_open_helper(inode, filp, dev);
+	retcode = drm_open_helper(filp, minor);
 	if (!retcode) {
 		atomic_inc(&dev->counts[_DRM_STAT_OPENS]);
 		if (!dev->open_count++)
@@ -226,10 +226,10 @@ static int drm_cpu_valid(void)
  * Creates and initializes a drm_file structure for the file private data in \p
  * filp and add it into the double linked list in \p dev.
  */
-static int drm_open_helper(struct inode *inode, struct file *filp,
-			   struct drm_device * dev)
+static int drm_open_helper(struct file *filp,
+			   struct drm_minor *minor)
 {
-	int minor_id = iminor(inode);
+	struct drm_device *dev = minor->dev;
 	struct drm_file *priv;
 	int ret;
 
@@ -240,7 +240,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 	if (dev->switch_power_state != DRM_SWITCH_POWER_ON)
 		return -EINVAL;
 
-	DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor_id);
+	DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -250,7 +250,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 	priv->filp = filp;
 	priv->uid = current_euid();
 	priv->pid = task_pid_nr(current);
-	priv->minor = idr_find(&drm_minors_idr, minor_id);
+	priv->minor = minor;
 	priv->ioctl_count = 0;
 	/* for compatibility root is always authenticated */
 	priv->authenticated = capable(CAP_SYS_ADMIN);
@@ -558,7 +558,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	}
 
 	/* drop the reference held my the file priv */
-	drm_master_put(&file_priv->master);
+	if (file_priv->master)
+		drm_master_put(&file_priv->master);
 	file_priv->is_master = 0;
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->struct_mutex);
-- 
1.7.8.5

[PATCH 02/16] drm: move dev_mapping to the minor node

[Ilija Hadzic]

Make dev_mapping per-minor instead of per device. This is
a preparatory patch for introducing render nodes. This
will allow per-node instead of per-device mapping range,
once we introduce render nodes.

Patch derived from 7c5cc4f63556e351e9e5980ed22accad410e3fdc
originally authored by Dave Airlie.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_drv.c              |    1 -
 drivers/gpu/drm/drm_fops.c             |    8 ++++----
 drivers/gpu/drm/drm_vm.c               |    9 +++++++++
 drivers/gpu/drm/i915/i915_gem.c        |    7 +++----
 drivers/gpu/drm/nouveau/nouveau_gem.c  |    4 ++--
 drivers/gpu/drm/radeon/radeon_object.c |    4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c    |    6 +++---
 drivers/gpu/drm/ttm/ttm_bo.c           |    7 ++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |    5 ++---
 drivers/staging/omapdrm/omap_gem.c     |   10 ++++------
 include/drm/drmP.h                     |    3 ++-
 include/drm/drm_mem_util.h             |    3 +++
 include/drm/ttm/ttm_bo_driver.h        |    3 ++-
 13 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d166bd0..a4d7d44 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -236,7 +236,6 @@ int drm_lastclose(struct drm_device * dev)
 	    !drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_dma_takedown(dev);
 
-	dev->dev_mapping = NULL;
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG("lastclose completed\n");
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 98cb064..4498d76 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -141,10 +141,10 @@ int drm_open(struct inode *inode, struct file *filp)
 	}
 	if (!retcode) {
 		mutex_lock(&dev->struct_mutex);
-		if (minor->type == DRM_MINOR_LEGACY) {
-			if (dev->dev_mapping == NULL)
-				dev->dev_mapping = inode->i_mapping;
-			else if (dev->dev_mapping != inode->i_mapping)
+		if (minor->type == DRM_MINOR_LEGACY || minor->type == DRM_MINOR_RENDER) {
+			if (minor->dev_mapping == NULL)
+				minor->dev_mapping = inode->i_mapping;
+			else if (minor->dev_mapping != inode->i_mapping)
 				retcode = -ENODEV;
 		}
 		mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 55cd615..bcd15b0 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -687,3 +687,12 @@ int drm_mmap(struct file *filp, struct vm_area_struct *vma)
 	return ret;
 }
 EXPORT_SYMBOL(drm_mmap);
+
+void drm_unmap_mapping(struct drm_device *dev, loff_t const holebegin,
+		       loff_t const holelen)
+{
+	if (dev->primary->dev_mapping)
+		unmap_mapping_range(dev->primary->dev_mapping,
+				    holebegin, holelen, 1);
+}
+EXPORT_SYMBOL(drm_unmap_mapping);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 19a06c2..5eb0294 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1215,10 +1215,9 @@ 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)obj->base.map_list.hash.key<<PAGE_SHIFT,
-				    obj->base.size, 1);
+	drm_unmap_mapping(obj->base.dev,
+			  (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
+			  obj->base.size);
 
 	obj->fault_mappable = false;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 7ce3fde..63521af 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -202,8 +202,8 @@ nouveau_gem_ioctl_new(struct drm_device *dev, void *data,
 	struct nouveau_bo *nvbo = NULL;
 	int ret = 0;
 
-	if (unlikely(dev_priv->ttm.bdev.dev_mapping == NULL))
-		dev_priv->ttm.bdev.dev_mapping = dev_priv->dev->dev_mapping;
+	if (unlikely(dev_priv->ttm.bdev.mapping_priv == NULL))
+		dev_priv->ttm.bdev.mapping_priv = (void *)dev;
 
 	if (!dev_priv->engine.vram.flags_valid(dev, req->info.tile_flags)) {
 		NV_ERROR(dev, "bad page flags: 0x%08x\n", req->info.tile_flags);
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 342deac..837c7eb 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -115,8 +115,8 @@ int radeon_bo_create(struct radeon_device *rdev,
 
 	size = ALIGN(size, PAGE_SIZE);
 
-	if (unlikely(rdev->mman.bdev.dev_mapping == NULL)) {
-		rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
+	if (unlikely(rdev->mman.bdev.mapping_priv == NULL)) {
+		rdev->mman.bdev.mapping_priv = (void *)rdev->ddev;
 	}
 	if (kernel) {
 		type = ttm_bo_type_kernel;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index f493c64..7bedbf8 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -753,9 +753,9 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	}
 	DRM_INFO("radeon: %uM of GTT memory ready.\n",
 		 (unsigned)(rdev->mc.gtt_size / (1024 * 1024)));
-	if (unlikely(rdev->mman.bdev.dev_mapping == NULL)) {
-		rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
-	}
+
+	if (unlikely(rdev->mman.bdev.mapping_priv == NULL))
+		rdev->mman.bdev.mapping_priv = (void *)rdev->ddev;
 
 	r = radeon_ttm_debugfs_init(rdev);
 	if (r) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7c3a57d..40bc5f6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -38,6 +38,7 @@
 #include <linux/file.h>
 #include <linux/module.h>
 #include <linux/atomic.h>
+#include "drm/drm_mem_util.h"
 
 #define TTM_ASSERT_LOCKED(param)
 #define TTM_DEBUG(fmt, arg...)
@@ -1579,7 +1580,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
 	bdev->nice_mode = true;
 	INIT_LIST_HEAD(&bdev->ddestroy);
-	bdev->dev_mapping = NULL;
+	bdev->mapping_priv = NULL;
 	bdev->glob = glob;
 	bdev->need_dma32 = need_dma32;
 	bdev->val_seq = 0;
@@ -1623,9 +1624,9 @@ void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
 	loff_t offset = (loff_t) bo->addr_space_offset;
 	loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
 
-	if (!bdev->dev_mapping)
+	if (!bdev->mapping_priv)
 		return;
-	unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
+	drm_unmap_mapping(bdev->mapping_priv, offset, holelen);
 	ttm_mem_io_free_vm(bo);
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 1760aba..6a4adc0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -770,9 +770,8 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
 
 	file_priv->driver_priv = vmw_fp;
 
-	if (unlikely(dev_priv->bdev.dev_mapping == NULL))
-		dev_priv->bdev.dev_mapping =
-			file_priv->filp->f_path.dentry->d_inode->i_mapping;
+	if (unlikely(dev_priv->bdev.mapping_priv == NULL))
+		dev_priv->bdev.mapping_priv = (void *)dev;
 
 	return 0;
 
diff --git a/drivers/staging/omapdrm/omap_gem.c b/drivers/staging/omapdrm/omap_gem.c
index b7d6f88..22a5f39 100644
--- a/drivers/staging/omapdrm/omap_gem.c
+++ b/drivers/staging/omapdrm/omap_gem.c
@@ -150,13 +150,11 @@ static struct {
 static void evict_entry(struct drm_gem_object *obj,
 		enum tiler_fmt fmt, struct usergart_entry *entry)
 {
-	if (obj->dev->dev_mapping) {
-		size_t size = PAGE_SIZE * usergart[fmt].height;
-		loff_t off = mmap_offset(obj) +
-				(entry->obj_pgoff << PAGE_SHIFT);
-		unmap_mapping_range(obj->dev->dev_mapping, off, size, 1);
-	}
+	size_t size = PAGE_SIZE * usergart[fmt].height;
+	loff_t off = mmap_offset(obj) +
+		(entry->obj_pgoff << PAGE_SHIFT);
 
+	drm_unmap_mapping(obj->dev, off, size);
 	entry->obj = NULL;
 }
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index cfd921f..eeb377a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -997,6 +997,8 @@ struct drm_minor {
 	struct drm_master *master; /* currently active master for this node */
 	struct list_head master_list;
 	struct drm_mode_group mode_group;
+
+	struct address_space *dev_mapping;
 };
 
 /* mode specified on the command line */
@@ -1152,7 +1154,6 @@ struct drm_device {
 	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
 	void *dev_private;		/**< device private data */
 	void *mm_private;
-	struct address_space *dev_mapping;
 	struct drm_sigdata sigdata;	   /**< For block_all_signals */
 	sigset_t sigmask;
 
diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
index 6bd325f..820afbb 100644
--- a/include/drm/drm_mem_util.h
+++ b/include/drm/drm_mem_util.h
@@ -62,4 +62,7 @@ static __inline void drm_free_large(void *ptr)
 	vfree(ptr);
 }
 
+struct drm_device;
+extern void drm_unmap_mapping(struct drm_device *dev, loff_t const holebegin,
+			      loff_t const holelen);
 #endif
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index d43e892..4a05aca 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -40,6 +40,7 @@
 #include "linux/spinlock.h"
 
 struct ttm_backend;
+struct drm_device;
 
 struct ttm_backend_func {
 	/**
@@ -558,7 +559,7 @@ struct ttm_bo_device {
 	 */
 
 	bool nice_mode;
-	struct address_space *dev_mapping;
+	struct drm_device *mapping_priv;
 
 	/*
 	 * Internal protection.
-- 
1.7.8.5

[PATCH 03/16] drm: add support for render nodes

[Ilija Hadzic]

Add support for creating multiple render nodes on the same
physical GPU.

Patch derived from 7c5cc4f63556e351e9e5980ed22accad410e3fdc
originally authored by Dave Airlie.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_crtc.c |    1 +
 drivers/gpu/drm/drm_stub.c |   32 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_vm.c   |    7 +++++++
 include/drm/drmP.h         |    4 +++-
 include/drm/drm_crtc.h     |    1 +
 5 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d2d9dc5..f081873 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -950,6 +950,7 @@ int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group)
 	group->num_encoders = 0;
 	return 0;
 }
+EXPORT_SYMBOL(drm_mode_group_init);
 
 int drm_mode_group_init_legacy_group(struct drm_device *dev,
 				     struct drm_mode_group *group)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 6d7b083..53033d3 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -269,6 +269,7 @@ int drm_fill_in_dev(struct drm_device *dev,
 	INIT_LIST_HEAD(&dev->vmalist);
 	INIT_LIST_HEAD(&dev->maplist);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
+	INIT_LIST_HEAD(&dev->render_minor_list);
 
 	spin_lock_init(&dev->count_lock);
 	spin_lock_init(&dev->event_lock);
@@ -355,6 +356,7 @@ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type)
 	new_minor->dev = dev;
 	new_minor->index = minor_id;
 	INIT_LIST_HEAD(&new_minor->master_list);
+	INIT_LIST_HEAD(&new_minor->render_node_list);
 
 	idr_replace(&drm_minors_idr, new_minor, minor_id);
 
@@ -398,6 +400,29 @@ err_idr:
 	return ret;
 }
 
+int drm_create_minor_render(struct drm_device *dev, struct drm_minor **minor_p)
+{
+	int ret;
+	struct drm_minor *minor;
+
+	ret = drm_get_minor(dev, &minor, DRM_MINOR_RENDER);
+	if (ret)
+		return ret;
+
+	list_add_tail(&minor->render_node_list, &dev->render_minor_list);
+	return 0;
+}
+
+int drm_destroy_minor_render(struct drm_device *dev)
+{
+	struct drm_minor *minor, *tmp;
+
+	list_for_each_entry_safe(minor, tmp, &dev->render_minor_list, render_node_list) {
+		drm_put_minor(&minor);
+	}
+	return 0;
+}
+
 /**
  * Put a secondary minor number.
  *
@@ -414,6 +439,8 @@ int drm_put_minor(struct drm_minor **minor_p)
 
 	DRM_DEBUG("release secondary minor %d\n", minor->index);
 
+	list_del(&minor->render_node_list);
+
 	if (minor->type == DRM_MINOR_LEGACY)
 		drm_proc_cleanup(minor, drm_proc_root);
 #if defined(CONFIG_DEBUG_FS)
@@ -476,9 +503,12 @@ void drm_put_dev(struct drm_device *dev)
 
 	drm_ctxbitmap_cleanup(dev);
 
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		drm_put_minor(&dev->control);
 
+		drm_destroy_minor_render(dev);
+	}
+
 	if (driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);
 
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index bcd15b0..7cf67dd 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -691,8 +691,15 @@ EXPORT_SYMBOL(drm_mmap);
 void drm_unmap_mapping(struct drm_device *dev, loff_t const holebegin,
 		       loff_t const holelen)
 {
+	struct drm_minor *minor;
 	if (dev->primary->dev_mapping)
 		unmap_mapping_range(dev->primary->dev_mapping,
 				    holebegin, holelen, 1);
+
+	list_for_each_entry(minor, &dev->render_minor_list, render_node_list) {
+		if (minor->dev_mapping)
+			unmap_mapping_range(minor->dev_mapping,
+					    holebegin, holelen, 1);
+	}
 }
 EXPORT_SYMBOL(drm_unmap_mapping);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index eeb377a..d9eee26 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -999,6 +999,8 @@ struct drm_minor {
 	struct drm_mode_group mode_group;
 
 	struct address_space *dev_mapping;
+
+	struct list_head render_node_list;
 };
 
 /* mode specified on the command line */
@@ -1162,7 +1164,7 @@ struct drm_device {
 	unsigned int agp_buffer_token;
 	struct drm_minor *control;		/**< Control node for card */
 	struct drm_minor *primary;		/**< render type primary screen head */
-
+	struct list_head render_minor_list;
         struct drm_mode_config mode_config;	/**< Current mode config */
 
 	/** \name GEM information */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 9595c2c..fc87b31 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -849,6 +849,7 @@ extern char *drm_get_dvi_i_select_name(int val);
 extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
+extern int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 				 struct i2c_adapter *adapter);
-- 
1.7.8.5

[PATCH 04/16] drm: initial multiple nodes ioctl work.

[Ilija Hadzic]

From: Dave Airlie <airlied@redhat.com>

just adds some unchecked ioctls to setup the nodes.

Signed-off-by: Dave Airlie <airlied@redhat.com>

v2: - original ioctl numbers are now taken, use next available
    - resolve some trivial conflicts due to bit-rot that
      occurred since the original patch was created

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_drv.c  |    4 ++-
 drivers/gpu/drm/drm_stub.c |   56 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm.h          |    3 ++
 include/drm/drmP.h         |    6 ++++
 include/drm/drm_mode.h     |   14 +++++++++++
 5 files changed, 82 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a4d7d44..dd04322 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -159,7 +159,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED)
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_RENDER_NODE_CREATE, drm_render_node_create_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_RENDER_NODE_REMOVE, drm_render_node_remove_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED)
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 53033d3..442e03c 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -522,3 +522,59 @@ void drm_put_dev(struct drm_device *dev)
 	kfree(dev);
 }
 EXPORT_SYMBOL(drm_put_dev);
+
+int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
+				 struct drm_file *file_priv)
+{
+	struct drm_render_node_create *args = data;
+	int ret;
+	struct drm_minor *new_minor;
+	int total_ids, i;
+	uint32_t __user *ids_ptr;
+	ret = drm_create_minor_render(dev, &new_minor);
+	if (ret)
+		goto out;
+
+	args->node_minor_id = new_minor->index;
+
+	if (args->num_crtc == 0 &&
+	    args->num_encoder == 0 &&
+	    args->num_connector == 0)
+		goto out;
+	if (args->num_crtc == 0 ||
+	    args->num_encoder == 0 ||
+	    args->num_connector == 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = drm_mode_group_init(dev, &new_minor->mode_group);
+	if (ret)
+		goto out;
+
+	ids_ptr = (uint32_t __user *)(unsigned long)args->id_list_ptr;
+	total_ids = args->num_crtc + args->num_encoder + args->num_connector;
+	for (i = 0; i < total_ids; i++) {
+		if (get_user(new_minor->mode_group.id_list[i], &ids_ptr[i])) {
+			ret = -EFAULT;
+			goto out_put;
+		}
+	}
+out_put:
+	drm_put_minor(&new_minor);
+out:
+	return ret;
+}
+
+int drm_render_node_remove_ioctl(struct drm_device *dev, void *data,
+				 struct drm_file *file_priv)
+{
+	struct drm_render_node_remove *args = data;
+	struct drm_minor *del_minor, *tmp;
+
+	list_for_each_entry_safe(del_minor, tmp, &dev->render_minor_list, render_node_list) {
+		if (del_minor->index == args->node_minor_id)
+			drm_put_minor(&del_minor);
+	}
+	return 0;
+}
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 34a7b89..d2a893b 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -639,6 +639,9 @@ struct drm_get_cap {
 #define DRM_IOCTL_GEM_OPEN		DRM_IOWR(0x0b, struct drm_gem_open)
 #define DRM_IOCTL_GET_CAP		DRM_IOWR(0x0c, struct drm_get_cap)
 
+#define DRM_IOCTL_RENDER_NODE_CREATE    DRM_IOWR(0x0d, struct drm_render_node_create)
+#define DRM_IOCTL_RENDER_NODE_REMOVE    DRM_IOWR(0x0e, struct drm_render_node_remove)
+
 #define DRM_IOCTL_SET_UNIQUE		DRM_IOW( 0x10, struct drm_unique)
 #define DRM_IOCTL_AUTH_MAGIC		DRM_IOW( 0x11, struct drm_auth)
 #define DRM_IOCTL_BLOCK			DRM_IOWR(0x12, struct drm_block)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d9eee26..3bc5c71 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1458,6 +1458,12 @@ extern void drm_master_put(struct drm_master **master);
 
 extern void drm_put_dev(struct drm_device *dev);
 extern int drm_put_minor(struct drm_minor **minor);
+
+extern int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv);
+extern int drm_render_node_remove_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv);
+
 extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 2a2acda..2a403bb 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -441,4 +441,18 @@ struct drm_mode_destroy_dumb {
 	uint32_t handle;
 };
 
+/* render node create and remove functions
+   if crtc/encoders/connectors all == 0 then gpgpu node */
+struct drm_render_node_create {
+	__u32 node_minor_id;
+	__u32 num_crtc;
+	__u32 num_encoder;
+	__u32 num_connector;
+	__u64 id_list_ptr;
+};
+
+struct drm_render_node_remove {
+	__u32 node_minor_id;
+};
+
 #endif
-- 
1.7.8.5

Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.

[Ville Syrjälä]

On Thu, Mar 29, 2012 at 12:41:26PM -0400, Ilija Hadzic wrote:
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index 2a2acda..2a403bb 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -441,4 +441,18 @@ struct drm_mode_destroy_dumb {
>  	uint32_t handle;
>  };
>  
> +/* render node create and remove functions
> +   if crtc/encoders/connectors all == 0 then gpgpu node */
> +struct drm_render_node_create {
> +	__u32 node_minor_id;
> +	__u32 num_crtc;
> +	__u32 num_encoder;
> +	__u32 num_connector;

num_plane is missing

> +	__u64 id_list_ptr;
> +};
> +
> +struct drm_render_node_remove {
> +	__u32 node_minor_id;
> +};
> +
>  #endif

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.

[Ilija Hadzic]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 801 bytes --]



On Thu, 29 Mar 2012, Ville [iso-8859-1] Syrjälä wrote:

>> +/* render node create and remove functions
>> +   if crtc/encoders/connectors all == 0 then gpgpu node */
>> +struct drm_render_node_create {
>> +	__u32 node_minor_id;
>> +	__u32 num_crtc;
>> +	__u32 num_encoder;
>> +	__u32 num_connector;
>
> num_plane is missing
>

Probably because at the time the original patch was created, there was no 
support for planes. I agree, we need to throw planes into this, but I'd 
prefer to do this as a follow-on patch series, rather than amending old 
patches. If for no better reason, than to keep simple relationship between 
the code and it's author (this particular patch is original Dave's patch 
from the days when we had no planes, only rebased to current HEAD).

-- Ilija

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

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

Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.

[Ville Syrjälä]

On Thu, Mar 29, 2012 at 12:22:20PM -0500, Ilija Hadzic wrote:
> 
> 
> On Thu, 29 Mar 2012, Ville [iso-8859-1] Syrjälä wrote:
> 
> >> +/* render node create and remove functions
> >> +   if crtc/encoders/connectors all == 0 then gpgpu node */
> >> +struct drm_render_node_create {
> >> +	__u32 node_minor_id;
> >> +	__u32 num_crtc;
> >> +	__u32 num_encoder;
> >> +	__u32 num_connector;
> >
> > num_plane is missing
> >
> 
> Probably because at the time the original patch was created, there was no 
> support for planes. I agree, we need to throw planes into this, but I'd 
> prefer to do this as a follow-on patch series, rather than amending old 
> patches. If for no better reason, than to keep simple relationship between 
> the code and it's author (this particular patch is original Dave's patch 
> from the days when we had no planes, only rebased to current HEAD).

I think that just makes it harder to review.

It's also part of the ABI, so exposing half finished version of the
struct in the headers, only to later change it, seems like a bad idea.

-- 
Ville Syrjälä
Intel OTC

[PATCH 05/16] drm: separate render node descriptor from minor

[Ilija Hadzic]

drm_minor structure had a list_head pointer that was
used only for render nodes. control and primary nodes
do not live in the list and had this list pointer unused.

Create a separate drm_render_node structure that is used
to build the render node list and that points to the
coresponding minor. This avoids having fields that are used
only in some cases and also to allows adding render node
specific fields that primary and control minor may not care
about.

Also fix a few minor things:
 * change the names of functions that create and delete
   render nodes to more logical ones.
 * make create_render_node actually populate minor_p
   pointer passed to it so that the ioctl has
   some chance of working

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |   45 +++++++++++++++++++++++++------------------
 drivers/gpu/drm/drm_vm.c   |    9 ++++---
 include/drm/drmP.h         |    9 ++++++-
 3 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 442e03c..ba6cb52 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -269,7 +269,7 @@ int drm_fill_in_dev(struct drm_device *dev,
 	INIT_LIST_HEAD(&dev->vmalist);
 	INIT_LIST_HEAD(&dev->maplist);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
-	INIT_LIST_HEAD(&dev->render_minor_list);
+	INIT_LIST_HEAD(&dev->render_node_list);
 
 	spin_lock_init(&dev->count_lock);
 	spin_lock_init(&dev->event_lock);
@@ -356,7 +356,6 @@ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type)
 	new_minor->dev = dev;
 	new_minor->index = minor_id;
 	INIT_LIST_HEAD(&new_minor->master_list);
-	INIT_LIST_HEAD(&new_minor->render_node_list);
 
 	idr_replace(&drm_minors_idr, new_minor, minor_id);
 
@@ -400,25 +399,36 @@ err_idr:
 	return ret;
 }
 
-int drm_create_minor_render(struct drm_device *dev, struct drm_minor **minor_p)
+int drm_create_render_node(struct drm_device *dev, struct drm_minor **minor_p)
 {
 	int ret;
 	struct drm_minor *minor;
+	struct drm_render_node *render_node;
+
+	render_node = kmalloc(sizeof(struct drm_render_node), GFP_KERNEL);
+	if (!render_node)
+		return -ENOMEM;
 
 	ret = drm_get_minor(dev, &minor, DRM_MINOR_RENDER);
-	if (ret)
+	if (ret) {
+		kfree(render_node);
 		return ret;
-
-	list_add_tail(&minor->render_node_list, &dev->render_minor_list);
+	}
+	render_node->minor = minor;
+	*minor_p = minor;
+	list_add_tail(&render_node->list, &dev->render_node_list);
 	return 0;
 }
 
-int drm_destroy_minor_render(struct drm_device *dev)
+int drm_destroy_all_render_nodes(struct drm_device *dev)
 {
-	struct drm_minor *minor, *tmp;
+	struct drm_render_node *render_node, *tmp;
 
-	list_for_each_entry_safe(minor, tmp, &dev->render_minor_list, render_node_list) {
-		drm_put_minor(&minor);
+	list_for_each_entry_safe(render_node, tmp,
+				 &dev->render_node_list, list) {
+		list_del(&render_node->list);
+		drm_put_minor(&render_node->minor);
+		kfree(render_node);
 	}
 	return 0;
 }
@@ -439,8 +449,6 @@ int drm_put_minor(struct drm_minor **minor_p)
 
 	DRM_DEBUG("release secondary minor %d\n", minor->index);
 
-	list_del(&minor->render_node_list);
-
 	if (minor->type == DRM_MINOR_LEGACY)
 		drm_proc_cleanup(minor, drm_proc_root);
 #if defined(CONFIG_DEBUG_FS)
@@ -505,8 +513,7 @@ void drm_put_dev(struct drm_device *dev)
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		drm_put_minor(&dev->control);
-
-		drm_destroy_minor_render(dev);
+		drm_destroy_all_render_nodes(dev);
 	}
 
 	if (driver->driver_features & DRIVER_GEM)
@@ -531,7 +538,7 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	struct drm_minor *new_minor;
 	int total_ids, i;
 	uint32_t __user *ids_ptr;
-	ret = drm_create_minor_render(dev, &new_minor);
+	ret = drm_create_render_node(dev, &new_minor);
 	if (ret)
 		goto out;
 
@@ -570,11 +577,11 @@ int drm_render_node_remove_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
 	struct drm_render_node_remove *args = data;
-	struct drm_minor *del_minor, *tmp;
+	struct drm_render_node *del_node, *tmp;
 
-	list_for_each_entry_safe(del_minor, tmp, &dev->render_minor_list, render_node_list) {
-		if (del_minor->index == args->node_minor_id)
-			drm_put_minor(&del_minor);
+	list_for_each_entry_safe(del_node, tmp, &dev->render_node_list, list) {
+		if (del_node->minor->index == args->node_minor_id)
+			drm_put_minor(&del_node->minor);
 	}
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 7cf67dd..73146e3 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -691,14 +691,15 @@ EXPORT_SYMBOL(drm_mmap);
 void drm_unmap_mapping(struct drm_device *dev, loff_t const holebegin,
 		       loff_t const holelen)
 {
-	struct drm_minor *minor;
+	struct drm_render_node *render_node;
+
 	if (dev->primary->dev_mapping)
 		unmap_mapping_range(dev->primary->dev_mapping,
 				    holebegin, holelen, 1);
 
-	list_for_each_entry(minor, &dev->render_minor_list, render_node_list) {
-		if (minor->dev_mapping)
-			unmap_mapping_range(minor->dev_mapping,
+	list_for_each_entry(render_node, &dev->render_node_list, list) {
+		if (render_node->minor->dev_mapping)
+			unmap_mapping_range(render_node->minor->dev_mapping,
 					    holebegin, holelen, 1);
 	}
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3bc5c71..1131fd4 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -999,8 +999,13 @@ struct drm_minor {
 	struct drm_mode_group mode_group;
 
 	struct address_space *dev_mapping;
+};
 
-	struct list_head render_node_list;
+
+/* render node descriptor */
+struct drm_render_node {
+	struct list_head list;
+	struct drm_minor *minor;
 };
 
 /* mode specified on the command line */
@@ -1164,7 +1169,7 @@ struct drm_device {
 	unsigned int agp_buffer_token;
 	struct drm_minor *control;		/**< Control node for card */
 	struct drm_minor *primary;		/**< render type primary screen head */
-	struct list_head render_minor_list;
+	struct list_head render_node_list;
         struct drm_mode_config mode_config;	/**< Current mode config */
 
 	/** \name GEM information */
-- 
1.7.8.5

[PATCH 06/16] drm: cleanup render node ioctls

[Ilija Hadzic]

Fix a few bugs in render node create and destroy ioctl
(mostly handling of error cases). Also separate some
common code into a new function for destroying
the render node.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |   80 +++++++++++++++++++++++++++----------------
 1 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index ba6cb52..12260f0 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -420,17 +420,30 @@ int drm_create_render_node(struct drm_device *dev, struct drm_minor **minor_p)
 	return 0;
 }
 
-int drm_destroy_all_render_nodes(struct drm_device *dev)
+int drm_destroy_render_node(struct drm_device *dev, int index)
 {
-	struct drm_render_node *render_node, *tmp;
+	struct drm_render_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
+		if (node->minor->index == index) {
+			list_del(&node->list);
+			drm_put_minor(&node->minor);
+			kfree(node);
+			return 0;
+		}
+	}
+	return -ENODEV;
+}
 
-	list_for_each_entry_safe(render_node, tmp,
-				 &dev->render_node_list, list) {
-		list_del(&render_node->list);
-		drm_put_minor(&render_node->minor);
-		kfree(render_node);
+void drm_destroy_all_render_nodes(struct drm_device *dev)
+{
+	struct drm_render_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
+		list_del(&node->list);
+		drm_put_minor(&node->minor);
+		kfree(node);
 	}
-	return 0;
 }
 
 /**
@@ -538,38 +551,48 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	struct drm_minor *new_minor;
 	int total_ids, i;
 	uint32_t __user *ids_ptr;
-	ret = drm_create_render_node(dev, &new_minor);
-	if (ret)
-		goto out;
-
-	args->node_minor_id = new_minor->index;
 
+	/* trivial case: render node with no display resources */
 	if (args->num_crtc == 0 &&
 	    args->num_encoder == 0 &&
-	    args->num_connector == 0)
-		goto out;
-	if (args->num_crtc == 0 ||
-	    args->num_encoder == 0 ||
 	    args->num_connector == 0) {
-		ret = -EINVAL;
-		goto out;
+		ret = drm_create_render_node(dev, &new_minor);
+		if (!ret)
+			args->node_minor_id = new_minor->index;
+		return ret;
 	}
 
+	/* if we have display resources, then we need at least
+	 * one CRTC, one encoder and one connector */
+	if (args->num_crtc == 0 ||
+	    args->num_encoder == 0 ||
+	    args->num_connector == 0)
+		return -EINVAL;
+
+	ret = drm_create_render_node(dev, &new_minor);
+	if (ret)
+		return ret;
+
 	ret = drm_mode_group_init(dev, &new_minor->mode_group);
 	if (ret)
-		goto out;
+		goto out_del;
 
 	ids_ptr = (uint32_t __user *)(unsigned long)args->id_list_ptr;
 	total_ids = args->num_crtc + args->num_encoder + args->num_connector;
 	for (i = 0; i < total_ids; i++) {
 		if (get_user(new_minor->mode_group.id_list[i], &ids_ptr[i])) {
 			ret = -EFAULT;
-			goto out_put;
+			goto out_del;
 		}
 	}
-out_put:
-	drm_put_minor(&new_minor);
-out:
+	new_minor->mode_group.num_crtcs = args->num_crtc;
+	new_minor->mode_group.num_encoders = args->num_encoder;
+	new_minor->mode_group.num_connectors = args->num_connector;
+
+	args->node_minor_id = new_minor->index;
+	return 0;
+out_del:
+	drm_destroy_render_node(dev, new_minor->index);
 	return ret;
 }
 
@@ -577,11 +600,8 @@ int drm_render_node_remove_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
 	struct drm_render_node_remove *args = data;
-	struct drm_render_node *del_node, *tmp;
+	int ret;
 
-	list_for_each_entry_safe(del_node, tmp, &dev->render_node_list, list) {
-		if (del_node->minor->index == args->node_minor_id)
-			drm_put_minor(&del_node->minor);
-	}
-	return 0;
+	ret = drm_destroy_render_node(dev, args->node_minor_id);
+	return ret;
 }
-- 
1.7.8.5

[PATCH 07/16] drm: only allow render node ioctls through control node

[Ilija Hadzic]

The render-node manipulation ioctls are supposed to be
issued through control node only. Add a check and return
-EPERM to user space if access is attempted through a
node other than a control node.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 12260f0..fb241e6 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -552,6 +552,10 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	int total_ids, i;
 	uint32_t __user *ids_ptr;
 
+	/* allow access through control node only */
+	if (file_priv->minor != dev->control)
+		return -EPERM;
+
 	/* trivial case: render node with no display resources */
 	if (args->num_crtc == 0 &&
 	    args->num_encoder == 0 &&
@@ -602,6 +606,10 @@ int drm_render_node_remove_ioctl(struct drm_device *dev, void *data,
 	struct drm_render_node_remove *args = data;
 	int ret;
 
+	/* allow access through control node only */
+	if (file_priv->minor != dev->control)
+		return -EPERM;
+
 	ret = drm_destroy_render_node(dev, args->node_minor_id);
 	return ret;
 }
-- 
1.7.8.5

[PATCH 08/16] drm: do not remove a render node in use

[Ilija Hadzic]

Keep track of per-node open count and do not allow removal
of a render node in use.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_fops.c |    2 ++
 drivers/gpu/drm/drm_stub.c |    2 ++
 include/drm/drmP.h         |    1 +
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 4498d76..686236f 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -136,6 +136,7 @@ int drm_open(struct inode *inode, struct file *filp)
 	retcode = drm_open_helper(filp, minor);
 	if (!retcode) {
 		atomic_inc(&dev->counts[_DRM_STAT_OPENS]);
+		minor->open_count++;
 		if (!dev->open_count++)
 			retcode = drm_setup(dev);
 	}
@@ -573,6 +574,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	 */
 
 	atomic_inc(&dev->counts[_DRM_STAT_CLOSES]);
+	file_priv->minor->open_count--;
 	if (!--dev->open_count) {
 		if (atomic_read(&dev->ioctl_count)) {
 			DRM_ERROR("Device busy: %d\n",
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index fb241e6..aa4b34f 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -426,6 +426,8 @@ int drm_destroy_render_node(struct drm_device *dev, int index)
 
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		if (node->minor->index == index) {
+			if (node->minor->open_count)
+				return -EBUSY;
 			list_del(&node->list);
 			drm_put_minor(&node->minor);
 			kfree(node);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 1131fd4..981f005 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -983,6 +983,7 @@ struct drm_info_node {
 struct drm_minor {
 	int index;			/**< Minor device number */
 	int type;                       /**< Control or render */
+	int open_count;
 	dev_t device;			/**< Device number for mknod */
 	struct device kdev;		/**< Linux device */
 	struct drm_device *dev;
-- 
1.7.8.5

[PATCH 09/16] drm: allocate correct id_list size for a render node

[Ilija Hadzic]

When a new render node is created, the number of elements
of id_list allocated in drm_mode_group_init function should
not be the sum of all CRTCs, encoders, and connectors that
the device has, but the one specified by the ioctl that
created the node.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_crtc.c |   16 +++++++---------
 drivers/gpu/drm/drm_stub.c |    5 ++---
 include/drm/drm_crtc.h     |    2 +-
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f081873..1e0ff2d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -933,14 +933,8 @@ void drm_mode_config_init(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_config_init);
 
-int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group)
+int drm_mode_group_init(struct drm_mode_group *group, int total_objects)
 {
-	uint32_t total_objects = 0;
-
-	total_objects += dev->mode_config.num_crtc;
-	total_objects += dev->mode_config.num_connector;
-	total_objects += dev->mode_config.num_encoder;
-
 	group->id_list = kzalloc(total_objects * sizeof(uint32_t), GFP_KERNEL);
 	if (!group->id_list)
 		return -ENOMEM;
@@ -958,9 +952,13 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
-	int ret;
+	int ret, total_objects;
 
-	if ((ret = drm_mode_group_init(dev, group)))
+	total_objects = dev->mode_config.num_crtc;
+	total_objects += dev->mode_config.num_connector;
+	total_objects += dev->mode_config.num_encoder;
+	ret = drm_mode_group_init(group, total_objects);
+	if (ret)
 		return ret;
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index aa4b34f..e316a69 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -578,13 +578,12 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	ret = drm_create_render_node(dev, &new_minor);
 	if (ret)
 		return ret;
-
-	ret = drm_mode_group_init(dev, &new_minor->mode_group);
+	total_ids = args->num_crtc + args->num_encoder + args->num_connector;
+	ret = drm_mode_group_init(&new_minor->mode_group, total_ids);
 	if (ret)
 		goto out_del;
 
 	ids_ptr = (uint32_t __user *)(unsigned long)args->id_list_ptr;
-	total_ids = args->num_crtc + args->num_encoder + args->num_connector;
 	for (i = 0; i < total_ids; i++) {
 		if (get_user(new_minor->mode_group.id_list[i], &ids_ptr[i])) {
 			ret = -EFAULT;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fc87b31..2110a82 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -849,7 +849,7 @@ extern char *drm_get_dvi_i_select_name(int val);
 extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
-extern int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group);
+extern int drm_mode_group_init(struct drm_mode_group *group, int total_objects);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 				 struct i2c_adapter *adapter);
-- 
1.7.8.5

[PATCH 10/16] drm: add drm_mode_group_fini function

[Ilija Hadzic]

This is the opposite function of drm_mode_group_init. It will
be needed to properly cleanup the render node after removing it.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_crtc.c |   10 ++++++++++
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1e0ff2d..9af822b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -946,6 +946,16 @@ int drm_mode_group_init(struct drm_mode_group *group, int total_objects)
 }
 EXPORT_SYMBOL(drm_mode_group_init);
 
+void drm_mode_group_fini(struct drm_mode_group *group)
+{
+	kfree(group->id_list);
+	group->id_list = NULL;
+	group->num_crtcs = 0;
+	group->num_connectors = 0;
+	group->num_encoders = 0;
+}
+EXPORT_SYMBOL(drm_mode_group_fini);
+
 int drm_mode_group_init_legacy_group(struct drm_device *dev,
 				     struct drm_mode_group *group)
 {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 2110a82..86f4d9a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -850,6 +850,7 @@ extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
 extern int drm_mode_group_init(struct drm_mode_group *group, int total_objects);
+extern void drm_mode_group_fini(struct drm_mode_group *group);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 				 struct i2c_adapter *adapter);
-- 
1.7.8.5

[PATCH 11/16] drm: properly free id_list when a render node is removed

[Ilija Hadzic]

id_list is dynamically allocated when a render node is
created. Consequently, if must be freed when the render node
is removed, otherwise we have a memory leak.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index e316a69..e41882a 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -426,10 +426,13 @@ int drm_destroy_render_node(struct drm_device *dev, int index)
 
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		if (node->minor->index == index) {
+			struct drm_mode_group *group;
 			if (node->minor->open_count)
 				return -EBUSY;
+			group = &node->minor->mode_group;
 			list_del(&node->list);
 			drm_put_minor(&node->minor);
+			drm_mode_group_fini(group);
 			kfree(node);
 			return 0;
 		}
@@ -442,8 +445,11 @@ void drm_destroy_all_render_nodes(struct drm_device *dev)
 	struct drm_render_node *node, *tmp;
 
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
+		struct drm_mode_group *group;
+		group = &node->minor->mode_group;
 		list_del(&node->list);
 		drm_put_minor(&node->minor);
+		drm_mode_group_fini(group);
 		kfree(node);
 	}
 }
-- 
1.7.8.5

[PATCH 12/16] drm: call drm_mode_group_fini on primary node

[Ilija Hadzic]

In drm_put_dev, the whole device is being destroyed, so id_list
of the primary (legacy) node should also be cleaned up.
This plugs a memory leak that has probably existed even without
the render-node work.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index e41882a..b59203b 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -540,6 +540,7 @@ void drm_put_dev(struct drm_device *dev)
 	if (driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);
 
+	drm_mode_group_fini(&dev->primary->mode_group);
 	drm_put_minor(&dev->primary);
 
 	list_del(&dev->driver_item);
-- 
1.7.8.5

[PATCH 13/16] drm: more elaborate check for num_crtc/encoder/connector

[Ilija Hadzic]

User space can send us all kinds of nonsense for num_crtc, num_encoder
and num_connector. So far, we have been checking only for presence
of at least one CRTC/encoder/connector (barring the trivial case
of a render node with no display resources, i.e., GPGPU node).

This patch makes the ioctl fail if user space requests more resources
than the physical GPU has. This is primarily to protect the kmalloc
in drm_mode_group_init from hogging a big chunk of memory if some
bozo sends us a request for some huge number of CRTCs, encoders,
or connectors.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index b59203b..196892c 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -575,9 +575,11 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 		return ret;
 	}
 
-	/* if we have display resources, then we need at least
-	 * one CRTC, one encoder and one connector */
-	if (args->num_crtc == 0 ||
+	/* sanity check for requested num_crtc/num_encoder/num_connector */
+	if (args->num_crtc > dev->mode_config.num_crtc ||
+	    args->num_encoder > dev->mode_config.num_encoder ||
+	    args->num_encoder > dev->mode_config.num_connector ||
+	    args->num_crtc == 0 ||
 	    args->num_encoder == 0 ||
 	    args->num_connector == 0)
 		return -EINVAL;
-- 
1.7.8.5

[PATCH 14/16] drm: validate id list when creating a render node

[Ilija Hadzic]

Render node ioctl requires a list of DRM mode objects
in specific order: first all CRTCs, then all encoders,
followed by all connectors. Check that the IDs passed
from userland are in conformance with this requirement
and that they are consistent with specified num_crtc,
num_encoder and num_connector values.

Return -EINVAL to if the check fails. Otherwise, accept
the list and create the requested render node.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |   66 ++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 196892c..5b6459f 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -399,6 +399,53 @@ err_idr:
 	return ret;
 }
 
+static int drm_get_id_from_user(struct drm_device *dev,
+				uint32_t *id_dst,
+				uint32_t __user *id_src,
+				uint32_t expected_type)
+{
+	struct drm_mode_object *drmmode_obj;
+	uint32_t id;
+
+	if (get_user(id, id_src))
+		return -EFAULT;
+	drmmode_obj = drm_mode_object_find(dev, id, expected_type);
+	if (!drmmode_obj)
+		return -EINVAL;
+	*id_dst = id;
+	return 0;
+}
+
+#define DRM_RN_NUM_EXP_TYPES 3
+static const uint32_t expected_type_list[DRM_RN_NUM_EXP_TYPES] = {
+	DRM_MODE_OBJECT_CRTC,
+	DRM_MODE_OBJECT_ENCODER,
+	DRM_MODE_OBJECT_CONNECTOR
+};
+
+static int drm_get_render_node_resources(struct drm_device *dev,
+					 uint32_t *id_list,
+					 uint32_t __user *ids_ptr,
+					 int *resource_count)
+
+{
+	int s, e, i, j;
+	int ret;
+
+	for (e = 0, j = 0; j < DRM_RN_NUM_EXP_TYPES; j++) {
+		s = e;
+		e += resource_count[j];
+		for (i = s; i < e; i++) {
+			ret = drm_get_id_from_user(dev, &id_list[i],
+						   &ids_ptr[i],
+						   expected_type_list[j]);
+			if (ret)
+				return ret;
+		}
+	}
+	return 0;
+}
+
 int drm_create_render_node(struct drm_device *dev, struct drm_minor **minor_p)
 {
 	int ret;
@@ -558,8 +605,10 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	struct drm_render_node_create *args = data;
 	int ret;
 	struct drm_minor *new_minor;
-	int total_ids, i;
+	int total_ids;
+	int resource_count[DRM_RN_NUM_EXP_TYPES];
 	uint32_t __user *ids_ptr;
+	uint32_t *id_list;
 
 	/* allow access through control node only */
 	if (file_priv->minor != dev->control)
@@ -591,14 +640,15 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 	ret = drm_mode_group_init(&new_minor->mode_group, total_ids);
 	if (ret)
 		goto out_del;
-
+	resource_count[0] = args->num_crtc;
+	resource_count[1] = args->num_encoder;
+	resource_count[2] = args->num_connector;
 	ids_ptr = (uint32_t __user *)(unsigned long)args->id_list_ptr;
-	for (i = 0; i < total_ids; i++) {
-		if (get_user(new_minor->mode_group.id_list[i], &ids_ptr[i])) {
-			ret = -EFAULT;
-			goto out_del;
-		}
-	}
+	id_list = new_minor->mode_group.id_list;
+	ret = drm_get_render_node_resources(dev, id_list, ids_ptr,
+					    resource_count);
+	if (ret)
+		goto out_del;
 	new_minor->mode_group.num_crtcs = args->num_crtc;
 	new_minor->mode_group.num_encoders = args->num_encoder;
 	new_minor->mode_group.num_connectors = args->num_connector;
-- 
1.7.8.5

[PATCH 15/16] drm: keep track of which node holds which resource

[Ilija Hadzic]

Add fields to drm_crtc, drm_encoder and drm_connector that keep
track of which render node owns it. Assign ownership when resource
is added and revoke it when resource is destroyed. Do not allow
creation of a node that tries to claim resources that are already
in use by another node.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_crtc.c |    3 +
 drivers/gpu/drm/drm_stub.c |  104 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |    4 ++
 3 files changed, 111 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9af822b..199ef94 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -375,6 +375,7 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 
 	crtc->dev = dev;
 	crtc->funcs = funcs;
+	crtc->render_node_owner = -1;
 
 	mutex_lock(&dev->mode_config.mutex);
 
@@ -483,6 +484,7 @@ int drm_connector_init(struct drm_device *dev,
 
 	connector->dev = dev;
 	connector->funcs = funcs;
+	connector->render_node_owner = -1;
 	connector->connector_type = connector_type;
 	connector->connector_type_id =
 		++drm_connector_enum_list[connector_type].count; /* TODO */
@@ -553,6 +555,7 @@ int drm_encoder_init(struct drm_device *dev,
 	if (ret)
 		goto out;
 
+	encoder->render_node_owner = -1;
 	encoder->dev = dev;
 	encoder->encoder_type = encoder_type;
 	encoder->funcs = funcs;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 5b6459f..b025ad8 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -446,6 +446,90 @@ static int drm_get_render_node_resources(struct drm_device *dev,
 	return 0;
 }
 
+static int *drm_get_render_node_owner(struct drm_device *dev,
+				      uint32_t id, uint32_t type)
+{
+	struct drm_mode_object *obj;
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+
+	obj = drm_mode_object_find(dev, id, type);
+	if (!obj)
+		return NULL;
+	switch (type) {
+	case DRM_MODE_OBJECT_CRTC:
+		crtc = container_of(obj, struct drm_crtc, base);
+		return &crtc->render_node_owner;
+	case DRM_MODE_OBJECT_ENCODER:
+		encoder = container_of(obj, struct drm_encoder, base);
+		return &encoder->render_node_owner;
+	case DRM_MODE_OBJECT_CONNECTOR:
+		connector = container_of(obj, struct drm_connector, base);
+		return &connector->render_node_owner;
+	default:
+		return NULL;
+	}
+}
+
+static void drm_release_render_node_resources(struct drm_device *dev,
+					      uint32_t *id_list,
+					      int *resource_count,
+					      int minor)
+{
+	int *render_node_owner;
+	int s, e, i, j;
+
+	for (e = 0, j = 0; j < DRM_RN_NUM_EXP_TYPES; j++) {
+		s = e;
+		e += resource_count[j];
+		for (i = s; i < e; i++) {
+			render_node_owner =
+				drm_get_render_node_owner(dev, id_list[i],
+					expected_type_list[j]);
+			if (render_node_owner &&
+			    *render_node_owner == minor)
+				*render_node_owner = -1;
+		}
+	}
+}
+
+static int drm_claim_render_node_resources(struct drm_device *dev,
+					   uint32_t *id_list,
+					   int *resource_count,
+					   int minor)
+{
+	int *render_node_owner;
+	int s, e, i, j;
+	int ret = 0;
+
+	for (e = 0, j = 0; j < DRM_RN_NUM_EXP_TYPES; j++) {
+		s = e;
+		e += resource_count[j];
+		for (i = s; i < e; i++) {
+			render_node_owner =
+				drm_get_render_node_owner(dev, id_list[i],
+					expected_type_list[j]);
+			if (!render_node_owner) {
+				/* list was validated, not supposed to fail */
+				WARN_ON(1);
+				ret = -EFAULT;
+				goto out_release;
+			}
+			if (*render_node_owner != -1) {
+				ret = -EBUSY;
+				goto out_release;
+			}
+			*render_node_owner = minor;
+		}
+	}
+	return ret;
+
+out_release:
+	drm_release_render_node_resources(dev, id_list, resource_count, minor);
+	return ret;
+}
+
 int drm_create_render_node(struct drm_device *dev, struct drm_minor **minor_p)
 {
 	int ret;
@@ -474,10 +558,18 @@ int drm_destroy_render_node(struct drm_device *dev, int index)
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		if (node->minor->index == index) {
 			struct drm_mode_group *group;
+			int resource_count[DRM_RN_NUM_EXP_TYPES];
+
 			if (node->minor->open_count)
 				return -EBUSY;
 			group = &node->minor->mode_group;
 			list_del(&node->list);
+			resource_count[0] = group->num_crtcs;
+			resource_count[1] = group->num_encoders;
+			resource_count[2] = group->num_connectors;
+			drm_release_render_node_resources(dev, group->id_list,
+							  resource_count,
+							  node->minor->index);
 			drm_put_minor(&node->minor);
 			drm_mode_group_fini(group);
 			kfree(node);
@@ -493,8 +585,16 @@ void drm_destroy_all_render_nodes(struct drm_device *dev)
 
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		struct drm_mode_group *group;
+		int resource_count[DRM_RN_NUM_EXP_TYPES];
+
 		group = &node->minor->mode_group;
 		list_del(&node->list);
+		resource_count[0] = group->num_crtcs;
+		resource_count[1] = group->num_encoders;
+		resource_count[2] = group->num_connectors;
+		drm_release_render_node_resources(dev, group->id_list,
+						  resource_count,
+						  node->minor->index);
 		drm_put_minor(&node->minor);
 		drm_mode_group_fini(group);
 		kfree(node);
@@ -649,6 +749,10 @@ int drm_render_node_create_ioctl(struct drm_device *dev, void *data,
 					    resource_count);
 	if (ret)
 		goto out_del;
+	ret = drm_claim_render_node_resources(dev, id_list, resource_count,
+					      new_minor->index);
+	if (ret)
+		goto out_del;
 	new_minor->mode_group.num_crtcs = args->num_crtc;
 	new_minor->mode_group.num_encoders = args->num_encoder;
 	new_minor->mode_group.num_connectors = args->num_connector;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 86f4d9a..d4e6a8e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -374,6 +374,7 @@ struct drm_crtc {
 	struct drm_framebuffer *fb;
 
 	bool enabled;
+	int render_node_owner;
 
 	/* Requested mode from modesetting. */
 	struct drm_display_mode mode;
@@ -479,6 +480,8 @@ struct drm_encoder {
 	uint32_t possible_crtcs;
 	uint32_t possible_clones;
 
+	int render_node_owner;
+
 	struct drm_crtc *crtc;
 	const struct drm_encoder_funcs *funcs;
 	void *helper_private;
@@ -556,6 +559,7 @@ struct drm_connector {
 	struct list_head modes; /* list of modes on this connector */
 
 	enum drm_connector_status status;
+	int render_node_owner;
 
 	/* these are modes added by probing with DDC or the BIOS */
 	struct list_head probed_modes;
-- 
1.7.8.5

[PATCH 16/16] drm: hold mutex in critical sections of render-node code

[Ilija Hadzic]

Critical sections are parts of the code where we claim or
release resources (we don't want two render-node create or
remove ioctl called in the context of different processes
to claim part of requested resources because of the race).
Another critical section is manipulating the render node
list. We can use dev->struct_mutex for both.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_stub.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index b025ad8..8abfb08 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -480,6 +480,7 @@ static void drm_release_render_node_resources(struct drm_device *dev,
 	int *render_node_owner;
 	int s, e, i, j;
 
+	/* no lock, assume we were called with struct_mutex grabbed */
 	for (e = 0, j = 0; j < DRM_RN_NUM_EXP_TYPES; j++) {
 		s = e;
 		e += resource_count[j];
@@ -503,6 +504,7 @@ static int drm_claim_render_node_resources(struct drm_device *dev,
 	int s, e, i, j;
 	int ret = 0;
 
+	mutex_lock(&dev->struct_mutex);
 	for (e = 0, j = 0; j < DRM_RN_NUM_EXP_TYPES; j++) {
 		s = e;
 		e += resource_count[j];
@@ -523,10 +525,12 @@ static int drm_claim_render_node_resources(struct drm_device *dev,
 			*render_node_owner = minor;
 		}
 	}
+	mutex_unlock(&dev->struct_mutex);
 	return ret;
 
 out_release:
 	drm_release_render_node_resources(dev, id_list, resource_count, minor);
+	mutex_unlock(&dev->struct_mutex);
 	return ret;
 }
 
@@ -547,7 +551,9 @@ int drm_create_render_node(struct drm_device *dev, struct drm_minor **minor_p)
 	}
 	render_node->minor = minor;
 	*minor_p = minor;
+	mutex_lock(&dev->struct_mutex);
 	list_add_tail(&render_node->list, &dev->render_node_list);
+	mutex_unlock(&dev->struct_mutex);
 	return 0;
 }
 
@@ -555,13 +561,16 @@ int drm_destroy_render_node(struct drm_device *dev, int index)
 {
 	struct drm_render_node *node, *tmp;
 
+	mutex_lock(&dev->struct_mutex);
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		if (node->minor->index == index) {
 			struct drm_mode_group *group;
 			int resource_count[DRM_RN_NUM_EXP_TYPES];
 
-			if (node->minor->open_count)
+			if (node->minor->open_count) {
+				mutex_unlock(&dev->struct_mutex);
 				return -EBUSY;
+			}
 			group = &node->minor->mode_group;
 			list_del(&node->list);
 			resource_count[0] = group->num_crtcs;
@@ -570,12 +579,14 @@ int drm_destroy_render_node(struct drm_device *dev, int index)
 			drm_release_render_node_resources(dev, group->id_list,
 							  resource_count,
 							  node->minor->index);
+			mutex_unlock(&dev->struct_mutex);
 			drm_put_minor(&node->minor);
 			drm_mode_group_fini(group);
 			kfree(node);
 			return 0;
 		}
 	}
+	mutex_unlock(&dev->struct_mutex);
 	return -ENODEV;
 }
 
@@ -583,6 +594,7 @@ void drm_destroy_all_render_nodes(struct drm_device *dev)
 {
 	struct drm_render_node *node, *tmp;
 
+	mutex_lock(&dev->struct_mutex);
 	list_for_each_entry_safe(node, tmp, &dev->render_node_list, list) {
 		struct drm_mode_group *group;
 		int resource_count[DRM_RN_NUM_EXP_TYPES];
@@ -599,6 +611,7 @@ void drm_destroy_all_render_nodes(struct drm_device *dev)
 		drm_mode_group_fini(group);
 		kfree(node);
 	}
+	mutex_unlock(&dev->struct_mutex);
 }
 
 /**
-- 
1.7.8.5

Re: [RFC] Revive the work on render-nodes branch

[Ville Syrjälä]

On Thu, Mar 29, 2012 at 12:41:22PM -0400, Ilija Hadzic wrote:
> The following set of patches will revive the drm-render-nodes [1]
> branch that has been dormant in Dave Airlie's repository for some
> time. I rebased this branch to the latest drm-core-next and did some
> (hopefully useful) follow-up work.

The name "render node" is confusing. It makes me think there's no
modesetting stuff involved.

-- 
Ville Syrjälä
Intel OTC