Re: [PATCH v6 2/2] 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] Flawed implementation of PCON DSC passthrough for MST topologies incorrectly targets the root SST connector, leading to broken MST support, data races, and lockless accesses.
- [High] Mode validation unconditionally rejects all DSC modes for passthrough-only PCONs by capping the slice count with an all-zero DPCD capability.
--

commit f1a37f1f267d3517b3c9a3c61b716d9649b3222e
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 detects
passthrough capability at connect time and uses the downstream HDMI 2.1 sink
DSC capabilities instead of the PCON's encoder constraints to derive DSC
parameters.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0cd5e6b5034cf..e23e4e8b32bf2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> @@ -2524,10 +2567,20 @@ bool intel_dp_mode_valid_with_dsc(struct intel_connector *connector,
>  							 num_joined_pipes,
>  							 output_format,
>  							 pipe_bpp, INT_MAX);
> -	int dsc_slice_count = intel_dp_dsc_get_slice_count(connector,
> -							   mode_clock,
> -							   mode_hdisplay,
> -							   num_joined_pipes);
> +	int dsc_slice_count;
> +
> +	if (connector->dp.pcon_dsc_passthrough) {
> +		const struct drm_display_info *info = &connector->base.display_info;
> +
> +		dsc_slice_count = min_t(int, info->hdmi.dsc_cap.max_slices,
> +					drm_dp_dsc_sink_max_slice_count(connector->dp.dsc_dpcd,
> +									false));

[Severity: High]
Does this unconditionally reject all modes requiring DSC for passthrough-only
PCONs?

The commit message notes that the PCON encoder DPCD is all-zero for
passthrough-only devices. When connector->dp.dsc_dpcd is all-zero,
drm_dp_dsc_sink_max_slice_count() evaluates to 0. 

As a result, dsc_slice_count becomes 0 due to the min_t() cap, and the
subsequent check in intel_dp_mode_valid_with_dsc() (if dsc_slice_count == 0)
will return false, failing mode validation.

[ ... ]
> @@ -3755,6 +3833,10 @@ intel_dp_sink_set_dsc_decompression(struct intel_connector *connector,
>  {
>  	struct intel_display *display = to_intel_display(connector);
>  
> +	/* PCON passthrough: PCON forwards the stream, sink decompresses. */
> +	if (!connector->mst.dp && connector->dp.pcon_dsc_passthrough)
> +		return;

[Severity: High]
Will this short-circuit logic fail to bypass decompression for MST leaves?

For an MST leaf connector, connector->mst.dp points to the root intel_dp,
meaning the !connector->mst.dp check evaluates to false. This appears to
contradict the commit message's note about ensuring
intel_dp_sink_set_dsc_decompression() is correctly bypassed for PCON
passthrough in MST topologies.

[ ... ]
> @@ -4298,6 +4391,116 @@ void intel_dp_check_frl_training(struct intel_dp *intel_dp)
>  	}
>  }
>  
> +/*
> + * intel_dp_pcon_set_dsc_passthrough_cap - cache PCON DSC passthrough capability
[ ... ]
> + */
> +static void
> +intel_dp_pcon_set_dsc_passthrough_cap(struct intel_dp *intel_dp)
> +{
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	const struct drm_display_info *info;
> +
> +	if (!connector)
> +		return;
> +
> +	connector->dp.pcon_dsc_passthrough = false;
> +
> +	/*
> +	 * PCON DSC passthrough is only supported for SST direct connections.
> +	 * In MST topologies the intermediate hub may report incorrect virtual
> +	 * DPCD for the downstream PCON port, leading to mismatched DSC
> +	 * parameters.  Skip passthrough for MST until hub firmware correctly
> +	 * reflects the PCON's capabilities.
> +	 */
> +	if (connector->mst.port)
> +		return;

[Severity: High]
When called during MST connector mode validation, does this locklessly mutate
the root SST connector's state instead of evaluating the MST leaf?

Since mst_connector_get_ddc_modes() passes the root intel_dp
(connector->mst.dp), intel_dp->attached_connector resolves to the root SST
connector. 

Consequently, connector->mst.port is NULL for the root connector, the early
return is skipped, and it locklessly modifies the root connector's
pcon_dsc_passthrough flag using potentially stale display_info data.

[ ... ]
> +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]
Could this result in using the wrong display_info during an MST stream's
atomic check?

If this is called for an MST stream, intel_dp is the root DP device, making
intel_dp->attached_connector the root SST connector. This causes the function
to read the root connector's display_info rather than the MST leaf's
display_info.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 6a869d0f6ffc0..2653f2347f235 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
[ ... ]
> @@ -1451,6 +1451,13 @@ static int mst_connector_get_ddc_modes(struct drm_connector *_connector)
>  
>  	ret = intel_connector_update_modes(&connector->base, drm_edid);
>  
> +	/*
> +	 * Re-evaluate PCON DSC passthrough capability now that the MST
> +	 * connector's EDID has been parsed and display_info.hdmi.dsc_cap
> +	 * is populated.
> +	 */
> +	intel_dp_pcon_set_dsc_passthrough_cap(intel_dp);

[Severity: High]
Is intel_dp pointing to the root SST port here rather than the MST leaf?

As noted above, passing the root intel_dp into
intel_dp_pcon_set_dsc_passthrough_cap() causes the capability evaluation to
incorrectly target the root connector rather than the MST stream connector.

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