[PATCH v3 0/5] CHV PM fix & Improvements

[PATCH v3 0/5] CHV PM fix & Improvements

[deepak.s]

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

Adding few of PM fixes and Improvements for CHV/VLV.
Addressed few comments.

Deepak S (5):
  drm/i915/chv: Remove Wait for a previous gfx force-off
  drm/i915: Re-adjusting rc6 promotional timer for chv
  drm/i915/chv: Set min freq to efficient frequency on chv
  drm/i915/chv: Remove unused rps min function
  drm/i915: Setup static bias for GPU

 drivers/gpu/drm/i915/i915_drv.c |  6 ++++--
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 37 ++++++++++++++++---------------------
 3 files changed, 25 insertions(+), 23 deletions(-)

-- 
1.9.1

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

[PATCH v3 1/5] drm/i915/chv: Remove Wait for a previous gfx force-off

[deepak.s]

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

On CHV, PUNIT team confirmed that 'VLV_GFX_CLK_STATUS_BIT' is not a
sticky bit and it will always be set. So ignore Check for previous
Gfx force off during suspend and allow the force clk as part S0ix
Sequence

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 82f8be4..182d6a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1194,11 +1194,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 	int err;
 
 	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
-	WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
 
 #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
 	/* Wait for a previous force-off to settle */
-	if (force_on) {
+	if (force_on && !IS_CHERRYVIEW(dev_priv->dev)) {
+		/* WARN_ON only for the Valleyview */
+		WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
+
 		err = wait_for(!COND, 20);
 		if (err) {
 			DRM_ERROR("timeout waiting for GFX clock force-off (%08x)\n",
-- 
1.9.1

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

Re: [PATCH v3 1/5] drm/i915/chv: Remove Wait for a previous gfx force-off

[Ville Syrjälä]

On Sat, Mar 28, 2015 at 03:23:34PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> On CHV, PUNIT team confirmed that 'VLV_GFX_CLK_STATUS_BIT' is not a
> sticky bit and it will always be set. So ignore Check for previous
> Gfx force off during suspend and allow the force clk as part S0ix
> Sequence
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>

Yeah seems OK. We still do the "allow wake" dance even though we skip
the gunit register save, so I guess the force gfx clock makes sense as
part of that.

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 82f8be4..182d6a7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1194,11 +1194,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  	int err;
>  
>  	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
> -	WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
>  
>  #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
>  	/* Wait for a previous force-off to settle */
> -	if (force_on) {
> +	if (force_on && !IS_CHERRYVIEW(dev_priv->dev)) {
> +		/* WARN_ON only for the Valleyview */
> +		WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
> +
>  		err = wait_for(!COND, 20);
>  		if (err) {
>  			DRM_ERROR("timeout waiting for GFX clock force-off (%08x)\n",
> -- 
> 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 v3 1/5] drm/i915/chv: Remove Wait for a previous gfx force-off

[Deepak S]

On Monday 30 March 2015 03:37 PM, Ville Syrjälä wrote:
> On Sat, Mar 28, 2015 at 03:23:34PM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> On CHV, PUNIT team confirmed that 'VLV_GFX_CLK_STATUS_BIT' is not a
>> sticky bit and it will always be set. So ignore Check for previous
>> Gfx force off during suspend and allow the force clk as part S0ix
>> Sequence
>>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Yeah seems OK. We still do the "allow wake" dance even though we skip
> the gunit register save, so I guess the force gfx clock makes sense as
> part of that.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks Ville for reviewing.

>> ---
>>   drivers/gpu/drm/i915/i915_drv.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 82f8be4..182d6a7 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1194,11 +1194,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>>   	int err;
>>   
>>   	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
>> -	WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
>>   
>>   #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
>>   	/* Wait for a previous force-off to settle */
>> -	if (force_on) {
>> +	if (force_on && !IS_CHERRYVIEW(dev_priv->dev)) {
>> +		/* WARN_ON only for the Valleyview */
>> +		WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
>> +
>>   		err = wait_for(!COND, 20);
>>   		if (err) {
>>   			DRM_ERROR("timeout waiting for GFX clock force-off (%08x)\n",
>> -- 
>> 1.9.1

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

Re: [PATCH v3 1/5] drm/i915/chv: Remove Wait for a previous gfx force-off

[Daniel Vetter]

On Mon, Mar 30, 2015 at 01:07:21PM +0300, Ville Syrjälä wrote:
> On Sat, Mar 28, 2015 at 03:23:34PM +0530, deepak.s@linux.intel.com wrote:
> > From: Deepak S <deepak.s@linux.intel.com>
> > 
> > On CHV, PUNIT team confirmed that 'VLV_GFX_CLK_STATUS_BIT' is not a
> > sticky bit and it will always be set. So ignore Check for previous
> > Gfx force off during suspend and allow the force clk as part S0ix
> > Sequence
> > 
> > Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> 
> Yeah seems OK. We still do the "allow wake" dance even though we skip
> the gunit register save, so I guess the force gfx clock makes sense as
> part of that.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 82f8be4..182d6a7 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1194,11 +1194,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
> >  	int err;
> >  
> >  	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
> > -	WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
> >  
> >  #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
> >  	/* Wait for a previous force-off to settle */
> > -	if (force_on) {
> > +	if (force_on && !IS_CHERRYVIEW(dev_priv->dev)) {
> > +		/* WARN_ON only for the Valleyview */
> > +		WARN_ON(!!(val & VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
> > +
> >  		err = wait_for(!COND, 20);
> >  		if (err) {
> >  			DRM_ERROR("timeout waiting for GFX clock force-off (%08x)\n",
> > -- 
> > 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

-- 
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

[PATCH v3 2/5] drm/i915: Re-adjusting rc6 promotional timer for chv

[deepak.s]

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

After feedback from the hardware team we are changing the RC6
promotional timer to increase the power saving without
changing performance.

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fa4ccb3..44428e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4996,8 +4996,8 @@ static void cherryview_enable_rps(struct drm_device *dev)
 		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
 	I915_WRITE(GEN6_RC_SLEEP, 0);
 
-	/* TO threshold set to 1750 us ( 0x557 * 1.28 us) */
-	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
+	/* TO threshold set to 500 us ( 0x186 * 1.28 us) */
+	I915_WRITE(GEN6_RC6_THRESHOLD, 0x186);
 
 	/* allows RC6 residency counter to work */
 	I915_WRITE(VLV_COUNTER_CONTROL,
-- 
1.9.1

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

Re: [PATCH v3 2/5] drm/i915: Re-adjusting rc6 promotional timer for chv

[Daniel Vetter]

On Sat, Mar 28, 2015 at 03:23:35PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> After feedback from the hardware team we are changing the RC6
> promotional timer to increase the power saving without
> changing performance.
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fa4ccb3..44428e4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4996,8 +4996,8 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>  	I915_WRITE(GEN6_RC_SLEEP, 0);
>  
> -	/* TO threshold set to 1750 us ( 0x557 * 1.28 us) */
> -	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
> +	/* TO threshold set to 500 us ( 0x186 * 1.28 us) */
> +	I915_WRITE(GEN6_RC6_THRESHOLD, 0x186);
>  
>  	/* allows RC6 residency counter to work */
>  	I915_WRITE(VLV_COUNTER_CONTROL,
> -- 
> 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

[PATCH v3 3/5] 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 :(

v2: Change commit message

v3: set min_freq before idle_freq (chris)

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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 44428e4..2f7d2e0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4934,7 +4934,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

[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

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

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

[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

[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

[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

[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

[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

[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

[PATCH v3 4/5] drm/i915/chv: Remove unused rps min function

[deepak.s]

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

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

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2f7d2e0..88e71a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4665,24 +4665,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;
-- 
1.9.1

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

[PATCH v3 5/5] drm/i915: Setup static bias for GPU

[deepak.s]

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

Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..ea708ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,11 @@ enum skl_disp_power_wells {
 #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
 #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
 
+#define VLV_IOSFB_RPS_OVERRIDE	0x04
+#define VLV_OVERRIDE_RPS_MASK	1
+#define VLV_ENABLE_BIAS_SHARE	(1 << 1)
+#define VLV_BIAS_VAL	(6 << 2)
+
 #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
 #define VLV_RP_UP_EI_THRESHOLD			90
 #define VLV_RP_DOWN_EI_THRESHOLD		70
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 88e71a3..673612b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5015,6 +5015,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_RPS_MASK |
+		  VLV_ENABLE_BIAS_SHARE |
+		  VLV_BIAS_VAL;
+	vlv_punit_write(dev_priv, VLV_IOSFB_RPS_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
@@ -5099,6 +5105,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_RPS_MASK |
+		  VLV_ENABLE_BIAS_SHARE |
+		  VLV_BIAS_VAL;
+	vlv_punit_write(dev_priv, VLV_IOSFB_RPS_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
-- 
1.9.1

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

Re: [PATCH v3 5/5] drm/i915: Setup static bias for GPU

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6085
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              270/270              269/270
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  337/337              337/337
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  309/309              309/309
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt@gem_tiled_pread_pwrite      PASS(2)      FAIL(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 v3 5/5] drm/i915: Setup static bias for GPU

[Ville Syrjälä]

On Sat, Mar 28, 2015 at 03:23:38PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Based on the spec, Setting up static BIAS for GPU to improve the
> rps performace.

My understanding is that this would only improve the GPU performance
under TDP constrained use cases, at the cost of CPU performance. So
it's not some magic bullet that makes things just faster (tm).

> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b522eb6..ea708ba 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -670,6 +670,11 @@ enum skl_disp_power_wells {
>  #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>  
> +#define VLV_IOSFB_RPS_OVERRIDE	0x04
> +#define VLV_OVERRIDE_RPS_MASK	1
> +#define VLV_ENABLE_BIAS_SHARE	(1 << 1)
> +#define VLV_BIAS_VAL	(6 << 2)

Can we use better names for these (from configdb, say)?

> +
>  #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
>  #define VLV_RP_UP_EI_THRESHOLD			90
>  #define VLV_RP_DOWN_EI_THRESHOLD		70
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 88e71a3..673612b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5015,6 +5015,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
>  
> +	/* Setting Fixed Bias */
> +	val = VLV_OVERRIDE_RPS_MASK |
> +		  VLV_ENABLE_BIAS_SHARE |
> +		  VLV_BIAS_VAL;
> +	vlv_punit_write(dev_priv, VLV_IOSFB_RPS_OVERRIDE, val);
> +
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  
>  	/* RPS code assumes GPLL is used */
> @@ -5099,6 +5105,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>  
> +	/* Setting Fixed Bias */
> +	val = VLV_OVERRIDE_RPS_MASK |
> +		  VLV_ENABLE_BIAS_SHARE |
> +		  VLV_BIAS_VAL;
> +	vlv_punit_write(dev_priv, VLV_IOSFB_RPS_OVERRIDE, val);
> +
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  
>  	/* RPS code assumes GPLL is used */
> -- 
> 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 v2] drm/i915: Setup static bias for GPU

[deepak.s]

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

Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.

v2: rename reg defn to match spec. (Ville)

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..21c33c6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,11 @@ enum skl_disp_power_wells {
 #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
 #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
 
+#define VLV_TURBO_SOC_OVERRIDE	0x04
+#define 	VLV_OVERRIDE_EN	1
+#define 	VLV_SOC_TDP_EN	(1 << 1)
+#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
+
 #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
 
 /* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..318f587 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_EN |
+		  VLV_SOC_TDP_EN |
+		  VLV_BIAS_CPU_125_SOC_875;
+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_EN |
+		  VLV_SOC_TDP_EN |
+		  VLV_BIAS_CPU_125_SOC_875;
+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
-- 
1.9.1

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

[PATCH v3] drm/i915: Setup static bias for GPU

[deepak.s]

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

Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.

v2: rename reg defn to match spec. (Ville)

v3: Updated bias setting for chv (Deepak)

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..048987e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
 #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
 #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
 
+#define VLV_TURBO_SOC_OVERRIDE	0x04
+#define 	VLV_OVERRIDE_EN	1
+#define 	VLV_SOC_TDP_EN	(1 << 1)
+#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
+#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
+
 #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
 
 /* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..3689d0e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_EN |
+		  VLV_SOC_TDP_EN |
+		  CHV_BIAS_CPU_50_SOC_50;
+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
 
+	/* Setting Fixed Bias */
+	val = VLV_OVERRIDE_EN |
+		  VLV_SOC_TDP_EN |
+		  VLV_BIAS_CPU_125_SOC_875;
+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
 	/* RPS code assumes GPLL is used */
-- 
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: Setup static bias for GPU

[Ville Syrjälä]

On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Based on the spec, Setting up static BIAS for GPU to improve the
> rps performace.
> 
> v2: rename reg defn to match spec. (Ville)
> 
> v3: Updated bias setting for chv (Deepak)
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>

Matches the spec. Whether the chosen bias is really the best, I can't
really say. But favoring the GPU does seem like a sensible idea if we
want to keep the UI stuff fluid enough while there's some CPU heavy
tasks running at the same time.

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36805b6..048987e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -670,6 +670,12 @@ enum skl_disp_power_wells {
>  #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>  
> +#define VLV_TURBO_SOC_OVERRIDE	0x04
> +#define 	VLV_OVERRIDE_EN	1
> +#define 	VLV_SOC_TDP_EN	(1 << 1)
> +#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
> +#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
> +
>  #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
>  
>  /* vlv2 north clock has */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 78c89ff..3689d0e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
>  
> +	/* Setting Fixed Bias */
> +	val = VLV_OVERRIDE_EN |
> +		  VLV_SOC_TDP_EN |
> +		  CHV_BIAS_CPU_50_SOC_50;
> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> +
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  
>  	/* RPS code assumes GPLL is used */
> @@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>  
> +	/* Setting Fixed Bias */
> +	val = VLV_OVERRIDE_EN |
> +		  VLV_SOC_TDP_EN |
> +		  VLV_BIAS_CPU_125_SOC_875;
> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> +
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  
>  	/* RPS code assumes GPLL is used */
> -- 
> 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 v3] drm/i915: Setup static bias for GPU

[Deepak S]

On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
> On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> Based on the spec, Setting up static BIAS for GPU to improve the
>> rps performace.
>>
>> v2: rename reg defn to match spec. (Ville)
>>
>> v3: Updated bias setting for chv (Deepak)
>>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Matches the spec. Whether the chosen bias is really the best, I can't
> really say. But favoring the GPU does seem like a sensible idea if we
> want to keep the UI stuff fluid enough while there's some CPU heavy
> tasks running at the same time.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks Ville for reviewing,
Yes our aim is to keep user experience smooth.

>> ---
>>   drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
>>   drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
>>   2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 36805b6..048987e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -670,6 +670,12 @@ enum skl_disp_power_wells {
>>   #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
>>   #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>>   
>> +#define VLV_TURBO_SOC_OVERRIDE	0x04
>> +#define 	VLV_OVERRIDE_EN	1
>> +#define 	VLV_SOC_TDP_EN	(1 << 1)
>> +#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
>> +#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
>> +
>>   #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
>>   
>>   /* vlv2 north clock has */
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 78c89ff..3689d0e 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
>>   		   GEN6_RP_UP_BUSY_AVG |
>>   		   GEN6_RP_DOWN_IDLE_AVG);
>>   
>> +	/* Setting Fixed Bias */
>> +	val = VLV_OVERRIDE_EN |
>> +		  VLV_SOC_TDP_EN |
>> +		  CHV_BIAS_CPU_50_SOC_50;
>> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
>> +
>>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>>   
>>   	/* RPS code assumes GPLL is used */
>> @@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
>>   
>>   	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>>   
>> +	/* Setting Fixed Bias */
>> +	val = VLV_OVERRIDE_EN |
>> +		  VLV_SOC_TDP_EN |
>> +		  VLV_BIAS_CPU_125_SOC_875;
>> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
>> +
>>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>>   
>>   	/* RPS code assumes GPLL is used */
>> -- 
>> 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: Setup static bias for GPU

[Daniel Vetter]

On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
> 
> 
> On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
> >On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
> >>From: Deepak S <deepak.s@linux.intel.com>
> >>
> >>Based on the spec, Setting up static BIAS for GPU to improve the
> >>rps performace.
> >>
> >>v2: rename reg defn to match spec. (Ville)
> >>
> >>v3: Updated bias setting for chv (Deepak)
> >>
> >>Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >Matches the spec. Whether the chosen bias is really the best, I can't
> >really say. But favoring the GPU does seem like a sensible idea if we
> >want to keep the UI stuff fluid enough while there's some CPU heavy
> >tasks running at the same time.
> >
> >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Thanks Ville for reviewing,
> Yes our aim is to keep user experience smooth.

Since the aim is smooth UI ... how does this interact with the rps
boosting that was just enabled with Chris' patches for vlv/chv too?

A static bias seems a lot less what we want now that we should have
something dynamic. Specifically I'm thinking of

commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Apr 7 16:20:31 2015 +0100

    drm/i915: Boost GPU frequency if we detect outstanding pageflips

Cheers, Daniel

> 
> >>---
> >>  drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
> >>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
> >>  2 files changed, 18 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>index 36805b6..048987e 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
> >>  #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
> >>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
> >>+#define VLV_TURBO_SOC_OVERRIDE	0x04
> >>+#define 	VLV_OVERRIDE_EN	1
> >>+#define 	VLV_SOC_TDP_EN	(1 << 1)
> >>+#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
> >>+#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
> >>+
> >>  #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
> >>  /* vlv2 north clock has */
> >>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>index 78c89ff..3689d0e 100644
> >>--- a/drivers/gpu/drm/i915/intel_pm.c
> >>+++ b/drivers/gpu/drm/i915/intel_pm.c
> >>@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
> >>  		   GEN6_RP_UP_BUSY_AVG |
> >>  		   GEN6_RP_DOWN_IDLE_AVG);
> >>+	/* Setting Fixed Bias */
> >>+	val = VLV_OVERRIDE_EN |
> >>+		  VLV_SOC_TDP_EN |
> >>+		  CHV_BIAS_CPU_50_SOC_50;
> >>+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> >>+
> >>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >>  	/* RPS code assumes GPLL is used */
> >>@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >>  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
> >>+	/* Setting Fixed Bias */
> >>+	val = VLV_OVERRIDE_EN |
> >>+		  VLV_SOC_TDP_EN |
> >>+		  VLV_BIAS_CPU_125_SOC_875;
> >>+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> >>+
> >>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >>  	/* RPS code assumes GPLL is used */
> >>-- 
> >>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 v3] drm/i915: Setup static bias for GPU

[Ville Syrjälä]

On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote:
> On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
> > 
> > 
> > On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
> > >On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
> > >>From: Deepak S <deepak.s@linux.intel.com>
> > >>
> > >>Based on the spec, Setting up static BIAS for GPU to improve the
> > >>rps performace.
> > >>
> > >>v2: rename reg defn to match spec. (Ville)
> > >>
> > >>v3: Updated bias setting for chv (Deepak)
> > >>
> > >>Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> > >Matches the spec. Whether the chosen bias is really the best, I can't
> > >really say. But favoring the GPU does seem like a sensible idea if we
> > >want to keep the UI stuff fluid enough while there's some CPU heavy
> > >tasks running at the same time.
> > >
> > >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Thanks Ville for reviewing,
> > Yes our aim is to keep user experience smooth.
> 
> Since the aim is smooth UI ... how does this interact with the rps
> boosting that was just enabled with Chris' patches for vlv/chv too?
> 
> A static bias seems a lot less what we want now that we should have
> something dynamic. Specifically I'm thinking of
> 
> commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue Apr 7 16:20:31 2015 +0100
> 
>     drm/i915: Boost GPU frequency if we detect outstanding pageflips

Totally separate topic. This only affects how the Punit splits up the
available energy credits between the CPU and the GPU. So only relevant
when thermally constrained and both CPU and GPU would like to run
faster than the limit allows.

> 
> Cheers, Daniel
> 
> > 
> > >>---
> > >>  drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
> > >>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
> > >>  2 files changed, 18 insertions(+)
> > >>
> > >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > >>index 36805b6..048987e 100644
> > >>--- a/drivers/gpu/drm/i915/i915_reg.h
> > >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> > >>@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
> > >>  #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
> > >>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
> > >>+#define VLV_TURBO_SOC_OVERRIDE	0x04
> > >>+#define 	VLV_OVERRIDE_EN	1
> > >>+#define 	VLV_SOC_TDP_EN	(1 << 1)
> > >>+#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
> > >>+#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
> > >>+
> > >>  #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
> > >>  /* vlv2 north clock has */
> > >>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > >>index 78c89ff..3689d0e 100644
> > >>--- a/drivers/gpu/drm/i915/intel_pm.c
> > >>+++ b/drivers/gpu/drm/i915/intel_pm.c
> > >>@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
> > >>  		   GEN6_RP_UP_BUSY_AVG |
> > >>  		   GEN6_RP_DOWN_IDLE_AVG);
> > >>+	/* Setting Fixed Bias */
> > >>+	val = VLV_OVERRIDE_EN |
> > >>+		  VLV_SOC_TDP_EN |
> > >>+		  CHV_BIAS_CPU_50_SOC_50;
> > >>+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> > >>+
> > >>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > >>  	/* RPS code assumes GPLL is used */
> > >>@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
> > >>  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
> > >>+	/* Setting Fixed Bias */
> > >>+	val = VLV_OVERRIDE_EN |
> > >>+		  VLV_SOC_TDP_EN |
> > >>+		  VLV_BIAS_CPU_125_SOC_875;
> > >>+	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
> > >>+
> > >>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > >>  	/* RPS code assumes GPLL is used */
> > >>-- 
> > >>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

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

Re: [PATCH v3] drm/i915: Setup static bias for GPU

[Deepak S]

On Monday 04 May 2015 08:58 PM, Ville Syrjälä wrote:
> On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote:
>> On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
>>>
>>> On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
>>>> On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
>>>>> From: Deepak S <deepak.s@linux.intel.com>
>>>>>
>>>>> Based on the spec, Setting up static BIAS for GPU to improve the
>>>>> rps performace.
>>>>>
>>>>> v2: rename reg defn to match spec. (Ville)
>>>>>
>>>>> v3: Updated bias setting for chv (Deepak)
>>>>>
>>>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>>> Matches the spec. Whether the chosen bias is really the best, I can't
>>>> really say. But favoring the GPU does seem like a sensible idea if we
>>>> want to keep the UI stuff fluid enough while there's some CPU heavy
>>>> tasks running at the same time.
>>>>
>>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Thanks Ville for reviewing,
>>> Yes our aim is to keep user experience smooth.
>> Since the aim is smooth UI ... how does this interact with the rps
>> boosting that was just enabled with Chris' patches for vlv/chv too?
>>
>> A static bias seems a lot less what we want now that we should have
>> something dynamic. Specifically I'm thinking of
>>
>> commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
>> Author: Chris Wilson <chris@chris-wilson.co.uk>
>> Date:   Tue Apr 7 16:20:31 2015 +0100
>>
>>      drm/i915: Boost GPU frequency if we detect outstanding pageflips
> Totally separate topic. This only affects how the Punit splits up the
> available energy credits between the CPU and the GPU. So only relevant
> when thermally constrained and both CPU and GPU would like to run
> faster than the limit allows.
>
Thanks Ville.

This is completely a different topic, more to do with punit budget constraints & this does not impact "Boot RPS logic"

Thanks
Deepak

>> Cheers, Daniel
>>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
>>>>>   drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
>>>>>   2 files changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>>> index 36805b6..048987e 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>> @@ -670,6 +670,12 @@ enum skl_disp_power_wells {
>>>>>   #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
>>>>>   #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>>>>> +#define VLV_TURBO_SOC_OVERRIDE	0x04
>>>>> +#define 	VLV_OVERRIDE_EN	1
>>>>> +#define 	VLV_SOC_TDP_EN	(1 << 1)
>>>>> +#define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
>>>>> +#define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
>>>>> +
>>>>>   #define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
>>>>>   /* vlv2 north clock has */
>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>> index 78c89ff..3689d0e 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>> @@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
>>>>>   		   GEN6_RP_UP_BUSY_AVG |
>>>>>   		   GEN6_RP_DOWN_IDLE_AVG);
>>>>> +	/* Setting Fixed Bias */
>>>>> +	val = VLV_OVERRIDE_EN |
>>>>> +		  VLV_SOC_TDP_EN |
>>>>> +		  CHV_BIAS_CPU_50_SOC_50;
>>>>> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
>>>>> +
>>>>>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>>>>>   	/* RPS code assumes GPLL is used */
>>>>> @@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
>>>>>   	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>>>>> +	/* Setting Fixed Bias */
>>>>> +	val = VLV_OVERRIDE_EN |
>>>>> +		  VLV_SOC_TDP_EN |
>>>>> +		  VLV_BIAS_CPU_125_SOC_875;
>>>>> +	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
>>>>> +
>>>>>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>>>>>   	/* RPS code assumes GPLL is used */
>>>>> -- 
>>>>> 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 v3] drm/i915: Setup static bias for GPU

[Daniel Vetter]

On Tue, May 05, 2015 at 01:12:41PM +0530, Deepak S wrote:
> 
> 
> On Monday 04 May 2015 08:58 PM, Ville Syrjälä wrote:
> >On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote:
> >>On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
> >>>
> >>>On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
> >>>>On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
> >>>>>From: Deepak S <deepak.s@linux.intel.com>
> >>>>>
> >>>>>Based on the spec, Setting up static BIAS for GPU to improve the
> >>>>>rps performace.
> >>>>>
> >>>>>v2: rename reg defn to match spec. (Ville)
> >>>>>
> >>>>>v3: Updated bias setting for chv (Deepak)
> >>>>>
> >>>>>Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >>>>Matches the spec. Whether the chosen bias is really the best, I can't
> >>>>really say. But favoring the GPU does seem like a sensible idea if we
> >>>>want to keep the UI stuff fluid enough while there's some CPU heavy
> >>>>tasks running at the same time.
> >>>>
> >>>>Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>Thanks Ville for reviewing,
> >>>Yes our aim is to keep user experience smooth.
> >>Since the aim is smooth UI ... how does this interact with the rps
> >>boosting that was just enabled with Chris' patches for vlv/chv too?
> >>
> >>A static bias seems a lot less what we want now that we should have
> >>something dynamic. Specifically I'm thinking of
> >>
> >>commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
> >>Author: Chris Wilson <chris@chris-wilson.co.uk>
> >>Date:   Tue Apr 7 16:20:31 2015 +0100
> >>
> >>     drm/i915: Boost GPU frequency if we detect outstanding pageflips
> >Totally separate topic. This only affects how the Punit splits up the
> >available energy credits between the CPU and the GPU. So only relevant
> >when thermally constrained and both CPU and GPU would like to run
> >faster than the limit allows.
> >
> Thanks Ville.
> 
> This is completely a different topic, more to do with punit budget
> constraints & this does not impact "Boot RPS logic"

Well I thought it'd be related since the justification was better UI
interactivity. And that tends to not be a thermal constrained load but
something really spike-y. And a static bias doesn't seem like a solution
to that problem.

I'll just go ahead and merge, but still feels like at least I don't know
why exactly we need this.
-Daniel
-- 
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 v3] drm/i915: Setup static bias for GPU

[Deepak S]

On Wednesday 06 May 2015 02:32 PM, Daniel Vetter wrote:
> On Tue, May 05, 2015 at 01:12:41PM +0530, Deepak S wrote:
>>
>> On Monday 04 May 2015 08:58 PM, Ville Syrjälä wrote:
>>> On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote:
>>>> On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
>>>>> On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
>>>>>> On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepak.s@linux.intel.com wrote:
>>>>>>> From: Deepak S <deepak.s@linux.intel.com>
>>>>>>>
>>>>>>> Based on the spec, Setting up static BIAS for GPU to improve the
>>>>>>> rps performace.
>>>>>>>
>>>>>>> v2: rename reg defn to match spec. (Ville)
>>>>>>>
>>>>>>> v3: Updated bias setting for chv (Deepak)
>>>>>>>
>>>>>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>>>>> Matches the spec. Whether the chosen bias is really the best, I can't
>>>>>> really say. But favoring the GPU does seem like a sensible idea if we
>>>>>> want to keep the UI stuff fluid enough while there's some CPU heavy
>>>>>> tasks running at the same time.
>>>>>>
>>>>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Thanks Ville for reviewing,
>>>>> Yes our aim is to keep user experience smooth.
>>>> Since the aim is smooth UI ... how does this interact with the rps
>>>> boosting that was just enabled with Chris' patches for vlv/chv too?
>>>>
>>>> A static bias seems a lot less what we want now that we should have
>>>> something dynamic. Specifically I'm thinking of
>>>>
>>>> commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
>>>> Author: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Date:   Tue Apr 7 16:20:31 2015 +0100
>>>>
>>>>      drm/i915: Boost GPU frequency if we detect outstanding pageflips
>>> Totally separate topic. This only affects how the Punit splits up the
>>> available energy credits between the CPU and the GPU. So only relevant
>>> when thermally constrained and both CPU and GPU would like to run
>>> faster than the limit allows.
>>>
>> Thanks Ville.
>>
>> This is completely a different topic, more to do with punit budget
>> constraints & this does not impact "Boot RPS logic"
> Well I thought it'd be related since the justification was better UI
> interactivity. And that tends to not be a thermal constrained load but
> something really spike-y. And a static bias doesn't seem like a solution
> to that problem.
>
> I'll just go ahead and merge, but still feels like at least I don't know
> why exactly we need this.
> -Daniel

Hi Daniel,

Dynamic power bias is not supported by the HW or FW :(.
This static bias was recommended by HW team after analyzing the results on IA & GT

Thanks
Deepak

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

Re: [PATCH v3] drm/i915: Setup static bias for GPU

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6283
-------------------------------------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                 -1              264/264              263/264
BYT                 -3              227/227              224/227
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:intel_pch_fifo_underrun_irq_handler[i915]]*ERROR*PCH_transcoder_A_FIFO_underrun@PCH transcoder A FIFO underrun
*IVB  igt@gem_pwrite_pread@display-pwrite-blt-gtt_mmap-performance      PASS(4)      DMESG_FAIL(1)PASS(1)
(dmesg patch applied)drm:i915_context_is_banned[i915]]*ERROR*gpu_hanging_too_fast,banning@gpu hanging too
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(7)      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(4)      TIMEOUT(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] drm/i915: Setup static bias for GPU

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6281
-------------------------------------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                                  264/264              264/264
BYT                 -3              227/227              224/227
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@gem_caching@reads      PASS(2)      NO_RESULT(1)PASS(1)
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(5)      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(3)PASS(3)      TIMEOUT(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