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

[Joey Lu <a0987203069@gmail.com>]

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

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