Re: [PATCH 1/1] drm/xe/guc: Skip LRC indirect ring state size in engine state size

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 4/30/2026 12:26 AM, Satyanarayana K V P wrote:
> The engine state size reported to GuC via ADS should only include the
> engine state portion and should not include the indirect ring state page
> that comes after it in the context image. The GuC uses this size to restore
> only the engine state portion on watchdog resets.

The last sentence is slightly incorrect, the GuC restores what we tell 
it to restore, starting from the LRC engine state. Maybe change it to 
something like: "the GuC uses this size to overwrite the engine state in 
the LRC on watchdog resets and we don't want it to overwrite the 
indirect ring state as well"

>
> Fixes: d6219e1cd5e3 ("drm/xe: Add Indirect Ring State support")
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_guc_ads.c |  2 +-
>   drivers/gpu/drm/xe/xe_lrc.c     | 10 ++++++++--
>   drivers/gpu/drm/xe/xe_lrc.h     |  2 +-
>   3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index ce651da6f318..1edcc8875d35 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -520,7 +520,7 @@ static void guc_golden_lrc_init(struct xe_guc_ads *ads)
>   		 * on all engines).
>   		 */
>   		ads_blob_write(ads, ads.eng_state_size[guc_class],
> -			       real_size - xe_lrc_skip_size(xe));
> +			       real_size - xe_lrc_skip_size(gt));
>   		ads_blob_write(ads, ads.golden_context_lrca[guc_class],
>   			       addr_ggtt);
>   
> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> index 9db914584347..b83810ff4ff2 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.c
> +++ b/drivers/gpu/drm/xe/xe_lrc.c
> @@ -727,9 +727,15 @@ size_t xe_lrc_reg_size(struct xe_device *xe)
>   		return 80 * sizeof(u32);
>   }
>   
> -size_t xe_lrc_skip_size(struct xe_device *xe)
> +size_t xe_lrc_skip_size(struct xe_gt *gt)

This function needs documentation to clarify what the size it returns 
should be

>   {
> -	return LRC_PPHWSP_SIZE + xe_lrc_reg_size(xe);
> +	size_t size = LRC_PPHWSP_SIZE + xe_lrc_reg_size(gt_to_xe(gt));
> +
> +	/* Add indirect ring state page */

This comment might need to be updated (or removed) based on the description.

It might also be useful to add a comment to xe_gt_lrc_size() to mention 
that if we change the size there we might have to change it here as well.

Daniele

> +	if (xe_gt_has_indirect_ring_state(gt))
> +		size += LRC_INDIRECT_RING_STATE_SIZE;
> +
> +	return size;
>   }
>   
>   static inline u32 __xe_lrc_seqno_offset(struct xe_lrc *lrc)
> diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
> index e7c975f9e2d9..96d5243b30e2 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.h
> +++ b/drivers/gpu/drm/xe/xe_lrc.h
> @@ -130,7 +130,7 @@ u32 xe_lrc_parallel_ggtt_addr(struct xe_lrc *lrc);
>   struct iosys_map xe_lrc_parallel_map(struct xe_lrc *lrc);
>   
>   size_t xe_lrc_reg_size(struct xe_device *xe);
> -size_t xe_lrc_skip_size(struct xe_device *xe);
> +size_t xe_lrc_skip_size(struct xe_gt *gt);
>   
>   void xe_lrc_dump_default(struct drm_printer *p,
>   			 struct xe_gt *gt,