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

[Arkadiusz Hiler <arkadiusz.hiler@intel.com>]

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@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.

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?

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