Re: [Intel-xe] [PATCH 3/6] drm/xe: Fix kunit integration due to missing prototypes

[Matthew Auld <matthew.william.auld@gmail.com>]

On Wed, 22 Feb 2023 at 14:59, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> On Wed, Feb 22, 2023 at 10:02:08AM +0000, Matthew Auld wrote:
> >On Tue, 21 Feb 2023 at 23:34, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >>
> >> In order to avoid  -Werror=missing-prototypes, add the prototypes and
> >> move the functions to the end of the file, surrounded by
> >> `#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)`.
> >
> >Is everything under tests/ not excluded from compilation when !XE_KUNIT_TEST?
>
> yes, except the ifdef is not in tests/. Example:
> drivers/gpu/drm/xe/tests/xe_bo.c

AFAICT xe/tests is only compiled if XE_KUNIT_TEST is enabled (looking
at the Makefile). tests/xe_bo.c is included at the bottom of xe_bo.c
but that is also wrapped in ifdef. So not seeing why we need to add
the ifdef stuff directly in tests/xe_bo.c, for example.

>
> The way the kunit tests in xe are split is: each .o in tests/ will
> create a separate module. In order to test a function like we are
> doing, the function needs to be declared !static and exported so the
> other module can call it.
>
> There are other possible ways to test functions on a different
> compilation unit: https://kunit.dev/third_party/kernel/docs/tips.html?highlight=static%20function#testing-static-functions
> I'm not so familiar with kunit yet to be able to say which one
> is better for our case, so I just went with the easy fix.
>
> +Thomas
>
> Lucas De Marchi
>
> >
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/tests/xe_bo.c      | 54 +++++++++++++++------------
> >>  drivers/gpu/drm/xe/tests/xe_dma_buf.c |  2 +
> >>  drivers/gpu/drm/xe/tests/xe_migrate.c |  6 +++
> >>  3 files changed, 39 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/tests/xe_bo.c b/drivers/gpu/drm/xe/tests/xe_bo.c
> >> index 87ac21cc8ca9..2c94ddaacda6 100644
> >> --- a/drivers/gpu/drm/xe/tests/xe_bo.c
> >> +++ b/drivers/gpu/drm/xe/tests/xe_bo.c
> >> @@ -141,29 +141,6 @@ static void ccs_test_run_gt(struct xe_device *xe, struct xe_gt *gt,
> >>         xe_bo_put(bo);
> >>  }
> >>
> >> -static int ccs_test_run_device(struct xe_device *xe)
> >> -{
> >> -       struct kunit *test = xe_cur_kunit();
> >> -       struct xe_gt *gt;
> >> -       int id;
> >> -
> >> -       if (!xe_device_has_flat_ccs(xe)) {
> >> -               kunit_info(test, "Skipping non-flat-ccs device.\n");
> >> -               return 0;
> >> -       }
> >> -
> >> -       for_each_gt(gt, xe, id)
> >> -               ccs_test_run_gt(xe, gt, test);
> >> -
> >> -       return 0;
> >> -}
> >> -
> >> -void xe_ccs_migrate_kunit(struct kunit *test)
> >> -{
> >> -       xe_call_for_each_device(ccs_test_run_device);
> >> -}
> >> -EXPORT_SYMBOL(xe_ccs_migrate_kunit);
> >> -
> >>  static int evict_test_run_gt(struct xe_device *xe, struct xe_gt *gt, struct kunit *test)
> >>  {
> >>         struct xe_bo *bo, *external;
> >> @@ -278,6 +255,8 @@ static int evict_test_run_gt(struct xe_device *xe, struct xe_gt *gt, struct kuni
> >>         return 0;
> >>  }
> >>
> >> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
> >> +
> >>  static int evict_test_run_device(struct xe_device *xe)
> >>  {
> >>         struct kunit *test = xe_cur_kunit();
> >> @@ -296,8 +275,37 @@ static int evict_test_run_device(struct xe_device *xe)
> >>         return 0;
> >>  }
> >>
> >> +void xe_bo_evict_kunit(struct kunit *test);
> >> +
> >>  void xe_bo_evict_kunit(struct kunit *test)
> >>  {
> >>         xe_call_for_each_device(evict_test_run_device);
> >>  }
> >>  EXPORT_SYMBOL(xe_bo_evict_kunit);
> >> +
> >> +static int ccs_test_run_device(struct xe_device *xe)
> >> +{
> >> +       struct kunit *test = xe_cur_kunit();
> >> +       struct xe_gt *gt;
> >> +       int id;
> >> +
> >> +       if (!xe_device_has_flat_ccs(xe)) {
> >> +               kunit_info(test, "Skipping non-flat-ccs device.\n");
> >> +               return 0;
> >> +       }
> >> +
> >> +       for_each_gt(gt, xe, id)
> >> +               ccs_test_run_gt(xe, gt, test);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +void xe_ccs_migrate_kunit(struct kunit *test);
> >> +
> >> +void xe_ccs_migrate_kunit(struct kunit *test)
> >> +{
> >> +       xe_call_for_each_device(ccs_test_run_device);
> >> +}
> >> +EXPORT_SYMBOL(xe_ccs_migrate_kunit);
> >> +
> >> +#endif
> >> diff --git a/drivers/gpu/drm/xe/tests/xe_dma_buf.c b/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> >> index 615d22e3f731..d8ad135d0e04 100644
> >> --- a/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> >> +++ b/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> >> @@ -252,6 +252,8 @@ static int dma_buf_run_device(struct xe_device *xe)
> >>         return 0;
> >>  }
> >>
> >> +void xe_dma_buf_kunit(struct kunit *test);
> >> +
> >>  void xe_dma_buf_kunit(struct kunit *test)
> >>  {
> >>         xe_call_for_each_device(dma_buf_run_device);
> >> diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c b/drivers/gpu/drm/xe/tests/xe_migrate.c
> >> index 03a60d5b42f1..7be0c2543916 100644
> >> --- a/drivers/gpu/drm/xe/tests/xe_migrate.c
> >> +++ b/drivers/gpu/drm/xe/tests/xe_migrate.c
> >> @@ -352,6 +352,8 @@ static void xe_migrate_sanity_test(struct xe_migrate *m, struct kunit *test)
> >>         xe_bo_vunmap(m->pt_bo);
> >>  }
> >>
> >> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
> >> +
> >>  static int migrate_test_run_device(struct xe_device *xe)
> >>  {
> >>         struct kunit *test = xe_cur_kunit();
> >> @@ -371,8 +373,12 @@ static int migrate_test_run_device(struct xe_device *xe)
> >>         return 0;
> >>  }
> >>
> >> +void xe_migrate_sanity_kunit(struct kunit *test);
> >> +
> >>  void xe_migrate_sanity_kunit(struct kunit *test)
> >>  {
> >>         xe_call_for_each_device(migrate_test_run_device);
> >>  }
> >>  EXPORT_SYMBOL(xe_migrate_sanity_kunit);
> >> +
> >> +#endif
> >> --
> >> 2.39.0
> >>