From 1bb72532ac260a2d3982b40bdd4c936d779d0d16 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 2 Oct 2013 11:23:34 +0200 Subject: drm: add drm_dev_alloc() helper 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 Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_platform.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_platform.c') diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index f7a18c6ba4c..fb2721738a8 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); -- cgit v1.2.3-70-g09d2 From c22f0ace1926da399d9a16dfaf09174c1b03594c Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 2 Oct 2013 11:23:35 +0200 Subject: drm: merge device setup into drm_dev_register() 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. [airlied: keep passing flags through so drivers don't oops on load] Signed-off-by: David Herrmann Signed-off-by: Dave Airlie --- 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(-) (limited to 'drivers/gpu/drm/drm_platform.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d2758be37a9..743589dc47c 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, ent->driver_data); + 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 fb2721738a8..a0f91f85651 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, 0); 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 64bd52f1319..432994aafc3 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, unsigned long flags) +{ + 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, flags); + 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 34ad8edfe80..5ef353f77b5 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, 0); 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 ea545b5ad46..1973f796651 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1640,11 +1640,9 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map) #include -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, unsigned long flags); int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type); /*@}*/ -- cgit v1.2.3-70-g09d2 From 0dc8fe5985e01f238e7dc64ff1733cc0291811e8 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 2 Oct 2013 11:23:37 +0200 Subject: drm: introduce drm_dev_free() to fix error paths 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 Signed-off-by: Dave Airlie --- 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(-) (limited to 'drivers/gpu/drm/drm_platform.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 743589dc47c..cabe2bd702a 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 a0f91f85651..fc24fee8ec8 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 432994aafc3..3b5b07482de 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); @@ -501,6 +493,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 5ef353f77b5..b179b70e785 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 1973f796651..537833c9ab8 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1642,6 +1642,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, unsigned long flags); int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type); /*@}*/ -- cgit v1.2.3-70-g09d2