[igt-dev] [PATCH i-g-t] tests/i915/gem_sync: igt_require to check availability of engine in list_engines

[Zbigniew Kempczyński]

On Mon, Aug 23, 2021 at 11:39:21AM +0530, Melkaveri, Arjun wrote:
> On Mon, Aug 23, 2021 at 05:26:08AM +0200, Zbigniew Kempczyński wrote:
> > On Sun, Aug 22, 2021 at 05:46:26PM +0530, Arjun Melkaveri wrote:
> > > Replaced igt_assert with igt_require in list_engines , to
> > > avoid false failure of test case if engine is not supported
> > > or found .
> > > Test will return Test requirement not met.
> > > 
> > > Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> > > Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > > ---
> > >  tests/i915/gem_sync.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> > > index 6cb00c40..4ad264a2 100644
> > > --- a/tests/i915/gem_sync.c
> > > +++ b/tests/i915/gem_sync.c
> > > @@ -106,9 +106,9 @@ list_engines(int fd, const intel_ctx_t *ctx, unsigned ring)
> > >  		ied = intel_engine_list_for_ctx_cfg(fd, &ctx->cfg);
> > >  	} else {
> > >  		if (ctx->cfg.num_engines)
> > > -			igt_assert(ring < ctx->cfg.num_engines);
> > > +			igt_require(ring < ctx->cfg.num_engines);
> > >  		else
> > > -			igt_assert(gem_has_ring(fd, ring));
> > > +			igt_require(gem_has_ring(fd, ring));
> > 
> > To be honest I don't like this change. If we have invalid ring
> > here it smells like bug in the code. If caller is passed 
> > ctx it should be created over cfg, so it contains valid (imo)
> > engine set. Thus if someone passed invalid ring I would assume
> > this is bug in the code and assert is correct bahavior here.
> > 
> > What are scenarios you're trying to handle?
> > 
> > --
> > Zbigniew
> This test is asserting for render engine in platform that doesnt support it.
> It used to skip before test was converted to use intel_ctx_t .
> (https://patchwork.freedesktop.org/patch/442850/)

I think render engine should be excluded in DRM_I915_QUERY_ENGINE_INFO query.
If it is not imo this is a bug. Kernel shouldn't return render engine if it is
not available on some platform.

--
Zbigniew

> 
> -Arjun 
> > 
> > >  
> > >  		ied.engines[ied.nengines].flags = ring;
> > >  		strcpy(ied.engines[ied.nengines].name, " ");
> > > -- 
> > > 2.25.1
> > >