[Intel-gfx] [Intel-xe] [PATCH 34/42] drm/i915/lnl: Start using CDCLK through PLL

[Matt Roper]

On Wed, Aug 23, 2023 at 10:07:32AM -0700, Lucas De Marchi wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> 
> Introduce correspondent definitions and for choosing between CD2X CDCLK
> and PLL CDCLK as a source.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 14 ++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h            |  3 +++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index ed45a2cf5c9a..04937aaabcee 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1932,8 +1932,7 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		dg2_cdclk_squash_program(dev_priv, waveform);
>  
>  	val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
> -		bxt_cdclk_cd2x_pipe(dev_priv, pipe) |
> -		skl_cdclk_decimal(cdclk);
> +		bxt_cdclk_cd2x_pipe(dev_priv, pipe);
>  
>  	/*
>  	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> @@ -1942,6 +1941,17 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  	if ((IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) &&
>  	    cdclk >= 500000)
>  		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> +
> +	if (DISPLAY_VER(dev_priv) >= 20)
> +		/*
> +		 * Using CDCLK through PLL seems to be always better option when
> +		 * its supported, both in terms of performance and power
> +		 * consumption.
> +		 */

I'm not sure what this comment is based on.  But the table on bspec
68861 specifically tells us to set this bit for the cdclk table we
implemented in the last patch, so the logic is correct regardless.

> +		val |= CDCLK_SOURCE_SEL_CDCLK_PLL;
> +	else
> +		val |= skl_cdclk_decimal(cdclk);
> +
>  	intel_de_write(dev_priv, CDCLK_CTL, val);
>  
>  	if (pipe != INVALID_PIPE)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fa85530afac3..d5850761a75a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5933,6 +5933,9 @@ enum skl_power_gate {
>  #define  CDCLK_FREQ_540		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
>  #define  CDCLK_FREQ_337_308		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
>  #define  CDCLK_FREQ_675_617		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
> +#define  CDCLK_SOURCE_SEL_MASK		REG_BIT(25)
> +#define  CDCLK_SOURCE_SEL_CD2X		REG_FIELD_PREP(CDCLK_SOURCE_SEL_MASK, 0)

No need to make a single-bit "mask" or define the unused
CDCLK_SOURCE_SEL_CD2X here.  We can just define
CDCLK_SOURCE_SEL_CDCLK_PLL as REG_BIT(25) directly.


Matt

> +#define  CDCLK_SOURCE_SEL_CDCLK_PLL	REG_FIELD_PREP(CDCLK_SOURCE_SEL_MASK, 1)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_MASK	REG_GENMASK(23, 22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1_5	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
> -- 
> 2.40.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation