Message-Id: <20230918-imx8mp-dtsi-v1-0-1d008b3237c0@skidata.com>
Date:   Mon, 18 Sep 2023 00:39:56 +0200
From:   Benjamin Bara <bbara93@...il.com>
To:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Abel Vesa <abelvesa@...nel.org>, Peng Fan <peng.fan@....com>
Cc:     Frank Oltmanns <frank@...manns.dev>,
        Maxime Ripard <mripard@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org,
        Benjamin Bara <benjamin.bara@...data.com>,
        Adam Ford <aford173@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>
Subject: [PATCH 00/13] imx8mp: first clock propagation attempt (for LVDS)

Hi!

Target of this series is to dynamically set the rate of video_pll1 to
the required LVDS clock rate(s), which are configured by the panel, and
the lvds-bridge respectively.

Some background:
The LVDS panel requires two clocks: the crtc clock and the lvds clock.
The lvds rate is always 7x the crtc rate. On the imx8mp, these are
assigned to media_disp2_pix and media_ldb, which are both
clk-composite-8m. The rates are set by drm_client_modeset_commit() (and
later by fsl_ldb_atomic_enable()), and the fsl-ldb driver, first crtc,
then lvds. The parent is typically assigned to video_pll1, which is a
clk-pll14xx (pll1443x).

The main problem:
As the clk-composite-8m currently doesn't support CLK_SET_RATE_PARENT,
the crtc rate is not propagated to video_pll1, and therefore must be
assigned in the device-tree manually.

The idea:
Enable CLK_SET_RATE_PARENT, at least for media_disp2_pix and media_ldb.
When this is done, ensure that the pll1443x can be re-configured,
meaning it ensures that an already configured rate (crtc rate) is still
supported when a second child requires a different rate (lvds rate). As
the children have divider, the current approach is straight forward by
calculating the LCM of the required rates. During the rate change of the
PLL, it must ensure that all children still have the configured rate at
the end (and maybe also bypass the clock while doing so?). This is done
by implementing a notifier function for the clk-composite-8m. The tricky
part is now to find out if the rate change was intentional or not. This
is done by adding the "change trigger" to the notify data. In our case,
we now can infer if we aren't the change trigger, we need to keep the
existing rate after the PLL's rate change. We keep the existing rate by
modifying the new_rate of the clock's core, as we are quite late in an
already ongoing clock change process.

Future work:
The re-configuration of the PLL can definitely be improved for other use
cases where the children have more fancy inter-dependencies. That's one
of the main reasons I currently only touched the mentioned clocks.
Additionally, it might make sense to automatically re-parent if a
different possible parent suits better.
For the core part, I thought about extending my "unintentional change
check" so that the core ensures that the children keep the configured
rate, which might not be easy as the parent could be allowed to "round",
but it's not clear (at least to me yet) how much rounding is allowed. I
found a similar discussion posted here[1], therefore added Frank and
Maxime.

Thanks & regards,
Benjamin

[1] https://lore.kernel.org/lkml/20230825-pll-mipi_keep_rate-v1-0-35bc43570730@oltmanns.dev/

---
Benjamin Bara (13):
      arm64: dts: imx8mp: lvds_bridge: use root instead of composite
      arm64: dts: imx8mp: re-parent IMX8MP_CLK_MEDIA_MIPI_PHY1_REF
      clk: implement clk_hw_set_rate()
      clk: print debug message if parent change is ignored
      clk: keep track of the trigger of an ongoing clk_set_rate
      clk: keep track if a clock is explicitly configured
      clk: detect unintended rate changes
      clk: divider: stop early if an optimal divider is found
      clk: imx: pll14xx: consider active rate for re-config
      clk: imx: composite-8m: convert compute_dividers to void
      clk: imx: composite-8m: implement CLK_SET_RATE_PARENT
      clk: imx: imx8mp: allow LVDS clocks to set parent rate
      arm64: dts: imx8mp: remove assigned-clock-rate of IMX8MP_VIDEO_PLL1

 arch/arm64/boot/dts/freescale/imx8mp.dtsi |  14 +--
 drivers/clk/clk-divider.c                 |   9 ++
 drivers/clk/clk.c                         | 146 +++++++++++++++++++++++++++++-
 drivers/clk/imx/clk-composite-8m.c        |  89 +++++++++++++++---
 drivers/clk/imx/clk-imx8mp.c              |   4 +-
 drivers/clk/imx/clk-pll14xx.c             |  20 ++++
 drivers/clk/imx/clk.h                     |   4 +
 include/linux/clk-provider.h              |   2 +
 include/linux/clk.h                       |   2 +
 9 files changed, 261 insertions(+), 29 deletions(-)
---
base-commit: e143016b56ecb0fcda5bb6026b0a25fe55274f56
change-id: 20230913-imx8mp-dtsi-7c6e25907e0e

Best regards,
-- 
Benjamin Bara <benjamin.bara@...data.com>

Powered by blists - more mailing lists