Re: [PATCH v2 2/2] arm64: dts: mediatek: Introduce MT8188 Geralt platform based Ciri

[AngeloGioacchino Del Regno]

Il 11/11/24 08:10, Fei Shao ha scritto:
> On Fri, Nov 8, 2024 at 12:11 PM Fei Shao <fshao@xxxxxxxxxxxx> wrote:
> > On Thu, Nov 7, 2024 at 6:37 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > > Il 07/11/24 07:58, Fei Shao ha scritto:
> > > > On Wed, Nov 6, 2024 at 9:19 PM AngeloGioacchino Del Regno
> > > > <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > > > > Il 05/11/24 10:30, Fei Shao ha scritto:
> > > > > > Introduce MT8188-based Chromebook Ciri, also known commercially as
> > > > > > Lenovo Chromebook Duet (11", 9).
> > > > > >
> > > > > > Ciri is a detachable device based on the Geralt design, where Geralt is
> > > > > > the codename for the MT8188 platform. Ciri offers 8 SKUs to accommodate
> > > > > > different combinations of second-source components, including:
> > > > > > - audio codecs (RT5682S and ES8326)
> > > > > > - speaker amps (TAS2563 and MAX98390)
> > > > > > - MIPI-DSI panels (BOE nv110wum-l60 and IVO t109nw41)
> > > > > >
> > > > > > Signed-off-by: Fei Shao <fshao@xxxxxxxxxxxx>
> > > > > > ---
> [...]
> > > > > > +&pmic {
> > > > > > +     interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +};
> > > > > > +
> > > > > > +&scp {
> > > > >
> > > > > Is this SCP-dual or SCP?
> > > > > I see SCP, but I also see a SCP-Dual memory region... what's going on here?
> > > > >
> > > > > Of course, the SCP-Dual won't work if you don't override the compatible string...
> > > >
> > > > To clarify, the second SCP core is used for MIPI camera in downstream,
> > > > and I deliberately only describe the first SCP core here since the MTK
> > > > camera ISP driver isn't in upstream at the moment.
> > > > I had a fixup patch for removing the scp-dual reserved memory region,
> > > > but likely it was missing during the rebase... let me check again if
> > > > it can be removed, just in case there's firmware protecting the region
> > > > and the kernel shouldn't access it.
> > >
> > > Hmm... but the second SCP core can still be brought up, even if the MIPI Camera
> > > driver is not upstreamed yet, right?
> >
> > Well, that's true... and it should pave the way for validating the
> > driver with the upstreamed DT whenever that becomes available.
> >
> > > That shouldn't cause lockups and/or other kinds of bad behavior, and should
> > > bring up a core and just never use it, without any particular issues.
> > >
> > > If we can enable the secondary core, let's just go for it.. as that will help
> > > specifying the exact memory layout of this board (and failing to do that may
> > > create some other issues, that's why I'm proposing to enable that even if it
> > > is not really used in this case).
> > >
> > > What do you think? :-)
> >
> > Sure, that sounds good to me, too.
> > I started only with the essential DT bits to ensure the device can
> > boot, which it does, so I guess it's time to bring that back. I'll
> > incorporate that in v3.
> > I plan to fix up the single SCP core node to SCP-dual directly, so
> > please let me know if you prefer seeing that as an individual patch on
> > top (either option works for me).
>
> In fact, I noticed that it seems to require modifying mt8188.dtsi (and
> potentially mt8390-genio-700-evk.dts) to support the second SCP core,
> but I want to avoid doing so in this series if possible to keep this
> as a pure new .dts introduction (if that makes sense).
>
> I can think of 3 options here:
> 1. I resend this series *with* the single SCP core enabled in
> -geralt.dtsi. And then I send a follow-up series to introduce the
> second SCP core and update the affected .dts{,i}.
> 2. I resend this series *without* any SCP cores in -geralt.dtsi. And
> then I send a follow-up series to introduce both of the SCP cores at
> once, and update the affected .dts{,i}.
> 3. I delete the parent (mt8188) scp declaration and re-describe the
> dual-core SCP structure in -geralt.dtsi. This avoids touching
> mt8188.dtsi and mt8390-genio-700-evk but leaves the dual-core SCP
> stuff exclusively to geralt/ciri. I don't know if MT8390 wants to
> utilize the second SCP core or not.
>
> I guess (3) is less likely what we want down the line, but that's just
> for reference. Any preference/suggestion?

I want to verify with MediaTek whether the Genio 700 EVK can make use of the
dual-core SCP first, as it is highly possible that it indeed can and, if that
is the case, counting that any product development would be anyway based on
the EVK.... we will probably never see single-core SCP in MT8188 devicetrees
ever again.

Please send the Geralt/Ciri devicetrees without SCP support for now, so that
we can get the vast majority of the code upstream - which anyway is likely
95% of what you have right now... then I will either notify you for how to
proceed with the SCP (especially if I come to a conclusion before you send
the new series), or I will just send the SCP DT series on my own.

Cheers,
Angelo