Re: [PATCH 12/13] drm/xe/memirq: Dump additional source pages if MSI-X

["Levi, Ilia" <ilia.levi@intel.com>]

On 4/28/2026 5:27 PM, Michal Wajdeczko wrote:
> In MSI-X setup engines report their source/status as instance 0
> and we should dump additional source pages, not just the first one.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_memirq.c       | 35 +++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_memirq_types.h |  2 ++
>  2 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_memirq.c b/drivers/gpu/drm/xe/xe_memirq.c
> index 30266b258087..b6fdefaf9463 100644
> --- a/drivers/gpu/drm/xe/xe_memirq.c
> +++ b/drivers/gpu/drm/xe/xe_memirq.c
> @@ -169,6 +169,21 @@ static inline bool hw_reports_to_instance_zero(struct xe_memirq *memirq)
>  	return xe_device_has_msix(memirq_to_xe(memirq));
>  }
>  
> +static unsigned int max_instance(struct xe_tile *tile)
> +{
> +	enum xe_engine_class ec;
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	u32 emask = 0;
> +
> +	for_each_gt_on_tile(gt, tile, id) {
> +		for (ec = XE_ENGINE_CLASS_RENDER; ec < XE_ENGINE_CLASS_MAX; ec++)
> +			emask |= xe_hw_engine_mask_per_class(gt, ec);
> +	}
> +
> +	return fls(emask);


Wouldn't it be more readable to do something like:

    for_each_gt_on_tile(gt, tile, gtid)
        for_each_hw_engine(hwe, gt, id)
            max_inst = max(max_inst, hwe->instance);
    return max_inst + 1;

- Ilia

> +}
> +
>  static int memirq_alloc_pages(struct xe_memirq *memirq)
>  {
>  	struct xe_device *xe = memirq_to_xe(memirq);
> @@ -210,6 +225,9 @@ static int memirq_alloc_pages(struct xe_memirq *memirq)
>  		     xe_bo_ggtt_addr(bo), bo_size, XE_MEMIRQ_SOURCE_OFFSET(0),
>  		     XE_MEMIRQ_STATUS_OFFSET(0));
>  
> +	memirq->num_pages = hw_reports_to_instance_zero(memirq) ? max_instance(tile) : 1;


Don't you want to use max_instance in the calculation of BO size instead of XE_HW_ENGINE_MAX_INSTANCE?
- Ilia


> +	memirq_debug(memirq, "page instances: %u\n", memirq->num_pages);
> +
>  	return 0;
>  
>  out:
> @@ -525,6 +543,18 @@ bool xe_memirq_guc_sw_int_0_irq_pending(struct xe_memirq *memirq, struct xe_guc
>  				       guc_name(guc));
>  }
>  
> +static void memirq_dump_source_pages(struct xe_memirq *memirq)
> +{
> +	memirq_assert(memirq, !memirq->source.is_iomem);
> +
> +	for (int n = 0; n < memirq->num_pages; n++) {
> +		memirq_debug(memirq, "SOURCE %*ph\n", 32,
> +			     memirq->bo->vmap.vaddr + memirq_source_page_offset(memirq, n));
> +		memirq_debug(memirq, "SOURCE %*ph\n", 32,
> +			     memirq->bo->vmap.vaddr + memirq_source_page_offset(memirq, n) + 32);
> +	}
> +}
> +
>  /**
>   * xe_memirq_handler - The `Memory Based Interrupts`_ Handler.
>   * @memirq: the &xe_memirq
> @@ -544,9 +574,8 @@ void xe_memirq_handler(struct xe_memirq *memirq)
>  	if (!memirq->bo)
>  		return;
>  
> -	memirq_assert(memirq, !memirq->source.is_iomem);
> -	memirq_debug(memirq, "SOURCE %*ph\n", 32, memirq->source.vaddr);
> -	memirq_debug(memirq, "SOURCE %*ph\n", 32, memirq->source.vaddr + 32);
> +	if (IS_ENABLED(CONFIG_DRM_XE_DEBUG_MEMIRQ))
> +		memirq_dump_source_pages(memirq);
>  
>  	for_each_gt(gt, xe, gtid) {
>  		if (gt->tile != tile)
> diff --git a/drivers/gpu/drm/xe/xe_memirq_types.h b/drivers/gpu/drm/xe/xe_memirq_types.h
> index 02ac938cadb4..eab384342878 100644
> --- a/drivers/gpu/drm/xe/xe_memirq_types.h
> +++ b/drivers/gpu/drm/xe/xe_memirq_types.h
> @@ -14,6 +14,7 @@ struct xe_bo;
>   * struct xe_memirq - Data used by the `Memory Based Interrupts`_.
>   *
>   * @bo: buffer object with `Memory Based Interrupts Page Layout`_.
> + * @num_pages: number of per-instance source/status pages.
>   * @source: iosys pointer to `Interrupt Source Report Page`_.
>   * @status: iosys pointer to `Interrupt Status Report Page`_.
>   * @mask: iosys pointer to Interrupt Enable Mask.
> @@ -21,6 +22,7 @@ struct xe_bo;
>   */
>  struct xe_memirq {
>  	struct xe_bo *bo;
> +	unsigned int num_pages;
>  	struct iosys_map source;
>  	struct iosys_map status;
>  	struct iosys_map mask;