Re: [PATCH i-g-t v8 6/6] tests/kms_flip: test suspend at most twice on SNB && BMG

["Michał Grzelak" <michal.grzelak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]

On Wed, 25 Mar 2026, Thasleem, Mohammed wrote:
> On 09-03-2026 02:55 pm, Michał Grzelak wrote:
> 
> gettime_us() on SNB and BMG does not return proper time after starting
> suspend tests. This results in too many suspends issued before we hit
> the duration_ms. Break the loop on second execution.
> 
> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
> ---
>  tests/kms_flip.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index f17d027cc..3c5428ff0 100755
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1322,6 +1322,12 @@ static bool event_loop(struct test_output *o, unsigned duration_ms,
>  {
>  	unsigned long start, end;
>  	int count = 0;
> +	int devid;
> +	bool wa;
> +
> +	devid = intel_get_drm_devid(drm_fd);
> +
> +	wa = IS_SANDYBRIDGE(devid) || IS_BATTLEMAGE(devid);
> 
> Why only two specific platforms?

That I would also like to know. :)
As it is described in the first sentence of the commit message, these
two platforms have been reported by CI to encounter such issue. If it is
not clear, I can update the commit message.

> I think, platform specific wa not needed here, instead we can have generic execution for all,
> which can work on all platforms. plz check.

Since we are observing the issue only on those two platforms, I don't
see how to work around it without specifying those two platforms,
without changing the already present functionality.

>
>
>  	start = gettime_us();
> 
> @@ -1342,6 +1348,9 @@ static bool event_loop(struct test_output *o, unsigned duration_ms,
>  		if (count && (gettime_us() - start) / 1000 >= duration_ms)
>  			break;
> 
> +		if (count && wa && o->flags & TEST_SUSPEND)
> 
> if (count && wa && o->flags & TEST_SUSPEND) --> if (count && o->flags & TEST_SUSPEND)
> 
> +			break;
> +

I am unsure of this approach. This will break the loop on second run on
every suspend test, on every platform. While commit c75dcbdac51d
("tests/kms_flip: various improvements") says explicitly to do at least
two rounds of tests, so it would change functionality on every platform.
I can refactor the patch, but I would retain the WA. Can you provide
some rationale for changing the functionality on non-affected platforms?

BR,
Michał

>  		count++;
>  	}
> 
> 
> 
>