Re: [PATCH v2 01/21] drm/i915/dp_mst: Align TUs to avoid splitting symbols across MTPs

Ville Syrjälä
2024-05-22T18:22:56Z

On Mon, May 20, 2024 at 09:57:59PM +0300, Imre Deak wrote:
> Symbols consisting of multiple (4) TU timeslots may get split across
> MTPs when using 2 or 1 link lanes. Avoid this, as required by Bspec by
> aligning the allocated TUs to 2 when using 2 lanes and 4 when using 1
> lane.
>
> Atm, we also have to align the PBNs used to allocate BW along the MST
> path, since DRM core keeps track of its own TU value, derived from the
> PBN and that TU value must match what the driver calculates.
>
> On some platforms the alignment is only required on 8b/10b links, a
> follow-up patch will remove the limitation for those.
>
> Bspec: 49266, 68922
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 27 ++++++++++++++++++---
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index c772ba19c5477..c9c5d235744ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -207,6 +207,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
> int remote_bw_overhead;
> int link_bpp_x16;
> int remote_tu;
> + fixed20_12 pbn;
>
> drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp);
>
> @@ -237,11 +238,29 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
> * crtc_state->dp_m_n.tu), provided that the driver doesn't
> * enable SSC on the corresponding link.
> */
> - crtc_state->pbn = intel_dp_mst_calc_pbn(adjusted_mode->crtc_clock,
> - link_bpp_x16,
> - remote_bw_overhead);
> + pbn.full = dfixed_const(intel_dp_mst_calc_pbn(adjusted_mode->crtc_clock,
> + link_bpp_x16,
> + remote_bw_overhead));
> + remote_tu = DIV_ROUND_UP(pbn.full, mst_state->pbn_div.full);
>
> - remote_tu = DIV_ROUND_UP(dfixed_const(crtc_state->pbn), mst_state->pbn_div.full);
> + /*
> + * Aligning the TUs ensures that symbols consisting of multiple
> + * (4) symbol cycles don't get split between two consecutive
> + * MTPs, as required by Bspec.
> + * TODO: remove the alignment restriction for 128b/132b links
> + * on some platforms, where Bspec allows this.
> + */
> + remote_tu = ALIGN(remote_tu, 4 / crtc_state->lane_count);
> +
> + /*
> + * Also align PBNs accordingly, since MST core will derive its
> + * own copy of TU from the PBN in drm_dp_atomic_find_time_slots().
> + * The above comment about the difference between the PBN
> + * allocated for the whole path and the TUs allocated for the
> + * first branch device's link also applies here.
> + */
> + pbn.full = remote_tu * mst_state->pbn_div.full;
> + crtc_state->pbn = dfixed_trunc(pbn);
>
> drm_WARN_ON(&i915->drm, remote_tu < crtc_state->dp_m_n.tu);
> crtc_state->dp_m_n.tu = remote_tu;
> --
> 2.43.3

--
Ville Syrjälä
Intel