Bump. I saw your email in Sibi's patch series. His patch series is just one use case/user of this feature. This patch series is useful in general. Do plan to Ack this? Or thoughts on my earlier response?
Thanks, Saravana On Fri, Jun 28, 2019 at 1:26 PM Saravana Kannan <sarava...@google.com> wrote: > > On Thu, Jun 27, 2019 at 11:49 PM Viresh Kumar <viresh.ku...@linaro.org> wrote: > > > > On 26-06-19, 11:10, Saravana Kannan wrote: > > > On Tue, Jun 25, 2019 at 11:32 PM Viresh Kumar <viresh.ku...@linaro.org> > > > wrote: > > > > > > So, when a CPU changes frequency, we must change the performance state > > > > of PM domain and change frequency/bw of the cache synchronously. > > > > > > I mean, it's going to be changed when we get the CPUfreq transition > > > notifiers. From a correctness point of view, setting it inside the OPP > > > framework is not any better than doing it when we get the notifiers. > > > > That's what the problem is. All maintainers now a days ask people to > > stay away from notifiers and we are making that base of another new > > thing we are starting. > > In that case we can just add direct calls in cpufreq.c to let devfreq > know about the frequency changes. But then again, CPU is just one > example for this use case. I'm just using that because people are more > familiar with that. > > > Over that, with many cpufreq drivers we have fast switching enabled > > and notifiers disabled. How will they make these things work ? We > > still want to scale L3 in those cases as well. > > Nothing is preventing them from using the xlate OPP API I added to > figure out all the CPU to L3 frequency mapping and then set the L3 > frequency directly from the CPUfreq driver. > > Also, whether we use OPP framework or devfreq to set the L3 frequency, > it's going to block fast switching because both these frameworks have > APIs that can sleep. > > But really, most mobile use cases don't want to permanently tie L3 > freq to CPU freq. Having it go through devfreq and being able to > switch governors is a very important need for mobile products. > > Keep in mind that nothing in this series does any of the cpufreq stuff > yet. That'll need a few more changes. I was just using CPUfreq as an > example. > > > > I see this as "required for good performance". So I don't see it as > > > redefining required-opps. If someone wants good performance/power > > > balance they follow the "required-opps". Technically even the PM > > > pstates are required for good power. Otherwise, the system could leave > > > the voltage at max and stuff would still work. > > > > > > Also, the slave device might need to get input from multiple master > > > devices and aggregate the request before setting the slave device > > > frequency. So I don't think OPP framework would be the right place to > > > deal with those things. For example, L3 might (will) have different > > > mappings for big vs little cores. So that needs to be aggregated and > > > set properly by the slave device driver. Also, GPU might have a > > > mapping for L3 too. In which case the L3 slave driver needs to take > > > input from even more masters before it decides its frequency. But most > > > importantly, we still need the ability to change governors for L3. > > > Again these are just examples with L3 and it can get more complicated > > > based on the situation. > > > > > > Most importantly, instead of always going by mapping, one might decide > > > to scale the L3 based on some other governor (that looks at some HW > > > counter). Or just set it to performance governor for a use case for > > > which performance is more important. All of this comes for free with > > > devfreq and if we always set it from OPP framework we don't give this > > > required control to userspace. > > > > > > I think going through devfreq is the right approach for this. And we > > > can always rewrite the software if we find problems in the future. But > > > as it stands today, this will help cases like exynos without the need > > > for a lot of changes. Hope I've convinced you. > > > > I understand the aggregation thing and fully support that the > > aggregation can't happen in OPP core and must be done somewhere else. > > But the input can go from OPP core while the frequency is changing, > > isn't it ? > > I'm not opposed to OPP sending input to devfreq to let it know that a > master device frequency change is happening. But I think this is kinda > orthogonal to this patch series. > > Today the passive governor looks at the master device's devfreq > frequency changes to trigger the frequency change of the slave > devfreq. It neither supports tracking OPP frequency change nor CPUfreq > frequency change. If that's something we want to add, we can look into > that separately as passive governor (or a new governor) changes. > > But then not all devices (CPUfreq or otherwise) use OPP to set the > frequencies. So it's beneficial to have all of these frameworks as > inputs for devfreq passive (like) governor. CPUfreq is actually a bit > more tricky because we'll also have to track hotplug, etc. So direct > calls from CPUfreq to devfreq (similar to cpufreq stats tracking) > would be good. > > -Saravana
