All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
@ 2026-09-09 19:59 Matthew Schwartz
  2026-09-21 19:41 ` George Zhang
  2026-09-21 19:42 ` George Zhang
  0 siblings, 2 replies; 6+ messages in thread
From: Matthew Schwartz @ 2026-09-09 19:59 UTC (permalink / raw)
  To: amd-gfx
  Cc: Matthew Schwartz, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, David Airlie, Simona Vetter,
	Austin Zheng, Jun Lei, Alan Liu, Aurabindo Pillai, Alvin Lee,
	dri-devel, linux-kernel

VRR-capable sinks set allow_freesync even with VRR_ENABLED=0, letting
FPO stretch frames during UCLK switches. On a Navi 33 driving 4K120
through a DP-HDMI PCON, a 40fps compositor limiter then runs at 37fps.
This surfaced after commit d49086491bcb ("drm/amd/display: Adjust
freesync pcon whitelist").

Allow frame stretching only for VRR_STATE_ACTIVE_VARIABLE. Store the
proposed permission in the private DC state so TEST_ONLY and failed
checks cannot change the shared stream. Apply changed permissions at
commit and force full DC validation, including native DP and eDP.

Fixes: 7eaef1163b82 ("drm/amd/display: set allow_freesync parameter in DM")
Signed-off-by: Matthew Schwartz <matthew.schwartz@linux.dev>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 26 ++++++++++++++++
 .../display/amdgpu_dm/amdgpu_dm_freesync.c    |  1 -
 drivers/gpu/drm/amd/display/dc/core/dc.c      |  4 +++
 .../gpu/drm/amd/display/dc/core/dc_state.c    | 31 +++++++++++++++++++
 drivers/gpu/drm/amd/display/dc/dc_state.h     |  6 ++++
 drivers/gpu/drm/amd/display/dc/dc_stream.h    |  3 ++
 .../drm/amd/display/dc/dml/dcn32/dcn32_fpu.c  |  9 ++++--
 .../dml21_wrapper/dml21_translation_helper.c  |  9 ++++--
 .../dc/resource/dcn30/dcn30_resource.c        |  2 +-
 .../resource/dcn32/dcn32_resource_helpers.c   | 10 ++++--
 10 files changed, 92 insertions(+), 9 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 f49cc198fb0d..09c088b878bb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3835,6 +3835,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
 	unsigned long flags;
 	u32 target_vblank, last_flip_vblank;
 	bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
+	bool allow_freesync = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
 	bool cursor_update = false;
 	bool pflip_present = false;
 	bool immediate_flip = false;
@@ -4146,6 +4147,9 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
 			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
 		}
 		mutex_lock(&dm->dc_lock);
+		if (acrtc_state->update_type == UPDATE_TYPE_FULL &&
+		    allow_freesync != acrtc_state->stream->allow_freesync)
+			bundle->stream_update.allow_freesync = &allow_freesync;
 		update_planes_and_stream_adapter(dm->dc,
 					 acrtc_state->update_type,
 					 planes_count,
@@ -5413,6 +5417,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
 	struct dc_stream_state *new_stream;
 	struct amdgpu_device *adev = dm->adev;
 	int ret = 0;
+	bool allow_freesync, old_allow_freesync;
 
 	/*
 	 * TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
@@ -5668,6 +5673,27 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
 	amdgpu_dm_get_freesync_config_for_crtc(dm_new_crtc_state,
 				     dm_new_conn_state);
 
+	allow_freesync = dm_new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
+	old_allow_freesync = dm_old_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
+
+	/* Keep the proposed FreeSync permission in the private DC state. */
+	if (dm_new_crtc_state->stream != dm_old_crtc_state->stream ||
+	    old_allow_freesync != allow_freesync) {
+		ret = dm_atomic_get_state(state, &dm_state);
+		if (ret)
+			goto fail;
+
+		if (!dc_state_set_stream_allow_freesync(dm_state->context,
+							dm_new_crtc_state->stream,
+							allow_freesync)) {
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		/* DML must re-evaluate FPO when the permission changes. */
+		*lock_and_validation_needed = true;
+	}
+
 	return ret;
 
 fail:
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 281c6cec9f4b..9f40064d13c5 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
@@ -253,7 +253,6 @@ void amdgpu_dm_update_freesync_state_on_stream(
 	new_crtc_state->vrr_infopacket = vrr_infopacket;
 
 	new_stream->vrr_infopacket = vrr_infopacket;
-	new_stream->allow_freesync = mod_freesync_get_freesync_enabled(&vrr_params);
 
 	/*
 	 * HDMI ALLM: when Gaming-VRR is active (VRR_EN=1) and the sink
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index ebbd81995c38..6064cc992d91 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3175,6 +3175,10 @@ static struct dc_update_descriptor check_update_surfaces_for_stream(
 		if (stream_update->mst_bw_update)
 			su_flags->bits.mst_bw = 1;
 
+		if (stream_update->allow_freesync &&
+		    *stream_update->allow_freesync != stream_update->stream->allow_freesync)
+			su_flags->bits.fams_changed = 1;
+
 		if (stream_update->stream->freesync_on_desktop &&
 			(stream_update->vrr_infopacket || stream_update->allow_freesync ||
 				stream_update->vrr_active_variable || stream_update->vrr_active_fixed))
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index 1e4fe2f941ca..382858b6c1da 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -425,6 +425,7 @@ enum dc_status dc_state_add_stream(
 	}
 
 	state->streams[state->stream_count] = stream;
+	state->stream_status[state->stream_count].allow_freesync_valid = false;
 	dc_stream_retain(stream);
 	state->stream_count++;
 
@@ -746,6 +747,36 @@ struct dc_stream_status *dc_state_get_stream_status(
 	return status.stream_count > 0 ? status.stream_status[0] : NULL;
 }
 
+bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
+					const struct dc_stream_state *stream)
+{
+	unsigned int i;
+
+	for (i = 0; i < state->stream_count; i++) {
+		if (state->streams[i] == stream &&
+		    state->stream_status[i].allow_freesync_valid)
+			return state->stream_status[i].allow_freesync;
+	}
+
+	return stream->allow_freesync;
+}
+
+bool dc_state_set_stream_allow_freesync(struct dc_state *state,
+					const struct dc_stream_state *stream, bool allow_freesync)
+{
+	unsigned int i;
+
+	for (i = 0; i < state->stream_count; i++) {
+		if (state->streams[i] == stream) {
+			state->stream_status[i].allow_freesync = allow_freesync;
+			state->stream_status[i].allow_freesync_valid = true;
+			return true;
+		}
+	}
+
+	return false;
+}
+
 enum mall_stream_type dc_state_get_pipe_subvp_type(const struct dc_state *state,
 		const struct pipe_ctx *pipe_ctx)
 {
diff --git a/drivers/gpu/drm/amd/display/dc/dc_state.h b/drivers/gpu/drm/amd/display/dc/dc_state.h
index acf461225e9d..40ad063cec09 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_state.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_state.h
@@ -81,4 +81,10 @@ struct dc_get_status_options;
 enum dc_status dc_state_get_status(struct dc_state_status *status,
 	const struct dc_get_status_options *options);
 
+bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
+					const struct dc_stream_state *stream);
+
+bool dc_state_set_stream_allow_freesync(struct dc_state *state,
+					const struct dc_stream_state *stream, bool allow_freesync);
+
 #endif /* _DC_STATE_H_ */
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 934ae381e730..14d8da9f15e5 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -62,6 +62,9 @@ struct dc_stream_status {
 	bool is_abm_supported;
 	struct mall_stream_config mall_stream_config;
 	bool fpo_in_use;
+	/* Proposed permission for validation, without modifying the stream. */
+	bool allow_freesync;
+	bool allow_freesync_valid;
 };
 
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
index b5bd1358fdfd..a816e47153ad 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
@@ -844,7 +844,9 @@ static bool subvp_drr_schedulable(struct dc *dc, struct dc_state *context)
 			continue;
 
 		if (dc_state_get_pipe_subvp_type(context, drr_pipe) == SUBVP_NONE && drr_pipe->stream->ignore_msa_timing_param &&
-				(drr_pipe->stream->allow_freesync || drr_pipe->stream->vrr_active_variable || drr_pipe->stream->vrr_active_fixed)) {
+				(dc_state_get_stream_allow_freesync(context, drr_pipe->stream) ||
+				 drr_pipe->stream->vrr_active_variable ||
+				 drr_pipe->stream->vrr_active_fixed)) {
 			drr_found = true;
 			break;
 		}
@@ -3444,7 +3446,10 @@ bool dcn32_allow_subvp_high_refresh_rate(struct dc *dc, struct dc_state *context
 				uint32_t height = subvp_high_refresh_list.res[i].height;
 
 				if (dcn32_check_native_scaling_for_res(pipe, width, height)) {
-					if ((context->stream_count == 1 && !pipe->stream->allow_freesync) || context->stream_count > 1) {
+					if ((context->stream_count == 1 &&
+					     !dc_state_get_stream_allow_freesync(context,
+										 pipe->stream)) ||
+					    context->stream_count > 1) {
 						allow = true;
 						break;
 					}
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
index 3c25de54d55b..031f90955b95 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
@@ -125,6 +125,7 @@ static unsigned int calc_vblank_nom_lines(const struct dc_stream_state *stream,
 }
 
 static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cfg *timing,
+		const struct dc_state *context,
 		struct dc_stream_state *stream,
 		struct pipe_ctx *otg_master_pipe,
 		struct dml2_context *dml_ctx)
@@ -166,7 +167,7 @@ static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cf
 	timing->drr_config.enabled = stream->ignore_msa_timing_param;
 	timing->drr_config.drr_active_variable = stream->vrr_active_variable;
 	timing->drr_config.drr_active_fixed = stream->vrr_active_fixed;
-	timing->drr_config.disallowed = !stream->allow_freesync;
+	timing->drr_config.disallowed = !dc_state_get_stream_allow_freesync(context, stream);
 
 	/* limit min refresh rate to DC cap */
 	min_hardware_refresh_in_uhz = stream->timing.min_refresh_in_uhz;
@@ -923,6 +924,7 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
 	struct dml2_display_cfg *dml_dispcfg = &dml_ctx->v21.display_config;
 	unsigned int plane_count = 0;
 	struct pipe_ctx *otg_master_pipe;
+	struct dml2_timing_cfg *timing;
 
 	memset(&dml_ctx->v21.dml_to_dc_pipe_mapping, 0, sizeof(struct dml2_dml_to_dc_pipe_mapping));
 
@@ -952,7 +954,10 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
 
 		ASSERT(disp_cfg_stream_location >= 0 && disp_cfg_stream_location < __DML2_WRAPPER_MAX_STREAMS_PLANES__);
 
-		populate_dml21_timing_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing, context->streams[stream_index], otg_master_pipe, dml_ctx);
+		timing = &dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing;
+		populate_dml21_timing_config_from_stream_state(timing, context,
+							       context->streams[stream_index],
+							       otg_master_pipe, dml_ctx);
 		populate_dml21_output_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].output, context->streams[stream_index], otg_master_pipe);
 		populate_dml21_writeback_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].writeback, context->streams[stream_index]);
 		populate_dml21_stream_overrides_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location], context->streams[stream_index], &context->stream_status[stream_index]);
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
index dbb8bb7fc20d..5333c4cc9eba 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
@@ -2152,7 +2152,7 @@ bool dcn30_can_support_mclk_switch_using_fw_based_vblank_stretch(struct dc *dc,
 	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(context))
 		return false;
 
-	if (!context->streams[0]->allow_freesync)
+	if (!dc_state_get_stream_allow_freesync(context, context->streams[0]))
 		return false;
 
 	if (context->streams[0]->vrr_active_variable && (dc->debug.disable_fams_gaming == INGAME_FAMS_DISABLE))
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
index 602a0e4e5dc0..a86d3111703f 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
@@ -579,7 +579,7 @@ struct dc_stream_state *dcn32_can_support_mclk_switch_using_fw_based_vblank_stre
 	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(fpo_candidate_stream, fpo_vactive_margin_us, refresh_rate))
 		return NULL;
 
-	if (!fpo_candidate_stream->allow_freesync)
+	if (!dc_state_get_stream_allow_freesync(context, fpo_candidate_stream))
 		return NULL;
 
 	if (fpo_candidate_stream->vrr_active_variable &&
@@ -673,7 +673,9 @@ bool dcn32_subvp_drr_admissable(struct dc *dc, struct dc_state *context)
 				non_subvp_pipes++;
 				drr_psr_capable = (drr_psr_capable || dcn32_is_psr_capable(pipe));
 				if (pipe->stream->ignore_msa_timing_param &&
-						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
+				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
+				     pipe->stream->vrr_active_variable ||
+				     pipe->stream->vrr_active_fixed)) {
 					drr_pipe_found = true;
 				}
 			}
@@ -734,7 +736,9 @@ bool dcn32_subvp_vblank_admissable(struct dc *dc, struct dc_state *context, int
 				non_subvp_pipes++;
 				vblank_psr_capable = (vblank_psr_capable || dcn32_is_psr_capable(pipe));
 				if (pipe->stream->ignore_msa_timing_param &&
-						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
+				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
+				     pipe->stream->vrr_active_variable ||
+				     pipe->stream->vrr_active_fixed)) {
 					drr_pipe_found = true;
 				}
 			}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
  2026-09-09 19:59 [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc Matthew Schwartz
@ 2026-09-21 19:41 ` George Zhang
  2026-09-21 19:42 ` George Zhang
  1 sibling, 0 replies; 6+ messages in thread
From: George Zhang @ 2026-09-21 19:41 UTC (permalink / raw)
  To: amd-gfx; +Cc: matthew.schwartz, Fangzhi Zuo, Leo Li

Hi Matthew,

A bit curious on what this patch is fixing. Do you have any more context 
behind the motivation for this patch?

Thanks,
George

On 9/9/26 15:59, Matthew Schwartz wrote:
> VRR-capable sinks set allow_freesync even with VRR_ENABLED=0, letting
> FPO stretch frames during UCLK switches. On a Navi 33 driving 4K120
> through a DP-HDMI PCON, a 40fps compositor limiter then runs at 37fps.
> This surfaced after commit d49086491bcb ("drm/amd/display: Adjust
> freesync pcon whitelist").
> 
> Allow frame stretching only for VRR_STATE_ACTIVE_VARIABLE. Store the
> proposed permission in the private DC state so TEST_ONLY and failed
> checks cannot change the shared stream. Apply changed permissions at
> commit and force full DC validation, including native DP and eDP.
> 
> Fixes: 7eaef1163b82 ("drm/amd/display: set allow_freesync parameter in DM")
> Signed-off-by: Matthew Schwartz <matthew.schwartz@linux.dev>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 26 ++++++++++++++++
>   .../display/amdgpu_dm/amdgpu_dm_freesync.c    |  1 -
>   drivers/gpu/drm/amd/display/dc/core/dc.c      |  4 +++
>   .../gpu/drm/amd/display/dc/core/dc_state.c    | 31 +++++++++++++++++++
>   drivers/gpu/drm/amd/display/dc/dc_state.h     |  6 ++++
>   drivers/gpu/drm/amd/display/dc/dc_stream.h    |  3 ++
>   .../drm/amd/display/dc/dml/dcn32/dcn32_fpu.c  |  9 ++++--
>   .../dml21_wrapper/dml21_translation_helper.c  |  9 ++++--
>   .../dc/resource/dcn30/dcn30_resource.c        |  2 +-
>   .../resource/dcn32/dcn32_resource_helpers.c   | 10 ++++--
>   10 files changed, 92 insertions(+), 9 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 f49cc198fb0d..09c088b878bb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3835,6 +3835,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   	unsigned long flags;
>   	u32 target_vblank, last_flip_vblank;
>   	bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
> +	bool allow_freesync = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>   	bool cursor_update = false;
>   	bool pflip_present = false;
>   	bool immediate_flip = false;
> @@ -4146,6 +4147,9 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>   		}
>   		mutex_lock(&dm->dc_lock);
> +		if (acrtc_state->update_type == UPDATE_TYPE_FULL &&
> +		    allow_freesync != acrtc_state->stream->allow_freesync)
> +			bundle->stream_update.allow_freesync = &allow_freesync;
>   		update_planes_and_stream_adapter(dm->dc,
>   					 acrtc_state->update_type,
>   					 planes_count,
> @@ -5413,6 +5417,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>   	struct dc_stream_state *new_stream;
>   	struct amdgpu_device *adev = dm->adev;
>   	int ret = 0;
> +	bool allow_freesync, old_allow_freesync;
>   
>   	/*
>   	 * TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
> @@ -5668,6 +5673,27 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>   	amdgpu_dm_get_freesync_config_for_crtc(dm_new_crtc_state,
>   				     dm_new_conn_state);
>   
> +	allow_freesync = dm_new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
> +	old_allow_freesync = dm_old_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
> +
> +	/* Keep the proposed FreeSync permission in the private DC state. */
> +	if (dm_new_crtc_state->stream != dm_old_crtc_state->stream ||
> +	    old_allow_freesync != allow_freesync) {
> +		ret = dm_atomic_get_state(state, &dm_state);
> +		if (ret)
> +			goto fail;
> +
> +		if (!dc_state_set_stream_allow_freesync(dm_state->context,
> +							dm_new_crtc_state->stream,
> +							allow_freesync)) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +
> +		/* DML must re-evaluate FPO when the permission changes. */
> +		*lock_and_validation_needed = true;
> +	}
> +
>   	return ret;
>   
>   fail:
> 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 281c6cec9f4b..9f40064d13c5 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
> @@ -253,7 +253,6 @@ void amdgpu_dm_update_freesync_state_on_stream(
>   	new_crtc_state->vrr_infopacket = vrr_infopacket;
>   
>   	new_stream->vrr_infopacket = vrr_infopacket;
> -	new_stream->allow_freesync = mod_freesync_get_freesync_enabled(&vrr_params);
>   
>   	/*
>   	 * HDMI ALLM: when Gaming-VRR is active (VRR_EN=1) and the sink
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index ebbd81995c38..6064cc992d91 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -3175,6 +3175,10 @@ static struct dc_update_descriptor check_update_surfaces_for_stream(
>   		if (stream_update->mst_bw_update)
>   			su_flags->bits.mst_bw = 1;
>   
> +		if (stream_update->allow_freesync &&
> +		    *stream_update->allow_freesync != stream_update->stream->allow_freesync)
> +			su_flags->bits.fams_changed = 1;
> +
>   		if (stream_update->stream->freesync_on_desktop &&
>   			(stream_update->vrr_infopacket || stream_update->allow_freesync ||
>   				stream_update->vrr_active_variable || stream_update->vrr_active_fixed))
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> index 1e4fe2f941ca..382858b6c1da 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> @@ -425,6 +425,7 @@ enum dc_status dc_state_add_stream(
>   	}
>   
>   	state->streams[state->stream_count] = stream;
> +	state->stream_status[state->stream_count].allow_freesync_valid = false;
>   	dc_stream_retain(stream);
>   	state->stream_count++;
>   
> @@ -746,6 +747,36 @@ struct dc_stream_status *dc_state_get_stream_status(
>   	return status.stream_count > 0 ? status.stream_status[0] : NULL;
>   }
>   
> +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
> +					const struct dc_stream_state *stream)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < state->stream_count; i++) {
> +		if (state->streams[i] == stream &&
> +		    state->stream_status[i].allow_freesync_valid)
> +			return state->stream_status[i].allow_freesync;
> +	}
> +
> +	return stream->allow_freesync;
> +}
> +
> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
> +					const struct dc_stream_state *stream, bool allow_freesync)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < state->stream_count; i++) {
> +		if (state->streams[i] == stream) {
> +			state->stream_status[i].allow_freesync = allow_freesync;
> +			state->stream_status[i].allow_freesync_valid = true;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>   enum mall_stream_type dc_state_get_pipe_subvp_type(const struct dc_state *state,
>   		const struct pipe_ctx *pipe_ctx)
>   {
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_state.h b/drivers/gpu/drm/amd/display/dc/dc_state.h
> index acf461225e9d..40ad063cec09 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_state.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_state.h
> @@ -81,4 +81,10 @@ struct dc_get_status_options;
>   enum dc_status dc_state_get_status(struct dc_state_status *status,
>   	const struct dc_get_status_options *options);
>   
> +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
> +					const struct dc_stream_state *stream);
> +
> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
> +					const struct dc_stream_state *stream, bool allow_freesync);
> +
>   #endif /* _DC_STATE_H_ */
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index 934ae381e730..14d8da9f15e5 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -62,6 +62,9 @@ struct dc_stream_status {
>   	bool is_abm_supported;
>   	struct mall_stream_config mall_stream_config;
>   	bool fpo_in_use;
> +	/* Proposed permission for validation, without modifying the stream. */
> +	bool allow_freesync;
> +	bool allow_freesync_valid;
>   };
>   
>   
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> index b5bd1358fdfd..a816e47153ad 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> @@ -844,7 +844,9 @@ static bool subvp_drr_schedulable(struct dc *dc, struct dc_state *context)
>   			continue;
>   
>   		if (dc_state_get_pipe_subvp_type(context, drr_pipe) == SUBVP_NONE && drr_pipe->stream->ignore_msa_timing_param &&
> -				(drr_pipe->stream->allow_freesync || drr_pipe->stream->vrr_active_variable || drr_pipe->stream->vrr_active_fixed)) {
> +				(dc_state_get_stream_allow_freesync(context, drr_pipe->stream) ||
> +				 drr_pipe->stream->vrr_active_variable ||
> +				 drr_pipe->stream->vrr_active_fixed)) {
>   			drr_found = true;
>   			break;
>   		}
> @@ -3444,7 +3446,10 @@ bool dcn32_allow_subvp_high_refresh_rate(struct dc *dc, struct dc_state *context
>   				uint32_t height = subvp_high_refresh_list.res[i].height;
>   
>   				if (dcn32_check_native_scaling_for_res(pipe, width, height)) {
> -					if ((context->stream_count == 1 && !pipe->stream->allow_freesync) || context->stream_count > 1) {
> +					if ((context->stream_count == 1 &&
> +					     !dc_state_get_stream_allow_freesync(context,
> +										 pipe->stream)) ||
> +					    context->stream_count > 1) {
>   						allow = true;
>   						break;
>   					}
> diff --git a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> index 3c25de54d55b..031f90955b95 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> @@ -125,6 +125,7 @@ static unsigned int calc_vblank_nom_lines(const struct dc_stream_state *stream,
>   }
>   
>   static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cfg *timing,
> +		const struct dc_state *context,
>   		struct dc_stream_state *stream,
>   		struct pipe_ctx *otg_master_pipe,
>   		struct dml2_context *dml_ctx)
> @@ -166,7 +167,7 @@ static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cf
>   	timing->drr_config.enabled = stream->ignore_msa_timing_param;
>   	timing->drr_config.drr_active_variable = stream->vrr_active_variable;
>   	timing->drr_config.drr_active_fixed = stream->vrr_active_fixed;
> -	timing->drr_config.disallowed = !stream->allow_freesync;
> +	timing->drr_config.disallowed = !dc_state_get_stream_allow_freesync(context, stream);
>   
>   	/* limit min refresh rate to DC cap */
>   	min_hardware_refresh_in_uhz = stream->timing.min_refresh_in_uhz;
> @@ -923,6 +924,7 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>   	struct dml2_display_cfg *dml_dispcfg = &dml_ctx->v21.display_config;
>   	unsigned int plane_count = 0;
>   	struct pipe_ctx *otg_master_pipe;
> +	struct dml2_timing_cfg *timing;
>   
>   	memset(&dml_ctx->v21.dml_to_dc_pipe_mapping, 0, sizeof(struct dml2_dml_to_dc_pipe_mapping));
>   
> @@ -952,7 +954,10 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>   
>   		ASSERT(disp_cfg_stream_location >= 0 && disp_cfg_stream_location < __DML2_WRAPPER_MAX_STREAMS_PLANES__);
>   
> -		populate_dml21_timing_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing, context->streams[stream_index], otg_master_pipe, dml_ctx);
> +		timing = &dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing;
> +		populate_dml21_timing_config_from_stream_state(timing, context,
> +							       context->streams[stream_index],
> +							       otg_master_pipe, dml_ctx);
>   		populate_dml21_output_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].output, context->streams[stream_index], otg_master_pipe);
>   		populate_dml21_writeback_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].writeback, context->streams[stream_index]);
>   		populate_dml21_stream_overrides_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location], context->streams[stream_index], &context->stream_status[stream_index]);
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> index dbb8bb7fc20d..5333c4cc9eba 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> @@ -2152,7 +2152,7 @@ bool dcn30_can_support_mclk_switch_using_fw_based_vblank_stretch(struct dc *dc,
>   	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(context))
>   		return false;
>   
> -	if (!context->streams[0]->allow_freesync)
> +	if (!dc_state_get_stream_allow_freesync(context, context->streams[0]))
>   		return false;
>   
>   	if (context->streams[0]->vrr_active_variable && (dc->debug.disable_fams_gaming == INGAME_FAMS_DISABLE))
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> index 602a0e4e5dc0..a86d3111703f 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> @@ -579,7 +579,7 @@ struct dc_stream_state *dcn32_can_support_mclk_switch_using_fw_based_vblank_stre
>   	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(fpo_candidate_stream, fpo_vactive_margin_us, refresh_rate))
>   		return NULL;
>   
> -	if (!fpo_candidate_stream->allow_freesync)
> +	if (!dc_state_get_stream_allow_freesync(context, fpo_candidate_stream))
>   		return NULL;
>   
>   	if (fpo_candidate_stream->vrr_active_variable &&
> @@ -673,7 +673,9 @@ bool dcn32_subvp_drr_admissable(struct dc *dc, struct dc_state *context)
>   				non_subvp_pipes++;
>   				drr_psr_capable = (drr_psr_capable || dcn32_is_psr_capable(pipe));
>   				if (pipe->stream->ignore_msa_timing_param &&
> -						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
> +				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
> +				     pipe->stream->vrr_active_variable ||
> +				     pipe->stream->vrr_active_fixed)) {
>   					drr_pipe_found = true;
>   				}
>   			}
> @@ -734,7 +736,9 @@ bool dcn32_subvp_vblank_admissable(struct dc *dc, struct dc_state *context, int
>   				non_subvp_pipes++;
>   				vblank_psr_capable = (vblank_psr_capable || dcn32_is_psr_capable(pipe));
>   				if (pipe->stream->ignore_msa_timing_param &&
> -						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
> +				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
> +				     pipe->stream->vrr_active_variable ||
> +				     pipe->stream->vrr_active_fixed)) {
>   					drr_pipe_found = true;
>   				}
>   			}


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
  2026-09-09 19:59 [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc Matthew Schwartz
  2026-09-21 19:41 ` George Zhang
@ 2026-09-21 19:42 ` George Zhang
  2026-09-21 19:47   ` Matthew Schwartz
  1 sibling, 1 reply; 6+ messages in thread
From: George Zhang @ 2026-09-21 19:42 UTC (permalink / raw)
  To: amd-gfx; +Cc: matthew.schwartz, Fangzhi Zuo, Leo Li

Hi Matthew,

A bit curious on what this patch is fixing. Do you have any more context 
behind the motivation for this patch?

Thanks,
George

On 9/9/26 15:59, Matthew Schwartz wrote:
> VRR-capable sinks set allow_freesync even with VRR_ENABLED=0, letting
> FPO stretch frames during UCLK switches. On a Navi 33 driving 4K120
> through a DP-HDMI PCON, a 40fps compositor limiter then runs at 37fps.
> This surfaced after commit d49086491bcb ("drm/amd/display: Adjust
> freesync pcon whitelist").
> 
> Allow frame stretching only for VRR_STATE_ACTIVE_VARIABLE. Store the
> proposed permission in the private DC state so TEST_ONLY and failed
> checks cannot change the shared stream. Apply changed permissions at
> commit and force full DC validation, including native DP and eDP.
> 
> Fixes: 7eaef1163b82 ("drm/amd/display: set allow_freesync parameter in DM")
> Signed-off-by: Matthew Schwartz <matthew.schwartz@linux.dev>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 26 ++++++++++++++++
>   .../display/amdgpu_dm/amdgpu_dm_freesync.c    |  1 -
>   drivers/gpu/drm/amd/display/dc/core/dc.c      |  4 +++
>   .../gpu/drm/amd/display/dc/core/dc_state.c    | 31 +++++++++++++++++++
>   drivers/gpu/drm/amd/display/dc/dc_state.h     |  6 ++++
>   drivers/gpu/drm/amd/display/dc/dc_stream.h    |  3 ++
>   .../drm/amd/display/dc/dml/dcn32/dcn32_fpu.c  |  9 ++++--
>   .../dml21_wrapper/dml21_translation_helper.c  |  9 ++++--
>   .../dc/resource/dcn30/dcn30_resource.c        |  2 +-
>   .../resource/dcn32/dcn32_resource_helpers.c   | 10 ++++--
>   10 files changed, 92 insertions(+), 9 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 f49cc198fb0d..09c088b878bb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3835,6 +3835,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   	unsigned long flags;
>   	u32 target_vblank, last_flip_vblank;
>   	bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
> +	bool allow_freesync = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>   	bool cursor_update = false;
>   	bool pflip_present = false;
>   	bool immediate_flip = false;
> @@ -4146,6 +4147,9 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>   		}
>   		mutex_lock(&dm->dc_lock);
> +		if (acrtc_state->update_type == UPDATE_TYPE_FULL &&
> +		    allow_freesync != acrtc_state->stream->allow_freesync)
> +			bundle->stream_update.allow_freesync = &allow_freesync;
>   		update_planes_and_stream_adapter(dm->dc,
>   					 acrtc_state->update_type,
>   					 planes_count,
> @@ -5413,6 +5417,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>   	struct dc_stream_state *new_stream;
>   	struct amdgpu_device *adev = dm->adev;
>   	int ret = 0;
> +	bool allow_freesync, old_allow_freesync;
>   
>   	/*
>   	 * TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
> @@ -5668,6 +5673,27 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>   	amdgpu_dm_get_freesync_config_for_crtc(dm_new_crtc_state,
>   				     dm_new_conn_state);
>   
> +	allow_freesync = dm_new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
> +	old_allow_freesync = dm_old_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
> +
> +	/* Keep the proposed FreeSync permission in the private DC state. */
> +	if (dm_new_crtc_state->stream != dm_old_crtc_state->stream ||
> +	    old_allow_freesync != allow_freesync) {
> +		ret = dm_atomic_get_state(state, &dm_state);
> +		if (ret)
> +			goto fail;
> +
> +		if (!dc_state_set_stream_allow_freesync(dm_state->context,
> +							dm_new_crtc_state->stream,
> +							allow_freesync)) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +
> +		/* DML must re-evaluate FPO when the permission changes. */
> +		*lock_and_validation_needed = true;
> +	}
> +
>   	return ret;
>   
>   fail:
> 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 281c6cec9f4b..9f40064d13c5 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
> @@ -253,7 +253,6 @@ void amdgpu_dm_update_freesync_state_on_stream(
>   	new_crtc_state->vrr_infopacket = vrr_infopacket;
>   
>   	new_stream->vrr_infopacket = vrr_infopacket;
> -	new_stream->allow_freesync = mod_freesync_get_freesync_enabled(&vrr_params);
>   
>   	/*
>   	 * HDMI ALLM: when Gaming-VRR is active (VRR_EN=1) and the sink
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index ebbd81995c38..6064cc992d91 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -3175,6 +3175,10 @@ static struct dc_update_descriptor check_update_surfaces_for_stream(
>   		if (stream_update->mst_bw_update)
>   			su_flags->bits.mst_bw = 1;
>   
> +		if (stream_update->allow_freesync &&
> +		    *stream_update->allow_freesync != stream_update->stream->allow_freesync)
> +			su_flags->bits.fams_changed = 1;
> +
>   		if (stream_update->stream->freesync_on_desktop &&
>   			(stream_update->vrr_infopacket || stream_update->allow_freesync ||
>   				stream_update->vrr_active_variable || stream_update->vrr_active_fixed))
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> index 1e4fe2f941ca..382858b6c1da 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> @@ -425,6 +425,7 @@ enum dc_status dc_state_add_stream(
>   	}
>   
>   	state->streams[state->stream_count] = stream;
> +	state->stream_status[state->stream_count].allow_freesync_valid = false;
>   	dc_stream_retain(stream);
>   	state->stream_count++;
>   
> @@ -746,6 +747,36 @@ struct dc_stream_status *dc_state_get_stream_status(
>   	return status.stream_count > 0 ? status.stream_status[0] : NULL;
>   }
>   
> +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
> +					const struct dc_stream_state *stream)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < state->stream_count; i++) {
> +		if (state->streams[i] == stream &&
> +		    state->stream_status[i].allow_freesync_valid)
> +			return state->stream_status[i].allow_freesync;
> +	}
> +
> +	return stream->allow_freesync;
> +}
> +
> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
> +					const struct dc_stream_state *stream, bool allow_freesync)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < state->stream_count; i++) {
> +		if (state->streams[i] == stream) {
> +			state->stream_status[i].allow_freesync = allow_freesync;
> +			state->stream_status[i].allow_freesync_valid = true;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>   enum mall_stream_type dc_state_get_pipe_subvp_type(const struct dc_state *state,
>   		const struct pipe_ctx *pipe_ctx)
>   {
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_state.h b/drivers/gpu/drm/amd/display/dc/dc_state.h
> index acf461225e9d..40ad063cec09 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_state.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_state.h
> @@ -81,4 +81,10 @@ struct dc_get_status_options;
>   enum dc_status dc_state_get_status(struct dc_state_status *status,
>   	const struct dc_get_status_options *options);
>   
> +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
> +					const struct dc_stream_state *stream);
> +
> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
> +					const struct dc_stream_state *stream, bool allow_freesync);
> +
>   #endif /* _DC_STATE_H_ */
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index 934ae381e730..14d8da9f15e5 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -62,6 +62,9 @@ struct dc_stream_status {
>   	bool is_abm_supported;
>   	struct mall_stream_config mall_stream_config;
>   	bool fpo_in_use;
> +	/* Proposed permission for validation, without modifying the stream. */
> +	bool allow_freesync;
> +	bool allow_freesync_valid;
>   };
>   
>   
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> index b5bd1358fdfd..a816e47153ad 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> @@ -844,7 +844,9 @@ static bool subvp_drr_schedulable(struct dc *dc, struct dc_state *context)
>   			continue;
>   
>   		if (dc_state_get_pipe_subvp_type(context, drr_pipe) == SUBVP_NONE && drr_pipe->stream->ignore_msa_timing_param &&
> -				(drr_pipe->stream->allow_freesync || drr_pipe->stream->vrr_active_variable || drr_pipe->stream->vrr_active_fixed)) {
> +				(dc_state_get_stream_allow_freesync(context, drr_pipe->stream) ||
> +				 drr_pipe->stream->vrr_active_variable ||
> +				 drr_pipe->stream->vrr_active_fixed)) {
>   			drr_found = true;
>   			break;
>   		}
> @@ -3444,7 +3446,10 @@ bool dcn32_allow_subvp_high_refresh_rate(struct dc *dc, struct dc_state *context
>   				uint32_t height = subvp_high_refresh_list.res[i].height;
>   
>   				if (dcn32_check_native_scaling_for_res(pipe, width, height)) {
> -					if ((context->stream_count == 1 && !pipe->stream->allow_freesync) || context->stream_count > 1) {
> +					if ((context->stream_count == 1 &&
> +					     !dc_state_get_stream_allow_freesync(context,
> +										 pipe->stream)) ||
> +					    context->stream_count > 1) {
>   						allow = true;
>   						break;
>   					}
> diff --git a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> index 3c25de54d55b..031f90955b95 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
> @@ -125,6 +125,7 @@ static unsigned int calc_vblank_nom_lines(const struct dc_stream_state *stream,
>   }
>   
>   static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cfg *timing,
> +		const struct dc_state *context,
>   		struct dc_stream_state *stream,
>   		struct pipe_ctx *otg_master_pipe,
>   		struct dml2_context *dml_ctx)
> @@ -166,7 +167,7 @@ static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cf
>   	timing->drr_config.enabled = stream->ignore_msa_timing_param;
>   	timing->drr_config.drr_active_variable = stream->vrr_active_variable;
>   	timing->drr_config.drr_active_fixed = stream->vrr_active_fixed;
> -	timing->drr_config.disallowed = !stream->allow_freesync;
> +	timing->drr_config.disallowed = !dc_state_get_stream_allow_freesync(context, stream);
>   
>   	/* limit min refresh rate to DC cap */
>   	min_hardware_refresh_in_uhz = stream->timing.min_refresh_in_uhz;
> @@ -923,6 +924,7 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>   	struct dml2_display_cfg *dml_dispcfg = &dml_ctx->v21.display_config;
>   	unsigned int plane_count = 0;
>   	struct pipe_ctx *otg_master_pipe;
> +	struct dml2_timing_cfg *timing;
>   
>   	memset(&dml_ctx->v21.dml_to_dc_pipe_mapping, 0, sizeof(struct dml2_dml_to_dc_pipe_mapping));
>   
> @@ -952,7 +954,10 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>   
>   		ASSERT(disp_cfg_stream_location >= 0 && disp_cfg_stream_location < __DML2_WRAPPER_MAX_STREAMS_PLANES__);
>   
> -		populate_dml21_timing_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing, context->streams[stream_index], otg_master_pipe, dml_ctx);
> +		timing = &dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing;
> +		populate_dml21_timing_config_from_stream_state(timing, context,
> +							       context->streams[stream_index],
> +							       otg_master_pipe, dml_ctx);
>   		populate_dml21_output_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].output, context->streams[stream_index], otg_master_pipe);
>   		populate_dml21_writeback_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].writeback, context->streams[stream_index]);
>   		populate_dml21_stream_overrides_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location], context->streams[stream_index], &context->stream_status[stream_index]);
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> index dbb8bb7fc20d..5333c4cc9eba 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
> @@ -2152,7 +2152,7 @@ bool dcn30_can_support_mclk_switch_using_fw_based_vblank_stretch(struct dc *dc,
>   	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(context))
>   		return false;
>   
> -	if (!context->streams[0]->allow_freesync)
> +	if (!dc_state_get_stream_allow_freesync(context, context->streams[0]))
>   		return false;
>   
>   	if (context->streams[0]->vrr_active_variable && (dc->debug.disable_fams_gaming == INGAME_FAMS_DISABLE))
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> index 602a0e4e5dc0..a86d3111703f 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
> @@ -579,7 +579,7 @@ struct dc_stream_state *dcn32_can_support_mclk_switch_using_fw_based_vblank_stre
>   	if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(fpo_candidate_stream, fpo_vactive_margin_us, refresh_rate))
>   		return NULL;
>   
> -	if (!fpo_candidate_stream->allow_freesync)
> +	if (!dc_state_get_stream_allow_freesync(context, fpo_candidate_stream))
>   		return NULL;
>   
>   	if (fpo_candidate_stream->vrr_active_variable &&
> @@ -673,7 +673,9 @@ bool dcn32_subvp_drr_admissable(struct dc *dc, struct dc_state *context)
>   				non_subvp_pipes++;
>   				drr_psr_capable = (drr_psr_capable || dcn32_is_psr_capable(pipe));
>   				if (pipe->stream->ignore_msa_timing_param &&
> -						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
> +				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
> +				     pipe->stream->vrr_active_variable ||
> +				     pipe->stream->vrr_active_fixed)) {
>   					drr_pipe_found = true;
>   				}
>   			}
> @@ -734,7 +736,9 @@ bool dcn32_subvp_vblank_admissable(struct dc *dc, struct dc_state *context, int
>   				non_subvp_pipes++;
>   				vblank_psr_capable = (vblank_psr_capable || dcn32_is_psr_capable(pipe));
>   				if (pipe->stream->ignore_msa_timing_param &&
> -						(pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
> +				    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
> +				     pipe->stream->vrr_active_variable ||
> +				     pipe->stream->vrr_active_fixed)) {
>   					drr_pipe_found = true;
>   				}
>   			}


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
  2026-09-21 19:42 ` George Zhang
@ 2026-09-21 19:47   ` Matthew Schwartz
  2026-09-22 17:50     ` Leo Li
  0 siblings, 1 reply; 6+ messages in thread
From: Matthew Schwartz @ 2026-09-21 19:47 UTC (permalink / raw)
  To: George Zhang, amd-gfx; +Cc: Fangzhi Zuo, Leo Li

On 9/21/26 12:42 PM, George Zhang wrote:
> Hi Matthew,
> 
> A bit curious on what this patch is fixing. Do you have any more context behind the motivation for this patch?

Hi George,

After updating to Linux 7.2.x, I noticed gamescope's frame limiter falling
below the requested rate. The reproducer was a Navi 33 driving 4K120
through a DP-HDMI PCON, with VRR disabled. A 40fps limit was producing
roughly 37fps.

Gamescope was pacing against the fixed 120Hz refresh interval, but we
observed frame intervals stretching to around 9.34ms instead of 8.33ms.
The OTG vtotal also increased from 2249 to 2521. This was enough to
disrupt the limiter's pacing despite userspace leaving VRR_ENABLED=0.

Tracing this led to allow_freesync remaining true for a VRR-capable
sink even when the FreeSync state was inactive. That allowed FPO to
stretch vblank around UCLK switches. The PCON whitelist change exposed
this on our setup by making the sink eligible for that path.

The intent is to keep the frame period fixed when userspace has
disabled VRR, while retaining FPO eligibility when variable refresh is
actually requested. With the patch, the original setup holds the
requested 40fps again. I also tested VRR toggling on a Legion Go 2's
internal eDP OLED panel and did not observe a regression.

Thanks,
Matt

> 
> Thanks,
> George
> 
> On 9/9/26 15:59, Matthew Schwartz wrote:
>> VRR-capable sinks set allow_freesync even with VRR_ENABLED=0, letting
>> FPO stretch frames during UCLK switches. On a Navi 33 driving 4K120
>> through a DP-HDMI PCON, a 40fps compositor limiter then runs at 37fps.
>> This surfaced after commit d49086491bcb ("drm/amd/display: Adjust
>> freesync pcon whitelist").
>>
>> Allow frame stretching only for VRR_STATE_ACTIVE_VARIABLE. Store the
>> proposed permission in the private DC state so TEST_ONLY and failed
>> checks cannot change the shared stream. Apply changed permissions at
>> commit and force full DC validation, including native DP and eDP.
>>
>> Fixes: 7eaef1163b82 ("drm/amd/display: set allow_freesync parameter in DM")
>> Signed-off-by: Matthew Schwartz <matthew.schwartz@linux.dev>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 26 ++++++++++++++++
>>   .../display/amdgpu_dm/amdgpu_dm_freesync.c    |  1 -
>>   drivers/gpu/drm/amd/display/dc/core/dc.c      |  4 +++
>>   .../gpu/drm/amd/display/dc/core/dc_state.c    | 31 +++++++++++++++++++
>>   drivers/gpu/drm/amd/display/dc/dc_state.h     |  6 ++++
>>   drivers/gpu/drm/amd/display/dc/dc_stream.h    |  3 ++
>>   .../drm/amd/display/dc/dml/dcn32/dcn32_fpu.c  |  9 ++++--
>>   .../dml21_wrapper/dml21_translation_helper.c  |  9 ++++--
>>   .../dc/resource/dcn30/dcn30_resource.c        |  2 +-
>>   .../resource/dcn32/dcn32_resource_helpers.c   | 10 ++++--
>>   10 files changed, 92 insertions(+), 9 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 f49cc198fb0d..09c088b878bb 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -3835,6 +3835,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>>       unsigned long flags;
>>       u32 target_vblank, last_flip_vblank;
>>       bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
>> +    bool allow_freesync = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>>       bool cursor_update = false;
>>       bool pflip_present = false;
>>       bool immediate_flip = false;
>> @@ -4146,6 +4147,9 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>>               spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>>           }
>>           mutex_lock(&dm->dc_lock);
>> +        if (acrtc_state->update_type == UPDATE_TYPE_FULL &&
>> +            allow_freesync != acrtc_state->stream->allow_freesync)
>> +            bundle->stream_update.allow_freesync = &allow_freesync;
>>           update_planes_and_stream_adapter(dm->dc,
>>                        acrtc_state->update_type,
>>                        planes_count,
>> @@ -5413,6 +5417,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>>       struct dc_stream_state *new_stream;
>>       struct amdgpu_device *adev = dm->adev;
>>       int ret = 0;
>> +    bool allow_freesync, old_allow_freesync;
>>         /*
>>        * TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
>> @@ -5668,6 +5673,27 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
>>       amdgpu_dm_get_freesync_config_for_crtc(dm_new_crtc_state,
>>                        dm_new_conn_state);
>>   +    allow_freesync = dm_new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>> +    old_allow_freesync = dm_old_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
>> +
>> +    /* Keep the proposed FreeSync permission in the private DC state. */
>> +    if (dm_new_crtc_state->stream != dm_old_crtc_state->stream ||
>> +        old_allow_freesync != allow_freesync) {
>> +        ret = dm_atomic_get_state(state, &dm_state);
>> +        if (ret)
>> +            goto fail;
>> +
>> +        if (!dc_state_set_stream_allow_freesync(dm_state->context,
>> +                            dm_new_crtc_state->stream,
>> +                            allow_freesync)) {
>> +            ret = -EINVAL;
>> +            goto fail;
>> +        }
>> +
>> +        /* DML must re-evaluate FPO when the permission changes. */
>> +        *lock_and_validation_needed = true;
>> +    }
>> +
>>       return ret;
>>     fail:
>> 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 281c6cec9f4b..9f40064d13c5 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
>> @@ -253,7 +253,6 @@ void amdgpu_dm_update_freesync_state_on_stream(
>>       new_crtc_state->vrr_infopacket = vrr_infopacket;
>>         new_stream->vrr_infopacket = vrr_infopacket;
>> -    new_stream->allow_freesync = mod_freesync_get_freesync_enabled(&vrr_params);
>>         /*
>>        * HDMI ALLM: when Gaming-VRR is active (VRR_EN=1) and the sink
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> index ebbd81995c38..6064cc992d91 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> @@ -3175,6 +3175,10 @@ static struct dc_update_descriptor check_update_surfaces_for_stream(
>>           if (stream_update->mst_bw_update)
>>               su_flags->bits.mst_bw = 1;
>>   +        if (stream_update->allow_freesync &&
>> +            *stream_update->allow_freesync != stream_update->stream->allow_freesync)
>> +            su_flags->bits.fams_changed = 1;
>> +
>>           if (stream_update->stream->freesync_on_desktop &&
>>               (stream_update->vrr_infopacket || stream_update->allow_freesync ||
>>                   stream_update->vrr_active_variable || stream_update->vrr_active_fixed))
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>> index 1e4fe2f941ca..382858b6c1da 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>> @@ -425,6 +425,7 @@ enum dc_status dc_state_add_stream(
>>       }
>>         state->streams[state->stream_count] = stream;
>> +    state->stream_status[state->stream_count].allow_freesync_valid = false;
>>       dc_stream_retain(stream);
>>       state->stream_count++;
>>   @@ -746,6 +747,36 @@ struct dc_stream_status *dc_state_get_stream_status(
>>       return status.stream_count > 0 ? status.stream_status[0] : NULL;
>>   }
>>   +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
>> +                    const struct dc_stream_state *stream)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < state->stream_count; i++) {
>> +        if (state->streams[i] == stream &&
>> +            state->stream_status[i].allow_freesync_valid)
>> +            return state->stream_status[i].allow_freesync;
>> +    }
>> +
>> +    return stream->allow_freesync;
>> +}
>> +
>> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
>> +                    const struct dc_stream_state *stream, bool allow_freesync)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < state->stream_count; i++) {
>> +        if (state->streams[i] == stream) {
>> +            state->stream_status[i].allow_freesync = allow_freesync;
>> +            state->stream_status[i].allow_freesync_valid = true;
>> +            return true;
>> +        }
>> +    }
>> +
>> +    return false;
>> +}
>> +
>>   enum mall_stream_type dc_state_get_pipe_subvp_type(const struct dc_state *state,
>>           const struct pipe_ctx *pipe_ctx)
>>   {
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_state.h b/drivers/gpu/drm/amd/display/dc/dc_state.h
>> index acf461225e9d..40ad063cec09 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc_state.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc_state.h
>> @@ -81,4 +81,10 @@ struct dc_get_status_options;
>>   enum dc_status dc_state_get_status(struct dc_state_status *status,
>>       const struct dc_get_status_options *options);
>>   +bool dc_state_get_stream_allow_freesync(const struct dc_state *state,
>> +                    const struct dc_stream_state *stream);
>> +
>> +bool dc_state_set_stream_allow_freesync(struct dc_state *state,
>> +                    const struct dc_stream_state *stream, bool allow_freesync);
>> +
>>   #endif /* _DC_STATE_H_ */
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
>> index 934ae381e730..14d8da9f15e5 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
>> @@ -62,6 +62,9 @@ struct dc_stream_status {
>>       bool is_abm_supported;
>>       struct mall_stream_config mall_stream_config;
>>       bool fpo_in_use;
>> +    /* Proposed permission for validation, without modifying the stream. */
>> +    bool allow_freesync;
>> +    bool allow_freesync_valid;
>>   };
>>     diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
>> index b5bd1358fdfd..a816e47153ad 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
>> @@ -844,7 +844,9 @@ static bool subvp_drr_schedulable(struct dc *dc, struct dc_state *context)
>>               continue;
>>             if (dc_state_get_pipe_subvp_type(context, drr_pipe) == SUBVP_NONE && drr_pipe->stream->ignore_msa_timing_param &&
>> -                (drr_pipe->stream->allow_freesync || drr_pipe->stream->vrr_active_variable || drr_pipe->stream->vrr_active_fixed)) {
>> +                (dc_state_get_stream_allow_freesync(context, drr_pipe->stream) ||
>> +                 drr_pipe->stream->vrr_active_variable ||
>> +                 drr_pipe->stream->vrr_active_fixed)) {
>>               drr_found = true;
>>               break;
>>           }
>> @@ -3444,7 +3446,10 @@ bool dcn32_allow_subvp_high_refresh_rate(struct dc *dc, struct dc_state *context
>>                   uint32_t height = subvp_high_refresh_list.res[i].height;
>>                     if (dcn32_check_native_scaling_for_res(pipe, width, height)) {
>> -                    if ((context->stream_count == 1 && !pipe->stream->allow_freesync) || context->stream_count > 1) {
>> +                    if ((context->stream_count == 1 &&
>> +                         !dc_state_get_stream_allow_freesync(context,
>> +                                         pipe->stream)) ||
>> +                        context->stream_count > 1) {
>>                           allow = true;
>>                           break;
>>                       }
>> diff --git a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
>> index 3c25de54d55b..031f90955b95 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dml2_wrapper/dml21_wrapper/dml21_translation_helper.c
>> @@ -125,6 +125,7 @@ static unsigned int calc_vblank_nom_lines(const struct dc_stream_state *stream,
>>   }
>>     static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cfg *timing,
>> +        const struct dc_state *context,
>>           struct dc_stream_state *stream,
>>           struct pipe_ctx *otg_master_pipe,
>>           struct dml2_context *dml_ctx)
>> @@ -166,7 +167,7 @@ static void populate_dml21_timing_config_from_stream_state(struct dml2_timing_cf
>>       timing->drr_config.enabled = stream->ignore_msa_timing_param;
>>       timing->drr_config.drr_active_variable = stream->vrr_active_variable;
>>       timing->drr_config.drr_active_fixed = stream->vrr_active_fixed;
>> -    timing->drr_config.disallowed = !stream->allow_freesync;
>> +    timing->drr_config.disallowed = !dc_state_get_stream_allow_freesync(context, stream);
>>         /* limit min refresh rate to DC cap */
>>       min_hardware_refresh_in_uhz = stream->timing.min_refresh_in_uhz;
>> @@ -923,6 +924,7 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>>       struct dml2_display_cfg *dml_dispcfg = &dml_ctx->v21.display_config;
>>       unsigned int plane_count = 0;
>>       struct pipe_ctx *otg_master_pipe;
>> +    struct dml2_timing_cfg *timing;
>>         memset(&dml_ctx->v21.dml_to_dc_pipe_mapping, 0, sizeof(struct dml2_dml_to_dc_pipe_mapping));
>>   @@ -952,7 +954,10 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
>>             ASSERT(disp_cfg_stream_location >= 0 && disp_cfg_stream_location < __DML2_WRAPPER_MAX_STREAMS_PLANES__);
>>   -        populate_dml21_timing_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing, context->streams[stream_index], otg_master_pipe, dml_ctx);
>> +        timing = &dml_dispcfg->stream_descriptors[disp_cfg_stream_location].timing;
>> +        populate_dml21_timing_config_from_stream_state(timing, context,
>> +                                   context->streams[stream_index],
>> +                                   otg_master_pipe, dml_ctx);
>>           populate_dml21_output_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].output, context->streams[stream_index], otg_master_pipe);
>>           populate_dml21_writeback_config_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location].writeback, context->streams[stream_index]);
>>           populate_dml21_stream_overrides_from_stream_state(&dml_dispcfg->stream_descriptors[disp_cfg_stream_location], context->streams[stream_index], &context->stream_status[stream_index]);
>> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
>> index dbb8bb7fc20d..5333c4cc9eba 100644
>> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn30/dcn30_resource.c
>> @@ -2152,7 +2152,7 @@ bool dcn30_can_support_mclk_switch_using_fw_based_vblank_stretch(struct dc *dc,
>>       if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(context))
>>           return false;
>>   -    if (!context->streams[0]->allow_freesync)
>> +    if (!dc_state_get_stream_allow_freesync(context, context->streams[0]))
>>           return false;
>>         if (context->streams[0]->vrr_active_variable && (dc->debug.disable_fams_gaming == INGAME_FAMS_DISABLE))
>> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
>> index 602a0e4e5dc0..a86d3111703f 100644
>> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource_helpers.c
>> @@ -579,7 +579,7 @@ struct dc_stream_state *dcn32_can_support_mclk_switch_using_fw_based_vblank_stre
>>       if (!is_refresh_rate_support_mclk_switch_using_fw_based_vblank_stretch(fpo_candidate_stream, fpo_vactive_margin_us, refresh_rate))
>>           return NULL;
>>   -    if (!fpo_candidate_stream->allow_freesync)
>> +    if (!dc_state_get_stream_allow_freesync(context, fpo_candidate_stream))
>>           return NULL;
>>         if (fpo_candidate_stream->vrr_active_variable &&
>> @@ -673,7 +673,9 @@ bool dcn32_subvp_drr_admissable(struct dc *dc, struct dc_state *context)
>>                   non_subvp_pipes++;
>>                   drr_psr_capable = (drr_psr_capable || dcn32_is_psr_capable(pipe));
>>                   if (pipe->stream->ignore_msa_timing_param &&
>> -                        (pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
>> +                    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
>> +                     pipe->stream->vrr_active_variable ||
>> +                     pipe->stream->vrr_active_fixed)) {
>>                       drr_pipe_found = true;
>>                   }
>>               }
>> @@ -734,7 +736,9 @@ bool dcn32_subvp_vblank_admissable(struct dc *dc, struct dc_state *context, int
>>                   non_subvp_pipes++;
>>                   vblank_psr_capable = (vblank_psr_capable || dcn32_is_psr_capable(pipe));
>>                   if (pipe->stream->ignore_msa_timing_param &&
>> -                        (pipe->stream->allow_freesync || pipe->stream->vrr_active_variable || pipe->stream->vrr_active_fixed)) {
>> +                    (dc_state_get_stream_allow_freesync(context, pipe->stream) ||
>> +                     pipe->stream->vrr_active_variable ||
>> +                     pipe->stream->vrr_active_fixed)) {
>>                       drr_pipe_found = true;
>>                   }
>>               }
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
  2026-09-21 19:47   ` Matthew Schwartz
@ 2026-09-22 17:50     ` Leo Li
       [not found]       ` <a2a345da-a199-48f8-8461-ebd298aa252a@linux.dev>
  0 siblings, 1 reply; 6+ messages in thread
From: Leo Li @ 2026-09-22 17:50 UTC (permalink / raw)
  To: Matthew Schwartz, George Zhang, amd-gfx; +Cc: Fangzhi Zuo



On 2026-09-21 15:47, Matthew Schwartz wrote:
> Hi George,
> 
> After updating to Linux 7.2.x, I noticed gamescope's frame limiter falling
> below the requested rate. The reproducer was a Navi 33 driving 4K120
> through a DP-HDMI PCON, with VRR disabled. A 40fps limit was producing
> roughly 37fps.
> 
> Gamescope was pacing against the fixed 120Hz refresh interval, but we
> observed frame intervals stretching to around 9.34ms instead of 8.33ms.
> The OTG vtotal also increased from 2249 to 2521. This was enough to
> disrupt the limiter's pacing despite userspace leaving VRR_ENABLED=0.

Hi Matt,

I wonder if this has to do with re-introducing 2-frame vblank off for NV3+
DGPUs, which made it into 7.2:

fba211b078d6 ("Revert "drm/amd/display: Restore 5s vbl offdelay for NV3x+ DGPUs"")

Since gamescope is only updating every 3rd frame, it's possible vblanks were
turned off during those two frames, signaling driver to enable idle optimizations.

To test this idea, does bumping this line to something like 5 frames ((u64)50 *) help?
https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L3377

Thanks,
Leo 
> 
> Tracing this led to allow_freesync remaining true for a VRR-capable
> sink even when the FreeSync state was inactive. That allowed FPO to
> stretch vblank around UCLK switches. The PCON whitelist change exposed
> this on our setup by making the sink eligible for that path.
> 
> The intent is to keep the frame period fixed when userspace has
> disabled VRR, while retaining FPO eligibility when variable refresh is
> actually requested. With the patch, the original setup holds the
> requested 40fps again. I also tested VRR toggling on a Legion Go 2's
> internal eDP OLED panel and did not observe a regression.
> 
> Thanks,
> Matt


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc
       [not found]       ` <a2a345da-a199-48f8-8461-ebd298aa252a@linux.dev>
@ 2026-09-22 18:10         ` Leo Li
  0 siblings, 0 replies; 6+ messages in thread
From: Leo Li @ 2026-09-22 18:10 UTC (permalink / raw)
  To: Matthew Schwartz, George Zhang, amd-gfx; +Cc: Fangzhi Zuo



On 2026-09-22 14:04, Matthew Schwartz wrote:
> On 9/22/26 10:50 AM, Leo Li wrote:
>>
>>
>> On 2026-09-21 15:47, Matthew Schwartz wrote:
>>> Hi George,
>>>
>>> After updating to Linux 7.2.x, I noticed gamescope's frame limiter falling
>>> below the requested rate. The reproducer was a Navi 33 driving 4K120
>>> through a DP-HDMI PCON, with VRR disabled. A 40fps limit was producing
>>> roughly 37fps.
>>>
>>> Gamescope was pacing against the fixed 120Hz refresh interval, but we
>>> observed frame intervals stretching to around 9.34ms instead of 8.33ms.
>>> The OTG vtotal also increased from 2249 to 2521. This was enough to
>>> disrupt the limiter's pacing despite userspace leaving VRR_ENABLED=0.
>>
>> Hi Matt,
>>
>> I wonder if this has to do with re-introducing 2-frame vblank off for NV3+
>> DGPUs, which made it into 7.2:
>>
>> fba211b078d6 ("Revert "drm/amd/display: Restore 5s vbl offdelay for NV3x+ DGPUs"")
>>
>> Since gamescope is only updating every 3rd frame, it's possible vblanks were
>> turned off during those two frames, signaling driver to enable idle optimizations.
>>
>> To test this idea, does bumping this line to something like 5 frames ((u64)50 *) help?
>> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L3377
> 
> Tested this by reverting my own patch in the kernel and replacing it with this, and unfortunately the issue
> still reproduces here. In Hades 2, a 40FPS limit is locking to 37FPS erroneously.

Interesting... Maybe try reverting fba211b078d6 completely as well.

If that doesn't help, could you try isolating FPO by force disabling it (without applying this fix, of course)?
Set `disable_fpo_optimizations = true` in both dcn32_resource.c and dcn321_resource.c

- Leo

> 
> Matt
> 
>>
>> Thanks,
>> Leo 
>>>
>>> Tracing this led to allow_freesync remaining true for a VRR-capable
>>> sink even when the FreeSync state was inactive. That allowed FPO to
>>> stretch vblank around UCLK switches. The PCON whitelist change exposed
>>> this on our setup by making the sink eligible for that path.
>>>
>>> The intent is to keep the frame period fixed when userspace has
>>> disabled VRR, while retaining FPO eligibility when variable refresh is
>>> actually requested. With the patch, the original setup holds the
>>> requested 40fps again. I also tested VRR toggling on a Legion Go 2's
>>> internal eDP OLED panel and did not observe a regression.
>>>
>>> Thanks,
>>> Matt
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-22 18:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 19:59 [PATCH] drm/amd/display: only allow freesync on a VRR_ENABLED crtc Matthew Schwartz
2026-09-21 19:41 ` George Zhang
2026-09-21 19:42 ` George Zhang
2026-09-21 19:47   ` Matthew Schwartz
2026-09-22 17:50     ` Leo Li
     [not found]       ` <a2a345da-a199-48f8-8461-ebd298aa252a@linux.dev>
2026-09-22 18:10         ` Leo Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.