| Message ID | 20210106123939.18435-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/selftests: Break out of the lrc layout test after register mismatch | expand |
On 06/01/2021 12:39, Chris Wilson wrote: > AFter detecting a register mismatch between the protocontext and the > image generated by HW, immediately break out of the double loop. > (Otherwise we end up a second configuing error message.) s/configuing/confusing/? No use of dumping all differences? Why it is confusing? Regards, Tvrtko > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/selftest_lrc.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 3485cb7c431d..920979a89413 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -164,7 +164,7 @@ static int live_lrc_layout(void *arg) > > dw = 0; > do { > - u32 lri = hw[dw]; > + u32 lri = READ_ONCE(hw[dw]); > > if (lri == 0) { > dw++; > @@ -197,9 +197,11 @@ static int live_lrc_layout(void *arg) > dw++; > > while (lri) { > - if (hw[dw] != lrc[dw]) { > + u32 offset = READ_ONCE(hw[dw]); > + > + if (offset != lrc[dw]) { > pr_err("%s: Different registers found at dword %d, expected %x, found %x\n", > - engine->name, dw, hw[dw], lrc[dw]); > + engine->name, dw, offset, lrc[dw]); > err = -EINVAL; > break; > } > @@ -211,7 +213,7 @@ static int live_lrc_layout(void *arg) > dw += 2; > lri -= 2; > } > - } while ((lrc[dw] & ~BIT(0)) != MI_BATCH_BUFFER_END); > + } while (!err && (lrc[dw] & ~BIT(0)) != MI_BATCH_BUFFER_END); > > if (err) { > pr_info("%s: HW register image:\n", engine->name); >
Quoting Tvrtko Ursulin (2021-01-06 15:10:02) > > On 06/01/2021 12:39, Chris Wilson wrote: > > AFter detecting a register mismatch between the protocontext and the > > image generated by HW, immediately break out of the double loop. > > (Otherwise we end up a second configuing error message.) > > s/configuing/confusing/? > > No use of dumping all differences? Why it is confusing? rcs0: Different registers found at dword 2, expected 2244, found 2244 rcs0: Expected LRI command at dword 2, found 00002244 We then hexdump the entire page, both HW/SW, so we can try and figure out what went wrong. -Chris
On 06/01/2021 15:17, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2021-01-06 15:10:02) >> >> On 06/01/2021 12:39, Chris Wilson wrote: >>> AFter detecting a register mismatch between the protocontext and the >>> image generated by HW, immediately break out of the double loop. >>> (Otherwise we end up a second configuing error message.) >> >> s/configuing/confusing/? >> >> No use of dumping all differences? Why it is confusing? > > rcs0: Different registers found at dword 2, expected 2244, found 2244 > rcs0: Expected LRI command at dword 2, found 00002244 > > We then hexdump the entire page, both HW/SW, so we can try and figure out > what went wrong. Right, I see it now. 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 3485cb7c431d..920979a89413 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -164,7 +164,7 @@ static int live_lrc_layout(void *arg) dw = 0; do { - u32 lri = hw[dw]; + u32 lri = READ_ONCE(hw[dw]); if (lri == 0) { dw++; @@ -197,9 +197,11 @@ static int live_lrc_layout(void *arg) dw++; while (lri) { - if (hw[dw] != lrc[dw]) { + u32 offset = READ_ONCE(hw[dw]); + + if (offset != lrc[dw]) { pr_err("%s: Different registers found at dword %d, expected %x, found %x\n", - engine->name, dw, hw[dw], lrc[dw]); + engine->name, dw, offset, lrc[dw]); err = -EINVAL; break; } @@ -211,7 +213,7 @@ static int live_lrc_layout(void *arg) dw += 2; lri -= 2; } - } while ((lrc[dw] & ~BIT(0)) != MI_BATCH_BUFFER_END); + } while (!err && (lrc[dw] & ~BIT(0)) != MI_BATCH_BUFFER_END); if (err) { pr_info("%s: HW register image:\n", engine->name);
AFter detecting a register mismatch between the protocontext and the image generated by HW, immediately break out of the double loop. (Otherwise we end up a second configuing error message.) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)