[PATCH 1/2] clk: rockchip: rk3399: add 85.5 MHz rate to PLL rate table

[Alexey Charkov]

Hi Vasily,

On Fri, Oct 30, 2026 at 9:01 AM Vasily Khoruzhick <anarsoul at gmail.com> wrote:
>
> On Sat, Sep 5, 2026 at 11:31 PM Alexey Charkov <alchark at flipper.net> wrote:
> > > > I believe something is hardcoding the divisor (or leaving it at the
> > > > power-on default) in the actual DCLK of a VOP which the HDMI
> > > > controller uses, instead of modelling it properly as a mux (frac/div)
> > > > feeding off VPLL via another mux - both perfectly representable in the
> > > > clock framework and already envisaged in the clock driver, making the
> > > > PLL table patching unnecessary.
> > > >
> > > > Can you please try re-pointing the "ref" clock at DCLK_VOP0 (or 1,
> > > > depending on which one your HDMI controller uses)? Your clock summary
> > > > already shows that something is assigning its parent to DCLK_VOPx_DIV
> > > > and the latter's parent to PLL_VPLL, so rate changes the current
> > > > driver code does on VPLL propagate to the muxed and divided downstream
> > > > consumer as a side-effect rather than as an actual rate request.
> > >
> > > I don't think it is a reasonable ask. Wrong clock for the HDMI encoder
> > > would just result in non-working HDMI output and no feasible way for
> > > me to debug what is wrong.
> >
> > Hope you have a serial console or ssh connection to the board. If it
> > doesn't work you'll know soon enough.
> >
> > What I'm pointing out is that the clock setup implied by the current
> > dtsi doesn't make much sense: VOP's dclk is driven off VPLL via a mux
> > and divisor. If the encoder's pixel clock is truly driven directly off
> > VPLL then Rockchip designed the muxes and dividers for nothing, as
> > they will always be no-ops. This doesn't square, so something must be
> > wrong, and I believe it's the dtsi.
> >
> > I'll be happy to be proven wrong here.
>
> Hey Alexey,
>
> Sorry for the long delay - NVMe in my laptop died and I had to wait
> for a warranty replacement to arrive. I would have bought a new one if
> the price wasn't ridiculous :)
>
> The dtsi describes the hardware correctly. The HDMI PHY's reference
> clock input (TRM Part 1, 1.1.11: "13.5-600MHz input reference clock")
> is wired to VPLL with no mux or divider in between; it is not one of
> the CRU clocks in the TRM's clock tree at all.

The TRM unfortunately only says there is an input reference clock but
gives no detail on how it's wired.

> That's exactly what the commit that introduced the "vpll" clock says,
> from Rockchip's own engineers, see 5e3bc6d1ab48 ("drm/rockchip:
> dw_hdmi: introduce the VPLL clock setting")

That commit indeed mentions that there is no divider between the VPLL
and HDMI PHY. Looks like it's our only evidence, but I believe we
should trust the information in Mark Yao's commit message, given his
Rockchip sign-off line.

> This bug report itself is a solid proof of this wiring. With VPLL left
> at 74.25 MHz and the VOP on GPLL/7 = 84.857 MHz, the monitor reported
> the mode as 1195x768. 74.25/84.857 = 0.875, and 1366 * 0.875 = 1195.
> The TMDS clock came from VPLL while the sync timing came from the VOP.
> If the PHY were clocked from the dclk mux output, that mismatch could
> not exist.

Fair enough.

> The mux and dividers are not for nothing: there is one VPLL and two
> VOPs, and each VOP can also drive eDP, MIPI DSI and DP, whose PHYs
> have their own PLLs. When VOPB drives HDMI from VPLL/1, VOPL has to
> run from CPLL or GPLL through the divider, and the fractional divider
> exists for DSI panels with odd pixel clocks. For HDMI the divider is
> legitimately 1.
>
> This is also why the PLL table patch is needed under any scheme: the
> RK3399 PLL driver is table-only (rockchip_pll_determine_rate() snaps
> to the nearest lower table entry), and the PHY reference has to be
> 85.5 MHz exactly because nothing between VPLL and the PHY can divide
> it.
>
> And the VOP patch doesn't hardcode a divisor. The VOP still makes a
> real clk_set_rate() on its dclk at enable time. The bug was ordering:
> mode_fixup() rounded the pixel clock through the dclk while VPLL was
> still at its old rate, so the composite chose GPLL/7 and the encoder
> then asked VPLL for 84.857 MHz, which the table snapped to 74.25 MHz.
> With VPLL programmed first, the composite picks VPLL with divider 1
> because it is the exact match.

Ack. Thanks a lot for double-checking all of these - it's great to
have your explanations recorded on the mailing list for posterity!

Back to the patch itself. The new rate you're defining is consistent
with the existing entry for RK3036_PLL_RATE(1368000000, 1, 57, 1, 1,
1, 0), and the parameters are within spec (VCO rate of 1368000000
falls within the documented range of 800 MHz to 3.2 GHz). Feel free to
include my:

Reviewed-by: Alexey Charkov <alchark at flipper.net>

On the existing 1368000000 entry, I'm pretty sure it would produce
less jitter as RK3036_PLL_RATE(1368000000, 1, 114, 2, 1, 1, 0), by the
virtue of FVCO=2736MHz falling into the higher end of the allowed
band. This won't apply to your 85.5MHz rate though, because the
required divisors won't fit in two 3-bit registers.

Best regards,
Alexey