[PATCH V4 05/14] cpufreq: mediatek: Add opp notification support

[Rex-BC Chen]

On Mon, 2022-04-25 at 10:50 +0530, Viresh Kumar wrote:
> On 22-04-22, 15:52, Rex-BC Chen wrote:
> > From: "Andrew-sh.Cheng" <andrew-sh.cheng at mediatek.com>
> > 
> > > From this opp notifier, cpufreq should listen to opp notification
> > > and do
> 
> Why the extra ">" here ?
> 
> > proper actions when receiving events of disable and voltage
> > adjustment.
> > 
> > One of the user for this opp notifier is MediaTek SVS.
> > The MediaTek Smart Voltage Scaling (SVS) is a hardware which
> > calculates
> > suitable SVS bank voltages to OPP voltage table.
> > 
> > Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng at mediatek.com>
> > Signed-off-by: Jia-Wei Chang <jia-wei.chang at mediatek.com>
> > Signed-off-by: Rex-BC Chen <rex-bc.chen at mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno at collabora.com>
> > ---
> >  drivers/cpufreq/mediatek-cpufreq.c | 92
> > +++++++++++++++++++++++++++---
> >  1 file changed, 84 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/mediatek-cpufreq.c
> > b/drivers/cpufreq/mediatek-cpufreq.c
> > +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb,
> > +				    unsigned long event, void *data)
> > +{
> > +	struct dev_pm_opp *opp = data;
> > +	struct dev_pm_opp *new_opp;
> > +	struct mtk_cpu_dvfs_info *info;
> > +	unsigned long freq, volt;
> > +	struct cpufreq_policy *policy;
> > +	int ret = 0;
> > +
> > +	info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb);
> > +
> > +	if (event == OPP_EVENT_ADJUST_VOLTAGE) {
> 
> I don't see any call to dev_pm_opp_adjust_voltage() for your
> platform, how is
> this ever going to get called ?
> 

Hello Viresh,

Thanks for your review!
These two event OPP_EVENT_ADJUST_VOLTAGE and OPP_EVENT_DISABLE are
basically called by mediatek svs drivers.
It's not merged in mainline kernel and you can reger to [1].

[1]:
https://lore.kernel.org/all/20220420102044.10832-4-roger.lu@mediatek.com/

For OPP_EVENT_ADJUST_VOLTAGE:
mediatek svs will optimize the voltage, so mediatek svs will call
dev_pm_opp_adjust_voltage() to adjust frequency and voltage.

> > +		freq = dev_pm_opp_get_freq(opp);
> > +
> > +		mutex_lock(&info->reg_lock);
> > +		if (info->opp_freq == freq) {
> > +			volt = dev_pm_opp_get_voltage(opp);
> > +			ret = mtk_cpufreq_set_voltage(info, volt);
> > +			if (ret)
> > +				dev_err(info->cpu_dev,
> > +					"failed to scale voltage:
> > %d\n", ret);
> > +		}
> > +		mutex_unlock(&info->reg_lock);
> > +	} else if (event == OPP_EVENT_DISABLE) {
> > +		freq = dev_pm_opp_get_freq(opp);
> > +
> > +		/* case of current opp item is disabled */
> > +		if (info->opp_freq == freq) {
> > +			freq = 1;
> > +			new_opp = dev_pm_opp_find_freq_ceil(info-
> > >cpu_dev,
> > +							    &freq);
> > +			if (IS_ERR(new_opp)) {
> > +				dev_err(info->cpu_dev,
> > +					"all opp items are
> > disabled\n");
> > +				ret = PTR_ERR(new_opp);
> > +				return notifier_from_errno(ret);
> > +			}
> > +
> > +			dev_pm_opp_put(new_opp);
> > +			policy = cpufreq_cpu_get(info->opp_cpu);
> > +			if (policy) {
> > +				cpufreq_driver_target(policy, freq /
> > 1000,
> > +						      CPUFREQ_RELATION_
> > L);
> > +				cpufreq_cpu_put(policy);
> 
> IIUC, then you are trying to change the frequency if a currently used
> OPP is
> getting removed ? In that case, this problem is generic enough not to
> be fixed
> in a end driver. This should be fixed in core somehow.
> 

For OPP_EVENT_DISABLE:
when mediatek svs needs to be disable, the voltage should recover to
original voltage and frequency.

BRs,
Rex