[PATCH V2 1/4] docs/perf: Add explanation for DDR_CAP_AXI_ID_FILTER_ENHANCED quirk

[PATCH V2 1/4] docs/perf: Add explanation for DDR_CAP_AXI_ID_FILTER_ENHANCED quirk

[Joakim Zhang]

Add explanation for DDR_CAP_AXI_ID_FILTER_ENHANCED quirk.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
ChangeLog:
V1->V2:
	* improve the context a bit.
---
 Documentation/admin-guide/perf/imx-ddr.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/admin-guide/perf/imx-ddr.rst b/Documentation/admin-guide/perf/imx-ddr.rst
index 517a205abad6..b971df3117ba 100644
--- a/Documentation/admin-guide/perf/imx-ddr.rst
+++ b/Documentation/admin-guide/perf/imx-ddr.rst
@@ -50,3 +50,12 @@ in the driver.
   axi_id to monitor a specific id, rather than having to specify axi_mask.
   e.g.::
         perf stat -a -e imx8_ddr0/axid-read,axi_id=0x12/ cmd, which will monitor ARID=0x12
+
+* With DDR_CAP_AXI_ID_FILTER_ENHANCED quirk.
+  This is the extension of DDR_CAP_AXI_ID_FILTER quirk which is only support getting
+  bursts from DDR transaction, i.e. only can get DDR read or write requests. You
+  need select both DDR_CAP_AXI_ID_FILTER and DDR_CAP_AXI_ID_FILTER_ENHANCED quirks
+  together if HW supports enhanced filter. Enhanced filter means that it can count
+  bursts and bytes from DDR transaction at the same time as it implement another
+  set of data counters. DDR PMU driver always return bytes for enhanced filter as
+  it is more meaningful for users.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 2/4] bindings: perf: imx-ddr: Add new compatible string

[Joakim Zhang]

Add new compatible string for i.MX8MPlus DDR PMU core.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
ChangeLog:
V1->V2:
	* new add in V2.
---
 Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
index d77e3f26f9e6..7822a806ea0a 100644
--- a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
+++ b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: should be one of:
 	"fsl,imx8-ddr-pmu"
 	"fsl,imx8m-ddr-pmu"
+	"fsl,imx8mp-ddr-pmu"
 
 - reg: physical address and size
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 3/4] perf/imx_ddr: Add enhanced AXI ID filter support

[Joakim Zhang]

With DDR_CAP_AXI_ID_FILTER quirk, indicating HW supports AXI ID filter
which only can get bursts from DDR transaction, i.e. DDR read/write
requests.

This patch add DDR_CAP_AXI_ID_ENHANCED_FILTER quirk, indicating HW
supports AXI ID filter which can get bursts and bytes from DDR
transaction at the same time. We hope PMU always return bytes in the
driver due to it is more meaningful for users.

DDR_CAP_AXI_ID_ENHANCED_FILTER is based on DDR_CAP_AXI_ID_FILTER and
extend it a bit. So need select both above two qiurks together when
HW supports enhanced AXI ID filter.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
Changelog:
V1->V2:
	* use ddr_perf_is_filtered() helper to simply the code.
	* improve the commit message.
---
 drivers/perf/fsl_imx8_ddr_perf.c | 55 ++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
index ce7345745b42..17c817d89222 100644
--- a/drivers/perf/fsl_imx8_ddr_perf.c
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -45,7 +45,8 @@
 static DEFINE_IDA(ddr_ida);
 
 /* DDR Perf hardware feature */
-#define DDR_CAP_AXI_ID_FILTER          0x1     /* support AXI ID filter */
+#define DDR_CAP_AXI_ID_FILTER			BIT(1)     /* support AXI ID filter */
+#define DDR_CAP_AXI_ID_FILTER_ENHANCED		BIT(2)     /* support enhanced AXI ID filter */
 
 struct fsl_ddr_devtype_data {
 	unsigned int quirks;    /* quirks needed for different DDR Perf core */
@@ -178,6 +179,26 @@ static const struct attribute_group *attr_groups[] = {
 	NULL,
 };
 
+static bool ddr_perf_is_filtered(struct perf_event *event)
+{
+	return event->attr.config == 0x41 || event->attr.config == 0x42;
+}
+
+static u32 ddr_perf_filter_val(struct perf_event *event)
+{
+	return event->attr.config1;
+}
+
+static bool ddr_perf_filters_compatible(struct perf_event *a,
+					struct perf_event *b)
+{
+	if (!ddr_perf_is_filtered(a))
+		return true;
+	if (!ddr_perf_is_filtered(b))
+		return true;
+	return ddr_perf_filter_val(a) == ddr_perf_filter_val(b);
+}
+
 static u32 ddr_perf_alloc_counter(struct ddr_pmu *pmu, int event)
 {
 	int i;
@@ -209,27 +230,19 @@ static void ddr_perf_free_counter(struct ddr_pmu *pmu, int counter)
 
 static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter)
 {
-	return readl_relaxed(pmu->base + COUNTER_READ + counter * 4);
-}
+	struct perf_event *event = pmu->events[counter];
 
-static bool ddr_perf_is_filtered(struct perf_event *event)
-{
-	return event->attr.config == 0x41 || event->attr.config == 0x42;
-}
-
-static u32 ddr_perf_filter_val(struct perf_event *event)
-{
-	return event->attr.config1;
-}
-
-static bool ddr_perf_filters_compatible(struct perf_event *a,
-					struct perf_event *b)
-{
-	if (!ddr_perf_is_filtered(a))
-		return true;
-	if (!ddr_perf_is_filtered(b))
-		return true;
-	return ddr_perf_filter_val(a) == ddr_perf_filter_val(b);
+	/*
+	 * return bytes instead of bursts from ddr transaction for
+	 * axid-read and axid-write event if PMU core supports enhanced
+	 * filter.
+	 */
+	if ((pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) &&
+	    (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
+	    ddr_perf_is_filtered(event)) {
+		return readl_relaxed(pmu->base + COUNTER_DPCR1 + counter * 4);
+	} else
+		return readl_relaxed(pmu->base + COUNTER_READ + counter * 4);
 }
 
 static int ddr_perf_event_init(struct perf_event *event)
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 4/4] perf/imx_ddr: Add driver for DDR PMU in i.MX8MPlus

[Joakim Zhang]

Add driver for DDR PMU in i.MX8MPlus.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
ChangeLog:
V1->V2:
	* new add in V2.
---
 drivers/perf/fsl_imx8_ddr_perf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
index 17c817d89222..2f7bf74a2eec 100644
--- a/drivers/perf/fsl_imx8_ddr_perf.c
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -58,9 +58,14 @@ static const struct fsl_ddr_devtype_data imx8m_devtype_data = {
 	.quirks = DDR_CAP_AXI_ID_FILTER,
 };
 
+static const struct fsl_ddr_devtype_data imx8mp_devtype_data = {
+	.quirks = DDR_CAP_AXI_ID_FILTER | DDR_CAP_AXI_ID_FILTER_ENHANCED,
+};
+
 static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
 	{ .compatible = "fsl,imx8-ddr-pmu", .data = &imx8_devtype_data},
 	{ .compatible = "fsl,imx8m-ddr-pmu", .data = &imx8m_devtype_data},
+	{ .compatible = "fsl,imx8mp-ddr-pmu", .data = &imx8mp_devtype_data},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx_ddr_pmu_dt_ids);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 3/4] perf/imx_ddr: Add enhanced AXI ID filter support

[Will Deacon]

On Tue, Oct 29, 2019 at 07:06:19AM +0000, Joakim Zhang wrote:
> With DDR_CAP_AXI_ID_FILTER quirk, indicating HW supports AXI ID filter
> which only can get bursts from DDR transaction, i.e. DDR read/write
> requests.
> 
> This patch add DDR_CAP_AXI_ID_ENHANCED_FILTER quirk, indicating HW
> supports AXI ID filter which can get bursts and bytes from DDR
> transaction at the same time. We hope PMU always return bytes in the
> driver due to it is more meaningful for users.
> 
> DDR_CAP_AXI_ID_ENHANCED_FILTER is based on DDR_CAP_AXI_ID_FILTER and
> extend it a bit. So need select both above two qiurks together when
> HW supports enhanced AXI ID filter.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> Changelog:
> V1->V2:
> 	* use ddr_perf_is_filtered() helper to simply the code.
> 	* improve the commit message.
> ---
>  drivers/perf/fsl_imx8_ddr_perf.c | 55 ++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index ce7345745b42..17c817d89222 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -45,7 +45,8 @@
>  static DEFINE_IDA(ddr_ida);
>  
>  /* DDR Perf hardware feature */
> -#define DDR_CAP_AXI_ID_FILTER          0x1     /* support AXI ID filter */
> +#define DDR_CAP_AXI_ID_FILTER			BIT(1)     /* support AXI ID filter */
> +#define DDR_CAP_AXI_ID_FILTER_ENHANCED		BIT(2)     /* support enhanced AXI ID filter */

I still don't understand why you don't do something like this:

#define DDR_CAP_AXI_ID_FILTER		0x1 /* support AXI ID filter */
#define DDR_CAP_AXI_ID_FILTER_ENHANCED	0x3 /* support enhanced AXI ID filter */


static bool ddr_perf_is_enhanced_filtered(struct perf_event *event)
{
	unsigned int filt;
	struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);

	filt = pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED;
	return (filt == DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
		ddr_perf_is_filtered(event);
}


and then:

> +	/*
> +	 * return bytes instead of bursts from ddr transaction for
> +	 * axid-read and axid-write event if PMU core supports enhanced
> +	 * filter.
> +	 */
> +	if ((pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) &&
> +	    (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
> +	    ddr_perf_is_filtered(event)) {

static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter)
{
	struct perf_event *event = pmu->events[counter];
	void __iomem *base = pmu->base;

	base += ddr_perf_is_enhanced_filtered(event) ? COUNTER_DPCR1 :
						       COUNTER_READ;
	return readl_relaxed(base + counter * 4);
}


In patch 4 you can then do:

.quirks = DDR_CAP_AXI_ID_FILTER_ENHANCED;

Make sense?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH V2 3/4] perf/imx_ddr: Add enhanced AXI ID filter support

[Joakim Zhang]

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: 2019年10月31日 23:39
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx
> <linux-imx@nxp.com>; Frank Li <frank.li@nxp.com>;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH V2 3/4] perf/imx_ddr: Add enhanced AXI ID filter support
> 
> On Tue, Oct 29, 2019 at 07:06:19AM +0000, Joakim Zhang wrote:
> > With DDR_CAP_AXI_ID_FILTER quirk, indicating HW supports AXI ID filter
> > which only can get bursts from DDR transaction, i.e. DDR read/write
> > requests.
> >
> > This patch add DDR_CAP_AXI_ID_ENHANCED_FILTER quirk, indicating HW
> > supports AXI ID filter which can get bursts and bytes from DDR
> > transaction at the same time. We hope PMU always return bytes in the
> > driver due to it is more meaningful for users.
> >
> > DDR_CAP_AXI_ID_ENHANCED_FILTER is based on DDR_CAP_AXI_ID_FILTER
> and
> > extend it a bit. So need select both above two qiurks together when HW
> > supports enhanced AXI ID filter.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> > Changelog:
> > V1->V2:
> > 	* use ddr_perf_is_filtered() helper to simply the code.
> > 	* improve the commit message.
> > ---
> >  drivers/perf/fsl_imx8_ddr_perf.c | 55
> > ++++++++++++++++++++------------
> >  1 file changed, 34 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c
> > b/drivers/perf/fsl_imx8_ddr_perf.c
> > index ce7345745b42..17c817d89222 100644
> > --- a/drivers/perf/fsl_imx8_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> > @@ -45,7 +45,8 @@
> >  static DEFINE_IDA(ddr_ida);
> >
> >  /* DDR Perf hardware feature */
> > -#define DDR_CAP_AXI_ID_FILTER          0x1     /* support AXI ID
> filter */
> > +#define DDR_CAP_AXI_ID_FILTER			BIT(1)     /* support AXI ID
> filter */
> > +#define DDR_CAP_AXI_ID_FILTER_ENHANCED		BIT(2)     /*
> support enhanced AXI ID filter */
> 
> I still don't understand why you don't do something like this:

This could be better as user is aware that enhanced filter is based on previous filter.

> #define DDR_CAP_AXI_ID_FILTER		0x1 /* support AXI ID filter */
> #define DDR_CAP_AXI_ID_FILTER_ENHANCED	0x3 /* support enhanced AXI
> ID filter */
> 
> 
> static bool ddr_perf_is_enhanced_filtered(struct perf_event *event) {
> 	unsigned int filt;
> 	struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
> 
> 	filt = pmu->devtype_data->quirks &
> DDR_CAP_AXI_ID_FILTER_ENHANCED;
> 	return (filt == DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
> 		ddr_perf_is_filtered(event);
> }
> 
> 
> and then:
> 
> > +	/*
> > +	 * return bytes instead of bursts from ddr transaction for
> > +	 * axid-read and axid-write event if PMU core supports enhanced
> > +	 * filter.
> > +	 */
> > +	if ((pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) &&
> > +	    (pmu->devtype_data->quirks &
> DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
> > +	    ddr_perf_is_filtered(event)) {
> 
> static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter) {
> 	struct perf_event *event = pmu->events[counter];
> 	void __iomem *base = pmu->base;
> 
> 	base += ddr_perf_is_enhanced_filtered(event) ? COUNTER_DPCR1 :
> 						       COUNTER_READ;
> 	return readl_relaxed(base + counter * 4); }
> 
> 
> In patch 4 you can then do:
> 
> .quirks = DDR_CAP_AXI_ID_FILTER_ENHANCED;
> 
> Make sense?

Great, this makes code more modular. Will, thanks for your perfect coding!

Best Regards,
Joakim Zhang
> Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel