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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
