diff mbox series

perf: imx9_perf: introduce axi filter version

Message ID 20241120032109.1163146-1-xu.yang_2@nxp.com (mailing list archive)
State New
Headers show
Series perf: imx9_perf: introduce axi filter version | expand

Commit Message

Xu Yang Nov. 20, 2024, 3:21 a.m. UTC 
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>
---
 drivers/perf/fsl_imx9_ddr_perf.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Frank Li Nov. 20, 2024, 4:35 p.m. UTC | #1 
On Wed, Nov 20, 2024 at 11:21:09AM +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.

Introduce AXI filter version to refactor the driver and better extension.

>
> 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.

Such informaiton is not related with your this change. I suggest put it
to comment for DDR_PERF_AXI_FILTER_V1 and DDR_PERF_AXI_FILTER_V2.

>
> 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>
> ---
>  drivers/perf/fsl_imx9_ddr_perf.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> index 3c856d9a4e97..965345d78f7a 100644
> --- a/drivers/perf/fsl_imx9_ddr_perf.c
> +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> @@ -63,8 +63,12 @@
>
>  static DEFINE_IDA(ddr_ida);
>
> +#define DDR_PERF_AXI_FILTER_V1          0x1
> +#define DDR_PERF_AXI_FILTER_V2          0x2

V1 V2 is quite difficult to know the difference. Please add comments to
show the more detail about V1 and V2, you can resue from commit message.

> +
>  struct imx_ddr_devtype_data {
>  	const char *identifier;		/* system PMU identifier for userspace */
> +	unsigned int type;		/* AXI filter version */

variable name is better match function
	"filter_ver"

>  };
>
>  struct ddr_pmu {
> @@ -83,24 +87,27 @@ struct ddr_pmu {
>
>  static const struct imx_ddr_devtype_data imx91_devtype_data = {
>  	.identifier = "imx91",
> +	.type = DDR_PERF_AXI_FILTER_V1
>  };
>
>  static const struct imx_ddr_devtype_data imx93_devtype_data = {
>  	.identifier = "imx93",
> +	.type = DDR_PERF_AXI_FILTER_V1
>  };
>
>  static const struct imx_ddr_devtype_data imx95_devtype_data = {
>  	.identifier = "imx95",
> +	.type = 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->type == 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->type == DDR_PERF_AXI_FILTER_V2;
>  }
>
>  static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
> @@ -155,7 +162,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 +314,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->type != ddr_pmu->devtype_data->type))
>  		return 0;
>
>  	return attr->mode;
> @@ -624,11 +632,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
>
Xu Yang Nov. 21, 2024, 1:07 p.m. UTC | #2 
On Wed, Nov 20, 2024 at 11:35:10AM -0500, Frank Li wrote:
> On Wed, Nov 20, 2024 at 11:21:09AM +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.
> 
> Introduce AXI filter version to refactor the driver and better extension.

Okay.

> 
> >
> > 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.
> 
> Such informaiton is not related with your this change. I suggest put it
> to comment for DDR_PERF_AXI_FILTER_V1 and DDR_PERF_AXI_FILTER_V2.

Okay.

> 
> >
> > 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>
> > ---
> >  drivers/perf/fsl_imx9_ddr_perf.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> > index 3c856d9a4e97..965345d78f7a 100644
> > --- a/drivers/perf/fsl_imx9_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> > @@ -63,8 +63,12 @@
> >
> >  static DEFINE_IDA(ddr_ida);
> >
> > +#define DDR_PERF_AXI_FILTER_V1          0x1
> > +#define DDR_PERF_AXI_FILTER_V2          0x2
> 
> V1 V2 is quite difficult to know the difference. Please add comments to
> show the more detail about V1 and V2, you can resue from commit message.

Sure.

> 
> > +
> >  struct imx_ddr_devtype_data {
> >  	const char *identifier;		/* system PMU identifier for userspace */
> > +	unsigned int type;		/* AXI filter version */
> 
> variable name is better match function
> 	"filter_ver"

Okay.

Thanks,
Xu Yang

> 
> >  };
> >
> >  struct ddr_pmu {
> > @@ -83,24 +87,27 @@ struct ddr_pmu {
> >
> >  static const struct imx_ddr_devtype_data imx91_devtype_data = {
> >  	.identifier = "imx91",
> > +	.type = DDR_PERF_AXI_FILTER_V1
> >  };
> >
> >  static const struct imx_ddr_devtype_data imx93_devtype_data = {
> >  	.identifier = "imx93",
> > +	.type = DDR_PERF_AXI_FILTER_V1
> >  };
> >
> >  static const struct imx_ddr_devtype_data imx95_devtype_data = {
> >  	.identifier = "imx95",
> > +	.type = 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->type == 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->type == DDR_PERF_AXI_FILTER_V2;
> >  }
> >
> >  static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
> > @@ -155,7 +162,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 +314,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->type != ddr_pmu->devtype_data->type))
> >  		return 0;
> >
> >  	return attr->mode;
> > @@ -624,11 +632,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
> >
diff mbox series

Patch

diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
index 3c856d9a4e97..965345d78f7a 100644
--- a/drivers/perf/fsl_imx9_ddr_perf.c
+++ b/drivers/perf/fsl_imx9_ddr_perf.c
@@ -63,8 +63,12 @@ 
 
 static DEFINE_IDA(ddr_ida);
 
+#define DDR_PERF_AXI_FILTER_V1          0x1
+#define DDR_PERF_AXI_FILTER_V2          0x2
+
 struct imx_ddr_devtype_data {
 	const char *identifier;		/* system PMU identifier for userspace */
+	unsigned int type;		/* AXI filter version */
 };
 
 struct ddr_pmu {
@@ -83,24 +87,27 @@  struct ddr_pmu {
 
 static const struct imx_ddr_devtype_data imx91_devtype_data = {
 	.identifier = "imx91",
+	.type = DDR_PERF_AXI_FILTER_V1
 };
 
 static const struct imx_ddr_devtype_data imx93_devtype_data = {
 	.identifier = "imx93",
+	.type = DDR_PERF_AXI_FILTER_V1
 };
 
 static const struct imx_ddr_devtype_data imx95_devtype_data = {
 	.identifier = "imx95",
+	.type = 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->type == 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->type == DDR_PERF_AXI_FILTER_V2;
 }
 
 static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
@@ -155,7 +162,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 +314,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->type != ddr_pmu->devtype_data->type))
 		return 0;
 
 	return attr->mode;
@@ -624,11 +632,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);