diff mbox series

[RESEND,v2,4/4] media: chips-media: wave5: Support YUV422 raw pixel-formats on the encoder.

Message ID 20240311105623.20406-5-jackson.lee@chipsnmedia.com (mailing list archive)
State Changes Requested
Delegated to: Sebastian Fricke
Headers
Series Add auto suspend/resume,YUV422 format,SPS/PPS generation for each IDR |

Commit Message

jackson.lee March 11, 2024, 10:56 a.m. UTC 
  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(-)

Comments

Nicolas Dufresne April 18, 2024, 9:05 p.m. UTC | #1 
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;
jackson.lee April 22, 2024, 4:30 a.m. UTC | #2 
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?

Thanks
Jackson.



> >  	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;
Nicolas Dufresne April 22, 2024, 3:49 p.m. UTC | #3 
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
jackson.lee April 24, 2024, 2:19 a.m. UTC | #4 
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
diff mbox series

Patch

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;
 
 	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;