All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/i915: kill {bdw, bxt}_modeset_calc_cdclk
Date: Fri, 17 Feb 2017 15:51:57 +0200	[thread overview]
Message-ID: <20170217135157.GQ31595@intel.com> (raw)
In-Reply-To: <1487337727-9813-2-git-send-email-paulo.r.zanoni@intel.com>

On Fri, Feb 17, 2017 at 11:22:04AM -0200, Paulo Zanoni wrote:
> The functions are pretty much the same, except for the CDCLK and VCO
> calculations. Add BDW support to vlv_modeset_calc_cdclk() and add
> BXT/GLK support to skl_modeset_calc_cdclk(). The two reamining
> functions are still very similar, except for the fact that the vlv
> version doesn't touch the VCO. Further patches could unify them even
> more if that's desired.
> 
> While at it, merge some lines that can fit 80 columns in those
> functions.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 120 ++++++++++---------------------------
>  1 file changed, 30 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index d643c0c..d505ff1 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1499,45 +1499,18 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
>  static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk;
> -
> -	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = vlv_calc_cdclk(dev_priv, 0);
> -
> -		intel_state->cdclk.actual.cdclk = cdclk;
> -	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> -	}
> -
> -	return 0;
> -}
> -
> -static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
>  	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk;
>  
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Broadwell should also account for plane ratio once 64bpp pixel
> +	 * formats are supported.

BTW these restrictions affect pretty much all platforms, so specifying the
platforms in the comment is rather redundant.

>  	 */
> -	cdclk = bdw_calc_cdclk(max_pixclk);
> +	if (IS_BROADWELL(dev_priv))
> +		cdclk = bdw_calc_cdclk(max_pixclk);
> +	else
> +		cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
>  		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -1548,12 +1521,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	intel_state->cdclk.logical.cdclk = cdclk;
>  
>  	if (!intel_state->active_crtcs) {
> -		cdclk = bdw_calc_cdclk(0);
> +		if (IS_BROADWELL(dev_priv))
> +			cdclk = bdw_calc_cdclk(0);
> +		else
> +			cdclk = vlv_calc_cdclk(dev_priv, 0);
>  
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1561,57 +1536,26 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
> -	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	const int max_pixclk = intel_max_pixel_rate(state);
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk, vco;
>  
> -	vco = intel_state->cdclk.logical.vco;
> -	if (!vco)
> -		vco = dev_priv->skl_preferred_vco_freq;
> -
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Skylake/Kabylake should also account for plane ratio once
> +	 * 64bpp pixel formats are supported.
>  	 */
> -	cdclk = skl_calc_cdclk(max_pixclk, vco);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.vco = vco;
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = skl_calc_cdclk(0, vco);
> -
> -		intel_state->cdclk.actual.vco = vco;
> -		intel_state->cdclk.actual.cdclk = cdclk;
> -	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> -	}
> -
> -	return 0;
> -}
> -
> -static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk, vco;
> -
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		cdclk = glk_calc_cdclk(max_pixclk);
>  		vco = glk_de_pll_vco(dev_priv, cdclk);
> -	} else {
> +	} else if (IS_BROXTON(dev_priv)) {
>  		cdclk = bxt_calc_cdclk(max_pixclk);
>  		vco = bxt_de_pll_vco(dev_priv, cdclk);
> +	} else {
> +		vco = intel_state->cdclk.logical.vco;
> +		if (!vco)
> +			vco = dev_priv->skl_preferred_vco_freq;
> +		cdclk = skl_calc_cdclk(max_pixclk, vco);
>  	}
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
> @@ -1627,16 +1571,17 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
>  		if (IS_GEMINILAKE(dev_priv)) {
>  			cdclk = glk_calc_cdclk(0);
>  			vco = glk_de_pll_vco(dev_priv, cdclk);
> -		} else {
> +		} else if (IS_BROXTON(dev_priv)) {
>  			cdclk = bxt_calc_cdclk(0);
>  			vco = bxt_de_pll_vco(dev_priv, cdclk);
> +		} else {
> +			cdclk = skl_calc_cdclk(0, vco);
>  		}
>  
>  		intel_state->cdclk.actual.vco = vco;
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1823,24 +1768,19 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = chv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = vlv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		dev_priv->display.set_cdclk = bdw_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bdw_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_GEN9_LP(dev_priv)) {
>  		dev_priv->display.set_cdclk = bxt_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bxt_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		dev_priv->display.set_cdclk = skl_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			skl_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	}
>  
>  	if (IS_GEN9_BC(dev_priv))
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

  reply	other threads:[~2017-02-17 13:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-17 13:22 [PATCH 0/4] Small clocking code refactor Paulo Zanoni
2017-02-17 13:22 ` [PATCH 1/4] drm/i915: kill {bdw, bxt}_modeset_calc_cdclk Paulo Zanoni
2017-02-17 13:51   ` Ville Syrjälä [this message]
2017-02-18 15:13   ` David Weinehall
2017-02-17 13:22 ` [PATCH 2/4] drm/i915: add intel_calc_cdclk() Paulo Zanoni
2017-02-17 13:49   ` Ville Syrjälä
2017-02-17 20:37     ` Paulo Zanoni
2017-02-17 20:48       ` Ville Syrjälä
2017-02-17 13:22 ` [PATCH 3/4] drm/i915: remove potentially confusing IS_G4X checks Paulo Zanoni
2017-02-17 13:22 ` [PATCH 4/4] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni
2017-02-17 14:05   ` Ville Syrjälä
2017-02-17 15:17     ` Paulo Zanoni
2017-02-17 15:28       ` Ville Syrjälä
2017-02-17 15:22 ` ✓ Fi.CI.BAT: success for Small clocking code refactor Patchwork

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=20170217135157.GQ31595@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@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.