[PATCH v2 07/16] drm/i915/mst: adapt intel_dp_mtp_tu_compute_config() for 128b/132b SST

[Imre Deak]

On Thu, Dec 19, 2024 at 11:33:56PM +0200, Jani Nikula wrote:
> Handle 128b/132b SST in intel_dp_mtp_tu_compute_config(). The remote
> bandwidth overhead and time slot allocation are only relevant for MST;
> SST only needs the local bandwidth and a check that 64 slots isn't
> exceeded.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 109 +++++++++++---------
>  1 file changed, 61 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index d08824d2fefe..3fbf9163272b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -257,10 +257,7 @@ int intel_dp_mtp_tu_compute_config(struct intel_dp *intel_dp,
>  
>  	for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
>  		int local_bw_overhead;
> -		int remote_bw_overhead;
>  		int link_bpp_x16;
> -		int remote_tu;
> -		fixed20_12 pbn;
>  
>  		drm_dbg_kms(display->drm, "Trying bpp %d\n", bpp);
>  
> @@ -269,57 +266,73 @@ int intel_dp_mtp_tu_compute_config(struct intel_dp *intel_dp,
>  
>  		local_bw_overhead = intel_dp_mst_bw_overhead(crtc_state,
>  							     false, dsc_slice_count, link_bpp_x16);
> -		remote_bw_overhead = intel_dp_mst_bw_overhead(crtc_state,
> -							      true, dsc_slice_count, link_bpp_x16);
> -
>  		intel_dp_mst_compute_m_n(crtc_state,
>  					 local_bw_overhead,
>  					 link_bpp_x16,
>  					 &crtc_state->dp_m_n);
>  
> -		/*
> -		 * The TU size programmed to the HW determines which slots in
> -		 * an MTP frame are used for this stream, which needs to match
> -		 * the payload size programmed to the first downstream branch
> -		 * device's payload table.
> -		 *
> -		 * Note that atm the payload's PBN value DRM core sends via
> -		 * the ALLOCATE_PAYLOAD side-band message matches the payload
> -		 * size (which it calculates from the PBN value) it programs
> -		 * to the first branch device's payload table. The allocation
> -		 * in the payload table could be reduced though (to
> -		 * crtc_state->dp_m_n.tu), provided that the driver doesn't
> -		 * enable SSC on the corresponding link.
> -		 */
> -		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, 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 * pbn_div.full;
> -
> -		drm_WARN_ON(display->drm, remote_tu < crtc_state->dp_m_n.tu);
> -		crtc_state->dp_m_n.tu = remote_tu;
> +		if (intel_dp->is_mst) {
> +			int remote_bw_overhead;
> +			int remote_tu;
> +			fixed20_12 pbn;

Nit: pbn_div is only used for MST, so would (calculate/look it up from
mst_state) here. Also this MST block could be in a separate function,
perhaps for symmetry with the SST part in a function too, but this could
be a follow-up as well. Either way:

Reviewed-by: Imre Deak <imre.deak at intel.com>

> +
> +			remote_bw_overhead = intel_dp_mst_bw_overhead(crtc_state,
> +								      true, dsc_slice_count, link_bpp_x16);
> +
> +			/*
> +			 * The TU size programmed to the HW determines which slots in
> +			 * an MTP frame are used for this stream, which needs to match
> +			 * the payload size programmed to the first downstream branch
> +			 * device's payload table.
> +			 *
> +			 * Note that atm the payload's PBN value DRM core sends via
> +			 * the ALLOCATE_PAYLOAD side-band message matches the payload
> +			 * size (which it calculates from the PBN value) it programs
> +			 * to the first branch device's payload table. The allocation
> +			 * in the payload table could be reduced though (to
> +			 * crtc_state->dp_m_n.tu), provided that the driver doesn't
> +			 * enable SSC on the corresponding link.
> +			 */
> +			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, 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 * pbn_div.full;
> +
> +			drm_WARN_ON(display->drm, remote_tu < crtc_state->dp_m_n.tu);
> +			crtc_state->dp_m_n.tu = remote_tu;
> +
> +			slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr,
> +							      connector->port,
> +							      dfixed_trunc(pbn));
> +		} else {
> +			/* Same as above for remote_tu */
> +			crtc_state->dp_m_n.tu = ALIGN(crtc_state->dp_m_n.tu,
> +						      4 / crtc_state->lane_count);
> +
> +			if (crtc_state->dp_m_n.tu <= 64)
> +				slots = crtc_state->dp_m_n.tu;
> +			else
> +				slots = -EINVAL;
> +		}
>  
> -		slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr,
> -						      connector->port,
> -						      dfixed_trunc(pbn));
>  		if (slots == -EDEADLK)
>  			return slots;
>  
> -- 
> 2.39.5
>