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

2022-10-19 Thread Magali Lemes 
Em sex., 30 de set. de 2022 às 11:14, Harry Wentland
 escreveu:
>
>
>
> On 9/12/22 11:59, Maíra Canal wrote:
> > From: 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/
> >
> > Reviewed-by: David Gow 
> > Signed-off-by: Tales Aparecida 
> > Signed-off-by: Maíra Canal 
> > ---
> >  drivers/gpu/drm/amd/display/Kconfig   |  13 +
> >  drivers/gpu/drm/amd/display/Makefile  |   2 +-
> >  .../gpu/drm/amd/display/tests/.kunitconfig|   6 +
> >  drivers/gpu/drm/amd/display/tests/Makefile|  12 +
> >  .../display/tests/dc/basics/fixpt31_32_test.c | 232 ++
> >  5 files changed, 264 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/amd/display/tests/.kunitconfig
> >  create mode 100644 drivers/gpu/drm/amd/display/tests/Makefile
> >  create mode 100644 
> > drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c
> >
> > diff --git a/drivers/gpu/drm/amd/display/Kconfig 
> > b/drivers/gpu/drm/amd/display/Kconfig
> > index 96cbc87f7b6b..cc44cfe88607 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
>
> I would prefer if we kept the same prefix as for other
> configs in the file: "DRM_AMD_DC". Maybe name all the
> KUNIT configs here "DRM_AMD_DC_KUNIT_XYZ".
>
>
> > + bool "Enable KUnit tests for the 'basics' 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/Makefile 
> > b/drivers/gpu/drm/amd/display/Makefile
> > index 2633de77de5e..0f329aab13f0 100644
> > --- a/drivers/gpu/drm/amd/display/Makefile
> > +++ b/drivers/gpu/drm/amd/display/Makefile
> > @@ -43,7 +43,7 @@ endif
> >  #TODO: remove when Timing Sync feature is complete
> >  subdir-ccflags-y += -DBUILD_FEATURE_TIMING_SYNC=0
> >
> > -DAL_LIBS = amdgpu_dm dc  modules/freesync modules/color 
> > modules/info_packet modules/power dmub/src
> > +DAL_LIBS = amdgpu_dm dc  modules/freesync modules/color 
> > modules/info_packet modules/power dmub/src tests
>
> Can we put these tests into a 'kunit' directory instead of 'tests'?
> We use the codebase on other platforms and 'tests' might be
> confusing to some AMD-internal developers, whereas 'kunit' is
> explicit and will ensure people understand where these are coming
> from and how to use them.
>
> Other than the CONFIG and directory naming these tests look really
> nice. Thanks for your contribution.
>
> Harry
>

Hi, Harry.
Thank you for your feedback and comments. We'll change the directory
and config entries' names in a new version.

Magali

> >
> >  ifdef CONFIG_DRM_AMD_DC_HDCP
> >  DAL_LIBS += modules/hdcp
> > 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/Makefile 
> > b/drivers/gpu/drm/amd/display/tests/Makefile
> > new file mode 100644
> > index ..ef16497318e8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/amd/display/tests/Makefile
> > @@ -0,0 +1,12 @@
> > +# SPDX-License-Identifier: MIT
> > +#
> > +# Makefile for the KUnit Tests for DC
> > +#
> > +
> > +ifdef CONFIG_AMD_DC_BASICS_KUNIT_TEST
> > + DC_TESTS += dc/basics/fixpt31_32_test.o
> > +endif
> > +
> > +AMD_DAL_DC_TESTS = $(addprefix $(AMDDALPATH)/tests/,$(DC_TESTS))
> > +
> > +AMD_DISPLAY_FILES += $(AMD_DAL_DC_TESTS)
> > diff --git a/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c 
> > b/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c
> > new file mode 100644
> > index ..2fc489203499
> > --- /dev/null
> >

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

2022-09-30 Thread Harry Wentland 



On 9/12/22 11:59, Maíra Canal wrote:
> From: 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/
> 
> Reviewed-by: David Gow 
> Signed-off-by: Tales Aparecida 
> Signed-off-by: Maíra Canal 
> ---
>  drivers/gpu/drm/amd/display/Kconfig   |  13 +
>  drivers/gpu/drm/amd/display/Makefile  |   2 +-
>  .../gpu/drm/amd/display/tests/.kunitconfig|   6 +
>  drivers/gpu/drm/amd/display/tests/Makefile|  12 +
>  .../display/tests/dc/basics/fixpt31_32_test.c | 232 ++
>  5 files changed, 264 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/amd/display/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/amd/display/tests/Makefile
>  create mode 100644 
> drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c
> 
> diff --git a/drivers/gpu/drm/amd/display/Kconfig 
> b/drivers/gpu/drm/amd/display/Kconfig
> index 96cbc87f7b6b..cc44cfe88607 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

I would prefer if we kept the same prefix as for other
configs in the file: "DRM_AMD_DC". Maybe name all the
KUNIT configs here "DRM_AMD_DC_KUNIT_XYZ".


> + bool "Enable KUnit tests for the 'basics' 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/Makefile 
> b/drivers/gpu/drm/amd/display/Makefile
> index 2633de77de5e..0f329aab13f0 100644
> --- a/drivers/gpu/drm/amd/display/Makefile
> +++ b/drivers/gpu/drm/amd/display/Makefile
> @@ -43,7 +43,7 @@ endif
>  #TODO: remove when Timing Sync feature is complete
>  subdir-ccflags-y += -DBUILD_FEATURE_TIMING_SYNC=0
>  
> -DAL_LIBS = amdgpu_dm dc  modules/freesync modules/color 
> modules/info_packet modules/power dmub/src
> +DAL_LIBS = amdgpu_dm dc  modules/freesync modules/color 
> modules/info_packet modules/power dmub/src tests

Can we put these tests into a 'kunit' directory instead of 'tests'?
We use the codebase on other platforms and 'tests' might be
confusing to some AMD-internal developers, whereas 'kunit' is
explicit and will ensure people understand where these are coming
from and how to use them.

Other than the CONFIG and directory naming these tests look really
nice. Thanks for your contribution.

Harry

>  
>  ifdef CONFIG_DRM_AMD_DC_HDCP
>  DAL_LIBS += modules/hdcp
> 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/Makefile 
> b/drivers/gpu/drm/amd/display/tests/Makefile
> new file mode 100644
> index ..ef16497318e8
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/display/tests/Makefile
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: MIT
> +#
> +# Makefile for the KUnit Tests for DC
> +#
> +
> +ifdef CONFIG_AMD_DC_BASICS_KUNIT_TEST
> + DC_TESTS += dc/basics/fixpt31_32_test.o
> +endif
> +
> +AMD_DAL_DC_TESTS = $(addprefix $(AMDDALPATH)/tests/,$(DC_TESTS))
> +
> +AMD_DISPLAY_FILES += $(AMD_DAL_DC_TESTS)
> diff --git a/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c 
> b/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c
> new file mode 100644
> index ..2fc489203499
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/display/tests/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

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

2022-09-12 Thread Maíra Canal 
From: 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/

Reviewed-by: David Gow 
Signed-off-by: Tales Aparecida 
Signed-off-by: Maíra Canal 
---
 drivers/gpu/drm/amd/display/Kconfig   |  13 +
 drivers/gpu/drm/amd/display/Makefile  |   2 +-
 .../gpu/drm/amd/display/tests/.kunitconfig|   6 +
 drivers/gpu/drm/amd/display/tests/Makefile|  12 +
 .../display/tests/dc/basics/fixpt31_32_test.c | 232 ++
 5 files changed, 264 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/display/tests/.kunitconfig
 create mode 100644 drivers/gpu/drm/amd/display/tests/Makefile
 create mode 100644 
drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 96cbc87f7b6b..cc44cfe88607 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 KUnit tests for the 'basics' 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/Makefile 
b/drivers/gpu/drm/amd/display/Makefile
index 2633de77de5e..0f329aab13f0 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -43,7 +43,7 @@ endif
 #TODO: remove when Timing Sync feature is complete
 subdir-ccflags-y += -DBUILD_FEATURE_TIMING_SYNC=0
 
-DAL_LIBS = amdgpu_dm dcmodules/freesync modules/color 
modules/info_packet modules/power dmub/src
+DAL_LIBS = amdgpu_dm dcmodules/freesync modules/color 
modules/info_packet modules/power dmub/src tests
 
 ifdef CONFIG_DRM_AMD_DC_HDCP
 DAL_LIBS += modules/hdcp
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/Makefile 
b/drivers/gpu/drm/amd/display/tests/Makefile
new file mode 100644
index ..ef16497318e8
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/tests/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: MIT
+#
+# Makefile for the KUnit Tests for DC
+#
+
+ifdef CONFIG_AMD_DC_BASICS_KUNIT_TEST
+   DC_TESTS += dc/basics/fixpt31_32_test.o
+endif
+
+AMD_DAL_DC_TESTS = $(addprefix $(AMDDALPATH)/tests/,$(DC_TESTS))
+
+AMD_DISPLAY_FILES += $(AMD_DAL_DC_TESTS)
diff --git a/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c 
b/drivers/gpu/drm/amd/display/tests/dc/basics/fixpt31_32_test.c
new file mode 100644
index ..2fc489203499
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/tests/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