From patchwork Mon Sep 30 08:32:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 831750 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2B38111CA9 for ; Mon, 30 Sep 2024 08:33:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727685207; cv=none; b=fcz2dt5dq2cuKd5aQ4ErMhnAxspktCvMNJc9tiE0n1zJxRedl+eIS1AKtAivw9XRp2snb7YCXkHfnViDRvhi1x1ed4UBO4qQyM8F3ShX975L+pihxBSK7mWdFLIR5rvNGGebSziAwsZdbFr0jZ3C7lbFgSoDw36VCoFLCMaI4xM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727685207; c=relaxed/simple; bh=MC3H2ib4p1ZOfnsOF7w8rHpvoPlM/t2s1U7C5gPM1/8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=Cl+RiqQrdtoy90xeYUYvpiqqTNxofiQOWul/VbHG+eXUdzrGOO3funqa1l9DVyPc1lMwHO6bL5IbDQSrKXg/vURH7/V7VRi+YMbRG9cjZHkG7XR+iL4n2N1TVXzofg7Qv3LPFSfT8CPEOYTakIknaFQ2BMxKyMORLZCOuGNogRg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=YfsHsZTu; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="YfsHsZTu" Received: from fedora.local (unknown [95.131.45.214]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 95D37B7E; Mon, 30 Sep 2024 10:31:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1727685112; bh=MC3H2ib4p1ZOfnsOF7w8rHpvoPlM/t2s1U7C5gPM1/8=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=YfsHsZTuZ3VgA1d6jPLVBu9DLgUFx1Xi2qmhZra3Y/MwbQiVgvRRZi3t4uFcMDuFh 5ldGsXeuXwHakBDN0K+DSbvWliyeDaJRiRjSMZlS2gQ+o/BmCC31pssftyG3jL6qUL 73k7BueCZ5+ijqv+t2N0IPAHTq3AegDQsIWPdqLI= From: Jacopo Mondi Date: Mon, 30 Sep 2024 10:32:58 +0200 Subject: [PATCH v6 2/4] media: pisp_be: Remove config validation from schedule() Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-2-63d60f9dd10f@ideasonboard.com> References: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-0-63d60f9dd10f@ideasonboard.com> In-Reply-To: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-0-63d60f9dd10f@ideasonboard.com> To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: linux-media@vger.kernel.org, Jacopo Mondi X-Mailer: b4 0.15-dev-dedf8 X-Developer-Signature: v=1; a=openpgp-sha256; l=3030; i=jacopo.mondi@ideasonboard.com; h=from:subject:message-id; bh=MC3H2ib4p1ZOfnsOF7w8rHpvoPlM/t2s1U7C5gPM1/8=; b=owEBbQKS/ZANAwAIAXI0Bo8WoVY8AcsmYgBm+mJPVkjyrIYPsdOx2YtiOIzO77BhurkYbLxUy Z6ExHBwzweJAjMEAAEIAB0WIQS1xD1IgJogio9YOMByNAaPFqFWPAUCZvpiTwAKCRByNAaPFqFW PHL5D/0T2i5rL6YSiSvlotG3S/gzKG1dImDqpsV1dZ7ey2UzMqt4HLQeKgvhRqJWB3lt3NuWoT6 ibIGGZNQBIIL/2npzdWHAtxHSN73EKm97dRoOeZSgQBdJsSQyxTx0aeRVVyJpdH1dTCNtduOzvT DsNLan6rwhreZmekD0LDtTMxtD+cjghTLTGZ5+51NDFvzB/kE0heXL+W8XDMdyVZZ0od4jFOXJB 1ItGbMZDPMVwRTsfNujUSVidUfKgacDPdl/Ful11saLCzvzozLj5PWGliXjaceMseB2NkhSrxTm XrFN4HnJbKrzwrEFT6s7PifRwih65x/wXf8FbQZqB2R+hMSjt5OQCN/CodWvXg33jFJUMy9tQ7f 34d+hWBpRY3s+WUVOdBMu6rROa09JHSnFh9xFc59gi5/7ipCBL4w3LPqhFPnVIO0d4LQGYVUNIA gKYWO1pFEzBxgQVeY0J4wgA51XxxafoRdQYkn53F8puO00+4wv846dTdYfiATvLb9fgw0CzMask WrMD1fn8ayc1hjuaSj2Z6wONW/vxE5PdOvjFmXOoybIKuYqIHDBBeEjCug0A2O9hzxz7qZQxmyA Js09AGHbC4RTJuxo7ZqstFr+TWEOib4L/c3TkRuoSgTbd2oERPfagOR5Ncm4SCT1UyHlPitGt1F MYXimmfyIjm8y9g== X-Developer-Key: i=jacopo.mondi@ideasonboard.com; a=openpgp; fpr=72392EDC88144A65C701EA9BA5826A2587AD026B The config parameters buffer is already validated in pisp_be_validate_config() at .buf_prepare() time. However some of the same validations are also performed at pispbe_schedule() time. In particular the function checks that: 1) config.num_tiles is valid 2) At least one of the BAYER or RGB input is enabled The input config validation is already performed in pisp_be_validate_config() and while job.hw_enables is modified by pispbe_xlate_addrs(), the function only resets the input masks if - there is no input buffer available, but pispbe_prepare_job() fails before calling pispbe_xlate_addrs() in this case - bayer_enable is 0, but in this case rgb_enable is valid as guaranteed by pisp_be_validate_config() - only outputs are reset in rgb_enable For this reasons there is no need to repeat the check at pispbe_schedule() time. The num_tiles validation can be moved to pisp_be_validate_config() as well. As num_tiles is a u32 it can'be be < 0, so change the sanity check accordingly. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- .../media/platform/raspberrypi/pisp_be/pisp_be.c | 25 ++++++---------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 8ba1b9f43ba1590753d9601123f9261a3bf41b8e..41fd68b7757b38d4d71b27598b31888fa399c97c 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -588,24 +588,6 @@ static void pispbe_schedule(struct pispbe_dev *pispbe, bool clear_hw_busy) pispbe->hw_busy = true; spin_unlock_irqrestore(&pispbe->hw_lock, flags); - if (job.config->num_tiles <= 0 || - job.config->num_tiles > PISP_BACK_END_NUM_TILES || - !((job.hw_enables.bayer_enables | job.hw_enables.rgb_enables) & - PISP_BE_BAYER_ENABLE_INPUT)) { - /* - * Bad job. We can't let it proceed as it could lock up - * the hardware, or worse! - * - * For now, just force num_tiles to 0, which causes the - * H/W to do something bizarre but survivable. It - * increments (started,done) counters by more than 1, - * but we seem to survive... - */ - dev_dbg(pispbe->dev, "Bad job: invalid number of tiles: %u\n", - job.config->num_tiles); - job.config->num_tiles = 0; - } - pispbe_queue_job(pispbe, &job); return; @@ -703,6 +685,13 @@ static int pisp_be_validate_config(struct pispbe_dev *pispbe, return -EIO; } + if (config->num_tiles == 0 || + config->num_tiles > PISP_BACK_END_NUM_TILES) { + dev_dbg(dev, "%s: Invalid number of tiles: %d\n", __func__, + config->num_tiles); + return -EINVAL; + } + /* Ensure output config strides and buffer sizes match the V4L2 formats. */ fmt = &pispbe->node[TDN_OUTPUT_NODE].format; if (bayer_enables & PISP_BE_BAYER_ENABLE_TDN_OUTPUT) { From patchwork Mon Sep 30 08:33:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 831749 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3A9F511CA9 for ; Mon, 30 Sep 2024 08:33:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727685210; cv=none; b=TWQ7PeU69t5OfHjswRCFzLwgzMR9HW1oUYocGSPL30pI3bDojb2Vy7d4MpOZiaZWroZrK476Z1LG+C9fpWD15F+xnnbc6IE648UaK/IFT15l1lXCaK7kxjiD+R6c6wnud1ftZZupXerLsliZCfnUtGMlnl/65/QfN85I3UX0isk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727685210; c=relaxed/simple; bh=c8xaMOx5eNN5h0aFA5HA05QSMBs5dlneQNGC+y/HRis=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=dhgn7r4rgz81j+Wn3ojQ+rX/cz6XG+8HVCR0NI13TFGnjdDQZxSDrVrZG9iqssr+Aavn9xrdwey7FCOPAWP43zC6QWh0bSO/lNaCnMKeP2rkbT/FWkCocdB68nSnqnAH2CsN9bm0LiyuH10HT33oo6Yabxd0zP8LWyk0dkt+Its= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=gF8Xkek8; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gF8Xkek8" Received: from fedora.local (unknown [95.131.45.214]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0EB6539F; Mon, 30 Sep 2024 10:31:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1727685115; bh=c8xaMOx5eNN5h0aFA5HA05QSMBs5dlneQNGC+y/HRis=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=gF8Xkek8ulIv2naHSUdvO9w8NxS+ZgmnbOx1WL3tzsVsGQ+QMDE5pRSV9aHb+Mh2q x/9X5/TxPFpk4qCIt/GtArFH/q3mE/rACGYApIBiGJVzk3DGu0aTc5MWpU9TunMBtf x1ONB/czju0FcKMm2uqR7asDOtNHfPifCkGYKq8s= From: Jacopo Mondi Date: Mon, 30 Sep 2024 10:33:00 +0200 Subject: [PATCH v6 4/4] media: pisp_be: Fix pm_runtime underrun in probe Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-4-63d60f9dd10f@ideasonboard.com> References: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-0-63d60f9dd10f@ideasonboard.com> In-Reply-To: <20240930-pispbe-mainline-split-jobs-handling-v6-v6-0-63d60f9dd10f@ideasonboard.com> To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: linux-media@vger.kernel.org, Jacopo Mondi X-Mailer: b4 0.15-dev-dedf8 X-Developer-Signature: v=1; a=openpgp-sha256; l=3170; i=jacopo.mondi@ideasonboard.com; h=from:subject:message-id; bh=c8xaMOx5eNN5h0aFA5HA05QSMBs5dlneQNGC+y/HRis=; b=owEBbQKS/ZANAwAIAXI0Bo8WoVY8AcsmYgBm+mJPXfQQWeNX/+iQDi5wvde8xtVbCAIIDitYf WUysPkTRBOJAjMEAAEIAB0WIQS1xD1IgJogio9YOMByNAaPFqFWPAUCZvpiTwAKCRByNAaPFqFW PGW9D/0Q7xf8jLYTmLTsPW3NWnu3gsMVxywU/agjQ7hrra3Whiw0rJlprirCag+okDthn+OfUck EpOCUIVADux4rfJ4UvkuuILIAB6xdGt4UFmO8dcTU9aG+MXLf0SJLGSJrtAZ8ABqv7pECrl00Dr iLeqXQi0eYK/4CUaZaKy/V5ZG3RkLcJ30x491cT8PN7zLo1Ppix3tTF/ZI+Kz5m9jglXhoS/8RL ry3Q7FZC7I74Jbqj3EqJ1l5pcraaZlOnfcS+MzdGMf3jLrMCnRM6Oj14UcHs2ff2N7zInSq2DX/ nOHoY9h4Xx7h2PawVwCNzVKiqv9Fd6o66veYB7O87LyUJnh3Sv4HedPiNOYXl7pp3JeMv7WFg7W Czaiz+yJkw94Egync2M0lSVanKP0CFI8zAapyk6CVYTqwfaEPUjbo9EhWC/D5XJdn9Bn9MExb66 WotGYZuP7NpcDs+gFbT6N4SRa5+R9Mw81r1M4UgP6SgCRYY/8tzgo24k5ZquioJPbpEunr8nDx4 wpkb1EjQF+vlrCXbVIR+QyZAW3XWs+7piQ8ZvbF/EFD/EEAGUj+JCOIFRIILHZpRDEIAyGJL6nQ umcnZjKiVUo5Oqarf9Am7frydo12plvHgyzw9pUNIjpmGy3Tj6UwOk51oQ8fCVEWxk/rjyEjr5E BBGZQEO9KS6nPZg== X-Developer-Key: i=jacopo.mondi@ideasonboard.com; a=openpgp; fpr=72392EDC88144A65C701EA9BA5826A2587AD026B During the probe() routine, the PiSP BE driver needs to power up the interface in order to identify and initialize the hardware. The driver resumes the interface by calling the pispbe_runtime_resume() function directly, without going through the pm_runtime helpers, but later suspends it by calling pm_runtime_put_autosuspend(). This causes a PM usage count imbalance at probe time, notified by the runtime_pm framework with the below message in the system log: pispbe 1000880000.pisp_be: Runtime PM usage count underflow! Fix this by resuming the interface using the pm runtime helpers instead of calling the resume function directly and use the pm_runtime framework in the probe() error path. While at it, remove manual suspend of the interface in the remove() function. The driver cannot be unloaded if in use, so simply disable runtime pm. To simplify the implementation, make the driver depend on PM as the RPI5 platform where the ISP is integrated in uses the PM framework by default. Fixes: 12187bd5d4f8 ("media: raspberrypi: Add support for PiSP BE") Signed-off-by: Jacopo Mondi --- drivers/media/platform/raspberrypi/pisp_be/Kconfig | 1 + drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/Kconfig b/drivers/media/platform/raspberrypi/pisp_be/Kconfig index 46765a2e4c4d1573757ff842f208834216e582cb..a9e51fd94aadc6add70f883bfcea0c9fa91f0c4b 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/Kconfig +++ b/drivers/media/platform/raspberrypi/pisp_be/Kconfig @@ -3,6 +3,7 @@ config VIDEO_RASPBERRYPI_PISP_BE depends on V4L_PLATFORM_DRIVERS depends on VIDEO_DEV depends on ARCH_BCM2835 || COMPILE_TEST + depends on PM select VIDEO_V4L2_SUBDEV_API select MEDIA_CONTROLLER select VIDEOBUF2_DMA_CONTIG diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 1c1ced2b8d7017473ded018e4badbf0792da1819..c2f062b64380d848907a28bc2672e6d70fac8909 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -1718,7 +1718,7 @@ static int pispbe_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(pispbe->dev); pm_runtime_enable(pispbe->dev); - ret = pispbe_runtime_resume(pispbe->dev); + ret = pm_runtime_resume_and_get(pispbe->dev); if (ret) goto pm_runtime_disable_err; @@ -1740,7 +1740,7 @@ static int pispbe_probe(struct platform_device *pdev) disable_devs_err: pispbe_destroy_devices(pispbe); pm_runtime_suspend_err: - pispbe_runtime_suspend(pispbe->dev); + pm_runtime_put(pispbe->dev); pm_runtime_disable_err: pm_runtime_dont_use_autosuspend(pispbe->dev); pm_runtime_disable(pispbe->dev); @@ -1754,7 +1754,6 @@ static void pispbe_remove(struct platform_device *pdev) pispbe_destroy_devices(pispbe); - pispbe_runtime_suspend(pispbe->dev); pm_runtime_dont_use_autosuspend(pispbe->dev); pm_runtime_disable(pispbe->dev); }