On Wed, Aug 23, 2023 at 10:07:32AM -0700, Lucas De Marchi wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
>
> Introduce correspondent definitions and for choosing between CD2X CDCLK
> and PLL CDCLK as a source.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> 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
