Re: [PATCH 0/5] PowerManagement Toggle for PowerTOP

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: "Swaminathan, Nivedita" <nivedita.swaminathan@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Konno, Joe" <joe.konno@intel.com>,
	"Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 0/5] PowerManagement Toggle for PowerTOP
Date: Fri, 15 Apr 2016 18:12:32 +0000	[thread overview]
Message-ID: <1460744030.30898.68.camel@intel.com> (raw)
In-Reply-To: <20160414094857.GK2510@phenom.ffwll.local>

On Thu, 2016-04-14 at 11:48 +0200, Daniel Vetter wrote:
> On Wed, Apr 13, 2016 at 10:06:57PM +0000, Vivi, Rodrigo wrote:
> > On Wed, 2016-04-13 at 22:46 +0200, Daniel Vetter wrote:
> > > On Wed, Apr 13, 2016 at 10:38 PM, Vivi, Rodrigo <
> > > rodrigo.vivi@intel.com> wrote:
> > > > On Wed, 2016-04-13 at 16:50 +0200, Daniel Vetter wrote:
> > > > > On Wed, Apr 13, 2016 at 12:59:18PM +0000, Zanoni, Paulo R
> > > > > wrote:
> > > > > > Em Ter, 2016-04-12 às 12:18 -0700, Alexandra Yates
> > > > > > escreveu:
> > > > > > > This project is explained in detail on the HAS
> > > > > > > https://docs.google.com/a/intel.com/document/d/1E-en_xqfH
> > > > > > > gCnh
> > > > > > > D1Te
> > > > > > > s3f0
> > > > > > > 8UYrOc-etv2W-pU0ZErKdE/edit?usp=sharing
> > > > > > > 
> > > > > > > Summary:
> > > > > > > Permits the user to identify and toggle values for PSR,
> > > > > > > FBC,
> > > > > > > RC6,
> > > > > > > DRRS, and IPS
> > > > > > > under /sys/class/drm/card0/power/.  By enabling these
> > > > > > > features
> > > > > > > I'm
> > > > > > > looking to
> > > > > > > empower our customers, such as, power team, chrome OS,
> > > > > > > and
> > > > > > > platform
> > > > > > > integration teams
> > > > > > > to debug graphics power management features.
> > > > > > > 
> > > > > > > From the current patchset PSR, FBC, RC6, and, DRRS are
> > > > > > > complete
> > > > > > > and
> > > > > > > past BAT, modset,
> > > > > > > and suspend/resume tests.  For IPS I'm looking for
> > > > > > > feedback
> > > > > > > since
> > > > > > > IPS
> > > > > > > seems to change
> > > > > > > during runtime dynamically.  Additionally, as a future
> > > > > > > project
> > > > > > > IPS
> > > > > > > would be better
> > > > > > > served if it is implemented by using the atomic check,
> > > > > > > pre
> > > > > > > -commit,
> > > > > > > commit, post-commit,
> > > > > > > a good example of this is the PSR enable/disable
> > > > > > > implementation.
> > > > > > > 
> > > > > > > For this project to be completed needs to have in place
> > > > > > > the
> > > > > > > following
> > > > > > > components:
> > > > > > > (Need to be developed)
> > > > > > > * IPS toggle interface flesh-out
> > > > > > > * Tests added to intel-gpu-tools repo
> > > > > > > * Documentation for all sysfs added interfaces
> > > > > > > * PowerTOP component named: GFX-TOP. With the following
> > > > > > > requirements:
> > > > > > >   * It would be available only to developers
> > > > > > >   * It would allow the developers to toggle the
> > > > > > > interfaces
> > > > > > > from
> > > > > > >     the PowerTOP user interface.
> > > > > > >   * It would show the Power Consumption impact of
> > > > > > > toggling on
> > > > > > > and
> > > > > > > off
> > > > > > >     these settings.
> > > > > > > 
> > > > > > 
> > > > > > A small suggestion:
> > > > > > 
> > > > > > In the past, I've seen people testing i915.ko power saving
> > > > > > features
> > > > > > just by "toggling" the features through the i915 module
> > > > > > parameters
> > > > > > without doing anything else. In most of these cases, the
> > > > > > machine
> > > > > > was
> > > > > > not properly configured for power savings, so the
> > > > > > conclusion
> > > > > > was
> > > > > > often
> > > > > > that the feature didn't save any power. While this was true
> > > > > > (toggling
> > > > > > the feature didn't save any power), these people would have
> > > > > > reached
> > > > > > different conclusions if they had, for example, changed
> > > > > > their
> > > > > > disk
> > > > > > power management policies. Do we have any sort of plan to
> > > > > > try
> > > > > > to
> > > > > > educate the powertop/gfxtop users regarding these things?
> > > > 
> > > > But the toggling the module parameter doesn't change anything
> > > > because
> > > > it depends on the next modeset to have any effect o nmnost of
> > > > pm
> > > > features.
> > > > The idea is that we don't need this with the sysfs and the
> > > > toggle
> > > > should represent an immediate change on the feature behaviour.
> > > > Although
> > > > it is a fact that the features also depends on other conditions
> > > > like
> > > > PSR depends on a fully idle screen for few frames at least.
> > > 
> > > Well that part where the knob needs to kick the system into
> > > working
> > > correctly (what if we need an expensive w/a that's hard to reset
> > > at
> > > runtime) is what scares me.
> > > 
> > > > > > Maybe the powertop/gfxtop interface could expose some very
> > > > > > -easy
> > > > > > -to-
> > > > > > reach text explaining these things.
> > > > 
> > > > Good idea.
> > > > 
> > > > > 
> > > > > I think trying to extract/reuse those from the igt testcases
> > > > > might be
> > > > > really useful. I.e. add docs that explain
> > > > > a) what igt must past of a feature to be considered working
> > > > > b) add some functions to igt testcases for manually
> > > > > enabling/disabling a
> > > > > given rpm feature. The tests already know how to do that, and
> > > > > most
> > > > > have
> > > > > nice explanations about what all should be changed on top to
> > > > > make
> > > > > things
> > > > > work.
> > > > 
> > > > Actually I had the opposite direction in mind. I was expecting
> > > > we
> > > > could
> > > > make igt tests to start using this sysfs toggles.
> > > > The problem I see with current igt tests way is that we need to
> > > > change
> > > > the parameter and depend on the next full modeset happening.
> > > > Well
> > > > it is
> > > > true that we need to have the modeset on the tests anyway, but
> > > > I
> > > > believe we could reduce that restriction and also powertop
> > > > wouldn't
> > > > do
> > > > any modeset.
> > > 
> > > So taking more steps backwards: The goal here is to help debug
> > > runtime
> > > pm issues. The proposed solutions is that you can toggel stuff in
> > > powertop.
> > > 
> > > Is this really how the experts debug runtime pm issues?
> > 
> > I've seen many experts (non-gfx devs) around using the powertop for
> > debuging PM related issues and checking PC states, etc.
> > 
> > One very good case to have at least the informative tab there is
> > exactly help this use case: One developer in the power lab cannot
> > see
> > deep PC state residencies with screen on and look to the gfx tab
> > and
> > see if DMC is enabled or not and PSR is enabled or not. And the
> > extra
> > is if is not enabled but possible it would just toggle directly
> > from
> > that interface.
> > 
> > Or at least the informative with everything together they would
> > know if
> > i915 is somehow the culprit for blocking PC states without need to
> > hunt
> > different debugfs interfaces they are not familiar with.
> 
> I'm fully on board with you on the "figure out that i915 is the
> culprit".
> I don't get why we need to be able to toggle stuff from within
> powertop to
> do that. Information-only should be enough. Maybe even some of the
> hints
> Paulo suggested (like "you haven't loaded DMC, pls install").
> 
> Adding toggles into powertop - I have no idea how that helps with
> debugging, beyond the modparams we have already. This is also because
> I'm
> strongly opposed as "powertop the solution to enable rpm stuff by
> default", and that's seems to be the reason a lot of folks want these
> knobs.

