Re: [PATCH v6 1/5] drm/xe: Introduce dedicated config for memirq debug

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

On 18.09.2024 07:39, Ilia Levi wrote:
> From: Ilia Levi <ilia.levi@intel.com>
> 
> Separate config for debugging memory based interrupts (memirq)
> infrastructure.
> 
> Signed-off-by: Ilia Levi <ilia.levi@intel.com>

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

> ---

in the future please try to include patch changelog like:

v2: also convert logging macros (Michal)

>  drivers/gpu/drm/xe/Kconfig.debug | 12 ++++++++++++
>  drivers/gpu/drm/xe/xe_memirq.c   | 29 +++++++++++++++++++----------
>  2 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
> index bc177368af6c..2de0de41b8dd 100644
> --- a/drivers/gpu/drm/xe/Kconfig.debug
> +++ b/drivers/gpu/drm/xe/Kconfig.debug
> @@ -40,9 +40,21 @@ config DRM_XE_DEBUG_VM
>  
>  	  If in doubt, say "N".
>  
> +config DRM_XE_DEBUG_MEMIRQ
> +	bool "Enable extra memirq debugging"
> +	default n
> +	help
> +	  Choose this option to enable additional debugging info for
> +	  memory based interrupts.
> +
> +	  Recommended for driver developers only.
> +
> +	  If in doubt, say "N".
> +
>  config DRM_XE_DEBUG_SRIOV
>  	bool "Enable extra SR-IOV debugging"
>  	default n
> +	select DRM_XE_DEBUG_MEMIRQ
>  	help
>  	  Enable extra SR-IOV debugging info.
>  
> diff --git a/drivers/gpu/drm/xe/xe_memirq.c b/drivers/gpu/drm/xe/xe_memirq.c
> index 95b6e9d7b7db..ecb4870c3356 100644
> --- a/drivers/gpu/drm/xe/xe_memirq.c
> +++ b/drivers/gpu/drm/xe/xe_memirq.c
> @@ -20,10 +20,21 @@
>  #include "xe_map.h"
>  #include "xe_memirq.h"
>  #include "xe_sriov.h"
> -#include "xe_sriov_printk.h"
>  
>  #define memirq_assert(m, condition)	xe_tile_assert(memirq_to_tile(m), condition)
> -#define memirq_debug(m, msg...)		xe_sriov_dbg_verbose(memirq_to_xe(m), "MEMIRQ: " msg)
> +#define memirq_printk(m, _level, _fmt, ...)			\
> +	drm_##_level(&memirq_to_xe(m)->drm, "MEMIRQ%u: " _fmt,	\
> +		     memirq_to_tile(m)->id, ##__VA_ARGS__)
> +
> +#ifdef CONFIG_DRM_XE_DEBUG_MEMIRQ
> +#define memirq_debug(m, _fmt, ...)	memirq_printk(m, dbg, _fmt, ##__VA_ARGS__)
> +#else
> +#define memirq_debug(...)
> +#endif
> +
> +#define memirq_err(m, _fmt, ...)	memirq_printk(m, err, _fmt, ##__VA_ARGS__)
> +#define memirq_err_ratelimited(m, _fmt, ...)	\
> +	memirq_printk(m, err_ratelimited, _fmt, ##__VA_ARGS__)
>  
>  static struct xe_tile *memirq_to_tile(struct xe_memirq *memirq)
>  {
> @@ -157,8 +168,7 @@ static int memirq_alloc_pages(struct xe_memirq *memirq)
>  	return drmm_add_action_or_reset(&xe->drm, __release_xe_bo, memirq->bo);
>  
>  out:
> -	xe_sriov_err(memirq_to_xe(memirq),
> -		     "Failed to allocate memirq page (%pe)\n", ERR_PTR(err));
> +	memirq_err(memirq, "Failed to allocate memirq page (%pe)\n", ERR_PTR(err));
>  	return err;
>  }
>  
> @@ -299,9 +309,8 @@ int xe_memirq_init_guc(struct xe_memirq *memirq, struct xe_guc *guc)
>  	return 0;
>  
>  failed:
> -	xe_sriov_err(memirq_to_xe(memirq),
> -		     "Failed to setup report pages in %s (%pe)\n",
> -		     guc_name(guc), ERR_PTR(err));
> +	memirq_err(memirq, "Failed to setup report pages in %s (%pe)\n",
> +		   guc_name(guc), ERR_PTR(err));
>  	return err;
>  }
>  
> @@ -349,9 +358,9 @@ static bool memirq_received(struct xe_memirq *memirq, struct iosys_map *vector,
>  	value = iosys_map_rd(vector, offset, u8);
>  	if (value) {
>  		if (value != 0xff)
> -			xe_sriov_err_ratelimited(memirq_to_xe(memirq),
> -						 "Unexpected memirq value %#x from %s at %u\n",
> -						 value, name, offset);
> +			memirq_err_ratelimited(memirq,
> +					       "Unexpected memirq value %#x from %s at %u\n",
> +					       value, name, offset);
>  		iosys_map_wr(vector, offset, u8, 0x00);
>  	}
>