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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.