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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/11/22 04:19, David Gow wrote:
On Thu, Aug 11, 2022 at 11:05 AM 'Daniel Latypov' via KUnit
Development <kunit-dev@xxxxxxxxxxxxxxxx> wrote:

On Wed, Aug 10, 2022 at 5:40 PM Tales Aparecida
<tales.aparecida@xxxxxxxxx> 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 <<EOF
CONFIG_KUNIT=y
CONFIG_PCI=y
CONFIG_DRM=y
CONFIG_DRM_AMDGPU=y
CONFIG_DRM_AMD_DC=y
EOF

I haven't tried to apply this series locally yet to double-check, but
I'm pretty sure that means these tests should work as well.
Running under UML would hopefully have faster compile times and be
less resource heavy than a QEMU VM (but I've not measured to verify
that).



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-davidgow@xxxxxxxxxx/

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?= <mairacanal@xxxxxxxxxx>
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 <mairacanal@xxxxxxxxxx>
---
 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, &avail_size,
 				&entries,
 				(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
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1923,7 +1923,7 @@ static int kfd_cpumask_to_apic_id(const struct cpumask *cpumask)
 	first_cpu_of_numa_node = cpumask_first(cpumask);
 	if (first_cpu_of_numa_node >= nr_cpu_ids)
 		return -1;
-#ifdef CONFIG_X86_64
+#if IS_ENABLED(CONFIG_X86)
 	return cpu_data(first_cpu_of_numa_node).apicid;
 #else
 	return first_cpu_of_numa_node;
diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index 72e7a63da79f..d066af22ecea 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
 	bool "AMD DC - Enable new display engine"
 	default y
 	select SND_HDA_COMPONENT if SND_HDA_CORE
-	select DRM_AMD_DC_DCN if (X86 || PPC64)
+	select DRM_AMD_DC_DCN if (X86 || PPC64 || UML)
 	help
 	  Choose this option if you want to use the new display engine
 	  support for AMDGPU. This adds required support for Vega and
diff --git a/drivers/gpu/drm/amd/display/dc/gpio/hw_gpio.c b/drivers/gpu/drm/amd/display/dc/gpio/hw_gpio.c
index 660510842ecf..a591090784ee 100644
--- a/drivers/gpu/drm/amd/display/dc/gpio/hw_gpio.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/hw_gpio.c
@@ -39,7 +39,7 @@
 #define REG(reg)\
 	(gpio->regs->reg)
-static void store_registers(
+static void hw_gpio_store_registers(
 	struct hw_gpio *gpio)
 {
 	REG_GET(MASK_reg, MASK, &gpio->store.mask);
@@ -48,7 +48,7 @@ static void store_registers(
 	/* TODO store GPIO_MUX_CONTROL if we ever use it */
 }
-static void restore_registers(
+static void hw_gpio_restore_registers(
 	struct hw_gpio *gpio)
 {
 	REG_UPDATE(MASK_reg, MASK, gpio->store.mask);
@@ -63,7 +63,7 @@ bool dal_hw_gpio_open(
 {
 	struct hw_gpio *pin = FROM_HW_GPIO_PIN(ptr);
- store_registers(pin);
+	hw_gpio_store_registers(pin);
ptr->opened = (dal_hw_gpio_config_mode(pin, mode) == GPIO_RESULT_OK); @@ -132,7 +132,7 @@ void dal_hw_gpio_close(
 {
 	struct hw_gpio *pin = FROM_HW_GPIO_PIN(ptr);
- restore_registers(pin);
+	hw_gpio_restore_registers(pin);
ptr->mode = GPIO_MODE_UNKNOWN;
 	ptr->opened = false;
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index e4fcbf8a7eb5..825b80590f1b 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -1738,7 +1738,7 @@ static int smu7_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
static bool intel_core_rkl_chk(void)
 {
-#if IS_ENABLED(CONFIG_X86_64)
+#if IS_ENABLED(CONFIG_X86)
 	struct cpuinfo_x86 *c = &cpu_data(0);
return (c->x86 == 6 && c->x86_model == INTEL_FAM6_ROCKETLAKE);
--
2.37.1
------

Best Regards,
- Maíra Canal


Cheers,
-- David



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux