[PATCH 00/14] drm/atomic: Rework initial state allocation

[PATCH 00/14] drm/atomic: Rework initial state allocation

[Maxime Ripard]

Hi,

This series started from my work on the hardware state readout[1], and
more specifically a discussion with Thomas[2].

This series expands the work that has been merged recently to make
drm_private_obj and drm_private_state allocation a bit more consistent
and ended up creating a new atomic_create_state callback to allocate a
new state with no side effect.

The first patches are a documentation of the existing behaviour and some
random cleanups.

Then, we add the new atomic_create_state callback to every other DRM
object. Next, we leverage those new callbacks to create a new helper,
drm_mode_config_create_state() to create the initial state for all the
objects of a driver.

Finally, we hook that new helper in drm_dev_register and start
converting a few drivers.

This was tested on a TI SK-AM62, with the tidss driver.

Let me know what you think,
Maxime

1: https://lore.kernel.org/dri-devel/20250902-drm-state-readout-v1-0-14ad5315da3f@kernel.org/
2: https://lore.kernel.org/dri-devel/5920ffe5-b6b1-484b-b320-332b9eb9db82@suse.de/

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Maxime Ripard (14):
      drm/atomic: Document atomic state lifetime
      drm/atomic: Drop drm_private_state.obj assignment from create_state
      drm/mode-config: Mention drm_mode_config_reset() culprits
      drm/atomic-state-helper: Fix __drm_atomic_helper_plane_reset() doc typo
      drm/plane: Add new atomic_create_state callback
      drm/crtc: Add new atomic_create_state callback
      drm/connector: Add new atomic_create_state callback
      drm/mode-config: Create drm_mode_config_create_state()
      drm/drv: Call drm_mode_config_create_state() by default
      drm/atomic: Drop private obj state allocation
      drm/drv: Drop drm_mode_config_reset() from our skeleton
      drm/tidss: Drop call to drm_mode_config_reset at probe time
      drm/tidss: Convert to atomic_create_state
      drm/bridge_connector: Convert to atomic_create_state

 Documentation/gpu/drm-kms.rst                  |   6 ++
 drivers/gpu/drm/display/drm_bridge_connector.c |  15 ++-
 drivers/gpu/drm/drm_atomic.c                   |  67 ++++++++++--
 drivers/gpu/drm/drm_atomic_state_helper.c      | 128 +++++++++++++++++++++-
 drivers/gpu/drm/drm_drv.c                      |   6 +-
 drivers/gpu/drm/drm_mode_config.c              | 141 +++++++++++++++++++++++++
 drivers/gpu/drm/tidss/tidss_crtc.c             |  17 ++-
 drivers/gpu/drm/tidss/tidss_kms.c              |   2 -
 drivers/gpu/drm/tidss/tidss_plane.c            |   2 +-
 include/drm/drm_atomic_state_helper.h          |  12 +++
 include/drm/drm_connector.h                    |  13 +++
 include/drm/drm_crtc.h                         |  13 +++
 include/drm/drm_mode_config.h                  |   1 +
 include/drm/drm_plane.h                        |  13 +++
 14 files changed, 408 insertions(+), 28 deletions(-)
---
base-commit: 7b80021b1f0daed917ecc7f75077df799a8a698c
change-id: 20260310-drm-mode-config-init-1e1f52b745d0
prerequisite-change-id: 20251008-drm-private-obj-reset-ae1e2741027a:v5
prerequisite-patch-id: 9684f0ca4b16455c1340409561e8fb32f98b327a
prerequisite-patch-id: e177eb92b269436a94d7ef603d44436799be7469
prerequisite-patch-id: fef6d20ab33358c1db6cd9d21aa8ec0990cae758
prerequisite-patch-id: 2dcc96d43f34d8f6237829ed29d0087c092954d1

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH 01/14] drm/atomic: Document atomic state lifetime

[Maxime Ripard]

How drm_atomic_state structures and the various entity structures are
allocated and freed isn't really trivial, so let's document it.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 Documentation/gpu/drm-kms.rst |  6 +++++
 drivers/gpu/drm/drm_atomic.c  | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 2292e65f044c3bdebafbb8f83dfe7ac12e831273..017c7b196ed7ead4cf5fa8572e1f977d9e00dda8 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -280,10 +280,16 @@ structure, ordering of committing state changes to hardware is sequenced using
 :c:type:`struct drm_crtc_commit <drm_crtc_commit>`.
 
 Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed
 coverage of specific topics.
 
+Atomic State Lifetime
+---------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
+   :doc: state lifetime
+
 Handling Driver Private State
 -----------------------------
 
 .. kernel-doc:: drivers/gpu/drm/drm_atomic.c
    :doc: handling driver private state
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 4283ab4d06c581727cc98b1dc870bf69691ea654..92c6afc8f22c8307a59dc266aacdb8e03351409d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -45,10 +45,62 @@
 #include <drm/drm_colorop.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
 
+/**
+ * DOC: state lifetime
+ *
+ * &struct drm_atomic_state represents an update to video pipeline
+ * state. Despite its confusing name, it's actually a transient object
+ * that holds a state update as a collection of pointer to individual
+ * objects states. &struct drm_atomic_state has a much shorter lifetime
+ * than the objects states, since it's only allocated while preparing,
+ * checking or doing the update, while object states are allocated while
+ * the state will be, or is active in the hardware.
+ *
+ * Their respective lifetimes are:
+ *
+ * - at reset time, the object reset implementation will allocate a new,
+ *   default, state and will store it in the object state pointer.
+ *
+ * - whenever a new update is needed:
+ *
+ *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().
+ *
+ *   + we copy the state of each affected entity into our &struct
+ *     drm_atomic_state using drm_atomic_get_plane_state(),
+ *     drm_atomic_get_crtc_state(), drm_atomic_get_connector_state(), or
+ *     drm_atomic_get_private_obj_state(). That state can then be
+ *     modified.
+ *
+ *     At that point, &struct drm_atomic_state stores three state
+ *     pointers for that particular entity: the old, new, and existing
+ *     (called "state") states. The old state is the state currently
+ *     active in the hardware, which is either the one initialized by
+ *     reset() or a newer one if a commit has been made. The new state
+ *     is the state we just allocated and we might eventually commit to
+ *     the hardware. The existing state points to the state we'll
+ *     eventually have to free when the drm_atomic_state will be
+ *     destroyed, but points to the new state for now.
+ *
+ *   + After the state is populated, it is checked. If the check is
+ *     successful, the update is committed. Part of the commit is a call
+ *     to drm_atomic_helper_swap_state() which will turn the new states
+ *     into the active states. Doing so involves updating the objects
+ *     state pointer (&drm_crtc.state or similar) to point to the new
+ *     state, and the existing states will now point to the old states,
+ *     that used to be active but isn't anymore.
+ *
+ *   + When the commit is done, and when all references to our &struct
+ *     drm_atomic_state are put, drm_atomic_state_clear() runs and will
+ *     free all the old states.
+ *
+ *   + Now, we don't have any active &struct drm_atomic_state anymore,
+ *     and only the entity active states remain allocated.
+ */
+
 void __drm_crtc_commit_free(struct kref *kref)
 {
 	struct drm_crtc_commit *commit =
 		container_of(kref, struct drm_crtc_commit, ref);
 

-- 
2.53.0

[PATCH 02/14] drm/atomic: Drop drm_private_state.obj assignment from create_state

[Maxime Ripard]

The initial intent of the atomic_create_state helper was to simply
allocate a proper drm_private_state and returning it, without any side
effect.

However, the __drm_atomic_helper_private_obj_create_state() introduces a
side effect by setting the drm_private_obj.state to the newly allocated
state.

This assignment defeats the purpose, but is also redundant since
the only caller, drm_atomic_private_obj_init(), will also set this
pointer to the newly allocated state.

Let's drop the assignment in __drm_atomic_helper_private_obj_create_state().

Fixes: e7be39ed1716 ("drm/atomic-helper: Add private_obj atomic_create_state helper")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index bd6faa09f83b498b1417869493f31d876cd13914..323abc9926e084ad595768c06d5c5ee28c22c014 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -728,12 +728,10 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
 void __drm_atomic_helper_private_obj_create_state(struct drm_private_obj *obj,
 						  struct drm_private_state *state)
 {
 	if (state)
 		state->obj = obj;
-
-	obj->state = state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_private_obj_create_state);
 
 /**
  * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state

-- 
2.53.0

[PATCH 03/14] drm/mode-config: Mention drm_mode_config_reset() culprits

[Maxime Ripard]

drm_mode_config_reset() has the expectation that drm_private_states are
not reset.

This is especially significant for the DP MST and tunneling code that
expect to be preserved across a suspend/resume cycle, where
drm_mode_config_reset() is also used.

Let's document this expectation.

Link: https://lore.kernel.org/dri-devel/aOaQLx-7EpsHRwkH@ideak-desk/
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_mode_config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 84ae8a23a36786705fea7c0eafd7f20813b7c8f9..54c27376f9894ef5eee378bb5b1e5fc7049de922 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -187,10 +187,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
  * @dev: drm device
  *
  * This functions calls all the crtc's, encoder's and connector's ->reset
  * callback. Drivers can use this in e.g. their driver load or resume code to
  * reset hardware and software state.
+ *
+ * It's worth noting that @drm_private_obj structures are expected to be
+ * stable across suspend/resume cycles, and @drm_mode_config_reset()
+ * will not affect these structures.
  */
 void drm_mode_config_reset(struct drm_device *dev)
 {
 	struct drm_crtc *crtc;
 	struct drm_colorop *colorop;

-- 
2.53.0

[PATCH 04/14] drm/atomic-state-helper: Fix __drm_atomic_helper_plane_reset() doc typo

[Maxime Ripard]

A typo has slipped through in the __drm_atomic_helper_plane_reset()
documentation, probably due to copy and paste. It will not assign
drm_crtc state pointer, but rather the drm_plane's.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 323abc9926e084ad595768c06d5c5ee28c22c014..a1abf4247c348eca21da348c3893dd843b9ed391 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -302,11 +302,11 @@ EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
  * __drm_atomic_helper_plane_reset - reset state on plane
  * @plane: drm plane
  * @plane_state: plane state to assign
  *
  * Initializes the newly allocated @plane_state and assigns it to
- * the &drm_crtc->state pointer of @plane, usually required when
+ * the &drm_plane->state pointer of @plane, usually required when
  * initializing the drivers or when called from the &drm_plane_funcs.reset
  * hook.
  *
  * This is useful for drivers that subclass the plane state.
  */

-- 
2.53.0

[PATCH 05/14] drm/plane: Add new atomic_create_state callback

[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.

Instead of relying on the reset() callback, it created a new
atomic_create_state hook. This is helpful because reset is a bit
overloaded: it's used to create the initial software tate, reset it, but
also reset the hardware.

It can also be used either at probe time, to create the initial state
and possibly reset the hardware to an expected default, but also during
suspend/resume.

Both these cases come with different expectations too: during the
initialization, we want to initialize all states, but during
suspend/resume, drm_private_states for example are expected to be kept
around.

And reset() isn't fallible, which makes it harder to handle
initialization errors properly.

And this is only really relevant for some drivers, since all the helpers
for reset only create a new state, and don't touch the hardware at all.

It was thus decided to create a new hook that would allocate and
initialize a pristine state without any side effect:
atomic_create_state to untangle a bit some of it, and to separate the
initialization with the actual reset one might need during a
suspend/resume.

Let's continue the transition to the new pattern with planes.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 41 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c         | 18 ++++++++++++++
 include/drm/drm_atomic_state_helper.h     |  4 +++
 include/drm/drm_plane.h                   | 13 ++++++++++
 4 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index a1abf4247c348eca21da348c3893dd843b9ed391..898e168eeae1c3899121ffd13c79fae7803fcd2a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -318,10 +318,29 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 
 	plane->state = plane_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
 
+/**
+ * __drm_atomic_helper_plane_create_state - initializes plane state
+ * @plane: plane object
+ * @state: new state to initialize
+ *
+ * Initializes the newly allocated @state, usually required when
+ * initializing the drivers.
+ *
+ * @state is assumed to be zeroed.
+ *
+ * This is useful for drivers that subclass @drm_plane_state.
+ */
+void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
+					    struct drm_plane_state *state)
+{
+	__drm_atomic_helper_plane_reset(plane, state);
+}
+EXPORT_SYMBOL(__drm_atomic_helper_plane_create_state);
+
 /**
  * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for planes
  * @plane: drm plane
  *
  * Resets the atomic state for @plane by freeing the state pointer (which might
@@ -337,10 +356,32 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
 	if (plane->state)
 		__drm_atomic_helper_plane_reset(plane, plane->state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
 
+/**
+ * drm_atomic_helper_plane_create_state - default &drm_plane_funcs.atomic_create_state hook for planes
+ * @plane: plane object
+ *
+ * Initializes a pristine @drm_plane_state.
+ *
+ * This is useful for drivers that don't subclass @drm_plane_state.
+ */
+struct drm_plane_state *drm_atomic_helper_plane_create_state(struct drm_plane *plane)
+{
+	struct drm_plane_state *state;
+
+	state = kzalloc_obj(*state);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_plane_reset(plane, state);
+
+	return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_plane_create_state);
+
 /**
  * __drm_atomic_helper_plane_duplicate_state - copy atomic plane state
  * @plane: plane object
  * @state: atomic plane state
  *
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 54c27376f9894ef5eee378bb5b1e5fc7049de922..43fe64540b8ec4df4bb823d96db607e6bfe58824 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -180,10 +180,26 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
 	drm_connector_list_iter_end(&conn_iter);
 
 	return ret;
 }
 
+static int drm_mode_config_plane_create_state(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state;
+
+	if (!plane->funcs->atomic_create_state)
+		return 0;
+
+	plane_state = plane->funcs->atomic_create_state(plane);
+	if (IS_ERR(plane_state))
+		return PTR_ERR(plane_state);
+
+	plane->state = plane_state;
+
+	return 0;
+}
+
 /**
  * drm_mode_config_reset - call ->reset callbacks
  * @dev: drm device
  *
  * This functions calls all the crtc's, encoder's and connector's ->reset
@@ -207,10 +223,12 @@ void drm_mode_config_reset(struct drm_device *dev)
 		drm_colorop_reset(colorop);
 
 	drm_for_each_plane(plane, dev)
 		if (plane->funcs->reset)
 			plane->funcs->reset(plane);
+		else if (plane->funcs->atomic_create_state)
+			drm_mode_config_plane_create_state(plane);
 
 	drm_for_each_crtc(crtc, dev)
 		if (crtc->funcs->reset)
 			crtc->funcs->reset(crtc);
 
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index 900672c6ea90ba9cb87e38a7c84225972aee43c5..f56a91fca20e778d8ccb1767301dc00783bf40f3 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -51,12 +51,16 @@ struct drm_crtc_state *
 drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					  struct drm_crtc_state *state);
 
+void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
+					    struct drm_plane_state *state);
 void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
 					   struct drm_plane *plane);
+struct drm_plane_state *
+drm_atomic_helper_plane_create_state(struct drm_plane *plane);
 void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 				     struct drm_plane_state *state);
 void drm_atomic_helper_plane_reset(struct drm_plane *plane);
 void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 					       struct drm_plane_state *state);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 703ef4d1bbbcf084c43aa5e127d28691878061c4..4d4d511b681d50c17fbc593cce9f706d63e04a52 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -386,10 +386,23 @@ struct drm_plane_funcs {
 	 * 0 on success or a negative error code on failure.
 	 */
 	int (*set_property)(struct drm_plane *plane,
 			    struct drm_property *property, uint64_t val);
 
+	/**
+	 * @atomic_create_state:
+	 *
+	 * Allocates a pristine, initialized, state for the plane object
+	 * and returns it.
+	 *
+	 * RETURNS:
+	 *
+	 * A new, pristine, plane state instance or an error pointer
+	 * on failure.
+	 */
+	struct drm_plane_state *(*atomic_create_state)(struct drm_plane *plane);
+
 	/**
 	 * @atomic_duplicate_state:
 	 *
 	 * Duplicate the current atomic state for this plane and return it.
 	 * The core and helpers guarantee that any atomic state duplicated with

-- 
2.53.0

[PATCH 06/14] drm/crtc: Add new atomic_create_state callback

[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.

Instead of relying on the reset() callback, it created a new
atomic_create_state hook. This is helpful because reset is a bit
overloaded: it's used to create the initial software tate, reset it, but
also reset the hardware.

It can also be used either at probe time, to create the initial state
and possibly reset the hardware to an expected default, but also during
suspend/resume.

Both these cases come with different expectations too: during the
initialization, we want to initialize all states, but during
suspend/resume, drm_private_states for example are expected to be kept
around.

And reset() isn't fallible, which makes it harder to handle
initialization errors properly.

And this is only really relevant for some drivers, since all the helpers
for reset only create a new state, and don't touch the hardware at all.

It was thus decided to create a new hook that would allocate and
initialize a pristine state without any side effect:
atomic_create_state to untangle a bit some of it, and to separate the
initialization with the actual reset one might need during a
suspend/resume.

Let's continue the transition to the new pattern with crtcs.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 41 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c         | 18 ++++++++++++++
 include/drm/drm_atomic_state_helper.h     |  4 +++
 include/drm/drm_crtc.h                    | 13 ++++++++++
 4 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 898e168eeae1c3899121ffd13c79fae7803fcd2a..28fa315586c2530e50f0b470d9ad43f5c3a5e056 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -102,10 +102,29 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 
 	crtc->state = crtc_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_reset);
 
+/**
+ * __drm_atomic_helper_crtc_create_state - initializes crtc state
+ * @crtc: crtc object
+ * @state: new state to initialize
+ *
+ * Initializes the newly allocated @state, usually required when
+ * initializing the drivers.
+ *
+ * @state is assumed to be zeroed.
+ *
+ * This is useful for drivers that subclass @drm_crtc_state.
+ */
+void __drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc,
+					   struct drm_crtc_state *state)
+{
+	__drm_atomic_helper_crtc_reset(crtc, state);
+}
+EXPORT_SYMBOL(__drm_atomic_helper_crtc_create_state);
+
 /**
  * drm_atomic_helper_crtc_reset - default &drm_crtc_funcs.reset hook for CRTCs
  * @crtc: drm CRTC
  *
  * Resets the atomic state for @crtc by freeing the state pointer (which might
@@ -121,10 +140,32 @@ void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
 
 	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
 
+/**
+ * drm_atomic_helper_crtc_create_state - default &drm_crtc_funcs.atomic_create_state hook for crtcs
+ * @crtc: crtc object
+ *
+ * Initializes a pristine @drm_crtc_state.
+ *
+ * This is useful for drivers that don't subclass @drm_crtc_state.
+ */
+struct drm_crtc_state *drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc)
+{
+	struct drm_crtc_state *state;
+
+	state = kzalloc_obj(*state);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_crtc_reset(crtc, state);
+
+	return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_create_state);
+
 /**
  * __drm_atomic_helper_crtc_duplicate_state - copy atomic CRTC state
  * @crtc: CRTC object
  * @state: atomic CRTC state
  *
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 43fe64540b8ec4df4bb823d96db607e6bfe58824..6d47af4097071cfb93a2c23909dd5d643687111f 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -196,10 +196,26 @@ static int drm_mode_config_plane_create_state(struct drm_plane *plane)
 	plane->state = plane_state;
 
 	return 0;
 }
 
+static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
+{
+	struct drm_crtc_state *crtc_state;
+
+	if (!crtc->funcs->atomic_create_state)
+		return 0;
+
+	crtc_state = crtc->funcs->atomic_create_state(crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	crtc->state = crtc_state;
+
+	return 0;
+}
+
 /**
  * drm_mode_config_reset - call ->reset callbacks
  * @dev: drm device
  *
  * This functions calls all the crtc's, encoder's and connector's ->reset
@@ -229,10 +245,12 @@ void drm_mode_config_reset(struct drm_device *dev)
 			drm_mode_config_plane_create_state(plane);
 
 	drm_for_each_crtc(crtc, dev)
 		if (crtc->funcs->reset)
 			crtc->funcs->reset(crtc);
+		else if (crtc->funcs->atomic_create_state)
+			drm_mode_config_crtc_create_state(crtc);
 
 	drm_for_each_encoder(encoder, dev)
 		if (encoder->funcs && encoder->funcs->reset)
 			encoder->funcs->reset(encoder);
 
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index f56a91fca20e778d8ccb1767301dc00783bf40f3..d7e8ac09d81febef3a95ed460cd00b91ee3d05b2 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -43,10 +43,14 @@ struct drm_device;
 void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
 					  struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
+void __drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc,
+					   struct drm_crtc_state *state);
+struct drm_crtc_state *
+drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 					      struct drm_crtc_state *state);
 struct drm_crtc_state *
 drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 66278ffeebd68e69e9d0724ea5a33f93231e3f8b..30dbf430586ee7b106acbdfb28d27ea81c1377a6 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -624,10 +624,23 @@ struct drm_crtc_funcs {
 	 * 0 on success or a negative error code on failure.
 	 */
 	int (*set_property)(struct drm_crtc *crtc,
 			    struct drm_property *property, uint64_t val);
 
+	/**
+	 * @atomic_create_state:
+	 *
+	 * Allocates a pristine, initialized, state for the crtc object
+	 * and returns it.
+	 *
+	 * RETURNS:
+	 *
+	 * A new, pristine, crtc state instance or an error pointer
+	 * on failure.
+	 */
+	struct drm_crtc_state *(*atomic_create_state)(struct drm_crtc *crtc);
+
 	/**
 	 * @atomic_duplicate_state:
 	 *
 	 * Duplicate the current atomic state for this CRTC and return it.
 	 * The core and helpers guarantee that any atomic state duplicated with

-- 
2.53.0

[PATCH 07/14] drm/connector: Add new atomic_create_state callback

[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.

Instead of relying on the reset() callback, it created a new
atomic_create_state hook. This is helpful because reset is a bit
overloaded: it's used to create the initial software tate, reset it, but
also reset the hardware.

It can also be used either at probe time, to create the initial state
and possibly reset the hardware to an expected default, but also during
suspend/resume.

Both these cases come with different expectations too: during the
initialization, we want to initialize all states, but during
suspend/resume, drm_private_states for example are expected to be kept
around.

And reset() isn't fallible, which makes it harder to handle
initialization errors properly.

And this is only really relevant for some drivers, since all the helpers
for reset only create a new state, and don't touch the hardware at all.

It was thus decided to create a new hook that would allocate and
initialize a pristine state without any side effect:
atomic_create_state to untangle a bit some of it, and to separate the
initialization with the actual reset one might need during a
suspend/resume.

Let's continue the transition to the new pattern with connectors.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 42 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_mode_config.c         | 18 +++++++++++++
 include/drm/drm_atomic_state_helper.h     |  4 +++
 include/drm/drm_connector.h               | 13 ++++++++++
 4 files changed, 77 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 28fa315586c2530e50f0b470d9ad43f5c3a5e056..72cc88972e154d931e9ae828888ea6a7cfb95904 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -543,10 +543,29 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 
 	connector->state = conn_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_connector_reset);
 
+/**
+ * __drm_atomic_helper_connector_create_state - initializes connector state
+ * @connector: connector object
+ * @state: new state to initialize
+ *
+ * Initializes the newly allocated @state, usually required when
+ * initializing the drivers.
+ *
+ * @state is assumed to be zeroed.
+ *
+ * This is useful for drivers that subclass @drm_connector_state.
+ */
+void __drm_atomic_helper_connector_create_state(struct drm_connector *connector,
+						struct drm_connector_state *state)
+{
+	__drm_atomic_helper_connector_reset(connector, state);
+}
+EXPORT_SYMBOL(__drm_atomic_helper_connector_create_state);
+
 /**
  * drm_atomic_helper_connector_reset - default &drm_connector_funcs.reset hook for connectors
  * @connector: drm connector
  *
  * Resets the atomic state for @connector by freeing the state pointer (which
@@ -563,10 +582,33 @@ void drm_atomic_helper_connector_reset(struct drm_connector *connector)
 	kfree(connector->state);
 	__drm_atomic_helper_connector_reset(connector, conn_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_reset);
 
+/**
+ * drm_atomic_helper_connector_create_state - default &drm_connector_funcs.atomic_create_state hook for connectors
+ * @connector: connector object
+ *
+ * Initializes a pristine @drm_connector_state.
+ *
+ * This is useful for drivers that don't subclass @drm_connector_state.
+ */
+struct drm_connector_state *
+drm_atomic_helper_connector_create_state(struct drm_connector *connector)
+{
+	struct drm_connector_state *state;
+
+	state = kzalloc_obj(*state);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_connector_reset(connector, state);
+
+	return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_create_state);
+
 /**
  * drm_atomic_helper_connector_tv_margins_reset - Resets TV connector properties
  * @connector: DRM connector
  *
  * Resets the TV-related properties attached to a connector.
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 6d47af4097071cfb93a2c23909dd5d643687111f..95759a4b5176ff17032a9a267ec9de6980be0abc 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -212,10 +212,26 @@ static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
 	crtc->state = crtc_state;
 
 	return 0;
 }
 
+static int drm_mode_config_connector_create_state(struct drm_connector *connector)
+{
+	struct drm_connector_state *conn_state;
+
+	if (!connector->funcs->atomic_create_state)
+		return 0;
+
+	conn_state = connector->funcs->atomic_create_state(connector);
+	if (IS_ERR(conn_state))
+		return PTR_ERR(conn_state);
+
+	connector->state = conn_state;
+
+	return 0;
+}
+
 /**
  * drm_mode_config_reset - call ->reset callbacks
  * @dev: drm device
  *
  * This functions calls all the crtc's, encoder's and connector's ->reset
@@ -256,10 +272,12 @@ void drm_mode_config_reset(struct drm_device *dev)
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter)
 		if (connector->funcs->reset)
 			connector->funcs->reset(connector);
+		else if (connector->funcs->atomic_create_state)
+			drm_mode_config_connector_create_state(connector);
 	drm_connector_list_iter_end(&conn_iter);
 }
 EXPORT_SYMBOL(drm_mode_config_reset);
 
 /*
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index d7e8ac09d81febef3a95ed460cd00b91ee3d05b2..d5259cbfdb150553932d2b3c9c513bf8e773c96d 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -76,11 +76,15 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
 
 void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
 					       struct drm_connector *connector);
 void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 					 struct drm_connector_state *conn_state);
+void __drm_atomic_helper_connector_create_state(struct drm_connector *connector,
+						struct drm_connector_state *state);
 void drm_atomic_helper_connector_reset(struct drm_connector *connector);
+struct drm_connector_state *
+drm_atomic_helper_connector_create_state(struct drm_connector *connector);
 void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
 int drm_atomic_helper_connector_tv_check(struct drm_connector *connector,
 					 struct drm_atomic_state *state);
 void drm_atomic_helper_connector_tv_margins_reset(struct drm_connector *connector);
 void
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c18be8c19de0ab1e02e7d7b1fcd9f2ab5ef1dd15..6d73068de157144770931c64ce3f91e6f15ef0e4 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1509,10 +1509,23 @@ struct drm_connector_funcs {
 	 * when a connector is being hot-unplugged for drivers that support
 	 * connector hotplugging (e.g. DisplayPort MST).
 	 */
 	void (*destroy)(struct drm_connector *connector);
 
+	/**
+	 * @atomic_create_state:
+	 *
+	 * Allocates a pristine, initialized, state for the connector object
+	 * and returns it.
+	 *
+	 * RETURNS:
+	 *
+	 * A new, pristine, connector state instance or an error pointer
+	 * on failure.
+	 */
+	struct drm_connector_state *(*atomic_create_state)(struct drm_connector *connector);
+
 	/**
 	 * @atomic_duplicate_state:
 	 *
 	 * Duplicate the current atomic state for this connector and return it.
 	 * The core and helpers guarantee that any atomic state duplicated with

-- 
2.53.0

[PATCH 08/14] drm/mode-config: Create drm_mode_config_create_state()

[Maxime Ripard]

drm_mode_config_reset() can be used to create the initial state, but
also to return to the initial state, when doing a suspend/resume cycle
for example.

It also affects both the software and the hardware, and drivers can
either choose to reset the hardware as well. Most will just create an
empty state and the synchronisation between hardware and software states
will effectively be done when the first commit is done.

That dual role can be harmful, since some objects do need to be
initialized but also need to be preserved across a suspend/resume cycle.
drm_private_obj are such objects for example.

Thus, let's create another helper for drivers to call to initialize
their state when the driver is loaded, so we can make
drm_mode_config_reset() only about handling suspend/resume and similar.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic.c      | 12 +++++-
 drivers/gpu/drm/drm_mode_config.c | 83 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_mode_config.h     |  1 +
 3 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 92c6afc8f22c8307a59dc266aacdb8e03351409d..d1885f895cce78725419b6291f4dbe5563e3b240 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -58,12 +58,20 @@
  * checking or doing the update, while object states are allocated while
  * the state will be, or is active in the hardware.
  *
  * Their respective lifetimes are:
  *
- * - at reset time, the object reset implementation will allocate a new,
- *   default, state and will store it in the object state pointer.
+ * - at driver initialization time, the driver will allocate an initial,
+ *   pristine, state and will store it using
+ *   drm_mode_config_create_state(). Historically, this was one of
+ *   drm_mode_config_reset() job, so one might still encounter it in a
+ *   driver.
+ *
+ * - at reset time, for example during suspend/resume,
+ *   drm_mode_config_reset() will reset the software and hardware state
+ *   to a known default and will store it in the object state pointer.
+ *   Not all objects are affected by drm_mode_config_reset() though.
  *
  * - whenever a new update is needed:
  *
  *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().
  *
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 95759a4b5176ff17032a9a267ec9de6980be0abc..1a84ac063d381014eb8afd7116e1e8b7a8fc92ca 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -21,10 +21,11 @@
  */
 
 #include <linux/export.h>
 #include <linux/uaccess.h>
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_file.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_managed.h>
@@ -278,10 +279,92 @@ void drm_mode_config_reset(struct drm_device *dev)
 			drm_mode_config_connector_create_state(connector);
 	drm_connector_list_iter_end(&conn_iter);
 }
 EXPORT_SYMBOL(drm_mode_config_reset);
 
+/**
+ * drm_mode_config_create_state - Allocates the initial state
+ * @dev: drm device
+ *
+ * This functions creates the initial state for all the objects. Drivers
+ * can use this in e.g. their driver load to initialize its software
+ * state.
+ *
+ * It has two main differences with drm_mode_config_reset(): the reset()
+ * hooks aren't called and thus the hardware will be left untouched, but
+ * also the @drm_private_obj structures will be initialized as opposed
+ * to drm_mode_config_reset() that skips them.
+ *
+ * Returns: 0 on success, negative error value on failure.
+ */
+int drm_mode_config_create_state(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	struct drm_colorop *colorop;
+	struct drm_plane *plane;
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	struct drm_private_obj *privobj;
+	int ret;
+
+	drm_for_each_privobj(privobj, dev) {
+		struct drm_private_state *privobj_state;
+
+		if (privobj->state)
+			continue;
+
+		if (!privobj->funcs->atomic_create_state)
+			continue;
+
+		privobj_state = privobj->funcs->atomic_create_state(privobj);
+		if (IS_ERR(privobj_state))
+			return PTR_ERR(privobj_state);
+
+		privobj->state = privobj_state;
+	}
+
+	drm_for_each_colorop(colorop, dev) {
+		if (colorop->state)
+			continue;
+
+		// TODO: Implement atomic_create_state for colorop.
+		drm_colorop_reset(colorop);
+	}
+
+	drm_for_each_plane(plane, dev) {
+		if (plane->state)
+			continue;
+
+		ret = drm_mode_config_plane_create_state(plane);
+		if (ret)
+			return ret;
+	}
+
+	drm_for_each_crtc(crtc, dev) {
+		if (crtc->state)
+			continue;
+
+		ret = drm_mode_config_crtc_create_state(crtc);
+		if (ret)
+			return ret;
+	}
+
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		if (connector->state)
+			continue;
+
+		ret = drm_mode_config_connector_create_state(connector);
+		if (ret)
+			return ret;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mode_config_create_state);
+
 /*
  * Global properties
  */
 static const struct drm_prop_enum_list drm_plane_type_enum_list[] = {
 	{ DRM_PLANE_TYPE_OVERLAY, "Overlay" },
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 5e1dd0cfccde2d2bc00a09e98e4adc65833dad08..428ae75b7c4f193dfbd49c15a2f2ef32209a6cef 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -1000,9 +1000,10 @@ int __must_check drmm_mode_config_init(struct drm_device *dev);
 static inline int drm_mode_config_init(struct drm_device *dev)
 {
 	return drmm_mode_config_init(dev);
 }
 
+int drm_mode_config_create_state(struct drm_device *dev);
 void drm_mode_config_reset(struct drm_device *dev);
 void drm_mode_config_cleanup(struct drm_device *dev);
 
 #endif

-- 
2.53.0

[PATCH 09/14] drm/drv: Call drm_mode_config_create_state() by default

[Maxime Ripard]

Almost all drivers, and our documented skeleton, call
drm_mode_config_reset() prior to calling drm_dev_register() to
initialize its DRM object states.

Now that we have drm_mode_config_create_state() to create that initial
state if it doesn't exist, we can call it directly in
drm_dev_register(). That way, we know that the initial atomic state will
always be allocated without any boilerplate.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 2915118436ce8a6640cfb0c59936031990727ed1..820106d56ab399a39cac56d98662b5ddbcae8ded 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1097,10 +1097,14 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		ret = drm_modeset_register_all(dev);
 		if (ret)
 			goto err_unload;
+
+		ret = drm_mode_config_create_state(dev);
+		if (ret)
+			goto err_unload;
 	}
 	drm_panic_register(dev);
 	drm_client_sysrq_register(dev);
 
 	DRM_INFO("Initialized %s %d.%d.%d for %s on minor %d\n",

-- 
2.53.0

[PATCH 10/14] drm/atomic: Drop private obj state allocation

[Maxime Ripard]

Now that drm_dev_register() calls drm_mode_config_create_state() for
every modeset driver, the private obj states will be initialized at
driver registration automatically if they haven't already.

Thus, the explicit initial allocation we have in
drm_atomic_private_obj_init() is now redundant, and we can remove it.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d1885f895cce78725419b6291f4dbe5563e3b240..e8dc019364d27123c3a682a10323fba9ff76f9df 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -991,25 +991,18 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
  */
 int drm_atomic_private_obj_init(struct drm_device *dev,
 				struct drm_private_obj *obj,
 				const struct drm_private_state_funcs *funcs)
 {
-	struct drm_private_state *state;
 	memset(obj, 0, sizeof(*obj));
 
 	drm_modeset_lock_init(&obj->lock);
 
 	obj->dev = dev;
 	obj->funcs = funcs;
 	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
 
-	state = obj->funcs->atomic_create_state(obj);
-	if (IS_ERR(state))
-		return PTR_ERR(state);
-
-	obj->state = state;
-
 	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_private_obj_init);
 
 /**

-- 
2.53.0

[PATCH 11/14] drm/drv: Drop drm_mode_config_reset() from our skeleton

[Maxime Ripard]

Now that we have drm_mode_config_create_state() to create the initial
state called as part of drm_dev_register(), we can stop recommending
calling drm_mode_config_reset() to perform that initialization in our
skeleton.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 820106d56ab399a39cac56d98662b5ddbcae8ded..dc85d36670cf96dcd16e7eda8fe8563c6e7fc28d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -339,11 +339,11 @@ void drm_minor_release(struct drm_minor *minor)
  *
  *		// Further setup, display pipeline etc
  *
  *		platform_set_drvdata(pdev, drm);
  *
- *		drm_mode_config_reset(drm);
+ *		drm_mode_config_create_state(drm);
  *
  *		ret = drm_dev_register(drm);
  *		if (ret)
  *			return ret;
  *

-- 
2.53.0

[PATCH 12/14] drm/tidss: Drop call to drm_mode_config_reset at probe time

[Maxime Ripard]

Now that we have drm_mode_config_create_state() to create the initial
state called as part of drm_dev_register(), we don't need to call
drm_mode_config_reset() during the driver probe.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/tidss/tidss_kms.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
index 8bb93194e5ac686050c47f986b8cb6063eae22d3..b4779c09a1bfa5be7010bcd7d4f1379362996d2e 100644
--- a/drivers/gpu/drm/tidss/tidss_kms.c
+++ b/drivers/gpu/drm/tidss/tidss_kms.c
@@ -285,11 +285,9 @@ int tidss_modeset_init(struct tidss_device *tidss)
 
 	ret = drm_vblank_init(ddev, tidss->num_crtcs);
 	if (ret)
 		return ret;
 
-	drm_mode_config_reset(ddev);
-
 	dev_dbg(tidss->dev, "%s done\n", __func__);
 
 	return 0;
 }

-- 
2.53.0

[PATCH 13/14] drm/tidss: Convert to atomic_create_state

[Maxime Ripard]

Our driver uses reset to create the various object states, but only
calls the helper that allocate a new state. They are thus strictly
equivalent to the new atomic_create_state helpers, so let's switch to
these.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/tidss/tidss_crtc.c  | 17 +++++++----------
 drivers/gpu/drm/tidss/tidss_plane.c |  2 +-
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index a31c21c5f855ac8a94089dd3908e2510193b7d67..66e3d161c60bc14b2982cff4cdd43030d4086798 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -355,24 +355,21 @@ static void tidss_crtc_destroy_state(struct drm_crtc *crtc,
 
 	__drm_atomic_helper_crtc_destroy_state(&tstate->base);
 	kfree(tstate);
 }
 
-static void tidss_crtc_reset(struct drm_crtc *crtc)
+static struct drm_crtc_state *tidss_crtc_create_state(struct drm_crtc *crtc)
 {
 	struct tidss_crtc_state *tstate;
 
-	if (crtc->state)
-		tidss_crtc_destroy_state(crtc, crtc->state);
-
 	tstate = kzalloc_obj(*tstate);
-	if (!tstate) {
-		crtc->state = NULL;
-		return;
-	}
+	if (!tstate)
+		return ERR_PTR(-ENOMEM);
 
-	__drm_atomic_helper_crtc_reset(crtc, &tstate->base);
+	__drm_atomic_helper_crtc_create_state(crtc, &tstate->base);
+
+	return &tstate->base;
 }
 
 static struct drm_crtc_state *tidss_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct tidss_crtc_state *state, *current_state;
@@ -403,14 +400,14 @@ static void tidss_crtc_destroy(struct drm_crtc *crtc)
 	drm_crtc_cleanup(crtc);
 	kfree(tcrtc);
 }
 
 static const struct drm_crtc_funcs tidss_crtc_funcs = {
-	.reset = tidss_crtc_reset,
 	.destroy = tidss_crtc_destroy,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_create_state = tidss_crtc_create_state,
 	.atomic_duplicate_state = tidss_crtc_duplicate_state,
 	.atomic_destroy_state = tidss_crtc_destroy_state,
 	.enable_vblank = tidss_crtc_enable_vblank,
 	.disable_vblank = tidss_crtc_disable_vblank,
 };
diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
index aaa02c851c595aa3781ec2e6741af1999092aa40..518498d4576528a0ec59fd03cf27a87b1b3f1e6e 100644
--- a/drivers/gpu/drm/tidss/tidss_plane.c
+++ b/drivers/gpu/drm/tidss/tidss_plane.c
@@ -176,12 +176,12 @@ static const struct drm_plane_helper_funcs tidss_primary_plane_helper_funcs = {
 };
 
 static const struct drm_plane_funcs tidss_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.reset = drm_atomic_helper_plane_reset,
 	.destroy = drm_plane_destroy,
+	.atomic_create_state = drm_atomic_helper_plane_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
 struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,

-- 
2.53.0

[PATCH 14/14] drm/bridge_connector: Convert to atomic_create_state

[Maxime Ripard]

The connector created by drm_bridge_connector only initializes a
pristine state in reset, which is equivalent to that atomic_create_state
would expect. Let's convert to it.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index f686aa5c0ed9b84dbe5e0957df22d08aff2f1945..2f73576783f5f69ebce277a7537accefc94645a9 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -263,26 +263,33 @@ static void drm_bridge_connector_debugfs_init(struct drm_connector *connector,
 		if (bridge->funcs->debugfs_init)
 			bridge->funcs->debugfs_init(bridge, root);
 	}
 }
 
-static void drm_bridge_connector_reset(struct drm_connector *connector)
+static struct drm_connector_state *
+drm_bridge_connector_create_state(struct drm_connector *connector)
 {
 	struct drm_bridge_connector *bridge_connector =
 		to_drm_bridge_connector(connector);
+	struct drm_connector_state *conn_state;
+
+	conn_state = drm_atomic_helper_connector_create_state(connector);
+	if (IS_ERR(conn_state))
+		return conn_state;
 
-	drm_atomic_helper_connector_reset(connector);
 	if (bridge_connector->bridge_hdmi)
 		__drm_atomic_helper_connector_hdmi_reset(connector,
-							 connector->state);
+							 conn_state);
+
+	return conn_state;
 }
 
 static const struct drm_connector_funcs drm_bridge_connector_funcs = {
-	.reset = drm_bridge_connector_reset,
 	.detect = drm_bridge_connector_detect,
 	.force = drm_bridge_connector_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
+	.atomic_create_state = drm_bridge_connector_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.debugfs_init = drm_bridge_connector_debugfs_init,
 	.oob_hotplug_event = drm_bridge_connector_oob_hotplug_event,
 };

-- 
2.53.0

Re: [PATCH 01/14] drm/atomic: Document atomic state lifetime

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:06, Maxime Ripard wrote:
> How drm_atomic_state structures and the various entity structures are
> allocated and freed isn't really trivial, so let's document it.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  Documentation/gpu/drm-kms.rst |  6 +++++
>  drivers/gpu/drm/drm_atomic.c  | 52 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 2292e65f044c3bdebafbb8f83dfe7ac12e831273..017c7b196ed7ead4cf5fa8572e1f977d9e00dda8 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -280,10 +280,16 @@ structure, ordering of committing state changes to hardware is sequenced using
>  :c:type:`struct drm_crtc_commit <drm_crtc_commit>`.
>  
>  Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed
>  coverage of specific topics.
>  
> +Atomic State Lifetime
> +---------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> +   :doc: state lifetime
> +
>  Handling Driver Private State
>  -----------------------------
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_atomic.c
>     :doc: handling driver private state
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 4283ab4d06c581727cc98b1dc870bf69691ea654..92c6afc8f22c8307a59dc266aacdb8e03351409d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -45,10 +45,62 @@
>  #include <drm/drm_colorop.h>
>  
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
>  
> +/**
> + * DOC: state lifetime
> + *
> + * &struct drm_atomic_state represents an update to video pipeline
> + * state. Despite its confusing name, it's actually a transient object
> + * that holds a state update as a collection of pointer to individual
> + * objects states. &struct drm_atomic_state has a much shorter lifetime

Hmm, I think "a collection of pointers to individual object states". Hmm
or "objects' states"? I like the former.

> + * than the objects states, since it's only allocated while preparing,

"objects' states" or "object states".

> + * checking or doing the update, while object states are allocated while
> + * the state will be, or is active in the hardware.
> + *
> + * Their respective lifetimes are:
> + *
> + * - at reset time, the object reset implementation will allocate a new,
> + *   default, state and will store it in the object state pointer.

"object's". This is the "active state", is it?

> + *
> + * - whenever a new update is needed:
> + *
> + *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().
> + *
> + *   + we copy the state of each affected entity into our &struct
> + *     drm_atomic_state using drm_atomic_get_plane_state(),
> + *     drm_atomic_get_crtc_state(), drm_atomic_get_connector_state(), or
> + *     drm_atomic_get_private_obj_state(). That state can then be
> + *     modified.

Maybe clarify what is the state returned by these. It's the "active
state", isn't it, drm_crtc.state or similar?

> + *
> + *     At that point, &struct drm_atomic_state stores three state
> + *     pointers for that particular entity: the old, new, and existing
> + *     (called "state") states. The old state is the state currently
> + *     active in the hardware, which is either the one initialized by
> + *     reset() or a newer one if a commit has been made. The new state
> + *     is the state we just allocated and we might eventually commit to
> + *     the hardware. The existing state points to the state we'll
> + *     eventually have to free when the drm_atomic_state will be
> + *     destroyed, but points to the new state for now.

From this, I don't understand the difference between the old state and
the existing state. And if the existing state is the one we'll free,
isn't that the old state, not new state? Oh, is the existing state a
state we have to free when the drm_atomic_state would is freed? And at
this point the new state is the one, as it's not committed?

> + *
> + *   + After the state is populated, it is checked. If the check is
> + *     successful, the update is committed. Part of the commit is a call
> + *     to drm_atomic_helper_swap_state() which will turn the new states
> + *     into the active states. Doing so involves updating the objects

"object's"

> + *     state pointer (&drm_crtc.state or similar) to point to the new
> + *     state, and the existing states will now point to the old states,
> + *     that used to be active but isn't anymore.

"aren't"

I think I understand this, but... It kind of brings in a new state
concept, "active state".

> + *
> + *   + When the commit is done, and when all references to our &struct
> + *     drm_atomic_state are put, drm_atomic_state_clear() runs and will
> + *     free all the old states.
> + *
> + *   + Now, we don't have any active &struct drm_atomic_state anymore,
> + *     and only the entity active states remain allocated.
> + */
> +
Even if this is a bit hard to read, I think it really clarifies the
state lifetime.

 Tomi

Re: [PATCH 04/14] drm/atomic-state-helper: Fix __drm_atomic_helper_plane_reset() doc typo

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:06, Maxime Ripard wrote:
> A typo has slipped through in the __drm_atomic_helper_plane_reset()
> documentation, probably due to copy and paste. It will not assign
> drm_crtc state pointer, but rather the drm_plane's.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 323abc9926e084ad595768c06d5c5ee28c22c014..a1abf4247c348eca21da348c3893dd843b9ed391 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -302,11 +302,11 @@ EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>   * __drm_atomic_helper_plane_reset - reset state on plane
>   * @plane: drm plane
>   * @plane_state: plane state to assign
>   *
>   * Initializes the newly allocated @plane_state and assigns it to
> - * the &drm_crtc->state pointer of @plane, usually required when
> + * the &drm_plane->state pointer of @plane, usually required when
>   * initializing the drivers or when called from the &drm_plane_funcs.reset
>   * hook.
>   *
>   * This is useful for drivers that subclass the plane state.
>   */
> 

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

Re: [PATCH 11/14] drm/drv: Drop drm_mode_config_reset() from our skeleton

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:07, Maxime Ripard wrote:
> Now that we have drm_mode_config_create_state() to create the initial
> state called as part of drm_dev_register(), we can stop recommending
> calling drm_mode_config_reset() to perform that initialization in our
> skeleton.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 820106d56ab399a39cac56d98662b5ddbcae8ded..dc85d36670cf96dcd16e7eda8fe8563c6e7fc28d 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -339,11 +339,11 @@ void drm_minor_release(struct drm_minor *minor)
>   *
>   *		// Further setup, display pipeline etc
>   *
>   *		platform_set_drvdata(pdev, drm);
>   *
> - *		drm_mode_config_reset(drm);
> + *		drm_mode_config_create_state(drm);
>   *
>   *		ret = drm_dev_register(drm);
>   *		if (ret)
>   *			return ret;
>   *
> 

Shouldn't the call just be dropped, as drm_dev_register() does it?

 Tomi

Re: [PATCH 05/14] drm/plane: Add new atomic_create_state callback

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:06, Maxime Ripard wrote:
> Commit 47b5ac7daa46 ("drm/atomic: Add new atomic_create_state callback
> to drm_private_obj") introduced a new pattern for allocating drm object
> states.
> 
> Instead of relying on the reset() callback, it created a new
> atomic_create_state hook. This is helpful because reset is a bit
> overloaded: it's used to create the initial software tate, reset it, but
> also reset the hardware.
> 
> It can also be used either at probe time, to create the initial state
> and possibly reset the hardware to an expected default, but also during
> suspend/resume.
> 
> Both these cases come with different expectations too: during the
> initialization, we want to initialize all states, but during
> suspend/resume, drm_private_states for example are expected to be kept
> around.
> 
> And reset() isn't fallible, which makes it harder to handle
> initialization errors properly.
> 
> And this is only really relevant for some drivers, since all the helpers
> for reset only create a new state, and don't touch the hardware at all.
> 
> It was thus decided to create a new hook that would allocate and
> initialize a pristine state without any side effect:
> atomic_create_state to untangle a bit some of it, and to separate the
> initialization with the actual reset one might need during a
> suspend/resume.
> 
> Let's continue the transition to the new pattern with planes.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 41 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_mode_config.c         | 18 ++++++++++++++
>  include/drm/drm_atomic_state_helper.h     |  4 +++
>  include/drm/drm_plane.h                   | 13 ++++++++++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index a1abf4247c348eca21da348c3893dd843b9ed391..898e168eeae1c3899121ffd13c79fae7803fcd2a 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -318,10 +318,29 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  
>  	plane->state = plane_state;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
>  
> +/**
> + * __drm_atomic_helper_plane_create_state - initializes plane state
> + * @plane: plane object
> + * @state: new state to initialize
> + *
> + * Initializes the newly allocated @state, usually required when
> + * initializing the drivers.
> + *
> + * @state is assumed to be zeroed.
> + *
> + * This is useful for drivers that subclass @drm_plane_state.
> + */
> +void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
> +					    struct drm_plane_state *state)
> +{
> +	__drm_atomic_helper_plane_reset(plane, state);
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_plane_create_state);
> +

This calls __drm_atomic_helper_plane_reset(), which does "plane->state =
plane_state". From the description I understood this would be without
side effects. Is that not a side effect? Or should this call
__drm_atomic_helper_plane_state_reset() instead?

Same thing for the next two patches too.

>  /**
>   * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for planes
>   * @plane: drm plane
>   *
>   * Resets the atomic state for @plane by freeing the state pointer (which might
> @@ -337,10 +356,32 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
>  	if (plane->state)
>  		__drm_atomic_helper_plane_reset(plane, plane->state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
>  
> +/**
> + * drm_atomic_helper_plane_create_state - default &drm_plane_funcs.atomic_create_state hook for planes
> + * @plane: plane object
> + *
> + * Initializes a pristine @drm_plane_state.
> + *
> + * This is useful for drivers that don't subclass @drm_plane_state.
> + */
> +struct drm_plane_state *drm_atomic_helper_plane_create_state(struct drm_plane *plane)
> +{
> +	struct drm_plane_state *state;
> +
> +	state = kzalloc_obj(*state);
> +	if (!state)
> +		return ERR_PTR(-ENOMEM);
> +
> +	__drm_atomic_helper_plane_reset(plane, state);
> +
> +	return state;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_plane_create_state);
> +
>  /**
>   * __drm_atomic_helper_plane_duplicate_state - copy atomic plane state
>   * @plane: plane object
>   * @state: atomic plane state
>   *
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 54c27376f9894ef5eee378bb5b1e5fc7049de922..43fe64540b8ec4df4bb823d96db607e6bfe58824 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -180,10 +180,26 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>  	drm_connector_list_iter_end(&conn_iter);
>  
>  	return ret;
>  }
>  
> +static int drm_mode_config_plane_create_state(struct drm_plane *plane)
> +{
> +	struct drm_plane_state *plane_state;
> +
> +	if (!plane->funcs->atomic_create_state)
> +		return 0;
> +
> +	plane_state = plane->funcs->atomic_create_state(plane);
> +	if (IS_ERR(plane_state))
> +		return PTR_ERR(plane_state);
> +
> +	plane->state = plane_state;

And this one sets the plane->state, so maybe .atomic_create_state() is
not supposed to do that.

> +
> +	return 0;
> +}
> +
>  /**
>   * drm_mode_config_reset - call ->reset callbacks
>   * @dev: drm device
>   *
>   * This functions calls all the crtc's, encoder's and connector's ->reset
> @@ -207,10 +223,12 @@ void drm_mode_config_reset(struct drm_device *dev)
>  		drm_colorop_reset(colorop);
>  
>  	drm_for_each_plane(plane, dev)
>  		if (plane->funcs->reset)
>  			plane->funcs->reset(plane);
> +		else if (plane->funcs->atomic_create_state)
> +			drm_mode_config_plane_create_state(plane);
>  
>  	drm_for_each_crtc(crtc, dev)
>  		if (crtc->funcs->reset)
>  			crtc->funcs->reset(crtc);
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 900672c6ea90ba9cb87e38a7c84225972aee43c5..f56a91fca20e778d8ccb1767301dc00783bf40f3 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -51,12 +51,16 @@ struct drm_crtc_state *
>  drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc);
>  void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
>  void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					  struct drm_crtc_state *state);
>  
> +void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
> +					    struct drm_plane_state *state);
>  void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
>  					   struct drm_plane *plane);
> +struct drm_plane_state *
> +drm_atomic_helper_plane_create_state(struct drm_plane *plane);
>  void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  				     struct drm_plane_state *state);
>  void drm_atomic_helper_plane_reset(struct drm_plane *plane);
>  void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  					       struct drm_plane_state *state);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 703ef4d1bbbcf084c43aa5e127d28691878061c4..4d4d511b681d50c17fbc593cce9f706d63e04a52 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -386,10 +386,23 @@ struct drm_plane_funcs {
>  	 * 0 on success or a negative error code on failure.
>  	 */
>  	int (*set_property)(struct drm_plane *plane,
>  			    struct drm_property *property, uint64_t val);
>  
> +	/**
> +	 * @atomic_create_state:
> +	 *
> +	 * Allocates a pristine, initialized, state for the plane object
> +	 * and returns it.
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * A new, pristine, plane state instance or an error pointer
> +	 * on failure.
> +	 */
> +	struct drm_plane_state *(*atomic_create_state)(struct drm_plane *plane);
> +
>  	/**
>  	 * @atomic_duplicate_state:
>  	 *
>  	 * Duplicate the current atomic state for this plane and return it.
>  	 * The core and helpers guarantee that any atomic state duplicated with
> 

Re: [PATCH 12/14] drm/tidss: Drop call to drm_mode_config_reset at probe time

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:07, Maxime Ripard wrote:
> Now that we have drm_mode_config_create_state() to create the initial
> state called as part of drm_dev_register(), we don't need to call
> drm_mode_config_reset() during the driver probe.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/tidss/tidss_kms.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index 8bb93194e5ac686050c47f986b8cb6063eae22d3..b4779c09a1bfa5be7010bcd7d4f1379362996d2e 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -285,11 +285,9 @@ int tidss_modeset_init(struct tidss_device *tidss)
>  
>  	ret = drm_vblank_init(ddev, tidss->num_crtcs);
>  	if (ret)
>  		return ret;
>  
> -	drm_mode_config_reset(ddev);
> -
>  	dev_dbg(tidss->dev, "%s done\n", __func__);
>  
>  	return 0;
>  }
> 

There's actually a bug in the upstream tidss driver: it has another
drm_mode_config_reset() call in tidss_drv.c. I have an unsent patch to
drop one of those (the one removed in this patch). Without that patch of
mine, both drm_mode_config_reset() calls should be dropped to actually
test the functionality with tidss.

 Tomi

Re: [PATCH 13/14] drm/tidss: Convert to atomic_create_state

[Tomi Valkeinen]

Hi,

On 10/03/2026 18:07, Maxime Ripard wrote:
> Our driver uses reset to create the various object states, but only
> calls the helper that allocate a new state. They are thus strictly
> equivalent to the new atomic_create_state helpers, so let's switch to
> these.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/tidss/tidss_crtc.c  | 17 +++++++----------
>  drivers/gpu/drm/tidss/tidss_plane.c |  2 +-
>  2 files changed, 8 insertions(+), 11 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index a31c21c5f855ac8a94089dd3908e2510193b7d67..66e3d161c60bc14b2982cff4cdd43030d4086798 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -355,24 +355,21 @@ static void tidss_crtc_destroy_state(struct drm_crtc *crtc,
>  
>  	__drm_atomic_helper_crtc_destroy_state(&tstate->base);
>  	kfree(tstate);
>  }
>  
> -static void tidss_crtc_reset(struct drm_crtc *crtc)
> +static struct drm_crtc_state *tidss_crtc_create_state(struct drm_crtc *crtc)
>  {
>  	struct tidss_crtc_state *tstate;
>  
> -	if (crtc->state)
> -		tidss_crtc_destroy_state(crtc, crtc->state);
> -
>  	tstate = kzalloc_obj(*tstate);
> -	if (!tstate) {
> -		crtc->state = NULL;
> -		return;
> -	}
> +	if (!tstate)
> +		return ERR_PTR(-ENOMEM);
>  
> -	__drm_atomic_helper_crtc_reset(crtc, &tstate->base);
> +	__drm_atomic_helper_crtc_create_state(crtc, &tstate->base);
> +
> +	return &tstate->base;
>  }
>  
>  static struct drm_crtc_state *tidss_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>  	struct tidss_crtc_state *state, *current_state;
> @@ -403,14 +400,14 @@ static void tidss_crtc_destroy(struct drm_crtc *crtc)
>  	drm_crtc_cleanup(crtc);
>  	kfree(tcrtc);
>  }
>  
>  static const struct drm_crtc_funcs tidss_crtc_funcs = {
> -	.reset = tidss_crtc_reset,
>  	.destroy = tidss_crtc_destroy,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> +	.atomic_create_state = tidss_crtc_create_state,
>  	.atomic_duplicate_state = tidss_crtc_duplicate_state,
>  	.atomic_destroy_state = tidss_crtc_destroy_state,
>  	.enable_vblank = tidss_crtc_enable_vblank,
>  	.disable_vblank = tidss_crtc_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
> index aaa02c851c595aa3781ec2e6741af1999092aa40..518498d4576528a0ec59fd03cf27a87b1b3f1e6e 100644
> --- a/drivers/gpu/drm/tidss/tidss_plane.c
> +++ b/drivers/gpu/drm/tidss/tidss_plane.c
> @@ -176,12 +176,12 @@ static const struct drm_plane_helper_funcs tidss_primary_plane_helper_funcs = {
>  };
>  
>  static const struct drm_plane_funcs tidss_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
> -	.reset = drm_atomic_helper_plane_reset,
>  	.destroy = drm_plane_destroy,
> +	.atomic_create_state = drm_atomic_helper_plane_create_state,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  };
>  
>  struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
> 

Re: [PATCH 05/14] drm/plane: Add new atomic_create_state callback

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3479 bytes --]

Hi,

On Wed, Mar 11, 2026 at 09:13:18AM +0200, Tomi Valkeinen wrote:
> On 10/03/2026 18:06, Maxime Ripard wrote:
> > Commit 47b5ac7daa46 ("drm/atomic: Add new atomic_create_state callback
> > to drm_private_obj") introduced a new pattern for allocating drm object
> > states.
> > 
> > Instead of relying on the reset() callback, it created a new
> > atomic_create_state hook. This is helpful because reset is a bit
> > overloaded: it's used to create the initial software tate, reset it, but
> > also reset the hardware.
> > 
> > It can also be used either at probe time, to create the initial state
> > and possibly reset the hardware to an expected default, but also during
> > suspend/resume.
> > 
> > Both these cases come with different expectations too: during the
> > initialization, we want to initialize all states, but during
> > suspend/resume, drm_private_states for example are expected to be kept
> > around.
> > 
> > And reset() isn't fallible, which makes it harder to handle
> > initialization errors properly.
> > 
> > And this is only really relevant for some drivers, since all the helpers
> > for reset only create a new state, and don't touch the hardware at all.
> > 
> > It was thus decided to create a new hook that would allocate and
> > initialize a pristine state without any side effect:
> > atomic_create_state to untangle a bit some of it, and to separate the
> > initialization with the actual reset one might need during a
> > suspend/resume.
> > 
> > Let's continue the transition to the new pattern with planes.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/drm_atomic_state_helper.c | 41 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_mode_config.c         | 18 ++++++++++++++
> >  include/drm/drm_atomic_state_helper.h     |  4 +++
> >  include/drm/drm_plane.h                   | 13 ++++++++++
> >  4 files changed, 76 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index a1abf4247c348eca21da348c3893dd843b9ed391..898e168eeae1c3899121ffd13c79fae7803fcd2a 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -318,10 +318,29 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  
> >  	plane->state = plane_state;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
> >  
> > +/**
> > + * __drm_atomic_helper_plane_create_state - initializes plane state
> > + * @plane: plane object
> > + * @state: new state to initialize
> > + *
> > + * Initializes the newly allocated @state, usually required when
> > + * initializing the drivers.
> > + *
> > + * @state is assumed to be zeroed.
> > + *
> > + * This is useful for drivers that subclass @drm_plane_state.
> > + */
> > +void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
> > +					    struct drm_plane_state *state)
> > +{
> > +	__drm_atomic_helper_plane_reset(plane, state);
> > +}
> > +EXPORT_SYMBOL(__drm_atomic_helper_plane_create_state);
> > +
> 
> This calls __drm_atomic_helper_plane_reset(), which does "plane->state =
> plane_state". From the description I understood this would be without
> side effects. Is that not a side effect? Or should this call
> __drm_atomic_helper_plane_state_reset() instead?

Yeah, this was supposed to be the latter. Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 12/14] drm/tidss: Drop call to drm_mode_config_reset at probe time

[Tomi Valkeinen]

Hi,

On 11/03/2026 09:40, Tomi Valkeinen wrote:
> Hi,
> 
> On 10/03/2026 18:07, Maxime Ripard wrote:
>> Now that we have drm_mode_config_create_state() to create the initial
>> state called as part of drm_dev_register(), we don't need to call
>> drm_mode_config_reset() during the driver probe.
>>
>> Signed-off-by: Maxime Ripard <mripard@kernel.org>
>> ---
>>  drivers/gpu/drm/tidss/tidss_kms.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
>> index 8bb93194e5ac686050c47f986b8cb6063eae22d3..b4779c09a1bfa5be7010bcd7d4f1379362996d2e 100644
>> --- a/drivers/gpu/drm/tidss/tidss_kms.c
>> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
>> @@ -285,11 +285,9 @@ int tidss_modeset_init(struct tidss_device *tidss)
>>  
>>  	ret = drm_vblank_init(ddev, tidss->num_crtcs);
>>  	if (ret)
>>  		return ret;
>>  
>> -	drm_mode_config_reset(ddev);
>> -
>>  	dev_dbg(tidss->dev, "%s done\n", __func__);
>>  
>>  	return 0;
>>  }
>>
> 
> There's actually a bug in the upstream tidss driver: it has another
> drm_mode_config_reset() call in tidss_drv.c. I have an unsent patch to
> drop one of those (the one removed in this patch). Without that patch of
> mine, both drm_mode_config_reset() calls should be dropped to actually
> test the functionality with tidss.
I sent the fix:

https://lore.kernel.org/all/20260311-tidss-minor-fixes-v1-0-ee5e6e14a566%40ideasonboard.com/

 Tomi

Re: [PATCH 01/14] drm/atomic: Document atomic state lifetime

[Laurent Pinchart]

