[PATCH drm-misc-next v4 0/4] drm: verisilicon: convert drm_format to vs_format in atomic_check

[PATCH drm-misc-next v4 0/4] drm: verisilicon: convert drm_format to vs_format in atomic_check

[Icenowy Zheng]

This patchset tries to move the process of converting drm_format to
vs_format into plane's atomic_check callback (instead of the
atomic_commit one, which the process currently sits) for more proper
error handling (because atomic_commit cannot fail).

In addition, because of the original flow of primary plane's
atomic_check seems to be flawed, it's also updated before filling
vs_format is added there.

Icenowy Zheng (4):
  drm: verisilicon: make vs_format conversion function return int
  drm: verisilicon: subclass drm_plane_state
  drm: verisilicon: call atomic helper's plane state check even if no
    CRTC
  drm: verisilicon: fill plane's vs_format in atomic_check

 drivers/gpu/drm/verisilicon/vs_plane.c        | 54 ++++++++++++++++++-
 drivers/gpu/drm/verisilicon/vs_plane.h        | 18 ++++++-
 .../gpu/drm/verisilicon/vs_primary_plane.c    | 48 ++++++++++-------
 3 files changed, 98 insertions(+), 22 deletions(-)

-- 
2.52.0

[PATCH drm-misc-next v4 1/4] drm: verisilicon: make vs_format conversion function return int

[Icenowy Zheng]

This is for further proper invalid drm_format handling before committing
the plane state change.

The return value is not yet checked yet, and will be checked in
atomic_check in the future.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
Changes in v4:
- Reposition the R-b tag (in v3 it's wrongly placed before SoB).
Changes in v3:
- Add Thomas's R-b.
Changes in v2:
- Move a bogus #include clause to the next patch.

 drivers/gpu/drm/verisilicon/vs_plane.c | 6 ++++--
 drivers/gpu/drm/verisilicon/vs_plane.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
index 2f3953e588a34..fa88ed14e41d7 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_plane.c
@@ -12,7 +12,7 @@
 
 #include "vs_plane.h"
 
-void drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
+int drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
 {
 	switch (drm_format) {
 	case DRM_FORMAT_XRGB4444:
@@ -62,7 +62,7 @@ void drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
 		vs_format->color = VSDC_COLOR_FORMAT_A2R10G10B10;
 		break;
 	default:
-		pr_warn("Unexpected drm format!\n");
+		return -EINVAL;
 	}
 
 	switch (drm_format) {
@@ -101,6 +101,8 @@ void drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
 
 	/* N/A for non-YUV formats */
 	vs_format->uv_swizzle = false;
+
+	return 0;
 }
 
 dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h b/drivers/gpu/drm/verisilicon/vs_plane.h
index 41875ea3d66a5..a88cc19f2202e 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.h
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -63,7 +63,7 @@ struct vs_format {
 	bool uv_swizzle;
 };
 
-void drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format);
+int drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format);
 dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
 			      const struct drm_rect *src_rect);
 
-- 
2.52.0

[PATCH drm-misc-next v4 2/4] drm: verisilicon: subclass drm_plane_state

[Icenowy Zheng]

Create a subclass of drm_plane_state to store hardware-specific state
information (e.g. hardware plane format settings) in the future.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
Changes in v4:
- Add code clearing plane->state pointer after freeing it.
- Add Thomas's R-b.

Changes in v3:
- Switch to drm_WARN_ON().
- Stop to memdup the state.
- Move the code freeing the existing plane state to the branch checking
  whether it's not NULL.
- Rename the typecast function to `to_vs_plane_state()`.

Changes in v2:
- Add the #include clause for atomic state helpers, which was wrongly
  placed in the previous patch in v1.
- Switch to kzalloc_obj helper for allocating the state.

 drivers/gpu/drm/verisilicon/vs_plane.c        | 43 +++++++++++++++++++
 drivers/gpu/drm/verisilicon/vs_plane.h        | 14 ++++++
 .../gpu/drm/verisilicon/vs_primary_plane.c    |  6 +--
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
index fa88ed14e41d7..7c6b905c9e1fa 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_plane.c
@@ -6,9 +6,11 @@
 #include <linux/errno.h>
 #include <linux/printk.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_fb_dma_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_print.h>
 
 #include "vs_plane.h"
 
@@ -124,3 +126,44 @@ dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
 
 	return dma_addr;
 }
+
+struct drm_plane_state *vs_plane_duplicate_state(struct drm_plane *plane)
+{
+	struct vs_plane_state *vs_state;
+
+	if (drm_WARN_ON(plane->dev, !plane->state))
+		return NULL;
+
+	vs_state = kzalloc_obj(*vs_state, GFP_KERNEL);
+	if (!vs_state)
+		return NULL;
+
+	__drm_atomic_helper_plane_duplicate_state(plane, &vs_state->base);
+
+	return &vs_state->base;
+}
+
+void vs_plane_destroy_state(struct drm_plane *plane,
+			    struct drm_plane_state *state)
+{
+	__drm_atomic_helper_plane_destroy_state(state);
+	kfree(state);
+}
+
+/* Called during init to allocate the plane's atomic state. */
+void vs_plane_reset(struct drm_plane *plane)
+{
+	struct vs_plane_state *vs_state;
+
+	if (plane->state) {
+		__drm_atomic_helper_plane_destroy_state(plane->state);
+		kfree(plane->state);
+		plane->state = NULL;
+	}
+
+	vs_state = kzalloc_obj(*vs_state, GFP_KERNEL);
+	if (!vs_state)
+		return;
+
+	__drm_atomic_helper_plane_reset(plane, &vs_state->base);
+}
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h b/drivers/gpu/drm/verisilicon/vs_plane.h
index a88cc19f2202e..48ed8fc754d18 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.h
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -63,10 +63,24 @@ struct vs_format {
 	bool uv_swizzle;
 };
 
+struct vs_plane_state {
+	struct drm_plane_state base;
+};
+
+static inline struct vs_plane_state *to_vs_plane_state(struct drm_plane_state *state)
+{
+	return container_of(state, struct vs_plane_state, base);
+}
+
 int drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format);
 dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
 			      const struct drm_rect *src_rect);
 
+struct drm_plane_state *vs_plane_duplicate_state(struct drm_plane *plane);
+void vs_plane_destroy_state(struct drm_plane *plane,
+			    struct drm_plane_state *state);
+void vs_plane_reset(struct drm_plane *plane);
+
 struct drm_plane *vs_primary_plane_init(struct drm_device *dev, struct vs_dc *dc);
 
 #endif /* _VS_PLANE_H_ */
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index e8fcb5958615c..bad0bc5e3242d 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -145,10 +145,10 @@ static const struct drm_plane_helper_funcs vs_primary_plane_helper_funcs = {
 };
 
 static const struct drm_plane_funcs vs_primary_plane_funcs = {
-	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
-	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state	= vs_plane_destroy_state,
+	.atomic_duplicate_state	= vs_plane_duplicate_state,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.reset			= drm_atomic_helper_plane_reset,
+	.reset			= vs_plane_reset,
 	.update_plane		= drm_atomic_helper_update_plane,
 };
 
-- 
2.52.0

[PATCH drm-misc-next v4 3/4] drm: verisilicon: call atomic helper's plane state check even if no CRTC

[Icenowy Zheng]

The `drm_atomic_helper_check_plane_state()` helper function needs to be
called even if the plane is bound to no CRTCs.

Remove the early return in the primary plane's atomic_check, and use
NULL for crtc_state in this situation.

Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for Verisilicon display controllers")
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
Changes in v4:
- Add Fixed tag.
- Add Thomas's R-b.

New patch in v3.

 drivers/gpu/drm/verisilicon/vs_primary_plane.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index bad0bc5e3242d..421d6f9dc547b 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -26,14 +26,10 @@ static int vs_primary_plane_atomic_check(struct drm_plane *plane,
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
 										 plane);
 	struct drm_crtc *crtc = new_plane_state->crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *crtc_state = NULL;
 
-	if (!crtc)
-		return 0;
-
-	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-	if (WARN_ON(!crtc_state))
-		return -EINVAL;
+	if (crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 	return drm_atomic_helper_check_plane_state(new_plane_state,
 						   crtc_state,
-- 
2.52.0

[PATCH drm-misc-next v4 4/4] drm: verisilicon: fill plane's vs_format in atomic_check

[Icenowy Zheng]

Move the conversion from drm_format to vs_format to atomic_check, which
is before the point of no return and can properly bail out.

Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
Changes in v4:
- Make atomic_check early return if the plane isn't visible.
- Added Thomas's R-b.

Changes in v3:
- Move the code filling vs_format to after drm_atomic_helper_check_plane_state()
  call.
- Add drm_WARN_ON_ONCE when filling vs_format fails.
- Add code copying vs_format in vs_plane_duplicate_state (because it no
  longer memdup's the whole struct vs_plane_state).

Changes in v2:
- Add non-NULL check for drm_plane_state->fb.

 drivers/gpu/drm/verisilicon/vs_plane.c        |  7 +++-
 drivers/gpu/drm/verisilicon/vs_plane.h        |  2 ++
 .../gpu/drm/verisilicon/vs_primary_plane.c    | 36 +++++++++++++------
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
index 7c6b905c9e1fa..d81f7b8f4c650 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_plane.c
@@ -129,17 +129,22 @@ dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
 
 struct drm_plane_state *vs_plane_duplicate_state(struct drm_plane *plane)
 {
-	struct vs_plane_state *vs_state;
+	struct vs_plane_state *vs_state, *vs_state_old;
 
 	if (drm_WARN_ON(plane->dev, !plane->state))
 		return NULL;
 
+	vs_state_old = to_vs_plane_state(plane->state);
+
 	vs_state = kzalloc_obj(*vs_state, GFP_KERNEL);
 	if (!vs_state)
 		return NULL;
 
 	__drm_atomic_helper_plane_duplicate_state(plane, &vs_state->base);
 
+	memcpy(&vs_state->format, &vs_state_old->format,
+	       sizeof(struct vs_format));
+
 	return &vs_state->base;
 }
 
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h b/drivers/gpu/drm/verisilicon/vs_plane.h
index 48ed8fc754d18..f18c36ef4cd90 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.h
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -65,6 +65,8 @@ struct vs_format {
 
 struct vs_plane_state {
 	struct drm_plane_state base;
+
+	struct vs_format format;
 };
 
 static inline struct vs_plane_state *to_vs_plane_state(struct drm_plane_state *state)
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 421d6f9dc547b..3576dd524f5c4 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -25,17 +25,32 @@ static int vs_primary_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
 										 plane);
+	struct vs_plane_state *new_vs_plane_state = to_vs_plane_state(new_plane_state);
+	struct drm_framebuffer *fb = new_plane_state->fb;
 	struct drm_crtc *crtc = new_plane_state->crtc;
 	struct drm_crtc_state *crtc_state = NULL;
+	int ret;
 
 	if (crtc)
 		crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
-	return drm_atomic_helper_check_plane_state(new_plane_state,
-						   crtc_state,
-						   DRM_PLANE_NO_SCALING,
-						   DRM_PLANE_NO_SCALING,
-						   false, true);
+	ret = drm_atomic_helper_check_plane_state(new_plane_state,
+						  crtc_state,
+						  DRM_PLANE_NO_SCALING,
+						  DRM_PLANE_NO_SCALING,
+						  false, true);
+	if (ret)
+		return ret;
+
+	if (!new_plane_state->visible)
+		return 0;
+
+	ret = drm_format_to_vs_format(fb->format->format,
+				      &new_vs_plane_state->format);
+	if (drm_WARN_ON_ONCE(plane->dev, ret))
+		return ret;
+
+	return 0;
 }
 
 static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int output)
@@ -84,11 +99,11 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
 {
 	struct drm_plane_state *state = drm_atomic_get_new_plane_state(atomic_state,
 								       plane);
+	struct vs_plane_state *vs_state = to_vs_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_crtc *crtc = state->crtc;
 	struct vs_dc *dc;
 	struct vs_crtc *vcrtc;
-	struct vs_format fmt;
 	unsigned int output;
 	dma_addr_t dma_addr;
 
@@ -101,16 +116,15 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
 	output = vcrtc->id;
 	dc = vcrtc->dc;
 
-	drm_format_to_vs_format(state->fb->format->format, &fmt);
-
 	regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
 			   VSDC_FB_CONFIG_FMT_MASK,
-			   VSDC_FB_CONFIG_FMT(fmt.color));
+			   VSDC_FB_CONFIG_FMT(vs_state->format.color));
 	regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
 			   VSDC_FB_CONFIG_SWIZZLE_MASK,
-			   VSDC_FB_CONFIG_SWIZZLE(fmt.swizzle));
+			   VSDC_FB_CONFIG_SWIZZLE(vs_state->format.swizzle));
 	regmap_assign_bits(dc->regs, VSDC_FB_CONFIG(output),
-			   VSDC_FB_CONFIG_UV_SWIZZLE_EN, fmt.uv_swizzle);
+			   VSDC_FB_CONFIG_UV_SWIZZLE_EN,
+			   vs_state->format.uv_swizzle);
 
 	dma_addr = vs_fb_get_dma_addr(fb, &state->src);
 
-- 
2.52.0

Re: [PATCH drm-misc-next v4 0/4] drm: verisilicon: convert drm_format to vs_format in atomic_check

[Thomas Zimmermann]

Hi,

I only looked briefly over the update, but it seems good now.

Best regards
Thomas


