Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] PCON DSC passthrough is enabled without verifying the Intel display source supports DSC 1.2, forcing an incompatible bitstream on older platforms. - [High] DSC modes are incorrectly rejected for passthrough PCONs during mode validation due to reliance on zeroed DPCD capabilities. - [High] Data race on bitfield resulting in lost updates for connector->dp.dsc_decompression_enabled and pcon_dsc_passthrough. --
commit f0cec5b3c63643b719cd4fdc47fea560e78d2d25 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. > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 5f0fe18c0614e..90d77aa4d2346 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -564,6 +564,7 @@ struct intel_connector { > u8 dsc_hblank_expansion_quirk:1; > u8 dsc_throughput_quirk:1; > u8 dsc_decompression_enabled:1; > + u8 pcon_dsc_passthrough:1; /* PCON DSC PT supported and HDMI > sink DSC 1.2 */ [Severity: High] Could this introduce a data race on the bitfield, resulting in lost updates for connector->dp.dsc_decompression_enabled and pcon_dsc_passthrough? Concurrent read-modify-write operations can occur when hotplug/probe (where intel_dp_pcon_set_dsc_passthrough_cap() executes) races with asynchronous modeset commit tail operations that modify adjacent bits like dsc_decompression_enabled. > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 0cd5e6b5034cf..fb81cbfcdff6d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c [ ... ] > @@ -2524,10 +2563,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] Will this evaluate to 0 because the PCON's dsc_dpcd is zeroed? For a passthrough-only PCON, its decoder dsc_dpcd is zeroed. As a result, drm_dp_dsc_sink_max_slice_count() will return 0, which makes dsc_slice_count 0 and guarantees the mode will be rejected downstream. Additionally, during mode validation, intel_dp_mode_valid_format() calls intel_dp_dsc_compute_max_bpp(), which relies on align_max_sink_dsc_input_bpp(). Since the DPCD capabilities are zeroed, this also returns 0, causing modes requiring DSC to be incorrectly rejected. [ ... ] > @@ -4298,6 +4387,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; > + > + if (!intel_dp_is_hdmi_2_1_sink(intel_dp)) > + return; > + > + if (!(connector->dp.dsc_dpcd[0] & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) > + return; > + > + info = &connector->base.display_info; > + connector->dp.pcon_dsc_passthrough = info->hdmi.dsc_cap.v_1p2; [Severity: High] Does this enable passthrough without verifying the Intel display source supports DSC 1.2? If the source is older (e.g., pre-DISPLAY_VER 14 like Tiger Lake), it only supports DSC 1.1. Later in intel_dp_dsc_compute_params(), the VDSC engine will be configured for DSC 1.1: vdsc_cfg->dsc_version_minor = min(intel_dp_source_dsc_version_minor(display), 2); Could this result in sending a DSC 1.1 bitstream to an HDMI 2.1 sink that mandates DSC 1.2a, leading to corrupted output or display failures? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
