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

Deepak Singh Rawat
2018-10-16T16:14:31Z

>
> 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@linux.intel.com
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > > > > Cc: Daniel Stone <daniel@fooishbar.org>
> > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > Cc: igt-dev@lists.freedesktop.org
> > > > > Cc: petri.latvala@intel.com
> > > > > Cc: chris@chris-wilson.co.uk
> > > > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > > > ---
> > > > > 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@ffwll.ch>
> > > >
> > > > 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@lists.freedesktop.org