[Intel-xe] [PATCH 2/4] drm/xe: Add CONFIG_DRM_XE_PREEMPT_TIMEOUT

[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 2/4] drm/xe: Add CONFIG_DRM_XE_PREEMPT_TIMEOUT
> 
> Allow preemption timeout to be specified as a config option.
> 
> Signed-off-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura at intel.com>
> ---
>  drivers/gpu/drm/xe/Kconfig.profile | 11 +++++++++++
> drivers/gpu/drm/xe/xe_hw_engine.c  | 10 +++++++++-
> drivers/gpu/drm/xe/xe_hw_engine.h  |  5 +++++
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/Kconfig.profile
> b/drivers/gpu/drm/xe/Kconfig.profile
> index 9ad487647ca1..06742e192206 100644
> --- a/drivers/gpu/drm/xe/Kconfig.profile
> +++ b/drivers/gpu/drm/xe/Kconfig.profile
> @@ -22,6 +22,17 @@ config DRM_XE_TIMESLICE_MIN
>  	help
>  	  Configures the default min timeslice duration between multiple
>  	  contexts by guc scheduling.
> +config DRM_XE_PREEMPT_TIMEOUT
> +	int "Preempt timeout (ms, jiffy granularity)"
> +	default 640 # milliseconds

Why cant we represent this in us unit? At least it keeps uniformity with other code.

> +	help
> +	  How long to wait (in milliseconds) for a preemption event to occur
> +	  when submitting a new context. 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
>  	int "Default max  preempt timeout (us)"
>  	default 10000000 # microseconds
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index c44540684462..25bad007a85b 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -338,6 +338,11 @@ hw_engine_setup_default_state(struct
> xe_hw_engine *hwe)
>  	xe_rtp_process_to_sr(&ctx, engine_entries, &hwe->reg_sr);  }
> 
> +static u32 get_default_preempt_timeout(struct xe_hw_engine *hwe) {
> +	return XE_HW_ENGINE_PREEMPT_TIMEOUT;
> +}
> +
>  static void hw_engine_init_early(struct xe_gt *gt, struct xe_hw_engine *hwe,
>  				 enum xe_hw_engine_id id)
>  {
> @@ -370,7 +375,7 @@ static void hw_engine_init_early(struct xe_gt *gt,
> struct xe_hw_engine *hwe,
>  		hwe->eclass->sched_props.timeslice_us = 1 * 1000;
>  		hwe->eclass->sched_props.timeslice_min =
> XE_HW_ENGINE_TIMESLICE_MIN;
>  		hwe->eclass->sched_props.timeslice_max =
> XE_HW_ENGINE_TIMESLICE_MAX;
> -		hwe->eclass->sched_props.preempt_timeout_us = 640 *
> 1000;
> +		hwe->eclass->sched_props.preempt_timeout_us =
> +get_default_preempt_timeout(hwe);

I don't see purpose of using this API, also adding hwe argument does not seem to be any useful.

Thanks,
Tejas
>  		hwe->eclass->sched_props.preempt_timeout_min =
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN;
>  		hwe->eclass->sched_props.preempt_timeout_max =
> XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX;
>  		/* Record default props */
> @@ -562,6 +567,9 @@ int xe_hw_engines_init_early(struct xe_gt *gt)
>  	read_copy_fuses(gt);
>  	read_compute_fuses(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);
> +
>  	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 3d37d6d44261..582be209355b 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -30,6 +30,11 @@ struct drm_printer;
>  #else
>  #define XE_HW_ENGINE_TIMESLICE_MAX (10 * 1000 * 1000)  #endif
> +#ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT
> +#define XE_HW_ENGINE_PREEMPT_TIMEOUT
> (CONFIG_DRM_XE_PREEMPT_TIMEOUT *
> +1000) #else #define XE_HW_ENGINE_PREEMPT_TIMEOUT (640 * 1000)
> #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