From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C3B58C4332F for ; Tue, 22 Nov 2022 02:40:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E020F10E360; Tue, 22 Nov 2022 02:40:19 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1496A10E360 for ; Tue, 22 Nov 2022 02:40:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669084815; x=1700620815; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=XaY7UPnVweKYI8twtPn2LyW6ERV5uNcRk96sQaJMtGw=; b=g5/1L6Q6J4jhqNhhQoGZqxdadyGCIFG9OqR5XIlkH4XHT7TvyccAe+g9 lez3iuof3JAp3XasEucKeqQRmgSFvfF3PnPBBnMG0sDVaql2EQYbdeezA OOX076d7Oy9MgAME0S+rGpDW0W4r+ZC35ZVwwqdJTQu90nociJbQnMzuv +i1EeLMjZ8hW3FsibBXVqsDE5tySaB+UhbjjB+Z0/m8Zb93Ys1OejoxcG 4uUW/E3ObwPkdc++Yz1c7y9+ANvZNohq0c+ox7aguUfZSiQkD9DQKceoi Tfi8Mjc5moxZws7FALw9ZrP1QnLhQrclNMiuj+hPoRWRkxwk0qqdTRLbq Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10538"; a="312413986" X-IronPort-AV: E=Sophos;i="5.96,182,1665471600"; d="scan'208";a="312413986" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Nov 2022 18:40:14 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10538"; a="815940389" X-IronPort-AV: E=Sophos;i="5.96,182,1665471600"; d="scan'208";a="815940389" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.81.52]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Nov 2022 18:40:13 -0800 Date: Mon, 21 Nov 2022 18:40:00 -0800 Message-ID: <87tu2r67b3.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Riana Tauro In-Reply-To: <20221121072946.1013463-4-riana.tauro@intel.com> References: <20221121072946.1013463-1-riana.tauro@intel.com> <20221121072946.1013463-4-riana.tauro@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Sun, 20 Nov 2022 23:29:46 -0800, Riana Tauro wrote: > > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c > index 15b84c428f66..845058ed83ed 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c > @@ -61,9 +61,9 @@ int live_rc6_manual(void *arg) > res[0] = rc6_residency(rc6); > > dt = ktime_get(); > - rc0_power = libpower_get_energy_uJ(); > + rc0_power = libpower_get_energy_uJ(gt->i915); > msleep(250); > - rc0_power = libpower_get_energy_uJ() - rc0_power; > + rc0_power = libpower_get_energy_uJ(gt->i915) - rc0_power; > dt = ktime_sub(ktime_get(), dt); > res[1] = rc6_residency(rc6); > if ((res[1] - res[0]) >> 10) { > @@ -89,9 +89,9 @@ int live_rc6_manual(void *arg) > res[0] = rc6_residency(rc6); > intel_uncore_forcewake_flush(rc6_to_uncore(rc6), FORCEWAKE_ALL); > dt = ktime_get(); > - rc6_power = libpower_get_energy_uJ(); > + rc6_power = libpower_get_energy_uJ(gt->i915); > msleep(100); > - rc6_power = libpower_get_energy_uJ() - rc6_power; > + rc6_power = libpower_get_energy_uJ(gt->i915) - rc6_power; So: * arg for live_rps_power and live_rc6_manual is gt * freq's are per gt * rc6 residency is per gt But the power/energy we are using is per device (gt->i915)? And we expect device level power to be low when only one gt might be in rc6? Shouldn't all these functions be per gt? Specially when we might have multiple gt's soon. Or if per gt functions don't make in all cases have both device and gt level functions? i915 hwmon provides both gt and device/package/card level energy but iGFX till now reads a MSR which does not need gt or i915. So per gt I think should work for now. > dt = ktime_sub(ktime_get(), dt); > res[1] = rc6_residency(rc6); > if (res[1] == res[0]) { > diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c > index b8b0b0c7617e..6732aa7d4792 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_rps.c > +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c > @@ -1090,38 +1090,38 @@ int live_rps_interrupt(void *arg) > return err; > } > > -static u64 __measure_power(int duration_ms) > +static u64 __measure_power(struct intel_gt *gt, int duration_ms) > { > u64 dE, dt; > > dt = ktime_get(); > - dE = libpower_get_energy_uJ(); > + dE = libpower_get_energy_uJ(gt->i915); > usleep_range(1000 * duration_ms, 2000 * duration_ms); > - dE = libpower_get_energy_uJ() - dE; > + dE = libpower_get_energy_uJ(gt->i915) - dE; > dt = ktime_get() - dt; > > return div64_u64(1000 * 1000 * dE, dt); > } > > -static u64 measure_power(struct intel_rps *rps, int *freq) > +static u64 measure_power(struct intel_gt *gt, int *freq) > { > u64 x[5]; > int i; > > for (i = 0; i < 5; i++) > - x[i] = __measure_power(5); > + x[i] = __measure_power(gt, 5); > > - *freq = (*freq + intel_rps_read_actual_frequency(rps)) / 2; > + *freq = (*freq + intel_rps_read_actual_frequency(>->rps)) / 2; > > /* A simple triangle filter for better result stability */ > sort(x, 5, sizeof(*x), cmp_u64, NULL); > return div_u64(x[1] + 2 * x[2] + x[3], 4); > } > > -static u64 measure_power_at(struct intel_rps *rps, int *freq) > +static u64 measure_power_at(struct intel_gt *gt, int *freq) > { > - *freq = rps_set_check(rps, *freq); > - return measure_power(rps, freq); > + *freq = rps_set_check(>->rps, *freq); Hmm looks like this whole live_rps_power stuff is only for host turbo, not slpc. Anyway that's a future patch. > + return measure_power(gt, freq); > } > > int live_rps_power(void *arg) > @@ -1187,10 +1187,10 @@ int live_rps_power(void *arg) > } > > max.freq = rps->max_freq; > - max.power = measure_power_at(rps, &max.freq); > + max.power = measure_power_at(gt, &max.freq); > > min.freq = rps->min_freq; > - min.power = measure_power_at(rps, &min.freq); > + min.power = measure_power_at(gt, &min.freq); > > igt_spinner_end(&spin); > st_engine_heartbeat_enable(engine); > diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c > index fc1cdda82ec6..c4b14f2b268c 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_slpc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c > @@ -78,7 +78,7 @@ static u64 measure_power_at_freq(struct intel_gt *gt, int *freq, u64 *power) > if (err) > return err; > *freq = intel_rps_read_actual_frequency(>->rps); > - *power = measure_power(>->rps, freq); > + *power = measure_power(gt, freq); > > return err; > } > diff --git a/drivers/gpu/drm/i915/selftests/libpower.c b/drivers/gpu/drm/i915/selftests/libpower.c > index c66e993c5f85..df37cba30353 100644 > --- a/drivers/gpu/drm/i915/selftests/libpower.c > +++ b/drivers/gpu/drm/i915/selftests/libpower.c > @@ -6,29 +6,30 @@ > #include > > #include "i915_drv.h" > +#include "i915_hwmon.h" > #include "libpower.h" > > -bool libpower_supported(const struct drm_i915_private *i915) > -{ > - /* Discrete cards require hwmon integration */ > - if (IS_DGFX(i915)) > - return false; > - > - return libpower_get_energy_uJ(); > -} > - > -u64 libpower_get_energy_uJ(void) > +u64 libpower_get_energy_uJ(struct drm_i915_private *i915) > { > unsigned long long power; > u32 units; > + long energy_uJ = 0; > > - if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) > - return 0; > + if (IS_DGFX(i915)) { > +#if IS_REACHABLE(CONFIG_HWMON) > + if (i915_hwmon_get_energy(i915, &energy_uJ)) > +#endif No IS_REACHABLE stuff here. Declare i915_hwmon_get_energy similar to other functions in i915_hwmon.h. Thanks. -- Ashutosh > + return 0; > + } else { > + if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) > + return 0; > > - units = (power & 0x1f00) >> 8; > + units = (power & 0x1f00) >> 8; > > - if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power)) > - return 0; > + if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power)) > + return 0; > > - return (1000000 * power) >> units; /* convert to uJ */ > + energy_uJ = (1000000 * power) >> units; /* convert to uJ */ > + } > + return energy_uJ; > } > diff --git a/drivers/gpu/drm/i915/selftests/libpower.h b/drivers/gpu/drm/i915/selftests/libpower.h > index 5352981eb946..03a44611f9e9 100644 > --- a/drivers/gpu/drm/i915/selftests/libpower.h > +++ b/drivers/gpu/drm/i915/selftests/libpower.h > @@ -10,8 +10,10 @@ > > struct drm_i915_private; > > -bool libpower_supported(const struct drm_i915_private *i915); > - > -u64 libpower_get_energy_uJ(void); > +u64 libpower_get_energy_uJ(struct drm_i915_private *i915); > > +static inline bool libpower_supported(struct drm_i915_private *i915) > +{ > + return libpower_get_energy_uJ(i915); > +} > #endif /* SELFTEST_LIBPOWER_H */ > -- > 2.25.1 >