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

[Frank Li <Frank.li@nxp.com>]

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>

Reviewed-by: Frank Li <Frank.Li@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 const struct imx_ddr_devtype_data imx93_devtype_data = {
>  	.identifier = "imx93",
> +	.filter_ver = DDR_PERF_AXI_FILTER_V1
>  };
>
>  static const struct imx_ddr_devtype_data imx95_devtype_data = {
>  	.identifier = "imx95",
> +	.filter_ver = DDR_PERF_AXI_FILTER_V2
>  };
>
> -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;
>  }
>
> -static inline bool is_imx95(struct ddr_pmu *pmu)
> +static inline bool axi_filter_v2(struct ddr_pmu *pmu)
>  {
> -	return pmu->devtype_data == &imx95_devtype_data;
> +	return pmu->devtype_data->filter_ver == DDR_PERF_AXI_FILTER_V2;
>  }
>
>  static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
> @@ -155,7 +171,7 @@ static const struct attribute_group ddr_perf_cpumask_attr_group = {
>  struct imx9_pmu_events_attr {
>  	struct device_attribute attr;
>  	u64 id;
> -	const void *devtype_data;
> +	const struct imx_ddr_devtype_data *devtype_data;
>  };
>
>  static ssize_t ddr_pmu_event_show(struct device *dev,
> @@ -307,7 +323,8 @@ ddr_perf_events_attrs_is_visible(struct kobject *kobj,
>  	if (!eattr->devtype_data)
>  		return attr->mode;
>
> -	if (eattr->devtype_data != ddr_pmu->devtype_data)
> +	if ((eattr->devtype_data != ddr_pmu->devtype_data) &&
> +		(eattr->devtype_data->filter_ver != ddr_pmu->devtype_data->filter_ver))
>  		return 0;
>
>  	return attr->mode;
> @@ -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);
>
> -	if (is_imx95(pmu))
> +	if (axi_filter_v2(pmu))
>  		/* write beat, read beat2, read beat1, read beat */
>  		imx95_ddr_perf_monitor_config(pmu, event_id, counter, cfg1, cfg2);
>
> --
> 2.34.1
>