linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] regmap: flat: introduce register striding to save some memories
@ 2015-12-14  7:14 Xiubo Li
  2015-12-14 17:57 ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Xiubo Li @ 2015-12-14  7:14 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, Xiubo Li

Throughout the regcache-flat code, for the flat cache array, the
actual register address offsets are used to index the values, and
this will waste some memory spaces.

For example, on 64-BIT platform, device A has three registers:
    register offsets: {0x00, 0x08, 0x10}
    max_register: 0x10
    regsiter striding: 8

And the size of flat cache memory space will be:
    (max_register + 1 ) * sizeof(unsigned int) =
    (0x10 + 1) * sizeof(unsigned int) = 17 * 8 = 136 Bytes

Since map->reg_stride has been introduced and all extant register
addresses are a multiple of this value, it could use the address
offsets divide by the stride to determine the index.

Then the size of flat cache memory space will be:
    (max_register / reg_stride + 1 ) * sizeof(unsigned int) =
    (0x10 / 8 + 1) * sizeof(unsigned int) = 3 * 8 = 24 Bytes

And the bigger of the striding value, there will be more memory
space wasted. After introducing the register striding here can
save some memeories for the system.

Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 drivers/base/regmap/regcache-flat.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/base/regmap/regcache-flat.c b/drivers/base/regmap/regcache-flat.c
index 686c9e0..160d636 100644
--- a/drivers/base/regmap/regcache-flat.c
+++ b/drivers/base/regmap/regcache-flat.c
@@ -19,17 +19,19 @@
 static int regcache_flat_init(struct regmap *map)
 {
 	int i;
-	unsigned int *cache;
+	unsigned int index, *cache;
 
-	map->cache = kcalloc(map->max_register + 1, sizeof(unsigned int),
-			     GFP_KERNEL);
+	map->cache = kcalloc(map->max_register / map->reg_stride + 1,
+			     sizeof(unsigned int), GFP_KERNEL);
 	if (!map->cache)
 		return -ENOMEM;
 
 	cache = map->cache;
 
-	for (i = 0; i < map->num_reg_defaults; i++)
-		cache[map->reg_defaults[i].reg] = map->reg_defaults[i].def;
+	for (i = 0; i < map->num_reg_defaults; i++) {
+		index =  map->reg_defaults[i].reg / map->reg_stride;
+		cache[index] = map->reg_defaults[i].def;
+	}
 
 	return 0;
 }
@@ -45,9 +47,10 @@ static int regcache_flat_exit(struct regmap *map)
 static int regcache_flat_read(struct regmap *map,
 			      unsigned int reg, unsigned int *value)
 {
+	unsigned int index = reg / map->reg_stride;
 	unsigned int *cache = map->cache;
 
-	*value = cache[reg];
+	*value = cache[index];
 
 	return 0;
 }
@@ -55,9 +58,10 @@ static int regcache_flat_read(struct regmap *map,
 static int regcache_flat_write(struct regmap *map, unsigned int reg,
 			       unsigned int value)
 {
+	unsigned int index = reg / map->reg_stride;
 	unsigned int *cache = map->cache;
 
-	cache[reg] = value;
+	cache[index] = value;
 
 	return 0;
 }
-- 
1.8.3.1




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] regmap: flat: introduce register striding to save some memories
  2015-12-14  7:14 [PATCH] regmap: flat: introduce register striding to save some memories Xiubo Li
@ 2015-12-14 17:57 ` Mark Brown
       [not found]   ` <566FD138.4010601@cmss.chinamobile.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-12-14 17:57 UTC (permalink / raw)
  To: Xiubo Li; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

On Mon, Dec 14, 2015 at 03:14:34PM +0800, Xiubo Li wrote:

>  static int regcache_flat_write(struct regmap *map, unsigned int reg,
>  			       unsigned int value)
>  {
> +	unsigned int index = reg / map->reg_stride;

So, this does save some memory.  On the other hand one of the big goals
for the flat cache is to be as fast as possible to match the performance
of MMIO and divisions aren't the cheapest operations.  If this was the
rbtree cache then it'd not be an issue but in the flat cache speed is
definitely the top priority, at least by default.

I'm guessing for your particular register maps the performance isn't too
big an issue...  do you have any numbers for how much space you're
saving overall?  Is it worth finding a way to make this possible to
enable on maps that benefit?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] regmap: flat: introduce register striding to save somememories
       [not found]   ` <566FD138.4010601@cmss.chinamobile.com>
@ 2015-12-18  8:23     ` Mark Brown
       [not found]       ` <2015121816593812599420@cmss.chinamobile.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-12-18  8:23 UTC (permalink / raw)
  To: Xiubo Li; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On Tue, Dec 15, 2015 at 04:37:12PM +0800, Xiubo Li wrote:
> On 15/12/2015 01:57, Mark Brown wrote:
> >On Mon, Dec 14, 2015 at 03:14:34PM +0800, Xiubo Li wrote:

> >I'm guessing for your particular register maps the performance isn't too
> >big an issue...  do you have any numbers for how much space you're
> >saving overall?

> Not yet, but maybe in the future we could see a larger register
> striding, like 32 for MMIO using flat cache.

OK, well I'd like to se 

> >Is it worth finding a way to make this possible to
> >enable on maps that benefit?

> I'm thinking to use the bit rotation to improve the performance of
> the whole regmap.

> like:
> if(reg % reg_stride) --> if(!IS_ALIGNED(reg, reg_stride))
> ...
> index = reg / reg_stride; --> index = reg >> reg_stride_order;
> 
> And do you have any comment and suggestion for the above?

I think we'll need to continue supporting non power of two strides so
an unconditional conversion to shifts might be an issue - some weird DSP
probably does that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Re: [PATCH] regmap: flat: introduce register striding to save somememories
       [not found]       ` <2015121816593812599420@cmss.chinamobile.com>
@ 2015-12-30 17:58         ` Mark Brown
  2015-12-31  1:36           ` [PATCH] regmap: flat: introduce register striding to savesomememories Xiubo Li
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-12-30 17:58 UTC (permalink / raw)
  To: lixiubo; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Fri, Dec 18, 2015 at 04:59:38PM +0800, lixiubo@cmss.chinamobile.com wrote:

> > I think we'll need to continue supporting non power of two strides so
> > an unconditional conversion to shifts might be an issue - some weird DSP
> > probably does that.

> Yes, agreed. 

> IMO this won't happen to MMIO, and for the device using MMIO the
> register strides should equal to power of two. 

> Are there some cases I have met? 

DSPs exposed via I2C and SPI are the main things I'm worried about.
It's fairly common for DSPs to have unusual word sizes including things
like three bytes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] regmap: flat: introduce register striding to savesomememories
  2015-12-30 17:58         ` Mark Brown
@ 2015-12-31  1:36           ` Xiubo Li
  0 siblings, 0 replies; 5+ messages in thread
From: Xiubo Li @ 2015-12-31  1:36 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel



On 31/12/2015 01:58, Mark Brown wrote:
> On Fri, Dec 18, 2015 at 04:59:38PM +0800, lixiubo@cmss.chinamobile.com wrote:
>
>>> I think we'll need to continue supporting non power of two strides so
>>> an unconditional conversion to shifts might be an issue - some weird DSP
>>> probably does that.
>
>> Yes, agreed.
>
>> IMO this won't happen to MMIO, and for the device using MMIO the
>> register strides should equal to power of two.
>
>> Are there some cases I have met?
>
> DSPs exposed via I2C and SPI are the main things I'm worried about.
> It's fairly common for DSPs to have unusual word sizes including things
> like three bytes.
>

Yes, if so, for this case the non power of two strides should be still 
supported.

Thanks for your promotion, and I will think over of this carefully.

BRs

Xiubo Li





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-12-31  1:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-14  7:14 [PATCH] regmap: flat: introduce register striding to save some memories Xiubo Li
2015-12-14 17:57 ` Mark Brown
     [not found]   ` <566FD138.4010601@cmss.chinamobile.com>
2015-12-18  8:23     ` [PATCH] regmap: flat: introduce register striding to save somememories Mark Brown
     [not found]       ` <2015121816593812599420@cmss.chinamobile.com>
2015-12-30 17:58         ` Mark Brown
2015-12-31  1:36           ` [PATCH] regmap: flat: introduce register striding to savesomememories Xiubo Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).