| Message ID | 20230524083153.2046084-1-s.hauer@pengutronix.de |
|---|---|
| Headers | show |
| Series | Add perf support to the rockchip-dfi driver | expand |
Hi, On Wed, May 24, 2023 at 10:31:47AM +0200, Sascha Hauer wrote: > Add support for the RK3588 to the driver. The RK3588 has four DDR > channels with a register stride of 0x4000 between the channel > registers, also it has a DDRMON_CTRL register per channel. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 30 +++++++++++++++++++++++++++- > include/soc/rockchip/rk3588_grf.h | 18 +++++++++++++++++ > 2 files changed, 47 insertions(+), 1 deletion(-) > create mode 100644 include/soc/rockchip/rk3588_grf.h > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 23d66fe737975..1410d20f3df80 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -26,8 +26,9 @@ > #include <soc/rockchip/rockchip_grf.h> > #include <soc/rockchip/rk3399_grf.h> > #include <soc/rockchip/rk3568_grf.h> > +#include <soc/rockchip/rk3588_grf.h> > > -#define DMC_MAX_CHANNELS 2 > +#define DMC_MAX_CHANNELS 4 > > #define HIWORD_UPDATE(val, mask) ((val) | (mask) << 16) > > @@ -711,9 +712,36 @@ static int rk3568_dfi_init(struct rockchip_dfi *dfi) > return 0; > }; > > +static int rk3588_dfi_init(struct rockchip_dfi *dfi) > +{ > + struct regmap *regmap_pmu = dfi->regmap_pmu; > + u32 reg2, reg3, reg4; > + > + regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG2, ®2); > + regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG3, ®3); > + regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG4, ®4); > + > + dfi->ddr_type = FIELD_GET(RK3588_PMUGRF_OS_REG2_DRAMTYPE_INFO, reg2); > + > + if (FIELD_GET(RK3588_PMUGRF_OS_REG3_SYSREG_VERSION, reg3) >= 0x3) > + dfi->ddr_type |= FIELD_GET(RK3588_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3, reg3) << 3; > + > + dfi->buswidth[0] = FIELD_GET(RK3588_PMUGRF_OS_REG2_BW_CH0, reg2) == 0 ? 4 : 2; > + dfi->buswidth[1] = FIELD_GET(RK3588_PMUGRF_OS_REG2_BW_CH1, reg2) == 0 ? 4 : 2; > + dfi->buswidth[2] = FIELD_GET(RK3568_PMUGRF_OS_REG2_BW_CH0, reg4) == 0 ? 4 : 2; > + dfi->buswidth[3] = FIELD_GET(RK3588_PMUGRF_OS_REG2_BW_CH1, reg4) == 0 ? 4 : 2; > + dfi->channel_mask = FIELD_GET(RK3588_PMUGRF_OS_REG2_CH_INFO, reg2) | > + FIELD_GET(RK3588_PMUGRF_OS_REG2_CH_INFO, reg4) << 2; > + > + dfi->ddrmon_stride = 0x4000; > + > + return 0; > +}; > + > static const struct of_device_id rockchip_dfi_id_match[] = { > { .compatible = "rockchip,rk3399-dfi", .data = rk3399_dfi_init }, > { .compatible = "rockchip,rk3568-dfi", .data = rk3568_dfi_init }, > + { .compatible = "rockchip,rk3588-dfi", .data = rk3588_dfi_init }, > { }, > }; > > diff --git a/include/soc/rockchip/rk3588_grf.h b/include/soc/rockchip/rk3588_grf.h > new file mode 100644 > index 0000000000000..630b35a550640 > --- /dev/null > +++ b/include/soc/rockchip/rk3588_grf.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +#ifndef __SOC_RK3588_GRF_H > +#define __SOC_RK3588_GRF_H > + > +#define RK3588_PMUGRF_OS_REG2 0x208 > +#define RK3588_PMUGRF_OS_REG2_DRAMTYPE_INFO GENMASK(15, 13) > +#define RK3588_PMUGRF_OS_REG2_BW_CH0 GENMASK(3, 2) > +#define RK3588_PMUGRF_OS_REG2_BW_CH1 GENMASK(19, 18) > +#define RK3588_PMUGRF_OS_REG2_CH_INFO GENMASK(29, 28) > + > +#define RK3588_PMUGRF_OS_REG3 0x20c > +#define RK3588_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3 GENMASK(13, 12) > +#define RK3588_PMUGRF_OS_REG3_SYSREG_VERSION GENMASK(31, 28) > + > +#define RK3588_PMUGRF_OS_REG4 0x210 > +#define RK3588_PMUGRF_OS_REG5 0x214 > + > +#endif /* __SOC_RK3588_GRF_H */ > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:29AM +0200, Sascha Hauer wrote: > As a matter of fact the regmap_pmu already is mandatory because > it is used unconditionally in the driver. Bail out gracefully in > probe() rather than crashing later. > > Fixes: b9d1262bca0af ("PM / devfreq: event: support rockchip dfi controller") > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > > Notes: > Changes since v4: > - move to beginning of the series to make it easier to backport to stable > - Add a Fixes: tag > - add missing of_node_put() > > drivers/devfreq/event/rockchip-dfi.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 39ac069cabc75..74893c06aa087 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -193,14 +193,15 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > return dev_err_probe(dev, PTR_ERR(data->clk), > "Cannot get the clk pclk_ddr_mon\n"); > > - /* try to find the optional reference to the pmu syscon */ > node = of_parse_phandle(np, "rockchip,pmu", 0); > - if (node) { > - data->regmap_pmu = syscon_node_to_regmap(node); > - of_node_put(node); > - if (IS_ERR(data->regmap_pmu)) > - return PTR_ERR(data->regmap_pmu); > - } > + if (!node) > + return dev_err_probe(&pdev->dev, -ENODEV, "Can't find pmu_grf registers\n"); > + > + data->regmap_pmu = syscon_node_to_regmap(node); > + of_node_put(node); > + if (IS_ERR(data->regmap_pmu)) > + return PTR_ERR(data->regmap_pmu); > + > data->dev = dev; > > desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:30AM +0200, Sascha Hauer wrote: > No need for an extra allocation, just embed the struct > devfreq_event_desc into the private data struct. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 74893c06aa087..467f9f42d38f7 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -49,7 +49,7 @@ struct dmc_usage { > */ > struct rockchip_dfi { > struct devfreq_event_dev *edev; > - struct devfreq_event_desc *desc; > + struct devfreq_event_desc desc; > struct dmc_usage ch_usage[RK3399_DMC_NUM_CH]; > struct device *dev; > void __iomem *regs; > @@ -204,14 +204,10 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > > data->dev = dev; > > - desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); > - if (!desc) > - return -ENOMEM; > - > + desc = &data->desc; > desc->ops = &rockchip_dfi_ops; > desc->driver_data = data; > desc->name = np->name; > - data->desc = desc; > > data->edev = devm_devfreq_event_add_edev(&pdev->dev, desc); > if (IS_ERR(data->edev)) { > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:31AM +0200, Sascha Hauer wrote: > The variable name for the private data struct is 'info' in some > functions and 'data' in others. Both names do not give a clue what > type the variable has, so consistently use 'dfi'. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 72 ++++++++++++++-------------- > 1 file changed, 36 insertions(+), 36 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 467f9f42d38f7..e19e5acaa362c 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -59,13 +59,13 @@ struct rockchip_dfi { > > 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; > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > + void __iomem *dfi_regs = dfi->regs; > u32 val; > u32 ddr_type; > > /* get ddr type */ > - regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val); > + regmap_read(dfi->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val); > ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) & > RK3399_PMUGRF_DDRTYPE_MASK; > > @@ -84,28 +84,28 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) > > 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; > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > + void __iomem *dfi_regs = dfi->regs; > > writel_relaxed(SOFTWARE_DIS, dfi_regs + DDRMON_CTRL); > } > > static int rockchip_dfi_get_busier_ch(struct devfreq_event_dev *edev) > { > - struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > u32 tmp, max = 0; > u32 i, busier_ch = 0; > - void __iomem *dfi_regs = info->regs; > + void __iomem *dfi_regs = dfi->regs; > > rockchip_dfi_stop_hardware_counter(edev); > > /* Find out which channel is busier */ > for (i = 0; i < RK3399_DMC_NUM_CH; i++) { > - info->ch_usage[i].access = readl_relaxed(dfi_regs + > + dfi->ch_usage[i].access = readl_relaxed(dfi_regs + > DDRMON_CH0_DFI_ACCESS_NUM + i * 20) * 4; > - info->ch_usage[i].total = readl_relaxed(dfi_regs + > + dfi->ch_usage[i].total = readl_relaxed(dfi_regs + > DDRMON_CH0_COUNT_NUM + i * 20); > - tmp = info->ch_usage[i].access; > + tmp = dfi->ch_usage[i].access; > if (tmp > max) { > busier_ch = i; > max = tmp; > @@ -118,20 +118,20 @@ static int rockchip_dfi_get_busier_ch(struct devfreq_event_dev *edev) > > static int rockchip_dfi_disable(struct devfreq_event_dev *edev) > { > - struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > > rockchip_dfi_stop_hardware_counter(edev); > - clk_disable_unprepare(info->clk); > + clk_disable_unprepare(dfi->clk); > > return 0; > } > > static int rockchip_dfi_enable(struct devfreq_event_dev *edev) > { > - struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > int ret; > > - ret = clk_prepare_enable(info->clk); > + ret = clk_prepare_enable(dfi->clk); > if (ret) { > dev_err(&edev->dev, "failed to enable dfi clk: %d\n", ret); > return ret; > @@ -149,13 +149,13 @@ static int rockchip_dfi_set_event(struct devfreq_event_dev *edev) > static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > struct devfreq_event_data *edata) > { > - struct rockchip_dfi *info = devfreq_event_get_drvdata(edev); > + struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > int busier_ch; > > busier_ch = rockchip_dfi_get_busier_ch(edev); > > - edata->load_count = info->ch_usage[busier_ch].access; > - edata->total_count = info->ch_usage[busier_ch].total; > + edata->load_count = dfi->ch_usage[busier_ch].access; > + edata->total_count = dfi->ch_usage[busier_ch].total; > > return 0; > } > @@ -176,47 +176,47 @@ MODULE_DEVICE_TABLE(of, rockchip_dfi_id_match); > static int rockchip_dfi_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > - struct rockchip_dfi *data; > + struct rockchip_dfi *dfi; > struct devfreq_event_desc *desc; > struct device_node *np = pdev->dev.of_node, *node; > > - data = devm_kzalloc(dev, sizeof(struct rockchip_dfi), GFP_KERNEL); > - if (!data) > + dfi = devm_kzalloc(dev, sizeof(*dfi), GFP_KERNEL); > + if (!dfi) > return -ENOMEM; > > - data->regs = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(data->regs)) > - return PTR_ERR(data->regs); > + dfi->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(dfi->regs)) > + return PTR_ERR(dfi->regs); > > - data->clk = devm_clk_get(dev, "pclk_ddr_mon"); > - if (IS_ERR(data->clk)) > - return dev_err_probe(dev, PTR_ERR(data->clk), > + dfi->clk = devm_clk_get(dev, "pclk_ddr_mon"); > + if (IS_ERR(dfi->clk)) > + return dev_err_probe(dev, PTR_ERR(dfi->clk), > "Cannot get the clk pclk_ddr_mon\n"); > > node = of_parse_phandle(np, "rockchip,pmu", 0); > if (!node) > return dev_err_probe(&pdev->dev, -ENODEV, "Can't find pmu_grf registers\n"); > > - data->regmap_pmu = syscon_node_to_regmap(node); > + dfi->regmap_pmu = syscon_node_to_regmap(node); > of_node_put(node); > - if (IS_ERR(data->regmap_pmu)) > - return PTR_ERR(data->regmap_pmu); > + if (IS_ERR(dfi->regmap_pmu)) > + return PTR_ERR(dfi->regmap_pmu); > > - data->dev = dev; > + dfi->dev = dev; > > - desc = &data->desc; > + desc = &dfi->desc; > desc->ops = &rockchip_dfi_ops; > - desc->driver_data = data; > + desc->driver_data = dfi; > desc->name = np->name; > > - data->edev = devm_devfreq_event_add_edev(&pdev->dev, desc); > - if (IS_ERR(data->edev)) { > + dfi->edev = devm_devfreq_event_add_edev(&pdev->dev, desc); > + if (IS_ERR(dfi->edev)) { > dev_err(&pdev->dev, > "failed to add devfreq-event device\n"); > - return PTR_ERR(data->edev); > + return PTR_ERR(dfi->edev); > } > > - platform_set_drvdata(pdev, data); > + platform_set_drvdata(pdev, dfi); > > return 0; > } > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:32AM +0200, Sascha Hauer wrote: > Move the RK3399 specifics to a SoC specific init function to make > the way free for supporting other SoCs later. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > > Notes: > Changes since v4: > - use of_device_get_match_data() > - use a callback rather than a struct type as driver data > > drivers/devfreq/event/rockchip-dfi.c | 48 +++++++++++++++++++--------- > 1 file changed, 33 insertions(+), 15 deletions(-) > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index e19e5acaa362c..6b1ef29df7048 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -17,6 +17,7 @@ > #include <linux/slab.h> > #include <linux/list.h> > #include <linux/of.h> > +#include <linux/of_device.h> > > #include <soc/rockchip/rk3399_grf.h> > > @@ -55,27 +56,21 @@ struct rockchip_dfi { > void __iomem *regs; > struct regmap *regmap_pmu; > struct clk *clk; > + u32 ddr_type; > }; > > static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > void __iomem *dfi_regs = dfi->regs; > - u32 val; > - u32 ddr_type; > - > - /* get ddr type */ > - regmap_read(dfi->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val); > - ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) & > - RK3399_PMUGRF_DDRTYPE_MASK; > > /* clear DDRMON_CTRL setting */ > writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL); > > /* set ddr type to dfi */ > - if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3) > + if (dfi->ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3) > writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL); > - else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4) > + else if (dfi->ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4) > writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL); > > /* enable count, use software mode */ > @@ -167,8 +162,26 @@ static const struct devfreq_event_ops rockchip_dfi_ops = { > .set_event = rockchip_dfi_set_event, > }; > > +static int rk3399_dfi_init(struct rockchip_dfi *dfi) > +{ > + struct regmap *regmap_pmu = dfi->regmap_pmu; > + u32 val; > + > + dfi->clk = devm_clk_get(dfi->dev, "pclk_ddr_mon"); > + if (IS_ERR(dfi->clk)) > + return dev_err_probe(dfi->dev, PTR_ERR(dfi->clk), > + "Cannot get the clk pclk_ddr_mon\n"); > + > + /* get ddr type */ > + regmap_read(regmap_pmu, RK3399_PMUGRF_OS_REG2, &val); > + dfi->ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) & > + RK3399_PMUGRF_DDRTYPE_MASK; > + > + return 0; > +}; > + > static const struct of_device_id rockchip_dfi_id_match[] = { > - { .compatible = "rockchip,rk3399-dfi" }, > + { .compatible = "rockchip,rk3399-dfi", .data = rk3399_dfi_init }, > { }, > }; > MODULE_DEVICE_TABLE(of, rockchip_dfi_id_match); > @@ -179,6 +192,12 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > struct rockchip_dfi *dfi; > struct devfreq_event_desc *desc; > struct device_node *np = pdev->dev.of_node, *node; > + int (*soc_init)(struct rockchip_dfi *dfi); > + int ret; > + > + soc_init = of_device_get_match_data(&pdev->dev); > + if (!soc_init) > + return -EINVAL; > > dfi = devm_kzalloc(dev, sizeof(*dfi), GFP_KERNEL); > if (!dfi) > @@ -188,11 +207,6 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > if (IS_ERR(dfi->regs)) > return PTR_ERR(dfi->regs); > > - dfi->clk = devm_clk_get(dev, "pclk_ddr_mon"); > - if (IS_ERR(dfi->clk)) > - return dev_err_probe(dev, PTR_ERR(dfi->clk), > - "Cannot get the clk pclk_ddr_mon\n"); > - > node = of_parse_phandle(np, "rockchip,pmu", 0); > if (!node) > return dev_err_probe(&pdev->dev, -ENODEV, "Can't find pmu_grf registers\n"); > @@ -209,6 +223,10 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > desc->driver_data = dfi; > desc->name = np->name; > > + ret = soc_init(dfi); > + if (ret) > + return ret; > + > dfi->edev = devm_devfreq_event_add_edev(&pdev->dev, desc); > if (IS_ERR(dfi->edev)) { > dev_err(&pdev->dev, > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:34AM +0200, Sascha Hauer wrote: > The DDR_MON counters are free running counters. These are resetted to 0 > when starting them over like currently done when reading the current > counter values. > > Resetting the counters becomes a problem with perf support we want to > add later, because perf needs counters that are not modified elsewhere. > > This patch removes resetting the counters and keeps them running > instead. That means we no longer use the absolute counter values but > instead compare them with the counter values we read last time. Not > stopping the counters also has the impact that they are running while > we are reading them. We cannot read multiple timers atomically, so > the values do not exactly fit together. The effect should be negligible > though as the time between two measurements is some orders of magnitude > bigger than the time we need to read multiple registers. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > > Notes: > Changes since v4: > - rephrase commit message > - Drop unused variable > > drivers/devfreq/event/rockchip-dfi.c | 52 ++++++++++++++++------------ > 1 file changed, 30 insertions(+), 22 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 680f629da64fc..126bb744645b6 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -38,11 +38,15 @@ > #define DDRMON_CH1_COUNT_NUM 0x3c > #define DDRMON_CH1_DFI_ACCESS_NUM 0x40 > > -struct dmc_usage { > +struct dmc_count_channel { > u32 access; > u32 total; > }; > > +struct dmc_count { > + struct dmc_count_channel c[RK3399_DMC_NUM_CH]; > +}; > + > /* > * The dfi controller can monitor DDR load. It has an upper and lower threshold > * for the operating points. Whenever the usage leaves these bounds an event is > @@ -51,7 +55,7 @@ struct dmc_usage { > struct rockchip_dfi { > struct devfreq_event_dev *edev; > struct devfreq_event_desc desc; > - struct dmc_usage ch_usage[RK3399_DMC_NUM_CH]; > + struct dmc_count last_event_count; > struct device *dev; > void __iomem *regs; > struct regmap *regmap_pmu; > @@ -85,30 +89,18 @@ static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev *edev) > writel_relaxed(SOFTWARE_DIS, dfi_regs + DDRMON_CTRL); > } > > -static int rockchip_dfi_get_busier_ch(struct devfreq_event_dev *edev) > +static void rockchip_dfi_read_counters(struct devfreq_event_dev *edev, struct dmc_count *count) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > - u32 tmp, max = 0; > - u32 i, busier_ch = 0; > + u32 i; > void __iomem *dfi_regs = dfi->regs; > > - rockchip_dfi_stop_hardware_counter(edev); > - > - /* Find out which channel is busier */ > for (i = 0; i < RK3399_DMC_NUM_CH; i++) { > - dfi->ch_usage[i].access = readl_relaxed(dfi_regs + > + count->c[i].access = readl_relaxed(dfi_regs + > DDRMON_CH0_DFI_ACCESS_NUM + i * 20); > - dfi->ch_usage[i].total = readl_relaxed(dfi_regs + > + count->c[i].total = readl_relaxed(dfi_regs + > DDRMON_CH0_COUNT_NUM + i * 20); > - tmp = dfi->ch_usage[i].access; > - if (tmp > max) { > - busier_ch = i; > - max = tmp; > - } > } > - rockchip_dfi_start_hardware_counter(edev); > - > - return busier_ch; > } > > static int rockchip_dfi_disable(struct devfreq_event_dev *edev) > @@ -145,12 +137,28 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > struct devfreq_event_data *edata) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > - int busier_ch; > + struct dmc_count count; > + struct dmc_count *last = &dfi->last_event_count; > + u32 access = 0, total = 0; > + int i; > + > + rockchip_dfi_read_counters(edev, &count); > + > + /* We can only report one channel, so find the busiest one */ > + for (i = 0; i < RK3399_DMC_NUM_CH; i++) { > + u32 a = count.c[i].access - last->c[i].access; > + u32 t = count.c[i].total - last->c[i].total; > + > + if (a > access) { > + access = a; > + total = t; > + } > + } > > - busier_ch = rockchip_dfi_get_busier_ch(edev); > + edata->load_count = access * 4; > + edata->total_count = total; > > - edata->load_count = dfi->ch_usage[busier_ch].access * 4; > - edata->total_count = dfi->ch_usage[busier_ch].total; > + dfi->last_event_count = count; > > return 0; > } > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:35AM +0200, Sascha Hauer wrote: > Different Rockchip SoC variants have a different number of channels. > Introduce a channel mask to make the number of channels configurable > from SoC initialization code. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 126bb744645b6..82de24a027579 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -18,10 +18,11 @@ > #include <linux/list.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/bits.h> > > #include <soc/rockchip/rk3399_grf.h> > > -#define RK3399_DMC_NUM_CH 2 > +#define DMC_MAX_CHANNELS 2 > > /* DDRMON_CTRL */ > #define DDRMON_CTRL 0x04 > @@ -44,7 +45,7 @@ struct dmc_count_channel { > }; > > struct dmc_count { > - struct dmc_count_channel c[RK3399_DMC_NUM_CH]; > + struct dmc_count_channel c[DMC_MAX_CHANNELS]; > }; > > /* > @@ -61,6 +62,7 @@ struct rockchip_dfi { > struct regmap *regmap_pmu; > struct clk *clk; > u32 ddr_type; > + unsigned int channel_mask; > }; > > static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) > @@ -95,7 +97,9 @@ static void rockchip_dfi_read_counters(struct devfreq_event_dev *edev, struct dm > u32 i; > void __iomem *dfi_regs = dfi->regs; > > - for (i = 0; i < RK3399_DMC_NUM_CH; i++) { > + for (i = 0; i < DMC_MAX_CHANNELS; i++) { > + if (!(dfi->channel_mask & BIT(i))) > + continue; > count->c[i].access = readl_relaxed(dfi_regs + > DDRMON_CH0_DFI_ACCESS_NUM + i * 20); > count->c[i].total = readl_relaxed(dfi_regs + > @@ -145,9 +149,14 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > rockchip_dfi_read_counters(edev, &count); > > /* We can only report one channel, so find the busiest one */ > - for (i = 0; i < RK3399_DMC_NUM_CH; i++) { > - u32 a = count.c[i].access - last->c[i].access; > - u32 t = count.c[i].total - last->c[i].total; > + for (i = 0; i < DMC_MAX_CHANNELS; i++) { > + u32 a, t; > + > + if (!(dfi->channel_mask & BIT(i))) > + continue; > + > + a = count.c[i].access - last->c[i].access; > + t = count.c[i].total - last->c[i].total; > > if (a > access) { > access = a; > @@ -185,6 +194,8 @@ static int rk3399_dfi_init(struct rockchip_dfi *dfi) > dfi->ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) & > RK3399_PMUGRF_DDRTYPE_MASK; > > + dfi->channel_mask = GENMASK(1, 0); > + > return 0; > }; > > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:37AM +0200, Sascha Hauer wrote: > Use the HIWORD_UPDATE() define known from other rockchip drivers to > make the defines look less odd to the readers who've seen other > rockchip drivers. > > The HIWORD registers have their functional bits in the lower 16 bits > whereas the upper 16 bits contain a mask. Only the functional bits that > have the corresponding mask bit set are modified during a write. Although > the register writes look different, the end result should be the same, > at least there's no functional change intended with this patch. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 33 ++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 6bccb6fbcfc0c..6b3ef97b3be09 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -26,15 +26,19 @@ > > #define DMC_MAX_CHANNELS 2 > > +#define HIWORD_UPDATE(val, mask) ((val) | (mask) << 16) > + > /* DDRMON_CTRL */ > #define DDRMON_CTRL 0x04 > -#define CLR_DDRMON_CTRL (0x1f0000 << 0) > -#define LPDDR4_EN (0x10001 << 4) > -#define HARDWARE_EN (0x10001 << 3) > -#define LPDDR3_EN (0x10001 << 2) > -#define SOFTWARE_EN (0x10001 << 1) > -#define SOFTWARE_DIS (0x10000 << 1) > -#define TIME_CNT_EN (0x10001 << 0) > +#define DDRMON_CTRL_DDR4 BIT(5) > +#define DDRMON_CTRL_LPDDR4 BIT(4) > +#define DDRMON_CTRL_HARDWARE_EN BIT(3) > +#define DDRMON_CTRL_LPDDR23 BIT(2) > +#define DDRMON_CTRL_SOFTWARE_EN BIT(1) > +#define DDRMON_CTRL_TIMER_CNT_EN BIT(0) > +#define DDRMON_CTRL_DDR_TYPE_MASK (DDRMON_CTRL_DDR4 | \ > + DDRMON_CTRL_LPDDR4 | \ > + DDRMON_CTRL_LPDDR23) > > #define DDRMON_CH0_COUNT_NUM 0x28 > #define DDRMON_CH0_DFI_ACCESS_NUM 0x2c > @@ -73,16 +77,20 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) > void __iomem *dfi_regs = dfi->regs; > > /* clear DDRMON_CTRL setting */ > - writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL); > + writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_TIMER_CNT_EN | DDRMON_CTRL_SOFTWARE_EN | > + DDRMON_CTRL_HARDWARE_EN), dfi_regs + DDRMON_CTRL); > > /* set ddr type to dfi */ > if (dfi->ddr_type == ROCKCHIP_DDRTYPE_LPDDR3) > - writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL); > + writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_LPDDR23, DDRMON_CTRL_DDR_TYPE_MASK), > + dfi_regs + DDRMON_CTRL); > else if (dfi->ddr_type == ROCKCHIP_DDRTYPE_LPDDR4) > - writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL); > + writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_LPDDR4, DDRMON_CTRL_DDR_TYPE_MASK), > + dfi_regs + DDRMON_CTRL); > > /* enable count, use software mode */ > - writel_relaxed(SOFTWARE_EN, dfi_regs + DDRMON_CTRL); > + writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > + dfi_regs + DDRMON_CTRL); > } > > static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev *edev) > @@ -90,7 +98,8 @@ static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev *edev) > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > void __iomem *dfi_regs = dfi->regs; > > - writel_relaxed(SOFTWARE_DIS, dfi_regs + DDRMON_CTRL); > + writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > + dfi_regs + DDRMON_CTRL); > } > > static void rockchip_dfi_read_counters(struct devfreq_event_dev *edev, struct dmc_count *count) > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:42AM +0200, Sascha Hauer wrote: > When adding perf support later the DFI must be enabled when > either of devfreq-event or perf is active. Prepare for that > by adding a usage counter for the DFI. Also move enabling > and disabling of the clock away from the devfreq-event specific > functions to which the perf specific part won't have access. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 57 +++++++++++++++++++--------- > 1 file changed, 40 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index d39db5de7f19c..8a7af7c32ae0d 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -68,13 +68,28 @@ struct rockchip_dfi { > void __iomem *regs; > struct regmap *regmap_pmu; > struct clk *clk; > + int usecount; > + struct mutex mutex; > u32 ddr_type; > unsigned int channel_mask; > }; > > -static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > +static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > + int ret = 0; > + > + mutex_lock(&dfi->mutex); > + > + dfi->usecount++; > + if (dfi->usecount > 1) > + goto out; > + > + ret = clk_prepare_enable(dfi->clk); > + if (ret) { > + dev_err(&dfi->edev->dev, "failed to enable dfi clk: %d\n", ret); > + goto out; > + } > > /* clear DDRMON_CTRL setting */ > writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_TIMER_CNT_EN | DDRMON_CTRL_SOFTWARE_EN | > @@ -99,14 +114,30 @@ static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > /* enable count, use software mode */ > writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > +out: > + mutex_unlock(&dfi->mutex); > + > + return ret; > } > > -static void rockchip_dfi_stop_hardware_counter(struct rockchip_dfi *dfi) > +static void rockchip_dfi_disable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > > + mutex_lock(&dfi->mutex); > + > + dfi->usecount--; > + > + WARN_ON_ONCE(dfi->usecount < 0); > + > + if (dfi->usecount > 0) > + goto out; > + > writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > + clk_disable_unprepare(dfi->clk); > +out: > + mutex_unlock(&dfi->mutex); > } > > static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_count *count) > @@ -124,29 +155,20 @@ static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_coun > } > } > > -static int rockchip_dfi_disable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_disable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > > - rockchip_dfi_stop_hardware_counter(dfi); > - clk_disable_unprepare(dfi->clk); > + rockchip_dfi_disable(dfi); > > return 0; > } > > -static int rockchip_dfi_enable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_enable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > - int ret; > - > - ret = clk_prepare_enable(dfi->clk); > - if (ret) { > - dev_err(&edev->dev, "failed to enable dfi clk: %d\n", ret); > - return ret; > - } > > - rockchip_dfi_start_hardware_counter(dfi); > - return 0; > + return rockchip_dfi_enable(dfi); > } > > static int rockchip_dfi_set_event(struct devfreq_event_dev *edev) > @@ -190,8 +212,8 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > } > > static const struct devfreq_event_ops rockchip_dfi_ops = { > - .disable = rockchip_dfi_disable, > - .enable = rockchip_dfi_enable, > + .disable = rockchip_dfi_event_disable, > + .enable = rockchip_dfi_event_enable, > .get_event = rockchip_dfi_get_event, > .set_event = rockchip_dfi_set_event, > }; > @@ -272,6 +294,7 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > return PTR_ERR(dfi->regmap_pmu); > > dfi->dev = dev; > + mutex_init(&dfi->mutex); > > desc = &dfi->desc; > desc->ops = &rockchip_dfi_ops; > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:43AM +0200, Sascha Hauer wrote: > struct dmc_count_channel::total counts the clock cycles of the DDR > controller. Rename it accordingly to give the reader a better idea > what this is about. While at it, at some documentation to struct > dmc_count_channel. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 8a7af7c32ae0d..50e497455dc69 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -46,9 +46,14 @@ > #define DDRMON_CH1_COUNT_NUM 0x3c > #define DDRMON_CH1_DFI_ACCESS_NUM 0x40 > > +/** > + * struct dmc_count_channel - structure to hold counter values from the DDR controller > + * @access: Number of read and write accesses > + * @clock_cycles: DDR clock cycles > + */ > struct dmc_count_channel { > u32 access; > - u32 total; > + u32 clock_cycles; > }; > > struct dmc_count { > @@ -150,7 +155,7 @@ static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_coun > continue; > count->c[i].access = readl_relaxed(dfi_regs + > DDRMON_CH0_DFI_ACCESS_NUM + i * 20); > - count->c[i].total = readl_relaxed(dfi_regs + > + count->c[i].clock_cycles = readl_relaxed(dfi_regs + > DDRMON_CH0_COUNT_NUM + i * 20); > } > } > @@ -182,29 +187,29 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > struct dmc_count count; > struct dmc_count *last = &dfi->last_event_count; > - u32 access = 0, total = 0; > + u32 access = 0, clock_cycles = 0; > int i; > > rockchip_dfi_read_counters(dfi, &count); > > /* We can only report one channel, so find the busiest one */ > for (i = 0; i < DMC_MAX_CHANNELS; i++) { > - u32 a, t; > + u32 a, c; > > if (!(dfi->channel_mask & BIT(i))) > continue; > > a = count.c[i].access - last->c[i].access; > - t = count.c[i].total - last->c[i].total; > + c = count.c[i].clock_cycles - last->c[i].clock_cycles; > > if (a > access) { > access = a; > - total = t; > + clock_cycles = c; > } > } > > edata->load_count = access * 4; > - edata->total_count = total; > + edata->total_count = clock_cycles; > > dfi->last_event_count = count; > > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:45AM +0200, Sascha Hauer wrote: > The currently supported RK3399 has a stride of 20 between the channel > specific registers. Upcoming RK3588 has a different stride, so put > the stride into driver data to make it configurable. > While at it convert decimal 20 to hex 0x14 for consistency with RK3588 > which has a register stride 0x4000 and we want to write that in hex > as well. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index 88145688e3d9c..a872550a7caf5 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -112,6 +112,7 @@ struct rockchip_dfi { > int active_events; > int burst_len; > int buswidth[DMC_MAX_CHANNELS]; > + int ddrmon_stride; > }; > > static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > @@ -189,13 +190,13 @@ static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_coun > if (!(dfi->channel_mask & BIT(i))) > continue; > c->c[i].read_access = readl_relaxed(dfi_regs + > - DDRMON_CH0_RD_NUM + i * 20); > + DDRMON_CH0_RD_NUM + i * dfi->ddrmon_stride); > c->c[i].write_access = readl_relaxed(dfi_regs + > - DDRMON_CH0_WR_NUM + i * 20); > + DDRMON_CH0_WR_NUM + i * dfi->ddrmon_stride); > c->c[i].access = readl_relaxed(dfi_regs + > - DDRMON_CH0_DFI_ACCESS_NUM + i * 20); > + DDRMON_CH0_DFI_ACCESS_NUM + i * dfi->ddrmon_stride); > c->c[i].clock_cycles = readl_relaxed(dfi_regs + > - DDRMON_CH0_COUNT_NUM + i * 20); > + DDRMON_CH0_COUNT_NUM + i * dfi->ddrmon_stride); > } > } > > @@ -661,6 +662,8 @@ static int rk3399_dfi_init(struct rockchip_dfi *dfi) > dfi->buswidth[0] = FIELD_GET(RK3399_PMUGRF_OS_REG2_BW_CH0, val) == 0 ? 4 : 2; > dfi->buswidth[1] = FIELD_GET(RK3399_PMUGRF_OS_REG2_BW_CH1, val) == 0 ? 4 : 2; > > + dfi->ddrmon_stride = 0x14; > + > return 0; > }; > > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:46AM +0200, Sascha Hauer wrote: > The currently supported RK3399 has a set of registers per channel, but > it has only a single DDRMON_CTRL register. With upcoming RK3588 this > will be different, the RK3588 has a DDRMON_CTRL register per channel. > > Instead of expecting a single DDRMON_CTRL register, loop over the > channels and write the channel specific DDRMON_CTRL register. Break > out early out of the loop when there is only a single DDRMON_CTRL > register like on the RK3399. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/devfreq/event/rockchip-dfi.c | 72 ++++++++++++++++++---------- > 1 file changed, 48 insertions(+), 24 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index a872550a7caf5..23d66fe737975 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -113,12 +113,13 @@ struct rockchip_dfi { > int burst_len; > int buswidth[DMC_MAX_CHANNELS]; > int ddrmon_stride; > + bool ddrmon_ctrl_single; > }; > > static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > - int ret = 0; > + int i, ret = 0; > > mutex_lock(&dfi->mutex); > > @@ -132,29 +133,41 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > goto out; > } > > - /* clear DDRMON_CTRL setting */ > - writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_TIMER_CNT_EN | DDRMON_CTRL_SOFTWARE_EN | > - DDRMON_CTRL_HARDWARE_EN), dfi_regs + DDRMON_CTRL); > + for (i = 0; i < DMC_MAX_CHANNELS; i++) { > + u32 ctrl = 0; > > - /* set ddr type to dfi */ > - switch (dfi->ddr_type) { > - case ROCKCHIP_DDRTYPE_LPDDR2: > - case ROCKCHIP_DDRTYPE_LPDDR3: > - writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_LPDDR23, DDRMON_CTRL_DDR_TYPE_MASK), > - dfi_regs + DDRMON_CTRL); > - break; > - case ROCKCHIP_DDRTYPE_LPDDR4: > - case ROCKCHIP_DDRTYPE_LPDDR4X: > - writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_LPDDR4, DDRMON_CTRL_DDR_TYPE_MASK), > - dfi_regs + DDRMON_CTRL); > - break; > - default: > - break; > - } > + if (!(dfi->channel_mask & BIT(i))) > + continue; > > - /* enable count, use software mode */ > - writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > - dfi_regs + DDRMON_CTRL); > + /* clear DDRMON_CTRL setting */ > + writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_TIMER_CNT_EN | > + DDRMON_CTRL_SOFTWARE_EN | DDRMON_CTRL_HARDWARE_EN), > + dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL); > + > + /* set ddr type to dfi */ > + switch (dfi->ddr_type) { > + case ROCKCHIP_DDRTYPE_LPDDR2: > + case ROCKCHIP_DDRTYPE_LPDDR3: > + ctrl = DDRMON_CTRL_LPDDR23; > + break; > + case ROCKCHIP_DDRTYPE_LPDDR4: > + case ROCKCHIP_DDRTYPE_LPDDR4X: > + ctrl = DDRMON_CTRL_LPDDR4; > + break; > + default: > + break; > + } > + > + writel_relaxed(HIWORD_UPDATE(ctrl, DDRMON_CTRL_DDR_TYPE_MASK), > + dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL); > + > + /* enable count, use software mode */ > + writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > + dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL); > + > + if (dfi->ddrmon_ctrl_single) > + break; > + } > out: > mutex_unlock(&dfi->mutex); > > @@ -164,6 +177,7 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > static void rockchip_dfi_disable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > + int i; > > mutex_lock(&dfi->mutex); > > @@ -174,8 +188,17 @@ static void rockchip_dfi_disable(struct rockchip_dfi *dfi) > if (dfi->usecount > 0) > goto out; > > - writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > - dfi_regs + DDRMON_CTRL); > + for (i = 0; i < DMC_MAX_CHANNELS; i++) { > + if (!(dfi->channel_mask & BIT(i))) > + continue; > + > + writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > + dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL); > + > + if (dfi->ddrmon_ctrl_single) > + break; > + } > + > clk_disable_unprepare(dfi->clk); > out: > mutex_unlock(&dfi->mutex); > @@ -663,6 +686,7 @@ static int rk3399_dfi_init(struct rockchip_dfi *dfi) > dfi->buswidth[1] = FIELD_GET(RK3399_PMUGRF_OS_REG2_BW_CH1, val) == 0 ? 4 : 2; > > dfi->ddrmon_stride = 0x14; > + dfi->ddrmon_ctrl_single = true; > > return 0; > }; > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:48AM +0200, Sascha Hauer wrote: > Convert the Rockchip DFI binding to yaml. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > > Notes: > Changes since v4: > > - Revert to state of v3 (changes were lost in v4) > > .../bindings/devfreq/event/rockchip,dfi.yaml | 61 +++++++++++++++++++ > .../bindings/devfreq/event/rockchip-dfi.txt | 18 ------ > .../rockchip,rk3399-dmc.yaml | 2 +- > 3 files changed, 62 insertions(+), 19 deletions(-) > create mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml > delete mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt > > diff --git a/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml b/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml > new file mode 100644 > index 0000000000000..7a82f6ae0701e > --- /dev/null > +++ b/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/devfreq/event/rockchip,dfi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Rockchip DFI > + > +maintainers: > + - Sascha Hauer <s.hauer@pengutronix.de> > + > +properties: > + compatible: > + enum: > + - rockchip,rk3399-dfi > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: pclk_ddr_mon > + > + interrupts: > + maxItems: 1 > + > + reg: > + maxItems: 1 > + > + rockchip,pmu: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Phandle to the syscon managing the "PMU general register files". > + > +required: > + - compatible > + - clocks > + - clock-names > + - interrupts > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/clock/rk3308-cru.h> > + > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + dfi: dfi@ff630000 { > + compatible = "rockchip,rk3399-dfi"; > + reg = <0x00 0xff630000 0x00 0x4000>; > + interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>; > + rockchip,pmu = <&pmugrf>; > + clocks = <&cru PCLK_DDR_MON>; > + clock-names = "pclk_ddr_mon"; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt b/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt > deleted file mode 100644 > index 148191b0fc158..0000000000000 > --- a/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt > +++ /dev/null > @@ -1,18 +0,0 @@ > - > -* Rockchip rk3399 DFI device > - > -Required properties: > -- compatible: Must be "rockchip,rk3399-dfi". > -- reg: physical base address of each DFI and length of memory mapped region > -- rockchip,pmu: phandle to the syscon managing the "pmu general register files" > -- clocks: phandles for clock specified in "clock-names" property > -- clock-names : the name of clock used by the DFI, must be "pclk_ddr_mon"; > - > -Example: > - dfi: dfi@ff630000 { > - compatible = "rockchip,rk3399-dfi"; > - reg = <0x00 0xff630000 0x00 0x4000>; > - rockchip,pmu = <&pmugrf>; > - clocks = <&cru PCLK_DDR_MON>; > - clock-names = "pclk_ddr_mon"; > - }; > diff --git a/Documentation/devicetree/bindings/memory-controllers/rockchip,rk3399-dmc.yaml b/Documentation/devicetree/bindings/memory-controllers/rockchip,rk3399-dmc.yaml > index fb4920397d08e..aba8649aaeb10 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/rockchip,rk3399-dmc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/rockchip,rk3399-dmc.yaml > @@ -18,7 +18,7 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle > description: > Node to get DDR loading. Refer to > - Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt. > + Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml. > > clocks: > maxItems: 1 > -- > 2.39.2 >
Hi, On Wed, May 24, 2023 at 10:31:28AM +0200, Sascha Hauer wrote: > This is v5 of the series adding perf support to the rockchip DFI driver. > > A lot has changed in the perf driver since v4. First of all the review > feedback from Robin and Jonathan has been integrated. The perf driver > now not only supports monitoring the total DDR utilization, but also the > individual channels. I also reworked the way the raw 32bit counter > values are summed up to 64bit perf values, so hopefully the code is > easier to follow now. > > lockdep found out that that locking in the perf driver was broken, so I > reworked that as well. None of the perf hooks allows locking with > mutexes or spinlocks, so in perf it's not possible to enable the DFI > controller when needed. Instead I now unconditionally enable the DFI > controller during probe when perf is enabled. > > Furthermore the hrtimer I use for reading out the hardware counter > values before they overflow race with perf. Now a seqlock is used to > prevent that. > > The RK3588 device tree changes for the DFI were not part of v4. As > Vincent Legoll showed interest in testing this series the necessary > device tree changes are now part of this series. I tested the series on RK3588 EVB1. The read/write byts looks sensible. Sometimes cycles reads unrealistic values, though: Performance counter stats for 'system wide': 18446744070475110400 rockchip_ddr/cycles/ 828.63 MB rockchip_ddr/read-bytes/ 207.19 MB rockchip_ddr/read-bytes0/ 207.15 MB rockchip_ddr/read-bytes1/ 207.14 MB rockchip_ddr/read-bytes2/ 207.15 MB rockchip_ddr/read-bytes3/ 1.48 MB rockchip_ddr/write-bytes/ 0.37 MB rockchip_ddr/write-bytes0/ 0.37 MB rockchip_ddr/write-bytes1/ 0.37 MB rockchip_ddr/write-bytes2/ 0.38 MB rockchip_ddr/write-bytes3/ 830.12 MB rockchip_ddr/bytes/ 1.004239766 seconds time elapsed (This is with memdump running in parallel) Otherwise the series is Tested-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > Changes since v4: > - Add device tree changes for RK3588 > - Use seqlock to protect perf counter values from hrtimer > - Unconditionally enable DFI when perf is enabled > - Bring back changes to dts/binding patches that were lost in v4 > > Changes since v3: > - Add RK3588 support > > Changes since v2: > - Fix broken reference to binding > - Add Reviewed-by from Rob > > Changes since v1: > - Fix example to actually match the binding and fix the warnings resulted thereof > - Make addition of rockchip,rk3568-dfi an extra patch > > Sascha Hauer (25): > PM / devfreq: rockchip-dfi: Make pmu regmap mandatory > PM / devfreq: rockchip-dfi: Embed desc into private data struct > PM / devfreq: rockchip-dfi: use consistent name for private data > struct > PM / devfreq: rockchip-dfi: Add SoC specific init function > PM / devfreq: rockchip-dfi: dfi store raw values in counter struct > PM / devfreq: rockchip-dfi: Use free running counter > PM / devfreq: rockchip-dfi: introduce channel mask > PM / devfreq: rk3399_dmc,dfi: generalize DDRTYPE defines > PM / devfreq: rockchip-dfi: Clean up DDR type register defines > PM / devfreq: rockchip-dfi: Add RK3568 support > PM / devfreq: rockchip-dfi: Handle LPDDR2 correctly > PM / devfreq: rockchip-dfi: Handle LPDDR4X > PM / devfreq: rockchip-dfi: Pass private data struct to internal > functions > PM / devfreq: rockchip-dfi: Prepare for multiple users > PM / devfreq: rockchip-dfi: give variable a better name > PM / devfreq: rockchip-dfi: Add perf support > PM / devfreq: rockchip-dfi: make register stride SoC specific > PM / devfreq: rockchip-dfi: account for multiple DDRMON_CTRL registers > PM / devfreq: rockchip-dfi: add support for RK3588 > dt-bindings: devfreq: event: convert Rockchip DFI binding to yaml > dt-bindings: devfreq: event: rockchip,dfi: Add rk3568 support > dt-bindings: devfreq: event: rockchip,dfi: Add rk3588 support > arm64: dts: rockchip: rk3399: Enable DFI > arm64: dts: rockchip: rk356x: Add DFI > arm64: dts: rockchip: rk3588s: Add DFI > > .../bindings/devfreq/event/rockchip,dfi.yaml | 84 ++ > .../bindings/devfreq/event/rockchip-dfi.txt | 18 - > .../rockchip,rk3399-dmc.yaml | 2 +- > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 - > arch/arm64/boot/dts/rockchip/rk356x.dtsi | 7 + > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 16 + > drivers/devfreq/event/rockchip-dfi.c | 796 +++++++++++++++--- > drivers/devfreq/rk3399_dmc.c | 10 +- > include/soc/rockchip/rk3399_grf.h | 9 +- > include/soc/rockchip/rk3568_grf.h | 13 + > include/soc/rockchip/rk3588_grf.h | 18 + > include/soc/rockchip/rockchip_grf.h | 18 + > 12 files changed, 854 insertions(+), 138 deletions(-) > create mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml > delete mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt > create mode 100644 include/soc/rockchip/rk3568_grf.h > create mode 100644 include/soc/rockchip/rk3588_grf.h > create mode 100644 include/soc/rockchip/rockchip_grf.h > > -- > 2.39.2 >
Hello Sascha, Sebastian, On Wed, Jun 14, 2023 at 1:40 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > On Wed, May 24, 2023 at 10:31:28AM +0200, Sascha Hauer wrote: > > This is v5 of the series adding perf support to the rockchip DFI driver. > > [...] > > The RK3588 device tree changes for the DFI were not part of v4. As > > Vincent Legoll showed interest in testing this series the necessary > > device tree changes are now part of this series. > > I tested the series on RK3588 EVB1. The read/write byts looks > sensible. Sometimes cycles reads unrealistic values, though: > [...] > Otherwise the series is > > Tested-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > -- Sebastian I also tested this new version of the series on a Pine64 QuartzPro64 dev board. I applied the series on top of my local branch, which is based on Collabora's rockchip-3588 plus some QP64 DTS patches, and your V5 patch series. Looks like this is still working properly: -bash-5.1# uname -a Linux qp64 6.4.0-rc1-00140-g658dd2200e2a #24 SMP PREEMPT Wed Jun 14 15:50:34 CEST 2023 aarch64 GNU/Linux -bash-5.1# zgrep -i _dfi /proc/config.gz CONFIG_DEVFREQ_EVENT_ROCKCHIP_DFI=y -bash-5.1# perf list | grep rockchip_ddr rockchip_ddr/bytes/ [Kernel PMU event] rockchip_ddr/cycles/ [Kernel PMU event] rockchip_ddr/read-bytes/ [Kernel PMU event] rockchip_ddr/read-bytes0/ [Kernel PMU event] rockchip_ddr/read-bytes1/ [Kernel PMU event] rockchip_ddr/read-bytes2/ [Kernel PMU event] rockchip_ddr/read-bytes3/ [Kernel PMU event] rockchip_ddr/write-bytes/ [Kernel PMU event] rockchip_ddr/write-bytes0/ [Kernel PMU event] rockchip_ddr/write-bytes1/ [Kernel PMU event] rockchip_ddr/write-bytes2/ [Kernel PMU event] rockchip_ddr/write-bytes3/ [Kernel PMU event] # With no memory load -bash-5.1# perf stat -a -e rockchip_ddr/cycles/,rockchip_ddr/read-bytes/,rockchip_ddr/write-bytes/,rockchip_ddr/bytes/ sleep 1 Performance counter stats for 'system wide': 1058691047 rockchip_ddr/cycles/ 9.35 MB rockchip_ddr/read-bytes/ 0.57 MB rockchip_ddr/write-bytes/ 9.90 MB rockchip_ddr/bytes/ 1.002616498 seconds time elapsed # With a hog -bash-5.1# memtester 4G > /dev/null 2>&1 & -bash-5.1# perf stat -a -e rockchip_ddr/cycles/,rockchip_ddr/read-bytes/,rockchip_ddr/write-bytes/,rockchip_ddr/bytes/ sleep 10 Performance counter stats for 'system wide': 10561540038 rockchip_ddr/cycles/ 60212.59 MB rockchip_ddr/read-bytes/ 31313.03 MB rockchip_ddr/write-bytes/ 91525.60 MB rockchip_ddr/bytes/ 10.001651886 seconds time elapsed You can add my T-B, for the whole series: Tested-by: Vincent Legoll <vincent.legoll@gmail.com> Or is there something else you want me to test ? Thanks for your work Regards -- Vincent Legoll
Hi, On Wed, Jun 14, 2023 at 03:40:34PM +0200, Sebastian Reichel wrote: > I tested the series on RK3588 EVB1. The read/write byts looks > sensible. Sometimes cycles reads unrealistic values, though: > > 18446744070475110400 rockchip_ddr/cycles/ I have seen this going off a few times with and without memory pressure. If it's way off, it always seems to follow the same pattern: The upper 32 bits are 0xffffffff instead of 0x00000000 with the lower 32 bits containing sensible data. -- Sebastian
Hi, On Wed, Jun 14, 2023 at 3:27 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > On Wed, Jun 14, 2023 at 03:40:34PM +0200, Sebastian Reichel wrote: > > I tested the series on RK3588 EVB1. The read/write byts looks > > sensible. Sometimes cycles reads unrealistic values, though: > > > > 18446744070475110400 rockchip_ddr/cycles/ > > I have seen this going off a few times with and without memory > pressure. If it's way off, it always seems to follow the same > pattern: The upper 32 bits are 0xffffffff instead of 0x00000000 > with the lower 32 bits containing sensible data. How often is that happening ? I have been running this in a loop with varying sleep duration for the last few hours without hitting it, with and without memtester. REPEATS=1000 MAX_DURATION=100 OUT="/tmp/perf-dfi-rk3588-${REPEATS}x${MAX_DURATION}s.txt" echo -n > $OUT for i in $(seq $REPEATS) ; do DURATION=$(shuf -i "0-${MAX_DURATION}" -n 1) echo -n "${DURATION} : " >> $OUT perf stat -a -e rockchip_ddr/cycles/ sleep $DURATION 2>&1 | awk '/rockchip_ddr/ {printf("%X\n", int($1))}' >> $OUT done BTW, it's on a musl-libc arm64 void linux userspace.
Hi, On Wed, Jun 14, 2023 at 07:51:21PM +0000, Vincent Legoll wrote: > On Wed, Jun 14, 2023 at 3:27 PM Sebastian Reichel > <sebastian.reichel@collabora.com> wrote: > > On Wed, Jun 14, 2023 at 03:40:34PM +0200, Sebastian Reichel wrote: > > > I tested the series on RK3588 EVB1. The read/write byts looks > > > sensible. Sometimes cycles reads unrealistic values, though: > > > > > > 18446744070475110400 rockchip_ddr/cycles/ > > > > I have seen this going off a few times with and without memory > > pressure. If it's way off, it always seems to follow the same > > pattern: The upper 32 bits are 0xffffffff instead of 0x00000000 > > with the lower 32 bits containing sensible data. > > How often is that happening ? I saw it multiple times (4 or 5) within a few tries (I guess around 20). I could see it with and without applying load on the memory. Tests have been running globally for a second using 'sleep 1' (just like the example from Sascha Hauer in the perf patch) > BTW, it's on a musl-libc arm64 void linux userspace. In my case it's Debian unstable. -- Sebastian
Hi Sebastian, On Wed, Jun 14, 2023 at 03:40:34PM +0200, Sebastian Reichel wrote: > Hi, > > On Wed, May 24, 2023 at 10:31:28AM +0200, Sascha Hauer wrote: > > This is v5 of the series adding perf support to the rockchip DFI driver. > > > > A lot has changed in the perf driver since v4. First of all the review > > feedback from Robin and Jonathan has been integrated. The perf driver > > now not only supports monitoring the total DDR utilization, but also the > > individual channels. I also reworked the way the raw 32bit counter > > values are summed up to 64bit perf values, so hopefully the code is > > easier to follow now. > > > > lockdep found out that that locking in the perf driver was broken, so I > > reworked that as well. None of the perf hooks allows locking with > > mutexes or spinlocks, so in perf it's not possible to enable the DFI > > controller when needed. Instead I now unconditionally enable the DFI > > controller during probe when perf is enabled. > > > > Furthermore the hrtimer I use for reading out the hardware counter > > values before they overflow race with perf. Now a seqlock is used to > > prevent that. > > > > The RK3588 device tree changes for the DFI were not part of v4. As > > Vincent Legoll showed interest in testing this series the necessary > > device tree changes are now part of this series. > > I tested the series on RK3588 EVB1. The read/write byts looks > sensible. Sometimes cycles reads unrealistic values, though: > > Performance counter stats for 'system wide': > > 18446744070475110400 rockchip_ddr/cycles/ I'll have a look later this day. I remember seeing this, but I thought this had been resolved already. Thanks for your feedback so far. Sascha
On Wed, Jun 14, 2023 at 03:40:34PM +0200, Sebastian Reichel wrote: > Hi, > > On Wed, May 24, 2023 at 10:31:28AM +0200, Sascha Hauer wrote: > > This is v5 of the series adding perf support to the rockchip DFI driver. > > > > A lot has changed in the perf driver since v4. First of all the review > > feedback from Robin and Jonathan has been integrated. The perf driver > > now not only supports monitoring the total DDR utilization, but also the > > individual channels. I also reworked the way the raw 32bit counter > > values are summed up to 64bit perf values, so hopefully the code is > > easier to follow now. > > > > lockdep found out that that locking in the perf driver was broken, so I > > reworked that as well. None of the perf hooks allows locking with > > mutexes or spinlocks, so in perf it's not possible to enable the DFI > > controller when needed. Instead I now unconditionally enable the DFI > > controller during probe when perf is enabled. > > > > Furthermore the hrtimer I use for reading out the hardware counter > > values before they overflow race with perf. Now a seqlock is used to > > prevent that. > > > > The RK3588 device tree changes for the DFI were not part of v4. As > > Vincent Legoll showed interest in testing this series the necessary > > device tree changes are now part of this series. > > I tested the series on RK3588 EVB1. The read/write byts looks > sensible. Sometimes cycles reads unrealistic values, though: > > Performance counter stats for 'system wide': > > 18446744070475110400 rockchip_ddr/cycles/ This goes down to missing initialization of &dfi->last_perf_count, see my other mail. Will fix this in the next round. Sascha
This is v5 of the series adding perf support to the rockchip DFI driver. A lot has changed in the perf driver since v4. First of all the review feedback from Robin and Jonathan has been integrated. The perf driver now not only supports monitoring the total DDR utilization, but also the individual channels. I also reworked the way the raw 32bit counter values are summed up to 64bit perf values, so hopefully the code is easier to follow now. lockdep found out that that locking in the perf driver was broken, so I reworked that as well. None of the perf hooks allows locking with mutexes or spinlocks, so in perf it's not possible to enable the DFI controller when needed. Instead I now unconditionally enable the DFI controller during probe when perf is enabled. Furthermore the hrtimer I use for reading out the hardware counter values before they overflow race with perf. Now a seqlock is used to prevent that. The RK3588 device tree changes for the DFI were not part of v4. As Vincent Legoll showed interest in testing this series the necessary device tree changes are now part of this series. Changes since v4: - Add device tree changes for RK3588 - Use seqlock to protect perf counter values from hrtimer - Unconditionally enable DFI when perf is enabled - Bring back changes to dts/binding patches that were lost in v4 Changes since v3: - Add RK3588 support Changes since v2: - Fix broken reference to binding - Add Reviewed-by from Rob Changes since v1: - Fix example to actually match the binding and fix the warnings resulted thereof - Make addition of rockchip,rk3568-dfi an extra patch Sascha Hauer (25): PM / devfreq: rockchip-dfi: Make pmu regmap mandatory PM / devfreq: rockchip-dfi: Embed desc into private data struct PM / devfreq: rockchip-dfi: use consistent name for private data struct PM / devfreq: rockchip-dfi: Add SoC specific init function PM / devfreq: rockchip-dfi: dfi store raw values in counter struct PM / devfreq: rockchip-dfi: Use free running counter PM / devfreq: rockchip-dfi: introduce channel mask PM / devfreq: rk3399_dmc,dfi: generalize DDRTYPE defines PM / devfreq: rockchip-dfi: Clean up DDR type register defines PM / devfreq: rockchip-dfi: Add RK3568 support PM / devfreq: rockchip-dfi: Handle LPDDR2 correctly PM / devfreq: rockchip-dfi: Handle LPDDR4X PM / devfreq: rockchip-dfi: Pass private data struct to internal functions PM / devfreq: rockchip-dfi: Prepare for multiple users PM / devfreq: rockchip-dfi: give variable a better name PM / devfreq: rockchip-dfi: Add perf support PM / devfreq: rockchip-dfi: make register stride SoC specific PM / devfreq: rockchip-dfi: account for multiple DDRMON_CTRL registers PM / devfreq: rockchip-dfi: add support for RK3588 dt-bindings: devfreq: event: convert Rockchip DFI binding to yaml dt-bindings: devfreq: event: rockchip,dfi: Add rk3568 support dt-bindings: devfreq: event: rockchip,dfi: Add rk3588 support arm64: dts: rockchip: rk3399: Enable DFI arm64: dts: rockchip: rk356x: Add DFI arm64: dts: rockchip: rk3588s: Add DFI .../bindings/devfreq/event/rockchip,dfi.yaml | 84 ++ .../bindings/devfreq/event/rockchip-dfi.txt | 18 - .../rockchip,rk3399-dmc.yaml | 2 +- arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 - arch/arm64/boot/dts/rockchip/rk356x.dtsi | 7 + arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 16 + drivers/devfreq/event/rockchip-dfi.c | 796 +++++++++++++++--- drivers/devfreq/rk3399_dmc.c | 10 +- include/soc/rockchip/rk3399_grf.h | 9 +- include/soc/rockchip/rk3568_grf.h | 13 + include/soc/rockchip/rk3588_grf.h | 18 + include/soc/rockchip/rockchip_grf.h | 18 + 12 files changed, 854 insertions(+), 138 deletions(-) create mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml delete mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt create mode 100644 include/soc/rockchip/rk3568_grf.h create mode 100644 include/soc/rockchip/rk3588_grf.h create mode 100644 include/soc/rockchip/rockchip_grf.h