[PATCH v3 0/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support

[PATCH v3 0/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support

[Joey Lu]

This series adds support for the Verisilicon DCUltraLite 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 DCUltraLite 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 DCUltraLite 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 DCUltraLite 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.

Patch 1 generalises the verisilicon,dc DT binding to accommodate the
Nuvoton MA35D1 SoC-specific compatible and the variant's two-clock,
one-reset, single-port topology.

Patch 2 adds the register-level macros needed by the DCUltraLite ops.

Patches 3-5 introduce the driver changes in three logical steps: the
vs_dc_funcs hardware ops vtable with DC8200 ops extracted into
vs_dc8200.c; the DCUltraLite ops in vs_dcu_lite.c with the necessary
Kconfig and clock-optionality changes; and finally the DCUltraLite HWDB
entry that gates hardware recognition once all support is in place.

All patches have been tested on Nuvoton MA35D1 hardware.

Changes from v2:
  - [dt-bindings] Replaced standalone verisilicon,dc compatible with the
    SoC-specific nuvoton,ma35d1-dcu added to the existing enum list,
    paired with verisilicon,dc as the generic fallback; this matches the
    thead,th1520-dc8200 pattern and was explicitly requested by the
    reviewer.
  - [dt-bindings] Removed standalone 'port' property; kept 'ports' in the
    global required list; MA35D1 example now uses ports/port@0 structure,
    following reviewer feedback that a 'port' alias should not be added
    since DC8000 (single-port) also supports DP output.
  - [dt-bindings] Replaced additionalProperties with unevaluatedProperties
    to allow per-variant if/then clauses to add constraints cleanly.
  - [dt-bindings] Added separate allOf/if block for nuvoton,ma35d1-dcu
    constraining clock-names to [core, pix0] and reset-names to [core];
    the if/else structure from v2 is replaced by two independent if blocks.
  - [dt-bindings] Removed all description strings from if/then branches per
    reviewer request; descriptions remain only in the top-level properties.
  - [hwdb] Removed VSDC_MODEL_DC8200 and VSDC_MODEL_DCU_LITE macros; HWDB
    entries use literal values (0x8200, 0x0) with inline comments.
  - [hwdb] Added enum vs_dc_generation (VSDC_GEN_DC8000 / VSDC_GEN_DC8200)
    and a generation field to vs_chip_identity; funcs dispatch now uses
    generation instead of the model register value, per reviewer suggestion
    (DC8000 has model 0x8000 yet behaves like DCUltraLite with model 0x0).
  - [hwdb] Moved the DCUltraLite HWDB entry to the final patch in the
    series per reviewer request, making it a gate that is opened only
    after all supporting code is in place.
  - [ops] Split v2 patch 2 into two patches: register macros first, then
    the per-variant ops table, per reviewer suggestion.
  - [ops] Extracted DC8200-specific ops into vs_dc8200.c; DCUltraLite ops
    are in vs_dcu_lite.c; dispatch in vs_dc_probe uses generation field.

Joey Lu (5):
  dt-bindings: display: verisilicon,dc: generalize for single-output
    variants
  drm/verisilicon: add register-level macros for DCU Lite
  drm/verisilicon: introduce per-variant hardware ops table
  drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller
    support
  drm/verisilicon: add DCUltraLite chip identity to HWDB

 .../bindings/display/verisilicon,dc.yaml      | 103 ++++++++++++++---
 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         |  14 +++
 drivers/gpu/drm/verisilicon/vs_hwdb.h         |   6 +
 .../gpu/drm/verisilicon/vs_primary_plane.c    |  32 +-----
 .../drm/verisilicon/vs_primary_plane_regs.h   |   3 +
 14 files changed, 385 insertions(+), 67 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

[PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Joey Lu]

The existing schema hard-codes the five-clock/three-reset/dual-port
topology of the DC8200 IP block, preventing reuse for single-output
variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
SoC.

Rework the schema so that variant-specific constraints are expressed via
allOf/if blocks:

- Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
  generic verisilicon,dc fallback remains the driver-binding string.
- Relax the top-level clocks/resets definitions to minItems ranges so
  the base schema accepts both variants.
- Keep ports in the global required list and keep additionalProperties
  tightened to unevaluatedProperties.
- Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
  ahb, pix0, pix1), three-reset (core, axi, ahb).
- Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
  one-reset (core).
- Fix a stray space in the port@0 description.
- Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++---
 1 file changed, 90 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
index 9dc35ab973f2..db0260d874c5 100644
--- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
+++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
@@ -17,7 +17,8 @@ properties:
     items:
       - enum:
           - thead,th1520-dc8200
-      - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
+          - nuvoton,ma35d1-dcu
+      - const: verisilicon,dc  # DC IPs have discoverable ID/revision registers
 
   reg:
     maxItems: 1
@@ -26,6 +27,7 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 2
     items:
       - description: DC Core clock
       - description: DMA AXI bus clock
@@ -34,24 +36,19 @@ properties:
       - description: Pixel clock of output 1
 
   clock-names:
-    items:
-      - const: core
-      - const: axi
-      - const: ahb
-      - const: pix0
-      - const: pix1
+    minItems: 2
+    maxItems: 5
 
   resets:
+    minItems: 1
     items:
       - description: DC Core reset
       - description: DMA AXI bus reset
       - description: Configuration AHB bus reset
 
   reset-names:
-    items:
-      - const: core
-      - const: axi
-      - const: ahb
+    minItems: 1
+    maxItems: 3
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -59,7 +56,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.
 
@@ -77,7 +74,60 @@ required:
   - clock-names
   - ports
 
-additionalProperties: false
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: thead,th1520-dc8200
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 5
+
+        clock-names:
+          items:
+            - const: core
+            - const: axi
+            - const: ahb
+            - const: pix0
+            - const: pix1
+
+        resets:
+          minItems: 3
+          maxItems: 3
+
+        reset-names:
+          items:
+            - const: core
+            - const: axi
+            - const: ahb
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: nuvoton,ma35d1-dcu
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+
+        clock-names:
+          items:
+            - const: core
+            - const: pix0
+
+        resets:
+          maxItems: 1
+
+        reset-names:
+          items:
+            - const: core
+
+unevaluatedProperties: false
 
 examples:
   - |
@@ -120,3 +170,30 @@ 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 = "nuvoton,ma35d1-dcu", "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";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                dpi_out: endpoint {
+                    remote-endpoint = <&panel_in>;
+                };
+            };
+        };
+    };
-- 
2.43.0

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The top-level `clocks` definition retains a fixed `items` array with descriptions that conflict with the Nuvoton single-output variant, erroneously documenting its second clock as `DMA AXI bus clock`.
- [Low] The schema does not strictly enforce the single-output variant constraint for `nuvoton,ma35d1-dcu`, allowing a device tree to incorrectly define an invalid `port@1`.
--

commit 5f08249fbf664513d5ae1c658d2a7ebfc4f0b03e
Author: Joey Lu <a0987203069@gmail.com>

dt-bindings: display: verisilicon,dc: generalize for single-output variants

The existing schema hard-codes the five-clock/three-reset/dual-port topology
of the DC8200 IP block. This commit reworks the schema to express variant
specific constraints via allOf/if blocks, allowing reuse for single-output
variants like the Verisilicon DCUltraLite used in the Nuvoton MA35D1 SoC.

> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f20..db0260d874c53 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml

[ ... ]

> @@ -26,6 +27,7 @@ properties:
>      maxItems: 1
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: DC Core clock
>        - description: DMA AXI bus clock
> @@ -34,24 +36,19 @@ properties:

[Severity: Low]
Since the top-level clocks property retains a fixed items array, does this
cause the Nuvoton variant's second clock to incorrectly inherit the description
"DMA AXI bus clock" instead of its actual "pix0" clock?

Should the items array for clocks be moved to the per-variant if/then blocks
(similar to how clock-names is handled) so that the hardware interface is
documented accurately for each variant?

[ ... ]

> -additionalProperties: false
> +allOf:

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nuvoton,ma35d1-dcu
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: pix0
> +
> +        resets:
> +          maxItems: 1
> +
> +        reset-names:
> +          items:
> +            - const: core

[Severity: Low]
Because unevaluatedProperties: false operates globally, any property defined at
the top level (such as port@1 in the global ports object) is permitted for all
variants unless explicitly disallowed.

Since nuvoton,ma35d1-dcu is a single-output variant, should this block
explicitly forbid port@1 (e.g., by adding ports: properties: port@1: false) so
that device trees with invalid multiple ports cannot silently pass schema
validation?

> +
> +unevaluatedProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608023237.305036-1-a0987203069@gmail.com?part=1

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Icenowy Zheng]

在 2026-06-08一的 10:32 +0800，Joey Lu写道：
> The existing schema hard-codes the five-clock/three-reset/dual-port
> topology of the DC8200 IP block, preventing reuse for single-output
> variants such as the Verisilicon DCUltraLite used in the Nuvoton
> MA35D1
> SoC.
> 
> Rework the schema so that variant-specific constraints are expressed
> via
> allOf/if blocks:
> 
> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>   generic verisilicon,dc fallback remains the driver-binding string.
> - Relax the top-level clocks/resets definitions to minItems ranges so
>   the base schema accepts both variants.
> - Keep ports in the global required list and keep
> additionalProperties
>   tightened to unevaluatedProperties.
> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core,
> axi,
>   ahb, pix0, pix1), three-reset (core, axi, ahb).
> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core,
> pix0),
>   one-reset (core).
> - Fix a stray space in the port@0 description.
> - Add a DT example for the Nuvoton MA35D1 DCU Lite using
> ports/port@0.
> 
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
>  .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++-
> --
>  1 file changed, 90 insertions(+), 13 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f2..db0260d874c5 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> @@ -17,7 +17,8 @@ properties:
>      items:
>        - enum:
>            - thead,th1520-dc8200
> -      - const: verisilicon,dc # DC IPs have discoverable ID/revision
> registers
> +          - nuvoton,ma35d1-dcu
> +      - const: verisilicon,dc  # DC IPs have discoverable
> ID/revision registers

Ah is an extra space added here, which leads to this hunk looking
strange?

>  
>    reg:
>      maxItems: 1
> @@ -26,6 +27,7 @@ properties:
>      maxItems: 1
>  
>    clocks:
> +    minItems: 2

Maybe restrictions about the clock count shouldn't be inserted here,
and technically it's possible that only the pixel clock is controllable
by Linux (all other clocks are in a fixed configuration).

>      items:
>        - description: DC Core clock
>        - description: DMA AXI bus clock
> @@ -34,24 +36,19 @@ properties:
>        - description: Pixel clock of output 1
>  
>    clock-names:
> -    items:
> -      - const: core
> -      - const: axi
> -      - const: ahb
> -      - const: pix0
> -      - const: pix1

Ah I think the total list should still appear here, and they should be
corresponding to the descriptions above?

> +    minItems: 2
> +    maxItems: 5
>  
>    resets:
> +    minItems: 1
>      items:
>        - description: DC Core reset
>        - description: DMA AXI bus reset
>        - description: Configuration AHB bus reset
>  
>    reset-names:
> -    items:
> -      - const: core
> -      - const: axi
> -      - const: ahb

Ditto here.

> +    minItems: 1
> +    maxItems: 3
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -59,7 +56,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.
>  
> @@ -77,7 +74,60 @@ required:
>    - clock-names
>    - ports
>  
> -additionalProperties: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: thead,th1520-dc8200
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 5
> +          maxItems: 5
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: axi
> +            - const: ahb
> +            - const: pix0
> +            - const: pix1
> +
> +        resets:
> +          minItems: 3
> +          maxItems: 3
> +
> +        reset-names:
> +          items:
> +            - const: core
> +            - const: axi
> +            - const: ahb
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nuvoton,ma35d1-dcu
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: pix0
> +
> +        resets:

Do we have minItems: 1 here? (The DT schema validator always has some
quirks that I fail to remember, so I am not sure.)

> +          maxItems: 1
> +
> +        reset-names:
> +          items:
> +            - const: core
> +

I think resets should be described as required in both device-specific
bindings.

Thanks,
Icenowy

> +unevaluatedProperties: false
>  
>  examples:
>    - |
> @@ -120,3 +170,30 @@ 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 = "nuvoton,ma35d1-dcu", "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";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                dpi_out: endpoint {
> +                    remote-endpoint = <&panel_in>;
> +                };
> +            };
> +        };
> +    };

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Joey Lu]

On 6/8/2026 2:32 PM, Icenowy Zheng wrote:
> 在 2026-06-08一的 10:32 +0800，Joey Lu写道：
>> The existing schema hard-codes the five-clock/three-reset/dual-port
>> topology of the DC8200 IP block, preventing reuse for single-output
>> variants such as the Verisilicon DCUltraLite used in the Nuvoton
>> MA35D1
>> SoC.
>>
>> Rework the schema so that variant-specific constraints are expressed
>> via
>> allOf/if blocks:
>>
>> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>>    generic verisilicon,dc fallback remains the driver-binding string.
>> - Relax the top-level clocks/resets definitions to minItems ranges so
>>    the base schema accepts both variants.
>> - Keep ports in the global required list and keep
>> additionalProperties
>>    tightened to unevaluatedProperties.
>> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core,
>> axi,
>>    ahb, pix0, pix1), three-reset (core, axi, ahb).
>> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core,
>> pix0),
>>    one-reset (core).
>> - Fix a stray space in the port@0 description.
>> - Add a DT example for the Nuvoton MA35D1 DCU Lite using
>> ports/port@0.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++-
>> --
>>   1 file changed, 90 insertions(+), 13 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> index 9dc35ab973f2..db0260d874c5 100644
>> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> @@ -17,7 +17,8 @@ properties:
>>       items:
>>         - enum:
>>             - thead,th1520-dc8200
>> -      - const: verisilicon,dc # DC IPs have discoverable ID/revision
>> registers
>> +          - nuvoton,ma35d1-dcu
>> +      - const: verisilicon,dc  # DC IPs have discoverable
>> ID/revision registers
> Ah is an extra space added here, which leads to this hunk looking
> strange?
The extra space was added because `yamllint` reports "too few spaces 
before comment" (warning: comments) when only one space precedes the 
`#`. However, since this constitutes an unrelated whitespace change that 
makes the diff harder to read, I will revert to the original 
single-space form to keep the patch clean.
>>   
>>     reg:
>>       maxItems: 1
>> @@ -26,6 +27,7 @@ properties:
>>       maxItems: 1
>>   
>>     clocks:
>> +    minItems: 2
> Maybe restrictions about the clock count shouldn't be inserted here,
> and technically it's possible that only the pixel clock is controllable
> by Linux (all other clocks are in a fixed configuration).
Understood. I will remove the per-variant clock items descriptions from 
the top-level `clocks:` section and move them into the respective 
allOf/if blocks. The top-level will only carry `minItems`/`maxItems` for 
schema validation range.
>>       items:
>>         - description: DC Core clock
>>         - description: DMA AXI bus clock
>> @@ -34,24 +36,19 @@ properties:
>>         - description: Pixel clock of output 1
>>   
>>     clock-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
>> -      - const: pix0
>> -      - const: pix1
> Ah I think the total list should still appear here, and they should be
> corresponding to the descriptions above?
Understood. I will restore the full items list for `clock-names` at the 
top level (all five entries: core, axi, ahb, pix0, pix1) and add 
`minItems` to make it flexible. Per-variant allOf blocks will only 
constrain with `minItems`/`maxItems`.
>> +    minItems: 2
>> +    maxItems: 5
>>   
>>     resets:
>> +    minItems: 1
>>       items:
>>         - description: DC Core reset
>>         - description: DMA AXI bus reset
>>         - description: Configuration AHB bus reset
>>   
>>     reset-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
> Ditto here.
Understood. I will restore the full items list for `reset-names` at the 
top level (core, axi, ahb) with `minItems`. Same pattern as clock-names.
>> +    minItems: 1
>> +    maxItems: 3
>>   
>>     ports:
>>       $ref: /schemas/graph.yaml#/properties/ports
>> @@ -59,7 +56,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.
>>   
>> @@ -77,7 +74,60 @@ required:
>>     - clock-names
>>     - ports
>>   
>> -additionalProperties: false
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: thead,th1520-dc8200
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 5
>> +          maxItems: 5
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +            - const: pix0
>> +            - const: pix1
>> +
>> +        resets:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>> +        reset-names:
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nuvoton,ma35d1-dcu
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 2
>> +          maxItems: 2
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: pix0
>> +
>> +        resets:
> Do we have minItems: 1 here? (The DT schema validator always has some
> quirks that I fail to remember, so I am not sure.)
Yes, I will add `minItems: 1` to `resets:` in the nuvoton block.
>> +          maxItems: 1
>> +
>> +        reset-names:
>> +          items:
>> +            - const: core
>> +
> I think resets should be described as required in both device-specific
> bindings.
>
> Thanks,
> Icenowy

Understood. I will add `required: [resets, reset-names]` inside the 
`then:` block for both thead,th1520-dc8200 and nuvoton,ma35d1-dcu.

Many thanks!

>> +unevaluatedProperties: false
>>   
>>   examples:
>>     - |
>> @@ -120,3 +170,30 @@ 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 = "nuvoton,ma35d1-dcu", "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";
>> +
>> +        ports {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            port@0 {
>> +                reg = <0>;
>> +                dpi_out: endpoint {
>> +                    remote-endpoint = <&panel_in>;
>> +                };
>> +            };
>> +        };
>> +    };

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Krzysztof Kozlowski]

On Mon, Jun 08, 2026 at 10:32:33AM +0800, Joey Lu wrote:
> The existing schema hard-codes the five-clock/three-reset/dual-port
> topology of the DC8200 IP block, preventing reuse for single-output
> variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
> SoC.
> 
> Rework the schema so that variant-specific constraints are expressed via
> allOf/if blocks:
> 
> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>   generic verisilicon,dc fallback remains the driver-binding string.
> - Relax the top-level clocks/resets definitions to minItems ranges so
>   the base schema accepts both variants.
> - Keep ports in the global required list and keep additionalProperties
>   tightened to unevaluatedProperties.
> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
>   ahb, pix0, pix1), three-reset (core, axi, ahb).
> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
>   one-reset (core).
> - Fix a stray space in the port@0 description.
> - Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.
> 
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
>  .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++---
>  1 file changed, 90 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f2..db0260d874c5 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> @@ -17,7 +17,8 @@ properties:
>      items:
>        - enum:
>            - thead,th1520-dc8200
> -      - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
> +          - nuvoton,ma35d1-dcu
> +      - const: verisilicon,dc  # DC IPs have discoverable ID/revision registers

Why do you need to change indentation? Why introducing irrelevant
changes to the diff?

>  
>    reg:
>      maxItems: 1
> @@ -26,6 +27,7 @@ properties:
>      maxItems: 1
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: DC Core clock
>        - description: DMA AXI bus clock

That's not true anymore. In such case the list should also be defined
per variant and here only min/maxItems.


> @@ -34,24 +36,19 @@ properties:
>        - description: Pixel clock of output 1
>  
>    clock-names:
> -    items:
> -      - const: core
> -      - const: axi
> -      - const: ahb
> -      - const: pix0
> -      - const: pix1
> +    minItems: 2
> +    maxItems: 5
>  
>    resets:
> +    minItems: 1
>      items:
>        - description: DC Core reset
>        - description: DMA AXI bus reset
>        - description: Configuration AHB bus reset
>  
>    reset-names:
> -    items:
> -      - const: core
> -      - const: axi
> -      - const: ahb

This stays, with minItems. Variants only need min/maxItems



> +    minItems: 1
> +    maxItems: 3
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -59,7 +56,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.
>  
> @@ -77,7 +74,60 @@ required:
>    - clock-names
>    - ports
>  
> -additionalProperties: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: thead,th1520-dc8200
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 5
> +          maxItems: 5
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: axi
> +            - const: ahb
> +            - const: pix0
> +            - const: pix1
> +
> +        resets:
> +          minItems: 3
> +          maxItems: 3
> +
> +        reset-names:

minItems: 3

> +          items:
> +            - const: core
> +            - const: axi
> +            - const: ahb
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nuvoton,ma35d1-dcu
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: pix0
> +
> +        resets:
> +          maxItems: 1
> +
> +        reset-names:

maxItems: 1

> +          items:
> +            - const: core
> +
> +unevaluatedProperties: false

Stop making random changes to the binding.

Best regards,
Krzysztof

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Joey Lu]

On 6/8/2026 4:00 PM, Krzysztof Kozlowski wrote:
> On Mon, Jun 08, 2026 at 10:32:33AM +0800, Joey Lu wrote:
>> The existing schema hard-codes the five-clock/three-reset/dual-port
>> topology of the DC8200 IP block, preventing reuse for single-output
>> variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
>> SoC.
>>
>> Rework the schema so that variant-specific constraints are expressed via
>> allOf/if blocks:
>>
>> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>>    generic verisilicon,dc fallback remains the driver-binding string.
>> - Relax the top-level clocks/resets definitions to minItems ranges so
>>    the base schema accepts both variants.
>> - Keep ports in the global required list and keep additionalProperties
>>    tightened to unevaluatedProperties.
>> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
>>    ahb, pix0, pix1), three-reset (core, axi, ahb).
>> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
>>    one-reset (core).
>> - Fix a stray space in the port@0 description.
>> - Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++---
>>   1 file changed, 90 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> index 9dc35ab973f2..db0260d874c5 100644
>> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> @@ -17,7 +17,8 @@ properties:
>>       items:
>>         - enum:
>>             - thead,th1520-dc8200
>> -      - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
>> +          - nuvoton,ma35d1-dcu
>> +      - const: verisilicon,dc  # DC IPs have discoverable ID/revision registers
> Why do you need to change indentation? Why introducing irrelevant
> changes to the diff?
The extra space was introduced to satisfy `yamllint`'s "too few spaces 
before comment" warning, which requires two spaces before an inline `#`. 
Since this is an unrelated change that pollutes the diff, I will revert 
it to the original single-space form.
>>   
>>     reg:
>>       maxItems: 1
>> @@ -26,6 +27,7 @@ properties:
>>       maxItems: 1
>>   
>>     clocks:
>> +    minItems: 2
>>       items:
>>         - description: DC Core clock
>>         - description: DMA AXI bus clock
> That's not true anymore. In such case the list should also be defined
> per variant and here only min/maxItems.
>
Understood. I will remove the `items:` description list from the 
top-level `clocks:` and keep only `minItems`/`maxItems`. The per-variant 
items descriptions will be moved into the allOf/if blocks.
>> @@ -34,24 +36,19 @@ properties:
>>         - description: Pixel clock of output 1
>>   
>>     clock-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
>> -      - const: pix0
>> -      - const: pix1
>> +    minItems: 2
>> +    maxItems: 5
>>   
>>     resets:
>> +    minItems: 1
>>       items:
>>         - description: DC Core reset
>>         - description: DMA AXI bus reset
>>         - description: Configuration AHB bus reset
>>   
>>     reset-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
> This stays, with minItems. Variants only need min/maxItems
>
Understood. I will restore the top-level `clock-names` and `reset-names` 
items lists and add `minItems` to each. The per-variant allOf blocks 
will only carry `minItems`/`maxItems`.
>
>> +    minItems: 1
>> +    maxItems: 3
>>   
>>     ports:
>>       $ref: /schemas/graph.yaml#/properties/ports
>> @@ -59,7 +56,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.
>>   
>> @@ -77,7 +74,60 @@ required:
>>     - clock-names
>>     - ports
>>   
>> -additionalProperties: false
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: thead,th1520-dc8200
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 5
>> +          maxItems: 5
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +            - const: pix0
>> +            - const: pix1
>> +
>> +        resets:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>> +        reset-names:
> minItems: 3
Understood. I will add `minItems: 3` to `reset-names` in the 
thead,th1520-dc8200 block.
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nuvoton,ma35d1-dcu
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 2
>> +          maxItems: 2
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: pix0
>> +
>> +        resets:
>> +          maxItems: 1
>> +
>> +        reset-names:
> maxItems: 1
Understood. I will add `maxItems: 1` to `reset-names` in the nuvoton block.
>> +          items:
>> +            - const: core
>> +
>> +unevaluatedProperties: false
> Stop making random changes to the binding.
>
> Best regards,
> Krzysztof

Understood. I will revert to `additionalProperties: false` as in the 
original binding.

Many thanks!

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Krzysztof Kozlowski]

On Mon, Jun 08, 2026 at 10:32:33AM +0800, Joey Lu wrote:
> The existing schema hard-codes the five-clock/three-reset/dual-port
> topology of the DC8200 IP block, preventing reuse for single-output
> variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
> SoC.
> 
> Rework the schema so that variant-specific constraints are expressed via
> allOf/if blocks:
> 
> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>   generic verisilicon,dc fallback remains the driver-binding string.
> - Relax the top-level clocks/resets definitions to minItems ranges so
>   the base schema accepts both variants.
> - Keep ports in the global required list and keep additionalProperties
>   tightened to unevaluatedProperties.
> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
>   ahb, pix0, pix1), three-reset (core, axi, ahb).
> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
>   one-reset (core).
> - Fix a stray space in the port@0 description.
> - Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.

Difference in clocks and resets does not need a new new example.

Best regards,
Krzysztof

Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Joey Lu]

On 6/8/2026 4:02 PM, Krzysztof Kozlowski wrote:
> On Mon, Jun 08, 2026 at 10:32:33AM +0800, Joey Lu wrote:
>> The existing schema hard-codes the five-clock/three-reset/dual-port
>> topology of the DC8200 IP block, preventing reuse for single-output
>> variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
>> SoC.
>>
>> Rework the schema so that variant-specific constraints are expressed via
>> allOf/if blocks:
>>
>> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>>    generic verisilicon,dc fallback remains the driver-binding string.
>> - Relax the top-level clocks/resets definitions to minItems ranges so
>>    the base schema accepts both variants.
>> - Keep ports in the global required list and keep additionalProperties
>>    tightened to unevaluatedProperties.
>> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
>>    ahb, pix0, pix1), three-reset (core, axi, ahb).
>> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
>>    one-reset (core).
>> - Fix a stray space in the port@0 description.
>> - Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.
> Difference in clocks and resets does not need a new new example.
>
> Best regards,
> Krzysztof
Understood. I will remove the second example for nuvoton,ma35d1-dcu from 
the binding.

[PATCH v3 2/5] drm/verisilicon: add register-level macros for DCU Lite

[Joey Lu]

Add register-level constants needed by the forthcoming DCUltraLite
hardware ops:

  VSDC_DISP_IRQ_VSYNC(n) in vs_crtc_regs.h: bit mask for per-output
  VSYNC interrupt bits in DISP_IRQ_STA (0x147C) / DISP_IRQ_EN (0x1480),
  which are the IRQ registers used by DCUltraLite in place of the DC8200
  TOP_IRQ_ACK / TOP_IRQ_EN registers.

  VSDC_FB_CONFIG_ENABLE (bit 0), VSDC_FB_CONFIG_VALID (bit 3) and
  VSDC_FB_CONFIG_RESET (bit 4) in vs_primary_plane_regs.h: control bits
  in the FB_CONFIG register used by DCUltraLite for framebuffer enable
  and per-frame commit handshake.

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_primary_plane_regs.h | 3 +++
 2 files changed, 4 insertions(+)

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_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

[PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[Joey Lu]

The DC8200 and DCUltraLite share a broadly similar register layout but
differ in how the bridge, CRTC, primary plane and IRQ paths are driven.
Introduce a vs_dc_funcs vtable so each variant can supply its own
implementation without scattering conditionals across multiple files.

Add enum vs_dc_generation (VSDC_GEN_DC8000 / VSDC_GEN_DC8200) to
vs_hwdb.h and a generation field to struct vs_chip_identity.  Annotate
all four existing DC8200 HWDB entries with VSDC_GEN_DC8200.

Extract the DC8200-specific hardware ops into a new vs_dc8200.c:
  bridge_enable / bridge_disable - PANEL_CONFIG/START + CONFIG_EX commit
  enable_vblank / disable_vblank - TOP_IRQ_EN VSYNC bit
  plane_enable_ex / disable_ex / update_ex - FB_CONFIG_EX path
  irq_handler - reads TOP_IRQ_ACK

Update vs_bridge.c, vs_crtc.c, vs_primary_plane.c and vs_dc.c to
dispatch through dc->funcs instead of directly touching registers.
vs_crtc.c gains atomic_begin and atomic_flush hooks to allow variants
to gate per-frame commit cycles.

No behaviour change for existing 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           |  33 ++++++
 drivers/gpu/drm/verisilicon/vs_dc8200.c       | 107 ++++++++++++++++++
 drivers/gpu/drm/verisilicon/vs_hwdb.c         |   4 +
 drivers/gpu/drm/verisilicon/vs_hwdb.h         |   6 +
 .../gpu/drm/verisilicon/vs_primary_plane.c    |  32 +-----
 9 files changed, 197 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 426f4bcaa834..9d4cd16452fa 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -1,6 +1,6 @@
 # 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 \
+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 vs_cursor_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 0b8a35d09cd2..679d6541ba1b 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..d77d4a1babdf 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,10 @@ 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;
+extern const struct vs_dc_funcs vs_dcu_lite_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_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
index 2a0f7c59afa3..91524d16f778 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
@@ -94,6 +94,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
 		.revision = 0x5720,
 		.customer_id = ~0U,
 
+		.generation = VSDC_GEN_DC8200,
 		.display_count = 2,
 		.max_cursor_size = 64,
 		.formats = &vs_formats_no_yuv444,
@@ -103,6 +104,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
 		.revision = 0x5721,
 		.customer_id = 0x30B,
 
+		.generation = VSDC_GEN_DC8200,
 		.display_count = 2,
 		.max_cursor_size = 64,
 		.formats = &vs_formats_no_yuv444,
@@ -112,6 +114,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
 		.revision = 0x5720,
 		.customer_id = 0x310,
 
+		.generation = VSDC_GEN_DC8200,
 		.display_count = 2,
 		.max_cursor_size = 64,
 		.formats = &vs_formats_with_yuv444,
@@ -121,6 +124,7 @@ static struct vs_chip_identity vs_chip_identities[] = {
 		.revision = 0x5720,
 		.customer_id = 0x311,
 
+		.generation = VSDC_GEN_DC8200,
 		.display_count = 2,
 		.max_cursor_size = 64,
 		.formats = &vs_formats_no_yuv444,
diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h b/drivers/gpu/drm/verisilicon/vs_hwdb.h
index 2065ecb73043..a15c8b565604 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
@@ -9,6 +9,11 @@
 #include <linux/regmap.h>
 #include <linux/types.h>
 
+enum vs_dc_generation {
+	VSDC_GEN_DC8000,
+	VSDC_GEN_DC8200,
+};
+
 struct vs_formats {
 	const u32 *array;
 	unsigned int num;
@@ -19,6 +24,7 @@ struct vs_chip_identity {
 	u32 revision;
 	u32 customer_id;
 
+	enum vs_dc_generation generation;
 	u32 display_count;
 	/*
 	 * The hardware only supports square cursor planes, so this field
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

Re: [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[sashiko-bot]

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

New issues:
- [High] The disable sequence in `vs_dc8200_bridge_disable` reverses the order of clearing `VSDC_DISP_PANEL_START` and `VSDC_DISP_PANEL_CONFIG`.
- [High] Incorrect teardown sequence in CRTC disable path where pixel clock is disabled before calling the CRTC hardware disable hook.

Pre-existing issues:
- [High] The plane disable function incorrectly sets the enable bit instead of clearing it.
- [High] Calling `clk_prepare_enable()` and `clk_disable_unprepare()` in the CRTC atomic enable/disable callbacks violates atomic context rules.
- [High] Potential NULL pointer dereference if IRQ fires before drm_dev is initialized.
--

commit 7843af23baf865801f80d2910ca556ff5e144ece
Author: Joey Lu <a0987203069@gmail.com>
Subject: drm/verisilicon: introduce per-variant hardware ops table

The DC8200 and DCUltraLite share a broadly similar register layout but
differ in how the bridge, CRTC, primary plane and IRQ paths are driven.
Introduce a vs_dc_funcs vtable so each variant can supply its own
implementation without scattering conditionals across multiple files.

> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c b/drivers/gpu/drm/verisilicon/vs_crtc.c
> index 0b8a35d09cd2e..679d6541ba1b9 100644
> --- a/drivers/gpu/drm/verisilicon/vs_crtc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
[ ... ]
> @@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
>  	drm_crtc_vblank_off(crtc);
>  
>  	clk_disable_unprepare(dc->pix_clk[output]);

[Severity: High]
This is a pre-existing issue, but does calling clk_disable_unprepare() here
violate atomic context rules?

The DRM subsystem requires that CRTC atomic disable callbacks run in atomic
context where sleeping is forbidden, but clk_disable_unprepare() can sleep.

> +
> +	if (dc->funcs->crtc_disable)
> +		dc->funcs->crtc_disable(dc, output);
>  }

[Severity: High]
Could disabling the pixel clock before invoking the hardware-specific
crtc_disable callback cause bus hangs?

If the crtc_disable hook requires the pixel clock to be active to perform
register writes, turning it off first might result in dropped writes or
stalls. Should the clock be disabled after the hook is called, reversing
the setup sequence?

>  
>  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]));

[Severity: High]
This is a pre-existing issue, but does this clk_prepare_enable() call also
violate atomic context rules? 

Since atomic_enable runs in atomic context, calling sleeping functions can
cause kernel warnings or potential deadlocks during non-blocking commits.

>  
> +	if (dc->funcs->crtc_enable)
> +		dc->funcs->crtc_enable(dc, output);
> +
>  	drm_crtc_vblank_on(crtc);
>  }
>  
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c
> index dad9967bc10b8..c949570241890 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -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);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the IRQ to
fire before the DRM device is fully initialized?

If the bootloader leaves the display hardware active and generating VSYNC
interrupts, vs_dc_irq_handler() could execute and call vs_drm_handle_irq(),
which accesses dc->drm_dev. Since devm_request_irq() is called in
vs_dc_probe() before vs_drm_initialize() allocates dc->drm_dev, could
this lead to a NULL pointer dereference during early boot?

[ ... ]
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> new file mode 100644
> index 0000000000000..db9e1b3cd9032
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
[ ... ]
> +static void vs_dc8200_bridge_disable(struct vs_dc *dc, unsigned int output)
> +{
> +	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> +			  VSDC_DISP_PANEL_CONFIG_RUNNING);
> +	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> +			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
> +			  VSDC_DISP_PANEL_START_RUNNING(output));

[Severity: High]
Does this reversal of the clearing sequence cause bus stalls or visual tearing?

In the original code, VSDC_DISP_PANEL_START was cleared before
VSDC_DISP_PANEL_CONFIG. This refactored sequence disables the panel
configuration while the pipeline is potentially still active. Should
VSDC_DISP_PANEL_START be cleared first to safely stop the pipeline?

[ ... ]
> +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);
> +}

[Severity: High]
This is a pre-existing issue carried over from the original code, but
does this actually disable the plane?

By using regmap_set_bits() with VSDC_FB_CONFIG_EX_FB_EN, the frame buffer
enable bit is being set to 1 instead of being cleared. Should this use
regmap_clear_bits() instead to properly disable the hardware plane?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608023237.305036-1-a0987203069@gmail.com?part=3

Re: [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[Icenowy Zheng]

在 2026-06-08一的 10:32 +0800，Joey Lu写道：
> The DC8200 and DCUltraLite share a broadly similar register layout
> but
> differ in how the bridge, CRTC, primary plane and IRQ paths are
> driven.
> Introduce a vs_dc_funcs vtable so each variant can supply its own
> implementation without scattering conditionals across multiple files.
> 
> Add enum vs_dc_generation (VSDC_GEN_DC8000 / VSDC_GEN_DC8200) to
> vs_hwdb.h and a generation field to struct vs_chip_identity. 
> Annotate
> all four existing DC8200 HWDB entries with VSDC_GEN_DC8200.
> 
> Extract the DC8200-specific hardware ops into a new vs_dc8200.c:
>   bridge_enable / bridge_disable - PANEL_CONFIG/START + CONFIG_EX
> commit
>   enable_vblank / disable_vblank - TOP_IRQ_EN VSYNC bit
>   plane_enable_ex / disable_ex / update_ex - FB_CONFIG_EX path
>   irq_handler - reads TOP_IRQ_ACK
> 
> Update vs_bridge.c, vs_crtc.c, vs_primary_plane.c and vs_dc.c to
> dispatch through dc->funcs instead of directly touching registers.
> vs_crtc.c gains atomic_begin and atomic_flush hooks to allow variants
> to gate per-frame commit cycles.
> 
> No behaviour change for existing 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           |  33 ++++++
>  drivers/gpu/drm/verisilicon/vs_dc8200.c       | 107
> ++++++++++++++++++
>  drivers/gpu/drm/verisilicon/vs_hwdb.c         |   4 +
>  drivers/gpu/drm/verisilicon/vs_hwdb.h         |   6 +
>  .../gpu/drm/verisilicon/vs_primary_plane.c    |  32 +-----
>  9 files changed, 197 insertions(+), 51 deletions(-)
>  create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
============ 8< ==================
> 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);

The code here being called "bridge" is only internal to kernel. Naming
it in such a way is okay, but maybe naming it "panel" is better
(because they're configuring PANEL-named registers).

And, as the common code setting common fields of DcregPanelConfig0 is
still here, maybe the helper name should be named "panel_enable_ex" (or
"bridge_enable_ex") ?

>  }
>  
>  static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
====== 8< ==============
> 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);

The IRQ isn't handled in this helper.

So maybe call it "irq_ack"?

>  
>  	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..d77d4a1babdf 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);

See my comments elsewhere for the helper naming.

> +};
>  
>  struct vs_dc {
>  	struct regmap *regs;
============= 8< =================
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 2a0f7c59afa3..91524d16f778 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -94,6 +94,7 @@ static struct vs_chip_identity vs_chip_identities[]
> = {
>  		.revision = 0x5720,
>  		.customer_id = ~0U,
>  
> +		.generation = VSDC_GEN_DC8200,
>  		.display_count = 2,
>  		.max_cursor_size = 64,
>  		.formats = &vs_formats_no_yuv444,
> @@ -103,6 +104,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
>  		.revision = 0x5721,
>  		.customer_id = 0x30B,
>  
> +		.generation = VSDC_GEN_DC8200,
>  		.display_count = 2,
>  		.max_cursor_size = 64,
>  		.formats = &vs_formats_no_yuv444,
> @@ -112,6 +114,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
>  		.revision = 0x5720,
>  		.customer_id = 0x310,
>  
> +		.generation = VSDC_GEN_DC8200,
>  		.display_count = 2,
>  		.max_cursor_size = 64,
>  		.formats = &vs_formats_with_yuv444,
> @@ -121,6 +124,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
>  		.revision = 0x5720,
>  		.customer_id = 0x311,
>  
> +		.generation = VSDC_GEN_DC8200,
>  		.display_count = 2,
>  		.max_cursor_size = 64,
>  		.formats = &vs_formats_no_yuv444,
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> index 2065ecb73043..a15c8b565604 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> @@ -9,6 +9,11 @@
>  #include <linux/regmap.h>
>  #include <linux/types.h>
>  
> +enum vs_dc_generation {
> +	VSDC_GEN_DC8000,
> +	VSDC_GEN_DC8200,
> +};
> +
>  struct vs_formats {
>  	const u32 *array;
>  	unsigned int num;
> @@ -19,6 +24,7 @@ struct vs_chip_identity {
>  	u32 revision;
>  	u32 customer_id;
>  
> +	enum vs_dc_generation generation;
>  	u32 display_count;
>  	/*
>  	 * The hardware only supports square cursor planes, so this
> field
> 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);

Please note that all theae codes are for primary planes, maybe the
helper should be named mentioning primary. Overlay planes will need a
different codepath because they change different registers.

Thanks,
Icenowy

>  }
>  
>  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 = {

Re: [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[Joey Lu]

On 6/8/2026 2:24 PM, Icenowy Zheng wrote:
> 在 2026-06-08一的 10:32 +0800，Joey Lu写道：
>> The DC8200 and DCUltraLite share a broadly similar register layout
>> but
>> differ in how the bridge, CRTC, primary plane and IRQ paths are
>> driven.
>> Introduce a vs_dc_funcs vtable so each variant can supply its own
>> implementation without scattering conditionals across multiple files.
>>
>> Add enum vs_dc_generation (VSDC_GEN_DC8000 / VSDC_GEN_DC8200) to
>> vs_hwdb.h and a generation field to struct vs_chip_identity.
>> Annotate
>> all four existing DC8200 HWDB entries with VSDC_GEN_DC8200.
>>
>> Extract the DC8200-specific hardware ops into a new vs_dc8200.c:
>>    bridge_enable / bridge_disable - PANEL_CONFIG/START + CONFIG_EX
>> commit
>>    enable_vblank / disable_vblank - TOP_IRQ_EN VSYNC bit
>>    plane_enable_ex / disable_ex / update_ex - FB_CONFIG_EX path
>>    irq_handler - reads TOP_IRQ_ACK
>>
>> Update vs_bridge.c, vs_crtc.c, vs_primary_plane.c and vs_dc.c to
>> dispatch through dc->funcs instead of directly touching registers.
>> vs_crtc.c gains atomic_begin and atomic_flush hooks to allow variants
>> to gate per-frame commit cycles.
>>
>> No behaviour change for existing 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           |  33 ++++++
>>   drivers/gpu/drm/verisilicon/vs_dc8200.c       | 107
>> ++++++++++++++++++
>>   drivers/gpu/drm/verisilicon/vs_hwdb.c         |   4 +
>>   drivers/gpu/drm/verisilicon/vs_hwdb.h         |   6 +
>>   .../gpu/drm/verisilicon/vs_primary_plane.c    |  32 +-----
>>   9 files changed, 197 insertions(+), 51 deletions(-)
>>   create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
> ============ 8< ==================
>> 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);
> The code here being called "bridge" is only internal to kernel. Naming
> it in such a way is okay, but maybe naming it "panel" is better
> (because they're configuring PANEL-named registers).
>
> And, as the common code setting common fields of DcregPanelConfig0 is
> still here, maybe the helper name should be named "panel_enable_ex" (or
> "bridge_enable_ex") ?
Understood. I will rename `bridge_enable`/`bridge_disable` to 
`panel_enable_ex`/`panel_disable_ex` throughout: in `vs_dc_funcs`, 
`vs_dc8200.c`, `vs_dc8000.c`, and the call sites in `vs_bridge.c`.
>>   }
>>   
>>   static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
> ====== 8< ==============
>> 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);
> The IRQ isn't handled in this helper.
>
> So maybe call it "irq_ack"?
Understood. I will rename `irq_handler` to `irq_ack` in `vs_dc_funcs`, 
`vs_dc8200.c`, `vs_dc8000.c`, and the call site in `vs_dc.c`.
>>   
>>   	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..d77d4a1babdf 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);
> See my comments elsewhere for the helper naming.
Understood. I will rename all vtable members per the comments above: 
`panel_enable_ex`, `panel_disable_ex`, `primary_plane_enable`, 
`primary_plane_disable`, `primary_plane_update`, `irq_ack`.
>> +};
>>   
>>   struct vs_dc {
>>   	struct regmap *regs;
> ============= 8< =================
>> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> index 2a0f7c59afa3..91524d16f778 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> @@ -94,6 +94,7 @@ static struct vs_chip_identity vs_chip_identities[]
>> = {
>>   		.revision = 0x5720,
>>   		.customer_id = ~0U,
>>   
>> +		.generation = VSDC_GEN_DC8200,
>>   		.display_count = 2,
>>   		.max_cursor_size = 64,
>>   		.formats = &vs_formats_no_yuv444,
>> @@ -103,6 +104,7 @@ static struct vs_chip_identity
>> vs_chip_identities[] = {
>>   		.revision = 0x5721,
>>   		.customer_id = 0x30B,
>>   
>> +		.generation = VSDC_GEN_DC8200,
>>   		.display_count = 2,
>>   		.max_cursor_size = 64,
>>   		.formats = &vs_formats_no_yuv444,
>> @@ -112,6 +114,7 @@ static struct vs_chip_identity
>> vs_chip_identities[] = {
>>   		.revision = 0x5720,
>>   		.customer_id = 0x310,
>>   
>> +		.generation = VSDC_GEN_DC8200,
>>   		.display_count = 2,
>>   		.max_cursor_size = 64,
>>   		.formats = &vs_formats_with_yuv444,
>> @@ -121,6 +124,7 @@ static struct vs_chip_identity
>> vs_chip_identities[] = {
>>   		.revision = 0x5720,
>>   		.customer_id = 0x311,
>>   
>> +		.generation = VSDC_GEN_DC8200,
>>   		.display_count = 2,
>>   		.max_cursor_size = 64,
>>   		.formats = &vs_formats_no_yuv444,
>> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> index 2065ecb73043..a15c8b565604 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> @@ -9,6 +9,11 @@
>>   #include <linux/regmap.h>
>>   #include <linux/types.h>
>>   
>> +enum vs_dc_generation {
>> +	VSDC_GEN_DC8000,
>> +	VSDC_GEN_DC8200,
>> +};
>> +
>>   struct vs_formats {
>>   	const u32 *array;
>>   	unsigned int num;
>> @@ -19,6 +24,7 @@ struct vs_chip_identity {
>>   	u32 revision;
>>   	u32 customer_id;
>>   
>> +	enum vs_dc_generation generation;
>>   	u32 display_count;
>>   	/*
>>   	 * The hardware only supports square cursor planes, so this
>> field
>> 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);
> Please note that all theae codes are for primary planes, maybe the
> helper should be named mentioning primary. Overlay planes will need a
> different codepath because they change different registers.
>
> Thanks,
> Icenowy
Understood. To avoid confusion, I will rename `plane_enable_ex`, 
`plane_disable_ex`, and `plane_update_ex` to `primary_plane_enable`, 
`primary_plane_disable`, and `primary_plane_update` in `vs_dc_funcs`, 
`vs_dc8200.c`, and `vs_primary_plane.c`.
>>   }
>>   
>>   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 = {

Re: [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[Icenowy Zheng]

在 2026-06-08一的 17:45 +0800，Joey Lu写道：
> > > 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);
> > Please note that all theae codes are for primary planes, maybe the
> > helper should be named mentioning primary. Overlay planes will need
> > a
> > different codepath because they change different registers.
> > 
> > Thanks,
> > Icenowy
> Understood. To avoid confusion, I will rename `plane_enable_ex`, 
> `plane_disable_ex`, and `plane_update_ex` to `primary_plane_enable`, 
> `primary_plane_disable`, and `primary_plane_update` in `vs_dc_funcs`,
> `vs_dc8200.c`, and `vs_primary_plane.c`.

Maybe keep the `_ex` here as some operations is still on the common
codepath?

Thanks,
Icenowy

> > >   }
> > >   
> > >   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 = {

Re: [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table

[Joey Lu]

On 6/8/2026 6:06 PM, Icenowy Zheng wrote:
> 在 2026-06-08一的 17:45 +0800，Joey Lu写道：
>>>> 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);
>>> Please note that all theae codes are for primary planes, maybe the
>>> helper should be named mentioning primary. Overlay planes will need
>>> a
>>> different codepath because they change different registers.
>>>
>>> Thanks,
>>> Icenowy
>> Understood. To avoid confusion, I will rename `plane_enable_ex`,
>> `plane_disable_ex`, and `plane_update_ex` to `primary_plane_enable`,
>> `primary_plane_disable`, and `primary_plane_update` in `vs_dc_funcs`,
>> `vs_dc8200.c`, and `vs_primary_plane.c`.
> Maybe keep the `_ex` here as some operations is still on the common
> codepath?
>
> Thanks,
> Icenowy
Got it. I’ll keep the `_ex` suffix in place. That way it’s consistent 
with the common codepath operations, and the inline comments already 
makes the intent clear.
>>>>    }
>>>>    
>>>>    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 = {

[PATCH v3 4/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support

[Joey Lu]

The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltraLite display
controller whose register layout differs from the DC8200 in several
important ways:

1. No CONFIG_EX commit path: framebuffer updates use the 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; there is no multi-display sync start
   register.

3. Different IRQ registers: DCUltraLite 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: DCUltraLite 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 required.  Make axi_clk and
   ahb_clk optional (devm_clk_get_optional_enabled) so DCUltraLite
   nodes without those clocks are handled gracefully.

Add vs_dcu_lite.c implementing the vs_dc_funcs vtable for the above
differences.  The probe now selects vs_dcu_lite_funcs when the
identified generation is VSDC_GEN_DC8000 (DCUltraLite reads model 0x0,
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_dcu_lite.c | 78 +++++++++++++++++++++++
 4 files changed, 86 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 9d4cd16452fa..960af0861dfa 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -1,6 +1,6 @@
 # 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 \
+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 vs_cursor_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..81a8d9bf85bd 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.generation == VSDC_GEN_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_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

Re: [PATCH v3 4/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support

[sashiko-bot]

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

New issues:
- [High] The DCU Lite driver does not start the panel output because it fails to set the PANEL_CONFIG.RUNNING bit in the bridge enable path.
- [High] The DCU Lite driver does not implement a plane_disable_ex callback, breaking independent primary plane disable.
- [High] The pixel clock is disabled before the CRTC hardware is disabled, causing potential register access hangs.
- [High] The DCUltraLite IRQ handler fails to clear the interrupt status, leading to an interrupt storm.
- [Medium] The `irqs` variable in `vs_dcu_lite_irq_handler` may be used uninitialized if `regmap_read` fails.

Pre-existing issues:
- [High] The DC8200 plane disable function incorrectly sets the enable bit instead of clearing it.
- [High] The IRQ handler is registered before DRM initialization is complete and remains active after teardown, risking a NULL pointer dereference.
--

commit c0e566390dbc25177931f48aa8bb45a4670fc241
Author: Joey Lu <a0987203069@gmail.com>
Subject: drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support

This commit adds support for the Nuvoton MA35D1 DCUltraLite display
controller. It introduces vs_dcu_lite.c with a new vs_dc_funcs vtable and
adjusts the probe function to handle optional clocks.

> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
[ ... ]
> @@ -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.generation == VSDC_GEN_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");

[Severity: High]
This is a pre-existing issue, but I noticed a potential race during probe
and initialization later in vs_dc_probe().

vs_dc.c:vs_dc_probe() {
    ...
    ret = devm_request_irq(dev, irq, vs_dc_irq_handler, ...);
    ...
    ret = vs_drm_initialize(dc, pdev);
    ...
}

The IRQ handler is registered before vs_drm_initialize() completes. If an
interrupt fires early, vs_drm_handle_irq() will be invoked:

vs_drm.c:vs_drm_handle_irq() {
    ...
    if (dc->drm_dev->crtcs[i])
        drm_crtc_handle_vblank(&dc->drm_dev->crtcs[i]->base);
    ...
}

Could this cause a NULL pointer dereference since dc->drm_dev is not fully
set up until vs_drm_initialize() finishes?

> diff --git a/drivers/gpu/drm/verisilicon/vs_dcu_lite.c b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dcu_lite.c
> @@ -0,0 +1,78 @@
[ ... ]
> +static void vs_dcu_lite_bridge_enable(struct vs_dc *dc, unsigned int output)
> +{
> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> +			VSDC_FB_CONFIG_RESET);
> +}

[Severity: High]
The commit message notes that panel output starts when PANEL_CONFIG.RUNNING
is set. However, vs_dcu_lite_bridge_enable() only sets VSDC_FB_CONFIG_RESET
and does not appear to write to VSDC_DISP_PANEL_CONFIG.

Will the display output pipeline start successfully without setting the
RUNNING bit?

[ ... ]
> +static u32 vs_dcu_lite_irq_handler(struct vs_dc *dc)
> +{
> +	u32 irqs;
> +
> +	regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &irqs);
> +	return irqs;
> +}

[Severity: Medium]
If regmap_read() fails, the irqs variable remains uninitialized. Could this
result in returning a garbage value that gets processed as a bitmask of
triggered interrupts?

[Severity: High]
Also, vs_dcu_lite_irq_handler() reads the status register but does not seem
to perform a write operation to acknowledge or clear the pending interrupts.
Will this cause the IRQ line to remain asserted, potentially leading to an
interrupt storm?

> +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,
> +};

[Severity: High]
Looking at vs_primary_plane_atomic_disable() in vs_primary_plane.c, it
relies on plane_disable_ex() to disable the plane. Since vs_dcu_lite_funcs
lacks a plane_disable_ex callback, will the hardware keep scanning out the
old framebuffer if user-space disables the primary plane while keeping the
CRTC active?

[Severity: High]
This isn't a bug introduced by this patch, but since we are discussing plane
disable operations, I noticed an issue with the DC8200 equivalent.

vs_dc8200.c:vs_dc8200_plane_disable_ex() {
    regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
                    VSDC_FB_CONFIG_EX_FB_EN);
}

This function calls regmap_set_bits() with VSDC_FB_CONFIG_EX_FB_EN. Does
this incorrectly set the enable bit to 1 instead of clearing it, preventing
the primary plane from being properly disabled?

[Severity: High]
This is a pre-existing issue, but looking at how crtc_disable is used in
vs_crtc.c:vs_crtc_atomic_disable():

vs_crtc.c:vs_crtc_atomic_disable() {
    clk_disable_unprepare(dc->pix_clk[output]);

    if (dc->funcs->crtc_disable)
        dc->funcs->crtc_disable(dc, output);
    ...
}

The pixel clock is disabled before the crtc_disable hardware callback
accesses the registers. Could this cause bus faults or leave the hardware in
an invalid state since the pipeline's primary pixel clock is already off?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608023237.305036-1-a0987203069@gmail.com?part=4

Re: [PATCH v3 4/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support

[Icenowy Zheng]

在 2026-06-08一的 10:32 +0800，Joey Lu写道：
> The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltraLite display
> controller whose register layout differs from the DC8200 in several
> important ways:
> 
> 1. No CONFIG_EX commit path: framebuffer updates use the 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; there is no multi-display sync start
>    register.
> 
> 3. Different IRQ registers: DCUltraLite 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: DCUltraLite 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 required.  Make axi_clk and
>    ahb_clk optional (devm_clk_get_optional_enabled) so DCUltraLite
>    nodes without those clocks are handled gracefully.
> 
> Add vs_dcu_lite.c implementing the vs_dc_funcs vtable for the above

Nitpick: could you use vs_dc8000 to make things more aligned? (Although
I must admit that DCUltraLite is the first revision to be supported in
this codepath).

> differences.  The probe now selects vs_dcu_lite_funcs when the
> identified generation is VSDC_GEN_DC8000 (DCUltraLite reads model
> 0x0,
> revision 0x5560, customer_id 0x305).
> 
> Extend Kconfig to allow building on ARCH_MA35 platforms.

Maybe the Kconfig change could be in the last commit or a dedicated
commit before current ones? Because it's only meaningful after the HWDB
item is added.

Thanks,
Icenowy

> 
> 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_dcu_lite.c | 78
> +++++++++++++++++++++++
>  4 files changed, 86 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 9d4cd16452fa..960af0861dfa 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -1,6 +1,6 @@
>  # 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 \
> +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 vs_cursor_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..81a8d9bf85bd 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.generation == VSDC_GEN_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_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,
> +};

Re: [PATCH v3 4/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support

[Joey Lu]

On 6/8/2026 2:26 PM, Icenowy Zheng wrote:
> 在 2026-06-08一的 10:32 +0800，Joey Lu写道：
>> The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltraLite display
>> controller whose register layout differs from the DC8200 in several
>> important ways:
>>
>> 1. No CONFIG_EX commit path: framebuffer updates use the 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; there is no multi-display sync start
>>     register.
>>
>> 3. Different IRQ registers: DCUltraLite 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: DCUltraLite 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 required.  Make axi_clk and
>>     ahb_clk optional (devm_clk_get_optional_enabled) so DCUltraLite
>>     nodes without those clocks are handled gracefully.
>>
>> Add vs_dcu_lite.c implementing the vs_dc_funcs vtable for the above
> Nitpick: could you use vs_dc8000 to make things more aligned? (Although
> I must admit that DCUltraLite is the first revision to be supported in
> this codepath).
Understood. I will rename `vs_dcu_lite.c` to `vs_dc8000.c`, all internal 
functions from `vs_dcu_lite_*` to `vs_dc8000_*`, the exported symbol 
from `vs_dcu_lite_funcs` to `vs_dc8000_funcs`, and update the Makefile 
accordingly.
>> differences.  The probe now selects vs_dcu_lite_funcs when the
>> identified generation is VSDC_GEN_DC8000 (DCUltraLite reads model
>> 0x0,
>> revision 0x5560, customer_id 0x305).
>>
>> Extend Kconfig to allow building on ARCH_MA35 platforms.
> Maybe the Kconfig change could be in the last commit or a dedicated
> commit before current ones? Because it's only meaningful after the HWDB
> item is added.
>
> Thanks,
> Icenowy
Understood. The Kconfig change adding `ARCH_MA35` will be moved to a 
separate commit placed after the HWDB entry is added, or as the final 
commit in the series.
>> 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_dcu_lite.c | 78
>> +++++++++++++++++++++++
>>   4 files changed, 86 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 9d4cd16452fa..960af0861dfa 100644
>> --- a/drivers/gpu/drm/verisilicon/Makefile
>> +++ b/drivers/gpu/drm/verisilicon/Makefile
>> @@ -1,6 +1,6 @@
>>   # 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 \
>> +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 vs_cursor_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..81a8d9bf85bd 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.generation == VSDC_GEN_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_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,
>> +};

[PATCH v3 5/5] drm/verisilicon: add DCUltraLite chip identity to HWDB

[Joey Lu]

Register the Nuvoton MA35D1 DCUltraLite chip identity in
vs_chip_identities[]:
  model      = 0x0   (DCUltraLite; Verisilicon uses 0 for this IP)
  revision   = 0x5560
  customer_id = 0x305
  generation = VSDC_GEN_DC8000
  display_count = 1
  max_cursor_size = 32

Placing this entry last makes it the gate that enables MA35D1 hardware
recognition only after all the supporting ops and DTS changes are in
place.

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 drivers/gpu/drm/verisilicon/vs_hwdb.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
index 91524d16f778..7d630a667a3f 100644
--- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
+++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
@@ -129,6 +129,16 @@ static struct vs_chip_identity vs_chip_identities[] = {
 		.max_cursor_size = 64,
 		.formats = &vs_formats_no_yuv444,
 	},
+	{
+		.model = 0x0,		/* DCUltraLite */
+		.revision = 0x5560,
+		.customer_id = 0x305,
+
+		.generation = VSDC_GEN_DC8000,
+		.display_count = 1,
+		.max_cursor_size = 32,
+		.formats = &vs_formats_no_yuv444,
+	},
 };
 
 int vs_fill_chip_identity(struct regmap *regs,
-- 
2.43.0

Re: [PATCH v3 5/5] drm/verisilicon: add DCUltraLite chip identity to HWDB

[sashiko-bot]

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

Pre-existing issues:
- [High] A wildcard entry in vs_chip_identities shadows subsequent specific entries, making them unreachable.
- [High] The IRQ handler is registered before DRM structures are initialized, which can cause a NULL pointer dereference if an interrupt fires early or if initialization fails.
--

commit ef807eaf736a3cb84d7613b4d88301061b410a1b
Author: Joey Lu <a0987203069@gmail.com>

drm/verisilicon: add DCUltraLite chip identity to HWDB

This commit registers the Nuvoton MA35D1 DCUltraLite chip identity in
the hardware database. It places the entry last in the list to act as a
gate for hardware recognition.

> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 91524d16f7785..7d630a667a3f0 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -129,6 +129,16 @@ static struct vs_chip_identity vs_chip_identities[] = {

[Severity: High]
This is a pre-existing issue, but does the wildcard entry at the start of
vs_chip_identities prevent specific entries from being matched?

Looking at vs_chip_identities, the first entry uses a wildcard for the
customer ID:

static struct vs_chip_identity vs_chip_identities[] = {
	{
		.model = 0x8200,
		.revision = 0x5720,
		.customer_id = ~0U,
...

In vs_fill_chip_identity(), the search loop returns on the first match:

	for (i = 0; i < ARRAY_SIZE(vs_chip_identities); i++) {
		if (vs_chip_identities[i].model == model &&
		    vs_chip_identities[i].revision == revision &&
		    (vs_chip_identities[i].customer_id == customer_id ||
		     vs_chip_identities[i].customer_id == ~0U)) {
...

Because the ~0U entry comes first, wouldn't it shadow the subsequent specific 
entries for the same model and revision (like customer_id 0x310 and 0x311),
preventing their specific formats from ever being applied?

>  		.max_cursor_size = 64,
>  		.formats = &vs_formats_no_yuv444,
>  	},
> +	{
> +		.model = 0x0,		/* DCUltraLite */
> +		.revision = 0x5560,
> +		.customer_id = 0x305,
> +
> +		.generation = VSDC_GEN_DC8000,
> +		.display_count = 1,
> +		.max_cursor_size = 32,
> +		.formats = &vs_formats_no_yuv444,
> +	},
>  };
>  
>  int vs_fill_chip_identity(struct regmap *regs,

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for an early
interrupt to cause a NULL pointer dereference during probe?

In vs_dc_probe() from vs_dc.c, the IRQ handler is registered before DRM
structures are initialized:

	ret = devm_request_irq(dev, irq, vs_dc_irq_handler, 0,
			       dev_name(dev), dc);
...
	ret = vs_drm_initialize(dc, pdev);

If an interrupt fires during this window, the handler seems to unconditionally
dereference dc->drm_dev which is allocated later in vs_drm_initialize():

vs_drm_handle_irq() in vs_drm.c:
...
			if (dc->drm_dev->crtcs[i])
				drm_crtc_handle_vblank(&dc->drm_dev->crtcs[i]->base);

Could this lead to a panic if the hardware has a pending interrupt before DRM
is ready?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608023237.305036-1-a0987203069@gmail.com?part=5