The bytesperline and sizeimage of each plan are different for 8bit
and 10bit bitstreams. Using v4l2 common interface to calculate them
independently.
---
compare with v1:
- add patch 1/2/3/4
- change the calculation method for sizeimage for patch 5
---
Yunfei Dong (5):
media: mediatek: vcodec: fix incorrect MT2T format information
media: mediatek: vcodec: fix incorrect MT2R format information
media: mediatek: vcodec: add MM21 format definition
media: mediatek: vcodec: add MT21 format definition
media: mediatek: vcodec: fix incorrect sizeimage for 10bit bitstream
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++-------------
drivers/media/v4l2-core/v4l2-common.c | 12 +++++---
2 files changed, 16 insertions(+), 24 deletions(-)
--
2.18.0
Changing the bpp and hdiv values to make sure the bytesperline
of plane[0] and plane[1] are the same. The width and height are
64 align.
Fixes: 1dff2beb60d3 ("media: mediatek: vcodec: Add capture format to support 10bit raster mode")
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/media/v4l2-core/v4l2-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 8587cd14741c..cef1492dba22 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -267,8 +267,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
{ .format = V4L2_PIX_FMT_YUV48_12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 6, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
{ .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2,
.block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
- { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
- .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
+ { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2,
+ .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
/* YUV planar formats */
{ .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
--
2.18.0
Adding the definition of MM21 format to calculate bytesperline
and sizeimage of plane[0] and plane[1].
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/media/v4l2-core/v4l2-common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index cef1492dba22..0d5de132e07f 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -269,6 +269,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
.block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
{ .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2,
.block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
+ { .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
+ .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
/* YUV planar formats */
{ .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
--
2.18.0
Adding the definition of MT21 format to calculate bytesperline
and sizeimage of plane[0] and plane[1].
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/media/v4l2-core/v4l2-common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 0d5de132e07f..6dba989c2291 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
.block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
{ .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
.block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
+ { .format = V4L2_PIX_FMT_MT21C, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
+ .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
/* YUV planar formats */
{ .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
--
2.18.0
The sizeimage of each plane is calculated the same way for 8bit and
10bit bitstream. Using v4l2 common interface v4l2_fill_pixfmt_mp to
separate.
Fixes: 9d86be9bda6c ("media: mediatek: vcodec: Add driver to support 10bit")
Signed-off-by: Yunfei Dong <[email protected]>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++-------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index 9107707de6c4..fbfba69682ea 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -262,40 +262,28 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
int tmp_w, tmp_h;
/*
- * Find next closer width align 64, height align 64, size align
- * 64 rectangle
+ * Find next closer width align 64, heign align 64, size align 64 rectangle
* Note: This only get default value, the real HW needed value
* only available when ctx in MTK_STATE_HEADER state
*/
tmp_w = pix_fmt_mp->width;
tmp_h = pix_fmt_mp->height;
+
v4l_bound_align_image(&pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width, 6,
&pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height, 6,
9);
- if (pix_fmt_mp->width < tmp_w &&
- (pix_fmt_mp->width + 64) <= frmsize->max_width)
+ if (pix_fmt_mp->width < tmp_w && (pix_fmt_mp->width + 64) <= frmsize->max_width)
pix_fmt_mp->width += 64;
- if (pix_fmt_mp->height < tmp_h &&
- (pix_fmt_mp->height + 64) <= frmsize->max_height)
+ if (pix_fmt_mp->height < tmp_h && (pix_fmt_mp->height + 64) <= frmsize->max_height)
pix_fmt_mp->height += 64;
+ v4l2_fill_pixfmt_mp(pix_fmt_mp, fmt->fourcc, pix_fmt_mp->width, pix_fmt_mp->height);
mtk_v4l2_vdec_dbg(0, ctx,
- "before resize wxh=%dx%d, after resize wxh=%dx%d, sizeimage=%d",
+ "before resize:%dx%d, after resize:%dx%d, sizeimage=0x%x_0x%x",
tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
- pix_fmt_mp->width * pix_fmt_mp->height);
-
- pix_fmt_mp->num_planes = fmt->num_planes;
- pix_fmt_mp->plane_fmt[0].sizeimage =
- pix_fmt_mp->width * pix_fmt_mp->height;
- pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
-
- if (pix_fmt_mp->num_planes == 2) {
- pix_fmt_mp->plane_fmt[1].sizeimage =
- (pix_fmt_mp->width * pix_fmt_mp->height) / 2;
- pix_fmt_mp->plane_fmt[1].bytesperline =
- pix_fmt_mp->width;
- }
+ pix_fmt_mp->plane_fmt[0].sizeimage,
+ pix_fmt_mp->plane_fmt[1].sizeimage);
}
pix_fmt_mp->flags = 0;
--
2.18.0
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit :
> Adding the definition of MM21 format to calculate bytesperline
> and sizeimage of plane[0] and plane[1].
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index cef1492dba22..0d5de132e07f 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -269,6 +269,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
> { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2,
> .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
> + { .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
> + .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
MM21 has the same subsampling as NV12M, so hdiv should be 2 for correctness The
64x64 block is incorrect. The only difference between this and NV12M should be
with block.
>
> /* YUV planar formats */
> { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit :
> Adding the definition of MT21 format to calculate bytesperline
MT21C in the subject and description.
> and sizeimage of plane[0] and plane[1].
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 0d5de132e07f..6dba989c2291 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
> { .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
> .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
> + { .format = V4L2_PIX_FMT_MT21C, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2,
> + .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},
As you may notice, we have no support for compressed formats yet in this helper.
I believe for these formats we need some extra code to add the header size
calculation. One way could be to add a ops in that table for when a header is
needed. That ops would implement the format specific extra space calculation
which would be added to sizeimage of each planes (in in one of the planes
depending on the specifics of the format).
>
> /* YUV planar formats */
> { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
Hi,
sorry for the long delay.
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit :
> The bytesperline and sizeimage of each plan are different for 8bit
> and 10bit bitstreams. Using v4l2 common interface to calculate them
> independently.
>
> ---
> compare with v1:
> - add patch 1/2/3/4
> - change the calculation method for sizeimage for patch 5
> ---
> Yunfei Dong (5):
> media: mediatek: vcodec: fix incorrect MT2T format information
> media: mediatek: vcodec: fix incorrect MT2R format information
As per recommandation of the "Submitting Patches":
> On the other hand, if you make a single change to numerous files, group those
> changes into a single patch. Thus a single logical change is contained within
> a single patch.
In general, when two patches have an identical description its a sign they
should be one patch. Would be nice to squash these two.
> media: mediatek: vcodec: add MM21 format definition
> media: mediatek: vcodec: add MT21 format definition
Same.
> media: mediatek: vcodec: fix incorrect sizeimage for 10bit bitstream
>
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++-------------
> drivers/media/v4l2-core/v4l2-common.c | 12 +++++---
> 2 files changed, 16 insertions(+), 24 deletions(-)
>
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit :
> The sizeimage of each plane is calculated the same way for 8bit and
> 10bit bitstream. Using v4l2 common interface v4l2_fill_pixfmt_mp to
> separate.
>
> Fixes: 9d86be9bda6c ("media: mediatek: vcodec: Add driver to support 10bit")
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++-------------
> 1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> index 9107707de6c4..fbfba69682ea 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> @@ -262,40 +262,28 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
> int tmp_w, tmp_h;
>
> /*
> - * Find next closer width align 64, height align 64, size align
> - * 64 rectangle
> + * Find next closer width align 64, heign align 64, size align 64 rectangle
> * Note: This only get default value, the real HW needed value
> * only available when ctx in MTK_STATE_HEADER state
> */
> tmp_w = pix_fmt_mp->width;
> tmp_h = pix_fmt_mp->height;
> +
> v4l_bound_align_image(&pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width, 6,
> &pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height, 6,
> 9);
>
> - if (pix_fmt_mp->width < tmp_w &&
> - (pix_fmt_mp->width + 64) <= frmsize->max_width)
> + if (pix_fmt_mp->width < tmp_w && (pix_fmt_mp->width + 64) <= frmsize->max_width)
> pix_fmt_mp->width += 64;
> - if (pix_fmt_mp->height < tmp_h &&
> - (pix_fmt_mp->height + 64) <= frmsize->max_height)
> + if (pix_fmt_mp->height < tmp_h && (pix_fmt_mp->height + 64) <= frmsize->max_height)
> pix_fmt_mp->height += 64;
This seems overlay complicated, make sure your frmsize is valid (with
step_width/height set to 64 from your Mali requirement) and then simply call:
v4l2_apply_frmsize_constraints(...)
All this hand written clamping code with hardcoded values should go away.
>
> + v4l2_fill_pixfmt_mp(pix_fmt_mp, fmt->fourcc, pix_fmt_mp->width, pix_fmt_mp->height);
> mtk_v4l2_vdec_dbg(0, ctx,
> - "before resize wxh=%dx%d, after resize wxh=%dx%d, sizeimage=%d",
> + "before resize:%dx%d, after resize:%dx%d, sizeimage=0x%x_0x%x",
> tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
> - pix_fmt_mp->width * pix_fmt_mp->height);
> -
> - pix_fmt_mp->num_planes = fmt->num_planes;
> - pix_fmt_mp->plane_fmt[0].sizeimage =
> - pix_fmt_mp->width * pix_fmt_mp->height;
> - pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
> -
> - if (pix_fmt_mp->num_planes == 2) {
> - pix_fmt_mp->plane_fmt[1].sizeimage =
> - (pix_fmt_mp->width * pix_fmt_mp->height) / 2;
> - pix_fmt_mp->plane_fmt[1].bytesperline =
> - pix_fmt_mp->width;
> - }
> + pix_fmt_mp->plane_fmt[0].sizeimage,
> + pix_fmt_mp->plane_fmt[1].sizeimage);
> }
>
> pix_fmt_mp->flags = 0;