| Message ID | da848fcd5ca72a35d9a722e644719977a47bb7ba.1465382836.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 08, 2016 at 12:49:29PM +0200, Lukas Wunner wrote: > The PM core introduced the ability to keep devices runtime suspended > during the entire system suspend/resume process with commit aae4518b3124 > ("PM / sleep: Mechanism to avoid resuming runtime-suspended devices > unnecessarily"). Before this so-called "direct-complete" procedure was > introduced, devices were always runtime resumed only to be immediately > put to sleep again using their ->suspend hook. Direct-complete is > enabled by returning a positive value from the ->prepare hook. The PCI > core usually does this automatically. > > Direct-complete is only available for a device if all children use it as > well. Currently we cannot support direct-complete for DRM drivers > because the DRM core automatically registers multiple DRM minors which > belong to device class drm_class, and drm_class uses a struct dev_pm_ops > which lacks the ->prepare callback. > > While this could be solved by adding the missing ->prepare callback, > closer inspection shows that there are no DRM drivers left which declare > the legacy ->suspend and ->resume callbacks in their drm_driver struct. > The last ones to remove them were i915 with commit 1751fcf9f92e > ("drm/i915: Fix module initialisation, v2.") and exynos with commit > e7fefb1d5af5 ("drm/exynos: remove legacy ->suspend()/resume()"). > > Consequently the struct dev_pm_ops of drm_class is now dead code. Remove > it. If no dev_pm_ops is declared for a device, the PM core automatically > enables direct-complete for it, thereby making that mechanism available > to the parent DRM PCI devices. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> wow, awesome! /me pops some champange This is excellent really. Applied to drm-misc, but I'll wait with pushing out since I want to roll forward to the latest drm-next that Dave promised to push out tomorrow. -Daniel > --- > drivers/gpu/drm/drm_drv.c | 2 -- > drivers/gpu/drm/drm_sysfs.c | 71 --------------------------------------------- > include/drm/drmP.h | 2 -- > 3 files changed, 75 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index bff8922..8b2582a 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -605,8 +605,6 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, > ret = drm_minor_alloc(dev, DRM_MINOR_CONTROL); > if (ret) > goto err_minors; > - > - WARN_ON(driver->suspend || driver->resume); > } > > if (drm_core_check_feature(dev, DRIVER_RENDER)) { > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index fa7fadc..32dd821 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -32,75 +32,6 @@ static struct device_type drm_sysfs_device_minor = { > > struct class *drm_class; > > -/** > - * __drm_class_suspend - internal DRM class suspend routine > - * @dev: Linux device to suspend > - * @state: power state to enter > - * > - * Just figures out what the actual struct drm_device associated with > - * @dev is and calls its suspend hook, if present. > - */ > -static int __drm_class_suspend(struct device *dev, pm_message_t state) > -{ > - if (dev->type == &drm_sysfs_device_minor) { > - struct drm_minor *drm_minor = to_drm_minor(dev); > - struct drm_device *drm_dev = drm_minor->dev; > - > - if (drm_minor->type == DRM_MINOR_LEGACY && > - !drm_core_check_feature(drm_dev, DRIVER_MODESET) && > - drm_dev->driver->suspend) > - return drm_dev->driver->suspend(drm_dev, state); > - } > - return 0; > -} > - > -/** > - * drm_class_suspend - internal DRM class suspend hook. Simply calls > - * __drm_class_suspend() with the correct pm state. > - * @dev: Linux device to suspend > - */ > -static int drm_class_suspend(struct device *dev) > -{ > - return __drm_class_suspend(dev, PMSG_SUSPEND); > -} > - > -/** > - * drm_class_freeze - internal DRM class freeze hook. Simply calls > - * __drm_class_suspend() with the correct pm state. > - * @dev: Linux device to freeze > - */ > -static int drm_class_freeze(struct device *dev) > -{ > - return __drm_class_suspend(dev, PMSG_FREEZE); > -} > - > -/** > - * drm_class_resume - DRM class resume hook > - * @dev: Linux device to resume > - * > - * Just figures out what the actual struct drm_device associated with > - * @dev is and calls its resume hook, if present. > - */ > -static int drm_class_resume(struct device *dev) > -{ > - if (dev->type == &drm_sysfs_device_minor) { > - struct drm_minor *drm_minor = to_drm_minor(dev); > - struct drm_device *drm_dev = drm_minor->dev; > - > - if (drm_minor->type == DRM_MINOR_LEGACY && > - !drm_core_check_feature(drm_dev, DRIVER_MODESET) && > - drm_dev->driver->resume) > - return drm_dev->driver->resume(drm_dev); > - } > - return 0; > -} > - > -static const struct dev_pm_ops drm_class_dev_pm_ops = { > - .suspend = drm_class_suspend, > - .resume = drm_class_resume, > - .freeze = drm_class_freeze, > -}; > - > static char *drm_devnode(struct device *dev, umode_t *mode) > { > return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev)); > @@ -131,8 +62,6 @@ int drm_sysfs_init(void) > if (IS_ERR(drm_class)) > return PTR_ERR(drm_class); > > - drm_class->pm = &drm_class_dev_pm_ops; > - > err = class_create_file(drm_class, &class_attr_version.attr); > if (err) { > class_destroy(drm_class); > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index c5d2950..fc40543 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -416,8 +416,6 @@ struct drm_driver { > void (*postclose) (struct drm_device *, struct drm_file *); > void (*lastclose) (struct drm_device *); > int (*unload) (struct drm_device *); > - int (*suspend) (struct drm_device *, pm_message_t state); > - int (*resume) (struct drm_device *); > int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv); > int (*dma_quiescent) (struct drm_device *); > int (*context_dtor) (struct drm_device *dev, int context); > -- > 2.8.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index bff8922..8b2582a 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -605,8 +605,6 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, ret = drm_minor_alloc(dev, DRM_MINOR_CONTROL); if (ret) goto err_minors; - - WARN_ON(driver->suspend || driver->resume); } if (drm_core_check_feature(dev, DRIVER_RENDER)) { diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index fa7fadc..32dd821 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -32,75 +32,6 @@ static struct device_type drm_sysfs_device_minor = { struct class *drm_class; -/** - * __drm_class_suspend - internal DRM class suspend routine - * @dev: Linux device to suspend - * @state: power state to enter - * - * Just figures out what the actual struct drm_device associated with - * @dev is and calls its suspend hook, if present. - */ -static int __drm_class_suspend(struct device *dev, pm_message_t state) -{ - if (dev->type == &drm_sysfs_device_minor) { - struct drm_minor *drm_minor = to_drm_minor(dev); - struct drm_device *drm_dev = drm_minor->dev; - - if (drm_minor->type == DRM_MINOR_LEGACY && - !drm_core_check_feature(drm_dev, DRIVER_MODESET) && - drm_dev->driver->suspend) - return drm_dev->driver->suspend(drm_dev, state); - } - return 0; -} - -/** - * drm_class_suspend - internal DRM class suspend hook. Simply calls - * __drm_class_suspend() with the correct pm state. - * @dev: Linux device to suspend - */ -static int drm_class_suspend(struct device *dev) -{ - return __drm_class_suspend(dev, PMSG_SUSPEND); -} - -/** - * drm_class_freeze - internal DRM class freeze hook. Simply calls - * __drm_class_suspend() with the correct pm state. - * @dev: Linux device to freeze - */ -static int drm_class_freeze(struct device *dev) -{ - return __drm_class_suspend(dev, PMSG_FREEZE); -} - -/** - * drm_class_resume - DRM class resume hook - * @dev: Linux device to resume - * - * Just figures out what the actual struct drm_device associated with - * @dev is and calls its resume hook, if present. - */ -static int drm_class_resume(struct device *dev) -{ - if (dev->type == &drm_sysfs_device_minor) { - struct drm_minor *drm_minor = to_drm_minor(dev); - struct drm_device *drm_dev = drm_minor->dev; - - if (drm_minor->type == DRM_MINOR_LEGACY && - !drm_core_check_feature(drm_dev, DRIVER_MODESET) && - drm_dev->driver->resume) - return drm_dev->driver->resume(drm_dev); - } - return 0; -} - -static const struct dev_pm_ops drm_class_dev_pm_ops = { - .suspend = drm_class_suspend, - .resume = drm_class_resume, - .freeze = drm_class_freeze, -}; - static char *drm_devnode(struct device *dev, umode_t *mode) { return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev)); @@ -131,8 +62,6 @@ int drm_sysfs_init(void) if (IS_ERR(drm_class)) return PTR_ERR(drm_class); - drm_class->pm = &drm_class_dev_pm_ops; - err = class_create_file(drm_class, &class_attr_version.attr); if (err) { class_destroy(drm_class); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index c5d2950..fc40543 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -416,8 +416,6 @@ struct drm_driver { void (*postclose) (struct drm_device *, struct drm_file *); void (*lastclose) (struct drm_device *); int (*unload) (struct drm_device *); - int (*suspend) (struct drm_device *, pm_message_t state); - int (*resume) (struct drm_device *); int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv); int (*dma_quiescent) (struct drm_device *); int (*context_dtor) (struct drm_device *dev, int context);
The PM core introduced the ability to keep devices runtime suspended during the entire system suspend/resume process with commit aae4518b3124 ("PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily"). Before this so-called "direct-complete" procedure was introduced, devices were always runtime resumed only to be immediately put to sleep again using their ->suspend hook. Direct-complete is enabled by returning a positive value from the ->prepare hook. The PCI core usually does this automatically. Direct-complete is only available for a device if all children use it as well. Currently we cannot support direct-complete for DRM drivers because the DRM core automatically registers multiple DRM minors which belong to device class drm_class, and drm_class uses a struct dev_pm_ops which lacks the ->prepare callback. While this could be solved by adding the missing ->prepare callback, closer inspection shows that there are no DRM drivers left which declare the legacy ->suspend and ->resume callbacks in their drm_driver struct. The last ones to remove them were i915 with commit 1751fcf9f92e ("drm/i915: Fix module initialisation, v2.") and exynos with commit e7fefb1d5af5 ("drm/exynos: remove legacy ->suspend()/resume()"). Consequently the struct dev_pm_ops of drm_class is now dead code. Remove it. If no dev_pm_ops is declared for a device, the PM core automatically enables direct-complete for it, thereby making that mechanism available to the parent DRM PCI devices. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/gpu/drm/drm_drv.c | 2 -- drivers/gpu/drm/drm_sysfs.c | 71 --------------------------------------------- include/drm/drmP.h | 2 -- 3 files changed, 75 deletions(-)