RFC: Make igts for cross-driver stuff mandatory?

Submitted by Sean Paul on Oct. 25, 2018, 12:51 p.m.

Details

Message ID 20181025125149.GG154160@art_vandelay
State New
Headers show
Series "RFC: Make igts for cross-driver stuff mandatory?" ( rev: 2 ) in DRI devel

Not browsing as part of any series.

Commit Message

Sean Paul Oct. 25, 2018, 12:51 p.m. 
On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> Hi all,
> 
> This is just to collect feedback on this idea, and see whether the
> overall dri-devel community stands on all this. I think the past few
> cross-vendor uapi extensions all came with igts attached, and
> personally I think there's lots of value in having them: A
> cross-vendor interface isn't useful if every driver implements it
> slightly differently.
> 
> I think there's 2 questions here:
> 
> - Do we want to make such testcases mandatory?
> 

Yes, more testing == better code.


> - If yes, are we there yet, or is there something crucially missing
>   still?

In my experience, no. Last week while trying to replicate an intel-gfx CI
failure, I tried compiling igt for one of my (intel) chromebooks. It seems like
cross-compilation (or, in my case, just specifying
prefix/ld_library_path/sbin_path) is broken on igt. If we want to impose
restrictions across the entire subsystem, we need to make sure that everyone can
build and deploy igt easily.

I managed to hack around everything and get it working, but I still haven't
tried switching out the toolchain. Once we have some GitLab CI to validate
cross-compilation, then we can consider making IGT mandatory.

It's possible that I'm just a meson n00b and didn't use the right incantation,
so maybe it already works, but then we need better documentation.

I've pasted my horrible hacks below, I also didn't have libunwind, so removed
its usage.

Sean


/snip


From ab8c7d274c32559295b38d6ceeaabded14b207d4 Mon Sep 17 00:00:00 2001
From: Sean Paul <seanpaul@chromium.org>
Date: Thu, 25 Oct 2018 08:40:28 -0400
Subject: [PATCH] igt: Hacks to compile in CrOS chroot

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 lib/igt_core.c            | 78 ---------------------------------------
 lib/meson.build           |  1 -
 meson.build               |  4 +-
 tests/gem_userptr_blits.c |  2 +
 tools/meson.build         |  7 ----
 5 files changed, 5 insertions(+), 87 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 23bb858f..ca65d7cc 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -71,8 +71,6 @@ 
 #include "igt_sysrq.h"
 #include "igt_rc.h"
 
-#define UNW_LOCAL_ONLY
-#include <libunwind.h>
 #include <elfutils/libdwfl.h>
 
 #ifdef HAVE_LIBGEN_H
@@ -1209,63 +1207,6 @@  static void write_stderr(const char *str)
 
 static void print_backtrace(void)
 {
-	unw_cursor_t cursor;
-	unw_context_t uc;
-	int stack_num = 0;
-
-	Dwfl_Callbacks cbs = {
-		.find_elf = dwfl_linux_proc_find_elf,
-		.find_debuginfo = dwfl_standard_find_debuginfo,
-	};
-
-	Dwfl *dwfl = dwfl_begin(&cbs);
-
-	if (dwfl_linux_proc_report(dwfl, getpid())) {
-		dwfl_end(dwfl);
-		dwfl = NULL;
-	} else
-		dwfl_report_end(dwfl, NULL, NULL);
-
-	igt_info("Stack trace:\n");
-
-	unw_getcontext(&uc);
-	unw_init_local(&cursor, &uc);
-	while (unw_step(&cursor) > 0) {
-		char name[255];
-		unw_word_t off, ip;
-		Dwfl_Module *mod = NULL;
-
-		unw_get_reg(&cursor, UNW_REG_IP, &ip);
-
-		if (dwfl)
-			mod = dwfl_addrmodule(dwfl, ip);
-
-		if (mod) {
-			const char *src, *dwfl_name;
-			Dwfl_Line *line;
-			int lineno;
-			GElf_Sym sym;
-
-			line = dwfl_module_getsrc(mod, ip);
-			dwfl_name = dwfl_module_addrsym(mod, ip, &sym, NULL);
-
-			if (line && dwfl_name) {
-				src = dwfl_lineinfo(line, NULL, &lineno, NULL, NULL, NULL);
-				igt_info("  #%d %s:%d %s()\n", stack_num++, src, lineno, dwfl_name);
-				continue;
-			}
-		}
-
-		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
-			igt_info("  #%d [<unknown>+0x%x]\n", stack_num++,
-				 (unsigned int) ip);
-		else
-			igt_info("  #%d [%s+0x%x]\n", stack_num++, name,
-				 (unsigned int) off);
-	}
-
-	if (dwfl)
-		dwfl_end(dwfl);
 }
 
 static const char hex[] = "0123456789abcdef";
@@ -1420,25 +1361,6 @@  xprintf(const char *fmt, ...)
 
 static void print_backtrace_sig_safe(void)
 {
-	unw_cursor_t cursor;
-	unw_context_t uc;
-	int stack_num = 0;
-
-	write_stderr("Stack trace: \n");
-
-	unw_getcontext(&uc);
-	unw_init_local(&cursor, &uc);
-	while (unw_step(&cursor) > 0) {
-		char name[255];
-		unw_word_t off;
-
-		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
-			xstrlcpy(name, "<unknown>", 10);
-
-		xprintf(" #%d [%s+0x%x]\n", stack_num++, name,
-				(unsigned int) off);
-
-	}
 }
 
 void __igt_fail_assert(const char *domain, const char *file, const int line,
diff --git a/lib/meson.build b/lib/meson.build
index 7e2c9b7a..26cef0c6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -59,7 +59,6 @@  lib_deps = [
 	libkmod,
 	libprocps,
 	libudev,
-	libunwind,
 	libdw,
 	pciaccess,
 	pthreads,
diff --git a/meson.build b/meson.build
index eff35585..86ad6602 100644
--- a/meson.build
+++ b/meson.build
@@ -102,7 +102,6 @@  build_info += 'With libdrm: ' + ','.join(libdrm_info)
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
 libprocps = dependency('libprocps', required : true)
-libunwind = dependency('libunwind', required : true)
 libdw = dependency('libdw', required : true)
 ssl = dependency('openssl', required : true)
 pixman = dependency('pixman-1', required : true)
@@ -217,12 +216,15 @@  prefix = get_option('prefix')
 bindir = get_option('bindir')
 datadir = join_paths(get_option('datadir'), 'igt-gpu-tools')
 includedir = get_option('includedir')
+fullincdir = join_paths(prefix, includedir)
 libdir = get_option('libdir')
 libexecdir = join_paths(get_option('libexecdir'), 'igt-gpu-tools')
 amdgpudir = join_paths(libexecdir, 'amdgpu')
 mandir = get_option('mandir')
 pkgconfigdir = join_paths(libdir, 'pkgconfig')
 
+inc = [ include_directories(fullincdir, join_paths( fullincdir, 'libdrm')), inc ]
+
 if get_option('use_rpath')
 	# Set up runpath for the test executables towards libigt.so.
 	# The path should be relative to $ORIGIN so the library is
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index 909dd19d..92d5a6e1 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -1069,6 +1069,7 @@  static void store_dword_rand(int i915, unsigned int engine,
 
 static void test_readonly(int i915)
 {
+#if 0
 	unsigned char orig[SHA_DIGEST_LENGTH];
 	uint64_t aperture_size;
 	uint32_t whandle, rhandle;
@@ -1178,6 +1179,7 @@  static void test_readonly(int i915)
 
 	munmap(space, total);
 	munmap(pages, sz);
+#endif
 }
 
 static jmp_buf sigjmp;
diff --git a/tools/meson.build b/tools/meson.build
index 79f36aa9..5e990cd7 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -90,11 +90,6 @@  install_subdir('registers', install_dir : datadir,
 		 'Makefile', 'Makefile.in', 'Makefile.am',
 	       ])
 
-shared_library('intel_aubdump', 'aubdump.c',
-	       dependencies : [ lib_igt_chipset, dlsym ],
-	       name_prefix : '',
-	       install : true)
-
 executable('intel_gpu_top', 'intel_gpu_top.c',
 	   install : true,
 	   install_rpath : bindir_rpathdir,
@@ -104,7 +99,5 @@  conf_data = configuration_data()
 conf_data.set('prefix', prefix)
 conf_data.set('exec_prefix', '${prefix}')
 conf_data.set('libdir', join_paths('${prefix}', libdir))
-configure_file(input : 'intel_aubdump.in', output : 'intel_aubdump',
-               configuration : conf_data, install_dir : bindir)
 
 subdir('null_state_gen')

Comments

Eric Anholt Oct. 25, 2018, 4:35 p.m. 
Sean Paul <sean@poorly.run> writes:

> On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
>> Hi all,
>> 
>> This is just to collect feedback on this idea, and see whether the
>> overall dri-devel community stands on all this. I think the past few
>> cross-vendor uapi extensions all came with igts attached, and
>> personally I think there's lots of value in having them: A
>> cross-vendor interface isn't useful if every driver implements it
>> slightly differently.
>> 
>> I think there's 2 questions here:
>> 
>> - Do we want to make such testcases mandatory?
>> 
>
> Yes, more testing == better code.
>
>
>> - If yes, are we there yet, or is there something crucially missing
>>   still?
>
> In my experience, no. Last week while trying to replicate an intel-gfx CI
> failure, I tried compiling igt for one of my (intel) chromebooks. It seems like
> cross-compilation (or, in my case, just specifying
> prefix/ld_library_path/sbin_path) is broken on igt. If we want to impose
> restrictions across the entire subsystem, we need to make sure that everyone can
> build and deploy igt easily.
>
> I managed to hack around everything and get it working, but I still haven't
> tried switching out the toolchain. Once we have some GitLab CI to validate
> cross-compilation, then we can consider making IGT mandatory.
>
> It's possible that I'm just a meson n00b and didn't use the right incantation,
> so maybe it already works, but then we need better documentation.
>
> I've pasted my horrible hacks below, I also didn't have libunwind, so removed
> its usage.

I've also had to cut out libunwind for cross-compiling on many
occasions.  Worst library.
Zhou, David(ChunMing) Oct. 26, 2018, 3:49 a.m. 
Make igt for cross-driver, I think you should rename it first, not an intel specific. NO company wants their employee working on other company stuff.
You can rename it to DGT(drm graphics test), and published following  libdrm, or directly merge to libdrm, then everyone  can use it and develop it in same page, which is only my personal opinion. 😊

Regards,
David

> -----Original Message-----

> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Eric

> Anholt

> Sent: Friday, October 26, 2018 12:36 AM

> To: Sean Paul <sean@poorly.run>; Daniel Vetter <daniel.vetter@ffwll.ch>

> Cc: IGT development <igt-dev@lists.freedesktop.org>; Intel Graphics

> Development <intel-gfx@lists.freedesktop.org>; DRI Development <dri-

> devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org

> Subject: Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff

> mandatory?

> 

> Sean Paul <sean@poorly.run> writes:

> 

> > On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:

> >> Hi all,

> >>

> >> This is just to collect feedback on this idea, and see whether the

> >> overall dri-devel community stands on all this. I think the past few

> >> cross-vendor uapi extensions all came with igts attached, and

> >> personally I think there's lots of value in having them: A

> >> cross-vendor interface isn't useful if every driver implements it

> >> slightly differently.

> >>

> >> I think there's 2 questions here:

> >>

> >> - Do we want to make such testcases mandatory?

> >>

> >

> > Yes, more testing == better code.

> >

> >

> >> - If yes, are we there yet, or is there something crucially missing

> >>   still?

> >

> > In my experience, no. Last week while trying to replicate an intel-gfx

> > CI failure, I tried compiling igt for one of my (intel) chromebooks.

> > It seems like cross-compilation (or, in my case, just specifying

> > prefix/ld_library_path/sbin_path) is broken on igt. If we want to

> > impose restrictions across the entire subsystem, we need to make sure

> > that everyone can build and deploy igt easily.

> >

> > I managed to hack around everything and get it working, but I still

> > haven't tried switching out the toolchain. Once we have some GitLab CI

> > to validate cross-compilation, then we can consider making IGT mandatory.

> >

> > It's possible that I'm just a meson n00b and didn't use the right

> > incantation, so maybe it already works, but then we need better

> documentation.

> >

> > I've pasted my horrible hacks below, I also didn't have libunwind, so

> > removed its usage.

> 

> I've also had to cut out libunwind for cross-compiling on many occasions.

> Worst library.
Daniel Vetter Oct. 26, 2018, 8:32 a.m. 
On Fri, Oct 26, 2018 at 5:50 AM Zhou, David(ChunMing)
<David1.Zhou@amd.com> wrote:
>
> Make igt for cross-driver, I think you should rename it first, not an intel specific. NO company wants their employee working on other company stuff.
> You can rename it to DGT(drm graphics test), and published following  libdrm, or directly merge to libdrm, then everyone  can use it and develop it in same page, which is only my personal opinion.

We renamed it ot  IGT gpu tools, that was even enough for ARM folks.
If this is seriously what AMD expects before considering, I'm not sure
what to say.

Alex, Christian, is this the official AMD stance that you can't touch
stuff because of the letter i?
-Daniel


> Regards,
> David
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Eric
> > Anholt
> > Sent: Friday, October 26, 2018 12:36 AM
> > To: Sean Paul <sean@poorly.run>; Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: IGT development <igt-dev@lists.freedesktop.org>; Intel Graphics
> > Development <intel-gfx@lists.freedesktop.org>; DRI Development <dri-
> > devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
> > Subject: Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff
> > mandatory?
> >
> > Sean Paul <sean@poorly.run> writes:
> >
> > > On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> > >> Hi all,
> > >>
> > >> This is just to collect feedback on this idea, and see whether the
> > >> overall dri-devel community stands on all this. I think the past few
> > >> cross-vendor uapi extensions all came with igts attached, and
> > >> personally I think there's lots of value in having them: A
> > >> cross-vendor interface isn't useful if every driver implements it
> > >> slightly differently.
> > >>
> > >> I think there's 2 questions here:
> > >>
> > >> - Do we want to make such testcases mandatory?
> > >>
> > >
> > > Yes, more testing == better code.
> > >
> > >
> > >> - If yes, are we there yet, or is there something crucially missing
> > >>   still?
> > >
> > > In my experience, no. Last week while trying to replicate an intel-gfx
> > > CI failure, I tried compiling igt for one of my (intel) chromebooks.
> > > It seems like cross-compilation (or, in my case, just specifying
> > > prefix/ld_library_path/sbin_path) is broken on igt. If we want to
> > > impose restrictions across the entire subsystem, we need to make sure
> > > that everyone can build and deploy igt easily.
> > >
> > > I managed to hack around everything and get it working, but I still
> > > haven't tried switching out the toolchain. Once we have some GitLab CI
> > > to validate cross-compilation, then we can consider making IGT mandatory.
> > >
> > > It's possible that I'm just a meson n00b and didn't use the right
> > > incantation, so maybe it already works, but then we need better
> > documentation.
> > >
> > > I've pasted my horrible hacks below, I also didn't have libunwind, so
> > > removed its usage.
> >
> > I've also had to cut out libunwind for cross-compiling on many occasions.
> > Worst library.
Chunming Zhou Oct. 26, 2018, 8:48 a.m. 
On 2018年10月26日 16:32, Daniel Vetter wrote:
> On Fri, Oct 26, 2018 at 5:50 AM Zhou, David(ChunMing)
> <David1.Zhou@amd.com> wrote:
>> Make igt for cross-driver, I think you should rename it first, not an intel specific. NO company wants their employee working on other company stuff.
>> You can rename it to DGT(drm graphics test), and published following  libdrm, or directly merge to libdrm, then everyone  can use it and develop it in same page, which is only my personal opinion.
> We renamed it ot  IGT gpu tools, that was even enough for ARM folks.
> If this is seriously what AMD expects before considering, I'm not sure
> what to say.
>
> Alex, Christian, is this the official AMD stance that you can't touch
> stuff because of the letter i?
Nope, as I said last, this is just my personal thought. And I'm not sure 
what opinion of others.

-David
> -Daniel
>
>
>> Regards,
>> David
>>
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Eric
>>> Anholt
>>> Sent: Friday, October 26, 2018 12:36 AM
>>> To: Sean Paul <sean@poorly.run>; Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: IGT development <igt-dev@lists.freedesktop.org>; Intel Graphics
>>> Development <intel-gfx@lists.freedesktop.org>; DRI Development <dri-
>>> devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
>>> Subject: Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff
>>> mandatory?
>>>
>>> Sean Paul <sean@poorly.run> writes:
>>>
>>>> On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
>>>>> Hi all,
>>>>>
>>>>> This is just to collect feedback on this idea, and see whether the
>>>>> overall dri-devel community stands on all this. I think the past few
>>>>> cross-vendor uapi extensions all came with igts attached, and
>>>>> personally I think there's lots of value in having them: A
>>>>> cross-vendor interface isn't useful if every driver implements it
>>>>> slightly differently.
>>>>>
>>>>> I think there's 2 questions here:
>>>>>
>>>>> - Do we want to make such testcases mandatory?
>>>>>
>>>> Yes, more testing == better code.
>>>>
>>>>
>>>>> - If yes, are we there yet, or is there something crucially missing
>>>>>    still?
>>>> In my experience, no. Last week while trying to replicate an intel-gfx
>>>> CI failure, I tried compiling igt for one of my (intel) chromebooks.
>>>> It seems like cross-compilation (or, in my case, just specifying
>>>> prefix/ld_library_path/sbin_path) is broken on igt. If we want to
>>>> impose restrictions across the entire subsystem, we need to make sure
>>>> that everyone can build and deploy igt easily.
>>>>
>>>> I managed to hack around everything and get it working, but I still
>>>> haven't tried switching out the toolchain. Once we have some GitLab CI
>>>> to validate cross-compilation, then we can consider making IGT mandatory.
>>>>
>>>> It's possible that I'm just a meson n00b and didn't use the right
>>>> incantation, so maybe it already works, but then we need better
>>> documentation.
>>>> I've pasted my horrible hacks below, I also didn't have libunwind, so
>>>> removed its usage.
>>> I've also had to cut out libunwind for cross-compiling on many occasions.
>>> Worst library.
>
>
Alex Deucher Oct. 26, 2018, 2:41 p.m. 
On Fri, Oct 26, 2018 at 4:32 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Fri, Oct 26, 2018 at 5:50 AM Zhou, David(ChunMing)
> <David1.Zhou@amd.com> wrote:
> >
> > Make igt for cross-driver, I think you should rename it first, not an intel specific. NO company wants their employee working on other company stuff.
> > You can rename it to DGT(drm graphics test), and published following  libdrm, or directly merge to libdrm, then everyone  can use it and develop it in same page, which is only my personal opinion.
>
> We renamed it ot  IGT gpu tools, that was even enough for ARM folks.
> If this is seriously what AMD expects before considering, I'm not sure
> what to say.
>
> Alex, Christian, is this the official AMD stance that you can't touch
> stuff because of the letter i?

We don't have any restrictions.

Alex

> -Daniel
>
>
> > Regards,
> > David
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Eric
> > > Anholt
> > > Sent: Friday, October 26, 2018 12:36 AM
> > > To: Sean Paul <sean@poorly.run>; Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: IGT development <igt-dev@lists.freedesktop.org>; Intel Graphics
> > > Development <intel-gfx@lists.freedesktop.org>; DRI Development <dri-
> > > devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
> > > Subject: Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff
> > > mandatory?
> > >
> > > Sean Paul <sean@poorly.run> writes:
> > >
> > > > On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> > > >> Hi all,
> > > >>
> > > >> This is just to collect feedback on this idea, and see whether the
> > > >> overall dri-devel community stands on all this. I think the past few
> > > >> cross-vendor uapi extensions all came with igts attached, and
> > > >> personally I think there's lots of value in having them: A
> > > >> cross-vendor interface isn't useful if every driver implements it
> > > >> slightly differently.
> > > >>
> > > >> I think there's 2 questions here:
> > > >>
> > > >> - Do we want to make such testcases mandatory?
> > > >>
> > > >
> > > > Yes, more testing == better code.
> > > >
> > > >
> > > >> - If yes, are we there yet, or is there something crucially missing
> > > >>   still?
> > > >
> > > > In my experience, no. Last week while trying to replicate an intel-gfx
> > > > CI failure, I tried compiling igt for one of my (intel) chromebooks.
> > > > It seems like cross-compilation (or, in my case, just specifying
> > > > prefix/ld_library_path/sbin_path) is broken on igt. If we want to
> > > > impose restrictions across the entire subsystem, we need to make sure
> > > > that everyone can build and deploy igt easily.
> > > >
> > > > I managed to hack around everything and get it working, but I still
> > > > haven't tried switching out the toolchain. Once we have some GitLab CI
> > > > to validate cross-compilation, then we can consider making IGT mandatory.
> > > >
> > > > It's possible that I'm just a meson n00b and didn't use the right
> > > > incantation, so maybe it already works, but then we need better
> > > documentation.
> > > >
> > > > I've pasted my horrible hacks below, I also didn't have libunwind, so
> > > > removed its usage.
> > >
> > > I've also had to cut out libunwind for cross-compiling on many occasions.
> > > Worst library.
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx