On Wed, 28 Feb 2024, Shyam Sundar S K wrote:
>
>
> 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 :-)
Hi again,
I'm fine with adding _v2 struct (I could see they're not identical).
What I'm still left unsure if we spoke past each other so let me state
the question in more concrete terms:
- apmf_sbios_req has a field called stt_min_limit
- apmf_sbios_req_v2 has a field called update_stt_min
My question is, are those just the same but only named differently for
some reason, or does the "limit" and/or "update" difference actually imply
there's change in how that field is used?
Similar question applies to the other fields which look close but not
identical. There's no need for you to itemize and explain each field for
me specifically in the reply, I just prefer the same thing called the same
in both structs if that's the case.
It could be you tried to answer this with your second bullet but I just
don't understand its meaning deeply enough, thus I'm asking again, please
bear with me.
> 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.
