On Mon, May 05, 2014 at 06:27:17PM +0900, Michel Dänzer wrote:
> On 02.05.2014 22:29, Christian König wrote:
> > Am 02.05.2014 09:25, schrieb Michel Dänzer:
> >> On 29.04.2014 23:29, Christian König wrote:
> >>>
> >>> +static void radeon_flip_work_func(struct work_struct *__work)
> >>> {
> >> [...]
> >>> + if (radeon_crtc->flip_work) {
> >>> + DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> >>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> >>> + goto pflip_cleanup1;
> >>> + }
> >> I'm a little worried about this case. AFAICT this would drop the flip if
> >> a previous one is still pending? I'm not sure current userspace can
> >> actually hit this
> > Yeah, that concerned me as well. The old code dropped the the new flip
> > as well, so I'm pretty sure that the new handling is right and userspace
> > won't hit that.
> >
> > The only difference to the old code is that I've offloaded it to a
> > separate thread and so can't return -EBUSY any more.
>
> That's an important difference though: Userspace can react to the -EBUSY
> appropriately, but if flips get dropped silently, bad things will
> happen, such as the wrong buffer being scanned out.
>
> Keep in mind that we don't control all userspace, e.g. I expect there
> will be an increasing number of Wayland compositors using this
> functionality.
Rules on i915 are that if the pageflip confirmation event hasn't been sent
out yet the driver returns -EBUSY. The locking for that is done using
irq-save spinlocks, so should be doable to check this synchronously before
latching some kinda of async worker.
I didnt read one line of radeon code to check this though ;-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
