[PATCH 2/4] drm/crtc: Make init functions panic consitently and explicitly

Maxime Ripard mripard at kernel.org
Wed Dec 6 11:13:49 UTC 2023


All of the current CRTC init / allocation functions behave slightly
differently when it comes to argument sanitizing:

 - drm_crtc_init_with_planes() implicitly panics if the drm_device
   pointer, the drm_crtc pointer, or the drm_crtc_funcs pointer are
   NULL, and calls WARN_ON if there's no destroy implementation but
   goes on with the initialization.

 - drmm_crtc_init_with_planes() implicitly panics if the drm_device
   pointer, the drm_crtc pointer, or the drm_crtc_funcs pointer are
   NULL, and calls WARN_ON if there's no destroy implementation but
   goes on with the initialization.

 - drmm_crtc_alloc_with_planes() implicitly panics if the drm_device
   pointer, or the drm_crtc pointer are NULL, and calls WARN_ON if
   the drm_crtc_funcs pointer is NULL or there's no destroy
   implementation but goes on with the initialization.

The current consensus is that the drm_device pointer, the
drm_crtc_funcs pointer, and the drm_crtc pointer if relevant, should be
considered pre-requisite and the function should panic if we encounter
such a situation, and that returning an error in such a situation is not
welcome.

Let's make all functions consider those three pointers to be always set
and explicitly panic if they aren't. And let's document that behaviour
too.

Link: https://lore.kernel.org/dri-devel/20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org/
Signed-off-by: Maxime Ripard <mripard at kernel.org>
---
 drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++--
 include/drm/drm_crtc.h     |  3 +++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index df9bf3c9206e..87e5877b753d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -350,6 +350,9 @@ static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @crtc, or @funcs are NULL.
  */
 int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 			      struct drm_plane *primary,
@@ -360,6 +363,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 	va_list ap;
 	int ret;
 
+	BUG_ON(!dev);
+	BUG_ON(!crtc);
+	BUG_ON(!funcs);
 	WARN_ON(!funcs->destroy);
 
 	va_start(ap, name);
@@ -432,6 +438,9 @@ static int __drmm_crtc_init_with_planes(struct drm_device *dev,
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @crtc, or @funcs are NULL.
  */
 int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 			       struct drm_plane *primary,
@@ -442,6 +451,10 @@ int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 	va_list ap;
 	int ret;
 
+	BUG_ON(!dev);
+	BUG_ON(!crtc);
+	BUG_ON(!funcs);
+
 	va_start(ap, name);
 	ret = __drmm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
 					   name, ap);
@@ -465,8 +478,9 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device *dev,
 	va_list ap;
 	int ret;
 
-	if (WARN_ON(!funcs || funcs->destroy))
-		return ERR_PTR(-EINVAL);
+	BUG_ON(!dev);
+	BUG_ON(!funcs);
+	WARN_ON(funcs->destroy);
 
 	container = drmm_kzalloc(dev, size, GFP_KERNEL);
 	if (!container)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8b48a1974da3..fdcbc3ac9e08 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1248,6 +1248,9 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device *dev,
  *
  * Returns:
  * Pointer to new crtc, or ERR_PTR on failure.
+ *
+ * Panics:
+ * If @dev or @funcs are NULL.
  */
 #define drmm_crtc_alloc_with_planes(dev, type, member, primary, cursor, funcs, name, ...) \
 	((type *)__drmm_crtc_alloc_with_planes(dev, sizeof(type), \
-- 
2.43.0



More information about the dri-devel mailing list