* [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support
@ 2026-05-19 5:51 Joey Lu
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Joey Lu @ 2026-05-19 5:51 UTC (permalink / raw)
To: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
This series adds support for the Verisilicon DCU Lite display controller
as integrated in the Nuvoton MA35D1 SoC.
The Verisilicon DC driver and its DT binding were originally written by
Icenowy Zheng <zhengxingda@iscas.ac.cn> for the T-Head TH1520 SoC, which
carries a DC8200 IP block. The present series builds on that foundation
with gratitude to Icenowy for the original work.
The DCU Lite is a different variant in the DC IP family. While the two
IPs share a broadly similar register layout, a number of differences
prevent the existing driver from working on the MA35D1 without
modification:
- No CONFIG_EX commit path: the DC8200 staging registers
(FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT, FB_BLEND_CONFIG,
PANEL_CONFIG_EX) are absent. The DCU Lite uses enable (bit 0) and
reset (bit 4) bits in FB_CONFIG for direct framebuffer updates, and
requires a per-frame VALID bit toggle (FB_CONFIG bit 3) to latch
configuration changes.
- No PANEL_START register: panel output begins when
PANEL_CONFIG.RUNNING is set; the DC8200 multi-display sync start
register at 0x1CCC does not exist.
- Different IRQ registers: DISP_IRQ_STA at 0x147C / DISP_IRQ_EN at
0x1480, versus the DC8200's TOP_IRQ_ACK at 0x0010 / TOP_IRQ_EN at
0x0014.
- Simpler clock topology: two clocks ("core" bus gate and "pix0" pixel
divider); no axi or ahb clocks required.
- Single display output: no per-output indexing beyond index 0 is
needed.
- Hardware-discoverable identity: the DCU Lite exposes chip identity
registers whose model field reads 0x0 (revision 0x5560,
customer_id 0x305), allowing the existing vs_fill_chip_identity()
path to identify the variant purely through register reads. No
separate OF compatible string is introduced.
Patch 1 generalises the verisilicon,dc DT binding to accommodate
variants with flexible clock/reset counts and a single output, using
allOf/if-then-else to keep per-variant constraints in-schema.
Patches 2-4 introduce the driver changes in three logical steps:
register-level constants and the DCU Lite chip identity table entry;
the vs_dc_funcs hardware ops table with DC8200 ops extracted into
vs_dc8200.c; and finally the DCU Lite ops in vs_dcu_lite.c with the
necessary Kconfig and clock-optionality changes.
All patches have been tested on Nuvoton MA35D1 hardware.
Changes from v1:
- Corrected "DC8000" to "DC8200" throughout (the existing supported
IP is DC8200, not DC8000).
- Dropped the separate nuvoton,ma35d1-dcu.yaml; variant constraints
are now expressed inline in verisilicon,dc.yaml via allOf/if-then-else.
The MA35D1 uses the generic "verisilicon,dc" compatible string.
- Replaced the vs_dc_info platform-data flags approach with a
vs_dc_funcs hardware ops table, giving cleaner per-variant dispatch
without scattering if/else branches across multiple files.
- DCU Lite variant is identified through hardware registers rather than
the OF match table.
- Series split from 2 patches to 4 for clearer logical progression.
- Renamed plane ops in vs_dc_funcs: plane_enable/disable to
plane_enable_ex/disable_ex, plane_update_ext to plane_update_ex.
Joey Lu (4):
dt-bindings: display: verisilicon,dc: generalize for single-output
variants
drm/verisilicon: add model ID constants and DCU Lite chip identity
drm/verisilicon: introduce per-variant hardware ops table
drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller
support
.../bindings/display/verisilicon,dc.yaml | 135 ++++++++++++++----
drivers/gpu/drm/verisilicon/Kconfig | 2 +-
drivers/gpu/drm/verisilicon/Makefile | 2 +-
drivers/gpu/drm/verisilicon/vs_bridge.c | 20 +--
drivers/gpu/drm/verisilicon/vs_crtc.c | 38 ++++-
drivers/gpu/drm/verisilicon/vs_crtc_regs.h | 1 +
drivers/gpu/drm/verisilicon/vs_dc.c | 13 +-
drivers/gpu/drm/verisilicon/vs_dc.h | 33 +++++
drivers/gpu/drm/verisilicon/vs_dc8200.c | 107 ++++++++++++++
drivers/gpu/drm/verisilicon/vs_dcu_lite.c | 78 ++++++++++
drivers/gpu/drm/verisilicon/vs_hwdb.c | 16 ++-
drivers/gpu/drm/verisilicon/vs_hwdb.h | 3 +
.../gpu/drm/verisilicon/vs_primary_plane.c | 32 +----
.../drm/verisilicon/vs_primary_plane_regs.h | 3 +
14 files changed, 398 insertions(+), 85 deletions(-)
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_dcu_lite.c
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants
2026-05-19 5:51 [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
@ 2026-05-19 5:51 ` Joey Lu
2026-05-19 6:15 ` sashiko-bot
2026-05-19 7:26 ` [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: " Icenowy Zheng
2026-05-19 5:51 ` [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity Joey Lu
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Joey Lu @ 2026-05-19 5:51 UTC (permalink / raw)
To: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
The existing schema assumes a fixed clock/reset topology and dual-output
port structure matching the DC8200 IP block. This prevents reuse for
single-output variants such as the Verisilicon DCU Lite used in the
Nuvoton MA35D1 SoC.
Rework the schema so that variant-specific constraints are expressed
via allOf/if-then-else:
- The thead,th1520-dc8200 compatible keeps its existing five-clock,
three-reset, dual-port requirements.
- A standalone verisilicon,dc compatible covers IPs whose identity is
discovered entirely through hardware registers; these have flexible
clock and reset counts, a single 'port' property, and no 'ports'
requirement.
Changes to the base schema:
- Replace the fixed clock/reset items lists with minItems/maxItems
ranges; variant sub-schemas tighten the constraints via if-then-else.
- Add a 'port' property (graph.yaml single-port alias) alongside the
existing 'ports', for single-output variants.
- Drop the unconditional 'ports' requirement; each if-branch enforces
its own port topology.
- Tighten additionalProperties to unevaluatedProperties to allow
per-variant schemas to add their own constraints cleanly.
- Fix a stray space in the port@0 description.
- Add a DT example for the generic verisilicon,dc compatible
(Nuvoton MA35D1 DCU Lite).
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
.../bindings/display/verisilicon,dc.yaml | 135 ++++++++++++++----
1 file changed, 108 insertions(+), 27 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
index 9dc35ab973f2..3a814c2e083e 100644
--- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
+++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
@@ -14,10 +14,12 @@ properties:
pattern: "^display@[0-9a-f]+$"
compatible:
- items:
- - enum:
- - thead,th1520-dc8200
- - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
+ oneOf:
+ - items:
+ - enum:
+ - thead,th1520-dc8200
+ - const: verisilicon,dc
+ - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
reg:
maxItems: 1
@@ -26,32 +28,24 @@ properties:
maxItems: 1
clocks:
- items:
- - description: DC Core clock
- - description: DMA AXI bus clock
- - description: Configuration AHB bus clock
- - description: Pixel clock of output 0
- - description: Pixel clock of output 1
+ minItems: 2
+ maxItems: 5
clock-names:
- items:
- - const: core
- - const: axi
- - const: ahb
- - const: pix0
- - const: pix1
+ minItems: 2
+ maxItems: 5
resets:
- items:
- - description: DC Core reset
- - description: DMA AXI bus reset
- - description: Configuration AHB bus reset
+ minItems: 1
+ maxItems: 3
reset-names:
- items:
- - const: core
- - const: axi
- - const: ahb
+ minItems: 1
+ maxItems: 3
+
+ port:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Single video output port for single-output variants.
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -59,7 +53,7 @@ properties:
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
- description: The first output channel , endpoint 0 should be
+ description: The first output channel, endpoint 0 should be
used for DPI format output and endpoint 1 should be used
for DP format output.
@@ -75,9 +69,75 @@ required:
- interrupts
- clocks
- clock-names
- - ports
-additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: thead,th1520-dc8200
+ then:
+ properties:
+ clocks:
+ items:
+ - description: DC Core clock
+ - description: DMA AXI bus clock
+ - description: Configuration AHB bus clock
+ - description: Pixel clock of output 0
+ - description: Pixel clock of output 1
+
+ clock-names:
+ items:
+ - const: core
+ - const: axi
+ - const: ahb
+ - const: pix0
+ - const: pix1
+
+ resets:
+ items:
+ - description: DC Core reset
+ - description: DMA AXI bus reset
+ - description: Configuration AHB bus reset
+
+ reset-names:
+ items:
+ - const: core
+ - const: axi
+ - const: ahb
+
+ required:
+ - ports
+
+ else:
+ properties:
+ clocks:
+ items:
+ - description: Bus clock that gates register access
+ - description: Pixel clock divider for display timing
+
+ clock-names:
+ items:
+ - const: core
+ - const: pix0
+
+ resets:
+ maxItems: 1
+ description:
+ Reset line for the display controller.
+
+ reset-names:
+ items:
+ - const: core
+
+ required:
+ - port
+
+ not:
+ required:
+ - ports
+
+unevaluatedProperties: false
examples:
- |
@@ -120,3 +180,24 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+ #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
+
+ display@40260000 {
+ compatible = "verisilicon,dc";
+ reg = <0x40260000 0x20000>;
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk DCU_GATE>, <&clk DCUP_DIV>;
+ clock-names = "core", "pix0";
+ resets = <&sys MA35D1_RESET_DISP>;
+ reset-names = "core";
+
+ port {
+ dpi_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity
2026-05-19 5:51 [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
@ 2026-05-19 5:51 ` Joey Lu
2026-05-19 6:25 ` sashiko-bot
2026-05-19 7:37 ` Icenowy Zheng
2026-05-19 5:51 ` [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table Joey Lu
2026-05-19 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
3 siblings, 2 replies; 13+ messages in thread
From: Joey Lu @ 2026-05-19 5:51 UTC (permalink / raw)
To: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
Introduce symbolic constants VSDC_MODEL_DC8200 and VSDC_MODEL_DCU_LITE
to replace magic numbers in the hardware database and probe path.
Register the DCU Lite chip identity (model 0x0, revision 0x5560,
customer_id 0x305) in vs_chip_identities[], making the existing
vs_fill_chip_identity() path able to recognise Nuvoton MA35D1 hardware
purely through register reads.
Also add three register-level macros for forthcoming DCU Lite support:
- VSDC_DISP_IRQ_VSYNC(n) in vs_crtc_regs.h, for per-output VSYNC IRQ
bits used by the DCU Lite IRQ enable/status registers.
- VSDC_FB_CONFIG_ENABLE, VSDC_FB_CONFIG_VALID and VSDC_FB_CONFIG_RESET
in vs_primary_plane_regs.h, for the framebuffer enable and
commit-cycle bits used by the DCU Lite plane update path.
No behaviour change for existing DC8200 platforms.
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/gpu/drm/verisilicon/vs_crtc_regs.h | 1 +
drivers/gpu/drm/verisilicon/vs_hwdb.c | 16 ++++++++++++----
drivers/gpu/drm/verisilicon/vs_hwdb.h | 3 +++
.../gpu/drm/verisilicon/vs_primary_plane_regs.h | 3 +++
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
index c7930e817635..d4da22b08cd5 100644
--- a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
+++ b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
@@ -54,6 +54,7 @@
#define VSDC_DISP_GAMMA_DATA(n) (0x1460 + 0x4 * (n))
#define VSDC_DISP_IRQ_STA 0x147C
+#define VSDC_DISP_IRQ_VSYNC(n) BIT(n)
#define VSDC_DISP_IRQ_EN 0x1480
diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
index 09336af0900a..a25c4b16181d 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
@@ -90,7 +90,7 @@ static const struct vs_formats vs_formats_with_yuv444 = {
static struct vs_chip_identity vs_chip_identities[] = {
{
- .model = 0x8200,
+ .model = VSDC_MODEL_DC8200,
.revision = 0x5720,
.customer_id = ~0U,
@@ -98,7 +98,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
.formats = &vs_formats_no_yuv444,
},
{
- .model = 0x8200,
+ .model = VSDC_MODEL_DC8200,
.revision = 0x5721,
.customer_id = 0x30B,
@@ -106,7 +106,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
.formats = &vs_formats_no_yuv444,
},
{
- .model = 0x8200,
+ .model = VSDC_MODEL_DC8200,
.revision = 0x5720,
.customer_id = 0x310,
@@ -114,13 +114,21 @@ static struct vs_chip_identity vs_chip_identities[] = {
.formats = &vs_formats_with_yuv444,
},
{
- .model = 0x8200,
+ .model = VSDC_MODEL_DC8200,
.revision = 0x5720,
.customer_id = 0x311,
.display_count = 2,
.formats = &vs_formats_no_yuv444,
},
+ {
+ .model = VSDC_MODEL_DCU_LITE,
+ .revision = 0x5560,
+ .customer_id = 0x305,
+
+ .display_count = 1,
+ .formats = &vs_formats_no_yuv444,
+ },
};
int vs_fill_chip_identity(struct regmap *regs,
diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h b/drivers/gpu/drm/verisilicon/vs_hwdb.h
index 92192e4fa086..cca126bd2da5 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
@@ -9,6 +9,9 @@
#include <linux/regmap.h>
#include <linux/types.h>
+#define VSDC_MODEL_DC8200 0x8200
+#define VSDC_MODEL_DCU_LITE 0x0
+
struct vs_formats {
const u32 *array;
unsigned int num;
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
index cbb125c46b39..67d4b00f294e 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
@@ -16,6 +16,9 @@
#define VSDC_FB_STRIDE(n) (0x1408 + 0x4 * (n))
#define VSDC_FB_CONFIG(n) (0x1518 + 0x4 * (n))
+#define VSDC_FB_CONFIG_ENABLE BIT(0)
+#define VSDC_FB_CONFIG_VALID BIT(3)
+#define VSDC_FB_CONFIG_RESET BIT(4)
#define VSDC_FB_CONFIG_CLEAR_EN BIT(8)
#define VSDC_FB_CONFIG_ROT_MASK GENMASK(13, 11)
#define VSDC_FB_CONFIG_ROT(v) ((v) << 11)
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table
2026-05-19 5:51 [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
2026-05-19 5:51 ` [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity Joey Lu
@ 2026-05-19 5:51 ` Joey Lu
2026-05-19 6:41 ` sashiko-bot
2026-05-19 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
3 siblings, 1 reply; 13+ messages in thread
From: Joey Lu @ 2026-05-19 5:51 UTC (permalink / raw)
To: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
The DC driver directly calls DC8200-specific register sequences from
vs_bridge.c, vs_crtc.c and vs_primary_plane.c. Supporting a second IP
variant with a different register layout would require scattering
if/else branches throughout those files.
Instead, introduce struct vs_dc_funcs, a small vtable of function
pointers covering every hardware-specific operation:
bridge_enable/disable - panel output start/stop sequence
crtc_begin/flush - per-frame commit begin/end hooks
crtc_enable/disable - display output power on/off
enable_vblank/disable_vblank - IRQ mask for vsync events
plane_enable_ex/disable_ex - framebuffer enable/disable
plane_update_ex - variant-specific plane update registers
irq_handler - read and acknowledge pending IRQs
Extract all DC8200-specific register operations from vs_bridge.c,
vs_crtc.c, vs_primary_plane.c and vs_dc.c into a new vs_dc8200.c
source file that implements the full vs_dc_funcs vtable and exposes
vs_dc8200_funcs.
Add atomic_begin and atomic_flush hooks in vs_crtc.c to dispatch to
crtc_begin/crtc_flush; these are optional (NULL-checked) so that
variants without a per-frame commit cycle can leave them unimplemented.
After vs_fill_chip_identity() confirms a DC8200 (or compatible)
identity, vs_dc_probe() assigns dc->funcs = &vs_dc8200_funcs so all
callers automatically dispatch to the correct implementation.
No functional change for DC8200 platforms.
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/gpu/drm/verisilicon/Makefile | 2 +-
drivers/gpu/drm/verisilicon/vs_bridge.c | 20 +---
drivers/gpu/drm/verisilicon/vs_crtc.c | 38 ++++++-
drivers/gpu/drm/verisilicon/vs_dc.c | 6 +-
drivers/gpu/drm/verisilicon/vs_dc.h | 32 ++++++
drivers/gpu/drm/verisilicon/vs_dc8200.c | 107 ++++++++++++++++++
.../gpu/drm/verisilicon/vs_primary_plane.c | 32 +-----
7 files changed, 186 insertions(+), 51 deletions(-)
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
index fd8d805fbcde..f4fbd9f7d6a2 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
-verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
+verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c b/drivers/gpu/drm/verisilicon/vs_bridge.c
index 7a93049368db..6a9af10c64e6 100644
--- a/drivers/gpu/drm/verisilicon/vs_bridge.c
+++ b/drivers/gpu/drm/verisilicon/vs_bridge.c
@@ -162,15 +162,8 @@ static void vs_bridge_enable_common(struct vs_crtc *crtc,
VSDC_DISP_PANEL_CONFIG_DE_EN |
VSDC_DISP_PANEL_CONFIG_DAT_EN |
VSDC_DISP_PANEL_CONFIG_CLK_EN);
- regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
- VSDC_DISP_PANEL_CONFIG_RUNNING);
- regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
- VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
- regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
- VSDC_DISP_PANEL_START_RUNNING(output));
-
- regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
- VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
+
+ dc->funcs->bridge_enable(dc, output);
}
static void vs_bridge_atomic_enable_dpi(struct drm_bridge *bridge,
@@ -228,14 +221,7 @@ static void vs_bridge_atomic_disable(struct drm_bridge *bridge,
struct vs_dc *dc = crtc->dc;
unsigned int output = crtc->id;
- regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
- VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
- VSDC_DISP_PANEL_START_RUNNING(output));
- regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
- VSDC_DISP_PANEL_CONFIG_RUNNING);
-
- regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
- VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
+ dc->funcs->bridge_disable(dc, output);
}
static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c b/drivers/gpu/drm/verisilicon/vs_crtc.c
index 9080344398ca..a87caa6f73ba 100644
--- a/drivers/gpu/drm/verisilicon/vs_crtc.c
+++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
@@ -16,10 +16,33 @@
#include "vs_crtc_regs.h"
#include "vs_crtc.h"
#include "vs_dc.h"
-#include "vs_dc_top_regs.h"
#include "vs_drm.h"
#include "vs_plane.h"
+static void vs_crtc_atomic_begin(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
+ struct vs_dc *dc = vcrtc->dc;
+ unsigned int output = vcrtc->id;
+
+ if (dc->funcs->crtc_begin)
+ dc->funcs->crtc_begin(dc, output);
+}
+
+static void vs_crtc_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
+ struct vs_dc *dc = vcrtc->dc;
+ unsigned int output = vcrtc->id;
+
+ if (dc->funcs->crtc_flush)
+ dc->funcs->crtc_flush(dc, output);
+
+ drm_crtc_vblank_atomic_flush(crtc, state);
+}
+
static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_atomic_commit *state)
{
@@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
drm_crtc_vblank_off(crtc);
clk_disable_unprepare(dc->pix_clk[output]);
+
+ if (dc->funcs->crtc_disable)
+ dc->funcs->crtc_disable(dc, output);
}
static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
@@ -42,6 +68,9 @@ static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
drm_WARN_ON(&dc->drm_dev->base,
clk_prepare_enable(dc->pix_clk[output]));
+ if (dc->funcs->crtc_enable)
+ dc->funcs->crtc_enable(dc, output);
+
drm_crtc_vblank_on(crtc);
}
@@ -119,7 +148,8 @@ static bool vs_crtc_mode_fixup(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs vs_crtc_helper_funcs = {
- .atomic_flush = drm_crtc_vblank_atomic_flush,
+ .atomic_begin = vs_crtc_atomic_begin,
+ .atomic_flush = vs_crtc_atomic_flush,
.atomic_enable = vs_crtc_atomic_enable,
.atomic_disable = vs_crtc_atomic_disable,
.mode_set_nofb = vs_crtc_mode_set_nofb,
@@ -132,7 +162,7 @@ static int vs_crtc_enable_vblank(struct drm_crtc *crtc)
struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
struct vs_dc *dc = vcrtc->dc;
- regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN, VSDC_TOP_IRQ_VSYNC(vcrtc->id));
+ dc->funcs->enable_vblank(dc, vcrtc->id);
return 0;
}
@@ -142,7 +172,7 @@ static void vs_crtc_disable_vblank(struct drm_crtc *crtc)
struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
struct vs_dc *dc = vcrtc->dc;
- regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN, VSDC_TOP_IRQ_VSYNC(vcrtc->id));
+ dc->funcs->disable_vblank(dc, vcrtc->id);
}
static const struct drm_crtc_funcs vs_crtc_funcs = {
diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c
index dad9967bc10b..c94957024189 100644
--- a/drivers/gpu/drm/verisilicon/vs_dc.c
+++ b/drivers/gpu/drm/verisilicon/vs_dc.c
@@ -8,9 +8,7 @@
#include <linux/of.h>
#include <linux/of_graph.h>
-#include "vs_crtc.h"
#include "vs_dc.h"
-#include "vs_dc_top_regs.h"
#include "vs_drm.h"
#include "vs_hwdb.h"
@@ -33,7 +31,7 @@ static irqreturn_t vs_dc_irq_handler(int irq, void *private)
struct vs_dc *dc = private;
u32 irqs;
- regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);
+ irqs = dc->funcs->irq_handler(dc);
vs_drm_handle_irq(dc, irqs);
@@ -136,6 +134,8 @@ static int vs_dc_probe(struct platform_device *pdev)
dev_info(dev, "Found DC%x rev %x customer %x\n", dc->identity.model,
dc->identity.revision, dc->identity.customer_id);
+ dc->funcs = &vs_dc8200_funcs;
+
if (port_count > dc->identity.display_count) {
dev_err(dev, "too many downstream ports than HW capability\n");
ret = -EINVAL;
diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h b/drivers/gpu/drm/verisilicon/vs_dc.h
index ed1016f18758..45172c1a525c 100644
--- a/drivers/gpu/drm/verisilicon/vs_dc.h
+++ b/drivers/gpu/drm/verisilicon/vs_dc.h
@@ -14,6 +14,7 @@
#include <linux/reset.h>
#include <drm/drm_device.h>
+#include <drm/drm_plane.h>
#include "vs_hwdb.h"
@@ -22,6 +23,34 @@
struct vs_drm_dev;
struct vs_crtc;
+struct vs_dc;
+
+struct vs_dc_funcs {
+ /* Bridge: atomic_enable, atomic_disable */
+ void (*bridge_enable)(struct vs_dc *dc, unsigned int output);
+ void (*bridge_disable)(struct vs_dc *dc, unsigned int output);
+
+ /* CRTC: atomic_begin, atomic_flush */
+ void (*crtc_begin)(struct vs_dc *dc, unsigned int output);
+ void (*crtc_flush)(struct vs_dc *dc, unsigned int output);
+
+ /* CRTC: atomic_enable, atomic_disable */
+ void (*crtc_enable)(struct vs_dc *dc, unsigned int output);
+ void (*crtc_disable)(struct vs_dc *dc, unsigned int output);
+
+ /* CRTC: enable_vblank, disable_vblank */
+ void (*enable_vblank)(struct vs_dc *dc, unsigned int output);
+ void (*disable_vblank)(struct vs_dc *dc, unsigned int output);
+
+ /* Primary plane: atomic_enable, atomic_disable, atomic_update */
+ void (*plane_enable_ex)(struct vs_dc *dc, unsigned int output);
+ void (*plane_disable_ex)(struct vs_dc *dc, unsigned int output);
+ void (*plane_update_ex)(struct vs_dc *dc, unsigned int output,
+ struct drm_plane_state *state);
+
+ /* IRQ handler */
+ u32 (*irq_handler)(struct vs_dc *dc);
+};
struct vs_dc {
struct regmap *regs;
@@ -33,6 +62,9 @@ struct vs_dc {
struct vs_drm_dev *drm_dev;
struct vs_chip_identity identity;
+ const struct vs_dc_funcs *funcs;
};
+extern const struct vs_dc_funcs vs_dc8200_funcs;
+
#endif /* _VS_DC_H_ */
diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c b/drivers/gpu/drm/verisilicon/vs_dc8200.c
new file mode 100644
index 000000000000..db9e1b3cd903
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Icenowy Zheng <uwu@icenowy.me>
+ */
+
+#include <linux/regmap.h>
+
+#include "vs_bridge_regs.h"
+#include "vs_dc.h"
+#include "vs_dc_top_regs.h"
+#include "vs_plane.h"
+#include "vs_primary_plane_regs.h"
+
+static void vs_dc8200_bridge_enable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
+ VSDC_DISP_PANEL_CONFIG_RUNNING);
+ regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
+ VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
+ regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
+ VSDC_DISP_PANEL_START_RUNNING(output));
+
+ regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
+ VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
+}
+
+static void vs_dc8200_bridge_disable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
+ VSDC_DISP_PANEL_CONFIG_RUNNING);
+ regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
+ VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
+ VSDC_DISP_PANEL_START_RUNNING(output));
+
+ regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
+ VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
+}
+
+static void vs_dc8200_enable_vblank(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
+ VSDC_TOP_IRQ_VSYNC(output));
+}
+
+static void vs_dc8200_disable_vblank(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
+ VSDC_TOP_IRQ_VSYNC(output));
+}
+
+static void vs_dc8200_plane_commit(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+ VSDC_FB_CONFIG_EX_COMMIT);
+}
+
+static void vs_dc8200_plane_enable_ex(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+ VSDC_FB_CONFIG_EX_FB_EN);
+ regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+ VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
+ VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
+
+ vs_dc8200_plane_commit(dc, output);
+}
+
+static void vs_dc8200_plane_disable_ex(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+ VSDC_FB_CONFIG_EX_FB_EN);
+
+ vs_dc8200_plane_commit(dc, output);
+}
+
+static void vs_dc8200_plane_update_ex(struct vs_dc *dc, unsigned int output,
+ struct drm_plane_state *state)
+{
+ regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
+ VSDC_MAKE_PLANE_POS(state->crtc_x, state->crtc_y));
+ regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
+ VSDC_MAKE_PLANE_POS(state->crtc_x + state->crtc_w,
+ state->crtc_y + state->crtc_h));
+ regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
+ VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
+
+ vs_dc8200_plane_commit(dc, output);
+}
+
+static u32 vs_dc8200_irq_handler(struct vs_dc *dc)
+{
+ u32 irqs;
+
+ regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);
+ return irqs;
+}
+
+const struct vs_dc_funcs vs_dc8200_funcs = {
+ .bridge_enable = vs_dc8200_bridge_enable,
+ .bridge_disable = vs_dc8200_bridge_disable,
+ .enable_vblank = vs_dc8200_enable_vblank,
+ .disable_vblank = vs_dc8200_disable_vblank,
+ .plane_enable_ex = vs_dc8200_plane_enable_ex,
+ .plane_disable_ex = vs_dc8200_plane_disable_ex,
+ .plane_update_ex = vs_dc8200_plane_update_ex,
+ .irq_handler = vs_dc8200_irq_handler,
+};
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 1f2be41ae496..75bc36a078f7 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -53,12 +53,6 @@ static int vs_primary_plane_atomic_check(struct drm_plane *plane,
return 0;
}
-static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int output)
-{
- regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
- VSDC_FB_CONFIG_EX_COMMIT);
-}
-
static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
struct drm_atomic_commit *atomic_state)
{
@@ -69,13 +63,8 @@ static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
unsigned int output = vcrtc->id;
struct vs_dc *dc = vcrtc->dc;
- regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
- VSDC_FB_CONFIG_EX_FB_EN);
- regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
- VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
- VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
-
- vs_primary_plane_commit(dc, output);
+ if (dc->funcs->plane_enable_ex)
+ dc->funcs->plane_enable_ex(dc, output);
}
static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
@@ -88,10 +77,8 @@ static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
unsigned int output = vcrtc->id;
struct vs_dc *dc = vcrtc->dc;
- regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
- VSDC_FB_CONFIG_EX_FB_EN);
-
- vs_primary_plane_commit(dc, output);
+ if (dc->funcs->plane_disable_ex)
+ dc->funcs->plane_disable_ex(dc, output);
}
static void vs_primary_plane_atomic_update(struct drm_plane *plane,
@@ -133,18 +120,11 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
regmap_write(dc->regs, VSDC_FB_STRIDE(output),
fb->pitches[0]);
- regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
- VSDC_MAKE_PLANE_POS(state->crtc_x, state->crtc_y));
- regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
- VSDC_MAKE_PLANE_POS(state->crtc_x + state->crtc_w,
- state->crtc_y + state->crtc_h));
regmap_write(dc->regs, VSDC_FB_SIZE(output),
VSDC_MAKE_PLANE_SIZE(state->crtc_w, state->crtc_h));
- regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
- VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
-
- vs_primary_plane_commit(dc, output);
+ if (dc->funcs->plane_update_ex)
+ dc->funcs->plane_update_ex(dc, output, state);
}
static const struct drm_plane_helper_funcs vs_primary_plane_helper_funcs = {
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support
2026-05-19 5:51 [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
` (2 preceding siblings ...)
2026-05-19 5:51 ` [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table Joey Lu
@ 2026-05-19 5:51 ` Joey Lu
2026-05-19 7:17 ` sashiko-bot
2026-05-19 7:44 ` Icenowy Zheng
3 siblings, 2 replies; 13+ messages in thread
From: Joey Lu @ 2026-05-19 5:51 UTC (permalink / raw)
To: zhengxingda, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
The Nuvoton MA35D1 SoC integrates a Verisilicon DCU Lite display
controller. While its register layout is broadly similar to the DC8200,
several differences require dedicated hardware ops:
1. No CONFIG_EX commit path: framebuffer updates use enable (bit 0) and
reset (bit 4) bits in FB_CONFIG instead of the DC8200 staging registers
(FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT, FB_BLEND_CONFIG,
PANEL_CONFIG_EX).
2. No PANEL_START register: panel output starts when
PANEL_CONFIG.RUNNING is set; no multi-display sync start register
is used.
3. Different IRQ registers: DCU Lite uses DISP_IRQ_STA (0x147C) /
DISP_IRQ_EN (0x1480) versus DC8200's TOP_IRQ_ACK (0x0010) /
TOP_IRQ_EN (0x0014).
4. Per-frame commit cycle: DCU Lite requires the VALID bit in FB_CONFIG
to be set at the start of each atomic commit (crtc_begin) and cleared
after (crtc_flush).
5. Simpler clock topology: only "core" (bus gate) and "pix0" (pixel
divider) clocks; no axi or ahb clocks. Make axi_clk and ahb_clk
optional (devm_clk_get_optional_enabled) so DCU Lite nodes without
those clocks are handled gracefully.
Add vs_dcu_lite.c implementing the vs_dc_funcs vtable for the above
differences. After chip identity detection, vs_dc_probe() now selects
vs_dcu_lite_funcs when the identified model is VSDC_MODEL_DCU_LITE
(model register reads 0, revision 0x5560, customer_id 0x305).
Extend Kconfig to allow building on ARCH_MA35 platforms.
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/gpu/drm/verisilicon/Kconfig | 2 +-
drivers/gpu/drm/verisilicon/Makefile | 2 +-
drivers/gpu/drm/verisilicon/vs_dc.c | 9 ++-
drivers/gpu/drm/verisilicon/vs_dc.h | 1 +
drivers/gpu/drm/verisilicon/vs_dcu_lite.c | 78 +++++++++++++++++++++++
5 files changed, 87 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/verisilicon/vs_dcu_lite.c
diff --git a/drivers/gpu/drm/verisilicon/Kconfig b/drivers/gpu/drm/verisilicon/Kconfig
index 7cce86ec8603..295d246eb4b4 100644
--- a/drivers/gpu/drm/verisilicon/Kconfig
+++ b/drivers/gpu/drm/verisilicon/Kconfig
@@ -2,7 +2,7 @@
config DRM_VERISILICON_DC
tristate "DRM Support for Verisilicon DC-series display controllers"
depends on DRM && COMMON_CLK
- depends on RISCV || COMPILE_TEST
+ depends on RISCV || ARCH_MA35 || COMPILE_TEST
select DRM_BRIDGE_CONNECTOR
select DRM_CLIENT_SELECTION
select DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
index f4fbd9f7d6a2..bf88f627e65c 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
-verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
+verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o vs_dcu_lite.o vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c
index c94957024189..77bc63c629f7 100644
--- a/drivers/gpu/drm/verisilicon/vs_dc.c
+++ b/drivers/gpu/drm/verisilicon/vs_dc.c
@@ -90,13 +90,13 @@ static int vs_dc_probe(struct platform_device *pdev)
return PTR_ERR(dc->core_clk);
}
- dc->axi_clk = devm_clk_get_enabled(dev, "axi");
+ dc->axi_clk = devm_clk_get_optional_enabled(dev, "axi");
if (IS_ERR(dc->axi_clk)) {
dev_err(dev, "can't get axi clock\n");
return PTR_ERR(dc->axi_clk);
}
- dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
+ dc->ahb_clk = devm_clk_get_optional_enabled(dev, "ahb");
if (IS_ERR(dc->ahb_clk)) {
dev_err(dev, "can't get ahb clock\n");
return PTR_ERR(dc->ahb_clk);
@@ -134,7 +134,10 @@ static int vs_dc_probe(struct platform_device *pdev)
dev_info(dev, "Found DC%x rev %x customer %x\n", dc->identity.model,
dc->identity.revision, dc->identity.customer_id);
- dc->funcs = &vs_dc8200_funcs;
+ if (dc->identity.model == VSDC_MODEL_DC8200)
+ dc->funcs = &vs_dc8200_funcs;
+ else
+ dc->funcs = &vs_dcu_lite_funcs;
if (port_count > dc->identity.display_count) {
dev_err(dev, "too many downstream ports than HW capability\n");
diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h b/drivers/gpu/drm/verisilicon/vs_dc.h
index 45172c1a525c..d77d4a1babdf 100644
--- a/drivers/gpu/drm/verisilicon/vs_dc.h
+++ b/drivers/gpu/drm/verisilicon/vs_dc.h
@@ -66,5 +66,6 @@ struct vs_dc {
};
extern const struct vs_dc_funcs vs_dc8200_funcs;
+extern const struct vs_dc_funcs vs_dcu_lite_funcs;
#endif /* _VS_DC_H_ */
diff --git a/drivers/gpu/drm/verisilicon/vs_dcu_lite.c b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
new file mode 100644
index 000000000000..11ef57d5ebaa
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Joey Lu <yclu4@nuvoton.com>
+ */
+
+#include <linux/regmap.h>
+
+#include "vs_crtc_regs.h"
+#include "vs_dc.h"
+#include "vs_primary_plane_regs.h"
+
+static void vs_dcu_lite_bridge_enable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_RESET);
+}
+
+static void vs_dcu_lite_bridge_disable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_RESET);
+}
+
+static void vs_dcu_lite_crtc_begin(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_VALID);
+}
+
+static void vs_dcu_lite_crtc_flush(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_VALID);
+}
+
+static void vs_dcu_lite_crtc_enable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_ENABLE);
+}
+
+static void vs_dcu_lite_crtc_disable(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
+ VSDC_FB_CONFIG_ENABLE);
+}
+
+static void vs_dcu_lite_enable_vblank(struct vs_dc *dc, unsigned int output)
+{
+ regmap_set_bits(dc->regs, VSDC_DISP_IRQ_EN,
+ VSDC_DISP_IRQ_VSYNC(output));
+}
+
+static void vs_dcu_lite_disable_vblank(struct vs_dc *dc, unsigned int output)
+{
+ regmap_clear_bits(dc->regs, VSDC_DISP_IRQ_EN,
+ VSDC_DISP_IRQ_VSYNC(output));
+}
+
+static u32 vs_dcu_lite_irq_handler(struct vs_dc *dc)
+{
+ u32 irqs;
+
+ regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &irqs);
+ return irqs;
+}
+
+const struct vs_dc_funcs vs_dcu_lite_funcs = {
+ .bridge_enable = vs_dcu_lite_bridge_enable,
+ .bridge_disable = vs_dcu_lite_bridge_disable,
+ .crtc_begin = vs_dcu_lite_crtc_begin,
+ .crtc_flush = vs_dcu_lite_crtc_flush,
+ .crtc_enable = vs_dcu_lite_crtc_enable,
+ .crtc_disable = vs_dcu_lite_crtc_disable,
+ .enable_vblank = vs_dcu_lite_enable_vblank,
+ .disable_vblank = vs_dcu_lite_disable_vblank,
+ .irq_handler = vs_dcu_lite_irq_handler,
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
@ 2026-05-19 6:15 ` sashiko-bot
2026-05-19 7:26 ` [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: " Icenowy Zheng
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-19 6:15 UTC (permalink / raw)
To: Joey Lu; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing explicit array length constraints in variant-specific conditional blocks.
- [High] Generic catch-all compatible string enforces SoC-specific limitations and leaves array contents unvalidated.
- [Medium] Incomplete schema topology enforcement allows conflicting 'port' and 'ports' properties.
--
commit e03be5624915d85b3073bbb37ac0fe652c0887cc
Author: Joey Lu <a0987203069@gmail.com>
dt-bindings: display: verisilicon,dc: generalize for single-output variants
This commit updates the verisilicon,dc display controller binding to support
single-output variants like the Verisilicon DCU Lite. It transitions the
schema to use an allOf/if-then-else structure to differentiate between the
existing dual-port th1520-dc8200 and a new generic verisilicon,dc compatible.
The top-level constraints are relaxed, with variant-specific rules intended
to be enforced in the conditional blocks.
> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f20..3a814c2e083ee 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
[ ... ]
> @@ -75,9 +69,75 @@ required:
> - - ports
>
> -additionalProperties: false
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: thead,th1520-dc8200
> + then:
> + properties:
> + clocks:
> + items:
> + - description: DC Core clock
> + - description: DMA AXI bus clock
> + - description: Configuration AHB bus clock
> + - description: Pixel clock of output 0
> + - description: Pixel clock of output 1
> +
> + clock-names:
> + items:
> + - const: core
> + - const: axi
> + - const: ahb
> + - const: pix0
> + - const: pix1
> +
> + resets:
> + items:
> + - description: DC Core reset
> + - description: DMA AXI bus reset
> + - description: Configuration AHB bus reset
> +
> + reset-names:
> + items:
> + - const: core
> + - const: axi
> + - const: ahb
Does JSON schema validation require explicit minItems and maxItems here to
override the top-level ranges?
At the top level, clocks has minItems: 2 and maxItems: 5, and resets has
minItems: 1 and maxItems: 3. Providing an items array inside a conditional
block does not override those top-level ranges unless explicitly redefined.
Could a device tree node for the thead variant incorrectly pass validation
with only 2 clocks or 1 reset, completely bypassing the intended strict
hardware constraints?
> +
> + required:
> + - ports
Since the top-level schema now evaluates the port property globally, should
this branch explicitly forbid it?
Without rejecting port, for example by using properties: { port: false } or
not: { required: [port] }, could a malformed device tree node for the thead
variant define both port and ports simultaneously without triggering a schema
validation error?
> +
> + else:
> + properties:
> + clocks:
> + items:
> + - description: Bus clock that gates register access
> + - description: Pixel clock divider for display timing
> +
> + clock-names:
> + items:
> + - const: core
> + - const: pix0
The top-level rules allow up to maxItems: 5 for clock-names, but this block
only lists 2 items without setting an explicit maxItems: 2 constraint.
Could a node supply 3 to 5 clocks with unconstrained, undocumented string
names that completely bypass standard schema validation?
> +
> + resets:
> + maxItems: 1
> + description:
> + Reset line for the display controller.
This branch represents a generic fallback for verisilicon,dc, but caps resets
at 1, which appears to be a Nuvoton-specific constraint.
Will this cause validation failures for future hardware integrations of this
IP that require multiple resets?
Should this binding use a specific compatible string for the Nuvoton hardware,
such as nuvoton,ma35d1-dcu, instead of enforcing these limits on a generic
catch-all string?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055114.1886525-1-a0987203069@gmail.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity
2026-05-19 5:51 ` [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity Joey Lu
@ 2026-05-19 6:25 ` sashiko-bot
2026-05-19 7:37 ` Icenowy Zheng
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-19 6:25 UTC (permalink / raw)
To: Joey Lu; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Specific chip identities in the `vs_chip_identities[]` array are permanently shadowed by an earlier wildcard entry, making them unreachable.
- [High] Missing error checks on `regmap_read()` cause uninitialized stack variables to be used in hardware matching.
--
commit d87a3d0928c8de1f7d370f6130d55b01b2660eb9
Author: Joey Lu <a0987203069@gmail.com>
drm/verisilicon: add model ID constants and DCU Lite chip identity
This commit introduces symbolic constants for model IDs to replace magic
numbers and adds the DCU Lite chip identity to vs_chip_identities[]. It also
adds register-level macros for forthcoming DCU Lite support.
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 09336af0900ae..a25c4b16181d6 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -90,7 +90,7 @@ static const struct vs_formats vs_formats_with_yuv444 = {
>
> static struct vs_chip_identity vs_chip_identities[] = {
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
> .revision = 0x5720,
> .customer_id = ~0U,
This isn't a bug introduced by this patch, but does this first entry with
.customer_id = ~0U permanently shadow the later entries for the same model
and revision?
The loop in vs_fill_chip_identity() evaluates these sequentially and returns
on the first match. Since ~0U acts as a wildcard, it appears the later
specific entries for revision 0x5720 (like customer 0x310 which enables
YUV444) will never be reached.
[ ... ]
> @@ -114,13 +114,21 @@ static struct vs_chip_identity vs_chip_identities[] = {
> .formats = &vs_formats_with_yuv444,
> },
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
> .revision = 0x5720,
> .customer_id = 0x311,
>
> .display_count = 2,
> .formats = &vs_formats_no_yuv444,
> },
> + {
> + .model = VSDC_MODEL_DCU_LITE,
> + .revision = 0x5560,
> + .customer_id = 0x305,
> +
> + .display_count = 1,
> + .formats = &vs_formats_no_yuv444,
> + },
> };
>
> int vs_fill_chip_identity(struct regmap *regs,
> struct vs_chip_identity *ident)
Also not introduced by this patch, but looking at the rest of
vs_fill_chip_identity() just below this array, do we need to check the return
values of the regmap_read() calls?
It looks like the model, revision, and customer_id variables are declared on
the stack without initialization:
u32 model;
u32 revision;
u32 customer_id;
regmap_read(regs, VSDC_TOP_CHIP_MODEL, &model);
regmap_read(regs, VSDC_TOP_CHIP_REV, &revision);
regmap_read(regs, VSDC_TOP_CHIP_CUSTOMER_ID, &customer_id);
If regmap_read() fails (e.g., due to an unclocked device or hardware
timeout), it would leave these variables uninitialized.
Could this lead to undefined behavior when they are evaluated in the
vs_chip_identities loop condition, potentially matching a random hardware
profile?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055114.1886525-1-a0987203069@gmail.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table
2026-05-19 5:51 ` [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table Joey Lu
@ 2026-05-19 6:41 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-19 6:41 UTC (permalink / raw)
To: Joey Lu; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly added `crtc_disable` hook is invoked after the pixel clock is disabled, which will cause bus errors.
- [High] The register clearing sequence in `vs_dc8200_bridge_disable` is inverted compared to the original code, causing a functional regression.
- [Medium] Unconditional assignment of `dc->funcs` without verifying hardware identity, contradicting the commit message.
--
commit 9ad7489590b539e39997172552aadb2d24ac579b
Author: Joey Lu <a0987203069@gmail.com>
drm/verisilicon: introduce per-variant hardware ops table
This commit refactors hardware-specific operations into a vs_dc_funcs
vtable to support multiple IP variants without scattering if/else branches,
extracting DC8200-specific operations into vs_dc8200.c.
> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c b/drivers/gpu/drm/verisilicon/vs_crtc.c
> index 9080344398ca2..a87caa6f73bac 100644
> --- a/drivers/gpu/drm/verisilicon/vs_crtc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
[ ... ]
> @@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
> drm_crtc_vblank_off(crtc);
>
> clk_disable_unprepare(dc->pix_clk[output]);
> +
> + if (dc->funcs->crtc_disable)
> + dc->funcs->crtc_disable(dc, output);
> }
Will calling the crtc_disable hook here cause bus errors? It looks like
clk_disable_unprepare() disables the pixel clock before we call into the
hardware-specific teardown function, which likely accesses MMIO registers
while the clock is off. Symmetrically, vs_crtc_atomic_enable() invokes
crtc_enable after the clock is prepared and enabled.
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c
> index dad9967bc10b8..c949570241890 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
[ ... ]
> @@ -136,6 +134,8 @@ static int vs_dc_probe(struct platform_device *pdev)
> dev_info(dev, "Found DC%x rev %x customer %x\n", dc->identity.model,
> dc->identity.revision, dc->identity.customer_id);
>
> + dc->funcs = &vs_dc8200_funcs;
> +
> if (port_count > dc->identity.display_count) {
> dev_err(dev, "too many downstream ports than HW capability\n");
> ret = -EINVAL;
The commit message states "After vs_fill_chip_identity() confirms a DC8200
(or compatible) identity, vs_dc_probe() assigns dc->funcs = &vs_dc8200_funcs".
However, this assignment happens unconditionally without checking
dc->identity.model. Could this apply the wrong operations if probed with a
different hardware variant?
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> new file mode 100644
> index 0000000000000..db9e1b3cd9032
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
[ ... ]
> +static void vs_dc8200_bridge_disable(struct vs_dc *dc, unsigned int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> + VSDC_DISP_PANEL_CONFIG_RUNNING);
> + regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> + VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
> + VSDC_DISP_PANEL_START_RUNNING(output));
Is this register clearing sequence inverted compared to the original code?
The commit message says "No functional change for DC8200 platforms", but
the original code in vs_bridge_atomic_disable() cleared VSDC_DISP_PANEL_START
before clearing VSDC_DISP_PANEL_CONFIG:
regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
VSDC_DISP_PANEL_START_RUNNING(output));
regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
VSDC_DISP_PANEL_CONFIG_RUNNING);
Could reversing this order violate display hardware teardown constraints and
result in display glitches or pipeline lockups?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055114.1886525-1-a0987203069@gmail.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support
2026-05-19 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
@ 2026-05-19 7:17 ` sashiko-bot
2026-05-19 7:44 ` Icenowy Zheng
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-19 7:17 UTC (permalink / raw)
To: Joey Lu; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Pre-existing early IRQ request race condition leads to a NULL pointer dereference crash during probe.
- [High] The bridge_enable callback fails to set the PANEL_CONFIG.RUNNING bit, contradicting the commit message and preventing panel output.
- [High] Framebuffer enablement is incorrectly tied to the CRTC lifecycle rather than the Plane lifecycle, risking IOMMU faults or memory corruption.
- [High] The DCU Lite interrupt handler reads the status register but fails to acknowledge (clear) the interrupts, risking an interrupt storm.
- [High] Pre-existing heap buffer overflow in `vs_dc_probe()` due to unchecked `display_count` from hardware.
--
commit 234a098981976180e2d53f562af404422d7cd953
Author: Joey Lu <a0987203069@gmail.com>
drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support
This commit adds support for the Verisilicon DCU Lite display controller
found on the Nuvoton MA35D1 SoC. It implements a new vs_dc_funcs vtable
with dedicated hardware operations to account for differences from the
existing DC8200 controller.
> 2. No PANEL_START register: panel output starts when
> PANEL_CONFIG.RUNNING is set; no multi-display sync start register
> is used.
The commit message explicitly documents that panel output starts when
PANEL_CONFIG.RUNNING is set.
[ ... ]
> diff --git a/drivers/gpu/drm/verisilicon/vs_dcu_lite.c b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> new file mode 100644
> index 0000000000000..11ef57d5ebaa4
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> @@ -0,0 +1,78 @@
[ ... ]
> +static void vs_dcu_lite_bridge_enable(struct vs_dc *dc, unsigned int output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_RESET);
> +}
However, vs_dcu_lite_bridge_enable() sets VSDC_FB_CONFIG_RESET instead,
and never writes to PANEL_CONFIG.RUNNING.
Will this prevent panel output from starting? Also, could asserting a reset
bit during the enable path hold the framebuffer block in a continuous reset
state?
[ ... ]
> +static void vs_dcu_lite_crtc_enable(struct vs_dc *dc, unsigned int output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
> +
> +static void vs_dcu_lite_crtc_disable(struct vs_dc *dc, unsigned int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
The patch implements crtc_enable and crtc_disable to toggle
VSDC_FB_CONFIG_ENABLE, but leaves plane disable callbacks unimplemented.
If userspace dynamically disables the primary plane while keeping the CRTC
active, will the hardware framebuffer remain enabled?
Could this cause the hardware to continuously scan out from a disabled DRM
framebuffer that the kernel might unpin or free, leading to IOMMU faults?
[ ... ]
> +static u32 vs_dcu_lite_irq_handler(struct vs_dc *dc)
> +{
> + u32 irqs;
> +
> + regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &irqs);
> + return irqs;
> +}
The interrupt handler reads VSDC_DISP_IRQ_STA but does not appear to write
back to clear the pending interrupt bits.
If this status register requires a Write-1-to-Clear (W1C) operation, could
failing to explicitly acknowledge the interrupt cause the IRQ line to remain
asserted, resulting in an interrupt storm?
While reviewing this patch, I noticed a pre-existing issue in vs_dc_probe().
The code reads dc->identity.display_count from hardware registers and
loops up to this value to populate the dc->pix_clk array:
drivers/gpu/drm/verisilicon/vs_dc.c:vs_dc_probe() {
...
if (port_count > dc->identity.display_count) {
dev_err(dev, "too many downstream ports than HW capability\n");
ret = -EINVAL;
goto err_rst_assert;
}
for (i = 0; i < dc->identity.display_count; i++) {
snprintf(pixclk_name, sizeof(pixclk_name), "pix%u", i);
dc->pix_clk[i] = devm_clk_get(dev, pixclk_name);
...
}
While the code validates port_count <= VSDC_MAX_OUTPUTS and
port_count <= dc->identity.display_count, it doesn't appear to ensure
dc->identity.display_count <= VSDC_MAX_OUTPUTS.
Could a malfunctioning hardware device reporting a large display_count
cause an out-of-bounds write of clock pointers on the kernel heap?
There also appears to be a pre-existing race condition in vs_dc_probe()
where the IRQ is requested before the DRM device is initialized:
drivers/gpu/drm/verisilicon/vs_dc.c:vs_dc_probe() {
...
ret = devm_request_irq(dev, irq, vs_dc_irq_handler, 0,
dev_name(dev), dc);
...
ret = vs_drm_initialize(dc, pdev);
...
}
If the bootloader leaves the display controller active, could the hardware
immediately fire an interrupt once unmasked?
If this occurs, the interrupt handler vs_dc_irq_handler() would call
vs_drm_handle_irq(), which dereferences dc->drm_dev->crtcs[i]. Because
dc->drm_dev is not allocated and assigned until vs_drm_initialize() runs
later in the probe sequence, would this cause a NULL pointer dereference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055114.1886525-1-a0987203069@gmail.com?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: generalize for single-output variants
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
2026-05-19 6:15 ` sashiko-bot
@ 2026-05-19 7:26 ` Icenowy Zheng
2026-05-19 16:47 ` Conor Dooley
1 sibling, 1 reply; 13+ messages in thread
From: Icenowy Zheng @ 2026-05-19 7:26 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
在 2026-05-19二的 13:51 +0800,Joey Lu写道:
> The existing schema assumes a fixed clock/reset topology and dual-
> output
> port structure matching the DC8200 IP block. This prevents reuse for
> single-output variants such as the Verisilicon DCU Lite used in the
> Nuvoton MA35D1 SoC.
>
> Rework the schema so that variant-specific constraints are expressed
> via allOf/if-then-else:
>
> - The thead,th1520-dc8200 compatible keeps its existing five-clock,
> three-reset, dual-port requirements.
>
> - A standalone verisilicon,dc compatible covers IPs whose identity is
> discovered entirely through hardware registers; these have flexible
> clock and reset counts, a single 'port' property, and no 'ports'
> requirement.
>
> Changes to the base schema:
> - Replace the fixed clock/reset items lists with minItems/maxItems
> ranges; variant sub-schemas tighten the constraints via if-then-
> else.
> - Add a 'port' property (graph.yaml single-port alias) alongside the
> existing 'ports', for single-output variants.
> - Drop the unconditional 'ports' requirement; each if-branch enforces
> its own port topology.
> - Tighten additionalProperties to unevaluatedProperties to allow
> per-variant schemas to add their own constraints cleanly.
> - Fix a stray space in the port@0 description.
> - Add a DT example for the generic verisilicon,dc compatible
> (Nuvoton MA35D1 DCU Lite).
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> .../bindings/display/verisilicon,dc.yaml | 135 ++++++++++++++--
> --
> 1 file changed, 108 insertions(+), 27 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f2..3a814c2e083e 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> @@ -14,10 +14,12 @@ properties:
> pattern: "^display@[0-9a-f]+$"
>
> compatible:
> - items:
> - - enum:
> - - thead,th1520-dc8200
You should add a fallback compatible here for your SoC, in case its
integration gets something quirky; this compatible is usually not
consumed by the driver (see how thead,th1520-dc8200 exists in the
binding but not the driver).
> - - const: verisilicon,dc # DC IPs have discoverable ID/revision
> registers
> + oneOf:
> + - items:
> + - enum:
> + - thead,th1520-dc8200
> + - const: verisilicon,dc
> + - const: verisilicon,dc # DC IPs have discoverable
> ID/revision registers
>
> reg:
> maxItems: 1
> @@ -26,32 +28,24 @@ properties:
> maxItems: 1
>
> clocks:
> - items:
> - - description: DC Core clock
> - - description: DMA AXI bus clock
> - - description: Configuration AHB bus clock
> - - description: Pixel clock of output 0
> - - description: Pixel clock of output 1
> + minItems: 2
> + maxItems: 5
>
> clock-names:
> - items:
> - - const: core
> - - const: axi
> - - const: ahb
> - - const: pix0
> - - const: pix1
> + minItems: 2
> + maxItems: 5
>
> resets:
> - items:
> - - description: DC Core reset
> - - description: DMA AXI bus reset
> - - description: Configuration AHB bus reset
> + minItems: 1
> + maxItems: 3
>
> reset-names:
> - items:
> - - const: core
> - - const: axi
> - - const: ahb
> + minItems: 1
> + maxItems: 3
> +
> + port:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Single video output port for single-output
> variants.
Maybe the endpoint numbering rule needs a move to here? (I am not very
sure).
>
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
> @@ -59,7 +53,7 @@ properties:
> properties:
> port@0:
> $ref: /schemas/graph.yaml#/properties/port
> - description: The first output channel , endpoint 0 should be
> + description: The first output channel, endpoint 0 should be
> used for DPI format output and endpoint 1 should be used
> for DP format output.
>
> @@ -75,9 +69,75 @@ required:
> - interrupts
> - clocks
> - clock-names
> - - ports
>
> -additionalProperties: false
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: thead,th1520-dc8200
> + then:
> + properties:
> + clocks:
> + items:
> + - description: DC Core clock
> + - description: DMA AXI bus clock
> + - description: Configuration AHB bus clock
> + - description: Pixel clock of output 0
> + - description: Pixel clock of output 1
> +
> + clock-names:
> + items:
> + - const: core
> + - const: axi
> + - const: ahb
> + - const: pix0
> + - const: pix1
> +
> + resets:
> + items:
> + - description: DC Core reset
> + - description: DMA AXI bus reset
> + - description: Configuration AHB bus reset
> +
> + reset-names:
> + items:
> + - const: core
> + - const: axi
> + - const: ahb
> +
> + required:
> + - ports
> +
> + else:
> + properties:
> + clocks:
> + items:
> + - description: Bus clock that gates register access
> + - description: Pixel clock divider for display timing
Please don't make compatible-specific description strings for
individual compatibles, and keep these descriptions outside of the if.
The compatible-specific part should be used to specify what's required
for the specific SoC, for dt validation purpose.
BTW if the clock is both the working clock and bus clock for the
controller, I suggest listing it twice, except if the IP core is
provided without a dedicated core clock (in the case I suggest to use
"bus" only).
Here's an example for "listing it twice":
```
clocks = <&clk DCU_GATE>, <&clk DCU_GATE>, <&clk DCUP_DIV>;
clock-names = "core", "bus", "pix0";
```
Well nonetheless the name "core" does not match the description "Bus
clock that gates register access".
Thanks,
Icenowy
> +
> + clock-names:
> + items:
> + - const: core
> + - const: pix0
> +
> + resets:
> + maxItems: 1
> + description:
> + Reset line for the display controller.
> +
> + reset-names:
> + items:
> + - const: core
> +
> + required:
> + - port
> +
> + not:
> + required:
> + - ports
> +
> +unevaluatedProperties: false
>
> examples:
> - |
> @@ -120,3 +180,24 @@ examples:
> };
> };
> };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
> +
> + display@40260000 {
> + compatible = "verisilicon,dc";
> + reg = <0x40260000 0x20000>;
> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk DCU_GATE>, <&clk DCUP_DIV>;
> + clock-names = "core", "pix0";
> + resets = <&sys MA35D1_RESET_DISP>;
> + reset-names = "core";
> +
> + port {
> + dpi_out: endpoint {
> + remote-endpoint = <&panel_in>;
> + };
> + };
> + };
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity
2026-05-19 5:51 ` [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity Joey Lu
2026-05-19 6:25 ` sashiko-bot
@ 2026-05-19 7:37 ` Icenowy Zheng
1 sibling, 0 replies; 13+ messages in thread
From: Icenowy Zheng @ 2026-05-19 7:37 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
在 2026-05-19二的 13:51 +0800,Joey Lu写道:
> Introduce symbolic constants VSDC_MODEL_DC8200 and
> VSDC_MODEL_DCU_LITE
> to replace magic numbers in the hardware database and probe path.
>
> Register the DCU Lite chip identity (model 0x0, revision 0x5560,
> customer_id 0x305) in vs_chip_identities[], making the existing
> vs_fill_chip_identity() path able to recognise Nuvoton MA35D1
> hardware
> purely through register reads.
The HWDB change should be added in the end of the series, making it a
gate to the newly added changes that is finally opened when
everything's ready.
>
> Also add three register-level macros for forthcoming DCU Lite
> support:
> - VSDC_DISP_IRQ_VSYNC(n) in vs_crtc_regs.h, for per-output VSYNC IRQ
> bits used by the DCU Lite IRQ enable/status registers.
> - VSDC_FB_CONFIG_ENABLE, VSDC_FB_CONFIG_VALID and
> VSDC_FB_CONFIG_RESET
> in vs_primary_plane_regs.h, for the framebuffer enable and
> commit-cycle bits used by the DCU Lite plane update path.
Maybe you can split the register change
>
> No behaviour change for existing DC8200 platforms.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> drivers/gpu/drm/verisilicon/vs_crtc_regs.h | 1 +
> drivers/gpu/drm/verisilicon/vs_hwdb.c | 16 ++++++++++++--
> --
> drivers/gpu/drm/verisilicon/vs_hwdb.h | 3 +++
> .../gpu/drm/verisilicon/vs_primary_plane_regs.h | 3 +++
> 4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
> b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
> index c7930e817635..d4da22b08cd5 100644
> --- a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
> +++ b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
> @@ -54,6 +54,7 @@
> #define VSDC_DISP_GAMMA_DATA(n) (0x1460 +
> 0x4 * (n))
>
> #define VSDC_DISP_IRQ_STA 0x147C
> +#define VSDC_DISP_IRQ_VSYNC(n) BIT(n)
>
> #define VSDC_DISP_IRQ_EN 0x1480
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 09336af0900a..a25c4b16181d 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -90,7 +90,7 @@ static const struct vs_formats
> vs_formats_with_yuv444 = {
>
> static struct vs_chip_identity vs_chip_identities[] = {
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
I don't think such a macro is needed.
> .revision = 0x5720,
> .customer_id = ~0U,
>
> @@ -98,7 +98,7 @@ static struct vs_chip_identity vs_chip_identities[]
> = {
> .formats = &vs_formats_no_yuv444,
> },
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
> .revision = 0x5721,
> .customer_id = 0x30B,
>
> @@ -106,7 +106,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .formats = &vs_formats_no_yuv444,
> },
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
> .revision = 0x5720,
> .customer_id = 0x310,
>
> @@ -114,13 +114,21 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .formats = &vs_formats_with_yuv444,
> },
> {
> - .model = 0x8200,
> + .model = VSDC_MODEL_DC8200,
> .revision = 0x5720,
> .customer_id = 0x311,
>
> .display_count = 2,
> .formats = &vs_formats_no_yuv444,
> },
> + {
> + .model = VSDC_MODEL_DCU_LITE,
The number is 0x0 and the whole public name of this IP is
"DCUltraLite", w/o any numbers.
I suggest leave it at 0x0 and add a comment saying this is DCUltraLite
-- Verisilicon people are abusing suffix for their IP names now.
> + .revision = 0x5560,
> + .customer_id = 0x305,
> +
> + .display_count = 1,
> + .formats = &vs_formats_no_yuv444,
> + },
> };
>
> int vs_fill_chip_identity(struct regmap *regs,
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> index 92192e4fa086..cca126bd2da5 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> @@ -9,6 +9,9 @@
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +#define VSDC_MODEL_DC8200 0x8200
> +#define VSDC_MODEL_DCU_LITE 0x0
> +
> struct vs_formats {
> const u32 *array;
> unsigned int num;
> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> index cbb125c46b39..67d4b00f294e 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> @@ -16,6 +16,9 @@
> #define VSDC_FB_STRIDE(n) (0x1408 + 0x4 * (n))
>
> #define VSDC_FB_CONFIG(n) (0x1518 + 0x4 * (n))
> +#define VSDC_FB_CONFIG_ENABLE BIT(0)
> +#define VSDC_FB_CONFIG_VALID BIT(3)
> +#define VSDC_FB_CONFIG_RESET BIT(4)
Should the new IRQ register to be added here too?
Thanks,
Icenowy
> #define VSDC_FB_CONFIG_CLEAR_EN BIT(8)
> #define VSDC_FB_CONFIG_ROT_MASK GENMASK(13,
> 11)
> #define VSDC_FB_CONFIG_ROT(v) ((v) << 11)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support
2026-05-19 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
2026-05-19 7:17 ` sashiko-bot
@ 2026-05-19 7:44 ` Icenowy Zheng
1 sibling, 0 replies; 13+ messages in thread
From: Icenowy Zheng @ 2026-05-19 7:44 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
在 2026-05-19二的 13:51 +0800,Joey Lu写道:
> The Nuvoton MA35D1 SoC integrates a Verisilicon DCU Lite display
> controller. While its register layout is broadly similar to the
> DC8200,
> several differences require dedicated hardware ops:
>
> 1. No CONFIG_EX commit path: framebuffer updates use enable (bit 0)
> and
> reset (bit 4) bits in FB_CONFIG instead of the DC8200 staging
> registers
> (FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT, FB_BLEND_CONFIG,
> PANEL_CONFIG_EX).
>
> 2. No PANEL_START register: panel output starts when
> PANEL_CONFIG.RUNNING is set; no multi-display sync start register
> is used.
>
> 3. Different IRQ registers: DCU Lite uses DISP_IRQ_STA (0x147C) /
> DISP_IRQ_EN (0x1480) versus DC8200's TOP_IRQ_ACK (0x0010) /
> TOP_IRQ_EN (0x0014).
>
> 4. Per-frame commit cycle: DCU Lite requires the VALID bit in
> FB_CONFIG
> to be set at the start of each atomic commit (crtc_begin) and
> cleared
> after (crtc_flush).
>
> 5. Simpler clock topology: only "core" (bus gate) and "pix0" (pixel
> divider) clocks; no axi or ahb clocks. Make axi_clk and ahb_clk
> optional (devm_clk_get_optional_enabled) so DCU Lite nodes without
> those clocks are handled gracefully.
>
> Add vs_dcu_lite.c implementing the vs_dc_funcs vtable for the above
> differences. After chip identity detection, vs_dc_probe() now
> selects
> vs_dcu_lite_funcs when the identified model is VSDC_MODEL_DCU_LITE
> (model register reads 0, revision 0x5560, customer_id 0x305).
>
> Extend Kconfig to allow building on ARCH_MA35 platforms.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> drivers/gpu/drm/verisilicon/Kconfig | 2 +-
> drivers/gpu/drm/verisilicon/Makefile | 2 +-
> drivers/gpu/drm/verisilicon/vs_dc.c | 9 ++-
> drivers/gpu/drm/verisilicon/vs_dc.h | 1 +
> drivers/gpu/drm/verisilicon/vs_dcu_lite.c | 78
> +++++++++++++++++++++++
> 5 files changed, 87 insertions(+), 5 deletions(-)
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dcu_lite.c
>
> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> b/drivers/gpu/drm/verisilicon/Kconfig
> index 7cce86ec8603..295d246eb4b4 100644
> --- a/drivers/gpu/drm/verisilicon/Kconfig
> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> @@ -2,7 +2,7 @@
> config DRM_VERISILICON_DC
> tristate "DRM Support for Verisilicon DC-series display
> controllers"
> depends on DRM && COMMON_CLK
> - depends on RISCV || COMPILE_TEST
> + depends on RISCV || ARCH_MA35 || COMPILE_TEST
> select DRM_BRIDGE_CONNECTOR
> select DRM_CLIENT_SELECTION
> select DRM_DISPLAY_HELPER
> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> b/drivers/gpu/drm/verisilicon/Makefile
> index f4fbd9f7d6a2..bf88f627e65c 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> -verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
> vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
> +verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
> vs_dcu_lite.o vs_drm.o vs_hwdb.o vs_plane.o vs_primary_plane.o
>
> obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
> b/drivers/gpu/drm/verisilicon/vs_dc.c
> index c94957024189..77bc63c629f7 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -90,13 +90,13 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> return PTR_ERR(dc->core_clk);
> }
>
> - dc->axi_clk = devm_clk_get_enabled(dev, "axi");
> + dc->axi_clk = devm_clk_get_optional_enabled(dev, "axi");
> if (IS_ERR(dc->axi_clk)) {
> dev_err(dev, "can't get axi clock\n");
> return PTR_ERR(dc->axi_clk);
> }
>
> - dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
> + dc->ahb_clk = devm_clk_get_optional_enabled(dev, "ahb");
> if (IS_ERR(dc->ahb_clk)) {
> dev_err(dev, "can't get ahb clock\n");
> return PTR_ERR(dc->ahb_clk);
> @@ -134,7 +134,10 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
> >identity.model,
> dc->identity.revision, dc->identity.customer_id);
>
> - dc->funcs = &vs_dc8200_funcs;
> + if (dc->identity.model == VSDC_MODEL_DC8200)
Don't do that. The model value is only for matching hardware values,
not for detecting what's present. Don't forget that DC8000 has a model
value of 0x8000, but behaves similarly with DCUltraLite with a model
value of 0x0.
I suggest adding another field for assigning helper functions.
My suggestion is here:
```
enum vs_dc_generation {
VSDC_GEN_DC8000,
VSDC_GEN_DC8200
};
```
Thanks,
Icenowy
> + dc->funcs = &vs_dc8200_funcs;
> + else
> + dc->funcs = &vs_dcu_lite_funcs;
>
> if (port_count > dc->identity.display_count) {
> dev_err(dev, "too many downstream ports than HW
> capability\n");
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
> b/drivers/gpu/drm/verisilicon/vs_dc.h
> index 45172c1a525c..d77d4a1babdf 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
> @@ -66,5 +66,6 @@ struct vs_dc {
> };
>
> extern const struct vs_dc_funcs vs_dc8200_funcs;
> +extern const struct vs_dc_funcs vs_dcu_lite_funcs;
>
> #endif /* _VS_DC_H_ */
> diff --git a/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> new file mode 100644
> index 000000000000..11ef57d5ebaa
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Joey Lu <yclu4@nuvoton.com>
> + */
> +
> +#include <linux/regmap.h>
> +
> +#include "vs_crtc_regs.h"
> +#include "vs_dc.h"
> +#include "vs_primary_plane_regs.h"
> +
> +static void vs_dcu_lite_bridge_enable(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_RESET);
> +}
> +
> +static void vs_dcu_lite_bridge_disable(struct vs_dc *dc, unsigned
> int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_RESET);
> +}
> +
> +static void vs_dcu_lite_crtc_begin(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_VALID);
> +}
> +
> +static void vs_dcu_lite_crtc_flush(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_VALID);
> +}
> +
> +static void vs_dcu_lite_crtc_enable(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
> +
> +static void vs_dcu_lite_crtc_disable(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
> +
> +static void vs_dcu_lite_enable_vblank(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_DISP_IRQ_EN,
> + VSDC_DISP_IRQ_VSYNC(output));
> +}
> +
> +static void vs_dcu_lite_disable_vblank(struct vs_dc *dc, unsigned
> int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_DISP_IRQ_EN,
> + VSDC_DISP_IRQ_VSYNC(output));
> +}
> +
> +static u32 vs_dcu_lite_irq_handler(struct vs_dc *dc)
> +{
> + u32 irqs;
> +
> + regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &irqs);
> + return irqs;
> +}
> +
> +const struct vs_dc_funcs vs_dcu_lite_funcs = {
> + .bridge_enable = vs_dcu_lite_bridge_enable,
> + .bridge_disable =
> vs_dcu_lite_bridge_disable,
> + .crtc_begin = vs_dcu_lite_crtc_begin,
> + .crtc_flush = vs_dcu_lite_crtc_flush,
> + .crtc_enable = vs_dcu_lite_crtc_enable,
> + .crtc_disable = vs_dcu_lite_crtc_disable,
> + .enable_vblank = vs_dcu_lite_enable_vblank,
> + .disable_vblank =
> vs_dcu_lite_disable_vblank,
> + .irq_handler = vs_dcu_lite_irq_handler,
> +};
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: generalize for single-output variants
2026-05-19 7:26 ` [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: " Icenowy Zheng
@ 2026-05-19 16:47 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-05-19 16:47 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, ychuang3, schung, yclu4, dri-devel,
devicetree, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 9586 bytes --]
On Tue, May 19, 2026 at 03:26:58PM +0800, Icenowy Zheng wrote:
> 在 2026-05-19二的 13:51 +0800,Joey Lu写道:
> > The existing schema assumes a fixed clock/reset topology and dual-
> > output
> > port structure matching the DC8200 IP block. This prevents reuse for
> > single-output variants such as the Verisilicon DCU Lite used in the
> > Nuvoton MA35D1 SoC.
> >
> > Rework the schema so that variant-specific constraints are expressed
> > via allOf/if-then-else:
> >
> > - The thead,th1520-dc8200 compatible keeps its existing five-clock,
> > three-reset, dual-port requirements.
> >
> > - A standalone verisilicon,dc compatible covers IPs whose identity is
> > discovered entirely through hardware registers; these have flexible
> > clock and reset counts, a single 'port' property, and no 'ports'
> > requirement.
> >
> > Changes to the base schema:
> > - Replace the fixed clock/reset items lists with minItems/maxItems
> > ranges; variant sub-schemas tighten the constraints via if-then-
> > else.
> > - Add a 'port' property (graph.yaml single-port alias) alongside the
> > existing 'ports', for single-output variants.
> > - Drop the unconditional 'ports' requirement; each if-branch enforces
> > its own port topology.
> > - Tighten additionalProperties to unevaluatedProperties to allow
> > per-variant schemas to add their own constraints cleanly.
> > - Fix a stray space in the port@0 description.
> > - Add a DT example for the generic verisilicon,dc compatible
> > (Nuvoton MA35D1 DCU Lite).
> >
> > Signed-off-by: Joey Lu <a0987203069@gmail.com>
> > ---
> > .../bindings/display/verisilicon,dc.yaml | 135 ++++++++++++++--
> > --
> > 1 file changed, 108 insertions(+), 27 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > index 9dc35ab973f2..3a814c2e083e 100644
> > --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > @@ -14,10 +14,12 @@ properties:
> > pattern: "^display@[0-9a-f]+$"
> >
> > compatible:
> > - items:
> > - - enum:
> > - - thead,th1520-dc8200
>
> You should add a fallback compatible here for your SoC, in case its
> integration gets something quirky; this compatible is usually not
> consumed by the driver (see how thead,th1520-dc8200 exists in the
> binding but not the driver).
s/fallback compatible/soc-specific compatible/, but yes.
NAK to what's been done here, especially after the discussions on
earlier versions of this verisilicon binding.
pw-bot: changes-requested
>
> > - - const: verisilicon,dc # DC IPs have discoverable ID/revision
> > registers
> > + oneOf:
> > + - items:
> > + - enum:
> > + - thead,th1520-dc8200
> > + - const: verisilicon,dc
> > + - const: verisilicon,dc # DC IPs have discoverable
> > ID/revision registers
> >
> > reg:
> > maxItems: 1
> > @@ -26,32 +28,24 @@ properties:
> > maxItems: 1
> >
> > clocks:
> > - items:
> > - - description: DC Core clock
> > - - description: DMA AXI bus clock
> > - - description: Configuration AHB bus clock
> > - - description: Pixel clock of output 0
> > - - description: Pixel clock of output 1
> > + minItems: 2
> > + maxItems: 5
> >
> > clock-names:
> > - items:
> > - - const: core
> > - - const: axi
> > - - const: ahb
> > - - const: pix0
> > - - const: pix1
> > + minItems: 2
> > + maxItems: 5
> >
> > resets:
> > - items:
> > - - description: DC Core reset
> > - - description: DMA AXI bus reset
> > - - description: Configuration AHB bus reset
> > + minItems: 1
> > + maxItems: 3
> >
> > reset-names:
> > - items:
> > - - const: core
> > - - const: axi
> > - - const: ahb
> > + minItems: 1
> > + maxItems: 3
> > +
> > + port:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: Single video output port for single-output
> > variants.
>
> Maybe the endpoint numbering rule needs a move to here? (I am not very
> sure).
>
> >
> > ports:
> > $ref: /schemas/graph.yaml#/properties/ports
> > @@ -59,7 +53,7 @@ properties:
> > properties:
> > port@0:
> > $ref: /schemas/graph.yaml#/properties/port
> > - description: The first output channel , endpoint 0 should be
> > + description: The first output channel, endpoint 0 should be
> > used for DPI format output and endpoint 1 should be used
> > for DP format output.
> >
> > @@ -75,9 +69,75 @@ required:
> > - interrupts
> > - clocks
> > - clock-names
> > - - ports
> >
> > -additionalProperties: false
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: thead,th1520-dc8200
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: DC Core clock
> > + - description: DMA AXI bus clock
> > + - description: Configuration AHB bus clock
> > + - description: Pixel clock of output 0
> > + - description: Pixel clock of output 1
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: axi
> > + - const: ahb
> > + - const: pix0
> > + - const: pix1
> > +
> > + resets:
> > + items:
> > + - description: DC Core reset
> > + - description: DMA AXI bus reset
> > + - description: Configuration AHB bus reset
> > +
> > + reset-names:
> > + items:
> > + - const: core
> > + - const: axi
> > + - const: ahb
> > +
> > + required:
> > + - ports
> > +
> > + else:
> > + properties:
> > + clocks:
> > + items:
> > + - description: Bus clock that gates register access
> > + - description: Pixel clock divider for display timing
>
> Please don't make compatible-specific description strings for
> individual compatibles, and keep these descriptions outside of the if.
> The compatible-specific part should be used to specify what's required
> for the specific SoC, for dt validation purpose.
>
> BTW if the clock is both the working clock and bus clock for the
> controller, I suggest listing it twice, except if the IP core is
> provided without a dedicated core clock (in the case I suggest to use
> "bus" only).
I agree. If the same clock is provided to two+ ports on the IP, that
should still be two+ clocks in the devicetree.
>
> Here's an example for "listing it twice":
> ```
> clocks = <&clk DCU_GATE>, <&clk DCU_GATE>, <&clk DCUP_DIV>;
> clock-names = "core", "bus", "pix0";
> ```
>
> Well nonetheless the name "core" does not match the description "Bus
> clock that gates register access".
>
> Thanks,
> Icenowy
>
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: pix0
> > +
> > + resets:
> > + maxItems: 1
> > + description:
> > + Reset line for the display controller.
> > +
> > + reset-names:
> > + items:
> > + - const: core
> > +
> > + required:
> > + - port
> > +
> > + not:
> > + required:
> > + - ports
> > +
> > +unevaluatedProperties: false
> >
> > examples:
> > - |
> > @@ -120,3 +180,24 @@ examples:
> > };
> > };
> > };
> > +
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> > + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
> > +
> > + display@40260000 {
> > + compatible = "verisilicon,dc";
> > + reg = <0x40260000 0x20000>;
> > + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk DCU_GATE>, <&clk DCUP_DIV>;
> > + clock-names = "core", "pix0";
> > + resets = <&sys MA35D1_RESET_DISP>;
> > + reset-names = "core";
> > +
> > + port {
> > + dpi_out: endpoint {
> > + remote-endpoint = <&panel_in>;
> > + };
> > + };
> > + };
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-05-19 16:47 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 5:51 [PATCH v2 0/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
2026-05-19 5:51 ` [PATCH v2 1/4] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
2026-05-19 6:15 ` sashiko-bot
2026-05-19 7:26 ` [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: " Icenowy Zheng
2026-05-19 16:47 ` Conor Dooley
2026-05-19 5:51 ` [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity Joey Lu
2026-05-19 6:25 ` sashiko-bot
2026-05-19 7:37 ` Icenowy Zheng
2026-05-19 5:51 ` [PATCH v2 3/4] drm/verisilicon: introduce per-variant hardware ops table Joey Lu
2026-05-19 6:41 ` sashiko-bot
2026-05-19 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
2026-05-19 7:17 ` sashiko-bot
2026-05-19 7:44 ` Icenowy Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox