Re: [PATCH v2 3/3] drm/i915: Add engine name to workaround debug print

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 12/07/2019 08:07, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> There is a debug message in the workaround initialisation path that
> reports how many entries were added of each type. However, whitelist
> workarounds exist for multiple engines but the type name is just
> 'whitelist'. Tvrtko suggested adding the engine name to make the
> message more useful.
>
> v2: Updated the similar message in the workaround reset selftest.
>
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> CC: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko

> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c     | 15 ++++++++-------
>   .../gpu/drm/i915/gt/intel_workarounds_types.h   |  1 +
>   drivers/gpu/drm/i915/gt/selftest_workarounds.c  | 17 +++++------------
>   3 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 95be0f108f26..3b1fc7c8faa8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -50,9 +50,10 @@
>    * - Public functions to init or apply the given workaround type.
>    */
>   
> -static void wa_init_start(struct i915_wa_list *wal, const char *name)
> +static void wa_init_start(struct i915_wa_list *wal, const char *name, const char *engine_name)
>   {
>   	wal->name = name;
> +	wal->engine_name = engine_name;
>   }
>   
>   #define WA_LIST_CHUNK (1 << 4)
> @@ -74,8 +75,8 @@ static void wa_init_finish(struct i915_wa_list *wal)
>   	if (!wal->count)
>   		return;
>   
> -	DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
> -			 wal->wa_count, wal->name);
> +	DRM_DEBUG_DRIVER("Initialized %u %s workarounds on %s\n",
> +			 wal->wa_count, wal->name, wal->engine_name);
>   }
>   
>   static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
> @@ -591,7 +592,7 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
>   	if (engine->class != RENDER_CLASS)
>   		return;
>   
> -	wa_init_start(wal, name);
> +	wa_init_start(wal, name, engine->name);
>   
>   	if (IS_GEN(i915, 11))
>   		icl_ctx_workarounds_init(engine, wal);
> @@ -921,7 +922,7 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915)
>   {
>   	struct i915_wa_list *wal = &i915->gt_wa_list;
>   
> -	wa_init_start(wal, "GT");
> +	wa_init_start(wal, "GT", "global");
>   	gt_init_workarounds(i915, wal);
>   	wa_init_finish(wal);
>   }
> @@ -1192,7 +1193,7 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
>   	struct drm_i915_private *i915 = engine->i915;
>   	struct i915_wa_list *w = &engine->whitelist;
>   
> -	wa_init_start(w, "whitelist");
> +	wa_init_start(w, "whitelist", engine->name);
>   
>   	if (IS_GEN(i915, 11))
>   		icl_whitelist_build(engine);
> @@ -1384,7 +1385,7 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine)
>   	if (INTEL_GEN(engine->i915) < 8)
>   		return;
>   
> -	wa_init_start(wal, engine->name);
> +	wa_init_start(wal, "engine", engine->name);
>   	engine_init_workarounds(engine, wal);
>   	wa_init_finish(wal);
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> index 42ac1fb99572..e27ab1b710b3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> @@ -20,6 +20,7 @@ struct i915_wa {
>   
>   struct i915_wa_list {
>   	const char	*name;
> +	const char	*engine_name;
>   	struct i915_wa	*list;
>   	unsigned int	count;
>   	unsigned int	wa_count;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index fd1d47ba4b10..a0d8f1bfe0ad 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -25,11 +25,9 @@ static const struct wo_register {
>   	{ INTEL_GEMINILAKE, 0x731c }
>   };
>   
> -#define REF_NAME_MAX (INTEL_ENGINE_CS_MAX_NAME + 8)
>   struct wa_lists {
>   	struct i915_wa_list gt_wa_list;
>   	struct {
> -		char name[REF_NAME_MAX];
>   		struct i915_wa_list wa_list;
>   		struct i915_wa_list ctx_wa_list;
>   	} engine[I915_NUM_ENGINES];
> @@ -43,25 +41,20 @@ reference_lists_init(struct drm_i915_private *i915, struct wa_lists *lists)
>   
>   	memset(lists, 0, sizeof(*lists));
>   
> -	wa_init_start(&lists->gt_wa_list, "GT_REF");
> +	wa_init_start(&lists->gt_wa_list, "GT_REF", "global");
>   	gt_init_workarounds(i915, &lists->gt_wa_list);
>   	wa_init_finish(&lists->gt_wa_list);
>   
>   	for_each_engine(engine, i915, id) {
>   		struct i915_wa_list *wal = &lists->engine[id].wa_list;
> -		char *name = lists->engine[id].name;
>   
> -		snprintf(name, REF_NAME_MAX, "%s_REF", engine->name);
> -
> -		wa_init_start(wal, name);
> +		wa_init_start(wal, "REF", engine->name);
>   		engine_init_workarounds(engine, wal);
>   		wa_init_finish(wal);
>   
> -		snprintf(name, REF_NAME_MAX, "%s_CTX_REF", engine->name);
> -
>   		__intel_engine_init_ctx_wa(engine,
>   					   &lists->engine[id].ctx_wa_list,
> -					   name);
> +					   "CTX_REF");
>   	}
>   }
>   
> @@ -292,8 +285,8 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
>   	intel_wakeref_t wakeref;
>   	int err;
>   
> -	pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n",
> -		engine->whitelist.count, name);
> +	pr_info("Checking %d whitelisted registers on %s (RING_NONPRIV) [%s]\n",
> +		engine->whitelist.count, engine->name, name);
>   
>   	ctx = kernel_context(i915);
>   	if (IS_ERR(ctx))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx