-----Original Message-----
From: Intel-xe <intel-xe-boun...@lists.freedesktop.org> On Behalf Of Thomas
Zimmermann
Sent: Tuesday, October 8, 2024 4:59 AM
To: sim...@ffwll.ch; airl...@gmail.com; javi...@redhat.com; jfale...@redhat.com
Cc: dri-de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org;
intel-...@lists.freedesktop.org; intel...@lists.freedesktop.org; Thomas
Zimmermann <tzimmerm...@suse.de>; Alex Deucher <alexander.deuc...@amd.com>;
Christian König <christian.koe...@amd.com>; Xinhui Pan <xinhui....@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 <tzimmerm...@suse.de>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: "Christian König" <christian.koe...@amd.com>
> Cc: Xinhui Pan <xinhui....@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.cav...@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
>
>
