From: Manik Bajpai <manik.bajpai@intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <linux-media@vger.kernel.org>,
"Yan, Dongcheng" <dongcheng.yan@intel.com>,
Mehdi Djait <mehdi.djait@linux.intel.com>,
"Yu, Ong Hock" <ong.hock.yu@intel.com>,
"Ng, Khai Wen" <khai.wen.ng@intel.com>,
Antti Laakso <antti.laakso@linux.intel.com>,
"Bajpai, Manik" <manik.bajpai@intel.com>,
Divyamani Tripathi <divyamani.tripathi@intel.com>
Subject: Re: [PATCH v2 20/21] media: ipu6: Don't track power status, rely on runtime PM
Date: Tue, 22 Sep 2026 14:46:34 +0530 [thread overview]
Message-ID: <43468f29-3ff2-bbc5-42ee-05437d29ccd8@intel.com> (raw)
In-Reply-To: <20260917113923.59004-21-sakari.ailus@linux.intel.com>
Hi Sakari,
I am hacing issues in applying this patch. Please scroll down
for seeing the issue
On Thu, 17 Sep 2026, Sakari Ailus wrote:
> Rely on runtime PM suspending and resuming the device and so remove
> tracking of power state. Also remove the power_lock spinlock that
> serialised firmware initialisation and cleanup with the ISR. This is no
> longer necessary as the ISR now runs when the device is in RPM_ACTIVE
> state.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/pci/intel/ipu6/ipu6-fw-isys.c | 21 +++++++--------------
> drivers/media/pci/intel/ipu6/ipu6-isys.c | 16 ----------------
> drivers/media/pci/intel/ipu6/ipu6-isys.h | 5 -----
> drivers/media/pci/intel/ipu6/ipu7-fw-isys.c | 20 ++++++++++++--------
> 4 files changed, 19 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-fw-isys.c b/drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
> index 0214d7105fbe..f4f1cf7c86d2 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
> @@ -7,6 +7,7 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> +#include <linux/pm_runtime.h>
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> @@ -131,7 +132,6 @@ static int ipu6_fw_isys_close(struct ipu6_isys *isys)
> {
> struct device *dev = &isys->adev->auxdev.dev;
> int retry = IPU6_ISYS_CLOSE_RETRY;
> - unsigned long flags;
> void *fwctx;
> int ret;
>
> @@ -141,11 +141,9 @@ static int ipu6_fw_isys_close(struct ipu6_isys *isys)
> * to SP icache.
> * spinlock to wait the interrupt handler to be finished
> */
> - spin_lock_irqsave(&isys->power_lock, flags);
> ret = ipu6_fw_com_close(isys->fwctx);
> fwctx = isys->fwctx;
> isys->fwctx = NULL;
> - spin_unlock_irqrestore(&isys->power_lock, flags);
> if (ret)
> dev_err(dev, "Device close failure: %d\n", ret);
>
> @@ -158,9 +156,7 @@ static int ipu6_fw_isys_close(struct ipu6_isys *isys)
>
> if (ret) {
> dev_err(dev, "Device release time out %d\n", ret);
> - spin_lock_irqsave(&isys->power_lock, flags);
> isys->fwctx = fwctx;
> - spin_unlock_irqrestore(&isys->power_lock, flags);
> }
>
> return ret;
> @@ -531,9 +527,6 @@ int ipu6_isys_isr_one(struct ipu6_bus_device *adev)
> u32 index;
> u64 ts;
>
> - if (!isys->fwctx)
> - return 1;
> -
> resp = ipu6_fw_isys_get_resp(isys);
> if (!resp)
> return 1;
> @@ -699,12 +692,11 @@ irqreturn_t ipu6_isys_isr(struct ipu6_bus_device *adev)
> void __iomem *base = isys->pdata->base;
> u32 status_sw, status_csi;
> u32 ctrl0_status, ctrl0_clear;
> + int pm_status;
>
> - spin_lock(&isys->power_lock);
> - if (!isys->power) {
> - spin_unlock(&isys->power_lock);
> - return IRQ_NONE;
> - }
> + pm_status = pm_runtime_get_if_active(&adev->auxdev.dev);
> + if (!pm_status)
> + return 0;
>
> ctrl0_status = isys->pdata->ipdata->csi2.ctrl0_irq_status;
> ctrl0_clear = isys->pdata->ipdata->csi2.ctrl0_irq_clear;
> @@ -749,7 +741,8 @@ irqreturn_t ipu6_isys_isr(struct ipu6_bus_device *adev)
>
> writel(ISYS_UNISPART_IRQS, base + IPU6_REG_ISYS_UNISPART_IRQ_MASK);
>
> - spin_unlock(&isys->power_lock);
> + if (pm_status > 0)
> + pm_runtime_put(&adev->auxdev.dev);
>
> return IRQ_HANDLED;
> }
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys.c b/drivers/media/pci/intel/ipu6/ipu6-isys.c
> index fc6e87e9764b..60f5f9ea2910 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys.c
> @@ -759,7 +759,6 @@ static int isys_runtime_pm_resume(struct device *dev)
> struct ipu6_isys *isys = ipu6_bus_get_drvdata(adev);
> const struct ipu6_isys_internal_pdata *ipdata = isys->pdata->ipdata;
> struct ipu6_device *isp = adev->isp;
> - unsigned long flags;
> int ret;
>
> ret = ipu6_mmu_hw_init(adev->mmu);
> @@ -772,10 +771,6 @@ static int isys_runtime_pm_resume(struct device *dev)
> if (ret)
> goto err_mmu_hw_cleanup;
>
> - spin_lock_irqsave(&isys->power_lock, flags);
> - isys->power = 1;
> - spin_unlock_irqrestore(&isys->power_lock, flags);
> -
> if (IS_IPU7(isp)) {
> ipu7_isys_setup_hw(isys);
> } else {
> @@ -806,10 +801,6 @@ static int isys_runtime_pm_resume(struct device *dev)
> if (!ret)
> return 0;
>
> - spin_lock_irqsave(&isys->power_lock, flags);
> - isys->power = 0;
> - spin_unlock_irqrestore(&isys->power_lock, flags);
> -
> isys->phy_termcal_val = 0;
> cpu_latency_qos_update_request(&isys->pm_qos, PM_QOS_DEFAULT_VALUE);
>
> @@ -827,7 +818,6 @@ static int isys_runtime_pm_suspend(struct device *dev)
> struct ipu6_bus_device *adev = to_ipu6_bus_device(dev);
> struct ipu6_isys *isys = dev_get_drvdata(dev);
> struct ipu6_device *isp = adev->isp;
> - unsigned long flags;
> int ret = 0;
>
> isys->adev->auxdrv_data->fw_ops->close(isys);
> @@ -836,10 +826,6 @@ static int isys_runtime_pm_suspend(struct device *dev)
> ret = -EIO;
> }
>
> - spin_lock_irqsave(&isys->power_lock, flags);
> - isys->power = 0;
> - spin_unlock_irqrestore(&isys->power_lock, flags);
> -
> isys->phy_termcal_val = 0;
> cpu_latency_qos_update_request(&isys->pm_qos, PM_QOS_DEFAULT_VALUE);
>
> @@ -1016,8 +1002,6 @@ static int isys_probe(struct auxiliary_device *auxdev,
> isys->sensor_type = isys->pdata->ipdata->sensor_type_start;
>
> spin_lock_init(&isys->streams_lock);
> - spin_lock_init(&isys->power_lock);
> - isys->power = 0;
> isys->phy_termcal_val = 0;
>
> mutex_init(&isys->mutex);
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys.h b/drivers/media/pci/intel/ipu6/ipu6-isys.h
> index 4991078c18b2..2af20f56a965 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys.h
> @@ -80,9 +80,6 @@ struct sensor_async_sd {
> * @media_dev: Media device
> * @v4l2_dev: V4L2 device
> * @adev: ISYS bus device
> - * @power: Is ISYS powered on or not?
> - * @isr_bits: Which bits does the ISR handle?
> - * @power_lock: Serialise access to power (power state in general)
> * @csi2_rx_ctrl_cached: cached shared value between all CSI2 receivers
> * @streams_lock: serialise access to streams
> * @streams: streams per firmware stream ID
> @@ -99,8 +96,6 @@ struct ipu6_isys {
> struct v4l2_device v4l2_dev;
> struct ipu6_bus_device *adev;
>
> - int power;
> - spinlock_t power_lock;
> u32 isr_csi2_bits;
> u32 csi2_rx_ctrl_cached;
> spinlock_t streams_lock;
> diff --git a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
> index 50219b210ae9..0876cc54faa7 100644
> --- a/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
> +++ b/drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
> @@ -5,6 +5,7 @@
>
> #include <linux/cleanup.h>
> #include <linux/cacheflush.h>
> +#include <linux/pm_runtime.h>
>
> #include "ipu6-bus.h"
> #include "ipu6-dma.h"
> @@ -552,9 +553,6 @@ static int ipu7_isys_isr_one(struct ipu6_bus_device *adev)
> unsigned long flags;
> u64 ts;
>
> - if (!isys->fwctx)
> - return 1;
> -
> resp = ipu7_fw_isys_get_resp(isys);
> if (!resp)
> return 1;
> @@ -752,11 +750,11 @@ irqreturn_t ipu7_isys_isr(struct ipu6_bus_device *adev)
> void __iomem *base = isys->pdata->base;
> u32 status_sw, status_csi;
> u32 csi_offset, sw_offset;
> + int pm_status;
>
> - guard(spinlock)(&isys->power_lock);
> -
> - if (!isys->power)
> - return IRQ_NONE;
> + pm_status = pm_runtime_get_if_active(&adev->auxdev.dev);
> + if (!pm_status)
> + return 0;
>
> csi_offset = IPU7_IS_IO_CSI2_LEGACY_IRQ_CTRL_BASE;
> sw_offset = IPU7_IS_UC_CTRL_BASE;
> @@ -764,8 +762,11 @@ irqreturn_t ipu7_isys_isr(struct ipu6_bus_device *adev)
> status_csi = readl(base + csi_offset + IPU7_IRQ_CTL_STATUS);
> status_sw = readl(base + sw_offset + IPU7_TO_SW_IRQ_CNTL_STATUS);
>
> - if (!status_csi && !status_sw)
> + if (!status_csi && !status_sw) {
> + if (pm_status > 0)
> + pm_runtime_put(&adev->auxdev.dev);
> return IRQ_NONE;
> + }
>
> do {
> writel(status_sw, base + sw_offset + IPU7_TO_SW_IRQ_CNTL_CLEAR);
> @@ -788,5 +789,8 @@ irqreturn_t ipu7_isys_isr(struct ipu6_bus_device *adev)
> writel(IPU7_IS_UC_TO_SW_IRQ_MASK,
> base + sw_offset + IPU7_TO_SW_IRQ_CNTL_MASK_N);
>
> + if (pm_status > 0)
> + pm_runtime_put(&adev->auxdev.dev);
> +
> return IRQ_HANDLED;
> }
> --
> 2.47.3
>
>
Result of "patch -p1 --dry-run" in pine
checking file drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
checking file drivers/media/pci/intel/ipu6/ipu6-isys.c
checking file drivers/media/pci/intel/ipu6/ipu6-isys.h
checking file drivers/media/pci/intel/ipu6/ipu7-fw-isys.c
Hunk #1 FAILED at 5.
Hunk #2 succeeded at 551 (offset -1 lines).
Hunk #3 succeeded at 748 (offset -1 lines).
Hunk #4 succeeded at 760 (offset -1 lines).
Hunk #5 succeeded at 787 (offset -1 lines).
1 out of 5 hunks FAILED
My current tree looks like this:-
intel@pepper:~/s-ailus/linux-ipu6$ git log --pretty --oneline -25
a67d0bc689db (HEAD -> ipu6) media: ipu6: Move firmware init/cleanup to RPM callbacks
7a11adbbf11c media: ipu6: Serialise access to stream pointers by isys stream_lock
25b80e93ebca media: ipu6: Drop {get,put}_streams_opened()
ae57b074bbf9 media: ipu6: Bridge the gap between streams in V4L2 and IPU6 firmware
6640c40b9185 media: ipu6: Rework watermark setting
27cc9d1434f7 media: ipu6: Rework watermark calculation
bba5324133c7 media: ipu6: Avoid accessing av->streams before streaming
8fe5d941938e media: ipu6: Collect enabled stream IDs
f0144bcba9b1 media: ipu6: Remove nr_queues and nr_streaming fields in ipu6_isys_stream
4e23a8cadad1 media: ipu6: Add lockdep checks for CSI-2 streaming enable and disable
d5d977d60cae media: ipu6: Start streaming once all streams have started, stop when not
6987e506fe88 media: ipu6: Collect IPU streams into CSI-2 receiver sub-device context
ac8bcb8026c7 media: ipu6: Get watermark configuration directly from ipdata
4132a4c2835f media: ipu6: Clean up link frequency calculation
64c42c31525e media: ipu6: Always request a capture ack
3395c33b5160 media: ipu6: Rename misnamed out_free_watermark label in video init
6dbd34bdaf4e media: ipu6: Stream number on CSI-2 receiver source pads is always 0
5ec55ec5646e media: ipu6: Move streaming control to CSI-2 receiver driver
3e3070330170 media: ipu6: Fix releasing resources at failing streamon
8fa3385b6db1 (origin/ipu6) media: ipu6: Enable ipu7 and ipu7.5
09db11b21d85 media: ipu6: Add ipu7.5 mmu initialization data
553f8433c5d9 media: ipu6: Add ipu7.5 buttress support
8d8cd6c357e9 media: ipu6: Set model name for ipu7
0951f102d6c8 media: ipu6: Map ipu7 firmware
58c4286583cb media: ipu6: Move isys fw mapping to pci_probe
next prev parent reply other threads:[~2026-09-22 9:16 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 11:39 [PATCH v2 00/21] IPU6 multi-stream and metadata support preparation Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 01/21] media: ipu6: Fix releasing resources at failing streamon Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 02/21] media: ipu6: Move streaming control to CSI-2 receiver driver Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 03/21] media: ipu6: Stream number on CSI-2 receiver source pads is always 0 Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 04/21] media: ipu6: Rename misnamed out_free_watermark label in video init Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 05/21] media: ipu6: Always request a capture ack Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 06/21] media: ipu6: Clean up link frequency calculation Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 07/21] media: ipu6: Get watermark configuration directly from ipdata Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 08/21] media: ipu6: Collect IPU streams into CSI-2 receiver sub-device context Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 09/21] media: ipu6: Start streaming once all streams have started, stop when not Sakari Ailus
2026-09-18 7:36 ` Nicola Fiorillo
2026-09-18 20:56 ` Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 10/21] media: ipu6: Add lockdep checks for CSI-2 streaming enable and disable Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 11/21] media: ipu6: Remove nr_queues and nr_streaming fields in ipu6_isys_stream Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 12/21] media: ipu6: Collect enabled stream IDs Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 13/21] media: ipu6: Avoid accessing av->streams before streaming Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 14/21] media: ipu6: Rework watermark calculation Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 15/21] media: ipu6: Rework watermark setting Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 16/21] media: ipu6: Bridge the gap between streams in V4L2 and IPU6 firmware Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 17/21] media: ipu6: Drop {get,put}_streams_opened() Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 18/21] media: ipu6: Serialise access to stream pointers by isys stream_lock Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 19/21] media: ipu6: Move firmware init/cleanup to RPM callbacks Sakari Ailus
2026-09-17 11:39 ` [PATCH v2 20/21] media: ipu6: Don't track power status, rely on runtime PM Sakari Ailus
2026-09-22 9:16 ` Manik Bajpai [this message]
2026-09-17 11:39 ` [PATCH v2 21/21] media: ipu6: Support upstream sub-devices without get_frame_desc() Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43468f29-3ff2-bbc5-42ee-05437d29ccd8@intel.com \
--to=manik.bajpai@intel.com \
--cc=antti.laakso@linux.intel.com \
--cc=divyamani.tripathi@intel.com \
--cc=dongcheng.yan@intel.com \
--cc=khai.wen.ng@intel.com \
--cc=linux-media@vger.kernel.org \
--cc=mehdi.djait@linux.intel.com \
--cc=ong.hock.yu@intel.com \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox