-----Original Message-----
From: Will Deacon <will@kernel.org>
Sent: 2019年10月31日 23:39
To: Joakim Zhang <redacted>
Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx
[off-list ref]; Frank Li [off-list ref];
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:

quoted

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

quoted

extend it a bit. So need select both above two qiurks together when HW
supports enhanced AXI ID filter.

Signed-off-by: Joakim Zhang <redacted>
---
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 */

quoted

+#define DDR_CAP_AXI_ID_FILTER			BIT(1)     /* support AXI ID

filter */

quoted

+#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:

quoted

+	/*
+	 * 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) &&

quoted

+	    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