Messages in this thread | | | Date | Thu, 14 Sep 2023 16:15:53 +0200 | From | Maxime Ripard <> | Subject | Re: [PATCH v4] drm/ssd130x: Store the HW buffer in the driver-private CRTC state |
| |
On Thu, Sep 14, 2023 at 03:23:53PM +0200, Javier Martinez Canillas wrote: > Maxime Ripard <mripard@kernel.org> 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.
From a quick grep, it looks like it's the exception rather than the norm
> 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, > };
Ack.
I still believe we should be removing the helper part, those are not helpers. But it's not a big deal anyway.
Maxime [unhandled content-type:application/pgp-signature] | |