Op 22-06-15 om 17:31 schreef Daniel Vetter:
> 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.
Well, for !force_restore we want to do an initial sanitizing modeset too, but
with the full disabled state as reference point.
I'm not sure dupe state followed by a swap, is different from save old state,
and read out to current state.
> 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.
No need to wrap, readout touches everything so lock_all is fine.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
