linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Neukum <oneukum@suse.com>
To: "Tirdea, Irina" <irina.tirdea@intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Alan Stern <stern@rowland.harvard.edu>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Brown, Len" <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	"Purdila, Octavian" <octavian.purdila@intel.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
Date: Tue, 08 Sep 2015 09:35:44 +0200	[thread overview]
Message-ID: <1441697744.26994.21.camel@suse.com> (raw)
In-Reply-To: <1F3AC3675D538145B1661F571FE1805F2F0C5CBE@irsmsx105.ger.corp.intel.com>

On Tue, 2015-09-08 at 01:10 +0000, 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



  reply	other threads:[~2015-09-08  7:35 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-07 20:42 [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend Irina Tirdea
2015-09-07 21:20 ` Rafael J. Wysocki
2015-09-08  1:10   ` Tirdea, Irina
2015-09-08  7:35     ` Oliver Neukum [this message]
2015-09-08 20:56       ` Rafael J. Wysocki
2015-09-08 22:25         ` Ulf Hansson
2015-09-08 23:50           ` Rafael J. Wysocki
2015-09-09 11:13             ` Octavian Purdila
2015-09-09 12:22               ` Rafael J. Wysocki
2015-09-09 13:55                 ` Oliver Neukum
2015-09-09 15:02                   ` Octavian Purdila
2015-09-09 20:25                     ` Rafael J. Wysocki
2015-09-10  9:38                       ` Oliver Neukum
2015-09-21 12:29                       ` Pavel Machek
2015-09-09 15:20                 ` Alan Stern
2015-09-09 20:35                   ` Rafael J. Wysocki
2015-09-09 20:16                     ` Colin Cross
2015-09-21 12:30                   ` Pavel Machek
2015-09-21 14:38                     ` Alan Stern
2015-09-21 16:16                       ` Dmitry Torokhov
2015-09-21 16:34                         ` Alan Stern
2015-09-21 16:59                           ` Dmitry Torokhov
2015-09-21 17:32                             ` Alan Stern
2015-09-21 18:00                               ` Dmitry Torokhov
2015-09-21 20:02                                 ` Alan Stern
2015-09-21 20:56                                   ` Dmitry Torokhov
2015-09-22 12:05                                   ` Oliver Neukum
2015-09-22 14:15                                     ` Alan Stern
2015-09-22 14:31                                       ` Oliver Neukum
2015-09-22 15:22                                         ` Alan Stern
2015-09-23  3:03                                           ` Oliver Neukum
2015-09-23  7:27                                             ` Octavian Purdila
2015-09-23 14:55                                             ` Alan Stern
2015-09-25  0:43                                               ` Rafael J. Wysocki
2015-09-25 14:29                                                 ` Alan Stern
2015-09-25 20:15                                                   ` Rafael J. Wysocki
2015-09-25 21:13                                                     ` Alan Stern
2015-09-25 21:52                                                       ` Rafael J. Wysocki
2015-09-25 23:04                                                         ` Rafael J. Wysocki
2015-09-26 15:20                                                           ` Alan Stern
2015-09-27 13:41                                                             ` Rafael J. Wysocki
2015-09-27 14:27                                                               ` Alan Stern
2015-09-28 13:41                                                                 ` Rafael J. Wysocki
2015-09-28 14:29                                                                   ` Alan Stern
2015-09-28 20:03                                                                     ` Rafael J. Wysocki
2015-09-28 20:23                                                                       ` Alan Stern
2015-10-04 15:16                                                                         ` Pavel Machek
2015-09-27 17:02                                                               ` Pavel Machek
2015-09-28 13:47                                                                 ` Rafael J. Wysocki
2015-09-21 20:20                       ` Pavel Machek
2015-09-08 14:44     ` Alan Stern
2015-09-08 15:15       ` Rafael J. Wysocki
2015-09-08 15:00         ` Alan Stern
2015-09-08 20:28           ` Rafael J. Wysocki
2015-09-09 15:22             ` Alan Stern
2015-09-09  6:26       ` Oliver Neukum
2015-09-09 14:33         ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441697744.26994.21.camel@suse.com \
    --to=oneukum@suse.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=irina.tirdea@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=octavian.purdila@intel.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).