[PATCH 2/5] drm/bridge: samsung-dsim: reread ref clock before configuring PLL

[Inki Dae]

2023년 9월 4일 (월) 오후 8:05, Michael Tretter <m.tretter at pengutronix.de>님이 작성:

> On Mon, 04 Sep 2023 13:38:33 +0900, Inki Dae wrote:
> > 2023년 8월 29일 (화) 오전 12:59, Michael Tretter <m.tretter at pengutronix.de>님이
> 작성:
> >
> > >
> > > 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 at pengutronix.de>
> > > ---
> > >  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
> > >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230905/6db66061/attachment.htm>