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