[igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

Compared to the RFC[1] no changes to the patch itself, but igt moved
forward a lot:

- gitlab CI builds with: reduced configs/libraries, arm cross build
  and a sysroot build (should address all the build/cross platform
  concerns raised in the RFC discussions).

- tests reorganized into subdirectories so that the i915-gem tests
  don't clog the main/shared tests directory anymore

- quite a few more non-intel people contributing/reviewing/committing
  igt tests patches.

I think this addresses all the concerns raised in the RFC discussions,
and assuming there's enough Acks and no new issues that pop up, we can
go ahead with this.

1: https://patchwork.kernel.org/patch/10648851/
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Eric Anholt <eric@anholt.net>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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 a752aa561ea4..413915d6b7d2 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.20.1

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Eric Anholt]

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

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Compared to the RFC[1] no changes to the patch itself, but igt moved
> forward a lot:
>
> - gitlab CI builds with: reduced configs/libraries, arm cross build
>   and a sysroot build (should address all the build/cross platform
>   concerns raised in the RFC discussions).
>
> - tests reorganized into subdirectories so that the i915-gem tests
>   don't clog the main/shared tests directory anymore
>
> - quite a few more non-intel people contributing/reviewing/committing
>   igt tests patches.
>
> I think this addresses all the concerns raised in the RFC discussions,
> and assuming there's enough Acks and no new issues that pop up, we can
> go ahead with this.
>
> 1: https://patchwork.kernel.org/patch/10648851/
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

igt is a bit awkward to work in (the mailing list is very noisy with the
Intel CI being email-based instead of gitlab-based and most of the
traffic being Intel), but it's the right place to be putting shared
tests and hopefully that pain point goes away eventually using gitlab
MRs.

I think there are going to be some interesting questions on how to deal
with things like KMS properties that aren't amenable to
chamelium/writeback-based testing.  However, we should default to
requiring tests and only skip that when we agree collectively that
something isn't testable currently.

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 16, 2019 at 11:41 PM Eric Anholt <eric@anholt.net> wrote:
>
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>
> > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > forward a lot:
> >
> > - gitlab CI builds with: reduced configs/libraries, arm cross build
> >   and a sysroot build (should address all the build/cross platform
> >   concerns raised in the RFC discussions).
> >
> > - tests reorganized into subdirectories so that the i915-gem tests
> >   don't clog the main/shared tests directory anymore
> >
> > - quite a few more non-intel people contributing/reviewing/committing
> >   igt tests patches.
> >
> > I think this addresses all the concerns raised in the RFC discussions,
> > and assuming there's enough Acks and no new issues that pop up, we can
> > go ahead with this.
> >
> > 1: https://patchwork.kernel.org/patch/10648851/
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> igt is a bit awkward to work in (the mailing list is very noisy with the
> Intel CI being email-based instead of gitlab-based and most of the
> traffic being Intel), but it's the right place to be putting shared
> tests and hopefully that pain point goes away eventually using gitlab
> MRs.

I have a filter to mark all CI mails as read, to avoid the spam a bit.
And then check it only when I merge a series. But yeah, it's pain.

> I think there are going to be some interesting questions on how to deal
> with things like KMS properties that aren't amenable to
> chamelium/writeback-based testing.  However, we should default to
> requiring tests and only skip that when we agree collectively that
> something isn't testable currently.

Should I add a sentence clarifying this? We don't want to block
features on a new unproven/unwritten testing approach (e.g. "write an
entire vkms to test-drive that feature"), that doesn't make sense.
-Daniel

>
> Reviewed-by: Eric Anholt <eric@anholt.net>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Arkadiusz Hiler]

On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> Compared to the RFC[1] no changes to the patch itself, but igt moved
> forward a lot:
> 
> - gitlab CI builds with: reduced configs/libraries, arm cross build
>   and a sysroot build (should address all the build/cross platform
>   concerns raised in the RFC discussions).
> 
> - tests reorganized into subdirectories so that the i915-gem tests
>   don't clog the main/shared tests directory anymore
> 
> - quite a few more non-intel people contributing/reviewing/committing
>   igt tests patches.
> 
> I think this addresses all the concerns raised in the RFC discussions,
> and assuming there's enough Acks and no new issues that pop up, we can
> go ahead with this.
> 
> 1: https://patchwork.kernel.org/patch/10648851/
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Thanks for the trust! I hope we will serve the subsystem well.

I am happy about the influx of people from outside of i915 that we have
got in 2018 and I hope it will only get better this year.

There are still some cleanup tasks left when it comes driver-specific
test compartmentalization, but we are getting there :-)

Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Petri Latvala]

On Thu, Jan 17, 2019 at 01:01:00PM +0200, Arkadiusz Hiler wrote:
> On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > forward a lot:
> > 
> > - gitlab CI builds with: reduced configs/libraries, arm cross build
> >   and a sysroot build (should address all the build/cross platform
> >   concerns raised in the RFC discussions).
> > 
> > - tests reorganized into subdirectories so that the i915-gem tests
> >   don't clog the main/shared tests directory anymore
> > 
> > - quite a few more non-intel people contributing/reviewing/committing
> >   igt tests patches.
> > 
> > I think this addresses all the concerns raised in the RFC discussions,
> > and assuming there's enough Acks and no new issues that pop up, we can
> > go ahead with this.
> > 
> > 1: https://patchwork.kernel.org/patch/10648851/
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Thanks for the trust! I hope we will serve the subsystem well.
> 
> I am happy about the influx of people from outside of i915 that we have
> got in 2018 and I hope it will only get better this year.
> 
> There are still some cleanup tasks left when it comes driver-specific
> test compartmentalization, but we are getting there :-)
> 
> Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>


+1


Acked-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Liviu Dudau]

On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> Compared to the RFC[1] no changes to the patch itself, but igt moved
> forward a lot:
> 
> - gitlab CI builds with: reduced configs/libraries, arm cross build
>   and a sysroot build (should address all the build/cross platform
>   concerns raised in the RFC discussions).
> 
> - tests reorganized into subdirectories so that the i915-gem tests
>   don't clog the main/shared tests directory anymore
> 
> - quite a few more non-intel people contributing/reviewing/committing
>   igt tests patches.
> 
> I think this addresses all the concerns raised in the RFC discussions,
> and assuming there's enough Acks and no new issues that pop up, we can
> go ahead with this.
> 
> 1: https://patchwork.kernel.org/patch/10648851/
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  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 a752aa561ea4..413915d6b7d2 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.

From an aspirational point of view I am fine with this and you can have
my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.

From a practical point of view I would like to see a matrix of KMS APIs
that are being validated and the drivers that have been tested. Otherwise, 
the next person that comes and tries to add a new IOCTL, KMS property or new
file in sysfs is going to discover that he has subscribed to a much bigger
task of getting enough KMS drivers testable in the first place.

Best regards,
Liviu


> +
>  Validating changes with IGT
>  ---------------------------
>  
> -- 
> 2.20.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > forward a lot:
> >
> > - gitlab CI builds with: reduced configs/libraries, arm cross build
> >   and a sysroot build (should address all the build/cross platform
> >   concerns raised in the RFC discussions).
> >
> > - tests reorganized into subdirectories so that the i915-gem tests
> >   don't clog the main/shared tests directory anymore
> >
> > - quite a few more non-intel people contributing/reviewing/committing
> >   igt tests patches.
> >
> > I think this addresses all the concerns raised in the RFC discussions,
> > and assuming there's enough Acks and no new issues that pop up, we can
> > go ahead with this.
> >
> > 1: https://patchwork.kernel.org/patch/10648851/
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  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 a752aa561ea4..413915d6b7d2 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.
>
> From an aspirational point of view I am fine with this and you can have
> my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
>
> From a practical point of view I would like to see a matrix of KMS APIs
> that are being validated and the drivers that have been tested. Otherwise,
> the next person that comes and tries to add a new IOCTL, KMS property or new
> file in sysfs is going to discover that he has subscribed to a much bigger
> task of getting enough KMS drivers testable in the first place.

This is what the _new_ features is about, no expectation to write
tests for all the existing stuff. Although I think there's not really
any big gaps in igt anymore, we do have at least some (rather rough
and coarse in some case) test coverage for everything I think. Should
this be clarified further?
-Daniel

>
> Best regards,
> Liviu
>
>
> > +
> >  Validating changes with IGT
> >  ---------------------------
> >
> > --
> > 2.20.1
> >
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Liviu Dudau]

On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > forward a lot:
> > >
> > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >   and a sysroot build (should address all the build/cross platform
> > >   concerns raised in the RFC discussions).
> > >
> > > - tests reorganized into subdirectories so that the i915-gem tests
> > >   don't clog the main/shared tests directory anymore
> > >
> > > - quite a few more non-intel people contributing/reviewing/committing
> > >   igt tests patches.
> > >
> > > I think this addresses all the concerns raised in the RFC discussions,
> > > and assuming there's enough Acks and no new issues that pop up, we can
> > > go ahead with this.
> > >
> > > 1: https://patchwork.kernel.org/patch/10648851/
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > Cc: Sean Paul <sean@poorly.run>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  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 a752aa561ea4..413915d6b7d2 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.
> >
> > From an aspirational point of view I am fine with this and you can have
> > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> >
> > From a practical point of view I would like to see a matrix of KMS APIs
> > that are being validated and the drivers that have been tested. Otherwise,
> > the next person that comes and tries to add a new IOCTL, KMS property or new
> > file in sysfs is going to discover that he has subscribed to a much bigger
> > task of getting enough KMS drivers testable in the first place.
> 
> This is what the _new_ features is about, no expectation to write
> tests for all the existing stuff.

Yeah, but if the "tests for all the existing stuff" doesn't exist, your
_new_ feature tests are not going to mean much, doesn't it?

> Although I think there's not really
> any big gaps in igt anymore, we do have at least some (rather rough
> and coarse in some case) test coverage for everything I think.

I would like to see some proof of that in the form of ....


> Should this be clarified further?

an URL that points to a page similar to Mesa's GL supported features
would be nice to add here, from my point of view.

Best regards,
Liviu

> -Daniel
> 
> >
> > Best regards,
> > Liviu
> >
> >
> > > +
> > >  Validating changes with IGT
> > >  ---------------------------
> > >
> > > --
> > > 2.20.1
> > >
> >
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > >
> > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > forward a lot:
> > > >
> > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > >   and a sysroot build (should address all the build/cross platform
> > > >   concerns raised in the RFC discussions).
> > > >
> > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > >   don't clog the main/shared tests directory anymore
> > > >
> > > > - quite a few more non-intel people contributing/reviewing/committing
> > > >   igt tests patches.
> > > >
> > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > go ahead with this.
> > > >
> > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > Cc: Sean Paul <sean@poorly.run>
> > > > Cc: Eric Anholt <eric@anholt.net>
> > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > >  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 a752aa561ea4..413915d6b7d2 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.
> > >
> > > From an aspirational point of view I am fine with this and you can have
> > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > >
> > > From a practical point of view I would like to see a matrix of KMS APIs
> > > that are being validated and the drivers that have been tested. Otherwise,
> > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > task of getting enough KMS drivers testable in the first place.
> >
> > This is what the _new_ features is about, no expectation to write
> > tests for all the existing stuff.
>
> Yeah, but if the "tests for all the existing stuff" doesn't exist, your
> _new_ feature tests are not going to mean much, doesn't it?

Why? There's still good test coverage for the new stuff at least. And
eventually someone gets bored and fills in the gaps. This is how we've
created all the current igt testcases, so it works - mandatory igt for
new features is the rule for anything intel does since well over 5
years.

> > Although I think there's not really
> > any big gaps in igt anymore, we do have at least some (rather rough
> > and coarse in some case) test coverage for everything I think.
>
> I would like to see some proof of that in the form of ....
>
>
> > Should this be clarified further?
>
> an URL that points to a page similar to Mesa's GL supported features
> would be nice to add here, from my point of view.

There isn't even a list of upstream drm features ... how exactly
should that look like? Only thing I can tell you is that we had huge
amounts of todo for missing igt tests, and afaik we've worked them all
down. We = intel here. It's not perfect, but good enough that we've
essentially stopped doing any validation except by running igt. Of
course you can always write more tests, same way you can always
bikeshed a patch a few more times. Eventually there's dimishing
returns.
-Daniel

> Best regards,
> Liviu
>
> > -Daniel
> >
> > >
> > > Best regards,
> > > Liviu
> > >
> > >
> > > > +
> > > >  Validating changes with IGT
> > > >  ---------------------------
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Liviu Dudau]

On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > >
> > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > forward a lot:
> > > > >
> > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > >   and a sysroot build (should address all the build/cross platform
> > > > >   concerns raised in the RFC discussions).
> > > > >
> > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > >   don't clog the main/shared tests directory anymore
> > > > >
> > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > >   igt tests patches.
> > > > >
> > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > go ahead with this.
> > > > >
> > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > ---
> > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > >
> > > > From an aspirational point of view I am fine with this and you can have
> > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > >
> > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > task of getting enough KMS drivers testable in the first place.
> > >
> > > This is what the _new_ features is about, no expectation to write
> > > tests for all the existing stuff.
> >
> > Yeah, but if the "tests for all the existing stuff" doesn't exist, your
> > _new_ feature tests are not going to mean much, doesn't it?
> 
> Why? There's still good test coverage for the new stuff at least. And
> eventually someone gets bored and fills in the gaps. This is how we've
> created all the current igt testcases, so it works - mandatory igt for
> new features is the rule for anything intel does since well over 5
> years.

Because if you can't test basic stuff with igt how did you test your new
feature? What I'm saying is: if developer X is having to use igt for
a new feature then he is either: a) expecting that basic testing works for
his driver (or he has bootstrapping isssues) or that b) if the feature
doesn't get tested on driver A because that driver doesn't even have basic
tests passing for it then the feature is acceptable. And it will be helpful
to publish the list of igt tests that pass for each driver in KMS.

Alternatively, you need to explain better what "driver-agnostic testcase" is
(my reading: a testcase that can be run on any driver). Or point to some igt
testcases that are considered driver-agnostic and known to run on different
(read: non-x86) platforms.

> 
> > > Although I think there's not really
> > > any big gaps in igt anymore, we do have at least some (rather rough
> > > and coarse in some case) test coverage for everything I think.
> >
> > I would like to see some proof of that in the form of ....
> >
> >
> > > Should this be clarified further?
> >
> > an URL that points to a page similar to Mesa's GL supported features
> > would be nice to add here, from my point of view.
> 
> There isn't even a list of upstream drm features ...

Yeah, I know that! :)

> how exactly should that look like?

What about a list of igt tests then? OR we could put in the igt TODO file
the kind of tests we would like to have at some moment?

Best regards,
Liviu


> Only thing I can tell you is that we had huge
> amounts of todo for missing igt tests, and afaik we've worked them all
> down. We = intel here. It's not perfect, but good enough that we've
> essentially stopped doing any validation except by running igt. Of
> course you can always write more tests, same way you can always
> bikeshed a patch a few more times. Eventually there's dimishing
> returns.
> -Daniel
> 
> > Best regards,
> > Liviu
> >
> > > -Daniel
> > >
> > > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > >
> > > > > +
> > > > >  Validating changes with IGT
> > > > >  ---------------------------
> > > > >
> > > > > --
> > > > > 2.20.1
> > > > >
> > > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> > --
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Thu, Jan 17, 2019 at 3:54 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > >
> > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > > >
> > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > > forward a lot:
> > > > > >
> > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > > >   and a sysroot build (should address all the build/cross platform
> > > > > >   concerns raised in the RFC discussions).
> > > > > >
> > > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > > >   don't clog the main/shared tests directory anymore
> > > > > >
> > > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > > >   igt tests patches.
> > > > > >
> > > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > > go ahead with this.
> > > > > >
> > > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > > ---
> > > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > > >
> > > > > From an aspirational point of view I am fine with this and you can have
> > > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > > >
> > > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > > task of getting enough KMS drivers testable in the first place.
> > > >
> > > > This is what the _new_ features is about, no expectation to write
> > > > tests for all the existing stuff.
> > >
> > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your
> > > _new_ feature tests are not going to mean much, doesn't it?
> >
> > Why? There's still good test coverage for the new stuff at least. And
> > eventually someone gets bored and fills in the gaps. This is how we've
> > created all the current igt testcases, so it works - mandatory igt for
> > new features is the rule for anything intel does since well over 5
> > years.
>
> Because if you can't test basic stuff with igt how did you test your new
> feature? What I'm saying is: if developer X is having to use igt for
> a new feature then he is either: a) expecting that basic testing works for
> his driver (or he has bootstrapping isssues) or that b) if the feature
> doesn't get tested on driver A because that driver doesn't even have basic
> tests passing for it then the feature is acceptable. And it will be helpful
> to publish the list of igt tests that pass for each driver in KMS.

I think this is an entirely different discussion, something along the
lines of "new drivers/new features implemented on existing drivers"
must come with igt test results to prove it's working. Not what I'm
proposing here at all. It would be a good discussion to have, but a
different one I think.

The proposal here is only for new uapi: Thus far the requirement is
"driver + userspace", not it would be "driver + userspace + igt, if
generic feature".

> Alternatively, you need to explain better what "driver-agnostic testcase" is
> (my reading: a testcase that can be run on any driver). Or point to some igt
> testcases that are considered driver-agnostic and known to run on different
> (read: non-x86) platforms.

If this is just about clarifying "driver-agnostic testcase" then that
boils down to drm_open(DRIVER_ANY). Plus reasonable engineering calls
on what exactly agnostic means, because in practice I expect that
"works on first driver, doesn't on 2nd" is fairly common, no matter
which driver is the first one, and which is the second.

Now for a list of guaranteed-to-be-generic tests, I'm slowly pushing
vkms forward to get there. But it's going to take a lot of internships
to make vkms featureful enough to be useful in such a role.

If you're just looking for i915 test results, we publish them all. And
afaik no one else does so, so not really possible to point at anything
else.

> > > > Although I think there's not really
> > > > any big gaps in igt anymore, we do have at least some (rather rough
> > > > and coarse in some case) test coverage for everything I think.
> > >
> > > I would like to see some proof of that in the form of ....
> > >
> > >
> > > > Should this be clarified further?
> > >
> > > an URL that points to a page similar to Mesa's GL supported features
> > > would be nice to add here, from my point of view.
> >
> > There isn't even a list of upstream drm features ...
>
> Yeah, I know that! :)
>
> > how exactly should that look like?
>
> What about a list of igt tests then? OR we could put in the igt TODO file
> the kind of tests we would like to have at some moment?

I'm still not clear what your concern is and what you want to achieve
here. There's lots of good ideas we can push for around igt, so
arbitrarily filling a todo file isn't hard. But I also don't see the
point (we do have some todo items btw on gitlab I think).
-Daniel

>
> Best regards,
> Liviu
>
>
> > Only thing I can tell you is that we had huge
> > amounts of todo for missing igt tests, and afaik we've worked them all
> > down. We = intel here. It's not perfect, but good enough that we've
> > essentially stopped doing any validation except by running igt. Of
> > course you can always write more tests, same way you can always
> > bikeshed a patch a few more times. Eventually there's dimishing
> > returns.
> > -Daniel
> >
> > > Best regards,
> > > Liviu
> > >
> > > > -Daniel
> > > >
> > > > >
> > > > > Best regards,
> > > > > Liviu
> > > > >
> > > > >
> > > > > > +
> > > > > >  Validating changes with IGT
> > > > > >  ---------------------------
> > > > > >
> > > > > > --
> > > > > > 2.20.1
> > > > > >
> > > > >
> > > >
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > >
> > > --
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Liviu Dudau]

On Thu, Jan 17, 2019 at 04:59:49PM +0100, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 3:54 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > >
> > > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > > > >
> > > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > > > forward a lot:
> > > > > > >
> > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > > > >   and a sysroot build (should address all the build/cross platform
> > > > > > >   concerns raised in the RFC discussions).
> > > > > > >
> > > > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > > > >   don't clog the main/shared tests directory anymore
> > > > > > >
> > > > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > > > >   igt tests patches.
> > > > > > >
> > > > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > > > go ahead with this.
> > > > > > >
> > > > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > > > ---
> > > > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > > > >
> > > > > > From an aspirational point of view I am fine with this and you can have
> > > > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > > > >
> > > > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > > > task of getting enough KMS drivers testable in the first place.
> > > > >
> > > > > This is what the _new_ features is about, no expectation to write
> > > > > tests for all the existing stuff.
> > > >
> > > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your
> > > > _new_ feature tests are not going to mean much, doesn't it?
> > >
> > > Why? There's still good test coverage for the new stuff at least. And
> > > eventually someone gets bored and fills in the gaps. This is how we've
> > > created all the current igt testcases, so it works - mandatory igt for
> > > new features is the rule for anything intel does since well over 5
> > > years.
> >
> > Because if you can't test basic stuff with igt how did you test your new
> > feature? What I'm saying is: if developer X is having to use igt for
> > a new feature then he is either: a) expecting that basic testing works for
> > his driver (or he has bootstrapping isssues) or that b) if the feature
> > doesn't get tested on driver A because that driver doesn't even have basic
> > tests passing for it then the feature is acceptable. And it will be helpful
> > to publish the list of igt tests that pass for each driver in KMS.
> 
> I think this is an entirely different discussion, something along the
> lines of "new drivers/new features implemented on existing drivers"
> must come with igt test results to prove it's working. Not what I'm
> proposing here at all. It would be a good discussion to have, but a
> different one I think.

New features implemented on existing drivers is what I'm expecting to apply most
of the time when we add the new test requirement. The new UAPI is likely to
come out as a result of finding better ways to handle existing code, but I might
be wrong here.

> 
> The proposal here is only for new uapi: Thus far the requirement is
> "driver + userspace", not it would be "driver + userspace + igt, if
> generic feature".
> 
> > Alternatively, you need to explain better what "driver-agnostic testcase" is
> > (my reading: a testcase that can be run on any driver). Or point to some igt
> > testcases that are considered driver-agnostic and known to run on different
> > (read: non-x86) platforms.
> 
> If this is just about clarifying "driver-agnostic testcase" then that
> boils down to drm_open(DRIVER_ANY). Plus reasonable engineering calls
> on what exactly agnostic means, because in practice I expect that
> "works on first driver, doesn't on 2nd" is fairly common, no matter
> which driver is the first one, and which is the second.

Driver-agnostic to me means "it uses KMS API without any driver-specific #defines
or IOCTLs". Now, I agree that you can come up with examples where this breaks down
really quickly, like posting a framebuffer via atomic IOCTL is driver
agnostic until you start talking about modifiers (and even finding a common format
other than RGB888 might be a stretch). I am (slowly, sorry about that) starting to
understand your point of view, that when you have done driver and userspace 
development, adding an igt testcase is not a big deal, but I have recollections of
my first encounter with igt and finding it daunting to decide which tests should
be run at the most basic level to even claim that igt supports my driver. Assuming
a seasoned KMS driver developer now being faced with IGT tests as a new requirement,
I was trying to preempt complaints that the test suite in itself is not very easy
to "port" to a new driver.

> 
> Now for a list of guaranteed-to-be-generic tests, I'm slowly pushing
> vkms forward to get there. But it's going to take a lot of internships
> to make vkms featureful enough to be useful in such a role.
> 
> If you're just looking for i915 test results, we publish them all. And
> afaik no one else does so, so not really possible to point at anything
> else.

Sorry for not being in constant touch with i915 development (and being lazy), can
you share the URL for the published test results? Should that URL be added in the
documentation (for igt or kernel)?


> 
> > > > > Although I think there's not really
> > > > > any big gaps in igt anymore, we do have at least some (rather rough
> > > > > and coarse in some case) test coverage for everything I think.
> > > >
> > > > I would like to see some proof of that in the form of ....
> > > >
> > > >
> > > > > Should this be clarified further?
> > > >
> > > > an URL that points to a page similar to Mesa's GL supported features
> > > > would be nice to add here, from my point of view.
> > >
> > > There isn't even a list of upstream drm features ...
> >
> > Yeah, I know that! :)
> >
> > > how exactly should that look like?
> >
> > What about a list of igt tests then? OR we could put in the igt TODO file
> > the kind of tests we would like to have at some moment?
> 
> I'm still not clear what your concern is and what you want to achieve
> here. There's lots of good ideas we can push for around igt, so
> arbitrarily filling a todo file isn't hard. But I also don't see the
> point (we do have some todo items btw on gitlab I think).
> -Daniel

Concerned that a new requirement added to the DRM framework can be met with pushbacks
if the tool is not usable or generates false negatives. Concerned that adding a generic
feature like writeback into igt is taking an excessive amount of time to be reviewed
and merged and that in the future getting through this process is going to be mandatory
for any new feature.

Best regards,
Liviu

> 
> >
> > Best regards,
> > Liviu
> >
> >
> > > Only thing I can tell you is that we had huge
> > > amounts of todo for missing igt tests, and afaik we've worked them all
> > > down. We = intel here. It's not perfect, but good enough that we've
> > > essentially stopped doing any validation except by running igt. Of
> > > course you can always write more tests, same way you can always
> > > bikeshed a patch a few more times. Eventually there's dimishing
> > > returns.
> > > -Daniel
> > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > > > -Daniel
> > > > >
> > > > > >
> > > > > > Best regards,
> > > > > > Liviu
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > >  Validating changes with IGT
> > > > > > >  ---------------------------
> > > > > > >
> > > > > > > --
> > > > > > > 2.20.1
> > > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > Daniel Vetter
> > > > > Software Engineer, Intel Corporation
> > > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > > >
> > > > --
> > > > ====================
> > > > | I would like to |
> > > > | fix the world,  |
> > > > | but they're not |
> > > > | giving me the   |
> > > >  \ source code!  /
> > > >   ---------------
> > > >     ¯\_(ツ)_/¯
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> > --
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

Hi Daniel,

On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > forward a lot:
> > >
> > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >   and a sysroot build (should address all the build/cross platform
> > >   concerns raised in the RFC discussions).
> > >
> > > - tests reorganized into subdirectories so that the i915-gem tests
> > >   don't clog the main/shared tests directory anymore
> > >
> > > - quite a few more non-intel people contributing/reviewing/committing
> > >   igt tests patches.
> > >
> > > I think this addresses all the concerns raised in the RFC discussions,
> > > and assuming there's enough Acks and no new issues that pop up, we can
> > > go ahead with this.
> > >
> > > 1: https://patchwork.kernel.org/patch/10648851/
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > Cc: Sean Paul <sean@poorly.run>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  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 a752aa561ea4..413915d6b7d2 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.
> >
> > From an aspirational point of view I am fine with this and you can have
> > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> >
> > From a practical point of view I would like to see a matrix of KMS APIs
> > that are being validated and the drivers that have been tested. Otherwise,
> > the next person that comes and tries to add a new IOCTL, KMS property or new
> > file in sysfs is going to discover that he has subscribed to a much bigger
> > task of getting enough KMS drivers testable in the first place.
> 
> This is what the _new_ features is about, no expectation to write
> tests for all the existing stuff. Although I think there's not really
> any big gaps in igt anymore, we do have at least some (rather rough
> and coarse in some case) test coverage for everything I think. Should
> this be clarified further?
> -Daniel
> 

I share a similar view to Liviu here. I think this new requirement
raises the bar more than you intended.

By saying that all new features must be tested by igt, you're also
implying that a driver must run igt (at some basic level); before the
developers working on that driver can start trying to implement new
features. That puts an additional hurdle in the way of adding stuff
to KMS for people who aren't already using igt.

I'm all for testing, and UAPI being well proven before we merge it,
and even for a central KMS test suite. However, when we (Arm Mali-DP
people) have tried to implement things in igt it's been a battle,
because of various built-in assumptions which it made.

For example, most meaningful igt tests rely on CRC. Much of our HW
doesn't have CRC. CRC could be implemented in theory using writeback,
but that currently doesn't exist. That means you're effectively saying
that we (Arm) can't implement any new cross-device KMS features until
we've either:

 a) also implemented writeback-based CRC in igt OR
 b) implemented the new feature in someone else's driver which does
    support CRC.

That seems a bit out of order to me. It would be like me saying "all
KMS drivers must use Arm's test suite, which uses writeback and pixel
checking", and you'd be in a pickle because you don't have writeback.

In a similar vein, I remember having to fix igt on devices which
didn't have cursor planes, before I could even think about writing
tests.

If you can prove that issues like these are all resolved now in igt,
then great! Otherwise, I don't think igt is "ready" to be used as a
requirement for merging KMS kernel API.

Thanks,
-Brian

> >
> > Best regards,
> > Liviu
> >
> >
> > > +
> > >  Validating changes with IGT
> > >  ---------------------------
> > >
> > > --
> > > 2.20.1
> > >
> >
> > --
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Mon, Jan 21, 2019 at 12:54 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
>
> Hi Daniel,
>
> On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > >
> > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > forward a lot:
> > > >
> > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > >   and a sysroot build (should address all the build/cross platform
> > > >   concerns raised in the RFC discussions).
> > > >
> > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > >   don't clog the main/shared tests directory anymore
> > > >
> > > > - quite a few more non-intel people contributing/reviewing/committing
> > > >   igt tests patches.
> > > >
> > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > go ahead with this.
> > > >
> > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > Cc: Sean Paul <sean@poorly.run>
> > > > Cc: Eric Anholt <eric@anholt.net>
> > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > >  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 a752aa561ea4..413915d6b7d2 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.
> > >
> > > From an aspirational point of view I am fine with this and you can have
> > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > >
> > > From a practical point of view I would like to see a matrix of KMS APIs
> > > that are being validated and the drivers that have been tested. Otherwise,
> > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > task of getting enough KMS drivers testable in the first place.
> >
> > This is what the _new_ features is about, no expectation to write
> > tests for all the existing stuff. Although I think there's not really
> > any big gaps in igt anymore, we do have at least some (rather rough
> > and coarse in some case) test coverage for everything I think. Should
> > this be clarified further?
> > -Daniel
> >
>
> I share a similar view to Liviu here. I think this new requirement
> raises the bar more than you intended.
>
> By saying that all new features must be tested by igt, you're also
> implying that a driver must run igt (at some basic level); before the
> developers working on that driver can start trying to implement new
> features. That puts an additional hurdle in the way of adding stuff
> to KMS for people who aren't already using igt.
>
> I'm all for testing, and UAPI being well proven before we merge it,
> and even for a central KMS test suite. However, when we (Arm Mali-DP
> people) have tried to implement things in igt it's been a battle,
> because of various built-in assumptions which it made.
>
> For example, most meaningful igt tests rely on CRC. Much of our HW
> doesn't have CRC. CRC could be implemented in theory using writeback,
> but that currently doesn't exist. That means you're effectively saying
> that we (Arm) can't implement any new cross-device KMS features until
> we've either:
>
>  a) also implemented writeback-based CRC in igt OR
>  b) implemented the new feature in someone else's driver which does
>     support CRC.

We didn't just pick crcs for lols (or because that's all intel
supports), we picked it because it will work for both hw with crc and
hw with writeback. I checked with a pile of driver writers way back
(over irc), and the interface we picked is something pretty much all
display blocks (except the _very_ simple ones) should be able to
support. Same discussion also happened again when made the crc
interfaces in debugfs more generic.

> That seems a bit out of order to me. It would be like me saying "all
> KMS drivers must use Arm's test suite, which uses writeback and pixel
> checking", and you'd be in a pickle because you don't have writeback.
>
> In a similar vein, I remember having to fix igt on devices which
> didn't have cursor planes, before I could even think about writing
> tests.
>
> If you can prove that issues like these are all resolved now in igt,
> then great! Otherwise, I don't think igt is "ready" to be used as a
> requirement for merging KMS kernel API.

I don't think this is realistic (or Liviu's take on this): Expecting
that igt just works on all drivers, and that driver writers don't have
to do some ramp-up on a testsuite code base is just not going to
happen. If we go with a testsuite then there
- will be some ramp up required for people who've never used it
- it won't work on every driver under the sun

The later could perhaps be fixed if we go khronos-style on uapi
design, but pls note that the requirements for ratifying new khr
extensions are _much_ stricter than what we have right now, or what's
proposed here. Plus khr essentially got the testsuite for old gl
versions donated by google to them (they still don't have one for
legacy gl, only for modern gl and all versions of gles).

Also, the fundamental difference between igt and all these other kms
test suites is that they're all not open source. We're not going to
use a closed source to validate i915, nor are we going to pour
engineering resources into that.

So from a theoretical pov I think your argument has some merit, in
practice it boils down to "let's not have a test suite, mkay". Is that
your stance here?
-Daniel


>
> Thanks,
> -Brian
>
> > >
> > > Best regards,
> > > Liviu
> > >
> > >
> > > > +
> > > >  Validating changes with IGT
> > > >  ---------------------------
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > >
> > > --
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Mon, Jan 21, 2019 at 6:21 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Jan 21, 2019 at 12:54 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> >
> > Hi Daniel,
> >
> > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > >
> > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > forward a lot:
> > > > >
> > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > >   and a sysroot build (should address all the build/cross platform
> > > > >   concerns raised in the RFC discussions).
> > > > >
> > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > >   don't clog the main/shared tests directory anymore
> > > > >
> > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > >   igt tests patches.
> > > > >
> > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > go ahead with this.
> > > > >
> > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > ---
> > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > >
> > > > From an aspirational point of view I am fine with this and you can have
> > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > >
> > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > task of getting enough KMS drivers testable in the first place.
> > >
> > > This is what the _new_ features is about, no expectation to write
> > > tests for all the existing stuff. Although I think there's not really
> > > any big gaps in igt anymore, we do have at least some (rather rough
> > > and coarse in some case) test coverage for everything I think. Should
> > > this be clarified further?
> > > -Daniel
> > >
> >
> > I share a similar view to Liviu here. I think this new requirement
> > raises the bar more than you intended.
> >
> > By saying that all new features must be tested by igt, you're also
> > implying that a driver must run igt (at some basic level); before the
> > developers working on that driver can start trying to implement new
> > features. That puts an additional hurdle in the way of adding stuff
> > to KMS for people who aren't already using igt.
> >
> > I'm all for testing, and UAPI being well proven before we merge it,
> > and even for a central KMS test suite. However, when we (Arm Mali-DP
> > people) have tried to implement things in igt it's been a battle,
> > because of various built-in assumptions which it made.
> >
> > For example, most meaningful igt tests rely on CRC. Much of our HW
> > doesn't have CRC. CRC could be implemented in theory using writeback,
> > but that currently doesn't exist. That means you're effectively saying
> > that we (Arm) can't implement any new cross-device KMS features until
> > we've either:
> >
> >  a) also implemented writeback-based CRC in igt OR
> >  b) implemented the new feature in someone else's driver which does
> >     support CRC.
>
> We didn't just pick crcs for lols (or because that's all intel
> supports), we picked it because it will work for both hw with crc and
> hw with writeback. I checked with a pile of driver writers way back
> (over irc), and the interface we picked is something pretty much all
> display blocks (except the _very_ simple ones) should be able to
> support. Same discussion also happened again when made the crc
> interfaces in debugfs more generic.
>
> > That seems a bit out of order to me. It would be like me saying "all
> > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > checking", and you'd be in a pickle because you don't have writeback.
> >
> > In a similar vein, I remember having to fix igt on devices which
> > didn't have cursor planes, before I could even think about writing
> > tests.
> >
> > If you can prove that issues like these are all resolved now in igt,
> > then great! Otherwise, I don't think igt is "ready" to be used as a
> > requirement for merging KMS kernel API.

With a night's worth of sleep, this here is what annoys me - you
demand I "prove" that igt works everywhere. That's not realistic.
Intel is not going to pay the bill to get a CI farm for every drm
driver existing up&running, including fixing all the bugs that will
uncover (both in igt and even more so in drivers). Especially not if
mere mortals can't even buy the hardware. Nor is anyone else going to
do that. If there are some fundamental overall issues with igt then
I'm ofc happy to get them addressed (like the build issues raised a
few months ago).

There's also the options to somehow find funding for an igt clone, but
if you just find a bit of fund then realistically it's more effective
to throw it at igt. That's how google funded making igt work on
non-i915 drivers at least. Or you manage to open source one of the
existing internal/proprietary test suites, but I think that's going to
be worse than a fresh clone since on top of finding money you also
have to wait a few years for legal review.
-Danie


> I don't think this is realistic (or Liviu's take on this): Expecting
> that igt just works on all drivers, and that driver writers don't have
> to do some ramp-up on a testsuite code base is just not going to
> happen. If we go with a testsuite then there
> - will be some ramp up required for people who've never used it
> - it won't work on every driver under the sun
>
> The later could perhaps be fixed if we go khronos-style on uapi
> design, but pls note that the requirements for ratifying new khr
> extensions are _much_ stricter than what we have right now, or what's
> proposed here. Plus khr essentially got the testsuite for old gl
> versions donated by google to them (they still don't have one for
> legacy gl, only for modern gl and all versions of gles).
>
> Also, the fundamental difference between igt and all these other kms
> test suites is that they're all not open source. We're not going to
> use a closed source to validate i915, nor are we going to pour
> engineering resources into that.
>
> So from a theoretical pov I think your argument has some merit, in
> practice it boils down to "let's not have a test suite, mkay". Is that
> your stance here?
> -Daniel
>
>
> >
> > Thanks,
> > -Brian
> >
> > > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > >
> > > > > +
> > > > >  Validating changes with IGT
> > > > >  ---------------------------
> > > > >
> > > > > --
> > > > > 2.20.1
> > > > >
> > > >
> > > > --
> > > > ====================
> > > > | I would like to |
> > > > | fix the world,  |
> > > > | but they're not |
> > > > | giving me the   |
> > > >  \ source code!  /
> > > >   ---------------
> > > >     ¯\_(ツ)_/¯
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

Hi,

On Tue, Jan 22, 2019 at 09:53:20AM +0100, Daniel Vetter wrote:
> On Mon, Jan 21, 2019 at 6:21 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > On Mon, Jan 21, 2019 at 12:54 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > >
> > > Hi Daniel,
> > >
> > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > > >
> > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > > forward a lot:
> > > > > >
> > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > > >   and a sysroot build (should address all the build/cross platform
> > > > > >   concerns raised in the RFC discussions).
> > > > > >
> > > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > > >   don't clog the main/shared tests directory anymore
> > > > > >
> > > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > > >   igt tests patches.
> > > > > >
> > > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > > go ahead with this.
> > > > > >
> > > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > > ---
> > > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > > >
> > > > > From an aspirational point of view I am fine with this and you can have
> > > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > > >
> > > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > > task of getting enough KMS drivers testable in the first place.
> > > >
> > > > This is what the _new_ features is about, no expectation to write
> > > > tests for all the existing stuff. Although I think there's not really
> > > > any big gaps in igt anymore, we do have at least some (rather rough
> > > > and coarse in some case) test coverage for everything I think. Should
> > > > this be clarified further?
> > > > -Daniel
> > > >
> > >
> > > I share a similar view to Liviu here. I think this new requirement
> > > raises the bar more than you intended.
> > >
> > > By saying that all new features must be tested by igt, you're also
> > > implying that a driver must run igt (at some basic level); before the
> > > developers working on that driver can start trying to implement new
> > > features. That puts an additional hurdle in the way of adding stuff
> > > to KMS for people who aren't already using igt.
> > >
> > > I'm all for testing, and UAPI being well proven before we merge it,
> > > and even for a central KMS test suite. However, when we (Arm Mali-DP
> > > people) have tried to implement things in igt it's been a battle,
> > > because of various built-in assumptions which it made.
> > >
> > > For example, most meaningful igt tests rely on CRC. Much of our HW
> > > doesn't have CRC. CRC could be implemented in theory using writeback,
> > > but that currently doesn't exist. That means you're effectively saying
> > > that we (Arm) can't implement any new cross-device KMS features until
> > > we've either:
> > >
> > >  a) also implemented writeback-based CRC in igt OR
> > >  b) implemented the new feature in someone else's driver which does
> > >     support CRC.
> >
> > We didn't just pick crcs for lols (or because that's all intel
> > supports), we picked it because it will work for both hw with crc and
> > hw with writeback. I checked with a pile of driver writers way back
> > (over irc), and the interface we picked is something pretty much all
> > display blocks (except the _very_ simple ones) should be able to
> > support. Same discussion also happened again when made the crc
> > interfaces in debugfs more generic.

That doesn't really address my issue, but no matter. I guess I'm
having a hard time separating the existing igts from _new_ igts for
new features; so sorry about that.

Please appreciate that there's honestly a whole lot of work needed to
make existing igts work for CRC using writeback. There's no "nonblock"
or continuous CRC collection using writeback. You also need to request
your CRC before you make your commit, so that the FB can be attached
to the writeback connector. Fixing this up isn't small or trivial,
it's not just a case of "ramp-up", it's pervasive throughout the igt
tree.

That shouldn't need to impact your proposal for new tests, if they are
written with that in mind.

> >
> > > That seems a bit out of order to me. It would be like me saying "all
> > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > checking", and you'd be in a pickle because you don't have writeback.
> > >
> > > In a similar vein, I remember having to fix igt on devices which
> > > didn't have cursor planes, before I could even think about writing
> > > tests.
> > >
> > > If you can prove that issues like these are all resolved now in igt,
> > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > requirement for merging KMS kernel API.
> 
> With a night's worth of sleep, this here is what annoys me - you
> demand I "prove" that igt works everywhere. That's not realistic.
> Intel is not going to pay the bill to get a CI farm for every drm
> driver existing up&running, including fixing all the bugs that will
> uncover (both in igt and even more so in drivers). Especially not if
> mere mortals can't even buy the hardware. Nor is anyone else going to
> do that. If there are some fundamental overall issues with igt then
> I'm ofc happy to get them addressed (like the build issues raised a
> few months ago).

I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
not being deliberately awkward.

I genuinely don't know what the current state of "not-i915" testing is
in igt. Do you really not think it's a good idea to have that known
and documented before you make igt a mandatory part of the DRM tree?

Sure, there's commits from non-intel folks - heck there's even a few
from me. But I can tell you right away that doesn't mean that igt
works in any meaningful way on my platform. Does it work well enough
for me to add new test cases? Honestly I don't know.

Maybe you can suggest some suites which are expected to already be
fully generic and should run anywhere which we can use as
confirmation? To me, having some reasonable subset of the active
driver devs building and running that on their platforms and reporting
back before you merge this patch wouldn't be unreasonable or
outlandish.

> 
> There's also the options to somehow find funding for an igt clone, but
> if you just find a bit of fund then realistically it's more effective
> to throw it at igt. That's how google funded making igt work on
> non-i915 drivers at least. Or you manage to open source one of the
> existing internal/proprietary test suites, but I think that's going to
> be worse than a fresh clone since on top of finding money you also
> have to wait a few years for legal review.
> -Danie
> 
> 
> > I don't think this is realistic (or Liviu's take on this): Expecting
> > that igt just works on all drivers, and that driver writers don't have
> > to do some ramp-up on a testsuite code base is just not going to
> > happen. If we go with a testsuite then there
> > - will be some ramp up required for people who've never used it
> > - it won't work on every driver under the sun

That's fair, and I'll grant you that it's hard to push people to do
that ramp-up before it's mandatory.

> >
> > The later could perhaps be fixed if we go khronos-style on uapi
> > design, but pls note that the requirements for ratifying new khr
> > extensions are _much_ stricter than what we have right now, or what's
> > proposed here. Plus khr essentially got the testsuite for old gl
> > versions donated by google to them (they still don't have one for
> > legacy gl, only for modern gl and all versions of gles).

meh. There's both advantages and disadvantages to that. I do think
there's a lot of scope for more concrete API design docs.

> >
> > Also, the fundamental difference between igt and all these other kms
> > test suites is that they're all not open source. We're not going to
> > use a closed source to validate i915, nor are we going to pour
> > engineering resources into that.
> >

Yes, obviously we can't use a closed codebase as the test suite for
DRM. I don't think anyone has/would/will suggest that. I was making an
analogy, that if I came to you with an open test-suite, which was
written by Arm and which didn't run on your hardware, and said it was
about to become mandatory, you might have some complaints.

> > So from a theoretical pov I think your argument has some merit, in
> > practice it boils down to "let's not have a test suite, mkay". Is that
> > your stance here?

I think you're twisting my words a bit far there. That's clearly not
what I'm saying. I was quite explicit about that point:

	> I'm all for [...] a central KMS test suite.

If I could magically wish for anything I like, it would be for a
test-suite which was built from the ground up to be *the* generic KMS
test-suite.

We don't live in a magical world, so igt makes a good starting point.
I'm not disputing that. I'm just asking for a sanity check of your
assertions that igt is "ready" today.

Thanks,
-Brian

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
>
> Hi,
>
> On Tue, Jan 22, 2019 at 09:53:20AM +0100, Daniel Vetter wrote:
> > On Mon, Jan 21, 2019 at 6:21 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > On Mon, Jan 21, 2019 at 12:54 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > > >
> > > > Hi Daniel,
> > > >
> > > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
> > > > > >
> > > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > > > forward a lot:
> > > > > > >
> > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > > > >   and a sysroot build (should address all the build/cross platform
> > > > > > >   concerns raised in the RFC discussions).
> > > > > > >
> > > > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > > > >   don't clog the main/shared tests directory anymore
> > > > > > >
> > > > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > > > >   igt tests patches.
> > > > > > >
> > > > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > > > go ahead with this.
> > > > > > >
> > > > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > > > ---
> > > > > > >  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 a752aa561ea4..413915d6b7d2 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.
> > > > > >
> > > > > > From an aspirational point of view I am fine with this and you can have
> > > > > > my Acked-by: Liviu Dudau <liviu.dudau@arm.com>.
> > > > > >
> > > > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > > > task of getting enough KMS drivers testable in the first place.
> > > > >
> > > > > This is what the _new_ features is about, no expectation to write
> > > > > tests for all the existing stuff. Although I think there's not really
> > > > > any big gaps in igt anymore, we do have at least some (rather rough
> > > > > and coarse in some case) test coverage for everything I think. Should
> > > > > this be clarified further?
> > > > > -Daniel
> > > > >
> > > >
> > > > I share a similar view to Liviu here. I think this new requirement
> > > > raises the bar more than you intended.
> > > >
> > > > By saying that all new features must be tested by igt, you're also
> > > > implying that a driver must run igt (at some basic level); before the
> > > > developers working on that driver can start trying to implement new
> > > > features. That puts an additional hurdle in the way of adding stuff
> > > > to KMS for people who aren't already using igt.
> > > >
> > > > I'm all for testing, and UAPI being well proven before we merge it,
> > > > and even for a central KMS test suite. However, when we (Arm Mali-DP
> > > > people) have tried to implement things in igt it's been a battle,
> > > > because of various built-in assumptions which it made.
> > > >
> > > > For example, most meaningful igt tests rely on CRC. Much of our HW
> > > > doesn't have CRC. CRC could be implemented in theory using writeback,
> > > > but that currently doesn't exist. That means you're effectively saying
> > > > that we (Arm) can't implement any new cross-device KMS features until
> > > > we've either:
> > > >
> > > >  a) also implemented writeback-based CRC in igt OR
> > > >  b) implemented the new feature in someone else's driver which does
> > > >     support CRC.
> > >
> > > We didn't just pick crcs for lols (or because that's all intel
> > > supports), we picked it because it will work for both hw with crc and
> > > hw with writeback. I checked with a pile of driver writers way back
> > > (over irc), and the interface we picked is something pretty much all
> > > display blocks (except the _very_ simple ones) should be able to
> > > support. Same discussion also happened again when made the crc
> > > interfaces in debugfs more generic.
>
> That doesn't really address my issue, but no matter. I guess I'm
> having a hard time separating the existing igts from _new_ igts for
> new features; so sorry about that.
>
> Please appreciate that there's honestly a whole lot of work needed to
> make existing igts work for CRC using writeback. There's no "nonblock"
> or continuous CRC collection using writeback. You also need to request
> your CRC before you make your commit, so that the FB can be attached
> to the writeback connector. Fixing this up isn't small or trivial,
> it's not just a case of "ramp-up", it's pervasive throughout the igt
> tree.
>
> That shouldn't need to impact your proposal for new tests, if they are
> written with that in mind.

Hm, I've figured the simplest way to make this happen is to enable
writeback from the debugfs crc interface, and instead of just
collecting the crc from the writeback completion interrupt, you first
feed the entire framebuffer into a checksum function. That code
shouldn't need a full rewrite of igt, and is kinda how I envisaged
writeback-for-igt-crc would pan out.

It's at least how we've done it for vkms, and there the
compress-fb-into-crc is a few lines of code. Most important bit is
that you don't sample invisible stuff, i.e. don't sample the X in
XRGB8888 (which is what vkms uses internally right now), and to not
sampel the invisible stuff outside of the fb. We could put this into a
drm_framebuffer_helper.c, Noralf has a bunch of other related fb
helper functions in tinydrm which would also neatly fit in there.

It could be that this isn't fast enough to keep up with vblank, but
for all the single-shot crc tests it should be good enough. Old igt
had the mishabit of asking for multiple crcs (to hack around i915
issues), but I thought we've addressed all that when creating the
generic debugfs crc interfaces. The other issue is that you probably
can't enable writeback in all cases (some chips you can enable
writeback as a sidechannel to any active crtc, some need a dedicated
thing). I do think igt can cope with "sry no crc for this config"
though and should skip these (sub)tests if it's just not possible.

Rewriting all of igt to make writeback-only display blocks work with
them sounds like the wrong approach. Or at least one we should try
really hard to avoid.

> > > > That seems a bit out of order to me. It would be like me saying "all
> > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > checking", and you'd be in a pickle because you don't have writeback.
> > > >
> > > > In a similar vein, I remember having to fix igt on devices which
> > > > didn't have cursor planes, before I could even think about writing
> > > > tests.
> > > >
> > > > If you can prove that issues like these are all resolved now in igt,
> > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > requirement for merging KMS kernel API.
> >
> > With a night's worth of sleep, this here is what annoys me - you
> > demand I "prove" that igt works everywhere. That's not realistic.
> > Intel is not going to pay the bill to get a CI farm for every drm
> > driver existing up&running, including fixing all the bugs that will
> > uncover (both in igt and even more so in drivers). Especially not if
> > mere mortals can't even buy the hardware. Nor is anyone else going to
> > do that. If there are some fundamental overall issues with igt then
> > I'm ofc happy to get them addressed (like the build issues raised a
> > few months ago).
>
> I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> not being deliberately awkward.
>
> I genuinely don't know what the current state of "not-i915" testing is
> in igt. Do you really not think it's a good idea to have that known
> and documented before you make igt a mandatory part of the DRM tree?
>
> Sure, there's commits from non-intel folks - heck there's even a few
> from me. But I can tell you right away that doesn't mean that igt
> works in any meaningful way on my platform. Does it work well enough
> for me to add new test cases? Honestly I don't know.
>
> Maybe you can suggest some suites which are expected to already be
> fully generic and should run anywhere which we can use as
> confirmation? To me, having some reasonable subset of the active
> driver devs building and running that on their platforms and reporting
> back before you merge this patch wouldn't be unreasonable or
> outlandish.

So my Grand Plan is to get vkms up to par, and even get that
integrated into igt and drm autobuilds we'll run on gitlab CI. But
vkms is still a few internships away from being useful, atm it has one
primary plane and one non-plane cursor (it's not even a universal
cursor plane yet). Will give you _lots_ of skips.

But I guess we could fairly easily add vkms as one of the machines
we're testing to intel's CI farm, which means every patch and every
commit gets at least tested on non-i915, and you'll get tons of data
of which tests blow up, and even more which will just skip (due to
lack of features on vkms' side). We did have that situation a while
back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
accident (we do still run i915+amdgpu buffer sharing tests, since we
care about that). Took us a while to realize that igt preferred
running on amdgpu somehow. Running random non-i915 chips in our CI
farm isn't something that'll happen, but vkms should not be hard to
set up and get going.

Would that be useful if we get that done first?

> > There's also the options to somehow find funding for an igt clone, but
> > if you just find a bit of fund then realistically it's more effective
> > to throw it at igt. That's how google funded making igt work on
> > non-i915 drivers at least. Or you manage to open source one of the
> > existing internal/proprietary test suites, but I think that's going to
> > be worse than a fresh clone since on top of finding money you also
> > have to wait a few years for legal review.
> > -Danie
> >
> >
> > > I don't think this is realistic (or Liviu's take on this): Expecting
> > > that igt just works on all drivers, and that driver writers don't have
> > > to do some ramp-up on a testsuite code base is just not going to
> > > happen. If we go with a testsuite then there
> > > - will be some ramp up required for people who've never used it
> > > - it won't work on every driver under the sun
>
> That's fair, and I'll grant you that it's hard to push people to do
> that ramp-up before it's mandatory.
>
> > >
> > > The later could perhaps be fixed if we go khronos-style on uapi
> > > design, but pls note that the requirements for ratifying new khr
> > > extensions are _much_ stricter than what we have right now, or what's
> > > proposed here. Plus khr essentially got the testsuite for old gl
> > > versions donated by google to them (they still don't have one for
> > > legacy gl, only for modern gl and all versions of gles).
>
> meh. There's both advantages and disadvantages to that. I do think
> there's a lot of scope for more concrete API design docs.
>
> > >
> > > Also, the fundamental difference between igt and all these other kms
> > > test suites is that they're all not open source. We're not going to
> > > use a closed source to validate i915, nor are we going to pour
> > > engineering resources into that.
> > >
>
> Yes, obviously we can't use a closed codebase as the test suite for
> DRM. I don't think anyone has/would/will suggest that. I was making an
> analogy, that if I came to you with an open test-suite, which was
> written by Arm and which didn't run on your hardware, and said it was
> about to become mandatory, you might have some complaints.

Intel invests _lots_ of money into being first with open source. If
they wouldn't, I'd probably be working somewhere else and fighting for
that vendor's test suite :-) So I think it's not really symmetrical
situation. Heck if my mail would be daniel.vetter@arm.com I'd be busy
ramping arm validation up on igt and killing any internal test suites.
Aside: I've already killed 2 such tests suites here at intel, I'm good
at this ...

> > > So from a theoretical pov I think your argument has some merit, in
> > > practice it boils down to "let's not have a test suite, mkay". Is that
> > > your stance here?
>
> I think you're twisting my words a bit far there. That's clearly not
> what I'm saying. I was quite explicit about that point:
>
>         > I'm all for [...] a central KMS test suite.
>
> If I could magically wish for anything I like, it would be for a
> test-suite which was built from the ground up to be *the* generic KMS
> test-suite.
>
> We don't live in a magical world, so igt makes a good starting point.
> I'm not disputing that. I'm just asking for a sanity check of your
> assertions that igt is "ready" today.

I guess it boils down to what a clear goal for "ready" is, which is
useful, concrete and realistic. "Get vkms into igt CI" would be a
useful thing, and something that imo makes tons of sense. It's been on
my plan anyway. "Prove that igt is ready for everyone" otoh is too
vague to be actionable.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:

[snip]

> >
> > That doesn't really address my issue, but no matter. I guess I'm
> > having a hard time separating the existing igts from _new_ igts for
> > new features; so sorry about that.
> >
> > Please appreciate that there's honestly a whole lot of work needed to
> > make existing igts work for CRC using writeback. There's no "nonblock"
> > or continuous CRC collection using writeback. You also need to request
> > your CRC before you make your commit, so that the FB can be attached
> > to the writeback connector. Fixing this up isn't small or trivial,
> > it's not just a case of "ramp-up", it's pervasive throughout the igt
> > tree.
> >
> > That shouldn't need to impact your proposal for new tests, if they are
> > written with that in mind.
> 
> Hm, I've figured the simplest way to make this happen is to enable
> writeback from the debugfs crc interface, and instead of just
> collecting the crc from the writeback completion interrupt, you first
> feed the entire framebuffer into a checksum function. That code
> shouldn't need a full rewrite of igt, and is kinda how I envisaged
> writeback-for-igt-crc would pan out.
> 
> It's at least how we've done it for vkms, and there the
> compress-fb-into-crc is a few lines of code. Most important bit is
> that you don't sample invisible stuff, i.e. don't sample the X in
> XRGB8888 (which is what vkms uses internally right now), and to not
> sampel the invisible stuff outside of the fb. We could put this into a
> drm_framebuffer_helper.c, Noralf has a bunch of other related fb
> helper functions in tinydrm which would also neatly fit in there.
> 
> It could be that this isn't fast enough to keep up with vblank, but
> for all the single-shot crc tests it should be good enough. Old igt
> had the mishabit of asking for multiple crcs (to hack around i915
> issues), but I thought we've addressed all that when creating the
> generic debugfs crc interfaces. The other issue is that you probably
> can't enable writeback in all cases (some chips you can enable
> writeback as a sidechannel to any active crtc, some need a dedicated
> thing). I do think igt can cope with "sry no crc for this config"
> though and should skip these (sub)tests if it's just not possible.
> 
> Rewriting all of igt to make writeback-only display blocks work with
> them sounds like the wrong approach. Or at least one we should try
> really hard to avoid.
> 

It's an option. I wasn't super comfortable with the kernel doing so
much "stuff" under-the-hood, in terms of allocating another fb,
having some bandwidth available, having a datapath available...

Then I wasn't sure how to make debugfs hook in to the atomic commit
pathway, triggering a new commit with the framebuffer when userspace
turns CRC on and trying not to tread on the toes of the thing calling
the actual commit ioctl(). I think it would be pretty invasive.

Even after that, the semantics may be different enough (e.g. writeback
is oneshot) that tests would need to be changed anyway.

> > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > >
> > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > didn't have cursor planes, before I could even think about writing
> > > > > tests.
> > > > >
> > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > requirement for merging KMS kernel API.
> > >
> > > With a night's worth of sleep, this here is what annoys me - you
> > > demand I "prove" that igt works everywhere. That's not realistic.
> > > Intel is not going to pay the bill to get a CI farm for every drm
> > > driver existing up&running, including fixing all the bugs that will
> > > uncover (both in igt and even more so in drivers). Especially not if
> > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > do that. If there are some fundamental overall issues with igt then
> > > I'm ofc happy to get them addressed (like the build issues raised a
> > > few months ago).
> >
> > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > not being deliberately awkward.
> >
> > I genuinely don't know what the current state of "not-i915" testing is
> > in igt. Do you really not think it's a good idea to have that known
> > and documented before you make igt a mandatory part of the DRM tree?
> >
> > Sure, there's commits from non-intel folks - heck there's even a few
> > from me. But I can tell you right away that doesn't mean that igt
> > works in any meaningful way on my platform. Does it work well enough
> > for me to add new test cases? Honestly I don't know.
> >
> > Maybe you can suggest some suites which are expected to already be
> > fully generic and should run anywhere which we can use as
> > confirmation? To me, having some reasonable subset of the active
> > driver devs building and running that on their platforms and reporting
> > back before you merge this patch wouldn't be unreasonable or
> > outlandish.
> 
> So my Grand Plan is to get vkms up to par, and even get that
> integrated into igt and drm autobuilds we'll run on gitlab CI. But
> vkms is still a few internships away from being useful, atm it has one
> primary plane and one non-plane cursor (it's not even a universal
> cursor plane yet). Will give you _lots_ of skips.
> 
> But I guess we could fairly easily add vkms as one of the machines
> we're testing to intel's CI farm, which means every patch and every
> commit gets at least tested on non-i915, and you'll get tons of data
> of which tests blow up, and even more which will just skip (due to
> lack of features on vkms' side). We did have that situation a while
> back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> accident (we do still run i915+amdgpu buffer sharing tests, since we
> care about that). Took us a while to realize that igt preferred
> running on amdgpu somehow. Running random non-i915 chips in our CI
> farm isn't something that'll happen, but vkms should not be hard to
> set up and get going.
> 
> Would that be useful if we get that done first?

Sure, that would be useful. I'm not even saying I want it in CI
though, just for some people who aren't @intel.com to try it on their
HW and say "works for me".

> 
> > > There's also the options to somehow find funding for an igt clone, but
> > > if you just find a bit of fund then realistically it's more effective
> > > to throw it at igt. That's how google funded making igt work on
> > > non-i915 drivers at least. Or you manage to open source one of the
> > > existing internal/proprietary test suites, but I think that's going to
> > > be worse than a fresh clone since on top of finding money you also
> > > have to wait a few years for legal review.
> > > -Danie
> > >
> > >
> > > > I don't think this is realistic (or Liviu's take on this): Expecting
> > > > that igt just works on all drivers, and that driver writers don't have
> > > > to do some ramp-up on a testsuite code base is just not going to
> > > > happen. If we go with a testsuite then there
> > > > - will be some ramp up required for people who've never used it
> > > > - it won't work on every driver under the sun
> >
> > That's fair, and I'll grant you that it's hard to push people to do
> > that ramp-up before it's mandatory.
> >
> > > >
> > > > The later could perhaps be fixed if we go khronos-style on uapi
> > > > design, but pls note that the requirements for ratifying new khr
> > > > extensions are _much_ stricter than what we have right now, or what's
> > > > proposed here. Plus khr essentially got the testsuite for old gl
> > > > versions donated by google to them (they still don't have one for
> > > > legacy gl, only for modern gl and all versions of gles).
> >
> > meh. There's both advantages and disadvantages to that. I do think
> > there's a lot of scope for more concrete API design docs.
> >
> > > >
> > > > Also, the fundamental difference between igt and all these other kms
> > > > test suites is that they're all not open source. We're not going to
> > > > use a closed source to validate i915, nor are we going to pour
> > > > engineering resources into that.
> > > >
> >
> > Yes, obviously we can't use a closed codebase as the test suite for
> > DRM. I don't think anyone has/would/will suggest that. I was making an
> > analogy, that if I came to you with an open test-suite, which was
> > written by Arm and which didn't run on your hardware, and said it was
> > about to become mandatory, you might have some complaints.
> 
> Intel invests _lots_ of money into being first with open source. If
> they wouldn't, I'd probably be working somewhere else and fighting for
> that vendor's test suite :-) So I think it's not really symmetrical
> situation. Heck if my mail would be daniel.vetter@arm.com I'd be busy
> ramping arm validation up on igt and killing any internal test suites.
> Aside: I've already killed 2 such tests suites here at intel, I'm good
> at this ...
> 

As you say, we clearly aren't in a symmetrical situation. I can only
tell you how it looks from my side.

> > > > So from a theoretical pov I think your argument has some merit, in
> > > > practice it boils down to "let's not have a test suite, mkay". Is that
> > > > your stance here?
> >
> > I think you're twisting my words a bit far there. That's clearly not
> > what I'm saying. I was quite explicit about that point:
> >
> >         > I'm all for [...] a central KMS test suite.
> >
> > If I could magically wish for anything I like, it would be for a
> > test-suite which was built from the ground up to be *the* generic KMS
> > test-suite.
> >
> > We don't live in a magical world, so igt makes a good starting point.
> > I'm not disputing that. I'm just asking for a sanity check of your
> > assertions that igt is "ready" today.
> 
> I guess it boils down to what a clear goal for "ready" is, which is
> useful, concrete and realistic. "Get vkms into igt CI" would be a
> useful thing, and something that imo makes tons of sense. It's been on
> my plan anyway. "Prove that igt is ready for everyone" otoh is too
> vague to be actionable.

Actionable is good, this would be my suggestion for actionable:

> > Maybe you can suggest some suites which are expected to already be
> > fully generic and should run anywhere which we can use as
> > confirmation? To me, having some reasonable subset of the active
> > driver devs building and running that on their platforms and reporting
> > back before you merge this patch wouldn't be unreasonable or
> > outlandish.

The fuzzy bit is "reasonable subset": maybe amdgpu, vc4, msm, mali-dp,
vkms? Perhaps one suite with CRC and one without?

-Brian

> 
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
>
> On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
>
> [snip]
>
> > >
> > > That doesn't really address my issue, but no matter. I guess I'm
> > > having a hard time separating the existing igts from _new_ igts for
> > > new features; so sorry about that.
> > >
> > > Please appreciate that there's honestly a whole lot of work needed to
> > > make existing igts work for CRC using writeback. There's no "nonblock"
> > > or continuous CRC collection using writeback. You also need to request
> > > your CRC before you make your commit, so that the FB can be attached
> > > to the writeback connector. Fixing this up isn't small or trivial,
> > > it's not just a case of "ramp-up", it's pervasive throughout the igt
> > > tree.
> > >
> > > That shouldn't need to impact your proposal for new tests, if they are
> > > written with that in mind.
> >
> > Hm, I've figured the simplest way to make this happen is to enable
> > writeback from the debugfs crc interface, and instead of just
> > collecting the crc from the writeback completion interrupt, you first
> > feed the entire framebuffer into a checksum function. That code
> > shouldn't need a full rewrite of igt, and is kinda how I envisaged
> > writeback-for-igt-crc would pan out.
> >
> > It's at least how we've done it for vkms, and there the
> > compress-fb-into-crc is a few lines of code. Most important bit is
> > that you don't sample invisible stuff, i.e. don't sample the X in
> > XRGB8888 (which is what vkms uses internally right now), and to not
> > sampel the invisible stuff outside of the fb. We could put this into a
> > drm_framebuffer_helper.c, Noralf has a bunch of other related fb
> > helper functions in tinydrm which would also neatly fit in there.
> >
> > It could be that this isn't fast enough to keep up with vblank, but
> > for all the single-shot crc tests it should be good enough. Old igt
> > had the mishabit of asking for multiple crcs (to hack around i915
> > issues), but I thought we've addressed all that when creating the
> > generic debugfs crc interfaces. The other issue is that you probably
> > can't enable writeback in all cases (some chips you can enable
> > writeback as a sidechannel to any active crtc, some need a dedicated
> > thing). I do think igt can cope with "sry no crc for this config"
> > though and should skip these (sub)tests if it's just not possible.
> >
> > Rewriting all of igt to make writeback-only display blocks work with
> > them sounds like the wrong approach. Or at least one we should try
> > really hard to avoid.
> >
>
> It's an option. I wasn't super comfortable with the kernel doing so
> much "stuff" under-the-hood, in terms of allocating another fb,
> having some bandwidth available, having a datapath available...
>
> Then I wasn't sure how to make debugfs hook in to the atomic commit
> pathway, triggering a new commit with the framebuffer when userspace
> turns CRC on and trying not to tread on the toes of the thing calling
> the actual commit ioctl(). I think it would be pretty invasive.
>
> Even after that, the semantics may be different enough (e.g. writeback
> is oneshot) that tests would need to be changed anyway.

You'd need to rearm it somehow ofc, which probably needs some hacks to
bypass atomic. Wrt just doing an atomic commit: i915 already does that
to apply some workarounds for our crc generation. igt seems to cope
with an atomic commit happening in the background when setting up the
igt, so that's not a problem.

> > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > >
> > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > tests.
> > > > > >
> > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > requirement for merging KMS kernel API.
> > > >
> > > > With a night's worth of sleep, this here is what annoys me - you
> > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > driver existing up&running, including fixing all the bugs that will
> > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > do that. If there are some fundamental overall issues with igt then
> > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > few months ago).
> > >
> > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > not being deliberately awkward.
> > >
> > > I genuinely don't know what the current state of "not-i915" testing is
> > > in igt. Do you really not think it's a good idea to have that known
> > > and documented before you make igt a mandatory part of the DRM tree?
> > >
> > > Sure, there's commits from non-intel folks - heck there's even a few
> > > from me. But I can tell you right away that doesn't mean that igt
> > > works in any meaningful way on my platform. Does it work well enough
> > > for me to add new test cases? Honestly I don't know.
> > >
> > > Maybe you can suggest some suites which are expected to already be
> > > fully generic and should run anywhere which we can use as
> > > confirmation? To me, having some reasonable subset of the active
> > > driver devs building and running that on their platforms and reporting
> > > back before you merge this patch wouldn't be unreasonable or
> > > outlandish.
> >
> > So my Grand Plan is to get vkms up to par, and even get that
> > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > vkms is still a few internships away from being useful, atm it has one
> > primary plane and one non-plane cursor (it's not even a universal
> > cursor plane yet). Will give you _lots_ of skips.
> >
> > But I guess we could fairly easily add vkms as one of the machines
> > we're testing to intel's CI farm, which means every patch and every
> > commit gets at least tested on non-i915, and you'll get tons of data
> > of which tests blow up, and even more which will just skip (due to
> > lack of features on vkms' side). We did have that situation a while
> > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > care about that). Took us a while to realize that igt preferred
> > running on amdgpu somehow. Running random non-i915 chips in our CI
> > farm isn't something that'll happen, but vkms should not be hard to
> > set up and get going.
> >
> > Would that be useful if we get that done first?
>
> Sure, that would be useful. I'm not even saying I want it in CI
> though, just for some people who aren't @intel.com to try it on their
> HW and say "works for me".

There's a pretty huge difference between "I want acks from other
people" and "you need to prove this works everywhere" ... The prove
thing pretty much requires CI, or it's imo not really anywhere near
proving.

> > > > There's also the options to somehow find funding for an igt clone, but
> > > > if you just find a bit of fund then realistically it's more effective
> > > > to throw it at igt. That's how google funded making igt work on
> > > > non-i915 drivers at least. Or you manage to open source one of the
> > > > existing internal/proprietary test suites, but I think that's going to
> > > > be worse than a fresh clone since on top of finding money you also
> > > > have to wait a few years for legal review.
> > > > -Danie
> > > >
> > > >
> > > > > I don't think this is realistic (or Liviu's take on this): Expecting
> > > > > that igt just works on all drivers, and that driver writers don't have
> > > > > to do some ramp-up on a testsuite code base is just not going to
> > > > > happen. If we go with a testsuite then there
> > > > > - will be some ramp up required for people who've never used it
> > > > > - it won't work on every driver under the sun
> > >
> > > That's fair, and I'll grant you that it's hard to push people to do
> > > that ramp-up before it's mandatory.
> > >
> > > > >
> > > > > The later could perhaps be fixed if we go khronos-style on uapi
> > > > > design, but pls note that the requirements for ratifying new khr
> > > > > extensions are _much_ stricter than what we have right now, or what's
> > > > > proposed here. Plus khr essentially got the testsuite for old gl
> > > > > versions donated by google to them (they still don't have one for
> > > > > legacy gl, only for modern gl and all versions of gles).
> > >
> > > meh. There's both advantages and disadvantages to that. I do think
> > > there's a lot of scope for more concrete API design docs.
> > >
> > > > >
> > > > > Also, the fundamental difference between igt and all these other kms
> > > > > test suites is that they're all not open source. We're not going to
> > > > > use a closed source to validate i915, nor are we going to pour
> > > > > engineering resources into that.
> > > > >
> > >
> > > Yes, obviously we can't use a closed codebase as the test suite for
> > > DRM. I don't think anyone has/would/will suggest that. I was making an
> > > analogy, that if I came to you with an open test-suite, which was
> > > written by Arm and which didn't run on your hardware, and said it was
> > > about to become mandatory, you might have some complaints.
> >
> > Intel invests _lots_ of money into being first with open source. If
> > they wouldn't, I'd probably be working somewhere else and fighting for
> > that vendor's test suite :-) So I think it's not really symmetrical
> > situation. Heck if my mail would be daniel.vetter@arm.com I'd be busy
> > ramping arm validation up on igt and killing any internal test suites.
> > Aside: I've already killed 2 such tests suites here at intel, I'm good
> > at this ...
> >
>
> As you say, we clearly aren't in a symmetrical situation. I can only
> tell you how it looks from my side.
>
> > > > > So from a theoretical pov I think your argument has some merit, in
> > > > > practice it boils down to "let's not have a test suite, mkay". Is that
> > > > > your stance here?
> > >
> > > I think you're twisting my words a bit far there. That's clearly not
> > > what I'm saying. I was quite explicit about that point:
> > >
> > >         > I'm all for [...] a central KMS test suite.
> > >
> > > If I could magically wish for anything I like, it would be for a
> > > test-suite which was built from the ground up to be *the* generic KMS
> > > test-suite.
> > >
> > > We don't live in a magical world, so igt makes a good starting point.
> > > I'm not disputing that. I'm just asking for a sanity check of your
> > > assertions that igt is "ready" today.
> >
> > I guess it boils down to what a clear goal for "ready" is, which is
> > useful, concrete and realistic. "Get vkms into igt CI" would be a
> > useful thing, and something that imo makes tons of sense. It's been on
> > my plan anyway. "Prove that igt is ready for everyone" otoh is too
> > vague to be actionable.
>
> Actionable is good, this would be my suggestion for actionable:
>
> > > Maybe you can suggest some suites which are expected to already be
> > > fully generic and should run anywhere which we can use as
> > > confirmation? To me, having some reasonable subset of the active
> > > driver devs building and running that on their platforms and reporting
> > > back before you merge this patch wouldn't be unreasonable or
> > > outlandish.
>
> The fuzzy bit is "reasonable subset": maybe amdgpu, vc4, msm, mali-dp,
> vkms? Perhaps one suite with CRC and one without?

With this we're back to "Daniel, pls make sure igt works on all
drivers". In theory that sounds reasonable, in practice it means no
testsuite. vkms I can make happen I think, no problem. Also
vkms-without-some-features (like no crc - would need to be added, or
no vblank - already there), if people think that's useful. More, you
need to hire me :-)

If you just mean that I should make sure there's plenty of acks from
all these people, that's already a given.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Sean Paul]

On Tue, Jan 22, 2019 at 04:17:25PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> >

/snip

> 
> > > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > > >
> > > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > > tests.
> > > > > > >
> > > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > > requirement for merging KMS kernel API.
> > > > >
> > > > > With a night's worth of sleep, this here is what annoys me - you
> > > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > > driver existing up&running, including fixing all the bugs that will
> > > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > > do that. If there are some fundamental overall issues with igt then
> > > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > > few months ago).
> > > >
> > > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > > not being deliberately awkward.
> > > >
> > > > I genuinely don't know what the current state of "not-i915" testing is
> > > > in igt. Do you really not think it's a good idea to have that known
> > > > and documented before you make igt a mandatory part of the DRM tree?
> > > >
> > > > Sure, there's commits from non-intel folks - heck there's even a few
> > > > from me. But I can tell you right away that doesn't mean that igt
> > > > works in any meaningful way on my platform. Does it work well enough
> > > > for me to add new test cases? Honestly I don't know.
> > > >
> > > > Maybe you can suggest some suites which are expected to already be
> > > > fully generic and should run anywhere which we can use as
> > > > confirmation? To me, having some reasonable subset of the active
> > > > driver devs building and running that on their platforms and reporting
> > > > back before you merge this patch wouldn't be unreasonable or
> > > > outlandish.
> > >
> > > So my Grand Plan is to get vkms up to par, and even get that
> > > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > > vkms is still a few internships away from being useful, atm it has one
> > > primary plane and one non-plane cursor (it's not even a universal
> > > cursor plane yet). Will give you _lots_ of skips.
> > >
> > > But I guess we could fairly easily add vkms as one of the machines
> > > we're testing to intel's CI farm, which means every patch and every
> > > commit gets at least tested on non-i915, and you'll get tons of data
> > > of which tests blow up, and even more which will just skip (due to
> > > lack of features on vkms' side). We did have that situation a while
> > > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > > care about that). Took us a while to realize that igt preferred
> > > running on amdgpu somehow. Running random non-i915 chips in our CI
> > > farm isn't something that'll happen, but vkms should not be hard to
> > > set up and get going.
> > >
> > > Would that be useful if we get that done first?
> >
> > Sure, that would be useful. I'm not even saying I want it in CI
> > though, just for some people who aren't @intel.com to try it on their
> > HW and say "works for me".
> 
> There's a pretty huge difference between "I want acks from other
> people" and "you need to prove this works everywhere" ... The prove
> thing pretty much requires CI, or it's imo not really anywhere near
> proving.

My $0.02 since my interests are:
- Make sure all of our (CrOS) platforms are well-tested, and
- Ensure new features can be merged upstream without too much overhead

My initial reaction was the same as Brian's. I've "brought up" igt on a non-i915
device and it was a bit of a horror show, but I managed to get the test I wanted
running and lit the device on fire afterwards. Since then at least the toolchain
has been fixed and igt builds, so that's nice.

That said, the counterarguments presented have been pretty hypothetical. I get
that crc is a pain for ARM (Arm? arm? aRm?), but it seems like a worthwhile
investment to get it working so that they can leverage igt. Even if crc
generation is laggy. If, hypothetically, ARM has a new feature that requires an
igt test using crc, it seems like a good time to get crc working so the new
feature can be proven out.

I think we should at least have a solid plan for crc generation on ARM (that ARM
is happy with) before requiring igt across the board. That can be translated to
a TODO, and perhaps someone will be interested in picking that up.

Sean

/snip

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Tue, Jan 22, 2019 at 5:11 PM Sean Paul <sean@poorly.run> wrote:
> On Tue, Jan 22, 2019 at 04:17:25PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > >
> > > On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > > > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > >
>
> /snip
>
> >
> > > > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > > > >
> > > > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > > > tests.
> > > > > > > >
> > > > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > > > requirement for merging KMS kernel API.
> > > > > >
> > > > > > With a night's worth of sleep, this here is what annoys me - you
> > > > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > > > driver existing up&running, including fixing all the bugs that will
> > > > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > > > do that. If there are some fundamental overall issues with igt then
> > > > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > > > few months ago).
> > > > >
> > > > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > > > not being deliberately awkward.
> > > > >
> > > > > I genuinely don't know what the current state of "not-i915" testing is
> > > > > in igt. Do you really not think it's a good idea to have that known
> > > > > and documented before you make igt a mandatory part of the DRM tree?
> > > > >
> > > > > Sure, there's commits from non-intel folks - heck there's even a few
> > > > > from me. But I can tell you right away that doesn't mean that igt
> > > > > works in any meaningful way on my platform. Does it work well enough
> > > > > for me to add new test cases? Honestly I don't know.
> > > > >
> > > > > Maybe you can suggest some suites which are expected to already be
> > > > > fully generic and should run anywhere which we can use as
> > > > > confirmation? To me, having some reasonable subset of the active
> > > > > driver devs building and running that on their platforms and reporting
> > > > > back before you merge this patch wouldn't be unreasonable or
> > > > > outlandish.
> > > >
> > > > So my Grand Plan is to get vkms up to par, and even get that
> > > > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > > > vkms is still a few internships away from being useful, atm it has one
> > > > primary plane and one non-plane cursor (it's not even a universal
> > > > cursor plane yet). Will give you _lots_ of skips.
> > > >
> > > > But I guess we could fairly easily add vkms as one of the machines
> > > > we're testing to intel's CI farm, which means every patch and every
> > > > commit gets at least tested on non-i915, and you'll get tons of data
> > > > of which tests blow up, and even more which will just skip (due to
> > > > lack of features on vkms' side). We did have that situation a while
> > > > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > > > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > > > care about that). Took us a while to realize that igt preferred
> > > > running on amdgpu somehow. Running random non-i915 chips in our CI
> > > > farm isn't something that'll happen, but vkms should not be hard to
> > > > set up and get going.
> > > >
> > > > Would that be useful if we get that done first?
> > >
> > > Sure, that would be useful. I'm not even saying I want it in CI
> > > though, just for some people who aren't @intel.com to try it on their
> > > HW and say "works for me".
> >
> > There's a pretty huge difference between "I want acks from other
> > people" and "you need to prove this works everywhere" ... The prove
> > thing pretty much requires CI, or it's imo not really anywhere near
> > proving.
>
> My $0.02 since my interests are:
> - Make sure all of our (CrOS) platforms are well-tested, and
> - Ensure new features can be merged upstream without too much overhead
>
> My initial reaction was the same as Brian's. I've "brought up" igt on a non-i915
> device and it was a bit of a horror show, but I managed to get the test I wanted
> running and lit the device on fire afterwards. Since then at least the toolchain
> has been fixed and igt builds, so that's nice.
>
> That said, the counterarguments presented have been pretty hypothetical. I get
> that crc is a pain for ARM (Arm? arm? aRm?), but it seems like a worthwhile
> investment to get it working so that they can leverage igt. Even if crc
> generation is laggy. If, hypothetically, ARM has a new feature that requires an
> igt test using crc, it seems like a good time to get crc working so the new
> feature can be proven out.
>
> I think we should at least have a solid plan for crc generation on ARM (that ARM
> is happy with) before requiring igt across the board. That can be translated to
> a TODO, and perhaps someone will be interested in picking that up.

One thing to keep in mind is that this is only about _new_ features.
In my experience (and I killed a fair share of internal test suites)
that's the only way to get out of the chicken&egg problem if you have
a large project without a test suite. The second you start requiring
that the tests work even on old stuff, and have good coverage on
existing features your test suite is dead:
- no one is going to abandon their investement into existing test suites
- no one is going to be able to afford rewriting their existing tests in one go
- no one is going to fix all the bugs this will uncover

So requiring that igt works for everyone kills this idea of having a
shared test suite for kms. Which I think wouldn't be great, but I'm
also not going to push for this if this is how the community overall
feels like.

The only thing that works is limiting yourself on new stuff only, and
pretending the the existing dumpster fire of untested features, broken
drivers and not-generic enough tests doesn't exist. And I think igt
for non-i915 drivers is in a much better shape than igt was 5+ years
ago when we made it mandatory for anything intel does in the kernel.
Over the course of the past 5 years we've managed to mostly address
the gaps in igt from an intel pov. I think that's about the same time
I expect anyone else switching over to igt will need to fully get rid
of their existing test suite and integrated it all into igt. So again:
Requiring that work first just means we perpetually postpone having a
shared test suite by 5 years, forever (there's going to be new drivers
which then again wont work with igt). And most likely it won't happen,
because there's no incentive to switch over at all.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Liviu Dudau]

On Tue, Jan 22, 2019 at 05:28:19PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 5:11 PM Sean Paul <sean@poorly.run> wrote:
> > On Tue, Jan 22, 2019 at 04:17:25PM +0100, Daniel Vetter wrote:
> > > On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > > >
> > > > On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > > > > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > > >
> >
> > /snip
> >
> > >
> > > > > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > > > > >
> > > > > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > > > > tests.
> > > > > > > > >
> > > > > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > > > > requirement for merging KMS kernel API.
> > > > > > >
> > > > > > > With a night's worth of sleep, this here is what annoys me - you
> > > > > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > > > > driver existing up&running, including fixing all the bugs that will
> > > > > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > > > > do that. If there are some fundamental overall issues with igt then
> > > > > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > > > > few months ago).
> > > > > >
> > > > > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > > > > not being deliberately awkward.
> > > > > >
> > > > > > I genuinely don't know what the current state of "not-i915" testing is
> > > > > > in igt. Do you really not think it's a good idea to have that known
> > > > > > and documented before you make igt a mandatory part of the DRM tree?
> > > > > >
> > > > > > Sure, there's commits from non-intel folks - heck there's even a few
> > > > > > from me. But I can tell you right away that doesn't mean that igt
> > > > > > works in any meaningful way on my platform. Does it work well enough
> > > > > > for me to add new test cases? Honestly I don't know.
> > > > > >
> > > > > > Maybe you can suggest some suites which are expected to already be
> > > > > > fully generic and should run anywhere which we can use as
> > > > > > confirmation? To me, having some reasonable subset of the active
> > > > > > driver devs building and running that on their platforms and reporting
> > > > > > back before you merge this patch wouldn't be unreasonable or
> > > > > > outlandish.
> > > > >
> > > > > So my Grand Plan is to get vkms up to par, and even get that
> > > > > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > > > > vkms is still a few internships away from being useful, atm it has one
> > > > > primary plane and one non-plane cursor (it's not even a universal
> > > > > cursor plane yet). Will give you _lots_ of skips.
> > > > >
> > > > > But I guess we could fairly easily add vkms as one of the machines
> > > > > we're testing to intel's CI farm, which means every patch and every
> > > > > commit gets at least tested on non-i915, and you'll get tons of data
> > > > > of which tests blow up, and even more which will just skip (due to
> > > > > lack of features on vkms' side). We did have that situation a while
> > > > > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > > > > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > > > > care about that). Took us a while to realize that igt preferred
> > > > > running on amdgpu somehow. Running random non-i915 chips in our CI
> > > > > farm isn't something that'll happen, but vkms should not be hard to
> > > > > set up and get going.
> > > > >
> > > > > Would that be useful if we get that done first?
> > > >
> > > > Sure, that would be useful. I'm not even saying I want it in CI
> > > > though, just for some people who aren't @intel.com to try it on their
> > > > HW and say "works for me".
> > >
> > > There's a pretty huge difference between "I want acks from other
> > > people" and "you need to prove this works everywhere" ... The prove
> > > thing pretty much requires CI, or it's imo not really anywhere near
> > > proving.
> >
> > My $0.02 since my interests are:
> > - Make sure all of our (CrOS) platforms are well-tested, and
> > - Ensure new features can be merged upstream without too much overhead
> >
> > My initial reaction was the same as Brian's. I've "brought up" igt on a non-i915
> > device and it was a bit of a horror show, but I managed to get the test I wanted
> > running and lit the device on fire afterwards. Since then at least the toolchain
> > has been fixed and igt builds, so that's nice.
> >
> > That said, the counterarguments presented have been pretty hypothetical. I get
> > that crc is a pain for ARM (Arm? arm? aRm?), but it seems like a worthwhile
> > investment to get it working so that they can leverage igt. Even if crc
> > generation is laggy. If, hypothetically, ARM has a new feature that requires an
> > igt test using crc, it seems like a good time to get crc working so the new
> > feature can be proven out.
> >
> > I think we should at least have a solid plan for crc generation on ARM (that ARM
> > is happy with) before requiring igt across the board. That can be translated to
> > a TODO, and perhaps someone will be interested in picking that up.
> 
> One thing to keep in mind is that this is only about _new_ features.
> In my experience (and I killed a fair share of internal test suites)
> that's the only way to get out of the chicken&egg problem if you have
> a large project without a test suite. The second you start requiring
> that the tests work even on old stuff, and have good coverage on
> existing features your test suite is dead:
> - no one is going to abandon their investement into existing test suites
> - no one is going to be able to afford rewriting their existing tests in one go
> - no one is going to fix all the bugs this will uncover
> 
> So requiring that igt works for everyone kills this idea of having a
> shared test suite for kms. Which I think wouldn't be great, but I'm
> also not going to push for this if this is how the community overall
> feels like.

Sorry for resurecting this thread, I was on holiday for a week but I
want to keep my promise of keeping Daniel on an honest path as a DRM
co-maintainer:

Daniel, if you had that previous experience and if you know/knew that
"requiring that igt works for everyone would kill the idea", I think you
should have been more clear and forthcoming with the information and
also made it more explicit that you're trying to get *something* in place
and that means being forgiving with the past but less so with the future
APIs. It would have cut tremendously on the number of messages exchanged
between Arm-side and you.

Without being judgemental, from my side I'm seeing a disconnect between
what is being claimed that igt can do today and what I know I can extract
from igt on my driver. As a result, it makes me wonder if it is just the
Arm-developed drivers' problem or is it true for most of the non-PC drivers.
That was the reason for asking for results from other drivers: so that we
know where we are and plan for change. I obviously don't want to be left
behind but I also face the reality that being one of the first to bend igt
to work on my platforms requires effort that I need to justify to my management.
And I can only do so when I have information readily at my disposal so that
I can point them to some test runs that show that others are doing better than
us, or that we are failing in important areas. We're not trying to hold
anyone hostage, nor trying to kill the idea. We just need collaboration
and fair grounds.

That being said, if you feel that this thread has dragged on for too long,
please ignore this reply even if I will use it in the future for reference ;)

I would also like to add that I'm happy with the resolution presented last
week, when it was agreed that the change should say "should" and also contain
some blurb about the igt tests being created for API changes "when appropriate".

Best regards,
Liviu

> 
> The only thing that works is limiting yourself on new stuff only, and
> pretending the the existing dumpster fire of untested features, broken
> drivers and not-generic enough tests doesn't exist. And I think igt
> for non-i915 drivers is in a much better shape than igt was 5+ years
> ago when we made it mandatory for anything intel does in the kernel.
> Over the course of the past 5 years we've managed to mostly address
> the gaps in igt from an intel pov. I think that's about the same time
> I expect anyone else switching over to igt will need to fully get rid
> of their existing test suite and integrated it all into igt. So again:
> Requiring that work first just means we perpetually postpone having a
> shared test suite by 5 years, forever (there's going to be new drivers
> which then again wont work with igt). And most likely it won't happen,
> because there's no incentive to switch over at all.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

Hi Liviu,

On Mon, Jan 28, 2019 at 3:03 PM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>
> On Tue, Jan 22, 2019 at 05:28:19PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 5:11 PM Sean Paul <sean@poorly.run> wrote:
> > > On Tue, Jan 22, 2019 at 04:17:25PM +0100, Daniel Vetter wrote:
> > > > On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > > > >
> > > > > On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > > > > > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey@arm.com> wrote:
> > > > >
> > >
> > > /snip
> > >
> > > >
> > > > > > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > > > > > >
> > > > > > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > > > > > tests.
> > > > > > > > > >
> > > > > > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > > > > > requirement for merging KMS kernel API.
> > > > > > > >
> > > > > > > > With a night's worth of sleep, this here is what annoys me - you
> > > > > > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > > > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > > > > > driver existing up&running, including fixing all the bugs that will
> > > > > > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > > > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > > > > > do that. If there are some fundamental overall issues with igt then
> > > > > > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > > > > > few months ago).
> > > > > > >
> > > > > > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > > > > > not being deliberately awkward.
> > > > > > >
> > > > > > > I genuinely don't know what the current state of "not-i915" testing is
> > > > > > > in igt. Do you really not think it's a good idea to have that known
> > > > > > > and documented before you make igt a mandatory part of the DRM tree?
> > > > > > >
> > > > > > > Sure, there's commits from non-intel folks - heck there's even a few
> > > > > > > from me. But I can tell you right away that doesn't mean that igt
> > > > > > > works in any meaningful way on my platform. Does it work well enough
> > > > > > > for me to add new test cases? Honestly I don't know.
> > > > > > >
> > > > > > > Maybe you can suggest some suites which are expected to already be
> > > > > > > fully generic and should run anywhere which we can use as
> > > > > > > confirmation? To me, having some reasonable subset of the active
> > > > > > > driver devs building and running that on their platforms and reporting
> > > > > > > back before you merge this patch wouldn't be unreasonable or
> > > > > > > outlandish.
> > > > > >
> > > > > > So my Grand Plan is to get vkms up to par, and even get that
> > > > > > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > > > > > vkms is still a few internships away from being useful, atm it has one
> > > > > > primary plane and one non-plane cursor (it's not even a universal
> > > > > > cursor plane yet). Will give you _lots_ of skips.
> > > > > >
> > > > > > But I guess we could fairly easily add vkms as one of the machines
> > > > > > we're testing to intel's CI farm, which means every patch and every
> > > > > > commit gets at least tested on non-i915, and you'll get tons of data
> > > > > > of which tests blow up, and even more which will just skip (due to
> > > > > > lack of features on vkms' side). We did have that situation a while
> > > > > > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > > > > > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > > > > > care about that). Took us a while to realize that igt preferred
> > > > > > running on amdgpu somehow. Running random non-i915 chips in our CI
> > > > > > farm isn't something that'll happen, but vkms should not be hard to
> > > > > > set up and get going.
> > > > > >
> > > > > > Would that be useful if we get that done first?
> > > > >
> > > > > Sure, that would be useful. I'm not even saying I want it in CI
> > > > > though, just for some people who aren't @intel.com to try it on their
> > > > > HW and say "works for me".
> > > >
> > > > There's a pretty huge difference between "I want acks from other
> > > > people" and "you need to prove this works everywhere" ... The prove
> > > > thing pretty much requires CI, or it's imo not really anywhere near
> > > > proving.
> > >
> > > My $0.02 since my interests are:
> > > - Make sure all of our (CrOS) platforms are well-tested, and
> > > - Ensure new features can be merged upstream without too much overhead
> > >
> > > My initial reaction was the same as Brian's. I've "brought up" igt on a non-i915
> > > device and it was a bit of a horror show, but I managed to get the test I wanted
> > > running and lit the device on fire afterwards. Since then at least the toolchain
> > > has been fixed and igt builds, so that's nice.
> > >
> > > That said, the counterarguments presented have been pretty hypothetical. I get
> > > that crc is a pain for ARM (Arm? arm? aRm?), but it seems like a worthwhile
> > > investment to get it working so that they can leverage igt. Even if crc
> > > generation is laggy. If, hypothetically, ARM has a new feature that requires an
> > > igt test using crc, it seems like a good time to get crc working so the new
> > > feature can be proven out.
> > >
> > > I think we should at least have a solid plan for crc generation on ARM (that ARM
> > > is happy with) before requiring igt across the board. That can be translated to
> > > a TODO, and perhaps someone will be interested in picking that up.
> >
> > One thing to keep in mind is that this is only about _new_ features.
> > In my experience (and I killed a fair share of internal test suites)
> > that's the only way to get out of the chicken&egg problem if you have
> > a large project without a test suite. The second you start requiring
> > that the tests work even on old stuff, and have good coverage on
> > existing features your test suite is dead:
> > - no one is going to abandon their investement into existing test suites
> > - no one is going to be able to afford rewriting their existing tests in one go
> > - no one is going to fix all the bugs this will uncover
> >
> > So requiring that igt works for everyone kills this idea of having a
> > shared test suite for kms. Which I think wouldn't be great, but I'm
> > also not going to push for this if this is how the community overall
> > feels like.
>
> Sorry for resurecting this thread, I was on holiday for a week but I
> want to keep my promise of keeping Daniel on an honest path as a DRM
> co-maintainer:
>
> Daniel, if you had that previous experience and if you know/knew that
> "requiring that igt works for everyone would kill the idea", I think you
> should have been more clear and forthcoming with the information and
> also made it more explicit that you're trying to get *something* in place
> and that means being forgiving with the past but less so with the future
> APIs. It would have cut tremendously on the number of messages exchanged
> between Arm-side and you.

Yeah, I kinda assumed people knew some of those war-stories of trying
to make a test suite happen for an existing software stack. igt
ramping up for kms over the past few years, khr trying to get opengl
validated, mesa starting to use piglit a bit ahead of that, kernel
selftests. It's a very tough chicken&egg problem.

> Without being judgemental, from my side I'm seeing a disconnect between
> what is being claimed that igt can do today and what I know I can extract
> from igt on my driver. As a result, it makes me wonder if it is just the
> Arm-developed drivers' problem or is it true for most of the non-PC drivers.
> That was the reason for asking for results from other drivers: so that we
> know where we are and plan for change. I obviously don't want to be left
> behind but I also face the reality that being one of the first to bend igt
> to work on my platforms requires effort that I need to justify to my management.
> And I can only do so when I have information readily at my disposal so that
> I can point them to some test runs that show that others are doing better than
> us, or that we are failing in important areas. We're not trying to hold
> anyone hostage, nor trying to kill the idea. We just need collaboration
> and fair grounds.
>
> That being said, if you feel that this thread has dragged on for too long,
> please ignore this reply even if I will use it in the future for reference ;)

Thanks for replying, very much appreciated. I know I'm sounding
occasionally a bit frustrated on this since I want to move forward
quickly (I think we need to make this happen somehow for the long term
benefit of everyone working on upstream kms/drm). But I very much
don't want want to steam roll anyone here.

> I would also like to add that I'm happy with the resolution presented last
> week, when it was agreed that the change should say "should" and also contain
> some blurb about the igt tests being created for API changes "when appropriate".

Yeah I have v3 here with that already, but wanted to wait for your
input. I'll send that out now, but I think I'll leave your ack out so
you can ponder this some more.

Wrt merging: there's no rush here. If we get this in the next few
months I'm going to celebrate, if we get this in this year still I'm
happy, if not, mea culpa. Better to first make sure everyone is on the
same page.

I also think that one result of this discussion on v2 is that we need
some indicators of how healthy igt is on non-i915 platforms. I'll look
into what we can do with intel-gfx-ci (using vkms), but maybe
kernelci.org will have production results displayed in a useable
format first. vkms would be good anyway, to sanity check DRIVER_ANY
testcases I think. Waiting for this shouldn't hold up the great future
we're all aiming for really, and I think it'd be good to so.

I'll send out v3 and we can discuss this a bit more :-)

Cheers, Daniel

>
> Best regards,
> Liviu
>
> >
> > The only thing that works is limiting yourself on new stuff only, and
> > pretending the the existing dumpster fire of untested features, broken
> > drivers and not-generic enough tests doesn't exist. And I think igt
> > for non-i915 drivers is in a much better shape than igt was 5+ years
> > ago when we made it mandatory for anything intel does in the kernel.
> > Over the course of the past 5 years we've managed to mostly address
> > the gaps in igt from an intel pov. I think that's about the same time
> > I expect anyone else switching over to igt will need to fully get rid
> > of their existing test suite and integrated it all into igt. So again:
> > Requiring that work first just means we perpetually postpone having a
> > shared test suite by 5 years, forever (there's going to be new drivers
> > which then again wont work with igt). And most likely it won't happen,
> > because there's no incentive to switch over at all.
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Wentland, Harry]

On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> Compared to the RFC[1] no changes to the patch itself, but igt moved
> forward a lot:
> 
> - gitlab CI builds with: reduced configs/libraries, arm cross build
>   and a sysroot build (should address all the build/cross platform
>   concerns raised in the RFC discussions).
> 
> - tests reorganized into subdirectories so that the i915-gem tests
>   don't clog the main/shared tests directory anymore
> 
> - quite a few more non-intel people contributing/reviewing/committing
>   igt tests patches.
> 
> I think this addresses all the concerns raised in the RFC discussions,
> and assuming there's enough Acks and no new issues that pop up, we can
> go ahead with this.
> 
> 1: https://patchwork.kernel.org/patch/10648851/
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.

No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.

Harry

> ---
>  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 a752aa561ea4..413915d6b7d2 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
>  ---------------------------
>  
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
> On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > forward a lot:
> >
> > - gitlab CI builds with: reduced configs/libraries, arm cross build
> >   and a sysroot build (should address all the build/cross platform
> >   concerns raised in the RFC discussions).
> >
> > - tests reorganized into subdirectories so that the i915-gem tests
> >   don't clog the main/shared tests directory anymore
> >
> > - quite a few more non-intel people contributing/reviewing/committing
> >   igt tests patches.
> >
> > I think this addresses all the concerns raised in the RFC discussions,
> > and assuming there's enough Acks and no new issues that pop up, we can
> > go ahead with this.
> >
> > 1: https://patchwork.kernel.org/patch/10648851/
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
>
> No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.

We also discussed a bit writeback implementations on irc, and it looks
like you can't really use writeback to accurately test that your
compositing engine is programmed correctly, since on at least vc4,
malidp and msm (not yet merged upstream) the writeback engine can't be
shared with any other outputs, often it even needs a
dedicated/special-purpose CRTC (at least vc4 from what I can tell).
That means if you botch your programming and e.g. cause an underrun
scanning out continous-update outputs, then the writeback won't show
that to you, since it's composited separately. I guess we could teach
igt to run these tests on the special crtc->writeback pipeline only,
but essentially that's a new testcase, and not really testing the
actual display: It tests writeback, not hdmi/dp/panels/whatever real
outputs you have.

I'd say we'll shrug these cases off as "can't be reasonable tested,
won't have an igt". First driver team with hw that can be validated
gets to fill the gaps :-) In practice still going to be a lot better
than no tests at all, just exercising the feature will be useful, and
will make it a lot easier for the next team to add the crc based tests
on top.

-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Wentland, Harry]

On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
>> On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
>>> Compared to the RFC[1] no changes to the patch itself, but igt moved
>>> forward a lot:
>>>
>>> - gitlab CI builds with: reduced configs/libraries, arm cross build
>>>   and a sysroot build (should address all the build/cross platform
>>>   concerns raised in the RFC discussions).
>>>
>>> - tests reorganized into subdirectories so that the i915-gem tests
>>>   don't clog the main/shared tests directory anymore
>>>
>>> - quite a few more non-intel people contributing/reviewing/committing
>>>   igt tests patches.
>>>
>>> I think this addresses all the concerns raised in the RFC discussions,
>>> and assuming there's enough Acks and no new issues that pop up, we can
>>> go ahead with this.
>>>
>>> 1: https://patchwork.kernel.org/patch/10648851/
>>> Cc: Petri Latvala <petri.latvala@intel.com>
>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Cc: Sean Paul <sean@poorly.run>
>>> Cc: Eric Anholt <eric@anholt.net>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>
>> I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
>>
>> No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> 
> We also discussed a bit writeback implementations on irc, and it looks
> like you can't really use writeback to accurately test that your
> compositing engine is programmed correctly, since on at least vc4,
> malidp and msm (not yet merged upstream) the writeback engine can't be
> shared with any other outputs, often it even needs a
> dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> That means if you botch your programming and e.g. cause an underrun
> scanning out continous-update outputs, then the writeback won't show
> that to you, since it's composited separately. I guess we could teach
> igt to run these tests on the special crtc->writeback pipeline only,
> but essentially that's a new testcase, and not really testing the
> actual display: It tests writeback, not hdmi/dp/panels/whatever real
> outputs you have.
> 
> I'd say we'll shrug these cases off as "can't be reasonable tested,
> won't have an igt". First driver team with hw that can be validated
> gets to fill the gaps :-) In practice still going to be a lot better
> than no tests at all, just exercising the feature will be useful, and
> will make it a lot easier for the next team to add the crc based tests
> on top.
> 

