[PATCH v5 0/4] drm/atomic: Allocate drm_private_state through a callback

[PATCH v5 0/4] drm/atomic: Allocate drm_private_state through a callback

[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>
---
Changes in v5:
- Rebase to drm-misc-next
- Link to v4: https://lore.kernel.org/r/20260128-drm-private-obj-reset-v4-0-90891fa3d3b0@redhat.com

Changes in v4:
- Fix a circular dependencies between modules by calling
  __drm_atomic_helper_private_obj_create_state from
  __drm_atomic_helper_bridge_reset instead of
  drm_bridge_atomic_create_priv_state()
- Link to v3: https://lore.kernel.org/r/20260119-drm-private-obj-reset-v3-0-b931abe3a5e3@redhat.com

Changes in v3:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v2: https://lore.kernel.org/r/20251014-drm-private-obj-reset-v2-0-6dd60e985e9d@kernel.org

Changes in v2:
- Switch to a new hook instead of reset since some drm_private_objs want
  to persist across suspends
- Drop the call to drm_private_obj_funcs.reset in
  drm_mode_config_reset()
- Link to v1: https://lore.kernel.org/r/20251008-drm-private-obj-reset-v1-0-805ab43ae65a@kernel.org

---
Maxime Ripard (4):
      drm/amdgpu: Switch private_obj initialization to atomic_create_state
      drm/omapdrm: Switch private_obj initialization to atomic_create_state
      drm/tegra: Switch private_obj initialization to atomic_create_state
      drm/atomic: Remove state argument to drm_atomic_private_obj_init

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 51 ++++++++++++----------
 .../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                       | 22 +++-------
 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                 | 22 +++++++---
 drivers/gpu/drm/tegra/hub.c                        | 22 +++++++---
 drivers/gpu/drm/vc4/vc4_kms.c                      |  4 +-
 include/drm/drm_atomic.h                           |  1 -
 14 files changed, 76 insertions(+), 72 deletions(-)
---
base-commit: 196b2b95fec447c2c4460f753b277d840633fbef
change-id: 20251008-drm-private-obj-reset-ae1e2741027a

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

[PATCH v5 1/4] drm/amdgpu: Switch private_obj initialization to atomic_create_state

[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 atomic_create_state 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 | 52 ++++++++++++-----------
 1 file changed, 28 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 a8e4e3ab5e40276b3c723297ca6520688079e0a7..98ed54afb02864342bb2c04782d05c5fb6e38672 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4912,18 +4912,41 @@ static void dm_atomic_destroy_state(struct drm_private_obj *obj,
 		dc_state_release(dm_state->context);
 
 	kfree(dm_state);
 }
 
+static struct drm_private_state *
+dm_atomic_create_state(struct drm_private_obj *obj)
+{
+	struct amdgpu_device *adev = drm_to_adev(obj->dev);
+	struct dm_atomic_state *dm_state;
+	struct dc_state *context;
+
+	dm_state = kzalloc_obj(*dm_state);
+	if (!dm_state)
+		return ERR_PTR(-ENOMEM);
+
+	context = dc_state_create_current_copy(adev->dm.dc);
+	if (!context) {
+		kfree(dm_state);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	__drm_atomic_helper_private_obj_create_state(obj, &dm_state->base);
+	dm_state->context = context;
+
+	return &dm_state->base;
+}
+
 static struct drm_private_state_funcs dm_atomic_state_funcs = {
+	.atomic_create_state = dm_atomic_create_state,
 	.atomic_duplicate_state = dm_atomic_duplicate_state,
 	.atomic_destroy_state = dm_atomic_destroy_state,
 };
 
 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;
@@ -4939,46 +4962,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_obj(*state);
-	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.52.0

Claude review: drm/amdgpu: Switch private_obj initialization to atomic_create_state

[Claude Code Review Bot]

Patch Review

**Commit message**: Clear and well-written.

**Code review**:

The new `dm_atomic_create_state()` function looks correct:

```c
static struct drm_private_state *
dm_atomic_create_state(struct drm_private_obj *obj)
{
	struct amdgpu_device *adev = drm_to_adev(obj->dev);
	struct dm_atomic_state *dm_state;
	struct dc_state *context;

	dm_state = kzalloc_obj(*dm_state);
	if (!dm_state)
		return ERR_PTR(-ENOMEM);

	context = dc_state_create_current_copy(adev->dm.dc);
	if (!context) {
		kfree(dm_state);
		return ERR_PTR(-ENOMEM);
	}

	__drm_atomic_helper_private_obj_create_state(obj, &dm_state->base);
	dm_state->context = context;

	return &dm_state->base;
}
```

This is well-structured with proper error cleanup if `dc_state_create_current_copy()` fails.

**Issue: Missing return value check.** The call to `drm_atomic_private_obj_init()` now passes `NULL` as the state, which means it will internally call `atomic_create_state` — which can fail (returning `ERR_PTR(-ENOMEM)`). The return value is not checked:

```c
	drm_atomic_private_obj_init(adev_to_drm(adev),
				    &adev->dm.atomic_obj,
				    NULL,
				    &dm_atomic_state_funcs);
```

This should be something like:
```c
	ret = drm_atomic_private_obj_init(adev_to_drm(adev),
				    &adev->dm.atomic_obj,
				    NULL,
				    &dm_atomic_state_funcs);
	if (ret)
		return ret;
```

**Positive note**: The error path simplification is a nice side-effect. The old code was arguably buggy — it manually freed state on error after `drm_atomic_private_obj_init()` had already registered the object, which could lead to a use-after-free if cleanup later called `drm_atomic_private_obj_fini()`. The new code avoids that class of bug.

**Minor note (pre-existing)**: `dm_atomic_state_funcs` is not `const`, unlike the omap and tegra equivalents. Not introduced by this patch.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 2/4] drm/omapdrm: Switch private_obj initialization to atomic_create_state

[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 atomic_create_state 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 | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index bf0bad8c8cf120baa124b439f3fcbfe36c976d12..90832b4b8c9dd540c8778cb39de7cf80e8796857 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -273,25 +273,35 @@ 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 struct drm_private_state *
+omap_global_atomic_create_state(struct drm_private_obj *obj)
+{
+	struct omap_global_state *state;
+
+	state = kzalloc_obj(*state);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_private_obj_create_state(obj, &state->base);
+
+	return &state->base;
+}
+
 static const struct drm_private_state_funcs omap_global_state_funcs = {
+	.atomic_create_state = omap_global_atomic_create_state,
 	.atomic_duplicate_state = omap_global_duplicate_state,
 	.atomic_destroy_state = omap_global_destroy_state,
 };
 
 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_obj(*state);
-	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.52.0

Claude review: drm/omapdrm: Switch private_obj initialization to atomic_create_state

[Claude Code Review Bot]

Patch Review

**Commit message**: Clear and well-written.

**Code review**:

The new `omap_global_atomic_create_state()` is clean and correct:

```c
static struct drm_private_state *
omap_global_atomic_create_state(struct drm_private_obj *obj)
{
	struct omap_global_state *state;

	state = kzalloc_obj(*state);
	if (!state)
		return ERR_PTR(-ENOMEM);

	__drm_atomic_helper_private_obj_create_state(obj, &state->base);

	return &state->base;
}
```

Simple and follows the pattern correctly. Since `omap_global_state` has no additional resources to initialize (just `kzalloc` is sufficient), this is the minimal correct implementation.

**Same issue: Missing return value check:**

```c
	drm_atomic_private_obj_init(dev, &priv->glob_obj, NULL,
				    &omap_global_state_funcs);
	return 0;
```

This should propagate the return value of `drm_atomic_private_obj_init()` instead of unconditionally returning 0.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 3/4] drm/tegra: Switch private_obj initialization to atomic_create_state

[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 atomic_create_state 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 | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index a6fa196c28132ea9839c48639fb3214b32396d02..73190a4b4d0546be6d6cae746ba5d03ab8e98b92 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -823,11 +823,26 @@ static void tegra_display_hub_destroy_state(struct drm_private_obj *obj,
 		to_tegra_display_hub_state(state);
 
 	kfree(hub_state);
 }
 
+static struct drm_private_state *
+tegra_display_hub_create_state(struct drm_private_obj *obj)
+{
+	struct tegra_display_hub_state *hub_state;
+
+	hub_state = kzalloc_obj(*hub_state);
+	if (!hub_state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_private_obj_create_state(obj, &hub_state->base);
+
+	return &hub_state->base;
+}
+
 static const struct drm_private_state_funcs tegra_display_hub_state_funcs = {
+	.atomic_create_state = tegra_display_hub_create_state,
 	.atomic_duplicate_state = tegra_display_hub_duplicate_state,
 	.atomic_destroy_state = tegra_display_hub_destroy_state,
 };
 
 static struct tegra_display_hub_state *
@@ -939,17 +954,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_obj(*state);
-	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.52.0

Claude review: drm/tegra: Switch private_obj initialization to atomic_create_state

[Claude Code Review Bot]

Patch Review

**Commit message**: Clear and well-written.

**Code review**:

The new `tegra_display_hub_create_state()` is structurally identical to the omapdrm version and is correct:

```c
static struct drm_private_state *
tegra_display_hub_create_state(struct drm_private_obj *obj)
{
	struct tegra_display_hub_state *hub_state;

	hub_state = kzalloc_obj(*hub_state);
	if (!hub_state)
		return ERR_PTR(-ENOMEM);

	__drm_atomic_helper_private_obj_create_state(obj, &hub_state->base);

	return &hub_state->base;
}
```

**Same issue: Missing return value check:**

```c
	drm_atomic_private_obj_init(drm, &hub->base, NULL,
				    &tegra_display_hub_state_funcs);

	tegra->hub = hub;

	return 0;
```

Should check the return value and propagate errors.

**Note**: I realize this same return-value-not-checked pattern exists in many other already-converted callers (komeda, ingenic, dpu, mdp5, vc4, drm_bridge, drm_dp_mst, drm_dp_tunnel). This is a wider pre-existing issue, but patches 1-3 should not perpetuate it. At minimum the three patches being submitted should get it right.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 4/4] 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
atomic_create_state instead of the old ad-hoc initialization, we can
remove the state parameter from drm_private_obj_init and the fallback
code.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
---

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 -
 .../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                       | 22 +++++-----------------
 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, 19 insertions(+), 39 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 98ed54afb02864342bb2c04782d05c5fb6e38672..72cfba657d1f49b42e9f5f99fced653e7e24a4e9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4964,11 +4964,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 49b934c6dbdf0fdbf1dfa10bf74536481350a381..77b3f361091fc3e07d30cee4a74f69c7ea1f6e90 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
@@ -63,11 +63,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 *
@@ -116,11 +116,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 *
@@ -168,11 +168,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;
 }
 
@@ -222,11 +222,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;
 }
 
@@ -275,11 +275,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;
 }
 
@@ -328,11 +328,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;
 }
 
@@ -381,11 +381,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;
 }
 
@@ -435,11 +435,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 d8a732f21d3c04eaeefa64d5fe34d156369787ac..8757972e8e2427cbbab2e90d1c9c291d97e96c0a 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5763,11 +5763,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 f442430d8de78221ec88d0567dc68f2fa8d2b82a..6519b42447285b58892ac07a3570cf6c457e4c27 100644
--- a/drivers/gpu/drm/display/drm_dp_tunnel.c
+++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
@@ -1598,11 +1598,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 04925166df989bcb30b111739aa4ed5c84f3a5ae..4283ab4d06c581727cc98b1dc870bf69691ea654 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -918,11 +918,10 @@ 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.
@@ -930,37 +929,26 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
  * RETURNS:
  * Zero on success, error code on failure
  */
 int 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)
 {
+	struct drm_private_state *state;
 	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.atomic_create_state 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) {
-		state = obj->funcs->atomic_create_state(obj);
-		if (IS_ERR(state))
-			return PTR_ERR(state);
+	state = obj->funcs->atomic_create_state(obj);
+	if (IS_ERR(state))
+		return PTR_ERR(state);
 
-		obj->state = state;
-	} else {
-		obj->state = state;
-		state->obj = obj;
-	}
+	obj->state = state;
 
 	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_private_obj_init);
 
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index f8b0333a0a3bf5bdf4036d2f7ca5565934e44b4c..1868d512ffa1fabc8adab97bc7cb96fd32a78997 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -551,11 +551,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 9522a2e6ecd4166efc648a24e237b8166e426d48..4068114adf8c2068a94a3aaa308cf91847712acb 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1399,11 +1399,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 4fec37c63e7cb7c96b32191d680c89e20837b155..34545b9c8c33833c3cc65f3d111b3f3bf9a60137 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -899,11 +899,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 4495525139978e64e0c650aae0cdc1a57e0c8398..31743699d05f6667e37fe70d79549cfe736474fe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1159,11 +1159,10 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 
 	dev->mode_config.cursor_width = 512;
 	dev->mode_config.cursor_height = 512;
 
 	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
-				    NULL,
 				    &dpu_kms_global_state_funcs);
 
 	atomic_set(&dpu_kms->bandwidth_ref, 0);
 
 	rc = pm_runtime_resume_and_get(&dpu_kms->pdev->dev);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 1e3dc9bf9494c2b06ad5acaca74f7b17221b7833..2d26b07b06f54bbe8e26e538c155b8de994a4543 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -715,11 +715,10 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 	int ret;
 
 	mdp5_kms->dev = dev;
 
 	drm_atomic_private_obj_init(mdp5_kms->dev, &mdp5_kms->glob_state,
-				    NULL,
 				    &mdp5_global_state_funcs);
 
 	/* we need to set a default rate before enabling.  Set a safe
 	 * rate first, then figure out hw revision, and then set a
 	 * more optimal rate:
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 90832b4b8c9dd540c8778cb39de7cf80e8796857..ae678696fbacb0572869c7152a484fc5736ac44a 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -297,11 +297,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 73190a4b4d0546be6d6cae746ba5d03ab8e98b92..10d993b8d043ab08ee65ab4db1e31624c953d0f3 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -955,11 +955,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 0507f24adcdd05660b9ef349dd609b095d52a9f8..264b5e80c24d67f59dea8085f7d02d218db25697 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -114,11 +114,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);
 }
 
@@ -755,11 +755,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);
 }
 
@@ -847,11 +846,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 0b1b32bcd2bda1b92299fd369ba7c23b1c2d3dfa..f03cd199aee73fa8e15b2d9e16a53d134fc7de7d 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -736,11 +736,10 @@ struct drm_connector_state * __must_check
 drm_atomic_get_connector_state(struct drm_atomic_state *state,
 			       struct drm_connector *connector);
 
 int 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.52.0

Claude review: drm/atomic: Remove state argument to drm_atomic_private_obj_init

[Claude Code Review Bot]

Patch Review

**Commit message**: Clear. Has multiple Reviewed-by tags (Dmitry Baryshkov, Tomi Valkeinen, Liviu Dudau, Maíra Canal), which is good.

**Code review**:

The core change in `drm_atomic.c` is clean and correct:

```c
int drm_atomic_private_obj_init(struct drm_device *dev,
				struct drm_private_obj *obj,
				const struct drm_private_state_funcs *funcs)
{
	struct drm_private_state *state;
	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);

	state = obj->funcs->atomic_create_state(obj);
	if (IS_ERR(state))
		return PTR_ERR(state);

	obj->state = state;

	return 0;
}
```

The removal of the fallback path and state parameter is the logical conclusion of the conversion. The function is now simpler and has a single code path.

**Nit**: When the function returns an error from `atomic_create_state`, the object has already been added to `dev->mode_config.privobj_list` with `obj->state = NULL` (from memset). This could be problematic if anything iterates the list before the caller has a chance to clean up. Consider moving the `list_add_tail` after the successful state creation, or removing from the list on error. This is a pre-existing issue from the earlier NULL-handling code, but now is a good time to fix it.

All driver-side changes in this patch are purely mechanical removal of the `NULL` argument from every `drm_atomic_private_obj_init()` call — these are all correct.

The header change removing the parameter from the declaration is correct:

```c
 int 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);
```

**No issues with this patch itself.**

---

### Cover Letter Note

The v3 changelog was not filled in:

```
Changes in v3:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
```

This is a minor cosmetic issue but looks unprofessional in the archives. Should be fixed if a v6 is needed.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/atomic: Allocate drm_private_state through a callback

[Claude Code Review Bot]

Overall Series Review

Subject: drm/atomic: Allocate drm_private_state through a callback
Author: Maxime Ripard <mripard@kernel.org>
Patches: 5
Reviewed: 2026-02-27T15:05:36.753298

---

**Series: drm/atomic: Allocate drm_private_state through a callback (v5)**
**Author: Maxime Ripard <mripard@kernel.org>**

This is a well-structured and logical cleanup series. It converts the three remaining `drm_private_obj` users (amdgpu, omapdrm, tegra) from the old pattern of externally allocating and passing state to `drm_atomic_private_obj_init()`, to the newer `atomic_create_state` callback pattern. Once all users are converted, patch 4 removes the now-unnecessary `state` parameter.

The series is sensible and follows the established direction for the DRM atomic API. The driver conversions are mechanical and straightforward. However, there is one significant issue: **none of the three driver patches check the return value of `drm_atomic_private_obj_init()`**, which can now fail when it internally calls `atomic_create_state`. There is also a cosmetic issue with the cover letter.

**Overall verdict**: Needs minor fixes for return value checking before merging.

---

---
Generated by Claude Code Patch Reviewer