On Tue, Jun 02, 2026 at 09:24:29AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 30.05.26 um 15:59 schrieb Maxime Ripard:
> > Commit 47b5ac7daa46 ("drm/atomic: Add new atomic_create_state callback
> > to drm_private_obj") introduced a new pattern for allocating drm object
> > states: atomic_create_state, a dedicated hook that allocates and
> > initializes a pristine state without any side effect.
> >
> > The bridge atomic_reset callback is already fallible and in practice
> > only allocates and initializes state without touching hardware.
> > However, the reset name does not make this contract clear: callers
> > and implementers cannot tell from the name alone whether the hardware
> > will be affected or when the hook is safe to call.
> >
> > Add an atomic_create_state callback to drm_bridge_funcs to make the
> > contract explicit: allocate a pristine state, initialize it, no side
> > effects. The core calls it when available, falling back to
> > atomic_reset otherwise.
> >
> > Signed-off-by: Maxime Ripard <[email protected]>
> > ---
> > drivers/gpu/drm/drm_bridge.c | 8 ++++++--
> > include/drm/drm_bridge.h | 33 +++++++++++----------------------
> > 2 files changed, 17 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 687b36eea0c7..ef06c1aa509a 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -498,11 +498,14 @@ static struct drm_private_state *
> > drm_bridge_atomic_create_priv_state(struct drm_private_obj *obj)
> > {
> > struct drm_bridge *bridge = drm_priv_to_bridge(obj);
> > struct drm_bridge_state *state;
> > - state = bridge->funcs->atomic_reset(bridge);
> > + if (bridge->funcs->atomic_create_state)
> > + state = bridge->funcs->atomic_create_state(bridge);
> > + else
> > + state = bridge->funcs->atomic_reset(bridge);
> > if (IS_ERR(state))
> > return ERR_CAST(state);
> > return &state->base;
> > }
> > @@ -513,11 +516,12 @@ static const struct drm_private_state_funcs
> > drm_bridge_priv_state_funcs = {
> > .atomic_destroy_state = drm_bridge_atomic_destroy_priv_state,
> > };
> > static bool drm_bridge_is_atomic(struct drm_bridge *bridge)
> > {
> > - return bridge->funcs->atomic_reset != NULL;
> > + return (bridge->funcs->atomic_create_state ||
> > + bridge->funcs->atomic_reset);
> > }
> > /**
> > * drm_bridge_attach - attach the bridge to an encoder's chain
> > *
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 4ba3a5deef9a..a6c07d339afa 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -502,35 +502,24 @@ struct drm_bridge_funcs {
> > struct drm_bridge_state *bridge_state,
> > struct drm_crtc_state *crtc_state,
> > struct drm_connector_state *conn_state);
> > /**
> > - * @atomic_reset:
> > + * @atomic_create_state:
> > *
> > - * Reset the bridge to a predefined state (or retrieve its current
> > - * state) and return a &drm_bridge_state object matching this state.
> > - * This function is called at attach time.
> > - *
> > - * The atomic_reset hook is mandatory if the bridge implements any of
> > - * the atomic hooks, and should be left unassigned otherwise. For
> > - * bridges that don't subclass &drm_bridge_state, the
> > - * drm_atomic_helper_bridge_reset() helper function shall be used to
> > - * implement this hook.
> > - *
> > - * Note that the atomic_reset() semantics is not exactly matching the
> > - * reset() semantics found on other components (connector, plane, ...).
> > - *
> > - * 1. The reset operation happens when the bridge is attached, not when
> > - * drm_mode_config_reset() is called
> > - * 2. It's meant to be used exclusively on bridges that have been
> > - * converted to the ATOMIC API
> > + * Allocate a pristine, initialized, state for the bridge
> > + * object and return it. This callback must have no side
> > + * effects: in particular, the returned state must not be
> > + * assigned to the object's state pointer and it must not affect
> > + * the hardware state.
> > *
> > * RETURNS:
> > - * A valid drm_bridge_state object in case of success, an ERR_PTR()
> > - * giving the reason of the failure otherwise.
> > + *
> > + * A new, pristine, bridge state instance or an error pointer
> > + * on failure.
> > */
> > - struct drm_bridge_state *(*atomic_reset)(struct drm_bridge *bridge);
>
> This needs to remain for now, right?Yeah, Liu Ying pointed it out too, it's a screwup. I'll keep atomic_reset until all drivers are converted. Thanks for your review! Maxime
signature.asc
Description: PGP signature
