[PATCH i915 v7 1/2] i915: wait for fence in mmio_flip_work_func
Alex Goins
agoins at nvidia.com
Tue Nov 24 18:26:00 PST 2015
Thanks, Daniel. There sure are a lot of Daniels.
> > + else if (obj->base.dma_buf && obj->base.dma_buf->resv->fence_excl)
> > + return true;
>
> I'm not sure if this is really doing exactly what you want.
> When a reservation object's exclusive fence has signaled, I think the
> old pointer in fence_excl is not actually cleared; it keeps pointing
> to the old exclusive fence until that replaced by another.
>
> So, just nake null-check here is like saying "did this reservation
> object ever have an exclusive fence at some point in the past", which
> is not necessarily the same as "is there an exclusive fence associated
> with the buffer that I am about to flip"?
>
> The reservation object is a complicated rcu & ww_mutex protected
> beast, so I would be shy to access any of its fields directly.
Hm, well, the goal is to ensure that mmio_flip is used if we might need to wait
on a fence, so if there are false positives it shouldn't be lethal.
Nonetheless, you're probably right. Maybe it would be better to use
!reservation_object_test_signaled_rcu(test_all=FALSE), which should be TRUE iff
there is an unsignaled exclusive fence attached, meaning we might have to wait.
> > if (mmio_flip->req)
> > WARN_ON(__i915_wait_request(mmio_flip->req,
> > @@ -11196,6 +11203,12 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
> > false, NULL,
> > &mmio_flip->i915->rps.mmioflips));
> >
> > + /* For framebuffer backed by dmabuf, wait for fence */
> > + if (obj->base.dma_buf)
> > + WARN_ON(reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
> > + false, false,
> > + MAX_SCHEDULE_TIMEOUT) < 0);
> > +
>
> Hmm, don't you want an interrupt-able wait here?
> And if so, you probably don't want to WARN_ON() -ERESTARTSYS.
I had this as an interruptable wait previously, but I'm not sure how we would
actually handle an interrupt in that case. Notice also how
__i915_wait_request(interruptable=FALSE) just above is also an uninterruptable
wait.
Thanks,
Alex
More information about the dri-devel
mailing list