[v3,3/4] Documentation: devicetree: add PPMU events description
diff mbox series

Message ID 1555681688-19643-4-git-send-email-l.luba@partner.samsung.com
State Changes Requested
Headers show
Series
  • [v3,1/4] include: dt-bindings: add Performance Monitoring Unit for Exynos
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Lukasz Luba April 19, 2019, 1:48 p.m. UTC
Extend the documenation by events description with new 'event-data-type'
field. Add example how the event might be defined in DT.

Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
---
 .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Chanwoo Choi April 30, 2019, 6:16 a.m. UTC | #1
Hi Lukasz,

On 19. 4. 19. 오후 10:48, Lukasz Luba wrote:
> Extend the documenation by events description with new 'event-data-type'
> field. Add example how the event might be defined in DT.
> 
> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
> ---
>  .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> index 3e36c1d..47feb5f 100644
> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> @@ -145,3 +145,21 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>  			reg = <0x104d0000 0x2000>;
>  			status = "disabled";
>  		};
> +
> +The 'event' type specified in the PPMU node defines 'event-name'
> +which also contains 'id' number and optionally 'event-data-type'.
> +
> +Example:
> +
> +		events {
> +			ppmu_leftbus_0: ppmu-event0-leftbus {
> +				event-name = "ppmu-event0-leftbus";
> +				event-data-type = <PPMU_RO_DATA_CNT>;
> +			};
> +		};
> +
> +The 'event-data-type' defines the type of data which shell be counted
> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
> +all possible type, i.e. count read requests, count write data in bytes,
> +etc. This field is optional and when it is missing, the driver code will
> +use default data type.
> 

How about editing it as following?

--- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
+++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
@@ -10,14 +10,23 @@ The Exynos PPMU driver uses the devfreq-event class to provide event data
 to various devfreq devices. The devfreq devices would use the event data when
 derterming the current state of each IP.
 
-Required properties:
+Required properties for PPMU device:
 - compatible: Should be "samsung,exynos-ppmu" or "samsung,exynos-ppmu-v2.
 - reg: physical base address of each PPMU and length of memory mapped region.
 
-Optional properties:
+Optional properties for PPMU device:
 - clock-names : the name of clock used by the PPMU, "ppmu"
 - clocks : phandles for clock specified in "clock-names" property
 
+Required properties for 'events' child node of PPMU device:
+- event-name : the unique event name among PPMU device
+Optional properties for 'events' child node of PPMU device:
+- event-data-type : Define the type of data which shell be counted
+by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
+all possible type, i.e. count read requests, count write data in bytes,
+etc. This field is optional and when it is missing, the driver code
+will use default data type.
+
 Example1 : PPMUv1 nodes in exynos3250.dtsi are listed below.
 
                ppmu_dmc0: ppmu_dmc0@106a0000 {
@@ -145,3 +154,16 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
                        reg = <0x104d0000 0x2000>;
                        status = "disabled";
                };
+
+Example4 : 'event-data-type' in exynos4412-ppmu-common.dtsi are listed below.
+
+       &ppmu_dmc0 {
+               status = "okay";
+               events {
+                       ppmu_dmc0_3: ppmu-event3-dmc0 {
+                               event-name = "ppmu-event3-dmc0";
+                               event-data-type = <(PPMU_RO_DATA_CNT |
+                                               PPMU_WO_DATA_CNT)>;
+                       };
+               };
+       };
Lukasz Luba April 30, 2019, 9:23 p.m. UTC | #2
On 4/30/19 8:16 AM, Chanwoo Choi wrote:
> Hi Lukasz,
> 
> On 19. 4. 19. 오후 10:48, Lukasz Luba wrote:
>> Extend the documenation by events description with new 'event-data-type'
>> field. Add example how the event might be defined in DT.
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>>   .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> index 3e36c1d..47feb5f 100644
>> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> @@ -145,3 +145,21 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>>   			reg = <0x104d0000 0x2000>;
>>   			status = "disabled";
>>   		};
>> +
>> +The 'event' type specified in the PPMU node defines 'event-name'
>> +which also contains 'id' number and optionally 'event-data-type'.
>> +
>> +Example:
>> +
>> +		events {
>> +			ppmu_leftbus_0: ppmu-event0-leftbus {
>> +				event-name = "ppmu-event0-leftbus";
>> +				event-data-type = <PPMU_RO_DATA_CNT>;
>> +			};
>> +		};
>> +
>> +The 'event-data-type' defines the type of data which shell be counted
>> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
>> +all possible type, i.e. count read requests, count write data in bytes,
>> +etc. This field is optional and when it is missing, the driver code will
>> +use default data type.
>>
> 
> How about editing it as following?
> 
> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> @@ -10,14 +10,23 @@ The Exynos PPMU driver uses the devfreq-event class to provide event data
>   to various devfreq devices. The devfreq devices would use the event data when
>   derterming the current state of each IP.
>   
> -Required properties:
> +Required properties for PPMU device:
>   - compatible: Should be "samsung,exynos-ppmu" or "samsung,exynos-ppmu-v2.
>   - reg: physical base address of each PPMU and length of memory mapped region.
>   
> -Optional properties:
> +Optional properties for PPMU device:
>   - clock-names : the name of clock used by the PPMU, "ppmu"
>   - clocks : phandles for clock specified in "clock-names" property
>   
> +Required properties for 'events' child node of PPMU device:
> +- event-name : the unique event name among PPMU device
> +Optional properties for 'events' child node of PPMU device:
> +- event-data-type : Define the type of data which shell be counted
> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
> +all possible type, i.e. count read requests, count write data in bytes,
> +etc. This field is optional and when it is missing, the driver code
> +will use default data type.
> +
>   Example1 : PPMUv1 nodes in exynos3250.dtsi are listed below.
>   
>                  ppmu_dmc0: ppmu_dmc0@106a0000 {
> @@ -145,3 +154,16 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>                          reg = <0x104d0000 0x2000>;
>                          status = "disabled";
>                  };
> +
> +Example4 : 'event-data-type' in exynos4412-ppmu-common.dtsi are listed below.
> +
> +       &ppmu_dmc0 {
> +               status = "okay";
> +               events {
> +                       ppmu_dmc0_3: ppmu-event3-dmc0 {
> +                               event-name = "ppmu-event3-dmc0";
> +                               event-data-type = <(PPMU_RO_DATA_CNT |
> +                                               PPMU_WO_DATA_CNT)>;
> +                       };
> +               };
> +       };
> 
> I agree. It will be changed in the next version.

Regards,
Lukasz
Rob Herring April 30, 2019, 10:36 p.m. UTC | #3
On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
> Extend the documenation by events description with new 'event-data-type'
> field. Add example how the event might be defined in DT.

Why do we need event types in DT? We don't do this for other h/w such as 
ARM PMU.

> 
> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
> ---
>  .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> index 3e36c1d..47feb5f 100644
> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> @@ -145,3 +145,21 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>  			reg = <0x104d0000 0x2000>;
>  			status = "disabled";
>  		};
> +
> +The 'event' type specified in the PPMU node defines 'event-name'
> +which also contains 'id' number and optionally 'event-data-type'.
> +
> +Example:
> +
> +		events {
> +			ppmu_leftbus_0: ppmu-event0-leftbus {
> +				event-name = "ppmu-event0-leftbus";
> +				event-data-type = <PPMU_RO_DATA_CNT>;
> +			};
> +		};
> +
> +The 'event-data-type' defines the type of data which shell be counted
> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
> +all possible type, i.e. count read requests, count write data in bytes,
> +etc. This field is optional and when it is missing, the driver code will
> +use default data type.
> -- 
> 2.7.4
>
Lukasz Luba May 2, 2019, 8:52 a.m. UTC | #4
Hi Rob,

On 5/1/19 12:36 AM, Rob Herring wrote:
> On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
>> Extend the documenation by events description with new 'event-data-type'
>> field. Add example how the event might be defined in DT.
> 
> Why do we need event types in DT? We don't do this for other h/w such as
> ARM PMU.
In ARM PMU all the events are hard-coded into the driver code i.e. in v7
arch/arm/kernel/perf_event_v7.c
and are seen from perf. They are different type and for different
purpose. The Ecynos PPMU events are not seen in perf, they are
for internal monitoring and must not be reset by other actors like perf.
They are used by the 'bus drivers' to made some heuristics and tune the
internal settings, like frequency.

Chanwoo has written PPMU driver which relies on DT definition.
The DT events are used by other DT devices by phandle.
In Exynos 5x SoCs we have many 'bus devices' which use events to
monitor their usage and react accordingly.
---------------8<------------------------
[    4.140923] exynos-bus: new bus device registered: soc:bus_wcore ( 
84000 KHz ~ 400000 KHz)
[    4.149179] exynos-bus: new bus device registered: soc:bus_noc ( 
67000 KHz ~ 100000 KHz)
[    4.156825] exynos-bus: new bus device registered: soc:bus_fsys_apb 
(100000 KHz ~ 200000 KHz)
[    4.165071] exynos-bus: new bus device registered: soc:bus_fsys 
(100000 KHz ~ 200000 KHz)
[    4.173577] exynos-bus: new bus device registered: soc:bus_fsys2 ( 
75000 KHz ~ 150000 KHz)
[    4.182141] exynos-bus: new bus device registered: soc:bus_mfc ( 
96000 KHz ~ 333000 KHz)
[    4.190099] exynos-bus: new bus device registered: soc:bus_gen ( 
89000 KHz ~ 267000 KHz)
[    4.197953] exynos-bus: new bus device registered: soc:bus_peri ( 
67000 KHz ~  67000 KHz)
[    4.206523] exynos-bus: new bus device registered: soc:bus_g2d ( 
84000 KHz ~ 333000 KHz)
[    4.214516] exynos-bus: new bus device registered: soc:bus_g2d_acp ( 
67000 KHz ~ 267000 KHz)
[    4.222850] exynos-bus: new bus device registered: soc:bus_jpeg ( 
75000 KHz ~ 300000 KHz)
[    4.231052] exynos-bus: new bus device registered: soc:bus_jpeg_apb ( 
84000 KHz ~ 167000 KHz)
[    4.239202] exynos-bus: new bus device registered: soc:bus_disp1_fimd 
(120000 KHz ~ 200000 KHz)
[    4.248033] exynos-bus: new bus device registered: soc:bus_disp1 
(120000 KHz ~ 300000 KHz)
[    4.256304] exynos-bus: new bus device registered: 
soc:bus_gscl_scaler (150000 KHz ~ 300000 KHz)
[    4.265397] exynos-bus: new bus device registered: soc:bus_mscl ( 
84000 KHz ~ 400000 KHz)
-------------------------->8----------------------------------------
The PPMU driver made some assumption, though. It always monitors only
'read+write data bytes' as an event data type.
Thus, it is not possible to monitor other stuff and maybe improve the
heuristics.

This simple modification allows to define different data type, which is
acquired by the counter, still being compatible with all the Exynos
drivers in the existing implementation.

Regards,
Lukasz

> 
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>>   .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> index 3e36c1d..47feb5f 100644
>> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> @@ -145,3 +145,21 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>>   			reg = <0x104d0000 0x2000>;
>>   			status = "disabled";
>>   		};
>> +
>> +The 'event' type specified in the PPMU node defines 'event-name'
>> +which also contains 'id' number and optionally 'event-data-type'.
>> +
>> +Example:
>> +
>> +		events {
>> +			ppmu_leftbus_0: ppmu-event0-leftbus {
>> +				event-name = "ppmu-event0-leftbus";
>> +				event-data-type = <PPMU_RO_DATA_CNT>;
>> +			};
>> +		};
>> +
>> +The 'event-data-type' defines the type of data which shell be counted
>> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
>> +all possible type, i.e. count read requests, count write data in bytes,
>> +etc. This field is optional and when it is missing, the driver code will
>> +use default data type.
>> -- 
>> 2.7.4
>>
> 
>
Lukasz Luba May 2, 2019, 10:11 a.m. UTC | #5
Hi Chanwoo,

