* [PATCH 1/5] drm: add drm_dev_alloc() helper
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
@ 2013-10-02 9:23 ` David Herrmann
2013-10-02 9:23 ` [PATCH 2/5] drm: merge device setup into drm_dev_register() David Herrmann
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: David Herrmann @ 2013-10-02 9:23 UTC (permalink / raw)
To: dri-devel
Instead of managing device allocation+initialization in each bus-driver,
we should do that in a central place. drm_fill_in_dev() already does most
of it, but also requires the global drm lock for partial AGP device
registration.
Split both apart so we have a clean device initialization/allocation
phase, and a registration phase.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_pci.c | 4 +-
drivers/gpu/drm/drm_platform.c | 3 +-
drivers/gpu/drm/drm_stub.c | 121 +++++++++++++++++++++++++----------------
drivers/gpu/drm/drm_usb.c | 7 +--
include/drm/drmP.h | 2 +
5 files changed, 80 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 1f96cee..d2758be 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -322,7 +322,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
DRM_DEBUG("\n");
- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ dev = drm_dev_alloc(driver, &pdev->dev);
if (!dev)
return -ENOMEM;
@@ -331,8 +331,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
goto err_g1;
dev->pdev = pdev;
- dev->dev = &pdev->dev;
-
dev->pci_device = pdev->device;
dev->pci_vendor = pdev->vendor;
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index f7a18c6..fb27217 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -47,12 +47,11 @@ static int drm_get_platform_dev(struct platform_device *platdev,
DRM_DEBUG("\n");
- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ dev = drm_dev_alloc(driver, &platdev->dev);
if (!dev)
return -ENOMEM;
dev->platformdev = platdev;
- dev->dev = &platdev->dev;
mutex_lock(&drm_global_mutex);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 39d8645..64bd52f 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -260,60 +260,15 @@ int drm_fill_in_dev(struct drm_device *dev,
{
int retcode;
- INIT_LIST_HEAD(&dev->filelist);
- INIT_LIST_HEAD(&dev->ctxlist);
- INIT_LIST_HEAD(&dev->vmalist);
- INIT_LIST_HEAD(&dev->maplist);
- INIT_LIST_HEAD(&dev->vblank_event_list);
-
- spin_lock_init(&dev->count_lock);
- spin_lock_init(&dev->event_lock);
- mutex_init(&dev->struct_mutex);
- mutex_init(&dev->ctxlist_mutex);
-
- if (drm_ht_create(&dev->map_hash, 12)) {
- return -ENOMEM;
- }
-
- /* the DRM has 6 basic counters */
- dev->counters = 6;
- dev->types[0] = _DRM_STAT_LOCK;
- dev->types[1] = _DRM_STAT_OPENS;
- dev->types[2] = _DRM_STAT_CLOSES;
- dev->types[3] = _DRM_STAT_IOCTLS;
- dev->types[4] = _DRM_STAT_LOCKS;
- dev->types[5] = _DRM_STAT_UNLOCKS;
-
- dev->driver = driver;
-
if (dev->driver->bus->agp_init) {
retcode = dev->driver->bus->agp_init(dev);
- if (retcode)
- goto error_out_unreg;
- }
-
-
-
- retcode = drm_ctxbitmap_init(dev);
- if (retcode) {
- DRM_ERROR("Cannot allocate memory for context bitmap.\n");
- goto error_out_unreg;
- }
-
- if (driver->driver_features & DRIVER_GEM) {
- retcode = drm_gem_init(dev);
if (retcode) {
- DRM_ERROR("Cannot initialize graphics execution "
- "manager (GEM)\n");
- goto error_out_unreg;
+ drm_lastclose(dev);
+ return retcode;
}
}
return 0;
-
- error_out_unreg:
- drm_lastclose(dev);
- return retcode;
}
EXPORT_SYMBOL(drm_fill_in_dev);
@@ -490,3 +445,75 @@ void drm_unplug_dev(struct drm_device *dev)
mutex_unlock(&drm_global_mutex);
}
EXPORT_SYMBOL(drm_unplug_dev);
+
+/**
+ * drm_dev_alloc - Allocate new drm device
+ * @driver: DRM driver to allocate device for
+ * @parent: Parent device object
+ *
+ * Allocate and initialize a new DRM device. No device registration is done.
+ *
+ * RETURNS:
+ * Pointer to new DRM device, or NULL if out of memory.
+ */
+struct drm_device *drm_dev_alloc(struct drm_driver *driver,
+ struct device *parent)
+{
+ struct drm_device *dev;
+ int ret;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return NULL;
+
+ dev->dev = parent;
+ dev->driver = driver;
+
+ INIT_LIST_HEAD(&dev->filelist);
+ INIT_LIST_HEAD(&dev->ctxlist);
+ INIT_LIST_HEAD(&dev->vmalist);
+ INIT_LIST_HEAD(&dev->maplist);
+ INIT_LIST_HEAD(&dev->vblank_event_list);
+
+ spin_lock_init(&dev->count_lock);
+ spin_lock_init(&dev->event_lock);
+ mutex_init(&dev->struct_mutex);
+ mutex_init(&dev->ctxlist_mutex);
+
+ /* the DRM has 6 basic counters */
+ dev->counters = 6;
+ dev->types[0] = _DRM_STAT_LOCK;
+ dev->types[1] = _DRM_STAT_OPENS;
+ dev->types[2] = _DRM_STAT_CLOSES;
+ dev->types[3] = _DRM_STAT_IOCTLS;
+ dev->types[4] = _DRM_STAT_LOCKS;
+ dev->types[5] = _DRM_STAT_UNLOCKS;
+
+ if (drm_ht_create(&dev->map_hash, 12))
+ goto err_free;
+
+ ret = drm_ctxbitmap_init(dev);
+ if (ret) {
+ DRM_ERROR("Cannot allocate memory for context bitmap.\n");
+ goto err_ht;
+ }
+
+ if (driver->driver_features & DRIVER_GEM) {
+ ret = drm_gem_init(dev);
+ if (ret) {
+ DRM_ERROR("Cannot initialize graphics execution manager (GEM)\n");
+ goto err_ctxbitmap;
+ }
+ }
+
+ return dev;
+
+err_ctxbitmap:
+ drm_ctxbitmap_cleanup(dev);
+err_ht:
+ drm_ht_remove(&dev->map_hash);
+err_free:
+ kfree(dev);
+ return NULL;
+}
+EXPORT_SYMBOL(drm_dev_alloc);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index 8766472..34ad8ed 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -7,18 +7,15 @@ int drm_get_usb_dev(struct usb_interface *interface,
struct drm_driver *driver)
{
struct drm_device *dev;
- struct usb_device *usbdev;
int ret;
DRM_DEBUG("\n");
- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ dev = drm_dev_alloc(driver, &interface->dev);
if (!dev)
return -ENOMEM;
- usbdev = interface_to_usbdev(interface);
- dev->usbdev = usbdev;
- dev->dev = &interface->dev;
+ dev->usbdev = interface_to_usbdev(interface);
mutex_lock(&drm_global_mutex);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b46fb45..1d4c1d9 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1648,6 +1648,8 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map)
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_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
/*@}*/
--
1.8.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/5] drm: merge device setup into drm_dev_register()
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
2013-10-02 9:23 ` [PATCH 1/5] drm: add drm_dev_alloc() helper David Herrmann
@ 2013-10-02 9:23 ` David Herrmann
2013-10-03 13:15 ` Daniel Vetter
2013-10-02 9:23 ` [PATCH 3/5] drm: move drm_lastclose() to drm_fops.c David Herrmann
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: David Herrmann @ 2013-10-02 9:23 UTC (permalink / raw)
To: dri-devel
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. Instead, we use the recently introduced AGP cleanup helpers.
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 | 56 +++--------------------
drivers/gpu/drm/drm_platform.c | 54 ++--------------------
drivers/gpu/drm/drm_stub.c | 101 +++++++++++++++++++++++++++++++++--------
drivers/gpu/drm/drm_usb.c | 48 ++------------------
include/drm/drmP.h | 4 +-
5 files changed, 96 insertions(+), 167 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index d2758be..ffea035 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -328,7 +328,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;
@@ -338,65 +338,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 (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
- ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
- if (ret)
- goto err_g21;
- }
- 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 (dev->render)
- drm_put_minor(&dev->render);
-err_g21:
- 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 fb27217..a0e402e 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -53,48 +53,9 @@ static 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;
- }
-
- if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
- ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
- if (ret)
- goto err_g11;
- }
-
- 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,
@@ -102,17 +63,8 @@ static int drm_get_platform_dev(struct platform_device *platdev,
return 0;
-err_g3:
- drm_put_minor(&dev->primary);
-err_g2:
- if (dev->render)
- drm_put_minor(&dev->render);
-err_g11:
- 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;
}
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 64bd52f..4c8b7d8 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -254,25 +254,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.
*
@@ -452,6 +433,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.
@@ -517,3 +500,83 @@ 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;
+ }
+
+ if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+ ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+ if (ret)
+ goto err_control_node;
+ }
+
+ ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
+ if (ret)
+ goto err_render_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_render_node:
+ if (dev->render)
+ drm_put_minor(&dev->render);
+err_control_node:
+ if (dev->control)
+ drm_put_minor(&dev->control);
+err_agp:
+ if (dev->driver->bus->agp_destroy)
+ dev->driver->bus->agp_destroy(dev);
+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 34ad8ed..54d60f2 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -16,45 +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;
-
- if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
- ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
- if (ret)
- goto err_g11;
- }
- 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,
@@ -62,16 +28,8 @@ int drm_get_usb_dev(struct usb_interface *interface,
return 0;
-err_g3:
- drm_put_minor(&dev->primary);
-err_g2:
- if (dev->render)
- drm_put_minor(&dev->render);
-err_g11:
- 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 1d4c1d9..88e17b3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1645,11 +1645,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.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] drm: merge device setup into drm_dev_register()
2013-10-02 9:23 ` [PATCH 2/5] drm: merge device setup into drm_dev_register() David Herrmann
@ 2013-10-03 13:15 ` Daniel Vetter
2013-10-03 13:19 ` David Herrmann
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2013-10-03 13:15 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel
On Wed, Oct 02, 2013 at 11:23:35AM +0200, David Herrmann wrote:
> 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. Instead, we use the recently introduced AGP cleanup helpers.
>
> We also keep a DRIVER_MODESET condition around pci_set_drvdata() to keep
> semantics.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
[snip]
> +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;
> + }
Imo this should stay in drm_get_pci_dev since its pci specific - no other
bus type should ever bother with this really.
Looks good otherwise.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] drm: merge device setup into drm_dev_register()
2013-10-03 13:15 ` Daniel Vetter
@ 2013-10-03 13:19 ` David Herrmann
2013-10-03 13:21 ` Daniel Vetter
0 siblings, 1 reply; 13+ messages in thread
From: David Herrmann @ 2013-10-03 13:19 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel@lists.freedesktop.org
Hi Daniel
On Thu, Oct 3, 2013 at 3:15 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 02, 2013 at 11:23:35AM +0200, David Herrmann wrote:
>> 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. Instead, we use the recently introduced AGP cleanup helpers.
>>
>> We also keep a DRIVER_MODESET condition around pci_set_drvdata() to keep
>> semantics.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>
> [snip]
>
>> +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;
>> + }
>
> Imo this should stay in drm_get_pci_dev since its pci specific - no other
> bus type should ever bother with this really.
You mean I should _move_ it to drm_pci.c? It has never been
pci-specific. But sure, I can do that. It actually makes sense. But
I'd like to do that in a followup patch as it's a semantic change
that's not directly related. I will include it in v2.
Thanks
David
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] drm: merge device setup into drm_dev_register()
2013-10-03 13:19 ` David Herrmann
@ 2013-10-03 13:21 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2013-10-03 13:21 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel@lists.freedesktop.org
On Thu, Oct 3, 2013 at 3:19 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> You mean I should _move_ it to drm_pci.c? It has never been
> pci-specific. But sure, I can do that. It actually makes sense. But
> I'd like to do that in a followup patch as it's a semantic change
> that's not directly related. I will include it in v2.
Oh right it's always been in fill_in_dev, but never should have been
there ;-) Follow-up patch like you've suggested it better.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] drm: move drm_lastclose() to drm_fops.c
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
2013-10-02 9:23 ` [PATCH 1/5] drm: add drm_dev_alloc() helper David Herrmann
2013-10-02 9:23 ` [PATCH 2/5] drm: merge device setup into drm_dev_register() David Herrmann
@ 2013-10-02 9:23 ` David Herrmann
2013-10-02 9:23 ` [PATCH 4/5] drm: introduce drm_dev_free() to fix error paths David Herrmann
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: David Herrmann @ 2013-10-02 9:23 UTC (permalink / raw)
To: dri-devel
Try to keep all functions that handle DRM file_operations in drm_fops.c
so internal helpers can be marked static later.
This makes the split between the 3 core files more obvious:
- drm_stub.c: DRM device allocation/destruction and management
- drm_fops.c: DRM file_operations (except for ioctl)
- drm_drv.c: Global DRM init + ioctl handling
Well, ioctl handling is still spread throughout hundreds of source files,
but at least the others are clearly defined this way.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_drv.c | 70 ----------------------------------------------
drivers/gpu/drm/drm_fops.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e572dd2..34d372e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -170,76 +170,6 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
-/**
- * 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)
-{
- int i;
-
- if (drm_core_check_feature(dev, DRIVER_MODESET))
- return;
-
- atomic_set(&dev->ioctl_count, 0);
- atomic_set(&dev->vma_count, 0);
-
- for (i = 0; i < ARRAY_SIZE(dev->counts); i++)
- atomic_set(&dev->counts[i], 0);
-
- 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);
-
- dev->dev_mapping = NULL;
- mutex_unlock(&dev->struct_mutex);
-
- drm_legacy_dev_reinit(dev);
-
- DRM_DEBUG("lastclose completed\n");
- return 0;
-}
-
/** File operations structure */
static const struct file_operations drm_stub_fops = {
.owner = THIS_MODULE,
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 3f84277..8bc94ea 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -386,6 +386,76 @@ 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)
+{
+ int i;
+
+ if (drm_core_check_feature(dev, DRIVER_MODESET))
+ return;
+
+ atomic_set(&dev->ioctl_count, 0);
+ atomic_set(&dev->vma_count, 0);
+
+ for (i = 0; i < ARRAY_SIZE(dev->counts); i++)
+ atomic_set(&dev->counts[i], 0);
+
+ 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);
+
+ dev->dev_mapping = NULL;
+ mutex_unlock(&dev->struct_mutex);
+
+ drm_legacy_dev_reinit(dev);
+
+ DRM_DEBUG("lastclose completed\n");
+ return 0;
+}
+
+/**
* Release file.
*
* \param inode device inode
--
1.8.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5] drm: introduce drm_dev_free() to fix error paths
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
` (2 preceding siblings ...)
2013-10-02 9:23 ` [PATCH 3/5] drm: move drm_lastclose() to drm_fops.c David Herrmann
@ 2013-10-02 9:23 ` David Herrmann
2013-10-02 9:23 ` [PATCH 5/5] drm: move device unregistration into drm_dev_unregister() David Herrmann
2013-10-09 5:31 ` [PATCH 0/5] DRM device-alloc cleanup Dave Airlie
5 siblings, 0 replies; 13+ messages in thread
From: David Herrmann @ 2013-10-02 9:23 UTC (permalink / raw)
To: dri-devel
The error paths in DRM bus drivers currently leak memory as they don't
correctly revert drm_dev_alloc(). Introduce drm_dev_free() to free DRM
devices which haven't been registered, yet.
We must be careful not to introduce any side-effects with cleanups done in
drm_dev_free(). drm_ht_remove(), drm_ctxbitmap_cleanup() and
drm_gem_destroy() are all fine in that regard.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_pci.c | 2 +-
drivers/gpu/drm/drm_platform.c | 2 +-
drivers/gpu/drm/drm_stub.c | 35 +++++++++++++++++++++++++----------
drivers/gpu/drm/drm_usb.c | 2 +-
include/drm/drmP.h | 1 +
5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index ffea035..f0dc4f9 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -354,7 +354,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
err_pci:
pci_disable_device(pdev);
err_free:
- kfree(dev);
+ drm_dev_free(dev);
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 a0e402e..6b37409 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -64,7 +64,7 @@ static int drm_get_platform_dev(struct platform_device *platdev,
return 0;
err_free:
- kfree(dev);
+ drm_dev_free(dev);
return ret;
}
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4c8b7d8..397ab90 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -363,7 +363,6 @@ static void drm_unplug_minor(struct drm_minor *minor)
*/
void drm_put_dev(struct drm_device *dev)
{
- struct drm_driver *driver;
struct drm_map_list *r_list, *list_temp;
DRM_DEBUG("\n");
@@ -372,7 +371,6 @@ void drm_put_dev(struct drm_device *dev)
DRM_ERROR("cleanup called no dev\n");
return;
}
- driver = dev->driver;
drm_lastclose(dev);
@@ -386,9 +384,6 @@ void drm_put_dev(struct drm_device *dev)
list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
drm_rmmap(dev, r_list->map);
- drm_ht_remove(&dev->map_hash);
-
- drm_ctxbitmap_cleanup(dev);
if (drm_core_check_feature(dev, DRIVER_MODESET))
drm_put_minor(&dev->control);
@@ -396,14 +391,11 @@ void drm_put_dev(struct drm_device *dev)
if (dev->render)
drm_put_minor(&dev->render);
- if (driver->driver_features & DRIVER_GEM)
- drm_gem_destroy(dev);
-
drm_put_minor(&dev->primary);
list_del(&dev->driver_item);
- kfree(dev->devname);
- kfree(dev);
+
+ drm_dev_free(dev);
}
EXPORT_SYMBOL(drm_put_dev);
@@ -502,6 +494,29 @@ err_free:
EXPORT_SYMBOL(drm_dev_alloc);
/**
+ * drm_dev_free - Free DRM device
+ * @dev: DRM device to free
+ *
+ * Free a DRM device that has previously been allocated via drm_dev_alloc().
+ * You must not use kfree() instead or you will leak memory.
+ *
+ * This must not be called once the device got registered. Use drm_put_dev()
+ * instead, which then calls drm_dev_free().
+ */
+void drm_dev_free(struct drm_device *dev)
+{
+ if (dev->driver->driver_features & DRIVER_GEM)
+ drm_gem_destroy(dev);
+
+ drm_ctxbitmap_cleanup(dev);
+ drm_ht_remove(&dev->map_hash);
+
+ kfree(dev->devname);
+ kfree(dev);
+}
+EXPORT_SYMBOL(drm_dev_free);
+
+/**
* drm_dev_register - Register DRM device
* @dev: Device to register
*
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index 54d60f2..bace9d9 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -29,7 +29,7 @@ int drm_get_usb_dev(struct usb_interface *interface,
return 0;
err_free:
- kfree(dev);
+ drm_dev_free(dev);
return ret;
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 88e17b3..f4ec6b3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1647,6 +1647,7 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map)
struct drm_device *drm_dev_alloc(struct drm_driver *driver,
struct device *parent);
+void drm_dev_free(struct drm_device *dev);
int drm_dev_register(struct drm_device *dev);
int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
/*@}*/
--
1.8.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/5] drm: move device unregistration into drm_dev_unregister()
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
` (3 preceding siblings ...)
2013-10-02 9:23 ` [PATCH 4/5] drm: introduce drm_dev_free() to fix error paths David Herrmann
@ 2013-10-02 9:23 ` David Herrmann
2013-10-03 13:18 ` Daniel Vetter
2013-10-09 5:31 ` [PATCH 0/5] DRM device-alloc cleanup Dave Airlie
5 siblings, 1 reply; 13+ messages in thread
From: David Herrmann @ 2013-10-02 9:23 UTC (permalink / raw)
To: dri-devel
Analog to drm_dev_register(), we now provide drm_dev_unregister() which
does the reverse. drm_dev_put() is still in place and combines the calls
to drm_dev_unregister() and drm_dev_free() so buses don't have to change.
*_get() and *_put() are used for reference-counting in the kernel.
However, drm_dev_put() definitely does not do any kind of ref-counting.
Hence, use the more appropriate *_register(), *_unregister(), *_alloc()
and *_free() names.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_stub.c | 61 +++++++++++++++++++++++++++-------------------
include/drm/drmP.h | 1 +
2 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 397ab90..a5290b7 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -363,8 +363,6 @@ static void drm_unplug_minor(struct drm_minor *minor)
*/
void drm_put_dev(struct drm_device *dev)
{
- struct drm_map_list *r_list, *list_temp;
-
DRM_DEBUG("\n");
if (!dev) {
@@ -372,29 +370,7 @@ void drm_put_dev(struct drm_device *dev)
return;
}
- drm_lastclose(dev);
-
- if (dev->driver->unload)
- dev->driver->unload(dev);
-
- if (dev->driver->bus->agp_destroy)
- dev->driver->bus->agp_destroy(dev);
-
- drm_vblank_cleanup(dev);
-
- list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
- drm_rmmap(dev, r_list->map);
-
- if (drm_core_check_feature(dev, DRIVER_MODESET))
- drm_put_minor(&dev->control);
-
- if (dev->render)
- drm_put_minor(&dev->render);
-
- drm_put_minor(&dev->primary);
-
- list_del(&dev->driver_item);
-
+ drm_dev_unregister(dev);
drm_dev_free(dev);
}
EXPORT_SYMBOL(drm_put_dev);
@@ -595,3 +571,38 @@ out_unlock:
return ret;
}
EXPORT_SYMBOL(drm_dev_register);
+
+/**
+ * drm_dev_unregister - Unregister DRM device
+ * @dev: Device to unregister
+ *
+ * Unregister the DRM device from the system. This does the reverse of
+ * drm_dev_register() but does not deallocate the device. The caller must call
+ * drm_dev_free() to free all resources.
+ */
+void drm_dev_unregister(struct drm_device *dev)
+{
+ struct drm_map_list *r_list, *list_temp;
+
+ drm_lastclose(dev);
+
+ if (dev->driver->unload)
+ dev->driver->unload(dev);
+
+ if (dev->driver->bus->agp_destroy)
+ dev->driver->bus->agp_destroy(dev);
+
+ drm_vblank_cleanup(dev);
+
+ list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
+ drm_rmmap(dev, r_list->map);
+
+ if (dev->control)
+ drm_put_minor(&dev->control);
+ if (dev->render)
+ drm_put_minor(&dev->render);
+ drm_put_minor(&dev->primary);
+
+ list_del(&dev->driver_item);
+}
+EXPORT_SYMBOL(drm_dev_unregister);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index f4ec6b3..7516d22 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1649,6 +1649,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
struct device *parent);
void drm_dev_free(struct drm_device *dev);
int drm_dev_register(struct drm_device *dev);
+void drm_dev_unregister(struct drm_device *dev);
int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
/*@}*/
--
1.8.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] drm: move device unregistration into drm_dev_unregister()
2013-10-02 9:23 ` [PATCH 5/5] drm: move device unregistration into drm_dev_unregister() David Herrmann
@ 2013-10-03 13:18 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2013-10-03 13:18 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel
On Wed, Oct 02, 2013 at 11:23:38AM +0200, David Herrmann wrote:
> Analog to drm_dev_register(), we now provide drm_dev_unregister() which
> does the reverse. drm_dev_put() is still in place and combines the calls
> to drm_dev_unregister() and drm_dev_free() so buses don't have to change.
>
> *_get() and *_put() are used for reference-counting in the kernel.
> However, drm_dev_put() definitely does not do any kind of ref-counting.
> Hence, use the more appropriate *_register(), *_unregister(), *_alloc()
> and *_free() names.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
With the agp_init call moved back into pci code this series is
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We still have a bit a midlayer smell here, and to correctly fix the
init/cleanup order I think a bit more work is required. But this is a
definit step up imo.
-Daniel
> ---
> drivers/gpu/drm/drm_stub.c | 61 +++++++++++++++++++++++++++-------------------
> include/drm/drmP.h | 1 +
> 2 files changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 397ab90..a5290b7 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -363,8 +363,6 @@ static void drm_unplug_minor(struct drm_minor *minor)
> */
> void drm_put_dev(struct drm_device *dev)
> {
> - struct drm_map_list *r_list, *list_temp;
> -
> DRM_DEBUG("\n");
>
> if (!dev) {
> @@ -372,29 +370,7 @@ void drm_put_dev(struct drm_device *dev)
> return;
> }
>
> - drm_lastclose(dev);
> -
> - if (dev->driver->unload)
> - dev->driver->unload(dev);
> -
> - if (dev->driver->bus->agp_destroy)
> - dev->driver->bus->agp_destroy(dev);
> -
> - drm_vblank_cleanup(dev);
> -
> - list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
> - drm_rmmap(dev, r_list->map);
> -
> - if (drm_core_check_feature(dev, DRIVER_MODESET))
> - drm_put_minor(&dev->control);
> -
> - if (dev->render)
> - drm_put_minor(&dev->render);
> -
> - drm_put_minor(&dev->primary);
> -
> - list_del(&dev->driver_item);
> -
> + drm_dev_unregister(dev);
> drm_dev_free(dev);
> }
> EXPORT_SYMBOL(drm_put_dev);
> @@ -595,3 +571,38 @@ out_unlock:
> return ret;
> }
> EXPORT_SYMBOL(drm_dev_register);
> +
> +/**
> + * drm_dev_unregister - Unregister DRM device
> + * @dev: Device to unregister
> + *
> + * Unregister the DRM device from the system. This does the reverse of
> + * drm_dev_register() but does not deallocate the device. The caller must call
> + * drm_dev_free() to free all resources.
> + */
> +void drm_dev_unregister(struct drm_device *dev)
> +{
> + struct drm_map_list *r_list, *list_temp;
> +
> + drm_lastclose(dev);
> +
> + if (dev->driver->unload)
> + dev->driver->unload(dev);
> +
> + if (dev->driver->bus->agp_destroy)
> + dev->driver->bus->agp_destroy(dev);
> +
> + drm_vblank_cleanup(dev);
> +
> + list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
> + drm_rmmap(dev, r_list->map);
> +
> + if (dev->control)
> + drm_put_minor(&dev->control);
> + if (dev->render)
> + drm_put_minor(&dev->render);
> + drm_put_minor(&dev->primary);
> +
> + list_del(&dev->driver_item);
> +}
> +EXPORT_SYMBOL(drm_dev_unregister);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index f4ec6b3..7516d22 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1649,6 +1649,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> struct device *parent);
> void drm_dev_free(struct drm_device *dev);
> int drm_dev_register(struct drm_device *dev);
> +void drm_dev_unregister(struct drm_device *dev);
> int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
> /*@}*/
>
> --
> 1.8.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] DRM device-alloc cleanup
2013-10-02 9:23 [PATCH 0/5] DRM device-alloc cleanup David Herrmann
` (4 preceding siblings ...)
2013-10-02 9:23 ` [PATCH 5/5] drm: move device unregistration into drm_dev_unregister() David Herrmann
@ 2013-10-09 5:31 ` Dave Airlie
2013-10-09 6:00 ` Dave Airlie
5 siblings, 1 reply; 13+ messages in thread
From: Dave Airlie @ 2013-10-09 5:31 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel
On Wed, Oct 2, 2013 at 7:23 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> This cleans up the bus drivers in DRM. Instead of copying the device alloc/free
> semantics into each bus driver (drm_{pci,platform,usb}.c) we now have a central
> place in drm_stub.c.
>
> This introduces drm_dev_{alloc,free,register,unregister}(). They have the same
> semantics as most other kernel subsystems. *_alloc() allocates a new device and
> populates the static fields and sub-objects. *_free() frees an unregistered
> object allocated via *_alloc(). *_register() registers a DRM device and
> *_unregister() obviously unregisters a DRM device. A *_free() is still needed
> after calling *_unregister() (same as for "struct device"). No ref-counting is
> added as it is not required by any driver.
>
> Note that the bus drivers are modified to use the new helpers directly. However,
> I didn't modify the drivers to use *_unregister() and *_free() directly.
> Instead, the drm_put_dev() helper was modified to use these. Reason for that is
> that I have pending patches to make device hotplugging safer regarding mmaps.
> But these aren't ready, yet. Hopefully I can get them ready for rc5 or rc6.
>
> Tested on nouveau.
>
Okay I've merged this series, a follow-on to fix the AGP bits like
Daniel suggested would be good.
Dave.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/5] DRM device-alloc cleanup
2013-10-09 5:31 ` [PATCH 0/5] DRM device-alloc cleanup Dave Airlie
@ 2013-10-09 6:00 ` Dave Airlie
2013-10-09 8:05 ` Daniel Vetter
0 siblings, 1 reply; 13+ messages in thread
From: Dave Airlie @ 2013-10-09 6:00 UTC (permalink / raw)
To: David Herrmann; +Cc: dri-devel
On Wed, Oct 9, 2013 at 3:31 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Wed, Oct 2, 2013 at 7:23 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> This cleans up the bus drivers in DRM. Instead of copying the device alloc/free
>> semantics into each bus driver (drm_{pci,platform,usb}.c) we now have a central
>> place in drm_stub.c.
>>
>> This introduces drm_dev_{alloc,free,register,unregister}(). They have the same
>> semantics as most other kernel subsystems. *_alloc() allocates a new device and
>> populates the static fields and sub-objects. *_free() frees an unregistered
>> object allocated via *_alloc(). *_register() registers a DRM device and
>> *_unregister() obviously unregisters a DRM device. A *_free() is still needed
>> after calling *_unregister() (same as for "struct device"). No ref-counting is
>> added as it is not required by any driver.
>>
>> Note that the bus drivers are modified to use the new helpers directly. However,
>> I didn't modify the drivers to use *_unregister() and *_free() directly.
>> Instead, the drm_put_dev() helper was modified to use these. Reason for that is
>> that I have pending patches to make device hotplugging safer regarding mmaps.
>> But these aren't ready, yet. Hopefully I can get them ready for rc5 or rc6.
>>
>> Tested on nouveau.
>>
>
> Okay I've merged this series, a follow-on to fix the AGP bits like
> Daniel suggested would be good.
Actually this oopses i915 on startup, I fixed up the lack of passing
flags into drm_dev_register,
so it can be passed to load, and it seems to work now, I've squashed
my fix into the series in my tree
please read/review the series for your learning pleasure.
Dave.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/5] DRM device-alloc cleanup
2013-10-09 6:00 ` Dave Airlie
@ 2013-10-09 8:05 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2013-10-09 8:05 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel
On Wed, Oct 09, 2013 at 04:00:00PM +1000, Dave Airlie wrote:
> On Wed, Oct 9, 2013 at 3:31 PM, Dave Airlie <airlied@gmail.com> wrote:
> > On Wed, Oct 2, 2013 at 7:23 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> >> Hi
> >>
> >> This cleans up the bus drivers in DRM. Instead of copying the device alloc/free
> >> semantics into each bus driver (drm_{pci,platform,usb}.c) we now have a central
> >> place in drm_stub.c.
> >>
> >> This introduces drm_dev_{alloc,free,register,unregister}(). They have the same
> >> semantics as most other kernel subsystems. *_alloc() allocates a new device and
> >> populates the static fields and sub-objects. *_free() frees an unregistered
> >> object allocated via *_alloc(). *_register() registers a DRM device and
> >> *_unregister() obviously unregisters a DRM device. A *_free() is still needed
> >> after calling *_unregister() (same as for "struct device"). No ref-counting is
> >> added as it is not required by any driver.
> >>
> >> Note that the bus drivers are modified to use the new helpers directly. However,
> >> I didn't modify the drivers to use *_unregister() and *_free() directly.
> >> Instead, the drm_put_dev() helper was modified to use these. Reason for that is
> >> that I have pending patches to make device hotplugging safer regarding mmaps.
> >> But these aren't ready, yet. Hopefully I can get them ready for rc5 or rc6.
> >>
> >> Tested on nouveau.
> >>
> >
> > Okay I've merged this series, a follow-on to fix the AGP bits like
> > Daniel suggested would be good.
>
> Actually this oopses i915 on startup, I fixed up the lack of passing
> flags into drm_dev_register,
> so it can be passed to load, and it seems to work now, I've squashed
> my fix into the series in my tree
>
> please read/review the series for your learning pleasure.
Oops, shame on me for failing to spot the unconditional 0 passed for
flags. i915.ko really doesn't like that ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 13+ messages in thread