Re: [RESEND 05/12] drm/i915: add vlv_clock_get_cdclk()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Aug 19, 2025 at 03:53:35PM +0300, Jani Nikula wrote:
> Add vlv_clock_get_cdclk() helper to hide the details from the callers.
> 
> For now, this means running vlv_get_hpll_vco() twice in vlv_get_cdclk(),
> but this will be improved later.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c   | 4 +---
>  drivers/gpu/drm/i915/display/intel_display.c | 6 ++++++
>  drivers/gpu/drm/i915/display/intel_display.h | 1 +
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 65ad7d48dd39..e898c0541168 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -610,9 +610,7 @@ static void vlv_get_cdclk(struct intel_display *display,
>  	vlv_iosf_sb_get(display->drm, BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_PUNIT));
>  
>  	cdclk_config->vco = vlv_get_hpll_vco(display->drm);
> -	cdclk_config->cdclk = vlv_get_cck_clock(display->drm, "cdclk",
> -						CCK_DISPLAY_CLOCK_CONTROL,
> -						cdclk_config->vco);
> +	cdclk_config->cdclk = vlv_clock_get_cdclk(display->drm);
>  
>  	val = vlv_punit_read(display->drm, PUNIT_REG_DSPSSPM);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8baa5f898284..644028d0c7ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -204,6 +204,12 @@ int vlv_clock_get_czclk(struct drm_device *drm)
>  	return i915->czclk_freq;
>  }
>  
> +int vlv_clock_get_cdclk(struct drm_device *drm)
> +{
> +	return vlv_get_cck_clock(drm, "cdclk", CCK_DISPLAY_CLOCK_CONTROL,
> +				 vlv_get_hpll_vco(drm));

This is actually just vlv_get_cck_clock_hpll(), except that one has
the vlv_iosf_sb_get/put(CCK) stuff inside it whereas vlv_get_cck_clock()
needs the caller to grab that.

So that part of the vlv_clocks.c interface is now rather confusing.
The interface should probably be high level enough that the caller
doesn't have to deal with that get/put stuff at all...

> +}
> +
>  int vlv_clock_get_gpll(struct drm_device *drm)
>  {
>  	return vlv_get_cck_clock(drm, "GPLL ref", CCK_GPLL_CLOCK_CONTROL,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 5c9b57e94a65..9fdbc4ad5391 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -440,6 +440,7 @@ int vlv_get_cck_clock(struct drm_device *drm,
>  		      const char *name, u32 reg, int ref_freq);
>  int vlv_clock_get_hrawclk(struct drm_device *drm);
>  int vlv_clock_get_czclk(struct drm_device *drm);
> +int vlv_clock_get_cdclk(struct drm_device *drm);
>  int vlv_clock_get_gpll(struct drm_device *drm);
>  bool intel_has_pending_fb_unpin(struct intel_display *display);
>  void intel_encoder_destroy(struct drm_encoder *encoder);
> -- 
> 2.47.2

-- 
Ville Syrjälä
Intel