[igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Souza, Jose]

On Thu, 2018-06-28 at 15:21 +0200, Maarten Lankhorst wrote:
> Op 28-06-18 om 15:12 schreef Daniel Vetter:
> > On Thu, Jun 28, 2018 at 2:22 PM, Maarten Lankhorst
> > <maarten.lankhorst at linux.intel.com> wrote:
> > > Op 28-06-18 om 14:10 schreef Daniel Vetter:
> > > > On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst
> > > > wrote:
> > > > > Op 28-06-18 om 13:54 schreef Daniel Vetter:
> > > > > > On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst
> > > > > > wrote:
> > > > > > > Op 28-06-18 om 11:56 schreef Jani Nikula:
> > > > > > > > Running the tests with i915.disable_display=1 leads to
> > > > > > > > IGT errors. Skip
> > > > > > > > tests that need display.
> > > > > > > > 
> > > > > > > > References: http://patchwork.freedesktop.org/patch/msgi
> > > > > > > > d/20180608124057.6889-1-jani.nikula at intel.com
> > > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.co
> > > > > > > > m>
> > > > > > > > Cc: Daniel Vetter <daniel at ffwll.ch>
> > > > > > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > > > > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> > > > > > > > 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > What's the best paradigm for this? There's loads of
> > > > > > > > random and cargo
> > > > > > > > culted igt_requires for this stuff, with various checks
> > > > > > > > on pipes > 0
> > > > > > > > etc.
> > > > > > > > ---
> > > > > > > 
> > > > > > > igt_display_require_output() is the easiest check to find
> > > > > > > if an output
> > > > > > > is connected, should be approximately same as display
> > > > > > > enabled. :)
> > > > > > 
> > > > > > Unfortunately not quite. I think we do want to run these
> > > > > > tests here on
> > > > > > headless systems which have a display block, but no outputs
> > > > > > connected.
> > > > > > Just for better test coverage and all that.
> > > > > > 
> > > > > > I think what we want is a new
> > > > > > 
> > > > > > igt_require_display(int fd)
> > > > > > {
> > > > > >     igt_display_t display;
> > > > > >     igt_display_init(&display, fd);
> > > > > >     igt_require(display->n_pipes > 0, "drm driver without
> > > > > > display
> > > > > >     hardware\n");
> > > > > > }
> > > > > > 
> > > > > > gtkdoc plus wiring it up needed too ofc.
> > > > > > 
> > > > > > We could also patch up igt_display_init() to check for this
> > > > > > (and probably
> > > > > > should, not sure about that), but a very simply igt_require
> > > > > > helper that
> > > > > > only takes the fd seems useful to me for all the low-level
> > > > > > tests like
> > > > > > kms_addfb_basic here which don't want/need a full
> > > > > > igt_display_t
> > > > > > initialized.
> > > > > 
> > > > > But igt_display is not without side effects. It sets the
> > > > > atomic cap, which would be surprising in a require statement.
> > > > 
> > > > Hm, then maybe we need to open-coude the drmgetresources call +
> > > > checking
> > > > for numCrtcs. Not that many more lines really.
> > > > 
> > > > > i915 without pipes should probably unset DRIVER_MODESET.
> > > > 
> > > > Too late by that point, and we're not the only driver doing
> > > > this.
> > > 
> > > Well drmGetResources could work, but I wouldn't put it in core
> > > unless other stuff cares about it.
> > > Just a function in kms_addfb_basic.c is probably good enough. :)
> > 
> > There's other testcase which want this too, this was just an
> > example
> > ... There's a subtest in vgem_prime, and then there's more outside
> > of
> > BAT most likely.
> > -Daniel
> 
> Since this doesn't operate on igt_display, probably needs to be
> kmstest_require_pipe()?
> 
> With that + drmModeGetResources, I'm happy. :)

I just sent a patchset(https://patchwork.freedesktop.org/series/46630/)
unseting DRIVER_MODESET when display is disabled also added the lines
needed to check if a drm driver have modeset capability.

So what about test like this:

void igt_require_display(int fd)
{
	drmModeRes *resources;
	int pipes;
	uint64_t cap = 0;

	drmGetCap(fd, DRM_CAP_MODESET, &cap);
	igt_require_f(cap, "drm driver do not support modeset\n");

	resources = drmModeGetResources(fd);
	igt_assert(resources);
	pipes = resources->count_crtcs;
	drmModeFreeResources(resources);

	igt_require_f(pipes > 0, "drm driver without display
hardware\n");
}

I also have a patch calling it from all tests that needs display.

> 
> ~Maarten
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev