| Message ID | 20240311105623.20406-1-jackson.lee@chipsnmedia.com |
|---|---|
| Headers | show |
| Series | Add auto suspend/resume,YUV422 format,SPS/PPS generation for each IDR | expand |
Hello Hans, Sebastian, and Nicolas Can you please review the following patch for the V2 ? https://www.spinics.net/lists/kernel/msg5136434.html thanks. Jackson > -----Original Message----- > From: jackson.lee > Sent: Monday, March 11, 2024 7:57 PM > To: mchehab@kernel.org; nicolas@ndufresne.ca; > sebastian.fricke@collabora.com > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim > <lafley.kim@chipsnmedia.com>; b-brnich@ti.com; jackson.lee > <jackson.lee@chipsnmedia.com> > Subject: [RESEND PATCH v2 0/4] Add auto suspend/resume,YUV422 > format,SPS/PPS generation for each IDR > > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > 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. > > v4l2-compliance results: > ======================== > > v4l2-compliance 1.24.1, 64 bits, 64-bit time_t > > Buffer ioctls: > warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > supported > warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > supported > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, > Warnings: 2 Total for wave5-enc device /dev/video1: 45, Succeeded: 45, > Failed: 0, Warnings: 0 > > Fluster test results: > ===================== > > Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-Gst1.0 > Using 1 parallel job(s) > Ran 132/147 tests successfully in 94.782 secs > > (1 test fails because of not supporting to parse multi frames, 1 test > fails because of a missing frame and slight corruption, 2 tests fail > because of sizes which are incompatible with the IP, 11 tests fail because > of unsupported 10 bit format) > > Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0 > Using 1 parallel job(s) > Ran 77/135 tests successfully in 37.364 secs > > (58 fail because the hardware is unable to decode MBAFF / FMO / Field / > Extended profile streams.) > > > Chnage since v1: > ================= > > * For [PATCH v2 0/4] media: chips-media: wave5: Support SPS/PPS generation > for each IDR > - define a macro for register addresses > > * For [PATCH v2 1/4] media: chips-media: wave5: Support runtime > suspend/resume > - add auto suspend/resume > > * For [PATCH v2 2/4] media: chips-media: wave5: Use helpers to calculate > bytesperline and sizeimage > - use helper functions to calculate bytesperline and sizeimage > > * For [PATCH v2 3/4] media: chips-media: wave5: Support YUV422 raw pixel- > formats on the encoder > - remove unnecessary codes > > 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 (4): > media: chips-media: wave5: Support SPS/PPS generation for each IDR > media: chips-media: wave5: Support runtime suspend/resume > media: chips-media: wave5: Use helpers to calculate bytesperline and > sizeimage. > media: chips-media: wave5: Support YUV422 raw pixel-formats on the > encoder. > > .../platform/chips-media/wave5/wave5-helper.c | 24 ++ > .../platform/chips-media/wave5/wave5-helper.h | 4 + > .../platform/chips-media/wave5/wave5-hw.c | 23 +- > .../chips-media/wave5/wave5-vpu-dec.c | 261 +++++------------- > .../chips-media/wave5/wave5-vpu-enc.c | 260 +++++++++-------- > .../platform/chips-media/wave5/wave5-vpu.c | 43 +++ > .../platform/chips-media/wave5/wave5-vpu.h | 4 - > .../platform/chips-media/wave5/wave5-vpuapi.c | 14 +- > .../platform/chips-media/wave5/wave5-vpuapi.h | 1 + > .../chips-media/wave5/wave5-vpuconfig.h | 25 +- > .../media/platform/chips-media/wave5/wave5.h | 3 + > 11 files changed, 329 insertions(+), 333 deletions(-) > > -- > 2.43.0
Hello Sebastian, Hans and Nicolas Could you please review the below patch series ? https://patchwork.linuxtv.org/project/linux-media/list/?series=12404 thanks Jackson > -----Original Message----- > From: jackson.lee <jackson.lee@chipsnmedia.com> > Sent: Monday, March 11, 2024 7:56 PM > To: mchehab@kernel.org; nicolas@ndufresne.ca; > sebastian.fricke@collabora.com > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim > <lafley.kim@chipsnmedia.com>; b-brnich@ti.com; jackson.lee > <jackson.lee@chipsnmedia.com> > Subject: [RESEND PATCH v2 0/4] Add auto suspend/resume,YUV422 > format,SPS/PPS generation for each IDR > > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > 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. > > v4l2-compliance results: > ======================== > > v4l2-compliance 1.24.1, 64 bits, 64-bit time_t > > Buffer ioctls: > warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > supported > warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > supported > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, > Warnings: 2 Total for wave5-enc device /dev/video1: 45, Succeeded: 45, > Failed: 0, Warnings: 0 > > Fluster test results: > ===================== > > Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-Gst1.0 > Using 1 parallel job(s) > Ran 132/147 tests successfully in 94.782 secs > > (1 test fails because of not supporting to parse multi frames, 1 test > fails because of a missing frame and slight corruption, 2 tests fail > because of sizes which are incompatible with the IP, 11 tests fail because > of unsupported 10 bit format) > > Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0 > Using 1 parallel job(s) > Ran 77/135 tests successfully in 37.364 secs > > (58 fail because the hardware is unable to decode MBAFF / FMO / Field / > Extended profile streams.) > > > Chnage since v1: > ================= > > * For [PATCH v2 0/4] media: chips-media: wave5: Support SPS/PPS generation > for each IDR > - define a macro for register addresses > > * For [PATCH v2 1/4] media: chips-media: wave5: Support runtime > suspend/resume > - add auto suspend/resume > > * For [PATCH v2 2/4] media: chips-media: wave5: Use helpers to calculate > bytesperline and sizeimage > - use helper functions to calculate bytesperline and sizeimage > > * For [PATCH v2 3/4] media: chips-media: wave5: Support YUV422 raw pixel- > formats on the encoder > - remove unnecessary codes > > 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 (4): > media: chips-media: wave5: Support SPS/PPS generation for each IDR > media: chips-media: wave5: Support runtime suspend/resume > media: chips-media: wave5: Use helpers to calculate bytesperline and > sizeimage. > media: chips-media: wave5: Support YUV422 raw pixel-formats on the > encoder. > > .../platform/chips-media/wave5/wave5-helper.c | 24 ++ > .../platform/chips-media/wave5/wave5-helper.h | 4 + > .../platform/chips-media/wave5/wave5-hw.c | 23 +- > .../chips-media/wave5/wave5-vpu-dec.c | 261 +++++------------- > .../chips-media/wave5/wave5-vpu-enc.c | 260 +++++++++-------- > .../platform/chips-media/wave5/wave5-vpu.c | 43 +++ > .../platform/chips-media/wave5/wave5-vpu.h | 4 - > .../platform/chips-media/wave5/wave5-vpuapi.c | 14 +- > .../platform/chips-media/wave5/wave5-vpuapi.h | 1 + > .../chips-media/wave5/wave5-vpuconfig.h | 25 +- > .../media/platform/chips-media/wave5/wave5.h | 3 + > 11 files changed, 329 insertions(+), 333 deletions(-) > > -- > 2.43.0
Hello Sebastian Thanks for your reply. > -----Original Message----- > From: sebastian.fricke@collabora.com <sebastian.fricke@collabora.com> > Sent: Thursday, April 4, 2024 4:47 PM > To: jackson.lee <jackson.lee@chipsnmedia.com> > Cc: mchehab@kernel.org; nicolas@ndufresne.ca; hverkuil@xs4all.nl; linux- > media@vger.kernel.org; linux-kernel@vger.kernel.org; Nas Chung > <nas.chung@chipsnmedia.com>; lafley.kim <lafley.kim@chipsnmedia.com>; b- > brnich@ti.com > Subject: Re: [RESEND PATCH v2 0/4] Add auto suspend/resume,YUV422 > format,SPS/PPS generation for each IDR > > Hey Jackson, > > On 25.03.2024 06:19, jackson.lee wrote: > >Hello Sebastian, Hans and Nicolas > > > >Could you please review the below patch series ? > > Sorry for the delay, me and Nicolas are currently quite busy. So, the > review of the patchset will be delayed a bit longer, current goal would be > review this until the 20th of April. I'll try to keep you updated. > > Greetings, > Sebastian > > > > > > >https://patchwork.linuxtv.org/project/linux-media/list/?series=12404 > > > > > >thanks > >Jackson > > > >> -----Original Message----- > >> From: jackson.lee <jackson.lee@chipsnmedia.com> > >> Sent: Monday, March 11, 2024 7:56 PM > >> To: mchehab@kernel.org; nicolas@ndufresne.ca; > >> sebastian.fricke@collabora.com > >> Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > >> hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim > >> <lafley.kim@chipsnmedia.com>; b-brnich@ti.com; jackson.lee > >> <jackson.lee@chipsnmedia.com> > >> Subject: [RESEND PATCH v2 0/4] Add auto suspend/resume,YUV422 > >> format,SPS/PPS generation for each IDR > >> > >> From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > >> > >> 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. > >> > >> v4l2-compliance results: > >> ======================== > >> > >> v4l2-compliance 1.24.1, 64 bits, 64-bit time_t > >> > >> Buffer ioctls: > >> warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > >> supported > >> warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not > >> supported > >> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > >> test VIDIOC_EXPBUF: OK > >> test Requests: OK (Not Supported) > >> > >> Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, > >> Warnings: 2 Total for wave5-enc device /dev/video1: 45, Succeeded: > >> 45, > >> Failed: 0, Warnings: 0 > >> > >> Fluster test results: > >> ===================== > >> > >> Running test suite JCT-VC-HEVC_V1 with decoder > >> GStreamer-H.265-V4L2-Gst1.0 Using 1 parallel job(s) > >> Ran 132/147 tests successfully in 94.782 secs > >> > >> (1 test fails because of not supporting to parse multi frames, 1 test > >> fails because of a missing frame and slight corruption, 2 tests fail > >> because of sizes which are incompatible with the IP, 11 tests fail > >> because of unsupported 10 bit format) > >> > >> Running test suite JVT-AVC_V1 with decoder > >> GStreamer-H.264-V4L2-Gst1.0 Using 1 parallel job(s) > >> Ran 77/135 tests successfully in 37.364 secs > >> > >> (58 fail because the hardware is unable to decode MBAFF / FMO / > >> Field / Extended profile streams.) > >> > >> > >> Chnage since v1: > >> ================= > >> > >> * For [PATCH v2 0/4] media: chips-media: wave5: Support SPS/PPS > >> generation for each IDR > >> - define a macro for register addresses > >> > >> * For [PATCH v2 1/4] media: chips-media: wave5: Support runtime > >> suspend/resume > >> - add auto suspend/resume > >> > >> * For [PATCH v2 2/4] media: chips-media: wave5: Use helpers to > >> calculate bytesperline and sizeimage > >> - use helper functions to calculate bytesperline and sizeimage > >> > >> * For [PATCH v2 3/4] media: chips-media: wave5: Support YUV422 raw > >> pixel- formats on the encoder > >> - remove unnecessary codes > >> > >> 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 (4): > >> media: chips-media: wave5: Support SPS/PPS generation for each IDR > >> media: chips-media: wave5: Support runtime suspend/resume > >> media: chips-media: wave5: Use helpers to calculate bytesperline and > >> sizeimage. > >> media: chips-media: wave5: Support YUV422 raw pixel-formats on the > >> encoder. > >> > >> .../platform/chips-media/wave5/wave5-helper.c | 24 ++ > >> .../platform/chips-media/wave5/wave5-helper.h | 4 + > >> .../platform/chips-media/wave5/wave5-hw.c | 23 +- > >> .../chips-media/wave5/wave5-vpu-dec.c | 261 +++++------------- > >> .../chips-media/wave5/wave5-vpu-enc.c | 260 +++++++++-------- > >> .../platform/chips-media/wave5/wave5-vpu.c | 43 +++ > >> .../platform/chips-media/wave5/wave5-vpu.h | 4 - > >> .../platform/chips-media/wave5/wave5-vpuapi.c | 14 +- > >> .../platform/chips-media/wave5/wave5-vpuapi.h | 1 + > >> .../chips-media/wave5/wave5-vpuconfig.h | 25 +- > >> .../media/platform/chips-media/wave5/wave5.h | 3 + > >> 11 files changed, 329 insertions(+), 333 deletions(-) > >> > >> -- > >> 2.43.0 > >
Le lundi 11 mars 2024 à 19:56 +0900, jackson.lee a écrit : > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > Provide a control to toggle (0 = off / 1 = on), whether the SPS and > PPS are generated for every IDR. > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > --- > .../platform/chips-media/wave5/wave5-hw.c | 19 +++++++++++++++---- > .../chips-media/wave5/wave5-vpu-enc.c | 7 +++++++ > .../platform/chips-media/wave5/wave5-vpuapi.h | 1 + > 3 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c > index f1e022fb148e..4a262822bf17 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c > @@ -23,6 +23,15 @@ > #define FEATURE_AVC_ENCODER BIT(1) > #define FEATURE_HEVC_ENCODER BIT(0) > > +#define ENC_AVC_INTRA_IDR_PARAM_MASK 0x7ff > +#define ENC_AVC_INTRA_PERIOD 6 > +#define ENC_AVC_IDR_PERIOD 17 > +#define ENC_AVC_FORCED_IDR_HEADER 28 > + > +#define ENC_HEVC_INTRA_QP 3 > +#define ENC_HEVC_FORCED_IDR_HEADER 9 > +#define ENC_HEVC_INTRA_PERIOD 16 > + Perhaps add the suffix _SHIFT to these macro ? I think it can easily be confused with a default value otherwise. nit: can happen later, since this is how things got merged in this driver, but typically, registers offsets, shift and masks are usually consolidated in a single reg header. It would be nice to consider cleaning this up in future patches. > /* Decoder support fields */ > #define FEATURE_AVC_DECODER BIT(3) > #define FEATURE_HEVC_DECODER BIT(2) > @@ -1601,12 +1610,14 @@ int wave5_vpu_enc_init_seq(struct vpu_instance *inst) > > if (inst->std == W_AVC_ENC) > vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_INTRA_PARAM, p_param->intra_qp | > - ((p_param->intra_period & 0x7ff) << 6) | > - ((p_param->avc_idr_period & 0x7ff) << 17)); > + ((p_param->intra_period & ENC_AVC_INTRA_IDR_PARAM_MASK) << ENC_AVC_INTRA_PERIOD) | > + ((p_param->avc_idr_period & ENC_AVC_INTRA_IDR_PARAM_MASK) << ENC_AVC_IDR_PERIOD) | > + (p_param->forced_idr_header_enable << ENC_AVC_FORCED_IDR_HEADER)); > else if (inst->std == W_HEVC_ENC) > vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_INTRA_PARAM, > - p_param->decoding_refresh_type | (p_param->intra_qp << 3) | > - (p_param->intra_period << 16)); > + p_param->decoding_refresh_type | (p_param->intra_qp << ENC_HEVC_INTRA_QP) | > + (p_param->forced_idr_header_enable << ENC_HEVC_FORCED_IDR_HEADER) | > + (p_param->intra_period << ENC_HEVC_INTRA_PERIOD)); > > reg_val = (p_param->rdo_skip << 2) | > (p_param->lambda_scaling_enable << 3) | > 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 f29cfa3af94a..f04baa93a9b7 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > @@ -1061,6 +1061,9 @@ static int wave5_vpu_enc_s_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE: > inst->enc_param.entropy_coding_mode = ctrl->val; > break; > + case V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR: > + inst->enc_param.forced_idr_header_enable = ctrl->val; > + break; > case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT: > break; > default: > @@ -1219,6 +1222,7 @@ static void wave5_set_enc_openparam(struct enc_open_param *open_param, > else > open_param->wave_param.intra_refresh_arg = num_ctu_row; > } > + open_param->wave_param.forced_idr_header_enable = input.forced_idr_header_enable; in the long term, there is one too many abstraction in this driver, we should remove that. > } > > static int initialize_sequence(struct vpu_instance *inst) > @@ -1702,6 +1706,9 @@ static int wave5_vpu_open_enc(struct file *filp) > 0, 1, 1, 0); > v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops, > V4L2_CID_MIN_BUFFERS_FOR_OUTPUT, 1, 32, 1, 1); > + v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops, > + V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR, > + 0, 1, 1, 0); > > if (v4l2_ctrl_hdl->error) { > ret = -ENODEV; > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h > index 352f6e904e50..3ad6118550ac 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h > @@ -566,6 +566,7 @@ struct enc_wave_param { > u32 lambda_scaling_enable: 1; /* enable lambda scaling using custom GOP */ > u32 transform8x8_enable: 1; /* enable 8x8 intra prediction and 8x8 transform */ > u32 mb_level_rc_enable: 1; /* enable MB-level rate control */ > + u32 forced_idr_header_enable: 1; /* enable header encoding before IDR frame */ > }; > > struct enc_open_param { With the suggested _SHIFT suffix (or equivalent) added. Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Le lundi 11 mars 2024 à 19:56 +0900, jackson.lee a écrit : > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > Add support for the YUV422P, NV16, NV61, YUV422M, NV16M, NV61M raw pixel-formats to the Wave5 encoder. > All these formats have a chroma subsampling ratio of 4:2:2 and therefore require a new image size calculation as the driver previously only handled a ratio of 4:2:0. > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > --- > .../chips-media/wave5/wave5-vpu-enc.c | 59 +++++++++++++++++-- > 1 file changed, 54 insertions(+), 5 deletions(-) > > 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 5a32bb138158..77657f63a169 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > @@ -39,6 +39,24 @@ static const struct vpu_format enc_fmt_list[FMT_TYPES][MAX_FMTS] = { > { > .v4l2_pix_fmt = V4L2_PIX_FMT_NV21M, > }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422P, > + }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16, > + }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61, > + }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422M, > + }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16M, > + }, > + { > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61M, > + }, > } > }; > > @@ -101,13 +119,30 @@ static int start_encode(struct vpu_instance *inst, u32 *fail_res) > struct vb2_v4l2_buffer *dst_buf; > struct frame_buffer frame_buf; > struct enc_param pic_param; > - u32 stride = ALIGN(inst->dst_fmt.width, 32); > - u32 luma_size = (stride * inst->dst_fmt.height); > - u32 chroma_size = ((stride / 2) * (inst->dst_fmt.height / 2)); > + u32 stride = inst->src_fmt.plane_fmt[0].bytesperline; > + u32 luma_size = (stride * inst->src_fmt.height); > + u32 chroma_size = 0; The helper introduced in previous patch also calculate sizeimage for each planes, so no need for this code anymore. > > memset(&pic_param, 0, sizeof(struct enc_param)); > memset(&frame_buf, 0, sizeof(struct frame_buffer)); > > + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420M) > + chroma_size = luma_size / 4; > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M) > + chroma_size = luma_size / 2; > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422M) > + chroma_size = luma_size / 2; > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) > + chroma_size = luma_size; > + > dst_buf = v4l2_m2m_next_dst_buf(m2m_ctx); > if (!dst_buf) { > dev_dbg(inst->dev->dev, "%s: No destination buffer found\n", __func__); > @@ -490,11 +525,15 @@ static int wave5_vpu_enc_s_fmt_out(struct file *file, void *fh, struct v4l2_form > } > > if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12 || > - inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M) { > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M) { > inst->cbcr_interleave = true; > inst->nv21 = false; > } else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21 || > - inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M) { > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) { > inst->cbcr_interleave = true; > inst->nv21 = true; > } else { > @@ -1086,6 +1125,16 @@ static void wave5_set_enc_openparam(struct enc_open_param *open_param, > u32 num_ctu_row = ALIGN(inst->dst_fmt.height, 64) / 64; > u32 num_mb_row = ALIGN(inst->dst_fmt.height, 16) / 16; > > + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M || > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) > + open_param->src_format = FORMAT_422; > + else > + open_param->src_format = FORMAT_420; > + > open_param->wave_param.gop_preset_idx = PRESET_IDX_IPP_SINGLE; > open_param->wave_param.hvs_qp_scale = 2; > open_param->wave_param.hvs_max_delta_qp = 10;
Hi Jackson, Le lundi 22 avril 2024 à 04:30 +0000, jackson.lee a écrit : > Hi Nicolas > > > > -----Original Message----- > > From: Nicolas Dufresne <nicolas@ndufresne.ca> > > Sent: Friday, April 19, 2024 6:06 AM > > To: jackson.lee <jackson.lee@chipsnmedia.com>; mchehab@kernel.org; > > sebastian.fricke@collabora.com > > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > > hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim > > <lafley.kim@chipsnmedia.com>; b-brnich@ti.com > > Subject: Re: [RESEND PATCH v2 4/4] media: chips-media: wave5: Support YUV422 > > raw pixel-formats on the encoder. > > > > Le lundi 11 mars 2024 à 19:56 +0900, jackson.lee a écrit : > > > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > > > > > Add support for the YUV422P, NV16, NV61, YUV422M, NV16M, NV61M raw pixel- > > formats to the Wave5 encoder. > > > All these formats have a chroma subsampling ratio of 4:2:2 and therefore > > require a new image size calculation as the driver previously only handled a > > ratio of 4:2:0. > > > > > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > > > --- > > > .../chips-media/wave5/wave5-vpu-enc.c | 59 +++++++++++++++++-- > > > 1 file changed, 54 insertions(+), 5 deletions(-) > > > > > > 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 5a32bb138158..77657f63a169 100644 > > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > @@ -39,6 +39,24 @@ static const struct vpu_format > > enc_fmt_list[FMT_TYPES][MAX_FMTS] = { > > > { > > > .v4l2_pix_fmt = V4L2_PIX_FMT_NV21M, > > > }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422P, > > > + }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16, > > > + }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61, > > > + }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422M, > > > + }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16M, > > > + }, > > > + { > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61M, > > > + }, > > > } > > > }; > > > > > > @@ -101,13 +119,30 @@ static int start_encode(struct vpu_instance *inst, > > u32 *fail_res) > > > struct vb2_v4l2_buffer *dst_buf; > > > struct frame_buffer frame_buf; > > > struct enc_param pic_param; > > > - u32 stride = ALIGN(inst->dst_fmt.width, 32); > > > - u32 luma_size = (stride * inst->dst_fmt.height); > > > - u32 chroma_size = ((stride / 2) * (inst->dst_fmt.height / 2)); > > > + u32 stride = inst->src_fmt.plane_fmt[0].bytesperline; > > > + u32 luma_size = (stride * inst->src_fmt.height); > > > + u32 chroma_size = 0; > > > > The helper introduced in previous patch also calculate sizeimage for each > > planes, so no need for this code anymore. > > Your comment means the below code? > > u32 luma_size = inst->src_fmt.plane_fmt[0].sizeimage > u32 chroma_size = inst->src_fmt.plane_fmt[1].sizeimage > > > > > > > > > > memset(&pic_param, 0, sizeof(struct enc_param)); > > > memset(&frame_buf, 0, sizeof(struct frame_buffer)); > > > > > The below code could be removed. > > > > + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420 || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420M) > > > + chroma_size = luma_size / 4; > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12 || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21 || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M) > > > + chroma_size = luma_size / 2; > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422M) > > > + chroma_size = luma_size / 2; > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M || > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) > > > + chroma_size = luma_size; > > > + > > Is That right? Yes, using the src_fmt seems accurate for the encoder. cheers, Nicolas
Hi Nicolas > -----Original Message----- > From: Nicolas Dufresne <nicolas@ndufresne.ca> > Sent: Tuesday, April 23, 2024 12:49 AM > To: jackson.lee <jackson.lee@chipsnmedia.com>; mchehab@kernel.org; > sebastian.fricke@collabora.com > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim > <lafley.kim@chipsnmedia.com>; b-brnich@ti.com > Subject: Re: [RESEND PATCH v2 4/4] media: chips-media: wave5: Support YUV422 > raw pixel-formats on the encoder. > > Hi Jackson, > > Le lundi 22 avril 2024 à 04:30 +0000, jackson.lee a écrit : > > Hi Nicolas > > > > > > > -----Original Message----- > > > From: Nicolas Dufresne <nicolas@ndufresne.ca> > > > Sent: Friday, April 19, 2024 6:06 AM > > > To: jackson.lee <jackson.lee@chipsnmedia.com>; mchehab@kernel.org; > > > sebastian.fricke@collabora.com > > > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > > > hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; > > > lafley.kim <lafley.kim@chipsnmedia.com>; b-brnich@ti.com > > > Subject: Re: [RESEND PATCH v2 4/4] media: chips-media: wave5: > > > Support YUV422 raw pixel-formats on the encoder. > > > > > > Le lundi 11 mars 2024 à 19:56 +0900, jackson.lee a écrit : > > > > From: "Jackson.lee" <jackson.lee@chipsnmedia.com> > > > > > > > > Add support for the YUV422P, NV16, NV61, YUV422M, NV16M, NV61M raw > > > > pixel- > > > formats to the Wave5 encoder. > > > > All these formats have a chroma subsampling ratio of 4:2:2 and > > > > therefore > > > require a new image size calculation as the driver previously only > > > handled a ratio of 4:2:0. > > > > > > > > Signed-off-by: Jackson.lee <jackson.lee@chipsnmedia.com> > > > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > > > > --- > > > > .../chips-media/wave5/wave5-vpu-enc.c | 59 +++++++++++++++++-- > > > > 1 file changed, 54 insertions(+), 5 deletions(-) > > > > > > > > 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 5a32bb138158..77657f63a169 100644 > > > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > > @@ -39,6 +39,24 @@ static const struct vpu_format > > > enc_fmt_list[FMT_TYPES][MAX_FMTS] = { > > > > { > > > > .v4l2_pix_fmt = V4L2_PIX_FMT_NV21M, > > > > }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422P, > > > > + }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16, > > > > + }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61, > > > > + }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422M, > > > > + }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16M, > > > > + }, > > > > + { > > > > + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61M, > > > > + }, > > > > } > > > > }; > > > > > > > > @@ -101,13 +119,30 @@ static int start_encode(struct vpu_instance > > > > *inst, > > > u32 *fail_res) > > > > struct vb2_v4l2_buffer *dst_buf; > > > > struct frame_buffer frame_buf; > > > > struct enc_param pic_param; > > > > - u32 stride = ALIGN(inst->dst_fmt.width, 32); > > > > - u32 luma_size = (stride * inst->dst_fmt.height); > > > > - u32 chroma_size = ((stride / 2) * (inst->dst_fmt.height / 2)); > > > > + u32 stride = inst->src_fmt.plane_fmt[0].bytesperline; > > > > + u32 luma_size = (stride * inst->src_fmt.height); I reviewed the above code, again. The code would be proper. Because in case of NV12 color space, it has one planar, therefor the YUV value is in the same linear memory. To calculate luma_size, we need the above code. > > > > + u32 chroma_size = 0; > > > > > > The helper introduced in previous patch also calculate sizeimage for > > > each planes, so no need for this code anymore. > > > > Your comment means the below code? > > > > u32 luma_size = inst->src_fmt.plane_fmt[0].sizeimage > > u32 chroma_size = inst->src_fmt.plane_fmt[1].sizeimage > > > > > > > > > > > > > > > memset(&pic_param, 0, sizeof(struct enc_param)); > > > > memset(&frame_buf, 0, sizeof(struct frame_buffer)); > > > > > > > > The below code could be removed. > > > > > > + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420 || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420M) > > > > + chroma_size = luma_size / 4; > > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12 || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21 || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M) > > > > + chroma_size = luma_size / 2; > > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422M) > > > > + chroma_size = luma_size / 2; > > > > + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M || > > > > + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) > > > > + chroma_size = luma_size; > > > > + > > > > Is That right? > > Yes, using the src_fmt seems accurate for the encoder. > > cheers, > Nicolas
From: "Jackson.lee" <jackson.lee@chipsnmedia.com> 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. v4l2-compliance results: ======================== v4l2-compliance 1.24.1, 64 bits, 64-bit time_t Buffer ioctls: warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not supported warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 2 Total for wave5-enc device /dev/video1: 45, Succeeded: 45, Failed: 0, Warnings: 0 Fluster test results: ===================== Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-Gst1.0 Using 1 parallel job(s) Ran 132/147 tests successfully in 94.782 secs (1 test fails because of not supporting to parse multi frames, 1 test fails because of a missing frame and slight corruption, 2 tests fail because of sizes which are incompatible with the IP, 11 tests fail because of unsupported 10 bit format) Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0 Using 1 parallel job(s) Ran 77/135 tests successfully in 37.364 secs (58 fail because the hardware is unable to decode MBAFF / FMO / Field / Extended profile streams.) Chnage since v1: ================= * For [PATCH v2 0/4] media: chips-media: wave5: Support SPS/PPS generation for each IDR - define a macro for register addresses * For [PATCH v2 1/4] media: chips-media: wave5: Support runtime suspend/resume - add auto suspend/resume * For [PATCH v2 2/4] media: chips-media: wave5: Use helpers to calculate bytesperline and sizeimage - use helper functions to calculate bytesperline and sizeimage * For [PATCH v2 3/4] media: chips-media: wave5: Support YUV422 raw pixel-formats on the encoder - remove unnecessary codes 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 (4): media: chips-media: wave5: Support SPS/PPS generation for each IDR media: chips-media: wave5: Support runtime suspend/resume media: chips-media: wave5: Use helpers to calculate bytesperline and sizeimage. media: chips-media: wave5: Support YUV422 raw pixel-formats on the encoder. .../platform/chips-media/wave5/wave5-helper.c | 24 ++ .../platform/chips-media/wave5/wave5-helper.h | 4 + .../platform/chips-media/wave5/wave5-hw.c | 23 +- .../chips-media/wave5/wave5-vpu-dec.c | 261 +++++------------- .../chips-media/wave5/wave5-vpu-enc.c | 260 +++++++++-------- .../platform/chips-media/wave5/wave5-vpu.c | 43 +++ .../platform/chips-media/wave5/wave5-vpu.h | 4 - .../platform/chips-media/wave5/wave5-vpuapi.c | 14 +- .../platform/chips-media/wave5/wave5-vpuapi.h | 1 + .../chips-media/wave5/wave5-vpuconfig.h | 25 +- .../media/platform/chips-media/wave5/wave5.h | 3 + 11 files changed, 329 insertions(+), 333 deletions(-)