Re: [PATCH 8/8] devfreq: mediatek: cci devfreq register opp notification for SVS support

Hsin-Yi Wang
2019-05-24T08:04:47Z

On Thu, May 16, 2019 at 9:08 AM Andrew-sh.Cheng
<andrew-sh.cheng@mediatek.com> wrote:
>
> From: "Andrew-sh.Cheng" <andrew-sh.cheng@mediatek.com>
>
> SVS will change the voltage of opp item.
> CCI devfreq need to react to change frequency.
>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
> drivers/devfreq/mt8183-cci-devfreq.c | 63 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
> index 818a167c442f..250c963789f3 100644
> --- a/drivers/devfreq/mt8183-cci-devfreq.c
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -19,7 +19,10 @@ struct cci_devfreq {
> struct regulator *proc_reg;
> unsigned long proc_reg_uV;
> struct clk *cci_clk;
> + unsigned long freq;
> struct notifier_block nb;
> + struct notifier_block opp_nb;
> + int cci_min_freq;
> };
>
> static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
> @@ -65,17 +68,62 @@ static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
> return 0;
> }
>
> +static int ccidevfreq_opp_notifier(struct notifier_block *nb,
> +unsigned long event, void *data)
> +{
> + int ret;
> + struct dev_pm_opp *opp = data;
> + struct cci_devfreq *cci_df = container_of(nb, struct cci_devfreq,
> + opp_nb);
> + unsigned long freq, volt, cur_volt;
> +
> + if (event == OPP_EVENT_ADJUST_VOLTAGE) {
> + freq = dev_pm_opp_get_freq(opp);
> + /* current opp item is changed */
> + if (freq == cci_df->freq) {
> + volt = dev_pm_opp_get_voltage(opp);
> + cur_volt = regulator_get_voltage(cci_df->proc_reg);
> +
> + if (volt > cur_volt) {
> + /* need reduce freq */
> + mutex_lock(&cci_df->devfreq->lock);
> + ret = update_devfreq(cci_df->devfreq);
> + if (ret)
> + pr_err("Fail to reduce cci frequency by opp notification: %d\n",
This line is too long
> + ret);
> + mutex_unlock(&cci_df->devfreq->lock);
> + }
> + }
> +
> + if (freq == cci_df->cci_min_freq) {
> + volt = dev_pm_opp_get_voltage(opp);
> + regulator_set_voltage(cci_df->proc_reg, volt, INT_MAX);
> + }
> + } else if (event == OPP_EVENT_DISABLE) {
Is this case handled? If not, is this branch needed?
> + }
> +
> + return 0;
> +}
> +
> +
> static int mtk_cci_governor_get_target(struct devfreq *devfreq,
> unsigned long *freq)
> {
> struct cci_devfreq *cci_df;
> struct dev_pm_opp *opp;
> + int ret;
>
> cci_df = dev_get_drvdata(devfreq->dev.parent);
>
> /* find available frequency */
> opp = dev_pm_opp_find_freq_ceil_by_volt(devfreq->dev.parent,
> cci_df->proc_reg_uV);
> + ret = PTR_ERR_OR_ZERO(opp);
> + if (ret) {
> + pr_err("%s[%d], cannot find opp with voltage=%d: %d\n",
> + __func__, __LINE__, cci_df->proc_reg_uV, ret);
> + return ret;
> + }
> *freq = dev_pm_opp_get_freq(opp);
>
> return 0;
> @@ -87,9 +135,11 @@ static int mtk_cci_governor_event_handler(struct devfreq *devfreq,
> int ret;
> struct cci_devfreq *cci_df;
> struct notifier_block *nb;
> + struct notifier_block *opp_nb;
>
> cci_df = dev_get_drvdata(devfreq->dev.parent);
> nb = &cci_df->nb;
> + opp_nb = &cci_df->opp_nb;
>
> switch (event) {
> case DEVFREQ_GOV_START:
> @@ -100,6 +150,8 @@ static int mtk_cci_governor_event_handler(struct devfreq *devfreq,
> if (ret)
> pr_err("%s: failed to add governor: %d\n", __func__,
> ret);
> + opp_nb->notifier_call = ccidevfreq_opp_notifier;
> + dev_pm_opp_register_notifier(devfreq->dev.parent, opp_nb);
> break;
>
> case DEVFREQ_GOV_STOP:
> @@ -141,6 +193,8 @@ static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
> return ret;
> }
>
> + cci_df->freq = *freq;
> +
> return 0;
> }
>
> @@ -152,6 +206,8 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> {
> struct device *cci_dev = &pdev->dev;
> struct cci_devfreq *cci_df;
> + unsigned long freq, volt;
> + struct dev_pm_opp *opp;
> int ret;
>
> cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> @@ -181,6 +237,13 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> return ret;
> }
>
> + /* set voltage lower bound */
> + freq = 1;
> + opp = dev_pm_opp_find_freq_ceil(cci_dev, &freq);
> + cci_df->cci_min_freq = dev_pm_opp_get_freq(opp);
> + volt = dev_pm_opp_get_voltage(opp);
> + dev_pm_opp_put(opp);
> +
> platform_set_drvdata(pdev, cci_df);
>
> cci_df->devfreq = devm_devfreq_add_device(cci_dev,