lkml.org 
[lkml]   [2014]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V5 13/23] MIPS: BMIPS: Flush the readahead cache after DMA
On Sat, Dec 20, 2014 at 2:39 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> On Mon, Dec 15, 2014 at 1:43 AM, Jonas Gorski <jogo@openwrt.org> wrote:
>> On Fri, Dec 12, 2014 at 11:07 PM, Kevin Cernekee <cernekee@gmail.com> wrote:
>>> BMIPS 3300/435x/438x CPUs have a readahead cache that is separate from
>>> the L1/L2. During a DMA operation, accesses adjacent to a DMA buffer
>>> may cause parts of the DMA buffer to be prefetched into the RAC. To
>>> avoid possible coherency problems, flush the RAC upon DMA completion.
>>
>> According to what I have, any cpu [d-]cache invalidate operation
>> should already flush the full RAC unless explicitly disabled in the
>> RAC configuration - is this intended as an optimization/shortcut?
>
> Correct - performing a RAC flush instead of blasting the entire range
> again via CACHE instructions should be considerably faster in most
> cases. CACHE instructions are not pipelined on BMIPS3300/43xx.
>
> There may be a couple of old CPU versions (possibly 130nm) that don't
> automatically perform the RAC flush on each CACHE instruction. Also,
> a fun bit of trivia: MVA based cache flushes on B15 do flush the RAC,
> but index based instructions do not.

Because I'm laz^W^Wstill need to do some christmas shopping, I'll ask
a few dumb questions:

Since a RAC flush won't flush the I/D-caches themselves, I assume
there is no cache invalidate needed for BMIPS? Also is it still needed
if the RAC is setup to only prefetch instructions (which it seems to
be on bcm963xx)?

I also fail to find any RAC flushing on either bcm963xx or bcm947xx
SDK kernels, that's why I'm a bit wondering whether they really need
it. But maybe they always do explicit syncs, haven't checked that.

Furthermore, I see code to enable data prefetching in setup on
bcm963xx, so I wonder if it wouldn't make sense to add the RAC as an
extra node in DT / register/enable/configure it from bmips setup code
(because then we can also properly setup the address range in case the
bootloader didn't).

>>> static inline int cpu_needs_post_dma_flush(struct device *dev)
>>> {
>>
>> The place for it seems a bit misplaced; I would not expect
>> cpu_needs_post_dma_flush() to have any side effects.
>
> Maybe we should rename the function? To just cpu_post_dma_flush()?

Hm, not sure. Add a feature flag for that, or a callback. It is
essentially a second level cache I guess.

Also while reading dma-default.c, I wonder why dma_unmap_page checks
if cpu needs to flush, but dma_unmap_sg doesn't (disclamer: I don't
know anything about sg).

>
> (Or call a separate function from each site - but that seems unnecessary.)
>
>>> + if (boot_cpu_type() == CPU_BMIPS3300 ||
>>> + boot_cpu_type() == CPU_BMIPS4350 ||
>>> + boot_cpu_type() == CPU_BMIPS4380) {
>>> + void __iomem *cbr = BMIPS_GET_CBR();
>>> +
>>> + /* Flush stale data out of the readahead cache */
>>> + __raw_writel(0x100, cbr + BMIPS_RAC_CONFIG);
>>
>> Hm, according to what I have, bits [6:0] of RAC_CONFIG are R/W
>> configuration bits, and this will overwrite them:
>>
>> CFE> dm 0xff400000 4
>> ff400000: 02a07015 ..p.
>> CFE> sm 0xff400000 0x100 4
>> ff400000: 02a00000 ....
>>
>> (As far as I can tell, RAC was previously enabled for instruction
>> cache misses , and now isn't any more for anything, so effectively
>> disabled?)
>>
>> Also for BMIPS4350 (and I guess 4380) there seems to be a second
>> RAC_CONFIG register at 0x8, I guess for the second thread? Does it
>> need flushing, too?
>
> I'll defer to Florian for the final word since he has access to the
> documentation, but going from memory:
>
> RAC_CONFIG should probably be a read/modify/write. I'm pretty sure
> there are important RW configuration bits in there. I may have
> incorrectly translated the "set bit 8" code from here:
>
> https://github.com/Broadcom/stblinux-3.3/blob/master/linux/arch/mips/mm/c-brcmstb.c#L374
>
> There is only one RAC for all CPUs, and we've never had to flush
> anything via CBR+0x08.


What I see in recent bcm963xx SDKs is this:

void __init plat_mem_setup(void)
{
...
volatile unsigned long *cr;
uint32 mipsBaseAddr = MIPS_BASE;

cr = (void *)(mipsBaseAddr + MIPS_RAC_CR0);
*cr = *cr | RAC_D | RAC_PF_D;

#if defined(MIPS_RAC_CR1)
cr = (void *)(mipsBaseAddr + MIPS_RAC_CR1);
*cr = *cr | RAC_D | RAC_PF_D;
#endif
}

RAC_CR1 seems to be defined for BMIPS4350 based SoCs, while BMIPS3300
ones don't (well, 6318 doesn't. The older SDKs named the address range
register RAC_CR1, so for them this check is quite wrong :P)

But no references to RAC_FLH anywhere.


Jonas


\
 
 \ /
  Last update: 2014-12-20 14:21    [W:1.445 / U:0.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site