Re: [PATCH 1/8] drm/amd/display: Introduce KUnit tests for fixed31_32 library

2022-08-11 Thread Maíra Canal




On 8/11/22 04:19, David Gow wrote:

On Thu, Aug 11, 2022 at 11:05 AM 'Daniel Latypov' via KUnit
Development  wrote:


On Wed, Aug 10, 2022 at 5:40 PM Tales Aparecida
 wrote:


The fixed31_32 library performs a lot of the mathematical operations
involving fixed-point arithmetic and the conversion of integers to
fixed-point representation.

This unit tests intend to assure the proper functioning of the basic
mathematical operations of fixed-point arithmetic, such as
multiplication, conversion from fractional to fixed-point number,
and more. Use kunit_tool to run:

$ ./tools/testing/kunit/kunit.py run --arch=x86_64 \
 --kunitconfig=drivers/gpu/drm/amd/display/tests/


Nice, thanks for including a kunitconfig, that'll help a lot.

Just as an FYI: if you're working on top of torvalds/master, I think
you would no longer need --arch=x86_64.
Before, CONFIG_PCI=y was tricky to enable on UML, but commit
6fc3a8636a7b ("kunit: tool: Enable virtio/PCI by default on UML")
landed for 6.0.

I.e. I can run this command on torvalds/master w/ no other patches applied:

$ ./tools/testing/kunit/kunit.py config --kunitconfig=/dev/stdin <


There are still a few issues which prevent these tests from working on
UML I haven't had a chance to go through all of them yet, but I'll
drop a couple of quick responses to some of the individual patches.

The first thing to note is that amdgpu doesn't actually build on UML
at all without:
https://patchwork.kernel.org/project/linux-rdma/patch/20220218075727.2737623-3-david...@google.com/

IIRC, no-one liked spreading !defined(CONFIG_UML) everwhere. Replacing
it with IS_ENABLED(CONFIG_X86) also works, as X86_64 is defined on
UML, but X86 isn't.

The other issues are basically just other missing #ifdef checks or
dependencies. Plus there's a warning on my system even under x86_64
for integer overflow in the MIN_I64 definition.


Currently, we only support the tests to x86_64, as the DC core don't
build to UML yet. In the future, I intend to send the patch that enables
the tests to run on UML, but for the first iteration, we focused in running
the tests on x86.

If you want with UML, you can apply the following patch (which is working in
progress yet):
--
From cac02e5d714d78e1d69995383b818eec26661925 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ADra=20Canal?= 
Date: Sat, 23 Jul 2022 14:57:41 -0300
Subject: [PATCH] drm/amdgpu: Enable compilation under UML
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The User-Mode Linux configuration is used by KUnit to execute kernel
results directly. This removes the need for a test machine or a virtual
machine. Therefore, as KUnit tests are being added to AMDGPU, it is
interesting to enable compilation under UML, as it eases running the tests
on CI and developers' machines.

Also, the use of UML is encouraged by the KUnit team [1], as it is considered
a better practice to write tests that run on UML to tests that only run
under a particular architecture.

[1] 
https://docs.kernel.org/dev-tools/kunit/usage.html#writing-tests-for-other-architectures

Signed-off-by: Maíra Canal 
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c   | 6 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c   | 2 +-
 drivers/gpu/drm/amd/display/Kconfig | 2 +-
 drivers/gpu/drm/amd/display/dc/gpio/hw_gpio.c   | 8 
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index a5409531a2fd..bbed3284e78e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -1766,7 +1766,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, 
int *avail_size,
return 0;
 }
 
-#ifdef CONFIG_X86_64

+#if IS_ENABLED(CONFIG_X86)
 static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
uint32_t *num_entries,
struct crat_subtype_iolink *sub_type_hdr)
@@ -1825,7 +1825,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
size_t *size)
struct crat_subtype_generic *sub_type_hdr;
int avail_size = *size;
int numa_node_id;
-#ifdef CONFIG_X86_64
+#if IS_ENABLED(CONFIG_X86)
uint32_t entries = 0;
 #endif
int ret = 0;
@@ -1890,7 +1890,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
size_t *size)
sub_type_hdr->length);
 
 		/* Fill in Subtype: IO Link */

-#ifdef CONFIG_X86_64
+#if IS_ENABLED(CONFIG_X86)
ret = kfd_fill_iolink_info_for_cpu(numa_node_id, _size,
,
(struct crat_subtype_iolink *)sub_type_hdr);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 25990bec600d..5f0e58c430a1 100644
--- 

[PATCH 1/8] drm/amd/display: Introduce KUnit tests for fixed31_32 library

2022-08-10 Thread Tales Aparecida
The fixed31_32 library performs a lot of the mathematical operations
involving fixed-point arithmetic and the conversion of integers to
fixed-point representation.

This unit tests intend to assure the proper functioning of the basic
mathematical operations of fixed-point arithmetic, such as
multiplication, conversion from fractional to fixed-point number,
and more. Use kunit_tool to run:

$ ./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kunitconfig=drivers/gpu/drm/amd/display/tests/

Signed-off-by: Tales Aparecida 
---
 drivers/gpu/drm/amd/display/Kconfig   |  13 +
 .../drm/amd/display/dc/basics/fixpt31_32.c|   5 +
 .../gpu/drm/amd/display/tests/.kunitconfig|   6 +
 .../dc/basics/dc_basics_fixpt31_32_test.c | 232 ++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/display/tests/.kunitconfig
 create mode 100644 
drivers/gpu/drm/amd/display/tests/dc/basics/dc_basics_fixpt31_32_test.c

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 96cbc87f7b6b..27981ccb7032 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -55,4 +55,17 @@ config DRM_AMD_SECURE_DISPLAY
 Cooperate with specific DMCU FW.
 
 
+config AMD_DC_BASICS_KUNIT_TEST
+   bool "Enable unit 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 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/basics/fixpt31_32.c 
b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 1726bdf89bae..82747d8ab95a 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -490,3 +490,8 @@ int dc_fixpt_s4d19(struct fixed31_32 arg)
else
return ux_dy(arg.value, 4, 19);
 }
+
+#if IS_ENABLED(CONFIG_AMD_DC_BASICS_KUNIT_TEST)
+#include "../../tests/dc/basics/dc_basics_fixpt31_32_test.c"
+#endif
+
diff --git a/drivers/gpu/drm/amd/display/tests/.kunitconfig 
b/drivers/gpu/drm/amd/display/tests/.kunitconfig
new file mode 100644
index ..60f2ff8158f5
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/tests/.kunitconfig
@@ -0,0 +1,6 @@
+CONFIG_KUNIT=y
+CONFIG_PCI=y
+CONFIG_DRM=y
+CONFIG_DRM_AMDGPU=y
+CONFIG_DRM_AMD_DC=y
+CONFIG_AMD_DC_BASICS_KUNIT_TEST=y
\ No newline at end of file
diff --git 
a/drivers/gpu/drm/amd/display/tests/dc/basics/dc_basics_fixpt31_32_test.c 
b/drivers/gpu/drm/amd/display/tests/dc/basics/dc_basics_fixpt31_32_test.c
new file mode 100644
index ..2fc489203499
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/tests/dc/basics/dc_basics_fixpt31_32_test.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: MIT
+/* Unit tests for display/include/fixed31_32.h and dc/basics/fixpt31_32.c
+ *
+ * Copyright (C) 2022, Tales Aparecida 
+ */
+
+#include 
+#include "os_types.h"
+#include "fixed31_32.h"
+
+static const struct fixed31_32 dc_fixpt_minus_one = { -0x1LL };
+
+/**
+ * dc_fixpt_from_int_test - KUnit test for dc_fixpt_from_int
+ * @test: represents a running instance of a test.
+ */
+static void dc_fixpt_from_int_test(struct kunit *test)
+{
+   struct fixed31_32 res;
+
+   res = dc_fixpt_from_int(0);
+   KUNIT_EXPECT_EQ(test, res.value, dc_fixpt_zero.value);
+
+   res = dc_fixpt_from_int(1);
+   KUNIT_EXPECT_EQ(test, res.value, dc_fixpt_one.value);
+
+   res = dc_fixpt_from_int(-1);
+   KUNIT_EXPECT_EQ(test, res.value, -dc_fixpt_one.value);
+
+   res = dc_fixpt_from_int(INT_MAX);
+   KUNIT_EXPECT_EQ(test, res.value, 0x7FFFLL);
+
+   res = dc_fixpt_from_int(INT_MIN);
+   KUNIT_EXPECT_EQ(test, res.value,
+   0x8000LL); /* implicit negative signal */
+}
+
+/**
+ * dc_fixpt_from_fraction_test - KUnit test for dc_fixpt_from_fraction
+ * @test: represents a running instance of a test.
+ */
+static void dc_fixpt_from_fraction_test(struct kunit *test)
+{
+   struct fixed31_32 res;
+
+   /* Assert signal works as expected */
+   res = dc_fixpt_from_fraction(1LL, 1LL);
+   KUNIT_EXPECT_EQ(test, res.value, dc_fixpt_one.value);
+
+   res = dc_fixpt_from_fraction(-1LL, 1LL);
+   KUNIT_EXPECT_EQ(test, res.value, -dc_fixpt_one.value);
+
+   res = dc_fixpt_from_fraction(1LL, -1LL);
+   KUNIT_EXPECT_EQ(test, res.value, -dc_fixpt_one.value);
+
+   res = dc_fixpt_from_fraction(-1LL, -1LL);
+   KUNIT_EXPECT_EQ(test, res.value, dc_fixpt_one.value);
+
+   /* Assert that the greatest parameter values works as expected */
+   res =