I think that's reasonable. After all, we want to start somewhere.

Would it make sense to append something like ", if such a test can be reasonably made using IGT for the target HW." to make it clear to contributors that in cases like the one discussed this is at the reviewers discretion?

With that change (or anything else that clarifies your intentions as described above) I'd be happy to give my AB.

Harry

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Sean Paul]

On Tue, Jan 22, 2019 at 07:42:30PM +0000, Wentland, Harry wrote:
> 
> 
> On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
> >> On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> >>> Compared to the RFC[1] no changes to the patch itself, but igt moved
> >>> forward a lot:
> >>>
> >>> - gitlab CI builds with: reduced configs/libraries, arm cross build
> >>>   and a sysroot build (should address all the build/cross platform
> >>>   concerns raised in the RFC discussions).
> >>>
> >>> - tests reorganized into subdirectories so that the i915-gem tests
> >>>   don't clog the main/shared tests directory anymore
> >>>
> >>> - quite a few more non-intel people contributing/reviewing/committing
> >>>   igt tests patches.
> >>>
> >>> I think this addresses all the concerns raised in the RFC discussions,
> >>> and assuming there's enough Acks and no new issues that pop up, we can
> >>> go ahead with this.
> >>>
> >>> 1: https://patchwork.kernel.org/patch/10648851/
> >>> Cc: Petri Latvala <petri.latvala@intel.com>
> >>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >>> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >>> Cc: Sean Paul <sean@poorly.run>
> >>> Cc: Eric Anholt <eric@anholt.net>
> >>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>> Cc: Dave Airlie <airlied@redhat.com>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>
> >> I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
> >>
> >> No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> > 
> > We also discussed a bit writeback implementations on irc, and it looks
> > like you can't really use writeback to accurately test that your
> > compositing engine is programmed correctly, since on at least vc4,
> > malidp and msm (not yet merged upstream) the writeback engine can't be
> > shared with any other outputs, often it even needs a
> > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > That means if you botch your programming and e.g. cause an underrun
> > scanning out continous-update outputs, then the writeback won't show
> > that to you, since it's composited separately. I guess we could teach
> > igt to run these tests on the special crtc->writeback pipeline only,
> > but essentially that's a new testcase, and not really testing the
> > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > outputs you have.
> > 
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
> > 
> 
> I think that's reasonable. After all, we want to start somewhere.
> 
> Would it make sense to append something like ", if such a test can be reasonably made using IGT for the target HW." to make it clear to contributors that in cases like the one discussed this is at the reviewers discretion?
> 
> With that change (or anything else that clarifies your intentions as described above) I'd be happy to give my AB.

Me too

Sean

> 
> Harry
> 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

On Tue, Jan 22, 2019 at 02:53:37PM -0500, Sean Paul wrote:
> On Tue, Jan 22, 2019 at 07:42:30PM +0000, Wentland, Harry wrote:
> > 
> > 
> > On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
> > >> On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > >>> Compared to the RFC[1] no changes to the patch itself, but igt moved
> > >>> forward a lot:
> > >>>
> > >>> - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >>>   and a sysroot build (should address all the build/cross platform
> > >>>   concerns raised in the RFC discussions).
> > >>>
> > >>> - tests reorganized into subdirectories so that the i915-gem tests
> > >>>   don't clog the main/shared tests directory anymore
> > >>>
> > >>> - quite a few more non-intel people contributing/reviewing/committing
> > >>>   igt tests patches.
> > >>>
> > >>> I think this addresses all the concerns raised in the RFC discussions,
> > >>> and assuming there's enough Acks and no new issues that pop up, we can
> > >>> go ahead with this.
> > >>>
> > >>> 1: https://patchwork.kernel.org/patch/10648851/
> > >>> Cc: Petri Latvala <petri.latvala@intel.com>
> > >>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > >>> Cc: Liviu Dudau <liviu.dudau@arm.com>
> > >>> Cc: Sean Paul <sean@poorly.run>
> > >>> Cc: Eric Anholt <eric@anholt.net>
> > >>> Cc: Alex Deucher <alexander.deucher@amd.com>
> > >>> Cc: Dave Airlie <airlied@redhat.com>
> > >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > >>
> > >> I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
> > >>
> > >> No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> > > 
> > > We also discussed a bit writeback implementations on irc, and it looks
> > > like you can't really use writeback to accurately test that your
> > > compositing engine is programmed correctly, since on at least vc4,
> > > malidp and msm (not yet merged upstream) the writeback engine can't be
> > > shared with any other outputs, often it even needs a
> > > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > > That means if you botch your programming and e.g. cause an underrun
> > > scanning out continous-update outputs, then the writeback won't show
> > > that to you, since it's composited separately. I guess we could teach
> > > igt to run these tests on the special crtc->writeback pipeline only,
> > > but essentially that's a new testcase, and not really testing the
> > > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > > outputs you have.
> > > 
> > > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > > won't have an igt". First driver team with hw that can be validated
> > > gets to fill the gaps :-) In practice still going to be a lot better
> > > than no tests at all, just exercising the feature will be useful, and
> > > will make it a lot easier for the next team to add the crc based tests
> > > on top.
> > > 
> > 
> > I think that's reasonable. After all, we want to start somewhere.
> > 
> > Would it make sense to append something like ", if such a test can be reasonably made using IGT for the target HW." to make it clear to contributors that in cases like the one discussed this is at the reviewers discretion?
> > 
> > With that change (or anything else that clarifies your intentions as described above) I'd be happy to give my AB.
> 
> Me too
> 
> Sean

That sounds good to me too.

Thanks,
-Brian

> 
> > 
> > Harry
> > 
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Stone]

Hi,

On Tue, 22 Jan 2019 at 19:42, Wentland, Harry <Harry.Wentland@amd.com> wrote:
> On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
>
> I think that's reasonable. After all, we want to start somewhere.
>
> Would it make sense to append something like ", if such a test can be reasonably made using IGT for the target HW." to make it clear to contributors that in cases like the one discussed this is at the reviewers discretion?
>
> With that change (or anything else that clarifies your intentions as described above) I'd be happy to give my AB.

I could definitely get behind that as well. For what it's worth, we're
(after a couple of stalls due to upstream rewrites) pushing forward
execution of igt within KernelCI. The results are not yet visible
within kernelci.org, but the tests are at least executing and we're
hoping they'll be tracked and visible soon.

If it helps, maybe we could draw up a list somewhere (README in igt?
wiki?) of which tests seem to pass generically across a few drivers,
which ones are expected to pass, which ones don't but really should,
etc. I'm currently running on imx-drm (complicated by hitting a
page-cache BUG!), and a couple of others are getting results from
rockchip, vc4, and msm. Those are pretty well supported and actively
maintained, so should give us a decent first cut at such a list.

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Wed, 23 Jan 2019, Daniel Stone <daniel@fooishbar.org> wrote:
> If it helps, maybe we could draw up a list somewhere (README in igt?
> wiki?) of which tests seem to pass generically across a few drivers,
> which ones are expected to pass, which ones don't but really should,
> etc. I'm currently running on imx-drm (complicated by hitting a
> page-cache BUG!), and a couple of others are getting results from
> rockchip, vc4, and msm. Those are pretty well supported and actively
> maintained, so should give us a decent first cut at such a list.

See tests/intel-ci/README. igt supports test lists, we could maintain
similar lists for different needs.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:11:36PM +0200, Jani Nikula wrote:
> On Wed, 23 Jan 2019, Daniel Stone <daniel@fooishbar.org> wrote:
> > If it helps, maybe we could draw up a list somewhere (README in igt?
> > wiki?) of which tests seem to pass generically across a few drivers,
> > which ones are expected to pass, which ones don't but really should,
> > etc. I'm currently running on imx-drm (complicated by hitting a
> > page-cache BUG!), and a couple of others are getting results from
> > rockchip, vc4, and msm. Those are pretty well supported and actively
> > maintained, so should give us a decent first cut at such a list.
> 
> See tests/intel-ci/README. igt supports test lists, we could maintain
> similar lists for different needs.

Anything with DRIVER_ANY in it should work. I just noticed that we have a
few generic tests with only DRIVER_INTEL|DRIVER_AMDGPU, those should be
DRIVER_ANY.

I think the best way to get there is to integrate vkms into our CI (I
think that's something we can justify, we'll benefit from the better
overall testsuite ecosystem too), then we'd get test results for every
commit&patch. And would have at least some way to make sure the DRIVER_ANY
tests are somewhat reasonable. Quick discussion with Martin and Arek on
irc says this is doable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Tue, 22 Jan 2019, "Wentland, Harry" <Harry.Wentland@amd.com> wrote:
> Would it make sense to append something like ", if such a test can be
> reasonably made using IGT for the target HW." to make it clear to
> contributors that in cases like the one discussed this is at the
> reviewers discretion?

I think the simplest change would be to say API changes SHOULD have
driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:

   SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

I.e. s/need/should/. I think it also catches the spirit of the
discussion here; seems like everyone agrees having tests is a good goal.

You'll have to allow for reviewer/maintainer/community discretion no
matter what. Judging by the discussion, CRC based tests don't currently
meet the driver-agnostic requirement. Playing devil's advocate, you
could argue any new APIs couldn't be tested with CRC either, even if it
were the most reasonable approach for i915.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:03:40PM +0200, Jani Nikula wrote:
> On Tue, 22 Jan 2019, "Wentland, Harry" <Harry.Wentland@amd.com> wrote:
> > Would it make sense to append something like ", if such a test can be
> > reasonably made using IGT for the target HW." to make it clear to
> > contributors that in cases like the one discussed this is at the
> > reviewers discretion?
> 
> I think the simplest change would be to say API changes SHOULD have
> driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:
> 
>    SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>    may exist valid reasons in particular circumstances to ignore a
>    particular item, but the full implications must be understood and
>    carefully weighed before choosing a different course.
> 
> I.e. s/need/should/. I think it also catches the spirit of the
> discussion here; seems like everyone agrees having tests is a good goal.
> 
> You'll have to allow for reviewer/maintainer/community discretion no
> matter what. Judging by the discussion, CRC based tests don't currently
> meet the driver-agnostic requirement. Playing devil's advocate, you
> could argue any new APIs couldn't be tested with CRC either, even if it
> were the most reasonable approach for i915.

I think I'll combine both for v3, I wanted to do something like that
anyway to address Eric Anholt's similar concern.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
> > On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > forward a lot:
> > >
> > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >   and a sysroot build (should address all the build/cross platform
> > >   concerns raised in the RFC discussions).
> > >
> > > - tests reorganized into subdirectories so that the i915-gem tests
> > >   don't clog the main/shared tests directory anymore
> > >
> > > - quite a few more non-intel people contributing/reviewing/committing
> > >   igt tests patches.
> > >
> > > I think this addresses all the concerns raised in the RFC discussions,
> > > and assuming there's enough Acks and no new issues that pop up, we can
> > > go ahead with this.
> > >
> > > 1: https://patchwork.kernel.org/patch/10648851/
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > Cc: Sean Paul <sean@poorly.run>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >
> > I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
> >
> > No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> 
> We also discussed a bit writeback implementations on irc, and it looks
> like you can't really use writeback to accurately test that your
> compositing engine is programmed correctly, since on at least vc4,
> malidp and msm (not yet merged upstream) the writeback engine can't be
> shared with any other outputs, often it even needs a
> dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> That means if you botch your programming and e.g. cause an underrun
> scanning out continous-update outputs, then the writeback won't show
> that to you, since it's composited separately. I guess we could teach
> igt to run these tests on the special crtc->writeback pipeline only,
> but essentially that's a new testcase, and not really testing the
> actual display: It tests writeback, not hdmi/dp/panels/whatever real
> outputs you have.

That doesn't sound right for mali-dp at least. Our writeback is
synchronous with the normal output. If the normal output underruns
then the writeback shows that, and if the writeback overruns, then the
normal output dies too.

Our writeback encoder/connector is a "possible_clone" of the normal
one. So, to integrate it in igt I was intending to set up a cloned
output on the pipe being tested.

Cheers,
-Brian

> 
> I'd say we'll shrug these cases off as "can't be reasonable tested,
> won't have an igt". First driver team with hw that can be validated
> gets to fill the gaps :-) In practice still going to be a lot better
> than no tests at all, just exercising the feature will be useful, and
> will make it a lot easier for the next team to add the crc based tests
> on top.
> 
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 09:40:34AM +0000, Brian Starkey wrote:
> On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland@amd.com> wrote:
> > > On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > forward a lot:
> > > >
> > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > >   and a sysroot build (should address all the build/cross platform
> > > >   concerns raised in the RFC discussions).
> > > >
> > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > >   don't clog the main/shared tests directory anymore
> > > >
> > > > - quite a few more non-intel people contributing/reviewing/committing
> > > >   igt tests patches.
> > > >
> > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > go ahead with this.
> > > >
> > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > Cc: Sean Paul <sean@poorly.run>
> > > > Cc: Eric Anholt <eric@anholt.net>
> > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > >
> > > I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
> > >
> > > No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> > 
> > We also discussed a bit writeback implementations on irc, and it looks
> > like you can't really use writeback to accurately test that your
> > compositing engine is programmed correctly, since on at least vc4,
> > malidp and msm (not yet merged upstream) the writeback engine can't be
> > shared with any other outputs, often it even needs a
> > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > That means if you botch your programming and e.g. cause an underrun
> > scanning out continous-update outputs, then the writeback won't show
> > that to you, since it's composited separately. I guess we could teach
> > igt to run these tests on the special crtc->writeback pipeline only,
> > but essentially that's a new testcase, and not really testing the
> > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > outputs you have.
> 
> That doesn't sound right for mali-dp at least. Our writeback is
> synchronous with the normal output. If the normal output underruns
> then the writeback shows that, and if the writeback overruns, then the
> normal output dies too.
> 
> Our writeback encoder/connector is a "possible_clone" of the normal
> one. So, to integrate it in igt I was intending to set up a cloned
> output on the pipe being tested.

Hm, I grepped for that, didn't find the assignment anywhere. If you can do
possible_clones writeback, then I think doing the writeback-as-fake CRC in
the kernel is probably the best, since it's closest to testing the real
thing. Of course if the writeback connector is already used by igt (for a
writeback test) then the magic "auto" crc tap point would need to fail.
But I don't think that would be a problem for igt, since if you're using
both writeback _and_ crc I have no idea what you're trying to do :-)

I also wouldn't tie this crc writeback into atomic (except with a special
"crc writeback flag" to make sure atomic_check rejects any other use of
the writeback connector while it's used for crc generation), but just a
vblank driven worker that flips between 2 buffers and checksums the other
one.
-Daniel

> 
> Cheers,
> -Brian
> 
> > 
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
> > 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev