On Thu, 2019-07-11 at 19:19 +0200, Daniel Vetter wrote: > On Thu, Jul 11, 2019 at 03:20:52PM +0000, Ser, Simon wrote: > > On Thu, 2019-07-11 at 14:36 +0200, Daniel Vetter wrote: > > > On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg < > > > oleg.vasilev@intel.com> wrote: > > > > On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote: > > > > > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote: > > > > > > Currently, we have different sets of prime tests: > > > > > > - vgem+i915 > > > > > > - amdgpu+i915 > > > > > > - nv+i915 > > > > > > > > > > > > Those tests use vendor-specific ioctls, therefore, not > > > > > > interchangeable. > > > > > > The idea is to create a set of tests which are expected to > > > > > > work on > > > > > > any > > > > > > prime-compatible driver. It can be run with any combination > > > > > > of > > > > > > exporter+importer, where > > > > > > > > > > > > The first test is simple: > > > > > > 1. Exporter creates a framebuffer and fills it with a plain > > > > > > color > > > > > > 2. Fb is transferred to the importer > > > > > > 3. Importer modesets and computes pipe CRC > > > > > > 4. Importer draws the same color through cairo and compares > > > > > > CRC > > > > > > > > > > > > The initial motivation comes from the need to test prime > > > > > > support in > > > > > > vkms. > > > > > > > > > > > > Cc: Rodrigo Siqueira > > > > > > Cc: Daniel Vetter > > > > > > Signed-off-by: Oleg Vasilev > > > > > > --- > > > > > > tests/meson.build | 1 + > > > > > > tests/prime_generic.c | 238 > > > > > > ++++++++++++++++++++++++++++++++++++++++++ > > > > > > 2 files changed, 239 insertions(+) > > > > > > create mode 100644 tests/prime_generic.c > > > > > > > > > > > > diff --git a/tests/meson.build b/tests/meson.build > > > > > > index 34a74025..1c938e95 100644 > > > > > > --- a/tests/meson.build > > > > > > +++ b/tests/meson.build > > > > > > @@ -76,6 +76,7 @@ test_progs = [ > > > > > > 'prime_self_import', > > > > > > 'prime_udl', > > > > > > 'prime_vgem', > > > > > > + 'prime_generic', > > > > > > 'syncobj_basic', > > > > > > 'syncobj_wait', > > > > > > 'template', > > > > > > + > > > > > > +igt_main > > > > > > +{ > > > > > > + igt_fixture { > > > > > > + kmstest_set_vt_graphics_mode(); > > > > > > + } > > > > > > + igt_subtest("crc-vgem-vkms") > > > > > > + run_test_crc(DRIVER_VGEM, DRIVER_VKMS); > > > > > > + igt_subtest("crc-i915-vkms") > > > > > > + run_test_crc(DRIVER_INTEL, DRIVER_VKMS); > > > > > > + igt_subtest("crc-vgem-i915") > > > > > > + run_test_crc(DRIVER_VGEM, DRIVER_INTEL); > > > > > > + igt_subtest("crc-amd-i915") > > > > > > + run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL); > > > > > > + igt_subtest("crc-i915-amd") > > > > > > + run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU); > > > > > > > > > > This isn't any more generic than what we have e.g. in > > > > > prime_vgem > > > > > already. > > > > > > > > Here non-generic part is only those 10 lines of subtests > > > > definitions. > > > > The rest is common code. > > > > > > > > I guess, this separate subtest definitions can document the > > > > device > > > > pairs the test is expected to work + we can blacklist tests > > > > which we > > > > don't have the hardware for. > > > > > > But we already have tests for specific pairs, which can also test > > > specific stuff. In general we don't do tests with -gen9, gen11, > > > -gen8 > > > prefixes either. This here would be a first I'd say. > > > > > > > > The way to do a generic kms testcase is to use DRIVER_ANY > > > > > (yes some > > > > > of the > > > > > earlier conversion unfortunately used stuff like DRIVER_INTEL > > > > > | > > > > > DRIVER_AMDGPU). With that you also don't need your library > > > > > prep > > > > > patch, > > > > > because no need for a DRIVER_VKMS. > > > > > > > > But how can we open two different devices with DRIVER_ANY? If I > > > > put > > > > > > > > fd = drm_open_driver(DRIVER_ANY) > > > > fd2 = drm_open_driver(DRIVER_ANY) > > > > > > > > I get the same device opened two times, do I? > > > > > > One of them would be DRIVER_VGEM. And I think we already make > > > sure > > > that you don't accidentally get vgem for DRIVER_ANY. Otherwise I > > > guess > > > we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is > > > DRIVER_ANY_KMS here, which would also exclude vgem. So if it > > > doesn't > > > work already, small patch needed in lib/, but DRIVER_VKMS also > > > needs > > > that. And I really don't want more DRIVER_FOO defines for a plain > > > old > > > kms-only driver. kms is supposed to be a shared cross-driver > > > standard, > > > if we can't even write shared testcases it's just epic fail :-) > > > > > > > > Imo best to start out by converting prime_vgem.c for this, > > > > > and adding > > > > > for > > > > > the i915 specific tests (but _only_ for those) a specific "is > > > > > this > > > > > i915" > > > > > check, so we skip those tests. And replace DRIVER_INTEL by > > > > > DRIVER_ANY. > > > > > Well, anything except vgem :-) > > > > > > > > If I am not mistaken, all tests in prime_vgem are intel- > > > > specific :D > > > > > > > > Some ideas can be taken from it, but anyway, it probably needs > > > > a deep > > > > rewrite. > > > > > > > > And prime_vgem use only vgem as an exporter device. Here we can > > > > use any > > > > device. Sure, some tests would eventually use vgem-specific > > > > features, > > > > but some could remain more generic. > > > > > > Maybe I wasn't all that clear then, I think what we want to test > > > in a > > > generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test > > > the > > > render side using DRIVER_FOO+vgem. This way you only need to have > > > 1 > > > generic kms test, plus N tests for all the rendering drivers. > > > Instead > > > of M x N testcases like you're proposing here. vgem is just the > > > abstraction layer/mocking layer we use to separate these two > > > worlds. > > > And of course for specific sharing you can still do that, e.g. to > > > test > > > dma engine compatibility between amdgpu and i915, or nouveau and > > > i915. > > > But those tests don't need to involve kms. > > > > > > And yeah prime_vgem might need to be split into prime_i915_vgem > > > and > > > prime_kms_vgem. But all the display related testcases should be > > > possible to extract into such a prime_kms_vgem framework. > > > > One could also want to test drmPrimeHandleToFD and > > drmPrimeFDToHandle > > between e.g. amdgpu and i915. I guess it's using a different code > > path > > than gem. > > Yeah, but only if you also do some i915 or amdgpu specific command > submission. Otherwise if it's all cpu-only, then you wont hit any > additional path than what you can test with vgem already. Kinda my > point > really, there's no room really for a "generic" i915+amdgpu prime > test. As > soon as you look at that combo, it's all about specifics. > -Daniel Ok, now I got the point, thanks for the explanation. I will send an updated patch. Oleg