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

[PATCH] drm/i915: 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.

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 | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..2e1ed07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	vlv_force_gfx_clock(dev_priv, true);
-
-	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
-					dev_priv->rps.min_freq_softlimit);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(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] drm/i915: Update PM interrupts before updating the freq

[deepak.s]

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

We update the GT PM interrupts mask at the end of set rps. We observed even
though we are requesting a RPn or RP0, there is a chance to get a DOWN or UP
interrupts before interrupts mask. These extra interrupts are simply wasting
cpu cycles. In this patch we mask the interrupts for given freq before
requesting new frequency.

Signed-off-by: Deepak S <deepak.s@linux.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 2e1ed07..bbfe4f0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3879,12 +3879,12 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 	if (WARN_ONCE(IS_CHERRYVIEW(dev) && (val & 1),
 		      "Odd GPU freq value\n"))
 		val &= ~1;
+	
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	if (val != dev_priv->rps.cur_freq)
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
-
 	dev_priv->rps.cur_freq = val;
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
-- 
1.9.1

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

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

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5889
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              280/280              279/280
ILK                                  308/308              308/308
SNB                                  328/328              328/328
IVB                                  379/379              379/379
BYT                                  294/294              294/294
HSW                 -1              387/387              386/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_minor-unsync-interruptible      PASS(2)      DMESG_WARN(2)
*HSW  igt_gem_storedw_loop_bsd      PASS(2)      DMESG_WARN(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(4)      DMESG_WARN(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] drm/i915: Update PM interrupts before updating the freq

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5890
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              280/280              279/280
ILK                 -1              308/308              307/308
SNB                 -18              328/328              310/328
IVB                                  379/379              379/379
BYT                                  294/294              294/294
HSW                                  387/387              387/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_minor-unsync-normal      DMESG_WARN(1)PASS(5)      DMESG_WARN(1)PASS(1)
*ILK  igt_gem_unfence_active_buffers      PASS(2)      DMESG_WARN(1)PASS(1)
*SNB  igt_kms_rotation_crc_primary-rotation      NSPT(2)DMESG_WARN(1)PASS(3)      DMESG_FAIL(1)NSPT(1)
 SNB  igt_kms_rotation_crc_sprite-rotation      NSPT(2)DMESG_WARN(1)PASS(4)      NSPT(2)
 SNB  igt_pm_rpm_cursor      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_cursor-dpms      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_dpms-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_drm-resources-equal      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_fences      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_fences-dpms      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-execbuf      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-mmap-cpu      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-mmap-gtt      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-pread      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_i2c      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_modeset-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_pci-d3-state      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_rte      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
*BDW  igt_gem_gtt_hog      PASS(6)      DMESG_WARN(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] drm/i915: Remove wait for for punit to updates freq.

[Jani Nikula]

On Thu, 05 Mar 2015, 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.
>
> 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>

Deepak, these patches seem to have fallen through the cracks. Are they
still valid? Please rebase and repost if they are.

Ville, your opinion also appreciated.

BR,
Jani.



> ---
>  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e710b43..2e1ed07 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>   * * 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
> + * 3. Request the freq to Rpn.
>   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>   * 5. Unmask Turbo interrupts
>  */
> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> -	/* 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, dev_priv->rps.min_freq_softlimit);
>  		return;
>  	}
> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>  
>  	vlv_force_gfx_clock(dev_priv, true);
> -
> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> -
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> -					dev_priv->rps.min_freq_softlimit);
> -
> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> -				& GENFREQSTATUS) == 0, 100))
> -		DRM_ERROR("timed out waiting for Punit\n");
> -
> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>  	vlv_force_gfx_clock(dev_priv, false);
> -
> -	I915_WRITE(GEN6_PMINTRMSK,
> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>  }
>  
>  void gen6_rps_idle(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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Mon, Apr 13, 2015 at 02:55:12PM +0300, Jani Nikula wrote:
> On Thu, 05 Mar 2015, 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.
> >
> > 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>
> 
> Deepak, these patches seem to have fallen through the cracks. Are they
> still valid? Please rebase and repost if they are.
> 
> Ville, your opinion also appreciated.

I don't I have any VLVs old enough to hit this, so can't really confirm
one way or the other.

> 
> BR,
> Jani.
> 
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
> >  1 file changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e710b43..2e1ed07 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
> >   * * 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
> > + * 3. Request the freq to Rpn.
> >   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> >   * 5. Unmask Turbo interrupts
> >  */
> > @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >  {
> >  	struct drm_device *dev = dev_priv->dev;
> >  
> > -	/* 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)) {

Why was the stepping check removed?

> >  		valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >  		return;
> >  	}
> > @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
> >  
> >  	vlv_force_gfx_clock(dev_priv, true);
> > -
> > -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> > -
> > -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> > -					dev_priv->rps.min_freq_softlimit);
> > -
> > -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> > -				& GENFREQSTATUS) == 0, 100))
> > -		DRM_ERROR("timed out waiting for Punit\n");
> > -
> > +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >  	vlv_force_gfx_clock(dev_priv, false);
> > -
> > -	I915_WRITE(GEN6_PMINTRMSK,
> > -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> >  }
> >  
> >  void gen6_rps_idle(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
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> 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] drm/i915: Remove wait for for punit to updates freq.

[Deepak S]

On Monday 13 April 2015 05:40 PM, Ville Syrjälä wrote:
> On Mon, Apr 13, 2015 at 02:55:12PM +0300, Jani Nikula wrote:
>> On Thu, 05 Mar 2015, 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.
>>>
>>> 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>
>> Deepak, these patches seem to have fallen through the cracks. Are they
>> still valid? Please rebase and repost if they are.
>>
>> Ville, your opinion also appreciated.
> I don't I have any VLVs old enough to hit this, so can't really confirm
> one way or the other.

Hi Jesse, Can you please review the patch?

Thanks
Deepak

>> BR,
>> Jani.
>>
>>
>>
>>> ---
>>>   drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
>>>   1 file changed, 4 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index e710b43..2e1ed07 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>>>    * * 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
>>> + * 3. Request the freq to Rpn.
>>>    * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>>>    * 5. Unmask Turbo interrupts
>>>   */
>>> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>   {
>>>   	struct drm_device *dev = dev_priv->dev;
>>>   
>>> -	/* 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)) {
> Why was the stepping check removed?
>
>>>   		valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>>>   		return;
>>>   	}
>>> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>   		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>>>   
>>>   	vlv_force_gfx_clock(dev_priv, true);
>>> -
>>> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
>>> -
>>> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
>>> -					dev_priv->rps.min_freq_softlimit);
>>> -
>>> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
>>> -				& GENFREQSTATUS) == 0, 100))
>>> -		DRM_ERROR("timed out waiting for Punit\n");
>>> -
>>> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>>>   	vlv_force_gfx_clock(dev_priv, false);
>>> -
>>> -	I915_WRITE(GEN6_PMINTRMSK,
>>> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>>>   }
>>>   
>>>   void gen6_rps_idle(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
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> 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] drm/i915: Remove wait for for punit to updates freq.

[Jesse Barnes]

On 03/04/2015 08:08 PM, 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.
> 
> 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 | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e710b43..2e1ed07 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>   * * 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
> + * 3. Request the freq to Rpn.
>   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>   * 5. Unmask Turbo interrupts
>  */
> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> -	/* 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, dev_priv->rps.min_freq_softlimit);
>  		return;
>  	}
> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>  
>  	vlv_force_gfx_clock(dev_priv, true);
> -
> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> -
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> -					dev_priv->rps.min_freq_softlimit);
> -
> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> -				& GENFREQSTATUS) == 0, 100))
> -		DRM_ERROR("timed out waiting for Punit\n");
> -
> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>  	vlv_force_gfx_clock(dev_priv, false);
> -
> -	I915_WRITE(GEN6_PMINTRMSK,
> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>  }
>  
>  void gen6_rps_idle(struct drm_i915_private *dev_priv)
> 

Yeah I think this is fine (may need a rebase though, you can keep my r-b
if you do that in case Jani doesn't want to deal with the merge conflicts).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
> On 03/04/2015 08:08 PM, 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.
> > 
> > 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 | 20 ++++----------------
> >  1 file changed, 4 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e710b43..2e1ed07 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
> >   * * 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
> > + * 3. Request the freq to Rpn.
> >   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> >   * 5. Unmask Turbo interrupts
> >  */
> > @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >  {
> >  	struct drm_device *dev = dev_priv->dev;
> >  
> > -	/* 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, dev_priv->rps.min_freq_softlimit);
> >  		return;
> >  	}
> > @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
> >  
> >  	vlv_force_gfx_clock(dev_priv, true);
> > -
> > -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> > -
> > -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> > -					dev_priv->rps.min_freq_softlimit);
> > -
> > -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> > -				& GENFREQSTATUS) == 0, 100))
> > -		DRM_ERROR("timed out waiting for Punit\n");
> > -
> > +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >  	vlv_force_gfx_clock(dev_priv, false);
> > -
> > -	I915_WRITE(GEN6_PMINTRMSK,
> > -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> >  }
> >  
> >  void gen6_rps_idle(struct drm_i915_private *dev_priv)
> > 
> 
> Yeah I think this is fine (may need a rebase though, you can keep my r-b
> if you do that in case Jani doesn't want to deal with the merge conflicts).
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

The removal of the stepping check is still confusing me even if the
rest would be OK.

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

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

[Deepak S]

On Tuesday 28 April 2015 11:46 PM, Jesse Barnes wrote:
>
> Yeah I think this is fine (may need a rebase though, you can keep my r-b
> if you do that in case Jani doesn't want to deal with the merge conflicts).
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Sure Jesse, I will rebase the patch.

Thanks
Deepak

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

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

[Deepak S]

On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
> On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
>> On 03/04/2015 08:08 PM, 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.
>>>
>>> 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 | 20 ++++----------------
>>>   1 file changed, 4 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index e710b43..2e1ed07 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>>>    * * 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
>>> + * 3. Request the freq to Rpn.
>>>    * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>>>    * 5. Unmask Turbo interrupts
>>>   */
>>> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>   {
>>>   	struct drm_device *dev = dev_priv->dev;
>>>   
>>> -	/* 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, dev_priv->rps.min_freq_softlimit);
>>>   		return;
>>>   	}
>>> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>   		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>>>   
>>>   	vlv_force_gfx_clock(dev_priv, true);
>>> -
>>> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
>>> -
>>> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
>>> -					dev_priv->rps.min_freq_softlimit);
>>> -
>>> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
>>> -				& GENFREQSTATUS) == 0, 100))
>>> -		DRM_ERROR("timed out waiting for Punit\n");
>>> -
>>> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>>>   	vlv_force_gfx_clock(dev_priv, false);
>>> -
>>> -	I915_WRITE(GEN6_PMINTRMSK,
>>> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>>>   }
>>>   
>>>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
>>>
>> Yeah I think this is fine (may need a rebase though, you can keep my r-b
>> if you do that in case Jani doesn't want to deal with the merge conflicts).
>>
>> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> The removal of the stepping check is still confusing me even if the
> rest would be OK.
>
Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( )
We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

  


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

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

[Ville Syrjälä]

On Wed, Apr 29, 2015 at 08:20:20AM +0530, Deepak S wrote:
> 
> 
> On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
> > On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
> >> On 03/04/2015 08:08 PM, 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.
> >>>
> >>> 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 | 20 ++++----------------
> >>>   1 file changed, 4 insertions(+), 16 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>> index e710b43..2e1ed07 100644
> >>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
> >>>    * * 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
> >>> + * 3. Request the freq to Rpn.
> >>>    * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> >>>    * 5. Unmask Turbo interrupts
> >>>   */
> >>> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >>>   {
> >>>   	struct drm_device *dev = dev_priv->dev;
> >>>   
> >>> -	/* 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, dev_priv->rps.min_freq_softlimit);
> >>>   		return;
> >>>   	}
> >>> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >>>   		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
> >>>   
> >>>   	vlv_force_gfx_clock(dev_priv, true);
> >>> -
> >>> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> >>> -
> >>> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> >>> -					dev_priv->rps.min_freq_softlimit);
> >>> -
> >>> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> >>> -				& GENFREQSTATUS) == 0, 100))
> >>> -		DRM_ERROR("timed out waiting for Punit\n");
> >>> -
> >>> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >>>   	vlv_force_gfx_clock(dev_priv, false);
> >>> -
> >>> -	I915_WRITE(GEN6_PMINTRMSK,
> >>> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> >>>   }
> >>>   
> >>>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
> >>>
> >> Yeah I think this is fine (may need a rebase though, you can keep my r-b
> >> if you do that in case Jani doesn't want to deal with the merge conflicts).
> >>
> >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > The removal of the stepping check is still confusing me even if the
> > rest would be OK.
> >
> Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( )
> We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

But we shouldn't need the gfx clock force for the latest VLV
stepping(s), and we certainly didn't do it before. So why do
it now?

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

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

[Deepak S]

On Wednesday 29 April 2015 03:56 PM, Ville Syrjälä wrote:
> On Wed, Apr 29, 2015 at 08:20:20AM +0530, Deepak S wrote:
>>
>> On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
>>> On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
>>>> On 03/04/2015 08:08 PM, 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.
>>>>>
>>>>> 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 | 20 ++++----------------
>>>>>    1 file changed, 4 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>> index e710b43..2e1ed07 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>> @@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>>>>>     * * 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
>>>>> + * 3. Request the freq to Rpn.
>>>>>     * 4. Clear the Force GFX CLK ON bit so that Gfx can down
>>>>>     * 5. Unmask Turbo interrupts
>>>>>    */
>>>>> @@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>>>    {
>>>>>    	struct drm_device *dev = dev_priv->dev;
>>>>>    
>>>>> -	/* 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, dev_priv->rps.min_freq_softlimit);
>>>>>    		return;
>>>>>    	}
>>>>> @@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>>>>>    		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
>>>>>    
>>>>>    	vlv_force_gfx_clock(dev_priv, true);
>>>>> -
>>>>> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
>>>>> -
>>>>> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
>>>>> -					dev_priv->rps.min_freq_softlimit);
>>>>> -
>>>>> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
>>>>> -				& GENFREQSTATUS) == 0, 100))
>>>>> -		DRM_ERROR("timed out waiting for Punit\n");
>>>>> -
>>>>> +	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>>>>>    	vlv_force_gfx_clock(dev_priv, false);
>>>>> -
>>>>> -	I915_WRITE(GEN6_PMINTRMSK,
>>>>> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>>>>>    }
>>>>>    
>>>>>    void gen6_rps_idle(struct drm_i915_private *dev_priv)
>>>>>
>>>> Yeah I think this is fine (may need a rebase though, you can keep my r-b
>>>> if you do that in case Jani doesn't want to deal with the merge conflicts).
>>>>
>>>> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>>> The removal of the stepping check is still confusing me even if the
>>> rest would be OK.
>>>
>> Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( )
>> We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.
> But we shouldn't need the gfx clock force for the latest VLV
> stepping(s), and we certainly didn't do it before. So why do
> it now?
>
Hi Ville, This is keep code common across all the VLV stepping. :)

Thanks
Deepak


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

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

[Daniel Vetter]

On Thu, Apr 30, 2015 at 03:34:32PM +0530, Deepak S wrote:
> 
> 
> On Wednesday 29 April 2015 03:56 PM, Ville Syrjälä wrote:
> >On Wed, Apr 29, 2015 at 08:20:20AM +0530, Deepak S wrote:
> >>
> >>On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
> >>>On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
> >>>>On 03/04/2015 08:08 PM, 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.
> >>>>>
> >>>>>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 | 20 ++++----------------
> >>>>>   1 file changed, 4 insertions(+), 16 deletions(-)
> >>>>>
> >>>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>index e710b43..2e1ed07 100644
> >>>>>--- a/drivers/gpu/drm/i915/intel_pm.c
> >>>>>+++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
> >>>>>    * * 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
> >>>>>+ * 3. Request the freq to Rpn.
> >>>>>    * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> >>>>>    * 5. Unmask Turbo interrupts
> >>>>>   */
> >>>>>@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >>>>>   {
> >>>>>   	struct drm_device *dev = dev_priv->dev;
> >>>>>-	/* 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, dev_priv->rps.min_freq_softlimit);
> >>>>>   		return;
> >>>>>   	}
> >>>>>@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >>>>>   		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
> >>>>>   	vlv_force_gfx_clock(dev_priv, true);
> >>>>>-
> >>>>>-	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> >>>>>-
> >>>>>-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> >>>>>-					dev_priv->rps.min_freq_softlimit);
> >>>>>-
> >>>>>-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> >>>>>-				& GENFREQSTATUS) == 0, 100))
> >>>>>-		DRM_ERROR("timed out waiting for Punit\n");
> >>>>>-
> >>>>>+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >>>>>   	vlv_force_gfx_clock(dev_priv, false);
> >>>>>-
> >>>>>-	I915_WRITE(GEN6_PMINTRMSK,
> >>>>>-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> >>>>>   }
> >>>>>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
> >>>>>
> >>>>Yeah I think this is fine (may need a rebase though, you can keep my r-b
> >>>>if you do that in case Jani doesn't want to deal with the merge conflicts).
> >>>>
> >>>>Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >>>The removal of the stepping check is still confusing me even if the
> >>>rest would be OK.
> >>>
> >>Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( )
> >>We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.
> >But we shouldn't need the gfx clock force for the latest VLV
> >stepping(s), and we certainly didn't do it before. So why do
> >it now?
> >
> Hi Ville, This is keep code common across all the VLV stepping. :)

Makes sense (at least to me) but please add this explanation to the commit
message when resending so it won't get lost.
-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

[PATCH] drm/i915: Update PM interrupts before updating the freq

[Praveen Paneri]

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

Currently we update the freq before masking the interrupts, which can
allow new interrupts to occur before the frequency has changed. These
extra interrupts might waste some cpu cycles. This patch corrects
this by masking interrupts prior to updating the frequency.

Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Praveen Paneri <praveen.paneri@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 4e24d2b..1082123 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4482,14 +4482,14 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 		      "Odd GPU freq value\n"))
 		val &= ~1;
 
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+
 	if (val != dev_priv->rps.cur_freq) {
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 		if (!IS_CHERRYVIEW(dev_priv))
 			gen6_set_rps_thresholds(dev_priv, val);
 	}
 
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
-
 	dev_priv->rps.cur_freq = val;
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
-- 
1.9.1

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

Re: [PATCH] drm/i915: Update PM interrupts before updating the freq

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6773
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  303/303              303/303
SNB              +3                 309/316              312/316
IVB                                  343/343              343/343
BYT                                  285/285              285/285
HSW              +13                 367/381              380/381
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*SNB  igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*SNB  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*SNB  igt@pm_rpm@cursor      DMESG_WARN(1)      PASS(1)
*SNB  igt@pm_rpm@cursor-dpms      DMESG_FAIL(1)      FAIL(1)
*HSW  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_lpsp@non-edp      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@debugfs-read      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-idle      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-mmap-gtt      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-pread      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@i2c      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@modeset-non-lpsp      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@modeset-non-lpsp-stress-no-wait      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@pci-d3-state      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@reg-read-ioctl      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@rte      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@sysfs-read      DMESG_WARN(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] drm/i915: Update PM interrupts before updating the freq

[Chris Wilson]

On Fri, Jul 10, 2015 at 06:31:40PM +0530, Praveen Paneri wrote:
> From: Deepak S <deepak.s@intel.com>
> 
> Currently we update the freq before masking the interrupts, which can
> allow new interrupts to occur before the frequency has changed. These
> extra interrupts might waste some cpu cycles. This patch corrects
> this by masking interrupts prior to updating the frequency.

Well it won't waste CPU cycles as the interrupt is also masked by the
threshold limits, but there should be no harm at all in reordering the
patch so, and it does make a certain amount of sense.

> Signed-off-by: Deepak S <deepak.s@intel.com>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>

Quibbling over the language in the changelog aside,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Update PM interrupts before updating the freq

[Daniel Vetter]

On Sat, Jul 11, 2015 at 05:46:37PM +0100, Chris Wilson wrote:
> On Fri, Jul 10, 2015 at 06:31:40PM +0530, Praveen Paneri wrote:
> > From: Deepak S <deepak.s@intel.com>
> > 
> > Currently we update the freq before masking the interrupts, which can
> > allow new interrupts to occur before the frequency has changed. These
> > extra interrupts might waste some cpu cycles. This patch corrects
> > this by masking interrupts prior to updating the frequency.
> 
> Well it won't waste CPU cycles as the interrupt is also masked by the
> threshold limits, but there should be no harm at all in reordering the
> patch so, and it does make a certain amount of sense.

Added and ...
> 
> > Signed-off-by: Deepak S <deepak.s@intel.com>
> > Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> 
> Quibbling over the language in the changelog aside,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

queued for -next, thanks for the patch.
-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