lkml.org 
[lkml]   [2024]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 2/8] media: qcom: camss: Add subdev notify support
    From
    On 20/03/2024 14:11, Depeng Shao wrote:
    > From: Yongsheng Li <quic_yon@quicinc.com>
    >
    > The buf done irq and register update register are moved
    > to CSID in SM8550, so but the write master configuration
    > in VFE, in case adapt existing code logic. So add buf
    > done and register related subdev event, and use the notify
    > interface in the v4l2_device structure to communicate
    > between CSID and VFE driver.


    Shouldn't it be possible to just have a function to write internally ?

    You know the indexes of the CSID -> VFE connection.

    The subdev notify is I think not the right fit for this purpose within
    our driver.

    > diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
    > index fddccb69da13..4a9e5a2d1f92 100644
    > --- a/drivers/media/platform/qcom/camss/camss-csid.h
    > +++ b/drivers/media/platform/qcom/camss/camss-csid.h
    > @@ -147,6 +147,13 @@ struct csid_hw_ops {
    > * @csid: CSID device
    > */
    > void (*subdev_init)(struct csid_device *csid);
    > +
    > + /*
    > + * event - receive event from parent v4l2 device
    > + * @csid: CSID device
    > + */
    > + void (*event)(struct csid_device *csid,
    > + unsigned int evt_type, void *arg);
    > };
    >
    > struct csid_device {
    > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
    > index c9b7fe82b1f0..ffe1b95eea98 100644
    > --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
    > +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
    > @@ -61,6 +61,8 @@ struct csiphy_hw_ops {
    > void (*lanes_disable)(struct csiphy_device *csiphy,
    > struct csiphy_config *cfg);
    > irqreturn_t (*isr)(int irq, void *dev);
    > + void (*event)(struct csiphy_device *csiphy,
    > + unsigned int evt_type, void *arg);
    > };
    >
    > struct csiphy_device {
    > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
    > index 0572c9b08e11..9919fe0ff101 100644
    > --- a/drivers/media/platform/qcom/camss/camss-vfe.h
    > +++ b/drivers/media/platform/qcom/camss/camss-vfe.h
    > @@ -115,6 +115,8 @@ 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 (*event)(struct vfe_device *vfe,
    > + unsigned int evt_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 1923615f0eea..b57cd25bf6c7 100644
    > --- a/drivers/media/platform/qcom/camss/camss.c
    > +++ b/drivers/media/platform/qcom/camss/camss.c
    > @@ -1904,6 +1904,55 @@ static void camss_genpd_cleanup(struct camss *camss)
    > dev_pm_domain_detach(camss->genpd, true);
    > }
    >
    > +static void camss_v4l2_subdev_notify(struct v4l2_subdev *sd,
    > + unsigned int cmd, 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;
    > +
    > + if (camss->res->version != CAMSS_8550)
    > + return;
    > +
    > + switch (cmd) {
    > + case NOTIFY_BUF_DONE:
    > + csid = v4l2_get_subdevdata(sd);
    > + vfe = &(camss->vfe[csid->id]);
    > + if (vfe->ops->event)
    > + vfe->ops->event(vfe,
    > + NOTIFY_BUF_DONE, (void *)&evt_data);
    > + break;
    > +
    > + case NOTIFY_RUP:
    > + vfe_line = v4l2_get_subdevdata(sd);
    > + vfe = to_vfe(vfe_line);
    > + csid = &(camss->csid[vfe->id]);
    > +
    > + if (csid->ops->event)
    > + csid->ops->event(csid,
    > + NOTIFY_RUP, (void *)&evt_data);
    > + break;
    > +
    > + case NOTIFY_RUP_CLEAR:
    > + vfe_line = v4l2_get_subdevdata(sd);
    > + vfe = to_vfe(vfe_line);
    > + csid = &(camss->csid[vfe->id]);
    > +
    > + if (csid->ops->event)
    > + csid->ops->event(csid,
    > + NOTIFY_RUP_CLEAR, (void *)&evt_data);
    > +
    > + break;
    > +
    > + default:
    > + dev_err(camss->dev, "Not supported evt type\n");
    > + break;
    > + }
    > +}

    I'm really not sure I see a good reason for this.

    Why can't we just define calls between vfe and csid similar to

    drivers/media/platform/qcom/camss/camss-csid.c: ret = vfe_get(vfe);

    ---
    bod

    \
     
     \ /
      Last update: 2024-05-27 15:56    [W:5.368 / U:0.708 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site