From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 513FBCA9EA0 for ; Fri, 25 Oct 2019 03:52:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1129420867 for ; Fri, 25 Oct 2019 03:52:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Ce+0HrND" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2392314AbfJYDw0 (ORCPT ); Thu, 24 Oct 2019 23:52:26 -0400 Received: from mail-pf1-f194.google.com ([209.85.210.194]:41502 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392291AbfJYDw0 (ORCPT ); Thu, 24 Oct 2019 23:52:26 -0400 Received: by mail-pf1-f194.google.com with SMTP id q7so632099pfh.8 for ; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=Ce+0HrNDmFSivub2oBEJVlA0HHWdPOu5+6DmZIJe7npEriRRFg631g4lekMZY0vAiE X3w8JaBKz/xk8MUr8g0dIKO02+oQdsng0Yd15tGlAmwd6MIRY+eKa82ZieljWuM1/TXA lWrkonKegWaVKApjLAbjNz8voQ4f1oncASGEE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=qOUA3e2exDxs5J5aQxCYmW/fEhqzoRKixXEIsdUDdgVDlfzO3zAikTg5F0ojyhO/x/ TJm5yJCEPQN8q8f9ESfHRnLryQ5efGqQs0pQvbWYEHOJm04u1boMGwTyepXn38ygccLC bjRtIzxmxxyZzZmQi6D9idVGb1Ly1tXHjZBYnZKu49oF2/K8D/Bul7rs7IlAQksKR2u4 iPcoDE/SVBK35c2e4rPGNnMBM49Nur/himr63iDOHwrvnGX6pmNEc6NgWHmWCkZ0amfr NypczTPpl2K56HUireF4PBPS5Kyxb/V2Afte+NvWbmzGrAvPbs0cWaPnoTx2Ur85av5c oFOg== X-Gm-Message-State: APjAAAVtpCq/4QkkYmgrjOr2SRNu2XUye6N2YO5Qdp8Ei8VTbU1MjTR+ CelE5u934UnsCb5yGivojSxLmQ== X-Google-Smtp-Source: APXvYqwkwetVcs47sPZxLr5ISxVfMjs558/KCCIrpnAsnIMnYSTi2p/HmtYweJj67U8UZ7Y8mq9qFw== X-Received: by 2002:a63:4c1c:: with SMTP id z28mr1507674pga.167.1571975545045; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) Received: from chromium.org ([2401:fa00:8f:203:f5fe:2a5e:f953:c0ed]) by smtp.gmail.com with ESMTPSA id c16sm571818pja.2.2019.10.24.20.52.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Oct 2019 20:52:17 -0700 (PDT) Date: Fri, 25 Oct 2019 12:52:11 +0900 From: Tomasz Figa To: Jerry-ch Chen Cc: hans.verkuil@cisco.com, "laurent.pinchart+renesas@ideasonboard.com" , "matthias.bgg@gmail.com" , "mchehab@kernel.org" , "lkml@metux.net" , CK Hu =?utf-8?B?KOiDoeS/iuWFiSk=?= , "yuzhao@chromium.org" , "zwisler@chromium.org" , "linux-mediatek@lists.infradead.org" , "linux-arm-kernel@lists.infradead.org" , Sean Cheng =?utf-8?B?KOmEreaYh+W8mCk=?= , Sj Huang =?utf-8?B?KOm7g+S/oeeSiyk=?= , Christie Yu =?utf-8?B?KOa4uOmbheaDoCk=?= , Frederic Chen =?utf-8?B?KOmZs+S/iuWFgyk=?= , Jungo Lin =?utf-8?B?KOael+aYjuS/iik=?= , Po-Yang Huang =?utf-8?B?KOm7g+afj+mZvSk=?= , Rynn Wu =?utf-8?B?KOWQs+iCsuaBqSk=?= , "linux-media@vger.kernel.org" , srv_heupstream , "devicetree@vger.kernel.org" Subject: Re: [RFC PATCH V3 3/3] platform: mtk-isp: Add Mediatek FD driver Message-ID: <20191025035211.GA67000@chromium.org> References: <20190906101125.3784-1-Jerry-Ch.chen@mediatek.com> <20190906101125.3784-4-Jerry-Ch.chen@mediatek.com> <1571109375.3706.40.camel@mtksdccf07> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1571109375.3706.40.camel@mtksdccf07> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Tue, Oct 15, 2019 at 11:16:15AM +0800, Jerry-ch Chen wrote: > Hi Tomasz, > > On Fri, 2019-09-06 at 18:11 +0800, Jerry-ch Chen wrote: > > From: Jerry-ch Chen > > > > This patch adds the driver of Face Detection (FD) unit in > > Mediatek camera system, providing face detection function. > > > > 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: Jerry-ch Chen > > --- > > drivers/media/platform/Kconfig | 2 + > > drivers/media/platform/Makefile | 2 + > > drivers/media/platform/mtk-isp/fd/Kconfig | 19 + > > drivers/media/platform/mtk-isp/fd/Makefile | 5 + > > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 148 ++ > > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1219 +++++++++++++++++ > > include/uapi/linux/mtk-fd-v4l2-controls.h | 69 + > > include/uapi/linux/v4l2-controls.h | 4 + > > 8 files changed, 1468 insertions(+) > > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig > > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c > > create mode 100644 include/uapi/linux/mtk-fd-v4l2-controls.h > > [snip] > > +static int mtk_fd_job_abort(struct mtk_fd_dev *fd) > > +{ > > + u32 ret; > > + > > + ret = wait_for_completion_timeout(&fd->fd_irq_done, > > + msecs_to_jiffies(MTK_FD_HW_TIMEOUT)); > > + /* Reset FD HW */ > > + if (!ret) { > > + struct ipi_message fd_ipi_msg; > > + > > + fd_ipi_msg.cmd_id = MTK_FD_IPI_CMD_RESET; > > + if (scp_ipi_send(fd->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg, > > + sizeof(fd_ipi_msg), MTK_FD_IPI_SEND_TIMEOUT)) > > + dev_err(fd->dev, "FD Reset HW error\n"); > > + return -ETIMEDOUT; > > + } > > + return 0; > > +} > > + > > Continue the discussion about job abort in RFC v2, > > I think the job_abort callback in v4l2_m2m_ops() might be useful. > > ref: > https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/media/v4l2-core/v4l2-mem2mem.c#L398 > https://elixir.bootlin.com/linux/v5.4-rc2/source/include/media/v4l2-mem2mem.h#L43 > > in drivers/media/v4l2-core/v4l2-mem2mem.c #398 v4l2_m2m_cancel_job() > ... > if (m2m_ctx->job_flags & TRANS_RUNNING) { > spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags); > if (m2m_dev->m2m_ops->job_abort) > m2m_dev->m2m_ops->job_abort(m2m_ctx->priv); > dprintk("m2m_ctx %p running, will wait to complete\n", m2m_ctx); > wait_event(m2m_ctx->finished, > !(m2m_ctx->job_flags & TRANS_RUNNING)); > } ... > > If this operation is set, we might use the v4l2_m2m_cancel_job() when > suspend, and it will do mtk_fd_job_abort() here. > The expectation for .job_abort() is that signals the hardware to instantly abandon the current job. Do we have a way to tell the firmware/hardware to do so? Also, suspend must not abort the current job. Anything that was already running is expected to complete successfuly and further jobs should continue executing once the system resumes. [snip] > > + > > +static int mtk_fd_suspend(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + if (fd->fd_stream_count) > > + if (mtk_fd_job_abort(fd)) > > + mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR); > > + > > To avoid mtk_fd_hw_job_finish() trigger the next job, > I suppose that we could use v4l2_m2m_cancel_job instead of job_abort and > job_finish here. > > /** > * v4l2_m2m_cancel_job() - cancel pending jobs for the context > * @m2m_ctx: m2m context with jobs to be canceled > * > * In case of streamoff or release called on any context, > * 1] If the context is currently running, then abort job will be called > * 2] If the context is queued, then the context will be removed from > * the job_queue > */ > > or another way, > we may add a flag and implement mtk_fd_job_ready() that reads the flag > if we suspend, we set the flag and do job_abort and job_finish, even if > it try enqueue, it will still not really queue the job, until we reset > the flag in mtk_fd_resume(). > > how do you think? > As per my comment above, suspend must just pause the execution of the jobs. It must not cause any jobs to be skipped. After analyzing the m2m framework and existing m2m drivers I realized that they currently provide no way to correctly handle suspend/resume. Pi-Hsun has been looking into fixing this in crrev.com/c/1878112 and we'll send it upstream as soon as we get something that should handle all the cases correctly. > > + /* suspend FD HW */ > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + clk_disable_unprepare(fd->fd_clk); > > + dev_dbg(dev, "%s:disable clock\n", __func__); > > + > > + return 0; > > +} > > + > > +static int mtk_fd_resume(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + int ret; > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + ret = clk_prepare_enable(fd->fd_clk); > > + if (ret < 0) { > > + dev_dbg(dev, "Failed to open fd clk:%d\n", ret); > > + return ret; > > + } > > + > > + /* resume FD HW */ > > + writel(MTK_FD_SET_HW_ENABLE, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + writel(0x1, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + dev_dbg(dev, "%s:enable clock\n", __func__); By the way, we need to kick the m2m framework here to schedule further jobs. Pi-Hsun's patch will also take care of this. [snip] > > +/* Set the face angle and directions to be detected */ > > +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 1) > > + > > +/* Set image widths for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 2) > > + > > +/* Set image heights for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 3) > > + > > +/* Set the length of scale down size array */ > > +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 4) > > + > > +/* Set the detection speed, usually reducing accuracy. */ > > +#define V4L2_CID_MTK_FD_DETECT_SPEED (V4L2_CID_USER_MTK_FD_BASE + 5) > > + > > +/* Select the detection model or algorithm to be used. */ > > +#define V4L2_CID_MTK_FD_DETECTION_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6) > > + > > +/* We reserve 16 controls for this driver. */ > > +#define V4L2_CID_MTK_FD_MAX 16 > > + > > For these control IDs, I think the following should be remained as chip > specific controls. > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH, > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT and V4L2_CID_MTK_FD_SCALE_IMG_NUM > > Hope there would be standardizing face detection api that cover the rest > controls: V4L2_CID_MTK_FD_DETECT_POSE, V4L2_CID_MTK_FD_DETECT_SPEED and > V4L2_CID_MTK_FD_DETECTION_MODEL > > Would you have any suggestions on how to propose the standard face > detection apis? > Given no follow up feedback from the community, I think we can keep them as driver-specific, but should make sure that they have some reasonable default values in case an application doesn't recognize them. Best regards, Tomasz From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8E58CA9EA0 for ; Fri, 25 Oct 2019 03:52:56 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8A56920867 for ; Fri, 25 Oct 2019 03:52:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="sFHQE3Rf"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Ce+0HrND" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8A56920867 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=XGZ8RvpVCiEEfT4UMJbuoV5AG86C0IPgNoGtXSI7Gg0=; b=sFHQE3RfCTUVnP pH5joBjT4BzXIKXcdBLHiT4mitbuKRRX/aP8jDKa1wNo97vYrIqb2dtUhEbFJVZhAIPddHhIyETJG 5r1ygt4FaMeYSNSbPownm0t7RoUvXSHQuaRTdo4pY0gc/leTEWFW94pY0jOyXC8a36eH06gw8BkO7 n3jbX4I4TgZkoktF5uLOEPEhRmXFQR440EBvdDp+U+az3p+Ka90QvVwEWFXtWFjOPcfNqzYSg4pGH HmePtWxLBNRKmTI8BxI7T5LFiW8wQv9XMnuT9UrzhfnsoyRUeRqq0O43rM9e6hmkm/MgWpKjLR2pV 1r+oav7CSzkI94gGJTJQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iNqeX-0005as-Uf; Fri, 25 Oct 2019 03:52:41 +0000 Received: from mail-pf1-x444.google.com ([2607:f8b0:4864:20::444]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iNqeL-0005QU-M5 for linux-mediatek@lists.infradead.org; Fri, 25 Oct 2019 03:52:31 +0000 Received: by mail-pf1-x444.google.com with SMTP id 21so625516pfj.9 for ; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=Ce+0HrNDmFSivub2oBEJVlA0HHWdPOu5+6DmZIJe7npEriRRFg631g4lekMZY0vAiE X3w8JaBKz/xk8MUr8g0dIKO02+oQdsng0Yd15tGlAmwd6MIRY+eKa82ZieljWuM1/TXA lWrkonKegWaVKApjLAbjNz8voQ4f1oncASGEE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=ro8PkARg2ndn8vDMrfpldCCrpu0stRp0iFz7kWMisZ2eos/QO+Xxm6csVVf+fGbhDU JUfUeWisMrhrrKkLlLo1k7ho1IWswCaoaSNecPxesLjAbEKY+NOMnrmHqIgs/mYpPLcF ireE8UMw8VfCc8lk5ABYIluQ4KfBqzjzrZ5BJgCDLTLr13Cv0WJS13lzP/9Paw6G1Byt gXc0cPx+JyPD7XMVy+3GJBJnMk0ZJaaNzOsOHM3P8Ussz2RW85I8WImv28HiPxiKA81k u4IIkBrr8iGAeD2jawln7AydhKT+HUH7qQFhXBVTur2SSEwj5qomaYm16YYvA4EU/6r6 rrqw== X-Gm-Message-State: APjAAAXEOIZcKHbuHyYnSfvMLiJ2DPt9NUQVAIPFdqpuj7tcpn6S/FAh KOD01JgPLIhX1MWaiR0e16Akxg== X-Google-Smtp-Source: APXvYqwkwetVcs47sPZxLr5ISxVfMjs558/KCCIrpnAsnIMnYSTi2p/HmtYweJj67U8UZ7Y8mq9qFw== X-Received: by 2002:a63:4c1c:: with SMTP id z28mr1507674pga.167.1571975545045; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) Received: from chromium.org ([2401:fa00:8f:203:f5fe:2a5e:f953:c0ed]) by smtp.gmail.com with ESMTPSA id c16sm571818pja.2.2019.10.24.20.52.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Oct 2019 20:52:17 -0700 (PDT) Date: Fri, 25 Oct 2019 12:52:11 +0900 From: Tomasz Figa To: Jerry-ch Chen Subject: Re: [RFC PATCH V3 3/3] platform: mtk-isp: Add Mediatek FD driver Message-ID: <20191025035211.GA67000@chromium.org> References: <20190906101125.3784-1-Jerry-Ch.chen@mediatek.com> <20190906101125.3784-4-Jerry-Ch.chen@mediatek.com> <1571109375.3706.40.camel@mtksdccf07> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1571109375.3706.40.camel@mtksdccf07> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191024_205229_742472_D040BCA4 X-CRM114-Status: GOOD ( 31.33 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , Sean Cheng =?utf-8?B?KOmEreaYh+W8mCk=?= , "laurent.pinchart+renesas@ideasonboard.com" , Rynn Wu =?utf-8?B?KOWQs+iCsuaBqSk=?= , Christie Yu =?utf-8?B?KOa4uOmbheaDoCk=?= , srv_heupstream , Jungo Lin =?utf-8?B?KOael+aYjuS/iik=?= , Po-Yang Huang =?utf-8?B?KOm7g+afj+mZvSk=?= , "linux-mediatek@lists.infradead.org" , CK Hu =?utf-8?B?KOiDoeS/iuWFiSk=?= , Sj Huang =?utf-8?B?KOm7g+S/oeeSiyk=?= , "yuzhao@chromium.org" , "lkml@metux.net" , "zwisler@chromium.org" , Frederic Chen =?utf-8?B?KOmZs+S/iuWFgyk=?= , "matthias.bgg@gmail.com" , hans.verkuil@cisco.com, "mchehab@kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-media@vger.kernel.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Tue, Oct 15, 2019 at 11:16:15AM +0800, Jerry-ch Chen wrote: > Hi Tomasz, > > On Fri, 2019-09-06 at 18:11 +0800, Jerry-ch Chen wrote: > > From: Jerry-ch Chen > > > > This patch adds the driver of Face Detection (FD) unit in > > Mediatek camera system, providing face detection function. > > > > 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: Jerry-ch Chen > > --- > > drivers/media/platform/Kconfig | 2 + > > drivers/media/platform/Makefile | 2 + > > drivers/media/platform/mtk-isp/fd/Kconfig | 19 + > > drivers/media/platform/mtk-isp/fd/Makefile | 5 + > > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 148 ++ > > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1219 +++++++++++++++++ > > include/uapi/linux/mtk-fd-v4l2-controls.h | 69 + > > include/uapi/linux/v4l2-controls.h | 4 + > > 8 files changed, 1468 insertions(+) > > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig > > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c > > create mode 100644 include/uapi/linux/mtk-fd-v4l2-controls.h > > [snip] > > +static int mtk_fd_job_abort(struct mtk_fd_dev *fd) > > +{ > > + u32 ret; > > + > > + ret = wait_for_completion_timeout(&fd->fd_irq_done, > > + msecs_to_jiffies(MTK_FD_HW_TIMEOUT)); > > + /* Reset FD HW */ > > + if (!ret) { > > + struct ipi_message fd_ipi_msg; > > + > > + fd_ipi_msg.cmd_id = MTK_FD_IPI_CMD_RESET; > > + if (scp_ipi_send(fd->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg, > > + sizeof(fd_ipi_msg), MTK_FD_IPI_SEND_TIMEOUT)) > > + dev_err(fd->dev, "FD Reset HW error\n"); > > + return -ETIMEDOUT; > > + } > > + return 0; > > +} > > + > > Continue the discussion about job abort in RFC v2, > > I think the job_abort callback in v4l2_m2m_ops() might be useful. > > ref: > https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/media/v4l2-core/v4l2-mem2mem.c#L398 > https://elixir.bootlin.com/linux/v5.4-rc2/source/include/media/v4l2-mem2mem.h#L43 > > in drivers/media/v4l2-core/v4l2-mem2mem.c #398 v4l2_m2m_cancel_job() > ... > if (m2m_ctx->job_flags & TRANS_RUNNING) { > spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags); > if (m2m_dev->m2m_ops->job_abort) > m2m_dev->m2m_ops->job_abort(m2m_ctx->priv); > dprintk("m2m_ctx %p running, will wait to complete\n", m2m_ctx); > wait_event(m2m_ctx->finished, > !(m2m_ctx->job_flags & TRANS_RUNNING)); > } ... > > If this operation is set, we might use the v4l2_m2m_cancel_job() when > suspend, and it will do mtk_fd_job_abort() here. > The expectation for .job_abort() is that signals the hardware to instantly abandon the current job. Do we have a way to tell the firmware/hardware to do so? Also, suspend must not abort the current job. Anything that was already running is expected to complete successfuly and further jobs should continue executing once the system resumes. [snip] > > + > > +static int mtk_fd_suspend(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + if (fd->fd_stream_count) > > + if (mtk_fd_job_abort(fd)) > > + mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR); > > + > > To avoid mtk_fd_hw_job_finish() trigger the next job, > I suppose that we could use v4l2_m2m_cancel_job instead of job_abort and > job_finish here. > > /** > * v4l2_m2m_cancel_job() - cancel pending jobs for the context > * @m2m_ctx: m2m context with jobs to be canceled > * > * In case of streamoff or release called on any context, > * 1] If the context is currently running, then abort job will be called > * 2] If the context is queued, then the context will be removed from > * the job_queue > */ > > or another way, > we may add a flag and implement mtk_fd_job_ready() that reads the flag > if we suspend, we set the flag and do job_abort and job_finish, even if > it try enqueue, it will still not really queue the job, until we reset > the flag in mtk_fd_resume(). > > how do you think? > As per my comment above, suspend must just pause the execution of the jobs. It must not cause any jobs to be skipped. After analyzing the m2m framework and existing m2m drivers I realized that they currently provide no way to correctly handle suspend/resume. Pi-Hsun has been looking into fixing this in crrev.com/c/1878112 and we'll send it upstream as soon as we get something that should handle all the cases correctly. > > + /* suspend FD HW */ > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + clk_disable_unprepare(fd->fd_clk); > > + dev_dbg(dev, "%s:disable clock\n", __func__); > > + > > + return 0; > > +} > > + > > +static int mtk_fd_resume(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + int ret; > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + ret = clk_prepare_enable(fd->fd_clk); > > + if (ret < 0) { > > + dev_dbg(dev, "Failed to open fd clk:%d\n", ret); > > + return ret; > > + } > > + > > + /* resume FD HW */ > > + writel(MTK_FD_SET_HW_ENABLE, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + writel(0x1, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + dev_dbg(dev, "%s:enable clock\n", __func__); By the way, we need to kick the m2m framework here to schedule further jobs. Pi-Hsun's patch will also take care of this. [snip] > > +/* Set the face angle and directions to be detected */ > > +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 1) > > + > > +/* Set image widths for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 2) > > + > > +/* Set image heights for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 3) > > + > > +/* Set the length of scale down size array */ > > +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 4) > > + > > +/* Set the detection speed, usually reducing accuracy. */ > > +#define V4L2_CID_MTK_FD_DETECT_SPEED (V4L2_CID_USER_MTK_FD_BASE + 5) > > + > > +/* Select the detection model or algorithm to be used. */ > > +#define V4L2_CID_MTK_FD_DETECTION_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6) > > + > > +/* We reserve 16 controls for this driver. */ > > +#define V4L2_CID_MTK_FD_MAX 16 > > + > > For these control IDs, I think the following should be remained as chip > specific controls. > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH, > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT and V4L2_CID_MTK_FD_SCALE_IMG_NUM > > Hope there would be standardizing face detection api that cover the rest > controls: V4L2_CID_MTK_FD_DETECT_POSE, V4L2_CID_MTK_FD_DETECT_SPEED and > V4L2_CID_MTK_FD_DETECTION_MODEL > > Would you have any suggestions on how to propose the standard face > detection apis? > Given no follow up feedback from the community, I think we can keep them as driver-specific, but should make sure that they have some reasonable default values in case an application doesn't recognize them. Best regards, Tomasz _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3DD8CA9EA0 for ; Fri, 25 Oct 2019 03:52:34 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8370A20867 for ; Fri, 25 Oct 2019 03:52:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="qT8wq8Im"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Ce+0HrND" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8370A20867 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=YDcv4/GSp+8kPj3BOqfJtMGelk688U8ASdfIr6xD4G4=; b=qT8wq8ImjcJDqm e4rAjuRoyyA9fE1caXVBw5+88aQG7GF5MYJ4GMF2B22uk8ZR22fA2SKtULc2sjQJQgf0bstXcshO5 drl6nvcz1MoHvXsQkNk4zPmLXGFMh0gX2Lq8HgmwWnioghQcah5O0IZXuWO/vmTry2+a6SjV8K0jT 5XgMxJ10yyQwuOSqHhBpSkgYJZ17PVFUHm3ygb3FaWKOHw3sJ7jDTxS7TxbZfA1p0TIpttm7B4PRH YWUOBGoOP+1L/9Gk92vPy3yRsjlDWFCpKUefjVJALGjoJsiycUYt3Lc3dbyMYE+Em1XkhridbaQlG jAEkafaPc9VpFrcuGdKw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iNqeN-0005Re-0G; Fri, 25 Oct 2019 03:52:31 +0000 Received: from mail-pg1-x544.google.com ([2607:f8b0:4864:20::544]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iNqeI-0005QT-Ea for linux-arm-kernel@lists.infradead.org; Fri, 25 Oct 2019 03:52:28 +0000 Received: by mail-pg1-x544.google.com with SMTP id e10so605994pgd.11 for ; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=Ce+0HrNDmFSivub2oBEJVlA0HHWdPOu5+6DmZIJe7npEriRRFg631g4lekMZY0vAiE X3w8JaBKz/xk8MUr8g0dIKO02+oQdsng0Yd15tGlAmwd6MIRY+eKa82ZieljWuM1/TXA lWrkonKegWaVKApjLAbjNz8voQ4f1oncASGEE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VuqGlHCmiP7nazFalCZDat6IlaETXhzlf7ym/08sQW8=; b=qTQZQUx0ABN4/UUQQ85VnGn3NwnBlbAIXlkDyWXhMl9xik6vENEkDUeBW6Z/IuKhe2 hj6BmtERGWMrcaenDJgvy1xQrpZ7nGrDx/9N9+Fc1c9YCxwsNPcoCOIAJOdHUN1/0SHk ZWi9U0vnkNbifHbYEU8tXOdr29JCjwK94NschRK5U96O8yfO85mRFjFd8hjNRB40082G tgkBSUMVIkzTz1T9bVWfPDan809tLRbnke3+43dbOWh9onilGNlVayQvjtMvWFkyQBOv Qk+hIpp3zDP0dTNFEEAR9nvMQRq1jzGB8R0kHzlj+PW88IjBU9BI0AxcrvFBK8k3k1dN rVhQ== X-Gm-Message-State: APjAAAXgMxXRORd5zHMI1f59s5Zpsjk8HZmb7P1ioIPYeJU8NHYdl5m0 3HvfolzbD87AlAKwbGuVsGjQog== X-Google-Smtp-Source: APXvYqwkwetVcs47sPZxLr5ISxVfMjs558/KCCIrpnAsnIMnYSTi2p/HmtYweJj67U8UZ7Y8mq9qFw== X-Received: by 2002:a63:4c1c:: with SMTP id z28mr1507674pga.167.1571975545045; Thu, 24 Oct 2019 20:52:25 -0700 (PDT) Received: from chromium.org ([2401:fa00:8f:203:f5fe:2a5e:f953:c0ed]) by smtp.gmail.com with ESMTPSA id c16sm571818pja.2.2019.10.24.20.52.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Oct 2019 20:52:17 -0700 (PDT) Date: Fri, 25 Oct 2019 12:52:11 +0900 From: Tomasz Figa To: Jerry-ch Chen Subject: Re: [RFC PATCH V3 3/3] platform: mtk-isp: Add Mediatek FD driver Message-ID: <20191025035211.GA67000@chromium.org> References: <20190906101125.3784-1-Jerry-Ch.chen@mediatek.com> <20190906101125.3784-4-Jerry-Ch.chen@mediatek.com> <1571109375.3706.40.camel@mtksdccf07> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1571109375.3706.40.camel@mtksdccf07> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191024_205226_514263_2D281942 X-CRM114-Status: GOOD ( 32.91 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , Sean Cheng =?utf-8?B?KOmEreaYh+W8mCk=?= , "laurent.pinchart+renesas@ideasonboard.com" , Rynn Wu =?utf-8?B?KOWQs+iCsuaBqSk=?= , Christie Yu =?utf-8?B?KOa4uOmbheaDoCk=?= , srv_heupstream , Jungo Lin =?utf-8?B?KOael+aYjuS/iik=?= , Po-Yang Huang =?utf-8?B?KOm7g+afj+mZvSk=?= , "linux-mediatek@lists.infradead.org" , CK Hu =?utf-8?B?KOiDoeS/iuWFiSk=?= , Sj Huang =?utf-8?B?KOm7g+S/oeeSiyk=?= , "yuzhao@chromium.org" , "lkml@metux.net" , "zwisler@chromium.org" , Frederic Chen =?utf-8?B?KOmZs+S/iuWFgyk=?= , "matthias.bgg@gmail.com" , hans.verkuil@cisco.com, "mchehab@kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-media@vger.kernel.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Oct 15, 2019 at 11:16:15AM +0800, Jerry-ch Chen wrote: > Hi Tomasz, > > On Fri, 2019-09-06 at 18:11 +0800, Jerry-ch Chen wrote: > > From: Jerry-ch Chen > > > > This patch adds the driver of Face Detection (FD) unit in > > Mediatek camera system, providing face detection function. > > > > 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: Jerry-ch Chen > > --- > > drivers/media/platform/Kconfig | 2 + > > drivers/media/platform/Makefile | 2 + > > drivers/media/platform/mtk-isp/fd/Kconfig | 19 + > > drivers/media/platform/mtk-isp/fd/Makefile | 5 + > > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 148 ++ > > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1219 +++++++++++++++++ > > include/uapi/linux/mtk-fd-v4l2-controls.h | 69 + > > include/uapi/linux/v4l2-controls.h | 4 + > > 8 files changed, 1468 insertions(+) > > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig > > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c > > create mode 100644 include/uapi/linux/mtk-fd-v4l2-controls.h > > [snip] > > +static int mtk_fd_job_abort(struct mtk_fd_dev *fd) > > +{ > > + u32 ret; > > + > > + ret = wait_for_completion_timeout(&fd->fd_irq_done, > > + msecs_to_jiffies(MTK_FD_HW_TIMEOUT)); > > + /* Reset FD HW */ > > + if (!ret) { > > + struct ipi_message fd_ipi_msg; > > + > > + fd_ipi_msg.cmd_id = MTK_FD_IPI_CMD_RESET; > > + if (scp_ipi_send(fd->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg, > > + sizeof(fd_ipi_msg), MTK_FD_IPI_SEND_TIMEOUT)) > > + dev_err(fd->dev, "FD Reset HW error\n"); > > + return -ETIMEDOUT; > > + } > > + return 0; > > +} > > + > > Continue the discussion about job abort in RFC v2, > > I think the job_abort callback in v4l2_m2m_ops() might be useful. > > ref: > https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/media/v4l2-core/v4l2-mem2mem.c#L398 > https://elixir.bootlin.com/linux/v5.4-rc2/source/include/media/v4l2-mem2mem.h#L43 > > in drivers/media/v4l2-core/v4l2-mem2mem.c #398 v4l2_m2m_cancel_job() > ... > if (m2m_ctx->job_flags & TRANS_RUNNING) { > spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags); > if (m2m_dev->m2m_ops->job_abort) > m2m_dev->m2m_ops->job_abort(m2m_ctx->priv); > dprintk("m2m_ctx %p running, will wait to complete\n", m2m_ctx); > wait_event(m2m_ctx->finished, > !(m2m_ctx->job_flags & TRANS_RUNNING)); > } ... > > If this operation is set, we might use the v4l2_m2m_cancel_job() when > suspend, and it will do mtk_fd_job_abort() here. > The expectation for .job_abort() is that signals the hardware to instantly abandon the current job. Do we have a way to tell the firmware/hardware to do so? Also, suspend must not abort the current job. Anything that was already running is expected to complete successfuly and further jobs should continue executing once the system resumes. [snip] > > + > > +static int mtk_fd_suspend(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + if (fd->fd_stream_count) > > + if (mtk_fd_job_abort(fd)) > > + mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR); > > + > > To avoid mtk_fd_hw_job_finish() trigger the next job, > I suppose that we could use v4l2_m2m_cancel_job instead of job_abort and > job_finish here. > > /** > * v4l2_m2m_cancel_job() - cancel pending jobs for the context > * @m2m_ctx: m2m context with jobs to be canceled > * > * In case of streamoff or release called on any context, > * 1] If the context is currently running, then abort job will be called > * 2] If the context is queued, then the context will be removed from > * the job_queue > */ > > or another way, > we may add a flag and implement mtk_fd_job_ready() that reads the flag > if we suspend, we set the flag and do job_abort and job_finish, even if > it try enqueue, it will still not really queue the job, until we reset > the flag in mtk_fd_resume(). > > how do you think? > As per my comment above, suspend must just pause the execution of the jobs. It must not cause any jobs to be skipped. After analyzing the m2m framework and existing m2m drivers I realized that they currently provide no way to correctly handle suspend/resume. Pi-Hsun has been looking into fixing this in crrev.com/c/1878112 and we'll send it upstream as soon as we get something that should handle all the cases correctly. > > + /* suspend FD HW */ > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + clk_disable_unprepare(fd->fd_clk); > > + dev_dbg(dev, "%s:disable clock\n", __func__); > > + > > + return 0; > > +} > > + > > +static int mtk_fd_resume(struct device *dev) > > +{ > > + struct mtk_fd_dev *fd = dev_get_drvdata(dev); > > + int ret; > > + > > + if (pm_runtime_suspended(dev)) > > + return 0; > > + > > + ret = clk_prepare_enable(fd->fd_clk); > > + if (ret < 0) { > > + dev_dbg(dev, "Failed to open fd clk:%d\n", ret); > > + return ret; > > + } > > + > > + /* resume FD HW */ > > + writel(MTK_FD_SET_HW_ENABLE, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE); > > + writel(0x1, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN); > > + dev_dbg(dev, "%s:enable clock\n", __func__); By the way, we need to kick the m2m framework here to schedule further jobs. Pi-Hsun's patch will also take care of this. [snip] > > +/* Set the face angle and directions to be detected */ > > +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 1) > > + > > +/* Set image widths for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 2) > > + > > +/* Set image heights for an input image to be scaled down for face detection */ > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 3) > > + > > +/* Set the length of scale down size array */ > > +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 4) > > + > > +/* Set the detection speed, usually reducing accuracy. */ > > +#define V4L2_CID_MTK_FD_DETECT_SPEED (V4L2_CID_USER_MTK_FD_BASE + 5) > > + > > +/* Select the detection model or algorithm to be used. */ > > +#define V4L2_CID_MTK_FD_DETECTION_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6) > > + > > +/* We reserve 16 controls for this driver. */ > > +#define V4L2_CID_MTK_FD_MAX 16 > > + > > For these control IDs, I think the following should be remained as chip > specific controls. > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH, > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT and V4L2_CID_MTK_FD_SCALE_IMG_NUM > > Hope there would be standardizing face detection api that cover the rest > controls: V4L2_CID_MTK_FD_DETECT_POSE, V4L2_CID_MTK_FD_DETECT_SPEED and > V4L2_CID_MTK_FD_DETECTION_MODEL > > Would you have any suggestions on how to propose the standard face > detection apis? > Given no follow up feedback from the community, I think we can keep them as driver-specific, but should make sure that they have some reasonable default values in case an application doesn't recognize them. Best regards, Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel