Re: [PATCH 08/16] drm/i915/dp_mst: Rework pipe joiner logic in mode_valid

[Imre Deak <imre.deak@intel.com>]

On Thu, Jan 29, 2026 at 10:41:46PM +0530, Ankit Nautiyal wrote:
> Refactor the logic to get the number of joined pipes. Start with a single
> pipe and incrementally try additional pipes only if needed. While DSC
> overhead is not yet computed here, this restructuring prepares the code to
> support that in follow-up changes.
> 
> v2:
>  - Remove fallback in case force-joiner configuration fails. (Imre)
>  - Drop redundant MODE_OK assignment (Imre)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     |  2 -
>  drivers/gpu/drm/i915/display/intel_dp.h     |  3 +
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 91 ++++++++++++---------
>  3 files changed, 56 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a355900a31d9..febfea641e56 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1371,7 +1371,6 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  	return MODE_OK;
>  }
>  
> -static
>  int intel_dp_max_hdisplay_per_pipe(struct intel_display *display)
>  {
>  	return DISPLAY_VER(display) >= 30 ? 6144 : 5120;
> @@ -1434,7 +1433,6 @@ bool intel_dp_has_dsc(const struct intel_connector *connector)
>  	return true;
>  }
>  
> -static
>  bool intel_dp_can_join(struct intel_display *display,
>  		       int num_joined_pipes)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 25bfbfd291b0..6d409c1998c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -225,5 +225,8 @@ int intel_dp_compute_config_late(struct intel_encoder *encoder,
>  				 struct drm_connector_state *conn_state);
>  int intel_dp_sdp_min_guardband(const struct intel_crtc_state *crtc_state,
>  			       bool assume_all_enabled);
> +int intel_dp_max_hdisplay_per_pipe(struct intel_display *display);
> +bool intel_dp_can_join(struct intel_display *display,
> +		       int num_joined_pipes);
>  
>  #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index fc9367cc42ec..414c7ffc704e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1420,7 +1420,6 @@ mst_connector_mode_valid_ctx(struct drm_connector *_connector,
>  	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst.mgr;
>  	struct drm_dp_mst_port *port = connector->mst.port;
>  	const int min_bpp = 18;
> -	int max_dotclk = display->cdclk.max_dotclk_freq;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  	unsigned long bw_overhead_flags =
>  		DRM_DP_BW_OVERHEAD_MST | DRM_DP_BW_OVERHEAD_SSC_REF_CLK;
> @@ -1428,6 +1427,7 @@ mst_connector_mode_valid_ctx(struct drm_connector *_connector,
>  	bool dsc = false;
>  	int target_clock = mode->clock;
>  	int num_joined_pipes;
> +	int num_pipes;
>  
>  	if (drm_connector_is_unregistered(&connector->base)) {
>  		*status = MODE_ERROR;
> @@ -1480,47 +1480,62 @@ mst_connector_mode_valid_ctx(struct drm_connector *_connector,
>  		return 0;
>  	}
>  
> -	num_joined_pipes = intel_dp_num_joined_pipes(intel_dp, connector,
> -						     mode->hdisplay, target_clock);
> +	for (num_pipes = 1; num_pipes <= I915_MAX_PIPES; num_pipes++) {
> +		int max_dotclk = display->cdclk.max_dotclk_freq;
>  
> -	if (intel_dp_has_dsc(connector) && drm_dp_sink_supports_fec(connector->dp.fec_capability)) {
> -		/*
> -		 * TBD pass the connector BPC,
> -		 * for now U8_MAX so that max BPC on that platform would be picked
> -		 */
> -		int pipe_bpp = intel_dp_dsc_compute_max_bpp(connector, U8_MAX);
> -
> -		if (!drm_dp_is_uhbr_rate(max_link_clock))
> -			bw_overhead_flags |= DRM_DP_BW_OVERHEAD_FEC;
> -
> -		dsc = intel_dp_mode_valid_with_dsc(connector,
> -						   max_link_clock, max_lanes,
> -						   target_clock, mode->hdisplay,
> -						   num_joined_pipes,
> -						   INTEL_OUTPUT_FORMAT_RGB, pipe_bpp,
> -						   bw_overhead_flags);
> -	}
> -
> -	if (intel_dp_joiner_needs_dsc(display, num_joined_pipes) && !dsc) {
>  		*status = MODE_CLOCK_HIGH;

This needs to be inited before the loop.

> -		return 0;
> +
> +		if (connector->force_joined_pipes &&
> +		    num_pipes != connector->force_joined_pipes)
> +			continue;
> +
> +		num_joined_pipes = num_pipes;

No need for two variables.

> +
> +		if (!intel_dp_can_join(display, num_joined_pipes))
> +			continue;
> +
> +		if (mode->hdisplay > num_joined_pipes * intel_dp_max_hdisplay_per_pipe(display))
> +			continue;
> +
> +		if (intel_dp_has_dsc(connector) &&
> +		    drm_dp_sink_supports_fec(connector->dp.fec_capability)) {
> +			/*
> +			 * TBD pass the connector BPC,
> +			 * for now U8_MAX so that max BPC on that platform would be picked
> +			 */
> +			int pipe_bpp = intel_dp_dsc_compute_max_bpp(connector, U8_MAX);
> +
> +			if (!drm_dp_is_uhbr_rate(max_link_clock))
> +				bw_overhead_flags |= DRM_DP_BW_OVERHEAD_FEC;
> +
> +			dsc = intel_dp_mode_valid_with_dsc(connector,
> +							   max_link_clock, max_lanes,
> +							   target_clock, mode->hdisplay,
> +							   num_joined_pipes,
> +							   INTEL_OUTPUT_FORMAT_RGB, pipe_bpp,
> +							   bw_overhead_flags);
> +		}
> +
> +		if (intel_dp_joiner_needs_dsc(display, num_joined_pipes) && !dsc)
> +			continue;
> +
> +		if (mode_rate > max_rate && !dsc)
> +			continue;
> +
> +		*status = intel_mode_valid_max_plane_size(display, mode, num_joined_pipes);
> +
> +		if (*status != MODE_OK)
> +			continue;
> +
> +		max_dotclk *= num_joined_pipes;
> +
> +		if (mode->clock > max_dotclk)
> +			*status = MODE_CLOCK_HIGH;
> +
> +		if (status == MODE_OK)
> +			break;

Nit: would be better to match what intel_dp_mode_valid() is doing:

		if (mode->clock > max_dotclk) {
			*status = MODE_CLOCK_HIGH;
			continue;
		}

		break;

With the things above addressed:
Reviewed-by: Imre Deak <imre.deak@intel.com>

>  	}
>  
> -	if (mode_rate > max_rate && !dsc) {
> -		*status = MODE_CLOCK_HIGH;
> -		return 0;
> -	}
> -
> -	*status = intel_mode_valid_max_plane_size(display, mode, num_joined_pipes);
> -
> -	if (*status != MODE_OK)
> -		return 0;
> -
> -	max_dotclk *= num_joined_pipes;
> -
> -	if (mode->clock > max_dotclk)
> -		*status = MODE_CLOCK_HIGH;
> -
>  	return 0;
>  }
>  
> -- 
> 2.45.2
>