Re: [PATCH 7/8] drm/amd/display: Introduce KUnit tests to dc_dmub_srv library

[Maíra Canal <mairacanal@xxxxxxxxxx>]

On 8/11/22 04:37, David Gow wrote:
> On Thu, Aug 11, 2022 at 8:41 AM Tales Aparecida
> <tales.aparecida@xxxxxxxxx> wrote:
> > From: Maíra Canal <mairacanal@xxxxxxxxxx>
> >
> > Add unit test to the SubVP feature in order to avoid possible
> > regressions and assure the code robustness.
> >
> > Signed-off-by: Maíra Canal <mairacanal@xxxxxxxxxx>
> > Signed-off-by: Tales Aparecida <tales.aparecida@xxxxxxxxx>
> > ---
>
> FYI: This seems to have a dependency issue. See below.
>
> Otherwise, I haven't had a chance to review it properly yet, but I'll
> try to take a closer look over the next few days.
>
> Cheers,
> -- David
>
> >   drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c  |   4 +
> >   .../amd/display/tests/dc/dc_dmub_srv_test.c   | 285 ++++++++++++++++++
> >   2 files changed, 289 insertions(+)
> >   create mode 100644 drivers/gpu/drm/amd/display/tests/dc/dc_dmub_srv_test.c
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> > index 2d61c2a91cee..f5dd1f69840e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> > @@ -809,3 +809,7 @@ void dc_dmub_srv_log_diagnostic_data(struct dc_dmub_srv *dc_dmub_srv)
> >                  diag_data.is_cw0_enabled,
> >                  diag_data.is_cw6_enabled);
> >   }
> > +
> > +#if IS_ENABLED(CONFIG_AMD_DC_BASICS_KUNIT_TEST)
> > +#include "../tests/dc/dc_dmub_srv_test.c"
> > +#endif
> > diff --git a/drivers/gpu/drm/amd/display/tests/dc/dc_dmub_srv_test.c b/drivers/gpu/drm/amd/display/tests/dc/dc_dmub_srv_test.c
> > new file mode 100644
> > index 000000000000..051079cbf65e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/amd/display/tests/dc/dc_dmub_srv_test.c
> > @@ -0,0 +1,285 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * KUnit tests for dc_dmub_srv.c
> > + *
> > + * Copyright (C) 2022, Maíra Canal <mairacanal@xxxxxxxxxx>
> > + */
> > +
> > +#include <kunit/test.h>
> > +#include "dc_dmub_srv.h"
> > +
> > +struct populate_subvp_cmd_drr_info_test_case {
> > +       const char *desc;
> > +       struct dc *dc;
> > +       struct pipe_ctx *subvp_pipe;
> > +       struct pipe_ctx *vblank_pipe;
> > +       const u8 drr_in_use;
> > +       const u8 drr_window_size_ms;
> > +       const u16 min_vtotal_supported;
> > +       const u16 max_vtotal_supported;
> > +       const u8 use_ramping;
> > +};
> > +
> > +struct populate_subvp_cmd_drr_info_test_case populate_subvp_cmd_drr_info_cases[] = {
> > +       {
> > +               .desc = "Same Clock Frequency",
> > +               .dc = &(struct dc) {
> > +                       .caps = {
> > +                               .subvp_prefetch_end_to_mall_start_us = 0,
> > +                       }
> > +               },
> > +               .subvp_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 2784,
> > +                                       .v_addressable = 1080,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                               .mall_stream_config = {
> > +                                       .paired_stream = &(struct dc_stream_state) {
> > +                                               .timing = {
> > +                                                       .h_total = 3600,
> > +                                                       .v_total = 1111,
> > +                                                       .v_addressable = 1080,
> > +                                                       .v_front_porch = 3,
> > +                                                       .pix_clk_100hz = 1855800,
> > +                                               },
> > +                                       },
> > +                               },
> > +                       },
> > +               },
> > +               .vblank_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 2784,
> > +                                       .v_total = 1111,
> > +                                       .v_addressable = 1080,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                       },
> > +               },
> > +               .drr_in_use = true,
> > +               .use_ramping = false,
> > +               .drr_window_size_ms = 4,
> > +               .min_vtotal_supported = 2540,
> > +               .max_vtotal_supported = 2619,
> > +       },
> > +       {
> > +               .desc = "Same Clock Frequency with Prefetch End to Mall Start",
> > +               .dc = &(struct dc) {
> > +                       .caps = {
> > +                               .subvp_prefetch_end_to_mall_start_us = 500,
> > +                       }
> > +               },
> > +               .subvp_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 2784,
> > +                                       .v_addressable = 1080,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                               .mall_stream_config = {
> > +                                       .paired_stream = &(struct dc_stream_state) {
> > +                                               .timing = {
> > +                                                       .h_total = 3600,
> > +                                                       .v_total = 1111,
> > +                                                       .v_addressable = 1080,
> > +                                                       .v_front_porch = 3,
> > +                                                       .pix_clk_100hz = 1855800,
> > +                                               },
> > +                                       },
> > +                               },
> > +                       },
> > +               },
> > +               .vblank_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 2784,
> > +                                       .v_total = 1111,
> > +                                       .v_addressable = 1080,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                       },
> > +               },
> > +               .drr_in_use = true,
> > +               .use_ramping = false,
> > +               .drr_window_size_ms = 4,
> > +               .min_vtotal_supported = 2540,
> > +               .max_vtotal_supported = 2619,
> > +       },
> > +       {
> > +               .desc = "Same Clock Frequency Not Multiple of 2",
> > +               .dc = &(struct dc) {
> > +                       .caps = {
> > +                               .subvp_prefetch_end_to_mall_start_us = 0,
> > +                       }
> > +               },
> > +               .subvp_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 2784,
> > +                                       .v_addressable = 1080,
> > +                                       .pix_clk_100hz = 1866743,
> > +                               },
> > +                               .mall_stream_config = {
> > +                                       .paired_stream = &(struct dc_stream_state) {
> > +                                               .timing = {
> > +                                                       .h_total = 3600,
> > +                                                       .v_total = 2400,
> > +                                                       .v_addressable = 2360,
> > +                                                       .v_front_porch = 4,
> > +                                                       .pix_clk_100hz = 1866743,
> > +                                               },
> > +                                       },
> > +                               },
> > +                       },
> > +               },
> > +               .vblank_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 3600,
> > +                                       .v_total = 2400,
> > +                                       .v_addressable = 2360,
> > +                                       .pix_clk_100hz = 1866743,
> > +                               },
> > +                       },
> > +               },
> > +               .drr_in_use = true,
> > +               .use_ramping = false,
> > +               .drr_window_size_ms = 4,
> > +               .min_vtotal_supported = 1387,
> > +               .max_vtotal_supported = 2875,
> > +       },
> > +       {
> > +               .desc = "Different Clock Frequency for smaller h_total and v_total",
> > +               .dc = &(struct dc) {
> > +                       .caps = {
> > +                               .subvp_prefetch_end_to_mall_start_us = 300,
> > +                       }
> > +               },
> > +               .subvp_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 1280,
> > +                                       .v_addressable = 600,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                               .mall_stream_config = {
> > +                                       .paired_stream = &(struct dc_stream_state) {
> > +                                               .timing = {
> > +                                                       .h_total = 1280,
> > +                                                       .v_total = 720,
> > +                                                       .v_addressable = 600,
> > +                                                       .v_front_porch = 4,
> > +                                                       .pix_clk_100hz = 1866743,
> > +                                               },
> > +                                       },
> > +                               },
> > +                       },
> > +               },
> > +               .vblank_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 1280,
> > +                                       .v_total = 720,
> > +                                       .v_addressable = 600,
> > +                                       .pix_clk_100hz = 2100800,
> > +                               },
> > +                       },
> > +               },
> > +               .drr_in_use = true,
> > +               .use_ramping = false,
> > +               .drr_window_size_ms = 4,
> > +               .min_vtotal_supported = 1477,
> > +               .max_vtotal_supported = 9954,
> > +       },
> > +       {
> > +               .desc = "Different Clock Frequency for approximately 1920x1080",
> > +               .dc = &(struct dc) {
> > +                       .caps = {
> > +                               .subvp_prefetch_end_to_mall_start_us = 0,
> > +                       }
> > +               },
> > +               .subvp_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 1920,
> > +                                       .v_addressable = 1000,
> > +                                       .pix_clk_100hz = 1855800,
> > +                               },
> > +                               .mall_stream_config = {
> > +                                       .paired_stream = &(struct dc_stream_state) {
> > +                                               .timing = {
> > +                                                       .h_total = 1911,
> > +                                                       .v_total = 1080,
> > +                                                       .v_addressable = 1000,
> > +                                                       .v_front_porch = 7,
> > +                                                       .pix_clk_100hz = 1866743,
> > +                                               },
> > +                                       },
> > +                               },
> > +                       },
> > +               },
> > +               .vblank_pipe = &(struct pipe_ctx) {
> > +                       .stream = &(struct dc_stream_state) {
> > +                               .timing = {
> > +                                       .h_total = 1280,
> > +                                       .v_total = 720,
> > +                                       .v_addressable = 600,
> > +                                       .pix_clk_100hz = 2100800,
> > +                               },
> > +                       },
> > +               },
> > +               .drr_in_use = true,
> > +               .use_ramping = false,
> > +               .drr_window_size_ms = 4,
> > +               .min_vtotal_supported = 2482,
> > +               .max_vtotal_supported = 8971,
> > +       },
> > +};
> > +
> > +static void populate_subvp_cmd_drr_info_test_to_desc(struct
> > +               populate_subvp_cmd_drr_info_test_case * t, char *desc)
> > +{
> > +       strcpy(desc, t->desc);
> > +}
> > +
> > +KUNIT_ARRAY_PARAM(populate_subvp_cmd_drr_info, populate_subvp_cmd_drr_info_cases,
> > +                 populate_subvp_cmd_drr_info_test_to_desc);
> > +
> > +static void populate_subvp_cmd_drr_info_test(struct kunit *test)
> > +{
> > +       const struct populate_subvp_cmd_drr_info_test_case *test_param =
> > +               test->param_value;
> > +       struct dmub_cmd_fw_assisted_mclk_switch_pipe_data_v2 *pipe_data;
> > +
> > +       pipe_data = kunit_kzalloc(test,
> > +                                 sizeof(struct dmub_cmd_fw_assisted_mclk_switch_pipe_data_v2),
> > +                                 GFP_KERNEL);
> > +
> > +       populate_subvp_cmd_drr_info(test_param->dc, test_param->subvp_pipe,
> > +                                   test_param->vblank_pipe, pipe_data);
>
> Should this be hidden behind an #ifdef CONFIG_DRM_AMD_DC_DCN
>
> Otherwise, there are build issues under UML:
> ../drivers/gpu/drm/amd/amdgpu/../display/dc/../tests/dc/dc_dmub_srv_test.c:
> In function ‘populate_subvp_cmd_drr_info_test’:
> ../drivers/gpu/drm/amd/amdgpu/../display/dc/../tests/dc/dc_dmub_srv_test.c:260:9:
> error: implicit declaration of function ‘populate_subvp_cmd_drr_info’;
> did you mean ‘populate_subvp_cmd_drr_info_test’? [-Werror
> =implicit-function-declaration]
>   260 |         populate_subvp_cmd_drr_info(test_param->dc,
> test_param->subvp_pipe,
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |         populate_subvp_cmd_drr_info_test
> cc1: some warnings being treated as errors
> make[5]: *** [../scripts/Makefile.build:249:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.o] Error 1
>
> Making the test skip itself if this isn't enabled worked fine here.

Currently, we don't support UML as there are some build problems on 
AMDGPU. You can apply the patch that I sent previously to run the tests 
with UML, and this warning will not happen anymore, but for now, the 
tests will work properly only on x86_64.

Moreover, I don't know if it is worth adding an #ifdef 
CONFIG_DRM_AMD_DC_DCN on all the functions, as most of the functions 
that we are testing are built under CONFIG_DRM_AMD_DC_DCN.

Best Regards
- Maíra Canal

> > +
> > +       KUNIT_EXPECT_EQ(test, test_param->drr_in_use,
> > +                       pipe_data->pipe_config.vblank_data.drr_info.drr_in_use);
> > +       KUNIT_EXPECT_EQ(test, test_param->drr_window_size_ms,
> > +                       pipe_data->pipe_config.vblank_data.drr_info.drr_window_size_ms);
> > +       KUNIT_EXPECT_EQ(test, test_param->use_ramping,
> > +                       pipe_data->pipe_config.vblank_data.drr_info.use_ramping);
> > +       KUNIT_EXPECT_EQ(test, test_param->min_vtotal_supported,
> > +                       pipe_data->pipe_config.vblank_data.drr_info.min_vtotal_supported);
> > +       KUNIT_EXPECT_EQ(test, test_param->max_vtotal_supported,
> > +                       pipe_data->pipe_config.vblank_data.drr_info.max_vtotal_supported);
> > +}
> > +
> > +static struct kunit_case dc_dmub_srv_cases[] = {
> > +       KUNIT_CASE_PARAM(populate_subvp_cmd_drr_info_test, populate_subvp_cmd_drr_info_gen_params),
> > +       {  }
> > +};
> > +
> > +static struct kunit_suite dc_dmub_srv_suite = {
> > +       .name = "dc_dmub_srv",
> > +       .test_cases = dc_dmub_srv_cases,
> > +};
> > +
> > +kunit_test_suite(dc_dmub_srv_suite);
> > --
> > 2.37.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220811004010.61299-8-tales.aparecida%40gmail.com.