Date:   Thu, 10 Nov 2022 16:34:41 +0800
From:   Yicong Yang <yangyicong@...wei.com>
To:     Bagas Sanjaya <bagasdotme@...il.com>
CC:     <yangyicong@...ilicon.com>,
        Shaokun Zhang <zhangshaokun@...ilicon.com>,
        <liuqi6124@...il.com>, Jonathan Corbet <corbet@....net>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        John Garry <john.garry@...wei.com>,
        <jonathan.cameron@...wei.com>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-doc@...r.kernel.org>, <linuxarm@...wei.com>,
        <f.fangjian@...wei.com>, <prime.zeng@...wei.com>
Subject: Re: [PATCH 3/3] drivers/perf: hisi: Add TLP filter support

On 2022/11/10 12:16, Bagas Sanjaya wrote:
> On Tue, Oct 25, 2022 at 07:32:42PM +0800, Yicong Yang wrote:
>> +4. TLP Length filter
>> +When counting bandwidth, the data can be composed of certain parts of TLP
>> +packets. You can specify it through "len_mode":
>> +
>> +- 2'b00: Reserved (Do not use this since the behaviour is undefined)
>> +- 2'b01: Bandwidth of TLP payloads
>> +- 2'b10: Bandwidth of TLP headers
>> +- 2'b11: Bandwidth of both TLP payloads and headers
>> +
>> +For example, "len_mode=2" means only counting the bandwidth of TLP headers
>> +and "len_mode=3" means the final bandwidth data is composed of both TLP
>> +headers and payloads. You need to carefully using this to avoid losing
>> +data. For example you're likely to get no counts by "len_mode=1" if the TLPs
>> +on the traffic has no payload. This config is optional, by default it'll
>> +be 2'b11.
>> +
>> +Example usage of perf::
>> +
>> +  $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,len_mode=0x1/ sleep 5
> 
> The indentation is rather ugly, so I have applied the fixup:
> 

will apply the fix and include your patch in v2.

Thanks.

> ---- >8 ----
> 
> diff --git a/Documentation/admin-guide/perf/hisi-pcie-pmu.rst b/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> index 83a2ef11b1a08d..bae690dddbebfd 100644
> --- a/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> +++ b/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> @@ -106,21 +106,22 @@ Example usage of perf::
>    $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,thr_len=0x4,thr_mode=1/ sleep 5
>  
>  4. TLP Length filter
> -When counting bandwidth, the data can be composed of certain parts of TLP
> -packets. You can specify it through "len_mode":
>  
> -- 2'b00: Reserved (Do not use this since the behaviour is undefined)
> -- 2'b01: Bandwidth of TLP payloads
> -- 2'b10: Bandwidth of TLP headers
> -- 2'b11: Bandwidth of both TLP payloads and headers
> +   When counting bandwidth, the data can be composed of certain parts of TLP
> +   packets. You can specify it through "len_mode":
>  
> -For example, "len_mode=2" means only counting the bandwidth of TLP headers
> -and "len_mode=3" means the final bandwidth data is composed of both TLP
> -headers and payloads. You need to carefully using this to avoid losing
> -data. For example you're likely to get no counts by "len_mode=1" if the TLPs
> -on the traffic has no payload. This config is optional, by default it'll
> -be 2'b11.
> +   - 2'b00: Reserved (Do not use this since the behaviour is undefined)
> +   - 2'b01: Bandwidth of TLP payloads
> +   - 2'b10: Bandwidth of TLP headers
> +   - 2'b11: Bandwidth of both TLP payloads and headers
>  
> -Example usage of perf::
> +   For example, "len_mode=2" means only counting the bandwidth of TLP headers
> +   and "len_mode=3" means the final bandwidth data is composed of both TLP
> +   headers and payloads. You need to carefully using this to avoid losing
> +   data. For example you're likely to get no counts by "len_mode=1" if the TLPs
> +   on the traffic has no payload. This config is optional, by default it'll
> +   be 2'b11.
>  
> -  $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,len_mode=0x1/ sleep 5
> +   Example usage of perf::
> +
> +     $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,len_mode=0x1/ sleep 5
> 
> Also, for formatting consistency, you need to pick up the fix for rest of
> documentation (as separate patch in your series):
> 
> ---- >8 ----
> 
> From 5f286aceb959a7c70a56cb5c453d3ac0b844fb49 Mon Sep 17 00:00:00 2001
> From: Bagas Sanjaya <bagasdotme@...il.com>
> Date: Thu, 10 Nov 2022 11:03:08 +0700
> Subject: [PATCH] Documentation: perf: Indent filter options list
> 
> The "Filter options" list have a rather ugly indentation. Also, the first
> paragraph after list name is rendered without separator (as continuation
> from the name).
> 
> Align the list by indenting the list items and add a blank line
> separator for each list name.
> 
> Fixes: c8602008e247f5 ("docs: perf: Add description for HiSilicon PCIe PMU driver")
> Signed-off-by: Bagas Sanjaya <bagasdotme@...il.com>
> ---
>  .../admin-guide/perf/hisi-pcie-pmu.rst        | 80 ++++++++++---------
>  1 file changed, 43 insertions(+), 37 deletions(-)
> 
> diff --git a/Documentation/admin-guide/perf/hisi-pcie-pmu.rst b/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> index bae690dddbebfd..54fab870a2aefb 100644
> --- a/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> +++ b/Documentation/admin-guide/perf/hisi-pcie-pmu.rst
> @@ -48,62 +48,68 @@ Filter options
>  --------------
>  
>  1. Target filter
> -PMU could only monitor the performance of traffic downstream target Root Ports
> -or downstream target Endpoint. PCIe PMU driver support "port" and "bdf"
> -interfaces for users, and these two interfaces aren't supported at the same
> -time.
>  
> --port
> -"port" filter can be used in all PCIe PMU events, target Root Port can be
> -selected by configuring the 16-bits-bitmap "port". Multi ports can be selected
> -for AP-layer-events, and only one port can be selected for TL/DL-layer-events.
> +   PMU could only monitor the performance of traffic downstream target Root
> +   Ports or downstream target Endpoint. PCIe PMU driver support "port" and
> +   "bdf" interfaces for users, and these two interfaces aren't supported at the
> +   same time.
>  
> -For example, if target Root Port is 0000:00:00.0 (x8 lanes), bit0 of bitmap
> -should be set, port=0x1; if target Root Port is 0000:00:04.0 (x4 lanes),
> -bit8 is set, port=0x100; if these two Root Ports are both monitored, port=0x101.
> +   - port
>  
> -Example usage of perf::
> +     "port" filter can be used in all PCIe PMU events, target Root Port can be
> +     selected by configuring the 16-bits-bitmap "port". Multi ports can be
> +     selected for AP-layer-events, and only one port can be selected for
> +     TL/DL-layer-events.
>  
> -  $# perf stat -e hisi_pcie0_core0/rx_mwr_latency,port=0x1/ sleep 5
> +     For example, if target Root Port is 0000:00:00.0 (x8 lanes), bit0 of
> +     bitmap should be set, port=0x1; if target Root Port is 0000:00:04.0 (x4
> +     lanes), bit8 is set, port=0x100; if these two Root Ports are both
> +     monitored, port=0x101.
>  
> --bdf
> +     Example usage of perf::
>  
> -"bdf" filter can only be used in bandwidth events, target Endpoint is selected
> -by configuring BDF to "bdf". Counter only counts the bandwidth of message
> -requested by target Endpoint.
> +       $# perf stat -e hisi_pcie0_core0/rx_mwr_latency,port=0x1/ sleep 5
>  
> -For example, "bdf=0x3900" means BDF of target Endpoint is 0000:39:00.0.
> +   - bdf
>  
> -Example usage of perf::
> +     "bdf" filter can only be used in bandwidth events, target Endpoint is
> +     selected by configuring BDF to "bdf". Counter only counts the bandwidth of
> +     message requested by target Endpoint.
>  
> -  $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,bdf=0x3900/ sleep 5
> +     For example, "bdf=0x3900" means BDF of target Endpoint is 0000:39:00.0.
> +
> +     Example usage of perf::
> +
> +       $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,bdf=0x3900/ sleep 5
>  
>  2. Trigger filter
> -Event statistics start when the first time TLP length is greater/smaller
> -than trigger condition. You can set the trigger condition by writing "trig_len",
> -and set the trigger mode by writing "trig_mode". This filter can only be used
> -in bandwidth events.
>  
> -For example, "trig_len=4" means trigger condition is 2^4 DW, "trig_mode=0"
> -means statistics start when TLP length > trigger condition, "trig_mode=1"
> -means start when TLP length < condition.
> +   Event statistics start when the first time TLP length is greater/smaller
> +   than trigger condition. You can set the trigger condition by writing
> +   "trig_len", and set the trigger mode by writing "trig_mode". This filter can
> +   only be used in bandwidth events.
>  
> -Example usage of perf::
> +   For example, "trig_len=4" means trigger condition is 2^4 DW, "trig_mode=0"
> +   means statistics start when TLP length > trigger condition, "trig_mode=1"
> +   means start when TLP length < condition.
>  
> -  $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,trig_len=0x4,trig_mode=1/ sleep 5
> +   Example usage of perf::
> +
> +     $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,trig_len=0x4,trig_mode=1/ sleep 5
>  
>  3. Threshold filter
> -Counter counts when TLP length within the specified range. You can set the
> -threshold by writing "thr_len", and set the threshold mode by writing
> -"thr_mode". This filter can only be used in bandwidth events.
>  
> -For example, "thr_len=4" means threshold is 2^4 DW, "thr_mode=0" means
> -counter counts when TLP length >= threshold, and "thr_mode=1" means counts
> -when TLP length < threshold.
> +   Counter counts when TLP length within the specified range. You can set the
> +   threshold by writing "thr_len", and set the threshold mode by writing
> +   "thr_mode". This filter can only be used in bandwidth events.
>  
> -Example usage of perf::
> +   For example, "thr_len=4" means threshold is 2^4 DW, "thr_mode=0" means
> +   counter counts when TLP length >= threshold, and "thr_mode=1" means counts
> +   when TLP length < threshold.
>  
> -  $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,thr_len=0x4,thr_mode=1/ sleep 5
> +   Example usage of perf::
> +
> +     $# perf stat -e hisi_pcie0_core0/rx_mrd_flux,thr_len=0x4,thr_mode=1/ sleep 5
>  
>  4. TLP Length filter
>  
> 

Powered by blists - more mailing lists