Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

["Vasilev, Oleg" <oleg.vasilev@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7370 bytes --]

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 <rodrigosiqueiramelo@gmail.com>
> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > > > > ---
> > > > > >  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

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3261 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev