Unverified Commit c9b1150a authored by Aradhya Bhatia's avatar Aradhya Bhatia Committed by Maxime Ripard
Browse files

drm/atomic-helper: Re-order bridge chain pre-enable and post-disable 



Move the bridge pre_enable call before crtc enable, and the bridge
post_disable call after the crtc disable.

The sequence of enable after this patch will look 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 will look like:

	bridge[n]_disable
	...
	bridge[1]_disable

	encoder_disable
	crtc_disable

	bridge[1]_post_disable
	...
	bridge[n]_post_disable

The definition of bridge pre_enable hook says that,
"The display pipe (i.e. clocks and timing signals) feeding this bridge
will not yet be running when this callback is called".

Since CRTC is also a source feeding the bridge, it should not be enabled
before the bridges in the pipeline are pre_enabled. Fix that by
re-ordering the sequence of bridge pre_enable and bridge post_disable.

While at it, update the drm bridge API documentation as well.

Acked-by: default avatarDmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: default avatarTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Tested-by: default avatarTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tested-by: default avatarAlexander Sverdlin <alexander.sverdlin@siemens.com>
Signed-off-by: default avatarAradhya Bhatia <a-bhatia1@ti.com>
Signed-off-by: default avatarAradhya Bhatia <aradhya.bhatia@linux.dev>
Tested-by: default avatarDevarsh Thakkar <devarsht@ti.com>
Link: https://lore.kernel.org/r/20250605171524.27222-4-aradhya.bhatia@linux.dev


Signed-off-by: default avatarMaxime Ripard <mripard@kernel.org>
parent d5bef643

drivers/gpu/drm/drm_atomic_helper.c

+4 −4
Original line number Diff line number Diff line
@@ -1336,9 +1336,9 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *state) 
{

	encoder_bridge_disable(dev, state);



	encoder_bridge_post_disable(dev, state);



	crtc_disable(dev, state);



	encoder_bridge_post_disable(dev, state);

}



/**
@@ -1674,10 +1674,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)

{

	crtc_enable(dev, state);



	encoder_bridge_pre_enable(dev, state);



	crtc_enable(dev, state);



	encoder_bridge_enable(dev, state);



	drm_atomic_helper_commit_writebacks(dev, state);

include/drm/drm_bridge.h

+183 −66
Original line number Diff line number Diff line
@@ -165,17 +165,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:
@@ -188,17 +204,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.

	 *
@@ -241,18 +274,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.

	 *
@@ -266,19 +311,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:
@@ -291,17 +348,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.

	 */
@@ -311,18 +381,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,
@@ -330,16 +413,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,
@@ -348,16 +447,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.

	 */