[PATCH v1 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[PATCH v1 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Sagar Arun Kamble]

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c6188dd..bb2fd78 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
+	intel_runtime_pm_get(dev_priv);
+
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	val = intel_freq_opcode(dev_priv, val);
@@ -398,6 +400,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
+	intel_runtime_pm_put(dev_priv);
+
 	return count;
 }
 
@@ -433,6 +437,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
+	intel_runtime_pm_get(dev_priv);
+
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	val = intel_freq_opcode(dev_priv, val);
@@ -457,6 +463,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
+	intel_runtime_pm_put(dev_priv);
+
 	return count;
 
 }
-- 
1.9.1

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

Re: [PATCH v1 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Jani Nikula]

On Fri, 05 Feb 2016, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote:
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Please add a commit message saying why, and what blows up without
this. It may be obvious to you, but there's no harm in spelling it out.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index c6188dd..bb2fd78 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -398,6 +400,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return count;
>  }
>  
> @@ -433,6 +437,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -457,6 +463,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return count;
>  
>  }

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

✓ Fi.CI.BAT: success for series starting with [v1,1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Patchwork]

== Summary ==

Series 3131v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3131/revisions/1/mbox/


bdw-nuci7        total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc          total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
hsw-xps12        total:161  pass:151  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:164  pass:116  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430s        total:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1375/

9ae71c139227311e70dcc91d16d5acba34ce9a71 drm-intel-nightly: 2016y-02m-08d-09h-39m-29s UTC integration manifest
214eb3275be3cb8d63fb0ebe26f6664dc1a73dcf drm/i915: Hold RPM reference while setting freq limits through sysfs

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

Re: [PATCH v1 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Ville Syrjälä]

On Sat, Feb 06, 2016 at 12:25:01AM +0530, Sagar Arun Kamble wrote:
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index c6188dd..bb2fd78 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -398,6 +400,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +

There's a return statement somewhere within the function too. You need to drop
the rpm ref there as well.

>  	return count;
>  }
>  
> @@ -433,6 +437,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -457,6 +463,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return count;
>  
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Sagar Arun Kamble]

This changes ensures device is active when frequency limits are changed.
This is needed as we are writing to register RPNSWREQ in intel_set_rps.
If not done, might lead to undesired errors like:
[ 1965.189137] [drm:fw_domains_get] *ERROR* blitter: timed out waiting for forcewake ack to clear.

v2: Added elaborate commit message. (Jani)
    Fixing RPM reference drop in early exit paths. (Ville)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c6188dd..2d576b7 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
+	intel_runtime_pm_get(dev_priv);
+
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	val = intel_freq_opcode(dev_priv, val);
@@ -378,6 +380,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 	    val > dev_priv->rps.max_freq ||
 	    val < dev_priv->rps.min_freq_softlimit) {
 		mutex_unlock(&dev_priv->rps.hw_lock);
+		intel_runtime_pm_put(dev_priv);
 		return -EINVAL;
 	}
 
@@ -398,6 +401,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
+	intel_runtime_pm_put(dev_priv);
+
 	return count;
 }
 
@@ -433,6 +438,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
+	intel_runtime_pm_get(dev_priv);
+
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	val = intel_freq_opcode(dev_priv, val);
@@ -441,6 +448,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	    val > dev_priv->rps.max_freq ||
 	    val > dev_priv->rps.max_freq_softlimit) {
 		mutex_unlock(&dev_priv->rps.hw_lock);
+		intel_runtime_pm_put(dev_priv);
 		return -EINVAL;
 	}
 
@@ -457,6 +465,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
+	intel_runtime_pm_put(dev_priv);
+
 	return count;
 
 }
-- 
1.9.1

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

Re: [PATCH v2 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Ville Syrjälä]

