On Wed, 24 Nov 2021 at 18:57, Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> wrote: > > The sm8250 CAMSS CSID depends on the VFE it is attached to being powered on > and clocked prior to taking the CSID out of reset. > > It is possible to open just the CSID subdev from libcamera and attempt to > bring the CSID block up. > > If we do not first bring up the VFE the CSID will fail to come out of > reset. > > Tested-by: Julian Grahsl <jgrahsl@xxxxxxxx> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > --- > drivers/media/platform/qcom/camss/camss-csid.c | 12 +++++++++++- > drivers/media/platform/qcom/camss/camss-vfe.c | 4 ++-- > drivers/media/platform/qcom/camss/camss-vfe.h | 3 +++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > index 9ef6fbbeeddf..e6835b92695b 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.c > +++ b/drivers/media/platform/qcom/camss/camss-csid.c > @@ -156,10 +156,18 @@ static int csid_set_clock_rates(struct csid_device *csid) > static int csid_set_power(struct v4l2_subdev *sd, int on) > { > struct csid_device *csid = v4l2_get_subdevdata(sd); > - struct device *dev = csid->camss->dev; > + struct camss *camss = csid->camss; > + struct device *dev = camss->dev; > + struct vfe_device *vfe = &camss->vfe[csid->id]; > int ret; > > if (on) { > + if (camss->version == CAMSS_8250) { > + ret = vfe_get(vfe); > + if (ret < 0) > + return ret; > + } > + > ret = pm_runtime_resume_and_get(dev); > if (ret < 0) > return ret; > @@ -204,6 +212,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > camss_disable_clocks(csid->nclocks, csid->clock); > ret = csid->vdda ? regulator_disable(csid->vdda) : 0; > pm_runtime_put_sync(dev); > + if (camss->version == CAMSS_8250) > + vfe_put(vfe); > } > > return ret; > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c > index 5b5fe620914d..703ea39f1262 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c > @@ -575,7 +575,7 @@ static int vfe_check_clock_rates(struct vfe_device *vfe) > * > * Return 0 on success or a negative error code otherwise > */ > -static int vfe_get(struct vfe_device *vfe) > +int vfe_get(struct vfe_device *vfe) > { > int ret; > > @@ -637,7 +637,7 @@ static int vfe_get(struct vfe_device *vfe) > * vfe_put - Power down VFE module > * @vfe: VFE Device > */ > -static void vfe_put(struct vfe_device *vfe) > +void vfe_put(struct vfe_device *vfe) > { > mutex_lock(&vfe->power_lock); > > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h > index 6500474a749e..0eba04eb9b77 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.h > +++ b/drivers/media/platform/qcom/camss/camss-vfe.h > @@ -203,4 +203,7 @@ extern const struct vfe_hw_ops vfe_ops_4_8; > extern const struct vfe_hw_ops vfe_ops_170; > extern const struct vfe_hw_ops vfe_ops_480; > > +int vfe_get(struct vfe_device *vfe); > +void vfe_put(struct vfe_device *vfe); > + > #endif /* QC_MSM_CAMSS_VFE_H */ > -- > 2.33.0 Looks good to me! Reviewed-by: Robert Foss <robert.foss@xxxxxxxxxx>