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. > > 1. 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. > > > 2. 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. See: https://patchwork.kernel.org/patch/11084279/ -- Regards, Leonard _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
