On Tue, Nov 24, 2015 at 06:26:00PM -0800, Alex Goins wrote: > 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. Yeah I think that's a good idea for better encapsulation of reservation/fence internals. -Daniel > > > > 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 > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
