On Tue, Feb 04, 2025 at 03:57:57PM +0100, Maxime Ripard wrote:
> A bridge is considered atomic-enabled if it has an atomic_check
I think this part of the explanation is a bit misleading. This code just
checks if there is a need to atomic_check() it.
> implementation. However, atomic_check is optional and thus a driver
> might very well not provide an implementation, and yet still be atomic.
I think the change is correct, but I'd rather suggest implementing
separate drm_bridge_has_atomic_state() helper and using it here and in
two other cases.
>
> Switch to atomic_reset, which allocates the initial bridge state and is
> thus a better candidate.
>
> Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_bridge.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index d7c17a26e86fd9a3e5a39f335edff4ffce30dd79..ad91a0ac375a9c8cf82834354ec7f654a59a7292 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -794,23 +794,25 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
>
> static int drm_atomic_bridge_check(struct drm_bridge *bridge,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> {
> - if (bridge->funcs->atomic_check) {
> + if (bridge->funcs->atomic_reset) {
> struct drm_bridge_state *bridge_state;
> int ret;
>
> bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
> bridge);
> if (WARN_ON(!bridge_state))
> return -EINVAL;
>
> - ret = bridge->funcs->atomic_check(bridge, bridge_state,
> - crtc_state, conn_state);
> - if (ret)
> - return ret;
> + if (bridge->funcs->atomic_check) {
> + ret = bridge->funcs->atomic_check(bridge, bridge_state,
> + crtc_state, conn_state);
> + if (ret)
> + return ret;
> + }
> } else if (bridge->funcs->mode_fixup) {
> if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode,
> &crtc_state->adjusted_mode))
> return -EINVAL;
> }
>
> --
> 2.48.0
>
--
With best wishes
Dmitry
