Skip to content

Commit

Permalink
drm/amd/display: Do not wait for PSR disable on vbl enable
Browse files Browse the repository at this point in the history
[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 <chiahsuan.chung@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
  • Loading branch information
Leo Li authored and Alex Deucher committed Jan 10, 2025
1 parent f5860c8 commit aa6713f
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 17 deletions.
4 changes: 2 additions & 2 deletions drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
Original file line number Diff line number Diff line change
Expand Up @@ -9180,7 +9180,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
acrtc_state->stream->link->psr_settings.psr_dirty_rects_change_timestamp_ns =
timestamp_ns;
if (acrtc_state->stream->link->psr_settings.psr_allow_active)
amdgpu_dm_psr_disable(acrtc_state->stream);
amdgpu_dm_psr_disable(acrtc_state->stream, true);
mutex_unlock(&dm->dc_lock);
}
}
Expand Down Expand Up @@ -9350,7 +9350,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
if (acrtc_state->stream->link->replay_settings.replay_allow_active)
amdgpu_dm_replay_disable(acrtc_state->stream);
if (acrtc_state->stream->link->psr_settings.psr_allow_active)
amdgpu_dm_psr_disable(acrtc_state->stream);
amdgpu_dm_psr_disable(acrtc_state->stream, true);
}
mutex_unlock(&dm->dc_lock);

Expand Down
25 changes: 16 additions & 9 deletions drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "amdgpu_dm.h"
#include "dc.h"
#include "amdgpu_securedisplay.h"
#include "amdgpu_dm_psr.h"

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

mutex_lock(&adev->dm.dc_lock);

/* For PSR1, check that the panel has exited PSR */
if (stream_state->link->psr_settings.psr_version < DC_PSR_VERSION_SU_1)
amdgpu_dm_psr_wait_disable(stream_state);

/* Enable or disable CRTC CRC generation */
if (dm_is_crc_source_crtc(source) || source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE) {
if (!dc_stream_configure_crc(stream_state->ctx->dc,
Expand Down Expand Up @@ -644,6 +649,17 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)

}

/*
* Reading the CRC requires the vblank interrupt handler to be
* enabled. Keep a reference until CRC capture stops.
*/
enabled = amdgpu_dm_is_valid_crc_source(cur_crc_src);
if (!enabled && enable) {
ret = drm_crtc_vblank_get(crtc);
if (ret)
goto cleanup;
}

#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
/* Reset secure_display when we change crc source from debugfs */
amdgpu_dm_set_crc_window_default(crtc, crtc_state->stream);
Expand All @@ -654,16 +670,7 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
goto cleanup;
}

/*
* Reading the CRC requires the vblank interrupt handler to be
* enabled. Keep a reference until CRC capture stops.
*/
enabled = amdgpu_dm_is_valid_crc_source(cur_crc_src);
if (!enabled && enable) {
ret = drm_crtc_vblank_get(crtc);
if (ret)
goto cleanup;

if (dm_is_crc_source_dprx(source)) {
if (drm_dp_start_crc(aux, crtc)) {
DRM_DEBUG_DRIVER("dp start crc failed\n");
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ static void amdgpu_dm_crtc_set_panel_sr_feature(
amdgpu_dm_replay_enable(vblank_work->stream, true);
} else if (vblank_enabled) {
if (link->psr_settings.psr_version < DC_PSR_VERSION_SU_1 && is_sr_active)
amdgpu_dm_psr_disable(vblank_work->stream);
amdgpu_dm_psr_disable(vblank_work->stream, false);
} else if (link->psr_settings.psr_feature_enabled &&
allow_sr_entry && !is_sr_active && !is_crc_window_active) {

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -3693,7 +3693,7 @@ static int crc_win_update_set(void *data, u64 val)
/* PSR may write to OTG CRC window control register,
* so close it before starting secure_display.
*/
amdgpu_dm_psr_disable(acrtc->dm_irq_params.stream);
amdgpu_dm_psr_disable(acrtc->dm_irq_params.stream, true);

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

Expand Down
35 changes: 32 additions & 3 deletions drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,13 @@ void amdgpu_dm_psr_enable(struct dc_stream_state *stream)
*
* Return: true if success
*/
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream)
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream, bool wait)
{
unsigned int power_opt = 0;
bool psr_enable = false;

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

return dc_link_set_psr_allow_active(stream->link, &psr_enable, true, false, &power_opt);
return dc_link_set_psr_allow_active(stream->link, &psr_enable, wait, false, NULL);
}

/*
Expand Down Expand Up @@ -251,3 +250,33 @@ bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm)

return allow_active;
}

/**
* amdgpu_dm_psr_wait_disable() - Wait for eDP panel to exit PSR
* @stream: stream state attached to the eDP link
*
* Waits for a max of 500ms for the eDP panel to exit PSR.
*
* Return: true if panel exited PSR, false otherwise.
*/
bool amdgpu_dm_psr_wait_disable(struct dc_stream_state *stream)
{
enum dc_psr_state psr_state = PSR_STATE0;
struct dc_link *link = stream->link;
int retry_count;

if (link == NULL)
return false;

for (retry_count = 0; retry_count <= 1000; retry_count++) {
dc_link_get_psr_state(link, &psr_state);
if (psr_state == PSR_STATE0)
break;
udelay(500);
}

if (retry_count == 1000)
return false;

return true;
}
3 changes: 2 additions & 1 deletion drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
void amdgpu_dm_set_psr_caps(struct dc_link *link);
void amdgpu_dm_psr_enable(struct dc_stream_state *stream);
bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream);
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream);
bool amdgpu_dm_psr_disable(struct dc_stream_state *stream, bool wait);
bool amdgpu_dm_psr_disable_all(struct amdgpu_display_manager *dm);
bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm);
bool amdgpu_dm_psr_wait_disable(struct dc_stream_state *stream);

#endif /* AMDGPU_DM_AMDGPU_DM_PSR_H_ */

0 comments on commit aa6713f

Please sign in to comment.