Re: [PATCH v1 1/3] platform/x86/amd/pmc: Add support for Van Gogh SoC

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 10/24/2025 22:02, Mario Limonciello wrote:
> 
> 
> On 10/24/2025 11:08 AM, Antheas Kapenekakis wrote:
>> On Fri, 24 Oct 2025 at 17:43, Mario Limonciello
>> <mario.limonciello@amd.com> wrote:
>>>
>>>
>>>
>>> On 10/24/2025 10:21 AM, Antheas Kapenekakis wrote:
>>>> The ROG Xbox Ally (non-X) SoC features a similar architecture to the
>>>> Steam Deck. While the Steam Deck supports S3 (s2idle causes a crash),
>>>> this support was dropped by the Xbox Ally which only S0ix suspend.
>>>>
>>>> Since the handler is missing here, this causes the device to not
>>>> suspend
>>>> and the AMD GPU driver to crash while trying to resume afterwards
>>>> due to
>>>> a power hang.
>>>>
>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4659
>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>> ---
>>>>    drivers/platform/x86/amd/pmc/pmc.c | 3 +++
>>>>    drivers/platform/x86/amd/pmc/pmc.h | 1 +
>>>>    2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/
>>>> platform/x86/amd/pmc/pmc.c
>>>> index bd318fd02ccf..cae3fcafd4d7 100644
>>>> --- a/drivers/platform/x86/amd/pmc/pmc.c
>>>> +++ b/drivers/platform/x86/amd/pmc/pmc.c
>>>> @@ -106,6 +106,7 @@ static void amd_pmc_get_ip_info(struct
>>>> amd_pmc_dev *dev)
>>>>        switch (dev->cpu_id) {
>>>>        case AMD_CPU_ID_PCO:
>>>>        case AMD_CPU_ID_RN:
>>>> +     case AMD_CPU_ID_VG:
>>>>        case AMD_CPU_ID_YC:
>>>>        case AMD_CPU_ID_CB:
>>>>                dev->num_ips = 12;
>>>> @@ -517,6 +518,7 @@ static int amd_pmc_get_os_hint(struct
>>>> amd_pmc_dev *dev)
>>>>        case AMD_CPU_ID_PCO:
>>>>                return MSG_OS_HINT_PCO;
>>>>        case AMD_CPU_ID_RN:
>>>> +     case AMD_CPU_ID_VG:
>>>>        case AMD_CPU_ID_YC:
>>>>        case AMD_CPU_ID_CB:
>>>>        case AMD_CPU_ID_PS:
>>>> @@ -717,6 +719,7 @@ static const struct pci_device_id
>>>> pmc_pci_ids[] = {
>>>>        { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) },
>>>>        { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SP) },
>>>>        { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SHP) },
>>>> +     { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_VG) },
>>>>        { PCI_DEVICE(PCI_VENDOR_ID_AMD,
>>>> PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
>>>>        { PCI_DEVICE(PCI_VENDOR_ID_AMD,
>>>> PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
>>>>        { }
>>>> diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/
>>>> platform/x86/amd/pmc/pmc.h
>>>> index 62f3e51020fd..fe3f53eb5955 100644
>>>> --- a/drivers/platform/x86/amd/pmc/pmc.h
>>>> +++ b/drivers/platform/x86/amd/pmc/pmc.h
>>>> @@ -156,6 +156,7 @@ void amd_mp2_stb_deinit(struct amd_pmc_dev *dev);
>>>>    #define AMD_CPU_ID_RN                       0x1630
>>>>    #define AMD_CPU_ID_PCO                      AMD_CPU_ID_RV
>>>>    #define AMD_CPU_ID_CZN                      AMD_CPU_ID_RN
>>>> +#define AMD_CPU_ID_VG                        0x1645
>>>
>>> Can you see if 0xF14 gives you a reasonable value for the idle mask if
>>> you add it to amd_pmc_idlemask_read()?  Make a new define for it
>>> though,
>>> it shouldn't use the same define as 0x1a platforms.
>>
>> It does not work. Reports 0. I also tested the other ones, but the
>> 0x1a was the same as you said. All report 0x0.
> 
> It's possible the platform doesn't report an idle mask.
> 
> 0xF14 is where I would have expected it to report.
> 
> Shyam - can you look into this to see if it's in a different place
> than 0xF14 for Van Gogh?

Van Gogh is before Cezzane? I am bit surprised that pmc is getting
loaded there.

Antheas - what is the output of

#lspci -s 00:00.0

0xF14 index is meant for 1Ah (i.e. Strix and above)

> 
>>
>> Any idea why the OS hint only works 90% of the time?

What is the output of amd_pmc_dump_registers() when 10% of the time
when the OS_HINT is not working?

What I can surmise is, though pmc driver is sending the hint PMFW is
not taking any action (since the support in FW is missing)

> 
> If we get the idle mask reporting working we would have a better idea
> if that is what is reported wrong.
> 

IIRC, The concept of idlemask came only after cezzane that too after a
certain PMFW version. So I am not sure if idlemask actually exists.


> If I was to guess though; maybe GFX is still active.
> 
> Depending upon what's going wrong smu_fw_info might have some more
> information too.

That's a good point to try it out.

Thanks,
Shyam