On Mon, 9 Oct 2023, Shyam Sundar S K wrote:
> On 10/4/2023 5:44 PM, Ilpo Järvinen wrote:
> > On Sat, 30 Sep 2023, Shyam Sundar S K wrote:
> >
> >> PMF driver sends changing inputs from each subystem to TA for evaluating
> >> the conditions in the policy binary.
> >>
> >> Add initial support of plumbing in the PMF driver for Smart PC to get
> >> information from other subsystems in the kernel.
> >>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> >> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> >> new file mode 100644
> >> index 000000000000..3113bde051d9
> >> --- /dev/null
> >> +++ b/drivers/platform/x86/amd/pmf/spc.c
> >> @@ -0,0 +1,119 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * AMD Platform Management Framework Driver - Smart PC Capabilities
> >> + *
> >> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> >> + * All Rights Reserved.
> >> + *
> >> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> >> + * Patil Rajesh Reddy <Patil.Reddy@xxxxxxx>
> >> + */
> >> +
> >> +#include <acpi/button.h>
> >> +#include <linux/power_supply.h>
> >> +#include <linux/units.h>
> >> +#include "pmf.h"
> >> +
> >> +static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> >> +{
> >> + u16 max, avg = 0;
> >> + int i;
> >> +
> >> + memset(dev->buf, 0, sizeof(dev->m_table));
> >> + amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> >> + memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
> >> +
> >> + in->ev_info.socket_power = dev->m_table.apu_power + dev->m_table.dgpu_power;
> >> + in->ev_info.skin_temperature = dev->m_table.skin_temp;
> >> +
> >> + /* get the avg C0 residency of all the cores */
> >> + for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++)
> >> + avg += dev->m_table.avg_core_c0residency[i];
> >> +
> >> + /* get the max C0 residency of all the cores */
> >> + max = dev->m_table.avg_core_c0residency[0];
> >> + for (i = 1; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
> >> + if (dev->m_table.avg_core_c0residency[i] > max)
> >> + max = dev->m_table.avg_core_c0residency[i];
> >> + }
> >
> > My comments weren't either answered adequately or changes made here.
> > Please check the v1 comments. I hope it's not because you feel hurry to
> > get the next version out...
> >
> > I'm still unsure if the u16 thing can overflow because I don't know what's
> > the max value for avg_core_c0residency[i].
>
> the highest value for avg_core_c0residency[i] is merely a small number
> and hence I retained the avg variable as u16. Not sure if there a
> 'real' case where it can overflow.
Okay, if you think it's fine, no problem with it then (not that there's
a big advantage having it as u16 instead of e.g. unsigned int).
> Sorry, I missed to merge both into a single for loop. I will address
> this in v3.
Thanks.
--
i.
