Re: [PATCH v2] PM / Runtime: Introduce pm_runtime_get_noidle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/12/15 22:14, Rafael J. Wysocki wrote:
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.

How about pm_runtime_get_unless_idle(), which would be analogous to kref_get_unless_zero() ?

.Dave.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux