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

[Joey Lu <a0987203069@gmail.com>]

On 5/15/2026 4:38 PM, Icenowy Zheng wrote:
> 在 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
Thanks for pointing that out. To clarify, the TRM defines the states for 
this bit as:
1: PENDING (Shadow registers are locked; incoming CPU writes are held in 
the shadow buffer).
0: WORKING (Shadow registers are unfrozen/live; configuration is committed).

Regarding the Eswin reference, `dc_hw_enable_shadow_register(hw, false)` 
actually writes a 1 to isolate the registers during a batch update, and 
then transitions back to true (0) to flush them out.
My hardware test confirms this sequence:

- atomic_begin: We set VALID = 1. This safely freezes the current 
scanout configuration so we can update multiple registers (like frame 
addresses and pitches) without mid-frame tearing.
- atomic_update: We write the new register values safely into the frozen 
shadow space.
- atomic_flush: We clear VALID = 0. This releases the shadow lock, 
triggering the hardware to latch the whole batch atomically.

If we reverse this logic—leaving the bit at 1 after the flush — the 
hardware remains permanently stuck in the PENDING state, waiting to be 
unfrozen, which starves the display engine and results in the blank 
screen behavior I observed.
>> 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< ==========
>>>>>>>