Maxime Ripard <mripard@xxxxxxxxxx> writes: Hello Maxime, > Hi, > > On Wed, Sep 13, 2023 at 07:29:25AM +0200, Javier Martinez Canillas wrote: >> static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs = { >> .mode_valid = ssd130x_crtc_helper_mode_valid, >> - .atomic_check = drm_crtc_helper_atomic_check, >> + .atomic_check = ssd130x_crtc_helper_atomic_check, >> }; > > Sorry I didn't catch that sooner, but there's no reason to call that > function a helper. > Yeah, agreed that there's no reason but others drivers already add the _helper prefix for struct drm_*_helper_funcs callbacks, and I did that in this driver as well to follow (what appears to be?) a convention. So I've to that now for the struct drm_crtc_helper_funcs handlers to be consistent with the rest of the driver, e.g for plane: static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = ssd130x_primary_plane_helper_atomic_check, .atomic_update = ssd130x_primary_plane_helper_atomic_update, .atomic_disable = ssd130x_primary_plane_helper_atomic_disable, }; static const struct drm_plane_funcs ssd130x_primary_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .reset = ssd130x_primary_plane_reset, .atomic_duplicate_state = ssd130x_primary_plane_duplicate_state, .atomic_destroy_state = ssd130x_primary_plane_destroy_state, .destroy = drm_plane_cleanup, }; > With that fixed (and feel free to fix while applying) > > Acked-by: Maxime Ripard <mripard@xxxxxxxxxx> > > Maxime -- Best regards, Javier Martinez Canillas Core Platforms Red Hat