[PATCH 1/2] drm/verisilicon: add primary modifier for format tables

[PATCH 1/2] drm/verisilicon: add primary modifier for format tables

[Icenowy Zheng]

Currently the format tables are only used for the primary plane.

Add primary modifiers to names related to the tables.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_hwdb.c          | 12 ++++++------
 drivers/gpu/drm/verisilicon/vs_hwdb.h          |  4 ++--
 drivers/gpu/drm/verisilicon/vs_primary_plane.c |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
index 2a0f7c59afa3a..56aa450443068 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
@@ -10,7 +10,7 @@
 #include "vs_dc_top_regs.h"
 #include "vs_hwdb.h"
 
-static const u32 vs_formats_array_no_yuv444[] = {
+static const u32 vs_primary_formats_array_no_yuv444[] = {
 	DRM_FORMAT_XRGB4444,
 	DRM_FORMAT_XBGR4444,
 	DRM_FORMAT_RGBX4444,
@@ -44,7 +44,7 @@ static const u32 vs_formats_array_no_yuv444[] = {
 	/* TODO: non-RGB formats */
 };
 
-static const u32 vs_formats_array_with_yuv444[] = {
+static const u32 vs_primary_formats_array_with_yuv444[] = {
 	DRM_FORMAT_XRGB4444,
 	DRM_FORMAT_XBGR4444,
 	DRM_FORMAT_RGBX4444,
@@ -79,13 +79,13 @@ static const u32 vs_formats_array_with_yuv444[] = {
 };
 
 static const struct vs_formats vs_formats_no_yuv444 = {
-	.array = vs_formats_array_no_yuv444,
-	.num = ARRAY_SIZE(vs_formats_array_no_yuv444)
+	.primary_array = vs_primary_formats_array_no_yuv444,
+	.primary_num = ARRAY_SIZE(vs_primary_formats_array_no_yuv444)
 };
 
 static const struct vs_formats vs_formats_with_yuv444 = {
-	.array = vs_formats_array_with_yuv444,
-	.num = ARRAY_SIZE(vs_formats_array_with_yuv444)
+	.primary_array = vs_primary_formats_array_with_yuv444,
+	.primary_num = ARRAY_SIZE(vs_primary_formats_array_with_yuv444)
 };
 
 static struct vs_chip_identity vs_chip_identities[] = {
diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h b/drivers/gpu/drm/verisilicon/vs_hwdb.h
index 2065ecb730437..616076d931a57 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
@@ -10,8 +10,8 @@
 #include <linux/types.h>
 
 struct vs_formats {
-	const u32 *array;
-	unsigned int num;
+	const u32 *primary_array;
+	unsigned int primary_num;
 };
 
 struct vs_chip_identity {
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 1f2be41ae496c..8e94491693019 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -168,8 +168,8 @@ struct drm_plane *vs_primary_plane_init(struct drm_device *drm_dev, struct vs_dc
 
 	plane = drmm_universal_plane_alloc(drm_dev, struct drm_plane, dev, 0,
 					   &vs_primary_plane_funcs,
-					   dc->identity.formats->array,
-					   dc->identity.formats->num,
+					   dc->identity.formats->primary_array,
+					   dc->identity.formats->primary_num,
 					   NULL,
 					   DRM_PLANE_TYPE_PRIMARY,
 					   NULL);
-- 
2.52.0

[PATCH 2/2] drm/verisilicon: remove ARGB formats from primary plane

[Icenowy Zheng]

As the blending of the primary plane is currently explicitly disabled
(and it's not possible on DC8000), remove the ARGB formats from the
primary plane format tables.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_hwdb.c | 32 ---------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
index 56aa450443068..ebf6f843bc885 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
@@ -15,32 +15,16 @@ static const u32 vs_primary_formats_array_no_yuv444[] = {
 	DRM_FORMAT_XBGR4444,
 	DRM_FORMAT_RGBX4444,
 	DRM_FORMAT_BGRX4444,
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ABGR4444,
-	DRM_FORMAT_RGBA4444,
-	DRM_FORMAT_BGRA4444,
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_XBGR1555,
 	DRM_FORMAT_RGBX5551,
 	DRM_FORMAT_BGRX5551,
-	DRM_FORMAT_ARGB1555,
-	DRM_FORMAT_ABGR1555,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_BGRA5551,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_BGR565,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_RGBX8888,
 	DRM_FORMAT_BGRX8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_RGBA8888,
-	DRM_FORMAT_BGRA8888,
-	DRM_FORMAT_ARGB2101010,
-	DRM_FORMAT_ABGR2101010,
-	DRM_FORMAT_RGBA1010102,
-	DRM_FORMAT_BGRA1010102,
 	/* TODO: non-RGB formats */
 };
 
@@ -49,32 +33,16 @@ static const u32 vs_primary_formats_array_with_yuv444[] = {
 	DRM_FORMAT_XBGR4444,
 	DRM_FORMAT_RGBX4444,
 	DRM_FORMAT_BGRX4444,
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ABGR4444,
-	DRM_FORMAT_RGBA4444,
-	DRM_FORMAT_BGRA4444,
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_XBGR1555,
 	DRM_FORMAT_RGBX5551,
 	DRM_FORMAT_BGRX5551,
-	DRM_FORMAT_ARGB1555,
-	DRM_FORMAT_ABGR1555,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_BGRA5551,
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_BGR565,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_RGBX8888,
 	DRM_FORMAT_BGRX8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_RGBA8888,
-	DRM_FORMAT_BGRA8888,
-	DRM_FORMAT_ARGB2101010,
-	DRM_FORMAT_ABGR2101010,
-	DRM_FORMAT_RGBA1010102,
-	DRM_FORMAT_BGRA1010102,
 	/* TODO: non-RGB formats */
 };
 
-- 
2.52.0

Re: [PATCH 2/2] drm/verisilicon: remove ARGB formats from primary plane

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Unintentional removal of 10-bit color formats from the primary plane.

Pre-existing issues:
- [High] Plane `atomic_disable` crashes when called on a previously disabled plane.
- [High] Primary plane `atomic_disable` mistakenly enables the plane in the hardware register.
--

commit 53c18bf18dd1a86e14f16784c764ae73c2637d54
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/verisilicon: remove ARGB formats from primary plane
    
As the blending of the primary plane is currently explicitly disabled
(and it's not possible on DC8000), remove the ARGB formats from the
primary plane format tables.

> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 56aa450443068..ebf6f843bc885 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -15,32 +15,16 @@ static const u32 vs_primary_formats_array_no_yuv444[] = {
[ ... ]
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_RGBX8888,
>  	DRM_FORMAT_BGRX8888,
> -	DRM_FORMAT_ARGB8888,
> -	DRM_FORMAT_ABGR8888,
> -	DRM_FORMAT_RGBA8888,
> -	DRM_FORMAT_BGRA8888,
> -	DRM_FORMAT_ARGB2101010,
> -	DRM_FORMAT_ABGR2101010,
> -	DRM_FORMAT_RGBA1010102,
> -	DRM_FORMAT_BGRA1010102,

[Severity: Medium]
Does the removal of DRM_FORMAT_ARGB2101010 and its variants drop all 10-bit
color support from the primary plane?

Since the opaque 10-bit counterparts (like DRM_FORMAT_XRGB2101010) are
missing from vs_primary_formats_array_no_yuv444, any userspace compositor
attempting to use 10-bit color will find no supported formats and fail.

>  	/* TODO: non-RGB formats */
>  };
>  
> @@ -49,32 +33,16 @@ static const u32 vs_primary_formats_array_with_yuv444[] = {
[ ... ]
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_RGBX8888,
>  	DRM_FORMAT_BGRX8888,
> -	DRM_FORMAT_ARGB8888,
> -	DRM_FORMAT_ABGR8888,
> -	DRM_FORMAT_RGBA8888,
> -	DRM_FORMAT_BGRA8888,
> -	DRM_FORMAT_ARGB2101010,
> -	DRM_FORMAT_ABGR2101010,
> -	DRM_FORMAT_RGBA1010102,
> -	DRM_FORMAT_BGRA1010102,

[Severity: Medium]
Will this also remove 10-bit color support from the secondary array
vs_primary_formats_array_with_yuv444?

Just like the array above, the opaque 10-bit formats were not added,
removing 10-bit color support for userspace compositors on this plane.

>  	/* TODO: non-RGB formats */
>  };

[Severity: High]
This isn't a bug introduced by this patch, but can
vs_primary_plane_atomic_disable() crash when called on a previously disabled
plane?

drivers/gpu/drm/verisilicon/vs_primary_plane.c:vs_primary_plane_atomic_disable() {
	struct drm_plane_state *state = drm_atomic_get_old_plane_state(atomic_state,
								       plane);
	struct drm_crtc *crtc = state->crtc;
	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
	unsigned int output = vcrtc->id;
        ...

If a userspace process performs an atomic commit that targets a disabled plane
and keeps it disabled or moves it out of bounds (!state->visible), DRM helpers
may call atomic_update. The driver's atomic_update unconditionally calls
atomic_disable for invisible planes.

When atomic_disable retrieves old_plane_state->crtc, it is NULL since the
plane was already disabled. Would this result in a NULL pointer dereference
at vcrtc->id?

This exact same pattern exists in the cursor plane's disable path in
drivers/gpu/drm/verisilicon/vs_cursor_plane.c for
vs_cursor_plane_atomic_disable():

drivers/gpu/drm/verisilicon/vs_cursor_plane.c:vs_cursor_plane_atomic_disable() {
	struct drm_plane_state *state = drm_atomic_get_old_plane_state(atomic_state,
								       plane);
	struct drm_crtc *crtc = state->crtc;
	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
	unsigned int output = vcrtc->id;
        ...

[Severity: High]
This is a pre-existing issue, but does vs_primary_plane_atomic_disable()
mistakenly enable the plane in the hardware register instead of disabling it?

drivers/gpu/drm/verisilicon/vs_primary_plane.c:vs_primary_plane_atomic_disable() {
    ...
	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
			VSDC_FB_CONFIG_EX_FB_EN);
    ...
}

Whenever the primary plane is disabled (e.g., DPMS off, mode set, or
compositor exit), this code uses regmap_set_bits with VSDC_FB_CONFIG_EX_FB_EN.
This leaves the hardware plane active instead of disabling it.

When the DRM core subsequently frees the framebuffer memory, the display
controller hardware will continue to DMA read from the freed memory. Can this
cause a Use-After-Free and potential memory corruption or IOMMU faults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910095000.3505878-1-zhengxingda@iscas.ac.cn?part=2

Re: [PATCH 1/2] drm/verisilicon: add primary modifier for format tables

[Thomas Zimmermann]

Hi

Am 10.09.26 um 11:49 schrieb Icenowy Zheng:
> Currently the format tables are only used for the primary plane.
>
> Add primary modifiers to names related to the tables.
>
> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

for both patches.

> ---
>   drivers/gpu/drm/verisilicon/vs_hwdb.c          | 12 ++++++------
>   drivers/gpu/drm/verisilicon/vs_hwdb.h          |  4 ++--
>   drivers/gpu/drm/verisilicon/vs_primary_plane.c |  4 ++--
>   3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 2a0f7c59afa3a..56aa450443068 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -10,7 +10,7 @@
>   #include "vs_dc_top_regs.h"
>   #include "vs_hwdb.h"
>   
> -static const u32 vs_formats_array_no_yuv444[] = {
> +static const u32 vs_primary_formats_array_no_yuv444[] = {
>   	DRM_FORMAT_XRGB4444,
>   	DRM_FORMAT_XBGR4444,
>   	DRM_FORMAT_RGBX4444,
> @@ -44,7 +44,7 @@ static const u32 vs_formats_array_no_yuv444[] = {
>   	/* TODO: non-RGB formats */
>   };
>   
> -static const u32 vs_formats_array_with_yuv444[] = {
> +static const u32 vs_primary_formats_array_with_yuv444[] = {
>   	DRM_FORMAT_XRGB4444,
>   	DRM_FORMAT_XBGR4444,
>   	DRM_FORMAT_RGBX4444,
> @@ -79,13 +79,13 @@ static const u32 vs_formats_array_with_yuv444[] = {
>   };
>   
>   static const struct vs_formats vs_formats_no_yuv444 = {
> -	.array = vs_formats_array_no_yuv444,
> -	.num = ARRAY_SIZE(vs_formats_array_no_yuv444)
> +	.primary_array = vs_primary_formats_array_no_yuv444,
> +	.primary_num = ARRAY_SIZE(vs_primary_formats_array_no_yuv444)
>   };
>   
>   static const struct vs_formats vs_formats_with_yuv444 = {
> -	.array = vs_formats_array_with_yuv444,
> -	.num = ARRAY_SIZE(vs_formats_array_with_yuv444)
> +	.primary_array = vs_primary_formats_array_with_yuv444,
> +	.primary_num = ARRAY_SIZE(vs_primary_formats_array_with_yuv444)
>   };
>   
>   static struct vs_chip_identity vs_chip_identities[] = {
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> index 2065ecb730437..616076d931a57 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> @@ -10,8 +10,8 @@
>   #include <linux/types.h>
>   
>   struct vs_formats {
> -	const u32 *array;
> -	unsigned int num;
> +	const u32 *primary_array;
> +	unsigned int primary_num;
>   };
>   
>   struct vs_chip_identity {
> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 1f2be41ae496c..8e94491693019 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -168,8 +168,8 @@ struct drm_plane *vs_primary_plane_init(struct drm_device *drm_dev, struct vs_dc
>   
>   	plane = drmm_universal_plane_alloc(drm_dev, struct drm_plane, dev, 0,
>   					   &vs_primary_plane_funcs,
> -					   dc->identity.formats->array,
> -					   dc->identity.formats->num,
> +					   dc->identity.formats->primary_array,
> +					   dc->identity.formats->primary_num,
>   					   NULL,
>   					   DRM_PLANE_TYPE_PRIMARY,
>   					   NULL);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)