Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Bypassing the DP branch device check for MST streams enables VRR and AS-SDP on MST leaf ports that are PCONs, which is explicitly unsupported. - [Low] Triply duplicated comment block explaining the reuse of the vrr_capable_property. --
commit 0a351efb0934b17f15e6de16fd0583d3287db2d9 Author: Xiao Lu <[email protected]> Subject: drm/i915/display: Enable VRR/HDR over MST This commit enables Variable Refresh Rate (VRR) and HDR metadata transmission over DisplayPort Multi-Stream Transport (MST). It introduces caching of the MSA timing ignore capability to avoid hot path sideband reads. It also reuses SST properties for MST streams and lifts previous pipe_bpp caps. > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 113d767e62e9c..4b196d26d27f8 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -3107,9 +3110,12 @@ static bool intel_dp_needs_as_sdp(struct intel_dp > *intel_dp, > return false; > > /* > - * #TODO: Add AS SDP v1 support for PCONs (DP branch devices). > + * AS-SDP is not yet supported for PCONs (DP-to-HDMI converters). > + * MST hubs are also branch devices but do support AS-SDP forwarding > + * to downstream sinks; allow it for MST streams. > */ > - if (drm_dp_is_branch(intel_dp->dpcd)) > + if (drm_dp_is_branch(intel_dp->dpcd) && > + !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > return false; [Severity: Medium] Does this explicit exemption of MST streams enable AS-SDP packets even if the downstream MST leaf port is an unsupported PCON? Because MST streams are exempted from the branch device block here, AS-SDP packets will be sent to MST streams even when the leaf port is a PCON that doesn't fully support it. > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 3be1643f8d030..021d86b0ed833 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -1745,6 +1799,34 @@ static int > mst_topology_add_connector_properties(struct intel_dp *intel_dp, > if (connector->base.max_bpc_property) > drm_connector_attach_max_bpc_property(&connector->base, 6, 12); > > + /* > + * Reuse the vrr_capable prop from the eDP connector (which is always > + * initialized during driver load before device registration), because > + * we're not allowed to create new props after device registration. > + */ > + /* > + * Reuse the vrr_capable property from the root intel_dp SST connector > + * which was created during driver load (before device registration). > + * MST connectors are created after device registration, so we cannot > + * create new properties here - we must attach the existing one. > + */ > + /* > + * Reuse the vrr_capable property from the root intel_dp SST connector > + * which was created during driver load (before device registration). > + * MST connectors are created after device registration, so we cannot > + * create new properties here - we must attach the existing one. > + * Also assign connector->vrr_capable_property so that > + * drm_connector_set_vrr_capable_property() can update the value. > + */ [Severity: Low] This isn't a bug, but it looks like the comment block explaining the reuse of the vrr_capable_property was duplicated three times. > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > b/drivers/gpu/drm/i915/display/intel_vrr.c > index 51e4f3309b8b1..9737a19c1899f 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -55,8 +55,19 @@ bool intel_vrr_is_capable(struct intel_connector > *connector) > return false; > fallthrough; > case DRM_MODE_CONNECTOR_DisplayPort: > - if (connector->mst.dp) > - return false; > + if (connector->mst.dp) { > + /* > + * Use cached MSA timing ignore capability from the DFP > + * sink's virtual DPCD, set during connector init in > + * intel_dp_mst_read_decompression_port_dsc_caps(). > + * Avoid live sideband reads here as this function is > + * called from the atomic check path. > + */ > + if (!connector->dp.mst_msa_timing_par_ignore) > + return false; > + > + break; > + } [Severity: Medium] Will this early break for MST streams incorrectly bypass the branch device check for PCONs? By breaking out early for MST streams here, the function skips the subsequent drm_dp_is_branch() check. This means MST leaf ports that are PCONs are treated as direct VRR-capable sinks if they advertise MSA timing ignore in their virtual DPCD, which is explicitly unsupported. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
