Re: [igt-dev] [PATCH i-g-t] i915/i915_power: Also show requested and actual freq's

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 22 Mar 2023 16:08:55 -0700, Dixit, Ashutosh wrote:
>
> On Wed, 22 Mar 2023 10:09:15 -0700, Kamil Konieczny wrote:
> >
>
> Hi Riana/Kamil,
>
> >
> > On 2023-03-21 at 13:27:55 +0530, Riana Tauro wrote:
> > >
> > >
> > > On 3/21/2023 6:48 AM, Ashutosh Dixit wrote:
> > > > When power limits are in effect, in addition to measured power it is also
> > > > important to see the requested and actual freq's and see how they change
> > > > with set power limits. Add this to the test output.
> > > >
> > > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > > ---
> > > >   tests/i915/i915_power.c | 11 ++++++++++-
> > > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/i915/i915_power.c b/tests/i915/i915_power.c
> > > > index f2fd228698a..f0f10fc4d14 100644
> > > > --- a/tests/i915/i915_power.c
> > > > +++ b/tests/i915/i915_power.c
> > > > @@ -5,6 +5,7 @@
> > > >   #include "igt.h"
> > > >   #include "i915/gem.h"
> > > > +#include "igt_sysfs.h"
> >
> > Please keep it in alphabetical order.
> >
> > > >   #include "igt_power.h"
> > > >   IGT_TEST_DESCRIPTION("i915 power measurement tests");
> > > > @@ -27,9 +28,13 @@ static void sanity(int i915)
> > > >		double idle, busy;
> > > >		igt_spin_t *spin;
> > > >		uint64_t ahnd;
> > > > +	int dir, req, act;
> > > >   #define DURATION_SEC 2
> > > > +	dir = igt_sysfs_gt_open(i915, 0);
> > > > +	igt_assert_lt(0, dir);
> > > > +
> > > can be replaced with for_each_sysfs_gt_dirfd and making the test a dynamic
> > > subtest ?
> > >
> > > Thanks
> > > Riana Tauro
> > >
> >
> > I would prefer to keep it simple and have it as first step,
> > then make it dynamic subtest with macro.
>
> Because the i915_power library only exposes device level (not gt level)
> power IMO it doesn't make sense to have per gt subtests.
>
> But I agree that because we are scheduling spinners on all engines (and so
> all gt's) we should display power for all gt's. Specially when on platforms
> like MTL the render and media gt's have completely different operating
> freq's.
>
> So I've followed this line or reasoning and posted a v2. I have also
> include Zbigniew's patch which contains i915_for_each_gt macro there.

Also, I forgot to mention, I am using this IGT (as mentioned in the commit
description) to basically experiment with the PL1 power limit. Set a PL1
limit and see what the measured power and freq's are in response to that
PL1 limit.

>
> Thanks.
> --
> Ashutosh