Quoting Tvrtko Ursulin (2019-04-10 11:05:13)
>
> On 08/04/2019 10:17, Chris Wilson wrote:
> > +void __intel_context_enter(struct intel_context *ce)
> > +{
> > + struct drm_i915_private *i915 = ce->gem_context->i915;
> > +
> > + if (!i915->gt.active_requests++)
> > + i915_gem_unpark(i915);
> > +}
> > +
> > +void __intel_context_exit(struct intel_context *ce)
> > +{
> > + struct drm_i915_private *i915 = ce->gem_context->i915;
> > +
> > + GEM_BUG_ON(!i915->gt.active_requests);
> > + if (!--i915->gt.active_requests)
> > + i915_gem_park(i915);
> > +}
>
> In our normal nomenclature __intel_context_enter would be expected to be
> directly called by intel_context_enter on the 0 -> 1 refcnt transition.
> Here they are in fact common helpers, so one level of indirection, via a
> different layer. I found this a bit confusing at first. My usual remedy
> here is to prescribe a better name. __intel_gt_context_enter/exit? To
> designate the glue between backend and GT.
The use of gt/gem here is a placeholder. Here we will be iterating over
the engines (so just the one for the usual contexts, and num_siblings for
virtual).
intel_context_enter_engine() ?
And virtual_context_enter remains distinct.
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> > index ebc861b1a49e..95b1fdc5826a 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> > @@ -73,6 +73,26 @@ static inline void __intel_context_pin(struct intel_context *ce)
> >
> > void intel_context_unpin(struct intel_context *ce);
> >
> > +void __intel_context_enter(struct intel_context *ce);
> > +void __intel_context_exit(struct intel_context *ce);
> > +
> > +static inline void intel_context_enter(struct intel_context *ce)
> > +{
> > + if (!ce->active_count++)
> > + ce->ops->enter(ce);
> > +}
> > +
> > +static inline void intel_context_mark_active(struct intel_context *ce)
> > +{
>
> GEM_BUG_ON(ce->active_count == 0) ?
We're no longer in Kansas. I was wondering if I could start GT_BUG_ON
and include "intel_gt.h" with a goal of doing a search and replace within
gt/. That gives us some separation from GEM, and maybe we could even
start filtering based on need.
> And some lockdep_assert_held in all three?
Read on :(
The plan is for intel_context_enter/_exit to be under the
timeline->mutex, but that isn't realised for about another 30 patches.
mark_active has special protection because it gets used from the
serialised portion of intel_engine_park.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
