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

[Ghimiray, Himal Prasad]

Hi Niranjana,

On 14-08-2023 21:32, Niranjana Vishwanathapura wrote:
> 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.

Description mentions setting longer preemption time for compute engine. 
Whereas

code changes are doing it for RCS and CCS classes  both. Might be better 
to modify it.

>
> v2: Change unit to microseconds (Tejas)
>
> 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  |  5 +++++
>   3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/Kconfig.profile b/drivers/gpu/drm/xe/Kconfig.profile
> index a3655ee6978e..dd7f155371fa 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 (us, jiffy granularity)"
> +	default 7500000 # microseconds
> +	help
> +	  How long to wait (in microseconds) 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 8e5153cada31..e9ce34faa2da 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;
>   }
>   
>   static void hw_engine_init_early(struct xe_gt *gt, struct xe_hw_engine *hwe,
> @@ -570,6 +574,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);
>   
>   	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 71968ee2f600..cee0b40a62a2 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -35,6 +35,11 @@ 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
> +#else
> +#define XE_HW_ENGINE_PREEMPT_TIMEOUT_COMPUTE XE_HW_ENGINE_PREEMPT_TIMEOUT

In case of config being disabled, why not have longer preemption timeout 
for ccs and rcs engines ?

BR

Himal GHimiray

> +#endif
>   #ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN
>   #define XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN
>   #else