Re: [PATCH v5 06/29] media: rockchip: rga: fix too small buffer size

["Sven Püschel" <s.pueschel@pengutronix.de>]

Hi Nicolas,

On 5/8/26 11:11 PM, Nicolas Dufresne wrote:
> Le mardi 28 avril 2026 à 11:00 +0200, Sven Püschel a écrit :
>> Fix the command buffer size being only a quarter of the actual size.
>> The RGA_CMDBUF_SIZE macro was potentially intended to specify the length
>> of the cmdbuf u32 array pointer. But as it's used to specify the size of
>> the allocation, which is counted in bytes. Therefore adjust the macro
>> size to bytes as it better matches the variable name and adjust it's
>> users accordingly.
>>
>> As the command buffer is relatively small, it probably didn't caused
>> an issue due to being smaller than a single page.
>>
>> Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
>> Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
> Reviewed-by: Nicolas Dufresne  <nicolas.dufresne@collabora.com>
>
>> ---
>>   drivers/media/platform/rockchip/rga/rga-hw.c | 2 +-
>>   drivers/media/platform/rockchip/rga/rga-hw.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
>> index 43ed742a16492..d1618bb247501 100644
>> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
>> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
>> @@ -414,7 +414,7 @@ static void rga_cmd_set(struct rga_ctx *ctx,
>>   {
>>   	struct rockchip_rga *rga = ctx->rga;
>>   
>> -	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
>> +	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE);
> So we had a buffer overrun ?

 From my point of view, yes [1]:

     /* Create CMD buffer */
     rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
                        &rga->cmdbuf_phy, GFP_KERNEL,
                        DMA_ATTR_WRITE_COMBINE);


Given that 0x20 * 4 is smaller than a page, it probably didn't caused an 
issue as we didn't write out of the one page allocated.

Btw.: I've noticed it while reading a bit over the comments from 
sashiko.dev [2]. I'll also add this link for clarity to the comment in 
my next series (seems to be the common way of referring to sashiko.dev).

Sincerely
     Sven


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/rockchip/rga/rga.c?id=50897c955902c93ae71c38698abb910525ebdc89#n880

[2] 
https://sashiko.dev/#/patchset/20260325-spu-rga3-v4-0-e90ec1c61354%40pengutronix.de?part=10

>
> Nicolas
>
>>   
>>   	rga_cmd_set_src_addr(ctx, src->dma_desc_pa);
>>   	/*
>> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.h b/drivers/media/platform/rockchip/rga/rga-hw.h
>> index cc6bd7f5b0300..2b8537a5fd0d7 100644
>> --- a/drivers/media/platform/rockchip/rga/rga-hw.h
>> +++ b/drivers/media/platform/rockchip/rga/rga-hw.h
>> @@ -6,7 +6,7 @@
>>   #ifndef __RGA_HW_H__
>>   #define __RGA_HW_H__
>>   
>> -#define RGA_CMDBUF_SIZE 0x20
>> +#define RGA_CMDBUF_SIZE 0x80
>>   
>>   /* Hardware limits */
>>   #define MAX_WIDTH 8192