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

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/27/25 3:22 AM, Shyam Sundar S K wrote:
> 
> 
> 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.

IIRC the first Van Gogh parts actually came after Cezanne.  Around 
Rembrandt timeframe.  But nothing used s2idle at that time.  So I wasn't 
sure whether idle mask support was present.

I think that the Xbox Ally X is the first Van Gogh based part that has 
s2idle support.  My wonder was if idle mask reading made it into the 
updated PMFW on the Xbox Ally X but the offset was wrong.
> 
> 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
>