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.
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 <[email protected]>
---
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.
[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
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.
+ }
}
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?
I'll move `VSDC_DISP_PANEL_CONFIG_RUNNING` inside the
`has_config_ex` block in both the enable and disable paths. For
non-config_ex variants (DCUltra Lite), the disable path will instead
clear `VSDC_FB_CONFIG_RESET` to stall DPI output.
`VSDC_DISP_PANEL_CONFIG_RUNNING` will only be set/cleared for
DC8200 (has_config_ex=true).
- 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.
Covered by the same fix above — the else branch in
vs_bridge_atomic_disable() will handle the non-config_ex (DCUltra
Lite) path by clearing RESET.
}
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
I'll address all three issues in v2:
1. Add `VSDC_DISP_IRQ_VSYNC(n)` macro (BIT(n)) to vs_crtc_regs.h.
2. Use `VSDC_DISP_IRQ_VSYNC(vcrtc->id)` with
`regmap_set_bits`/`regmap_clear_bits` instead of hardcoding 0 and
using `regmap_write` (which would clobber other IRQ bits).
3. Add `uses_top_irq` bool to `struct vs_chip_identity` in vs_hwdb.h.
DC8200 entries will have `uses_top_irq = true`; the DCUltra Lite
entry (model=0x0, rev=0x5560) will have `uses_top_irq = false`.
+ 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.
I'll keep the name `vs_dc_regmap_cfg` — the original name from
upstream. A separate DCUltra Lite regmap config is not needed; the
hardware simply does not implement registers above 0x2000, and a regmap
window up to 0x2544 is harmless for that variant.
.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?
I'll drop `.data` from both `of_device_id` entries entirely.
`has_config_ex` and `uses_top_irq` will be added as fields to
`struct vs_chip_identity` in vs_hwdb.h, and populated for each entry
in `vs_chip_identities[]` in vs_hwdb.c. `vs_fill_chip_identity()` is
called unconditionally for all variants — chip-ID registers are present
on DCUltra Lite too. There is no need for a separate `vs_dc_info`
platform data structure.
+ { .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?
I'll add `uses_top_irq` as a bool field to `struct vs_chip_identity`
in vs_hwdb.h instead. The IRQ handler will branch on
`dc->identity.uses_top_irq` to read either `VSDC_TOP_IRQ_ACK` or
`VSDC_DISP_IRQ_STA`. No register address needs to be stored in the
hwdb struct.
+ 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.
I'll unify the IRQ handler: both paths will read their
respective IRQ status register and pass the raw `irqs` value to
`vs_drm_handle_irq()`. Since `VSDC_TOP_IRQ_VSYNC(n)` and
`VSDC_DISP_IRQ_VSYNC(n)` are both `BIT(n)`, the bit positions are
identical and `vs_drm_handle_irq()` handles both without change.
+ }
+
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.
I'll remove the family-gated block entirely. Resets will be
acquired unconditionally (they are optional, so absent ones are
skipped). axi and ahb clocks will use `devm_clk_get_optional_enabled()`
so they are silently skipped when not present in DT. The probe path
will be uniform for all variants.
}
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.
You are right. I'll revert to 8192x8192 for all variants. The
variant-specific max_width/max_height logic will be removed entirely
and vs_mode_config_init() will take no dc argument.
+ } 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 ?
Same as reply [a]. No `VSDC_FB_CONFIG_VALID` macro is needed.
For non-config_ex variants, `vs_primary_plane_commit()` is a no-op —
`VSDC_FB_CONFIG_ENABLE` (OUTPUT, bit 0) is set in `vs_crtc_atomic_enable()`
and `VSDC_FB_CONFIG_RESET` (bit 4) is managed in the bridge enable/disable
paths.
}
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.
I'll move these as suggested:
- `VSDC_FB_CONFIG_ENABLE` will be set in `vs_crtc_atomic_enable()` for
non-config_ex variants (after the pixel clock is enabled).
- `VSDC_FB_CONFIG_RESET` will be set in `vs_bridge_enable_common()` for
non-config_ex variants, after all timing registers are programmed.
- `VSDC_FB_CONFIG_RESET` will be cleared in `vs_bridge_atomic_disable()`
for non-config_ex variants to stall DPI output.
- The ENABLE/RESET writes will be removed from
`vs_primary_plane_atomic_update()`.
- `vs_primary_plane_commit()` will perform no action for non-config_ex
variants; the function only triggers `VSDC_FB_CONFIG_EX_COMMIT` for
config_ex variants.
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)
Same as reply [a]. No `VSDC_FB_CONFIG_VALID` macro is needed.
Thank you very much for taking the time to review my work. 🙂
+#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)
