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: Mon, 11 May 2026 17:47:51 +0800 [thread overview]
Message-ID: <93e69179dbc495188cfffd8015350b3a55ce7876.camel@iscas.ac.cn> (raw)
In-Reply-To: <20260511075142.54752-3-a0987203069@gmail.com>
在 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).
>
> 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 .
>
> 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.
>
> 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(output
> ));
> +
> + 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.
> + }
> }
>
> static void vs_bridge_atomic_enable_dpi(struct drm_bridge *bridge,
> @@ -228,14 +231,17 @@ static void vs_bridge_atomic_disable(struct
> drm_bridge *bridge,
> struct vs_dc *dc = crtc->dc;
> unsigned int output = crtc->id;
>
> - regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> - VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
> - VSDC_DISP_PANEL_START_RUNNING(output));
> regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> VSDC_DISP_PANEL_CONFIG_RUNNING);
This bit seems to be not present in DCUltraLite, instead should
DcregFrameBufferConfig0.RESET be clear, which will stall the DPI
timing?
>
> - 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 |
> +
> VSDC_DISP_PANEL_START_RUNNING(output));
> +
> + regmap_set_bits(dc->regs,
> VSDC_DISP_PANEL_CONFIG_EX(crtc->id),
> + VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> + }
Ditto.
> }
>
> static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
> b/drivers/gpu/drm/verisilicon/vs_crtc.c
> index 9080344398ca..2f3e6d41c657 100644
> --- a/drivers/gpu/drm/verisilicon/vs_crtc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
> @@ -18,6 +18,7 @@
> #include "vs_dc.h"
> #include "vs_dc_top_regs.h"
> #include "vs_drm.h"
> +#include "vs_hwdb.h"
> #include "vs_plane.h"
>
> static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -132,7 +133,11 @@ static int vs_crtc_enable_vblank(struct drm_crtc
> *crtc)
> struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
> + if (dc->info->family == VS_DC_FAMILY_DCULTRA_LITE)
> + regmap_write(dc->regs, VSDC_DISP_IRQ_EN, BIT(0));
Should `BIT(0)` be assigned with a named macro here, like
`VSDC_DISP_IRQ_VSYNC(0)` ?
In addition, even it's known that DC8000/DCUltraLite has only one
output, hardcoding 0 here isn't so good. The header file at [1] (which
is for DC8000) shows that two display interrupts and more other
interrupts are present.
BTW I don't like the idea of setting a "family" (because DC8000 behave
nearly the same with DCUltraLite), maybe use `.irq_reg = VSDC_DISP_IRQ`
(or a bool variable named `uses_top_irq`) is better?
[1]
https://github.com/milkv-megrez/rockos-u-boot/blob/c9221cf2fa77d39c0b241ab4b030c708e7ebe279/drivers/video/eswin/eswin_dc_reg.h#L2771
> + else
> + regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
> + VSDC_TOP_IRQ_VSYNC(vcrtc->id));
>
> return 0;
> }
> @@ -142,7 +147,11 @@ static void vs_crtc_disable_vblank(struct
> drm_crtc *crtc)
> struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
> + if (dc->info->family == VS_DC_FAMILY_DCULTRA_LITE)
> + regmap_write(dc->regs, VSDC_DISP_IRQ_EN, 0);
> + else
> + regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
> + VSDC_TOP_IRQ_VSYNC(vcrtc->id));
> }
>
> static const struct drm_crtc_funcs vs_crtc_funcs = {
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
> b/drivers/gpu/drm/verisilicon/vs_dc.c
> index dad9967bc10b..82a6a26f6d81 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -9,21 +9,45 @@
> #include <linux/of_graph.h>
>
> #include "vs_crtc.h"
> +#include "vs_crtc_regs.h"
> #include "vs_dc.h"
> #include "vs_dc_top_regs.h"
> #include "vs_drm.h"
> #include "vs_hwdb.h"
>
> -static const struct regmap_config vs_dc_regmap_cfg = {
> +static const struct regmap_config vs_dc8000_regmap_cfg = {
As what I said, the currently supported thing is DC8200, not DC8000.
> .reg_bits = 32,
> .val_bits = 32,
> .reg_stride = sizeof(u32),
> - /* VSDC_OVL_CONFIG_EX(1) */
> .max_register = 0x2544,
> };
>
> +static const struct regmap_config vs_dcultra_lite_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 0x2000,
> +};
> +
> +static const struct vs_dc_info vs_dc8000_info = {
> + .family = VS_DC_FAMILY_DC8000,
> + .has_chip_id = true,
> + .has_config_ex = true,
> + .regmap_cfg = &vs_dc8000_regmap_cfg,
> +};
> +
> +static const struct vs_dc_info vs_dcultra_lite_info = {
> + .family = VS_DC_FAMILY_DCULTRA_LITE,
> + .display_count = 1,
> + .has_chip_id = false,
> + .has_config_ex = false,
> + .regmap_cfg = &vs_dcultra_lite_regmap_cfg,
> + .formats = &vs_formats_no_yuv444,
> +};
> +
> static const struct of_device_id vs_dc_driver_dt_match[] = {
> - { .compatible = "verisilicon,dc" },
> + { .compatible = "verisilicon,dc", .data = &vs_dc8000_info },
"verisilicon,dc" is expected for DC8000 and 8200, and because of model,
rev and customer_id values are all present for DCUltraLite, the special
data might be dropped?
> + { .compatible = "nuvoton,ma35d1-dcu", .data =
> &vs_dcultra_lite_info },
> {},
> };
> MODULE_DEVICE_TABLE(of, vs_dc_driver_dt_match);
> @@ -33,6 +57,13 @@ static irqreturn_t vs_dc_irq_handler(int irq, void
> *private)
> struct vs_dc *dc = private;
> u32 irqs;
>
> + if (dc->info->family == VS_DC_FAMILY_DCULTRA_LITE) {
> + regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &irqs);
Maybe add an item in the hwdb for the IRQ register?
> + if (irqs & BIT(0))
> + vs_drm_handle_irq(dc,
> VSDC_TOP_IRQ_VSYNC(0));
> + return IRQ_HANDLED;
Now I think for DC8200, the irq number decoding should be done here
too.
> + }
> +
> regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);
>
> vs_drm_handle_irq(dc, irqs);
> @@ -43,6 +74,7 @@ static irqreturn_t vs_dc_irq_handler(int irq, void
> *private)
> static int vs_dc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + const struct vs_dc_info *info;
> struct vs_dc *dc;
> void __iomem *regs;
> unsigned int port_count, i;
> @@ -55,6 +87,10 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> return -ENODEV;
> }
>
> + info = of_device_get_match_data(dev);
> + if (!info)
> + return -ENODEV;
> +
> port_count = of_graph_get_port_count(dev->of_node);
> if (!port_count) {
> dev_err(dev, "can't find DC downstream ports\n");
> @@ -75,15 +111,31 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> if (!dc)
> return -ENOMEM;
>
> - dc->rsts[0].id = "core";
> - dc->rsts[1].id = "axi";
> - dc->rsts[2].id = "ahb";
> + dc->info = info;
>
> - ret = devm_reset_control_bulk_get_optional_shared(dev,
> VSDC_RESET_COUNT,
> - dc->rsts);
> - if (ret) {
> - dev_err(dev, "can't get reset lines\n");
> - return ret;
> + if (info->family == VS_DC_FAMILY_DC8000) {
> + dc->rsts[0].id = "core";
> + dc->rsts[1].id = "axi";
> + dc->rsts[2].id = "ahb";
> +
> + ret =
> devm_reset_control_bulk_get_optional_shared(dev,
> + VSDC_RESET_COUNT, dc->rsts);
> + if (ret) {
> + dev_err(dev, "can't get reset lines\n");
> + return ret;
> + }
> +
> + dc->axi_clk = devm_clk_get_enabled(dev, "axi");
> + if (IS_ERR(dc->axi_clk)) {
> + dev_err(dev, "can't get axi clock\n");
> + return PTR_ERR(dc->axi_clk);
> + }
> +
> + dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
> + if (IS_ERR(dc->ahb_clk)) {
> + dev_err(dev, "can't get ahb clock\n");
> + return PTR_ERR(dc->ahb_clk);
> + }
Please make these clocks optional, instead of wrap them in a "family
detection". The resets are already optional, see above.
> }
>
> dc->core_clk = devm_clk_get_enabled(dev, "core");
> @@ -92,28 +144,18 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> return PTR_ERR(dc->core_clk);
> }
>
> - dc->axi_clk = devm_clk_get_enabled(dev, "axi");
> - if (IS_ERR(dc->axi_clk)) {
> - dev_err(dev, "can't get axi clock\n");
> - return PTR_ERR(dc->axi_clk);
> - }
> -
> - dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
> - if (IS_ERR(dc->ahb_clk)) {
> - dev_err(dev, "can't get ahb clock\n");
> - return PTR_ERR(dc->ahb_clk);
> - }
> -
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> dev_err(dev, "can't get irq\n");
> return irq;
> }
>
> - ret = reset_control_bulk_deassert(VSDC_RESET_COUNT, dc-
> >rsts);
> - if (ret) {
> - dev_err(dev, "can't deassert reset lines\n");
> - return ret;
> + if (info->family == VS_DC_FAMILY_DC8000) {
> + ret = reset_control_bulk_deassert(VSDC_RESET_COUNT,
> dc->rsts);
> + if (ret) {
> + dev_err(dev, "can't deassert reset
> lines\n");
> + return ret;
> + }
> }
>
> regs = devm_platform_ioremap_resource(pdev, 0);
> @@ -123,23 +165,30 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> goto err_rst_assert;
> }
>
> - dc->regs = devm_regmap_init_mmio(dev, regs,
> &vs_dc_regmap_cfg);
> + dc->regs = devm_regmap_init_mmio(dev, regs, info-
> >regmap_cfg);
> if (IS_ERR(dc->regs)) {
> ret = PTR_ERR(dc->regs);
> goto err_rst_assert;
> }
>
> - ret = vs_fill_chip_identity(dc->regs, &dc->identity);
> - if (ret)
> - goto err_rst_assert;
> + if (info->has_chip_id) {
> + ret = vs_fill_chip_identity(dc->regs, &dc-
> >identity);
> + if (ret)
> + goto err_rst_assert;
>
> - dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
> >identity.model,
> - dc->identity.revision, dc->identity.customer_id);
> + dev_info(dev, "Found DC%x rev %x customer %x\n",
> + dc->identity.model, dc->identity.revision,
> + dc->identity.customer_id);
>
> - if (port_count > dc->identity.display_count) {
> - dev_err(dev, "too many downstream ports than HW
> capability\n");
> - ret = -EINVAL;
> - goto err_rst_assert;
> + if (port_count > dc->identity.display_count) {
> + dev_err(dev, "too many downstream ports than
> HW capability\n");
> + ret = -EINVAL;
> + goto err_rst_assert;
> + }
> + } else {
> + /* Fill identity from platform data */
> + dc->identity.display_count = info->display_count;
> + dc->identity.formats = info->formats;
> }
>
> for (i = 0; i < dc->identity.display_count; i++) {
> @@ -168,7 +217,8 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> return 0;
>
> err_rst_assert:
> - reset_control_bulk_assert(VSDC_RESET_COUNT, dc->rsts);
> + if (info->family == VS_DC_FAMILY_DC8000)
> + reset_control_bulk_assert(VSDC_RESET_COUNT, dc-
> >rsts);
> return ret;
> }
>
> @@ -180,7 +230,8 @@ static void vs_dc_remove(struct platform_device
> *pdev)
>
> dev_set_drvdata(&pdev->dev, NULL);
>
> - reset_control_bulk_assert(VSDC_RESET_COUNT, dc->rsts);
> + if (dc->info->family == VS_DC_FAMILY_DC8000)
> + reset_control_bulk_assert(VSDC_RESET_COUNT, dc-
> >rsts);
> }
>
> static void vs_dc_shutdown(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
> b/drivers/gpu/drm/verisilicon/vs_dc.h
> index ed1016f18758..f0613519af37 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
> @@ -31,6 +31,7 @@ struct vs_dc {
> struct clk *pix_clk[VSDC_MAX_OUTPUTS];
> struct reset_control_bulk_data rsts[VSDC_RESET_COUNT];
>
> + const struct vs_dc_info *info;
> struct vs_drm_dev *drm_dev;
> struct vs_chip_identity identity;
> };
> diff --git a/drivers/gpu/drm/verisilicon/vs_drm.c
> b/drivers/gpu/drm/verisilicon/vs_drm.c
> index fd259d53f49f..ff0fc6673006 100644
> --- a/drivers/gpu/drm/verisilicon/vs_drm.c
> +++ b/drivers/gpu/drm/verisilicon/vs_drm.c
> @@ -27,6 +27,7 @@
> #include "vs_dc.h"
> #include "vs_dc_top_regs.h"
> #include "vs_drm.h"
> +#include "vs_hwdb.h"
>
> #define DRIVER_NAME "verisilicon"
> #define DRIVER_DESC "Verisilicon DC-series display controller
> driver"
> @@ -72,12 +73,19 @@ static struct drm_mode_config_helper_funcs
> vs_mode_config_helper_funcs = {
> .atomic_commit_tail = drm_atomic_helper_commit_tail,
> };
>
> -static void vs_mode_config_init(struct drm_device *drm)
> +static void vs_mode_config_init(struct drm_device *drm, struct vs_dc
> *dc)
> {
> drm->mode_config.min_width = 0;
> drm->mode_config.min_height = 0;
> - drm->mode_config.max_width = 8192;
> - drm->mode_config.max_height = 8192;
> +
> + if (dc->info->family == VS_DC_FAMILY_DCULTRA_LITE) {
> + drm->mode_config.max_width = 1920;
> + drm->mode_config.max_height = 1080;
Surely only max width 1920 and max height 1080? Can a display of
1080x1920 (e.g. a portrait DSI panel behind a RGB2DSI bridge) be
supported? Can a 2170x60 display (MacBook Touch Bar panel, also a DSI
panel) be supported? Both displays here will have no higher pixel clock
than 1080p in the same refresh rate, although the width / height is
higher than your restriction.
In addition, these parameters decide how big a FB can be created -- the
FB might be scaned out by multiple devices (e.g. a USB DisplayLink
device scanning out the remaining part). The stride register is said to
have 17-bit width in the MA35D1 TRM, so the possible FB width could be
quite high -- assume the 17th bit is only for the value with one 1 and
all remaining 0, we get 65536 bytes stride; with 4-byte-per-pixel
divided this gets 16384 pixels -- the htotal/hdisplay/vtotal/vdisplay
fields in the manual has 15-bit field width, which can reach 32767.
I strongly suggest leave the original values here.
> + } else {
> + drm->mode_config.max_width = 8192;
> + drm->mode_config.max_height = 8192;
> + }
> +
> drm->mode_config.funcs = &vs_mode_config_funcs;
> drm->mode_config.helper_private =
> &vs_mode_config_helper_funcs;
> }
> @@ -125,7 +133,7 @@ int vs_drm_initialize(struct vs_dc *dc, struct
> platform_device *pdev)
> if (ret)
> return ret;
>
> - vs_mode_config_init(drm);
> + vs_mode_config_init(drm, dc);
>
> /* Enable connectors polling */
> drm_kms_helper_poll_init(drm);
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 09336af0900a..39402d75d841 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -78,7 +78,7 @@ static const u32 vs_formats_array_with_yuv444[] = {
> /* TODO: non-RGB formats */
> };
>
> -static const struct vs_formats vs_formats_no_yuv444 = {
> +const struct vs_formats vs_formats_no_yuv444 = {
> .array = vs_formats_array_no_yuv444,
> .num = ARRAY_SIZE(vs_formats_array_no_yuv444)
> };
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> index 92192e4fa086..655cf93ca3aa 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> @@ -14,6 +14,29 @@ struct vs_formats {
> unsigned int num;
> };
>
> +enum vs_dc_family {
> + VS_DC_FAMILY_DC8000,
> + VS_DC_FAMILY_DCULTRA_LITE,
> +};
> +
> +/**
> + * struct vs_dc_info - per-SoC DC platform data
> + * @family: DC IP family (DC8000, DCUltra Lite, etc.)
> + * @display_count: number of display outputs (0 = auto-detect
> from DT/HW)
> + * @has_chip_id: whether chip identity registers exist
> + * @has_config_ex: whether CONFIG_EX commit mechanism exists
> + * @regmap_cfg: regmap configuration for this
> variant
> + * @formats: supported pixel formats (NULL = auto-detect
> from chip ID)
> + */
> +struct vs_dc_info {
> + enum vs_dc_family family;
> + u32 display_count;
> + bool has_chip_id;
> + bool has_config_ex;
> + const struct regmap_config *regmap_cfg;
> + const struct vs_formats *formats;
> +};
> +
> struct vs_chip_identity {
> u32 model;
> u32 revision;
> @@ -23,6 +46,8 @@ struct vs_chip_identity {
> const struct vs_formats *formats;
> };
>
> +extern const struct vs_formats vs_formats_no_yuv444;
> +
> int vs_fill_chip_identity(struct regmap *regs,
> struct vs_chip_identity *ident);
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 1f2be41ae496..197d5d683e22 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -55,8 +55,9 @@ static int vs_primary_plane_atomic_check(struct
> drm_plane *plane,
>
> static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int
> output)
> {
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_COMMIT);
> + if (dc->info->has_config_ex)
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_COMMIT);
Should VALID bit be written here instead of doing nothing on
DC8000/DCUltraLite ?
> }
>
> static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
> @@ -69,11 +70,13 @@ static void vs_primary_plane_atomic_enable(struct
> drm_plane *plane,
> unsigned int output = vcrtc->id;
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_FB_EN);
> - regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
> - VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
> + if (dc->info->has_config_ex) {
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_FB_EN);
> + regmap_update_bits(dc->regs,
> VSDC_FB_CONFIG_EX(output),
> +
> VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
> +
> VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
> + }
>
> vs_primary_plane_commit(dc, output);
> }
> @@ -88,8 +91,9 @@ static void vs_primary_plane_atomic_disable(struct
> drm_plane *plane,
> unsigned int output = vcrtc->id;
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_FB_EN);
> + if (dc->info->has_config_ex)
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_FB_EN);
>
> vs_primary_plane_commit(dc, output);
> }
> @@ -126,6 +130,11 @@ static void
> vs_primary_plane_atomic_update(struct drm_plane *plane,
> VSDC_FB_CONFIG_UV_SWIZZLE_EN,
> vs_state->format.uv_swizzle);
>
> + /* DCUltra Lite requires explicit enable/reset bits in
> FB_CONFIG */
> + if (!dc->info->has_config_ex)
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE |
> VSDC_FB_CONFIG_RESET);
Should VSDC_FB_CONFIG_RESET be only set when it's ready to output the
signal (at least all timing is programmed)? I think it should be
programmed in crtc/bridge instead of primary plane, although it's in
the DcregFrameBufferConfig0 register (obviously this sounds a little
weird, this might be why they changed this in DC8200).
When ENABLE (OUTPUT in the document) is cleared, all pixels will be
blacked out; so I think it's better to set ENABLE in CRTC, and then set
RESET in bridge (doing the work of encoder in this driver) -- it seems
that for DC8000/DCUltraLite the primary plane is not possible to be
disabled.
> dma_addr = vs_fb_get_dma_addr(fb, &state->src);
>
> regmap_write(dc->regs, VSDC_FB_ADDRESS(output),
> @@ -133,16 +142,18 @@ static void
> vs_primary_plane_atomic_update(struct drm_plane *plane,
> regmap_write(dc->regs, VSDC_FB_STRIDE(output),
> fb->pitches[0]);
>
> - regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
> - VSDC_MAKE_PLANE_POS(state->crtc_x, state-
> >crtc_y));
> - regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
> - VSDC_MAKE_PLANE_POS(state->crtc_x + state-
> >crtc_w,
> - state->crtc_y + state-
> >crtc_h));
> regmap_write(dc->regs, VSDC_FB_SIZE(output),
> VSDC_MAKE_PLANE_SIZE(state->crtc_w, state-
> >crtc_h));
>
> - regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> - VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
> + if (dc->info->has_config_ex) {
> + regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
> + VSDC_MAKE_PLANE_POS(state->crtc_x,
> state->crtc_y));
> + regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
> + VSDC_MAKE_PLANE_POS(state->crtc_x +
> state->crtc_w,
> + state->crtc_y +
> state->crtc_h));
> + regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> + VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
> + }
>
> vs_primary_plane_commit(dc, output);
> }
> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> index cbb125c46b39..288064760b48 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
> @@ -16,6 +16,8 @@
> #define VSDC_FB_STRIDE(n) (0x1408 + 0x4 * (n))
>
> #define VSDC_FB_CONFIG(n) (0x1518 + 0x4 * (n))
> +#define VSDC_FB_CONFIG_ENABLE BIT(0)
As I mentioned that the VALID bit is quite important, please add it
here (you can call it "COMMIT" too if you like).
#define VSDC_FB_CONFIG_VALID BIT(3)
> +#define VSDC_FB_CONFIG_RESET BIT(4)
> #define VSDC_FB_CONFIG_CLEAR_EN BIT(8)
> #define VSDC_FB_CONFIG_ROT_MASK GENMASK(13,
> 11)
> #define VSDC_FB_CONFIG_ROT(v) ((v) << 11)
next prev parent reply other threads:[~2026-05-11 9:48 UTC|newest]
Thread overview: 15+ 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-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 [this message]
2026-05-12 7:45 ` Joey Lu
2026-05-12 8:11 ` Icenowy Zheng
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 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=93e69179dbc495188cfffd8015350b3a55ce7876.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