| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <236cfe43-8321-4168-8630-fb9528f581bd@linaro.org>
Date: Wed, 10 Jul 2024 12:54:58 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Depeng Shao <quic_depengs@...cinc.com>, rfoss@...nel.org,
todor.too@...il.com, mchehab@...nel.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org
Cc: quic_eberman@...cinc.com, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH 11/13] media: qcom: camss: Add notify interface in camss
driver
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@...cinc.com>
> ---
> .../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® 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
Powered by blists - more mailing lists