[v5,1/8] drm/i915: Cache current cdclk frequency in dev_priv

Submitted by Mika Kahola on June 2, 2015, 7:20 a.m.

Details

Message ID 1433229666-3152-2-git-send-email-mika.kahola@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Mika Kahola June 2, 2015, 7:20 a.m.
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather that extracting the current cdclk freuqncy every time someone
wants to know it, cache the current value and use that. VLV/CHV already
stored a cached value there so just expand that to cover all platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

v2: Rebased to the latest
v3: Rebased to the latest
v4: Rebased to the latest

Signed-off-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 16e159d..4dd00a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5747,7 +5747,7 @@  static int valleyview_get_vco(struct drm_i915_private *dev_priv)
 	return vco_freq[hpll_freq] * 1000;
 }
 
-static void vlv_update_cdclk(struct drm_device *dev)
+static void intel_update_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -5760,7 +5760,14 @@  static void vlv_update_cdclk(struct drm_device *dev)
 	 * BSpec erroneously claims we should aim for 4MHz, but
 	 * in fact 1MHz is the correct frequency.
 	 */
-	I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
+	if (IS_VALLEYVIEW(dev)) {
+		/*
+		 * Program the gmbus_freq based on the cdclk frequency.
+		 * BSpec erroneously claims we should aim for 4MHz, but
+		 * in fact 1MHz is the correct frequency.
+		 */
+		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
+	}
 }
 
 /* Adjust CDclk dividers to allow high res or save power if possible */
@@ -5826,7 +5833,7 @@  static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 
 	mutex_unlock(&dev_priv->sb_lock);
 
-	vlv_update_cdclk(dev);
+	intel_update_cdclk(dev);
 }
 
 static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
@@ -5867,7 +5874,7 @@  static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	vlv_update_cdclk(dev);
+	intel_update_cdclk(dev);
 }
 
 static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
@@ -6056,6 +6063,8 @@  static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
 	}
+
+	intel_update_cdclk(dev);
 }
 
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
@@ -9479,6 +9488,7 @@  static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	}
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+	intel_update_cdclk(dev_priv->dev);
 }
 
 /*
@@ -13273,6 +13283,8 @@  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))
@@ -14848,13 +14860,9 @@  static void i915_disable_vga(struct drm_device *dev)
 
 void intel_modeset_init_hw(struct drm_device *dev)
 {
+	intel_update_cdclk(dev);
 	intel_prepare_ddi(dev);
-
-	if (IS_VALLEYVIEW(dev))
-		vlv_update_cdclk(dev);
-
 	intel_init_clock_gating(dev);
-
 	intel_enable_gt_powersave(dev);
 }
 

Comments

On Tue, 02 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather that extracting the current cdclk freuqncy every time someone
> wants to know it, cache the current value and use that. VLV/CHV already
> stored a cached value there so just expand that to cover all platforms.

All of the below:

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> v2: Rebased to the latest
> v3: Rebased to the latest
> v4: Rebased to the latest
>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>

Should be turned into:

> v2: Rebased to the latest
> v3: Rebased to the latest
> v4: Rebased to the latest
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

to conform to existing style. See git log for examples.

Damien had a comment to the previous version of this patch [1], but I
don't see it addressed either in the patch or in reply to his mail. You
should do one or the other.

BR,
Jani.


[1] http://mid.gmane.org/20150528182433.GJ24676@strange.ger.corp.intel.com



> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 16e159d..4dd00a5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5747,7 +5747,7 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  	return vco_freq[hpll_freq] * 1000;
>  }
>  
> -static void vlv_update_cdclk(struct drm_device *dev)
> +static void intel_update_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> @@ -5760,7 +5760,14 @@ static void vlv_update_cdclk(struct drm_device *dev)
>  	 * BSpec erroneously claims we should aim for 4MHz, but
>  	 * in fact 1MHz is the correct frequency.
>  	 */
> -	I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> +	if (IS_VALLEYVIEW(dev)) {
> +		/*
> +		 * Program the gmbus_freq based on the cdclk frequency.
> +		 * BSpec erroneously claims we should aim for 4MHz, but
> +		 * in fact 1MHz is the correct frequency.
> +		 */
> +		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> +	}
>  }
>  
>  /* Adjust CDclk dividers to allow high res or save power if possible */
> @@ -5826,7 +5833,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  
>  	mutex_unlock(&dev_priv->sb_lock);
>  
> -	vlv_update_cdclk(dev);
> +	intel_update_cdclk(dev);
>  }
>  
>  static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
> @@ -5867,7 +5874,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
>  	}
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	vlv_update_cdclk(dev);
> +	intel_update_cdclk(dev);
>  }
>  
>  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> @@ -6056,6 +6063,8 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
>  	}
> +
> +	intel_update_cdclk(dev);
>  }
>  
>  static void valleyview_crtc_enable(struct drm_crtc *crtc)
> @@ -9479,6 +9488,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>  	}
>  
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +	intel_update_cdclk(dev_priv->dev);
>  }
>  
>  /*
> @@ -13273,6 +13283,8 @@ 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))
> @@ -14848,13 +14860,9 @@ static void i915_disable_vga(struct drm_device *dev)
>  
>  void intel_modeset_init_hw(struct drm_device *dev)
>  {
> +	intel_update_cdclk(dev);
>  	intel_prepare_ddi(dev);
> -
> -	if (IS_VALLEYVIEW(dev))
> -		vlv_update_cdclk(dev);
> -
>  	intel_init_clock_gating(dev);
> -
>  	intel_enable_gt_powersave(dev);
>  }
>  
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2015-06-02 at 18:17 +0300, Jani Nikula wrote:
> On Tue, 02 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rather that extracting the current cdclk freuqncy every time someone
> > wants to know it, cache the current value and use that. VLV/CHV already
> > stored a cached value there so just expand that to cover all platforms.
> 
> All of the below:
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > v2: Rebased to the latest
> > v3: Rebased to the latest
> > v4: Rebased to the latest
> >
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >
> > Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Should be turned into:
> 
> > v2: Rebased to the latest
> > v3: Rebased to the latest
> > v4: Rebased to the latest
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> 
> to conform to existing style. See git log for examples.
> 
> Damien had a comment to the previous version of this patch [1], but I
> don't see it addressed either in the patch or in reply to his mail. You
> should do one or the other.
> 
> BR,
> Jani.
> 
> 
> [1] http://mid.gmane.org/20150528182433.GJ24676@strange.ger.corp.intel.com
> 
Ok. I'll revise the patches and resend them.

Cheers,
Mika

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++---------
> >  1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 16e159d..4dd00a5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5747,7 +5747,7 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> >  	return vco_freq[hpll_freq] * 1000;
> >  }
> >  
> > -static void vlv_update_cdclk(struct drm_device *dev)
> > +static void intel_update_cdclk(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > @@ -5760,7 +5760,14 @@ static void vlv_update_cdclk(struct drm_device *dev)
> >  	 * BSpec erroneously claims we should aim for 4MHz, but
> >  	 * in fact 1MHz is the correct frequency.
> >  	 */
> > -	I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> > +	if (IS_VALLEYVIEW(dev)) {
> > +		/*
> > +		 * Program the gmbus_freq based on the cdclk frequency.
> > +		 * BSpec erroneously claims we should aim for 4MHz, but
> > +		 * in fact 1MHz is the correct frequency.
> > +		 */
> > +		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> > +	}
> >  }
> >  
> >  /* Adjust CDclk dividers to allow high res or save power if possible */
> > @@ -5826,7 +5833,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> >  
> >  	mutex_unlock(&dev_priv->sb_lock);
> >  
> > -	vlv_update_cdclk(dev);
> > +	intel_update_cdclk(dev);
> >  }
> >  
> >  static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
> > @@ -5867,7 +5874,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
> >  	}
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> > -	vlv_update_cdclk(dev);
> > +	intel_update_cdclk(dev);
> >  }
> >  
> >  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> > @@ -6056,6 +6063,8 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
> >  
> >  		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> >  	}
> > +
> > +	intel_update_cdclk(dev);
> >  }
> >  
> >  static void valleyview_crtc_enable(struct drm_crtc *crtc)
> > @@ -9479,6 +9488,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
> >  	}
> >  
> >  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> > +	intel_update_cdclk(dev_priv->dev);
> >  }
> >  
> >  /*
> > @@ -13273,6 +13283,8 @@ 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))
> > @@ -14848,13 +14860,9 @@ static void i915_disable_vga(struct drm_device *dev)
> >  
> >  void intel_modeset_init_hw(struct drm_device *dev)
> >  {
> > +	intel_update_cdclk(dev);
> >  	intel_prepare_ddi(dev);
> > -
> > -	if (IS_VALLEYVIEW(dev))
> > -		vlv_update_cdclk(dev);
> > -
> >  	intel_init_clock_gating(dev);
> > -
> >  	intel_enable_gt_powersave(dev);
> >  }
> >  
> > -- 
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>