From: Thomas Zimmermann tzimmermann@suse.de
[ Upstream commit 5dd0b96118e09a3725e3f83543e133b1fd02c18c ]
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.q... Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Alex Deucher alexander.deucher@amd.com (cherry picked from commit 612ec7c69d04cb58beb1332c2806da9f2f47a3ae) Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Based on my analysis of the commit and the code changes, here is my assessment:
**Answer: YES**
This commit should be backported to stable kernel trees. Here's my extensive explanation:
## 1. **The commit fixes a real circular locking dependency bug**
The commit message clearly states this fixes a "circular dependency with the client-list mutex" that was "Reported when combining radeon with another DRM driver." This is a real bug that can lead to deadlocks in production systems.
## 2. **The fix is minimal and focused**
The code changes are very small and targeted: - In `radeon_suspend_kms()`: Changed from calling `drm_client_dev_suspend(dev, true)` with console_lock held to `drm_client_dev_suspend(dev, false)` without holding the lock - In `radeon_resume_kms()`: Removed the console_lock/unlock calls around `drm_client_dev_resume()`
The second parameter to `drm_client_dev_suspend()` indicates whether the caller holds the console lock. By changing it from `true` to `false`, radeon now follows the same pattern as other DRM drivers.
## 3. **This aligns radeon with other DRM drivers**
The commit message explicitly states: "This is what all other DRM drivers so." Looking at the similar commits provided: - Similar Commit #1 shows radeon being updated to use the new client suspend/resume infrastructure - Similar Commit #4 shows amdgpu (the successor to radeon) already using `drm_client_dev_suspend(dev, false)` without holding the console lock
## 4. **The bug has real-world impact**
The issue was reported by a real user (Jeff Johnson) and occurs "when combining radeon with another DRM driver." Multi-GPU systems are increasingly common, especially with integrated + discrete GPU configurations.
## 5. **Low regression risk**
The change removes lock acquisition rather than adding it, which generally has lower risk. The fbdev DRM client will acquire the console lock when needed (as mentioned in the commit message), so the necessary locking still occurs - just at the correct layer to avoid the circular dependency.
## 6. **Similar to historical fixes**
Looking at Similar Commit #5 (amdgpu console lock restructuring), we can see there's a history of console lock ordering issues in AMD GPU drivers that have been fixed in the past. This suggests these types of fixes are considered important enough for the maintainers.
## 7. **The fix is already cherry-picked**
The commit message shows "(cherry picked from commit 612ec7c69d04cb58beb1332c2806da9f2f47a3ae)" indicating this was already selected for backporting by maintainers.
The only potential concern would be ensuring that the DRM client infrastructure this depends on is present in the stable kernels being targeted, but given that this is fixing a bug in existing code paths, the infrastructure must already be there.
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 bbd39348a7aba..6f50cfdfe5a2e 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; }