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

[Joey Lu]

On 6/8/2026 6:06 PM, Icenowy Zheng wrote:
> 在 2026-06-08一的 17:45 +0800，Joey Lu写道：
> > > > 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
> > > > int
> > > > output)
> > > > -{
> > > > -	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
> > 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`.
> Maybe keep the `_ex` here as some operations is still on the common
> codepath?
>
> Thanks,
> Icenowy
Got it. I’ll keep the `_ex` suffix in place. That way it’s consistent 
with the common codepath operations, and the inline comments already 
makes the intent clear.
> > > >    }
> > > >    
> > > >    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-
> > > > > crtc_h));
> > > >    
> > > > -	regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> > > > -		     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_funcs
> > > > vs_primary_plane_helper_funcs = {