Re: [PATCH] clk: sunxi-ng: div: implement set_rate_and_parent

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

Thanks a lot for the quick review!

So the patch itself is OK, but I think there is also something wrong
with the GPU mod clock. The GPU clock is not a standard divider, but
actually a fractional one. The formula for the output is

    Clock Source * ((16-M)/16)

with M being 0~15.

You were right, and it was worse than I expected.  I checked the T527
user manual (v0.92, section 2.7.6.58: "FACTOR_M: mask M cycles at 16
cycles") and then measured it on the Orange Pi 4A, forcing each OPP
and reading the real GPU frequency with the Mali cycle counter.  With
the current linear model:

  OPP request    programmed    measured
  150 MHz        600M, M=3     ~487 MHz
  200 MHz        800M, M=3      648 MHz
  300 MHz        600M, M=1      560 MHz
  400 MHz        800M, M=1      749 MHz
  600 MHz        600M, M=0      599 MHz

so every OPP below 600 MHz silently overclocks (thermal throttling to
"400 MHz" actually raises the clock to 750 MHz).  Interestingly the
vendor BSP models this register as a linear divider too, so the
vendor kernel has the same mislabelling; it also removed the 800M
parent from its parent list citing GPU job faults.

I have a fix working: a small ccu type implementing the cycle-masking
semantics, with the A523 GPU clock switched over to it, the 800M
parent dropped from the selectable parents, and the OPP table for the
Orange Pi 4A.  Re-measured with the same cycle-counter method the five
OPPs now come out at 149/199/300/399/597 MHz from the intended
parents.  I'll send it as a follow-up series referencing this thread.

And we're also missing a notifier to switch the GPU clock away from
the PLL while the PLL is being changed.

Agreed — the series also registers the existing sunxi-ng mux notifier
on pll-gpu (cpux precedent), parking the GPU on pll-periph0-600M
during PLL rate changes.  The manual states the mux switch is
glitch-free.  Nothing retunes pll-gpu at runtime yet with the standard
OPPs, but the higher speed-bin operating points (648-792 MHz) will
need it as a live parent.

Do you plan on implementing GPU DVFS on mainline?

Yes — see above; the series is on its way.  The speed-bin OPPs (SID
efuse gated) are left for a follow-up.  Happy to test any related
series on this board in the meantime.

Juan Manuel