Re: [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine

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

On 03/07/2019 10:17, Chris Wilson wrote:
> During post-reset resume, we call intel_mocs_init_engine to reinitialise
> the MOCS registers. Suprisingly, especially when enhanced by lockdep,
> the acquisition of the forcewake lock around each register write takes a
> substantial portion of the reset time. We don't need to use the
> individual forcewake here as we can assume that the caller is holding a
> blanket forcewake for the reset&resume and the resume is serialised.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index ae6cbf0d517c..290a5e9b90b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   	unsigned int index;
>   	u32 unused_value;
>   
> +	/* Called under a blanket forcewake */
> +	assert_forcewakes_active(uncore, FORCEWAKE_ALL);
> +

Assert is strictly speaking a bit weak since forcewake status can 
theoretically change until the actual access below. But in our current 
code we indeed hold it for the whole reset.

I don't have any actual ideas on how to fundamentally improve the 
assert. Thought to have it after the writes is the only thing which came 
to mind. Thoughts?

Regards,

Tvrtko

>   	if (!get_mocs_settings(gt, &table))
>   		return;
>   
> @@ -355,16 +358,16 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   	for (index = 0; index < table.size; index++) {
>   		u32 value = get_entry_control(&table, index);
>   
> -		intel_uncore_write(uncore,
> -				   mocs_register(engine->id, index),
> -				   value);
> +		intel_uncore_write_fw(uncore,
> +				      mocs_register(engine->id, index),
> +				      value);
>   	}
>   
>   	/* All remaining entries are also unused */
>   	for (; index < table.n_entries; index++)
> -		intel_uncore_write(uncore,
> -				   mocs_register(engine->id, index),
> -				   unused_value);
> +		intel_uncore_write_fw(uncore,
> +				      mocs_register(engine->id, index),
> +				      unused_value);
>   }
>   
>   /**
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx