Re: [PATCH v2 3/4] drm/amd/display: Add HDMI 2.1 VRR support from HF-VSDB

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The HF-VSDB VRR fallback is unconditionally overwritten to false for HDMI TMDS sinks because they lack an AMD FreeSync VCP code.
--

--- Patch [3]: [PATCH v2 3/4] drm/amd/display: Add HDMI 2.1 VRR support from HF-VSDB ---
commit 3db5053c0ea2b238965bfd80def111ee342642b8
Author: Fangzhi Zuo <[email protected]>

drm/amd/display: Add HDMI 2.1 VRR support from HF-VSDB

This commit adds support for HDMI 2.1 VRR capabilities derived from the
HDMI Forum VSDB (HF-VSDB) as a fallback. It parses the VRR ranges and sets
the FreeSync capability appropriately when an AMD VSDB is missing or invalid.

> 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
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
[ ... ]
> @@ -3856,6 +3865,59 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
[ ... ]
> +		/*
> +		 * If AMD VSDB didn't provide a valid FreeSync range, fall back to
> +		 * the HDMI 2.1 VRR capability parsed from the HF-VSDB.
> +		 */
> +		if (!freesync_capable && connector->display_info.hdmi.vrr_cap.supported) {
[ ... ]
> +				if (amdgpu_dm_connector->max_vfreq -
> +				    amdgpu_dm_connector->min_vfreq > 10)
> +					freesync_capable = true;

[Severity: Medium]
Will this newly enabled HF-VSDB fallback be unconditionally overwritten to 
false for HDMI TMDS sinks?

For a monitor advertising VRR only via HF-VSDB on an HDMI TMDS link, the
AMD VSDB parsing fails, leaving sink->edid_caps.freesync_vcp_code as 0.

While this new fallback block correctly sets freesync_capable to true, later
in amdgpu_dm_update_freesync_caps(), there is an existing check:

	if ((sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A ||
		as_type == FREESYNC_TYPE_PCON_IN_WHITELIST) &&
		(!sink->edid_caps.freesync_vcp_code ||
		(sink->edid_caps.freesync_vcp_code && !sink->mccs_caps.freesync_supported)))
		freesync_capable = false;

Since sink_signal == SIGNAL_TYPE_HDMI_TYPE_A and freesync_vcp_code is 0,
does this existing logic silently disable the fallback feature being
introduced here?

> +
> +				connector->display_info.monitor_range.min_vfreq =
> +					amdgpu_dm_connector->min_vfreq;
> +				connector->display_info.monitor_range.max_vfreq =
> +					amdgpu_dm_connector->max_vfreq;
> +			}
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3