Hi Ulf, On Mon, Jan 27, 2020 at 3:55 PM Ulf Hansson <ulf.hans...@linaro.org> wrote: > > On Fri, 10 Jan 2020 at 02:53, Nicolas Boichat <drink...@chromium.org> wrote: > > > > +Ulf to keep me honest on the power domains > > > > On Thu, Jan 9, 2020 at 10:08 PM Steven Price <steven.pr...@arm.com> wrote: > > > > > > On 08/01/2020 05:23, Nicolas Boichat wrote: > > > > When there is a single power domain per device, the core will > > > > ensure the power domains are all switched on. > > > > > > > > However, when there are multiple ones, as in MT8183 Bifrost GPU, > > > > we need to handle them in driver code. > > > > > > > > > > > > Signed-off-by: Nicolas Boichat <drink...@chromium.org> > > > > --- > > > > > > > > The downstream driver we use on chromeos-4.19 currently uses 2 > > > > additional devices in device tree to accomodate for this [1], but > > > > I believe this solution is cleaner. > > > > > > I'm not sure what is best, but it seems odd to encode this into the > > > Panfrost driver itself - it doesn't have any knowledge of what to do with > > > these power domains. The naming of the domains looks suspiciously like > > > someone thought that e.g. only half of the cores could be powered, but it > > > doesn't look like that was implemented in the chromeos driver linked and > > > anyway that is *meant* to be automatic in the hardware! (I.e. if you only > > > power up one cores in one core stack then the PDC should only enable the > > > power domain for that set of cores). > > > > This is actually implemented in the Chrome OS driver [1]. IMHO power > > domains are a bit confusing [2]: > > i. If there's only 1 power domain in the device, then the core takes > > care of power on the domain (based on pm_runtime) > > ii. If there's more than 1 power domain, then the device needs to > > link the domains manually. > > > > So the Chrome OS [1] driver takes approach (i), by creating 3 devices, > > each with 1 power domain that is switched on/off automatically using > > pm_runtime. > > > > This patch takes approach (ii) with device links to handle the extra > > domains. > > > > I believe the latter is more upstream-friendly, but, as always, > > suggestions welcome. > > Apologies for the late reply. A few comments below.
No worries, than for the helpful reply! > If the device is partitioned across multiple PM domains (it may need > several power rails), then that should be described with the "multi PM > domain" approach in the DTS. As in (ii). > > Using "device links" is however optional, as it may depend on the use > case. If all multiple PM domains needs to be powered on/off together, > then it's certainly recommended to use device links. That's the case here, there's no support for turning on/off the domains individually. > However, if the PM domains can be powered on/off independently (one > can be on while another is off), then it's probably easier to operate > directly with runtime PM, on the returned struct *device from > dev_pm_domain_attach_by_id(). > > Also note, there is dev_pm_domain_attach_by_name(), which allows us to > specify a name for the PM domain in the DTS, rather than using an > index. This may be more future proof to use. Agree, probably better to have actual names than just "counting" the number of domains like I do, especially as we have a compatible struct anyway. I'll update the patch. > [...] > > Hope this helps. > > Kind regards > Uffe _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
