Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230818-samsung-dsim-v1-0-b39716db6b7a@pengutronix.de> <20230818-samsung-dsim-v1-2-b39716db6b7a@pengutronix.de>
In-Reply-To: <20230818-samsung-dsim-v1-2-b39716db6b7a@pengutronix.de>
From:   Inki Dae <daeinki@gmail.com>
Date:   Mon, 4 Sep 2023 13:38:33 +0900
Message-ID: <CAAQKjZOuRVsF7vE6ghBG7KH_QkE-5_UXjXMY080ynzZLpDjs7w@mail.gmail.com>
Subject: Re: [PATCH 2/5] drm/bridge: samsung-dsim: reread ref clock before
 configuring PLL
To:     Michael Tretter <m.tretter@pengutronix.de>
Cc:     Jagan Teki <jagan@amarulasolutions.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Andrzej Hajda <andrzej.hajda@intel.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Robert Foss <rfoss@kernel.org>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

2023=EB=85=84 8=EC=9B=94 29=EC=9D=BC (=ED=99=94) =EC=98=A4=EC=A0=84 12:59, =
Michael Tretter <m.tretter@pengutronix.de>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=
=84=B1:

>
> 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@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/brid=
ge/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 sam=
sung_dsim *dsi,
>         u16 m;
>         u32 reg;
>
> -       fin =3D dsi->pll_clk_rate;
> +       if (dsi->pll_clk)
> +               fin =3D clk_get_rate(dsi->pll_clk);
> +       else
> +               fin =3D 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?

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.

Thanks,
Inki Dae

>         fout =3D 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_d=
sim *dsi)
>         u32 lane_polarities[5] =3D { 0 };
>         struct device_node *endpoint;
>         int i, nr_lanes, ret;
> -       struct clk *pll_clk;
>
>         ret =3D samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequen=
cy",
>                                        &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 =3D devm_clk_get(dev, "sclk_mipi");
> -               if (!IS_ERR(pll_clk))
> -                       dsi->pll_clk_rate =3D clk_get_rate(pll_clk);
> -               else
> -                       return PTR_ERR(pll_clk);
> +               dsi->pll_clk =3D 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/samsu=
ng-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
>