Hello Rain, On Fri, 10 Oct 2025 17:14:52 +0800 Rain Yang <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: > >> > > >> >> On Thu, 9 Oct 2025 22:00:39 +0800 > >> >> Rain Yang <[email protected]> wrote: > >> >> > >> >> > From: Rain Yang <[email protected]> > >> >> > > >> >> > 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 <[email protected]> > >> >> > Signed-off-by: Rain Yang <[email protected]> > >> >> > --- > >> >> > 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
