[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251205-drm-seq-fix-v1-1-fda68fa1b3de@ideasonboard.com>
Date: Fri, 05 Dec 2025 11:51:48 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Dmitry Baryshkov <lumag@...nel.org>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Jyri Sarha <jyri.sarha@....fi>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
Marek Szyprowski <m.szyprowski@...sung.com>,
Louis-Alexis Eyraud <louisalexis.eyraud@...labora.com>,
Aradhya Bhatia <aradhya.bhatia@...ux.dev>,
Linus Walleij <linusw@...nel.org>, Chaoyi Chen <chaoyi.chen@...k-chips.com>,
Vicente Bergas <vicencb@...il.com>,
Marek Vasut <marek.vasut+renesas@...lbox.org>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, stable@...r.kernel.org
Subject: [PATCH 1/4] Revert "drm/atomic-helper: Re-order bridge chain
pre-enable and post-disable"
This reverts commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1.
Changing the enable/disable sequence has caused regressions on multiple
platforms: R-Car, MCDE, Rockchip. A series (see link below) was sent to
fix these, but it was decided that it's better to revert the original
patch and change the enable/disable sequence only in the tidss driver.
Reverting this commit breaks tidss's DSI and OLDI outputs, which will be
fixed in the following commits.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Link: https://lore.kernel.org/all/20251202-mcde-drm-regression-thirdfix-v6-0-f1bffd4ec0fa%40kernel.org/
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Cc: stable@...r.kernel.org # v6.17+
---
drivers/gpu/drm/drm_atomic_helper.c | 8 +-
include/drm/drm_bridge.h | 249 ++++++++++--------------------------
2 files changed, 70 insertions(+), 187 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 10adac9397cf..ef97f37560b2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1341,9 +1341,9 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
{
encoder_bridge_disable(dev, state);
- crtc_disable(dev, state);
-
encoder_bridge_post_disable(dev, state);
+
+ crtc_disable(dev, state);
}
/**
@@ -1682,10 +1682,10 @@ encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
struct drm_atomic_state *state)
{
- encoder_bridge_pre_enable(dev, state);
-
crtc_enable(dev, state);
+ encoder_bridge_pre_enable(dev, state);
+
encoder_bridge_enable(dev, state);
drm_atomic_helper_commit_writebacks(dev, state);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0ff7ab4aa868..dbafe136833f 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -176,33 +176,17 @@ struct drm_bridge_funcs {
/**
* @disable:
*
- * The @disable callback should disable the bridge.
+ * This callback should disable the bridge. It is called right before
+ * the preceding element in the display pipe is disabled. If the
+ * preceding element is a bridge this means it's called before that
+ * bridge's @disable vfunc. If the preceding element is a &drm_encoder
+ * it's called right before the &drm_encoder_helper_funcs.disable,
+ * &drm_encoder_helper_funcs.prepare or &drm_encoder_helper_funcs.dpms
+ * hook.
*
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is still running when this callback is called.
*
- *
- * If the preceding element is a &drm_bridge, then this is called before
- * that bridge is disabled via one of:
- *
- * - &drm_bridge_funcs.disable
- * - &drm_bridge_funcs.atomic_disable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called before the encoder is disabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_disable
- * - &drm_encoder_helper_funcs.prepare
- * - &drm_encoder_helper_funcs.disable
- * - &drm_encoder_helper_funcs.dpms
- *
- * and the CRTC is disabled via one of:
- *
- * - &drm_crtc_helper_funcs.prepare
- * - &drm_crtc_helper_funcs.atomic_disable
- * - &drm_crtc_helper_funcs.disable
- * - &drm_crtc_helper_funcs.dpms.
- *
* The @disable callback is optional.
*
* NOTE:
@@ -215,34 +199,17 @@ struct drm_bridge_funcs {
/**
* @post_disable:
*
- * The bridge must assume that the display pipe (i.e. clocks and timing
- * signals) feeding this bridge is no longer running when the
- * @post_disable is called.
+ * This callback should disable the bridge. It is called right after the
+ * preceding element in the display pipe is disabled. If the preceding
+ * element is a bridge this means it's called after that bridge's
+ * @post_disable function. If the preceding element is a &drm_encoder
+ * it's called right after the encoder's
+ * &drm_encoder_helper_funcs.disable, &drm_encoder_helper_funcs.prepare
+ * or &drm_encoder_helper_funcs.dpms hook.
*
- * This callback should perform all the actions required by the hardware
- * after it has stopped receiving signals from the preceding element.
- *
- * If the preceding element is a &drm_bridge, then this is called after
- * that bridge is post-disabled (unless marked otherwise by the
- * @pre_enable_prev_first flag) via one of:
- *
- * - &drm_bridge_funcs.post_disable
- * - &drm_bridge_funcs.atomic_post_disable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called after the encoder is disabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_disable
- * - &drm_encoder_helper_funcs.prepare
- * - &drm_encoder_helper_funcs.disable
- * - &drm_encoder_helper_funcs.dpms
- *
- * and the CRTC is disabled via one of:
- *
- * - &drm_crtc_helper_funcs.prepare
- * - &drm_crtc_helper_funcs.atomic_disable
- * - &drm_crtc_helper_funcs.disable
- * - &drm_crtc_helper_funcs.dpms
+ * The bridge must assume that the display pipe (i.e. clocks and timing
+ * signals) feeding it is no longer running when this callback is
+ * called.
*
* The @post_disable callback is optional.
*
@@ -285,30 +252,18 @@ struct drm_bridge_funcs {
/**
* @pre_enable:
*
- * The display pipe (i.e. clocks and timing signals) feeding this bridge
- * will not yet be running when the @pre_enable is called.
- *
- * This callback should perform all the necessary actions to prepare the
- * bridge to accept signals from the preceding element.
- *
- * If the preceding element is a &drm_bridge, then this is called before
- * that bridge is pre-enabled (unless marked otherwise by
- * @pre_enable_prev_first flag) via one of:
- *
- * - &drm_bridge_funcs.pre_enable
- * - &drm_bridge_funcs.atomic_pre_enable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called before the CRTC is enabled via one of:
- *
- * - &drm_crtc_helper_funcs.atomic_enable
- * - &drm_crtc_helper_funcs.commit
- *
- * and the encoder is enabled via one of:
+ * This callback should enable the bridge. It is called right before
+ * the preceding element in the display pipe is enabled. If the
+ * preceding element is a bridge this means it's called before that
+ * bridge's @pre_enable function. If the preceding element is a
+ * &drm_encoder it's called right before the encoder's
+ * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or
+ * &drm_encoder_helper_funcs.dpms hook.
*
- * - &drm_encoder_helper_funcs.atomic_enable
- * - &drm_encoder_helper_funcs.enable
- * - &drm_encoder_helper_funcs.commit
+ * The display pipe (i.e. clocks and timing signals) feeding this bridge
+ * will not yet be running when this callback is called. The bridge must
+ * not enable the display link feeding the next bridge in the chain (if
+ * there is one) when this callback is called.
*
* The @pre_enable callback is optional.
*
@@ -322,31 +277,19 @@ struct drm_bridge_funcs {
/**
* @enable:
*
- * The @enable callback should enable the bridge.
+ * This callback should enable the bridge. It is called right after
+ * the preceding element in the display pipe is enabled. If the
+ * preceding element is a bridge this means it's called after that
+ * bridge's @enable function. If the preceding element is a
+ * &drm_encoder it's called right after the encoder's
+ * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or
+ * &drm_encoder_helper_funcs.dpms hook.
*
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
*
- * If the preceding element is a &drm_bridge, then this is called after
- * that bridge is enabled via one of:
- *
- * - &drm_bridge_funcs.enable
- * - &drm_bridge_funcs.atomic_enable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called after the CRTC is enabled via one of:
- *
- * - &drm_crtc_helper_funcs.atomic_enable
- * - &drm_crtc_helper_funcs.commit
- *
- * and the encoder is enabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_enable
- * - &drm_encoder_helper_funcs.enable
- * - drm_encoder_helper_funcs.commit
- *
* The @enable callback is optional.
*
* NOTE:
@@ -359,30 +302,17 @@ struct drm_bridge_funcs {
/**
* @atomic_pre_enable:
*
- * The display pipe (i.e. clocks and timing signals) feeding this bridge
- * will not yet be running when the @atomic_pre_enable is called.
- *
- * This callback should perform all the necessary actions to prepare the
- * bridge to accept signals from the preceding element.
- *
- * If the preceding element is a &drm_bridge, then this is called before
- * that bridge is pre-enabled (unless marked otherwise by
- * @pre_enable_prev_first flag) via one of:
- *
- * - &drm_bridge_funcs.pre_enable
- * - &drm_bridge_funcs.atomic_pre_enable
+ * This callback should enable the bridge. It is called right before
+ * the preceding element in the display pipe is enabled. If the
+ * preceding element is a bridge this means it's called before that
+ * bridge's @atomic_pre_enable or @pre_enable function. If the preceding
+ * element is a &drm_encoder it's called right before the encoder's
+ * &drm_encoder_helper_funcs.atomic_enable hook.
*
- * If the preceding element of the bridge is a display controller, then
- * this callback is called before the CRTC is enabled via one of:
- *
- * - &drm_crtc_helper_funcs.atomic_enable
- * - &drm_crtc_helper_funcs.commit
- *
- * and the encoder is enabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_enable
- * - &drm_encoder_helper_funcs.enable
- * - &drm_encoder_helper_funcs.commit
+ * The display pipe (i.e. clocks and timing signals) feeding this bridge
+ * will not yet be running when this callback is called. The bridge must
+ * not enable the display link feeding the next bridge in the chain (if
+ * there is one) when this callback is called.
*
* The @atomic_pre_enable callback is optional.
*/
@@ -392,31 +322,18 @@ struct drm_bridge_funcs {
/**
* @atomic_enable:
*
- * The @atomic_enable callback should enable the bridge.
+ * This callback should enable the bridge. It is called right after
+ * the preceding element in the display pipe is enabled. If the
+ * preceding element is a bridge this means it's called after that
+ * bridge's @atomic_enable or @enable function. If the preceding element
+ * is a &drm_encoder it's called right after the encoder's
+ * &drm_encoder_helper_funcs.atomic_enable hook.
*
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
*
- * If the preceding element is a &drm_bridge, then this is called after
- * that bridge is enabled via one of:
- *
- * - &drm_bridge_funcs.enable
- * - &drm_bridge_funcs.atomic_enable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called after the CRTC is enabled via one of:
- *
- * - &drm_crtc_helper_funcs.atomic_enable
- * - &drm_crtc_helper_funcs.commit
- *
- * and the encoder is enabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_enable
- * - &drm_encoder_helper_funcs.enable
- * - drm_encoder_helper_funcs.commit
- *
* The @atomic_enable callback is optional.
*/
void (*atomic_enable)(struct drm_bridge *bridge,
@@ -424,32 +341,16 @@ struct drm_bridge_funcs {
/**
* @atomic_disable:
*
- * The @atomic_disable callback should disable the bridge.
+ * This callback should disable the bridge. It is called right before
+ * the preceding element in the display pipe is disabled. If the
+ * preceding element is a bridge this means it's called before that
+ * bridge's @atomic_disable or @disable vfunc. If the preceding element
+ * is a &drm_encoder it's called right before the
+ * &drm_encoder_helper_funcs.atomic_disable hook.
*
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is still running when this callback is called.
*
- * If the preceding element is a &drm_bridge, then this is called before
- * that bridge is disabled via one of:
- *
- * - &drm_bridge_funcs.disable
- * - &drm_bridge_funcs.atomic_disable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called before the encoder is disabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_disable
- * - &drm_encoder_helper_funcs.prepare
- * - &drm_encoder_helper_funcs.disable
- * - &drm_encoder_helper_funcs.dpms
- *
- * and the CRTC is disabled via one of:
- *
- * - &drm_crtc_helper_funcs.prepare
- * - &drm_crtc_helper_funcs.atomic_disable
- * - &drm_crtc_helper_funcs.disable
- * - &drm_crtc_helper_funcs.dpms.
- *
* The @atomic_disable callback is optional.
*/
void (*atomic_disable)(struct drm_bridge *bridge,
@@ -458,34 +359,16 @@ struct drm_bridge_funcs {
/**
* @atomic_post_disable:
*
- * The bridge must assume that the display pipe (i.e. clocks and timing
- * signals) feeding this bridge is no longer running when the
- * @atomic_post_disable is called.
- *
- * This callback should perform all the actions required by the hardware
- * after it has stopped receiving signals from the preceding element.
+ * This callback should disable the bridge. It is called right after the
+ * preceding element in the display pipe is disabled. If the preceding
+ * element is a bridge this means it's called after that bridge's
+ * @atomic_post_disable or @post_disable function. If the preceding
+ * element is a &drm_encoder it's called right after the encoder's
+ * &drm_encoder_helper_funcs.atomic_disable hook.
*
- * If the preceding element is a &drm_bridge, then this is called after
- * that bridge is post-disabled (unless marked otherwise by the
- * @pre_enable_prev_first flag) via one of:
- *
- * - &drm_bridge_funcs.post_disable
- * - &drm_bridge_funcs.atomic_post_disable
- *
- * If the preceding element of the bridge is a display controller, then
- * this callback is called after the encoder is disabled via one of:
- *
- * - &drm_encoder_helper_funcs.atomic_disable
- * - &drm_encoder_helper_funcs.prepare
- * - &drm_encoder_helper_funcs.disable
- * - &drm_encoder_helper_funcs.dpms
- *
- * and the CRTC is disabled via one of:
- *
- * - &drm_crtc_helper_funcs.prepare
- * - &drm_crtc_helper_funcs.atomic_disable
- * - &drm_crtc_helper_funcs.disable
- * - &drm_crtc_helper_funcs.dpms
+ * The bridge must assume that the display pipe (i.e. clocks and timing
+ * signals) feeding it is no longer running when this callback is
+ * called.
*
* The @atomic_post_disable callback is optional.
*/
--
2.43.0
Powered by blists - more mailing lists