On 13.01.2025 09:26, Hajda, Andrzej wrote:
>
> W dniu 10.01.2025 o 16:34, Maciej Patelczyk pisze:
>> When calculating time difference between two timespec values use
>> library function which does it right.
>>
>> Previous way wasn't working well on slower machines. It was only
>> taking the tv_nsec field from struct timespec.
>>
>> Fixes: bbf1730baf ("tests/xe_eudebug_online: Debug client which
>> runs workloads on EU")
>> CC: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
>> Signed-off-by: Maciej Patelczyk <maciej.patelczyk at intel.com>
>> ---
>> tests/intel/xe_eudebug_online.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/intel/xe_eudebug_online.c
>> b/tests/intel/xe_eudebug_online.c
>> index 052774a3b..f532387af 100644
>> --- a/tests/intel/xe_eudebug_online.c
>> +++ b/tests/intel/xe_eudebug_online.c
>> @@ -2235,6 +2235,11 @@ static int find_suitable_engines(struct
>> drm_xe_engine_class_instance **hwes,
>> return engine_count;
>> }
>> +static uint64_t timespecs_diff_us(struct timespec *ts1, struct
>> timespec *ts2)
>> +{
>> + return (uint64_t)(fabs(igt_time_elapsed(ts1, ts2)) * USEC_PER_SEC);
>
>
> It seems igt_time_elapsed handles correctly case ts1 > ts2, so the
> conversion looks correct.
>
> Something to bikeshed is the conversion from int type to double, then
> int again, up to you.
>
> I think there is no need to explicit conversion to uint64_t, no big deal.
>
> Reviewed-by: Andrzej Hajda <andrzej.hajda at intel.com>
>
> Regards
> Andrzej
>
>
Thanks Andrzej.
I wanted encapsulate all in that function so that no further changes are
needed.
I could change the diff variable to double but since WORKLOAD_DELAY_US
is long int what's the point?
Some conversion is unavoidable anyway.
Regards,
Maciej
>> +}
>> +
>> /**
>> * SUBTEST: breakpoint-many-sessions-single-tile
>> * Description:
>> @@ -2255,7 +2260,7 @@ static void test_many_sessions_on_tiles(int fd,
>> bool multi_tile)
>> struct online_debug_data **data;
>> struct drm_xe_engine_class_instance **hwe;
>> struct drm_xe_eudebug_event_eu_attention *eus;
>> - uint64_t current_t, next_t, diff;
>> + uint64_t diff;
>> int attempt_mask = 0, final_mask, should_break;
>> int i;
>> @@ -2318,10 +2323,8 @@ static void test_many_sessions_on_tiles(int
>> fd, bool multi_tile)
>> igt_assert_eq(attempt_mask, final_mask);
>> for (i = 0; i < n - 1; i++) {
>> - /* Convert timestamps to microseconds */
>> - current_t = data[i]->exception_arrived.tv_nsec * 1000;
>> - next_t = data[i + 1]->exception_arrived.tv_nsec * 1000;
>> - diff = current_t < next_t ? next_t - current_t : current_t -
>> next_t;
>> + diff = timespecs_diff_us(&data[i]->exception_arrived,
>> + &data[i + 1]->exception_arrived);
>> if (multi_tile)
>> igt_assert_f(diff < WORKLOAD_DELAY_US,