| Message ID | cover.1589528491.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | opp: core: add regulators enable and disable | expand |
Hi Viresh, On 15.05.2020 09:57, Viresh Kumar wrote: > This series reintroduces the usage of regulator_enable/disable() to the > OPP core after the previous attempt was reverted [1] shortly after getting > applied. This time the regulator is enabled only after it is configured > by the OPP core. > > Marek, Kamil and Clément: Can you guys please test this out and report > if this doesn't work as expected ? Works fine for my test cases, especially Samsung Chromebook Peach-Pit/Pi still boots fine. Feel free to add to the both patches: Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Thanks! Best regards
Hi Viresh, On Fri, 15 May 2020 at 09:57, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hi, > > This series reintroduces the usage of regulator_enable/disable() to the > OPP core after the previous attempt was reverted [1] shortly after getting > applied. This time the regulator is enabled only after it is configured > by the OPP core. > > Marek, Kamil and Clément: Can you guys please test this out and report > if this doesn't work as expected ? I have reviewed the patch and it seems fine for my use case. Unfortunately I can't test it until next week. Acked-by: Clément Péron <peron.clem@gmail.com> Regards, Clement > > -- > viresh > > [1] https://lore.kernel.org/lkml/20191017102758.8104-1-m.szyprowski@samsung.com/ > > Kamil Konieczny (1): > opp: core: add regulators enable and disable > > Viresh Kumar (1): > opp: Reorder the code for !target_freq case > > drivers/opp/core.c | 39 ++++++++++++++++++++++++++++++++++----- > drivers/opp/opp.h | 2 ++ > 2 files changed, 36 insertions(+), 5 deletions(-) > > -- > 2.25.0.rc1.19.g042ed3e048af >
Hi, On Fri, 15 May 2020 at 14:00, Clément Péron <peron.clem@gmail.com> wrote: > > Hi Viresh, > > On Fri, 15 May 2020 at 09:57, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > Hi, > > > > This series reintroduces the usage of regulator_enable/disable() to the > > OPP core after the previous attempt was reverted [1] shortly after getting > > applied. This time the regulator is enabled only after it is configured > > by the OPP core. > > > > Marek, Kamil and Clément: Can you guys please test this out and report > > if this doesn't work as expected ? > > I have reviewed the patch and it seems fine for my use case. > Unfortunately I can't test it until next week. Ok, before the patch the regulator was released by regulator_late_cleanup() : [ 33.756849] vdd-gpu: disabling Now it works fine and the vdd-gpu is no more disabled. Tested-by: Clément Péron <peron.clem@gmail.com> Regards, Clement > > Acked-by: Clément Péron <peron.clem@gmail.com> > > Regards, > Clement > > > > > > -- > > viresh > > > > [1] https://lore.kernel.org/lkml/20191017102758.8104-1-m.szyprowski@samsung.com/ > > > > Kamil Konieczny (1): > > opp: core: add regulators enable and disable > > > > Viresh Kumar (1): > > opp: Reorder the code for !target_freq case > > > > drivers/opp/core.c | 39 ++++++++++++++++++++++++++++++++++----- > > drivers/opp/opp.h | 2 ++ > > 2 files changed, 36 insertions(+), 5 deletions(-) > > > > -- > > 2.25.0.rc1.19.g042ed3e048af > >
Hi Viresh, On Thu, 21 May 2020 at 14:23, Clément Péron <peron.clem@gmail.com> wrote: > > Hi, > > On Fri, 15 May 2020 at 14:00, Clément Péron <peron.clem@gmail.com> wrote: > > > > Hi Viresh, > > > > On Fri, 15 May 2020 at 09:57, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > Hi, > > > > > > This series reintroduces the usage of regulator_enable/disable() to the > > > OPP core after the previous attempt was reverted [1] shortly after getting > > > applied. This time the regulator is enabled only after it is configured > > > by the OPP core. > > > > > > Marek, Kamil and Clément: Can you guys please test this out and report > > > if this doesn't work as expected ? > > > > I have reviewed the patch and it seems fine for my use case. > > Unfortunately I can't test it until next week. > > Ok, before the patch the regulator was released by regulator_late_cleanup() : > [ 33.756849] vdd-gpu: disabling > > Now it works fine and the vdd-gpu is no more disabled. Today, I compiled my kernel without any program requiring GPU computing at boot. This makes the dev_pm_opp_set_rate() to never be called and so the regulator is not enabled before the regulator framework switches off all the regulators that haven't been enabled. Unfortunately switching off the GPU regulator makes my board hang.. I'm not sure what is the best approach to fix this. Is it required that the dev_pm_opp_set_rate() must be called one time at the GPU driver init? Panfost already calls devfreq_recommended_opp() and dev_pm_opp_put() but that doesn't trigger dev_pm_opp_set_rate(). Thanks for your help, BR, Clement > > Tested-by: Clément Péron <peron.clem@gmail.com> > > Regards, > Clement > > > > > Acked-by: Clément Péron <peron.clem@gmail.com> > > > > Regards, > > Clement > > > > > > > > > > -- > > > viresh > > > > > > [1] https://lore.kernel.org/lkml/20191017102758.8104-1-m.szyprowski@samsung.com/ > > > > > > Kamil Konieczny (1): > > > opp: core: add regulators enable and disable > > > > > > Viresh Kumar (1): > > > opp: Reorder the code for !target_freq case > > > > > > drivers/opp/core.c | 39 ++++++++++++++++++++++++++++++++++----- > > > drivers/opp/opp.h | 2 ++ > > > 2 files changed, 36 insertions(+), 5 deletions(-) > > > > > > -- > > > 2.25.0.rc1.19.g042ed3e048af > > >
On 03-09-22, 22:35, Clément Péron wrote: > Today, I compiled my kernel without any program requiring GPU > computing at boot. This makes the dev_pm_opp_set_rate() to never be > called and so the regulator is not enabled before the regulator > framework switches off all the regulators that haven't been enabled. > > Unfortunately switching off the GPU regulator makes my board hang.. Why does the board hang? I mean the kernel should boot fine with the GPU disabled, isn't it ? Or is the regulator shared with some other critical resource, or something else. > I'm not sure what is the best approach to fix this. > > Is it required that the dev_pm_opp_set_rate() must be called one time > at the GPU driver init? Right now, Yes. And it looks like the right approach as well. > Panfost already calls devfreq_recommended_opp() and dev_pm_opp_put() > but that doesn't trigger dev_pm_opp_set_rate(). Can you also point to your code ? Which file are you working on ?
Hi Viresh, On Mon, 5 Sept 2022 at 06:35, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 03-09-22, 22:35, Clément Péron wrote: > > Today, I compiled my kernel without any program requiring GPU > > computing at boot. This makes the dev_pm_opp_set_rate() to never be > > called and so the regulator is not enabled before the regulator > > framework switches off all the regulators that haven't been enabled. > > > > Unfortunately switching off the GPU regulator makes my board hang.. > > Why does the board hang? I mean the kernel should boot fine with the > GPU disabled, isn't it ? Or is the regulator shared with some other > critical resource, or something else. The regulator is dedicated to the GPU and the board could certainly run without GPU, the issue is that the driver (here panfrost) may do some regular access to GPU memory (I suppose). > > > I'm not sure what is the best approach to fix this. > > > > Is it required that the dev_pm_opp_set_rate() must be called one time > > at the GPU driver init? > > Right now, Yes. And it looks like the right approach as well. > > > Panfost already calls devfreq_recommended_opp() and dev_pm_opp_put() > > but that doesn't trigger dev_pm_opp_set_rate(). > > Can you also point to your code ? Which file are you working on ? The code I'm pointing is panfrost_devfreq_init() in drivers/gpu/drm/panfrost/panfrost_devfreq.c Regards, Clement > > -- > viresh
On 05-09-22, 10:28, Clément Péron wrote: > Hi Viresh, > > On Mon, 5 Sept 2022 at 06:35, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 03-09-22, 22:35, Clément Péron wrote: > > > Today, I compiled my kernel without any program requiring GPU > > > computing at boot. I thought you disabled most of GPU stuff here and so thought panfrost devfreq will be gone too :) > > > This makes the dev_pm_opp_set_rate() to never be > > > called and so the regulator is not enabled before the regulator > > > framework switches off all the regulators that haven't been enabled. > > > > > > Unfortunately switching off the GPU regulator makes my board hang.. > > > > Why does the board hang? I mean the kernel should boot fine with the > > GPU disabled, isn't it ? Or is the regulator shared with some other > > critical resource, or something else. > > The regulator is dedicated to the GPU and the board could certainly > run without GPU, the issue is that the driver (here panfrost) may do > some regular access to GPU memory (I suppose). So we do need GPU to be functional at this point ? i.e. both clk/regulators should be enabled. > The code I'm pointing is panfrost_devfreq_init() in > drivers/gpu/drm/panfrost/panfrost_devfreq.c I think it would be better to call dev_pm_opp_set_opp() after calling devfreq_recommended_opp() in this driver, since you already have the OPP known to you.