From patchwork Wed Sep 7 11:47:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 12968927 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6504DC54EE9 for ; Wed, 7 Sep 2022 11:49:08 +0000 (UTC) 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:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=dTszA8lLTdAx4DFWPWKGcUpWr502tOtnMx+VXRk3eF0=; b=pL5YUQg/1P78+4 3fSXKh5xCWdr18G5OSMQA5gWMWyzd4FGnIH+JOXnmtYsHOXx6/tXeAD0isuRV/921gsNGMLGY7BRx FX7mkdO09YfmbfIjKOGKLO+TD7bMIR7pXoIc/HgOJXBe0CL+BGLnSCvTEA2/IAL32PvbJ6DyMRTz+ 7Pnb7Y0SoW4h8NbSJ2vMcwkjrlyoIFXxLA+ZMxAh/6PIFMxpdHk7x3Nd5EHO+VQme7jHrXh50ahGY lfVsDBPBVL78u1YfSbFyh7MLsCcefosMvZGmJvNTRQtsVL//frhmpOrnLRgQxu7YlYjbnlDuCC7Oa aNWe6Hdsv9toQZS20K8A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oVtXM-005poY-IZ; Wed, 07 Sep 2022 11:48:08 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oVtXH-005pl3-Tv for linux-arm-kernel@lists.infradead.org; Wed, 07 Sep 2022 11:48:05 +0000 Received: from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EC9D4DD; Wed, 7 Sep 2022 13:47:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1662551279; bh=5Gr6bH8FY/a+xkGxzxUB9O3UJ8qXYspamAEUzoCXd/I=; h=From:To:Cc:Subject:Date:From; b=Cqb942fZYtP8wy6ZAcvycqM00+nUuMy+HxHOW6mxV21wa9PLGYdrAXoG4giNnEtop LRxrSSO8ay+JEtmxkbgRMrFoIEjlFNRAVTi2iCdG3Elm8heH8djspN6cbmlAWMIzAa 6n4wGjexhsk/cfh1aVamZzhx9NAKK1CkhFP6gz2s= From: Paul Elder To: Rui Miguel Silva , Steve Longerbeam , linux-media@vger.kernel.org Cc: Paul Elder , Laurent Pinchart , Philipp Zabel , Mauro Carvalho Chehab , Greg Kroah-Hartman , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , linux-staging@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] media: imx7-media-csi: Add support for fast-tracking queued buffers Date: Wed, 7 Sep 2022 20:47:37 +0900 Message-Id: <20220907114737.1127612-1-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220907_044804_161788_ABFE7ACC X-CRM114-Status: GOOD ( 24.63 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The CSI hardware compatible with this driver handles buffers using a ping-pong mechanism with two sets of destination addresses. Normally, when an interrupt comes in to signal the completion of one buffer, say FB0, it assigns the next buffer in the queue to the next FB0, and the hardware starts to capture into FB1 in the meantime. In a buffer underrun situation, in the above example without loss of generality, if a new buffer is queued before the interrupt for FB0 comes in, we can program the buffer into FB1 (which is programmed with a dummy buffer, as there is a buffer underrun). This of course races with the interrupt that signals FB0 completion, as once that interrupt comes in, we are no longer guaranteed that the programming of FB1 was in time and must assume it was too late. This race is resolved partly by locking the programming of FB1. If it came after the interrupt for FB0, then the variable that is used to determine which FB to program would have been swapped by the interrupt handler. This alone isn't sufficient, however, because the interrupt could still be generated (thus the hardware starts capturing into the other fb) while the fast-tracking routine has the irq lock. Thus, after programming the fb register to fast-track the buffer, the isr also must be checked to confirm that an interrupt didn't come in the meantime. If it has, we must assume that programming the register for the fast-tracked buffer was not in time, and queue the buffer normally. Signed-off-by: Paul Elder Acked-by: Rui Miguel Silva Reviewed-by: Laurent Pinchart --- Changes in v2: - fix the potential race condition where the interrupt comes in while the fast tracking routine has the irqlock - change return value from int to bool --- drivers/staging/media/imx/imx7-media-csi.c | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c index a0553c24cce4..0ebef44a7627 100644 --- a/drivers/staging/media/imx/imx7-media-csi.c +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -1296,12 +1296,75 @@ static int imx7_csi_video_buf_prepare(struct vb2_buffer *vb) return 0; } +static bool imx7_csi_fast_track_buffer(struct imx7_csi *csi, + struct imx7_csi_vb2_buffer *buf) +{ + unsigned long flags; + dma_addr_t phys; + int buf_num; + u32 isr; + + if (!csi->is_streaming) + return false; + + phys = vb2_dma_contig_plane_dma_addr(&buf->vbuf.vb2_buf, 0); + + /* + * buf_num holds the fb id of the most recently (*not* the next + * anticipated) triggered interrupt. Without loss of generality, if + * buf_num is 0 and we get to this section before the irq for fb2, the + * buffer that we are fast-tracking into fb1 should be programmed in + * time to be captured into. If the irq for fb2 already happened, then + * buf_num would be 1, and we would fast-track the buffer into fb2 + * instead. This guarantees that we won't try to fast-track into fb1 + * and race against the start-of-capture into fb1. + * + * We only fast-track the buffer if the currently programmed buffer is + * a dummy buffer. We can check the active_vb2_buf instead as it is + * always modified along with programming the fb[1,2] registers via the + * lock (besides setup and cleanup). + */ + + spin_lock_irqsave(&csi->irqlock, flags); + + buf_num = csi->buf_num; + if (csi->active_vb2_buf[buf_num]) { + spin_unlock_irqrestore(&csi->irqlock, flags); + return false; + } + + imx7_csi_update_buf(csi, phys, buf_num); + + isr = imx7_csi_reg_read(csi, CSI_CSISR); + /* + * The interrupt for the /other/ fb just came (the isr hasn't run yet + * though, because we have the lock here); we can't be sure we've + * programmed buf_num fb in time, so queue the buffer to the buffer + * queue normally. No need to undo writing the fb register, since we + * won't return it as active_vb2_buf is NULL, so it's okay to + * potentially write it to both fb1 and fb2; only the one where it was + * queued normally will be returned. + */ + if (isr & (buf_num ? BIT_DMA_TSF_DONE_FB1 : BIT_DMA_TSF_DONE_FB2)) { + spin_unlock_irqrestore(&csi->irqlock, flags); + return false; + } + + csi->active_vb2_buf[buf_num] = buf; + + spin_unlock_irqrestore(&csi->irqlock, flags); + return true; +} + static void imx7_csi_video_buf_queue(struct vb2_buffer *vb) { struct imx7_csi *csi = vb2_get_drv_priv(vb->vb2_queue); struct imx7_csi_vb2_buffer *buf = to_imx7_csi_vb2_buffer(vb); unsigned long flags; + if (imx7_csi_fast_track_buffer(csi, buf)) + return; + spin_lock_irqsave(&csi->q_lock, flags); list_add_tail(&buf->list, &csi->ready_q);