Re: [PATCH v7 4/5] media: platform: mediatek: isp: add mediatek ISP3.0 camsv

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

Hi, Julien:

On Thu, 2024-11-21 at 09:53 +0100, Julien Stephan wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> From: Phi-bang Nguyen <pnguyen@baylibre.com>
> 
> This driver provides a path to bypass the SoC ISP so that image data
> coming from the SENINF can go directly into memory without any image
> processing. This allows the use of an external ISP.
> 
> Signed-off-by: Phi-bang Nguyen <pnguyen@baylibre.com>
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> [Paul Elder fix irq locking]
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Co-developed-by: Julien Stephan <jstephan@baylibre.com>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---

[snip]

> +static irqreturn_t isp_irq_camsv30(int irq, void *data)
> +{
> +       struct mtk_cam_dev *cam_dev = (struct mtk_cam_dev *)data;
> +       struct mtk_cam_dev_buffer *buf;
> +       unsigned int irq_status;
> +
> +       spin_lock(&cam_dev->buf_list_lock);
> +
> +       irq_status = mtk_camsv30_read(cam_dev, CAMSV_INT_STATUS);
> +
> +       if (irq_status & INT_ST_MASK_CAMSV_ERR)
> +               dev_err(cam_dev->dev, "irq error 0x%lx\n",
> +                       irq_status & INT_ST_MASK_CAMSV_ERR);
> +
> +       /* De-queue frame */
> +       if (irq_status & CAMSV_IRQ_PASS1_DON) {
> +               cam_dev->sequence++;
> +
> +               buf = list_first_entry_or_null(&cam_dev->buf_list,
> +                                              struct mtk_cam_dev_buffer,
> +                                              list);
> +               if (buf) {
> +                       buf->v4l2_buf.sequence = cam_dev->sequence;
> +                       buf->v4l2_buf.vb2_buf.timestamp =
> +                               ktime_get_ns();
> +                       vb2_buffer_done(&buf->v4l2_buf.vb2_buf,
> +                                       VB2_BUF_STATE_DONE);
> +                       list_del(&buf->list);
> +               }
> +
> +               buf = list_first_entry_or_null(&cam_dev->buf_list,
> +                                              struct mtk_cam_dev_buffer,
> +                                              list);
> +               if (buf)
> +                       mtk_camsv30_update_buffers_add(cam_dev, buf);

If buf == NULL, so hardware would automatically stop DMA?
I don't know how this hardware work.
Below is my imagine about this hardware.

1. Software use CAMSV_IMGO_FBC_RCNT_INC to increase software buffer index.
2. Hardware has a hardware buffer index. After hardware finish one frame, hardware buffer index increase.
3. After software buffer index increase, hardware start DMA.
4. When hardware buffer index is equal to software buffer index, hardware automatically stop DMA.

Does the hardware work as my imagine?
If hardware could automatically stop DMA, add comment to describe.
If hardware could not automatically stop DMA, software should do something to stop DMA when buf == NULL.

Regards,
CK

> +       }
> +
> +       spin_unlock(&cam_dev->buf_list_lock);
> +
> +       return IRQ_HANDLED;
> +}
> +