On 01/08/2019 15:54, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-08-01 15:17:32)
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)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
Is our GPU always "0000:00:02.0"? CI will tell me.
It always has been. (With a few additional 2.1 for Windows95 multihead
where each head had to be a unique device!)
One hopes that by now it is firmly ingrained that it will always be
kept to 00:02.0.
Re-assuring, thanks! However still some tests to do before I am happy
this is upstream worthy, not least CI.
---
drivers/gpu/drm/i915/i915_pmu.c | 27 +++++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_pmu.h | 4 ++++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e0e0180bca7c..9a404d85c4e9 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 pdev->bus->parent == NULL &&
pci_domain_nr(pdev->bus) == 0 ?
Aha, thanks!
+ pdev->bus->number == 0 &&
+ PCI_SLOT(pdev->devfn) == 2 &&
+ PCI_FUNC(pdev->devfn) == 0;
I am surprised there isn't already a convenience function. None that I
could find.
+}
+
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";
Makes sense, and quite a neat solution.
+ 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,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
err_unreg:
perf_pmu_unregister(&pmu->base);
+err_name:
+ if (!is_igp(i915->drm.pdev))
+ kfree(pmu->name);
kfree_const(pmu->name);
err:
pmu->base.event_init = NULL;
free_event_attributes(pmu);
@@ -1116,5 +1137,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
perf_pmu_unregister(&pmu->base);
pmu->base.event_init = NULL;
+ if (!is_igp(i915->drm.pdev))
+ kfree(pmu->name);
kfree_const(pmu->name);
Works for me, I wonder what PeterZ will say...
In what sense?
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Thanks,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
