drm: Remove dev_pm_ops from drm_class
diff mbox

Message ID da848fcd5ca72a35d9a722e644719977a47bb7ba.1465382836.git.lukas@wunner.de
State New
Headers show

Commit Message

Lukas Wunner June 8, 2016, 10:49 a.m. UTC
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(-)

Comments

Daniel Vetter June 8, 2016, 12:15 p.m. UTC | #1
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

Patch
diff mbox

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);