[PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume

[PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume

[Thomas Zimmermann]

Radeon is the only driver acquiring the console lock on suspend and
resume by itself. This creates cyclic locking dependencies with the
client-list mutex.

Drop the console locking from radeon and let fbdev client handle the
console locking as necessary. Adopts the behavior of all other DRM
drivers. Fixes the cyclic locking.

Tested by repeatedly suspeding/resuming on Radeon HD 6570.

Thomas Zimmermann (2):
  drm/radeon: Do not hold console lock while suspending clients
  drm/radeon: Do not hold console lock during resume

 drivers/gpu/drm/radeon/radeon_device.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

-- 
2.50.0

[PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients

[Thomas Zimmermann]

The radeon driver holds the console lock while suspending in-kernel
DRM clients. This creates a circular dependency with the client-list
mutex, which is supposed to be acquired first. Reported when combining
radeon with another DRM driver.

Therefore, do not take the console lock in radeon, but let the fbdev
DRM client acquire the lock when needed. This is what all other DRM
drivers so.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reported-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
Closes: https://lore.kernel.org/dri-devel/0a087cfd-bd4c-48f1-aa2f-4a3b12593935@oss.qualcomm.com/
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/radeon/radeon_device.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index bbd39348a7ab..6f50cfdfe5a2 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1635,11 +1635,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 		pci_set_power_state(pdev, PCI_D3hot);
 	}
 
-	if (notify_clients) {
-		console_lock();
-		drm_client_dev_suspend(dev, true);
-		console_unlock();
-	}
+	if (notify_clients)
+		drm_client_dev_suspend(dev, false);
+
 	return 0;
 }
 
-- 
2.50.0

Re: [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients

[Alex Deucher]

Applied the series.  Thanks!

Alex

On Tue, Jul 15, 2025 at 5:58 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The radeon driver holds the console lock while suspending in-kernel
> DRM clients. This creates a circular dependency with the client-list
> mutex, which is supposed to be acquired first. Reported when combining
> radeon with another DRM driver.
>
> Therefore, do not take the console lock in radeon, but let the fbdev
> DRM client acquire the lock when needed. This is what all other DRM
> drivers so.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reported-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
> Closes: https://lore.kernel.org/dri-devel/0a087cfd-bd4c-48f1-aa2f-4a3b12593935@oss.qualcomm.com/
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/radeon/radeon_device.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index bbd39348a7ab..6f50cfdfe5a2 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1635,11 +1635,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>                 pci_set_power_state(pdev, PCI_D3hot);
>         }
>
> -       if (notify_clients) {
> -               console_lock();
> -               drm_client_dev_suspend(dev, true);
> -               console_unlock();
> -       }
> +       if (notify_clients)
> +               drm_client_dev_suspend(dev, false);
> +
>         return 0;
>  }
>
> --
> 2.50.0
>

[PATCH 2/2] drm/radeon: Do not hold console lock during resume

[Thomas Zimmermann]

The function radeon_resume_kms() acquires the console lock. It is
inconsistent, as it depends on the notify_client argument. That
lock then covers a number of suspend operations that are unrelated
to the console.

Remove the calls to console_lock() and console_unlock() from the
radeon function. The console lock is only required by DRM's fbdev
emulation, which acquires it as necessary.

Also fixes a possible circular dependency between the console lock
and the client-list mutex, where the mutex is supposed to be taken
first.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_device.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 6f50cfdfe5a2..7a3e510327b7 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -26,7 +26,6 @@
  *          Jerome Glisse
  */
 
-#include <linux/console.h>
 #include <linux/efi.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
@@ -1659,17 +1658,11 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	if (notify_clients) {
-		console_lock();
-	}
 	if (resume) {
 		pci_set_power_state(pdev, PCI_D0);
 		pci_restore_state(pdev);
-		if (pci_enable_device(pdev)) {
-			if (notify_clients)
-				console_unlock();
+		if (pci_enable_device(pdev))
 			return -1;
-		}
 	}
 	/* resume AGP if in use */
 	radeon_agp_resume(rdev);
@@ -1745,10 +1738,8 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)
 	if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled)
 		radeon_pm_compute_clocks(rdev);
 
-	if (notify_clients) {
-		drm_client_dev_resume(dev, true);
-		console_unlock();
-	}
+	if (notify_clients)
+		drm_client_dev_resume(dev, false);
 
 	return 0;
 }
-- 
2.50.0