From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [RFC 12/44] drm/i915: Disable hardware semaphores when GPU scheduler is enabled Date: Wed, 2 Jul 2014 11:16:32 -0700 Message-ID: <20140702111632.1d33a8e2@jbarnes-desktop> References: <1403803475-16337-1-git-send-email-John.C.Harrison@Intel.com> <1403803475-16337-13-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-pa0-f43.google.com (mail-pa0-f43.google.com [209.85.220.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 90E776E5D3 for ; Wed, 2 Jul 2014 11:15:48 -0700 (PDT) Received: by mail-pa0-f43.google.com with SMTP id lf10so12956320pab.16 for ; Wed, 02 Jul 2014 11:15:48 -0700 (PDT) In-Reply-To: <1403803475-16337-13-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:03 +0100 John.C.Harrison@Intel.com wrote: > From: John Harrison > > Hardware sempahores require seqno values to be continuously incrementing. > However, the scheduler's reordering of batch buffers means that the seqno values > going through the hardware could be out of order. Thus semaphores can not be > used. > > On the other hand, the scheduler superceeds the need for hardware semaphores > anyway. Having one ring stall waiting for something to complete on another ring > is inefficient if that ring could be working on some other, independent task. > This is what the scheduler is meant to do - keep the hardware as busy as > possible by reordering batch buffers to avoid dependency stalls. > --- > drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++ > drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++++ > drivers/gpu/drm/i915/i915_scheduler.h | 1 + > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++++ > 4 files changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index e2bfdda..748b13a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -33,6 +33,7 @@ > #include "i915_drv.h" > #include "i915_trace.h" > #include "intel_drv.h" > +#include "i915_scheduler.h" > > #include > #include > @@ -468,6 +469,14 @@ void intel_detect_pch(struct drm_device *dev) > > bool i915_semaphore_is_enabled(struct drm_device *dev) > { > + /* Hardware semaphores are not compatible with the scheduler due to the > + * seqno values being potentially out of order. However, semaphores are > + * also not required as the scheduler will handle interring dependencies > + * and try do so in a way that does not cause dead time on the hardware. > + */ > + if (i915_scheduler_is_enabled(dev)) > + return 0; > + > if (INTEL_INFO(dev)->gen < 6) > return false; > > diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c > index e9aa566..d9c1879 100644 > --- a/drivers/gpu/drm/i915/i915_scheduler.c > +++ b/drivers/gpu/drm/i915/i915_scheduler.c > @@ -26,6 +26,15 @@ > #include "intel_drv.h" > #include "i915_scheduler.h" > > +bool i915_scheduler_is_enabled(struct drm_device *dev) > +{ > +#ifdef CONFIG_DRM_I915_SCHEDULER > + return true; > +#else > + return false; > +#endif > +} I think this should be: if (dev_priv->scheduler) return true; return false; instead? Otherwise looks fine. -- Jesse Barnes, Intel Open Source Technology Center