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

[Maxime Ripard <mripard@kernel.org>]

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