Re: [PATCH v3 2/2] drm/xe/guc: Enable the Dynamic Inhibit Context Switch optimization

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 28.03.2025 18:28, Daniele Ceraolo Spurio wrote:
> The Dynamic Inhibit Context Switch is an optimization aimed at reducing
> the amount of time the HW is stuck waiting on an unsatisfied semaphore.
> When this optimization is enabled, the GuC will dynamically modify the
> CTX_CTRL_INHIBIT_SYN_CTX_SWITCH in the CTX_CONTEXT_CONTROL register of
> lrcs to enable immediate switching out on an usatisfied semaphore wait

s/lrcs/LRCs

and typo usatisfied

> when multiple contexts are competing for time on the same engine.
> 
> This feature is available on recent HW from GuC 70.40.1 onwards and it
> is enabled via a per-VF feature opt-in.
> 
> v2: rebase
> v3: switch to using guc_buf_cache instead of dedicated alloc
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Julia Filipchuk <julia.filipchuk@intel.com>
> ---
>  drivers/gpu/drm/xe/abi/guc_klvs_abi.h | 13 +++++++++++++
>  drivers/gpu/drm/xe/xe_guc.c           |  8 ++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> index 7d142c70b811..185d46f6c46a 100644
> --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> @@ -134,11 +134,24 @@ enum  {
>   *      Adds an extra dword to the XE_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR G2H
>   *      containing the type of the CAT error. On HW that does not support
>   *      reporting the CAT error type, the extra dword is set to 0xdeadbeef.
> + *
> + * _`GUC_KLV_OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH` : 0x4003
> + *      This KLV enables the Dynamic Inhibit Context Switch optimization, which
> + *      consists in the GuC setting the CTX_CTRL_INHIBIT_SYN_CTX_SWITCH bit to
> + *      zero in the CTX_CONTEXT_CONTROL register of LRCs that are submitted
> + *      to an oversubscribed engine. This will cause those contexts to be
> + *      switched out immediately if they hit an unsatisfied semaphore wait
> + *      (instead of waiting the full timeslice duration). The bit is instead set
> + *      to one if a single context is queued on the engine, to avoid it being
> + *      switched out if there isn't another context that can run in its place.
>   */
>  
>  #define GUC_KLV_OPT_IN_FEATURE_EXT_CAT_ERR_TYPE_KEY 0x4001
>  #define GUC_KLV_OPT_IN_FEATURE_EXT_CAT_ERR_TYPE_LEN 0u
>  
> +#define GUC_KLV_OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH_KEY 0x4003
> +#define GUC_KLV_OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH_LEN 0u
> +
>  /**
>   * DOC: GuC VGT Policy KLVs
>   *
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index efb1afa1d42d..c13ef2cb2d9a 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -606,6 +606,14 @@ int xe_guc_opt_in_features_enable(struct xe_guc *guc)
>  	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 7, 0))
>  		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);
>  
> +	/*
> +	 * Dynamic ICS is available for PVC and Xe2 and newer platforms. It
> +	 * requires GuC v70.40.1, which maps to compatibility version v1.18.4
> +	 */
> +	if ((xe->info.platform == XE_PVC || GRAPHICS_VER(xe) >= 20) &&
> +	    GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 18, 4))

maybe promote to xe_guc_supports_dynamic_inhibit_ctx_switch(guc) ?

> +		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH);
> +
>  	if (count) {
>  		xe_assert(xe, count <= OPT_IN_MAX_DWORDS);
>  

otherwise LGTM so,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>