Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] `intel_dp_check_frl_training` improperly evaluates source control mode support, potentially initiating FRL training and writing PCON registers on non-PCON or non-HDMI branch devices. -- commit 4303b79dbb0d5f0f36551e50e709e33e49cee039 Author: Alexander Kaplan <[email protected]> drm/dp: Read the PCON max FRL bandwidth only for HDMI DFPs This commit fixes an issue where the PCON max FRL bandwidth was read without checking the DFP type, branch presence, or whether detailed capability info was available. By properly scoping the parsing, it prevents incorrect mode pruning on adapters that replicate the DFP type byte across the descriptor. > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c > b/drivers/gpu/drm/display/drm_dp_helper.c > index 9c31e14cc413b..3328909c8db47 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -3686,6 +3686,15 @@ int drm_dp_get_pcon_max_frl_bw(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > int bw; > u8 buf; > > + if (!drm_dp_is_branch(dpcd)) > + return 0; > + > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE) > == 0) > + return 0; > + > + if ((port_cap[0] & DP_DS_PORT_TYPE_MASK) != DP_DS_PORT_TYPE_HDMI) > + return 0; > + [Severity: High] This is a pre-existing issue, but seeing the capability checks added here raises a question about similar logic in the i915 driver. In drivers/gpu/drm/i915/display/intel_dp.c, the intel_dp_check_frl_training() function appears to blindly check downstream_ports[2] without verifying if detailed capabilities are available: drivers/gpu/drm/i915/display/intel_dp.c:intel_dp_check_frl_training() { ... if (!(intel_dp->downstream_ports[2] & DP_PCON_SOURCE_CTL_MODE) || !intel_dp_is_hdmi_2_1_sink(intel_dp) || intel_dp->frl.is_trained) return; ... } If detailed capability info is not available, downstream_ports contains 1-byte descriptors per port. Doesn't this mean downstream_ports[2] would represent the third downstream port rather than byte 2 of Port 0? If Port 2 happens to have bit 5 set, could this cause the driver to incorrectly assume it's a PCON supporting source control mode, triggering FRL training logic on non-PCON or non-HDMI branch devices? Even if the training gracefully aborts due to max_pcon_frl_bw being 0, it looks like the error path unconditionally calls intel_dp_pcon_set_tmds_mode(), which writes to DP_PCON_HDMI_LINK_CONFIG_1. Could writing to these registers on generic MST hubs or docks cause hardware hangs or link training failures? Should intel_dp_check_frl_training() be updated with similar checks to ensure we only evaluate DP_PCON_SOURCE_CTL_MODE when appropriate? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
