| Message ID | b731b713d8738239c26361ece7f5cadea035b353.1568240476.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Initialise thermal framework earlier during boot | expand |
On 12-09-19, 04:02, Amit Kucheria wrote: > Allow qcom-hw driver to initialise right after the cpufreq and thermal > subsystems are initialised in core_initcall so we get earlier access to > thermal mitigation. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 4b0b50403901..04676cc82ba6 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > { > return platform_driver_register(&qcom_cpufreq_hw_driver); > } > -device_initcall(qcom_cpufreq_hw_init); > +postcore_initcall(qcom_cpufreq_hw_init); Even core_initcall should work just fine because of the ordering in the Makefile in cpufreq directory. Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > Allow qcom-hw driver to initialise right after the cpufreq and thermal > subsystems are initialised in core_initcall so we get earlier access to > thermal mitigation. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 4b0b50403901..04676cc82ba6 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > { > return platform_driver_register(&qcom_cpufreq_hw_driver); > } > -device_initcall(qcom_cpufreq_hw_init); > +postcore_initcall(qcom_cpufreq_hw_init); I am fine with core framework initcall pushed to earlier initcall levels if required, but for individual/platform specific drivers I am not so happy to see that. This goes against the grand plan of single common kernel strategy by Android moving all drivers as modules. We might decide to make this a module. Also there are few cpufreq drivers that are modules. Will they have issues ? If not, why do we need this change at all. Needing thermal mitigation during boot this earlier is still too much of expectation, I would rather boot slowly than relying on this feature. -- Regards, Sudeep
Hi Sudeep, On 17/09/2019 11:34, Sudeep Holla wrote: > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: >> Allow qcom-hw driver to initialise right after the cpufreq and thermal >> subsystems are initialised in core_initcall so we get earlier access to >> thermal mitigation. >> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> >> --- >> drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c >> index 4b0b50403901..04676cc82ba6 100644 >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >> @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) >> { >> return platform_driver_register(&qcom_cpufreq_hw_driver); >> } >> -device_initcall(qcom_cpufreq_hw_init); >> +postcore_initcall(qcom_cpufreq_hw_init); > > I am fine with core framework initcall pushed to earlier initcall levels > if required, but for individual/platform specific drivers I am not so > happy to see that. > > This goes against the grand plan of single common kernel strategy by > Android moving all drivers as modules. We might decide to make this > a module. module = mounted file system = very late initialization Is that the plan? Force every driver to load too late? There are core drivers which must be loaded as soon as possible. If the qcom driver is one of them, then what is the problem? "The grand plan" will have to solve this first before doing the module move. > Also there are few cpufreq drivers that are modules. Will > they have issues ? If not, why do we need this change at all. Because some boards don't have thermal issues with the cpufreq drivers as module, other boards have. > Needing > thermal mitigation during boot this earlier is still too much of > expectation, I would rather boot slowly than relying on this feature. And what if we want to boot faster? The boot time is one of a key point of benchmark.
On 12/09/2019 00:32, Amit Kucheria wrote: > Allow qcom-hw driver to initialise right after the cpufreq and thermal > subsystems are initialised in core_initcall so we get earlier access to > thermal mitigation. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 4b0b50403901..04676cc82ba6 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > { > return platform_driver_register(&qcom_cpufreq_hw_driver); > } > -device_initcall(qcom_cpufreq_hw_init); > +postcore_initcall(qcom_cpufreq_hw_init); > > static void __exit qcom_cpufreq_hw_exit(void) > { >
Hi Daniel On Tuesday 17 Sep 2019 at 14:47:22 (+0200), Daniel Lezcano wrote: > > Hi Sudeep, > > On 17/09/2019 11:34, Sudeep Holla wrote: > > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > >> Allow qcom-hw driver to initialise right after the cpufreq and thermal > >> subsystems are initialised in core_initcall so we get earlier access to > >> thermal mitigation. > >> > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > >> --- > >> drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > >> index 4b0b50403901..04676cc82ba6 100644 > >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c > >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > >> @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > >> { > >> return platform_driver_register(&qcom_cpufreq_hw_driver); > >> } > >> -device_initcall(qcom_cpufreq_hw_init); > >> +postcore_initcall(qcom_cpufreq_hw_init); > > > > I am fine with core framework initcall pushed to earlier initcall levels > > if required, but for individual/platform specific drivers I am not so > > happy to see that. > > > > This goes against the grand plan of single common kernel strategy by > > Android moving all drivers as modules. We might decide to make this > > a module. > > module = mounted file system = very late initialization > > Is that the plan? Force every driver to load too late? > > There are core drivers which must be loaded as soon as possible. If the > qcom driver is one of them, then what is the problem? > > "The grand plan" will have to solve this first before doing the module > move. > > > Also there are few cpufreq drivers that are modules. Will > > they have issues ? If not, why do we need this change at all. > > Because some boards don't have thermal issues with the cpufreq drivers > as module, other boards have. > > > Needing > > thermal mitigation during boot this earlier is still too much of > > expectation, I would rather boot slowly than relying on this feature. > > And what if we want to boot faster? The boot time is one of a key point > of benchmark. Could you share test results for this ? It'd be nice to see what if the gains in boot time outweight the additional pain for android folks ... Thanks, Quentin
On Tue, Sep 17, 2019 at 02:47:22PM +0200, Daniel Lezcano wrote: > > Hi Sudeep, > > On 17/09/2019 11:34, Sudeep Holla wrote: > > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > >> Allow qcom-hw driver to initialise right after the cpufreq and thermal > >> subsystems are initialised in core_initcall so we get earlier access to > >> thermal mitigation. > >> > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > >> --- > >> drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > >> index 4b0b50403901..04676cc82ba6 100644 > >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c > >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > >> @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > >> { > >> return platform_driver_register(&qcom_cpufreq_hw_driver); > >> } > >> -device_initcall(qcom_cpufreq_hw_init); > >> +postcore_initcall(qcom_cpufreq_hw_init); > > > > I am fine with core framework initcall pushed to earlier initcall levels > > if required, but for individual/platform specific drivers I am not so > > happy to see that. > > > > This goes against the grand plan of single common kernel strategy by > > Android moving all drivers as modules. We might decide to make this > > a module. > > module = mounted file system = very late initialization > > Is that the plan? Force every driver to load too late? > Yes. Something similar to what we have on desktops/servers. > There are core drivers which must be loaded as soon as possible. If the > qcom driver is one of them, then what is the problem? > I am fine with that if it's really issue but it shouldn't become the defacto trend. > "The grand plan" will have to solve this first before doing the module > move. > Sure, I just expressed my view as it looks to be going in different direction for me. > > Also there are few cpufreq drivers that are modules. Will > > they have issues ? If not, why do we need this change at all. > > Because some boards don't have thermal issues with the cpufreq drivers > as module, other boards have. > OK, so this platform boots with default high OPP and needs thermal mitigation that early ? If so, that's fine. > > Needing > > thermal mitigation during boot this earlier is still too much of > > expectation, I would rather boot slowly than relying on this feature. > > And what if we want to boot faster? The boot time is one of a key point > of benchmark. > I understand the requirement, though for me it's really sounds stupid. As Quentin pointed out, it would be good to get all those benchmark details, and preferably in the commit log so that we can look back whenever someone else take the same approach later. -- Regards, Sudeep
On 17-09-19, 10:34, Sudeep Holla wrote: > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > > -device_initcall(qcom_cpufreq_hw_init); > > +postcore_initcall(qcom_cpufreq_hw_init); > > I am fine with core framework initcall pushed to earlier initcall levels > if required, but for individual/platform specific drivers I am not so > happy to see that. > > This goes against the grand plan of single common kernel strategy by > Android moving all drivers as modules. Its been long that I got the opportunity to work on drivers directly, but as far as I remember (which should be incorrect based on your reply) we can still build a driver as module even if it has some postcore_initcall() declarations. They will execute at module insertion. Is that incorrect ? If not, then how is it going to affect android effort ?
On Wed, Sep 18, 2019 at 02:39:38PM +0530, Viresh Kumar wrote: > On 17-09-19, 10:34, Sudeep Holla wrote: > > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > > > -device_initcall(qcom_cpufreq_hw_init); > > > +postcore_initcall(qcom_cpufreq_hw_init); > > > > I am fine with core framework initcall pushed to earlier initcall levels > > if required, but for individual/platform specific drivers I am not so > > happy to see that. > > > > This goes against the grand plan of single common kernel strategy by > > Android moving all drivers as modules. > > Its been long that I got the opportunity to work on drivers directly, but as far > as I remember (which should be incorrect based on your reply) we can still build > a driver as module even if it has some postcore_initcall() declarations. They > will execute at module insertion. Is that incorrect ? If not, then how is it > going to affect android effort ? > Ah no, I am not referring to building as module. As you mention, that may work just fine. I was referring to timing dependency during boot. The idea is minimize the number of such initcall dependency. They should all work fine even as modules and should have least dependency on initcall sequence. -- Regards, Sudeep
On 18-09-19, 10:17, Sudeep Holla wrote: > Ah no, I am not referring to building as module. As you mention, that may > work just fine. I was referring to timing dependency during boot. The idea > is minimize the number of such initcall dependency. They should all work > fine even as modules and should have least dependency on initcall sequence. Yeah, so things work fine for them right now but that can be improved by changing the sequence of boot here. And that's what Amit is trying to do here. Even if android builds this as a module later, things will continue to work but that may not be the best performance/boot-time wise. When I had a discussion about this with Amit earlier, I asked him to send patches even if he doesn't have any performance numbers for it as this is a platform driver and I find it okay for them to decide the boot sequence that they think is the best :)
Hi Sudeep, On Tue, Sep 17, 2019 at 6:20 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Tue, Sep 17, 2019 at 02:47:22PM +0200, Daniel Lezcano wrote: > > > > Hi Sudeep, > > > > On 17/09/2019 11:34, Sudeep Holla wrote: > > > On Thu, Sep 12, 2019 at 04:02:34AM +0530, Amit Kucheria wrote: > > >> Allow qcom-hw driver to initialise right after the cpufreq and thermal > > >> subsystems are initialised in core_initcall so we get earlier access to > > >> thermal mitigation. > > >> > > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > > >> --- > > >> drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > >> index 4b0b50403901..04676cc82ba6 100644 > > >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c > > >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > >> @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) > > >> { > > >> return platform_driver_register(&qcom_cpufreq_hw_driver); > > >> } > > >> -device_initcall(qcom_cpufreq_hw_init); > > >> +postcore_initcall(qcom_cpufreq_hw_init); > > > > > > I am fine with core framework initcall pushed to earlier initcall levels > > > if required, but for individual/platform specific drivers I am not so > > > happy to see that. > > > > > > This goes against the grand plan of single common kernel strategy by > > > Android moving all drivers as modules. We might decide to make this > > > a module. > > > > module = mounted file system = very late initialization > > > > Is that the plan? Force every driver to load too late? > > > > Yes. Something similar to what we have on desktops/servers. > > > There are core drivers which must be loaded as soon as possible. If the > > qcom driver is one of them, then what is the problem? > > > > I am fine with that if it's really issue but it shouldn't become the > defacto trend. I didn't convert other HW drivers on purpose since it's really up to the platform to decide. I have tested with all drivers converted to core_initcall and didn't find any boot issues on kernelci. > > "The grand plan" will have to solve this first before doing the module > > move. > > > > Sure, I just expressed my view as it looks to be going in different > direction for me. > > > > Also there are few cpufreq drivers that are modules. Will > > > they have issues ? If not, why do we need this change at all. > > > > Because some boards don't have thermal issues with the cpufreq drivers > > as module, other boards have. > > > > OK, so this platform boots with default high OPP and needs thermal > mitigation that early ? If so, that's fine. That is indeed the case - 30-40 degree rise in under 50ms can be seen on some of these platforms. > > > Needing > > > thermal mitigation during boot this earlier is still too much of > > > expectation, I would rather boot slowly than relying on this feature. > > > > And what if we want to boot faster? The boot time is one of a key point > > of benchmark. > > > > I understand the requirement, though for me it's really sounds stupid. Is it stupid if the SoC was being used in automotive with a 2s (or less) startup requirement? :-) > As Quentin pointed out, it would be good to get all those benchmark > details, and preferably in the commit log so that we can look back > whenever someone else take the same approach later. I'm traveling for Connect this week but will try to post some logs with initcall_debug turned on where changing these initcalls shaves off several seconds. Just to reassure everyone that GKI isn't forgotten, the next set of patches will actually add module support for the tsens driver so AOSP can make them modules and things will still work. Regards, Amit
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 4b0b50403901..04676cc82ba6 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -327,7 +327,7 @@ static int __init qcom_cpufreq_hw_init(void) { return platform_driver_register(&qcom_cpufreq_hw_driver); } -device_initcall(qcom_cpufreq_hw_init); +postcore_initcall(qcom_cpufreq_hw_init); static void __exit qcom_cpufreq_hw_exit(void) {
Allow qcom-hw driver to initialise right after the cpufreq and thermal subsystems are initialised in core_initcall so we get earlier access to thermal mitigation. Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> --- drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)