Hi Hugo,

On Tue, Oct 28, 2025 12:39 PM, Hugo Villeneuve wrote:

> > > If we arrive at this point, it seems that these values:
> > >     priv->mux_dsi_div_params.dsi_div_a
> > >     priv->mux_dsi_div_params.dsi_div_b
> > >
> > > were not initialised by the previous loop. Is this expected? If yes, 
> > > maybe a comment would help?
>
> So are the uninitialised values valid at all?

Actually, after some more testing......I remember what I did.

The PLL5 needs to support both the MIPI-DSI and DPI (Parallel) use cases.

But, since the execution paths are different in the kernel for MIPI vs DPI, I 
needed to make the default settings for DPI
knowing that if MIPI-DSI was used, they would get overwritten (hence the new 
API was introduced)

However, current defaults today in the driver are illegal for DPI, even though 
they clearly work in real life on all the RZ/G2UL boards.
        priv->mux_dsi_div_params.clksrc = 1; /* Use clk src 1 for DSI */
        priv->mux_dsi_div_params.dsi_div_a = 1; /* Divided by 2 */
        priv->mux_dsi_div_params.dsi_div_b = 2; /* Divided by 3 */

Side note, that code comment "Use clk src 1 for DSI " is wrong ...it should say 
"DPI"

So what we have today works fine, but technically does not match the hardware 
manual for the DPI case.
I need to change the code around for the DPI case and test again to make sure 
nothing breaks.

Ugh!

> > dev_warn(priv->dev, "Failed to calculate exact PLL5 settings\n");
>
> Similar to my comment above, would it be a good idea to add something like 
> "Failed to calculate exact PLL5 settings, using defaults\n" ?

I can agree to that.
I'll change the message.

Chris

Reply via email to