* [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
@ 2022-04-26 13:50 Benjamin Gaignard
2022-04-26 15:07 ` Sebastian Fricke
2022-05-03 13:12 ` Ezequiel Garcia
0 siblings, 2 replies; 4+ messages in thread
From: Benjamin Gaignard @ 2022-04-26 13:50 UTC (permalink / raw)
To: ezequiel, p.zabel, mchehab, gregkh
Cc: linux-media, linux-rockchip, linux-staging, linux-kernel, jon,
aford173, kernel, Benjamin Gaignard
Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2
register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT
flag value.
This fix CAINIT_G_SHARP_3 test in fluster.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
index 6deb31b7b993..503f4b028bc5 100644
--- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
+++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
@@ -194,13 +194,8 @@ static void set_params(struct hantro_ctx *ctx)
hantro_reg_write(vpu, &g2_max_cu_qpd_depth, 0);
}
- if (pps->flags & V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT) {
- hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
- hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
- } else {
- hantro_reg_write(vpu, &g2_cb_qp_offset, 0);
- hantro_reg_write(vpu, &g2_cr_qp_offset, 0);
- }
+ hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
+ hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
hantro_reg_write(vpu, &g2_filt_offset_beta, pps->pps_beta_offset_div2);
hantro_reg_write(vpu, &g2_filt_offset_tc, pps->pps_tc_offset_div2);
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
2022-04-26 13:50 [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
@ 2022-04-26 15:07 ` Sebastian Fricke
2022-05-03 13:12 ` Ezequiel Garcia
1 sibling, 0 replies; 4+ messages in thread
From: Sebastian Fricke @ 2022-04-26 15:07 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: ezequiel, p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
linux-staging, linux-kernel, jon, aford173, kernel
Hey Benjamin,
On 26.04.2022 15:50, Benjamin Gaignard wrote:
>Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2
>register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT
>flag value.
>This fix CAINIT_G_SHARP_3 test in fluster.
just a small typo:
s/fix/fixes the/
Greetings,
Sebastian
>
>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>---
> drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>index 6deb31b7b993..503f4b028bc5 100644
>--- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>+++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>@@ -194,13 +194,8 @@ static void set_params(struct hantro_ctx *ctx)
> hantro_reg_write(vpu, &g2_max_cu_qpd_depth, 0);
> }
>
>- if (pps->flags & V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT) {
>- hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
>- hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>- } else {
>- hantro_reg_write(vpu, &g2_cb_qp_offset, 0);
>- hantro_reg_write(vpu, &g2_cr_qp_offset, 0);
>- }
>+ hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
>+ hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>
> hantro_reg_write(vpu, &g2_filt_offset_beta, pps->pps_beta_offset_div2);
> hantro_reg_write(vpu, &g2_filt_offset_tc, pps->pps_tc_offset_div2);
>--
>2.32.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
2022-04-26 13:50 [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
2022-04-26 15:07 ` Sebastian Fricke
@ 2022-05-03 13:12 ` Ezequiel Garcia
2022-05-03 13:32 ` Benjamin Gaignard
1 sibling, 1 reply; 4+ messages in thread
From: Ezequiel Garcia @ 2022-05-03 13:12 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Philipp Zabel, Mauro Carvalho Chehab, Greg KH, linux-media,
open list:ARM/Rockchip SoC...,
open list:STAGING SUBSYSTEM, Linux Kernel Mailing List, jon,
Adam Ford, Collabora Kernel ML
Hi Benjamin,
On Tue, Apr 26, 2022 at 10:50 AM Benjamin Gaignard
<benjamin.gaignard@collabora.com> wrote:
>
> Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2
> register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT
> flag value.
I would say we need more justification why this is correct, or at least
checking what the reference vendor implementation is doing (and mentioning
in the commit description so we can track it in the future).
> This fix CAINIT_G_SHARP_3 test in fluster.
>
This could sound like a tad a pedantic detail, but I'd say it's
important we stop refering to tests
as "fluster tests", and instead say something more correct as "HEVC
conformance test CAINIT_G_SHARP_3".
Also, when we are fixing conformance tests, let's please add the
Fluster score (in this case, I think it's
OK to refer to Fluster).
PS: Same comments apply to patch "media: hantro: HEVC: Fix reference
frames management".
Thanks,
Ezequiel
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> index 6deb31b7b993..503f4b028bc5 100644
> --- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> @@ -194,13 +194,8 @@ static void set_params(struct hantro_ctx *ctx)
> hantro_reg_write(vpu, &g2_max_cu_qpd_depth, 0);
> }
>
> - if (pps->flags & V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT) {
> - hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
> - hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
> - } else {
> - hantro_reg_write(vpu, &g2_cb_qp_offset, 0);
> - hantro_reg_write(vpu, &g2_cr_qp_offset, 0);
> - }
> + hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
> + hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>
> hantro_reg_write(vpu, &g2_filt_offset_beta, pps->pps_beta_offset_div2);
> hantro_reg_write(vpu, &g2_filt_offset_tc, pps->pps_tc_offset_div2);
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
2022-05-03 13:12 ` Ezequiel Garcia
@ 2022-05-03 13:32 ` Benjamin Gaignard
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Gaignard @ 2022-05-03 13:32 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Philipp Zabel, Mauro Carvalho Chehab, Greg KH, linux-media,
open list:ARM/Rockchip SoC...,
open list:STAGING SUBSYSTEM, Linux Kernel Mailing List, jon,
Adam Ford, Collabora Kernel ML
Le 03/05/2022 à 15:12, Ezequiel Garcia a écrit :
> Hi Benjamin,
>
> On Tue, Apr 26, 2022 at 10:50 AM Benjamin Gaignard
> <benjamin.gaignard@collabora.com> wrote:
>> Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2
>> register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT
>> flag value.
> I would say we need more justification why this is correct, or at least
> checking what the reference vendor implementation is doing (and mentioning
> in the commit description so we can track it in the future).
Yes that is what the vendor implementation is doing.
>
>> This fix CAINIT_G_SHARP_3 test in fluster.
>>
> This could sound like a tad a pedantic detail, but I'd say it's
> important we stop refering to tests
> as "fluster tests", and instead say something more correct as "HEVC
> conformance test CAINIT_G_SHARP_3".
As you want.
>
> Also, when we are fixing conformance tests, let's please add the
> Fluster score (in this case, I think it's
> OK to refer to Fluster).
We are fixing bugs in parallel in the driver, the uAPI and GStreamer
so fluster score evolution reflect that progression and maybe not only
what this patch is fixing.
The best I could says here is that patch fix HEVC conformance test
CAINIT_G_SHARP_3 so fluster score increase by one.
Regards,
Benjamin
>
> PS: Same comments apply to patch "media: hantro: HEVC: Fix reference
> frames management".
>
> Thanks,
> Ezequiel
>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> ---
>> drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 9 ++-------
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>> index 6deb31b7b993..503f4b028bc5 100644
>> --- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>> +++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>> @@ -194,13 +194,8 @@ static void set_params(struct hantro_ctx *ctx)
>> hantro_reg_write(vpu, &g2_max_cu_qpd_depth, 0);
>> }
>>
>> - if (pps->flags & V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT) {
>> - hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
>> - hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>> - } else {
>> - hantro_reg_write(vpu, &g2_cb_qp_offset, 0);
>> - hantro_reg_write(vpu, &g2_cr_qp_offset, 0);
>> - }
>> + hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
>> + hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>>
>> hantro_reg_write(vpu, &g2_filt_offset_beta, pps->pps_beta_offset_div2);
>> hantro_reg_write(vpu, &g2_filt_offset_tc, pps->pps_tc_offset_div2);
>> --
>> 2.32.0
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-03 13:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 13:50 [PATCH v2] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
2022-04-26 15:07 ` Sebastian Fricke
2022-05-03 13:12 ` Ezequiel Garcia
2022-05-03 13:32 ` Benjamin Gaignard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).