Re: [PATCH 2/3] drm/i915/pmu: add event_to_pmu() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 26/10/2023 11:22, Jani Nikula wrote:
On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote:
On Wed, Oct 25, 2023 at 11:20:25AM +0100, Tvrtko Ursulin wrote:

On 24/10/2023 13:42, Jani Nikula wrote:
On Tue, 24 Oct 2023, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote:
Hi Jani,

On Mon, Oct 23, 2023 at 06:02:55PM +0300, Jani Nikula wrote:
It's tedious to duplicate the container_of() everywhere. Add a helper.

Also do the logical steps of first getting from struct perf_event to
struct i915_pmu, and then from struct i915_pmu to struct
drm_i915_private if needed, instead of perf_event->i915->pmu. Not all
places even need the i915 pointer.

Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_pmu.c | 45 +++++++++++++++------------------
   1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index dcae2fcd8d36..d45b40ec6d47 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -31,6 +31,11 @@
   static cpumask_t i915_pmu_cpumask;
   static unsigned int i915_pmu_target_cpu = -1;
+static struct i915_pmu *event_to_pmu(struct perf_event *event)

I would call it perfevent (or perf_event), event is too generic.
We have other kind of events, too.

Fair enough.

Counter argument is that i915_pmu.c consistently names this event (which is
likely lifted from other PMU drivers) so is the proposal to churn it all, or
create an inconsistency?

The first that comes to my mind is that the debugger is also
using the term "event"... on the other hand there is no debugger
in i915.

Have you settled on this? I don't care either way, could apply either
patch.

To me it is clear that preference should be to remain consistent within the file, that is, leave it as you originally had.

Regards,

Tvrtko



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux