Re: [PATCH 1/4] drm/amd/display: Add 2.1 FreeSync support for AMD VSDB EDID Block

On 2026-07-30 13:17, Fangzhi Zuo wrote:
> From: Fangzhi Zuo <[email protected]>
> 
> why:
> HDMI FRL sinks were not parsed for the AMD VSDB and no VTEM info packet
> was emitted for them, so 2.1 FreeSync over HDMI FRL did not work. It is
> backward-compatible with 2.0 FreeSync.
> 
> how:
> - Accept SIGNAL_TYPE_HDMI_FRL alongside SIGNAL_TYPE_HDMI_TYPE_A when
>   parsing the AMD VSDB in amdgpu_dm_update_freesync_caps().
> - Build and send the VTEM info packet via mod_build_infopacket_vtem()
>   when the stream signal is HDMI FRL during the freesync state update.
> - Set the VTEM Data_Set_Length to 0 when no VTEM feature is enabled.
>   build_infopacket_header_vtem() hardcodes Data_Set_Length = 4, so a VTEM
>   with Data_Set_Length = 4 would be transmitted even when no VTEM feature
>   is enabled (VRR_EN = 0 and no FVA), e.g. when the sink advertises
>   VRRMIN = 0 and vrr_capable is false. This fails HDMI GCTS HF1-58 step
>   6.2.
>   The VTEM must keep being transmitted every MTW while VRR is enabled
>   (HF1-58 steps 8.1 and 8.3), so it cannot simply be suppressed per
>   frame. Instead, follow the MLDS option in HDMI 2.1 10.10.2.4: keep
>   transmitting the VTEM but set Data_Set_Length = 0 when no feature is
>   enabled. When VRR becomes active the full Data_Set_Length = 4 payload
>   with VRR_EN = 1 is sent as before.
> 
> Signed-off-by: Fangzhi Zuo <[email protected]>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   3 +
>  .../display/amdgpu_dm/amdgpu_dm_connector.c   |   4 +-
>  .../amd/display/modules/inc/mod_info_packet.h |   4 +
>  .../display/modules/info_packet/info_packet.c | 109 ++++++++++++++++++
>  4 files changed, 119 insertions(+), 1 deletion(-)
> 
> 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 5a9afc0607b2..ccf882a22a57 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3881,6 +3881,9 @@ static void update_freesync_state_on_stream(
>  		&vrr_infopacket,
>  		pack_sdp_v1_3);
>  
> +	if (new_stream->sink->sink_signal == SIGNAL_TYPE_HDMI_FRL)
> +		mod_build_infopacket_vtem(new_stream, &vrr_params, 0, &vrr_infopacket);
> +
>  	new_crtc_state->freesync_vrr_info_changed |=
>  		(memcmp(&new_crtc_state->vrr_infopacket,
>  			&vrr_infopacket,
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 5e3dfeaed76b..2deb5abae264 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -3631,7 +3631,9 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  			amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
>  		}
>  
> -	} else if (drm_edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
> +	} else if (drm_edid &&
> +		  (sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A ||
> +		   sink->sink_signal == SIGNAL_TYPE_HDMI_FRL)) {
>  		i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
>  		if (i >= 0) {
>  			amdgpu_dm_connector->vsdb_info = vsdb_info;
> diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h b/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
> index eee8206bc531..5181d889fe7f 100644
> --- a/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
> +++ b/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
> @@ -67,6 +67,10 @@ struct AS_Df_params {
>  	struct frame_duration_op decrease;
>  };
>  
> +void mod_build_infopacket_vtem(const struct dc_stream_state *stream,
> +		const struct mod_vrr_params *vrr, int fva_factor,
> +		struct dc_info_packet *infopacket);
> +
>  void mod_build_adaptive_sync_infopacket(const struct dc_stream_state *stream,
>  		enum adaptive_sync_type asType, const struct AS_Df_params *param,
>  		struct dc_info_packet *info_packet);
> diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> index f5ac4bf32a78..e956c707ac50 100644
> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> @@ -291,6 +291,21 @@ void set_vsc_packet_colorimetry_data(
>  	info_packet->sb[18] = 0;
>  }
>  
> +static void setFieldWithMask(unsigned char *dest, unsigned int mask, unsigned int value)

Don't use camel-case. We should rename it to set_field_with_mask(...).

> +{
> +	unsigned int shift = 0;
> +
> +	if (!mask || !dest)
> +		return;
> +
> +	while (!((mask >> shift) & 1))
> +		shift++;
> +
> +	*dest = *dest & ~mask;
> +	value = value & (mask >> shift);
> +	*dest = *dest | (value << shift);
> +}
> +
>  void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
>  		struct dc_info_packet *info_packet,
>  		enum dc_color_space cs,
> @@ -644,6 +659,100 @@ void mod_build_hf_vsif_infopacket(const struct dc_stream_state *stream,
>  		info_packet->valid = true;
>  }
>  
> +static void build_vtem_infopacket_data(const struct dc_stream_state *stream,
> +		const struct mod_vrr_params *vrr, int fva_factor,
> +		struct dc_info_packet *infopacket)
> +{
> +	unsigned int fieldRateInHz;

Same here: field_rate_in_hz

> +
> +	/* FVA Factor setting */
> +	setFieldWithMask(&infopacket->sb[VTEM_MD0], MASK_VTEM_MD0__FVA_FACTOR_M1,
> +			(fva_factor > 0)?(fva_factor-1):0);

Formatting. There should be whitespace around the `?`, `-`, and `:`.

> +	/* VRR Parameters */
> +	if (vrr->state == VRR_STATE_ACTIVE_VARIABLE ||
> +				vrr->state == VRR_STATE_ACTIVE_FIXED) {

Formatting. The vrr->state on the second line should align with
the vrr->state on the prior line.

> +		setFieldWithMask(&infopacket->sb[VTEM_MD0], MASK_VTEM_MD0__VRR_EN, 1);
> +	} else {
> +		setFieldWithMask(&infopacket->sb[VTEM_MD0], MASK_VTEM_MD0__VRR_EN, 0);
> +	}
> +
> +	if (vrr->state == VRR_STATE_ACTIVE_FIXED)
> +		setFieldWithMask(&infopacket->sb[VTEM_MD0], MASK_VTEM_MD0__M_CONST, vrr->m_const);
> +
> +	if (!stream->timing.vic) {
> +		setFieldWithMask(&infopacket->sb[VTEM_MD1], MASK_VTEM_MD1__BASE_VFRONT,
> +				stream->timing.v_front_porch);
> +
> +
> +		/* TODO: In dal2, we check mode flags for a reduced blanking timing.
> +		 * Need a way to relay that information to this function.
> +		 * if("ReducedBlanking")
> +		 * {
> +		 *   setFieldWithMask(&infopacket->sb[VRR_VTEM_MD2], MASK__VRR_VTEM_MD2__RB, 1;
> +		 * }
> +		 */
> +
> +		fieldRateInHz = stream->timing.pix_clk_100hz * 100;
> +		fieldRateInHz /= stream->timing.h_total;
> +		fieldRateInHz = (fieldRateInHz + stream->timing.v_total / 2)
> +						/ stream->timing.v_total;
> +
> +		setFieldWithMask(&infopacket->sb[VTEM_MD2],  MASK_VTEM_MD2__BASE_REFRESH_RATE_98,
> +				fieldRateInHz >> 8);
> +		setFieldWithMask(&infopacket->sb[VTEM_MD3], MASK_VTEM_MD3__BASE_REFRESH_RATE_07,
> +				fieldRateInHz);
> +
> +	}
> +
> +	/*
> +	 * When no VTEM feature is enabled (neither VRR nor FVA), signal a
> +	 * zero-length data set (MLDS) by clearing Data_Set_Length. HDMI 2.1
> +	 * 10.10.2.4 requires the Source to either stop transmitting the VTEM
> +	 * or set Data_Set_Length = 0 when no feature is enabled; keeping the
> +	 * VTEM with Data_Set_Length = 0 preserves the every-MTW cadence while
> +	 * staying compliant (e.g. HDMI GCTS HF1-58 step 6.2).
> +	 */
> +	if (vrr->state != VRR_STATE_ACTIVE_VARIABLE &&
> +	    vrr->state != VRR_STATE_ACTIVE_FIXED && fva_factor == 0)
> +		setFieldWithMask(&infopacket->sb[VTEM_PB6],
> +				 MASK_VTEM_PB6__DATA_SET_LENGTH_LSB, 0);
> +
> +	infopacket->valid = true;
> +}
> +
> +static void build_infopacket_header_vtem(enum signal_type signal,
> +		struct dc_info_packet *infopacket)
> +{
> +	// HEADER
> +
> +	// HB0, HB1, HB2 indicates PacketType VTEMPacket

Formatting: convert these C++-style comments to C-style.

> +	infopacket->hb0 = 0x7F;
> +	infopacket->hb1 = 0xC0;
> +	infopacket->hb2 = 0x00; //sequence_index
> +
> +	setFieldWithMask(&infopacket->sb[VTEM_PB0], MASK_VTEM_PB0__VFR, 1);
> +	setFieldWithMask(&infopacket->sb[VTEM_PB2], MASK_VTEM_PB2__ORGANIZATION_ID, 1);
> +	setFieldWithMask(&infopacket->sb[VTEM_PB3], MASK_VTEM_PB3__DATA_SET_TAG_MSB, 0);
> +	setFieldWithMask(&infopacket->sb[VTEM_PB4], MASK_VTEM_PB4__DATA_SET_TAG_LSB, 1);
> +	setFieldWithMask(&infopacket->sb[VTEM_PB5], MASK_VTEM_PB5__DATA_SET_LENGTH_MSB, 0);
> +	setFieldWithMask(&infopacket->sb[VTEM_PB6], MASK_VTEM_PB6__DATA_SET_LENGTH_LSB, 4);
> +}
> +
> +void mod_build_infopacket_vtem(const struct dc_stream_state *stream,
> +		const struct mod_vrr_params *vrr, int fva_factor,
> +		struct dc_info_packet *infopacket)
> +{
> +	//VTEM info packet for HdmiVrr
> +

C++-style comment

> +	memset(infopacket, 0, sizeof(struct dc_info_packet));
> +
> +	//VTEM Packet is structured differently

C++-style comment

Happy to have all these style comments fixed separately in an amd-dal commit
or a new asdn commit.

Either way, this is
Reviewed-by: Harry Wentland <[email protected]>

Harry

> +	build_infopacket_header_vtem(stream->signal, infopacket);
> +	build_vtem_infopacket_data(stream, vrr, fva_factor, infopacket);
> +
> +	infopacket->valid = true;
> +}
> +
>  void mod_build_adaptive_sync_infopacket(const struct dc_stream_state *stream,
>  		enum adaptive_sync_type asType,
>  		const struct AS_Df_params *param,