On 10/16/2024 01:26, Corey Hickey wrote:
> On 2024-10-15 03:40, Shyam Sundar S K wrote:
>>> This time, I get some debug lines from amd_pmc_dump_registers(). I am
>>> including my debug patch here--I think it gives a bit of context that
>>> I can understand better.
>>>
>>>
>>> <6>[ 1143.655752] amd_pmc_probe: 1
>>> <6>[ 1143.655763] amd_pmc_probe: 2
>>> <6>[ 1143.655764] amd_pmc_probe: 3
>>> <6>[ 1143.655773] amd_pmc_probe: 4
>>> <6>[ 1143.655796] amd_pmc_probe: 5
>>> <6>[ 1143.655797] amd_pmc_probe: 6
>>> <6>[ 1143.655798] amd_pmc_is_stb_supported cpu_id: 5352
>>> <7>[ 1143.684758] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_RESPONSE:1
>>> <7>[ 1143.684768] amd_pmc AMDI0009:00:
>>> AMD_S2D_REGISTER_ARGUMENT:100000
>>> <7>[ 1143.684770] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_MESSAGE:85
>>> <6>[ 1143.684772] amd_pmc_s2d_init size: 1048576
>>> <3>[ 1143.684873] amd_pmc AMDI0009:00: SMU cmd failed. err: 0xff
>>> <7>[ 1143.684886] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_RESPONSE:ff
>>> <7>[ 1143.684894] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_ARGUMENT:5
>>> <7>[ 1143.684901] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_MESSAGE:85
>>> <6>[ 1143.684903] amd_pmc_s2d_init s2d_dram_size ret: -5
>>> <7>[ 1143.715734] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_RESPONSE:1
>>> <7>[ 1143.715741] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_ARGUMENT:0
>>> <7>[ 1143.715744] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_MESSAGE:85
>>> <7>[ 1143.746780] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_RESPONSE:1
>>> <7>[ 1143.746790] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_ARGUMENT:0
>>> <7>[ 1143.746793] amd_pmc AMDI0009:00: AMD_S2D_REGISTER_MESSAGE:85
>>> <6>[ 1143.746795] amd_pmc_s2d_init p_a_l: 0 p_a_hi: 0 s_p_a: 0 sz:
>>> 16777216
>>>
>>
>> High and low addresses are zero, because STB is not enabled on your
>> system. So S2D (Spill to DRAM) mailbox commands are expected to fail.
>> You will have to contact Frame.work team to get you the STB feature
>> enabled.
>
> Thank you for checking the logs.
>
> Would it be valid to have the driver check for this, as in the
> following (untested) patch?
>
>
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c
> b/drivers/platform/x86/amd/pmc/pmc.c
> index 6ca497473d78..148d57fc7b95 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -1001,6 +1001,11 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev
> *dev)
> amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low,
> dev->s2d_msg_id, true);
> amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi,
> dev->s2d_msg_id, true);
>
> + if (!phys_addr_hi && !phys_addr_low) {
> + printk(KERN_WARNING "amd_pmc: STB is not enabled on
> the system; disable enable_stb or contact system vendor\n");
> + return -ENOMEM;
> + }
> +
> stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
>
> /* Clear msg_port for other SMU operation */
>
>
> Something like that could have helped me understand the situation is
> better. If a patch like that would be welcome, I can test and submit
> it.
Sorry I missed this thread. Yes, please submit this change for review.
Instead of returning -ENOMEM, just return -EINVAL (as the address
obtained from PMFW is incorrect).
Thanks,
Shyam
