Re: [PATCH v2 2/2] drm/xe: Remove H2G reads in CT send path in non-debug builds

[Francois Dugast <francois.dugast@intel.com>]

mOn Fri, Feb 13, 2026 at 01:16:25PM -0800, Matthew Brost wrote:
> A single VRAM read on BMG can take over 1µs. While small, this is a
> non-trivial amount of time in a hot path. Remove the descriptor H2G read
> (potentially a VRAM access) from non-debug builds, as this
> error-checking code is not needed outside of debug configurations.

About the change itself: I understand the performance benefit and this would
make the code consistent with the 2 other reads.

But the existing block under "if (IS_ENABLED(CONFIG_DRM_XE_DEBUG))" seems wrong
because it is not just about optionally printing some debug statements, in case
of error the function actually returns a different value if CONFIG_DRM_XE_DEBUG
is enabled or not (goto), and this return value is used in __guc_ct_send_locked.

Francois

> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_ct.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 6a96bea40720..f200d3ee9d22 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -939,22 +939,22 @@ static int h2g_write(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  	u32 full_len;
>  	struct iosys_map map = IOSYS_MAP_INIT_OFFSET(&h2g->cmds,
>  							 tail * sizeof(u32));
> -	u32 desc_status;
>  
>  	full_len = len + GUC_CTB_HDR_LEN;
>  
>  	lockdep_assert_held(&ct->lock);
>  	xe_gt_assert(gt, full_len <= GUC_CTB_MSG_MAX_LEN);
>  
> -	desc_status = desc_read(xe, h2g, status);
> -	if (desc_status) {
> -		xe_gt_err(gt, "CT write: non-zero status: %u\n", desc_status);
> -		goto corrupted;
> -	}
> -
>  	if (IS_ENABLED(CONFIG_DRM_XE_DEBUG)) {
>  		u32 desc_tail = desc_read(xe, h2g, tail);
>  		u32 desc_head = desc_read(xe, h2g, head);
> +		u32 desc_status;
> +
> +		desc_status = desc_read(xe, h2g, status);
> +		if (desc_status) {
> +			xe_gt_err(gt, "CT write: non-zero status: %u\n", desc_status);
> +			goto corrupted;
> +		}
>  
>  		if (tail != desc_tail) {
>  			desc_write(xe, h2g, status, desc_status | GUC_CTB_STATUS_MISMATCH);
> -- 
> 2.34.1
>