| Message ID | 20200518102911.3463-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftests: Refactor sibling selection | expand |
On 18/05/2020 11:29, Chris Wilson wrote: > Tvrtko spotted that some selftests were using 'break' not 'continue', > which will fail for discontiguous engine layouts such as on Icelake > (which may have vcs0 and vcs2). > > Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/selftest_lrc.c | 68 ++++++++++---------------- > 1 file changed, 27 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 824f99c4cc7c..94854a467e66 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -3600,13 +3600,30 @@ static int nop_virtual_engine(struct intel_gt *gt, > return err; > } > > +static unsigned int select_siblings(struct intel_gt *gt, > + unsigned int class, > + struct intel_engine_cs **siblings) > +{ > + unsigned int n = 0; > + unsigned int inst; > + > + for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > + if (!gt->engine_class[class][inst]) > + continue; > + > + siblings[n++] = gt->engine_class[class][inst]; > + } > + > + return n; > +} > + > static int live_virtual_engine(void *arg) > { > struct intel_gt *gt = arg; > struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; > struct intel_engine_cs *engine; > enum intel_engine_id id; > - unsigned int class, inst; > + unsigned int class; > int err; > > if (intel_uc_uses_guc_submission(>->uc)) > @@ -3624,13 +3641,7 @@ static int live_virtual_engine(void *arg) > for (class = 0; class <= MAX_ENGINE_CLASS; class++) { > int nsibling, n; > > - nsibling = 0; > - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > - if (!gt->engine_class[class][inst]) > - continue; > - > - siblings[nsibling++] = gt->engine_class[class][inst]; > - } > + nsibling = select_siblings(gt, class, siblings); > if (nsibling < 2) > continue; > > @@ -3739,7 +3750,7 @@ static int live_virtual_mask(void *arg) > { > struct intel_gt *gt = arg; > struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; > - unsigned int class, inst; > + unsigned int class; > int err; > > if (intel_uc_uses_guc_submission(>->uc)) > @@ -3748,13 +3759,7 @@ static int live_virtual_mask(void *arg) > for (class = 0; class <= MAX_ENGINE_CLASS; class++) { > unsigned int nsibling; > > - nsibling = 0; > - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > - if (!gt->engine_class[class][inst]) > - break; > - > - siblings[nsibling++] = gt->engine_class[class][inst]; > - } > + nsibling = select_siblings(gt, class, siblings); > if (nsibling < 2) > continue; > > @@ -3876,7 +3881,7 @@ static int live_virtual_preserved(void *arg) > { > struct intel_gt *gt = arg; > struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; > - unsigned int class, inst; > + unsigned int class; > > /* > * Check that the context image retains non-privileged (user) registers > @@ -3894,13 +3899,7 @@ static int live_virtual_preserved(void *arg) > for (class = 0; class <= MAX_ENGINE_CLASS; class++) { > int nsibling, err; > > - nsibling = 0; > - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > - if (!gt->engine_class[class][inst]) > - continue; > - > - siblings[nsibling++] = gt->engine_class[class][inst]; > - } > + nsibling = select_siblings(gt, class, siblings); > if (nsibling < 2) > continue; > > @@ -4111,7 +4110,7 @@ static int live_virtual_bond(void *arg) > }; > struct intel_gt *gt = arg; > struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; > - unsigned int class, inst; > + unsigned int class; > int err; > > if (intel_uc_uses_guc_submission(>->uc)) > @@ -4121,14 +4120,7 @@ static int live_virtual_bond(void *arg) > const struct phase *p; > int nsibling; > > - nsibling = 0; > - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > - if (!gt->engine_class[class][inst]) > - break; > - > - GEM_BUG_ON(nsibling == ARRAY_SIZE(siblings)); > - siblings[nsibling++] = gt->engine_class[class][inst]; > - } > + nsibling = select_siblings(gt, class, siblings); > if (nsibling < 2) > continue; > > @@ -4266,7 +4258,7 @@ static int live_virtual_reset(void *arg) > { > struct intel_gt *gt = arg; > struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; > - unsigned int class, inst; > + unsigned int class; > > /* > * Check that we handle a reset event within a virtual engine. > @@ -4284,13 +4276,7 @@ static int live_virtual_reset(void *arg) > for (class = 0; class <= MAX_ENGINE_CLASS; class++) { > int nsibling, err; > > - nsibling = 0; > - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { > - if (!gt->engine_class[class][inst]) > - continue; > - > - siblings[nsibling++] = gt->engine_class[class][inst]; > - } > + nsibling = select_siblings(gt, class, siblings); > if (nsibling < 2) > continue; > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index 824f99c4cc7c..94854a467e66 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -3600,13 +3600,30 @@ static int nop_virtual_engine(struct intel_gt *gt, return err; } +static unsigned int select_siblings(struct intel_gt *gt, + unsigned int class, + struct intel_engine_cs **siblings) +{ + unsigned int n = 0; + unsigned int inst; + + for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { + if (!gt->engine_class[class][inst]) + continue; + + siblings[n++] = gt->engine_class[class][inst]; + } + + return n; +} + static int live_virtual_engine(void *arg) { struct intel_gt *gt = arg; struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; struct intel_engine_cs *engine; enum intel_engine_id id; - unsigned int class, inst; + unsigned int class; int err; if (intel_uc_uses_guc_submission(>->uc)) @@ -3624,13 +3641,7 @@ static int live_virtual_engine(void *arg) for (class = 0; class <= MAX_ENGINE_CLASS; class++) { int nsibling, n; - nsibling = 0; - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { - if (!gt->engine_class[class][inst]) - continue; - - siblings[nsibling++] = gt->engine_class[class][inst]; - } + nsibling = select_siblings(gt, class, siblings); if (nsibling < 2) continue; @@ -3739,7 +3750,7 @@ static int live_virtual_mask(void *arg) { struct intel_gt *gt = arg; struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; - unsigned int class, inst; + unsigned int class; int err; if (intel_uc_uses_guc_submission(>->uc)) @@ -3748,13 +3759,7 @@ static int live_virtual_mask(void *arg) for (class = 0; class <= MAX_ENGINE_CLASS; class++) { unsigned int nsibling; - nsibling = 0; - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { - if (!gt->engine_class[class][inst]) - break; - - siblings[nsibling++] = gt->engine_class[class][inst]; - } + nsibling = select_siblings(gt, class, siblings); if (nsibling < 2) continue; @@ -3876,7 +3881,7 @@ static int live_virtual_preserved(void *arg) { struct intel_gt *gt = arg; struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; - unsigned int class, inst; + unsigned int class; /* * Check that the context image retains non-privileged (user) registers @@ -3894,13 +3899,7 @@ static int live_virtual_preserved(void *arg) for (class = 0; class <= MAX_ENGINE_CLASS; class++) { int nsibling, err; - nsibling = 0; - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { - if (!gt->engine_class[class][inst]) - continue; - - siblings[nsibling++] = gt->engine_class[class][inst]; - } + nsibling = select_siblings(gt, class, siblings); if (nsibling < 2) continue; @@ -4111,7 +4110,7 @@ static int live_virtual_bond(void *arg) }; struct intel_gt *gt = arg; struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; - unsigned int class, inst; + unsigned int class; int err; if (intel_uc_uses_guc_submission(>->uc)) @@ -4121,14 +4120,7 @@ static int live_virtual_bond(void *arg) const struct phase *p; int nsibling; - nsibling = 0; - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { - if (!gt->engine_class[class][inst]) - break; - - GEM_BUG_ON(nsibling == ARRAY_SIZE(siblings)); - siblings[nsibling++] = gt->engine_class[class][inst]; - } + nsibling = select_siblings(gt, class, siblings); if (nsibling < 2) continue; @@ -4266,7 +4258,7 @@ static int live_virtual_reset(void *arg) { struct intel_gt *gt = arg; struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; - unsigned int class, inst; + unsigned int class; /* * Check that we handle a reset event within a virtual engine. @@ -4284,13 +4276,7 @@ static int live_virtual_reset(void *arg) for (class = 0; class <= MAX_ENGINE_CLASS; class++) { int nsibling, err; - nsibling = 0; - for (inst = 0; inst <= MAX_ENGINE_INSTANCE; inst++) { - if (!gt->engine_class[class][inst]) - continue; - - siblings[nsibling++] = gt->engine_class[class][inst]; - } + nsibling = select_siblings(gt, class, siblings); if (nsibling < 2) continue;
Tvrtko spotted that some selftests were using 'break' not 'continue', which will fail for discontiguous engine layouts such as on Icelake (which may have vcs0 and vcs2). Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 68 ++++++++++---------------- 1 file changed, 27 insertions(+), 41 deletions(-)