linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc()
@ 2025-06-06  8:41 Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API Luca Ceresoli
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Luca Ceresoli @ 2025-06-06  8:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec
  Cc: Anusha Srivatsa, Paul Kocialkowski, Dmitry Baryshkov,
	Hervé Codina, Hui Pu, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

This small series adds a few kunit tests for the new DRM bridge allocation
flow, based on the recently introduced devm_drm_bridge_alloc() [0].

It is part of the work towards removal of bridges from a still existing DRM
pipeline without use-after-free.

The steps in the grand plan [1] are:

 1. ➜ add refcounting to DRM bridges (struct drm_bridge)
 2. handle gracefully atomic updates during bridge removal
 3. avoid DSI host drivers to have dangling pointers to DSI devices
 4. finish the hotplug bridge work, removing the "always-disconnected"
    connector, moving code to the core and potentially removing the
    hotplug-bridge itself (this needs to be clarified as points 1-3 are
    developed)

This series is part of step 1 of the grand plan.

Current tasks in step 1 of the grand plan:

 A. ✔ add new alloc API and refcounting -> (now in drm-misc-next)
 B. … convert all bridge drivers to new API (in progress)
 C. ➜ kunit tests (this series)
 D. after (B), add get/put to drm_bridge_add/remove() + attach/detech()
 E. after (B), convert accessors; this is a large work and can be done
    in chunks
 F. debugfs improvements

[0] https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/0cc6aadd7fc1e629b715ea3d1ba537ef2da95eec
[1] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/t/#u

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v9:
- Patch 1: rename structs according to conventions
- Patch 2: swap .destroy and .detach declarations
- Patch 3: rework to not introduce a new funcs struct, rename according to
           conventions, use more appropriate kunit test macros, etc
- Link to v8: https://lore.kernel.org/r/20250516-drm-bridge-alloc-doc-test-v8-0-7e356fd58ba5@bootlin.com

Changes in v8:
- Remove documentation patch
- Add patch to convert existing kunit tests to use devm_drm_bridge_alloc()
- Add tests for bridge deallocation (based on adding a .destroy callback)
- Link to v7: https://lore.kernel.org/r/20250409-drm-bridge-alloc-doc-test-v7-0-a3ca4b97597f@bootlin.com

---
Luca Ceresoli (3):
      drm/tests: bridge: convert to devm_drm_bridge_alloc() API
      drm/bridge: add a .destroy func
      drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc()

 drivers/gpu/drm/drm_bridge.c            |   2 +
 drivers/gpu/drm/tests/drm_bridge_test.c | 176 +++++++++++++++++++++++++-------
 include/drm/drm_bridge.h                |  10 ++
 3 files changed, 152 insertions(+), 36 deletions(-)
---
base-commit: 3671f37777589194c44bb9351568c13eee43da3c
change-id: 20250408-drm-bridge-alloc-doc-test-267df0def880

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v9 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API
  2025-06-06  8:41 [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc() Luca Ceresoli
@ 2025-06-06  8:41 ` Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 2/3] drm/bridge: add a .destroy func Luca Ceresoli
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Luca Ceresoli @ 2025-06-06  8:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec
  Cc: Anusha Srivatsa, Paul Kocialkowski, Dmitry Baryshkov,
	Hervé Codina, Hui Pu, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

Use the new DRM bridge allocation API, which is the only supported now, for
the kunit tests.

This change is more massive than for the typical DRM bridge driver because
struct drm_bridge_init_priv currently embeds a struct drm_bridge, which is
not supported anymore. We now have to use devm_drm_bridge_alloc() to
dynamically allocate a "private driver struct", which is a bit awkward here
because there is no real bridge driver. Thus let's add a "dummy" DRM bridge
struct to represent it.

As a nice cleanup we can now move the enable_count and disable_count
members, which are counting bridge-specific events, into the new "private
driver struct" (and avoid adding new unnecessary indirections).

Also add a trivial bridge_to_dummy_bridge() function just like many drivers
do.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v9:
* rename struct dummy_drm_bridge to drm_bridge_priv (and related variables
  accordingly)
* slightly improve struct drm_bridge_priv docs
* fix typo in commit message, improve commit message

This patch was added in v8.
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 87 +++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index ff88ec2e911c9cc9a718483f09d4c764f45f991a..af1b9f9694b0091e6eaef78cb71e39327ad8c3fa 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -10,29 +10,43 @@
 
 #include <kunit/test.h>
 
+/*
+ * Mimick the typical "private" struct defined by a bridge driver, which
+ * embeds a bridge plus other fields.
+ *
+ * Having at least one member before @bridge ensures we test non-zero
+ * @bridge offset.
+ */
+struct drm_bridge_priv {
+	unsigned int enable_count;
+	unsigned int disable_count;
+	struct drm_bridge bridge;
+};
+
 struct drm_bridge_init_priv {
 	struct drm_device drm;
 	struct drm_plane *plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder encoder;
-	struct drm_bridge bridge;
+	struct drm_bridge_priv *test_bridge;
 	struct drm_connector *connector;
-	unsigned int enable_count;
-	unsigned int disable_count;
 };
 
+static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct drm_bridge_priv, bridge);
+}
+
 static void drm_test_bridge_enable(struct drm_bridge *bridge)
 {
-	struct drm_bridge_init_priv *priv =
-		container_of(bridge, struct drm_bridge_init_priv, bridge);
+	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
 
 	priv->enable_count++;
 }
 
 static void drm_test_bridge_disable(struct drm_bridge *bridge)
 {
-	struct drm_bridge_init_priv *priv =
-		container_of(bridge, struct drm_bridge_init_priv, bridge);
+	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
 
 	priv->disable_count++;
 }
@@ -45,8 +59,7 @@ static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
 static void drm_test_bridge_atomic_enable(struct drm_bridge *bridge,
 					  struct drm_atomic_state *state)
 {
-	struct drm_bridge_init_priv *priv =
-		container_of(bridge, struct drm_bridge_init_priv, bridge);
+	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
 
 	priv->enable_count++;
 }
@@ -54,8 +67,7 @@ static void drm_test_bridge_atomic_enable(struct drm_bridge *bridge,
 static void drm_test_bridge_atomic_disable(struct drm_bridge *bridge,
 					   struct drm_atomic_state *state)
 {
-	struct drm_bridge_init_priv *priv =
-		container_of(bridge, struct drm_bridge_init_priv, bridge);
+	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
 
 	priv->disable_count++;
 }
@@ -102,6 +114,10 @@ drm_test_bridge_init(struct kunit *test, const struct drm_bridge_funcs *funcs)
 	if (IS_ERR(priv))
 		return ERR_CAST(priv);
 
+	priv->test_bridge = devm_drm_bridge_alloc(dev, struct drm_bridge_priv, bridge, funcs);
+	if (IS_ERR(priv->test_bridge))
+		return ERR_CAST(priv->test_bridge);
+
 	drm = &priv->drm;
 	priv->plane = drm_kunit_helper_create_primary_plane(test, drm,
 							    NULL,
@@ -125,9 +141,8 @@ drm_test_bridge_init(struct kunit *test, const struct drm_bridge_funcs *funcs)
 
 	enc->possible_crtcs = drm_crtc_mask(priv->crtc);
 
-	bridge = &priv->bridge;
+	bridge = &priv->test_bridge->bridge;
 	bridge->type = DRM_MODE_CONNECTOR_VIRTUAL;
-	bridge->funcs = funcs;
 
 	ret = drm_kunit_bridge_add(test, bridge);
 	if (ret)
@@ -173,7 +188,7 @@ static void drm_test_drm_bridge_get_current_state_atomic(struct kunit *test)
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
 
 retry_commit:
-	bridge = &priv->bridge;
+	bridge = &priv->test_bridge->bridge;
 	bridge_state = drm_atomic_get_bridge_state(state, bridge);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_state);
 
@@ -228,7 +243,7 @@ static void drm_test_drm_bridge_get_current_state_legacy(struct kunit *test)
 	 * locking. The function would return NULL in all cases anyway,
 	 * so we don't really have any concurrency to worry about.
 	 */
-	bridge = &priv->bridge;
+	bridge = &priv->test_bridge->bridge;
 	KUNIT_EXPECT_NULL(test, drm_bridge_get_current_state(bridge));
 }
 
@@ -253,7 +268,7 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic(struct kunit *test)
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_bridge_init_priv *priv;
 	struct drm_display_mode *mode;
-	struct drm_bridge *bridge;
+	struct drm_bridge_priv *bridge_priv;
 	int ret;
 
 	priv = drm_test_bridge_init(test, &drm_test_bridge_atomic_funcs);
@@ -279,14 +294,14 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic(struct kunit *test)
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
-	bridge = &priv->bridge;
-	KUNIT_ASSERT_EQ(test, priv->enable_count, 1);
-	KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
+	bridge_priv = priv->test_bridge;
+	KUNIT_ASSERT_EQ(test, bridge_priv->enable_count, 1);
+	KUNIT_ASSERT_EQ(test, bridge_priv->disable_count, 0);
 
 	drm_modeset_acquire_init(&ctx, 0);
 
 retry_reset:
-	ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
+	ret = drm_bridge_helper_reset_crtc(&bridge_priv->bridge, &ctx);
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
 		goto retry_reset;
@@ -296,8 +311,8 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic(struct kunit *test)
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
-	KUNIT_EXPECT_EQ(test, priv->enable_count, 2);
-	KUNIT_EXPECT_EQ(test, priv->disable_count, 1);
+	KUNIT_EXPECT_EQ(test, bridge_priv->enable_count, 2);
+	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 1);
 }
 
 /*
@@ -309,7 +324,7 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_bridge_init_priv *priv;
 	struct drm_display_mode *mode;
-	struct drm_bridge *bridge;
+	struct drm_bridge_priv *bridge_priv;
 	int ret;
 
 	priv = drm_test_bridge_init(test, &drm_test_bridge_atomic_funcs);
@@ -318,14 +333,14 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
 	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
 
-	bridge = &priv->bridge;
-	KUNIT_ASSERT_EQ(test, priv->enable_count, 0);
-	KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
+	bridge_priv = priv->test_bridge;
+	KUNIT_ASSERT_EQ(test, bridge_priv->enable_count, 0);
+	KUNIT_ASSERT_EQ(test, bridge_priv->disable_count, 0);
 
 	drm_modeset_acquire_init(&ctx, 0);
 
 retry_reset:
-	ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
+	ret = drm_bridge_helper_reset_crtc(&bridge_priv->bridge, &ctx);
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
 		goto retry_reset;
@@ -335,8 +350,8 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
-	KUNIT_EXPECT_EQ(test, priv->enable_count, 0);
-	KUNIT_EXPECT_EQ(test, priv->disable_count, 0);
+	KUNIT_EXPECT_EQ(test, bridge_priv->enable_count, 0);
+	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 0);
 }
 
 /*
@@ -348,7 +363,7 @@ static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_bridge_init_priv *priv;
 	struct drm_display_mode *mode;
-	struct drm_bridge *bridge;
+	struct drm_bridge_priv *bridge_priv;
 	int ret;
 
 	priv = drm_test_bridge_init(test, &drm_test_bridge_legacy_funcs);
@@ -374,14 +389,14 @@ static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
-	bridge = &priv->bridge;
-	KUNIT_ASSERT_EQ(test, priv->enable_count, 1);
-	KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
+	bridge_priv = priv->test_bridge;
+	KUNIT_ASSERT_EQ(test, bridge_priv->enable_count, 1);
+	KUNIT_ASSERT_EQ(test, bridge_priv->disable_count, 0);
 
 	drm_modeset_acquire_init(&ctx, 0);
 
 retry_reset:
-	ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
+	ret = drm_bridge_helper_reset_crtc(&bridge_priv->bridge, &ctx);
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
 		goto retry_reset;
@@ -391,8 +406,8 @@ static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
-	KUNIT_EXPECT_EQ(test, priv->enable_count, 2);
-	KUNIT_EXPECT_EQ(test, priv->disable_count, 1);
+	KUNIT_EXPECT_EQ(test, bridge_priv->enable_count, 2);
+	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 1);
 }
 
 static struct kunit_case drm_bridge_helper_reset_crtc_tests[] = {

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v9 2/3] drm/bridge: add a .destroy func
  2025-06-06  8:41 [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc() Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API Luca Ceresoli
@ 2025-06-06  8:41 ` Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 3/3] drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc() Luca Ceresoli
  2025-06-06 13:51 ` [PATCH v9 0/3] drm/bridge: add kunit " Maxime Ripard
  3 siblings, 0 replies; 5+ messages in thread
From: Luca Ceresoli @ 2025-06-06  8:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec
  Cc: Anusha Srivatsa, Paul Kocialkowski, Dmitry Baryshkov,
	Hervé Codina, Hui Pu, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

Some users of DRM bridges may need to execute specific code just before
deallocation.

As of now the only known user would be KUnit tests.

Suggested-by: Maxime Ripard <mripard@kernel.org>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---

Changes in v9:
- move .destroy before .detach in struct drm_bridge_funcs declaration for
  alphabetic ordering
- Add Maxime's Acked-by
- Fix typo in commit message

This patch was added in v8. The .destroy callback had appeared in v5 as
well [5], but as part of a larger patch and for different reasons that do
not apply anymore.

[5] https://lore.kernel.org/all/20241231-hotplug-drm-bridge-v5-3-173065a1ece1@bootlin.com/#t
---
 drivers/gpu/drm/drm_bridge.c |  2 ++
 include/drm/drm_bridge.h     | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index d0e81639927a73422671b7b43f3e8a946f1d4300..bf34cc5ab0316889186ddf5ebb3a984b09eeb86c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -203,6 +203,8 @@ static void __drm_bridge_free(struct kref *kref)
 {
 	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
 
+	if (bridge->funcs->destroy)
+		bridge->funcs->destroy(bridge);
 	kfree(bridge->container);
 }
 
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0af5db244db8580ea0c9af1d9a373b7bf62ee699..dbd6503f33ba917fbcfb73b985d9c34910f04e0f 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -77,6 +77,16 @@ struct drm_bridge_funcs {
 	int (*attach)(struct drm_bridge *bridge, struct drm_encoder *encoder,
 		      enum drm_bridge_attach_flags flags);
 
+	/**
+	 * @destroy:
+	 *
+	 * This callback is invoked when the bridge is about to be
+	 * deallocated.
+	 *
+	 * The @destroy callback is optional.
+	 */
+	void (*destroy)(struct drm_bridge *bridge);
+
 	/**
 	 * @detach:
 	 *

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v9 3/3] drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc()
  2025-06-06  8:41 [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc() Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API Luca Ceresoli
  2025-06-06  8:41 ` [PATCH v9 2/3] drm/bridge: add a .destroy func Luca Ceresoli
@ 2025-06-06  8:41 ` Luca Ceresoli
  2025-06-06 13:51 ` [PATCH v9 0/3] drm/bridge: add kunit " Maxime Ripard
  3 siblings, 0 replies; 5+ messages in thread
From: Luca Ceresoli @ 2025-06-06  8:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec
  Cc: Anusha Srivatsa, Paul Kocialkowski, Dmitry Baryshkov,
	Hervé Codina, Hui Pu, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

Add KUnit tests for the newly introduced devm_drm_bridge_alloc().

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changed in v9:
 - fold drm_bridge_alloc_test_ctx into drm_bridge_init_priv (do not
   introduce another test context struct specific to alloc tests)
 - use the existing func sets for .destroy (do not introduce another struct
   drm_ridge_funcs specific to alloc tests)
 - adapt existing tests accordingly by setting .data, to allow .destroy to
   work
 - rename 'ctx' variables to 'priv'
 - add comment describing new tests
 - use KUNIT_EXPECT_TRUE instead of KUNIT_ASSERT_TRUE where applicable

Changed in v8:
 - rebase on new patch converting drm_bridge_test.c to
   devm_drm_bridge_alloc()
 - add check that bridge is removed (thanks to the .destroy callback)
 - add a check with get/put

Changed in v7:
 - rebase on current drm-misc-next, which now has a drm_bridge_test.c file
 - cleanup commit message

Changed in v6:
 - update to new devm_drm_bridge_alloc() API
 - remove drm_test_drm_bridge_put test, not straightforward to write with
   the new API and the current notification mechanism
 - do not allocate a drm_device: a bridge is allocated without one
 - rename some identifiers for easier code reading

This patch was added in v5.
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 89 +++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index af1b9f9694b0091e6eaef78cb71e39327ad8c3fa..887020141c7f42b5ae74655f9544c224d3b07605 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -8,6 +8,7 @@
 #include <drm/drm_bridge_helper.h>
 #include <drm/drm_kunit_helpers.h>
 
+#include <kunit/device.h>
 #include <kunit/test.h>
 
 /*
@@ -21,15 +22,19 @@ struct drm_bridge_priv {
 	unsigned int enable_count;
 	unsigned int disable_count;
 	struct drm_bridge bridge;
+	void *data;
 };
 
 struct drm_bridge_init_priv {
 	struct drm_device drm;
+	/** @dev: device, only for tests not needing a whole drm_device */
+	struct device *dev;
 	struct drm_plane *plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder encoder;
 	struct drm_bridge_priv *test_bridge;
 	struct drm_connector *connector;
+	bool destroyed;
 };
 
 static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
@@ -37,6 +42,14 @@ static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
 	return container_of(bridge, struct drm_bridge_priv, bridge);
 }
 
+static void drm_test_bridge_priv_destroy(struct drm_bridge *bridge)
+{
+	struct drm_bridge_priv *bridge_priv = bridge_to_priv(bridge);
+	struct drm_bridge_init_priv *priv = (struct drm_bridge_init_priv *)bridge_priv->data;
+
+	priv->destroyed = true;
+}
+
 static void drm_test_bridge_enable(struct drm_bridge *bridge)
 {
 	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
@@ -52,6 +65,7 @@ static void drm_test_bridge_disable(struct drm_bridge *bridge)
 }
 
 static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
+	.destroy		= drm_test_bridge_priv_destroy,
 	.enable			= drm_test_bridge_enable,
 	.disable		= drm_test_bridge_disable,
 };
@@ -73,6 +87,7 @@ static void drm_test_bridge_atomic_disable(struct drm_bridge *bridge,
 }
 
 static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
+	.destroy		= drm_test_bridge_priv_destroy,
 	.atomic_enable		= drm_test_bridge_atomic_enable,
 	.atomic_disable		= drm_test_bridge_atomic_disable,
 	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
@@ -118,6 +133,8 @@ drm_test_bridge_init(struct kunit *test, const struct drm_bridge_funcs *funcs)
 	if (IS_ERR(priv->test_bridge))
 		return ERR_CAST(priv->test_bridge);
 
+	priv->test_bridge->data = priv;
+
 	drm = &priv->drm;
 	priv->plane = drm_kunit_helper_create_primary_plane(test, drm,
 							    NULL,
@@ -422,11 +439,83 @@ static struct kunit_suite drm_bridge_helper_reset_crtc_test_suite = {
 	.test_cases = drm_bridge_helper_reset_crtc_tests,
 };
 
+static int drm_test_bridge_alloc_init(struct kunit *test)
+{
+	struct drm_bridge_init_priv *priv;
+
+	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	priv->dev = kunit_device_register(test, "drm-bridge-dev");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);
+
+	test->priv = priv;
+
+	priv->test_bridge = devm_drm_bridge_alloc(priv->dev, struct drm_bridge_priv, bridge,
+						  &drm_test_bridge_atomic_funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->test_bridge);
+
+	priv->test_bridge->data = priv;
+
+	KUNIT_ASSERT_FALSE(test, priv->destroyed);
+
+	return 0;
+}
+
+/*
+ * Test that a bridge is freed when the device is destroyed in lack of
+ * other drm_bridge_get/put() operations.
+ */
+static void drm_test_drm_bridge_alloc_basic(struct kunit *test)
+{
+	struct drm_bridge_init_priv *priv = test->priv;
+
+	KUNIT_ASSERT_FALSE(test, priv->destroyed);
+
+	kunit_device_unregister(test, priv->dev);
+	KUNIT_EXPECT_TRUE(test, priv->destroyed);
+}
+
+/*
+ * Test that a bridge is not freed when the device is destroyed when there
+ * is still a reference to it, and freed when that reference is put.
+ */
+static void drm_test_drm_bridge_alloc_get_put(struct kunit *test)
+{
+	struct drm_bridge_init_priv *priv = test->priv;
+
+	KUNIT_ASSERT_FALSE(test, priv->destroyed);
+
+	drm_bridge_get(&priv->test_bridge->bridge);
+	KUNIT_EXPECT_FALSE(test, priv->destroyed);
+
+	kunit_device_unregister(test, priv->dev);
+	KUNIT_EXPECT_FALSE(test, priv->destroyed);
+
+	drm_bridge_put(&priv->test_bridge->bridge);
+	KUNIT_EXPECT_TRUE(test, priv->destroyed);
+}
+
+static struct kunit_case drm_bridge_alloc_tests[] = {
+	KUNIT_CASE(drm_test_drm_bridge_alloc_basic),
+	KUNIT_CASE(drm_test_drm_bridge_alloc_get_put),
+	{ }
+};
+
+static struct kunit_suite drm_bridge_alloc_test_suite = {
+	.name = "drm_bridge_alloc",
+	.init = drm_test_bridge_alloc_init,
+	.test_cases = drm_bridge_alloc_tests,
+};
+
 kunit_test_suites(
 	&drm_bridge_get_current_state_test_suite,
 	&drm_bridge_helper_reset_crtc_test_suite,
+	&drm_bridge_alloc_test_suite,
 );
 
 MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
+MODULE_AUTHOR("Luca Ceresoli <luca.ceresoli@bootlin.com>");
+
 MODULE_DESCRIPTION("Kunit test for drm_bridge functions");
 MODULE_LICENSE("GPL");

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc()
  2025-06-06  8:41 [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc() Luca Ceresoli
                   ` (2 preceding siblings ...)
  2025-06-06  8:41 ` [PATCH v9 3/3] drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc() Luca Ceresoli
@ 2025-06-06 13:51 ` Maxime Ripard
  3 siblings, 0 replies; 5+ messages in thread
From: Maxime Ripard @ 2025-06-06 13:51 UTC (permalink / raw)
  To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli
  Cc: Maxime Ripard, Anusha Srivatsa, Paul Kocialkowski,
	Dmitry Baryshkov, Hervé Codina, Hui Pu, Thomas Petazzoni,
	dri-devel, linux-kernel

On Fri, 06 Jun 2025 10:41:18 +0200, Luca Ceresoli wrote:
> This small series adds a few kunit tests for the new DRM bridge allocation
> flow, based on the recently introduced devm_drm_bridge_alloc() [0].
> 
> It is part of the work towards removal of bridges from a still existing DRM
> pipeline without use-after-free.
> 
> The steps in the grand plan [1] are:
> 
> [...]

Applied to misc/kernel.git (drm-misc-next).

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-06 13:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-06  8:41 [PATCH v9 0/3] drm/bridge: add kunit tests for devm_drm_bridge_alloc() Luca Ceresoli
2025-06-06  8:41 ` [PATCH v9 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API Luca Ceresoli
2025-06-06  8:41 ` [PATCH v9 2/3] drm/bridge: add a .destroy func Luca Ceresoli
2025-06-06  8:41 ` [PATCH v9 3/3] drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc() Luca Ceresoli
2025-06-06 13:51 ` [PATCH v9 0/3] drm/bridge: add kunit " Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).