From: Nikolaus Voss <nv@vosn.de>
To: Alexander Stein <alexander.stein@ew.tq-group.com>,
Liu Ying <victor.liu@nxp.com>,
Luca Ceresoli <luca.ceresoli@bootlin.com>,
Fabio Estevam <festevam@denx.de>, Marek Vasut <marex@denx.de>,
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>,
miquel.raynal@bootlin.com, nikolaus.voss@haag-streit.com
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4] drm: bridge: fsl-ldb: fixup mode on freq mismatch
Date: Thur, 19 Dec 2024 11:54:01 +0100 [thread overview]
Message-ID: <20241219105416.4AE0D1201@mail.steuer-voss.de> (raw)
LDB clock has to be a fixed multiple of the pixel clock.
Although LDB and pixel clock have a common source, this
constraint cannot be satisfied for any pixel clock at a
fixed source clock.
Violating this constraint leads to flickering and distorted
lines on the attached display.
To overcome this, there are these approches:
1. Modify the base PLL clock statically by changing the
device tree, implies calculating the PLL clock by
hand, e.g. commit 4fbb73416b10 ("arm64: dts:
imx8mp-phyboard-pollux: Set Video PLL1 frequency to 506.8 MHz")
2. Walk down the clock tree and modify the source clock.
Proposed patch series by Miquel Raynal:
[PATCH 0/5] clk: Fix simple video pipelines on i.MX8
3. This patch: check constraint violation in
drm_bridge_funcs.atomic_check() and adapt the pixel
clock in drm_display_mode.adjusted_mode accordingly.
Fixes: 463db5c2ed4a ("drm: bridge: ldb: Implement simple Freescale i.MX8MP LDB bridge")
Cc: <stable@vger.kernel.org> # 6.12.x, 6.6.x
Signed-off-by: Nikolaus Voss <nv@vosn.de>
---
v2:
- use .atomic_check() instead of .mode_fixup() (Dmitry Baryshkov)
- add Fixes tag (Liu Ying)
- use fsl_ldb_link_frequency() and drop const qualifier for
struct fsl_ldb* (Liu Ying)
v3:
- fix kernel test robot warning: fsl-ldb.c:125:30:
warning: omitting the parameter name in a function definition
is a C23 extension [-Wc23-extensions]
- fix/rephrase commit text due to discussion with Marek Vasut,
Liu Ying and Miquel Raynal
- only calculate and set pixel clock if ldb is not already
configured to the matching frequency
v4:
- handle mode changes correctly: recalculate pixel clock when
mode changes occur
- tested on 6.12.x and 6.6.x stable branches
drivers/gpu/drm/bridge/fsl-ldb.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 0e4bac7dd04ff..97ca522556718 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -121,6 +121,36 @@ static int fsl_ldb_attach(struct drm_bridge *bridge,
bridge, flags);
}
+static int fsl_ldb_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *connector_state)
+{
+ struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
+ const struct drm_display_mode *mode = &crtc_state->mode;
+ unsigned long requested_freq =
+ fsl_ldb_link_frequency(fsl_ldb, mode->clock);
+ unsigned long freq = clk_round_rate(fsl_ldb->clk, requested_freq);
+
+ if (crtc_state->mode_changed && (freq != requested_freq)) {
+ /*
+ * this will lead to flicker and incomplete lines on
+ * the attached display, adjust the CRTC clock
+ * accordingly.
+ */
+ struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
+ int pclk = freq / fsl_ldb_link_frequency(fsl_ldb, 1);
+
+ dev_warn(fsl_ldb->dev, "Adjusted pixel clk to match LDB clk (%d kHz -> %d kHz)!\n",
+ adjusted_mode->clock, pclk);
+
+ adjusted_mode->clock = pclk;
+ adjusted_mode->crtc_clock = pclk;
+ }
+
+ return 0;
+}
+
static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
struct drm_bridge_state *old_bridge_state)
{
@@ -280,6 +310,7 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge,
static const struct drm_bridge_funcs funcs = {
.attach = fsl_ldb_attach,
+ .atomic_check = fsl_ldb_atomic_check,
.atomic_enable = fsl_ldb_atomic_enable,
.atomic_disable = fsl_ldb_atomic_disable,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
--
2.43.0
next reply other threads:[~2024-12-19 11:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-19 10:54 Nikolaus Voss [this message]
2024-12-19 12:39 ` Martin Kepplinger
2024-12-23 19:01 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241219105416.4AE0D1201@mail.steuer-voss.de \
--to=nv@vosn.de \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=airlied@gmail.com \
--cc=alexander.stein@ew.tq-group.com \
--cc=andrzej.hajda@intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=festevam@denx.de \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=marex@denx.de \
--cc=miquel.raynal@bootlin.com \
--cc=neil.armstrong@linaro.org \
--cc=nikolaus.voss@haag-streit.com \
--cc=rfoss@kernel.org \
--cc=victor.liu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®