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.
Reviewed-by: Sebastian Reichel <[email protected]>
Signed-off-by: Sascha Hauer <[email protected]>
---
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 23. 7. 4. 18:32, 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.
>
> Reviewed-by: Sebastian Reichel <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> 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++) {
Instead of DMC_MAX_CHANNELS defintion,
you can initialize the max channel in each rkXXXX_dfi_init() like 'dfi->channel_count'.
It reduces the unnecessary loop by initializing the proper max channel.
> + 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;
> };
>
--
Best Regards,
Samsung Electronics
Chanwoo Choi
On Sat, Oct 07, 2023 at 02:21:10AM +0900, Chanwoo Choi wrote:
> Hi,
>
> On 23. 7. 4. 18:32, 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.
> >
> > Reviewed-by: Sebastian Reichel <[email protected]>
> > Signed-off-by: Sascha Hauer <[email protected]>
> > ---
> > 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++) {
>
> Instead of DMC_MAX_CHANNELS defintion,
> you can initialize the max channel in each rkXXXX_dfi_init() like 'dfi->channel_count'.
> It reduces the unnecessary loop by initializing the proper max channel.
That is not easily possible. Some SoCs, eg the RK3588 have four
channels, but not all channels are necessarily enabled it also
might not be the first channels that are enabled. On a RK3588
the channel mask might for example be 0b0101.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, Oct 16, 2023 at 01:22:16PM +0200, Sascha Hauer wrote:
> On Sat, Oct 07, 2023 at 02:21:10AM +0900, Chanwoo Choi wrote:
> > Hi,
> >
> > On 23. 7. 4. 18:32, 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.
> > >
> > > Reviewed-by: Sebastian Reichel <[email protected]>
> > > Signed-off-by: Sascha Hauer <[email protected]>
> > > ---
> > > 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++) {
> >
> > Instead of DMC_MAX_CHANNELS defintion,
> > you can initialize the max channel in each rkXXXX_dfi_init() like 'dfi->channel_count'.
> > It reduces the unnecessary loop by initializing the proper max channel.
>
> That is not easily possible. Some SoCs, eg the RK3588 have four
> channels, but not all channels are necessarily enabled it also
> might not be the first channels that are enabled. On a RK3588
> the channel mask might for example be 0b0101.
Nah, forget this comment. Of course I can initialize a variable with a
maximum value of channels that could be available on this SoC and only
iterate over these. Will do.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 23. 10. 16. 21:45, Sascha Hauer wrote:
> On Mon, Oct 16, 2023 at 01:22:16PM +0200, Sascha Hauer wrote:
>> On Sat, Oct 07, 2023 at 02:21:10AM +0900, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 23. 7. 4. 18:32, 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.
>>>>
>>>> Reviewed-by: Sebastian Reichel <[email protected]>
>>>> Signed-off-by: Sascha Hauer <[email protected]>
>>>> ---
>>>> 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++) {
>>>
>>> Instead of DMC_MAX_CHANNELS defintion,
>>> you can initialize the max channel in each rkXXXX_dfi_init() like 'dfi->channel_count'.
>>> It reduces the unnecessary loop by initializing the proper max channel.
>>
>> That is not easily possible. Some SoCs, eg the RK3588 have four
>> channels, but not all channels are necessarily enabled it also
>> might not be the first channels that are enabled. On a RK3588
>> the channel mask might for example be 0b0101.
>
> Nah, forget this comment. Of course I can initialize a variable with a
> maximum value of channels that could be available on this SoC and only
> iterate over these. Will do.
>
Thanks.
--
Best Regards,
Samsung Electronics
Chanwoo Choi