Re: [PATCH 4/4] drm/i915: Update modeset programming to use intermediate state (v2)

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 10-09-15 om 03:57 schreef Matt Roper:
> As suggested by Ville, the general flow should now roughly follow:
>
>         // whatever the user wanted
>         compute_final_atomic_state()
>
>         // include all crtcs in the intermediate state which are
>         // getting disabled (even temporarily to perform a modeset)
>         compute_intermediate_atomic_state()
>
>         ret = check_state_change(old, intermediate)
>         ret = check_state_change(intermediate, new)
>
>         // commit all planes in one go to make them pop out as
>         // atomically as possible
>         for_each_crtc_in(intermediate) {
>                 commit_planes()
>         }
>
>         for_each_crtc_in(intermediate) {
>                 disable_crtc()
>         }
>
>         for_each_crtc_in(new) {
>                 if (!currently_active)
>                         crtc_enable()
>         }
>
>         // commit all planes in one go to make them pop in as atomically
>         // as possible
>         for_each_crtc_in(new) {
>                 commit_planes()
>         }
>
> v2: Because we're potentially performing two state swaps here, the
>     actual set of FB's that need to be cleaned up at the end of the
>     process may need to be fetched from the intermediate state rather
>     than the final state, so use our own intel_cleanup_planes() rather
>     than the helper version.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> I know Maarten had some reservations about this approach, so hopefully
> providing an implementation will allow us to continue the discussion and come
> to an agreement on whether or not intermediate states are the way to go.
I still don't like it. Intermediate wm's should be calculated in the check function, if it
can potentially fail.

The final state should be swapped in right away, not any intermediate state or async
modesets will never work.

And nothing should depend on the current state in the crtc_disable callbacks, if it
does it's a bug or it needs to get passed the old crtc_state so it knows what to disable.
This is probably why crtc->config is not dead yet.

Not committing DPLL state right after swap_state is a special case right now, but
that's easily fixed by changing pll->active from a refcount to a crtc mask.

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx