| Message ID | 20231023150256.438331-2-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915/pmu: add pmu_to_i915() helper | expand |
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@intel.com> > --- > 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. > +{ > + return container_of(event->pmu, struct i915_pmu, base); > +} > + > static struct drm_i915_private *pmu_to_i915(struct i915_pmu *pmu) > { > return container_of(pmu, struct drm_i915_private, pmu); > @@ -510,8 +515,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer) > > static void i915_pmu_event_destroy(struct perf_event *event) > { > - struct drm_i915_private *i915 = > - container_of(event->pmu, typeof(*i915), pmu.base); > + struct i915_pmu *pmu = event_to_pmu(event); > + struct drm_i915_private *i915 = pmu_to_i915(pmu); perf_event_to_i915() ? Andi
On Tue, 24 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@intel.com> >> --- >> 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. > >> +{ >> + return container_of(event->pmu, struct i915_pmu, base); >> +} >> + >> static struct drm_i915_private *pmu_to_i915(struct i915_pmu *pmu) >> { >> return container_of(pmu, struct drm_i915_private, pmu); >> @@ -510,8 +515,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer) >> >> static void i915_pmu_event_destroy(struct perf_event *event) >> { >> - struct drm_i915_private *i915 = >> - container_of(event->pmu, typeof(*i915), pmu.base); >> + struct i915_pmu *pmu = event_to_pmu(event); >> + struct drm_i915_private *i915 = pmu_to_i915(pmu); > > perf_event_to_i915() ? Nah. Most places that need i915 also need pmu. Feels a bit much to add a helper to combine two helpers. Thanks for the review. BR, Jani. > > Andi
On 24/10/2023 13:42, Jani Nikula wrote: > On Tue, 24 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@intel.com> >>> --- >>> 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? Regards, Tvrtko > >> >>> +{ >>> + return container_of(event->pmu, struct i915_pmu, base); >>> +} >>> + >>> static struct drm_i915_private *pmu_to_i915(struct i915_pmu *pmu) >>> { >>> return container_of(pmu, struct drm_i915_private, pmu); >>> @@ -510,8 +515,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer) >>> >>> static void i915_pmu_event_destroy(struct perf_event *event) >>> { >>> - struct drm_i915_private *i915 = >>> - container_of(event->pmu, typeof(*i915), pmu.base); >>> + struct i915_pmu *pmu = event_to_pmu(event); >>> + struct drm_i915_private *i915 = pmu_to_i915(pmu); >> >> perf_event_to_i915() ? > > Nah. Most places that need i915 also need pmu. Feels a bit much to add a > helper to combine two helpers. > > Thanks for the review. > > BR, > Jani. > >> >> Andi >
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@linux.intel.com> 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@intel.com> > > > > --- > > > > 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. Andi
On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@linux.intel.com> 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@intel.com> >> > > > --- >> > > > 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. BR, Jani.
On 26/10/2023 11:22, Jani Nikula wrote: > On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@linux.intel.com> 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@intel.com> >>>>>> --- >>>>>> 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
Hi, > On 26/10/2023 11:22, Jani Nikula wrote: > > On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@linux.intel.com> 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@intel.com> > > > > > > > --- > > > > > > > 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
On 26/10/2023 11:36, Andi Shyti wrote: > Hi, > >> On 26/10/2023 11:22, Jani Nikula wrote: >>> On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@linux.intel.com> 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@intel.com> >>>>>>>> --- >>>>>>>> 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... :( $ grep "struct perf_event \*event" . -r | wc -l 1912 $ grep "struct perf_event \*perf_event" . -r | wc -l 5 ;) Now seriously, I don't mind perf_event, as long as _whole_ i915_pmu.c is switched over. At which point I questioned would the churn be worth it. Regards, Tvrtko >> 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
On Thu, Oct 26, 2023 at 11:51:02AM +0100, Tvrtko Ursulin wrote: > On 26/10/2023 11:36, Andi Shyti wrote: > > > On 26/10/2023 11:22, Jani Nikula wrote: > > > > On Wed, 25 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> 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@linux.intel.com> 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@intel.com> > > > > > > > > > --- > > > > > > > > > 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... > > :( > > $ grep "struct perf_event \*event" . -r | wc -l > 1912 > $ grep "struct perf_event \*perf_event" . -r | wc -l > 5 > > ;) with "I haven't settled on this", I meant that the debugger has not been posted upstream for i915 and it won't be. It's going to go in the XE driver. > Now seriously, I don't mind perf_event, as long as _whole_ i915_pmu.c is > switched over. At which point I questioned would the churn be worth it. I like Jani's patch, of course your grep search concludes the the discussion, so that I'm not going to argue agains "event" as name :-) Acked-by: Andi Shyti <andi.shyti@linux.intel.com> Andi
On Thu, 26 Oct 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote: > On Thu, Oct 26, 2023 at 11:51:02AM +0100, Tvrtko Ursulin wrote: >> Now seriously, I don't mind perf_event, as long as _whole_ i915_pmu.c is >> switched over. At which point I questioned would the churn be worth it. > > I like Jani's patch, of course your grep search concludes the > the discussion, so that I'm not going to argue agains "event" > as name :-) > > Acked-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks. Pushed with the original patch; if someone feels inclined to change the naming, patches are, as always, welcome. BR, Jani.
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) +{ + return container_of(event->pmu, struct i915_pmu, base); +} + static struct drm_i915_private *pmu_to_i915(struct i915_pmu *pmu) { return container_of(pmu, struct drm_i915_private, pmu); @@ -510,8 +515,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer) static void i915_pmu_event_destroy(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); drm_WARN_ON(&i915->drm, event->parent); @@ -577,8 +582,8 @@ config_status(struct drm_i915_private *i915, u64 config) static int engine_event_init(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); struct intel_engine_cs *engine; engine = intel_engine_lookup_user(i915, engine_event_class(event), @@ -591,9 +596,8 @@ static int engine_event_init(struct perf_event *event) static int i915_pmu_event_init(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); - struct i915_pmu *pmu = &i915->pmu; + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); int ret; if (pmu->closed) @@ -633,9 +637,8 @@ static int i915_pmu_event_init(struct perf_event *event) static u64 __i915_pmu_event_read(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); - struct i915_pmu *pmu = &i915->pmu; + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); u64 val = 0; if (is_engine_event(event)) { @@ -691,10 +694,8 @@ static u64 __i915_pmu_event_read(struct perf_event *event) static void i915_pmu_event_read(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = event_to_pmu(event); struct hw_perf_event *hwc = &event->hw; - struct i915_pmu *pmu = &i915->pmu; u64 prev, new; if (pmu->closed) { @@ -712,10 +713,9 @@ static void i915_pmu_event_read(struct perf_event *event) static void i915_pmu_enable(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); const unsigned int bit = event_bit(event); - struct i915_pmu *pmu = &i915->pmu; unsigned long flags; if (bit == -1) @@ -776,10 +776,9 @@ static void i915_pmu_enable(struct perf_event *event) static void i915_pmu_disable(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = event_to_pmu(event); + struct drm_i915_private *i915 = pmu_to_i915(pmu); const unsigned int bit = event_bit(event); - struct i915_pmu *pmu = &i915->pmu; unsigned long flags; if (bit == -1) @@ -823,9 +822,7 @@ static void i915_pmu_disable(struct perf_event *event) static void i915_pmu_event_start(struct perf_event *event, int flags) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); - struct i915_pmu *pmu = &i915->pmu; + struct i915_pmu *pmu = event_to_pmu(event); if (pmu->closed) return; @@ -844,9 +841,7 @@ static void i915_pmu_event_stop(struct perf_event *event, int flags) static int i915_pmu_event_add(struct perf_event *event, int flags) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); - struct i915_pmu *pmu = &i915->pmu; + struct i915_pmu *pmu = event_to_pmu(event); if (pmu->closed) return -ENODEV;
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@intel.com> --- drivers/gpu/drm/i915/i915_pmu.c | 45 +++++++++++++++------------------ 1 file changed, 20 insertions(+), 25 deletions(-)