Re: [PATCH 4/4] drm/i915: Update workarounds selftest for read only regs

Tvrtko Ursulin
2019-06-18T06:42:31Z

On 18/06/2019 02:01, John.C.Harrison@Intel.com wrote:
> From: "Robert M. Fosha" <robert.m.fosha@intel.com>
>
> Updates the live_workarounds selftest to handle whitelisted
> registers that are flagged as read only.
>
> Signed-off-by: Robert M. Fosha <robert.m.fosha@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> .../gpu/drm/i915/gt/selftest_workarounds.c | 43 +++++++++++++++++--
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index c8d335d63f9c..eb6d3aa2c8cc 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -408,6 +408,29 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
> return false;
> }
>
> +static bool ro_register(u32 reg)
> +{
> + if (reg & RING_FORCE_TO_NONPRIV_RD)
> + return true;
> +
> + return false;
> +}
> +
> +static int whitelist_writable_count(struct intel_engine_cs *engine)
> +{
> + int count = engine->whitelist.count;
> + int i;
> +
> + for (i = 0; i < engine->whitelist.count; i++) {
> + u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> +
> + if (ro_register(reg))
> + count--;
> + }
> +
> + return count;
> +}
> +
> static int check_dirty_whitelist(struct i915_gem_context *ctx,
> struct intel_engine_cs *engine)
> {
> @@ -463,6 +486,9 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> if (wo_register(engine, reg))
> continue;
>
> + if (ro_register(reg))
> + continue;
> +
> srm = MI_STORE_REGISTER_MEM;
> lrm = MI_LOAD_REGISTER_MEM;
> if (INTEL_GEN(ctx->i915) >= 8)
> @@ -734,9 +760,13 @@ static int read_whitelisted_registers(struct i915_gem_context *ctx,
>
> for (i = 0; i < engine->whitelist.count; i++) {
> u64 offset = results->node.start + sizeof(u32) * i;
> + u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> +
> + /* Clear RD only and WR only flags */
> + reg &= ~(RING_FORCE_TO_NONPRIV_RD | RING_FORCE_TO_NONPRIV_WR);
>
> *cs++ = srm;
> - *cs++ = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> + *cs++ = reg;
> *cs++ = lower_32_bits(offset);
> *cs++ = upper_32_bits(offset);
> }
> @@ -769,9 +799,14 @@ static int scrub_whitelisted_registers(struct i915_gem_context *ctx,
> goto err_batch;
> }
>
> - *cs++ = MI_LOAD_REGISTER_IMM(engine->whitelist.count);
> + *cs++ = MI_LOAD_REGISTER_IMM(whitelist_writable_count(engine));
> for (i = 0; i < engine->whitelist.count; i++) {
> - *cs++ = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> + u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> +
> + if (ro_register(reg))
> + continue;
> +

Are we not able to test the read-only property at all?

Regards,

Tvrtko

> + *cs++ = reg;
> *cs++ = 0xffffffff;
> }
> *cs++ = MI_BATCH_BUFFER_END;
> @@ -956,7 +991,7 @@ static int live_isolated_whitelist(void *arg)
> }
>
> for_each_engine(engine, i915, id) {
> - if (!engine->whitelist.count)
> + if (!whitelist_writable_count(engine))
> continue;
>
> /* Read default values */