[Intel-gfx] [PATCH v4 9/9] drm/i915/perf: Add support for OA media units

[Dixit, Ashutosh]

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