From e17280758cc0b4f3d7065554006adcb87448f6c0 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 23 Jul 2014 11:38:38 +0200 Subject: drm: make sysfs device always available for minors For each minor we allocate a sysfs device as minor->kdev. Currently, this is allocated and registered in drm_minor_register(). This makes it impossible to add sysfs-attributes to the device before it is registered. Therefore, they are not added atomically, nor can we move device_add() *after* ->load() is called. This patch makes minor->kdev available early, but only adds the device during minor-registration. Note that the registration is still called before ->load() as debugfs needs to be split, too. This will be fixed in follow-ups. Reviewed-by: Daniel Vetter Reviewed-by: Alex Deucher Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_stub.c | 22 ++++++++--- drivers/gpu/drm/drm_sysfs.c | 90 +++++++++++++++++++-------------------------- include/drm/drmP.h | 3 +- 3 files changed, 54 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 8b24db5..09c6bfb 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -281,9 +281,19 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) minor->index = r; + minor->kdev = drm_sysfs_minor_alloc(minor); + if (IS_ERR(minor->kdev)) { + r = PTR_ERR(minor->kdev); + goto err_index; + } + *drm_minor_get_slot(dev, type) = minor; return 0; +err_index: + spin_lock_irqsave(&drm_minor_lock, flags); + idr_remove(&drm_minors_idr, minor->index); + spin_unlock_irqrestore(&drm_minor_lock, flags); err_free: kfree(minor); return r; @@ -300,6 +310,7 @@ static void drm_minor_free(struct drm_device *dev, unsigned int type) return; drm_mode_group_destroy(&minor->mode_group); + put_device(minor->kdev); spin_lock_irqsave(&drm_minor_lock, flags); idr_remove(&drm_minors_idr, minor->index); @@ -327,11 +338,9 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type) return ret; } - ret = drm_sysfs_device_add(minor); - if (ret) { - DRM_ERROR("DRM: Error sysfs_device_add.\n"); + ret = device_add(minor->kdev); + if (ret) goto err_debugfs; - } /* replace NULL with @minor so lookups will succeed from now on */ spin_lock_irqsave(&drm_minor_lock, flags); @@ -352,7 +361,7 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type) unsigned long flags; minor = *drm_minor_get_slot(dev, type); - if (!minor || !minor->kdev) + if (!minor || !device_is_registered(minor->kdev)) return; /* replace @minor with NULL so lookups will fail from now on */ @@ -360,8 +369,9 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type) idr_replace(&drm_minors_idr, NULL, minor->index); spin_unlock_irqrestore(&drm_minor_lock, flags); + device_del(minor->kdev); + dev_set_drvdata(minor->kdev, NULL); /* safety belt */ drm_debugfs_cleanup(minor); - drm_sysfs_device_remove(minor); } /** diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 7827dad..ab1a5f6 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -493,71 +493,55 @@ static void drm_sysfs_release(struct device *dev) } /** - * drm_sysfs_device_add - adds a class device to sysfs for a character driver - * @dev: DRM device to be added - * @head: DRM head in question + * drm_sysfs_minor_alloc() - Allocate sysfs device for given minor + * @minor: minor to allocate sysfs device for * - * Add a DRM device to the DRM's device model class. We use @dev's PCI device - * as the parent for the Linux device, and make sure it has a file containing - * the driver we're using (for userspace compatibility). + * This allocates a new sysfs device for @minor and returns it. The device is + * not registered nor linked. The caller has to use device_add() and + * device_del() to register and unregister it. + * + * Note that dev_get_drvdata() on the new device will return the minor. + * However, the device does not hold a ref-count to the minor nor to the + * underlying drm_device. This is unproblematic as long as you access the + * private data only in sysfs callbacks. device_del() disables those + * synchronously, so they cannot be called after you cleanup a minor. */ -int drm_sysfs_device_add(struct drm_minor *minor) +struct device *drm_sysfs_minor_alloc(struct drm_minor *minor) { - char *minor_str; + const char *minor_str; + struct device *kdev; int r; if (minor->type == DRM_MINOR_CONTROL) minor_str = "controlD%d"; - else if (minor->type == DRM_MINOR_RENDER) - minor_str = "renderD%d"; - else - minor_str = "card%d"; - - minor->kdev = kzalloc(sizeof(*minor->kdev), GFP_KERNEL); - if (!minor->kdev) { - r = -ENOMEM; - goto error; - } - - device_initialize(minor->kdev); - minor->kdev->devt = MKDEV(DRM_MAJOR, minor->index); - minor->kdev->class = drm_class; - minor->kdev->type = &drm_sysfs_device_minor; - minor->kdev->parent = minor->dev->dev; - minor->kdev->release = drm_sysfs_release; - dev_set_drvdata(minor->kdev, minor); - - r = dev_set_name(minor->kdev, minor_str, minor->index); + else if (minor->type == DRM_MINOR_RENDER) + minor_str = "renderD%d"; + else + minor_str = "card%d"; + + kdev = kzalloc(sizeof(*kdev), GFP_KERNEL); + if (!kdev) + return ERR_PTR(-ENOMEM); + + device_initialize(kdev); + kdev->devt = MKDEV(DRM_MAJOR, minor->index); + kdev->class = drm_class; + kdev->type = &drm_sysfs_device_minor; + kdev->parent = minor->dev->dev; + kdev->release = drm_sysfs_release; + dev_set_drvdata(kdev, minor); + + r = dev_set_name(kdev, minor_str, minor->index); if (r < 0) - goto error; - - r = device_add(minor->kdev); - if (r < 0) - goto error; - - return 0; + goto err_free; -error: - DRM_ERROR("device create failed %d\n", r); - put_device(minor->kdev); - return r; -} + return kdev; -/** - * drm_sysfs_device_remove - remove DRM device - * @dev: DRM device to remove - * - * This call unregisters and cleans up a class device that was created with a - * call to drm_sysfs_device_add() - */ -void drm_sysfs_device_remove(struct drm_minor *minor) -{ - if (minor->kdev) - device_unregister(minor->kdev); - minor->kdev = NULL; +err_free: + put_device(kdev); + return ERR_PTR(r); } - /** * drm_class_device_register - Register a struct device in the drm class. * diff --git a/include/drm/drmP.h b/include/drm/drmP.h index c480b44..458385e 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1502,9 +1502,8 @@ extern int drm_pci_set_unique(struct drm_device *dev, struct drm_sysfs_class; extern struct class *drm_sysfs_create(struct module *owner, char *name); extern void drm_sysfs_destroy(void); -extern int drm_sysfs_device_add(struct drm_minor *minor); +extern struct device *drm_sysfs_minor_alloc(struct drm_minor *minor); extern void drm_sysfs_hotplug_event(struct drm_device *dev); -extern void drm_sysfs_device_remove(struct drm_minor *minor); extern int drm_sysfs_connector_add(struct drm_connector *connector); extern void drm_sysfs_connector_remove(struct drm_connector *connector); -- cgit v1.1