Re: [PATCH 11/11] drm/i915/cdclk: Unify cdclk max() parameter order

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In some places we do
>  min_cdclk = max(min_cdclk, other_min_cdclk)
> and in other places we have the arguments swapped as
>  min_cdclk = max(other_min_cdclk, min_cdclk)
>
> Unify everyone to use the first order of arguments, because
> it looks cleaner, especially within intel_crtc_compute_min_cdclk()
> which is doing a lot of these back-to-back.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_audio.c |  8 ++++----
>  drivers/gpu/drm/i915/display/intel_bw.c    |  2 +-
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 18 +++++++++---------
>  3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index af0bfdc44072..047cc5a2ef1f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -997,10 +997,10 @@ int intel_audio_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	    crtc_state->lane_count == 4) {
>  		if (DISPLAY_VER(display) == 10) {
>  			/* Display WA #1145: glk */
> -			min_cdclk = max(316800, min_cdclk);
> +			min_cdclk = max(min_cdclk, 316800);
>  		} else if (DISPLAY_VER(display) == 9 || IS_BROADWELL(dev_priv)) {
>  			/* Display WA #1144: skl,bxt */
> -			min_cdclk = max(432000, min_cdclk);
> +			min_cdclk = max(min_cdclk, 432000);
>  		}
>  	}
>  
> @@ -1009,7 +1009,7 @@ int intel_audio_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
>  	 */
>  	if (DISPLAY_VER(display) >= 9)
> -		min_cdclk = max(2 * 96000, min_cdclk);
> +		min_cdclk = max(min_cdclk, 2 * 96000);
>  
>  	/*
>  	 * "For DP audio configuration, cdclk frequency shall be set to
> @@ -1020,7 +1020,7 @@ int intel_audio_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	 */
>  	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
>  	    intel_crtc_has_dp_encoder(crtc_state))
> -		min_cdclk = max(crtc_state->port_clock, min_cdclk);
> +		min_cdclk = max(min_cdclk, crtc_state->port_clock);
>  
>  	return min_cdclk;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 47036d4abb33..5f91b009df0d 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -1256,7 +1256,7 @@ int intel_bw_min_cdclk(struct drm_i915_private *i915,
>  	min_cdclk = intel_bw_dbuf_min_cdclk(i915, bw_state);
>  
>  	for_each_pipe(i915, pipe)
> -		min_cdclk = max(bw_state->min_cdclk[pipe], min_cdclk);
> +		min_cdclk = max(min_cdclk, bw_state->min_cdclk[pipe]);
>  
>  	return min_cdclk;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 9f38dd14b2d8..f16a37ef7316 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2799,7 +2799,7 @@ static int intel_planes_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	int min_cdclk = 0;
>  
>  	for_each_intel_plane_on_crtc(display->drm, crtc, plane)
> -		min_cdclk = max(crtc_state->min_cdclk[plane->id], min_cdclk);
> +		min_cdclk = max(min_cdclk, crtc_state->min_cdclk[plane->id]);
>  
>  	return min_cdclk;
>  }
> @@ -2812,10 +2812,10 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  		return 0;
>  
>  	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
> -	min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
> -	min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
> -	min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
> -	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
> +	min_cdclk = max(min_cdclk, hsw_ips_min_cdclk(crtc_state));
> +	min_cdclk = max(min_cdclk, intel_audio_min_cdclk(crtc_state));
> +	min_cdclk = max(min_cdclk, vlv_dsi_min_cdclk(crtc_state));
> +	min_cdclk = max(min_cdclk, intel_planes_min_cdclk(crtc_state));
>  	min_cdclk = max(min_cdclk, intel_vdsc_min_cdclk(crtc_state));
>  
>  	return min_cdclk;
> @@ -2868,7 +2868,7 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
>  	min_cdclk = max(cdclk_state->force_min_cdclk,
>  			cdclk_state->bw_min_cdclk);
>  	for_each_pipe(display, pipe)
> -		min_cdclk = max(cdclk_state->min_cdclk[pipe], min_cdclk);
> +		min_cdclk = max(min_cdclk, cdclk_state->min_cdclk[pipe]);
>  
>  	/*
>  	 * Avoid glk_force_audio_cdclk() causing excessive screen
> @@ -2880,7 +2880,7 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
>  	 */
>  	if (IS_GEMINILAKE(dev_priv) && cdclk_state->active_pipes &&
>  	    !is_power_of_2(cdclk_state->active_pipes))
> -		min_cdclk = max(2 * 96000, min_cdclk);
> +		min_cdclk = max(min_cdclk, 2 * 96000);
>  
>  	if (min_cdclk > display->cdclk.max_cdclk_freq) {
>  		drm_dbg_kms(display->drm,
> @@ -2936,8 +2936,8 @@ static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
>  
>  	min_voltage_level = 0;
>  	for_each_pipe(display, pipe)
> -		min_voltage_level = max(cdclk_state->min_voltage_level[pipe],
> -					min_voltage_level);
> +		min_voltage_level = max(min_voltage_level,
> +					cdclk_state->min_voltage_level[pipe]);
>  
>  	return min_voltage_level;
>  }

-- 
Jani Nikula, Intel