Hi Javier Am 31.08.22 um 13:12 schrieb Javier Martinez Canillas:
The simpledrm_primary_plane_helper_atomic_check() function is more complex than needed. It first checks drm_atomic_helper_check_plane_state() returns value to decide whether to return this or zero. But it could just return that function return value directly. It also does a check if new_plane_state->visible isn't set, but returns zero regardless. Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> --- drivers/gpu/drm/tiny/simpledrm.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index a81f91814595..0be47f40247a 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -485,21 +485,14 @@ static int simpledrm_primary_plane_helper_atomic_check(struct drm_plane *plane, struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(new_state, plane); struct drm_crtc *new_crtc = new_plane_state->crtc; struct drm_crtc_state *new_crtc_state = NULL; - int ret;if (new_crtc)new_crtc_state = drm_atomic_get_new_crtc_state(new_state, new_crtc);- ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,- DRM_PLANE_NO_SCALING, - DRM_PLANE_NO_SCALING, - false, false); - if (ret) - return ret; - else if (!new_plane_state->visible) - return 0; - - return 0; + return drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state, + DRM_PLANE_NO_SCALING, + DRM_PLANE_NO_SCALING, + false, false);
I'm undecided on this change. I know it's correct and more to the point. But the call's logic is non-intuitive: the call either returns an error or we have to test ->visible afterwards. So I wrote it explicitly.
I saw that your change to ssd130x also uses the pattern. If we find more such drivers, we could implement the atomic check as a helper. I suggest drm_plane_helper_atomic_check_fixed() in drm_plane_helper.c
Best regards Thomas
}static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
-- 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
