On Thu, Aug 11, 2022 at 8:40 AM Tales Aparecida
<tales.aparecida@xxxxxxxxx> wrote:
>
> From: Maíra Canal <maira.canal@xxxxxx>
>
> 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@xxxxxx>
> Co-developed-by: Magali Lemes <magalilemes00@xxxxxxxxx>
> Signed-off-by: Magali Lemes <magalilemes00@xxxxxxxxx>
> Co-developed-by: Tales Aparecida <tales.aparecida@xxxxxxxxx>
> Signed-off-by: Tales Aparecida <tales.aparecida@xxxxxxxxx>
> ---
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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
