* [PATCH] media: verisilicon: AV1: Fix enable cdef computation
@ 2025-12-08 9:52 Benjamin Gaignard
2025-12-08 9:52 ` [PATCH] media: verisilicon: AV1: Fix tx mode bit setting Benjamin Gaignard
2025-12-08 19:45 ` [PATCH] media: verisilicon: AV1: Fix enable cdef computation Nicolas Dufresne
0 siblings, 2 replies; 4+ messages in thread
From: Benjamin Gaignard @ 2025-12-08 9:52 UTC (permalink / raw)
To: nicolas.dufresne, p.zabel, mchehab, heiko, hverkuil
Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
kernel, Benjamin Gaignard
Testing V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF flag isn't enough
to know if cdef bit has to be set.
If any of the used cdef fields isn't zero then we must enable
cdef feature on the hardware.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Fixes: 727a400686a2c ("media: verisilicon: Add Rockchip AV1 decoder")
---
.../platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
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 e4703bb6be7c..f4f7cb45b1f1 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -1396,8 +1396,16 @@ static void rockchip_vpu981_av1_dec_set_cdef(struct hantro_ctx *ctx)
u16 luma_sec_strength = 0;
u32 chroma_pri_strength = 0;
u16 chroma_sec_strength = 0;
+ bool enable_cdef;
int i;
+ enable_cdef = !(cdef->bits == 0 &&
+ cdef->damping_minus_3 == 0 &&
+ cdef->y_pri_strength[0] == 0 &&
+ cdef->y_sec_strength[0] == 0 &&
+ cdef->uv_pri_strength[0] == 0 &&
+ cdef->uv_sec_strength[0] == 0);
+ hantro_reg_write(vpu, &av1_enable_cdef, enable_cdef);
hantro_reg_write(vpu, &av1_cdef_bits, cdef->bits);
hantro_reg_write(vpu, &av1_cdef_damping, cdef->damping_minus_3);
@@ -1953,8 +1961,6 @@ static void rockchip_vpu981_av1_dec_set_parameters(struct hantro_ctx *ctx)
!!(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_SHOW_FRAME));
hantro_reg_write(vpu, &av1_switchable_motion_mode,
!!(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_IS_MOTION_MODE_SWITCHABLE));
- hantro_reg_write(vpu, &av1_enable_cdef,
- !!(ctrls->sequence->flags & V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF));
hantro_reg_write(vpu, &av1_allow_masked_compound,
!!(ctrls->sequence->flags
& V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND));
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] media: verisilicon: AV1: Fix tx mode bit setting
2025-12-08 9:52 [PATCH] media: verisilicon: AV1: Fix enable cdef computation Benjamin Gaignard
@ 2025-12-08 9:52 ` Benjamin Gaignard
2025-12-08 19:32 ` Nicolas Dufresne
2025-12-08 19:45 ` [PATCH] media: verisilicon: AV1: Fix enable cdef computation Nicolas Dufresne
1 sibling, 1 reply; 4+ messages in thread
From: Benjamin Gaignard @ 2025-12-08 9:52 UTC (permalink / raw)
To: nicolas.dufresne, p.zabel, mchehab, heiko, hverkuil
Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
kernel, Benjamin Gaignard
If tx_mode field is non-zero then write it value + 2 else
write 0.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Fixes: 727a400686a2c ("media: verisilicon: Add Rockchip AV1 decoder")
---
drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 2 +-
1 file changed, 1 insertion(+), 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 f4f7cb45b1f1..fccdece51b1b 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -2005,7 +2005,7 @@ static void rockchip_vpu981_av1_dec_set_parameters(struct hantro_ctx *ctx)
!!(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_ALLOW_HIGH_PRECISION_MV));
hantro_reg_write(vpu, &av1_comp_pred_mode,
(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_REFERENCE_SELECT) ? 2 : 0);
- hantro_reg_write(vpu, &av1_transform_mode, (ctrls->frame->tx_mode == 1) ? 3 : 4);
+ hantro_reg_write(vpu, &av1_transform_mode, ctrls->frame->tx_mode ? ctrls->frame->tx_mode + 2 : 0);
hantro_reg_write(vpu, &av1_max_cb_size,
(ctrls->sequence->flags
& V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK) ? 7 : 6);
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] media: verisilicon: AV1: Fix tx mode bit setting
2025-12-08 9:52 ` [PATCH] media: verisilicon: AV1: Fix tx mode bit setting Benjamin Gaignard
@ 2025-12-08 19:32 ` Nicolas Dufresne
0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Dufresne @ 2025-12-08 19:32 UTC (permalink / raw)
To: Benjamin Gaignard, p.zabel, mchehab, heiko, hverkuil
Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
kernel
[-- Attachment #1.1: Type: text/plain, Size: 2261 bytes --]
Hi,
Le lundi 08 décembre 2025 à 10:52 +0100, Benjamin Gaignard a écrit :
> If tx_mode field is non-zero then write it value + 2 else
it -> its
> write 0.
Please don't sent a an In-reply-to: email for a patch that isn't part of a
series.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 727a400686a2c ("media: verisilicon: Add Rockchip AV1 decoder")
> ---
> drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 2 +-
> 1 file changed, 1 insertion(+), 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 f4f7cb45b1f1..fccdece51b1b 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> @@ -2005,7 +2005,7 @@ static void
> rockchip_vpu981_av1_dec_set_parameters(struct hantro_ctx *ctx)
> !!(ctrls->frame->flags &
> V4L2_AV1_FRAME_FLAG_ALLOW_HIGH_PRECISION_MV));
> hantro_reg_write(vpu, &av1_comp_pred_mode,
> (ctrls->frame->flags &
> V4L2_AV1_FRAME_FLAG_REFERENCE_SELECT) ? 2 : 0);
> - hantro_reg_write(vpu, &av1_transform_mode, (ctrls->frame->tx_mode ==
> 1) ? 3 : 4);
> + hantro_reg_write(vpu, &av1_transform_mode, ctrls->frame->tx_mode ?
> ctrls->frame->tx_mode + 2 : 0);
That all seem very hacky. Let's step back, and use that as inspiration for a
useful commit message. From bitstream we have:
tx_mode:
0 == 4x4 only
1 == SELECT
2 == LARGEST
And the HW have:
0 == 4x4 only
1 == 8x8 and less
2 == 16x16 and less
3 == 32x32 and less
4 == Select
Since the two enums have no relation (except for value 0), I'd suggest to
translate this in a switch (with nice enums so its readable).
0 -> 0
1 -> 4
2 -> 3
Would be good to check if we can refine with some other params to reach HW value
1 and 2, but otherwise, that mapping is sufficient. Tha hacky +2 is really
obscure to me and I'd rather not do that upstream.
Nicolas
> hantro_reg_write(vpu, &av1_max_cb_size,
> (ctrls->sequence->flags
> & V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK) ?
> 7 : 6);
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: verisilicon: AV1: Fix enable cdef computation
2025-12-08 9:52 [PATCH] media: verisilicon: AV1: Fix enable cdef computation Benjamin Gaignard
2025-12-08 9:52 ` [PATCH] media: verisilicon: AV1: Fix tx mode bit setting Benjamin Gaignard
@ 2025-12-08 19:45 ` Nicolas Dufresne
1 sibling, 0 replies; 4+ messages in thread
From: Nicolas Dufresne @ 2025-12-08 19:45 UTC (permalink / raw)
To: Benjamin Gaignard, p.zabel, mchehab, heiko, hverkuil
Cc: linux-media, linux-rockchip, linux-arm-kernel, linux-kernel,
kernel, amazingfate
[-- Attachment #1.1: Type: text/plain, Size: 2902 bytes --]
Hi,
Le lundi 08 décembre 2025 à 10:52 +0100, Benjamin Gaignard a écrit :
> Testing V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF flag isn't enough
> to know if cdef bit has to be set.
> If any of the used cdef fields isn't zero then we must enable
> cdef feature on the hardware.
I think the problem goes the other way around.
If all the fields of the CDEF parameters are zero (which is the default), then
av1_enable_cdef register needs to be unset (despite the
V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF possibly being set).
Its interesting to note that the other AV1 decoder also ignores this flag.
Though, I don't have enough data to add something to the doc to try and convince
future driver writers to not use it.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: 727a400686a2c ("media: verisilicon: Add Rockchip AV1 decoder")
This is missing:
Reported-by: Jianfeng Liu <liujianfeng1994@gmail.com>
Link: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4786
Please also include my Rb in v2 with correct commit message.
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Nicolas
> ---
> .../platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> 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 e4703bb6be7c..f4f7cb45b1f1 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> @@ -1396,8 +1396,16 @@ static void rockchip_vpu981_av1_dec_set_cdef(struct hantro_ctx *ctx)
> u16 luma_sec_strength = 0;
> u32 chroma_pri_strength = 0;
> u16 chroma_sec_strength = 0;
> + bool enable_cdef;
> int i;
>
> + enable_cdef = !(cdef->bits == 0 &&
> + cdef->damping_minus_3 == 0 &&
> + cdef->y_pri_strength[0] == 0 &&
> + cdef->y_sec_strength[0] == 0 &&
> + cdef->uv_pri_strength[0] == 0 &&
> + cdef->uv_sec_strength[0] == 0);
> + hantro_reg_write(vpu, &av1_enable_cdef, enable_cdef);
> hantro_reg_write(vpu, &av1_cdef_bits, cdef->bits);
> hantro_reg_write(vpu, &av1_cdef_damping, cdef->damping_minus_3);
>
> @@ -1953,8 +1961,6 @@ static void rockchip_vpu981_av1_dec_set_parameters(struct hantro_ctx *ctx)
> !!(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_SHOW_FRAME));
> hantro_reg_write(vpu, &av1_switchable_motion_mode,
> !!(ctrls->frame->flags & V4L2_AV1_FRAME_FLAG_IS_MOTION_MODE_SWITCHABLE));
> - hantro_reg_write(vpu, &av1_enable_cdef,
> - !!(ctrls->sequence->flags & V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF));
> hantro_reg_write(vpu, &av1_allow_masked_compound,
> !!(ctrls->sequence->flags
> & V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND));
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-12-08 19:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-08 9:52 [PATCH] media: verisilicon: AV1: Fix enable cdef computation Benjamin Gaignard
2025-12-08 9:52 ` [PATCH] media: verisilicon: AV1: Fix tx mode bit setting Benjamin Gaignard
2025-12-08 19:32 ` Nicolas Dufresne
2025-12-08 19:45 ` [PATCH] media: verisilicon: AV1: Fix enable cdef computation Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox