[RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Add new option to sysfs control interface, allowing the user to force suspend the device. This is useful for devices that need to be suspended when closing the lid of a laptop or the screen of a mobile device, while userspace still holds open handles to it and the system does not enter system suspend. Add the "off" option to the sysfs control power interface, along with the already present "on" and "auto". When this attribute is set to "off", the device will be force suspended by calling its runtime suspend callback and disabling runtime power management so that further acceses to the device will not change the actual state. The device can be resumed by setting the attribute to "on" or "auto". The behaviour of the interface when switching only between "on" and "auto" states remains unchanged. Signed-off-by: Irina Tirdea --- Hi, This is a proposal for suspending devices when the closing the lid of a laptop or the screen of a mobile device. I am testing this with a Goodix touchscreen [1] for an Android mobile device. Android has an userspace layer (power HAL) that would normally close the touchscreen when the screen is closed. Android touchscreen drivers usually provide a custom sysfs interface to allow this. This would be better implemented in a common place, to avoid code duplication and to simplify the driver code (as previosly discussed in [1]). I know there are more ways to implement this, so I would appreciate your feedback. Thank you, Irina [1] https://lkml.org/lkml/2015/9/7/329 [2] https://lkml.org/lkml/2014/7/15/928 drivers/base/power/main.c| 8 +++ drivers/base/power/runtime.c | 141 --- drivers/base/power/sysfs.c | 22 --- drivers/usb/core/sysfs.c | 3 +- include/linux/pm.h | 8 ++- include/linux/pm_runtime.h | 18 +- include/trace/events/rpm.h | 6 +- 7 files changed, 169 insertions(+), 37 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 9717d5f..e497c38 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -927,6 +927,9 @@ static void device_complete(struct device *dev, pm_message_t state) device_unlock(dev); + if (dev->power.runtime_mode == RPM_MODE_OFF) + pm_runtime_force_suspend(dev); + pm_runtime_put(dev); } @@ -1596,6 +1599,11 @@ static int device_prepare(struct device *dev, pm_message_t state) spin_lock_irq(&dev->power.lock); dev->power.direct_complete = ret > 0 && state.event == PM_EVENT_SUSPEND; spin_unlock_irq(&dev->power.lock); + + if (!dev->power.direct_complete && + dev->power.runtime_mode == RPM_MODE_OFF) + pm_runtime_enable(dev); + return 0; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 5070c4f..0d94b8c 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1189,49 +1189,144 @@ void pm_runtime_enable(struct device *dev) } EXPORT_SYMBOL_GPL(pm_runtime_enable); +static int dev_pm_mode_on(struct device *dev, void *data) +{ + return dev->power.runtime_mode != RPM_MODE_OFF; +} + /** - * pm_runtime_forbid - Block runtime PM of a device. + * pm_runtime_force_off - Block runtime PM of a device and keep it suspended. * @dev: Device to handle. * - * Increase the device's usage count and clear its power.runtime_auto flag, - * so that it cannot be suspended at run time until pm_runtime_allow() is called - * for it. + * Change the device's power.runtime_mode field to RPM_MODE_OFF and + * force runtime suspend (by running the device's runtime suspend + * and disabling runtime power management), so that it cannot be + * resumed at run time until pm_runtime_force_on() or + * pm_runtime_force_auto() is called for it. */ -void pm_runtime_forbid(struct device *dev) +int pm_runtime_force_off(struct device *dev) { + int mode, ret; + spin_lock_irq(&dev->power.lock); - if (!dev->power.runtime_auto) - goto out; + mode = dev->power.runtime_mode; + if (mode == RPM_MODE_OFF) { + spin_unlock_irq(&dev->power.lock); + return 0; + } - dev->power.runtime_auto = false; - atomic_inc(&dev->power.usage_count); - rpm_resume(dev, 0); + /* Cannot force suspend if runtime pm is disabled */ + if (!pm_runtime_enabled(dev)) { + spin_unlock_irq(&dev->power.lock); + return -EACCES; + } - out: + /* +* Cannot force suspend if children are on/auto and +* device ignore_children flag is not set. +*/ + if (!dev->power.ignore_children && + device_for_each_child(dev, NULL, dev_pm_mode_on)) { + spin_unlock_irq(&dev->power.lock); + return -EBUSY; + } + + dev->power.runtime_mode = RPM_MODE_OFF; + if (mode == RPM_MODE_ON) + atomic_dec(&dev->power.usage_c
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Monday, September 07, 2015 11:42:41 PM Irina Tirdea wrote: > Add new option to sysfs control interface, allowing the user to force > suspend the device. Had we thought this had been a good idea, we'd have added that thing to the interface from the start. The problem with it is that user space generally doesn't know when it is safe to suspend a device, so it cannot force anything into runtime suspend. > This is useful for devices that need to be > suspended when closing the lid of a laptop or the screen of a mobile > device, while userspace still holds open handles to it and the > system does not enter system suspend. > > Add the "off" option to the sysfs control power interface, along with > the already present "on" and "auto". When this attribute is set to > "off", the device will be force suspended by calling its runtime > suspend callback and disabling runtime power management so that > further acceses to the device will not change the actual state. And how is user space supposed to know that it doesn't break things this way? > The device can be resumed by setting the attribute to "on" or "auto". > The behaviour of the interface when switching only between "on" > and "auto" states remains unchanged. > > Signed-off-by: Irina Tirdea > --- > > Hi, > > This is a proposal for suspending devices when the closing the lid of > a laptop or the screen of a mobile device. > > I am testing this with a Goodix touchscreen [1] for an Android mobile > device. Android has an userspace layer (power HAL) that would normally > close the touchscreen when the screen is closed. Android touchscreen > drivers usually provide a custom sysfs interface to allow this. > This would be better implemented in a common place, to avoid code > duplication and to simplify the driver code (as previosly discussed > in [1]). > > I know there are more ways to implement this, so I would appreciate > your feedback. So the feedback is that this is not going to work in general. Please use a different approach. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
> -Original Message- > From: linux-input-ow...@vger.kernel.org > [mailto:linux-input-ow...@vger.kernel.org] On Behalf Of Rafael J. Wysocki > Sent: 08 September, 2015 0:20 > To: Tirdea, Irina > Cc: Alan Stern; linux...@vger.kernel.org; linux-in...@vger.kernel.org; > linux-kernel@vger.kernel.org; Brown, Len; Pavel Machek; > Purdila, Octavian; Dmitry Torokhov > Subject: Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing > runtime suspend > > On Monday, September 07, 2015 11:42:41 PM Irina Tirdea wrote: > > Add new option to sysfs control interface, allowing the user to force > > suspend the device. > > Had we thought this had been a good idea, we'd have added that thing to > the interface from the start. > > The problem with it is that user space generally doesn't know when it is > safe to suspend a device, so it cannot force anything into runtime suspend. > Yes, this is generally true. However, in the scenario I mentioned this is exactly what is happening. When turning off the screen of a mobile device, the user space is the one that suspends the devices that are not needed in order to save power (like touchscreens). Each such driver export an enable/disable attribute that calls the same code as resume/suspend (e.g. touchscreen drivers ads7846, ad7877 and most Android drivers not merged upstream). This adds more complexity to every driver by adding one more logical power state. It would be good to have a common interface instead of doing this in every driver. I might have not used "forced" in the proper way here. What I mean by it is that the device can be runtime suspended while ignoring the runtime usage count. > > This is useful for devices that need to be > > suspended when closing the lid of a laptop or the screen of a mobile > > device, while user space still holds open handles to it and the > > system does not enter system suspend. > > > > Add the "off" option to the sysfs control power interface, along with > > the already present "on" and "auto". When this attribute is set to > > "off", the device will be force suspended by calling its runtime > > suspend callback and disabling runtime power management so that > > further accesses to the device will not change the actual state. > > And how is user space supposed to know that it doesn't break things > this way? > In this implementation, user space is only allowed to change the states bottom-up in the sysfs hierarchy (it cannot force suspend a device if it has children that have not been suspended by user space). Another way to do this is to keep runtime power management enabled but to force the usage count to 0 while the device is set to runtime mode "off". > > The device can be resumed by setting the attribute to "on" or "auto". > > The behavior of the interface when switching only between "on" > > and "auto" states remains unchanged. > > > > Signed-off-by: Irina Tirdea > > --- > > > > Hi, > > > > This is a proposal for suspending devices when the closing the lid of > > a laptop or the screen of a mobile device. > > > > I am testing this with a Goodix touchscreen [1] for an Android mobile > > device. Android has an user space layer (power HAL) that would normally > > close the touchscreen when the screen is closed. Android touchscreen > > drivers usually provide a custom sysfs interface to allow this. > > This would be better implemented in a common place, to avoid code > > duplication and to simplify the driver code (as previously discussed > > in [1]). > > > > I know there are more ways to implement this, so I would appreciate > > your feedback. > > So the feedback is that this is not going to work in general. Please > use a different approach. > Would it work if this would be a capability that individual drivers need to declare? In the previous discussion thread , there were a couple of options mentioned, but none seemed to reach a consensus. You mentioned adding a "more aggressive runtime PM mode" [1]. I'm not sure how this would work except for adding a sysfs attribute that would trigger a runtime suspend while ignoring usage count. Would that be a better direction? Thank you, Irina [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: > However, in the scenario I mentioned this is exactly what is happening. > When turning off the screen of a mobile device, the user space Would you explain why user space doesn't simply stop using those devices, which in turn will make them idle? There are obvious cases like keyboards or SCSI hosts where the kernel controls stuff, but could you state where you expect this to be most useful? Or why devices cannot be closed, e.g. you need to be sure settings be not lost or is it something else? > is the one that suspends the devices that are not needed in order to save > power > (like touchscreens). Each such driver export an enable/disable attribute > that calls the same code as resume/suspend (e.g. touchscreen drivers ads7846, > ad7877 and most Android drivers not merged upstream). This adds more > complexity to every driver by adding one more logical power state. > It would be good to have a common interface instead of doing this in > every Now these are two distinct questions. 1. a common interface 2. a capability implemented in common code It is important to keep that apart. I suppose if we want this at all #1 is a given. #2 however may be impossible in a generic manner > I might have not used "forced" in the proper way here. What I mean by it is > that > the device can be runtime suspended while ignoring the runtime usage count. That is highly problematic. You'd need to audit the locking in every driver. Right now elevating the count means that suspend()/resume() cannot race with user space, as in the case of the system suspending user space is frozen. > In this implementation, user space is only allowed to change the states > bottom-up in the sysfs hierarchy (it cannot force suspend a device if it > has children that have not been suspended by user space). That is obviously not enough. Take the worst case: we are flashing some firmware. Or far more harmless: a key is has been pressed on a keyboard > Would it work if this would be a capability that individual drivers need > to declare? For some drivers. But it needs support in the driver. Right now we can make a device idle by calling close(). In fact we can benefit for example in mice from this. But it needs support in the drivers. > In the previous discussion thread , there were a couple of options > mentioned, but none seemed to reach a consensus. You mentioned > adding a "more aggressive runtime PM mode" [1]. I'm not sure how That would have to be done on a per driver base. > this would work except for adding a sysfs attribute that would trigger > a runtime suspend while ignoring usage count. Would that be a > better direction? No. If we want this at all, we need a new callback to notify drivers that user space is temporarily uninterested in a device. And the reverse of course. The power model is good. We must not assume that devices can be suspended at will. If we do this at all, we ought to see it as giving strong hints to drivers when a device can be considered idle. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 8 Sep 2015, Tirdea, Irina wrote: > In the previous discussion thread , there were a couple of options > mentioned, but none seemed to reach a consensus. You mentioned > adding a "more aggressive runtime PM mode" [1]. I'm not sure how > this would work except for adding a sysfs attribute that would trigger > a runtime suspend while ignoring usage count. Would that be a > better direction? > > Thank you, > Irina > > [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 Purely as a matter of interest, in that email Rafael also mentioned that he and I had discussed a way to disable remote wakeup during runtime suspend. Oddly enough, the method we decided upon was to add an "off" option to /sys/.../power/control. :-) It would not put the device into runtime suspend immediately, like you are proposing. Instead it would mean the same as the "auto" mode, except that remote wakeup should be disabled during runtime suspend. We never got around to implementing this, however. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tuesday, September 08, 2015 10:44:04 AM Alan Stern wrote: > On Tue, 8 Sep 2015, Tirdea, Irina wrote: > > > In the previous discussion thread , there were a couple of options > > mentioned, but none seemed to reach a consensus. You mentioned > > adding a "more aggressive runtime PM mode" [1]. I'm not sure how > > this would work except for adding a sysfs attribute that would trigger > > a runtime suspend while ignoring usage count. Would that be a > > better direction? > > > > Thank you, > > Irina > > > > [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 > > Purely as a matter of interest, in that email Rafael also mentioned > that he and I had discussed a way to disable remote wakeup during > runtime suspend. Oddly enough, the method we decided upon was to add > an "off" option to /sys/.../power/control. :-) Wasn't that /sys/devices/.../power/wakeup rather? > It would not put the device into runtime suspend immediately, like you > are proposing. Instead it would mean the same as the "auto" mode, > except that remote wakeup should be disabled during runtime suspend. > > We never got around to implementing this, however. I don't think this is what we discussed then really. There is a fundamental problem with forcing things into runtime suspend from user space, because that may happen in a wrong time. In other words, the kernel can't guarantee that the device would actually be able to go into runtime suspend when requested. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 8 Sep 2015, Rafael J. Wysocki wrote: > > > [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 > > > > Purely as a matter of interest, in that email Rafael also mentioned > > that he and I had discussed a way to disable remote wakeup during > > runtime suspend. Oddly enough, the method we decided upon was to add > > an "off" option to /sys/.../power/control. :-) > > Wasn't that /sys/devices/.../power/wakeup rather? Not the way I remember. Of course, it's possible that we misunderstood each other at the time. > > It would not put the device into runtime suspend immediately, like you > > are proposing. Instead it would mean the same as the "auto" mode, > > except that remote wakeup should be disabled during runtime suspend. > > > > We never got around to implementing this, however. > > I don't think this is what we discussed then really. > > There is a fundamental problem with forcing things into runtime suspend > from user space, because that may happen in a wrong time. In other words, > the kernel can't guarantee that the device would actually be able to go > into runtime suspend when requested. Exactly. What we discussed at LinuxCon wasn't forcing things into runtime suspend; it was disabling remote wakeup during runtime suspend. And even though the topic was quite different from Irina's proposal, we ended up settling on the same API (according to my recollection). Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi, On Tue, Sep 8, 2015 at 5:00 PM, Alan Stern wrote: > On Tue, 8 Sep 2015, Rafael J. Wysocki wrote: > >> > > [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 >> > >> > Purely as a matter of interest, in that email Rafael also mentioned >> > that he and I had discussed a way to disable remote wakeup during >> > runtime suspend. Oddly enough, the method we decided upon was to add >> > an "off" option to /sys/.../power/control. :-) >> >> Wasn't that /sys/devices/.../power/wakeup rather? > > Not the way I remember. Of course, it's possible that we misunderstood > each other at the time. > >> > It would not put the device into runtime suspend immediately, like you >> > are proposing. Instead it would mean the same as the "auto" mode, >> > except that remote wakeup should be disabled during runtime suspend. >> > >> > We never got around to implementing this, however. >> >> I don't think this is what we discussed then really. >> >> There is a fundamental problem with forcing things into runtime suspend >> from user space, because that may happen in a wrong time. In other words, >> the kernel can't guarantee that the device would actually be able to go >> into runtime suspend when requested. > > Exactly. What we discussed at LinuxCon wasn't forcing things into > runtime suspend; it was disabling remote wakeup during runtime suspend. > > And even though the topic was quite different from Irina's proposal, we > ended up settling on the same API (according to my recollection). So I remember that differently. My idea was to add a third value to /sys/devices/.../power/wakeup (in addition to "disabled" and "enabled") so user space can indicate that remote wakeup should not be enabled for runtime suspend for the device (since there's no way to indicate that today). I don't see how /sys/devices/.../power/control might help here to be honest. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, Sep 8, 2015 at 9:35 AM, Oliver Neukum wrote: > On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: [cut] >> this would work except for adding a sysfs attribute that would trigger >> a runtime suspend while ignoring usage count. Would that be a >> better direction? > > No. If we want this at all, we need a new callback to notify drivers > that user space is temporarily uninterested in a device. And the reverse > of course. > The power model is good. We must not assume that devices can be > suspended at will. If we do this at all, we ought to see it as giving > strong hints to drivers when a device can be considered idle. This is a good summary in my view. The only thing we can add, realistically, is an interface for user space to "kick" drivers to check if the devices they handle may be suspended at this point (or to run their ->runtime_idle callbacks IOW). That would be quite similar to autosuspend except that the "kick" will come from user space rather than from a timer function in the kernel. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On 8 September 2015 at 22:56, Rafael J. Wysocki wrote: > On Tue, Sep 8, 2015 at 9:35 AM, Oliver Neukum wrote: >> On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: > > [cut] > >>> this would work except for adding a sysfs attribute that would trigger >>> a runtime suspend while ignoring usage count. Would that be a >>> better direction? >> >> No. If we want this at all, we need a new callback to notify drivers >> that user space is temporarily uninterested in a device. And the reverse >> of course. >> The power model is good. We must not assume that devices can be >> suspended at will. If we do this at all, we ought to see it as giving >> strong hints to drivers when a device can be considered idle. > > This is a good summary in my view. > > The only thing we can add, realistically, is an interface for user > space to "kick" drivers to check if the devices they handle may be > suspended at this point (or to run their ->runtime_idle callbacks > IOW). > > That would be quite similar to autosuspend except that the "kick" will > come from user space rather than from a timer function in the kernel. Apologize for interrupting the discussion! Unless I miss the point, I assumes the above is somewhat already achievable via sysfs when changing the value of the auto-suspend timeout, since it triggers a call to pm_runtime_set_autosuspend_delay()... Also, according to the discussion so far, it seems like we are on agreement that we should really think twice when considering to extend the sysfs interface for runtime PM. >From the change-log/description to $subject patch, I fail to understand *why* the regular runtime PM *autosuspend* feature isn't sufficient. Perhaps Irina can elaborate more on the use case, to help me get a better understanding of the issue!? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi, On Wed, Sep 9, 2015 at 12:25 AM, Ulf Hansson wrote: > On 8 September 2015 at 22:56, Rafael J. Wysocki wrote: >> On Tue, Sep 8, 2015 at 9:35 AM, Oliver Neukum wrote: >>> On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: >> >> [cut] >> this would work except for adding a sysfs attribute that would trigger a runtime suspend while ignoring usage count. Would that be a better direction? >>> >>> No. If we want this at all, we need a new callback to notify drivers >>> that user space is temporarily uninterested in a device. And the reverse >>> of course. >>> The power model is good. We must not assume that devices can be >>> suspended at will. If we do this at all, we ought to see it as giving >>> strong hints to drivers when a device can be considered idle. >> >> This is a good summary in my view. >> >> The only thing we can add, realistically, is an interface for user >> space to "kick" drivers to check if the devices they handle may be >> suspended at this point (or to run their ->runtime_idle callbacks >> IOW). >> >> That would be quite similar to autosuspend except that the "kick" will >> come from user space rather than from a timer function in the kernel. > > Apologize for interrupting the discussion! > > Unless I miss the point, I assumes the above is somewhat already > achievable via sysfs when changing the value of the auto-suspend > timeout, since it triggers a call to > pm_runtime_set_autosuspend_delay()... Well, from the initial comment in drivers/base/power/sysfs.c: * *NOTE: The autosuspend_delay_ms attribute and the autosuspend_delay *value are used only if the driver calls pm_runtime_use_autosuspend(). * Some drivers don't do that and they would be the primary target audience for the new interface (if we agreed that it was useful after all). > Also, according to the discussion so far, it seems like we are on > agreement that we should really think twice when considering to extend > the sysfs interface for runtime PM. That certainly is correct and not limited to runtime PM. :-) > From the change-log/description to $subject patch, I fail to > understand *why* the regular runtime PM *autosuspend* feature isn't > sufficient. Perhaps Irina can elaborate more on the use case, to help > me get a better understanding of the issue!? My understanding is that the idea would be to trigger an attempt to suspend via a specific event (eg. lid closes) rather then via an inactivity timer. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-08 at 10:44 -0400, Alan Stern wrote: > It would not put the device into runtime suspend immediately, like you > are proposing. Instead it would mean the same as the "auto" mode, > except that remote wakeup should be disabled during runtime suspend. Hi, this proposal is incomplete. If you don't want remote wakeup you imply that input is no longer needed or possible. If that is already known, we can just as well inform the driver, so that it can cease IO for input. Yet that is not necessarily the only scenario. For example if you run a screensaver, you might not care for where the user touches the screen, but the event as such is valuable. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, Sep 9, 2015 at 2:50 AM, Rafael J. Wysocki wrote: > > Hi, > > On Wed, Sep 9, 2015 at 12:25 AM, Ulf Hansson wrote: > > On 8 September 2015 at 22:56, Rafael J. Wysocki wrote: > >> On Tue, Sep 8, 2015 at 9:35 AM, Oliver Neukum wrote: > >>> On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: > >> > >> [cut] > >> > this would work except for adding a sysfs attribute that would trigger > a runtime suspend while ignoring usage count. Would that be a > better direction? > >>> > >>> No. If we want this at all, we need a new callback to notify drivers > >>> that user space is temporarily uninterested in a device. And the reverse > >>> of course. > >>> The power model is good. We must not assume that devices can be > >>> suspended at will. If we do this at all, we ought to see it as giving > >>> strong hints to drivers when a device can be considered idle. > >> > >> This is a good summary in my view. > >> > >> The only thing we can add, realistically, is an interface for user > >> space to "kick" drivers to check if the devices they handle may be > >> suspended at this point (or to run their ->runtime_idle callbacks > >> IOW). > >> > >> That would be quite similar to autosuspend except that the "kick" will > >> come from user space rather than from a timer function in the kernel. > > > > Apologize for interrupting the discussion! > > > > Unless I miss the point, I assumes the above is somewhat already > > achievable via sysfs when changing the value of the auto-suspend > > timeout, since it triggers a call to > > pm_runtime_set_autosuspend_delay()... > > Well, from the initial comment in drivers/base/power/sysfs.c: > > * > *NOTE: The autosuspend_delay_ms attribute and the autosuspend_delay > *value are used only if the driver calls pm_runtime_use_autosuspend(). > * > > Some drivers don't do that and they would be the primary target > audience for the new interface (if we agreed that it was useful after > all). > > > Also, according to the discussion so far, it seems like we are on > > agreement that we should really think twice when considering to extend > > the sysfs interface for runtime PM. > > That certainly is correct and not limited to runtime PM. :-) > > > From the change-log/description to $subject patch, I fail to > > understand *why* the regular runtime PM *autosuspend* feature isn't > > sufficient. Perhaps Irina can elaborate more on the use case, to help > > me get a better understanding of the issue!? > > My understanding is that the idea would be to trigger an attempt to > suspend via a specific event (eg. lid closes) rather then via an > inactivity timer. > The best example and actually the very specific problem we want to solve is handling touchscreens on a phone / tablet. When the screen is turned off, it is ideal to suspend the touchscreen for two reasons: to lower the power consumption as much as possible and to prevent interrupts to wake-up the CPU when the user touches the device, and thus save even more power as we allow the CPU to stay in deep idle states for longer periods. Note that when the screen is turned-on again, we want to resume the touchscreen so that it can send events again. This is different then the lid closes examples, as in that case the user can not generate new events and thus the usual autosuspend feature is probably good enough (if the suspend power and autosuspend power consumption is similar). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi, On Wed, Sep 9, 2015 at 1:13 PM, Octavian Purdila wrote: > On Wed, Sep 9, 2015 at 2:50 AM, Rafael J. Wysocki wrote: >> >> Hi, >> >> On Wed, Sep 9, 2015 at 12:25 AM, Ulf Hansson wrote: >> > On 8 September 2015 at 22:56, Rafael J. Wysocki wrote: >> >> On Tue, Sep 8, 2015 at 9:35 AM, Oliver Neukum wrote: >> >>> On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: >> >> >> >> [cut] >> >> >> this would work except for adding a sysfs attribute that would trigger >> a runtime suspend while ignoring usage count. Would that be a >> better direction? >> >>> >> >>> No. If we want this at all, we need a new callback to notify drivers >> >>> that user space is temporarily uninterested in a device. And the reverse >> >>> of course. >> >>> The power model is good. We must not assume that devices can be >> >>> suspended at will. If we do this at all, we ought to see it as giving >> >>> strong hints to drivers when a device can be considered idle. >> >> >> >> This is a good summary in my view. >> >> >> >> The only thing we can add, realistically, is an interface for user >> >> space to "kick" drivers to check if the devices they handle may be >> >> suspended at this point (or to run their ->runtime_idle callbacks >> >> IOW). >> >> >> >> That would be quite similar to autosuspend except that the "kick" will >> >> come from user space rather than from a timer function in the kernel. >> > >> > Apologize for interrupting the discussion! >> > >> > Unless I miss the point, I assumes the above is somewhat already >> > achievable via sysfs when changing the value of the auto-suspend >> > timeout, since it triggers a call to >> > pm_runtime_set_autosuspend_delay()... >> >> Well, from the initial comment in drivers/base/power/sysfs.c: >> >> * >> *NOTE: The autosuspend_delay_ms attribute and the autosuspend_delay >> *value are used only if the driver calls pm_runtime_use_autosuspend(). >> * >> >> Some drivers don't do that and they would be the primary target >> audience for the new interface (if we agreed that it was useful after >> all). >> >> > Also, according to the discussion so far, it seems like we are on >> > agreement that we should really think twice when considering to extend >> > the sysfs interface for runtime PM. >> >> That certainly is correct and not limited to runtime PM. :-) >> >> > From the change-log/description to $subject patch, I fail to >> > understand *why* the regular runtime PM *autosuspend* feature isn't >> > sufficient. Perhaps Irina can elaborate more on the use case, to help >> > me get a better understanding of the issue!? >> >> My understanding is that the idea would be to trigger an attempt to >> suspend via a specific event (eg. lid closes) rather then via an >> inactivity timer. >> > > The best example and actually the very specific problem we want to > solve is handling touchscreens on a phone / tablet. When the screen is > turned off, it is ideal to suspend the touchscreen for two reasons: to > lower the power consumption as much as possible and to prevent > interrupts to wake-up the CPU when the user touches the device, and > thus save even more power as we allow the CPU to stay in deep idle > states for longer periods. > > Note that when the screen is turned-on again, we want to resume the > touchscreen so that it can send events again. In fact, then, what you need seems to be the feature discussed by Alan and me some time ago allowing remote wakeup do be disabled for runtime PM from user space as that in combination with autosuspend should address your use case. > This is different then the lid closes examples, as in that case the > user can not generate new events and thus the usual autosuspend > feature is probably good enough (if the suspend power and autosuspend > power consumption is similar). Autosuspend just means checking the device state periodically and suspending it if idle (not in use). It doesn't affect the energy consumption in the suspended state. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 2015-09-09 at 14:22 +0200, Rafael J. Wysocki wrote: > > Note that when the screen is turned-on again, we want to resume the > > touchscreen so that it can send events again. Why is it impractical to close the fd for the touchscreen? > > In fact, then, what you need seems to be the feature discussed by Alan > and me some time ago allowing remote wakeup do be disabled for runtime > PM from user space as that in combination with autosuspend should > address your use case. I'd doubt that. Suppose you put the phone into your pocket while the device isn't suspended. The continuous stream of spurious events will keep it awake. The ability to disable remote wakeup is necessary but not sufficient. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 9 Sep 2015, Oliver Neukum wrote: > On Tue, 2015-09-08 at 10:44 -0400, Alan Stern wrote: > > It would not put the device into runtime suspend immediately, like you > > are proposing. Instead it would mean the same as the "auto" mode, > > except that remote wakeup should be disabled during runtime suspend. > > Hi, > > this proposal is incomplete. If you don't want remote wakeup you > imply that input is no longer needed or possible. If that is > already known, we can just as well inform the driver, so that > it can cease IO for input. Like I said, it was never implemented. For that reason, it was never completely fleshed out. > Yet that is not necessarily the only scenario. For example > if you run a screensaver, you might not care for where the > user touches the screen, but the event as such is valuable. I suspect it's not worth the effort to distinguish between getting an event with all the details and merely knowing that an event occurred. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, Sep 9, 2015 at 4:55 PM, Oliver Neukum wrote: > On Wed, 2015-09-09 at 14:22 +0200, Rafael J. Wysocki wrote: >> > Note that when the screen is turned-on again, we want to resume the >> > touchscreen so that it can send events again. > > Why is it impractical to close the fd for the touchscreen? > I am not sure, but I think it is this way due to historical reasons. On Android devices early-suspend was used originally to save power when the screen was turned-off but the device was not suspend. Later Android moved to power HAL [1] and run-time PM for some devices, however that is not sufficient for touchscreen. i2c touchscreen devices usually have two low-power states: a deep power state where event collection is disabled and the device needs to be poked via i2c to restart collecting events and a shallow power state where the device reduces the internal polling rate after it is idle for some time. The latter it is usually implemented directly in hardware. That means that you can't really implemented auto-suspend with the deep power state, since the device can not resume itself. To address this limitation, Android used early suspend (and then the power HAL mechanism) where the upper layers signals when you can turn on or off certain devices. I have added Colin and Arve to this thread who maybe can answer this better. >> In fact, then, what you need seems to be the feature discussed by Alan >> and me some time ago allowing remote wakeup do be disabled for runtime >> PM from user space as that in combination with autosuspend should >> address your use case. > > I'd doubt that. Suppose you put the phone into your pocket while > the device isn't suspended. The continuous stream of spurious events > will keep it awake. I agree. > The ability to disable remote wakeup is necessary but not sufficient. > I don't know enough about remote wake-up, but do we even need to use it for this kind of devices? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 9 Sep 2015, Rafael J. Wysocki wrote: > > The best example and actually the very specific problem we want to > > solve is handling touchscreens on a phone / tablet. When the screen is > > turned off, it is ideal to suspend the touchscreen for two reasons: to > > lower the power consumption as much as possible and to prevent > > interrupts to wake-up the CPU when the user touches the device, and > > thus save even more power as we allow the CPU to stay in deep idle > > states for longer periods. > > > > Note that when the screen is turned-on again, we want to resume the > > touchscreen so that it can send events again. > > In fact, then, what you need seems to be the feature discussed by Alan > and me some time ago allowing remote wakeup do be disabled for runtime > PM from user space as that in combination with autosuspend should > address your use case. That, plus they want the touchscreen to go into runtime suspend whenever the screen is off (was this not the main reason for the patch?). It seems to me that it should be possible to arrange for this to happen simply by making userspace close the touchscreen device when the screen is turned off. Or am I missing something? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 8 Sep 2015, Rafael J. Wysocki wrote: > Hi, > > On Tue, Sep 8, 2015 at 5:00 PM, Alan Stern wrote: > > On Tue, 8 Sep 2015, Rafael J. Wysocki wrote: > > > >> > > [1] http://marc.info/?l=linux-input&m=140564626306396&w=2 > >> > > >> > Purely as a matter of interest, in that email Rafael also mentioned > >> > that he and I had discussed a way to disable remote wakeup during > >> > runtime suspend. Oddly enough, the method we decided upon was to add > >> > an "off" option to /sys/.../power/control. :-) > >> > >> Wasn't that /sys/devices/.../power/wakeup rather? > > > > Not the way I remember. Of course, it's possible that we misunderstood > > each other at the time. > > > >> > It would not put the device into runtime suspend immediately, like you > >> > are proposing. Instead it would mean the same as the "auto" mode, > >> > except that remote wakeup should be disabled during runtime suspend. > >> > > >> > We never got around to implementing this, however. > >> > >> I don't think this is what we discussed then really. > >> > >> There is a fundamental problem with forcing things into runtime suspend > >> from user space, because that may happen in a wrong time. In other words, > >> the kernel can't guarantee that the device would actually be able to go > >> into runtime suspend when requested. > > > > Exactly. What we discussed at LinuxCon wasn't forcing things into > > runtime suspend; it was disabling remote wakeup during runtime suspend. > > > > And even though the topic was quite different from Irina's proposal, we > > ended up settling on the same API (according to my recollection). > > So I remember that differently. > > My idea was to add a third value to /sys/devices/.../power/wakeup (in > addition to "disabled" and "enabled") so user space can indicate that > remote wakeup should not be enabled for runtime suspend for the device > (since there's no way to indicate that today). I don't see how > /sys/devices/.../power/control might help here to be honest. You're right, that does make more sense than what I was thinking. My memory must have gotten messed up. RAM corruption, no doubt... I think I need an EDAC brain. :-) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wednesday, September 09, 2015 06:02:02 PM Octavian Purdila wrote: > On Wed, Sep 9, 2015 at 4:55 PM, Oliver Neukum wrote: > > On Wed, 2015-09-09 at 14:22 +0200, Rafael J. Wysocki wrote: > >> > Note that when the screen is turned-on again, we want to resume the > >> > touchscreen so that it can send events again. > > > > Why is it impractical to close the fd for the touchscreen? > > > > I am not sure, but I think it is this way due to historical reasons. > On Android devices early-suspend was used originally to save power > when the screen was turned-off but the device was not suspend. Later > Android moved to power HAL [1] and run-time PM for some devices, > however that is not sufficient for touchscreen. > > i2c touchscreen devices usually have two low-power states: a deep > power state where event collection is disabled and the device needs to > be poked via i2c to restart collecting events and a shallow power > state where the device reduces the internal polling rate after it is > idle for some time. The latter it is usually implemented directly in > hardware. That means that you can't really implemented auto-suspend > with the deep power state, since the device can not resume itself. > > To address this limitation, Android used early suspend (and then the > power HAL mechanism) where the upper layers signals when you can turn > on or off certain devices. > > I have added Colin and Arve to this thread who maybe can answer this better. > > >> In fact, then, what you need seems to be the feature discussed by Alan > >> and me some time ago allowing remote wakeup do be disabled for runtime > >> PM from user space as that in combination with autosuspend should > >> address your use case. > > > > I'd doubt that. Suppose you put the phone into your pocket while > > the device isn't suspended. The continuous stream of spurious events > > will keep it awake. Why would they be regarded as spurious then? They are just regular touch panel events in that case, aren't they? > I agree. > > > The ability to disable remote wakeup is necessary but not sufficient. > > > > I don't know enough about remote wake-up, but do we even need to use > it for this kind of devices? Well, if the device is capable of generating wakeup events while suspended, the current expected behavior is to do that, but of course that needs to be enabled by the driver/bus type too. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wednesday, September 09, 2015 11:20:25 AM Alan Stern wrote: > On Wed, 9 Sep 2015, Rafael J. Wysocki wrote: > > > > The best example and actually the very specific problem we want to > > > solve is handling touchscreens on a phone / tablet. When the screen is > > > turned off, it is ideal to suspend the touchscreen for two reasons: to > > > lower the power consumption as much as possible and to prevent > > > interrupts to wake-up the CPU when the user touches the device, and > > > thus save even more power as we allow the CPU to stay in deep idle > > > states for longer periods. > > > > > > Note that when the screen is turned-on again, we want to resume the > > > touchscreen so that it can send events again. > > > > In fact, then, what you need seems to be the feature discussed by Alan > > and me some time ago allowing remote wakeup do be disabled for runtime > > PM from user space as that in combination with autosuspend should > > address your use case. > > That, plus they want the touchscreen to go into runtime suspend > whenever the screen is off (was this not the main reason for the > patch?). Right. > It seems to me that it should be possible to arrange for this to happen > simply by making userspace close the touchscreen device when the screen > is turned off. Or am I missing something? Honestly, I don't know. Octavian, Irina, any reasons why things can't be done as Alan is suggesting? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, Sep 9, 2015 at 1:35 PM, Rafael J. Wysocki wrote: > > On Wednesday, September 09, 2015 11:20:25 AM Alan Stern wrote: > > On Wed, 9 Sep 2015, Rafael J. Wysocki wrote: > > > > > > The best example and actually the very specific problem we want to > > > > solve is handling touchscreens on a phone / tablet. When the screen is > > > > turned off, it is ideal to suspend the touchscreen for two reasons: to > > > > lower the power consumption as much as possible and to prevent > > > > interrupts to wake-up the CPU when the user touches the device, and > > > > thus save even more power as we allow the CPU to stay in deep idle > > > > states for longer periods. > > > > > > > > Note that when the screen is turned-on again, we want to resume the > > > > touchscreen so that it can send events again. > > > > > > In fact, then, what you need seems to be the feature discussed by Alan > > > and me some time ago allowing remote wakeup do be disabled for runtime > > > PM from user space as that in combination with autosuspend should > > > address your use case. > > > > That, plus they want the touchscreen to go into runtime suspend > > whenever the screen is off (was this not the main reason for the > > patch?). > > Right. > > > It seems to me that it should be possible to arrange for this to happen > > simply by making userspace close the touchscreen device when the screen > > is turned off. Or am I missing something? > > Honestly, I don't know. > > Octavian, Irina, any reasons why things can't be done as Alan is suggesting? Early Android used early suspend, which notified various kernel drivers that the screen was turning off so they could go into low power states. That meant there was no reason to close the touchscreen fd - the kernel already knew about the screen off event. We then got rid of the early suspend hack and moved everything into userspace using the power HAL and sysfs files. Getting rid of the touchscreen sysfs files and closing the touchscreen on screen off was on the nice-to-have list, but hasn't made it onto anybody's todo list. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 2015-09-09 at 22:25 +0200, Rafael J. Wysocki wrote: > > > > > > I'd doubt that. Suppose you put the phone into your pocket while > > > the device isn't suspended. The continuous stream of spurious > events > > > will keep it awake. > > Why would they be regarded as spurious then? They are just regular > touch panel > events in that case, aren't they? These events are not expected to be caused by the user's hand. But it raises a design question; whose job it is to handle such information? It makes no sense to gather events from a touchscreen if you suspect the phone is randomly rubbing at things or to take video from a camera if you know that the lid is closed covering the lens. I think we can agree to that. The thing is that we handle all other availability in kernel space. You can argue that user space has an agreed interface (evdev, V4L or whatever) and it is the kernel's job to react if it learns that a device becomes temporarily unavailable and this is merely a question of adding an interface to the kernel by which user space can feed such information to the kernel. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi! > > > > > >> > 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. Well.. .excess input events means that your phone now sends (meaningful, thanks to advanced predictions) messages to your friends... Better not do that. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
> > >> In fact, then, what you need seems to be the feature discussed by Alan > > >> and me some time ago allowing remote wakeup do be disabled for runtime > > >> PM from user space as that in combination with autosuspend should > > >> address your use case. > > > > > > I'd doubt that. Suppose you put the phone into your pocket while > > > the device isn't suspended. The continuous stream of spurious events > > > will keep it awake. > > Why would they be regarded as spurious then? They are just regular touch > panel > events in that case, aren't they? >From userspace... they are spurious. Userspace does not known that your device is commonly placed into the pocket, and the events it sees are not from user. I have mainline X/mate running on n900 cellphone. And this means battery life is in "2 hours" range. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed 2015-09-09 11:20:25, Alan Stern wrote: > On Wed, 9 Sep 2015, Rafael J. Wysocki wrote: > > > > The best example and actually the very specific problem we want to > > > solve is handling touchscreens on a phone / tablet. When the screen is > > > turned off, it is ideal to suspend the touchscreen for two reasons: to > > > lower the power consumption as much as possible and to prevent > > > interrupts to wake-up the CPU when the user touches the device, and > > > thus save even more power as we allow the CPU to stay in deep idle > > > states for longer periods. > > > > > > Note that when the screen is turned-on again, we want to resume the > > > touchscreen so that it can send events again. > > > > In fact, then, what you need seems to be the feature discussed by Alan > > and me some time ago allowing remote wakeup do be disabled for runtime > > PM from user space as that in combination with autosuspend should > > address your use case. > > That, plus they want the touchscreen to go into runtime suspend > whenever the screen is off (was this not the main reason for the > patch?). > > It seems to me that it should be possible to arrange for this to happen > simply by making userspace close the touchscreen device when the screen > is turned off. Or am I missing something? Well... that's not what existing userspace expects. Your X windows server will not close the touchscreen. ..and it would be nice to have enough hardware abstraction in the kernel so that X can be used on phones... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, 21 Sep 2015, Pavel Machek wrote: > > > In fact, then, what you need seems to be the feature discussed by Alan > > > and me some time ago allowing remote wakeup do be disabled for runtime > > > PM from user space as that in combination with autosuspend should > > > address your use case. > > > > That, plus they want the touchscreen to go into runtime suspend > > whenever the screen is off (was this not the main reason for the > > patch?). > > > > It seems to me that it should be possible to arrange for this to happen > > simply by making userspace close the touchscreen device when the screen > > is turned off. Or am I missing something? > > Well... that's not what existing userspace expects. Your X windows > server will not close the touchscreen. Surely that's a userspace issue, rather than a kernel problem? The X server does have some notion of power management and power savings; why not extend that notion to include touchscreens? > ..and it would be nice to have enough hardware abstraction in the > kernel so that X can be used on phones... What -- not Wayland?! :-) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, Sep 21, 2015 at 10:38:46AM -0400, Alan Stern wrote: > On Mon, 21 Sep 2015, Pavel Machek wrote: > > > > > In fact, then, what you need seems to be the feature discussed by Alan > > > > and me some time ago allowing remote wakeup do be disabled for runtime > > > > PM from user space as that in combination with autosuspend should > > > > address your use case. > > > > > > That, plus they want the touchscreen to go into runtime suspend > > > whenever the screen is off (was this not the main reason for the > > > patch?). > > > > > > It seems to me that it should be possible to arrange for this to happen > > > simply by making userspace close the touchscreen device when the screen > > > is turned off. Or am I missing something? > > > > Well... that's not what existing userspace expects. Your X windows > > server will not close the touchscreen. > > Surely that's a userspace issue, rather than a kernel problem? The X > server does have some notion of power management and power savings; why > not extend that notion to include touchscreens? It is not really practical: there are many consumers of input events, if we build infrastructure to control it and proxy all users through it, why not have it in kernel? Plus, there are users of input events directly in the kernel, such as legacy VT/keyboard, or Android/ChromeOS cpufreq_interactive governor that monitors user activity and bumps up CPU speed when user actively interacts with the device. They would keep input devices active even though user might not be actually able to use some input devices. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, 21 Sep 2015, Dmitry Torokhov wrote: > On Mon, Sep 21, 2015 at 10:38:46AM -0400, Alan Stern wrote: > > On Mon, 21 Sep 2015, Pavel Machek wrote: > > > > > > > In fact, then, what you need seems to be the feature discussed by Alan > > > > > and me some time ago allowing remote wakeup do be disabled for runtime > > > > > PM from user space as that in combination with autosuspend should > > > > > address your use case. > > > > > > > > That, plus they want the touchscreen to go into runtime suspend > > > > whenever the screen is off (was this not the main reason for the > > > > patch?). > > > > > > > > It seems to me that it should be possible to arrange for this to happen > > > > simply by making userspace close the touchscreen device when the screen > > > > is turned off. Or am I missing something? > > > > > > Well... that's not what existing userspace expects. Your X windows > > > server will not close the touchscreen. > > > > Surely that's a userspace issue, rather than a kernel problem? The X > > server does have some notion of power management and power savings; why > > not extend that notion to include touchscreens? > > It is not really practical: there are many consumers of input events, if > we build infrastructure to control it and proxy all users through it, > why not have it in kernel? Plus, there are users of input events > directly in the kernel, such as legacy VT/keyboard, or Android/ChromeOS > cpufreq_interactive governor that monitors user activity and bumps up > CPU speed when user actively interacts with the device. They would keep > input devices active even though user might not be actually able to > use some input devices. It sounds like you are suggesting there should be a general mechanism for userspace to tell the kernel (or the input core) to ignore all events from a particular input device -- or even from all input devices -- thereby allowing those devices to go to low power. I don't like to think of this as "forcing runtime suspend". It's more like telling the kernel that a device is no longer being used, so the natural runtime PM mechanism can put it in runtime suspend. Perhaps another way to think about it is that these input devices should not increment their runtime usage counter as part of the open routine; they should use something other than the number of open file references to indicate when they can go into runtime suspend. (I'm not sure what else they should use, though.) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, Sep 21, 2015 at 12:34:56PM -0400, Alan Stern wrote: > On Mon, 21 Sep 2015, Dmitry Torokhov wrote: > > > On Mon, Sep 21, 2015 at 10:38:46AM -0400, Alan Stern wrote: > > > On Mon, 21 Sep 2015, Pavel Machek wrote: > > > > > > > > > In fact, then, what you need seems to be the feature discussed by > > > > > > Alan > > > > > > and me some time ago allowing remote wakeup do be disabled for > > > > > > runtime > > > > > > PM from user space as that in combination with autosuspend should > > > > > > address your use case. > > > > > > > > > > That, plus they want the touchscreen to go into runtime suspend > > > > > whenever the screen is off (was this not the main reason for the > > > > > patch?). > > > > > > > > > > It seems to me that it should be possible to arrange for this to > > > > > happen > > > > > simply by making userspace close the touchscreen device when the > > > > > screen > > > > > is turned off. Or am I missing something? > > > > > > > > Well... that's not what existing userspace expects. Your X windows > > > > server will not close the touchscreen. > > > > > > Surely that's a userspace issue, rather than a kernel problem? The X > > > server does have some notion of power management and power savings; why > > > not extend that notion to include touchscreens? > > > > It is not really practical: there are many consumers of input events, if > > we build infrastructure to control it and proxy all users through it, > > why not have it in kernel? Plus, there are users of input events > > directly in the kernel, such as legacy VT/keyboard, or Android/ChromeOS > > cpufreq_interactive governor that monitors user activity and bumps up > > CPU speed when user actively interacts with the device. They would keep > > input devices active even though user might not be actually able to > > use some input devices. > > It sounds like you are suggesting there should be a general mechanism > for userspace to tell the kernel (or the input core) to ignore all > events from a particular input device -- or even from all input devices > -- thereby allowing those devices to go to low power. Yes. In ChromeOS we have a custim "inhibit" control that: 1. Tells input core to ignore all events form a given device 2. Allows driver to put device in low power mode if driver desires to do so. The driver can do it via runtime PM or on it's own. Usually on it's own since when using runtime PM userspace may disable it, which may not be desirable. I would love to have something generic instead of input-specific. > > I don't like to think of this as "forcing runtime suspend". It's more > like telling the kernel that a device is no longer being used, so the > natural runtime PM mechanism can put it in runtime suspend. I'd call it "accelerating" runtime suspend. Userspace tells the kernel that it intends not to use given device and kernel reacts accordingly. > > Perhaps another way to think about it is that these input devices > should not increment their runtime usage counter as part of the open > routine; they should use something other than the number of open file > references to indicate when they can go into runtime suspend. (I'm not > sure what else they should use, though.) I do not really want input specific support; as I mentioned before we have something like that in ChromeOS kernels but I was hesitant bringing it upstream as I believe it is not necessarily input device specific and I would love to have it implemented at device core level. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, 21 Sep 2015, Dmitry Torokhov wrote: > > It sounds like you are suggesting there should be a general mechanism > > for userspace to tell the kernel (or the input core) to ignore all > > events from a particular input device -- or even from all input devices > > -- thereby allowing those devices to go to low power. > > Yes. In ChromeOS we have a custim "inhibit" control that: > > 1. Tells input core to ignore all events form a given device > 2. Allows driver to put device in low power mode if driver desires to do > so. The driver can do it via runtime PM or on it's own. Usually on it's > own since when using runtime PM userspace may disable it, which may not > be desirable. > > I would love to have something generic instead of input-specific. > > > > > I don't like to think of this as "forcing runtime suspend". It's more > > like telling the kernel that a device is no longer being used, so the > > natural runtime PM mechanism can put it in runtime suspend. > > I'd call it "accelerating" runtime suspend. Userspace tells the kernel > that it intends not to use given device and kernel reacts accordingly. Okay. > > Perhaps another way to think about it is that these input devices > > should not increment their runtime usage counter as part of the open > > routine; they should use something other than the number of open file > > references to indicate when they can go into runtime suspend. (I'm not > > sure what else they should use, though.) > > I do not really want input specific support; as I mentioned before we > have something like that in ChromeOS kernels but I was hesitant bringing > it upstream as I believe it is not necessarily input device specific and > I would love to have it implemented at device core level. That's not a bad idea. On the other hand, there must be lots of devices which would not be suitable for this. Disk drives, for instance. 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 if the I/O request originated from somewhere in the kernel, not from the user? Or maybe the driver would want to carry out the request, overriding the "inhibit" control temporarily. Does it simply turn off the control, meaning that the device won't go back into runtime suspend until userspace turns the control on again? Or if the driver doesn't turn off the "inhibit" control, then how does it know when it can safely put the device back into runtime suspend? Qustions like these make me think that this mechanism is best suited for a kind of device that doesn't handle I/O requests. In other words, something that just reports events as they occur -- which is another way of describing an input device! Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, Sep 21, 2015 at 01:32:38PM -0400, Alan Stern wrote: > On Mon, 21 Sep 2015, Dmitry Torokhov wrote: > > > > It sounds like you are suggesting there should be a general mechanism > > > for userspace to tell the kernel (or the input core) to ignore all > > > events from a particular input device -- or even from all input devices > > > -- thereby allowing those devices to go to low power. > > > > Yes. In ChromeOS we have a custim "inhibit" control that: > > > > 1. Tells input core to ignore all events form a given device > > 2. Allows driver to put device in low power mode if driver desires to do > > so. The driver can do it via runtime PM or on it's own. Usually on it's > > own since when using runtime PM userspace may disable it, which may not > > be desirable. > > > > I would love to have something generic instead of input-specific. > > > > > > > > I don't like to think of this as "forcing runtime suspend". It's more > > > like telling the kernel that a device is no longer being used, so the > > > natural runtime PM mechanism can put it in runtime suspend. > > > > I'd call it "accelerating" runtime suspend. Userspace tells the kernel > > that it intends not to use given device and kernel reacts accordingly. > > Okay. > > > > Perhaps another way to think about it is that these input devices > > > should not increment their runtime usage counter as part of the open > > > routine; they should use something other than the number of open file > > > references to indicate when they can go into runtime suspend. (I'm not > > > sure what else they should use, though.) > > > > I do not really want input specific support; as I mentioned before we > > have something like that in ChromeOS kernels but I was hesitant bringing > > it upstream as I believe it is not necessarily input device specific and > > I would love to have it implemented at device core level. > > That's not a bad idea. On the other hand, there must be lots of > devices which would not be suitable for this. Disk drives, for > instance. Of course. > > 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? > > What if the I/O request originated from somewhere in the > kernel, not from the user? I think we should treat in-kernel users as all other users. > > Or maybe the driver would want to carry out the request, > overriding the "inhibit" control temporarily. Does it simply > turn off the control, meaning that the device won't go back > into runtime suspend until userspace turns the control on > again? > > Or if the driver doesn't turn off the "inhibit" control, then > how does it know when it can safely put the device back into > runtime suspend? > > Qustions like these make me think that this mechanism is best suited > for a kind of device that doesn't handle I/O requests. In other words, > something that just reports events as they occur -- which is another > way of describing an input device! Or maybe IIO device. Or hwmon. Or something else. I think if we allow drivers (or subsystems) to opt in into this mechanism it will solve much of worries about disks and similar devices that indeed not very suitable for such mechanism. Thanks. - Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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. Of course, this issue doesn't arise for devices that merely report external events. > > What if the I/O request originated from somewhere in the > > kernel, not from the user? > > I think we should treat in-kernel users as all other users. > > > > > Or maybe the driver would want to carry out the request, > > overriding the "inhibit" control temporarily. Does it simply > > turn off the control, meaning that the device won't go back > > into runtime suspend until userspace turns the control on > > again? > > > > Or if the driver doesn't turn off the "inhibit" control, then > > how does it know when it can safely put the device back into > > runtime suspend? > > > > Qustions like these make me think that this mechanism is best suited > > for a kind of device that doesn't handle I/O requests. In other words, > > something that just reports events as they occur -- which is another > > way of describing an input device! > > Or maybe IIO device. Or hwmon. Or something else. I think if we allow > drivers (or subsystems) to opt in into this mechanism it will solve much > of worries about disks and similar devices that indeed not very suitable > for such mechanism. Should the mechanism really be per-device? Or would it be more useful to have a single "inhibit" setting that affected all the relevant devices at once? 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. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon 2015-09-21 10:38:46, Alan Stern wrote: > On Mon, 21 Sep 2015, Pavel Machek wrote: > > > > > In fact, then, what you need seems to be the feature discussed by Alan > > > > and me some time ago allowing remote wakeup do be disabled for runtime > > > > PM from user space as that in combination with autosuspend should > > > > address your use case. > > > > > > That, plus they want the touchscreen to go into runtime suspend > > > whenever the screen is off (was this not the main reason for the > > > patch?). > > > > > > It seems to me that it should be possible to arrange for this to happen > > > simply by making userspace close the touchscreen device when the screen > > > is turned off. Or am I missing something? > > > > Well... that's not what existing userspace expects. Your X windows > > server will not close the touchscreen. > > Surely that's a userspace issue, rather than a kernel problem? The X > server does have some notion of power management and power savings; why > not extend that notion to include touchscreens? Well... once upon a time, it was kernel job to mask differences between different hardware platforms. In a way, the hardware is "buggy" -- if your mouse clicked randomly when you were not holding it in your hand it would be buggy... and it would be nice for kernel to fix that "bug". Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, Sep 21, 2015 at 04:02:01PM -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. > > Of course, this issue doesn't arise for devices that merely report > external events. > > > > What if the I/O request originated from somewhere in the > > > kernel, not from the user? > > > > I think we should treat in-kernel users as all other users. > > > > > > > > Or maybe the driver would want to carry out the request, > > > overriding the "inhibit" control temporarily. Does it simply > > > turn off the control, meaning that the device won't go back > > > into runtime suspend until userspace turns the control on > > > again? > > > > > > Or if the driver doesn't turn off the "inhibit" control, then > > > how does it know when it can safely put the device back into > > > runtime suspend? > > > > > > Qustions like these make me think that this mechanism is best suited > > > for a kind of device that doesn't handle I/O requests. In other words, > > > something that just reports events as they occur -- which is another > > > way of describing an input device! > > > > Or maybe IIO device. Or hwmon. Or something else. I think if we allow > > drivers (or subsystems) to opt in into this mechanism it will solve much > > of worries about disks and similar devices that indeed not very suitable > > for such mechanism. > > Should the mechanism really be per-device? Or would it be more useful > to have a single "inhibit" setting that affected all the relevant > devices at once? Definitely per device. Consider your laptop with external monitor and keyboard connected. When you close the lid you want to inhibit internal keyboard, touchpad and touchscreen while leaving external keyboard and mouse working. That's just one scenario. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, Sep 23, 2015 at 6:03 AM, 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. > > > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi! > > > > 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 Well... In "cellphone goes to pocket" case, you want to turn off I/O even if the touchscreen can not support runtime suspend. See parents in the thread for explanation. Pavel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Sunday, September 27, 2015 07:02:17 PM Pavel Machek wrote: > Hi! Hi, > > > > > 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 > > Well... In "cellphone goes to pocket" case, you want to turn off I/O even if > the touchscreen can not support runtime suspend. > > See parents in the thread for explanation. You seem to be confusing the ability to go into low-power states with supporting runtime PM. The latter by no means requires the former. Also "cellphone goes to pocket" is really two different cases. One is when the user indicated "I'm not going to use the phone going forward" somehow (like by pressing a screen-off button) and one is when (s)he didn't. In the second case we really have no reason to discard any input and in the first one we may as well go straight for runtime suspend (or even for system suspend for that matter). Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Hi Alan, On Mon, Sep 28, 2015 at 4:29 PM, Alan Stern 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, 28 Sep 2015, Rafael J. Wysocki wrote: > Hi Alan, > > On Mon, Sep 28, 2015 at 4:29 PM, Alan Stern 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 in
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/