From: Leo Li <[email protected]> [Why]
APU DCN generations since DCN3.5 have the capability to power down almost all of the DCN hw block during idle periods. This is referred to as IPS -- idle power states. In combination with a panel remote-buffer feature (like PSR or Panel Replay), IPS can save additional power. Once DCN is in an IPS, no register access can occur. This includes control registers for vblank interrupts; IPS must first be exited. Transitioning in or out of IPS requires synchronization with the rest of DC, as it powers up or down DCN, and may communicate with other MCUs on the SOC to do so. This is done via the dc_lock mutex. While calling enable_vblank, the DRM vblank core holds spinlocks that prevent blocking operations. Yet acquiring the dc_lock mutex is blocking. Thus, IPS can not be exited piror to programming vblank interrupt registers from within enable_vblank. At least not in a race-free way. Prior to this change, amdgpu_dm was exiting IPS(*) without holding the dc_lock, opening the door for races: https://gitlab.freedesktop.org/drm/amd/-/issues/5233 (*) From touching the interrupt registers. All register reads today have an implicit IPS exit, see dm_read_reg_func() To solve this, the prepare_vblank_enable callback can be implemented to exit IPS, as it is called from process context. [How] Implement the prepare_vblank_enable callback for amdgpu_dm. In it, the dc_lock mutex is acquired, and IPS is exited. v2: Add missing semicolon, add docstring for prepare_vbl_disallow_idle Signed-off-by: Leo Li <[email protected]> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 9 +++++ .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 4 +++ .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 36 +++++++++++++++++-- 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 0346052f2e574..842a93e2d6ce0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9682,6 +9682,7 @@ static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state, * We also need vupdate irq for the actual core vblank handling * at end of vblank. */ + WARN_ON(drm_crtc_vblank_prepare(new_state->base.crtc) != 0); WARN_ON(amdgpu_dm_crtc_set_vupdate_irq(new_state->base.crtc, true) != 0); WARN_ON(drm_crtc_vblank_get(new_state->base.crtc) != 0); drm_dbg_driver(new_state->base.crtc->dev, "%s: crtc=%u VRR off->on: Get vblank ref\n", @@ -10108,6 +10109,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, */ if (acrtc_attach->base.state->event && acrtc_state->active_planes > 0) { + drm_crtc_vblank_prepare(pcrtc); drm_crtc_vblank_get(pcrtc); spin_lock_irqsave(&pcrtc->dev->event_lock, flags); @@ -10124,6 +10126,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, &acrtc_state->stream->vrr_infopacket; } } else if (cursor_update && acrtc_state->active_planes > 0) { + drm_crtc_vblank_prepare(pcrtc); spin_lock_irqsave(&pcrtc->dev->event_lock, flags); if (acrtc_attach->base.state->event) { drm_crtc_vblank_get(pcrtc); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 7065b20aa2e6b..a99612fb3467a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -587,6 +587,15 @@ struct amdgpu_display_manager { */ uint32_t active_vblank_irq_count; + /** + * @prepare_vbl_disallow_idle: + * + * Set to true when idle has been disallowed. Set to false when vblank + * interrupts have been enabled. i.e. idle re-allow on vblank disable is + * blocked if this is true. + */ + bool prepare_vbl_disallow_idle; + #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) /** * @secure_display_ctx: diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index e20aa74380665..7839b56859391 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -656,6 +656,10 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) */ enabled = amdgpu_dm_is_valid_crc_source(cur_crc_src); if (!enabled && enable) { + ret = drm_crtc_vblank_prepare(crtc); + if (ret) + goto cleanup; + ret = drm_crtc_vblank_get(crtc); if (ret) goto cleanup; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c index 38f9ea313dcbb..dd693419111db 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c @@ -258,8 +258,8 @@ static void amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work) else if (dm->active_vblank_irq_count) dm->active_vblank_irq_count--; - if (dm->active_vblank_irq_count > 0) - dc_allow_idle_optimizations(dm->dc, false); + /* prepare_vblank_enable must disallow idle first */ + ASSERT(dm->dc->idle_optimizations_allowed == false); /* * Control PSR based on vblank requirements from OS @@ -277,7 +277,13 @@ static void amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work) vblank_work->acrtc->dm_irq_params.allow_sr_entry); } - if (dm->active_vblank_irq_count == 0) { + /* + * If this worker runs disable between prepare_vblank and enable_vblank, + * we need to block idle re-allow. Leave it to the next vblank disable + * to re-allow idle. + */ + if (dm->active_vblank_irq_count == 0 && + !READ_ONCE(dm->prepare_vbl_disallow_idle)) { dc_post_update_surfaces_to_stream(dm->dc); r = amdgpu_dpm_pause_power_profile(adev, true); @@ -308,6 +314,8 @@ static inline int amdgpu_dm_crtc_set_vblank(struct drm_crtc *crtc, bool enable) int irq_type; int rc = 0; + ASSERT(dm->dc->idle_optimizations_allowed == false); + if (enable && !acrtc->base.enabled) { drm_dbg_vbl(crtc->dev, "Reject vblank enable on unconfigured CRTC %d (enabled=%d)\n", @@ -399,6 +407,9 @@ static inline int amdgpu_dm_crtc_set_vblank(struct drm_crtc *crtc, bool enable) } #endif + /* Ensure compiler emits the write before worker is queued */ + WRITE_ONCE(dm->prepare_vbl_disallow_idle, false); + if (amdgpu_in_reset(adev)) return 0; @@ -423,6 +434,24 @@ static inline int amdgpu_dm_crtc_set_vblank(struct drm_crtc *crtc, bool enable) return 0; } +static int amdgpu_prepare_enable_vblank(struct drm_crtc *crtc) +{ + struct amdgpu_device *adev = drm_to_adev(crtc->dev); + struct amdgpu_display_manager *dm = &adev->dm; + + guard(mutex)(&adev->dm.dc_lock); + + if (dm->dc->idle_optimizations_allowed) { + /* Prevent the disable worker from re-allowing idle until + * interrupts are enabled. Ensure compiler emits the write + * before disallowing idle. */ + WRITE_ONCE(dm->prepare_vbl_disallow_idle, true); + dc_allow_idle_optimizations(dm->dc, false); + } + + return 0; +} + int amdgpu_dm_crtc_enable_vblank(struct drm_crtc *crtc) { return amdgpu_dm_crtc_set_vblank(crtc, true); @@ -590,6 +619,7 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .verify_crc_source = amdgpu_dm_crtc_verify_crc_source, .get_crc_sources = amdgpu_dm_crtc_get_crc_sources, .get_vblank_counter = amdgpu_get_vblank_counter_kms, + .prepare_enable_vblank = amdgpu_prepare_enable_vblank, .enable_vblank = amdgpu_dm_crtc_enable_vblank, .disable_vblank = amdgpu_dm_crtc_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, -- 2.51.0
