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

From: Daniel Vetter <daniel@ffwll.ch>
To: Deepak Singh Rawat <drawat@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	linux-graphics-maintainer <linux-graphics-maintainer@vmware.com>
Subject: Re: [PATCH 04/18] drm/selftest: Add drm damage helper selftest
Date: Thu, 11 Oct 2018 18:17:19 +0200	[thread overview]
Message-ID: <20181011161719.GI31561@phenom.ffwll.local> (raw)
In-Reply-To: <BYAPR05MB4181C70609B2BB80CDA9A52FBAE70@BYAPR05MB4181.namprd05.prod.outlook.com>

On Tue, Oct 09, 2018 at 10:43:04PM +0000, Deepak Singh Rawat wrote:
> > 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
> > 
> > I think it'd be better to add the damage tests to the existing drm-helper
> > selftest (but as a separate file ofc), using the
> > 
> > test-drm-helper-$(CONFIG_DRM_DEBUG_SELFTEST) +=
> > 
> > syntax. That avoids having to write yet another igt wrapper test for this.
> > Also, if you cc intel-gfx@lists.fd.o with this our CI will run the new
> > tests for you automatically.
> > 
> > Cheers, Daniel
> 
> Hi Daniel, thanks for the review.
> 
> It looks like adding the damage tests to existing drm-helper-selftest need either
> renaming the module name (test-drm-helper.ko) or change the filename of
> test-drm-helper.c. This is because now test-drm-helper.c and
> test-drm-damage-helper.c have to link together and kernel module is of same
> name which is causing circular dependency. Both of which looks overkill to me.

I think this is actually the right thing to do here. Sorry for not being
clear.

> If it is alright with you can we have separate test module. I already had a patch
> to integrate the damage test module with igt.

Yeah that needs to be adjusted too. Kinda wondering why we don't yet have
an igt wrapper for the existing drm-helper stuff ...
-Daniel

> 
> Thanks,
> Deepak
> 
> > 
> > 
> > > diff --git a/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > b/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > > new file mode 100644
> > > index 000000000000..3a1cbe05bef0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/selftests/drm_damage_helper_selftests.h
> > > @@ -0,0 +1,22 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +selftest(damage_iter_no_damage, igt_damage_iter_no_damage)
> > > +selftest(damage_iter_no_damage_fractional_src,
> > igt_damage_iter_no_damage_fractional_src)
> > > +selftest(damage_iter_no_damage_src_moved,
> > igt_damage_iter_no_damage_src_moved)
> > > +selftest(damage_iter_no_damage_fractional_src_moved,
> > igt_damage_iter_no_damage_fractional_src_moved)
> > > +selftest(damage_iter_no_damage_not_visible,
> > igt_damage_iter_no_damage_not_visible)
> > > +selftest(damage_iter_no_damage_no_crtc,
> > igt_damage_iter_no_damage_no_crtc)
> > > +selftest(damage_iter_no_damage_no_fb,
> > igt_damage_iter_no_damage_no_fb)
> > > +selftest(damage_iter_simple_damage,
> > igt_damage_iter_simple_damage)
> > > +selftest(damage_iter_single_damage, igt_damage_iter_single_damage)
> > > +selftest(damage_iter_single_damage_intersect_src,
> > igt_damage_iter_single_damage_intersect_src)
> > > +selftest(damage_iter_single_damage_outside_src,
> > igt_damage_iter_single_damage_outside_src)
> > > +selftest(damage_iter_single_damage_fractional_src,
> > igt_damage_iter_single_damage_fractional_src)
> > > +selftest(damage_iter_single_damage_intersect_fractional_src,
> > igt_damage_iter_single_damage_intersect_fractional_src)
> > > +selftest(damage_iter_single_damage_outside_fractional_src,
> > igt_damage_iter_single_damage_outside_fractional_src)
> > > +selftest(damage_iter_single_damage_src_moved,
> > igt_damage_iter_single_damage_src_moved)
> > > +selftest(damage_iter_single_damage_fractional_src_moved,
> > igt_damage_iter_single_damage_fractional_src_moved)
> > > +selftest(damage_iter_damage, igt_damage_iter_damage)
> > > +selftest(damage_iter_damage_one_intersect,
> > igt_damage_iter_damage_one_intersect)
> > > +selftest(damage_iter_damage_one_outside,
> > igt_damage_iter_damage_one_outside)
> > > +selftest(damage_iter_damage_src_moved,
> > igt_damage_iter_damage_src_moved)
> > > +selftest(damage_iter_damage_not_visible,
> > igt_damage_iter_damage_not_visible)

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel