Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Fri, Oct 19, 2018 at 4:51 AM 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? > > - If yes, are we there yet, or is there something crucially missing > still? > > And of course there's a bunch of details to figure out. Like we > probably want to also recommend the selftests/unit-tests in > drivers/gpu/drm/selftest, since fairly often that's a much more > effective approach to checking the details than from userspace. > > Feedback and thoughts very much appreciated. As long as we can fix up any cross platform issues, this seems reasonable to me. Alex > > Cheers, Daniel > --- > Documentation/gpu/drm-uapi.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 4b4bf2c5eac5..91cf6e4b6303 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly > unintuitive meaning of > Testing and validation > == > > +Testing Requirements for userspace API > +-- > + > +New cross-driver userspace interface extensions, like new IOCTL, new KMS > +properties, new files in sysfs or anything else that constitutes an API > change > +need to have driver-agnostic testcases in IGT for that feature. > + > Validating changes with IGT > --- > > -- > 2.19.1 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
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
Date: Thu, 25 Oct 2018 08:40:28 -0400
Subject: [PATCH] igt: Hacks to compile in CrOS chroot
Signed-off-by: Sean Paul
---
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(-)
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
#include
#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();
-
- 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();
- unw_init_local(, );
- while (unw_step() > 0) {
- char name[255];
- unw_word_t off, ip;
- Dwfl_Module *mod = NULL;
-
- unw_get_reg(, UNW_REG_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, , NULL);
-
- if (line && dwfl_name) {
- src = dwfl_lineinfo(line, NULL, , NULL,
NULL, NULL);
- igt_info(" #%d %s:%d %s()\n", stack_num++,
src, lineno, dwfl_name);
- continue;
- }
- }
-
- if (unw_get_proc_name(, name, 255, ) < 0)
- igt_info(" #%d [+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();
- unw_init_local(, );
- while (unw_step() > 0) {
- char name[255];
- unw_word_t off;
-
- if (unw_get_proc_name(, name, 255, ) < 0)
- xstrlcpy(name, "", 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
Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Thu, Oct 25, 2018 at 11:58 AM Liviu Dudau wrote: > On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote: > > Hi all, > > Hi, > > (Replying from my personal address as the work email seems to have let > this one go to /dev/null) > > > > > 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 would like to also get some clarification on where we are standing on > "tests vs 'real code'" stanza. Does making igt tests mandatory replace > the need for 'real code' or does it add to the list of requirements? If > the later, then I think the bar rises in terms of showing igts' > usefulness / benefits. It would be in addition. The "real code" userspace is for validating the overall design. Igt (and unit tests) are more for checking all the details, error handling, and having a regression test suite going forward. > > I think there's 2 questions here: > > > > - Do we want to make such testcases mandatory? > > I'm a bit reluctant to make it so by fiat. I think that showing the > usefulness of having igts tests to newcomers (by adding with this patch > some more information about why IGT is a good place to add your testing to) > and getting more mature drivers to get tested under IGT on a regular basis > would make adoption of IGT for testing a community standard. There's 2 motivations here for me: - Most of the generic features in the past 1-2 did come with igts, but sometimes those igts have been treated as 2nd class and forgotten. I think it's at least the right time to discuss whether we should make them an integral part of uapi development. - The other bit is that our intel CI is catching a lot of regressions, and people have started to cc: intel-gfx to get their non-intel patches validated too. So all these igts we have do seem to provide real value in keeping things working. > > - If yes, are we there yet, or is there something crucially missing > > still? > > I'm just getting back into IGT by refreshing the writeback patches, but > by looking at the commit log I get the impression that there aren't that > many patches that target drivers other than Intel's. Are all the non-Intel > patches so generic that one doesn't need to specify a target driver for > those changes (in which case great, but then why is Intel's so different?), > or are the others not bothered with IGT support? So this discussion isn't about igt overall, but just about cross-driver features. The tests we have in igt for intel are a lot more: - We also validate all the intel-specific bits with igts. - We validate all the interactions between generic stuff and intel-specific uapi with igts. - We validate hw features with igt. And finally we've made igts mandatory for all things intel more than 5 years ago. So there's considerable head start. The other side is that I'm only suggesting that we make igts mandatory for cross-driver interfaces. Naturally the tests for those aren't aimed at a specific driver, but use the DRIVER_ANY flag. And the work needed to make that work on a specific driver is usually rather small - e.g. vmwgfx (which doesn't support GEM at all) required very small changes to get things going. Now we do have some tests for other drivers (vc4 and a few amdgpu), but not anything else. So I'd say the lack of non-intel targetted patches is a good thing. > At the moment I'm a bit on the fence on this. Not having spent too much > time with IGT in the last 6 months, I'm probably closer to a newcomer in > my attitude towards IGT and at the moment I'm not clear on how to answer the > "Why?" and "What is in it for me?" questions. Well all the usual reasons for testing: - Only way to make sure different implementations are working correct. - Only real way to make sure regressions are caught before everything is on fire. Of course this gets a lot better if there's also CI running them (which we do for intel, and are open to anyone submitting stuff to it), but just having the tests is already a big step. Cheers, Daniel > Best regards, > Liviu > > > > > And of course there's a bunch of details to figure out. Like we > > probably want to also recommend the selftests/unit-tests in > > drivers/gpu/drm/selftest, since fairly often that's a much more > > effective approach to checking the details than from userspace. > > > > Feedback and thoughts very much appreciated. > > > > Cheers, Daniel > > --- > > Documentation/gpu/drm-uapi.rst | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > > index 4b4bf2c5eac5..91cf6e4b6303 100644 > > --- a/Documentation/gpu/drm-uapi.rst > > +++ b/Documentation/gpu/drm-uapi.rst > > @@ -238,6
Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> Hi all,
Hi,
(Replying from my personal address as the work email seems to have let
this one go to /dev/null)
>
> 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 would like to also get some clarification on where we are standing on
"tests vs 'real code'" stanza. Does making igt tests mandatory replace
the need for 'real code' or does it add to the list of requirements? If
the later, then I think the bar rises in terms of showing igts'
usefulness / benefits.
>
> I think there's 2 questions here:
>
> - Do we want to make such testcases mandatory?
I'm a bit reluctant to make it so by fiat. I think that showing the
usefulness of having igts tests to newcomers (by adding with this patch
some more information about why IGT is a good place to add your testing to)
and getting more mature drivers to get tested under IGT on a regular basis
would make adoption of IGT for testing a community standard.
>
> - If yes, are we there yet, or is there something crucially missing
> still?
I'm just getting back into IGT by refreshing the writeback patches, but
by looking at the commit log I get the impression that there aren't that
many patches that target drivers other than Intel's. Are all the non-Intel
patches so generic that one doesn't need to specify a target driver for
those changes (in which case great, but then why is Intel's so different?),
or are the others not bothered with IGT support?
At the moment I'm a bit on the fence on this. Not having spent too much
time with IGT in the last 6 months, I'm probably closer to a newcomer in
my attitude towards IGT and at the moment I'm not clear on how to answer the
"Why?" and "What is in it for me?" questions.
Best regards,
Liviu
>
> And of course there's a bunch of details to figure out. Like we
> probably want to also recommend the selftests/unit-tests in
> drivers/gpu/drm/selftest, since fairly often that's a much more
> effective approach to checking the details than from userspace.
>
> Feedback and thoughts very much appreciated.
>
> Cheers, Daniel
> ---
> Documentation/gpu/drm-uapi.rst | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 4b4bf2c5eac5..91cf6e4b6303 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly
> unintuitive meaning of
> Testing and validation
> ==
>
> +Testing Requirements for userspace API
> +--
> +
> +New cross-driver userspace interface extensions, like new IOCTL, new KMS
> +properties, new files in sysfs or anything else that constitutes an API
> change
> +need to have driver-agnostic testcases in IGT for that feature.
> +
> Validating changes with IGT
> ---
>
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
/`\
/ : |
_.._ | '/
/`\| /
| .-._ '-"` (
|_/ / o o\
| == () ==
\ -- / __
/ ---<_ |
|___
| \\ \ | I would like to fix the world but |
/
| | \\__ \ | no one gives me the source code. |
/
/ ; |.__) / |__|
\
(_/.-. ; /__)
(_\
{ `| \_/
'-\ / |
| / |
/ \ '-.
\__|-'
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
