<html><body><p>
<pre>
On Mon, 2025-07-07 at 09:31 +0800, shangyao lin wrote:
> From: "shangyao.lin" <shangyao.lin@mediatek.com>
>
> Introduce state management and debugging mechanisms for the MediaTek ISP7.x
> camsys platform. State management establishes control over ISP operations and
> events, defining distinct states for request handling, sensor control, and
> frame synchronization, ensuring proper event processing. The debugging
> mechanism ensures stable operation and timely data collection during anomalies.
>
> ---
>
> Changes in v2:
> - Removed mtk_cam-debug.c and mtk_cam-debug.h, along with related code
> - Various fixes per review comments
>
> Signed-off-by: shangyao.lin <shangyao.lin@mediatek.com>
> ---
> .../mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c | 1409 +++++++++++++++++
> .../mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.h | 131 ++
> 2 files changed, 1540 insertions(+)
> create mode 100755 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c

Why this file is executable?

> create mode 100755 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.h

Ditto.

>
> diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c
> new file mode 100755
> index 000000000000..15af1eac5444
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c
> @@ -0,0 +1,1409 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.

2025

> + */
> +
> +#include <linux/list.h>
> +#include <linux/math64.h>
> +#include <linux/of.h>
> +#include <linux/videodev2.h>
> +#include <linux/hrtimer.h>

Alphabetic order.

> +
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-ioctl.h>

Alphabetic order.

> +
> +#include "mtk_cam.h"
> +#include "mtk_cam-ctrl.h"
> +#include "mtk_cam-pool.h"
> +#include "mtk_cam-raw.h"
> +

Drop this blank line.

> +#include "mtk_cam-regs-mt8188.h"
> +

Drop this blank line.

> +#include "mtk_camera-v4l2-controls.h"
> +
> +#define SENSOR_SET_DEADLINE_MS 18
> +#define SENSOR_SET_RESERVED_MS 7
> +#define SENSOR_SET_DEADLINE_MS_60FPS 6
> +#define SENSOR_SET_RESERVED_MS_60FPS 6
> +#define STATE_NUM_AT_SOF 3
> +#define INITIAL_DROP_FRAME_CNT 1
> +
> +enum MTK_CAMSYS_STATE_RESULT {
> +STATE_RESULT_TRIGGER_CQ = 0,
> +STATE_RESULT_PASS_CQ_INIT,
> +STATE_RESULT_PASS_CQ_SW_DELAY,
> +STATE_RESULT_PASS_CQ_SCQ_DELAY,
> +STATE_RESULT_PASS_CQ_HW_DELAY,

After call mtk_camsys_raw_state_handle(), just check STATE_RESULT_TRIGGER_CQ or not.
There result is just for print error log.
Direct print error in mtk_camsys_raw_state_handle() and drop these definition.

> +};
> +
> +void state_transition(struct mtk_camsys_ctrl_state *state_entry,

static void state_transition(struct mtk_camsys_ctrl_state *state_entry,

> + enum MTK_CAMSYS_STATE_IDX from,
> + enum MTK_CAMSYS_STATE_IDX to)
> +{
> +if (state_entry->estate == from)
> +state_entry->estate = to;
> +}
> +

[snip]

> +void mtk_cam_set_sensor_full(struct mtk_cam_request_stream_data *s_data,
> + struct mtk_camsys_sensor_ctrl *sensor_ctrl)
> +{
> +struct mtk_cam_device *cam;
> +struct mtk_cam_ctx *ctx;
> +struct mtk_cam_request *req;
> +struct mtk_raw_device *raw_dev;
> +unsigned int time_after_sof = 0;
> +
> +/* EnQ this request's state element to state_list (STATE:READY) */
> +spin_lock(&sensor_ctrl->camsys_state_lock);
> +list_add_tail(&s_data->state.state_element,
> + &sensor_ctrl->camsys_state_list);
> +atomic_set(&sensor_ctrl->sensor_request_seq_no, s_data->frame_seq_no);
> +spin_unlock(&sensor_ctrl->camsys_state_lock);
> +
> +/* sensor_worker task */
> +ctx = mtk_cam_s_data_get_ctx(s_data);
> +cam = ctx->cam;
> +req = mtk_cam_s_data_get_req(s_data);
> +
> +dev_dbg(cam->dev, "%s:%s:ctx(%d) req(%d):sensor try set start\n",
> +__func__, req->req.debug_str, ctx->stream_id, s_data->frame_seq_no);
> +
> +if (ctx->used_raw_num && mtk_cam_req_frame_sync_start(req))
> +dev_dbg(cam->dev,
> +"%s:%s:ctx(%d): sensor ctrl with frame sync - start\n",
> +__func__, req->req.debug_str, ctx->stream_id);
> +
> +/*
> + * request setup
> + * 1st frame sensor setting is treated like normal frame
> + * and is set with other settings like max fps.
> + * 2nd is special, only exposure is set.
> + */
> +if (s_data->flags & MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_EN) {

In patch [8/13] I says ctx->sensor is always NULL, and MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_EN is set only when ctx->sensor is not NULL.
So drop MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_EN.

> +v4l2_ctrl_request_setup(&req->req,
> +s_data->sensor->ctrl_handler);
> +time_after_sof =
> +div_u64(ktime_get_boottime_ns(), 1000000) -
> +ctx->sensor_ctrl.sof_time;
> +dev_dbg(cam->dev,
> +"[SOF+%dms] Sensor request:%d[ctx:%d] setup\n",
> +time_after_sof, s_data->frame_seq_no,
> +ctx->stream_id);
> +}
> +
> +state_transition(&s_data->state, E_STATE_READY, E_STATE_SENSOR);
> +
> +if (ctx->used_raw_num && mtk_cam_req_frame_sync_end(req))
> +dev_dbg(cam->dev,
> +"%s:ctx(%d): sensor ctrl with frame sync - stop\n",
> +__func__, ctx->stream_id);
> +
> +if (ctx->used_raw_num) {
> +raw_dev = get_main_raw_dev(ctx->cam, ctx->pipe);
> +if (raw_dev && atomic_read(&raw_dev->vf_en) == 0 &&
> + ctx->sensor_ctrl.initial_cq_done == 1 &&
> + s_data->frame_seq_no == 1)
> +mtk_cam_stream_on(raw_dev, ctx);
> +}
> +
> +dev_dbg(cam->dev, "%s:%s:ctx(%d)req(%d):sensor done at SOF+%dms\n",
> +__func__, req->req.debug_str, ctx->stream_id,
> +s_data->frame_seq_no, time_after_sof);
> +
> +mtk_cam_complete_sensor_hdl(s_data);
> +}
> +

[snip]

> +static int
> +mtk_camsys_raw_state_handle(struct mtk_raw_device *raw_dev,
> + struct mtk_camsys_sensor_ctrl *sensor_ctrl,
> + struct mtk_camsys_ctrl_state **current_state,
> + struct mtk_camsys_irq_info *irq_info)
> +{
> +struct mtk_cam_ctx *ctx = sensor_ctrl->ctx;
> +struct mtk_camsys_ctrl_state *state_temp, *state_outer = NULL;
> +struct mtk_camsys_ctrl_state *state_inner = NULL;
> +struct mtk_camsys_ctrl_state *state_rec[STATE_NUM_AT_SOF];
> +struct mtk_cam_request_stream_data *req_stream_data;
> +int frame_idx_inner = irq_info->frame_idx_inner;
> +int stateidx;
> +int que_cnt = 0;
> +int write_cnt;
> +u64 time_boot = ktime_get_boottime_ns();
> +u64 time_mono = ktime_get_ns();
> +int working_req_found = 0;
> +
> +/* List state-queue status*/
> +spin_lock(&sensor_ctrl->camsys_state_lock);
> +list_for_each_entry(state_temp, &sensor_ctrl->camsys_state_list,
> + state_element) {
> +req_stream_data = mtk_cam_ctrl_state_to_req_s_data(state_temp);
> +stateidx = atomic_read(&sensor_ctrl->sensor_request_seq_no) -
> + req_stream_data->frame_seq_no;
> +if (stateidx < STATE_NUM_AT_SOF && stateidx > -1) {
> +state_rec[stateidx] = state_temp;
> +if (stateidx == 0)
> +working_req_found = 1;
> +
> +/* Find outer state element */
> +if (state_temp->estate == E_STATE_OUTER ||
> + state_temp->estate == E_STATE_CAMMUX_OUTER ||
> + state_temp->estate == E_STATE_OUTER_HW_DELAY) {
> +state_outer = state_temp;
> +mtk_cam_set_timestamp(req_stream_data,
> + time_boot, time_mono);
> +}
> +
> +/* Find inner state element request */
> +if (state_temp->estate == E_STATE_INNER ||
> + state_temp->estate == E_STATE_INNER_HW_DELAY)
> +state_inner = state_temp;
> +
> +dev_dbg(raw_dev->dev,
> +"[SOF] STATE_CHECK [N-%d] Req:%d / State:%d\n",
> +stateidx, req_stream_data->frame_seq_no,
> +state_rec[stateidx]->estate);
> +}
> +/* counter for state queue*/
> +que_cnt++;
> +}
> +spin_unlock(&sensor_ctrl->camsys_state_lock);
> +
> +/* HW imcomplete case */
> +if (state_inner) {
> +req_stream_data = mtk_cam_ctrl_state_to_req_s_data(state_inner);
> +write_cnt = (atomic_read(&sensor_ctrl->isp_request_seq_no) / 256)
> +* 256 + irq_info->write_cnt;
> +
> +if (frame_idx_inner > atomic_read(&sensor_ctrl->isp_request_seq_no) ||
> + atomic_read(&req_stream_data->frame_done_work.is_queued) == 1) {
> +dev_dbg_ratelimited(raw_dev->dev,
> + "[SOF] frame done work too late frames. req(%d),ts(%llu)\n",
> + req_stream_data->frame_seq_no,
> + irq_info->ts_ns);
> +} else if (write_cnt >= req_stream_data->frame_seq_no) {
> +dev_info_ratelimited(raw_dev->dev,
> + "[SOF] frame done reading lost %d frames. req(%d),ts(%llu)\n",
> + write_cnt - req_stream_data->frame_seq_no + 1,
> + req_stream_data->frame_seq_no,
> + irq_info->ts_ns);
> +mtk_cam_set_timestamp(req_stream_data,
> + time_boot - 1000, time_mono - 1000);
> +mtk_camsys_frame_done(ctx, write_cnt, ctx->stream_id);
> +} else if ((write_cnt >= req_stream_data->frame_seq_no - 1) &&
> + irq_info->fbc_cnt == 0) {
> +dev_info_ratelimited(raw_dev->dev,
> + "[SOF] frame done reading lost frames. req(%d),ts(%llu)\n",
> + req_stream_data->frame_seq_no, irq_info->ts_ns);
> +mtk_cam_set_timestamp(req_stream_data,
> + time_boot - 1000, time_mono - 1000);
> +mtk_camsys_frame_done(ctx, write_cnt + 1, ctx->stream_id);
> +} else {
> +state_transition(state_inner,
> + E_STATE_INNER, E_STATE_INNER_HW_DELAY);
> +if (state_outer) {
> +state_transition(state_outer,
> + E_STATE_OUTER,
> + E_STATE_OUTER_HW_DELAY);
> +state_transition(state_outer,
> + E_STATE_CAMMUX_OUTER,
> + E_STATE_OUTER_HW_DELAY);
> +}
> +dev_info_ratelimited(raw_dev->dev,
> + "[SOF] HW_IMCOMPLETE state cnt(%d,%d),req(%d),ts(%llu)\n",
> + write_cnt, irq_info->write_cnt,
> + req_stream_data->frame_seq_no,
> + irq_info->ts_ns);
> +return STATE_RESULT_PASS_CQ_HW_DELAY;
> +}
> +}
> +
> +/* Transit outer state to inner state */
> +if (state_outer && sensor_ctrl->sensorsetting_wq) {
> +req_stream_data = mtk_cam_ctrl_state_to_req_s_data(state_outer);
> +if (atomic_read(&sensor_ctrl->initial_drop_frame_cnt) == 0 &&
> + req_stream_data->frame_seq_no > frame_idx_inner) {
> +dev_info(raw_dev->dev,
> + "[SOF-noDBLOAD] HW delay outer_no:%d, inner_idx:%d <= processing_idx:%d,ts:%llu\n",
> + req_stream_data->frame_seq_no, frame_idx_inner,
> + atomic_read(&sensor_ctrl->isp_request_seq_no),
> + irq_info->ts_ns);
> +return STATE_RESULT_PASS_CQ_HW_DELAY;
> +}
> +
> +if (atomic_read(&sensor_ctrl->initial_drop_frame_cnt) == 0 &&
> + req_stream_data->frame_seq_no == frame_idx_inner) {
> +if (frame_idx_inner >
> + atomic_read(&sensor_ctrl->isp_request_seq_no)) {
> +state_transition(state_outer,
> + E_STATE_OUTER_HW_DELAY,
> + E_STATE_INNER_HW_DELAY);
> +state_transition(state_outer,
> + E_STATE_OUTER,
> + E_STATE_INNER);
> +state_transition(state_outer,
> + E_STATE_CAMMUX_OUTER,
> + E_STATE_INNER);

State would change to E_STATE_INNER only in mtk_camsys_raw_state_handle().
mtk_camsys_raw_state_handle() is called only when ctx->sensor is not NULL.
In patch [8/13] I say ctx-sensor is always NULL,
so state would never change to E_STATE_INNER.
Drop it.

> +atomic_set(&sensor_ctrl->isp_request_seq_no, frame_idx_inner);
> +dev_dbg(raw_dev->dev,
> +"[SOF-DBLOAD] frame_seq_no:%d, OUTER->INNER state:%d,ts:%llu\n",
> +req_stream_data->frame_seq_no,
> +state_outer->estate, irq_info->ts_ns);
> +}
> +}
> +}
> +
> +/* Trigger high resolution timer to try sensor setting */
> +sensor_ctrl->sof_time = div_u64(irq_info->ts_ns, 1000000);
> +mtk_cam_sof_timer_setup(ctx);
> +
> +if (que_cnt > 1 && state_rec[1]) {
> +state_temp = state_rec[1];
> +req_stream_data = mtk_cam_ctrl_state_to_req_s_data(state_temp);
> +if (req_stream_data->frame_seq_no == 1)
> +state_transition(state_temp,
> + E_STATE_SENSOR, E_STATE_INNER);
> +}
> +
> +if (que_cnt > 0) {
> +if (working_req_found && state_rec[0]) {
> +if (state_rec[0]->estate == E_STATE_READY) {
> +dev_info(raw_dev->dev, "[SOF] sensor delay ts:%llu\n",
> + irq_info->ts_ns);
> +req_stream_data =
> +mtk_cam_ctrl_state_to_req_s_data(state_rec[0]);
> +req_stream_data->flags |=
> +MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_DELAYED;
> +}
> +
> +if (state_rec[0]->estate == E_STATE_SENINF)
> +dev_info(raw_dev->dev, "[SOF] sensor switch delay\n");
> +
> +/* CQ triggering judgment*/
> +if (state_rec[0]->estate == E_STATE_SENSOR) {
> +*current_state = state_rec[0];
> +return STATE_RESULT_TRIGGER_CQ;
> +}
> +/* last SCQ triggering delay judgment*/
> +if (state_rec[0]->estate == E_STATE_CQ_SCQ_DELAY) {
> +state_transition(state_rec[0],
> + E_STATE_CQ_SCQ_DELAY,
> + E_STATE_OUTER);
> +dev_info(raw_dev->dev, "[SOF] SCQ_DELAY state:%d ts:%llu\n",
> + state_rec[0]->estate, irq_info->ts_ns);
> +return STATE_RESULT_PASS_CQ_SCQ_DELAY;
> +}
> +} else {
> +dev_dbg(raw_dev->dev, "[SOF] working request not found\n");
> +}
> +}
> +return STATE_RESULT_PASS_CQ_SW_DELAY;
> +}
> +
> +static void mtk_camsys_raw_frame_start(struct mtk_raw_device *raw_dev,
> + struct mtk_cam_ctx *ctx,
> + struct mtk_camsys_irq_info *irq_info)
> +{
> +unsigned int dequeued_frame_seq_no = irq_info->frame_idx_inner;
> +struct mtk_cam_request_stream_data *req_stream_data;
> +struct mtk_camsys_sensor_ctrl *sensor_ctrl = &ctx->sensor_ctrl;
> +struct mtk_cam_working_buf_entry *buf_entry;
> +struct mtk_camsys_ctrl_state *current_state;
> +struct mtk_cam_buffer *buf;
> +
> +dma_addr_t base_addr;
> +enum MTK_CAMSYS_STATE_RESULT state_handle_ret;
> +
> +/* touch watchdog*/
> +if (watchdog_scenario(ctx))

In patch [8/13] I says ctx->sensor is always NULL.
watchdog_scenario() return true only when ctx->sensor is not NULL,
so watchdog_scenario() would always return false.
Drop all watchdog related code.

> +mtk_ctx_watchdog_kick(ctx);
> +/* inner register dequeue number */
> +ctx->dequeued_frame_seq_no = dequeued_frame_seq_no;
> +/* Send V4L2_EVENT_FRAME_SYNC event */
> +mtk_cam_event_frame_sync(ctx->pipe, dequeued_frame_seq_no);
> +
> +/* Handle directly enque buffers */
> +spin_lock(&ctx->cam->dma_processing_lock);
> +list_for_each_entry(buf, &ctx->cam->dma_processing, list) {
> +if (buf->state.estate == E_BUF_STATE_COMPOSED) {
> +spin_unlock(&ctx->cam->dma_processing_lock);
> +goto apply_cq;
> +}
> +}
> +spin_unlock(&ctx->cam->dma_processing_lock);
> +buf = NULL;
> +current_state = NULL;
> +
> +/* Find request of this dequeued frame */
> +req_stream_data =
> +mtk_cam_get_req_s_data(ctx, ctx->stream_id, dequeued_frame_seq_no);
> +
> +if (ctx->sensor) {

In patch [8/13] I says ctx->sensor is always NULL, so drop these code.

> +state_handle_ret =
> +mtk_camsys_raw_state_handle(raw_dev, sensor_ctrl,
> + &current_state, irq_info);

mtk_camsys_raw_state_handle() is called only here, but ctx->sensor is always NULL, so mtk_camsys_raw_state_handle() would never be called.
Drop mtk_camsys_raw_state_handle().

> +
> +if (state_handle_ret != STATE_RESULT_TRIGGER_CQ) {
> +dev_dbg(raw_dev->dev, "[SOF] CQ drop s:%d deq:%d\n",
> +state_handle_ret, dequeued_frame_seq_no);
> +return;
> +}
> +}
> +
> +apply_cq:
> +/* Update CQ base address if needed */
> +if (ctx->composed_frame_seq_no <= dequeued_frame_seq_no) {
> +dev_info_ratelimited(raw_dev->dev,
> + "SOF[ctx:%d-#%d], CQ isn't updated [composed_frame_deq (%d) ts:%llu]\n",
> + ctx->stream_id, dequeued_frame_seq_no,
> + ctx->composed_frame_seq_no, irq_info->ts_ns);
> +return;
> +}
> +/* apply next composed buffer */
> +spin_lock(&ctx->composed_buffer_list.lock);
> +if (list_empty(&ctx->composed_buffer_list.list)) {
> +dev_info_ratelimited(raw_dev->dev,
> + "SOF_INT_ST, no buffer update, cq_num:%d, frame_seq:%d, ts:%llu\n",
> + ctx->composed_frame_seq_no, dequeued_frame_seq_no,
> + irq_info->ts_ns);
> +spin_unlock(&ctx->composed_buffer_list.lock);
> +} else {
> +buf_entry = list_first_entry(&ctx->composed_buffer_list.list,
> + struct mtk_cam_working_buf_entry,
> + list_entry);
> +list_del(&buf_entry->list_entry);
> +ctx->composed_buffer_list.cnt--;
> +spin_unlock(&ctx->composed_buffer_list.lock);
> +spin_lock(&ctx->processing_buffer_list.lock);
> +list_add_tail(&buf_entry->list_entry,
> + &ctx->processing_buffer_list.list);
> +ctx->processing_buffer_list.cnt++;
> +spin_unlock(&ctx->processing_buffer_list.lock);
> +base_addr = buf_entry->buffer.iova;
> +mtk_cam_raw_apply_cq(raw_dev, base_addr,
> + buf_entry->cq_desc_size,
> + buf_entry->cq_desc_offset,
> + buf_entry->sub_cq_desc_size,
> + buf_entry->sub_cq_desc_offset);
> +
> +if (buf) {
> +buf->state.estate = E_BUF_STATE_CQ;
> +return;
> +}
> +
> +/* Transit state from Sensor -> CQ */
> +if (ctx->sensor) {
> +/* req_stream_data of req_cq*/
> +req_stream_data = mtk_cam_ctrl_state_to_req_s_data(current_state);
> +state_transition(current_state,
> + E_STATE_SENSOR, E_STATE_CQ);

In patch [8/13] I says ctx->sensor is always NULL, so these code would not execute.
This is the only code that change state to E_STATE_CQ but it never execute,
so E_STATE_CQ is useless. Drop it.

> +
> +dev_dbg(raw_dev->dev,
> +"SOF[ctx:%d-#%d], CQ-%d is update, composed:%d, cq_addr:0x%pad, time:%lld, monotime:%lld\n",
> +ctx->stream_id, dequeued_frame_seq_no,
> +req_stream_data->frame_seq_no,
> +ctx->composed_frame_seq_no, &base_addr,
> +req_stream_data->timestamp,
> +req_stream_data->timestamp_mono);
> +}
> +}
> +}
> +

[snip]

> +void mtk_camsys_frame_done(struct mtk_cam_ctx *ctx,
> + unsigned int frame_seq_no,
> + unsigned int pipe_id)

static void mtk_camsys_frame_done(

> +{
> +struct mtk_cam_req_work *frame_done_work;
> +struct mtk_cam_request_stream_data *req_stream_data;
> +struct mtk_cam_buffer *buf;
> +struct mtk_cam_working_buf_entry *buf_entry = NULL;
> +bool is_pending_buffer = false;
> +
> +spin_lock(&ctx->cam->dma_processing_lock);
> +if (!list_empty(&ctx->cam->dma_processing)) {
> +buf = list_first_entry(&ctx->cam->dma_processing,
> + struct mtk_cam_buffer, list);
> +list_del(&buf->list);
> +ctx->cam->dma_processing_count--;
> +vb2_buffer_done(&buf->vbb.vb2_buf, VB2_BUF_STATE_DONE);
> +is_pending_buffer = true;
> +}
> +spin_unlock(&ctx->cam->dma_processing_lock);
> +if (is_pending_buffer) {
> +spin_lock(&ctx->processing_buffer_list.lock);
> +if (!list_empty(&ctx->processing_buffer_list.list)) {
> +buf_entry =
> +list_first_entry(&ctx->processing_buffer_list.list,
> + struct mtk_cam_working_buf_entry,
> + list_entry);
> +list_del(&buf_entry->list_entry);
> +ctx->processing_buffer_list.cnt--;
> +}
> +spin_unlock(&ctx->processing_buffer_list.lock);
> +if (buf_entry)
> +mtk_cam_working_buf_put(buf_entry);
> +
> +mtk_cam_buf_try_queue(ctx);
> +return;
> +}
> +
> +req_stream_data = mtk_cam_get_req_s_data(ctx, pipe_id, frame_seq_no);
> +if (!req_stream_data) {
> +dev_err(ctx->cam->dev,
> + "%s:ctx-%d:pipe-%d:req(%d) not found!\n",
> + __func__, ctx->stream_id, pipe_id, frame_seq_no);
> +return;
> +}
> +
> +if (atomic_read(&req_stream_data->frame_done_work.is_queued)) {
> +dev_info(ctx->cam->dev,
> + "already queue done work req:%d seq:%d pipe_id:%d\n",
> + req_stream_data->frame_seq_no, frame_seq_no, pipe_id);
> +return;
> +}
> +
> +atomic_set(&req_stream_data->frame_done_work.is_queued, 1);
> +frame_done_work = &req_stream_data->frame_done_work;
> +queue_work(ctx->frame_done_wq, &frame_done_work->work);
> +}
> +

[snip]

> +static bool mtk_camsys_is_all_cq_done(struct mtk_cam_ctx *ctx,
> + unsigned int pipe_id)
> +{
> +unsigned int all_subdevs = 0;
> +bool ret = false;
> +
> +spin_lock(&ctx->first_cq_lock);
> +if (ctx->is_first_cq_done) {
> +ret = true;
> +spin_unlock(&ctx->first_cq_lock);
> +goto EXIT;
> +}
> +
> +// update cq done status

Run checkpatch.pl before you send out patch.
checkpatch would tell you not to use c++ style comment.

> +ctx->cq_done_status |= (1 << pipe_id);
> +
> +// check cq done status
> +if (ctx->used_raw_num)
> +all_subdevs |= (1 << ctx->pipe->id);
> +if ((ctx->cq_done_status & all_subdevs) == all_subdevs) {
> +ctx->is_first_cq_done = 1;
> +ret = true;
> +}
> +spin_unlock(&ctx->first_cq_lock);
> +dev_info(ctx->cam->dev,
> + "[1st-CQD] all done:%d, pipe_id:%d (using raw:%d)\n",
> + ctx->is_first_cq_done, pipe_id, ctx->used_raw_num);

Why print is_first_cq_done?
I think this is not necessary.

> +EXIT:
> +return ret;
> +}
> +
> +static int mtk_camsys_event_handle_raw(struct mtk_cam_device *cam,
> + unsigned int engine_id,
> + struct mtk_camsys_irq_info *irq_info)
> +{
> +struct mtk_raw_device *raw_dev;
> +struct mtk_cam_ctx *ctx;
> +
> +raw_dev = dev_get_drvdata(cam->raw.devs[engine_id]);
> +ctx = mtk_cam_find_ctx(cam, &raw_dev->pipeline->subdev.entity);
> +if (!ctx) {
> +dev_err(raw_dev->dev, "cannot find ctx\n");
> +return -EINVAL;
> +}
> +
> +/* raw's CQ done */
> +if (irq_info->irq_type & 1 << CAMSYS_IRQ_SETTING_DONE) {
> +if (mtk_camsys_is_all_cq_done(ctx, ctx->pipe->id))
> +mtk_camsys_raw_cq_done(raw_dev, ctx,
> + irq_info->frame_idx);
> +}
> +
> +/* raw's DMA done, we only allow AFO done here */
> +if (irq_info->irq_type & 1 << CAMSYS_IRQ_AFO_DONE)
> +mtk_cam_meta1_done(ctx, ctx->dequeued_frame_seq_no, ctx->stream_id);
> +
> +/* raw's SW done */
> +if (irq_info->irq_type & 1 << CAMSYS_IRQ_FRAME_DONE) {
> +mtk_camsys_frame_done(ctx, ctx->dequeued_frame_seq_no,
> +ctx->stream_id);
> +}
> +/* raw's SOF */
> +if (irq_info->irq_type & 1 << CAMSYS_IRQ_FRAME_START) {
> +if (atomic_read(&raw_dev->vf_en) == 0) {
> +dev_info(raw_dev->dev, "skip sof event when vf off\n");
> +return 0;
> +}
> +mtk_camsys_raw_frame_start(raw_dev, ctx, irq_info);
> +}
> +
> +return 0;
> +}
> +

[snip]

> +
> +struct mtk_camsys_irq_normal_data {
> +/* with normal_status */

mtk_camsys_irq_normal_data is nothing. Drop it.

> +};
> +
> +struct mtk_camsys_irq_error_data {
> +/* with error_status */
> +int err_status;
> +};
> +
> +struct mtk_camsys_irq_info {
> +enum MTK_CAMSYS_IRQ_EVENT irq_type;
> +u64 ts_ns;
> +int frame_idx;
> +int frame_idx_inner;
> +bool sub_engine;

sub_engine is useless. Drop it.

> +int write_cnt;
> +int fbc_cnt;
> +union {
> +struct mtk_camsys_irq_normal_data n;
> +struct mtk_camsys_irq_error_data e;

e is never used. Drop it.

> +};
> +};
> +
> +enum MTK_CAMSYS_STATE_IDX {
> +/* For state analysis and controlling for request */
> +E_STATE_READY = 0x0,
> +E_STATE_SENINF,

state never change to E_STATE_SENINF. Drop it.

> +E_STATE_SENSOR,
> +E_STATE_CQ,
> +E_STATE_OUTER,
> +E_STATE_CAMMUX_OUTER_CFG,

state never change to E_STATE_CAMMUX_OUTER_CFG. Drop it.

> +E_STATE_CAMMUX_OUTER,

state never change to E_STATE_CAMMUX_OUTER. Drop it.

> +E_STATE_INNER,
> +E_STATE_DONE_NORMAL,
> +E_STATE_CQ_SCQ_DELAY,
> +E_STATE_CAMMUX_OUTER_CFG_DELAY,

state never change to E_STATE_CAMMUX_OUTER_CFG_DELAY. Drop it.

> +E_STATE_OUTER_HW_DELAY,
> +E_STATE_INNER_HW_DELAY,
> +E_STATE_DONE_MISMATCH,
> +};
> +
> +struct mtk_camsys_ctrl_state {
> +enum MTK_CAMSYS_STATE_IDX estate;
> +struct list_head state_element;
> +};
> +
> +struct mtk_camsys_link_ctrl {

mtk_camsys_link_ctrl is useless. Drop it.

> +struct mtk_raw_pipeline *pipe;
> +struct media_pad remote;
> +struct mtk_cam_ctx *swapping_ctx;
> +u8 active;
> +u8 wait_exchange;
> +};
> +
> +/* per stream from sensor */
> +struct mtk_camsys_sensor_ctrl {
> +struct mtk_cam_ctx *ctx;
> +struct kthread_worker *sensorsetting_wq;
> +struct kthread_work work;
> +struct hrtimer sensor_deadline_timer;
> +u64 sof_time;
> +int timer_req_sensor;
> +int timer_req_event;
> +atomic_t sensor_enq_seq_no;
> +atomic_t sensor_request_seq_no;
> +atomic_t isp_request_seq_no;
> +atomic_t isp_enq_seq_no;
> +atomic_t last_drained_seq_no;
> +int initial_cq_done;
> +atomic_t initial_drop_frame_cnt;
> +struct list_head camsys_state_list;
> +spinlock_t camsys_state_lock; /* protect camsys_state_list */
> +/* link change ctrl */
> +struct mtk_camsys_link_ctrl link_ctrl;

link_ctrl is useless. Drop it.

> +struct mtk_cam_request *link_change_req;

link_change_req is useless. Drop it.

Regards,
CK

> +};
> +



</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice
 ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe
 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->