I was talking to PM engineers today on their Summit and they are really
interrested in a way to toggle features on/off easily.

They also told if debugfs is the only way that powertop should have to
deal with it.

So I see two ways here:

1. Stable informative only sysfs interface with powertop always
displaying it. No toggles

2. Stable informative only sysfs interface with powertop always displaying it. Toggle on debugfs only not implemented on powertop.

3. Powertop showing status via debugfs, but not toggling it. Plus Toggle on debugfs only not implemented on powertop.

4. Powertop informative and toggle using debugfs. Probably forcing
powertop to create a flag --debug-only with a warning that it is
unsafe.

What do you think: 1, or 2, or 3, or 4, or none?

> 
> Imo if something doesn't work that should, the next step is to file a
> bug
> report with the i915 experts to fix this. Not knobs and hacks in
> powertop
> ...
> 
> /me fighting a bit a holy war
> 
> > > At least when I hack around my first questions are:
> > > - do we get residenency/how much, and maybe what exactly is
> > > blocking
> > > it. I think we discussed this at the meeting and agreed that
> > > more/officially exposed residency counters.
> > 
> > The problem is that we don't have this counters when power wells
> > are
> > down. So we lost all DC counters and PSR counter when DC entry.
> > So no residencies. But I still like the idea of the status.
> 
> And I guess no way to at least somewhat fake those in software?
> 
> > > - once I have a suspect I keep digging into that direction, using
> > > debugfs/unsafe module options, debugfs information, whatever.
> > > 
> > > I fully agree that we can try to do a better job on documenting
> > > this
> > > stuff, and maybe exposing more information through stable
> > > interfaces.
> > > 
> > > But adding new abi to enable/disable stuff sounds like jumping
> > > into
> > > the middle of the use-case story, ignoring everything else. And I
> > > think that was roughly what we agreed at that meeting - we can
> > > add
> > > knobs when it makes sense, but only when it makes sense. I'm
> > > still
> > > not
> > > sold on the "debug by powertop" solution.
> > 
> > what about at least informative for now?
> 
> Useful information sounds good. I'm freaking out about the
> maintenance of
> adding knobs and stuff that's supposed to work.
>  
> > Watermark is my bad. I forgot to include it and also to toggle it
> > we
> > would need to add support for disabling it what we don't have at
> > all
> > but I believe we should have anyways.
> 
> Watermarks was a bit a trick example, since imo adding code to
> fake-disable low-power watermark levels is nonsense. It's only going
> to
> help with very specific bugs, and if you don't understand the wm
> programming model for your platforms it's going to be totally useless
> information. Misconfiguring the driver to make it crap doesn't really
> tell
> you a lot ...
> -Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx