Re: [igt-dev] [RFC PATCH v8 5/5] tests: gem_exec_basic: add "exec-ctx" buffer execution demo test - Andi Shyti via igt-dev

From: Andi Shyti via igt-dev <igt-dev@lists.freedesktop.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: IGT dev <igt-dev@lists.freedesktop.org>,
	Andi Shyti <andi@etezian.org>,
	Petri Latvala <petri.latvala@intel.com>
Subject: Re: [igt-dev] [RFC PATCH v8 5/5] tests: gem_exec_basic: add "exec-ctx" buffer execution demo test
Date: Wed, 13 Feb 2019 03:02:36 +0200	[thread overview]
Message-ID: <20190213010236.GB4740@intel.intel> (raw)
In-Reply-To: <155001739294.13414.4413937487335739365@skylake-alporthouse-com>

Hi Chris,

> > +       igt_subtest("exec-ctx") {
> > +               uint32_t ctx_id;
> > +               struct intel_execution_engine2 *e2;
> > +               int index_map = 0;
> > +
> > +               igt_require_gem_engine_list(fd);
> > +               ctx_id = gem_context_create(fd);
> > +
> > +               for_each_engine2(fd, ctx_id, e2)
> > +                       igt_assert(gem_has_ring_by_idx(fd, ++index_map,
> > +                                                      ctx_id));
> 
> Since the iterator here is e2, it looks quite abusive not to use the
> iterator... I don't think this serves as a good example of
> for_each_engine2().

I thought about hiding e2 inside for_each_engine2, but I also
thought the user might want to use it, e.g.:

  for_each_engine2(fd, ctx_id, e2)
    printf("%s\n", e2->name);

if I keep it hidden then, although it's there, it's not visible.
But, yes, I can hide it inside the for_each_engine2 and perhaps
have another iterator where e2 is meant to be used.

Thanks,
Andi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev