On 2024-08-27 13:49, Louis Chauvet wrote:
> Le 19/08/24 - 16:56, Harry Wentland a écrit :
>
> [...]
>
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
>> b/drivers/gpu/drm/vkms/vkms_composer.c
>> index 3d6785d081f2..3ecda70c2b55 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>> @@ -435,3 +435,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const
>> char *src_name)
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index 3d6785d081f2..3ecda70c2b55 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -435,3 +435,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
> *src_name)
>
> return ret;
> }
> +
> +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
> +#include "tests/vkms_color_tests.c"
> +#endif>
>> return ret;
>> }
>> +
>> +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
>> +#include "tests/vkms_color_tests.c"
>> +#endif
>
> This is very strange to include a .c in this file, is it something done a
> lot in the kernel? I can find only one occurence of this pattern in the
> kernel [1], the other tests are in their own modules.
>
> In addition it crate many warning during compilations:
> warning: symbol 'test_*' was not declared. Should it be static?
>
> As other tests will be introduced (yuv [2], config [3]), it is maybe
> interesting to introduce a new module as [2] is doing?
The VISIBLE_IF_KUNIT et al. is much nicer than including a .c file.
Thanks for pointing me to them. Will change this.
Harry
>
> [1]: https://elixir.bootlin.com/linux/v6.11-rc5/source/fs/ext4/mballoc.c#L7047
> [2]: https://lore.kernel.org/all/20240809-yuv-v10-14-1a7c76416...@bootlin.com/
> [3]:
> https://lore.kernel.org/all/20240814-google-remove-crtc-index-from-parameter-v1-15-6e179abf9...@bootlin.com/
>
