From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9920FD7832E for ; Mon, 2 Dec 2024 15:08:00 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3563610E790; Mon, 2 Dec 2024 15:08:00 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="bpQLzgI0"; dkim-atps=neutral Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by gabe.freedesktop.org (Postfix) with ESMTPS id A73C810E790; Mon, 2 Dec 2024 15:07:59 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 41B7AA40EDD; Mon, 2 Dec 2024 15:06:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 20ACDC4CEDA; Mon, 2 Dec 2024 15:07:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1733152078; bh=Znclq/zzTXpSoR6+meWmkAfqzY+2c5VrITqNiZrZqQM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bpQLzgI0JGP8xrDGf/kxIl5TcRYS37pFNZ115YibxagwtFonaQVpXcnjcFCBesx3B 949hAUEU7V8hMO+LYZt5K9RoG1oWOicf8GB2dP/e72EYziw9XjcjRTTXecFqFztOhO /TdhiuM+9mIty2DC6bG6Bdmj8BF+UblnejQPA8sWKhiT2g520du3JwuGBwMDznZEX1 hU+f8CF0pf7Yz5Lcwvbo/JAohSaSOhVFI2o+dBFCXvvN7HbmM37RiqtVwNlt/50W51 vi8YO9RJKP2G5fjRx1lBrBhFJEUMJVg3XG9wveQItOQ8XLZd6706/25lF8G4S/PNWp cWephrEHpVXvg== Date: Mon, 2 Dec 2024 16:07:56 +0100 From: Maxime Ripard To: Imre Deak Cc: Jani Nikula , intel-gfx@lists.freedesktop.org, Maarten Lankhorst , Thomas Zimmermann , Dave Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, Rodrigo Vivi Subject: Re: [PATCH v2 1/4] drm/dp: Add a way to init/add a connector in separate steps Message-ID: <20241202-bald-finicky-coyote-e9ff4c@houat> References: <20241126161859.1858058-1-imre.deak@intel.com> <20241126161859.1858058-2-imre.deak@intel.com> <20241129-wild-cobra-of-thunder-829d1f@houat> <20241202-real-benevolent-skunk-a9f5e0@houat> <87ldwy5lvb.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha384; protocol="application/pgp-signature"; boundary="zlixp4geqfwx2xs4" Content-Disposition: inline In-Reply-To: X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" --zlixp4geqfwx2xs4 Content-Type: text/plain; protected-headers=v1; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH v2 1/4] drm/dp: Add a way to init/add a connector in separate steps MIME-Version: 1.0 On Mon, Dec 02, 2024 at 03:24:43PM +0200, Imre Deak wrote: > On Mon, Dec 02, 2024 at 02:07:36PM +0200, Jani Nikula wrote: > > On Mon, 02 Dec 2024, Maxime Ripard wrote: > > > It's not about whether we have a problem or not: you introduce new > > > framework functions, you need to have kunit tests to check their > > > behaviour. > >=20 > > I don't fundamentally disagree with that goal, but it does seem like a > > pretty drastic policy change. I don't recall a discussion where we made > > that decision, nor can I find any documentation stating this. Or what > > exactly the requirement is; it's totally unclear to me. > >=20 > > Had I been involved, I would've pointed out that while adding tests is > > good, it inevitably increases the friction of adding new stuff to drm > > core. It's super tempting for people to just get their jobs done. If > > doing the right thing adds yet another hurdle, we may see more stuff > > being added in drivers instead of drm core. > >=20 > > (Case in point, we already hacked around the problem being solved here > > with commit d58f65df2dcb ("drm/i915/dp_mst: Fix connector initialization > > in intel_dp_add_mst_connector()"). We could've just dropped the ball > > right there.) >=20 > Fwiw, in this case adding tests for drm_connector_init_core() and > drm_connector_add() looks simple enough. >=20 > IIUC it's the 3 testcases in drmm_connector_init_tests[] performed for > drm_connector_init_core() and additional 3 test cases checking that (1) > drm_connector_init_core() doesn't add the connector to the connector > list, (2) drm_connector_add() adds it and (3) drm_connector_add() fails > (by not adding the connector to the list and emitting a dmesg WARN) if > drm_connector_init_core() was not called for the connector previously. > For the last test I actually need to add the corresponding assert/early > return to drm_connector_add(). >=20 > If Maxim could confirm the above, I could resend the patchset adding > these tests. Yep, sounds great, thanks! Maxime --zlixp4geqfwx2xs4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCZ03NSwAKCRAnX84Zoj2+ dm1SAYC5dcscCqJzr68dXCO5+48WpeGh/Ekh0Ojqf69MbFSay4iiFW1UjPIgE8if qd5FAIcBgKpIIgO8h1VjlWTo8ZpYnYtLzn4SRLihYzQIfDKFK86iHpiu7g9D9cEn qFfbYTW+wQ== =5UCA -----END PGP SIGNATURE----- --zlixp4geqfwx2xs4--