Re: [PATCH] soc: imx: Add power domain driver support for i.mx8m family

[Tim Harvey]

On Mon, May 4, 2020 at 2:19 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
>
> Am Montag, den 27.04.2020, 15:37 +0000 schrieb Jacky Bai:
> > > -----Original Message-----
> > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > > Sent: Monday, April 27, 2020 11:11 PM
> > > To: Abel Vesa <abel.vesa@xxxxxxx>; Jacky Bai <ping.bai@xxxxxxx>; Shawn
> > > Guo <shawnguo@xxxxxxxxxx>; Sascha Hauer <kernel@xxxxxxxxxxxxxx>; Liam
> > > Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>
> > > Cc: Aisheng Dong <aisheng.dong@xxxxxxx>; dl-linux-imx
> > > <linux-imx@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Linux Kernel
> > > Mailing List <linux-kernel@xxxxxxxxxxxxxxx>
> > > Subject: Re: [PATCH] soc: imx: Add power domain driver support for i.mx8m
> > > family
> > >
> > > Am Montag, den 27.04.2020, 17:58 +0300 schrieb Abel Vesa:
> > > > From: Jacky Bai <ping.bai@xxxxxxx>
> > > >
> > > > The i.MX8M family is a set of NXP product focus on delivering the
> > > > latest and greatest video and audio experience combining
> > > > state-of-the-art media-specific features with high-performance
> > > > processing while optimized for lowest power consumption.
> > > >
> > > > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > > > this family. A GPC module is used to manage all the PU power domain
> > > > on/off. But the situation is that the number of power domains & the
> > > > power up sequence has significate difference on those SoCs. Even on
> > > > the same SoC. The power up sequence still has big difference. It makes
> > > > us hard to reuse the GPCv2 driver to cover the whole i.MX8M family.
> > > > Each time a new SoC is supported in the mainline kernel, we need to
> > > > modify the GPCv2 driver to support it. We need to add or modify
> > > > hundred lines of code in worst case.
> > > > It is a bad practice for the driver maintainability.
> > > >
> > > > This driver add a more generic power domain driver that the actual
> > > > power on/off is done by TF-A code. the abstraction give us the
> > > > possibility that using one driver to cover the whole i.MX8M family in
> > > > kernel side.
> > > >
> > >
> > > Again: what does this driver bring to the table, other than moving a fraction of
> > > the power domain functionality into the firmware?
> > >
> > > The discussions on the last submissions of this driver already established that
> > > we can't move all functionality for the power domains into the firmware, as
> > > controlling regulators is probably not easy to do from this context. Also the
> > > TF-A side implementation of this driver is "interesting" IMHO, it does stuff like
> > > accessing the clock controller registers without any locking or other means of
> > > mutual exclusion with the Linux kernel clock controller driver.
> > >
> >
> > The clock handling is in kernel side through CCF, not in ATF. See the patch below.
> >
> > > Why can't we just extend the existing GPCv2 driver with support for the other
> > > i.MX8M family members?
> > >
> >
> > The reason that why I donât like to extend the GPCv2 is that when doing domain on/off,
> > We need to access some special control register in each domain & do some special flow,
> > These control register(mediamix block control, vpumix block control) is not in GPC
> > module's address range. No benefit to reuse the GPCv2. Only bring complexity to the
> > GPCv2 driver each time a new SoC is added.
> >
> > Yes, the i.MX8M power domain support has been pending for a while. ARM guys rejected this patchset
> > because they suggest us to use SCMI rather than SiP. But SCMI is only partial suitable for our
> > case.
>
> Can you please point me to the most resent version of the TF-A side
> implementation of this? The i.MX8MM implementation in the
> imx_5.4.3_2.0.0 branch in the codeaurora imx-atf repo still contains
> writes to the clock controller register range.
>
> Also I would love to learn why the GPC needs to access Mediamix and
> VPUmix domain registers. If you are talking about the NOC configuration
> I would strongly suggest that those should be handled by a Linux side
> interconnect driver, this has nothing to do with the power domain
> sequencing, it just happens to lose state over the power down and needs
> to be reprogrammed after power on. The NOC configuration though is use-
> case dependent, so this should be properly handled in a rich OS driver.
>
> Sure we needs to extend the Linux side GPC driver for each new SoC
> generation, but that's no different from any other hardware driver in
> Linux. Drivers are the abstraction around the hardware, there is no
> need to invent another one if there are no clear benefits.
>

Jacky / Abel,

Any movement on this? As I see it the lack of imx8mm power-domain
support in the kernel is holding up USB, PCIe, VPU, and perhaps
GPU/CSI as well. I would tend to agree that hiding this functionality
in the TF-A is probably not the best approach especially as that
requires a NXP version of the TF-A. I really don't see the issue with
the gpc driver getting a little more complicated if it needs to. There
is bound to be some complication as there is such a large variation of
IMX8 products out there! (talk about confusing!).

Best Regards,

Tim