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.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
