[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1908dc86d948ba02f1133633e08652a12610edea.camel@mediatek.com>
Date: Tue, 19 Nov 2024 05:56:47 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
"christian.koenig@....com" <christian.koenig@....com>, "mchehab@...nel.org"
<mchehab@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"robh@...nel.org" <robh@...nel.org>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Shu-hsiang Yang (楊舒翔)
<Shu-hsiang.Yang@...iatek.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"yunkec@...omium.org" <yunkec@...omium.org>, "linaro-mm-sig@...ts.linaro.org"
<linaro-mm-sig@...ts.linaro.org>, "linux-media@...r.kernel.org"
<linux-media@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, Yaya Chang (張雅清)
<Yaya.Chang@...iatek.com>, Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Teddy Chen (陳乾元) <Teddy.Chen@...iatek.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "hidenorik@...omium.org"
<hidenorik@...omium.org>, Shun-Yi Wang (王順億)
<Shun-Yi.Wang@...iatek.com>
Subject: Re: [PATCH v1 05/10] media: platform: mediatek: add isp_7x camsys
unit
Hi, Shu-hsiang:
On Wed, 2024-10-16 at 11:43 +0800, CK Hu wrote:
> Hi, Shu-hsiang:
>
> On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote:
> > Introduces the top media device driver for the MediaTek ISP7X CAMSYS.
> > The driver maintains the camera system, including sub-device management,
> > DMA operations, and integration with the V4L2 framework. It handles
> > request stream data, buffer management, and MediaTek-specific features,
> > and pipeline management, streaming control, error handling mechanism.
> > Additionally, it aggregates sub-drivers for the camera interface, raw
> > and yuv pipelines.
> >
> > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@...iatek.com>
> > ---
>
> [snip]
>
> > +static int mtk_cam_probe(struct platform_device *pdev)
> > +{
> > + struct mtk_cam_device *cam_dev;
> > + struct device *dev = &pdev->dev;
> > + struct resource *res;
> > + int ret;
> > + unsigned int i;
> > +
> > + dev_dbg(dev, "camsys | start %s\n", __func__);
> > +
> > + /* initialize structure */
> > + cam_dev = devm_kzalloc(dev, sizeof(*cam_dev), GFP_KERNEL);
> > + if (!cam_dev)
> > + return -ENOMEM;
> > +
> > + if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34))) {
> > + dev_err(dev, "%s: No suitable DMA available\n", __func__);
> > + return -EIO;
> > + }
> > +
> > + if (!dev->dma_parms) {
> > + dev->dma_parms =
> > + devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL);
> > + if (!dev->dma_parms)
> > + return -ENOMEM;
> > + }
> > +
> > + dma_set_max_seg_size(dev, UINT_MAX);
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res) {
> > + dev_err(dev, "failed to get mem\n");
> > + return -ENODEV;
> > + }
> > +
> > + cam_dev->base = devm_ioremap_resource(dev, res);
>
> I can not find any where to write register of this device.
> If so, I think we need not to probe this driver.
> The rest software control can be setup by other driver.
Ignore my previous comment. I find you need to power on/off this device.
So this probe is necessary.
Regards,
CK
>
> Regards,
> CK
>
> > + if (IS_ERR(cam_dev->base)) {
> > + dev_err(dev, "failed to map register base\n");
> > + return PTR_ERR(cam_dev->base);
> > + }
> > +
> > + cam_dev->dev = dev;
> > + dev_set_drvdata(dev, cam_dev);
> > +
> > + cam_dev->composer_cnt = 0;
> > + cam_dev->num_seninf_devices = 0;
> > +
> > + cam_dev->max_stream_num = MTKCAM_SUBDEV_MAX;
> > + cam_dev->ctxs = devm_kcalloc(dev, cam_dev->max_stream_num,
> > + sizeof(*cam_dev->ctxs), GFP_KERNEL);
> > + if (!cam_dev->ctxs)
> > + return -ENOMEM;
> > +
> > + cam_dev->streaming_ctx = 0;
> > + for (i = 0; i < cam_dev->max_stream_num; i++)
> > + mtk_cam_ctx_init(cam_dev->ctxs + i, cam_dev, i);
> > +
> > + cam_dev->running_job_count = 0;
> > + spin_lock_init(&cam_dev->pending_job_lock);
> > + spin_lock_init(&cam_dev->running_job_lock);
> > + INIT_LIST_HEAD(&cam_dev->pending_job_list);
> > + INIT_LIST_HEAD(&cam_dev->running_job_list);
> > +
> > + cam_dev->dma_processing_count = 0;
> > + spin_lock_init(&cam_dev->dma_pending_lock);
> > + spin_lock_init(&cam_dev->dma_processing_lock);
> > + INIT_LIST_HEAD(&cam_dev->dma_pending);
> > + INIT_LIST_HEAD(&cam_dev->dma_processing);
> > +
> > + mutex_init(&cam_dev->queue_lock);
> > +
> > + pm_runtime_enable(dev);
> > +
> > + ret = mtk_cam_of_rproc(cam_dev, pdev);
> > + if (ret)
> > + goto fail_destroy_mutex;
> > +
> > + ret = register_sub_drivers(dev);
> > + if (ret) {
> > + dev_err(dev, "fail to register_sub_drivers\n");
> > + goto fail_destroy_mutex;
> > + }
> > +
> > + /* register mtk_cam as all isp subdev async parent */
> > + cam_dev->notifier.ops = &mtk_cam_async_nf_ops;
> > + v4l2_async_nf_init(&cam_dev->notifier, &cam_dev->v4l2_dev);
> > + ret = mtk_cam_async_subdev_add(dev); /* wait all isp sub drivers */
> > + if (ret) {
> > + dev_err(dev, "%s failed mtk_cam_async_subdev_add\n", __func__);
> > + goto fail_unregister_sub_drivers;
> > + }
> > +
> > + ret = v4l2_async_nf_register(&cam_dev->notifier);
> > + if (ret) {
> > + dev_err(dev, "%s async_nf_register ret:%d\n", __func__, ret);
> > + v4l2_async_nf_cleanup(&cam_dev->notifier);
> > + goto fail_unregister_sub_drivers;
> > + }
> > +
> > + ret = mtk_cam_debug_fs_init(cam_dev);
> > + if (ret < 0)
> > + goto fail_unregister_async_nf;
> > +
> > + dev_info(dev, "camsys | [%s] success\n", __func__);
> > +
> > + return 0;
> > +
> > +fail_unregister_async_nf:
> > + v4l2_async_nf_unregister(&cam_dev->notifier);
> > +
> > +fail_unregister_sub_drivers:
> > + unregister_sub_drivers(dev);
> > +
> > +fail_destroy_mutex:
> > + mutex_destroy(&cam_dev->queue_lock);
> > +
> > + return ret;
> > +}
> > +
Powered by blists - more mailing lists