[PATCH 00/14] Add SOF/EOF sequence logic

[PATCH 00/14] Add SOF/EOF sequence logic

[bryan.odonoghue]

This series enables SOF AND EOF event tracking making use of both events.
Whilst the code is actually very simple the functionality we want to build
off of that simple code is not.

- SOF
  Serves two functions. Firstly. Informing user-space of CSID SOF events.
  Standard Linux V4L2_EVENT_FRAME_SYNC is trigger from this event.
  User-space wants to capture this trigger because it allows user-space to
  better align the time a frame begins to events elsewhere such as IMU
  event timestamps.

  That functionality is directly applicable in this series.

  However we will also build on the SOF event in later patches to allow us
  to begin programming of shadow-registers for the next camera frame.

  When the EOF event happens, we switch from one shadow bank to the next.
  This is only possible if you have the SOF event.

  So SOF serves the immediate purpose of timestamp alignment in user-space
  as well as the 'evil plan' to trigger off of it programming camera
  parameters for the next frame.

- EOF
  EOF even on its own is useful because it allows us to increment
  vb.sequence for each frame as per the spec not just when we have
  buffer-data.

There is another purpose to having an SOF and EOF event with sequence
numbers and that is.

User-space submits a parameters buffer. Kernel accepts that parameters
buffer but can make no guarantee about _when_ the entire buffer will be
applied.

So when kernel finishes application of the entire params sequence the
returned buffer contains the sequence number - from the EOF tracking which
informs user-space _which_ frame vb.sequence its given params buffer was
applied.

Similarly when kernel produces a statistics buffer that buffer will have a
sequence number - and that sequence number will align to the frame sequence
number that the statistics describe.

Thus SOF/EOF tracking is fundamental to passing parameters, returning
statistics to user-space and for the IPA algorithm in user-space to
understand which stats, params and frames are related.

The first patch in the series addresses a corner-case when a quiescent
system can take quite a while to wake from idle, too long in fact such that
the SOF event is effectively missed.

On 'normal' systems doing lots of things, this is not a problem we would
see however on my test X1-CRD system running from ramdisk - there's
basically nothing else happening and the wake-time becomes apparent.

This is not strictly speaking a bugfix since right now nothing in CAMSS
cares about SOF or EOF so if we take an additional 2.5 ms to process a
data-buffer on a fully quiescent system, that's not the end of the world.

OTOH as soon as you start to care about SOF/EOF then missing those events
is no bueno.

The CPU latency value chosen is 100% stolen from IPU7 which seems like a
good starting point - the stealing obviously - and happens to also work.

This series is really an enabling series for more work to come but even on
its own is an improvement on the current situation.

Signed-off-by: Bryan O'Donoghue <bod@kernel.org>
---
Bryan O'Donoghue (14):
      media: qcom: camss: Introduce CPU latency QoS during streaming
      media: qcom: camss: vfe: Add eof_seq to vfe_line
      media: qcom: camss: Introduce SOF sequence tracking
      media: qcom: camss: Introduce EOF sequence tracking
      media: qcom: camss: csid-gen3: Enable RDI SOF driving
      media: qcom: camss: csid-gen3: Enable RDI EOF tracking
      media: qcom: camss: csid-680: Enable RDI SOF tracking
      media: qcom: camss: csid-680: Enable RDI EOF tracking
      media: qcom: camss: csid-gen2: Enable RDI SOF tracking
      media: qcom: camss: csid-gen2: Enable RDI EOF tracking
      media: qcom: camss: Switch on frame_sync_en logic for sm8650
      media: qcom: camss: Switch on frame_sync_en for x1e/x2e
      media: qcom: camss: Switch on frame_sync_en for sm8250
      media: qcom: camss: Switch on frame_sync_en for sdm845

 drivers/media/platform/qcom/camss/camss-csid-680.c | 21 ++++++
 .../media/platform/qcom/camss/camss-csid-gen2.c    | 19 ++++++
 .../media/platform/qcom/camss/camss-csid-gen3.c    | 31 +++++++++
 drivers/media/platform/qcom/camss/camss-csid.c     | 74 +++++++++++++++++++++-
 drivers/media/platform/qcom/camss/camss-csid.h     | 31 +++++++++
 drivers/media/platform/qcom/camss/camss-vfe-17x.c  |  5 +-
 drivers/media/platform/qcom/camss/camss-vfe.c      |  9 ++-
 drivers/media/platform/qcom/camss/camss-vfe.h      |  1 +
 drivers/media/platform/qcom/camss/camss-video.c    |  4 ++
 drivers/media/platform/qcom/camss/camss.c          | 44 +++++++++++++
 drivers/media/platform/qcom/camss/camss.h          |  7 ++
 11 files changed, 243 insertions(+), 3 deletions(-)
---
base-commit: 6d0933ae9d46674a6ea63a87e9e9ab4e662670c9
change-id: 20261029-v4l2-frame-sync-0d6fb94ab6e6

Best regards,
--  
---
bod

[PATCH 01/14] media: qcom: camss: Introduce CPU latency QoS during streaming

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

On a quiescent system - for example the X1 CRD booted to a simple ramdisk
rootfs during development of V4L2_EVENT_FRAME_SYNC it was observed that
interrupt for buffer done could be delayed until after the SOF for the next
frame.

Fiddling about with the interrupt bits, clearing them and changing the
logic of V4L2_EVENT_FRAME_SYNC to buffer completion didn't work and besides
"felt wrong".

Tracing it looks like this:
<idle>-0       [000] d.h1.    36.374380: csid_isr: rdi0 sof=1 done=0
<idle>-0       [000] d.h1.    36.406601: csid_isr: rdi0 sof=1 done=1
<idle>-0       [000] d.h1.    36.439910: csid_isr: rdi0 sof=1 done=1
<idle>-0       [000] d.h1.    36.473245: csid_isr: rdi0 sof=1 done=1
<idle>-0       [000] d.h1.    36.504786: csid_isr: rdi0 sof=0 done=1
<idle>-0       [000] d.h1.    36.506350: csid_isr: rdi0 sof=1 done=0

Tracing with /dev/cpu_dma_latency = 0 looks like this:
<idle>-0       [000] d.h1.   556.149115: csid_isr: rdi0 sof=1 done=0
<idle>-0       [000] d.h1.   556.181338: csid_isr: rdi0 sof=0 done=1
<idle>-0       [000] d.h1.   556.182880: csid_isr: rdi0 sof=1 done=0
<idle>-0       [000] d.h1.   556.214643: csid_isr: rdi0 sof=0 done=1
<idle>-0       [000] d.h1.   556.216185: csid_isr: rdi0 sof=1 done=0

The second is the pattern you expect CSI SOF followed by data.

We will require SOF to track and understand which data-frame is correlated
to which camera stastics and parameters frame. On an ordinary system with
several things pinging off at once, WiFi, display, keyboard you will be
unlikely to hit deep idle and hence unlikely to see the long exit. However
we should design for the case where the system is otherwise quiescent with
only the camera streaming.

Choosing a request of 300us - 1000us seemed to work fine but cutting the
margin finer, similar to the value on IPU6.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-video.c |  4 ++++
 drivers/media/platform/qcom/camss/camss.c       | 27 +++++++++++++++++++++++++
 drivers/media/platform/qcom/camss/camss.h       |  7 +++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f13154..191cb71d02e19 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -286,6 +286,8 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
 			goto error;
 	}
 
+	camss_cpu_latency_get(video->camss);
+
 	return 0;
 
 error:
@@ -306,6 +308,8 @@ static void video_stop_streaming(struct vb2_queue *q)
 	struct v4l2_subdev *subdev;
 	int ret;
 
+	camss_cpu_latency_put(video->camss);
+
 	entity = &vdev->entity;
 	while (1) {
 		pad = &entity->pads[0];
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index c54185c93a8a9..06e1bf82f4836 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -5064,6 +5064,29 @@ void camss_buf_done(struct camss *camss, int hw_id, int port_id)
 	}
 }
 
+#define CAMSS_CPU_LATENCY_US	300
+
+void camss_cpu_latency_get(struct camss *camss)
+{
+	mutex_lock(&camss->cpu_latency_lock);
+
+	if (camss->cpu_latency_users++ == 0)
+		cpu_latency_qos_add_request(&camss->cpu_latency,
+					    CAMSS_CPU_LATENCY_US);
+
+	mutex_unlock(&camss->cpu_latency_lock);
+}
+
+void camss_cpu_latency_put(struct camss *camss)
+{
+	mutex_lock(&camss->cpu_latency_lock);
+
+	if (--camss->cpu_latency_users == 0)
+		cpu_latency_qos_remove_request(&camss->cpu_latency);
+
+	mutex_unlock(&camss->cpu_latency_lock);
+}
+
 /*
  * camss_register_entities - Register subdev nodes and create links
  * @camss: CAMSS device
@@ -5429,6 +5452,10 @@ static int camss_probe(struct platform_device *pdev)
 	camss->dev = dev;
 	platform_set_drvdata(pdev, camss);
 
+	ret = devm_mutex_init(dev, &camss->cpu_latency_lock);
+	if (ret)
+		return ret;
+
 	camss->csiphy = devm_kcalloc(dev, camss->res->csiphy_num,
 				     sizeof(*camss->csiphy), GFP_KERNEL);
 	if (!camss->csiphy)
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 7dc2fd9f03e0d..9341fdc9bca0e 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -11,6 +11,8 @@
 #define QC_MSM_CAMSS_H
 
 #include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/pm_qos.h>
 #include <linux/types.h>
 #include <media/v4l2-async.h>
 #include <media/v4l2-device.h>
@@ -141,6 +143,9 @@ struct camss {
 	struct icc_path *icc_path[ICC_SM8250_COUNT];
 	const struct camss_resources *res;
 	bool legacy_phy;
+	struct mutex cpu_latency_lock;	/* Guards cpu_latency_users */
+	unsigned int cpu_latency_users;
+	struct pm_qos_request cpu_latency;
 };
 
 struct camss_camera_interface {
@@ -182,5 +187,7 @@ void camss_delete(struct camss *camss);
 void camss_buf_done(struct camss *camss, int hw_id, int port_id);
 void camss_reg_update(struct camss *camss, int hw_id,
 		      int port_id, bool is_clear);
+void camss_cpu_latency_get(struct camss *camss);
+void camss_cpu_latency_put(struct camss *camss);
 
 #endif /* QC_MSM_CAMSS_H */

-- 
2.56.0

[PATCH 02/14] media: qcom: camss: vfe: Add eof_seq to vfe_line

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Add sof_seq to vfe_line - this variable will be used to correlate reported
frame-numbers to EOF events when the underlying implementation supports EOF
tracking.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-vfe.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index ae9dad353a378..2d38133764b6e 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -94,6 +94,7 @@ struct vfe_line {
 	struct vfe_output output;
 	const struct camss_format_info *formats;
 	unsigned int nformats;
+	atomic_t *eof_seq;
 };
 
 struct vfe_device;

-- 
2.56.0

[PATCH 03/14] media: qcom: camss: Introduce SOF sequence tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

In order to communicate to user-space CSI2 SOF events we will need to hook
into and control v4l2_ctrl_subdev_subscribe_event. In-line with other
implementations of FRAME_SYNC have CSI decoders which support the event
always generate it in our case if and only if an atomic user count
indicates it should be so.

A flag in the CSID descriptor indicates if frame_sync_en is supported so
for each SoC that supports it this flag should be true. Since bool is false
by default no negative enumeration of existing or in-flight CSID is
required.

Subsequent patches will enable on a per-CSID basis.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid.c | 54 +++++++++++++++++++++++++-
 drivers/media/platform/qcom/camss/camss-csid.h | 12 ++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index a00791b00feb7..2f1426e77e87c 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -760,6 +760,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 static int csid_set_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	unsigned int i;
 	int ret;
 
 	if (enable) {
@@ -778,6 +779,9 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
 	}
 
 	if (csid->phy.need_vc_update) {
+		for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+			atomic_set(&csid->sof_seq[i], 0);
+
 		csid->res->hw_ops->configure_stream(csid, enable);
 		csid->phy.need_vc_update = false;
 	}
@@ -1316,9 +1320,57 @@ static int csid_link_setup(struct media_entity *entity,
 	return 0;
 }
 
+static int csid_frame_sync_add(struct v4l2_subscribed_event *sev, unsigned int elems)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(vdev_to_v4l2_subdev(sev->fh->vdev));
+
+	atomic_inc(&csid->frame_sync_users[sev->id]);
+
+	return 0;
+}
+
+static void csid_frame_sync_del(struct v4l2_subscribed_event *sev)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(vdev_to_v4l2_subdev(sev->fh->vdev));
+
+	atomic_dec(&csid->frame_sync_users[sev->id]);
+}
+
+static const struct v4l2_subscribed_event_ops csid_frame_sync_ops = {
+	.add = csid_frame_sync_add,
+	.del = csid_frame_sync_del,
+};
+
+static int csid_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+				struct v4l2_event_subscription *sub)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+
+	if (sub->type != V4L2_EVENT_FRAME_SYNC)
+		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
+
+	if (sub->id >= MSM_CSID_MAX_SRC_STREAMS || !csid->res->frame_sync_en)
+		return -EINVAL;
+
+	return v4l2_event_subscribe(fh, sub, 0, &csid_frame_sync_ops);
+}
+
+void csid_sof_event(struct csid_device *csid, unsigned int id)
+{
+	struct v4l2_event event = {
+		.type = V4L2_EVENT_FRAME_SYNC,
+		.id = id,
+		.u.frame_sync.frame_sequence =
+			atomic_fetch_inc(&csid->sof_seq[id]),
+	};
+
+	if (atomic_read(&csid->frame_sync_users[id]))
+		v4l2_event_queue(csid->subdev.devnode, &event);
+}
+
 static const struct v4l2_subdev_core_ops csid_core_ops = {
 	.s_power = csid_set_power,
-	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.subscribe_event = csid_subscribe_event,
 	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
 };
 
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 5296b10f6bac8..2e48b08b93592 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -141,6 +141,7 @@ struct csid_hw_ops {
 
 struct csid_subdev_resources {
 	bool is_lite;
+	bool frame_sync_en;
 	const struct csid_hw_ops *hw_ops;
 	const struct parent_dev_ops *parent_dev_ops;
 	const struct csid_formats *formats;
@@ -167,6 +168,8 @@ struct csid_device {
 	struct v4l2_ctrl_handler ctrls;
 	struct v4l2_ctrl *testgen_mode;
 	const struct csid_subdev_resources *res;
+	atomic_t frame_sync_users[MSM_CSID_MAX_SRC_STREAMS];
+	atomic_t sof_seq[MSM_CSID_MAX_SRC_STREAMS];
 };
 
 struct camss_subdev_resources;
@@ -248,4 +251,13 @@ u32 csid_hw_version(struct csid_device *csid);
 u32 csid_src_pad_code(struct csid_device *csid, u32 sink_code,
 		      unsigned int match_format_idx, u32 match_code);
 
+/*
+ * csid_sof_event - Track SOF generate V4L2_EVENT_FRAME_SYNC
+ * @csid: CSID device
+ * @id: CSID src-stream ID
+ *
+ *  Called from the CSID interrupt handler on SOF of the path driving the event.
+ */
+void csid_sof_event(struct csid_device *csid, unsigned int id);
+
 #endif /* QC_MSM_CAMSS_CSID_H */

-- 
2.56.0

[PATCH 04/14] media: qcom: camss: Introduce EOF sequence tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

We want to correlate statistics buffers with data-buffers. To do that we
will need to have a sequence number driven by the EOF signal.

In fact even without correlating params, stats and data the vb.sequence
should be based on EOF not just incrementing when a data-buffer arrives.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid.c    | 24 +++++++++++++++++++++--
 drivers/media/platform/qcom/camss/camss-csid.h    | 19 ++++++++++++++++++
 drivers/media/platform/qcom/camss/camss-vfe-17x.c |  5 ++++-
 drivers/media/platform/qcom/camss/camss-vfe.c     |  9 ++++++++-
 4 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 2f1426e77e87c..2564eeea01d73 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -779,9 +779,10 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
 	}
 
 	if (csid->phy.need_vc_update) {
-		for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+		for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
 			atomic_set(&csid->sof_seq[i], 0);
-
+			atomic_set(&csid->eof_seq[i], -1);
+		}
 		csid->res->hw_ops->configure_stream(csid, enable);
 		csid->phy.need_vc_update = false;
 	}
@@ -1368,6 +1369,25 @@ void csid_sof_event(struct csid_device *csid, unsigned int id)
 		v4l2_event_queue(csid->subdev.devnode, &event);
 }
 
+void csid_eof_event(struct csid_device *csid, unsigned int id)
+{
+	atomic_fetch_inc(&csid->eof_seq[id]);
+}
+
+atomic_t *csid_get_eof_seq(struct media_pad *pad)
+{
+	struct csid_device *csid;
+
+	if (!pad || pad->index < MSM_CSID_PAD_FIRST_SRC)
+		return NULL;
+
+	csid = v4l2_get_subdevdata(media_entity_to_v4l2_subdev(pad->entity));
+	if (!csid->res->frame_sync_en)
+		return NULL;
+
+	return &csid->eof_seq[pad->index - MSM_CSID_PAD_FIRST_SRC];
+}
+
 static const struct v4l2_subdev_core_ops csid_core_ops = {
 	.s_power = csid_set_power,
 	.subscribe_event = csid_subscribe_event,
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 2e48b08b93592..7abbb602805dd 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -170,6 +170,7 @@ struct csid_device {
 	const struct csid_subdev_resources *res;
 	atomic_t frame_sync_users[MSM_CSID_MAX_SRC_STREAMS];
 	atomic_t sof_seq[MSM_CSID_MAX_SRC_STREAMS];
+	atomic_t eof_seq[MSM_CSID_MAX_SRC_STREAMS];
 };
 
 struct camss_subdev_resources;
@@ -260,4 +261,22 @@ u32 csid_src_pad_code(struct csid_device *csid, u32 sink_code,
  */
 void csid_sof_event(struct csid_device *csid, unsigned int id);
 
+/*
+ * csid_eof_event - Track EOF
+ * @csid: CSID device
+ * @id: CSID src-stream ID
+ *
+ *  Called from the CSID interrupt handler on EOF of the path driving the event.
+ */
+void csid_eof_event(struct csid_device *csid, unsigned int id);
+
+/*
+ * csid_get_eof_seq - Get a pointer to the EOF frame sequence variable.
+ * @pad: Remote pad of a consumer's sink pad
+ *
+ * Return the counter or NULL if @pad isn't a CSID or the CSID doesn't support
+ * EOF tracking.
+ */
+atomic_t *csid_get_eof_seq(struct media_pad *pad);
+
 #endif /* QC_MSM_CAMSS_CSID_H */
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index 62c2405359a06..bc791a56edbab 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -491,7 +491,10 @@ static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
 	}
 
 	ready_buf->vb.vb2_buf.timestamp = ts;
-	ready_buf->vb.sequence = output->sequence++;
+	if (line->eof_seq)
+		ready_buf->vb.sequence = atomic_read(line->eof_seq);
+	else
+		ready_buf->vb.sequence = output->sequence++;
 
 	index = 0;
 	output->buf[0] = output->buf[1];
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 2680a9914765f..a4ecfb151242c 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -474,7 +474,10 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
 	}
 
 	ready_buf->vb.vb2_buf.timestamp = ts;
-	ready_buf->vb.sequence = output->sequence++;
+	if (line->eof_seq)
+		ready_buf->vb.sequence = atomic_read(line->eof_seq);
+	else
+		ready_buf->vb.sequence = output->sequence++;
 
 	index = 0;
 	output->buf[0] = output->buf[1];
@@ -508,6 +511,7 @@ int vfe_enable_output_v2(struct vfe_line *line)
 	struct vfe_output *output = &line->output;
 	const struct vfe_hw_ops *ops = vfe->res->hw_ops;
 	struct media_pad *sensor_pad;
+	struct media_pad *csid_pad;
 	unsigned long flags;
 	unsigned int frame_skip = 0;
 	unsigned int i;
@@ -523,6 +527,9 @@ int vfe_enable_output_v2(struct vfe_line *line)
 			frame_skip = VFE_FRAME_DROP_VAL - 1;
 	}
 
+	csid_pad = media_pad_remote_pad_first(&line->pads[MSM_VFE_PAD_SINK]);
+	line->eof_seq = csid_get_eof_seq(csid_pad);
+
 	spin_lock_irqsave(&vfe->output_lock, flags);
 
 	ops->reg_update_clear(vfe, line->id);

-- 
2.56.0

[PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Add in silicon enabling code to generate SOF and trigger the routine which
produces V4L2_EVENT_FRAME_SYNC based on SOF per standard v4l2 behaviour.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../media/platform/qcom/camss/camss-csid-gen3.c    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 0fdbf75fb27db..6a538d825f78b 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -34,6 +34,7 @@
 #define		CSID_RUP_AUP_RDI(rdi)		((BIT(4) | BIT(20)) << (rdi))
 
 #define CSID_TOP_IRQ_STATUS		0x7C
+#define		TOP_IRQ_RDI(rdi)		BIT(8 + (rdi))
 #define		 TOP_IRQ_STATUS_RESET_DONE	BIT(0)
 
 #define CSID_TOP_IRQ_MASK		0x80
@@ -58,6 +59,9 @@
 #define CSID_CSI2_RDIN_IRQ_STATUS(rdi)	(0xEC + 0x10 * (rdi))
 #define		RUP_DONE_IRQ_STATUS		BIT(23)
 
+#define CSID_CSI2_RDIN_IRQ_MASK(rdi)	(0xF0 + 0x10 * (rdi))
+#define		INFO_INPUT_SOF_IRQ		BIT(12)
+
 #define CSID_CSI2_RDIN_IRQ_CLEAR(rdi)	(0xF4 + 0x10 * (rdi))
 #define CSID_CSI2_RDIN_IRQ_SET(rdi)	(0xF8 + 0x10 * (rdi))
 
@@ -219,6 +223,25 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	writel(val, csid->base + CSID_RDI_CFG0(port));
 }
 
+static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi)
+{
+	u32 val = 0;
+
+	if (enable)
+		val = INFO_INPUT_SOF_IRQ;
+
+	writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
+
+	val = readl(csid->base + CSID_TOP_IRQ_MASK);
+
+	if (enable)
+		val |= TOP_IRQ_RDI(rdi);
+	else
+		val &= ~TOP_IRQ_RDI(rdi);
+
+	writel(val, csid->base + CSID_TOP_IRQ_MASK);
+}
+
 static void csid_configure_stream(struct csid_device *csid, u8 enable)
 {
 	u8 i;
@@ -230,6 +253,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 		if (csid->phy.en_vc & BIT(i)) {
 			__csid_configure_rdi_stream(csid, enable, i, 0);
 			__csid_configure_rx(csid, &csid->phy, 0);
+			__csid_configure_rdi_irq(csid, enable, i);
 			__csid_ctrl_rdi(csid, enable, i);
 		}
 }
@@ -279,6 +303,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
 			val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
 			writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
 
+			if (val & INFO_INPUT_SOF_IRQ)
+				csid_sof_event(csid, i);
+
 			if (val & RUP_DONE_IRQ_STATUS)
 				/* clear the reg update bit */
 				csid_subdev_reg_update(csid, i, true);

-- 
2.56.0

[PATCH 06/14] media: qcom: camss: csid-gen3: Enable RDI EOF tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Enable the EOF interrupt and notify csid_eof_event() to drive the
vb.sequence.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-gen3.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 6a538d825f78b..5a7a328f6c0e5 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -61,6 +61,7 @@
 
 #define CSID_CSI2_RDIN_IRQ_MASK(rdi)	(0xF0 + 0x10 * (rdi))
 #define		INFO_INPUT_SOF_IRQ		BIT(12)
+#define		INFO_INPUT_EOF_IRQ		BIT(9)
 
 #define CSID_CSI2_RDIN_IRQ_CLEAR(rdi)	(0xF4 + 0x10 * (rdi))
 #define CSID_CSI2_RDIN_IRQ_SET(rdi)	(0xF8 + 0x10 * (rdi))
@@ -228,7 +229,7 @@ static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi
 	u32 val = 0;
 
 	if (enable)
-		val = INFO_INPUT_SOF_IRQ;
+		val = INFO_INPUT_SOF_IRQ | INFO_INPUT_EOF_IRQ;
 
 	writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
 
@@ -306,6 +307,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
 			if (val & INFO_INPUT_SOF_IRQ)
 				csid_sof_event(csid, i);
 
+			if (val & INFO_INPUT_EOF_IRQ)
+				csid_eof_event(csid, i);
+
 			if (val & RUP_DONE_IRQ_STATUS)
 				/* clear the reg update bit */
 				csid_subdev_reg_update(csid, i, true);

-- 
2.56.0

[PATCH 07/14] media: qcom: camss: csid-680: Enable RDI SOF tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Similarly to the sm8650 we can quite easily switch on V4L2_EVENT_FRAME_SYNC
for x1e/x2e which uses csid-680.

Do so now.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-680.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 345a67c8fb947..452640e72f6d5 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -296,6 +296,20 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	writel(val, csid->base + CSID_RDI_CFG0(port));
 }
 
+static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi)
+{
+	u32 val;
+
+	val = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
+
+	if (enable)
+		val |= CSID_CSI2_RDIN_INFO_INPUT_SOF;
+	else
+		val &= ~CSID_CSI2_RDIN_INFO_INPUT_SOF;
+
+	writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
+}
+
 static void csid_configure_stream(struct csid_device *csid, u8 enable)
 {
 	int i;
@@ -307,6 +321,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 		if (csid->phy.en_vc & BIT(i)) {
 			__csid_configure_rdi_stream(csid, enable, i, 0);
 			__csid_configure_rx(csid, &csid->phy, 0);
+			__csid_configure_rdi_irq(csid, enable, i);
 			__csid_ctrl_rdi(csid, enable, i);
 		}
 	}
@@ -395,6 +410,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
 		if (val)
 			writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
 
+		if (val & CSID_CSI2_RDIN_INFO_INPUT_SOF)
+			csid_sof_event(csid, i);
+
 		if (val & CSID_CSI2_RDIN_RUP_DONE)
 			csid_rup_complete(csid, i);
 

-- 
2.56.0

[PATCH 08/14] media: qcom: camss: csid-680: Enable RDI EOF tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Enable the EOF interrupt and notify csid_eof_event() to drive the
vb.sequence.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-680.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 452640e72f6d5..2113ce13eb56f 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -303,9 +303,9 @@ static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi
 	val = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
 
 	if (enable)
-		val |= CSID_CSI2_RDIN_INFO_INPUT_SOF;
+		val |= CSID_CSI2_RDIN_INFO_INPUT_SOF | CSID_CSI2_RDIN_INFO_INPUT_EOF;
 	else
-		val &= ~CSID_CSI2_RDIN_INFO_INPUT_SOF;
+		val &= ~(CSID_CSI2_RDIN_INFO_INPUT_SOF | CSID_CSI2_RDIN_INFO_INPUT_EOF);
 
 	writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
 }
@@ -413,6 +413,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
 		if (val & CSID_CSI2_RDIN_INFO_INPUT_SOF)
 			csid_sof_event(csid, i);
 
+		if (val & CSID_CSI2_RDIN_INFO_INPUT_EOF)
+			csid_eof_event(csid, i);
+
 		if (val & CSID_CSI2_RDIN_RUP_DONE)
 			csid_rup_complete(csid, i);
 

-- 
2.56.0

[PATCH 09/14] media: qcom: camss: csid-gen2: Enable RDI SOF tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Add silicon specific changes to get SOF triggering V4L2_EVENT_FRAME_SYNC
'gen2' CSID. Covers SDM845, SDM670, SM6150, SM6350, SC7280, SC8280XP,
SM8250.

Tested thoroughly on sm8250 only for now. An additional patch is required
in camss.c to enable this functionality on a per-SoC basis.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sm8250
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-gen2.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index eadcb2f7e3aaa..a351ac4b91728 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -38,6 +38,8 @@
 #define CSID_CSI2_RDIN_IRQ_SET(rdi)		((csid_is_lite(csid) ? 0x3C : 0x4C) \
 						 + 0x10 * (rdi))
 
+#define		RDIN_IRQ_INFO_INPUT_SOF		BIT(12)
+
 #define CSID_TOP_IRQ_STATUS	0x70
 #define		TOP_IRQ_STATUS_RESET_DONE 0
 #define CSID_TOP_IRQ_MASK	0x74
@@ -322,6 +324,16 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
 }
 
+static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi)
+{
+	u32 val = 0;
+
+	if (enable)
+		val = RDIN_IRQ_INFO_INPUT_SOF;
+
+	writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
+}
+
 static void csid_configure_stream(struct csid_device *csid, u8 enable)
 {
 	struct csid_testgen_config *tg = &csid->testgen;
@@ -335,6 +347,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 
 			__csid_configure_rdi_stream(csid, enable, i, 0);
 			__csid_configure_rx(csid, &csid->phy, 0);
+			__csid_configure_rdi_irq(csid, enable, i);
 			__csid_ctrl_rdi(csid, enable, i);
 		}
 }
@@ -373,6 +386,8 @@ static irqreturn_t csid_isr(int irq, void *dev)
 		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));
+			if (val & RDIN_IRQ_INFO_INPUT_SOF)
+				csid_sof_event(csid, i);
 		}
 
 	val = 1 << IRQ_CMD_CLEAR;

-- 
2.56.0

[PATCH 10/14] media: qcom: camss: csid-gen2: Enable RDI EOF tracking

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Enable the EOF interrupt and notify csid_eof_event() to drive the
vb.sequence.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-gen2.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index a351ac4b91728..44f437c3f1ace 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -39,6 +39,7 @@
 						 + 0x10 * (rdi))
 
 #define		RDIN_IRQ_INFO_INPUT_SOF		BIT(12)
+#define		RDIN_IRQ_INFO_INPUT_EOF		BIT(9)
 
 #define CSID_TOP_IRQ_STATUS	0x70
 #define		TOP_IRQ_STATUS_RESET_DONE 0
@@ -329,7 +330,7 @@ static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi
 	u32 val = 0;
 
 	if (enable)
-		val = RDIN_IRQ_INFO_INPUT_SOF;
+		val = RDIN_IRQ_INFO_INPUT_SOF | RDIN_IRQ_INFO_INPUT_EOF;
 
 	writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
 }
@@ -388,6 +389,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
 			writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
 			if (val & RDIN_IRQ_INFO_INPUT_SOF)
 				csid_sof_event(csid, i);
+
+			if (val & RDIN_IRQ_INFO_INPUT_EOF)
+				csid_eof_event(csid, i);
 		}
 
 	val = 1 << IRQ_CMD_CLEAR;

-- 
2.56.0

[PATCH 11/14] media: qcom: camss: Switch on frame_sync_en logic for sm8650

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Switch frame_sync_en to true to track and consume SOF/EOF data.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sm8650-hdk
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 06e1bf82f4836..cd5a297a26970 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3433,6 +3433,7 @@ static const struct camss_subdev_resources csid_res_sm8650[] = {
 		.reg = { "csid0" },
 		.interrupt = { "csid0" },
 		.csid = {
+			.frame_sync_en = true,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.hw_ops = &csid_ops_gen3,
 			.formats = &csid_formats_gen2,
@@ -3447,6 +3448,7 @@ static const struct camss_subdev_resources csid_res_sm8650[] = {
 		.reg = { "csid1" },
 		.interrupt = { "csid1" },
 		.csid = {
+			.frame_sync_en = true,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.hw_ops = &csid_ops_gen3,
 			.formats = &csid_formats_gen2,
@@ -3461,6 +3463,7 @@ static const struct camss_subdev_resources csid_res_sm8650[] = {
 		.reg = { "csid2" },
 		.interrupt = { "csid2" },
 		.csid = {
+			.frame_sync_en = true,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.hw_ops = &csid_ops_gen3,
 			.formats = &csid_formats_gen2,
@@ -3476,6 +3479,7 @@ static const struct camss_subdev_resources csid_res_sm8650[] = {
 		.reg = { "csid_lite0" },
 		.interrupt = { "csid_lite0" },
 		.csid = {
+			.frame_sync_en = true,
 			.is_lite = true,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.hw_ops = &csid_ops_gen3,
@@ -3492,6 +3496,7 @@ static const struct camss_subdev_resources csid_res_sm8650[] = {
 		.reg = { "csid_lite1" },
 		.interrupt = { "csid_lite1" },
 		.csid = {
+			.frame_sync_en = true,
 			.is_lite = true,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.hw_ops = &csid_ops_gen3,

-- 
2.56.0

[PATCH 12/14] media: qcom: camss: Switch on frame_sync_en for x1e/x2e

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Switch frame_sync_en to true to track and consume SOF/EOF data.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e CRD
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index cd5a297a26970..10bf91b467f66 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4243,6 +4243,7 @@ static const struct camss_subdev_resources csid_res_x1e80100[] = {
 		.reg = { "csid0" },
 		.interrupt = { "csid0" },
 		.csid = {
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_680,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -4263,6 +4264,7 @@ static const struct camss_subdev_resources csid_res_x1e80100[] = {
 		.reg = { "csid1" },
 		.interrupt = { "csid1" },
 		.csid = {
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_680,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -4283,6 +4285,7 @@ static const struct camss_subdev_resources csid_res_x1e80100[] = {
 		.reg = { "csid2" },
 		.interrupt = { "csid2" },
 		.csid = {
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_680,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -4304,6 +4307,7 @@ static const struct camss_subdev_resources csid_res_x1e80100[] = {
 		.interrupt = { "csid_lite0" },
 		.csid = {
 			.is_lite = true,
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_680,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -4326,6 +4330,7 @@ static const struct camss_subdev_resources csid_res_x1e80100[] = {
 		.interrupt = { "csid_lite1" },
 		.csid = {
 			.is_lite = true,
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_680,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2

-- 
2.56.0

[PATCH 13/14] media: qcom: camss: Switch on frame_sync_en for sm8250

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Switch frame_sync_en to true to track and consume SOF/EOF data.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # rb5
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 10bf91b467f66..23f0b63580bae 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2058,6 +2058,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.reg = { "csid0" },
 		.interrupt = { "csid0" },
 		.csid = {
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2075,6 +2076,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.reg = { "csid1" },
 		.interrupt = { "csid1" },
 		.csid = {
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2092,6 +2094,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.interrupt = { "csid2" },
 		.csid = {
 			.is_lite = true,
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2109,6 +2112,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.interrupt = { "csid3" },
 		.csid = {
 			.is_lite = true,
+			.frame_sync_en = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2

-- 
2.56.0

[PATCH 14/14] media: qcom: camss: Switch on frame_sync_en for sdm845

[bryan.odonoghue]

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Switch frame_sync_en to true to track and consume SOF/EOF data.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 23f0b63580bae..1e470d7583743 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1361,6 +1361,7 @@ static const struct camss_subdev_resources csid_res_845[] = {
 		.reg = { "csid0" },
 		.interrupt = { "csid0" },
 		.csid = {
+			.frame_sync_en = 1,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -1389,6 +1390,7 @@ static const struct camss_subdev_resources csid_res_845[] = {
 		.reg = { "csid1" },
 		.interrupt = { "csid1" },
 		.csid = {
+			.frame_sync_en = 1,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -1418,6 +1420,7 @@ static const struct camss_subdev_resources csid_res_845[] = {
 		.interrupt = { "csid2" },
 		.csid = {
 			.is_lite = true,
+			.frame_sync_en = 1,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2

-- 
2.56.0

Re: [PATCH 01/14] media: qcom: camss: Introduce CPU latency QoS during streaming

[Loic Poulain]

Hi Bryan,

On Thu, Oct 29, 2026 at 10:55 AM <bryan.odonoghue@linaro.org> wrote:
>
> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> On a quiescent system - for example the X1 CRD booted to a simple ramdisk
> rootfs during development of V4L2_EVENT_FRAME_SYNC it was observed that
> interrupt for buffer done could be delayed until after the SOF for the next
> frame.
>
> Fiddling about with the interrupt bits, clearing them and changing the
> logic of V4L2_EVENT_FRAME_SYNC to buffer completion didn't work and besides
> "felt wrong".
>
> Tracing it looks like this:
> <idle>-0       [000] d.h1.    36.374380: csid_isr: rdi0 sof=1 done=0
> <idle>-0       [000] d.h1.    36.406601: csid_isr: rdi0 sof=1 done=1
> <idle>-0       [000] d.h1.    36.439910: csid_isr: rdi0 sof=1 done=1
> <idle>-0       [000] d.h1.    36.473245: csid_isr: rdi0 sof=1 done=1
> <idle>-0       [000] d.h1.    36.504786: csid_isr: rdi0 sof=0 done=1
> <idle>-0       [000] d.h1.    36.506350: csid_isr: rdi0 sof=1 done=0
>
> Tracing with /dev/cpu_dma_latency = 0 looks like this:
> <idle>-0       [000] d.h1.   556.149115: csid_isr: rdi0 sof=1 done=0
> <idle>-0       [000] d.h1.   556.181338: csid_isr: rdi0 sof=0 done=1
> <idle>-0       [000] d.h1.   556.182880: csid_isr: rdi0 sof=1 done=0
> <idle>-0       [000] d.h1.   556.214643: csid_isr: rdi0 sof=0 done=1
> <idle>-0       [000] d.h1.   556.216185: csid_isr: rdi0 sof=1 done=0
>
> The second is the pattern you expect CSI SOF followed by data.
>
> We will require SOF to track and understand which data-frame is correlated
> to which camera stastics and parameters frame. On an ordinary system with
> several things pinging off at once, WiFi, display, keyboard you will be
> unlikely to hit deep idle and hence unlikely to see the long exit. However
> we should design for the case where the system is otherwise quiescent with
> only the camera streaming.
>
> Choosing a request of 300us - 1000us seemed to work fine but cutting the
> margin finer, similar to the value on IPU6.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/camss/camss-video.c |  4 ++++
>  drivers/media/platform/qcom/camss/camss.c       | 27 +++++++++++++++++++++++++
>  drivers/media/platform/qcom/camss/camss.h       |  7 +++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 0852eb6f13154..191cb71d02e19 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -286,6 +286,8 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
>                         goto error;
>         }
>
> +       camss_cpu_latency_get(video->camss);
> +
>         return 0;
>
>  error:
> @@ -306,6 +308,8 @@ static void video_stop_streaming(struct vb2_queue *q)
>         struct v4l2_subdev *subdev;
>         int ret;
>
> +       camss_cpu_latency_put(video->camss);
> +
>         entity = &vdev->entity;
>         while (1) {
>                 pad = &entity->pads[0];
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index c54185c93a8a9..06e1bf82f4836 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -5064,6 +5064,29 @@ void camss_buf_done(struct camss *camss, int hw_id, int port_id)
>         }
>  }
>
> +#define CAMSS_CPU_LATENCY_US   300
> +
> +void camss_cpu_latency_get(struct camss *camss)
> +{
> +       mutex_lock(&camss->cpu_latency_lock);
> +
> +       if (camss->cpu_latency_users++ == 0)
> +               cpu_latency_qos_add_request(&camss->cpu_latency,
> +                                           CAMSS_CPU_LATENCY_US);
> +
> +       mutex_unlock(&camss->cpu_latency_lock);

I agree that we need such a latency request mechanism, but the current
approach requires additional logic and locking.

What about one of the following alternatives instead?
- Move the latency request handling to the suspend/resume paths. The
request lifecycle would then be naturally serialized and balanced, and
in practice there are very few cases where the device is resumed but
not actively streaming.
- Alternatively, add a dedicated latency request instance to each VFE
and let each VFE manage its own request during start/stop. That would
keep the ownership local to the VFE and avoid the need for extra
locking altogether.

Regards,
Loic

> +}
> +
> +void camss_cpu_latency_put(struct camss *camss)
> +{
> +       mutex_lock(&camss->cpu_latency_lock);
> +
> +       if (--camss->cpu_latency_users == 0)
> +               cpu_latency_qos_remove_request(&camss->cpu_latency);
> +
> +       mutex_unlock(&camss->cpu_latency_lock);
> +}
> +
>  /*
>   * camss_register_entities - Register subdev nodes and create links
>   * @camss: CAMSS device
> @@ -5429,6 +5452,10 @@ static int camss_probe(struct platform_device *pdev)
>         camss->dev = dev;
>         platform_set_drvdata(pdev, camss);
>
> +       ret = devm_mutex_init(dev, &camss->cpu_latency_lock);
> +       if (ret)
> +               return ret;
> +
>         camss->csiphy = devm_kcalloc(dev, camss->res->csiphy_num,
>                                      sizeof(*camss->csiphy), GFP_KERNEL);
>         if (!camss->csiphy)
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 7dc2fd9f03e0d..9341fdc9bca0e 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -11,6 +11,8 @@
>  #define QC_MSM_CAMSS_H
>
>  #include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_qos.h>
>  #include <linux/types.h>
>  #include <media/v4l2-async.h>
>  #include <media/v4l2-device.h>
> @@ -141,6 +143,9 @@ struct camss {
>         struct icc_path *icc_path[ICC_SM8250_COUNT];
>         const struct camss_resources *res;
>         bool legacy_phy;
> +       struct mutex cpu_latency_lock;  /* Guards cpu_latency_users */
> +       unsigned int cpu_latency_users;
> +       struct pm_qos_request cpu_latency;
>  };
>
>  struct camss_camera_interface {
> @@ -182,5 +187,7 @@ void camss_delete(struct camss *camss);
>  void camss_buf_done(struct camss *camss, int hw_id, int port_id);
>  void camss_reg_update(struct camss *camss, int hw_id,
>                       int port_id, bool is_clear);
> +void camss_cpu_latency_get(struct camss *camss);
> +void camss_cpu_latency_put(struct camss *camss);
>
>  #endif /* QC_MSM_CAMSS_H */
>
> --
> 2.56.0
>

Re: [PATCH 01/14] media: qcom: camss: Introduce CPU latency QoS during streaming

[Bryan O'Donoghue]

On 29/10/2026 10:10, Loic Poulain wrote:
>> +
>> +void camss_cpu_latency_get(struct camss *camss)
>> +{
>> +       mutex_lock(&camss->cpu_latency_lock);
>> +
>> +       if (camss->cpu_latency_users++ == 0)
>> +               cpu_latency_qos_add_request(&camss->cpu_latency,
>> +                                           CAMSS_CPU_LATENCY_US);
>> +
>> +       mutex_unlock(&camss->cpu_latency_lock);
> I agree that we need such a latency request mechanism, but the current
> approach requires additional logic and locking.
> 
> What about one of the following alternatives instead?
> - Move the latency request handling to the suspend/resume paths. The
> request lifecycle would then be naturally serialized and balanced, and
> in practice there are very few cases where the device is resumed but
> not actively streaming.
> - Alternatively, add a dedicated latency request instance to each VFE
> and let each VFE manage its own request during start/stop. That would
> keep the ownership local to the VFE and avoid the need for extra
> locking altogether.
> 
> Regards,
> Loic

yeah both of those would work.

I'll have a think about that, thx

---
bod

Re: [PATCH 00/14] Add SOF/EOF sequence logic

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

On 10/29/2026 11:55 AM, bryan.odonoghue@linaro.org wrote:
> This series enables SOF AND EOF event tracking making use of both events.
> Whilst the code is actually very simple the functionality we want to build
> off of that simple code is not.
> 
> - SOF
>   Serves two functions. Firstly. Informing user-space of CSID SOF events.
>   Standard Linux V4L2_EVENT_FRAME_SYNC is trigger from this event.
>   User-space wants to capture this trigger because it allows user-space to
>   better align the time a frame begins to events elsewhere such as IMU
>   event timestamps.
> 
>   That functionality is directly applicable in this series.
> 
>   However we will also build on the SOF event in later patches to allow us
>   to begin programming of shadow-registers for the next camera frame.
> 
>   When the EOF event happens, we switch from one shadow bank to the next.
>   This is only possible if you have the SOF event.
> 
>   So SOF serves the immediate purpose of timestamp alignment in user-space
>   as well as the 'evil plan' to trigger off of it programming camera
>   parameters for the next frame.
> 
> - EOF
>   EOF even on its own is useful because it allows us to increment
>   vb.sequence for each frame as per the spec not just when we have
>   buffer-data.
> 

It is great to finally have V4L2_EVENT_FRAME_SYNC support in CAMSS. 🙂

I would like to share my opinion on tagging vb.sequence at EOF.

The EOF event is received from the CSID decoder when the EOF short packet
is decoded. In contrast, buffer completion is reported when the write
master has finished writing the corresponding buffer.

Because the processing pipeline includes buffering and line buffers,
buf_done on the pixel path may be received after the EOF.
Conversely, when cropping is enabled on an output, buf_done may be
received before EOF.

That is why using EOF to assign the buffer sequence may result in inconsistent
sequence numbers across buffers.

I would propose:

SOF
Increment the frame sequence and generate the V4L2_EVENT_FRAME_SYNC event
for userspace.

REG_UPDATE
Once REG_UPDATE occurs, we have a guarantee that the configuration for the
new frame has been latched. At this point, we can iterate over all active
buffers being processed and assign them the current sequence number.
The bus-write active-status register indicates which address is currently being 
processed by the bus-write engine and could also be used to detect frame skips,
if necessary.

BUSWR_DONE
Complete the corresponding buffer as usual. Optionally, we can verify whether its
sequence has already been updated. If it has not, we can determine how the buffer
should be handled.

This approach ties the buffer sequence to the point at which the frame configuration
is latched, rather than to the arrival of EOF, which may not be ordered consistently
with buffer completion.

~Gjorgji

Re: [PATCH 00/14] Add SOF/EOF sequence logic

[Bryan O'Donoghue]

On 29/10/2026 13:28, Gjorgji Rosikopulos (Consultant) wrote:
> Hi Bryan,
> 
> On 10/29/2026 11:55 AM,bryan.odonoghue@linaro.org wrote:
>> This series enables SOF AND EOF event tracking making use of both events.
>> Whilst the code is actually very simple the functionality we want to build
>> off of that simple code is not.
>>
>> - SOF
>>    Serves two functions. Firstly. Informing user-space of CSID SOF events.
>>    Standard Linux V4L2_EVENT_FRAME_SYNC is trigger from this event.
>>    User-space wants to capture this trigger because it allows user-space to
>>    better align the time a frame begins to events elsewhere such as IMU
>>    event timestamps.
>>
>>    That functionality is directly applicable in this series.
>>
>>    However we will also build on the SOF event in later patches to allow us
>>    to begin programming of shadow-registers for the next camera frame.
>>
>>    When the EOF event happens, we switch from one shadow bank to the next.
>>    This is only possible if you have the SOF event.
>>
>>    So SOF serves the immediate purpose of timestamp alignment in user-space
>>    as well as the 'evil plan' to trigger off of it programming camera
>>    parameters for the next frame.
>>
>> - EOF
>>    EOF even on its own is useful because it allows us to increment
>>    vb.sequence for each frame as per the spec not just when we have
>>    buffer-data.
>>
> It is great to finally have V4L2_EVENT_FRAME_SYNC support in CAMSS. 🙂
> 
> I would like to share my opinion on tagging vb.sequence at EOF.
> 
> The EOF event is received from the CSID decoder when the EOF short packet
> is decoded. In contrast, buffer completion is reported when the write
> master has finished writing the corresponding buffer.
> 
> Because the processing pipeline includes buffering and line buffers,
> buf_done on the pixel path may be received after the EOF.
> Conversely, when cropping is enabled on an output, buf_done may be
> received before EOF.
> 
> That is why using EOF to assign the buffer sequence may result in inconsistent
> sequence numbers across buffers.
> 
> I would propose:
> 
> SOF
> Increment the frame sequence and generate the V4L2_EVENT_FRAME_SYNC event
> for userspace.
> 
> REG_UPDATE
> Once REG_UPDATE occurs, we have a guarantee that the configuration for the
> new frame has been latched. At this point, we can iterate over all active
> buffers being processed and assign them the current sequence number.
> The bus-write active-status register indicates which address is currently being
> processed by the bus-write engine and could also be used to detect frame skips,
> if necessary.
> 
> BUSWR_DONE
> Complete the corresponding buffer as usual. Optionally, we can verify whether its
> sequence has already been updated. If it has not, we can determine how the buffer
> should be handled.
> 
> This approach ties the buffer sequence to the point at which the frame configuration
> is latched, rather than to the arrival of EOF, which may not be ordered consistently
> with buffer completion.

Come to think of it, I'm not entirely sure an EOF or reg-update 
additional variable is required.

SOF.seq should do.

- Reading SOF.seq @ crop is fine because the SOF.seq always preceds
- Reading SOF.seq @ buf_done is fine ditto
- Reading SOF.seq for params return stamping is correct
- Reading SOF.seq for stats frame correlation is also correct

We want the EOF signal but we don't need a second seqno to accompany. 
And with one source of seq truth we don't need to change the point at 
which we latch since pre-frame-drop SOF < everythign else.

---
bod

Re: [PATCH 00/14] Add SOF/EOF sequence logic

[Gjorgji Rosikopulos (Consultant)]

On 10/29/2026 4:44 PM, Bryan O'Donoghue wrote:
> On 29/10/2026 13:28, Gjorgji Rosikopulos (Consultant) wrote:
>> Hi Bryan,
>>
>> On 10/29/2026 11:55 AM,bryan.odonoghue@linaro.org wrote:
>>> This series enables SOF AND EOF event tracking making use of both events.
>>> Whilst the code is actually very simple the functionality we want to build
>>> off of that simple code is not.
>>>
>>> - SOF
>>>    Serves two functions. Firstly. Informing user-space of CSID SOF events.
>>>    Standard Linux V4L2_EVENT_FRAME_SYNC is trigger from this event.
>>>    User-space wants to capture this trigger because it allows user-space to
>>>    better align the time a frame begins to events elsewhere such as IMU
>>>    event timestamps.
>>>
>>>    That functionality is directly applicable in this series.
>>>
>>>    However we will also build on the SOF event in later patches to allow us
>>>    to begin programming of shadow-registers for the next camera frame.
>>>
>>>    When the EOF event happens, we switch from one shadow bank to the next.
>>>    This is only possible if you have the SOF event.
>>>
>>>    So SOF serves the immediate purpose of timestamp alignment in user-space
>>>    as well as the 'evil plan' to trigger off of it programming camera
>>>    parameters for the next frame.
>>>
>>> - EOF
>>>    EOF even on its own is useful because it allows us to increment
>>>    vb.sequence for each frame as per the spec not just when we have
>>>    buffer-data.
>>>
>> It is great to finally have V4L2_EVENT_FRAME_SYNC support in CAMSS. 🙂
>>
>> I would like to share my opinion on tagging vb.sequence at EOF.
>>
>> The EOF event is received from the CSID decoder when the EOF short packet
>> is decoded. In contrast, buffer completion is reported when the write
>> master has finished writing the corresponding buffer.
>>
>> Because the processing pipeline includes buffering and line buffers,
>> buf_done on the pixel path may be received after the EOF.
>> Conversely, when cropping is enabled on an output, buf_done may be
>> received before EOF.
>>
>> That is why using EOF to assign the buffer sequence may result in inconsistent
>> sequence numbers across buffers.
>>
>> I would propose:
>>
>> SOF
>> Increment the frame sequence and generate the V4L2_EVENT_FRAME_SYNC event
>> for userspace.
>>
>> REG_UPDATE
>> Once REG_UPDATE occurs, we have a guarantee that the configuration for the
>> new frame has been latched. At this point, we can iterate over all active
>> buffers being processed and assign them the current sequence number.
>> The bus-write active-status register indicates which address is currently being
>> processed by the bus-write engine and could also be used to detect frame skips,
>> if necessary.
>>
>> BUSWR_DONE
>> Complete the corresponding buffer as usual. Optionally, we can verify whether its
>> sequence has already been updated. If it has not, we can determine how the buffer
>> should be handled.
>>
>> This approach ties the buffer sequence to the point at which the frame configuration
>> is latched, rather than to the arrival of EOF, which may not be ordered consistently
>> with buffer completion.
> 
> Come to think of it, I'm not entirely sure an EOF or reg-update 
> additional variable is required.
> 
> SOF.seq should do.
> 
> - Reading SOF.seq @ crop is fine because the SOF.seq always preceds
> - Reading SOF.seq @ buf_done is fine ditto
> - Reading SOF.seq for params return stamping is correct
> - Reading SOF.seq for stats frame correlation is also correct
> 
> We want the EOF signal but we don't need a second seqno to accompany. 
> And with one source of seq truth we don't need to change the point at 
> which we latch since pre-frame-drop SOF < everythign else.

I agree. REG_UPDATE is only necessary if we decide to use the bus-write active-status
register to determine which buffer is currently being processed. This can give real guarantee
that we are not missing something on heavily loaded systems when irq-s were disabled for some time,
in that case sequence may not be in sync anyway...

~Gjorgji

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Loic Poulain]

On Thu, Oct 29, 2026 at 10:55 AM <bryan.odonoghue@linaro.org> wrote:
>
> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> Add in silicon enabling code to generate SOF and trigger the routine which
> produces V4L2_EVENT_FRAME_SYNC based on SOF per standard v4l2 behaviour.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../media/platform/qcom/camss/camss-csid-gen3.c    | 27 ++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> index 0fdbf75fb27db..6a538d825f78b 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> @@ -34,6 +34,7 @@
>  #define                CSID_RUP_AUP_RDI(rdi)           ((BIT(4) | BIT(20)) << (rdi))
>
>  #define CSID_TOP_IRQ_STATUS            0x7C
> +#define                TOP_IRQ_RDI(rdi)                BIT(8 + (rdi))
>  #define                 TOP_IRQ_STATUS_RESET_DONE      BIT(0)
>
>  #define CSID_TOP_IRQ_MASK              0x80
> @@ -58,6 +59,9 @@
>  #define CSID_CSI2_RDIN_IRQ_STATUS(rdi) (0xEC + 0x10 * (rdi))
>  #define                RUP_DONE_IRQ_STATUS             BIT(23)
>
> +#define CSID_CSI2_RDIN_IRQ_MASK(rdi)   (0xF0 + 0x10 * (rdi))
> +#define                INFO_INPUT_SOF_IRQ              BIT(12)
> +
>  #define CSID_CSI2_RDIN_IRQ_CLEAR(rdi)  (0xF4 + 0x10 * (rdi))
>  #define CSID_CSI2_RDIN_IRQ_SET(rdi)    (0xF8 + 0x10 * (rdi))
>
> @@ -219,6 +223,25 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>         writel(val, csid->base + CSID_RDI_CFG0(port));
>  }
>
> +static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi)
> +{
> +       u32 val = 0;
> +
> +       if (enable)
> +               val = INFO_INPUT_SOF_IRQ;
> +
> +       writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
> +
> +       val = readl(csid->base + CSID_TOP_IRQ_MASK);
> +
> +       if (enable)
> +               val |= TOP_IRQ_RDI(rdi);
> +       else
> +               val &= ~TOP_IRQ_RDI(rdi);
> +
> +       writel(val, csid->base + CSID_TOP_IRQ_MASK);
> +}
> +
>  static void csid_configure_stream(struct csid_device *csid, u8 enable)
>  {
>         u8 i;
> @@ -230,6 +253,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>                 if (csid->phy.en_vc & BIT(i)) {
>                         __csid_configure_rdi_stream(csid, enable, i, 0);
>                         __csid_configure_rx(csid, &csid->phy, 0);
> +                       __csid_configure_rdi_irq(csid, enable, i);

Is this something that should also be tied to frame_sync_en?

Also, just thinking out loud, are there cases where we'd want to
toggle this dynamically at runtime based on any V4L2 configuration or
pipeline path? For example, should it be enabled for the pixel path
but disabled for RDI?

I'm asking because one of the appreciated strengths of CAMSS is its
relatively low-latency and lightweight capture path. As we add more
features, it may be worth preserving that characteristic where
possible. Not saying this particular feature has a meaningful latency
or compute cost, but more as a general design consideration.

Regards,
Loic

>                         __csid_ctrl_rdi(csid, enable, i);
>                 }
>  }
> @@ -279,6 +303,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
>                         val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
>                         writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
>
> +                       if (val & INFO_INPUT_SOF_IRQ)
> +                               csid_sof_event(csid, i);
> +
>                         if (val & RUP_DONE_IRQ_STATUS)
>                                 /* clear the reg update bit */
>                                 csid_subdev_reg_update(csid, i, true);
>
> --
> 2.56.0
>

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Loic Poulain]

On Thu, Oct 29, 2026 at 4:43 PM Loic Poulain
<loic.poulain@oss.qualcomm.com> wrote:
>
> On Thu, Oct 29, 2026 at 10:55 AM <bryan.odonoghue@linaro.org> wrote:
> >
> > From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >
> > Add in silicon enabling code to generate SOF and trigger the routine which
> > produces V4L2_EVENT_FRAME_SYNC based on SOF per standard v4l2 behaviour.
> >
> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > ---
> >  .../media/platform/qcom/camss/camss-csid-gen3.c    | 27 ++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> > index 0fdbf75fb27db..6a538d825f78b 100644
> > --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> > +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> > @@ -34,6 +34,7 @@
> >  #define                CSID_RUP_AUP_RDI(rdi)           ((BIT(4) | BIT(20)) << (rdi))
> >
> >  #define CSID_TOP_IRQ_STATUS            0x7C
> > +#define                TOP_IRQ_RDI(rdi)                BIT(8 + (rdi))
> >  #define                 TOP_IRQ_STATUS_RESET_DONE      BIT(0)
> >
> >  #define CSID_TOP_IRQ_MASK              0x80
> > @@ -58,6 +59,9 @@
> >  #define CSID_CSI2_RDIN_IRQ_STATUS(rdi) (0xEC + 0x10 * (rdi))
> >  #define                RUP_DONE_IRQ_STATUS             BIT(23)
> >
> > +#define CSID_CSI2_RDIN_IRQ_MASK(rdi)   (0xF0 + 0x10 * (rdi))
> > +#define                INFO_INPUT_SOF_IRQ              BIT(12)
> > +
> >  #define CSID_CSI2_RDIN_IRQ_CLEAR(rdi)  (0xF4 + 0x10 * (rdi))
> >  #define CSID_CSI2_RDIN_IRQ_SET(rdi)    (0xF8 + 0x10 * (rdi))
> >
> > @@ -219,6 +223,25 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
> >         writel(val, csid->base + CSID_RDI_CFG0(port));
> >  }
> >
> > +static void __csid_configure_rdi_irq(struct csid_device *csid, u8 enable, u8 rdi)
> > +{
> > +       u32 val = 0;
> > +
> > +       if (enable)
> > +               val = INFO_INPUT_SOF_IRQ;
> > +
> > +       writel(val, csid->base + CSID_CSI2_RDIN_IRQ_MASK(rdi));
> > +
> > +       val = readl(csid->base + CSID_TOP_IRQ_MASK);
> > +
> > +       if (enable)
> > +               val |= TOP_IRQ_RDI(rdi);
> > +       else
> > +               val &= ~TOP_IRQ_RDI(rdi);
> > +
> > +       writel(val, csid->base + CSID_TOP_IRQ_MASK);
> > +}
> > +
> >  static void csid_configure_stream(struct csid_device *csid, u8 enable)
> >  {
> >         u8 i;
> > @@ -230,6 +253,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
> >                 if (csid->phy.en_vc & BIT(i)) {
> >                         __csid_configure_rdi_stream(csid, enable, i, 0);
> >                         __csid_configure_rx(csid, &csid->phy, 0);
> > +                       __csid_configure_rdi_irq(csid, enable, i);
>
> Is this something that should also be tied to frame_sync_en?
>
> Also, just thinking out loud, are there cases where we'd want to
> toggle this dynamically at runtime based on any V4L2 configuration or
> pipeline path? For example, should it be enabled for the pixel path
> but disabled for RDI?

Obviously, if userspace subscribes to the related framesync event, we
should enable it regardless.

>
> I'm asking because one of the appreciated strengths of CAMSS is its
> relatively low-latency and lightweight capture path. As we add more
> features, it may be worth preserving that characteristic where
> possible. Not saying this particular feature has a meaningful latency
> or compute cost, but more as a general design consideration.
>
> Regards,
> Loic
>
> >                         __csid_ctrl_rdi(csid, enable, i);
> >                 }
> >  }
> > @@ -279,6 +303,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
> >                         val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
> >                         writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
> >
> > +                       if (val & INFO_INPUT_SOF_IRQ)
> > +                               csid_sof_event(csid, i);
> > +
> >                         if (val & RUP_DONE_IRQ_STATUS)
> >                                 /* clear the reg update bit */
> >                                 csid_subdev_reg_update(csid, i, true);
> >
> > --
> > 2.56.0
> >

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Bryan O'Donoghue]

On 29/10/2026 15:43, Loic Poulain wrote:
>>                          __csid_configure_rx(csid, &csid->phy, 0);
>> +                       __csid_configure_rdi_irq(csid, enable, i);
> Is this something that should also be tied to frame_sync_en?
> 
> Also, just thinking out loud, are there cases where we'd want to
> toggle this dynamically at runtime based on any V4L2 configuration or
> pipeline path? For example, should it be enabled for the pixel path
> but disabled for RDI?
> 
> I'm asking because one of the appreciated strengths of CAMSS is its
> relatively low-latency and lightweight capture path. As we add more
> features, it may be worth preserving that characteristic where
> possible. Not saying this particular feature has a meaningful latency
> or compute cost, but more as a general design consideration.
> 
> Regards,
> Loic
> 
>>                          __csid_ctrl_rdi(csid, enable, i);
>>                  }
>>   }
>> @@ -279,6 +303,9 @@ static irqreturn_t csid_isr(int irq, void *dev)
>>                          val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
>>                          writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
>>
>> +                       if (val & INFO_INPUT_SOF_IRQ)
>> +                               csid_sof_event(csid, i);

I considered dynamic enable but

1. The design here is to move ~ everything to SOF vb.sequence
2. Standard design pattern with FRAME_SYNC is to generate the event
    having consumers either trigger on it or not

For that second case I opted to only send the signal when we know 
there's a consumer though

=>

+	if (atomic_read(&csid->frame_sync_users[id]))
+		v4l2_event_queue(csid->subdev.devnode, &event);

---
bod

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Bryan O'Donoghue]

On 29/10/2026 15:46, Loic Poulain wrote:
>> Is this something that should also be tied to frame_sync_en?
>>
>> Also, just thinking out loud, are there cases where we'd want to
>> toggle this dynamically at runtime based on any V4L2 configuration or
>> pipeline path? For example, should it be enabled for the pixel path
>> but disabled for RDI?
> Obviously, if userspace subscribes to the related framesync event, we
> should enable it regardless.

That's the behaviour already :)

Again though we want SOF so that we can drive seqno which really 
_should_ come from the CSID signal not from buf_done() *

---
bod

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Gjorgji Rosikopulos (Consultant)]

On 10/29/2026 5:49 PM, Bryan O'Donoghue wrote:
> On 29/10/2026 15:46, Loic Poulain wrote:
>>> Is this something that should also be tied to frame_sync_en?
>>>
>>> Also, just thinking out loud, are there cases where we'd want to
>>> toggle this dynamically at runtime based on any V4L2 configuration or
>>> pipeline path? For example, should it be enabled for the pixel path
>>> but disabled for RDI?
>> Obviously, if userspace subscribes to the related framesync event, we
>> should enable it regardless.
> 
> That's the behaviour already :)
> 
> Again though we want SOF so that we can drive seqno which really 
> _should_ come from the CSID signal not from buf_done() *

My opinion is if userspace does not need on SOF events, the buffer
sequence may not be  important. However, for use cases that require
bundling and matching frames from different outputs such as DOL,
it makes sense to tag them with the same sequence number.

Therefore, if this feature is enabled/disabled if userspace
subscribes to the frame sync event, this behavior should be clearly
documented.

~Gjorgji

> 
> ---
> bod

Re: [PATCH 05/14] media: qcom: camss: csid-gen3: Enable RDI SOF driving

[Loic Poulain]

On Thu, Oct 29, 2026 at 5:25 PM Gjorgji Rosikopulos (Consultant)
<gjorgji.rosikopulos@oss.qualcomm.com> wrote:
>
>
>
> On 10/29/2026 5:49 PM, Bryan O'Donoghue wrote:
> > On 29/10/2026 15:46, Loic Poulain wrote:
> >>> Is this something that should also be tied to frame_sync_en?
> >>>
> >>> Also, just thinking out loud, are there cases where we'd want to
> >>> toggle this dynamically at runtime based on any V4L2 configuration or
> >>> pipeline path? For example, should it be enabled for the pixel path
> >>> but disabled for RDI?
> >> Obviously, if userspace subscribes to the related framesync event, we
> >> should enable it regardless.
> >
> > That's the behaviour already :)
> >
> > Again though we want SOF so that we can drive seqno which really
> > _should_ come from the CSID signal not from buf_done() *
>
> My opinion is if userspace does not need on SOF events, the buffer
> sequence may not be  important. However, for use cases that require
> bundling and matching frames from different outputs such as DOL,
> it makes sense to tag them with the same sequence number.
>
> Therefore, if this feature is enabled/disabled if userspace
> subscribes to the frame sync event, this behavior should be clearly
> documented.

Ok then probably better to have a consistent behavior and always
leverage the SOF for sequencing when available.

Regards,
Loic