All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Revive the work on render-nodes branch
@ 2012-03-29 16:41 Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 01/16] drm: do not push inode down into drm_open_helper Ilija Hadzic
                   ` (16 more replies)
  0 siblings, 17 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 01/16] drm: do not push inode down into drm_open_helper
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 02/16] drm: move dev_mapping to the minor node Ilija Hadzic
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 02/16] drm: move dev_mapping to the minor node
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 01/16] drm: do not push inode down into drm_open_helper Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 03/16] drm: add support for render nodes Ilija Hadzic
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 03/16] drm: add support for render nodes
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 01/16] drm: do not push inode down into drm_open_helper Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 02/16] drm: move dev_mapping to the minor node Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 04/16] drm: initial multiple nodes ioctl work Ilija Hadzic
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 04/16] drm: initial multiple nodes ioctl work.
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (2 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 03/16] drm: add support for render nodes Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 17:15   ` Ville Syrjälä
  2012-03-29 16:41 ` [PATCH 05/16] drm: separate render node descriptor from minor Ilija Hadzic
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel; +Cc: Dave Airlie

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

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

* [PATCH 05/16] drm: separate render node descriptor from minor
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (3 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 04/16] drm: initial multiple nodes ioctl work Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 06/16] drm: cleanup render node ioctls Ilija Hadzic
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 06/16] drm: cleanup render node ioctls
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (4 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 05/16] drm: separate render node descriptor from minor Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 07/16] drm: only allow render node ioctls through control node Ilija Hadzic
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 07/16] drm: only allow render node ioctls through control node
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (5 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 06/16] drm: cleanup render node ioctls Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 08/16] drm: do not remove a render node in use Ilija Hadzic
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 08/16] drm: do not remove a render node in use
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (6 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 07/16] drm: only allow render node ioctls through control node Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 09/16] drm: allocate correct id_list size for a render node Ilija Hadzic
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 09/16] drm: allocate correct id_list size for a render node
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (7 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 08/16] drm: do not remove a render node in use Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 10/16] drm: add drm_mode_group_fini function Ilija Hadzic
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 10/16] drm: add drm_mode_group_fini function
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (8 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 09/16] drm: allocate correct id_list size for a render node Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 11/16] drm: properly free id_list when a render node is removed Ilija Hadzic
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 11/16] drm: properly free id_list when a render node is removed
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (9 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 10/16] drm: add drm_mode_group_fini function Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 12/16] drm: call drm_mode_group_fini on primary node Ilija Hadzic
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 12/16] drm: call drm_mode_group_fini on primary node
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (10 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 11/16] drm: properly free id_list when a render node is removed Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 13/16] drm: more elaborate check for num_crtc/encoder/connector Ilija Hadzic
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 13/16] drm: more elaborate check for num_crtc/encoder/connector
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (11 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 12/16] drm: call drm_mode_group_fini on primary node Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 14/16] drm: validate id list when creating a render node Ilija Hadzic
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 14/16] drm: validate id list when creating a render node
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (12 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 13/16] drm: more elaborate check for num_crtc/encoder/connector Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 15/16] drm: keep track of which node holds which resource Ilija Hadzic
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 15/16] drm: keep track of which node holds which resource
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (13 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 14/16] drm: validate id list when creating a render node Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 16:41 ` [PATCH 16/16] drm: hold mutex in critical sections of render-node code Ilija Hadzic
  2012-03-29 17:16 ` [RFC] Revive the work on render-nodes branch Ville Syrjälä
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* [PATCH 16/16] drm: hold mutex in critical sections of render-node code
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (14 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 15/16] drm: keep track of which node holds which resource Ilija Hadzic
@ 2012-03-29 16:41 ` Ilija Hadzic
  2012-03-29 17:16 ` [RFC] Revive the work on render-nodes branch Ville Syrjälä
  16 siblings, 0 replies; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 16:41 UTC (permalink / raw)
  To: dri-devel

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

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

* Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.
  2012-03-29 16:41 ` [PATCH 04/16] drm: initial multiple nodes ioctl work Ilija Hadzic
@ 2012-03-29 17:15   ` Ville Syrjälä
  2012-03-29 17:22     ` Ilija Hadzic
  0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2012-03-29 17:15 UTC (permalink / raw)
  To: Ilija Hadzic; +Cc: Dave Airlie, dri-devel

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

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

* Re: [RFC] Revive the work on render-nodes branch
  2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
                   ` (15 preceding siblings ...)
  2012-03-29 16:41 ` [PATCH 16/16] drm: hold mutex in critical sections of render-node code Ilija Hadzic
@ 2012-03-29 17:16 ` Ville Syrjälä
  16 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2012-03-29 17:16 UTC (permalink / raw)
  To: Ilija Hadzic; +Cc: dri-devel

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

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

* Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.
  2012-03-29 17:15   ` Ville Syrjälä
@ 2012-03-29 17:22     ` Ilija Hadzic
  2012-03-29 18:01       ` Ville Syrjälä
  0 siblings, 1 reply; 21+ messages in thread
From: Ilija Hadzic @ 2012-03-29 17:22 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Dave Airlie, dri-devel

[-- 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

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

* Re: [PATCH 04/16] drm: initial multiple nodes ioctl work.
  2012-03-29 17:22     ` Ilija Hadzic
@ 2012-03-29 18:01       ` Ville Syrjälä
  0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2012-03-29 18:01 UTC (permalink / raw)
  To: Ilija Hadzic; +Cc: Dave Airlie, dri-devel

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

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

end of thread, other threads:[~2012-03-29 17:53 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-29 16:41 [RFC] Revive the work on render-nodes branch Ilija Hadzic
2012-03-29 16:41 ` [PATCH 01/16] drm: do not push inode down into drm_open_helper Ilija Hadzic
2012-03-29 16:41 ` [PATCH 02/16] drm: move dev_mapping to the minor node Ilija Hadzic
2012-03-29 16:41 ` [PATCH 03/16] drm: add support for render nodes Ilija Hadzic
2012-03-29 16:41 ` [PATCH 04/16] drm: initial multiple nodes ioctl work Ilija Hadzic
2012-03-29 17:15   ` Ville Syrjälä
2012-03-29 17:22     ` Ilija Hadzic
2012-03-29 18:01       ` Ville Syrjälä
2012-03-29 16:41 ` [PATCH 05/16] drm: separate render node descriptor from minor Ilija Hadzic
2012-03-29 16:41 ` [PATCH 06/16] drm: cleanup render node ioctls Ilija Hadzic
2012-03-29 16:41 ` [PATCH 07/16] drm: only allow render node ioctls through control node Ilija Hadzic
2012-03-29 16:41 ` [PATCH 08/16] drm: do not remove a render node in use Ilija Hadzic
2012-03-29 16:41 ` [PATCH 09/16] drm: allocate correct id_list size for a render node Ilija Hadzic
2012-03-29 16:41 ` [PATCH 10/16] drm: add drm_mode_group_fini function Ilija Hadzic
2012-03-29 16:41 ` [PATCH 11/16] drm: properly free id_list when a render node is removed Ilija Hadzic
2012-03-29 16:41 ` [PATCH 12/16] drm: call drm_mode_group_fini on primary node Ilija Hadzic
2012-03-29 16:41 ` [PATCH 13/16] drm: more elaborate check for num_crtc/encoder/connector Ilija Hadzic
2012-03-29 16:41 ` [PATCH 14/16] drm: validate id list when creating a render node Ilija Hadzic
2012-03-29 16:41 ` [PATCH 15/16] drm: keep track of which node holds which resource Ilija Hadzic
2012-03-29 16:41 ` [PATCH 16/16] drm: hold mutex in critical sections of render-node code Ilija Hadzic
2012-03-29 17:16 ` [RFC] Revive the work on render-nodes branch Ville Syrjälä

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.