Received-SPF: pass (google.com: domain of linux-kernel+bounces-59750-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <fccdc181727307f52a36f3bb621d6a4e192096da.camel@ndufresne.ca>
Subject: Re: [PATCH v2] dt-bindings: media: Add sram-size Property for Wave5
From: Nicolas Dufresne <nicolas@ndufresne.ca>
To: Devarsh Thakkar <devarsht@ti.com>, Brandon Brnich <b-brnich@ti.com>, 
	Nishanth Menon
	 <nm@ti.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Nas Chung
 <nas.chung@chipsnmedia.com>, Jackson Lee <jackson.lee@chipsnmedia.com>, 
 Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring
 <robh+dt@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
 linux-media@vger.kernel.org, devicetree@vger.kernel.org, 
 linux-kernel@vger.kernel.org, Vignesh Raghavendra <vigneshr@ti.com>, Darren
 Etheridge <detheridge@ti.com>
Date: Fri, 09 Feb 2024 12:42:40 -0500
In-Reply-To: <ab029558-fc04-854e-1f97-785f5cec0681@ti.com>
References: <20240201184238.2542695-1-b-brnich@ti.com>
	 <1209b7cf-5be2-4107-aa6b-d67a32ea3737@linaro.org>
	 <20240202125257.p4astjuxpzr5ltjs@dragster>
	 <8091a8cf-c1c0-49b0-b136-1ad0d185aa6a@linaro.org>
	 <20240202155813.szxvi7bfp5xh7rvw@babble>
	 <adfef53c-d64e-4855-ab61-101b6fa419e5@linaro.org>
	 <20240205141255.z5kybm42qld44tdz@portfolio>
	 <20240205192003.3qns6cxqurqnnj7c@udba0500997>
	 <ab029558-fc04-854e-1f97-785f5cec0681@ti.com>
Autocrypt: addr=nicolas@ndufresne.ca; prefer-encrypt=mutual; keydata=mQGiBEUQN0MRBACQYceNSezSdMjx7sx6gwKkMghrrODgl3B0eXBTgNp6c431IfOOEsdvkoOh1kwoYcQgbg4MXw6beOltysX4e8fFWsiRkc2nvvRW9ir9kHDm49MkBLqaDjTqOkYKNMiurFW+gozpr/lUW15QqT6v68RYe0zRdtwGZqeLzX2LVuukGwCg4AISzswrrYHNV7vQLcbaUhPgIl0D+gILYT9TJgAEK4YHW+bFRcY+cgUFoLQqQayECMlctKoLOE69nIYOc/hDr9uih1wxrQ/yL0NJvQCohSPyoyLF9b2EuIGhQVp05XP7FzlTxhYvGO/DtO08ec85+bTfVBMV6eeY4MS3ZU+1z7ObD7Pf29YjyTehN2Dan6w1g2rBk5MoA/9nDocSlk4pbFpsYSFmVHsDiAOFje3+iY4ftVDKunKYWMhwRVBjAREOByBagmRau0cLEcElpf4hX5f978GoxSGIsiKoDAlXX+ICDOWC1/EXhEEmBR1gL0QJgiVviNyLfGJlZWnPjw6xhhmtHYWTDxBOP5peztyc2PqeKsLsLWzAr7RDTmljb2xhcyBEdWZyZXNuZSAoQi4gU2MuIEluZm9ybWF0aXF1ZSkgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPohgBBMRAgAgBQJFlCyOAhsDBgsJCAcDAgQVAggDBBYCAwECHgECF4AACgkQcVMCLawGqBwhLQCgzYlrLBj6KIAZ4gmsfjXD6ZtddT8AoIeGDicVq5WvMHNWign6ApQcZUihtElOaWNvbGFzIER1ZnJlc25lIChCLiBTYy4gSW5mb3JtYXRpcXVlKSA8bmljb2xhcy5kdWZyZXNuZUBjb2xsYWJvcmEuY28udWs+iGIEExECACIFAkuzca8CGwMGCwkIBwMCBhUIAgkKCwQWA
 gMBAh4BAheAAAoJEHFTAi2sBqgcQX8An2By6LDEeMxi4B9hUbpvRnzaaeNqA J9Rox8rfqHZnSErw9bCHiBwvwJZ77QxTmljb2xhcyBEdWZyZXNuZSA8bmljb2xhcy5kdWZyZXNuZUBjb2xsYWJvcmEuY29tPohiBBMRAgAiBQJNzZzPAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRBxUwItrAaoHLlxAKCYAGf4JL7DYDLs/188CPMGuwLypwCfWKc9DorA9f5pyYlD5pQo6SgSoiC0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPohiBBMRAgAiBQJVwNwgAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRBxUwItrAaoHCZ4AJ0QwU6/G4c7h9CkMBT9ZxGLX4KSnQCgq0P7CX7hv/M7HeyfMFZe8t3vAEW0RE5pY29sYXMgRHVmcmVzbmUgKEIuIFNjLiBJbmZvcm1hdGlxdWUpIDxuaWNvbGFzZEBibHVlc3RyZWFrdGVjaC5jb20+iGAEExECACAFAkZjGzoCGwMGCwkIBwMCBBUCCAMEFgIDAQIeAQIXgAAKCRBxUwItrAaoHBl7AJ0d2lrzshMmJaik/EaDEakzEwqgxQCg0JVZMZm9gRfEou1FvinuZxwf/mu0R05pY29sYXMgRHVmcmVzbmUgKEIgU2MuIEluZm9ybWF0aXF1ZSkgPG5pY29sYXMuZHVmcmVzbmVAdXNoZXJicm9va2UuY2E+iGAEExECACAFAkUQN0MCGwMGCwkIBwMCBBUCCAMEFgIDAQIeAQIXgAAKCRBxUwItrAaoHPTnAJ0WGgJJVspoctAvEcI00mtp5WAFGgCgr+E7ItOqZEHAs+xabBgknYZIFPW5Ag0ERRA3UhAIAJ0rxl2HsVg/nSOAUt7U/T/W+RKzVAlD9orCB0pRVvyWNxSr8MHcH
 mWCxykLuB34ouM4GuDVRKfGnqLzJRBfjs7Ax9K2FI3Odund9xpviLCt1jFC0K XL04RebrFT7xjDfocDaSLFvgxMVs/Jr2/ckKPId1oKvgYgt/o+MzUabKyFB8wIvq4GMtj3LoBKLCie2nCaSt7uVUt6q2t5bNWrd3lO6/mWn7YMc5Hsn33H9pS0+9szw6m3dG08eMKNueDlt72QxiYl2rhjzkT4ltKEkFgYBdyrtIj1UO6eX+YXb4E1rCMJrdjBSgqDPK1sWHC7gliy+izr+XTHuFwlfy8gBpsAAwUIAJJNus64gri4HAL632eqVpza83EphX1IuHzLi1LlMnQ9Tm7XKag46NhmJbOByMG33LwBsBdLjjHQSVkYZFWUifq+NWSFC/kqlb72vW8rBAv64+i3QdfxK9FWbweiRsPpvuHjJQuecbPDJpubLaxKbu2aqLCN5LuHXvdQr6KiXwabT+OJ9AJAqHG7q4IEzg4RNUVn9AS6L8bxqMSocjqpWNBCY2efCVd/c6k4Acv6jXu+wDAZEbWXK+71uaUHExhigBYBpiHGrobe32YlTVE/XEIzKKywhm/Hkn5YKWzumLte6xiD9JhKabmD7uqIvLt2twUpz4BdPzj0dvGlSmvFcaaISQQYEQIACQUCRRA3UgIbDAAKCRBxUwItrAaoHJLyAKDeS3AFowM3f1Y3OFU6XRCTKK2ZhwCfT/7P9WDjkkmiq5AfeOiwVlpuHtM=
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

Le jeudi 08 f=C3=A9vrier 2024 =C3=A0 11:52 +0530, Devarsh Thakkar a =C3=A9c=
rit=C2=A0:
> I think even with the approach selected in [1] i.e. referring the
> mmio-sram node using DT property, you can still use dynamic SRAM
> allocation.
> The driver can still allocate from global sram pool dynamically using
> of_gen_pool API as being explained here [3] i.e allocate when first
> instance is opened and free up later when no instances are running.
>=20
> But I agree with Nishanth's point too that we may not want to give all
> of SRAM to VPU. For e.g. on AM62A we have 64KiB SRAM and a 1080p
> use-case requires 48KiB and even higher for 4K so if there is another
> peripheral who is referring this sram node, then it may not get enough
> as VPU will hog the major chunk (or all) of it while it is running and
> this is where an optional property like sram-size will help to cap the
> max sram usage for VPU and so this helps especially on platforms with
> limited SRAM availability.
>=20
> As I understand, the sram size allocation is dependent on resolution and
> once programmed can't be changed until all instances of VPU are done,
> and we can't predict how many instances user will launch and with what
> resolutions.
>=20
> So here's the flow we had thought of some time back :
> 1) Define worst case sram size (per 4K use-case as I believe that's the
> max for CnM wave521c) as a macro in driver
>=20
> Then the condition for determining sram size to be allocated should be
> as below=C2=A0 :
>=20
> 2) When first instance of VPU is opened, allocate as per sram-size if
> sram-size property is specified.
>=20
> 3) If sram-size is not specified then :
> =C2=A0=C2=A0 -> Allocate as per worst case size macro defined in driver f=
rom sram
> pool,
> =C2=A0=C2=A0 -> If worst case size of SRAM > max SRAM size, then allocate
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 max SRAM size
>=20
> 4). When all of the instances of VPU are closed, then free up all
> allocated SRAM.
>=20
> [3] :
> https://wiki.analog.com/resources/tools-software/linuxdsp/docs/linux-kern=
el-and-drivers/sram

Only issue here is that DT is not a use case configuration file. That DT
parameter is meant for HW that simply cannot be operated without it. This i=
s
also edgy, because it also means that it should only be used if that inform=
ation
is not static and vary unpredictably per SoC, which seems generally unlikel=
y.=C2=A0

The Wave5 IP *can* work without it, so it should resort to something more
dynamic. User configuration should be sorted out at the OS level.

Nicolas