[Intel-xe] [PATCH 3/4] drm/xe: Add CONFIG_DRM_XE_PREEMPT_TIMEOUT_COMPUTE

[Upadhyay, Tejas]

> -----Original Message-----
> From: Vishwanathapura, Niranjana <niranjana.vishwanathapura at intel.com>
> Sent: Tuesday, August 8, 2023 11:54 PM
> To: intel-xe at lists.freedesktop.org
> Cc: Harrison, John C <john.c.harrison at intel.com>; Upadhyay, Tejas
> <tejas.upadhyay at intel.com>
> Subject: [PATCH 3/4] drm/xe: Add
> CONFIG_DRM_XE_PREEMPT_TIMEOUT_COMPUTE
> 
> Allow preemption timeout for compute engines to be specified as a config
> option. The purpose is actually to give compute tasks longer time to reach a
> pre-emption point after a pre-emption request has been issued. This is
> necessary because Gen12 does not support mid-thread pre-emption and
> compute tasks can have long running threads.
> 
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> Signed-off-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura at intel.com>
> ---
>  drivers/gpu/drm/xe/Kconfig.profile | 12 ++++++++++++
> drivers/gpu/drm/xe/xe_hw_engine.c  |  8 +++++++-
> drivers/gpu/drm/xe/xe_hw_engine.h  |  6 ++++++
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/Kconfig.profile
> b/drivers/gpu/drm/xe/Kconfig.profile
> index 06742e192206..5ca3305a22b6 100644
> --- a/drivers/gpu/drm/xe/Kconfig.profile
> +++ b/drivers/gpu/drm/xe/Kconfig.profile
> @@ -31,6 +31,18 @@ config DRM_XE_PREEMPT_TIMEOUT
>  	  an arbitration point and yield to HW before the timer expires, the
>  	  HW will be reset to allow the more important context to execute.
> 
> +	  Value should be within the
> DRM_XE_PREEMPT_TIMEOUT_MIN/MAX limits.
> +	  This is adjustable via
> XE_ENGINE_SET_PROPERTY_PREEMPTION_TIMEOUT.
> +config DRM_XE_PREEMPT_TIMEOUT_COMPUTE
> +	int "Preempt timeout for compute engines (ms, jiffy granularity)"
> +	default 7500 # milliseconds

Same comment as previous patch about us unit.

> +	help
> +	  How long to wait (in milliseconds) for a preemption event to occur
> +	  when submitting a new context to a compute capable engine. If the
> +	  current context does not hit an arbitration point and yield to HW
> +	  before the timer expires, the HW will be reset to allow the more
> +	  important context to execute.
> +
>  	  Value should be within the
> DRM_XE_PREEMPT_TIMEOUT_MIN/MAX limits.
>  	  This is adjustable via
> XE_ENGINE_SET_PROPERTY_PREEMPTION_TIMEOUT.
>  config DRM_XE_PREEMPT_TIMEOUT_MAX
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 25bad007a85b..57a0fb50852c 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -340,7 +340,11 @@ hw_engine_setup_default_state(struct
> xe_hw_engine *hwe)
> 
>  static u32 get_default_preempt_timeout(struct xe_hw_engine *hwe)  {
> -	return XE_HW_ENGINE_PREEMPT_TIMEOUT;
> +	if (hwe->class == XE_ENGINE_CLASS_RENDER ||
> +	    hwe->class == XE_ENGINE_CLASS_COMPUTE)
> +		return XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE;
> +	else
> +		return XE_HW_ENGINE_PREEMPT_TIMEOUT;
>  }

Probably API in previous patch continuation of this change, but it does not make sense to introduce change in previous patch at least.

> 
>  static void hw_engine_init_early(struct xe_gt *gt, struct xe_hw_engine *hwe,
> @@ -569,6 +573,8 @@ int xe_hw_engines_init_early(struct xe_gt *gt)
> 
>  	BUILD_BUG_ON(XE_HW_ENGINE_PREEMPT_TIMEOUT <
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN);
>  	BUILD_BUG_ON(XE_HW_ENGINE_PREEMPT_TIMEOUT >
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX);
> +	BUILD_BUG_ON(XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE <
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN);
> +	BUILD_BUG_ON(XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE >
> +XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX);

Can this be done shorter with,
BUILD_BUG_ON(get_default_preempt_timeout(hwe) <
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN); 
BUILD_BUG_ON(get_default_preempt_timeout(hwe) > XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX);

Thanks,
Tejas
> 
>  	for (i = 0; i < ARRAY_SIZE(gt->hw_engines); i++)
>  		hw_engine_init_early(gt, &gt->hw_engines[i], i); diff --git
> a/drivers/gpu/drm/xe/xe_hw_engine.h
> b/drivers/gpu/drm/xe/xe_hw_engine.h
> index 582be209355b..c4f0969efc06 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -35,6 +35,12 @@ struct drm_printer;
>  #else
>  #define XE_HW_ENGINE_PREEMPT_TIMEOUT (640 * 1000)  #endif
> +#ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT_COMPUTE
> +#define XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE \
> +	(CONFIG_DRM_XE_PREEMPT_TIMEOUT_COMPUTE * 1000) #else
> #define
> +XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE
> XE_HW_ENGINE_PREEMPT_TIMEOUT
> +#endif
>  #ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN  #define
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN
> CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN  #else
> --
> 2.21.0.rc0.32.g243a4c7e27