[PATCH 0/2] media: camss: Simplify and improve power management of VFEs

[PATCH 0/2] media: camss: Simplify and improve power management of VFEs

[Vladimir Zapolskiy]

Since a rework of CAMSS power domain management for newer platforms done in
commit 2f6f8af67203 ("media: camss: Refactor VFE power domain toggling"),
all operations over CAMSS imply enablement of all power domains described
in the correspondent device tree node. Apparently it's too excessive and it's
quite a complicated scheme to allow simple addition of newer platforms with
even more power domains.

I would appreciate, if somebody can test the changes on db820 for any probable
regressions.

The change is based on changes in clock framework [1] and on a recent fix-up [2]
in camss.

[1] https://lore.kernel.org/linux-clk/20220519214133.1728979-1-vladimir.zapolskiy@linaro.org/
[2] https://lore.kernel.org/linux-media/20220704220814.629130-1-vladimir.zapolskiy@linaro.org/

Vladimir Zapolskiy (2):
  media: camss: Collect information about a number of lite VFEs
  media: camss: Split power domain management

 .../media/platform/qcom/camss/camss-vfe-170.c | 20 +++++++-
 .../media/platform/qcom/camss/camss-vfe-480.c | 20 +++++++-
 drivers/media/platform/qcom/camss/camss.c     | 50 ++++++++++---------
 drivers/media/platform/qcom/camss/camss.h     |  1 +
 4 files changed, 66 insertions(+), 25 deletions(-)

-- 
2.33.0

[PATCH 1/2] media: camss: Collect information about a number of lite VFEs

[Vladimir Zapolskiy]

VFE lite IPs are found on CAMSS with TITAN_TOP power domains, and in
some aspects these types of VFEs are different, in particular there
is no need to enable VFE power domains to operate over VFE lite IPs.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 20 +++++++++++---------
 drivers/media/platform/qcom/camss/camss.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 7a929f19e79b..795eebd9af6c 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1170,7 +1170,7 @@ static int camss_init_subdevices(struct camss *camss)
 	}
 
 	/* note: SM8250 requires VFE to be initialized before CSID */
-	for (i = 0; i < camss->vfe_num; i++) {
+	for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++) {
 		ret = msm_vfe_subdev_init(camss, &camss->vfe[i],
 					  &vfe_res[i], i);
 		if (ret < 0) {
@@ -1242,7 +1242,7 @@ static int camss_register_entities(struct camss *camss)
 		goto err_reg_ispif;
 	}
 
-	for (i = 0; i < camss->vfe_num; i++) {
+	for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++) {
 		ret = msm_vfe_register_entities(&camss->vfe[i],
 						&camss->v4l2_dev);
 		if (ret < 0) {
@@ -1314,7 +1314,7 @@ static int camss_register_entities(struct camss *camss)
 				}
 	} else {
 		for (i = 0; i < camss->csid_num; i++)
-			for (k = 0; k < camss->vfe_num; k++)
+			for (k = 0; k < camss->vfe_num + camss->vfe_lite_num; k++)
 				for (j = 0; j < camss->vfe[k].line_num; j++) {
 					struct v4l2_subdev *csid = &camss->csid[i].subdev;
 					struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
@@ -1338,7 +1338,7 @@ static int camss_register_entities(struct camss *camss)
 	return 0;
 
 err_link:
-	i = camss->vfe_num;
+	i = camss->vfe_num + camss->vfe_lite_num;
 err_reg_vfe:
 	for (i--; i >= 0; i--)
 		msm_vfe_unregister_entities(&camss->vfe[i]);
@@ -1377,7 +1377,7 @@ static void camss_unregister_entities(struct camss *camss)
 
 	msm_ispif_unregister_entities(camss->ispif);
 
-	for (i = 0; i < camss->vfe_num; i++)
+	for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++)
 		msm_vfe_unregister_entities(&camss->vfe[i]);
 }
 
@@ -1579,13 +1579,15 @@ static int camss_probe(struct platform_device *pdev)
 		camss->version = CAMSS_845;
 		camss->csiphy_num = 4;
 		camss->csid_num = 3;
-		camss->vfe_num = 3;
+		camss->vfe_num = 2;
+		camss->vfe_lite_num = 1;
 	} else if (of_device_is_compatible(dev->of_node,
 					   "qcom,sm8250-camss")) {
 		camss->version = CAMSS_8250;
 		camss->csiphy_num = 6;
 		camss->csid_num = 4;
-		camss->vfe_num = 4;
+		camss->vfe_num = 2;
+		camss->vfe_lite_num = 2;
 	} else {
 		return -EINVAL;
 	}
@@ -1607,8 +1609,8 @@ static int camss_probe(struct platform_device *pdev)
 			return -ENOMEM;
 	}
 
-	camss->vfe = devm_kcalloc(dev, camss->vfe_num, sizeof(*camss->vfe),
-				  GFP_KERNEL);
+	camss->vfe = devm_kcalloc(dev, camss->vfe_num + camss->vfe_lite_num,
+				  sizeof(*camss->vfe), GFP_KERNEL);
 	if (!camss->vfe)
 		return -ENOMEM;
 
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 0db80cadbbaa..3acd2b3403e8 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -97,6 +97,7 @@ struct camss {
 	struct csid_device *csid;
 	struct ispif_device *ispif;
 	int vfe_num;
+	int vfe_lite_num;
 	struct vfe_device *vfe;
 	atomic_t ref_count;
 	int genpd_num;
-- 
2.33.0

[PATCH 2/2] media: camss: Split power domain management

[Vladimir Zapolskiy]

There are three cases of power domain management on supported platforms:
1) CAMSS on MSM8916, where a single VFE power domain is operated outside
   of the camss device driver,
2) CAMSS on MSM8996 and SDM630/SDM660, where two VFE power domains are
   managed separately by the camss device driver, the power domains are
   linked and unlinked on demand by their functions vfe_pm_domain_on()
   and vfe_pm_domain_off() respectively,
3) CAMSS on SDM845 and SM8250 platforms, and there are two VFE power
   domains and their parent power domain TITAN_TOP, the latter one
   shall be turned on prior to turning on any of VFE power domains.

Due to a previously missing link between TITAN_TOP and VFEx power domains
in the latter case, which is now fixed by [1], it was decided always to
turn on all found VFE power domains and TITAN_TOP power domain, even if
just one particular VFE is needed to be enabled or none of VFE power domains
are required, for instance the latter case is when vfe_lite is in use.
This misusage becomes more incovenient and clumsy, if next generations are
to be supported, for instance CAMSS on SM8450 has three VFE power domains.

The change splits the power management support for platforms with TITAN_TOP
parent power domain, and, since 'power-domain-names' property is not present
in camss device tree nodes, the assumption is that the first N power domains
from the 'power-domains' list correspond to VFE power domains, and, if the
number of power domains is greater than number of non-lite VFEs, then the
last power domain from the list is the TITAN_TOP power domain.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../media/platform/qcom/camss/camss-vfe-170.c | 20 ++++++++++++-
 .../media/platform/qcom/camss/camss-vfe-480.c | 20 ++++++++++++-
 drivers/media/platform/qcom/camss/camss.c     | 30 ++++++++++---------
 3 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
index 600150cfc4f7..8e506a805d11 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
@@ -687,7 +687,12 @@ static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
  */
 static void vfe_pm_domain_off(struct vfe_device *vfe)
 {
-	/* nop */
+	struct camss *camss = vfe->camss;
+
+	if (vfe->id >= camss->vfe_num)
+		return;
+
+	device_link_del(camss->genpd_link[vfe->id]);
 }
 
 /*
@@ -696,6 +701,19 @@ static void vfe_pm_domain_off(struct vfe_device *vfe)
  */
 static int vfe_pm_domain_on(struct vfe_device *vfe)
 {
+	struct camss *camss = vfe->camss;
+	enum vfe_line_id id = vfe->id;
+
+	if (id >= camss->vfe_num)
+		return 0;
+
+	camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id],
+						DL_FLAG_STATELESS |
+						DL_FLAG_PM_RUNTIME |
+						DL_FLAG_RPM_ACTIVE);
+	if (!camss->genpd_link[id])
+		return -EINVAL;
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
index 129585110393..3aa962b5663b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
@@ -494,7 +494,12 @@ static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
  */
 static void vfe_pm_domain_off(struct vfe_device *vfe)
 {
-	/* nop */
+	struct camss *camss = vfe->camss;
+
+	if (vfe->id >= camss->vfe_num)
+		return;
+
+	device_link_del(camss->genpd_link[vfe->id]);
 }
 
 /*
@@ -503,6 +508,19 @@ static void vfe_pm_domain_off(struct vfe_device *vfe)
  */
 static int vfe_pm_domain_on(struct vfe_device *vfe)
 {
+	struct camss *camss = vfe->camss;
+	enum vfe_line_id id = vfe->id;
+
+	if (id >= camss->vfe_num)
+		return 0;
+
+	camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id],
+						DL_FLAG_STATELESS |
+						DL_FLAG_PM_RUNTIME |
+						DL_FLAG_RPM_ACTIVE);
+	if (!camss->genpd_link[id])
+		return -EINVAL;
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 795eebd9af6c..f009297ba182 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1453,7 +1453,6 @@ static const struct media_device_ops camss_media_ops = {
 static int camss_configure_pd(struct camss *camss)
 {
 	struct device *dev = camss->dev;
-	int last_pm_domain = 0;
 	int i;
 	int ret;
 
@@ -1484,32 +1483,34 @@ static int camss_configure_pd(struct camss *camss)
 	if (!camss->genpd_link)
 		return -ENOMEM;
 
+	/*
+	 * VFE power domains are in the beginning of the list, and while all
+	 * power domains should be attached, only if TITAN_TOP power domain is
+	 * found in the list, it should be linked over here.
+	 */
 	for (i = 0; i < camss->genpd_num; i++) {
 		camss->genpd[i] = dev_pm_domain_attach_by_id(camss->dev, i);
 		if (IS_ERR(camss->genpd[i])) {
 			ret = PTR_ERR(camss->genpd[i]);
 			goto fail_pm;
 		}
+	}
 
-		camss->genpd_link[i] = device_link_add(camss->dev, camss->genpd[i],
-						       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
-						       DL_FLAG_RPM_ACTIVE);
-		if (!camss->genpd_link[i]) {
-			dev_pm_domain_detach(camss->genpd[i], true);
+	if (i > camss->vfe_num) {
+		camss->genpd_link[i - 1] = device_link_add(camss->dev, camss->genpd[i - 1],
+							   DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
+							   DL_FLAG_RPM_ACTIVE);
+		if (!camss->genpd_link[i - 1]) {
 			ret = -EINVAL;
 			goto fail_pm;
 		}
-
-		last_pm_domain = i;
 	}
 
 	return 0;
 
 fail_pm:
-	for (i = 0; i < last_pm_domain; i++) {
-		device_link_del(camss->genpd_link[i]);
+	for (--i ; i >= 0; i--)
 		dev_pm_domain_detach(camss->genpd[i], true);
-	}
 
 	return ret;
 }
@@ -1711,10 +1712,11 @@ void camss_delete(struct camss *camss)
 	if (camss->genpd_num == 1)
 		return;
 
-	for (i = 0; i < camss->genpd_num; i++) {
-		device_link_del(camss->genpd_link[i]);
+	if (camss->genpd_num > camss->vfe_num)
+		device_link_del(camss->genpd_link[camss->genpd_num - 1]);
+
+	for (i = 0; i < camss->genpd_num; i++)
 		dev_pm_domain_detach(camss->genpd[i], true);
-	}
 }
 
 /*
-- 
2.33.0

Re: [PATCH 2/2] media: camss: Split power domain management

[Robert Foss]

On Tue, 5 Jul 2022 at 00:15, Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> There are three cases of power domain management on supported platforms:
> 1) CAMSS on MSM8916, where a single VFE power domain is operated outside
>    of the camss device driver,
> 2) CAMSS on MSM8996 and SDM630/SDM660, where two VFE power domains are
>    managed separately by the camss device driver, the power domains are
>    linked and unlinked on demand by their functions vfe_pm_domain_on()
>    and vfe_pm_domain_off() respectively,
> 3) CAMSS on SDM845 and SM8250 platforms, and there are two VFE power
>    domains and their parent power domain TITAN_TOP, the latter one
>    shall be turned on prior to turning on any of VFE power domains.
>
> Due to a previously missing link between TITAN_TOP and VFEx power domains
> in the latter case, which is now fixed by [1], it was decided always to
> turn on all found VFE power domains and TITAN_TOP power domain, even if
> just one particular VFE is needed to be enabled or none of VFE power domains
> are required, for instance the latter case is when vfe_lite is in use.
> This misusage becomes more incovenient and clumsy, if next generations are
> to be supported, for instance CAMSS on SM8450 has three VFE power domains.
>
> The change splits the power management support for platforms with TITAN_TOP
> parent power domain, and, since 'power-domain-names' property is not present
> in camss device tree nodes, the assumption is that the first N power domains
> from the 'power-domains' list correspond to VFE power domains, and, if the
> number of power domains is greater than number of non-lite VFEs, then the
> last power domain from the list is the TITAN_TOP power domain.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  .../media/platform/qcom/camss/camss-vfe-170.c | 20 ++++++++++++-
>  .../media/platform/qcom/camss/camss-vfe-480.c | 20 ++++++++++++-
>  drivers/media/platform/qcom/camss/camss.c     | 30 ++++++++++---------
>  3 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
> index 600150cfc4f7..8e506a805d11 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
> @@ -687,7 +687,12 @@ static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
>   */
>  static void vfe_pm_domain_off(struct vfe_device *vfe)
>  {
> -       /* nop */
> +       struct camss *camss = vfe->camss;
> +
> +       if (vfe->id >= camss->vfe_num)
> +               return;
> +
> +       device_link_del(camss->genpd_link[vfe->id]);
>  }
>
>  /*
> @@ -696,6 +701,19 @@ static void vfe_pm_domain_off(struct vfe_device *vfe)
>   */
>  static int vfe_pm_domain_on(struct vfe_device *vfe)
>  {
> +       struct camss *camss = vfe->camss;
> +       enum vfe_line_id id = vfe->id;
> +
> +       if (id >= camss->vfe_num)
> +               return 0;
> +
> +       camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id],
> +                                               DL_FLAG_STATELESS |
> +                                               DL_FLAG_PM_RUNTIME |
> +                                               DL_FLAG_RPM_ACTIVE);
> +       if (!camss->genpd_link[id])
> +               return -EINVAL;
> +
>         return 0;
>  }
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> index 129585110393..3aa962b5663b 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
> @@ -494,7 +494,12 @@ static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
>   */
>  static void vfe_pm_domain_off(struct vfe_device *vfe)
>  {
> -       /* nop */
> +       struct camss *camss = vfe->camss;
> +
> +       if (vfe->id >= camss->vfe_num)
> +               return;
> +
> +       device_link_del(camss->genpd_link[vfe->id]);
>  }
>
>  /*
> @@ -503,6 +508,19 @@ static void vfe_pm_domain_off(struct vfe_device *vfe)
>   */
>  static int vfe_pm_domain_on(struct vfe_device *vfe)
>  {
> +       struct camss *camss = vfe->camss;
> +       enum vfe_line_id id = vfe->id;
> +
> +       if (id >= camss->vfe_num)
> +               return 0;
> +
> +       camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id],
> +                                               DL_FLAG_STATELESS |
> +                                               DL_FLAG_PM_RUNTIME |
> +                                               DL_FLAG_RPM_ACTIVE);
> +       if (!camss->genpd_link[id])
> +               return -EINVAL;
> +
>         return 0;
>  }
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 795eebd9af6c..f009297ba182 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1453,7 +1453,6 @@ static const struct media_device_ops camss_media_ops = {
>  static int camss_configure_pd(struct camss *camss)
>  {
>         struct device *dev = camss->dev;
> -       int last_pm_domain = 0;
>         int i;
>         int ret;
>
> @@ -1484,32 +1483,34 @@ static int camss_configure_pd(struct camss *camss)
>         if (!camss->genpd_link)
>                 return -ENOMEM;
>
> +       /*
> +        * VFE power domains are in the beginning of the list, and while all
> +        * power domains should be attached, only if TITAN_TOP power domain is
> +        * found in the list, it should be linked over here.
> +        */
>         for (i = 0; i < camss->genpd_num; i++) {
>                 camss->genpd[i] = dev_pm_domain_attach_by_id(camss->dev, i);
>                 if (IS_ERR(camss->genpd[i])) {
>                         ret = PTR_ERR(camss->genpd[i]);
>                         goto fail_pm;
>                 }
> +       }
>
> -               camss->genpd_link[i] = device_link_add(camss->dev, camss->genpd[i],
> -                                                      DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
> -                                                      DL_FLAG_RPM_ACTIVE);
> -               if (!camss->genpd_link[i]) {
> -                       dev_pm_domain_detach(camss->genpd[i], true);
> +       if (i > camss->vfe_num) {
> +               camss->genpd_link[i - 1] = device_link_add(camss->dev, camss->genpd[i - 1],
> +                                                          DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
> +                                                          DL_FLAG_RPM_ACTIVE);
> +               if (!camss->genpd_link[i - 1]) {
>                         ret = -EINVAL;
>                         goto fail_pm;
>                 }
> -
> -               last_pm_domain = i;
>         }
>
>         return 0;
>
>  fail_pm:
> -       for (i = 0; i < last_pm_domain; i++) {
> -               device_link_del(camss->genpd_link[i]);
> +       for (--i ; i >= 0; i--)
>                 dev_pm_domain_detach(camss->genpd[i], true);
> -       }
>
>         return ret;
>  }
> @@ -1711,10 +1712,11 @@ void camss_delete(struct camss *camss)
>         if (camss->genpd_num == 1)
>                 return;
>
> -       for (i = 0; i < camss->genpd_num; i++) {
> -               device_link_del(camss->genpd_link[i]);
> +       if (camss->genpd_num > camss->vfe_num)
> +               device_link_del(camss->genpd_link[camss->genpd_num - 1]);
> +
> +       for (i = 0; i < camss->genpd_num; i++)
>                 dev_pm_domain_detach(camss->genpd[i], true);
> -       }
>  }
>
>  /*
> --
> 2.33.0
>

Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH 1/2] media: camss: Collect information about a number of lite VFEs

[Robert Foss]

On Tue, 5 Jul 2022 at 00:15, Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> VFE lite IPs are found on CAMSS with TITAN_TOP power domains, and in
> some aspects these types of VFEs are different, in particular there
> is no need to enable VFE power domains to operate over VFE lite IPs.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  drivers/media/platform/qcom/camss/camss.c | 20 +++++++++++---------
>  drivers/media/platform/qcom/camss/camss.h |  1 +
>  2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 7a929f19e79b..795eebd9af6c 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1170,7 +1170,7 @@ static int camss_init_subdevices(struct camss *camss)
>         }
>
>         /* note: SM8250 requires VFE to be initialized before CSID */
> -       for (i = 0; i < camss->vfe_num; i++) {
> +       for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++) {
>                 ret = msm_vfe_subdev_init(camss, &camss->vfe[i],
>                                           &vfe_res[i], i);
>                 if (ret < 0) {
> @@ -1242,7 +1242,7 @@ static int camss_register_entities(struct camss *camss)
>                 goto err_reg_ispif;
>         }
>
> -       for (i = 0; i < camss->vfe_num; i++) {
> +       for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++) {
>                 ret = msm_vfe_register_entities(&camss->vfe[i],
>                                                 &camss->v4l2_dev);
>                 if (ret < 0) {
> @@ -1314,7 +1314,7 @@ static int camss_register_entities(struct camss *camss)
>                                 }
>         } else {
>                 for (i = 0; i < camss->csid_num; i++)
> -                       for (k = 0; k < camss->vfe_num; k++)
> +                       for (k = 0; k < camss->vfe_num + camss->vfe_lite_num; k++)
>                                 for (j = 0; j < camss->vfe[k].line_num; j++) {
>                                         struct v4l2_subdev *csid = &camss->csid[i].subdev;
>                                         struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
> @@ -1338,7 +1338,7 @@ static int camss_register_entities(struct camss *camss)
>         return 0;
>
>  err_link:
> -       i = camss->vfe_num;
> +       i = camss->vfe_num + camss->vfe_lite_num;
>  err_reg_vfe:
>         for (i--; i >= 0; i--)
>                 msm_vfe_unregister_entities(&camss->vfe[i]);
> @@ -1377,7 +1377,7 @@ static void camss_unregister_entities(struct camss *camss)
>
>         msm_ispif_unregister_entities(camss->ispif);
>
> -       for (i = 0; i < camss->vfe_num; i++)
> +       for (i = 0; i < camss->vfe_num + camss->vfe_lite_num; i++)
>                 msm_vfe_unregister_entities(&camss->vfe[i]);
>  }
>
> @@ -1579,13 +1579,15 @@ static int camss_probe(struct platform_device *pdev)
>                 camss->version = CAMSS_845;
>                 camss->csiphy_num = 4;
>                 camss->csid_num = 3;
> -               camss->vfe_num = 3;
> +               camss->vfe_num = 2;
> +               camss->vfe_lite_num = 1;
>         } else if (of_device_is_compatible(dev->of_node,
>                                            "qcom,sm8250-camss")) {
>                 camss->version = CAMSS_8250;
>                 camss->csiphy_num = 6;
>                 camss->csid_num = 4;
> -               camss->vfe_num = 4;
> +               camss->vfe_num = 2;
> +               camss->vfe_lite_num = 2;
>         } else {
>                 return -EINVAL;
>         }
> @@ -1607,8 +1609,8 @@ static int camss_probe(struct platform_device *pdev)
>                         return -ENOMEM;
>         }
>
> -       camss->vfe = devm_kcalloc(dev, camss->vfe_num, sizeof(*camss->vfe),
> -                                 GFP_KERNEL);
> +       camss->vfe = devm_kcalloc(dev, camss->vfe_num + camss->vfe_lite_num,
> +                                 sizeof(*camss->vfe), GFP_KERNEL);
>         if (!camss->vfe)
>                 return -ENOMEM;
>
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 0db80cadbbaa..3acd2b3403e8 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -97,6 +97,7 @@ struct camss {
>         struct csid_device *csid;
>         struct ispif_device *ispif;
>         int vfe_num;
> +       int vfe_lite_num;
>         struct vfe_device *vfe;
>         atomic_t ref_count;
>         int genpd_num;
> --
> 2.33.0
>

Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH 0/2] media: camss: Simplify and improve power management of VFEs

[Vladimir Zapolskiy]

On 7/5/22 01:15, Vladimir Zapolskiy wrote:
> Since a rework of CAMSS power domain management for newer platforms done in
> commit 2f6f8af67203 ("media: camss: Refactor VFE power domain toggling"),
> all operations over CAMSS imply enablement of all power domains described
> in the correspondent device tree node. Apparently it's too excessive and it's
> quite a complicated scheme to allow simple addition of newer platforms with
> even more power domains.
> 
> I would appreciate, if somebody can test the changes on db820 for any probable
> regressions.
> 
> The change is based on changes in clock framework [1] and on a recent fix-up [2]
> in camss.
> 
> [1] https://lore.kernel.org/linux-clk/20220519214133.1728979-1-vladimir.zapolskiy@linaro.org/
> [2] https://lore.kernel.org/linux-media/20220704220814.629130-1-vladimir.zapolskiy@linaro.org/
> 
> Vladimir Zapolskiy (2):
>    media: camss: Collect information about a number of lite VFEs
>    media: camss: Split power domain management
> 
>   .../media/platform/qcom/camss/camss-vfe-170.c | 20 +++++++-
>   .../media/platform/qcom/camss/camss-vfe-480.c | 20 +++++++-
>   drivers/media/platform/qcom/camss/camss.c     | 50 ++++++++++---------
>   drivers/media/platform/qcom/camss/camss.h     |  1 +
>   4 files changed, 66 insertions(+), 25 deletions(-)
> 

Gentle ping, I hope the changes are good to be included.

--
Best wishes,
Vladimir