Re: [PATCH v4 3/6] drm/verisilicon: introduce per-variant hardware ops table

[Joey Lu]

On 6/15/2026 4:37 PM, Icenowy Zheng wrote:
> 在 2026-06-15一的 14:50 +0800，Joey Lu写道：
>
> > +
> >   	drm_crtc_vblank_on(crtc);
> >   }
> >   
> > @@ -119,7 +148,8 @@ static bool vs_crtc_mode_fixup(struct drm_crtc
> > *crtc,
> >   }
> >   
> >   static const struct drm_crtc_helper_funcs vs_crtc_helper_funcs = {
> > -	.atomic_flush	= drm_crtc_vblank_atomic_flush,
> > +	.atomic_begin	= vs_crtc_atomic_begin,
> > +	.atomic_flush	= vs_crtc_atomic_flush,
> >   	.atomic_enable	= vs_crtc_atomic_enable,
> >   	.atomic_disable	= vs_crtc_atomic_disable,
> >   	.mode_set_nofb	= vs_crtc_mode_set_nofb,
> > @@ -132,7 +162,7 @@ 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));
> > +	dc->funcs->enable_vblank(dc, vcrtc->id);
> >   
> >   	return 0;
> >   }
> > @@ -142,7 +172,7 @@ 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));
> > +	dc->funcs->disable_vblank(dc, 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..9729b693d360 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_ack(dc);
> The definition of bits in 0x0010 seems to be different to ones in
> 0x147C, although the first 2 bits are the same. (e.g. `BIT(2)` is
> "cursor interrupt" in DC8000 0x147C but "secure reset done" in DC8200
> 0x0010).
>
> Should some kind of translation be done in `irq_ack` ? (and add a
> unified interrupt definition that aims to be the translation target).
>
> Thanks,
> Icenowy
I will add a set of unified IRQ bit definitions (e.g. 
`VSDC_IRQ_VSYNC(n)`) in a shared header. Each `irq_ack` implementation 
will translate its hardware-specific register bits into those unified 
bits before returning, so `vs_drm_handle_irq` can use the unified 
definitions without knowing which register layout was used. Currently 
the VSYNC bits (BIT(0)/BIT(1)) coincide between DC8200 0x0010 and DC8000 
0x147C, so the translation is trivial; this change will prevent future 
confusion from diverging bit meanings (such as BIT(2)).
> >   
> >   	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..544e1a37065b 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 (*panel_enable_ex)(struct vs_dc *dc, unsigned int
> > output);
> > +	void (*panel_disable_ex)(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 (*primary_plane_enable_ex)(struct vs_dc *dc, unsigned
> > int output);
> > +	void (*primary_plane_disable_ex)(struct vs_dc *dc, unsigned
> > int output);
> > +	void (*primary_plane_update_ex)(struct vs_dc *dc, unsigned
> > int output,
> > +					struct drm_plane_state
> > *state);
> > +
> > +	/* IRQ acknowledge */
> > +	u32 (*irq_ack)(struct vs_dc *dc);
> > +};
> >   
> >   struct vs_dc {
> >   	struct regmap *regs;
> > @@ -33,6 +62,9 @@ struct vs_dc {
> >   
> >   	struct vs_drm_dev *drm_dev;
> >   	struct vs_chip_identity identity;
> > +	const struct vs_dc_funcs *funcs;
> >   };
> >   
> > +extern const struct vs_dc_funcs vs_dc8200_funcs;
> > +
> >   #endif /* _VS_DC_H_ */