On 15/09/2017 11:38, Chris Wilson wrote:
Quoting Ville Syrjälä (2017-09-15 11:34:03)
On Fri, Sep 15, 2017 at 11:07:21AM +0100, Tvrtko Ursulin wrote:
On 15/09/2017 09:51, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2017-09-15 07:56:00)
On 14/09/2017 21:36, Ville Syrjälä wrote:
On Mon, Sep 11, 2017 at 04:25:50PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Extract code from i915_energy_uJ (debugfs) so it can be used by
other callers in future patches.
v2: Rebase.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 17 +----------------
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 25 +++++++++++++++++++++++++
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6338018f655d..b3a4a66bf7c4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2780,26 +2780,11 @@ static int i915_sink_crc(struct seq_file *m, void *data)
static int i915_energy_uJ(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
- unsigned long long power;
- u32 units;
if (INTEL_GEN(dev_priv) < 6)
return -ENODEV;
- intel_runtime_pm_get(dev_priv);
-
- if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) {
- intel_runtime_pm_put(dev_priv);
- return -ENODEV;
- }
-
- units = (power & 0x1f00) >> 8;
- power = I915_READ(MCH_SECP_NRG_STTS);
- power = (1000000 * power) >> units; /* convert to uJ */
-
- intel_runtime_pm_put(dev_priv);
-
- seq_printf(m, "%llu", power);
+ seq_printf(m, "%llu", intel_energy_uJ(dev_priv));
Isn't this the same thing as the package energy you get from rapl? Can't
we just nuke this private implementation entirely and rely on whatever
rapl gives us?
If so I think we could leave it out of i915 PMU. I tried looking for
MCH_SECP_NRG_STTS in bspec but did not find it though? Is your bspec fu
perhaps better and you could have a look?
I had it there for the convenience of grabbing everything through the
one interface (which I still think has merit). I don't I ever compiled
in the rapl user interface...
You mean the RAPL PMU, PERF_EVENTS_INTEL_RAPL?
I suspect he means sysfs. IIRC this perf+rapl marriage is somewhat
recent (or at least I still remeber having to fix it when it was
introduced because it broke the sysfs interface).
I hadn't even heard of the perf interface. Yes, if it is available
through perf, that's just what we want.
Dropping it then. And I've checked in the meantime it is reporting
identical numbers to power/energy-gpu/ provided by the intel_rapl_perf
module.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
