[PATCH] drm/i915: don't enable autosuspend on platforms without RPM support

[PATCH] drm/i915: don't enable autosuspend on platforms without RPM support

[Imre Deak]

Since

commit 357597e51dd1aa3cf764d322abf89217e3dcd7bb
Author: Imre Deak <imre.deak@intel.com>
Date:   Tue Nov 10 06:12:22 2015 +0200

    drm/i915: remove HAS_RUNTIME_PM check from RPM get/put/assert helpers

this file is writeable also on platforms without RPM support, but
userspace (at least IGT) depends on this file being unchangable to
determine whether the device supports autosuspend. So restore the old
behavior.

This gets rid of igt/pm_rpm failures on old platforms without RPM
support, where the test should be skipped.

Testcase: igt/pm_rpm/basic-rte
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index a0b9eaf..ddbdbff 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2309,18 +2309,21 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = dev_priv->dev;
 	struct device *device = &dev->pdev->dev;
 
+	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
+	pm_runtime_mark_last_busy(device);
+
 	/*
 	 * Take a permanent reference to disable the RPM functionality and drop
 	 * it only when unloading the driver. Use the low level get/put helpers,
 	 * so the driver's own RPM reference tracking asserts also work on
 	 * platforms without RPM support.
 	 */
-	if (!HAS_RUNTIME_PM(dev))
+	if (!HAS_RUNTIME_PM(dev)) {
+		pm_runtime_dont_use_autosuspend(device);
 		pm_runtime_get_sync(device);
-
-	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
-	pm_runtime_mark_last_busy(device);
-	pm_runtime_use_autosuspend(device);
+	} else {
+		pm_runtime_use_autosuspend(device);
+	}
 
 	/*
 	 * The core calls the driver load handler with an RPM reference held.
-- 
2.5.0

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

Re: [PATCH] drm/i915: don't enable autosuspend on platforms without RPM support

[Chris Wilson]

On Thu, Dec 17, 2015 at 07:04:33PM +0200, Imre Deak wrote:
> Since
> 
> commit 357597e51dd1aa3cf764d322abf89217e3dcd7bb
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Tue Nov 10 06:12:22 2015 +0200
> 
>     drm/i915: remove HAS_RUNTIME_PM check from RPM get/put/assert helpers
> 
> this file is writeable also on platforms without RPM support, but

Which file?

> userspace (at least IGT) depends on this file being unchangable to
> determine whether the device supports autosuspend. So restore the old
> behavior.
> 
> This gets rid of igt/pm_rpm failures on old platforms without RPM
> support, where the test should be skipped.
> 
> Testcase: igt/pm_rpm/basic-rte
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index a0b9eaf..ddbdbff 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2309,18 +2309,21 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
>  
> +	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> +	pm_runtime_mark_last_busy(device);
> +
>  	/*
>  	 * Take a permanent reference to disable the RPM functionality and drop
>  	 * it only when unloading the driver. Use the low level get/put helpers,
>  	 * so the driver's own RPM reference tracking asserts also work on
>  	 * platforms without RPM support.
>  	 */
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev)) {
> +		pm_runtime_dont_use_autosuspend(device);
>  		pm_runtime_get_sync(device);
> -
> -	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> -	pm_runtime_mark_last_busy(device);
> -	pm_runtime_use_autosuspend(device);
> +	} else {
> +		pm_runtime_use_autosuspend(device);
> +	}

This seems to make sense to me.
-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: don't enable autosuspend on platforms without RPM support

[Imre Deak]

On to, 2015-12-17 at 17:19 +0000, Chris Wilson wrote:
> On Thu, Dec 17, 2015 at 07:04:33PM +0200, Imre Deak wrote:
> > Since
> > 
> > commit 357597e51dd1aa3cf764d322abf89217e3dcd7bb
> > Author: Imre Deak <imre.deak@intel.com>
> > Date:   Tue Nov 10 06:12:22 2015 +0200
> > 
> >     drm/i915: remove HAS_RUNTIME_PM check from RPM get/put/assert
> > helpers
> > 
> > this file is writeable also on platforms without RPM support, but
> 
> Which file?

sysfs power/autosuspend_delay_ms is only writeable by userspace if we
call pm_runtime_use_autosuspend().

> 
> > userspace (at least IGT) depends on this file being unchangable to
> > determine whether the device supports autosuspend. So restore the
> > old
> > behavior.
> > 
> > This gets rid of igt/pm_rpm failures on old platforms without RPM
> > support, where the test should be skipped.
> > 
> > Testcase: igt/pm_rpm/basic-rte
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index a0b9eaf..ddbdbff 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -2309,18 +2309,21 @@ void intel_runtime_pm_enable(struct
> > drm_i915_private *dev_priv)
> >  	struct drm_device *dev = dev_priv->dev;
> >  	struct device *device = &dev->pdev->dev;
> >  
> > +	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > +	pm_runtime_mark_last_busy(device);
> > +
> >  	/*
> >  	 * Take a permanent reference to disable the RPM
> > functionality and drop
> >  	 * it only when unloading the driver. Use the low level
> > get/put helpers,
> >  	 * so the driver's own RPM reference tracking asserts also
> > work on
> >  	 * platforms without RPM support.
> >  	 */
> > -	if (!HAS_RUNTIME_PM(dev))
> > +	if (!HAS_RUNTIME_PM(dev)) {
> > +		pm_runtime_dont_use_autosuspend(device);
> >  		pm_runtime_get_sync(device);
> > -
> > -	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > -	pm_runtime_mark_last_busy(device);
> > -	pm_runtime_use_autosuspend(device);
> > +	} else {
> > +		pm_runtime_use_autosuspend(device);
> > +	}
> 
> This seems to make sense to me.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on ac2305b6c91b9a84cc12566016ece257c3ebcba3 drm-intel-nightly: 2015y-12m-17d-16h-19m-23s UTC integration manifest

Test igt@kms_flip@basic-flip-vs-wf_vblank on bsw-nuc-2 pass -> dmesg-warn
Test igt@kms_flip@basic-flip-vs-wf_vblank on skl-i7k-2 dmesg-fail -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence on bsw-nuc-2 dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-dpms on ilk-hp8440p pass -> dmesg-warn
Test igt@pm_rpm@basic-rte on ilk-hp8440p fail -> skip
Test igt@pm_rpm@basic-rte on ivb-t430s dmesg-fail -> skip
Test igt@pm_rpm@basic-pci-d3-state on ilk-hp8440p fail -> skip
Test igt@pm_rpm@basic-pci-d3-state on ivb-t430s fail -> skip
Test igt@kms_flip@basic-plain-flip on ivb-t430s pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b on snb-x220t dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on snb-x220t pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c on skl-i7k-2 dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-modeset on bsw-nuc-2 dmesg-warn -> pass

bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:99   dwarn:1   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:122  dwarn:5   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_705/

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

Re: [PATCH] drm/i915: don't enable autosuspend on platforms without RPM support

[David Weinehall]

On Thu, Dec 17, 2015 at 07:04:33PM +0200, Imre Deak wrote:
> Since
> 
> commit 357597e51dd1aa3cf764d322abf89217e3dcd7bb
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Tue Nov 10 06:12:22 2015 +0200
> 
>     drm/i915: remove HAS_RUNTIME_PM check from RPM get/put/assert helpers
> 
> this file is writeable also on platforms without RPM support, but
> userspace (at least IGT) depends on this file being unchangable to
> determine whether the device supports autosuspend. So restore the old
> behavior.
> 
> This gets rid of igt/pm_rpm failures on old platforms without RPM
> support, where the test should be skipped.
> 
> Testcase: igt/pm_rpm/basic-rte
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index a0b9eaf..ddbdbff 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2309,18 +2309,21 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
>  
> +	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> +	pm_runtime_mark_last_busy(device);
> +
>  	/*
>  	 * Take a permanent reference to disable the RPM functionality and drop
>  	 * it only when unloading the driver. Use the low level get/put helpers,
>  	 * so the driver's own RPM reference tracking asserts also work on
>  	 * platforms without RPM support.
>  	 */
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev)) {
> +		pm_runtime_dont_use_autosuspend(device);
>  		pm_runtime_get_sync(device);
> -
> -	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> -	pm_runtime_mark_last_busy(device);
> -	pm_runtime_use_autosuspend(device);
> +	} else {
> +		pm_runtime_use_autosuspend(device);
> +	}
>  
>  	/*
>  	 * The core calls the driver load handler with an RPM reference held.

Reviewed-by: David Weinehall <david.weinehall@intel.com>


Kind regards, David Weinehall
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: don't enable autosuspend on platforms without RPM support

[Imre Deak]

On pe, 2015-12-18 at 12:32 +0200, David Weinehall wrote:
> On Thu, Dec 17, 2015 at 07:04:33PM +0200, Imre Deak wrote:
> > Since
> > 
> > commit 357597e51dd1aa3cf764d322abf89217e3dcd7bb
> > Author: Imre Deak <imre.deak@intel.com>
> > Date:   Tue Nov 10 06:12:22 2015 +0200
> > 
> >     drm/i915: remove HAS_RUNTIME_PM check from RPM get/put/assert
> > helpers
> > 
> > this file is writeable also on platforms without RPM support, but
> > userspace (at least IGT) depends on this file being unchangable to
> > determine whether the device supports autosuspend. So restore the
> > old
> > behavior.
> > 
> > This gets rid of igt/pm_rpm failures on old platforms without RPM
> > support, where the test should be skipped.
> > 
> > Testcase: igt/pm_rpm/basic-rte
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index a0b9eaf..ddbdbff 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -2309,18 +2309,21 @@ void intel_runtime_pm_enable(struct
> > drm_i915_private *dev_priv)
> >  	struct drm_device *dev = dev_priv->dev;
> >  	struct device *device = &dev->pdev->dev;
> >  
> > +	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > +	pm_runtime_mark_last_busy(device);
> > +
> >  	/*
> >  	 * Take a permanent reference to disable the RPM
> > functionality and drop
> >  	 * it only when unloading the driver. Use the low level
> > get/put helpers,
> >  	 * so the driver's own RPM reference tracking asserts also
> > work on
> >  	 * platforms without RPM support.
> >  	 */
> > -	if (!HAS_RUNTIME_PM(dev))
> > +	if (!HAS_RUNTIME_PM(dev)) {
> > +		pm_runtime_dont_use_autosuspend(device);
> >  		pm_runtime_get_sync(device);
> > -
> > -	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > -	pm_runtime_mark_last_busy(device);
> > -	pm_runtime_use_autosuspend(device);
> > +	} else {
> > +		pm_runtime_use_autosuspend(device);
> > +	}
> >  
> >  	/*
> >  	 * The core calls the driver load handler with an RPM
> > reference held.
> 
> Reviewed-by: David Weinehall <david.weinehall@intel.com>

Thanks for the review, I pushed the patch to dinq.
While applying I clarified the commit message about which sysfs file
this change is about and fixed the reference to the regressing commit,
it was one before the commit I mentioned.

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