[PATCH] sh: sh2a: Optimise cache flush for writethrough mode.

[PATCH] sh: sh2a: Optimise cache flush for writethrough mode.

[Phil Edworthy]

When the operand cache is set to writethough mode, there is no
need to flush data.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 arch/sh/mm/cache-sh2a.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/sh/mm/cache-sh2a.c b/arch/sh/mm/cache-sh2a.c
index 1f51225..56bb5f2 100644
--- a/arch/sh/mm/cache-sh2a.c
+++ b/arch/sh/mm/cache-sh2a.c
@@ -112,6 +112,7 @@ static void sh2a_flush_icache_range(void *args)
 
 	for (v = start; v < end; v+=L1_CACHE_BYTES) {
 		unsigned long addr = (v & 0x000007f0);
+#ifdef CONFIG_CACHE_WRITEBACK
 		int way;
 		/* O-Cache writeback */
 		for (way = 0; way < 4; way++) {
@@ -121,6 +122,7 @@ static void sh2a_flush_icache_range(void *args)
 				__raw_writel(data, CACHE_OC_ADDRESS_ARRAY | addr | (way << 11));
 			}
 		}
+#endif
 		/* I-Cache invalidate */
 		__raw_writel(addr,
 			  CACHE_IC_ADDRESS_ARRAY | addr | 0x00000008);
-- 
1.7.0.4

Re: [PATCH] sh: sh2a: Optimise cache flush for writethrough mode.

[Paul Mundt]

On Wed, Nov 30, 2011 at 07:21:53AM +0000, Phil Edworthy wrote:
> When the operand cache is set to writethough mode, there is no
> need to flush data.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

What about the other routines?

You are presumably stipulating that any access to the OC address array
outside of cacheline invalidation should be omitted for write-through
cache mode, which suggests that quite a lot of the file in question can
simply be nopped out.

At the very least sh2a__flush_wback_region() could deal with the same
treatment. For the invalidation routines you presumably still need the
V-bit clearing, though.

sh2a__flush_invalidate_region() also bears closer inspection.

Re: [PATCH] sh: sh2a: Optimise cache flush for writethrough mode.

[phil.edworthy]

Hi Paul,

> Subject: Re: [PATCH] sh: sh2a: Optimise cache flush for writethrough 
mode.
> 
> On Wed, Nov 30, 2011 at 07:21:53AM +0000, Phil Edworthy wrote:
> > When the operand cache is set to writethough mode, there is no
> > need to flush data.
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> What about the other routines?
> 
> You are presumably stipulating that any access to the OC address array
> outside of cacheline invalidation should be omitted for write-through
> cache mode, which suggests that quite a lot of the file in question can
> simply be nopped out.
> 
> At the very least sh2a__flush_wback_region() could deal with the same
> treatment. For the invalidation routines you presumably still need the
> V-bit clearing, though.
> 
> sh2a__flush_invalidate_region() also bears closer inspection.

It's worth mentioning that this patch is essentially a work around for an 
issue with write-back mode. sh2a_flush_icache_range is passed a large 
address range which takes ages to flush/invalidate. For write-back mode, 
instead of indexing on the address, perhaps we can walk through all the 
cache entries and check for an address match. That's for another day...

I agree that the other functions can be improved. I ignored them as the 
main performance issue is with sh2a_flush_icache_range. I wasn't entirely 
sure when some of the other functions are used. Are 
sh2a__flush_wback_region and sh2a__flush_purge_region purely OC related? 
And sh2a__flush_invalidate_region and sh2a_flush_icache_range work on both 
the IC and OC?

Phil

Re: [PATCH] sh: sh2a: Optimise cache flush for writethrough mode.

[Paul Mundt]

On Wed, Nov 30, 2011 at 09:56:35AM +0000, phil.edworthy@renesas.com wrote:
> It's worth mentioning that this patch is essentially a work around for an 
> issue with write-back mode. sh2a_flush_icache_range is passed a large 
> address range which takes ages to flush/invalidate. For write-back mode, 
> instead of indexing on the address, perhaps we can walk through all the 
> cache entries and check for an address match. That's for another day...
> 
That's a common pitfall for anything using the address array for clearing
the entries. We have some simple heuristics in the cache-sh4.c code:

..
        /* If there are too many pages then just blow away the caches */
        if (((end - start) >> PAGE_SHIFT) >= MAX_ICACHE_PAGES) {
                local_flush_cache_all(NULL);
                return;
        }
..

A similar thing should probably be done for the SH-2/2A cases simply utilizing
CCR-based global invalidation.

> I agree that the other functions can be improved. I ignored them as the 
> main performance issue is with sh2a_flush_icache_range. I wasn't entirely 
> sure when some of the other functions are used. Are 
> sh2a__flush_wback_region and sh2a__flush_purge_region purely OC related? 
> And sh2a__flush_invalidate_region and sh2a_flush_icache_range work on both 
> the IC and OC?
> 
All of __flush_wback/purge/invalidate_region() are region based routines for OC
management. You get an idea of the abstraction from arch/sh/mm/flush-sh4.c. In general:

	__flush_wback_region - OC writeback
	__flush_invalidate_region - OC invalidate
	__flush_purge_region - OC writeback & invalidate

You can see a trivial usage example through the consistent DMA's
dma_cache_sync() and so on, relative to DMA direction.

flush_icache_range() handles I-cache invalidation for non-snoopable stores, and also
contends with things like I-cache aliases and the like. While semantically it's
not terribly complex, it's a bit of a sledgehammer given the different cases in
which it gets used. Whether the D-cache handling needs to be implemented or not
is likewise CPU dependent. In cases where the I-cache is not able to snoop
D-cache accesses we also need to deal with the D-cache explicitly. On the SMP
parts it's also further complicated by the fact that the bulk of the parts do
not broadcast I-cache block invalidates, and the snoop controller only handles
the D-cache (though the same I/D race exists given that the I-cache doesn't
have any connection to the snoop controller).

You can get a bit of an overview in Documentation/cachetlb.txt if you're
interested in some of the requirements.