-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unnecessary enforcement of 32-boundaries in oni_read_frame() #16
Comments
To be fair, while it might have originated from xillybus requirements, it now is more related to the general ONIX pipeline than any specific driver. This is because, for performance, we just pack the pipeline endpoint into 32-bit words. (Part of it is still 16-bit words, but we are actively working towards making the whole pipeline work natively at 32bit). However, while this is true of all our current hardware, it is not necessarily true for any possible ONI implementation. That said, it is still good to cater for the possibility of some hardware having specific word sizes, as the opposite will entail unnecessary buffering (e.g.: asking for 5 bytes from a device only capable of streaming 16-bit words). I think the proper solution would be to let the onidrivers to specify their read word size and use it instead of a fixed size. Of course, if this results in padding being required, it will be the hardware responsibility (as it is now, since it is the ONIX hardware which adds padding zeroes) |
In this line we round our frame data reads up to the next 32-bit boundary.
liboni/api/liboni/oni.c
Line 676 in 132a12d
AFAIK, this was implemented way back in the day when Xillybus was being called directly in the library, with a 32-bit wide read FIFO used to stream data, and therefore anything other than 32-bit reads could corrupt the stream. However, now we are doing block reads which can be set to any byte size so long as they are big enough to store the largest frame produced in the device table. Is there still a reason we are enforcing this? If the hardware does not produce 32-bit aligned data, this rounding operation will result in a hard crash, but this seems totally unnecessary. In fact, I discovered this limitation because I'm modifying the onidriver_test.c to remove some of its limitations. In doing so I forgot to make sure it produced 32-bit aligned data. Rather than forcing this, I removed this step and now return
rsize
and everything worked fine.The text was updated successfully, but these errors were encountered: