Re: [RFC PATCH 2/7] drm/i915: Read hw state into an atomic state struct, try 2.

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jun 19, 2015 at 10:02:25AM +0200, Maarten Lankhorst wrote:
>  /* Scan out the current hw modeset state, sanitizes it and maps it into the drm
> @@ -15491,37 +15569,61 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  				  bool force_restore)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum pipe pipe;
> -	struct intel_crtc *crtc;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
>  	struct intel_encoder *encoder;
> +	struct drm_atomic_state *state;
> +	struct intel_shared_dpll_config shared_dplls[I915_NUM_PLLS];
>  	int i;
>  
> -	intel_modeset_readout_hw_state(dev);
> -
> -	/*
> -	 * Now that we have the config, copy it to each CRTC struct
> -	 * Note that this could go away if we move to using crtc_config
> -	 * checking everywhere.
> -	 */
> -	for_each_intel_crtc(dev, crtc) {
> -		if (crtc->active && i915.fastboot) {
> -			intel_mode_from_pipe_config(&crtc->base.mode,
> -						    crtc->config);
> -			DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
> -				      crtc->base.base.id);
> -			drm_mode_debug_printmodeline(&crtc->base.mode);
> -		}
> +	state = intel_modeset_readout_hw_state(dev);
> +	if (IS_ERR(state)) {
> +		DRM_ERROR("Failed to read out hw state\n");
> +		return;
>  	}
>  
> +	drm_atomic_helper_swap_state(dev, state);
> +
> +	/* swap sw/hw dpll state */
> +	intel_atomic_duplicate_dpll_state(dev_priv, shared_dplls);
> +	intel_shared_dpll_commit(state);
> +	memcpy(to_intel_atomic_state(state)->shared_dpll,
> +	       shared_dplls, sizeof(*shared_dplls) * dev_priv->num_shared_dpll);

This is imo way too much open-coding of stuff that's already there. What I
had in mind for force_restore was a generic helper to duplicate all the
state objects into a drm_atomic_state, without calling any of the
->atomic_check functions or anything. Then the sequence for restoring
state would be.

1. saved_state = drm_atomic_helper_duplicate_state()
2. intel_readout_hw_sate() <- this would just update ->state and sanitize
it.
3. drm_atomic_commit(saved_state) wrapped in a WARN_ON to make sure it
always works.

Ofc we still need the ww-mutex deadlock avoidance wrapped around this, but
since atomic_commit will first compute all derived state there's no need
to duplicate all the i915-internal state too. That should all work like
with any other normal modeset.
-Daniel
-- 
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