Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1230103 > unrolled thread

Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

Started byOliver Neukum <oneukum@suse.com>
First post2015-09-22 14:10 +0200
Last post2015-09-28 15:20 +0200
Articles 20 on this page of 23 — 7 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Oliver Neukum <oneukum@suse.com> - 2015-09-22 14:10 +0200
    Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-22 16:20 +0200
      Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Oliver Neukum <oneukum@suse.de> - 2015-09-22 16:40 +0200
        Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-22 17:30 +0200
          Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Oliver Neukum <oneukum@suse.de> - 2015-09-23 05:10 +0200
            Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Octavian Purdila <octavian.purdila@intel.com> - 2015-09-23 09:30 +0200
            Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-23 17:00 +0200
              Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-25 02:20 +0200
                Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-25 16:40 +0200
                  Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-25 21:50 +0200
                    Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-25 23:20 +0200
                      Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-25 23:30 +0200
                        Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-26 00:40 +0200
                          Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-26 17:30 +0200
                            Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-27 15:20 +0200
                              Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-27 16:30 +0200
                                Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-28 15:20 +0200
                                  Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-28 16:30 +0200
                                    Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend "Rafael J. Wysocki" <rafael@kernel.org> - 2015-09-28 22:10 +0200
                                      Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Alan Stern <stern@rowland.harvard.edu> - 2015-09-28 22:30 +0200
                                        Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Pavel Machek <pavel@ucw.cz> - 2015-10-04 17:20 +0200
                              Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing  runtime suspend Pavel Machek <pavel@ucw.cz> - 2015-09-27 19:10 +0200
                                Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-28 15:20 +0200

Page 1 of 2  [1] 2  Next page →


#1230103 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

FromOliver Neukum <oneukum@suse.com>
Date2015-09-22 14:10 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qbuyu-4T3-3@gated-at.bofh.it>
On Mon, 2015-09-21 at 16:02 -0400, Alan Stern wrote:
> On Mon, 21 Sep 2015, Dmitry Torokhov wrote:
> 
> > > What happens if the "inhibit" control is turned on and the driver puts 
> > > the device into runtime suspend, but then an I/O request arrives?
> > > 
> > > 	If the I/O request originated from userspace, it means the
> > > 	user is violating the terms of the "inhibit" control.  Should
> > > 	the request simply fail?
> > 
> > What user? User that inhibited it or user that tried to use the device?
> 
> Normally they would be the same.  But even if they aren't, someone has 
> violated the kernel interface: The first user told the kernel a 
> particular device wasn't going to be used, and then the second user 
> tried to use it.

If we assume that user space speaks with a uniform voice on that
issue, it can just as well close the device. It seems to me that
declaring a device idle is a privileged operation.

> Of course, this issue doesn't arise for devices that merely report 
> external events.

Indeed. We can handle output to suspended devices by waking them.
I don't see why this case is different. We are talking about input
only.

> The runtime-PM "usage" value for these devices is a little tricky to 
> calculate.  It should be nonzero if there are any open files _and_ the 
> device isn't "inhibited".  I don't know the best way to represent that 
> kind of condition in the runtime PM framework.

Does that make sense in the generic framework at all? I still
think that drivers should cease IO for input in such cases.
That should involve a common callback, but no counter.

	Regards
		Oliver



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1230250

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-22 16:20 +0200
Message-ID<qbwAi-7K8-15@gated-at.bofh.it>
In reply to#1230103
On Tue, 22 Sep 2015, Oliver Neukum wrote:

> Indeed. We can handle output to suspended devices by waking them.
> I don't see why this case is different. We are talking about input
> only.
> 
> > The runtime-PM "usage" value for these devices is a little tricky to 
> > calculate.  It should be nonzero if there are any open files _and_ the 
> > device isn't "inhibited".  I don't know the best way to represent that 
> > kind of condition in the runtime PM framework.
> 
> Does that make sense in the generic framework at all? I still
> think that drivers should cease IO for input in such cases.
> That should involve a common callback, but no counter.

I'm not sure I understand what you're saying.  Are you suggesting that
this "inhibit" mechanism should involve a new callback different from
the existing runtime-PM callbacks?  And when this new callback is
invoked, drivers should cancel existing input requests (these devices
are input-only) and go to low power?

This would create a parallel runtime-PM mechanism which is independent
of the existing one.  Is that really a good idea?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1230286

FromOliver Neukum <oneukum@suse.de>
Date2015-09-22 16:40 +0200
Message-ID<qbwTF-86z-55@gated-at.bofh.it>
In reply to#1230250
On Tue, 2015-09-22 at 10:15 -0400, Alan Stern wrote:
> On Tue, 22 Sep 2015, Oliver Neukum wrote:
> 
> > Indeed. We can handle output to suspended devices by waking them.
> > I don't see why this case is different. We are talking about input
> > only.
> > 
> > > The runtime-PM "usage" value for these devices is a little tricky to 
> > > calculate.  It should be nonzero if there are any open files _and_ the 
> > > device isn't "inhibited".  I don't know the best way to represent that 
> > > kind of condition in the runtime PM framework.
> > 
> > Does that make sense in the generic framework at all? I still
> > think that drivers should cease IO for input in such cases.
> > That should involve a common callback, but no counter.
> 
> I'm not sure I understand what you're saying.  Are you suggesting that
> this "inhibit" mechanism should involve a new callback different from

Yes, there is no necessary relation to power management. If you put
your phone into your pocket, you will want to inhibit the touchscreen
even if that doesn't save power.

> the existing runtime-PM callbacks?  And when this new callback is
> invoked, drivers should cancel existing input requests (these devices
> are input-only) and go to low power?

Cancel, yes, going to low power is a consequence which needn't bother
the power subsystem. You need a callback. If there are spurious
events, the current heuristics will keep devices awake.
You must discard them anyway, as they are spurious. There's no point
in transporting over the bus at all. We can cease IO for input.

> This would create a parallel runtime-PM mechanism which is independent
> of the existing one.  Is that really a good idea?

It isn't strictly PM. It helps PM to do a better job, but
conceptually it is independent.

	Regards
		Oliver



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1230329

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-22 17:30 +0200
Message-ID<qbxG3-P2-35@gated-at.bofh.it>
In reply to#1230286
On Tue, 22 Sep 2015, Oliver Neukum wrote:

> > I'm not sure I understand what you're saying.  Are you suggesting that
> > this "inhibit" mechanism should involve a new callback different from
> 
> Yes, there is no necessary relation to power management. If you put
> your phone into your pocket, you will want to inhibit the touchscreen
> even if that doesn't save power.
> 
> > the existing runtime-PM callbacks?  And when this new callback is
> > invoked, drivers should cancel existing input requests (these devices
> > are input-only) and go to low power?
> 
> Cancel, yes, going to low power is a consequence which needn't bother
> the power subsystem.

Going to low power needn't involve the power subsystem?  That sounds 
weird.

>  You need a callback. If there are spurious
> events, the current heuristics will keep devices awake.
> You must discard them anyway, as they are spurious. There's no point
> in transporting over the bus at all. We can cease IO for input.
> 
> > This would create a parallel runtime-PM mechanism which is independent
> > of the existing one.  Is that really a good idea?
> 
> It isn't strictly PM. It helps PM to do a better job, but
> conceptually it is independent.

So my next question is: _How_ can this help PM to do a better job?  
That is, what are the mechanisms?

One you have already stated: Lack of spurious events will help prevent 
unwanted wakeups (or unwanted failures to go to sleep).

But Dmitry made a stronger claim: Inhibiting an input device should 
allow the device to go to low power.  I would like to know how we can 
implement this cleanly.  The most straightforward approach is to use 
runtime PM, but it's not obvious how this can be made to work with the 
current API.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1231118

FromOliver Neukum <oneukum@suse.de>
Date2015-09-23 05:10 +0200
Message-ID<qbIBr-8lQ-5@gated-at.bofh.it>
In reply to#1230329
On Tue, 2015-09-22 at 11:22 -0400, Alan Stern wrote:
> On Tue, 22 Sep 2015, Oliver Neukum wrote:
>  
> > Cancel, yes, going to low power is a consequence which needn't bother
> > the power subsystem.
> 
> Going to low power needn't involve the power subsystem?  That sounds 
> weird.

Think of it like rfkill. It makes sense to suspend an rfkilled device.
It still is the job of the driver to report that its device is idle.

> >  You need a callback. If there are spurious
> > events, the current heuristics will keep devices awake.
> > You must discard them anyway, as they are spurious. There's no point
> > in transporting over the bus at all. We can cease IO for input.
> > 
> > > This would create a parallel runtime-PM mechanism which is independent
> > > of the existing one.  Is that really a good idea?
> > 
> > It isn't strictly PM. It helps PM to do a better job, but
> > conceptually it is independent.
> 
> So my next question is: _How_ can this help PM to do a better job?  
> That is, what are the mechanisms?

"inhibit" -> driver stops input -> driver sets PM count to zero
-> PM subsystem acts

To go from the first to the second step a callback is needed

> One you have already stated: Lack of spurious events will help prevent 
> unwanted wakeups (or unwanted failures to go to sleep).

That too. We also save CPU cycles.

> But Dmitry made a stronger claim: Inhibiting an input device should 
> allow the device to go to low power.  I would like to know how we can 
> implement this cleanly.  The most straightforward approach is to use 
> runtime PM, but it's not obvious how this can be made to work with the 
> current API.

Yes, we can use the current API.
The key is that you think of the mechanism as induced idleness,
not forced suspend. We already have a perfectly working mechanism
for suspending idle devices.

	Regards
		Oliver




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1231188

FromOctavian Purdila <octavian.purdila@intel.com>
Date2015-09-23 09:30 +0200
Message-ID<qbMF3-5Lu-5@gated-at.bofh.it>
In reply to#1231118
On Wed, Sep 23, 2015 at 6:03 AM, Oliver Neukum <oneukum@suse.de> wrote:
>
> On Tue, 2015-09-22 at 11:22 -0400, Alan Stern wrote:
> > On Tue, 22 Sep 2015, Oliver Neukum wrote:
> >
> > > Cancel, yes, going to low power is a consequence which needn't bother
> > > the power subsystem.
> >
> > Going to low power needn't involve the power subsystem?  That sounds
> > weird.
>
> Think of it like rfkill. It makes sense to suspend an rfkilled device.
> It still is the job of the driver to report that its device is idle.
>
> > >  You need a callback. If there are spurious
> > > events, the current heuristics will keep devices awake.
> > > You must discard them anyway, as they are spurious. There's no point
> > > in transporting over the bus at all. We can cease IO for input.
> > >
> > > > This would create a parallel runtime-PM mechanism which is independent
> > > > of the existing one.  Is that really a good idea?
> > >
> > > It isn't strictly PM. It helps PM to do a better job, but
> > > conceptually it is independent.
> >
> > So my next question is: _How_ can this help PM to do a better job?
> > That is, what are the mechanisms?
>
> "inhibit" -> driver stops input -> driver sets PM count to zero
> -> PM subsystem acts
>
> To go from the first to the second step a callback is needed
>

The IIO drivers use this model. The application keeps the fd open but
there is a buffer enable switch to enable / disable input. Based on
that trigger drivers use pm runtime put operations to induce PM
idleness (and pm runtime get to wakeup the device).

> > One you have already stated: Lack of spurious events will help prevent
> > unwanted wakeups (or unwanted failures to go to sleep).
>
> That too. We also save CPU cycles.
>
> > But Dmitry made a stronger claim: Inhibiting an input device should
> > allow the device to go to low power.  I would like to know how we can
> > implement this cleanly.  The most straightforward approach is to use
> > runtime PM, but it's not obvious how this can be made to work with the
> > current API.
>
> Yes, we can use the current API.
> The key is that you think of the mechanism as induced idleness,
> not forced suspend. We already have a perfectly working mechanism
> for suspending idle devices.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1231490

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-23 17:00 +0200
Message-ID<qbTGx-7k3-9@gated-at.bofh.it>
In reply to#1231118
On Wed, 23 Sep 2015, Oliver Neukum wrote:

> On Tue, 2015-09-22 at 11:22 -0400, Alan Stern wrote:
> > On Tue, 22 Sep 2015, Oliver Neukum wrote:
> >  
> > > Cancel, yes, going to low power is a consequence which needn't bother
> > > the power subsystem.
> > 
> > Going to low power needn't involve the power subsystem?  That sounds 
> > weird.
> 
> Think of it like rfkill. It makes sense to suspend an rfkilled device.
> It still is the job of the driver to report that its device is idle.

Reporting that the device is idle _does_ involve the power subsystem.  
But never mind...

> > So my next question is: _How_ can this help PM to do a better job?  
> > That is, what are the mechanisms?
> 
> "inhibit" -> driver stops input -> driver sets PM count to zero
> -> PM subsystem acts

Your third step here poses problems.  There is no runtime-PM API a 
driver or subsystem can use to set its usage counter to 0.  All it can 
do is increment or decrement the counter.

> To go from the first to the second step a callback is needed
> 
> > One you have already stated: Lack of spurious events will help prevent 
> > unwanted wakeups (or unwanted failures to go to sleep).
> 
> That too. We also save CPU cycles.
> 
> > But Dmitry made a stronger claim: Inhibiting an input device should 
> > allow the device to go to low power.  I would like to know how we can 
> > implement this cleanly.  The most straightforward approach is to use 
> > runtime PM, but it's not obvious how this can be made to work with the 
> > current API.
> 
> Yes, we can use the current API.
> The key is that you think of the mechanism as induced idleness,
> not forced suspend. We already have a perfectly working mechanism
> for suspending idle devices.

After thinking some more about this, here's what I've got.

Let's talk about input-only devices being in an "idle" state or a
"running" state:

	When the device is in the idle state, its driver does not 
	communicate with the device.  In particular, the driver does 
	not monitor for events or report them to the rest of the 
	kernel.  It is appropriate to put the device in runtime suspend
	with remote wakeup disabled.

	When the device is in the running state, its driver receives
	event reports from the device and propagates them forward.
	The subsystem/driver may choose to put the device in runtime
	suspend between events with remote wakeup enabled (like we do
	for USB keyboards if no LEDs are on) or it may choose to leave
	the device at full power the whole time.

The idea is that a device is in the idle state whenever the open file 
count is 0 _or_ it is "inhibited"; otherwise it is in the running 
state.

I tried to come up with a way to do some of this work in a central
core.  All I could think of was that the core should detect state
changes and inform the subsystem/driver when they occur.  Everything
else (starting and stopping I/O, adjusting the runtime-PM usage
counter) would have to be done by the subsystem/driver.

The problem is that a core generally isn't aware of when a file 
reference is opened or released.  Only the driver is.  Which means 
there's nothing that the core can do here; the driver and subsystem 
have to manage pretty much the whole thing.  A simple example:

open()
{
	mutex_lock(&dev->open_mutex);
	if (dev->open_count++ == 0 && !dev->inhibited) {
		pm_runtime_get_sync(dev);
		start_io(dev);
	}
	mutex_unlock(&dev->open_mutex);
}

release()
{
	mutex_lock(&dev->open_mutex);
	if (--dev->open_count == 0 && !dev->inhibited) {
		stop_io(dev);
		pm_runtime_put(dev);
	}
	mutex_unlock(&dev->open_mutex);
}

inhibit()
{
	mutex_lock(&dev->open_mutex);
	dev->inhibited = true;
	if (dev->open_count > 0) {
		stop_io(dev);
		pm_runtime_put(dev);
	}
	mutex_unlock(&dev->open_mutex);
}

uninhibit()
{
	mutex_lock(&dev->open_mutex);
	dev->inhibited = false;
	if (dev->open_count > 0) {
		pm_runtime_get_sync(dev);
		start_io(dev);
	}
	mutex_unlock(&dev->open_mutex);
}

This doesn't leave much room for the PM core or anything else.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1232502 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-25 02:20 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qcoU2-22j-5@gated-at.bofh.it>
In reply to#1231490
On Wednesday, September 23, 2015 10:55:52 AM Alan Stern wrote:
> On Wed, 23 Sep 2015, Oliver Neukum wrote:
> 
> > On Tue, 2015-09-22 at 11:22 -0400, Alan Stern wrote:
> > > On Tue, 22 Sep 2015, Oliver Neukum wrote:
> > >  
> > > > Cancel, yes, going to low power is a consequence which needn't bother
> > > > the power subsystem.
> > > 
> > > Going to low power needn't involve the power subsystem?  That sounds 
> > > weird.
> > 
> > Think of it like rfkill. It makes sense to suspend an rfkilled device.
> > It still is the job of the driver to report that its device is idle.
> 
> Reporting that the device is idle _does_ involve the power subsystem.  
> But never mind...
> 
> > > So my next question is: _How_ can this help PM to do a better job?  
> > > That is, what are the mechanisms?
> > 
> > "inhibit" -> driver stops input -> driver sets PM count to zero
> > -> PM subsystem acts
> 
> Your third step here poses problems.  There is no runtime-PM API a 
> driver or subsystem can use to set its usage counter to 0.  All it can 
> do is increment or decrement the counter.
> 
> > To go from the first to the second step a callback is needed
> > 
> > > One you have already stated: Lack of spurious events will help prevent 
> > > unwanted wakeups (or unwanted failures to go to sleep).
> > 
> > That too. We also save CPU cycles.
> > 
> > > But Dmitry made a stronger claim: Inhibiting an input device should 
> > > allow the device to go to low power.  I would like to know how we can 
> > > implement this cleanly.  The most straightforward approach is to use 
> > > runtime PM, but it's not obvious how this can be made to work with the 
> > > current API.
> > 
> > Yes, we can use the current API.
> > The key is that you think of the mechanism as induced idleness,
> > not forced suspend. We already have a perfectly working mechanism
> > for suspending idle devices.
> 
> After thinking some more about this, here's what I've got.
> 
> Let's talk about input-only devices being in an "idle" state or a
> "running" state:
> 
> 	When the device is in the idle state, its driver does not 
> 	communicate with the device.  In particular, the driver does 
> 	not monitor for events or report them to the rest of the 
> 	kernel.  It is appropriate to put the device in runtime suspend
> 	with remote wakeup disabled.
> 
> 	When the device is in the running state, its driver receives
> 	event reports from the device and propagates them forward.
> 	The subsystem/driver may choose to put the device in runtime
> 	suspend between events with remote wakeup enabled (like we do
> 	for USB keyboards if no LEDs are on) or it may choose to leave
> 	the device at full power the whole time.
> 
> The idea is that a device is in the idle state whenever the open file 
> count is 0 _or_ it is "inhibited"; otherwise it is in the running 
> state.
> 
> I tried to come up with a way to do some of this work in a central
> core.  All I could think of was that the core should detect state
> changes and inform the subsystem/driver when they occur.  Everything
> else (starting and stopping I/O, adjusting the runtime-PM usage
> counter) would have to be done by the subsystem/driver.
> 
> The problem is that a core generally isn't aware of when a file 
> reference is opened or released.  Only the driver is.  Which means 
> there's nothing that the core can do here; the driver and subsystem 
> have to manage pretty much the whole thing.  A simple example:
> 
> open()
> {
> 	mutex_lock(&dev->open_mutex);
> 	if (dev->open_count++ == 0 && !dev->inhibited) {
> 		pm_runtime_get_sync(dev);
> 		start_io(dev);
> 	}
> 	mutex_unlock(&dev->open_mutex);
> }
> 
> release()
> {
> 	mutex_lock(&dev->open_mutex);
> 	if (--dev->open_count == 0 && !dev->inhibited) {
> 		stop_io(dev);
> 		pm_runtime_put(dev);
> 	}
> 	mutex_unlock(&dev->open_mutex);
> }
> 
> inhibit()
> {
> 	mutex_lock(&dev->open_mutex);
> 	dev->inhibited = true;
> 	if (dev->open_count > 0) {
> 		stop_io(dev);
> 		pm_runtime_put(dev);
> 	}
> 	mutex_unlock(&dev->open_mutex);
> }
> 
> uninhibit()
> {
> 	mutex_lock(&dev->open_mutex);
> 	dev->inhibited = false;
> 	if (dev->open_count > 0) {
> 		pm_runtime_get_sync(dev);
> 		start_io(dev);
> 	}
> 	mutex_unlock(&dev->open_mutex);
> }
> 
> This doesn't leave much room for the PM core or anything else.

We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
bit in struct dev_pm_info or something similar).

That is actually quite important, because (a) we can save energy but not
configuring the device to do remote wakeup in the first place and (b) that
may involve more than just the driver (for example, disabling PCI or ACPI
remote wakeup involves the bus type or similar).

So it looks like we need to be able to distinguish between "runtime suspend
with remote wakeup" and "runtime suspend without remote wakeup".

And if we do the latter, we may not even need the "inhibit" thing any more,
because suspended devices without that are not configured to do remote wakeup
cannot really signal anything in the majority of cases.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1232850

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-25 16:40 +0200
Message-ID<qcCki-4ao-13@gated-at.bofh.it>
In reply to#1232502
On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:

> We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
> but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
> bit in struct dev_pm_info or something similar).
> 
> That is actually quite important, because (a) we can save energy but not
> configuring the device to do remote wakeup in the first place and (b) that
> may involve more than just the driver (for example, disabling PCI or ACPI
> remote wakeup involves the bus type or similar).
> 
> So it looks like we need to be able to distinguish between "runtime suspend
> with remote wakeup" and "runtime suspend without remote wakeup".
> 
> And if we do the latter, we may not even need the "inhibit" thing any more,
> because suspended devices without that are not configured to do remote wakeup
> cannot really signal anything in the majority of cases.

That works only for drivers that use autosuspend to go to low power in
between events.  It doesn't work for drivers that remain at full power 
as long as the device file is open.  That kind of driver does require 
an "inhibit" interface.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233043 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-25 21:50 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qcHah-2Fx-1@gated-at.bofh.it>
In reply to#1232850
On Friday, September 25, 2015 10:29:55 AM Alan Stern wrote:
> On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> 
> > We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
> > but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
> > bit in struct dev_pm_info or something similar).
> > 
> > That is actually quite important, because (a) we can save energy but not
> > configuring the device to do remote wakeup in the first place and (b) that
> > may involve more than just the driver (for example, disabling PCI or ACPI
> > remote wakeup involves the bus type or similar).
> > 
> > So it looks like we need to be able to distinguish between "runtime suspend
> > with remote wakeup" and "runtime suspend without remote wakeup".
> > 
> > And if we do the latter, we may not even need the "inhibit" thing any more,
> > because suspended devices without that are not configured to do remote wakeup
> > cannot really signal anything in the majority of cases.
> 
> That works only for drivers that use autosuspend to go to low power in
> between events.  It doesn't work for drivers that remain at full power 
> as long as the device file is open.  That kind of driver does require 
> an "inhibit" interface.

Or an interface allowing user space to trigger pm_request_idle() for them.

So user space would change the "no remote wakeup" setting and then do the
"try to suspend now" thing.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233073

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-25 23:20 +0200
Message-ID<qcIzo-4Ol-17@gated-at.bofh.it>
In reply to#1233043
On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:

> On Friday, September 25, 2015 10:29:55 AM Alan Stern wrote:
> > On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> > 
> > > We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
> > > but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
> > > bit in struct dev_pm_info or something similar).
> > > 
> > > That is actually quite important, because (a) we can save energy but not
> > > configuring the device to do remote wakeup in the first place and (b) that
> > > may involve more than just the driver (for example, disabling PCI or ACPI
> > > remote wakeup involves the bus type or similar).
> > > 
> > > So it looks like we need to be able to distinguish between "runtime suspend
> > > with remote wakeup" and "runtime suspend without remote wakeup".
> > > 
> > > And if we do the latter, we may not even need the "inhibit" thing any more,
> > > because suspended devices without that are not configured to do remote wakeup
> > > cannot really signal anything in the majority of cases.
> > 
> > That works only for drivers that use autosuspend to go to low power in
> > between events.  It doesn't work for drivers that remain at full power 
> > as long as the device file is open.  That kind of driver does require 
> > an "inhibit" interface.
> 
> Or an interface allowing user space to trigger pm_request_idle() for them.
> 
> So user space would change the "no remote wakeup" setting and then do the
> "try to suspend now" thing.

So something like:

	echo on >/sys/.../power/control  (in case the device was
			already in runtime suspend with wakeups enabled)
	echo off >/sys/.../power/wakeup
	echo auto >/sys/.../power/control

This should work.  But it would require that the driver doesn't
increment the usage counter when the device file is opened.  I can
imagine this might lead to trouble if you're dealing with hardware that
doesn't support remote wakeup very well.  The driver wouldn't be able
to work around the hardware issue by incrementing the usage counter.

In real life this might not be a serious issue.  I don't know.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233075 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-25 23:30 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qcIJ4-50M-19@gated-at.bofh.it>
In reply to#1233073
On Friday, September 25, 2015 05:13:04 PM Alan Stern wrote:
> On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> 
> > On Friday, September 25, 2015 10:29:55 AM Alan Stern wrote:
> > > On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> > > 
> > > > We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
> > > > but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
> > > > bit in struct dev_pm_info or something similar).
> > > > 
> > > > That is actually quite important, because (a) we can save energy but not
> > > > configuring the device to do remote wakeup in the first place and (b) that
> > > > may involve more than just the driver (for example, disabling PCI or ACPI
> > > > remote wakeup involves the bus type or similar).
> > > > 
> > > > So it looks like we need to be able to distinguish between "runtime suspend
> > > > with remote wakeup" and "runtime suspend without remote wakeup".
> > > > 
> > > > And if we do the latter, we may not even need the "inhibit" thing any more,
> > > > because suspended devices without that are not configured to do remote wakeup
> > > > cannot really signal anything in the majority of cases.
> > > 
> > > That works only for drivers that use autosuspend to go to low power in
> > > between events.  It doesn't work for drivers that remain at full power 
> > > as long as the device file is open.  That kind of driver does require 
> > > an "inhibit" interface.
> > 
> > Or an interface allowing user space to trigger pm_request_idle() for them.
> > 
> > So user space would change the "no remote wakeup" setting and then do the
> > "try to suspend now" thing.
> 
> So something like:
> 
> 	echo on >/sys/.../power/control  (in case the device was
> 			already in runtime suspend with wakeups enabled)
> 	echo off >/sys/.../power/wakeup
> 	echo auto >/sys/.../power/control

That, or there may be an additional value, say "aggressive", to write to the
control file in which case it becomes just

echo aggressive >/sys/.../power/control

> 
> This should work.  But it would require that the driver doesn't
> increment the usage counter when the device file is opened.

Right.

> I can imagine this might lead to trouble if you're dealing with hardware that
> doesn't support remote wakeup very well.  The driver wouldn't be able
> to work around the hardware issue by incrementing the usage counter.

Or the "aggressive" mode wouldn't work for it.

> In real life this might not be a serious issue.  I don't know.

Me neither.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233098 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-26 00:40 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qcJOO-6x2-9@gated-at.bofh.it>
In reply to#1233075
On Friday, September 25, 2015 11:52:23 PM Rafael J. Wysocki wrote:
> On Friday, September 25, 2015 05:13:04 PM Alan Stern wrote:
> > On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> > 
> > > On Friday, September 25, 2015 10:29:55 AM Alan Stern wrote:
> > > > On Fri, 25 Sep 2015, Rafael J. Wysocki wrote:
> > > > 
> > > > > We are missing the "no remote wakeup" bit now (well, there is a PM QoS flag,
> > > > > but it isn't very useful, so I'd prefer to replace it with a "no remote wakeup"
> > > > > bit in struct dev_pm_info or something similar).
> > > > > 
> > > > > That is actually quite important, because (a) we can save energy but not
> > > > > configuring the device to do remote wakeup in the first place and (b) that
> > > > > may involve more than just the driver (for example, disabling PCI or ACPI
> > > > > remote wakeup involves the bus type or similar).
> > > > > 
> > > > > So it looks like we need to be able to distinguish between "runtime suspend
> > > > > with remote wakeup" and "runtime suspend without remote wakeup".
> > > > > 
> > > > > And if we do the latter, we may not even need the "inhibit" thing any more,
> > > > > because suspended devices without that are not configured to do remote wakeup
> > > > > cannot really signal anything in the majority of cases.
> > > > 
> > > > That works only for drivers that use autosuspend to go to low power in
> > > > between events.  It doesn't work for drivers that remain at full power 
> > > > as long as the device file is open.  That kind of driver does require 
> > > > an "inhibit" interface.
> > > 
> > > Or an interface allowing user space to trigger pm_request_idle() for them.
> > > 
> > > So user space would change the "no remote wakeup" setting and then do the
> > > "try to suspend now" thing.
> > 
> > So something like:
> > 
> > 	echo on >/sys/.../power/control  (in case the device was
> > 			already in runtime suspend with wakeups enabled)
> > 	echo off >/sys/.../power/wakeup
> > 	echo auto >/sys/.../power/control
> 
> That, or there may be an additional value, say "aggressive", to write to the
> control file in which case it becomes just
> 
> echo aggressive >/sys/.../power/control

That said I suppose that the "off" value for the "wakeup" file might also be
useful in some other cases, so it likely is a better approach.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233215

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-26 17:30 +0200
Message-ID<qcZAd-3T8-11@gated-at.bofh.it>
In reply to#1233098
On Sat, 26 Sep 2015, Rafael J. Wysocki wrote:

> > > So something like:
> > > 
> > > 	echo on >/sys/.../power/control  (in case the device was
> > > 			already in runtime suspend with wakeups enabled)
> > > 	echo off >/sys/.../power/wakeup
> > > 	echo auto >/sys/.../power/control

Cases where the driver wants to avoid runtime suspend (while the device
is active) because of bad wakeup support in the hardware can be handled
easily enough.  The runtime-idle or runtime-suspend callback routine
can check whether wakeup == off; if it isn't then the callback should
return -EBUSY.  Thus the driver can prevent runtime suspend without any
need to increment the usage counter.

> > That, or there may be an additional value, say "aggressive", to write to the
> > control file in which case it becomes just
> > 
> > echo aggressive >/sys/.../power/control
> 
> That said I suppose that the "off" value for the "wakeup" file might also be
> useful in some other cases, so it likely is a better approach.

We still need some sort of "inhibit" callback for cases where the
driver doesn't want to go into runtime suspend but does want to turn
off all I/O.  Should this callback be triggered when the user writes
"off" to power/wakeup, or when the user writes "inhibit" to
power/control, or should there be a separate sysfs attribute?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233660 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-27 15:20 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qdk1Y-8ek-15@gated-at.bofh.it>
In reply to#1233215
On Saturday, September 26, 2015 11:20:50 AM Alan Stern wrote:
> On Sat, 26 Sep 2015, Rafael J. Wysocki wrote:
> 
> > > > So something like:
> > > > 
> > > > 	echo on >/sys/.../power/control  (in case the device was
> > > > 			already in runtime suspend with wakeups enabled)
> > > > 	echo off >/sys/.../power/wakeup
> > > > 	echo auto >/sys/.../power/control
> 
> Cases where the driver wants to avoid runtime suspend (while the device
> is active) because of bad wakeup support in the hardware can be handled
> easily enough.  The runtime-idle or runtime-suspend callback routine
> can check whether wakeup == off; if it isn't then the callback should
> return -EBUSY.  Thus the driver can prevent runtime suspend without any
> need to increment the usage counter.

Right.

> > > That, or there may be an additional value, say "aggressive", to write to the
> > > control file in which case it becomes just
> > > 
> > > echo aggressive >/sys/.../power/control
> > 
> > That said I suppose that the "off" value for the "wakeup" file might also be
> > useful in some other cases, so it likely is a better approach.
> 
> We still need some sort of "inhibit" callback for cases where the
> driver doesn't want to go into runtime suspend but does want to turn
> off all I/O.  Should this callback be triggered when the user writes
> "off" to power/wakeup, or when the user writes "inhibit" to
> power/control, or should there be a separate sysfs attribute?

My first thought is that if there is a separate attribute, then it only actually
makes sense for devices that generate input events, while the "off" thing may
be generally useful in principle (eg. it may indicate to disable PME for the
device to the PCI layer etc).

OTOH, the additional "inhibit" attribute may only be exposed if the corresponding
callback is present, so I'm not really sure.

Question is, though, what's the use case for turning off I/O when we don't
go into runtime suspend.  After all, runtime suspend need not mean putting
the device into any kind of low-power state and the "off" thing may very
well be defined to mean that all input is discarded if the device is
runtime-suspended and the device is not configured to do remote wakeup
then.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233677

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-27 16:30 +0200
Message-ID<qdl7H-1k0-7@gated-at.bofh.it>
In reply to#1233660
On Sun, 27 Sep 2015, Rafael J. Wysocki wrote:

> On Saturday, September 26, 2015 11:20:50 AM Alan Stern wrote:
> > On Sat, 26 Sep 2015, Rafael J. Wysocki wrote:
> > 
> > > > > So something like:
> > > > > 
> > > > > 	echo on >/sys/.../power/control  (in case the device was
> > > > > 			already in runtime suspend with wakeups enabled)
> > > > > 	echo off >/sys/.../power/wakeup
> > > > > 	echo auto >/sys/.../power/control

> > We still need some sort of "inhibit" callback for cases where the
> > driver doesn't want to go into runtime suspend but does want to turn
> > off all I/O.  Should this callback be triggered when the user writes
> > "off" to power/wakeup, or when the user writes "inhibit" to
> > power/control, or should there be a separate sysfs attribute?
> 
> My first thought is that if there is a separate attribute, then it only actually
> makes sense for devices that generate input events, while the "off" thing may
> be generally useful in principle (eg. it may indicate to disable PME for the
> device to the PCI layer etc).

I'm not sure how much sense that distinction makes.  It seems to me the
only time you want to ignore potential wakeup events is if you want to
ignore _all_ input.  Which is basically what "inhibit" means.

This suggests we forget about power/wakeup == "off" and introduce an 
"inhibit" attribute instead.

> OTOH, the additional "inhibit" attribute may only be exposed if the corresponding
> callback is present, so I'm not really sure.

It could be a separate attribute, or it could be a new entry for
power/control.  Come to think of it, a separate attribute might be
better.  Otherwise we would lose track of whether runtime suspend was
permitted (the "on" vs. "auto" distinction) when the device was
inhibited.  I can imagine someone might want to forbid runtime suspend
but still inhibit a device.

However, I agree that there's no point registering a separate attribute
or accepting a write of "inhibit" to power/control if there's no
corresponding callback.

> Question is, though, what's the use case for turning off I/O when we don't
> go into runtime suspend.  After all, runtime suspend need not mean putting
> the device into any kind of low-power state and the "off" thing may very
> well be defined to mean that all input is discarded if the device is
> runtime-suspended and the device is not configured to do remote wakeup
> then.

Well, I suppose there might be a driver that supports inhibit but
doesn't support runtime PM, unlikely as that seems.  Or the driver
might support both but the user might leave power/control == "on" while
inhibiting the device.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1234140 — Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-09-28 15:20 +0200
SubjectRe: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Message-ID<qdGvv-6My-3@gated-at.bofh.it>
In reply to#1233677
On Sunday, September 27, 2015 10:27:25 AM Alan Stern wrote:
> On Sun, 27 Sep 2015, Rafael J. Wysocki wrote:
> 
> > On Saturday, September 26, 2015 11:20:50 AM Alan Stern wrote:
> > > On Sat, 26 Sep 2015, Rafael J. Wysocki wrote:
> > > 
> > > > > > So something like:
> > > > > > 
> > > > > > 	echo on >/sys/.../power/control  (in case the device was
> > > > > > 			already in runtime suspend with wakeups enabled)
> > > > > > 	echo off >/sys/.../power/wakeup
> > > > > > 	echo auto >/sys/.../power/control
> 
> > > We still need some sort of "inhibit" callback for cases where the
> > > driver doesn't want to go into runtime suspend but does want to turn
> > > off all I/O.  Should this callback be triggered when the user writes
> > > "off" to power/wakeup, or when the user writes "inhibit" to
> > > power/control, or should there be a separate sysfs attribute?
> > 
> > My first thought is that if there is a separate attribute, then it only actually
> > makes sense for devices that generate input events, while the "off" thing may
> > be generally useful in principle (eg. it may indicate to disable PME for the
> > device to the PCI layer etc).
> 
> I'm not sure how much sense that distinction makes.  It seems to me the
> only time you want to ignore potential wakeup events is if you want to
> ignore _all_ input.  Which is basically what "inhibit" means.

The other case I had in mind is specific to the PCI layer and might be better
served by adding an "ignore PME" flag to PCI devices.

> This suggests we forget about power/wakeup == "off" and introduce an 
> "inhibit" attribute instead.

If we do that, can it still be regarded as a PM attribute?

And what about the corresponding callback?  Should that be a PM callback or
a general one?

> > OTOH, the additional "inhibit" attribute may only be exposed if the corresponding
> > callback is present, so I'm not really sure.
> 
> It could be a separate attribute, or it could be a new entry for
> power/control.  Come to think of it, a separate attribute might be
> better.  Otherwise we would lose track of whether runtime suspend was
> permitted (the "on" vs. "auto" distinction) when the device was
> inhibited.  I can imagine someone might want to forbid runtime suspend
> but still inhibit a device.
> 
> However, I agree that there's no point registering a separate attribute
> or accepting a write of "inhibit" to power/control if there's no
> corresponding callback.
> 
> > Question is, though, what's the use case for turning off I/O when we don't
> > go into runtime suspend.  After all, runtime suspend need not mean putting
> > the device into any kind of low-power state and the "off" thing may very
> > well be defined to mean that all input is discarded if the device is
> > runtime-suspended and the device is not configured to do remote wakeup
> > then.
> 
> Well, I suppose there might be a driver that supports inhibit but
> doesn't support runtime PM, unlikely as that seems.  Or the driver
> might support both but the user might leave power/control == "on" while
> inhibiting the device.

That sounds like a general rather than PM-related mechanism then.

I guess we need a real use case for that last thing or it will be rather
difficult to convince Greg to accept the patch. :-)

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1234209

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-28 16:30 +0200
Message-ID<qdHBh-21o-37@gated-at.bofh.it>
In reply to#1234140
On Mon, 28 Sep 2015, Rafael J. Wysocki wrote:

> > This suggests we forget about power/wakeup == "off" and introduce an 
> > "inhibit" attribute instead.
> 
> If we do that, can it still be regarded as a PM attribute?

Why not?  Consider this: Is there any reason to support inhibit when
CONFIG_PM is disabled?  I can't come up with any.

> And what about the corresponding callback?  Should that be a PM callback or
> a general one?

Well, if "inhibit" is a PM attribute then the callback should be a PM 
callback.  :-)

> > > Question is, though, what's the use case for turning off I/O when we don't
> > > go into runtime suspend.  After all, runtime suspend need not mean putting
> > > the device into any kind of low-power state and the "off" thing may very
> > > well be defined to mean that all input is discarded if the device is
> > > runtime-suspended and the device is not configured to do remote wakeup
> > > then.
> > 
> > Well, I suppose there might be a driver that supports inhibit but
> > doesn't support runtime PM, unlikely as that seems.  Or the driver
> > might support both but the user might leave power/control == "on" while
> > inhibiting the device.
> 
> That sounds like a general rather than PM-related mechanism then.

I don't follow your reasoning.

> I guess we need a real use case for that last thing or it will be rather
> difficult to convince Greg to accept the patch. :-)

The hard part is to come up with a design that Greg agrees with.  If 
the design is okay, there's no reason not to accept the patch.

One of the questions amounts to this: Do we want to allow situations
where input is inhibited but the user prevents the device from going
into runtime suspend by setting power/control = "on"?  If the answer is
Yes then "inhibit" should be a separate attribute.  Otherwise, we can 
just let "inhibit" be another setting in power/control.

Another question is: Do we want to make it easy for drivers to support
inhibit while still incrementing their PM usage counter every time the
device file is opened?  If we do then inhibit must be considered
separate from runtime suspend, because a device _can't_ go directly
into runtime suspend when the usage counter is > 1.  If we don't then 
we will most likely have to change the runtime-PM support in some 
drivers before they can implement inhibit.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1234412

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2015-09-28 22:10 +0200
Message-ID<qdMUj-1fb-35@gated-at.bofh.it>
In reply to#1234209
Hi Alan,

On Mon, Sep 28, 2015 at 4:29 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 28 Sep 2015, Rafael J. Wysocki wrote:
>
>> > This suggests we forget about power/wakeup == "off" and introduce an
>> > "inhibit" attribute instead.
>>
>> If we do that, can it still be regarded as a PM attribute?
>
> Why not?  Consider this: Is there any reason to support inhibit when
> CONFIG_PM is disabled?  I can't come up with any.

Well, the "I don't want any input from you now, because the phone is
going into a pocket" case?

It isn't stticlty dependent on PM.

>> And what about the corresponding callback?  Should that be a PM callback or
>> a general one?
>
> Well, if "inhibit" is a PM attribute then the callback should be a PM
> callback.  :-)
>
>> > > Question is, though, what's the use case for turning off I/O when we don't
>> > > go into runtime suspend.  After all, runtime suspend need not mean putting
>> > > the device into any kind of low-power state and the "off" thing may very
>> > > well be defined to mean that all input is discarded if the device is
>> > > runtime-suspended and the device is not configured to do remote wakeup
>> > > then.
>> >
>> > Well, I suppose there might be a driver that supports inhibit but
>> > doesn't support runtime PM, unlikely as that seems.  Or the driver
>> > might support both but the user might leave power/control == "on" while
>> > inhibiting the device.
>>
>> That sounds like a general rather than PM-related mechanism then.
>
> I don't follow your reasoning.

Support for "inhibit" and lack of runtime PM support means that the
feature has nothing to do with PM any more AFAICS.

That's why I think it may be regarded by more than just PM.  It should
make runtime PM behave in a specific way if supported, but then it
should work withot it too, shouldn't it?

>> I guess we need a real use case for that last thing or it will be rather
>> difficult to convince Greg to accept the patch. :-)
>
> The hard part is to come up with a design that Greg agrees with.  If
> the design is okay, there's no reason not to accept the patch.
>
> One of the questions amounts to this: Do we want to allow situations
> where input is inhibited but the user prevents the device from going
> into runtime suspend by setting power/control = "on"?  If the answer is
> Yes then "inhibit" should be a separate attribute.  Otherwise, we can
> just let "inhibit" be another setting in power/control.

I think that the answer really is "yes" in general as long as it makes
sense to discard input without closing the device.

My understanding of "inhibit" would be "discard all input including
any wakeup events from now on".  It would be quite natural for runtime
suspend to trigger if that is set (unless disabled or not supported,
of course), but I don't think that this should be a requirement.

> Another question is: Do we want to make it easy for drivers to support
> inhibit while still incrementing their PM usage counter every time the
> device file is opened?  If we do then inhibit must be considered
> separate from runtime suspend, because a device _can't_ go directly
> into runtime suspend when the usage counter is > 1.  If we don't then
> we will most likely have to change the runtime-PM support in some
> drivers before they can implement inhibit.

My opinion is that "inhibit" should affect PM, but should not require
PM to function (there's no technical reason for that).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1234425

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-09-28 22:30 +0200
Message-ID<qdNdE-1FC-17@gated-at.bofh.it>
In reply to#1234412
On Mon, 28 Sep 2015, Rafael J. Wysocki wrote:

> Hi Alan,
> 
> On Mon, Sep 28, 2015 at 4:29 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Mon, 28 Sep 2015, Rafael J. Wysocki wrote:
> >
> >> > This suggests we forget about power/wakeup == "off" and introduce an
> >> > "inhibit" attribute instead.
> >>
> >> If we do that, can it still be regarded as a PM attribute?
> >
> > Why not?  Consider this: Is there any reason to support inhibit when
> > CONFIG_PM is disabled?  I can't come up with any.
> 
> Well, the "I don't want any input from you now, because the phone is
> going into a pocket" case?

But who would make a phone without CONFIG_PM?  If you're sufficiently 
unconcerned about power usage that you turn off CONFIG_PM, then you 
probably don't care about getting excess input events either.

> It isn't stticlty dependent on PM.

No, not strictly.  But it is closely enough related that people
shouldn't mind if it becomes part of the PM code.

> >> > Well, I suppose there might be a driver that supports inhibit but
> >> > doesn't support runtime PM, unlikely as that seems.  Or the driver
> >> > might support both but the user might leave power/control == "on" while
> >> > inhibiting the device.
> >>
> >> That sounds like a general rather than PM-related mechanism then.
> >
> > I don't follow your reasoning.
> 
> Support for "inhibit" and lack of runtime PM support means that the
> feature has nothing to do with PM any more AFAICS.

My example above referred to support in a single driver, not support in 
the system as a whole.  By the same reasoning, since some drivers 
support system sleep but not runtime PM, system sleep must have nothing 
to do with PM.  :-)

> That's why I think it may be regarded by more than just PM.  It should
> make runtime PM behave in a specific way if supported, but then it
> should work withot it too, shouldn't it?

If you want inhibit to be part of the device core rather than the PM
core, that's okay with me.

> My opinion is that "inhibit" should affect PM, but should not require
> PM to function (there's no technical reason for that).

All right.  Then a design should be straightforward.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web