[PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.

[PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.

v2: Do forcewake before setting idle frequency (ville)
    Update function comments to match the code (Deepak)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
 1 file changed, 11 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..3df929a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
 
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
  *
  * * If Gfx is Idle, then
- * 1. Mask Turbo interrupts
- * 2. Bring up Gfx clock
- * 3. Change the freq to Rpn and wait till P-Unit updates freq
- * 4. Clear the Force GFX CLK ON bit so that Gfx can down
- * 5. Unmask Turbo interrupts
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV and latest VLV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+	/* CHV don't need to force the gfx clock */
+	if (IS_CHERRYVIEW(dev)) {
 		valleyview_set_rps(dev_priv->dev, val);
 		return;
 	}
 
-	/*
-	 * When we are idle.  Drop to min voltage state.
-	 */
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 
-	/* Mask turbo interrupt so that they will not come in between */
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
-	vlv_force_gfx_clock(dev_priv, true);
-
-	dev_priv->rps.cur_freq = val;
-
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
-	gen6_set_rps_thresholds(dev_priv, val);
-	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+	/* Wake up the media well, as that takes a lot less
+	 * power than the Render well. */
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+	valleyview_set_rps(dev_priv->dev, val);
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
 }
 
 void gen6_rps_busy(struct drm_i915_private *dev_priv)
-- 
1.9.1

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

[PATCH v2 2/2] drm/i915/chv: Extend set idle rps wa to chv

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

It is obsered on BSW that requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if we let it enter rc6 with a
high frequency Vnn also remains high. Extending vlv_set_rps_idle()
workaround on CHV/BSW.

suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3df929a..852f756 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev)) {
-		valleyview_set_rps(dev_priv->dev, val);
-		return;
-	}
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 
-- 
1.9.1

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

[PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(

Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 852f756..b6b14a4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
 	return rp1;
 }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
 static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp1;
@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
-- 
1.9.1

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6282
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                 -1              264/264              263/264
BYT                 -4              227/227              223/227
BDW                 -1              318/318              317/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*IVB  igt@gem_storedw_batches_loop@normal      PASS(2)      FAIL(1)PASS(1)
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(18)      TIMEOUT(1)PASS(1)
*BYT  igt@gem_exec_parse@bitmasks      FAIL(1)PASS(7)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
 BYT  igt@gem_pipe_control_store_loop@fresh-buffer      FAIL(1)TIMEOUT(10)PASS(9)      TIMEOUT(2)
*BYT  igt@gem_tiled_pread      FAIL(1)PASS(4)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
*BDW  igt@gem_userptr_blits@forked-sync-swapping-multifd-mempressure-normal      PASS(2)      NO_RESULT(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.

[Ville Syrjälä]

On Fri, May 08, 2015 at 08:43:10PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> When GPU is idle on VLV, Request freq to punit should be good enough to
> get the voltage back to VNN. Also, make sure gfx clock force applies
> before requesting the freq fot vlv.
> 
> v2: Do forcewake before setting idle frequency (ville)
>     Update function comments to match the code (Deepak)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
> suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
>  1 file changed, 11 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a7516ed..3df929a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>  	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
>  }
>  
> -/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
> +/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
>   *
>   * * If Gfx is Idle, then
> - * 1. Mask Turbo interrupts
> - * 2. Bring up Gfx clock
> - * 3. Change the freq to Rpn and wait till P-Unit updates freq
> - * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> - * 5. Unmask Turbo interrupts
> + * 1. Forcewake Media well.
> + * 2. Request idle freq.
> + * 3. Release Forcewake of Media well.
>  */
>  static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	u32 val = dev_priv->rps.idle_freq;
>  
> -	/* CHV and latest VLV don't need to force the gfx clock */
> -	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
> +	/* CHV don't need to force the gfx clock */
> +	if (IS_CHERRYVIEW(dev)) {
>  		valleyview_set_rps(dev_priv->dev, val);
>  		return;
>  	}
>  
> -	/*
> -	 * When we are idle.  Drop to min voltage state.
> -	 */
> -
>  	if (dev_priv->rps.cur_freq <= val)
>  		return;
>  
> -	/* Mask turbo interrupt so that they will not come in between */
> -	I915_WRITE(GEN6_PMINTRMSK,
> -		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
> -
> -	vlv_force_gfx_clock(dev_priv, true);
> -
> -	dev_priv->rps.cur_freq = val;
> -
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> -
> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> -				& GENFREQSTATUS) == 0, 100))
> -		DRM_ERROR("timed out waiting for Punit\n");
> -
> -	gen6_set_rps_thresholds(dev_priv, val);
> -	vlv_force_gfx_clock(dev_priv, false);
> -
> -	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
> +	/* Wake up the media well, as that takes a lot less
> +	 * power than the Render well. */
> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
> +	valleyview_set_rps(dev_priv->dev, val);
> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);

_put

With that fixed this is:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  void gen6_rps_busy(struct drm_i915_private *dev_priv)
> -- 
> 1.9.1

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

Re: [PATCH v2 2/2] drm/i915/chv: Extend set idle rps wa to chv

[Ville Syrjälä]

On Fri, May 08, 2015 at 08:43:11PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> It is obsered on BSW that requesting a new frequency from Punit
> does nothing when the GPU is in rc6, and if we let it enter rc6 with a
> high frequency Vnn also remains high.

I would perhaps rephrase that as "slightly higher than at the
minimum frequency" since it does drop most of the way, at least on
my BSW.

Otherwise this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> Extending vlv_set_rps_idle()
> workaround on CHV/BSW.
> 
> suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3df929a..852f756 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>  */
>  static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_device *dev = dev_priv->dev;
>  	u32 val = dev_priv->rps.idle_freq;
>  
> -	/* CHV don't need to force the gfx clock */
> -	if (IS_CHERRYVIEW(dev)) {
> -		valleyview_set_rps(dev_priv->dev, val);
> -		return;
> -	}
> -
>  	if (dev_priv->rps.cur_freq <= val)
>  		return;
>  
> -- 
> 1.9.1

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Fri, May 08, 2015 at 08:43:12PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> frequency to RPn, punit is failing to change the input voltage to
> minimum :(

As I mentioned I've been unable to reproduce that particular problem
on my BSW. Perhaps add a note about that in the commit message.

> 
> Since Punit validates the rps range [RPe, RP0]. This patch
> removes unused cherryview_rps_min_freq function.

But I can accept that we should stick to the validated range, so I
can slap an r-b on the patch anyway:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> v2: Change commit message
> 
> v3: set min_freq before idle_freq (chris)
> 
> v4: Squash 'Remove unused rps min function' patch
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 852f756..b6b14a4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  	return rp1;
>  }
>  
> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	u32 val, rpn;
> -
> -	if (dev->pdev->revision >= 0x20) {
> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> -		       FB_GFX_FREQ_FUSE_MASK);
> -	} else { /* For pre-production hardware */
> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> -	}
> -
> -	return rpn;
> -}
> -
>  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp1;
> @@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>  			 dev_priv->rps.rp1_freq);
>  
> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> +	/* PUnit validated range is only [RPe, RP0] */
> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -- 
> 1.9.1

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

Re: [PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.

[Deepak S]

On Friday 08 May 2015 10:04 PM, Ville Syrjälä wrote:
> On Fri, May 08, 2015 at 08:43:10PM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> When GPU is idle on VLV, Request freq to punit should be good enough to
>> get the voltage back to VNN. Also, make sure gfx clock force applies
>> before requesting the freq fot vlv.
>>
>> v2: Do forcewake before setting idle frequency (ville)
>>      Update function comments to match the code (Deepak)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
>> suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
>>   1 file changed, 11 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index a7516ed..3df929a 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>>   	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
>>   }
>>   
>> -/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
>> +/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
>>    *
>>    * * If Gfx is Idle, then
>> - * 1. Mask Turbo interrupts
>> - * 2. Bring up Gfx clock
>> - * 3. Change the freq to Rpn and wait till P-Unit updates freq
>> - * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>> - * 5. Unmask Turbo interrupts
>> + * 1. Forcewake Media well.
>> + * 2. Request idle freq.
>> + * 3. Release Forcewake of Media well.
>>   */
>>   static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>   {
>>   	struct drm_device *dev = dev_priv->dev;
>>   	u32 val = dev_priv->rps.idle_freq;
>>   
>> -	/* CHV and latest VLV don't need to force the gfx clock */
>> -	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
>> +	/* CHV don't need to force the gfx clock */
>> +	if (IS_CHERRYVIEW(dev)) {
>>   		valleyview_set_rps(dev_priv->dev, val);
>>   		return;
>>   	}
>>   
>> -	/*
>> -	 * When we are idle.  Drop to min voltage state.
>> -	 */
>> -
>>   	if (dev_priv->rps.cur_freq <= val)
>>   		return;
>>   
>> -	/* Mask turbo interrupt so that they will not come in between */
>> -	I915_WRITE(GEN6_PMINTRMSK,
>> -		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>> -
>> -	vlv_force_gfx_clock(dev_priv, true);
>> -
>> -	dev_priv->rps.cur_freq = val;
>> -
>> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>> -
>> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
>> -				& GENFREQSTATUS) == 0, 100))
>> -		DRM_ERROR("timed out waiting for Punit\n");
>> -
>> -	gen6_set_rps_thresholds(dev_priv, val);
>> -	vlv_force_gfx_clock(dev_priv, false);
>> -
>> -	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
>> +	/* Wake up the media well, as that takes a lot less
>> +	 * power than the Render well. */
>> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
>> +	valleyview_set_rps(dev_priv->dev, val);
>> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
> _put
>
> With that fixed this is:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Oops :)  Copy paste problem :)

>>   }
>>   
>>   void gen6_rps_busy(struct drm_i915_private *dev_priv)
>> -- 
>> 1.9.1

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Deepak S]

