2023-01-17 23:46:58

by Lizhi Hou

[permalink] [raw]
Subject: [RESEND PATCH V11 XDMA 0/2] xilinx XDMA driver

Hello,

This V11 of patch series is to provide the platform driver to support the
Xilinx XDMA subsystem. The XDMA subsystem is used in conjunction with the
PCI Express IP block to provide high performance data transfer between host
memory and the card's DMA subsystem. It also provides up to 16 user
interrupt wires to user logic that generate interrupts to the host.

+-------+ +-------+ +-----------+
PCIe | | | | | |
Tx/Rx | | | | AXI | |
<=======> | PCIE | <===> | XDMA | <====>| User Logic|
| | | | | |
+-------+ +-------+ +-----------+

The XDMA has been used for Xilinx Alveo PCIe devices.
And it is also integrated into Versal ACAP DMA and Bridge Subsystem.
https://www.xilinx.com/products/boards-and-kits/alveo.html
https://docs.xilinx.com/r/en-US/pg344-pcie-dma-versal/Introduction-to-the-DMA-and-Bridge-Subsystems

The device driver for any FPGA based PCIe device which leverages XDMA can
call the standard dmaengine APIs to discover and use the XDMA subsystem
without duplicating the XDMA driver code in its own driver.

Changes since v10:
- Added Tested-by Martin Tuma [email protected]

Changes since v9:
- Cleanup code based on review comments.

Changes since v8:
- Fixed test robot failure on s390.

Changes since v7:
- Used pci device pointer for dma_pool_create().

Changes since v6:
- Fixed descriptor filling bug.

Changes since v5:
- Modified user logic interrupt APIs to handle user logic IP which does not
have its own register to enable/disable interrupt.
- Clean up code based on review comments.

Changes since v4:
- Modified user logic interrupt APIs.

Changes since v3:
- Added one patch to support user logic interrupt.

Changes since v2:
- Removed tasklet.
- Fixed regression bug introduced to V2.
- Test Robot warning.

Changes since v1:
- Moved filling hardware descriptor to xdma_prep_device_sg().
- Changed hardware descriptor enum to "struct xdma_hw_desc".
- Minor changes from code review comments.

Lizhi Hou (2):
dmaengine: xilinx: xdma: Add xilinx xdma driver
dmaengine: xilinx: xdma: Add user logic interrupt support

MAINTAINERS | 11 +
drivers/dma/Kconfig | 14 +
drivers/dma/xilinx/Makefile | 1 +
drivers/dma/xilinx/xdma-regs.h | 173 ++++
drivers/dma/xilinx/xdma.c | 1004 ++++++++++++++++++++++++
include/linux/dma/amd_xdma.h | 16 +
include/linux/platform_data/amd_xdma.h | 34 +
7 files changed, 1253 insertions(+)
create mode 100644 drivers/dma/xilinx/xdma-regs.h
create mode 100644 drivers/dma/xilinx/xdma.c
create mode 100644 include/linux/dma/amd_xdma.h
create mode 100644 include/linux/platform_data/amd_xdma.h

--
2.27.0


2023-01-17 23:48:13

by Lizhi Hou

[permalink] [raw]
Subject: [RESEND PATCH V11 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

The Xilinx DMA/Bridge Subsystem for PCIe (XDMA) provides up to 16 user
interrupt wires to user logic that generate interrupts to the host.
This patch adds APIs to enable/disable user logic interrupt for a given
interrupt wire index.

Signed-off-by: Lizhi Hou <[email protected]>
Signed-off-by: Sonal Santan <[email protected]>
Signed-off-by: Max Zhen <[email protected]>
Signed-off-by: Brian Xu <[email protected]>
Tested-by: Martin Tuma <[email protected]>
---
MAINTAINERS | 1 +
drivers/dma/xilinx/xdma.c | 85 ++++++++++++++++++++++++++++++++++++
include/linux/dma/amd_xdma.h | 16 +++++++
3 files changed, 102 insertions(+)
create mode 100644 include/linux/dma/amd_xdma.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d598c4e23901..eaf6590dda19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22583,6 +22583,7 @@ L: [email protected]
S: Supported
F: drivers/dma/xilinx/xdma-regs.h
F: drivers/dma/xilinx/xdma.c
+F: include/linux/dma/amd_xdma.h
F: include/linux/platform_data/amd_xdma.h

XILINX ZYNQMP DPDMA DRIVER
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 118528295fb7..846f10317bba 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -25,6 +25,7 @@
#include <linux/dmapool.h>
#include <linux/regmap.h>
#include <linux/dmaengine.h>
+#include <linux/dma/amd_xdma.h>
#include <linux/platform_device.h>
#include <linux/platform_data/amd_xdma.h>
#include <linux/dma-mapping.h>
@@ -713,6 +714,7 @@ static int xdma_set_vector_reg(struct xdma_device *xdev, u32 vec_tbl_start,
static int xdma_irq_init(struct xdma_device *xdev)
{
u32 irq = xdev->irq_start;
+ u32 user_irq_start;
int i, j, ret;

/* return failure if there are not enough IRQs */
@@ -755,6 +757,18 @@ static int xdma_irq_init(struct xdma_device *xdev)
goto failed_init_c2h;
}

+ /* config user IRQ registers if needed */
+ user_irq_start = XDMA_CHAN_NUM(xdev);
+ if (xdev->irq_num > user_irq_start) {
+ ret = xdma_set_vector_reg(xdev, XDMA_IRQ_USER_VEC_NUM,
+ user_irq_start,
+ xdev->irq_num - user_irq_start);
+ if (ret) {
+ xdma_err(xdev, "failed to set user vectors: %d", ret);
+ goto failed_init_c2h;
+ }
+ }
+
/* enable interrupt */
ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_CHAN_INT_EN_W1S, ~0);
if (ret)
@@ -780,6 +794,77 @@ static bool xdma_filter_fn(struct dma_chan *chan, void *param)
return chan_info->dir == xdma_chan->dir;
}

+/**
+ * xdma_disable_user_irq - Disable user interrupt
+ * @pdev: Pointer to the platform_device structure
+ * @irq_num: System IRQ number
+ */
+void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+ u32 user_irq_index;
+
+ user_irq_index = irq_num - xdev->irq_start;
+ if (user_irq_index < XDMA_CHAN_NUM(xdev) ||
+ user_irq_index >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq number");
+ return;
+ }
+ user_irq_index -= XDMA_CHAN_NUM(xdev);
+
+ xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1C,
+ (1 << user_irq_index));
+}
+EXPORT_SYMBOL(xdma_disable_user_irq);
+
+/**
+ * xdma_enable_user_irq - Enable user logic interrupt
+ * @pdev: Pointer to the platform_device structure
+ * @irq_num: System IRQ number
+ */
+int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+ u32 user_irq_index;
+ int ret;
+
+ user_irq_index = irq_num - xdev->irq_start;
+ if (user_irq_index < XDMA_CHAN_NUM(xdev) ||
+ user_irq_index >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq number");
+ return -EINVAL;
+ }
+ user_irq_index -= XDMA_CHAN_NUM(xdev);
+
+ ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1S,
+ (1 << user_irq_index));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(xdma_enable_user_irq);
+
+/**
+ * xdma_get_user_irq - Get system IRQ number
+ * @pdev: Pointer to the platform_device structure
+ * @user_irq_index: User logic IRQ wire index
+ *
+ * Return: The system IRQ number allocated for the given wire index.
+ */
+int xdma_get_user_irq(struct platform_device *pdev, u32 user_irq_index)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+
+ if (XDMA_CHAN_NUM(xdev) + user_irq_index >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq index");
+ return -EINVAL;
+ }
+
+ return xdev->irq_start + XDMA_CHAN_NUM(xdev) + user_irq_index;
+}
+EXPORT_SYMBOL(xdma_get_user_irq);
+
/**
* xdma_remove - Driver remove function
* @pdev: Pointer to the platform_device structure
diff --git a/include/linux/dma/amd_xdma.h b/include/linux/dma/amd_xdma.h
new file mode 100644
index 000000000000..ceba69ed7cb4
--- /dev/null
+++ b/include/linux/dma/amd_xdma.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2022, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _DMAENGINE_AMD_XDMA_H
+#define _DMAENGINE_AMD_XDMA_H
+
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+
+int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num);
+void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num);
+int xdma_get_user_irq(struct platform_device *pdev, u32 user_irq_index);
+
+#endif /* _DMAENGINE_AMD_XDMA_H */
--
2.27.0

2023-01-18 07:20:55

by Vinod Koul

[permalink] [raw]
Subject: Re: [RESEND PATCH V11 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

On 17-01-23, 12:54, Lizhi Hou wrote:
> The Xilinx DMA/Bridge Subsystem for PCIe (XDMA) provides up to 16 user
> interrupt wires to user logic that generate interrupts to the host.
> This patch adds APIs to enable/disable user logic interrupt for a given
> interrupt wire index.
>
> Signed-off-by: Lizhi Hou <[email protected]>
> Signed-off-by: Sonal Santan <[email protected]>
> Signed-off-by: Max Zhen <[email protected]>
> Signed-off-by: Brian Xu <[email protected]>
> Tested-by: Martin Tuma <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/dma/xilinx/xdma.c | 85 ++++++++++++++++++++++++++++++++++++
> include/linux/dma/amd_xdma.h | 16 +++++++
> 3 files changed, 102 insertions(+)
> create mode 100644 include/linux/dma/amd_xdma.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d598c4e23901..eaf6590dda19 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22583,6 +22583,7 @@ L: [email protected]
> S: Supported
> F: drivers/dma/xilinx/xdma-regs.h
> F: drivers/dma/xilinx/xdma.c
> +F: include/linux/dma/amd_xdma.h
> F: include/linux/platform_data/amd_xdma.h
>
> XILINX ZYNQMP DPDMA DRIVER
> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> index 118528295fb7..846f10317bba 100644
> --- a/drivers/dma/xilinx/xdma.c
> +++ b/drivers/dma/xilinx/xdma.c
> @@ -25,6 +25,7 @@
> #include <linux/dmapool.h>
> #include <linux/regmap.h>
> #include <linux/dmaengine.h>
> +#include <linux/dma/amd_xdma.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/amd_xdma.h>
> #include <linux/dma-mapping.h>
> @@ -713,6 +714,7 @@ static int xdma_set_vector_reg(struct xdma_device *xdev, u32 vec_tbl_start,
> static int xdma_irq_init(struct xdma_device *xdev)
> {
> u32 irq = xdev->irq_start;
> + u32 user_irq_start;
> int i, j, ret;
>
> /* return failure if there are not enough IRQs */
> @@ -755,6 +757,18 @@ static int xdma_irq_init(struct xdma_device *xdev)
> goto failed_init_c2h;
> }
>
> + /* config user IRQ registers if needed */
> + user_irq_start = XDMA_CHAN_NUM(xdev);
> + if (xdev->irq_num > user_irq_start) {
> + ret = xdma_set_vector_reg(xdev, XDMA_IRQ_USER_VEC_NUM,
> + user_irq_start,
> + xdev->irq_num - user_irq_start);
> + if (ret) {
> + xdma_err(xdev, "failed to set user vectors: %d", ret);
> + goto failed_init_c2h;
> + }
> + }
> +
> /* enable interrupt */
> ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_CHAN_INT_EN_W1S, ~0);
> if (ret)
> @@ -780,6 +794,77 @@ static bool xdma_filter_fn(struct dma_chan *chan, void *param)
> return chan_info->dir == xdma_chan->dir;
> }
>
> +/**
> + * xdma_disable_user_irq - Disable user interrupt
> + * @pdev: Pointer to the platform_device structure
> + * @irq_num: System IRQ number
> + */
> +void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num)
> +{
> + struct xdma_device *xdev = platform_get_drvdata(pdev);
> + u32 user_irq_index;
> +
> + user_irq_index = irq_num - xdev->irq_start;
> + if (user_irq_index < XDMA_CHAN_NUM(xdev) ||
> + user_irq_index >= xdev->irq_num) {
> + xdma_err(xdev, "invalid user irq number");
> + return;
> + }
> + user_irq_index -= XDMA_CHAN_NUM(xdev);
> +
> + xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1C,
> + (1 << user_irq_index));
> +}
> +EXPORT_SYMBOL(xdma_disable_user_irq);
> +
> +/**
> + * xdma_enable_user_irq - Enable user logic interrupt
> + * @pdev: Pointer to the platform_device structure
> + * @irq_num: System IRQ number
> + */
> +int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num)
> +{
> + struct xdma_device *xdev = platform_get_drvdata(pdev);
> + u32 user_irq_index;
> + int ret;
> +
> + user_irq_index = irq_num - xdev->irq_start;
> + if (user_irq_index < XDMA_CHAN_NUM(xdev) ||
> + user_irq_index >= xdev->irq_num) {
> + xdma_err(xdev, "invalid user irq number");
> + return -EINVAL;
> + }
> + user_irq_index -= XDMA_CHAN_NUM(xdev);
> +
> + ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1S,
> + (1 << user_irq_index));
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(xdma_enable_user_irq);
> +
> +/**
> + * xdma_get_user_irq - Get system IRQ number
> + * @pdev: Pointer to the platform_device structure
> + * @user_irq_index: User logic IRQ wire index
> + *
> + * Return: The system IRQ number allocated for the given wire index.
> + */
> +int xdma_get_user_irq(struct platform_device *pdev, u32 user_irq_index)
> +{
> + struct xdma_device *xdev = platform_get_drvdata(pdev);
> +
> + if (XDMA_CHAN_NUM(xdev) + user_irq_index >= xdev->irq_num) {
> + xdma_err(xdev, "invalid user irq index");
> + return -EINVAL;
> + }
> +
> + return xdev->irq_start + XDMA_CHAN_NUM(xdev) + user_irq_index;
> +}
> +EXPORT_SYMBOL(xdma_get_user_irq);
> +
> /**
> * xdma_remove - Driver remove function
> * @pdev: Pointer to the platform_device structure
> diff --git a/include/linux/dma/amd_xdma.h b/include/linux/dma/amd_xdma.h
> new file mode 100644
> index 000000000000..ceba69ed7cb4
> --- /dev/null
> +++ b/include/linux/dma/amd_xdma.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2022, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _DMAENGINE_AMD_XDMA_H
> +#define _DMAENGINE_AMD_XDMA_H
> +
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +
> +int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num);
> +void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num);
> +int xdma_get_user_irq(struct platform_device *pdev, u32 user_irq_index);

who is the user of these APIs? It is not clear to me how this is to be
used...


> +
> +#endif /* _DMAENGINE_AMD_XDMA_H */
> --
> 2.27.0

