Peter, Thomas,
If you could spare a moment for some brainstorming on the topic of
uncore PMU and multiple providers it would be appreciated.
So from i915 we export some metrics as uncore PMU, which shows up under
/sys/devices/i915. Shortsightedness or what, we did not realize that one
day we could have more than one i915 device in a system which now
creates a problem, or at least raises a question on naming.
The patch below works around this by appending the PCI device name to
additional instances of i915 when it registers with perf_pmu_register.
Question is if there is a better solution, or if not, whether you are
aware of any plans to extend the perf core to better support this? Are
there any other uncore PMU providers in an identical situation?
Regards,
Tvrtko
On 01/08/2019 16:54, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
With discrete graphics system can have both integrated and discrete GPU
handled by i915.
Currently we use a fixed name ("i915") when registering as the uncore PMU
provider which stops working in this case.
To fix this we add the PCI device name string to non-integrated devices
handled by us. Integrated devices keep the legacy name preserving
backward compatibility.
v2:
* Detect IGP and keep legacy name. (Michal)
* Use PCI device name as suffix. (Michal, Chris)
v3:
* Constify the name. (Chris)
* Use pci_domain_nr. (Chris)
v4:
* Fix kfree_const usage. (Chris)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_pmu.c | 25 +++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_pmu.h | 4 ++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e0e0180bca7c..e0fea227077e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
cpuhp_remove_multi_state(cpuhp_slot);
}
+static bool is_igp(struct pci_dev *pdev)
+{
+ /* IGP is 0000:00:02.0 */
+ return pci_domain_nr(pdev->bus) == 0 &&
+ pdev->bus->number == 0 &&
+ PCI_SLOT(pdev->devfn) == 2 &&
+ PCI_FUNC(pdev->devfn) == 0;
+}
+
void i915_pmu_register(struct drm_i915_private *i915)
{
struct i915_pmu *pmu = &i915->pmu;
@@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
pmu->timer.function = i915_sample;
- ret = perf_pmu_register(&pmu->base, "i915", -1);
- if (ret)
+ if (!is_igp(i915->drm.pdev))
+ pmu->name = kasprintf(GFP_KERNEL,
+ "i915-%s",
+ dev_name(i915->drm.dev));
+ else
+ pmu->name = "i915";
+ if (!pmu->name)
goto err;
+ ret = perf_pmu_register(&pmu->base, pmu->name, -1);
+ if (ret)
+ goto err_name;
+
ret = i915_pmu_register_cpuhp_state(pmu);
if (ret)
goto err_unreg;
@@ -1095,6 +1113,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
err_unreg:
perf_pmu_unregister(&pmu->base);
+err_name:
+ kfree_const(pmu->name);
err:
pmu->base.event_init = NULL;
free_event_attributes(pmu);
@@ -1116,5 +1136,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
perf_pmu_unregister(&pmu->base);
pmu->base.event_init = NULL;
+ kfree_const(pmu->name);
free_event_attributes(pmu);
}
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 4fc4f2478301..ff24f3bb0102 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -46,6 +46,10 @@ struct i915_pmu {
* @base: PMU base.
*/
struct pmu base;
+ /**
+ * @name: Name as registered with perf core.
+ */
+ const char *name;
/**
* @lock: Lock protecting enable mask and ref count handling.
*/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
