Re: [PATCH v4 1/8] drm/xe: Move user engine class mappings to functions

[Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7418 bytes --]


On 12/22/23 13:15, Riana Tauro wrote:
> Move user engine class <-> hw engine class to function
> calls so that it can be used in different files.
>
> No functional changes.
>
> v2: change array to function
> v3: rebase
>     add header xe_drm
>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c | 19 ++---------
>  drivers/gpu/drm/xe/xe_hw_engine.c  | 51 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_hw_engine.h  |  3 ++
>  drivers/gpu/drm/xe/xe_query.c      | 23 ++------------
>  4 files changed, 58 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 44fe8097b7cd..366f5714c6f7 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -482,31 +482,16 @@ static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue
>  	return 0;
>  }
>  
> -static const enum xe_engine_class user_to_xe_engine_class[] = {
> -	[DRM_XE_ENGINE_CLASS_RENDER] = XE_ENGINE_CLASS_RENDER,
> -	[DRM_XE_ENGINE_CLASS_COPY] = XE_ENGINE_CLASS_COPY,
> -	[DRM_XE_ENGINE_CLASS_VIDEO_DECODE] = XE_ENGINE_CLASS_VIDEO_DECODE,
> -	[DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE] = XE_ENGINE_CLASS_VIDEO_ENHANCE,
> -	[DRM_XE_ENGINE_CLASS_COMPUTE] = XE_ENGINE_CLASS_COMPUTE,
> -};
> -
>  static struct xe_hw_engine *
>  find_hw_engine(struct xe_device *xe,
>  	       struct drm_xe_engine_class_instance eci)
>  {
> -	u32 idx;
> -
> -	if (eci.engine_class > ARRAY_SIZE(user_to_xe_engine_class))
> -		return NULL;
>  
>  	if (eci.gt_id >= xe->info.gt_count)
>  		return NULL;
>  
> -	idx = array_index_nospec(eci.engine_class,
> -				 ARRAY_SIZE(user_to_xe_engine_class));
> -
>  	return xe_gt_hw_engine(xe_device_get_gt(xe, eci.gt_id),
> -			       user_to_xe_engine_class[idx],
> +			       xe_hw_engine_from_user_class(eci.engine_class),
>  			       eci.engine_instance, true);

in xe_gt_hw_engine introduce a check at the beginning for hw_engine_class and return NULL if it
is XE_ENGINE_CLASS_MAX, so that we can skip for_each_hw_engine.
>  }
>  
> @@ -532,7 +517,7 @@ static u32 bind_exec_queue_logical_mask(struct xe_device *xe, struct xe_gt *gt,
>  			continue;
>  
>  		if (hwe->class ==
> -		    user_to_xe_engine_class[DRM_XE_ENGINE_CLASS_COPY])
> +		    xe_hw_engine_from_user_class(DRM_XE_ENGINE_CLASS_COPY))
>  			logical_mask |= BIT(hwe->logical_instance);
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 832989c83a25..ab78019b44ff 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -6,6 +6,7 @@
>  #include "xe_hw_engine.h"
>  
>  #include <drm/drm_managed.h>
> +#include <drm/xe_drm.h>
>  
>  #include "regs/xe_engine_regs.h"
>  #include "regs/xe_gt_regs.h"
> @@ -290,6 +291,56 @@ static u32 hw_engine_mmio_read32(struct xe_hw_engine *hwe, struct xe_reg reg)
>  	return xe_mmio_read32(hwe->gt, reg);
>  }
>  
> +/**
> + * xe_hw_engine_to_user_class - converts xe hw engine to user engine class
> + * @engine_class: hw engine class
> + *
> + * Returns: user engine class on success, -1 on error
> + */
> +u16 xe_hw_engine_to_user_class(enum xe_engine_class engine_class)
> +{
> +	switch (engine_class) {
> +	case XE_ENGINE_CLASS_RENDER:
> +		return DRM_XE_ENGINE_CLASS_RENDER;
> +	case XE_ENGINE_CLASS_COPY:
> +		return DRM_XE_ENGINE_CLASS_COPY;
> +	case XE_ENGINE_CLASS_VIDEO_DECODE:
> +		return DRM_XE_ENGINE_CLASS_VIDEO_DECODE;
> +	case XE_ENGINE_CLASS_VIDEO_ENHANCE:
> +		return DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE;
> +	case XE_ENGINE_CLASS_COMPUTE:
> +		return DRM_XE_ENGINE_CLASS_COMPUTE;
> +	default:
> +		XE_WARN_ON(engine_class);
> +		return -1;
> +	}
> +}
> +
> +/**
> + * xe_hw_engine_from_user_class - converts xe user engine class to hw engine class
> + * @engine_class: user engine class
> + *
> + * Returns: hw engine class on success
> + */
> +enum xe_engine_class xe_hw_engine_from_user_class(u16 engine_class)
> +{
> +	switch (engine_class) {
> +	case DRM_XE_ENGINE_CLASS_RENDER:
> +		return XE_ENGINE_CLASS_RENDER;
> +	case DRM_XE_ENGINE_CLASS_COPY:
> +		return XE_ENGINE_CLASS_COPY;
> +	case DRM_XE_ENGINE_CLASS_VIDEO_DECODE:
> +		return XE_ENGINE_CLASS_VIDEO_DECODE;
> +	case DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE:
> +		return XE_ENGINE_CLASS_VIDEO_ENHANCE;
> +	case DRM_XE_ENGINE_CLASS_COMPUTE:
> +		return XE_ENGINE_CLASS_COMPUTE;
> +	default:
> +		XE_WARN_ON(engine_class);
> +		return XE_ENGINE_CLASS_MAX;
> +	}
> +}
> +
>  void xe_hw_engine_enable_ring(struct xe_hw_engine *hwe)
>  {
>  	u32 ccs_mask =
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h b/drivers/gpu/drm/xe/xe_hw_engine.h
> index 71968ee2f600..89ca96063644 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -62,6 +62,9 @@ void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p);
>  void xe_hw_engine_setup_default_lrc_state(struct xe_hw_engine *hwe);
>  
>  bool xe_hw_engine_is_reserved(struct xe_hw_engine *hwe);
> +enum xe_engine_class xe_hw_engine_from_user_class(u16 engine_class);
> +u16 xe_hw_engine_to_user_class(enum xe_engine_class engine_class);
> +
>  static inline bool xe_hw_engine_is_valid(struct xe_hw_engine *hwe)
>  {
>  	return hwe->name;
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index 9b35673b286c..d4793e79e283 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -22,22 +22,6 @@
>  #include "xe_mmio.h"
>  #include "xe_ttm_vram_mgr.h"
>  
> -static const u16 xe_to_user_engine_class[] = {
> -	[XE_ENGINE_CLASS_RENDER] = DRM_XE_ENGINE_CLASS_RENDER,
> -	[XE_ENGINE_CLASS_COPY] = DRM_XE_ENGINE_CLASS_COPY,
> -	[XE_ENGINE_CLASS_VIDEO_DECODE] = DRM_XE_ENGINE_CLASS_VIDEO_DECODE,
> -	[XE_ENGINE_CLASS_VIDEO_ENHANCE] = DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE,
> -	[XE_ENGINE_CLASS_COMPUTE] = DRM_XE_ENGINE_CLASS_COMPUTE,
> -};
> -
> -static const enum xe_engine_class user_to_xe_engine_class[] = {
> -	[DRM_XE_ENGINE_CLASS_RENDER] = XE_ENGINE_CLASS_RENDER,
> -	[DRM_XE_ENGINE_CLASS_COPY] = XE_ENGINE_CLASS_COPY,
> -	[DRM_XE_ENGINE_CLASS_VIDEO_DECODE] = XE_ENGINE_CLASS_VIDEO_DECODE,
> -	[DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE] = XE_ENGINE_CLASS_VIDEO_ENHANCE,
> -	[DRM_XE_ENGINE_CLASS_COMPUTE] = XE_ENGINE_CLASS_COMPUTE,
> -};
> -
>  static size_t calc_hw_engine_info_size(struct xe_device *xe)
>  {
>  	struct xe_hw_engine *hwe;
> @@ -139,10 +123,7 @@ query_engine_cycles(struct xe_device *xe,
>  	if (!gt)
>  		return -EINVAL;
>  
> -	if (eci->engine_class >= ARRAY_SIZE(user_to_xe_engine_class))
> -		return -EINVAL;
> -
> -	hwe = xe_gt_hw_engine(gt, user_to_xe_engine_class[eci->engine_class],
> +	hwe = xe_gt_hw_engine(gt, xe_hw_engine_from_user_class(eci->engine_class),
>  			      eci->engine_instance, true);

looks like this changed, may be rebase on latest.
>  	if (!hwe)
>  		return -EINVAL;
> @@ -208,7 +189,7 @@ static int query_engines(struct xe_device *xe,
>  				continue;
>  
>  			engines->engines[i].instance.engine_class =
> -				xe_to_user_engine_class[hwe->class];
> +				xe_hw_engine_to_user_class(hwe->class);
>  			engines->engines[i].instance.engine_instance =
>  				hwe->logical_instance;
>  			engines->engines[i].instance.gt_id = gt->info.id;

with that addressed Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>

Regards,
Aravind.

[-- Attachment #2: Type: text/html, Size: 8500 bytes --]