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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 4D627CD4F48 for ; Mon, 18 May 2026 07:17:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=M9099tiDekmWa8//UFxvEVDIrktS3edR0WDOjpoiEiA=; b=srebeFky1jnJFYg4Jt3XEsCpWL NVPyXOfhmTZCi683pqtA3lF2QL3M5DqcUnad1sjslDqy6+dfSYoidgrr9/ddBm0fRTjlsqTQZKkA/ MH2R8nIhpTQzWPx1tiDXumTMV709n+uo6SmXt6edyCeo4kbu3uKn2DAvuoLNR3NMHNfArgMD4G4et FCbRe78p69sckRzXQjMxOB8VbO5gLMkSNvdwnwk7VOtpNglhWIoZozyFcviW3WiBwnl9WQlLpqOL6 EStYPySxavXEMEPNHlS5zdTF9FmI8ceQaxU4yqv4yGM2GRfCkhXnTOAHE4zAw0I9k9DMEv4teYEnY U4RrnTEw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wOsDl-0000000EYHT-0yM6; Mon, 18 May 2026 07:17:01 +0000 Received: from bali.collaboradmins.com ([2a01:4f8:201:9162::2]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wOsDj-0000000EYGM-1vAL; Mon, 18 May 2026 07:17:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1779088614; bh=KqXz31fFcllBghfI8pU+i0+gahHJxk311r9iPzSXzpE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Ro/navyn0/DVaD7IcAs2bwN9U0+qbMBiTf0oUjhMcD0ROwgB/USIvQN1YZMsz174L qBnN+vWuGWz1lVeR2jYgmElu1mjPxSMMGXIGAjsej3MU7FVBjkaUWJZstMBCZOlCyO Lv4cAJJONcuvfiREYLHFMuSa+lE3sWIqJRj8ubGV6P0FReLdNK4Fk54j0akWhe7mYb Ym19npygDaWlq5bpJbStix+kkZv9vaF6FznF0nLYxsVhvf2siW6oSF6AwFgJReakda iiCScxbzTpDsRQNchfi+ZPOY5WefpAq+Cs+QuebRutmtjhkxc98uX1vYr3I/U2XKv+ 5Ebczo3CckW+Q== Received: from fedora (unknown [100.64.0.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id BDEFE17E04E2; Mon, 18 May 2026 09:16:53 +0200 (CEST) Date: Mon, 18 May 2026 09:16:50 +0200 From: Boris Brezillon To: Chia-I Wu Cc: Liviu Dudau , Marcin =?UTF-8?B?xZpsdXNhcno=?= , Ketil Johnsen , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Jonathan Corbet , Shuah Khan , Sumit Semwal , Benjamin Gaignard , Brian Starkey , John Stultz , "T.J. Mercier" , Christian =?UTF-8?B?S8O2bmln?= , Steven Price , Daniel Almeida , Alice Ryhl , Matthias Brugger , AngeloGioacchino Del Regno , dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, Florent Tomasin , nd@arm.com Subject: Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor Message-ID: <20260518091650.5a7a4f4a@fedora> In-Reply-To: References: <20260505140516.1372388-1-ketil.johnsen@arm.com> <20260505140516.1372388-5-ketil.johnsen@arm.com> <20260505181523.49a3d85c@fedora> <20260507135356.5428d50d@fedora> <20260512161111.0cb7000e@fedora> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260518_001659_655067_408E2E91 X-CRM114-Status: GOOD ( 44.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 13 May 2026 12:31:32 -0700 Chia-I Wu wrote: > On Tue, May 12, 2026 at 8:39=E2=80=AFAM Liviu Dudau = wrote: > > > > On Tue, May 12, 2026 at 04:11:11PM +0200, Boris Brezillon wrote: =20 > > > On Tue, 12 May 2026 14:47:27 +0100 > > > Liviu Dudau wrote: > > > =20 > > > > On Thu, May 07, 2026 at 01:53:56PM +0200, Boris Brezillon wrote: =20 > > > > > On Thu, 7 May 2026 11:02:26 +0200 > > > > > Marcin =C5=9Alusarz wrote: > > > > > =20 > > > > > > On Tue, May 05, 2026 at 06:15:23PM +0200, Boris Brezillon wrote= : =20 > > > > > > > > @@ -277,9 +286,21 @@ int panthor_device_init(struct panthor= _device *ptdev) > > > > > > > > return ret; > > > > > > > > } > > > > > > > > > > > > > > > > + /* If a protected heap name is specified but not found,= defer the probe until created */ > > > > > > > > + if (protected_heap_name && strlen(protected_heap_name))= { =20 > > > > > > > > > > > > > > Do we really need this strlen() > 0? Won't dma_heap_find() fa= il is the > > > > > > > name is "" already? =20 > > > > > > > > > > > > If dma_heap_find() will fail, then the whole probe with fail to= o. > > > > > > This check prevents that. =20 > > > > > > > > > > Yeah, that's also a questionable design choice. I mean, we can > > > > > currently probe and boot the FW even though we never setup the > > > > > protected FW sections, so why should we defer the probe here? Can= 't we > > > > > just retry the next time a group with the protected bit is create= d and > > > > > fail if we can find a protected heap? =20 > > > > > > > > The problem we have with the current firmware is that it does a num= ber of setup steps at "boot" > > > > time only. One of the steps is preparing its internal structures fo= r when it enters protected > > > > mode and it stores them in the buffer passed in at firmware loading= . We cannot later run the > > > > process when we have a group with protected mode set. =20 > > > > > > No, but we can force a full/slow reset and have that thing > > > re-initialized, can't we? I mean, that's basically what we do when a > > > fast reset fails: we re-initialize all the sections and reset again, = at > > > which point the FW should start from a fresh state, and be able to > > > properly initialize the protected-related stuff if protected sections > > > are populated. Am I missing something? =20 > > > > Right, we can do that. For some reason I keep associating the reset wit= h the > > error handling and not with "normal" operations. =20 > I kind of hope we end up with either >=20 > - panthor knows the exact heap to use and fails with EPROBE_DEFER if > the heap is missing, or > - panthor gets a dma-buf from userspace and does the full reset > - userspace also needs to provide a dma-buf for each protected > group for the suspend buffer >=20 > than something in-between. The latter is more ad-hoc and basically > kicks the issue to the userspace. Indeed, the second option is more ad-hoc, but when you think about it, userspace has to have this knowledge, because it needs to know the dma-heap to use for buffer allocation that cross a device boundary anyway. Think about frames produced by a video decoder, and composited by the GPU into a protected scanout buffer that's passed to the KMS device. Why would the GPU driver be source of truth when it comes to choosing the heap to use to allocate protected buffers for the video decoder or those used for the display? >=20 > For the former, expressing the relation in DT seems to be the best, > but only if possible :-). Otherwise, a kconfig option (instead of > module param) should be easier to work with. >=20 > Looking at the userspace implementation, can we also have an panthor > ioctl to return the heap to userspace? Yes, it's something we can add, but again, I'm questioning the usefulness of this: how can we ensure the heap used by panthor to allocate its protected FW buffers is suitable for scanout buffers (buffers that can be used by display drivers). There needs to be a glue leaving in usersland and taking the decision, and I'm not too sure trusting any of the component in the chain (vdec, gpu, display) is the right thing to do.