All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
Date: Tue, 1 Sep 2020 15:29:52 -0400	[thread overview]
Message-ID: <20200901192952.GA386451@intel.com> (raw)
In-Reply-To: <20200901182758.29203-1-anusha.srivatsa@intel.com>

On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> We currenty check for platform at multiple parts in the driver
> to grab the correct PLL. Let us begin to centralize it through a
> helper function.
> 
> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25 +++++++++++--------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c9013f8f766f..7440836c5e44 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  			pll->info->name, onoff(state), onoff(cur_state));
>  }
>  
> +static
> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
> +				    struct intel_shared_dpll *pll)
> +{
> +
> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
> +			return MG_PLL_ENABLE(0);
> +
> +	return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
>  /**
>   * intel_prepare_shared_dpll - call a dpll's prepare hook
>   * @crtc_state: CRTC, and its state, which has a shared dpll
> @@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> -	if (IS_ELKHARTLAKE(dev_priv) &&
> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -	}
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  			     struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {

there's probably something else that we can do now with the power_{put,get}
to get rid of the, now, doubled if checks...

> -		enable_reg = MG_PLL_ENABLE(0);
>  
>  		/*
>  		 * We need to disable DC states when this DPLL is enabled.
> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
>  		icl_pll_disable(dev_priv, pll, enable_reg);

but here, at least, let's clean this function now...
move this call above and out of the if and delete the one below
and keep just the power_put inside the if...

>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
> -- 
> 2.25.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-09-01 19:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 18:27 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
2020-09-01 19:00 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev2) Patchwork
2020-09-01 19:19 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-09-01 19:29 ` Rodrigo Vivi [this message]
2020-09-02 19:30   ` [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Srivatsa, Anusha
2020-09-02 21:31     ` Vivi, Rodrigo
2020-09-03 17:04       ` Srivatsa, Anusha
2020-09-08 21:00         ` Vivi, Rodrigo
  -- strict thread matches above, loose matches on Subject: below --
2020-09-14 17:57 Anusha Srivatsa
2020-09-10 23:48 Anusha Srivatsa
2020-09-08 23:39 Anusha Srivatsa
2020-09-09 13:06 ` Vivi, Rodrigo
2020-09-10 13:30 ` Jani Nikula
2020-09-10 17:05   ` Srivatsa, Anusha
2020-08-28 21:58 Anusha Srivatsa
2020-08-31 13:42 ` Ville Syrjälä
2020-08-31 19:03   ` Srivatsa, Anusha
2020-08-31 19:47     ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200901192952.GA386451@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=anusha.srivatsa@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.