Re: [PATCH RFC 01/25] drm/edid: Add a query for vrr range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm: false positive VRR capability for HDMI monitors with generic EDID range limits
- [Medium] drm: off-by-one payload length checks skip valid VRR limits in HF-VSDB
--

commit e42644d1c43977633d9a0e950d2749308a75d1b1
Author: Derek Foreman <derek.foreman@collabora.com>

drm/edid: Add a query for vrr range

This commit adds parsing for minimum and maximum VRR vertical rates from the
EDID.

> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3148038747b6e..a1494028e0f5e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
[ ... ]
> @@ -6309,6 +6309,25 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc,
>  		hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES;
>  }
>  
> +static void drm_parse_vrr_info(struct drm_display_info *info, const u8 *hf_scds)
> +{
> +	struct drm_monitor_range_info *range = &info->monitor_range;
> +	const int pld_len = cea_db_payload_len(hf_scds);
> +
> +	if (pld_len < 10)
> +		return;

[Severity: Medium]
Does this overly restrictive check skip parsing the VRR minimum frequency when
the payload is exactly 9 bytes?

If the payload length is 9, which is sufficient to contain the VRR minimum
at index 9, the condition evaluates to true and triggers an early return.
The monitor could lose HDMI VRR support or fall back to incorrect limits.

> +
> +	if (!range->min_vfreq)
> +		range->min_vfreq = FIELD_GET(DRM_EDID_VRR_MIN_MASK, hf_scds[9]);

[Severity: High]
Does this zero-check prevent the driver from overriding generic VESA limits
with explicit HDMI VRR limits?

Since generic VESA limits are parsed first and populate the monitor_range
structure, this check might actively prevent the function from applying the
explicit HDMI VRR limits, sealing the false positive capability check below.

> +
> +	if (pld_len < 11)
> +		return;

[Severity: Medium]
Does this similarly skip parsing the VRR maximum frequency when the payload
is exactly 10 bytes?

If the payload length is 10, which contains the VRR maximum at index 10,
this check causes an early return and skips reading the data.

> +
> +	if (!range->max_vfreq)
> +		range->max_vfreq = (FIELD_GET(DRM_EDID_VRR_MAX_UPPER_MASK, hf_scds[9]) << 8) |
> +				    hf_scds[10];
> +}
> +
>  /* Sink Capability Data Structure */
>  static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>  				      const u8 *hf_scds)
[ ... ]
> @@ -6371,6 +6390,13 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>  		dsc_support = true;
>  	}
>  
> +	drm_parse_vrr_info(info, hf_scds);
> +
> +	if (info->monitor_range.min_vfreq > 0)
> +		hdmi->vrr_capable = true;
> +	else
> +		hdmi->vrr_capable = false;

[Severity: High]
Does this unconditionally assign HDMI VRR capability based on a field that
might be populated by generic VESA monitor range limits?

If a monitor provides generic VESA continuous frequency range limits but
does not support explicit HDMI VRR in the HF-VSDB, it might still have a
non-zero min_vfreq. This could falsely mark the monitor as HDMI VRR capable,
causing the driver to attempt VRR signaling on unsupported hardware and
leading to blank screens, flickering, or modeset failures.

> +
>  	drm_dbg_kms(connector->dev,
>  		    "[CONNECTOR:%d:%s] HF-VSDB: max TMDS clock: %d KHz, HDMI 2.1 support: %s, DSC 1.2 support: %s\n",
>  		    connector->base.id, connector->name,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-vrr-limiter-uapi-v1-0-2fcd7d011646@collabora.com?part=1