<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">2023년 9월 4일 (월) 오후 8:05, Michael Tretter <<a href="mailto:m.tretter@pengutronix.de">m.tretter@pengutronix.de</a>>님이 작성:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, 04 Sep 2023 13:38:33 +0900, Inki Dae wrote:<br>
> 2023년 8월 29일 (화) 오전 12:59, Michael Tretter <<a href="mailto:m.tretter@pengutronix.de" target="_blank" rel="noreferrer">m.tretter@pengutronix.de</a>>님이 작성:<br>
> <br>
> ><br>
> > The PLL reference clock may change at runtime. Thus, reading the clock<br>
> > rate during probe is not sufficient to correctly configure the PLL for<br>
> > the expected hs clock.<br>
> ><br>
> > Read the actual rate of the reference clock before calculating the PLL<br>
> > configuration parameters.<br>
> ><br>
> > Signed-off-by: Michael Tretter <<a href="mailto:m.tretter@pengutronix.de" target="_blank" rel="noreferrer">m.tretter@pengutronix.de</a>><br>
> > ---<br>
> > drivers/gpu/drm/bridge/samsung-dsim.c | 16 +++++++++-------<br>
> > include/drm/bridge/samsung-dsim.h | 1 +<br>
> > 2 files changed, 10 insertions(+), 7 deletions(-)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c<br>
> > index 6778f1751faa..da90c2038042 100644<br>
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c<br>
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c<br>
> > @@ -611,7 +611,12 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,<br>
> > u16 m;<br>
> > u32 reg;<br>
> ><br>
> > - fin = dsi->pll_clk_rate;<br>
> > + if (dsi->pll_clk)<br>
> > + fin = clk_get_rate(dsi->pll_clk);<br>
> > + else<br>
> > + fin = dsi->pll_clk_rate;<br>
> > + dev_dbg(dsi->dev, "PLL ref clock freq %lu\n", fin);<br>
> > +<br>
> <br>
> Could you share us the actual use case that in runtime the pll<br>
> reference clock can be changed?<br>
<br>
On i.MX8M Nano, the VIDEO_PLL_CLK drives the DISPLAY_PIXEL_CLK_ROOT, which is<br>
used as pixel clock by the LCDIF. Changes to the pixel clock propagate to the<br>
VIDEO_PLL_CLK and may reconfigure the VIDEO_PLL_CLK. This is done to reduce<br>
the error on the pixel clock.<br>
<br>
As the ADV3575 as MIPI-DSI device reconstructs the pixel clock, it is<br>
necessary to keep the pixel clock and MIDI-DSI reference clock in<br>
sync. This can be done by using the VIDEO_PLL_CLK to drive the PLL reference<br>
clock (MIPI_DSI_CORE_CLK_ROOT). Without this, a connected HDMI Monitor will<br>
occasionally loose sync.<br>
<br>
In this setup, a mode change that changes the pixel clock may change the<br>
VIDEO_PLL, which will change the PLL reference clock.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Thanks for sharing.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> <br>
> This patch is trying to change clock binding behavior which is<br>
> described in dt binding[1]<br>
> [1] Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml<br>
> <br>
> It says,<br>
> "DISM oscillator clock frequency. If absent, the clock frequency of<br>
> sclk_mipi will be used instead."<br>
> <br>
> However, this patch makes the sclk_mipi to be used first.<br>
<br>
No, the behavior, as described in the dt binding, is preserved by the hunk<br>
below. dsi->pll_clk is only set, if the samsung,pll-clock-frequency property<br>
is absent. If the dt property exists, dsi->pll_clk will be NULL and<br>
dsi->pll_clk_rate will be used here.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">It's true. No behavior change. There was my mistake. Thanks. :)</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Michael<br>
<br>
> <br>
> Thanks,<br>
> Inki Dae<br>
> <br>
> > fout = samsung_dsim_pll_find_pms(dsi, fin, freq, &p, &m, &s);<br>
> > if (!fout) {<br>
> > dev_err(dsi->dev,<br>
> > @@ -1821,18 +1826,15 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)<br>
> > u32 lane_polarities[5] = { 0 };<br>
> > struct device_node *endpoint;<br>
> > int i, nr_lanes, ret;<br>
> > - struct clk *pll_clk;<br>
> ><br>
> > ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",<br>
> > &dsi->pll_clk_rate, 1);<br>
> > /* If it doesn't exist, read it from the clock instead of failing */<br>
> > if (ret < 0) {<br>
> > dev_dbg(dev, "Using sclk_mipi for pll clock frequency\n");<br>
> > - pll_clk = devm_clk_get(dev, "sclk_mipi");<br>
> > - if (!IS_ERR(pll_clk))<br>
> > - dsi->pll_clk_rate = clk_get_rate(pll_clk);<br>
> > - else<br>
> > - return PTR_ERR(pll_clk);<br>
> > + dsi->pll_clk = devm_clk_get(dev, "sclk_mipi");<br>
> > + if (IS_ERR(dsi->pll_clk))<br>
> > + return PTR_ERR(dsi->pll_clk);<br>
> > }<br>
> ><br>
> > /* If it doesn't exist, use pixel clock instead of failing */<br>
> > diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h<br>
> > index 05100e91ecb9..31ff88f152fb 100644<br>
> > --- a/include/drm/bridge/samsung-dsim.h<br>
> > +++ b/include/drm/bridge/samsung-dsim.h<br>
> > @@ -87,6 +87,7 @@ struct samsung_dsim {<br>
> > void __iomem *reg_base;<br>
> > struct phy *phy;<br>
> > struct clk **clks;<br>
> > + struct clk *pll_clk;<br>
> > struct regulator_bulk_data supplies[2];<br>
> > int irq;<br>
> > struct gpio_desc *te_gpio;<br>
> ><br>
> > --<br>
> > 2.39.2<br>
> ><br>
> <br>
</blockquote></div></div></div>