Hi Georgi,
On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote:
Hi Artur,
On 7/23/19 15:20, Artur Świgoń wrote:
This patch adds interconnect functionality to the exynos-bus devfreq driver.
The SoC topology is a graph (or, more specifically, a tree) and most of its edges are taken from the devfreq parent-child hierarchy (cf. Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous patch adds missing edges to the DT (under the name 'parent'). Due to unspecified relative probing order, -EPROBE_DEFER may be propagated to guarantee that a child is probed before its parent.
Each bus is now an interconnect provider and an interconnect node as well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers itself as a node. Node IDs are not hardcoded but rather assigned at runtime, in probing order (subject to the above-mentioned exception regarding relative order). This approach allows for using this driver with various Exynos SoCs.
I am not familiar with the Exynos bus topology, but it seems to me that it's not represented correctly. An interconnect provider with just a single node (port) is odd. I would expect that each provider consists of multiple master and slave nodes. This data would be used by a framework to understand what are the links and how the traffic flows between the IP blocks and through which buses.
To summarize the exynos-bus topology[1] used by the devfreq driver: There are many data buses for data transfer in Samsung Exynos SoC. Every bus has its own clock. Buses often share power lines, in which case one of the buses on the power line is referred to as 'parent' (or as 'devfreq' in the DT). In the particular case of Exynos4412[1][2], the topology can be expressed as follows:
bus_dmc -- bus_acp -- bus_c2c
bus_leftbus -- bus_rightbus -- bus_display -- bus_fsys -- bus_peri -- bus_mfc
Where bus_dmc and bus_leftbus probably could be referred to as masters, and the following indented nodes as slaves. Patch 08/11 of this RFC additionally adds the following to the DT:
bus_dmc -- bus_leftbus
Which makes the topology a valid tree.
The exynos-bus concept in devfreq[3] is designed in such a way that every bus is probed separately as a platform device, and is a largely independent entity. This RFC proposes an extension to the existing devfreq driver that basically provides a simple QoS to ensure minimum clock frequency for selected buses (possibly overriding devfreq governor calculations) using the interconnect framework.
The hierarchy is modelled in such a way that every bus is an interconnect node. On the other hand, what is considered an interconnect provider here is quite arbitrary, but for the reasons mentioned in the above paragraph, this RFC assumes that every bus is a provider of itself as a node. Using an alternative singleton provider approach was deemed more complicated since the 'dev' field in 'struct icc_provider' has to be set to something meaningful and we are tied to the 'samsung,exynos-bus' compatible string in the driver (and multiple instances of exynos-bus probed in indeterminate relative order).
I'm looking forward to hearing any additional thoughts you may have on this topic.
Best regards,
Hi Artur,
On 8/1/19 10:59, Artur Świgoń wrote:
Hi Georgi,
On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote:
Hi Artur,
On 7/23/19 15:20, Artur Świgoń wrote:
This patch adds interconnect functionality to the exynos-bus devfreq driver.
The SoC topology is a graph (or, more specifically, a tree) and most of its edges are taken from the devfreq parent-child hierarchy (cf. Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous patch adds missing edges to the DT (under the name 'parent'). Due to unspecified relative probing order, -EPROBE_DEFER may be propagated to guarantee that a child is probed before its parent.
Each bus is now an interconnect provider and an interconnect node as well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers itself as a node. Node IDs are not hardcoded but rather assigned at runtime, in probing order (subject to the above-mentioned exception regarding relative order). This approach allows for using this driver with various Exynos SoCs.
I am not familiar with the Exynos bus topology, but it seems to me that it's not represented correctly. An interconnect provider with just a single node (port) is odd. I would expect that each provider consists of multiple master and slave nodes. This data would be used by a framework to understand what are the links and how the traffic flows between the IP blocks and through which buses.
To summarize the exynos-bus topology[1] used by the devfreq driver: There are many data buses for data transfer in Samsung Exynos SoC. Every bus has its own clock. Buses often share power lines, in which case one of the buses on the power line is referred to as 'parent' (or as 'devfreq' in the DT). In the particular case of Exynos4412[1][2], the topology can be expressed as follows:
bus_dmc -- bus_acp -- bus_c2c
bus_leftbus -- bus_rightbus -- bus_display -- bus_fsys -- bus_peri -- bus_mfc
Where bus_dmc and bus_leftbus probably could be referred to as masters, and the following indented nodes as slaves. Patch 08/11 of this RFC additionally adds the following to the DT:
bus_dmc -- bus_leftbus
Which makes the topology a valid tree.
The exynos-bus concept in devfreq[3] is designed in such a way that every bus is probed separately as a platform device, and is a largely independent entity.
This RFC proposes an extension to the existing devfreq driver that basically provides a simple QoS to ensure minimum clock frequency for selected buses (possibly overriding devfreq governor calculations) using the interconnect framework.
The hierarchy is modelled in such a way that every bus is an interconnect node. On the other hand, what is considered an interconnect provider here is quite arbitrary, but for the reasons mentioned in the above paragraph, this RFC assumes that every bus is a provider of itself as a node. Using an alternative
IIUC, in case we want to transfer data between the display and the memory controller, the path would look like this:
display --> bus_display --> bus_leftbus --> bus_dmc --> memory
But the bus_display for example would have not one, but two nodes (ports), right? One representing the link to the display controller and another one representing the link to bus_leftbus? So i think that all the buses should have at least two nodes, to represent each end of the wire.
singleton provider approach was deemed more complicated since the 'dev' field in 'struct icc_provider' has to be set to something meaningful and we are tied to the 'samsung,exynos-bus' compatible string in the driver (and multiple instances of exynos-bus probed in indeterminate relative order).
Sure, the rest makes sense to me.
Thanks, Georgi
Hi,
On Wed, 2019-08-07 at 17:21 +0300, Georgi Djakov wrote:
Hi Artur,
On 8/1/19 10:59, Artur Świgoń wrote:
Hi Georgi,
On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote:
Hi Artur,
On 7/23/19 15:20, Artur Świgoń wrote:
This patch adds interconnect functionality to the exynos-bus devfreq driver.
The SoC topology is a graph (or, more specifically, a tree) and most of its edges are taken from the devfreq parent-child hierarchy (cf. Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous patch adds missing edges to the DT (under the name 'parent'). Due to unspecified relative probing order, -EPROBE_DEFER may be propagated to guarantee that a child is probed before its parent.
Each bus is now an interconnect provider and an interconnect node as well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers itself as a node. Node IDs are not hardcoded but rather assigned at runtime, in probing order (subject to the above-mentioned exception regarding relative order). This approach allows for using this driver with various Exynos SoCs.
I am not familiar with the Exynos bus topology, but it seems to me that it's not represented correctly. An interconnect provider with just a single node (port) is odd. I would expect that each provider consists of multiple master and slave nodes. This data would be used by a framework to understand what are the links and how the traffic flows between the IP blocks and through which buses.
To summarize the exynos-bus topology[1] used by the devfreq driver: There are many data buses for data transfer in Samsung Exynos SoC. Every bus has its own clock. Buses often share power lines, in which case one of the buses on the power line is referred to as 'parent' (or as 'devfreq' in the DT). In the particular case of Exynos4412[1][2], the topology can be expressed as follows:
bus_dmc -- bus_acp -- bus_c2c
bus_leftbus -- bus_rightbus -- bus_display -- bus_fsys -- bus_peri -- bus_mfc
Where bus_dmc and bus_leftbus probably could be referred to as masters, and the following indented nodes as slaves. Patch 08/11 of this RFC additionally adds the following to the DT:
bus_dmc -- bus_leftbus
Which makes the topology a valid tree.
The exynos-bus concept in devfreq[3] is designed in such a way that every bus is probed separately as a platform device, and is a largely independent entity.
This RFC proposes an extension to the existing devfreq driver that basically provides a simple QoS to ensure minimum clock frequency for selected buses (possibly overriding devfreq governor calculations) using the interconnect framework.
The hierarchy is modelled in such a way that every bus is an interconnect node. On the other hand, what is considered an interconnect provider here is quite arbitrary, but for the reasons mentioned in the above paragraph, this RFC assumes that every bus is a provider of itself as a node. Using an alternative
IIUC, in case we want to transfer data between the display and the memory controller, the path would look like this:
display --> bus_display --> bus_leftbus --> bus_dmc --> memory
But the bus_display for example would have not one, but two nodes (ports), right? One representing the link to the display controller and another one representing the link to bus_leftbus? So i think that all the buses should have at least two nodes, to represent each end of the wire.
I do not think we really need that for our simple tree hierarchy. Of course, I can split every tree node into two nodes/ports (e.g., 'in' for children and 'out' for parent), but neither 'display' nor 'memory' from your diagram above are registered with the interconnect framework (only buses are). The devfreq devices used in the driver are virtual anyway.
singleton provider approach was deemed more complicated since the 'dev' field in 'struct icc_provider' has to be set to something meaningful and we are tied to the 'samsung,exynos-bus' compatible string in the driver (and multiple instances of exynos-bus probed in indeterminate relative order).
Sure, the rest makes sense to me.
Thanks, Georgi
Regards,
dri-devel@lists.freedesktop.org
-
Artur Świgoń -
Georgi Djakov