[PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[Benjamin Gaignard]

RK3588 post-processor block is able to convert 10 bits streams
into 8 bits pixels format.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Fixes: 003afda97c65 ("media: verisilicon: Enable AV1 decoder on rk3588")
---
 drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
index f97527670783..964122e7c355 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
@@ -82,7 +82,6 @@ static const struct hantro_fmt rockchip_vpu981_postproc_fmts[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_NV12,
 		.codec_mode = HANTRO_MODE_NONE,
-		.match_depth = true,
 		.postprocessed = true,
 		.frmsize = {
 			.min_width = ROCKCHIP_VPU981_MIN_SIZE,
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] media: verisilicon: Fix auxiliary buffers allocation size

[Benjamin Gaignard]

Use v4l2_av1_tile_info->tile_cols to know the number of colons
in the frame. This made auxiliary buffers meory size computation
more accurate.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
---
 .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index cc4483857489..65e8f2d07400 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -257,7 +257,8 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
 	struct hantro_av1_dec_ctrls *ctrls = &av1_dec->ctrls;
-	unsigned int num_tile_cols = 1 << ctrls->tile_group_entry->tile_col;
+	const struct v4l2_av1_tile_info *tile_info = &ctrls->frame->tile_info;
+	unsigned int num_tile_cols = tile_info->tile_cols;
 	unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 + 1, 64);
 	unsigned int height_in_sb = height / 64;
 	unsigned int stripe_num = ((height + 8) + 63) / 64;
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: Fix auxiliary buffers allocation size

[Nicolas Dufresne]

Hi,

Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
> Use v4l2_av1_tile_info->tile_cols to know the number of colons
> in the frame. This made auxiliary buffers meory size computation
> more accurate.

Seems like this is potentially going to impact some conformance tests. Anything
to report from fluster results ?

Nicolas

> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
> ---
>  .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> index cc4483857489..65e8f2d07400 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> @@ -257,7 +257,8 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
>  	struct hantro_dev *vpu = ctx->dev;
>  	struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
>  	struct hantro_av1_dec_ctrls *ctrls = &av1_dec->ctrls;
> -	unsigned int num_tile_cols = 1 << ctrls->tile_group_entry->tile_col;
> +	const struct v4l2_av1_tile_info *tile_info = &ctrls->frame->tile_info;
> +	unsigned int num_tile_cols = tile_info->tile_cols;
>  	unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 + 1, 64);
>  	unsigned int height_in_sb = height / 64;
>  	unsigned int stripe_num = ((height + 8) + 63) / 64;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: Fix auxiliary buffers allocation size

[Benjamin Gaignard]

Le 04/04/2024 à 20:00, Nicolas Dufresne a écrit :
> Hi,
>
> Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
>> Use v4l2_av1_tile_info->tile_cols to know the number of colons
>> in the frame. This made auxiliary buffers meory size computation
>> more accurate.
> Seems like this is potentially going to impact some conformance tests. Anything
> to report from fluster results ?

Flusters AV1 score is the same.
Maybe we have been lucky when allocating memory until now.
That said the test stream have 8 tile columns which is unusual but admitted by AV1 specifications.

Benjamin

>
> Nicolas
>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
>> ---
>>   .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> index cc4483857489..65e8f2d07400 100644
>> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> @@ -257,7 +257,8 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
>>   	struct hantro_dev *vpu = ctx->dev;
>>   	struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
>>   	struct hantro_av1_dec_ctrls *ctrls = &av1_dec->ctrls;
>> -	unsigned int num_tile_cols = 1 << ctrls->tile_group_entry->tile_col;
>> +	const struct v4l2_av1_tile_info *tile_info = &ctrls->frame->tile_info;
>> +	unsigned int num_tile_cols = tile_info->tile_cols;
>>   	unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 + 1, 64);
>>   	unsigned int height_in_sb = height / 64;
>>   	unsigned int stripe_num = ((height + 8) + 63) / 64;
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: Fix auxiliary buffers allocation size

[Benjamin Gaignard]

Le 05/04/2024 à 10:13, Benjamin Gaignard a écrit :
>
> Le 04/04/2024 à 20:00, Nicolas Dufresne a écrit :
>> Hi,
>>
>> Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
>>> Use v4l2_av1_tile_info->tile_cols to know the number of colons
>>> in the frame. This made auxiliary buffers meory size computation
>>> more accurate.
>> Seems like this is potentially going to impact some conformance 
>> tests. Anything
>> to report from fluster results ?
>
> Flusters AV1 score is the same.
> Maybe we have been lucky when allocating memory until now.
> That said the test stream have 8 tile columns which is unusual but 
> admitted by AV1 specifications.

Gentle ping,
Regards,
Benjamin

>
> Benjamin
>
>>
>> Nicolas
>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>>> Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
>>> ---
>>> .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git 
>>> a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c 
>>> b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>>> index cc4483857489..65e8f2d07400 100644
>>> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>>> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>>> @@ -257,7 +257,8 @@ static int 
>>> rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
>>>       struct hantro_dev *vpu = ctx->dev;
>>>       struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
>>>       struct hantro_av1_dec_ctrls *ctrls = &av1_dec->ctrls;
>>> -    unsigned int num_tile_cols = 1 << 
>>> ctrls->tile_group_entry->tile_col;
>>> +    const struct v4l2_av1_tile_info *tile_info = 
>>> &ctrls->frame->tile_info;
>>> +    unsigned int num_tile_cols = tile_info->tile_cols;
>>>       unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 
>>> + 1, 64);
>>>       unsigned int height_in_sb = height / 64;
>>>       unsigned int stripe_num = ((height + 8) + 63) / 64;
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: Fix auxiliary buffers allocation size

[Nicolas Dufresne]

Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
> Use v4l2_av1_tile_info->tile_cols to know the number of colons
> in the frame. This made auxiliary buffers meory size computation
> more accurate.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> index cc4483857489..65e8f2d07400 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> @@ -257,7 +257,8 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
>  	struct hantro_dev *vpu = ctx->dev;
>  	struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
>  	struct hantro_av1_dec_ctrls *ctrls = &av1_dec->ctrls;
> -	unsigned int num_tile_cols = 1 << ctrls->tile_group_entry->tile_col;
> +	const struct v4l2_av1_tile_info *tile_info = &ctrls->frame->tile_info;
> +	unsigned int num_tile_cols = tile_info->tile_cols;
>  	unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 + 1, 64);
>  	unsigned int height_in_sb = height / 64;
>  	unsigned int stripe_num = ((height + 8) + 63) / 64;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[Nicolas Dufresne]

Hi,

Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
> RK3588 post-processor block is able to convert 10 bits streams
> into 8 bits pixels format.

Does it come with any HDR to SDR capabilities ? cause stripping off 2 bits means
that tone mapping will cause a lot of banding as it won't have the expected
precision. I'm simply trying to make up the big portrait so we don't just offer
yet another foot gun. But perhaps its fine to offer this, its just that we don't
have a mechanism to report which pixel format in the selection will cause data
lost.

Nicolas

> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 003afda97c65 ("media: verisilicon: Enable AV1 decoder on rk3588")
> ---
>  drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> index f97527670783..964122e7c355 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> @@ -82,7 +82,6 @@ static const struct hantro_fmt rockchip_vpu981_postproc_fmts[] = {
>  	{
>  		.fourcc = V4L2_PIX_FMT_NV12,
>  		.codec_mode = HANTRO_MODE_NONE,
> -		.match_depth = true,
>  		.postprocessed = true,
>  		.frmsize = {
>  			.min_width = ROCKCHIP_VPU981_MIN_SIZE,


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[Benjamin Gaignard]

Le 04/04/2024 à 19:59, Nicolas Dufresne a écrit :
> Hi,
>
> Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
>> RK3588 post-processor block is able to convert 10 bits streams
>> into 8 bits pixels format.
> Does it come with any HDR to SDR capabilities ? cause stripping off 2 bits means
> that tone mapping will cause a lot of banding as it won't have the expected
> precision. I'm simply trying to make up the big portrait so we don't just offer
> yet another foot gun. But perhaps its fine to offer this, its just that we don't
> have a mechanism to report which pixel format in the selection will cause data
> lost.

No it just to enable post-processor capabilities like we have already for G2/HEVC.
Since it is a post-processor pixel format it will be enumerated after V4L2_PIX_FMT_NV15_4L4
so it will update to userland to decide to use it or not.

Benjamin

>
> Nicolas
>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> Fixes: 003afda97c65 ("media: verisilicon: Enable AV1 decoder on rk3588")
>> ---
>>   drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>> index f97527670783..964122e7c355 100644
>> --- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>> +++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>> @@ -82,7 +82,6 @@ static const struct hantro_fmt rockchip_vpu981_postproc_fmts[] = {
>>   	{
>>   		.fourcc = V4L2_PIX_FMT_NV12,
>>   		.codec_mode = HANTRO_MODE_NONE,
>> -		.match_depth = true,
>>   		.postprocessed = true,
>>   		.frmsize = {
>>   			.min_width = ROCKCHIP_VPU981_MIN_SIZE,
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[Benjamin Gaignard]

Le 05/04/2024 à 10:17, Benjamin Gaignard a écrit :
>
> Le 04/04/2024 à 19:59, Nicolas Dufresne a écrit :
>> Hi,
>>
>> Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
>>> RK3588 post-processor block is able to convert 10 bits streams
>>> into 8 bits pixels format.
>> Does it come with any HDR to SDR capabilities ? cause stripping off 2 
>> bits means
>> that tone mapping will cause a lot of banding as it won't have the 
>> expected
>> precision. I'm simply trying to make up the big portrait so we don't 
>> just offer
>> yet another foot gun. But perhaps its fine to offer this, its just 
>> that we don't
>> have a mechanism to report which pixel format in the selection will 
>> cause data
>> lost.
>
> No it just to enable post-processor capabilities like we have already 
> for G2/HEVC.
> Since it is a post-processor pixel format it will be enumerated after 
> V4L2_PIX_FMT_NV15_4L4
> so it will update to userland to decide to use it or not.

Gentle ping,
Regards,
Benjamin

>
> Benjamin
>
>>
>> Nicolas
>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>>> Fixes: 003afda97c65 ("media: verisilicon: Enable AV1 decoder on 
>>> rk3588")
>>> ---
>>>   drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c 
>>> b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>>> index f97527670783..964122e7c355 100644
>>> --- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>>> +++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
>>> @@ -82,7 +82,6 @@ static const struct hantro_fmt 
>>> rockchip_vpu981_postproc_fmts[] = {
>>>       {
>>>           .fourcc = V4L2_PIX_FMT_NV12,
>>>           .codec_mode = HANTRO_MODE_NONE,
>>> -        .match_depth = true,
>>>           .postprocessed = true,
>>>           .frmsize = {
>>>               .min_width = ROCKCHIP_VPU981_MIN_SIZE,
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] media: verisilicon: AV1: Be more fexible on postproc capabilities

[Nicolas Dufresne]

Le jeudi 28 mars 2024 à 10:34 +0100, Benjamin Gaignard a écrit :
> RK3588 post-processor block is able to convert 10 bits streams
> into 8 bits pixels format.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 003afda97c65 ("media: verisilicon: Enable AV1 decoder on rk3588")

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> index f97527670783..964122e7c355 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> @@ -82,7 +82,6 @@ static const struct hantro_fmt rockchip_vpu981_postproc_fmts[] = {
>  	{
>  		.fourcc = V4L2_PIX_FMT_NV12,
>  		.codec_mode = HANTRO_MODE_NONE,
> -		.match_depth = true,
>  		.postprocessed = true,
>  		.frmsize = {
>  			.min_width = ROCKCHIP_VPU981_MIN_SIZE,


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel