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.

Reviewed-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Luca Ceresoli <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  5 +++--
 drivers/gpu/drm/drm_bridge.c              |  8 ++++++--
 include/drm/drm_bridge.h                  | 19 ++++++++++++++++++-
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index f0688f2d83fe..9dfb9b6ba392 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -892,12 +892,13 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
  * @bridge: the bridge this state refers to
  *
  * @state is assumed to be zeroed.
  *
  * Initializes the bridge state to default values. This is meant to be called
- * by the bridge &drm_bridge_funcs.atomic_reset hook for bridges that subclass
- * the bridge state.
+ * by the bridge &drm_bridge_funcs.atomic_create_state or
+ * &drm_bridge_funcs.atomic_reset hook for bridges that subclass the bridge
+ * state.
  */
 void __drm_atomic_helper_bridge_state_init(struct drm_bridge_state *state,
                                           struct drm_bridge *bridge)
 {
        __drm_atomic_helper_private_obj_create_state(&bridge->base, 
&state->base);
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1daac8a7f4c9..776856e23592 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -540,11 +540,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;
 }
@@ -555,11 +558,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 00a95f927e34..70e574fbf034 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -528,10 +528,26 @@ struct drm_bridge_funcs {
         * A valid drm_bridge_state object in case of success, an ERR_PTR()
         * giving the reason of the failure otherwise.
         */
        struct drm_bridge_state *(*atomic_reset)(struct drm_bridge *bridge);
 
+       /**
+        * @atomic_create_state:
+        *
+        * 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 new, pristine, bridge state instance or an error pointer
+        * on failure.
+        */
+       struct drm_bridge_state *(*atomic_create_state)(struct drm_bridge 
*bridge);
+
        /**
         * @detect:
         *
         * Check if anything is attached to the bridge output.
         *
@@ -1373,11 +1389,12 @@ drm_bridge_get_current_state(struct drm_bridge *bridge)
        /*
         * Only atomic bridges will have bridge->base initialized by
         * drm_atomic_private_obj_init(), so we need to make sure we're
         * working with one before we try to use the lock.
         */
-       if (!bridge->funcs || !bridge->funcs->atomic_reset)
+       if (!bridge->funcs ||
+           !(bridge->funcs->atomic_reset || 
bridge->funcs->atomic_create_state))
                return NULL;
 
        drm_modeset_lock_assert_held(&bridge->base.lock);
 
        if (!bridge->base.state)

-- 
2.54.0

Reply via email to