From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [RFC 17/44] drm/i915: Prelude to splitting i915_gem_do_execbuffer in two Date: Wed, 2 Jul 2014 11:34:23 -0700 Message-ID: <20140702113423.189ec11c@jbarnes-desktop> References: <1403803475-16337-1-git-send-email-John.C.Harrison@Intel.com> <1403803475-16337-18-git-send-email-John.C.Harrison@Intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pd0-f176.google.com (mail-pd0-f176.google.com [209.85.192.176]) by gabe.freedesktop.org (Postfix) with ESMTP id AD3B76E5D3 for ; Wed, 2 Jul 2014 11:33:39 -0700 (PDT) Received: by mail-pd0-f176.google.com with SMTP id ft15so12351761pdb.35 for ; Wed, 02 Jul 2014 11:33:39 -0700 (PDT) In-Reply-To: <1403803475-16337-18-git-send-email-John.C.Harrison@Intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: John.C.Harrison@Intel.com Cc: Intel-GFX@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 26 Jun 2014 18:24:08 +0100 John.C.Harrison@Intel.com wrote: > From: John Harrison > > The scheduler decouples the submission of batch buffers to the driver with their > submission to the hardware. This basically means splitting the execbuffer() > function in half. This change rearranges some code ready for the split to occur. > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index ec274ef..fda9187 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -32,6 +32,7 @@ > #include "i915_trace.h" > #include "intel_drv.h" > #include > +#include "i915_scheduler.h" > > #define __EXEC_OBJECT_HAS_PIN (1<<31) > #define __EXEC_OBJECT_HAS_FENCE (1<<30) > @@ -874,10 +875,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring, > if (flush_domains & I915_GEM_DOMAIN_GTT) > wmb(); > > - /* Unconditionally invalidate gpu caches and ensure that we do flush > - * any residual writes from the previous batch. > - */ > - return intel_ring_invalidate_all_caches(ring); > + return 0; > } > > static bool > @@ -1219,8 +1217,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > } > } > > - intel_runtime_pm_get(dev_priv); > - > ret = i915_mutex_lock_interruptible(dev); > if (ret) > goto pre_mutex_err; > @@ -1331,6 +1327,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > if (ret) > goto err; > > + i915_gem_execbuffer_move_to_active(&eb->vmas, ring); > + > + /* To be split into two functions here... */ > + > + intel_runtime_pm_get(dev_priv); > + > + /* Unconditionally invalidate gpu caches and ensure that we do flush > + * any residual writes from the previous batch. > + */ > + ret = intel_ring_invalidate_all_caches(ring); > + if (ret) > + goto err; > + > + /* Switch to the correct context for the batch */ > ret = i915_switch_context(ring, ctx); > if (ret) > goto err; > @@ -1381,7 +1391,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > > trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags); > > - i915_gem_execbuffer_move_to_active(&eb->vmas, ring); > i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); > > err: I'd like Chris to take a look too, but it looks safe afaict. Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center