Hi Artur and Leonard,
On 19. 8. 9. 오전 12:00, Leonard Crestez wrote:
On 29.07.2019 04:49, Chanwoo Choi wrote:
On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
This patch adds interconnect functionality to the exynos-bus devfreq driver.
The devfreq target() callback provided by exynos-bus now selects either the frequency calculated by the devfreq governor or the frequency requested via the interconnect API for the given node, whichever is higher.
Basically, I agree to support the QoS requirement between devices. But, I think that need to consider the multiple cases.
- When changing the devfreq governor by user,
For example of the connection between bus_dmc/leftbus/display on patch8, there are possible multiple cases with various devfreq governor which is changed on the runtime by user through sysfs interface.
If users changes the devfreq governor as following: Before,
- bus_dmc (simple_ondemand, available frequency 100/200/300/400 MHz)
--> bus_leftbus(simple_ondemand, available frequency 100/200/300/400 MHz) ----> bus_display(passive)
After changed governor of bus_dmc, if the min_freq by interconnect requirement is 400Mhz,
- bus_dmc (powersave) : min_freq and max_freq and cur_freq is 100MHz
--> bus_leftbus(simple_ondemand) : cur_freq is 400Mhz ----> bus_display(passive)
The final frequency is 400MHz of bus_dmc even if the min_freq/max_freq/cur_freq is 100MHz. It cannot show the correct min_freq/max_freq through devfreq sysfs interface.
- When disabling the some frequency by devfreq-thermal throttling,
This patch checks the min_freq of interconnect requirement in the exynos_bus_target() and exynos_bus_passive_target(). Also, it cannot show the correct min_freq/max_freq through devfreq sysfs interface.
For example of bus_dmc bus,
- The available frequencies are 100MHz, 200MHz, 300MHz, 400MHz
- Disable 400MHz by devfreq-thermal throttling
- min_freq is 100MHz
- max_freq is 300MHz
- min_freq of interconnect is 400MHz
In result, the final frequency is 400MHz by exynos_bus_target() There are no problem for working. But, the user cannot know reason why cur_freq is 400MHz even if max_freq is 300MHz.
Basically, update_devfreq() considers the all constraints of min_freq/max_freq to decide the proper target frequency.
I think that applying the interconnect min_freq via dev_pm_qos can help with many of these concerns: update_devfreq controls all the min/max handling, sysfs is accurate and better decisions can be made regarding thermal. Enforcing constraints in the core is definitely better.
This wouldn't even be a very big change, you don't need to actually move the interconnect code outside of devfreq. Just make every devfreq/icc node register a dev_pm_qos_request for itself during registration and call dev_pm_qos_update_request inside exynos_bus_icc_set.
I agree this approach of Leonard. If we use the dev_pm_qos[1] feature, it resolve the issue of my comment1/2.
In summary, when receive the minimum frequency requirement from interconnect path, the each bus uses the dev_pm_qos interface in order to inform the minimum frequency from interconnect to devfreq. And then the devfreq core will execute the update_devfreq() with all frequency requirements as following: - the user input (min/max frequency though devfreq sysfs - the target frequency provided by devfreq governor - the minimum frequency from interconnect path