[Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

[Ville Syrjälä]

On Tue, May 12, 2015 at 03:33:12PM +0200, Maarten Lankhorst wrote:
> Op 12-05-15 om 10:18 schreef Daniel Vetter:
> > On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
> >> @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
> >>  	/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
> >>  	intel_state->clip.x1 = 0;
> >>  	intel_state->clip.y1 = 0;
> >> -	intel_state->clip.x2 =
> >> -		crtc_state->base.active ? crtc_state->pipe_src_w : 0;
> >> -	intel_state->clip.y2 =
> >> -		crtc_state->base.active ? crtc_state->pipe_src_h : 0;
> >> -
> >> -	/*
> >> -	 * Disabling a plane is always okay; we just need to update
> >> -	 * fb tracking in a special way since cleanup_fb() won't
> >> -	 * get called by the plane helpers.
> >> -	 */
> >> -	if (state->fb == NULL && plane->state->fb != NULL) {
> >> -		/*
> >> -		 * 'prepare' is never called when plane is being disabled, so
> >> -		 * we need to handle frontbuffer tracking as a special case
> >> -		 */
> >> -		intel_crtc->atomic.disabled_planes |=
> >> -			(1 << drm_plane_index(plane));
> >> -	}
> >> +	drm_crtc_get_hv_timing(&crtc_state->mode,
> >> +			       &intel_state->clip.x2,
> >> +			       &intel_state->clip.y2);
> > Imo this is obfuscating things a bit, why not just unconditionally copy
> > pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
> > most platforms must match the primary plane window except for gen2 and
> > gen9+.
> pipe_src_* is calculated in the same way,

Except we may end up rounding pipe_src_w down if we have
double-wide/dual link lvds etc., and we definitely need to clip the
planes against the real pipe src size.

> and at the time of plane validation the crtc validation hasn't run yet,
> so pipe_src_* contains outdated values which break in interesting ways.

Just check crtc first. Or actually we probably need pre + post crtc
checks since we want to do wm compute and such after the planes have
been handled.

-- 
Ville Syrjälä
Intel OTC