Re: [RFC, V2, 09/11] media: platform: Add Mediatek ISP P1 device driver

[Drew Davenport <ddavenport@chromium.org>]

Hi Jungo,

On Fri, May 10, 2019 at 09:58:04AM +0800, Jungo Lin wrote:
> This patch adds the Mediatek ISP P1 HW control device driver.
> It handles the ISP HW configuration, provides interrupt handling and
> initializes the V4L2 device nodes and other functions.

A few comments inline.

> 
> Signed-off-by: Jungo Lin <jungo.lin@mediatek.com>
> ---
>  .../mtk-isp/isp_50/cam/mtk_cam-regs.h         |  149 ++
>  .../platform/mtk-isp/isp_50/cam/mtk_cam.c     | 1206 +++++++++++++++++
>  .../platform/mtk-isp/isp_50/cam/mtk_cam.h     |  300 ++++
>  3 files changed, 1655 insertions(+)
>  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h
>  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c
>  create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h
> 
> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h
> new file mode 100644
> index 000000000000..342f0e0e9837
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Ryan Yu <ryan.yu@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef _CAM_REGS_H
> +#define _CAM_REGS_H
> +
> +/* TG Bit Mask */
> +#define VFDATA_EN_BIT	BIT(0)
> +#define CMOS_EN_BIT	BIT(0)
> +
> +/* normal signal bit */
> +#define VS_INT_ST	BIT(0)
> +#define HW_PASS1_DON_ST	BIT(11)
> +#define SOF_INT_ST	BIT(12)
> +#define SW_PASS1_DON_ST	BIT(30)
> +
> +/* err status bit */
> +#define TG_ERR_ST	BIT(4)
> +#define TG_GBERR_ST	BIT(5)
> +#define CQ_CODE_ERR_ST	BIT(6)
> +#define CQ_APB_ERR_ST	BIT(7)
> +#define CQ_VS_ERR_ST	BIT(8)
> +#define AMX_ERR_ST	BIT(15)
> +#define RMX_ERR_ST	BIT(16)
> +#define BMX_ERR_ST	BIT(17)
> +#define RRZO_ERR_ST	BIT(18)
> +#define AFO_ERR_ST	BIT(19)
> +#define IMGO_ERR_ST	BIT(20)
> +#define AAO_ERR_ST	BIT(21)
> +#define PSO_ERR_ST	BIT(22)
> +#define LCSO_ERR_ST	BIT(23)
> +#define BNR_ERR_ST	BIT(24)
> +#define LSCI_ERR_ST	BIT(25)
> +#define DMA_ERR_ST	BIT(29)
> +
> +/* CAM DMA done status */
> +#define FLKO_DONE_ST	BIT(4)
> +#define AFO_DONE_ST	BIT(5)
> +#define AAO_DONE_ST	BIT(7)
> +#define PSO_DONE_ST	BIT(14)

Please align the values using tabs here and elsewhere.

> +
> +/* IRQ signal mask */
> +#define INT_ST_MASK_CAM	( \
> +			VS_INT_ST |\
> +			SOF_INT_ST |\
> +			HW_PASS1_DON_ST |\
> +			SW_PASS1_DON_ST)
> +
> +/* IRQ Warning Mask */
> +#define INT_ST_MASK_CAM_WARN	(\
> +				RRZO_ERR_ST |\
> +				AFO_ERR_ST |\
> +				IMGO_ERR_ST |\
> +				AAO_ERR_ST |\
> +				PSO_ERR_ST | \
> +				LCSO_ERR_ST |\
> +				BNR_ERR_ST |\
> +				LSCI_ERR_ST)
> +
> +/* IRQ Error Mask */
> +#define INT_ST_MASK_CAM_ERR	(\
> +				TG_ERR_ST |\
> +				TG_GBERR_ST |\
> +				CQ_CODE_ERR_ST |\
> +				CQ_APB_ERR_ST |\
> +				CQ_VS_ERR_ST |\
> +				BNR_ERR_ST |\
> +				RMX_ERR_ST |\
> +				BMX_ERR_ST |\
> +				BNR_ERR_ST |\
> +				LSCI_ERR_ST |\
> +				DMA_ERR_ST)
> +
> +/* IRQ Signal Log Mask */
> +#define INT_ST_LOG_MASK_CAM	(\
> +				SOF_INT_ST |\
> +				SW_PASS1_DON_ST |\
> +				VS_INT_ST |\
> +				TG_ERR_ST |\
> +				TG_GBERR_ST |\
> +				RRZO_ERR_ST |\
> +				AFO_ERR_ST |\
> +				IMGO_ERR_ST |\
> +				AAO_ERR_ST |\
> +				DMA_ERR_ST)
> +
> +/* DMA Event Notification Mask */
> +#define DMA_ST_MASK_CAM	(\
> +			AFO_DONE_ST |\
> +			AAO_DONE_ST |\
> +			PSO_DONE_ST |\
> +			FLKO_DONE_ST)
> +
> +/* Status check */
> +#define REG_CTL_EN		0x0004
> +#define REG_CTL_DMA_EN		0x0008
> +#define REG_CTL_FMT_SEL		0x0010
> +#define REG_CTL_EN2		0x0018
> +#define REG_CTL_RAW_INT_EN	0x0020
> +#define REG_CTL_RAW_INT_STAT	0x0024
> +#define REG_CTL_RAW_INT2_STAT	0x0034
> +#define REG_CTL_RAW_INT3_STAT	0x00c4
> +#define REG_CTL_TWIN_STAT	0x0050
> +
> +#define REG_TG_SEN_MODE		0x0230
> +#define REG_TG_SEN_GRAB_PIX	0x0238
> +#define REG_TG_SEN_GRAB_LIN	0x023c
> +#define REG_TG_VF_CON		0x0234
> +#define REG_TG_SUB_PERIOD	0x02a4
> +
> +#define REG_IMGO_BASE_ADDR	0x1020
> +#define REG_RRZO_BASE_ADDR	0x1050
> +
> +/* Error status log */
> +#define REG_IMGO_ERR_STAT	0x1360
> +#define REG_RRZO_ERR_STAT	0x1364
> +#define REG_AAO_ERR_STAT	0x1368
> +#define REG_AFO_ERR_STAT	0x136c
> +#define REG_LCSO_ERR_STAT	0x1370
> +#define REG_UFEO_ERR_STAT	0x1374
> +#define REG_PDO_ERR_STAT	0x1378
> +#define REG_BPCI_ERR_STAT	0x137c
> +#define REG_LSCI_ERR_STAT	0x1384
> +#define REG_PDI_ERR_STAT	0x138c
> +#define REG_LMVO_ERR_STAT	0x1390
> +#define REG_FLKO_ERR_STAT	0x1394
> +#define REG_PSO_ERR_STAT	0x13a0
> +
> +/* ISP command */
> +#define REG_CQ_THR0_BASEADDR	0x0198
> +#define REG_HW_FRAME_NUM	0x13b8
> +
> +/* META */
> +#define REG_META0_VB2_INDEX	0x14dc
> +#define REG_META1_VB2_INDEX	0x151c
> +
> +#endif	/* _CAM_REGS_H */
> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c
> new file mode 100644
> index 000000000000..fc874ec8f7f0
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c
> @@ -0,0 +1,1206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Ryan Yu <ryan.yu@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/cdev.h>
> +#include <linux/compat.h>
> +#include <linux/fs.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/ktime.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/mtk_scp.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/remoteproc.h>
> +#include <linux/sched/clock.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/videodev2.h>
> +#include <linux/vmalloc.h>
> +
> +#include "mtk_cam.h"
> +#include "mtk_cam-regs.h"
> +#include "mtk_cam-smem.h"
> +
> +static const struct of_device_id mtk_isp_of_ids[] = {
> +	{.compatible = "mediatek,mt8183-camisp",},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mtk_isp_of_ids);
> +
> +/* list of clocks required by isp cam */
> +static const char * const mtk_isp_clks[] = {
> +	"CAMSYS_CAM_CGPDN", "CAMSYS_CAMTG_CGPDN"
> +};
> +
> +static void isp_dump_dma_status(struct isp_device *isp_dev)
> +{
> +	dev_err(isp_dev->dev,
> +		"IMGO:0x%x, RRZO:0x%x, AAO=0x%x, AFO=0x%x, LMVO=0x%x\n",
> +		readl(isp_dev->regs + REG_IMGO_ERR_STAT),
> +		readl(isp_dev->regs + REG_RRZO_ERR_STAT),
> +		readl(isp_dev->regs + REG_AAO_ERR_STAT),
> +		readl(isp_dev->regs + REG_AFO_ERR_STAT),
> +		readl(isp_dev->regs + REG_LMVO_ERR_STAT));
> +	dev_err(isp_dev->dev,
> +		"LCSO=0x%x, PSO=0x%x, FLKO=0x%x, BPCI:0x%x, LSCI=0x%x\n",
> +		readl(isp_dev->regs + REG_LCSO_ERR_STAT),
> +		readl(isp_dev->regs + REG_PSO_ERR_STAT),
> +		readl(isp_dev->regs + REG_FLKO_ERR_STAT),
> +		readl(isp_dev->regs + REG_BPCI_ERR_STAT),
> +		readl(isp_dev->regs + REG_LSCI_ERR_STAT));
> +}
> +
> +static void mtk_isp_notify(struct mtk_isp_p1_ctx *isp_ctx,
> +			   unsigned int request_fd,
> +			   unsigned int frame_seq_no,
> +			   struct list_head *list_buf,
> +			   enum vb2_buffer_state state)
> +{
> +	struct isp_p1_device *p1_dev = p1_ctx_to_dev(isp_ctx);
> +	struct device *dev = &p1_dev->pdev->dev;
> +	struct mtk_cam_dev_finish_param fram_param;
> +
> +	fram_param.list_buf = list_buf;
> +	fram_param.request_fd = request_fd;
> +	fram_param.frame_seq_no = frame_seq_no;
> +	fram_param.state = state;
> +	dev_dbg(dev, "request fd:%d frame_seq_no:%d\n",
> +		fram_param.request_fd,
> +		fram_param.frame_seq_no);
> +	mtk_cam_dev_job_finish(p1_dev->cam_dev, &fram_param);
> +}
> +
> +static void isp_deque_frame(struct isp_p1_device *p1_dev,
> +			    unsigned int node_id, int vb2_index,
> +			    int frame_seq_no)
> +{
> +	struct mtk_cam_dev *cam_dev = p1_dev->cam_dev;
> +	struct device *dev = &p1_dev->pdev->dev;
> +	struct vb2_queue *vb2_queue = &cam_dev->mem2mem2_nodes[node_id].vbq;
> +	struct vb2_buffer *vb;
> +	struct vb2_v4l2_buffer *vbb;
> +
> +	if (!cam_dev->mem2mem2_nodes[node_id].enabled)
> +		return;
> +
> +	mutex_lock(vb2_queue->lock);
> +	list_for_each_entry(vb, &vb2_queue->queued_list, queued_entry) {
> +		vbb = to_vb2_v4l2_buffer(vb);
> +		if (vbb->request_fd < 0 &&
> +		    vb->index == vb2_index &&
> +		    vb->state == VB2_BUF_STATE_ACTIVE) {
> +			dev_dbg(dev, "%s:%d:%d", __func__, node_id, vb2_index);
> +			vbb->vb2_buf.timestamp = ktime_get_ns();
> +			vbb->sequence = frame_seq_no;
> +			vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> +		}
> +	}
> +	mutex_unlock(vb2_queue->lock);
> +}
> +
> +static void isp_deque_request_frame(struct isp_p1_device *p1_dev,
> +				    int frame_seq_no)
> +{
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct device *dev = &p1_dev->pdev->dev;
> +	struct mtk_isp_queue_job *framejob, *tmp;
> +	struct isp_queue *p1_enqueue_list = &isp_ctx->p1_enqueue_list;
> +
> +	/* Match dequeue work and enqueue frame */
> +	spin_lock(&p1_enqueue_list->lock);
> +	list_for_each_entry_safe(framejob, tmp, &p1_enqueue_list->queue,
> +				 list_entry) {
> +		dev_dbg(dev,
> +			"%s frame_seq_no:%d, target frame_seq_no:%d\n",
> +			__func__,
> +			framejob->frame_seq_no, frame_seq_no);
> +		/* Match by the en-queued request number */
> +		if (framejob->frame_seq_no == frame_seq_no) {
> +			/* Pass to user space */
> +			mtk_isp_notify(isp_ctx,
> +				       framejob->request_fd,
> +				       framejob->frame_seq_no,
> +				       &framejob->list_buf,
> +				       VB2_BUF_STATE_DONE);
> +			atomic_dec(&p1_enqueue_list->queue_cnt);
> +			dev_dbg(dev,
> +				"frame_seq_no:%d is done, queue_cnt:%d\n",
> +				framejob->frame_seq_no,
> +				atomic_read(&p1_enqueue_list->queue_cnt));
> +
> +			/* remove only when frame ready */
> +			list_del(&framejob->list_entry);
> +			kfree(framejob);
> +			break;
> +		} else if (framejob->frame_seq_no < frame_seq_no) {
> +			/* Pass to user space for frame drop */
> +			mtk_isp_notify(isp_ctx,
> +				       framejob->request_fd,
> +				       framejob->frame_seq_no,
> +				       &framejob->list_buf,
> +				       VB2_BUF_STATE_ERROR);
> +			atomic_dec(&p1_enqueue_list->queue_cnt);
> +			dev_dbg(dev,
> +				"frame_seq_no:%d drop, queue_cnt:%d\n",
> +				framejob->frame_seq_no,
> +				atomic_read(&p1_enqueue_list->queue_cnt));
> +
> +			/* remove only drop frame */
> +			list_del(&framejob->list_entry);
> +			kfree(framejob);
> +		}
> +	}
> +	spin_unlock(&p1_enqueue_list->lock);
> +}
> +
> +static int isp_deque_work(void *data)
> +{
> +	struct isp_p1_device *p1_dev = (struct isp_p1_device *)data;
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct device *dev = &p1_dev->pdev->dev;
> +	struct mtk_cam_dev *cam_dev = p1_dev->cam_dev;
> +	struct mtk_cam_dev_stat_event_data event_data;
> +	atomic_t *irq_data_end = &isp_ctx->irq_data_end;
> +	atomic_t *irq_data_start = &isp_ctx->irq_data_start;
> +	unsigned long flags;
> +	int ret, i;
> +
> +	while (1) {
> +		ret = wait_event_interruptible(isp_ctx->isp_deque_thread.wq,
> +					       (atomic_read(irq_data_end) !=
> +					       atomic_read(irq_data_start)) ||
> +					       kthread_should_stop());
> +
> +		if (kthread_should_stop())
> +			break;
> +
> +		if (ret == ERESTARTSYS) {
> +			dev_err(dev, "interrupted by a signal!\n");
> +			continue;
> +		}
> +
> +		spin_lock_irqsave(&isp_ctx->irq_dequeue_lock, flags);
> +		i = atomic_read(&isp_ctx->irq_data_start);
> +		memcpy(&event_data, &isp_ctx->irq_event_datas[i],
> +		       sizeof(event_data));
> +		memset(&isp_ctx->irq_event_datas[i], 0x00, sizeof(event_data));
> +		atomic_set(&isp_ctx->irq_data_start, ++i & 0x3);
> +		spin_unlock_irqrestore(&isp_ctx->irq_dequeue_lock, flags);
> +
> +		if (event_data.irq_status_mask & VS_INT_ST) {
> +			/* Notify specific HW events to user space */
> +			mtk_cam_dev_event_frame_sync(cam_dev,
> +						     event_data.frame_seq_no);
> +			dev_dbg(dev,
> +				"event IRQ:0x%x DMA:0x%x is sent\n",
> +				event_data.irq_status_mask,
> +				event_data.dma_status_mask);
> +		}
> +
> +		if (event_data.dma_status_mask & AAO_DONE_ST) {
> +			isp_deque_frame(p1_dev,
> +					MTK_CAM_P1_META_OUT_0,
> +					event_data.meta0_vb2_index,
> +					event_data.frame_seq_no);
> +		}
> +
> +		if (event_data.irq_status_mask & SW_PASS1_DON_ST) {
> +			isp_deque_frame(p1_dev,
> +					MTK_CAM_P1_META_OUT_0,
> +					event_data.meta0_vb2_index,
> +					event_data.frame_seq_no);
> +
> +			isp_deque_request_frame(p1_dev,
> +						event_data.frame_seq_no);
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int irq_handle_sof(struct isp_device *isp_dev,
> +			  dma_addr_t base_addr,
> +			  unsigned int frame_num)
> +{
> +	unsigned int cq_addr_index;
> +	struct isp_p1_device *p1_dev = get_p1_device(isp_dev->dev);
> +	int cq_num = atomic_read(&p1_dev->isp_ctx.composed_frame_id);
> +
> +	if (cq_num > frame_num) {
> +		cq_addr_index = frame_num % CQ_BUFFER_COUNT;
> +
> +		writel(base_addr +
> +			(dma_addr_t)(CQ_ADDRESS_OFFSET * cq_addr_index),
> +			isp_dev->regs + REG_CQ_THR0_BASEADDR);
> +		dev_dbg(isp_dev->dev,
> +			"SOF_INT_ST, update next, cq_num:%d, frame_num:%d cq_addr:%d",
> +			cq_num, frame_num, cq_addr_index);
> +	} else {
> +		dev_dbg(isp_dev->dev,
> +			"SOF_INT_ST, wait next, cq_num:%d, frame_num:%d",
> +			cq_num, frame_num);
> +	}
> +
> +	isp_dev->sof_count += 1;
> +
> +	return cq_num;
> +}
> +
> +static int irq_handle_notify_event(struct isp_device *isp_dev,
> +				   unsigned int irqstatus,
> +				   unsigned int dmastatus)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(isp_dev->dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&isp_ctx->irq_dequeue_lock, flags);
> +	i = atomic_read(&isp_ctx->irq_data_end);
> +	isp_ctx->irq_event_datas[i].frame_seq_no = isp_dev->current_frame;
> +	isp_ctx->irq_event_datas[i].meta0_vb2_index = isp_dev->meta0_vb2_index;
> +	isp_ctx->irq_event_datas[i].meta1_vb2_index = isp_dev->meta1_vb2_index;
> +	isp_ctx->irq_event_datas[i].irq_status_mask |=
> +		(irqstatus & INT_ST_MASK_CAM);
> +	isp_ctx->irq_event_datas[i].dma_status_mask |=
> +		(dmastatus & DMA_ST_MASK_CAM);
> +	atomic_set(&isp_ctx->irq_data_end, ++i & 0x3);
> +	spin_unlock_irqrestore(&isp_ctx->irq_dequeue_lock, flags);
> +
> +	wake_up_interruptible(&isp_ctx->isp_deque_thread.wq);
> +
> +	dev_dbg(isp_dev->dev,
> +		"%s IRQ:0x%x DMA:0x%x seq:%d idx0:%d idx1:%d\n",
> +		__func__,
> +		(irqstatus & INT_ST_MASK_CAM),
> +		(dmastatus & DMA_ST_MASK_CAM),
> +		isp_dev->current_frame,
> +		isp_dev->meta0_vb2_index,
> +		isp_dev->meta1_vb2_index);
> +
> +	return 0;
> +}
> +
> +irqreturn_t isp_irq_cam(int irq, void *data)
> +{
> +	struct isp_device *isp_dev = (struct isp_device *)data;
> +	struct isp_p1_device *p1_dev = get_p1_device(isp_dev->dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct device *dev = isp_dev->dev;
> +	unsigned int cardinalnum, cq_num, hw_frame_num;
> +	unsigned int meta0_vb2_index, meta1_vb2_index;
> +	unsigned int irqstatus, errstatus, warnstatus, dmastatus;
> +	unsigned long flags;
> +
> +	/* Check the streaming is off or not */
> +	if (!p1_dev->cam_dev->streaming)
> +		return IRQ_HANDLED;
> +
> +	cardinalnum = isp_dev->isp_hw_module - ISP_CAM_A_IDX;
> +	cq_num = 0;
> +
> +	spin_lock_irqsave(&isp_dev->spinlock_irq, flags);
> +	irqstatus = readl(isp_dev->regs + REG_CTL_RAW_INT_STAT);
> +	dmastatus = readl(isp_dev->regs + REG_CTL_RAW_INT2_STAT);
> +	hw_frame_num = readl(isp_dev->regs + REG_HW_FRAME_NUM);
> +	meta0_vb2_index = readl(isp_dev->regs + REG_META0_VB2_INDEX);
> +	meta1_vb2_index = readl(isp_dev->regs + REG_META1_VB2_INDEX);
> +	spin_unlock_irqrestore(&isp_dev->spinlock_irq, flags);
> +
> +	/* Ignore unnecessary IRQ */
> +	if (irqstatus == 0)
> +		return IRQ_HANDLED;
> +
> +	errstatus = irqstatus & INT_ST_MASK_CAM_ERR;
> +	warnstatus = irqstatus & INT_ST_MASK_CAM_WARN;
> +	irqstatus = irqstatus & INT_ST_MASK_CAM;
> +
> +	/* sof , done order check . */
> +	spin_lock_irqsave(&isp_dev->spinlock_irq, flags);
> +	if ((irqstatus & HW_PASS1_DON_ST) && (irqstatus & SOF_INT_ST)) {
> +		dev_warn(dev,
> +			 "isp sof_don block, sof_cnt:%d\n",
> +			 isp_dev->sof_count);
> +
> +		/* Notify IRQ event and enqueue ready frame */
> +		irq_handle_notify_event(isp_dev, irqstatus, dmastatus);
> +		isp_dev->current_frame = hw_frame_num;
> +		isp_dev->meta0_vb2_index = meta0_vb2_index;
> +		isp_dev->meta1_vb2_index = meta1_vb2_index;
> +	} else {
> +		if (irqstatus & SOF_INT_ST) {
> +			isp_dev->current_frame = hw_frame_num;
> +			isp_dev->meta0_vb2_index = meta0_vb2_index;
> +			isp_dev->meta1_vb2_index = meta1_vb2_index;
> +		}
> +
> +		if ((irqstatus & INT_ST_MASK_CAM) ||
> +		    (dmastatus & DMA_ST_MASK_CAM))
> +			irq_handle_notify_event(isp_dev, irqstatus, dmastatus);
> +	}
> +	spin_unlock_irqrestore(&isp_dev->spinlock_irq, flags);
> +
> +	if (irqstatus & SOF_INT_ST)
> +		cq_num = irq_handle_sof(isp_dev, isp_ctx->scp_mem_iova,
> +					hw_frame_num);
> +
> +	if (irqstatus & SW_PASS1_DON_ST) {
> +		int num = atomic_dec_return(&isp_ctx->composing_frame);
> +
> +		dev_dbg(dev, "SW_PASS1_DON_ST queued frame:%d\n", num);
> +		/* Notify TX thread to send if TX frame is blocked */
> +		wake_up_interruptible
> +				(&isp_ctx->composer_tx_thread.wq);
> +	}
> +
> +	/* check ISP error status */
> +	if (errstatus) {
> +		dev_err(dev,
> +			"raw_int_err:0x%x/0x%x/0x%x\n",
> +			irqstatus, warnstatus, errstatus);
> +
> +		/* show DMA errors in detail */
> +		if (errstatus & DMA_ERR_ST)
> +			isp_dump_dma_status(isp_dev);
> +	}
> +
> +	if (irqstatus & INT_ST_LOG_MASK_CAM)
> +		dev_dbg(dev, IRQ_STAT_STR,
> +			'A' + cardinalnum,
> +			isp_dev->sof_count,
> +			irqstatus,
> +			dmastatus,
> +			hw_frame_num,
> +			cq_num);
> +	return IRQ_HANDLED;
> +}
> +
> +static int enable_sys_clock(struct isp_p1_device *p1_dev)
> +{
> +	struct device *dev = &p1_dev->pdev->dev;
> +	int ret;
> +
> +	dev_info(dev, "- %s dev id:%d\n", __func__, dev->id);
> +
> +	ret = clk_bulk_prepare_enable(p1_dev->isp_clk.num_clks,
> +				      p1_dev->isp_clk.clk_list);
> +	if (ret < 0)
> +		goto clk_err;
> +	return 0;
> +clk_err:
> +	dev_err(dev, "cannot pre-en isp_cam clock:%d\n", ret);
> +	clk_bulk_disable_unprepare(p1_dev->isp_clk.num_clks,
> +				   p1_dev->isp_clk.clk_list);
> +	return ret;
> +}
> +
> +static void disable_sys_clock(struct isp_p1_device *p1_dev)
> +{
> +	struct device *dev = &p1_dev->pdev->dev;
> +
> +	dev_info(dev, "- %s dev id:%d\n", __func__, dev->id);
> +	clk_bulk_disable_unprepare(p1_dev->isp_clk.num_clks,
> +				   p1_dev->isp_clk.clk_list);
> +}
> +
> +static int mtk_isp_probe(struct platform_device *pdev)
> +{
> +	struct isp_p1_device *p1_dev;
> +	struct mtk_isp_p1_ctx *isp_ctx;
> +	struct isp_device *isp_dev;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret;
> +	unsigned int i;
> +
> +	/* Allocate context */
> +	p1_dev = devm_kzalloc(dev, sizeof(*p1_dev), GFP_KERNEL);
> +	if (!p1_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, p1_dev);
> +	isp_ctx = &p1_dev->isp_ctx;
> +	p1_dev->pdev = pdev;
> +
> +	p1_dev->isp_devs =
> +		devm_kzalloc(dev,
> +			     sizeof(struct isp_device) * ISP_DEV_NODE_NUM,
> +			     GFP_KERNEL);
> +	if (!p1_dev->isp_devs)
> +		return -ENOMEM;
> +
> +	p1_dev->cam_dev =
> +		devm_kzalloc(dev, sizeof(struct mtk_cam_dev), GFP_KERNEL);
> +	if (!p1_dev->isp_devs)
> +		return -ENOMEM;
> +
> +	/* iomap registers */
> +	for (i = ISP_CAMSYS_CONFIG_IDX; i < ISP_DEV_NODE_NUM; i++) {
> +		isp_dev = &p1_dev->isp_devs[i];
> +		isp_dev->isp_hw_module = i;
> +		isp_dev->dev = dev;
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		isp_dev->regs = devm_ioremap_resource(dev, res);
> +
> +		dev_info(dev, "cam%u, map_addr=0x%lx\n",
> +			 i, (unsigned long)isp_dev->regs);
> +
> +		if (!isp_dev->regs)
> +			return PTR_ERR(isp_dev->regs);
> +
> +		/* support IRQ from ISP_CAM_A_IDX */
> +		if (i >= ISP_CAM_A_IDX) {
> +			/* reg & interrupts index is shifted with 1  */
> +			isp_dev->irq = platform_get_irq(pdev, i - 1);
> +			if (isp_dev->irq > 0) {
> +				ret = devm_request_irq(dev, isp_dev->irq,
> +						       isp_irq_cam,
> +						       IRQF_SHARED,
> +						       dev_driver_string(dev),
> +						       (void *)isp_dev);
> +				if (ret) {
> +					dev_err(dev,
> +						"req_irq fail, dev:%s irq=%d\n",
> +						dev->of_node->name,
> +						isp_dev->irq);
> +					return ret;
> +				}
> +				dev_info(dev, "Registered irq=%d, ISR:%s\n",
> +					 isp_dev->irq, dev_driver_string(dev));
> +			}
> +		}
> +		spin_lock_init(&isp_dev->spinlock_irq);
> +	}
> +
> +	p1_dev->isp_clk.num_clks = ARRAY_SIZE(mtk_isp_clks);
> +	p1_dev->isp_clk.clk_list =
> +		devm_kcalloc(dev,
> +			     p1_dev->isp_clk.num_clks,
> +			     sizeof(*p1_dev->isp_clk.clk_list),
> +			     GFP_KERNEL);
> +	if (!p1_dev->isp_clk.clk_list)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < p1_dev->isp_clk.num_clks; ++i)
> +		p1_dev->isp_clk.clk_list->id = mtk_isp_clks[i];
> +
> +	ret = devm_clk_bulk_get(dev,
> +				p1_dev->isp_clk.num_clks,
> +				p1_dev->isp_clk.clk_list);
> +	if (ret) {
> +		dev_err(dev, "cannot get isp cam clock:%d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Initialize reserved DMA memory */
> +	ret = mtk_cam_reserved_memory_init(p1_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to configure DMA memory\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the v4l2 common part */
> +	ret = mtk_cam_dev_init(pdev, p1_dev->cam_dev);
> +	if (ret)
> +		return ret;
> +
> +	spin_lock_init(&isp_ctx->p1_enqueue_list.lock);
> +	atomic_set(&p1_dev->isp_ctx.isp_user_cnt, 0);
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +}
> +
> +static int mtk_isp_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct isp_p1_device *p1_dev = dev_get_drvdata(dev);
> +
> +	pm_runtime_disable(dev);
> +	mtk_cam_dev_release(pdev, p1_dev->cam_dev);
> +
> +	return 0;
> +}
> +
> +static int mtk_isp_suspend(struct device *dev)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct isp_device *isp_dev;
> +	unsigned int reg_val;
> +	int usercount, module;
> +
> +	module = p1_dev->isp_ctx.isp_hw_module;
> +	usercount = atomic_read(&p1_dev->isp_ctx.isp_user_cnt);
> +
> +	dev_dbg(dev, "- %s:%d\n", __func__, usercount);
> +
> +	/* If no user count, no further action */
> +	if (!usercount)
> +		return 0;
> +
> +	isp_dev = &p1_dev->isp_devs[module];
> +	reg_val = readl(isp_dev->regs + REG_TG_VF_CON);
> +	if (reg_val & VFDATA_EN_BIT) {
> +		dev_dbg(dev, "Cam:%d suspend, disable VF\n", module);
> +		/* disable VF */
> +		writel((reg_val & (~VFDATA_EN_BIT)),
> +		       isp_dev->regs + REG_TG_VF_CON);
> +		/*
> +		 * After VF enable, The TG frame count will be reset to 0;
> +		 */
> +		reg_val = readl(isp_dev->regs + REG_TG_SEN_MODE);
> +		writel((reg_val & (~CMOS_EN_BIT)),
> +		       isp_dev->regs +  + REG_TG_SEN_MODE);
> +	}
> +
> +	disable_sys_clock(p1_dev);
> +
> +	return 0;
> +}
> +
> +static int mtk_isp_resume(struct device *dev)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct isp_device *isp_dev;
> +	unsigned int reg_val;
> +	int module, usercount;
> +
> +	module = p1_dev->isp_ctx.isp_hw_module;
> +	usercount = atomic_read(&p1_dev->isp_ctx.isp_user_cnt);
> +
> +	dev_dbg(dev, "- %s:%d\n", __func__, usercount);
> +
> +	/* If no user count, no further action */
> +	if (!usercount)
> +		return 0;
> +
> +	enable_sys_clock(p1_dev);
> +
> +	/* V4L2 stream-on phase & restore HW stream-on status */
> +	if (p1_dev->cam_dev->streaming) {
> +		isp_dev = &p1_dev->isp_devs[module];
> +		dev_dbg(dev, "Cam:%d resume,enable VF\n", module);
> +		/* Enable CMOS */
> +		reg_val = readl(isp_dev->regs + REG_TG_SEN_MODE);
> +		writel((reg_val | CMOS_EN_BIT),
> +		       isp_dev->regs + REG_TG_SEN_MODE);
> +		/* Enable VF */
> +		reg_val = readl(isp_dev->regs + REG_TG_VF_CON);
> +		writel((reg_val | VFDATA_EN_BIT),
> +		       isp_dev->regs + REG_TG_VF_CON);
> +	}
> +	return 0;
> +}
> +
> +static int isp_setup_scp_rproc(struct isp_p1_device *p1_dev)
> +{
> +	phandle rproc_phandle;
> +	struct device *dev = &p1_dev->pdev->dev;
> +	int ret;
> +
> +	p1_dev->scp_pdev = scp_get_pdev(p1_dev->pdev);
> +	if (!p1_dev->scp_pdev) {
> +		dev_err(dev, "Failed to get scp device\n");
> +		return -ENODEV;
> +	}
> +	ret = of_property_read_u32(dev->of_node, "mediatek,scp",
> +				   &rproc_phandle);
> +	if (ret) {
> +		dev_err(dev, "fail to get rproc_phandle:%d\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	p1_dev->rproc_handle = rproc_get_by_phandle(rproc_phandle);
> +	dev_dbg(dev, "p1 rproc_phandle: 0x%pK\n\n",
> +		p1_dev->rproc_handle);
> +	if (!p1_dev->rproc_handle) {
> +		dev_err(dev, "fail to get rproc_handle\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = rproc_boot(p1_dev->rproc_handle);
> +	if (ret < 0) {
> +		/*
> +		 * Return 0 if downloading firmware successfully,
> +		 * otherwise it is failed
> +		 */
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int isp_init_context(struct isp_p1_device *p1_dev)
> +{
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct device *dev = &p1_dev->pdev->dev;
> +	unsigned int i;
> +
> +	dev_dbg(dev, "init irq work thread\n");
> +	if (!isp_ctx->isp_deque_thread.thread) {
> +		mutex_init(&isp_ctx->composer_tx_lock);
> +		init_waitqueue_head(&isp_ctx->isp_deque_thread.wq);
> +		isp_ctx->isp_deque_thread.thread =
> +			kthread_run(isp_deque_work, (void *)p1_dev,
> +				    "isp_deque_work");
> +		if (IS_ERR(isp_ctx->isp_deque_thread.thread)) {
> +			dev_err(dev, "unable to alloc kthread\n");
> +			isp_ctx->isp_deque_thread.thread = NULL;
> +			return -ENOMEM;
> +		}
> +	}
> +	spin_lock_init(&isp_ctx->irq_dequeue_lock);
> +
> +	INIT_LIST_HEAD(&isp_ctx->p1_enqueue_list.queue);
> +	atomic_set(&isp_ctx->p1_enqueue_list.queue_cnt, 0);
> +
> +	for (i = 0; i < ISP_DEV_NODE_NUM; i++)
> +		spin_lock_init(&p1_dev->isp_devs[i].spinlock_irq);
> +
> +	spin_lock_init(&isp_ctx->p1_enqueue_list.lock);
> +	spin_lock_init(&isp_ctx->composer_txlist.lock);
> +
> +	atomic_set(&isp_ctx->irq_data_end, 0);
> +	atomic_set(&isp_ctx->irq_data_start, 0);
> +	return 0;
> +}
> +
> +static int isp_uninit_context(struct isp_p1_device *p1_dev)
> +{
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct mtk_isp_queue_job *framejob, *tmp_framejob;
> +
> +	spin_lock_irq(&isp_ctx->p1_enqueue_list.lock);
> +	list_for_each_entry_safe(framejob, tmp_framejob,
> +				 &isp_ctx->p1_enqueue_list.queue, list_entry) {
> +		list_del(&framejob->list_entry);
> +		kfree(framejob);
> +	}
> +	spin_unlock_irq(&isp_ctx->p1_enqueue_list.lock);
> +
> +	atomic_set(&isp_ctx->isp_user_cnt, 0);
> +
> +	if (!IS_ERR(isp_ctx->isp_deque_thread.thread)) {
> +		kthread_stop(isp_ctx->isp_deque_thread.thread);
> +		wake_up_interruptible(&isp_ctx->isp_deque_thread.wq);
> +		isp_ctx->isp_deque_thread.thread = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static unsigned int get_enable_dma_ports(struct mtk_cam_dev *cam_dev)

I think s/enable/enabled would be a clearer name for both the function
and local variable.

> +{
> +	unsigned int enable_dma_ports, i;
> +
> +	/* Get the enabled meta DMA ports */
> +	enable_dma_ports = 0;
> +	for (i = 0; i < cam_dev->dev_node_num; i++) {
> +		if (cam_dev->mem2mem2_nodes[i].enabled)
> +			enable_dma_ports |=
> +				cam_dev->mem2mem2_nodes[i].desc.dma_port;
> +	}
> +	dev_dbg(&cam_dev->pdev->dev, "%s enable_dma_ports:0x%x",
> +		__func__, enable_dma_ports);
> +
> +	return enable_dma_ports;
> +}
> +
> +/* Utility functions */
> +static unsigned int get_sensor_pixel_id(unsigned int fmt)
> +{
> +	switch (fmt) {
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	case MEDIA_BUS_FMT_SBGGR12_1X12:
> +	case MEDIA_BUS_FMT_SBGGR14_1X14:
> +		return raw_pxl_id_b;
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG12_1X12:
> +	case MEDIA_BUS_FMT_SGBRG14_1X14:
> +		return raw_pxl_id_gb;
> +	case MEDIA_BUS_FMT_SGRBG8_1X8:
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +	case MEDIA_BUS_FMT_SGRBG12_1X12:
> +	case MEDIA_BUS_FMT_SGRBG14_1X14:
> +		return raw_pxl_id_gr;
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +	case MEDIA_BUS_FMT_SRGGB14_1X14:
> +		return raw_pxl_id_r;
> +	default:
> +		return raw_pxl_id_b;
> +	}
> +}
> +
> +static unsigned int get_sensor_fmt(unsigned int fmt)
> +{
> +	switch (fmt) {
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SGRBG8_1X8:
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +		return img_fmt_bayer8;
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +		return img_fmt_bayer10;
> +	case MEDIA_BUS_FMT_SBGGR12_1X12:
> +	case MEDIA_BUS_FMT_SGBRG12_1X12:
> +	case MEDIA_BUS_FMT_SGRBG12_1X12:
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +		return img_fmt_bayer12;
> +	case MEDIA_BUS_FMT_SBGGR14_1X14:
> +	case MEDIA_BUS_FMT_SGBRG14_1X14:
> +	case MEDIA_BUS_FMT_SGRBG14_1X14:
> +	case MEDIA_BUS_FMT_SRGGB14_1X14:
> +		return img_fmt_bayer14;
> +	default:
> +		return img_fmt_unknown;
> +	}
> +}
> +
> +static unsigned int get_img_fmt(unsigned int fourcc)
> +{
> +	switch (fourcc) {
> +	case V4L2_PIX_FMT_MTISP_B8:
> +		return img_fmt_bayer8;
> +	case V4L2_PIX_FMT_MTISP_F8:
> +		return img_fmt_fg_bayer8;
> +	case V4L2_PIX_FMT_MTISP_B10:
> +		return img_fmt_bayer10;
> +	case V4L2_PIX_FMT_MTISP_F10:
> +		return img_fmt_fg_bayer10;
> +	case V4L2_PIX_FMT_MTISP_B12:
> +		return img_fmt_bayer12;
> +	case V4L2_PIX_FMT_MTISP_F12:
> +		return img_fmt_fg_bayer12;
> +	case V4L2_PIX_FMT_MTISP_B14:
> +		return img_fmt_bayer14;
> +	case V4L2_PIX_FMT_MTISP_F14:
> +		return img_fmt_fg_bayer14;
> +	default:
> +		return img_fmt_unknown;
> +	}
> +}
> +
> +static unsigned int get_pixel_byte(unsigned int fourcc)
> +{
> +	switch (fourcc) {
> +	case V4L2_PIX_FMT_MTISP_B8:
> +	case V4L2_PIX_FMT_MTISP_F8:
> +		return 8;
> +	case V4L2_PIX_FMT_MTISP_B10:
> +	case V4L2_PIX_FMT_MTISP_F10:
> +		return 10;
> +	case V4L2_PIX_FMT_MTISP_B12:
> +	case V4L2_PIX_FMT_MTISP_F12:
> +		return 12;
> +	case V4L2_PIX_FMT_MTISP_B14:
> +	case V4L2_PIX_FMT_MTISP_F14:
> +		return 14;
> +	case V4L2_PIX_FMT_MTISP_U8:
> +	case V4L2_PIX_FMT_MTISP_U10:
> +	case V4L2_PIX_FMT_MTISP_U12:
> +	case V4L2_PIX_FMT_MTISP_U14:
> +		return 16;
> +	default:
> +		return 10;
> +	}
> +}
> +
> +static void composer_deinit_done_cb(void *data)
> +{
> +	struct isp_p1_device *p1_dev = p1_ctx_to_dev(data);
> +
> +	disable_sys_clock(p1_dev);
> +	/* Notify PM */
> +	pm_runtime_put_sync(&p1_dev->pdev->dev);
> +}
> +
> +/* ISP P1 interface functions */
> +int mtk_isp_open(struct device *dev)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	s32 usercount = atomic_inc_return(&isp_ctx->isp_user_cnt);
> +	int ret;
> +
> +	dev_dbg(dev, "%s usercount=%d\n", __func__, usercount);
> +
> +	if (usercount == 1) {
> +		ret = isp_setup_scp_rproc(p1_dev);
> +		if (ret)
> +			goto scp_err;
> +
> +		/* ISP HW INIT */
> +		isp_ctx->isp_hw_module = ISP_CAM_B_IDX;
> +		/* Use pure RAW as default HW path */
> +		isp_ctx->isp_raw_path = ISP_PURE_RAW_PATH;
> +		/* Check enabled DMAs which is configured by media setup */
> +		isp_ctx->enable_dma_ports =
> +			get_enable_dma_ports(p1_dev->cam_dev);
> +
> +		if (!isp_ctx->enable_dma_ports) {
> +			dev_dbg(dev, "No DMAs are enabled\n");
> +			ret = -EINVAL;
> +			goto scp_err;
> +		}
> +
> +		pm_runtime_get_sync(dev);
> +
> +		ret = isp_init_context(p1_dev);
> +		if (ret)
> +			goto ctx_err;
> +		ret = isp_composer_init(isp_ctx);
> +		if (ret)
> +			goto composer_err;
> +		ret = isp_composer_hw_init(isp_ctx);
> +		if (ret)
> +			goto composer_err;
> +
> +		isp_composer_meta_config(&p1_dev->isp_ctx,
> +					 isp_ctx->enable_dma_ports);
> +	}
> +
> +	return 0;
> +composer_err:
> +	isp_uninit_context(p1_dev);
> +ctx_err:
> +	pm_runtime_put_sync(dev);
> +scp_err:
> +	atomic_dec_return(&isp_ctx->isp_user_cnt);
> +	return ret;
> +}
> +
> +int mtk_isp_release(struct device *dev)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +
> +	if (atomic_dec_and_test(&p1_dev->isp_ctx.isp_user_cnt)) {
> +		isp_composer_hw_deinit(isp_ctx, composer_deinit_done_cb);
> +		isp_uninit_context(p1_dev);
> +	}
> +
> +	dev_dbg(dev, "%s usercount=%d\n", __func__,
> +		atomic_read(&p1_dev->isp_ctx.isp_user_cnt));
> +
> +	return 0;
> +}
> +
> +int mtk_isp_config(struct device *dev)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct p1_config_param config_param;
> +	struct mtk_cam_dev *cam_dev = p1_dev->cam_dev;
> +	struct v4l2_subdev_format sd_format;
> +	unsigned int sd_width, sd_height;
> +	unsigned int enable_dma_ports, idx;
> +	int ret;
> +
> +	p1_dev->isp_devs[isp_ctx->isp_hw_module].current_frame = 0;
> +	p1_dev->isp_devs[isp_ctx->isp_hw_module].sof_count = 0;
> +
> +	isp_ctx->frame_seq_no = 1;
> +	atomic_set(&isp_ctx->composed_frame_id, 0);
> +
> +	/* Get the enabled DMA ports */
> +	enable_dma_ports = isp_ctx->enable_dma_ports;
> +	dev_dbg(dev, "%s enable_dma_ports:0x%x", __func__, enable_dma_ports);
> +
> +	/* sensor config */
> +	sd_format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	ret = v4l2_subdev_call(cam_dev->sensor,
> +			       pad, get_fmt, NULL, &sd_format);
> +
> +	if (ret) {
> +		dev_dbg(dev, "sensor:%s g_fmt on failed:%d\n",
> +			cam_dev->sensor->entity.name, ret);
> +		return -EPERM;
> +	}
> +
> +	dev_dbg(dev,
> +		"sensor get_fmt ret=%d, w=%d, h=%d, code=0x%x, field=%d, color=%d\n",
> +		ret, sd_format.format.width, sd_format.format.height,
> +		sd_format.format.code, sd_format.format.field,
> +		sd_format.format.colorspace);
> +
> +	config_param.cfg_in_param.continuous = 0x1;
> +	config_param.cfg_in_param.subsample = 0x0;
> +	/* fix to one pixel mode in default */
> +	config_param.cfg_in_param.pixel_mode = one_pixel_mode;
> +	/* support normal pattern in default */
> +	config_param.cfg_in_param.data_pattern = 0x0;
> +
> +	config_param.cfg_in_param.crop.left = 0x0;
> +	config_param.cfg_in_param.crop.top = 0x0;
> +
> +	config_param.cfg_in_param.raw_pixel_id =
> +		get_sensor_pixel_id(sd_format.format.code);
> +	config_param.cfg_in_param.img_fmt =
> +		get_sensor_fmt(sd_format.format.code);
> +	config_param.cfg_in_param.crop.width = sd_format.format.width;
> +	config_param.cfg_in_param.crop.height = sd_format.format.height;
> +	sd_width = sd_format.format.width;
> +	sd_height = sd_format.format.height;
> +
> +	idx = MTK_CAM_P1_MAIN_STREAM_OUT;

The idx variable is unnecessary. Just use MTK_CAM_P1_... to index into
mem2mem2_nodes directly here and below.

> +	if ((enable_dma_ports & R_IMGO) == R_IMGO) {
> +		struct v4l2_format *imgo_fmt =
> +			&p1_dev->cam_dev->mem2mem2_nodes[idx].vdev_fmt;
> +
> +		config_param.cfg_main_param.pure_raw = isp_ctx->isp_raw_path;
> +		config_param.cfg_main_param.pure_raw_pack = 1;
> +		config_param.cfg_main_param.bypass = 0;
> +
> +		config_param.cfg_main_param.output.img_fmt =
> +			get_img_fmt(imgo_fmt->fmt.pix_mp.pixelformat);
> +		config_param.cfg_main_param.output.pixel_byte =
> +			get_pixel_byte(imgo_fmt->fmt.pix_mp.pixelformat);
> +		config_param.cfg_main_param.output.size.w =
> +			imgo_fmt->fmt.pix_mp.width;
> +		config_param.cfg_main_param.output.size.h =
> +			imgo_fmt->fmt.pix_mp.height;
> +
> +		config_param.cfg_main_param.output.size.stride =
> +			imgo_fmt->fmt.pix_mp.plane_fmt[0].bytesperline;
> +		config_param.cfg_main_param.output.size.xsize =
> +			imgo_fmt->fmt.pix_mp.plane_fmt[0].bytesperline;
> +
> +		config_param.cfg_main_param.output.crop.left = 0x0;
> +		config_param.cfg_main_param.output.crop.top = 0x0;
> +
> +		config_param.cfg_main_param.output.crop.width = sd_width;
> +		config_param.cfg_main_param.output.crop.height = sd_height;
> +
> +		WARN_ONCE(imgo_fmt->fmt.pix_mp.width > sd_width ||
> +			  imgo_fmt->fmt.pix_mp.height > sd_height,
> +			  "img out:%d:%d in:%d:%d",
> +			  imgo_fmt->fmt.pix_mp.width,
> +			  imgo_fmt->fmt.pix_mp.height,
> +			  sd_width,
> +			  sd_height);
> +
> +		dev_dbg(dev,
> +			"imgo pixel_byte:%d img_fmt:0x%x raw:%d\n",
> +			config_param.cfg_main_param.output.pixel_byte,
> +			config_param.cfg_main_param.output.img_fmt,
> +			config_param.cfg_main_param.pure_raw);
> +		dev_dbg(dev,
> +			"imgo param:size=%0dx%0d, stride:%d,xsize:%d,crop=%0dx%0d\n",
> +			config_param.cfg_main_param.output.size.w,
> +			config_param.cfg_main_param.output.size.h,
> +			config_param.cfg_main_param.output.size.stride,
> +			config_param.cfg_main_param.output.size.xsize,
> +			config_param.cfg_main_param.output.crop.width,
> +			config_param.cfg_main_param.output.crop.height);
> +	} else {
> +		config_param.cfg_main_param.bypass = 1;
> +	}
> +
> +	idx = MTK_CAM_P1_PACKED_BIN_OUT;
> +	if ((enable_dma_ports & R_RRZO) == R_RRZO) {
> +		struct v4l2_format *rrzo_fmt =
> +			&p1_dev->cam_dev->mem2mem2_nodes[idx].vdev_fmt;
> +
> +		config_param.cfg_resize_param.bypass = 0;
> +		config_param.cfg_resize_param.output.img_fmt =
> +			get_img_fmt(rrzo_fmt->fmt.pix_mp.pixelformat);
> +		config_param.cfg_resize_param.output.pixel_byte =
> +			get_pixel_byte(rrzo_fmt->fmt.pix_mp.pixelformat);
> +		config_param.cfg_resize_param.output.size.w =
> +			rrzo_fmt->fmt.pix_mp.width;
> +		config_param.cfg_resize_param.output.size.h =
> +			rrzo_fmt->fmt.pix_mp.height;
> +		config_param.cfg_resize_param.output.size.stride =
> +			rrzo_fmt->fmt.pix_mp.plane_fmt[0].bytesperline;
> +		config_param.cfg_resize_param.output.size.xsize =
> +			rrzo_fmt->fmt.pix_mp.plane_fmt[0].bytesperline;
> +
> +		config_param.cfg_resize_param.output.crop.left = 0x0;
> +		config_param.cfg_resize_param.output.crop.top = 0x0;
> +		config_param.cfg_resize_param.output.crop.width = sd_width;
> +		config_param.cfg_resize_param.output.crop.height = sd_height;
> +
> +		WARN_ONCE(rrzo_fmt->fmt.pix_mp.width > sd_width ||
> +			  rrzo_fmt->fmt.pix_mp.height > sd_height,
> +			  "rrz out:%d:%d in:%d:%d",
> +			  rrzo_fmt->fmt.pix_mp.width,
> +			  rrzo_fmt->fmt.pix_mp.height,
> +			  sd_width,
> +			  sd_height);
> +
> +		dev_dbg(dev, "rrzo pixel_byte:%d img_fmt:0x%x\n",
> +			config_param.cfg_resize_param.output.pixel_byte,
> +			config_param.cfg_resize_param.output.img_fmt);
> +		dev_dbg(dev,
> +			"rrzo param:size=%0dx%0d,stride:%d,xsize:%d,crop=%0dx%0d\n",
> +			config_param.cfg_resize_param.output.size.w,
> +			config_param.cfg_resize_param.output.size.h,
> +			config_param.cfg_resize_param.output.size.stride,
> +			config_param.cfg_resize_param.output.size.xsize,
> +			config_param.cfg_resize_param.output.crop.width,
> +			config_param.cfg_resize_param.output.crop.height);
> +	} else {
> +		config_param.cfg_resize_param.bypass = 1;
> +	}
> +
> +	/* Configure meta DMAs info. */
> +	config_param.cfg_meta_param.enabled_meta_dmas = enable_dma_ports;
> +
> +	isp_composer_hw_config(isp_ctx, &config_param);
> +
> +	dev_dbg(dev, "%s done\n", __func__);
> +	return 0;
> +}
> +
> +int mtk_isp_enqueue(struct device *dev,
> +		    unsigned int dma_port,
> +		    struct mtk_cam_dev_buffer *buffer)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct mtk_isp_scp_p1_cmd frameparams;
> +
> +	memset(&frameparams, 0, sizeof(frameparams));
> +
> +	frameparams.cmd_id = ISP_CMD_ENQUEUE_META;
> +	frameparams.meta_frame.enabled_dma = dma_port;
> +	frameparams.meta_frame.vb_index = buffer->vbb.vb2_buf.index;
> +	frameparams.meta_frame.meta_addr.iova = buffer->daddr;
> +	frameparams.meta_frame.meta_addr.scp_addr = buffer->scp_addr;
> +
> +	isp_composer_enqueue(isp_ctx, &frameparams, SCP_ISP_CMD);
> +
> +	return 0;
> +}
> +
> +int mtk_isp_req_enqueue(struct device *dev,
> +			struct mtk_cam_dev_start_param *frameparamsbase)
> +{
> +	struct isp_p1_device *p1_dev = get_p1_device(dev);
> +	struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> +	struct p1_frame_param frameparams;
> +	struct mtk_isp_queue_job *framejob;
> +	struct mtk_cam_dev_buffer **bundle_buffers;
> +	unsigned int i, idx;
> +
> +	framejob = kzalloc(sizeof(*framejob), GFP_ATOMIC);
> +	memset(framejob, 0, sizeof(*framejob));
> +	memset(&frameparams, 0, sizeof(frameparams));
> +	INIT_LIST_HEAD(&framejob->list_buf);
> +
> +	bundle_buffers = &frameparamsbase->buffers[0];
> +	frameparams.frame_seq_no = isp_ctx->frame_seq_no++;
> +	frameparams.sof_idx =
> +		p1_dev->isp_devs[isp_ctx->isp_hw_module].sof_count;
> +	framejob->request_fd = frameparamsbase->request_fd;
> +	framejob->frame_seq_no = frameparams.frame_seq_no;
> +
> +	idx = MTK_CAM_P1_META_IN_0;
> +	if (bundle_buffers[idx]) {
> +		frameparams.tuning_addr.iova =
> +			bundle_buffers[idx]->daddr;
> +		frameparams.tuning_addr.scp_addr =
> +			bundle_buffers[idx]->scp_addr;
> +		list_add_tail(&bundle_buffers[idx]->list,
> +			      &framejob->list_buf);
> +	}
> +
> +	/* Image output */
> +	idx = MTK_CAM_P1_MAIN_STREAM_OUT;
> +	if (bundle_buffers[idx]) {
> +		frameparams.img_dma_buffers[0].buffer.iova =
> +			bundle_buffers[idx]->daddr;
> +		frameparams.img_dma_buffers[0].buffer.scp_addr =
> +			bundle_buffers[idx]->scp_addr;
> +		dev_dbg(dev, "main stream address iova:0x%x\n",
> +			frameparams.img_dma_buffers[0].buffer.iova);
> +		list_add_tail(&bundle_buffers[idx]->list,
> +			      &framejob->list_buf);
> +	}
> +
> +	/* Resize output */
> +	idx = MTK_CAM_P1_PACKED_BIN_OUT;
> +	if (bundle_buffers[idx]) {
> +		frameparams.img_dma_buffers[1].buffer.iova =
> +			bundle_buffers[idx]->daddr;
> +		frameparams.img_dma_buffers[1].buffer.scp_addr =
> +			bundle_buffers[idx]->scp_addr;
> +		dev_dbg(dev, "packed out address iova:0x%x\n",
> +			frameparams.img_dma_buffers[1].buffer.iova);
> +		list_add_tail(&bundle_buffers[idx]->list,
> +			      &framejob->list_buf);
> +	}
> +
> +	/* Meta output DMAs */
> +	for (i = 0; i < MAX_META_DMA_NODES; i++) {
> +		idx = MTK_CAM_P1_META_OUT_0 + i;
> +		if (bundle_buffers[idx]) {
> +			frameparams.meta_addrs[i].iova =
> +			  bundle_buffers[idx]->daddr;
> +			frameparams.meta_addrs[i].scp_addr =
> +			  bundle_buffers[idx]->scp_addr;
> +			list_add_tail(&bundle_buffers[idx]->list,
> +				      &framejob->list_buf);
> +		} else {
> +			frameparams.meta_addrs[i].iova = 0;
> +			frameparams.meta_addrs[i].scp_addr = 0;
> +		}
> +	}
> +
> +	spin_lock(&isp_ctx->p1_enqueue_list.lock);
> +	list_add_tail(&framejob->list_entry, &isp_ctx->p1_enqueue_list.queue);
> +	atomic_inc(&isp_ctx->p1_enqueue_list.queue_cnt);
> +	spin_unlock(&isp_ctx->p1_enqueue_list.lock);
> +
> +	isp_composer_enqueue(isp_ctx, &frameparams, SCP_ISP_FRAME);
> +	dev_dbg(dev, "request fd:%d frame_seq_no:%d is queued cnt:%d\n",
> +		frameparamsbase->request_fd,
> +		frameparams.frame_seq_no,
> +		atomic_read(&isp_ctx->p1_enqueue_list.queue_cnt));
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops mtk_isp_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(mtk_isp_suspend, mtk_isp_resume)
> +	SET_RUNTIME_PM_OPS(mtk_isp_suspend, mtk_isp_resume, NULL)
> +};
> +
> +static struct platform_driver mtk_isp_driver = {
> +	.probe   = mtk_isp_probe,
> +	.remove  = mtk_isp_remove,
> +	.driver  = {
> +		.name  = "mtk-cam",
> +		.of_match_table = of_match_ptr(mtk_isp_of_ids),
> +		.pm     = &mtk_isp_pm_ops,
> +	}
> +};
> +
> +module_platform_driver(mtk_isp_driver);
> +
> +MODULE_DESCRIPTION("Camera ISP driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h
> new file mode 100644
> index 000000000000..6cf8bb4ba93a
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h
> @@ -0,0 +1,300 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Ryan Yu <ryan.yu@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __CAMERA_ISP_H
> +#define __CAMERA_ISP_H
> +
> +#include <linux/cdev.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioctl.h>
> +#include <linux/irqreturn.h>
> +#include <linux/miscdevice.h>
> +#include <linux/pm_qos.h>
> +#include <linux/scatterlist.h>
> +
> +#include "mtk_cam-dev.h"
> +#include "mtk_cam-scp.h"
> +
> +#define CAM_A_MAX_WIDTH		3328U
> +#define CAM_A_MAX_HEIGHT	2496U
> +#define CAM_B_MAX_WIDTH		5376U
> +#define CAM_B_MAX_HEIGHT	4032U
> +
> +#define CAM_MIN_WIDTH		80U
> +#define CAM_MIN_HEIGHT		60U
> +
> +#define IMG_MAX_WIDTH		CAM_B_MAX_WIDTH
> +#define IMG_MAX_HEIGHT		CAM_B_MAX_HEIGHT
> +#define IMG_MIN_WIDTH		CAM_MIN_WIDTH
> +#define IMG_MIN_HEIGHT		CAM_MIN_HEIGHT
> +
> +#define RRZ_MAX_WIDTH		CAM_B_MAX_WIDTH
> +#define RRZ_MAX_HEIGHT		CAM_B_MAX_HEIGHT
> +#define RRZ_MIN_WIDTH		CAM_MIN_WIDTH
> +#define RRZ_MIN_HEIGHT		CAM_MIN_HEIGHT
> +
> +#define R_IMGO		BIT(0)
> +#define R_RRZO		BIT(1)
> +#define R_AAO		BIT(3)
> +#define R_AFO		BIT(4)
> +#define R_LCSO		BIT(5)
> +#define R_PDO		BIT(6)
> +#define R_LMVO		BIT(7)
> +#define R_FLKO		BIT(8)
> +#define R_RSSO		BIT(9)
> +#define R_PSO		BIT(10)
> +
> +#define ISP_COMPOSING_MAX_NUM		4
> +#define ISP_FRAME_COMPOSING_MAX_NUM	3
> +
> +#define IRQ_DATA_BUF_SIZE		4
> +#define COMPOSRE_EVENT_BUF_SIZE		4
> +
> +#define CQ_ADDRESS_OFFSET		0x640
> +#define CQ_BUFFER_COUNT			3

Please align macro values using tabs.

> +
> +#define IRQ_STAT_STR "cam%c, SOF_%d irq(0x%x), " \
> +			"dma(0x%x), frame_num(%d)/cq_num(%d)\n"
> +
> +/*
> + * In order with the sequence of device nodes defined in dtsi rule,
> + * one hardware module should be mapping to one node.
> + */
> +enum isp_dev_node_enum {
> +	ISP_CAMSYS_CONFIG_IDX = 0,
> +	ISP_CAM_UNI_IDX,
> +	ISP_CAM_A_IDX,
> +	ISP_CAM_B_IDX,
> +	ISP_DEV_NODE_NUM
> +};
> +
> +/* Image RAW path for ISP P1 module. */
> +enum isp_raw_path_enum {
> +	ISP_PROCESS_RAW_PATH = 0,
> +	ISP_PURE_RAW_PATH
> +};
> +
> +enum {
> +	img_fmt_unknown		= 0x0000,
> +	img_fmt_raw_start	= 0x2200,
> +	img_fmt_bayer8		= img_fmt_raw_start,
> +	img_fmt_bayer10,
> +	img_fmt_bayer12,
> +	img_fmt_bayer14,
> +	img_fmt_fg_bayer8,
> +	img_fmt_fg_bayer10,
> +	img_fmt_fg_bayer12,
> +	img_fmt_fg_bayer14,
> +};
> +
> +enum {
> +	raw_pxl_id_b   = 0,
> +	raw_pxl_id_gb,
> +	raw_pxl_id_gr,
> +	raw_pxl_id_r
> +};
> +
> +enum {
> +	default_pixel_mode = 0,
> +	one_pixel_mode,
> +	two_pixel_mode,
> +	four_pixel_mode,
> +	pixel_mode_num,
> +};
> +
> +enum mtk_isp_scp_ipi_type {
> +	SCP_ISP_CMD = 0,
> +	SCP_ISP_FRAME,
> +};
> +
> +struct isp_queue {
> +	struct list_head queue;
> +	atomic_t queue_cnt;
> +	spinlock_t lock; /* queue attributes protection */
> +};
> +
> +struct isp_thread {
> +	struct task_struct *thread;
> +	wait_queue_head_t wq;
> +};
> +
> +struct mtk_isp_queue_work {
> +	union {
> +		struct mtk_isp_scp_p1_cmd cmd;
> +		struct p1_frame_param frameparams;
> +	};
> +	struct list_head list_entry;
> +	enum mtk_isp_scp_ipi_type type;
> +};
> +
> +struct mtk_cam_dev_stat_event_data {
> +	__u32 frame_seq_no;
> +	__u32 meta0_vb2_index;
> +	__u32 meta1_vb2_index;
> +	__u32 irq_status_mask;
> +	__u32 dma_status_mask;
> +};
> +
> +struct mtk_isp_queue_job {
> +	struct list_head list_entry;
> +	struct list_head list_buf;
> +	unsigned int request_fd;
> +	unsigned int frame_seq_no;
> +};
> +
> +struct isp_clk_struct {
> +	int num_clks;
> +	struct clk_bulk_data *clk_list;
> +};
> +
> +struct isp_device {
> +	struct device *dev;
> +	void __iomem *regs;
> +	int irq;
> +	spinlock_t spinlock_irq; /* ISP reg setting integrity */
> +	unsigned int current_frame;
> +	unsigned int meta0_vb2_index;
> +	unsigned int meta1_vb2_index;
> +	u8 sof_count;
> +	u8 isp_hw_module;
> +};
> +
> +struct mtk_isp_p1_ctx {
> +	struct isp_queue composer_txlist;
> +	struct isp_thread composer_tx_thread;
> +	atomic_t cmd_queued;
> +	struct mutex composer_tx_lock; /* isp composer work protection */
> +
> +	struct isp_thread composer_rx_thread;
> +	struct mtk_isp_scp_p1_cmd composer_evts[COMPOSRE_EVENT_BUF_SIZE];
> +	atomic_t composer_evts_start;
> +	atomic_t composer_evts_end;
> +	spinlock_t composer_evts_lock; /* SCP events protection */
> +	/* increase after ipi */
> +	atomic_t ipi_occupied;
> +	/* increase after frame enqueue */
> +	atomic_t composing_frame;
> +	/* current composed frame id */
> +	atomic_t composed_frame_id;
> +
> +	struct isp_queue p1_enqueue_list;
> +
> +	struct isp_thread isp_deque_thread;
> +	struct mtk_cam_dev_stat_event_data irq_event_datas[IRQ_DATA_BUF_SIZE];
> +	atomic_t irq_data_start;
> +	atomic_t irq_data_end;
> +	spinlock_t irq_dequeue_lock; /* ISP frame dequeuq protection */
> +
> +	dma_addr_t scp_mem_pa;
> +	dma_addr_t scp_mem_iova;
> +	struct sg_table sgtable;
> +
> +	/* increase after open, decrease when close */
> +	atomic_t isp_user_cnt;
> +	/* frame sequence number, increase per en-queue*/
> +	int frame_seq_no;
> +	unsigned int isp_hw_module;
> +	unsigned int isp_raw_path;
> +	unsigned int enable_dma_ports;
> +
> +	void (*composer_deinit_donecb)(void *isp_ctx);
> +
> +	struct list_head list;
> +};
> +
> +struct isp_p1_device {
> +	struct platform_device *pdev;
> +
> +	/* for SCP driver  */
> +	struct platform_device *scp_pdev;
> +	struct rproc *rproc_handle;
> +
> +	struct mtk_isp_p1_ctx isp_ctx;
> +	struct isp_clk_struct isp_clk;

What's the benefit of having mtk_isp_p1_ctx and isp_clk_struct in
separate structs? They are only ever used in isp_p1_device.

> +	struct mtk_cam_dev *cam_dev;
> +	struct isp_device *isp_devs;

Similarly, why are these allocated at runtime rather then just members
of the struct?

In mtk_isp_probe the struct isp_p1_device is allocated, and immediately
afterwards the struct mtk_cam_dev and struct isp_devices are. There will
only ever be ISP_DEV_NODE_NUM isp_devices.

Could this be changed to:
	struct mtk_cam_dev cam_dev;
	struct isp_device isp_devs[ISP_DEV_NODE_NUM];
?

> +};
> +
> +static inline struct isp_p1_device *
> +p1_ctx_to_dev(const struct mtk_isp_p1_ctx *__p1_ctx)
> +{
> +	return container_of(__p1_ctx, struct isp_p1_device, isp_ctx);
> +}
> +
> +static inline struct isp_p1_device *get_p1_device(struct device *dev)
> +{
> +	return ((struct isp_p1_device *)dev_get_drvdata(dev));
> +}
> +
> +int isp_composer_init(struct mtk_isp_p1_ctx *isp_ctx);
> +int isp_composer_hw_init(struct mtk_isp_p1_ctx *isp_ctx);
> +void isp_composer_meta_config(struct mtk_isp_p1_ctx *isp_ctx,
> +			      unsigned int dma);
> +void isp_composer_hw_config(struct mtk_isp_p1_ctx *isp_ctx,
> +			    struct p1_config_param *config_param);
> +void isp_composer_stream(struct mtk_isp_p1_ctx *isp_ctx, int on);
> +void isp_composer_hw_deinit(struct mtk_isp_p1_ctx *isp_ctx,
> +			    void (*donecb)(void *data));
> +void isp_composer_enqueue(struct mtk_isp_p1_ctx *isp_ctx,
> +			  void *data,
> +			  enum mtk_isp_scp_ipi_type type);

These functions are declared here, but implemented in mtk_cam-scp.c.
Can the funtion declarations be moved to mtk_cam-scp.h?

> +
> +/**
> + * mtk_isp_open - open isp driver and initialize related resources.
> + *
> + * @dev:	isp device.
> + *
> + */
> +int mtk_isp_open(struct device *dev);
> +
> +/**
> + * mtk_isp_release - release isp driver and related resources.
> + *
> + * @dev:	isp device.
> + *
> + */
> +int mtk_isp_release(struct device *dev);
> +
> +/**
> + * mtk_isp_config - output image & meta data configuration.
> + *
> + * @dev:	isp device.
> + *
> + */
> +int mtk_isp_config(struct device *dev);
> +
> +/**
> + * mtk_isp_req_enqueue - enqueue a frame bundle (per-frame basis) to ISP driver.
> + *
> + * @dev:	isp device.
> + * @frameparamsbase: pointer to &struct mtk_cam_dev_start_param.
> + *
> + */
> +int mtk_isp_req_enqueue(struct device *dev,
> +			struct mtk_cam_dev_start_param *frameparamsbase);
> +
> +/**
> + * mtk_isp_enqueue - enqueue a single frame to ISP driver
> + * for non-per-frame DMA.
> + *
> + * @dev:	isp device.
> + * @buffer: pointer to &struct mtk_cam_dev_buffer.
> + *
> + */
> +int mtk_isp_enqueue(struct device *dev,
> +		    unsigned int dma_idx,
> +		    struct mtk_cam_dev_buffer *buffer);
> +#endif /*__CAMERA_ISP_H*/
> -- 
> 2.18.0
>