[PATCH RFC 06/19] drm/bridge: Create drm_bridge_chain_xx() wrappers

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Aug 21 14:45:04 UTC 2019


Hi Boris,

Thank you for the patch.

On Thu, Aug 08, 2019 at 05:11:37PM +0200, Boris Brezillon wrote:
> DRM bridges should not be operated independently. Let's rename the
> drm_bridge_xxx() helpers that take the first bridge of the chain and
> iterate over the whole chain into drm_bridge_chain_xx(). We also pass
> it the encoder containing the bridge chain instead of the dereferencing
> encoder->bridge.

I'm not sure about this. I do agree that the helpers operate on a chain,
but I think they're use for calling them on any bridge, especially in
conjunction with your work. The way I see it is that when a bridge in
the chain needs a custom enable/disable sequence (flagged by some kind
of flag in the bridge structure), the helpers will not automatically
propagate the calls down the chain, and let the bridge call the pre/post
enable/disable operations on the downstream bridge. This means that
those bridges with special needs will have to call the helpers on the
next bridge down the chain, and thus require keeping the ability to do
so.

We could of course propose two sets of helpers, one taking a bridge
pointer, and another one taking an encoder pointer, but I think it's a
bit overkill. Especially if you consider my comments earlier in this
series where I propose moving the custom sequence feature to bridges
instead of encoders, I don't think this patch will be needed.

> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c     |  19 +-
>  drivers/gpu/drm/drm_bridge.c            | 365 +++++++++++++-----------
>  drivers/gpu/drm/drm_crtc_helper.c       |  27 +-
>  drivers/gpu/drm/drm_probe_helper.c      |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c |   8 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c     |   4 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c           |   8 +-
>  include/drm/drm_bridge.h                |  64 +++--
>  8 files changed, 273 insertions(+), 224 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 15ea61877122..0deb54099570 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -445,8 +445,9 @@ mode_fixup(struct drm_atomic_state *state)
>  		encoder = new_conn_state->best_encoder;
>  		funcs = encoder->helper_private;
>  
> -		ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
> -				&new_crtc_state->adjusted_mode);
> +		ret = drm_bridge_chain_mode_fixup(encoder,
> +					&new_crtc_state->mode,
> +					&new_crtc_state->adjusted_mode);
>  		if (!ret) {
>  			DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
>  			return -EINVAL;
> @@ -511,7 +512,7 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
>  		return ret;
>  	}
>  
> -	ret = drm_bridge_mode_valid(encoder->bridge, mode);
> +	ret = drm_bridge_chain_mode_valid(encoder, mode);
>  	if (ret != MODE_OK) {
>  		DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
>  		return ret;
> @@ -1031,7 +1032,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  		 * it away), so we won't call disable hooks twice.
>  		 */
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_atomic_bridge_disable(encoder->bridge, old_state);
> +			drm_atomic_bridge_chain_disable(encoder, old_state);
>  
>  		/* Right function depends upon target state. */
>  		if (funcs) {
> @@ -1046,8 +1047,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  		}
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_atomic_bridge_post_disable(encoder->bridge,
> -						       old_state);
> +			drm_atomic_bridge_chain_post_disable(encoder,
> +							     old_state);
>  	}
>  
>  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
> @@ -1228,7 +1229,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			funcs->mode_set(encoder, mode, adjusted_mode);
>  		}
>  
> -		drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
> +		drm_bridge_chain_mode_set(encoder, mode, adjusted_mode);
>  	}
>  }
>  
> @@ -1346,7 +1347,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  		 * it away), so we won't call enable hooks twice.
>  		 */
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_atomic_bridge_pre_enable(encoder->bridge, old_state);
> +			drm_atomic_bridge_chain_pre_enable(encoder, old_state);
>  
>  		if (funcs) {
>  			if (funcs->atomic_enable)
> @@ -1358,7 +1359,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  		}
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_atomic_bridge_enable(encoder->bridge, old_state);
> +			drm_atomic_bridge_chain_enable(encoder, old_state);
>  	}
>  
>  	drm_atomic_helper_commit_writebacks(dev, old_state);
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index cba537c99e43..2bf9a19e11bf 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -171,24 +171,9 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>   * For detailed specification of the bridge callbacks see &drm_bridge_funcs.
>   */
>  
> -/**
> - * drm_bridge_mode_fixup - fixup proposed mode for all bridges in the
> - *			   encoder chain
> - * @bridge: bridge control structure
> - * @mode: desired mode to be set for the bridge
> - * @adjusted_mode: updated mode that works for this bridge
> - *
> - * Calls &drm_bridge_funcs.mode_fixup for all the bridges in the
> - * encoder chain, starting from the first bridge to the last.
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - *
> - * RETURNS:
> - * true on success, false on failure
> - */
> -bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> -			const struct drm_display_mode *mode,
> -			struct drm_display_mode *adjusted_mode)
> +static bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> +				  const struct drm_display_mode *mode,
> +				  struct drm_display_mode *adjusted_mode)
>  {
>  	bool ret = true;
>  
> @@ -202,25 +187,10 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(drm_bridge_mode_fixup);
>  
> -/**
> - * drm_bridge_mode_valid - validate the mode against all bridges in the
> - * 			   encoder chain.
> - * @bridge: bridge control structure
> - * @mode: desired mode to be validated
> - *
> - * Calls &drm_bridge_funcs.mode_valid for all the bridges in the encoder
> - * chain, starting from the first bridge to the last. If at least one bridge
> - * does not accept the mode the function returns the error code.
> - *
> - * Note: the bridge passed should be the one closest to the encoder.
> - *
> - * RETURNS:
> - * MODE_OK on success, drm_mode_status Enum error code on failure
> - */
> -enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> -					   const struct drm_display_mode *mode)
> +static enum drm_mode_status
> +drm_bridge_mode_valid(struct drm_bridge *bridge,
> +		      const struct drm_display_mode *mode)
>  {
>  	enum drm_mode_status ret = MODE_OK;
>  
> @@ -235,19 +205,8 @@ enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>  
>  	return drm_bridge_mode_valid(bridge->next, mode);
>  }
> -EXPORT_SYMBOL(drm_bridge_mode_valid);
>  
> -/**
> - * drm_bridge_disable - disables all bridges in the encoder chain
> - * @bridge: bridge control structure
> - *
> - * Calls &drm_bridge_funcs.disable op for all the bridges in the encoder
> - * chain, starting from the last bridge to the first. These are called before
> - * calling the encoder's prepare op.
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_bridge_disable(struct drm_bridge *bridge)
> +static void drm_bridge_disable(struct drm_bridge *bridge)
>  {
>  	if (!bridge)
>  		return;
> @@ -257,19 +216,8 @@ void drm_bridge_disable(struct drm_bridge *bridge)
>  	if (bridge->funcs->disable)
>  		bridge->funcs->disable(bridge);
>  }
> -EXPORT_SYMBOL(drm_bridge_disable);
>  
> -/**
> - * drm_bridge_post_disable - cleans up after disabling all bridges in the encoder chain
> - * @bridge: bridge control structure
> - *
> - * Calls &drm_bridge_funcs.post_disable op for all the bridges in the
> - * encoder chain, starting from the first bridge to the last. These are called
> - * after completing the encoder's prepare op.
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_bridge_post_disable(struct drm_bridge *bridge)
> +static void drm_bridge_post_disable(struct drm_bridge *bridge)
>  {
>  	if (!bridge)
>  		return;
> @@ -279,23 +227,10 @@ void drm_bridge_post_disable(struct drm_bridge *bridge)
>  
>  	drm_bridge_post_disable(bridge->next);
>  }
> -EXPORT_SYMBOL(drm_bridge_post_disable);
>  
> -/**
> - * drm_bridge_mode_set - set proposed mode for all bridges in the
> - *			 encoder chain
> - * @bridge: bridge control structure
> - * @mode: desired mode to be set for the bridge
> - * @adjusted_mode: updated mode that works for this bridge
> - *
> - * Calls &drm_bridge_funcs.mode_set op for all the bridges in the
> - * encoder chain, starting from the first bridge to the last.
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_bridge_mode_set(struct drm_bridge *bridge,
> -			 const struct drm_display_mode *mode,
> -			 const struct drm_display_mode *adjusted_mode)
> +static void drm_bridge_mode_set(struct drm_bridge *bridge,
> +				const struct drm_display_mode *mode,
> +				const struct drm_display_mode *adjusted_mode)
>  {
>  	if (!bridge)
>  		return;
> @@ -305,20 +240,8 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
>  
>  	drm_bridge_mode_set(bridge->next, mode, adjusted_mode);
>  }
> -EXPORT_SYMBOL(drm_bridge_mode_set);
>  
> -/**
> - * drm_bridge_pre_enable - prepares for enabling all
> - *			   bridges in the encoder chain
> - * @bridge: bridge control structure
> - *
> - * Calls &drm_bridge_funcs.pre_enable op for all the bridges in the encoder
> - * chain, starting from the last bridge to the first. These are called
> - * before calling the encoder's commit op.
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_bridge_pre_enable(struct drm_bridge *bridge)
> +static void drm_bridge_pre_enable(struct drm_bridge *bridge)
>  {
>  	if (!bridge)
>  		return;
> @@ -328,19 +251,8 @@ void drm_bridge_pre_enable(struct drm_bridge *bridge)
>  	if (bridge->funcs->pre_enable)
>  		bridge->funcs->pre_enable(bridge);
>  }
> -EXPORT_SYMBOL(drm_bridge_pre_enable);
>  
> -/**
> - * drm_bridge_enable - enables all bridges in the encoder chain
> - * @bridge: bridge control structure
> - *
> - * Calls &drm_bridge_funcs.enable op for all the bridges in the encoder
> - * chain, starting from the first bridge to the last. These are called
> - * after completing the encoder's commit op.
> - *
> - * Note that the bridge passed should be the one closest to the encoder
> - */
> -void drm_bridge_enable(struct drm_bridge *bridge)
> +static void drm_bridge_enable(struct drm_bridge *bridge)
>  {
>  	if (!bridge)
>  		return;
> @@ -350,22 +262,127 @@ void drm_bridge_enable(struct drm_bridge *bridge)
>  
>  	drm_bridge_enable(bridge->next);
>  }
> -EXPORT_SYMBOL(drm_bridge_enable);
>  
>  /**
> - * drm_atomic_bridge_disable - disables all bridges in the encoder chain
> - * @bridge: bridge control structure
> - * @state: atomic state being committed
> + * drm_bridge_chain_mode_fixup - fixup proposed mode for all bridges in the
> + *				 encoder chain
> + * @encoder: encoder object
> + * @mode: desired mode to be set for the bridge
> + * @adjusted_mode: updated mode that works for this bridge
>   *
> - * Calls &drm_bridge_funcs.atomic_disable (falls back on
> - * &drm_bridge_funcs.disable) op for all the bridges in the encoder chain,
> - * starting from the last bridge to the first. These are called before calling
> - * &drm_encoder_helper_funcs.atomic_disable
> + * Calls &drm_bridge_funcs.mode_fixup for all the bridges in the
> + * encoder chain, starting from the first bridge to the last.
>   *
> - * Note: the bridge passed should be the one closest to the encoder
> + * RETURNS:
> + * true on success, false on failure
>   */
> -void drm_atomic_bridge_disable(struct drm_bridge *bridge,
> -			       struct drm_atomic_state *state)
> +bool drm_bridge_chain_mode_fixup(struct drm_encoder *encoder,
> +				 const struct drm_display_mode *mode,
> +				 struct drm_display_mode *adjusted_mode)
> +{
> +	return drm_bridge_mode_fixup(encoder->bridge, mode, adjusted_mode);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_mode_fixup);
> +
> +/**
> + * drm_bridge_chain_mode_valid - validate the mode against all bridges in the
> + *				 encoder chain.
> + * @encoder: encoder object
> + * @mode: desired mode to be validated
> + *
> + * Calls &drm_bridge_funcs.mode_valid for all the bridges in the encoder
> + * chain, starting from the first bridge to the last. If at least one bridge
> + * does not accept the mode the function returns the error code.
> + *
> + * RETURNS:
> + * MODE_OK on success, drm_mode_status Enum error code on failure
> + */
> +enum drm_mode_status
> +drm_bridge_chain_mode_valid(struct drm_encoder *encoder,
> +			    const struct drm_display_mode *mode)
> +{
> +	return drm_bridge_mode_valid(encoder->bridge, mode);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
> +
> +/**
> + * drm_bridge_chain_disable - disables all bridges in the encoder chain
> + * @encoder: encoder object
> + *
> + * Calls &drm_bridge_funcs.disable op for all the bridges in the encoder
> + * chain, starting from the last bridge to the first. These are called before
> + * calling the encoder's prepare op.
> + */
> +void drm_bridge_chain_disable(struct drm_encoder *encoder)
> +{
> +	drm_bridge_disable(encoder->bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_disable);
> +
> +/**
> + * drm_bridge_chain_post_disable - cleans up after disabling all bridges in the
> + *				   encoder chain
> + * @encoder: encoder object
> + *
> + * Calls &drm_bridge_funcs.post_disable op for all the bridges in the
> + * encoder chain, starting from the first bridge to the last. These are called
> + * after completing the encoder's prepare op.
> + */
> +void drm_bridge_chain_post_disable(struct drm_encoder *encoder)
> +{
> +	drm_bridge_post_disable(encoder->bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_post_disable);
> +
> +/**
> + * drm_bridge_chain_mode_set - set proposed mode for all bridges in the
> + *			       encoder chain
> + * @encoder: encoder object
> + * @mode: desired mode to be set for the encoder chain
> + * @adjusted_mode: updated mode that works for this encoder chain
> + *
> + * Calls &drm_bridge_funcs.mode_set op for all the bridges in the
> + * encoder chain, starting from the first bridge to the last.
> + */
> +void drm_bridge_chain_mode_set(struct drm_encoder *encoder,
> +			       const struct drm_display_mode *mode,
> +			       const struct drm_display_mode *adjusted_mode)
> +{
> +	drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_mode_set);
> +
> +/**
> + * drm_bridge_chain_pre_enable - prepares for enabling all bridges in the
> + *				 encoder chain
> + * @encoder: encoder object
> + *
> + * Calls &drm_bridge_funcs.pre_enable op for all the bridges in the encoder
> + * chain, starting from the last bridge to the first. These are called
> + * before calling the encoder's commit op.
> + */
> +void drm_bridge_chain_pre_enable(struct drm_encoder *encoder)
> +{
> +	drm_bridge_pre_enable(encoder->bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_pre_enable);
> +
> +/**
> + * drm_bridge_chain_enable - enables all bridges in the encoder chain
> + * @encoder: encoder object
> + *
> + * Calls &drm_bridge_funcs.enable op for all the bridges in the encoder
> + * chain, starting from the first bridge to the last. These are called
> + * after completing the encoder's commit op.
> + */
> +void drm_bridge_chain_enable(struct drm_encoder *encoder)
> +{
> +	drm_bridge_enable(encoder->bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_chain_enable);
> +
> +static void drm_atomic_bridge_disable(struct drm_bridge *bridge,
> +				      struct drm_atomic_state *state)
>  {
>  	if (!bridge)
>  		return;
> @@ -377,23 +394,9 @@ void drm_atomic_bridge_disable(struct drm_bridge *bridge,
>  	else if (bridge->funcs->disable)
>  		bridge->funcs->disable(bridge);
>  }
> -EXPORT_SYMBOL(drm_atomic_bridge_disable);
>  
> -/**
> - * drm_atomic_bridge_post_disable - cleans up after disabling all bridges in the
> - *				    encoder chain
> - * @bridge: bridge control structure
> - * @state: atomic state being committed
> - *
> - * Calls &drm_bridge_funcs.atomic_post_disable (falls back on
> - * &drm_bridge_funcs.post_disable) op for all the bridges in the encoder chain,
> - * starting from the first bridge to the last. These are called after completing
> - * &drm_encoder_helper_funcs.atomic_disable
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_atomic_bridge_post_disable(struct drm_bridge *bridge,
> -				    struct drm_atomic_state *state)
> +static void drm_atomic_bridge_post_disable(struct drm_bridge *bridge,
> +					   struct drm_atomic_state *state)
>  {
>  	if (!bridge)
>  		return;
> @@ -405,23 +408,9 @@ void drm_atomic_bridge_post_disable(struct drm_bridge *bridge,
>  
>  	drm_atomic_bridge_post_disable(bridge->next, state);
>  }
> -EXPORT_SYMBOL(drm_atomic_bridge_post_disable);
>  
> -/**
> - * drm_atomic_bridge_pre_enable - prepares for enabling all bridges in the
> - *				  encoder chain
> - * @bridge: bridge control structure
> - * @state: atomic state being committed
> - *
> - * Calls &drm_bridge_funcs.atomic_pre_enable (falls back on
> - * &drm_bridge_funcs.pre_enable) op for all the bridges in the encoder chain,
> - * starting from the last bridge to the first. These are called before calling
> - * &drm_encoder_helper_funcs.atomic_enable
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge,
> -				  struct drm_atomic_state *state)
> +static void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge,
> +					 struct drm_atomic_state *state)
>  {
>  	if (!bridge)
>  		return;
> @@ -433,22 +422,9 @@ void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge,
>  	else if (bridge->funcs->pre_enable)
>  		bridge->funcs->pre_enable(bridge);
>  }
> -EXPORT_SYMBOL(drm_atomic_bridge_pre_enable);
>  
> -/**
> - * drm_atomic_bridge_enable - enables all bridges in the encoder chain
> - * @bridge: bridge control structure
> - * @state: atomic state being committed
> - *
> - * Calls &drm_bridge_funcs.atomic_enable (falls back on
> - * &drm_bridge_funcs.enable) op for all the bridges in the encoder chain,
> - * starting from the first bridge to the last. These are called after completing
> - * &drm_encoder_helper_funcs.atomic_enable
> - *
> - * Note: the bridge passed should be the one closest to the encoder
> - */
> -void drm_atomic_bridge_enable(struct drm_bridge *bridge,
> -			      struct drm_atomic_state *state)
> +static void drm_atomic_bridge_enable(struct drm_bridge *bridge,
> +				     struct drm_atomic_state *state)
>  {
>  	if (!bridge)
>  		return;
> @@ -460,7 +436,76 @@ void drm_atomic_bridge_enable(struct drm_bridge *bridge,
>  
>  	drm_atomic_bridge_enable(bridge->next, state);
>  }
> -EXPORT_SYMBOL(drm_atomic_bridge_enable);
> +
> +/**
> + * drm_atomic_bridge_chain_disable - disables all bridges in the encoder chain
> + * @encoder: encoder object
> + * @state: atomic state being committed
> + *
> + * Calls &drm_bridge_funcs.atomic_disable (falls back on
> + * &drm_bridge_funcs.disable) op for all the bridges in the encoder chain,
> + * starting from the last bridge to the first. These are called before calling
> + * &drm_encoder_helper_funcs.atomic_disable
> + */
> +void drm_atomic_bridge_chain_disable(struct drm_encoder *encoder,
> +				     struct drm_atomic_state *state)
> +{
> +	drm_atomic_bridge_disable(encoder->bridge, state);
> +}
> +EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
> +
> +/**
> + * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges
> + *					  in the encoder chain
> + * @encoder: encoder object
> + * @state: atomic state being committed
> + *
> + * Calls &drm_bridge_funcs.atomic_post_disable (falls back on
> + * &drm_bridge_funcs.post_disable) op for all the bridges in the encoder chain,
> + * starting from the first bridge to the last. These are called after completing
> + * &drm_encoder_helper_funcs.atomic_disable
> + */
> +void drm_atomic_bridge_chain_post_disable(struct drm_encoder *encoder,
> +					  struct drm_atomic_state *state)
> +{
> +	drm_atomic_bridge_post_disable(encoder->bridge, state);
> +}
> +EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
> +
> +/**
> + * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in
> + *					the encoder chain
> + * @encoder: encoder object
> + * @state: atomic state being committed
> + *
> + * Calls &drm_bridge_funcs.atomic_pre_enable (falls back on
> + * &drm_bridge_funcs.pre_enable) op for all the bridges in the encoder chain,
> + * starting from the last bridge to the first. These are called before calling
> + * &drm_encoder_helper_funcs.atomic_enable
> + */
> +void drm_atomic_bridge_chain_pre_enable(struct drm_encoder *encoder,
> +					struct drm_atomic_state *state)
> +{
> +	drm_atomic_bridge_pre_enable(encoder->bridge, state);
> +}
> +EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
> +
> +/**
> + * drm_atomic_bridge_chain_enable - enables all bridges in the encoder chain
> + * @encoder: encoder object
> + * @state: atomic state being committed
> + *
> + * Calls &drm_bridge_funcs.atomic_enable (falls back on
> + * &drm_bridge_funcs.enable) op for all the bridges in the encoder chain,
> + * starting from the first bridge to the last. These are called after completing
> + * &drm_encoder_helper_funcs.atomic_enable
> + */
> +void drm_atomic_bridge_chain_enable(struct drm_encoder *encoder,
> +				    struct drm_atomic_state *state)
> +{
> +	drm_atomic_bridge_enable(encoder->bridge, state);
> +}
> +EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
>  
>  #ifdef CONFIG_OF
>  /**
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 87dae37fec12..f8a361d396df 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -161,7 +161,7 @@ drm_encoder_disable(struct drm_encoder *encoder)
>  		return;
>  
>  	if (!encoder->custom_bridge_enable_disable_seq)
> -		drm_bridge_disable(encoder->bridge);
> +		drm_bridge_chain_disable(encoder);
>  
>  	if (encoder_funcs->disable)
>  		(*encoder_funcs->disable)(encoder);
> @@ -169,7 +169,7 @@ drm_encoder_disable(struct drm_encoder *encoder)
>  		(*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
>  
>  	if (!encoder->custom_bridge_enable_disable_seq)
> -		drm_bridge_post_disable(encoder->bridge);
> +		drm_bridge_chain_post_disable(encoder);
>  }
>  
>  static void __drm_helper_disable_unused_functions(struct drm_device *dev)
> @@ -329,8 +329,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (!encoder_funcs)
>  			continue;
>  
> -		ret = drm_bridge_mode_fixup(encoder->bridge,
> -			mode, adjusted_mode);
> +		ret = drm_bridge_chain_mode_fixup(encoder, mode,
> +						  adjusted_mode);
>  		if (!ret) {
>  			DRM_DEBUG_KMS("Bridge fixup failed\n");
>  			goto done;
> @@ -368,14 +368,14 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  			continue;
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_bridge_disable(encoder->bridge);
> +			drm_bridge_chain_disable(encoder);
>  
>  		/* Disable the encoders as the first thing we do. */
>  		if (encoder_funcs->prepare)
>  			encoder_funcs->prepare(encoder);
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_bridge_post_disable(encoder->bridge);
> +			drm_bridge_chain_post_disable(encoder);
>  	}
>  
>  	drm_crtc_prepare_encoders(dev);
> @@ -403,7 +403,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  		if (encoder_funcs->mode_set)
>  			encoder_funcs->mode_set(encoder, mode, adjusted_mode);
>  
> -		drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
> +		drm_bridge_chain_mode_set(encoder, mode, adjusted_mode);
>  	}
>  
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> @@ -419,13 +419,13 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  			continue;
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_bridge_pre_enable(encoder->bridge);
> +			drm_bridge_chain_pre_enable(encoder);
>  
>  		if (encoder_funcs->commit)
>  			encoder_funcs->commit(encoder);
>  
>  		if (!encoder->custom_bridge_enable_disable_seq)
> -			drm_bridge_enable(encoder->bridge);
> +			drm_bridge_chain_enable(encoder);
>  	}
>  
>  	/* Calculate and store various constants which
> @@ -824,7 +824,6 @@ static int drm_helper_choose_encoder_dpms(struct drm_encoder *encoder)
>  /* Helper which handles bridge ordering around encoder dpms */
>  static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode)
>  {
> -	struct drm_bridge *bridge = encoder->bridge;
>  	const struct drm_encoder_helper_funcs *encoder_funcs;
>  
>  	encoder_funcs = encoder->helper_private;
> @@ -833,9 +832,9 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode)
>  
>  	if (!encoder->custom_bridge_enable_disable_seq) {
>  		if (mode == DRM_MODE_DPMS_ON)
> -			drm_bridge_pre_enable(bridge);
> +			drm_bridge_chain_pre_enable(encoder);
>  		else
> -			drm_bridge_disable(bridge);
> +			drm_bridge_chain_disable(encoder);
>  	}
>  
>  	if (encoder_funcs->dpms)
> @@ -843,9 +842,9 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode)
>  
>  	if (!encoder->custom_bridge_enable_disable_seq) {
>  		if (mode == DRM_MODE_DPMS_ON)
> -			drm_bridge_enable(bridge);
> +			drm_bridge_chain_enable(encoder);
>  		else
> -			drm_bridge_post_disable(bridge);
> +			drm_bridge_chain_post_disable(encoder);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 351cbc40f0f8..461199bc28c0 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -113,7 +113,7 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
>  			continue;
>  		}
>  
> -		ret = drm_bridge_mode_valid(encoder->bridge, mode);
> +		ret = drm_bridge_chain_mode_valid(encoder, mode);
>  		if (ret != MODE_OK) {
>  			/* There is also no point in continuing for crtc check
>  			 * here. */
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 8593c87bdf96..9a96bc48b81f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1389,7 +1389,7 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
>  		if (ret < 0)
>  			goto err_put_sync;
>  	} else {
> -		drm_bridge_pre_enable(encoder->bridge);
> +		drm_bridge_chain_pre_enable(encoder);
>  	}
>  
>  	exynos_dsi_set_display_mode(dsi);
> @@ -1400,7 +1400,7 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
>  		if (ret < 0)
>  			goto err_display_disable;
>  	} else {
> -		drm_bridge_enable(encoder->bridge);
> +		drm_bridge_chain_enable(encoder);
>  	}
>  
>  	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> @@ -1425,10 +1425,10 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
>  	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>  
>  	drm_panel_disable(dsi->panel);
> -	drm_bridge_disable(encoder->bridge);
> +	drm_bridge_chain_disable(encoder);
>  	exynos_dsi_set_display_enable(dsi, false);
>  	drm_panel_unprepare(dsi->panel);
> -	drm_bridge_post_disable(encoder->bridge);
> +	drm_bridge_chain_post_disable(encoder);
>  	dsi->state &= ~DSIM_STATE_ENABLED;
>  	pm_runtime_put_sync(dsi->dev);
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index c79b1f855d89..6ec3d2584539 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1247,8 +1247,8 @@ static int mtk_hdmi_conn_mode_valid(struct drm_connector *conn,
>  		struct drm_display_mode adjusted_mode;
>  
>  		drm_mode_copy(&adjusted_mode, mode);
> -		if (!drm_bridge_mode_fixup(hdmi->bridge.next, mode,
> -					   &adjusted_mode))
> +		if (!drm_bridge_chain_mode_fixup(hdmi->bridge.encoder, mode,
> +						 &adjusted_mode))
>  			return MODE_BAD;
>  	}
>  
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index b670ca473786..d7cd720c4efa 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -752,9 +752,9 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
>  	struct vc4_dsi *dsi = vc4_encoder->dsi;
>  	struct device *dev = &dsi->pdev->dev;
>  
> -	drm_bridge_disable(encoder->bridge);
> +	drm_bridge_chain_disable(encoder);
>  	vc4_dsi_ulps(dsi, true);
> -	drm_bridge_post_disable(encoder->bridge);
> +	drm_bridge_chain_post_disable(encoder);
>  
>  	clk_disable_unprepare(dsi->pll_phy_clock);
>  	clk_disable_unprepare(dsi->escape_clock);
> @@ -1054,7 +1054,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  
>  	vc4_dsi_ulps(dsi, false);
>  
> -	drm_bridge_pre_enable(encoder->bridge);
> +	drm_bridge_chain_pre_enable(encoder);
>  
>  	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
>  		DSI_PORT_WRITE(DISP0_CTRL,
> @@ -1071,7 +1071,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  			       DSI_DISP0_ENABLE);
>  	}
>  
> -	drm_bridge_enable(encoder->bridge);
> +	drm_bridge_chain_enable(encoder);
>  
>  	if (debug_dump_regs) {
>  		struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 7616f6562fe4..3057e2153f62 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -254,9 +254,10 @@ struct drm_bridge_funcs {
>  	 * there is one) when this callback is called.
>  	 *
>  	 * Note that this function will only be invoked in the context of an
> -	 * atomic commit. It will not be invoked from &drm_bridge_pre_enable. It
> -	 * would be prudent to also provide an implementation of @pre_enable if
> -	 * you are expecting driver calls into &drm_bridge_pre_enable.
> +	 * atomic commit. It will not be invoked from
> +	 * &drm_bridge_chain_pre_enable. It would be prudent to also provide an
> +	 * implementation of @pre_enable if you are expecting driver calls into
> +	 * &drm_bridge_chain_pre_enable.
>  	 *
>  	 * The @atomic_pre_enable callback is optional.
>  	 */
> @@ -279,9 +280,9 @@ struct drm_bridge_funcs {
>  	 * chain if there is one.
>  	 *
>  	 * Note that this function will only be invoked in the context of an
> -	 * atomic commit. It will not be invoked from &drm_bridge_enable. It
> -	 * would be prudent to also provide an implementation of @enable if
> -	 * you are expecting driver calls into &drm_bridge_enable.
> +	 * atomic commit. It will not be invoked from &drm_bridge_chain_enable.
> +	 * It would be prudent to also provide an implementation of @enable if
> +	 * you are expecting driver calls into &drm_bridge_chain_enable.
>  	 *
>  	 * The enable callback is optional.
>  	 */
> @@ -301,9 +302,10 @@ struct drm_bridge_funcs {
>  	 * signals) feeding it is still running when this callback is called.
>  	 *
>  	 * Note that this function will only be invoked in the context of an
> -	 * atomic commit. It will not be invoked from &drm_bridge_disable. It
> -	 * would be prudent to also provide an implementation of @disable if
> -	 * you are expecting driver calls into &drm_bridge_disable.
> +	 * atomic commit. It will not be invoked from
> +	 * &drm_bridge_chain_disable. It would be prudent to also provide an
> +	 * implementation of @disable if you are expecting driver calls into
> +	 * &drm_bridge_chain_disable.
>  	 *
>  	 * The disable callback is optional.
>  	 */
> @@ -325,10 +327,11 @@ struct drm_bridge_funcs {
>  	 * called.
>  	 *
>  	 * Note that this function will only be invoked in the context of an
> -	 * atomic commit. It will not be invoked from &drm_bridge_post_disable.
> +	 * atomic commit. It will not be invoked from
> +	 * &drm_bridge_chain_post_disable.
>  	 * It would be prudent to also provide an implementation of
>  	 * @post_disable if you are expecting driver calls into
> -	 * &drm_bridge_post_disable.
> +	 * &drm_bridge_chain_post_disable.
>  	 *
>  	 * The post_disable callback is optional.
>  	 */
> @@ -406,27 +409,28 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np);
>  int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  		      struct drm_bridge *previous);
>  
> -bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> -			   const struct drm_display_mode *mode,
> -			   struct drm_display_mode *adjusted_mode);
> -enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> -					   const struct drm_display_mode *mode);
> -void drm_bridge_disable(struct drm_bridge *bridge);
> -void drm_bridge_post_disable(struct drm_bridge *bridge);
> -void drm_bridge_mode_set(struct drm_bridge *bridge,
> -			 const struct drm_display_mode *mode,
> -			 const struct drm_display_mode *adjusted_mode);
> -void drm_bridge_pre_enable(struct drm_bridge *bridge);
> -void drm_bridge_enable(struct drm_bridge *bridge);
> +bool drm_bridge_chain_mode_fixup(struct drm_encoder *encoder,
> +				 const struct drm_display_mode *mode,
> +				 struct drm_display_mode *adjusted_mode);
> +enum drm_mode_status
> +drm_bridge_chain_mode_valid(struct drm_encoder *encoder,
> +			    const struct drm_display_mode *mode);
> +void drm_bridge_chain_disable(struct drm_encoder *encoder);
> +void drm_bridge_chain_post_disable(struct drm_encoder *encoder);
> +void drm_bridge_chain_mode_set(struct drm_encoder *encoder,
> +			       const struct drm_display_mode *mode,
> +			       const struct drm_display_mode *adjusted_mode);
> +void drm_bridge_chain_pre_enable(struct drm_encoder *encoder);
> +void drm_bridge_chain_enable(struct drm_encoder *encoder);
>  
> -void drm_atomic_bridge_disable(struct drm_bridge *bridge,
> -			       struct drm_atomic_state *state);
> -void drm_atomic_bridge_post_disable(struct drm_bridge *bridge,
> +void drm_atomic_bridge_chain_disable(struct drm_encoder *encoder,
> +				     struct drm_atomic_state *state);
> +void drm_atomic_bridge_chain_post_disable(struct drm_encoder *encoder,
> +					  struct drm_atomic_state *state);
> +void drm_atomic_bridge_chain_pre_enable(struct drm_encoder *encoder,
> +					struct drm_atomic_state *state);
> +void drm_atomic_bridge_chain_enable(struct drm_encoder *encoder,
>  				    struct drm_atomic_state *state);
> -void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge,
> -				  struct drm_atomic_state *state);
> -void drm_atomic_bridge_enable(struct drm_bridge *bridge,
> -			      struct drm_atomic_state *state);
>  
>  #ifdef CONFIG_DRM_PANEL_BRIDGE
>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> -- 
> 2.21.0
> 

-- 
Regards,

Laurent Pinchart


More information about the dri-devel mailing list