Re: [igt-dev] [PATCH i-g-t] i915/gem_create: Do not build create-clear for MIPS

[Petri Latvala <petri.latvala@intel.com>]

On Mon, Apr 01, 2019 at 04:39:24PM +0200, Guillaume Tucker wrote:
> The MIPS architecture doesn't provide the hardware atomics that are
> required for the "create-clear" sub-test such as
> __sync_add_and_fetch().  As a simple and pragmatic solution, disable
> this sub-test when building for MIPS.  A better approach would be to
> add a fallback implementation for these operations.
> 
> Fixes: 6727e17c00b2 ("i915/gem_create: Verify that all new objects are clear")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  tests/i915/gem_create.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
> index 2a861ca8a7ec..8a48496e6c19 100644
> --- a/tests/i915/gem_create.c
> +++ b/tests/i915/gem_create.c
> @@ -142,6 +142,7 @@ static void invalid_nonaligned_size(int fd)
>  	gem_close(fd, handle);
>  }
>  
> +#if !defined(__mips__) /* MIPS doesn't provide the required hardware atomics */
>  static uint64_t get_npages(uint64_t *global, uint64_t npages)
>  {
>  	uint64_t try, old, max;
> @@ -208,6 +209,7 @@ static void always_clear(int i915, int timeout)
>  	for (int i = 0; i < ncpus; i++)
>  		pthread_join(thread[i], NULL);
>  }
> +#endif /* !defined(__mips__) */
>  
>  igt_main
>  {
> @@ -231,6 +233,8 @@ igt_main
>  	igt_subtest("create-invalid-nonaligned")
>  		invalid_nonaligned_size(fd);
>  
> +#if !defined(__mips__)
>  	igt_subtest("create-clear")
>  		always_clear(fd, 30);
> +#endif
>  }


It's a bit ugly. I wonder how much work a fallback mechanism would be?

The test is i915 specific and using those on non-x86 architectures
sounds silly. We could limit building tests/i915/* only if
host_machine.cpu_family() is x86 or x86_64. But that requires
revisiting this issue if ever the day comes when i915 can be used on
other architectures *cough*.

Apropos, compile-testing on MIPS in gitlab-CI?

A compile-tested-only fallback mechanism suggestion, and a bad spot
for placing the fallback functions:

diff --git a/meson.build b/meson.build
index 557400a5..0552e858 100644
--- a/meson.build
+++ b/meson.build
@@ -246,6 +246,9 @@ endif
 have = cc.has_function('memfd_create', prefix : '''#include <sys/mman.h>''', args : '-D_GNU_SOURCE')
 config.set10('HAVE_MEMFD_CREATE', have)
 
+have_atomics = cc.compiles('void f() { int x, y; __sync_add_and_fetch(&x, y); }')
+config.set10('HAVE_BUILTIN_ATOMICS', have_atomics)
+
 add_project_arguments('-D_GNU_SOURCE', language : 'c')
 add_project_arguments('-include', 'config.h', language : 'c')
 
diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
index 2a861ca8..615bb475 100644
--- a/tests/i915/gem_create.c
+++ b/tests/i915/gem_create.c
@@ -62,6 +62,18 @@ IGT_TEST_DESCRIPTION("This is a test for the extended & old gem_create ioctl,"
 		     " that includes allocation of object from stolen memory"
 		     " and shmem.");
 
+#if !HAVE_BUILTIN_ATOMICS
+int __sync_add_and_fetch(void *ptr, uint64_t val)
+{
+  igt_assert_f(false, "Don't have builtin atomics\n");
+}
+
+int __sync_val_compare_and_swap(void *ptr, uint64_t old, uint64_t new)
+{
+  igt_assert_f(false, "Don't have builtin atomics\n");
+}
+#endif
+
 #define CLEAR(s) memset(&s, 0, sizeof(s))
 #define PAGE_SIZE 4096
 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev