From patchwork Fri Oct 22 07:52:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eugen Hristev X-Patchwork-Id: 12577371 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1FD5C433F5 for ; Fri, 22 Oct 2021 08:11:13 +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 AFBB3610EA for ; Fri, 22 Oct 2021 08:11:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AFBB3610EA Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=microchip.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=hSI8cz9YY/bAkGeIsO3qs7ce6hCZZ0iuM7Hp55bYZfw=; b=fIPjpxRMomHvSz FnyUKr5PqI2DpZN91gmR4lvAK2pahb2AGvF4VqBBcH8OhJnOxA13UB2yEGWyD81RtNx1EeVyZe4bB SXR1jV86pLAd+T2p3+yAljCUZU7kNdysYUKTuC4oxaycHl6oZzHGRoDkQgE+yDE5bi5Tm6XuuZBwe 2pm53Mdu6dMVE/TFuA6KKhmxdKFPRPRwcgJEgR6R4nhgdRHVnnZcbYpFR5y86jPJXJBOx1QfvWJdc hRO4Ic49ChoePRoDdwMjY4rLauf1lYMvqh6WXYzvG8DWHWVGvgLKOOtCHysfBlqQlCz67ui3zDgKk 2TkCqS4GbGQ2EiESfJGg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdpbR-00A7oW-9w; Fri, 22 Oct 2021 08:08:38 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdpOB-00A2bd-CO for linux-arm-kernel@lists.infradead.org; Fri, 22 Oct 2021 07:54:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1634889295; x=1666425295; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=1GT8uqsv1PvTE+w2q+eOd1uaZzWC5KHZ8sqS85toEos=; b=UErz2OkU8Ezsya7ozGYmTkd/UdgkpN/wn/REv00ASMGOvZCTz0gjbiwz zgftbAYf7ChnXXbGcB0ui9XHWZctPUzLumktl9NGAtthOySGbh/D/B10/ idawFj3kDFl4JOu+g6474ZnO4UqQRXHiA95hqZYc6bTfYixPRymNWL7Wg 3+vuiHJ8dv3XksMqETPLJI/gogVwLmIPpPN0JcwXwZ+gLm9DqRChwNv0/ R8sttzuARICrw1xxPifNnxSVNCAMH9Mqk7qg2Y0F9tyFKVH//t+5DuK88 re1fpIV0FUEbLx0YbTvz6+Z7wIHRb80yKc84q5ve1V62exEyzLeurK9mm A==; IronPort-SDR: GjcNflvWfkh5na0E2ZC3jyHo3m+dK+n/f/FFivCstzNKx8s89kXKRnPl6zL4Yq7aLtzJ97zX9r +DqbXXrMzQUUfRjwdKxOtEjgxwIv7r7y1iBaHUwGHhFxLQ0avUMr5/4OtPE5ZDWkinX1Vn926c nSbPusXwGBg2UU+ME4y/cAnFuZxn6ITCW8PnKxnLGU3n8nM8b9y/OcYjCk7ncGcuXBw0PuocOK ETwlUMgVTcVmEi92hS3grSCmehDbkjrvrNNFgzqBRJUyTPEWITetAuMSuxVEfMb1PP3j7TrXrY neMq0LDg2GOW/gEGBvYMsYjX X-IronPort-AV: E=Sophos;i="5.87,172,1631602800"; d="scan'208";a="141320579" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa3.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 22 Oct 2021 00:54:54 -0700 Received: from chn-vm-ex02.mchp-main.com (10.10.85.144) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Fri, 22 Oct 2021 00:54:54 -0700 Received: from ROB-ULT-M18282.microchip.com (10.10.115.15) by chn-vm-ex02.mchp-main.com (10.10.85.144) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Fri, 22 Oct 2021 00:54:50 -0700 From: Eugen Hristev To: , , , Subject: [PATCH 19/21] media: atmel: atmel-isc-base: use mutex to lock awb workqueue from streaming Date: Fri, 22 Oct 2021 10:52:45 +0300 Message-ID: <20211022075247.518880-20-eugen.hristev@microchip.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211022075247.518880-1-eugen.hristev@microchip.com> References: <20211022075247.518880-1-eugen.hristev@microchip.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211022_005455_488851_B653ECC2 X-CRM114-Status: GOOD ( 20.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jacopo@jmondi.org, robh+dt@kernel.org, sakari.ailus@iki.fi, laurent.pinchart@ideasonboard.com, Eugen Hristev Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The AWB workqueue runs in a kernel thread and needs to be synchronized w.r.t. the streaming status. It is possible that streaming is stopped while the AWB workq is running. In this case it is likely that the check for isc->stop is done at one point in time, but the AWB computations are done later, including a call to isc_update_profile, which requires streaming to be started. Thus , isc_update_profile will fail if during this operation sequence the streaming was stopped. To solve this issue, a mutex is added, that will serialize the awb work and streaming stopping, with the mention that either streaming is stopped completely including termination of the last frame is done, and after that the AWB work can check stream status and stop; either first AWB work is completed and after that the streaming can stop correctly. The awb spin lock cannot be used since this spinlock is taken in the same context and using it in the stop streaming will result in a recursion BUG. Signed-off-by: Eugen Hristev --- drivers/media/platform/atmel/atmel-isc-base.c | 31 ++++++++++++++++--- drivers/media/platform/atmel/atmel-isc.h | 1 + 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c index a0e4bc75d1a2..7ebe0a2d130f 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -402,6 +402,7 @@ static void isc_stop_streaming(struct vb2_queue *vq) struct isc_buffer *buf; int ret; + mutex_lock(&isc->awb_mutex); v4l2_ctrl_activate(isc->do_wb_ctrl, false); isc->stop = true; @@ -411,6 +412,8 @@ static void isc_stop_streaming(struct vb2_queue *vq) v4l2_err(&isc->v4l2_dev, "Timeout waiting for end of the capture\n"); + mutex_unlock(&isc->awb_mutex); + /* Disable DMA interrupt */ regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE); @@ -1417,10 +1420,6 @@ static void isc_awb_work(struct work_struct *w) u32 min, max; int ret; - /* streaming is not active anymore */ - if (isc->stop) - return; - if (ctrls->hist_stat != HIST_ENABLED) return; @@ -1471,7 +1470,24 @@ static void isc_awb_work(struct work_struct *w) } regmap_write(regmap, ISC_HIS_CFG + isc->offsets.his, hist_id | baysel | ISC_HIS_CFG_RAR); + + /* + * We have to make sure the streaming has not stopped meanwhile. + * ISC requires a frame to clock the internal profile update. + * To avoid issues, lock the sequence with a mutex + */ + mutex_lock(&isc->awb_mutex); + + /* streaming is not active anymore */ + if (isc->stop) { + mutex_unlock(&isc->awb_mutex); + return; + }; + isc_update_profile(isc); + + mutex_unlock(&isc->awb_mutex); + /* if awb has been disabled, we don't need to start another histogram */ if (ctrls->awb) regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ); @@ -1550,6 +1566,8 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl) isc_update_awb_ctrls(isc); + mutex_lock(&isc->awb_mutex); + if (!isc->stop) { /* * If we are streaming, we can update profile to @@ -1564,6 +1582,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl) */ v4l2_ctrl_activate(isc->do_wb_ctrl, false); } + mutex_unlock(&isc->awb_mutex); /* if we have autowhitebalance on, start histogram procedure */ if (ctrls->awb == ISC_WB_AUTO && !isc->stop && @@ -1755,6 +1774,7 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier, { struct isc_device *isc = container_of(notifier->v4l2_dev, struct isc_device, v4l2_dev); + mutex_destroy(&isc->awb_mutex); cancel_work_sync(&isc->awb_work); video_unregister_device(&isc->video_dev); v4l2_ctrl_handler_free(&isc->ctrls.handler); @@ -1866,6 +1886,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier) isc->current_subdev = container_of(notifier, struct isc_subdev_entity, notifier); mutex_init(&isc->lock); + mutex_init(&isc->awb_mutex); + init_completion(&isc->comp); /* Initialize videobuf2 queue */ @@ -1941,6 +1963,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier) video_unregister_device(vdev); isc_async_complete_err: + mutex_destroy(&isc->awb_mutex); mutex_destroy(&isc->lock); return ret; } diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h index 27292c50915e..93ee000279f2 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -294,6 +294,7 @@ struct isc_device { struct work_struct awb_work; struct mutex lock; /* serialize access to file operations */ + struct mutex awb_mutex; /* serialize access to streaming status from awb work queue */ spinlock_t awb_lock; /* serialize access to DMA buffers from awb work queue */ struct regmap_field *pipeline[ISC_PIPE_LINE_NODE_NUM];