olofk / serv

SERV - The SErial RISC-V CPU
ISC License
1.43k stars 188 forks source link

servant: add iCESugar board support #37

Closed nalzok closed 4 years ago

olofk commented 4 years ago

Thanks. This looks good, but did you solve the strange baud rate issue?

olofk commented 4 years ago

Hm... I remember now that @imuguruza had a similar issue and got it fixed with this commit https://github.com/olofk/serv/commit/dd65d6097a31f14283ddbeb813f8d154b829ce05 Looking at the fix now, I really can't understand how this works. It looks like the baud rate is increased from 43200 to 115200 by doubling the PLL output frequency from 16 to 32 MHz. It makes no sense to me.

Perhaps you could just try to do the same thing and set out_freq to 32 and see if it's suddenly runs with baud rate 115200. Maybe it's some PLL issue I have missed

nalzok commented 4 years ago

It looks like the baud rate is increased from 43200 to 115200 by doubling the PLL output frequency from 16 to 32 MHz. It makes no sense to me.

This is exactly what I have observed! When the CPU runs at 16MHz, the baud rate is ~43200. As soon as I set the CPU clock to 32MHz, the baud rate magically becomes 115200. Doesn't make sense to me too...

I'm going to use the logic analyzerr to find out the "real" CPU clock later!

olofk commented 4 years ago

Please do. It would be very interesting to see the real clock frequency. I will search also to see if this is a known issue.

In the meantime, I'm happy to accept the PR if you change frequency to 32

olofk commented 4 years ago

I think I found the problem now. Running icepll -o 16 gives me

F_PLLIN:    12.000 MHz (given)
F_PLLOUT:   16.000 MHz (requested)
F_PLLOUT:   15.938 MHz (achieved)

FEEDBACK: SIMPLE
F_PFD:   12.000 MHz
F_VCO: 1020.000 MHz

DIVR:  0 (4'b0000)
DIVF: 84 (7'b1010100)
DIVQ:  6 (3'b110)

FILTER_RANGE: 1 (3'b001)

So, this means that it couldn't find parameters to create a 16MHz and had to choose a 15.938 MHz clock instead. Looking at the data sheets, the lowest allowed frequency is 16MHz so this is outside of spec and probably not supported. icepll and/or the FuseSoC icepll generator should fail here instead of producing non-working PLL settings

olofk commented 4 years ago

You could also try to set freq_out to 17, which will produce a 16.875MHz clock which is hopefully close enough

nalzok commented 4 years ago

It turns out that the maximum sampling frequency of my logic analyzer is 24MHz, so I divided the clock by 16 with

reg[3:0] counter = 0;
always @(posedge wb_clk)
begin
    counter <= counter + 1;
end

assign debug = &counter;

With freq_out : 32, the logic analyzer registers

libsigrok 0.6.0-git-400bc4f
Acquisition with 1/8 channels at 24 MHz
D6:00000001 00000000 00010000 00000001 00000000 00010000 00000001 00000000
D6:00010000 00000001 00000000 00010000 00000000 00000000 00000000 00000000
D6:00000000 00000000 00000000 00000000 00001000 00000000 10000000 00001000
D6:00000000 10000000 00001000 00000000 10000000 00001000 00000000 10000000
D6:00001000 00000000 10000000 00001000 00000000 10000000 00001000 00000000
D6:10000000 00001000 00000000 10000000 00000000 00000000 00000000 00000000
D6:00000000 00000000 00000000 00000000 01000000 00000100 00000000 01000000
D6:00000100 00000000 01000000 00000100 00000000 01000000 00000100 00000000

Since there is a 1 for every eleven 0, the frequency of debug is 24HMz / 12 = 2Mhz, and thus the real frequency of the CPU should be 2MHz * 16 = 32Mhz. Seems legitimate to me.

When changing to freq_out : 16, I'm getting

libsigrok 0.6.0-git-400bc4f
Acquisition with 1/8 channels at 24 MHz
D6:00001100 00000000 00000000 00001100 00000000 00000000 00001100 00000000
D6:00000000 00001100 00000000 00000000 00001100 00000000 00000000 00000100
D6:00000000 00000000 00000100 00000000 00000000 00000100 00000000 00000000
D6:00000100 00000000 00000000 00000100 00000000 00000000 00000110 00000000
D6:00000000 00000110 00000000 00000000 00000110 00000000 00000000 00000110
D6:00000000 00000000 00000110 00000000 00000000 00000010 00000000 00000000
D6:00000010 00000000 00000000 00000010 00000000 00000000 00000010 00000000
D6:00000000 00000010 00000000 00000000 00000011 00000000 00000000 00000011

The repetition is roughly two 1 followed by twenty-two 0, so yeah, the "real" clock is about 16MHz. You can see some imperfect repetitions which contains only one 1, though.

In the meantime, I'm happy to accept the PR if you change frequency to 32

The contributed icesugar target already runs at 32HMz. I have updated README.md to clarify.

olofk commented 4 years ago

Fantastic! Thanks for your contribution and for the detailed debugging. Picked and pushed

nalzok commented 4 years ago

Glad you have found the problem! I can confirms that freq_out : 17 works at the baud rate of 57600 on iCESugar.

Maybe we should also use freq_out : 17 or freq_out : 32 for iCEBreaker? Not sure how things work on that board though, since I don't have an iCEBreaker at hand.

olofk commented 4 years ago

Cool. Thanks for confirming. I don't have an icebreaker myself, but as you say, 17 is probably a safer choice. But I also sent a fix to icepll that I hope will take care of this the right way https://github.com/YosysHQ/icestorm/pull/270