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

[Maarten Lankhorst]

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/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. :)
> 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. :)

~Maarten