[Intel-gfx] [PATCH 37/42] drm/i915: swap state correctly in intel_atomic_commit

[Daniel Vetter]

On Tue, May 12, 2015 at 04:16:13PM +0200, Maarten Lankhorst wrote:
> Op 12-05-15 om 15:03 schreef Daniel Vetter:
> > On Mon, May 11, 2015 at 04:25:13PM +0200, Maarten Lankhorst wrote:
> >> crtc->config is gone, swap swap swap. :D
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_atomic.c  | 36 ++----------------------------------
> >>  drivers/gpu/drm/i915/intel_display.c |  7 +++++--
> >>  2 files changed, 7 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> >> index ace6aeeb1359..0315dc44b17a 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -130,7 +130,6 @@ int intel_atomic_commit(struct drm_device *dev,
> >>  			bool async)
> >>  {
> >>  	int ret;
> >> -	int i;
> >>  
> >>  	if (async) {
> >>  		DRM_DEBUG_KMS("i915 does not yet support async commit\n");
> >> @@ -142,48 +141,17 @@ int intel_atomic_commit(struct drm_device *dev,
> >>  		return ret;
> >>  
> >>  	/* Point of no return */
> >> +	drm_atomic_helper_swap_state(dev, state);
> >>  
> >>  	/*
> >>  	 * FIXME:  The proper sequence here will eventually be:
> >>  	 *
> >> -	 * drm_atomic_helper_swap_state(dev, state)
> >>  	 * drm_atomic_helper_commit_modeset_disables(dev, state);
> >>  	 * drm_atomic_helper_commit_planes(dev, state);
> >>  	 * drm_atomic_helper_commit_modeset_enables(dev, state);
> >> -	 * drm_atomic_helper_wait_for_vblanks(dev, state);
> >> -	 * drm_atomic_helper_cleanup_planes(dev, state);
> >> -	 * drm_atomic_state_free(state);
> >> -	 *
> >> -	 * once we have full atomic modeset.  For now, just manually update
> >> -	 * plane states to avoid clobbering good states with dummy states
> >> -	 * while nuclear pageflipping.
> >>  	 */
> >> -	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> >> -		struct drm_plane *plane = state->planes[i];
> >> -
> >> -		if (!plane)
> >> -			continue;
> >> -
> >> -		plane->state->state = state;
> >> -		swap(state->plane_states[i], plane->state);
> >> -		plane->state->state = NULL;
> >> -	}
> >> -
> >> -	/* swap crtc_scaler_state */
> >> -	for (i = 0; i < dev->mode_config.num_crtc; i++) {
> >> -		struct drm_crtc *crtc = state->crtcs[i];
> >> -		if (!crtc) {
> >> -			continue;
> >> -		}
> >> -
> >> -		to_intel_crtc_state(crtc->state)->scaler_state =
> >> -			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> >> -
> >> -		if (INTEL_INFO(dev)->gen >= 9)
> >> -			skl_detach_scalers(to_intel_crtc(crtc));
> >> -	}
> >> -
> >>  	drm_atomic_helper_commit_planes(dev, state);
> >> +
> >>  	drm_atomic_helper_wait_for_vblanks(dev, state);
> >>  	drm_atomic_helper_cleanup_planes(dev, state);
> >>  	drm_atomic_state_free(state);
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 65ee2cd0437c..105bdf84e46a 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -13202,11 +13202,14 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> >>  
> >>  	/* Perform vblank evasion around commit operation */
> >>  	if (intel_crtc->atomic.evade &&
> >> -	    !dev_priv->power_domains.init_power_on)
> >> +	    !dev_priv->power_domains.init_power_on) {
> >> +		if (dev_priv->info.gen >= 9)
> >> +			skl_detach_scalers(to_intel_crtc(crtc));
> > Don't we need to update skl_detach_scalers to look at the other config
> > now? Or is this an issue with the patch splitting?
> >
> > Also why is this protected by atomic.evade?
> Mostly to not run if a modeset's done, that already kills the scalers.

As mentioned in another reply we don't really want to disable the evade
logic for modesets. And if this is to be avoided for !modeset then it's
clearer to check for that explicitly.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch