Re: [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt

Daniel Vetter
2015-04-20T16:02:12Z

On Fri, Apr 17, 2015 at 04:49:18PM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>
> > On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote:
> >> On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote:
> >> > On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote:
> >> > > On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote:
> >> > > > We load the ppgtt ptes once per gpu reset/driver load/resume and
> >> > > > that's all that's needed. Note that this only blows up when we're
> >> > > > using the allocate_va_range funcs and not the special-purpose ones
> >> > > > used. With this change we can get rid of that duplication.
> >> > >
> >> > > Honestly, I would prefer the test to be rewritten so that the
> >> > > information on which vm was being used was passed along and not that
> >> > > magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly
> >> > > what vm it bound the objects into, and yet towards the end we decide to
> >> > > guess again. Also, I would rather the execbuffer test be moved to
> >> > > i915_gem_context since it is scattering internal knowledge about.
> >> >
> >> > Yeah I agree that there's more room for cleanup. I've done this minimal
> >> > patch purely to shut up the bogus WARN_ONs when I tried to unify the
> >> > gen6/7 pt alloc code in the next patch. Since it's bogus.
> >>
> >> How about:
> >
> > Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx
> > switch logic and failed, and I wasn't fully convinced that the gen7 one
> > won't WARN if we actually enable full ppgtt. Given all that I decided to
> > go with the most minimal patch and just removed the offending bogus WARN.
> > But Mika/Michel promised to hang around for eventual cleanups?
>
> Yes. There is more to come after this series.
> I can include Chris's suggestion.

No r-b as an interim solution to move this patch series forward? Without
this we'll WARN.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation