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@xxxxxxx> 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@xxxxxxxxx> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx> > > > Cc: Liviu Dudau <liviu.dudau@xxxxxxx> > > > Cc: Sean Paul <sean@xxxxxxxxxx> > > > Cc: Eric Anholt <eric@xxxxxxxxxx> > > > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > > > Cc: Dave Airlie <airlied@xxxxxxxxxx> > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > > --- > > > 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@xxxxxxx>. > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
