On Wed, Sep 11, 2024 at 06:43:34PM +0530, Ankit Nautiyal wrote:
> Add a helper to compute the number of pipes to be joined.
> The num of pipes joined will depend on whether the joiner is required or
> is forced through the debugfs.
>
> Signed-off-by: Ankit Nautiyal <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 46 +++++++++++++++------
> drivers/gpu/drm/i915/display/intel_dp.h | 6 +--
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 ++++---
> 3 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 5c30d4488141..333624fcebd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1261,17 +1261,33 @@ 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;
> +}
>
> - return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120 ||
> - connector->force_joined_pipes == 2;
> +int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
> + struct intel_connector *connector,
> + int hdisplay, int clock)
s/joiner_pipes/num_pipes/ would seem a more appropriate name
if we want to make i return 1 for the non-joiner cases...
> +{
> + 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_has_joiner(intel_dp) &&
> + intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay,
> clock))
> + return 2;
> + }
> +
> + return 0;
... which I thought we did, but here we are returning 0?
> }
>
> bool intel_dp_has_dsc(const struct intel_connector *connector)
> @@ -1309,6 +1325,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> u8 dsc_slice_count = 0;
> enum drm_mode_status status;
> bool dsc = false, joiner = false;
> + int num_joined_pipes;
>
> status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> if (status != MODE_OK)
> @@ -1329,11 +1346,14 @@ 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)) {
> + num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> + mode->hdisplay,
> target_clock);
> +
> + if (num_joined_pipes == 2) {
> joiner = true;
> max_dotclk *= 2;
> }
This could become nan unconditional
max_dotclock *= num_pipes;
if we did the return 1 for non-joiner cases.
> +
> if (target_clock > max_dotclk)
> return MODE_CLOCK_HIGH;
>
> @@ -2523,15 +2543,17 @@ intel_dp_compute_link_config(struct intel_encoder
> *encoder,
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> struct link_config_limits limits;
> bool dsc_needed, joiner_needs_dsc;
> + int num_joined_pipes;
> int ret = 0;
>
> if (pipe_config->fec_enable &&
> !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))
> + num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> +
> adjusted_mode->crtc_hdisplay,
> +
> adjusted_mode->crtc_clock);
> + if (num_joined_pipes == 2)
> pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
This could be a bit more generic with something like:
if (num_pipes > 1)
joiner_pipes = GENNMASK(pipe + num_pipes - 1, pipe);
But maybe you're doing some that stuff later. I'll keep reading...
>
> joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915,
> pipe_config->joiner_pipes);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index 1b9aaddd8c35..bc9a82d82df2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -153,9 +153,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,
> bool bigjoiner);
> -bool intel_dp_need_joiner(struct intel_dp *intel_dp,
> - struct intel_connector *connector,
> - int hdisplay, int clock);
> +int intel_dp_compute_joiner_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 15541932b809..383b3e38df52 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -568,6 +568,7 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
> &pipe_config->hw.adjusted_mode;
> struct link_config_limits limits;
> bool dsc_needed, joiner_needs_dsc;
> + int num_joined_pipes;
> int ret = 0;
>
> if (pipe_config->fec_enable &&
> @@ -577,9 +578,10 @@ 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))
> + num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> +
> adjusted_mode->crtc_hdisplay,
> +
> adjusted_mode->crtc_clock);
> + if (num_joined_pipes == 2)
> pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>
> pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> @@ -1426,6 +1428,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> *connector,
> u16 dsc_max_compressed_bpp = 0;
> u8 dsc_slice_count = 0;
> int target_clock = mode->clock;
> + int num_joined_pipes;
>
> if (drm_connector_is_unregistered(connector)) {
> *status = MODE_ERROR;
> @@ -1465,8 +1468,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)) {
> + num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp,
> intel_connector,
> + mode->hdisplay,
> target_clock);
> + if (num_joined_pipes == 2) {
> joiner = true;
> max_dotclk *= 2;
> }
> --
> 2.45.2
--
Ville Syrjälä
Intel
