Re: [PATCH 2/2] drm/verisilicon: add support for Nuvoton MA35D1 DCUltra Lite display controller

[Icenowy Zheng <zhengxingda@iscas.ac.cn>]

在 2026-05-12二的 18:59 +0800，Joey Lu写道：
> 
> On 5/12/2026 6:01 PM, Icenowy Zheng wrote:
> > 在 2026-05-12二的 17:06 +0800，Joey Lu写道：
> > 
> > ======= 8< =============
> > > > > > > diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c
> > > > > > > b/drivers/gpu/drm/verisilicon/vs_bridge.c
> > > > > > > index 7a93049368db..225af322de32 100644
> > > > > > > --- a/drivers/gpu/drm/verisilicon/vs_bridge.c
> > > > > > > +++ b/drivers/gpu/drm/verisilicon/vs_bridge.c
> > > > > > > @@ -164,13 +164,16 @@ static void
> > > > > > > vs_bridge_enable_common(struct
> > > > > > > vs_crtc *crtc,
> > > > > > >     			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(ou
> > > > > > > tput));
> > > > > > >     
> > > > > > > -	regmap_set_bits(dc->regs,
> > > > > > > VSDC_DISP_PANEL_CONFIG_EX(crtc-
> > > > > > > > id),
> > > > > > > -
> > > > > > > 			VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> > > > > > > +	if (dc->info->has_config_ex) {
> > > > > > > +		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_RU
> > > > > > > NNIN
> > > > > > > G(ou
> > > > > > > tput
> > > > > > > ));
> > > > > > > +
> > > > > > > +		regmap_set_bits(dc->regs,
> > > > > > > VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
> > > > > > > +				VSDC_DISP_PANEL_CONFIG_E
> > > > > > > X_CO
> > > > > > > MMIT
> > > > > > > );
> > > > > > Should the commit operation happen on DC8000/DCUltraLite
> > > > > > too?
> > > > > > (By
> > > > > > writing to DcregFrameBufferConfig0.VALID).
> > > > > > 
> > > > > > Many registers written has "Note: This field is double
> > > > > > buffered" in
> > > > > > the
> > > > > > DCUltraLite documentation.
> > > > > > 
> > > > > > I suggest create a static function for commit -- write to
> > > > > > the
> > > > > > corresponding commit bit on DC8200, and write to
> > > > > > DcregFrameBufferConfig0.VALID on DC8000/DCUltraLite.
> > > > > [a] There is no commit operation for DCUltra Lite.
> > > > > I'll not add a `VSDC_FB_CONFIG_VALID` macro. VALID (BIT(3))
> > > > > is a
> > > > > hardware-managed double-buffer status bit: hardware writes
> > > > > 1=PENDING
> > > > > when a new register set is ready and clears to 0=WORKING
> > > > > after
> > > > > the
> > > > > VBLANK copy. Software must never write it, and there is no
> > > > > polling
> > > > > use
> > > > It seems to be writable and controls whether register buffering
> > > > is
> > > > enabled, see [1].
> > > > 
> > > > The description of this bit in MA35D1 TRM says "This ensures a
> > > > frame
> > > > will always start with a valid working set if this register is
> > > > programmed last, which reduces the need for SW to wait for the
> > > > start of
> > > > a VBLANK signal in order to ensure all states are loaded before
> > > > the
> > > > next VBLANK", which indicates some kind of "committing write",
> > > > although
> > > > the code at [1] seems to indicate that double buffering is only
> > > > enabled
> > > > when bit is cleared.
> > > > 
> > > > Anyway this bit should be programmable, and "Software must
> > > > never
> > > > write
> > > > it" contradicts with the MA35D1 TRM.
> > > > 
> > > > Thanks,
> > > > Icenowy
> > > > 
> > > > [1]
> > > > https://github.com/rockos-riscv/rockos-kernel/blob/rockos-v6.6.y/drivers/gpu/drm/eswin/es_dc_hw.c#L993
> > > Thank you for the correction. I'll add
> > > `#define VSDC_FB_CONFIG_VALID BIT(3)` to vs_primary_plane_regs.h
> > > and
> > > write it in `vs_primary_plane_commit()` for non-config_ex
> > > variants.
> > > > > case in the driver that requires a named constant. For non-
> > > > > config_ex
> > > > > variants, `vs_primary_plane_commit()` performs no commit
> > > > > operation —
> > > > > `VSDC_FB_CONFIG_ENABLE` (OUTPUT, BIT(0)) is set in
> > > > > `vs_crtc_atomic_enable()` and `VSDC_FB_CONFIG_RESET` (BIT(4))
> > > > > is
> > > > > set/cleared in the bridge enable/disable paths.
> > Well according to the driver code for DC8000 from Eswin, and the
> > bit
> > named "VALID", maybe it should be cleared before programming the
> > registers, and set after programming registers, to make the process
> > of
> > programming registers atomic from the perspective of the display
> > controller.
> > 
> > Anyway this should require testing on real hardware to verify.
> > 
> > By the way, I see multiple peripheral drivers for MA35D1 get
> > applied in
> > the torvalds tree, but the device tree is still only a skeleton;
> > when
> > will the device tree be updated?
> > 
> > Thanks,
> > Icenowy
> 
> Thanks for pointing this out. I’ll perform tests on real hardware
> since 
> I haven’t used this bit before.
> 
> As for the device tree, we plan to update it comprehensively after 
> completing several major IPs, with the goal of releasing the update 
> later this year.

Well I bought a MA35D1 board (MYIR MYB-LMA35 + RGB LCD) earlier this
year (and this is where I got the MA35D1 identification register
values). Hope I can have a chance to test this driver by myself.

As MMC, Ethernet and USB support is all applied, maybe it's already
worthy to update the device tree ;-)

Thanks,
Icenowy

> 
> > > > ========= 8< ==========
> > > >