Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] The HF-VSDB VRR fallback is negated for HDMI TMDS connections due to a pre-existing AMD FreeSync MCCS check resetting `freesync_capable` to `false`. Pre-existing issues: - [High] The highest refresh rate mode cache (`freesync_vid_base`) is never invalidated across hotplugs, corrupting `max_vfreq` calculations for subsequently connected displays. -- --- Patch [3]: [PATCH v4 3/4] drm/amd/display: Add HDMI 2.1 VRR support from HF-VSDB --- commit cf4d7fc4ac615fcafd91467c91d4c4582773cd6b Author: Fangzhi Zuo <[email protected]> drm/amd/display: Add HDMI 2.1 VRR support from HF-VSDB This commit updates the FreeSync capabilities logic to fall back to the HDMI 2.1 VRR range parsed from the HF-VSDB when the AMD VSDB does not provide a valid range. > 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 9eaa20ec528ec..2fe7bfb1e88f7 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 [ ... ] > @@ -3856,6 +3865,59 @@ void amdgpu_dm_update_freesync_caps(struct > drm_connector *connector, > connector->display_info.monitor_range.max_vfreq > = vsdb_info.max_refresh_rate_hz; > } > } > + > + drm_dbg_driver(adev_to_drm(adev), > + "VRR: amd_vsdb i=%d fs_sup=%d min=%d max=%d > fs_capable=%d\n", > + i, vsdb_info.freesync_supported, > + vsdb_info.min_refresh_rate_hz, > + vsdb_info.max_refresh_rate_hz, freesync_capable); > + > + /* > + * 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) { > + struct drm_hdmi_vrr_cap *vrr_cap = > + &connector->display_info.hdmi.vrr_cap; > + > + drm_dbg_driver(adev_to_drm(adev), > + "VRR: HF-VSDB fallback: hdmi_vrr=1 > vrr_cap[sup=%d min=%d max=%d] mrange_max=%d\n", > + vrr_cap->supported, vrr_cap->vrr_min, > vrr_cap->vrr_max, > + > connector->display_info.monitor_range.max_vfreq); > + > + if (vrr_cap->supported && vrr_cap->vrr_min > 0) { > + amdgpu_dm_connector->min_vfreq = > vrr_cap->vrr_min; > + amdgpu_dm_connector->max_vfreq = > vrr_cap->vrr_max ? > + vrr_cap->vrr_max : > + > connector->display_info.monitor_range.max_vfreq; > + > + /* > + * VRRMAX = 0 in the HF-VSDB means "up to the > Base > + * Refresh Rate". If the EDID also did not > provide a > + * monitor range max, fall back to the Base > Refresh > + * Rate (the highest refresh rate of the > preferred > + * timing) so a valid VRR range is still > reported to > + * userspace. > + */ > + if (!amdgpu_dm_connector->max_vfreq) { > + struct drm_display_mode *brr_mode = > + > amdgpu_dm_get_highest_refresh_rate_mode(amdgpu_dm_connector, true); [Severity: High] This is a pre-existing issue, but does amdgpu_dm_get_highest_refresh_rate_mode() return stale data across hotplugs? It appears aconnector->freesync_vid_base is never cleared on hotplug: amdgpu_dm_get_highest_refresh_rate_mode() { ... if (aconnector->freesync_vid_base.clock != 0) return &aconnector->freesync_vid_base; ... } If a high-refresh monitor is unplugged and an HDMI 2.1 VRR monitor with VRRMAX = 0 is plugged into the same port, will this use the cached high-refresh mode of the disconnected monitor and calculate an incorrect max_vfreq? > + > + if (brr_mode) > + amdgpu_dm_connector->max_vfreq = > + > drm_mode_vrefresh(brr_mode); > + } > + > + if (amdgpu_dm_connector->max_vfreq - > + amdgpu_dm_connector->min_vfreq > 10) > + freesync_capable = true; [Severity: High] Does this new fallback get negated later in amdgpu_dm_update_freesync_caps() for HDMI TMDS connections? When an HDMI monitor without an AMD VSDB is connected, freesync_vcp_code remains 0. Later in this same function, the pre-existing MCCS check does this: 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->edid_caps.freesync_vcp_code evaluates to true for TMDS, could this prevent the HF-VSDB fallback from working for these displays? > + > + 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
