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

Maxime Ripard posted 14 patches 4 weeks ago
There is a newer version of this series
[PATCH 08/14] drm/mode-config: Create drm_mode_config_create_state()
Posted by Maxime Ripard 4 weeks ago
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
Re: [PATCH 08/14] drm/mode-config: Create drm_mode_config_create_state()
Posted by Laurent Pinchart 3 weeks, 1 day ago
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 08/14] drm/mode-config: Create drm_mode_config_create_state()
Posted by Maxime Ripard 2 weeks, 5 days ago
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