Hi Sascha, On 2023-02-07 13:51, Sascha Hauer wrote: > On Tue, Feb 07, 2023 at 11:01:26AM +0000, Jonas Karlman wrote: >> Hi Sascha, >> >> On 2023-02-07 09:44, Sascha Hauer wrote: >>> The Rockchip PLL drivers are currently table based and support only >>> the most common pixelclocks. Discard all modes we cannot achieve >>> at all. Normally the desired pixelclocks have an exact match in the >>> PLL driver, nevertheless allow for a 0.1% error just in case. >>> >>> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@xxxxxxxxx> >>> Tested-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> >>> Tested-by: Dan Johansen <strit@xxxxxxxxxxx> >>> Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@xxxxxxxxxxxxxx>>>> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> >>> --- >>> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >>> index feba6b9becd6c..725952811752b 100644 >>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >>> @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data, >>> { >>> struct rockchip_hdmi *hdmi = data; >>> const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg; >>> - int pclk = mode->clock * 1000; >>> + int rpclk, pclk = mode->clock * 1000; >>> bool exact_match = hdmi->plat_data->phy_force_vendor; >>> int i; >>> >>> + rpclk = clk_round_rate(hdmi->ref_clk, pclk); >>> + if (abs(rpclk - pclk) > pclk / 1000) >>> + return MODE_NOCLOCK; >> >> The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock. > > That's a bit unfortunate as we can't do this check then on these SoCs. > > The clock is likely actually there in the system and maybe even in the > clock driver, just not wired up to the HDMI. I don't know which one it > is though, so I am afraid there's not much I can do about it other than > just skipping the check when the clock is not there. For rk3228/rk3328 I think just skipping the rate check when there is no ref_clk should be fine. The clock is provided by phy-rockchip-inno-hdmi.c for rk3228/rk3328. And I recall we used to add ref/vpll clock to device tree to do something similar in LibreELEC with our initial work-in-progress HDMI2.0 patches. We have since then opted to just filter modes based on clk_round_rate for !phy_force_vendor and instead extend the inno-hdmi pll table with common hdmi/dvi clock rates, see [1] for the current state of RK HDMI2.0 related patches we use in LibreELEC. Hopefully I can find some time in coming weeks to restart the work of testing and sending those patches upstream, rebased on top of this series. [1] https://github.com/Kwiboo/linux-rockchip/compare/v6.2-rc7...rockchip-6.2-hdmi2.0 Regards, Jonas > > Sascha >