--
~Vinod

2023-01-18 15:44:37

by Martin Tůma

[permalink] [raw]
Subject: Re: [RESEND PATCH V11 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

>> +++ b/include/linux/dma/amd_xdma.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2022, Advanced Micro Devices, Inc.
>> + */
>> +
>> +#ifndef _DMAENGINE_AMD_XDMA_H
>> +#define _DMAENGINE_AMD_XDMA_H
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +
>> +int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num);
>> +void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num);
>> +int xdma_get_user_irq(struct platform_device *pdev, u32 user_irq_index);
>
> who is the user of these APIs? It is not clear to me how this is to be
> used...
>

The APIs are used by the PCIe card devices/drivers that use XDMA.
Without them the "user IRQs" provided by the XDMA HW can not be used by
the PCIe card drivers. If you look at the XDMA HW overview:
https://docs.xilinx.com/r/en-US/pg195-pcie-dma/Overview?tocId=O_EMX26J5IsdubL4i3XJ_w
those APIs control the "IRQ module" block.

For a linux driver using them see the mgb4 v4l2 driver:
https://patchwork.kernel.org/project/linux-media/patch/[email protected]/

M.

2023-01-18 16:27:16

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [RESEND PATCH V11 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

On 1/18/23 07:28, Martin Tůma wrote:
>>> +++ b/include/linux/dma/amd_xdma.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +/*
>>> + * Copyright (C) 2022, Advanced Micro Devices, Inc.
>>> + */
>>> +
>>> +#ifndef _DMAENGINE_AMD_XDMA_H
>>> +#define _DMAENGINE_AMD_XDMA_H
>>> +
>>> +#include <linux/interrupt.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num);
>>> +void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num);
>>> +int xdma_get_user_irq(struct platform_device *pdev, u32
>>> user_irq_index);
>>
>> who is the user of these APIs? It is not clear to me how this is to be
>> used...
>>
>
> The APIs are used by the PCIe card devices/drivers that use XDMA.
> Without them the "user IRQs" provided by the XDMA HW can not be used
> by the PCIe card drivers. If you look at the XDMA HW overview:
> https://docs.xilinx.com/r/en-US/pg195-pcie-dma/Overview?tocId=O_EMX26J5IsdubL4i3XJ_w
>
> those APIs control the "IRQ module" block.
>
> For a linux driver using them see the mgb4 v4l2 driver:
> https://patchwork.kernel.org/project/linux-media/patch/[email protected]/


Rather than custom functions this should probably be integrated with the
generic IRQ framework registering a IRQ chip that provides IRQs that can
be requested by the consumer.

2023-01-18 17:28:57

by Martin Tůma

[permalink] [raw]
Subject: Re: [RESEND PATCH V11 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support

On 18. 01. 23 16:33, Lars-Peter Clausen wrote:
> On 1/18/23 07:28, Martin Tůma wrote:
>>>> +++ b/include/linux/dma/amd_xdma.h
>>>> @@ -0,0 +1,16 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Copyright (C) 2022, Advanced Micro Devices, Inc.
>>>> + */
>>>> +
>>>> +#ifndef _DMAENGINE_AMD_XDMA_H
>>>> +#define _DMAENGINE_AMD_XDMA_H
>>>> +
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +int xdma_enable_user_irq(struct platform_device *pdev, u32 irq_num);
>>>> +void xdma_disable_user_irq(struct platform_device *pdev, u32 irq_num);
>>>> +int xdma_get_user_irq(struct platform_device *pdev, u32
>>>> user_irq_index);
>>>
>>> who is the user of these APIs? It is not clear to me how this is to be
>>> used...
>>>
>>
>> The APIs are used by the PCIe card devices/drivers that use XDMA.
>> Without them the "user IRQs" provided by the XDMA HW can not be used
>> by the PCIe card drivers. If you look at the XDMA HW overview:
>> https://docs.xilinx.com/r/en-US/pg195-pcie-dma/Overview?tocId=O_EMX26J5IsdubL4i3XJ_w
>> those APIs control the "IRQ module" block.
>>
>> For a linux driver using them see the mgb4 v4l2 driver:
>> https://patchwork.kernel.org/project/linux-media/patch/[email protected]/
>
>
> Rather than custom functions this should probably be integrated with the
> generic IRQ framework registering a IRQ chip that provides IRQs that can
> be requested by the consumer.
>

I'm not sure if this makes sense. Those IRQs are "special" as they
control the user logic (there are additional IRQs in XDMA used by the
DMA engine itself). Only the "user IRQs" have this control logic but all
the IRQs are available as standard MSI/MSI-X IRQs in the kernel. There
is also the problem with the IRQ numbering as the XDMA HW can be
configured with a variable number of total IRQs and "user IRQ #X" can be
a different MSI/MSI-X IRQ depending on the number of configured DMA
channels. This API ensures that you get the correct user logic "wire"
independent of the XDMA configuration you have.

Finally, I'm not sure how hard or even if possible it is to integrate
the special handling of some MSI/MSI-X IRQs into the generic IRQ
framework as the "IRQ chip" will be some generic PCIe IRQ chip
(depending on the mainboard). But maybe that's not a problem.

M.