Hey Bryan,
On Thu, 11 Nov 2021 at 17:14, 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.
I think the same thing is possible for all Gen2/Titan based camss
architectures, and this fix should be enabled for CAMSS_845 too.
With that fixed, please add my r-b.
Reviewed-by: Robert Foss <robert.foss@xxxxxxxxxx>
>
> 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
>
