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

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

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_xqfHgCnhD1Te
> > > 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.

> > 
> > 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.

> -Daniel
> 
> > 
> > 
> > > Your review of this patchset is intended to contribute to its
> > > full
> > > maturity so that when we reach the PowerTOP development all
> > > pieces
> > > would
> > > be ready for commit.  
> > > 
> > > Thank you in advance,
> > > 
> > > Alexandra Yates (5):
> > >   drm/i915: Add sys PSR toggle interface
> > >   drm/i915: Add sys FBC toggle interface
> > >   drm/i915: Add sys RC6 toggle interface
> > >   drm/i915: Add sys drrs toggle interface
> > >   drm-i915: Add sys IPS toggle interface
> > > 
> > >  drivers/gpu/drm/i915/i915_debugfs.c  |   5 +-
> > >  drivers/gpu/drm/i915/i915_drv.c      |   5 +-
> > >  drivers/gpu/drm/i915/i915_drv.h      |  12 ++
> > >  drivers/gpu/drm/i915/i915_sysfs.c    | 362
> > > ++++++++++++++++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/intel_color.c   |  12 +-
> > >  drivers/gpu/drm/i915/intel_ddi.c     |  15 +-
> > >  drivers/gpu/drm/i915/intel_display.c |  55 ++++--
> > >  drivers/gpu/drm/i915/intel_dp.c      |  48 +++--
> > >  drivers/gpu/drm/i915/intel_drv.h     |  21 +-
> > >  drivers/gpu/drm/i915/intel_fbc.c     |  29 ++-
> > >  drivers/gpu/drm/i915/intel_pm.c      |  53 ++++-
> > >  drivers/gpu/drm/i915/intel_psr.c     |  36 +++-
> > >  12 files changed, 588 insertions(+), 65 deletions(-)
> > > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx