[PATCH 1/2] drm/verisilicon: set blend mode for the primary plane

[PATCH 1/2] drm/verisilicon: set blend mode for the primary plane

[Icenowy Zheng]

Blend modes are now required to expose pixel formats w/ alpha.

As it's the primary plane and blending is explicitly disabled, just
expose PIXEL_NONE blend mode.

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

diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 1f2be41ae496c..8d58682d88ef8 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -7,6 +7,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -179,5 +180,7 @@ struct drm_plane *vs_primary_plane_init(struct drm_device *drm_dev, struct vs_dc
 
 	drm_plane_helper_add(plane, &vs_primary_plane_helper_funcs);
 
+	drm_plane_create_blend_mode_property(plane,
+					     BIT(DRM_MODE_BLEND_PIXEL_NONE));
 	return plane;
 }
-- 
2.52.0

[PATCH 2/2] drm/verisilicon: set blend mode for the cursor plane

[Icenowy Zheng]

Blend mode properties are now required to expose pixel formats w/ alpha.

Experiments show that the fixed blending mode for the cursor seems to be
COVERAGE:

- With a cursor plane filled with R=G=0, B=0xff, A=0x40, the cursor is
  visible on a pure-white background, which means the background is
  multiplied.
- With a cursor plane filled with R=G=B=0xff, A=0x40, the cursor isn't
  pure white and non-white patterns can be see through, which means the
  cursor is multiplied.

Add a fixed COVERAGE blend mode property for the cursor plane.

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

diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
index fa4f601dd0c87..36e3664373fa1 100644
--- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
@@ -11,6 +11,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -268,5 +269,7 @@ struct drm_plane *vs_cursor_plane_init(struct drm_device *drm_dev,
 
 	drm_plane_helper_add(plane, &vs_cursor_plane_helper_funcs);
 
+	drm_plane_create_blend_mode_property(plane,
+					     BIT(DRM_MODE_BLEND_COVERAGE));
 	return plane;
 }
-- 
2.52.0

Re: [PATCH 1/2] drm/verisilicon: set blend mode for the primary plane

[sashiko-bot]

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

New issues:
- [Medium] The return value of `drm_plane_create_blend_mode_property` is ignored.

Pre-existing issues:
- [Critical] Dereferencing a NULL `crtc` pointer in `vs_primary_plane_atomic_disable` during offscreen plane transitions.
- [High] `vs_primary_plane_atomic_disable` incorrectly sets the enable bit instead of clearing it.
--

commit 9aa67f5c63b091d89b8b4a7a3066a896f35cf108
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/verisilicon: set blend mode for the primary plane

Blend modes are now required to expose pixel formats w/ alpha.
    
As it's the primary plane and blending is explicitly disabled, just
expose PIXEL_NONE blend mode.

> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 1f2be41ae496c..8d58682d88ef8 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -7,6 +7,7 @@
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_blend.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_framebuffer.h>

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but can this dereference a NULL pointer during
offscreen plane transitions?

If userspace commits an atomic state where a plane transitions from fully
disabled (old_state->crtc == NULL) to assigned but offscreen
(new_state->crtc != NULL but new_state->visible == false), the DRM core
calls vs_primary_plane_atomic_update().

This function sees !state->visible and delegates to the disable function:

vs_primary_plane_atomic_update() {
    ...
	if (!state->visible) {
		vs_primary_plane_atomic_disable(plane, atomic_state);
		return;
	}
    ...
}

The disable function then fetches old_state->crtc, which is NULL, and
dereferences it:

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;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does
vs_primary_plane_atomic_disable() incorrectly set the enable bit instead of
clearing it?

vs_primary_plane_atomic_disable() {
    ...
	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
			VSDC_FB_CONFIG_EX_FB_EN);

	vs_primary_plane_commit(dc, output);
}

When the plane needs to be disabled, the core calls this function, which uses
regmap_set_bits() with the VSDC_FB_CONFIG_EX_FB_EN bit.

Should this use regmap_clear_bits() instead? The hardware plane is never
disabled, which can cause visual corruption, unexpected display state, and
potential memory access faults if the buffer memory is subsequently freed
while the hardware is still scanning it out.

> @@ -179,5 +180,7 @@ struct drm_plane *vs_primary_plane_init(struct drm_device *drm_dev, struct vs_dc
>  
>  	drm_plane_helper_add(plane, &vs_primary_plane_helper_funcs);
>  
> +	drm_plane_create_blend_mode_property(plane,
> +					     BIT(DRM_MODE_BLEND_PIXEL_NONE));

[Severity: Medium]
Is it intentional to ignore the return value of
drm_plane_create_blend_mode_property()?

If property creation fails (e.g. due to memory exhaustion), the plane is
initialized without the requested blend mode property, breaking userspace
expectations for alpha formats. Should this function check for an error code
and handle the failure?

>  	return plane;
>  }

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

Re: [PATCH 2/2] drm/verisilicon: set blend mode for the cursor plane

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The cursor plane's blend mode property is created supporting only COVERAGE, but the default atomic state initialization sets it to PREMULTI, causing atomic commits to fail.
--

commit 8e0ec23e99807ff97fec9437ae15ac650e658045
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/verisilicon: set blend mode for the cursor plane

This commit adds a fixed COVERAGE blend mode property for the cursor plane,
as blend mode properties are now required to expose pixel formats with alpha.

> diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> index fa4f601dd0c87..36e3664373fa1 100644
> --- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
[ ... ]
> @@ -268,5 +269,7 @@ struct drm_plane *vs_cursor_plane_init(struct drm_device *drm_dev,
>  
>  	drm_plane_helper_add(plane, &vs_cursor_plane_helper_funcs);
>  
> +	drm_plane_create_blend_mode_property(plane,
> +					     BIT(DRM_MODE_BLEND_COVERAGE));

[Severity: High]
Will this cause atomic commits to fail during state restoration?

Looking at the plane setup, vs_cursor_plane_funcs uses the default
drm_atomic_helper_plane_reset() hook, which unconditionally initializes the
plane state's pixel_blend_mode to DRM_MODE_BLEND_PREMULTI.

Because this property is created supporting only DRM_MODE_BLEND_COVERAGE,
there is a mismatch with the initial state. When userspace queries the
initial state, it reads PREMULTI. If it attempts to write this state back
during an atomic commit (for instance, during a VT switch in a compositor),
the commit will fail with -EINVAL since PREMULTI is not in the supported
blend mode enum list.

>  	return plane;
>  }

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