-
Notifications
You must be signed in to change notification settings - Fork 59
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
piolib: support transfer lengths that don't fit in 16 bits #107
Comments
That's a lot of data - what's your use case? I'm not against in principle, it's just a matter of not breaking anything in the process. It might be that, due to the alignment requirements, switching to uint32_t would just work. |
I'm driving HUB75 matrices with PIO. Ideally I'd submit a whole frame with one ioctl, but even a 64x32 matrix with 10 bitplanes exceeds 64kB in the encoding I'm using. And folks like to use several of these matrices in daisy chain mode. Right now I'm just submitting 32kB at a time and trying to determine whether some artifacts I'm seeing are due to a flaw in my code or are due to a pause in the bitstream between ioctls. (a smaller 32x16 display that just needs one data submission per frame doesn't have these artifacts) |
(@ladyada ping for interest) |
In many places, the number of bytes in a data transfer is stored as a 32-bit (or greater) value, but the ioctl API to the kernel stores it in a 16-bit field, alongside which is a 16-bit gap due to the alignment requirements of the pointer that follows. Increase the size of data_bytes to a uint32_t, so that it can handle larger transfers. Since using different sizes as either end can lead to unpredictable results, a separate IOCTL is used for larger transfers, allowing the kernel support to be determined and backwards compatibility to be maintained. See: raspberrypi#107 Signed-off-by: Phil Elwell <[email protected]>
In many places, the number of bytes in a data transfer is stored as a 32-bit (or greater) value, but the ioctl API to the kernel stores it in a 16-bit field, alongside which is a 16-bit gap due to the alignment requirements of the pointer that follows. Increase the size of data_bytes to a uint32_t, so that it can handle larger transfers. Since using different sizes as either end can lead to unpredictable results, a separate IOCTL is used for larger transfers, allowing the kernel support to be determined and backwards compatibility to be maintained. See: raspberrypi#107 Signed-off-by: Phil Elwell <[email protected]>
In many places, the number of bytes in a data transfer is stored as a 32-bit (or greater) value, but the ioctl API to the kernel stores it in a 16-bit field, alongside which is a 16-bit gap due to the alignment requirements of the pointer that follows. Increase the size of data_bytes to a uint32_t, so that it can handle larger transfers. Since using different sizes as either end can lead to unpredictable results, a separate IOCTL is used for larger transfers, allowing the kernel support to be determined and backwards compatibility to be maintained. See: raspberrypi#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
There are now two PRs for this: one for piolib (#108), and one for the kernel driver (raspberrypi/linux#6543). Both are needed for larger transfers to work. The library should simply fail to do larger transfers with a non-updated kernel. |
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Thanks! |
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
In many places, the number of bytes in a data transfer is stored as a 32-bit (or greater) value, but the ioctl API to the kernel stores it in a 16-bit field, alongside which is a 16-bit gap due to the alignment requirements of the pointer that follows. Increase the size of data_bytes to a uint32_t, so that it can handle larger transfers. Since using different sizes as either end can lead to unpredictable results, a separate IOCTL is used for larger transfers, allowing the kernel support to be determined and backwards compatibility to be maintained. See: raspberrypi#107 Signed-off-by: Phil Elwell <[email protected]>
In many places, the number of bytes in a data transfer is stored as a 32-bit (or greater) value, but the ioctl API to the kernel stores it in a 16-bit field, alongside which is a 16-bit gap due to the alignment requirements of the pointer that follows. Increase the size of data_bytes to a uint32_t, so that it can handle larger transfers. Since using different sizes as either end can lead to unpredictable results, a separate IOCTL is used for larger transfers, allowing the kernel support to be determined and backwards compatibility to be maintained. See: #107 Signed-off-by: Phil Elwell <[email protected]>
That's all been merged now. The driver change missed yesterdays rpi-update release, but you can install a kernel containing the updated driver using either |
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
This appears to be working, thanks! I don't know if this issue should be closed now, or if it ought to wait for a released kernel to support it? |
The kernel will be released as sure as night follows day, so let's close this now. |
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Is there something else I need to do to make this work besides sudo rpi-updates rpi-6.6.y and compiling? I made a sample: This works fine for BUF_SIZE 32768 but BUF_SIZE 32768*2 seg faults first time and locks the os requiring power cycle the 2nd time. |
I think |
This works now if I set pio_sm_config_xfer(pio, sm, PIO_DIR_TO_SM, 32768, 2);//this is max size but though I'm confused what pio_sm_config_xfer(pio, sm, PIO_DIR_TO_SM, 32768, 2) does exactly. Does it just basically split into multiple 32768 transitions or is this number just ignored? |
The kernel code handling this API call is https://github.com/raspberrypi/linux/blob/rpi-6.6.y/drivers/misc/rp1-pio.c#L606 (but be aware there's some translation of arguments between the userspace From what I understand reading it, this allocates DMA buffers in the Linux kernel's address space. The numbers given are the buffer size & the count of buffers, and there can be up to 4 buffers. When the transmission actually occurs, data is copied from userspace to the buffer, and then the dma is submitted (https://github.com/raspberrypi/linux/blob/a18d9ced4965462cb7b3b4252ada440395105308/drivers/misc/rp1-pio.c#L726). When the number of buffers is greater than 1, more an one dma can be submitted, which likely reduces the gaps between the dma transmissions. Reasons for using fewer or smaller buffers? I'm not sure. Linux DMA-capable memory might be limited, and the returns for larger buffer sizes seemed to be modest in my own testing. However, I was throughput-oriented in my testing (though I would have expected 5ms gaps as you noted in #117 to be apparent in my application) |
I've been thinking about how to do it to implement a ping pong buffer and I guess way one way could be use 2 sm. First reads in number of bytes to shift with mov x, num_bytes and then does decrement until end reached. Afterwards can wait on irq 0 and sets irq1. Second sm waits on irq 1 and does same thing. Irq is not available in userspace but seems assembly could should still work for interprocess communication shouldn't it? |
There are two PRs - one in the kernel (raspberrypi/linux#6640) and one here (#119) - that together support larger DMA buffers. You will still end up with small gaps between the buffers - I'm not aware of a way of adding DMA descriptors to a chain while the controller is running - but you get to decide how frequent they are. Another approach would be to use cyclic transfers, which is how ALSA manages transfers to I2S soundcards, but that's another level of complexity. |
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Provide remote access to the PIO hardware in RP1. There is a single instance, with 4 state machines. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Support larger data transfers Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: More logical probe sequence Sort the probe function initialisation into a more logical order. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Minor cosmetic tweaks No functional change. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Add in-kernel DMA support Add kernel-facing implementations of pio_sm_config_xfer and pio_xm_xfer_data. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Handle probe errors Ensure that rp1_pio_open fails if the device failed to probe. Link: raspberrypi#6593 Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: SM_CONFIG_XFER32 = larger DMA bufs Add an ioctl type - SM_CONFIG_XFER32 - that takes uints for the buf_size and buf_count values. Signed-off-by: Phil Elwell <[email protected]> misc/rp1-pio: Fix copy/paste error in pio_rp1.h As per the subject, there was a copy/paste error that caused pio_sm_unclaim from a driver to result in a call to pio_sm_claim. Fix it. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Fix parameter checks wihout client Passing bad parameters to an API call without a pio pointer will cause a NULL pointer exception when the persistent error is set. Guard against that. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Convert floats to 24.8 fixed point Floating point arithmetic is not supported in the kernel, so use fixed point instead. Signed-off-by: Phil Elwell <[email protected]>
Provide remote access to the PIO hardware in RP1. There is a single instance, with 4 state machines. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Support larger data transfers Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: More logical probe sequence Sort the probe function initialisation into a more logical order. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Minor cosmetic tweaks No functional change. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Add in-kernel DMA support Add kernel-facing implementations of pio_sm_config_xfer and pio_xm_xfer_data. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Handle probe errors Ensure that rp1_pio_open fails if the device failed to probe. Link: #6593 Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: SM_CONFIG_XFER32 = larger DMA bufs Add an ioctl type - SM_CONFIG_XFER32 - that takes uints for the buf_size and buf_count values. Signed-off-by: Phil Elwell <[email protected]> misc/rp1-pio: Fix copy/paste error in pio_rp1.h As per the subject, there was a copy/paste error that caused pio_sm_unclaim from a driver to result in a call to pio_sm_claim. Fix it. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Fix parameter checks wihout client Passing bad parameters to an API call without a pio pointer will cause a NULL pointer exception when the persistent error is set. Guard against that. Signed-off-by: Phil Elwell <[email protected]> misc: rp1-pio: Convert floats to 24.8 fixed point Floating point arithmetic is not supported in the kernel, so use fixed point instead. Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
Add a separate IOCTL for larger transfer with a 32-bit data_bytes field. See: raspberrypi/utils#107 Signed-off-by: Phil Elwell <[email protected]>
The structure used for data transfers is limited to a 16-bit count of data_bytes.
Our application would like to do larger transfers. Right now we are working around this limitation by submitting several buffers of data sequentially, but it would be preferable if we could submit all the data in one go:
The text was updated successfully, but these errors were encountered: