[PATCH] media: camss: Do not attach an already attached power domain on MSM8916 platform

[PATCH] media: camss: Do not attach an already attached power domain on MSM8916 platform

[Vladimir Zapolskiy]

The change to dynamically allocated power domains neglected a case of
CAMSS on MSM8916 platform, where a single VFE power domain is neither
attached, linked or managed in runtime in any way explicitly.

This is a special case and it shall be kept as is, because the power
domain management is done outside of the driver, and it's very different
in comparison to all other platforms supported by CAMSS.

Fixes: 929684b7ef4d ("media: camss: Allocate power domain resources dynamically")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index bf716b171c02..9e2899a0cdf4 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1684,6 +1684,14 @@ static int camss_configure_pd(struct camss *camss)
 		return camss->genpd_num;
 	}
 
+	/*
+	 * If a platform device has just one power domain, then it is attached
+	 * at platform_probe() level, thus there shall be no need and even no
+	 * option to attach it again, this is the case for CAMSS on MSM8916.
+	 */
+	if (camss->genpd_num == 1)
+		return 0;
+
 	camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
 					  sizeof(*camss->genpd), GFP_KERNEL);
 	if (!camss->genpd)
@@ -1923,6 +1931,9 @@ void camss_delete(struct camss *camss)
 
 	pm_runtime_disable(camss->dev);
 
+	if (camss->genpd_num == 1)
+		return;
+
 	for (i = 0; i < camss->genpd_num; i++) {
 		device_link_del(camss->genpd_link[i]);
 		dev_pm_domain_detach(camss->genpd[i], true);
-- 
2.33.0

Re: [PATCH] media: camss: Do not attach an already attached power domain on MSM8916 platform

[Vladimir Zapolskiy]

On 7/4/22 13:57, Vladimir Zapolskiy wrote:
> The change to dynamically allocated power domains neglected a case of
> CAMSS on MSM8916 platform, where a single VFE power domain is neither
> attached, linked or managed in runtime in any way explicitly.
> 
> This is a special case and it shall be kept as is, because the power
> domain management is done outside of the driver, and it's very different
> in comparison to all other platforms supported by CAMSS.
> 
> Fixes: 929684b7ef4d ("media: camss: Allocate power domain resources dynamically")

Fixes: 6b1814e26989 ("media: camss: Allocate power domain resources dynamically")

is the correct commit id found on media/master, please let me know, if anyone
wishes me to resend the fix.

> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index bf716b171c02..9e2899a0cdf4 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1684,6 +1684,14 @@ static int camss_configure_pd(struct camss *camss)
>   		return camss->genpd_num;
>   	}
>   
> +	/*
> +	 * If a platform device has just one power domain, then it is attached
> +	 * at platform_probe() level, thus there shall be no need and even no
> +	 * option to attach it again, this is the case for CAMSS on MSM8916.
> +	 */
> +	if (camss->genpd_num == 1)
> +		return 0;
> +
>   	camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
>   					  sizeof(*camss->genpd), GFP_KERNEL);
>   	if (!camss->genpd)
> @@ -1923,6 +1931,9 @@ void camss_delete(struct camss *camss)
>   
>   	pm_runtime_disable(camss->dev);
>   
> +	if (camss->genpd_num == 1)
> +		return;
> +
>   	for (i = 0; i < camss->genpd_num; i++) {
>   		device_link_del(camss->genpd_link[i]);
>   		dev_pm_domain_detach(camss->genpd[i], true);

--
Best wishes,
Vladimir

Re: [PATCH] media: camss: Do not attach an already attached power domain on MSM8916 platform

[Robert Foss]

On Mon, 4 Jul 2022 at 13:08, Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> On 7/4/22 13:57, Vladimir Zapolskiy wrote:
> > The change to dynamically allocated power domains neglected a case of
> > CAMSS on MSM8916 platform, where a single VFE power domain is neither
> > attached, linked or managed in runtime in any way explicitly.
> >
> > This is a special case and it shall be kept as is, because the power
> > domain management is done outside of the driver, and it's very different
> > in comparison to all other platforms supported by CAMSS.
> >
> > Fixes: 929684b7ef4d ("media: camss: Allocate power domain resources dynamically")
>
> Fixes: 6b1814e26989 ("media: camss: Allocate power domain resources dynamically")

6b1814e26989 is the correct commit id to use.

>
> is the correct commit id found on media/master, please let me know, if anyone
> wishes me to resend the fix.
>
> > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> > ---
> >   drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> > index bf716b171c02..9e2899a0cdf4 100644
> > --- a/drivers/media/platform/qcom/camss/camss.c
> > +++ b/drivers/media/platform/qcom/camss/camss.c
> > @@ -1684,6 +1684,14 @@ static int camss_configure_pd(struct camss *camss)
> >               return camss->genpd_num;
> >       }
> >
> > +     /*
> > +      * If a platform device has just one power domain, then it is attached
> > +      * at platform_probe() level, thus there shall be no need and even no
> > +      * option to attach it again, this is the case for CAMSS on MSM8916.
> > +      */
> > +     if (camss->genpd_num == 1)
> > +             return 0;
> > +
> >       camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
> >                                         sizeof(*camss->genpd), GFP_KERNEL);
> >       if (!camss->genpd)
> > @@ -1923,6 +1931,9 @@ void camss_delete(struct camss *camss)
> >
> >       pm_runtime_disable(camss->dev);
> >
> > +     if (camss->genpd_num == 1)
> > +             return;
> > +
> >       for (i = 0; i < camss->genpd_num; i++) {
> >               device_link_del(camss->genpd_link[i]);
> >               dev_pm_domain_detach(camss->genpd[i], true);
>
> --

With the commit id fixed, please add my r-b.

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