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

[Niranjana Vishwanathapura]

On Wed, Aug 16, 2023 at 04:05:39PM +0530, Ghimiray, Himal Prasad wrote:
>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.
>

Thanks Himal for the review.
The Kconfig description mentions it correctly as for compute capable
engines. Got missed in the commit description.

BTW, I noticed that we probably only need this for GEN12 platforms.
Hence, I will remove this patch from the patch series for now.

>>
>>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 ?
>

If this compute config is disabled, that means user wants to default config
to apply for these engines as well.

Niranjana

>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