Re: [PATCH v2] perf: imx9_perf: Introduce AXI filter version to refactor the driver and better extension

Xu Yang
2024-12-10T02:06:26Z

Hi Will,

On Mon, Dec 09, 2024 at 03:44:20PM +0000, Will Deacon wrote:
> On Mon, Nov 25, 2024 at 06:43:38PM +0800, Xu Yang wrote:
> > The imx93 is the first supported DDR PMU that supports read transaction,
> > write transaction and read beats events which corresponding respecitively
> > to counter 2, 3 and 4.
> >
> > However, transaction-based AXI match has low accuracy when get total bits
> > compared to beats-based. And imx93 doesn't assign AXI_ID to each master.
> > So axi filter is not used widely on imx93. This could be regards as AXI
> > filter version 1.
> >
> > To improve the AXI filter capability, imx95 supports 1 read beats and 3
> > write beats event which corresponding respecitively to counter 2-5. imx95
> > also detailed AXI_ID allocation so that most of the master could be count
> > individually. This could be regards as AXI filter version 2.
> >
> > This will introduce AXI filter version to refactor the driver and support
> > better extension, such as coming imx943.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> > - modify subject
> > - add comments for AXI_FILTER version
> > - type -> filter_ver
> > ---
> > drivers/perf/fsl_imx9_ddr_perf.c | 33 ++++++++++++++++++++++++--------
> > 1 file changed, 25 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> > index 3c856d9a4e97..e2c2c674b6d2 100644
> > --- a/drivers/perf/fsl_imx9_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> > @@ -63,8 +63,21 @@
> >
> > static DEFINE_IDA(ddr_ida);
> >
> > +/*
> > + * V1 support 1 read transaction, 1 write transaction and 1 read beats
> > + * event which corresponding respecitively to counter 2, 3 and 4.
> > + */
> > +#define DDR_PERF_AXI_FILTER_V1 0x1
> > +
> > +/*
> > + * V2 support 1 read beats and 3 write beats events which corresponding
> > + * respecitively to counter 2-5.
> > + */
> > +#define DDR_PERF_AXI_FILTER_V2 0x2
> > +
> > struct imx_ddr_devtype_data {
> > const char *identifier; /* system PMU identifier for userspace */
> > + unsigned int filter_ver; /* AXI filter version */
> > };
> >
> > struct ddr_pmu {
> > @@ -83,24 +96,27 @@ struct ddr_pmu {
> >
> > static const struct imx_ddr_devtype_data imx91_devtype_data = {
> > .identifier = "imx91",
> > + .filter_ver = DDR_PERF_AXI_FILTER_V1
> > };
>
> [...]
>
> > -static inline bool is_imx93(struct ddr_pmu *pmu)
> > +static inline bool axi_filter_v1(struct ddr_pmu *pmu)
> > {
> > - return pmu->devtype_data == &imx93_devtype_data;
> > + return pmu->devtype_data->filter_ver == DDR_PERF_AXI_FILTER_V1;
> > }
>
> [...]
>
> > @@ -624,11 +641,11 @@ static int ddr_perf_event_add(struct perf_event *event, int flags)
> > hwc->idx = counter;
> > hwc->state |= PERF_HES_STOPPED;
> >
> > - if (is_imx93(pmu))
> > + if (axi_filter_v1(pmu))
> > /* read trans, write trans, read beat */
> > imx93_ddr_perf_monitor_config(pmu, event_id, counter, cfg1, cfg2);
>
> Hmm, doesn't this change mean we now enable this for imx91 as well? My
> reading of the commit message is that imx93 was the first chip which
> supports this.

Yes, it's enabled for imx91 too. In fact, imx91 is compatible with imx93.
They use same configuration for axi filter.

Thanks,
Xu Yang

>
> Will