All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ashutosh Dixit <ashutosh.dixit@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	dri-devel@lists.freedesktop.org
Subject: [PATCH] drm/i915/pmu: Match frequencies reported by PMU and sysfs
Date: Mon,  3 Oct 2022 12:24:19 -0700	[thread overview]
Message-ID: <20221003192419.3541088-1-ashutosh.dixit@intel.com> (raw)

PMU and sysfs use different wakeref's to "interpret" zero freq. Sysfs uses
runtime PM wakeref (see intel_rps_read_punit_req and
intel_rps_read_actual_frequency). PMU uses the GT parked/unparked
wakeref. In general the GT wakeref is held for less time that the runtime
PM wakeref which causes PMU to report a lower average freq than the average
freq obtained from sampling sysfs.

To resolve this, use the same freq functions (and wakeref's) in PMU as
those used in sysfs.

Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7025
Reported-by: Ashwin Kumar Kulkarni <ashwin.kumar.kulkarni@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..eda03f264792 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -371,37 +371,16 @@ static void
 frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 {
 	struct drm_i915_private *i915 = gt->i915;
-	struct intel_uncore *uncore = gt->uncore;
 	struct i915_pmu *pmu = &i915->pmu;
 	struct intel_rps *rps = &gt->rps;
 
 	if (!frequency_sampling_enabled(pmu))
 		return;
 
-	/* Report 0/0 (actual/requested) frequency while parked. */
-	if (!intel_gt_pm_get_if_awake(gt))
-		return;
-
 	if (pmu->enable & config_mask(I915_PMU_ACTUAL_FREQUENCY)) {
-		u32 val;
-
-		/*
-		 * We take a quick peek here without using forcewake
-		 * so that we don't perturb the system under observation
-		 * (forcewake => !rc6 => increased power use). We expect
-		 * that if the read fails because it is outside of the
-		 * mmio power well, then it will return 0 -- in which
-		 * case we assume the system is running at the intended
-		 * frequency. Fortunately, the read should rarely fail!
-		 */
-		val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
-		if (val)
-			val = intel_rps_get_cagf(rps, val);
-		else
-			val = rps->cur_freq;
-
 		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
-				intel_gpu_freq(rps, val), period_ns / 1000);
+				intel_rps_read_actual_frequency(rps),
+				period_ns / 1000);
 	}
 
 	if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) {
@@ -409,8 +388,6 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 				intel_rps_get_requested_frequency(rps),
 				period_ns / 1000);
 	}
-
-	intel_gt_pm_put_async(gt);
 }
 
 static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
-- 
2.34.1


WARNING: multiple messages have this Message-ID (diff)
From: Ashutosh Dixit <ashutosh.dixit@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH] drm/i915/pmu: Match frequencies reported by PMU and sysfs
Date: Mon,  3 Oct 2022 12:24:19 -0700	[thread overview]
Message-ID: <20221003192419.3541088-1-ashutosh.dixit@intel.com> (raw)

PMU and sysfs use different wakeref's to "interpret" zero freq. Sysfs uses
runtime PM wakeref (see intel_rps_read_punit_req and
intel_rps_read_actual_frequency). PMU uses the GT parked/unparked
wakeref. In general the GT wakeref is held for less time that the runtime
PM wakeref which causes PMU to report a lower average freq than the average
freq obtained from sampling sysfs.

To resolve this, use the same freq functions (and wakeref's) in PMU as
those used in sysfs.

Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7025
Reported-by: Ashwin Kumar Kulkarni <ashwin.kumar.kulkarni@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..eda03f264792 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -371,37 +371,16 @@ static void
 frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 {
 	struct drm_i915_private *i915 = gt->i915;
-	struct intel_uncore *uncore = gt->uncore;
 	struct i915_pmu *pmu = &i915->pmu;
 	struct intel_rps *rps = &gt->rps;
 
 	if (!frequency_sampling_enabled(pmu))
 		return;
 
-	/* Report 0/0 (actual/requested) frequency while parked. */
-	if (!intel_gt_pm_get_if_awake(gt))
-		return;
-
 	if (pmu->enable & config_mask(I915_PMU_ACTUAL_FREQUENCY)) {
-		u32 val;
-
-		/*
-		 * We take a quick peek here without using forcewake
-		 * so that we don't perturb the system under observation
-		 * (forcewake => !rc6 => increased power use). We expect
-		 * that if the read fails because it is outside of the
-		 * mmio power well, then it will return 0 -- in which
-		 * case we assume the system is running at the intended
-		 * frequency. Fortunately, the read should rarely fail!
-		 */
-		val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
-		if (val)
-			val = intel_rps_get_cagf(rps, val);
-		else
-			val = rps->cur_freq;
-
 		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
-				intel_gpu_freq(rps, val), period_ns / 1000);
+				intel_rps_read_actual_frequency(rps),
+				period_ns / 1000);
 	}
 
 	if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) {
@@ -409,8 +388,6 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 				intel_rps_get_requested_frequency(rps),
 				period_ns / 1000);
 	}
-
-	intel_gt_pm_put_async(gt);
 }
 
 static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
-- 
2.34.1


             reply	other threads:[~2022-10-03 19:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-03 19:24 Ashutosh Dixit [this message]
2022-10-03 19:24 ` [Intel-gfx] [PATCH] drm/i915/pmu: Match frequencies reported by PMU and sysfs Ashutosh Dixit
2022-10-03 23:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-10-04  6:20 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-10-04  9:29 ` [PATCH] " Tvrtko Ursulin
2022-10-04  9:29   ` [Intel-gfx] " Tvrtko Ursulin
2022-10-04 13:00   ` Tvrtko Ursulin
2022-10-04 13:00     ` [Intel-gfx] " Tvrtko Ursulin
2022-10-04 13:05     ` Tvrtko Ursulin
2022-10-04 13:05       ` [Intel-gfx] " Tvrtko Ursulin
2022-10-05  7:40     ` Dixit, Ashutosh
2022-10-05  7:40       ` [Intel-gfx] " Dixit, Ashutosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221003192419.3541088-1-ashutosh.dixit@intel.com \
    --to=ashutosh.dixit@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.