[PATCH 2/8] drm/amd/display: Introduce KUnit tests to the bw_fixed library

David Gow davidgow at google.com
Thu Aug 11 07:34:50 UTC 2022


On Thu, Aug 11, 2022 at 8:40 AM Tales Aparecida
<tales.aparecida at gmail.com> wrote:
>
> From: Maíra Canal <maira.canal at usp.br>
>
> KUnit unifies the test structure and provides helper tools that simplify
> the development of tests. Basic use case allows running tests as regular
> processes, which makes easier to run unit tests on a development machine
> and to integrate the tests in a CI system.
>
> This commit introduces a unit test to the bw_fixed library, which
> performs a lot of the mathematical operations involving fixed-point
> arithmetic and the conversion of integers to fixed-point representation
> inside the Display Mode Library.
>
> As fixed-point representation is the base foundation of the DML calcs
> operations, 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.  You can run it with: ./tools/testing/kunit/kunit.py run \
>         --arch=x86_64 \
>         --kunitconfig=drivers/gpu/drm/amd/display/tests/
>
> Signed-off-by: Maíra Canal <maira.canal at usp.br>
> Co-developed-by: Magali Lemes <magalilemes00 at gmail.com>
> Signed-off-by: Magali Lemes <magalilemes00 at gmail.com>
> Co-developed-by: Tales Aparecida <tales.aparecida at gmail.com>
> Signed-off-by: Tales Aparecida <tales.aparecida at gmail.com>
> ---

Not directly related to this patch, but I get a whole stack of
warnings about the definition of MIN_I64 causing integer overflow:
../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/../../../tests/dc/dml/calcs/bw_fixed_test.c:214:31:
note: in expansion of macro ‘MIN_I64’
 214 |         KUNIT_EXPECT_EQ(test, MIN_I64 + 1, res.value);
     |                               ^~~~~~~
../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/bw_fixed.c:30:19:
warning: integer overflow in expression ‘-9223372036854775808’ of type
‘long long int’ results in ‘-9223372036854775808’ [-Woverflow]
  30 |         (int64_t)(-(1LL << 63))
     |                   ^

This seems to fix it (I'll re-send it out as a separate patch so gmail
doesn't mangle it once I'm a bit more convinced it's the best
implementation):

--- 8< ---
>From 84e84664873dc9e98dff5ee9f74d95872e6cd423 Mon Sep 17 00:00:00 2001
From: David Gow <davidgow at google.com>
Date: Thu, 11 Aug 2022 15:21:02 +0800
Subject: [PATCH] drm/amd/display: MIN_I64 definition causes overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The definition of MIN_I64 in bw_fixed.c can cause gcc to whinge about
integer overflow, because it is treated as a positive value, which is
then negated. The temporary postive value is not necessarily
representable.

This causes the following warning:
../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/bw_fixed.c:30:19:
warning: integer overflow in expression ‘-9223372036854775808’ of type
‘long long int’ results in ‘-9223372036854775808’ [-Woverflow]
  30 |         (int64_t)(-(1LL << 63))
     |                   ^

Writing out (INT_MIN - 1) - 1 works instead.

Signed-off-by: David Gow <davidgow at google.com>
---
drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c
b/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c
index fbe8d0661396..3850f7f0f679 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c
@@ -26,12 +26,12 @@
#include "bw_fixed.h"


-#define MIN_I64 \
-       (int64_t)(-(1LL << 63))
-
#define MAX_I64 \
       (int64_t)((1ULL << 63) - 1)

+#define MIN_I64 \
+       (-MAX_I64 - 1)
+
#define FRACTIONAL_PART_MASK \
       ((1ULL << BW_FIXED_BITS_PER_FRACTIONAL_PART) - 1)

--
2.37.1.595.g718a3a8f04-goog
--- 8< ---

Otherwise, this test seems to okay. I'll review it (and the series)
more properly over then next few days.

Cheers,
-- David


More information about the amd-gfx mailing list