On Wed, Mar 11, 2026 at 08:44:24AM +0200, Tomi Valkeinen wrote:
> On 10/03/2026 18:06, Maxime Ripard wrote:
> > How drm_atomic_state structures and the various entity structures are
> > allocated and freed isn't really trivial, so let's document it.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  Documentation/gpu/drm-kms.rst |  6 +++++
> >  drivers/gpu/drm/drm_atomic.c  | 52 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 2292e65f044c3bdebafbb8f83dfe7ac12e831273..017c7b196ed7ead4cf5fa8572e1f977d9e00dda8 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -280,10 +280,16 @@ structure, ordering of committing state changes to hardware is sequenced using
> >  :c:type:`struct drm_crtc_commit <drm_crtc_commit>`.
> >  
> >  Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed
> >  coverage of specific topics.
> >  
> > +Atomic State Lifetime
> > +---------------------
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> > +   :doc: state lifetime
> > +
> >  Handling Driver Private State
> >  -----------------------------
> >  
> >  .. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> >     :doc: handling driver private state
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 4283ab4d06c581727cc98b1dc870bf69691ea654..92c6afc8f22c8307a59dc266aacdb8e03351409d 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -45,10 +45,62 @@
> >  #include <drm/drm_colorop.h>
> >  
> >  #include "drm_crtc_internal.h"
> >  #include "drm_internal.h"
> >  
> > +/**
> > + * DOC: state lifetime
> > + *
> > + * &struct drm_atomic_state represents an update to video pipeline
> > + * state. Despite its confusing name, it's actually a transient object

I wonder if we'll rename it one day :-)

> > + * that holds a state update as a collection of pointer to individual
> > + * objects states. &struct drm_atomic_state has a much shorter lifetime
> 
> Hmm, I think "a collection of pointers to individual object states". Hmm
> or "objects' states"? I like the former.
> 
> > + * than the objects states, since it's only allocated while preparing,
> 
> "objects' states" or "object states".
> 
> > + * checking or doing the update, while object states are allocated while

Maybe s/doing/committing/ if you want to use KMS terms.

> > + * the state will be, or is active in the hardware.

The second part sounds weird. I'd write

"while object states are allocated when preparing the update and kept
alive as long as they are active in the hardware."

or something similar. Writing "device" instead of "hardware" could also
be better.

> > + *
> > + * Their respective lifetimes are:
> > + *
> > + * - at reset time, the object reset implementation will allocate a new,
> > + *   default, state and will store it in the object state pointer.

s/default,/default/

> 
> "object's". This is the "active state", is it?
> 
> > + *
> > + * - whenever a new update is needed:
> > + *
> > + *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().

The first part doesn't use first person pronouns, you may want to be
consistent across the whole text and use a descriptive style here too.

> > + *
> > + *   + we copy the state of each affected entity into our &struct
> > + *     drm_atomic_state using drm_atomic_get_plane_state(),
> > + *     drm_atomic_get_crtc_state(), drm_atomic_get_connector_state(), or
> > + *     drm_atomic_get_private_obj_state(). That state can then be
> > + *     modified.
> 
> Maybe clarify what is the state returned by these. It's the "active
> state", isn't it, drm_crtc.state or similar?

Yes, it's not very clear. The text should describe whether the
drm_atomic_state just points to the active state of the entities, or
duplicates them. "copy the state" is ambiguous.

> > + *
> > + *     At that point, &struct drm_atomic_state stores three state
> > + *     pointers for that particular entity: the old, new, and existing

s/that particular/any affected/

> > + *     (called "state") states. The old state is the state currently
> > + *     active in the hardware, which is either the one initialized by
> > + *     reset() or a newer one if a commit has been made. The new state
> > + *     is the state we just allocated and we might eventually commit to
> > + *     the hardware. The existing state points to the state we'll
> > + *     eventually have to free when the drm_atomic_state will be
> > + *     destroyed, but points to the new state for now.

s/but/and/

> From this, I don't understand the difference between the old state and
> the existing state. And if the existing state is the one we'll free,
> isn't that the old state, not new state? Oh, is the existing state a
> state we have to free when the drm_atomic_state would is freed? And at
> this point the new state is the one, as it's not committed?
> 
> > + *
> > + *   + After the state is populated, it is checked. If the check is
> > + *     successful, the update is committed. Part of the commit is a call
> > + *     to drm_atomic_helper_swap_state() which will turn the new states
> > + *     into the active states. Doing so involves updating the objects
> 
> "object's"
> 
> > + *     state pointer (&drm_crtc.state or similar) to point to the new
> > + *     state, and the existing states will now point to the old states,
> > + *     that used to be active but isn't anymore.
> 
> "aren't"
> 
> I think I understand this, but... It kind of brings in a new state
> concept, "active state".
> 
> > + *
> > + *   + When the commit is done, and when all references to our &struct
> > + *     drm_atomic_state are put, drm_atomic_state_clear() runs and will
> > + *     free all the old states.

Technically you're freeing the "existing" state per your nomenclature
above, which points to the old state. The word "existing" seems to make
things harder to describe, there may be an opportunity for better
vocabulary.

> > + *
> > + *   + Now, we don't have any active &struct drm_atomic_state anymore,
> > + *     and only the entity active states remain allocated.
> > + */
> > +
> 
> Even if this is a bit hard to read, I think it really clarifies the
> state lifetime.

It's certainly a useful addition to the documentation.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/14] drm/atomic: Drop drm_private_state.obj assignment from create_state

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:06:54PM +0100, Maxime Ripard wrote:
> The initial intent of the atomic_create_state helper was to simply
> allocate a proper drm_private_state and returning it, without any side
> effect.
> 
> However, the __drm_atomic_helper_private_obj_create_state() introduces a
> side effect by setting the drm_private_obj.state to the newly allocated
> state.
> 
> This assignment defeats the purpose, but is also redundant since
> the only caller, drm_atomic_private_obj_init(), will also set this
> pointer to the newly allocated state.

__drm_atomic_helper_bridge_reset() is called in many places. Most of
them are .atomic_create_state() handlers, called from
drm_atomic_private_obj_init() only. The only exception is
__drm_atomic_helper_bridge_reset(). Are you sure this patch won't cause
any negative side effect there ? The commit message should explain why.

> 
> Let's drop the assignment in __drm_atomic_helper_private_obj_create_state().
> 
> Fixes: e7be39ed1716 ("drm/atomic-helper: Add private_obj atomic_create_state helper")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index bd6faa09f83b498b1417869493f31d876cd13914..323abc9926e084ad595768c06d5c5ee28c22c014 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -728,12 +728,10 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>  void __drm_atomic_helper_private_obj_create_state(struct drm_private_obj *obj,
>  						  struct drm_private_state *state)
>  {
>  	if (state)
>  		state->obj = obj;
> -
> -	obj->state = state;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_private_obj_create_state);
>  
>  /**
>   * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 03/14] drm/mode-config: Mention drm_mode_config_reset() culprits

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:06:55PM +0100, Maxime Ripard wrote:
> drm_mode_config_reset() has the expectation that drm_private_states are
> not reset.

I initially misinterpreted that. I don't think it's an "expectation".
The function simply does not reset the private objects states. If you
want to keep "expectation", you can write

drm_mode_config_reset() has the expectation that drm_private_states do
not need to be reset.

> This is especially significant for the DP MST and tunneling code that
> expect to be preserved across a suspend/resume cycle, where
> drm_mode_config_reset() is also used.
> 
> Let's document this expectation.
> 
> Link: https://lore.kernel.org/dri-devel/aOaQLx-7EpsHRwkH@ideak-desk/
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_mode_config.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 84ae8a23a36786705fea7c0eafd7f20813b7c8f9..54c27376f9894ef5eee378bb5b1e5fc7049de922 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -187,10 +187,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>   * @dev: drm device
>   *
>   * This functions calls all the crtc's, encoder's and connector's ->reset
>   * callback. Drivers can use this in e.g. their driver load or resume code to
>   * reset hardware and software state.
> + *
> + * It's worth noting that @drm_private_obj structures are expected to be

s/It's worth noting/Note/

> + * stable across suspend/resume cycles, and @drm_mode_config_reset()
> + * will not affect these structures.

s/will/does/

With this addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>   */
>  void drm_mode_config_reset(struct drm_device *dev)
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_colorop *colorop;
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/14] drm/atomic-state-helper: Fix __drm_atomic_helper_plane_reset() doc typo

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:06:56PM +0100, Maxime Ripard wrote:
> A typo has slipped through in the __drm_atomic_helper_plane_reset()
> documentation, probably due to copy and paste. It will not assign
> drm_crtc state pointer, but rather the drm_plane's.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 323abc9926e084ad595768c06d5c5ee28c22c014..a1abf4247c348eca21da348c3893dd843b9ed391 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -302,11 +302,11 @@ EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>   * __drm_atomic_helper_plane_reset - reset state on plane
>   * @plane: drm plane
>   * @plane_state: plane state to assign
>   *
>   * Initializes the newly allocated @plane_state and assigns it to
> - * the &drm_crtc->state pointer of @plane, usually required when
> + * the &drm_plane->state pointer of @plane, usually required when

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>   * initializing the drivers or when called from the &drm_plane_funcs.reset
>   * hook.
>   *
>   * This is useful for drivers that subclass the plane state.
>   */
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/14] drm/plane: Add new atomic_create_state callback

[Laurent Pinchart]

On Wed, Mar 11, 2026 at 09:13:18AM +0200, Tomi Valkeinen wrote:
> On 10/03/2026 18:06, Maxime Ripard wrote:
> > Commit 47b5ac7daa46 ("drm/atomic: Add new atomic_create_state callback
> > to drm_private_obj") introduced a new pattern for allocating drm object
> > states.
> > 
> > Instead of relying on the reset() callback, it created a new
> > atomic_create_state hook. This is helpful because reset is a bit
> > overloaded: it's used to create the initial software tate, reset it, but

s/tate/state/

> > also reset the hardware.
> > 
> > It can also be used either at probe time, to create the initial state
> > and possibly reset the hardware to an expected default, but also during
> > suspend/resume.
> > 
> > Both these cases come with different expectations too: during the
> > initialization, we want to initialize all states, but during
> > suspend/resume, drm_private_states for example are expected to be kept
> > around.
> > 
> > And reset() isn't fallible, which makes it harder to handle
> > initialization errors properly.
> > 
> > And this is only really relevant for some drivers, since all the helpers
> > for reset only create a new state, and don't touch the hardware at all.
> > 
> > It was thus decided to create a new hook that would allocate and
> > initialize a pristine state without any side effect:
> > atomic_create_state to untangle a bit some of it, and to separate the
> > initialization with the actual reset one might need during a
> > suspend/resume.
> > 
> > Let's continue the transition to the new pattern with planes.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/drm_atomic_state_helper.c | 41 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_mode_config.c         | 18 ++++++++++++++
> >  include/drm/drm_atomic_state_helper.h     |  4 +++
> >  include/drm/drm_plane.h                   | 13 ++++++++++
> >  4 files changed, 76 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index a1abf4247c348eca21da348c3893dd843b9ed391..898e168eeae1c3899121ffd13c79fae7803fcd2a 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -318,10 +318,29 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  
> >  	plane->state = plane_state;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
> >  
> > +/**
> > + * __drm_atomic_helper_plane_create_state - initializes plane state
> > + * @plane: plane object
> > + * @state: new state to initialize
> > + *
> > + * Initializes the newly allocated @state, usually required when
> > + * initializing the drivers.
> > + *
> > + * @state is assumed to be zeroed.
> > + *
> > + * This is useful for drivers that subclass @drm_plane_state.
> > + */
> > +void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
> > +					    struct drm_plane_state *state)
> > +{
> > +	__drm_atomic_helper_plane_reset(plane, state);
> > +}
> > +EXPORT_SYMBOL(__drm_atomic_helper_plane_create_state);
> > +
> 
> This calls __drm_atomic_helper_plane_reset(), which does "plane->state =
> plane_state". From the description I understood this would be without
> side effects. Is that not a side effect? Or should this call
> __drm_atomic_helper_plane_state_reset() instead?

If you want to dedicate the word "reset" to code paths that reset the
hardware or initialize the state based on the hardware state, I would
also rename __drm_atomic_helper_plane_state_reset() to
__drm_atomic_helper_plane_state_init(), as its purpose is to initialize
a new state.

> Same thing for the next two patches too.
> 
> >  /**
> >   * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for planes
> >   * @plane: drm plane
> >   *
> >   * Resets the atomic state for @plane by freeing the state pointer (which might
> > @@ -337,10 +356,32 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
> >  	if (plane->state)
> >  		__drm_atomic_helper_plane_reset(plane, plane->state);
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
> >  
> > +/**
> > + * drm_atomic_helper_plane_create_state - default &drm_plane_funcs.atomic_create_state hook for planes
> > + * @plane: plane object
> > + *
> > + * Initializes a pristine @drm_plane_state.
> > + *
> > + * This is useful for drivers that don't subclass @drm_plane_state.
> > + */
> > +struct drm_plane_state *drm_atomic_helper_plane_create_state(struct drm_plane *plane)
> > +{
> > +	struct drm_plane_state *state;
> > +
> > +	state = kzalloc_obj(*state);
> > +	if (!state)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	__drm_atomic_helper_plane_reset(plane, state);
> > +
> > +	return state;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_plane_create_state);
> > +
> >  /**
> >   * __drm_atomic_helper_plane_duplicate_state - copy atomic plane state
> >   * @plane: plane object
> >   * @state: atomic plane state
> >   *
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 54c27376f9894ef5eee378bb5b1e5fc7049de922..43fe64540b8ec4df4bb823d96db607e6bfe58824 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -180,10 +180,26 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> >  	drm_connector_list_iter_end(&conn_iter);
> >  
> >  	return ret;
> >  }
> >  
> > +static int drm_mode_config_plane_create_state(struct drm_plane *plane)
> > +{
> > +	struct drm_plane_state *plane_state;
> > +
> > +	if (!plane->funcs->atomic_create_state)
> > +		return 0;
> > +
> > +	plane_state = plane->funcs->atomic_create_state(plane);
> > +	if (IS_ERR(plane_state))
> > +		return PTR_ERR(plane_state);
> > +
> > +	plane->state = plane_state;
> 
> And this one sets the plane->state, so maybe .atomic_create_state() is
> not supposed to do that.
> 
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * drm_mode_config_reset - call ->reset callbacks
> >   * @dev: drm device
> >   *
> >   * This functions calls all the crtc's, encoder's and connector's ->reset
> > @@ -207,10 +223,12 @@ void drm_mode_config_reset(struct drm_device *dev)
> >  		drm_colorop_reset(colorop);
> >  
> >  	drm_for_each_plane(plane, dev)
> >  		if (plane->funcs->reset)
> >  			plane->funcs->reset(plane);
> > +		else if (plane->funcs->atomic_create_state)
> > +			drm_mode_config_plane_create_state(plane);

While at it I'd add curly braces for the loop.

> >  
> >  	drm_for_each_crtc(crtc, dev)
> >  		if (crtc->funcs->reset)
> >  			crtc->funcs->reset(crtc);
> >  
> > diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> > index 900672c6ea90ba9cb87e38a7c84225972aee43c5..f56a91fca20e778d8ccb1767301dc00783bf40f3 100644
> > --- a/include/drm/drm_atomic_state_helper.h
> > +++ b/include/drm/drm_atomic_state_helper.h
> > @@ -51,12 +51,16 @@ struct drm_crtc_state *
> >  drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc);
> >  void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
> >  void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> >  					  struct drm_crtc_state *state);
> >  
> > +void __drm_atomic_helper_plane_create_state(struct drm_plane *plane,
> > +					    struct drm_plane_state *state);
> >  void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> >  					   struct drm_plane *plane);
> > +struct drm_plane_state *
> > +drm_atomic_helper_plane_create_state(struct drm_plane *plane);
> >  void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  				     struct drm_plane_state *state);
> >  void drm_atomic_helper_plane_reset(struct drm_plane *plane);
> >  void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> >  					       struct drm_plane_state *state);
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index 703ef4d1bbbcf084c43aa5e127d28691878061c4..4d4d511b681d50c17fbc593cce9f706d63e04a52 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -386,10 +386,23 @@ struct drm_plane_funcs {
> >  	 * 0 on success or a negative error code on failure.
> >  	 */
> >  	int (*set_property)(struct drm_plane *plane,
> >  			    struct drm_property *property, uint64_t val);
> >  
> > +	/**
> > +	 * @atomic_create_state:
> > +	 *
> > +	 * Allocates a pristine, initialized, state for the plane object
> > +	 * and returns it.
> > +	 *
> > +	 * RETURNS:
> > +	 *
> > +	 * A new, pristine, plane state instance or an error pointer
> > +	 * on failure.
> > +	 */
> > +	struct drm_plane_state *(*atomic_create_state)(struct drm_plane *plane);
> > +
> >  	/**
> >  	 * @atomic_duplicate_state:
> >  	 *
> >  	 * Duplicate the current atomic state for this plane and return it.
> >  	 * The core and helpers guarantee that any atomic state duplicated with
> > 
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 08/14] drm/mode-config: Create drm_mode_config_create_state()

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:07:00PM +0100, Maxime Ripard wrote:
> drm_mode_config_reset() can be used to create the initial state, but
> also to return to the initial state, when doing a suspend/resume cycle
> for example.
> 
> It also affects both the software and the hardware, and drivers can
> either choose to reset the hardware as well. Most will just create an

"either" requires an "or".

> empty state and the synchronisation between hardware and software states
> will effectively be done when the first commit is done.
> 
> That dual role can be harmful, since some objects do need to be
> initialized but also need to be preserved across a suspend/resume cycle.
> drm_private_obj are such objects for example.
> 
> Thus, let's create another helper for drivers to call to initialize
> their state when the driver is loaded, so we can make
> drm_mode_config_reset() only about handling suspend/resume and similar.

Does that mean that the state initialization at probe time from the
hardware state is planned to be handled by .atomic_create_state() ?

> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic.c      | 12 +++++-
>  drivers/gpu/drm/drm_mode_config.c | 83 +++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  1 +
>  3 files changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 92c6afc8f22c8307a59dc266aacdb8e03351409d..d1885f895cce78725419b6291f4dbe5563e3b240 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -58,12 +58,20 @@
>   * checking or doing the update, while object states are allocated while
>   * the state will be, or is active in the hardware.
>   *
>   * Their respective lifetimes are:
>   *
> - * - at reset time, the object reset implementation will allocate a new,
> - *   default, state and will store it in the object state pointer.
> + * - at driver initialization time, the driver will allocate an initial,
> + *   pristine, state and will store it using
> + *   drm_mode_config_create_state(). Historically, this was one of
> + *   drm_mode_config_reset() job, so one might still encounter it in a
> + *   driver.

Do you have plans to port drivers to the new API, to drop this last
sentence sooner than later ?

> + *
> + * - at reset time, for example during suspend/resume,
> + *   drm_mode_config_reset() will reset the software and hardware state
> + *   to a known default and will store it in the object state pointer.
> + *   Not all objects are affected by drm_mode_config_reset() though.

I have a bit of trouble understanding the design. How can
drm_mode_config_reset() decide which objects should have their state
reset (i.e. all objects but private objects in the current
implementation), shouldn't that be a driver decision ?

>   *
>   * - whenever a new update is needed:
>   *
>   *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().
>   *
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 95759a4b5176ff17032a9a267ec9de6980be0abc..1a84ac063d381014eb8afd7116e1e8b7a8fc92ca 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -21,10 +21,11 @@
>   */
>  
>  #include <linux/export.h>
>  #include <linux/uaccess.h>
>  
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_framebuffer.h>
>  #include <drm/drm_managed.h>
> @@ -278,10 +279,92 @@ void drm_mode_config_reset(struct drm_device *dev)
>  			drm_mode_config_connector_create_state(connector);
>  	drm_connector_list_iter_end(&conn_iter);
>  }
>  EXPORT_SYMBOL(drm_mode_config_reset);
>  
> +/**
> + * drm_mode_config_create_state - Allocates the initial state
> + * @dev: drm device
> + *
> + * This functions creates the initial state for all the objects. Drivers
> + * can use this in e.g. their driver load to initialize its software

I think you meant s/its/their/

> + * state.
> + *
> + * It has two main differences with drm_mode_config_reset(): the reset()
> + * hooks aren't called and thus the hardware will be left untouched, but
> + * also the @drm_private_obj structures will be initialized as opposed
> + * to drm_mode_config_reset() that skips them.
> + *
> + * Returns: 0 on success, negative error value on failure.
> + */
> +int drm_mode_config_create_state(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_colorop *colorop;
> +	struct drm_plane *plane;
> +	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +	struct drm_private_obj *privobj;
> +	int ret;
> +
> +	drm_for_each_privobj(privobj, dev) {
> +		struct drm_private_state *privobj_state;
> +
> +		if (privobj->state)
> +			continue;
> +
> +		if (!privobj->funcs->atomic_create_state)
> +			continue;
> +
> +		privobj_state = privobj->funcs->atomic_create_state(privobj);
> +		if (IS_ERR(privobj_state))
> +			return PTR_ERR(privobj_state);
> +
> +		privobj->state = privobj_state;

Either a helper function is missing, or you forgot to call it.

> +	}
> +
> +	drm_for_each_colorop(colorop, dev) {
> +		if (colorop->state)
> +			continue;
> +
> +		// TODO: Implement atomic_create_state for colorop.

Oops :-)

> +		drm_colorop_reset(colorop);
> +	}
> +
> +	drm_for_each_plane(plane, dev) {
> +		if (plane->state)
> +			continue;
> +
> +		ret = drm_mode_config_plane_create_state(plane);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	drm_for_each_crtc(crtc, dev) {
> +		if (crtc->state)
> +			continue;
> +
> +		ret = drm_mode_config_crtc_create_state(crtc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		if (connector->state)
> +			continue;
> +
> +		ret = drm_mode_config_connector_create_state(connector);
> +		if (ret)
> +			return ret;
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_config_create_state);
> +
>  /*
>   * Global properties
>   */
>  static const struct drm_prop_enum_list drm_plane_type_enum_list[] = {
>  	{ DRM_PLANE_TYPE_OVERLAY, "Overlay" },
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 5e1dd0cfccde2d2bc00a09e98e4adc65833dad08..428ae75b7c4f193dfbd49c15a2f2ef32209a6cef 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -1000,9 +1000,10 @@ int __must_check drmm_mode_config_init(struct drm_device *dev);
>  static inline int drm_mode_config_init(struct drm_device *dev)
>  {
>  	return drmm_mode_config_init(dev);
>  }
>  
> +int drm_mode_config_create_state(struct drm_device *dev);
>  void drm_mode_config_reset(struct drm_device *dev);
>  void drm_mode_config_cleanup(struct drm_device *dev);
>  
>  #endif
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 09/14] drm/drv: Call drm_mode_config_create_state() by default

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:07:01PM +0100, Maxime Ripard wrote:
> Almost all drivers, and our documented skeleton, call
> drm_mode_config_reset() prior to calling drm_dev_register() to
> initialize its DRM object states.
> 
> Now that we have drm_mode_config_create_state() to create that initial
> state if it doesn't exist, we can call it directly in
> drm_dev_register(). That way, we know that the initial atomic state will
> always be allocated without any boilerplate.

Should most drivers now stop calling drm_mode_config_reset() at probe
time ?

> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 2915118436ce8a6640cfb0c59936031990727ed1..820106d56ab399a39cac56d98662b5ddbcae8ded 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1097,10 +1097,14 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		ret = drm_modeset_register_all(dev);
>  		if (ret)
>  			goto err_unload;
> +
> +		ret = drm_mode_config_create_state(dev);
> +		if (ret)
> +			goto err_unload;
>  	}
>  	drm_panic_register(dev);
>  	drm_client_sysrq_register(dev);
>  
>  	DRM_INFO("Initialized %s %d.%d.%d for %s on minor %d\n",

-- 
Regards,

Laurent Pinchart

Re: [PATCH 10/14] drm/atomic: Drop private obj state allocation

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:07:02PM +0100, Maxime Ripard wrote:
> Now that drm_dev_register() calls drm_mode_config_create_state() for
> every modeset driver, the private obj states will be initialized at
> driver registration automatically if they haven't already.
> 
> Thus, the explicit initial allocation we have in
> drm_atomic_private_obj_init() is now redundant, and we can remove it.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_atomic.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d1885f895cce78725419b6291f4dbe5563e3b240..e8dc019364d27123c3a682a10323fba9ff76f9df 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -991,25 +991,18 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>   */
>  int drm_atomic_private_obj_init(struct drm_device *dev,
>  				struct drm_private_obj *obj,
>  				const struct drm_private_state_funcs *funcs)
>  {
> -	struct drm_private_state *state;
>  	memset(obj, 0, sizeof(*obj));
>  
>  	drm_modeset_lock_init(&obj->lock);
>  
>  	obj->dev = dev;
>  	obj->funcs = funcs;
>  	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
>  
> -	state = obj->funcs->atomic_create_state(obj);
> -	if (IS_ERR(state))
> -		return PTR_ERR(state);
> -
> -	obj->state = state;
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_private_obj_init);
>  
>  /**
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 13/14] drm/tidss: Convert to atomic_create_state

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:07:05PM +0100, Maxime Ripard wrote:
> Our driver uses reset to create the various object states, but only
> calls the helper that allocate a new state. They are thus strictly
> equivalent to the new atomic_create_state helpers, so let's switch to
> these.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/tidss/tidss_crtc.c  | 17 +++++++----------
>  drivers/gpu/drm/tidss/tidss_plane.c |  2 +-
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index a31c21c5f855ac8a94089dd3908e2510193b7d67..66e3d161c60bc14b2982cff4cdd43030d4086798 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -355,24 +355,21 @@ static void tidss_crtc_destroy_state(struct drm_crtc *crtc,
>  
>  	__drm_atomic_helper_crtc_destroy_state(&tstate->base);
>  	kfree(tstate);
>  }
>  
> -static void tidss_crtc_reset(struct drm_crtc *crtc)
> +static struct drm_crtc_state *tidss_crtc_create_state(struct drm_crtc *crtc)
>  {
>  	struct tidss_crtc_state *tstate;
>  
> -	if (crtc->state)
> -		tidss_crtc_destroy_state(crtc, crtc->state);
> -
>  	tstate = kzalloc_obj(*tstate);
> -	if (!tstate) {
> -		crtc->state = NULL;
> -		return;
> -	}
> +	if (!tstate)
> +		return ERR_PTR(-ENOMEM);
>  
> -	__drm_atomic_helper_crtc_reset(crtc, &tstate->base);
> +	__drm_atomic_helper_crtc_create_state(crtc, &tstate->base);
> +
> +	return &tstate->base;
>  }
>  
>  static struct drm_crtc_state *tidss_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>  	struct tidss_crtc_state *state, *current_state;
> @@ -403,14 +400,14 @@ static void tidss_crtc_destroy(struct drm_crtc *crtc)
>  	drm_crtc_cleanup(crtc);
>  	kfree(tcrtc);
>  }
>  
>  static const struct drm_crtc_funcs tidss_crtc_funcs = {
> -	.reset = tidss_crtc_reset,
>  	.destroy = tidss_crtc_destroy,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> +	.atomic_create_state = tidss_crtc_create_state,
>  	.atomic_duplicate_state = tidss_crtc_duplicate_state,
>  	.atomic_destroy_state = tidss_crtc_destroy_state,
>  	.enable_vblank = tidss_crtc_enable_vblank,
>  	.disable_vblank = tidss_crtc_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
> index aaa02c851c595aa3781ec2e6741af1999092aa40..518498d4576528a0ec59fd03cf27a87b1b3f1e6e 100644
> --- a/drivers/gpu/drm/tidss/tidss_plane.c
> +++ b/drivers/gpu/drm/tidss/tidss_plane.c
> @@ -176,12 +176,12 @@ static const struct drm_plane_helper_funcs tidss_primary_plane_helper_funcs = {
>  };
>  
>  static const struct drm_plane_funcs tidss_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
> -	.reset = drm_atomic_helper_plane_reset,
>  	.destroy = drm_plane_destroy,
> +	.atomic_create_state = drm_atomic_helper_plane_create_state,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  };
>  
>  struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 14/14] drm/bridge_connector: Convert to atomic_create_state

[Laurent Pinchart]

On Tue, Mar 10, 2026 at 05:07:06PM +0100, Maxime Ripard wrote:
> The connector created by drm_bridge_connector only initializes a
> pristine state in reset, which is equivalent to that atomic_create_state
> would expect. Let's convert to it.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/display/drm_bridge_connector.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index f686aa5c0ed9b84dbe5e0957df22d08aff2f1945..2f73576783f5f69ebce277a7537accefc94645a9 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -263,26 +263,33 @@ static void drm_bridge_connector_debugfs_init(struct drm_connector *connector,
>  		if (bridge->funcs->debugfs_init)
>  			bridge->funcs->debugfs_init(bridge, root);
>  	}
>  }
>  
> -static void drm_bridge_connector_reset(struct drm_connector *connector)
> +static struct drm_connector_state *
> +drm_bridge_connector_create_state(struct drm_connector *connector)
>  {
>  	struct drm_bridge_connector *bridge_connector =
>  		to_drm_bridge_connector(connector);
> +	struct drm_connector_state *conn_state;
> +
> +	conn_state = drm_atomic_helper_connector_create_state(connector);
> +	if (IS_ERR(conn_state))
> +		return conn_state;
>  
> -	drm_atomic_helper_connector_reset(connector);
>  	if (bridge_connector->bridge_hdmi)
>  		__drm_atomic_helper_connector_hdmi_reset(connector,

It would be good to rename this function too, to avoid using "reset".

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> -							 connector->state);
> +							 conn_state);
> +
> +	return conn_state;
>  }
>  
>  static const struct drm_connector_funcs drm_bridge_connector_funcs = {
> -	.reset = drm_bridge_connector_reset,
>  	.detect = drm_bridge_connector_detect,
>  	.force = drm_bridge_connector_force,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.atomic_create_state = drm_bridge_connector_create_state,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.debugfs_init = drm_bridge_connector_debugfs_init,
>  	.oob_hotplug_event = drm_bridge_connector_oob_hotplug_event,
>  };
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/14] drm/atomic: Drop drm_private_state.obj assignment from create_state

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

Hi Laurent,

Thanks for your review

On Mon, Mar 16, 2026 at 05:49:13PM +0200, Laurent Pinchart wrote:
> On Tue, Mar 10, 2026 at 05:06:54PM +0100, Maxime Ripard wrote:
> > The initial intent of the atomic_create_state helper was to simply
> > allocate a proper drm_private_state and returning it, without any side
> > effect.
> > 
> > However, the __drm_atomic_helper_private_obj_create_state() introduces a
> > side effect by setting the drm_private_obj.state to the newly allocated
> > state.
> > 
> > This assignment defeats the purpose, but is also redundant since
> > the only caller, drm_atomic_private_obj_init(), will also set this
> > pointer to the newly allocated state.
> 
> __drm_atomic_helper_bridge_reset() is called in many places. Most of
> them are .atomic_create_state() handlers, called from
> drm_atomic_private_obj_init() only. The only exception is
> __drm_atomic_helper_bridge_reset().

__drm_atomic_helper_bridge_reset (and any bridge reset implementation)
is also called through atomic_create_state and
drm_atomic_private_obj_init(), so there's nothing special about it.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 01/14] drm/atomic: Document atomic state lifetime

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]

Hi Tomi,

Thanks for your review

On Wed, Mar 11, 2026 at 08:44:24AM +0200, Tomi Valkeinen wrote:
> > + *
> > + *     At that point, &struct drm_atomic_state stores three state
> > + *     pointers for that particular entity: the old, new, and existing
> > + *     (called "state") states. The old state is the state currently
> > + *     active in the hardware, which is either the one initialized by
> > + *     reset() or a newer one if a commit has been made. The new state
> > + *     is the state we just allocated and we might eventually commit to
> > + *     the hardware. The existing state points to the state we'll
> > + *     eventually have to free when the drm_atomic_state will be
> > + *     destroyed, but points to the new state for now.
> 
> From this, I don't understand the difference between the old state and
> the existing state. And if the existing state is the one we'll free,
> isn't that the old state, not new state? Oh, is the existing state a
> state we have to free when the drm_atomic_state would is freed? And at
> this point the new state is the one, as it's not committed?

Thanks for pointing it out, I need to update this part. state is never
the active one, because drm_atomic_state disappears(ish) when the new
state is committed and thus, by the time an object state is active,
there's no drm_atomic_state to hold it anymore.

When a new drm_atomic_state is allocated, and we call
drm_atomic_get_$OBJECT_state, old state is filled with the current
active state, new state is filled with a copy of it we can modify.

The third pointer (that used to be called state) points to the state we
need to destroy if we destroy drm_atomic_state. Before atomic_commit,
it's the new state we didn't commit (probably to do an atomic_check).
After atomic_commit, the new state has become the active state, and we
need to destroy the old state (the state that got replaced). That
pointer is now called state_to_destroy which should be more obvious.

> > + *     state pointer (&drm_crtc.state or similar) to point to the new
> > + *     state, and the existing states will now point to the old states,
> > + *     that used to be active but isn't anymore.
> 
> "aren't"
> 
> I think I understand this, but... It kind of brings in a new state
> concept, "active state".

The active state is never really held by drm_atomic_state but by
drm_$OBJECT->state. drm_atomic_state is really more of an update
description (so something that might eventually become active) rather
than the actual active state.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 08/14] drm/mode-config: Create drm_mode_config_create_state()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6879 bytes --]

Hi Laurent,

On Mon, Mar 16, 2026 at 06:32:16PM +0200, Laurent Pinchart wrote:
> On Tue, Mar 10, 2026 at 05:07:00PM +0100, Maxime Ripard wrote:
> > drm_mode_config_reset() can be used to create the initial state, but
> > also to return to the initial state, when doing a suspend/resume cycle
> > for example.
> > 
> > It also affects both the software and the hardware, and drivers can
> > either choose to reset the hardware as well. Most will just create an
> 
> "either" requires an "or".
> 
> > empty state and the synchronisation between hardware and software states
> > will effectively be done when the first commit is done.
> > 
> > That dual role can be harmful, since some objects do need to be
> > initialized but also need to be preserved across a suspend/resume cycle.
> > drm_private_obj are such objects for example.
> > 
> > Thus, let's create another helper for drivers to call to initialize
> > their state when the driver is loaded, so we can make
> > drm_mode_config_reset() only about handling suspend/resume and similar.
> 
> Does that mean that the state initialization at probe time from the
> hardware state is planned to be handled by .atomic_create_state() ?

Not atomic_create_state itself, but through an extra
atomic_readout_state callback (that's the current plan at least).

The hardware readout is only really relevant at driver probe anyway,
while we could need to create a new blank state in multiple occasions
(starting with reset if we decouple sw and hw reset).

> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/drm_atomic.c      | 12 +++++-
> >  drivers/gpu/drm/drm_mode_config.c | 83 +++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_mode_config.h     |  1 +
> >  3 files changed, 94 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 92c6afc8f22c8307a59dc266aacdb8e03351409d..d1885f895cce78725419b6291f4dbe5563e3b240 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -58,12 +58,20 @@
> >   * checking or doing the update, while object states are allocated while
> >   * the state will be, or is active in the hardware.
> >   *
> >   * Their respective lifetimes are:
> >   *
> > - * - at reset time, the object reset implementation will allocate a new,
> > - *   default, state and will store it in the object state pointer.
> > + * - at driver initialization time, the driver will allocate an initial,
> > + *   pristine, state and will store it using
> > + *   drm_mode_config_create_state(). Historically, this was one of
> > + *   drm_mode_config_reset() job, so one might still encounter it in a
> > + *   driver.
> 
> Do you have plans to port drivers to the new API, to drop this last
> sentence sooner than later ?

Yes, definitely. I didn't want to do it in the first version because I
wasn't too sure how it was going to be received, but that's definitely
something I intend to do once the dust settles a bit.

> > + *
> > + * - at reset time, for example during suspend/resume,
> > + *   drm_mode_config_reset() will reset the software and hardware state
> > + *   to a known default and will store it in the object state pointer.
> > + *   Not all objects are affected by drm_mode_config_reset() though.
> 
> I have a bit of trouble understanding the design. How can
> drm_mode_config_reset() decide which objects should have their state
> reset (i.e. all objects but private objects in the current
> implementation), shouldn't that be a driver decision ?

It's not really what I meant. drm_mode_config_reset() should reset
everything it can (ie. everything with a reset callback). That being
said, private_objs are explicitly excluded from that everything. Whether
you want to implement reset or not is a driver decision, whether we want
to reset private_objs or not isn't.

> >   *
> >   * - whenever a new update is needed:
> >   *
> >   *   + we allocate a new &struct drm_atomic_state using drm_atomic_state_alloc().
> >   *
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 95759a4b5176ff17032a9a267ec9de6980be0abc..1a84ac063d381014eb8afd7116e1e8b7a8fc92ca 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -21,10 +21,11 @@
> >   */
> >  
> >  #include <linux/export.h>
> >  #include <linux/uaccess.h>
> >  
> > +#include <drm/drm_atomic.h>
> >  #include <drm/drm_drv.h>
> >  #include <drm/drm_encoder.h>
> >  #include <drm/drm_file.h>
> >  #include <drm/drm_framebuffer.h>
> >  #include <drm/drm_managed.h>
> > @@ -278,10 +279,92 @@ void drm_mode_config_reset(struct drm_device *dev)
> >  			drm_mode_config_connector_create_state(connector);
> >  	drm_connector_list_iter_end(&conn_iter);
> >  }
> >  EXPORT_SYMBOL(drm_mode_config_reset);
> >  
> > +/**
> > + * drm_mode_config_create_state - Allocates the initial state
> > + * @dev: drm device
> > + *
> > + * This functions creates the initial state for all the objects. Drivers
> > + * can use this in e.g. their driver load to initialize its software
> 
> I think you meant s/its/their/
> 
> > + * state.
> > + *
> > + * It has two main differences with drm_mode_config_reset(): the reset()
> > + * hooks aren't called and thus the hardware will be left untouched, but
> > + * also the @drm_private_obj structures will be initialized as opposed
> > + * to drm_mode_config_reset() that skips them.
> > + *
> > + * Returns: 0 on success, negative error value on failure.
> > + */
> > +int drm_mode_config_create_state(struct drm_device *dev)
> > +{
> > +	struct drm_crtc *crtc;
> > +	struct drm_colorop *colorop;
> > +	struct drm_plane *plane;
> > +	struct drm_connector *connector;
> > +	struct drm_connector_list_iter conn_iter;
> > +	struct drm_private_obj *privobj;
> > +	int ret;
> > +
> > +	drm_for_each_privobj(privobj, dev) {
> > +		struct drm_private_state *privobj_state;
> > +
> > +		if (privobj->state)
> > +			continue;
> > +
> > +		if (!privobj->funcs->atomic_create_state)
> > +			continue;
> > +
> > +		privobj_state = privobj->funcs->atomic_create_state(privobj);
> > +		if (IS_ERR(privobj_state))
> > +			return PTR_ERR(privobj_state);
> > +
> > +		privobj->state = privobj_state;
> 
> Either a helper function is missing, or you forgot to call it.

Did I? I'm calling atomic_create_state right above, what else did you
have in mind?

> > +	}
> > +
> > +	drm_for_each_colorop(colorop, dev) {
> > +		if (colorop->state)
> > +			continue;
> > +
> > +		// TODO: Implement atomic_create_state for colorop.
> 
> Oops :-)

WIP:
https://lore.kernel.org/dri-devel/20260319-nocturnal-mighty-chupacabra-87cbd0@houat/ :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 09/14] drm/drv: Call drm_mode_config_create_state() by default

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On Mon, Mar 16, 2026 at 06:33:54PM +0200, Laurent Pinchart wrote:
> On Tue, Mar 10, 2026 at 05:07:01PM +0100, Maxime Ripard wrote:
> > Almost all drivers, and our documented skeleton, call
> > drm_mode_config_reset() prior to calling drm_dev_register() to
> > initialize its DRM object states.
> > 
> > Now that we have drm_mode_config_create_state() to create that initial
> > state if it doesn't exist, we can call it directly in
> > drm_dev_register(). That way, we know that the initial atomic state will
> > always be allocated without any boilerplate.
> 
> Should most drivers now stop calling drm_mode_config_reset() at probe
> time ?

Yes, that's my intention. For the vast majority of them, reset is just
about allocating and committing a blank state anyway.

However, for the drivers that do need to reset the hardware, we should
keep the door open (and / or provide an alternative mechanism).

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]