Re: [PATCH 2/2] drm/i915: Move intel_disable_shared_dpll() into ilk_pch_post_disable()

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

On Mon, 10 Mar 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On ILK-IVB only PCH outputs use shared dplls. Move the relevant
> intel_disable_shared_dpll() into ilk_pch_post_disable() to make
> that clear (and if we extend the dpll mgr to cover all plls we need
> different enable/disable points anyway for the PCH vs. CPU eDP cases).
> The intel_enable_shared_dpll() counterpart was already in
> ilk_pch_enable() anyway, so this is the more symmetric place for the
> disable as well.
>
> 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_display.c     | 2 --
>  drivers/gpu/drm/i915/display/intel_pch_display.c | 4 ++++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 0e0e5285ad97..f21c52a7c10a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1770,12 +1770,10 @@ static void ilk_crtc_disable(struct intel_atomic_state *state,
>  	if (old_crtc_state->has_pch_encoder)
>  		ilk_pch_post_disable(state, crtc);
>  
>  	intel_set_cpu_fifo_underrun_reporting(display, pipe, true);
>  	intel_set_pch_fifo_underrun_reporting(display, pipe, true);
> -
> -	intel_disable_shared_dpll(old_crtc_state);
>  }
>  
>  static void hsw_crtc_disable(struct intel_atomic_state *state,
>  			     struct intel_crtc *crtc)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_pch_display.c b/drivers/gpu/drm/i915/display/intel_pch_display.c
> index 99f6d6f53fa7..bde69b361d6e 100644
> --- a/drivers/gpu/drm/i915/display/intel_pch_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_pch_display.c
> @@ -458,10 +458,12 @@ void ilk_pch_disable(struct intel_atomic_state *state,
>  
>  void ilk_pch_post_disable(struct intel_atomic_state *state,
>  			  struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, crtc);
>  	enum pipe pipe = crtc->pipe;
>  
>  	ilk_disable_pch_transcoder(crtc);
>  
>  	if (HAS_PCH_CPT(dev_priv)) {
> @@ -474,10 +476,12 @@ void ilk_pch_post_disable(struct intel_atomic_state *state,
>  		intel_de_rmw(dev_priv, PCH_DPLL_SEL,
>  			     TRANS_DPLL_ENABLE(pipe) | TRANS_DPLLB_SEL(pipe), 0);
>  	}
>  
>  	ilk_fdi_pll_disable(crtc);
> +
> +	intel_disable_shared_dpll(old_crtc_state);
>  }
>  
>  static void ilk_pch_clock_get(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);

-- 
Jani Nikula, Intel