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 = {