Message ID | 1487256430-7625-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:07 Maarten Lankhorst wrote: > This function becomes a lot simpler when having passed both the old and > new state to it. Looking at all callers, it seems that old_plane_state > is never NULL so the check can be dropped. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 7 ++++--- > drivers/gpu/drm/drm_plane_helper.c | 2 +- > include/drm/drm_atomic_helper.h | 26 ++++++++------------------ > 3 files changed, 13 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 7d432d9a18cf..ea544bddc29b > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device > *dev, if (!funcs) > continue; > > - disabling = drm_atomic_plane_disabling(plane, old_plane_state); > + disabling = drm_atomic_plane_disabling(old_plane_state, > + new_plane_state); > > if (active_only) { > /* > @@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct > drm_crtc_state *old_crtc_state) > > WARN_ON(plane->state->crtc && plane->state->crtc != crtc); > > - if (drm_atomic_plane_disabling(plane, old_plane_state) && > + if (drm_atomic_plane_disabling(old_plane_state, plane->state) && > plane_funcs->atomic_disable) > plane_funcs->atomic_disable(plane, old_plane_state); > else if (plane->state->crtc || > - drm_atomic_plane_disabling(plane, old_plane_state)) > + drm_atomic_plane_disabling(old_plane_state, plane- >state)) > plane_funcs->atomic_update(plane, old_plane_state); > } > > diff --git a/drivers/gpu/drm/drm_plane_helper.c > b/drivers/gpu/drm/drm_plane_helper.c index 148688fb920a..f4d70dd5e3e4 > 100644 > --- a/drivers/gpu/drm/drm_plane_helper.c > +++ b/drivers/gpu/drm/drm_plane_helper.c > @@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane, > * Drivers may optionally implement the ->atomic_disable callback, so > * special-case that here. > */ > - if (drm_atomic_plane_disabling(plane, plane_state) && > + if (drm_atomic_plane_disabling(plane_state, plane->state) && > plane_funcs->atomic_disable) > plane_funcs->atomic_disable(plane, plane_state); > else > diff --git a/include/drm/drm_atomic_helper.h > b/include/drm/drm_atomic_helper.h index 9ceda379ce58..dc16274987c7 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc > *crtc, __drm_atomic_get_current_plane_state((crtc_state)->state, \ plane))) > > -/* > +/** > * drm_atomic_plane_disabling - check whether a plane is being disabled > - * @plane: plane object > - * @old_state: previous atomic state > + * @old_plane_state: old atomic plane state > + * @new_plane_state: new atomic plane state > * > * Checks the atomic state of a plane to determine whether it's being > disabled * or not. This also WARNs if it detects an invalid state (both > CRTC and FB @@ -233,28 +233,18 @@ int > drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, * True if the > plane is being disabled, false otherwise. > */ > static inline bool > -drm_atomic_plane_disabling(struct drm_plane *plane, > - struct drm_plane_state *old_state) > +drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state, > + struct drm_plane_state *new_plane_state) > { > /* > * When disabling a plane, CRTC and FB should always be NULL together. > * Anything else should be considered a bug in the atomic core, so we > * gently warn about it. > */ > - WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) || > - (plane->state->crtc != NULL && plane->state->fb == NULL)); > + WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) || > + (new_plane_state->crtc != NULL && new_plane_state->fb == NULL)); > > - /* > - * When using the transitional helpers, old_state may be NULL. If so, > - * we know nothing about the current state and have to assume that it > - * might be enabled. > - * > - * When using the atomic helpers, old_state won't be NULL. Therefore > - * this check assumes that either the driver will have reconstructed > - * the correct state in ->reset() or that the driver will have taken > - * appropriate measures to disable all planes. > - */ > - return (!old_state || old_state->crtc) && !plane->state->crtc; > + return old_plane_state->crtc && !new_plane_state->crtc; > } > > #endif /* DRM_ATOMIC_HELPER_H_ */
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 7d432d9a18cf..ea544bddc29b 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs) continue; - disabling = drm_atomic_plane_disabling(plane, old_plane_state); + disabling = drm_atomic_plane_disabling(old_plane_state, + new_plane_state); if (active_only) { /* @@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state) WARN_ON(plane->state->crtc && plane->state->crtc != crtc); - if (drm_atomic_plane_disabling(plane, old_plane_state) && + if (drm_atomic_plane_disabling(old_plane_state, plane->state) && plane_funcs->atomic_disable) plane_funcs->atomic_disable(plane, old_plane_state); else if (plane->state->crtc || - drm_atomic_plane_disabling(plane, old_plane_state)) + drm_atomic_plane_disabling(old_plane_state, plane->state)) plane_funcs->atomic_update(plane, old_plane_state); } diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 148688fb920a..f4d70dd5e3e4 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane, * Drivers may optionally implement the ->atomic_disable callback, so * special-case that here. */ - if (drm_atomic_plane_disabling(plane, plane_state) && + if (drm_atomic_plane_disabling(plane_state, plane->state) && plane_funcs->atomic_disable) plane_funcs->atomic_disable(plane, plane_state); else diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 9ceda379ce58..dc16274987c7 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, __drm_atomic_get_current_plane_state((crtc_state)->state, \ plane))) -/* +/** * drm_atomic_plane_disabling - check whether a plane is being disabled - * @plane: plane object - * @old_state: previous atomic state + * @old_plane_state: old atomic plane state + * @new_plane_state: new atomic plane state * * Checks the atomic state of a plane to determine whether it's being disabled * or not. This also WARNs if it detects an invalid state (both CRTC and FB @@ -233,28 +233,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, * True if the plane is being disabled, false otherwise. */ static inline bool -drm_atomic_plane_disabling(struct drm_plane *plane, - struct drm_plane_state *old_state) +drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state, + struct drm_plane_state *new_plane_state) { /* * When disabling a plane, CRTC and FB should always be NULL together. * Anything else should be considered a bug in the atomic core, so we * gently warn about it. */ - WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) || - (plane->state->crtc != NULL && plane->state->fb == NULL)); + WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) || + (new_plane_state->crtc != NULL && new_plane_state->fb == NULL)); - /* - * When using the transitional helpers, old_state may be NULL. If so, - * we know nothing about the current state and have to assume that it - * might be enabled. - * - * When using the atomic helpers, old_state won't be NULL. Therefore - * this check assumes that either the driver will have reconstructed - * the correct state in ->reset() or that the driver will have taken - * appropriate measures to disable all planes. - */ - return (!old_state || old_state->crtc) && !plane->state->crtc; + return old_plane_state->crtc && !new_plane_state->crtc; } #endif /* DRM_ATOMIC_HELPER_H_ */
This function becomes a lot simpler when having passed both the old and new state to it. Looking at all callers, it seems that old_plane_state is never NULL so the check can be dropped. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 7 ++++--- drivers/gpu/drm/drm_plane_helper.c | 2 +- include/drm/drm_atomic_helper.h | 26 ++++++++------------------ 3 files changed, 13 insertions(+), 22 deletions(-)