Re: [PATCH 10/14] drm/atomic-helper: Disable planes when suspending

From: Daniel Vetter <daniel@ffwll.ch>
To: Liu Ying <gnuiyl@gmail.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 10/14] drm/atomic-helper: Disable planes when suspending
Date: Tue, 24 May 2016 13:00:08 +0200	[thread overview]
Message-ID: <20160524110008.GV27098@phenom.ffwll.local> (raw)
In-Reply-To: <1464084653-16684-11-git-send-email-gnuiyl@gmail.com>

On Tue, May 24, 2016 at 06:10:49PM +0800, Liu Ying wrote:
> We should disable planes explicitly when suspending.
> Especially, this is meaningful for those display controllers which
> don't support active planes without relevant CRTCs being enabled.
> 
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>

Recommended way is to call drm_atomic_helper_disable_planes_on_crtc in
your crtc's ->disable() callback if your hw needs this. This is a general
problem (test e.g. dpms), not just an issue in suspend code.

Also unsetting the planes from state has a semantic meaning: It unpins the
backing storage, which is definitely not what we want for suspend/resume.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4befe25..5331d95 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1967,7 +1967,7 @@ commit:
>   *
>   * Loops through all connectors, finding those that aren't turned off and then
>   * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> - * that they are connected to.
> + * that they are connected to.  The relevant planes are deactivated as well.
>   *
>   * This is used for example in suspend/resume to disable all currently active
>   * functions when suspending.
> @@ -1997,6 +1997,7 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>  	drm_for_each_connector(conn, dev) {
>  		struct drm_crtc *crtc = conn->state->crtc;
>  		struct drm_crtc_state *crtc_state;
> +		struct drm_plane *plane;
>  
>  		if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
>  			continue;
> @@ -2008,6 +2009,21 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>  		}
>  
>  		crtc_state->active = false;
> +
> +		drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
> +			struct drm_plane_state *plane_state;
> +
> +			plane_state = drm_atomic_get_plane_state(state, plane);
> +			if (IS_ERR(plane_state)) {
> +				err = PTR_ERR(plane_state);
> +				goto free;
> +			}
> +
> +			err = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> +			if (err != 0)
> +				goto free;
> +			drm_atomic_set_fb_for_plane(plane_state, NULL);
> +		}
>  	}
>  
>  	err = drm_atomic_commit(state);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/dri-devel