Re: [Intel-gfx] [PATCH v2] PM / Runtime: Introduce pm_runtime_get_noidle

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

On Thu, 2015-12-10 at 22:42 +0100, Rafael J. Wysocki wrote:
> On Thursday, December 10, 2015 10:36:37 PM Rafael J. Wysocki wrote:
> > On Thursday, December 10, 2015 11:43:50 AM Imre Deak wrote:
> > > On Thu, 2015-12-10 at 01:58 +0100, Rafael J. Wysocki wrote:
> > > > On Wednesday, December 09, 2015 06:22:19 PM Joonas Lahtinen
> > > > wrote:
> > > > > Introduce pm_runtime_get_noidle to for situations where it is
> > > > > not
> > > > > desireable to touch an idling device. One use scenario is
> > > > > periodic
> > > > > hangchecks performed by the drm/i915 driver which can be
> > > > > omitted
> > > > > on a device in a runtime idle state.
> > > > > 
> > > > > v2:
> > > > > - Fix inconsistent return value when !CONFIG_PM.
> > > > > - Update documentation for bool return value
> > > > > 
> > > > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.c
> > > > > om>
> > > > > Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > Cc: linux-pm@vger.kernel.org
> > > > 
> > > > Well, I don't quite see how this can be used in a non-racy way
> > > > without doing an additional pm_runtime_resume() or something
> > > > like
> > > > that in the same code path.
> > > 
> > > We don't want to resume, that would be the whole point. We'd like
> > > to
> > > ensure that we hold a reference _and_ the device is already
> > > active. So
> > > AFAICS we'd need to check runtime_status == RPM_ACTIVE in
> > > addition
> > > after taking the reference.
> > 
> > Right, and that under the lock.
> 
> Which basically means you can call pm_runtime_resume() just fine,
> because it will do nothing if the status is RPM_ACTIVE already.
> 
> So really, why don't you use pm_runtime_get_sync()?

The difference would be that if the status is not RPM_ACTIVE already we
would drop the reference and report error. The caller would in this
case forego of doing something, since we the device is suspended or on
the way to being suspended. One example of such a scenario is a
watchdog like functionality: the watchdog work would
call pm_runtime_get_noidle() and check if the device is ok by doing
some HW access, but only if the device is powered. Otherwise the work
item would do nothing (meaning it also won't reschedule itself). The
watchdog work would get rescheduled next time the device is woken up
and some work is submitted to the device.

--Imre