On Fri, 10 Mar 2023 08:39:27 -0800, Umesh Nerlige Ramappa wrote:
>
Hi Umesh,
> On Thu, Mar 09, 2023 at 03:57:48PM -0800, Dixit, Ashutosh wrote:
> > On Tue, 07 Mar 2023 12:16:11 -0800, Umesh Nerlige Ramappa wrote:
> >>
> >> -static int gen8_configure_context(struct i915_gem_context *ctx,
> >> +static int gen8_configure_context(struct i915_perf_stream *stream,
> >> + struct i915_gem_context *ctx,
> >> struct flex *flex, unsigned int count)
> >> {
> >> struct i915_gem_engines_iter it;
> >> @@ -2573,7 +2594,8 @@ static int gen8_configure_context(struct i915_gem_context *ctx,
> >> for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
> >> GEM_BUG_ON(ce == ce->engine->kernel_context);
> >>
> >> - if (!engine_supports_oa(ce->engine))
> >> + if (!engine_supports_oa(ce->engine) ||
> >> + ce->engine->class != stream->engine->class)
> >> continue;
> >>
> >> /* Otherwise OA settings will be set upon first use */
> >> @@ -2704,7 +2726,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
> >>
> >> spin_unlock(&i915->gem.contexts.lock);
> >>
> >> - err = gen8_configure_context(ctx, regs, num_regs);
> >> + err = gen8_configure_context(stream, ctx, regs, num_regs);
> >> if (err) {
> >> i915_gem_context_put(ctx);
> >> return err;
> >> @@ -2724,7 +2746,8 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
> >> for_each_uabi_engine(engine, i915) {
> >> struct intel_context *ce = engine->kernel_context;
> >>
> >> - if (!engine_supports_oa(ce->engine))
> >> + if (!engine_supports_oa(ce->engine) ||
> >> + ce->engine->class != stream->engine->class)
> >> continue;
> >>
> >> regs[0].value = intel_sseu_make_rpcs(engine->gt, &ce->sseu);
> >> @@ -2749,6 +2772,9 @@ gen12_configure_all_contexts(struct i915_perf_stream *stream,
> >> },
> >> };
> >>
> >> + if (stream->engine->class != RENDER_CLASS)
> >> + return 0;
> >> +
> >> return oa_configure_all_contexts(stream,
> >> regs, ARRAY_SIZE(regs),
> >> active);
> >
> > Can you please explain the above changes? Why are we checking for
> > engine->class above? Should we be checking for both class and instance? Or
> > all engines connected to an OA unit (multiple classes can be connected to
> > an OA unit and be different from stream->engine->class, e.g. VDBOX and
> > VEBOX)? oa_configure_all_contexts is also called from
> > lrc_configure_all_contexts.
>
> Only render (and compute when we support it) have OA specific configuration
> in the context image. Media engines do not have any context specific
> configurations.
Yes I remember you answered this previously too. My question still is why
did we make the 2 instances of this change above:
>From the original code in drm-tip:
if (engine->class != RENDER_CLASS)
continue;
To the final code (changed in two patches):
if (!engine_supports_oa(ce->engine) ||
ce->engine->class != stream->engine->class)
continue;
Thanks.
--
Ashutosh
