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

Michał Grzelak
2026-03-26T09:29:02Z

[-- 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++;
> }
>
>
>
>