[PATCH] drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[PATCH] drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Chris Wilson]

commit 09731280028ce03e6a27e1998137f1775a2839f3
Author: Imre Deak <imre.deak@intel.com>
Date:   Wed Feb 17 14:17:42 2016 +0200

    drm/i915: Add helper to get a display power ref if it was already enabled

left the rpm wakelock assertions unbalanced if CONFIG_PM was disabled as
intel_runtime_pm_get_if_in_use() would return true without incrementing
the local bookkeeping required for the assertions.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
CC: Mika Kuoppala <mika.kuoppala@intel.com>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e2329768902c..4172e73212cd 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2365,22 +2365,20 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	struct device *device = &dev->pdev->dev;
-	int ret;
 
-	if (!IS_ENABLED(CONFIG_PM))
-		return true;
+	if (IS_ENABLED(CONFIG_PM)) {
+		int ret = pm_runtime_get_if_in_use(device);
 
-	ret = pm_runtime_get_if_in_use(device);
-
-	/*
-	 * In cases runtime PM is disabled by the RPM core and we get an
-	 * -EINVAL return value we are not supposed to call this function,
-	 * since the power state is undefined. This applies atm to the
-	 * late/early system suspend/resume handlers.
-	 */
-	WARN_ON_ONCE(ret < 0);
-	if (ret <= 0)
-		return false;
+		/*
+		 * In cases runtime PM is disabled by the RPM core and we get
+		 * an -EINVAL return value we are not supposed to call this
+		 * function, since the power state is undefined. This applies
+		 * atm to the late/early system suspend/resume handlers.
+		 */
+		WARN_ON_ONCE(ret < 0);
+		if (ret <= 0)
+			return false;
+	}
 
 	atomic_inc(&dev_priv->pm.wakeref_count);
 	assert_rpm_wakelock_held(dev_priv);
-- 
2.7.0

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

Re: [PATCH] drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Imre Deak]

On Thu, 2016-02-25 at 21:10 +0000, Chris Wilson wrote:
> commit 09731280028ce03e6a27e1998137f1775a2839f3
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Wed Feb 17 14:17:42 2016 +0200
> 
>     drm/i915: Add helper to get a display power ref if it was already
> enabled
> 
> left the rpm wakelock assertions unbalanced if CONFIG_PM was disabled
> as
> intel_runtime_pm_get_if_in_use() would return true without
> incrementing
> the local bookkeeping required for the assertions.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Mika Kuoppala <mika.kuoppala@intel.com>
> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>

Arg, I broke this in v3. Thanks for catching it:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 26 ++++++++++++-----------
> ---
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e2329768902c..4172e73212cd 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2365,22 +2365,20 @@ bool intel_runtime_pm_get_if_in_use(struct
> drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
> -	int ret;
>  
> -	if (!IS_ENABLED(CONFIG_PM))
> -		return true;
> +	if (IS_ENABLED(CONFIG_PM)) {
> +		int ret = pm_runtime_get_if_in_use(device);
>  
> -	ret = pm_runtime_get_if_in_use(device);
> -
> -	/*
> -	 * In cases runtime PM is disabled by the RPM core and we
> get an
> -	 * -EINVAL return value we are not supposed to call this
> function,
> -	 * since the power state is undefined. This applies atm to
> the
> -	 * late/early system suspend/resume handlers.
> -	 */
> -	WARN_ON_ONCE(ret < 0);
> -	if (ret <= 0)
> -		return false;
> +		/*
> +		 * In cases runtime PM is disabled by the RPM core
> and we get
> +		 * an -EINVAL return value we are not supposed to
> call this
> +		 * function, since the power state is undefined.
> This applies
> +		 * atm to the late/early system suspend/resume
> handlers.
> +		 */
> +		WARN_ON_ONCE(ret < 0);
> +		if (ret <= 0)
> +			return false;
> +	}
>  
>  	atomic_inc(&dev_priv->pm.wakeref_count);
>  	assert_rpm_wakelock_held(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Imre Deak]

On pe, 2016-02-26 at 00:20 +0200, Imre Deak wrote:
> On Thu, 2016-02-25 at 21:10 +0000, Chris Wilson wrote:
> > commit 09731280028ce03e6a27e1998137f1775a2839f3
> > Author: Imre Deak <imre.deak@intel.com>
> > Date:   Wed Feb 17 14:17:42 2016 +0200
> > 
> >     drm/i915: Add helper to get a display power ref if it was
> > already
> > enabled
> > 
> > left the rpm wakelock assertions unbalanced if CONFIG_PM was
> > disabled
> > as
> > intel_runtime_pm_get_if_in_use() would return true without
> > incrementing
> > the local bookkeeping required for the assertions.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > CC: Mika Kuoppala <mika.kuoppala@intel.com>
> > CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> 
> Arg, I broke this in v3. Thanks for catching it:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks for the patch I pushed it to drm-intel-next-queued.

Dave, this fixes one patch in the following pull request:
https://lists.freedesktop.org/archives/intel-gfx/2016-February/088249.html

--Imre

> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 26 ++++++++++++---------
> > --
> > ---
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index e2329768902c..4172e73212cd 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -2365,22 +2365,20 @@ bool intel_runtime_pm_get_if_in_use(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	struct drm_device *dev = dev_priv->dev;
> >  	struct device *device = &dev->pdev->dev;
> > -	int ret;
> >  
> > -	if (!IS_ENABLED(CONFIG_PM))
> > -		return true;
> > +	if (IS_ENABLED(CONFIG_PM)) {
> > +		int ret = pm_runtime_get_if_in_use(device);
> >  
> > -	ret = pm_runtime_get_if_in_use(device);
> > -
> > -	/*
> > -	 * In cases runtime PM is disabled by the RPM core and we
> > get an
> > -	 * -EINVAL return value we are not supposed to call this
> > function,
> > -	 * since the power state is undefined. This applies atm to
> > the
> > -	 * late/early system suspend/resume handlers.
> > -	 */
> > -	WARN_ON_ONCE(ret < 0);
> > -	if (ret <= 0)
> > -		return false;
> > +		/*
> > +		 * In cases runtime PM is disabled by the RPM core
> > and we get
> > +		 * an -EINVAL return value we are not supposed to
> > call this
> > +		 * function, since the power state is undefined.
> > This applies
> > +		 * atm to the late/early system suspend/resume
> > handlers.
> > +		 */
> > +		WARN_ON_ONCE(ret < 0);
> > +		if (ret <= 0)
> > +			return false;
> > +	}
> >  
> >  	atomic_inc(&dev_priv->pm.wakeref_count);
> >  	assert_rpm_wakelock_held(dev_priv);
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.BAT: failure for drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Patchwork]

== Series Details ==

Series: drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)
URL   : https://patchwork.freedesktop.org/series/3827/
State : failure

== Summary ==

Series 3827v1 drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)
http://patchwork.freedesktop.org/api/1.0/series/3827/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> INCOMPLETE (snb-dellxps)
Test gem_sync:
        Subgroup basic-bsd:
                pass       -> DMESG-FAIL (hsw-brixbox)
                pass       -> DMESG-FAIL (ilk-hp8440p)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (snb-x220t)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (bsw-nuc-2)
                fail       -> PASS       (bdw-nuci7)
                fail       -> PASS       (bdw-ultra)

bdw-nuci7        total:165  pass:154  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:168  pass:154  dwarn:0   dfail:0   fail:0   skip:14 
bsw-nuc-2        total:168  pass:136  dwarn:1   dfail:0   fail:1   skip:30 
byt-nuc          total:168  pass:143  dwarn:0   dfail:0   fail:0   skip:25 
hsw-brixbox      total:168  pass:153  dwarn:0   dfail:1   fail:0   skip:14 
hsw-gt2          total:168  pass:157  dwarn:0   dfail:1   fail:0   skip:10 
ilk-hp8440p      total:168  pass:117  dwarn:0   dfail:2   fail:0   skip:49 
ivb-t430s        total:168  pass:153  dwarn:0   dfail:0   fail:1   skip:14 
skl-i7k-2        total:168  pass:151  dwarn:1   dfail:0   fail:0   skip:16 
snb-dellxps      total:154  pass:132  dwarn:0   dfail:0   fail:1   skip:20 
snb-x220t        total:168  pass:144  dwarn:0   dfail:0   fail:3   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1479/

38f0cd5314ecb02d15d019631e388d3642275600 drm-intel-nightly: 2016y-02m-25d-15h-33m-53s UTC integration manifest
f7a63002de7ec2829aeae48527217bc4d1d9b106 drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

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

Re: ✗ Fi.CI.BAT: failure for drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Imre Deak]

On Fri, 2016-02-26 at 07:00 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Balance assert_rpm_wakelock_held() for
> !IS_ENABLED(CONFIG_PM)
> URL   : https://patchwork.freedesktop.org/series/3827/
> State : failure
> 
> == Summary ==
> 
> Series 3827v1 drm/i915: Balance assert_rpm_wakelock_held() for
> !IS_ENABLED(CONFIG_PM)
> http://patchwork.freedesktop.org/api/1.0/series/3827/revisions/1/mbox
> /
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> INCOMPLETE (snb-dellxps)
> Test gem_sync:
>         Subgroup basic-bsd:
>                 pass       -> DMESG-FAIL (hsw-brixbox)
>                 pass       -> DMESG-FAIL (ilk-hp8440p)
> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> FAIL       (snb-x220t)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 dmesg-warn -> PASS       (byt-nuc)
>         Subgroup basic-rte:
>                 pass       -> DMESG-WARN (bsw-nuc-2)
>                 fail       -> PASS       (bdw-nuci7)
>                 fail       -> PASS       (bdw-ultra)

All tests are run with CONFIG_PM enabled, and the patch changes
behavior only with !CONFIG_PM. Even in case of !CONFIG_PM only the
generation of wakeref messages are affected and not any other
functionality. So I'm confident the above failures are unrelated.

> bdw-
> nuci7        total:165  pass:154  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> bdw-
> ultra        total:168  pass:154  dwarn:0   dfail:0   fail:0   skip:1
> 4 
> bsw-nuc-
> 2        total:168  pass:136  dwarn:1   dfail:0   fail:1   skip:30 
> byt-
> nuc          total:168  pass:143  dwarn:0   dfail:0   fail:0   skip:2
> 5 
> hsw-
> brixbox      total:168  pass:153  dwarn:0   dfail:1   fail:0   skip:1
> 4 
> hsw-
> gt2          total:168  pass:157  dwarn:0   dfail:1   fail:0   skip:1
> 0 
> ilk-
> hp8440p      total:168  pass:117  dwarn:0   dfail:2   fail:0   skip:4
> 9 
> ivb-
> t430s        total:168  pass:153  dwarn:0   dfail:0   fail:1   skip:1
> 4 
> skl-i7k-
> 2        total:168  pass:151  dwarn:1   dfail:0   fail:0   skip:16 
> snb-
> dellxps      total:154  pass:132  dwarn:0   dfail:0   fail:1   skip:2
> 0 
> snb-
> x220t        total:168  pass:144  dwarn:0   dfail:0   fail:3   skip:2
> 1 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1479/
> 
> 38f0cd5314ecb02d15d019631e388d3642275600 drm-intel-nightly: 2016y-
> 02m-25d-15h-33m-53s UTC integration manifest
> f7a63002de7ec2829aeae48527217bc4d1d9b106 drm/i915: Balance
> assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx