diff mbox series

[v5,7/8] drm/amd/display: Introduce KUnit tests to dc_dmub_srv library

Message ID 20240222155811.44096-8-Rodrigo.Siqueira@amd.com (mailing list archive)
State New, archived
Headers show
Series drm/amd/display: Introduce KUnit to Display Mode Library | expand

Commit Message

Rodrigo Siqueira Jordao Feb. 22, 2024, 3:56 p.m. UTC
From: Maíra Canal <mairacanal@riseup.net>

Add a unit test to the SubVP feature in order to avoid possible
regressions and ensure code robustness. In particular, this new test
validates the expected parameters when using 4k144 and 4k240 displays.

Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Co-developed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/gpu/drm/amd/display/Kconfig           |  13 ++
 drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c  |   4 +
 .../drm/amd/display/test/kunit/.kunitconfig   |   1 +
 .../display/test/kunit/dc/dc_dmub_srv_test.c  | 159 ++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/display/test/kunit/dc/dc_dmub_srv_test.c

Comments

Jani Nikula Feb. 26, 2024, 11:12 a.m. UTC | #1
On Thu, 22 Feb 2024, Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> wrote:
> diff --git a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
> index eb6f81601757..4c5861ad58bd 100644
> --- a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
> +++ b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
> @@ -4,5 +4,6 @@ CONFIG_DRM=y
>  CONFIG_DRM_AMDGPU=y
>  CONFIG_DRM_AMD_DC=y
>  CONFIG_AMD_DC_BASICS_KUNIT_TEST=y
> +CONFIG_AMD_DC_KUNIT_TEST=y
>  CONFIG_DCE_KUNIT_TEST=y
>  CONFIG_DML_KUNIT_TEST=y

A bit random patch to comment on, but this hunk demonstrates the point:

Should all the configs have DRM_AMD_ prefix to put them in a
"namespace"?


BR,
Jani.
Rodrigo Siqueira Jordao Feb. 28, 2024, 2:42 p.m. UTC | #2
On 2/26/24 04:12, Jani Nikula wrote:
> On Thu, 22 Feb 2024, Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> wrote:
>> diff --git a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
>> index eb6f81601757..4c5861ad58bd 100644
>> --- a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
>> +++ b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
>> @@ -4,5 +4,6 @@ CONFIG_DRM=y
>>   CONFIG_DRM_AMDGPU=y
>>   CONFIG_DRM_AMD_DC=y
>>   CONFIG_AMD_DC_BASICS_KUNIT_TEST=y
>> +CONFIG_AMD_DC_KUNIT_TEST=y
>>   CONFIG_DCE_KUNIT_TEST=y
>>   CONFIG_DML_KUNIT_TEST=y
> 
> A bit random patch to comment on, but this hunk demonstrates the point:
> 
> Should all the configs have DRM_AMD_ prefix to put them in a
> "namespace"?
> 

You are right! I'll fix this in the next version.

Thanks
Siqueira

> 
> BR,
> Jani.
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index ab52b135db85..11b0e54262f3 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -90,4 +90,17 @@  config AMD_DC_BASICS_KUNIT_TEST
 
 		If unsure, say N.
 
+config AMD_DC_KUNIT_TEST
+	bool "Enable KUnit tests for the 'utils' sub-component of DAL" if !KUNIT_ALL_TESTS
+	depends on DRM_AMD_DC && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+		Enables unit tests for the basics folder of Display Core. Only useful for
+		kernel devs running KUnit.
+
+		For more information on KUnit and unit tests in general please refer to
+		the KUnit documentation in Documentation/dev-tools/kunit/.
+
+		If unsure, say N.
+
 endmenu
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 6083b1dcf050..7aafdfeac60e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
+++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
@@ -1438,3 +1438,7 @@  bool dc_wake_and_execute_gpint(const struct dc_context *ctx, enum dmub_gpint_com
 
 	return result;
 }
+
+#if IS_ENABLED(CONFIG_AMD_DC_KUNIT_TEST)
+#include "../test/kunit/dc/dc_dmub_srv_test.c"
+#endif
diff --git a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
index eb6f81601757..4c5861ad58bd 100644
--- a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
+++ b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig
@@ -4,5 +4,6 @@  CONFIG_DRM=y
 CONFIG_DRM_AMDGPU=y
 CONFIG_DRM_AMD_DC=y
 CONFIG_AMD_DC_BASICS_KUNIT_TEST=y
+CONFIG_AMD_DC_KUNIT_TEST=y
 CONFIG_DCE_KUNIT_TEST=y
 CONFIG_DML_KUNIT_TEST=y
diff --git a/drivers/gpu/drm/amd/display/test/kunit/dc/dc_dmub_srv_test.c b/drivers/gpu/drm/amd/display/test/kunit/dc/dc_dmub_srv_test.c
new file mode 100644
index 000000000000..d12c4e3816b5
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/test/kunit/dc/dc_dmub_srv_test.c
@@ -0,0 +1,159 @@ 
+// 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"
+
+/**
+ * DOC: overview
+ *
+ * The file dc_dumb_srv.c has many functions that work as an interface to
+ * generate some of the DMUB parameters. To offload some of the complexity from
+ * the DMUB, the 'dc_dmub_srv.c' file provides functions that perform
+ * mathematical calculations to generate the parameter that will be passed to
+ * the DMUB to enable specific configurations.
+ */
+
+/**
+ * struct populate_subvp_cmd_drr_info_test_case - Fields for subvp validation
+ *
+ * The function populate_subvp_cmd_drr_info() performs calculations based on
+ * different pipe context timing values. This struct maintains those fields
+ * required to be passed to the populate_subvp_cmd_drr_info.
+ */
+struct populate_subvp_cmd_drr_info_test_case {
+	const char *desc;
+	/**
+	* @dc: In the specific context of populate_subvp_cmd_drr_info() test,
+	* we only care about the DC capabilities.
+	*/
+	struct dc *dc;
+
+	/**
+	 * @subvp_pipe: This parameter plays an essential role in the
+	 * populate_subvp_cmd_drr_info validation because it will be used to
+	 * derive some of the parameters for the max VTotal, but it is also
+	 * employed in a pointer validation that extracts the phantom timing
+	 * from the context.
+	 */
+	struct pipe_ctx *subvp_pipe;
+
+	/**
+	 * @vblank_pipe: This field keeps the DRR timing values used in the Max
+	 * and Min VTotal calculation.
+	 */
+	struct pipe_ctx *vblank_pipe;
+
+	/**
+	 * @context: In the context of populate_subvp_cmd_drr_info(), this
+	 * field it is only necessary to fulfill the requirements for
+	 * dc_state_get_paired_subvp_stream() helper.
+	 */
+	struct dc_state *context;
+};
+
+const struct dc_stream_status mock_dc_stream_state_returned_from_get_paired = {
+	.mall_stream_config =  (struct mall_stream_config) {
+		.paired_stream = &(struct dc_stream_state) {
+			.timing = {
+				.v_total = 216,
+				.h_total = 4000,
+				.v_addressable = 149,
+				.pix_clk_100hz = 5332500,
+				.v_front_porch = 1,
+			},
+		}
+	}
+};
+
+struct pipe_ctx mock_vblank_pipe_parameter = {
+	.stream = &(struct dc_stream_state) {
+		.timing = {
+			.v_total = 2250,
+			.h_total = 4400,
+			.v_addressable = 2160,
+			.pix_clk_100hz = 23760000,
+		},
+	},
+};
+
+const struct pipe_ctx mock_subvp_pipe_parameter = {
+	.stream = &(struct dc_stream_state) {
+		.timing = {
+			.h_total = 4000,
+			.v_addressable = 2160,
+			.pix_clk_100hz = 5332500,
+		},
+	},
+};
+
+struct populate_subvp_cmd_drr_info_test_case subvp_4k144_4k240_case = {
+	.desc = "4k144 and 4k240 displays are the perfect scenario for SubVP",
+	.dc = &(struct dc) {
+		.caps = {
+			.subvp_prefetch_end_to_mall_start_us = 15,
+			.subvp_fw_processing_delay_us = 15,
+		}
+	},
+
+	.subvp_pipe = (struct pipe_ctx *) &mock_subvp_pipe_parameter,
+	.vblank_pipe = &mock_vblank_pipe_parameter,
+	.context = &(struct dc_state) {
+		.stream_count = 1,
+		.streams[0] = mock_subvp_pipe_parameter.stream,
+		.stream_status[0] = mock_dc_stream_state_returned_from_get_paired,
+	},
+};
+
+/**
+ * populate_subvp_cmd_drr_info_with_4k144_4k240_parameters - Check two display with 4k144 and 4k240
+ *
+ * @test: Kunit parameter
+ *
+ * One of the scenarios where SubVP can perform really well is in a
+ * high-resolution display with a high refresh rate. In this sense, this test
+ * targets the parameter configuration for 4k144 and 4k240.
+ */
+static void populate_subvp_cmd_drr_info_with_4k144_4k240_parameters(struct kunit *test)
+{
+	struct dmub_cmd_fw_assisted_mclk_switch_pipe_data_v2 *pipe_data;
+	struct populate_subvp_cmd_drr_info_test_case tmp = subvp_4k144_4k240_case;
+
+	pipe_data = kunit_kzalloc(test,
+				  sizeof(struct dmub_cmd_fw_assisted_mclk_switch_pipe_data_v2),
+				  GFP_KERNEL);
+
+	populate_subvp_cmd_drr_info(tmp.dc,
+				    tmp.context,
+				    tmp.subvp_pipe,
+				    tmp.vblank_pipe,
+				    pipe_data);
+
+	// DRR must be in use
+	KUNIT_EXPECT_EQ(test, true, pipe_data->pipe_config.vblank_data.drr_info.drr_in_use);
+
+	// Use ramp should not be enable
+	KUNIT_EXPECT_EQ(test, false, pipe_data->pipe_config.vblank_data.drr_info.use_ramping);
+
+	// Expects 4ms for the DRR window size
+	KUNIT_EXPECT_EQ(test, 4, pipe_data->pipe_config.vblank_data.drr_info.drr_window_size_ms);
+
+	KUNIT_EXPECT_EQ(test, 2906, pipe_data->pipe_config.vblank_data.drr_info.min_vtotal_supported);
+	KUNIT_EXPECT_EQ(test, 7267, pipe_data->pipe_config.vblank_data.drr_info.max_vtotal_supported);
+}
+
+static struct kunit_case dc_dmub_srv_cases[] = {
+	KUNIT_CASE(populate_subvp_cmd_drr_info_with_4k144_4k240_parameters),
+	{  }
+};
+
+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);