Quoting Tvrtko Ursulin (2019-11-25 10:44:15)
>
> On 24/11/2019 17:05, Chris Wilson wrote:
> > +static inline struct i915_request *
> > +intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > +{
> > + struct i915_request *rq;
> > +
> > + /*
> > + * The engine->kernel_context is special as it is used inside
> > + * the engine-pm barrier (see __engine_park()), circumventing
> > + * the usual mutexes and relying on the engine-pm barrier
> > + * instead. So whenever we use the engine->kernel_context
> > + * outside of the barrier, we must manually handle the
> > + * engine wakeref to serialise with the use inside.
> > + */
> > + intel_engine_pm_get(engine);
> > + rq = i915_request_create(engine->kernel_context);
> > + intel_engine_pm_put(engine);
>
> i915_request_add does not have to be covered by the pm ref?
No, the normal course of action is:
i915_request_create:
mutex_lock(timeline->mutex)
intel_context_enter:
if (!ce->active_count++) {
intel_engine_pm_get();
intel_timeline_enter();
}
With the primary purpose of reducing atomics behind a simple usage
counter covered by the timeline->mutex.
Of course, engine-pm tries to be clever because it has to be called
under any timeline->mutex from retire, and so tries to fake
timeline->mutex itself.
> I am slightly confused by how patch converts some to this helper and at
> some places it open codes it.
There were a few perf benchmarks, that I thought better to not add the
extra pair of atomics, and intel_engine_heartbeat.c is supposed to
cognisant of the rules and only operates on an active engine-pm.
The intent of intel_engine_create_kernel_request() was to apply a simple
fixup with a comment as to what is going on with the kernel_context.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
