Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i8771445c:Fastmail
Date:   Mon, 15 Aug 2022 10:42:13 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Thomas Zimmermann <tzimmermann@suse.de>
Cc:     Jernej Skrabec <jernej.skrabec@gmail.com>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Samuel Holland <samuel@sholland.org>,
        Daniel Vetter <daniel@ffwll.ch>, Emma Anholt <emma@anholt.net>,
        David Airlie <airlied@linux.ie>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Kevin Hilman <khilman@baylibre.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Dom Cobley <dom@raspberrypi.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        linux-amlogic@lists.infradead.org, linux-sunxi@lists.linux.dev,
        Phil Elwell <phil@raspberrypi.com>,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 08/35] drm/client: Add some tests for
 drm_connector_pick_cmdline_mode()
Message-ID: <20220815084213.olcvp6qhmhhykz6t@houat>
References: <20220728-rpi-analog-tv-properties-v1-0-3d53ae722097@cerno.tech>
 <20220728-rpi-analog-tv-properties-v1-8-3d53ae722097@cerno.tech>
 <14d698dc-172c-773f-7ea1-7101b9f5b77f@suse.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="rlnnmrn6tqqsyh2j"
Content-Disposition: inline
In-Reply-To: <14d698dc-172c-773f-7ea1-7101b9f5b77f@suse.de>
Precedence: bulk


--rlnnmrn6tqqsyh2j
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Thomas,

On Tue, Aug 02, 2022 at 12:14:29PM +0200, Thomas Zimmermann wrote:
> Am 29.07.22 um 18:34 schrieb Maxime Ripard:
> > drm_connector_pick_cmdline_mode() is in charge of finding a proper
> > drm_display_mode from the definition we got in the video=3D command line
> > argument.
> >=20
> > Let's add some unit tests to make sure we're not getting any regressions
> > there.
> >=20
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >=20
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm=
_client_modeset.c
> > index bbc535cc50dd..ee6b8f193c24 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -1237,3 +1237,7 @@ int drm_client_modeset_dpms(struct drm_client_dev=
 *client, int mode)
> >   	return ret;
> >   }
> >   EXPORT_SYMBOL(drm_client_modeset_dpms);
> > +
> > +#ifdef CONFIG_DRM_KUNIT_TEST
> > +#include "tests/drm_mode_test.c"
> > +#endif
>=20
> Including source files is somewhat ugly, prolongs compile times and could
> even interfere with the actual source code. Can we do this in some other
> way?

Yeah, this irks me a bit as well, but it's the preferred way of doing it
according to the kunit doc:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/usage.html#testing-s=
tatic-functions

> I suggest to add the tests here and export them for use in the test case.
> Something like
>=20
> #ifdef CONFIG_DRM_KUNIT_TEST
> static drm_mode_res_1920_1080_60()
> {
>   ...
> }
>=20
> struct kunit_case drm_mode_tests[] =3D {
>   drm_mode_res_1920_1080_60
> };
> EXPORT_SYMBOL(drm_mode_tests);
> #endif
>=20
> This would add the tests next to the tested code, but leave the test driv=
er
> in drm_mode_test.c.

The test suite is fairly small for now, but if we end up with dozens of
tests like what is there for the command line parser (which could happen
for that kind of functions), I'm very afraid that the original source
file will become unreadable, while this has the advantage to keep the
original file readability.

Maxime

--rlnnmrn6tqqsyh2j
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYvoG5QAKCRDj7w1vZxhR
xc0mAQDuFbvOznqy7wPFfo7D22siO3vakpbi8JYCjof/aUhSWQEAh1/POG4ZPo9G
5liKYUHEyb8jsiIsA5GvnKqbiUcWNQw=
=hjzk
-----END PGP SIGNATURE-----

--rlnnmrn6tqqsyh2j--