On Thursday, December 10, 2015 11:20:40 PM Imre Deak wrote:
> 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@xxxxxxxxxxxxxxxxxx>
> > > > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > > > > Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
> > > > > > Cc: linux-pm@xxxxxxxxxxxxxxx
> > > > >
> > > > > 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.
So first of all the name "pm_runtime_get_noidle" doesn't make sense.
I guess what you need is something like
bool pm_runtime_get_if_active(struct device *dev)
{
unsigned log flags;
bool ret;
spin_lock_irqsave(&dev->power.lock, flags);
if (dev->power.runtime_status == RPM_ACTIVE) {
atomic_inc(&dev->power.usage_count);
ret = true;
} else {
ret = false;
}
spin_unlock_irqrestore(&dev->power.lock, flags);
}
and the caller will simply bail out if "false" is returned, but if "true"
is returned, it will have to drop the usage count, right?
Thanks,
Rafael
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
