Re: [PATCH 05/12] drm/xe/memirq: Report errors using structured xe_log API

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Loss of diagnostic precision by downgrading the logging context from tile-level to device-level.
- [Low] The `memirq_err()` macro was left defined in the file despite all its calls being replaced.
--

--- Patch [5]: [PATCH 05/12] drm/xe/memirq: Report errors using structured xe_log API ---
commit 081cec958cab790284a251bc663834e4569a80b2
Author: Dnyaneshwar Bhadane <[email protected]>

drm/xe/memirq: Report errors using structured xe_log API

This patch replaces memirq_err() calls with xe_log_err() using the
MEMIRQ component tag for memory-based IRQ failures.

> diff --git a/drivers/gpu/drm/xe/xe_memirq.c b/drivers/gpu/drm/xe/xe_memirq.c
> index 9dfe965cb46ec..45eae58d418f0 100644
> --- a/drivers/gpu/drm/xe/xe_memirq.c
> +++ b/drivers/gpu/drm/xe/xe_memirq.c

[Severity: Low]
This isn't a bug, but did you mean to also remove the definition of
the memirq_err() macro at the top of this file? It appears to be
dead code now that all invocations have been replaced.

[ ... ]

> @@ -232,7 +233,7 @@ static int memirq_alloc_pages(struct xe_memirq *memirq)
>  	return 0;
>  
>  out:
> -	memirq_err(memirq, "Failed to allocate memirq page (%pe)\n", ERR_PTR(err));
> +	xe_log_err(xe, MEMIRQ, err, "Failed to allocate memirq page (%pe)\n", ERR_PTR(err));

[Severity: Medium]
Does this change lose diagnostic precision by downgrading the logging
context from tile-level to device-level?

By explicitly passing the xe device instead of the available tile
variable, the location type resolves to the device, omitting the
specific failing tile ID from the structured log.

[ ... ]

> @@ -370,7 +371,7 @@ int xe_memirq_init_guc(struct xe_memirq *memirq, struct xe_guc *guc)
>  	return 0;
>  
>  failed:
> -	memirq_err(memirq, "Failed to setup report pages in %s (%pe)\n",
> +	xe_log_err(memirq_to_xe(memirq), MEMIRQ, err, "Failed to setup report pages in %s (%pe)\n",
>  		   guc_name(guc), ERR_PTR(err));
>  	return err;
>  }

[Severity: Medium]
Similarly, does passing memirq_to_xe(memirq) here instead of resolving
the tile or GT lose subsystem context for this log message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5