[Intel-gfx] [PATCH v3 4/7] drm/i915: add support for checking if we hold an RPM reference

Chris Wilson chris at chris-wilson.co.uk
Thu Nov 12 14:27:20 PST 2015


On Thu, Nov 12, 2015 at 10:49:29PM +0200, Imre Deak wrote:
> On Thu, 2015-11-12 at 20:41 +0000, Chris Wilson wrote:
> > On Thu, Nov 12, 2015 at 07:50:09PM +0200, Imre Deak wrote:
> > > On to, 2015-11-12 at 17:04 +0000, Chris Wilson wrote:
> > > > As it stands since we don't actually cancel the hangcheck when we
> > > drop
> > > > the rpm wakelock in intel_mark_idle() it can very well come to pass
> > > > that
> > > > we execute this whilst the device is asleep. However, if the device
> > > > is
> > > > alseep, we now that we are no longer executing.
> > > 
> > > But how could this run while asleep, since we flush the work in the
> > > runtime suspend handler before turning off the HW? But even if it can't
> > > run your idea would be clearer imo..
> > 
> > We shouldn't be flushing the hangcheck worker there. That's a leaky
> > abstraction attempting to paper over something that shouldn't have been
> > a problem in the first place. Note that there is a similar issue with
> > the existing request waiters that currently may race against
> > intel_mark_idle().
> 
> Ok. I think your idea is an improvement, but it will change
> functionality, so how about doing it as a follow-up?

Yes, since intel_runtime_suspend() already has a mechanism that should
avoid the assert, relaxing that assert and removing that bit of
unnecessary work from the rpm suspend path can be done separately. (I
hadn't realised that cancel_work had snuck in there.)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list