Re: [PATCH v3 3/4] media: mediatek: add MT8188 AIE driver

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote:
> From: Bo Kong <Bo.Kong@mediatek.com>
> 
> Add a V4L2 sub-device driver for MT8188 AIE.
> 
> Signed-off-by: Bo Kong <Bo.Kong@mediatek.com>
> ---

[snip]

> +static int mtk_aie_ctrl_type_op_validate(const struct v4l2_ctrl *ctrl,
> +					 union v4l2_ctrl_ptr ptr)
> +{
> +	struct mtk_aie_ctx *ctx = ctrl_to_ctx(ctrl);
> +	struct mtk_aie_dev *fd;
> +	struct v4l2_ctrl_aie_init *p_aie_init;
> +	struct v4l2_ctrl_aie_param *p_aie_param;
> +
> +	if (!ctx)
> +		return -EINVAL;
> +
> +	fd = ctx->fd_dev;
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_MTK_AIE_PARAM:
> +		p_aie_param = ptr.p;
> +
> +		switch (p_aie_param->fd_mode) {
> +		case FDMODE:
> +		case ATTRIBUTEMODE:
> +		case FLDMODE:
> +			break;
> +		default:
> +			dev_err(ctx->dev, "AIE err:  mode: %d\n", p_aie_param->fd_mode);
> +				return -EINVAL;
> +		}
> +
> +		switch (p_aie_param->src_img_fmt) {
> +		case FMT_YUV_2P:
> +		case FMT_YVU_2P:
> +		case FMT_YUYV:
> +		case FMT_YVYU:
> +		case FMT_UYVY:
> +		case FMT_VYUY:
> +		case FMT_MONO:
> +		case FMT_YUV420_2P:
> +		case FMT_YUV420_1P:
> +			break;
> +		default:
> +			dev_err(ctx->dev, "AIE err:  fmt: %d\n", p_aie_param->src_img_fmt);
> +			return -EINVAL;
> +		}
> +
> +		if (p_aie_param->src_img_width >
> +				fd->base_para->max_img_width ||
> +			p_aie_param->src_img_height >
> +				fd->base_para->max_img_height ||
> +			p_aie_param->src_img_width == 0 ||
> +			p_aie_param->src_img_height == 0) {

Why max_img_width and max_img_height is passed from user space?
I think it's the hardware limitation and should be defined in driver.

Regards,
CK

> +			dev_err(fd->dev, "AIE err: Src_WD: %d Src_HT: %d\n",
> +				p_aie_param->src_img_width,
> +				p_aie_param->src_img_height);
> +
> +			dev_err(fd->dev,
> +				"AIE err: MAX_Src_WD: %d MAX_Src_HT: %d\n",
> +				fd->base_para->max_img_width,
> +				fd->base_para->max_img_height);
> +
> +			return -EINVAL;
> +		}
> +
> +		if (p_aie_param->pyramid_base_width
> +				> fd->base_para->max_pyramid_width ||
> +			p_aie_param->pyramid_base_height
> +				> fd->base_para->max_pyramid_height ||
> +			p_aie_param->number_of_pyramid > 3 ||
> +			p_aie_param->number_of_pyramid <= 0) {
> +			dev_err(fd->dev, "AIE err: base w: %d h: %d num: %d\n",
> +				p_aie_param->pyramid_base_width,
> +				p_aie_param->pyramid_base_height,
> +				p_aie_param->number_of_pyramid);
> +
> +			dev_err(fd->dev, "AIE err: max w: %d h: %d\n",
> +				fd->base_para->max_pyramid_width,
> +				fd->base_para->max_pyramid_height);
> +
> +			return -EINVAL;
> +		}
> +
> +		break;
> +
> +	case V4L2_CID_MTK_AIE_INIT:
> +		p_aie_init = ptr.p;
> +		if (!p_aie_init->max_img_width || !p_aie_init->max_img_height ||
> +		    !p_aie_init->pyramid_width || !p_aie_init->pyramid_height) {
> +			dev_err(fd->dev,
> +				"AIE INIT err: max_w: %d max_h: %d, p_w: %d p_h: %d\n",
> +				p_aie_init->max_img_width, p_aie_init->max_img_height,
> +				p_aie_init->pyramid_width, p_aie_init->pyramid_height);
> +
> +			return -EINVAL;
> +		}
> +
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +