Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm: drm_display_info_is_vrr_capable falsely reports VRR capability for fixed-refresh monitors - [Medium] drm: drm_display_info_is_vrr_capable fails to detect AMD FreeSync over HDMI --
commit 2bcca4d009b9b65ca1552a608f777146fe2b4086 Author: Nicolas Frattaroli <[email protected]> drm/connector: Add drm_display_info_is_vrr_capable This commit adds a function to return whether a sink, as described by a drm_display_info struct, is capable of receiving VRR signals. It checks the HDMI specific vrr_capable field or evaluates whether the monitor range's minimum frequency is above 0. > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index d94c86bfed866..fc7d9fcf9d9f6 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c [ ... ] > @@ -1378,6 +1378,25 @@ int drm_display_info_set_bus_formats(struct > drm_display_info *info, > +/** > + * drm_display_info_is_vrr_capable - Check whether sink is VRR capable > + * @info: pointer to a &struct drm_display_info representing the sink > + * > + * Check whether the sink supports variable refresh rate signals. > + * > + * Returns: > + * - true if variable refresh rate is supported by the sink > + * - false otherwise > + */ > +bool drm_display_info_is_vrr_capable(const struct drm_display_info *info) > +{ > + if (info->is_hdmi) > + return info->hdmi.vrr_capable; [Severity: Medium] Does this miss AMD FreeSync capabilities for older HDMI monitors? For HDMI displays, drm_display_info_is_vrr_capable unconditionally returns info->hdmi.vrr_capable. This field is exclusively set to true when parsing an HDMI Forum Sink Capability Data Structure (HF-VSDB). Older monitors use an AMD Vendor Specific Data Block instead, which the DRM core parses into info->amd_vsdb.freesync_supported. By ignoring info->amd_vsdb.freesync_supported, will this produce false negatives and fail to enable VRR for older FreeSync HDMI monitors? > + > + return info->monitor_range.min_vfreq > 0; > +} [Severity: High] Will this falsely report VRR capability for fixed-refresh monitors? The min_vfreq field is unconditionally populated by drm_get_monitor_range() from the standard EDID Monitor Range Limits descriptor (Tag 0xFD) for any monitor with the DRM_EDID_FEATURE_CONTINUOUS_FREQ flag set. Most modern standard monitors use this descriptor to declare safe sync bounds, not VRR capability. As a result, will standard fixed-refresh monitors be incorrectly reported as VRR-capable to userspace, potentially causing sync loss, screen blanking, or visual glitches if VRR is enabled? Additionally, a prior commit in this same series modifies drm_parse_hdmi_forum_scds() in drm_edid.c to set hdmi->vrr_capable using this identical min_vfreq > 0 check. Does that earlier logic also break HDMI VRR detection in the same way, since min_vfreq is already populated by drm_get_monitor_range()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
