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

[Joey Lu <a0987203069@gmail.com>]

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@gmail.com>
>>>> ---
>>>>    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);