Re: [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS

From: Chanwoo Choi
Date: Thu Sep 30 2021 - 00:14:28 EST


Hi Samuel,


On 9/29/21 1:42 PM, Samuel Holland wrote:
> This driver works by adjusting the divider on the DRAM controller's
> module clock. Thus there is no fixed set of OPPs, only "full speed" down
> to "quarter speed" (or whatever the maximum divider is on that variant).
>
> It makes use of the MDFS hardware in the MBUS, in "DFS" mode, which
> takes care of updating registers during the critical section while DRAM
> is inaccessible.
>
> This driver should support several sunxi SoCs, starting with the A33,
> which have a DesignWare DDR3 controller with merged PHY register space
> and the matching MBUS register layout (so not A63 or later). However,
> the driver has only been tested on the A64/H5, so those are the only
> compatibles enabled for now.
>
> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> ---
> drivers/devfreq/Kconfig | 8 +
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/sun8i-a33-mbus.c | 482 +++++++++++++++++++++++++++++++
> 3 files changed, 491 insertions(+)
> create mode 100644 drivers/devfreq/sun8i-a33-mbus.c
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index e87d01c0b76a..b94eb04761f6 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -132,6 +132,14 @@ config ARM_RK3399_DMC_DEVFREQ
> It sets the frequency for the memory controller and reads the usage counts
> from hardware.
>
> +config ARM_SUN8I_A33_MBUS_DEVFREQ
> + tristate "sun8i/sun50i MBUS DEVFREQ Driver"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + help
> + This adds the DEVFREQ driver for the MBUS controller in some
> + Allwinner sun8i (A33 through H3) and sun50i (A64 and H5) SoCs.
> +
> source "drivers/devfreq/event/Kconfig"
>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index a16333ea7034..0b6be92a25d9 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
> obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o
> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o
> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> +obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
>
> # DEVFREQ Event Drivers
> diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
> new file mode 100644
> index 000000000000..00f24850cf8d
> --- /dev/null
> +++ b/drivers/devfreq/sun8i-a33-mbus.c
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (C) 2020-2021 Samuel Holland <samuel@xxxxxxxxxxxx>
> +//
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +
> +#define MBUS_CR 0x0000
> +#define MBUS_CR_GET_DRAM_TYPE(x) (((x) >> 16) & 0x7)
> +#define MBUS_CR_DRAM_TYPE_DDR2 2
> +#define MBUS_CR_DRAM_TYPE_DDR3 3
> +#define MBUS_CR_DRAM_TYPE_DDR4 4
> +#define MBUS_CR_DRAM_TYPE_LPDDR2 6
> +#define MBUS_CR_DRAM_TYPE_LPDDR3 7
> +
> +#define MBUS_TMR 0x000c
> +#define MBUS_TMR_PERIOD(x) ((x) - 1)
> +
> +#define MBUS_PMU_CFG 0x009c
> +#define MBUS_PMU_CFG_PERIOD(x) (((x) - 1) << 16)
> +#define MBUS_PMU_CFG_UNIT (0x3 << 1)
> +#define MBUS_PMU_CFG_UNIT_B (0x0 << 1)
> +#define MBUS_PMU_CFG_UNIT_KB (0x1 << 1)
> +#define MBUS_PMU_CFG_UNIT_MB (0x2 << 1)
> +#define MBUS_PMU_CFG_ENABLE (0x1 << 0)
> +
> +#define MBUS_PMU_BWCR(n) (0x00a0 + (0x04 * (n)))
> +
> +#define MBUS_TOTAL_BWCR MBUS_PMU_BWCR(5)
> +#define MBUS_TOTAL_BWCR_H616 MBUS_PMU_BWCR(13)
> +
> +#define MBUS_MDFSCR 0x0100
> +#define MBUS_MDFSCR_BUFFER_TIMING (0x1 << 15)
> +#define MBUS_MDFSCR_PAD_HOLD (0x1 << 13)
> +#define MBUS_MDFSCR_BYPASS (0x1 << 4)
> +#define MBUS_MDFSCR_MODE (0x1 << 1)
> +#define MBUS_MDFSCR_MODE_DFS (0x0 << 1)
> +#define MBUS_MDFSCR_MODE_CFS (0x1 << 1)
> +#define MBUS_MDFSCR_START (0x1 << 0)
> +
> +#define MBUS_MDFSMRMR 0x0108
> +
> +#define DRAM_PWRCTL 0x0004
> +#define DRAM_PWRCTL_SELFREF_EN (0x1 << 0)
> +
> +#define DRAM_RFSHTMG 0x0090
> +#define DRAM_RFSHTMG_TREFI(x) ((x) << 16)
> +#define DRAM_RFSHTMG_TRFC(x) ((x) << 0)
> +
> +#define DRAM_VTFCR 0x00b8
> +#define DRAM_VTFCR_VTF_ENABLE (0x3 << 8)
> +
> +#define DRAM_ODTMAP 0x0120
> +
> +#define DRAM_DX_MAX 4
> +
> +#define DRAM_DXnGCR0(n) (0x0344 + 0x80 * (n))
> +#define DRAM_DXnGCR0_DXODT (0x3 << 4)
> +#define DRAM_DXnGCR0_DXODT_DYNAMIC (0x0 << 4)
> +#define DRAM_DXnGCR0_DXODT_ENABLED (0x1 << 4)
> +#define DRAM_DXnGCR0_DXODT_DISABLED (0x2 << 4)
> +#define DRAM_DXnGCR0_DXEN (0x1 << 0)
> +
> +struct sun8i_a33_mbus_variant {
> + u32 min_dram_divider;
> + u32 max_dram_divider;
> + u32 odt_freq_mhz;
> +};
> +
> +struct sun8i_a33_mbus {
> + const struct sun8i_a33_mbus_variant *variant;
> + void __iomem *reg_dram;
> + void __iomem *reg_mbus;
> + struct clk *clk_bus;
> + struct clk *clk_dram;
> + struct clk *clk_mbus;
> + struct devfreq *devfreq_dram;
> + struct devfreq_simple_ondemand_data gov_data;
> + struct devfreq_dev_profile profile;
> + u32 data_width;
> + u32 nominal_bw;
> + u32 odtmap;
> + u32 tREFI_ns;
> + u32 tRFC_ns;
> + unsigned long freq_table[];
> +};
> +
> +/*
> + * The unit for this value is (MBUS clock cycles / MBUS_TMR_PERIOD). When
> + * MBUS_TMR_PERIOD is programmed to match the MBUS clock frequency in MHz, as
> + * it is during DRAM init and during probe, the resulting unit is microseconds.
> + */
> +static int pmu_period = 50000;
> +module_param(pmu_period, int, 0644);
> +MODULE_PARM_DESC(pmu_period, "Bandwidth measurement period (microseconds)");
> +
> +static u32 sun8i_a33_mbus_get_peak_bw(struct sun8i_a33_mbus *priv)
> +{
> + /* Returns the peak transfer (in KiB) during any single PMU period. */
> + return readl_relaxed(priv->reg_mbus + MBUS_TOTAL_BWCR);
> +}
> +
> +static void sun8i_a33_mbus_restart_pmu_counters(struct sun8i_a33_mbus *priv)
> +{
> + u32 pmu_cfg = MBUS_PMU_CFG_PERIOD(pmu_period) | MBUS_PMU_CFG_UNIT_KB;
> +
> + /* All PMU counters are cleared on a disable->enable transition. */
> + writel_relaxed(pmu_cfg,
> + priv->reg_mbus + MBUS_PMU_CFG);
> + writel_relaxed(pmu_cfg | MBUS_PMU_CFG_ENABLE,
> + priv->reg_mbus + MBUS_PMU_CFG);
> +
> +}
> +
> +static void sun8i_a33_mbus_update_nominal_bw(struct sun8i_a33_mbus *priv,
> + u32 ddr_freq_mhz)
> +{
> + /*
> + * Nominal bandwidth (KiB per PMU period):
> + *
> + * DDR transfers microseconds KiB
> + * ------------- * ------------ * --------
> + * microsecond PMU period transfer
> + */
> + priv->nominal_bw = ddr_freq_mhz * pmu_period * priv->data_width / 1024;
> +}
> +
> +static int sun8i_a33_mbus_set_dram_freq(struct sun8i_a33_mbus *priv,
> + unsigned long freq)
> +{
> + u32 ddr_freq_mhz = freq / USEC_PER_SEC; /* DDR */
> + u32 dram_freq_mhz = ddr_freq_mhz / 2; /* SDR */
> + u32 mctl_freq_mhz = dram_freq_mhz / 2; /* HDR */
> + u32 dxodt, mdfscr, pwrctl, vtfcr;
> + u32 i, tREFI_32ck, tRFC_ck;
> + int ret;
> +
> + /* The rate change is not effective until the MDFS process runs. */
> + ret = clk_set_rate(priv->clk_dram, freq);
> + if (ret)
> + return ret;
> +
> + /* Disable automatic self-refesh and VTF before starting MDFS. */
> + pwrctl = readl_relaxed(priv->reg_dram + DRAM_PWRCTL) &
> + ~DRAM_PWRCTL_SELFREF_EN;
> + writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
> + vtfcr = readl_relaxed(priv->reg_dram + DRAM_VTFCR);
> + writel_relaxed(vtfcr & ~DRAM_VTFCR_VTF_ENABLE,
> + priv->reg_dram + DRAM_VTFCR);
> +
> + /* Set up MDFS and enable double buffering for timing registers. */
> + mdfscr = MBUS_MDFSCR_MODE_DFS |
> + MBUS_MDFSCR_BYPASS |
> + MBUS_MDFSCR_PAD_HOLD |
> + MBUS_MDFSCR_BUFFER_TIMING;
> + writel(mdfscr, priv->reg_mbus + MBUS_MDFSCR);
> +
> + /* Update the buffered copy of RFSHTMG. */
> + tREFI_32ck = priv->tREFI_ns * mctl_freq_mhz / 1000 / 32;
> + tRFC_ck = DIV_ROUND_UP(priv->tRFC_ns * mctl_freq_mhz, 1000);
> + writel(DRAM_RFSHTMG_TREFI(tREFI_32ck) | DRAM_RFSHTMG_TRFC(tRFC_ck),
> + priv->reg_dram + DRAM_RFSHTMG);
> +
> + /* Enable ODT if needed, or disable it to save power. */
> + if (priv->odtmap && dram_freq_mhz > priv->variant->odt_freq_mhz) {
> + dxodt = DRAM_DXnGCR0_DXODT_DYNAMIC;
> + writel(priv->odtmap, priv->reg_dram + DRAM_ODTMAP);
> + } else {
> + dxodt = DRAM_DXnGCR0_DXODT_DISABLED;
> + writel(0, priv->reg_dram + DRAM_ODTMAP);
> + }
> + for (i = 0; i < DRAM_DX_MAX; ++i) {
> + void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
> +
> + writel((readl(reg) & ~DRAM_DXnGCR0_DXODT) | dxodt, reg);
> + }
> +
> + dev_dbg(priv->devfreq_dram->dev.parent,
> + "Setting DRAM to %u MHz, tREFI=%u, tRFC=%u, ODT=%s\n",
> + dram_freq_mhz, tREFI_32ck, tRFC_ck,
> + dxodt == DRAM_DXnGCR0_DXODT_DYNAMIC ? "dynamic" : "disabled");
> +
> + /* Trigger hardware MDFS. */
> + writel(mdfscr | MBUS_MDFSCR_START, priv->reg_mbus + MBUS_MDFSCR);
> + ret = readl_poll_timeout_atomic(priv->reg_mbus + MBUS_MDFSCR, mdfscr,
> + !(mdfscr & MBUS_MDFSCR_START), 10, 1000);
> + if (ret)
> + return ret;
> +
> + /* Disable double buffering. */
> + writel(0, priv->reg_mbus + MBUS_MDFSCR);
> +
> + /* Restore VTF configuration. */
> + writel_relaxed(vtfcr, priv->reg_dram + DRAM_VTFCR);
> +
> + /* Enable automatic self-refresh at the lowest frequency only. */
> + if (freq == priv->freq_table[0])
> + pwrctl |= DRAM_PWRCTL_SELFREF_EN;
> + writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
> +
> + sun8i_a33_mbus_restart_pmu_counters(priv);
> + sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq_mhz);
> +
> + return 0;
> +}
> +
> +static int sun8i_a33_mbus_set_dram_target(struct device *dev,
> + unsigned long *freq, u32 flags)
> +{
> + struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> + struct devfreq *devfreq = priv->devfreq_dram;
> + int ret;
> +
> + devfreq_recommended_freq(devfreq, freq, flags);
> +
> + if (*freq == devfreq->previous_freq)
> + return 0;
> +
> + ret = sun8i_a33_mbus_set_dram_freq(priv, *freq);
> + if (ret) {
> + dev_warn(dev, "failed to set DRAM frequency: %d\n", ret);
> + *freq = devfreq->previous_freq;
> + }
> +
> + return ret;
> +}
> +
> +static int sun8i_a33_mbus_get_dram_status(struct device *dev,
> + struct devfreq_dev_status *stat)
> +{
> + struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> + stat->busy_time = sun8i_a33_mbus_get_peak_bw(priv);
> + stat->total_time = priv->nominal_bw;
> + stat->current_frequency = priv->devfreq_dram->previous_freq;
> +
> + sun8i_a33_mbus_restart_pmu_counters(priv);
> +
> + dev_dbg(dev, "Using %lu/%lu (%lu%%) at %lu MHz\n",
> + stat->busy_time, stat->total_time,
> + DIV_ROUND_CLOSEST(stat->busy_time * 100, stat->total_time),
> + stat->current_frequency / USEC_PER_SEC);
> +
> + return 0;
> +}
> +
> +static int sun8i_a33_mbus_hw_init(struct device *dev,
> + struct sun8i_a33_mbus *priv,
> + unsigned long ddr_freq)
> +{
> + u32 i, mbus_cr, mbus_freq_mhz;
> +
> + /* Choose tREFI and tRFC to match the configured DRAM type. */
> + mbus_cr = readl_relaxed(priv->reg_mbus + MBUS_CR);
> + switch (MBUS_CR_GET_DRAM_TYPE(mbus_cr)) {
> + case MBUS_CR_DRAM_TYPE_DDR2:
> + case MBUS_CR_DRAM_TYPE_DDR3:
> + case MBUS_CR_DRAM_TYPE_DDR4:
> + priv->tREFI_ns = 7800;
> + priv->tRFC_ns = 350;
> + break;
> + case MBUS_CR_DRAM_TYPE_LPDDR2:
> + case MBUS_CR_DRAM_TYPE_LPDDR3:
> + priv->tREFI_ns = 3900;
> + priv->tRFC_ns = 210;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Save ODTMAP so it can be restored when raising the frequency. */
> + priv->odtmap = readl_relaxed(priv->reg_dram + DRAM_ODTMAP);
> +
> + /* Compute the DRAM data bus width by counting enabled DATx8 blocks. */
> + for (i = 0; i < DRAM_DX_MAX; ++i) {
> + void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
> +
> + if (!(readl_relaxed(reg) & DRAM_DXnGCR0_DXEN))
> + break;
> + }
> + priv->data_width = i;
> +
> + dev_dbg(dev, "Detected %u-bit %sDDRx with%s ODT\n",
> + priv->data_width * 8,
> + MBUS_CR_GET_DRAM_TYPE(mbus_cr) > 4 ? "LP" : "",
> + priv->odtmap ? "" : "out");
> +
> + /* Program MBUS_TMR such that the PMU period unit is microseconds. */
> + mbus_freq_mhz = clk_get_rate(priv->clk_mbus) / USEC_PER_SEC;
> + writel_relaxed(MBUS_TMR_PERIOD(mbus_freq_mhz),
> + priv->reg_mbus + MBUS_TMR);
> +
> + /* "Master Ready Mask Register" bits must be set or MDFS will block. */
> + writel_relaxed(0xffffffff, priv->reg_mbus + MBUS_MDFSMRMR);
> +
> + sun8i_a33_mbus_restart_pmu_counters(priv);
> + sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq / USEC_PER_SEC);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused sun8i_a33_mbus_suspend(struct device *dev)
> +{
> + struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(priv->clk_bus);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused sun8i_a33_mbus_resume(struct device *dev)
> +{
> + struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> + return clk_prepare_enable(priv->clk_bus);
> +}
> +
> +static int sun8i_a33_mbus_probe(struct platform_device *pdev)
> +{
> + const struct sun8i_a33_mbus_variant *variant;
> + struct device *dev = &pdev->dev;
> + struct sun8i_a33_mbus *priv;
> + unsigned long base_freq;
> + unsigned int max_state;
> + unsigned int div;
> + const char *err;
> + int i, ret;
> +
> + variant = device_get_match_data(dev);
> + if (!variant)
> + return -EINVAL;
> +
> + max_state = variant->max_dram_divider - variant->min_dram_divider + 1;
> +
> + priv = devm_kzalloc(dev, struct_size(priv, freq_table, max_state), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + priv->variant = variant;
> +
> + priv->reg_dram = devm_platform_ioremap_resource_byname(pdev, "dram");
> + if (IS_ERR(priv->reg_dram))
> + return PTR_ERR(priv->reg_dram);
> +
> + priv->reg_mbus = devm_platform_ioremap_resource_byname(pdev, "mbus");
> + if (IS_ERR(priv->reg_mbus))
> + return PTR_ERR(priv->reg_mbus);
> +
> + priv->clk_bus = devm_clk_get(dev, "bus");
> + if (IS_ERR(priv->clk_bus))
> + return dev_err_probe(dev, PTR_ERR(priv->clk_bus),
> + "failed to get bus clock\n");
> +
> + priv->clk_dram = devm_clk_get(dev, "dram");
> + if (IS_ERR(priv->clk_dram))
> + return dev_err_probe(dev, PTR_ERR(priv->clk_dram),
> + "failed to get dram clock\n");
> +
> + priv->clk_mbus = devm_clk_get(dev, "mbus");
> + if (IS_ERR(priv->clk_mbus))
> + return dev_err_probe(dev, PTR_ERR(priv->clk_mbus),
> + "failed to get mbus clock\n");
> +
> + ret = clk_prepare_enable(priv->clk_bus);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to enable bus clock\n");
> +
> + /* Lock the DRAM clock rate to keep priv->nominal_bw in sync. */
> + ret = clk_rate_exclusive_get(priv->clk_dram);
> + if (ret) {
> + err = "failed to lock dram clock rate\n";
> + goto err_disable_bus;
> + }
> +
> + /* Lock the MBUS clock rate to keep MBUS_TMR_PERIOD in sync. */
> + ret = clk_rate_exclusive_get(priv->clk_mbus);
> + if (ret) {
> + err = "failed to lock mbus clock rate\n";
> + goto err_unlock_dram;
> + }
> +
> + priv->gov_data.upthreshold = 10;
> + priv->gov_data.downdifferential = 5;
> +
> + priv->profile.initial_freq = clk_get_rate(priv->clk_dram);
> + priv->profile.polling_ms = 1000;
> + priv->profile.target = sun8i_a33_mbus_set_dram_target;
> + priv->profile.get_dev_status = sun8i_a33_mbus_get_dram_status;
> + priv->profile.freq_table = priv->freq_table;
> + priv->profile.max_state = max_state;
> +
> + base_freq = clk_get_rate(clk_get_parent(priv->clk_dram));
> + for (i = 0, div = variant->max_dram_divider; i < max_state; ++i, --div)
> + priv->freq_table[i] = base_freq / div;

This patchset supports the case without OPP.
But, if you use dev_pm_opp_add() function at this point,
you can use the existing devfreq core without any modification for no OPP case.

Why don't you use dev_pm_opp_add at this point?

(snip)

--
Best Regards,
Chanwoo Choi
Samsung Electronics