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

[Zbigniew Kempczyński]

On Tue, Jun 15, 2021 at 01:23:19PM -0500, 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.

Both interfaces returns same engine set with the respect of the order
what shouldn't be the problem at all. SAs we started to be more familiar
with your code for_each_ctx_engine(fd, &intel_ctx_0(), e) looks good 
for me and I know what to expect on that context. Please change to that
- diff will be similar to others where you'd replaced 
__for_each_physical_engine to for_each_ctx_engine().

--
Zbigniew