Re: [PATCH v3 1/2] drm/i915/dp: fix PBN in ALLOCATE_PAYLOAD request to use actual video bandwidth

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/dp: DP MST VCPI timeslot mismatch and WARN_ON trigger
--

commit 2b87b995c0537aad3132b5f2622d64dea6c8fb7c
Author: Xiao Lu <xiaolu.xie@intel.com>

drm/i915/dp: fix PBN in ALLOCATE_PAYLOAD request to use actual video bandwidth
    
This commit updates intel_dp_mtp_tu_compute_config() to separate the raw
video bandwidth PBN from the aligned hardware TU PBN. The raw PBN is now
passed to drm_dp_atomic_find_time_slots() to fix payload allocation
rejections.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 6a869d0f6ffc0..4d088282df781 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c

[ ... ]

> @@ -397,7 +398,21 @@ int intel_dp_mtp_tu_compute_config(struct intel_dp *intel_dp,
>  
>  			slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst.mgr,
>  							      connector->mst.port,
> -							      dfixed_trunc(pbn));
> +							      raw_pbn);

[Severity: High]
Does this cause a mismatch in the programmed VCPI payload size?

By passing the unaligned raw_pbn here, the DRM core calculates an unaligned
VCPI timeslot allocation that is programmed into the downstream branch
device. 

Since the i915 hardware transmitter still expects the lane-count aligned
remote_tu, the source hardware and branch device will now have mismatched
payload sizes. Can this mismatch break DP MST frame synchronization and
cause data drops or display corruption?

Additionally, this mismatch appears to deterministically trigger an existing
validation warning later in intel_dp_mtp_tu_compute_config():

	if (slots >= 0) {
		drm_WARN_ON(display->drm, slots != crtc_state->dp_m_n.tu);
		break;
	}

Will this unconditionally fire during modesets where the required timeslots
are not a multiple of the lane count?

> +
> +			drm_dbg_kms(display->drm,
> +				    "MST TU %s pipe %c: clock=%d lanes=%d bpp=%d link_bpp=%d fec=%d\n",
> +				    connector->base.name,
> +				    pipe_name(to_intel_crtc(crtc_state->uapi.crtc)->pipe),
> +				    crtc_state->port_clock, crtc_state->lane_count,
> +				    fxp_q4_to_int(bpp_x16), fxp_q4_to_int(link_bpp_x16),
> +				    crtc_state->fec_enable);
> +			drm_dbg_kms(display->drm,
> +				    "MST TU %s: overhead=%d raw_pbn=%d pbn=%d pbn_div=%d tu=%d slots=%d full_pbn=%d\n",
> +				    connector->base.name,
> +				    remote_bw_overhead, raw_pbn,
> +				    dfixed_trunc(pbn), dfixed_trunc(mst_state->pbn_div),
> +				    remote_tu, slots, connector->mst.port->full_pbn);
>  
>  			/* TODO: Check this already in drm_dp_atomic_find_time_slots(). */
>  			if (slots > mst_state->total_avail_slots)

[Severity: High]
Does this limit check accurately reflect the required slots?

This check now validates the unaligned slots. Because the source hardware
uses remote_tu (which is aligned and thus potentially larger), could the
hardware's aligned timeslots exceed the physical 64-slot limit even if
the unaligned slots pass this check?

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