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 <[email protected]> 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/[email protected]?part=1
