[Intel-gfx] [PATCH 02/37] drm/i915: Provide a hook for selftests

[Tvrtko Ursulin]

On 13/01/2017 10:22, Chris Wilson wrote:
> On Fri, Jan 13, 2017 at 10:12:16AM +0000, Tvrtko Ursulin wrote:
>>
>> On 13/01/2017 08:31, Chris Wilson wrote:
>>> On Wed, Jan 11, 2017 at 09:09:02PM +0000, Chris Wilson wrote:
>>>> Some pieces of code are independent of hardware but are very tricky to
>>>> exercise through the normal userspace ABI or via debugfs hooks. Being
>>>> able to create mock unit tests and execute them through CI is vital.
>>>> Start by adding a central point where we can execute unit tests and
>>>> a parameter to enable them. This is disabled by default as the
>>>> expectation is that these tests will occasionally explode.
>>>>
>>>> To facilitate integration with igt, any parameter beginning with
>>>> i915.igt__ is interpreted as a subtest executable independently via
>>>> igt/drv_selftest.
>>>>
>>>> Two classes of selftests are recognised: mock unit tests and integration
>>>> tests. Mock unit tests are run as soon as the module is loaded, before
>>>> the device is probed. At that point there is no driver instantiated and
>>>> all hw interactions must be "mocked". This is very useful for writing
>>>> universal tests to exercise code not typically run on a broad range of
>>>> architectures. Alternatively, you can hook into the live selftests and
>>>> run when the device has been instantiated - hw interactions are real.
>>>
>>> One problem I'm running into is that i915_selftest_ is a long prefix,
>>> especially when we get to something like i915_selftest_timeout(jiffies,
>>> 							       "format",
>>> 							       args);
>>>
>>> I'm tempted by /i915_selftest_/igt_/. Thoughts?
>>
>> Like an overall rename presumably and not just this helper? I thinks
>> that's fine.
>
> I think .../drm/i915/selftests/ is ok (and so i915_selftest.h), I was
> thinking of all the helpers like igt_timeout(), igt_subtests()
>
> Hmm. Otoh, if i915_selftest.h talks all about igt_foo that seems wrong.
> .../drm/i915/igt/ ?  Then we'd have #include "igt/i915_gem_object.c"
> Not sure if that is as clear as #include "selftests/i915_gem_object.c"
>
> So just the helpers, imo, should be igt_foo(). And maybe split
> i915_selftest.h between the test boilerplate and the igt helpers.

Just the helpers sounds fine. i915_selftest.h is small enough I think to 
keep everything in there.

Regards,

Tvrtko