[PATCH 0/3] media: ipu3-cio2: support concurrent CSI-2 streams

[PATCH 0/3] media: ipu3-cio2: support concurrent CSI-2 streams

[Joe Dzado]

The ipu3-cio2 driver currently assumes a single active capture: every
stream is pinned to DMA channel 0 (CIO2_DMA_CHAN) and tracked through a
single cur_queue pointer, with the IRQ handler assuming all completion
and start-of-frame events belong to that one queue. On platforms with
more than one sensor wired to separate CSI-2 ports (e.g. the Microsoft
Surface RGB + IR cameras), starting a second stream reprograms channel 0
and cur_queue out from under the first, so only one sensor can capture
at a time.

This series lets the driver use the DMA channels the hardware actually
has, so multiple sensors can stream concurrently:

  1/3 dispatches interrupts by DMA channel instead of a single cur_queue
      and converts the streaming state to a per-queue count (no
      functional change);
  2/3 serialises concurrent hw_init()/hw_exit() with a new lock and
      guards the block-shared setup so bringing up a second stream does
      not tear down the first (still single-channel, no functional
      change);
  3/3 maps each CSI-2 port to its own DMA channel (2*port), with a
      dma_per_port module parameter to restore the old behaviour.

Each patch builds on its own and is bisectable. Tested on a Microsoft
Surface Laptop (i5-7200U, Kaby Lake) with the OV9734 (colour) and OV7251
(IR) sensors streaming concurrently on CIO2 ports 1 and 2 for several
days without errors.

Joe Dzado (3):
  media: ipu3-cio2: track the streaming queue by DMA channel
  media: ipu3-cio2: serialise concurrent stream start and stop
  media: ipu3-cio2: give each CSI-2 port its own DMA channel

 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 283 ++++++++++++++---------
 drivers/media/pci/intel/ipu3/ipu3-cio2.h |  10 +-
 2 files changed, 188 insertions(+), 105 deletions(-)

-- 
2.56.0

[PATCH 1/3] media: ipu3-cio2: track the streaming queue by DMA channel

[Joe Dzado]

Prepare for more than one CSI-2 port streaming at once. Replace the
single cur_queue pointer with a chan_queue[] map indexed by DMA channel,
and give each queue a dma_chan it owns (cio2_queue_dma_chan() still
returns CIO2_DMA_CHAN for every port, so only one channel is used for
now). The IRQ handler, buffer-done and SOF paths dispatch via
chan_queue[] instead of assuming cur_queue, and all register accesses
use the queue's channel.

cio2->streaming becomes a count of streaming queues plus a per-queue
flag, and suspend/resume iterate the queues instead of the lone
cur_queue. No functional change: a single stream still uses channel 0.
Signed-off-by: Joe Dzado <joe.dzado@icloud.com>

---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 125 +++++++++++++++--------
 drivers/media/pci/intel/ipu3/ipu3-cio2.h |   8 +-
 2 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 4d2b23129..06ce58e23 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -350,6 +350,12 @@ static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
 	return 0;
 };
 
+/* Each queue (CSI-2 port) owns one DMA channel. */
+static unsigned int cio2_queue_dma_chan(unsigned int port)
+{
+	return CIO2_DMA_CHAN;
+}
+
 static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 {
 	static const int NUM_VCS = 4;
@@ -364,6 +370,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	void __iomem *const base = cio2->base;
 	u8 lanes, csi2bus = q->csi2.port;
 	u8 sensor_vc = SENSOR_VIR_CH_DFLT;
+	const unsigned int chan = q->dma_chan;
 	struct cio2_csi2_timing timing = { 0 };
 	int i, r;
 
@@ -438,8 +445,8 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_LEVEL_NOT_PULSE);
 	writel(CIO2_INT_EN_EXT_OE_MASK, base + CIO2_REG_INT_EN_EXT_OE);
 
-	writel(CIO2_REG_INT_EN_IRQ | CIO2_INT_IOC(CIO2_DMA_CHAN) |
-	       CIO2_REG_INT_EN_IOS(CIO2_DMA_CHAN),
+	writel(CIO2_REG_INT_EN_IRQ | CIO2_INT_IOC(chan) |
+	       CIO2_REG_INT_EN_IOS(chan),
 	       base + CIO2_REG_INT_EN);
 
 	writel((CIO2_PXM_PXF_FMT_CFG_BPP_10 | CIO2_PXM_PXF_FMT_CFG_PCK_64B)
@@ -451,7 +458,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	       q->csi_rx_base + CIO2_REG_MIPIBE_LP_LUT_ENTRY(ENTRY));
 	writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_COMP_FORMAT(sensor_vc));
 	writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_FORCE_RAW8);
-	writel(0, base + CIO2_REG_PXM_SID2BID0(csi2bus));
+	writel(chan, base + CIO2_REG_PXM_SID2BID0(csi2bus));
 
 	writel(lanes, q->csi_rx_base + CIO2_REG_CSIRX_NOF_ENABLED_LANES);
 	writel(CIO2_CGC_PRIM_TGE |
@@ -486,7 +493,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	}
 
 	/* Enable DMA */
-	writel(PFN_DOWN(q->fbpt_bus_addr), base + CIO2_REG_CDMABA(CIO2_DMA_CHAN));
+	writel(PFN_DOWN(q->fbpt_bus_addr), base + CIO2_REG_CDMABA(chan));
 
 	writel(num_buffers1 << CIO2_CDMAC0_FBPT_LEN_SHIFT |
 	       FBPT_WIDTH << CIO2_CDMAC0_FBPT_WIDTH_SHIFT |
@@ -494,10 +501,10 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	       CIO2_CDMAC0_FBPT_UPDATE_FIFO_FULL |
 	       CIO2_CDMAC0_DMA_EN |
 	       CIO2_CDMAC0_DMA_INTR_ON_FS |
-	       CIO2_CDMAC0_DMA_HALTED, base + CIO2_REG_CDMAC0(CIO2_DMA_CHAN));
+	       CIO2_CDMAC0_DMA_HALTED, base + CIO2_REG_CDMAC0(chan));
 
 	writel(1 << CIO2_CDMAC1_LINENUMUPDATE_SHIFT,
-	       base + CIO2_REG_CDMAC1(CIO2_DMA_CHAN));
+	       base + CIO2_REG_CDMAC1(chan));
 
 	writel(0, base + CIO2_REG_PBM_FOPN_ABORT);
 
@@ -525,6 +532,7 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q)
 	struct device *dev = &cio2->pci_dev->dev;
 	void __iomem *const base = cio2->base;
 	unsigned int i;
+	const unsigned int chan = q->dma_chan;
 	u32 value;
 	int ret;
 
@@ -535,12 +543,12 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q)
 	writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_ENABLE);
 
 	/* Halt DMA */
