On Wed, Sep 18, 2024 at 08:13:33PM +0530, Ankit Nautiyal wrote:
> Add a helper to compute the number of pipes required.
> This will depend on whether the joiner is required or is forced through
> the debugfs. If no joiner is required the helper returns 1.
>
> v2:
> -Return 1 if no joiner is required. (Ville)
> -Change the suffix from joined_pipes to num_pipes. (Ville)
> -Use number of pipes while calculating joined_pipe masks and
> max_dotclk. (Ville)
>
> Signed-off-by: Ankit Nautiyal <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 51 +++++++++++++--------
> drivers/gpu/drm/i915/display/intel_dp.h | 6 +--
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 ++++------
> 3 files changed, 44 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2e35a81fa6d1..96ad048b68cf 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1270,17 +1270,35 @@ intel_dp_mode_valid_downstream(struct intel_connector
> *connector,
> return MODE_OK;
> }
>
> -bool intel_dp_need_joiner(struct intel_dp *intel_dp,
> - struct intel_connector *connector,
> - int hdisplay, int clock)
> +static
> +bool intel_dp_needs_bigjoiner(struct intel_dp *intel_dp,
> + struct intel_connector *connector,
> + int hdisplay, int clock)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>
> if (!intel_dp_has_joiner(intel_dp))
> return false;
>
> - return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120 ||
> - connector->force_joined_pipes == 2;
> + return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120;
> +}
> +
> +int intel_dp_compute_num_pipes(struct intel_dp *intel_dp,
> + struct intel_connector *connector,
> + int hdisplay, int clock)
intel_dp_compute_num_joined_pipes() to be a bit more specfic?
Or even just intel_dp_num_joined_pipes()?
> +{
> + switch (connector->force_joined_pipes) {
> + case 2:
> + return connector->force_joined_pipes;
> + default:
> + MISSING_CASE(connector->force_joined_pipes);
> + fallthrough;
> + case 0:
> + if (intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay,
> clock))
> + return 2;
> + }
> +
> + return 1;
I think that could be simplified to just:
{
if (force_joined_pipes)
return force_joined_pipes;
if (need_bigjoiner())
return 2;
return 1;
}
Apart from that this looks good.
Reviewed-by: Ville Syrjälä <[email protected]>
> }
>
> bool intel_dp_has_dsc(const struct intel_connector *connector)
> @@ -1317,7 +1335,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> u16 dsc_max_compressed_bpp = 0;
> u8 dsc_slice_count = 0;
> enum drm_mode_status status;
> - bool dsc = false, joiner = false;
> + bool dsc = false;
> int num_joined_pipes;
>
> status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> @@ -1339,13 +1357,9 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> target_clock = fixed_mode->clock;
> }
>
> - if (intel_dp_need_joiner(intel_dp, connector,
> - mode->hdisplay, target_clock)) {
> - joiner = true;
> - max_dotclk *= 2;
> - }
> -
> - num_joined_pipes = joiner ? 2 : 1;
> + num_joined_pipes = intel_dp_compute_num_pipes(intel_dp, connector,
> + mode->hdisplay,
> target_clock);
> + max_dotclk *= num_joined_pipes;
>
> if (target_clock > max_dotclk)
> return MODE_CLOCK_HIGH;
> @@ -2552,12 +2566,11 @@ intel_dp_compute_link_config(struct intel_encoder
> *encoder,
> !intel_dp_supports_fec(intel_dp, connector, pipe_config))
> return -EINVAL;
>
> - if (intel_dp_need_joiner(intel_dp, connector,
> - adjusted_mode->crtc_hdisplay,
> - adjusted_mode->crtc_clock))
> - pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
> -
> - num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
> + num_joined_pipes = intel_dp_compute_num_pipes(intel_dp, connector,
> +
> adjusted_mode->crtc_hdisplay,
> +
> adjusted_mode->crtc_clock);
> + if (num_joined_pipes > 1)
> + pipe_config->joiner_pipes = GENMASK(crtc->pipe +
> num_joined_pipes - 1, crtc->pipe);
>
> joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, num_joined_pipes);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index a0a31fb64716..d72ca99e3a1c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -154,9 +154,9 @@ int intel_dp_dsc_sink_max_compressed_bpp(const struct
> intel_connector *connector
> u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
> int mode_clock, int mode_hdisplay,
> int num_joined_pipes);
> -bool intel_dp_need_joiner(struct intel_dp *intel_dp,
> - struct intel_connector *connector,
> - int hdisplay, int clock);
> +int intel_dp_compute_num_pipes(struct intel_dp *intel_dp,
> + struct intel_connector *connector,
> + int hdisplay, int clock);
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7debefd4a0d6..df380f6ee76c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -580,12 +580,11 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return -EINVAL;
>
> - if (intel_dp_need_joiner(intel_dp, connector,
> - adjusted_mode->crtc_hdisplay,
> - adjusted_mode->crtc_clock))
> - pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
> -
> - num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
> + num_joined_pipes = intel_dp_compute_num_pipes(intel_dp, connector,
> +
> adjusted_mode->crtc_hdisplay,
> +
> adjusted_mode->crtc_clock);
> + if (num_joined_pipes > 1)
> + pipe_config->joiner_pipes = GENMASK(crtc->pipe +
> num_joined_pipes - 1, crtc->pipe);
>
> pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> @@ -1427,7 +1426,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> *connector,
> int max_dotclk = to_i915(connector->dev)->display.cdclk.max_dotclk_freq;
> int max_rate, mode_rate, max_lanes, max_link_clock;
> int ret;
> - bool dsc = false, joiner = false;
> + bool dsc = false;
> u16 dsc_max_compressed_bpp = 0;
> u8 dsc_slice_count = 0;
> int target_clock = mode->clock;
> @@ -1471,13 +1470,9 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> *connector,
> * corresponding link capabilities of the sink) in case the
> * stream is uncompressed for it by the last branch device.
> */
> - if (intel_dp_need_joiner(intel_dp, intel_connector,
> - mode->hdisplay, target_clock)) {
> - joiner = true;
> - max_dotclk *= 2;
> - }
> -
> - num_joined_pipes = joiner ? 2 : 1;
> + num_joined_pipes = intel_dp_compute_num_pipes(intel_dp, intel_connector,
> + mode->hdisplay,
> target_clock);
> + max_dotclk *= num_joined_pipes;
>
> ret = drm_modeset_lock(&mgr->base.lock, ctx);
> if (ret)
> --
> 2.45.2
--
Ville Syrjälä
Intel