On 4/30/19 8:16 AM, Chanwoo Choi wrote:
> Hi Lukasz,
> 
> On 19. 4. 19. 오후 10:48, Lukasz Luba wrote:
>> Extend the documenation by events description with new 'event-data-type'
>> field. Add example how the event might be defined in DT.
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>>   .../devicetree/bindings/devfreq/event/exynos-ppmu.txt  | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> index 3e36c1d..47feb5f 100644
>> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
>> @@ -145,3 +145,21 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>>   			reg = <0x104d0000 0x2000>;
>>   			status = "disabled";
>>   		};
>> +
>> +The 'event' type specified in the PPMU node defines 'event-name'
>> +which also contains 'id' number and optionally 'event-data-type'.
>> +
>> +Example:
>> +
>> +		events {
>> +			ppmu_leftbus_0: ppmu-event0-leftbus {
>> +				event-name = "ppmu-event0-leftbus";
>> +				event-data-type = <PPMU_RO_DATA_CNT>;
>> +			};
>> +		};
>> +
>> +The 'event-data-type' defines the type of data which shell be counted
>> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
>> +all possible type, i.e. count read requests, count write data in bytes,
>> +etc. This field is optional and when it is missing, the driver code will
>> +use default data type.
>>
> 
> How about editing it as following?
> 
> --- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
> @@ -10,14 +10,23 @@ The Exynos PPMU driver uses the devfreq-event class to provide event data
>   to various devfreq devices. The devfreq devices would use the event data when
>   derterming the current state of each IP.
>   
> -Required properties:
> +Required properties for PPMU device:
>   - compatible: Should be "samsung,exynos-ppmu" or "samsung,exynos-ppmu-v2.
>   - reg: physical base address of each PPMU and length of memory mapped region.
>   
> -Optional properties:
> +Optional properties for PPMU device:
>   - clock-names : the name of clock used by the PPMU, "ppmu"
>   - clocks : phandles for clock specified in "clock-names" property
>   
> +Required properties for 'events' child node of PPMU device:
> +- event-name : the unique event name among PPMU device
> +Optional properties for 'events' child node of PPMU device:
> +- event-data-type : Define the type of data which shell be counted
> +by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
> +all possible type, i.e. count read requests, count write data in bytes,
> +etc. This field is optional and when it is missing, the driver code
> +will use default data type.
> +
>   Example1 : PPMUv1 nodes in exynos3250.dtsi are listed below.
>   
>                  ppmu_dmc0: ppmu_dmc0@106a0000 {
> @@ -145,3 +154,16 @@ Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
>                          reg = <0x104d0000 0x2000>;
>                          status = "disabled";
>                  };
> +
> +Example4 : 'event-data-type' in exynos4412-ppmu-common.dtsi are listed below.
> +
> +       &ppmu_dmc0 {
> +               status = "okay";
> +               events {
> +                       ppmu_dmc0_3: ppmu-event3-dmc0 {
> +                               event-name = "ppmu-event3-dmc0";
> +                               event-data-type = <(PPMU_RO_DATA_CNT |
> +                                               PPMU_WO_DATA_CNT)>;
> +                       };
> +               };
> +       };
> 
> 
I will also add your Signed-off-by to this patch, similar to what we
have agreed for patch 'PATCH v3 2/4'.

Regards,
Lukasz
Rob Herring May 2, 2019, 8:24 p.m. UTC | #6
On Thu, May 2, 2019 at 3:52 AM Lukasz Luba <l.luba@partner.samsung.com> wrote:
>
> Hi Rob,
>
> On 5/1/19 12:36 AM, Rob Herring wrote:
> > On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
> >> Extend the documenation by events description with new 'event-data-type'
> >> field. Add example how the event might be defined in DT.
> >
> > Why do we need event types in DT? We don't do this for other h/w such as
> > ARM PMU.
> In ARM PMU all the events are hard-coded into the driver code i.e. in v7
> arch/arm/kernel/perf_event_v7.c
> and are seen from perf. They are different type and for different
> purpose. The Ecynos PPMU events are not seen in perf, they are
> for internal monitoring and must not be reset by other actors like perf.
> They are used by the 'bus drivers' to made some heuristics and tune the
> internal settings, like frequency.
>
> Chanwoo has written PPMU driver which relies on DT definition.
> The DT events are used by other DT devices by phandle.

How is that done? I don't see anything in the binding for that.

Rob
Lukasz Luba May 6, 2019, 10:29 a.m. UTC | #7
Hi Rob,

On 5/2/19 10:24 PM, Rob Herring wrote:
> On Thu, May 2, 2019 at 3:52 AM Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>
>> Hi Rob,
>>
>> On 5/1/19 12:36 AM, Rob Herring wrote:
>>> On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
>>>> Extend the documenation by events description with new 'event-data-type'
>>>> field. Add example how the event might be defined in DT.
>>>
>>> Why do we need event types in DT? We don't do this for other h/w such as
>>> ARM PMU.
>> In ARM PMU all the events are hard-coded into the driver code i.e. in v7
>> arch/arm/kernel/perf_event_v7.c
>> and are seen from perf. They are different type and for different
>> purpose. The Ecynos PPMU events are not seen in perf, they are
>> for internal monitoring and must not be reset by other actors like perf.
>> They are used by the 'bus drivers' to made some heuristics and tune the
>> internal settings, like frequency.
>>
>> Chanwoo has written PPMU driver which relies on DT definition.
>> The DT events are used by other DT devices by phandle.
> 
> How is that done? I don't see anything in the binding for that.
Here are the DT devices and how they are pinned together:
- declared devfreq events:
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-ppmu-common.dtsi
- devfreq events pinned to the bus device:
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-odroid-common.dtsi#L107
- the bus device itself:
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412.dtsi#L457

