Re: [igt-dev] [PATCH i-g-t] igt: Check drmModeGetResources()

Daniel Vetter
2018-10-04T19:19:51Z

On Thu, Oct 04, 2018 at 04:14:15PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2018-10-04 14:27:05)
> > On Thu, Oct 04, 2018 at 03:44:46PM +0300, Arkadiusz Hiler wrote:
> > > On Wed, Oct 03, 2018 at 08:43:49PM +0100, Chris Wilson wrote:
> > > > If KMS is not supported on the device, drmModeGetResources() will return
> > > > NULL, often this is an indication that we should not attempt to run the
> > > > test. Although it would be preferred to use something like
> > > > igt_require_display() as the canonical check and assert that
> > > > drmModeGetResources() did not hit an error, it is not always practical
> > > > as the tests do not utilize the common igt_display abstraction.
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > >
> > > and pushed
> >
> > I still think a kmstest_require() or so, plus then removing the pile of
> > add-how igt_require we just sprinkled all over, would be more maintainable
> > long term. kmstest_require() would also allow us to drop a not-so-cryptic
> > message into logs.
>
> But what does kmstest mean? igt_require_kms(), which can be the raw
> drm_mode_getresources ioctl, matches my expectations of a lowlevel check.
> But is that as descriptive for the tests that are just using
> drmModeGetResources() directly? That is the conundrum.

We should probably change the kmstest_ prefix to something slightly more
meaningful. And yeah there might be a few too many shades of grey with the
low-level tests. But fore the high-level ones I think outright requiring
that you have an output and at least one pipe seems reasonable.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation