From: Nicolas Dufresne <nicolas@ndufresne.ca>
To: "jackson.lee" <jackson.lee@chipsnmedia.com>,
mchehab@kernel.org, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, nas.chung@chipsnmedia.com
Cc: lafley.kim@chipsnmedia.com, b-brnich@ti.com
Subject: Re: [RESEND PATCH v0 5/5] wave5 : Fixed the wrong buffer size formula.
Date: Wed, 07 Feb 2024 13:31:59 -0500 [thread overview]
Message-ID: <90e82dd29a10fb196332e9228fa318febb5e035b.camel@ndufresne.ca> (raw)
In-Reply-To: <20240131013046.15687-6-jackson.lee@chipsnmedia.com>
Hi,
Le mercredi 31 janvier 2024 à 10:30 +0900, jackson.lee a écrit :
> 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.
This patch will go away as soon as you have ported to v4l2-common as requested
in patch 1/5. It will also make future addition of pixel formats less tedious.
regards,
Nicolas
>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> ---
> .../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 19018ace41b6..762973d0677b 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -152,46 +152,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;
> @@ -638,6 +638,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;
> }
> @@ -653,12 +655,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;
> }
> @@ -681,8 +688,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;
> }
> @@ -1229,8 +1238,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) {
> @@ -1806,6 +1815,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;
next prev parent reply other threads:[~2024-02-07 18:32 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-31 1:30 [RESEND PATCH v0 0/5] wave5 codec driver jackson.lee
2024-01-31 1:30 ` [RESEND PATCH v0 1/5] wave5 : Support yuv422 input format for encoder jackson.lee
2024-02-07 17:55 ` Nicolas Dufresne
2024-02-08 9:42 ` Sebastian Fricke
2024-02-16 6:48 ` jackson.lee
2024-01-31 1:30 ` [RESEND PATCH v0 2/5] wave5: Support to prepend sps/pps to IDR frame jackson.lee
2024-02-07 18:00 ` Nicolas Dufresne
2024-02-08 10:01 ` Sebastian Fricke
2024-02-16 7:12 ` jackson.lee
2024-01-31 1:30 ` [RESEND PATCH v0 3/5] wave5 : Support runtime suspend/resume jackson.lee
2024-02-07 18:29 ` Nicolas Dufresne
2024-02-19 4:04 ` jackson.lee
2024-02-19 21:20 ` Nicolas Dufresne
2024-02-20 5:12 ` jackson.lee
2024-02-20 19:06 ` Nicolas Dufresne
2024-02-21 2:27 ` jackson.lee
2024-02-21 15:26 ` Nicolas Dufresne
2024-02-22 1:36 ` jackson.lee
2024-02-22 3:57 ` jackson.lee
2024-02-22 20:55 ` Nicolas Dufresne
2024-02-23 1:06 ` jackson.lee
2024-02-23 15:16 ` Nicolas Dufresne
2024-02-23 18:05 ` Nicolas Dufresne
2024-02-23 18:09 ` Nicolas Dufresne
2024-02-26 0:56 ` jackson.lee
2024-02-26 1:33 ` jackson.lee
2024-03-04 14:12 ` Nicolas Dufresne
2024-03-05 0:54 ` jackson.lee
2024-02-22 20:42 ` Nicolas Dufresne
2024-01-31 1:30 ` [RESEND PATCH v0 4/5] wave5: Use the bitstream buffer size from host jackson.lee
2024-01-31 17:36 ` Andrew Davis
2024-02-02 1:55 ` jackson.lee
2024-01-31 1:30 ` [RESEND PATCH v0 5/5] wave5 : Fixed the wrong buffer size formula jackson.lee
2024-02-07 18:31 ` Nicolas Dufresne [this message]
2024-02-08 10:36 ` [RESEND PATCH v0 0/5] wave5 codec driver Sebastian Fricke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=90e82dd29a10fb196332e9228fa318febb5e035b.camel@ndufresne.ca \
--to=nicolas@ndufresne.ca \
--cc=b-brnich@ti.com \
--cc=jackson.lee@chipsnmedia.com \
--cc=lafley.kim@chipsnmedia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=nas.chung@chipsnmedia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®