From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id E3E5D487 for ; Sat, 25 Jul 2015 19:50:27 +0000 (UTC) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id AA260121 for ; Sat, 25 Jul 2015 19:50:26 +0000 (UTC) Date: Sat, 25 Jul 2015 12:50:16 -0700 From: Josh Triplett To: "Rafael J. Wysocki" Message-ID: <20150725195016.GB9753@x> References: <1489458.8WDRattPkl@vostro.rjw.lan> <1700413.HOtzHoJBt9@vostro.rjw.lan> <20150722182519.GB26620@cloud> <2425484.kWOCROb332@vostro.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2425484.kWOCROb332@vostro.rjw.lan> Cc: "Brown, Len" , "ksummit-discuss@lists.linuxfoundation.org" , Alan Stern , Kristen Carlson Accardi , Grant Likely Subject: Re: [Ksummit-discuss] [TECH TOPIC] System-wide interface to specify the level of PM tuning List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sat, Jul 25, 2015 at 12:36:18AM +0200, Rafael J. Wysocki wrote: > On Wednesday, July 22, 2015 11:25:19 AM josh@joshtriplett.org wrote: > > 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 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 wrote: > > > > >> > On Friday, July 17, 2015 01:41:56 PM Daniel Vetter wrote: > > > > >> >> On Thu, Jul 16, 2015 at 5:58 PM, Greg KH 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? > > I really don't think so, maybe except for some corner things. > > > And if so, why don't we just set their defaults to DTRT? > > But we seem to have more and more tunables for things that "don't work > everywhere", while at the same time the subset of systems where they *do* work > is quite substantial. > > In many cases we simply don't have enough information to create a black or white > list for them, so we disable them by default and add a tunable to enable them > "in case someone needs" them. This bothers me a bit, because it doesn't seem > to be sustainable in the long term. I agree completely. However, it seems like there's always enough information for a whitelist, if you know at least one system it works on. Then people can submit patches extending the whitelist. But in any case, this seems to argue against a global "use less power" switch, because any one of the underlying options might potentially break on the user's system. Any that can be safely toggled by a master switch should just come pre-toggled. :) - Josh Triplett