mbox series

[00/10] MTK: Undesired set_rate on main PLLs and GPU DVFS

Message ID 20221024102307.33722-1-angelogioacchino.delregno@collabora.com
Headers
Series MTK: Undesired set_rate on main PLLs and GPU DVFS |

Message

AngeloGioacchino Del Regno Oct. 24, 2022, 10:22 a.m. UTC 
  There's no reason to declare CLK_SET_RATE_PARENT on main system PLL
dividers, as any rate change on those (mainpll, syspll, univpll) will
change clock rates on critical system peripherals and busses.

This change was performed only on SoCs that I could test... I'm sure
that the same can (and should) be done on more MTK clock drivers for
practically all MTK SoCs, but I don't feel confident in pushing things
that I couldn't test, so that's done only for MT8173/83/86/92/95 and
on MT6795.
While at it, I've also added the much needed clock notifier treatment
on MT8186 for GPU DVFS, like done on other clock drivers in the past.

A broader explanation about why this series is needed can be found in
the first commit that drops this flag from fixed dividers of main and
univ PLLs (on MT8186) but, for the ones reading mainly the cover letter,
here's a copy-paste of that commit description:

The mainpll and univpll clocks are used as clock sources for multiple
peripherals of different kind, some of which are critical (like AXIs);
a rate change on any of these two will produce a rate change on many
devices and that's likely to produce system instability if not done
correctly: this is the reason why we have "fixed factor" clocks, used
by MUX clocks to provide different rates based on PLL output dividers.
    
Though, there's one fundamental issue that must be resolved somehow:
    
When performing GPU DVFS, we get a rate request that will try to change
the frequency of MAINPLL due to the CLK_TOP_MFG mux having clk26m,
mfgpll (the GPU dedicated PLL), mainpll_d3, mainpll_d5 (fixed factor
dividers) as possible parents.
    
In order to solve that, there are two ways:
 1. Add new "fake" mainpll_d3_fixed, mainpll_d5_fixed clocks, clones
    of mainpll_d3, mainpll_d5 clocks, for the only purpose of not
    declaring CLK_SET_RATE_PARENT; or
 2. Simply drop said flag from the original dividers.
    
After some careful validation, I cannot see anything calling a rate
change request during runtime for MAINPLL, nor for UNIVPLL (which would,
again, mean that we're reclocking lots of peripherals at once!), so it
is safe *and sane* to simply remove the CLK_SET_RATE_PARENT flag to all
of the main/univpll fixed factor divider clocks.
    
Besides, if for any (doubtful) reason main/univpll rate change will be
required in the future, it's still possible to call that on the PLL main
clocks, so we're still covered anyway.

AngeloGioacchino Del Regno (10):
  clk: mediatek: clk-mtk: Allow specifying flags on mtk_fixed_factor
    clocks
  clk: mediatek: mt8186-topckgen: Drop flags for main/univpll fixed
    factors
  clk: mediatek: mt8183: Compress top_divs array entries
  clk: mediatek: mt8183: Drop flags for sys/univpll fixed factors
  clk: mediatek: mt8173: Drop flags for main/sys/univpll fixed factors
  clk: mediatek: mt6795-topckgen: Drop flags for main/sys/univpll fixed
    factors
  clk: mediatek: mt8192: Drop flags for main/univpll fixed factors
  clk: mediatek: mt8195-topckgen: Drop flags for main/univpll fixed
    factors
  clk: mediatek: mt8186-mfg: Propagate rate changes to parent
  clk: mediatek: mt8186-topckgen: Add GPU clock mux notifier

 drivers/clk/mediatek/clk-mt6795-topckgen.c |  76 ++++----
 drivers/clk/mediatek/clk-mt8173.c          |  76 ++++----
 drivers/clk/mediatek/clk-mt8183.c          | 216 +++++++--------------
 drivers/clk/mediatek/clk-mt8186-mfg.c      |   5 +-
 drivers/clk/mediatek/clk-mt8186-topckgen.c |  89 ++++++---
 drivers/clk/mediatek/clk-mt8192.c          |  76 ++++----
 drivers/clk/mediatek/clk-mt8195-topckgen.c |  78 ++++----
 drivers/clk/mediatek/clk-mtk.c             |   2 +-
 drivers/clk/mediatek/clk-mtk.h             |   7 +-
 9 files changed, 293 insertions(+), 332 deletions(-)

Comments

Chen-Yu Tsai Nov. 29, 2022, 7:03 a.m. UTC | #1 
On Mon, 24 Oct 2022 12:22:57 +0200, AngeloGioacchino Del Regno wrote:
> There's no reason to declare CLK_SET_RATE_PARENT on main system PLL
> dividers, as any rate change on those (mainpll, syspll, univpll) will
> change clock rates on critical system peripherals and busses.
> 
> This change was performed only on SoCs that I could test... I'm sure
> that the same can (and should) be done on more MTK clock drivers for
> practically all MTK SoCs, but I don't feel confident in pushing things
> that I couldn't test, so that's done only for MT8173/83/86/92/95 and
> on MT6795.
> While at it, I've also added the much needed clock notifier treatment
> on MT8186 for GPU DVFS, like done on other clock drivers in the past.
> 
> [...]

Applied, thanks!

[01/10] clk: mediatek: clk-mtk: Allow specifying flags on mtk_fixed_factor clocks
        commit: 672c779e4cff5f4a103077e9b398f144c85db802
[02/10] clk: mediatek: mt8186-topckgen: Drop flags for main/univpll fixed factors
        commit: 295de9d0d063cc576c5c6322aeeda64d3579d9e5
[03/10] clk: mediatek: mt8183: Compress top_divs array entries
        commit: 23037ab63336a4a1d98645bf7ee76240ed20bc65
[04/10] clk: mediatek: mt8183: Drop flags for sys/univpll fixed factors
        commit: c01d64ca5166fa88d27c7c4a2a294dd10d20f780
[05/10] clk: mediatek: mt8173: Drop flags for main/sys/univpll fixed factors
        commit: f757c9e951b89c40db41592a22785b5a25c58224
[06/10] clk: mediatek: mt6795-topckgen: Drop flags for main/sys/univpll fixed factors
        commit: 0cf308ee3472019539582ee279b637beb34ad2ff
[07/10] clk: mediatek: mt8192: Drop flags for main/univpll fixed factors
        commit: b56603285f7e323591267bec9a9d6950e9bdb7cb
[08/10] clk: mediatek: mt8195-topckgen: Drop flags for main/univpll fixed factors
        commit: 327eeb6c240ab9293ab730789ea651fbc3fe6123
[09/10] clk: mediatek: mt8186-mfg: Propagate rate changes to parent
        commit: ecc639ddbe0d7bf1c66f6c69ee54ee005484d886
[10/10] clk: mediatek: mt8186-topckgen: Add GPU clock mux notifier
        commit: 3256ea4f6582d2cb9b63ad96451957c217a52582

Best regards,