On Friday 08 May 2015 10:09 PM, Ville Syrjälä wrote:
> On Fri, May 08, 2015 at 08:43:12PM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
>> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
>> frequency to RPn, punit is failing to change the input voltage to
>> minimum :(
> As I mentioned I've been unable to reproduce that particular problem
> on my BSW. Perhaps add a note about that in the commit message.
>
Issue is Vgg_in voltage not getting dropped :(. Vnn observation is same as what your seeing.
We saw this issue on CHV platform and confirmed by punit team.
Let me update the commit msg.

>> Since Punit validates the rps range [RPe, RP0]. This patch
>> removes unused cherryview_rps_min_freq function.
> But I can accept that we should stick to the validated range, so I
> can slap an r-b on the patch anyway:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>> v2: Change commit message
>>
>> v3: set min_freq before idle_freq (chris)
>>
>> v4: Squash 'Remove unused rps min function' patch
>>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>>   1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 852f756..b6b14a4 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>   	return rp1;
>>   }
>>   
>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
>> -{
>> -	struct drm_device *dev = dev_priv->dev;
>> -	u32 val, rpn;
>> -
>> -	if (dev->pdev->revision >= 0x20) {
>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
>> -		       FB_GFX_FREQ_FUSE_MASK);
>> -	} else { /* For pre-production hardware */
>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
>> -	}
>> -
>> -	return rpn;
>> -}
>> -
>>   static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>   {
>>   	u32 val, rp1;
>> @@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>>   			 dev_priv->rps.rp1_freq);
>>   
>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
>> +	/* PUnit validated range is only [RPe, RP0] */
>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>>   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>>   			 dev_priv->rps.min_freq);
>> -- 
>> 1.9.1

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

Re: [PATCH v2 2/2] drm/i915/chv: Extend set idle rps wa to chv

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6362
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                 -1              302/302              301/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@drv_suspend@fence-restore-tiled2untiled      PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:drm_edid_block_valid[drm]]*ERROR*EDID_checksum_is_invalid,remainder_is@EDID checksum is .* remainder is
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915/vlv: Remove wait for for punit to updates freq.

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.

v2: Do forcewake before setting idle frequency (ville)
    Update function comments to match the code (Deepak)

v3: Fix get/put across idle frequency Request. (Ville)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
 1 file changed, 11 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..82dfdbc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
 
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
  *
  * * If Gfx is Idle, then
- * 1. Mask Turbo interrupts
- * 2. Bring up Gfx clock
- * 3. Change the freq to Rpn and wait till P-Unit updates freq
- * 4. Clear the Force GFX CLK ON bit so that Gfx can down
- * 5. Unmask Turbo interrupts
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV and latest VLV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+	/* CHV don't need to force the gfx clock */
+	if (IS_CHERRYVIEW(dev)) {
 		valleyview_set_rps(dev_priv->dev, val);
 		return;
 	}
 
-	/*
-	 * When we are idle.  Drop to min voltage state.
-	 */
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 
-	/* Mask turbo interrupt so that they will not come in between */
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
-	vlv_force_gfx_clock(dev_priv, true);
-
-	dev_priv->rps.cur_freq = val;
-
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
-	gen6_set_rps_thresholds(dev_priv, val);
-	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+	/* Wake up the media well, as that takes a lot less
+	 * power than the Render well. */
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+	valleyview_set_rps(dev_priv->dev, val);
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA);
 }
 
 void gen6_rps_busy(struct drm_i915_private *dev_priv)
-- 
1.9.1

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

[PATCH v2] drm/i915/chv: Extend set idle rps wa to chv

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

It is observed on BSW that requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if we let GPU enter rc6
with a high frequency, Vnn remains slightly higher than at minimum
frequency. Extending vlv_set_rps_idle() workaround on CHV/BSW.

v2: Update commit msg (Ville)

suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 82dfdbc..064f11a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev)) {
-		valleyview_set_rps(dev_priv->dev, val);
-		return;
-	}
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 
-- 
1.9.1

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

[PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the vgg input voltage to
minimum :(

Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 064f11a..c229d7e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
 	return rp1;
 }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
 static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp1;
@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
-- 
1.9.1

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

Re: [PATCH v3] drm/i915/vlv: Remove wait for for punit to updates freq.

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6370
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Daniel Vetter]

On Sat, May 09, 2015 at 06:15:46PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> frequency to RPn, punit is failing to change the vgg input voltage to
> minimum :(
> 
> Since Punit validates the rps range [RPe, RP0]. This patch
> removes unused cherryview_rps_min_freq function.
> 
> v2: Change commit message
> 
> v3: set min_freq before idle_freq (chris)
> 
> v4: Squash 'Remove unused rps min function' patch
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

All three patches merged to dinq, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 064f11a..c229d7e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  	return rp1;
>  }
>  
> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	u32 val, rpn;
> -
> -	if (dev->pdev->revision >= 0x20) {
> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> -		       FB_GFX_FREQ_FUSE_MASK);
> -	} else { /* For pre-production hardware */
> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> -	}
> -
> -	return rpn;
> -}
> -
>  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp1;
> @@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>  			 dev_priv->rps.rp1_freq);
>  
> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> +	/* PUnit validated range is only [RPe, RP0] */
> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Sat, May 09, 2015 at 11:04:28AM +0530, Deepak S wrote:
> 
> 
> On Friday 08 May 2015 10:09 PM, Ville Syrjälä wrote:
> > On Fri, May 08, 2015 at 08:43:12PM +0530, deepak.s@linux.intel.com wrote:
> >> From: Deepak S <deepak.s@linux.intel.com>
> >>
> >> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> >> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> >> frequency to RPn, punit is failing to change the input voltage to
> >> minimum :(
> > As I mentioned I've been unable to reproduce that particular problem
> > on my BSW. Perhaps add a note about that in the commit message.
> >
> Issue is Vgg_in voltage not getting dropped :(. Vnn observation is same as what your seeing.

I think I was observing Vgg, not Vnn.

> We saw this issue on CHV platform and confirmed by punit team.
> Let me update the commit msg.
> 
> >> Since Punit validates the rps range [RPe, RP0]. This patch
> >> removes unused cherryview_rps_min_freq function.
> > But I can accept that we should stick to the validated range, so I
> > can slap an r-b on the patch anyway:
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >> v2: Change commit message
> >>
> >> v3: set min_freq before idle_freq (chris)
> >>
> >> v4: Squash 'Remove unused rps min function' patch
> >>
> >> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
> >>   1 file changed, 2 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 852f756..b6b14a4 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >>   	return rp1;
> >>   }
> >>   
> >> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> >> -{
> >> -	struct drm_device *dev = dev_priv->dev;
> >> -	u32 val, rpn;
> >> -
> >> -	if (dev->pdev->revision >= 0x20) {
> >> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> >> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> >> -		       FB_GFX_FREQ_FUSE_MASK);
> >> -	} else { /* For pre-production hardware */
> >> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> >> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> >> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> >> -	}
> >> -
> >> -	return rpn;
> >> -}
> >> -
> >>   static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >>   {
> >>   	u32 val, rp1;
> >> @@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> >>   			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
> >>   			 dev_priv->rps.rp1_freq);
> >>   
> >> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> >> +	/* PUnit validated range is only [RPe, RP0] */
> >> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
> >>   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> >>   			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> >>   			 dev_priv->rps.min_freq);
> >> -- 
> >> 1.9.1

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Daniel Vetter]

On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> frequency to RPn, punit is failing to change the input voltage to
> minimum :(
> 
> Since Punit validates the rps range [RPe, RP0]. This patch
> removes unused cherryview_rps_min_freq function.
> 
> v2: Change commit message
> 
> v3: set min_freq before idle_freq (chris)
> 
> v4: Squash 'Remove unused rps min function' patch
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a7516ed..78c89ff 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  	return rp1;
>  }
>  
> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	u32 val, rpn;
> -
> -	if (dev->pdev->revision >= 0x20) {
> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> -		       FB_GFX_FREQ_FUSE_MASK);
> -	} else { /* For pre-production hardware */
> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> -	}
> -
> -	return rpn;
> -}
> -
>  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp1;
> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>  			 dev_priv->rps.rp1_freq);
>  
> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> +	/* PUnit validated range is only [RPe, RP0] */
> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;

Shouldn't we instead just adjust the softlimit instead of the hardlimit?
At least it sounds like this isn't all that clear-cut and maybe we want to
allow userspace to still go below. Similar to how with gpu overclocking we
allow headroom but set the softlimit only to the safe range.
-Daniel

>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Deepak S]

On Thursday 30 April 2015 07:35 PM, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 02:19:07PM +0300, Ville Syrjälä wrote:
>> On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
>>> As you suggested it would be better to extend the VLV WA to
>>> CHV also to make sure we drop the voltage when idle.
>>>
>>> Below is the sequence I think we should follow (based  on your comments).
>>> 1. forcewake power wells
>>> 2. do gfx force clock on
>>> 3. request freq to punit
>>> 4. release gfx force clock on
>>> 5. release forcewake of power wells.
>>>
>>> Please share your thoughts?
>> I'm thinking we shouldn't need the gfx clock force since forcewake
>> should already cause the clock to be enabled.
>>
>> I've also not verified what happens if we drop the forcewake before
>> Punit has actually finished the frequency change. I'll try to hack
>> up some kind of test to see if I can make that happen.
> OK, after some hacking I see that the forcewake is enough, and also
> we don't seem to need to wait for the Punit to finish the frequency
> change before dropping forcewake. It seems to finish the change even
> after forcewake has been dropped.
>
> Also for a bit of extra micro optimization we should perhaps wake up
> the media well only, as that takes a lot less power than the render
> well.

Thanks Ville. I will submit new patch to extend the WA to CHV

Thanks
Deepak

>>> Thanks
>>> Deepak
>>>
>>>>> So based on my tests this patch feels a bit wrong.
>>>>>
>>>>>> Since Punit validates the rps range [RPe, RP0]. This patch
>>>>>> removes unused cherryview_rps_min_freq function.
>>>>>>
>>>>>> v2: Change commit message
>>>>>>
>>>>>> v3: set min_freq before idle_freq (chris)
>>>>>>
>>>>>> v4: Squash 'Remove unused rps min function' patch
>>>>>>
>>>>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>>>>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>>>>>>    1 file changed, 2 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> index a7516ed..78c89ff 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>>>>    	return rp1;
>>>>>>    }
>>>>>>    
>>>>>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
>>>>>> -{
>>>>>> -	struct drm_device *dev = dev_priv->dev;
>>>>>> -	u32 val, rpn;
>>>>>> -
>>>>>> -	if (dev->pdev->revision >= 0x20) {
>>>>>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
>>>>>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
>>>>>> -		       FB_GFX_FREQ_FUSE_MASK);
>>>>>> -	} else { /* For pre-production hardware */
>>>>>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
>>>>>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
>>>>>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
>>>>>> -	}
>>>>>> -
>>>>>> -	return rpn;
>>>>>> -}
>>>>>> -
>>>>>>    static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>>>>    {
>>>>>>    	u32 val, rp1;
>>>>>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>>>>>>    			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>>>>>>    			 dev_priv->rps.rp1_freq);
>>>>>>    
>>>>>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
>>>>>> +	/* PUnit validated range is only [RPe, RP0] */
>>>>>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>>>>>>    	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>>>>>>    			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>>>>>>    			 dev_priv->rps.min_freq);
>>>>>> -- 
>>>>>> 1.9.1
>>>>> -- 
>>>>> Ville Syrjälä
>>>>> Intel OTC
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Ville Syrjälä
>> Intel OTC

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 02:19:07PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
> > As you suggested it would be better to extend the VLV WA to
> > CHV also to make sure we drop the voltage when idle.
> > 
> > Below is the sequence I think we should follow (based  on your comments).
> > 1. forcewake power wells
> > 2. do gfx force clock on
> > 3. request freq to punit
> > 4. release gfx force clock on
> > 5. release forcewake of power wells.
> > 
> > Please share your thoughts?
> 
> I'm thinking we shouldn't need the gfx clock force since forcewake
> should already cause the clock to be enabled.
> 
> I've also not verified what happens if we drop the forcewake before
> Punit has actually finished the frequency change. I'll try to hack
> up some kind of test to see if I can make that happen.

OK, after some hacking I see that the forcewake is enough, and also
we don't seem to need to wait for the Punit to finish the frequency
change before dropping forcewake. It seems to finish the change even
after forcewake has been dropped.

Also for a bit of extra micro optimization we should perhaps wake up
the media well only, as that takes a lot less power than the render
well.

> 
> > 
> > Thanks
> > Deepak
> > 
> > >> So based on my tests this patch feels a bit wrong.
> > >>
> > >>> Since Punit validates the rps range [RPe, RP0]. This patch
> > >>> removes unused cherryview_rps_min_freq function.
> > >>>
> > >>> v2: Change commit message
> > >>>
> > >>> v3: set min_freq before idle_freq (chris)
> > >>>
> > >>> v4: Squash 'Remove unused rps min function' patch
> > >>>
> > >>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> > >>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >>> ---
> > >>>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
> > >>>   1 file changed, 2 insertions(+), 19 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > >>> index a7516ed..78c89ff 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_pm.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> > >>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
> > >>>   	return rp1;
> > >>>   }
> > >>>   
> > >>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> > >>> -{
> > >>> -	struct drm_device *dev = dev_priv->dev;
> > >>> -	u32 val, rpn;
> > >>> -
> > >>> -	if (dev->pdev->revision >= 0x20) {
> > >>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> > >>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> > >>> -		       FB_GFX_FREQ_FUSE_MASK);
> > >>> -	} else { /* For pre-production hardware */
> > >>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> > >>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> > >>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> > >>> -	}
> > >>> -
> > >>> -	return rpn;
> > >>> -}
> > >>> -
> > >>>   static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
> > >>>   {
> > >>>   	u32 val, rp1;
> > >>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> > >>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
> > >>>   			 dev_priv->rps.rp1_freq);
> > >>>   
> > >>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> > >>> +	/* PUnit validated range is only [RPe, RP0] */
> > >>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
> > >>>   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> > >>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> > >>>   			 dev_priv->rps.min_freq);
> > >>> -- 
> > >>> 1.9.1
> > >> -- 
> > >> Ville Syrjälä
> > >> Intel OTC
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
> 
> 
> On Thursday 30 April 2015 01:23 AM, Ville Syrjälä wrote:
> > On Wed, Apr 29, 2015 at 06:31:56PM +0300, Ville Syrjälä wrote:
> >> On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepak.s@linux.intel.com wrote:
> >>> From: Deepak S <deepak.s@linux.intel.com>
> >>>
> >>> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> >>> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> >>> frequency to RPn, punit is failing to change the input voltage to
> >>> minimum :(
> >> So far I can't reproduce this problem on my BSW. In fact what I see
> >> that the voltage at RPn is lower than the voltage at RPe, even while
> >> we're in rc6.
> >>
> >> without forcewake:
> >> RPn -> 0x66
> >> RPe -> 0x67
> >> RP0 -> 0x69
> >>
> >> with forcewake:
> >> RPn -> 0x66
> >> RPe -> 0x76
> >> RP0 -> 0x9d
> >>
> >> Also asking Punit to change the frequency after the GPU has gone to
> >> rc6 does absolutely nothing (remind anyone of VLV?). I think I need to
> >> retest my VLV C0 to see if my earlier observations there were accurate.
> >> The shared Vnn rail does make it harder to observe this stuff on
> >> VLV though.
> > I went back to my VLVs (had a B3 and C0 actually). And I'm seeing the
> > exact same behaviour on both, ie. requesting a new frequency from Punit
> > does nothing when the GPU is in rc6, and if I let it enter rc6 with a
> > high frequency Vnn also remains high. Previously I had thought that C0
> > fixed this, but now it definitely shows the same problem here. I must
> > have had some accidental forcewake somewhere when I originally tested
> > it,
> >
> > So based on that, your other patch to remove the stepping check from
> > vlv_set_rps_idle() is in fact correct.
> >
> > The question remains however what should we do with CHV. According to my
> > testing to get the minimum voltage we should keep RPn around, and we
> > should also do the vlv_set_rps_idle() workaround on CHV.
> >
> > Oh and I also observed something else on VLV. Normally when entering rc6
> > the GPLL ref clock gets trunk gated at CCK (by Punit I assume). However
> > when using the vlv_gfx_clock_force() that doesn't happen. So I'm not
> > entirely sure the GPLL gets turned off properly in that case. Maybe we
> > should just use forcewake instead? Oh and BTW, CHV doesn't do the trunk
> > gating in either case. I'm not sure where to check if the GPLL is
> > actually running or not.
> >
> Hi Ville,
> 
> Thanks Ville for verifying on VLV and CHV. Its interesting to see when Idle,
> Voltage not dropping to Vnn on CHV :( This was supposed to be fixed
> in BSW/CHV :(.

Well it does drop, but not quite all the way if the current
frequency is above RPn.

I just tried to look at the energy numbers from RAPL a bit. Based
on that the difference in Vgg when forcewake is off is rather
insignificant, or the Vgg portion of the always-on well is so small
that it doesn't really matter.

With forcewake enabled I see a clear difference between RPn and
RPe though. But that might mostly matter for cases where would grab
forcewake without actually feeding any work to the GPU. That might
be rare enough to not make much of a difference in practice.

> As you suggested it would be better to extend the VLV WA to
> CHV also to make sure we drop the voltage when idle.
> 
> Below is the sequence I think we should follow (based  on your comments).
> 1. forcewake power wells
> 2. do gfx force clock on
> 3. request freq to punit
> 4. release gfx force clock on
> 5. release forcewake of power wells.
> 
> Please share your thoughts?

I'm thinking we shouldn't need the gfx clock force since forcewake
should already cause the clock to be enabled.

I've also not verified what happens if we drop the forcewake before
Punit has actually finished the frequency change. I'll try to hack
up some kind of test to see if I can make that happen.

> 
> Thanks
> Deepak
> 
> >> So based on my tests this patch feels a bit wrong.
> >>
> >>> Since Punit validates the rps range [RPe, RP0]. This patch
> >>> removes unused cherryview_rps_min_freq function.
> >>>
> >>> v2: Change commit message
> >>>
> >>> v3: set min_freq before idle_freq (chris)
> >>>
> >>> v4: Squash 'Remove unused rps min function' patch
> >>>
> >>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> ---
> >>>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
> >>>   1 file changed, 2 insertions(+), 19 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>> index a7516ed..78c89ff 100644
> >>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >>>   	return rp1;
> >>>   }
> >>>   
> >>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> >>> -{
> >>> -	struct drm_device *dev = dev_priv->dev;
> >>> -	u32 val, rpn;
> >>> -
> >>> -	if (dev->pdev->revision >= 0x20) {
> >>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> >>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> >>> -		       FB_GFX_FREQ_FUSE_MASK);
> >>> -	} else { /* For pre-production hardware */
> >>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> >>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> >>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> >>> -	}
> >>> -
> >>> -	return rpn;
> >>> -}
> >>> -
> >>>   static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >>>   {
> >>>   	u32 val, rp1;
> >>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> >>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
> >>>   			 dev_priv->rps.rp1_freq);
> >>>   
> >>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> >>> +	/* PUnit validated range is only [RPe, RP0] */
> >>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
> >>>   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> >>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> >>>   			 dev_priv->rps.min_freq);
> >>> -- 
> >>> 1.9.1
> >> -- 
> >> Ville Syrjälä
> >> Intel OTC
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Deepak S]

On Thursday 30 April 2015 01:23 AM, Ville Syrjälä wrote:
> On Wed, Apr 29, 2015 at 06:31:56PM +0300, Ville Syrjälä wrote:
>> On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepak.s@linux.intel.com wrote:
>>> From: Deepak S <deepak.s@linux.intel.com>
>>>
>>> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
>>> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
>>> frequency to RPn, punit is failing to change the input voltage to
>>> minimum :(
>> So far I can't reproduce this problem on my BSW. In fact what I see
>> that the voltage at RPn is lower than the voltage at RPe, even while
>> we're in rc6.
>>
>> without forcewake:
>> RPn -> 0x66
>> RPe -> 0x67
>> RP0 -> 0x69
>>
>> with forcewake:
>> RPn -> 0x66
>> RPe -> 0x76
>> RP0 -> 0x9d
>>
>> Also asking Punit to change the frequency after the GPU has gone to
>> rc6 does absolutely nothing (remind anyone of VLV?). I think I need to
>> retest my VLV C0 to see if my earlier observations there were accurate.
>> The shared Vnn rail does make it harder to observe this stuff on
>> VLV though.
> I went back to my VLVs (had a B3 and C0 actually). And I'm seeing the
> exact same behaviour on both, ie. requesting a new frequency from Punit
> does nothing when the GPU is in rc6, and if I let it enter rc6 with a
> high frequency Vnn also remains high. Previously I had thought that C0
> fixed this, but now it definitely shows the same problem here. I must
> have had some accidental forcewake somewhere when I originally tested
> it,
>
> So based on that, your other patch to remove the stepping check from
> vlv_set_rps_idle() is in fact correct.
>
> The question remains however what should we do with CHV. According to my
> testing to get the minimum voltage we should keep RPn around, and we
> should also do the vlv_set_rps_idle() workaround on CHV.
>
> Oh and I also observed something else on VLV. Normally when entering rc6
> the GPLL ref clock gets trunk gated at CCK (by Punit I assume). However
> when using the vlv_gfx_clock_force() that doesn't happen. So I'm not
> entirely sure the GPLL gets turned off properly in that case. Maybe we
> should just use forcewake instead? Oh and BTW, CHV doesn't do the trunk
> gating in either case. I'm not sure where to check if the GPLL is
> actually running or not.
>
Hi Ville,

Thanks Ville for verifying on VLV and CHV. Its interesting to see when Idle,
Voltage not dropping to Vnn on CHV :( This was supposed to be fixed
in BSW/CHV :(. As you suggested it would be better to extend the VLV WA to
CHV also to make sure we drop the voltage when idle.

Below is the sequence I think we should follow (based  on your comments).
1. forcewake power wells
2. do gfx force clock on
3. request freq to punit
4. release gfx force clock on
5. release forcewake of power wells.

Please share your thoughts?

Thanks
Deepak

>> So based on my tests this patch feels a bit wrong.
>>
>>> Since Punit validates the rps range [RPe, RP0]. This patch
>>> removes unused cherryview_rps_min_freq function.
>>>
>>> v2: Change commit message
>>>
>>> v3: set min_freq before idle_freq (chris)
>>>
>>> v4: Squash 'Remove unused rps min function' patch
>>>
>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>>>   1 file changed, 2 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index a7516ed..78c89ff 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>   	return rp1;
>>>   }
>>>   
>>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
>>> -{
>>> -	struct drm_device *dev = dev_priv->dev;
>>> -	u32 val, rpn;
>>> -
>>> -	if (dev->pdev->revision >= 0x20) {
>>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
>>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
>>> -		       FB_GFX_FREQ_FUSE_MASK);
>>> -	} else { /* For pre-production hardware */
>>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
>>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
>>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
>>> -	}
>>> -
>>> -	return rpn;
>>> -}
>>> -
>>>   static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>   {
>>>   	u32 val, rp1;
>>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>>>   			 dev_priv->rps.rp1_freq);
>>>   
>>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
>>> +	/* PUnit validated range is only [RPe, RP0] */
>>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>>>   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>>>   			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>>>   			 dev_priv->rps.min_freq);
>>> -- 
>>> 1.9.1
>> -- 
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6282
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                 -1              264/264              263/264
BYT                 -4              227/227              223/227
BDW                 -1              318/318              317/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*IVB  igt@gem_storedw_batches_loop@normal      PASS(2)      FAIL(1)PASS(1)
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(6)      TIMEOUT(1)PASS(1)
*BYT  igt@gem_exec_parse@bitmasks      FAIL(1)PASS(4)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
 BYT  igt@gem_pipe_control_store_loop@fresh-buffer      FAIL(1)TIMEOUT(4)PASS(3)      TIMEOUT(2)
*BYT  igt@gem_tiled_pread      FAIL(1)PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
*BDW  igt@gem_userptr_blits@forked-sync-swapping-multifd-mempressure-normal      PASS(2)      NO_RESULT(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Wed, Apr 29, 2015 at 06:31:56PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepak.s@linux.intel.com wrote:
> > From: Deepak S <deepak.s@linux.intel.com>
> > 
> > After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> > Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> > frequency to RPn, punit is failing to change the input voltage to
> > minimum :(
> 
> So far I can't reproduce this problem on my BSW. In fact what I see
> that the voltage at RPn is lower than the voltage at RPe, even while
> we're in rc6.
> 
> without forcewake:
> RPn -> 0x66
> RPe -> 0x67
> RP0 -> 0x69
> 
> with forcewake:
> RPn -> 0x66
> RPe -> 0x76
> RP0 -> 0x9d
> 
> Also asking Punit to change the frequency after the GPU has gone to
> rc6 does absolutely nothing (remind anyone of VLV?). I think I need to
> retest my VLV C0 to see if my earlier observations there were accurate.
> The shared Vnn rail does make it harder to observe this stuff on
> VLV though.

I went back to my VLVs (had a B3 and C0 actually). And I'm seeing the
exact same behaviour on both, ie. requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if I let it enter rc6 with a
high frequency Vnn also remains high. Previously I had thought that C0
fixed this, but now it definitely shows the same problem here. I must
have had some accidental forcewake somewhere when I originally tested
it,

So based on that, your other patch to remove the stepping check from
vlv_set_rps_idle() is in fact correct.

The question remains however what should we do with CHV. According to my
testing to get the minimum voltage we should keep RPn around, and we
should also do the vlv_set_rps_idle() workaround on CHV.

Oh and I also observed something else on VLV. Normally when entering rc6
the GPLL ref clock gets trunk gated at CCK (by Punit I assume). However
when using the vlv_gfx_clock_force() that doesn't happen. So I'm not
entirely sure the GPLL gets turned off properly in that case. Maybe we
should just use forcewake instead? Oh and BTW, CHV doesn't do the trunk
gating in either case. I'm not sure where to check if the GPLL is
actually running or not.

> 
> So based on my tests this patch feels a bit wrong.
> 
> > 
> > Since Punit validates the rps range [RPe, RP0]. This patch
> > removes unused cherryview_rps_min_freq function.
> > 
> > v2: Change commit message
> > 
> > v3: set min_freq before idle_freq (chris)
> > 
> > v4: Squash 'Remove unused rps min function' patch
> > 
> > Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
> >  1 file changed, 2 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index a7516ed..78c89ff 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >  	return rp1;
> >  }
> >  
> > -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> > -{
> > -	struct drm_device *dev = dev_priv->dev;
> > -	u32 val, rpn;
> > -
> > -	if (dev->pdev->revision >= 0x20) {
> > -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> > -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> > -		       FB_GFX_FREQ_FUSE_MASK);
> > -	} else { /* For pre-production hardware */
> > -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> > -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> > -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> > -	}
> > -
> > -	return rpn;
> > -}
> > -
> >  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rp1;
> > @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> >  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
> >  			 dev_priv->rps.rp1_freq);
> >  
> > -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> > +	/* PUnit validated range is only [RPe, RP0] */
> > +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
> >  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> >  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> >  			 dev_priv->rps.min_freq);
> > -- 
> > 1.9.1
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Ville Syrjälä]

On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
> Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
> frequency to RPn, punit is failing to change the input voltage to
> minimum :(

So far I can't reproduce this problem on my BSW. In fact what I see
that the voltage at RPn is lower than the voltage at RPe, even while
we're in rc6.

without forcewake:
RPn -> 0x66
RPe -> 0x67
RP0 -> 0x69

with forcewake:
RPn -> 0x66
RPe -> 0x76
RP0 -> 0x9d

Also asking Punit to change the frequency after the GPU has gone to
rc6 does absolutely nothing (remind anyone of VLV?). I think I need to
retest my VLV C0 to see if my earlier observations there were accurate.
The shared Vnn rail does make it harder to observe this stuff on
VLV though.

So based on my tests this patch feels a bit wrong.

> 
> Since Punit validates the rps range [RPe, RP0]. This patch
> removes unused cherryview_rps_min_freq function.
> 
> v2: Change commit message
> 
> v3: set min_freq before idle_freq (chris)
> 
> v4: Squash 'Remove unused rps min function' patch
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a7516ed..78c89ff 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  	return rp1;
>  }
>  
> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	u32 val, rpn;
> -
> -	if (dev->pdev->revision >= 0x20) {
> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> -		       FB_GFX_FREQ_FUSE_MASK);
> -	} else { /* For pre-production hardware */
> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> -	}
> -
> -	return rpn;
> -}
> -
>  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp1;
> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>  			 dev_priv->rps.rp1_freq);
>  
> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> +	/* PUnit validated range is only [RPe, RP0] */
> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -- 
> 1.9.1

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

[PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(

Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..78c89ff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
 	return rp1;
 }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
 static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp1;
@@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
-- 
1.9.1

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