2024-03-28 09:35:43

by Benjamin Gaignard

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

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

Signed-off-by: Benjamin Gaignard <[email protected]>
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



2024-03-28 09:35:53

by Benjamin Gaignard

[permalink] [raw]
Subject: [PATCH] media: verisilicon: Fix auxiliary buffers allocation size

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 <[email protected]>
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


2024-04-04 18:01:26

by Nicolas Dufresne

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

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 <[email protected]>
> 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;


2024-04-04 19:08:40

by Nicolas Dufresne

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

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 <[email protected]>
> 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,


2024-04-05 08:13:46

by Benjamin Gaignard

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


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 <[email protected]>
>> 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;
>

2024-04-05 08:18:03

by Benjamin Gaignard

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


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 <[email protected]>
>> 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,
>