On 6/8/2026 2:24 PM, Icenowy Zheng wrote:
Understood. I will rename `bridge_enable`/`bridge_disable` to `panel_enable_ex`/`panel_disable_ex` throughout: in `vs_dc_funcs`, `vs_dc8200.c`, `vs_dc8000.c`, and the call sites in `vs_bridge.c`.在 2026-06-08一的 10:32 +0800，Joey Lu写道：The DC8200 and DCUltraLite share a broadly similar register layout but differ in how the bridge, CRTC, primary plane and IRQ paths are driven. Introduce a vs_dc_funcs vtable so each variant can supply its own implementation without scattering conditionals across multiple files. Add enum vs_dc_generation (VSDC_GEN_DC8000 / VSDC_GEN_DC8200) to vs_hwdb.h and a generation field to struct vs_chip_identity. Annotate all four existing DC8200 HWDB entries with VSDC_GEN_DC8200. Extract the DC8200-specific hardware ops into a new vs_dc8200.c: bridge_enable / bridge_disable - PANEL_CONFIG/START + CONFIG_EX commit enable_vblank / disable_vblank - TOP_IRQ_EN VSYNC bit plane_enable_ex / disable_ex / update_ex - FB_CONFIG_EX path irq_handler - reads TOP_IRQ_ACK Update vs_bridge.c, vs_crtc.c, vs_primary_plane.c and vs_dc.c to dispatch through dc->funcs instead of directly touching registers. vs_crtc.c gains atomic_begin and atomic_flush hooks to allow variants to gate per-frame commit cycles. No behaviour change for existing DC8200 platforms. Signed-off-by: Joey Lu <[email protected]> --- drivers/gpu/drm/verisilicon/Makefile | 2 +- drivers/gpu/drm/verisilicon/vs_bridge.c | 20 +--- drivers/gpu/drm/verisilicon/vs_crtc.c | 38 ++++++- drivers/gpu/drm/verisilicon/vs_dc.c | 6 +- drivers/gpu/drm/verisilicon/vs_dc.h | 33 ++++++ drivers/gpu/drm/verisilicon/vs_dc8200.c | 107 ++++++++++++++++++ drivers/gpu/drm/verisilicon/vs_hwdb.c | 4 + drivers/gpu/drm/verisilicon/vs_hwdb.h | 6 + .../gpu/drm/verisilicon/vs_primary_plane.c | 32 +----- 9 files changed, 197 insertions(+), 51 deletions(-) create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c============ 8< ==================diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c b/drivers/gpu/drm/verisilicon/vs_bridge.c index 7a93049368db..6a9af10c64e6 100644 --- a/drivers/gpu/drm/verisilicon/vs_bridge.c +++ b/drivers/gpu/drm/verisilicon/vs_bridge.c @@ -162,15 +162,8 @@ static void vs_bridge_enable_common(struct vs_crtc *crtc, VSDC_DISP_PANEL_CONFIG_DE_EN | VSDC_DISP_PANEL_CONFIG_DAT_EN | 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); + + dc->funcs->bridge_enable(dc, output);The code here being called "bridge" is only internal to kernel. Naming it in such a way is okay, but maybe naming it "panel" is better (because they're configuring PANEL-named registers). And, as the common code setting common fields of DcregPanelConfig0 is still here, maybe the helper name should be named "panel_enable_ex" (or "bridge_enable_ex") ?
Understood. I will rename `irq_handler` to `irq_ack` in `vs_dc_funcs`, `vs_dc8200.c`, `vs_dc8000.c`, and the call site in `vs_dc.c`.}static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {====== 8< ==============diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c b/drivers/gpu/drm/verisilicon/vs_dc.c index dad9967bc10b..c94957024189 100644 --- a/drivers/gpu/drm/verisilicon/vs_dc.c +++ b/drivers/gpu/drm/verisilicon/vs_dc.c @@ -8,9 +8,7 @@ #include <linux/of.h> #include <linux/of_graph.h>-#include "vs_crtc.h"#include "vs_dc.h" -#include "vs_dc_top_regs.h" #include "vs_drm.h" #include "vs_hwdb.h"@@ -33,7 +31,7 @@ static irqreturn_t vs_dc_irq_handler(int irq, void*private) struct vs_dc *dc = private; u32 irqs;- regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);+ irqs = dc->funcs->irq_handler(dc);The IRQ isn't handled in this helper. So maybe call it "irq_ack"?
Understood. I will rename all vtable members per the comments above: `panel_enable_ex`, `panel_disable_ex`, `primary_plane_enable`, `primary_plane_disable`, `primary_plane_update`, `irq_ack`.vs_drm_handle_irq(dc, irqs); @@ -136,6 +134,8 @@ static int vs_dc_probe(struct platform_device*pdev) dev_info(dev, "Found DC%x rev %x customer %x\n", dc-identity.model,dc->identity.revision, dc->identity.customer_id);+ dc->funcs = &vs_dc8200_funcs;+ if (port_count > dc->identity.display_count) { dev_err(dev, "too many downstream ports than HW capability\n"); ret = -EINVAL; diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h b/drivers/gpu/drm/verisilicon/vs_dc.h index ed1016f18758..d77d4a1babdf 100644 --- a/drivers/gpu/drm/verisilicon/vs_dc.h +++ b/drivers/gpu/drm/verisilicon/vs_dc.h @@ -14,6 +14,7 @@ #include <linux/reset.h>#include <drm/drm_device.h>+#include <drm/drm_plane.h>#include "vs_hwdb.h" @@ -22,6 +23,34 @@ struct vs_drm_dev;struct vs_crtc; +struct vs_dc; + +struct vs_dc_funcs { + /* Bridge: atomic_enable, atomic_disable */ + void (*bridge_enable)(struct vs_dc *dc, unsigned int output); + void (*bridge_disable)(struct vs_dc *dc, unsigned int output); + + /* CRTC: atomic_begin, atomic_flush */ + void (*crtc_begin)(struct vs_dc *dc, unsigned int output); + void (*crtc_flush)(struct vs_dc *dc, unsigned int output); + + /* CRTC: atomic_enable, atomic_disable */ + void (*crtc_enable)(struct vs_dc *dc, unsigned int output); + void (*crtc_disable)(struct vs_dc *dc, unsigned int output); + + /* CRTC: enable_vblank, disable_vblank */ + void (*enable_vblank)(struct vs_dc *dc, unsigned int output); + void (*disable_vblank)(struct vs_dc *dc, unsigned int output); + + /* Primary plane: atomic_enable, atomic_disable, atomic_update */ + void (*plane_enable_ex)(struct vs_dc *dc, unsigned int output); + void (*plane_disable_ex)(struct vs_dc *dc, unsigned int output); + void (*plane_update_ex)(struct vs_dc *dc, unsigned int output, + struct drm_plane_state *state); + + /* IRQ handler */ + u32 (*irq_handler)(struct vs_dc *dc);See my comments elsewhere for the helper naming.
Understood. To avoid confusion, I will rename `plane_enable_ex`, `plane_disable_ex`, and `plane_update_ex` to `primary_plane_enable`, `primary_plane_disable`, and `primary_plane_update` in `vs_dc_funcs`, `vs_dc8200.c`, and `vs_primary_plane.c`.+};struct vs_dc {struct regmap *regs;============= 8< =================diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c b/drivers/gpu/drm/verisilicon/vs_hwdb.c index 2a0f7c59afa3..91524d16f778 100644 --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c @@ -94,6 +94,7 @@ static struct vs_chip_identity vs_chip_identities[] = { .revision = 0x5720, .customer_id = ~0U,+ .generation = VSDC_GEN_DC8200,.display_count = 2, .max_cursor_size = 64, .formats = &vs_formats_no_yuv444, @@ -103,6 +104,7 @@ static struct vs_chip_identity vs_chip_identities[] = { .revision = 0x5721, .customer_id = 0x30B,+ .generation = VSDC_GEN_DC8200,.display_count = 2, .max_cursor_size = 64, .formats = &vs_formats_no_yuv444, @@ -112,6 +114,7 @@ static struct vs_chip_identity vs_chip_identities[] = { .revision = 0x5720, .customer_id = 0x310,+ .generation = VSDC_GEN_DC8200,.display_count = 2, .max_cursor_size = 64, .formats = &vs_formats_with_yuv444, @@ -121,6 +124,7 @@ static struct vs_chip_identity vs_chip_identities[] = { .revision = 0x5720, .customer_id = 0x311,+ .generation = VSDC_GEN_DC8200,.display_count = 2, .max_cursor_size = 64, .formats = &vs_formats_no_yuv444, diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h b/drivers/gpu/drm/verisilicon/vs_hwdb.h index 2065ecb73043..a15c8b565604 100644 --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h @@ -9,6 +9,11 @@ #include <linux/regmap.h> #include <linux/types.h>+enum vs_dc_generation {+ VSDC_GEN_DC8000, + VSDC_GEN_DC8200, +}; + struct vs_formats { const u32 *array; unsigned int num; @@ -19,6 +24,7 @@ struct vs_chip_identity { u32 revision; u32 customer_id;+ enum vs_dc_generation generation;u32 display_count; /* * The hardware only supports square cursor planes, so this field diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c index 1f2be41ae496..75bc36a078f7 100644 --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c @@ -53,12 +53,6 @@ static int vs_primary_plane_atomic_check(struct drm_plane *plane, return 0; }-static void vs_primary_plane_commit(struct vs_dc *dc, unsigned intoutput) -{ - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output), - VSDC_FB_CONFIG_EX_COMMIT); -} - static void vs_primary_plane_atomic_enable(struct drm_plane *plane, struct drm_atomic_commit *atomic_state) { @@ -69,13 +63,8 @@ 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)); - - vs_primary_plane_commit(dc, output); + if (dc->funcs->plane_enable_ex) + dc->funcs->plane_enable_ex(dc, output);Please note that all theae codes are for primary planes, maybe the helper should be named mentioning primary. Overlay planes will need a different codepath because they change different registers. Thanks, Icenowy
}static void vs_primary_plane_atomic_disable(struct drm_plane *plane,@@ -88,10 +77,8 @@ 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); - - vs_primary_plane_commit(dc, output); + if (dc->funcs->plane_disable_ex) + dc->funcs->plane_disable_ex(dc, output); }static void vs_primary_plane_atomic_update(struct drm_plane *plane,@@ -133,18 +120,11 @@ 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-- regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),crtc_h));- VSDC_FB_BLEND_CONFIG_BLEND_DISABLE); - - vs_primary_plane_commit(dc, output); + if (dc->funcs->plane_update_ex) + dc->funcs->plane_update_ex(dc, output, state); }static const struct drm_plane_helper_funcsvs_primary_plane_helper_funcs = {
