Re: [PATCH v7 7/7] drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

[Icenowy Zheng <zhengxingda@iscas.ac.cn>]

在 2026-09-21一的 10:43 +0800，Joey Lu写道：
> 
> Icenowy Zheng 於 2026/9/18 下午 01:52 寫道:
> > 在 2026-09-18五的 11:01 +0800，Joey Lu写道：
> > > vs_dc8200_primary_plane_disable_ex() calls regmap_set_bits() on
> > > VSDC_FB_CONFIG_EX_FB_EN instead of regmap_clear_bits(), so
> > > disabling
> > > the primary plane on DC8200-family hardware actually leaves the
> > > framebuffer enable bit instead of clearing it.
> > > 
> > > This bug predates this series: it was carried over unchanged from
> > > vs_primary_plane_atomic_disable() when patch "drm/verisilicon:
> > > introduce per-variant hardware ops table" split the DC8200-
> > > specific
> > > implementation out into vs_dc8200.c.
> > > 
> > > Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for
> > > Verisilicon
> > > display controllers")
> > Maybe it'd be better to fix this before adding DC variant
> > abstraction,
> > for easier backporting.
> > 
> > Thanks,
> > Icenowy
> Makes sense. Should I send it as a standalone patch targeting
> drm-misc-fixes (separate from this series), or do you have a
> different preference for how to split it?

I prefer a standalone patch, although changing the patch order to
before other drm/verisilicon changes is also okay.

In fact there's another point that seems to be fixed, and that fix
should be more easier with this patchset --
vs_primary_plane_atomic_update() (which operates with new plane state)
calls vs_primary_plane_atomic_disable() (which operates with old plane
state). With the refactor in this patchset, both calls can be directed
into the primary_plane_disable_ex callback (or maybe _ex could be
removed because no further hardware operation is present).

Thanks,
Icenowy

> > 
> > > Signed-off-by: Joey Lu <a0987203069@gmail.com>
> > > ---
> > >   drivers/gpu/drm/verisilicon/vs_dc8200.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c
> > > b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> > > index f72da10295e1b..25b5906a1c5fc 100644
> > > --- a/drivers/gpu/drm/verisilicon/vs_dc8200.c
> > > +++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> > > @@ -70,8 +70,8 @@ static void
> > > vs_dc8200_primary_plane_enable_ex(struct vs_dc *dc, unsigned int
> > > out
> > >   
> > >   static void vs_dc8200_primary_plane_disable_ex(struct vs_dc
> > > *dc,
> > > unsigned int output)
> > >   {
> > > -	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> > > -			VSDC_FB_CONFIG_EX_FB_EN);
> > > +	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> > > +			  VSDC_FB_CONFIG_EX_FB_EN);
> > >   
> > >   	vs_dc8200_plane_commit(dc, output);
> > >   }