Skip to content

Commit 145b25d

Browse files
Ray6161heftig
authored andcommitted
drm/amd/display: Fix ISM dc_lock deadlock during suspend
[ Upstream commit 3714fe2 ] CachyOS users reported a regression in shutdown/reboot behavior on 7.1 kernels: the display turns off, but the machine does not power down. Reverting ISM fixes the regression, and this upstream fix addresses the same ISM dc_lock/workqueue deadlock in the suspend/shutdown paths. [Why] System hang observed during suspend/resume while video is playing. amdgpu_dm_ism_disable() is called under dc_lock and waits for ISM delayed work via disable_delayed_work_sync(). The work handlers themselves take dc_lock, producing an ABBA deadlock when a worker is in flight at suspend time. [How] Split the disable path into two phases with opposite locking contracts: 1. amdgpu_dm_ism_disable() -- quiesces workers, must NOT hold dc_lock. 2. amdgpu_dm_ism_force_full_power() (new) -- drives the ISM FSM back to FULL_POWER_RUNNING, must hold dc_lock. Fixes: 7540034 ("drm/amd/display: Add Idle state manager(ISM)") Link: CachyOS/linux-cachyos#900 Cc: stable@vger.kernel.org # 7.1.y Reviewed-by: Sun peng (Leo) Li <sunpeng.li@amd.com> Signed-off-by: Ray Wu <ray.wu@amd.com> Signed-off-by: Ivan Lipski <ivan.lipski@amd.com> Tested-by: Dan Wheeler <daniel.wheeler@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Signed-off-by: Peter Jung <admin@ptr1337.dev>
1 parent d878a80 commit 145b25d

3 files changed

Lines changed: 70 additions & 12 deletions

File tree

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2260,9 +2260,16 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
22602260
adev->dm.idle_workqueue = NULL;
22612261
}
22622262

2263-
/* Disable ISM before dc_destroy() invalidates dm->dc */
2263+
/*
2264+
* Disable ISM before dc_destroy() invalidates dm->dc.
2265+
*
2266+
* Quiesce workers first without dc_lock (they take dc_lock
2267+
* themselves, so syncing under it would deadlock), then drive the
2268+
* FSM back to FULL_POWER_RUNNING under dc_lock.
2269+
*/
2270+
amdgpu_dm_ism_disable(&adev->dm);
22642271
scoped_guard(mutex, &adev->dm.dc_lock)
2265-
amdgpu_dm_ism_disable(&adev->dm);
2272+
amdgpu_dm_ism_force_full_power(&adev->dm);
22662273

22672274
amdgpu_dm_destroy_drm_device(&adev->dm);
22682275

@@ -3290,9 +3297,14 @@ static int dm_suspend(struct amdgpu_ip_block *ip_block)
32903297
if (amdgpu_in_reset(adev)) {
32913298
enum dc_status res;
32923299

3300+
/* Quiesce ISM workers before taking dc_lock (workers take
3301+
* dc_lock themselves; syncing under it would deadlock).
3302+
*/
3303+
amdgpu_dm_ism_disable(dm);
3304+
32933305
mutex_lock(&dm->dc_lock);
32943306

3295-
amdgpu_dm_ism_disable(dm);
3307+
amdgpu_dm_ism_force_full_power(dm);
32963308
dc_allow_idle_optimizations(adev->dm.dc, false);
32973309

32983310
dm->cached_dc_state = dc_state_create_copy(dm->dc->current_state);
@@ -3326,8 +3338,13 @@ static int dm_suspend(struct amdgpu_ip_block *ip_block)
33263338

33273339
amdgpu_dm_irq_suspend(adev);
33283340

3341+
/*
3342+
* Quiesce ISM workers before taking dc_lock (workers take dc_lock
3343+
* themselves; syncing under it would deadlock).
3344+
*/
3345+
amdgpu_dm_ism_disable(dm);
33293346
scoped_guard(mutex, &dm->dc_lock)
3330-
amdgpu_dm_ism_disable(dm);
3347+
amdgpu_dm_ism_force_full_power(dm);
33313348

33323349
hpd_rx_irq_work_suspend(dm);
33333350

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.c

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -524,35 +524,75 @@ static void dm_ism_sso_delayed_work_func(struct work_struct *work)
524524
}
525525

526526
/**
527-
* amdgpu_dm_ism_disable - Disable the ISM
527+
* amdgpu_dm_ism_disable - Quiesce ISM workers
528528
*
529529
* @dm: The amdgpu display manager
530530
*
531-
* Disable the idle state manager by disabling any ISM work, canceling pending
532-
* work, and waiting for in-progress work to finish. After disabling, the system
533-
* is left in DM_ISM_STATE_FULL_POWER_RUNNING state.
531+
* Cancels and disables any pending or in-flight ISM delayed work and waits
532+
* for in-progress work to finish. After this returns, no ISM worker can run
533+
* and subsequent mod_delayed_work() calls become no-ops via
534+
* clear_pending_if_disabled().
535+
*
536+
* Must NOT be called with dc_lock held: the workers themselves take dc_lock,
537+
* so a synchronous wait under dc_lock would deadlock.
538+
*
539+
* The caller is responsible for driving the FSM back to FULL_POWER_RUNNING
540+
* (under dc_lock) by calling amdgpu_dm_ism_force_full_power().
534541
*/
535542
void amdgpu_dm_ism_disable(struct amdgpu_display_manager *dm)
536543
{
537544
struct drm_crtc *crtc;
538545
struct amdgpu_crtc *acrtc;
539546
struct amdgpu_dm_ism *ism;
540547

541-
ASSERT(mutex_is_locked(&dm->dc_lock));
548+
/*
549+
* Caller must NOT hold dc_lock: the ISM delayed work handlers
550+
* acquire dc_lock themselves, so waiting for them via
551+
* disable_delayed_work_sync() while holding dc_lock would
552+
* self-deadlock against an in-flight worker.
553+
*/
554+
lockdep_assert_not_held(&dm->dc_lock);
542555

543556
drm_for_each_crtc(crtc, dm->ddev) {
544557
acrtc = to_amdgpu_crtc(crtc);
545558
ism = &acrtc->ism;
546559

547-
/* Cancel and disable any pending work */
548560
disable_delayed_work_sync(&ism->delayed_work);
549561
disable_delayed_work_sync(&ism->sso_delayed_work);
562+
}
563+
}
564+
565+
/**
566+
* amdgpu_dm_ism_force_full_power - Force every CRTC's ISM FSM to FULL_POWER
567+
*
568+
* @dm: The amdgpu display manager
569+
*
570+
* Sends DM_ISM_EVENT_EXIT_IDLE_REQUESTED to every CRTC's ISM, leaving each
571+
* FSM in FULL_POWER_RUNNING. Intended to be paired with
572+
* amdgpu_dm_ism_disable(): callers should first quiesce workers (without
573+
* dc_lock), then take dc_lock and call this helper.
574+
*
575+
* Must be called with dc_lock held.
576+
*/
577+
void amdgpu_dm_ism_force_full_power(struct amdgpu_display_manager *dm)
578+
{
579+
struct drm_crtc *crtc;
580+
struct amdgpu_crtc *acrtc;
581+
582+
/*
583+
* Caller must hold dc_lock: commit_event() drives the FSM and
584+
* may touch dc state via dc_allow_idle_optimizations() etc.
585+
*/
586+
lockdep_assert_held(&dm->dc_lock);
587+
588+
drm_for_each_crtc(crtc, dm->ddev) {
589+
acrtc = to_amdgpu_crtc(crtc);
550590

551591
/*
552592
* When disabled, leave in FULL_POWER_RUNNING state.
553-
* EXIT_IDLE will not queue any work
593+
* EXIT_IDLE will not queue any work.
554594
*/
555-
amdgpu_dm_ism_commit_event(ism,
595+
amdgpu_dm_ism_commit_event(&acrtc->ism,
556596
DM_ISM_EVENT_EXIT_IDLE_REQUESTED);
557597
}
558598
}

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ void amdgpu_dm_ism_fini(struct amdgpu_dm_ism *ism);
146146
void amdgpu_dm_ism_commit_event(struct amdgpu_dm_ism *ism,
147147
enum amdgpu_dm_ism_event event);
148148
void amdgpu_dm_ism_disable(struct amdgpu_display_manager *dm);
149+
void amdgpu_dm_ism_force_full_power(struct amdgpu_display_manager *dm);
149150
void amdgpu_dm_ism_enable(struct amdgpu_display_manager *dm);
150151

151152
#endif

0 commit comments

Comments
 (0)