Re: [RESEND PATCH v2 3/4] media: chips-media: wave5: Use helpers to calculate bytesperline and sizeimage.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

very nice cleanup.

Le lundi 11 mars 2024 à 19:56 +0900, jackson.lee a écrit :
> From: "Jackson.lee" <jackson.lee@xxxxxxxxxxxxxxx>
> 
> Use v4l2-common helper functions to calculate bytesperline and sizeimage, instead of calculating in a wave5 driver directly.
> In case of raw(YUV) v4l2_pix_format, the wave5 driver updates v4l2_pix_format_mplane struct through v4l2_fill_pixfmt_mp() function.
> Encoder and Decoder need same bytesperline and sizeimage values for same v4l2_pix_format.
> So, a wave5_update_pix_fmt is refactored to support both together.
> 
> Signed-off-by: Jackson.lee <jackson.lee@xxxxxxxxxxxxxxx>
> Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
> ---
>  .../platform/chips-media/wave5/wave5-helper.c |  24 ++
>  .../platform/chips-media/wave5/wave5-helper.h |   4 +
>  .../chips-media/wave5/wave5-vpu-dec.c         | 245 ++++--------------
>  .../chips-media/wave5/wave5-vpu-enc.c         | 179 +++++--------
>  .../platform/chips-media/wave5/wave5-vpu.h    |   4 -
>  .../chips-media/wave5/wave5-vpuconfig.h       |  25 +-
>  6 files changed, 164 insertions(+), 317 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> index 8433ecab230c..53cad4d17aa7 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> @@ -7,6 +7,8 @@
>  
>  #include "wave5-helper.h"
>  
> +#define DEFAULT_BS_SIZE(width, height) ((width) * (height) / 8 * 3)
> +
>  const char *state_to_str(enum vpu_instance_state state)
>  {
>  	switch (state) {
> @@ -211,3 +213,25 @@ void wave5_return_bufs(struct vb2_queue *q, u32 state)
>  		v4l2_m2m_buf_done(vbuf, state);
>  	}
>  }
> +
> +void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp,
> +			  unsigned int width,
> +			  unsigned int height,
> +			  const struct v4l2_frmsize_stepwise frmsize)
> +{
> +	v4l2_apply_frmsize_constraints(&width, &height, &frmsize);
> +
> +	if (pix_mp->pixelformat == V4L2_PIX_FMT_HEVC ||
> +	    pix_mp->pixelformat == V4L2_PIX_FMT_H264) {

I notice the caller already index the frmsize between RAW and CODEC. I'd say
pass the information as an argument so we don't have to update code later when
more codecs are added.

> +		pix_mp->width = width;
> +		pix_mp->height = height;
> +		pix_mp->num_planes = 1;
> +		pix_mp->plane_fmt[0].bytesperline = 0;
> +		pix_mp->plane_fmt[0].sizeimage = max(DEFAULT_BS_SIZE(width, height),
> +						     pix_mp->plane_fmt[0].sizeimage);
> +	} else {
> +		v4l2_fill_pixfmt_mp(pix_mp, pix_mp->pixelformat, width, height);
> +	}
> +	pix_mp->flags = 0;
> +	pix_mp->field = V4L2_FIELD_NONE;
> +}
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.h b/drivers/media/platform/chips-media/wave5/wave5-helper.h
> index 6cee1c14d3ce..a8ecd1920207 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-helper.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.h
> @@ -28,4 +28,8 @@ const struct vpu_format *wave5_find_vpu_fmt_by_idx(unsigned int idx,
>  						   const struct vpu_format fmt_list[MAX_FMTS]);
>  enum wave_std wave5_to_vpu_std(unsigned int v4l2_pix_fmt, enum vpu_instance_type type);
>  void wave5_return_bufs(struct vb2_queue *q, u32 state);
> +void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp,
> +			  unsigned int width,
> +			  unsigned int height,
> +			  const struct v4l2_frmsize_stepwise frmsize);
>  #endif
> 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 a199877c643b..43d0a21193dd 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -11,115 +11,74 @@
>  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
>  #define VPU_DEC_DRV_NAME "wave5-dec"
>  
> -#define DEFAULT_SRC_SIZE(width, height) ({			\
> -	(width) * (height) / 8 * 3;					\
> -})
> -
>  static const struct vpu_format dec_fmt_list[FMT_TYPES][MAX_FMTS] = {
>  	[VPU_FMT_TYPE_CODEC] = {
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_HEVC,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_H264,
> -			.max_width = 8192,
> -			.min_width = 32,
> -			.max_height = 4320,
> -			.min_height = 32,
>  		},

I notice that min/max was different per codec, but in this patch you actually
make them both identical. Is that an error, otherwise the change should be
underlined in the commit message.

If you really need per codec granularity, you could define the common frmsize
array before, and set a pointer here into the static array. This way you won't
have to type the same thing over and over, but still have per codec granularity.

>  	},
>  	[VPU_FMT_TYPE_RAW] = {
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV420,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV12,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV21,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV422P,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV16,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV61,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV420M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV12M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV21M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV422M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV16M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV61M,
> -			.max_width = 8192,
> -			.min_width = 8,
> -			.max_height = 4320,
> -			.min_height = 8,
>  		},
>  	}
>  };
>  
> +static const struct v4l2_frmsize_stepwise dec_frmsize[FMT_TYPES] = {
> +	[VPU_FMT_TYPE_CODEC] = {
> +		.min_width = W5_MIN_DEC_PIC_WIDTH,
> +		.max_width = W5_MAX_DEC_PIC_WIDTH,
> +		.step_width = W5_DEC_CODEC_STEP_WIDTH,
> +		.min_height = W5_MIN_DEC_PIC_HEIGHT,
> +		.max_height = W5_MAX_DEC_PIC_HEIGHT,
> +		.step_height = W5_DEC_CODEC_STEP_HEIGHT,
> +	},
> +	[VPU_FMT_TYPE_RAW] = {
> +		.min_width = W5_MIN_DEC_PIC_WIDTH,
> +		.max_width = W5_MAX_DEC_PIC_WIDTH,
> +		.step_width = W5_DEC_RAW_STEP_WIDTH,
> +		.min_height = W5_MIN_DEC_PIC_HEIGHT,
> +		.max_height = W5_MAX_DEC_PIC_HEIGHT,
> +		.step_height = W5_DEC_RAW_STEP_HEIGHT,
> +	},
> +};
> +
>  /*
>   * Make sure that the state switch is allowed and add logging for debugging
>   * purposes
> @@ -234,74 +193,6 @@ static void wave5_handle_src_buffer(struct vpu_instance *inst, dma_addr_t rd_ptr
>  	inst->remaining_consumed_bytes = consumed_bytes;
>  }
>  
> -static void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp, unsigned int width,
> -				 unsigned int height)
> -{
> -	switch (pix_mp->pixelformat) {
> -	case V4L2_PIX_FMT_YUV420:
> -	case V4L2_PIX_FMT_NV12:
> -	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;
> -		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;
> -		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;
> -		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;
> -		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;
> -		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;
> -		break;
> -	default:
> -		pix_mp->width = width;
> -		pix_mp->height = height;
> -		pix_mp->plane_fmt[0].bytesperline = 0;
> -		pix_mp->plane_fmt[0].sizeimage = max(DEFAULT_SRC_SIZE(width, height),
> -						     pix_mp->plane_fmt[0].sizeimage);
> -		break;
> -	}
> -}
> -
>  static int start_decode(struct vpu_instance *inst, u32 *fail_res)
>  {
>  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> @@ -397,9 +288,11 @@ static int handle_dynamic_resolution_change(struct vpu_instance *inst)
>  			initial_info->pic_crop_rect.top - initial_info->pic_crop_rect.bottom;
>  
>  		wave5_update_pix_fmt(&inst->src_fmt, initial_info->pic_width,
> -				     initial_info->pic_height);
> +						     initial_info->pic_height,
> +						     dec_frmsize[VPU_FMT_TYPE_CODEC]);
>  		wave5_update_pix_fmt(&inst->dst_fmt, initial_info->pic_width,
> -				     initial_info->pic_height);
> +						     initial_info->pic_height,
> +						     dec_frmsize[VPU_FMT_TYPE_RAW]);
>  	}
>  
>  	v4l2_event_queue_fh(fh, &vpu_event_src_ch);
> @@ -548,12 +441,7 @@ static int wave5_vpu_dec_enum_framesizes(struct file *f, void *fh, struct v4l2_f
>  	}
>  
>  	fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
> -	fsize->stepwise.min_width = vpu_fmt->min_width;
> -	fsize->stepwise.max_width = vpu_fmt->max_width;
> -	fsize->stepwise.step_width = 1;
> -	fsize->stepwise.min_height = vpu_fmt->min_height;
> -	fsize->stepwise.max_height = vpu_fmt->max_height;
> -	fsize->stepwise.step_height = 1;
> +	fsize->stepwise = dec_frmsize[VPU_FMT_TYPE_CODEC];
>  
>  	return 0;
>  }
> @@ -589,14 +477,10 @@ static int wave5_vpu_dec_try_fmt_cap(struct file *file, void *fh, struct v4l2_fo
>  		width = inst->dst_fmt.width;
>  		height = inst->dst_fmt.height;
>  		f->fmt.pix_mp.pixelformat = inst->dst_fmt.pixelformat;
> -		f->fmt.pix_mp.num_planes = inst->dst_fmt.num_planes;
>  	} else {
> -		const struct v4l2_format_info *info = v4l2_format_info(vpu_fmt->v4l2_pix_fmt);
> -
> -		width = clamp(f->fmt.pix_mp.width, vpu_fmt->min_width, vpu_fmt->max_width);
> -		height = clamp(f->fmt.pix_mp.height, vpu_fmt->min_height, vpu_fmt->max_height);
> +		width = f->fmt.pix_mp.width;
> +		height = f->fmt.pix_mp.height;
>  		f->fmt.pix_mp.pixelformat = vpu_fmt->v4l2_pix_fmt;
> -		f->fmt.pix_mp.num_planes = info->mem_planes;
>  	}
>  
>  	if (p_dec_info->initial_info_obtained) {
> @@ -604,9 +488,9 @@ static int wave5_vpu_dec_try_fmt_cap(struct file *file, void *fh, struct v4l2_fo
>  		height = inst->dst_fmt.height;
>  	}
>  
> -	wave5_update_pix_fmt(&f->fmt.pix_mp, width, height);
> -	f->fmt.pix_mp.flags = 0;
> -	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> +	wave5_update_pix_fmt(&f->fmt.pix_mp, width,
> +					     height,
> +					     dec_frmsize[VPU_FMT_TYPE_RAW]);
>  	f->fmt.pix_mp.colorspace = inst->colorspace;
>  	f->fmt.pix_mp.ycbcr_enc = inst->ycbcr_enc;
>  	f->fmt.pix_mp.quantization = inst->quantization;
> @@ -719,6 +603,7 @@ static int wave5_vpu_dec_try_fmt_out(struct file *file, void *fh, struct v4l2_fo
>  {
>  	struct vpu_instance *inst = wave5_to_vpu_inst(fh);
>  	const struct vpu_format *vpu_fmt;
> +	int width, height;
>  
>  	dev_dbg(inst->dev->dev,
>  		"%s: fourcc: %u width: %u height: %u num_planes: %u colorspace: %u field: %u\n",
> @@ -727,20 +612,18 @@ static int wave5_vpu_dec_try_fmt_out(struct file *file, void *fh, struct v4l2_fo
>  
>  	vpu_fmt = wave5_find_vpu_fmt(f->fmt.pix_mp.pixelformat, dec_fmt_list[VPU_FMT_TYPE_CODEC]);
>  	if (!vpu_fmt) {
> +		width = inst->src_fmt.width;
> +		height = inst->src_fmt.height;
>  		f->fmt.pix_mp.pixelformat = inst->src_fmt.pixelformat;
> -		f->fmt.pix_mp.num_planes = inst->src_fmt.num_planes;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, inst->src_fmt.width, inst->src_fmt.height);
>  	} else {
> -		int width = clamp(f->fmt.pix_mp.width, vpu_fmt->min_width, vpu_fmt->max_width);
> -		int height = clamp(f->fmt.pix_mp.height, vpu_fmt->min_height, vpu_fmt->max_height);
> -
> +		width = f->fmt.pix_mp.width;
> +		height = f->fmt.pix_mp.height;
>  		f->fmt.pix_mp.pixelformat = vpu_fmt->v4l2_pix_fmt;
> -		f->fmt.pix_mp.num_planes = 1;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, width, height);
>  	}
>  
> -	f->fmt.pix_mp.flags = 0;
> -	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> +	wave5_update_pix_fmt(&f->fmt.pix_mp, width,
> +					     height,
> +					     dec_frmsize[VPU_FMT_TYPE_CODEC]);
>  
>  	return 0;
>  }
> @@ -782,7 +665,9 @@ static int wave5_vpu_dec_s_fmt_out(struct file *file, void *fh, struct v4l2_form
>  	inst->quantization = f->fmt.pix_mp.quantization;
>  	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);
> +	wave5_update_pix_fmt(&inst->dst_fmt, f->fmt.pix_mp.width,
> +					     f->fmt.pix_mp.height,
> +					     dec_frmsize[VPU_FMT_TYPE_RAW]);
>  
>  	return 0;
>  }
> @@ -1005,6 +890,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);
> @@ -1018,31 +904,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]);
>  		}
>  	}
>  
> @@ -1564,20 +1428,15 @@ static const struct vb2_ops wave5_vpu_dec_vb2_ops = {
>  static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
>  				     struct v4l2_pix_format_mplane *dst_fmt)
>  {
> -	unsigned int dst_pix_fmt = dec_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
> -	const struct v4l2_format_info *dst_fmt_info = v4l2_format_info(dst_pix_fmt);
> -
>  	src_fmt->pixelformat = dec_fmt_list[VPU_FMT_TYPE_CODEC][0].v4l2_pix_fmt;
> -	src_fmt->field = V4L2_FIELD_NONE;
> -	src_fmt->flags = 0;
> -	src_fmt->num_planes = 1;
> -	wave5_update_pix_fmt(src_fmt, 720, 480);
> -
> -	dst_fmt->pixelformat = dst_pix_fmt;
> -	dst_fmt->field = V4L2_FIELD_NONE;
> -	dst_fmt->flags = 0;
> -	dst_fmt->num_planes = dst_fmt_info->mem_planes;
> -	wave5_update_pix_fmt(dst_fmt, 736, 480);
> +	wave5_update_pix_fmt(src_fmt, W5_DEF_DEC_PIC_WIDTH,
> +				      W5_DEF_DEC_PIC_HEIGHT,
> +				      dec_frmsize[VPU_FMT_TYPE_CODEC]);
> +
> +	dst_fmt->pixelformat = dec_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
> +	wave5_update_pix_fmt(dst_fmt, W5_DEF_DEC_PIC_WIDTH,
> +				      W5_DEF_DEC_PIC_HEIGHT,
> +				      dec_frmsize[VPU_FMT_TYPE_RAW]);
>  }
>  
>  static int wave5_vpu_dec_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> 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 013e2bb37fbb..5a32bb138158 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -15,65 +15,52 @@ static const struct vpu_format enc_fmt_list[FMT_TYPES][MAX_FMTS] = {
>  	[VPU_FMT_TYPE_CODEC] = {
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_HEVC,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_H264,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  	},
>  	[VPU_FMT_TYPE_RAW] = {
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV420,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV12,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV21,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_YUV420M,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV12M,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  		{
>  			.v4l2_pix_fmt = V4L2_PIX_FMT_NV21M,
> -			.max_width = W5_MAX_ENC_PIC_WIDTH,
> -			.min_width = W5_MIN_ENC_PIC_WIDTH,
> -			.max_height = W5_MAX_ENC_PIC_HEIGHT,
> -			.min_height = W5_MIN_ENC_PIC_HEIGHT,
>  		},
>  	}
>  };
>  
> +static const struct v4l2_frmsize_stepwise enc_frmsize[FMT_TYPES] = {
> +	[VPU_FMT_TYPE_CODEC] = {
> +		.min_width = W5_MIN_ENC_PIC_WIDTH,
> +		.max_width = W5_MAX_ENC_PIC_WIDTH,
> +		.step_width = W5_ENC_CODEC_STEP_WIDTH,
> +		.min_height = W5_MIN_ENC_PIC_HEIGHT,
> +		.max_height = W5_MAX_ENC_PIC_HEIGHT,
> +		.step_height = W5_ENC_CODEC_STEP_HEIGHT,
> +	},
> +	[VPU_FMT_TYPE_RAW] = {
> +		.min_width = W5_MIN_ENC_PIC_WIDTH,
> +		.max_width = W5_MAX_ENC_PIC_WIDTH,
> +		.step_width = W5_ENC_RAW_STEP_WIDTH,
> +		.min_height = W5_MIN_ENC_PIC_HEIGHT,
> +		.max_height = W5_MAX_ENC_PIC_HEIGHT,
> +		.step_height = W5_ENC_RAW_STEP_HEIGHT,
> +	},
> +};
> +
>  static int switch_state(struct vpu_instance *inst, enum vpu_instance_state state)
>  {
>  	switch (state) {
> @@ -106,46 +93,6 @@ static int switch_state(struct vpu_instance *inst, enum vpu_instance_state state
>  	return -EINVAL;
>  }
>  
> -static void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp, unsigned int width,
> -				 unsigned int height)
> -{
> -	switch (pix_mp->pixelformat) {
> -	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;
> -		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;
> -		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;
> -		break;
> -	default:
> -		pix_mp->width = width;
> -		pix_mp->height = height;
> -		pix_mp->plane_fmt[0].bytesperline = 0;
> -		pix_mp->plane_fmt[0].sizeimage = width * height / 8 * 3;
> -		break;
> -	}
> -}
> -
>  static int start_encode(struct vpu_instance *inst, u32 *fail_res)
>  {
>  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> @@ -360,13 +307,8 @@ static int wave5_vpu_enc_enum_framesizes(struct file *f, void *fh, struct v4l2_f
>  			return -EINVAL;
>  	}
>  
> -	fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
> -	fsize->stepwise.min_width = vpu_fmt->min_width;
> -	fsize->stepwise.max_width = vpu_fmt->max_width;
> -	fsize->stepwise.step_width = 1;
> -	fsize->stepwise.min_height = vpu_fmt->min_height;
> -	fsize->stepwise.max_height = vpu_fmt->max_height;
> -	fsize->stepwise.step_height = 1;
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise = enc_frmsize[VPU_FMT_TYPE_CODEC];
>  
>  	return 0;
>  }
> @@ -392,6 +334,7 @@ static int wave5_vpu_enc_try_fmt_cap(struct file *file, void *fh, struct v4l2_fo
>  {
>  	struct vpu_instance *inst = wave5_to_vpu_inst(fh);
>  	const struct vpu_format *vpu_fmt;
> +	int width, height;
>  
>  	dev_dbg(inst->dev->dev, "%s: fourcc: %u width: %u height: %u num_planes: %u field: %u\n",
>  		__func__, f->fmt.pix_mp.pixelformat, f->fmt.pix_mp.width, f->fmt.pix_mp.height,
> @@ -399,20 +342,18 @@ static int wave5_vpu_enc_try_fmt_cap(struct file *file, void *fh, struct v4l2_fo
>  
>  	vpu_fmt = wave5_find_vpu_fmt(f->fmt.pix_mp.pixelformat, enc_fmt_list[VPU_FMT_TYPE_CODEC]);
>  	if (!vpu_fmt) {
> +		width = inst->dst_fmt.width;
> +		height = inst->dst_fmt.height;
>  		f->fmt.pix_mp.pixelformat = inst->dst_fmt.pixelformat;
> -		f->fmt.pix_mp.num_planes = inst->dst_fmt.num_planes;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, inst->dst_fmt.width, inst->dst_fmt.height);
>  	} else {
> -		int width = clamp(f->fmt.pix_mp.width, vpu_fmt->min_width, vpu_fmt->max_width);
> -		int height = clamp(f->fmt.pix_mp.height, vpu_fmt->min_height, vpu_fmt->max_height);
> -
> +		width = f->fmt.pix_mp.width;
> +		height = f->fmt.pix_mp.height;
>  		f->fmt.pix_mp.pixelformat = vpu_fmt->v4l2_pix_fmt;
> -		f->fmt.pix_mp.num_planes = 1;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, width, height);
>  	}
>  
> -	f->fmt.pix_mp.flags = 0;
> -	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> +	wave5_update_pix_fmt(&f->fmt.pix_mp, width,
> +					     height,
> +					     enc_frmsize[VPU_FMT_TYPE_CODEC]);
>  	f->fmt.pix_mp.colorspace = inst->colorspace;
>  	f->fmt.pix_mp.ycbcr_enc = inst->ycbcr_enc;
>  	f->fmt.pix_mp.quantization = inst->quantization;
> @@ -500,6 +441,7 @@ static int wave5_vpu_enc_try_fmt_out(struct file *file, void *fh, struct v4l2_fo
>  {
>  	struct vpu_instance *inst = wave5_to_vpu_inst(fh);
>  	const struct vpu_format *vpu_fmt;
> +	int width, height;
>  
>  	dev_dbg(inst->dev->dev, "%s: fourcc: %u width: %u height: %u num_planes: %u field: %u\n",
>  		__func__, f->fmt.pix_mp.pixelformat, f->fmt.pix_mp.width, f->fmt.pix_mp.height,
> @@ -507,21 +449,18 @@ static int wave5_vpu_enc_try_fmt_out(struct file *file, void *fh, struct v4l2_fo
>  
>  	vpu_fmt = wave5_find_vpu_fmt(f->fmt.pix_mp.pixelformat, enc_fmt_list[VPU_FMT_TYPE_RAW]);
>  	if (!vpu_fmt) {
> +		width = inst->src_fmt.width;
> +		height = inst->src_fmt.height;
>  		f->fmt.pix_mp.pixelformat = inst->src_fmt.pixelformat;
> -		f->fmt.pix_mp.num_planes = inst->src_fmt.num_planes;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, inst->src_fmt.width, inst->src_fmt.height);
>  	} else {
> -		int width = clamp(f->fmt.pix_mp.width, vpu_fmt->min_width, vpu_fmt->max_width);
> -		int height = clamp(f->fmt.pix_mp.height, vpu_fmt->min_height, vpu_fmt->max_height);
> -		const struct v4l2_format_info *info = v4l2_format_info(vpu_fmt->v4l2_pix_fmt);
> -
> +		width = f->fmt.pix_mp.width;
> +		height = f->fmt.pix_mp.height;
>  		f->fmt.pix_mp.pixelformat = vpu_fmt->v4l2_pix_fmt;
> -		f->fmt.pix_mp.num_planes = info->mem_planes;
> -		wave5_update_pix_fmt(&f->fmt.pix_mp, width, height);
>  	}
>  
> -	f->fmt.pix_mp.flags = 0;
> -	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> +	wave5_update_pix_fmt(&f->fmt.pix_mp, width,
> +					     height,
> +					     enc_frmsize[VPU_FMT_TYPE_RAW]);
>  
>  	return 0;
>  }
> @@ -568,7 +507,11 @@ static int wave5_vpu_enc_s_fmt_out(struct file *file, void *fh, struct v4l2_form
>  	inst->quantization = f->fmt.pix_mp.quantization;
>  	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);
> +	wave5_update_pix_fmt(&inst->dst_fmt, f->fmt.pix_mp.width,
> +					     f->fmt.pix_mp.height,
> +					     enc_frmsize[VPU_FMT_TYPE_CODEC]);
> +	inst->conf_win.width = inst->dst_fmt.width;
> +	inst->conf_win.height = inst->dst_fmt.height;
>  
>  	return 0;
>  }
> @@ -584,12 +527,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;
>  	}
> @@ -612,8 +560,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;
>  }
> @@ -1151,8 +1101,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) {
> @@ -1456,20 +1406,15 @@ static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
>  static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
>  				     struct v4l2_pix_format_mplane *dst_fmt)
>  {
> -	unsigned int src_pix_fmt = enc_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
> -	const struct v4l2_format_info *src_fmt_info = v4l2_format_info(src_pix_fmt);
> -
> -	src_fmt->pixelformat = src_pix_fmt;
> -	src_fmt->field = V4L2_FIELD_NONE;
> -	src_fmt->flags = 0;
> -	src_fmt->num_planes = src_fmt_info->mem_planes;
> -	wave5_update_pix_fmt(src_fmt, 416, 240);
> +	src_fmt->pixelformat = enc_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
> +	wave5_update_pix_fmt(src_fmt, W5_DEF_ENC_PIC_WIDTH,
> +				      W5_DEF_ENC_PIC_HEIGHT,
> +				      enc_frmsize[VPU_FMT_TYPE_RAW]);
>  
>  	dst_fmt->pixelformat = enc_fmt_list[VPU_FMT_TYPE_CODEC][0].v4l2_pix_fmt;
> -	dst_fmt->field = V4L2_FIELD_NONE;
> -	dst_fmt->flags = 0;
> -	dst_fmt->num_planes = 1;
> -	wave5_update_pix_fmt(dst_fmt, 416, 240);
> +	wave5_update_pix_fmt(dst_fmt, W5_DEF_ENC_PIC_WIDTH,
> +				      W5_DEF_ENC_PIC_HEIGHT,
> +				      enc_frmsize[VPU_FMT_TYPE_CODEC]);
>  }
>  
>  static int wave5_vpu_enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> @@ -1734,6 +1679,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;
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.h b/drivers/media/platform/chips-media/wave5/wave5-vpu.h
> index 32b7fd3730b5..691d6341fcda 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.h
> @@ -38,10 +38,6 @@ enum vpu_fmt_type {
>  
>  struct vpu_format {
>  	unsigned int v4l2_pix_fmt;
> -	unsigned int max_width;
> -	unsigned int min_width;
> -	unsigned int max_height;
> -	unsigned int min_height;
>  };
>  
>  static inline struct vpu_instance *wave5_to_vpu_inst(struct v4l2_fh *vfh)
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> index d9751eedb0f9..e6a34ae7084e 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> @@ -30,10 +30,27 @@
>  
>  #define MAX_NUM_INSTANCE                32
>  
> -#define W5_MIN_ENC_PIC_WIDTH            256
> -#define W5_MIN_ENC_PIC_HEIGHT           128
> -#define W5_MAX_ENC_PIC_WIDTH            8192
> -#define W5_MAX_ENC_PIC_HEIGHT           8192
> +#define W5_DEF_DEC_PIC_WIDTH            720U
> +#define W5_DEF_DEC_PIC_HEIGHT           480U
> +#define W5_MIN_DEC_PIC_WIDTH            32U
> +#define W5_MIN_DEC_PIC_HEIGHT           32U
> +#define W5_MAX_DEC_PIC_WIDTH            8192U
> +#define W5_MAX_DEC_PIC_HEIGHT           4320U
> +#define W5_DEC_CODEC_STEP_WIDTH         1U
> +#define W5_DEC_CODEC_STEP_HEIGHT        1U
> +#define W5_DEC_RAW_STEP_WIDTH           32U
> +#define W5_DEC_RAW_STEP_HEIGHT          16U
> +
> +#define W5_DEF_ENC_PIC_WIDTH            416U
> +#define W5_DEF_ENC_PIC_HEIGHT           240U
> +#define W5_MIN_ENC_PIC_WIDTH            256U
> +#define W5_MIN_ENC_PIC_HEIGHT           128U
> +#define W5_MAX_ENC_PIC_WIDTH            8192U
> +#define W5_MAX_ENC_PIC_HEIGHT           8192U
> +#define W5_ENC_CODEC_STEP_WIDTH         8U
> +#define W5_ENC_CODEC_STEP_HEIGHT        8U
> +#define W5_ENC_RAW_STEP_WIDTH           32U
> +#define W5_ENC_RAW_STEP_HEIGHT          16U
>  
>  //  application specific configuration
>  #define VPU_ENC_TIMEOUT                 60000


I'll have to give this one a second pass on next revision, but nice work.

Nicolas






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux