Re: [PATCH v2] drm/i915/pmu: Only enumerate available counters in sysfs

Chris Wilson
2018-01-09T14:43:42Z

Quoting Tvrtko Ursulin (2018-01-09 12:20:05)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Switch over to dynamically creating device attributes, which are in turn
> used by the perf core to expose available counters in sysfs.
>
> This way we do not expose counters which are not avaiable on the current
> platform, and are so more consistent between what we reply to open
> attempts via the perf_event_open(2), and what is discoverable in sysfs.
>
> v2:
> * Simplify attribute pointer freeing loop.
> * Changed attr init from macro to function.
> * More common error unwind. (Chris Wilson)
> * Rename some locals. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> @@ -337,30 +372,10 @@ static int i915_pmu_event_init(struct perf_event *event)
> if (!cpumask_test_cpu(event->cpu, &i915_pmu_cpumask))
> return -EINVAL;
>
> - if (is_engine_event(event)) {
> + if (is_engine_event(event))
> ret = engine_event_init(event);
> - } else {
> - ret = 0;
> - switch (event->attr.config) {
> - case I915_PMU_ACTUAL_FREQUENCY:
> - if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> - /* Requires a mutex for sampling! */
> - ret = -ENODEV;
> - case I915_PMU_REQUESTED_FREQUENCY:
> - if (INTEL_GEN(i915) < 6)
> - ret = -ENODEV;
> - break;
> - case I915_PMU_INTERRUPTS:
> - break;
> - case I915_PMU_RC6_RESIDENCY:
> - if (!HAS_RC6(i915))
> - ret = -ENODEV;
> - break;
> - default:
> - ret = -ENOENT;
> - break;
> - }
> - }
> + else
> + ret = config_status(i915, event->attr.config);;

Found something! s/;;/;/

It was fairly readable, keeping the sysfs clean seems a reasonable
goal, and I couldn't find anything significant to complain about,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris