* [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
@ 2019-09-09 19:22 ` frederic.chen
2019-09-13 21:48 ` Rob Herring
2019-10-02 9:22 ` Sakari Ailus
2019-09-09 19:22 ` [RFC PATCH V3 2/5] dts: arm64: mt8183: Add DIP nodes frederic.chen
` (5 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: frederic.chen @ 2019-09-09 19:22 UTC (permalink / raw)
To: hans.verkuil, laurent.pinchart+renesas, tfiga, matthias.bgg, mchehab
Cc: shik, devicetree, Sean.Cheng, Rynn.Wu, Allan.Yang,
srv_heupstream, holmes.chiou, suleiman, Jerry-ch.Chen, jungo.lin,
sj.huang, yuzhao, linux-mediatek, zwisler, christie.yu,
frederic.chen, linux-arm-kernel, linux-media
From: Frederic Chen <frederic.chen@mediatek.com>
This patch adds DT binding documentation for the Digital Image
Processing (DIP) unit of camera ISP system on Mediatek's SoCs.
It depends on the SCP and MDP 3 patch as following:
1. dt-bindings: Add a binding for Mediatek SCP
https://patchwork.kernel.org/patch/11027247/
2. dt-binding: mt8183: Add Mediatek MDP3 dt-bindings
https://patchwork.kernel.org/patch/10945603/
Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
---
.../bindings/media/mediatek,mt8183-dip.txt | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8183-dip.txt
diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8183-dip.txt b/Documentation/devicetree/bindings/media/mediatek,mt8183-dip.txt
new file mode 100644
index 000000000000..3a0435513089
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8183-dip.txt
@@ -0,0 +1,40 @@
+* Mediatek Digital Image Processor (DIP)
+
+Digital Image Processor (DIP) unit in Mediatek ISP system is responsible for
+image content adjustment according to the tuning parameters. DIP can process
+the image form memory buffer and output the processed image to multiple output
+buffers. Furthermore, it can support demosaicing and noise reduction on the
+images.
+
+Required properties:
+- compatible: "mediatek,mt8183-dip"
+- reg: Physical base address and length of the function block register space
+- interrupts: interrupt number to the cpu
+- iommus: should point to the respective IOMMU block with master port as
+ argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
+ for details.
+- mediatek,mdp3: should point to the respective mdp block. DIP hardware
+ connects to MDP and we can get the processed image with both effect of the
+ two blocks.
+- mediatek,larb: must contain the local arbiters in the current SoCs, see
+ Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
+ for details.
+- mediatek,scp: should point to the scp node since the we use SCP
+ coprocessor to control DIP hardware
+- clocks: must contain the local arbiters 5 (LARB5) and DIP clock
+- clock-names: must contain "larb5" and "dip"
+
+Example:
+ dip: dip@15022000 {
+ compatible = "mediatek,mt8183-dip";
+ mediatek,larb = <&larb5>;
+ mediatek,mdp3 = <&mdp_rdma0>;
+ mediatek,scp = <&scp>;
+ iommus = <&iommu M4U_PORT_CAM_IMGI>;
+ reg = <0 0x15022000 0 0x6000>;
+ interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&imgsys CLK_IMG_LARB5>,
+ <&imgsys CLK_IMG_DIP>;
+ clock-names = "larb5",
+ "dip";
+ };
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH V3 2/5] dts: arm64: mt8183: Add DIP nodes
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
2019-09-09 19:22 ` [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings frederic.chen
@ 2019-09-09 19:22 ` frederic.chen
2019-09-09 19:22 ` [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig frederic.chen
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: frederic.chen @ 2019-09-09 19:22 UTC (permalink / raw)
To: hans.verkuil, laurent.pinchart+renesas, tfiga, matthias.bgg, mchehab
Cc: shik, devicetree, Sean.Cheng, Rynn.Wu, Allan.Yang,
srv_heupstream, holmes.chiou, suleiman, Jerry-ch.Chen, jungo.lin,
sj.huang, yuzhao, linux-mediatek, zwisler, christie.yu,
frederic.chen, linux-arm-kernel, linux-media
From: Frederic Chen <frederic.chen@mediatek.com>
This patch adds nodes for Digital Image Processing (DIP). DIP is
embedded in Mediatek SoCs and works with the co-processor to
adjust image content according to tuning input data. It also
provides image format conversion, resizing, and rotation
features.
Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 0f2646c9eb65..d7b0fb8230f0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -513,6 +513,22 @@
#clock-cells = <1>;
};
+ dip: dip@15022000 {
+ compatible = "mediatek,mt8183-dip";
+ mediatek,larb = <&larb5>;
+ mediatek,mdp3 = <&mdp_rdma0>;
+ mediatek,scp = <&scp>;
+ iommus = <&iommu M4U_PORT_CAM_IMGI>;
+ reg = <0 0x15022000 0 0x6000>;
+ interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_LOW>;
+ clocks =
+ <&imgsys CLK_IMG_LARB5>,
+ <&imgsys CLK_IMG_DIP>;
+ clock-names =
+ "larb5",
+ "dip";
+ };
+
vdecsys: syscon@16000000 {
compatible = "mediatek,mt8183-vdecsys", "syscon";
reg = <0 0x16000000 0 0x1000>;
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
2019-09-09 19:22 ` [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings frederic.chen
2019-09-09 19:22 ` [RFC PATCH V3 2/5] dts: arm64: mt8183: Add DIP nodes frederic.chen
@ 2019-09-09 19:22 ` frederic.chen
2019-10-02 9:20 ` Sakari Ailus
2019-09-09 19:22 ` [RFC PATCH V3 5/5] media: platform: mtk-mdp3: Add struct tuning_addr and img_sw_buffer frederic.chen
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: frederic.chen @ 2019-09-09 19:22 UTC (permalink / raw)
To: hans.verkuil, laurent.pinchart+renesas, tfiga, matthias.bgg, mchehab
Cc: shik, devicetree, Sean.Cheng, Rynn.Wu, Allan.Yang,
srv_heupstream, holmes.chiou, suleiman, Jerry-ch.Chen, jungo.lin,
sj.huang, yuzhao, linux-mediatek, zwisler, christie.yu,
frederic.chen, linux-arm-kernel, linux-media
From: Frederic Chen <frederic.chen@mediatek.com>
This patch adds KConfig for Mediatek Digital Image Processing
driver(DIP). DIP is embedded in Mediatek SoCs. It provides
image format conversion, resizing, and rotation function.
Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
---
drivers/media/platform/Kconfig | 2 ++
drivers/media/platform/mtk-isp/Kconfig | 19 +++++++++++++++++++
2 files changed, 21 insertions(+)
create mode 100644 drivers/media/platform/mtk-isp/Kconfig
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 0c725d4dcf80..b8501e1b134f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -32,6 +32,8 @@ source "drivers/media/platform/davinci/Kconfig"
source "drivers/media/platform/omap/Kconfig"
+source "drivers/media/platform/mtk-isp/Kconfig"
+
config VIDEO_ASPEED
tristate "Aspeed AST2400 and AST2500 Video Engine driver"
depends on VIDEO_V4L2
diff --git a/drivers/media/platform/mtk-isp/Kconfig b/drivers/media/platform/mtk-isp/Kconfig
new file mode 100644
index 000000000000..a4267db70c24
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/Kconfig
@@ -0,0 +1,19 @@
+config VIDEO_MEDIATEK_ISP_DIP
+ bool "Mediatek Digital Image Processing function"
+ select VIDEO_V4L2_SUBDEV_API
+ select VIDEOBUF2_DMA_CONTIG
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_V4L2
+ select VIDEOBUF2_MEMOPS
+ select MEDIA_CONTROLLER
+
+ default n
+ help
+ Support the basic Digital Image Processing (DIP) driver.
+
+ DIP driver provides image format conversion, resizing,
+ and rotation function through the low power hardware.
+ DIP also supports multiple output feature. It can
+ generate two or more output image with different effect
+ based on a single input image at the same time.
+
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH V3 5/5] media: platform: mtk-mdp3: Add struct tuning_addr and img_sw_buffer
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
` (2 preceding siblings ...)
2019-09-09 19:22 ` [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig frederic.chen
@ 2019-09-09 19:22 ` frederic.chen
2019-10-02 9:21 ` Sakari Ailus
[not found] ` <20190909192244.9367-5-frederic.chen@mediatek.com>
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: frederic.chen @ 2019-09-09 19:22 UTC (permalink / raw)
To: hans.verkuil, laurent.pinchart+renesas, tfiga, matthias.bgg, mchehab
Cc: shik, devicetree, Sean.Cheng, Rynn.Wu, Allan.Yang,
srv_heupstream, holmes.chiou, suleiman, Jerry-ch.Chen, jungo.lin,
sj.huang, yuzhao, linux-mediatek, zwisler, christie.yu,
frederic.chen, linux-arm-kernel, linux-media
From: Frederic Chen <frederic.chen@mediatek.com>
We added a struct tuning_addr which contains a field "present"
so that the driver can tell the firmware if we have user tuning
dataor not.
The strcut img_sw_buffer is also added. This struct has no cpu address
field and uses a handle instead so that we don't pass a cpu address
to co-processor.
Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
---
drivers/media/platform/mtk-mdp3/mtk-img-ipi.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp3/mtk-img-ipi.h b/drivers/media/platform/mtk-mdp3/mtk-img-ipi.h
index 9fabe7e8b71d..f61e61faf636 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-img-ipi.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-img-ipi.h
@@ -38,6 +38,12 @@ struct img_addr {
u32 iova; /* Used by IOMMU HW access */
} __attribute__ ((__packed__));
+struct tuning_addr {
+ u32 present;
+ u32 pa; /* Used by CM4 access */
+ u32 iova; /* Used by IOMMU HW access */
+} __attribute__ ((__packed__));
+
struct img_sw_addr {
u64 va; /* Used by APMCU access */
u32 pa; /* Used by CM4 access */
@@ -105,16 +111,21 @@ struct img_ipi_frameparam {
u64 drv_data;
struct img_input inputs[IMG_MAX_HW_INPUTS];
struct img_output outputs[IMG_MAX_HW_OUTPUTS];
- struct img_addr tuning_data;
+ struct tuning_addr tuning_data;
struct img_addr subfrm_data;
struct img_sw_addr config_data;
struct img_sw_addr self_data;
/* u8 pq_data[]; */
} __attribute__ ((__packed__));
+struct img_sw_buffer {
+ u64 handle; /* Used by APMCU access */
+ u32 scp_addr; /* Used by CM4 access */
+} __attribute__ ((__packed__));
+
struct img_ipi_param {
u8 usage;
- struct img_sw_addr frm_param;
+ struct img_sw_buffer frm_param;
} __attribute__ ((__packed__));
struct img_frameparam {
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver
[not found] ` <20190909192244.9367-5-frederic.chen@mediatek.com>
@ 2019-09-10 4:04 ` Tomasz Figa
2019-09-11 17:41 ` Frederic Chen
0 siblings, 1 reply; 15+ messages in thread
From: Tomasz Figa @ 2019-09-10 4:04 UTC (permalink / raw)
To: Frederic Chen (陳俊元)
Cc: Shik Chen, devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, Rynn Wu (吳育恩),
Christie Yu (游雅惠),
srv_heupstream, Allan Yang (楊智鈞),
Holmes Chiou (邱挺),
suleiman, Jerry-ch Chen, Jungo Lin (林明俊),
Sj Huang, yuzhao, Hans Verkuil, zwisler, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
Hi Frederic,
On Tue, Sep 10, 2019 at 4:23 AM <frederic.chen@mediatek.com> wrote:
>
> From: Frederic Chen <frederic.chen@mediatek.com>
>
> This patch adds the driver of Digital Image Processing (DIP)
> unit in Mediatek ISP system, providing image format
> conversion, resizing, and rotation features.
>
> The mtk-isp directory will contain drivers for multiple IP
> blocks found in Mediatek ISP system. It will include ISP
> Pass 1 driver(CAM), sensor interface driver, DIP driver and
> face detection driver.
>
> Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> ---
> drivers/media/platform/mtk-isp/Makefile | 7 +
> .../media/platform/mtk-isp/isp_50/Makefile | 7 +
> .../platform/mtk-isp/isp_50/dip/Makefile | 18 +
> .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.c | 650 +++++
> .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.h | 566 +++++
> .../platform/mtk-isp/isp_50/dip/mtk_dip-hw.h | 156 ++
> .../platform/mtk-isp/isp_50/dip/mtk_dip-sys.c | 521 ++++
> .../mtk-isp/isp_50/dip/mtk_dip-v4l2.c | 2255 +++++++++++++++++
> 8 files changed, 4180 insertions(+)
> create mode 100644 drivers/media/platform/mtk-isp/Makefile
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/Makefile
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.c
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.h
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-hw.h
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-sys.c
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-v4l2.c
>
Thanks for sending v3!
I'm going to do a full review a bit later, but please check one
comment about power handling below.
Other than that one comment, from a quick look, I think we only have a
number of style issues left. Thanks for the hard work!
[snip]
> +static void dip_runner_func(struct work_struct *work)
> +{
> + struct mtk_dip_request *req = mtk_dip_hw_mdp_work_to_req(work);
> + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> + struct img_config *config_data =
> + (struct img_config *)req->working_buf->config_data.vaddr;
> +
> + /*
> + * Call MDP/GCE API to do HW excecution
> + * Pass the framejob to MDP driver
> + */
> + pm_runtime_get_sync(dip_dev->dev);
> + mdp_cmdq_sendtask(dip_dev->mdp_pdev, config_data,
> + &req->img_fparam.frameparam, NULL, false,
> + dip_mdp_cb_func, req);
> +}
[snip]
> +static void dip_composer_workfunc(struct work_struct *work)
> +{
> + struct mtk_dip_request *req = mtk_dip_hw_fw_work_to_req(work);
> + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> + struct img_ipi_param ipi_param;
> + struct mtk_dip_hw_subframe *buf;
> + int ret;
> +
> + down(&dip_dev->sem);
> +
> + buf = mtk_dip_hw_working_buf_alloc(req->dip_pipe->dip_dev);
> + if (!buf) {
> + dev_err(req->dip_pipe->dip_dev->dev,
> + "%s:%s:req(%p): no free working buffer available\n",
> + __func__, req->dip_pipe->desc->name, req);
> + }
> +
> + req->working_buf = buf;
> + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> + &buf->buffer);
> + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> + &buf->config_data);
> + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> +
> + if (!req->img_fparam.frameparam.tuning_data.present) {
> + /*
> + * When user enqueued without tuning buffer,
> + * it would use driver internal buffer.
> + */
> + dev_dbg(dip_dev->dev,
> + "%s: frame_no(%d) has no tuning_data\n",
> + __func__, req->img_fparam.frameparam.frame_no);
> +
> + mtk_dip_wbuf_to_ipi_tuning_addr
> + (&req->img_fparam.frameparam.tuning_data,
> + &buf->tuning_buf);
> + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> + }
> +
> + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> + &buf->frameparam);
> + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> + sizeof(req->img_fparam.frameparam));
> + ipi_param.usage = IMG_IPI_FRAME;
> + ipi_param.frm_param.handle = req->id;
> + ipi_param.frm_param.scp_addr = (u32)buf->frameparam.scp_daddr;
> +
> + mutex_lock(&dip_dev->hw_op_lock);
> + atomic_inc(&dip_dev->num_composing);
> + ret = scp_ipi_send(dip_dev->scp_pdev, SCP_IPI_DIP, &ipi_param,
> + sizeof(ipi_param), 0);
We're not holding the pm_runtime enable count here
(pm_runtime_get_sync() wasn't called), so rproc_shutdown() might have
been called. Wouldn't that affect the ability for this IPI to run?
We had a related discussion with Jerry on the FD series and I think
the conclusion is:
a) if there is any state that needs to be preserved between jobs, that
would be cleared by rproc_shutdown() then we need to call
rproc_boot/shutdown() when we start/stop streaming.
b) it there is no such state, we can keep them inside runtime PM
callbacks, but we need to call pm_runtime_get_sync() before sending an
IPI and pm_runtime_mark_last_busy() + pm_runtime_put_autosuspend()
after the SCP signals completion. In this case the runtime PM
autosuspend delay should be set to around 2-3 times the delay needed
for rproc_shutdown() + rproc_boot() to complete.
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver
2019-09-10 4:04 ` [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver Tomasz Figa
@ 2019-09-11 17:41 ` Frederic Chen
2019-09-12 5:58 ` Tomasz Figa
0 siblings, 1 reply; 15+ messages in thread
From: Frederic Chen @ 2019-09-11 17:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: Shik Chen, devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, Rynn Wu (吳育恩),
Christie Yu (游雅惠),
srv_heupstream, Allan Yang (楊智鈞),
Holmes Chiou (邱挺),
suleiman, Jerry-ch Chen, Jungo Lin (林明俊),
Sj Huang, yuzhao, Hans Verkuil, zwisler, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
Linux Media Mailing List
Hi Tomasz,
I appreciate your helpful comments.
On Tue, 2019-09-10 at 13:04 +0900, Tomasz Figa wrote:
> Hi Frederic,
>
> On Tue, Sep 10, 2019 at 4:23 AM <frederic.chen@mediatek.com> wrote:
> >
> > From: Frederic Chen <frederic.chen@mediatek.com>
> >
> > This patch adds the driver of Digital Image Processing (DIP)
> > unit in Mediatek ISP system, providing image format
> > conversion, resizing, and rotation features.
> >
> > The mtk-isp directory will contain drivers for multiple IP
> > blocks found in Mediatek ISP system. It will include ISP
> > Pass 1 driver(CAM), sensor interface driver, DIP driver and
> > face detection driver.
> >
> > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> > ---
> > drivers/media/platform/mtk-isp/Makefile | 7 +
> > .../media/platform/mtk-isp/isp_50/Makefile | 7 +
> > .../platform/mtk-isp/isp_50/dip/Makefile | 18 +
> > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.c | 650 +++++
> > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.h | 566 +++++
> > .../platform/mtk-isp/isp_50/dip/mtk_dip-hw.h | 156 ++
> > .../platform/mtk-isp/isp_50/dip/mtk_dip-sys.c | 521 ++++
> > .../mtk-isp/isp_50/dip/mtk_dip-v4l2.c | 2255 +++++++++++++++++
> > 8 files changed, 4180 insertions(+)
> > create mode 100644 drivers/media/platform/mtk-isp/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.c
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.h
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-hw.h
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-sys.c
> > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-v4l2.c
> >
>
> Thanks for sending v3!
>
> I'm going to do a full review a bit later, but please check one
> comment about power handling below.
>
> Other than that one comment, from a quick look, I think we only have a
> number of style issues left. Thanks for the hard work!
>
> [snip]
> > +static void dip_runner_func(struct work_struct *work)
> > +{
> > + struct mtk_dip_request *req = mtk_dip_hw_mdp_work_to_req(work);
> > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > + struct img_config *config_data =
> > + (struct img_config *)req->working_buf->config_data.vaddr;
> > +
> > + /*
> > + * Call MDP/GCE API to do HW excecution
> > + * Pass the framejob to MDP driver
> > + */
> > + pm_runtime_get_sync(dip_dev->dev);
> > + mdp_cmdq_sendtask(dip_dev->mdp_pdev, config_data,
> > + &req->img_fparam.frameparam, NULL, false,
> > + dip_mdp_cb_func, req);
> > +}
> [snip]
> > +static void dip_composer_workfunc(struct work_struct *work)
> > +{
> > + struct mtk_dip_request *req = mtk_dip_hw_fw_work_to_req(work);
> > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > + struct img_ipi_param ipi_param;
> > + struct mtk_dip_hw_subframe *buf;
> > + int ret;
> > +
> > + down(&dip_dev->sem);
> > +
> > + buf = mtk_dip_hw_working_buf_alloc(req->dip_pipe->dip_dev);
> > + if (!buf) {
> > + dev_err(req->dip_pipe->dip_dev->dev,
> > + "%s:%s:req(%p): no free working buffer available\n",
> > + __func__, req->dip_pipe->desc->name, req);
> > + }
> > +
> > + req->working_buf = buf;
> > + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> > + &buf->buffer);
> > + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> > + &buf->config_data);
> > + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> > +
> > + if (!req->img_fparam.frameparam.tuning_data.present) {
> > + /*
> > + * When user enqueued without tuning buffer,
> > + * it would use driver internal buffer.
> > + */
> > + dev_dbg(dip_dev->dev,
> > + "%s: frame_no(%d) has no tuning_data\n",
> > + __func__, req->img_fparam.frameparam.frame_no);
> > +
> > + mtk_dip_wbuf_to_ipi_tuning_addr
> > + (&req->img_fparam.frameparam.tuning_data,
> > + &buf->tuning_buf);
> > + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> > + }
> > +
> > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> > + &buf->frameparam);
> > + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> > + sizeof(req->img_fparam.frameparam));
> > + ipi_param.usage = IMG_IPI_FRAME;
> > + ipi_param.frm_param.handle = req->id;
> > + ipi_param.frm_param.scp_addr = (u32)buf->frameparam.scp_daddr;
> > +
> > + mutex_lock(&dip_dev->hw_op_lock);
> > + atomic_inc(&dip_dev->num_composing);
> > + ret = scp_ipi_send(dip_dev->scp_pdev, SCP_IPI_DIP, &ipi_param,
> > + sizeof(ipi_param), 0);
>
> We're not holding the pm_runtime enable count here
> (pm_runtime_get_sync() wasn't called), so rproc_shutdown() might have
> been called. Wouldn't that affect the ability for this IPI to run?
>
> We had a related discussion with Jerry on the FD series and I think
> the conclusion is:
> a) if there is any state that needs to be preserved between jobs, that
> would be cleared by rproc_shutdown() then we need to call
> rproc_boot/shutdown() when we start/stop streaming.
> b) it there is no such state, we can keep them inside runtime PM
> callbacks, but we need to call pm_runtime_get_sync() before sending an
> IPI and pm_runtime_mark_last_busy() + pm_runtime_put_autosuspend()
> after the SCP signals completion. In this case the runtime PM
> autosuspend delay should be set to around 2-3 times the delay needed
> for rproc_shutdown() + rproc_boot() to complete.
Since each IMG_IPI_FRAME command is stateless, I would like to
use pm_runtime_get_sync()/ pm_runtime_mark_last_busy()/
pm_runtime_put_autosuspend() to fix this issue (solution b).
>
> Best regards,
> Tomasz
Sincerely,
Frederic Chen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver
2019-09-11 17:41 ` Frederic Chen
@ 2019-09-12 5:58 ` Tomasz Figa
2019-09-19 9:41 ` Frederic Chen
0 siblings, 1 reply; 15+ messages in thread
From: Tomasz Figa @ 2019-09-12 5:58 UTC (permalink / raw)
To: Frederic Chen
Cc: Shik Chen, devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, Rynn Wu (吳育恩),
Christie Yu (游雅惠),
srv_heupstream, Allan Yang (楊智鈞),
Holmes Chiou (邱挺),
suleiman, Jerry-ch Chen, Jungo Lin (林明俊),
Sj Huang, yuzhao, Hans Verkuil, zwisler, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
On Thu, Sep 12, 2019 at 2:41 AM Frederic Chen
<frederic.chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> I appreciate your helpful comments.
>
>
> On Tue, 2019-09-10 at 13:04 +0900, Tomasz Figa wrote:
> > Hi Frederic,
> >
> > On Tue, Sep 10, 2019 at 4:23 AM <frederic.chen@mediatek.com> wrote:
> > >
> > > From: Frederic Chen <frederic.chen@mediatek.com>
> > >
> > > This patch adds the driver of Digital Image Processing (DIP)
> > > unit in Mediatek ISP system, providing image format
> > > conversion, resizing, and rotation features.
> > >
> > > The mtk-isp directory will contain drivers for multiple IP
> > > blocks found in Mediatek ISP system. It will include ISP
> > > Pass 1 driver(CAM), sensor interface driver, DIP driver and
> > > face detection driver.
> > >
> > > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> > > ---
> > > drivers/media/platform/mtk-isp/Makefile | 7 +
> > > .../media/platform/mtk-isp/isp_50/Makefile | 7 +
> > > .../platform/mtk-isp/isp_50/dip/Makefile | 18 +
> > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.c | 650 +++++
> > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.h | 566 +++++
> > > .../platform/mtk-isp/isp_50/dip/mtk_dip-hw.h | 156 ++
> > > .../platform/mtk-isp/isp_50/dip/mtk_dip-sys.c | 521 ++++
> > > .../mtk-isp/isp_50/dip/mtk_dip-v4l2.c | 2255 +++++++++++++++++
> > > 8 files changed, 4180 insertions(+)
> > > create mode 100644 drivers/media/platform/mtk-isp/Makefile
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/Makefile
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.c
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.h
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-hw.h
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-sys.c
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-v4l2.c
> > >
> >
> > Thanks for sending v3!
> >
> > I'm going to do a full review a bit later, but please check one
> > comment about power handling below.
> >
> > Other than that one comment, from a quick look, I think we only have a
> > number of style issues left. Thanks for the hard work!
> >
> > [snip]
> > > +static void dip_runner_func(struct work_struct *work)
> > > +{
> > > + struct mtk_dip_request *req = mtk_dip_hw_mdp_work_to_req(work);
> > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > + struct img_config *config_data =
> > > + (struct img_config *)req->working_buf->config_data.vaddr;
> > > +
> > > + /*
> > > + * Call MDP/GCE API to do HW excecution
> > > + * Pass the framejob to MDP driver
> > > + */
> > > + pm_runtime_get_sync(dip_dev->dev);
> > > + mdp_cmdq_sendtask(dip_dev->mdp_pdev, config_data,
> > > + &req->img_fparam.frameparam, NULL, false,
> > > + dip_mdp_cb_func, req);
> > > +}
> > [snip]
> > > +static void dip_composer_workfunc(struct work_struct *work)
> > > +{
> > > + struct mtk_dip_request *req = mtk_dip_hw_fw_work_to_req(work);
> > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > + struct img_ipi_param ipi_param;
> > > + struct mtk_dip_hw_subframe *buf;
> > > + int ret;
> > > +
> > > + down(&dip_dev->sem);
> > > +
> > > + buf = mtk_dip_hw_working_buf_alloc(req->dip_pipe->dip_dev);
> > > + if (!buf) {
> > > + dev_err(req->dip_pipe->dip_dev->dev,
> > > + "%s:%s:req(%p): no free working buffer available\n",
> > > + __func__, req->dip_pipe->desc->name, req);
> > > + }
> > > +
> > > + req->working_buf = buf;
> > > + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> > > + &buf->buffer);
> > > + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> > > + &buf->config_data);
> > > + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> > > +
> > > + if (!req->img_fparam.frameparam.tuning_data.present) {
> > > + /*
> > > + * When user enqueued without tuning buffer,
> > > + * it would use driver internal buffer.
> > > + */
> > > + dev_dbg(dip_dev->dev,
> > > + "%s: frame_no(%d) has no tuning_data\n",
> > > + __func__, req->img_fparam.frameparam.frame_no);
> > > +
> > > + mtk_dip_wbuf_to_ipi_tuning_addr
> > > + (&req->img_fparam.frameparam.tuning_data,
> > > + &buf->tuning_buf);
> > > + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> > > + }
> > > +
> > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> > > + &buf->frameparam);
> > > + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> > > + sizeof(req->img_fparam.frameparam));
> > > + ipi_param.usage = IMG_IPI_FRAME;
> > > + ipi_param.frm_param.handle = req->id;
> > > + ipi_param.frm_param.scp_addr = (u32)buf->frameparam.scp_daddr;
> > > +
> > > + mutex_lock(&dip_dev->hw_op_lock);
> > > + atomic_inc(&dip_dev->num_composing);
> > > + ret = scp_ipi_send(dip_dev->scp_pdev, SCP_IPI_DIP, &ipi_param,
> > > + sizeof(ipi_param), 0);
> >
> > We're not holding the pm_runtime enable count here
> > (pm_runtime_get_sync() wasn't called), so rproc_shutdown() might have
> > been called. Wouldn't that affect the ability for this IPI to run?
> >
> > We had a related discussion with Jerry on the FD series and I think
> > the conclusion is:
> > a) if there is any state that needs to be preserved between jobs, that
> > would be cleared by rproc_shutdown() then we need to call
> > rproc_boot/shutdown() when we start/stop streaming.
> > b) it there is no such state, we can keep them inside runtime PM
> > callbacks, but we need to call pm_runtime_get_sync() before sending an
> > IPI and pm_runtime_mark_last_busy() + pm_runtime_put_autosuspend()
> > after the SCP signals completion. In this case the runtime PM
> > autosuspend delay should be set to around 2-3 times the delay needed
> > for rproc_shutdown() + rproc_boot() to complete.
>
> Since each IMG_IPI_FRAME command is stateless, I would like to
> use pm_runtime_get_sync()/ pm_runtime_mark_last_busy()/
> pm_runtime_put_autosuspend() to fix this issue (solution b).
What does IMG_IPI_INIT do then? Do we need it at all?
I'm worried about the fact that we call rproc_boot(), IMG_IPI_INIT and
then rproc_shutdown(). The latter can completely shutdown the SCP and
clear any state there. How would the effects of IMG_IPI_INIT be
preserved until IMG_IPI_FRAME is called?
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings
2019-09-09 19:22 ` [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings frederic.chen
@ 2019-09-13 21:48 ` Rob Herring
2019-10-02 9:22 ` Sakari Ailus
1 sibling, 0 replies; 15+ messages in thread
From: Rob Herring @ 2019-09-13 21:48 UTC (permalink / raw)
To: frederic.chen
Cc: laurent.pinchart+renesas, Rynn.Wu, Allan.Yang, suleiman,
Jerry-ch.Chen, jungo.lin, hans.verkuil, frederic.chen,
linux-media, devicetree, holmes.chiou, shik, yuzhao,
linux-mediatek, matthias.bgg, mchehab, linux-arm-kernel,
Sean.Cheng, srv_heupstream, sj.huang, tfiga, christie.yu,
zwisler
On Tue, 10 Sep 2019 03:22:40 +0800, <frederic.chen@mediatek.com> wrote:
> From: Frederic Chen <frederic.chen@mediatek.com>
>
> This patch adds DT binding documentation for the Digital Image
> Processing (DIP) unit of camera ISP system on Mediatek's SoCs.
>
> It depends on the SCP and MDP 3 patch as following:
>
> 1. dt-bindings: Add a binding for Mediatek SCP
> https://patchwork.kernel.org/patch/11027247/
> 2. dt-binding: mt8183: Add Mediatek MDP3 dt-bindings
> https://patchwork.kernel.org/patch/10945603/
>
> Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> ---
> .../bindings/media/mediatek,mt8183-dip.txt | 40 +++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8183-dip.txt
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver
2019-09-12 5:58 ` Tomasz Figa
@ 2019-09-19 9:41 ` Frederic Chen
2019-09-19 9:44 ` Tomasz Figa
0 siblings, 1 reply; 15+ messages in thread
From: Frederic Chen @ 2019-09-19 9:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: Shik Chen, devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, Rynn Wu (吳育恩),
Christie Yu (游雅惠),
srv_heupstream, Allan Yang (楊智鈞),
Holmes Chiou (邱挺),
suleiman, Jerry-ch Chen, Jungo Lin (林明俊),
Sj Huang, yuzhao, Hans Verkuil, zwisler, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
Linux Media Mailing List
Dear Tomasz,
On Thu, 2019-09-12 at 14:58 +0900, Tomasz Figa wrote:
> On Thu, Sep 12, 2019 at 2:41 AM Frederic Chen
> <frederic.chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > I appreciate your helpful comments.
> >
> >
> > On Tue, 2019-09-10 at 13:04 +0900, Tomasz Figa wrote:
> > > Hi Frederic,
> > >
> > > On Tue, Sep 10, 2019 at 4:23 AM <frederic.chen@mediatek.com> wrote:
> > > >
> > > > From: Frederic Chen <frederic.chen@mediatek.com>
> > > >
> > > > This patch adds the driver of Digital Image Processing (DIP)
> > > > unit in Mediatek ISP system, providing image format
> > > > conversion, resizing, and rotation features.
> > > >
> > > > The mtk-isp directory will contain drivers for multiple IP
> > > > blocks found in Mediatek ISP system. It will include ISP
> > > > Pass 1 driver(CAM), sensor interface driver, DIP driver and
> > > > face detection driver.
> > > >
> > > > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> > > > ---
> > > > drivers/media/platform/mtk-isp/Makefile | 7 +
> > > > .../media/platform/mtk-isp/isp_50/Makefile | 7 +
> > > > .../platform/mtk-isp/isp_50/dip/Makefile | 18 +
> > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.c | 650 +++++
> > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.h | 566 +++++
> > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-hw.h | 156 ++
> > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-sys.c | 521 ++++
> > > > .../mtk-isp/isp_50/dip/mtk_dip-v4l2.c | 2255 +++++++++++++++++
> > > > 8 files changed, 4180 insertions(+)
> > > > create mode 100644 drivers/media/platform/mtk-isp/Makefile
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/Makefile
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.c
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.h
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-hw.h
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-sys.c
> > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-v4l2.c
> > > >
> > >
> > > Thanks for sending v3!
> > >
> > > I'm going to do a full review a bit later, but please check one
> > > comment about power handling below.
> > >
> > > Other than that one comment, from a quick look, I think we only have a
> > > number of style issues left. Thanks for the hard work!
> > >
> > > [snip]
> > > > +static void dip_runner_func(struct work_struct *work)
> > > > +{
> > > > + struct mtk_dip_request *req = mtk_dip_hw_mdp_work_to_req(work);
> > > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > > + struct img_config *config_data =
> > > > + (struct img_config *)req->working_buf->config_data.vaddr;
> > > > +
> > > > + /*
> > > > + * Call MDP/GCE API to do HW excecution
> > > > + * Pass the framejob to MDP driver
> > > > + */
> > > > + pm_runtime_get_sync(dip_dev->dev);
> > > > + mdp_cmdq_sendtask(dip_dev->mdp_pdev, config_data,
> > > > + &req->img_fparam.frameparam, NULL, false,
> > > > + dip_mdp_cb_func, req);
> > > > +}
> > > [snip]
> > > > +static void dip_composer_workfunc(struct work_struct *work)
> > > > +{
> > > > + struct mtk_dip_request *req = mtk_dip_hw_fw_work_to_req(work);
> > > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > > + struct img_ipi_param ipi_param;
> > > > + struct mtk_dip_hw_subframe *buf;
> > > > + int ret;
> > > > +
> > > > + down(&dip_dev->sem);
> > > > +
> > > > + buf = mtk_dip_hw_working_buf_alloc(req->dip_pipe->dip_dev);
> > > > + if (!buf) {
> > > > + dev_err(req->dip_pipe->dip_dev->dev,
> > > > + "%s:%s:req(%p): no free working buffer available\n",
> > > > + __func__, req->dip_pipe->desc->name, req);
> > > > + }
> > > > +
> > > > + req->working_buf = buf;
> > > > + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> > > > + &buf->buffer);
> > > > + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> > > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> > > > + &buf->config_data);
> > > > + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> > > > +
> > > > + if (!req->img_fparam.frameparam.tuning_data.present) {
> > > > + /*
> > > > + * When user enqueued without tuning buffer,
> > > > + * it would use driver internal buffer.
> > > > + */
> > > > + dev_dbg(dip_dev->dev,
> > > > + "%s: frame_no(%d) has no tuning_data\n",
> > > > + __func__, req->img_fparam.frameparam.frame_no);
> > > > +
> > > > + mtk_dip_wbuf_to_ipi_tuning_addr
> > > > + (&req->img_fparam.frameparam.tuning_data,
> > > > + &buf->tuning_buf);
> > > > + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> > > > + }
> > > > +
> > > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> > > > + &buf->frameparam);
> > > > + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> > > > + sizeof(req->img_fparam.frameparam));
> > > > + ipi_param.usage = IMG_IPI_FRAME;
> > > > + ipi_param.frm_param.handle = req->id;
> > > > + ipi_param.frm_param.scp_addr = (u32)buf->frameparam.scp_daddr;
> > > > +
> > > > + mutex_lock(&dip_dev->hw_op_lock);
> > > > + atomic_inc(&dip_dev->num_composing);
> > > > + ret = scp_ipi_send(dip_dev->scp_pdev, SCP_IPI_DIP, &ipi_param,
> > > > + sizeof(ipi_param), 0);
> > >
> > > We're not holding the pm_runtime enable count here
> > > (pm_runtime_get_sync() wasn't called), so rproc_shutdown() might have
> > > been called. Wouldn't that affect the ability for this IPI to run?
> > >
> > > We had a related discussion with Jerry on the FD series and I think
> > > the conclusion is:
> > > a) if there is any state that needs to be preserved between jobs, that
> > > would be cleared by rproc_shutdown() then we need to call
> > > rproc_boot/shutdown() when we start/stop streaming.
> > > b) it there is no such state, we can keep them inside runtime PM
> > > callbacks, but we need to call pm_runtime_get_sync() before sending an
> > > IPI and pm_runtime_mark_last_busy() + pm_runtime_put_autosuspend()
> > > after the SCP signals completion. In this case the runtime PM
> > > autosuspend delay should be set to around 2-3 times the delay needed
> > > for rproc_shutdown() + rproc_boot() to complete.
> >
> > Since each IMG_IPI_FRAME command is stateless, I would like to
> > use pm_runtime_get_sync()/ pm_runtime_mark_last_busy()/
> > pm_runtime_put_autosuspend() to fix this issue (solution b).
>
> What does IMG_IPI_INIT do then? Do we need it at all?
>
> I'm worried about the fact that we call rproc_boot(), IMG_IPI_INIT and
> then rproc_shutdown(). The latter can completely shutdown the SCP and
> clear any state there. How would the effects of IMG_IPI_INIT be
> preserved until IMG_IPI_FRAME is called?
>
The command IMG_IPI_INIT is to initialize the DIP hardware. Although the
DIP hardware status is not cleared when SCP is powered off, it can still
be cleared after mtk_dip_runtime_suspend() is called (it means that DIP
is going to be powered off).
I would like to re-initialize the DIP with IMG_IPI_INIT in
mtk_dip_runtime_resume() to handle this case. Is is OK?
> Best regards,
> Tomasz
Sincerely,
Frederic Chen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver
2019-09-19 9:41 ` Frederic Chen
@ 2019-09-19 9:44 ` Tomasz Figa
0 siblings, 0 replies; 15+ messages in thread
From: Tomasz Figa @ 2019-09-19 9:44 UTC (permalink / raw)
To: Frederic Chen
Cc: Shik Chen, devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, Rynn Wu (吳育恩),
Christie Yu (游雅惠),
srv_heupstream, Allan Yang (楊智鈞),
Holmes Chiou (邱挺),
suleiman, Jerry-ch Chen, Jungo Lin (林明俊),
Sj Huang, yuzhao, Hans Verkuil, zwisler, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
On Thu, Sep 19, 2019 at 6:41 PM Frederic Chen
<frederic.chen@mediatek.com> wrote:
>
> Dear Tomasz,
>
>
> On Thu, 2019-09-12 at 14:58 +0900, Tomasz Figa wrote:
> > On Thu, Sep 12, 2019 at 2:41 AM Frederic Chen
> > <frederic.chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > I appreciate your helpful comments.
> > >
> > >
> > > On Tue, 2019-09-10 at 13:04 +0900, Tomasz Figa wrote:
> > > > Hi Frederic,
> > > >
> > > > On Tue, Sep 10, 2019 at 4:23 AM <frederic.chen@mediatek.com> wrote:
> > > > >
> > > > > From: Frederic Chen <frederic.chen@mediatek.com>
> > > > >
> > > > > This patch adds the driver of Digital Image Processing (DIP)
> > > > > unit in Mediatek ISP system, providing image format
> > > > > conversion, resizing, and rotation features.
> > > > >
> > > > > The mtk-isp directory will contain drivers for multiple IP
> > > > > blocks found in Mediatek ISP system. It will include ISP
> > > > > Pass 1 driver(CAM), sensor interface driver, DIP driver and
> > > > > face detection driver.
> > > > >
> > > > > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> > > > > ---
> > > > > drivers/media/platform/mtk-isp/Makefile | 7 +
> > > > > .../media/platform/mtk-isp/isp_50/Makefile | 7 +
> > > > > .../platform/mtk-isp/isp_50/dip/Makefile | 18 +
> > > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.c | 650 +++++
> > > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-dev.h | 566 +++++
> > > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-hw.h | 156 ++
> > > > > .../platform/mtk-isp/isp_50/dip/mtk_dip-sys.c | 521 ++++
> > > > > .../mtk-isp/isp_50/dip/mtk_dip-v4l2.c | 2255 +++++++++++++++++
> > > > > 8 files changed, 4180 insertions(+)
> > > > > create mode 100644 drivers/media/platform/mtk-isp/Makefile
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/Makefile
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.c
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-dev.h
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-hw.h
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-sys.c
> > > > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/dip/mtk_dip-v4l2.c
> > > > >
> > > >
> > > > Thanks for sending v3!
> > > >
> > > > I'm going to do a full review a bit later, but please check one
> > > > comment about power handling below.
> > > >
> > > > Other than that one comment, from a quick look, I think we only have a
> > > > number of style issues left. Thanks for the hard work!
> > > >
> > > > [snip]
> > > > > +static void dip_runner_func(struct work_struct *work)
> > > > > +{
> > > > > + struct mtk_dip_request *req = mtk_dip_hw_mdp_work_to_req(work);
> > > > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > > > + struct img_config *config_data =
> > > > > + (struct img_config *)req->working_buf->config_data.vaddr;
> > > > > +
> > > > > + /*
> > > > > + * Call MDP/GCE API to do HW excecution
> > > > > + * Pass the framejob to MDP driver
> > > > > + */
> > > > > + pm_runtime_get_sync(dip_dev->dev);
> > > > > + mdp_cmdq_sendtask(dip_dev->mdp_pdev, config_data,
> > > > > + &req->img_fparam.frameparam, NULL, false,
> > > > > + dip_mdp_cb_func, req);
> > > > > +}
> > > > [snip]
> > > > > +static void dip_composer_workfunc(struct work_struct *work)
> > > > > +{
> > > > > + struct mtk_dip_request *req = mtk_dip_hw_fw_work_to_req(work);
> > > > > + struct mtk_dip_dev *dip_dev = req->dip_pipe->dip_dev;
> > > > > + struct img_ipi_param ipi_param;
> > > > > + struct mtk_dip_hw_subframe *buf;
> > > > > + int ret;
> > > > > +
> > > > > + down(&dip_dev->sem);
> > > > > +
> > > > > + buf = mtk_dip_hw_working_buf_alloc(req->dip_pipe->dip_dev);
> > > > > + if (!buf) {
> > > > > + dev_err(req->dip_pipe->dip_dev->dev,
> > > > > + "%s:%s:req(%p): no free working buffer available\n",
> > > > > + __func__, req->dip_pipe->desc->name, req);
> > > > > + }
> > > > > +
> > > > > + req->working_buf = buf;
> > > > > + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> > > > > + &buf->buffer);
> > > > > + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> > > > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> > > > > + &buf->config_data);
> > > > > + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> > > > > +
> > > > > + if (!req->img_fparam.frameparam.tuning_data.present) {
> > > > > + /*
> > > > > + * When user enqueued without tuning buffer,
> > > > > + * it would use driver internal buffer.
> > > > > + */
> > > > > + dev_dbg(dip_dev->dev,
> > > > > + "%s: frame_no(%d) has no tuning_data\n",
> > > > > + __func__, req->img_fparam.frameparam.frame_no);
> > > > > +
> > > > > + mtk_dip_wbuf_to_ipi_tuning_addr
> > > > > + (&req->img_fparam.frameparam.tuning_data,
> > > > > + &buf->tuning_buf);
> > > > > + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> > > > > + }
> > > > > +
> > > > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> > > > > + &buf->frameparam);
> > > > > + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> > > > > + sizeof(req->img_fparam.frameparam));
> > > > > + ipi_param.usage = IMG_IPI_FRAME;
> > > > > + ipi_param.frm_param.handle = req->id;
> > > > > + ipi_param.frm_param.scp_addr = (u32)buf->frameparam.scp_daddr;
> > > > > +
> > > > > + mutex_lock(&dip_dev->hw_op_lock);
> > > > > + atomic_inc(&dip_dev->num_composing);
> > > > > + ret = scp_ipi_send(dip_dev->scp_pdev, SCP_IPI_DIP, &ipi_param,
> > > > > + sizeof(ipi_param), 0);
> > > >
> > > > We're not holding the pm_runtime enable count here
> > > > (pm_runtime_get_sync() wasn't called), so rproc_shutdown() might have
> > > > been called. Wouldn't that affect the ability for this IPI to run?
> > > >
> > > > We had a related discussion with Jerry on the FD series and I think
> > > > the conclusion is:
> > > > a) if there is any state that needs to be preserved between jobs, that
> > > > would be cleared by rproc_shutdown() then we need to call
> > > > rproc_boot/shutdown() when we start/stop streaming.
> > > > b) it there is no such state, we can keep them inside runtime PM
> > > > callbacks, but we need to call pm_runtime_get_sync() before sending an
> > > > IPI and pm_runtime_mark_last_busy() + pm_runtime_put_autosuspend()
> > > > after the SCP signals completion. In this case the runtime PM
> > > > autosuspend delay should be set to around 2-3 times the delay needed
> > > > for rproc_shutdown() + rproc_boot() to complete.
> > >
> > > Since each IMG_IPI_FRAME command is stateless, I would like to
> > > use pm_runtime_get_sync()/ pm_runtime_mark_last_busy()/
> > > pm_runtime_put_autosuspend() to fix this issue (solution b).
> >
> > What does IMG_IPI_INIT do then? Do we need it at all?
> >
> > I'm worried about the fact that we call rproc_boot(), IMG_IPI_INIT and
> > then rproc_shutdown(). The latter can completely shutdown the SCP and
> > clear any state there. How would the effects of IMG_IPI_INIT be
> > preserved until IMG_IPI_FRAME is called?
> >
>
> The command IMG_IPI_INIT is to initialize the DIP hardware. Although the
> DIP hardware status is not cleared when SCP is powered off, it can still
> be cleared after mtk_dip_runtime_suspend() is called (it means that DIP
> is going to be powered off).
Great, that's exactly what I wanted to confirm. Thanks. :)
>
> I would like to re-initialize the DIP with IMG_IPI_INIT in
> mtk_dip_runtime_resume() to handle this case. Is is OK?
>
Makes sense.
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 0/5] media: platform: Add support for Digital Image Processing (DIP) on mt8183 SoC
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
` (4 preceding siblings ...)
[not found] ` <20190909192244.9367-5-frederic.chen@mediatek.com>
@ 2019-09-23 12:11 ` Sakari Ailus
2023-08-19 5:43 ` Paul Menzel
6 siblings, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2019-09-23 12:11 UTC (permalink / raw)
To: frederic.chen
Cc: laurent.pinchart+renesas, Rynn.Wu, Allan.Yang, suleiman,
Jerry-ch.Chen, jungo.lin, hans.verkuil, linux-media, devicetree,
holmes.chiou, shik, yuzhao, linux-mediatek, matthias.bgg,
mchehab, linux-arm-kernel, Sean.Cheng, srv_heupstream, sj.huang,
tfiga, christie.yu, zwisler
Hi Frederic,
On Tue, Sep 10, 2019 at 03:22:39AM +0800, frederic.chen@mediatek.com wrote:
> Hello,
>
> This RFC patch series added Digital Image Processing (DIP) driver on Mediatek
> mt8183 SoC. It belongs to the Mediatek's ISP driver series based on V4L2 and
> media controller framework. I posted the main part of the DIP driver as RFC to
> discuss first and would like some review comments.
Could you post media-ctl -p and media-ctl --print-dot outputs for this
driver, please?
--
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig
2019-09-09 19:22 ` [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig frederic.chen
@ 2019-10-02 9:20 ` Sakari Ailus
0 siblings, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2019-10-02 9:20 UTC (permalink / raw)
To: frederic.chen
Cc: laurent.pinchart+renesas, Rynn.Wu, Allan.Yang, suleiman,
Jerry-ch.Chen, jungo.lin, hans.verkuil, linux-media, devicetree,
holmes.chiou, shik, yuzhao, linux-mediatek, matthias.bgg,
mchehab, linux-arm-kernel, Sean.Cheng, srv_heupstream, sj.huang,
tfiga, christie.yu, zwisler
Hi Frederic,
On Tue, Sep 10, 2019 at 03:22:42AM +0800, frederic.chen@mediatek.com wrote:
> From: Frederic Chen <frederic.chen@mediatek.com>
>
> This patch adds KConfig for Mediatek Digital Image Processing
> driver(DIP). DIP is embedded in Mediatek SoCs. It provides
> image format conversion, resizing, and rotation function.
>
> Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
Please merge this to the patch adding the driver.
--
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 5/5] media: platform: mtk-mdp3: Add struct tuning_addr and img_sw_buffer
2019-09-09 19:22 ` [RFC PATCH V3 5/5] media: platform: mtk-mdp3: Add struct tuning_addr and img_sw_buffer frederic.chen
@ 2019-10-02 9:21 ` Sakari Ailus
0 siblings, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2019-10-02 9:21 UTC (permalink / raw)
To: frederic.chen
Cc: laurent.pinchart+renesas, Rynn.Wu, Allan.Yang, suleiman,
Jerry-ch.Chen, jungo.lin, hans.verkuil, linux-media, devicetree,
holmes.chiou, shik, yuzhao, linux-mediatek, matthias.bgg,
mchehab, linux-arm-kernel, Sean.Cheng, srv_heupstream, sj.huang,
tfiga, christie.yu, zwisler
On Tue, Sep 10, 2019 at 03:22:44AM +0800, frederic.chen@mediatek.com wrote:
> From: Frederic Chen <frederic.chen@mediatek.com>
>
> We added a struct tuning_addr which contains a field "present"
> so that the driver can tell the firmware if we have user tuning
> dataor not.
>
> The strcut img_sw_buffer is also added. This struct has no cpu address
> field and uses a handle instead so that we don't pass a cpu address
> to co-processor.
>
> Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
If the driver depends on this patch, it needs to come before the driver.
--
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings
2019-09-09 19:22 ` [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings frederic.chen
2019-09-13 21:48 ` Rob Herring
@ 2019-10-02 9:22 ` Sakari Ailus
1 sibling, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2019-10-02 9:22 UTC (permalink / raw)
To: frederic.chen
Cc: laurent.pinchart+renesas, Rynn.Wu, Allan.Yang, suleiman,
Jerry-ch.Chen, jungo.lin, hans.verkuil, linux-media, devicetree,
holmes.chiou, shik, yuzhao, linux-mediatek, matthias.bgg,
mchehab, linux-arm-kernel, Sean.Cheng, srv_heupstream, sj.huang,
tfiga, christie.yu, zwisler
Hi Frederic,
On Tue, Sep 10, 2019 at 03:22:40AM +0800, frederic.chen@mediatek.com wrote:
> From: Frederic Chen <frederic.chen@mediatek.com>
>
> This patch adds DT binding documentation for the Digital Image
> Processing (DIP) unit of camera ISP system on Mediatek's SoCs.
>
> It depends on the SCP and MDP 3 patch as following:
>
> 1. dt-bindings: Add a binding for Mediatek SCP
> https://patchwork.kernel.org/patch/11027247/
> 2. dt-binding: mt8183: Add Mediatek MDP3 dt-bindings
> https://patchwork.kernel.org/patch/10945603/
>
> Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> ---
> .../bindings/media/mediatek,mt8183-dip.txt | 40 +++++++++++++++++++
Could you add a MAINTAINERS entry for this, please? Same for the driver.
--
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH V3 0/5] media: platform: Add support for Digital Image Processing (DIP) on mt8183 SoC
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
` (5 preceding siblings ...)
2019-09-23 12:11 ` [RFC PATCH V3 0/5] media: platform: Add support for Digital Image Processing (DIP) on mt8183 SoC Sakari Ailus
@ 2023-08-19 5:43 ` Paul Menzel
6 siblings, 0 replies; 15+ messages in thread
From: Paul Menzel @ 2023-08-19 5:43 UTC (permalink / raw)
To: Frederic Chen
Cc: hans.verkuil, laurent.pinchart+renesas, tfiga, matthias.bgg,
mchehab, yuzhao, zwisler, linux-mediatek, linux-arm-kernel,
Sean.Cheng, sj.huang, christie.yu, holmes.chiou, Jerry-ch.Chen,
jungo.lin, Rynn.Wu, linux-media, srv_heupstream, devicetree,
shik, suleiman, Allan.Yang, Sakari Ailus, Guenter Roeck
[Cc: +Sakari, +Guenter]
Dear Frederic, dear Linux folks,
Am 09.09.19 um 21:22 schrieb frederic.chen@mediatek.com:
> This RFC patch series added Digital Image Processing (DIP) driver on Mediatek
> mt8183 SoC. It belongs to the Mediatek's ISP driver series based on V4L2 and
> media controller framework. I posted the main part of the DIP driver as RFC to
> discuss first and would like some review comments.
>
> I appreciate the comment of Tomasz in RFC V2. The RFC V3 patch addressed on all
> issues reviewed in V2 except the one about Mediatek proprietary MDP stride,
> depth and raw depth usage which is still under discussion. I will refactor
> the related parts once we come to the conclusion.
>
> You can check the following URL for the detail.
> http://lists.infradead.org/pipermail/linux-mediatek/2019-September/023254.html
>
>
> In V3, I also removed all workaround solution about the following V4L2
> compliance tool issues so that we got the related failed result.
>
> 1. Request API test doesn't know which buffers of the video devices are
> required so we got failed in testRequests()
>
> 2. V4L2 compliance test check if the driver return error when passing an
> invalid image size, but in vb2_create_bufs() case, we don't know if the
> size check is required or not.
>
> Please see the following URL for the detail.
> http://lists.infradead.org/pipermail/linux-mediatek/2019-June/020884.html
>
>
> Besides that, we got a new issue about the test case. When receiving the
> VIDIOC_SUBDEV_G_FMT ioctl on a DIP sub device's pad which connects with a
> meta video device, we return -EINVEL since it doesn't represent an image
> data flow (no width and height information), but the test case expects
> that the driver return some media format information.
>
> Sub-Device ioctls (Sink Pad 1):
> fail: v4l2-test-subdevs.cpp(352): doioctl(node, VIDIOC_SUBDEV_G_FMT, &fmt)
> test Try VIDIOC_SUBDEV_G/S_FMT: FAIL
>
>
> ==============
> Introduction
> ==============
>
> Digital Image Processing (DIP) unit can accept the tuning parameters and
> adjust the image content in Mediatek ISP system. Furthermore, it performs
> demosaicing and noise reduction on the image to support the advanced camera
> features of the application. The DIP driver also support image format
> conversion, resizing and rotation with its hardware path.
>
> The driver is implemented with V4L2 and media controller framework. We
> have the following entities describing the DIP path. Since a DIP frame has
> multiple buffers, the driver uses Request API to control the multiple
> buffer's enqueue flow.
>
> 1. Meta (output video device): connects to DIP sub device. It accepts the
> input tuning buffer from userspace. The metadata interface used currently
> is only a temporary solution to kick off driver development and is not
> ready for reviewed yet.
>
> 2. RAW (output video device): connects to DIP sub device. It accepts input
> image buffer from userspace.
>
> 3. DIP (sub device): connects to MDP-0 and MDP-1. When processing an image,
> DIP hardware support multiple output images with different size and format
> so it needs two capture video devices to return the streaming data to the
> user.
>
> 4. MDP-0 (capture video device): return the processed image data.
>
> 5. MDP-1 (capture video device): return the processed image data, the
> image size and format can be different from the ones of MDP-0.
>
> The overall file structure of the DIP driver is as following:
>
> * mtk_dip-v4l2.c: implements DIP platform driver, V4L2 and vb2 operations.
>
> * mtk_dip-sys.c: implements the hardware job handling flow including the part of
> interaction with the SCP and MDP.
>
> * mtk_dip-dev.c: implements dip pipe utilities. DIP driver supports 3 software
> pipes (preview, capture and reprocessing) at the same time. All
> the pipes share the same DIP hardware to process the images.
Thank you for your work. I use the Lenovo IdeaPad Duet Chromebook
(google/kukui variant of google/krane), and noticed the messages below
using the camera in the browser with recent ChromeOS:
[ 0.000000] Linux version 5.10.180-22631-gc8e37fc5f0ab
(chrome-bot@chromeos-release-builder-us-central1-b-x32-66-okmh)
(Chromium OS 17.0_pre496208_p20230501-r6 clang version 17.0.0
(/mnt/host/source/src/third_party/llvm-project
98f5a340975bc00197c57e39eb4ca26e2da0e8a2), LLD 17.0.0) #1 SMP PREEMPT
Wed Jul 26 19:01:55 PDT 2023
[…]
[ 2766.733517] mtk-cam-dip 15022000.dip: req(0xffffff8e5fdc9800),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.737034] req(0xffffff8e5fdc9800),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.772352] mtk-cam-dip 15022000.dip: req(0xffffff8d88002000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.775906] req(0xffffff8d88002000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.788790] mtk-cam-dip 15022000.dip: req(0xffffff8d88000000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.792327] req(0xffffff8d88000000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.830257] mtk-cam-dip 15022000.dip: req(0xffffff8e5ff46000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.833806] req(0xffffff8e5ff46000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.869589] mtk-cam-dip 15022000.dip: req(0xffffff8e5ff44000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.873104] req(0xffffff8e5ff44000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.889804] mtk-cam-dip 15022000.dip: req(0xffffff8e5ff41000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.893351] req(0xffffff8e5ff41000),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.939293] mtk-cam-dip 15022000.dip: req(0xffffff8e5ff43800),
req->dip_pipe(0xffffff8d829a0398)
[ 2766.942827] req(0xffffff8e5ff43800),
req->dip_pipe(0xffffff8d829a0398)
Search for that in the upstream Linux kernel, I found out, the the DIP
support is not upstreamed yet, and also has not been added to the
Chromium OS Linux kernel branches chromeos-5.15 and chromeos-6.1 [1].
Were you able to come to a conclusion regarding the two(?) issues
mentioned in your cover letter, so this series can be re-posted as
non-RFC? The driver seems to work well on millions(?) of devices, so
it’d be great to have it upstream.
[…]
Kind regards,
Paul
[1]:
https://chromium-review.googlesource.com/q/I1d1ba58cbdcdcc161b140398fc26b24ec2134cdb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-08-19 5:45 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190909192244.9367-1-frederic.chen@mediatek.com>
2019-09-09 19:22 ` [RFC PATCH V3 1/5] dt-bindings: mt8183: Added DIP dt-bindings frederic.chen
2019-09-13 21:48 ` Rob Herring
2019-10-02 9:22 ` Sakari Ailus
2019-09-09 19:22 ` [RFC PATCH V3 2/5] dts: arm64: mt8183: Add DIP nodes frederic.chen
2019-09-09 19:22 ` [RFC PATCH V3 3/5] media: platform: Add Mediatek DIP driver KConfig frederic.chen
2019-10-02 9:20 ` Sakari Ailus
2019-09-09 19:22 ` [RFC PATCH V3 5/5] media: platform: mtk-mdp3: Add struct tuning_addr and img_sw_buffer frederic.chen
2019-10-02 9:21 ` Sakari Ailus
[not found] ` <20190909192244.9367-5-frederic.chen@mediatek.com>
2019-09-10 4:04 ` [RFC PATCH V3 4/5] platform: mtk-isp: Add Mediatek DIP driver Tomasz Figa
2019-09-11 17:41 ` Frederic Chen
2019-09-12 5:58 ` Tomasz Figa
2019-09-19 9:41 ` Frederic Chen
2019-09-19 9:44 ` Tomasz Figa
2019-09-23 12:11 ` [RFC PATCH V3 0/5] media: platform: Add support for Digital Image Processing (DIP) on mt8183 SoC Sakari Ailus
2023-08-19 5:43 ` Paul Menzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).