Re: [PATCH 43/58] drm/i915: stage modeset output changes

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Sun, 19 Aug 2012 21:13:00 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> This is the core of the new modeset logic.
> 
> The current code which is based upon the crtc helper code first
> updates all the link of the new display pipeline and then calls the
> lower-level set_mode function to execute the required callbacks to get
> there. The issue with this approach is that for disabling we need to
> know the _current_ display pipe state, not the new one.
> 
> Hence we need to stage the new state of the display pipe and only
> update it once we have disabled the current configuration and before we
> start to update the hw registers with the new configuration.
> 
> This patch here just prepares the ground by switching the new output
> state computation to these staging pointers. To make it clearer,
> rename the old update_output_state function to stage_output_state.
> 
> A few peculiarities:
> - We're also calling the set_mode function at various places to update
>   properties. Hence after a successfule modeset we need to stage the
>   current configuration (for otherwise we might fall back again). This
>   happens automatically because as part of the (successful) modeset we
>   need to copy the staged state to the real one. But for the hw
>   readout code we need to make sure that this happens, too.
> - Teach the new staged output state computation code the required
>   smarts to handle the disabling of outputs. The current code handles
>   this in a special case, but to better handle global modeset changes
>   covering more than one crtc, we want to do this all in the same
>   low-level modeset code.
> - The actual modeset code is still a bit ugly and wants to know the new
>   crtc->enabled state a bit early. Follow-on patches will clean that
>   up, for now we have to apply the staged output configuration early,
>   outside of the set_mode functions.
> - Improve/add comments in stage_output_state.
> 
> Essentially all that is left to do now is move the disabling code into
> set_mode and then move the staged state update code also into
> set_mode, at the right place between disabling things and calling the
> mode_set callbacks for the new configuration.
> 
> v2: Disabling a crtc works by passing in a NULL mode or fb, userspace
> doesn't hand in the list of connectors. We therefore need to detect
> this case manually and tear down all the output links.
> 
> v3: Properly update the output staging pointers after having read out
> the hw state.
> 
> v4: Simplify the code, add more DRM_DEBUG_KMS output and check a few
> assumptions with WARN_ON. Essentially all things that I've noticed
> while debugging issues in other places of the code.
> 
> v4: Correctly disable the old set of connectors when enabling an
> already enabled crtc on a new set of crtc. Reported by Paulo Zanoni.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center