From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54C2E1CAA65; Mon, 4 May 2026 16:26:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777912003; cv=none; b=Cmk/9Fhg/6G2T/n00TBTBzoOSNYMFO8516yn7/EQpxt8HBRL6QZBzewLh0A2GqPaXY2+4c9XBo/yvnoXsMy8fRsl5bEoHiFz8dJbTEiYjKpoqNsltVnXXn+ISn57n01A08cmG2nEnKo1MFbsShEMyXtFbWeUEHXKw7R6ZG1gK+0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777912003; c=relaxed/simple; bh=E9px648KO3wjHBF0FvoQJ7oWqClwxx9bwOStqChZ5Dg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=P95pw0JAONBHDobKjHhzhdH05gTpwDkXlRiEiDd6gHtMLcQbsSzAgPLmBgVkppZAs3zSOefJPd2uuUQaihfU0msF5jQv7d8+LpyqkpJrcHgvFAS6wYom7idgTG8ySwY5Xxv304QayKHO0nR8LXb2NrpQD+HVRuYBJ0awvdatoUw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=C7nsBiw9; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="C7nsBiw9" Received: from killaraus.ideasonboard.com (2001-14ba-703d-e500--2a1.rev.dnainternet.fi [IPv6:2001:14ba:703d:e500::2a1]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D36F9C; Mon, 4 May 2026 18:26:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1777911995; bh=E9px648KO3wjHBF0FvoQJ7oWqClwxx9bwOStqChZ5Dg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C7nsBiw9DfJdqe3vAIYvrHSaWv8vOFVzMj0n+NiBzUAPMyhk8h2a2SM2Bulaujsx8 d1pRVOk4Ncb3L1muy/CHM/aX8ZQX1GlcGVUsDs0tNhAx+haCjF7MzMb+9wF1uQ5IaL OgYAB8rIqmJUXUEvwtBpDVsHyESYrjqNYazNcKwY= Date: Mon, 4 May 2026 19:26:36 +0300 From: Laurent Pinchart To: Maxime Ripard Cc: Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Jonathan Corbet , Shuah Khan , Dmitry Baryshkov , Jyri Sarha , Tomi Valkeinen , Andrzej Hajda , Neil Armstrong , Robert Foss , Jonas Karlman , Jernej Skrabec , Simon Ser , Harry Wentland , Melissa Wen , Sebastian Wick , Alex Hung , Jani Nikula , Rodrigo Vivi , Joonas Lahtinen , Tvrtko Ursulin , Chen-Yu Tsai , Samuel Holland , Dave Stevenson , =?utf-8?B?TWHDrXJh?= Canal , Raspberry Pi Kernel Maintenance , dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Daniel Stone , intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev Subject: Re: [PATCH v3 01/20] drm/atomic: Document atomic state lifetime Message-ID: <20260504162636.GF1344263@killaraus.ideasonboard.com> References: <20260424-drm-mode-config-init-v3-0-8b68d9db0d8b@kernel.org> <20260424-drm-mode-config-init-v3-1-8b68d9db0d8b@kernel.org> Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260424-drm-mode-config-init-v3-1-8b68d9db0d8b@kernel.org> Hi Maxime, Thank you for the patch. On Fri, Apr 24, 2026 at 12:18:41PM +0200, Maxime Ripard wrote: > How drm_atomic_state structures and the various entity structures are > allocated and freed isn't really trivial. Document it. > > Signed-off-by: Maxime Ripard > --- > Documentation/gpu/drm-kms.rst | 6 +++++ > drivers/gpu/drm/drm_atomic.c | 55 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 2292e65f044c..017c7b196ed7 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 `. > > 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 41c57063f3b4..253a00f450b0 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -45,10 +45,65 @@ > #include > > #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 pointers 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 committing the update, while object states are allocated > + * when preparing the update and kept alive as long as they are active > + * in the device. > + * > + * 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: > + * > + * + A new &struct drm_atomic_state is allocated using > + * drm_atomic_state_alloc(). > + * > + * + The current active state of affected entity is copied into this s/affected entity/affected entities/ but maybe clearer, I'd write + The current active state of all entities affected by the update is copied ... > + * new &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(). This new state can then be > + * modified. > + * > + * At that point, &struct drm_atomic_state stores three state > + * pointers for any affected entity: the "old" and "new" states, and > + * state_to_destroy. 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 state_to_destroy points to the state we'll eventually have to > + * free when the drm_atomic_state will be destroyed, and points to > + * the new state for now since the old state is still the active > + * state. > + * > + * + 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 object's > + * state pointer (&drm_crtc.state or similar) to point to the new > + * state, and state_to_destroy will now point to the old states, > + * that used to be active but aren'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 state_to_destroy (ie. old states). I would also mention here that the drm_atomic_state itself is freed at this point (unless I'm mistaken and the drm_atomic_state still lives on for some time, in which case this misunderstanding is probably a sign that the correct behaviour should be documented :-)). With this, Reviewed-by: Laurent Pinchart > + * > + * + 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); > -- Regards, Laurent Pinchart