Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number

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

On 05/04/2017 10:30, Oscar Mateo wrote:
> Not really needed, but makes the next change a little bit more compact.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c       | 8 ++++++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h      | 4 +++-
>  drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +-
>  3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index abc0a9c..530f822 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -71,7 +71,7 @@
>  		.init_legacy = intel_init_bsd_ring_buffer,
>  	},
>  	[VCS2] = {
> -		.name = "vcs2",
> +		.name = "vcs",

Rename the field to class_name perhaps?

>  		.hw_id = VCS2_HW,
>  		.exec_id = I915_EXEC_BSD,
>  		.class = VIDEO_DECODE_CLASS,
> @@ -100,6 +100,7 @@
>  {
>  	const struct engine_info *info = &intel_engines[id];
>  	struct intel_engine_cs *engine;
> +	char instance[3] = "";
>
>  	GEM_BUG_ON(dev_priv->engine[id]);
>  	engine = kzalloc(sizeof(*engine), GFP_KERNEL);
> @@ -108,7 +109,10 @@
>
>  	engine->id = id;
>  	engine->i915 = dev_priv;
> -	engine->name = info->name;
> +	/* For historical reasons the engines are called: name, name2... */
> +	if (info->instance)
> +		snprintf(instance, sizeof(instance), "%u", info->instance + 1);
> +	snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance);

Since Chris has recently renamed all the engines, I'd say who cares 
about the numbering scheme. Just drop it for simplicity.

>  	engine->exec_id = info->exec_id;
>  	engine->hw_id = engine->guc_id = info->hw_id;
>  	engine->mmio_base = info->mmio_base;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 5c1a27f..d617049 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -187,9 +187,11 @@ enum intel_engine_id {
>  	VECS
>  };
>
> +#define INTEL_ENGINE_CS_MAX_NAME 8
> +
>  struct intel_engine_cs {
>  	struct drm_i915_private *i915;
> -	const char	*name;
> +	char name[INTEL_ENGINE_CS_MAX_NAME];
>  	enum intel_engine_id id;
>  	unsigned int exec_id;
>  	unsigned int hw_id;
> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
> index b89050e..4a1ffca 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
> @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>
>  	/* minimal engine setup for requests */
>  	engine->base.i915 = i915;
> -	engine->base.name = name;
> +	strncpy(engine->base.name, name, sizeof(engine->base.name) - 1);

Can this miss to null-terminate? Or it relies on the mock_engine being 
kzalloced?

>  	engine->base.id = id++;
>  	engine->base.status_page.page_addr = (void *)(engine + 1);
>
>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx