Hi, > 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. no... unfortunately not... > To me it is clear that preference should be to remain consistent within the > file, that is, leave it as you originally had. ... so I'm not going to be strong on this... please feel free to ignore my comment, then. Thanks! Andi
