Re: [PATCH v2 4/4] drm/amd/display: Add HDMI ALLM support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Enabling ALLM replaces the HDMI 1.4 VSIF with the HF-VSIF, dropping `hdmi_vic` signaling required for legacy 4K formats.

Pre-existing issues:
- [High] HDMI VRR (VTEM) is artificially restricted to FRL links, breaking HDMI VRR over TMDS.
--

--- Patch [4]: [PATCH v2 4/4] drm/amd/display: Add HDMI ALLM support ---
commit 4ebe402fb25e863740386dee88638b0ecc01f7ba
Author: Fangzhi Zuo <jerry.zuo@amd.com>

drm/amd/display: Add HDMI ALLM support

This commit adds HDMI 2.1 Auto Low-Latency Mode (ALLM) support by setting
ALLM_Mode in the HF-VSIF. It builds the HF-VSIF when the sink advertises ALLM
and the content type is Game or Gaming-VRR is active.

[ ... ]

> 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 df39bc70ec6c3..6cfd7526ab56a 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
> @@ -1572,8 +1572,22 @@ create_stream_for_sink(struct drm_connector *connector,
>  	update_stream_signal(stream, sink);
>  
>  	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A ||
> -	    stream->signal == SIGNAL_TYPE_HDMI_FRL)
> -		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket, false, false);
> +	    stream->signal == SIGNAL_TYPE_HDMI_FRL) {
> +		/*
> +		 * Enable HDMI ALLM (Auto Low-Latency Mode) when the sink
> +		 * advertises ALLM in the SCDS and the content type is Game.
> +		 * Setting content-type = Game is how userspace requests the
> +		 * Sink's low-latency mode (HDMI GCTS HF1-56).
> +		 */
> +		bool allm = connector->display_info.hdmi.allm &&
> +			stream->content_type == DISPLAY_CONTENT_TYPE_GAME;
> +
> +		drm_dbg_driver(dev,
> +			       "ALLM: set mode: sink_allm=%d content_type=%d -> ALLM_Mode=%d\n",
> +			    connector->display_info.hdmi.allm, stream->content_type, allm);
> +
> +		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket, allm, allm);

[Severity: High]
Does enabling ALLM replace the HDMI 1.4 VSIF with the HF-VSIF, dropping
hdmi_vic signaling for legacy 4K formats?

When ALLM is requested, calling mod_build_hf_vsif_infopacket() with allm=true
overrides the VSIF payload in vsp_infopacket.

Looking at the implementation in info_packet.c:mod_build_hf_vsif_infopacket():

if (!bALLM) {
        info_packet->sb[1] = 0x03;
        ...
} else {
        info_packet->sb[1] = 0xD8;
        ...

This acts as a mutually exclusive switch rather than populating two separate
info frames. If a user selects a display mode requiring hdmi_vic and sets the
content type to Game, will this cause the display to lose format signaling and
result in a black screen or corrupted display?

[ ... ]

> 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 cb49184813dee..c4bc6e717df93 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
> @@ -252,6 +252,37 @@ void amdgpu_dm_update_freesync_state_on_stream(
>  	new_stream->vrr_infopacket = vrr_infopacket;

[Severity: High]
This is a pre-existing issue, but does restricting VTEM generation strictly to
FRL links break HDMI VRR on HDMI 2.0 displays?

HDMI 2.1 VRR requires the VTEM packet to operate. Looking earlier in
amdgpu_dm_update_freesync_state_on_stream():

if (new_stream->sink->sink_signal == SIGNAL_TYPE_HDMI_FRL)
        mod_build_infopacket_vtem(new_stream, &vrr_params, 0, &vrr_infopacket);

By checking for SIGNAL_TYPE_HDMI_FRL, it appears the driver will silently
skip VTEM generation for TMDS-only sinks that support VRR (via firmware
updates or HDMI 2.1 features on TMDS), causing VRR to fail to engage.

>  	new_stream->allow_freesync = mod_freesync_get_freesync_enabled(&vrr_params);
>  
> +	/*
> +	 * HDMI ALLM: transmit the HF-VSIF with ALLM_Mode=1 when the sink
> +	 * advertises ALLM in the SCDS and either the content type is Game
> +	 * (HF1-56) or Gaming-VRR is active (VRR_EN=1, HF1-58 step 8.3.1).
> +	 * HDMI 2.1 7.6.6.
> +	 */
> +	if (new_stream->signal == SIGNAL_TYPE_HDMI_TYPE_A ||
> +	    new_stream->signal == SIGNAL_TYPE_HDMI_FRL) {
> +		struct dc_info_packet vsp_infopacket = {0};
> +		bool sink_allm = aconn && aconn->base.display_info.hdmi.allm;
> +		bool allm = sink_allm &&
> +			(new_stream->content_type == DISPLAY_CONTENT_TYPE_GAME ||
> +			 vrr_params.state == VRR_STATE_ACTIVE_VARIABLE ||
> +			 vrr_params.state == VRR_STATE_ACTIVE_FIXED);
> +		bool allm_changed;
> +
> +		mod_build_hf_vsif_infopacket(new_stream, &vsp_infopacket, allm, allm);
> +
> +		allm_changed = memcmp(&new_stream->vsp_infopacket, &vsp_infopacket,
> +				      sizeof(vsp_infopacket)) != 0;
> +		new_crtc_state->freesync_vrr_info_changed |= allm_changed;
> +		new_stream->vsp_infopacket = vsp_infopacket;

[Severity: High]
As noted above in create_stream_for_sink(), does unconditionally replacing
vsp_infopacket with the HF-VSIF here cause the display to lose hdmi_vic
signaling when Gaming-VRR is active?

> +
> +		if (allm_changed)
> +			drm_dbg_driver(adev_to_drm(adev),
> +				       "ALLM: flip on crtc=%u: sink_allm=%d content_type=%d vrr_state=%d -> ALLM_Mode=%d\n",
> +				    new_crtc_state->base.crtc->base.id,
> +				    sink_allm,
> +				    new_stream->content_type, vrr_params.state, allm);
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806205449.16806-1-jerry.zuo@amd.com?part=4