Re: [PATCH 0/6] perf/imx_ddr: Add namespace for i.MX8 DDR Perf

[John Garry <john.garry@huawei.com>]

On 08/04/2020 06:04, Joakim Zhang wrote:
>>   . Can you add this, regenerate,
>> and retry:
>>
>> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
>> index 675aec9881ce..16dcf00c792a 100644
>> --- a/tools/perf/pmu-events/jevents.c
>> +++ b/tools/perf/pmu-events/jevents.c
>> @@ -258,6 +258,7 @@ static struct map {
>>          { "hisi_sccl,l3c", "hisi_sccl,l3c" },
>>          /* it's not realistic to keep adding these, we need something more
>> scalable ... */
>>          { "smmuv3_pmcg", "smmuv3_pmcg" },
>> +       { "imx8_ddr", "imx8_ddr" },
>>          { "L3PMC", "amd_l3" },
>>          {}
> Yes, events can work after adding this code, however, metrics still can't work, it seems that metricgroup fails to parse the metric expression. You may change something from metricgroup.c causing this issue.

That code should be effectively the same.

What does perf list show for the metrics now?

> 
>>> root@imx8mmevk:~# ./perf stat -a -I 1000 -M imx8mm_ddr0_read.all event

Can you run with -v option and share the result?

>>> syntax error: ''
>>>                         \___ parser error
>>>
>>>    Usage: perf stat [<options>] [<command>]
>>>
>>>       -M, --metrics <metric/metric group list>
>>>                             monitor specified metrics or metric groups
>>> (separated by ,)
>>>
>>> However, it can work on branch:
>> private-topic-perf-5.6-pmu-events-upstream-v2. What changes have you made
>> compared to last?
>>> Could you help do below change? Since some Socs may have two ddr
>> controller(ddr0/ddr1) with the same event code.
>>
>> The perf tool can handle that. So just run perf with -v option, and it will show
>> event count breakdown per PMU, like this:
>>
>> root@ubuntu:/# ./perf stat -v -e smmuv3_pmcg.l1_tlb sleep 1 Using CPUID
>> 0x00000000480fd010 Using SYSID HIP08
>> -> smmuv3_pmcg_200100020/event=0x8a/
>> -> smmuv3_pmcg_200140020/event=0x8a/
>> -> smmuv3_pmcg_100020/event=0x8a/
>> -> smmuv3_pmcg_140020/event=0x8a/
>> -> smmuv3_pmcg_200148020/event=0x8a/
>> -> smmuv3_pmcg_148020/event=0x8a/
>> smmuv3_pmcg.l1_tlb: 0 1001221690 1001221690
>> smmuv3_pmcg.l1_tlb: 0 1001220090 1001220090
>> smmuv3_pmcg.l1_tlb: 101 1001219660 1001219660
>> smmuv3_pmcg.l1_tlb: 0 1001219010 1001219010
>> smmuv3_pmcg.l1_tlb: 0 1001218360 1001218360
>> smmuv3_pmcg.l1_tlb: 134 1001217850 1001217850
>>
>> Here smmuv3_pmcg_200100020, smmuv3_pmcg_200140020, etc are all
>> instances of the same PMU, like your DDRC.
>>
>> And if you want to run just for a single PMU, try like:
>>
>> ./perf stat -e smmuv3_pmcg_200148020/smmuv3_pmcg.l1_tlb/
> Thank you for your hint, it is really useful.
> 
> Then please help update the ddrc.json and metric.json:

ok, fine

> ddrc.json:
> [
>     {
>             "BriefDescription": "ddr cycles event",
>             "EventCode": "0x00",
>             "EventName": "imx8_ddr.cycles",

note: if you have just "cycles" here, instead of "imx8_ddr.cycles", then 
the HW PMU event will not show in perf list and you only get the alias 
under “imx8_ddr” group. This can be good if your PMU has so many events 
and you don't want to show them all. For hisilicon PMU, we have over 600 
HW PMU events.

However, as I see, a downside is that many PMUs have "cycles" events, 
and if you just use "cycles" as perf event, then it may just use the 
incorrect PMU by accident. I need to check this more.

thanks,
John

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