Re: [Intel-xe] [PATCH] drm/xe/hw_engine: Document difference between instance and logical instance

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Oct 05, 2023 at 09:57:29AM +0000, Francois Dugast wrote:
> Add explanation provided by Matt next to the definition, so that
> it can easily found later.

First of all, thank you so much for adding this here. Just for the record
yesterday I was confused when I noticed that the logical_engines are actually
per gt and not a global number, and though that it was identical to the
@instance itself. My bad, but then Matt reminded us about the gaps in TGL.

> 
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hw_engine_types.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_types.h b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> index 5d4ee2904240..aa8b5a2165c5 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_types.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> @@ -114,7 +114,13 @@ struct xe_hw_engine {
>  	enum xe_engine_class class;
>  	/** @instance: physical instance of this hw engine */
>  	u16 instance;
> -	/** @logical_instance: logical instance of this hw engine */
> +	/**
> +	 * @logical_instance: logical instance of this hw engine. This is
> +	 * different from @instance because there can be gaps in physical
> +	 * engines. For example on TGL VCS0 and CVS2 map to logical

here I believe we could avoid the specific platform codename and have
something more generic. but if we need the platform we should to with
the full platform name.

Then there's the typo in VCS2.

> +	 * instances 0 and 1. Only logical instances should be exposed to
> +	 * user space.

Maybe also worth mentioning that the construct is a sequence from 0..n
per engine-class, per-gt. And then remove that FIXME comment around the
assignment?

maybe something like:

"Logical identification of this hardware engine. A 0..n sequencing number
assigned at driver initialization for each engine of a giving class of
a specific GT. Although it is per-class and per-GT, it might be different
from the @engine on platforms where we can have gaps on the hardware
assigned number such as platforms where we have VCS0 and VCS2, but no
VCS1. Only logical instance numbers should be exposed to user space."


(Although I confess I keep asking myself why don't we align with the
hardware assigned number. User space drivers also use the same spec
as kernel)

> +	 */
>  	u16 logical_instance;
>  	/** @mmio_base: MMIO base address of this hw engine*/
>  	u32 mmio_base;
> -- 
> 2.34.1
>