[PATCH 0/5] drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages

[PATCH 0/5] drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages

[Jonathan Cavitt]

Currently, all but one uses of generic_handle_irq_safe() report error
messages using .*_err_ratelimited() error reporting helper functions.
These helper functions declare their error messages in the following
form:

"error handling [COMPONENT NAME] irq: %d\n"

.*_err_ratelimited() already logs "error" as a part of the error
message, so declaring the error is redundant.  Reword it.

The last usage of generic_handle_irq_safe() is in xe_i2c_irq_present(),
and it currently lacks the same error reporting as in the other cases.
This is not intentional, so put some logging there.

While we're here, we should also update the XE cases to use
xe_err_ratelimited() instead of directly calling drm_err_ratelimited().
However, mixing drm error reporting functions with xe error reporting
functions in the same file looks unprofessional, so update all drm error
reporting functions in xe_heci_gsc.c and xe_i2c.c to use their xe
counterparts.

This includes updating drm_warn_once() to use xe_warn_once() instead.
Unfortunately, xe_warn_once() does not exist, so create it.

Jonathan Cavitt (5):
  drm/xe/printk: Add xe_warn_once()
  drm/xe/heci: Use xe print functions in xe_heci_gsc.c
  drm/xe/i2c: Use xe print functions in xe_i2c.c
  drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages
  drm/xe/i2c: Report i2c irq handler issue

 .../gpu/drm/i915/display/intel_lpe_audio.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_gsc.c           |  2 +-
 drivers/gpu/drm/xe/xe_heci_gsc.c              | 21 +++++++++----------
 drivers/gpu/drm/xe/xe_i2c.c                   | 11 ++++++----
 drivers/gpu/drm/xe/xe_printk.h                |  3 +++
 5 files changed, 22 insertions(+), 17 deletions(-)

-- 
2.53.0

[PATCH 1/5] drm/xe/printk: Add xe_warn_once()

[Jonathan Cavitt]

Create a wrapper function xe_warn_once() that calls drm_warn_once().

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 drivers/gpu/drm/xe/xe_printk.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_printk.h b/drivers/gpu/drm/xe/xe_printk.h
index c5be2385aa95..afa787fcdeb6 100644
--- a/drivers/gpu/drm/xe/xe_printk.h
+++ b/drivers/gpu/drm/xe/xe_printk.h
@@ -27,6 +27,9 @@
 #define xe_warn(_xe, _fmt, ...) \
 	xe_printk((_xe), warn, _fmt, ##__VA_ARGS__)
 
+#define xe_warn_once(_xe, _fmt, ...) \
+	xe_printk((_xe), warn_once, _fmt, ##__VA_ARGS__)
+
 #define xe_notice(_xe, _fmt, ...) \
 	xe_printk((_xe), notice, _fmt, ##__VA_ARGS__)
 
-- 
2.53.0

Re: [PATCH 1/5] drm/xe/printk: Add xe_warn_once()

[Michal Wajdeczko]

On 6/5/2026 6:04 PM, Jonathan Cavitt wrote:
> Create a wrapper function xe_warn_once() that calls drm_warn_once().
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_printk.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_printk.h b/drivers/gpu/drm/xe/xe_printk.h
> index c5be2385aa95..afa787fcdeb6 100644
> --- a/drivers/gpu/drm/xe/xe_printk.h
> +++ b/drivers/gpu/drm/xe/xe_printk.h
> @@ -27,6 +27,9 @@
>  #define xe_warn(_xe, _fmt, ...) \
>  	xe_printk((_xe), warn, _fmt, ##__VA_ARGS__)
>  
> +#define xe_warn_once(_xe, _fmt, ...) \
> +	xe_printk((_xe), warn_once, _fmt, ##__VA_ARGS__)
> +
>  #define xe_notice(_xe, _fmt, ...) \
>  	xe_printk((_xe), notice, _fmt, ##__VA_ARGS__)
>  

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

[Jonathan Cavitt]

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"
 #include "xe_platform_types.h"
 #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);
 	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);
 		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);
 
 	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) {
-		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");
 		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);
 }
 
 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");
 		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);
 }
-- 
2.53.0

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

[Michal Wajdeczko]

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'

>  }

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

[Cavitt, Jonathan]

-----Original Message-----
From: Wajdeczko, Michal <michal.wajdeczko@intel.com> 
Sent: Friday, June 5, 2026 11:42 AM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
Cc: Gupta, Saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; Jadav, Raag <raag.jadav@intel.com>
Subject: Re: [PATCH 2/5] drm/xe/heci: Use xe print functions in xe_heci_gsc.c
> 
> 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?

I think fixing this should be addressed in a different patch series.

> 
> >  	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)

I think this also should be addressed in a different patch series.

I can apply the rest of the revision notes, though.
-Jonathan Cavitt

> 
> > -		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'
> 
> >  }
> 
> 

[PATCH 3/5] drm/xe/i2c: Use xe print functions in xe_i2c.c

[Jonathan Cavitt]

Update xe_i2c.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_i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 706783863d07..148e82e01ae8 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2025 Intel Corporation.
  */
 
-#include <drm/drm_print.h>
 #include <linux/array_size.h>
 #include <linux/container_of.h>
 #include <linux/device.h>
@@ -30,6 +29,7 @@
 #include "xe_device.h"
 #include "xe_i2c.h"
 #include "xe_mmio.h"
+#include "xe_printk.h"
 #include "xe_sriov.h"
 #include "xe_survivability_mode.h"
 
@@ -285,7 +285,7 @@ void xe_i2c_pm_suspend(struct xe_device *xe)
 		return;
 
 	xe_mmio_rmw32(mmio, I2C_CONFIG_PMCSR, PCI_PM_CTRL_STATE_MASK, (__force u32)PCI_D3hot);
-	drm_dbg(&xe->drm, "pmcsr: 0x%08x\n", xe_mmio_read32(mmio, I2C_CONFIG_PMCSR));
+	xe_dbg(xe, "pmcsr: 0x%08x\n", xe_mmio_read32(mmio, I2C_CONFIG_PMCSR));
 }
 
 void xe_i2c_pm_resume(struct xe_device *xe, bool d3cold)
@@ -299,7 +299,7 @@ void xe_i2c_pm_resume(struct xe_device *xe, bool d3cold)
 		xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
 
 	xe_mmio_rmw32(mmio, I2C_CONFIG_PMCSR, PCI_PM_CTRL_STATE_MASK, (__force u32)PCI_D0);
-	drm_dbg(&xe->drm, "pmcsr: 0x%08x\n", xe_mmio_read32(mmio, I2C_CONFIG_PMCSR));
+	xe_dbg(xe, "pmcsr: 0x%08x\n", xe_mmio_read32(mmio, I2C_CONFIG_PMCSR));
 }
 
 static void xe_i2c_remove(void *data)
-- 
2.53.0

[PATCH 4/5] drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages

[Jonathan Cavitt]

Currently, all but one uses of generic_handle_irq_safe() report error
messages using .*_err_ratelimited() error reporting helper functions.
These helper functions declare their error messages in the following
form:

"error handling [COMPONENT NAME] irq: %d\n"

.*_err_ratelimited() already logs "error" as a part of the error
message, so declaring the error is redundant.  Reword it.

v2:
- Use drm_err_ratelimited() correctly (jcavitt)

v3:
- Use xe_err_ratelimited() instead (Jadav)
- Split into patch series (jcavitt)

Suggested-by: Raag Jadav <raag.jadav@intel.com>
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 drivers/gpu/drm/i915/display/intel_lpe_audio.c | 2 +-
 drivers/gpu/drm/i915/gt/intel_gsc.c            | 2 +-
 drivers/gpu/drm/xe/xe_heci_gsc.c               | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
index 022ad18044bf..e756ed1d1132 100644
--- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
@@ -265,7 +265,7 @@ void intel_lpe_audio_irq_handler(struct intel_display *display)
 	ret = generic_handle_irq_safe(display->audio.lpe.irq);
 	if (ret)
 		drm_err_ratelimited(display->drm,
-				    "error handling LPE audio irq: %d\n", ret);
+				    "failed to handle LPE audio irq: %d\n", ret);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
index 050d909fb4f8..c2d76b0bc5fe 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
@@ -286,7 +286,7 @@ static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
 
 	ret = generic_handle_irq_safe(gt->gsc.intf[intf_id].irq);
 	if (ret)
-		gt_err_ratelimited(gt, "error handling GSC irq: %d\n", ret);
+		gt_err_ratelimited(gt, "failed to handle GSC irq: %d\n", ret);
 }
 
 void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
index d716371fbbe9..477004058834 100644
--- a/drivers/gpu/drm/xe/xe_heci_gsc.c
+++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
@@ -223,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)
-		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);
+		xe_err_ratelimited(xe, "failed to handle GSC irq: %d\n", ret);
 }
 
 void xe_heci_csc_irq_handler(struct xe_device *xe, u32 iir)
@@ -243,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)
-		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);
+		xe_err_ratelimited(xe, "failed to handle GSC irq: %d\n", ret);
 }
-- 
2.53.0

Re: [PATCH 4/5] drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages

[Michal Wajdeczko]

On 6/5/2026 6:04 PM, Jonathan Cavitt wrote:
> Currently, all but one uses of generic_handle_irq_safe() report error
> messages using .*_err_ratelimited() error reporting helper functions.
> These helper functions declare their error messages in the following
> form:
> 
> "error handling [COMPONENT NAME] irq: %d\n"

maybe better message would be:

	"[COMPONENT NAME]: irq handling failed (%pe)\n"> 
> .*_err_ratelimited() already logs "error" as a part of the error
> message, so declaring the error is redundant.  Reword it.
> 
> v2:
> - Use drm_err_ratelimited() correctly (jcavitt)
> 
> v3:
> - Use xe_err_ratelimited() instead (Jadav)
> - Split into patch series (jcavitt)
> 
> Suggested-by: Raag Jadav <raag.jadav@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_lpe_audio.c | 2 +-
>  drivers/gpu/drm/i915/gt/intel_gsc.c            | 2 +-
>  drivers/gpu/drm/xe/xe_heci_gsc.c               | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> index 022ad18044bf..e756ed1d1132 100644
> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> @@ -265,7 +265,7 @@ void intel_lpe_audio_irq_handler(struct intel_display *display)
>  	ret = generic_handle_irq_safe(display->audio.lpe.irq);
>  	if (ret)
>  		drm_err_ratelimited(display->drm,
> -				    "error handling LPE audio irq: %d\n", ret);
> +				    "failed to handle LPE audio irq: %d\n", ret);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
> index 050d909fb4f8..c2d76b0bc5fe 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
> @@ -286,7 +286,7 @@ static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
>  
>  	ret = generic_handle_irq_safe(gt->gsc.intf[intf_id].irq);
>  	if (ret)
> -		gt_err_ratelimited(gt, "error handling GSC irq: %d\n", ret);
> +		gt_err_ratelimited(gt, "failed to handle GSC irq: %d\n", ret);

hmm, changing the same line twice in the one series looks weird
can't we change it in the previous patch ?

>  }
>  
>  void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
> diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
> index d716371fbbe9..477004058834 100644
> --- a/drivers/gpu/drm/xe/xe_heci_gsc.c
> +++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
> @@ -223,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)
> -		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);
> +		xe_err_ratelimited(xe, "failed to handle GSC irq: %d\n", ret);
>  }
>  
>  void xe_heci_csc_irq_handler(struct xe_device *xe, u32 iir)
> @@ -243,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)
> -		xe_err_ratelimited(xe, "error handling GSC irq: %d\n", ret);
> +		xe_err_ratelimited(xe, "failed to handle GSC irq: %d\n", ret);
>  }

[PATCH 5/5] drm/xe/i2c: Report i2c irq handler issue

[Jonathan Cavitt]

Error logging is expected but not included in xe_i2c_irq_handler() for
generic_handle_irq_safe(), so add error logging there.

This issue was caught by static analysis.

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpu/drm/xe/xe_i2c.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 148e82e01ae8..42851e95f75f 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -177,12 +177,15 @@ static bool xe_i2c_irq_present(struct xe_device *xe)
 void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
 {
 	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
+	int ret;
 
 	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
 		return;
 
 	/* Forward interrupt to I2C adapter */
-	generic_handle_irq_safe(xe->i2c->adapter_irq);
+	ret = generic_handle_irq_safe(xe->i2c->adapter_irq);
+	if (ret)
+		xe_err_ratelimited(xe, "failed to handle i2c irq: %d\n", ret);
 
 	/* Deassert after I2C adapter clears the interrupt */
 	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
-- 
2.53.0

✓ i915.CI.BAT: success for drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]

== Series Details ==

Series: drm/{i915, xe}: Refactor generic_handle_irq_safe() error messages (rev2)
URL   : https://patchwork.freedesktop.org/series/167911/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_18637 -> Patchwork_167911v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167911v2/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_167911v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_pm_rpm@basic-rte:
    - bat-rpls-4:         [PASS][1] -> [DMESG-WARN][2] ([i915#13400])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18637/bat-rpls-4/igt@kms_pm_rpm@basic-rte.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167911v2/bat-rpls-4/igt@kms_pm_rpm@basic-rte.html

  
  [i915#13400]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13400


Build changes
-------------

  * Linux: CI_DRM_18637 -> Patchwork_167911v2

  CI-20190529: 20190529
  CI_DRM_18637: 0ecb0a41cb32dfc7620a903629a6951ef1aaea52 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8950: 8950
  Patchwork_167911v2: 0ecb0a41cb32dfc7620a903629a6951ef1aaea52 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167911v2/index.html

[-- Attachment #2: Type: text/html, Size: 2124 bytes --]