>> >>> >>> I suspect mipi_dsi_pxclk_div was added to achieve fractional vclk/bitclk ratios, >>> since it doesn't exist on AXG. Not sure we would ever need it... and none >>> of the other upstream DSI drivers supports such setups. >>> >>> The main reasons I set only mipi_dsi_pxclk in DT is because : >>> 1) the DSI controller requires a bitclk to respond, pclk is not enough >>> 2) GP0 is disabled with an invalid config at cold boot, thus we cannot >>> rely on a default/safe rate on an initial prepare_enable(). >>> This permits setting initial valid state for the DSI controller, while >>> the actual bitclk and vclk are calculated dynamically with panel/bridge >>> runtime parameters. >> Nothing against setting rate in DT when it is static. Setting it then >> overriding it is not easy to follow. > > Yup, would be simpler to only have parenting set in DT, since it must > stay static, I'm fine trying to move rate setup to code. > >> To work around GP0 not being set, assuming you want to keep rate >> propagation as it is, you could call clk_set_rate() on cts_encl (possibly w/o >> enabling it) to force a setup on gp0 then clk_prepare_enable() on >> pxclk. You'd get a your safe rate on GP0 and the clock you need on pxclk. >> It is a bit hackish. Might be better to claim gp0 in your driver to >> manage it directly, cutting rate propagation above it to control each >> branch of the subtree as you need. It seems you need to have control over >> that anyway and it would be clear GP0 is expected to belong to DSI. > > Controlling the PLL from the DSI controller seems violating too much layers, > DSI controller driver is not feed directly by the PLL so it's a non-sense > regarding DT properties. Not sure what you mean by this. You have shown in your the commit message that the DSI clocks make significant subtree. I don't see a problem if you need to manage the root of that subtree. I'd be great if you didn't need to, but it is what it is ... > > Setting a safe clock from the DSI controller probe is an idea, but again I > don't know which value I should use... You mentionned that the problem comes DSI bridges that needs to change at runtime. I don't know much about those TBH, but is there anyway you can come up with a static GP0 rate that would then be able to divide to serve all the rates bridge would need in your use case ? GP0 can go a lot higher than ~100MHz and there are dividers unused in the tree it seems. I suppose there is a finite number of required rate for each use case ? If there are not too many and there is a common divider that allows a common rate GP0 can do, it would solve your problem. It's a lot of if but It is worth checking. This is how audio works and DT assigned rate is a good match in this case. > > I'll review the clk parenting flags and try to hack something. > > Thanks, > Neil > >
