Re: [PATCH RFC] drm/atomic: Disable planes on blanked CRTC and enable on unblank

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Nov 05, 2015 at 05:03:09PM +0200, Jyri Sarha wrote:
> Disable planes if they are on to be blanked CRTC and enable them when
> the CRTC is turned back on by DMPS.
> 
> This is desirable on HW that loses its context on blanking. When
> planes are enabled and disabled with the associated CRTCs, there is no
> need to restore the plane context in runtime_resume callback.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> We would need something like this to get rid off OMAPDSS somewhat
> messy runtime_resume code. How does this look, could this generic
> solution be refined to be acceptable to mainline, or should we start
> looking a local solution to omapdrm?
> 
> BTW, with this patch the planes are sometimes disabled multiple
> times. So probably a boolean (or maybe two like on crtc) should be
> added to drm_plane_state to track and avoid multiple shutdowns.

The recommended way to do this is to call drm_atomic_add_affected_planes
from your crtc's atomic_check callback when active_changed is set. This
will also take care of enabling all planes again. For disabling we don't
yet have a helper yet, but it would be easy to create a
drm_atomic_helper_disable_planes(crtc) which just walks over all planes in
an atomic update that are currently using the given crtc and disables it
using plane_helper_funcs->atomic_disable. That would again require
drm_atomic_add_affected_planes to be called first.

This way current helper behaviour is unchanged, but it'd be easy to
construct the behaviour you want using helpers with
drm_atomic_add_affected_planes called from the crtc->disable hook.

I thought there was an rfc patch somewhere for this, but I can't find it
any more.

Cheers, Daniel
> 
>  drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++--
>  include/drm/drm_atomic_helper.h     |  7 +++++++
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index aecb5d6..92fcaef 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1994,8 +1994,8 @@ EXPORT_SYMBOL(drm_atomic_helper_page_flip);
>   *
>   * This is the main helper function provided by the atomic helper framework for
>   * implementing the legacy DPMS connector interface. It computes the new desired
> - * ->active state for the corresponding CRTC (if the connector is enabled) and
> - *  updates it.
> + * ->active state for the corresponding CRTC and planes on it (if the connector
> + * is enabled) and updates it.
>   *
>   * Returns:
>   * Returns 0 on success, negative errno numbers on failure.
> @@ -2008,6 +2008,7 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_crtc *crtc;
>  	struct drm_connector *tmp_connector;
> +	struct drm_plane *tmp_plane;
>  	int ret;
>  	bool active = false;
>  	int old_mode = connector->dpms;
> @@ -2046,6 +2047,20 @@ retry:
>  	}
>  	crtc_state->active = active;
>  
> +	/* Collect associated plane states to global state object. */
> +	list_for_each_entry(tmp_plane, &config->plane_list, head) {
> +		struct drm_plane_state *plane_state;
> +
> +		if (tmp_plane->state->crtc != crtc)
> +			continue;
> +
> +		plane_state = drm_atomic_get_plane_state(state, tmp_plane);
> +		if (IS_ERR(plane_state)) {
> +			ret = PTR_ERR(plane_state);
> +			goto fail;
> +		}
> +	}
> +
>  	ret = drm_atomic_commit(state);
>  	if (ret != 0)
>  		goto fail;
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 11266d14..89c327f 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -173,6 +173,13 @@ drm_atomic_plane_disabling(struct drm_plane *plane,
>  		(plane->state->crtc != NULL && plane->state->fb == NULL));
>  
>  	/*
> +	 * If the plane is on a crtc that is not going to be active the plane
> +	 * it self can be disabled too.
> +	 */
> +	if (plane->state->crtc && !plane->state->crtc->state->active)
> +		return true;
> +
> +	/*
>  	 * 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.
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel