[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