Hi

Am 16.07.26 um 11:01 schrieb Ze Huang:
Convert gm12u320 to direct primary plane, CRTC and encoder setup.

Keep shadow-plane helper state, framebuffer access helpers and
no-scaling plane-state check from simple-KMS path.

Reviewed-by: Thomas Zimmermann <[email protected]>
Tested-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Ze Huang <[email protected]>
---
  drivers/gpu/drm/tiny/gm12u320.c | 132 ++++++++++++++++++++++++++++++++--------
  1 file changed, 107 insertions(+), 25 deletions(-)

[...]
-static void gm12u320_pipe_enable(struct drm_simple_display_pipe *pipe,
-                                struct drm_crtc_state *crtc_state,
-                                struct drm_plane_state *plane_state)
+static void gm12u320_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+                                              struct drm_atomic_commit *commit)
  {
        struct drm_rect rect = { 0, 0, GM12U320_USER_WIDTH, GM12U320_HEIGHT };
-       struct gm12u320_device *gm12u320 = to_gm12u320(pipe->crtc.dev);
+       struct gm12u320_device *gm12u320 = to_gm12u320(crtc->dev);
+       struct drm_plane_state *plane_state = gm12u320->plane.state;

Did you see the reply from the Sashiko bot?

What happens is that user space can apply multiple atomic commits in a row, but they are applied to hardware asynchronously. So if you take the plane state here directly from the plane, it could have been replaced by a later atomic commit already.  Rather get the correct plane state with the helper drm_atomic_get_new_plane_state(). I did not look at all of the series' patches for this problem, but the rule applies to all of the mode-setting code. Best regards Thomas
        struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(plane_state);
gm12u320->fb_update.draw_status_timeout = FIRST_FRAME_TIMEOUT;
        gm12u320_fb_mark_dirty(plane_state->fb, &shadow_plane_state->data[0], 
&rect);
  }
-static void gm12u320_pipe_disable(struct drm_simple_display_pipe *pipe)
+static void gm12u320_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+                                               struct drm_atomic_commit 
*commit)
  {
-       struct gm12u320_device *gm12u320 = to_gm12u320(pipe->crtc.dev);
+       struct gm12u320_device *gm12u320 = to_gm12u320(crtc->dev);
gm12u320_stop_fb_update(gm12u320);
  }
-static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
-                                struct drm_plane_state *old_state)
+static void gm12u320_plane_helper_atomic_update(struct drm_plane *plane,
+                                               struct drm_atomic_commit 
*commit)
  {
-       struct drm_plane_state *state = pipe->plane.state;
+       struct drm_plane_state *old_state = 
drm_atomic_get_old_plane_state(commit, plane);
+       struct drm_plane_state *state = drm_atomic_get_new_plane_state(commit, 
plane);
        struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
        struct drm_rect rect;
+ if (!state->fb)
+               return;
+
        if (drm_atomic_helper_damage_merged(old_state, state, &rect))
                gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], 
&rect);
  }
-static const struct drm_simple_display_pipe_funcs gm12u320_pipe_funcs = {
-       .enable     = gm12u320_pipe_enable,
-       .disable    = gm12u320_pipe_disable,
-       .update     = gm12u320_pipe_update,
-       DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
+static const struct drm_plane_funcs gm12u320_plane_funcs = {
+       .update_plane   = drm_atomic_helper_update_plane,
+       .disable_plane  = drm_atomic_helper_disable_plane,
+       .destroy        = drm_plane_cleanup,
+       DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static int gm12u320_plane_helper_atomic_check(struct drm_plane *plane,
+                                             struct drm_atomic_commit *commit)
+{
+       struct drm_plane_state *plane_state = 
drm_atomic_get_new_plane_state(commit, plane);
+       struct drm_crtc *crtc = plane_state->crtc;
+       struct drm_crtc_state *crtc_state = NULL;
+
+       if (crtc)
+               crtc_state = drm_atomic_get_new_crtc_state(commit, crtc);
+
+       return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+                                                  DRM_PLANE_NO_SCALING,
+                                                  DRM_PLANE_NO_SCALING,
+                                                  false, false);
+}
+
+static const struct drm_plane_helper_funcs gm12u320_plane_helper_funcs = {
+       DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+       .atomic_check   = gm12u320_plane_helper_atomic_check,
+       .atomic_update  = gm12u320_plane_helper_atomic_update,
+};
+
+static int gm12u320_crtc_helper_atomic_check(struct drm_crtc *crtc,
+                                            struct drm_atomic_commit *commit)
+{
+       struct drm_crtc_state *crtc_state = 
drm_atomic_get_new_crtc_state(commit, crtc);
+       int ret;
+
+       if (crtc_state->enable) {
+               ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+               if (ret)
+                       return ret;
+       }
+
+       return drm_atomic_add_affected_planes(commit, crtc);
+}
+
+static const struct drm_crtc_helper_funcs gm12u320_crtc_helper_funcs = {
+       .atomic_check   = gm12u320_crtc_helper_atomic_check,
+       .atomic_enable  = gm12u320_crtc_helper_atomic_enable,
+       .atomic_disable = gm12u320_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs gm12u320_crtc_funcs = {
+       .set_config             = drm_atomic_helper_set_config,
+       .page_flip              = drm_atomic_helper_page_flip,
+       .reset                  = drm_atomic_helper_crtc_reset,
+       .destroy                = drm_crtc_cleanup,
+       .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+       .atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_encoder_funcs gm12u320_encoder_funcs = {
+       .destroy = drm_encoder_cleanup,
  };
static const uint32_t gm12u320_pipe_formats[] = {
@@ -677,13 +743,29 @@ static int gm12u320_usb_probe(struct usb_interface 
*interface,
        if (ret)
                return ret;
- ret = drm_simple_display_pipe_init(&gm12u320->dev,
-                                          &gm12u320->pipe,
-                                          &gm12u320_pipe_funcs,
-                                          gm12u320_pipe_formats,
-                                          ARRAY_SIZE(gm12u320_pipe_formats),
-                                          gm12u320_pipe_modifiers,
-                                          &gm12u320->conn);
+       ret = drm_universal_plane_init(dev, &gm12u320->plane, 0,
+                                      &gm12u320_plane_funcs,
+                                      gm12u320_pipe_formats,
+                                      ARRAY_SIZE(gm12u320_pipe_formats),
+                                      gm12u320_pipe_modifiers,
+                                      DRM_PLANE_TYPE_PRIMARY, NULL);
+       if (ret)
+               return ret;
+       drm_plane_helper_add(&gm12u320->plane, &gm12u320_plane_helper_funcs);
+
+       ret = drm_crtc_init_with_planes(dev, &gm12u320->crtc, &gm12u320->plane, 
NULL,
+                                       &gm12u320_crtc_funcs, NULL);
+       if (ret)
+               return ret;
+       drm_crtc_helper_add(&gm12u320->crtc, &gm12u320_crtc_helper_funcs);
+
+       ret = drm_encoder_init(dev, &gm12u320->encoder, &gm12u320_encoder_funcs,
+                              DRM_MODE_ENCODER_NONE, NULL);
+       if (ret)
+               return ret;
+       gm12u320->encoder.possible_crtcs = drm_crtc_mask(&gm12u320->crtc);
+
+       ret = drm_connector_attach_encoder(&gm12u320->conn, &gm12u320->encoder);
        if (ret)
                return ret;

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)


Reply via email to