[PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Thomas Zimmermann]

No caller of the client resume/suspend helpers holds the console
lock. The last such cases were removed from radeon in the patch
series at [1]. Now remove the related parameter and the TODO items.

v2:
- update placeholders for CONFIG_DRM_CLIENT=n

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/series/151624/ # [1]
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  8 ++++----
 drivers/gpu/drm/clients/drm_fbdev_client.c | 14 ++++----------
 drivers/gpu/drm/clients/drm_log.c          |  4 ++--
 drivers/gpu/drm/drm_client_event.c         | 16 ++++++++--------
 drivers/gpu/drm/drm_modeset_helper.c       |  6 +++---
 drivers/gpu/drm/i915/i915_driver.c         |  6 +++---
 drivers/gpu/drm/nouveau/nouveau_display.c  |  4 ++--
 drivers/gpu/drm/radeon/radeon_device.c     |  4 ++--
 drivers/gpu/drm/xe/display/xe_display.c    |  6 +++---
 include/drm/drm_client.h                   | 14 ++------------
 include/drm/drm_client_event.h             |  8 ++++----
 11 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a77000c2e0bb..f068e26d5080 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5212,7 +5212,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 		dev_warn(adev->dev, "smart shift update failed\n");
 
 	if (notify_clients)
-		drm_client_dev_suspend(adev_to_drm(adev), false);
+		drm_client_dev_suspend(adev_to_drm(adev));
 
 	cancel_delayed_work_sync(&adev->delayed_init_work);
 
@@ -5346,7 +5346,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)
 	flush_delayed_work(&adev->delayed_init_work);
 
 	if (notify_clients)
-		drm_client_dev_resume(adev_to_drm(adev), false);
+		drm_client_dev_resume(adev_to_drm(adev));
 
 	amdgpu_ras_resume(adev);
 
@@ -5951,7 +5951,7 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
 				if (r)
 					goto out;
 
-				drm_client_dev_resume(adev_to_drm(tmp_adev), false);
+				drm_client_dev_resume(adev_to_drm(tmp_adev));
 
 				/*
 				 * The GPU enters bad state once faulty pages
@@ -6286,7 +6286,7 @@ static void amdgpu_device_halt_activities(struct amdgpu_device *adev,
 		 */
 		amdgpu_unregister_gpu_instance(tmp_adev);
 
-		drm_client_dev_suspend(adev_to_drm(tmp_adev), false);
+		drm_client_dev_suspend(adev_to_drm(tmp_adev));
 
 		/* disable ras on ALL IPs */
 		if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev) &&
diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c b/drivers/gpu/drm/clients/drm_fbdev_client.c
index f894ba52bdb5..ec5ab9f30547 100644
--- a/drivers/gpu/drm/clients/drm_fbdev_client.c
+++ b/drivers/gpu/drm/clients/drm_fbdev_client.c
@@ -62,26 +62,20 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 	return ret;
 }
 
-static int drm_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
+static int drm_fbdev_client_suspend(struct drm_client_dev *client)
 {
 	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
 
-	if (holds_console_lock)
-		drm_fb_helper_set_suspend(fb_helper, true);
-	else
-		drm_fb_helper_set_suspend_unlocked(fb_helper, true);
+	drm_fb_helper_set_suspend_unlocked(fb_helper, true);
 
 	return 0;
 }
 
-static int drm_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock)
+static int drm_fbdev_client_resume(struct drm_client_dev *client)
 {
 	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
 
-	if (holds_console_lock)
-		drm_fb_helper_set_suspend(fb_helper, false);
-	else
-		drm_fb_helper_set_suspend_unlocked(fb_helper, false);
+	drm_fb_helper_set_suspend_unlocked(fb_helper, false);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index d239f1e3c456..fd8556dd58ed 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -319,7 +319,7 @@ static int drm_log_client_hotplug(struct drm_client_dev *client)
 	return 0;
 }
 
-static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_lock)
+static int drm_log_client_suspend(struct drm_client_dev *client)
 {
 	struct drm_log *dlog = client_to_drm_log(client);
 
@@ -328,7 +328,7 @@ static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_l
 	return 0;
 }
 
-static int drm_log_client_resume(struct drm_client_dev *client, bool _console_lock)
+static int drm_log_client_resume(struct drm_client_dev *client)
 {
 	struct drm_log *dlog = client_to_drm_log(client);
 
diff --git a/drivers/gpu/drm/drm_client_event.c b/drivers/gpu/drm/drm_client_event.c
index c83196ad8b59..c3baeb4d4e6b 100644
--- a/drivers/gpu/drm/drm_client_event.c
+++ b/drivers/gpu/drm/drm_client_event.c
@@ -122,7 +122,7 @@ void drm_client_dev_restore(struct drm_device *dev)
 	mutex_unlock(&dev->clientlist_mutex);
 }
 
-static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
+static int drm_client_suspend(struct drm_client_dev *client)
 {
 	struct drm_device *dev = client->dev;
 	int ret = 0;
@@ -131,7 +131,7 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
 		return 0;
 
 	if (client->funcs && client->funcs->suspend)
-		ret = client->funcs->suspend(client, holds_console_lock);
+		ret = client->funcs->suspend(client);
 	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
 
 	client->suspended = true;
@@ -139,20 +139,20 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
 	return ret;
 }
 
-void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
+void drm_client_dev_suspend(struct drm_device *dev)
 {
 	struct drm_client_dev *client;
 
 	mutex_lock(&dev->clientlist_mutex);
 	list_for_each_entry(client, &dev->clientlist, list) {
 		if (!client->suspended)
-			drm_client_suspend(client, holds_console_lock);
+			drm_client_suspend(client);
 	}
 	mutex_unlock(&dev->clientlist_mutex);
 }
 EXPORT_SYMBOL(drm_client_dev_suspend);
 
-static int drm_client_resume(struct drm_client_dev *client, bool holds_console_lock)
+static int drm_client_resume(struct drm_client_dev *client)
 {
 	struct drm_device *dev = client->dev;
 	int ret = 0;
@@ -161,7 +161,7 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
 		return 0;
 
 	if (client->funcs && client->funcs->resume)
-		ret = client->funcs->resume(client, holds_console_lock);
+		ret = client->funcs->resume(client);
 	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
 
 	client->suspended = false;
@@ -172,14 +172,14 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
 	return ret;
 }
 
-void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
+void drm_client_dev_resume(struct drm_device *dev)
 {
 	struct drm_client_dev *client;
 
 	mutex_lock(&dev->clientlist_mutex);
 	list_for_each_entry(client, &dev->clientlist, list) {
 		if  (client->suspended)
-			drm_client_resume(client, holds_console_lock);
+			drm_client_resume(client);
 	}
 	mutex_unlock(&dev->clientlist_mutex);
 }
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 988735560570..a57f6a10ada4 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -203,10 +203,10 @@ int drm_mode_config_helper_suspend(struct drm_device *dev)
 	if (dev->mode_config.poll_enabled)
 		drm_kms_helper_poll_disable(dev);
 
-	drm_client_dev_suspend(dev, false);
+	drm_client_dev_suspend(dev);
 	state = drm_atomic_helper_suspend(dev);
 	if (IS_ERR(state)) {
-		drm_client_dev_resume(dev, false);
+		drm_client_dev_resume(dev);
 
 		/*
 		 * Don't enable polling if it was never initialized
@@ -252,7 +252,7 @@ int drm_mode_config_helper_resume(struct drm_device *dev)
 		DRM_ERROR("Failed to resume (%d)\n", ret);
 	dev->mode_config.suspend_state = NULL;
 
-	drm_client_dev_resume(dev, false);
+	drm_client_dev_resume(dev);
 
 	/*
 	 * Don't enable polling if it is not initialized
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 95165e45de74..162e50315beb 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -978,7 +978,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 	intel_runtime_pm_disable(&i915->runtime_pm);
 	intel_power_domains_disable(display);
 
-	drm_client_dev_suspend(&i915->drm, false);
+	drm_client_dev_suspend(&i915->drm);
 	if (intel_display_device_present(display)) {
 		drm_kms_helper_poll_disable(&i915->drm);
 		intel_display_driver_disable_user_access(display);
@@ -1060,7 +1060,7 @@ static int i915_drm_suspend(struct drm_device *dev)
 	/* We do a lot of poking in a lot of registers, make sure they work
 	 * properly. */
 	intel_power_domains_disable(display);
-	drm_client_dev_suspend(dev, false);
+	drm_client_dev_suspend(dev);
 	if (intel_display_device_present(display)) {
 		drm_kms_helper_poll_disable(dev);
 		intel_display_driver_disable_user_access(display);
@@ -1257,7 +1257,7 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_opregion_resume(display);
 
-	drm_client_dev_resume(dev, false);
+	drm_client_dev_resume(dev);
 
 	intel_power_domains_enable(display);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 54aed3656a4c..00515623a2cc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -765,7 +765,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
 {
 	struct nouveau_display *disp = nouveau_display(dev);
 
-	drm_client_dev_suspend(dev, false);
+	drm_client_dev_suspend(dev);
 
 	if (drm_drv_uses_atomic_modeset(dev)) {
 		if (!runtime) {
@@ -796,7 +796,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
 		}
 	}
 
-	drm_client_dev_resume(dev, false);
+	drm_client_dev_resume(dev);
 }
 
 int
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 9e35b14e2bf0..60afaa8e56b4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1635,7 +1635,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 	}
 
 	if (notify_clients)
-		drm_client_dev_suspend(dev, false);
+		drm_client_dev_suspend(dev);
 
 	return 0;
 }
@@ -1739,7 +1739,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
 		radeon_pm_compute_clocks(rdev);
 
 	if (notify_clients)
-		drm_client_dev_resume(dev, false);
+		drm_client_dev_resume(dev);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 19e691fccf8c..d3cc6181842c 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -324,7 +324,7 @@ void xe_display_pm_suspend(struct xe_device *xe)
 	 * properly.
 	 */
 	intel_power_domains_disable(display);
-	drm_client_dev_suspend(&xe->drm, false);
+	drm_client_dev_suspend(&xe->drm);
 
 	if (intel_display_device_present(display)) {
 		drm_kms_helper_poll_disable(&xe->drm);
@@ -356,7 +356,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)
 		return;
 
 	intel_power_domains_disable(display);
-	drm_client_dev_suspend(&xe->drm, false);
+	drm_client_dev_suspend(&xe->drm);
 
 	if (intel_display_device_present(display)) {
 		drm_kms_helper_poll_disable(&xe->drm);
@@ -481,7 +481,7 @@ void xe_display_pm_resume(struct xe_device *xe)
 
 	intel_opregion_resume(display);
 
-	drm_client_dev_resume(&xe->drm, false);
+	drm_client_dev_resume(&xe->drm);
 
 	intel_power_domains_enable(display);
 }
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index bdd845e383ef..3556928d3938 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -70,13 +70,8 @@ struct drm_client_funcs {
 	 * Called when suspending the device.
 	 *
 	 * This callback is optional.
-	 *
-	 * FIXME: Some callers hold the console lock when invoking this
-	 *        function. This interferes with fbdev emulation, which
-	 *        also tries to acquire the lock. Push the console lock
-	 *        into the callback and remove 'holds_console_lock'.
 	 */
-	int (*suspend)(struct drm_client_dev *client, bool holds_console_lock);
+	int (*suspend)(struct drm_client_dev *client);
 
 	/**
 	 * @resume:
@@ -84,13 +79,8 @@ struct drm_client_funcs {
 	 * Called when resuming the device from suspend.
 	 *
 	 * This callback is optional.
-	 *
-	 * FIXME: Some callers hold the console lock when invoking this
-	 *        function. This interferes with fbdev emulation, which
-	 *        also tries to acquire the lock. Push the console lock
-	 *        into the callback and remove 'holds_console_lock'.
 	 */
-	int (*resume)(struct drm_client_dev *client, bool holds_console_lock);
+	int (*resume)(struct drm_client_dev *client);
 };
 
 /**
diff --git a/include/drm/drm_client_event.h b/include/drm/drm_client_event.h
index 1d544d3a3228..985d6f02a4c4 100644
--- a/include/drm/drm_client_event.h
+++ b/include/drm/drm_client_event.h
@@ -11,8 +11,8 @@ struct drm_device;
 void drm_client_dev_unregister(struct drm_device *dev);
 void drm_client_dev_hotplug(struct drm_device *dev);
 void drm_client_dev_restore(struct drm_device *dev);
-void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock);
-void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock);
+void drm_client_dev_suspend(struct drm_device *dev);
+void drm_client_dev_resume(struct drm_device *dev);
 #else
 static inline void drm_client_dev_unregister(struct drm_device *dev)
 { }
@@ -20,9 +20,9 @@ static inline void drm_client_dev_hotplug(struct drm_device *dev)
 { }
 static inline void drm_client_dev_restore(struct drm_device *dev)
 { }
-static inline void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
+static inline void drm_client_dev_suspend(struct drm_device *dev)
 { }
-static inline void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
+static inline void drm_client_dev_resume(struct drm_device *dev)
 { }
 #endif
 
-- 
2.51.0

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Rodrigo Vivi]

On Wed, Oct 01, 2025 at 04:37:03PM +0200, Thomas Zimmermann wrote:
> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.
> 
> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/151624/ # [1]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
(for both xe and i915 changes)

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  8 ++++----
>  drivers/gpu/drm/clients/drm_fbdev_client.c | 14 ++++----------
>  drivers/gpu/drm/clients/drm_log.c          |  4 ++--
>  drivers/gpu/drm/drm_client_event.c         | 16 ++++++++--------
>  drivers/gpu/drm/drm_modeset_helper.c       |  6 +++---
>  drivers/gpu/drm/i915/i915_driver.c         |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_display.c  |  4 ++--
>  drivers/gpu/drm/radeon/radeon_device.c     |  4 ++--
>  drivers/gpu/drm/xe/display/xe_display.c    |  6 +++---
>  include/drm/drm_client.h                   | 14 ++------------
>  include/drm/drm_client_event.h             |  8 ++++----
>  11 files changed, 37 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a77000c2e0bb..f068e26d5080 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5212,7 +5212,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>  		dev_warn(adev->dev, "smart shift update failed\n");
>  
>  	if (notify_clients)
> -		drm_client_dev_suspend(adev_to_drm(adev), false);
> +		drm_client_dev_suspend(adev_to_drm(adev));
>  
>  	cancel_delayed_work_sync(&adev->delayed_init_work);
>  
> @@ -5346,7 +5346,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)
>  	flush_delayed_work(&adev->delayed_init_work);
>  
>  	if (notify_clients)
> -		drm_client_dev_resume(adev_to_drm(adev), false);
> +		drm_client_dev_resume(adev_to_drm(adev));
>  
>  	amdgpu_ras_resume(adev);
>  
> @@ -5951,7 +5951,7 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
>  				if (r)
>  					goto out;
>  
> -				drm_client_dev_resume(adev_to_drm(tmp_adev), false);
> +				drm_client_dev_resume(adev_to_drm(tmp_adev));
>  
>  				/*
>  				 * The GPU enters bad state once faulty pages
> @@ -6286,7 +6286,7 @@ static void amdgpu_device_halt_activities(struct amdgpu_device *adev,
>  		 */
>  		amdgpu_unregister_gpu_instance(tmp_adev);
>  
> -		drm_client_dev_suspend(adev_to_drm(tmp_adev), false);
> +		drm_client_dev_suspend(adev_to_drm(tmp_adev));
>  
>  		/* disable ras on ALL IPs */
>  		if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev) &&
> diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c b/drivers/gpu/drm/clients/drm_fbdev_client.c
> index f894ba52bdb5..ec5ab9f30547 100644
> --- a/drivers/gpu/drm/clients/drm_fbdev_client.c
> +++ b/drivers/gpu/drm/clients/drm_fbdev_client.c
> @@ -62,26 +62,20 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>  	return ret;
>  }
>  
> -static int drm_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_fbdev_client_suspend(struct drm_client_dev *client)
>  {
>  	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
>  
> -	if (holds_console_lock)
> -		drm_fb_helper_set_suspend(fb_helper, true);
> -	else
> -		drm_fb_helper_set_suspend_unlocked(fb_helper, true);
> +	drm_fb_helper_set_suspend_unlocked(fb_helper, true);
>  
>  	return 0;
>  }
>  
> -static int drm_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_fbdev_client_resume(struct drm_client_dev *client)
>  {
>  	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
>  
> -	if (holds_console_lock)
> -		drm_fb_helper_set_suspend(fb_helper, false);
> -	else
> -		drm_fb_helper_set_suspend_unlocked(fb_helper, false);
> +	drm_fb_helper_set_suspend_unlocked(fb_helper, false);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index d239f1e3c456..fd8556dd58ed 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -319,7 +319,7 @@ static int drm_log_client_hotplug(struct drm_client_dev *client)
>  	return 0;
>  }
>  
> -static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_lock)
> +static int drm_log_client_suspend(struct drm_client_dev *client)
>  {
>  	struct drm_log *dlog = client_to_drm_log(client);
>  
> @@ -328,7 +328,7 @@ static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_l
>  	return 0;
>  }
>  
> -static int drm_log_client_resume(struct drm_client_dev *client, bool _console_lock)
> +static int drm_log_client_resume(struct drm_client_dev *client)
>  {
>  	struct drm_log *dlog = client_to_drm_log(client);
>  
> diff --git a/drivers/gpu/drm/drm_client_event.c b/drivers/gpu/drm/drm_client_event.c
> index c83196ad8b59..c3baeb4d4e6b 100644
> --- a/drivers/gpu/drm/drm_client_event.c
> +++ b/drivers/gpu/drm/drm_client_event.c
> @@ -122,7 +122,7 @@ void drm_client_dev_restore(struct drm_device *dev)
>  	mutex_unlock(&dev->clientlist_mutex);
>  }
>  
> -static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_client_suspend(struct drm_client_dev *client)
>  {
>  	struct drm_device *dev = client->dev;
>  	int ret = 0;
> @@ -131,7 +131,7 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
>  		return 0;
>  
>  	if (client->funcs && client->funcs->suspend)
> -		ret = client->funcs->suspend(client, holds_console_lock);
> +		ret = client->funcs->suspend(client);
>  	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
>  
>  	client->suspended = true;
> @@ -139,20 +139,20 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
>  	return ret;
>  }
>  
> -void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
> +void drm_client_dev_suspend(struct drm_device *dev)
>  {
>  	struct drm_client_dev *client;
>  
>  	mutex_lock(&dev->clientlist_mutex);
>  	list_for_each_entry(client, &dev->clientlist, list) {
>  		if (!client->suspended)
> -			drm_client_suspend(client, holds_console_lock);
> +			drm_client_suspend(client);
>  	}
>  	mutex_unlock(&dev->clientlist_mutex);
>  }
>  EXPORT_SYMBOL(drm_client_dev_suspend);
>  
> -static int drm_client_resume(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_client_resume(struct drm_client_dev *client)
>  {
>  	struct drm_device *dev = client->dev;
>  	int ret = 0;
> @@ -161,7 +161,7 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
>  		return 0;
>  
>  	if (client->funcs && client->funcs->resume)
> -		ret = client->funcs->resume(client, holds_console_lock);
> +		ret = client->funcs->resume(client);
>  	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
>  
>  	client->suspended = false;
> @@ -172,14 +172,14 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
>  	return ret;
>  }
>  
> -void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
> +void drm_client_dev_resume(struct drm_device *dev)
>  {
>  	struct drm_client_dev *client;
>  
>  	mutex_lock(&dev->clientlist_mutex);
>  	list_for_each_entry(client, &dev->clientlist, list) {
>  		if  (client->suspended)
> -			drm_client_resume(client, holds_console_lock);
> +			drm_client_resume(client);
>  	}
>  	mutex_unlock(&dev->clientlist_mutex);
>  }
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 988735560570..a57f6a10ada4 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -203,10 +203,10 @@ int drm_mode_config_helper_suspend(struct drm_device *dev)
>  	if (dev->mode_config.poll_enabled)
>  		drm_kms_helper_poll_disable(dev);
>  
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>  	state = drm_atomic_helper_suspend(dev);
>  	if (IS_ERR(state)) {
> -		drm_client_dev_resume(dev, false);
> +		drm_client_dev_resume(dev);
>  
>  		/*
>  		 * Don't enable polling if it was never initialized
> @@ -252,7 +252,7 @@ int drm_mode_config_helper_resume(struct drm_device *dev)
>  		DRM_ERROR("Failed to resume (%d)\n", ret);
>  	dev->mode_config.suspend_state = NULL;
>  
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>  
>  	/*
>  	 * Don't enable polling if it is not initialized
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 95165e45de74..162e50315beb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -978,7 +978,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	intel_runtime_pm_disable(&i915->runtime_pm);
>  	intel_power_domains_disable(display);
>  
> -	drm_client_dev_suspend(&i915->drm, false);
> +	drm_client_dev_suspend(&i915->drm);
>  	if (intel_display_device_present(display)) {
>  		drm_kms_helper_poll_disable(&i915->drm);
>  		intel_display_driver_disable_user_access(display);
> @@ -1060,7 +1060,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>  	/* We do a lot of poking in a lot of registers, make sure they work
>  	 * properly. */
>  	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>  	if (intel_display_device_present(display)) {
>  		drm_kms_helper_poll_disable(dev);
>  		intel_display_driver_disable_user_access(display);
> @@ -1257,7 +1257,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_opregion_resume(display);
>  
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>  
>  	intel_power_domains_enable(display);
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 54aed3656a4c..00515623a2cc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -765,7 +765,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
>  {
>  	struct nouveau_display *disp = nouveau_display(dev);
>  
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>  
>  	if (drm_drv_uses_atomic_modeset(dev)) {
>  		if (!runtime) {
> @@ -796,7 +796,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
>  		}
>  	}
>  
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>  }
>  
>  int
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 9e35b14e2bf0..60afaa8e56b4 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1635,7 +1635,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>  	}
>  
>  	if (notify_clients)
> -		drm_client_dev_suspend(dev, false);
> +		drm_client_dev_suspend(dev);
>  
>  	return 0;
>  }
> @@ -1739,7 +1739,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
>  		radeon_pm_compute_clocks(rdev);
>  
>  	if (notify_clients)
> -		drm_client_dev_resume(dev, false);
> +		drm_client_dev_resume(dev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 19e691fccf8c..d3cc6181842c 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -324,7 +324,7 @@ void xe_display_pm_suspend(struct xe_device *xe)
>  	 * properly.
>  	 */
>  	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(&xe->drm, false);
> +	drm_client_dev_suspend(&xe->drm);
>  
>  	if (intel_display_device_present(display)) {
>  		drm_kms_helper_poll_disable(&xe->drm);
> @@ -356,7 +356,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)
>  		return;
>  
>  	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(&xe->drm, false);
> +	drm_client_dev_suspend(&xe->drm);
>  
>  	if (intel_display_device_present(display)) {
>  		drm_kms_helper_poll_disable(&xe->drm);
> @@ -481,7 +481,7 @@ void xe_display_pm_resume(struct xe_device *xe)
>  
>  	intel_opregion_resume(display);
>  
> -	drm_client_dev_resume(&xe->drm, false);
> +	drm_client_dev_resume(&xe->drm);
>  
>  	intel_power_domains_enable(display);
>  }
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index bdd845e383ef..3556928d3938 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -70,13 +70,8 @@ struct drm_client_funcs {
>  	 * Called when suspending the device.
>  	 *
>  	 * This callback is optional.
> -	 *
> -	 * FIXME: Some callers hold the console lock when invoking this
> -	 *        function. This interferes with fbdev emulation, which
> -	 *        also tries to acquire the lock. Push the console lock
> -	 *        into the callback and remove 'holds_console_lock'.
>  	 */
> -	int (*suspend)(struct drm_client_dev *client, bool holds_console_lock);
> +	int (*suspend)(struct drm_client_dev *client);
>  
>  	/**
>  	 * @resume:
> @@ -84,13 +79,8 @@ struct drm_client_funcs {
>  	 * Called when resuming the device from suspend.
>  	 *
>  	 * This callback is optional.
> -	 *
> -	 * FIXME: Some callers hold the console lock when invoking this
> -	 *        function. This interferes with fbdev emulation, which
> -	 *        also tries to acquire the lock. Push the console lock
> -	 *        into the callback and remove 'holds_console_lock'.
>  	 */
> -	int (*resume)(struct drm_client_dev *client, bool holds_console_lock);
> +	int (*resume)(struct drm_client_dev *client);
>  };
>  
>  /**
> diff --git a/include/drm/drm_client_event.h b/include/drm/drm_client_event.h
> index 1d544d3a3228..985d6f02a4c4 100644
> --- a/include/drm/drm_client_event.h
> +++ b/include/drm/drm_client_event.h
> @@ -11,8 +11,8 @@ struct drm_device;
>  void drm_client_dev_unregister(struct drm_device *dev);
>  void drm_client_dev_hotplug(struct drm_device *dev);
>  void drm_client_dev_restore(struct drm_device *dev);
> -void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock);
> -void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock);
> +void drm_client_dev_suspend(struct drm_device *dev);
> +void drm_client_dev_resume(struct drm_device *dev);
>  #else
>  static inline void drm_client_dev_unregister(struct drm_device *dev)
>  { }
> @@ -20,9 +20,9 @@ static inline void drm_client_dev_hotplug(struct drm_device *dev)
>  { }
>  static inline void drm_client_dev_restore(struct drm_device *dev)
>  { }
> -static inline void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
> +static inline void drm_client_dev_suspend(struct drm_device *dev)
>  { }
> -static inline void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
> +static inline void drm_client_dev_resume(struct drm_device *dev)
>  { }
>  #endif
>  
> -- 
> 2.51.0
> 

✗ i915.CI.BAT: failure for drm/client: Remove holds_console_lock parameter from suspend/resume (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3612 bytes --]

== Series Details ==

Series: drm/client: Remove holds_console_lock parameter from suspend/resume (rev2)
URL   : https://patchwork.freedesktop.org/series/155288/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_17294 -> Patchwork_155288v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_155288v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_155288v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/index.html

Participating hosts (44 -> 42)
------------------------------

  Missing    (2): bat-arlh-2 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_155288v2:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - bat-rplp-1:         [PASS][1] -> [DMESG-WARN][2] +1 other test dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17294/bat-rplp-1/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/bat-rplp-1/igt@i915_selftest@live.html

  
Known issues
------------

  Here are the changes found in Patchwork_155288v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-bsw-nick:        [PASS][3] -> [ABORT][4] ([i915#12904]) +1 other test abort
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17294/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@i915_module_load@load:
    - bat-mtlp-9:         [PASS][5] -> [DMESG-WARN][6] ([i915#13494])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17294/bat-mtlp-9/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/bat-mtlp-9/igt@i915_module_load@load.html

  * igt@i915_selftest@live@sanitycheck:
    - bat-dg2-14:         [PASS][7] -> [ABORT][8] ([i915#14201]) +1 other test abort
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17294/bat-dg2-14/igt@i915_selftest@live@sanitycheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/bat-dg2-14/igt@i915_selftest@live@sanitycheck.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests:
    - bat-apl-1:          [ABORT][9] ([i915#12904]) -> [PASS][10] +1 other test pass
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17294/bat-apl-1/igt@dmabuf@all-tests.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/bat-apl-1/igt@dmabuf@all-tests.html

  
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904
  [i915#13494]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13494
  [i915#14201]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14201


Build changes
-------------

  * Linux: CI_DRM_17294 -> Patchwork_155288v2

  CI-20190529: 20190529
  CI_DRM_17294: 5f87abb254011980c8332008cfff72d2cfab4952 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8568: 8568
  Patchwork_155288v2: 5f87abb254011980c8332008cfff72d2cfab4952 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155288v2/index.html

[-- Attachment #2: Type: text/html, Size: 4404 bytes --]

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Petr Vorel]

Hi Thomas,

> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.

> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
...

LGTM now.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Andi Shyti]

Hi Thomas,

On Wed, Oct 01, 2025 at 04:37:03PM +0200, Thomas Zimmermann wrote:
> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.
> 
> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/151624/ # [1]

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Danilo Krummrich]

On 10/1/25 4:37 PM, Thomas Zimmermann wrote:
> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.
> 
> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/151624/ # [1]
Acked-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Thomas Zimmermann]

Reviewed-by: Lyude Paul <lyude@redhat.com>

via irc: 
https://people.freedesktop.org/~cbrill/dri-log/?channel=nouveau&highlight_names=&date=2025-10-13&show_html=true

Am 01.10.25 um 16:37 schrieb Thomas Zimmermann:
> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.
>
> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/151624/ # [1]
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  8 ++++----
>   drivers/gpu/drm/clients/drm_fbdev_client.c | 14 ++++----------
>   drivers/gpu/drm/clients/drm_log.c          |  4 ++--
>   drivers/gpu/drm/drm_client_event.c         | 16 ++++++++--------
>   drivers/gpu/drm/drm_modeset_helper.c       |  6 +++---
>   drivers/gpu/drm/i915/i915_driver.c         |  6 +++---
>   drivers/gpu/drm/nouveau/nouveau_display.c  |  4 ++--
>   drivers/gpu/drm/radeon/radeon_device.c     |  4 ++--
>   drivers/gpu/drm/xe/display/xe_display.c    |  6 +++---
>   include/drm/drm_client.h                   | 14 ++------------
>   include/drm/drm_client_event.h             |  8 ++++----
>   11 files changed, 37 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a77000c2e0bb..f068e26d5080 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5212,7 +5212,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>   		dev_warn(adev->dev, "smart shift update failed\n");
>   
>   	if (notify_clients)
> -		drm_client_dev_suspend(adev_to_drm(adev), false);
> +		drm_client_dev_suspend(adev_to_drm(adev));
>   
>   	cancel_delayed_work_sync(&adev->delayed_init_work);
>   
> @@ -5346,7 +5346,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)
>   	flush_delayed_work(&adev->delayed_init_work);
>   
>   	if (notify_clients)
> -		drm_client_dev_resume(adev_to_drm(adev), false);
> +		drm_client_dev_resume(adev_to_drm(adev));
>   
>   	amdgpu_ras_resume(adev);
>   
> @@ -5951,7 +5951,7 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
>   				if (r)
>   					goto out;
>   
> -				drm_client_dev_resume(adev_to_drm(tmp_adev), false);
> +				drm_client_dev_resume(adev_to_drm(tmp_adev));
>   
>   				/*
>   				 * The GPU enters bad state once faulty pages
> @@ -6286,7 +6286,7 @@ static void amdgpu_device_halt_activities(struct amdgpu_device *adev,
>   		 */
>   		amdgpu_unregister_gpu_instance(tmp_adev);
>   
> -		drm_client_dev_suspend(adev_to_drm(tmp_adev), false);
> +		drm_client_dev_suspend(adev_to_drm(tmp_adev));
>   
>   		/* disable ras on ALL IPs */
>   		if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev) &&
> diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c b/drivers/gpu/drm/clients/drm_fbdev_client.c
> index f894ba52bdb5..ec5ab9f30547 100644
> --- a/drivers/gpu/drm/clients/drm_fbdev_client.c
> +++ b/drivers/gpu/drm/clients/drm_fbdev_client.c
> @@ -62,26 +62,20 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>   	return ret;
>   }
>   
> -static int drm_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_fbdev_client_suspend(struct drm_client_dev *client)
>   {
>   	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
>   
> -	if (holds_console_lock)
> -		drm_fb_helper_set_suspend(fb_helper, true);
> -	else
> -		drm_fb_helper_set_suspend_unlocked(fb_helper, true);
> +	drm_fb_helper_set_suspend_unlocked(fb_helper, true);
>   
>   	return 0;
>   }
>   
> -static int drm_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_fbdev_client_resume(struct drm_client_dev *client)
>   {
>   	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
>   
> -	if (holds_console_lock)
> -		drm_fb_helper_set_suspend(fb_helper, false);
> -	else
> -		drm_fb_helper_set_suspend_unlocked(fb_helper, false);
> +	drm_fb_helper_set_suspend_unlocked(fb_helper, false);
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index d239f1e3c456..fd8556dd58ed 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -319,7 +319,7 @@ static int drm_log_client_hotplug(struct drm_client_dev *client)
>   	return 0;
>   }
>   
> -static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_lock)
> +static int drm_log_client_suspend(struct drm_client_dev *client)
>   {
>   	struct drm_log *dlog = client_to_drm_log(client);
>   
> @@ -328,7 +328,7 @@ static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_l
>   	return 0;
>   }
>   
> -static int drm_log_client_resume(struct drm_client_dev *client, bool _console_lock)
> +static int drm_log_client_resume(struct drm_client_dev *client)
>   {
>   	struct drm_log *dlog = client_to_drm_log(client);
>   
> diff --git a/drivers/gpu/drm/drm_client_event.c b/drivers/gpu/drm/drm_client_event.c
> index c83196ad8b59..c3baeb4d4e6b 100644
> --- a/drivers/gpu/drm/drm_client_event.c
> +++ b/drivers/gpu/drm/drm_client_event.c
> @@ -122,7 +122,7 @@ void drm_client_dev_restore(struct drm_device *dev)
>   	mutex_unlock(&dev->clientlist_mutex);
>   }
>   
> -static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_client_suspend(struct drm_client_dev *client)
>   {
>   	struct drm_device *dev = client->dev;
>   	int ret = 0;
> @@ -131,7 +131,7 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
>   		return 0;
>   
>   	if (client->funcs && client->funcs->suspend)
> -		ret = client->funcs->suspend(client, holds_console_lock);
> +		ret = client->funcs->suspend(client);
>   	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
>   
>   	client->suspended = true;
> @@ -139,20 +139,20 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_
>   	return ret;
>   }
>   
> -void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
> +void drm_client_dev_suspend(struct drm_device *dev)
>   {
>   	struct drm_client_dev *client;
>   
>   	mutex_lock(&dev->clientlist_mutex);
>   	list_for_each_entry(client, &dev->clientlist, list) {
>   		if (!client->suspended)
> -			drm_client_suspend(client, holds_console_lock);
> +			drm_client_suspend(client);
>   	}
>   	mutex_unlock(&dev->clientlist_mutex);
>   }
>   EXPORT_SYMBOL(drm_client_dev_suspend);
>   
> -static int drm_client_resume(struct drm_client_dev *client, bool holds_console_lock)
> +static int drm_client_resume(struct drm_client_dev *client)
>   {
>   	struct drm_device *dev = client->dev;
>   	int ret = 0;
> @@ -161,7 +161,7 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
>   		return 0;
>   
>   	if (client->funcs && client->funcs->resume)
> -		ret = client->funcs->resume(client, holds_console_lock);
> +		ret = client->funcs->resume(client);
>   	drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
>   
>   	client->suspended = false;
> @@ -172,14 +172,14 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_l
>   	return ret;
>   }
>   
> -void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
> +void drm_client_dev_resume(struct drm_device *dev)
>   {
>   	struct drm_client_dev *client;
>   
>   	mutex_lock(&dev->clientlist_mutex);
>   	list_for_each_entry(client, &dev->clientlist, list) {
>   		if  (client->suspended)
> -			drm_client_resume(client, holds_console_lock);
> +			drm_client_resume(client);
>   	}
>   	mutex_unlock(&dev->clientlist_mutex);
>   }
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 988735560570..a57f6a10ada4 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -203,10 +203,10 @@ int drm_mode_config_helper_suspend(struct drm_device *dev)
>   	if (dev->mode_config.poll_enabled)
>   		drm_kms_helper_poll_disable(dev);
>   
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>   	state = drm_atomic_helper_suspend(dev);
>   	if (IS_ERR(state)) {
> -		drm_client_dev_resume(dev, false);
> +		drm_client_dev_resume(dev);
>   
>   		/*
>   		 * Don't enable polling if it was never initialized
> @@ -252,7 +252,7 @@ int drm_mode_config_helper_resume(struct drm_device *dev)
>   		DRM_ERROR("Failed to resume (%d)\n", ret);
>   	dev->mode_config.suspend_state = NULL;
>   
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>   
>   	/*
>   	 * Don't enable polling if it is not initialized
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 95165e45de74..162e50315beb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -978,7 +978,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>   	intel_runtime_pm_disable(&i915->runtime_pm);
>   	intel_power_domains_disable(display);
>   
> -	drm_client_dev_suspend(&i915->drm, false);
> +	drm_client_dev_suspend(&i915->drm);
>   	if (intel_display_device_present(display)) {
>   		drm_kms_helper_poll_disable(&i915->drm);
>   		intel_display_driver_disable_user_access(display);
> @@ -1060,7 +1060,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>   	/* We do a lot of poking in a lot of registers, make sure they work
>   	 * properly. */
>   	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>   	if (intel_display_device_present(display)) {
>   		drm_kms_helper_poll_disable(dev);
>   		intel_display_driver_disable_user_access(display);
> @@ -1257,7 +1257,7 @@ static int i915_drm_resume(struct drm_device *dev)
>   
>   	intel_opregion_resume(display);
>   
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>   
>   	intel_power_domains_enable(display);
>   
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 54aed3656a4c..00515623a2cc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -765,7 +765,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
>   {
>   	struct nouveau_display *disp = nouveau_display(dev);
>   
> -	drm_client_dev_suspend(dev, false);
> +	drm_client_dev_suspend(dev);
>   
>   	if (drm_drv_uses_atomic_modeset(dev)) {
>   		if (!runtime) {
> @@ -796,7 +796,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
>   		}
>   	}
>   
> -	drm_client_dev_resume(dev, false);
> +	drm_client_dev_resume(dev);
>   }
>   
>   int
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 9e35b14e2bf0..60afaa8e56b4 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1635,7 +1635,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>   	}
>   
>   	if (notify_clients)
> -		drm_client_dev_suspend(dev, false);
> +		drm_client_dev_suspend(dev);
>   
>   	return 0;
>   }
> @@ -1739,7 +1739,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
>   		radeon_pm_compute_clocks(rdev);
>   
>   	if (notify_clients)
> -		drm_client_dev_resume(dev, false);
> +		drm_client_dev_resume(dev);
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 19e691fccf8c..d3cc6181842c 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -324,7 +324,7 @@ void xe_display_pm_suspend(struct xe_device *xe)
>   	 * properly.
>   	 */
>   	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(&xe->drm, false);
> +	drm_client_dev_suspend(&xe->drm);
>   
>   	if (intel_display_device_present(display)) {
>   		drm_kms_helper_poll_disable(&xe->drm);
> @@ -356,7 +356,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)
>   		return;
>   
>   	intel_power_domains_disable(display);
> -	drm_client_dev_suspend(&xe->drm, false);
> +	drm_client_dev_suspend(&xe->drm);
>   
>   	if (intel_display_device_present(display)) {
>   		drm_kms_helper_poll_disable(&xe->drm);
> @@ -481,7 +481,7 @@ void xe_display_pm_resume(struct xe_device *xe)
>   
>   	intel_opregion_resume(display);
>   
> -	drm_client_dev_resume(&xe->drm, false);
> +	drm_client_dev_resume(&xe->drm);
>   
>   	intel_power_domains_enable(display);
>   }
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index bdd845e383ef..3556928d3938 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -70,13 +70,8 @@ struct drm_client_funcs {
>   	 * Called when suspending the device.
>   	 *
>   	 * This callback is optional.
> -	 *
> -	 * FIXME: Some callers hold the console lock when invoking this
> -	 *        function. This interferes with fbdev emulation, which
> -	 *        also tries to acquire the lock. Push the console lock
> -	 *        into the callback and remove 'holds_console_lock'.
>   	 */
> -	int (*suspend)(struct drm_client_dev *client, bool holds_console_lock);
> +	int (*suspend)(struct drm_client_dev *client);
>   
>   	/**
>   	 * @resume:
> @@ -84,13 +79,8 @@ struct drm_client_funcs {
>   	 * Called when resuming the device from suspend.
>   	 *
>   	 * This callback is optional.
> -	 *
> -	 * FIXME: Some callers hold the console lock when invoking this
> -	 *        function. This interferes with fbdev emulation, which
> -	 *        also tries to acquire the lock. Push the console lock
> -	 *        into the callback and remove 'holds_console_lock'.
>   	 */
> -	int (*resume)(struct drm_client_dev *client, bool holds_console_lock);
> +	int (*resume)(struct drm_client_dev *client);
>   };
>   
>   /**
> diff --git a/include/drm/drm_client_event.h b/include/drm/drm_client_event.h
> index 1d544d3a3228..985d6f02a4c4 100644
> --- a/include/drm/drm_client_event.h
> +++ b/include/drm/drm_client_event.h
> @@ -11,8 +11,8 @@ struct drm_device;
>   void drm_client_dev_unregister(struct drm_device *dev);
>   void drm_client_dev_hotplug(struct drm_device *dev);
>   void drm_client_dev_restore(struct drm_device *dev);
> -void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock);
> -void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock);
> +void drm_client_dev_suspend(struct drm_device *dev);
> +void drm_client_dev_resume(struct drm_device *dev);
>   #else
>   static inline void drm_client_dev_unregister(struct drm_device *dev)
>   { }
> @@ -20,9 +20,9 @@ static inline void drm_client_dev_hotplug(struct drm_device *dev)
>   { }
>   static inline void drm_client_dev_restore(struct drm_device *dev)
>   { }
> -static inline void drm_client_dev_suspend(struct drm_device *dev, bool holds_console_lock)
> +static inline void drm_client_dev_suspend(struct drm_device *dev)
>   { }
> -static inline void drm_client_dev_resume(struct drm_device *dev, bool holds_console_lock)
> +static inline void drm_client_dev_resume(struct drm_device *dev)
>   { }
>   #endif
>   

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v2] drm/client: Remove holds_console_lock parameter from suspend/resume

[Jocelyn Falempe]

On 01/10/2025 16:37, Thomas Zimmermann wrote:
> No caller of the client resume/suspend helpers holds the console
> lock. The last such cases were removed from radeon in the patch
> series at [1]. Now remove the related parameter and the TODO items.
> 
> v2:
> - update placeholders for CONFIG_DRM_CLIENT=n
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/series/151624/ # [1]

For drm_log:
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>