From mboxrd@z Thu Jan 1 00:00:00 1970 From: Damien Lespiau Subject: Re: [PATCH] drm/i915/bdw: Populate lrc with aliasing ppgtt if required Date: Mon, 18 Aug 2014 17:49:29 +0100 Message-ID: <20140818164929.GD32149@strange.ger.corp.intel.com> References: <1408373642-16514-1-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 mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 5AAD36E4AA for ; Mon, 18 Aug 2014 09:49:34 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1408373642-16514-1-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 Mon, Aug 18, 2014 at 03:54:02PM +0100, Thomas Daniel wrote: > A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops > on boot. Add a check so that is full PPGTT is not in use the context is > populated with the aliasing PPGTT. > > Issue: VIZ-4278 > Signed-off-by: Thomas Daniel > --- > drivers/gpu/drm/i915/intel_lrc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index c096b9b..79a6b91 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1452,12 +1452,19 @@ static int > populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_obj, > struct intel_engine_cs *ring, struct intel_ringbuffer *ringbuf) > { > + struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_i915_gem_object *ring_obj = ringbuf->obj; > struct i915_hw_ppgtt *ppgtt = ctx->ppgtt; It's a bit weird to leave ppgtt initialized here when you're going to always override it below. > struct page *page; > uint32_t *reg_state; > int ret; > > + if (USES_FULL_PPGTT(dev)) > + ppgtt = ctx->ppgtt; > + else > + ppgtt = dev_priv->mm.aliasing_ppgtt; > + The patch drom daniel you mention removes the usage of USES_FULL_PPGTT() to directly test ctx->ppgtt. You may want to reproduce that here ie. ppgtt = ctx->ppgtt; if (!ppgtt) ppgtt = dev_priv->mm.aliasing_ppgtt; doesn't really matter either way I guess, but can we at least not set then always override ppgtt? Thanks, -- Damien