Re: [PATCH v2 14/20] drm/i915: Make intel_display_suspend atomic, try 2.

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 07, 2015 at 09:08:25AM +0200, Maarten Lankhorst wrote:
> Calculate all state using a normal transition, but afterwards fudge
> crtc->state->active back to its old value. This should still allow
> state restore in setup_hw_state to work properly.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90396
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

What we could do here (just as an experiment really, I haven't thought
through the implications for locking) is duplicate the state here _before_
we suspend everything. Then on force_restore we just use the saved state
from here for resuming. Would get rid of the failures on resume.

Otoh we need that code for the lid notifier still, so probably meh.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  2 files changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f7b1fc28142c..9c461ea84d77 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6219,12 +6219,58 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
>   * turn all crtc's off, but do not adjust state
>   * This has to be paired with a call to intel_modeset_setup_hw_state.
>   */
> -void intel_display_suspend(struct drm_device *dev)
> +int intel_display_suspend(struct drm_device *dev)
>  {
> +	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
> +	struct drm_atomic_state *state;
>  	struct drm_crtc *crtc;
> +	unsigned crtc_mask = 0;
> +	int ret = 0;
> +
> +	if (WARN_ON(!ctx))
> +		return 0;
> +
> +	lockdep_assert_held(&ctx->ww_ctx);
> +	state = drm_atomic_state_alloc(dev);
> +	if (WARN_ON(!state))
> +		return -ENOMEM;
> +
> +	state->acquire_ctx = ctx;
> +	state->allow_modeset = true;
> +
> +	for_each_crtc(dev, crtc) {
> +		struct drm_crtc_state *crtc_state =
> +			drm_atomic_get_crtc_state(state, crtc);
>  
> -	for_each_crtc(dev, crtc)
> -		intel_crtc_disable_noatomic(crtc);
> +		ret = PTR_ERR_OR_ZERO(crtc_state);
> +		if (ret)
> +			goto free;
> +
> +		if (!crtc_state->active)
> +			continue;
> +
> +		crtc_state->active = false;
> +		crtc_mask |= 1 << drm_crtc_index(crtc);
> +	}
> +
> +	if (crtc_mask) {
> +		ret = intel_set_mode(state);
> +
> +		if (!ret) {
> +			for_each_crtc(dev, crtc)
> +				if (crtc_mask & (1 << drm_crtc_index(crtc)))
> +					crtc->state->active = true;
> +
> +			return ret;
> +		}
> +	}
> +
> +free:
> +	if (ret)
> +		DRM_ERROR("Suspending crtc's failed with %i\n", ret);
> +	drm_atomic_state_free(state);
> +	return ret;
>  }
>  
>  /* Master function to enable/disable CRTC and corresponding power wells */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 525b4137521c..66e2016e0a96 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -992,7 +992,7 @@ int intel_pch_rawclk(struct drm_device *dev);
>  void intel_mark_busy(struct drm_device *dev);
>  void intel_mark_idle(struct drm_device *dev);
>  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> -void intel_display_suspend(struct drm_device *dev);
> +int intel_display_suspend(struct drm_device *dev);
>  int intel_crtc_control(struct drm_crtc *crtc, bool enable);
>  void intel_crtc_update_dpms(struct drm_crtc *crtc);
>  void intel_encoder_destroy(struct drm_encoder *encoder);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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