On Mon, Feb 18, 2019 at 02:08:12PM +0000, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> > On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> > > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > > > of all driver tests, so move it into the i915/ stable.
> > > > > >
> > > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
> > > > > > Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
> > > > > > Acked-by: Petri Latvala <petri.latvala@xxxxxxxxx>
> > > > > > ---
> > > > > > tests/Makefile.sources | 5 ++++-
> > > > > > tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > > > tests/meson.build | 2 +-
> > > > > > 3 files changed, 5 insertions(+), 2 deletions(-)
> > > > > > rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > >
> > > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > > --- a/tests/Makefile.sources
> > > > > > +++ b/tests/Makefile.sources
> > > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > > > kms_dp_dsc \
> > > > > > kms_draw_crc \
> > > > > > kms_fbcon_fbt \
> > > > > > - kms_fence_pin_leak \
> > > > > > kms_flip \
> > > > > > kms_flip_event_leak \
> > > > > > kms_flip_tiling \
> > > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > > > vgem_slow \
> > > > > > $(NULL)
> > > > > >
> > > > > > +TESTS_progs += \
> > > > > > + i915/kms_fence_pin_leak \
> > > > > > + $(NULL)
> > > > >
> > > > > This just moves it around, so we will end up having binary named
> > > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > >
> > > > > That still will install as
> > > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > >
> > > > > If you want to prefix it:
> > > > > TESTS_progs += i915_kms_fence_pin_leak
> > > > > i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > >
> > > > > Oterwise:
> > > > > TESTS_progs += kms_fence_pin_leak
> > > > > kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > >
> > > > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > > > similarity index 100%
> > > > > > rename from tests/kms_fence_pin_leak.c
> > > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index ec980651a..08e55b9c0 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > > > 'kms_dp_dsc',
> > > > > > 'kms_draw_crc',
> > > > > > 'kms_fbcon_fbt',
> > > > > > - 'kms_fence_pin_leak',
> > > > > > 'kms_flip',
> > > > > > 'kms_flip_event_leak',
> > > > > > 'kms_flip_tiling',
> > > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > > > 'fb_tiling',
> > > > > > 'getparams_basic',
> > > > > > 'hangman',
> > > > > > + 'kms_fence_pin_leak',
> > > > > > 'missed_irq',
> > > > > > 'module_load',
> > > > > > 'query',
> > > > >
> > > > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > > > top of i915_progs just to be more explicit.
> > > > >
> > > > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > > > yet?
> > > >
> > > > I'd rather not have tests renamed. That's my personal preference. Either
> > > > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > > > or it should be installed under tests/i915/.
> >
> > You mean you want the binaries to be a straight s/\.c//?
>
> Yes. Or that we don't report test names at all, but the path to the
> source of the test.
>
> > I personally like the current way mostly because we avoid 'i915/i915_'
> > redundancy and the resulting binaries dir is flat, which makes them
> > PATH-friendly.
>
> They should not be in the PATH!!!!
>
> There doesn't need to be any i915/i915_ redundancy as we then have i915/
> in the name.
That's why its libexec after installing, but I admit to committing the
crime of adding $SRC/build/tests to PATH at times, for convenience when
working on tests.
I think that we are pretty far from being able to handle
igt@tata/toto@subtoto in the CI (quotation needed, Petri?)
and i915/i915_ would be overall less messy of the two.
> > The way we handle gem_ adds a little inconsistency, but i915_gem_ would
> > be too mouthful.
> >
> > But if what you are proposing is really essential we can stir the pot
> > some more and i915_ all the .c(s).
> >
> > > As a case in point, the renaming of benchmarks by meson breaks
> > > scripts. Please do not do that.
> >
> > I lack the context here. Do we have any inconsistencies in how autotools
> > and meson generate binaries? Which scripts are getting broken?
>
> Yes, meson adds _bench but all of our scripts in igt expect the
> existing names. That may only be 15 different scripts...
> -Chris
Oh. Let me check what we can do about it. I'll make it consisten one way or
the other :-)
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
