On Fri, Nov 20, 2015 at 12:03:41PM +0200, Jyri Sarha wrote:

Add drm_crtc_helper_disable_planes() for disabling all planes associated with the given CRTC and having disable callback. This can be used for instance in CRTC helper disable callback.

Signed-off-by: Jyri Sarha jsarha@ti.com

So it appears that this is the only missing piece for disabling and enabling planes with their associated CRTCs for the times when the CRTCs are blanked or otherwise unused.

drivers/gpu/drm/drm_crtc_helper.c | 26 ++++++++++++++++++++++++++ include/drm/drm_crtc_helper.h | 1 + 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index ef53475..33dddd5 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -410,6 +410,32 @@ done: } EXPORT_SYMBOL(drm_crtc_helper_set_mode);

+/**

• • drm_crtc_helper_disable_planes - helper to disable CRTC's planes
• • @crtc: CRTC
• • Disables all planes associated with the given CRTC and having
• • disable callback. This can be used for instance in CRTC helper

s/in CRTC/in the CRTC/

• • disable callback.

"... to disable all planes before shutting down the display pipeline." just to clarify usage.

One open is whether we should call the crtc-level atomic_begin/flush functions. I think adding that would be good, to avoid tearing while disabling planes. If drivers need different behaviour we can always add a knob to select the right one (i.e. with or without atomic_begin/flush).

• */

+void drm_crtc_helper_disable_planes(struct drm_crtc *crtc)

This function only makes sense for atomic drivers, so should be in drm_atomic_helper.c. drm_crtc_helper.c is the legacy helper library for non-atomic drivers.

Also needs a different name then, for example drm_atomic_helper_disable_planes_on_crtc, for consistency with drm_atomic_helper_commit_planes_on_crtc

+{

• struct drm_plane *plane;
• int i;
• 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 (plane_funcs->atomic_disable)
  

• 	plane_funcs->atomic_disable(plane, NULL);
  

Imo WARN_ON(!plane_funcs->atomic_disable); since calling this function without having disable hooks really doesn't make sense. Please also update the kerneldoc with something like "It is a bug to call this function without having implemented the ->atomic_disable() plane hook."

Cheers, Daniel

• }

+} +EXPORT_SYMBOL(drm_crtc_helper_disable_planes);

static void drm_crtc_helper_disable(struct drm_crtc *crtc) { diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 3febb4b..d47051f 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -189,6 +189,7 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb); +extern void drm_crtc_helper_disable_planes(struct drm_crtc *crtc); extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc); extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);

-- 1.9.1