Skip to content

Commit aa6713f

Browse files
leo-sunli1alexdeucher
authored andcommitted
drm/amd/display: Do not wait for PSR disable on vbl enable
[Why] Outside of a modeset/link configuration change, we should not have to wait for the panel to exit PSR. Depending on the panel and it's state, it may take multiple frames for it to exit PSR. Therefore, waiting in all scenarios may cause perceived stuttering, especially in combination with faster vblank shutdown. [How] PSR1 disable is hooked up to the vblank enable event, and vice versa. In case of vblank enable, do not wait for panel to exit PSR, but still wait in all other cases. We also avoid a call to unnecessarily change power_opts on disable - this ends up sending another command to dmcub fw. When testing against IGT, some crc tests like kms_plane_alpha_blend and amd_hotplug were failing due to CRC timeouts. This was found to be caused by the early return before HW has fully exited PSR1. Fix this by first making sure we grab a vblank reference, then waiting for panel to exit PSR1, before programming hw for CRC generation. Fixes: 58a261b ("drm/amd/display: use a more lax vblank enable policy for older ASICs") Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3743 Reviewed-by: Tom Chung <[email protected]> Signed-off-by: Leo Li <[email protected]> Signed-off-by: Tom Chung <[email protected]> Tested-by: Daniel Wheeler <[email protected]> Signed-off-by: Alex Deucher <[email protected]>
1 parent f5860c8 commit aa6713f

File tree

6 files changed

+54
-17
lines changed

6 files changed

+54
-17
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9180,7 +9180,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
91809180
acrtc_state->stream->link->psr_settings.psr_dirty_rects_change_timestamp_ns =
91819181
timestamp_ns;
91829182
if (acrtc_state->stream->link->psr_settings.psr_allow_active)
9183-
amdgpu_dm_psr_disable(acrtc_state->stream);
9183+
amdgpu_dm_psr_disable(acrtc_state->stream, true);
91849184
mutex_unlock(&dm->dc_lock);
91859185
}
91869186
}
@@ -9350,7 +9350,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
93509350
if (acrtc_state->stream->link->replay_settings.replay_allow_active)
93519351
amdgpu_dm_replay_disable(acrtc_state->stream);
93529352
if (acrtc_state->stream->link->psr_settings.psr_allow_active)
9353-
amdgpu_dm_psr_disable(acrtc_state->stream);
9353+
amdgpu_dm_psr_disable(acrtc_state->stream, true);
93549354
}
93559355
mutex_unlock(&dm->dc_lock);
93569356

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "amdgpu_dm.h"
3131
#include "dc.h"
3232
#include "amdgpu_securedisplay.h"
33+
#include "amdgpu_dm_psr.h"
3334

3435
static const char *const pipe_crc_sources[] = {
3536
"none",
@@ -507,6 +508,10 @@ int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc,
507508

508509
mutex_lock(&adev->dm.dc_lock);
509510

511+
/* For PSR1, check that the panel has exited PSR */
512+
if (stream_state->link->psr_settings.psr_version < DC_PSR_VERSION_SU_1)
513+
amdgpu_dm_psr_wait_disable(stream_state);
514+
510515
/* Enable or disable CRTC CRC generation */
511516
if (dm_is_crc_source_crtc(source) || source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE) {
512517
if (!dc_stream_configure_crc(stream_state->ctx->dc,
@@ -644,6 +649,17 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
644649

645650
}
646651

652+
/*
653+
* Reading the CRC requires the vblank interrupt handler to be
654+
* enabled. Keep a reference until CRC capture stops.
655+
*/
656+
enabled = amdgpu_dm_is_valid_crc_source(cur_crc_src);
657+
if (!enabled && enable) {
658+
ret = drm_crtc_vblank_get(crtc);
659+
if (ret)
660+
goto cleanup;
661+
}
662+
647663
#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
648664
/* Reset secure_display when we change crc source from debugfs */
649665
amdgpu_dm_set_crc_window_default(crtc, crtc_state->stream);
@@ -654,16 +670,7 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
654670
goto cleanup;
655671
}
656672

657-
/*
658-
* Reading the CRC requires the vblank interrupt handler to be
659-
* enabled. Keep a reference until CRC capture stops.
660-
*/
661-
enabled = amdgpu_dm_is_valid_crc_source(cur_crc_src);
662673
if (!enabled && enable) {
663-
ret = drm_crtc_vblank_get(crtc);
664-
if (ret)
665-
goto cleanup;
666-
667674
if (dm_is_crc_source_dprx(source)) {
668675
if (drm_dp_start_crc(aux, crtc)) {
669676
DRM_DEBUG_DRIVER("dp start crc failed\n");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ static void amdgpu_dm_crtc_set_panel_sr_feature(
142142
amdgpu_dm_replay_enable(vblank_work->stream, true);
143143
} else if (vblank_enabled) {
144144
if (link->psr_settings.psr_version < DC_PSR_VERSION_SU_1 && is_sr_active)
145-
amdgpu_dm_psr_disable(vblank_work->stream);
145+
amdgpu_dm_psr_disable(vblank_work->stream, false);
146146
} else if (link->psr_settings.psr_feature_enabled &&
147147
allow_sr_entry && !is_sr_active && !is_crc_window_active) {
148148

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3693,7 +3693,7 @@ static int crc_win_update_set(void *data, u64 val)
36933693
/* PSR may write to OTG CRC window control register,
36943694
* so close it before starting secure_display.
36953695
*/
3696-
amdgpu_dm_psr_disable(acrtc->dm_irq_params.stream);
3696+
amdgpu_dm_psr_disable(acrtc->dm_irq_params.stream, true);
36973697

36983698
spin_lock_irq(&adev_to_drm(adev)->event_lock);
36993699

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,13 @@ void amdgpu_dm_psr_enable(struct dc_stream_state *stream)
201201
*
202202
* Return: true if success
203203
*/
204-
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream)
204+
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream, bool wait)
205205
{
206-
unsigned int power_opt = 0;
207206
bool psr_enable = false;
208207

209208
DRM_DEBUG_DRIVER("Disabling psr...\n");
210209

211-
return dc_link_set_psr_allow_active(stream->link, &psr_enable, true, false, &power_opt);
210+
return dc_link_set_psr_allow_active(stream->link, &psr_enable, wait, false, NULL);
212211
}
213212

214213
/*
@@ -251,3 +250,33 @@ bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm)
251250

252251
return allow_active;
253252
}
253+
254+
/**
255+
* amdgpu_dm_psr_wait_disable() - Wait for eDP panel to exit PSR
256+
* @stream: stream state attached to the eDP link
257+
*
258+
* Waits for a max of 500ms for the eDP panel to exit PSR.
259+
*
260+
* Return: true if panel exited PSR, false otherwise.
261+
*/
262+
bool amdgpu_dm_psr_wait_disable(struct dc_stream_state *stream)
263+
{
264+
enum dc_psr_state psr_state = PSR_STATE0;
265+
struct dc_link *link = stream->link;
266+
int retry_count;
267+
268+
if (link == NULL)
269+
return false;
270+
271+
for (retry_count = 0; retry_count <= 1000; retry_count++) {
272+
dc_link_get_psr_state(link, &psr_state);
273+
if (psr_state == PSR_STATE0)
274+
break;
275+
udelay(500);
276+
}
277+
278+
if (retry_count == 1000)
279+
return false;
280+
281+
return true;
282+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@
3434
void amdgpu_dm_set_psr_caps(struct dc_link *link);
3535
void amdgpu_dm_psr_enable(struct dc_stream_state *stream);
3636
bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream);
37-
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream);
37+
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream, bool wait);
3838
bool amdgpu_dm_psr_disable_all(struct amdgpu_display_manager *dm);
3939
bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm);
40+
bool amdgpu_dm_psr_wait_disable(struct dc_stream_state *stream);
4041

4142
#endif /* AMDGPU_DM_AMDGPU_DM_PSR_H_ */

0 commit comments

Comments
 (0)