Re: [PATCH] drm/i915: Fix up the vma aliasing ppgtt binding

Daniel Vetter
2015-05-04T09:18:22Z

On Mon, May 04, 2015 at 10:06:51AM +0100, Chris Wilson wrote:
> On Mon, May 04, 2015 at 10:49:30AM +0200, Daniel Vetter wrote:
> > On Fri, Apr 24, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index 9e06180..bbeb6c3 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -1949,8 +1949,6 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma,
> > >
> > > BUG_ON(!i915_is_ggtt(vma->vm));
> > > intel_gtt_insert_sg_entries(vma->ggtt_view.pages, entry, flags);
> > > -
> > > - vma->bound |= GLOBAL_BIND;
> >
> > Hm, why remove this? I added this to avoid double-binding on platforms
> > where one implies the other binding type. Maybe should have been a bit
> > more consistent with bound |= GLOBAL_BIND | LOCAL_BIND.
>
> Hmm, didn't we end with doing vma->bound = bind_flags? But I think if
> you did if (!HAS_PPGTT) bind_flags |= GLOBAL_BIND in i915_vma_bind, then
> we have all the vma->bound processing in the one place.

Yeah I think that's the better option and makes it clearer what's going
on. I'll send out a patch for that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation