[PATCH v3 10/12] drm/radeon: Suspend and resume clients with client helpers

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Thomas Zimmermann
Sent: Tuesday, October 8, 2024 4:59 AM
To: simona at ffwll.ch; airlied at gmail.com; javierm at redhat.com; jfalempe at redhat.com
Cc: dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org; Thomas Zimmermann <tzimmermann at suse.de>; Alex Deucher <alexander.deucher at amd.com>; Christian König <christian.koenig at amd.com>; Xinhui Pan <Xinhui.Pan at amd.com>
Subject: [PATCH v3 10/12] drm/radeon: Suspend and resume clients with client helpers
> 
> Replace calls to drm_fb_helper_set_suspend() with calls to the client
> functions drm_client_dev_suspend() and drm_client_dev_resume(). Any
> registered in-kernel client will now receive suspend and resume events.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: "Christian König" <christian.koenig at amd.com>
> Cc: Xinhui Pan <Xinhui.Pan at amd.com>

LGTM, though referring to drm_fb_helper_set_suspend in the commit
message when we're replacing radeon_fbdev_set_suspend, while
functionally correct (as that's what's being called internally), might be
a bit obfuscatory.

It's not a big deal, you don't have to change it.

Though you should probably wait for a radeon expert to review this
patch before deciding to push.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> ---
>  drivers/gpu/drm/radeon/radeon_device.c | 19 ++++++++++---------
>  drivers/gpu/drm/radeon/radeon_fbdev.c  |  6 ------
>  drivers/gpu/drm/radeon/radeon_mode.h   |  3 ---
>  3 files changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 554b236c2328..6f071e61f764 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -35,6 +35,7 @@
>  #include <linux/vgaarb.h>
>  
>  #include <drm/drm_cache.h>
> +#include <drm/drm_client_event.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_file.h>
> @@ -1542,7 +1543,7 @@ void radeon_device_fini(struct radeon_device *rdev)
>   * Called at driver suspend.
>   */
>  int radeon_suspend_kms(struct drm_device *dev, bool suspend,
> -		       bool fbcon, bool freeze)
> +		       bool notify_clients, bool freeze)
>  {
>  	struct radeon_device *rdev;
>  	struct pci_dev *pdev;
> @@ -1634,9 +1635,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>  		pci_set_power_state(pdev, PCI_D3hot);
>  	}
>  
> -	if (fbcon) {
> +	if (notify_clients) {
>  		console_lock();
> -		radeon_fbdev_set_suspend(rdev, 1);
> +		drm_client_dev_suspend(dev, true);
>  		console_unlock();
>  	}
>  	return 0;
> @@ -1649,7 +1650,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>   * Returns 0 for success or an error on failure.
>   * Called at driver resume.
>   */
> -int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
> +int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
>  {
>  	struct drm_connector *connector;
>  	struct radeon_device *rdev = dev->dev_private;
> @@ -1660,14 +1661,14 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> -	if (fbcon) {
> +	if (notify_clients) {
>  		console_lock();
>  	}
>  	if (resume) {
>  		pci_set_power_state(pdev, PCI_D0);
>  		pci_restore_state(pdev);
>  		if (pci_enable_device(pdev)) {
> -			if (fbcon)
> +			if (notify_clients)
>  				console_unlock();
>  			return -1;
>  		}
> @@ -1730,7 +1731,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	/* reset hpd state */
>  	radeon_hpd_init(rdev);
>  	/* blat the mode back in */
> -	if (fbcon) {
> +	if (notify_clients) {
>  		drm_helper_resume_force_mode(dev);
>  		/* turn on display hw */
>  		drm_modeset_lock_all(dev);
> @@ -1746,8 +1747,8 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled)
>  		radeon_pm_compute_clocks(rdev);
>  
> -	if (fbcon) {
> -		radeon_fbdev_set_suspend(rdev, 0);
> +	if (notify_clients) {
> +		drm_client_dev_resume(dev, true);
>  		console_unlock();
>  	}
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
> index 0aa20c8df546..d4a58bd679db 100644
> --- a/drivers/gpu/drm/radeon/radeon_fbdev.c
> +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c
> @@ -288,12 +288,6 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>  	return ret;
>  }
>  
> -void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
> -{
> -	if (rdev_to_drm(rdev)->fb_helper)
> -		drm_fb_helper_set_suspend(rdev_to_drm(rdev)->fb_helper, state);
> -}
> -
>  bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
>  {
>  	struct drm_fb_helper *fb_helper = rdev_to_drm(rdev)->fb_helper;
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 12a1d99a1815..4063d3801e81 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -942,13 +942,10 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>  				    struct drm_fb_helper_surface_size *sizes);
>  #define RADEON_FBDEV_DRIVER_OPS \
>  	.fbdev_probe = radeon_fbdev_driver_fbdev_probe
> -void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state);
>  bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj);
>  #else
>  #define RADEON_FBDEV_DRIVER_OPS \
>  	.fbdev_probe = NULL
> -static inline void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
> -{ }
>  static inline bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
>  {
>  	return false;
> -- 
> 2.46.0
> 
>