Hi Am 25.07.22 um 17:46 schrieb Javier Martinez Canillas:
On 7/20/22 16:27, Thomas Zimmermann wrote:Replace the simple-KMS helpers with the regular atomic helpers. The regular helpers are better architectured and therefore allow for easier code sharing among drivers. No functional changes.Acked-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> But I've a question below...Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> --- drivers/gpu/drm/tiny/simpledrm.c | 283 ++++++++++++++++++++----------- 1 file changed, 180 insertions(+), 103 deletions(-)[...]+static void simpledrm_crtc_helper_atomic_enable(struct drm_crtc *crtc, + struct drm_atomic_state *old_state) +{ + /* + * Always enabled; screen updates are performed by + * the primary plane's atomic_update function. + */ +} + +static void simpledrm_crtc_helper_atomic_disable(struct drm_crtc *crtc, + struct drm_atomic_state *old_state) +{ + /* + * Always enabled; disabling clears the screen in the + * primary plane's atomic_disable function. + */ +}...do we really need to have these ? Can't we just not set them ?+ +static const struct drm_crtc_helper_funcs simpledrm_crtc_helper_funcs = { + .mode_valid = simpledrm_crtc_helper_mode_valid, + .atomic_check = simpledrm_crtc_helper_atomic_check, + .atomic_enable = simpledrm_crtc_helper_atomic_enable, + .atomic_disable = simpledrm_crtc_helper_atomic_disable, +}; +looking at https://elixir.bootlin.com/linux/latest/source/include/drm/drm_modeset_helper_vtables.h#L703 that says the .atomic_{en,dis}able handlers are optional.
The code also looks like we don't need the helpers. I mostly added them for the comments they contain, but I can also add those next to simpledrm_crtc_helper_funcs.
Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