Regards,
Lukasz

> 
> Rob
> 
>
Lukasz Luba May 7, 2019, 9:05 a.m. UTC | #8
Hi Rob,

On 5/6/19 12:29 PM, Lukasz Luba wrote:
> Hi Rob,
> 
> On 5/2/19 10:24 PM, Rob Herring wrote:
>> On Thu, May 2, 2019 at 3:52 AM Lukasz Luba 
>> <l.luba@partner.samsung.com> wrote:
>>>
>>> Hi Rob,
>>>
>>> On 5/1/19 12:36 AM, Rob Herring wrote:
>>>> On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
>>>>> Extend the documenation by events description with new 
>>>>> 'event-data-type'
>>>>> field. Add example how the event might be defined in DT.
>>>>
>>>> Why do we need event types in DT? We don't do this for other h/w 
>>>> such as
>>>> ARM PMU.
>>> In ARM PMU all the events are hard-coded into the driver code i.e. in v7
>>> arch/arm/kernel/perf_event_v7.c
>>> and are seen from perf. They are different type and for different
>>> purpose. The Ecynos PPMU events are not seen in perf, they are
>>> for internal monitoring and must not be reset by other actors like perf.
>>> They are used by the 'bus drivers' to made some heuristics and tune the
>>> internal settings, like frequency.
>>>
>>> Chanwoo has written PPMU driver which relies on DT definition.
>>> The DT events are used by other DT devices by phandle.
>>
>> How is that done? I don't see anything in the binding for that.
> Here are the DT devices and how they are pinned together:
> - declared devfreq events:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-ppmu-common.dtsi 
> 
> - devfreq events pinned to the bus device:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-odroid-common.dtsi#L107 
> 
> - the bus device itself:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412.dtsi#L457 
> 
> 
> Regards,
> Lukasz
> 
>>
>> Rob
>>
>>
Can I send the next version of the patch set, which has Chanwoo's
suggestions, or do you have some objections to this PPMU entries?

Regards,
Lukasz
Rob Herring May 7, 2019, 4:50 p.m. UTC | #9
On Tue, May 7, 2019 at 4:05 AM Lukasz Luba <l.luba@partner.samsung.com> wrote:
>
> Hi Rob,
>
> On 5/6/19 12:29 PM, Lukasz Luba wrote:
> > Hi Rob,
> >
> > On 5/2/19 10:24 PM, Rob Herring wrote:
> >> On Thu, May 2, 2019 at 3:52 AM Lukasz Luba
> >> <l.luba@partner.samsung.com> wrote:
> >>>
> >>> Hi Rob,
> >>>
> >>> On 5/1/19 12:36 AM, Rob Herring wrote:
> >>>> On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
> >>>>> Extend the documenation by events description with new
> >>>>> 'event-data-type'
> >>>>> field. Add example how the event might be defined in DT.
> >>>>
> >>>> Why do we need event types in DT? We don't do this for other h/w
> >>>> such as
> >>>> ARM PMU.
> >>> In ARM PMU all the events are hard-coded into the driver code i.e. in v7
> >>> arch/arm/kernel/perf_event_v7.c
> >>> and are seen from perf. They are different type and for different
> >>> purpose. The Ecynos PPMU events are not seen in perf, they are
> >>> for internal monitoring and must not be reset by other actors like perf.
> >>> They are used by the 'bus drivers' to made some heuristics and tune the
> >>> internal settings, like frequency.
> >>>
> >>> Chanwoo has written PPMU driver which relies on DT definition.
> >>> The DT events are used by other DT devices by phandle.
> >>
> >> How is that done? I don't see anything in the binding for that.
> > Here are the DT devices and how they are pinned together:
> > - declared devfreq events:
> > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-ppmu-common.dtsi
> >
> > - devfreq events pinned to the bus device:
> > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-odroid-common.dtsi#L107
> >
> > - the bus device itself:
> > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412.dtsi#L457
> >
> >
> > Regards,
> > Lukasz
> >
> >>
> >> Rob
> >>
> >>
> Can I send the next version of the patch set, which has Chanwoo's
> suggestions, or do you have some objections to this PPMU entries?