-	writel(0, base + CIO2_REG_CDMAC0(CIO2_DMA_CHAN));
-	ret = readl_poll_timeout(base + CIO2_REG_CDMAC0(CIO2_DMA_CHAN),
+	writel(0, base + CIO2_REG_CDMAC0(chan));
+	ret = readl_poll_timeout(base + CIO2_REG_CDMAC0(chan),
 				 value, value & CIO2_CDMAC0_DMA_HALTED,
 				 4000, 2000000);
 	if (ret)
-		dev_err(dev, "DMA %i can not be halted\n", CIO2_DMA_CHAN);
+		dev_err(dev, "DMA %u can not be halted\n", chan);
 
 	for (i = 0; i < CIO2_NUM_PORTS; i++) {
 		writel(readl(base + CIO2_REG_PXM_FRF_CFG(i)) |
@@ -553,11 +561,11 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q)
 static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
 {
 	struct device *dev = &cio2->pci_dev->dev;
-	struct cio2_queue *q = cio2->cur_queue;
+	struct cio2_queue *q = cio2->chan_queue[dma_chan];
 	struct cio2_fbpt_entry *entry;
 	u64 ns = ktime_get_ns();
 
-	if (dma_chan >= CIO2_QUEUES) {
+	if (!q) {
 		dev_err(dev, "bad DMA channel %i\n", dma_chan);
 		return;
 	}
@@ -711,7 +719,8 @@ static void cio2_irq_handle_once(struct cio2_device *cio2, u32 int_status)
 		for (d = 0; d < CIO2_NUM_DMA_CHAN; d++)
 			if (int_status & CIO2_INT_IOC(d)) {
 				clr |= CIO2_INT_IOC(d);
-				cio2_buffer_done(cio2, d);
+				if (cio2->chan_queue[d])
+					cio2_buffer_done(cio2, d);
 			}
 		int_status &= ~clr;
 	}
@@ -724,9 +733,9 @@ static void cio2_irq_handle_once(struct cio2_device *cio2, u32 int_status)
 		for (d = 0; d < CIO2_NUM_DMA_CHAN; d++)
 			if (int_status & CIO2_INT_IOS_IOLN(d)) {
 				clr |= CIO2_INT_IOS_IOLN(d);
-				if (d == CIO2_DMA_CHAN)
+				if (cio2->chan_queue[d])
 					cio2_queue_event_sof(cio2,
-							     cio2->cur_queue);
+							     cio2->chan_queue[d]);
 			}
 		int_status &= ~clr;
 	}
@@ -929,7 +938,7 @@ static void cio2_vb2_buf_queue(struct vb2_buffer *vb)
 	 */
 	local_irq_save(flags);
 
-	fbpt_rp = (readl(cio2->base + CIO2_REG_CDMARI(CIO2_DMA_CHAN))
+	fbpt_rp = (readl(cio2->base + CIO2_REG_CDMARI(q->dma_chan))
 		   >> CIO2_CDMARI_FBPT_RP_SHIFT)
 		   & CIO2_CDMARI_FBPT_RP_MASK;
 
@@ -996,7 +1005,6 @@ static int cio2_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct device *dev = &cio2->pci_dev->dev;
 	int r;
 
-	cio2->cur_queue = q;
 	atomic_set(&q->frame_sequence, 0);
 
 	r = pm_runtime_resume_and_get(dev);
@@ -1009,21 +1017,28 @@ static int cio2_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
 	if (r)
 		goto fail_pipeline;
 
+	cio2->chan_queue[q->dma_chan] = q;
 	r = cio2_hw_init(cio2, q);
-	if (r)
+	if (r) {
+		cio2->chan_queue[q->dma_chan] = NULL;
 		goto fail_hw;
+	}
+	cio2->streaming++;
+	q->streaming = true;
 
 	/* Start streaming on sensor */
 	r = v4l2_subdev_call(q->sensor, video, s_stream, 1);
 	if (r)
 		goto fail_csi2_subdev;
 
-	cio2->streaming = true;
-
 	return 0;
 
 fail_csi2_subdev:
 	cio2_hw_exit(cio2, q);
+	synchronize_irq(cio2->pci_dev->irq);
+	cio2->chan_queue[q->dma_chan] = NULL;
+	cio2->streaming--;
+	q->streaming = false;
 fail_hw:
 	video_device_pipeline_stop(&q->vdev);
 fail_pipeline:
@@ -1045,10 +1060,12 @@ static void cio2_vb2_stop_streaming(struct vb2_queue *vq)
 
 	cio2_hw_exit(cio2, q);
 	synchronize_irq(cio2->pci_dev->irq);
+	cio2->chan_queue[q->dma_chan] = NULL;
+	cio2->streaming--;
+	q->streaming = false;
 	cio2_vb2_return_all_buffers(q, VB2_BUF_STATE_ERROR);
 	video_device_pipeline_stop(&q->vdev);
 	pm_runtime_put(dev);
-	cio2->streaming = false;
 }
 
 static const struct vb2_ops cio2_vb2_ops = {
@@ -1498,6 +1515,7 @@ static int cio2_queue_init(struct cio2_device *cio2, struct cio2_queue *q)
 
 	/* Initialize miscellaneous variables */
 	mutex_init(&q->lock);
+	q->dma_chan = cio2_queue_dma_chan(q - cio2->queue);	/* queue[] is indexed by port */
 
 	q->format.width = default_width;
 	q->format.height = default_height;
@@ -1895,21 +1913,26 @@ static int __maybe_unused cio2_suspend(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
-	struct cio2_queue *q = cio2->cur_queue;
+	struct cio2_queue *q;
+	unsigned int i;
 	int r;
 
 	dev_dbg(dev, "cio2 suspend\n");
 	if (!cio2->streaming)
 		return 0;
 
-	/* Stop stream */
-	r = v4l2_subdev_call(q->sensor, video, s_stream, 0);
-	if (r) {
-		dev_err(dev, "failed to stop sensor streaming\n");
-		return r;
+	/* Stop streams */
+	for (i = 0; i < CIO2_QUEUES; i++) {
+		q = &cio2->queue[i];
+		if (!q->streaming)
+			continue;
+		r = v4l2_subdev_call(q->sensor, video, s_stream, 0);
+		if (r) {
+			dev_err(dev, "failed to stop sensor streaming\n");
+			return r;
+		}
+		cio2_hw_exit(cio2, q);
 	}
-
-	cio2_hw_exit(cio2, q);
 	synchronize_irq(pci_dev->irq);
 
 	pm_runtime_force_suspend(dev);
@@ -1918,9 +1941,14 @@ static int __maybe_unused cio2_suspend(struct device *dev)
 	 * Upon resume, hw starts to process the fbpt entries from beginning,
 	 * so relocate the queued buffs to the fbpt head before suspend.
 	 */
-	cio2_fbpt_rearrange(cio2, q);
-	q->bufs_first = 0;
-	q->bufs_next = 0;
+	for (i = 0; i < CIO2_QUEUES; i++) {
+		q = &cio2->queue[i];
+		if (!q->streaming)
+			continue;
+		cio2_fbpt_rearrange(cio2, q);
+		q->bufs_first = 0;
+		q->bufs_next = 0;
+	}
 
 	return 0;
 }
@@ -1928,30 +1956,41 @@ static int __maybe_unused cio2_suspend(struct device *dev)
 static int __maybe_unused cio2_resume(struct device *dev)
 {
 	struct cio2_device *cio2 = dev_get_drvdata(dev);
-	struct cio2_queue *q = cio2->cur_queue;
+	unsigned int i, streaming = cio2->streaming;
+	struct cio2_queue *q;
 	int r;
 
 	dev_dbg(dev, "cio2 resume\n");
-	if (!cio2->streaming)
+	if (!streaming)
 		return 0;
-	/* Start stream */
+	/* Start streams */
 	r = pm_runtime_force_resume(dev);
 	if (r < 0) {
 		dev_err(dev, "failed to set power %d\n", r);
 		return r;
 	}
 
-	r = cio2_hw_init(cio2, q);
-	if (r) {
-		dev_err(dev, "fail to init cio2 hw\n");
-		return r;
-	}
+	/* hw_init does the shared setup only for the first queue it brings up */
+	cio2->streaming = 0;
+	for (i = 0; i < CIO2_QUEUES; i++) {
+		q = &cio2->queue[i];
+		if (!q->streaming)
+			continue;
+		r = cio2_hw_init(cio2, q);
+		cio2->streaming++;
+		if (r) {
+			dev_err(dev, "fail to init cio2 hw\n");
+			break;
+		}
 
-	r = v4l2_subdev_call(q->sensor, video, s_stream, 1);
-	if (r) {
-		dev_err(dev, "fail to start sensor streaming\n");
-		cio2_hw_exit(cio2, q);
+		r = v4l2_subdev_call(q->sensor, video, s_stream, 1);
+		if (r) {
+			dev_err(dev, "fail to start sensor streaming\n");
+			cio2_hw_exit(cio2, q);
+			break;
+		}
 	}
+	cio2->streaming = streaming;
 
 	return r;
 }
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index 192581909..a6e363a4b 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -367,6 +367,9 @@ struct cio2_queue {
 	unsigned int bufs_first;	/* Index of the first used entry */
 	unsigned int bufs_next;	/* Index of the first unused entry */
 	atomic_t bufs_queued;
+
+	unsigned int dma_chan;	/* DMA channel owned by this queue */
+	bool streaming;
 };
 
 struct cio2_device {
@@ -374,11 +377,12 @@ struct cio2_device {
 	void __iomem *base;
 	struct v4l2_device v4l2_dev;
 	struct cio2_queue queue[CIO2_QUEUES];
-	struct cio2_queue *cur_queue;
+	/* DMA channel -> streaming queue, for the IRQ handler */
+	struct cio2_queue *chan_queue[CIO2_NUM_DMA_CHAN];
 	/* mutex to be used by video_device */
 	struct mutex lock;
 
-	bool streaming;
+	unsigned int streaming;	/* number of streaming queues */
 	struct v4l2_async_notifier notifier;
 	struct media_device media_dev;
 
-- 
2.56.0

[PATCH 2/3] media: ipu3-cio2: serialise concurrent stream start and stop

[Joe Dzado]

Make cio2_hw_init()/cio2_hw_exit() safe to call while another queue is
already streaming. A new hw_lock serialises channel setup/teardown and
the streaming bookkeeping. cio2_hw_init() writes the block-shared setup
(PBM watermarks/arbiter, HPLL/ISCLK, CGC, LTR, full DMA-channel reset,
global interrupt-status clear) only for the first queue brought up, and
preserves the other channels' INT_EN/INT_STS bits otherwise.
cio2_hw_exit() aborts only its own port and clears only its own channel
interrupts rather than tearing down every port.

Still single-channel in practice (every queue maps to channel 0), so no
functional change; this only adds the locking and shared-state guards.
Signed-off-by: Joe Dzado <joe.dzado@icloud.com>

---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 154 ++++++++++++++---------
 drivers/media/pci/intel/ipu3/ipu3-cio2.h |   2 +
 2 files changed, 94 insertions(+), 62 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 06ce58e23..071c5ea4a 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -371,6 +371,8 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	u8 lanes, csi2bus = q->csi2.port;
 	u8 sensor_vc = SENSOR_VIR_CH_DFLT;
 	const unsigned int chan = q->dma_chan;
+	/* another queue already streaming: leave shared blocks alone */
+	const bool first = !cio2->streaming;
 	struct cio2_csi2_timing timing = { 0 };
 	int i, r;
 
@@ -402,30 +404,34 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 			CIO2_REG_CSIRX_DLY_CNT_SETTLE(i));
 	}
 
-	writel(CIO2_PBM_WMCTRL1_MIN_2CK |
-	       CIO2_PBM_WMCTRL1_MID1_2CK |
-	       CIO2_PBM_WMCTRL1_MID2_2CK, base + CIO2_REG_PBM_WMCTRL1);
-	writel(CIO2_PBM_WMCTRL2_HWM_2CK << CIO2_PBM_WMCTRL2_HWM_2CK_SHIFT |
-	       CIO2_PBM_WMCTRL2_LWM_2CK << CIO2_PBM_WMCTRL2_LWM_2CK_SHIFT |
-	       CIO2_PBM_WMCTRL2_OBFFWM_2CK <<
-	       CIO2_PBM_WMCTRL2_OBFFWM_2CK_SHIFT |
-	       CIO2_PBM_WMCTRL2_TRANSDYN << CIO2_PBM_WMCTRL2_TRANSDYN_SHIFT |
-	       CIO2_PBM_WMCTRL2_OBFF_MEM_EN, base + CIO2_REG_PBM_WMCTRL2);
-	writel(CIO2_PBM_ARB_CTRL_LANES_DIV <<
-	       CIO2_PBM_ARB_CTRL_LANES_DIV_SHIFT |
-	       CIO2_PBM_ARB_CTRL_LE_EN |
-	       CIO2_PBM_ARB_CTRL_PLL_POST_SHTDN <<
-	       CIO2_PBM_ARB_CTRL_PLL_POST_SHTDN_SHIFT |
-	       CIO2_PBM_ARB_CTRL_PLL_AHD_WK_UP <<
-	       CIO2_PBM_ARB_CTRL_PLL_AHD_WK_UP_SHIFT,
-	       base + CIO2_REG_PBM_ARB_CTRL);
+	if (first) {
+		writel(CIO2_PBM_WMCTRL1_MIN_2CK |
+		       CIO2_PBM_WMCTRL1_MID1_2CK |
+		       CIO2_PBM_WMCTRL1_MID2_2CK, base + CIO2_REG_PBM_WMCTRL1);
+		writel(CIO2_PBM_WMCTRL2_HWM_2CK << CIO2_PBM_WMCTRL2_HWM_2CK_SHIFT |
+		       CIO2_PBM_WMCTRL2_LWM_2CK << CIO2_PBM_WMCTRL2_LWM_2CK_SHIFT |
+		       CIO2_PBM_WMCTRL2_OBFFWM_2CK <<
+		       CIO2_PBM_WMCTRL2_OBFFWM_2CK_SHIFT |
+		       CIO2_PBM_WMCTRL2_TRANSDYN << CIO2_PBM_WMCTRL2_TRANSDYN_SHIFT |
+		       CIO2_PBM_WMCTRL2_OBFF_MEM_EN, base + CIO2_REG_PBM_WMCTRL2);
+		writel(CIO2_PBM_ARB_CTRL_LANES_DIV <<
+		       CIO2_PBM_ARB_CTRL_LANES_DIV_SHIFT |
+		       CIO2_PBM_ARB_CTRL_LE_EN |
+		       CIO2_PBM_ARB_CTRL_PLL_POST_SHTDN <<
+		       CIO2_PBM_ARB_CTRL_PLL_POST_SHTDN_SHIFT |
+		       CIO2_PBM_ARB_CTRL_PLL_AHD_WK_UP <<
+		       CIO2_PBM_ARB_CTRL_PLL_AHD_WK_UP_SHIFT,
+		       base + CIO2_REG_PBM_ARB_CTRL);
+	}
 	writel(CIO2_CSIRX_STATUS_DLANE_HS_MASK,
 	       q->csi_rx_base + CIO2_REG_CSIRX_STATUS_DLANE_HS);
 	writel(CIO2_CSIRX_STATUS_DLANE_LP_MASK,
 	       q->csi_rx_base + CIO2_REG_CSIRX_STATUS_DLANE_LP);
 
-	writel(CIO2_FB_HPLL_FREQ, base + CIO2_REG_FB_HPLL_FREQ);
-	writel(CIO2_ISCLK_RATIO, base + CIO2_REG_ISCLK_RATIO);
+	if (first) {
+		writel(CIO2_FB_HPLL_FREQ, base + CIO2_REG_FB_HPLL_FREQ);
+		writel(CIO2_ISCLK_RATIO, base + CIO2_REG_ISCLK_RATIO);
+	}
 
 	/* Configure MIPI backend */
 	for (i = 0; i < NUM_VCS; i++)
@@ -445,8 +451,8 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_LEVEL_NOT_PULSE);
 	writel(CIO2_INT_EN_EXT_OE_MASK, base + CIO2_REG_INT_EN_EXT_OE);
 
-	writel(CIO2_REG_INT_EN_IRQ | CIO2_INT_IOC(chan) |
-	       CIO2_REG_INT_EN_IOS(chan),
+	writel((first ? 0 : readl(base + CIO2_REG_INT_EN)) |
+	       CIO2_REG_INT_EN_IRQ | CIO2_INT_IOC(chan) | CIO2_REG_INT_EN_IOS(chan),
 	       base + CIO2_REG_INT_EN);
 
 	writel((CIO2_PXM_PXF_FMT_CFG_BPP_10 | CIO2_PXM_PXF_FMT_CFG_PCK_64B)
@@ -461,35 +467,41 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	writel(chan, base + CIO2_REG_PXM_SID2BID0(csi2bus));
 
 	writel(lanes, q->csi_rx_base + CIO2_REG_CSIRX_NOF_ENABLED_LANES);
-	writel(CIO2_CGC_PRIM_TGE |
-	       CIO2_CGC_SIDE_TGE |
-	       CIO2_CGC_XOSC_TGE |
-	       CIO2_CGC_D3I3_TGE |
-	       CIO2_CGC_CSI2_INTERFRAME_TGE |
-	       CIO2_CGC_CSI2_PORT_DCGE |
-	       CIO2_CGC_SIDE_DCGE |
-	       CIO2_CGC_PRIM_DCGE |
-	       CIO2_CGC_ROSC_DCGE |
-	       CIO2_CGC_XOSC_DCGE |
-	       CIO2_CGC_CLKGATE_HOLDOFF << CIO2_CGC_CLKGATE_HOLDOFF_SHIFT |
-	       CIO2_CGC_CSI_CLKGATE_HOLDOFF
-	       << CIO2_CGC_CSI_CLKGATE_HOLDOFF_SHIFT, base + CIO2_REG_CGC);
-	writel(CIO2_LTRCTRL_LTRDYNEN, base + CIO2_REG_LTRCTRL);
-	writel(CIO2_LTRVAL0_VAL << CIO2_LTRVAL02_VAL_SHIFT |
-	       CIO2_LTRVAL0_SCALE << CIO2_LTRVAL02_SCALE_SHIFT |
-	       CIO2_LTRVAL1_VAL << CIO2_LTRVAL13_VAL_SHIFT |
-	       CIO2_LTRVAL1_SCALE << CIO2_LTRVAL13_SCALE_SHIFT,
-	       base + CIO2_REG_LTRVAL01);
-	writel(CIO2_LTRVAL2_VAL << CIO2_LTRVAL02_VAL_SHIFT |
-	       CIO2_LTRVAL2_SCALE << CIO2_LTRVAL02_SCALE_SHIFT |
-	       CIO2_LTRVAL3_VAL << CIO2_LTRVAL13_VAL_SHIFT |
-	       CIO2_LTRVAL3_SCALE << CIO2_LTRVAL13_SCALE_SHIFT,
-	       base + CIO2_REG_LTRVAL23);
-
-	for (i = 0; i < CIO2_NUM_DMA_CHAN; i++) {
-		writel(0, base + CIO2_REG_CDMABA(i));
-		writel(0, base + CIO2_REG_CDMAC0(i));
-		writel(0, base + CIO2_REG_CDMAC1(i));
+	if (first) {
+		writel(CIO2_CGC_PRIM_TGE |
+		       CIO2_CGC_SIDE_TGE |
+		       CIO2_CGC_XOSC_TGE |
+		       CIO2_CGC_D3I3_TGE |
+		       CIO2_CGC_CSI2_INTERFRAME_TGE |
+		       CIO2_CGC_CSI2_PORT_DCGE |
+		       CIO2_CGC_SIDE_DCGE |
+		       CIO2_CGC_PRIM_DCGE |
+		       CIO2_CGC_ROSC_DCGE |
+		       CIO2_CGC_XOSC_DCGE |
+		       CIO2_CGC_CLKGATE_HOLDOFF << CIO2_CGC_CLKGATE_HOLDOFF_SHIFT |
+		       CIO2_CGC_CSI_CLKGATE_HOLDOFF
+		       << CIO2_CGC_CSI_CLKGATE_HOLDOFF_SHIFT, base + CIO2_REG_CGC);
+		writel(CIO2_LTRCTRL_LTRDYNEN, base + CIO2_REG_LTRCTRL);
+		writel(CIO2_LTRVAL0_VAL << CIO2_LTRVAL02_VAL_SHIFT |
+		       CIO2_LTRVAL0_SCALE << CIO2_LTRVAL02_SCALE_SHIFT |
+		       CIO2_LTRVAL1_VAL << CIO2_LTRVAL13_VAL_SHIFT |
+		       CIO2_LTRVAL1_SCALE << CIO2_LTRVAL13_SCALE_SHIFT,
+		       base + CIO2_REG_LTRVAL01);
+		writel(CIO2_LTRVAL2_VAL << CIO2_LTRVAL02_VAL_SHIFT |
+		       CIO2_LTRVAL2_SCALE << CIO2_LTRVAL02_SCALE_SHIFT |
+		       CIO2_LTRVAL3_VAL << CIO2_LTRVAL13_VAL_SHIFT |
+		       CIO2_LTRVAL3_SCALE << CIO2_LTRVAL13_SCALE_SHIFT,
+		       base + CIO2_REG_LTRVAL23);
+
+		for (i = 0; i < CIO2_NUM_DMA_CHAN; i++) {
+			writel(0, base + CIO2_REG_CDMABA(i));
+			writel(0, base + CIO2_REG_CDMAC0(i));
+			writel(0, base + CIO2_REG_CDMAC1(i));
+		}
+	} else {
+		writel(0, base + CIO2_REG_CDMABA(chan));
+		writel(0, base + CIO2_REG_CDMAC0(chan));
+		writel(0, base + CIO2_REG_CDMAC1(chan));
 	}
 
 	/* Enable DMA */
@@ -506,7 +518,8 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 	writel(1 << CIO2_CDMAC1_LINENUMUPDATE_SHIFT,
 	       base + CIO2_REG_CDMAC1(chan));
 
-	writel(0, base + CIO2_REG_PBM_FOPN_ABORT);
+	writel(first ? 0 : readl(base + CIO2_REG_PBM_FOPN_ABORT) &
+	       ~CIO2_PBM_FOPN_ABORT(csi2bus), base + CIO2_REG_PBM_FOPN_ABORT);
 
 	writel(CIO2_PXM_FRF_CFG_CRC_TH << CIO2_PXM_FRF_CFG_CRC_TH_SHIFT |
 	       CIO2_PXM_FRF_CFG_MSK_ECC_DPHY_NR |
@@ -516,9 +529,15 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 
 	/* Clear interrupts */
 	writel(CIO2_IRQCTRL_MASK, q->csi_rx_base + CIO2_REG_IRQCTRL_CLEAR);
-	writel(~0, base + CIO2_REG_INT_STS_EXT_OE);
-	writel(~0, base + CIO2_REG_INT_STS_EXT_IE);
-	writel(~0, base + CIO2_REG_INT_STS);
+	if (first) {
+		writel(~0, base + CIO2_REG_INT_STS_EXT_OE);
+		writel(~0, base + CIO2_REG_INT_STS_EXT_IE);
+		writel(~0, base + CIO2_REG_INT_STS);
+	} else {	/* don't eat the other streams' pending interrupts */
+		writel(0xffU << (8U * csi2bus), base + CIO2_REG_INT_STS_EXT_IE);
+		writel(CIO2_INT_IOC(chan) | CIO2_INT_IOS_IOLN(chan),
+		       base + CIO2_REG_INT_STS);
+	}
 
 	/* Enable devices, starting from the last device in the pipe */
 	writel(1, q->csi_rx_base + CIO2_REG_MIPIBE_ENABLE);
@@ -531,8 +550,7 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q)
 {
 	struct device *dev = &cio2->pci_dev->dev;
 	void __iomem *const base = cio2->base;
-	unsigned int i;
-	const unsigned int chan = q->dma_chan;
+	const unsigned int chan = q->dma_chan, port = q->csi2.port;
 	u32 value;
 	int ret;
 
@@ -550,12 +568,14 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q)
 	if (ret)
 		dev_err(dev, "DMA %u can not be halted\n", chan);
 
-	for (i = 0; i < CIO2_NUM_PORTS; i++) {
-		writel(readl(base + CIO2_REG_PXM_FRF_CFG(i)) |
-		       CIO2_PXM_FRF_CFG_ABORT, base + CIO2_REG_PXM_FRF_CFG(i));
-		writel(readl(base + CIO2_REG_PBM_FOPN_ABORT) |
-		       CIO2_PBM_FOPN_ABORT(i), base + CIO2_REG_PBM_FOPN_ABORT);
-	}
+	/* only this queue's port: others may still be streaming */
+	writel(readl(base + CIO2_REG_PXM_FRF_CFG(port)) |
+	       CIO2_PXM_FRF_CFG_ABORT, base + CIO2_REG_PXM_FRF_CFG(port));
+	writel(readl(base + CIO2_REG_PBM_FOPN_ABORT) |
+	       CIO2_PBM_FOPN_ABORT(port), base + CIO2_REG_PBM_FOPN_ABORT);
+	writel(readl(base + CIO2_REG_INT_EN) &
+	       ~(CIO2_INT_IOC(chan) | CIO2_REG_INT_EN_IOS(chan)),
+	       base + CIO2_REG_INT_EN);
 }
 
 static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
@@ -1017,14 +1037,17 @@ static int cio2_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
 	if (r)
 		goto fail_pipeline;
 
+	mutex_lock(&cio2->hw_lock);
 	cio2->chan_queue[q->dma_chan] = q;
 	r = cio2_hw_init(cio2, q);
 	if (r) {
 		cio2->chan_queue[q->dma_chan] = NULL;
+		mutex_unlock(&cio2->hw_lock);
 		goto fail_hw;
 	}
 	cio2->streaming++;
 	q->streaming = true;
+	mutex_unlock(&cio2->hw_lock);
 
 	/* Start streaming on sensor */
 	r = v4l2_subdev_call(q->sensor, video, s_stream, 1);
@@ -1034,11 +1057,13 @@ static int cio2_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 
 fail_csi2_subdev:
+	mutex_lock(&cio2->hw_lock);
 	cio2_hw_exit(cio2, q);
 	synchronize_irq(cio2->pci_dev->irq);
 	cio2->chan_queue[q->dma_chan] = NULL;
 	cio2->streaming--;
 	q->streaming = false;
+	mutex_unlock(&cio2->hw_lock);
 fail_hw:
 	video_device_pipeline_stop(&q->vdev);
 fail_pipeline:
@@ -1058,11 +1083,13 @@ static void cio2_vb2_stop_streaming(struct vb2_queue *vq)
 	if (v4l2_subdev_call(q->sensor, video, s_stream, 0))
 		dev_err(dev, "failed to stop sensor streaming\n");
 
+	mutex_lock(&cio2->hw_lock);
 	cio2_hw_exit(cio2, q);
 	synchronize_irq(cio2->pci_dev->irq);
 	cio2->chan_queue[q->dma_chan] = NULL;
 	cio2->streaming--;
 	q->streaming = false;
+	mutex_unlock(&cio2->hw_lock);
 	cio2_vb2_return_all_buffers(q, VB2_BUF_STATE_ERROR);
 	video_device_pipeline_stop(&q->vdev);
 	pm_runtime_put(dev);
@@ -1731,6 +1758,7 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 		return r;
 
 	mutex_init(&cio2->lock);
+	mutex_init(&cio2->hw_lock);
 
 	cio2->media_dev.dev = dev;
 	strscpy(cio2->media_dev.model, CIO2_DEVICE_NAME,
@@ -1783,6 +1811,7 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 	media_device_cleanup(&cio2->media_dev);
 fail_mutex_destroy:
 	mutex_destroy(&cio2->lock);
+	mutex_destroy(&cio2->hw_lock);
 	cio2_fbpt_exit_dummy(cio2);
 
 	return r;
@@ -1800,6 +1829,7 @@ static void cio2_pci_remove(struct pci_dev *pci_dev)
 	v4l2_device_unregister(&cio2->v4l2_dev);
 	media_device_cleanup(&cio2->media_dev);
 	mutex_destroy(&cio2->lock);
+	mutex_destroy(&cio2->hw_lock);
 
 	pm_runtime_forbid(&pci_dev->dev);
 	pm_runtime_get_noresume(&pci_dev->dev);
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index a6e363a4b..1d169fa1e 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -381,6 +381,8 @@ struct cio2_device {
 	struct cio2_queue *chan_queue[CIO2_NUM_DMA_CHAN];
 	/* mutex to be used by video_device */
 	struct mutex lock;
+	/* serialises hw_init/hw_exit of concurrent queues */
+	struct mutex hw_lock;
 
 	unsigned int streaming;	/* number of streaming queues */
 	struct v4l2_async_notifier notifier;
-- 
2.56.0

[PATCH 3/3] media: ipu3-cio2: give each CSI-2 port its own DMA channel

[Joe Dzado]

Map queue N to DMA channel 2*N so several sensors can stream
concurrently. Even channels are used because the IOS interrupt bits are
shared within channel pairs, keeping every queue's interrupts distinct.
A dma_per_port module parameter (default on) restores the old
single-channel behaviour if needed.
Signed-off-by: Joe Dzado <joe.dzado@icloud.com>

---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 071c5ea4a..f92ac4010 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -350,10 +350,18 @@ static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
 	return 0;
 };
 
-/* Each queue (CSI-2 port) owns one DMA channel. */
+/*
+ * Each queue (CSI-2 port) gets its own DMA channel so several sensors can stream at once.
+ * Channels 0, 2, 4, 6: IOS interrupt bits are shared by channel pairs (0/1, 2/3, ...) and
+ * IOC bits by pairs above 3, so even channels keep every queue's interrupts distinct.
+ */
+static bool dma_per_port = true;
+module_param(dma_per_port, bool, 0444);
+MODULE_PARM_DESC(dma_per_port, "Use a DMA channel per CSI-2 port (allows concurrent streams)");
+
 static unsigned int cio2_queue_dma_chan(unsigned int port)
 {
-	return CIO2_DMA_CHAN;
+	return dma_per_port ? 2 * port : CIO2_DMA_CHAN;
 }
 
 static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
-- 
2.56.0

Re: [PATCH 0/3] media: ipu3-cio2: support concurrent CSI-2 streams

[joe.dzado]

I’ve never submitted a patch before, so I’m not sure if I’m following the right process, but I found something while I was creating a person/gesture detection pipeline on my Surface Laptop (1st generation, 2017) — DMI product_name=Surface Laptop (no "2"+), i5-7200U.

I wanted to use the RGB and IR cameras at the same time and found I could not. So, I got a patch working. 

— Joe


> On Oct 30, 2026, at 6:23 PM, Joe Dzado <joe.dzado@icloud.com> wrote:
> 
> The ipu3-cio2 driver currently assumes a single active capture: every
> stream is pinned to DMA channel 0 (CIO2_DMA_CHAN) and tracked through a
> single cur_queue pointer, with the IRQ handler assuming all completion
> and start-of-frame events belong to that one queue. On platforms with
> more than one sensor wired to separate CSI-2 ports (e.g. the Microsoft
> Surface RGB + IR cameras), starting a second stream reprograms channel 0
> and cur_queue out from under the first, so only one sensor can capture
> at a time.
> 
> This series lets the driver use the DMA channels the hardware actually
> has, so multiple sensors can stream concurrently:
> 
>  1/3 dispatches interrupts by DMA channel instead of a single cur_queue
>      and converts the streaming state to a per-queue count (no
>      functional change);
>  2/3 serialises concurrent hw_init()/hw_exit() with a new lock and
>      guards the block-shared setup so bringing up a second stream does
>      not tear down the first (still single-channel, no functional
>      change);
>  3/3 maps each CSI-2 port to its own DMA channel (2*port), with a
>      dma_per_port module parameter to restore the old behaviour.
> 
> Each patch builds on its own and is bisectable. Tested on a Microsoft
> Surface Laptop (i5-7200U, Kaby Lake) with the OV9734 (colour) and OV7251
> (IR) sensors streaming concurrently on CIO2 ports 1 and 2 for several
> days without errors.
> 
> Joe Dzado (3):
>  media: ipu3-cio2: track the streaming queue by DMA channel
>  media: ipu3-cio2: serialise concurrent stream start and stop
>  media: ipu3-cio2: give each CSI-2 port its own DMA channel
> 
> drivers/media/pci/intel/ipu3/ipu3-cio2.c | 283 ++++++++++++++---------
> drivers/media/pci/intel/ipu3/ipu3-cio2.h |  10 +-
> 2 files changed, 188 insertions(+), 105 deletions(-)
> 
> -- 
> 2.56.0
>