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

[Ilia Levi]

On 28/08/2024 23:42, Michal Wajdeczko wrote:
> 
> 
> 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 at intel.com>
>> Reviewed-by: Jonathan Cavitt <jonathan.cavitt at 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" ?
> 

I hope to keep this change completely equivalent for the previous flows - both VF and non VF/non MSI-X.
For a number of registers below it's not exactly clear whether they need to be set for MSI-X or not, but I'd rather fix the MSI-X case when needed than accidentally break a working non MSI-X flow.
- Ilia

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