On Fri, Aug 08, 2014 at 01:03:53PM +0000, Thierry, Michel wrote: > > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf > > Of Daniel Vetter > > Sent: Wednesday, August 06, 2014 2:05 PM > > To: Intel Graphics Development > > Cc: Daniel Vetter > > Subject: [PATCH 08/15] drm/i915: Fix up checks for aliasing > ppgtt > > > > A subsequent patch will no longer initialize the aliasing ppgtt if we > > have full ppgtt enabled, since we simply don't need that any more. > > > > Unfortunately a few places check for the aliasing ppgtt instead of > > checking for ppgtt in general. Fix them up. > > > > One special case are the gtt offset and size macros, which have some > > code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt > > is _not_ a logical address space, so passing that in as the vm is > > plain and simple a bug. So just WARN about it and carry on - we have a > > gracefully fall-through anyway if we can't find the vma. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_cmd_parser.c | 4 +--- > > drivers/gpu/drm/i915/i915_dma.c | 2 +- > > drivers/gpu/drm/i915/i915_gem.c | 8 ++------ > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > 4 files changed, 5 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c > > b/drivers/gpu/drm/i915/i915_cmd_parser.c > > index dea99d92fb4a..c45856bcc8b9 100644 > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > > @@ -842,8 +842,6 @@ finish: > > */ > > bool i915_needs_cmd_parser(struct intel_engine_cs *ring) > > { > > - struct drm_i915_private *dev_priv = ring->dev->dev_private; > > - > > if (!ring->needs_cmd_parser) > > return false; > > > > @@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs > > *ring) > > * disabled. That will cause all of the parser's PPGTT checks to > > * fail. For now, disable parsing when PPGTT is off. > > */ > > - if (!dev_priv->mm.aliasing_ppgtt) > > + if (USES_PPGTT(ring->dev)) > > return false; > > > > return (i915.enable_cmd_parser == 1); > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > b/drivers/gpu/drm/i915/i915_dma.c > > index 2e7f03ad5ee2..e5ac1a6e9d26 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void > > *data, > > value = HAS_WT(dev); > > break; > > case I915_PARAM_HAS_ALIASING_PPGTT: > > - value = dev_priv->mm.aliasing_ppgtt || > > USES_FULL_PPGTT(dev); > > + value = USES_PPGTT(dev); > > break; > > case I915_PARAM_HAS_WAIT_TIMEOUT: > > value = 1; > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index d8399ee622b9..a79deb189146 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct > > drm_i915_gem_object *o, > > struct drm_i915_private *dev_priv = o->base.dev->dev_private; > > struct i915_vma *vma; > > > > - if (!dev_priv->mm.aliasing_ppgtt || > > - vm == &dev_priv->mm.aliasing_ppgtt->base) > > - vm = &dev_priv->gtt.base; > > + WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); > > > > list_for_each_entry(vma, &o->vma_list, vma_link) { > > if (vma->vm == vm) > > @@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct > > drm_i915_gem_object *o, > > struct drm_i915_private *dev_priv = o->base.dev->dev_private; > > struct i915_vma *vma; > > > > - if (!dev_priv->mm.aliasing_ppgtt || > > - vm == &dev_priv->mm.aliasing_ppgtt->base) > > - vm = &dev_priv->gtt.base; > > + WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); > > > > BUG_ON(list_empty(&o->vma_list)); > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 05969f03c0c1..390e38325b37 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct > > intel_engine_cs *ring, > > u64 offset, u32 len, > > unsigned flags) > > { > > - struct drm_i915_private *dev_priv = ring->dev->dev_private; > > - bool ppgtt = dev_priv->mm.aliasing_ppgtt != NULL && > > - !(flags & I915_DISPATCH_SECURE); > > + bool ppgtt = USES_PPGTT(ring->dev) && !(flags & > > I915_DISPATCH_SECURE); > > int ret; > > > > ret = intel_ring_begin(ring, 4); > > -- > > 1.9.3 > > > Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more > gen8_ppgtt_info's fault, so I'm sending a follow up patch for it. > Unless you want to combine them... > Reviewed-by: Michel Thierry <michel.thierry@xxxxxxxxx> Actually I spotted that one and decided that I can't break things worse than it is - the aliasing ppgtt for full ppgtt is completely unused (except for these checks), so dumping it doesn't add value. To check: Does your r-b apply here still even without any fixup for gen8 ppgtt_info? -Daniel > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
