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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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 */
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx