diff mbox series

[RFC,v2,09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

Message ID 1587120084-18990-10-git-send-email-john.garry@huawei.com (mailing list archive)
State New, archived
Headers show
Series perf pmu-events: Support event aliasing for system PMUs | expand

Commit Message

John Garry April 17, 2020, 10:41 a.m. UTC
From: Joakim Zhang <qiangqing.zhang@nxp.com>

Add JSON metrics for imx8mm DDR Perf.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/freescale/imx8mm/sys/ddrc.json      | 39 ++++++++++++++++++++++
 .../arch/arm64/freescale/imx8mm/sys/metrics.json   | 18 ++++++++++
 tools/perf/pmu-events/jevents.c                    |  1 +
 3 files changed, 58 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json

Comments

Joakim Zhang April 20, 2020, 4:17 a.m. UTC | #1
> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月17日 18:41
> To: peterz@infradead.org; mingo@redhat.com; acme@kernel.org;
> mark.rutland@arm.com; alexander.shishkin@linux.intel.com;
> jolsa@redhat.com; namhyung@kernel.org; will@kernel.org
> Cc: ak@linux.intel.com; linuxarm@huawei.com; linux-kernel@vger.kernel.org;
> Joakim Zhang <qiangqing.zhang@nxp.com>; irogers@google.com;
> robin.murphy@arm.com; zhangshaokun@hisilicon.com;
> linux-arm-kernel@lists.infradead.org; John Garry <john.garry@huawei.com>
> Subject: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> 
> Add JSON metrics for imx8mm DDR Perf.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  .../arch/arm64/freescale/imx8mm/sys/ddrc.json      | 39
> ++++++++++++++++++++++
>  .../arch/arm64/freescale/imx8mm/sys/metrics.json   | 18 ++++++++++
>  tools/perf/pmu-events/jevents.c                    |  1 +
>  3 files changed, 58 insertions(+)
>  create mode 100644
> tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
>  create mode 100644
> tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> 
> diff --git
> a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> new file mode 100644
> index 000000000000..8a3dae61a48f
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> @@ -0,0 +1,39 @@
> +[
> +   {
> +           "BriefDescription": "ddr cycles event",
> +           "EventCode": "0x00",
> +           "EventName": "imx8_ddr.cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr read-cycles event",
> +           "EventCode": "0x2a",
> +           "EventName": "imx8_ddr.read_cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr write-cycles event",
> +           "EventCode": "0x2b",
> +           "EventName": "imx8_ddr.write_cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr read event",
> +           "EventCode": "0x35",
> +           "EventName": "imx8_ddr.read",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr write event",
> +           "EventCode": "0x38",
> +           "EventName": "imx8_ddr.write",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   }
> +]
Hi John,

Tested from branch: private-topic-perf-5.7-sys-pmu-events-v2

DDR events test is okay on both 8MM and 8QM.


> diff --git
> a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> new file mode 100644
> index 000000000000..b6a776ca7cc2
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> @@ -0,0 +1,18 @@
> +[
> +   {
> +	    "BriefDescription": "bytes all masters read from ddr based on
> read-cycles event",
> +	    "MetricName": "imx8mm_ddr_read.all",
> +	    "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
> +	    "ScaleUnit": "9.765625e-4MB",
> +	    "Unit": "imx8_ddr",
> +	    "Compat": "i.mx8mm"
> +    },
> +   {
> +	    "BriefDescription": "bytes all masters write to ddr based on
> write-cycles event",
> +	    "MetricName": "imx8mm_ddr_write.all",
> +	    "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
> +	    "ScaleUnit": "9.765625e-4MB",
> +	    "Unit": "imx8_ddr",
> +	    "Compat": "i.mx8mm"
> +    }
> +]

However, it seems that there are small defects from metric.

Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into "ScaleUnit": "9.765625e-4KB", this is a mistake.

Then, you can see that test is okay from 8MM. However, metric would add twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks incorrect.

8MM:
root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.write_cycles * 4 * 4 for imx8mm_ddr_write.all
found event imx8_ddr.write_cycles
adding {imx8_ddr.write_cycles}:W
imx8_ddr.write_cycles -> imx8_ddr0/event=0x2b/
imx8_ddr.write_cycles: 13153 1000495125 1000495125
#           time             counts unit events
     1.000476625              13153      imx8_ddr.write_cycles     #    205.5 MB  imx8mm_ddr_write.all
imx8_ddr.write_cycles: 3582 1000681375 1000681375
     2.001167750               3582      imx8_ddr.write_cycles     #     56.0 MB  imx8mm_ddr_write.all


8QM:
root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
found event imx8_ddr.read_cycles
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
found event imx8_ddr.read_cycles
adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles: 22748 1000378750 1000378750
imx8_ddr.read_cycles: 24640 1000376625 1000376625
imx8_ddr.read_cycles: 22800 1000375125 1000375125
imx8_ddr.read_cycles: 24616 1000372625 1000372625
#           time             counts unit events
     1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
     1.000377250              47416      imx8_ddr.read_cycles
imx8_ddr.read_cycles: 32672 1000454375 1000454375
imx8_ddr.read_cycles: 37888 1000457250 1000457250
imx8_ddr.read_cycles: 32736 1000460250 1000460250
imx8_ddr.read_cycles: 38012 1000463000 1000463000
     2.000812375              70560      imx8_ddr.read_cycles      #   1102.5 MB  imx8qm_ddr_read.all
     2.000812375              70748      imx8_ddr.read_cycles


Best Regards,
Joakim Zhang
> \ No newline at end of file
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 76a84ec2ffc8..efdade0194af 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -257,6 +257,7 @@ static struct map {
>  	{ "hisi_sccl,hha", "hisi_sccl,hha" },
>  	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
>  	/* it's not realistic to keep adding these, we need something more
> scalable ... */
> +	{ "imx8_ddr", "imx8_ddr" },
>  	{ "smmuv3_pmcg", "smmuv3_pmcg" },
>  	{ "L3PMC", "amd_l3" },
>  	{}
> --
> 2.16.4
John Garry April 20, 2020, 10:50 a.m. UTC | #2
On 20/04/2020 05:17, Joakim Zhang wrote:
> However, it seems that there are small defects from metric.
> 
> Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into "ScaleUnit": "9.765625e-4KB", this is a mistake.

ok

> 
> Then, you can see that test is okay from 8MM. However, metric would add twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks incorrect.
> 
> 8MM:
> root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.write_cycles * 4 * 4 for imx8mm_ddr_write.all
> found event imx8_ddr.write_cycles
> adding {imx8_ddr.write_cycles}:W
> imx8_ddr.write_cycles -> imx8_ddr0/event=0x2b/
> imx8_ddr.write_cycles: 13153 1000495125 1000495125
> #           time             counts unit events
>       1.000476625              13153      imx8_ddr.write_cycles     #    205.5 MB  imx8mm_ddr_write.all
> imx8_ddr.write_cycles: 3582 1000681375 1000681375
>       2.001167750               3582      imx8_ddr.write_cycles     #     56.0 MB  imx8mm_ddr_write.all
> 
> 
> 8QM:
> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all

Note: for this example, I don't know why you didn't use 
imx8mm_ddr_write.all, as for your 8MM test, so we can compare the same.

> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
> found event imx8_ddr.read_cycles
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
> found event imx8_ddr.read_cycles
> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles: 22748 1000378750 1000378750
> imx8_ddr.read_cycles: 24640 1000376625 1000376625
> imx8_ddr.read_cycles: 22800 1000375125 1000375125
> imx8_ddr.read_cycles: 24616 1000372625 1000372625
> #           time             counts unit events
>       1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
>       1.000377250              47416      imx8_ddr.read_cycles
> imx8_ddr.read_cycles: 32672 1000454375 1000454375
> imx8_ddr.read_cycles: 37888 1000457250 1000457250
> imx8_ddr.read_cycles: 32736 1000460250 1000460250
> imx8_ddr.read_cycles: 38012 1000463000 1000463000
>       2.000812375              70560      imx8_ddr.read_cycles      #   1102.5 MB  imx8qm_ddr_read.all
>       2.000812375              70748      imx8_ddr.read_cycles
> 

I that is just how the aliases work. But how about trying:

./perf stat -v -a -I 1000 -M imx8_ddr0/imx8qm_ddr_read.all/


for just ddr0

I know that the following worked for non-metrics for aliases on a 
specific HW PMU, so I guess should also work for metrics:

./perf stat -e smmuv3_pmcg_200148020/smmuv3_pmcg.l1_tlb/

Thanks,
John
Joakim Zhang April 20, 2020, 11:25 a.m. UTC | #3
> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月20日 18:51
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; peterz@infradead.org;
> mingo@redhat.com; acme@kernel.org; mark.rutland@arm.com;
> alexander.shishkin@linux.intel.com; jolsa@redhat.com;
> namhyung@kernel.org; will@kernel.org
> Cc: irogers@google.com; ak@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; linux-kernel@vger.kernel.org; Zhangshaokun
> <zhangshaokun@hisilicon.com>; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> On 20/04/2020 05:17, Joakim Zhang wrote:
> > However, it seems that there are small defects from metric.
> >
> > Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into
> "ScaleUnit": "9.765625e-4KB", this is a mistake.
> 
> ok
> 
> >
> > Then, you can see that test is okay from 8MM. However, metric would add
> twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks
> incorrect.
> >
> > 8MM:
> > root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
> > Using CPUID 0x00000000410fd030 metric expr imx8_ddr.write_cycles * 4 *
> > 4 for imx8mm_ddr_write.all found event imx8_ddr.write_cycles adding
> > {imx8_ddr.write_cycles}:W imx8_ddr.write_cycles ->
> > imx8_ddr0/event=0x2b/
> > imx8_ddr.write_cycles: 13153 1000495125 1000495125
> > #           time             counts unit events
> >       1.000476625              13153      imx8_ddr.write_cycles
> #    205.5 MB  imx8mm_ddr_write.all
> > imx8_ddr.write_cycles: 3582 1000681375 1000681375
> >       2.001167750               3582      imx8_ddr.write_cycles
> #     56.0 MB  imx8mm_ddr_write.all
> >
> >
> > 8QM:
> > root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
> 
> Note: for this example, I don't know why you didn't use imx8mm_ddr_write.all,
> as for your 8MM test, so we can compare the same.

Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change nothing else.

> > Using CPUID 0x00000000410fd030
> > metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all found
> > event imx8_ddr.read_cycles metric expr imx8_ddr.read_cycles * 4 * 4
> > for imx8qm_ddr_read.all found event imx8_ddr.read_cycles adding
> > {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> > imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/ imx8_ddr.read_cycles ->
> > imx8_ddr1/event=0x2a/ imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> > imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> > imx8_ddr.read_cycles: 22748 1000378750 1000378750
> > imx8_ddr.read_cycles: 24640 1000376625 1000376625
> > imx8_ddr.read_cycles: 22800 1000375125 1000375125
> > imx8_ddr.read_cycles: 24616 1000372625 1000372625
> > #           time             counts unit events
> >       1.000377250              47388      imx8_ddr.read_cycles
> #    740.4 MB  imx8qm_ddr_read.all
> >       1.000377250              47416      imx8_ddr.read_cycles
> > imx8_ddr.read_cycles: 32672 1000454375 1000454375
> > imx8_ddr.read_cycles: 37888 1000457250 1000457250
> > imx8_ddr.read_cycles: 32736 1000460250 1000460250
> > imx8_ddr.read_cycles: 38012 1000463000 1000463000
> >       2.000812375              70560      imx8_ddr.read_cycles
> #   1102.5 MB  imx8qm_ddr_read.all
> >       2.000812375              70748      imx8_ddr.read_cycles
> >
> 
> I that is just how the aliases work. But how about trying:
> 
> ./perf stat -v -a -I 1000 -M imx8_ddr0/imx8qm_ddr_read.all/
> 
> 
> for just ddr0
> 
> I know that the following worked for non-metrics for aliases on a specific HW
> PMU, so I guess should also work for metrics:
> 
> ./perf stat -e smmuv3_pmcg_200148020/smmuv3_pmcg.l1_tlb/

Yes, this can work for non-metrics, I tried before, it seems unsupported for metric.

You may misunderstand me, now I doesn't ask for a specific HW PMU(ddr0 or ddr1), the issue is that it would add and calculate twice when more than one HW PMU.

I also did below change which is implemented at my local side, but it can't work on your branch.
--- a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -2,7 +2,7 @@
    {
            "BriefDescription": "bytes all masters read from ddr based on read-cycles event",
            "MetricName": "imx8mm_ddr_read.all",
-           "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
+           "MetricExpr": "imx8_ddr0\\/read\\-cycles\\/ * 4 * 4",
            "ScaleUnit": "9.765625e-4MB",
            "Unit": "imx8_ddr",
            "Compat": "i.mx8mm"
@@ -10,9 +10,9 @@
    {
            "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
            "MetricName": "imx8mm_ddr_write.all",
-           "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
+           "MetricExpr": "imx8_ddr0\\/read\\-cycles\\/ * 4 * 4",
            "ScaleUnit": "9.765625e-4MB",
            "Unit": "imx8_ddr",
            "Compat": "i.mx8mm"
     }
-]
\ No newline at end of file
+]

root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
Using CPUID 0x00000000410fd080
metric expr imx8_ddr0\/read\-cycles\/ * 4 * 4 for imx8mm_ddr_read.all
found event imx8_ddr0
found event read-cycles
metric expr imx8_ddr0\/read\-cycles\/ * 4 * 4 for imx8mm_ddr_read.all
found event imx8_ddr0
found event read-cycles
adding {imx8_ddr0,read-cycles}:W,{imx8_ddr0,read-cycles}:W
event syntax error: '{imx8_ddr0,read-cycles}:W,{imx8_ddr0,read-cycles}:W'
                      \___ parser error

 Usage: perf stat [<options>] [<command>]

    -M, --metrics <metric/metric group list>
                          monitor specified metrics or metric groups (separated by ,)

Now the metricgroup can't parse above metric expression, it shouldn't be.

Best Regards,
Joakim Zhang
> Thanks,
> John
John Garry April 20, 2020, 2:20 p.m. UTC | #4
On 20/04/2020 12:25, Joakim Zhang wrote:
>>> imx8_ddr.write_cycles: 13153 1000495125 1000495125
>>> #           time             counts unit events
>>>        1.000476625              13153      imx8_ddr.write_cycles
>> #    205.5 MB  imx8mm_ddr_write.all
>>> imx8_ddr.write_cycles: 3582 1000681375 1000681375
>>>        2.001167750               3582      imx8_ddr.write_cycles
>> #     56.0 MB  imx8mm_ddr_write.all
>>>
>>> 8QM:
>>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
>> Note: for this example, I don't know why you didn't use imx8mm_ddr_write.all,
>> as for your 8MM test, so we can compare the same.
> Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change nothing else.

Well it's hard to even keep up - let alone help -  when you're debugging 
QM support, which is not supported in this series (only MM is), and I 
don't know exactly what is in this JSON who have created (for QM).

For a start, the MM json will use "i.mx8mm" compat, which I figure 
should not work for QM. Please explain this.

Thanks,
John

> 
>>> Using CPUID 0x00000000410fd030
>>> metric expr imx8_ddr.read_cycles * 4 * 4 for i
Joakim Zhang April 21, 2020, 2:40 a.m. UTC | #5
> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月20日 22:20
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; peterz@infradead.org;
> mingo@redhat.com; acme@kernel.org; mark.rutland@arm.com;
> alexander.shishkin@linux.intel.com; jolsa@redhat.com;
> namhyung@kernel.org; will@kernel.org
> Cc: irogers@google.com; ak@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; linux-kernel@vger.kernel.org; Zhangshaokun
> <zhangshaokun@hisilicon.com>; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> On 20/04/2020 12:25, Joakim Zhang wrote:
> >>> imx8_ddr.write_cycles: 13153 1000495125 1000495125
> >>> #           time             counts unit events
> >>>        1.000476625              13153
> imx8_ddr.write_cycles
> >> #    205.5 MB  imx8mm_ddr_write.all
> >>> imx8_ddr.write_cycles: 3582 1000681375 1000681375
> >>>        2.001167750               3582
> imx8_ddr.write_cycles
> >> #     56.0 MB  imx8mm_ddr_write.all
> >>>
> >>> 8QM:
> >>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
> >> Note: for this example, I don't know why you didn't use
> >> imx8mm_ddr_write.all, as for your 8MM test, so we can compare the same.
> > Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change
> nothing else.
> 
> Well it's hard to even keep up - let alone help -  when you're debugging QM
> support, which is not supported in this series (only MM is), and I don't know
> exactly what is in this JSON who have created (for QM).
> 
> For a start, the MM json will use "i.mx8mm" compat, which I figure should not
> work for QM. Please explain this.

For common events, cycles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), read(event=0x35), write(event=0x38), all these events listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are compatible for all i.MX8 DDR Perf, only AXI events are various from each SoC. These events tested okay for MX8MM and MX8QM.

Same situation, metrics listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) is also compatible for all i.MX8 DDR Perf, since metric expression only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).

Generally speaking, now pmu events and metrics on your branch should support both MX8MM and MX8QM without any change, as long as they export "i.mx8mm" identifier.

As I mentioned before, pmu events tested okay for MX8MM and MX8QM. Metric also tested okay for MX8MM.
For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it would add metric twice which I think if it is possible to improve it in your serials. 

I guess the root cause is that "imx8_ddr.read_cycles" contains two HW PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and metricgroup can't handle it at present.

8QM:
root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all 
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all 
found event imx8_ddr.read_cycles
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all 
found event imx8_ddr.read_cycles
adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles: 22748 1000378750 1000378750
imx8_ddr.read_cycles: 24640 1000376625 1000376625
imx8_ddr.read_cycles: 22800 1000375125 1000375125
imx8_ddr.read_cycles: 24616 1000372625 1000372625
#           time             counts unit events
     1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
     1.000377250              47416      imx8_ddr.read_cycles

Best Regards,
Joakim Zhang
> Thanks,
> John
> 
> >
> >>> Using CPUID 0x00000000410fd030
> >>> metric expr imx8_ddr.read_cycles * 4 * 4 for i
John Garry April 21, 2020, 12:28 p.m. UTC | #6
On 21/04/2020 03:40, Joakim Zhang wrote:
> For common events, cycles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), read(event=0x35), write(event=0x38), all these events listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are compatible for all i.MX8 DDR Perf, only AXI events are various from each SoC. These events tested okay for MX8MM and MX8QM.
> 
> Same situation, metrics listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) is also compatible for all i.MX8 DDR Perf, since metric expression only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).
> 
> Generally speaking, now pmu events and metrics on your branch should support both MX8MM and MX8QM without any change, as long as they export "i.mx8mm" identifier.

Right, but MX8QM should export "i.mx8qm" identifier for upstream eventually.

> 
> As I mentioned before, pmu events tested okay for MX8MM and MX8QM. Metric also tested okay for MX8MM.
> For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it would add metric twice which I think if it is possible to improve it in your serials.
> 
> I guess the root cause is that "imx8_ddr.read_cycles" contains two HW PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and metricgroup can't handle it at present.

It should be ok, but I'll check it.

> 
> 8QM:
> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
> found event imx8_ddr.read_cycles
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
> found event imx8_ddr.read_cycles
> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles: 22748 1000378750 1000378750
> imx8_ddr.read_cycles: 24640 1000376625 1000376625
> imx8_ddr.read_cycles: 22800 1000375125 1000375125
> imx8_ddr.read_cycles: 24616 1000372625 1000372625
> #           time             counts unit events
>       1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
>       1.000377250              47416      imx8_ddr.read_cycles

john
John Garry April 27, 2020, 8:09 a.m. UTC | #7
On 21/04/2020 13:28, John Garry wrote:
>> cles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), 
>> read(event=0x35), write(event=0x38), all these events listed in file 
>> (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are 
>> compatible for all i.MX8 DDR Perf, only AXI events are various from 
>> each SoC. These events tested okay for MX8MM and MX8QM.
>>
>> Same situation, metrics listed in file 
>> (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) 
>> is also compatible for all i.MX8 DDR Perf, since metric expression 
>> only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).
>>
>> Generally speaking, now pmu events and metrics on your branch should 
>> support both MX8MM and MX8QM without any change, as long as they 
>> export "i.mx8mm" identifier.
> 
> Right, but MX8QM should export "i.mx8qm" identifier for upstream 
> eventually.
> 
>>
>> As I mentioned before, pmu events tested okay for MX8MM and MX8QM. 
>> Metric also tested okay for MX8MM.
>> For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it 
>> would add metric twice which I think if it is possible to improve it 
>> in your serials.
>>
>> I guess the root cause is that "imx8_ddr.read_cycles" contains two HW 
>> PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and 
>> metricgroup can't handle it at present.
> 
> It should be ok, but I'll check it.
> 

ok, I think I see the issue here. We add a metric per PMU erroneously. 
We don't see an issue for printing metrics, as the code does not error 
when adding clones when printing (which we do).

I'll try to fix this week.

Thanks,
John

>>
>> 8QM:
>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
>> Using CPUID 0x00000000410fd030
>> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
>> found event imx8_ddr.read_cycles
>> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
>> found event imx8_ddr.read_cycles
>> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
>> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
>> imx8_ddr.read_cycles: 22748 1000378750 1000378750
>> imx8_ddr.read_cycles: 24640 1000376625 100
diff mbox series

Patch

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
new file mode 100644
index 000000000000..8a3dae61a48f
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
@@ -0,0 +1,39 @@ 
+[
+   {
+           "BriefDescription": "ddr cycles event",
+           "EventCode": "0x00",
+           "EventName": "imx8_ddr.cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read-cycles event",
+           "EventCode": "0x2a",
+           "EventName": "imx8_ddr.read_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write-cycles event",
+           "EventCode": "0x2b",
+           "EventName": "imx8_ddr.write_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read event",
+           "EventCode": "0x35",
+           "EventName": "imx8_ddr.read",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write event",
+           "EventCode": "0x38",
+           "EventName": "imx8_ddr.write",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   }
+]
+
+
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
new file mode 100644
index 000000000000..b6a776ca7cc2
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -0,0 +1,18 @@ 
+[
+   {
+	    "BriefDescription": "bytes all masters read from ddr based on read-cycles event",
+	    "MetricName": "imx8mm_ddr_read.all",
+	    "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4MB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    },
+   {
+	    "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
+	    "MetricName": "imx8mm_ddr_write.all",
+	    "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4MB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    }
+]
\ No newline at end of file
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 76a84ec2ffc8..efdade0194af 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -257,6 +257,7 @@  static struct map {
 	{ "hisi_sccl,hha", "hisi_sccl,hha" },
 	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
 	/* it's not realistic to keep adding these, we need something more scalable ... */
+	{ "imx8_ddr", "imx8_ddr" },
 	{ "smmuv3_pmcg", "smmuv3_pmcg" },
 	{ "L3PMC", "amd_l3" },
 	{}