On Thu, Jun 01, 2017 at 01:24:55PM +0300, Joonas Lahtinen wrote:
> On to, 2017-06-01 at 12:03 +0200, Michal Wajdeczko wrote:
> > On Thu, Jun 01, 2017 at 10:04:46AM +0100, Chris Wilson wrote:
> > >
> > > void i915_ggtt_disable_guc(struct drm_i915_private *i915)
> > > {
> > > - if (i915->ggtt.invalidate == guc_ggtt_invalidate)
> > > - i915->ggtt.invalidate = gen6_ggtt_invalidate;
> > > + /* We should only be called after i915_ggtt_enable_guc() */
> > > + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
> > > +
> > > + i915->ggtt.invalidate = gen6_ggtt_invalidate;
> > > }
> >
> > While this looks correct today, it may not work in the future if we
> > will need somethig other than gen6_ggtt_invalidate() as base invalidate
> > function or guc_gtt_invalidate() as the one for the guc. Just a head up.
>
> Currently the assignment is directly to gen6_ggtt_invalidate, no
> questions asked. So I don't think the assert could be much more :)
>
> Maybe GuC code should backup the invalidate function before overriding.
Possibly, but I hope the alternatives are a little better known so that
we don't have to do save/restore of function pointers too often.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
