dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@gmail.com>
To: dri-devel@lists.freedesktop.org
Subject: [RFC 2/9] drm: merge device setup into drm_dev_register()
Date: Wed, 24 Jul 2013 15:43:51 +0200	[thread overview]
Message-ID: <1374673438-26251-3-git-send-email-dh.herrmann@gmail.com> (raw)
In-Reply-To: <1374673438-26251-1-git-send-email-dh.herrmann@gmail.com>

All bus drivers do device setup themselves. This requires us to adjust all
of them if we introduce new core features. Thus, merge all these into a
uniform drm_dev_register() helper.

Note that this removes the drm_lastclose() error path for AGP as it is
horribly broken. Moreover, no bus driver called this in any other error
path either. Follow up patches will fix AGP error paths.

We also keep a DRIVER_MODESET condition around pci_set_drvdata() to keep
semantics.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_pci.c      | 47 ++++------------------
 drivers/gpu/drm/drm_platform.c | 45 ++-------------------
 drivers/gpu/drm/drm_stub.c     | 91 +++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/drm_usb.c      | 39 ++----------------
 include/drm/drmP.h             |  4 +-
 5 files changed, 86 insertions(+), 140 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index fa1de13..ab861ca 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -318,7 +318,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 
 	ret = pci_enable_device(pdev);
 	if (ret)
-		goto err_g1;
+		goto err_free;
 
 	dev->pdev = pdev;
 	dev->pci_device = pdev->device;
@@ -328,56 +328,23 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	dev->hose = pdev->sysdata;
 #endif
 
-	mutex_lock(&drm_global_mutex);
-
-	if ((ret = drm_fill_in_dev(dev, ent, driver))) {
-		printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
-		goto err_g2;
-	}
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		pci_set_drvdata(pdev, dev);
-		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
-		if (ret)
-			goto err_g2;
-	}
-
-	if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY)))
-		goto err_g3;
 
-	if (dev->driver->load) {
-		ret = dev->driver->load(dev, ent->driver_data);
-		if (ret)
-			goto err_g4;
-	}
-
-	/* setup the grouping for the legacy output */
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		ret = drm_mode_group_init_legacy_group(dev,
-						&dev->primary->mode_group);
-		if (ret)
-			goto err_g4;
-	}
-
-	list_add_tail(&dev->driver_item, &driver->device_list);
+	ret = drm_dev_register(dev);
+	if (ret)
+		goto err_pci;
 
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, pci_name(pdev), dev->primary->index);
 
-	mutex_unlock(&drm_global_mutex);
 	return 0;
 
-err_g4:
-	drm_put_minor(&dev->primary);
-err_g3:
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		drm_put_minor(&dev->control);
-err_g2:
+err_pci:
 	pci_disable_device(pdev);
-err_g1:
+err_free:
 	kfree(dev);
-	mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_get_pci_dev);
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index f7cfbb5..396713e 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -53,42 +53,9 @@ int drm_get_platform_dev(struct platform_device *platdev,
 
 	dev->platformdev = platdev;
 
-	mutex_lock(&drm_global_mutex);
-
-	ret = drm_fill_in_dev(dev, NULL, driver);
-
-	if (ret) {
-		printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
-		goto err_g1;
-	}
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
-		if (ret)
-			goto err_g1;
-	}
-
-	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
+	ret = drm_dev_register(dev);
 	if (ret)
-		goto err_g2;
-
-	if (dev->driver->load) {
-		ret = dev->driver->load(dev, 0);
-		if (ret)
-			goto err_g3;
-	}
-
-	/* setup the grouping for the legacy output */
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		ret = drm_mode_group_init_legacy_group(dev,
-				&dev->primary->mode_group);
-		if (ret)
-			goto err_g3;
-	}
-
-	list_add_tail(&dev->driver_item, &driver->device_list);
-
-	mutex_unlock(&drm_global_mutex);
+		goto err_free;
 
 	DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
