* [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; 14+ 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] 14+ 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 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, 1 reply; 14+ 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] 14+ 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 7:26 ` Icenowy Zheng 2026-05-19 16:47 ` Conor Dooley 0 siblings, 1 reply; 14+ 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] 14+ 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 2026-05-20 3:06 ` Joey Lu 0 siblings, 1 reply; 14+ 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] 14+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: generalize for single-output variants 2026-05-19 16:47 ` Conor Dooley @ 2026-05-20 3:06 ` Joey Lu 2026-05-20 4:07 ` Icenowy Zheng 0 siblings, 1 reply; 14+ messages in thread From: Joey Lu @ 2026-05-20 3:06 UTC (permalink / raw) To: Conor Dooley, Icenowy Zheng Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel, linux-kernel On 5/20/2026 12:47 AM, Conor Dooley wrote: > 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 Understood. I will add `nuvoton,ma35d1-dcu` as the SoC-specific compatible string paired with `verisilicon,dc` as the generic fallback, matching the pattern used for `thead,th1520-dc8200`. The standalone `verisilicon,dc` compatible will be removed from the binding. The driver match table is not changed since hardware detection is done via ID registers. >>> - - 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). I will add a description to the `port` property noting that endpoint 0 is used for DPI output, which is the only output type for DCUltraLite. >>> >>> 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 Understood. I will remove all description strings from the if/else branches; the if/then clauses will only constrain clock-names and reset-names items (name values only, no descriptions). Regarding clock naming: DCU_GATE on MA35D1 is a peripheral gate clock without a separate dedicated core working clock, so I will keep "core" as the name and drop the misleading description "Bus clock that gates register access". The description mismatch was entirely in the if/else strings which are now removed. Thanks. >> >>> + >>> + 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] 14+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: generalize for single-output variants 2026-05-20 3:06 ` Joey Lu @ 2026-05-20 4:07 ` Icenowy Zheng 2026-05-21 5:41 ` Joey Lu 0 siblings, 1 reply; 14+ messages in thread From: Icenowy Zheng @ 2026-05-20 4:07 UTC (permalink / raw) To: Joey Lu, Conor Dooley Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel, linux-kernel 在 2026-05-20三的 11:06 +0800，Joey Lu写道： > On 5/20/2026 12:47 AM, Conor Dooley wrote: > > 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 > Understood. I will add `nuvoton,ma35d1-dcu` as the SoC-specific > compatible string paired with `verisilicon,dc` as the generic > fallback, > matching the pattern used for `thead,th1520-dc8200`. The standalone > `verisilicon,dc` compatible will be removed from the binding. The > driver No, please don't remove compatible strings from existing binding, and the generic compatible is still used for driver binding. The SoC-specific compatible is informative here, it needs to exist, but it doesn't supersede "verisilicon,dc" . In addition, the SoC-specific compatible is also used for verification of the SoC device tree, which is the reason if clauses exist with compatible match and additional constraints (e.g. for the nuvoton DCU it's invalid to have a 2nd output port). > match table is not changed since hardware detection is done via ID > registers. > > > > - - 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). > I will add a description to the `port` property noting that endpoint > 0 > is used for DPI output, which is the only output type for > DCUltraLite. Please note that DC8000 exists, which is single-port but supports both DPI and DP. > > > > > > > > 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 > > Understood. I will remove all description strings from the if/else > branches; the if/then clauses will only constrain clock-names and > reset-names items (name values only, no descriptions). Regarding > clock Well I think a required properties list is also needed in the if/then clause, to prevent DT's from lacking properties. > naming: DCU_GATE on MA35D1 is a peripheral gate clock without a > separate > dedicated core working clock, so I will keep "core" as the name and Do you mean there's no seperate dedicated bus clock? I find that in the clock driver dcu_gate has no parent as bus clocks -- its parent is dcu_mux, and dcu_mux's 2 parents are both pll ("epll_div2" and "syspll"). Thanks, Icenowy > drop > the misleading description "Bus clock that gates register access". > The > description mismatch was entirely in the if/else strings which are > now > removed. > > Thanks. > > > > > > > > + > > > > + 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] 14+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: generalize for single-output variants 2026-05-20 4:07 ` Icenowy Zheng @ 2026-05-21 5:41 ` Joey Lu 0 siblings, 0 replies; 14+ messages in thread From: Joey Lu @ 2026-05-21 5:41 UTC (permalink / raw) To: Icenowy Zheng, Conor Dooley Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel, linux-kernel On 5/20/2026 12:07 PM, Icenowy Zheng wrote: > 在 2026-05-20三的 11:06 +0800，Joey Lu写道： >> On 5/20/2026 12:47 AM, Conor Dooley wrote: >>> 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 >> Understood. I will add `nuvoton,ma35d1-dcu` as the SoC-specific >> compatible string paired with `verisilicon,dc` as the generic >> fallback, >> matching the pattern used for `thead,th1520-dc8200`. The standalone >> `verisilicon,dc` compatible will be removed from the binding. The >> driver > No, please don't remove compatible strings from existing binding, and > the generic compatible is still used for driver binding. > > The SoC-specific compatible is informative here, it needs to exist, but > it doesn't supersede "verisilicon,dc" . > > In addition, the SoC-specific compatible is also used for verification > of the SoC device tree, which is the reason if clauses exist with > compatible match and additional constraints (e.g. for the nuvoton DCU > it's invalid to have a 2nd output port). Sorry for the misunderstanding. I now see that a standalone generic fallback compatible is not preferred here, and that the SoC-specific compatible is strictly required for DT validation. I will add `nuvoton,ma35d1-dcu` as the SoC-specific compatible string in the existing compatible items list, without adding or removing anything else. >> match table is not changed since hardware detection is done via ID >> registers. >>>>> - - 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). >> I will add a description to the `port` property noting that endpoint >> 0 >> is used for DPI output, which is the only output type for >> DCUltraLite. > Please note that DC8000 exists, which is single-port but supports both > DPI and DP. To make it simple, the `port` property will not be added. `ports` remains the sole port property and is kept in the global `required:` list as in the original. The MA35D1 example will use `ports { port@0 { ... } }`, consistent with how other single-output DT nodes are written in the kernel. >>>>> >>>>> 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 >> Understood. I will remove all description strings from the if/else >> branches; the if/then clauses will only constrain clock-names and >> reset-names items (name values only, no descriptions). Regarding >> clock > Well I think a required properties list is also needed in the if/then > clause, to prevent DT's from lacking properties. Since `ports` is kept in the global `required:` list, neither if/then block needs a `required:` entry for port topology. Each if/then only constrains clock-names and reset-names for DT validation. The `else` branch has been eliminated; each variant has its own independent `if/then` in the `allOf` array. >> naming: DCU_GATE on MA35D1 is a peripheral gate clock without a >> separate >> dedicated core working clock, so I will keep "core" as the name and > Do you mean there's no seperate dedicated bus clock? I find that in the > clock driver dcu_gate has no parent as bus clocks -- its parent is > dcu_mux, and dcu_mux's 2 parents are both pll ("epll_div2" and > "syspll"). > > Thanks, > Icenowy You are right — DCU_GATE has no parent as a bus clock. For this case, I prefer to keep "core" as the sole gate clock name alongside "pix0". Thanks. Here is what the v3 yaml would look like: ```yaml compatible: items: - enum: [nuvoton,ma35d1-dcu, thead,th1520-dc8200] - const: verisilicon,dc properties: clocks: minItems: 2, items with descriptions resets: minItems: 1, items with descriptions required: [compatible, reg, interrupts, clocks, clock-names, ports] allOf: - if: compatible contains thead,th1520-dc8200 then: clock-names: [core, axi, ahb, pix0, pix1] reset-names: [core, axi, ahb] - if: compatible contains nuvoton,ma35d1-dcu then: clock-names: [core, pix0] reset-names: [core] ``` >> drop >> the misleading description "Bus clock that gates register access". >> The >> description mismatch was entirely in the if/else strings which are >> now >> removed. >> >> Thanks. >> >>>>> + >>>>> + 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] 14+ 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 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, 1 reply; 14+ 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] 14+ 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 7:37 ` Icenowy Zheng 2026-05-20 3:08 ` Joey Lu 0 siblings, 1 reply; 14+ 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] 14+ messages in thread
* Re: [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity 2026-05-19 7:37 ` Icenowy Zheng @ 2026-05-20 3:08 ` Joey Lu 0 siblings, 0 replies; 14+ messages in thread From: Joey Lu @ 2026-05-20 3:08 UTC (permalink / raw) To: Icenowy Zheng, maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh, krzk+dt, conor+dt Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel, linux-kernel On 5/19/2026 3:37 PM, Icenowy Zheng wrote: > 在 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 Understood. I will split the register macro additions into a separate patch: one for the new vs_crtc_regs.h IRQ macro and one for the vs_primary_plane_regs.h FB_CONFIG bits, keeping them independent of the HWDB identity 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. Understood. I will remove `VSDC_MODEL_DC8200` and use the literal `0x8200` directly in vs_hwdb.c with a comment. >> .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. Understood. I will remove the `VSDC_MODEL_DCU_LITE` macro and use `0x0` directly with a `/* DCUltraLite */` comment in vs_hwdb.c. >> + .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 `VSDC_DISP_IRQ_VSYNC(n)` is a bit-mask for the IRQ status/enable registers (`VSDC_DISP_IRQ_STA` / `VSDC_DISP_IRQ_EN`) which already live in vs_crtc_regs.h. Keeping it there alongside the register addresses it operates on is cleaner than splitting the IRQ definitions across two headers. >> #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] 14+ 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 5:51 ` [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu 3 siblings, 0 replies; 14+ 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] 14+ 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:44 ` Icenowy Zheng 3 siblings, 1 reply; 14+ 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] 14+ 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:44 ` Icenowy Zheng 2026-05-20 3:09 ` Joey Lu 0 siblings, 1 reply; 14+ 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] 14+ messages in thread
* Re: [PATCH v2 4/4] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support 2026-05-19 7:44 ` Icenowy Zheng @ 2026-05-20 3:09 ` Joey Lu 0 siblings, 0 replies; 14+ messages in thread From: Joey Lu @ 2026-05-20 3:09 UTC (permalink / raw) To: Icenowy Zheng, maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh, krzk+dt, conor+dt Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel, linux-kernel On 5/19/2026 3:44 PM, Icenowy Zheng wrote: > 在 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 Understood. I will add `enum vs_dc_generation` to vs_hwdb.h and a `generation` field to `vs_chip_identity`. Each entry in `vs_chip_identities[]` will set `.generation` accordingly (DC8200 entries → `VSDC_GEN_DC8200`; DCUltraLite → `VSDC_GEN_DC8000`). The probe will then branch on `dc->identity.generation == VSDC_GEN_DC8200` instead of the model register value. >> + 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] 14+ messages in thread
end of thread, other threads:[~2026-05-21 5:41 UTC | newest] Thread overview: 14+ 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 7:26 ` [PATCH v2 1/4] dt-bindings: display: verisilicon, dc: " Icenowy Zheng 2026-05-19 16:47 ` Conor Dooley 2026-05-20 3:06 ` Joey Lu 2026-05-20 4:07 ` Icenowy Zheng 2026-05-21 5:41 ` 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 7:37 ` Icenowy Zheng 2026-05-20 3:08 ` Joey Lu 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 2026-05-19 7:44 ` Icenowy Zheng 2026-05-20 3:09 ` Joey Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox