Re: [Ksummit-discuss] [TECH TOPIC] System-wide interface to specify the level of PM tuning

[josh@joshtriplett.org]

On Wed, Jul 22, 2015 at 07:25:40PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 22, 2015 09:18:34 AM Daniel Vetter wrote:
> > On Wed, Jul 22, 2015 at 3:12 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > > On Tuesday, July 21, 2015 01:09:52 AM Daniel Vetter wrote:
> > >> On Tue, Jul 21, 2015 at 12:21 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > >> > On Friday, July 17, 2015 01:41:56 PM Daniel Vetter wrote:
> > >> >> On Thu, Jul 16, 2015 at 5:58 PM, Greg KH <greg@kroah.com> wrote:
> > >> >> > On Thu, Jul 16, 2015 at 08:53:02AM -0700, Josh Triplett wrote:
> > >> >> >> But the kernel already has quirk tables for various hardware, and that
> > >> >> >> seems appropriate to continue putting in the kernel.
> > >> >> >
> > >> >> > For some types of devices, sure.  For others, like broken USB keyboards
> > >> >> > that can't handle autosuspend, no.  For those we need a userspace
> > >> >> > _whitelist_ that udev can use.  So there's no one answer that works for
> > >> >> > all types of quirks.
> > >> >>
> > >> >> Whether white or blacklist or some other mixed thing doesn't really
> > >> >> matter. Imo the important part is that driver maintainers are in the
> > >> >> best position to maintain that, and pushing it out to anyone else is
> > >> >> just really not doing our jobs. And I think for most of these quirk
> > >> >> lists the kernel does seem like the most appropriate place. If the
> > >> >> list becomes giantic then we can move it to userspace (if that's
> > >> >> really a problem, afaik no one proposed yet to move device match
> > >> >> tables into userspace either and that's kinda the same thing really).
> > >> >> But as long as there's no white/black/whatever list yet starting in
> > >> >> the kernel is imo the right place.
> > >> >
> > >> > Well, I'm wondering, then, why i915.enable_psr is not enabled by default,
> > >> > for one example?
> > >> >
> > >> > Failing to enable it prevents some SoCs from using the deepest available
> > >> > C-states which in turn hurts battery life of the systems containing them
> > >> > quite a bit, so there surely is a reason to have it enabled.
> > >>
> > >> Because it's broken on a lot of machines, and it takes a pile of
> > >> effort to fix it.
> > >
> > > And there are quite a few subsystems having similar issues here and there.
> > >
> > > People who aren't aware of those command line/Kconfig/sysfs switches will
> > > never enable those features even though they may work well on their
> > > machines and may actually be necessary to save energy.
> > 
> > In my experience there's way too many people around who know about
> > these knobs but have no idea that they might be somewhat dangerous.
> > And then I get another bug report about a known bug just because
> > someone read a blog somewhere. Nowadays almost all i915 tuning knobs
> > are marked as _unsafe and taint your kernel if you touch them.
> > 
> > >> That work is under way now, but for a long time
> > >> priorities set by management where much more set on chasing the next
> > >> shiny thing. Took a few years of making noises about dropping it all
> > >> if it doesn't get fixed.
> > >>
> > >> This is actually a perfect example of what I mean with "hey it works
> > >> on my machine here, but I can't be bothered to fix up the corner-cases
> > >> so let's keep it disabled and move on". And the corner cases are hung
> > >> machines and frozen displays (and a few other things), and we
> > >> inflicted that a few times on Linus even.
> > >
> > > So among other things this topic is about a mechanism to possibly enable
> > > multiple things like that in one go instead of having to switch multiple
> > > knobs in various places (and needing to know about them in the first
> > > place).
> > 
> > I know, but at least for i915 I don't want it: When we know it's safe
> > to do we already enable all the power/performance tuning we have, and
> > if we know it's unsafe we don't want users to enable it themselves. If
> > you have a very specific product (which is not a generic distro or
> > anything) and have done careful testing and cross-checked with
> > developers then you can of course just enable these features. But then
> > you also don't need a new option, you can just change the driver
> > defaults directly.
> 
> OK
> 
> So perhaps the question should be whether or not there is any viable approach
> we can use to avoid or at least reduce the amount of all of the mostly manual
> tuning to get as much battery life from systems as they can provide.  And that
> cannot involve user space realistically, because some variants of it may just
> not do whatever we expect them to do.

Tunables like those in i915 aren't going to be able to use any better
approach, specifically because they don't work everywhere; if we had a
list of systems they were safe on, we'd set up a quirk list and turn
them on by default.  Do we have any tunables that *aren't* in that
category?  And if so, why don't we just set their defaults to DTRT?

- Josh Triplett