On 2/27/2024 18:58, Ilpo Järvinen wrote: > On Tue, 27 Feb 2024, Shyam Sundar S K wrote: > >> Update the APMF function index 2 for family 1Ah, that gets the >> information of SBIOS requests (like the pending requests from BIOS, > > extra space. > >> custom notifications, updation of power limits etc). >> >> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx> >> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> >> --- >> drivers/platform/x86/amd/pmf/acpi.c | 6 ++++++ >> drivers/platform/x86/amd/pmf/pmf.h | 13 +++++++++++++ >> 2 files changed, 19 insertions(+) >> >> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c >> index 1f287a147c57..1b2a099c0cef 100644 >> --- a/drivers/platform/x86/amd/pmf/acpi.c >> +++ b/drivers/platform/x86/amd/pmf/acpi.c >> @@ -166,6 +166,12 @@ int apmf_get_auto_mode_def(struct amd_pmf_dev *pdev, struct apmf_auto_mode *data >> return apmf_if_call_store_buffer(pdev, APMF_FUNC_AUTO_MODE, data, sizeof(*data)); >> } >> >> +int apmf_get_sbios_requests_v2(struct amd_pmf_dev *pdev, struct apmf_sbios_req_v2 *req) >> +{ >> + return apmf_if_call_store_buffer(pdev, APMF_FUNC_SBIOS_REQUESTS, >> + req, sizeof(*req)); > > Fix the alignment please. > >> +} >> + >> int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req) >> { >> return apmf_if_call_store_buffer(pdev, APMF_FUNC_SBIOS_REQUESTS, >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h >> index 4364af72a7a3..f11d2a348696 100644 >> --- a/drivers/platform/x86/amd/pmf/pmf.h >> +++ b/drivers/platform/x86/amd/pmf/pmf.h >> @@ -116,6 +116,18 @@ struct apmf_sbios_req { >> u8 skin_temp_hs2; >> } __packed; >> >> +struct apmf_sbios_req_v2 { >> + u16 size; >> + u32 pending_req; >> + u8 rsvd; >> + u32 update_ppt_pmf; >> + u32 update_ppt_pmf_apu_only; >> + u32 update_stt_min; >> + u8 update_stt_apu; >> + u8 update_stt_hs2; > > Is it intentional that these do not match with the names in struct > apmf_sbios_req? I mean some of the fields look suspiciously close in name > so is the purpose still the same and somebody just invented new names for the > same field? The idea is to optimize certain fields in the BIOS menu that OEMs have to feed in while making the right choices for the power settings for different features. The entire series is targeted towards that where the interface between the driver and the BIOS is improvised so that: - Multiple features can link to one state, so OEMs doesn’t need to program same parameters in multiple locations. - If we need to add new power controller limits , we don’t have to touch APMF functions, its more expandable adding new fields in APS methods. To answers to your question of apmf_sbios_req vs apmf_sbios_req_v2: It calls for a new struct _v2, because: - AMT support has been dropped so there shall be no pending events from DYTC (like the CQL and AMT) - As per the new design, the PMFW has given control to set PPT and STT limits and no pending requests on updating SPL limits. But as per names, I don't think there is no new invention :-) FYI, here PPT means Power Packaging Tracking, so it could be SPPT (Slow PPT) or FPPT (Fast PPT) and SST means Skin Temperature Tracking. I will address your other remarks. Thanks, Shyam