Received-SPF: pass (google.com: domain of linux-kernel+bounces-122759-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Nas Chung <nas.chung@chipsnmedia.com>
To: Ivan Bornyakov <brnkv.i1@gmail.com>
CC: "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, jackson.lee
	<jackson.lee@chipsnmedia.com>, Mauro Carvalho Chehab <mchehab@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
 prop
Thread-Topic: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size"
 DT prop
Thread-Index: AQHafn94hZZm1U4NUE6FWrHOISEj9rFLXGrQgAAqpQCAAWluEA==
Date: Thu, 28 Mar 2024 10:16:53 +0000
Message-ID:
 <SL2P216MB1246499CC9FED9BFB5B11DA3FB3B2@SL2P216MB1246.KORP216.PROD.OUTLOOK.COM>
References: <20240325064102.9278-1-brnkv.i1@gmail.com>
 <20240325064102.9278-5-brnkv.i1@gmail.com>
 <SL2P216MB1246537DD623B813453B28F9FB342@SL2P216MB1246.KORP216.PROD.OUTLOOK.COM>
 <5hd7duzqhgdxpmvom3opkhwxkq55dmitk4gwdl4dy46q662in6@xxkmvdj6plqb>
In-Reply-To: <5hd7duzqhgdxpmvom3opkhwxkq55dmitk4gwdl4dy46q662in6@xxkmvdj6plqb>
Accept-Language: ko-KR, en-US
Content-Language: ko-KR
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?OoamVhWpp2Yvu6WKLYhPXV0OIbPYKa91dg09S86lHBCDwvP/yKJtzaOmjrvW?=
 =?us-ascii?Q?WMstP4bXEVWdid5j965bNgyTNywGJBAK8AMRl+tGHcfxtBHYgOtEE0s3a3zJ?=
 =?us-ascii?Q?lkMET+eex5LVnn3aFis7t9IznP92ypishO9I0/mrQ1Q3Ov7rcWScu4kxxonM?=
 =?us-ascii?Q?Kh/XwJudyDeoueufJ3cYe+m5eovsIWO/1LeCTzS5bRBWHCkVAFiDvHPppOU2?=
 =?us-ascii?Q?bida8WWBqQZA5OO41M6devk4QpRV4bMI5PjgroJZYgbTja5A1x0EWzgeK060?=
 =?us-ascii?Q?+hvLJ4G5hbhESSVMfJJguBoz0BaDTzMR8NnAwOZCFJr1TST6xpaLI3KB/Rw4?=
 =?us-ascii?Q?krBMbUUPL3wSP22zOb0N5G0u8I/K4ZkDih5AV5OoeuCeSy5nbFLS7fZZNrgA?=
 =?us-ascii?Q?Dbfq3TjnrRJYfZY+hZmMA8R7W6wV26qeVY3kt6gbLMZFOMyx1aYzkOFESwr6?=
 =?us-ascii?Q?12gmvmvs9Imol27SpPrdTEEo9z7MLLPFnyB42cg4aXDrThrRpVWr2inZohj4?=
 =?us-ascii?Q?wgcdMc3jWHR8UaNZ7ftpxgrqXfxFqpon9a+DwQmdgOly2NpZlb9vu8FZaQ6V?=
 =?us-ascii?Q?b+DJoBA38IzgXM6n+VsVbJRF2IcYu6+0p1PKyl7hha2F5xjpCpvpPY2FG28f?=
 =?us-ascii?Q?guSmTUb4/GKgpTzlO0nCrWJaRDW6cQ08TTU5nwgmjB8PdM97xw1oRj7vOMQv?=
 =?us-ascii?Q?cjOx+0m7lf19vMVrkWN7+M9+R+hhHFRA3jvUnl/LBrsmuWdFgtXkWxHH5sc2?=
 =?us-ascii?Q?1t5KJxGcp0Wj34qcz5oLsx2OjxbNa5hgnbyJjuKS0YmxTbde+dl9P8Wr7fqz?=
 =?us-ascii?Q?bQc1rZ5RMJpCeUxZJVc6/7hD51/uBIG0LCjMBYUYHtx1wKftkwkTrKs1SDuC?=
 =?us-ascii?Q?AJ4sGOLb2hszKYQAs4bm2geUWiR5AV/mF0O/pPpXViszqnqFyNNj0slTlu42?=
 =?us-ascii?Q?fEDQy35Lh7hR7D2eJ//M/bsKySZm1Hf9foB2aA5p0PqxFlixreGHf9R2D47X?=
 =?us-ascii?Q?AZibZvn5SwbL277vSHY6TvRjEP45nko1oR1venJwy8uqFPRk1GHnjGKVXBVG?=
 =?us-ascii?Q?xyt+tlzaJfSijDZGk2XRA78/wk6blouYVYnCRs3S1hzAsaWoPwkeXYuJ8Q2o?=
 =?us-ascii?Q?oGcJDDQLUeg7+/q3dY3BuMjIc1EqDZmMneB4tnRsVLu8/+79U+irKwqjbE2h?=
 =?us-ascii?Q?XoazQC+JUkHcF03m3KsvTz38ogGL1BXoYP+JCLQllYLT4b8mQLqIpNgASHhK?=
 =?us-ascii?Q?n3lIm4bxT9NIuKRU+/c9ewCvqLCgfZelidu+QBXiB5mD7eRBJ6Ca+WRixRZk?=
 =?us-ascii?Q?mazDjQVvHMjqd797am9FLUPolNq+9a9a+uXzWNC6to8fMq0YMp+GC6F88cGm?=
 =?us-ascii?Q?oxYIgqyuUGVT+Nuirh3fglFNjwpoU5DFp7yQQTWRCZ4KL1qRBQy5sed+e6Nl?=
 =?us-ascii?Q?TROw2zssmk/uvaC5JAP3lGGKn8WrUIbiKYzGFqGrPv+OOMCKRBkcz9CFHJLF?=
 =?us-ascii?Q?uhO0rMbH1UFcWC+3z9nO4Hb32nLYZ8JEs8dvHdjDfBls9yIqTREDx0V+I7tQ?=
 =?us-ascii?Q?OwJlk9YQ5L0fSAyLSdAXqgvAD+1XeWRt1J4xi0Jo?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: chipsnmedia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SL2P216MB1246.KORP216.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-Network-Message-Id: c1f72d6d-239d-480a-43a1-08dc4f10309a
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Mar 2024 10:16:53.2716
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4d70c8e9-142b-4389-b7f2-fa8a3c68c467
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Eu38k9xH+XDb5i183QU0BQ7I+JS4J+3tUxmQaEOOE5iNcbO7hhVHExyOY80/6TMXPG4E5Zs/NmAw5jqtEddOdBHEoCslsbLzg64l89EQEC8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PU4P216MB1915

Hi, Ivan.

>-----Original Message-----
>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>Sent: Wednesday, March 27, 2024 9:27 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>
>Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>Subject: Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-
>size" DT prop
>
>On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
>> Hi, Ivan.
>>
>> >-----Original Message-----
>> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >Sent: Monday, March 25, 2024 3:41 PM
>> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
>> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
><mchehab@kernel.org>;
>> >Philipp Zabel <p.zabel@pengutronix.de>
>> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
>> >linux-kernel@vger.kernel.org
>> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
>> >prop
>> >
>> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
>> >excessive "sram-size" device-tree property as genalloc is already able
>> >to determine available memory.
>> >
>> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >---
>> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
>> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
>> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
>> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
>> > 4 files changed, 13 insertions(+), 18 deletions(-)
>> >
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >index 3809f70bc0b4..a63fffed55e9 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
>> >*vpu_dev, struct vpu_buf *array,
>> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
>> > {
>> > 	struct vpu_buf *vb =3D &vpu_dev->sram_buf;
>> >+	dma_addr_t daddr;
>> >+	void *vaddr;
>> >+	size_t size;
>> >
>> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
>> >+	if (!vpu_dev->sram_pool || vb->vaddr)
>> > 		return;
>> >
>> >-	if (!vb->vaddr) {
>> >-		vb->size =3D vpu_dev->sram_size;
>> >-		vb->vaddr =3D gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
>> >-					       &vb->daddr);
>> >-		if (!vb->vaddr)
>> >-			vb->size =3D 0;
>> >+	size =3D min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
>> >>sram_pool));
>> >+	vaddr =3D gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>> >+	if (vaddr) {
>> >+		vb->vaddr =3D vaddr;
>> >+		vb->daddr =3D daddr;
>> >+		vb->size =3D size;
>> > 	}
>> >
>> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
>> >0x%p\n",
>> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device
>*vpu_dev)
>> > 	if (!vb->size || !vb->vaddr)
>> > 		return;
>> >
>> >-	if (vb->vaddr)
>> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
>> >-			      vb->size);
>> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
>> >>size);
>> >
>> > 	memset(vb, 0, sizeof(*vb));
>> > }
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >index 1e631da58e15..2a972cddf4a6 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
>> >*pdev)
>> > 		goto err_reset_assert;
>> > 	}
>> >
>> >-	ret =3D of_property_read_u32(pdev->dev.of_node, "sram-size",
>> >-				   &dev->sram_size);
>> >-	if (ret) {
>> >-		dev_warn(&pdev->dev, "sram-size not found\n");
>> >-		dev->sram_size =3D 0;
>> >-	}
>> >-
>> > 	dev->sram_pool =3D of_gen_pool_get(pdev->dev.of_node, "sram", 0);
>> > 	if (!dev->sram_pool)
>> > 		dev_warn(&pdev->dev, "sram node not found\n");
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >index da530fd98964..975d96b22191 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >@@ -750,7 +750,6 @@ struct vpu_device {
>> > 	struct vpu_attr attr;
>> > 	struct vpu_buf common_mem;
>> > 	u32 last_performance_cycles;
>> >-	u32 sram_size;
>> > 	struct gen_pool *sram_pool;
>> > 	struct vpu_buf sram_buf;
>> > 	void __iomem *vdb_register;
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>vpuconfig.h
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >index d9751eedb0f9..9d99afb78c89 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >@@ -28,6 +28,8 @@
>> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K,
>AVC
>> >40K
>> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>> >
>> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>>
>> WAVE521 can support 8K stream decoding/encoding.
>> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
>>
>> And, Current driver always enable sec_axi_info option if sram buffer is
>allocated.
>> But, we have to enable/disable the sec_axi_info option after checking
>the allocated sram size is enough to decode/encode current bitstream
>resolution.
>
>Do we really? As an experiment I tried to provide to Wave515 1KB of SRAM
>memory and decoded 4k sample file was fine...
>

You can think It seems like driver works fine.
But, This is not the behavior we expect.
There is a possibility that unexpected problems may occur.

>> Wave5 can enable/disable the sec_axi_info option for each instance.
>>
>> How about handle sram-size through match_data ?
>> I can find some drivers which use match_data to configure the sram size.
>>
>> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
>device.
>> - .sram_size =3D (64 * 1024);
>> Driver just allocate the sram-size for max supported resolution of each
>device, and we don't need to check the sram-size is enough or not.
>>
>> Thanks.
>> Nas.
>>
>> >+
>> > #define MAX_NUM_INSTANCE                32
>> >
>> > #define W5_MIN_ENC_PIC_WIDTH            256
>> >--
>> >2.44.0
>>