Re: [PATCH 11/13] media: qcom: camss: Add notify interface in camss driver

From: Bryan O'Donoghue
Date: Wed Jul 10 2024 - 07:55:20 EST

On 09/07/2024 17:06, Depeng Shao wrote:
The main v4l2 process logic in camss vfe subdev driver, so the vfe driver
handles the buf done irq and register update configuration. But the buf
done irq and register update configuration have been moved CSID HW in
Titan 780 and other new platform, so vfe driver needs to call into CSID
driver to configure the register update. And CSID driver also needs to
call into vfe driver to notify of the buf done irq.

Adding this notify interface in camss structure to do the subdevs cross
communication to decouple CSID and VFE, the subdevs can add an interface
to process the message what is routed from other subdevices, then we can
process the cross communication easily.

Signed-off-by: Depeng Shao <quic_depengs@xxxxxxxxxxx>
---
.../platform/qcom/camss/camss-csid-gen3.c | 38 +++++++++++++++
.../media/platform/qcom/camss/camss-csid.h | 8 ++++
.../media/platform/qcom/camss/camss-vfe-780.c | 29 +++++++++++-
drivers/media/platform/qcom/camss/camss-vfe.h | 1 +
drivers/media/platform/qcom/camss/camss.c | 47 +++++++++++++++++++
drivers/media/platform/qcom/camss/camss.h | 9 ++++
6 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 17fd7c5499de..585054948255 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -312,6 +312,18 @@ static u32 csid_hw_version(struct csid_device *csid)
return hw_version;
}
+static void csid_reg_update(struct csid_device *csid, int port_id)
+{
+ csid->reg_update |= REG_UPDATE_RDI(csid, port_id);
+ writel_relaxed(csid->reg_update, csid->base + CSID_REG_UPDATE_CMD);
+}
+
+static inline void csid_reg_update_clear(struct csid_device *csid,
+ int port_id)
+{
+ csid->reg_update &= ~REG_UPDATE_RDI(csid, port_id);
+}
+
/*
* csid_isr - CSID module interrupt service routine
* @irq: Interrupt line
@@ -341,6 +353,14 @@ 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 (buf_done_val & BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i)) {
+ /* For Titan 780, Buf Done IRQ&REG has been moved to CSID from VFE.
+ * Once CSID received Buf Done, need notify this event to VFE.
+ * Trigger VFE to handle Buf Done process.
+ */
+ csid->camss->notify(&csid->subdev, CAMSS_MSG_BUF_DONE, (void *)&i);

Instead of a generic notify function with an enum passed to a switch lets just have a dedicated function for each functional callback.

csid->camss->reg_update_cmd();
csid->camss->reg_clear_cmd();

We can get rid of a switch and an additional enum that way, plus redundant "event not supported" error checking.

+ }
}
val = 1 << IRQ_CMD_CLEAR;
@@ -434,6 +454,23 @@ static void csid_subdev_init(struct csid_device *csid)
csid->testgen.nmodes = CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2;
}
+static void csid_subdev_process_msg(struct csid_device *csid, unsigned int msg_type, void *arg)
+{
+ int msg_data = *(int *)arg;
+
+ switch (msg_type) {
+ case CAMSS_MSG_RUP:
+ csid_reg_update(csid, msg_data);
+ break;
+ case CAMSS_MSG_RUP_CLEAR:
+ csid_reg_update_clear(csid, msg_data);
+ break;
+ default:
+ dev_err(csid->camss->dev, "NOT Supported EVT Type\n");
+ break;
+ }
+}
+
const struct csid_hw_ops csid_ops_gen3 = {
.configure_stream = csid_configure_stream,
.configure_testgen_pattern = csid_configure_testgen_pattern,
@@ -442,4 +479,5 @@ const struct csid_hw_ops csid_ops_gen3 = {
.reset = csid_reset,
.src_pad_code = csid_src_pad_code,
.subdev_init = csid_subdev_init,
+ .process_msg = csid_subdev_process_msg,
};
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index ae5b6b0dc0ea..714a8db855fd 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -152,6 +152,14 @@ struct csid_hw_ops {
* @csid: CSID device
*/
void (*subdev_init)(struct csid_device *csid);
+
+ /*
+ * process_msg - receive message from other sub device
+ * @csid: CSID device
+ * @evt_type: event type
+ * @arg: arguments
+ */
+ void (*process_msg)(struct csid_device *csid, unsigned int evt_type, void *arg);
};
struct csid_subdev_resources {
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c
index abef2d5b9c2e..3279fe53b987 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-780.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
@@ -131,13 +131,23 @@ static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr,
static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
{
- /* TODO: Add register update support */
+ int port_id = line_id;
+
+ /* RUP(register update) registers has beem moved to CSID in Titan 780.
+ * Notify the event of trigger RUP.
+ */
+ vfe->camss->notify(&vfe->line[line_id].subdev, CAMSS_MSG_RUP, (void *)&port_id);
}
static inline void vfe_reg_update_clear(struct vfe_device *vfe,
enum vfe_line_id line_id)
{
- /* TODO: Add register update clear support */
+ int port_id = line_id;
+
+ /* RUP(register update) registers has beem moved to CSID in Titan 780.
+ * Notify the event of trigger RUP clear.
+ */
+ vfe->camss->notify(&vfe->line[line_id].subdev, CAMSS_MSG_RUP_CLEAR, (void *)&port_id);
}
/*
@@ -390,6 +400,20 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
vfe->video_ops = vfe_video_ops_780;
}
+static void vfe_subdev_process_msg(struct vfe_device *vfe, unsigned int msg_type, void *arg)
+{
+ int port_id = *(int *)arg;
+
+ switch (msg_type) {
+ case CAMSS_MSG_BUF_DONE:
+ vfe_buf_done(vfe, port_id);
+ break;
+
+ default:
+ break;
+ }
+}
+
const struct vfe_hw_ops vfe_ops_780 = {
.global_reset = NULL,
.hw_version = vfe_hw_version,
@@ -401,4 +425,5 @@ const struct vfe_hw_ops vfe_ops_780 = {
.vfe_enable = vfe_enable,
.vfe_halt = vfe_halt,
.vfe_wm_stop = vfe_wm_stop,
+ .process_msg = vfe_subdev_process_msg,
};
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index 10e2cc3c0b83..a8b09ce9941b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -115,6 +115,7 @@ struct vfe_hw_ops {
int (*vfe_halt)(struct vfe_device *vfe);
void (*violation_read)(struct vfe_device *vfe);
void (*vfe_wm_stop)(struct vfe_device *vfe, u8 wm);
+ void (*process_msg)(struct vfe_device *vfe, unsigned int msg_type, void *arg);
};
struct vfe_isr_ops {
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 1f1f44f6fbb2..abeb0918e47d 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2202,6 +2202,52 @@ static void camss_genpd_cleanup(struct camss *camss)
dev_pm_domain_detach(camss->genpd, true);
}
+static void camss_notify_msg(struct v4l2_subdev *sd,
+ unsigned int msg_type, void *arg)
+{
+ struct v4l2_device *v4l2_dev = sd->v4l2_dev;
+ struct camss *camss = to_camss(v4l2_dev);
+ struct vfe_device *vfe;
+ struct vfe_line *vfe_line;
+ struct csid_device *csid;
+ int evt_data = *(int *)arg;
+
+ switch (msg_type) {
+ case CAMSS_MSG_BUF_DONE:
+ csid = v4l2_get_subdevdata(sd);
+ vfe = &(camss->vfe[csid->id]);
+ if (vfe->res->hw_ops->process_msg)
+ vfe->res->hw_ops->process_msg(vfe,
+ CAMSS_MSG_BUF_DONE, (void *)&evt_data);
+ break;
+
+ case CAMSS_MSG_RUP:
+ vfe_line = v4l2_get_subdevdata(sd);
+ vfe = to_vfe(vfe_line);
+ csid = &(camss->csid[vfe->id]);
+
+ if (csid->res->hw_ops->process_msg)
+ csid->res->hw_ops->process_msg(csid,
+ CAMSS_MSG_RUP, (void *)&evt_data);
+ break;
+
+ case CAMSS_MSG_RUP_CLEAR:
+ vfe_line = v4l2_get_subdevdata(sd);
+ vfe = to_vfe(vfe_line);
+ csid = &(camss->csid[vfe->id]);
+
+ if (csid->res->hw_ops->process_msg)
+ csid->res->hw_ops->process_msg(csid,
+ CAMSS_MSG_RUP_CLEAR, (void *)&evt_data);
+
+ break;
+
+ default:
+ dev_err(camss->dev, "Not supported evt type\n");
+ break;
+ }
+}

Instead of having a central swiss army knife notify() callback these should be CSID or VFE specific callbacks vfe->buf_done() csid->msg_rup() etc.

---
bod