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,
