From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id B97EF10E413 for ; Wed, 22 Mar 2023 23:09:14 +0000 (UTC) Date: Wed, 22 Mar 2023 16:08:55 -0700 Message-ID: <87ttyctmd4.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Kamil Konieczny , igt-dev@lists.freedesktop.org, Riana Tauro , Zbigniew Kempczynski In-Reply-To: <20230322170915.fgetw55xiab27msp@kamilkon-desk1> References: <20230321011800.3025307-1-ashutosh.dixit@intel.com> <20230322170915.fgetw55xiab27msp@kamilkon-desk1> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH i-g-t] i915/i915_power: Also show requested and actual freq's List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: 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 > > > --- > > > 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. Thanks. -- Ashutosh