[Intel-xe] [PATCH 6/7] drm/xe/reg_sr: Save errors for kunit integration

[Lucas De Marchi]

On Mon, Mar 27, 2023 at 08:29:41PM +0200, Michał Winiarski wrote:
>On Tue, Mar 21, 2023 at 03:05:26PM -0700, Lucas De Marchi wrote:
>> When there's an entry that is dropped when xe_reg_sr_add(), there's
>> not much we can do other than reporting the error - it's for certain a
>> driver issue or conflicting workarounds/tunings. Save the number of
>> errors to be used later by kunit to report where it happens.
>
>Why there's not much we can do?
>We can propagate the error to the upper layers and fail the probe.
>Fail-fast?

I thought about adding a patch to propagate the errors, but it looked
like the wrong thing to do. Note that the following test is for the WAs.
Completely failing the probe() because we have a duplicate or incompatible
entries would be the equivalent of spreading BUG_ON() in the driver so
we can "fail fast".  Duplicate entries for example are completly
harmless (but undesired).  Even for incompatible entries, I don't
think it justifies blocking the module probe.

Lucas De Marchi

>
>The following test still makes sense (and LGTM), since it's still a good
>idea to be able to validate everything without having to probe() on
>actual platform. I would expect it to check return values rather than
>sr->errors though.
>
>-Michał
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_reg_sr.c       | 8 ++++++++
>>  drivers/gpu/drm/xe/xe_reg_sr_types.h | 4 ++++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
>> index 9eaf1be27886..83f29aeb9250 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
>> @@ -84,6 +84,13 @@ static bool compatible_entries(const struct xe_reg_sr_entry *e1,
>>  	return true;
>>  }
>>
>> +static void reg_sr_inc_error(struct xe_reg_sr *sr)
>> +{
>> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
>> +	sr->errors++;
>> +#endif
>> +}
>> +
>>  int xe_reg_sr_add(struct xe_reg_sr *sr, u32 reg,
>>  		  const struct xe_reg_sr_entry *e)
>>  {
>> @@ -121,6 +128,7 @@ int xe_reg_sr_add(struct xe_reg_sr *sr, u32 reg,
>>  	DRM_ERROR("Discarding save-restore reg %04lx (clear: %08x, set: %08x, masked: %s): ret=%d\n",
>>  		  idx, e->clr_bits, e->set_bits,
>>  		  str_yes_no(e->masked_reg), ret);
>> +	reg_sr_inc_error(sr);
>>
>>  	return ret;
>>  }
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr_types.h b/drivers/gpu/drm/xe/xe_reg_sr_types.h
>> index 3d2257891005..91469784fd90 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr_types.h
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr_types.h
>> @@ -32,6 +32,10 @@ struct xe_reg_sr {
>>  	} pool;
>>  	struct xarray xa;
>>  	const char *name;
>> +
>> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
>> +	unsigned int errors;
>> +#endif
>>  };
>>
>>  #endif
>> --
>> 2.39.0
>>