diff mbox series

[v3,07/10] drm/tegra: gr3d: Convert into devm_pm_domain_attach_list()

Message ID 20240822224547.385095-8-ulf.hansson@linaro.org
State New
Headers show
Series OPP/pmdomain: Assign required_devs for required OPPs through genpd | expand

Commit Message

Ulf Hansson Aug. 22, 2024, 10:45 p.m. UTC 
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 devm_pm_domain_attach_list().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- Updated commitmsg.
	- Converted to devm mangaged version of dev_pm_domain_attach_list()

---
 drivers/gpu/drm/tegra/gr3d.c | 38 +++++++-----------------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

Comments

Thierry Reding Aug. 23, 2024, 11:53 a.m. UTC | #1 
On Fri, Aug 23, 2024 at 12:45:44AM 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 devm_pm_domain_attach_list().
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v3:
> 	- Updated commitmsg.
> 	- Converted to devm mangaged version of dev_pm_domain_attach_list()
> 
> ---
>  drivers/gpu/drm/tegra/gr3d.c | 38 +++++++-----------------------------
>  1 file changed, 7 insertions(+), 31 deletions(-)

I'm not very familiar with most of the OPP bits in this driver, but it
looks like the corresponding code is now in the core, so this seems
fine:

Acked-by: Thierry Reding <treding@nvidia.com>

On a related note: we have two other case on Tegra where we attach to
multiple PM domains (drivers/usb/host/xhci-tegra.c and
drivers/usb/gadget/udc/tegra-xudc.c). Both of those don't use OPP, but
I wonder if they could also be simplified using the new
devm_pm_domain_attach_list() function?

Thierry
Ulf Hansson Aug. 26, 2024, 10:57 a.m. UTC | #2 
On Fri, 23 Aug 2024 at 13:53, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Fri, Aug 23, 2024 at 12:45:44AM 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 devm_pm_domain_attach_list().
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >
> > Changes in v3:
> >       - Updated commitmsg.
> >       - Converted to devm mangaged version of dev_pm_domain_attach_list()
> >
> > ---
> >  drivers/gpu/drm/tegra/gr3d.c | 38 +++++++-----------------------------
> >  1 file changed, 7 insertions(+), 31 deletions(-)
>
> I'm not very familiar with most of the OPP bits in this driver, but it
> looks like the corresponding code is now in the core, so this seems
> fine:
>
> Acked-by: Thierry Reding <treding@nvidia.com>

Thanks!

>
> On a related note: we have two other case on Tegra where we attach to
> multiple PM domains (drivers/usb/host/xhci-tegra.c and
> drivers/usb/gadget/udc/tegra-xudc.c). Both of those don't use OPP, but
> I wonder if they could also be simplified using the new
> devm_pm_domain_attach_list() function?

Yes, they certainly can!

I was planning to send a couple of patches simplifying a couple of
more drivers here and there (including those on Tegra), but I haven't
yet made it to it. If you do it before me, feel free to keep me on cc.

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 00c8564520e7..d52433cf939a 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 = devm_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;
 }