drm/sysfs: sort out minor and connector device object lifetimes.
authorDave Airlie <airlied@redhat.com>
Fri, 11 Oct 2013 04:07:25 +0000 (14:07 +1000)
committerDave Airlie <airlied@redhat.com>
Tue, 22 Oct 2013 08:37:40 +0000 (09:37 +0100)
So drm was abusing device lifetimes, by having embedded device structures
in the minor and connector it meant that the lifetime of the internal drm
objects (drm_minor and drm_connector) were tied to the lifetime of the device
files in sysfs, so if something kept those files opened the current code
would kfree the objects and things would go downhill from there.

Now in reality there is no need for these lifetimes to be so intertwined,
especailly with hotplugging of devices where we wish to remove the sysfs
and userspace facing pieces before we can unwind the internal objects due
to open userspace files or mmaps, so split the objects out so the struct
device is no longer embedded and do what fbdev does and just allocate
and remove the sysfs inodes separately.

Signed-off-by: Dave Airlie <airlied@redhat.com>
drivers/gpu/drm/drm_sysfs.c
drivers/gpu/drm/gma500/cdv_intel_dp.c
drivers/gpu/drm/i915/i915_irq.c
drivers/gpu/drm/i915/i915_sysfs.c
drivers/gpu/drm/i915/intel_dp.c
drivers/gpu/drm/i915/intel_panel.c
drivers/gpu/drm/nouveau/nouveau_backlight.c
drivers/gpu/drm/radeon/atombios_encoders.c
drivers/gpu/drm/radeon/radeon_legacy_encoders.c
include/drm/drmP.h
include/drm/drm_crtc.h

index 2290b3b7383247a6dde176d71ee1e6fe18d7b4c7..dae42c79154f3de596a06c95d1e996750fe6e35c 100644 (file)
@@ -22,8 +22,8 @@
 #include <drm/drm_core.h>
 #include <drm/drmP.h>
 
-#define to_drm_minor(d) container_of(d, struct drm_minor, kdev)
-#define to_drm_connector(d) container_of(d, struct drm_connector, kdev)
+#define to_drm_minor(d) dev_get_drvdata(d)
+#define to_drm_connector(d) dev_get_drvdata(d)
 
 static struct device_type drm_sysfs_device_minor = {
        .name = "drm_minor"
@@ -162,20 +162,6 @@ void drm_sysfs_destroy(void)
        drm_class = NULL;
 }
 
-/**
- * drm_sysfs_device_release - do nothing
- * @dev: Linux device
- *
- * Normally, this would free the DRM device associated with @dev, along
- * with cleaning up any other stuff.  But we do that in the DRM core, so
- * this function can just return and hope that the core does its job.
- */
-static void drm_sysfs_device_release(struct device *dev)
-{
-       memset(dev, 0, sizeof(struct device));
-       return;
-}
-
 /*
  * Connector properties
  */
@@ -394,29 +380,26 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
        int i;
        int ret;
 
-       /* We shouldn't get called more than once for the same connector */
-       BUG_ON(device_is_registered(&connector->kdev));
-
-       connector->kdev.parent = &dev->primary->kdev;
-       connector->kdev.class = drm_class;
-       connector->kdev.release = drm_sysfs_device_release;
+       if (connector->kdev)
+               return 0;
 
+       /* We shouldn't get called more than once for the same connector */
+       connector->kdev = device_create(drm_class, dev->primary->kdev,
+                                       0, connector, "card%d-%s",
+                                       dev->primary->index, drm_get_connector_name(connector));
        DRM_DEBUG("adding \"%s\" to sysfs\n",
                  drm_get_connector_name(connector));
 
-       dev_set_name(&connector->kdev, "card%d-%s",
-                    dev->primary->index, drm_get_connector_name(connector));
-       ret = device_register(&connector->kdev);
-
-       if (ret) {
-               DRM_ERROR("failed to register connector device: %d\n", ret);
+       if (IS_ERR(connector->kdev)) {
+               DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
+               ret = PTR_ERR(connector->kdev);
                goto out;
        }
 
        /* Standard attributes */
 
        for (attr_cnt = 0; attr_cnt < ARRAY_SIZE(connector_attrs); attr_cnt++) {
-               ret = device_create_file(&connector->kdev, &connector_attrs[attr_cnt]);
+               ret = device_create_file(connector->kdev, &connector_attrs[attr_cnt]);
                if (ret)
                        goto err_out_files;
        }
@@ -433,7 +416,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
                case DRM_MODE_CONNECTOR_Component:
                case DRM_MODE_CONNECTOR_TV:
                        for (opt_cnt = 0; opt_cnt < ARRAY_SIZE(connector_attrs_opt1); opt_cnt++) {
-                               ret = device_create_file(&connector->kdev, &connector_attrs_opt1[opt_cnt]);
+                               ret = device_create_file(connector->kdev, &connector_attrs_opt1[opt_cnt]);
                                if (ret)
                                        goto err_out_files;
                        }
@@ -442,7 +425,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
                        break;
        }
 
-       ret = sysfs_create_bin_file(&connector->kdev.kobj, &edid_attr);
+       ret = sysfs_create_bin_file(&connector->kdev->kobj, &edid_attr);
        if (ret)
                goto err_out_files;
 
@@ -453,10 +436,11 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 
 err_out_files:
        for (i = 0; i < opt_cnt; i++)
-               device_remove_file(&connector->kdev, &connector_attrs_opt1[i]);
+               device_remove_file(connector->kdev, &connector_attrs_opt1[i]);
        for (i = 0; i < attr_cnt; i++)
-               device_remove_file(&connector->kdev, &connector_attrs[i]);
-       device_unregister(&connector->kdev);
+               device_remove_file(connector->kdev, &connector_attrs[i]);
+       put_device(connector->kdev);
+       device_unregister(connector->kdev);
 
 out:
        return ret;
@@ -480,16 +464,17 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
        int i;
 
-       if (!connector->kdev.parent)
+       if (!connector->kdev)
                return;
        DRM_DEBUG("removing \"%s\" from sysfs\n",
                  drm_get_connector_name(connector));
 
        for (i = 0; i < ARRAY_SIZE(connector_attrs); i++)
-               device_remove_file(&connector->kdev, &connector_attrs[i]);
-       sysfs_remove_bin_file(&connector->kdev.kobj, &edid_attr);
-       device_unregister(&connector->kdev);
-       connector->kdev.parent = NULL;
+               device_remove_file(connector->kdev, &connector_attrs[i]);
+       sysfs_remove_bin_file(&connector->kdev->kobj, &edid_attr);
+       put_device(connector->kdev);
+       device_unregister(connector->kdev);
+       connector->kdev = NULL;
 }
 EXPORT_SYMBOL(drm_sysfs_connector_remove);
 
@@ -508,7 +493,7 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
 
        DRM_DEBUG("generating hotplug event\n");
 
-       kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, envp);
+       kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
@@ -523,15 +508,8 @@ EXPORT_SYMBOL(drm_sysfs_hotplug_event);
  */
 int drm_sysfs_device_add(struct drm_minor *minor)
 {
-       int err;
        char *minor_str;
 
-       minor->kdev.parent = minor->dev->dev;
-
-       minor->kdev.class = drm_class;
-       minor->kdev.release = drm_sysfs_device_release;
-       minor->kdev.devt = minor->device;
-       minor->kdev.type = &drm_sysfs_device_minor;
        if (minor->type == DRM_MINOR_CONTROL)
                minor_str = "controlD%d";
         else if (minor->type == DRM_MINOR_RENDER)
@@ -539,18 +517,14 @@ int drm_sysfs_device_add(struct drm_minor *minor)
         else
                 minor_str = "card%d";
 
-       dev_set_name(&minor->kdev, minor_str, minor->index);
-
-       err = device_register(&minor->kdev);
-       if (err) {
-               DRM_ERROR("device add failed: %d\n", err);
-               goto err_out;
+       minor->kdev = device_create(drm_class, minor->dev->dev,
+                                   MKDEV(DRM_MAJOR, minor->index),
+                                   minor, minor_str, minor->index);
+       if (IS_ERR(minor->kdev)) {
+               DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev));
+               return PTR_ERR(minor->kdev);
        }
-
        return 0;
-
-err_out:
-       return err;
 }
 
 /**
@@ -562,9 +536,9 @@ err_out:
  */
 void drm_sysfs_device_remove(struct drm_minor *minor)
 {
-       if (minor->kdev.parent)
-               device_unregister(&minor->kdev);
-       minor->kdev.parent = NULL;
+       if (minor->kdev)
+               device_destroy(drm_class, MKDEV(DRM_MAJOR, minor->index));
+       minor->kdev = NULL;
 }
 
 
index f4eb43573cada7a7a5d2af753a8dcc4a29c898e8..f88a1815d87c4e795492ead6ef1c4c07a4d31b89 100644 (file)
@@ -666,7 +666,7 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
        strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1);
        intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
        intel_dp->adapter.algo_data = &intel_dp->algo;
-       intel_dp->adapter.dev.parent = &connector->base.kdev;
+       intel_dp->adapter.dev.parent = connector->base.kdev;
 
        if (is_edp(encoder))
                cdv_intel_edp_panel_vdd_on(encoder);
index 418ad642c7425a6bdda68adff6cb4357b4071000..d1739d3bdae953275c1918c5f3093fef53f05109 100644 (file)
@@ -942,7 +942,7 @@ static void ivybridge_parity_work(struct work_struct *work)
                parity_event[4] = kasprintf(GFP_KERNEL, "SLICE=%d", slice);
                parity_event[5] = NULL;
 
-               kobject_uevent_env(&dev_priv->dev->primary->kdev.kobj,
+               kobject_uevent_env(&dev_priv->dev->primary->kdev->kobj,
                                   KOBJ_CHANGE, parity_event);
 
                DRM_DEBUG("Parity error: Slice = %d, Row = %d, Bank = %d, Sub bank = %d.\n",
@@ -1539,7 +1539,7 @@ static void i915_error_work_func(struct work_struct *work)
        char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL };
        int ret;
 
-       kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
+       kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, error_event);
 
        /*
         * Note that there's only one work item which does gpu resets, so we
@@ -1553,7 +1553,7 @@ static void i915_error_work_func(struct work_struct *work)
         */
        if (i915_reset_in_progress(error) && !i915_terminally_wedged(error)) {
                DRM_DEBUG_DRIVER("resetting chip\n");
-               kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE,
+               kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE,
                                   reset_event);
 
                /*
@@ -1580,7 +1580,7 @@ static void i915_error_work_func(struct work_struct *work)
                        smp_mb__before_atomic_inc();
                        atomic_inc(&dev_priv->gpu_error.reset_counter);
 
-                       kobject_uevent_env(&dev->primary->kdev.kobj,
+                       kobject_uevent_env(&dev->primary->kdev->kobj,
                                           KOBJ_CHANGE, reset_done_event);
                } else {
                        atomic_set(&error->reset_counter, I915_WEDGED);
index 3ae47951c1bd875ee1d07d8d4a35efe5e9c866b9..1beec51b8e26d3c4e6885444932b17b09bfbb9cc 100644 (file)
@@ -32,7 +32,7 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-#define dev_to_drm_minor(d) container_of((d), struct drm_minor, kdev)
+#define dev_to_drm_minor(d) dev_get_drvdata((d))
 
 #ifdef CONFIG_PM
 static u32 calc_residency(struct drm_device *dev, const u32 reg)
@@ -75,7 +75,7 @@ show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf)
 static ssize_t
 show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-       struct drm_minor *dminor = dev_to_drm_minor(kdev);
+       struct drm_minor *dminor = dev_get_drvdata(kdev);
        u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6);
        return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency);
 }
@@ -543,19 +543,19 @@ void i915_setup_sysfs(struct drm_device *dev)
 
 #ifdef CONFIG_PM
        if (INTEL_INFO(dev)->gen >= 6) {
-               ret = sysfs_merge_group(&dev->primary->kdev.kobj,
+               ret = sysfs_merge_group(&dev->primary->kdev->kobj,
                                        &rc6_attr_group);
                if (ret)
                        DRM_ERROR("RC6 residency sysfs setup failed\n");
        }
 #endif
        if (HAS_L3_DPF(dev)) {
-               ret = device_create_bin_file(&dev->primary->kdev, &dpf_attrs);
+               ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs);
                if (ret)
                        DRM_ERROR("l3 parity sysfs setup failed\n");
 
                if (NUM_L3_SLICES(dev) > 1) {
-                       ret = device_create_bin_file(&dev->primary->kdev,
+                       ret = device_create_bin_file(dev->primary->kdev,
                                                     &dpf_attrs_1);
                        if (ret)
                                DRM_ERROR("l3 parity slice 1 setup failed\n");
@@ -564,13 +564,13 @@ void i915_setup_sysfs(struct drm_device *dev)
 
        ret = 0;
        if (IS_VALLEYVIEW(dev))
-               ret = sysfs_create_files(&dev->primary->kdev.kobj, vlv_attrs);
+               ret = sysfs_create_files(&dev->primary->kdev->kobj, vlv_attrs);
        else if (INTEL_INFO(dev)->gen >= 6)
-               ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs);
+               ret = sysfs_create_files(&dev->primary->kdev->kobj, gen6_attrs);
        if (ret)
                DRM_ERROR("RPS sysfs setup failed\n");
 
-       ret = sysfs_create_bin_file(&dev->primary->kdev.kobj,
+       ret = sysfs_create_bin_file(&dev->primary->kdev->kobj,
                                    &error_state_attr);
        if (ret)
                DRM_ERROR("error_state sysfs setup failed\n");
@@ -578,14 +578,14 @@ void i915_setup_sysfs(struct drm_device *dev)
 
 void i915_teardown_sysfs(struct drm_device *dev)
 {
-       sysfs_remove_bin_file(&dev->primary->kdev.kobj, &error_state_attr);
+       sysfs_remove_bin_file(&dev->primary->kdev->kobj, &error_state_attr);
        if (IS_VALLEYVIEW(dev))
-               sysfs_remove_files(&dev->primary->kdev.kobj, vlv_attrs);
+               sysfs_remove_files(&dev->primary->kdev->kobj, vlv_attrs);
        else
-               sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs);
-       device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs_1);
-       device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
+               sysfs_remove_files(&dev->primary->kdev->kobj, gen6_attrs);
+       device_remove_bin_file(dev->primary->kdev,  &dpf_attrs_1);
+       device_remove_bin_file(dev->primary->kdev,  &dpf_attrs);
 #ifdef CONFIG_PM
-       sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
+       sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6_attr_group);
 #endif
 }
index 4f52ec75b39feb13ab03db507dc7a1c7a8b52931..d5bd349105e5ef86033e401efc882f654c50d1d1 100644 (file)
@@ -737,7 +737,7 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
        strncpy(intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1);
        intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
        intel_dp->adapter.algo_data = &intel_dp->algo;
-       intel_dp->adapter.dev.parent = &intel_connector->base.kdev;
+       intel_dp->adapter.dev.parent = intel_connector->base.kdev;
 
        ironlake_edp_panel_vdd_on(intel_dp);
        ret = i2c_dp_aux_add_bus(&intel_dp->adapter);
index 54684168de1e66d24a03521a8695da58e3d4e2cc..c81020923ee4217f7b645d7d8a6d6921518e9844 100644 (file)
@@ -722,7 +722,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
        }
        dev_priv->backlight.device =
                backlight_device_register("intel_backlight",
-                                         &connector->kdev, dev,
+                                         connector->kdev, dev,
                                          &intel_panel_bl_ops, &props);
 
        if (IS_ERR(dev_priv->backlight.device)) {
index 2ffad2176b7fc7a2e2f6a7ee90d653286e4151bc..630f6e84fc01989da5f3a8957bab9d450f888c92 100644 (file)
@@ -82,7 +82,7 @@ nv40_backlight_init(struct drm_connector *connector)
        memset(&props, 0, sizeof(struct backlight_properties));
        props.type = BACKLIGHT_RAW;
        props.max_brightness = 31;
-       bd = backlight_device_register("nv_backlight", &connector->kdev, drm,
+       bd = backlight_device_register("nv_backlight", connector->kdev, drm,
                                       &nv40_bl_ops, &props);
        if (IS_ERR(bd))
                return PTR_ERR(bd);
@@ -204,7 +204,7 @@ nv50_backlight_init(struct drm_connector *connector)
        memset(&props, 0, sizeof(struct backlight_properties));
        props.type = BACKLIGHT_RAW;
        props.max_brightness = 100;
-       bd = backlight_device_register("nv_backlight", &connector->kdev,
+       bd = backlight_device_register("nv_backlight", connector->kdev,
                                       nv_encoder, ops, &props);
        if (IS_ERR(bd))
                return PTR_ERR(bd);
index 32923d2f60021105a5092826bcd7810ca054c960..28e2dc48e01525a7c01965def343fc0398b541a6 100644 (file)
@@ -213,7 +213,7 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
        props.type = BACKLIGHT_RAW;
        snprintf(bl_name, sizeof(bl_name),
                 "radeon_bl%d", dev->primary->index);
-       bd = backlight_device_register(bl_name, &drm_connector->kdev,
+       bd = backlight_device_register(bl_name, drm_connector->kdev,
                                       pdata, &radeon_atom_backlight_ops, &props);
        if (IS_ERR(bd)) {
                DRM_ERROR("Backlight registration failed\n");
index 62cd512f5c8d4161ff3708204fe32f560566402e..c89971d904c36696ad17e6ac4ac68f017a81b27c 100644 (file)
@@ -392,7 +392,7 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
        props.type = BACKLIGHT_RAW;
        snprintf(bl_name, sizeof(bl_name),
                 "radeon_bl%d", dev->primary->index);
-       bd = backlight_device_register(bl_name, &drm_connector->kdev,
+       bd = backlight_device_register(bl_name, drm_connector->kdev,
                                       pdata, &radeon_backlight_ops, &props);
        if (IS_ERR(bd)) {
                DRM_ERROR("Backlight registration failed\n");
index 2b954adf5bd4da2e8b8c384e8d0f4951440d8737..7062307a4a2d7f656950a70bcd72bdf0fac4a4dc 100644 (file)
@@ -1046,7 +1046,7 @@ struct drm_minor {
        int index;                      /**< Minor device number */
        int type;                       /**< Control or render */
        dev_t device;                   /**< Device number for mknod */
-       struct device kdev;             /**< Linux device */
+       struct device *kdev;            /**< Linux device */
        struct drm_device *dev;
 
        struct dentry *debugfs_root;
index ba407f6b4f1fc1c4a2548e0cee1e0da623eb7d20..755dd2532612431bcf5f9d8520758bba172f593c 100644 (file)
@@ -595,7 +595,7 @@ enum drm_connector_force {
  */
 struct drm_connector {
        struct drm_device *dev;
-       struct device kdev;
+       struct device *kdev;
        struct device_attribute *attr;
        struct list_head head;