On Friday, December 11, 2015 04:59:45 PM Ulf Hansson wrote: > On 11 December 2015 at 16:13, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > > On Friday, December 11, 2015 01:03:50 PM Ulf Hansson wrote: > >> [...] > >> > >> >> > > >> >> > 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 > >> > > >> > >> Why not just: > >> > >> pm_runtime_get_noresume(): > >> if (RPM_ACTIVE) > >> "do some actions" > >> pm_runtime_put(); > > > > Because that's racy? > > Right, that was too easy. :-) > > > > > What if the rpm_suspend() is running for the device, but it hasn't changed > > the status yet? > > So if we can add a pm_runtime_barrier() or even simplifier, just hold > the spin_lock when checking if the rpm status is RPM_ACTIVE. That would work too, but then we'd need to carry out one extra atomic operation. Thanks, Rafael _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx