[PATCH 00/16] drm/atomic: Switch drm_private_obj to reset

[PATCH 00/16] drm/atomic: Switch drm_private_obj to reset

[Maxime Ripard]

Hi,

This series started from my work on the hardware state readout[1], and
was suggested by Dmitry[2].

This series deal with the fact that drm_private_obj (and thus bridges)
are not initialized using the same pattern than any other object. This
series solves that inconsistency by aligning it to what we're doing for
all the other objects.

This was tested on a TI SK-AM62, with three bridges.

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/zvqtehg66dbrrdmik6ylo2kdk74umfzo5hbfkizwsb352nlyqv@jgouvmbfwa4x/

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Maxime Ripard (16):
      drm/atomic: Add dev pointer to drm_private_obj
      drm/atomic: Add reset to drm_private_obj
      drm/atomic-helper: Add private_obj reset helper
      drm/bridge: Switch private_obj initialization to reset
      drm/dp_mst: Switch private_obj initialization to reset
      drm/dp_tunnel: Switch private_obj initialization to reset
      drm/amdgpu: Switch private_obj initialization to reset
      drm/arm: komeda: Switch private_obj initialization to reset
      drm/ingenic: Switch private_obj initialization to reset
      drm/msm: mdp5: Switch private_obj initialization to reset
      drm/msm: dpu1: Switch private_obj initialization to reset
      drm/omapdrm: Switch private_obj initialization to reset
      drm/tegra: Switch private_obj initialization to reset
      drm/vc4: Switch private_obj initialization to reset
      drm/atomic: Remove state argument to drm_atomic_private_obj_init
      drm/mode_config: Call private obj reset with the other objects

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  53 ++---
 .../gpu/drm/arm/display/komeda/komeda_pipeline.h   |   2 +
 .../drm/arm/display/komeda/komeda_private_obj.c    | 232 +++++++++++++++------
 drivers/gpu/drm/display/drm_dp_mst_topology.c      |  38 ++--
 drivers/gpu/drm/display/drm_dp_tunnel.c            |  28 ++-
 drivers/gpu/drm/drm_atomic.c                       |   6 +-
 drivers/gpu/drm/drm_atomic_state_helper.c          |  24 +++
 drivers/gpu/drm/drm_bridge.c                       |  30 +--
 drivers/gpu/drm/drm_mode_config.c                  |   6 +
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c          |  30 ++-
 drivers/gpu/drm/ingenic/ingenic-ipu.c              |  30 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |  30 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c           |  30 ++-
 drivers/gpu/drm/omapdrm/omap_drv.c                 |  24 ++-
 drivers/gpu/drm/tegra/hub.c                        |  24 ++-
 drivers/gpu/drm/vc4/vc4_kms.c                      |  73 +++++--
 include/drm/drm_atomic.h                           |  15 +-
 include/drm/drm_atomic_state_helper.h              |   3 +
 18 files changed, 473 insertions(+), 205 deletions(-)
---
base-commit: aa1c2b073ad23847dd2e7bdc7d30009f34ed7f59
change-id: 20251008-drm-private-obj-reset-ae1e2741027a

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

[PATCH 01/16] drm/atomic: Add dev pointer to drm_private_obj

[Maxime Ripard]

All the objects that need to implement some callbacks in KMS have a
pointer in there structure to the main drm_device.

However, it's not the case for drm_private_objs, which makes it harder
than it needs to be to implement some of its callbacks. Let's add that
pointer.

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

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ed5359a71f7e2cd8fa52b993e62ee65f8fed4537..39cb1479ac4d58cd71cf41d27d0d2a8a58ef5791 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -790,10 +790,11 @@ drm_atomic_private_obj_init(struct drm_device *dev,
 {
 	memset(obj, 0, sizeof(*obj));
 
 	drm_modeset_lock_init(&obj->lock);
 
+	obj->dev = dev;
 	obj->state = state;
 	obj->funcs = funcs;
 	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
 
 	state->obj = obj;
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 38636a593c9d98cadda85ccd67326cb152f0dd27..dac70f685361d8d29844acd1b0cc2f04f43a9499 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -282,10 +282,15 @@ struct drm_private_state_funcs {
  * commit to complete as the first step of
  * &drm_mode_config_helper_funcs.atomic_commit_tail, similar to
  * drm_atomic_helper_wait_for_dependencies().
  */
 struct drm_private_obj {
+	/**
+	 * @dev: parent DRM device
+	 */
+	struct drm_device *dev;
+
 	/**
 	 * @head: List entry used to attach a private object to a &drm_device
 	 * (queued to &drm_mode_config.privobj_list).
 	 */
 	struct list_head head;

-- 
2.51.0

Re: [PATCH 01/16] drm/atomic: Add dev pointer to drm_private_obj

[Tomi Valkeinen]

Hi,

On 08/10/2025 15:03, Maxime Ripard wrote:
> All the objects that need to implement some callbacks in KMS have a
> pointer in there structure to the main drm_device.
> 
> However, it's not the case for drm_private_objs, which makes it harder
> than it needs to be to implement some of its callbacks. Let's add that
> pointer.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

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

 Tomi

>  drivers/gpu/drm/drm_atomic.c | 1 +
>  include/drm/drm_atomic.h     | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ed5359a71f7e2cd8fa52b993e62ee65f8fed4537..39cb1479ac4d58cd71cf41d27d0d2a8a58ef5791 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -790,10 +790,11 @@ drm_atomic_private_obj_init(struct drm_device *dev,
>  {
>  	memset(obj, 0, sizeof(*obj));
>  
>  	drm_modeset_lock_init(&obj->lock);
>  
> +	obj->dev = dev;
>  	obj->state = state;
>  	obj->funcs = funcs;
>  	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
>  
>  	state->obj = obj;
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 38636a593c9d98cadda85ccd67326cb152f0dd27..dac70f685361d8d29844acd1b0cc2f04f43a9499 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -282,10 +282,15 @@ struct drm_private_state_funcs {
>   * commit to complete as the first step of
>   * &drm_mode_config_helper_funcs.atomic_commit_tail, similar to
>   * drm_atomic_helper_wait_for_dependencies().
>   */
>  struct drm_private_obj {
> +	/**
> +	 * @dev: parent DRM device
> +	 */
> +	struct drm_device *dev;
> +
>  	/**
>  	 * @head: List entry used to attach a private object to a &drm_device
>  	 * (queued to &drm_mode_config.privobj_list).
>  	 */
>  	struct list_head head;
> 

Re: [PATCH 01/16] drm/atomic: Add dev pointer to drm_private_obj

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:03:59PM +0200, Maxime Ripard wrote:
> All the objects that need to implement some callbacks in KMS have a
> pointer in there structure to the main drm_device.
> 
> However, it's not the case for drm_private_objs, which makes it harder
> than it needs to be to implement some of its callbacks. Let's add that
> pointer.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic.c | 1 +
>  include/drm/drm_atomic.h     | 5 +++++
>  2 files changed, 6 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 02/16] drm/atomic: Add reset to drm_private_obj

[Maxime Ripard]

The drm_private_obj initialization was inconsistent with the rest of the
KMS objects. Indeed, it required to pass a preallocated state in
drm_private_obj_init(), while all the others objects would have a reset
callback that would be called later on to create the state.

Let's prepare for the migration of all private objs implementation by
introducing a reset callback in drm_private_state_funcs, and by calling
it if the passed state is NULL.

The latter will be removed eventually, once every driver has been
converted.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic.c | 15 +++++++++++++--
 include/drm/drm_atomic.h     |  9 +++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 39cb1479ac4d58cd71cf41d27d0d2a8a58ef5791..45c26294e712fd36b43e87548072c3c0e9af1887 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -791,15 +791,26 @@ drm_atomic_private_obj_init(struct drm_device *dev,
 	memset(obj, 0, sizeof(*obj));
 
 	drm_modeset_lock_init(&obj->lock);
 
 	obj->dev = dev;
-	obj->state = state;
 	obj->funcs = funcs;
 	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
 
-	state->obj = obj;
+	/*
+	 * Not all users of drm_atomic_private_obj_init have been
+	 * converted to using &drm_private_obj_funcs.reset yet. For the
+	 * time being, let's only call reset if the passed state is
+	 * NULL. Otherwise, we will fallback to the previous behaviour.
+	 */
+	if (!state) {
+		if (obj->funcs->reset)
+			obj->funcs->reset(obj);
+	} else {
+		obj->state = state;
+		state->obj = obj;
+	}
 }
 EXPORT_SYMBOL(drm_atomic_private_obj_init);
 
 /**
  * drm_atomic_private_obj_fini - finalize private object
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index dac70f685361d8d29844acd1b0cc2f04f43a9499..fbac6d4c75fc86535cf153745b6132f8705c808a 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -205,10 +205,19 @@ struct drm_private_state;
  * added to the atomic states is expected to have an implementation of these
  * hooks and pass a pointer to its drm_private_state_funcs struct to
  * drm_atomic_get_private_obj_state().
  */
 struct drm_private_state_funcs {
+	/**
+	 * @reset:
+	 *
+	 * Resets the private state to its default state, and the
+	 * hardware to off if any.. This function isn't called by the
+	 * core directly, only through drm_mode_config_reset().
+	 */
+	void (*reset)(struct drm_private_obj *obj);
+
 	/**
 	 * @atomic_duplicate_state:
 	 *
 	 * Duplicate the current state of the private object and return it. It
 	 * is an error to call this before obj->state has been initialized.

-- 
2.51.0

Re: [PATCH 02/16] drm/atomic: Add reset to drm_private_obj

[Tomi Valkeinen]

Hi,

On 08/10/2025 15:04, Maxime Ripard wrote:
> The drm_private_obj initialization was inconsistent with the rest of the
> KMS objects. Indeed, it required to pass a preallocated state in
> drm_private_obj_init(), while all the others objects would have a reset
> callback that would be called later on to create the state.
> 
> Let's prepare for the migration of all private objs implementation by
> introducing a reset callback in drm_private_state_funcs, and by calling
> it if the passed state is NULL.
> 
> The latter will be removed eventually, once every driver has been
> converted.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic.c | 15 +++++++++++++--
>  include/drm/drm_atomic.h     |  9 +++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 39cb1479ac4d58cd71cf41d27d0d2a8a58ef5791..45c26294e712fd36b43e87548072c3c0e9af1887 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -791,15 +791,26 @@ drm_atomic_private_obj_init(struct drm_device *dev,
>  	memset(obj, 0, sizeof(*obj));
>  
>  	drm_modeset_lock_init(&obj->lock);
>  
>  	obj->dev = dev;
> -	obj->state = state;
>  	obj->funcs = funcs;
>  	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
>  
> -	state->obj = obj;
> +	/*
> +	 * Not all users of drm_atomic_private_obj_init have been
> +	 * converted to using &drm_private_obj_funcs.reset yet. For the
> +	 * time being, let's only call reset if the passed state is
> +	 * NULL. Otherwise, we will fallback to the previous behaviour.
> +	 */
> +	if (!state) {
> +		if (obj->funcs->reset)
> +			obj->funcs->reset(obj);
> +	} else {
> +		obj->state = state;
> +		state->obj = obj;
> +	}
>  }
>  EXPORT_SYMBOL(drm_atomic_private_obj_init);
>  
>  /**
>   * drm_atomic_private_obj_fini - finalize private object
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index dac70f685361d8d29844acd1b0cc2f04f43a9499..fbac6d4c75fc86535cf153745b6132f8705c808a 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -205,10 +205,19 @@ struct drm_private_state;
>   * added to the atomic states is expected to have an implementation of these
>   * hooks and pass a pointer to its drm_private_state_funcs struct to
>   * drm_atomic_get_private_obj_state().
>   */
>  struct drm_private_state_funcs {
> +	/**
> +	 * @reset:
> +	 *
> +	 * Resets the private state to its default state, and the
> +	 * hardware to off if any.. This function isn't called by the

The text above may need some massaging. The double period, at least.
"Reset private hardware and software state to off" ?

Other than that:

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

 Tomi

> +	 * core directly, only through drm_mode_config_reset().
> +	 */
> +	void (*reset)(struct drm_private_obj *obj);
> +
>  	/**
>  	 * @atomic_duplicate_state:
>  	 *
>  	 * Duplicate the current state of the private object and return it. It
>  	 * is an error to call this before obj->state has been initialized.
> 

[PATCH 03/16] drm/atomic-helper: Add private_obj reset helper

[Maxime Ripard]

Now that we have a reset callback for drm_private_objs, we can provide a
helper for it.

It's somewhat different from the other similar helpers though, because
we definitely expect drm_private_obj to be subclassed. It wouldn't make
sense for a driver to use it as-is.

So we can't provide a straight implementation of the reset callback, but
rather we provide the parts that will deal with the drm_private_obj
initialization, and we will leave the allocation and initialization of
the subclass to drivers.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 24 ++++++++++++++++++++++++
 include/drm/drm_atomic_state_helper.h     |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..f88007fe8dba2e79d5942deec3cfdd7757c1a460 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -707,10 +707,34 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 	__drm_atomic_helper_connector_destroy_state(state);
 	kfree(state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
 
+/**
+ * __drm_atomic_helper_private_obj_reset - reset state on private objects
+ * @obj: private object
+ * @state: new state to initialize
+ *
+ * Initializes the newly allocated @state and assigns it to the
+ * &drm_private_obj->state pointer of @obj, usually required when
+ * initializing the drivers or when called from the
+ * &drm_private_state_funcs.reset hook.
+ *
+ * @obj is assumed to be zeroed.
+ *
+ * This is useful for drivers that use private states.
+ */
+void __drm_atomic_helper_private_obj_reset(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_reset);
+
 /**
  * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
  * @obj: CRTC object
  * @state: new private object state
  *
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index b9740edb26586d58f99a5223902bb8e333ac75a2..150ea227c595eab8c45b106baf09ce5b27a89a5a 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -82,10 +82,13 @@ struct drm_connector_state *
 drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector);
 void
 __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state);
 void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 					  struct drm_connector_state *state);
+
+void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
+					   struct drm_private_state *state);
 void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
 						     struct drm_private_state *state);
 
 void __drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge,
 						struct drm_bridge_state *state);

-- 
2.51.0

Re: [PATCH 03/16] drm/atomic-helper: Add private_obj reset helper

[Tomi Valkeinen]

Hi,

On 08/10/2025 15:04, Maxime Ripard wrote:
> Now that we have a reset callback for drm_private_objs, we can provide a
> helper for it.
> 
> It's somewhat different from the other similar helpers though, because
> we definitely expect drm_private_obj to be subclassed. It wouldn't make
> sense for a driver to use it as-is.
> 
> So we can't provide a straight implementation of the reset callback, but
> rather we provide the parts that will deal with the drm_private_obj
> initialization, and we will leave the allocation and initialization of
> the subclass to drivers.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

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

 Tomi

>  drivers/gpu/drm/drm_atomic_state_helper.c | 24 ++++++++++++++++++++++++
>  include/drm/drm_atomic_state_helper.h     |  3 +++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..f88007fe8dba2e79d5942deec3cfdd7757c1a460 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -707,10 +707,34 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  	__drm_atomic_helper_connector_destroy_state(state);
>  	kfree(state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>  
> +/**
> + * __drm_atomic_helper_private_obj_reset - reset state on private objects
> + * @obj: private object
> + * @state: new state to initialize
> + *
> + * Initializes the newly allocated @state and assigns it to the
> + * &drm_private_obj->state pointer of @obj, usually required when
> + * initializing the drivers or when called from the
> + * &drm_private_state_funcs.reset hook.
> + *
> + * @obj is assumed to be zeroed.
> + *
> + * This is useful for drivers that use private states.
> + */
> +void __drm_atomic_helper_private_obj_reset(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_reset);
> +
>  /**
>   * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
>   * @obj: CRTC object
>   * @state: new private object state
>   *
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index b9740edb26586d58f99a5223902bb8e333ac75a2..150ea227c595eab8c45b106baf09ce5b27a89a5a 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -82,10 +82,13 @@ struct drm_connector_state *
>  drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector);
>  void
>  __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state);
>  void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  					  struct drm_connector_state *state);
> +
> +void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> +					   struct drm_private_state *state);
>  void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
>  						     struct drm_private_state *state);
>  
>  void __drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge,
>  						struct drm_bridge_state *state);
> 

Re: [PATCH 03/16] drm/atomic-helper: Add private_obj reset helper

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:01PM +0200, Maxime Ripard wrote:
> Now that we have a reset callback for drm_private_objs, we can provide a
> helper for it.
> 
> It's somewhat different from the other similar helpers though, because
> we definitely expect drm_private_obj to be subclassed. It wouldn't make
> sense for a driver to use it as-is.
> 
> So we can't provide a straight implementation of the reset callback, but
> rather we provide the parts that will deal with the drm_private_obj
> initialization, and we will leave the allocation and initialization of
> the subclass to drivers.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 24 ++++++++++++++++++++++++
>  include/drm/drm_atomic_state_helper.h     |  3 +++
>  2 files changed, 27 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 04/16] drm/bridge: Switch private_obj initialization to reset

[Maxime Ripard]

The bridge implementation relies on a drm_private_obj, that is
initialized by allocating and initializing a state, and then passing it
to drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/drm_bridge.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 630b5e6594e0affad9ba48791207c7b403da5db8..cc346412b0205288ec7ee5a7d80a897ad9659404 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -394,13 +394,27 @@ drm_bridge_atomic_destroy_priv_state(struct drm_private_obj *obj,
 	struct drm_bridge *bridge = drm_priv_to_bridge(obj);
 
 	bridge->funcs->atomic_destroy_state(bridge, state);
 }
 
+static void
+drm_bridge_reset_priv_state(struct drm_private_obj *obj)
+{
+	struct drm_bridge *bridge = drm_priv_to_bridge(obj);
+	struct drm_bridge_state *state;
+
+	state = bridge->funcs->atomic_reset(bridge);
+	if (IS_ERR(state))
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &state->base);
+}
+
 static const struct drm_private_state_funcs drm_bridge_priv_state_funcs = {
 	.atomic_duplicate_state = drm_bridge_atomic_duplicate_priv_state,
 	.atomic_destroy_state = drm_bridge_atomic_destroy_priv_state,
+	.reset = drm_bridge_reset_priv_state,
 };
 
 static bool drm_bridge_is_atomic(struct drm_bridge *bridge)
 {
 	return bridge->funcs->atomic_reset != NULL;
@@ -462,30 +476,17 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 		ret = bridge->funcs->attach(bridge, encoder, flags);
 		if (ret < 0)
 			goto err_reset_bridge;
 	}
 
-	if (drm_bridge_is_atomic(bridge)) {
-		struct drm_bridge_state *state;
-
-		state = bridge->funcs->atomic_reset(bridge);
-		if (IS_ERR(state)) {
-			ret = PTR_ERR(state);
-			goto err_detach_bridge;
-		}
-
+	if (drm_bridge_is_atomic(bridge))
 		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
-					    &state->base,
+					    NULL,
 					    &drm_bridge_priv_state_funcs);
-	}
 
 	return 0;
 
-err_detach_bridge:
-	if (bridge->funcs->detach)
-		bridge->funcs->detach(bridge);
-
 err_reset_bridge:
 	bridge->dev = NULL;
 	bridge->encoder = NULL;
 	list_del(&bridge->chain_node);
 

-- 
2.51.0

Re: [PATCH 04/16] drm/bridge: Switch private_obj initialization to reset

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:02PM +0200, Maxime Ripard wrote:
> The bridge implementation relies on a drm_private_obj, that is
> initialized by allocating and initializing a state, and then passing it
> to drm_private_obj_init.
> 
> Since we're gradually moving away from that pattern to the more
> established one relying on a reset implementation, let's migrate this
> instance to the new pattern.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---
> 
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/gpu/drm/drm_bridge.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> @@ -462,30 +476,17 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  		ret = bridge->funcs->attach(bridge, encoder, flags);
>  		if (ret < 0)
>  			goto err_reset_bridge;
>  	}
>  
> -	if (drm_bridge_is_atomic(bridge)) {
> -		struct drm_bridge_state *state;
> -
> -		state = bridge->funcs->atomic_reset(bridge);
> -		if (IS_ERR(state)) {
> -			ret = PTR_ERR(state);
> -			goto err_detach_bridge;
> -		}
> -
> +	if (drm_bridge_is_atomic(bridge))
>  		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
> -					    &state->base,
> +					    NULL,
>  					    &drm_bridge_priv_state_funcs);

This is now very idiomatic, I like it!

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH 04/16] drm/bridge: Switch private_obj initialization to reset

[kernel test robot]

Hi Maxime,

kernel test robot noticed the following build errors:

[auto build test ERROR on aa1c2b073ad23847dd2e7bdc7d30009f34ed7f59]

url:    https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-atomic-Add-dev-pointer-to-drm_private_obj/20251009-195815
base:   aa1c2b073ad23847dd2e7bdc7d30009f34ed7f59
patch link:    https://lore.kernel.org/r/20251008-drm-private-obj-reset-v1-4-805ab43ae65a%40kernel.org
patch subject: [PATCH 04/16] drm/bridge: Switch private_obj initialization to reset
config: i386-randconfig-011-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101750.mdgz7wUC-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510101750.mdgz7wUC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510101750.mdgz7wUC-lkp@intel.com/

All errors (new ones prefixed by >>):

>> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
   depmod: ERROR: Found 2 modules in dependency cycles!
   make[3]: *** [scripts/Makefile.modinst:132: depmod] Error 1 shuffle=982479975
   make[3]: Target '__modinst' not remade because of errors.
   make[2]: *** [Makefile:1917: modules_install] Error 2 shuffle=982479975
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=982479975
   make[1]: Target 'modules_install' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=982479975
   make: Target 'modules_install' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Maxime Ripard]

The DP MST implementation relies on a drm_private_obj, that is
initialized by allocating and initializing a state, and then passing it
to drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
 
 	kfree(mst_state->commit_deps);
 	kfree(mst_state);
 }
 
+static void drm_dp_mst_reset(struct drm_private_obj *obj)
+{
+	struct drm_dp_mst_topology_mgr *mgr =
+		to_dp_mst_topology_mgr(obj);
+	struct drm_dp_mst_topology_state *mst_state;
+
+	if (obj->state) {
+		drm_dp_mst_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
+	if (!mst_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &mst_state->base);
+
+	mst_state->total_avail_slots = 63;
+	mst_state->start_slot = 1;
+
+	mst_state->mgr = mgr;
+	INIT_LIST_HEAD(&mst_state->payloads);
+}
+
 static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
 						 struct drm_dp_mst_branch *branch)
 {
 	while (port->parent) {
 		if (port->parent == branch)
@@ -5619,10 +5643,11 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state)
 EXPORT_SYMBOL(drm_dp_mst_atomic_check);
 
 const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
 	.atomic_duplicate_state = drm_dp_mst_duplicate_state,
 	.atomic_destroy_state = drm_dp_mst_destroy_state,
+	.reset = drm_dp_mst_reset,
 };
 EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
 
 /**
  * drm_atomic_get_mst_topology_state: get MST topology state
@@ -5705,12 +5730,10 @@ EXPORT_SYMBOL(drm_atomic_get_new_mst_topology_state);
 int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 				 struct drm_device *dev, struct drm_dp_aux *aux,
 				 int max_dpcd_transaction_bytes, int max_payloads,
 				 int conn_base_id)
 {
-	struct drm_dp_mst_topology_state *mst_state;
-
 	mutex_init(&mgr->lock);
 	mutex_init(&mgr->qlock);
 	mutex_init(&mgr->delayed_destroy_lock);
 	mutex_init(&mgr->up_req_lock);
 	mutex_init(&mgr->probe_lock);
@@ -5740,22 +5763,12 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	mgr->aux = aux;
 	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
 	mgr->max_payloads = max_payloads;
 	mgr->conn_base_id = conn_base_id;
 
-	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
-	if (mst_state == NULL)
-		return -ENOMEM;
-
-	mst_state->total_avail_slots = 63;
-	mst_state->start_slot = 1;
-
-	mst_state->mgr = mgr;
-	INIT_LIST_HEAD(&mst_state->payloads);
-
 	drm_atomic_private_obj_init(dev, &mgr->base,
-				    &mst_state->base,
+				    NULL,
 				    &drm_dp_mst_topology_state_funcs);
 
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);

-- 
2.51.0

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Ville Syrjälä]

On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> The DP MST implementation relies on a drm_private_obj, that is
> initialized by allocating and initializing a state, and then passing it
> to drm_private_obj_init.
> 
> Since we're gradually moving away from that pattern to the more
> established one relying on a reset implementation, let's migrate this
> instance to the new pattern.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
>  
>  	kfree(mst_state->commit_deps);
>  	kfree(mst_state);
>  }
>  
> +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> +{
> +	struct drm_dp_mst_topology_mgr *mgr =
> +		to_dp_mst_topology_mgr(obj);
> +	struct drm_dp_mst_topology_state *mst_state;
> +
> +	if (obj->state) {
> +		drm_dp_mst_destroy_state(obj, obj->state);
> +		obj->state = NULL;

I'm not a big fan of this "free+reallocate without any way to handle
failures" pattern.

Currently i915 does not do any of this, and so there are no unhandled
points of failure. But the mst and tunneling changes here force it
on i915 as well.

I think for the common things it would be nicer to just implement
the reset as just that "a reset of the current state", and leave
the "let's play fast and loose with kmalloc() failures" to the
drivers that want it.

That said I haven't even thought through whether this mst and
tunneling state reset might have some undesired side effects
since we previously did none of it. I suspect it should be fine,
but at least the mst code does some questionable things with
the state so not 100% sure.

Imre, do you recall if we might somehow depend on preserving
the state across drm_mode_config_reset()?

> +	}
> +
> +	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> +	if (!mst_state)
> +		return;
> +
> +	__drm_atomic_helper_private_obj_reset(obj, &mst_state->base);
> +
> +	mst_state->total_avail_slots = 63;
> +	mst_state->start_slot = 1;
> +
> +	mst_state->mgr = mgr;
> +	INIT_LIST_HEAD(&mst_state->payloads);
> +}
> +
>  static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
>  						 struct drm_dp_mst_branch *branch)
>  {
>  	while (port->parent) {
>  		if (port->parent == branch)
> @@ -5619,10 +5643,11 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state)
>  EXPORT_SYMBOL(drm_dp_mst_atomic_check);
>  
>  const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
>  	.atomic_duplicate_state = drm_dp_mst_duplicate_state,
>  	.atomic_destroy_state = drm_dp_mst_destroy_state,
> +	.reset = drm_dp_mst_reset,
>  };
>  EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
>  
>  /**
>   * drm_atomic_get_mst_topology_state: get MST topology state
> @@ -5705,12 +5730,10 @@ EXPORT_SYMBOL(drm_atomic_get_new_mst_topology_state);
>  int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
>  				 struct drm_device *dev, struct drm_dp_aux *aux,
>  				 int max_dpcd_transaction_bytes, int max_payloads,
>  				 int conn_base_id)
>  {
> -	struct drm_dp_mst_topology_state *mst_state;
> -
>  	mutex_init(&mgr->lock);
>  	mutex_init(&mgr->qlock);
>  	mutex_init(&mgr->delayed_destroy_lock);
>  	mutex_init(&mgr->up_req_lock);
>  	mutex_init(&mgr->probe_lock);
> @@ -5740,22 +5763,12 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
>  	mgr->aux = aux;
>  	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
>  	mgr->max_payloads = max_payloads;
>  	mgr->conn_base_id = conn_base_id;
>  
> -	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> -	if (mst_state == NULL)
> -		return -ENOMEM;
> -
> -	mst_state->total_avail_slots = 63;
> -	mst_state->start_slot = 1;
> -
> -	mst_state->mgr = mgr;
> -	INIT_LIST_HEAD(&mst_state->payloads);
> -
>  	drm_atomic_private_obj_init(dev, &mgr->base,
> -				    &mst_state->base,
> +				    NULL,
>  				    &drm_dp_mst_topology_state_funcs);
>  
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
> 
> -- 
> 2.51.0

-- 
Ville Syrjälä
Intel

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Maxime Ripard]

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

On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > The DP MST implementation relies on a drm_private_obj, that is
> > initialized by allocating and initializing a state, and then passing it
> > to drm_private_obj_init.
> > 
> > Since we're gradually moving away from that pattern to the more
> > established one relying on a reset implementation, let's migrate this
> > instance to the new pattern.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> >  1 file changed, 26 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> >  
> >  	kfree(mst_state->commit_deps);
> >  	kfree(mst_state);
> >  }
> >  
> > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > +{
> > +	struct drm_dp_mst_topology_mgr *mgr =
> > +		to_dp_mst_topology_mgr(obj);
> > +	struct drm_dp_mst_topology_state *mst_state;
> > +
> > +	if (obj->state) {
> > +		drm_dp_mst_destroy_state(obj, obj->state);
> > +		obj->state = NULL;
> 
> I'm not a big fan of this "free+reallocate without any way to handle
> failures" pattern.
> 
> Currently i915 does not do any of this, and so there are no unhandled
> points of failure. But the mst and tunneling changes here force it on
> i915 as well.

A very theoretical point of failure. If I'm counting right,
drm_dp_mst_topology_state takes 72 bytes. Any GFP_KERNEL allocation
of less than 8 pages cannot fail.

So, sure, it's less satisfying to look at, but that's about it. It's
just as safe and reliable.

> I think for the common things it would be nicer to just implement
> the reset as just that "a reset of the current state", and leave
> the "let's play fast and loose with kmalloc() failures" to the
> drivers that want it.

I'm sorry, but I have no idea what you mean by that. It's using the
exact same pattern we've been using since forever for every driver
(except, apparently, i915).

What would you like to change to that pattern to accomodate i915
requirements?

Maxime

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

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Ville Syrjälä]

On Wed, Oct 08, 2025 at 04:53:20PM +0200, Maxime Ripard wrote:
> On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > > The DP MST implementation relies on a drm_private_obj, that is
> > > initialized by allocating and initializing a state, and then passing it
> > > to drm_private_obj_init.
> > > 
> > > Since we're gradually moving away from that pattern to the more
> > > established one relying on a reset implementation, let's migrate this
> > > instance to the new pattern.
> > > 
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> > >  1 file changed, 26 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> > >  
> > >  	kfree(mst_state->commit_deps);
> > >  	kfree(mst_state);
> > >  }
> > >  
> > > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > > +{
> > > +	struct drm_dp_mst_topology_mgr *mgr =
> > > +		to_dp_mst_topology_mgr(obj);
> > > +	struct drm_dp_mst_topology_state *mst_state;
> > > +
> > > +	if (obj->state) {
> > > +		drm_dp_mst_destroy_state(obj, obj->state);
> > > +		obj->state = NULL;
> > 
> > I'm not a big fan of this "free+reallocate without any way to handle
> > failures" pattern.
> > 
> > Currently i915 does not do any of this, and so there are no unhandled
> > points of failure. But the mst and tunneling changes here force it on
> > i915 as well.
> 
> A very theoretical point of failure. If I'm counting right,
> drm_dp_mst_topology_state takes 72 bytes. Any GFP_KERNEL allocation
> of less than 8 pages cannot fail.

Until you start to inject failures, or you blow up the size of the
state. You wouldn't think to check for potential NULL pointer
dereferences before doing that, nor should you have to.

I'd rather not leave potential footguns lying around intentionally.

> So, sure, it's less satisfying to look at, but that's about it. It's
> just as safe and reliable.
> 
> > I think for the common things it would be nicer to just implement
> > the reset as just that "a reset of the current state", and leave
> > the "let's play fast and loose with kmalloc() failures" to the
> > drivers that want it.
> 
> I'm sorry, but I have no idea what you mean by that. It's using the
> exact same pattern we've been using since forever for every driver
> (except, apparently, i915).

The reset() stuff was added specifically to deal with drivers
that didn't have readout. i915 has always had full readout and
never needed it.

> What would you like to change to that pattern to accomodate i915
> requirements?

I think the reset should be ideally done without reallocation,
or perhaps just don't implement reset for the things that don't
need it (mst and tunneling in this case).

-- 
Ville Syrjälä
Intel

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Maxime Ripard]

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

On Wed, Oct 08, 2025 at 07:12:28PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 08, 2025 at 04:53:20PM +0200, Maxime Ripard wrote:
> > On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > > > The DP MST implementation relies on a drm_private_obj, that is
> > > > initialized by allocating and initializing a state, and then passing it
> > > > to drm_private_obj_init.
> > > > 
> > > > Since we're gradually moving away from that pattern to the more
> > > > established one relying on a reset implementation, let's migrate this
> > > > instance to the new pattern.
> > > > 
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> > > >  1 file changed, 26 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> > > >  
> > > >  	kfree(mst_state->commit_deps);
> > > >  	kfree(mst_state);
> > > >  }
> > > >  
> > > > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > > > +{
> > > > +	struct drm_dp_mst_topology_mgr *mgr =
> > > > +		to_dp_mst_topology_mgr(obj);
> > > > +	struct drm_dp_mst_topology_state *mst_state;
> > > > +
> > > > +	if (obj->state) {
> > > > +		drm_dp_mst_destroy_state(obj, obj->state);
> > > > +		obj->state = NULL;
> > > 
> > > I'm not a big fan of this "free+reallocate without any way to handle
> > > failures" pattern.
> > > 
> > > Currently i915 does not do any of this, and so there are no unhandled
> > > points of failure. But the mst and tunneling changes here force it on
> > > i915 as well.
> > 
> > A very theoretical point of failure. If I'm counting right,
> > drm_dp_mst_topology_state takes 72 bytes. Any GFP_KERNEL allocation
> > of less than 8 pages cannot fail.
> 
> Until you start to inject failures, or you blow up the size of the
> state. You wouldn't think to check for potential NULL pointer
> dereferences before doing that, nor should you have to.
> 
> I'd rather not leave potential footguns lying around intentionally.

I'm not sure it's reasonable to expect a structure to grow by three
orders of magnitude, but we can add static build checks on the size of
the structure if it makes you more comfortable.

> > So, sure, it's less satisfying to look at, but that's about it. It's
> > just as safe and reliable.
> > 
> > > I think for the common things it would be nicer to just implement
> > > the reset as just that "a reset of the current state", and leave
> > > the "let's play fast and loose with kmalloc() failures" to the
> > > drivers that want it.
> > 
> > I'm sorry, but I have no idea what you mean by that. It's using the
> > exact same pattern we've been using since forever for every driver
> > (except, apparently, i915).
> 
> The reset() stuff was added specifically to deal with drivers
> that didn't have readout. i915 has always had full readout and
> never needed it.
> 
> > What would you like to change to that pattern to accomodate i915
> > requirements?
> 
> I think the reset should be ideally done without reallocation,
> or perhaps just don't implement reset for the things that don't
> need it (mst and tunneling in this case).

I'll be leaving aside i915 because it's quite far from actually using
all that infrastructure. From what I understand, your main concern is
that the drm_private_obj would be reset during a suspend, and the DP
objs should actually persist.

Thomas has been floating the idea recently to create a new helper to
supersede reset which would only reset the state itself, and not the
hardware. If we're doing that split, I guess it would mean that we would
need a new callback to allocate the initial state, since reset does both.

Would you feel better about creating an atomic_create_state hook or
something?

Maxime

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

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Ville Syrjälä]

On Thu, Oct 09, 2025 at 04:42:15PM +0200, Maxime Ripard wrote:
> On Wed, Oct 08, 2025 at 07:12:28PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 08, 2025 at 04:53:20PM +0200, Maxime Ripard wrote:
> > > On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > > > > The DP MST implementation relies on a drm_private_obj, that is
> > > > > initialized by allocating and initializing a state, and then passing it
> > > > > to drm_private_obj_init.
> > > > > 
> > > > > Since we're gradually moving away from that pattern to the more
> > > > > established one relying on a reset implementation, let's migrate this
> > > > > instance to the new pattern.
> > > > > 
> > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > ---
> > > > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> > > > >  1 file changed, 26 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > > > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> > > > >  
> > > > >  	kfree(mst_state->commit_deps);
> > > > >  	kfree(mst_state);
> > > > >  }
> > > > >  
> > > > > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > > > > +{
> > > > > +	struct drm_dp_mst_topology_mgr *mgr =
> > > > > +		to_dp_mst_topology_mgr(obj);
> > > > > +	struct drm_dp_mst_topology_state *mst_state;
> > > > > +
> > > > > +	if (obj->state) {
> > > > > +		drm_dp_mst_destroy_state(obj, obj->state);
> > > > > +		obj->state = NULL;
> > > > 
> > > > I'm not a big fan of this "free+reallocate without any way to handle
> > > > failures" pattern.
> > > > 
> > > > Currently i915 does not do any of this, and so there are no unhandled
> > > > points of failure. But the mst and tunneling changes here force it on
> > > > i915 as well.
> > > 
> > > A very theoretical point of failure. If I'm counting right,
> > > drm_dp_mst_topology_state takes 72 bytes. Any GFP_KERNEL allocation
> > > of less than 8 pages cannot fail.
> > 
> > Until you start to inject failures, or you blow up the size of the
> > state. You wouldn't think to check for potential NULL pointer
> > dereferences before doing that, nor should you have to.
> > 
> > I'd rather not leave potential footguns lying around intentionally.
> 
> I'm not sure it's reasonable to expect a structure to grow by three
> orders of magnitude, but we can add static build checks on the size of
> the structure if it makes you more comfortable.

I'm not really talking about any specific state structures here,
but in general. This thing should be robust even if someone needs
to add a private object with a ginormouse state.

> 
> > > So, sure, it's less satisfying to look at, but that's about it. It's
> > > just as safe and reliable.
> > > 
> > > > I think for the common things it would be nicer to just implement
> > > > the reset as just that "a reset of the current state", and leave
> > > > the "let's play fast and loose with kmalloc() failures" to the
> > > > drivers that want it.
> > > 
> > > I'm sorry, but I have no idea what you mean by that. It's using the
> > > exact same pattern we've been using since forever for every driver
> > > (except, apparently, i915).
> > 
> > The reset() stuff was added specifically to deal with drivers
> > that didn't have readout. i915 has always had full readout and
> > never needed it.
> > 
> > > What would you like to change to that pattern to accomodate i915
> > > requirements?
> > 
> > I think the reset should be ideally done without reallocation,
> > or perhaps just don't implement reset for the things that don't
> > need it (mst and tunneling in this case).
> 
> I'll be leaving aside i915 because it's quite far from actually using
> all that infrastructure. From what I understand, your main concern is
> that the drm_private_obj would be reset during a suspend, and the DP
> objs should actually persist.

With my i915 hat on, yes. That would be a functional change
you're introducing here that may or may not be good.

But in general I think simply getting rid of that unchecked 
kmalloc() would be a good idea. There's no real reason to keep it
other than being lazy and not implementing the state reset without
it.

> 
> Thomas has been floating the idea recently to create a new helper to
> supersede reset which would only reset the state itself, and not the
> hardware. If we're doing that split, I guess it would mean that we would
> need a new callback to allocate the initial state, since reset does both.
> 
> Would you feel better about creating an atomic_create_state hook or
> something?

I suppose you could add a hook for it. Sort of the initial version
of .duplicate_state() I guess.

Doesn't really need a hook though when you could just either just:
- pass the state to the init (which I guess was exactly what private
  objs did, but other object types didn't do)
- add a new function that just sets the obj->state pointer, if you
  want to hide that from the drivers a bit more
- just set the obj->state pointer by hand (which we do eg. in i915
  intel_crtc_alloc())

Or did you want to use this also after the object init somewhere?
.duplicate_state() is often implemented with a memdup() so wouldn't
really help there at least.

-- 
Ville Syrjälä
Intel

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Imre Deak]

On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > The DP MST implementation relies on a drm_private_obj, that is
> > initialized by allocating and initializing a state, and then passing it
> > to drm_private_obj_init.
> > 
> > Since we're gradually moving away from that pattern to the more
> > established one relying on a reset implementation, let's migrate this
> > instance to the new pattern.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> >  1 file changed, 26 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> >  
> >  	kfree(mst_state->commit_deps);
> >  	kfree(mst_state);
> >  }
> >  
> > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > +{
> > +	struct drm_dp_mst_topology_mgr *mgr =
> > +		to_dp_mst_topology_mgr(obj);
> > +	struct drm_dp_mst_topology_state *mst_state;
> > +
> > +	if (obj->state) {
> > +		drm_dp_mst_destroy_state(obj, obj->state);
> > +		obj->state = NULL;
> 
> I'm not a big fan of this "free+reallocate without any way to handle
> failures" pattern.
> 
> Currently i915 does not do any of this, and so there are no unhandled
> points of failure. But the mst and tunneling changes here force it
> on i915 as well.
> 
> I think for the common things it would be nicer to just implement
> the reset as just that "a reset of the current state", and leave
> the "let's play fast and loose with kmalloc() failures" to the
> drivers that want it.
> 
> That said I haven't even thought through whether this mst and
> tunneling state reset might have some undesired side effects
> since we previously did none of it. I suspect it should be fine,
> but at least the mst code does some questionable things with
> the state so not 100% sure.
> 
> Imre, do you recall if we might somehow depend on preserving
> the state across drm_mode_config_reset()?

Yes, the stream payload info in the MST state and the stream BW info in
the tunnel state is computed for the active streams (i.e. CRTCs) before
suspend and this info is reused after resume. These active streams must
be restored to their pre-suspend state after resume, which will need the
above payload/BW info.

The restore should happen without recomputing the state for CRTCs, so
the payload/BW info should be also preserved across suspend/resume.

crtc/plane/connector objects which have the reset semantic added now in
this patch for private objects do preserve their state across
suspend/resume, see drm_atomic_helper_duplicate_state() and
drm_atomic_helper_commit_duplicated_state().

> > +	}
> > +
> > +	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> > +	if (!mst_state)
> > +		return;
> > +
> > +	__drm_atomic_helper_private_obj_reset(obj, &mst_state->base);
> > +
> > +	mst_state->total_avail_slots = 63;
> > +	mst_state->start_slot = 1;
> > +
> > +	mst_state->mgr = mgr;
> > +	INIT_LIST_HEAD(&mst_state->payloads);
> > +}
> > +
> >  static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
> >  						 struct drm_dp_mst_branch *branch)
> >  {
> >  	while (port->parent) {
> >  		if (port->parent == branch)
> > @@ -5619,10 +5643,11 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state)
> >  EXPORT_SYMBOL(drm_dp_mst_atomic_check);
> >  
> >  const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
> >  	.atomic_duplicate_state = drm_dp_mst_duplicate_state,
> >  	.atomic_destroy_state = drm_dp_mst_destroy_state,
> > +	.reset = drm_dp_mst_reset,
> >  };
> >  EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
> >  
> >  /**
> >   * drm_atomic_get_mst_topology_state: get MST topology state
> > @@ -5705,12 +5730,10 @@ EXPORT_SYMBOL(drm_atomic_get_new_mst_topology_state);
> >  int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
> >  				 struct drm_device *dev, struct drm_dp_aux *aux,
> >  				 int max_dpcd_transaction_bytes, int max_payloads,
> >  				 int conn_base_id)
> >  {
> > -	struct drm_dp_mst_topology_state *mst_state;
> > -
> >  	mutex_init(&mgr->lock);
> >  	mutex_init(&mgr->qlock);
> >  	mutex_init(&mgr->delayed_destroy_lock);
> >  	mutex_init(&mgr->up_req_lock);
> >  	mutex_init(&mgr->probe_lock);
> > @@ -5740,22 +5763,12 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
> >  	mgr->aux = aux;
> >  	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
> >  	mgr->max_payloads = max_payloads;
> >  	mgr->conn_base_id = conn_base_id;
> >  
> > -	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> > -	if (mst_state == NULL)
> > -		return -ENOMEM;
> > -
> > -	mst_state->total_avail_slots = 63;
> > -	mst_state->start_slot = 1;
> > -
> > -	mst_state->mgr = mgr;
> > -	INIT_LIST_HEAD(&mst_state->payloads);
> > -
> >  	drm_atomic_private_obj_init(dev, &mgr->base,
> > -				    &mst_state->base,
> > +				    NULL,
> >  				    &drm_dp_mst_topology_state_funcs);
> >  
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
> > 
> > -- 
> > 2.51.0
> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 07:24:15PM +0300, Imre Deak wrote:
> On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > > The DP MST implementation relies on a drm_private_obj, that is
> > > initialized by allocating and initializing a state, and then passing it
> > > to drm_private_obj_init.
> > > 
> > > Since we're gradually moving away from that pattern to the more
> > > established one relying on a reset implementation, let's migrate this
> > > instance to the new pattern.
> > > 
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> > >  1 file changed, 26 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> > >  
> > >  	kfree(mst_state->commit_deps);
> > >  	kfree(mst_state);
> > >  }
> > >  
> > > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > > +{
> > > +	struct drm_dp_mst_topology_mgr *mgr =
> > > +		to_dp_mst_topology_mgr(obj);
> > > +	struct drm_dp_mst_topology_state *mst_state;
> > > +
> > > +	if (obj->state) {
> > > +		drm_dp_mst_destroy_state(obj, obj->state);
> > > +		obj->state = NULL;
> > 
> > I'm not a big fan of this "free+reallocate without any way to handle
> > failures" pattern.
> > 
> > Currently i915 does not do any of this, and so there are no unhandled
> > points of failure. But the mst and tunneling changes here force it
> > on i915 as well.
> > 
> > I think for the common things it would be nicer to just implement
> > the reset as just that "a reset of the current state", and leave
> > the "let's play fast and loose with kmalloc() failures" to the
> > drivers that want it.
> > 
> > That said I haven't even thought through whether this mst and
> > tunneling state reset might have some undesired side effects
> > since we previously did none of it. I suspect it should be fine,
> > but at least the mst code does some questionable things with
> > the state so not 100% sure.
> > 
> > Imre, do you recall if we might somehow depend on preserving
> > the state across drm_mode_config_reset()?
> 
> Yes, the stream payload info in the MST state and the stream BW info in
> the tunnel state is computed for the active streams (i.e. CRTCs) before
> suspend and this info is reused after resume. These active streams must
> be restored to their pre-suspend state after resume, which will need the
> above payload/BW info.
> 
> The restore should happen without recomputing the state for CRTCs, so
> the payload/BW info should be also preserved across suspend/resume.
> 
> crtc/plane/connector objects which have the reset semantic added now in
> this patch for private objects do preserve their state across
> suspend/resume, see drm_atomic_helper_duplicate_state() and
> drm_atomic_helper_commit_duplicated_state().

Doesn't this mean that we should implement handling of private objects
in those functions too? E.g. we track resource allocation in the private
object. It should also be restored to exactly the same state as it was
before suspend.


-- 
With best wishes
Dmitry

Re: [PATCH 05/16] drm/dp_mst: Switch private_obj initialization to reset

[Imre Deak]

On Wed, Oct 15, 2025 at 01:35:47AM +0300, Dmitry Baryshkov wrote:
> On Wed, Oct 08, 2025 at 07:24:15PM +0300, Imre Deak wrote:
> > On Wed, Oct 08, 2025 at 05:06:43PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 08, 2025 at 02:04:03PM +0200, Maxime Ripard wrote:
> > > > The DP MST implementation relies on a drm_private_obj, that is
> > > > initialized by allocating and initializing a state, and then passing it
> > > > to drm_private_obj_init.
> > > > 
> > > > Since we're gradually moving away from that pattern to the more
> > > > established one relying on a reset implementation, let's migrate this
> > > > instance to the new pattern.
> > > > 
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++++++++++++++++++---------
> > > >  1 file changed, 26 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > index 64e5c176d5cce9df9314f77a0b4c97662c30c070..255fbdcea9f0b6376d15439e3da1dc02be472a20 100644
> > > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > @@ -5181,10 +5181,34 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> > > >  
> > > >  	kfree(mst_state->commit_deps);
> > > >  	kfree(mst_state);
> > > >  }
> > > >  
> > > > +static void drm_dp_mst_reset(struct drm_private_obj *obj)
> > > > +{
> > > > +	struct drm_dp_mst_topology_mgr *mgr =
> > > > +		to_dp_mst_topology_mgr(obj);
> > > > +	struct drm_dp_mst_topology_state *mst_state;
> > > > +
> > > > +	if (obj->state) {
> > > > +		drm_dp_mst_destroy_state(obj, obj->state);
> > > > +		obj->state = NULL;
> > > 
> > > I'm not a big fan of this "free+reallocate without any way to handle
> > > failures" pattern.
> > > 
> > > Currently i915 does not do any of this, and so there are no unhandled
> > > points of failure. But the mst and tunneling changes here force it
> > > on i915 as well.
> > > 
> > > I think for the common things it would be nicer to just implement
> > > the reset as just that "a reset of the current state", and leave
> > > the "let's play fast and loose with kmalloc() failures" to the
> > > drivers that want it.
> > > 
> > > That said I haven't even thought through whether this mst and
> > > tunneling state reset might have some undesired side effects
> > > since we previously did none of it. I suspect it should be fine,
> > > but at least the mst code does some questionable things with
> > > the state so not 100% sure.
> > > 
> > > Imre, do you recall if we might somehow depend on preserving
> > > the state across drm_mode_config_reset()?
> > 
> > Yes, the stream payload info in the MST state and the stream BW info in
> > the tunnel state is computed for the active streams (i.e. CRTCs) before
> > suspend and this info is reused after resume. These active streams must
> > be restored to their pre-suspend state after resume, which will need the
> > above payload/BW info.
> > 
> > The restore should happen without recomputing the state for CRTCs, so
> > the payload/BW info should be also preserved across suspend/resume.
> > 
> > crtc/plane/connector objects which have the reset semantic added now in
> > this patch for private objects do preserve their state across
> > suspend/resume, see drm_atomic_helper_duplicate_state() and
> > drm_atomic_helper_commit_duplicated_state().
> 
> Doesn't this mean that we should implement handling of private objects
> in those functions too? E.g. we track resource allocation in the private
> object. It should also be restored to exactly the same state as it was
> before suspend.

Yes, I think so. Ville mentioned that someone has sent already patches
to that extent. Note that, things also do work atm without the
save/restore for the mst, tunnel private objects, at least for i915,
since everything does get recomputed during resume: the effectively
reset/zeroed private object state (due to the last disabling modeset
during suspend) will get recomputed from other states (crtc state, sink
capabilities) during resume. However this dependency on the other states
should be removed eventually and then saving/restoring the private obj
state would be required.

> -- 
> With best wishes
> Dmitry

[PATCH 06/16] drm/dp_tunnel: Switch private_obj initialization to reset

[Maxime Ripard]

The DP tunnel implementation relies on a drm_private_obj, that is
initialized by allocating and initializing a state, and then passing it
to drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/display/drm_dp_tunnel.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_tunnel.c b/drivers/gpu/drm/display/drm_dp_tunnel.c
index 43f13a7c79b931beb230f8afe20afa0ebcf5ed8d..ae6c6ca99417bf7d99186dd2648f0525c633a8d5 100644
--- a/drivers/gpu/drm/display/drm_dp_tunnel.c
+++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
@@ -1495,13 +1495,31 @@ tunnel_group_duplicate_state(struct drm_private_obj *obj)
 static void tunnel_group_destroy_state(struct drm_private_obj *obj, struct drm_private_state *state)
 {
 	free_group_state(to_group_state(state));
 }
 
+static void tunnel_group_reset(struct drm_private_obj *obj)
+{
+	struct drm_dp_tunnel_group_state *group_state;
+
+	if (obj->state) {
+		tunnel_group_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	group_state = kzalloc(sizeof(*group_state), GFP_KERNEL);
+	if (!group_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &group_state->base);
+	INIT_LIST_HEAD(&group_state->tunnel_states);
+}
+
 static const struct drm_private_state_funcs tunnel_group_funcs = {
 	.atomic_duplicate_state = tunnel_group_duplicate_state,
 	.atomic_destroy_state = tunnel_group_destroy_state,
+	.reset = tunnel_group_reset,
 };
 
 /**
  * drm_dp_tunnel_atomic_get_state - get/allocate the new atomic state for a tunnel
  * @state: Atomic state
@@ -1579,23 +1597,15 @@ drm_dp_tunnel_atomic_get_new_state(struct drm_atomic_state *state,
 }
 EXPORT_SYMBOL(drm_dp_tunnel_atomic_get_new_state);
 
 static bool init_group(struct drm_dp_tunnel_mgr *mgr, struct drm_dp_tunnel_group *group)
 {
-	struct drm_dp_tunnel_group_state *group_state;
-
-	group_state = kzalloc(sizeof(*group_state), GFP_KERNEL);
-	if (!group_state)
-		return false;
-
-	INIT_LIST_HEAD(&group_state->tunnel_states);
-
 	group->mgr = mgr;
 	group->available_bw = -1;
 	INIT_LIST_HEAD(&group->tunnels);
 
-	drm_atomic_private_obj_init(mgr->dev, &group->base, &group_state->base,
+	drm_atomic_private_obj_init(mgr->dev, &group->base, NULL,
 				    &tunnel_group_funcs);
 
 	return true;
 }
 

-- 
2.51.0

[PATCH 07/16] drm/amdgpu: Switch private_obj initialization to reset

[Maxime Ripard]

The amdgpu driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <siqueira@igalia.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 54 +++++++++++++----------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 62defeccbb5ca09c89523fc4112d2085bbdbb0a9..1bcbfd814d53bb443b7503ffacb109c900b67b5f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4675,18 +4675,43 @@ static void dm_atomic_destroy_state(struct drm_private_obj *obj,
 		dc_state_release(dm_state->context);
 
 	kfree(dm_state);
 }
 
+static void dm_atomic_reset(struct drm_private_obj *obj)
+{
+	struct amdgpu_device *adev = drm_to_adev(obj->dev);
+	struct dm_atomic_state *dm_state;
+	struct dc_state *context;
+
+	if (obj->state) {
+		dm_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	dm_state = kzalloc(sizeof(*dm_state), GFP_KERNEL);
+	if (!dm_state)
+		return;
+
+	context = dc_state_create_current_copy(adev->dm.dc);
+	if (!context) {
+		kfree(dm_state);
+		return;
+	}
+
+	__drm_atomic_helper_private_obj_reset(obj, &dm_state->base);
+	dm_state->context = context;
+}
+
 static struct drm_private_state_funcs dm_atomic_state_funcs = {
 	.atomic_duplicate_state = dm_atomic_duplicate_state,
 	.atomic_destroy_state = dm_atomic_destroy_state,
+	.reset = dm_atomic_reset,
 };
 
 static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
 {
-	struct dm_atomic_state *state;
 	int r;
 
 	adev->mode_info.mode_config_initialized = true;
 
 	adev_to_drm(adev)->mode_config.funcs = (void *)&amdgpu_dm_mode_funcs;
@@ -4702,46 +4727,27 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
 	else
 		adev_to_drm(adev)->mode_config.prefer_shadow = 1;
 	/* indicates support for immediate flip */
 	adev_to_drm(adev)->mode_config.async_page_flip = true;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
-	state->context = dc_state_create_current_copy(adev->dm.dc);
-	if (!state->context) {
-		kfree(state);
-		return -ENOMEM;
-	}
-
 	drm_atomic_private_obj_init(adev_to_drm(adev),
 				    &adev->dm.atomic_obj,
-				    &state->base,
+				    NULL,
 				    &dm_atomic_state_funcs);
 
 	r = amdgpu_display_modeset_create_props(adev);
-	if (r) {
-		dc_state_release(state->context);
-		kfree(state);
+	if (r)
 		return r;
-	}
 
 #ifdef AMD_PRIVATE_COLOR
-	if (amdgpu_dm_create_color_properties(adev)) {
-		dc_state_release(state->context);
-		kfree(state);
+	if (amdgpu_dm_create_color_properties(adev))
 		return -ENOMEM;
-	}
 #endif
 
 	r = amdgpu_dm_audio_init(adev);
-	if (r) {
-		dc_state_release(state->context);
-		kfree(state);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
 
 #define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12

-- 
2.51.0

[PATCH 08/16] drm/arm: komeda: Switch private_obj initialization to reset

[Maxime Ripard]

The ARM komeda driver relies on a number of drm_private_objs, that are
initialized by allocating and initializing a state, and then passing it
to drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Liviu Dudau <liviu.dudau@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_pipeline.h   |   2 +
 .../drm/arm/display/komeda/komeda_private_obj.c    | 232 +++++++++++++++------
 2 files changed, 170 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index ac8725e248537e5737d16cd36860401c42073500..37b9e92202443cc72adc0666ed047d4f77d79782 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -126,10 +126,12 @@ struct komeda_component {
 	 * @funcs: chip functions to access HW
 	 */
 	const struct komeda_component_funcs *funcs;
 };
 
+#define to_component(o)	container_of(o, struct komeda_component, obj)
+
 /**
  * struct komeda_component_output
  *
  * a component has multiple outputs, if want to know where the data
  * comes from, only know the component is not enough, we still need to know
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
index 914400c4af73824e52dda76425a73a74e681a146..0c7b7a5dd10900016f64df2a86d54f9178d1cf5e 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
@@ -38,26 +38,39 @@ komeda_layer_atomic_destroy_state(struct drm_private_obj *obj,
 	struct komeda_layer_state *st = to_layer_st(priv_to_comp_st(state));
 
 	kfree(st);
 }
 
+static void
+komeda_layer_reset(struct drm_private_obj *obj)
+{
+	struct komeda_layer_state *st;
+
+	if (obj->state) {
+		komeda_layer_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_layer_obj_funcs = {
 	.atomic_duplicate_state	= komeda_layer_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_layer_atomic_destroy_state,
+	.reset			= komeda_layer_reset,
 };
 
 static int komeda_layer_obj_add(struct komeda_kms_dev *kms,
 				struct komeda_layer *layer)
 {
-	struct komeda_layer_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &layer->base;
-	drm_atomic_private_obj_init(&kms->base, &layer->base.obj, &st->base.obj,
+	drm_atomic_private_obj_init(&kms->base, &layer->base.obj, NULL,
 				    &komeda_layer_obj_funcs);
 	return 0;
 }
 
 static struct drm_private_state *
@@ -80,27 +93,40 @@ komeda_scaler_atomic_destroy_state(struct drm_private_obj *obj,
 				   struct drm_private_state *state)
 {
 	kfree(to_scaler_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_scaler_reset(struct drm_private_obj *obj)
+{
+	struct komeda_scaler_state *st;
+
+	if (obj->state) {
+		komeda_scaler_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_scaler_obj_funcs = {
 	.atomic_duplicate_state	= komeda_scaler_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_scaler_atomic_destroy_state,
+	.reset			= komeda_scaler_reset,
 };
 
 static int komeda_scaler_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_scaler *scaler)
 {
-	struct komeda_scaler_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &scaler->base;
 	drm_atomic_private_obj_init(&kms->base,
-				    &scaler->base.obj, &st->base.obj,
+				    &scaler->base.obj, NULL,
 				    &komeda_scaler_obj_funcs);
 	return 0;
 }
 
 static struct drm_private_state *
@@ -123,26 +149,39 @@ komeda_compiz_atomic_destroy_state(struct drm_private_obj *obj,
 				   struct drm_private_state *state)
 {
 	kfree(to_compiz_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_compiz_reset(struct drm_private_obj *obj)
+{
+	struct komeda_compiz_state *st;
+
+	if (obj->state) {
+		komeda_compiz_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_compiz_obj_funcs = {
 	.atomic_duplicate_state	= komeda_compiz_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_compiz_atomic_destroy_state,
+	.reset			= komeda_compiz_reset,
 };
 
 static int komeda_compiz_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_compiz *compiz)
 {
-	struct komeda_compiz_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &compiz->base;
-	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj, &st->base.obj,
+	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj, NULL,
 				    &komeda_compiz_obj_funcs);
 
 	return 0;
 }
 
@@ -166,27 +205,40 @@ komeda_splitter_atomic_destroy_state(struct drm_private_obj *obj,
 				     struct drm_private_state *state)
 {
 	kfree(to_splitter_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_splitter_reset(struct drm_private_obj *obj)
+{
+	struct komeda_splitter_state *st;
+
+	if (obj->state) {
+		komeda_splitter_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_splitter_obj_funcs = {
 	.atomic_duplicate_state	= komeda_splitter_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_splitter_atomic_destroy_state,
+	.reset			= komeda_splitter_reset,
 };
 
 static int komeda_splitter_obj_add(struct komeda_kms_dev *kms,
 				   struct komeda_splitter *splitter)
 {
-	struct komeda_splitter_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &splitter->base;
 	drm_atomic_private_obj_init(&kms->base,
-				    &splitter->base.obj, &st->base.obj,
+				    &splitter->base.obj, NULL,
 				    &komeda_splitter_obj_funcs);
 
 	return 0;
 }
 
@@ -209,27 +261,40 @@ static void komeda_merger_atomic_destroy_state(struct drm_private_obj *obj,
 					       struct drm_private_state *state)
 {
 	kfree(to_merger_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_merger_reset(struct drm_private_obj *obj)
+{
+	struct komeda_merger_state *st;
+
+	if (obj->state) {
+		komeda_merger_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_merger_obj_funcs = {
 	.atomic_duplicate_state	= komeda_merger_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_merger_atomic_destroy_state,
+	.reset			= komeda_merger_reset,
 };
 
 static int komeda_merger_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_merger *merger)
 {
-	struct komeda_merger_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &merger->base;
 	drm_atomic_private_obj_init(&kms->base,
-				    &merger->base.obj, &st->base.obj,
+				    &merger->base.obj, NULL,
 				    &komeda_merger_obj_funcs);
 
 	return 0;
 }
 
@@ -253,26 +318,39 @@ komeda_improc_atomic_destroy_state(struct drm_private_obj *obj,
 				   struct drm_private_state *state)
 {
 	kfree(to_improc_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_improc_reset(struct drm_private_obj *obj)
+{
+	struct komeda_improc_state *st;
+
+	if (obj->state) {
+		komeda_improc_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_improc_obj_funcs = {
 	.atomic_duplicate_state	= komeda_improc_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_improc_atomic_destroy_state,
+	.reset			= komeda_improc_reset,
 };
 
 static int komeda_improc_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_improc *improc)
 {
-	struct komeda_improc_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &improc->base;
-	drm_atomic_private_obj_init(&kms->base, &improc->base.obj, &st->base.obj,
+	drm_atomic_private_obj_init(&kms->base, &improc->base.obj, NULL,
 				    &komeda_improc_obj_funcs);
 
 	return 0;
 }
 
@@ -296,26 +374,39 @@ komeda_timing_ctrlr_atomic_destroy_state(struct drm_private_obj *obj,
 					 struct drm_private_state *state)
 {
 	kfree(to_ctrlr_st(priv_to_comp_st(state)));
 }
 
+static void
+komeda_timing_ctrlr_reset(struct drm_private_obj *obj)
+{
+	struct komeda_timing_ctrlr_state *st;
+
+	if (obj->state) {
+		komeda_timing_ctrlr_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->base.obj);
+	komeda_component_state_reset(&st->base);
+	st->base.component = to_component(obj);
+}
+
 static const struct drm_private_state_funcs komeda_timing_ctrlr_obj_funcs = {
 	.atomic_duplicate_state	= komeda_timing_ctrlr_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_timing_ctrlr_atomic_destroy_state,
+	.reset			= komeda_timing_ctrlr_reset,
 };
 
 static int komeda_timing_ctrlr_obj_add(struct komeda_kms_dev *kms,
 				       struct komeda_timing_ctrlr *ctrlr)
 {
-	struct komeda_compiz_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->base.component = &ctrlr->base;
-	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj, &st->base.obj,
+	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj, NULL,
 				    &komeda_timing_ctrlr_obj_funcs);
 
 	return 0;
 }
 
@@ -340,26 +431,39 @@ komeda_pipeline_atomic_destroy_state(struct drm_private_obj *obj,
 				     struct drm_private_state *state)
 {
 	kfree(priv_to_pipe_st(state));
 }
 
+static void
+komeda_pipeline_reset(struct drm_private_obj *obj)
+{
+	struct komeda_pipeline_state *st;
+
+	if (obj->state) {
+		komeda_pipeline_atomic_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &st->obj);
+	st->active_comps = 0;
+	st->pipe = container_of(obj, struct komeda_pipeline, obj);
+}
+
 static const struct drm_private_state_funcs komeda_pipeline_obj_funcs = {
 	.atomic_duplicate_state	= komeda_pipeline_atomic_duplicate_state,
 	.atomic_destroy_state	= komeda_pipeline_atomic_destroy_state,
+	.reset			= komeda_pipeline_reset,
 };
 
 static int komeda_pipeline_obj_add(struct komeda_kms_dev *kms,
 				   struct komeda_pipeline *pipe)
 {
-	struct komeda_pipeline_state *st;
-
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
-	if (!st)
-		return -ENOMEM;
-
-	st->pipe = pipe;
-	drm_atomic_private_obj_init(&kms->base, &pipe->obj, &st->obj,
+	drm_atomic_private_obj_init(&kms->base, &pipe->obj, NULL,
 				    &komeda_pipeline_obj_funcs);
 
 	return 0;
 }
 

-- 
2.51.0

[PATCH 09/16] drm/ingenic: Switch private_obj initialization to reset

[Maxime Ripard]

The ingenic driver relies on two drm_private_objs, that are initialized
by allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Paul Cercueil <paul@crapouillou.net>
Cc: linux-mips@vger.kernel.org
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 30 +++++++++++++++++++-----------
 drivers/gpu/drm/ingenic/ingenic-ipu.c     | 30 ++++++++++++++++++------------
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 9db1ceaed5188a4ef0897280dc72108eb3815b5f..18f20d96f6e4a7d9e5209ee770c7b4fc81adbad7 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -947,10 +947,26 @@ static void ingenic_drm_destroy_state(struct drm_private_obj *obj,
 	struct ingenic_drm_private_state *priv_state = to_ingenic_drm_priv_state(state);
 
 	kfree(priv_state);
 }
 
+static void ingenic_drm_reset(struct drm_private_obj *obj)
+{
+	struct ingenic_drm_private_state *priv_state;
+
+	if (obj->state) {
+		ingenic_drm_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	priv_state = kzalloc(sizeof(*priv_state), GFP_KERNEL);
+	if (!priv_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &priv_state->base);
+}
+
 DEFINE_DRM_GEM_DMA_FOPS(ingenic_drm_fops);
 
 static const struct drm_driver ingenic_drm_driver_data = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 	.name			= "ingenic-drm",
@@ -1029,10 +1045,11 @@ static struct drm_mode_config_helper_funcs ingenic_drm_mode_config_helpers = {
 };
 
 static const struct drm_private_state_funcs ingenic_drm_private_state_funcs = {
 	.atomic_duplicate_state = ingenic_drm_duplicate_state,
 	.atomic_destroy_state = ingenic_drm_destroy_state,
+	.reset = ingenic_drm_reset,
 };
 
 static void ingenic_drm_unbind_all(void *d)
 {
 	struct ingenic_drm *priv = d;
@@ -1080,11 +1097,10 @@ static void ingenic_drm_atomic_private_obj_fini(struct drm_device *drm, void *pr
 }
 
 static int ingenic_drm_bind(struct device *dev, bool has_components)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	struct ingenic_drm_private_state *private_state;
 	const struct jz_soc_info *soc_info;
 	struct ingenic_drm *priv;
 	struct clk *parent_clk;
 	struct drm_plane *primary;
 	struct drm_bridge *bridge;
@@ -1380,23 +1396,17 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	if (ret) {
 		dev_err(dev, "Unable to register clock notifier\n");
 		goto err_devclk_disable;
 	}
 
-	private_state = kzalloc(sizeof(*private_state), GFP_KERNEL);
-	if (!private_state) {
-		ret = -ENOMEM;
-		goto err_clk_notifier_unregister;
-	}
-
-	drm_atomic_private_obj_init(drm, &priv->private_obj, &private_state->base,
+	drm_atomic_private_obj_init(drm, &priv->private_obj, NULL,
 				    &ingenic_drm_private_state_funcs);
 
 	ret = drmm_add_action_or_reset(drm, ingenic_drm_atomic_private_obj_fini,
 				       &priv->private_obj);
 	if (ret)
-		goto err_private_state_free;
+		goto err_clk_notifier_unregister;
 
 	ret = drm_dev_register(drm, 0);
 	if (ret) {
 		dev_err(dev, "Failed to register DRM driver\n");
 		goto err_clk_notifier_unregister;
@@ -1404,12 +1414,10 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 	drm_client_setup(drm, NULL);
 
 	return 0;
 
-err_private_state_free:
-	kfree(private_state);
 err_clk_notifier_unregister:
 	clk_notifier_unregister(parent_clk, &priv->clock_nb);
 err_devclk_disable:
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 26ebf424d63ec21ccee80221745c3e8bcc6b3d7f..ddaf80052f03b8e366c89a6562b430a400b6dacd 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -748,13 +748,30 @@ static void ingenic_ipu_destroy_state(struct drm_private_obj *obj,
 	struct ingenic_ipu_private_state *priv_state = to_ingenic_ipu_priv_state(state);
 
 	kfree(priv_state);
 }
 
+static void ingenic_ipu_reset(struct drm_private_obj *obj)
+{
+	struct ingenic_ipu_private_state *priv_state;
+
+	if (obj->state) {
+		ingenic_ipu_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	priv_state = kzalloc(sizeof(*priv_state), GFP_KERNEL);
+	if (!priv_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &priv_state->base);
+}
+
 static const struct drm_private_state_funcs ingenic_ipu_private_state_funcs = {
 	.atomic_duplicate_state = ingenic_ipu_duplicate_state,
 	.atomic_destroy_state = ingenic_ipu_destroy_state,
+	.reset = ingenic_ipu_reset,
 };
 
 static irqreturn_t ingenic_ipu_irq_handler(int irq, void *arg)
 {
 	struct ingenic_ipu *ipu = arg;
@@ -791,11 +808,10 @@ static const struct regmap_config ingenic_ipu_regmap_config = {
 };
 
 static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	struct ingenic_ipu_private_state *private_state;
 	const struct soc_info *soc_info;
 	struct drm_device *drm = d;
 	struct drm_plane *plane;
 	struct ingenic_ipu *ipu;
 	void __iomem *base;
@@ -885,24 +901,14 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
 	if (err) {
 		dev_err(dev, "Unable to prepare clock\n");
 		return err;
 	}
 
-	private_state = kzalloc(sizeof(*private_state), GFP_KERNEL);
-	if (!private_state) {
-		err = -ENOMEM;
-		goto err_clk_unprepare;
-	}
-
-	drm_atomic_private_obj_init(drm, &ipu->private_obj, &private_state->base,
+	drm_atomic_private_obj_init(drm, &ipu->private_obj, NULL,
 				    &ingenic_ipu_private_state_funcs);
 
 	return 0;
-
-err_clk_unprepare:
-	clk_unprepare(ipu->clk);
-	return err;
 }
 
 static void ingenic_ipu_unbind(struct device *dev,
 			       struct device *master, void *d)
 {

-- 
2.51.0

[PATCH 10/16] drm/msm: mdp5: Switch private_obj initialization to reset

[Maxime Ripard]

The MSM mdp5 driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Rob Clark <robin.clark@oss.qualcomm.com>
Cc: Dmitry Baryshkov <lumag@kernel.org>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 5b6ca8dd929e1870b7228af93da03886524f5f20..44aef7eb8e9073bc9a4bab03c1d6c41313c56ac7 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -112,10 +112,30 @@ static void mdp5_global_destroy_state(struct drm_private_obj *obj,
 	struct mdp5_global_state *mdp5_state = to_mdp5_global_state(state);
 
 	kfree(mdp5_state);
 }
 
+static void mdp5_global_reset(struct drm_private_obj *obj)
+{
+	struct drm_device *dev = obj->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
+	struct mdp5_global_state *mdp5_state;
+
+	if (obj->state) {
+		mdp5_global_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
+	if (!mdp5_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &mdp5_state->base);
+	mdp5_state->mdp5_kms = mdp5_kms;
+}
+
 static void mdp5_global_print_state(struct drm_printer *p,
 				    const struct drm_private_state *state)
 {
 	struct mdp5_global_state *mdp5_state = to_mdp5_global_state(state);
 
@@ -125,24 +145,17 @@ static void mdp5_global_print_state(struct drm_printer *p,
 
 static const struct drm_private_state_funcs mdp5_global_state_funcs = {
 	.atomic_duplicate_state = mdp5_global_duplicate_state,
 	.atomic_destroy_state = mdp5_global_destroy_state,
 	.atomic_print_state = mdp5_global_print_state,
+	.reset = mdp5_global_reset,
 };
 
 static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms)
 {
-	struct mdp5_global_state *state;
-
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
-	state->mdp5_kms = mdp5_kms;
-
 	drm_atomic_private_obj_init(mdp5_kms->dev, &mdp5_kms->glob_state,
-				    &state->base,
+				    NULL,
 				    &mdp5_global_state_funcs);
 	return 0;
 }
 
 static void mdp5_enable_commit(struct msm_kms *kms)

-- 
2.51.0

Re: [PATCH 10/16] drm/msm: mdp5: Switch private_obj initialization to reset

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:08PM +0200, Maxime Ripard wrote:
> The MSM mdp5 driver relies on a drm_private_obj, that is initialized by
> allocating and initializing a state, and then passing it to
> drm_private_obj_init.
> 
> Since we're gradually moving away from that pattern to the more
> established one relying on a reset implementation, let's migrate this
> instance to the new pattern.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---
> 
> Cc: Rob Clark <robin.clark@oss.qualcomm.com>
> Cc: Dmitry Baryshkov <lumag@kernel.org>
> Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
>  
>  static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms)

static void (up to you). Or just inline it.

>  {
> -	struct mdp5_global_state *state;
> -
> -	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (!state)
> -		return -ENOMEM;
> -
> -	state->mdp5_kms = mdp5_kms;
> -
>  	drm_atomic_private_obj_init(mdp5_kms->dev, &mdp5_kms->glob_state,
> -				    &state->base,
> +				    NULL,
>  				    &mdp5_global_state_funcs);
>  	return 0;
>  }
>  
>  static void mdp5_enable_commit(struct msm_kms *kms)
> 
> -- 
> 2.51.0
> 

-- 
With best wishes
Dmitry

[PATCH 11/16] drm/msm: dpu1: Switch private_obj initialization to reset

[Maxime Ripard]

The MSM dpu1 driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Rob Clark <robin.clark@oss.qualcomm.com>
Cc: Dmitry Baryshkov <lumag@kernel.org>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a306077647c317af9345eeff13082230906b5767..04baf072e8510ce8260f1ec609ba8f2b22f7c11e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -365,10 +365,30 @@ static void dpu_kms_global_destroy_state(struct drm_private_obj *obj,
 	struct dpu_global_state *dpu_state = to_dpu_global_state(state);
 
 	kfree(dpu_state);
 }
 
+static void dpu_kms_global_reset(struct drm_private_obj *obj)
+{
+	struct drm_device *dev = obj->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
+	struct dpu_global_state *dpu_state;
+
+	if (obj->state) {
+		dpu_kms_global_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	dpu_state = kzalloc(sizeof(*dpu_state), GFP_KERNEL);
+	if (!dpu_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &dpu_state->base);
+	dpu_state->rm = &dpu_kms->rm;
+}
+
 static void dpu_kms_global_print_state(struct drm_printer *p,
 				       const struct drm_private_state *state)
 {
 	const struct dpu_global_state *global_state = to_dpu_global_state(state);
 
@@ -377,26 +397,19 @@ static void dpu_kms_global_print_state(struct drm_printer *p,
 
 static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
 	.atomic_duplicate_state = dpu_kms_global_duplicate_state,
 	.atomic_destroy_state = dpu_kms_global_destroy_state,
 	.atomic_print_state = dpu_kms_global_print_state,
+	.reset = dpu_kms_global_reset,
 };
 
 static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
 {
-	struct dpu_global_state *state;
-
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
 	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
-				    &state->base,
+				    NULL,
 				    &dpu_kms_global_state_funcs);
 
-	state->rm = &dpu_kms->rm;
-
 	return 0;
 }
 
 static void dpu_kms_global_obj_fini(struct dpu_kms *dpu_kms)
 {

-- 
2.51.0

Re: [PATCH 11/16] drm/msm: dpu1: Switch private_obj initialization to reset

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:09PM +0200, Maxime Ripard wrote:
> The MSM dpu1 driver relies on a drm_private_obj, that is initialized by
> allocating and initializing a state, and then passing it to
> drm_private_obj_init.
> 
> Since we're gradually moving away from that pattern to the more
> established one relying on a reset implementation, let's migrate this
> instance to the new pattern.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---
> 
> Cc: Rob Clark <robin.clark@oss.qualcomm.com>
> Cc: Dmitry Baryshkov <lumag@kernel.org>
> Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
>  static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)

It can become static void now (up to you though).

>  {
> -	struct dpu_global_state *state;
> -
> -	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (!state)
> -		return -ENOMEM;
> -
>  	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
> -				    &state->base,
> +				    NULL,
>  				    &dpu_kms_global_state_funcs);
>  
> -	state->rm = &dpu_kms->rm;
> -
>  	return 0;
>  }
>  
>  static void dpu_kms_global_obj_fini(struct dpu_kms *dpu_kms)

Possibly we should also inline these two functions now.

Anyway:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



>  {
> 
> -- 
> 2.51.0
> 

-- 
With best wishes
Dmitry

[PATCH 12/16] drm/omapdrm: Switch private_obj initialization to reset

[Maxime Ripard]

The omapdrm driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 794267f0f007850e43949f93be5c98d0e32a84ea..4c556da5a5cae3685d929679f43260c51459e8a9 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -272,25 +272,37 @@ static void omap_global_destroy_state(struct drm_private_obj *obj,
 	struct omap_global_state *omap_state = to_omap_global_state(state);
 
 	kfree(omap_state);
 }
 
+static void omap_global_reset(struct drm_private_obj *obj)
+{
+	struct omap_global_state *state;
+
+	if (obj->state) {
+		omap_global_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &state->base);
+}
+
 static const struct drm_private_state_funcs omap_global_state_funcs = {
 	.atomic_duplicate_state = omap_global_duplicate_state,
 	.atomic_destroy_state = omap_global_destroy_state,
+	.reset = omap_global_reset,
 };
 
 static int omap_global_obj_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	struct omap_global_state *state;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
-	drm_atomic_private_obj_init(dev, &priv->glob_obj, &state->base,
+	drm_atomic_private_obj_init(dev, &priv->glob_obj, NULL,
 				    &omap_global_state_funcs);
 	return 0;
 }
 
 static void omap_global_obj_fini(struct omap_drm_private *priv)

-- 
2.51.0

Re: [PATCH 12/16] drm/omapdrm: Switch private_obj initialization to reset

[Tomi Valkeinen]

Hi,

On 08/10/2025 15:04, Maxime Ripard wrote:
> The omapdrm driver relies on a drm_private_obj, that is initialized by
> allocating and initializing a state, and then passing it to
> drm_private_obj_init.
> 
> Since we're gradually moving away from that pattern to the more
> established one relying on a reset implementation, let's migrate this
> instance to the new pattern.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---
> 
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---

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

 Tomi

>  drivers/gpu/drm/omapdrm/omap_drv.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 794267f0f007850e43949f93be5c98d0e32a84ea..4c556da5a5cae3685d929679f43260c51459e8a9 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -272,25 +272,37 @@ static void omap_global_destroy_state(struct drm_private_obj *obj,
>  	struct omap_global_state *omap_state = to_omap_global_state(state);
>  
>  	kfree(omap_state);
>  }
>  
> +static void omap_global_reset(struct drm_private_obj *obj)
> +{
> +	struct omap_global_state *state;
> +
> +	if (obj->state) {
> +		omap_global_destroy_state(obj, obj->state);
> +		obj->state = NULL;
> +	}
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return;
> +
> +	__drm_atomic_helper_private_obj_reset(obj, &state->base);
> +}
> +
>  static const struct drm_private_state_funcs omap_global_state_funcs = {
>  	.atomic_duplicate_state = omap_global_duplicate_state,
>  	.atomic_destroy_state = omap_global_destroy_state,
> +	.reset = omap_global_reset,
>  };
>  
>  static int omap_global_obj_init(struct drm_device *dev)
>  {
>  	struct omap_drm_private *priv = dev->dev_private;
> -	struct omap_global_state *state;
>  
> -	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (!state)
> -		return -ENOMEM;
> -
> -	drm_atomic_private_obj_init(dev, &priv->glob_obj, &state->base,
> +	drm_atomic_private_obj_init(dev, &priv->glob_obj, NULL,
>  				    &omap_global_state_funcs);
>  	return 0;
>  }
>  
>  static void omap_global_obj_fini(struct omap_drm_private *priv)
> 

[PATCH 13/16] drm/tegra: Switch private_obj initialization to reset

[Maxime Ripard]

The tegra driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: linux-tegra@vger.kernel.org
---
 drivers/gpu/drm/tegra/hub.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 8f779f23dc0904d38b14d3f3a928a07fc9e601ad..6cae71bc3744a22e4b3804b19869e8b16fd60ecd 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -822,13 +822,30 @@ static void tegra_display_hub_destroy_state(struct drm_private_obj *obj,
 		to_tegra_display_hub_state(state);
 
 	kfree(hub_state);
 }
 
+static void tegra_display_hub_reset(struct drm_private_obj *obj)
+{
+	struct tegra_display_hub_state *hub_state;
+
+	if (obj->state) {
+		tegra_display_hub_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	hub_state = kzalloc(sizeof(*hub_state), GFP_KERNEL);
+	if (!hub_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &hub_state->base);
+}
+
 static const struct drm_private_state_funcs tegra_display_hub_state_funcs = {
 	.atomic_duplicate_state = tegra_display_hub_duplicate_state,
 	.atomic_destroy_state = tegra_display_hub_destroy_state,
+	.reset = tegra_display_hub_reset,
 };
 
 static struct tegra_display_hub_state *
 tegra_display_hub_get_state(struct tegra_display_hub *hub,
 			    struct drm_atomic_state *state)
@@ -938,17 +955,12 @@ void tegra_display_hub_atomic_commit(struct drm_device *drm,
 static int tegra_display_hub_init(struct host1x_client *client)
 {
 	struct tegra_display_hub *hub = to_tegra_display_hub(client);
 	struct drm_device *drm = dev_get_drvdata(client->host);
 	struct tegra_drm *tegra = drm->dev_private;
-	struct tegra_display_hub_state *state;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
-	drm_atomic_private_obj_init(drm, &hub->base, &state->base,
+	drm_atomic_private_obj_init(drm, &hub->base, NULL,
 				    &tegra_display_hub_state_funcs);
 
 	tegra->hub = hub;
 
 	return 0;

-- 
2.51.0

[PATCH 14/16] drm/vc4: Switch private_obj initialization to reset

[Maxime Ripard]

The vc4 driver relies on a drm_private_obj, that is initialized by
allocating and initializing a state, and then passing it to
drm_private_obj_init.

Since we're gradually moving away from that pattern to the more
established one relying on a reset implementation, let's migrate this
instance to the new pattern.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: "Maíra Canal" <mcanal@igalia.com>
Cc: kernel-list@raspberrypi.com
---
 drivers/gpu/drm/vc4/vc4_kms.c | 75 +++++++++++++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 8f983edb81ff0e3b11bbc8465e69f838050f0d07..f50ffc1c4b62ae3e180743f9cad4c6d0bc62a922 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -82,13 +82,30 @@ static void vc4_ctm_destroy_state(struct drm_private_obj *obj,
 	struct vc4_ctm_state *ctm_state = to_vc4_ctm_state(state);
 
 	kfree(ctm_state);
 }
 
+static void vc4_ctm_reset(struct drm_private_obj *obj)
+{
+	struct vc4_ctm_state *ctm_state;
+
+	if (obj->state) {
+		vc4_ctm_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
+	if (!ctm_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &ctm_state->base);
+}
+
 static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
 	.atomic_duplicate_state = vc4_ctm_duplicate_state,
 	.atomic_destroy_state = vc4_ctm_destroy_state,
+	.reset = vc4_ctm_reset,
 };
 
 static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -96,19 +113,13 @@ static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
 }
 
 static int vc4_ctm_obj_init(struct vc4_dev *vc4)
 {
-	struct vc4_ctm_state *ctm_state;
-
 	drm_modeset_lock_init(&vc4->ctm_state_lock);
 
-	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
-	if (!ctm_state)
-		return -ENOMEM;
-
-	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, &ctm_state->base,
+	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, NULL,
 				    &vc4_ctm_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL);
 }
 
@@ -715,13 +726,30 @@ static void vc4_load_tracker_destroy_state(struct drm_private_obj *obj,
 
 	load_state = to_vc4_load_tracker_state(state);
 	kfree(load_state);
 }
 
+static void vc4_load_tracker_reset(struct drm_private_obj *obj)
+{
+	struct vc4_load_tracker_state *load_state;
+
+	if (obj->state) {
+		vc4_load_tracker_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
+	if (!load_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &load_state->base);
+}
+
 static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
 	.atomic_duplicate_state = vc4_load_tracker_duplicate_state,
 	.atomic_destroy_state = vc4_load_tracker_destroy_state,
+	.reset = vc4_load_tracker_reset,
 };
 
 static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -729,18 +757,12 @@ static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
 	drm_atomic_private_obj_fini(&vc4->load_tracker);
 }
 
 static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
 {
-	struct vc4_load_tracker_state *load_state;
-
-	load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
-	if (!load_state)
-		return -ENOMEM;
-
 	drm_atomic_private_obj_init(&vc4->base, &vc4->load_tracker,
-				    &load_state->base,
+				    NULL,
 				    &vc4_load_tracker_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL);
 }
 
@@ -797,14 +819,31 @@ static void vc4_hvs_channels_print_state(struct drm_printer *p,
 		drm_printf(p, "\t\tin use=%d\n", hvs_state->fifo_state[i].in_use);
 		drm_printf(p, "\t\tload=%lu\n", hvs_state->fifo_state[i].fifo_load);
 	}
 }
 
+static void vc4_hvs_channels_reset(struct drm_private_obj *obj)
+{
+	struct vc4_hvs_state *hvs_state;
+
+	if (obj->state) {
+		vc4_hvs_channels_destroy_state(obj, obj->state);
+		obj->state = NULL;
+	}
+
+	hvs_state = kzalloc(sizeof(*hvs_state), GFP_KERNEL);
+	if (!hvs_state)
+		return;
+
+	__drm_atomic_helper_private_obj_reset(obj, &hvs_state->base);
+}
+
 static const struct drm_private_state_funcs vc4_hvs_state_funcs = {
 	.atomic_duplicate_state = vc4_hvs_channels_duplicate_state,
 	.atomic_destroy_state = vc4_hvs_channels_destroy_state,
 	.atomic_print_state = vc4_hvs_channels_print_state,
+	.reset = vc4_hvs_channels_reset,
 };
 
 static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -812,18 +851,12 @@ static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
 	drm_atomic_private_obj_fini(&vc4->hvs_channels);
 }
 
 static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4)
 {
-	struct vc4_hvs_state *state;
-
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
 	drm_atomic_private_obj_init(&vc4->base, &vc4->hvs_channels,
-				    &state->base,
+				    NULL,
 				    &vc4_hvs_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL);
 }
 

-- 
2.51.0

[PATCH 15/16] drm/atomic: Remove state argument to drm_atomic_private_obj_init

[Maxime Ripard]

Now that all drm_private_objs users have been converted to use reset
instead of the old ad-hoc initialization, we can remove the state
parameter from drm_private_obj_init and the fallback code.

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

To: Liviu Dudau <liviu.dudau@arm.com>
To: Andrzej Hajda <andrzej.hajda@intel.com>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Robert Foss <rfoss@kernel.org>
To: Paul Cercueil <paul@crapouillou.net>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Thierry Reding <thierry.reding@gmail.com>
To: Mikko Perttunen <mperttunen@nvidia.com>
To: Jonathan Hunter <jonathanh@nvidia.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Rodrigo Siqueira <siqueira@igalia.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: "Maíra Canal" <mcanal@igalia.com>
Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-tegra@vger.kernel.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c       |  1 -
 drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c | 16 ++++++++--------
 drivers/gpu/drm/display/drm_dp_mst_topology.c           |  1 -
 drivers/gpu/drm/display/drm_dp_tunnel.c                 |  2 +-
 drivers/gpu/drm/drm_atomic.c                            | 17 ++---------------
 drivers/gpu/drm/drm_bridge.c                            |  1 -
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c               |  2 +-
 drivers/gpu/drm/ingenic/ingenic-ipu.c                   |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c                 |  1 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c                |  1 -
 drivers/gpu/drm/omapdrm/omap_drv.c                      |  2 +-
 drivers/gpu/drm/tegra/hub.c                             |  2 +-
 drivers/gpu/drm/vc4/vc4_kms.c                           |  4 +---
 include/drm/drm_atomic.h                                |  1 -
 14 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1bcbfd814d53bb443b7503ffacb109c900b67b5f..a8b2c7639440dbf16baa2741490db193019f7bc4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4729,11 +4729,10 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
 	/* indicates support for immediate flip */
 	adev_to_drm(adev)->mode_config.async_page_flip = true;
 
 	drm_atomic_private_obj_init(adev_to_drm(adev),
 				    &adev->dm.atomic_obj,
-				    NULL,
 				    &dm_atomic_state_funcs);
 
 	r = amdgpu_display_modeset_create_props(adev);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
index 0c7b7a5dd10900016f64df2a86d54f9178d1cf5e..403f9c10d4cd1e70319d40c2ad267ab76fd24bff 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
@@ -66,11 +66,11 @@ static const struct drm_private_state_funcs komeda_layer_obj_funcs = {
 };
 
 static int komeda_layer_obj_add(struct komeda_kms_dev *kms,
 				struct komeda_layer *layer)
 {
-	drm_atomic_private_obj_init(&kms->base, &layer->base.obj, NULL,
+	drm_atomic_private_obj_init(&kms->base, &layer->base.obj,
 				    &komeda_layer_obj_funcs);
 	return 0;
 }
 
 static struct drm_private_state *
@@ -122,11 +122,11 @@ static const struct drm_private_state_funcs komeda_scaler_obj_funcs = {
 
 static int komeda_scaler_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_scaler *scaler)
 {
 	drm_atomic_private_obj_init(&kms->base,
-				    &scaler->base.obj, NULL,
+				    &scaler->base.obj,
 				    &komeda_scaler_obj_funcs);
 	return 0;
 }
 
 static struct drm_private_state *
@@ -177,11 +177,11 @@ static const struct drm_private_state_funcs komeda_compiz_obj_funcs = {
 };
 
 static int komeda_compiz_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_compiz *compiz)
 {
-	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj, NULL,
+	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj,
 				    &komeda_compiz_obj_funcs);
 
 	return 0;
 }
 
@@ -234,11 +234,11 @@ static const struct drm_private_state_funcs komeda_splitter_obj_funcs = {
 
 static int komeda_splitter_obj_add(struct komeda_kms_dev *kms,
 				   struct komeda_splitter *splitter)
 {
 	drm_atomic_private_obj_init(&kms->base,
-				    &splitter->base.obj, NULL,
+				    &splitter->base.obj,
 				    &komeda_splitter_obj_funcs);
 
 	return 0;
 }
 
@@ -290,11 +290,11 @@ static const struct drm_private_state_funcs komeda_merger_obj_funcs = {
 
 static int komeda_merger_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_merger *merger)
 {
 	drm_atomic_private_obj_init(&kms->base,
-				    &merger->base.obj, NULL,
+				    &merger->base.obj,
 				    &komeda_merger_obj_funcs);
 
 	return 0;
 }
 
@@ -346,11 +346,11 @@ static const struct drm_private_state_funcs komeda_improc_obj_funcs = {
 };
 
 static int komeda_improc_obj_add(struct komeda_kms_dev *kms,
 				 struct komeda_improc *improc)
 {
-	drm_atomic_private_obj_init(&kms->base, &improc->base.obj, NULL,
+	drm_atomic_private_obj_init(&kms->base, &improc->base.obj,
 				    &komeda_improc_obj_funcs);
 
 	return 0;
 }
 
@@ -402,11 +402,11 @@ static const struct drm_private_state_funcs komeda_timing_ctrlr_obj_funcs = {
 };
 
 static int komeda_timing_ctrlr_obj_add(struct komeda_kms_dev *kms,
 				       struct komeda_timing_ctrlr *ctrlr)
 {
-	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj, NULL,
+	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj,
 				    &komeda_timing_ctrlr_obj_funcs);
 
 	return 0;
 }
 
@@ -459,11 +459,11 @@ static const struct drm_private_state_funcs komeda_pipeline_obj_funcs = {
 };
 
 static int komeda_pipeline_obj_add(struct komeda_kms_dev *kms,
 				   struct komeda_pipeline *pipe)
 {
-	drm_atomic_private_obj_init(&kms->base, &pipe->obj, NULL,
+	drm_atomic_private_obj_init(&kms->base, &pipe->obj,
 				    &komeda_pipeline_obj_funcs);
 
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 255fbdcea9f0b6376d15439e3da1dc02be472a20..2831e0b77ab120ef313ad44b0f35df37a0fb59dd 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5764,11 +5764,10 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
 	mgr->max_payloads = max_payloads;
 	mgr->conn_base_id = conn_base_id;
 
 	drm_atomic_private_obj_init(dev, &mgr->base,
-				    NULL,
 				    &drm_dp_mst_topology_state_funcs);
 
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
diff --git a/drivers/gpu/drm/display/drm_dp_tunnel.c b/drivers/gpu/drm/display/drm_dp_tunnel.c
index ae6c6ca99417bf7d99186dd2648f0525c633a8d5..b6e6f0e01c972db2def5164deeff34838ede544f 100644
--- a/drivers/gpu/drm/display/drm_dp_tunnel.c
+++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
@@ -1601,11 +1601,11 @@ static bool init_group(struct drm_dp_tunnel_mgr *mgr, struct drm_dp_tunnel_group
 {
 	group->mgr = mgr;
 	group->available_bw = -1;
 	INIT_LIST_HEAD(&group->tunnels);
 
-	drm_atomic_private_obj_init(mgr->dev, &group->base, NULL,
+	drm_atomic_private_obj_init(mgr->dev, &group->base,
 				    &tunnel_group_funcs);
 
 	return true;
 }
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 45c26294e712fd36b43e87548072c3c0e9af1887..e409919b0ccb632e869b4a6f8462731484755b73 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -773,44 +773,31 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
 
 /**
  * drm_atomic_private_obj_init - initialize private object
  * @dev: DRM device this object will be attached to
  * @obj: private object
- * @state: initial private object state
  * @funcs: pointer to the struct of function pointers that identify the object
  * type
  *
  * Initialize the private object, which can be embedded into any
  * driver private object that needs its own atomic state.
  */
 void
 drm_atomic_private_obj_init(struct drm_device *dev,
 			    struct drm_private_obj *obj,
-			    struct drm_private_state *state,
 			    const struct drm_private_state_funcs *funcs)
 {
 	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);
 
-	/*
-	 * Not all users of drm_atomic_private_obj_init have been
-	 * converted to using &drm_private_obj_funcs.reset yet. For the
-	 * time being, let's only call reset if the passed state is
-	 * NULL. Otherwise, we will fallback to the previous behaviour.
-	 */
-	if (!state) {
-		if (obj->funcs->reset)
-			obj->funcs->reset(obj);
-	} else {
-		obj->state = state;
-		state->obj = obj;
-	}
+	if (obj->funcs->reset)
+		obj->funcs->reset(obj);
 }
 EXPORT_SYMBOL(drm_atomic_private_obj_init);
 
 /**
  * drm_atomic_private_obj_fini - finalize private object
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index cc346412b0205288ec7ee5a7d80a897ad9659404..107bf1d984511496a3766d77de4d1e8a821eaeef 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -478,11 +478,10 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 			goto err_reset_bridge;
 	}
 
 	if (drm_bridge_is_atomic(bridge))
 		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
-					    NULL,
 					    &drm_bridge_priv_state_funcs);
 
 	return 0;
 
 err_reset_bridge:
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 18f20d96f6e4a7d9e5209ee770c7b4fc81adbad7..a721353a9cba410f002c81d082eb2601c1830024 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1396,11 +1396,11 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	if (ret) {
 		dev_err(dev, "Unable to register clock notifier\n");
 		goto err_devclk_disable;
 	}
 
-	drm_atomic_private_obj_init(drm, &priv->private_obj, NULL,
+	drm_atomic_private_obj_init(drm, &priv->private_obj,
 				    &ingenic_drm_private_state_funcs);
 
 	ret = drmm_add_action_or_reset(drm, ingenic_drm_atomic_private_obj_fini,
 				       &priv->private_obj);
 	if (ret)
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index ddaf80052f03b8e366c89a6562b430a400b6dacd..e4de737d159e03a1e61015e42e000a83c9231357 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -901,11 +901,11 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
 	if (err) {
 		dev_err(dev, "Unable to prepare clock\n");
 		return err;
 	}
 
-	drm_atomic_private_obj_init(drm, &ipu->private_obj, NULL,
+	drm_atomic_private_obj_init(drm, &ipu->private_obj,
 				    &ingenic_ipu_private_state_funcs);
 
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 04baf072e8510ce8260f1ec609ba8f2b22f7c11e..d7257e888269aa0b4add4d3a911a9403ef590582 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -403,11 +403,10 @@ static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
 };
 
 static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
 {
 	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
-				    NULL,
 				    &dpu_kms_global_state_funcs);
 
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 44aef7eb8e9073bc9a4bab03c1d6c41313c56ac7..893f29a1fbdd51d22425f52638c74cc97cbe95bf 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -151,11 +151,10 @@ static const struct drm_private_state_funcs mdp5_global_state_funcs = {
 };
 
 static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms)
 {
 	drm_atomic_private_obj_init(mdp5_kms->dev, &mdp5_kms->glob_state,
-				    NULL,
 				    &mdp5_global_state_funcs);
 	return 0;
 }
 
 static void mdp5_enable_commit(struct msm_kms *kms)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 4c556da5a5cae3685d929679f43260c51459e8a9..c14e8648b86044c90d2f9e93dbbe497086289f4f 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -298,11 +298,11 @@ static const struct drm_private_state_funcs omap_global_state_funcs = {
 
 static int omap_global_obj_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 
-	drm_atomic_private_obj_init(dev, &priv->glob_obj, NULL,
+	drm_atomic_private_obj_init(dev, &priv->glob_obj,
 				    &omap_global_state_funcs);
 	return 0;
 }
 
 static void omap_global_obj_fini(struct omap_drm_private *priv)
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 6cae71bc3744a22e4b3804b19869e8b16fd60ecd..67e3b8ca4a5204fd29a64b0c80a5290bb6b093ea 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -956,11 +956,11 @@ static int tegra_display_hub_init(struct host1x_client *client)
 {
 	struct tegra_display_hub *hub = to_tegra_display_hub(client);
 	struct drm_device *drm = dev_get_drvdata(client->host);
 	struct tegra_drm *tegra = drm->dev_private;
 
-	drm_atomic_private_obj_init(drm, &hub->base, NULL,
+	drm_atomic_private_obj_init(drm, &hub->base,
 				    &tegra_display_hub_state_funcs);
 
 	tegra->hub = hub;
 
 	return 0;
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index f50ffc1c4b62ae3e180743f9cad4c6d0bc62a922..b1f2fb29a97b5cd23d819af9ff4ff407a511d2dc 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -115,11 +115,11 @@ static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
 
 static int vc4_ctm_obj_init(struct vc4_dev *vc4)
 {
 	drm_modeset_lock_init(&vc4->ctm_state_lock);
 
-	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, NULL,
+	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager,
 				    &vc4_ctm_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL);
 }
 
@@ -758,11 +758,10 @@ static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
 }
 
 static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
 {
 	drm_atomic_private_obj_init(&vc4->base, &vc4->load_tracker,
-				    NULL,
 				    &vc4_load_tracker_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL);
 }
 
@@ -852,11 +851,10 @@ static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
 }
 
 static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4)
 {
 	drm_atomic_private_obj_init(&vc4->base, &vc4->hvs_channels,
-				    NULL,
 				    &vc4_hvs_state_funcs);
 
 	return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL);
 }
 
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index fbac6d4c75fc86535cf153745b6132f8705c808a..68e30bce7b318ea524df9b47a9e88bb0c7b77c6b 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -617,11 +617,10 @@ struct drm_connector_state * __must_check
 drm_atomic_get_connector_state(struct drm_atomic_state *state,
 			       struct drm_connector *connector);
 
 void drm_atomic_private_obj_init(struct drm_device *dev,
 				 struct drm_private_obj *obj,
-				 struct drm_private_state *state,
 				 const struct drm_private_state_funcs *funcs);
 void drm_atomic_private_obj_fini(struct drm_private_obj *obj);
 
 struct drm_private_state * __must_check
 drm_atomic_get_private_obj_state(struct drm_atomic_state *state,

-- 
2.51.0

Re: [PATCH 15/16] drm/atomic: Remove state argument to drm_atomic_private_obj_init

[Tomi Valkeinen]

Hi,

On 08/10/2025 15:04, Maxime Ripard wrote:
> Now that all drm_private_objs users have been converted to use reset
> instead of the old ad-hoc initialization, we can remove the state
> parameter from drm_private_obj_init and the fallback code.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---


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

 Tomi


> 
> To: Liviu Dudau <liviu.dudau@arm.com>
> To: Andrzej Hajda <andrzej.hajda@intel.com>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Robert Foss <rfoss@kernel.org>
> To: Paul Cercueil <paul@crapouillou.net>
> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> To: Thierry Reding <thierry.reding@gmail.com>
> To: Mikko Perttunen <mperttunen@nvidia.com>
> To: Jonathan Hunter <jonathanh@nvidia.com>
> To: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: Rodrigo Siqueira <siqueira@igalia.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: "Maíra Canal" <mcanal@igalia.com>
> Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-tegra@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c       |  1 -
>  drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c | 16 ++++++++--------
>  drivers/gpu/drm/display/drm_dp_mst_topology.c           |  1 -
>  drivers/gpu/drm/display/drm_dp_tunnel.c                 |  2 +-
>  drivers/gpu/drm/drm_atomic.c                            | 17 ++---------------
>  drivers/gpu/drm/drm_bridge.c                            |  1 -
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c               |  2 +-
>  drivers/gpu/drm/ingenic/ingenic-ipu.c                   |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c                 |  1 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c                |  1 -
>  drivers/gpu/drm/omapdrm/omap_drv.c                      |  2 +-
>  drivers/gpu/drm/tegra/hub.c                             |  2 +-
>  drivers/gpu/drm/vc4/vc4_kms.c                           |  4 +---
>  include/drm/drm_atomic.h                                |  1 -
>  14 files changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 1bcbfd814d53bb443b7503ffacb109c900b67b5f..a8b2c7639440dbf16baa2741490db193019f7bc4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4729,11 +4729,10 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
>  	/* indicates support for immediate flip */
>  	adev_to_drm(adev)->mode_config.async_page_flip = true;
>  
>  	drm_atomic_private_obj_init(adev_to_drm(adev),
>  				    &adev->dm.atomic_obj,
> -				    NULL,
>  				    &dm_atomic_state_funcs);
>  
>  	r = amdgpu_display_modeset_create_props(adev);
>  	if (r)
>  		return r;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
> index 0c7b7a5dd10900016f64df2a86d54f9178d1cf5e..403f9c10d4cd1e70319d40c2ad267ab76fd24bff 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
> @@ -66,11 +66,11 @@ static const struct drm_private_state_funcs komeda_layer_obj_funcs = {
>  };
>  
>  static int komeda_layer_obj_add(struct komeda_kms_dev *kms,
>  				struct komeda_layer *layer)
>  {
> -	drm_atomic_private_obj_init(&kms->base, &layer->base.obj, NULL,
> +	drm_atomic_private_obj_init(&kms->base, &layer->base.obj,
>  				    &komeda_layer_obj_funcs);
>  	return 0;
>  }
>  
>  static struct drm_private_state *
> @@ -122,11 +122,11 @@ static const struct drm_private_state_funcs komeda_scaler_obj_funcs = {
>  
>  static int komeda_scaler_obj_add(struct komeda_kms_dev *kms,
>  				 struct komeda_scaler *scaler)
>  {
>  	drm_atomic_private_obj_init(&kms->base,
> -				    &scaler->base.obj, NULL,
> +				    &scaler->base.obj,
>  				    &komeda_scaler_obj_funcs);
>  	return 0;
>  }
>  
>  static struct drm_private_state *
> @@ -177,11 +177,11 @@ static const struct drm_private_state_funcs komeda_compiz_obj_funcs = {
>  };
>  
>  static int komeda_compiz_obj_add(struct komeda_kms_dev *kms,
>  				 struct komeda_compiz *compiz)
>  {
> -	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj, NULL,
> +	drm_atomic_private_obj_init(&kms->base, &compiz->base.obj,
>  				    &komeda_compiz_obj_funcs);
>  
>  	return 0;
>  }
>  
> @@ -234,11 +234,11 @@ static const struct drm_private_state_funcs komeda_splitter_obj_funcs = {
>  
>  static int komeda_splitter_obj_add(struct komeda_kms_dev *kms,
>  				   struct komeda_splitter *splitter)
>  {
>  	drm_atomic_private_obj_init(&kms->base,
> -				    &splitter->base.obj, NULL,
> +				    &splitter->base.obj,
>  				    &komeda_splitter_obj_funcs);
>  
>  	return 0;
>  }
>  
> @@ -290,11 +290,11 @@ static const struct drm_private_state_funcs komeda_merger_obj_funcs = {
>  
>  static int komeda_merger_obj_add(struct komeda_kms_dev *kms,
>  				 struct komeda_merger *merger)
>  {
>  	drm_atomic_private_obj_init(&kms->base,
> -				    &merger->base.obj, NULL,
> +				    &merger->base.obj,
>  				    &komeda_merger_obj_funcs);
>  
>  	return 0;
>  }
>  
> @@ -346,11 +346,11 @@ static const struct drm_private_state_funcs komeda_improc_obj_funcs = {
>  };
>  
>  static int komeda_improc_obj_add(struct komeda_kms_dev *kms,
>  				 struct komeda_improc *improc)
>  {
> -	drm_atomic_private_obj_init(&kms->base, &improc->base.obj, NULL,
> +	drm_atomic_private_obj_init(&kms->base, &improc->base.obj,
>  				    &komeda_improc_obj_funcs);
>  
>  	return 0;
>  }
>  
> @@ -402,11 +402,11 @@ static const struct drm_private_state_funcs komeda_timing_ctrlr_obj_funcs = {
>  };
>  
>  static int komeda_timing_ctrlr_obj_add(struct komeda_kms_dev *kms,
>  				       struct komeda_timing_ctrlr *ctrlr)
>  {
> -	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj, NULL,
> +	drm_atomic_private_obj_init(&kms->base, &ctrlr->base.obj,
>  				    &komeda_timing_ctrlr_obj_funcs);
>  
>  	return 0;
>  }
>  
> @@ -459,11 +459,11 @@ static const struct drm_private_state_funcs komeda_pipeline_obj_funcs = {
>  };
>  
>  static int komeda_pipeline_obj_add(struct komeda_kms_dev *kms,
>  				   struct komeda_pipeline *pipe)
>  {
> -	drm_atomic_private_obj_init(&kms->base, &pipe->obj, NULL,
> +	drm_atomic_private_obj_init(&kms->base, &pipe->obj,
>  				    &komeda_pipeline_obj_funcs);
>  
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 255fbdcea9f0b6376d15439e3da1dc02be472a20..2831e0b77ab120ef313ad44b0f35df37a0fb59dd 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -5764,11 +5764,10 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
>  	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
>  	mgr->max_payloads = max_payloads;
>  	mgr->conn_base_id = conn_base_id;
>  
>  	drm_atomic_private_obj_init(dev, &mgr->base,
> -				    NULL,
>  				    &drm_dp_mst_topology_state_funcs);
>  
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
> diff --git a/drivers/gpu/drm/display/drm_dp_tunnel.c b/drivers/gpu/drm/display/drm_dp_tunnel.c
> index ae6c6ca99417bf7d99186dd2648f0525c633a8d5..b6e6f0e01c972db2def5164deeff34838ede544f 100644
> --- a/drivers/gpu/drm/display/drm_dp_tunnel.c
> +++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
> @@ -1601,11 +1601,11 @@ static bool init_group(struct drm_dp_tunnel_mgr *mgr, struct drm_dp_tunnel_group
>  {
>  	group->mgr = mgr;
>  	group->available_bw = -1;
>  	INIT_LIST_HEAD(&group->tunnels);
>  
> -	drm_atomic_private_obj_init(mgr->dev, &group->base, NULL,
> +	drm_atomic_private_obj_init(mgr->dev, &group->base,
>  				    &tunnel_group_funcs);
>  
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 45c26294e712fd36b43e87548072c3c0e9af1887..e409919b0ccb632e869b4a6f8462731484755b73 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -773,44 +773,31 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  
>  /**
>   * drm_atomic_private_obj_init - initialize private object
>   * @dev: DRM device this object will be attached to
>   * @obj: private object
> - * @state: initial private object state
>   * @funcs: pointer to the struct of function pointers that identify the object
>   * type
>   *
>   * Initialize the private object, which can be embedded into any
>   * driver private object that needs its own atomic state.
>   */
>  void
>  drm_atomic_private_obj_init(struct drm_device *dev,
>  			    struct drm_private_obj *obj,
> -			    struct drm_private_state *state,
>  			    const struct drm_private_state_funcs *funcs)
>  {
>  	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);
>  
> -	/*
> -	 * Not all users of drm_atomic_private_obj_init have been
> -	 * converted to using &drm_private_obj_funcs.reset yet. For the
> -	 * time being, let's only call reset if the passed state is
> -	 * NULL. Otherwise, we will fallback to the previous behaviour.
> -	 */
> -	if (!state) {
> -		if (obj->funcs->reset)
> -			obj->funcs->reset(obj);
> -	} else {
> -		obj->state = state;
> -		state->obj = obj;
> -	}
> +	if (obj->funcs->reset)
> +		obj->funcs->reset(obj);
>  }
>  EXPORT_SYMBOL(drm_atomic_private_obj_init);
>  
>  /**
>   * drm_atomic_private_obj_fini - finalize private object
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index cc346412b0205288ec7ee5a7d80a897ad9659404..107bf1d984511496a3766d77de4d1e8a821eaeef 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -478,11 +478,10 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  			goto err_reset_bridge;
>  	}
>  
>  	if (drm_bridge_is_atomic(bridge))
>  		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
> -					    NULL,
>  					    &drm_bridge_priv_state_funcs);
>  
>  	return 0;
>  
>  err_reset_bridge:
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 18f20d96f6e4a7d9e5209ee770c7b4fc81adbad7..a721353a9cba410f002c81d082eb2601c1830024 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -1396,11 +1396,11 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>  	if (ret) {
>  		dev_err(dev, "Unable to register clock notifier\n");
>  		goto err_devclk_disable;
>  	}
>  
> -	drm_atomic_private_obj_init(drm, &priv->private_obj, NULL,
> +	drm_atomic_private_obj_init(drm, &priv->private_obj,
>  				    &ingenic_drm_private_state_funcs);
>  
>  	ret = drmm_add_action_or_reset(drm, ingenic_drm_atomic_private_obj_fini,
>  				       &priv->private_obj);
>  	if (ret)
> diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> index ddaf80052f03b8e366c89a6562b430a400b6dacd..e4de737d159e03a1e61015e42e000a83c9231357 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> @@ -901,11 +901,11 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
>  	if (err) {
>  		dev_err(dev, "Unable to prepare clock\n");
>  		return err;
>  	}
>  
> -	drm_atomic_private_obj_init(drm, &ipu->private_obj, NULL,
> +	drm_atomic_private_obj_init(drm, &ipu->private_obj,
>  				    &ingenic_ipu_private_state_funcs);
>  
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 04baf072e8510ce8260f1ec609ba8f2b22f7c11e..d7257e888269aa0b4add4d3a911a9403ef590582 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -403,11 +403,10 @@ static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
>  };
>  
>  static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
>  {
>  	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
> -				    NULL,
>  				    &dpu_kms_global_state_funcs);
>  
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 44aef7eb8e9073bc9a4bab03c1d6c41313c56ac7..893f29a1fbdd51d22425f52638c74cc97cbe95bf 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -151,11 +151,10 @@ static const struct drm_private_state_funcs mdp5_global_state_funcs = {
>  };
>  
>  static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms)
>  {
>  	drm_atomic_private_obj_init(mdp5_kms->dev, &mdp5_kms->glob_state,
> -				    NULL,
>  				    &mdp5_global_state_funcs);
>  	return 0;
>  }
>  
>  static void mdp5_enable_commit(struct msm_kms *kms)
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 4c556da5a5cae3685d929679f43260c51459e8a9..c14e8648b86044c90d2f9e93dbbe497086289f4f 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -298,11 +298,11 @@ static const struct drm_private_state_funcs omap_global_state_funcs = {
>  
>  static int omap_global_obj_init(struct drm_device *dev)
>  {
>  	struct omap_drm_private *priv = dev->dev_private;
>  
> -	drm_atomic_private_obj_init(dev, &priv->glob_obj, NULL,
> +	drm_atomic_private_obj_init(dev, &priv->glob_obj,
>  				    &omap_global_state_funcs);
>  	return 0;
>  }
>  
>  static void omap_global_obj_fini(struct omap_drm_private *priv)
> diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> index 6cae71bc3744a22e4b3804b19869e8b16fd60ecd..67e3b8ca4a5204fd29a64b0c80a5290bb6b093ea 100644
> --- a/drivers/gpu/drm/tegra/hub.c
> +++ b/drivers/gpu/drm/tegra/hub.c
> @@ -956,11 +956,11 @@ static int tegra_display_hub_init(struct host1x_client *client)
>  {
>  	struct tegra_display_hub *hub = to_tegra_display_hub(client);
>  	struct drm_device *drm = dev_get_drvdata(client->host);
>  	struct tegra_drm *tegra = drm->dev_private;
>  
> -	drm_atomic_private_obj_init(drm, &hub->base, NULL,
> +	drm_atomic_private_obj_init(drm, &hub->base,
>  				    &tegra_display_hub_state_funcs);
>  
>  	tegra->hub = hub;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index f50ffc1c4b62ae3e180743f9cad4c6d0bc62a922..b1f2fb29a97b5cd23d819af9ff4ff407a511d2dc 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -115,11 +115,11 @@ static void vc4_ctm_obj_fini(struct drm_device *dev, void *unused)
>  
>  static int vc4_ctm_obj_init(struct vc4_dev *vc4)
>  {
>  	drm_modeset_lock_init(&vc4->ctm_state_lock);
>  
> -	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, NULL,
> +	drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager,
>  				    &vc4_ctm_state_funcs);
>  
>  	return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL);
>  }
>  
> @@ -758,11 +758,10 @@ static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
>  }
>  
>  static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
>  {
>  	drm_atomic_private_obj_init(&vc4->base, &vc4->load_tracker,
> -				    NULL,
>  				    &vc4_load_tracker_state_funcs);
>  
>  	return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL);
>  }
>  
> @@ -852,11 +851,10 @@ static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
>  }
>  
>  static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4)
>  {
>  	drm_atomic_private_obj_init(&vc4->base, &vc4->hvs_channels,
> -				    NULL,
>  				    &vc4_hvs_state_funcs);
>  
>  	return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL);
>  }
>  
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index fbac6d4c75fc86535cf153745b6132f8705c808a..68e30bce7b318ea524df9b47a9e88bb0c7b77c6b 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -617,11 +617,10 @@ struct drm_connector_state * __must_check
>  drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  			       struct drm_connector *connector);
>  
>  void drm_atomic_private_obj_init(struct drm_device *dev,
>  				 struct drm_private_obj *obj,
> -				 struct drm_private_state *state,
>  				 const struct drm_private_state_funcs *funcs);
>  void drm_atomic_private_obj_fini(struct drm_private_obj *obj);
>  
>  struct drm_private_state * __must_check
>  drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
> 

Re: [PATCH 15/16] drm/atomic: Remove state argument to drm_atomic_private_obj_init

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:13PM +0200, Maxime Ripard wrote:
> Now that all drm_private_objs users have been converted to use reset
> instead of the old ad-hoc initialization, we can remove the state
> parameter from drm_private_obj_init and the fallback code.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> ---
> 
> To: Liviu Dudau <liviu.dudau@arm.com>
> To: Andrzej Hajda <andrzej.hajda@intel.com>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Robert Foss <rfoss@kernel.org>
> To: Paul Cercueil <paul@crapouillou.net>
> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> To: Thierry Reding <thierry.reding@gmail.com>
> To: Mikko Perttunen <mperttunen@nvidia.com>
> To: Jonathan Hunter <jonathanh@nvidia.com>
> To: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: Rodrigo Siqueira <siqueira@igalia.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: "Maíra Canal" <mcanal@igalia.com>
> Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-tegra@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c       |  1 -
>  drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c | 16 ++++++++--------
>  drivers/gpu/drm/display/drm_dp_mst_topology.c           |  1 -
>  drivers/gpu/drm/display/drm_dp_tunnel.c                 |  2 +-
>  drivers/gpu/drm/drm_atomic.c                            | 17 ++---------------
>  drivers/gpu/drm/drm_bridge.c                            |  1 -
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c               |  2 +-
>  drivers/gpu/drm/ingenic/ingenic-ipu.c                   |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c                 |  1 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c                |  1 -
>  drivers/gpu/drm/omapdrm/omap_drv.c                      |  2 +-
>  drivers/gpu/drm/tegra/hub.c                             |  2 +-
>  drivers/gpu/drm/vc4/vc4_kms.c                           |  4 +---
>  include/drm/drm_atomic.h                                |  1 -
>  14 files changed, 16 insertions(+), 37 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 16/16] drm/mode_config: Call private obj reset with the other objects

[Maxime Ripard]

Now that we have all the drm_private_obj users relying on the reset
implementation, we can move that call from drm_private_obj_init, where
it was initially supposed to happen, to drm_mode_config_reset, which is
the location reset is called for every other object in KMS.

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

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e409919b0ccb632e869b4a6f8462731484755b73..5e76ae017117ca25a2620b8b3cad4f0d622448fe 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -791,13 +791,10 @@ drm_atomic_private_obj_init(struct drm_device *dev,
 	drm_modeset_lock_init(&obj->lock);
 
 	obj->dev = dev;
 	obj->funcs = funcs;
 	list_add_tail(&obj->head, &dev->mode_config.privobj_list);
-
-	if (obj->funcs->reset)
-		obj->funcs->reset(obj);
 }
 EXPORT_SYMBOL(drm_atomic_private_obj_init);
 
 /**
  * drm_atomic_private_obj_fini - finalize private object
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 25f376869b3a41d47bbe72b0df3e35cad142f3e6..76fcf80fdcec4337992b35ac741189bb32ee670d 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>
@@ -193,12 +194,17 @@ void drm_mode_config_reset(struct drm_device *dev)
 {
 	struct drm_crtc *crtc;
 	struct drm_plane *plane;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
+	struct drm_private_obj *obj;
 	struct drm_connector_list_iter conn_iter;
 
+	drm_for_each_privobj(obj, dev)
+		if (obj->funcs->reset)
+			obj->funcs->reset(obj);
+
 	drm_for_each_plane(plane, dev)
 		if (plane->funcs->reset)
 			plane->funcs->reset(plane);
 
 	drm_for_each_crtc(crtc, dev)

-- 
2.51.0

Re: [PATCH 16/16] drm/mode_config: Call private obj reset with the other objects

[Dmitry Baryshkov]

On Wed, Oct 08, 2025 at 02:04:14PM +0200, Maxime Ripard wrote:
> Now that we have all the drm_private_obj users relying on the reset
> implementation, we can move that call from drm_private_obj_init, where
> it was initially supposed to happen, to drm_mode_config_reset, which is
> the location reset is called for every other object in KMS.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic.c      | 3 ---
>  drivers/gpu/drm/drm_mode_config.c | 6 ++++++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry