[PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains

[Lucas Stach]

Hi Laurent,

Am Sonntag, dem 20.02.2022 um 13:35 +0200 schrieb Laurent Pinchart:
> Hi Lucas,
> 
> On Sat, Feb 19, 2022 at 01:02:21AM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> > > This adds driver support for all the GPC power domains found on
> > > the i.MX8MP SoC.
> > > 
> > > Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> > > ---
> > >  drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 386 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > > index 01f46b078df3..a7c92bdfc53b 100644
> > > --- a/drivers/soc/imx/gpcv2.c
> > > +++ b/drivers/soc/imx/gpcv2.c
> 
> [snip]
> 
> > > @@ -137,6 +183,21 @@
> > >  #define IMX8MN_DISPMIX_HSK_PWRDNREQN		BIT(7)
> > >  #define IMX8MN_HSIO_HSK_PWRDNREQN		BIT(5)
> > >  
> > > +#define IMX8MP_MEDIAMIX_PWRDNACKN		BIT(3)
> > 
> > This should be bit 30.
> 
> With this fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> with a soon to be posted driver for the MEDIA_BLK_CTRL.
> 
> While this shouldn't be a blocker, I'm wondering how we should deal with
> the NOC configuration that TF-A handles in the power domain code ([1]).
> The reference manual doesn't document the registers, which doesn't help.
> 
Yes, that doesn't help. My hope was that at some point we could get
around to add proper interconnect drivers for those NoC nodes and have
the description for those scheduling parameters in the DT, but without
any documentation this will probably be a hard nut to crack.

> There are also two registers in the MEDIA_BLK_CTRL that are specific to
> the LCDIF and ISI, see [2]. Would you recommend dealing with them in the
> imx8m-blk-ctrl driver (maybe in the power domain notifier, the same way
> we set bit 8 in the CLK_EN register), or through a syscon phandle
> directly in the LCDIF and ISI drivers ?

For now I think it would be good enough to initialize those registers
in the power domain notifier. I don't think the ISI or LCDIF drivers
have any more information available that would make it beneficial to
change those values on the fly. As long as they are just static init
values, writing them once from the PM notifier should be good enough.

Regards,
Lucas

> 
> [1] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n156
> [2] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n146
> 
> > > +#define IMX8MP_HDMIMIX_PWRDNACKN		BIT(29)
> > > +#define IMX8MP_HSIOMIX_PWRDNACKN		BIT(28)
> > > +#define IMX8MP_VPUMIX_PWRDNACKN			BIT(26)
> > > +#define IMX8MP_GPUMIX_PWRDNACKN			BIT(25)
> > > +#define IMX8MP_MLMIX_PWRDNACKN			(BIT(23) | BIT(24))
> > > +#define IMX8MP_AUDIOMIX_PWRDNACKN		(BIT(20) | BIT(31))
> > > +#define IMX8MP_MEDIAMIX_PWRDNREQN		BIT(14)
> > > +#define IMX8MP_HDMIMIX_PWRDNREQN		BIT(13)
> > > +#define IMX8MP_HSIOMIX_PWRDNREQN		BIT(12)
> > > +#define IMX8MP_VPUMIX_PWRDNREQN			BIT(10)
> > > +#define IMX8MP_GPUMIX_PWRDNREQN			BIT(9)
> > > +#define IMX8MP_MLMIX_PWRDNREQN			(BIT(7) | BIT(8))
> > > +#define IMX8MP_AUDIOMIX_PWRDNREQN		(BIT(4) | BIT(15))
> > > +
> > >  /*
> > >   * The PGC offset values in Reference Manual
> > >   * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> 
> [snip]
>