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

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

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