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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <767085562b5efb43f248e8528bb154a6c30d3999.camel@mediatek.com>
Date: Fri, 22 Nov 2024 08:41:32 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "mchehab@...nel.org" <mchehab@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
	Andy Hsieh (謝智皓) <Andy.Hsieh@...iatek.com>,
	Julien Stephan <jstephan@...libre.com>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "laurent.pinchart@...asonboard.com"
	<laurent.pinchart@...asonboard.com>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"paul.elder@...asonboard.com" <paul.elder@...asonboard.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "fsylvestre@...libre.com"
	<fsylvestre@...libre.com>, "pnguyen@...libre.com" <pnguyen@...libre.com>
Subject: Re: [PATCH v7 4/5] media: platform: mediatek: isp: add mediatek
 ISP3.0 camsv

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@...libre.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@...libre.com>
> Signed-off-by: Florian Sylvestre <fsylvestre@...libre.com>
> [Paul Elder fix irq locking]
> Signed-off-by: Paul Elder <paul.elder@...asonboard.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Co-developed-by: Julien Stephan <jstephan@...libre.com>
> Signed-off-by: Julien Stephan <jstephan@...libre.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;
> +}
> +