Re: [PATCH v7 07/26] PM / devfreq: rockchip-dfi: introduce channel mask

[Sascha Hauer <s.hauer@pengutronix.de>]

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 <sebastian.reichel@collabora.com>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  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 |