@@ -96,14 +63,8 @@ int drm_get_platform_dev(struct platform_device *platdev,
 
 	return 0;
 
-err_g3:
-	drm_put_minor(&dev->primary);
-err_g2:
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		drm_put_minor(&dev->control);
-err_g1:
+err_free:
 	kfree(dev);
-	mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_get_platform_dev);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 7b37906..bc1d860 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -250,25 +250,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
-int drm_fill_in_dev(struct drm_device *dev,
-			   const struct pci_device_id *ent,
-			   struct drm_driver *driver)
-{
-	int retcode;
-
-	if (dev->driver->bus->agp_init) {
-		retcode = dev->driver->bus->agp_init(dev);
-		if (retcode) {
-			drm_lastclose(dev);
-			return retcode;
-		}
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_fill_in_dev);
-
-
 /**
  * Get a secondary minor number.
  *
@@ -463,6 +444,8 @@ EXPORT_SYMBOL(drm_unplug_dev);
  * @parent: Parent device object
  *
  * Allocate and initialize a new DRM device. No device registration is done.
+ * Call drm_dev_register() to advertice the device to user space and register it
+ * with other core subsystems.
  *
  * RETURNS:
  * Pointer to new DRM device, or NULL if out of memory.
@@ -528,3 +511,73 @@ err_free:
 	return NULL;
 }
 EXPORT_SYMBOL(drm_dev_alloc);
+
+/**
+ * drm_dev_register - Register DRM device
+ * @dev: Device to register
+ *
+ * Register the DRM device @dev with the system, advertise device to user-space
+ * and start normal device operation. @dev must be allocated via drm_dev_alloc()
+ * previously.
+ *
+ * Never call this twice on any device!
+ *
+ * RETURNS:
+ * 0 on success, negative error code on failure.
+ */
+int drm_dev_register(struct drm_device *dev)
+{
+	int ret;
+
+	mutex_lock(&drm_global_mutex);
+
+	if (dev->driver->bus->agp_init) {
+		ret = dev->driver->bus->agp_init(dev);
+		if (ret)
+			goto out_unlock;
+	}
+
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
+		if (ret)
+			goto err_agp;
+	}
+
+	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
+	if (ret)
+		goto err_control_node;
+
+	if (dev->driver->load) {
+		ret = dev->driver->load(dev, 0);
+		if (ret)
+			goto err_primary_node;
+	}
+
+	/* setup grouping for legacy outputs */
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		ret = drm_mode_group_init_legacy_group(dev,
+				&dev->primary->mode_group);
+		if (ret)
+			goto err_unload;
+	}
+
+	list_add_tail(&dev->driver_item, &dev->driver->device_list);
+
+	ret = 0;
+	goto out_unlock;
+
+err_unload:
+	if (dev->driver->unload)
+		dev->driver->unload(dev);
+err_primary_node:
+	drm_put_minor(&dev->primary);
+err_control_node:
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
+		drm_put_minor(&dev->control);
+err_agp:
+	/* FIXME: cleanup AGP leftovers */
+out_unlock:
+	mutex_unlock(&drm_global_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(drm_dev_register);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index 7f016e4..54d60f2 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -16,39 +16,11 @@ int drm_get_usb_dev(struct usb_interface *interface,
 		return -ENOMEM;
 
 	dev->usbdev = interface_to_usbdev(interface);
-
-	mutex_lock(&drm_global_mutex);
-
-	ret = drm_fill_in_dev(dev, NULL, driver);
-	if (ret) {
-		printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
-		goto err_g1;
-	}
-
 	usb_set_intfdata(interface, dev);
-	ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
-	if (ret)
-		goto err_g1;
 
-	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
+	ret = drm_dev_register(dev);
 	if (ret)
-		goto err_g2;
-
-	if (dev->driver->load) {
-		ret = dev->driver->load(dev, 0);
-		if (ret)
-			goto err_g3;
-	}
-
-	/* setup the grouping for the legacy output */
-	ret = drm_mode_group_init_legacy_group(dev,
-					       &dev->primary->mode_group);
-	if (ret)
-		goto err_g3;
-
-	list_add_tail(&dev->driver_item, &driver->device_list);
-
-	mutex_unlock(&drm_global_mutex);
+		goto err_free;
 
 	DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
@@ -56,13 +28,8 @@ int drm_get_usb_dev(struct usb_interface *interface,
 
 	return 0;
 
-err_g3:
-	drm_put_minor(&dev->primary);
-err_g2:
-	drm_put_minor(&dev->control);
-err_g1:
+err_free:
 	kfree(dev);
-	mutex_unlock(&drm_global_mutex);
 	return ret;
 
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b1f00c5..ea5e130 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1702,11 +1702,9 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map)
 
 #include <drm/drm_mem_util.h>
 
-extern int drm_fill_in_dev(struct drm_device *dev,
-			   const struct pci_device_id *ent,
-			   struct drm_driver *driver);
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 				 struct device *parent);
+int drm_dev_register(struct drm_device *dev);
 int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
 /*@}*/
 
-- 
1.8.3.3

  parent reply	other threads:[~2013-07-24 13:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-24 13:43 [RFC 0/9] DRM: Device Handling Cleanup David Herrmann
2013-07-24 13:43 ` [RFC 1/9] drm: add drm_dev_alloc() helper David Herrmann
2013-07-24 13:43 ` David Herrmann [this message]
2013-07-24 13:43 ` [RFC 3/9] drm/agp: fix AGP cleanup paths David Herrmann
2013-07-27  6:05   ` Dave Airlie
2013-07-27 12:09     ` David Herrmann
2013-07-24 13:43 ` [RFC 4/9] drm: move drm_lastclose() to drm_fops.c David Herrmann
2013-07-24 13:43 ` [RFC 5/9] drm: introduce drm_dev_free() to fix error paths David Herrmann
2013-07-24 13:43 ` [RFC 6/9] drm: move device unregistration into drm_dev_unregister() David Herrmann
2013-07-24 13:43 ` [RFC 7/9] percpu_rw_sempahore: export symbols for modules David Herrmann
2013-07-24 13:43 ` [RFC 8/9] drm: track pending user-space actions David Herrmann
2013-07-24 14:03   ` Maarten Lankhorst
2013-07-24 14:24     ` David Herrmann
2013-07-24 14:45       ` Maarten Lankhorst
2013-07-24 13:43 ` [RFC 9/9] drm: support immediate unplugging David Herrmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1374673438-26251-3-git-send-email-dh.herrmann@gmail.com \
    --to=dh.herrmann@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox