* [PATCH] drm/panthor: attach the driver's multiple power domains @ 2025-10-09 14:00 Rain Yang 2025-10-09 14:08 ` Boris Brezillon 0 siblings, 1 reply; 7+ messages in thread From: Rain Yang @ 2025-10-09 14:00 UTC (permalink / raw) To: imx, dri-devel, linux-kernel Cc: boris.brezillon, steven.price, liviu.dudau, maarten.lankhorst, mripard, tzimmermann, airlied, simona, Rain Yang, Prabhu Sundararaj From: Rain Yang <jiyu.yang@nxp.com> Some platforms, such as i.MX95, utilize multiple power domains that need to be attached explicitly. This patch ensures that the driver properly attaches all available power domains using devm_pm_domain_attach_list(). Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> Signed-off-by: Rain Yang <jiyu.yang@nxp.com> --- drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ drivers/gpu/drm/panthor/panthor_device.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c index f0b2da5b2b96..6f40d053b16c 100644 --- a/drivers/gpu/drm/panthor/panthor_device.c +++ b/drivers/gpu/drm/panthor/panthor_device.c @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) if (ret) return ret; + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); + if (ret < 0) { + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); + return ret; + } + ret = panthor_devfreq_init(ptdev); if (ret) return ret; diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index 4fc7cf2aeed5..5ecb541ec67b 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -196,6 +196,8 @@ struct panthor_device { /** @recovery_needed: True when a resume attempt failed. */ atomic_t recovery_needed; } pm; + /** @pm: Power management related data. */ + struct dev_pm_domain_list *pd_list; /** @profile_mask: User-set profiling flags for job accounting. */ u32 profile_mask; -- 2.39.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-09 14:00 [PATCH] drm/panthor: attach the driver's multiple power domains Rain Yang @ 2025-10-09 14:08 ` Boris Brezillon 2025-10-09 14:09 ` Boris Brezillon 0 siblings, 1 reply; 7+ messages in thread From: Boris Brezillon @ 2025-10-09 14:08 UTC (permalink / raw) To: Rain Yang Cc: imx, dri-devel, linux-kernel, steven.price, liviu.dudau, maarten.lankhorst, mripard, tzimmermann, airlied, simona, Rain Yang, Prabhu Sundararaj On Thu, 9 Oct 2025 22:00:39 +0800 Rain Yang <jiyu.yang@oss.nxp.com> wrote: > From: Rain Yang <jiyu.yang@nxp.com> > > Some platforms, such as i.MX95, utilize multiple power domains that need > to be attached explicitly. This patch ensures that the driver properly > attaches all available power domains using devm_pm_domain_attach_list(). > > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> > --- > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index f0b2da5b2b96..6f40d053b16c 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) > if (ret) > return ret; > > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); > + if (ret < 0) { > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); > + return ret; > + } > + > ret = panthor_devfreq_init(ptdev); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > index 4fc7cf2aeed5..5ecb541ec67b 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -196,6 +196,8 @@ struct panthor_device { > /** @recovery_needed: True when a resume attempt failed. */ > atomic_t recovery_needed; > } pm; Add a blank line here. > + /** @pm: Power management related data. */ > + struct dev_pm_domain_list *pd_list; > Do we even need to keep the pd_list in panthor_device if we don't do anything with it? > /** @profile_mask: User-set profiling flags for job accounting. */ > u32 profile_mask; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-09 14:08 ` Boris Brezillon @ 2025-10-09 14:09 ` Boris Brezillon 2025-10-09 15:06 ` Rain Yang 0 siblings, 1 reply; 7+ messages in thread From: Boris Brezillon @ 2025-10-09 14:09 UTC (permalink / raw) To: Rain Yang Cc: imx, dri-devel, linux-kernel, steven.price, liviu.dudau, maarten.lankhorst, mripard, tzimmermann, airlied, simona, Rain Yang, Prabhu Sundararaj On Thu, 9 Oct 2025 16:08:20 +0200 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Thu, 9 Oct 2025 22:00:39 +0800 > Rain Yang <jiyu.yang@oss.nxp.com> wrote: > > > From: Rain Yang <jiyu.yang@nxp.com> > > > > Some platforms, such as i.MX95, utilize multiple power domains that need > > to be attached explicitly. This patch ensures that the driver properly > > attaches all available power domains using devm_pm_domain_attach_list(). > > > > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> > > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> > > --- > > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ > > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > > index f0b2da5b2b96..6f40d053b16c 100644 > > --- a/drivers/gpu/drm/panthor/panthor_device.c > > +++ b/drivers/gpu/drm/panthor/panthor_device.c > > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) > > if (ret) > > return ret; > > > > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); > > + if (ret < 0) { > > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); > > + return ret; > > + } > > + > > ret = panthor_devfreq_init(ptdev); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > > index 4fc7cf2aeed5..5ecb541ec67b 100644 > > --- a/drivers/gpu/drm/panthor/panthor_device.h > > +++ b/drivers/gpu/drm/panthor/panthor_device.h > > @@ -196,6 +196,8 @@ struct panthor_device { > > /** @recovery_needed: True when a resume attempt failed. */ > > atomic_t recovery_needed; > > } pm; > > Add a blank line here. > > > + /** @pm: Power management related data. */ Also, the comment is wrong, and it would probably make sense to move that to the pm sub-struct since this is PM related. > > + struct dev_pm_domain_list *pd_list; > > > > Do we even need to keep the pd_list in panthor_device if we don't do > anything with it? > > > /** @profile_mask: User-set profiling flags for job accounting. */ > > u32 profile_mask; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-09 14:09 ` Boris Brezillon @ 2025-10-09 15:06 ` Rain Yang 2025-10-09 15:23 ` Boris Brezillon 0 siblings, 1 reply; 7+ messages in thread From: Rain Yang @ 2025-10-09 15:06 UTC (permalink / raw) To: Boris Brezillon Cc: airlied, boris.brezillon, dri-devel, imx, linux-kernel, liviu.dudau, maarten.lankhorst, mripard, prabhu.sundararaj, simona, steven.price, tzimmermann, xianzhong.li On Thu, Oct 09, 2025 at 04:09:29PM +0200, Boris Brezillon wrote: >On Thu, 9 Oct 2025 16:08:20 +0200 >Boris Brezillon <boris.brezillon@collabora.com> wrote: > >> On Thu, 9 Oct 2025 22:00:39 +0800 >> Rain Yang <jiyu.yang@oss.nxp.com> wrote: >> >> > From: Rain Yang <jiyu.yang@nxp.com> >> > >> > Some platforms, such as i.MX95, utilize multiple power domains that need >> > to be attached explicitly. This patch ensures that the driver properly >> > attaches all available power domains using devm_pm_domain_attach_list(). >> > >> > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> >> > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> >> > --- >> > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ >> > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ >> > 2 files changed, 8 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c >> > index f0b2da5b2b96..6f40d053b16c 100644 >> > --- a/drivers/gpu/drm/panthor/panthor_device.c >> > +++ b/drivers/gpu/drm/panthor/panthor_device.c >> > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) >> > if (ret) >> > return ret; >> > >> > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); >> > + if (ret < 0) { >> > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); >> > + return ret; >> > + } >> > + >> > ret = panthor_devfreq_init(ptdev); >> > if (ret) >> > return ret; >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h >> > index 4fc7cf2aeed5..5ecb541ec67b 100644 >> > --- a/drivers/gpu/drm/panthor/panthor_device.h >> > +++ b/drivers/gpu/drm/panthor/panthor_device.h >> > @@ -196,6 +196,8 @@ struct panthor_device { >> > /** @recovery_needed: True when a resume attempt failed. */ >> > atomic_t recovery_needed; >> > } pm; >> >> Add a blank line here. >> >> > + /** @pm: Power management related data. */ > >Also, the comment is wrong, and it would probably make sense to move >that to the pm sub-struct since this is PM related. thanks, will fix it next version. > >> > + struct dev_pm_domain_list *pd_list; >> > >> >> Do we even need to keep the pd_list in panthor_device if we don't do >> anything with it? The second power domain is typically used for frequency scaling. The driver works fine when there's only one single power domain. That said, I will update the implementation to attach multiple power domains only when the domain count is greater than one. >> >> > /** @profile_mask: User-set profiling flags for job accounting. */ >> > u32 profile_mask; >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-09 15:06 ` Rain Yang @ 2025-10-09 15:23 ` Boris Brezillon 2025-10-10 9:14 ` Rain Yang 0 siblings, 1 reply; 7+ messages in thread From: Boris Brezillon @ 2025-10-09 15:23 UTC (permalink / raw) To: Rain Yang Cc: airlied, dri-devel, imx, linux-kernel, liviu.dudau, maarten.lankhorst, mripard, prabhu.sundararaj, simona, steven.price, tzimmermann, xianzhong.li On Thu, 9 Oct 2025 23:06:17 +0800 Rain Yang <jiyu.yang@oss.nxp.com> wrote: > On Thu, Oct 09, 2025 at 04:09:29PM +0200, Boris Brezillon wrote: > >On Thu, 9 Oct 2025 16:08:20 +0200 > >Boris Brezillon <boris.brezillon@collabora.com> wrote: > > > >> On Thu, 9 Oct 2025 22:00:39 +0800 > >> Rain Yang <jiyu.yang@oss.nxp.com> wrote: > >> > >> > From: Rain Yang <jiyu.yang@nxp.com> > >> > > >> > Some platforms, such as i.MX95, utilize multiple power domains that need > >> > to be attached explicitly. This patch ensures that the driver properly > >> > attaches all available power domains using devm_pm_domain_attach_list(). > >> > > >> > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> > >> > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> > >> > --- > >> > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ > >> > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ > >> > 2 files changed, 8 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > >> > index f0b2da5b2b96..6f40d053b16c 100644 > >> > --- a/drivers/gpu/drm/panthor/panthor_device.c > >> > +++ b/drivers/gpu/drm/panthor/panthor_device.c > >> > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) > >> > if (ret) > >> > return ret; > >> > > >> > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); > >> > + if (ret < 0) { > >> > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); > >> > + return ret; > >> > + } > >> > + > >> > ret = panthor_devfreq_init(ptdev); > >> > if (ret) > >> > return ret; > >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > >> > index 4fc7cf2aeed5..5ecb541ec67b 100644 > >> > --- a/drivers/gpu/drm/panthor/panthor_device.h > >> > +++ b/drivers/gpu/drm/panthor/panthor_device.h > >> > @@ -196,6 +196,8 @@ struct panthor_device { > >> > /** @recovery_needed: True when a resume attempt failed. */ > >> > atomic_t recovery_needed; > >> > } pm; > >> > >> Add a blank line here. > >> > >> > + /** @pm: Power management related data. */ > > > >Also, the comment is wrong, and it would probably make sense to move > >that to the pm sub-struct since this is PM related. > thanks, will fix it next version. > > > >> > + struct dev_pm_domain_list *pd_list; > >> > > >> > >> Do we even need to keep the pd_list in panthor_device if we don't do > >> anything with it? > The second power domain is typically used for frequency scaling. The driver > works fine when there's only one single power domain. That said, I will update > the implementation to attach multiple power domains only when the domain count > is greater than one. That's not what I meant. What I mean is that the panthor_device::pd_list is never used, because the extra power domains get attached to the struct device directly, and the PM core automatically enables all PDs on resume and disable them on suspend. Because it's a devm_ function, you don't need it to detach the pd_list at ::remove() time either. TLDR; that means you can pass devm_pm_domain_attach_list() a local pd_list instead of ptdev->pd_list and you can thus drop panthor_device::pd_list altogether, I think. If you intend to use the pd_list for manual PD control in panthor in a follow-up patchset, this should be mentioned in the commit message. > >> > >> > /** @profile_mask: User-set profiling flags for job accounting. */ > >> > u32 profile_mask; > >> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-09 15:23 ` Boris Brezillon @ 2025-10-10 9:14 ` Rain Yang 2025-10-10 9:31 ` Boris Brezillon 0 siblings, 1 reply; 7+ messages in thread From: Rain Yang @ 2025-10-10 9:14 UTC (permalink / raw) To: Boris Brezillon Cc: airlied, boris.brezillon, dri-devel, imx, linux-kernel, liviu.dudau, maarten.lankhorst, mripard, prabhu.sundararaj, simona, steven.price, tzimmermann, xianzhong.li On Thu, Oct 09, 2025 at 05:23:20PM +0200, Boris Brezillon wrote: >On Thu, 9 Oct 2025 23:06:17 +0800 >Rain Yang <jiyu.yang@oss.nxp.com> wrote: > >> On Thu, Oct 09, 2025 at 04:09:29PM +0200, Boris Brezillon wrote: >> >On Thu, 9 Oct 2025 16:08:20 +0200 >> >Boris Brezillon <boris.brezillon@collabora.com> wrote: >> > >> >> On Thu, 9 Oct 2025 22:00:39 +0800 >> >> Rain Yang <jiyu.yang@oss.nxp.com> wrote: >> >> >> >> > From: Rain Yang <jiyu.yang@nxp.com> >> >> > >> >> > Some platforms, such as i.MX95, utilize multiple power domains that need >> >> > to be attached explicitly. This patch ensures that the driver properly >> >> > attaches all available power domains using devm_pm_domain_attach_list(). >> >> > >> >> > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> >> >> > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> >> >> > --- >> >> > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ >> >> > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ >> >> > 2 files changed, 8 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c >> >> > index f0b2da5b2b96..6f40d053b16c 100644 >> >> > --- a/drivers/gpu/drm/panthor/panthor_device.c >> >> > +++ b/drivers/gpu/drm/panthor/panthor_device.c >> >> > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) >> >> > if (ret) >> >> > return ret; >> >> > >> >> > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); >> >> > + if (ret < 0) { >> >> > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); >> >> > + return ret; >> >> > + } >> >> > + >> >> > ret = panthor_devfreq_init(ptdev); >> >> > if (ret) >> >> > return ret; >> >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h >> >> > index 4fc7cf2aeed5..5ecb541ec67b 100644 >> >> > --- a/drivers/gpu/drm/panthor/panthor_device.h >> >> > +++ b/drivers/gpu/drm/panthor/panthor_device.h >> >> > @@ -196,6 +196,8 @@ struct panthor_device { >> >> > /** @recovery_needed: True when a resume attempt failed. */ >> >> > atomic_t recovery_needed; >> >> > } pm; >> >> >> >> Add a blank line here. >> >> >> >> > + /** @pm: Power management related data. */ >> > >> >Also, the comment is wrong, and it would probably make sense to move >> >that to the pm sub-struct since this is PM related. >> thanks, will fix it next version. >> > >> >> > + struct dev_pm_domain_list *pd_list; >> >> > >> >> >> >> Do we even need to keep the pd_list in panthor_device if we don't do >> >> anything with it? >> The second power domain is typically used for frequency scaling. The driver >> works fine when there's only one single power domain. That said, I will update >> the implementation to attach multiple power domains only when the domain count >> is greater than one. > >That's not what I meant. What I mean is that the >panthor_device::pd_list is never used, because the extra power domains >get attached to the struct device directly, and the PM core >automatically enables all PDs on resume and disable them on suspend. >Because it's a devm_ function, you don't need it to detach the pd_list >at ::remove() time either. TLDR; that means you can pass >devm_pm_domain_attach_list() a local pd_list instead of ptdev->pd_list >and you can thus drop panthor_device::pd_list altogether, I think. > >If you intend to use the pd_list for manual PD control in panthor in a >follow-up patchset, this should be mentioned in the commit message. thanks for your suggestion, pd_list will be allocated by devm_kmalloc. I have not found a general solution to modify the frequency via SCMI perf and OPP framework except the manual PD control, I'd appreciate it if there is any idea. > >> >> >> >> > /** @profile_mask: User-set profiling flags for job accounting. */ >> >> > u32 profile_mask; >> >> >> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/panthor: attach the driver's multiple power domains 2025-10-10 9:14 ` Rain Yang @ 2025-10-10 9:31 ` Boris Brezillon 0 siblings, 0 replies; 7+ messages in thread From: Boris Brezillon @ 2025-10-10 9:31 UTC (permalink / raw) To: Rain Yang Cc: airlied, dri-devel, imx, linux-kernel, liviu.dudau, maarten.lankhorst, mripard, prabhu.sundararaj, simona, steven.price, tzimmermann, xianzhong.li Hello Rain, On Fri, 10 Oct 2025 17:14:52 +0800 Rain Yang <jiyu.yang@oss.nxp.com> wrote: > On Thu, Oct 09, 2025 at 05:23:20PM +0200, Boris Brezillon wrote: > >On Thu, 9 Oct 2025 23:06:17 +0800 > >Rain Yang <jiyu.yang@oss.nxp.com> wrote: > > > >> On Thu, Oct 09, 2025 at 04:09:29PM +0200, Boris Brezillon wrote: > >> >On Thu, 9 Oct 2025 16:08:20 +0200 > >> >Boris Brezillon <boris.brezillon@collabora.com> wrote: > >> > > >> >> On Thu, 9 Oct 2025 22:00:39 +0800 > >> >> Rain Yang <jiyu.yang@oss.nxp.com> wrote: > >> >> > >> >> > From: Rain Yang <jiyu.yang@nxp.com> > >> >> > > >> >> > Some platforms, such as i.MX95, utilize multiple power domains that need > >> >> > to be attached explicitly. This patch ensures that the driver properly > >> >> > attaches all available power domains using devm_pm_domain_attach_list(). > >> >> > > >> >> > Signed-off-by: Prabhu Sundararaj <prabhu.sundararaj@nxp.com> > >> >> > Signed-off-by: Rain Yang <jiyu.yang@nxp.com> > >> >> > --- > >> >> > drivers/gpu/drm/panthor/panthor_device.c | 6 ++++++ > >> >> > drivers/gpu/drm/panthor/panthor_device.h | 2 ++ > >> >> > 2 files changed, 8 insertions(+) > >> >> > > >> >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > >> >> > index f0b2da5b2b96..6f40d053b16c 100644 > >> >> > --- a/drivers/gpu/drm/panthor/panthor_device.c > >> >> > +++ b/drivers/gpu/drm/panthor/panthor_device.c > >> >> > @@ -218,6 +218,12 @@ int panthor_device_init(struct panthor_device *ptdev) > >> >> > if (ret) > >> >> > return ret; > >> >> > > >> >> > + ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &ptdev->pd_list); > >> >> > + if (ret < 0) { > >> >> > + drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); > >> >> > + return ret; > >> >> > + } > >> >> > + > >> >> > ret = panthor_devfreq_init(ptdev); > >> >> > if (ret) > >> >> > return ret; > >> >> > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > >> >> > index 4fc7cf2aeed5..5ecb541ec67b 100644 > >> >> > --- a/drivers/gpu/drm/panthor/panthor_device.h > >> >> > +++ b/drivers/gpu/drm/panthor/panthor_device.h > >> >> > @@ -196,6 +196,8 @@ struct panthor_device { > >> >> > /** @recovery_needed: True when a resume attempt failed. */ > >> >> > atomic_t recovery_needed; > >> >> > } pm; > >> >> > >> >> Add a blank line here. > >> >> > >> >> > + /** @pm: Power management related data. */ > >> > > >> >Also, the comment is wrong, and it would probably make sense to move > >> >that to the pm sub-struct since this is PM related. > >> thanks, will fix it next version. > >> > > >> >> > + struct dev_pm_domain_list *pd_list; > >> >> > > >> >> > >> >> Do we even need to keep the pd_list in panthor_device if we don't do > >> >> anything with it? > >> The second power domain is typically used for frequency scaling. The driver > >> works fine when there's only one single power domain. That said, I will update > >> the implementation to attach multiple power domains only when the domain count > >> is greater than one. > > > >That's not what I meant. What I mean is that the > >panthor_device::pd_list is never used, because the extra power domains > >get attached to the struct device directly, and the PM core > >automatically enables all PDs on resume and disable them on suspend. > >Because it's a devm_ function, you don't need it to detach the pd_list > >at ::remove() time either. TLDR; that means you can pass > >devm_pm_domain_attach_list() a local pd_list instead of ptdev->pd_list > >and you can thus drop panthor_device::pd_list altogether, I think. > > > >If you intend to use the pd_list for manual PD control in panthor in a > >follow-up patchset, this should be mentioned in the commit message. > thanks for your suggestion, pd_list will be allocated by devm_kmalloc. There's no extra allocation needed. Just do: struct dev_pm_domain_list *pd_list; ... ret = devm_pm_domain_attach_list(ptdev->base.dev, NULL, &pd_list); if (ret < 0) { drm_err(&ptdev->base, "attach power domains failed, ret=%d", ret); return ret; } and that's it. > > I have not found a general solution to modify the frequency via SCMI perf > and OPP framework except the manual PD control, I'd appreciate it if > there is any idea. What's the manual PD control you're talking about? I don't see anything using the pd_list in this patch. Is there another patchset on top of this one that you haven't posted yet? Dunno if that's of any help, but this patchset [1] might give you some ideas. Regards, Boris [1]https://lwn.net/Articles/1040831 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-10 9:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-09 14:00 [PATCH] drm/panthor: attach the driver's multiple power domains Rain Yang 2025-10-09 14:08 ` Boris Brezillon 2025-10-09 14:09 ` Boris Brezillon 2025-10-09 15:06 ` Rain Yang 2025-10-09 15:23 ` Boris Brezillon 2025-10-10 9:14 ` Rain Yang 2025-10-10 9:31 ` Boris Brezillon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).