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

[PATCH v2] drm/gma500: 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
that includes ARGB8888. The display engine programs the primary plane
with DISPPLANE_32BPP_NO_ALPHA, so it does not support per-pixel alpha
and must not advertise alpha formats. Since commit 860e748bddcc
("drm: ensure blend mode supported if pixel format with alpha exposed"),
drm_mode_config_validate() warns about this at probe time.

Replace drm_crtc_init() with a driver-owned primary plane that
advertises only XRGB8888. The plane is allocated by
drmm_universal_plane_alloc() and the "pixel blend mode" property is
not needed because no format with alpha is exposed.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
v1->v2:
  - Open-code DRM_PLANE_NON_ATOMIC_FUNCS without .destroy, which
    drmm_universal_plane_alloc() rejects
  - Use drmm_universal_plane_alloc() for automatic cleanup
  - Move the formats comment above the array

 drivers/gpu/drm/gma500/psb_intel_display.c | 34 +++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
index 0df75a4a7739..f79484b9d593 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_display.c
@@ -9,8 +9,11 @@
 #include <linux/delay.h>
 #include <linux/i2c.h>
 
+#include <drm/drm_fourcc.h>
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_plane_helper.h>
 #include <drm/drm_print.h>
 
 #include "framebuffer.h"
@@ -471,11 +474,25 @@ static void psb_intel_cursor_init(struct drm_device *dev,
 	REG_WRITE(base[gma_crtc->pipe], 0);
 }
 
+/*
+ * The display engine programs the primary plane with
+ * DISPPLANE_32BPP_NO_ALPHA, so no formats with alpha.
+ */
+static const uint32_t gma_primary_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static const struct drm_plane_funcs gma_primary_plane_funcs = {
+	.update_plane = drm_plane_helper_update_primary,
+	.disable_plane = drm_plane_helper_disable_primary,
+};
+
 void psb_intel_crtc_init(struct drm_device *dev, int pipe,
 		     struct psb_intel_mode_device *mode_dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct gma_crtc *gma_crtc;
+	struct drm_plane *primary;
 	int i;
 
 	/* We allocate a extra array of drm_connector pointers
@@ -494,7 +511,17 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
 		return;
 	}
 
-	drm_crtc_init(dev, &gma_crtc->base, &gma_crtc_funcs);
+	primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0,
+					     &gma_primary_plane_funcs,
+					     gma_primary_formats,
+					     ARRAY_SIZE(gma_primary_formats),
+					     NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(primary))
+		goto err_free_crtc;
+
+	if (drm_crtc_init_with_planes(dev, &gma_crtc->base, primary, NULL,
+				      &gma_crtc_funcs, NULL))
+		goto err_free_crtc;
 
 	/* Set the CRTC clock functions from chip specific data */
 	gma_crtc->clock_funcs = dev_priv->ops->clock_funcs;
@@ -524,6 +551,11 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
 
 	/* Set to true so that the pipe is forced off on initial config. */
 	gma_crtc->active = true;
+	return;
+
+err_free_crtc:
+	kfree(gma_crtc->crtc_state);
+	kfree(gma_crtc);
 }
 
 struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Freeing `gma_crtc` on `drm_crtc_init_with_planes()` failure causes a Use-After-Free because the CRTC remains linked in `dev->mode_config.crtc_list`.
--

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

drm/gma500: Create the primary plane in the driver

drm_crtc_init() creates the primary plane from a fixed format list
that includes ARGB8888. The display engine programs the primary plane
with DISPPLANE_32BPP_NO_ALPHA, so it does not support per-pixel alpha
and must not advertise alpha formats. This commit replaces drm_crtc_init()
with a driver-owned primary plane that advertises only XRGB8888.

> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> index 0df75a4a77396..f79484b9d593e 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
[ ... ]
> @@ -494,7 +511,17 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>  		return;
>  	}
>  
> -	drm_crtc_init(dev, &gma_crtc->base, &gma_crtc_funcs);
> +	primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0,
> +					     &gma_primary_plane_funcs,
> +					     gma_primary_formats,
> +					     ARRAY_SIZE(gma_primary_formats),
> +					     NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (IS_ERR(primary))
> +		goto err_free_crtc;
> +
> +	if (drm_crtc_init_with_planes(dev, &gma_crtc->base, primary, NULL,
> +				      &gma_crtc_funcs, NULL))
> +		goto err_free_crtc;

[Severity: High]
Could this error handling path leave a freed pointer linked in the DRM core?

Looking at __drm_crtc_init_with_planes(), the CRTC is added to the list early
in the function:

	list_add_tail(&crtc->head, &config->crtc_list);
	crtc->index = config->num_crtc++;

If a subsequent initialization step fails (such as drm_crtc_crc_init()),
__drm_crtc_init_with_planes() returns an error without removing the CRTC from
the list.

When jumping to err_free_crtc here, gma_crtc is freed via kfree() below. Might
this create a use-after-free condition when dev->mode_config.crtc_list is later
traversed?

[ ... ]
> @@ -524,6 +551,11 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>  
>  	/* Set to true so that the pipe is forced off on initial config. */
>  	gma_crtc->active = true;
> +	return;
> +
> +err_free_crtc:
> +	kfree(gma_crtc->crtc_state);
> +	kfree(gma_crtc);
>  }
>  
>  struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)

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

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

[Patrik Jakobsson]

On Thu, Sep 3, 2026 at 9:12 AM <oushixiong1025@163.com> wrote:
>
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> drm_crtc_init() creates the primary plane from a fixed format list
> that includes ARGB8888. The display engine programs the primary plane
> with DISPPLANE_32BPP_NO_ALPHA, so it does not support per-pixel alpha
> and must not advertise alpha formats. Since commit 860e748bddcc
> ("drm: ensure blend mode supported if pixel format with alpha exposed"),
> drm_mode_config_validate() warns about this at probe time.
>
> Replace drm_crtc_init() with a driver-owned primary plane that
> advertises only XRGB8888. The plane is allocated by
> drmm_universal_plane_alloc() and the "pixel blend mode" property is
> not needed because no format with alpha is exposed.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>

Thanks for the patch. Applied to drm-misc-next

-Patrik

> ---
> v1->v2:
>   - Open-code DRM_PLANE_NON_ATOMIC_FUNCS without .destroy, which
>     drmm_universal_plane_alloc() rejects
>   - Use drmm_universal_plane_alloc() for automatic cleanup
>   - Move the formats comment above the array
>
>  drivers/gpu/drm/gma500/psb_intel_display.c | 34 +++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> index 0df75a4a7739..f79484b9d593 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
> @@ -9,8 +9,11 @@
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
>
> +#include <drm/drm_fourcc.h>
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_plane_helper.h>
>  #include <drm/drm_print.h>
>
>  #include "framebuffer.h"
> @@ -471,11 +474,25 @@ static void psb_intel_cursor_init(struct drm_device *dev,
>         REG_WRITE(base[gma_crtc->pipe], 0);
>  }
>
> +/*
> + * The display engine programs the primary plane with
> + * DISPPLANE_32BPP_NO_ALPHA, so no formats with alpha.
> + */
> +static const uint32_t gma_primary_formats[] = {
> +       DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_plane_funcs gma_primary_plane_funcs = {
> +       .update_plane = drm_plane_helper_update_primary,
> +       .disable_plane = drm_plane_helper_disable_primary,
> +};
> +
>  void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>                      struct psb_intel_mode_device *mode_dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>         struct gma_crtc *gma_crtc;
> +       struct drm_plane *primary;
>         int i;
>
>         /* We allocate a extra array of drm_connector pointers
> @@ -494,7 +511,17 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>                 return;
>         }
>
> -       drm_crtc_init(dev, &gma_crtc->base, &gma_crtc_funcs);
> +       primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0,
> +                                            &gma_primary_plane_funcs,
> +                                            gma_primary_formats,
> +                                            ARRAY_SIZE(gma_primary_formats),
> +                                            NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> +       if (IS_ERR(primary))
> +               goto err_free_crtc;
> +
> +       if (drm_crtc_init_with_planes(dev, &gma_crtc->base, primary, NULL,
> +                                     &gma_crtc_funcs, NULL))
> +               goto err_free_crtc;
>
>         /* Set the CRTC clock functions from chip specific data */
>         gma_crtc->clock_funcs = dev_priv->ops->clock_funcs;
> @@ -524,6 +551,11 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>
>         /* Set to true so that the pipe is forced off on initial config. */
>         gma_crtc->active = true;
> +       return;
> +
> +err_free_crtc:
> +       kfree(gma_crtc->crtc_state);
> +       kfree(gma_crtc);
>  }
>
>  struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
> --
> 2.25.1
>
>
> No virus found
>                 Checked by Hillstone Network AntiVirus
>