Re: [PATCH 0/5] imx: support noc settings with power domain

[Lucas Stach]

Am Mittwoch, dem 06.04.2022 um 10:55 +0000 schrieb Peng Fan:
> > Subject: Re: [PATCH 0/5] imx: support noc settings with power domain
> > 
> > Hi Peng,
> > 
> > Am Mittwoch, dem 06.04.2022 um 16:23 +0800 schrieb Peng Fan (OSS):
> > > From: Peng Fan <peng.fan@xxxxxxx>
> > > 
> > > i.MX8MP has a design that NoC(Not main NoC) is distributed in multiple
> > > blocks, such as vpumix, hsiomix and etc. The access to NoC requires
> > > power domain on and blk ctrl settings configured.
> > > 
> > > So the design here is for mixes that not have blk-ctrl, configure the
> > > NoC in gpcv2 driver, for mixes that have blk-ctrl, configure the NoC
> > > in blk-ctrl drivers.
> > > 
> > > This v1 patchset not apply on Shawn's tree, I picked up Lucas's HSIO
> > > and Laurent's mediablk patches, then worked out this patchset:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > > 
> > ub.com%2FMrVan%2Flinux%2Ftree%2Fnoc-imx8mp&amp;data=04%7C01%7
> > Cpeng.fan
> > > %40nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b
> > 4c6fa92cd9
> > > 
> > 9c5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbG
> > Zsb3d8eyJWIj
> > > 
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> > 000&am
> > > 
> > p;sdata=ZVeHFy%2FEaWPhAj%2BURGIDXoWYdX5eeQoEIeZYZoxPPNo%3D&a
> > mp;reserve
> > > d=0
> > > 
> > > Note: This interconnect related functions not added. This patchset is
> > > only to replace the function did in NXP downstream:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> > > 
> > ce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > Fimx
> > > 
> > 8m%2Fimx8mp%2Fgpc.c%3Fh%3Dlf_v2.4%23n157&amp;data=04%7C01%7C
> > peng.fan%4
> > > 
> > 0nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b4c6fa
> > 92cd99c
> > > 
> > 5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbGZs
> > b3d8eyJWIjoi
> > > 
> > MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> > 00&amp;
> > > 
> > sdata=eLawc3SJQBRmVwcOA2%2B6u6d2ZaYxqcO4Gm%2FqEJpqxFE%3D&a
> > mp;reserved=
> > > 0
> > 
> > As a general comment I think this is implemented the wrong way around.
> > 
> > Neither GPC, nor the blk-ctrl should poke into the NoC registers directly. The
> > NoC driver should attach itself to the power domain via a notifier (same as
> > the blk-ctrl does with the GPC domains) and should do the necessary NoC
> > configuration when the power domain is powered up.
> 
> If separate NoC in a standalone driver, NoC may be configured not as early as
> power domain up. Saying lcdif is running, NoC driver probe starts w/o defer
> probe.

The right way to solve this would be to actually implement the
interconnect bits, so that consumers like the LCDIF that have specific
NOC bandwidth/latency requirements could request them from the NoC
driver. Proper probe deferral would come naturally with this.

The static NoC configuration per domain is quite a cludge IHMO, maybe
due to the decision to not open up any information about this part of
the SoC. Spreading support for this hack into multiple drivers doesn't
sound like a direction we want to take for upstream. At minimum we
could try to define the interconnect DT bits, so that the LCDIF driver,
etc. could attach to the NoC driver, giving us proper probe defer
behavior, even if the actual configuration is still static.

Regards,
Lucas