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

[Daniel Vetter]

On Fri, Oct 05, 2018 at 10:44:17AM +0300, Arkadiusz Hiler wrote:
> On Thu, Oct 04, 2018 at 09:19:43PM +0200, Daniel Vetter wrote:
> > 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 at chris-wilson.co.uk>
> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at 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.
> 
> Yeah. Sorry for missing some of them, I have botched up escaping of "("
> which made the search fail silently for me.
> 
> Something to improve in my tooling ;-)
> 
> > > 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
> 
> I think that the overall change here, from hard assert to require, was
> in a good direction. And even better that this discussion followed.
> 
> I am with Chris on this one. Doing the check on resources carries more
> meaning than any other options I have seen here.
> 
> It would be nice to have this consolidated in a single slick
> yyy_require_zzz but naming is hard.
> 
> Any ideas for a better name for kmstest and a good name for the require?

drmmode is what we use in libdrm. Not sure that's better.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch