search

zephray / VerilogBoy

A Pi emulating a GameBoy sounds cheap. What about an FPGA?
https://hackaday.io/project/57660-verilogboy
Other
465 stars 57 forks source link

Halt on USB #20

Open jroever opened 5 years ago

jroever commented 5 years ago

Tried your latest panog1 to play with usb - got some life after removing old MIF includes in pick ROM and RAM modules - but running into HALT on USB. Was wondering if a) you have similar issues and b) could this be related to ISP1760 bug (found mention of workaround in SAF1760 documentation chapter 17.3.)

zephray commented 5 years ago

I am not sure what exact situation you are in (like Halt on what device, when does it halt, etc.) But I can confirm I am having the similar issues in some cases:

According to the datasheet,

When low-speed or full-speed devices are connected through a high-speed hub, the problem will not occur

On PanoG1 RevC, there is one on-board HS hub (SMSC 2513), means external devices would be always connected through the HS hub. But If your device is a RevB, then, probably.

jroever commented 5 years ago

Thanks for the quick and detailed response! Yes, I have a rev C and had forgotten about the external HUB (... but that's probably why that hub got added in Rev C - mystery solved :) I was indeed trying with PS4 controller ... so if the halt is expected then I can start focusing on getting mouse HID to work instead (want ultra low latency trackball) - planning to do the INT/IRQ processing in HW and only do the device enumeration on pico - should be interesting hybrid. Thanks for all the solid pre-work to get USB alive!

zephray commented 5 years ago

Yes, the PS4 controller is, fairly complicated. If you own a second gen PS4 controller as I did, it is capable of transmitting audio through the USB. The controller interface is, I remember the third or forth interface provided by the controller. I have to admit I havn't done any debugging about the PS4 controller situation... Will try to look into it. Software and hardware hybrid sounds pretty interesting! By the way, in the ISP/SAF1760, I see for the interrupt transfer, there are few data bytes slots in the PTD. I am not sure what it is used for, and I am currently not using them, but my guess would be, it could be possible to just get the data from there. Just some thoughts. Good luck, would like to hear more updates.

jroever commented 5 years ago

... not sure if it makes any difference - but I just notice that isp1760.c line 182: isp_write_dword(ISP_PORT_1_CONTROL, 0x00800018); seems to be setting reserved bits 2 & 1 not as per spec - should probably be 0x0080001e

jroever commented 5 years ago

argh - never mind - I am such a sequential reader ;) just saw the footnote ...

zephray commented 5 years ago

I did few modifications to optimize the RAM usage, now RAM size for PicoRV32 is reduced to 8KB (Not sure if 4KB is achievable or not). Tried my DualShock 4 again, and it mysteriously started working. Not sure why though.

jroever commented 5 years ago

Interesting - I did find a few isp1760 settings that did not set the reserved bits as suggested - but it did not seem to change USB functionality. I can now do a get_report on the control endpoint for a logitech mouse - but the same does not work for my kensington trackball or my Blue Pill USB mouse (which will be my centepede trackball controller) ... they don't seem to support getting reports other than on the IRQ endpoint - which I have not working yet. Do you have keyboard and gamepads working with IRQ or via polling on the control port? ISP1760 documentation is horrid - it talks about periodic transaction support and frame list registers - but once you dig down the USBCMD registers to enable some of this functionality aren't even present (or at least not documented) - sigh! Would be good to know how far you got with this ...

zephray commented 5 years ago

Well, I only know the case for gamepads, the HID report should be sent through the interrupt endpoint, rather than the control endpoint.

So, what I have implemented here is... Polling on the interrupt endpoint.

Basically in my gamepad HID class driver, it will register a periodic interrupt transfer to the device driver: https://github.com/zephray/VerilogBoy/blob/3b1de6af93990f8c6b7b30b9fa3d141c6fbc8e42/target/panog1/fw/firmware/usb_gamepad.c#L386

Then this periodic transfer is saved in the drivers periodic transfer list. But rather than using hardware IRQ or whatever periodic transfer function that ISP1760 is supported, I am just doing polling. The main function is supposed to call the polling function periodically: https://github.com/zephray/VerilogBoy/blob/3b1de6af93990f8c6b7b30b9fa3d141c6fbc8e42/target/panog1/fw/firmware/isp1760.c#L931

This function would go through all the registered transfers, start an interrupt transfer, and wait till it finishes, read the payload back, and call the registered callback (from HID class driver).

Note this is not how things normally should be done. One should probably not wait on the interrupt transfer to be done, and it should probably not relying on my main function to call the driver to go and get the report. But basically I am just confused with ISP1760's documentation and implemented whatever I guessed should work.

Now all these are working, being able to get the data from interrupt endpoint, so it's usable, but probably not ideal.

zephray commented 5 years ago

By the way, I used to try implement an interrupt based Interrupt Endpoint transfer:

  1. Start an interrupt transfer but not wait for the finish.
  2. When it finishes (either success or fail) an interrupt should be generated.
  3. If it was success, read the payload out and call the HID class driver callback to parse the report.
  4. Whatever the result was, start an new interrupt transfer and go to step 2.

The issue is, I don't always get the interrupt. Namely, when it stalls. Then if I don't get the interrupt, there will be no retry, and there will be no data, the loop just stuck on the step 2. This is probably related to my interrupt register configuration. In practice, the loop can run for around 5-15s before it stucks at step 2.

Again, I am not sure if this is how things should be done.

jroever commented 5 years ago

Thanks, that helped a lot. Now I don't have to waste time trying to go down the road you already took ... I think I will try and see if I can use the SOF interrupt instead ... if I can get that going then we can have a kick off method that is synced with the USB timing but does not rely on success of prior transactions BTW: in your G1 flash instructions - using firmware.elf instead of .bin works fine for me ... are you doing something special to alter the .bin? PS: Two questions https://github.com/zephray/VerilogBoy/blob/3b1de6af93990f8c6b7b30b9fa3d141c6fbc8e42/target/panog1/fw/firmware/isp1760.c#L463-L468

I have not done anything with HS devices, but shouldn't micro_frame for HS be zero? Why are you setting micro_scs to FE not FF as per ISP1760?

zephray commented 5 years ago

Hi. No, I am not altering the .bin... For me, using .elf will generate the correct .mcs and works just fine, but the bar graph shown in the iMPACT are showing the raw .elf file size rather than the data size. I just feel like it is confusing so I recommend use .bin. For the micro_frame and micro_scs... I just wrote it to be FF at first to set it to maximum value possible, for no specific reason. Setting micro_scs to FE is likely a bug, I was thinking something like the DONEMAP when I wrote these, but apparently it's not. I have to say, in this driver, many values are basically arbitrarily chosen when I was writing the driver. Just feel free to ask and point out if it doesn't make much sense. Pull Requests would be very appreciated.

jroever commented 5 years ago

Digging a bit deeper - I could not find where complete split transfers are done in isp1760.c - I only see the start splits for FS/LS endpoints ... https://github.com/zephray/VerilogBoy/blob/3b1de6af93990f8c6b7b30b9fa3d141c6fbc8e42/target/panog1/fw/firmware/isp1760.c#L461

My code is currently a completely mutilated verilogboy clone while I am experimenting ... will do a pull request once I clean things back up :)

jroever commented 5 years ago

Ok, I think I see it now - sneaky :) HW updates the SC bit ... So it seems the ISP1760 will take care of both start split and complete split issued by SW as a single isp_submit? Why are there two registered PTDs then? Or are the two somehow linked and one becomes the start-split and the other the complete-split? It's a wee bit confusion ...

https://github.com/westerndigitalcorporation/RISC-V-Linux/blob/13cb16d5e8e11ebca490cad50cc5abbd222839a1/linux/drivers/usb/isp1760/isp1760-hcd.c#L535-L668 seems to have loads of useful information ... is that the linux reference you made it usb.h? would be good to give the full link

jroever commented 5 years ago

I noticed that in firmware.asm the irq routine starts at e000_0008 but PROGADDR_IRQ for picorv is e000_0010 - was that intended?

zephray commented 5 years ago

No... It’s a bug, apparently when I moved from RV32I to RV32IC, the address changed. I will fix it.

ggsubs commented 5 years ago

I had issues getting a Midi keyboard up and running. The issue was about mismatching transfer lengths - a 0x4000 bit was coming on some devices. Masking it in isp1760.c solved it: 


    if (direction == DIRECTION_IN) {
        if (result == ISP_SUCCESS) {
            actual_transfer_length = actual_transfer_length & 0x0FFF;
zephray commented 5 years ago

I had issues getting a Midi keyboard up and running. The issue was about mismatching transfer lengths - a 0x4000 bit was coming on some devices. Masking it in isp1760.c solved it:

Thanks for pointing out, applied the fix in the last commit.