On 08/05/2023 18:52, Umesh Nerlige Ramappa wrote:
On Fri, May 05, 2023 at 05:58:11PM -0700, Umesh Nerlige Ramappa wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Given how the metrics are already exported, we also need to run sampling
over engines from all GTs.
Problem of GT frequencies is left for later.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_pmu.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c
b/drivers/gpu/drm/i915/i915_pmu.c
index 7ece883a7d95..67fa6cd77529 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -10,6 +10,7 @@
#include "gt/intel_engine_pm.h"
#include "gt/intel_engine_regs.h"
#include "gt/intel_engine_user.h"
+#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
#include "gt/intel_gt_regs.h"
#include "gt/intel_rc6.h"
@@ -414,8 +415,9 @@ static enum hrtimer_restart i915_sample(struct
hrtimer *hrtimer)
struct drm_i915_private *i915 =
container_of(hrtimer, struct drm_i915_private, pmu.timer);
struct i915_pmu *pmu = &i915->pmu;
- struct intel_gt *gt = to_gt(i915);
unsigned int period_ns;
+ struct intel_gt *gt;
+ unsigned int i;
ktime_t now;
if (!READ_ONCE(pmu->timer_enabled))
@@ -431,8 +433,13 @@ static enum hrtimer_restart i915_sample(struct
hrtimer *hrtimer)
* grabbing the forcewake. However the potential error from timer
call-
* back delay greatly dominates this so we keep it simple.
*/
- engines_sample(gt, period_ns);
- frequency_sample(gt, period_ns);
+
+ for_each_gt(gt, i915, i) {
+ engines_sample(gt, period_ns);
+
+ if (i == 0) /* FIXME */
+ frequency_sample(gt, period_ns);
If the current series is already handling the FIXME at a later patch, I
would just change this to a comment - /* Support gt0 for now */
With or without that, this is
Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
@Tvrtko, Note that I am only transporting the patches (unmodified) from
internal to upstream, so assuming I am still a valid reviewer. If not,
let me know.
I think that is okay.
More of a problem is when you make comments like the above "I would just
change" - the question then is are you expecting me to make that change?
;) I think it would be best if you handled such tweaks in the series. In
this particular patch it is probably not really required since it gets
overwritten later as you say. It's probably just a left-over untidiness
from "back in the day".
Regards,
Tvrtko
