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

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

在 2026-05-15五的 14:25 +0800，Joey Lu写道：
> 
> On 5/12/2026 9:12 PM, Icenowy Zheng wrote:
> > 在 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_E
> > > > > > > > > N);
> > > > > > > > >      	regmap_set_bits(dc->regs,
> > > > > > > > > VSDC_DISP_PANEL_CONFIG(output),
> > > > > > > > >      			VSDC_DISP_PANEL_CONFIG_RUNNI
> > > > > > > > > NG);
> > > > > > > > > -	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_RUNNIN
> > > > > > > > > G(ou
> > > > > > > > > tput));
> > > > > > > > >      
> > > > > > > > > -	regmap_set_bits(dc->regs,
> > > > > > > > > VSDC_DISP_PANEL_CONFIG_EX(crtc-
> > > > > > > > > > id),
> > > > > > > > > -
> > > > > > > > > 			VSDC_DISP_PANEL_CONFIG_EX_CO
> > > > > > > > > MMIT);
> > > > > > > > > +	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_STAR
> > > > > > > > > T_RU
> > > > > > > > > NNIN
> > > > > > > > > G(ou
> > > > > > > > > tput
> > > > > > > > > ));
> > > > > > > > > +
> > > > > > > > > +		regmap_set_bits(dc->regs,
> > > > > > > > > VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
> > > > > > > > > +				VSDC_DISP_PANEL_CONF
> > > > > > > > > IG_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
> 
> Yes you can!
> 
> I have performed hardware validation on the MA35D1 and found that
> this 
> bit acts as a manual latch for the shadow registers rather than an 
> auto-clearing trigger, which clarifies the slightly ambiguous 
> description in the TRM.
> 
> Following your suggestion, I will align the implementation with
> ESWIN's 
> DC8000 logic: setting the VALID bit at atomic_begin and clearing it

Ah do you mean clearing it at begin and setting it at flush?

In the Eswin driver (which seems to be based on reference code by
Verisilicon, I saw similar driver code for DC8200),
dc_hw_enable_shadow_register() clears VALID bit when enable is true,
and all register setting sequences calls that function with enable =
true before setting and enable = false after setting.

In addition, considering this bit is called "VALID" instead of
"INVALID", I think it represents that the DC will apply the new setting
when it's set and keep the current setting when it's cleared, so I
think it should be cleared before modeset sequence and be set after
modeset sequence.

Thanks,
Icenowy

> at 
> atomic_flush. My tests confirm this allows the hardware to latch the 
> plane configuration correctly while avoiding the blank screen issues 
> observed with other configurations.
> 
> I am preparing the v2 patchset with this change, along with the 
> requested commit splits, and will submit it shortly.🙂
> 
> > > > > > ========= 8< ==========
> > > > > >