All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume
@ 2025-07-15  9:50 Thomas Zimmermann
  2025-07-15  9:50 ` [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients Thomas Zimmermann
  2025-07-15  9:50 ` [PATCH 2/2] drm/radeon: Do not hold console lock during resume Thomas Zimmermann
  0 siblings, 2 replies; 4+ messages in thread
From: Thomas Zimmermann @ 2025-07-15  9:50 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona
  Cc: amd-gfx, dri-devel, 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients
  2025-07-15  9:50 [PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume Thomas Zimmermann
@ 2025-07-15  9:50 ` Thomas Zimmermann
  2025-07-15 13:53   ` Alex Deucher
  2025-07-15  9:50 ` [PATCH 2/2] drm/radeon: Do not hold console lock during resume Thomas Zimmermann
  1 sibling, 1 reply; 4+ messages in thread
From: Thomas Zimmermann @ 2025-07-15  9:50 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona
  Cc: amd-gfx, dri-devel, Thomas Zimmermann, Jeff Johnson,
	Ville Syrjälä

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/radeon: Do not hold console lock during resume
  2025-07-15  9:50 [PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume Thomas Zimmermann
  2025-07-15  9:50 ` [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients Thomas Zimmermann
@ 2025-07-15  9:50 ` Thomas Zimmermann
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Zimmermann @ 2025-07-15  9:50 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona
  Cc: amd-gfx, dri-devel, 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients
  2025-07-15  9:50 ` [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients Thomas Zimmermann
@ 2025-07-15 13:53   ` Alex Deucher
  0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2025-07-15 13:53 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: alexander.deucher, christian.koenig, airlied, simona, amd-gfx,
	dri-devel, Jeff Johnson, Ville Syrjälä

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-15 13:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15  9:50 [PATCH 0/2] drm/radeon: Do not acquire console lock during suspend/resume Thomas Zimmermann
2025-07-15  9:50 ` [PATCH 1/2] drm/radeon: Do not hold console lock while suspending clients Thomas Zimmermann
2025-07-15 13:53   ` Alex Deucher
2025-07-15  9:50 ` [PATCH 2/2] drm/radeon: Do not hold console lock during resume Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.