Re: [PATCH 1/4] drm/i915: Fix error path in i915_drm_resume_early

Imre Deak
2016-04-18T08:06:58Z

On ma, 2016-04-18 at 09:00 +0100, Chris Wilson wrote:
> On Mon, Apr 18, 2016 at 10:04:19AM +0300, Imre Deak wrote:
> > If system resume fails, this may lead to a runtime PM wake
> > reference
> > underflow used for runtime PM state checking.
>
> If a later resume succeeds!
>
> > Fixes: 1f814daca43a ("drm/i915: add support for checking if we hold
> > an RPM reference")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> > ---
> >
> > As this only fixes a debugging feature, it's not for -fixes or
> > stable.
>
> Following a catastrophic device failure.
>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 50bc05f..d550ae2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -842,11 +842,11 @@ static int i915_drm_resume_early(struct
> > drm_device *dev)
> >       !(dev_priv->suspended_to_idle && dev_priv-
> > >csr.dmc_payload))
> >   intel_power_domains_init_hw(dev_priv, true);
> >
> > + enable_rpm_wakeref_asserts(dev_priv);
> > +
> >  out:
> >   dev_priv->suspended_to_idle = false;
>
> I'm not even sure about twiddling anything here, an early return if
> pci_enable_device() fails would be clearer.

Yea good point I was also thinking about it. I have to check a bit what
exactly happens in case resume fails (for example wrt. subsequent
IOCTLs) and can fix this up.

--Imre