Re: [PATCH v2 5/9] drm/xe/lrc: Refactor out engine id to hwe conversion

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Fri, May 01, 2026 at 05:53:38PM -0700, Umesh Nerlige Ramappa wrote:
>We need to define more helpers that read engine ID specific register, so
>move that logic outside of get_ctx_timestamp().
>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
> drivers/gpu/drm/xe/xe_lrc.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>index 2ee52efb9219..92419e5058fd 100644
>--- a/drivers/gpu/drm/xe/xe_lrc.c
>+++ b/drivers/gpu/drm/xe/xe_lrc.c
>@@ -2620,17 +2620,27 @@ void xe_lrc_snapshot_free(struct xe_lrc_snapshot *snapshot)
> 	kfree(snapshot);
> }
>
>-static int get_ctx_timestamp(struct xe_lrc *lrc, u32 engine_id, u64 *reg_ctx_ts)
>+static struct xe_hw_engine *engine_id_to_hwe(struct xe_gt *gt, u32 engine_id)
> {
> 	u16 class = REG_FIELD_GET(ENGINE_CLASS_ID, engine_id);
> 	u16 instance = REG_FIELD_GET(ENGINE_INSTANCE_ID, engine_id);
>+	struct xe_hw_engine *hwe = xe_gt_hw_engine(gt, class, instance, false);
>+
>+	if (xe_gt_WARN_ONCE(gt, !hwe || xe_hw_engine_is_reserved(hwe),
>+			    "Unexpected engine class:instance %d:%d for utilization\n",
>+			    class, instance))
>+		return NULL;
>+
>+	return hwe;
>+}

Looks good. But the error message doesn't say which function is causing this.
And also it will only print it once though there are multiple paths (scenarios)
to get to here. Probably that is fine as things will be broken anyway if this
happens.

Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

>+
>+static int get_ctx_timestamp(struct xe_lrc *lrc, u32 engine_id, u64 *reg_ctx_ts)
>+{
> 	struct xe_hw_engine *hwe;
> 	u64 val;
>
>-	hwe = xe_gt_hw_engine(lrc->gt, class, instance, false);
>-	if (xe_gt_WARN_ONCE(lrc->gt, !hwe || xe_hw_engine_is_reserved(hwe),
>-			    "Unexpected engine class:instance %d:%d for context utilization\n",
>-			    class, instance))
>+	hwe = engine_id_to_hwe(lrc->gt, engine_id);
>+	if (!hwe)
> 		return -1;
>
> 	if (lrc_to_xe(lrc)->info.has_64bit_timestamp)
>-- 
>2.43.0
>