Message-Id: <20241121-ge-ian-debug-imx8-clk-tree-v1-0-0f1b722588fe@bootlin.com>
Date: Thu, 21 Nov 2024 18:41:10 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Abel Vesa <abelvesa@...nel.org>, Peng Fan <peng.fan@....com>, 
 Michael Turquette <mturquette@...libre.com>, 
 Stephen Boyd <sboyd@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
 Sascha Hauer <s.hauer@...gutronix.de>, 
 Pengutronix Kernel Team <kernel@...gutronix.de>, 
 Fabio Estevam <festevam@...il.com>, Ying Liu <victor.liu@....com>, 
 Marek Vasut <marex@...x.de>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
 linux-clk@...r.kernel.org, imx@...ts.linux.dev, 
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
 dri-devel@...ts.freedesktop.org, Abel Vesa <abel.vesa@...aro.org>, 
 Herve Codina <herve.codina@...tlin.com>, 
 Luca Ceresoli <luca.ceresoli@...tlin.com>, 
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, Ian Ray <ian.ray@...com>, 
 Miquel Raynal <miquel.raynal@...tlin.com>, stable@...r.kernel.org
Subject: [PATCH 0/5] clk: Fix simple video pipelines on i.MX8

Recent changes in the clock tree have set CLK_SET_RATE_PARENT to the two
LCDIF pixel clocks. The idea is, instead of using assigned-clock
properties to set upstream PLL rates to high frequencies and hoping that
a single divisor (namely media_disp[12]_pix) will be close enough in
most cases, we should tell the clock core to use the PLL to properly
derive an accurate pixel clock rate in the first place. Here is the
situation.

[Before ff06ea04e4cf ("clk: imx: clk-imx8mp: Allow media_disp pixel clock reconfigure parent rate")]

Before setting CLK_SET_RATE_PARENT to the media_disp[12]_pix clocks, the sequence of events was:
- PLL is assigned to a high rate,
- media_disp[12]_pix is set to approximately freq A by using a single divisor,
- media_ldb is set to approximately freq 7*A by using another single divisor.
=> The display was working, but the pixel clock was inaccurate.

[After ff06ea04e4cf ("clk: imx: clk-imx8mp: Allow media_disp pixel clock reconfigure parent rate")]

After setting CLK_SET_RATE_PARENT to the media_disp[12]_pix clocks, the
sequence of events became:
- media_disp[12]_pix is set to freq A by using a divisor of 1 and
  setting video_pll1 to freq A.
- media_ldb is trying to compute its divisor to set freq 7*A, but the
  upstream PLL is to low, it does not recompute it, so it ends up
  setting a divisor of 1 and being at freq A instead of 7*A.
=> The display is sadly no longer working

[After applying PATCH "clk: imx: clk-imx8mp: Allow LDB serializer clock reconfigure parent rate"]

This is a commit from Marek, which is, I believe going in the right
direction, so I am including it. Just with this change, the situation is
slightly different, but the result is the same:
- media_disp[12]_pix is set to freq A by using a divisor of 1 and
  setting video_pll1 to freq A.
- media_ldb is set to 7*A by using a divisor of 1 and setting video_pll1
  to freq 7*A.
  /!\ This as the side effect of changing media_disp[12]_pix from freq A
  to freq 7*A.
=> The display is still not working

[After applying this series]

The goal of the following patches is to prevent clock subtree walks to
"just recalculate" the pixel clocks, ignoring the fact that they should
no longer change. They should adapt their divisors to the new upstream
rates instead. As a result, the display pipeline is working again.

Note: if more than one display is connected, we need the LDB driver to
act accordingly, thus the LDB driver must be adapted. Also, if accurate
pixel clocks are not possible with two different displays, we will still
need (at least for now) to make sure one of them is reparented to
another PLL, like the audio PLL (but audio PLL are of a different kind,
and are slightly less accurate).

So this series aims at fixing the i.MX8MP display pipeline for simple
setups. Said otherwise, returning to the same level of support as
before, but with (hopefully) more accurate frequencies. I believe this
approach manages to fix both Marek situation and all people using a
straightforward LCD based setup. For more complex setups, we need more
smartness from DRM and clk, but this is gonna take a bit of time.

---
Marek Vasut (1):
      clk: imx: clk-imx8mp: Allow LDB serializer clock reconfigure parent rate

Miquel Raynal (4):
      clk: Add a helper to determine a clock rate
      clk: Split clk_calc_subtree()
      clk: Add flag to prevent frequency changes when walking subtrees
      clk: imx: imx8mp: Prevent media clocks to be incompatibly changed

 drivers/clk/clk.c            | 39 ++++++++++++++++++++++++++++++++-------
 drivers/clk/imx/clk-imx8mp.c |  6 +++---
 include/linux/clk-provider.h |  2 ++
 3 files changed, 37 insertions(+), 10 deletions(-)
---
base-commit: 62facaf164585923d081eedcb6871f4ff3c2e953
change-id: 20241121-ge-ian-debug-imx8-clk-tree-bd325aa866f1

Best regards,
-- 
Miquel Raynal <miquel.raynal@...tlin.com>

Powered by blists - more mailing lists