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

[Chris Wilson]

On Tue, Jul 21, 2015 at 03:59:25PM +0200, Maarten Lankhorst wrote:
> Op 21-07-15 om 15:31 schreef 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).
> Ok but right now we don't sync this.. won't blocking on flips for moving the mouse in Xorg be bad for performance?

Off the top of my head, we only need to worry about modesets, which 
presumedly are serialized between the planes.

> > 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.
> I think the current infrastructure will die eventually and perhaps be done asynchronously. I just don't know yet what will replace it.
> 
> >>>> +	/* 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.
> Is that only needed during full modesets, or when changing PIPESRC too?

The triggers are the display modeline values (whatever HTOTAL/VTOTAL
registers collectively are called), so just full modesets.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre