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

Drew Davenport
2019-05-24T21:20:15Z

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
>