From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 09/53] drm/i915/bdw: Initialization for Logical Ring Contexts Date: Wed, 18 Jun 2014 22:24:36 +0200 Message-ID: <20140618202436.GZ5821@phenom.ffwll.local> References: <1402673891-14618-1-git-send-email-oscar.mateo@intel.com> <1402673891-14618-10-git-send-email-oscar.mateo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f178.google.com (mail-we0-f178.google.com [74.125.82.178]) by gabe.freedesktop.org (Postfix) with ESMTP id 90C406E78E for ; Wed, 18 Jun 2014 13:24:43 -0700 (PDT) Received: by mail-we0-f178.google.com with SMTP id x48so1376484wes.23 for ; Wed, 18 Jun 2014 13:24:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1402673891-14618-10-git-send-email-oscar.mateo@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: oscar.mateo@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jun 13, 2014 at 04:37:27PM +0100, oscar.mateo@intel.com wrote: > From: Oscar Mateo > > Early in the series we had our own gen8_gem_context_init/fini > functions, but the truth is they now look almost the same as the > legacy hw context init/fini functions. We can always split them > later if this ceases to be the case. > > Also, we do not fall back to legacy ringbuffers when logical ring > context initialization fails (not very likely to happen and, even > if it does, hw contexts would probably fail as well). > > Signed-off-by: Oscar Mateo > --- > drivers/gpu/drm/i915/i915_gem_context.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 801b891..3f3fb36 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -416,7 +416,13 @@ int i915_gem_context_init(struct drm_device *dev) > if (WARN_ON(dev_priv->ring[RCS].default_context)) > return 0; > > - if (HAS_HW_CONTEXTS(dev)) { > + dev_priv->lrc_enabled = intel_enable_execlists(dev); > + > + if (dev_priv->lrc_enabled) { > + /* NB: intentionally left blank. We will allocate our own > + * backing objects as we need them, thank you very much */ > + dev_priv->hw_context_size = 0; > + } else if (HAS_HW_CONTEXTS(dev)) { > dev_priv->hw_context_size = round_up(get_context_size(dev), 4096); > if (dev_priv->hw_context_size > (1<<20)) { > DRM_DEBUG_DRIVER("Disabling HW Contexts; invalid size %d\n", > @@ -436,7 +442,9 @@ int i915_gem_context_init(struct drm_device *dev) > for (i = 0; i < I915_NUM_RINGS; i++) > dev_priv->ring[i].default_context = ctx; > > - DRM_DEBUG_DRIVER("%s context support initialized\n", dev_priv->hw_context_size ? "HW" : "fake"); > + DRM_DEBUG_DRIVER("%s context support initialized\n", > + dev_priv->lrc_enabled ? "LR" : > + dev_priv->hw_context_size ? "HW" : "fake"); > return 0; > } > > @@ -765,9 +773,12 @@ int i915_switch_context(struct intel_engine_cs *ring, > return do_switch(ring, to); > } > > -static bool hw_context_enabled(struct drm_device *dev) > +static bool contexts_enabled(struct drm_device *dev) > { > - return to_i915(dev)->hw_context_size; > + struct drm_i915_private *dev_priv = to_i915(dev); > + > + /* FIXME: this would be cleaner with a "context type" enum */ > + return dev_priv->lrc_enabled || dev_priv->hw_context_size; Since you have a bunch of if ladders the usual approach isn't an enum but a vfunc table to abstract behaviour. Think object types instead of switch statements. Style bikeshed though (presume code later on doesn't have excesses here). -Daniel > } > > int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > @@ -778,7 +789,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > struct intel_context *ctx; > int ret; > > - if (!hw_context_enabled(dev)) > + if (!contexts_enabled(dev)) > return -ENODEV; > > ret = i915_mutex_lock_interruptible(dev); > -- > 1.9.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch