| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 4 Apr 2024 10:02:35 +0200
From: "sebastian.fricke@...labora.com" <sebastian.fricke@...labora.com>
To: Nas Chung <nas.chung@...psnmedia.com>
Cc: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jackson.lee" <jackson.lee@...psnmedia.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>
Subject: Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
prop
Hey Nas,
On 01.04.2024 09:28, Nas Chung wrote:
>Hi, Sebastian.
>
>>-----Original Message-----
>>From: Ivan Bornyakov <brnkv.i1@...il.com>
>>Sent: Thursday, March 28, 2024 8:36 PM
>>To: Nas Chung <nas.chung@...psnmedia.com>
>>Cc: linux-media@...r.kernel.org; linux-kernel@...r.kernel.org;
>>jackson.lee <jackson.lee@...psnmedia.com>; Mauro Carvalho Chehab
>><mchehab@...nel.org>; Philipp Zabel <p.zabel@...gutronix.de>
>>Subject: Re: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop
>>"sram-size" DT prop
>>
>>On Thu, Mar 28, 2024 at 10:16:53AM +0000, Nas Chung wrote:
>>> Hi, Ivan.
>>>
>>> >-----Original Message-----
>>> >From: Ivan Bornyakov <brnkv.i1@...il.com>
>>> >Sent: Wednesday, March 27, 2024 9:27 PM
>>> >To: Nas Chung <nas.chung@...psnmedia.com>
>>> >Cc: linux-media@...r.kernel.org; linux-kernel@...r.kernel.org;
>>> >jackson.lee <jackson.lee@...psnmedia.com>; Mauro Carvalho Chehab
>>> ><mchehab@...nel.org>; Philipp Zabel <p.zabel@...gutronix.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@...il.com>
>>> >> >Sent: Monday, March 25, 2024 3:41 PM
>>> >> >To: Nas Chung <nas.chung@...psnmedia.com>; jackson.lee
>>> >> ><jackson.lee@...psnmedia.com>; Mauro Carvalho Chehab
>>> ><mchehab@...nel.org>;
>>> >> >Philipp Zabel <p.zabel@...gutronix.de>
>>> >> >Cc: Ivan Bornyakov <brnkv.i1@...il.com>; linux-
>>media@...r.kernel.org;
>>> >> >linux-kernel@...r.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@...il.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 = &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 = vpu_dev->sram_size;
>>> >> >- vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb-
>>>size,
>>> >> >- &vb->daddr);
>>> >> >- if (!vb->vaddr)
>>> >> >- vb->size = 0;
>>> >> >+ size = min_t(size_t, WAVE5_MAX_SRAM_SIZE,
>>gen_pool_avail(vpu_dev-
>>> >> >>sram_pool));
>>> >> >+ vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>>> >> >+ if (vaddr) {
>>> >> >+ vb->vaddr = vaddr;
>>> >> >+ vb->daddr = daddr;
>>> >> >+ vb->size = 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 = 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 = 0;
>>> >> >- }
>>> >> >-
>>> >> > dev->sram_pool = 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.
>>>
>>
>>Ok, then we either
>>
>> 1) don't try to allocate any availible SRAM memory up to
>> match_data->sram_size, but allocate exact match_data->sram_size
>>
>>or
>>
>> 2) allocate any available SRAM memory up to match_data->sram_size, but
>> check for allocated size before writing to registers W5_USE_SEC_AXI
>> and W5_CMD_ENC_PIC_USE_SEC_AXI
>>
>>With second variant I won't be able to add said check for Wave521, as I
>>don't know its memory requirements.
>>
>>Also would this check be SoC specific or would it be common for any SoC
>>with same Wave5xx IP?
>>
>>> >> 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.
>
>I proposed using match_data to allocate different sram size for each device.
>Do you think this is a reasonable approach ?
As discussed here:
https://patchwork.linuxtv.org/project/linux-media/patch/20240201184238.2542695-1-b-brnich@ti.com/
To quote Brandon Brnich from TI:
If static SRAM allocation is the correct method to go, then this series
can be ignored and I will add section in device tree and remove check
for parameter in driver as that will now be a bug.
I would like to adhere to that.
>
>Thanks.
>Nas.
Greetings,
Sebastian
>
>>> >>
>>> >> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
>>> >device.
>>> >> - .sram_size = (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
>>> >>
Powered by blists - more mailing lists