[PATCH i915 v7 1/2] i915: wait for fence in mmio_flip_work_func

[Daniel Kurtz]

Hi Alex,

Drive-by review because I was just reviewing something similar for a
different device...

On Wed, Nov 25, 2015 at 6:15 AM, Alex Goins <agoins at nvidia.com> wrote:
> If a buffer is backed by dmabuf, wait on its reservation object's exclusive
> fence before flipping.
>
> v2: First commit
> v3: Remove object_name_lock acquire
> v4: Move wait ahead of mark_page_flip_active
>     Use crtc->primary->fb to get GEM object instead of pending_flip_obj
>     use_mmio_flip() return true when exclusive fence is attached
>     Wait only on exclusive fences, interruptible with no timeout
> v5: Move wait from do_mmio_flip to mmio_flip_work_func
>     Style tweaks to more closely match rest of file
> v6: Change back to unintteruptible wait to match __i915_wait_request due to
>     inability to properly handle interrupted wait.
>     Warn on error code from waiting.
>
> Signed-off-by: Alex Goins <agoins at nvidia.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b2270d5..bacf336 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -44,6 +44,8 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_rect.h>
>  #include <linux/dma_remapping.h>
> +#include <linux/reservation.h>
> +#include <linux/dma-buf.h>
>
>  /* Primary plane formats for gen <= 3 */
>  static const uint32_t i8xx_primary_formats[] = {
> @@ -11088,6 +11090,8 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
>                 return true;
>         else if (i915.enable_execlists)
>                 return true;
> +       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.

>         else
>                 return ring != i915_gem_request_get_ring(obj->last_write_req);
>  }
> @@ -11189,6 +11193,9 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
>  {
>         struct intel_mmio_flip *mmio_flip =
>                 container_of(work, struct intel_mmio_flip, work);
> +       struct intel_framebuffer *intel_fb =
> +               to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
> +       struct drm_i915_gem_object *obj = intel_fb->obj;
>
>         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.

-Dan

>         intel_do_mmio_flip(mmio_flip->crtc);
>
>         i915_gem_request_unreference__unlocked(mmio_flip->req);
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel