Re: [PATCH v4 4/6] drm/verisilicon: add DC8000 (DCUltraLite) display controller support

From: Joey Lu

Date: Sun Jun 21 2026 - 22:31:07 EST

On 6/18/2026 6:33 PM, Icenowy Zheng wrote:

(CC'ed clk maintainers for weird clock gate bit)

在 2026-06-17三的 18:35 +0800，Joey Lu写道：

On 6/15/2026 4:51 PM, Icenowy Zheng wrote:

在 2026-06-15一的 14:50 +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 DC8000
nodes
    without those clocks are handled gracefully.

Add vs_dc8000.c implementing the vs_dc_funcs vtable for the above
differences. The probe now selects vs_dc8000_funcs when the
identified
generation is VSDC_GEN_DC8000 (DCUltraLite reads model 0x0,
revision 0x5560, customer_id 0x305).

Signed-off-by: Joey Lu <a0987203069@xxxxxxxxx>
---
  drivers/gpu/drm/verisilicon/Makefile    | 2 +-
  drivers/gpu/drm/verisilicon/vs_dc.c     | 9 ++-
  drivers/gpu/drm/verisilicon/vs_dc.h     | 1 +
  drivers/gpu/drm/verisilicon/vs_dc8000.c | 78
+++++++++++++++++++++++++
  4 files changed, 86 insertions(+), 4 deletions(-)
  create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8000.c

diff --git a/drivers/gpu/drm/verisilicon/Makefile
b/drivers/gpu/drm/verisilicon/Makefile
index 9d4cd16452fa..d2fd8e4dff24 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_dc8000.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 9729b693d360..9499fffbca58 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");

Please make the clock change a separated patch for atomicity.

BTW the MA35D1 manual's clock tree shows that DCUltra appears on
AXI2
ACLK, AHB_HCLK2, behind a mux of SYS-PLL/EPLL-DIV2 (which seems to
be
the core clock), and behind a divider (which seems to be the pixel
clock).

However it's weird that only one DCUltra Clock Enable Bit exists
despite both bus clocks have "ICG" (I think it means "Integrated
Clock
Gating"). In addition the linux clk-ma35d1 driver assigns
"dcu_gate" as
a downstream of "dcu_mux", although the Figure 6.5-2 in the TRM
shows
no ICG after the "Display core CLK" mux.

Is the two bus clocks controlled by a single gate bit, and is the
bit
also gating DC core clock?

Thanks,
Icenowy

I will split the axi/ahb optional-clock change into its own patch in
v5
for atomicity.
Regarding the MA35D1 clock tree: from the TRM, the single "dcu_gate"
bit
gates both bus clocks (AXI ACLK and AHB HCLK) together with the
display
core clock through the same ICG cell. The clk-ma35d1 driver exposes
only
"dcu_gate" (downstream of "dcu_mux") and does not provide separate

Then it's one of the case that the clock tree doesn't properly
represent the hardware, which is bad. However, as three gates share the
same bit, I am not sure how to represent such kind of thing in the
common clk framework.

axi/ahb clock entries. Therefore the MA35D1 DT binding will use only
two
clocks ("core" and "pix0"); making axi and ahb optional in the driver
is the correct approach, and this will be stated clearly in the
split-out patch.

I agree to make them optional, although these two clocks do exist in
the hardware of MA35D1.

Thanks,
Icenowy

As mentioned in the DT binding reply, the absence of separate AXI/AHB clock entries for DCU in clk driver is due to the hardware design constraint of a single shared enable bit, not a driver oversight. In v5, the axi and ahb clock fetches in `vs_dc_probe` will be split into their own patch and made optional via `devm_clk_get_optional_enabled`, with a comment explaining that on MA35D1 the AXI and AHB bus clocks share the single `dcu_gate` enable bit and are therefore not separately exposed by the clock driver.

   if (IS_ERR(dc->ahb_clk)) {
   dev_err(dev, "can't get ahb clock\n");
   return PTR_ERR(dc->ahb_clk);