Am 31.03.26 um 08:01 schrieb Icenowy Zheng:
> This patchset tries to move the process of converting drm_format to
> vs_format into plane's atomic_check callback (instead of the
> atomic_commit one, which the process currently sits) for more proper
> error handling (because atomic_commit cannot fail).
>
> In addition, because of the original flow of primary plane's
> atomic_check seems to be flawed, it's also updated before filling
> vs_format is added there.
>
> Icenowy Zheng (4):
>    drm: verisilicon: make vs_format conversion function return int
>    drm: verisilicon: subclass drm_plane_state
>    drm: verisilicon: call atomic helper's plane state check even if no
>      CRTC
>    drm: verisilicon: fill plane's vs_format in atomic_check
>
>   drivers/gpu/drm/verisilicon/vs_plane.c        | 54 ++++++++++++++++++-
>   drivers/gpu/drm/verisilicon/vs_plane.h        | 18 ++++++-
>   .../gpu/drm/verisilicon/vs_primary_plane.c    | 48 ++++++++++-------
>   3 files changed, 98 insertions(+), 22 deletions(-)
>

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

Re: [PATCH drm-misc-next v4 0/4] drm: verisilicon: convert drm_format to vs_format in atomic_check

[Icenowy Zheng]

在 2026-03-31二的 08:27 +0200，Thomas Zimmermann写道：
> Hi,
> 
> I only looked briefly over the update, but it seems good now.

Could you have a more thorough look on this patchset (and if possible,
apply it)?

Thanks
Icenowy

> 
> Best regards
> Thomas
> 
> 
> Am 31.03.26 um 08:01 schrieb Icenowy Zheng:
> > This patchset tries to move the process of converting drm_format to
> > vs_format into plane's atomic_check callback (instead of the
> > atomic_commit one, which the process currently sits) for more
> > proper
> > error handling (because atomic_commit cannot fail).
> > 
> > In addition, because of the original flow of primary plane's
> > atomic_check seems to be flawed, it's also updated before filling
> > vs_format is added there.
> > 
> > Icenowy Zheng (4):
> >    drm: verisilicon: make vs_format conversion function return int
> >    drm: verisilicon: subclass drm_plane_state
> >    drm: verisilicon: call atomic helper's plane state check even if
> > no
> >      CRTC
> >    drm: verisilicon: fill plane's vs_format in atomic_check
> > 
> >   drivers/gpu/drm/verisilicon/vs_plane.c        | 54
> > ++++++++++++++++++-
> >   drivers/gpu/drm/verisilicon/vs_plane.h        | 18 ++++++-
> >   .../gpu/drm/verisilicon/vs_primary_plane.c    | 48 ++++++++++----
> > ---
> >   3 files changed, 98 insertions(+), 22 deletions(-)
> > 

Re: [PATCH drm-misc-next v4 0/4] drm: verisilicon: convert drm_format to vs_format in atomic_check

[Thomas Zimmermann]

Hi

Am 02.05.26 um 15:47 schrieb Icenowy Zheng:
> 在 2026-03-31二的 08:27 +0200，Thomas Zimmermann写道：
>> Hi,
>>
>> I only looked briefly over the update, but it seems good now.
> Could you have a more thorough look on this patchset (and if possible,
> apply it)?

Merged now. I thought this got in already.

Best regards
Thomas

>
> Thanks
> Icenowy
>
>> Best regards
>> Thomas
>>
>>
>> Am 31.03.26 um 08:01 schrieb Icenowy Zheng:
>>> This patchset tries to move the process of converting drm_format to
>>> vs_format into plane's atomic_check callback (instead of the
>>> atomic_commit one, which the process currently sits) for more
>>> proper
>>> error handling (because atomic_commit cannot fail).
>>>
>>> In addition, because of the original flow of primary plane's
>>> atomic_check seems to be flawed, it's also updated before filling
>>> vs_format is added there.
>>>
>>> Icenowy Zheng (4):
>>>     drm: verisilicon: make vs_format conversion function return int
>>>     drm: verisilicon: subclass drm_plane_state
>>>     drm: verisilicon: call atomic helper's plane state check even if
>>> no
>>>       CRTC
>>>     drm: verisilicon: fill plane's vs_format in atomic_check
>>>
>>>    drivers/gpu/drm/verisilicon/vs_plane.c        | 54
>>> ++++++++++++++++++-
>>>    drivers/gpu/drm/verisilicon/vs_plane.h        | 18 ++++++-
>>>    .../gpu/drm/verisilicon/vs_primary_plane.c    | 48 ++++++++++----
>>> ---
>>>    3 files changed, 98 insertions(+), 22 deletions(-)
>>>

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