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

[Joey Lu <a0987203069@gmail.com>]

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(output));
>>>>>>     
>>>>>> -	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_RUNNIN
>>>>>> G(ou
>>>>>> tput
>>>>>> ));
>>>>>> +
>>>>>> +		regmap_set_bits(dc->regs,
>>>>>> VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
>>>>>> +				VSDC_DISP_PANEL_CONFIG_EX_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.

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