[Intel-gfx] [PATCH 5/5] drm/i915: Make wait_for_flips interruptible.

[Chris Wilson]

On Tue, Jul 21, 2015 at 02:33:33PM +0200, Maarten Lankhorst wrote:
> >> +	if (plane->type == DRM_PLANE_TYPE_PRIMARY && old_obj) {
> > Would feel safer is we just asked if the CRTC had pending flips
> > irrespective of old_obj. Do you plan on moving the pending flips from
> > CRTC to plane? That would seem to be implied here.
> This preserves old behavior since page flips can only happen on primary planes.
> Do you want cursor updates to wait on pending primary flips?

Cursor updates need to be serialized with primary updates, yes (or else
there are some fun scenarios in which you can hang the display engine).

What I was actually thinking was using plane->state for tracking the
flips. Having just reread what I wrote that didn't come across. I would
rather see fewer primary_plane special cases and more no-ops on
secondary planes. I think you will end up using such infrastructure
on the secondary planes eventually.

> >> +	/* Big Hammer, we also need to ensure that any pending
> >> +	 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
> >> +	 * current scanout is retired before unpinning the old
> >> +	 * framebuffer. Note that we rely on userspace rendering
> >> +	 * into the buffer attached to the pipe they are waiting
> >> +	 * on. If not, userspace generates a GPU hang with IPEHR
> >> +	 * point to the MI_WAIT_FOR_EVENT.
> >> +	 *
> >> +	 * This should only fail upon a hung GPU, in which case we
> >> +	 * can safely continue.
> >> +	 */
> >> +	if (!ret && plane->type == DRM_PLANE_TYPE_PRIMARY && old_obj)
> >> +		ret = i915_gem_object_wait_rendering(old_obj, true);
> > Technically I can create a batch with a WAIT_ON_EVENT for a secondary
> > plane as well. This path need only be done in a modeset, not for a
> > simple plane flip. (But we actually need to serialise with rendering to
> > old_obj before regarding the plane flip as complete.) Is there a
> > plane-prepare-modeset hook?
> The comment didn't explain that, is hiding this behind a needs_modeset(crtc_state) sufficient here?

For the purpose of limiting the wait to when the pipe dimensions may
change, yes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre