2024-02-02 07:04:31

by jackson.lee

Subject: [PATCH v1 0/5] wave5 codec driver

The wave5 codec driver is a stateful encoder/decoder.
The following patches is for supporting yuv422 inpuy format, supporting
runtime suspend/resume feature and extra things.

Change since v0:
=================
The DEFAULT_SRC_SIZE macro was defined using multiple lines,
To make a simple define, tab and multiple lines has been removed,
The macro is defined using one line.


jackson.lee (5):
wave5 : Support yuv422 input format for encoder.
wave5: Support to prepend sps/pps to IDR frame.
wave5 : Support runtime suspend/resume.
wave5: Use the bitstream buffer size from host.
wave5 : Fixed the wrong buffer size formula.

.../platform/chips-media/wave5/wave5-hw.c | 11 +-
.../chips-media/wave5/wave5-vpu-dec.c | 86 +++++-----
.../chips-media/wave5/wave5-vpu-enc.c | 157 +++++++++++++++---
.../platform/chips-media/wave5/wave5-vpu.c | 68 ++++++++
.../platform/chips-media/wave5/wave5-vpuapi.c | 7 +
.../platform/chips-media/wave5/wave5-vpuapi.h | 1 +
.../media/platform/chips-media/wave5/wave5.h | 3 +
7 files changed, 253 insertions(+), 80 deletions(-)

--
2.43.0

2024-02-02 07:05:50

by jackson.lee

Subject: [PATCH v1 5/5] wave5 : Fixed the wrong buffer size formula.

S_FMT/G_FMT should report the buffer size based on aligned width and height.
And, Host can set the real encoding size through s_selection and g_selection.
So, Driver should use the conf_win information for encoding size instead of size of S_FMT/G_FMT.

Signed-off-by: Nas Chung <[email protected]>
Signed-off-by: Jackson Lee <[email protected]>
---
.../chips-media/wave5/wave5-vpu-dec.c | 77 +++++++------------
.../chips-media/wave5/wave5-vpu-enc.c | 77 +++++++++++--------
2 files changed, 72 insertions(+), 82 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 328a7a8f26c5..fb9449908ebd 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -243,54 +243,54 @@ static void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp, unsigned
case V4L2_PIX_FMT_NV21:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height * 3 / 2;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height * 3 / 2;
break;
case V4L2_PIX_FMT_YUV422P:
case V4L2_PIX_FMT_NV16:
case V4L2_PIX_FMT_NV61:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height * 2;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height * 2;
break;
case V4L2_PIX_FMT_YUV420M:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[1].sizeimage = width * height / 4;
- pix_mp->plane_fmt[2].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[2].sizeimage = width * height / 4;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height / 4;
+ pix_mp->plane_fmt[2].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[2].sizeimage = pix_mp->width * pix_mp->height / 4;
break;
case V4L2_PIX_FMT_NV12M:
case V4L2_PIX_FMT_NV21M:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[1].sizeimage = width * height / 2;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height / 2;
break;
case V4L2_PIX_FMT_YUV422M:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[1].sizeimage = width * height / 2;
- pix_mp->plane_fmt[2].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[2].sizeimage = width * height / 2;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height / 2;
+ pix_mp->plane_fmt[2].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[2].sizeimage = pix_mp->width * pix_mp->height / 2;
break;
case V4L2_PIX_FMT_NV16M:
case V4L2_PIX_FMT_NV61M:
pix_mp->width = round_up(width, 32);
pix_mp->height = round_up(height, 16);
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = width * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[1].sizeimage = width * height;
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height;
break;
default:
pix_mp->width = width;
@@ -1003,6 +1003,7 @@ static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buff
struct vpu_instance *inst = vb2_get_drv_priv(q);
struct v4l2_pix_format_mplane inst_format =
(q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? inst->src_fmt : inst->dst_fmt;
+ unsigned int i;

dev_dbg(inst->dev->dev, "%s: num_buffers: %u | num_planes: %u | type: %u\n", __func__,
*num_buffers, *num_planes, q->type);
@@ -1016,31 +1017,9 @@ static int wave5_vpu_dec_queue_setup(struct vb2_queue *q, unsigned int *num_buff
if (*num_buffers < inst->fbc_buf_count)
*num_buffers = inst->fbc_buf_count;

- if (*num_planes == 1) {
- if (inst->output_format == FORMAT_422)
- sizes[0] = inst_format.width * inst_format.height * 2;
- else
- sizes[0] = inst_format.width * inst_format.height * 3 / 2;
- dev_dbg(inst->dev->dev, "%s: size[0]: %u\n", __func__, sizes[0]);
- } else if (*num_planes == 2) {
- sizes[0] = inst_format.width * inst_format.height;
- if (inst->output_format == FORMAT_422)
- sizes[1] = inst_format.width * inst_format.height;
- else
- sizes[1] = inst_format.width * inst_format.height / 2;
- dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u\n",
- __func__, sizes[0], sizes[1]);
- } else if (*num_planes == 3) {
- sizes[0] = inst_format.width * inst_format.height;
- if (inst->output_format == FORMAT_422) {
- sizes[1] = inst_format.width * inst_format.height / 2;
- sizes[2] = inst_format.width * inst_format.height / 2;
- } else {
- sizes[1] = inst_format.width * inst_format.height / 4;
- sizes[2] = inst_format.width * inst_format.height / 4;
- }
- dev_dbg(inst->dev->dev, "%s: size[0]: %u | size[1]: %u | size[2]: %u\n",
- __func__, sizes[0], sizes[1], sizes[2]);
+ for (i = 0; i < *num_planes; i++) {
+ sizes[i] = inst_format.plane_fmt[i].sizeimage;
+ dev_dbg(inst->dev->dev, "%s: size[%u]: %u\n", __func__, i, sizes[i]);
}
}

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
index 62be7cdfccd8..6898d1513f11 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
@@ -150,46 +150,46 @@ static void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp, unsigned
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_NV12:
case V4L2_PIX_FMT_NV21:
- pix_mp->width = width;
- pix_mp->height = height;
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height * 3 / 2;
+ pix_mp->width = round_up(width, 32);
+ pix_mp->height = round_up(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height * 3 / 2;
break;
case V4L2_PIX_FMT_YUV420M:
- pix_mp->width = width;
- pix_mp->height = height;
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[1].sizeimage = round_up(width, 32) * height / 4;
- pix_mp->plane_fmt[2].bytesperline = round_up(width, 32) / 2;
- pix_mp->plane_fmt[2].sizeimage = round_up(width, 32) * height / 4;
+ pix_mp->width = round_up(width, 32);
+ pix_mp->height = round_up(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height / 4;
+ pix_mp->plane_fmt[2].bytesperline = pix_mp->width / 2;
+ pix_mp->plane_fmt[2].sizeimage = pix_mp->width * pix_mp->height / 4;
break;
case V4L2_PIX_FMT_NV12M:
case V4L2_PIX_FMT_NV21M:
- pix_mp->width = width;
- pix_mp->height = height;
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[1].sizeimage = round_up(width, 32) * height / 2;
+ pix_mp->width = round_up(width, 32);
+ pix_mp->height = round_up(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height / 2;
break;
case V4L2_PIX_FMT_YUV422P:
case V4L2_PIX_FMT_NV16:
case V4L2_PIX_FMT_NV61:
- pix_mp->width = width;
- pix_mp->height = height;
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height * 2;
+ pix_mp->width = round_up(width, 32);
+ pix_mp->height = round_up(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height * 2;
break;
case V4L2_PIX_FMT_NV16M:
case V4L2_PIX_FMT_NV61M:
- pix_mp->width = width;
- pix_mp->height = height;
- pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height;
- pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
- pix_mp->plane_fmt[1].sizeimage = round_up(width, 32) * height;
+ pix_mp->width = round_up(width, 32);
+ pix_mp->height = round_up(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[0].sizeimage = pix_mp->width * pix_mp->height;
+ pix_mp->plane_fmt[1].bytesperline = pix_mp->width;
+ pix_mp->plane_fmt[1].sizeimage = pix_mp->width * pix_mp->height;
break;
default:
pix_mp->width = width;
@@ -636,6 +636,8 @@ static int wave5_vpu_enc_s_fmt_out(struct file *file, void *fh, struct v4l2_form
inst->xfer_func = f->fmt.pix_mp.xfer_func;

wave5_update_pix_fmt(&inst->dst_fmt, f->fmt.pix_mp.width, f->fmt.pix_mp.height);
+ inst->conf_win.width = inst->dst_fmt.width;
+ inst->conf_win.height = inst->dst_fmt.height;

return 0;
}
@@ -651,12 +653,17 @@ static int wave5_vpu_enc_g_selection(struct file *file, void *fh, struct v4l2_se
switch (s->target) {
case V4L2_SEL_TGT_CROP_DEFAULT:
case V4L2_SEL_TGT_CROP_BOUNDS:
- case V4L2_SEL_TGT_CROP:
s->r.left = 0;
s->r.top = 0;
s->r.width = inst->dst_fmt.width;
s->r.height = inst->dst_fmt.height;
break;
+ case V4L2_SEL_TGT_CROP:
+ s->r.left = 0;
+ s->r.top = 0;
+ s->r.width = inst->conf_win.width;
+ s->r.height = inst->conf_win.height;
+ break;
default:
return -EINVAL;
}
@@ -679,8 +686,10 @@ static int wave5_vpu_enc_s_selection(struct file *file, void *fh, struct v4l2_se

s->r.left = 0;
s->r.top = 0;
- s->r.width = inst->src_fmt.width;
- s->r.height = inst->src_fmt.height;
+ s->r.width = min(s->r.width, inst->dst_fmt.width);
+ s->r.height = min(s->r.height, inst->dst_fmt.height);
+
+ inst->conf_win = s->r;

return 0;
}
@@ -1227,8 +1236,8 @@ static void wave5_set_enc_openparam(struct enc_open_param *open_param,
open_param->wave_param.lambda_scaling_enable = 1;

open_param->line_buf_int_en = true;
- open_param->pic_width = inst->dst_fmt.width;
- open_param->pic_height = inst->dst_fmt.height;
+ open_param->pic_width = inst->conf_win.width;
+ open_param->pic_height = inst->conf_win.height;
open_param->frame_rate_info = inst->frame_rate;
open_param->rc_enable = inst->rc_enable;
if (inst->rc_enable) {
@@ -1804,6 +1813,8 @@ static int wave5_vpu_open_enc(struct file *filp)
v4l2_ctrl_handler_setup(v4l2_ctrl_hdl);

wave5_set_default_format(&inst->src_fmt, &inst->dst_fmt);
+ inst->conf_win.width = inst->dst_fmt.width;
+ inst->conf_win.height = inst->dst_fmt.height;
inst->colorspace = V4L2_COLORSPACE_REC709;
inst->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
inst->quantization = V4L2_QUANTIZATION_DEFAULT;
--
2.43.0

2024-02-02 08:22:11

by Sebastian Fricke

Subject: Re: [PATCH v1 0/5] wave5 codec driver

Hey Jackson,

thanks for sending the patches!

I would advise to not send the next version of a patch series with
multiple patches too quickly. So far the patches only got a single very
minor review comment and others might already work on a deeper review
and test. There is no hard rule but I would say give such a series at
least a week or two to gather some review comments before sending the
next version, as it is always a bit of hassle to adjust to a new
version and you don't want to spam the mailing list.
Should you not receive any more review comments, then please go ahead
and ping some relevant people (Hans Verkuil, people from Collabora that
developed the patches, etc.) to review, either per mail or per IRC on
the OFTC server on the channel #linux-media.

Greetings,
Sebastian

On 02.02.2024 16:03, jackson.lee wrote:
>The wave5 codec driver is a stateful encoder/decoder.
>The following patches is for supporting yuv422 inpuy format, supporting
>runtime suspend/resume feature and extra things.
>
>Change since v0:
>=================
>The DEFAULT_SRC_SIZE macro was defined using multiple lines,
>To make a simple define, tab and multiple lines has been removed,
>The macro is defined using one line.
>
>
>jackson.lee (5):
> wave5 : Support yuv422 input format for encoder.
> wave5: Support to prepend sps/pps to IDR frame.
> wave5 : Support runtime suspend/resume.
> wave5: Use the bitstream buffer size from host.
> wave5 : Fixed the wrong buffer size formula.
>
> .../platform/chips-media/wave5/wave5-hw.c | 11 +-
> .../chips-media/wave5/wave5-vpu-dec.c | 86 +++++-----
> .../chips-media/wave5/wave5-vpu-enc.c | 157 +++++++++++++++---
> .../platform/chips-media/wave5/wave5-vpu.c | 68 ++++++++
> .../platform/chips-media/wave5/wave5-vpuapi.c | 7 +
> .../platform/chips-media/wave5/wave5-vpuapi.h | 1 +
> .../media/platform/chips-media/wave5/wave5.h | 3 +
> 7 files changed, 253 insertions(+), 80 deletions(-)
>
>--
>2.43.0
>
>

2024-02-02 08:40:59

by jackson.lee

Subject: RE: [PATCH v1 0/5] wave5 codec driver

Hello Sebastian

Thanks for your advice.

> -----Original Message-----
> From: Sebastian Fricke <[email protected]>
> Sent: Friday, February 2, 2024 5:16 PM
> To: jackson.lee <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; lafley.kim <[email protected]>; b-
> [email protected]; [email protected]; Nas Chung <[email protected]>
> Subject: Re: [PATCH v1 0/5] wave5 codec driver
>
> Hey Jackson,
>
> thanks for sending the patches!
>
> I would advise to not send the next version of a patch series with
> multiple patches too quickly. So far the patches only got a single very
> minor review comment and others might already work on a deeper review and
> test. There is no hard rule but I would say give such a series at least a
> week or two to gather some review comments before sending the next version,
> as it is always a bit of hassle to adjust to a new version and you don't
> want to spam the mailing list.
> Should you not receive any more review comments, then please go ahead and
> ping some relevant people (Hans Verkuil, people from Collabora that
> developed the patches, etc.) to review, either per mail or per IRC on the
> OFTC server on the channel #linux-media.
>
> Greetings,
> Sebastian
>
> On 02.02.2024 16:03, jackson.lee wrote:
> >The wave5 codec driver is a stateful encoder/decoder.
> >The following patches is for supporting yuv422 inpuy format, supporting
> >runtime suspend/resume feature and extra things.
> >
> >Change since v0:
> >=================
> >The DEFAULT_SRC_SIZE macro was defined using multiple lines, To make a
> >simple define, tab and multiple lines has been removed, The macro is
> >defined using one line.
> >
> >
> >jackson.lee (5):
> > wave5 : Support yuv422 input format for encoder.
> > wave5: Support to prepend sps/pps to IDR frame.
> > wave5 : Support runtime suspend/resume.
> > wave5: Use the bitstream buffer size from host.
> > wave5 : Fixed the wrong buffer size formula.
> >
> > .../platform/chips-media/wave5/wave5-hw.c | 11 +-
> > .../chips-media/wave5/wave5-vpu-dec.c | 86 +++++-----
> > .../chips-media/wave5/wave5-vpu-enc.c | 157 +++++++++++++++---
> > .../platform/chips-media/wave5/wave5-vpu.c | 68 ++++++++
> > .../platform/chips-media/wave5/wave5-vpuapi.c | 7 +
> > .../platform/chips-media/wave5/wave5-vpuapi.h | 1 +
> > .../media/platform/chips-media/wave5/wave5.h | 3 +
> > 7 files changed, 253 insertions(+), 80 deletions(-)
> >
> >--
> >2.43.0
> >
> >