[igt-dev] [PATCH i-g-t 01/79] lib/i915/gem_submission_measure: Take an optional intel_ctx_cfg_t

[Dixit, Ashutosh]

On Thu, 17 Jun 2021 12:12:08 -0700, Jason Ekstrand wrote:
>
> If provided, the engine (or ALL_ENGINES) is relative to the given
> context config.  This is intended to be transitional.  We'll get rid of
> all the __for_each_physical_engine stuff later.

Please add your SOB here.

There are couple of comments below but this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>

> @@ -372,8 +373,10 @@ __measure_ringsize(int i915, unsigned int engine)
>	return count / 2 - 2;
>  }
>
> -unsigned int gem_submission_measure(int i915, unsigned int engine)
> +unsigned int gem_submission_measure(int i915, const intel_ctx_cfg_t *cfg,
> +				    unsigned int engine)

I was thinking since we are already creating an intel_ctx_t in the function
we can just create an intel_ctx_t for the "engine" arg, so shouldn't need
the "cfg". But turns out "engine" is intel_execution_engine2->flags and
there is at least for now no straightforward way to create a intel_ctx_t
for intel_execution_engine2->flags. That is probably why "cfg" is
introduced.

> @@ -381,19 +384,41 @@ unsigned int gem_submission_measure(int i915, unsigned int engine)
>	if (!nonblock)
>		fcntl(i915, F_SETFL, fcntl(i915, F_GETFL) | O_NONBLOCK);
>
> +	if (cfg) {
> +		if (gem_has_contexts(i915))
> +			ctx = intel_ctx_create(i915, cfg);
> +		else
> +			ctx = intel_ctx_0(i915);

I would say this check should be done in intel_ctx_create() itself. Right
now it is being done in intel_ctx_create_all_physical() but we should move
it inside intel_ctx_create() to avoid these kinds of situations. Anyway not
an issue for now, we can clean it up later.