Re: [PATCH v2] 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 3 potential issue(s) to consider:
- [High] Hardcoded usage of `intel_dp->attached_connector` accesses the root SST connector in shared DP code paths, breaking MST support.
- [High] Function signature mismatch in the call to `intel_hdmi_dsc_get_num_slices()` causes compilation failure.
- [High] PCON DSC passthrough fundamentally breaks downstream DSC helpers due to an unhandled all-zero `dsc_dpcd`.
--

commit 8891a63d7de717cf65ea1f4d21b654db63caa8ce
Author: Xiao Lu <xiaolu.xie@intel.com>

    drm/i915/dp: enable DSC passthrough for HDMI 2.1 PCON devices

    This patch adds explicit driver control of PCON DSC passthrough.
    It derives DSC parameters from the downstream HDMI sink's EDID DSC
    capabilities rather than from the PCON's DSC encoder DPCD (which is
    all-zero for passthrough-only PCONs).

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2393,7 +2403,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 (intel_dp->attached_connector->dp.pcon_dsc_passthrough) {

[Severity: High]
Does hardcoding intel_dp->attached_connector here cause issues for MST
configurations? For MST, attached_connector points to the root SST connector,
not the active MST leaf branch connector undergoing the modeset. This might
incorrectly evaluate the pcon_dsc_passthrough flag of the root connector
instead of the active MST connector.

> +		/*
> +		 * 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;

[Severity: High]
Does PCON DSC passthrough fundamentally break downstream DSC helpers like
intel_dp_dsc_compute_params() and intel_dp_dsc_supports_format() due to the
unhandled all-zero dsc_dpcd?

The patch notes that the PCON's DSC encoder DPCD is all-zero, but fails to
update critical DSC calculation pathways to bypass the DPCD checks. For example,
intel_dp_dsc_compute_params() and intel_dp_dsc_supports_format() continue
to query connector->dp.dsc_dpcd. This causes them to calculate 0 for
properties like line_buf_depth or return false for supported formats,
causing the driver to immediately bail out with -EINVAL.

[ ... ]

> @@ -4298,6 +4360,90 @@ void intel_dp_check_frl_training(struct intel_dp *intel_dp)
> +static void
> +intel_dp_pcon_set_dsc_passthrough_cap(struct intel_dp *intel_dp)
> +{
> +	struct intel_connector *connector = intel_dp->attached_connector;

[Severity: High]
Does this unconditionally modify the root SST connector state even when invoked
for an MST branch device? Using intel_dp->attached_connector instead of the
active connector could cause state corruption for MST setups.

[ ... ]

> +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;

[Severity: High]
Will this read EDID capabilities from the wrong connector's display_info
during DSC config computation for MST endpoints? Using
intel_dp->attached_connector fetches the root connector's capabilities rather
than the downstream MST leaf.

[ ... ]

> +	/*
> +	 * 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).
> +	 */
> +	target_slices = intel_hdmi_dsc_get_num_slices(crtc_state,
> +						      hdmi_max_slices,
> +						      crtc_state->hw.adjusted_mode.hdisplay,
> +						      hdmi_max_slices,
> +						      hdmi_throughput);

[Severity: High]
Does this call to intel_hdmi_dsc_get_num_slices() fail to compile?

The function is being called with 5 arguments (passing crtc_state as the
first argument), but it expects 6 arguments, with the first argument being a
const struct drm_display_mode *mode. This would lead to a build failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906062032.857322-1-xiaolu.xie@intel.com?part=1