From patchwork Fri Jan 21 13:14:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eugen Hristev X-Patchwork-Id: 534404 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D905FC433FE for ; Fri, 21 Jan 2022 13:16:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1380613AbiAUNQC (ORCPT ); Fri, 21 Jan 2022 08:16:02 -0500 Received: from esa.microchip.iphmx.com ([68.232.154.123]:4251 "EHLO esa.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1380605AbiAUNQB (ORCPT ); Fri, 21 Jan 2022 08:16:01 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1642770961; x=1674306961; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=YZru3/QBZXrRNtdO1rPveO1bY1mVoT2xWIXCdHdFzIw=; b=t5Mg7ZnRjtt0MN83WCfu1cN4FPxMexfJZ7vUopKPW1imGpvWdPJnaygp i4wl/DkP7NsWi9aFniuDzXSW2fbBGdOQlYAxVDMblCq1UWDL00hi0E/0X PiSj4gzA2rK2+cd93q0uD/5iBVFz4yeCYep5lZ582byG1bGhy/SST1qzi NsBf5AAqCuoGvRpGeKW2GJLWzfCJnGcyYrKevrd0WN06ZjHmR5hX08spQ OaNp9siOPsqZFVyVc+S4oxxiMOrcth5ieEpACAe41yZ8WFnD4G/6dqnAd 0VNyJiYfz2qkFKunEj5oUilrE+ZN0xjf/YFcDid+pf97yrV7NP9C8I5Up Q==; IronPort-SDR: uUkUmY5ojFv05GESCdqxR32Wxun1QNrZ0ziQJaQLHsShUHiR9ZhvJZZMuKQnLsnJpnlK9ERuOM rWa5kNTkisil0tJ8YeDnh4EavWDAxm+cFg/hvWXGpEo2Z54M/BCDyJenCfqK+PwWVAB+cbCtDB X0NY31iUxxmC6A0NGSmpUSPRz5RYe5E/1vGnUsZq49ZrEWq5mzszffY1zAoDcB3RTfcu6S2Zf9 o1h+fntF97dBQ/YKegRYAy9g5MSSw9VFkqKI0NYihNGLCSaRBUrnMka9jQhfawcU760tQtWkaO wbElDuuHHnVaA6coHfBwt5QW X-IronPort-AV: E=Sophos;i="5.88,304,1635231600"; d="scan'208";a="143468650" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa4.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 21 Jan 2022 06:16:00 -0700 Received: from chn-vm-ex04.mchp-main.com (10.10.85.152) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Fri, 21 Jan 2022 06:15:58 -0700 Received: from ROB-ULT-M18282.microchip.com (10.10.115.15) by chn-vm-ex04.mchp-main.com (10.10.85.152) with Microsoft SMTP Server id 15.1.2375.17 via Frontend Transport; Fri, 21 Jan 2022 06:15:45 -0700 From: Eugen Hristev To: , , , , , , CC: , , , Eugen Hristev Subject: [PATCH v4 05/11] media: atmel: atmel-isc-base: use mutex to lock awb workqueue from streaming Date: Fri, 21 Jan 2022 15:14:10 +0200 Message-ID: <20220121131416.603972-6-eugen.hristev@microchip.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com> References: <20220121131416.603972-1-eugen.hristev@microchip.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.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 vb2_start_streaming_called 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 6b482270eb93..f7a88399bd54 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -400,6 +400,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); /* Wait until the end of the current frame */ @@ -407,6 +408,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); @@ -1395,10 +1398,6 @@ static void isc_awb_work(struct work_struct *w) u32 min, max; int ret; - /* streaming is not active anymore */ - if (!vb2_start_streaming_called(&isc->vb2_vidq)) - return; - if (ctrls->hist_stat != HIST_ENABLED) return; @@ -1453,7 +1452,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 (!vb2_start_streaming_called(&isc->vb2_vidq)) { + 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); @@ -1532,6 +1548,8 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl) isc_update_awb_ctrls(isc); + mutex_lock(&isc->awb_mutex); + if (vb2_start_streaming_called(&isc->vb2_vidq)) { /* * If we are streaming, we can update profile to @@ -1546,6 +1564,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 && @@ -1738,6 +1757,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); @@ -1848,6 +1868,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 */ @@ -1927,6 +1949,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 c9234c90ae58..04ea5e340808 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -305,6 +305,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];