Quoting Tvrtko Ursulin (2018-02-06 18:33:11) > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > We are not allowed to call intel_runtime_pm_get from the PMU counter read > callback since the former can sleep, and the latter is running under IRQ > context. > > To workaround this, we record the last known RC6 and while runtime > suspended estimate its increase by querying the runtime PM core > timestamps. > > Downside of this approach is that we can temporarily lose a chunk of RC6 > time, from the last PMU read-out to runtime suspend entry, but that will > eventually catch up, once device comes back online and in the presence of > PMU queries. > > Also, we have to be careful not to overshoot the RC6 estimate, so once > resumed after a period of approximation, we only update the counter once > it catches up. With the observation that RC6 is increasing while the > device is suspended, this should not pose a problem and can only cause > slight inaccuracies due clock base differences. > > v2: Simplify by estimating on top of PM core counters. (Imre) > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104943 > Fixes: 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics") > Testcase: igt/perf_pmu/rc6-runtime-pm > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Imre Deak <imre.deak@xxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > Cc: David Airlie <airlied@xxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > --- > drivers/gpu/drm/i915/i915_pmu.c | 93 ++++++++++++++++++++++++++++++++++------- > drivers/gpu/drm/i915/i915_pmu.h | 6 +++ > 2 files changed, 84 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 1c440460255d..bfc402d47609 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -415,7 +415,81 @@ static int i915_pmu_event_init(struct perf_event *event) > return 0; > } > > -static u64 __i915_pmu_event_read(struct perf_event *event) > +static u64 get_rc6(struct drm_i915_private *i915, bool locked) > +{ > + unsigned long flags; > + u64 val; > + > + if (intel_runtime_pm_get_if_in_use(i915)) { > + val = intel_rc6_residency_ns(i915, IS_VALLEYVIEW(i915) ? > + VLV_GT_RENDER_RC6 : > + GEN6_GT_GFX_RC6); > + > + if (HAS_RC6p(i915)) > + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6p); > + > + if (HAS_RC6pp(i915)) > + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6pp); > + > + intel_runtime_pm_put(i915); > + > + /* > + * If we are coming back from being runtime suspended we must > + * be careful not to report a larger value than returned > + * previously. > + */ > + > + if (!locked) > + spin_lock_irqsave(&i915->pmu.lock, flags); > + > + if (val >= i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) { > + i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = 0; > + i915->pmu.sample[__I915_SAMPLE_RC6].cur = val; > + } else { > + val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur; > + } > + > + if (!locked) > + spin_unlock_irqrestore(&i915->pmu.lock, flags); > + } else { > + struct pci_dev *pdev = i915->drm.pdev; > + struct device *kdev = &pdev->dev; > + unsigned long flags2; > + > + /* > + * We are runtime suspended. > + * > + * Report the delta from when the device was suspended to now, > + * on top of the last known real value, as the approximated RC6 > + * counter value. > + */ > + if (!locked) > + spin_lock_irqsave(&i915->pmu.lock, flags); > + > + spin_lock_irqsave(&kdev->power.lock, flags2); > + > + if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) > + i915->pmu.suspended_jiffies_last = > + kdev->power.suspended_jiffies; > + > + val = kdev->power.suspended_jiffies - > + i915->pmu.suspended_jiffies_last; > + val += jiffies - kdev->power.accounting_timestamp; > + > + spin_unlock_irqrestore(&kdev->power.lock, flags2); > + > + val = jiffies_to_nsecs(val); > + val += i915->pmu.sample[__I915_SAMPLE_RC6].cur; > + i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val; > + > + if (!locked) > + spin_unlock_irqrestore(&i915->pmu.lock, flags); > + } > + > + return val; > +} I feel slightly dirty, but the dance checks out. -Chris _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
