Quoting Tvrtko Ursulin (2019-08-01 11:18:25)
> 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.
>
> Add an unique instance number in form of "i915-<instance>" to each
> registered PMU to support this. First instance keeps the old name to
> preserve compatibility with existing userspace.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
> One problem is how to tie cars and PMUs.
>
> Should we append something better than the opaque instance (which
> depends on probing order)? Like some bus slot id? But then we have to
> say goodbye to preserving the "legacy" name "i915".
>
> I couldn't find that perf supports anything for these scenarios.
>
> Or maybe we can add a new file (sysfs?), which would be correctly under
> the bus hierarchy, and would export the name of the PMU instance to
> use?
We definitely need to add some means of uniquely identifying i915 into
sysfs, and for us to link from bus/pci/devices/ It seems odd that this
is not a solved problem :| (Which means I've probably missed how it is
meant to be done.)
Storing a pmu-name inside sysfs seems reasonable, and postpones the
problem of persistent device names for i915 until later :)
(Fwiw, how about using i915-${pci_addr}, or even just ${pci_addr}? I
expect that perf will grow multiple lookup paths if more hotpluggable
devices start supporting perf eventss)
> ---
> drivers/gpu/drm/i915/i915_pmu.c | 22 ++++++++++++++++++++--
> drivers/gpu/drm/i915/i915_pmu.h | 8 ++++++++
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 7c48fcf0cccb..105246ae5160 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -1053,6 +1053,8 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
> cpuhp_remove_multi_state(cpuhp_slot);
> }
>
> +static atomic_t i915_pmu_instance = ATOMIC_INIT(0);
> +
> void i915_pmu_register(struct drm_i915_private *i915)
> {
> struct i915_pmu *pmu = &i915->pmu;
> @@ -1083,9 +1085,17 @@ 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);
> + pmu->instance = atomic_inc_return(&i915_pmu_instance) - 1;
> + if (pmu->instance)
> + pmu->name = kasprintf(GFP_KERNEL, "i915-%u", pmu->instance);
> + else
> + pmu->name = "i915";
> + if (!pmu->name)
> + goto err_instance;
> +
> + ret = perf_pmu_register(&pmu->base, pmu->name, -1);
Iirc, the name is basically only used to construct the perf sysfs, and
the tools extract the event names and tags from the sysfs. So I wonder
if we could add a symlink...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
