On 12/07/2019 08:07, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> Newer hardware supports extra feature in the whitelist registers. This
> patch updates the selftest to test that entries marked as read only
> are actually read only.
>
> v2: Removed all use of 'rsvd' for read-only registers to avoid
> ambiguous code or error messages.
Work's never done. :) You can follow up with a patch which adds engine looping to live_reset_whitelist.
I was looking at your test results and wondering why no new whitelists:
<6> [486.665700] i915: Running intel_workarounds_live_selftests/live_reset_whitelist
<6> [486.665706] Checking 4 whitelisted registers on rcs0 (RING_NONPRIV) [engine]
<7> [486.666281] [drm:intel_power_well_enable [i915]] enabling always-on
<5> [486.668777] i915 0000:00:02.0: Resetting rcs0 for live_workarounds
<6> [486.669900] Checking 4 whitelisted registers on rcs0 (RING_NONPRIV) [device]
<5> [486.671042] i915 0000:00:02.0: Resetting chip for live_workarounds
Regards,
Tvrtko
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> CC: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
> .../gpu/drm/i915/gt/selftest_workarounds.c | 49 +++++++++++++------
> 1 file changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index 466dcc8214c3..fd1d47ba4b10 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -485,12 +485,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> u32 srm, lrm, rsvd;
> u32 expect;
> int idx;
> + bool ro_reg;
>
> if (wo_register(engine, reg))
> continue;
>
> - if (ro_register(reg))
> - continue;
> + ro_reg = ro_register(reg);
>
> srm = MI_STORE_REGISTER_MEM;
> lrm = MI_LOAD_REGISTER_MEM;
> @@ -591,24 +591,35 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> }
>
> GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
> - rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
> - if (!rsvd) {
> - pr_err("%s: Unable to write to whitelisted register %x\n",
> - engine->name, reg);
> - err = -EINVAL;
> - goto out_unpin;
> + if (!ro_reg) {
> + /* detect write masking */
> + rsvd = results[ARRAY_SIZE(values)];
> + if (!rsvd) {
> + pr_err("%s: Unable to write to whitelisted register %x\n",
> + engine->name, reg);
> + err = -EINVAL;
> + goto out_unpin;
> + }
> }
>
> expect = results[0];
> idx = 1;
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> - expect = reg_write(expect, values[v], rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, values[v], rsvd);
> +
> if (results[idx] != expect)
> err++;
> idx++;
> }
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> - expect = reg_write(expect, ~values[v], rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, ~values[v], rsvd);
> +
> if (results[idx] != expect)
> err++;
> idx++;
> @@ -617,15 +628,22 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> pr_err("%s: %d mismatch between values written to whitelisted register [%x], and values read back!\n",
> engine->name, err, reg);
>
> - pr_info("%s: Whitelisted register: %x, original value %08x, rsvd %08x\n",
> - engine->name, reg, results[0], rsvd);
> + if (ro_reg)
> + pr_info("%s: Whitelisted read-only register: %x, original value %08x\n",
> + engine->name, reg, results[0]);
> + else
> + pr_info("%s: Whitelisted register: %x, original value %08x, rsvd %08x\n",
> + engine->name, reg, results[0], rsvd);
>
> expect = results[0];
> idx = 1;
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> u32 w = values[v];
>
> - expect = reg_write(expect, w, rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, w, rsvd);
> pr_info("Wrote %08x, read %08x, expect %08x\n",
> w, results[idx], expect);
> idx++;
> @@ -633,7 +651,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> u32 w = ~values[v];
>
> - expect = reg_write(expect, w, rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, w, rsvd);
> pr_info("Wrote %08x, read %08x, expect %08x\n",
> w, results[idx], expect);
> idx++;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
