| Message ID | 20220420102044.10832-1-roger.lu@mediatek.com |
|---|---|
| Headers | show |
| Series | soc: mediatek: SVS: introduce MTK SVS | expand |
Hi Roger, Roger Lu <roger.lu@mediatek.com> writes: > The Smart Voltage Scaling(SVS) engine is a piece of hardware > which calculates suitable SVS bank voltages to OPP voltage table. > Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck > when receiving OPP_EVENT_ADJUST_VOLTAGE. > > 1. SVS driver uses OPP adjust event in [1] to update OPP table voltage part. > 2. SVS driver gets thermal/GPU device by node [2][3] and CPU device by get_cpu_device(). > After retrieving subsys device, SVS driver calls device_link_add() to make sure probe/suspend callback priority. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=25cb20a212a1f989385dfe23230817e69c62bee5 > [2] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=b325ce39785b1408040d90365a6ab1aa36e94f87 > [3] https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.16-next/dts64&id=a8168cebf1bca1b5269e8a7eb2626fb76814d6e2 > > Change since v23: > - Change wording from "Mediatek" to "MediaTek" (uppercase T) in mtk-svs.yaml. > - Use cpuidle_pause_and_lock() to prevent system from entering cpuidle instead of applying pm_qos APIs. > - Add kfree() at the end of svs_probe() when encountering probe fail. > - Change MODULE_LICENSE from "GPL v2" to "GPL". > - Add nvmem_cell_put() in error handling when nvmem_cell_read() encounters fail. I also gave you a reviewed-by on v23, but here it is again: Reviewed-by: Kevin Hilman <khilman@baylibre.com> That being said, it would be really nice to see an integration tree where this was all tested on mainline (e.g. v5.17, or v5.18-rc) For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin board, it fails to probe[1] because there is no CCI node in the upstream mt8183.dtsi. I'm assuming this series is also not very useful without the CPUfreq series from Rex, so being able to test this, CCI and CPUfreq together on MT8183 on a mainline kernel would be very helpful. Kevin [1] [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe fail
Hi Roger, Roger Lu <roger.lu@mediatek.com> writes: > The Smart Voltage Scaling(SVS) engine is a piece of hardware > which calculates suitable SVS bank voltages to OPP voltage table. > Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck > when receiving OPP_EVENT_ADJUST_VOLTAGE. > > Signed-off-by: Roger Lu <roger.lu@mediatek.com> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Can SVS work with one or the other clusters disabled? It seems like it should still be able to work. However, if you disable the 2nd cluster (e.g. by passing `maxcpus=4` on the kernel command-line, the SVS driver will fail to probe. I dont' think it's a blocker for merging this series, but making the probe a bit more robust so it can handle the cluster being disabled would be nice additional fix for later. For example, upstream kernel on mt8183-pumpkin board is very unstable with the 2nd cluster enabled (I'm still trying to debug why), but I have to boot with `maxcpus=4` on the cmdline, otherwise kernel fails to boot, so that's how I noticed this probe failure with SVS. Kevin
Hi Kevin, Thanks very much for the feedback. On Wed, 2022-04-20 at 17:06 -0700, Kevin Hilman wrote: > Hi Roger, > > > Roger Lu <roger.lu@mediatek.com> writes: > > > The Smart Voltage Scaling(SVS) engine is a piece of hardware > > which calculates suitable SVS bank voltages to OPP voltage table. > > Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck > > when receiving OPP_EVENT_ADJUST_VOLTAGE. > > > > Signed-off-by: Roger Lu <roger.lu@mediatek.com> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > Can SVS work with one or the other clusters disabled? It seems like it > should still be able to work. However, if you disable the 2nd cluster > (e.g. by passing `maxcpus=4` on the kernel command-line, the SVS driver > will fail to probe. > > I dont' think it's a blocker for merging this series, but making the > probe a bit more robust so it can handle the cluster being disabled > would be nice additional fix for later. > > For example, upstream kernel on mt8183-pumpkin board is very unstable > with the 2nd cluster enabled (I'm still trying to debug why), but I have > to boot with `maxcpus=4` on the cmdline, otherwise kernel fails to boot, > so that's how I noticed this probe failure with SVS. On mt8183 platform, I think SVS can work with one or the other clusters disabled. However, SVS is supposed to be enabled after system is stable for not involving any power unstable concern. If we want part of SVS works at the development stage, we can disable non- working svs bank as below. Thanks. --- a/drivers/soc/mediatek/mtk-svs.c +++ b/drivers/soc/mediatek/mtk-svs.c @@ -2164,7 +2164,7 @@ static struct svs_bank svs_mt8183_banks[] = { .cpu_id = 4, .buck_name = "proc", .volt_flags = SVSB_INIT01_VOLT_INC_ONLY, - .mode_support = SVSB_MODE_INIT01 | SVSB_MODE_INIT02, + .mode_support = SVSB_MODE_ALL_DISABLE, .opp_count = MAX_OPP_ENTRIES, > > Kevin
Hi Kevin, On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: > Hi Roger, > > > Roger Lu <roger.lu@mediatek.com> writes: > > > The Smart Voltage Scaling(SVS) engine is a piece of hardware > > which calculates suitable SVS bank voltages to OPP voltage table. > > Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck > > when receiving OPP_EVENT_ADJUST_VOLTAGE. > > > > 1. SVS driver uses OPP adjust event in [1] to update OPP table voltage part. > > 2. SVS driver gets thermal/GPU device by node [2][3] and CPU device by > > get_cpu_device(). > > After retrieving subsys device, SVS driver calls device_link_add() to make > > sure probe/suspend callback priority. > > > > [1] > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp*linux-next&id=25cb20a212a1f989385dfe23230817e69c62bee5__;Lw!!CTRNKA9wMg0ARbw!3gWsdVuiyF0iafrmVINP9FVz7fjGB1UqTPLfMNWEhsl96RDPB-Se6Q-g3F8daK-u$ > > > > [2] > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp*linux-next&id=b325ce39785b1408040d90365a6ab1aa36e94f87__;Lw!!CTRNKA9wMg0ARbw!3gWsdVuiyF0iafrmVINP9FVz7fjGB1UqTPLfMNWEhsl96RDPB-Se6Q-g3Lel3h4j$ > > > > [3] > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.16-next*dts64&id=a8168cebf1bca1b5269e8a7eb2626fb76814d6e2__;Lw!!CTRNKA9wMg0ARbw!3gWsdVuiyF0iafrmVINP9FVz7fjGB1UqTPLfMNWEhsl96RDPB-Se6Q-g3KhMdm00$ > > > > > > Change since v23: > > - Change wording from "Mediatek" to "MediaTek" (uppercase T) in mtk- > > svs.yaml. > > - Use cpuidle_pause_and_lock() to prevent system from entering cpuidle > > instead of applying pm_qos APIs. > > - Add kfree() at the end of svs_probe() when encountering probe fail. > > - Change MODULE_LICENSE from "GPL v2" to "GPL". > > - Add nvmem_cell_put() in error handling when nvmem_cell_read() encounters > > fail. > > I also gave you a reviewed-by on v23, but here it is again: > > Reviewed-by: Kevin Hilman <khilman@baylibre.com> > > > That being said, it would be really nice to see an integration tree > where this was all tested on mainline (e.g. v5.17, or v5.18-rc) > > For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin > board, it fails to probe[1] because there is no CCI node in the upstream > mt8183.dtsi. > > I'm assuming this series is also not very useful without the CPUfreq > series from Rex, so being able to test this, CCI and CPUfreq together on > MT8183 on a mainline kernel would be very helpful. > > Kevin > > [1] > [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node > [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe fail Just share. I've tested this series on below two platforms and it works as expected. - mt8183-Krane (kernel-v5.10) - mt8192-Hayato (kernel-v5.4) Sincerely, Roger Lu.
Hi Roger, Roger Lu <roger.lu@mediatek.com> writes: > On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: [...] >> That being said, it would be really nice to see an integration tree >> where this was all tested on mainline (e.g. v5.17, or v5.18-rc) >> >> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin >> board, it fails to probe[1] because there is no CCI node in the upstream >> mt8183.dtsi. >> >> I'm assuming this series is also not very useful without the CPUfreq >> series from Rex, so being able to test this, CCI and CPUfreq together on >> MT8183 on a mainline kernel would be very helpful. >> >> Kevin >> >> [1] >> [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node >> [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe fail > > Just share. I've tested this series on below two platforms and it works as > expected. > - mt8183-Krane (kernel-v5.10) > - mt8192-Hayato (kernel-v5.4) Unfortunately testing on v5.4 and v5.10 with lots of other additional out-of-tree patches does not give much confidence that this series works with upstream, especially when I've given a few reasons why it will not work uptream. The examples I gave above for CCI and CPUs/cluster disable are good examples, but another one I forgot to mention is the dependency on Mali. The SVS driver will never probe because it also depens on a "mali" node, which doesn't exist upstream either (but panfrost does, and acutually loads/probes fine on v5.17/v5.18) so this should be fixed to work with upstream panfrost. IMO, in order for this to be merged upstream, it should at least have some basic validation with upstream, and so far I have not even been able to make it successfuly probe. To do that, you will need to either provide a list of the dependencies for testing this with mainline (e.g. CCI series, CPUfreq series, any DT changes), or even better, an integration tree based on recent mainline (e.g. v5.17 stable, or v5.18-rc) which shows all the patches (in addition to this series) used to validate this on mainline. Thanks, Kevin
Hi Kevin, On Thu, 2022-04-21 at 12:41 -0700, Kevin Hilman wrote: > Hi Roger, > > Roger Lu <roger.lu@mediatek.com> writes: > > > On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: > > [...] > > > > That being said, it would be really nice to see an integration tree > > > where this was all tested on mainline (e.g. v5.17, or v5.18-rc) > > > > > > For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin > > > board, it fails to probe[1] because there is no CCI node in the upstream > > > mt8183.dtsi. > > > > > > I'm assuming this series is also not very useful without the CPUfreq > > > series from Rex, so being able to test this, CCI and CPUfreq together on > > > MT8183 on a mainline kernel would be very helpful. > > > > > > Kevin > > > > > > [1] > > > [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node > > > [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe > > > fail > > > > Just share. I've tested this series on below two platforms and it works as > > expected. > > - mt8183-Krane (kernel-v5.10) > > - mt8192-Hayato (kernel-v5.4) > > Unfortunately testing on v5.4 and v5.10 with lots of other additional > out-of-tree patches does not give much confidence that this series works > with upstream, especially when I've given a few reasons why it will not > work uptream. > > The examples I gave above for CCI and CPUs/cluster disable are good > examples, but another one I forgot to mention is the dependency on Mali. > The SVS driver will never probe because it also depens on a "mali" node, > which doesn't exist upstream either (but panfrost does, and acutually > loads/probes fine on v5.17/v5.18) so this should be fixed to work with > upstream panfrost. > > IMO, in order for this to be merged upstream, it should at least have > some basic validation with upstream, and so far I have not even been > able to make it successfuly probe. To do that, you will need to either > provide a list of the dependencies for testing this with mainline > (e.g. CCI series, CPUfreq series, any DT changes), or even better, an > integration tree based on recent mainline (e.g. v5.17 stable, or > v5.18-rc) which shows all the patches (in addition to this series) used > to validate this on mainline. No problem. We'll find a machine that can be run correctly with recent mainline (e.g. v5.17 stable, or v5.18-rc) and add patches (CCI series + CPUfreq series + any DT changes) to test this SVS series. Thanks very much. > > Thanks, > > Kevin > > >
On 22/04/2022 04:24, Roger Lu wrote: > Hi Kevin, > > On Thu, 2022-04-21 at 12:41 -0700, Kevin Hilman wrote: >> Hi Roger, >> >> Roger Lu <roger.lu@mediatek.com> writes: >> >>> On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: >> >> [...] >> >>>> That being said, it would be really nice to see an integration tree >>>> where this was all tested on mainline (e.g. v5.17, or v5.18-rc) >>>> >>>> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin >>>> board, it fails to probe[1] because there is no CCI node in the upstream >>>> mt8183.dtsi. >>>> >>>> I'm assuming this series is also not very useful without the CPUfreq >>>> series from Rex, so being able to test this, CCI and CPUfreq together on >>>> MT8183 on a mainline kernel would be very helpful. >>>> >>>> Kevin >>>> >>>> [1] >>>> [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node >>>> [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe >>>> fail >>> >>> Just share. I've tested this series on below two platforms and it works as >>> expected. >>> - mt8183-Krane (kernel-v5.10) >>> - mt8192-Hayato (kernel-v5.4) >> >> Unfortunately testing on v5.4 and v5.10 with lots of other additional >> out-of-tree patches does not give much confidence that this series works >> with upstream, especially when I've given a few reasons why it will not >> work uptream. >> >> The examples I gave above for CCI and CPUs/cluster disable are good >> examples, but another one I forgot to mention is the dependency on Mali. >> The SVS driver will never probe because it also depens on a "mali" node, >> which doesn't exist upstream either (but panfrost does, and acutually >> loads/probes fine on v5.17/v5.18) so this should be fixed to work with >> upstream panfrost. >> >> IMO, in order for this to be merged upstream, it should at least have >> some basic validation with upstream, and so far I have not even been >> able to make it successfuly probe. To do that, you will need to either >> provide a list of the dependencies for testing this with mainline >> (e.g. CCI series, CPUfreq series, any DT changes), or even better, an >> integration tree based on recent mainline (e.g. v5.17 stable, or >> v5.18-rc) which shows all the patches (in addition to this series) used >> to validate this on mainline. > > No problem. We'll find a machine that can be run correctly with recent mainline > (e.g. v5.17 stable, or v5.18-rc) and add patches (CCI series + CPUfreq series + > any DT changes) to test this SVS series. Thanks very much. > Thanks Roger. I'll wait until this got tested with upstream Linux, before I will apply all the patches. Regards, Matthias
On Fri, Apr 22, 2022 at 11:38 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > > > > On 22/04/2022 04:24, Roger Lu wrote: > > Hi Kevin, > > > > On Thu, 2022-04-21 at 12:41 -0700, Kevin Hilman wrote: > >> Hi Roger, > >> > >> Roger Lu <roger.lu@mediatek.com> writes: > >> > >>> On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: > >> > >> [...] > >> > >>>> That being said, it would be really nice to see an integration tree > >>>> where this was all tested on mainline (e.g. v5.17, or v5.18-rc) > >>>> > >>>> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin > >>>> board, it fails to probe[1] because there is no CCI node in the upstream > >>>> mt8183.dtsi. > >>>> > >>>> I'm assuming this series is also not very useful without the CPUfreq > >>>> series from Rex, so being able to test this, CCI and CPUfreq together on > >>>> MT8183 on a mainline kernel would be very helpful. > >>>> > >>>> Kevin > >>>> > >>>> [1] > >>>> [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node > >>>> [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe > >>>> fail > >>> > >>> Just share. I've tested this series on below two platforms and it works as > >>> expected. > >>> - mt8183-Krane (kernel-v5.10) > >>> - mt8192-Hayato (kernel-v5.4) > >> > >> Unfortunately testing on v5.4 and v5.10 with lots of other additional > >> out-of-tree patches does not give much confidence that this series works > >> with upstream, especially when I've given a few reasons why it will not > >> work uptream. > >> > >> The examples I gave above for CCI and CPUs/cluster disable are good > >> examples, but another one I forgot to mention is the dependency on Mali. > >> The SVS driver will never probe because it also depens on a "mali" node, > >> which doesn't exist upstream either (but panfrost does, and acutually > >> loads/probes fine on v5.17/v5.18) so this should be fixed to work with > >> upstream panfrost. > >> > >> IMO, in order for this to be merged upstream, it should at least have > >> some basic validation with upstream, and so far I have not even been > >> able to make it successfuly probe. To do that, you will need to either > >> provide a list of the dependencies for testing this with mainline > >> (e.g. CCI series, CPUfreq series, any DT changes), or even better, an > >> integration tree based on recent mainline (e.g. v5.17 stable, or > >> v5.18-rc) which shows all the patches (in addition to this series) used > >> to validate this on mainline. > > > > No problem. We'll find a machine that can be run correctly with recent mainline > > (e.g. v5.17 stable, or v5.18-rc) and add patches (CCI series + CPUfreq series + > > any DT changes) to test this SVS series. Thanks very much. > > > > Thanks Roger. I'll wait until this got tested with upstream Linux, before I will > apply all the patches. Hi everyone, I've put together an integration test branch: https://github.com/wens/linux/commits/mt8183-cpufreq-cci-svs-test This branch is based on next-20220422 and includes the following series: - ANX7625 DPI support v2 https://lore.kernel.org/all/20220422084720.959271-1-xji@analogixsemi.com/ - MTK SVS v24 https://lore.kernel.org/all/20220420102044.10832-1-roger.lu@mediatek.com/ - MTK cpufreq v4 https://lore.kernel.org/all/20220422075239.16437-1-rex-bc.chen@mediatek.com/ - PM / devfreq core patches from http://git.kernel.org/chanwoo/h/devfreq-testing PM / devfreq: Export devfreq_get_freq_range symbol within devfreq PM / devfreq: Add cpu based scaling support to passive governor PM / devfreq: passive: Reduce duplicate code when passive_devfreq case PM / devfreq: passive: Update frequency when start governor - CCI devfreq v2 https://lore.kernel.org/all/20220408052150.22536-1-johnson.wang@mediatek.com/ And some patches of my own to fix some errors. See the last handful of patches including and after the fixup! one. This was tested on Juniper (Acer Chromebook Spin 311) that has MT8183. Looking at the mcu_*_sel clocks from /sys/kernel/debug/clk/clk_summary , it does seem like things are happening, though I'm not sure how to thoroughly test this, especially SVS. Hope this unblocks things for everyone involved. Regards ChenYu
Chen-Yu Tsai <wenst@chromium.org> writes: > On Fri, Apr 22, 2022 at 11:38 PM Matthias Brugger > <matthias.bgg@gmail.com> wrote: >> >> >> >> On 22/04/2022 04:24, Roger Lu wrote: >> > Hi Kevin, >> > >> > On Thu, 2022-04-21 at 12:41 -0700, Kevin Hilman wrote: >> >> Hi Roger, >> >> >> >> Roger Lu <roger.lu@mediatek.com> writes: >> >> >> >>> On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: >> >> >> >> [...] >> >> >> >>>> That being said, it would be really nice to see an integration tree >> >>>> where this was all tested on mainline (e.g. v5.17, or v5.18-rc) >> >>>> >> >>>> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin >> >>>> board, it fails to probe[1] because there is no CCI node in the upstream >> >>>> mt8183.dtsi. >> >>>> >> >>>> I'm assuming this series is also not very useful without the CPUfreq >> >>>> series from Rex, so being able to test this, CCI and CPUfreq together on >> >>>> MT8183 on a mainline kernel would be very helpful. >> >>>> >> >>>> Kevin >> >>>> >> >>>> [1] >> >>>> [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node >> >>>> [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe >> >>>> fail >> >>> >> >>> Just share. I've tested this series on below two platforms and it works as >> >>> expected. >> >>> - mt8183-Krane (kernel-v5.10) >> >>> - mt8192-Hayato (kernel-v5.4) >> >> >> >> Unfortunately testing on v5.4 and v5.10 with lots of other additional >> >> out-of-tree patches does not give much confidence that this series works >> >> with upstream, especially when I've given a few reasons why it will not >> >> work uptream. >> >> >> >> The examples I gave above for CCI and CPUs/cluster disable are good >> >> examples, but another one I forgot to mention is the dependency on Mali. >> >> The SVS driver will never probe because it also depens on a "mali" node, >> >> which doesn't exist upstream either (but panfrost does, and acutually >> >> loads/probes fine on v5.17/v5.18) so this should be fixed to work with >> >> upstream panfrost. >> >> >> >> IMO, in order for this to be merged upstream, it should at least have >> >> some basic validation with upstream, and so far I have not even been >> >> able to make it successfuly probe. To do that, you will need to either >> >> provide a list of the dependencies for testing this with mainline >> >> (e.g. CCI series, CPUfreq series, any DT changes), or even better, an >> >> integration tree based on recent mainline (e.g. v5.17 stable, or >> >> v5.18-rc) which shows all the patches (in addition to this series) used >> >> to validate this on mainline. >> > >> > No problem. We'll find a machine that can be run correctly with recent mainline >> > (e.g. v5.17 stable, or v5.18-rc) and add patches (CCI series + CPUfreq series + >> > any DT changes) to test this SVS series. Thanks very much. >> > >> >> Thanks Roger. I'll wait until this got tested with upstream Linux, before I will >> apply all the patches. > > Hi everyone, > > I've put together an integration test branch: > > https://github.com/wens/linux/commits/mt8183-cpufreq-cci-svs-test > > This branch is based on next-20220422 and includes the following series: > > - ANX7625 DPI support v2 > https://lore.kernel.org/all/20220422084720.959271-1-xji@analogixsemi.com/ > - MTK SVS v24 > https://lore.kernel.org/all/20220420102044.10832-1-roger.lu@mediatek.com/ > - MTK cpufreq v4 > https://lore.kernel.org/all/20220422075239.16437-1-rex-bc.chen@mediatek.com/ > - PM / devfreq core patches from > http://git.kernel.org/chanwoo/h/devfreq-testing > PM / devfreq: Export devfreq_get_freq_range symbol within devfreq > PM / devfreq: Add cpu based scaling support to passive governor > PM / devfreq: passive: Reduce duplicate code when passive_devfreq case > PM / devfreq: passive: Update frequency when start governor > - CCI devfreq v2 > https://lore.kernel.org/all/20220408052150.22536-1-johnson.wang@mediatek.com/ > > And some patches of my own to fix some errors. See the last handful of > patches including and after the fixup! one. Thanks for setting up this branch. > This was tested on Juniper (Acer Chromebook Spin 311) that has MT8183. Is there an upstream DT for this platform? I tried this series on mt8183-pumpkin, and since the upstream DT doesn't define a CCI regulator, the CCI driver fails to probe. Without CCI, the SVS driver also fails to probe. So the platform boots, but has neither CCI nor SVS. If I add a regulator for CCI[1], it goes farther, but then fails more noisly[2] > Looking at the mcu_*_sel clocks from /sys/kernel/debug/clk/clk_summary , > it does seem like things are happening, though I'm not sure how to > thoroughly test this, especially SVS. I think you're probably seeing CPU DVFS (CPUfreq) working, but I suspect neither CCI or SVS have successfully loaded. Kevin [1] diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts index b288b508fa4c..e064c06dc0d7 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts +++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts @@ -384,6 +384,10 @@ &mfg { domain-supply = <&mt6358_vgpu_reg>; }; +&cci { + proc-supply = <&mt6358_vproc12_reg>; +}; + &cpu0 { proc-supply = <&mt6358_vproc12_reg>; }; [2] [ 0.560083] mtk-ccifreq cci: devfreq_add_device: Unable to start governor for the device [ 0.576083] ------------[ cut here ]------------ [ 0.576670] WARNING: CPU: 3 PID: 1 at drivers/devfreq/governor_passive.c:382 devfreq_passive_event_handler+0x80/0x3a0 [ 0.578021] Modules linked in: [ 0.578413] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc3-next-20220422-05886-g9cd0610279c1-dirty #58 [ 0.579653] Hardware name: Pumpkin MT8183 (DT) [ 0.580217] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.581097] pc : devfreq_passive_event_handler+0x80/0x3a0 [ 0.581780] lr : devfreq_passive_event_handler+0x7c/0x3a0 [ 0.582463] sp : ffff80000808ba40 [ 0.582883] x29: ffff80000808ba40 x28: 0000000000000000 x27: fffffffffffffdfb [ 0.583788] x26: ffff0000037ba810 x25: ffff80000808bb40 x24: ffff80000a406640 [ 0.584691] x23: ffff8000099153c8 x22: ffff0000037ba800 x21: 0000000000000000 [ 0.585595] x20: ffff0000035c40a0 x19: ffff0000035c4080 x18: 0000000000000000 [ 0.586267] mmc0: new ultra high speed SDR104 SDIO card at address 0001 [ 0.586498] x17: 6f6620726f6e7265 x16: 766f672074726174 x15: 000006837f6218f7 [ 0.588233] x14: 0000000000000320 x13: 0000000000000001 x12: 0000000000000000 [ 0.589137] x11: 0000000000000001 x10: 0000000000000a50 x9 : ffff80000808b7a0 [ 0.590041] x8 : ffff0000028b0ab0 x7 : ffff00007fb19d00 x6 : 00000000076832c3 [ 0.590945] x5 : 00000000410fd030 x4 : 0000000000000000 x3 : ffff80000a3f70e8 [ 0.591849] x2 : 0000000000000000 x1 : ffff0000028b0000 x0 : 00000000fffffffe [ 0.592753] Call trace: [ 0.593065] devfreq_passive_event_handler+0x80/0x3a0 [ 0.593706] devfreq_remove_device+0x38/0xd0 [ 0.594247] devfreq_add_device+0x328/0x5f0 [ 0.594778] devm_devfreq_add_device+0x64/0xb0 [ 0.595341] mtk_ccifreq_probe+0x340/0x4e0 [ 0.595860] platform_probe+0x68/0xe0 [ 0.596330] really_probe.part.0+0x9c/0x29c [ 0.596862] __driver_probe_device+0x98/0x144 [ 0.597416] driver_probe_device+0xac/0x140 [ 0.597948] __driver_attach+0xf8/0x190 [ 0.598436] bus_for_each_dev+0x70/0xd0 [ 0.598925] driver_attach+0x24/0x30 [ 0.599380] bus_add_driver+0x14c/0x1f0 [ 0.599868] driver_register+0x78/0x130 [ 0.600356] __platform_driver_register+0x28/0x34 [ 0.600954] mtk_ccifreq_platdrv_init+0x1c/0x28 [ 0.601531] do_one_initcall+0x50/0x1c0 [ 0.602021] kernel_init_freeable+0x20c/0x290 [ 0.602576] kernel_init+0x28/0x13c [ 0.603024] ret_from_fork+0x10/0x20 [ 0.603480] ---[ end trace 0000000000000000 ]---
On Tue, May 3, 2022 at 6:08 AM Kevin Hilman <khilman@kernel.org> wrote: > > Chen-Yu Tsai <wenst@chromium.org> writes: > > > On Fri, Apr 22, 2022 at 11:38 PM Matthias Brugger > > <matthias.bgg@gmail.com> wrote: > >> > >> > >> > >> On 22/04/2022 04:24, Roger Lu wrote: > >> > Hi Kevin, > >> > > >> > On Thu, 2022-04-21 at 12:41 -0700, Kevin Hilman wrote: > >> >> Hi Roger, > >> >> > >> >> Roger Lu <roger.lu@mediatek.com> writes: > >> >> > >> >>> On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote: > >> >> > >> >> [...] > >> >> > >> >>>> That being said, it would be really nice to see an integration tree > >> >>>> where this was all tested on mainline (e.g. v5.17, or v5.18-rc) > >> >>>> > >> >>>> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin > >> >>>> board, it fails to probe[1] because there is no CCI node in the upstream > >> >>>> mt8183.dtsi. > >> >>>> > >> >>>> I'm assuming this series is also not very useful without the CPUfreq > >> >>>> series from Rex, so being able to test this, CCI and CPUfreq together on > >> >>>> MT8183 on a mainline kernel would be very helpful. > >> >>>> > >> >>>> Kevin > >> >>>> > >> >>>> [1] > >> >>>> [ 0.573332] mtk-svs 1100b000.svs: cannot find cci node > >> >>>> [ 0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe > >> >>>> fail > >> >>> > >> >>> Just share. I've tested this series on below two platforms and it works as > >> >>> expected. > >> >>> - mt8183-Krane (kernel-v5.10) > >> >>> - mt8192-Hayato (kernel-v5.4) > >> >> > >> >> Unfortunately testing on v5.4 and v5.10 with lots of other additional > >> >> out-of-tree patches does not give much confidence that this series works > >> >> with upstream, especially when I've given a few reasons why it will not > >> >> work uptream. > >> >> > >> >> The examples I gave above for CCI and CPUs/cluster disable are good > >> >> examples, but another one I forgot to mention is the dependency on Mali. > >> >> The SVS driver will never probe because it also depens on a "mali" node, > >> >> which doesn't exist upstream either (but panfrost does, and acutually > >> >> loads/probes fine on v5.17/v5.18) so this should be fixed to work with > >> >> upstream panfrost. > >> >> > >> >> IMO, in order for this to be merged upstream, it should at least have > >> >> some basic validation with upstream, and so far I have not even been > >> >> able to make it successfuly probe. To do that, you will need to either > >> >> provide a list of the dependencies for testing this with mainline > >> >> (e.g. CCI series, CPUfreq series, any DT changes), or even better, an > >> >> integration tree based on recent mainline (e.g. v5.17 stable, or > >> >> v5.18-rc) which shows all the patches (in addition to this series) used > >> >> to validate this on mainline. > >> > > >> > No problem. We'll find a machine that can be run correctly with recent mainline > >> > (e.g. v5.17 stable, or v5.18-rc) and add patches (CCI series + CPUfreq series + > >> > any DT changes) to test this SVS series. Thanks very much. > >> > > >> > >> Thanks Roger. I'll wait until this got tested with upstream Linux, before I will > >> apply all the patches. > > > > Hi everyone, > > > > I've put together an integration test branch: > > > > https://github.com/wens/linux/commits/mt8183-cpufreq-cci-svs-test > > > > This branch is based on next-20220422 and includes the following series: > > > > - ANX7625 DPI support v2 > > https://lore.kernel.org/all/20220422084720.959271-1-xji@analogixsemi.com/ > > - MTK SVS v24 > > https://lore.kernel.org/all/20220420102044.10832-1-roger.lu@mediatek.com/ > > - MTK cpufreq v4 > > https://lore.kernel.org/all/20220422075239.16437-1-rex-bc.chen@mediatek.com/ > > - PM / devfreq core patches from > > http://git.kernel.org/chanwoo/h/devfreq-testing > > PM / devfreq: Export devfreq_get_freq_range symbol within devfreq > > PM / devfreq: Add cpu based scaling support to passive governor > > PM / devfreq: passive: Reduce duplicate code when passive_devfreq case > > PM / devfreq: passive: Update frequency when start governor > > - CCI devfreq v2 > > https://lore.kernel.org/all/20220408052150.22536-1-johnson.wang@mediatek.com/ > > > > And some patches of my own to fix some errors. See the last handful of > > patches including and after the fixup! one. > > Thanks for setting up this branch. > > > This was tested on Juniper (Acer Chromebook Spin 311) that has MT8183. > > Is there an upstream DT for this platform? mt8183-kukui-jacuzzi-juniper-sku16.dtb > I tried this series on mt8183-pumpkin, and since the upstream DT doesn't > define a CCI regulator, the CCI driver fails to probe. Without CCI, the > SVS driver also fails to probe. So the platform boots, but has neither > CCI nor SVS. > > If I add a regulator for CCI[1], it goes farther, but then fails more > noisly[2] Interesting. That doesn't happen on my system. > > Looking at the mcu_*_sel clocks from /sys/kernel/debug/clk/clk_summary , > > it does seem like things are happening, though I'm not sure how to > > thoroughly test this, especially SVS. > > I think you're probably seeing CPU DVFS (CPUfreq) working, but I suspect > neither CCI or SVS have successfully loaded. I can see CCI devfreq stats, and the clk rate being changed around: # cat /sys/class/devfreq/cci/trans_stat From : To : 273000000 338000000 403000000 463000000 546000000 624000000 689000000 767000000 845000000 871000000 923000000 9620000001027000000109200000011440000001196000000 time(ms) 273000000: 0 207 48 74 71 34 27 9 7 9 2 1 3 5 3 13 159715 * 338000000: 268 0 95 65 17 11 2 11 6 9 1 0 4 2 1 11 48589 403000000: 54 99 0 76 19 5 4 7 3 1 2 1 1 0 0 10 13320 463000000: 61 83 73 0 35 7 6 7 1 0 1 1 2 0 2 14 11628 546000000: 63 51 21 20 0 11 3 2 2 0 2 0 2 1 0 9 7919 624000000: 28 9 8 8 7 0 10 2 2 3 1 0 2 3 0 14 3030 689000000: 19 8 4 9 7 4 0 6 6 1 4 2 4 1 1 7 3908 767000000: 4 10 8 8 5 3 3 0 1 1 1 1 1 0 1 12 667 845000000: 2 4 2 3 3 3 3 1 0 1 4 1 1 0 0 10 417 871000000: 1 1 2 2 7 3 2 1 1 0 1 1 1 0 0 14 341 923000000: 2 4 1 2 3 2 1 0 0 1 0 1 4 2 0 8 131 962000000: 2 2 0 1 1 1 1 0 1 3 1 0 2 0 0 5 147 1027000000: 0 4 0 2 2 2 6 1 1 3 1 2 0 5 0 5 105 1092000000: 0 4 2 2 1 3 2 2 0 3 1 3 0 0 6 17 193 1144000000: 1 0 2 2 1 0 1 0 0 0 0 0 0 2 0 20 105 1196000000: 8 18 16 19 8 8 12 10 7 2 9 6 7 25 15 0 14210 Total transition : 2422 For SVS, it looks like we need to make Panfrost/OPP understand the 2 regulator supply design of the GPU: mtk-svs 1100b000.svs: M_HW_RES0: 0x00120090 mtk-svs 1100b000.svs: M_HW_RES1: 0xe6fcbb1b mtk-svs 1100b000.svs: M_HW_RES2: 0x478b47c7 mtk-svs 1100b000.svs: M_HW_RES3: 0xaafbf757 mtk-svs 1100b000.svs: M_HW_RES4: 0xe6fca3ec mtk-svs 1100b000.svs: M_HW_RES5: 0x47bd4b90 mtk-svs 1100b000.svs: M_HW_RES6: 0xaafb87ab mtk-svs 1100b000.svs: M_HW_RES7: 0xe6fc9032 mtk-svs 1100b000.svs: M_HW_RES8: 0x4bbd4bf1 mtk-svs 1100b000.svs: M_HW_RES9: 0xaafbd97f mtk-svs 1100b000.svs: M_HW_RES14: 0xf6ce0dcb mtk-svs 1100b000.svs: M_HW_RES15: 0x025a0015 mtk-svs 1100b000.svs: M_HW_RES16: 0xe6fc8d1f mtk-svs 1100b000.svs: M_HW_RES17: 0x47bd47f1 mtk-svs 1100b000.svs: M_HW_RES18: 0xaafbc050 panfrost 13040000.gpu: opp_parse_supplies: Invalid number of elements in opp-microvolt property (2) with supplies (1) panfrost 13040000.gpu: _of_add_opp_table_v2: Failed to add OPP, -22 SVSB_GPU: add opp table fail: -22 mtk-svs 1100b000.svs: svs bank resource setup fail: -22 mtk-svs: probe of 1100b000.svs failed with error -22 Regards ChenYu > Kevin > > > > [1] > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts > index b288b508fa4c..e064c06dc0d7 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts > +++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts > @@ -384,6 +384,10 @@ &mfg { > domain-supply = <&mt6358_vgpu_reg>; > }; > > +&cci { > + proc-supply = <&mt6358_vproc12_reg>; > +}; > + > &cpu0 { > proc-supply = <&mt6358_vproc12_reg>; > }; > > > [2] > [ 0.560083] mtk-ccifreq cci: devfreq_add_device: Unable to start governor for the device > [ 0.576083] ------------[ cut here ]------------ > [ 0.576670] WARNING: CPU: 3 PID: 1 at drivers/devfreq/governor_passive.c:382 devfreq_passive_event_handler+0x80/0x3a0 > [ 0.578021] Modules linked in: > [ 0.578413] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc3-next-20220422-05886-g9cd0610279c1-dirty #58 > [ 0.579653] Hardware name: Pumpkin MT8183 (DT) > [ 0.580217] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 0.581097] pc : devfreq_passive_event_handler+0x80/0x3a0 > [ 0.581780] lr : devfreq_passive_event_handler+0x7c/0x3a0 > [ 0.582463] sp : ffff80000808ba40 > [ 0.582883] x29: ffff80000808ba40 x28: 0000000000000000 x27: fffffffffffffdfb > [ 0.583788] x26: ffff0000037ba810 x25: ffff80000808bb40 x24: ffff80000a406640 > [ 0.584691] x23: ffff8000099153c8 x22: ffff0000037ba800 x21: 0000000000000000 > [ 0.585595] x20: ffff0000035c40a0 x19: ffff0000035c4080 x18: 0000000000000000 > [ 0.586267] mmc0: new ultra high speed SDR104 SDIO card at address 0001 > [ 0.586498] x17: 6f6620726f6e7265 x16: 766f672074726174 x15: 000006837f6218f7 > [ 0.588233] x14: 0000000000000320 x13: 0000000000000001 x12: 0000000000000000 > [ 0.589137] x11: 0000000000000001 x10: 0000000000000a50 x9 : ffff80000808b7a0 > [ 0.590041] x8 : ffff0000028b0ab0 x7 : ffff00007fb19d00 x6 : 00000000076832c3 > [ 0.590945] x5 : 00000000410fd030 x4 : 0000000000000000 x3 : ffff80000a3f70e8 > [ 0.591849] x2 : 0000000000000000 x1 : ffff0000028b0000 x0 : 00000000fffffffe > [ 0.592753] Call trace: > [ 0.593065] devfreq_passive_event_handler+0x80/0x3a0 > [ 0.593706] devfreq_remove_device+0x38/0xd0 > [ 0.594247] devfreq_add_device+0x328/0x5f0 > [ 0.594778] devm_devfreq_add_device+0x64/0xb0 > [ 0.595341] mtk_ccifreq_probe+0x340/0x4e0 > [ 0.595860] platform_probe+0x68/0xe0 > [ 0.596330] really_probe.part.0+0x9c/0x29c > [ 0.596862] __driver_probe_device+0x98/0x144 > [ 0.597416] driver_probe_device+0xac/0x140 > [ 0.597948] __driver_attach+0xf8/0x190 > [ 0.598436] bus_for_each_dev+0x70/0xd0 > [ 0.598925] driver_attach+0x24/0x30 > [ 0.599380] bus_add_driver+0x14c/0x1f0 > [ 0.599868] driver_register+0x78/0x130 > [ 0.600356] __platform_driver_register+0x28/0x34 > [ 0.600954] mtk_ccifreq_platdrv_init+0x1c/0x28 > [ 0.601531] do_one_initcall+0x50/0x1c0 > [ 0.602021] kernel_init_freeable+0x20c/0x290 > [ 0.602576] kernel_init+0x28/0x13c > [ 0.603024] ret_from_fork+0x10/0x20 > [ 0.603480] ---[ end trace 0000000000000000 ]--- >