[igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

Rodrigo Vivi rodrigo.vivi at gmail.com
Thu Jan 21 19:55:56 UTC 2021


On Thu, Dec 10, 2020 at 2:42 AM Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> Quoting Anshuman Gupta (2020-12-10 05:02:48)
> > On 2020-12-09 at 16:47:01 +0000, Chris Wilson wrote:
> > > Quoting Anshuman Gupta (2020-12-09 16:25:02)
> > > > On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > > > > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > > > > Create a separate igt test group and move package C
> > > > > > state in to this subgroup.
> > > > > > Run powertop --auto-tune to tune SOC power configuration
> > > > > > for package C state tests.
> > > > > >
> > > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > > > > > ---
> > > > > >  tests/i915/i915_pm_rpm.c | 35
> +++++++++++++++++++++++++++++++----
> > > > > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > > > > index af55b569..42bc44d9 100644
> > > > > > --- a/tests/i915/i915_pm_rpm.c
> > > > > > +++ b/tests/i915/i915_pm_rpm.c
> > > > > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > > > > >         /* XXX Also we can test wake up via exec nop */
> > > > > >  }
> > > > > >
> > > > > > +static bool setup_powertop(void)
> > > > > > +{
> > > > > > +       FILE *fp;
> > > > > > +       char tmp[512];
> > > > > > +
> > > > > > +       fp = popen("powertop --auto-tune", "r");
> > > > >
> > > > > Doesn't this defeat the point of having it work out of the box?
> > May be misunderstood your comment, is it PC state or powertop should work
> > out of box ?
>
> Powermanagement should not require the user to configure it before it
> can work.
>
> powertop in particular may tweak the gfx, which is verboten.
>
> Manually perform any configuration you think is required, and warn if the
> initial configuration does not support reaching the lowest pc state the
> system can. File bugs.
>

I understand why Anshuman did this. There are many many components
out there that are blocking the PC10 and this powertop --auto-tune
is trying to take care of all the corner cases to get the "best" of all the
components so we can try to garantee that we are not getting blocked by
other components.

However this is very very dangerous. It toggles a lot of unsafe and
untested cases.

I have lost a laptop right after a powertop --auto-tune and have never used
that
in my life again. Coincidence? Maybe... but I don't want to take the risk
in my
machine and I definitely cannot recommend its use.


> -Chris
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20210121/28eda9c8/attachment.htm>


More information about the igt-dev mailing list