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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 18/11/24 06:26, Fei Shao ha scritto:
> On Thu, Nov 14, 2024 at 6:09 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 11/11/24 08:10, Fei Shao ha scritto:
>>> On Fri, Nov 8, 2024 at 12:11 PM Fei Shao <fshao@chromium.org> wrote:
>>>>
>>>> On Thu, Nov 7, 2024 at 6:37 PM AngeloGioacchino Del Regno
>>>> <angelogioacchino.delregno@collabora.com> 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@collabora.com> 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@chromium.org>
>>>>>>>> ---
>>> [...]
>>>>>>>> +&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.
> 
> Noted. This is currently on the back burner as I have other tasks at
> hand, but I'll get back to it ASAP (hopefully later this week).
> 

There's no rush. It's too late for 6.13 anyway, so take your time :-)


> Thanks,
> Fei
> 
>>
>> Cheers,
>> Angelo
>>