Re: [PATCH 2/5] drm/xe/heci: Use xe print functions in xe_heci_gsc.c

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

On 6/5/2026 6:04 PM, Jonathan Cavitt wrote:
> Update xe_heci_gsc.c to use the xe error reporting helper functions in
> xe_printk.h instead of directly calling the associated drm print
> functions from drm_print.h
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_heci_gsc.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
> index 5af8903e10af..d716371fbbe9 100644
> --- a/drivers/gpu/drm/xe/xe_heci_gsc.c
> +++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
> @@ -8,10 +8,9 @@
>  #include <linux/pci.h>
>  #include <linux/sizes.h>
>  
> -#include <drm/drm_print.h>
> -
>  #include "xe_device_types.h"
>  #include "xe_heci_gsc.h"
> +#include "xe_printk.h"
>  #include "regs/xe_gsc_regs.h"

nit: this one is at wrong place
>  #include "xe_platform_types.h"

and xe_printk.h should be here

>  #include "xe_survivability_mode.h"
> @@ -112,13 +111,13 @@ static int heci_gsc_irq_setup(struct xe_device *xe)
>  
>  	heci_gsc->irq = irq_alloc_desc(0);

nit: what about moving to devm_irq_alloc_desc() first?

>  	if (heci_gsc->irq < 0) {
> -		drm_err(&xe->drm, "gsc irq error %d\n", heci_gsc->irq);
> +		xe_err(xe, "gsc irq error %d\n", heci_gsc->irq);

can we print error code in more friendly way using %pe

and I guess we should use "GSC" name, not "gsc", so maybe:

	xe_err(xe, "GSC: irq allocation failed (%pe)\n", ERR_PTR(..

>  		return heci_gsc->irq;
>  	}
>  
>  	ret = heci_gsc_irq_init(heci_gsc->irq);
>  	if (ret < 0)
> -		drm_err(&xe->drm, "gsc irq init failed %d\n", ret);
> +		xe_err(xe, "gsc irq init failed %d\n", ret);

	xe_err(xe, "GSC: irq initialization failed (%pe)\n", ERR_PTR(..

>  
>  	return ret;
>  }
> @@ -151,7 +150,7 @@ static int heci_gsc_add_device(struct xe_device *xe, const struct heci_gsc_def *
>  
>  	ret = auxiliary_device_init(aux_dev);
>  	if (ret < 0) {
> -		drm_err(&xe->drm, "gsc aux init failed %d\n", ret);
> +		xe_err(xe, "gsc aux init failed %d\n", ret);
>  		kfree(adev);
>  		return ret;
>  	}
> @@ -159,7 +158,7 @@ static int heci_gsc_add_device(struct xe_device *xe, const struct heci_gsc_def *
>  	heci_gsc->adev = adev; /* needed by the notifier */
>  	ret = auxiliary_device_add(aux_dev);
>  	if (ret < 0) {
> -		drm_err(&xe->drm, "gsc aux add failed %d\n", ret);
> +		xe_err(xe, "gsc aux add failed %d\n", ret);
>  		heci_gsc->adev = NULL;
>  
>  		/* adev will be freed with the put_device() and .release sequence */
> @@ -190,7 +189,7 @@ int xe_heci_gsc_init(struct xe_device *xe)
>  	}
>  
>  	if (!def || !def->name) {

missing def looks like our coding error, shouldn't we just use xe_assert()?

missing def->name is also our coding error,
we should have no runtime checks for it (except xe_assert)

> -		drm_warn(&xe->drm, "HECI is not implemented!\n");
> +		xe_warn(xe, "HECI is not implemented!\n");
>  		return 0;
>  	}
>  
> @@ -215,7 +214,7 @@ void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir)
>  		return;
>  
>  	if (!xe->info.has_heci_gscfi) {
> -		drm_warn_once(&xe->drm, "GSC irq: not supported");
> +		xe_warn_once(xe, "GSC irq: not supported");

		xe_warn_once(xe, "GSC: unexpected irq %#x\n", iir);

>  		return;
>  	}
>  
> @@ -224,7 +223,7 @@ void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir)
>  
>  	ret = generic_handle_irq_safe(xe->heci_gsc.irq);
>  	if (ret)
> -		drm_err_ratelimited(&xe->drm, "error handling GSC irq: %d\n", ret);
> +		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);

		xe_err_ratelimited(xe, "GSC: irq handling failed (%pe)\n", 

>  }
>  
>  void xe_heci_csc_irq_handler(struct xe_device *xe, u32 iir)
> @@ -235,7 +234,7 @@ void xe_heci_csc_irq_handler(struct xe_device *xe, u32 iir)
>  		return;
>  
>  	if (!xe->info.has_heci_cscfi) {
> -		drm_warn_once(&xe->drm, "CSC irq: not supported");
> +		xe_warn_once(xe, "CSC irq: not supported");

		xe_warn_once(xe, "CSC: unexpected irq %#x\n", iir);

>  		return;
>  	}
>  
> @@ -244,5 +243,5 @@ void xe_heci_csc_irq_handler(struct xe_device *xe, u32 iir)
>  
>  	ret = generic_handle_irq_safe(xe->heci_gsc.irq);
>  	if (ret)
> -		drm_err_ratelimited(&xe->drm, "error handling GSC irq: %d\n", ret);
> +		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);

GSC or CSC ? function is 'heci_csc'

>  }