All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/icl: pass cfgcr* register around instead of pll_id
Date: Mon, 18 Mar 2019 15:31:52 +0200	[thread overview]
Message-ID: <20190318133152.GJ3888@intel.com> (raw)
In-Reply-To: <20190316004526.12065-1-lucas.demarchi@intel.com>

On Fri, Mar 15, 2019 at 05:45:26PM -0700, Lucas De Marchi wrote:
> The caller already knows what platform that is and what register should
> be used. Instead of keep adding if/else chains on a leaf functions,
> let the caller pass the register.
> 
> We read cfgcr0 twice for CNL, but we were already doing that anyway.
> 
> icl_calc_dp_combo_pll_link() is only used for ICL, but let's keep
> consistency with cnl_calc_wrpll_link().
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c        |  4 +++-
>  drivers/gpu/drm/i915/intel_ddi.c      | 25 ++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 +++---
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h      |  2 +-
>  5 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index beb30d9a855c..28f5da697693 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1183,7 +1183,9 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  
>  	/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
>  	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> -	pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +	pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv,
> +						      ICL_DPLL_CFGCR0(pll_id),
> +						      ICL_DPLL_CFGCR1(pll_id));
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
>  	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 69aa0d148795..24675ef8b262 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1304,18 +1304,13 @@ static int skl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>  }
>  
>  int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> -			enum intel_dpll_id pll_id)
> +			i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg)
>  {
>  	u32 cfgcr0, cfgcr1;
>  	u32 p0, p1, p2, dco_freq, ref_clock;
>  
> -	if (INTEL_GEN(dev_priv) >= 11) {
> -		cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
> -		cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
> -	} else {
> -		cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> -		cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll_id));
> -	}
> +	cfgcr0 = I915_READ(cfgcr0_reg);
> +	cfgcr1 = I915_READ(cfgcr1_reg);

Don't we alredy have the dpll state read out at this point?
I thought I changed at least some of these to not read it out
directly.

>  
>  	p0 = cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
>  	p2 = cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
> @@ -1471,17 +1466,23 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	i915_reg_t cfgcr0_reg, cfgcr1_reg;
>  	enum port port = encoder->port;
>  	int link_clock = 0;
>  	u32 pll_id;
>  
>  	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> +	cfgcr0_reg = ICL_DPLL_CFGCR0(pll_id);
> +	cfgcr1_reg = ICL_DPLL_CFGCR1(pll_id);
> +
>  	if (intel_port_is_combophy(dev_priv, port)) {
>  		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> -			link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +			link_clock = cnl_calc_wrpll_link(dev_priv, cfgcr0_reg,
> +							 cfgcr1_reg);
>  		else
>  			link_clock = icl_calc_dp_combo_pll_link(dev_priv,
> -								pll_id);
> +								cfgcr0_reg,
> +								cfgcr1_reg);
>  	} else {
>  		if (pll_id == DPLL_ID_ICL_TBTPLL)
>  			link_clock = icl_calc_tbt_pll_link(dev_priv, port);
> @@ -1506,7 +1507,9 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
>  	cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
>  
>  	if (cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
> -		link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +		link_clock = cnl_calc_wrpll_link(dev_priv,
> +						 CNL_DPLL_CFGCR0(pll_id),
> +						 CNL_DPLL_CFGCR1(pll_id));
>  	} else {
>  		link_clock = cfgcr0 & DPLL_CFGCR0_LINK_RATE_MASK;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index ca975213da2a..ed4024f4394b 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2569,7 +2569,7 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
>  }
>  
>  int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
> -			       u32 pll_id)
> +			       i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg)
>  {
>  	u32 cfgcr0, cfgcr1;
>  	u32 pdiv, kdiv, qdiv_mode, qdiv_ratio, dco_integer, dco_fraction;
> @@ -2577,8 +2577,8 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
>  	int index, n_entries, link_clock;
>  
>  	/* Read back values from DPLL CFGCR registers */
> -	cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
> -	cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
> +	cfgcr0 = I915_READ(cfgcr0_reg);
> +	cfgcr1 = I915_READ(cfgcr1_reg);
>  
>  	dco_integer = cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK;
>  	dco_fraction = (cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index 40e8391a92f2..0aa504e9bfbf 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -342,7 +342,7 @@ void intel_shared_dpll_init(struct drm_device *dev);
>  void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
>  			      struct intel_dpll_hw_state *hw_state);
>  int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
> -			       u32 pll_id);
> +			       i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg);
>  int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv);
>  enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port);
>  bool intel_dpll_is_combophy(enum intel_dpll_id id);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d9f188ef21f4..9d0bf1914773 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1659,7 +1659,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>  				     bool enable);
>  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
>  int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> -			enum intel_dpll_id pll_id);
> +			i915_reg_t cgfcr0_reg, i915_reg_t cgfcr1_reg);
>  
>  unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
>  				   int color_plane, unsigned int height);
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2019-03-18 13:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-16  0:45 [PATCH] drm/i915/icl: pass cfgcr* register around instead of pll_id Lucas De Marchi
2019-03-16  1:32 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-03-16  8:14 ` ✓ Fi.CI.IGT: " Patchwork
2019-03-18 13:31 ` Ville Syrjälä [this message]
2019-03-18 18:40   ` [PATCH] " Lucas De Marchi
2019-03-18 18:53     ` Ville Syrjälä
2019-03-18 21:58       ` Lucas De Marchi
2019-03-18 23:33       ` Lucas De Marchi
2019-03-19 11:09         ` 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=20190318133152.GJ3888@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    /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.