[v4,0/4] Add JSON metrics for Yitian710 DDR

  Hi all,

I add an identifier sysfs file for the yitian710 SoC DDR to allow
userspace to identify the specific implementation of the device,
so that the perf tool can match the corresponding uncore events and
metrics through the identifier. Then added yitian710 SoC DDR
metrics and events alias.

Change since v3:
- Split the CMN and ali_drw patches. This patchset only contains
  ali_drw PMU related patches. The CMN metric related patches will
  be in another patchset.
- Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/

$perf list:
...
ali_drw:
  chi_rxdat
       [A packet at CHI RXDAT interface (write data). Unit: ali_drw]
  chi_rxrsp
       [A packet at CHI RXRSP interface. Unit: ali_drw]
  chi_txdat
       [A packet at CHI TXDAT interface (read data). Unit: ali_drw]
  chi_txreq
       [A packet at CHI TXREQ interface (request). Unit: ali_drw]
  cycle
       [The ddr cycle. Unit: ali_drw]
...
ali_drw:
  ddr_read_bandwidth.all
       [The ddr read bandwidth(MB/s). Unit: ali_drw ]
  ddr_write_bandwidth.all
       [The ddr write bandwidth(MB/s). Unit: ali_drw ]
...

$perf stat -M ddr_read_bandwidth.all ./test

Performance counter stats for 'system wide':

            38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
     1,000,957,941 ns   duration_time

       1.000957941 seconds time elapsed

Jing Zhang (4):
  driver/perf: Add identifier sysfs file for Yitian 710 DDR
  perf jevents: Add support for Yitian 710 DDR PMU aliasing
  perf vendor events: Add JSON metrics for Yitian 710 DDR
  docs: perf: Update metric usage for Alibaba's T-Head PMU driver

 Documentation/admin-guide/perf/alibaba_pmu.rst     |   5 +
 drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
 .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
 .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
 tools/perf/pmu-events/jevents.py                   |   1 +
 5 files changed, 426 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
  

Hello,

On Tue, Jun 20, 2023 at 12:17 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>
> Hi all,
>
> I add an identifier sysfs file for the yitian710 SoC DDR to allow
> userspace to identify the specific implementation of the device,
> so that the perf tool can match the corresponding uncore events and
> metrics through the identifier. Then added yitian710 SoC DDR
> metrics and events alias.
>
> Change since v3:
> - Split the CMN and ali_drw patches. This patchset only contains
>   ali_drw PMU related patches. The CMN metric related patches will
>   be in another patchset.
> - Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/
>
> $perf list:
> ...
> ali_drw:
>   chi_rxdat
>        [A packet at CHI RXDAT interface (write data). Unit: ali_drw]
>   chi_rxrsp
>        [A packet at CHI RXRSP interface. Unit: ali_drw]
>   chi_txdat
>        [A packet at CHI TXDAT interface (read data). Unit: ali_drw]
>   chi_txreq
>        [A packet at CHI TXREQ interface (request). Unit: ali_drw]
>   cycle
>        [The ddr cycle. Unit: ali_drw]
> ...
> ali_drw:
>   ddr_read_bandwidth.all
>        [The ddr read bandwidth(MB/s). Unit: ali_drw ]
>   ddr_write_bandwidth.all
>        [The ddr write bandwidth(MB/s). Unit: ali_drw ]
> ...
>
> $perf stat -M ddr_read_bandwidth.all ./test
>
> Performance counter stats for 'system wide':
>
>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
>      1,000,957,941 ns   duration_time
>
>        1.000957941 seconds time elapsed
>
> Jing Zhang (4):
>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
>   perf vendor events: Add JSON metrics for Yitian 710 DDR
>   docs: perf: Update metric usage for Alibaba's T-Head PMU driver

So patch 1 is for the kernel, and patch 2-4 depend on it, right?

I'm curious why the first patch is needed, presumably the PMU
should have 'ali_drw' in the name already.  Do we use substring
match for the compat name in the JSON metric?

Thanks,
Namhyung

>
>  Documentation/admin-guide/perf/alibaba_pmu.rst     |   5 +
>  drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>  .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
>  tools/perf/pmu-events/jevents.py                   |   1 +
>  5 files changed, 426 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>
> --
> 1.8.3.1
>
  

在 2023/6/21 上午3:04, Namhyung Kim 写道:
> Hello,
> 
> On Tue, Jun 20, 2023 at 12:17 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>
>> Hi all,
>>
>> I add an identifier sysfs file for the yitian710 SoC DDR to allow
>> userspace to identify the specific implementation of the device,
>> so that the perf tool can match the corresponding uncore events and
>> metrics through the identifier. Then added yitian710 SoC DDR
>> metrics and events alias.
>>
>> Change since v3:
>> - Split the CMN and ali_drw patches. This patchset only contains
>>   ali_drw PMU related patches. The CMN metric related patches will
>>   be in another patchset.
>> - Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/
>>
>> $perf list:
>> ...
>> ali_drw:
>>   chi_rxdat
>>        [A packet at CHI RXDAT interface (write data). Unit: ali_drw]
>>   chi_rxrsp
>>        [A packet at CHI RXRSP interface. Unit: ali_drw]
>>   chi_txdat
>>        [A packet at CHI TXDAT interface (read data). Unit: ali_drw]
>>   chi_txreq
>>        [A packet at CHI TXREQ interface (request). Unit: ali_drw]
>>   cycle
>>        [The ddr cycle. Unit: ali_drw]
>> ...
>> ali_drw:
>>   ddr_read_bandwidth.all
>>        [The ddr read bandwidth(MB/s). Unit: ali_drw ]
>>   ddr_write_bandwidth.all
>>        [The ddr write bandwidth(MB/s). Unit: ali_drw ]
>> ...
>>
>> $perf stat -M ddr_read_bandwidth.all ./test
>>
>> Performance counter stats for 'system wide':
>>
>>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
>>      1,000,957,941 ns   duration_time
>>
>>        1.000957941 seconds time elapsed
>>
>> Jing Zhang (4):
>>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
>>   perf vendor events: Add JSON metrics for Yitian 710 DDR
>>   docs: perf: Update metric usage for Alibaba's T-Head PMU driver
> 
> So patch 1 is for the kernel, and patch 2-4 depend on it, right?
> 

Hi Namhyung,

Yes, patch 2-4 depend on patch 1.

> I'm curious why the first patch is needed, presumably the PMU
> should have 'ali_drw' in the name already.  Do we use substring
> match for the compat name in the JSON metric?
> 

The main purpose of patch 1 is to add an identifier so that the Compat
field can match the corresponding event when defining aliases or metrics
for events.

For example, "Unit" can match "ali_drw" in the name and different SoCs may
be able to match ali_drw, but they may have different events, and even if
the events are the same, the meanings may be different. Therefore, the
Compat field is needed to match the Identifier to confirm which type and
revision of PMU the current SoC has. Therefore, both "Unit" and "Compat"
need to be matched at the same time. Although it seems that ali_drw is
redundantly matched currently, it is meaningful for future expansion.

Thanks,
Jing

> Thanks,
> Namhyung
> 
>>
>>  Documentation/admin-guide/perf/alibaba_pmu.rst     |   5 +
>>  drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
>>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>>  .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
>>  tools/perf/pmu-events/jevents.py                   |   1 +
>>  5 files changed, 426 insertions(+)
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>>
>> --
>> 1.8.3.1
>>
  

On Tue, Jun 20, 2023 at 8:08 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>
>
>
> 在 2023/6/21 上午3:04, Namhyung Kim 写道:
> > Hello,
> >
> > On Tue, Jun 20, 2023 at 12:17 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
> >>
> >> Hi all,
> >>
> >> I add an identifier sysfs file for the yitian710 SoC DDR to allow
> >> userspace to identify the specific implementation of the device,
> >> so that the perf tool can match the corresponding uncore events and
> >> metrics through the identifier. Then added yitian710 SoC DDR
> >> metrics and events alias.
> >>
> >> Change since v3:
> >> - Split the CMN and ali_drw patches. This patchset only contains
> >>   ali_drw PMU related patches. The CMN metric related patches will
> >>   be in another patchset.
> >> - Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/
> >>
> >> $perf list:
> >> ...
> >> ali_drw:
> >>   chi_rxdat
> >>        [A packet at CHI RXDAT interface (write data). Unit: ali_drw]
> >>   chi_rxrsp
> >>        [A packet at CHI RXRSP interface. Unit: ali_drw]
> >>   chi_txdat
> >>        [A packet at CHI TXDAT interface (read data). Unit: ali_drw]
> >>   chi_txreq
> >>        [A packet at CHI TXREQ interface (request). Unit: ali_drw]
> >>   cycle
> >>        [The ddr cycle. Unit: ali_drw]
> >> ...
> >> ali_drw:
> >>   ddr_read_bandwidth.all
> >>        [The ddr read bandwidth(MB/s). Unit: ali_drw ]
> >>   ddr_write_bandwidth.all
> >>        [The ddr write bandwidth(MB/s). Unit: ali_drw ]
> >> ...
> >>
> >> $perf stat -M ddr_read_bandwidth.all ./test
> >>
> >> Performance counter stats for 'system wide':
> >>
> >>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
> >>      1,000,957,941 ns   duration_time
> >>
> >>        1.000957941 seconds time elapsed
> >>
> >> Jing Zhang (4):
> >>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
> >>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
> >>   perf vendor events: Add JSON metrics for Yitian 710 DDR
> >>   docs: perf: Update metric usage for Alibaba's T-Head PMU driver
> >
> > So patch 1 is for the kernel, and patch 2-4 depend on it, right?
> >
>
> Hi Namhyung,
>
> Yes, patch 2-4 depend on patch 1.
>
> > I'm curious why the first patch is needed, presumably the PMU
> > should have 'ali_drw' in the name already.  Do we use substring
> > match for the compat name in the JSON metric?
> >
>
> The main purpose of patch 1 is to add an identifier so that the Compat
> field can match the corresponding event when defining aliases or metrics
> for events.
>
> For example, "Unit" can match "ali_drw" in the name and different SoCs may
> be able to match ali_drw, but they may have different events, and even if
> the events are the same, the meanings may be different. Therefore, the
> Compat field is needed to match the Identifier to confirm which type and
> revision of PMU the current SoC has. Therefore, both "Unit" and "Compat"
> need to be matched at the same time. Although it seems that ali_drw is
> redundantly matched currently, it is meaningful for future expansion.

I see, thanks for the explanation.

I think you need to route the kernel patch differently.  I can apply the tools
part once the kernel patch gets Acks from others.

Thanks,
Namhyung
  

在 2023/6/21 下午1:00, Namhyung Kim 写道:
> On Tue, Jun 20, 2023 at 8:08 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>
>>
>>
>> 在 2023/6/21 上午3:04, Namhyung Kim 写道:
>>> Hello,
>>>
>>> On Tue, Jun 20, 2023 at 12:17 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> I add an identifier sysfs file for the yitian710 SoC DDR to allow
>>>> userspace to identify the specific implementation of the device,
>>>> so that the perf tool can match the corresponding uncore events and
>>>> metrics through the identifier. Then added yitian710 SoC DDR
>>>> metrics and events alias.
>>>>
>>>> Change since v3:
>>>> - Split the CMN and ali_drw patches. This patchset only contains
>>>>   ali_drw PMU related patches. The CMN metric related patches will
>>>>   be in another patchset.
>>>> - Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/
>>>>
>>>> $perf list:
>>>> ...
>>>> ali_drw:
>>>>   chi_rxdat
>>>>        [A packet at CHI RXDAT interface (write data). Unit: ali_drw]
>>>>   chi_rxrsp
>>>>        [A packet at CHI RXRSP interface. Unit: ali_drw]
>>>>   chi_txdat
>>>>        [A packet at CHI TXDAT interface (read data). Unit: ali_drw]
>>>>   chi_txreq
>>>>        [A packet at CHI TXREQ interface (request). Unit: ali_drw]
>>>>   cycle
>>>>        [The ddr cycle. Unit: ali_drw]
>>>> ...
>>>> ali_drw:
>>>>   ddr_read_bandwidth.all
>>>>        [The ddr read bandwidth(MB/s). Unit: ali_drw ]
>>>>   ddr_write_bandwidth.all
>>>>        [The ddr write bandwidth(MB/s). Unit: ali_drw ]
>>>> ...
>>>>
>>>> $perf stat -M ddr_read_bandwidth.all ./test
>>>>
>>>> Performance counter stats for 'system wide':
>>>>
>>>>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
>>>>      1,000,957,941 ns   duration_time
>>>>
>>>>        1.000957941 seconds time elapsed
>>>>
>>>> Jing Zhang (4):
>>>>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>>>>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
>>>>   perf vendor events: Add JSON metrics for Yitian 710 DDR
>>>>   docs: perf: Update metric usage for Alibaba's T-Head PMU driver
>>>
>>> So patch 1 is for the kernel, and patch 2-4 depend on it, right?
>>>
>>
>> Hi Namhyung,
>>
>> Yes, patch 2-4 depend on patch 1.
>>
>>> I'm curious why the first patch is needed, presumably the PMU
>>> should have 'ali_drw' in the name already.  Do we use substring
>>> match for the compat name in the JSON metric?
>>>
>>
>> The main purpose of patch 1 is to add an identifier so that the Compat
>> field can match the corresponding event when defining aliases or metrics
>> for events.
>>
>> For example, "Unit" can match "ali_drw" in the name and different SoCs may
>> be able to match ali_drw, but they may have different events, and even if
>> the events are the same, the meanings may be different. Therefore, the
>> Compat field is needed to match the Identifier to confirm which type and
>> revision of PMU the current SoC has. Therefore, both "Unit" and "Compat"
>> need to be matched at the same time. Although it seems that ali_drw is
>> redundantly matched currently, it is meaningful for future expansion.
> 
> I see, thanks for the explanation.
> 
> I think you need to route the kernel patch differently.  I can apply the tools
> part once the kernel patch gets Acks from others.
> 

Ok, Thank you.

Thanks,
Jing

> Thanks,
> Namhyung
  

On Tue, Jun 20, 2023 at 10:00:46PM -0700, Namhyung Kim wrote:
> On Tue, Jun 20, 2023 at 8:08 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
> > 在 2023/6/21 上午3:04, Namhyung Kim 写道:
> > > I'm curious why the first patch is needed, presumably the PMU
> > > should have 'ali_drw' in the name already.  Do we use substring
> > > match for the compat name in the JSON metric?
> > >
> >
> > The main purpose of patch 1 is to add an identifier so that the Compat
> > field can match the corresponding event when defining aliases or metrics
> > for events.
> >
> > For example, "Unit" can match "ali_drw" in the name and different SoCs may
> > be able to match ali_drw, but they may have different events, and even if
> > the events are the same, the meanings may be different. Therefore, the
> > Compat field is needed to match the Identifier to confirm which type and
> > revision of PMU the current SoC has. Therefore, both "Unit" and "Compat"
> > need to be matched at the same time. Although it seems that ali_drw is
> > redundantly matched currently, it is meaningful for future expansion.
> 
> I see, thanks for the explanation.
> 
> I think you need to route the kernel patch differently.  I can apply the tools
> part once the kernel patch gets Acks from others.

Sorry, I missed this initially as I didn't realise there were kernel changes
hidden in this series (I saw "JSON" and ignored it...). Given that the 6.5
merge window is now open, I'll pick the kernel change up for 6.6 when I
start queueing patches in a few weeks.

Will
  

在 2023/6/26 下午8:44, Will Deacon 写道:
> On Tue, Jun 20, 2023 at 10:00:46PM -0700, Namhyung Kim wrote:
>> On Tue, Jun 20, 2023 at 8:08 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>> 在 2023/6/21 上午3:04, Namhyung Kim 写道:
>>>> I'm curious why the first patch is needed, presumably the PMU
>>>> should have 'ali_drw' in the name already.  Do we use substring
>>>> match for the compat name in the JSON metric?
>>>>
>>>
>>> The main purpose of patch 1 is to add an identifier so that the Compat
>>> field can match the corresponding event when defining aliases or metrics
>>> for events.
>>>
>>> For example, "Unit" can match "ali_drw" in the name and different SoCs may
>>> be able to match ali_drw, but they may have different events, and even if
>>> the events are the same, the meanings may be different. Therefore, the
>>> Compat field is needed to match the Identifier to confirm which type and
>>> revision of PMU the current SoC has. Therefore, both "Unit" and "Compat"
>>> need to be matched at the same time. Although it seems that ali_drw is
>>> redundantly matched currently, it is meaningful for future expansion.
>>
>> I see, thanks for the explanation.
>>
>> I think you need to route the kernel patch differently.  I can apply the tools
>> part once the kernel patch gets Acks from others.
> 
> Sorry, I missed this initially as I didn't realise there were kernel changes
> hidden in this series (I saw "JSON" and ignored it...). Given that the 6.5
> merge window is now open, I'll pick the kernel change up for 6.6 when I
> start queueing patches in a few weeks.
> 
> Will

Thank you will, maybe it is because I did not describe this series well, and
it is easy to cause the patch to be ignored. I will pay attention to this
problem in the future.

Thanks,
Jing
  

On Tue, 20 Jun 2023 15:12:32 +0800, Jing Zhang wrote:
> I add an identifier sysfs file for the yitian710 SoC DDR to allow
> userspace to identify the specific implementation of the device,
> so that the perf tool can match the corresponding uncore events and
> metrics through the identifier. Then added yitian710 SoC DDR
> metrics and events alias.
> 
> Change since v3:
> - Split the CMN and ali_drw patches. This patchset only contains
>   ali_drw PMU related patches. The CMN metric related patches will
>   be in another patchset.
> - Link: https://lore.kernel.org/all/1685438374-33287-1-git-send-email-renyu.zj@linux.alibaba.com/
> 
> [...]

Applied first patch to will (for-next/perf), thanks!

[1/4] driver/perf: Add identifier sysfs file for Yitian 710 DDR
      https://git.kernel.org/will/c/cbbc6fdd85be

Cheers,