Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Incorrect connector reference in MST DSC slice configuration causes unconditional failure. - [Medium] PCON hardware maximum slice limit is ignored during passthrough configuration. --
commit 76a629f18177f4364fe642d6c3207fc584a095ea Author: Xiao Lu <[email protected]> drm/i915/dp: enable DSC passthrough for HDMI 2.1 PCON devices HDMI 2.1 PCON devices (e.g. Parade PS196) that do not have an internal DSC encoder can still forward a DSC-compressed stream from the DP source to the downstream HDMI 2.1 sink unchanged (passthrough mode). The HDMI sink (e.g. Lenovo ThinkVision Pro 27UD-10) then decompresses the stream itself. This mode is signalled by DP_DSC_PASSTHROUGH_IS_SUPPORTED (DPCD 0x060 bit1) on the PCON. > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 0cd5e6b5034cf..e0e90219a150d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c [ ... ] > @@ -2393,7 +2424,19 @@ int intel_dp_dsc_compute_config(struct intel_dp > *intel_dp, > } > } > > - if (!intel_dp_dsc_get_slice_config(connector, adjusted_mode->crtc_clock, > + if (connector->dp.pcon_dsc_passthrough) { > + /* > + * PCON passthrough: no PCON encoder DPCD, derive slice config > + * from the downstream HDMI 2.1 sink DSC capabilities in EDID. > + */ > + if (!intel_dp_pcon_passthrough_dsc_slice_config(intel_dp, > + pipe_config, > + > &pipe_config->dsc.slice_config)) { > + drm_dbg_kms(display->drm, > + "PCON DSC passthrough: no valid slice > config\n"); > + return -EINVAL; > + } > + } else if (!intel_dp_dsc_get_slice_config(connector, > adjusted_mode->crtc_clock, > adjusted_mode->crtc_hdisplay, > num_joined_pipes, > &pipe_config->dsc.slice_config)) > return -EINVAL; [ ... ] > +static bool > +intel_dp_pcon_passthrough_dsc_slice_config(struct intel_dp *intel_dp, > + const struct intel_crtc_state > *crtc_state, > + struct intel_dsc_slice_config > *config_ret) > +{ > + struct intel_display *display = to_intel_display(intel_dp); > + struct intel_connector *connector = intel_dp->attached_connector; > + const struct drm_display_info *info = &connector->base.display_info; > + int num_joined_pipes = intel_crtc_num_joined_pipes(crtc_state); > + int hdmi_throughput = info->hdmi.dsc_cap.clk_per_slice; > + int hdmi_max_slices = info->hdmi.dsc_cap.max_slices; > + int target_slices; > + int slices_per_pipe; [Severity: High] Does this code correctly handle MST topologies? The caller intel_dp_dsc_compute_config() already has the correct connector available, but does not pass it down. Instead, intel_dp_pcon_passthrough_dsc_slice_config() derives it via intel_dp->attached_connector. For an MST topology, this points to the physical root connector (the hub), not the virtual MST connector corresponding to the display. The root connector's display_info lacks the downstream HDMI sink's DSC capabilities from EDID, which causes info->hdmi.dsc_cap.max_slices and clk_per_slice to be zero, leading intel_hdmi_dsc_get_num_slices() to fail unconditionally. Should intel_dp_pcon_passthrough_dsc_slice_config() accept an explicit struct intel_connector * parameter from its caller to ensure MST streams use the correct EDID capabilities? > + > + /* > + * Derive the required slice count from the downstream HDMI 2.1 sink > + * DSC capabilities in EDID (not from the PCON encoder DPCD which is > + * all-zero for passthrough-only devices). > + */ > + /* > + * src_max_slice_width: pass INT_MAX so the HDMI spec limit of 2720px > + * applies; the source VDSC HW limit (5120px) is higher so it is not > + * the binding constraint here. > + * src_max_slices: use hdmi_max_slices as the binding upper limit since > + * the PCON passthrough capability bit (already checked) is the gate; > + * the for-loop below further constrains to what the Intel VDSC HW can > + * actually produce. > + */ > + target_slices = > intel_hdmi_dsc_get_num_slices(&crtc_state->hw.adjusted_mode, > + crtc_state->output_format, > + hdmi_max_slices, INT_MAX, > + hdmi_max_slices, > + hdmi_throughput); [Severity: Medium] Does this configuration respect the PCON adapter's passthrough limits? While intel_dp_mode_valid_with_dsc() properly caps the validated slice count using the PCON's own passthrough limit from dsc_dpcd, this function directly passes hdmi_max_slices as the src_max_slices parameter to intel_hdmi_dsc_get_num_slices(). If the HDMI 2.1 sink supports a higher maximum DSC slice count than the intermediate PCON adapter, the source VDSC engine could be configured with a slice count that the PCON cannot forward. Should this be bounded by the PCON's DPCD slice limit to ensure the source is not programmed beyond the adapter's capabilities? > + if (!target_slices) > + return false; > + -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
