Re: [PATCH v2 4/5] drm/xe: memirq handler changes

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

On 28.08.2024 11:18, Ilia Levi wrote:
> This patch exposes an interrupt processing handler for a single hw engine.
> This handler also caters for the new "reports to engine instance 0" mode.
> Use this handler from the VF use-case as well.
> 
> Signed-off-by: Ilia Levi <ilia.levi@intel.com>
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_memirq.c | 31 ++++++++++++++++++++++++-------
>  drivers/gpu/drm/xe/xe_memirq.h |  1 +
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_memirq.c b/drivers/gpu/drm/xe/xe_memirq.c
> index a1d4bc3f9e42..c388e480b5c8 100644
> --- a/drivers/gpu/drm/xe/xe_memirq.c
> +++ b/drivers/gpu/drm/xe/xe_memirq.c
> @@ -383,6 +383,28 @@ static void memirq_dispatch_guc(struct xe_memirq *memirq, struct iosys_map *stat
>  		xe_guc_irq_handler(guc, GUC_INTR_GUC2HOST);
>  }
>  
> +/**
> + * xe_memirq_hwe_handler - Check and process interrupts for a specific HW engine.
> + * @memirq: the &xe_memirq
> + * @hwe: the hw engine to process
> + *
> + * This function reads and dispatches `Memory Based Interrupts` for the provided HW engine.
> + */
> +void xe_memirq_hwe_handler(struct xe_memirq *memirq, struct xe_hw_engine *hwe)

shouldn't this be a static memirq_hwe_handler() function?
then there is no need for DOC

> +{
> +	u16 offset = memirq->reports_to_e0 ? hwe->e0_irq_offset : hwe->irq_offset;
> +	u16 instance = memirq->reports_to_e0 ? hwe->instance : 0;

with tweaked hwe->irq_offset in hw_engine_init_early() likely this won't
be needed ...


> +
> +	struct iosys_map src_offset = IOSYS_MAP_INIT_OFFSET(&memirq->bo->vmap,
> +							    XE_MEMIRQ_SOURCE_OFFSET(instance));

empty line should be here not between instance/src above

> +	if (memirq_received(memirq, &src_offset, offset, "SRC")) {
> +		struct iosys_map status_offset =
> +			IOSYS_MAP_INIT_OFFSET(&memirq->bo->vmap,
> +					      XE_MEMIRQ_STATUS_OFFSET(instance) + offset * SZ_16);
> +		memirq_dispatch_engine(memirq, &status_offset, hwe);
> +	}
> +}
> +
>  /**
>   * xe_memirq_handler - The `Memory Based Interrupts`_ Handler.
>   * @memirq: the &xe_memirq
> @@ -410,13 +432,8 @@ void xe_memirq_handler(struct xe_memirq *memirq)
>  		if (gt->tile != tile)
>  			continue;
>  
> -		for_each_hw_engine(hwe, gt, id) {
> -			if (memirq_received(memirq, &memirq->source, hwe->irq_offset, "SRC")) {
> -				map = IOSYS_MAP_INIT_OFFSET(&memirq->status,
> -							    hwe->irq_offset * SZ_16);
> -				memirq_dispatch_engine(memirq, &map, hwe);
> -			}
> -		}
> +		for_each_hw_engine(hwe, gt, id)
> +			xe_memirq_hwe_handler(memirq, hwe);
>  	}
>  
>  	/* GuC and media GuC (if present) must be checked separately */
> diff --git a/drivers/gpu/drm/xe/xe_memirq.h b/drivers/gpu/drm/xe/xe_memirq.h
> index 98e2b61ef973..8a9334482a9b 100644
> --- a/drivers/gpu/drm/xe/xe_memirq.h
> +++ b/drivers/gpu/drm/xe/xe_memirq.h
> @@ -20,6 +20,7 @@ u32 xe_memirq_enable_ptr(struct xe_memirq *memirq);
>  
>  void xe_memirq_reset(struct xe_memirq *memirq);
>  void xe_memirq_postinstall(struct xe_memirq *memirq);
> +void xe_memirq_hwe_handler(struct xe_memirq *memirq, struct xe_hw_engine *hwe);

seems to be not used outside of xe_memirq.c

>  void xe_memirq_handler(struct xe_memirq *memirq);
>  
>  int xe_memirq_init_guc(struct xe_memirq *memirq, struct xe_guc *guc);