Re: [PATCH -next v2 2/2] drm: verisilicon: add support for cursor planes
From: Thomas Zimmermann
Date: Wed May 06 2026 - 02:49:05 EST
Hi
Am 05.05.26 um 10:25 schrieb Icenowy Zheng:
[...]
The thing being checked is the driver's HWDB (instead of user input),+ /*Rather make this drm_dbg(). User space might just run an
+ * Only certain PoT square sizes is supported, the minimum
supported
+ * size is 32x32 and the maximum known size is 256x256.
+ */
+ drm_WARN_ON_ONCE(plane->dev, dc->identity.max_cursor_size
< 32 ||
+ dc->identity.max_cursor_size
256);
atomic_check
on an atomic_state to test if it is supported. That should not leave
warnings in the logs.
if this check fails the driver is surely buggy and misoperating.
Got it. It's a hardware limit.
Or maybe this check should be moved to the creating code of cursor
planes?
Yeah, makes sense. Tested it once when you create the cursor plane.
Best regards
Thomas
Yes, I checked this function and it will set visible to false (thus+Not needed. The case of !fb would have been detected by
+ if (!is_power_of_2(new_plane_state->crtc_w) ||
+ new_plane_state->crtc_w < 32 ||
+ new_plane_state->crtc_w > dc-
identity.max_cursor_size)+ return -EINVAL;
+
+ if (new_plane_state->crtc_w != new_plane_state->crtc_h)
+ return -EINVAL;
+
+ /* Check if the cursor is inside the register fields'
range */
+ if (!vs_cursor_plane_check_coord(new_plane_state->crtc_x)
||
+ !vs_cursor_plane_check_coord(new_plane_state->crtc_y))
+ return -EINVAL;
+
+ if (fb) {
drm_atomic_helper_check_plane_state(). Best regards Thomas
here early return is triggered) when no FB is bound.
Thanks,
Icenowy
+ /* Extra line padding isn't supported */
+ if (fb->pitches[0] !=
+ drm_format_info_min_pitch(fb->format, 0,
+ new_plane_state-
crtc_w))+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void vs_cursor_plane_commit(struct vs_dc *dc, unsigned int
output)
+{
+ regmap_set_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_COMMIT |
+ VSDC_CURSOR_CONFIG_IMG_UPDATE);
+}
+
+static void vs_cursor_plane_atomic_enable(struct drm_plane *plane,
+ struct drm_atomic_state
*atomic_state)
+{
+ struct drm_plane_state *state =
drm_atomic_get_new_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;
+ struct vs_dc *dc = vcrtc->dc;
+
+ regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_FMT_MASK,
+ VSDC_CURSOR_CONFIG_FMT_ARGB8888);
+
+ vs_cursor_plane_commit(dc, output);
+}
+
+static void vs_cursor_plane_atomic_disable(struct drm_plane
*plane,
+ struct
drm_atomic_state *atomic_state)
+{
+ 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;
+ struct vs_dc *dc = vcrtc->dc;
+
+ regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_FMT_MASK,
+ VSDC_CURSOR_CONFIG_FMT_OFF);
+
+ vs_cursor_plane_commit(dc, output);
+}
+
+static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state
*atomic_state)
+{
+ struct drm_plane_state *state =
drm_atomic_get_new_plane_state(atomic_state,
+
plane);
+ struct drm_framebuffer *fb = state->fb;
+ struct drm_crtc *crtc = state->crtc;
+ struct vs_dc *dc;
+ struct vs_crtc *vcrtc;
+ unsigned int output;
+ dma_addr_t dma_addr;
+
+ if (!state->visible) {
+ vs_cursor_plane_atomic_disable(plane,
atomic_state);
+ return;
+ }
+
+ vcrtc = drm_crtc_to_vs_crtc(crtc);
+ output = vcrtc->id;
+ dc = vcrtc->dc;
+
+ /* Other sizes should be rejected by atomic_check */
+ switch (state->crtc_w) {
+ case 32:
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_SIZE_MASK,
+ VSDC_CURSOR_CONFIG_SIZE_32);
+ break;
+ case 64:
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_SIZE_MASK,
+ VSDC_CURSOR_CONFIG_SIZE_64);
+ break;
+ case 128:
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_SIZE_MASK,
+ VSDC_CURSOR_CONFIG_SIZE_128);
+ break;
+ case 256:
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_SIZE_MASK,
+ VSDC_CURSOR_CONFIG_SIZE_256);
+ break;
+ }
+
+ dma_addr = vs_fb_get_dma_addr(fb, &state->src);
+
+ regmap_write(dc->regs, VSDC_CURSOR_ADDRESS(output),
+ lower_32_bits(dma_addr));
+
+ /*
+ * The X_OFF and Y_OFF fields define which point does the
LOCATION
+ * register represent in the cursor image, and LOCATION
register
+ * values are unsigned. To for positive left-top
coordinates the
+ * offset is set to 0 and the location is set to the
coordinate, for
+ * negative coordinates the location is set to 0 and the
offset
+ * is set to the opposite number of the coordinate to
offset the
+ * cursor image partly off-screen.
+ */
+ if (state->crtc_x >= 0) {
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_X_OFF_MASK,
0);
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_LOCATION(output),
+ VSDC_CURSOR_LOCATION_X_MASK,
+ VSDC_CURSOR_LOCATION_X(state-
crtc_x));+ } else {
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_X_OFF_MASK,
+ -state->crtc_x);
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_LOCATION(output),
+ VSDC_CURSOR_LOCATION_X_MASK,
0);
+ }
+
+ if (state->crtc_y >= 0) {
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_Y_OFF_MASK,
0);
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_LOCATION(output),
+ VSDC_CURSOR_LOCATION_Y_MASK,
+ VSDC_CURSOR_LOCATION_Y(state-
crtc_y));+ } else {
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_CONFIG(output),
+ VSDC_CURSOR_CONFIG_Y_OFF_MASK,
+ -state->crtc_y);
+ regmap_update_bits(dc->regs,
VSDC_CURSOR_LOCATION(output),
+ VSDC_CURSOR_LOCATION_Y_MASK,
0);
+ }
+
+ vs_cursor_plane_commit(dc, output);
+}
+
+static const struct drm_plane_helper_funcs
vs_cursor_plane_helper_funcs = {
+ .atomic_check = vs_cursor_plane_atomic_check,
+ .atomic_update = vs_cursor_plane_atomic_update,
+ .atomic_enable = vs_cursor_plane_atomic_enable,
+ .atomic_disable = vs_cursor_plane_atomic_disable,
+};
+
+static const struct drm_plane_funcs vs_cursor_plane_funcs = {
+ .atomic_destroy_state =
drm_atomic_helper_plane_destroy_state,
+ .atomic_duplicate_state =
drm_atomic_helper_plane_duplicate_state,
+ .disable_plane = drm_atomic_helper_disable_plane,
+ .reset = drm_atomic_helper_plane_reset,
+ .update_plane = drm_atomic_helper_update_plane,
+};
+
+static const u32 vs_cursor_plane_formats[] = {
+ DRM_FORMAT_ARGB8888,
+};
+
+static const u64 vs_cursor_plane_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID, /* sentinel */
+};
+
+struct drm_plane *vs_cursor_plane_init(struct drm_device *drm_dev,
+ struct vs_dc *dc)
+{
+ struct drm_plane *plane;
+
+ plane = drmm_universal_plane_alloc(drm_dev, struct
drm_plane, dev, 0,
+ &vs_cursor_plane_funcs,
+
vs_cursor_plane_formats,
+
ARRAY_SIZE(vs_cursor_plane_formats),
+
vs_cursor_plane_modifiers,
+ DRM_PLANE_TYPE_CURSOR,
+ NULL);
+
+ if (IS_ERR(plane))
+ return plane;
+
+ drm_plane_helper_add(plane,
&vs_cursor_plane_helper_funcs);
+
+ return plane;
+}
diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane_regs.h
b/drivers/gpu/drm/verisilicon/vs_cursor_plane_regs.h
new file mode 100644
index 0000000000000..99693f2c95b94
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane_regs.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2025 Icenowy Zheng <uwu@xxxxxxxxxx>
+ *
+ * Based on vs_dc_hw.h, which is:
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#ifndef _VS_CURSOR_PLANE_REGS_H_
+#define _VS_CURSOR_PLANE_REGS_H_
+
+#include <linux/bits.h>
+
+#define VSDC_CURSOR_CONFIG(n) (0x1468 + 0x1080 * (n))
+#define VSDC_CURSOR_CONFIG_FMT_MASK GENMASK(1, 0)
+#define VSDC_CURSOR_CONFIG_FMT_ARGB8888 (0x2 << 0)
+#define VSDC_CURSOR_CONFIG_FMT_OFF (0x0 << 0)
+#define VSDC_CURSOR_CONFIG_IMG_UPDATE BIT(2)
+#define VSDC_CURSOR_CONFIG_COMMIT BIT(3)
+#define VSDC_CURSOR_CONFIG_SIZE_MASK GENMASK(7, 5)
+#define VSDC_CURSOR_CONFIG_SIZE_32 (0x0 << 5)
+#define VSDC_CURSOR_CONFIG_SIZE_64 (0x1 << 5)
+#define VSDC_CURSOR_CONFIG_SIZE_128 (0x2 << 5)
+#define VSDC_CURSOR_CONFIG_SIZE_256 (0x3 << 5)
+#define VSDC_CURSOR_CONFIG_Y_OFF_MASK GENMASK(12, 8)
+#define VSDC_CURSOR_CONFIG_Y_OFF(v) ((v) << 8)
+#define VSDC_CURSOR_CONFIG_X_OFF_MASK GENMASK(20, 16)
+#define VSDC_CURSOR_CONFIG_X_OFF(v) ((v) << 16)
+
+#define VSDC_CURSOR_ADDRESS(n) (0x146C + 0x1080 * (n))
+
+#define VSDC_CURSOR_LOCATION(n) (0x1470 + 0x1080 *
(n))
+#define VSDC_CURSOR_LOCATION_X_MASK GENMASK(14, 0)
+#define VSDC_CURSOR_LOCATION_X(v) ((v) << 0)
+#define VSDC_CURSOR_LOCATION_Y_MASK GENMASK(30, 16)
+#define VSDC_CURSOR_LOCATION_Y(v) ((v) << 16)
+
+#define VSDC_CURSOR_BACKGROUND(n) (0x1474 + 0x1080 * (n))
+#define VSDC_CURSOR_BACKGRUOND_DEFAULT 0x00FFFFFF
+
+#define VSDC_CURSOR_FOREGROUND(n) (0x1478 + 0x1080 * (n))
+#define VSDC_CURSOR_FOREGRUOND_DEFAULT 0x00AAAAAA
+
+#endif /* _VS_CURSOR_PLANE_REGS_H_ */
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h
b/drivers/gpu/drm/verisilicon/vs_plane.h
index 41875ea3d66a5..60b5b3a1bc22a 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.h
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -68,5 +68,6 @@ dma_addr_t vs_fb_get_dma_addr(struct
drm_framebuffer *fb,
const struct drm_rect *src_rect);
struct drm_plane *vs_primary_plane_init(struct drm_device *dev,
struct vs_dc *dc);
+struct drm_plane *vs_cursor_plane_init(struct drm_device *dev,
struct vs_dc *dc);
#endif /* _VS_PLANE_H_ */
--
--
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)