Re: [PATCH v1 05/10] media: platform: mediatek: add isp_7x camsys unit

[CK Hu (胡俊光)]

Hi, Shu-hsiang:

On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote:
> Introduces the top media device driver for the MediaTek ISP7X CAMSYS.
> The driver maintains the camera system, including sub-device management,
> DMA operations, and integration with the V4L2 framework. It handles
> request stream data, buffer management, and MediaTek-specific features,
> and pipeline management, streaming control, error handling mechanism.
> Additionally, it aggregates sub-drivers for the camera interface, raw
> and yuv pipelines.
> 
> Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@xxxxxxxxxxxx>
> ---

[snip]

> +
> +static int mtk_cam_fill_img_buf(struct mtkcam_ipi_img_output *img_out,
> +				const struct v4l2_format *f, dma_addr_t daddr)
> +{
> +	u32 pixelformat = f->fmt.pix_mp.pixelformat;
> +	u32 width = f->fmt.pix_mp.width;
> +	u32 height = f->fmt.pix_mp.height;
> +	const struct v4l2_plane_pix_format *plane = &f->fmt.pix_mp.plane_fmt[0];
> +	u32 stride = plane->bytesperline;
> +	u32 aligned_width;
> +	unsigned int addr_offset = 0;
> +	int i;
> +
> +	if (is_mtk_format(pixelformat)) {
> +		const struct mtk_format_info *info;
> +
> +		info = mtk_format_info(pixelformat);
> +		if (!info)
> +			return -EINVAL;
> +
> +		aligned_width = stride / info->bpp[0];
> +		if (info->mem_planes == 1) {
> +			if (is_yuv_ufo(pixelformat)) {
> +				aligned_width = ALIGN(width, 64);
> +				img_out->buf[0][0].iova = daddr;
> +				img_out->fmt.stride[0] = aligned_width * info->bit_r_num
> +							 / info->bit_r_den;
> +				img_out->buf[0][0].size = img_out->fmt.stride[0] * height;
> +				img_out->buf[0][0].size += img_out->fmt.stride[0] * height / 2;
> +				img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height;
> +				img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height
> +							   / 2;
> +				img_out->buf[0][0].size += sizeof(struct ufbc_buffer_header);
> +
> +				pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n",
> +					 0, img_out->fmt.stride[0], img_out->buf[0][0].size,
> +					 (unsigned long)img_out->buf[0][0].iova);
> +			} else if (is_raw_ufo(pixelformat)) {
> +				aligned_width = ALIGN(width, 64);
> +				img_out->buf[0][0].iova = daddr;
> +				img_out->fmt.stride[0] = aligned_width * info->bit_r_num /
> +							 info->bit_r_den;
> +				img_out->buf[0][0].size = img_out->fmt.stride[0] * height;
> +				img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height;
> +				img_out->buf[0][0].size += sizeof(struct ufbc_buffer_header);
> +
> +				pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n",
> +					 0, img_out->fmt.stride[0],
> +					 img_out->buf[0][0].size,
> +					 (unsigned long)img_out->buf[0][0].iova);
> +			} else {
> +				for (i = 0; i < info->comp_planes; i++) {
> +					unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> +					unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> +
> +					img_out->buf[0][i].iova = daddr + addr_offset;
> +					img_out->fmt.stride[i] = info->bpp[i] *
> +						DIV_ROUND_UP(aligned_width, hdiv);
> +					img_out->buf[0][i].size = img_out->fmt.stride[i]
> +						* DIV_ROUND_UP(height, vdiv);
> +					addr_offset += img_out->buf[0][i].size;
> +
> +					pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n",
> +						 i, img_out->fmt.stride[i],
> +						 img_out->buf[0][i].size,
> +						 (unsigned long)img_out->buf[0][i].iova);
> +				}
> +			}
> +		} else {
> +			pr_debug("do not support non contiguous mplane\n");
> +		}
> +	} else {
> +		const struct v4l2_format_info *info;
> +
> +		info = v4l2_format_info(pixelformat);
> +		if (!info)
> +			return -EINVAL;
> +
> +		aligned_width = stride / info->bpp[0];
> +		if (info->mem_planes == 1) {
> +			for (i = 0; i < info->comp_planes; i++) {

This for-loop is identical with the for-loop in is_mtk_format-is-true block.
So the checking of is_mtk_format is redundant.

Regards,
CK

> +				unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> +				unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> +
> +				img_out->buf[0][i].iova = daddr + addr_offset;
> +				img_out->fmt.stride[i] = info->bpp[i] *
> +					DIV_ROUND_UP(aligned_width, hdiv);
> +				img_out->buf[0][i].size = img_out->fmt.stride[i]
> +					* DIV_ROUND_UP(height, vdiv);
> +				addr_offset += img_out->buf[0][i].size;
> +
> +				pr_debug("stride:%d plane_size:%d addr:0x%lx\n",
> +					 img_out->fmt.stride[i],
> +					 img_out->buf[0][i].size,
> +					 (unsigned long)img_out->buf[0][i].iova);
> +			}
> +		} else {
> +			pr_debug("do not support non contiguous mplane\n");
> +		}
> +	}
> +
> +	return 0;
> +}
> +