[igt-dev] [PATCH i-g-t 54/77] lib/i915: Use for_each_physical_ring for submission tests

[Dixit, Ashutosh]

On Tue, 15 Jun 2021 11:23:19 -0700, Jason Ekstrand wrote:
>
> On Tue, Jun 15, 2021 at 3:44 AM Zbigniew Kempczyński
> <zbigniew.kempczynski at intel.com> wrote:
> >
> > On Mon, Jun 14, 2021 at 11:38:39AM -0500, Jason Ekstrand wrote:
> > > This does make the assumption that ctx0 has the default set of engines
> > > but, now that we've converted everything to intel_ctx_t, this assumption
> > > should be ok.
> > >
> > > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > > ---
> > >  lib/i915/gem_submission.c | 13 +++++--------
> > >  1 file changed, 5 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
> > > index bd4bbb3ef..1abc73ec7 100644
> > > --- a/lib/i915/gem_submission.c
> > > +++ b/lib/i915/gem_submission.c
> > > @@ -33,6 +33,7 @@
> > >  #include "i915/gem.h"
> > >  #include "i915/gem_create.h"
> > >  #include "i915/gem_engine_topology.h"
> > > +#include "i915/gem_ring.h"
> > >  #include "i915/gem_submission.h"
> > >
> > >  #include "igt_core.h"
> > > @@ -200,10 +201,8 @@ void gem_test_engine(int i915, unsigned int engine)
> > >       gem_write(i915, obj.handle, 0, &bbe, sizeof(bbe));
> > >
> > >       if (engine == ALL_ENGINES) {
> > > -             const struct intel_execution_engine2 *e2;
> > > -
> > > -             __for_each_physical_engine(i915, e2) {
> > > -                     execbuf.flags = e2->flags;
> > > +             for_each_physical_ring(e, i915) {
> > > +                     execbuf.flags = eb_ring(e);
> > >                       gem_execbuf(i915, &execbuf);
> >
> > Why not to use for_each_ctx_engine() with intel intel_ctx_0 here?
>
> We could.  I do kind-of like the fact that it's obvious and clear that
> it's testing physical rings instead of going through the engines API.
> It does say explicitly in the docs that it takes an I915_EXEC_RING
> enum.  But I'd be happy to change if you think it'd be more clear that
> way.

Since the code uses __for_each_physical_engine it should be running on all
the physical engines I'd think, so it would have to go through the engines
API. When the code was written "rings" and engines might have been
identical.