Re: [PATCH v5 11/21] drm/bridge: inno-hdmi: Add .mode_valid platform operation

[Michal Wilczynski]

On 9/30/26 12:19, Heiko Stübner wrote:
> Am Dienstag, 29. September 2026, 12:31:08 Mitteleuropäische Sommerzeit schrieb Michal Wilczynski:
>> inno_hdmi_bridge_mode_valid() validates the pixel clock against
>> hdmi->refclk, but that clock is optional and is only present on
>> platforms that describe a "ref" clock. Platforms where the pixel clock
>> is produced by a separate device - such as the StarFive JH7110, whose
>> PHY is both the clock and the PHY provider - have no "ref" clock, so
>> the check is skipped entirely and every mode is reported as valid.
>>
>> A mode the platform cannot generate is then advertised to userspace.
>> The subsequent modeset appears to succeed, since the atomic enable path
>> cannot fail, and the display silently stays blank.
>>
>> Add a .mode_valid platform operation so platforms can reject modes they
>> are unable to drive. Platforms that do not implement it are unaffected.
>>
>> Reviewed-by: Joshua Peisach <jpeisach@xxxxxxxxxx>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
> 
> In patch 19 you add the jh7110-inno-hdmi-phy variant of the phy which
> registers a clock provider for the pixel clock. Isn't that the refclk you're
> missing?

It's the same clock the controller already consumes, except it is not
the ref clock it is the pixel clock and on the JH7110 those are two
separate clocks.

>From TRM [1] table 5-3 page 530 for the HDMI block:

u0_hdmi_tx.clk_mclk   <- clk_hdmitx0_mclk     51.2 MHz
u0_hdmi_tx.clk_bclk   <- clk_hdmitx0_sck      12.8 MHz
u0_hdmi_tx.clk_sys    <- clk_disp_apb         51.2 MHz
u0_hdmi_tx.clk_ref    <- clk_hdmi_phy_ref     24 MHz, fixed

and in the Source section of the same table:

clk_hdmitx0_pixelclk  <- u0_hdmi_tx.clk_pix   max 297 MHz

So clk_ref is a fixed 24 MHz input, while the pixel clock is an output
from the same block, which voutcrg then routes to the DC8200 pix0/pix1
MUXes and the DSI TX clk_dpi. Calling it "ref" would be wrong.

There is other reason as well. hdmi->refclk is fetched with
devm_clk_get_optional_enabled() so it stays enabled for the lifetime
of the controller, and init_hw() passes clk_get_rate(refclk) to
inno_hdmi_i2c_init() for the DDC divider.
Applied to a clock that moves between 25 and 297 MHz per
mode and reads back 0 before the first modeset they would enable the
pre PLL at probe and compute the DDC divider from a meaningless rate.

[1] - https://doc-en.rvspace.org/JH7110/PDF/JH7110_TRM_StarFive_Preliminary_V2.pdf


> 
> Thanks
> Heiko
> 
> 
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>