Re: [PATCH v2] drm/amd/display: set allow_freesync before validating a new stream

[Alex Hung <alex.hung@amd.com>]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 10/26/26 01:33, Mikhail Gavrilov wrote:
> After resume from S3, DC enables SubVP on a single FreeSync display at
> a high refresh rate, although dcn32_allow_subvp_high_refresh_rate() is
> meant to keep SubVP off that configuration ("For single display cases,
> freesync must be disabled").  On an RX 7900 XTX driving a FreeSync TV
> at 3840x2160@120, amdgpu_dm_capabilities reports sub-viewport disabled
> after boot and enabled after "rtcwake -m mem", and the memory clock
> starts switching.  It stays that way until a later commit re-validates
> the state; a CRTC gamma change (toggling Night Light) turns SubVP off
> again.
> 
> The policy reads dc_stream_state::allow_freesync while the new state
> is validated, but amdgpu_dm sets that field only in
> amdgpu_dm_update_freesync_state_on_stream(), on the first flip after
> the commit.  A stream created by a modeset is therefore validated as
> if FreeSync were off.  On resume, dm_destroy_cached_state() drops the
> streams and forces a modeset, so the resumed state is validated
> without FreeSync.
> 
> Set allow_freesync in amdgpu_dm_get_freesync_config_for_crtc(), which
> atomic check calls for every active CRTC and which already writes
> ignore_msa_timing_param to the stream.  It uses vrr_supported, the
> condition the flip-time value is derived from.  With this change the
> same TV keeps SubVP disabled across "rtcwake -m mem".
> 
> On this machine SubVP is also what exposes the single-frame scanout
> corruption reported in the thread linked below, so the FreeSync
> setting stopped working as a workaround after every suspend.
> 
> Fixes: 87f0c16e0eeb ("drm/amd/display: Enable SubVP for high refresh rate displays")
> Link: https://lore.kernel.org/all/20260829010525.29935-1-mikhail.v.gavrilov@gmail.com/
> Link: https://gitlab.freedesktop.org/drm/amd/-/work_items/5714#note_3736349
> Tested-by: Josh Kasten <jkasten@gmail.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
> v2:
>   - Rebased onto amd-staging-drm-next, where vrr_supported also depends
>     on vrr_timing_valid, so v1 no longer applied there.  No functional
>     change.
>   - Added Tested-by from Josh Kasten, who ran v2 on next-20261023 (the
>     file is the same there as in amd-staging-drm-next) and saw SubVP
>     stay disabled across rtcwake with FreeSync enabled.
>   
> v1: https://lore.kernel.org/all/20261018221726.27259-1-mikhail.v.gavrilov@gmail.com/
> 
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_freesync.c    | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_freesync.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_freesync.c
> index 386ba88c57c6..936a899aef00 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_freesync.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_freesync.c
> @@ -170,6 +170,14 @@ void amdgpu_dm_get_freesync_config_for_crtc(
>   					vrefresh <= aconnector->max_vfreq &&
>   					vrr_timing_valid;
>   
> +	/*
> +	 * DC reads allow_freesync while validating the new state, for example
> +	 * to keep SubVP off a single FreeSync display, but it is otherwise set
> +	 * only on the first flip after the commit.  Set it here, so that a
> +	 * stream created by this commit is not validated without it.
> +	 */
> +	new_crtc_state->stream->allow_freesync = new_crtc_state->vrr_supported;
> +
>   	if (new_crtc_state->vrr_supported) {
>   		new_crtc_state->stream->ignore_msa_timing_param = true;
>   		fs_vid_mode = new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED;