[PATCH 0/4] OPP: Re-work code to drop _opp_attach|detach_genpd()

[PATCH 0/4] OPP: Re-work code to drop _opp_attach|detach_genpd()

[Ulf Hansson]

Note, this series is the next step beyond another genpd/OPP series [1] that was
recently submitted (and still being discussed for merging). In other words to
test this, that other series needs to be applied as the first step.

This series converts the final users of _opp_attach|detach_genpd() to use
dev_pm_domain_attach|detach_list(). Then the final patch drops the redundant
code for _opp_attach|detach_genpd().

Please help to test and review!

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/all/20240718234319.356451-1-ulf.hansson@linaro.org/

Ulf Hansson (4):
  drm/tegra: gr3d: Convert into dev_pm_domain_attach|detach_list()
  media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain
  cpufreq: qcom-nvmem: Convert to dev_pm_domain_attach|detach_list()
  OPP: Drop redundant _opp_attach|detach_genpd()

 drivers/cpufreq/qcom-cpufreq-nvmem.c          | 79 +++++----------
 drivers/gpu/drm/tegra/gr3d.c                  | 46 +++------
 drivers/media/platform/qcom/venus/core.c      |  8 +-
 drivers/media/platform/qcom/venus/core.h      |  6 +-
 .../media/platform/qcom/venus/pm_helpers.c    | 31 ++----
 drivers/opp/core.c                            | 96 +------------------
 drivers/opp/opp.h                             |  3 +-
 include/linux/pm_opp.h                        | 38 +-------
 8 files changed, 56 insertions(+), 251 deletions(-)

-- 
2.34.1

[PATCH 1/4] drm/tegra: gr3d: Convert into dev_pm_domain_attach|detach_list()

[Ulf Hansson]

Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
and manage the device-link, let's avoid the boilerplate-code by converting
into dev_pm_domain_attach|detach_list.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/gpu/drm/tegra/gr3d.c | 46 ++++++++++--------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 00c8564520e7..4de1ea0fc7c0 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -46,6 +46,7 @@ struct gr3d {
 	unsigned int nclocks;
 	struct reset_control_bulk_data resets[RST_GR3D_MAX];
 	unsigned int nresets;
+	struct dev_pm_domain_list *pd_list;
 
 	DECLARE_BITMAP(addr_regs, GR3D_NUM_REGS);
 };
@@ -369,18 +370,12 @@ static int gr3d_power_up_legacy_domain(struct device *dev, const char *name,
 	return 0;
 }
 
-static void gr3d_del_link(void *link)
-{
-	device_link_del(link);
-}
-
 static int gr3d_init_power(struct device *dev, struct gr3d *gr3d)
 {
-	static const char * const opp_genpd_names[] = { "3d0", "3d1", NULL };
-	const u32 link_flags = DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME;
-	struct device **opp_virt_devs, *pd_dev;
-	struct device_link *link;
-	unsigned int i;
+	struct dev_pm_domain_attach_data pd_data = {
+		.pd_names = (const char *[]) { "3d0", "3d1" },
+		.num_pd_names = 2,
+	};
 	int err;
 
 	err = of_count_phandle_with_args(dev->of_node, "power-domains",
@@ -414,29 +409,10 @@ static int gr3d_init_power(struct device *dev, struct gr3d *gr3d)
 	if (dev->pm_domain)
 		return 0;
 
-	err = devm_pm_opp_attach_genpd(dev, opp_genpd_names, &opp_virt_devs);
-	if (err)
+	err = dev_pm_domain_attach_list(dev, &pd_data, &gr3d->pd_list);
+	if (err < 0)
 		return err;
 
-	for (i = 0; opp_genpd_names[i]; i++) {
-		pd_dev = opp_virt_devs[i];
-		if (!pd_dev) {
-			dev_err(dev, "failed to get %s power domain\n",
-				opp_genpd_names[i]);
-			return -EINVAL;
-		}
-
-		link = device_link_add(dev, pd_dev, link_flags);
-		if (!link) {
-			dev_err(dev, "failed to link to %s\n", dev_name(pd_dev));
-			return -EINVAL;
-		}
-
-		err = devm_add_action_or_reset(dev, gr3d_del_link, link);
-		if (err)
-			return err;
-	}
-
 	return 0;
 }
 
@@ -527,13 +503,13 @@ static int gr3d_probe(struct platform_device *pdev)
 
 	err = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
 	if (err)
-		return err;
+		goto err;
 
 	err = host1x_client_register(&gr3d->client.base);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
 			err);
-		return err;
+		goto err;
 	}
 
 	/* initialize address register map */
@@ -541,6 +517,9 @@ static int gr3d_probe(struct platform_device *pdev)
 		set_bit(gr3d_addr_regs[i], gr3d->addr_regs);
 
 	return 0;
+err:
+	dev_pm_domain_detach_list(gr3d->pd_list);
+	return err;
 }
 
 static void gr3d_remove(struct platform_device *pdev)
@@ -549,6 +528,7 @@ static void gr3d_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 	host1x_client_unregister(&gr3d->client.base);
+	dev_pm_domain_detach_list(gr3d->pd_list);
 }
 
 static int __maybe_unused gr3d_runtime_suspend(struct device *dev)
-- 
2.34.1

Re: [PATCH 1/4] drm/tegra: gr3d: Convert into dev_pm_domain_attach|detach_list()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

On Tue, Jul 23, 2024 at 04:46:07PM GMT, Ulf Hansson wrote:
> Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> and manage the device-link, let's avoid the boilerplate-code by converting
> into dev_pm_domain_attach|detach_list.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/gpu/drm/tegra/gr3d.c | 46 ++++++++++--------------------------
>  1 file changed, 13 insertions(+), 33 deletions(-)

Applied to drm-misc-next, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] drm/tegra: gr3d: Convert into dev_pm_domain_attach|detach_list()

[Ulf Hansson]

On Wed, 28 Aug 2024 at 17:06, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Tue, Jul 23, 2024 at 04:46:07PM GMT, Ulf Hansson wrote:
> > Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> > and manage the device-link, let's avoid the boilerplate-code by converting
> > into dev_pm_domain_attach|detach_list.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/gpu/drm/tegra/gr3d.c | 46 ++++++++++--------------------------
> >  1 file changed, 13 insertions(+), 33 deletions(-)
>
> Applied to drm-misc-next, thanks.

Please drop this from your tree. I already have a patch [1] that you
have acked, which is a newer version of $subject patch.

Sorry if this was unclear.

Kind regards
Uffe

[1]
https://lore.kernel.org/all/CAPDyKFqZ9XXi5_-essaGbdWBDLX8uR5nO9vDJCExBGV+10KqZA@mail.gmail.com/

Re: [PATCH 1/4] drm/tegra: gr3d: Convert into dev_pm_domain_attach|detach_list()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

On Wed, Aug 28, 2024 at 05:26:40PM GMT, Ulf Hansson wrote:
> On Wed, 28 Aug 2024 at 17:06, Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Tue, Jul 23, 2024 at 04:46:07PM GMT, Ulf Hansson wrote:
> > > Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> > > and manage the device-link, let's avoid the boilerplate-code by converting
> > > into dev_pm_domain_attach|detach_list.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >  drivers/gpu/drm/tegra/gr3d.c | 46 ++++++++++--------------------------
> > >  1 file changed, 13 insertions(+), 33 deletions(-)
> >
> > Applied to drm-misc-next, thanks.
> 
> Please drop this from your tree. I already have a patch [1] that you
> have acked, which is a newer version of $subject patch.
> 
> Sorry if this was unclear.

Ugh... indeed. Sorry, I was confusing these. Unfortunately I had pushed
to drm-misc-next already, so I can't drop, but I can revert.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Ulf Hansson]

Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
and manage the device-link, let's avoid the boilerplate-code by converting
into dev_pm_domain_attach|detach_list.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/media/platform/qcom/venus/core.c      |  8 ++---
 drivers/media/platform/qcom/venus/core.h      |  6 +---
 .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index ce206b709754..a422bbb3b610 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -709,7 +709,7 @@ static const struct venus_resources sdm845_res_v2 = {
 	.vcodec_clks_num = 2,
 	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0", "vcodec1" },
 	.vcodec_pmdomains_num = 3,
-	.opp_pmdomain = (const char *[]) { "cx", NULL },
+	.opp_pmdomain = (const char *[]) { "cx" },
 	.vcodec_num = 2,
 	.max_load = 3110400,	/* 4096x2160@90 */
 	.hfi_version = HFI_VERSION_4XX,
@@ -758,7 +758,7 @@ static const struct venus_resources sc7180_res = {
 	.vcodec_clks_num = 2,
 	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
 	.vcodec_pmdomains_num = 2,
-	.opp_pmdomain = (const char *[]) { "cx", NULL },
+	.opp_pmdomain = (const char *[]) { "cx" },
 	.vcodec_num = 1,
 	.hfi_version = HFI_VERSION_4XX,
 	.vpu_version = VPU_VERSION_AR50,
@@ -815,7 +815,7 @@ static const struct venus_resources sm8250_res = {
 	.vcodec_clks_num = 1,
 	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
 	.vcodec_pmdomains_num = 2,
-	.opp_pmdomain = (const char *[]) { "mx", NULL },
+	.opp_pmdomain = (const char *[]) { "mx" },
 	.vcodec_num = 1,
 	.max_load = 7833600,
 	.hfi_version = HFI_VERSION_6XX,
@@ -874,7 +874,7 @@ static const struct venus_resources sc7280_res = {
 	.vcodec_clks_num = 2,
 	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
 	.vcodec_pmdomains_num = 2,
-	.opp_pmdomain = (const char *[]) { "cx", NULL },
+	.opp_pmdomain = (const char *[]) { "cx" },
 	.vcodec_num = 1,
 	.hfi_version = HFI_VERSION_6XX,
 	.vpu_version = VPU_VERSION_IRIS2_1,
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 6a77de374454..aec587e6294f 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -132,9 +132,7 @@ struct venus_format {
  * @vcodec1_clks: an array of vcodec1 struct clk pointers
  * @video_path: an interconnect handle to video to/from memory path
  * @cpucfg_path: an interconnect handle to cpu configuration path
- * @has_opp_table: does OPP table exist
  * @pmdomains:	a pointer to a list of pmdomains
- * @opp_dl_venus: an device-link for device OPP
  * @opp_pmdomain: an OPP power-domain
  * @resets: an array of reset signals
  * @vdev_dec:	a reference to video device structure for decoder instances
@@ -185,10 +183,8 @@ struct venus_core {
 	struct clk *vcodec1_clks[VIDC_VCODEC_CLKS_NUM_MAX];
 	struct icc_path *video_path;
 	struct icc_path *cpucfg_path;
-	bool has_opp_table;
 	struct dev_pm_domain_list *pmdomains;
-	struct device_link *opp_dl_venus;
-	struct device *opp_pmdomain;
+	struct dev_pm_domain_list *opp_pmdomain;
 	struct reset_control *resets[VIDC_RESETS_NUM_MAX];
 	struct video_device *vdev_dec;
 	struct video_device *vdev_enc;
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 502822059498..e133683871aa 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -857,7 +857,6 @@ static int venc_power_v4(struct device *dev, int on)
 static int vcodec_domains_get(struct venus_core *core)
 {
 	int ret;
-	struct device **opp_virt_dev;
 	struct device *dev = core->dev;
 	const struct venus_resources *res = core->res;
 	struct dev_pm_domain_attach_data vcodec_data = {
@@ -865,6 +864,11 @@ static int vcodec_domains_get(struct venus_core *core)
 		.num_pd_names = res->vcodec_pmdomains_num,
 		.pd_flags = PD_FLAG_NO_DEV_LINK,
 	};
+	struct dev_pm_domain_attach_data opp_pd_data = {
+		.pd_names = res->opp_pmdomain,
+		.num_pd_names = 1,
+		.pd_flags = PD_FLAG_DEV_LINK_ON,
+	};
 
 	if (!res->vcodec_pmdomains_num)
 		goto skip_pmdomains;
@@ -874,24 +878,14 @@ static int vcodec_domains_get(struct venus_core *core)
 		return ret;
 
 skip_pmdomains:
-	if (!core->res->opp_pmdomain)
+	if (!res->opp_pmdomain)
 		return 0;
 
 	/* Attach the power domain for setting performance state */
-	ret = devm_pm_opp_attach_genpd(dev, res->opp_pmdomain, &opp_virt_dev);
+	ret = dev_pm_domain_attach_list(dev, &opp_pd_data, &core->opp_pmdomain);
 	if (ret)
 		goto opp_attach_err;
 
-	core->opp_pmdomain = *opp_virt_dev;
-	core->opp_dl_venus = device_link_add(dev, core->opp_pmdomain,
-					     DL_FLAG_RPM_ACTIVE |
-					     DL_FLAG_PM_RUNTIME |
-					     DL_FLAG_STATELESS);
-	if (!core->opp_dl_venus) {
-		ret = -ENODEV;
-		goto opp_attach_err;
-	}
-
 	return 0;
 
 opp_attach_err:
@@ -902,12 +896,7 @@ static int vcodec_domains_get(struct venus_core *core)
 static void vcodec_domains_put(struct venus_core *core)
 {
 	dev_pm_domain_detach_list(core->pmdomains);
-
-	if (!core->has_opp_table)
-		return;
-
-	if (core->opp_dl_venus)
-		device_link_del(core->opp_dl_venus);
+	dev_pm_domain_detach_list(core->opp_pmdomain);
 }
 
 static int core_resets_reset(struct venus_core *core)
@@ -996,9 +985,7 @@ static int core_get_v4(struct venus_core *core)
 
 	if (core->res->opp_pmdomain) {
 		ret = devm_pm_opp_of_add_table(dev);
-		if (!ret) {
-			core->has_opp_table = true;
-		} else if (ret != -ENODEV) {
+		if (ret && ret != -ENODEV) {
 			dev_err(dev, "invalid OPP table in device tree\n");
 			return ret;
 		}
-- 
2.34.1

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Stanimir Varbanov]

Hi Ulf,

Thank you for the patch!

On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
> Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> and manage the device-link, let's avoid the boilerplate-code by converting
> into dev_pm_domain_attach|detach_list.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/media/platform/qcom/venus/core.c      |  8 ++---
>   drivers/media/platform/qcom/venus/core.h      |  6 +---
>   .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
>   3 files changed, 14 insertions(+), 31 deletions(-)
> 

Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>

I'll pick it through linux-media.

-- 
regards,
Stan

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Ulf Hansson]

On Tue, 20 Aug 2024 at 22:48, Stanimir Varbanov
<stanimir.k.varbanov@gmail.com> wrote:
>
> Hi Ulf,
>
> Thank you for the patch!
>
> On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
> > Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> > and manage the device-link, let's avoid the boilerplate-code by converting
> > into dev_pm_domain_attach|detach_list.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >   drivers/media/platform/qcom/venus/core.c      |  8 ++---
> >   drivers/media/platform/qcom/venus/core.h      |  6 +---
> >   .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
> >   3 files changed, 14 insertions(+), 31 deletions(-)
> >
>
> Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>

Thanks!

>
> I'll pick it through linux-media.

Please don't.

I should have stated that this depends on another series [1] - and
they need either to go together or we need to defer $subject patch
until the next release cycle.

Kind regards
Uffe

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Ulf Hansson]

On Wed, 21 Aug 2024 at 10:56, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 20 Aug 2024 at 22:48, Stanimir Varbanov
> <stanimir.k.varbanov@gmail.com> wrote:
> >
> > Hi Ulf,
> >
> > Thank you for the patch!
> >
> > On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
> > > Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> > > and manage the device-link, let's avoid the boilerplate-code by converting
> > > into dev_pm_domain_attach|detach_list.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >   drivers/media/platform/qcom/venus/core.c      |  8 ++---
> > >   drivers/media/platform/qcom/venus/core.h      |  6 +---
> > >   .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
> > >   3 files changed, 14 insertions(+), 31 deletions(-)
> > >
> >
> > Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
>
> Thanks!
>
> >
> > I'll pick it through linux-media.
>
> Please don't.
>
> I should have stated that this depends on another series [1] - and
> they need either to go together or we need to defer $subject patch
> until the next release cycle.
>
> Kind regards
> Uffe

Forgot the link, here it is:
[1]
https://lore.kernel.org/all/20240718234319.356451-1-ulf.hansson@linaro.org/

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Stanimir Varbanov]

Hi Ulf,

On 21.08.24 г. 11:56 ч., Ulf Hansson wrote:
> On Tue, 20 Aug 2024 at 22:48, Stanimir Varbanov
> <stanimir.k.varbanov@gmail.com> wrote:
>>
>> Hi Ulf,
>>
>> Thank you for the patch!
>>
>> On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
>>> Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
>>> and manage the device-link, let's avoid the boilerplate-code by converting
>>> into dev_pm_domain_attach|detach_list.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>    drivers/media/platform/qcom/venus/core.c      |  8 ++---
>>>    drivers/media/platform/qcom/venus/core.h      |  6 +---
>>>    .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
>>>    3 files changed, 14 insertions(+), 31 deletions(-)
>>>
>>
>> Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
> 
> Thanks!
> 
>>
>> I'll pick it through linux-media.
> 
> Please don't.
> 
> I should have stated that this depends on another series [1] - and
> they need either to go together or we need to defer $subject patch
> until the next release cycle.

Sure, then I guess we will deffer venus patch until the preparation 
series is merged to avoid conflicts. Thank you!

> 
> Kind regards
> Uffe

-- 
regards,
Stan

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Ulf Hansson]

On Thu, 22 Aug 2024 at 20:05, Stanimir Varbanov
<stanimir.k.varbanov@gmail.com> wrote:
>
> Hi Ulf,
>
> On 21.08.24 г. 11:56 ч., Ulf Hansson wrote:
> > On Tue, 20 Aug 2024 at 22:48, Stanimir Varbanov
> > <stanimir.k.varbanov@gmail.com> wrote:
> >>
> >> Hi Ulf,
> >>
> >> Thank you for the patch!
> >>
> >> On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
> >>> Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
> >>> and manage the device-link, let's avoid the boilerplate-code by converting
> >>> into dev_pm_domain_attach|detach_list.
> >>>
> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >>> ---
> >>>    drivers/media/platform/qcom/venus/core.c      |  8 ++---
> >>>    drivers/media/platform/qcom/venus/core.h      |  6 +---
> >>>    .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
> >>>    3 files changed, 14 insertions(+), 31 deletions(-)
> >>>
> >>
> >> Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
> >
> > Thanks!
> >
> >>
> >> I'll pick it through linux-media.
> >
> > Please don't.
> >
> > I should have stated that this depends on another series [1] - and
> > they need either to go together or we need to defer $subject patch
> > until the next release cycle.
>
> Sure, then I guess we will deffer venus patch until the preparation
> series is merged to avoid conflicts. Thank you!

Assuming the preparation series gets accepted, maybe we can give it a
try via my pmdomain tree? Or do expect to land a lot of code that
could conflict?

I also realized that I already have a different series [1] queued in
my pmdomain tree from Dikshita Agarwal (reviewed by Bryan), that moves
an existing call for dev_pm_domain_attach() to the new
devm_pm_domain_attach() helper. So far I haven't received any reports
about conflicts from linux-next, so it looks good I think.

Kind regards
Uffe

[1]
https://lore.kernel.org/all/CAPDyKFqsHL3uatmLZaRzZ_GfkZw-+fURQNSEgvmrf-ini+WHng@mail.gmail.com/

Re: [PATCH 2/4] media: venus: Use dev_pm_domain_attach|detach_list() for OPP PM domain

[Stanimir Varbanov]

Hi Ulf,

On 23.08.24 г. 0:40 ч., Ulf Hansson wrote:
> On Thu, 22 Aug 2024 at 20:05, Stanimir Varbanov
> <stanimir.k.varbanov@gmail.com> wrote:
>>
>> Hi Ulf,
>>
>> On 21.08.24 г. 11:56 ч., Ulf Hansson wrote:
>>> On Tue, 20 Aug 2024 at 22:48, Stanimir Varbanov
>>> <stanimir.k.varbanov@gmail.com> wrote:
>>>>
>>>> Hi Ulf,
>>>>
>>>> Thank you for the patch!
>>>>
>>>> On 23.07.24 г. 17:46 ч., Ulf Hansson wrote:
>>>>> Rather than hooking up the PM domains through devm_pm_opp_attach_genpd()
>>>>> and manage the device-link, let's avoid the boilerplate-code by converting
>>>>> into dev_pm_domain_attach|detach_list.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>     drivers/media/platform/qcom/venus/core.c      |  8 ++---
>>>>>     drivers/media/platform/qcom/venus/core.h      |  6 +---
>>>>>     .../media/platform/qcom/venus/pm_helpers.c    | 31 ++++++-------------
>>>>>     3 files changed, 14 insertions(+), 31 deletions(-)
>>>>>
>>>>
>>>> Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
>>>
>>> Thanks!
>>>
>>>>
>>>> I'll pick it through linux-media.
>>>
>>> Please don't.
>>>
>>> I should have stated that this depends on another series [1] - and
>>> they need either to go together or we need to defer $subject patch
>>> until the next release cycle.
>>
>> Sure, then I guess we will deffer venus patch until the preparation
>> series is merged to avoid conflicts. Thank you!
> 
> Assuming the preparation series gets accepted, maybe we can give it a
> try via my pmdomain tree? Or do expect to land a lot of code that
> could conflict?

Please take it via pmdomain tree. Thank you!

> 
> I also realized that I already have a different series [1] queued in
> my pmdomain tree from Dikshita Agarwal (reviewed by Bryan), that moves
> an existing call for dev_pm_domain_attach() to the new
> devm_pm_domain_attach() helper. So far I haven't received any reports
> about conflicts from linux-next, so it looks good I think.
> 
> Kind regards
> Uffe
> 
> [1]
> https://lore.kernel.org/all/CAPDyKFqsHL3uatmLZaRzZ_GfkZw-+fURQNSEgvmrf-ini+WHng@mail.gmail.com/

-- 
regards,
Stan

[PATCH 3/4] cpufreq: qcom-nvmem: Convert to dev_pm_domain_attach|detach_list()

[Ulf Hansson]

Rather than hooking up the PM domains through _opp_attach_genpd() and
manually manage runtime PM for the corresponding virtual devices created by
genpd during attach, let's avoid the boilerplate-code by converting into
dev_pm_domain_attach|detach_list.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 79 +++++++++-------------------
 1 file changed, 26 insertions(+), 53 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index ea05d9d67490..65739ce98658 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -52,12 +52,13 @@ struct qcom_cpufreq_match_data {
 			   struct nvmem_cell *speedbin_nvmem,
 			   char **pvs_name,
 			   struct qcom_cpufreq_drv *drv);
-	const char **genpd_names;
+	const char **pd_names;
+	unsigned int num_pd_names;
 };
 
 struct qcom_cpufreq_drv_cpu {
 	int opp_token;
-	struct device **virt_devs;
+	struct dev_pm_domain_list *pd_list;
 };
 
 struct qcom_cpufreq_drv {
@@ -394,8 +395,6 @@ static int qcom_cpufreq_ipq8074_name_version(struct device *cpu_dev,
 	return 0;
 }
 
-static const char *generic_genpd_names[] = { "perf", NULL };
-
 static const struct qcom_cpufreq_match_data match_data_kryo = {
 	.get_version = qcom_cpufreq_kryo_name_version,
 };
@@ -406,13 +405,13 @@ static const struct qcom_cpufreq_match_data match_data_krait = {
 
 static const struct qcom_cpufreq_match_data match_data_msm8909 = {
 	.get_version = qcom_cpufreq_simple_get_version,
-	.genpd_names = generic_genpd_names,
+	.pd_names = (const char *[]) { "perf" },
+	.num_pd_names = 1,
 };
 
-static const char *qcs404_genpd_names[] = { "cpr", NULL };
-
 static const struct qcom_cpufreq_match_data match_data_qcs404 = {
-	.genpd_names = qcs404_genpd_names,
+	.pd_names = (const char *[]) { "cpr" },
+	.num_pd_names = 1,
 };
 
 static const struct qcom_cpufreq_match_data match_data_ipq6018 = {
@@ -427,28 +426,16 @@ static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
 	.get_version = qcom_cpufreq_ipq8074_name_version,
 };
 
-static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
+static void qcom_cpufreq_suspend_pd_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
 {
-	const char * const *name = drv->data->genpd_names;
+	struct dev_pm_domain_list *pd_list = drv->cpus[cpu].pd_list;
 	int i;
 
-	if (!drv->cpus[cpu].virt_devs)
+	if (!pd_list)
 		return;
 
-	for (i = 0; *name; i++, name++)
-		device_set_awake_path(drv->cpus[cpu].virt_devs[i]);
-}
-
-static void qcom_cpufreq_put_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
-{
-	const char * const *name = drv->data->genpd_names;
-	int i;
-
-	if (!drv->cpus[cpu].virt_devs)
-		return;
-
-	for (i = 0; *name; i++, name++)
-		pm_runtime_put(drv->cpus[cpu].virt_devs[i]);
+	for (i = 0; i < pd_list->num_pds; i++)
+		device_set_awake_path(pd_list->pd_devs[i]);
 }
 
 static int qcom_cpufreq_probe(struct platform_device *pdev)
@@ -505,7 +492,6 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
 	of_node_put(np);
 
 	for_each_possible_cpu(cpu) {
-		struct device **virt_devs = NULL;
 		struct dev_pm_opp_config config = {
 			.supported_hw = NULL,
 		};
@@ -524,12 +510,7 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
 				config.prop_name = pvs_name;
 		}
 
-		if (drv->data->genpd_names) {
-			config.genpd_names = drv->data->genpd_names;
-			config.virt_devs = &virt_devs;
-		}
-
-		if (config.supported_hw || config.genpd_names) {
+		if (config.supported_hw) {
 			drv->cpus[cpu].opp_token = dev_pm_opp_set_config(cpu_dev, &config);
 			if (drv->cpus[cpu].opp_token < 0) {
 				ret = drv->cpus[cpu].opp_token;
@@ -538,25 +519,17 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
 			}
 		}
 
-		if (virt_devs) {
-			const char * const *name = config.genpd_names;
-			int i, j;
-
-			for (i = 0; *name; i++, name++) {
-				ret = pm_runtime_resume_and_get(virt_devs[i]);
-				if (ret) {
-					dev_err(cpu_dev, "failed to resume %s: %d\n",
-						*name, ret);
+		if (drv->data->pd_names) {
+			struct dev_pm_domain_attach_data attach_data = {
+				.pd_names = drv->data->pd_names,
+				.num_pd_names = drv->data->num_pd_names,
+				.pd_flags = PD_FLAG_DEV_LINK_ON,
+			};
 
-					/* Rollback previous PM runtime calls */
-					name = config.genpd_names;
-					for (j = 0; *name && j < i; j++, name++)
-						pm_runtime_put(virt_devs[j]);
-
-					goto free_opp;
-				}
-			}
-			drv->cpus[cpu].virt_devs = virt_devs;
+			ret = dev_pm_domain_attach_list(cpu_dev, &attach_data,
+							&drv->cpus[cpu].pd_list);
+			if (ret < 0)
+				goto free_opp;
 		}
 	}
 
@@ -572,7 +545,7 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
 
 free_opp:
 	for_each_possible_cpu(cpu) {
-		qcom_cpufreq_put_virt_devs(drv, cpu);
+		dev_pm_domain_detach_list(drv->cpus[cpu].pd_list);
 		dev_pm_opp_clear_config(drv->cpus[cpu].opp_token);
 	}
 	return ret;
@@ -586,7 +559,7 @@ static void qcom_cpufreq_remove(struct platform_device *pdev)
 	platform_device_unregister(cpufreq_dt_pdev);
 
 	for_each_possible_cpu(cpu) {
-		qcom_cpufreq_put_virt_devs(drv, cpu);
+		dev_pm_domain_detach_list(drv->cpus[cpu].pd_list);
 		dev_pm_opp_clear_config(drv->cpus[cpu].opp_token);
 	}
 }
@@ -597,7 +570,7 @@ static int qcom_cpufreq_suspend(struct device *dev)
 	unsigned int cpu;
 
 	for_each_possible_cpu(cpu)
-		qcom_cpufreq_suspend_virt_devs(drv, cpu);
+		qcom_cpufreq_suspend_pd_devs(drv, cpu);
 
 	return 0;
 }
-- 
2.34.1

[PATCH 4/4] OPP: Drop redundant _opp_attach|detach_genpd()

[Ulf Hansson]

There no longer any users of _opp_attach|detach_genpd(), hence let's drop
it along with the corresponding exported functions.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c     | 96 +-----------------------------------------
 drivers/opp/opp.h      |  3 +-
 include/linux/pm_opp.h | 38 +----------------
 3 files changed, 3 insertions(+), 134 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index cad7e84c9ad3..66cac7a1d9db 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2360,86 +2360,6 @@ static void _opp_put_config_regulators_helper(struct opp_table *opp_table)
 		opp_table->config_regulators = NULL;
 }
 
-static void _opp_detach_genpd(struct opp_table *opp_table)
-{
-	int index;
-
-	for (index = 0; index < opp_table->required_opp_count; index++) {
-		if (!opp_table->required_devs[index])
-			continue;
-
-		dev_pm_domain_detach(opp_table->required_devs[index], false);
-	}
-}
-
-/*
- * Multiple generic power domains for a device are supported with the help of
- * virtual genpd devices, which are created for each consumer device - genpd
- * pair. These are the device structures which are attached to the power domain
- * and are required by the OPP core to set the performance state of the genpd.
- * The same API also works for the case where single genpd is available and so
- * we don't need to support that separately.
- *
- * This helper will normally be called by the consumer driver of the device
- * "dev", as only that has details of the genpd names.
- *
- * This helper needs to be called once with a list of all genpd to attach.
- * Otherwise the original device structure will be used instead by the OPP core.
- *
- * The order of entries in the names array must match the order in which
- * "required-opps" are added in DT.
- */
-static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev,
-			const char * const *names, struct device ***virt_devs)
-{
-	struct device *virt_dev;
-	int index = 0, ret = -EINVAL;
-	const char * const *name = names;
-
-	if (!opp_table->required_devs) {
-		dev_err(dev, "Required OPPs not available, can't attach genpd\n");
-		return -EINVAL;
-	}
-
-	/* Genpd core takes care of propagation to parent genpd */
-	if (opp_table->is_genpd) {
-		dev_err(dev, "%s: Operation not supported for genpds\n", __func__);
-		return -EOPNOTSUPP;
-	}
-
-	/* Checking only the first one is enough ? */
-	if (opp_table->required_devs[0])
-		return 0;
-
-	while (*name) {
-		if (index >= opp_table->required_opp_count) {
-			dev_err(dev, "Index can't be greater than required-opp-count - 1, %s (%d : %d)\n",
-				*name, opp_table->required_opp_count, index);
-			goto err;
-		}
-
-		virt_dev = dev_pm_domain_attach_by_name(dev, *name);
-		if (IS_ERR_OR_NULL(virt_dev)) {
-			ret = virt_dev ? PTR_ERR(virt_dev) : -ENODEV;
-			dev_err(dev, "Couldn't attach to pm_domain: %d\n", ret);
-			goto err;
-		}
-
-		index++;
-		name++;
-	}
-
-	if (virt_devs)
-		*virt_devs = opp_table->required_devs;
-
-	return 0;
-
-err:
-	_opp_detach_genpd(opp_table);
-	return ret;
-
-}
-
 static int _opp_set_required_dev(struct opp_table *opp_table,
 				 struct device *dev,
 				 struct device *required_dev,
@@ -2516,9 +2436,6 @@ static void _opp_clear_config(struct opp_config_data *data)
 {
 	if (data->flags & OPP_CONFIG_REQUIRED_DEV)
 		_opp_put_required_dev(data->opp_table, data->index);
-	else if (data->flags & OPP_CONFIG_GENPD)
-		_opp_detach_genpd(data->opp_table);
-
 	if (data->flags & OPP_CONFIG_REGULATOR)
 		_opp_put_regulators(data->opp_table);
 	if (data->flags & OPP_CONFIG_SUPPORTED_HW)
@@ -2630,18 +2547,7 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
 		data->flags |= OPP_CONFIG_REGULATOR;
 	}
 
-	/* Attach genpds */
-	if (config->genpd_names) {
-		if (config->required_dev)
-			goto err;
-
-		ret = _opp_attach_genpd(opp_table, dev, config->genpd_names,
-					config->virt_devs);
-		if (ret)
-			goto err;
-
-		data->flags |= OPP_CONFIG_GENPD;
-	} else if (config->required_dev && config->required_opp_table) {
+	if (config->required_dev && config->required_opp_table) {
 		ret = _opp_set_required_dev(opp_table, dev,
 					    config->required_dev,
 					    config->required_opp_table);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 5b5a4bd89c9e..318a4ecbabf1 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -34,8 +34,7 @@ extern struct list_head opp_tables;
 #define OPP_CONFIG_REGULATOR_HELPER	BIT(2)
 #define OPP_CONFIG_PROP_NAME		BIT(3)
 #define OPP_CONFIG_SUPPORTED_HW		BIT(4)
-#define OPP_CONFIG_GENPD		BIT(5)
-#define OPP_CONFIG_REQUIRED_DEV		BIT(6)
+#define OPP_CONFIG_REQUIRED_DEV		BIT(5)
 
 /**
  * struct opp_config_data - data for set config operations
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5fade5c4de40..451a7465a605 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -62,11 +62,7 @@ typedef int (*config_clks_t)(struct device *dev, struct opp_table *opp_table,
  * @supported_hw: Array of hierarchy of versions to match.
  * @supported_hw_count: Number of elements in the array.
  * @regulator_names: Array of pointers to the names of the regulator, NULL terminated.
- * @genpd_names: Null terminated array of pointers containing names of genpd to
- *		attach. Mutually exclusive with required_dev.
- * @virt_devs: Pointer to return the array of genpd virtual devices. Mutually
- *		exclusive with required_dev.
- * @required_dev: Required OPP device. Mutually exclusive with genpd_names/virt_devs.
+ * @required_dev: Required OPP device.
  * @required_opp_table: The corresponding required OPP table for @required_dev.
  *
  * This structure contains platform specific OPP configurations for the device.
@@ -80,8 +76,6 @@ struct dev_pm_opp_config {
 	const unsigned int *supported_hw;
 	unsigned int supported_hw_count;
 	const char * const *regulator_names;
-	const char * const *genpd_names;
-	struct device ***virt_devs;
 	struct device *required_dev;
 	struct opp_table *required_opp_table;
 };
@@ -677,36 +671,6 @@ static inline void dev_pm_opp_put_config_regulators(int token)
 	dev_pm_opp_clear_config(token);
 }
 
-/* genpd helpers */
-static inline int dev_pm_opp_attach_genpd(struct device *dev,
-					  const char * const *names,
-					  struct device ***virt_devs)
-{
-	struct dev_pm_opp_config config = {
-		.genpd_names = names,
-		.virt_devs = virt_devs,
-	};
-
-	return dev_pm_opp_set_config(dev, &config);
-}
-
-static inline void dev_pm_opp_detach_genpd(int token)
-{
-	dev_pm_opp_clear_config(token);
-}
-
-static inline int devm_pm_opp_attach_genpd(struct device *dev,
-					   const char * const *names,
-					   struct device ***virt_devs)
-{
-	struct dev_pm_opp_config config = {
-		.genpd_names = names,
-		.virt_devs = virt_devs,
-	};
-
-	return devm_pm_opp_set_config(dev, &config);
-}
-
 /* prop-name helpers */
 static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
 {
-- 
2.34.1

Re: [PATCH 4/4] OPP: Drop redundant _opp_attach|detach_genpd()

[Viresh Kumar]

On 23-07-24, 16:46, Ulf Hansson wrote:
> There no longer any users of _opp_attach|detach_genpd(), hence let's drop
> it along with the corresponding exported functions.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/opp/core.c     | 96 +-----------------------------------------
>  drivers/opp/opp.h      |  3 +-
>  include/linux/pm_opp.h | 38 +----------------
>  3 files changed, 3 insertions(+), 134 deletions(-)

Nice !

-- 
viresh