Message-ID: <53bcf684-54e7-13e2-2b41-26b6791f7469@samsung.com>
Date:   Fri, 20 May 2022 10:11:16 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Ansuel Smith <ansuelsmth@...il.com>,
        Rob Herring <robh+dt@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq
 scaling driver

Hi Ansuel,


The 5.19-rc1 will support the cpu based scaling support[2]
as your needed. So that if you use passive governor with CPUFREQ_PARENT_DEV,
you can scale the cache frequency according to cpu frequency change.

[1] https://patchwork.kernel.org/project/linux-pm/patch/3acd6c32-6e78-dfc2-3e45-84f69a7d5f36@samsung.com/
[2] PM / devfreq: Add cpu based scaling support to passive governor


Best Regards,
Chanwoo Choi

On 9/30/20 1:29 AM, Ansuel Smith wrote:
> Qcom L2 Krait CPUs use the generic cpufreq-dt driver and doesn't actually
> scale the Cache frequency when the CPU frequency is changed. This
> devfreq driver register with the cpu notifier and scale the Cache
> based on the max Freq across all core as the CPU cache is shared across
> all of them. If provided this also scale the voltage of the regulator
> attached to the CPU cache. The scaling logic is based on the CPU freq
> and the 3 scaling interval are set by the device dts.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> ---
> v2:
> * Change cpu-freq to qcom,cpu-freq
> * Skip freq change if prev is the same target freq
> 
>  drivers/devfreq/Kconfig               |  10 +
>  drivers/devfreq/Makefile              |   1 +
>  drivers/devfreq/krait-cache-devfreq.c | 301 ++++++++++++++++++++++++++
>  3 files changed, 312 insertions(+)
>  create mode 100644 drivers/devfreq/krait-cache-devfreq.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 37dc40d1fcfb..99051aaf9c5e 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -143,6 +143,16 @@ config ARM_RK3399_DMC_DEVFREQ
>  	  It sets the frequency for the memory controller and reads the usage counts
>  	  from hardware.
>  
> +config ARM_KRAIT_CACHE_DEVFREQ
> +	tristate "Scaling support for Krait CPU Cache Devfreq"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	help
> +	  This adds the DEVFREQ driver for the Krait CPU L2 Cache shared by all cores.
> +
> +	  The driver register with the cpufreq notifier and find the right frequency
> +	  based on the max frequency across all core and the range set in the device
> +	  dts. If provided this scale also the regulator attached to the l2 cache.
> +
>  source "drivers/devfreq/event/Kconfig"
>  
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 3ca1ad0ecb97..bb87925a6a2d 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>  obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>  obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>  obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
> +obj-$(CONFIG_ARM_KRAIT_CACHE_DEVFREQ)	+= krait-cache-devfreq.o
>  
>  # DEVFREQ Event Drivers
>  obj-$(CONFIG_PM_DEVFREQ_EVENT)		+= event/
> diff --git a/drivers/devfreq/krait-cache-devfreq.c b/drivers/devfreq/krait-cache-devfreq.c
> new file mode 100644
> index 000000000000..101a13b6927a
> --- /dev/null
> +++ b/drivers/devfreq/krait-cache-devfreq.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/cpufreq.h>
> +#include <linux/devfreq.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pm_opp.h>
> +
> +#include "governor.h"
> +
> +struct krait_data {
> +	struct device *dev;
> +	struct devfreq *devfreq;
> +
> +	struct clk *l2_clk;
> +
> +	unsigned long *freq_table; /* L2 bus clock rate */
> +	unsigned int *l2_cpufreq; /* L2 target CPU frequency */
> +
> +	struct notifier_block nb;
> +};
> +
> +static int krait_cache_set_opp(struct dev_pm_set_opp_data *data)
> +{
> +	unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
> +	struct dev_pm_opp_supply *supply = &data->new_opp.supplies[0];
> +	struct regulator *reg = data->regulators[0];
> +	struct clk *clk = data->clk;
> +	struct krait_data *kdata;
> +	unsigned long idle_freq;
> +	int ret;
> +
> +	kdata = (struct krait_data *)dev_get_drvdata(data->dev);
> +
> +	idle_freq = kdata->freq_table[0];
> +
> +	if (reg) {
> +		ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
> +						    supply->u_volt,
> +						    supply->u_volt_max);
> +		if (ret)
> +			goto exit;
> +	}
> +
> +	/*
> +	 * Set to idle bin if switching from normal to high bin
> +	 * or vice versa. It has been notice that a bug is triggered
> +	 * in cache scaling when more than one bin is scaled, to fix
> +	 * this we first need to transition to the base rate and then
> +	 * to target rate
> +	 */
> +	if (likely(freq != idle_freq && old_freq != idle_freq)) {
> +		ret = clk_set_rate(clk, idle_freq);
> +		if (ret)
> +			goto exit;
> +	}
> +
> +	ret = clk_set_rate(clk, freq);
> +	if (ret)
> +		goto exit;
> +
> +exit:
> +	return ret;
> +};
> +
> +static int krait_cache_target(struct device *dev, unsigned long *freq,
> +			      u32 flags)
> +{
> +	return dev_pm_opp_set_rate(dev, *freq);
> +};
> +
> +static int krait_cache_get_dev_status(struct device *dev,
> +				      struct devfreq_dev_status *stat)
> +{
> +	struct krait_data *data = dev_get_drvdata(dev);
> +
> +	stat->busy_time = 0;
> +	stat->total_time = 0;
> +	stat->current_frequency = clk_get_rate(data->l2_clk);
> +
> +	return 0;
> +};
> +
> +static int krait_cache_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> +	struct krait_data *data = dev_get_drvdata(dev);
> +
> +	*freq = clk_get_rate(data->l2_clk);
> +
> +	return 0;
> +};
> +
> +static struct devfreq_dev_profile tegra_devfreq_profile = {
> +	.target = krait_cache_target,
> +	.get_dev_status = krait_cache_get_dev_status,
> +	.get_cur_freq = krait_cache_get_cur_freq
> +};
> +
> +static int krait_cache_notifier(struct notifier_block *nb, unsigned long action,
> +				void *v)
> +{
> +	struct cpufreq_freqs *freqs;
> +	unsigned int cpu, cur_cpu;
> +	struct krait_data *data;
> +	struct devfreq *devfreq;
> +	unsigned long freq;
> +	int ret = 0;
> +
> +	if (action != CPUFREQ_POSTCHANGE)
> +		return NOTIFY_OK;
> +
> +	data = container_of(nb, struct krait_data, nb);
> +	devfreq = data->devfreq;
> +
> +	mutex_lock_nested(&devfreq->lock, SINGLE_DEPTH_NESTING);
> +
> +	freqs = (struct cpufreq_freqs *)v;
> +	freq = freqs->new;
> +	cur_cpu = freqs->policy->cpu;
> +
> +	/* find the max freq across all core */
> +	for_each_present_cpu(cpu)
> +		if (cpu != cur_cpu)
> +			freq = max(freq, (unsigned long)cpufreq_quick_get(cpu));
> +
> +	devfreq->governor->get_target_freq(devfreq, &freq);
> +
> +	if (devfreq->previous_freq == freq)
> +		goto out;
> +
> +	ret = devfreq->profile->target(data->dev, &freq, 0);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (devfreq->profile->freq_table &&
> +	    (devfreq_update_status(devfreq, freq)))
> +		dev_err(data->dev,
> +			"Couldn't update frequency transition information.\n");
> +
> +	devfreq->previous_freq = freq;
> +
> +out:
> +	mutex_unlock(&devfreq->lock);
> +	return notifier_from_errno(ret);
> +};
> +
> +static int krait_cache_governor_get_target(struct devfreq *devfreq,
> +					   unsigned long *freq)
> +{
> +	unsigned int *l2_cpufreq;
> +	unsigned long *freq_table;
> +	unsigned long target_freq = *freq;
> +	struct krait_data *data = dev_get_drvdata(devfreq->dev.parent);
> +
> +	l2_cpufreq = data->l2_cpufreq;
> +	freq_table = data->freq_table;
> +
> +	/*
> +	 * Find the highest l2 freq interval based on the max cpufreq
> +	 * across all core
> +	 */
> +	while (*(l2_cpufreq = l2_cpufreq + 1) && target_freq >= *l2_cpufreq)
> +		freq_table = freq_table + 1;
> +
> +	*freq = *freq_table;
> +
> +	return 0;
> +};
> +
> +static int krait_cache_governor_event_handler(struct devfreq *devfreq,
> +					      unsigned int event, void *data)
> +{
> +	struct krait_data *kdata = dev_get_drvdata(devfreq->dev.parent);
> +	int ret = 0;
> +
> +	switch (event) {
> +	case DEVFREQ_GOV_START:
> +		kdata->nb.notifier_call = krait_cache_notifier;
> +		ret = cpufreq_register_notifier(&kdata->nb,
> +						CPUFREQ_TRANSITION_NOTIFIER);
> +		break;
> +
> +	case DEVFREQ_GOV_STOP:
> +		cpufreq_unregister_notifier(&kdata->nb,
> +					    CPUFREQ_TRANSITION_NOTIFIER);
> +		break;
> +	}
> +
> +	return ret;
> +};
> +
> +static struct devfreq_governor krait_devfreq_governor = {
> +	.name = "krait_governor",
> +	.get_target_freq = krait_cache_governor_get_target,
> +	.event_handler = krait_cache_governor_event_handler,
> +	.immutable = true,
> +};
> +
> +static int krait_cache_probe(struct platform_device *pdev)
> +{
> +	int ret, count;
> +	struct opp_table *table;
> +	struct krait_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->dev = dev;
> +
> +	data->l2_clk = devm_clk_get(dev, "l2");
> +	if (IS_ERR(data->l2_clk))
> +		return PTR_ERR(data->l2_clk);
> +
> +	table = dev_pm_opp_set_regulators(dev, (const char *[]){ "l2" }, 1);
> +	if (IS_ERR(table)) {
> +		ret = PTR_ERR(table);
> +		dev_err(dev, "failed to set regulators %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = PTR_ERR_OR_ZERO(
> +		dev_pm_opp_register_set_opp_helper(dev, krait_cache_set_opp));
> +	if (ret)
> +		return ret;
> +
> +	ret = dev_pm_opp_of_add_table(dev);
> +	if (ret) {
> +		dev_err(dev, "failed to parse L2 freq thresholds\n");
> +		return ret;
> +	}
> +
> +	count = dev_pm_opp_get_opp_count(dev);
> +
> +	data->l2_cpufreq =
> +		devm_kzalloc(dev, sizeof(unsigned int) * count, GFP_KERNEL);
> +	if (!data->l2_cpufreq)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(node, "qcom,l2-cpufreq", data->l2_cpufreq,
> +					 count);
> +	if (ret) {
> +		dev_err(dev, "failed to parse L2 cpufreq thresholds\n");
> +		return ret;
> +	}
> +
> +	ret = devfreq_add_governor(&krait_devfreq_governor);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to add governor: %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	data->devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
> +					   "krait_governor", NULL);
> +
> +	/* Cache freq_table to quickly get it when needed */
> +	data->freq_table = data->devfreq->profile->freq_table;
> +
> +	if (IS_ERR(data->devfreq))
> +		return PTR_ERR(data->devfreq);
> +
> +	return 0;
> +};
> +
> +static int krait_cache_remove(struct platform_device *pdev)
> +{
> +	struct krait_data *data = platform_get_drvdata(pdev);
> +
> +	dev_pm_opp_remove_table(data->dev);
> +
> +	return 0;
> +};
> +
> +static const struct of_device_id krait_cache_match_table[] = {
> +	{ .compatible = "qcom,krait-cache" },
> +	{}
> +};
> +
> +static struct platform_driver krait_cache_driver = {
> +	.probe		= krait_cache_probe,
> +	.remove		= krait_cache_remove,
> +	.driver		= {
> +		.name   = "krait-cache-scaling",
> +		.of_match_table = krait_cache_match_table,
> +	},
> +};
> +module_platform_driver(krait_cache_driver);
> +
> +MODULE_DESCRIPTION("Krait CPU Cache Scaling driver");
> +MODULE_AUTHOR("Ansuel Smith <ansuelsmth@...il.com>");
> +MODULE_LICENSE("GPL v2");
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Powered by blists - more mailing lists