On Mon, Feb 08, 2016 at 10:47:11PM +0530, Sagar Arun Kamble wrote:
> This changes ensures device is active when frequency limits are changed.
> This is needed as we are writing to register RPNSWREQ in intel_set_rps.
> If not done, might lead to undesired errors like:
> [ 1965.189137] [drm:fw_domains_get] *ERROR* blitter: timed out waiting for forcewake ack to clear.
> 
> v2: Added elaborate commit message. (Jani)
>     Fixing RPM reference drop in early exit paths. (Ville)
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index c6188dd..2d576b7 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -378,6 +380,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  	    val > dev_priv->rps.max_freq ||
>  	    val < dev_priv->rps.min_freq_softlimit) {
>  		mutex_unlock(&dev_priv->rps.hw_lock);
> +		intel_runtime_pm_put(dev_priv);
>  		return -EINVAL;
>  	}
>  
> @@ -398,6 +401,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return count;
>  }
>  
> @@ -433,6 +438,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	val = intel_freq_opcode(dev_priv, val);
> @@ -441,6 +448,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	    val > dev_priv->rps.max_freq ||
>  	    val > dev_priv->rps.max_freq_softlimit) {
>  		mutex_unlock(&dev_priv->rps.hw_lock);
> +		intel_runtime_pm_put(dev_priv);
>  		return -EINVAL;
>  	}
>  
> @@ -457,6 +465,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return count;
>  
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

✓ Fi.CI.BAT: success for series starting with [v2,1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs (rev2)

[Patchwork]

== Summary ==

Series 3131v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3131/revisions/2/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                skip       -> PASS       (bsw-nuc-2)
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                fail       -> PASS       (ilk-hp8440p)
                pass       -> SKIP       (byt-nuc)
        Subgroup suspend-read-crc-pipe-c:
                fail       -> SKIP       (ilk-hp8440p)

bdw-nuci7        total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc          total:164  pass:140  dwarn:0   dfail:0   fail:0   skip:24 
hsw-brixbox      total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:164  pass:116  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430s        total:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
snb-dellxps      total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1379/

b29e4845d2dc187da666855b83d0e79045a748f7 drm-intel-nightly: 2016y-02m-08d-14h-15m-14s UTC integration manifest
98dd3bbb27328d58472a4429972ba5506b51355d drm/i915: Hold RPM reference while setting freq limits through sysfs

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

Re: [PATCH v2 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Ville Syrjälä]

On Mon, Feb 08, 2016 at 07:20:11PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 08, 2016 at 10:47:11PM +0530, Sagar Arun Kamble wrote:
> > This changes ensures device is active when frequency limits are changed.
> > This is needed as we are writing to register RPNSWREQ in intel_set_rps.
> > If not done, might lead to undesired errors like:
> > [ 1965.189137] [drm:fw_domains_get] *ERROR* blitter: timed out waiting for forcewake ack to clear.
> > 
> > v2: Added elaborate commit message. (Jani)
> >     Fixing RPM reference drop in early exit paths. (Ville)
> > 
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This seems to have fallem through some crack. I went and pushed it to dinq
now. Thanks for the patch.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_sysfs.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > index c6188dd..2d576b7 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> >  
> >  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> >  
> > +	intel_runtime_pm_get(dev_priv);
> > +
> >  	mutex_lock(&dev_priv->rps.hw_lock);
> >  
> >  	val = intel_freq_opcode(dev_priv, val);
> > @@ -378,6 +380,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> >  	    val > dev_priv->rps.max_freq ||
> >  	    val < dev_priv->rps.min_freq_softlimit) {
> >  		mutex_unlock(&dev_priv->rps.hw_lock);
> > +		intel_runtime_pm_put(dev_priv);
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -398,6 +401,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> >  
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> > +	intel_runtime_pm_put(dev_priv);
> > +
> >  	return count;
> >  }
> >  
> > @@ -433,6 +438,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> >  
> >  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> >  
> > +	intel_runtime_pm_get(dev_priv);
> > +
> >  	mutex_lock(&dev_priv->rps.hw_lock);
> >  
> >  	val = intel_freq_opcode(dev_priv, val);
> > @@ -441,6 +448,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> >  	    val > dev_priv->rps.max_freq ||
> >  	    val > dev_priv->rps.max_freq_softlimit) {
> >  		mutex_unlock(&dev_priv->rps.hw_lock);
> > +		intel_runtime_pm_put(dev_priv);
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -457,6 +465,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> >  
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> > +	intel_runtime_pm_put(dev_priv);
> > +
> >  	return count;
> >  
> >  }
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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