On Wed, Nov 23, 2022 at 11:28 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
This series introduce Kunit tests to the vc4 KMS driver, but unlike what we have been doing so far in KMS, it actually tests the atomic modesetting code.
In order to do so, I've had to improve a fair bit on the Kunit helpers already found in the tree in order to register a full blown and somewhat functional KMS driver.
It's of course relying on a mock so that we can test it anywhere. The mocking approach created a number of issues, the main one being that we need to create a decent mock in the first place, see patch 22. The basic idea is that I created some structures to provide a decent approximation of the actual hardware, and that would support both major architectures supported by vc4.
This is of course meant to evolve over time and support more tests, but I've focused on testing the HVS FIFO assignment code which is fairly tricky (and the tests have actually revealed one more bug with our current implementation). I used to have a userspace implementation of those tests, where I would copy and paste the kernel code and run the tests on a regular basis. It's was obviously fairly suboptimal, so it seemed like the perfect testbed for that series.
Let me know what you think, Maxime
To: David Airlie airlied@gmail.com To: Daniel Vetter daniel@ffwll.ch To: Maarten Lankhorst maarten.lankhorst@linux.intel.com To: Maxime Ripard mripard@kernel.org To: Thomas Zimmermann tzimmermann@suse.de Cc: Dave Stevenson dave.stevenson@raspberrypi.com Cc: Javier Martinez Canillas javierm@redhat.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Maíra Canal mairacanal@riseup.net Cc: Brendan Higgins brendan.higgins@linux.dev Cc: David Gow davidgow@google.com Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech
Hi Maxime,
Thanks very much for this! I'm really excited to see these sorts of tests being written.
I was able to successfully run these under qemu with: ./tools/testing/kunit/kunit.py run --kunitconfig drivers/gpu/drm/vc4/tests --arch arm64 --cross_compile=aarch64-linux-gnu- (and also with clang, using --make_options LLVM=1 instead of the --cross_compile flag)
On the other hand, they don't compile as a module: ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_crtc.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_output.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_plane.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/tests/drm_managed_test.o ERROR: modpost: "vc4_drm_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc5_drm_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "drm_kunit_helper_alloc_device" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "__drm_kunit_helper_alloc_drm_device_with_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "__vc4_hvs_alloc" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_dummy_plane" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_mock_pv" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_dummy_output" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_kms_load" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_txp_crtc_data" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! WARNING: modpost: suppressed 17 unresolved symbol warnings because there were too many)
Most of those are just the need to export some symbols. There's some work underway to support conditionally exporting symbols only if KUnit is enabled, which may help: https://lore.kernel.org/linux-kselftest/20221102175959.2921063-1-rmoar@googl...
Otherwise, I suspect the better short-term solution would just be to require that the tests are built-in (or at least compiled into whatever of the drm/vc4 modules makes most sense).
The only other thing which has me a little confused is the naming of some of the functions, specifically with the __ prefix. Is it just for internal functions (many of them aren't static, but maybe they could use the VISIBLE_IF_KUNIT macro if that makes sense), or for versions of functions which accept extra arguments? Not a big deal (and maybe it's a DRM naming convention I'm ignorant of), but I couldn't quite find a pattern on my first read through.
But on the whole, these look good from a KUnit point-of-view. It's really to see some solid mocking and driver testing, too. There would be ways to avoid passing the 'struct kunit' around in more places (or to store extra data as a kunit_resource), but I think it's better overall to pass it around like you have in this case -- it's certainly more compatible with things which might span threads (e.g. the workqueues).
Thanks a bunch, -- David