Received-SPF: pass (google.com: domain of linux-kernel+bounces-140057-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: jackson.lee <jackson.lee@chipsnmedia.com>
To: Ivan Bornyakov <brnkv.i1@gmail.com>
CC: Nas Chung <nas.chung@chipsnmedia.com>, Mauro Carvalho Chehab
	<mchehab@kernel.org>, Philipp Zabel <p.zabel@pengutronix.de>, Sebastian
 Fricke <sebastian.fricke@collabora.com>, "linux-media@vger.kernel.org"
	<linux-media@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: RE: [PATCH v3 4/5] media: chips-media: wave5: drop "sram-size" DT
 prop
Thread-Topic: RE: [PATCH v3 4/5] media: chips-media: wave5: drop "sram-size"
 DT prop
Thread-Index: AQHah3giAH17WLkhgEm3NaPxm3r//7FfYuJQgAA5ZYCAAyPFEA==
Date: Thu, 11 Apr 2024 08:11:36 +0000
Message-ID:
 <SE1P216MB1303CB49E8C7C83904EA337CED052@SE1P216MB1303.KORP216.PROD.OUTLOOK.COM>
References: <20240405164112.24571-1-brnkv.i1@gmail.com>
 <20240405164112.24571-5-brnkv.i1@gmail.com>
 <SE1P216MB130327BE82553DA3F34BBF17ED072@SE1P216MB1303.KORP216.PROD.OUTLOOK.COM>
 <zrwlu2e2slf7g4izorqbzwahysz6a3diux6zjyfmgtkofzdpuj@cvfl4kqhzvef>
In-Reply-To: <zrwlu2e2slf7g4izorqbzwahysz6a3diux6zjyfmgtkofzdpuj@cvfl4kqhzvef>
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?T1hnOynlRth13UfpocrQIxOnyE7qMTB+YLhnbrUslAT2SNERJJ2IgkgFWtgv?=
 =?us-ascii?Q?5GtDozwXi7hmt3PscAcisIBTqJP9B1BeSEqEhl8B5v2Z19FY9pROzaKR7TH7?=
 =?us-ascii?Q?uEh+ETQ439l991Z7rST/mzkxaqvG/EXIfAdxP+D6zztC78NZrW7BC59Vb0Ez?=
 =?us-ascii?Q?YQipqmoeVcINvjjQbomP0PCI4uznxoAuN5ZOtaMb1rEv9+Czgia3nn6o078u?=
 =?us-ascii?Q?FahyCbnL4G9HLFAEPfR7euygGVuwYf696pxp5Mmwf6o2siPWBpIf1rsqz15O?=
 =?us-ascii?Q?121t/SaxEYLVnq4iYhkx09Qeqt+t75Xw/Ahbqhi5W3WS9hxdkq4bDNFg/xY2?=
 =?us-ascii?Q?uiTxArasU9EBntmc979XVln7NXu53n2LRXVq+2dq31WLJJ3CN3AMYuuq17J9?=
 =?us-ascii?Q?/1e5yVHO4VwrR0yda7LZHf9YnvbLVzGGACfV9wwijf5tsFaNHIIt5mP/4XDl?=
 =?us-ascii?Q?wa/0+sEvMrYGI2fSthDJ0ietEJ3m0ESWyBbrmSTpyD+DX0FC9YtlVSpdnE0+?=
 =?us-ascii?Q?htk5rurKNVcQmn5/LPeepr4MUcDBlu11grTiGFrAtFCz9A/+Y1DU5uovZT7t?=
 =?us-ascii?Q?B+4inrHV0h8H/0JrKMD9QuoF3DbqWR9uKS1SkR16Dw5djW/QSCcnFTRrjXOq?=
 =?us-ascii?Q?O5g3Ss01mBq4mlYz9fmSrIYfWsGDJqAt0Eu54Ue/h0Y3Fqb7JFD1eZNAXk1b?=
 =?us-ascii?Q?EfDX6HPRQtQnTMdQUzelZi8pfT4Xv5FmEtc6CKpqqsTTNmmhqO+7voWz92cl?=
 =?us-ascii?Q?G65PP1s/A3iNo0wkXjFAR9aj05erp6Qfhy+UTcs2xEG3EP5AYtvSL94OWa5R?=
 =?us-ascii?Q?eI9vRvsXD4u+CmKnLC0fNQe1v4UoHuyE+FopezE5CdDHWUBKKaPDhgjHIA+h?=
 =?us-ascii?Q?BwCYGou7OshauU80lnzlwq17AgJjTmFEdkn+0MN00iXjr1KhpzbG0bOZkQ3A?=
 =?us-ascii?Q?3i9QIBfOclLUaAwGOA53UPhxLnxaIH4WiHBN4zftqefAuACK0IYEqEDOSuxR?=
 =?us-ascii?Q?8x41krvNz992VgHbT4zuFaYInzhsIYcbfIK8/NeF/MUhoyLib989B7O5vTlM?=
 =?us-ascii?Q?oyONR3FkO9oRVVY9tizCaK7EwZf3k5hKd5SGByjSUNa99aGQJgEmYInklpiJ?=
 =?us-ascii?Q?CCpxNk0Fxez0aLGUqyrlsn+Ljgk4S3p3DceaZw8i6ASNHofDrL0kwHuyfZln?=
 =?us-ascii?Q?hrWbVQLzVdQ2Feh/goCQTKw0qlSePNIvYu1AbMCbbqBzwP9T8hD9L1VdSwL5?=
 =?us-ascii?Q?g0ondlpYAqglagj3qG9JAKsQ6Y4xDPW3vXqLCdq8/I5QN8m1jSXBClGMFAEC?=
 =?us-ascii?Q?RCn01+Ry9NfaxJPEuvTTKO2PgvK+qM4BHdlIiotUWFJbnxmuDwCcR9U/wXK7?=
 =?us-ascii?Q?W7Q/TI5zeI+ThiG2qrH7le+yRYcnXWak52FWBQnUd5OfDDgY6Zpn/EcjQAX8?=
 =?us-ascii?Q?dGYYWUF63XQ4GyAnxAvd2GKdZnLIWjQSw0mCp2MIIM1/3yRNlyQIyzKJeEuX?=
 =?us-ascii?Q?ihRomLttnyGjVtimpY5RwXb8bPVpjR6OjPexVYhm2toeQWVw3IZ5kKhxs07B?=
 =?us-ascii?Q?MQin3iU6LtZVkmuHNpTX2RR4XY7pXqWKT4jp9Q1kHFdUh/mbz9VwILeAFPZ5?=
 =?us-ascii?Q?1A=3D=3D?=
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: SE1P216MB1303.KORP216.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-Network-Message-Id: 64dd73ab-c133-4a3d-9691-08dc59ff0244
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Apr 2024 08:11:36.8923
 (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: NdG1PayBCNQAUijLfwhTNsAHs3W2jkv1cGBtehLRWzf4ZZlZxkDZSuawkS58BjK5lQJ2Zx69LZglJlIwwOknmiM0gtV8i1n8VyWa8gq0Bqg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PU4P216MB2306

Hi Ivan

> -----Original Message-----
> From: Ivan Bornyakov <brnkv.i1@gmail.com>
> Sent: Tuesday, April 9, 2024 5:12 PM
> To: jackson.lee <jackson.lee@chipsnmedia.com>
> Cc: Nas Chung <nas.chung@chipsnmedia.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>; Sebastian
> Fricke <sebastian.fricke@collabora.com>; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: RE: [PATCH v3 4/5] media: chips-media: wave5: drop "sram-
> size" DT prop
>=20
> Hi, Jackson
>=20
> On Tue, Apr 09, 2024 at 04:50:15AM +0000, jackson.lee wrote:
> > Hey Ivan
> >
> > > -----Original Message-----
> > > From: Ivan Bornyakov <brnkv.i1@gmail.com>
> > > Sent: Saturday, April 6, 2024 1:41 AM
> > > 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>;
> > > Sebastian Fricke <sebastian.fricke@collabora.com>
> > > Cc: Ivan Bornyakov <brnkv.i1@gmail.com>;
> > > linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: [PATCH v3 4/5] media: chips-media: wave5: drop "sram-size"
> > > DT prop
> > >
> > > Move excessive "sram-size" device-tree property to device match data.
> > > Also change SRAM memory allocation strategy: instead of allocation
> > > exact sram_size bytes, allocate all available SRAM memory up to
> sram_size.
> > > Add placeholders wave5_vpu_dec_validate_sec_axi() and
> > > wave5_vpu_enc_validate_sec_axi() for validation that allocated SRAM
> > > memory is enough to decode/encode bitstream of given resolution.
> > >
> > > Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
> > > ---
> > >  .../platform/chips-media/wave5/wave5-hw.c     | 62 +++++++++++++++++=
--
> > >  .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++---
> > >  .../platform/chips-media/wave5/wave5-vpu.c    | 11 ++--
> > >  3 files changed, 72 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > index cdd0a0948a94..36f2fc818013 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > @@ -843,6 +843,36 @@ int wave5_vpu_dec_register_framebuffer(struct
> > > vpu_instance *inst, struct frame_b
> > >  	return ret;
> > >  }
> > >
> > > +static u32 wave5_vpu_dec_validate_sec_axi(struct vpu_instance *inst)
> {
> > > +	struct dec_info *p_dec_info =3D &inst->codec_info->dec_info;
> > > +	u32 bit_size =3D 0, ip_size =3D 0, lf_size =3D 0, ret =3D 0;
> >
> > The bit_size, ip_size and 1f_size is always 0? If so, why are you using
> them ?
> >
>=20
> Since I don't have documentation on Wave521, this is a placeholder for
> someone who have documentation to write proper SRAM size validation, henc=
e
> TODO comment.
>=20
> In the next patch "media: chips-media: wave5: support Wave515 decoder"
> I added validation of SRAM usage for Wave515, for which I do have
> documentation.
>=20
> >
> > > +	u32 sram_size =3D inst->dev->sram_size;
> > > +
> > > +	if (!sram_size)
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * TODO: calculate bit_size, ip_size, lf_size from inst-
> > > >src_fmt.width
> > > +	 * and inst->codec_info->dec_info.initial_info.luma_bitdepth
> > > +	 */
> > > +
> > > +	if (p_dec_info->sec_axi_info.use_bit_enable && sram_size >=3D
> > > bit_size) {
> > > +		ret |=3D BIT(0);
> > > +		sram_size -=3D bit_size;
> > > +	}
> > > +
> > > +	if (p_dec_info->sec_axi_info.use_ip_enable && sram_size >=3D
> > > +ip_size)
> > > {
> > > +		ret |=3D BIT(9);
> > > +		sram_size -=3D ip_size;
> > > +	}
> > > +
> > > +	if (p_dec_info->sec_axi_info.use_lf_row_enable && sram_size >=3D
> > > lf_size)
> > > +		ret |=3D BIT(15);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  int wave5_vpu_decode(struct vpu_instance *inst, u32 *fail_res)  {
> > >  	u32 reg_val;
> > > @@ -855,9 +885,7 @@ int wave5_vpu_decode(struct vpu_instance *inst,
> > > u32
> > > *fail_res)
> > >  	vpu_write_reg(inst->dev, W5_BS_OPTION,
> > > get_bitstream_options(p_dec_info));
> > >
> > >  	/* secondary AXI */
> > > -	reg_val =3D p_dec_info->sec_axi_info.use_bit_enable |
> > > -		(p_dec_info->sec_axi_info.use_ip_enable << 9) |
> > > -		(p_dec_info->sec_axi_info.use_lf_row_enable << 15);
> > > +	reg_val =3D wave5_vpu_dec_validate_sec_axi(inst);
> > >  	vpu_write_reg(inst->dev, W5_USE_SEC_AXI, reg_val);
> > >
> > >  	/* set attributes of user buffer */ @@ -1938,6 +1966,31 @@ int
> > > wave5_vpu_enc_register_framebuffer(struct
> > > device *dev, struct vpu_instance *
> > >  	return ret;
> > >  }
> > >
> > > +static u32 wave5_vpu_enc_validate_sec_axi(struct vpu_instance *inst)
> {
> > > +	struct enc_info *p_enc_info =3D &inst->codec_info->enc_info;
> > > +	u32 rdo_size =3D 0, lf_size =3D 0, ret =3D 0;
> >
> > The rdo_size and 1f_size is always 0? If so, why are you using them ?
> >
>=20
> Same as above. It is a placeholder for someone else to implement these.
>=20
> > > +	u32 sram_size =3D inst->dev->sram_size;
> > > +
> > > +	if (!sram_size)
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * TODO: calculate rdo_size and lf_size from inst->src_fmt.width
> > > and
> > > +	 * inst->codec_info-
> > > >enc_info.open_param.wave_param.internal_bit_depth
> > > +	 */
> > > +
> > > +	if (p_enc_info->sec_axi_info.use_enc_rdo_enable && sram_size >=3D
> > > rdo_size) {
> > > +		ret |=3D BIT(11);
> > > +		sram_size -=3D rdo_size;
> > > +	}
> > > +
> > > +	if (p_enc_info->sec_axi_info.use_enc_lf_enable && sram_size >=3D
> > > lf_size)
> > > +		ret |=3D BIT(15);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  int wave5_vpu_encode(struct vpu_instance *inst, struct enc_param
> > > *option,
> > > u32 *fail_res)  {
> > >  	u32 src_frame_format;
> > > @@ -1959,8 +2012,7 @@ int wave5_vpu_encode(struct vpu_instance
> > > *inst, struct enc_param *option, u32 *f
> > >
> > >  	vpu_write_reg(inst->dev, W5_CMD_ENC_PIC_SRC_AXI_SEL,
> > > DEFAULT_SRC_AXI);
> > >  	/* secondary AXI */
> > > -	reg_val =3D (p_enc_info->sec_axi_info.use_enc_rdo_enable << 11) |
> > > -		(p_enc_info->sec_axi_info.use_enc_lf_enable << 15);
> > > +	reg_val =3D wave5_vpu_enc_validate_sec_axi(inst);
> > >  	vpu_write_reg(inst->dev, W5_CMD_ENC_PIC_USE_SEC_AXI, reg_val);
> > >
> > >  	vpu_write_reg(inst->dev, W5_CMD_ENC_PIC_REPORT_PARAM, 0); diff --
> > > git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> > > index 3809f70bc0b4..556de2f043fe 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >
> >
> > The below code is not based on the current upstream code. Where did you
> get the original code ?
> >
>=20
> What do you mean? This patch series is based on the latest linux-next.


I was confused, please ignore the above comment.

>=20
> > > @@ -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, vpu_dev->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..9e93969ab6db 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > @@ -25,6 +25,7 @@
> > >  struct wave5_match_data {
> > >  	int flags;
> > >  	const char *fw_name;
> > > +	u32 sram_size;
> > >  };
> > >
> > >  int wave5_vpu_wait_interrupt(struct vpu_instance *inst, unsigned
> > > int
> > > timeout) @@ -177,17 +178,12 @@ 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");
> > >
> > > +	dev->sram_size =3D match_data->sram_size;
> > > +
> > >  	dev->product_code =3D wave5_vdi_read_register(dev,
> > > VPU_PRODUCT_CODE_REGISTER);
> > >  	ret =3D wave5_vdi_init(&pdev->dev);
> > >  	if (ret < 0) {
> > > @@ -281,6 +277,7 @@ static void wave5_vpu_remove(struct
> > > platform_device
> > > *pdev)  static const struct wave5_match_data ti_wave521c_data =3D {
> > >  	.flags =3D WAVE5_IS_ENC | WAVE5_IS_DEC,
> > >  	.fw_name =3D "cnm/wave521c_k3_codec_fw.bin",
> > > +	.sram_size =3D (64 * 1024),
> > >  };
> > >
> > >  static const struct of_device_id wave5_dt_ids[] =3D {
> > > --
> > > 2.44.0
> >