[PATCH 0/4] drm/amd/display: Update Display Core unit tests

[Tales]

Em sáb., 20 de abr. de 2024 às 15:50, Joao Paulo Pereira da Silva
<jppaulo11 at usp.br> escreveu:
>
> Hey, I'm interested in contributing for display tests from this patch-set.
> I've noticed potential updates related to both refactoring and optimization.
> This patch-set applies these suggestions.
>

Hi,

It's great to see this moving forward!

Overall the suggested changes make sense to me, and honestly I already don't
remember the discussions that went behind some of them. The only thing that
I would like to raise for you, and anyone else reviewing this, is that
apparently
there are now stronger feeling towards the "preferred way"[1] to handle tests in
static functions, using EXPORT_SYMBOL_IF_KUNIT (or EXPORT_SYMBOL_FOR_TESTS_ONLY
in the case of DRM), so they might be more adequate to work on
refactoring this code.

[1]: https://lore.kernel.org/all/5z66ivuhfrzrnuzt6lwjfm5fuozxlgqsco3qb5rfzyf6mil5ms@2svqtlcncyjj/

Kind regards,
Tales

>
> [WHY]
>
> 1.      The single test suite in the file
>         test/kunit/dc/dml/calcs/bw_fixed_test.c, which tests some static
>         functions defined in the dc/basics/bpw_fixed.c, is not being run.
>         According to kunit documentation
>         (https://www.kernel.org/doc/html/latest/dev-tools/kunit/usage.html#testing-static-functions),
>         there are two strategies for testing
>         static functions, but none of them seem to be configured. Additionally,
>         it appears that the Config DCE_KUNIT_TEST should be associated with this
>         test, since it was introduced in the same patch of the test
>         (https://lore.kernel.org/amd-gfx/20240222155811.44096-3-Rodrigo.Siqueira@amd.com/),
>         but it is not being used anywhere in the display driver.
>
> 2.      Also, according to the documentation, "The display/tests folder replicates
>         the folder hierarchy of the display folder". However, note that this test file
>         (test/kunit/dc/dml/calcs/bw_fixed_test.c) has a conflicting path with the file
>         that is being tested (dc/basics/bw_fixed.c).
>
> 3.      Config Names and Helps are a bit misleading and don't follow a strict
>         pattern. For example, the config DML_KUNIT_TEST indicates that it is used
>         to activate tests for the Display Core Engine, but instead activates tests
>         for the Display Core Next. Also, note the different name patterns in
>         DML_KUNIT_TEST and AMD_DC_BASICS_KUNIT_TEST.
>
> 4.      The test suite dcn21_update_bw_bounding_box_test_suite configures an init
>         function that doesn't need to be executed before every test, but only once
>         before the suite runs.
>
> 5.      There are some not updated info in the Documentation, such as the
>         recommended command to run the tests:
>         $ ./tools/testing/kunit/kunit.py run --arch=x86_64 \
>         --kunitconfig=drivers/gpu/drm/amd/display/tests
>         (it doesn't work since there is no .kunitconfig in
>         drivers/gpu/drm/amd/display/tests)
>
>
> [HOW]
>
> 1. Revise Config names and Help blocks.
>
> 2.      Change the path of the test file bw_fixed_test from
>         test/kunit/dc/dml/calcs/bw_fixed_test.c to test/kunit/dc/basics/bw_fixed_test.c
>         to make it consistent with the Documentation and the other display driver
>         tests. Make this same test file run by importing it conditionally in the file
>         dc/basics/bw_fixed_test.c.
>
> 3.      Turn the test init function of the suite
>         dcn21_update_bw_bounding_box_test_suite into a suite init.
>
> 4.      Update Documentation
>
> Joao Paulo Pereira da Silva (4):
>   drm/amd/display: Refactor AMD display KUnit tests configs
>   drm/amd/display/test: Fix kunit test that is not running
>   drm/amd/display/test: Optimize kunit test suite
>     dml_dcn20_fpu_dcn21_update_bw_bounding_box_test
>   Documentation/gpu: Update AMD Display Core Unit Test documentation
>
>  .../gpu/amdgpu/display/display-test.rst       | 20 ++++++------
>  drivers/gpu/drm/amd/display/Kconfig           | 31 ++++++-------------
>  .../gpu/drm/amd/display/dc/basics/bw_fixed.c  |  3 ++
>  drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c  |  2 +-
>  .../dc/dml/dcn20/display_mode_vba_20.c        |  2 +-
>  .../dc/dml/dcn20/display_rq_dlg_calc_20.c     |  2 +-
>  .../drm/amd/display/test/kunit/.kunitconfig   |  7 ++---
>  .../gpu/drm/amd/display/test/kunit/Makefile   |  4 +--
>  .../dc/{dml/calcs => basics}/bw_fixed_test.c  |  0
>  .../test/kunit/dc/dml/dcn20/dcn20_fpu_test.c  |  6 ++--
>  10 files changed, 32 insertions(+), 45 deletions(-)
>  rename drivers/gpu/drm/amd/display/test/kunit/dc/{dml/calcs => basics}/bw_fixed_test.c (100%)
>
> --
> 2.44.0
>