RE: [PATCH v3 04/18] drm/selftest: Add drm damage helper selftest

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

 



> 
> On Tue, Oct 16, 2018 at 02:21:17PM +0200, Daniel Vetter wrote:
> > On Mon, Oct 15, 2018 at 04:11:41PM +0000, Deepak Singh Rawat wrote:
> > > > On Wed, Oct 10, 2018 at 05:16:43PM -0700, Deepak Rawat wrote:
> > > > > Selftest for drm damage helper iterator functions.
> > > > >
> > > > > Cc: ville.syrjala@xxxxxxxxxxxxxxx
> > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > > > Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> > > > > Cc: Daniel Stone <daniel@xxxxxxxxxxxxx>
> > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > > Cc: igt-dev@xxxxxxxxxxxxxxxxxxxxx
> > > > > Cc: petri.latvala@xxxxxxxxx
> > > > > Cc: chris@xxxxxxxxxxxxxxxxxx
> > > > > Signed-off-by: Deepak Rawat <drawat@xxxxxxxxxx>
> > > > > ---
> > > > >  drivers/gpu/drm/selftests/Makefile            |   3 +-
> > > > >  .../selftests/drm_damage_helper_selftests.h   |  22 +
> > > > >  .../drm/selftests/test-drm_damage_helper.c    | 844
> > > > ++++++++++++++++++
> > > > >  3 files changed, 868 insertions(+), 1 deletion(-)
> > > > >  create mode 100644
> > > > drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > > > >  create mode 100644 drivers/gpu/drm/selftests/test-
> > > > drm_damage_helper.c
> > > > >
> > > > > diff --git a/drivers/gpu/drm/selftests/Makefile
> > > > b/drivers/gpu/drm/selftests/Makefile
> > > > > index 9fc349fa18e9..88ac216f5962 100644
> > > > > --- a/drivers/gpu/drm/selftests/Makefile
> > > > > +++ b/drivers/gpu/drm/selftests/Makefile
> > > > > @@ -1 +1,2 @@
> > > > > -obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-
> drm-
> > > > helper.o
> > > > > +obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-
> drm-
> > > > helper.o \
> > > > > +				    test-drm_damage_helper.o
> > > >
> > > > With the testcase intagrated into the test-drm-helper.ko module, for
> > > > patches 1-4 in this series:
> > > >
> > > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > >
> > > > Obviously needs some adjusting on the igt side too, since we seem to
> be
> > > > missing the igt scaffolding for tests-drm-helper.ko.
> > > > -Daniel
> > >
> > > Hi Daniel,
> > >
> > > Thanks for the review. I am a little confused here. Should we have single
> > > kernel module for drm plane helper selftest and damage helper selftest?
> > > Also shall I rename the kernel selfttest to kms_*?
> > >
> > > For user-space igt test it should be it makes sense to rename to
> kms_selftets?
> >
> > Since I went back&forth on this way too many times:
> > - igt should be called kms_selftest. Please work together with igt
> >   maintainers (Arek and Petri), since we also need to update the CI
> >   building infrastructure to make sure it updates the list of subtests
> >   implemented by the kernel.
> >
> > - Kernel module I'd call test-drm_modeset.ko. That kernel module can then
> >   include the existing test-drm-helper.c (could probably rename to
> >   test-drm_plane_helper.c for clarity) and your new damage helper (named
> >   test-drm_damage_helper.c for consistency).
> >
> > Does that make sense to everyone?
> 
> I was trying to add some selftests, as well here [1], with that in
> mind, I think it makes sense to have just one module, call it
> "test-drm_modeset" or whatever and separate the tests source code base
> on whatever core functionality they are testing.
> 
> Besides compiling everything together, probably some stuff will have
> to move out of test-drm-helper.c into some common header. For example
> this "FAIL/FAIL_ON" macros
> 

Hi,

Thanks for your input. I have similar change in mind after suggestion from
Daniel. Below is initial draft I did yesterday, will move common code to
a common header.

I hope this aligns with what you are doing.

---
 drivers/gpu/drm/selftests/Makefile            |  4 ++-
 .../gpu/drm/selftests/drm_helper_selftests.c  | 27 +++++++++++++++++++
 .../gpu/drm/selftests/drm_helper_selftests.h  | 15 +++++------
 ...-helper.c => drm_plane_helper_selftests.c} | 16 ++++-------
 .../selftests/drm_plane_helper_selftests.h    |  9 +++++++
 5 files changed, 51 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/selftests/drm_helper_selftests.c
 rename drivers/gpu/drm/selftests/{test-drm-helper.c => drm_plane_helper_selftests.c} (96%)
 create mode 100644 drivers/gpu/drm/selftests/drm_plane_helper_selftests.h

diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
index 9fc349fa18e9..560117d64658 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -1 +1,3 @@
-obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm-helper.o
+test-drm_helper-y := drm_helper_selftests.o drm_plane_helper_selftests.o
+
+obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_helper.o
diff --git a/drivers/gpu/drm/selftests/drm_helper_selftests.c b/drivers/gpu/drm/selftests/drm_helper_selftests.c
new file mode 100644
index 000000000000..873db462fa35
--- /dev/null
+++ b/drivers/gpu/drm/selftests/drm_helper_selftests.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test cases for the drm_kms_helper functions
+ */
+
+#include <linux/module.h>
+
+#include "drm_helper_selftests.h"
+
+static int __init test_drm_helper_init(void)
+{
+	int err;
+
+	err = drm_run_plane_helper_selftests();
+
+	return err;
+}
+
+static void __exit test_drm_helper_exit(void)
+{
+}
+
+module_init(test_drm_helper_init);
+module_exit(test_drm_helper_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/selftests/drm_helper_selftests.h b/drivers/gpu/drm/selftests/drm_helper_selftests.h
index 9771290ed228..82d076a20bb3 100644
--- a/drivers/gpu/drm/selftests/drm_helper_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_helper_selftests.h
@@ -1,9 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/* List each unit test as selftest(name, function)
- *
- * The name is used as both an enum and expanded as igt__name to create
- * a module parameter. It must be unique and legal for a C identifier.
- *
- * Tests are executed in order by igt/drm_selftests_helper
- */
-selftest(check_plane_state, igt_check_plane_state)
+
+#ifndef __TEST_DRM_HELPER_H__
+#define __TEST_DRM_HELPER_H__
+
+int drm_run_plane_helper_selftests(void);
+
+#endif
diff --git a/drivers/gpu/drm/selftests/test-drm-helper.c b/drivers/gpu/drm/selftests/drm_plane_helper_selftests.c
similarity index 96%
rename from drivers/gpu/drm/selftests/test-drm-helper.c
rename to drivers/gpu/drm/selftests/drm_plane_helper_selftests.c
index a015712b43e8..11941c14a791 100644
--- a/drivers/gpu/drm/selftests/test-drm-helper.c
+++ b/drivers/gpu/drm/selftests/drm_plane_helper_selftests.c
@@ -1,16 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Test cases for the drm_kms_helper functions
+ * Test cases for the drm_plane_helper functions
  */
 
-#define pr_fmt(fmt) "drm_kms_helper: " fmt
-
-#include <linux/module.h>
+#define pr_fmt(fmt) "drm_plane_helper: " fmt
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_modes.h>
 
-#define TESTS "drm_helper_selftests.h"
+#define TESTS "drm_plane_helper_selftests.h"
 #include "drm_selftest.h"
 
 #define FAIL(test, msg, ...) \
@@ -232,7 +231,7 @@ static int igt_check_plane_state(void *ignored)
 
 #include "drm_selftest.c"
 
-static int __init test_drm_helper_init(void)
+int drm_run_plane_helper_selftests(void)
 {
 	int err;
 
@@ -240,8 +239,3 @@ static int __init test_drm_helper_init(void)
 
 	return err > 0 ? 0 : err;
 }
-
-module_init(test_drm_helper_init);
-
-MODULE_AUTHOR("Intel Corporation");
-MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/selftests/drm_plane_helper_selftests.h b/drivers/gpu/drm/selftests/drm_plane_helper_selftests.h
new file mode 100644
index 000000000000..9771290ed228
--- /dev/null
+++ b/drivers/gpu/drm/selftests/drm_plane_helper_selftests.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* List each unit test as selftest(name, function)
+ *
+ * The name is used as both an enum and expanded as igt__name to create
+ * a module parameter. It must be unique and legal for a C identifier.
+ *
+ * Tests are executed in order by igt/drm_selftests_helper
+ */
+selftest(check_plane_state, igt_check_plane_state)
-- 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux