[PATCH v2 0/4] media: camss: sm8250: Virtual channels support for SM8250

[PATCH v2 0/4] media: camss: sm8250: Virtual channels support for SM8250

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

These patches add support for multiple CSID virtual channels for 
Qualcomm's SM8250 platform.

Changed for v2:
- code syntax improvements
- The info print for enabled channels was demoted to a dbg print. Can be
  enabled with dynamic debug, e.g.:
echo "file drivers/media/platform/qcom/camss/* +p" > /sys/kernel/debug/dynamic_debug/control

NOTE: These changes depend on the multistream series that as of yet is
still not merged officially upstream. The multistream changes
allow for multiple clients on the same pipeline, which is otherwise not
possible. For development and testing multistream series v14 was used:

https://patchwork.kernel.org/project/linux-media/patch/20220831141357.1396081-2-tomi.valkeinen@ideasonboard.com/

The CSID hardware on SM8250 can demux the input data stream into
maximum of 4 multiple streams depending on virtual channel (vc)
or data type (dt) configuration.

Situations in which this is useful:
- HDR sensors that produce a 2-frame HDR output, e.g. a light and a dark frame
  (the setup we used  for testing, with the imx412 sensor),
  or a 3-frame HDR output - light, medium-lit, dark frame.
- sensors with additional metadata that is streamed over a different
  virtual channel/datatype.
- sensors that stream frames with multiple resolutions in the same pixel
  data stream

With these changes, the CSID entity has, as it did previously, a single
sink port (0), and always exposes 4 source ports (1, 2,3, 4). The
virtual channel configuration is determined by which of the source ports
are linked to an output VFE line. For example, the link below will
configure the CSID driver to enable vc 0 and vc 1:

media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]'

which will be demuxed and propagated into /dev/video0
and /dev/video1 respectively. With this, the userspace can use
any normal V4L2 client app to start/stop/queue/dequeue from these
video nodes. Tested with the yavta app.

Note also that implicit format propagation has been removed, so format
for the linked source ports has to be set manually, e.g.:

media-ctl -V '"msm_csid0":1[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_csid0":2[fmt:SRGGB10/3840x2160]'

Previously setting the format on the sink port of the CSID entity would
set the same format on the source port. However, with CSID demuxing there's
no guarantee that the format on the source ports should be the same,
in fact it can be different for each source port.

Milen Mitkov (4):
  media: camss: sm8250: Virtual channels for CSID
  media: camss: vfe: Reserve VFE lines on stream start and link to CSID
  media: camss: vfe-480: Multiple outputs support for SM8250
  media: camss: sm8250: Pipeline starting and stopping for multiple
    virtual channels

 .../platform/qcom/camss/camss-csid-gen2.c     | 55 +++++++++++------
 .../media/platform/qcom/camss/camss-csid.c    | 41 +++++++------
 .../media/platform/qcom/camss/camss-csid.h    | 11 +++-
 .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++-------
 drivers/media/platform/qcom/camss/camss-vfe.c |  7 +++
 .../media/platform/qcom/camss/camss-video.c   | 21 ++++++-
 drivers/media/platform/qcom/camss/camss.c     |  2 +-
 7 files changed, 134 insertions(+), 64 deletions(-)

-- 
2.37.3

[PATCH 1/4] media: camss: sm8250: Virtual channels for CSID

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

CSID hardware on SM8250 can demux up to 4 simultaneous streams
based on virtual channel (vc) or datatype (dt).
The CSID subdevice entity now has 4 source ports that can be
enabled/disabled and thus can control which virtual channels
are enabled. Datatype demuxing not tested.

The implicit propagation of port formats has been removed
(e.g. previously setting sink port format would set the same format
to source port), because the source port is now not guaranteed to
follow the same  format as the sink port.
So port formats have to be set explicitly.

CSID's s_stream is called when any stream is started or stopped.
It will call configure_streams() that will rewrite IRQ settings to HW.
When multiple streams are running simultaneously there is an issue
when writing IRQ settings for one stream while another is still
running, thus avoid re-writing settings if they were not changed
in link setup, or by fully powering off the CSID hardware.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 .../platform/qcom/camss/camss-csid-gen2.c     | 55 ++++++++++++-------
 .../media/platform/qcom/camss/camss-csid.c    | 41 ++++++++------
 .../media/platform/qcom/camss/camss-csid.h    | 11 +++-
 3 files changed, 68 insertions(+), 39 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index 2031bde13a93..feb0f2ba982c 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -334,13 +334,14 @@ static const struct csid_format csid_formats[] = {
 	},
 };
 
-static void csid_configure_stream(struct csid_device *csid, u8 enable)
+static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 channel)
 {
 	struct csid_testgen_config *tg = &csid->testgen;
 	u32 val;
 	u32 phy_sel = 0;
 	u8 lane_cnt = csid->phy.lane_cnt;
-	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_SRC];
+	/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
+	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + channel];
 	const struct csid_format *format = csid_get_fmt_entry(csid->formats, csid->nformats,
 							      input_format->code);
 
@@ -351,8 +352,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 		phy_sel = csid->phy.csiphy_id;
 
 	if (enable) {
-		u8 vc = 0; /* Virtual Channel 0 */
-		u8 dt_id = vc * 4;
+		u8 vc = channel; /* mapping between virtual channel and RDIn index */
+		u8 dt_id = vc;
 
 		if (tg->enabled) {
 			/* Config Test Generator */
@@ -395,42 +396,42 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 		val |= format->data_type << RDI_CFG0_DATA_TYPE;
 		val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
 		val |= dt_id << RDI_CFG0_DT_ID;
-		writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
+		writel_relaxed(val, csid->base + CSID_RDI_CFG0(channel));
 
 		/* CSID_TIMESTAMP_STB_POST_IRQ */
 		val = 2 << RDI_CFG1_TIMESTAMP_STB_SEL;
-		writel_relaxed(val, csid->base + CSID_RDI_CFG1(0));
+		writel_relaxed(val, csid->base + CSID_RDI_CFG1(channel));
 
 		val = 1;
-		writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(0));
+		writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(channel));
 
 		val = 0;
-		writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(0));
+		writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(channel));
 
 		val = 1;
-		writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(0));
+		writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(channel));
 
 		val = 0;
-		writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(0));
+		writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(channel));
 
 		val = 1;
-		writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(0));
+		writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(channel));
 
 		val = 0;
-		writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(0));
+		writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(channel));
 
 		val = 1;
-		writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(0));
+		writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(channel));
 
 		val = 0;
-		writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(0));
+		writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(channel));
 
 		val = 0;
-		writel_relaxed(val, csid->base + CSID_RDI_CTRL(0));
+		writel_relaxed(val, csid->base + CSID_RDI_CTRL(channel));
 
-		val = readl_relaxed(csid->base + CSID_RDI_CFG0(0));
+		val = readl_relaxed(csid->base + CSID_RDI_CFG0(channel));
 		val |=  1 << RDI_CFG0_ENABLE;
-		writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
+		writel_relaxed(val, csid->base + CSID_RDI_CFG0(channel));
 	}
 
 	if (tg->enabled) {
@@ -456,7 +457,16 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 		val = HALT_CMD_RESUME_AT_FRAME_BOUNDARY << RDI_CTRL_HALT_CMD;
 	else
 		val = HALT_CMD_HALT_AT_FRAME_BOUNDARY << RDI_CTRL_HALT_CMD;
-	writel_relaxed(val, csid->base + CSID_RDI_CTRL(0));
+	writel_relaxed(val, csid->base + CSID_RDI_CTRL(channel));
+}
+
+static void csid_configure_stream(struct csid_device *csid, u8 enable)
+{
+	u8 i;
+	/* Loop through all enabled VCs and configure stream for each */
+	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+		if (csid->phy.en_vc & BIT(i))
+			__csid_configure_stream(csid, enable, i);
 }
 
 static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
@@ -502,6 +512,7 @@ static irqreturn_t csid_isr(int irq, void *dev)
 	struct csid_device *csid = dev;
 	u32 val;
 	u8 reset_done;
+	int i;
 
 	val = readl_relaxed(csid->base + CSID_TOP_IRQ_STATUS);
 	writel_relaxed(val, csid->base + CSID_TOP_IRQ_CLEAR);
@@ -510,8 +521,12 @@ static irqreturn_t csid_isr(int irq, void *dev)
 	val = readl_relaxed(csid->base + CSID_CSI2_RX_IRQ_STATUS);
 	writel_relaxed(val, csid->base + CSID_CSI2_RX_IRQ_CLEAR);
 
-	val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(0));
-	writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(0));
+	/* Read and clear IRQ status for each enabled RDI channel */
+	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+		if (csid->phy.en_vc & BIT(i)) {
+			val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
+			writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
+		}
 
 	val = 1 << IRQ_CMD_CLEAR;
 	writel_relaxed(val, csid->base + CSID_IRQ_CMD);
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 88f188e0f750..fdb636f70010 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -209,6 +209,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 		}
 
 		csid->ops->hw_version(csid);
+
+		csid->phy.need_vc_update = true;
 	} else {
 		disable_irq(csid->irq);
 		camss_disable_clocks(csid->nclocks, csid->clock);
@@ -249,7 +251,10 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
 			return -ENOLINK;
 	}
 
-	csid->ops->configure_stream(csid, enable);
+	if (csid->phy.need_vc_update) {
+		csid->ops->configure_stream(csid, enable);
+		csid->phy.need_vc_update = false;
+	}
 
 	return 0;
 }
@@ -468,16 +473,6 @@ static int csid_set_format(struct v4l2_subdev *sd,
 	csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
 	*format = fmt->format;
 
-	/* Propagate the format from sink to source */
-	if (fmt->pad == MSM_CSID_PAD_SINK) {
-		format = __csid_get_format(csid, sd_state, MSM_CSID_PAD_SRC,
-					   fmt->which);
-
-		*format = fmt->format;
-		csid_try_format(csid, sd_state, MSM_CSID_PAD_SRC, format,
-				fmt->which);
-	}
-
 	return 0;
 }
 
@@ -738,7 +733,6 @@ static int csid_link_setup(struct media_entity *entity,
 		struct csid_device *csid;
 		struct csiphy_device *csiphy;
 		struct csiphy_lanes_cfg *lane_cfg;
-		struct v4l2_subdev_format format = { 0 };
 
 		sd = media_entity_to_v4l2_subdev(entity);
 		csid = v4l2_get_subdevdata(sd);
@@ -761,11 +755,22 @@ static int csid_link_setup(struct media_entity *entity,
 		lane_cfg = &csiphy->cfg.csi2->lane_cfg;
 		csid->phy.lane_cnt = lane_cfg->num_data;
 		csid->phy.lane_assign = csid_get_lane_assign(lane_cfg);
+	}
+	/* Decide which virtual channels to enable based on which source pads are enabled */
+	if (local->flags & MEDIA_PAD_FL_SOURCE) {
+		struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+		struct csid_device *csid = v4l2_get_subdevdata(sd);
+		struct device *dev = csid->camss->dev;
+
+		if (flags & MEDIA_LNK_FL_ENABLED)
+			csid->phy.en_vc |= BIT(local->index - 1);
+		else
+			csid->phy.en_vc &= ~BIT(local->index - 1);
 
-		/* Reset format on source pad to sink pad format */
-		format.pad = MSM_CSID_PAD_SRC;
-		format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-		csid_set_format(&csid->subdev, NULL, &format);
+		csid->phy.need_vc_update = true;
+
+		dev_dbg(dev, "%s: Enabled CSID virtual channels mask 0x%x\n",
+			__func__, csid->phy.en_vc);
 	}
 
 	return 0;
@@ -816,6 +821,7 @@ int msm_csid_register_entity(struct csid_device *csid,
 	struct v4l2_subdev *sd = &csid->subdev;
 	struct media_pad *pads = csid->pads;
 	struct device *dev = csid->camss->dev;
+	int i;
 	int ret;
 
 	v4l2_subdev_init(sd, &csid_v4l2_ops);
@@ -852,7 +858,8 @@ int msm_csid_register_entity(struct csid_device *csid,
 	}
 
 	pads[MSM_CSID_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
-	pads[MSM_CSID_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
+	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; ++i)
+		pads[i].flags = MEDIA_PAD_FL_SOURCE;
 
 	sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
 	sd->entity.ops = &csid_media_ops;
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index f06040e44c51..d4b48432a097 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -19,8 +19,13 @@
 #include <media/v4l2-subdev.h>
 
 #define MSM_CSID_PAD_SINK 0
-#define MSM_CSID_PAD_SRC 1
-#define MSM_CSID_PADS_NUM 2
+#define MSM_CSID_PAD_FIRST_SRC 1
+#define MSM_CSID_PADS_NUM 5
+
+#define MSM_CSID_PAD_SRC (MSM_CSID_PAD_FIRST_SRC)
+
+/* CSID hardware can demultiplex up to 4 outputs */
+#define MSM_CSID_MAX_SRC_STREAMS	4
 
 #define DATA_TYPE_EMBEDDED_DATA_8BIT	0x12
 #define DATA_TYPE_YUV420_8BIT		0x18
@@ -81,6 +86,8 @@ struct csid_phy_config {
 	u8 csiphy_id;
 	u8 lane_cnt;
 	u32 lane_assign;
+	u32 en_vc;
+	u8 need_vc_update;
 };
 
 struct csid_device;
-- 
2.37.3

[PATCH 2/4] media: camss: vfe: Reserve VFE lines on stream start and link to CSID

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

For multiple virtual channels support, each VFE line can be in either
ON, RESERVED or OFF states. This allows the starting and stopping
of a VFE line independently of other active VFE lines.

Also, link the CSID entity's source ports to corresponding VFE lines.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 drivers/media/platform/qcom/camss/camss-vfe.c | 7 +++++++
 drivers/media/platform/qcom/camss/camss.c     | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index a26e4a5d87b6..cd8ac0478cf1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -738,8 +738,10 @@ static int vfe_set_stream(struct v4l2_subdev *sd, int enable)
 	struct vfe_line *line = v4l2_get_subdevdata(sd);
 	struct vfe_device *vfe = to_vfe(line);
 	int ret;
+	int i;
 
 	if (enable) {
+		line->output.state = VFE_OUTPUT_RESERVED;
 		ret = vfe->ops->vfe_enable(line);
 		if (ret < 0)
 			dev_err(vfe->camss->dev,
@@ -749,6 +751,11 @@ static int vfe_set_stream(struct v4l2_subdev *sd, int enable)
 		if (ret < 0)
 			dev_err(vfe->camss->dev,
 				"Failed to disable vfe outputs\n");
+
+		/* At least one VFE line remains, return -EBUSY to avoid premature pipeline stop */
+		for (i = 0; i < vfe->line_num; i++)
+			if (vfe->line[i].output.state != VFE_OUTPUT_OFF)
+				return -EBUSY;
 	}
 
 	return ret;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 1118c40886d5..63653ac3e056 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1320,7 +1320,7 @@ static int camss_register_entities(struct camss *camss)
 					struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
 
 					ret = media_create_pad_link(&csid->entity,
-								    MSM_CSID_PAD_SRC,
+								    MSM_CSID_PAD_FIRST_SRC + j,
 								    &vfe->entity,
 								    MSM_VFE_PAD_SINK,
 								    0);
-- 
2.37.3

[PATCH 3/4] media: camss: vfe-480: Multiple outputs support for SM8250

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

On SM8250 each VFE supports at least 3 RDI channels, or 4
in case of VFE-Lite, so add appropriate IRQ setup and handling.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++-------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
index 129585110393..537dede334be 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
@@ -94,6 +94,8 @@ static inline int bus_irq_mask_0_comp_done(struct vfe_device *vfe, int n)
 #define RDI_WM(n)			((IS_LITE ? 0 : 23) + (n))
 #define RDI_COMP_GROUP(n)		((IS_LITE ? 0 : 11) + (n))
 
+#define MAX_VFE_OUTPUT_LINES	4
+
 static u32 vfe_hw_version(struct vfe_device *vfe)
 {
 	u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
@@ -171,12 +173,26 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,
 
 static void vfe_enable_irq_common(struct vfe_device *vfe)
 {
-	/* enable only the IRQs used: rup and comp_done irqs for RDI0 */
+	/* enable reset ack IRQ and top BUS status IRQ */
 	writel_relaxed(IRQ_MASK_0_RESET_ACK | IRQ_MASK_0_BUS_TOP_IRQ,
 		       vfe->base + VFE_IRQ_MASK(0));
-	writel_relaxed(BUS_IRQ_MASK_0_RDI_RUP(vfe, 0) |
-		       BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)),
-		       vfe->base + VFE_BUS_IRQ_MASK(0));
+}
+
+static void vfe_enable_lines_irq(struct vfe_device *vfe)
+{
+	u32 bus_irq_mask;
+	int i;
+
+	for (i = 0; i < MAX_VFE_OUTPUT_LINES; i++) {
+		/* Enable IRQ for newly added lines, but also keep already running lines's IRQ */
+		if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
+		    vfe->line[i].output.state == VFE_OUTPUT_ON) {
+			bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
+					| BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));
+			}
+	}
+
+	writel_relaxed(bus_irq_mask, vfe->base + VFE_BUS_IRQ_MASK(0));
 }
 
 static void vfe_isr_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id);
@@ -193,6 +209,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 {
 	struct vfe_device *vfe = dev;
 	u32 status;
+	int i;
 
 	status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
 	writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
@@ -207,11 +224,14 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 		writel_relaxed(status, vfe->base + VFE_BUS_IRQ_CLEAR(0));
 		writel_relaxed(1, vfe->base + VFE_BUS_IRQ_CLEAR_GLOBAL);
 
-		if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, 0))
-			vfe_isr_reg_update(vfe, 0);
+		/* Loop through all WMs IRQs */
+		for (i = 0; i < MSM_VFE_IMAGE_MASTERS_NUM; i++) {
+			if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, i))
+				vfe_isr_reg_update(vfe, i);
 
-		if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)))
-			vfe_isr_wm_done(vfe, 0);
+			if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i)))
+				vfe_isr_wm_done(vfe, i);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -234,24 +254,23 @@ static int vfe_get_output(struct vfe_line *line)
 	struct vfe_device *vfe = to_vfe(line);
 	struct vfe_output *output;
 	unsigned long flags;
-	int wm_idx;
 
 	spin_lock_irqsave(&vfe->output_lock, flags);
 
 	output = &line->output;
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is running\n");
 		goto error;
 	}
 
 	output->wm_num = 1;
 
-	wm_idx = vfe_reserve_wm(vfe, line->id);
-	if (wm_idx < 0) {
-		dev_err(vfe->camss->dev, "Can not reserve wm\n");
-		goto error_get_wm;
-	}
-	output->wm_idx[0] = wm_idx;
+	/* Correspondence between VFE line number and WM number.
+	 * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
+	 * Note this 1:1 mapping will not work for PIX streams.
+	 */
+	output->wm_idx[0] = line->id;
+	vfe->wm_output_map[line->id] = line->id;
 
 	output->drop_update_idx = 0;
 
@@ -259,11 +278,9 @@ static int vfe_get_output(struct vfe_line *line)
 
 	return 0;
 
-error_get_wm:
-	vfe_release_wm(vfe, output->wm_idx[0]);
-	output->state = VFE_OUTPUT_OFF;
 error:
 	spin_unlock_irqrestore(&vfe->output_lock, flags);
+	output->state = VFE_OUTPUT_OFF;
 
 	return -EINVAL;
 }
@@ -279,7 +296,7 @@ static int vfe_enable_output(struct vfe_line *line)
 
 	vfe_reg_update_clear(vfe, line->id);
 
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
 			output->state);
 		spin_unlock_irqrestore(&vfe->output_lock, flags);
@@ -360,6 +377,8 @@ static int vfe_enable(struct vfe_line *line)
 
 	vfe->stream_count++;
 
+	vfe_enable_lines_irq(vfe);
+
 	mutex_unlock(&vfe->stream_lock);
 
 	ret = vfe_get_output(line);
@@ -548,7 +567,7 @@ static const struct camss_video_ops vfe_video_ops_480 = {
 static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 {
 	vfe->video_ops = vfe_video_ops_480;
-	vfe->line_num = 1;
+	vfe->line_num = MAX_VFE_OUTPUT_LINES;
 }
 
 const struct vfe_hw_ops vfe_ops_480 = {
-- 
2.37.3

[PATCH 4/4] media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

Use the multistream series function video_device_pipeline_alloc_start
to allows multiple clients of the same pipeline.

If any of the entities in the pipeline doesn't return success at stop
(e.g. if a VFE line remains running), the full pipeline won't be stopped.
This allows for stopping and starting streams at any point without
disrupting the other running streams.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 .../media/platform/qcom/camss/camss-video.c   | 21 ++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 81fb3a5bc1d5..b042faf3dcda 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -353,6 +353,7 @@ static int video_get_subdev_format(struct camss_video *video,
 
 	fmt.pad = pad;
 	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt.stream = 0;
 
 	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
 	if (ret)
@@ -493,9 +494,11 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
 	struct v4l2_subdev *subdev;
 	int ret;
 
-	ret = video_device_pipeline_start(vdev, &video->pipe);
-	if (ret < 0)
+	ret = video_device_pipeline_alloc_start(vdev);
+	if (ret < 0) {
+		dev_err(video->camss->dev, "Failed to start media pipeline: %d\n", ret);
 		return ret;
+	}
 
 	ret = video_check_format(video);
 	if (ret < 0)
@@ -536,6 +539,7 @@ static void video_stop_streaming(struct vb2_queue *q)
 	struct media_entity *entity;
 	struct media_pad *pad;
 	struct v4l2_subdev *subdev;
+	int ret;
 
 	entity = &vdev->entity;
 	while (1) {
@@ -550,7 +554,18 @@ static void video_stop_streaming(struct vb2_queue *q)
 		entity = pad->entity;
 		subdev = media_entity_to_v4l2_subdev(entity);
 
-		v4l2_subdev_call(subdev, video, s_stream, 0);
+		ret = v4l2_subdev_call(subdev, video, s_stream, 0);
+
+		if (ret == -EBUSY) {
+			/* Don't stop if other instances of the pipeline are still running */
+			dev_dbg(video->camss->dev, "Video pipeline still used, don't stop streaming.\n");
+			return;
+		}
+
+		if (ret) {
+			dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
+			return;
+		}
 	}
 
 	video_device_pipeline_stop(vdev);
-- 
2.37.3

Re: [PATCH 4/4] media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels

[Bryan O'Donoghue]

On 26/09/2022 15:25, quic_mmitkov@quicinc.com wrote:
> +	fmt.stream = 0;

Thanks for updating your series.

I downloaded and applied but the above is generating an error for me on 
linux-next..

Its probably because its late/early but I don't see which branch you are 
working from here ?

---
bod

Re: [PATCH 4/4] media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels

[Milen Mitkov (Consultant)]

Hi Bryan,

What is the error you're getting?

I am testing on the linux-stable-22-09-09-imx577-camss branch. I wanted 
to try on the newest one 
(linux-stable-22-09-14-qrb5165-rb5-vision-mezzanine) but the multistream 
pathes wouldn't apply cleanly there.

On 28/09/2022 05:44, Bryan O'Donoghue wrote:
> On 26/09/2022 15:25, quic_mmitkov@quicinc.com wrote:
>> +    fmt.stream = 0;
>
> Thanks for updating your series.
>
> I downloaded and applied but the above is generating an error for me 
> on linux-next..
>
> Its probably because its late/early but I don't see which branch you 
> are working from here ?
>
> ---
> bod

Re: [PATCH 4/4] media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels

[Bryan O'Donoghue]

On 28/09/2022 12:35, Milen Mitkov (Consultant) wrote:
> Hi Bryan,
> 
> What is the error you're getting?
> 
> I am testing on the linux-stable-22-09-09-imx577-camss branch. I wanted 
> to try on the newest one 
> (linux-stable-22-09-14-qrb5165-rb5-vision-mezzanine) but the multistream 
> pathes wouldn't apply cleanly there.

I tried linux-next as at last night but the compilation exploded, so I 
went back just a little bit further in time to pick up a reasonable set 
of linux-media stuff in linux-next and applied yours and my changes on top.

To ssh://git.linaro.org/people/bryan.odonoghue/kernel.git
  * [new branch]                HEAD -> linux-next+camss-changes-v4

---
bod

Re: [PATCH 1/4] media: camss: sm8250: Virtual channels for CSID

[Robert Foss]

Hey Milen,

Thanks for submitting this series.

On Mon, 26 Sept 2022 at 16:25, <quic_mmitkov@quicinc.com> wrote:
>
> From: Milen Mitkov <quic_mmitkov@quicinc.com>
>
> CSID hardware on SM8250 can demux up to 4 simultaneous streams
> based on virtual channel (vc) or datatype (dt).
> The CSID subdevice entity now has 4 source ports that can be
> enabled/disabled and thus can control which virtual channels
> are enabled. Datatype demuxing not tested.
>
> The implicit propagation of port formats has been removed
> (e.g. previously setting sink port format would set the same format
> to source port), because the source port is now not guaranteed to
> follow the same  format as the sink port.
> So port formats have to be set explicitly.
>
> CSID's s_stream is called when any stream is started or stopped.
> It will call configure_streams() that will rewrite IRQ settings to HW.
> When multiple streams are running simultaneously there is an issue
> when writing IRQ settings for one stream while another is still
> running, thus avoid re-writing settings if they were not changed
> in link setup, or by fully powering off the CSID hardware.
>
> Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
> ---
>  .../platform/qcom/camss/camss-csid-gen2.c     | 55 ++++++++++++-------
>  .../media/platform/qcom/camss/camss-csid.c    | 41 ++++++++------
>  .../media/platform/qcom/camss/camss-csid.h    | 11 +++-
>  3 files changed, 68 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index 2031bde13a93..feb0f2ba982c 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -334,13 +334,14 @@ static const struct csid_format csid_formats[] = {
>         },
>  };
>
> -static void csid_configure_stream(struct csid_device *csid, u8 enable)
> +static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 channel)
>  {
>         struct csid_testgen_config *tg = &csid->testgen;
>         u32 val;
>         u32 phy_sel = 0;
>         u8 lane_cnt = csid->phy.lane_cnt;
> -       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_SRC];
> +       /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
> +       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + channel];
>         const struct csid_format *format = csid_get_fmt_entry(csid->formats, csid->nformats,
>                                                               input_format->code);
>
> @@ -351,8 +352,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>                 phy_sel = csid->phy.csiphy_id;
>
>         if (enable) {
> -               u8 vc = 0; /* Virtual Channel 0 */
> -               u8 dt_id = vc * 4;
> +               u8 vc = channel; /* mapping between virtual channel and RDIn index */

Is there any reason for having two variables (vc & channel)
representing the same thing? If not, let's keep just keep vc.

> +               u8 dt_id = vc;
>
>                 if (tg->enabled) {
>                         /* Config Test Generator */
> @@ -395,42 +396,42 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>                 val |= format->data_type << RDI_CFG0_DATA_TYPE;
>                 val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>                 val |= dt_id << RDI_CFG0_DT_ID;
> -               writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_CFG0(channel));
>
>                 /* CSID_TIMESTAMP_STB_POST_IRQ */
>                 val = 2 << RDI_CFG1_TIMESTAMP_STB_SEL;
> -               writel_relaxed(val, csid->base + CSID_RDI_CFG1(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_CFG1(channel));
>
>                 val = 1;
> -               writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(channel));
>
>                 val = 0;
> -               writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(channel));
>
>                 val = 1;
> -               writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(channel));
>
>                 val = 0;
> -               writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(channel));
>
>                 val = 1;
> -               writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(channel));
>
>                 val = 0;
> -               writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(channel));
>
>                 val = 1;
> -               writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(channel));
>
>                 val = 0;
> -               writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(channel));
>
>                 val = 0;
> -               writel_relaxed(val, csid->base + CSID_RDI_CTRL(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_CTRL(channel));
>
> -               val = readl_relaxed(csid->base + CSID_RDI_CFG0(0));
> +               val = readl_relaxed(csid->base + CSID_RDI_CFG0(channel));
>                 val |=  1 << RDI_CFG0_ENABLE;
> -               writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
> +               writel_relaxed(val, csid->base + CSID_RDI_CFG0(channel));
>         }
>
>         if (tg->enabled) {
> @@ -456,7 +457,16 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>                 val = HALT_CMD_RESUME_AT_FRAME_BOUNDARY << RDI_CTRL_HALT_CMD;
>         else
>                 val = HALT_CMD_HALT_AT_FRAME_BOUNDARY << RDI_CTRL_HALT_CMD;
> -       writel_relaxed(val, csid->base + CSID_RDI_CTRL(0));
> +       writel_relaxed(val, csid->base + CSID_RDI_CTRL(channel));
> +}
> +
> +static void csid_configure_stream(struct csid_device *csid, u8 enable)
> +{
> +       u8 i;
> +       /* Loop through all enabled VCs and configure stream for each */
> +       for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
> +               if (csid->phy.en_vc & BIT(i))
> +                       __csid_configure_stream(csid, enable, i);
>  }
>
>  static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
> @@ -502,6 +512,7 @@ static irqreturn_t csid_isr(int irq, void *dev)
>         struct csid_device *csid = dev;
>         u32 val;
>         u8 reset_done;
> +       int i;
>
>         val = readl_relaxed(csid->base + CSID_TOP_IRQ_STATUS);
>         writel_relaxed(val, csid->base + CSID_TOP_IRQ_CLEAR);
> @@ -510,8 +521,12 @@ static irqreturn_t csid_isr(int irq, void *dev)
>         val = readl_relaxed(csid->base + CSID_CSI2_RX_IRQ_STATUS);
>         writel_relaxed(val, csid->base + CSID_CSI2_RX_IRQ_CLEAR);
>
> -       val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(0));
> -       writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(0));
> +       /* Read and clear IRQ status for each enabled RDI channel */
> +       for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
> +               if (csid->phy.en_vc & BIT(i)) {
> +                       val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
> +                       writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
> +               }
>
>         val = 1 << IRQ_CMD_CLEAR;
>         writel_relaxed(val, csid->base + CSID_IRQ_CMD);
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 88f188e0f750..fdb636f70010 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -209,6 +209,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
>                 }
>
>                 csid->ops->hw_version(csid);
> +
> +               csid->phy.need_vc_update = true;
>         } else {
>                 disable_irq(csid->irq);
>                 camss_disable_clocks(csid->nclocks, csid->clock);
> @@ -249,7 +251,10 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
>                         return -ENOLINK;
>         }
>
> -       csid->ops->configure_stream(csid, enable);
> +       if (csid->phy.need_vc_update) {
> +               csid->ops->configure_stream(csid, enable);
> +               csid->phy.need_vc_update = false;
> +       }
>
>         return 0;
>  }
> @@ -468,16 +473,6 @@ static int csid_set_format(struct v4l2_subdev *sd,
>         csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
>         *format = fmt->format;
>
> -       /* Propagate the format from sink to source */
> -       if (fmt->pad == MSM_CSID_PAD_SINK) {
> -               format = __csid_get_format(csid, sd_state, MSM_CSID_PAD_SRC,
> -                                          fmt->which);
> -
> -               *format = fmt->format;
> -               csid_try_format(csid, sd_state, MSM_CSID_PAD_SRC, format,
> -                               fmt->which);
> -       }
> -
>         return 0;
>  }
>
> @@ -738,7 +733,6 @@ static int csid_link_setup(struct media_entity *entity,
>                 struct csid_device *csid;
>                 struct csiphy_device *csiphy;
>                 struct csiphy_lanes_cfg *lane_cfg;
> -               struct v4l2_subdev_format format = { 0 };
>
>                 sd = media_entity_to_v4l2_subdev(entity);
>                 csid = v4l2_get_subdevdata(sd);
> @@ -761,11 +755,22 @@ static int csid_link_setup(struct media_entity *entity,
>                 lane_cfg = &csiphy->cfg.csi2->lane_cfg;
>                 csid->phy.lane_cnt = lane_cfg->num_data;
>                 csid->phy.lane_assign = csid_get_lane_assign(lane_cfg);
> +       }
> +       /* Decide which virtual channels to enable based on which source pads are enabled */
> +       if (local->flags & MEDIA_PAD_FL_SOURCE) {
> +               struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> +               struct csid_device *csid = v4l2_get_subdevdata(sd);
> +               struct device *dev = csid->camss->dev;
> +
> +               if (flags & MEDIA_LNK_FL_ENABLED)
> +                       csid->phy.en_vc |= BIT(local->index - 1);
> +               else
> +                       csid->phy.en_vc &= ~BIT(local->index - 1);
>
> -               /* Reset format on source pad to sink pad format */
> -               format.pad = MSM_CSID_PAD_SRC;
> -               format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> -               csid_set_format(&csid->subdev, NULL, &format);
> +               csid->phy.need_vc_update = true;
> +
> +               dev_dbg(dev, "%s: Enabled CSID virtual channels mask 0x%x\n",
> +                       __func__, csid->phy.en_vc);
>         }
>
>         return 0;
> @@ -816,6 +821,7 @@ int msm_csid_register_entity(struct csid_device *csid,
>         struct v4l2_subdev *sd = &csid->subdev;
>         struct media_pad *pads = csid->pads;
>         struct device *dev = csid->camss->dev;
> +       int i;
>         int ret;
>
>         v4l2_subdev_init(sd, &csid_v4l2_ops);
> @@ -852,7 +858,8 @@ int msm_csid_register_entity(struct csid_device *csid,
>         }
>
>         pads[MSM_CSID_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> -       pads[MSM_CSID_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
> +       for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; ++i)
> +               pads[i].flags = MEDIA_PAD_FL_SOURCE;
>
>         sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>         sd->entity.ops = &csid_media_ops;
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index f06040e44c51..d4b48432a097 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -19,8 +19,13 @@
>  #include <media/v4l2-subdev.h>
>
>  #define MSM_CSID_PAD_SINK 0
> -#define MSM_CSID_PAD_SRC 1
> -#define MSM_CSID_PADS_NUM 2
> +#define MSM_CSID_PAD_FIRST_SRC 1
> +#define MSM_CSID_PADS_NUM 5
> +
> +#define MSM_CSID_PAD_SRC (MSM_CSID_PAD_FIRST_SRC)
> +
> +/* CSID hardware can demultiplex up to 4 outputs */
> +#define MSM_CSID_MAX_SRC_STREAMS       4
>
>  #define DATA_TYPE_EMBEDDED_DATA_8BIT   0x12
>  #define DATA_TYPE_YUV420_8BIT          0x18
> @@ -81,6 +86,8 @@ struct csid_phy_config {
>         u8 csiphy_id;
>         u8 lane_cnt;
>         u32 lane_assign;
> +       u32 en_vc;
> +       u8 need_vc_update;
>  };
>
>  struct csid_device;
> --
> 2.37.3q
>

With the above fixed, please add my r-b.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH 2/4] media: camss: vfe: Reserve VFE lines on stream start and link to CSID

[Robert Foss]

On Mon, 26 Sept 2022 at 16:25, <quic_mmitkov@quicinc.com> wrote:
>
> From: Milen Mitkov <quic_mmitkov@quicinc.com>
>
> For multiple virtual channels support, each VFE line can be in either
> ON, RESERVED or OFF states. This allows the starting and stopping
> of a VFE line independently of other active VFE lines.
>
> Also, link the CSID entity's source ports to corresponding VFE lines.
>
> Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 7 +++++++
>  drivers/media/platform/qcom/camss/camss.c     | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index a26e4a5d87b6..cd8ac0478cf1 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -738,8 +738,10 @@ static int vfe_set_stream(struct v4l2_subdev *sd, int enable)
>         struct vfe_line *line = v4l2_get_subdevdata(sd);
>         struct vfe_device *vfe = to_vfe(line);
>         int ret;
> +       int i;
>
>         if (enable) {
> +               line->output.state = VFE_OUTPUT_RESERVED;
>                 ret = vfe->ops->vfe_enable(line);
>                 if (ret < 0)
>                         dev_err(vfe->camss->dev,
> @@ -749,6 +751,11 @@ static int vfe_set_stream(struct v4l2_subdev *sd, int enable)
>                 if (ret < 0)
>                         dev_err(vfe->camss->dev,
>                                 "Failed to disable vfe outputs\n");
> +
> +               /* At least one VFE line remains, return -EBUSY to avoid premature pipeline stop */
> +               for (i = 0; i < vfe->line_num; i++)
> +                       if (vfe->line[i].output.state != VFE_OUTPUT_OFF)
> +                               return -EBUSY;
>         }
>
>         return ret;
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 1118c40886d5..63653ac3e056 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1320,7 +1320,7 @@ static int camss_register_entities(struct camss *camss)
>                                         struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
>
>                                         ret = media_create_pad_link(&csid->entity,
> -                                                                   MSM_CSID_PAD_SRC,
> +                                                                   MSM_CSID_PAD_FIRST_SRC + j,
>                                                                     &vfe->entity,
>                                                                     MSM_VFE_PAD_SINK,
>                                                                     0);
> --
> 2.37.3
>

Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH 3/4] media: camss: vfe-480: Multiple outputs support for SM8250

[Robert Foss]

On Mon, 26 Sept 2022 at 16:25, <quic_mmitkov@quicinc.com> wrote:
>
> From: Milen Mitkov <quic_mmitkov@quicinc.com>
>
> On SM8250 each VFE supports at least 3 RDI channels, or 4
> in case of VFE-Lite, so add appropriate IRQ setup and handling.
>
> Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
> ---
>  .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++-------
>  1 file changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> index 129585110393..537dede334be 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> @@ -94,6 +94,8 @@ static inline int bus_irq_mask_0_comp_done(struct vfe_device *vfe, int n)
>  #define RDI_WM(n)                      ((IS_LITE ? 0 : 23) + (n))
>  #define RDI_COMP_GROUP(n)              ((IS_LITE ? 0 : 11) + (n))
>
> +#define MAX_VFE_OUTPUT_LINES   4
> +
>  static u32 vfe_hw_version(struct vfe_device *vfe)
>  {
>         u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
> @@ -171,12 +173,26 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,
>
>  static void vfe_enable_irq_common(struct vfe_device *vfe)
>  {
> -       /* enable only the IRQs used: rup and comp_done irqs for RDI0 */
> +       /* enable reset ack IRQ and top BUS status IRQ */
>         writel_relaxed(IRQ_MASK_0_RESET_ACK | IRQ_MASK_0_BUS_TOP_IRQ,
>                        vfe->base + VFE_IRQ_MASK(0));
> -       writel_relaxed(BUS_IRQ_MASK_0_RDI_RUP(vfe, 0) |
> -                      BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)),
> -                      vfe->base + VFE_BUS_IRQ_MASK(0));
> +}
> +
> +static void vfe_enable_lines_irq(struct vfe_device *vfe)
> +{
> +       u32 bus_irq_mask;
> +       int i;
> +
> +       for (i = 0; i < MAX_VFE_OUTPUT_LINES; i++) {
> +               /* Enable IRQ for newly added lines, but also keep already running lines's IRQ */
> +               if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
> +                   vfe->line[i].output.state == VFE_OUTPUT_ON) {
> +                       bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
> +                                       | BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));
> +                       }
> +       }
> +
> +       writel_relaxed(bus_irq_mask, vfe->base + VFE_BUS_IRQ_MASK(0));
>  }
>
>  static void vfe_isr_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id);
> @@ -193,6 +209,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>  {
>         struct vfe_device *vfe = dev;
>         u32 status;
> +       int i;
>
>         status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
>         writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
> @@ -207,11 +224,14 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>                 writel_relaxed(status, vfe->base + VFE_BUS_IRQ_CLEAR(0));
>                 writel_relaxed(1, vfe->base + VFE_BUS_IRQ_CLEAR_GLOBAL);
>
> -               if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, 0))
> -                       vfe_isr_reg_update(vfe, 0);
> +               /* Loop through all WMs IRQs */
> +               for (i = 0; i < MSM_VFE_IMAGE_MASTERS_NUM; i++) {
> +                       if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, i))
> +                               vfe_isr_reg_update(vfe, i);
>
> -               if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)))
> -                       vfe_isr_wm_done(vfe, 0);
> +                       if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i)))
> +                               vfe_isr_wm_done(vfe, i);
> +               }
>         }
>
>         return IRQ_HANDLED;
> @@ -234,24 +254,23 @@ static int vfe_get_output(struct vfe_line *line)
>         struct vfe_device *vfe = to_vfe(line);
>         struct vfe_output *output;
>         unsigned long flags;
> -       int wm_idx;
>
>         spin_lock_irqsave(&vfe->output_lock, flags);
>
>         output = &line->output;
> -       if (output->state != VFE_OUTPUT_OFF) {
> +       if (output->state > VFE_OUTPUT_RESERVED) {
>                 dev_err(vfe->camss->dev, "Output is running\n");
>                 goto error;
>         }
>
>         output->wm_num = 1;
>
> -       wm_idx = vfe_reserve_wm(vfe, line->id);
> -       if (wm_idx < 0) {
> -               dev_err(vfe->camss->dev, "Can not reserve wm\n");
> -               goto error_get_wm;
> -       }
> -       output->wm_idx[0] = wm_idx;
> +       /* Correspondence between VFE line number and WM number.
> +        * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
> +        * Note this 1:1 mapping will not work for PIX streams.
> +        */
> +       output->wm_idx[0] = line->id;
> +       vfe->wm_output_map[line->id] = line->id;
>
>         output->drop_update_idx = 0;
>
> @@ -259,11 +278,9 @@ static int vfe_get_output(struct vfe_line *line)
>
>         return 0;
>
> -error_get_wm:
> -       vfe_release_wm(vfe, output->wm_idx[0]);
> -       output->state = VFE_OUTPUT_OFF;
>  error:
>         spin_unlock_irqrestore(&vfe->output_lock, flags);
> +       output->state = VFE_OUTPUT_OFF;
>
>         return -EINVAL;
>  }
> @@ -279,7 +296,7 @@ static int vfe_enable_output(struct vfe_line *line)
>
>         vfe_reg_update_clear(vfe, line->id);
>
> -       if (output->state != VFE_OUTPUT_OFF) {
> +       if (output->state > VFE_OUTPUT_RESERVED) {
>                 dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
>                         output->state);
>                 spin_unlock_irqrestore(&vfe->output_lock, flags);
> @@ -360,6 +377,8 @@ static int vfe_enable(struct vfe_line *line)
>
>         vfe->stream_count++;
>
> +       vfe_enable_lines_irq(vfe);
> +
>         mutex_unlock(&vfe->stream_lock);
>
>         ret = vfe_get_output(line);
> @@ -548,7 +567,7 @@ static const struct camss_video_ops vfe_video_ops_480 = {
>  static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
>  {
>         vfe->video_ops = vfe_video_ops_480;
> -       vfe->line_num = 1;
> +       vfe->line_num = MAX_VFE_OUTPUT_LINES;
>  }
>
>  const struct vfe_hw_ops vfe_ops_480 = {
> --
> 2.37.3
>


Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH 3/4] media: camss: vfe-480: Multiple outputs support for SM8250

[Bryan O'Donoghue]

On 21/09/2022 15:10, quic_mmitkov@quicinc.com wrote:
> From: Milen Mitkov <quic_mmitkov@quicinc.com>
> 
> On SM8250 each VFE supports at least 3 RDI channels, or 4
> in case of VFE-Lite, so add appropriate IRQ setup and handling.
> 
> Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
> ---
>   .../media/platform/qcom/camss/camss-vfe-480.c | 60 ++++++++++++-------
>   1 file changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> index 129585110393..04272d085e5b 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> @@ -94,6 +94,8 @@ static inline int bus_irq_mask_0_comp_done(struct vfe_device *vfe, int n)
>   #define RDI_WM(n)			((IS_LITE ? 0 : 23) + (n))
>   #define RDI_COMP_GROUP(n)		((IS_LITE ? 0 : 11) + (n))
>   
> +#define MAX_VFE_OUTPUT_LINES	4
> +
>   static u32 vfe_hw_version(struct vfe_device *vfe)
>   {
>   	u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
> @@ -171,12 +173,25 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,
>   
>   static void vfe_enable_irq_common(struct vfe_device *vfe)
>   {
> -	/* enable only the IRQs used: rup and comp_done irqs for RDI0 */
> +	/* enable reset ack IRQ and top BUS status IRQ */
>   	writel_relaxed(IRQ_MASK_0_RESET_ACK | IRQ_MASK_0_BUS_TOP_IRQ,
>   		       vfe->base + VFE_IRQ_MASK(0));
> -	writel_relaxed(BUS_IRQ_MASK_0_RDI_RUP(vfe, 0) |
> -		       BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)),
> -		       vfe->base + VFE_BUS_IRQ_MASK(0));
> +}
> +
> +static void vfe_enable_lines_irq(struct vfe_device *vfe)
> +{
> +	u32 bus_irq_mask;
> +	int i;
> +
> +	for (i = 0; i < MAX_VFE_OUTPUT_LINES; i++) {
> +		/* Enable IRQ for newly added lines, but also keep already running lines's IRQ */
> +		if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
> +		    vfe->line[i].output.state == VFE_OUTPUT_ON)
> +			bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
> +					| BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));

A multi-line should be enclosed with {}

if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
     vfe->line[i].output.state == VFE_OUTPUT_ON) {
	bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
			| BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));
}

---
bod

[PATCH 3/4] media: camss: vfe-480: Multiple outputs support for SM8250

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

On SM8250 each VFE supports at least 3 RDI channels, or 4
in case of VFE-Lite, so add appropriate IRQ setup and handling.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 .../media/platform/qcom/camss/camss-vfe-480.c | 60 ++++++++++++-------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
index 129585110393..04272d085e5b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
@@ -94,6 +94,8 @@ static inline int bus_irq_mask_0_comp_done(struct vfe_device *vfe, int n)
 #define RDI_WM(n)			((IS_LITE ? 0 : 23) + (n))
 #define RDI_COMP_GROUP(n)		((IS_LITE ? 0 : 11) + (n))
 
+#define MAX_VFE_OUTPUT_LINES	4
+
 static u32 vfe_hw_version(struct vfe_device *vfe)
 {
 	u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
@@ -171,12 +173,25 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,
 
 static void vfe_enable_irq_common(struct vfe_device *vfe)
 {
-	/* enable only the IRQs used: rup and comp_done irqs for RDI0 */
+	/* enable reset ack IRQ and top BUS status IRQ */
 	writel_relaxed(IRQ_MASK_0_RESET_ACK | IRQ_MASK_0_BUS_TOP_IRQ,
 		       vfe->base + VFE_IRQ_MASK(0));
-	writel_relaxed(BUS_IRQ_MASK_0_RDI_RUP(vfe, 0) |
-		       BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)),
-		       vfe->base + VFE_BUS_IRQ_MASK(0));
+}
+
+static void vfe_enable_lines_irq(struct vfe_device *vfe)
+{
+	u32 bus_irq_mask;
+	int i;
+
+	for (i = 0; i < MAX_VFE_OUTPUT_LINES; i++) {
+		/* Enable IRQ for newly added lines, but also keep already running lines's IRQ */
+		if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
+		    vfe->line[i].output.state == VFE_OUTPUT_ON)
+			bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
+					| BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));
+	}
+
+	writel_relaxed(bus_irq_mask, vfe->base + VFE_BUS_IRQ_MASK(0));
 }
 
 static void vfe_isr_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id);
@@ -193,6 +208,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 {
 	struct vfe_device *vfe = dev;
 	u32 status;
+	int i;
 
 	status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
 	writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
@@ -207,11 +223,14 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 		writel_relaxed(status, vfe->base + VFE_BUS_IRQ_CLEAR(0));
 		writel_relaxed(1, vfe->base + VFE_BUS_IRQ_CLEAR_GLOBAL);
 
-		if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, 0))
-			vfe_isr_reg_update(vfe, 0);
+		/* Loop through all WMs IRQs */
+		for (i = 0; i < MSM_VFE_IMAGE_MASTERS_NUM; i++) {
+			if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, i))
+				vfe_isr_reg_update(vfe, i);
 
-		if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)))
-			vfe_isr_wm_done(vfe, 0);
+			if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i)))
+				vfe_isr_wm_done(vfe, i);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -234,24 +253,23 @@ static int vfe_get_output(struct vfe_line *line)
 	struct vfe_device *vfe = to_vfe(line);
 	struct vfe_output *output;
 	unsigned long flags;
-	int wm_idx;
 
 	spin_lock_irqsave(&vfe->output_lock, flags);
 
 	output = &line->output;
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is running\n");
 		goto error;
 	}
 
 	output->wm_num = 1;
 
-	wm_idx = vfe_reserve_wm(vfe, line->id);
-	if (wm_idx < 0) {
-		dev_err(vfe->camss->dev, "Can not reserve wm\n");
-		goto error_get_wm;
-	}
-	output->wm_idx[0] = wm_idx;
+	/* Correspondence between VFE line number and WM number.
+	 * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
+	 * Note this 1:1 mapping will not work for PIX streams.
+	 */
+	output->wm_idx[0] = line->id;
+	vfe->wm_output_map[line->id] = line->id;
 
 	output->drop_update_idx = 0;
 
@@ -259,11 +277,9 @@ static int vfe_get_output(struct vfe_line *line)
 
 	return 0;
 
-error_get_wm:
-	vfe_release_wm(vfe, output->wm_idx[0]);
-	output->state = VFE_OUTPUT_OFF;
 error:
 	spin_unlock_irqrestore(&vfe->output_lock, flags);
+	output->state = VFE_OUTPUT_OFF;
 
 	return -EINVAL;
 }
@@ -279,7 +295,7 @@ static int vfe_enable_output(struct vfe_line *line)
 
 	vfe_reg_update_clear(vfe, line->id);
 
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
 			output->state);
 		spin_unlock_irqrestore(&vfe->output_lock, flags);
@@ -360,6 +376,8 @@ static int vfe_enable(struct vfe_line *line)
 
 	vfe->stream_count++;
 
+	vfe_enable_lines_irq(vfe);
+
 	mutex_unlock(&vfe->stream_lock);
 
 	ret = vfe_get_output(line);
@@ -548,7 +566,7 @@ static const struct camss_video_ops vfe_video_ops_480 = {
 static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 {
 	vfe->video_ops = vfe_video_ops_480;
-	vfe->line_num = 1;
+	vfe->line_num = MAX_VFE_OUTPUT_LINES;
 }
 
 const struct vfe_hw_ops vfe_ops_480 = {
-- 
2.37.3

[PATCH 3/4] media: camss: vfe-480: Multiple outputs support for SM8250

[quic_mmitkov]

From: Milen Mitkov <quic_mmitkov@quicinc.com>

On SM8250 each VFE supports at least 3 RDI channels, or 4
in case of VFE-Lite, so add appropriate IRQ setup and handling.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
---
 .../media/platform/qcom/camss/camss-vfe-480.c | 60 ++++++++++++-------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
index 129585110393..04272d085e5b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
@@ -94,6 +94,8 @@ static inline int bus_irq_mask_0_comp_done(struct vfe_device *vfe, int n)
 #define RDI_WM(n)			((IS_LITE ? 0 : 23) + (n))
 #define RDI_COMP_GROUP(n)		((IS_LITE ? 0 : 11) + (n))
 
+#define MAX_VFE_OUTPUT_LINES	4
+
 static u32 vfe_hw_version(struct vfe_device *vfe)
 {
 	u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
@@ -171,12 +173,25 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,
 
 static void vfe_enable_irq_common(struct vfe_device *vfe)
 {
-	/* enable only the IRQs used: rup and comp_done irqs for RDI0 */
+	/* enable reset ack IRQ and top BUS status IRQ */
 	writel_relaxed(IRQ_MASK_0_RESET_ACK | IRQ_MASK_0_BUS_TOP_IRQ,
 		       vfe->base + VFE_IRQ_MASK(0));
-	writel_relaxed(BUS_IRQ_MASK_0_RDI_RUP(vfe, 0) |
-		       BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)),
-		       vfe->base + VFE_BUS_IRQ_MASK(0));
+}
+
+static void vfe_enable_lines_irq(struct vfe_device *vfe)
+{
+	u32 bus_irq_mask;
+	int i;
+
+	for (i = 0; i < MAX_VFE_OUTPUT_LINES; i++) {
+		/* Enable IRQ for newly added lines, but also keep already running lines's IRQ */
+		if (vfe->line[i].output.state == VFE_OUTPUT_RESERVED ||
+		    vfe->line[i].output.state == VFE_OUTPUT_ON)
+			bus_irq_mask |= BUS_IRQ_MASK_0_RDI_RUP(vfe, i)
+					| BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i));
+	}
+
+	writel_relaxed(bus_irq_mask, vfe->base + VFE_BUS_IRQ_MASK(0));
 }
 
 static void vfe_isr_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id);
@@ -193,6 +208,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 {
 	struct vfe_device *vfe = dev;
 	u32 status;
+	int i;
 
 	status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
 	writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
@@ -207,11 +223,14 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 		writel_relaxed(status, vfe->base + VFE_BUS_IRQ_CLEAR(0));
 		writel_relaxed(1, vfe->base + VFE_BUS_IRQ_CLEAR_GLOBAL);
 
-		if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, 0))
-			vfe_isr_reg_update(vfe, 0);
+		/* Loop through all WMs IRQs */
+		for (i = 0; i < MSM_VFE_IMAGE_MASTERS_NUM; i++) {
+			if (status & BUS_IRQ_MASK_0_RDI_RUP(vfe, i))
+				vfe_isr_reg_update(vfe, i);
 
-		if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(0)))
-			vfe_isr_wm_done(vfe, 0);
+			if (status & BUS_IRQ_MASK_0_COMP_DONE(vfe, RDI_COMP_GROUP(i)))
+				vfe_isr_wm_done(vfe, i);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -234,24 +253,23 @@ static int vfe_get_output(struct vfe_line *line)
 	struct vfe_device *vfe = to_vfe(line);
 	struct vfe_output *output;
 	unsigned long flags;
-	int wm_idx;
 
 	spin_lock_irqsave(&vfe->output_lock, flags);
 
 	output = &line->output;
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is running\n");
 		goto error;
 	}
 
 	output->wm_num = 1;
 
-	wm_idx = vfe_reserve_wm(vfe, line->id);
-	if (wm_idx < 0) {
-		dev_err(vfe->camss->dev, "Can not reserve wm\n");
-		goto error_get_wm;
-	}
-	output->wm_idx[0] = wm_idx;
+	/* Correspondence between VFE line number and WM number.
+	 * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
+	 * Note this 1:1 mapping will not work for PIX streams.
+	 */
+	output->wm_idx[0] = line->id;
+	vfe->wm_output_map[line->id] = line->id;
 
 	output->drop_update_idx = 0;
 
@@ -259,11 +277,9 @@ static int vfe_get_output(struct vfe_line *line)
 
 	return 0;
 
-error_get_wm:
-	vfe_release_wm(vfe, output->wm_idx[0]);
-	output->state = VFE_OUTPUT_OFF;
 error:
 	spin_unlock_irqrestore(&vfe->output_lock, flags);
+	output->state = VFE_OUTPUT_OFF;
 
 	return -EINVAL;
 }
@@ -279,7 +295,7 @@ static int vfe_enable_output(struct vfe_line *line)
 
 	vfe_reg_update_clear(vfe, line->id);
 
-	if (output->state != VFE_OUTPUT_OFF) {
+	if (output->state > VFE_OUTPUT_RESERVED) {
 		dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
 			output->state);
 		spin_unlock_irqrestore(&vfe->output_lock, flags);
@@ -360,6 +376,8 @@ static int vfe_enable(struct vfe_line *line)
 
 	vfe->stream_count++;
 
+	vfe_enable_lines_irq(vfe);
+
 	mutex_unlock(&vfe->stream_lock);
 
 	ret = vfe_get_output(line);
@@ -548,7 +566,7 @@ static const struct camss_video_ops vfe_video_ops_480 = {
 static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 {
 	vfe->video_ops = vfe_video_ops_480;
-	vfe->line_num = 1;
+	vfe->line_num = MAX_VFE_OUTPUT_LINES;
 }
 
 const struct vfe_hw_ops vfe_ops_480 = {
-- 
2.37.3