Em sáb., 20 de abr. de 2024 às 15:50, Joao Paulo Pereira da Silva <jppaulo11@xxxxxx> 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@xxxxxxx/), > 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 >
