From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 11/43] drm/i915/bdw: Render moot context reset and switch with Execlists Date: Mon, 11 Aug 2014 16:30:15 +0200 Message-ID: <20140811143015.GH10500@phenom.ffwll.local> References: <1406217891-8912-1-git-send-email-thomas.daniel@intel.com> <1406217891-8912-12-git-send-email-thomas.daniel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id B0C226E26D for ; Mon, 11 Aug 2014 07:30:03 -0700 (PDT) Received: by mail-wi0-f177.google.com with SMTP id ho1so4302309wib.4 for ; Mon, 11 Aug 2014 07:30:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1406217891-8912-12-git-send-email-thomas.daniel@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Thomas Daniel Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Jul 24, 2014 at 05:04:19PM +0100, Thomas Daniel wrote: > From: Oscar Mateo > > These two functions make no sense in an Logical Ring Context & Execlists > world. > > v2: We got rid of lrc_enabled and centralized everything in the sanitized > i915.enbale_execlists instead. > > Signed-off-by: Oscar Mateo > --- > drivers/gpu/drm/i915/i915_gem_context.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index fbe7278..288f5de 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -380,6 +380,9 @@ void i915_gem_context_reset(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > int i; > > + if (i915.enable_execlists) > + return; This will conflict badly with Alistair's patch at a functional level. I'm pretty sure that we want _some_ form of reset for the context state, since the hw didn't just magically load the previously running context. So NACK on this hunk. > + > /* Prevent the hardware from restoring the last context (which hung) on > * the next switch */ > for (i = 0; i < I915_NUM_RINGS; i++) { > @@ -514,6 +517,9 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv) > ppgtt->enable(ppgtt); > } > > + if (i915.enable_execlists) > + return 0; Again this conflicts with Alistair's patch. Furthermore it looks redudant since you no-op out i915_switch_context separately. > + > /* FIXME: We should make this work, even in reset */ > if (i915_reset_in_progress(&dev_priv->gpu_error)) > return 0; > @@ -769,6 +775,9 @@ int i915_switch_context(struct intel_engine_cs *ring, > { > struct drm_i915_private *dev_priv = ring->dev->dev_private; > > + if (i915.enable_execlists) > + return 0; I've hoped we don't need this with the legacy ringbuffer cmdsubmission fuly split out. If there are other paths (resume, gpu reset) where this comes into play then I guess we need to look at where the best place is to make this call. So until this comes with a bit a better justification I'll punt on this for now. -Daniel > + > WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); > > if (to->legacy_hw_ctx.rcs_state == NULL) { /* We have the fake context */ > -- > 1.7.9.5 > > _______________________________________________ > 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