[PATCH 03/12] drm: call ->firstopen() before ->open()

David Herrmann dh.herrmann at gmail.com
Wed Jul 23 08:26:38 PDT 2014


Lets order things correctly:
 ->load()
   ->fistopen()
     ->open()
     ->close()
   ->lastclose()
 ->unload()

This doesn't change much as only savage and radeon use ->firstopen() and
they just do map-initialization. Therefore, the global drm mutex makes
sure there cannot be any other f_op between ->open() and ->firstopen().
However, once we get rid of that lock, we really want ->firstopen() to
initialize the device before ->open() is called.

Furthermore, this fixes the clean-up path in drm_open(). We currently
don't cleanup the drm_file object if ->firstopen() fails.

Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
---
 drivers/gpu/drm/drm_fops.c | 139 +++++++++++++++++++++------------------------
 include/drm/drmP.h         |   2 +-
 2 files changed, 66 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 021fe5d..8e73519 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -45,7 +45,7 @@ EXPORT_SYMBOL(drm_global_mutex);
 
 static int drm_open_helper(struct file *filp, struct drm_minor *minor);
 
-static int drm_setup(struct drm_device * dev)
+static int drm_firstopen(struct drm_device * dev)
 {
 	int ret;
 
@@ -66,6 +66,57 @@ static int drm_setup(struct drm_device * dev)
 }
 
 /**
+ * drm_legacy_dev_reinit
+ *
+ * Reinitializes a legacy/ums drm device in it's lastclose function.
+ */
+static void drm_legacy_dev_reinit(struct drm_device *dev)
+{
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
+		return;
+
+	dev->sigdata.lock = NULL;
+
+	dev->context_flag = 0;
+	dev->last_context = 0;
+	dev->if_version = 0;
+}
+
+void drm_lastclose(struct drm_device * dev)
+{
+	struct drm_vma_entry *vma, *vma_temp;
+
+	DRM_DEBUG("\n");
+
+	if (dev->driver->lastclose)
+		dev->driver->lastclose(dev);
+	DRM_DEBUG("driver lastclose completed\n");
+
+	if (dev->irq_enabled && !drm_core_check_feature(dev, DRIVER_MODESET))
+		drm_irq_uninstall(dev);
+
+	mutex_lock(&dev->struct_mutex);
+
+	drm_agp_clear(dev);
+
+	drm_legacy_sg_cleanup(dev);
+
+	/* Clear vma list (only built for debugging) */
+	list_for_each_entry_safe(vma, vma_temp, &dev->vmalist, head) {
+		list_del(&vma->head);
+		kfree(vma);
+	}
+
+	drm_legacy_dma_takedown(dev);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	drm_legacy_dev_reinit(dev);
+
+	DRM_DEBUG("lastclose completed\n");
+}
+
+/**
  * Open file.
  *
  * \param inode device inode
@@ -81,31 +132,33 @@ int drm_open(struct inode *inode, struct file *filp)
 	struct drm_device *dev;
 	struct drm_minor *minor;
 	int retcode;
-	int need_setup = 0;
 
 	minor = drm_minor_acquire(iminor(inode));
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
 	dev = minor->dev;
-	if (!dev->open_count++)
-		need_setup = 1;
 
 	/* share address_space across all char-devs of a single device */
 	filp->f_mapping = dev->anon_inode->i_mapping;
 
+	if (!dev->open_count) {
+		retcode = drm_firstopen(dev);
+		if (retcode)
+			goto err_minor;
+	}
+	++dev->open_count;
+
 	retcode = drm_open_helper(filp, minor);
 	if (retcode)
 		goto err_undo;
-	if (need_setup) {
-		retcode = drm_setup(dev);
-		if (retcode)
-			goto err_undo;
-	}
+
 	return 0;
 
 err_undo:
-	dev->open_count--;
+	if (!--dev->open_count)
+		drm_lastclose(dev);
+err_minor:
 	drm_minor_release(minor);
 	return retcode;
 }
@@ -346,67 +399,6 @@ static void drm_events_release(struct drm_file *file_priv)
 }
 
 /**
- * drm_legacy_dev_reinit
- *
- * Reinitializes a legacy/ums drm device in it's lastclose function.
- */
-static void drm_legacy_dev_reinit(struct drm_device *dev)
-{
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		return;
-
-	dev->sigdata.lock = NULL;
-
-	dev->context_flag = 0;
-	dev->last_context = 0;
-	dev->if_version = 0;
-}
-
-/**
- * Take down the DRM device.
- *
- * \param dev DRM device structure.
- *
- * Frees every resource in \p dev.
- *
- * \sa drm_device
- */
-int drm_lastclose(struct drm_device * dev)
-{
-	struct drm_vma_entry *vma, *vma_temp;
-
-	DRM_DEBUG("\n");
-
-	if (dev->driver->lastclose)
-		dev->driver->lastclose(dev);
-	DRM_DEBUG("driver lastclose completed\n");
-
-	if (dev->irq_enabled && !drm_core_check_feature(dev, DRIVER_MODESET))
-		drm_irq_uninstall(dev);
-
-	mutex_lock(&dev->struct_mutex);
-
-	drm_agp_clear(dev);
-
-	drm_legacy_sg_cleanup(dev);
-
-	/* Clear vma list (only built for debugging) */
-	list_for_each_entry_safe(vma, vma_temp, &dev->vmalist, head) {
-		list_del(&vma->head);
-		kfree(vma);
-	}
-
-	drm_legacy_dma_takedown(dev);
-
-	mutex_unlock(&dev->struct_mutex);
-
-	drm_legacy_dev_reinit(dev);
-
-	DRM_DEBUG("lastclose completed\n");
-	return 0;
-}
-
-/**
  * Release file.
  *
  * \param inode device inode
@@ -423,7 +415,6 @@ int drm_release(struct inode *inode, struct file *filp)
 	struct drm_file *file_priv = filp->private_data;
 	struct drm_minor *minor = file_priv->minor;
 	struct drm_device *dev = minor->dev;
-	int retcode = 0;
 
 	mutex_lock(&drm_global_mutex);
 
@@ -541,7 +532,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	 */
 
 	if (!--dev->open_count) {
-		retcode = drm_lastclose(dev);
+		drm_lastclose(dev);
 		if (drm_device_is_unplugged(dev))
 			drm_put_dev(dev);
 	}
@@ -549,7 +540,7 @@ int drm_release(struct inode *inode, struct file *filp)
 
 	drm_minor_release(minor);
 
-	return retcode;
+	return 0;
 }
 EXPORT_SYMBOL(drm_release);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d3d9be6..d1730c5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1181,7 +1181,7 @@ extern long drm_ioctl(struct file *filp,
 		      unsigned int cmd, unsigned long arg);
 extern long drm_compat_ioctl(struct file *filp,
 			     unsigned int cmd, unsigned long arg);
-extern int drm_lastclose(struct drm_device *dev);
+extern void drm_lastclose(struct drm_device *dev);
 extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
 
 				/* Device support (drm_fops.h) */
-- 
2.0.2



More information about the dri-devel mailing list