Am Donnerstag, dem 19.12.2024 um 11:54 +0100 schrieb Nikolaus Voss:
> 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@xxxxxxxxxxxxxxx> # 6.12.x, 6.6.x
> Signed-off-by: Nikolaus Voss <nv@xxxxxxx>
>
> ---
> 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,
without judging the approaches you list above, this works for me on
imx8mp as well,
Tested-by: Martin Kepplinger <martink@xxxxxxxxx>
thank you,
martin
