[Intel-gfx] [PATCH 25/55] drm/i915: Update i915_gem_object_sync() to take a request structure

[Chris Wilson]

On Thu, Jun 18, 2015 at 01:14:56PM +0100, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
> 
> The plan is to pass requests around as the basic submission tracking structure
> rather than rings and contexts. This patch updates the i915_gem_object_sync()
> code path.
> 
> v2: Much more complex patch to share a single request between the sync and the
> page flip. The _sync() function now supports lazy allocation of the request
> structure. That is, if one is passed in then that will be used. If one is not,
> then a request will be allocated and passed back out. Note that the _sync() code
> does not necessarily require a request. Thus one will only be created until
> certain situations. The reason the lazy allocation must be done within the
> _sync() code itself is because the decision to need one or not is not really
> something that code above can second guess (except in the case where one is
> definitely not required because no ring is passed in).
> 
> The call chains above _sync() now support passing a request through which most
> callers passing in NULL and assuming that no request will be required (because
> they also pass in NULL for the ring and therefore can't be generating any ring
> code).
> 
> The exeception is intel_crtc_page_flip() which now supports having a request
> returned from _sync(). If one is, then that request is shared by the page flip
> (if the page flip is of a type to need a request). If _sync() does not generate
> a request but the page flip does need one, then the page flip path will create
> its own request.
> 
> v3: Updated comment description to be clearer about 'to_req' parameter (Tomas
> Elf review request). Rebased onto newer tree that significantly changed the
> synchronisation code.
> 
> v4: Updated comments from review feedback (Tomas Elf)
> 
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> Reviewed-by: Tomas Elf <tomas.elf at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |    4 ++-
>  drivers/gpu/drm/i915/i915_gem.c            |   48 +++++++++++++++++++++-------
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
>  drivers/gpu/drm/i915/intel_display.c       |   17 +++++++---
>  drivers/gpu/drm/i915/intel_drv.h           |    3 +-
>  drivers/gpu/drm/i915/intel_fbdev.c         |    2 +-
>  drivers/gpu/drm/i915/intel_lrc.c           |    2 +-
>  drivers/gpu/drm/i915/intel_overlay.c       |    2 +-
>  8 files changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 64a10fa..f69e9cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2778,7 +2778,8 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
>  
>  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
>  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
> -			 struct intel_engine_cs *to);
> +			 struct intel_engine_cs *to,
> +			 struct drm_i915_gem_request **to_req);

Nope. Did you forget to reorder the code to ensure that the request is
allocated along with the context switch at the start of execbuf?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre