Re: [PATCH i-g-t] i915/perf: Fix loop termination logic in two tests

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 17 Jan 2024 20:55:25 -0800, Ashutosh Dixit wrote:
>

Hi Umesh,

> The code in af9910b3967d ("i915/perf: Check regularly if we are done
> reading reports") does not match the commit description. Fix the code to
> match the commit description.
>
> Fixes: af9910b3967d ("i915/perf: Check regularly if we are done reading reports")
> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  tests/intel/perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/intel/perf.c b/tests/intel/perf.c
> index 3565d61cc393..94738b8a58b4 100644
> --- a/tests/intel/perf.c
> +++ b/tests/intel/perf.c
> @@ -3052,7 +3052,7 @@ test_buffer_fill(const struct intel_execution_engine2 *e)
>						first_timestamp = oa_timestamp(report, fmt);
>					last_timestamp = oa_timestamp(report, fmt);
>
> -					if (((last_timestamp - first_timestamp) * oa_period) < (fill_duration / 2))
> +					if (((last_timestamp - first_timestamp) * oa_period) >= (fill_duration / 2))
>						break;
>
>					if (oa_report_is_periodic(oa_exponent, report)) {
> @@ -3279,7 +3279,7 @@ test_enable_disable(const struct intel_execution_engine2 *e)
>						  oa_report_is_periodic(oa_exponent, report),
>						  oa_report_get_ctx_id(report));
>
> -					if (((last_timestamp - first_timestamp) * oa_period) < (fill_duration / 2))
> +					if (((last_timestamp - first_timestamp) * oa_period) >= (fill_duration / 2))
>						break;
>
>					if (oa_report_is_periodic(oa_exponent, report)) {

So there are CI failures with this patch, which I was expecting since I was
seeing the failures locally.

I ran into failures in the two tests above after making some change to Xe
OA code (after removing report headers) and this patch fixed those
failures. To me the code in af9910b3967d looks obviously wrong, I am not
sure what failures it was solving. I think the right thing to do would be
to revert af9910b3967d.

Anyway, no need to do anything for i915, but to get round failures in the
latest Xe code I am dropping the two lines above (basically reverting
af9910b3967d) for Xe.

Thanks.
--
Ashutosh