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

[Maarten Lankhorst]

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/msgid/20180608124057.6889-1-jani.nikula@intel.com
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>>> 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. :)

~Maarten