From patchwork Mon Aug 26 14:43:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13777985 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 66CC118FC7C for ; Mon, 26 Aug 2024 14:44:02 +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=1724683444; cv=none; b=l+IVdjGsopPz9lKQFNxWZbQPX7wxIPMx6EK/DAi1plR0QGFu/TzGhSJlQWORs0YdpxbDFyoBXa00krKEXYceco1LzflOXlF7NyilUszV9CLp0ioiQkkRqqb9FUZH/0qQHmP68w0XJi3LYtMgfT6mKm8c92UJoi5JrjgajAcwt+Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724683444; c=relaxed/simple; bh=9oQYutyaMOqF0Dhr+m43f5SOj9YtHMZEAOH2lpITvso=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=cAjz2j16b0kE0ng/d7aiap5qoHTdUQgEJM5EXpEFuqUaq424YtOpZ4qrnluIC/rGa6NSa2S+Fey9lvMRhH4+KC2eUmDO0yqOJflgpi/rhAV8u4F3mqOroMeQbN1iYXgoFiZ/PVIJjJc2PAdFfsamg7VIiRtCKr9pS3Ftomdg8gY= 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=EFHApiqX; 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="EFHApiqX" Received: from ideasonboard.com (mob-5-90-142-90.net.vodafone.it [5.90.142.90]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D285741; Mon, 26 Aug 2024 16:42:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724683374; bh=9oQYutyaMOqF0Dhr+m43f5SOj9YtHMZEAOH2lpITvso=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EFHApiqXWYYCvzObALFtXqmxSKvB7Kb7KrUyU9RWvt78uO+MV3H342S5EKJwW+xOO ehuFh17wBg4psCq4ARpuo6yNsM8u1fNAGwqn3mfthx+X0s1bF0JsRqfdyS8XSowxqG 7XdStoJe0s7/1U1gD5gf3ttFMEfLdlHfuo8Fh7Sg= From: Jacopo Mondi To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: Jacopo Mondi , linux-media@vger.kernel.org Subject: [PATCH 1/4] media: pisp_be: Drop reference to non-existing function Date: Mon, 26 Aug 2024 16:43:34 +0200 Message-ID: <20240826144338.463683-2-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> References: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 A comment in the pisp_be driver references to the pispbe_schedule_internal() which doesn't exist. Drop it. Signed-off-by: Jacopo Mondi --- drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 65ff2382cffe..8ba1b9f43ba1 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -368,10 +368,7 @@ static void pispbe_xlate_addrs(struct pispbe_dev *pispbe, ret = pispbe_get_planes_addr(addrs, buf[MAIN_INPUT_NODE], &pispbe->node[MAIN_INPUT_NODE]); if (ret <= 0) { - /* - * This shouldn't happen; pispbe_schedule_internal should insist - * on an input. - */ + /* Shouldn't happen, we have validated an input is available. */ dev_warn(pispbe->dev, "ISP-BE missing input\n"); hw_en->bayer_enables = 0; hw_en->rgb_enables = 0; From patchwork Mon Aug 26 14:43:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13777986 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 4D15E18FC7C for ; Mon, 26 Aug 2024 14:44:05 +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=1724683446; cv=none; b=Ege0EGUW6p2JGAd3Pgek9JJqdAlfTR9v+mGjdsgf6WXQoHbE4aTdyfuRaszy6EH/J/ziME8VwELt5qXL6vGEcoP3Z0LWm4SMpPbffu6lqLQGz8g+Zt0rnf1/XG2ltvPyP8Wio97yuYOdo/vY/+Yn2DMrYsrPpfC6QhBmNRFrXZo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724683446; c=relaxed/simple; bh=Wdxv9SQ6HPlRDOYbzWq3gMXeob61suNvNjG4TSNiDsE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=T6Sr+VOLG98QDAEvNTqirr64cjtP5w81HlAPagv30PGBp5ISWBLAxN+daDZmjmmlvD01nXFsJeLSAn4hlwzL5KMpYu3/2oV9dOxflToky4DzwBhmFsKs5t2K+id4aXSWN2XkyzHk/PIJ8x4mxw117zcCZT1GkZlQWvaXWkp3VNg= 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=HMm0dnMN; 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="HMm0dnMN" Received: from ideasonboard.com (mob-5-90-142-90.net.vodafone.it [5.90.142.90]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 14C4C8D4; Mon, 26 Aug 2024 16:42:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724683375; bh=Wdxv9SQ6HPlRDOYbzWq3gMXeob61suNvNjG4TSNiDsE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HMm0dnMNtrLjj1L5fDUlgM8rio7icyuHAzsnGFtxvCqpGkYLO9GYqzAkMDWuDme3S 8cGECiCsdSrWePZAXXGOnS8lBbdqou8+4Suh08BOA5DjGUWwbbTx2zaE4+41TBY7UG KE3XR8rF3nNY3QgWcGz4EiXrpML1MKSlK4YRc68k= From: Jacopo Mondi To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: Jacopo Mondi , linux-media@vger.kernel.org Subject: [PATCH 2/4] media: pisp_be: Remove config validation from schedule() Date: Mon, 26 Aug 2024 16:43:35 +0200 Message-ID: <20240826144338.463683-3-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> References: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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 validation is already performed in pisp_be_validate_config() and there is no need to repeat that 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 --- .../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 8ba1b9f43ba1..73a5c88e25d0 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 -EIO; + } + /* 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 Aug 26 14:43:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13777987 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 D4C5518BBBF for ; Mon, 26 Aug 2024 14:44:05 +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=1724683447; cv=none; b=pjRDoDsuTAG2YGK9OAg7D4H5s8reblX3GwPKKSsTbaNKX5HfnmrSW89x3co1NUXW38gnRCO1VvE3JGan2ev31rEnz7wexAmTjuu+8w6idbwzQ6xgTWNL/KyjnCnnkFL2yCHMpalA0EIITXqgXOgpKrZPJeFKtMh+3GHsogfILSQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724683447; c=relaxed/simple; bh=rNfj1r2BmYuLAhonK75fj6llVhCusCLkj0fbb4C4avA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=u86xfEr+qMHGH9iWYQjpsj84IunmOvEeuOl1fW2J96Nb+1rEMTLcQibKT5s3dK5uZMmH3Is4//uZY1pYz57ceaann9FrQI/+2P/TSPplZ2U5YIM8MmT5wWQ+sKFCE+TmHQ5pLGk6HuPedjzrbgfjDf89nNxDzvkcVwVM4cziWHo= 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=sK0lMCSe; 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="sK0lMCSe" Received: from ideasonboard.com (mob-5-90-142-90.net.vodafone.it [5.90.142.90]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 088319FF; Mon, 26 Aug 2024 16:42:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724683376; bh=rNfj1r2BmYuLAhonK75fj6llVhCusCLkj0fbb4C4avA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sK0lMCSeXuuLSTnIUCt2bcrgCtN0wDLR6ZWZuwNG519pmUWjtS42yRR2zq8M9tlHf iT9FgAn7V4jXbJskh/VAEuPBOuSBKVI/ZEb+KXGZIUemCdL5HhU7ZeXyWWirluSG9/ 7mh5bqgKH5auRQBDpB4arQ2A5CUqP93BUPCA36zM= From: Jacopo Mondi To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: Jacopo Mondi , linux-media@vger.kernel.org Subject: [PATCH 3/4] media: pisp-be: Split jobs creation and scheduling Date: Mon, 26 Aug 2024 16:43:36 +0200 Message-ID: <20240826144338.463683-4-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> References: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Currently the 'pispbe_schedule()' function does two things: 1) Tries to assemble a job by inspecting all the video node queues to make sure all the required buffers are available 2) Submit the job to the hardware The pispbe_schedule() function is called at: - video device start_streaming() time - video device qbuf() time - irq handler As assembling a job requires inspecting all queues, it is a rather time consuming operation which is better not run in IRQ context. To avoid the executing the time consuming job creation in interrupt context split the job creation and job scheduling in two distinct operations. When a well-formed job is created, append it to the newly introduced 'pispbe->job_queue' where it will be dequeued from by the scheduling routine. At start_streaming() and qbuf() time immediately try to schedule a job if one has been created as the irq handler routing is only called when a job has completed, and we can't solely rely on it for scheduling new jobs. Signed-off-by: Jacopo Mondi --- .../platform/raspberrypi/pisp_be/pisp_be.c | 137 +++++++++++------- 1 file changed, 83 insertions(+), 54 deletions(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 73a5c88e25d0..f42541bb4827 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -190,6 +190,8 @@ struct pispbe_hw_enables { /* Records a job configuration and memory addresses. */ struct pispbe_job_descriptor { + struct list_head queue; + struct pispbe_buffer *buffers[PISPBE_NUM_NODES]; dma_addr_t hw_dma_addrs[N_HW_ADDRESSES]; struct pisp_be_tiles_config *config; struct pispbe_hw_enables hw_enables; @@ -215,8 +217,10 @@ struct pispbe_dev { unsigned int sequence; u32 streaming_map; struct pispbe_job queued_job, running_job; - spinlock_t hw_lock; /* protects "hw_busy" flag and streaming_map */ + /* protects "hw_busy" flag, streaming_map and job_queue*/ + spinlock_t hw_lock; bool hw_busy; /* non-zero if a job is queued or is being started */ + struct list_head job_queue; int irq; u32 hw_version; u8 done, started; @@ -440,41 +444,50 @@ static void pispbe_xlate_addrs(struct pispbe_dev *pispbe, * For Output0, Output1, Tdn and Stitch, a buffer only needs to be * available if the blocks are enabled in the config. * - * Needs to be called with hw_lock held. + * If all the buffers required to form a job are available, append the + * job descriptor to the job queue to be later queued to the HW. * * Returns 0 if a job has been successfully prepared, < 0 otherwise. */ -static int pispbe_prepare_job(struct pispbe_dev *pispbe, - struct pispbe_job_descriptor *job) +static int pispbe_prepare_job(struct pispbe_dev *pispbe) { struct pispbe_buffer *buf[PISPBE_NUM_NODES] = {}; + struct pispbe_job_descriptor *job; + unsigned int streaming_map; unsigned int config_index; struct pispbe_node *node; - unsigned long flags; - lockdep_assert_held(&pispbe->hw_lock); + scoped_guard(spinlock_irqsave, &pispbe->hw_lock) { + static const u32 mask = BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE); - memset(job, 0, sizeof(struct pispbe_job_descriptor)); + if ((pispbe->streaming_map & mask) != mask) + return -ENODEV; - if (((BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE)) & - pispbe->streaming_map) != - (BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE))) - return -ENODEV; + /* + * Take a copy of streaming_map: nodes activated after this + * point are ignored when preparing this job. + */ + streaming_map = pispbe->streaming_map; + } + + job = kzalloc(sizeof(*job), GFP_KERNEL); + if (!job) + return -ENOMEM; node = &pispbe->node[CONFIG_NODE]; - spin_lock_irqsave(&node->ready_lock, flags); - buf[CONFIG_NODE] = list_first_entry_or_null(&node->ready_queue, - struct pispbe_buffer, - ready_list); - if (buf[CONFIG_NODE]) { + + scoped_guard(spinlock_irqsave, &node->ready_lock) { + buf[CONFIG_NODE] = list_first_entry_or_null(&node->ready_queue, + struct pispbe_buffer, + ready_list); + if (!buf[CONFIG_NODE]) { + kfree(job); + return -ENODEV; + } + list_del(&buf[CONFIG_NODE]->ready_list); - pispbe->queued_job.buf[CONFIG_NODE] = buf[CONFIG_NODE]; + job->buffers[CONFIG_NODE] = buf[CONFIG_NODE]; } - spin_unlock_irqrestore(&node->ready_lock, flags); - - /* Exit early if no config buffer has been queued. */ - if (!buf[CONFIG_NODE]) - return -ENODEV; config_index = buf[CONFIG_NODE]->vb.vb2_buf.index; job->config = &pispbe->config[config_index]; @@ -495,7 +508,7 @@ static int pispbe_prepare_job(struct pispbe_dev *pispbe, continue; buf[i] = NULL; - if (!(pispbe->streaming_map & BIT(i))) + if (!(streaming_map & BIT(i))) continue; if ((!(rgb_en & PISP_BE_RGB_ENABLE_OUTPUT0) && @@ -522,25 +535,27 @@ static int pispbe_prepare_job(struct pispbe_dev *pispbe, node = &pispbe->node[i]; /* Pull a buffer from each V4L2 queue to form the queued job */ - spin_lock_irqsave(&node->ready_lock, flags); + spin_lock(&node->ready_lock); buf[i] = list_first_entry_or_null(&node->ready_queue, struct pispbe_buffer, ready_list); if (buf[i]) { list_del(&buf[i]->ready_list); - pispbe->queued_job.buf[i] = buf[i]; + job->buffers[i] = buf[i]; } - spin_unlock_irqrestore(&node->ready_lock, flags); + spin_unlock(&node->ready_lock); if (!buf[i] && !ignore_buffers) goto err_return_buffers; } - pispbe->queued_job.valid = true; - /* Convert buffers to DMA addresses for the hardware */ pispbe_xlate_addrs(pispbe, job, buf); + spin_lock(&pispbe->hw_lock); + list_add_tail(&job->queue, &pispbe->job_queue); + spin_unlock(&pispbe->hw_lock); + return 0; err_return_buffers: @@ -551,33 +566,41 @@ static int pispbe_prepare_job(struct pispbe_dev *pispbe, continue; /* Return the buffer to the ready_list queue */ - spin_lock_irqsave(&n->ready_lock, flags); + spin_lock(&n->ready_lock); list_add(&buf[i]->ready_list, &n->ready_queue); - spin_unlock_irqrestore(&n->ready_lock, flags); + spin_unlock(&n->ready_lock); } - memset(&pispbe->queued_job, 0, sizeof(pispbe->queued_job)); + kfree(job); return -ENODEV; } static void pispbe_schedule(struct pispbe_dev *pispbe, bool clear_hw_busy) { - struct pispbe_job_descriptor job; - unsigned long flags; - int ret; + struct pispbe_job_descriptor *job; - spin_lock_irqsave(&pispbe->hw_lock, flags); + scoped_guard(spinlock_irqsave, &pispbe->hw_lock) { + if (clear_hw_busy) + pispbe->hw_busy = false; - if (clear_hw_busy) - pispbe->hw_busy = false; + if (pispbe->hw_busy) + return; - if (pispbe->hw_busy) - goto unlock_and_return; + job = list_first_entry_or_null(&pispbe->job_queue, + struct pispbe_job_descriptor, + queue); + if (!job) + return; - ret = pispbe_prepare_job(pispbe, &job); - if (ret) - goto unlock_and_return; + list_del(&job->queue); + + for (unsigned int i = 0; i < PISPBE_NUM_NODES; i++) + pispbe->queued_job.buf[i] = job->buffers[i]; + pispbe->queued_job.valid = true; + + pispbe->hw_busy = true; + } /* * We can kick the job off without the hw_lock, as this can @@ -585,16 +608,8 @@ static void pispbe_schedule(struct pispbe_dev *pispbe, bool clear_hw_busy) * only when the following job has been queued and an interrupt * is rised. */ - pispbe->hw_busy = true; - spin_unlock_irqrestore(&pispbe->hw_lock, flags); - - pispbe_queue_job(pispbe, &job); - - return; - -unlock_and_return: - /* No job has been queued, just release the lock and return. */ - spin_unlock_irqrestore(&pispbe->hw_lock, flags); + pispbe_queue_job(pispbe, job); + kfree(job); } static void pispbe_isr_jobdone(struct pispbe_dev *pispbe, @@ -857,7 +872,8 @@ static void pispbe_node_buffer_queue(struct vb2_buffer *buf) * Every time we add a buffer, check if there's now some work for the hw * to do. */ - pispbe_schedule(pispbe, false); + if (!pispbe_prepare_job(pispbe)) + pispbe_schedule(pispbe, false); } static int pispbe_node_start_streaming(struct vb2_queue *q, unsigned int count) @@ -883,7 +899,8 @@ static int pispbe_node_start_streaming(struct vb2_queue *q, unsigned int count) node->pispbe->streaming_map); /* Maybe we're ready to run. */ - pispbe_schedule(pispbe, false); + if (!pispbe_prepare_job(pispbe)) + pispbe_schedule(pispbe, false); return 0; @@ -935,6 +952,16 @@ static void pispbe_node_stop_streaming(struct vb2_queue *q) spin_lock_irqsave(&pispbe->hw_lock, flags); pispbe->streaming_map &= ~BIT(node->id); + + /* Release all jobs once all nodes have stopped streaming. */ + if (pispbe->streaming_map == 0) { + struct pispbe_job_descriptor *job, *temp; + + list_for_each_entry_safe(job, temp, &pispbe->job_queue, queue) { + list_del(&job->queue); + kfree(job); + } + } spin_unlock_irqrestore(&pispbe->hw_lock, flags); pm_runtime_mark_last_busy(pispbe->dev); @@ -1677,6 +1704,8 @@ static int pispbe_probe(struct platform_device *pdev) if (!pispbe) return -ENOMEM; + INIT_LIST_HEAD(&pispbe->job_queue); + dev_set_drvdata(&pdev->dev, pispbe); pispbe->dev = &pdev->dev; platform_set_drvdata(pdev, pispbe); From patchwork Mon Aug 26 14:43:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13777988 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 5F0AD1925A4 for ; Mon, 26 Aug 2024 14:44:08 +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=1724683449; cv=none; b=sijyhR2AtKCRcL/1pDIpLJq0/Rsv0mCZ921WqpjuNRtrvzuDLN0NxHJuewEpuYCMI0qNMIGeZUraQMK+ns3H4vTLPC/eXx//EjsstKsPcxTO/yjTXNnlDjZnSlEsZWLZ6/u4KLhMKP9ksPYCcPW4VXpH2MwrHOW0sEuyicP54/M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724683449; c=relaxed/simple; bh=pZV1epGz/wTS1hfjeW2geB4ciytD9LN/DMC0xOZO8Mw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=p+KPQvN03JZXsy8cuXNNrRlFBgOWb6ej5GMD5PN42ehnCSoJ7AFOov02233QNPfW6l/Urq75Fz1ufLiPV39UFilyp3xRYiqTjnzn8MSsDtiRY4R5R12R62RU6uZEAO5787kLK0ruvJbiN2AadE5fR8IENQIKmuNUevHbJOcYfO0= 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=V6Vj206Y; 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="V6Vj206Y" Received: from ideasonboard.com (mob-5-90-142-90.net.vodafone.it [5.90.142.90]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DC6D9A8F; Mon, 26 Aug 2024 16:42:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724683377; bh=pZV1epGz/wTS1hfjeW2geB4ciytD9LN/DMC0xOZO8Mw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=V6Vj206YITen4eMqbRamPzs84dStSBV75Cl0SXgWC7duuOuNETf6v1pTJggmSigDr 813BxjQGHSaMYeH071Z2NdgZHx7rpjwG99yn9Yw/PCN06dlwbfRRLLGULEEjeecn1S sK7wz1TzkXvCXsiERmvElw2I6CUleJbny1oe9eTE= From: Jacopo Mondi To: Naushir Patuck , Nick Hollinghurst , David Plowman , Dave Stevenson , Laurent Pinchart Cc: Jacopo Mondi , linux-media@vger.kernel.org Subject: [PATCH 4/4] media: pisp_be: Fix pm_runtime underrun in probe Date: Mon, 26 Aug 2024 16:43:37 +0200 Message-ID: <20240826144338.463683-5-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> References: <20240826144338.463683-1-jacopo.mondi@ideasonboard.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The pisp_be driver uses and depends on runtime_pm. During the probe() routine, the driver needs to power up the interface in order to identify and initialize the hardware and it later suspends it at the end of probe(). The driver erroneously 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. Signed-off-by: Jacopo Mondi --- drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index f42541bb4827..fd4fec0f5d99 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -1741,7 +1741,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;