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 X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D6CBC64E7B for ; Tue, 1 Dec 2020 17:08:38 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C691120679 for ; Tue, 1 Dec 2020 17:08:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="cudKydFe"; dkim=temperror (0-bit key) header.d=cerno.tech header.i=@cerno.tech header.b="nzFriMZy"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="qahOjexe" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C691120679 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cerno.tech Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=9EwCGws7eG+woeulI/Bdc4YOx3jHl++1t8tanRZUAww=; b=cudKydFeb0Ua1wgX7gmptulDU TxG9xqlY2UdWUj8z3Git8j4/+J/KV95h2hFDG022QJAD2N9vJN+pCcbIstuIBg2E//yUIt5HzZYWJ vgE8TjVnn8axC2FzU9iN9G9z+W/1PwX8lGonGYZ6VUYGeogqr7nl05YTaAwL1p8mbbkormLh3TvVc n9LrnsL3Gj44gxLws9rqt+tJLYME/LAO0zC8iLWAOKQsnajE9Wd/OLvGuwAorIwJepBw7hGHlK1xl vw0kOSekqKQGX2jg0H4MxnpVzj05qn7f7yBESc5pWfLiZd1wJIdV8x1ELV92tHTuLR9SqUaEbUxSe Oy87keW+w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kk97E-0005ul-B2; Tue, 01 Dec 2020 17:07:00 +0000 Received: from new1-smtp.messagingengine.com ([66.111.4.221]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kk97A-0005uK-Lj; Tue, 01 Dec 2020 17:06:58 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailnew.nyi.internal (Postfix) with ESMTP id 506485804DF; Tue, 1 Dec 2020 12:06:53 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Tue, 01 Dec 2020 12:06:53 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=Xd6YAdRZAzxCRta9Q2GO691ERFj 8lTLDd0OONx3VR7Y=; b=nzFriMZy/B9T0a2mZez5WqDg34pqluEv+kVPTdqwZmW NGIvIz9YCD8A/LSaUikbzMJmmE2gRkE0db+M8WYLq3K5G9H68pGyKoLJDSdrKrzi SZHuZuzqKOrcU9YqpKU40jRFVslV2WrszqS9g0rx4AVQgWci1JvXBLIJsqr5/qsz n0nUJAlCpr27JY2Da+V/CXqO8o0EnA8F5lIezwivmndvRKXoHHyDQb/cJfKE0byD i0bTWIR/EmdJZXKKB2O5UeD5ae7t9q31raYAinM+yz4YgXO2IQHyOM8vWuWnx8Pa RaQbuXuoejVZJsqWGKi2Hs0iDSGT7PhscQrBTABKjYQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=Xd6YAd RZAzxCRta9Q2GO691ERFj8lTLDd0OONx3VR7Y=; b=qahOjexe5lLpNScI+3Ef/D FRE89sGJvpgJuhcn6Mef601uLSqtLIuYuPx1m61KoPa1HZVn6230aaP84Lmi6qCN OBBcs66imAaI/N7ig70V7XiHp+r83U1ZTLBjBkKIZxwsz6wP2UTWTF1QJdE+s9j0 e9k8uXN0Og+8AvAjlktgxs+F6vTyGxtThXDvGLmDxvBetIxr3PvH9WA2V51rvZ2Y VMdiu4lCqtmYx5zXpo+uAB3Y+glVvC1ffMUVCU8BNdFi7b/AJq8rsoXLVhCdZZBc SlXcbKUctG8JxgoriVWZczhSgWQvRZjkO6yHWZph8kCu21QdH/YeatEpppCvw3Sg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrudeivddgleelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepleekgeehhfdutdeljefgleejffehfffgieejhffgueefhfdtveetgeehieeh gedunecukfhppeeltddrkeelrdeikedrjeeinecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepmhgrgihimhgvsegtvghrnhhordhtvggthh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 8EEB83280063; Tue, 1 Dec 2020 12:06:50 -0500 (EST) Date: Tue, 1 Dec 2020 18:06:49 +0100 From: Maxime Ripard To: Thomas Zimmermann Subject: Re: [PATCH 6/8] drm/vc4: kms: Wait on previous FIFO users before a commit Message-ID: <20201201170649.g2uo6c7ngxdvfw3j@gilmour> References: <20201113152956.139663-1-maxime@cerno.tech> <20201113152956.139663-7-maxime@cerno.tech> <8c5f37b3-a4ff-d4d0-ebe0-8bc931416293@suse.de> MIME-Version: 1.0 In-Reply-To: <8c5f37b3-a4ff-d4d0-ebe0-8bc931416293@suse.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201201_120657_861661_A0012615 X-CRM114-Status: GOOD ( 29.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Tim Gover , Dave Stevenson , David Airlie , Maarten Lankhorst , dri-devel@lists.freedesktop.org, Eric Anholt , Rob Herring , bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, Daniel Vetter , Frank Rowand , Phil Elwell , linux-arm-kernel@lists.infradead.org Content-Type: multipart/mixed; boundary="===============7405887329575887898==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============7405887329575887898== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rp2onweshvrmr67z" Content-Disposition: inline --rp2onweshvrmr67z Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Thomas, On Fri, Nov 20, 2020 at 02:19:45PM +0100, Thomas Zimmermann wrote: > Am 13.11.20 um 16:29 schrieb Maxime Ripard: > > If we're having two subsequent, non-blocking, commits on two different > > CRTCs that share no resources, there's no guarantee on the order of > > execution of both commits. >=20 > Can there only ever be two commits that flip order? It needs a bit of bad luck, but the patch 2 provides a bit more details. Basically, if there's two subsequent non-blocking commits, affecting different CRTCs without anything shared between those CRTCs (so no plane, encoder or connector in common), you have no guarantee on the commit order. Most of the time it's not a big deal precisely because they don't share anything. However if the private state is shared between the CRTCs then it becomes an issue and we need to make sure that the ordering is respected. > > However, the second one will consider the first one as the old state, > > and will be in charge of freeing it once that second commit is done. > >=20 > > If the first commit happens after that second commit, it might access > > some resources related to its state that has been freed, resulting in a > > use-after-free bug. > >=20 > > The standard DRM objects are protected against this, but our HVS private > > state isn't so let's make sure we wait for all the previous FIFO users > > to finish their commit before going with our own. >=20 > I'd appreciate a comment in the code that explains a bit how this works. > It's sort of clear to me, but not enough to fully get it. I'm not sure to get what "this" means and what do you want me to comment there? > >=20 > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/vc4/vc4_kms.c | 118 +++++++++++++++++++++++++++++++++- > > 1 file changed, 117 insertions(+), 1 deletion(-) > >=20 > > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_km= s.c > > index 3034a5a6637e..849bc6b4cea4 100644 > > --- a/drivers/gpu/drm/vc4/vc4_kms.c > > +++ b/drivers/gpu/drm/vc4/vc4_kms.c > > @@ -40,6 +40,11 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct= drm_private_state *priv) > > struct vc4_hvs_state { > > struct drm_private_state base; > > unsigned int unassigned_channels; > > + > > + struct { > > + unsigned in_use: 1; > > + struct drm_crtc_commit *last_user; >=20 > Can these updates run concurrently? If so, the concurrency control via > in_use is dubious. No, there's only ever one commit being done. We just need to make sure they are in the right order. I'll address your other comments Maxime --rp2onweshvrmr67z Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX8Z4KQAKCRDj7w1vZxhR xRdQAQDoXrTwcEx5wJMf83iJozqAbfCdz7arV6qk2uQFSKxBfAD/amH5tGFvCrOx XbpFO45SKECLhELNs5ADEMKKwVXE1wE= =LWnB -----END PGP SIGNATURE----- --rp2onweshvrmr67z-- --===============7405887329575887898== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============7405887329575887898==--