From: "Maíra Canal" <mairacanal@riseup.net>
To: David Gow <davidgow@google.com>,
Tales Aparecida <tales.aparecida@gmail.com>
Cc: andrealmeid@riseup.net, "Thomas Zimmermann" <tzimmermann@suse.de>,
siqueirajordao@riseup.net, "Jonathan Corbet" <corbet@lwn.net>,
"David Airlie" <airlied@linux.ie>,
"Rodrigo Siqueira" <Rodrigo.Siqueira@amd.com>,
"Pan, Xinhui" <Xinhui.Pan@amd.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
mwen@igalia.com, "Leo Li" <sunpeng.li@amd.com>,
amd-gfx@lists.freedesktop.org, "Daniel Vetter" <daniel@ffwll.ch>,
"Alex Deucher" <alexander.deucher@amd.com>,
magalilemes00@gmail.com, "Isabella Basso" <isabbasso@riseup.net>,
"Trevor Woerner" <twoerner@gmail.com>,
"Harry Wentland" <harry.wentland@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"KUnit Development" <kunit-dev@googlegroups.com>
Subject: Re: [PATCH 7/8] drm/amd/display: Introduce KUnit tests to dc_dmub_srv library
Date: Thu, 11 Aug 2022 10:38:29 -0300 [thread overview]
Message-ID: <1716b7b6-9fd0-214e-aeab-55220aa70a9d@riseup.net> (raw)
In-Reply-To: <CABVgOSmJm0W1OAk7Ja8zb_WMcj=kXx7-w9J747k5sRySi1Jyfw@mail.gmail.com>
On 8/11/22 04:37, David Gow wrote:
> On Thu, Aug 11, 2022 at 8:41 AM Tales Aparecida
> <tales.aparecida@gmail.com> wrote:
>>
>> From: Maíra Canal <mairacanal@riseup.net>
>>
>> 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@riseup.net>
>> Signed-off-by: Tales Aparecida <tales.aparecida@gmail.com>
>> ---
>
> 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@riseup.net>
>> + */
>> +
>> +#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@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220811004010.61299-8-tales.aparecida%40gmail.com.
next prev parent reply other threads:[~2022-08-11 13:38 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-11 0:40 [PATCH 0/8] drm/amd/display: Introduce KUnit to Display Mode Library Tales Aparecida
2022-08-11 0:40 ` [PATCH 1/8] drm/amd/display: Introduce KUnit tests for fixed31_32 library Tales Aparecida
2022-08-11 3:04 ` Daniel Latypov
2022-08-11 7:19 ` David Gow
2022-08-11 13:22 ` Maíra Canal
2022-08-13 5:56 ` David Gow
2022-08-11 0:40 ` [PATCH 2/8] drm/amd/display: Introduce KUnit tests to the bw_fixed library Tales Aparecida
2022-08-11 7:34 ` David Gow
2022-08-11 20:55 ` Tales Lelo da Aparecida
2022-08-11 0:40 ` [PATCH 3/8] drm/amd/display: Introduce Kunit tests to display_rq_dlg_calc_20 Tales Aparecida
2022-08-11 0:40 ` [PATCH 4/8] drm/amd/display: Introduce KUnit tests to the display_mode_vba library Tales Aparecida
2022-08-11 0:40 ` [PATCH 5/8] drm/amd/display: Introduce KUnit to dcn20/display_mode_vba_20 library Tales Aparecida
2022-08-11 0:40 ` [PATCH 6/8] drm/amd/display: Introduce KUnit tests for dcn20_fpu Tales Aparecida
2022-08-11 0:40 ` [PATCH 7/8] drm/amd/display: Introduce KUnit tests to dc_dmub_srv library Tales Aparecida
2022-08-11 7:37 ` David Gow
2022-08-11 13:38 ` Maíra Canal [this message]
2022-08-11 17:53 ` David Gow
2022-08-11 0:40 ` [PATCH 8/8] Documentation/gpu: Add Display Core Unit Test documentation Tales Aparecida
2022-08-11 11:22 ` [PATCH 0/8] drm/amd/display: Introduce KUnit to Display Mode Library Christian König
2022-08-11 13:09 ` Maíra Canal
2022-08-12 6:58 ` David Gow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1716b7b6-9fd0-214e-aeab-55220aa70a9d@riseup.net \
--to=mairacanal@riseup.net \
--cc=Rodrigo.Siqueira@amd.com \
--cc=Xinhui.Pan@amd.com \
--cc=airlied@linux.ie \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=andrealmeid@riseup.net \
--cc=christian.koenig@amd.com \
--cc=corbet@lwn.net \
--cc=daniel@ffwll.ch \
--cc=davidgow@google.com \
--cc=harry.wentland@amd.com \
--cc=isabbasso@riseup.net \
--cc=kunit-dev@googlegroups.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=magalilemes00@gmail.com \
--cc=mripard@kernel.org \
--cc=mwen@igalia.com \
--cc=siqueirajordao@riseup.net \
--cc=sunpeng.li@amd.com \
--cc=tales.aparecida@gmail.com \
--cc=twoerner@gmail.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.