Re: [PATCH 2/4] drm/i915: Read czclk from CCK on vlv/chv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2015-09-24 at 23:29 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> As with the cdclk, read out czclk from CCK as well. This gives us the
> real current value and avoids having to decode fuses and whatnot.
> 
> Also store it in kHz under dev_priv like we do for cdlck since it's not
> just an rps related clock, and having it in kHz is more
> standard/convenient for some things.
> 
> Imre also pointed out that we currently fail to read czclk on VLV, which
> means the PFI credit programming isn't working as expected.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Yep, much better to have a generic way to read out all the CCK clocks,
removes a lot of special casing. It's also true that these clocks can
change, though let's note that czclk is fixed. Looks ok to me:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>

Below some nitpicks about naming.

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_pm.c      | 25 ++--------
>  4 files changed, 60 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2b5d587..77c9312 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1146,7 +1146,6 @@ struct intel_gen6_power_mgmt {
>  	u8 efficient_freq;	/* AKA RPe. Pre-determined balanced frequency */
>  	u8 rp1_freq;		/* "less than" RP0 power/freqency */
>  	u8 rp0_freq;		/* Non-overclocked max frequency. */
> -	u32 cz_freq;
>  
>  	u8 up_threshold; /* Current %busy required to uplock */
>  	u8 down_threshold; /* Current %busy required to downclock */
> @@ -1810,6 +1809,7 @@ struct drm_i915_private {
>  	unsigned int cdclk_freq, max_cdclk_freq;
>  	unsigned int max_dotclk_freq;
>  	unsigned int hpll_freq;
> +	unsigned int czclk_freq;
>  
>  	/**
>  	 * wq - Driver workqueue for GEM.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f22fb80..83218032 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -728,6 +728,7 @@ enum skl_disp_power_wells {
>  #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> +#define CCK_CZ_CLOCK_CONTROL			0x62
>  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
>  #define  CCK_TRUNK_FORCE_ON			(1 << 17)
>  #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8567b46..d668897 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -132,6 +132,42 @@ struct intel_limit {
>  	intel_p2_t	    p2;
>  };
>  
> +/* returns HPLL frequency in kHz */
> +static int valleyview_get_vco(struct drm_i915_private *dev_priv)

This wasn't added in this patch, but I would call it get_cck_ref or
similar. VCO is too generic and HPLL isn't accurate, since the 800MHz
clock is a bypass clock directly from iCLK.

> +{
> +	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> +
> +	/* Obtain SKU information */
> +	mutex_lock(&dev_priv->sb_lock);
> +	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
> +		CCK_FUSE_HPLL_FREQ_MASK;
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +	return vco_freq[hpll_freq] * 1000;
> +}
> +
> +static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> +				  const char *name, u32 reg)

I would have called it simply get_cck_clock for the above reason.

> +{
> +	u32 val;
> +	int divider;
> +
> +	if (dev_priv->hpll_freq == 0)
> +		dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +	val = vlv_cck_read(dev_priv, reg);
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +	divider = val & CCK_FREQUENCY_VALUES;
> +
> +	WARN((val & CCK_FREQUENCY_STATUS) !=
> +	     (divider << CCK_FREQUENCY_STATUS_SHIFT),
> +	     "%s change in progress\n", name);
> +
> +	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> +}
> +
>  int
>  intel_pch_rawclk(struct drm_device *dev)
>  {
> @@ -175,6 +211,17 @@ int intel_hrawclk(struct drm_device *dev)
>  	}
>  }
>  
> +static void intel_update_czclk(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_VALLEYVIEW(dev_priv))
> +		return;
> +
> +	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> +						      CCK_CZ_CLOCK_CONTROL);
> +
> +	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> +}
> +
>  static inline u32 /* units of 100MHz */
>  intel_fdi_link_freq(struct drm_device *dev)
>  {
> @@ -5749,20 +5796,6 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  		DRM_ERROR("DBuf power enable timeout\n");
>  }
>  
> -/* returns HPLL frequency in kHz */
> -static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> -{
> -	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> -
> -	/* Obtain SKU information */
> -	mutex_lock(&dev_priv->sb_lock);
> -	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
> -		CCK_FUSE_HPLL_FREQ_MASK;
> -	mutex_unlock(&dev_priv->sb_lock);
> -
> -	return vco_freq[hpll_freq] * 1000;
> -}
> -
>  /* Adjust CDclk dividers to allow high res or save power if possible */
>  static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  {
> @@ -5983,7 +6016,7 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
>  	else
>  		default_credits = PFI_CREDIT(8);
>  
> -	if (DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 1000) >= dev_priv->rps.cz_freq) {
> +	if (dev_priv->cdclk_freq >= dev_priv->czclk_freq) {
>  		/* CHV suggested value is 31 or 63 */
>  		if (IS_CHERRYVIEW(dev_priv))
>  			credits = PFI_CREDIT_63;
> @@ -6715,24 +6748,8 @@ static int haswell_get_display_clock_speed(struct drm_device *dev)
>  
>  static int valleyview_get_display_clock_speed(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 val;
> -	int divider;
> -
> -	if (dev_priv->hpll_freq == 0)
> -		dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> -
> -	mutex_lock(&dev_priv->sb_lock);
> -	val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> -	mutex_unlock(&dev_priv->sb_lock);
> -
> -	divider = val & CCK_FREQUENCY_VALUES;
> -
> -	WARN((val & CCK_FREQUENCY_STATUS) !=
> -	     (divider << CCK_FREQUENCY_STATUS_SHIFT),
> -	     "cdclk change in progress\n");
> -
> -	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> +	return vlv_get_cck_clock_hpll(to_i915(dev), "cdclk",
> +				      CCK_DISPLAY_CLOCK_CONTROL);
>  }
>  
>  static int ilk_get_display_clock_speed(struct drm_device *dev)
> @@ -13323,8 +13340,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	intel_update_cdclk(dev);
> -
>  	if (HAS_DDI(dev))
>  		intel_ddi_pll_init(dev);
>  	else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> @@ -14836,6 +14851,9 @@ void intel_modeset_init(struct drm_device *dev)
>  		}
>  	}
>  
> +	intel_update_czclk(dev_priv);
> +	intel_update_cdclk(dev);
> +
>  	intel_shared_dpll_init(dev);
>  
>  	/* Just disable it once at startup */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab5ac5e..896a95c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5482,25 +5482,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>  	mutex_unlock(&dev_priv->sb_lock);
>  
>  	switch ((val >> 2) & 0x7) {
> -	case 0:
> -	case 1:
> -		dev_priv->rps.cz_freq = 200;
> -		dev_priv->mem_freq = 1600;
> -		break;
> -	case 2:
> -		dev_priv->rps.cz_freq = 267;
> -		dev_priv->mem_freq = 1600;
> -		break;
>  	case 3:
> -		dev_priv->rps.cz_freq = 333;
>  		dev_priv->mem_freq = 2000;
>  		break;
> -	case 4:
> -		dev_priv->rps.cz_freq = 320;
> -		dev_priv->mem_freq = 1600;
> -		break;
> -	case 5:
> -		dev_priv->rps.cz_freq = 400;
> +	default:
>  		dev_priv->mem_freq = 1600;
>  		break;
>  	}
> @@ -7327,7 +7312,7 @@ static int vlv_gpu_freq_div(unsigned int czclk_freq)
>  
>  static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  {
> -	int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
> +	int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>  
>  	div = vlv_gpu_freq_div(czclk_freq);
>  	if (div < 0)
> @@ -7338,7 +7323,7 @@ static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  
>  static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  {
> -	int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
> +	int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>  
>  	mul = vlv_gpu_freq_div(czclk_freq);
>  	if (mul < 0)
> @@ -7349,7 +7334,7 @@ static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  
>  static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  {
> -	int div, czclk_freq = dev_priv->rps.cz_freq;
> +	int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>  
>  	div = vlv_gpu_freq_div(czclk_freq) / 2;
>  	if (div < 0)
> @@ -7360,7 +7345,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  
>  static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  {
> -	int mul, czclk_freq = dev_priv->rps.cz_freq;
> +	int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>  
>  	mul = vlv_gpu_freq_div(czclk_freq) / 2;
>  	if (mul < 0)


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux