On Mon, Nov 23, 2015 at 12:44:35PM +0200, Jyri Sarha wrote: > Add drm_atomic_helper_disable_planes_on_crtc() for disabling all > planes associated with the given CRTC. This can be used for instance > in the CRTC helper disable callback to disable all planes before > shutting down the display pipeline. > > Signed-off-by: Jyri Sarha <jsarha@xxxxxx> > --- > This a follow version to "[PATCH RFC] drm/crtc_helper: Add > drm_crtc_helper_disable_planes()"-patch [1], with Daniel Vetter's > review comments [2] implemented. Hope I got everything right this > time. Thanks a lot for the prompt review. When resending a patch please add a revision log to remind reviewers of what you changed. Otherwise they have to dig out the old thread again to figure that out. E.g. v2 per Daniel's feedback: - Improve kerneldoc. - WARN_ON when atomic_disable is missing. - Also use crtc_funcs->atomic_begin/atomic_flush optionally. > > Best regards, > Jyri > > [1] http://www.spinics.net/lists/dri-devel/msg94720.html > [2] http://www.spinics.net/lists/dri-devel/msg94722.html > > drivers/gpu/drm/drm_atomic_helper.c | 52 +++++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic_helper.h | 2 ++ > 2 files changed, 54 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index a53ed7a..229c810 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1281,6 +1281,58 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, > EXPORT_SYMBOL(drm_atomic_helper_commit_planes); > > /** > + * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's planes > + * @crtc: CRTC > + * @atomic: if set, synchronize with CRTC's atomic_begin/flush hooks > + * > + * Disables all planes associated with the given CRTC. This can be > + * used for instance in the CRTC helper disable callback to disable > + * all planes before shutting down the display pipeline. > + * > + * If there are planes to disable and the atomic-parameter is set the > + * function calls the CRTC's atomic_begin hook before and atomic_flush > + * hook after disabling the planes. > + * > + * It is a bug to call this function without having implemented the > + * ->atomic_disable() plane hook. > + */ > +void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc, > + bool atomic) > +{ > + const struct drm_crtc_helper_funcs *crtc_funcs = > + crtc->helper_private; > + struct drm_plane *plane; > + bool flush = false; > + > + if (!crtc_funcs || !crtc_funcs->atomic_begin) > + atomic = false; > + > + drm_for_each_plane(plane, crtc->dev) { > + const struct drm_plane_helper_funcs *plane_funcs = > + plane->helper_private; > + > + if (plane->state->crtc != crtc || !plane_funcs) > + continue; > + > + if (atomic && !flush) { > + crtc_funcs->atomic_begin(crtc, NULL); > + flush = true; I think it's clearer if you do that upfront with a simple if (crtc_funcs->atomic_begin && atomic) crtc_funcs->atomic_begin(); > + } > + > + WARN_ON(!plane_funcs->atomic_disable); > + if (plane_funcs->atomic_disable) > + plane_funcs->atomic_disable(plane, NULL); > + } > + > + if (flush) { > + WARN_ON(!crtc_funcs->atomic_flush); > + if (crtc_funcs->atomic_flush) > + crtc_funcs->atomic_flush(crtc, NULL); > + } Same here below. Imo the tracking you do in flush isn't required, since drivers can opt to not implement either atomic_begin or atomic_flush (on some hw you only need atomic_flush, and your code would break such a driver here). In short the code flow for atomic_begin/flush should look exactly like in drm_atomic_helper_commit_planes_on_crtc, except for the added && atomic check. Otherwise looks good. Cheers, Daniel > +} > +EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc); > + > +/** > * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc > * @old_crtc_state: atomic state object with the old crtc state > * > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 8cba54a..b7d4237 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -62,6 +62,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, > void drm_atomic_helper_cleanup_planes(struct drm_device *dev, > struct drm_atomic_state *old_state); > void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state); > +void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc, > + bool atomic); > > void drm_atomic_helper_swap_state(struct drm_device *dev, > struct drm_atomic_state *state); > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
