From: Thomas Zimmermann <tzimmermann@suse.de>
To: Aradhya Bhatia <aradhya.bhatia@linux.dev>,
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Andrzej Hajda <andrzej.hajda@intel.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Robert Foss <rfoss@kernel.org>, Jonas Karlman <jonas@kwiboo.se>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: Nishanth Menon <nm@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>,
Devarsh Thakkar <devarsht@ti.com>,
Praneeth Bajjuri <praneeth@ti.com>, Udit Kumar <u-kumar1@ti.com>,
Jayesh Choudhary <j-choudhary@ti.com>,
Alexander Sverdlin <alexander.sverdlin@siemens.com>,
DRI Development List <dri-devel@lists.freedesktop.org>,
Linux Kernel List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v12 4/5] drm/bridge: Update the bridge enable/disable doc
Date: Wed, 4 Jun 2025 10:56:18 +0200 [thread overview]
Message-ID: <b5c43a16-4f8a-480e-9c99-f984dc0697f7@suse.de> (raw)
In-Reply-To: <20250406131642.171240-5-aradhya.bhatia@linux.dev>
Am 06.04.25 um 15:16 schrieb Aradhya Bhatia:
> Now that the bridges get pre-enabled before the CRTC is enabled, and get
> post-disabled after the CRTC is disabled, update the function
> descriptions to accurately reflect the updated scenario.
>
> The enable sequence for the display pipeline looks like:
>
> bridge[n]_pre_enable
> ...
> bridge[1]_pre_enable
>
> crtc_enable
> encoder_enable
>
> bridge[1]_enable
> ...
> bridge[n]_enable
>
> And, the disable sequence for the display pipeline looks like:
>
> bridge[n]_disable
> ...
> bridge[1]_disable
>
> encoder_disable
> crtc_disable
>
> bridge[1]_post_disable
> ...
> bridge[n]_post_disable
>
> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
I think this patch could be merged with patch 3.
> ---
> include/drm/drm_bridge.h | 249 ++++++++++++++++++++++++++++-----------
> 1 file changed, 183 insertions(+), 66 deletions(-)
>
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index cdad3b78a195..e5ccbefa60a8 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -164,17 +164,33 @@ struct drm_bridge_funcs {
> /**
> * @disable:
> *
> - * 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 @disable callback should disable the bridge.
> *
> * 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:
> @@ -187,17 +203,34 @@ struct drm_bridge_funcs {
> /**
> * @post_disable:
> *
> - * 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.
> - *
> * 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.
> + * signals) feeding this bridge is no longer running when the
> + * @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.
> + *
> + * 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 @post_disable callback is optional.
> *
> @@ -240,18 +273,30 @@ struct drm_bridge_funcs {
> /**
> * @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 @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.
> - *
> * 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.
> + * 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:
> + *
> + * - &drm_encoder_helper_funcs.atomic_enable
> + * - &drm_encoder_helper_funcs.enable
> + * - &drm_encoder_helper_funcs.commit
> *
> * The @pre_enable callback is optional.
> *
> @@ -265,19 +310,31 @@ struct drm_bridge_funcs {
> /**
> * @enable:
> *
> - * 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 @enable callback should enable the bridge.
> *
> * 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:
> @@ -290,17 +347,30 @@ struct 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.
> - *
> * 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.
> + * 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
> + *
> + * 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 @atomic_pre_enable callback is optional.
> */
> @@ -310,18 +380,31 @@ struct drm_bridge_funcs {
> /**
> * @atomic_enable:
> *
> - * 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 @atomic_enable callback should enable the bridge.
> *
> * 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,
> @@ -329,16 +412,32 @@ struct drm_bridge_funcs {
> /**
> * @atomic_disable:
> *
> - * 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 @atomic_disable callback should disable the bridge.
> *
> * 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,
> @@ -347,16 +446,34 @@ struct drm_bridge_funcs {
> /**
> * @atomic_post_disable:
> *
> - * 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.
> - *
> * 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.
> + * 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.
> + *
> + * 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 @atomic_post_disable callback is optional.
> */
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
next prev parent reply other threads:[~2025-06-04 8:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-06 13:16 [PATCH v12 0/5] drm/atomic-helper: Re-order CRTC and Bridge ops Aradhya Bhatia
2025-04-06 13:16 ` [PATCH v12 1/5] drm/atomic-helper: Refactor crtc & encoder-bridge op loops into separate functions Aradhya Bhatia
2025-06-04 8:21 ` Thomas Zimmermann
2025-04-06 13:16 ` [PATCH v12 2/5] drm/atomic-helper: Separate out bridge pre_enable/post_disable from enable/disable Aradhya Bhatia
2025-06-04 8:38 ` Thomas Zimmermann
2025-04-06 13:16 ` [PATCH v12 3/5] drm/atomic-helper: Re-order bridge chain pre-enable and post-disable Aradhya Bhatia
2025-06-04 8:50 ` Thomas Zimmermann
2025-04-06 13:16 ` [PATCH v12 4/5] drm/bridge: Update the bridge enable/disable doc Aradhya Bhatia
2025-04-07 9:22 ` Tomi Valkeinen
2025-06-04 8:56 ` Thomas Zimmermann [this message]
2025-04-06 13:16 ` [PATCH v12 5/5] drm/bridge: cdns-dsi: Use pre_enable/post_disable to enable/disable Aradhya Bhatia
2025-05-16 12:23 ` [PATCH v12 0/5] drm/atomic-helper: Re-order CRTC and Bridge ops Tomi Valkeinen
2025-05-26 10:40 ` Tomi Valkeinen
2025-06-03 12:48 ` Tomi Valkeinen
2025-06-04 8:16 ` Thomas Zimmermann
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=b5c43a16-4f8a-480e-9c99-f984dc0697f7@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@gmail.com \
--cc=alexander.sverdlin@siemens.com \
--cc=andrzej.hajda@intel.com \
--cc=aradhya.bhatia@linux.dev \
--cc=devarsht@ti.com \
--cc=dmitry.baryshkov@oss.qualcomm.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=j-choudhary@ti.com \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=nm@ti.com \
--cc=praneeth@ti.com \
--cc=rfoss@kernel.org \
--cc=simona@ffwll.ch \
--cc=tomi.valkeinen@ideasonboard.com \
--cc=u-kumar1@ti.com \
--cc=vigneshr@ti.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®