Re: [PATCH v8] drm/i915/dp: enable DSC passthrough for HDMI 2.1 PCON devices

[sashiko-bot@kernel.org]

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 <xiaolu.xie@intel.com>

    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/20260909051308.388381-1-xiaolu.xie@intel.com?part=1