Devicetree
 help / color / mirror / Atom feed
From: Icenowy Zheng <zhengxingda@iscas.ac.cn>
To: Joey Lu <a0987203069@gmail.com>,
	maarten.lankhorst@linux.intel.com,  mripard@kernel.org,
	tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
		robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org
Cc: ychuang3@nuvoton.com, schung@nuvoton.com, yclu4@nuvoton.com,
	 dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] drm/verisilicon: add support for Nuvoton MA35D1 DCUltra Lite display controller
Date: Tue, 12 May 2026 16:11:12 +0800	[thread overview]
Message-ID: <3b94806073de8bd1d79aa7ec956493f67679e46b.camel@iscas.ac.cn> (raw)
In-Reply-To: <de35406e-874d-4bdd-be7f-3d74dc37b13f@gmail.com>

在 2026-05-12二的 15:45 +0800,Joey Lu写道:
> 
> On 5/11/2026 5:47 PM, Icenowy Zheng wrote:
> > 在 2026-05-11一的 15:51 +0800,Joey Lu写道:
> > > The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltra Lite
> > > display
> > > controller, which is a previous generation of the DC8000 series.
> > > While
> > > the general register layout is similar to the DC8000, there are
> > > several
> > > key differences that require per-variant handling in the driver.
> > > 
> > > Add a vs_dc_info platform data structure (in vs_hwdb.h) to
> > > describe
> > > per-IP-variant capabilities, and use it throughout the driver to
> > > select
> > > the correct code paths at runtime.
> > > 
> > > Key differences between DC8000 and DCUltra Lite handled:
> > What the driver supports now is DC8200, DC8000 have the following
> > point
> > 1~4 the same with DCUltraLite (different to DC8200).
> Understood. I'll rename all `vs_dc8000_*` identifiers to
> `vs_dc8200_*` in v2. The commit message will also be corrected to say
> that points 1~4 are differences from DC8200, not DC8000.
> > > 1. No chip identity registers (0x0020-0x0030): DCUltra Lite uses
> > > static
> > >     platform data instead of reading model/revision/customer_id
> > > from
> > > HW.
> > My test shows that revision and customer_id is correctly present,
> > only
> > model is 0 -- I think this can be also considered as a valid model
> > value because the IP name has also no model number.
> > 
> > The revision number is 0x5560 and customer id is 0x305 .
> Thank you for testing. I'll drop the `has_chip_id` flag
> entirely. In v2, `vs_fill_chip_identity()` will be called for all
> variants. A new entry will be added to `vs_chip_identities[]` in
> vs_hwdb.c with model=0x0, revision=0x5560, customer_id=0x305,
> display_count=1, and `vs_formats_no_yuv444`.
> e.g. verisilicon-dc 40260000.display: Found DC0 rev 5560 customer 305
> > 
> > > 2. No CONFIG_EX commit mechanism: DC8000 uses registers at 0x1CC0
> > >     (FB_CONFIG_EX), 0x24D8 (FB_TOP_LEFT), 0x24E0
> > > (FB_BOTTOM_RIGHT),
> > >     0x2510 (FB_BLEND_CONFIG), 0x2518 (PANEL_CONFIG_EX). DCUltra
> > > Lite
> > >     omits all of these and instead uses enable/reset bits in
> > > FB_CONFIG
> > >     (bit 0 = enable, bit 4 = reset) for direct framebuffer
> > > updates.
> > > 
> > > 3. No PANEL_START register (0x1CCC): DCUltra Lite panel output
> > > starts
> > >     when PANEL_CONFIG.RUNNING is set; no separate multi-display
> > > sync
> > >     start register is needed.
> > > 
> > > 4. Different IRQ registers: DCUltra Lite uses 0x147C (IRQ_STA) /
> > >     0x1480 (IRQ_EN); DC8000 uses 0x0010 (IRQ_ACK) / 0x0014
> > > (IRQ_EN).
> > > 
> > > 5. Different clock/reset topology: DCUltra Lite requires only
> > > "core"
> > >     (bus gate) and "pix0" (pixel divider) clocks with no reset
> > > lines
> > >     managed by the driver. DC8000 needs core/axi/ahb clocks and
> > > three
> > >     resets.
> > It's possible that your SoC integration combines core clock gate
> > with
> > bus clock gate instead of bus clock gate not existing.
> Agreed. In v2 I'll remove the family-gated clock handling and
> instead use `devm_clk_get_optional_enabled()` for the axi and ahb
> clocks, so they are simply skipped if not present in DT. Resets are
> already optional. This keeps the probe path uniform and handles any
> SoC-specific clock combinations naturally.

Maybe this could be splitted as another patch, to make single commits
smaller.

> > > 6. Single output only: DCUltra Lite has one display output; per-
> > > output
> > >     index logic is still in place but display_count is fixed at
> > > 1.
> > > 
> > > 7. Reduced register space: max_register is 0x2000 vs DC8000's
> > > 0x2544.
> > > 
> > > Add the "nuvoton,ma35d1-dcu" compatible string to the OF match
> > > table,
> > > extend Kconfig to allow building on ARCH_MA35 platforms, and
> > > expose
> > > vs_formats_no_yuv444 as the default format table for DCUltra Lite
> > > (YUV444 blending is a DC8000-only feature).
> > > 
> > > All changes have been tested on Nuvoton MA35D1 hardware and are
> > > functioning correctly.
> > > 
> > > Signed-off-by: Joey Lu <a0987203069@gmail.com>
> > > ---
> > >   drivers/gpu/drm/verisilicon/Kconfig           |   2 +-
> > >   drivers/gpu/drm/verisilicon/vs_bridge.c       |  28 ++--
> > >   drivers/gpu/drm/verisilicon/vs_crtc.c         |  13 +-
> > >   drivers/gpu/drm/verisilicon/vs_dc.c           | 129
> > > ++++++++++++----
> > > --
> > >   drivers/gpu/drm/verisilicon/vs_dc.h           |   1 +
> > >   drivers/gpu/drm/verisilicon/vs_drm.c          |  16 ++-
> > >   drivers/gpu/drm/verisilicon/vs_hwdb.c         |   2 +-
> > >   drivers/gpu/drm/verisilicon/vs_hwdb.h         |  25 ++++
> > >   .../gpu/drm/verisilicon/vs_primary_plane.c    |  43 +++---
> > >   .../drm/verisilicon/vs_primary_plane_regs.h   |   2 +
> > >   10 files changed, 187 insertions(+), 74 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> > > b/drivers/gpu/drm/verisilicon/Kconfig
> > > index 7cce86ec8603..295d246eb4b4 100644
> > > --- a/drivers/gpu/drm/verisilicon/Kconfig
> > > +++ b/drivers/gpu/drm/verisilicon/Kconfig
> > > @@ -2,7 +2,7 @@
> > >   config DRM_VERISILICON_DC
> > >   	tristate "DRM Support for Verisilicon DC-series display
> > > controllers"
> > >   	depends on DRM && COMMON_CLK
> > > -	depends on RISCV || COMPILE_TEST
> > > +	depends on RISCV || ARCH_MA35 || COMPILE_TEST
> > >   	select DRM_BRIDGE_CONNECTOR
> > >   	select DRM_CLIENT_SELECTION
> > >   	select DRM_DISPLAY_HELPER
> > > 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_RUNNING(ou
> > > tput
> > > ));
> > > +
> > > +		regmap_set_bits(dc->regs,
> > > VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
> > > +				VSDC_DISP_PANEL_CONFIG_EX_COMMIT
> > > );
> > 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

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

========= 8< ==========


  reply	other threads:[~2026-05-12  8:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-11  7:51 [PATCH 0/2] drm/verisilicon: add Nuvoton MA35D1 DCUltra Lite support Joey Lu
2026-05-11  7:51 ` [PATCH 1/2] dt-bindings: display: verisilicon,dc: generalize for DCUltra Lite variant Joey Lu
2026-05-11  9:49   ` Rob Herring (Arm)
2026-05-11  9:59   ` Icenowy Zheng
2026-05-12  8:02     ` Joey Lu
2026-05-12  0:41   ` sashiko-bot
2026-05-11  7:51 ` [PATCH 2/2] drm/verisilicon: add support for Nuvoton MA35D1 DCUltra Lite display controller Joey Lu
2026-05-11  9:47   ` Icenowy Zheng
2026-05-12  7:45     ` Joey Lu
2026-05-12  8:11       ` Icenowy Zheng [this message]
2026-05-12  9:06         ` Joey Lu
2026-05-12 10:01           ` Icenowy Zheng
2026-05-12 10:59             ` Joey Lu
2026-05-12 13:12               ` Icenowy Zheng
2026-05-12  1:12   ` sashiko-bot
2026-05-12  8:24   ` Thomas Zimmermann
2026-05-12  9:10     ` Joey Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b94806073de8bd1d79aa7ec956493f67679e46b.camel@iscas.ac.cn \
    --to=zhengxingda@iscas.ac.cn \
    --cc=a0987203069@gmail.com \
    --cc=airlied@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=robh@kernel.org \
    --cc=schung@nuvoton.com \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    --cc=ychuang3@nuvoton.com \
    --cc=yclu4@nuvoton.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox