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

[Imre Deak <imre.deak@intel.com>]

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