Re: [PATCH v2 5/5] drm/xe: separate memirq reset/postinstall from vf

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

On 28.08.2024 11:18, Ilia Levi wrote:
> Up until now only VF used Memory Based Interrupts (memirq).
> Moving reset/postinstall flows out of VF to cater for other usages.
> 
> Signed-off-by: Ilia Levi <ilia.levi@intel.com>
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c |  2 +-
>  drivers/gpu/drm/xe/xe_device.h |  6 ++++++
>  drivers/gpu/drm/xe/xe_irq.c    | 36 +++++++++++++++++++---------------
>  3 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index c4f8bfd08b09..a23fb0c81ca6 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -660,7 +660,7 @@ int xe_device_probe(struct xe_device *xe)
>  		err = xe_ggtt_init_early(tile->mem.ggtt);
>  		if (err)
>  			return err;
> -		if (IS_SRIOV_VF(xe)) {
> +		if (xe_device_needs_memirq(xe)) {
>  			err = xe_memirq_init(&tile->memirq, false);
>  			if (err)
>  				return err;
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index f052c06a2d2f..4ba07617084c 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -9,6 +9,7 @@
>  #include <drm/drm_util.h>
>  
>  #include "xe_device_types.h"
> +#include "xe_sriov.h"
>  
>  static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>  {
> @@ -154,6 +155,11 @@ static inline bool xe_device_has_memirq(struct xe_device *xe)
>  	return GRAPHICS_VERx100(xe) >= 1250;
>  }
>  
> +static inline bool xe_device_needs_memirq(struct xe_device *xe)
> +{
> +	return IS_SRIOV_VF(xe) && xe_device_has_memirq(xe);
> +}

maybe worth to introduce this helper earlier in the series?

> +
>  u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size);
>  
>  void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 18a5272d9bb0..a080d4869b16 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -559,17 +559,15 @@ static void vf_irq_reset(struct xe_device *xe)
>  
>  	xe_assert(xe, IS_SRIOV_VF(xe));
>  
> -	if (GRAPHICS_VERx100(xe) < 1210)
> -		xelp_intr_disable(xe);
> -	else
> +	if (GRAPHICS_VERx100(xe) >= 1210) {
>  		xe_assert(xe, xe_device_has_memirq(xe));
> -
> -	for_each_tile(tile, xe, id) {
> -		if (xe_device_has_memirq(xe))
> -			xe_memirq_reset(&tile->memirq);
> -		else
> -			gt_irq_reset(tile);
> +		return;
>  	}
> +
> +	xelp_intr_disable(xe);
> +
> +	for_each_tile(tile, xe, id)
> +		gt_irq_reset(tile);
>  }
>  
>  static void xe_irq_reset(struct xe_device *xe)
> @@ -577,6 +575,11 @@ static void xe_irq_reset(struct xe_device *xe)
>  	struct xe_tile *tile;
>  	u8 id;
>  
> +	if (xe_device_needs_memirq(xe)) {
> +		for_each_tile(tile, xe, id)
> +			xe_memirq_reset(&tile->memirq);
> +	}
> +

"else" or "return" ?

>  	if (IS_SRIOV_VF(xe))
>  		return vf_irq_reset(xe);
>  
> @@ -604,13 +607,6 @@ static void xe_irq_reset(struct xe_device *xe)
>  
>  static void vf_irq_postinstall(struct xe_device *xe)
>  {
> -	struct xe_tile *tile;
> -	unsigned int id;
> -
> -	for_each_tile(tile, xe, id)
> -		if (xe_device_has_memirq(xe))
> -			xe_memirq_postinstall(&tile->memirq);
> -
>  	if (GRAPHICS_VERx100(xe) < 1210)
>  		xelp_intr_enable(xe, true);
>  	else
> @@ -619,6 +615,14 @@ static void vf_irq_postinstall(struct xe_device *xe)
>  
>  static void xe_irq_postinstall(struct xe_device *xe)
>  {
> +	struct xe_tile *tile;
> +	unsigned int id;
> +
> +	if (xe_device_needs_memirq(xe)) {
> +		for_each_tile(tile, xe, id)
> +			xe_memirq_postinstall(&tile->memirq);
> +	}
> +
>  	if (IS_SRIOV_VF(xe))
>  		return vf_irq_postinstall(xe);
>