* [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
@ 2022-05-03 13:55 Benjamin Gaignard
2022-05-03 14:48 ` Ezequiel Garcia
2022-05-03 14:49 ` Ezequiel Garcia
0 siblings, 2 replies; 3+ messages in thread
From: Benjamin Gaignard @ 2022-05-03 13:55 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.
The vendor code does the case to set these values.
This fix conformance test CAINIT_G_SHARP_3.
Fluster HEVC score is increase by one with this patch.
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] 3+ messages in thread
* Re: [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
2022-05-03 13:55 [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
@ 2022-05-03 14:48 ` Ezequiel Garcia
2022-05-03 14:49 ` Ezequiel Garcia
1 sibling, 0 replies; 3+ messages in thread
From: Ezequiel Garcia @ 2022-05-03 14:48 UTC (permalink / raw
To: Benjamin Gaignard
Cc: p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
linux-staging, linux-kernel, jon, aford173, kernel
On Tue, May 03, 2022 at 03:55:29PM +0200, 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.
> The vendor code does the case to set these values.
s/case/same
> This fix conformance test CAINIT_G_SHARP_3.
>
> Fluster HEVC score is increase by one with this patch.
>
Saying "score is increased by one" is not all that useful.
I still believe seeing the Fluster score would be adding real
information.
The score you have without this patch, and using upstream GStreamer
is the "current" score. Then the score you get with the patch applied,
is the score you get after the fix.
And this is actually good as you would also give more information
by clarifying the score is the result of GStreamer (commit $sha)
plus Linux.
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Patch looks fine, but I believe you still have some challenges on the commit
descriptions, and so we iterate a lot on them.
How about you proof-read them first (or you ask colleagues to proof-read
them)?
A useful tip I've profit from is to let patches sit for a few days,
then re-read and amend the commit before sending them.
Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Thanks!
Ezequiel
> ---
> 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] 3+ messages in thread
* Re: [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values
2022-05-03 13:55 [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
2022-05-03 14:48 ` Ezequiel Garcia
@ 2022-05-03 14:49 ` Ezequiel Garcia
1 sibling, 0 replies; 3+ messages in thread
From: Ezequiel Garcia @ 2022-05-03 14:49 UTC (permalink / raw
To: Benjamin Gaignard
Cc: p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
linux-staging, linux-kernel, jon, aford173, kernel
On Tue, May 03, 2022 at 03:55:29PM +0200, 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.
> The vendor code does the case to set these values.
> This fix conformance test CAINIT_G_SHARP_3.
>
Another silly nitpick: s/fix/fixes.
> Fluster HEVC score is increase by one with this patch.
>
> 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] 3+ messages in thread
end of thread, other threads:[~2022-05-03 14:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-03 13:55 [PATCH v3] media: hantro: HEVC: unconditionnaly set pps_{cb/cr}_qp_offset values Benjamin Gaignard
2022-05-03 14:48 ` Ezequiel Garcia
2022-05-03 14:49 ` Ezequiel Garcia
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 read-only IMAP folder(s) and NNTP newsgroup(s).