[PATCH] drm/radeon: Create the primary plane in the driver

[PATCH] drm/radeon: Create the primary plane in the driver

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

drm_crtc_init() creates the primary plane from a fixed format list
without a "pixel blend mode" property, so drm_mode_config_validate()
warns although the display engine does support per-pixel alpha on the
primary surface.

Replace it with a driver-owned primary plane advertising XRGB8888 and
ARGB8888, with the blend mode property set to the PREMULTI mode
userspace has always assumed when the property was absent.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
 drivers/gpu/drm/radeon/radeon_display.c | 42 ++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index aac6733ddd82..058ec866f02b 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -30,6 +30,7 @@
 
 #include <asm/div64.h>
 
+#include <drm/drm_blend.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
@@ -38,6 +39,8 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modeset_helper.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
 #include <drm/radeon_drm.h>
@@ -664,6 +667,19 @@ radeon_crtc_set_config(struct drm_mode_set *set,
 	return ret;
 }
 
+static const uint32_t radeon_primary_formats[] = {
+	/*
+	 * The display engine programs an ARGB8888 surface format for both
+	 * XRGB8888 and ARGB8888 framebuffers.
+	 */
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ARGB8888,
+};
+
+static const struct drm_plane_funcs radeon_primary_plane_funcs = {
+	DRM_PLANE_NON_ATOMIC_FUNCS,
+};
+
 static const struct drm_crtc_funcs radeon_crtc_funcs = {
 	.cursor_set2 = radeon_crtc_cursor_set2,
 	.cursor_move = radeon_crtc_cursor_move,
@@ -681,6 +697,8 @@ static void radeon_crtc_init(struct drm_device *dev, int index)
 {
 	struct radeon_device *rdev = dev->dev_private;
 	struct radeon_crtc *radeon_crtc;
+	struct drm_plane *primary;
+	int ret;
 
 	radeon_crtc = kzalloc_obj(*radeon_crtc);
 	if (radeon_crtc == NULL)
@@ -693,7 +711,29 @@ static void radeon_crtc_init(struct drm_device *dev, int index)
 		return;
 	}
 
-	drm_crtc_init(dev, &radeon_crtc->base, &radeon_crtc_funcs);
+	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
+					      &radeon_primary_plane_funcs,
+					      radeon_primary_formats,
+					      ARRAY_SIZE(radeon_primary_formats),
+					      NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(primary)) {
+		destroy_workqueue(radeon_crtc->flip_queue);
+		kfree(radeon_crtc);
+		return;
+	}
+
+	ret = drm_crtc_init_with_planes(dev, &radeon_crtc->base, primary, NULL,
+					&radeon_crtc_funcs, NULL);
+	if (ret) {
+		dev_err(dev->dev, "Failed to init CRTC %d: %d\n", index, ret);
+		drm_plane_cleanup(primary);
+		kfree(primary);
+		destroy_workqueue(radeon_crtc->flip_queue);
+		return;
+	}
+
+	drm_plane_create_blend_mode_property(primary,
+					     BIT(DRM_MODE_BLEND_PREMULTI));
 
 	drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256);
 	radeon_crtc->crtc_id = index;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH] drm/radeon: Create the primary plane in the driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Backwards Compatibility / Functional Regression: Restricting supported formats on primary plane
- [Medium] Memory leak of `radeon_crtc` on the error path when `drm_crtc_init_with_planes()` fails.
--

commit a5951b1d6bc3cbef461bfcabdab5aeff88a28da4
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/radeon: Create the primary plane in the driver

This commit replaces the fixed format primary plane created by
drm_crtc_init() with a driver-owned primary plane in the radeon driver.
This allows the driver to set the blend mode property to PREMULTI, resolving
warnings during drm_mode_config_validate().

> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index aac6733ddd82c..058ec866f02b8 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
[ ... ]
> @@ -664,6 +667,19 @@ radeon_crtc_set_config(struct drm_mode_set *set,
>  	return ret;
>  }
>  
> +static const uint32_t radeon_primary_formats[] = {
> +	/*
> +	 * The display engine programs an ARGB8888 surface format for both
> +	 * XRGB8888 and ARGB8888 framebuffers.
> +	 */
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_ARGB8888,
> +};
> +
> +static const struct drm_plane_funcs radeon_primary_plane_funcs = {
> +	DRM_PLANE_NON_ATOMIC_FUNCS,
> +};
> +
>  static const struct drm_crtc_funcs radeon_crtc_funcs = {
>  	.cursor_set2 = radeon_crtc_cursor_set2,
>  	.cursor_move = radeon_crtc_cursor_move,
[ ... ]
> @@ -693,7 +711,29 @@ static void radeon_crtc_init(struct drm_device *dev, int index)
>  		return;
>  	}
>  
> -	drm_crtc_init(dev, &radeon_crtc->base, &radeon_crtc_funcs);
> +	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
> +					      &radeon_primary_plane_funcs,
> +					      radeon_primary_formats,
> +					      ARRAY_SIZE(radeon_primary_formats),
> +					      NULL, DRM_PLANE_TYPE_PRIMARY, NULL);

[Severity: High]
Will this break backwards compatibility for userspace programs configuring
legacy pixel formats?

The array radeon_primary_formats explicitly lists only DRM_FORMAT_XRGB8888
and DRM_FORMAT_ARGB8888. When __drm_universal_plane_alloc() is called here,
it leaves primary->format_default as false (since it is zero-initialized).

Because of this, the core DRM modeset logic in drm_mode_setcrtc() will now
strictly check pixel formats against this restricted list and return -EINVAL
for other formats. The hardware supports and previously accepted legacy
formats (like RGB565, 8-bit, 15-bit, and 24-bit color) which are handled in
radeon_crtc_do_set_base().

> +	if (IS_ERR(primary)) {
> +		destroy_workqueue(radeon_crtc->flip_queue);
> +		kfree(radeon_crtc);
> +		return;
> +	}
> +
> +	ret = drm_crtc_init_with_planes(dev, &radeon_crtc->base, primary, NULL,
> +					&radeon_crtc_funcs, NULL);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to init CRTC %d: %d\n", index, ret);
> +		drm_plane_cleanup(primary);
> +		kfree(primary);
> +		destroy_workqueue(radeon_crtc->flip_queue);
> +		return;
> +	}

[Severity: Medium]
Does this error path leak the dynamically allocated radeon_crtc object?

If drm_crtc_init_with_planes() fails during device initialization, the code
cleans up the primary plane and destroys the flip queue, but it appears to
return without calling kfree(radeon_crtc).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902100739.627697-1-oushixiong1025@163.com?part=1