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 919F410E9AF for ; Fri, 27 Jan 2023 16:55:51 +0000 (UTC) Date: Fri, 27 Jan 2023 08:55:48 -0800 Message-ID: <87lelngbmz.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Kamil Konieczny , igt-dev@lists.freedesktop.org, Riana Tauro , Ashutosh Dixit In-Reply-To: <20230127164603.ckc3cn2p2ecmgcua@kamilkon-desk1> References: <20230127160813.139460-1-ashutosh.dixit@intel.com> <20230127160813.139460-2-ashutosh.dixit@intel.com> <20230127164603.ckc3cn2p2ecmgcua@kamilkon-desk1> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [igt-dev] [PATCH i-g-t 1/2] i915/i915_power: Sanity check power measurement List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Fri, 27 Jan 2023 08:46:03 -0800, Kamil Konieczny wrote: > > Hi Ashutosh, > > looks much better, few small nits left, see below. > > On 2023-01-27 at 08:08:12 -0800, Ashutosh Dixit wrote: > > Add a test to sanity check power measurement on integrated/discrete by > > requiring power under load to be > 0 and >=3D idle power. > > > > v2: Code reorganization (Kamil K) > > v3: More code reorganization (Kamil K) > > > > Signed-off-by: Ashutosh Dixit > > Reviewed-by: Riana Tauro > > --- > > tests/i915/i915_power.c | 70 +++++++++++++++++++++++++++++++++++++++++ > > tests/meson.build | 1 + > > 2 files changed, 71 insertions(+) > > create mode 100644 tests/i915/i915_power.c > > > > diff --git a/tests/i915/i915_power.c b/tests/i915/i915_power.c > > new file mode 100644 > > index 00000000000..533cf769b54 > > --- /dev/null > > +++ b/tests/i915/i915_power.c > > @@ -0,0 +1,70 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright =A9 2023 Intel Corporation > > + */ > > + > > +#include "igt.h" > > +#include "i915/gem.h" > > +#include "igt_power.h" > > + > > +IGT_TEST_DESCRIPTION("i915 power measurement tests"); > > + > > +static double measure_power(struct igt_power *pwr, int sleep_duration_= sec) > ----------------------------------------------------- ^ > unsigned int No this is double, see igt_power_get_mW. > > > +{ > > + struct power_sample sample[2]; > > + > > + igt_power_get_energy(pwr, &sample[0]); > > + usleep(sleep_duration_sec * USEC_PER_SEC); > > + igt_power_get_energy(pwr, &sample[1]); > > + > > + return igt_power_get_mW(pwr, &sample[0], &sample[1]); > > +} > > + > > +static void sanity(int i915) > > +{ > > + const intel_ctx_t *ctx; > > + struct igt_power pwr; > > + double idle, busy; > > + igt_spin_t *spin; > > + uint64_t ahnd; > > + > > + /* Idle power */ > > + igt_require(!igt_power_open(i915, &pwr, "gpu")); > > + gem_quiescent_gpu(i915); > > + idle =3D measure_power(&pwr, 2); > ---------------------------------- ^ > > > + > > + /* Busy power */ > > + ctx =3D intel_ctx_create_all_physical(i915); > > + ahnd =3D get_reloc_ahnd(i915, ctx->id); > > + spin =3D igt_spin_new(i915, .ahnd =3D ahnd, .ctx =3D ctx, > > + .engine =3D ALL_ENGINES, > > + .flags =3D IGT_SPIN_POLL_RUN); > > + /* Wait till at least one spinner starts */ > > + igt_spin_busywait_until_started(spin); > > + busy =3D measure_power(&pwr, 2); > ---------------------------------- ^ > Make it define or const as you are using it twice and this > should be the same. Will fix this up. > > With that fixed: > Reviewed-by: Kamil Konieczny Thanks! > > -- > Kamil > > > + igt_free_spins(i915); > > + put_ahnd(ahnd); > > + intel_ctx_destroy(i915, ctx); > > + igt_power_close(&pwr); > > + > > + igt_info("Measured power: idle: %g mW, busy: %g mW\n", idle, busy); > > + igt_assert(idle >=3D 0 && busy > 0 && busy > idle); > > +} > > + > > +igt_main > > +{ > > + int i915; > > + > > + igt_fixture { > > + i915 =3D drm_open_driver_master(DRIVER_INTEL); > > + } > > + > > + igt_describe("Sanity check gpu power measurement"); > > + igt_subtest("sanity") { > > + sanity(i915); > > + } > > + > > + igt_fixture { > > + close(i915); > > + } > > +} > > diff --git a/tests/meson.build b/tests/meson.build > > index cce9d89e1c8..d93a07c9ed6 100644 > > --- a/tests/meson.build > > +++ b/tests/meson.build > > @@ -213,6 +213,7 @@ i915_progs =3D [ > > 'i915_pm_dc', > > 'i915_pm_rps', > > 'i915_pm_sseu', > > + 'i915_power', > > 'i915_query', > > 'i915_selftest', > > 'i915_suspend', > > -- > > 2.38.0 > >