| --- Log opened Wed Sep 04 00:00:41 2013 | ||
| stekern | _franck_: you now have your name in wb_sdram_ctrl too ;) | 05:25 |
|---|---|---|
| olofk | blueCmd_: Congratulations | 05:46 |
| olofk | Hmm.. looks like my test bench stops because I run out of random :( | 06:00 |
| olofk | Where can I buy more random? | 06:00 |
| stekern | at any random store | 06:01 |
| olofk | It's ok now. I found some more | 06:10 |
| olofk | But now I got a read mismatch after 16931 transactions | 06:12 |
| blueCmd_ | _franck_: moving to Ireland to start work next thursday so not just yet ;) | 06:19 |
| olofk | blueCmd_: Are you hired to spread the gospel of OpenRISC to Ireland? :) | 06:38 |
| olofk | stekern: Is it expected to always have to do a refill when I write data and read it back? | 06:40 |
| olofk | Because my failing transaction never enters refill | 06:40 |
| stekern | olofk: no, if the data to be written is in the cache buffer, it should just write it there and not refill on read | 06:42 |
| stekern | ...but if the data is *not* in the cache buffer, then you of course should expect a refill | 06:48 |
| stekern | and to elaborate a bit on how the cache works (since you asked about it earlier, but maybe you've figured it out already). It's actually a pretty ordinary write-through cache, but with only *one* cache line | 06:50 |
| stekern | each port has it's own "cache" and they are coherent (i.e. writes (should) go to all ports caches) | 06:51 |
| stekern | and writes (as you noticed) go through a fifo | 06:52 |
| blueCmd_ | olofk: hah, sadly no | 06:52 |
| olofk | stekern: And bufhit is asserted when the requested address is cached? | 07:12 |
| stekern | yes | 07:14 |
| olofk | Looks like the cache isn't updated. I found that the value I'm getting back has been written in an earlier transaction | 07:18 |
| stekern | ok, is this within a single port? | 07:20 |
| stekern | or is it the coherency logic that is failing? | 07:20 |
| olofk | Yes. Each master writes to a different memory area | 07:21 |
| olofk | So single port | 07:21 |
| olofk | It works if I separate the requests by a few cycles | 07:21 |
| stekern | mmkay... | 07:21 |
| stekern | so, when the write happens, is bufhit asserted then? | 07:23 |
| olofk | w8. Need to rerun with less wait time between transactions | 07:25 |
| olofk | The transaction is two data long, and the second data fails. On the first, bufhit is asserted, but not on the second | 07:27 |
| olofk | It's a wrap burst, with addresses 0x31C, 300 | 07:27 |
| olofk | bte = 10 (Wrap 8?) | 07:28 |
| olofk | yep. wrap 8 | 07:28 |
| olofk | ah.. yes.. and the previous transaction did a 1 cycle access to address 0x314. That might be what makes it confused | 07:31 |
| stekern | hmm, with the latest bugfixes I've done, I see that the writes are a bit weird... | 07:31 |
| stekern | but that's probably unrelated to this | 07:31 |
| stekern | non-burst writes are now written twice to the fifo | 07:33 |
| olofk | Three cycles between accesses seems to do the trick | 07:36 |
| stekern | maybe it's the handshaking latency between sdram and wb that is causing that | 07:38 |
| olofk | hmmm... it's not the RAW timing for that cycle that seems to be the problem. It's the delay from the previous read cycle that needs to be increased | 07:41 |
| stekern | hmm, I'm starting to doubt that this was entirely correct: https://github.com/skristiansson/wb_sdram_ctrl/commit/37b0b5c74da35a89feb627c159c0798c725a5d09 | 07:43 |
| olofk | I haven't really figured out enough of the code to analyze that patch | 07:45 |
| LoneTech | without understanding the rest of it I can't tell if either is right, but it does look like it's conflating the issues of burst and acknowledge behaviour | 07:46 |
| olofk | But I see now that when it fails, the refill block that is read from the previous transaction hasn't finished when the failing transaction starts | 07:46 |
| LoneTech | IIRC the WB spec also had an odd and possibly unintended requirement that the byte selects not vary | 07:47 |
| olofk | I suspect that the data written in the failing transaction is overwritten by the data that was read back from the ram | 07:48 |
| olofk | LoneTech: wb_sel is static here, but I noticed that too. And I agree that it feels like unintended | 07:48 |
| LoneTech | this change seems to remove the double write prevention for non-bursts | 07:52 |
| olofk | stekern: https://www.dropbox.com/s/4z0hhbtxkrsoj6n/wave.bmp | 07:52 |
| stekern | olofk: I think that this is needed for the writes to be correct: http://pastie.org/8296745 | 07:52 |
| stekern | (it will still dual write into the cache, but that's not so important) | 07:53 |
| LoneTech | (non-bursts including end of bursts) | 07:54 |
| olofk | stekern: Apply against master? | 07:55 |
| stekern | yes, but there might be more dragons now when I think about it... | 07:56 |
| olofk | Should I try it anyway? | 07:57 |
| stekern | ah, now I was just imagining the dragons, I think that should be ok | 07:58 |
| stekern | s/now/no | 07:58 |
| stekern | the dragons are only related to the buf write, since the addres and data that go there is delayed by one clock cycle | 07:59 |
| stekern | ... I should probably change that | 07:59 |
| stekern | so, yes, try that | 07:59 |
| olofk | Nope. Sorry | 08:00 |
| olofk | But I'm not completely sure about the state of my wb_port.v now. Haven't got git on the machine I run it on, so it's getting a bit patchy now :) | 08:01 |
| olofk | Nah. it looks good. Only difference was the patch from _franck_ | 08:04 |
| olofk | ok. Second data in the last transaction is never written to the cache | 08:15 |
| stekern | because bufhit isn't asserted? | 08:21 |
| olofk | Yes, bufhit is only asserted for the first data (of two) in the write | 08:26 |
| olofk | I can see that the first is written correctly to the bufram | 08:26 |
| olofk | You want another bmp? :) | 08:28 |
| stekern | i'm on the phone, so can't look at the bmp, but isn't the second write within the cacheline? | 08:28 |
| olofk | Nope | 08:29 |
| stekern | so then the write is correct | 08:30 |
| olofk | Is it? | 08:30 |
| stekern | but how can it not be within the cacheline? | 08:30 |
| olofk | Hmm.. did I misunderstand "within the cacheline"? | 08:31 |
| olofk | Only one of two is written to the bufram | 08:31 |
| stekern | ok, i got that. the question is, is that correct? | 08:32 |
| olofk | I suppose not, because when I read it back, I'm getting an old value | 08:32 |
| stekern | so, then it is within the cacheline (i.e. the address you sre writing to in the second write is in cache) | 08:34 |
| olofk | ah... ok. Yes. I think that's the problem, because the previous transaction refills that position | 08:35 |
| stekern | so it's the bufhit logic that bugs out | 08:36 |
| stekern | i've got a hunch, could it be that the sdram is still refilling when the write happens in wb domain | 08:39 |
| stekern | sdram domain | 08:39 |
| stekern | sdram domain is refilling, wb domain writes is what I'm trying to say... | 08:41 |
| olofk | Yes, that's what I'm thinking too | 08:42 |
| olofk | Especially since it works if I delay the write for one cycle, so that the previous refill from SDRAM finishes first | 08:43 |
| stekern | I have guard against that in IDLE stat before going to write, can that condition change during the burst? | 08:43 |
| stekern | if so, the guard have to be added to WRITE state as well | 08:44 |
| stekern | or can something slip through that guard | 08:45 |
| stekern | yes, it can... | 08:46 |
| stekern | (i'm just reading the code from memory though) | 08:46 |
| olofk | Now that's seriously spooky | 08:47 |
| stekern | so, in IDLE, we have to check that all bits in buf_clean is set before going to write | 08:48 |
| stekern | test that | 08:48 |
| stekern | use the _wb signal of buf_clean | 08:48 |
| olofk | Where do you check that all data from SDRAM has been returned during a refill? | 08:49 |
| stekern | what's spooky? | 08:50 |
| olofk | 08:46 < stekern> (i'm just reading the code from memory though) | 08:50 |
| olofk | Hmm.. should I just prevent the transition to WRITE, or should I disable wb_write_ack and wb_write_bufram too? | 08:52 |
| stekern | If I understand your question right, when all bits in buf_clean is set, the refiöl is done | 08:52 |
| olofk | ah. ok | 08:53 |
| stekern | (spooky) but I physically read the code just an hour ago | 08:53 |
| stekern | put it in the guard clause for the whoöe write thing | 08:54 |
| olofk | Hmmm... adding & &buf_clean_wb to the if statement was not a good idea | 08:58 |
| olofk | No failing transaction, mainly because no transactions at all are being processed | 08:59 |
| stekern | do bufhit & bufclean... | 09:02 |
| olofk | Lunch now | 09:05 |
| ams | overrated. | 09:12 |
| olofk | ams: That's what she said! | 09:42 |
| olofk | stekern: Still not looking any good. Trying to figure out something | 10:00 |
| olofk | stekern: I got something working now | 10:06 |
| olofk | Something like this: http://pastie.org/8296987 | 10:11 |
| olofk | Intendation is fucked up. Haven't got verilog-mode on this machine | 10:11 |
| olofk | Rerunning with 3*200000 transactions to see what new bugs I can uncover :) | 10:13 |
| olofk | Yep. That works too | 10:17 |
| olofk | Now I just need to rerun it with different clock combinations, and allow masters to access the same memory to check for cache coherence bugs | 10:17 |
| olofk | Oh.. and increase the maximum burst length | 10:18 |
| olofk | Larger bursts works fine | 10:21 |
| olofk | But faster wb_clk than sdram_clk fails :( | 10:27 |
| stekern | yes, I can imagine that will bring out corner cases | 11:34 |
| stekern | doesn't sound like a very likely scenario though | 11:35 |
| stekern | to have bus freq > memory freq | 11:35 |
| stekern | but if there's some obvious bugs that it brings out, it's of course good | 11:36 |
| stekern | hmm, looking at your patch, it doesn't actually exclude the corner case I thought were causing the bug | 11:39 |
| stekern | how can your condition ever be true? | 11:40 |
| stekern | obviously it can since it fixes the bug you are seeing, but I don't understand it | 11:41 |
| stekern | ah, I missed a ')' in the patch | 11:41 |
| stekern | so it does exclude the corner case I was thinking about | 11:42 |
| stekern | but it excludes too much ;) | 11:43 |
| stekern | it should only be: bufhit & !refill_busy | 11:44 |
| stekern | I don't understand why 'bufhit & (&bufclean_wb)' didn't work though | 11:48 |
| stekern | olofk: so the question is, doesn't 'bufhit & !refill_busy' work? | 11:52 |
| _franck_ | I've never tried my jtag_vpi thing on complete soc until today. That's not very usable... | 13:58 |
| _franck_ | That's so slow | 13:58 |
| _franck_ | olofk: stekern do you making a testbench for every system in orpsocv3 worth it ? We could have only the generic system for debugging purpose. | 17:53 |
| _franck_ | *think | 17:54 |
| stekern | _franck_: I think we *should* | 18:14 |
| stekern | that said, I know it's a problem with altera and xilinx stuff not playing with iverilog | 18:15 |
| stekern | i.e. you need modelsim for it | 18:15 |
| _franck_ | I did a pull request for openrisc/orpsocv3 but I think I can pull those changes myself there... | 19:17 |
| _franck_ | stekern: I sent you a patch (github pull request) to change arbiter.v -> wb_port_arbiter.v in your wb_sdram_ctrl | 19:41 |
| stekern | _franck_: pulled | 19:56 |
| stekern | nice, github makes a link to the right commit if you reference it by its hash in another commit | 20:03 |
| _franck_ | my Cyclone II does not support NEW_DATA_WITH_NBE_READ for its dpram... | 20:42 |
| _franck_ | stekern: do you think we can avoid this situation in your wb_sdram_ctrl and remove this option in dpram_altera ? | 20:43 |
| --- Log closed Thu Sep 05 00:00:42 2013 | ||
Generated by irclog2html.py 2.15.2 by Marius Gedminas - find it at mg.pov.lt!
