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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx