Re: [PATCH v2] drm/i915: Serialise presentation with imported dmabufs

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 14, 2016 at 09:56:01PM +0100, Chris Wilson wrote:
> obj->base.dma_buf represents a dma-buf exported from this object (for
> use by others). On the contrary, obj->base.import_attach represents the
> source dma-buf that was used to create this object (if any). When
> serialising with third parties, we need to wait on their rendering via
> the import attachment as well as their rendering on our exported
> dma-buf.
> 
> v2: Wait on both import and export.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Alex Goins <agoins@nvidia.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.h | 45 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c   | 48 ++++++++++++++++++----------------
>  2 files changed, 71 insertions(+), 22 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.h b/drivers/gpu/drm/i915/i915_gem_dmabuf.h
> new file mode 100644
> index 000000000000..092bb363693c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _I915_GEM_DMABUF_H_
> +#define _I915_GEM_DMABUF_H_
> +
> +#include <linux/dma-buf.h>
> +
> +static inline struct reservation_object *
> +i915_gem_object_get_dmabuf_resv(struct drm_i915_gem_object *obj)
> +{
> +	struct dma_buf *dma_buf;
> +
> +	if (obj->base.import_attach)
> +		dma_buf = obj->base.import_attach->dmabuf;
> +	else if (obj->base.dma_buf)
> +		dma_buf = obj->base.dma_buf;
> +	else
> +		return NULL;
> +
> +	return dma_buf->resv;
> +}
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 801e4c17dd8d..33531e6cb236 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -36,6 +36,7 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "i915_gem_dmabuf.h"
>  #include "intel_dsi.h"
>  #include "i915_trace.h"
>  #include <drm/drm_atomic.h>
> @@ -46,7 +47,6 @@
>  #include <drm/drm_rect.h>
>  #include <linux/dma_remapping.h>
>  #include <linux/reservation.h>
> -#include <linux/dma-buf.h>
>  
>  static bool is_mmio_work(struct intel_flip_work *work)
>  {
> @@ -11405,6 +11405,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
>  static bool use_mmio_flip(struct intel_engine_cs *engine,
>  			  struct drm_i915_gem_object *obj)
>  {
> +	struct reservation_object *resv;
> +
>  	/*
>  	 * This is not being used for older platforms, because
>  	 * non-availability of flip done interrupt forces us to use
> @@ -11425,12 +11427,12 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
>  		return true;
>  	else if (i915.enable_execlists)
>  		return true;
> -	else if (obj->base.dma_buf &&
> -		 !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv,
> -						       false))
> +
> +	resv = i915_gem_object_get_dmabuf_resv(obj);
> +	if (resv && !reservation_object_test_signaled_rcu(resv, false))
>  		return true;
> -	else
> -		return engine != i915_gem_request_get_engine(obj->last_write_req);
> +
> +	return engine != i915_gem_request_get_engine(obj->last_write_req);
>  }
>  
>  static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> @@ -11519,6 +11521,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
>  	struct intel_framebuffer *intel_fb =
>  		to_intel_framebuffer(crtc->base.primary->fb);
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
> +	struct reservation_object *resv;
>  
>  	if (work->flip_queued_req)
>  		WARN_ON(__i915_wait_request(work->flip_queued_req,
> @@ -11526,11 +11529,10 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
>  					    &dev_priv->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,
> +	resv = i915_gem_object_get_dmabuf_resv(obj);
> +	if (resv)
> +		WARN_ON(reservation_object_wait_timeout_rcu(resv, false, false,
>  							    MAX_SCHEDULE_TIMEOUT) < 0);
> -
>  	intel_pipe_update_start(crtc);
>  
>  	if (INTEL_GEN(dev_priv) >= 9)
> @@ -13922,6 +13924,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
> +	struct reservation_object *resv;
>  	int ret = 0;
>  
>  	if (!obj && !old_obj)
> @@ -13951,12 +13954,17 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  		}
>  	}
>  
> +	if (!obj) {
> +		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
> +		return 0;
> +	}
> +
>  	/* For framebuffer backed by dmabuf, wait for fence */
> -	if (obj && obj->base.dma_buf) {
> +	resv = i915_gem_object_get_dmabuf_resv(obj);
> +	if (resv) {
>  		long lret;
>  
> -		lret = reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
> -							   false, true,
> +		lret = reservation_object_wait_timeout_rcu(resv, false, true,
>  							   MAX_SCHEDULE_TIMEOUT);
>  		if (lret == -ERESTARTSYS)
>  			return lret;
> @@ -13964,9 +13972,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  		WARN(lret < 0, "waiting returns %li\n", lret);
>  	}
>  
> -	if (!obj) {
> -		ret = 0;
> -	} else if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
>  	    INTEL_INFO(dev)->cursor_needs_physical) {
>  		int align = IS_I830(dev) ? 16 * 1024 : 256;
>  		ret = i915_gem_object_attach_phys(obj, align);
> @@ -13977,13 +13983,11 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	}
>  
>  	if (ret == 0) {
> -		if (obj) {
> -			struct intel_plane_state *plane_state =
> -				to_intel_plane_state(new_state);
> +		struct intel_plane_state *plane_state =
> +			to_intel_plane_state(new_state);
>  
> -			i915_gem_request_assign(&plane_state->wait_req,
> -						obj->last_write_req);
> -		}
> +		i915_gem_request_assign(&plane_state->wait_req,
> +					obj->last_write_req);
>  
>  		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
>  	}
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx