[Intel-gfx] [PATCH 2/4] drm/i915: restore cursor and sprite state when forcing a config restore

[Daniel Vetter]

On Tue, Mar 26, 2013 at 09:25:44AM -0700, Jesse Barnes wrote:
> Needed for VT switchless resume.
> 
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7307974..093006b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9106,6 +9106,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum pipe pipe;
>  	u32 tmp;
> +	struct drm_plane *plane;
>  	struct intel_crtc *crtc;
>  	struct intel_encoder *encoder;
>  	struct intel_connector *connector;
> @@ -9210,8 +9211,20 @@ setup_pipes:
>  
>  	if (force_restore) {
>  		for_each_pipe(pipe) {
> -			intel_crtc_restore_mode(dev_priv->pipe_to_crtc_mapping[pipe]);
> +			struct drm_crtc *crtc =
> +				dev_priv->pipe_to_crtc_mapping[pipe];
> +			struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +			intel_crtc_restore_mode(crtc);

I've just noticed that crtc_restore_mode is a bit buggy, since it
unconditionally enables the pipe. Which is the wrong thing to do if the
pipe has a mode set, but is disabled with dpms off.

> +			if (intel_crtc->cursor_visible) {
> +				/* Force update for previously enabled cursor */
> +				intel_crtc->cursor_visible = false;
> +				intel_crtc_update_cursor(&intel_crtc->base,
> +							 true);
> +			}

We already have an unconditional cursor enable call in the various
platform_crtc_enable functions. Do we really neeed this? Or should we
actually do this ->cursor_visible dance in there instead of here?

>  		}
> +		list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +			intel_plane_restore(plane);

Thinking more about this we have the same plane restore issue for dpms.
Thanks to my utterly lazy legacy overlay code userspace has to take care
of things already, but what about we move that fix here to the right
place? Would also be useful for the atomic modeset support, where we want
to light up a cursor/plane config right away ...

With those things fixed I think the only thing we need to do here is walk
all the connectors and do a dpms update. Presuming that the crtc->active
tracking is reflecting reality (i.e. all false) it should do the right
thing and restore stuff (at least if your crtc disabling doesn't update
the dpms state - disabled crtc means implicitly dpms off).

Like I've said this has the added benefit that the dpms code is shared
with the resume code.

Cheers,
Your royal pita maintainer ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch