dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.16k stars 582 forks source link

ILI9341 needn't limit range of SPI buffer size #1792

Closed HumJ0218 closed 2 years ago

HumJ0218 commented 2 years ago

https://github.com/dotnet/iot/blob/d84c3e8b25b5c3e3363f096647487ab62ee0b26f/src/devices/Ili9341/Ili9341.cs#L56

My Raspberry Pi 4B has set spidev.bufsiz=1048576 so I can set buffer to more than 65536.

I tried to delete this limit. It worked well and didn't throw any exception. (while SPI frequency was over than 80MHz! )

I thought this limit could be removed if it wasn't necessary.

PS. Every time it is refreshed, the screen will flash once, is this a "feature" of this device?

pgrawehr commented 2 years ago

I'm going to work with this device quite soon and so will test your proposal, but I have some other tasks first.

HumJ0218 commented 2 years ago

I'm going to work with this device quite soon and so will test your proposal, but I have some other tasks first.

It doesn't matter, I'm not in a hurry

krwq commented 2 years ago

we should try with both RPi3, RPi4 and FT4222 and see the results - IMO if one of them fails we should have a limit or probe for board. We don't want APIs to fail by default when using on RPI3 as it might not be obvious to figure out that culprit is bufferSize, I think increasing/using default buffer size is a minor optimization while user experience is much more important so make sure to remove only when not seeing issues

HumJ0218 commented 2 years ago

https://github.com/dotnet/iot/blob/2db2f0212e00c94ec57f705a46b34f8b6e5bb4d1/src/devices/Common/System/Device/Spi/SpiBusInfo.cs#L19

I found this can tell the buffer size. Maybe devices can use this to have limit

pgrawehr commented 2 years ago

After some testing, I agree that this test should be removed. If we need a SPI buffer size limitation, it should be in the driver, not in a random binding (with seemingly random limits). The test failed for me because I was using a firmata connection, where the maximum buffer size is by default only 25 bytes.

The SpiBusInfo can't be used in a binding either, because that only applies to the standard SPI bus, but would be completely wrong if connected trough FT4222/Firmata.

@HumJ0218 Are you going to create a PR on this or shall I?

HumJ0218 commented 2 years ago

I will create PR ASAP

HumJ0218 commented 2 years ago

@pgrawehr I have a UMFT4222EV module. But I can't use it. It always throw this exception:

Unable to load DLL 'ftd2xx' or one of its dependencies: The specified module could not be found. (0x8007007E)

My OS is WIndows 11 Pro 21H2 22000.493 x64

krwq commented 2 years ago

it should be in the driver, not in a random binding (with seemingly random limits)

depends where the SPI limit is - if you can send 80kb at once but device can only handle 65kb then you should check the limit in the binding... I'd start with trying out some most popular configuration, random hack in the binding doesn't hurt while not working binding does...

pgrawehr commented 2 years ago

@krwq True, but here the opposite is true. The device can handle very big data streams, but typically the SPI driver cannot. Here the binding dictates that the SPI buffer size must be between 4kb and 64kb, which is apparently just random. It works perfectly well (although slow), with smaller sizes and with larger ones as well. I'll check the specs again, but I haven't seen any limitation on the amount of data you can send at once (You wouldn't normally go above the full size of a frame, though)

HumJ0218 commented 2 years ago

@pgrawehr according to my testing, using RPi4, ILI9341 can handle whole frame data at maxmum 83Mhz (when 84 screen will be noising) and there is about 30 FPS.

pgrawehr commented 2 years ago

Closing via #1802