Re: [RFC PATCH v1 4/6] PM / devfreq: event: support rockchip dfi controller
Hi Thierry, On 2016年06月04日 00:54, Thierry Reding wrote: On Fri, Jun 03, 2016 at 05:55:17PM +0800, Lin Huang wrote: [...] + ret = clk_prepare_enable(data->clk); + if (ret) { + dev_err(&pdev->dev, "failed to enable clk: %d\n", ret); + clk_disable_unprepare(data->clk); + return ret; + } This is going to give you a large WARN. clk_prepare_enable() already leaves the clock in a proper state when it fails (i.e. it calls clk_unprepare() if the clk_enable() part failed), so calling clk_disable_unprepare() upon failure is going to unbalance the reference counts. Thanks for reminding, i will fix it next version. Thierry -- Lin Huang
Re: [RFC PATCH v1 4/6] PM / devfreq: event: support rockchip dfi controller
On 2016年06月03日 18:26, Chanwoo Choi wrote: Hi Lin, I add the some comment on below. If you modify it, You can add my acked-by tag. Looks good to me. Thanks for you reviewing, i will update the code folloiwing your comment. Acked-by: Chanwoo Choi Also, I'd like you to add me to mail thread on next version because I'm supporter of devfreq-event. I am sorry for missing you mail in before patch:-[ , will add you to mail thread next vesion. On 2016년 06월 03일 18:55, Lin Huang wrote: on rk3399 platform, there is dfi conroller can monitor ddr load, base on this result, we can do ddr freqency scaling. Signed-off-by: Lin Huang --- Changes in v1: - NOne drivers/devfreq/event/Kconfig| 7 + drivers/devfreq/event/Makefile | 1 + drivers/devfreq/event/rockchip-dfi.c | 265 +++ 3 files changed, 273 insertions(+) create mode 100644 drivers/devfreq/event/rockchip-dfi.c diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig index 1e8b4f4..6ebdc13 100644 --- a/drivers/devfreq/event/Kconfig +++ b/drivers/devfreq/event/Kconfig @@ -30,4 +30,11 @@ config DEVFREQ_EVENT_EXYNOS_PPMU (Platform Performance Monitoring Unit) counters to estimate the utilization of each module. +config DEVFREQ_EVENT_ROCKCHIP_DFI + bool "ROCKCHIP DFI DEVFREQ event Driver" + depends on ARCH_ROCKCHIP + help + This add the devfreq-event driver for Rockchip SoC. It provides DFI You better to full name of 'DFI' abbreviation as following: - DFI (Dxxx Fxxx Ixxx) + driver to count ddr load. + endif # PM_DEVFREQ_EVENT diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile index 3d6afd3..dda7090 100644 --- a/drivers/devfreq/event/Makefile +++ b/drivers/devfreq/event/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o +obj-$(CONFIG_DEVFREQ_EVENT_ROCKCHIP_DFI) += rockchip-dfi.o diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c new file mode 100644 index 000..e3b020f9 --- /dev/null +++ b/drivers/devfreq/event/rockchip-dfi.c @@ -0,0 +1,265 @@ +/* + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd + * Author: Lin Huang + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define RK3399_DMC_NUM_CH 2 + +/* DDRMON_CTRL */ +#define DDRMON_CTRL0x04 +#define CLR_DDRMON_CTRL(0x1f << 0) +#define LPDDR4_EN (0x10001 << 4) +#define HARDWARE_EN(0x10001 << 3) +#define LPDDR3_EN (0x10001 << 2) +#define SOFTWARE_EN(0x10001 << 1) +#define TIME_CNT_EN(0x10001 << 0) + +#define DDRMON_CH0_COUNT_NUM 0x28 +#define DDRMON_CH0_DFI_ACCESS_NUM 0x2c +#define DDRMON_CH1_COUNT_NUM 0x3c +#define DDRMON_CH1_DFI_ACCESS_NUM 0x40 + +/* pmu grf */ +#define PMUGRF_OS_REG2 0x308 +#define DDRTYPE_SHIFT 13 +#define DDRTYPE_MASK 7 + +enum { + DDR3 = 3, + LPDDR3 = 6, + LPDDR4 = 7, + UNUSED = 0xFF +}; + +struct dmc_usage { + u32 access; + u32 total; +}; + +struct rockchip_dfi { + struct devfreq_event_dev *edev; + struct devfreq_event_desc *desc; + struct dmc_usage ch_usage[RK3399_DMC_NUM_CH]; + struct device *dev; + void __iomem *regs; + struct regmap *regmap_pmu; + struct clk *clk; +}; + +static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) +{ + struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); + void __iomem *dfi_regs = info->regs; + u32 val; + u32 ddr_type; + + /* get ddr type */ + regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val); + ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK; + + /* clear DDRMON_CTRL setting */ + writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL); + + /* set ddr type to dfi */ + if (ddr_type == LPDDR3) + writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL); + else if (ddr_type == LPDDR4) + writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL); + + /* enable count, use software mode */ + writel_relaxed(SOFTWARE_EN, dfi_regs + DDRMON_CTRL); +} + +static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev *edev) +{ + struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); + void __iomem *df
Re: [RFC PATCH v1 4/6] PM / devfreq: event: support rockchip dfi controller
On Fri, Jun 03, 2016 at 05:55:17PM +0800, Lin Huang wrote: [...] > + ret = clk_prepare_enable(data->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable clk: %d\n", ret); > + clk_disable_unprepare(data->clk); > + return ret; > + } This is going to give you a large WARN. clk_prepare_enable() already leaves the clock in a proper state when it fails (i.e. it calls clk_unprepare() if the clk_enable() part failed), so calling clk_disable_unprepare() upon failure is going to unbalance the reference counts. Thierry signature.asc Description: PGP signature
Re: [RFC PATCH v1 4/6] PM / devfreq: event: support rockchip dfi controller
Hi Lin, I add the some comment on below. If you modify it, You can add my acked-by tag. Looks good to me. Acked-by: Chanwoo Choi Also, I'd like you to add me to mail thread on next version because I'm supporter of devfreq-event. On 2016년 06월 03일 18:55, Lin Huang wrote: > on rk3399 platform, there is dfi conroller can monitor > ddr load, base on this result, we can do ddr freqency > scaling. > > Signed-off-by: Lin Huang > --- > Changes in v1: > - NOne > > drivers/devfreq/event/Kconfig| 7 + > drivers/devfreq/event/Makefile | 1 + > drivers/devfreq/event/rockchip-dfi.c | 265 > +++ > 3 files changed, 273 insertions(+) > create mode 100644 drivers/devfreq/event/rockchip-dfi.c > > diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig > index 1e8b4f4..6ebdc13 100644 > --- a/drivers/devfreq/event/Kconfig > +++ b/drivers/devfreq/event/Kconfig > @@ -30,4 +30,11 @@ config DEVFREQ_EVENT_EXYNOS_PPMU > (Platform Performance Monitoring Unit) counters to estimate the > utilization of each module. > > +config DEVFREQ_EVENT_ROCKCHIP_DFI > + bool "ROCKCHIP DFI DEVFREQ event Driver" > + depends on ARCH_ROCKCHIP > + help > + This add the devfreq-event driver for Rockchip SoC. It provides DFI You better to full name of 'DFI' abbreviation as following: - DFI (Dxxx Fxxx Ixxx) > + driver to count ddr load. > + > endif # PM_DEVFREQ_EVENT > diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile > index 3d6afd3..dda7090 100644 > --- a/drivers/devfreq/event/Makefile > +++ b/drivers/devfreq/event/Makefile > @@ -2,3 +2,4 @@ > > obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o > obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o > +obj-$(CONFIG_DEVFREQ_EVENT_ROCKCHIP_DFI) += rockchip-dfi.o > diff --git a/drivers/devfreq/event/rockchip-dfi.c > b/drivers/devfreq/event/rockchip-dfi.c > new file mode 100644 > index 000..e3b020f9 > --- /dev/null > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -0,0 +1,265 @@ > +/* > + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd > + * Author: Lin Huang > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define RK3399_DMC_NUM_CH2 > + > +/* DDRMON_CTRL */ > +#define DDRMON_CTRL 0x04 > +#define CLR_DDRMON_CTRL (0x1f << 0) > +#define LPDDR4_EN(0x10001 << 4) > +#define HARDWARE_EN (0x10001 << 3) > +#define LPDDR3_EN(0x10001 << 2) > +#define SOFTWARE_EN (0x10001 << 1) > +#define TIME_CNT_EN (0x10001 << 0) > + > +#define DDRMON_CH0_COUNT_NUM 0x28 > +#define DDRMON_CH0_DFI_ACCESS_NUM0x2c > +#define DDRMON_CH1_COUNT_NUM 0x3c > +#define DDRMON_CH1_DFI_ACCESS_NUM0x40 > + > +/* pmu grf */ > +#define PMUGRF_OS_REG2 0x308 > +#define DDRTYPE_SHIFT13 > +#define DDRTYPE_MASK 7 > + > +enum { > + DDR3 = 3, > + LPDDR3 = 6, > + LPDDR4 = 7, > + UNUSED = 0xFF > +}; > + > +struct dmc_usage { > + u32 access; > + u32 total; > +}; > + > +struct rockchip_dfi { > + struct devfreq_event_dev *edev; > + struct devfreq_event_desc *desc; > + struct dmc_usage ch_usage[RK3399_DMC_NUM_CH]; > + struct device *dev; > + void __iomem *regs; > + struct regmap *regmap_pmu; > + struct clk *clk; > +}; > + > +static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev > *edev) > +{ > + struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + void __iomem *dfi_regs = info->regs; > + u32 val; > + u32 ddr_type; > + > + /* get ddr type */ > + regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val); > + ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK; > + > + /* clear DDRMON_CTRL setting */ > + writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL); > + > + /* set ddr type to dfi */ > + if (ddr_type == LPDDR3) > + writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL); > + else if (ddr_type == LPDDR4) > + writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL); > + > + /* enable count, use software mode */ > + writel_relaxed(SOFTWARE_EN, dfi_regs + DDRMON_CTRL); > +} > + > +static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev > *edev) > +{ > + struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + void __iomem *dfi_regs = info->regs