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

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

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

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