I think the existing binding which this is based on needs some
changes, so it's pointless really for me to comment on additions.

Rob
Lukasz Luba May 8, 2019, 8:28 a.m. UTC | #10
Hi Rob,

On 5/7/19 6:50 PM, Rob Herring wrote:
> On Tue, May 7, 2019 at 4:05 AM Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>
>> Hi Rob,
>>
>> On 5/6/19 12:29 PM, Lukasz Luba wrote:
>>> Hi Rob,
>>>
>>> On 5/2/19 10:24 PM, Rob Herring wrote:
>>>> On Thu, May 2, 2019 at 3:52 AM Lukasz Luba
>>>> <l.luba@partner.samsung.com> wrote:
>>>>>
>>>>> Hi Rob,
>>>>>
>>>>> On 5/1/19 12:36 AM, Rob Herring wrote:
>>>>>> On Fri, Apr 19, 2019 at 03:48:07PM +0200, Lukasz Luba wrote:
>>>>>>> Extend the documenation by events description with new
>>>>>>> 'event-data-type'
>>>>>>> field. Add example how the event might be defined in DT.
>>>>>>
>>>>>> Why do we need event types in DT? We don't do this for other h/w
>>>>>> such as
>>>>>> ARM PMU.
>>>>> In ARM PMU all the events are hard-coded into the driver code i.e. in v7
>>>>> arch/arm/kernel/perf_event_v7.c
>>>>> and are seen from perf. They are different type and for different
>>>>> purpose. The Ecynos PPMU events are not seen in perf, they are
>>>>> for internal monitoring and must not be reset by other actors like perf.
>>>>> They are used by the 'bus drivers' to made some heuristics and tune the
>>>>> internal settings, like frequency.
>>>>>
>>>>> Chanwoo has written PPMU driver which relies on DT definition.
>>>>> The DT events are used by other DT devices by phandle.
>>>>
>>>> How is that done? I don't see anything in the binding for that.
>>> Here are the DT devices and how they are pinned together:
>>> - declared devfreq events:
>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-ppmu-common.dtsi
>>>
>>> - devfreq events pinned to the bus device:
>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412-odroid-common.dtsi#L107
>>>
>>> - the bus device itself:
>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/exynos4412.dtsi#L457
>>>
>>>
>>> Regards,
>>> Lukasz
>>>
>>>>
>>>> Rob
>>>>
>>>>
>> Can I send the next version of the patch set, which has Chanwoo's
>> suggestions, or do you have some objections to this PPMU entries?
> 
> I think the existing binding which this is based on needs some
> changes, so it's pointless really for me to comment on additions.
Maybe the bindings description is not perfect, but it contains
examples which might help clarifying the idea.
Regarding the real value of the patch set, it is needed for some
research. Currently, the Odroid xu3/4 is the best 'mainline' platform
with big.LITTLE, has good performance and these counters.
Willy, who is doing his PhD, wants to experiment with it. I agree that
it could be better documented but it requires more effort.
I will extend the documentation it in my free time slots (it is not my
main task for now).

For now, I have received comments and parts of code from Chanwoo which
made the next patch set mature and ready to merge (IMHO).

If you have suggestions how to improve these bindings or links for good
examples, I would really appreciate. Thank you for your time and review.

Regards,
Lukasz

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
index 3e36c1d..47feb5f 100644
--- a/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
+++ b/Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
@@ -145,3 +145,21 @@  Example3 : PPMUv2 nodes in exynos5433.dtsi are listed below.
 			reg = <0x104d0000 0x2000>;
 			status = "disabled";
 		};
+
+The 'event' type specified in the PPMU node defines 'event-name'
+which also contains 'id' number and optionally 'event-data-type'.
+
+Example:
+
+		events {
+			ppmu_leftbus_0: ppmu-event0-leftbus {
+				event-name = "ppmu-event0-leftbus";
+				event-data-type = <PPMU_RO_DATA_CNT>;
+			};
+		};
+
+The 'event-data-type' defines the type of data which shell be counted
+by the counter. You can check include/dt-bindings/pmu/exynos_ppmu.h for
+all possible type, i.e. count read requests, count write data in bytes,
+etc. This field is optional and when it is missing, the driver code will
+use default data type.