| Message ID | 1555681688-19643-4-git-send-email-l.luba@partner.samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v3,1/4] include: dt-bindings: add Performance Monitoring Unit for Exynos | expand |
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)>; + }; + }; + };
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
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 >
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 >> > >
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
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
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 > >
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
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
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
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.
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(+)