linux icon indicating copy to clipboard operation
linux copied to clipboard

rp1-pio config and transfer not right synced.

Open pa3gsb opened this issue 6 months ago • 7 comments

Describe the bug


Title: rp1_pio_sm_config_xfer_internal() should support external DMA buffer configuration (zero-copy use cases)

Description:

The current implementation of rp1_pio_sm_config_xfer_internal() always allocates internal DMA buffers using dma_alloc_coherent(). However, rp1_pio_sm_xfer_data() supports passing an external dma_addr, allowing the user to manage DMA buffers independently.

This creates an inconsistency:

  • The configuration function (*_config_xfer_internal) does not allow setting up the DMA channel and FIFO transfer without also allocating bounce buffers.
  • The transfer function (*_xfer_data) allows external buffers, but the configuration function does not account for this setup.

Further info iam defining a driver for a radio card (called radioberry). To use the pio makes it really nice and iam offload the CPU in my first experiments i saw a reduction from 40% CPU util to 4%.

Curious to the reply?

Johan

This prevents more advanced or zero-copy DMA use cases, where DMA buffers are allocated and managed externally (e.g., shared buffers, user-mapped memory, etc.).

Suggested improvement:

Enhance rp1_pio_sm_config_xfer_internal() to support different DMA buffer allocation modes:

enum rp1_pio_dma_alloc_mode {
    RP1_PIO_DMA_ALLOC_INTERNAL,  // current behavior
    RP1_PIO_DMA_ALLOC_EXTERNAL,  // setup DMA channel, but skip buffer allocation
    RP1_PIO_DMA_ALLOC_NONE       // configure only, without managing any buffers
};

This would allow developers to configure DMA with:

  • Internal bounce buffers (default),
  • Externally managed buffers (used with pre-mapped dma_addr),
  • Or just set up the DMA channel and slave config (for full manual control).

Benefits:

  • Supports zero-copy DMA patterns.
  • Provides flexibility for integrating with other subsystems.
  • Maintains backward compatibility.

Steps to reproduce the behaviour

no steps but just looking into the code

Device (s)

Raspberry Pi 5

System

latest.

Logs

No response

Additional context

No response

pa3gsb avatar Jun 22 '25 09:06 pa3gsb

Can you not just give rp1_pio_sm_config_xfer (and hence rp1_pio_sm_config_xfer_internal) a buf_count of 0? That will then cause it to claim the DMA channel etc. without allocating any DMA-coherent memory.

pelwell avatar Jun 23 '25 14:06 pelwell

iam afraid that iam not able to point to the right device when i have to make my own coherent allocations

dbi->buf = dma_alloc_coherent(dma->chan->device->dev, buf_size, &dbi->dma_addr, GFP_KERNEL);

pa3gsb avatar Jun 23 '25 20:06 pa3gsb

You could always use something like this from the rp1 driver:

	rp1_node = of_find_node_by_name(NULL, "rp1");
	if (!rp1_node) {
		dev_err(&pdev->dev, "failed to find RP1 DT node\n");
		return -EINVAL;
	}

	pcie_pdev = of_find_device_by_node(rp1_node->parent);

but I take your point. Passing in physical addresses rather than DMA addresses would probably solve the problem, but then the conversion to DMA addresses would happen at point of use.

I'll give it some thought.

pelwell avatar Jun 24 '25 09:06 pelwell

i added a simple method which worked for me.

void *rp1_pio_sm_get_virt_buffer(struct rp1_pio_client *client,
                                 uint sm, uint dir, int index)
{
    struct rp1_pio_device *pio = client->pio;
    struct dma_info       *dma;

    /* Valideer argumenten */
    if (sm >= RP1_PIO_SMS_COUNT || dir >= RP1_PIO_DIR_COUNT)
        return NULL;
    dma = &pio->dma_configs[sm][dir];
    if (index < 0 || index >= dma->buf_count)
        return NULL;
    /* “bufs” array is van type struct dma_buf_info bufs[DMA_BOUNCE_BUFFER_COUNT]; */
    return dma->bufs[index].buf;
}
EXPORT_SYMBOL_GPL(rp1_pio_sm_get_virt_buffer);

pa3gsb avatar Jun 24 '25 10:06 pa3gsb

i did modify my driver to use your idea passing a 0 into buf_count.

but i receive the following error: radioberry: Failed to configure DMA -22 return -EINVAL;

static int rp1_pio_sm_config_xfer_internal(struct rp1_pio_client *client, uint sm, uint dir,
					   uint buf_size, uint buf_count)
{
	struct rp1_pio_sm_set_dmactrl_args set_dmactrl_args;
	struct rp1_pio_device *pio = client->pio;
	struct platform_device *pdev = pio->pdev;
	struct device *dev = &pdev->dev;
	struct dma_slave_config config = {};
	phys_addr_t fifo_addr;
	struct dma_info *dma;
	uint32_t dma_mask;
	char chan_name[4];
	int ret = 0;

	if (sm >= RP1_PIO_SMS_COUNT || dir >= RP1_PIO_DIR_COUNT)
		return -EINVAL;
	if ((buf_count || buf_size) &&
	    (!buf_size || (buf_size & 3) ||
	     **!buf_count** || buf_count > DMA_BOUNCE_BUFFER_COUNT))
		return -EINVAL;

pa3gsb avatar Jun 25 '25 17:06 pa3gsb

Did mod the driver; removed the precondition; and give it a try The device for the alloc is: brcm-pcie 1000120000.pcie: Radioberry DMA loopt via device 1000120000.pcie

Iam only receiving 0 values.

i suppose iam getting a wrong DMA controller: PCIe DMA channel, not an RP1 DMA channel.

pa3gsb avatar Jun 25 '25 18:06 pa3gsb

i think the idea of adding a new method is a good direction. so i added a PR https://github.com/raspberrypi/linux/pull/6927

pa3gsb avatar Jun 26 '25 18:06 pa3gsb