| Message ID | 20201030125149.8227-1-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Exynos: Simple QoS for exynos-bus using interconnect | expand |
Hi Sylwester,
When I tested this patchset on Odroid-U3,
After setting 0 bps by interconnect[1][2],
the frequency of devfreq devs sustain the high frequency
according to the pm qos request.
So, I try to find the cause of this situation.
In result, it seems that interconnect exynos driver
updates the pm qos request to devfreq device
during the kernel booting. Do you know why the exynos
interconnect driver request the pm qos during probe
without the mixer request?
PS. The passive governor has a bug related to PM_QOS interface.
So, I posted the patch[4].
[1] interconnect_graph
root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph
digraph {
rankdir = LR
node [shape = record]
subgraph cluster_1 {
label = "soc:bus_dmc"
"2:bus_dmc" [label="2:bus_dmc
|avg_bw=0kBps
|peak_bw=0kBps"]
}
subgraph cluster_2 {
label = "soc:bus_leftbus"
"3:bus_leftbus" [label="3:bus_leftbus
|avg_bw=0kBps
|peak_bw=0kBps"]
}
subgraph cluster_3 {
label = "soc:bus_display"
"4:bus_display" [label="4:bus_display
|avg_bw=0kBps
|peak_bw=0kBps"]
}
"3:bus_leftbus" -> "2:bus_dmc"
"4:bus_display" -> "3:bus_leftbus"
[2] interconnect_summary
root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary
node tag avg peak
--------------------------------------------------------------------
bus_dmc 0 0
12c10000.mixer 0 0 0
bus_leftbus 0 0
12c10000.mixer 0 0 0
bus_display 0 0
12c10000.mixer 0 0 0
[3] devfreq_summary
root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary
dev parent_dev governor timer polling_ms cur_freq_Hz min_freq_Hz max_freq_Hz
------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------
soc:bus_dmc null simple_ondemand deferrable 50 400000000 400000000 400000000
soc:bus_acp soc:bus_dmc passive null 0 267000000 100000000 267000000
soc:bus_c2c soc:bus_dmc passive null 0 400000000 100000000 400000000
soc:bus_leftbus null simple_ondemand deferrable 50 200000000 200000000 200000000
soc:bus_rightbus soc:bus_leftbus passive null 0 200000000 100000000 200000000
soc:bus_display soc:bus_leftbus passive null 0 200000000 200000000 200000000
soc:bus_fsys soc:bus_leftbus passive null 0 134000000 100000000 134000000
soc:bus_peri soc:bus_leftbus passive null 0 100000000 50000000 100000000
soc:bus_mfc soc:bus_leftbus passive null 0 200000000 100000000 200000000
[4] PM / devfreq: passive: Update frequency when start governor
https://patchwork.kernel.org/project/linux-pm/patch/20201103070646.18687-1-cw00.choi@samsung.com/
On 10/30/20 9:51 PM, Sylwester Nawrocki wrote:
>
> This patchset adds interconnect API support for the Exynos SoC "samsung,
> exynos-bus" compatible devices, which already have their corresponding
> exynos-bus driver in the devfreq subsystem. Complementing the devfreq
> driver with an interconnect functionality allows to ensure the QoS
> requirements of devices accessing the system memory (e.g. video processing
> devices) are fulfilled and aallows to avoid issues like the one discussed
> in thread [1].
>
> This patch series adds implementation of the interconnect provider per each
> "samsung,exynos-bus" compatible DT node, with one interconnect node per
> provider. The interconnect code which was previously added as a part of
> the devfreq driver has been converted to a separate platform driver.
> In the devfreq a corresponding virtual child platform device is registered.
> Integration of devfreq and interconnect frameworks is achieved through
> the PM QoS API.
>
> A sample interconnect consumer for exynos-mixer is added in patches 5/6,
> 6/6, it is currently added only for exynos4412 and allows to address the
> mixer DMA underrun error issues [1].
>
> Changes since v6:
> - the interconnect consumer DT bindings are now used to describe dependencies
> of the interconnects (samsung,exynos-bus nodes),
> - bus-width property replaced with samsung,data-clk-ratio,
> - adaptation to recent changes in the interconnect code
> (of_icc_get_from_provider(), icc_node_add()).
>
> The series has been tested on Odroid U3 board. It is based on v5.10-rc1.
>
> --
> Regards,
> Sylwester
>
>
> Changes since v5:
> - addition of "bus-width: DT property, which specifies data width
> of the interconnect bus (patches 1...2/6),
> - addition of synchronization of the interconnect bandwidth setting
> with VSYNC (patch 6/6).
>
> Changes since v3 [4] (v4 skipped to align with patchset [1]), detailed
> changes are listed in each patch:
> - conversion to a separate interconnect (platform) driver,
> - an update of the DT binding documenting new optional properties:
> #interconnect-cells, samsung,interconnect-parent in "samsung,exynos-bus"
> nodes,
> - new DT properties added to the SoC, rather than to the board specific
> files.
>
> Changes since v2 [5]:
> - Use icc_std_aggregate().
> - Implement a different modification of apply_constraints() in
> drivers/interconnect/core.c (patch 03).
> - Use 'exynos,interconnect-parent-node' in the DT instead of
> 'devfreq'/'parent', depending on the bus.
> - Rebase on DT patches that deprecate the 'devfreq' DT property.
> - Improve error handling, including freeing generated IDs on failure.
> - Remove exynos_bus_icc_connect() and add exynos_bus_icc_get_parent().
>
> Changes since v1 [6]:
> - Rebase on coupled regulators patches.
> - Use dev_pm_qos_*() API instead of overriding frequency in
> exynos_bus_target().
> - Use IDR for node ID allocation.
> - Reverse order of multiplication and division in
> mixer_set_memory_bandwidth() (patch 07) to avoid integer overflow.
>
>
> References:
> [1] https://patchwork.kernel.org/patch/10861757/ (original issue)
> [2] https://protect2.fireeye.com/v1/url?k=383efc40-67a5c559-383f770f-000babff3793-a505fcd0b7477e5e&q=1&e=ad8ffb9f-f90b-49a7-a3df-2ab066a8c4ee&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-samsung-soc%2Fmsg70014.html
> [3] https://protect2.fireeye.com/v1/url?k=13f0c488-4c6bfd91-13f14fc7-000babff3793-98a59bf1c5c6f1fb&q=1&e=ad8ffb9f-f90b-49a7-a3df-2ab066a8c4ee&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg810722.html
> [4] https://lore.kernel.org/linux-pm/20191220115653.6487-1-a.swigon@samsung.com
> [5] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC)
> [6] https://patchwork.kernel.org/cover/11152595/ (v2 of this RFC)
>
>
> Artur Świgoń (1):
> ARM: dts: exynos: Add interconnects to Exynos4412 mixer
>
> Sylwester Nawrocki (5):
> dt-bindings: devfreq: Add documentation for the interconnect
> properties
> interconnect: Add generic interconnect driver for Exynos SoCs
> PM / devfreq: exynos-bus: Add registration of interconnect child
> device
> ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes
> drm: exynos: mixer: Add interconnect support
>
> .../devicetree/bindings/devfreq/exynos-bus.txt | 68 ++++++-
> arch/arm/boot/dts/exynos4412.dtsi | 7 +
> drivers/devfreq/exynos-bus.c | 17 ++
> drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++-
> drivers/interconnect/Kconfig | 1 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/exynos/Kconfig | 6 +
> drivers/interconnect/exynos/Makefile | 4 +
> drivers/interconnect/exynos/exynos.c | 198 +++++++++++++++++++++
> 9 files changed, 438 insertions(+), 10 deletions(-)
> create mode 100644 drivers/interconnect/exynos/Kconfig
> create mode 100644 drivers/interconnect/exynos/Makefile
> create mode 100644 drivers/interconnect/exynos/exynos.c
>
> --
> 2.7.4
>
>
>
Hi Chanwoo and Sylwester, On 11/3/20 09:54, Chanwoo Choi wrote: > Hi Sylwester, > > When I tested this patchset on Odroid-U3, > After setting 0 bps by interconnect[1][2], > the frequency of devfreq devs sustain the high frequency > according to the pm qos request. > > So, I try to find the cause of this situation. > In result, it seems that interconnect exynos driver > updates the pm qos request to devfreq device > during the kernel booting. Do you know why the exynos > interconnect driver request the pm qos during probe > without the mixer request? That's probably because of the sync_state support, that was introduced recently. The icc_sync_state callback needs to be added to the driver (i just left a comment on that patch), and then check again if it works. The idea of the sync_state is that there could be multiple users of a path and we must wait for all consumers to tell their bandwidth needs. Otherwise the first consumer may lower the bandwidth or disable a path needed for another consumer (driver), which has not probed yet. So we maintain a floor bandwidth until everyone has probed. By default the floor bandwidth is INT_MAX, but can be overridden by implementing the get_bw() callback. Thanks, Georgi > > PS. The passive governor has a bug related to PM_QOS interface. > So, I posted the patch[4]. > > > [1] interconnect_graph > root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph > digraph { > rankdir = LR > node [shape = record] > subgraph cluster_1 { > label = "soc:bus_dmc" > "2:bus_dmc" [label="2:bus_dmc > |avg_bw=0kBps > |peak_bw=0kBps"] > } > subgraph cluster_2 { > label = "soc:bus_leftbus" > "3:bus_leftbus" [label="3:bus_leftbus > |avg_bw=0kBps > |peak_bw=0kBps"] > } > subgraph cluster_3 { > label = "soc:bus_display" > "4:bus_display" [label="4:bus_display > |avg_bw=0kBps > |peak_bw=0kBps"] > } > "3:bus_leftbus" -> "2:bus_dmc" > "4:bus_display" -> "3:bus_leftbus" > > > [2] interconnect_summary > root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary > node tag avg peak > -------------------------------------------------------------------- > bus_dmc 0 0 > 12c10000.mixer 0 0 0 > bus_leftbus 0 0 > 12c10000.mixer 0 0 0 > bus_display 0 0 > 12c10000.mixer 0 0 0 > > > [3] devfreq_summary > root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary > dev parent_dev governor timer polling_ms cur_freq_Hz min_freq_Hz max_freq_Hz > ------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------ > soc:bus_dmc null simple_ondemand deferrable 50 400000000 400000000 400000000 > soc:bus_acp soc:bus_dmc passive null 0 267000000 100000000 267000000 > soc:bus_c2c soc:bus_dmc passive null 0 400000000 100000000 400000000 > soc:bus_leftbus null simple_ondemand deferrable 50 200000000 200000000 200000000 > soc:bus_rightbus soc:bus_leftbus passive null 0 200000000 100000000 200000000 > soc:bus_display soc:bus_leftbus passive null 0 200000000 200000000 200000000 > soc:bus_fsys soc:bus_leftbus passive null 0 134000000 100000000 134000000 > soc:bus_peri soc:bus_leftbus passive null 0 100000000 50000000 100000000 > soc:bus_mfc soc:bus_leftbus passive null 0 200000000 100000000 200000000 > > > [4] PM / devfreq: passive: Update frequency when start governor > https://patchwork.kernel.org/project/linux-pm/patch/20201103070646.18687-1-cw00.choi@samsung.com/ > >
Hi Georgi, On 11/3/20 5:29 PM, Georgi Djakov wrote: > Hi Chanwoo and Sylwester, > > On 11/3/20 09:54, Chanwoo Choi wrote: >> Hi Sylwester, >> >> When I tested this patchset on Odroid-U3, >> After setting 0 bps by interconnect[1][2], >> the frequency of devfreq devs sustain the high frequency >> according to the pm qos request. >> >> So, I try to find the cause of this situation. >> In result, it seems that interconnect exynos driver >> updates the pm qos request to devfreq device >> during the kernel booting. Do you know why the exynos >> interconnect driver request the pm qos during probe >> without the mixer request? > > That's probably because of the sync_state support, that was introduced > recently. The icc_sync_state callback needs to be added to the driver > (i just left a comment on that patch), and then check again if it works. > > The idea of the sync_state is that there could be multiple users of a > path and we must wait for all consumers to tell their bandwidth needs. > Otherwise the first consumer may lower the bandwidth or disable a path > needed for another consumer (driver), which has not probed yet. So we > maintain a floor bandwidth until everyone has probed. By default the floor > bandwidth is INT_MAX, but can be overridden by implementing the get_bw() > callback. Thanks for guide. I tested it with your comment of patch2. It is well working without problem as I mentioned previously. I caught the reset operation of PM QoS requested from interconnect on kernel log. In result, after completed the kernel booting, there is no pm qos request if hdmi cable is not connected. [Test Result] 1. Set 622080 Bps with HDMI cable root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary; dev parent_dev governor timer polling_ms cur_freq_Hz min_freq_Hz max_freq_Hz ------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------ soc:bus_dmc null simple_ondemand deferrable 50 400000000 155520000 400000000 soc:bus_acp soc:bus_dmc passive null 0 267000000 100000000 267000000 soc:bus_c2c soc:bus_dmc passive null 0 400000000 100000000 400000000 soc:bus_leftbus null simple_ondemand deferrable 50 200000000 100000000 200000000 soc:bus_rightbus soc:bus_leftbus passive null 0 200000000 100000000 200000000 soc:bus_display soc:bus_leftbus passive null 0 200000000 160000000 200000000 soc:bus_fsys soc:bus_leftbus passive null 0 134000000 100000000 134000000 soc:bus_peri soc:bus_leftbus passive null 0 100000000 50000000 100000000 soc:bus_mfc soc:bus_leftbus passive null 0 200000000 100000000 200000000 root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph; digraph { rankdir = LR node [shape = record] subgraph cluster_1 { label = "soc:bus_dmc" "2:bus_dmc" [label="2:bus_dmc |avg_bw=622080kBps |peak_bw=622080kBps"] } subgraph cluster_2 { label = "soc:bus_leftbus" "3:bus_leftbus" [label="3:bus_leftbus |avg_bw=622080kBps |peak_bw=622080kBps"] } subgraph cluster_3 { label = "soc:bus_display" "4:bus_display" [label="4:bus_display |avg_bw=622080kBps |peak_bw=622080kBps"] } "3:bus_leftbus" -> "2:bus_dmc" "4:bus_display" -> "3:bus_leftbus" }root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary; node tag avg peak -------------------------------------------------------------------- bus_dmc 622080 622080 12c10000.mixer 0 622080 622080 bus_leftbus 622080 622080 12c10000.mixer 0 622080 622080 bus_display 622080 622080 12c10000.mixer 0 622080 622080 2. Set 0Bps without HDMI cable root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary; dev parent_dev governor timer polling_ms cur_freq_Hz min_freq_Hz max_freq_Hz ------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------ soc:bus_dmc null simple_ondemand deferrable 50 100000000 100000000 400000000 soc:bus_acp soc:bus_dmc passive null 0 100000000 100000000 267000000 soc:bus_c2c soc:bus_dmc passive null 0 100000000 100000000 400000000 soc:bus_leftbus null simple_ondemand deferrable 50 100000000 100000000 200000000 soc:bus_rightbus soc:bus_leftbus passive null 0 100000000 100000000 200000000 soc:bus_display soc:bus_leftbus passive null 0 160000000 160000000 200000000 soc:bus_fsys soc:bus_leftbus passive null 0 100000000 100000000 134000000 soc:bus_peri soc:bus_leftbus passive null 0 50000000 50000000 100000000 soc:bus_mfc soc:bus_leftbus passive null 0 100000000 100000000 200000000 root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph; digraph { rankdir = LR node [shape = record] subgraph cluster_1 { label = "soc:bus_dmc" "2:bus_dmc" [label="2:bus_dmc |avg_bw=0kBps |peak_bw=0kBps"] } subgraph cluster_2 { label = "soc:bus_leftbus" "3:bus_leftbus" [label="3:bus_leftbus |avg_bw=0kBps |peak_bw=0kBps"] } subgraph cluster_3 { label = "soc:bus_display" "4:bus_display" [label="4:bus_display |avg_bw=0kBps |peak_bw=0kBps"] } "3:bus_leftbus" -> "2:bus_dmc" "4:bus_display" -> "3:bus_leftbus" }root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary; node tag avg peak -------------------------------------------------------------------- bus_dmc 0 0 12c10000.mixer 0 0 0 bus_leftbus 0 0 12c10000.mixer 0 0 0 bus_display 0 0 12c10000.mixer 0 0 0 Thanks, Chanwoo Choi > > Thanks, > Georgi > >> >> PS. The passive governor has a bug related to PM_QOS interface. >> So, I posted the patch[4]. >> >> >> [1] interconnect_graph >> root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph >> digraph { >> rankdir = LR >> node [shape = record] >> subgraph cluster_1 { >> label = "soc:bus_dmc" >> "2:bus_dmc" [label="2:bus_dmc >> |avg_bw=0kBps >> |peak_bw=0kBps"] >> } >> subgraph cluster_2 { >> label = "soc:bus_leftbus" >> "3:bus_leftbus" [label="3:bus_leftbus >> |avg_bw=0kBps >> |peak_bw=0kBps"] >> } >> subgraph cluster_3 { >> label = "soc:bus_display" >> "4:bus_display" [label="4:bus_display >> |avg_bw=0kBps >> |peak_bw=0kBps"] >> } >> "3:bus_leftbus" -> "2:bus_dmc" >> "4:bus_display" -> "3:bus_leftbus" >> >> >> [2] interconnect_summary >> root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary >> node tag avg peak >> -------------------------------------------------------------------- >> bus_dmc 0 0 >> 12c10000.mixer 0 0 0 >> bus_leftbus 0 0 >> 12c10000.mixer 0 0 0 >> bus_display 0 0 >> 12c10000.mixer 0 0 0 >> >> >> [3] devfreq_summary >> root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary >> dev parent_dev governor timer polling_ms cur_freq_Hz min_freq_Hz max_freq_Hz >> ------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------ >> soc:bus_dmc null simple_ondemand deferrable 50 400000000 400000000 400000000 >> soc:bus_acp soc:bus_dmc passive null 0 267000000 100000000 267000000 >> soc:bus_c2c soc:bus_dmc passive null 0 400000000 100000000 400000000 >> soc:bus_leftbus null simple_ondemand deferrable 50 200000000 200000000 200000000 >> soc:bus_rightbus soc:bus_leftbus passive null 0 200000000 100000000 200000000 >> soc:bus_display soc:bus_leftbus passive null 0 200000000 200000000 200000000 >> soc:bus_fsys soc:bus_leftbus passive null 0 134000000 100000000 134000000 >> soc:bus_peri soc:bus_leftbus passive null 0 100000000 50000000 100000000 >> soc:bus_mfc soc:bus_leftbus passive null 0 200000000 100000000 200000000 >> >> >> [4] PM / devfreq: passive: Update frequency when start governor >> https://patchwork.kernel.org/project/linux-pm/patch/20201103070646.18687-1-cw00.choi@samsung.com/ >> >> > >
Hi Chanwoo, Georgi On 03.11.2020 09:53, Chanwoo Choi wrote: > On 11/3/20 5:29 PM, Georgi Djakov wrote: >> On 11/3/20 09:54, Chanwoo Choi wrote: >>> When I tested this patchset on Odroid-U3, >>> After setting 0 bps by interconnect[1][2], >>> the frequency of devfreq devs sustain the high frequency >>> according to the pm qos request. >>> >>> So, I try to find the cause of this situation. >>> In result, it seems that interconnect exynos driver >>> updates the pm qos request to devfreq device >>> during the kernel booting. Do you know why the exynos >>> interconnect driver request the pm qos during probe >>> without the mixer request? >> >> That's probably because of the sync_state support, that was introduced >> recently. The icc_sync_state callback needs to be added to the driver >> (i just left a comment on that patch), and then check again if it works. >> >> The idea of the sync_state is that there could be multiple users of a >> path and we must wait for all consumers to tell their bandwidth needs. >> Otherwise the first consumer may lower the bandwidth or disable a path >> needed for another consumer (driver), which has not probed yet. So we >> maintain a floor bandwidth until everyone has probed. By default the floor >> bandwidth is INT_MAX, but can be overridden by implementing the get_bw() >> callback. Thanks for detailed explanation Georgi. > Thanks for guide. I tested it with your comment of patch2. > It is well working without problem as I mentioned previously. > > I caught the reset operation of PM QoS requested from interconnect > on kernel log. In result, after completed the kernel booting, > there is no pm qos request if hdmi cable is not connected. Thanks for the bug report Chanwoo, it's related to the sync_state feature as you guys already figured out. I had to reorder some code in the interconnect driver probe() to avoid some issues, i.e. to register PM QoS request before icc_node_add() call but I forgot to check initial state of the bus frequencies. I thought the get_bw implementation might be needed but the default behaviour seems fine, the PM QoS derived bus frequencies will be clamped in the devfreq to valid OPP values. Chanwoo, in order to set the bandwidth to 0 we could also just blank the display. Below are some of the commands I use for testing. # blank display (disable the mixer entirely) echo 4 > /sys/devices/platform/exynos-drm/graphics/fb0/blank # unblank display echo 0 > /sys/devices/platform/exynos-drm/graphics/fb0/blank # modetest with 2 planes (higher bandwidth test) ./modetest -s 47:1920x1080 -P 45:1920x1080 -v
Hi Sylwester, On 11/3/20 7:12 PM, Sylwester Nawrocki wrote: > Hi Chanwoo, Georgi > > On 03.11.2020 09:53, Chanwoo Choi wrote: >> On 11/3/20 5:29 PM, Georgi Djakov wrote: >>> On 11/3/20 09:54, Chanwoo Choi wrote: > >>>> When I tested this patchset on Odroid-U3, >>>> After setting 0 bps by interconnect[1][2], >>>> the frequency of devfreq devs sustain the high frequency >>>> according to the pm qos request. >>>> >>>> So, I try to find the cause of this situation. >>>> In result, it seems that interconnect exynos driver >>>> updates the pm qos request to devfreq device >>>> during the kernel booting. Do you know why the exynos >>>> interconnect driver request the pm qos during probe >>>> without the mixer request? >>> >>> That's probably because of the sync_state support, that was introduced >>> recently. The icc_sync_state callback needs to be added to the driver >>> (i just left a comment on that patch), and then check again if it works. >>> >>> The idea of the sync_state is that there could be multiple users of a >>> path and we must wait for all consumers to tell their bandwidth needs. >>> Otherwise the first consumer may lower the bandwidth or disable a path >>> needed for another consumer (driver), which has not probed yet. So we >>> maintain a floor bandwidth until everyone has probed. By default the floor >>> bandwidth is INT_MAX, but can be overridden by implementing the get_bw() >>> callback. > > Thanks for detailed explanation Georgi. > >> Thanks for guide. I tested it with your comment of patch2. >> It is well working without problem as I mentioned previously. >> >> I caught the reset operation of PM QoS requested from interconnect >> on kernel log. In result, after completed the kernel booting, >> there is no pm qos request if hdmi cable is not connected. > > Thanks for the bug report Chanwoo, it's related to the sync_state > feature as you guys already figured out. I had to reorder some code > in the interconnect driver probe() to avoid some issues, > i.e. to register PM QoS request before icc_node_add() call but > I forgot to check initial state of the bus frequencies. > > I thought the get_bw implementation might be needed but the default > behaviour seems fine, the PM QoS derived bus frequencies will be > clamped in the devfreq to valid OPP values. > > Chanwoo, in order to set the bandwidth to 0 we could also just blank > the display. Below are some of the commands I use for testing. > > # blank display (disable the mixer entirely) > echo 4 > /sys/devices/platform/exynos-drm/graphics/fb0/blank > > # unblank display > echo 0 > /sys/devices/platform/exynos-drm/graphics/fb0/blank > > # modetest with 2 planes (higher bandwidth test) > ./modetest -s 47:1920x1080 -P 45:1920x1080 -v > Thanks for the test guide.
This patchset adds interconnect API support for the Exynos SoC "samsung, exynos-bus" compatible devices, which already have their corresponding exynos-bus driver in the devfreq subsystem. Complementing the devfreq driver with an interconnect functionality allows to ensure the QoS requirements of devices accessing the system memory (e.g. video processing devices) are fulfilled and aallows to avoid issues like the one discussed in thread [1]. This patch series adds implementation of the interconnect provider per each "samsung,exynos-bus" compatible DT node, with one interconnect node per provider. The interconnect code which was previously added as a part of the devfreq driver has been converted to a separate platform driver. In the devfreq a corresponding virtual child platform device is registered. Integration of devfreq and interconnect frameworks is achieved through the PM QoS API. A sample interconnect consumer for exynos-mixer is added in patches 5/6, 6/6, it is currently added only for exynos4412 and allows to address the mixer DMA underrun error issues [1]. Changes since v6: - the interconnect consumer DT bindings are now used to describe dependencies of the interconnects (samsung,exynos-bus nodes), - bus-width property replaced with samsung,data-clk-ratio, - adaptation to recent changes in the interconnect code (of_icc_get_from_provider(), icc_node_add()). The series has been tested on Odroid U3 board. It is based on v5.10-rc1. -- Regards, Sylwester Changes since v5: - addition of "bus-width: DT property, which specifies data width of the interconnect bus (patches 1...2/6), - addition of synchronization of the interconnect bandwidth setting with VSYNC (patch 6/6). Changes since v3 [4] (v4 skipped to align with patchset [1]), detailed changes are listed in each patch: - conversion to a separate interconnect (platform) driver, - an update of the DT binding documenting new optional properties: #interconnect-cells, samsung,interconnect-parent in "samsung,exynos-bus" nodes, - new DT properties added to the SoC, rather than to the board specific files. Changes since v2 [5]: - Use icc_std_aggregate(). - Implement a different modification of apply_constraints() in drivers/interconnect/core.c (patch 03). - Use 'exynos,interconnect-parent-node' in the DT instead of 'devfreq'/'parent', depending on the bus. - Rebase on DT patches that deprecate the 'devfreq' DT property. - Improve error handling, including freeing generated IDs on failure. - Remove exynos_bus_icc_connect() and add exynos_bus_icc_get_parent(). Changes since v1 [6]: - Rebase on coupled regulators patches. - Use dev_pm_qos_*() API instead of overriding frequency in exynos_bus_target(). - Use IDR for node ID allocation. - Reverse order of multiplication and division in mixer_set_memory_bandwidth() (patch 07) to avoid integer overflow. References: [1] https://patchwork.kernel.org/patch/10861757/ (original issue) [2] https://www.spinics.net/lists/linux-samsung-soc/msg70014.html [3] https://www.spinics.net/lists/arm-kernel/msg810722.html [4] https://lore.kernel.org/linux-pm/20191220115653.6487-1-a.swigon@samsung.com [5] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC) [6] https://patchwork.kernel.org/cover/11152595/ (v2 of this RFC) Artur Świgoń (1): ARM: dts: exynos: Add interconnects to Exynos4412 mixer Sylwester Nawrocki (5): dt-bindings: devfreq: Add documentation for the interconnect properties interconnect: Add generic interconnect driver for Exynos SoCs PM / devfreq: exynos-bus: Add registration of interconnect child device ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes drm: exynos: mixer: Add interconnect support .../devicetree/bindings/devfreq/exynos-bus.txt | 68 ++++++- arch/arm/boot/dts/exynos4412.dtsi | 7 + drivers/devfreq/exynos-bus.c | 17 ++ drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++- drivers/interconnect/Kconfig | 1 + drivers/interconnect/Makefile | 1 + drivers/interconnect/exynos/Kconfig | 6 + drivers/interconnect/exynos/Makefile | 4 + drivers/interconnect/exynos/exynos.c | 198 +++++++++++++++++++++ 9 files changed, 438 insertions(+), 10 deletions(-) create mode 100644 drivers/interconnect/exynos/Kconfig create mode 100644 drivers/interconnect/exynos/Makefile create mode 100644 drivers/interconnect/exynos/exynos.c -- 2.7.4