Hi Janne,
On 2026-01-05 at 22:03:47 +0100, Janne Grunau wrote:
> Test that the call succeeds, to subsequent calls do not produce the same
> timestamps and invalid flag values are rejected.
> 

+cc Alyssa and Neal

> Signed-off-by: Janne Grunau <[email protected]>
> ---
>  lib/igt_asahi.c              | 14 ++++++++++
>  lib/igt_asahi.h              |  3 +++
>  tests/asahi/asahi_get_time.c | 63 
> ++++++++++++++++++++++++++++++++++++++++++++
>  tests/asahi/meson.build      |  1 +
>  4 files changed, 81 insertions(+)
> 
> diff --git a/lib/igt_asahi.c b/lib/igt_asahi.c
> index 
> 90d2c190f0dd05e372af0eefaed22d2b2a26eb71..17ac60749a3a5b03722403386f8b16cc0caeff3c
>  100644
> --- a/lib/igt_asahi.c
> +++ b/lib/igt_asahi.c
> @@ -42,3 +42,17 @@ void igt_asahi_get_params(int fd, uint32_t param_group, 
> void *params, size_t siz
>       else
>               do_ioctl(fd, DRM_IOCTL_ASAHI_GET_PARAMS, &get_params);
>  }
> +
> +/**
> + * igt_asahi_get_time:
> + * @fd: device file descriptor
> + * @get_time: pointer to drm_asahi_get_time struct
> + * @err: expected error code, 0 for success
> + */
> +void igt_asahi_get_time(int fd, struct drm_asahi_get_time *get_time, int err)
> +{
> +     if (err)
> +             do_ioctl_err(fd, DRM_IOCTL_ASAHI_GET_TIME, get_time, err);
> +     else
> +             do_ioctl(fd, DRM_IOCTL_ASAHI_GET_TIME, get_time);
> +}
> diff --git a/lib/igt_asahi.h b/lib/igt_asahi.h
> index 
> f0ac3fbf428a8050957eab0e9b259f68b5ecd0cd..a15acc5a08092fae0b3a569c527087082e6fc05c
>  100644
> --- a/lib/igt_asahi.h
> +++ b/lib/igt_asahi.h
> @@ -7,6 +7,9 @@
>  #include <stddef.h>
>  #include <stdint.h>
>  
> +#include "asahi_drm.h"
> +

imho this should be in previous patch.

>  void igt_asahi_get_params(int fd, uint32_t param_group, void *data, size_t 
> size, int err);
> +void igt_asahi_get_time(int fd, struct drm_asahi_get_time *get_time, int 
> err);
>  
>  #endif /* ASAHI_IOCTL_H */
> diff --git a/tests/asahi/asahi_get_time.c b/tests/asahi/asahi_get_time.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..91c865d0f8c65b410771b491758a81b4d4a96044
> --- /dev/null
> +++ b/tests/asahi/asahi_get_time.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: MIT
> +// SPDX-FileCopyrightText: Copyright (C) Asahi Linux contributors
> +
> +#include "igt.h"
> +#include "igt_core.h"
> +#include "igt_asahi.h"

It should be before igt_core.h

> +#include "asahi_drm.h"

Now it is redundant, you have it in igt_asahi.h

> +#include <stdint.h>

Should be first include but it is also redundant as it
is in igt_asahi.h

> +
> +int igt_main()
> +{
> +     int fd;
> +
> +     igt_fixture() {
> +             fd = drm_open_driver_render(DRIVER_ASAHI);
> +     }
> +
> +     igt_describe("Query GPU device time.");
> +     igt_subtest("get-time") {
> +             struct drm_asahi_get_time time = {};
> +
> +             igt_asahi_get_time(fd, &time, 0);
> +             // Nothing to assert on, the timestamp could have any value

Use C-style comments:
                /* Nothing to assert on, the timestamp could have any value */
So why not just printing it with igt_info()

> +     }
> +
> +     igt_describe("Query GPU device time twice and compare timestamps.");
> +     igt_subtest("get-time-compare") {
> +             struct drm_asahi_get_time time1 = {};
> +             struct drm_asahi_get_time time2 = {};
> +
> +             igt_asahi_get_time(fd, &time1, 0);
> +
> +             // sleep for 100 micro seconds to ensure

Same here, C-style /* comment... */

> +             usleep(100);
> +
> +             igt_asahi_get_time(fd, &time2, 0);
> +
> +             // assert that the timestamps are different

Same here.

> +             igt_assert(time1.gpu_timestamp != time2.gpu_timestamp);

You could also use igt_assert_noteq so you will get also actual
value printed in case they are the same.

> +     }
> +
> +     igt_describe("Query GPU device time with invalid flags values.");
> +     igt_subtest_group() {
> +             struct drm_asahi_get_time time = {};
> +
> +             igt_subtest("get-time-flags-1") {

imho better name invalid-time-flags-1

> +                     time.flags = 1;
> +                     igt_asahi_get_time(fd, &time, EINVAL);
> +             }
> +             igt_subtest("get-time-flags-2") {

invalid-time-flags-2

> +                     time.flags = 2;
> +                     igt_asahi_get_time(fd, &time, EINVAL);
> +             }
> +             igt_subtest("get-time-flags-uint64-max") {

invalid-time-flags-max

All above are minors so after fix imho you could add r-b
from Alyssa and Neal.

Regards,
Kamil

> +                     time.flags = UINT64_MAX;
> +                     igt_asahi_get_time(fd, &time, EINVAL);
> +             }
> +     }
> +
> +     igt_fixture() {
> +             drm_close_driver(fd);
> +     }
> +}
> diff --git a/tests/asahi/meson.build b/tests/asahi/meson.build
> index 
> 909e146295e83f558ef7378f814ded55adaafe2b..2997017924f72443a7b5ad907c52a9976f464810
>  100644
> --- a/tests/asahi/meson.build
> +++ b/tests/asahi/meson.build
> @@ -1,5 +1,6 @@
>  asahi_progs = [
>       'asahi_get_params',
> +     'asahi_get_time',
>  ]
>  
>  foreach prog : asahi_progs
> 
> -- 
> 2.52.0
> 

Reply via email to