Re: [PATCH v5 08/16] drm/bridge: Add helper to reset bridge pipeline

[Simona Vetter <simona.vetter@ffwll.ch>]

On Tue, Mar 04, 2025 at 12:10:51PM +0100, Maxime Ripard wrote:
> Let's provide an helper to make it easier for bridge drivers to
> power-cycle their bridge.
> 
> In order to avoid a circular dependency between that new helper and
> drm_atomic_helper_reset_crtc(), this new helper will be in a
> drm_bridge_helper.c file to follow the pattern we have for other
> objects.
> 
> Co-developed-by: Simona Vetter <simona.vetter@ffwll.ch>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/Makefile            |  1 +
>  drivers/gpu/drm/drm_bridge_helper.c | 55 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_bridge_helper.h     | 12 ++++++++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 4cd054188faf0b3dec442efd99ae52604541bce1..5a332f7d3ecccb6e5e1fb9fb811eab7aa5a84a21 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -131,10 +131,11 @@ obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
>  #
>  
>  drm_kms_helper-y := \
>  	drm_atomic_helper.o \
>  	drm_atomic_state_helper.o \
> +	drm_bridge_helper.o \
>  	drm_crtc_helper.o \
>  	drm_damage_helper.o \
>  	drm_flip_work.o \
>  	drm_format_helper.o \
>  	drm_gem_atomic_helper.o \
> diff --git a/drivers/gpu/drm/drm_bridge_helper.c b/drivers/gpu/drm/drm_bridge_helper.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..521df9d5dcb3fef842a95cdb2865a8b8d09b0fbe
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_bridge_helper.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_helper.h>
> +#include <drm/drm_modeset_lock.h>
> +
> +/**
> + * drm_bridge_helper_reset_crtc - Reset the pipeline feeding a bridge
> + * @bridge: DRM bridge to reset
> + * @ctx: lock acquisition context
> + *
> + * Reset a @bridge pipeline. It will power-cycle all active components
> + * between the CRTC and connector that bridge is connected to.

You need to minimally add that the same caveats and limitations of
drm_atomic_helper_reset_crtc() apply to this function here.

Might also be good to link from that kerneldoc to here so people can find
stuff easier, or maybe the overview section I've asked for once that's
landed.

With the kerneldoc polished:

Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Signed-off-by: Simona Vetter <simona.vetter@intel.com>

> + *
> + * Returns:
> + *
> + * 0 on success or a negative error code on failure. If the error
> + * returned is EDEADLK, the whole atomic sequence must be restarted.
> + */
> +int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge,
> +				 struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_connector *connector;
> +	struct drm_encoder *encoder = bridge->encoder;
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_crtc *crtc;
> +	int ret;
> +
> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> +	if (ret)
> +		return ret;
> +
> +	connector = drm_atomic_get_connector_for_encoder(encoder, ctx);
> +	if (IS_ERR(connector)) {
> +		ret = PTR_ERR(connector);
> +		goto out;
> +	}
> +
> +	if (!connector->state) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	crtc = connector->state->crtc;
> +	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
> +	if (ret)
> +		goto out;
> +
> +out:
> +	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_bridge_helper_reset_crtc);
> diff --git a/include/drm/drm_bridge_helper.h b/include/drm/drm_bridge_helper.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..6c35b479ec2a84aa43283351a6f18e49a9f9c177
> --- /dev/null
> +++ b/include/drm/drm_bridge_helper.h
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#ifndef __DRM_BRIDGE_HELPER_H_
> +#define __DRM_BRIDGE_HELPER_H_
> +
> +struct drm_bridge;
> +struct drm_modeset_acquire_ctx;
> +
> +int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge,
> +				 struct drm_modeset_acquire_ctx *ctx);
> +
> +#endif // __DRM_BRIDGE_HELPER_H_
> 
> -- 
> 2.48.1
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch