Re: [PATCH V5 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support
From: Lizhi Hou
Date: Tue Oct 04 2022 - 12:25:36 EST
On 10/4/22 03:26, Martin Tůma wrote:
On 03. 10. 22 17:52, Lizhi Hou wrote:
On 10/3/22 06:59, Martin Tůma wrote:
On 29. 09. 22 1:58, 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 <lizhi.hou@xxxxxxx>
Signed-off-by: Sonal Santan <sonal.santan@xxxxxxx>
Signed-off-by: Max Zhen <max.zhen@xxxxxxx>
Signed-off-by: Brian Xu <brian.xu@xxxxxxx>
---
MAINTAINERS | 1 +
drivers/dma/xilinx/xdma.c | 65
++++++++++++++++++++++++++++++++++++
include/linux/dma/amd_xdma.h | 16 +++++++++
3 files changed, 82 insertions(+)
create mode 100644 include/linux/dma/amd_xdma.h
diff --git a/MAINTAINERS b/MAINTAINERS
index c1be0b2e378a..019d84b2b086 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21691,6 +21691,7 @@ L: dmaengine@xxxxxxxxxxxxxxx
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 58a57e03bef5..13f627445c4a 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>
@@ -736,6 +737,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 */
@@ -786,6 +788,18 @@ static int xdma_irq_init(struct xdma_device
*xdev)
goto failed;
}
+ /* config user IRQ registers if needed */
+ user_irq_start = xdev->h2c_chan_num + xdev->c2h_chan_num;
+ 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;
+ }
+ }
+
/* enable interrupt */
ret = xdma_enable_intr(xdev);
if (ret) {
@@ -816,6 +830,57 @@ static bool xdma_filter_fn(struct dma_chan
*chan, void *param)
return true;
}
+/**
+ * xdma_disable_user_irq - Disable user interrupt
+ * @pdev: Pointer to the platform_device structure
+ * @user_irq_index: User IRQ index
+ */
+void xdma_disable_user_irq(struct platform_device *pdev, u32
user_irq_index)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+
+ if (xdev->h2c_chan_num + xdev->c2h_chan_num + user_irq_index >=
+ xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq index");
+ return;
+ }
+
+ 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
+ * @user_irq_index: User logic IRQ wire index
+ * @irq: Pointer to returning system IRQ number
+ */
+int xdma_enable_user_irq(struct platform_device *pdev, u32
user_irq_index,
+ u32 *irq)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+ u32 user_irq;
+ int ret;
+
+ user_irq = xdev->h2c_chan_num + xdev->c2h_chan_num +
user_irq_index;
+ if (user_irq >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq index");
+ return -EINVAL;
+ }
+
+ ret = xdma_write_reg(xdev, XDMA_IRQ_BASE,
XDMA_IRQ_USER_INT_EN_W1S,
+ (1 << user_irq_index));
+ if (ret) {
+ xdma_err(xdev, "set user irq mask failed, %d", ret);
+ return ret;
+ }
+ *irq = user_irq + xdev->irq_start;
+
+ return 0;
+}
+EXPORT_SYMBOL(xdma_enable_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..91fb02ff93a7
--- /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
user_irq_index,
+ u32 *irq);
+void xdma_disable_user_irq(struct platform_device *pdev, u32
user_irq_index);
+
+#endif /* _DMAENGINE_AMD_XDMA_H */
Hi,
While rewriting our V4L2 driver to use your XDMA driver, I realized,
that the API for the user interrupts is still not fully usable. If
the expected outcome is that the "parent" driver using the xdma
driver knows nothing about the IRQ allocation in XDMA, then the
xdma_enable_user_irq() function must be split into two separate
functions:
int xdma_enable_user_irq(struct platform_device *pdev, u32
user_irq_index)
and
int xdma_get_system_irq(u32 user_irq_index)
that returns the system IRQ number for the given user_irq_index.
Because without it you can not get the system IRQ number without
enabling the irq at the same time which is usually not what you want.
Is this because the user logic you are using does not have
disable/enable interrupt by itself? I thought that the user logic IP
would have its own register to enable/disable interrupt.
Yes. Our HW has no "own" registers to disable the IRQs. As such
registers are already there in XDMA, there was no need for them.
And xdma_enable_user_irq() will only be called once for creating the
user logic platform device. Then, user logic IP driver would use its
own register to control interrupts. The existing platform driver does
not call any xdma_* to control interrupt.
As you can see, there is HW that was designed to use the XDMA
registers during normal operation and separate enable/disable and irq
mapping functions are required. Maybe it is not the best HW design,
but it can evidently happen and then the driver author is screwed with
the merged API.
Ok. To support this case, I will separate the API as you mentioned.
Thanks,
Lizhi
M.
Thanks,
Lizhi
As a workaround, you can compute the system IRQ yourself with the
knowledge of the XDMA internals (this is what I have to do at the
moment in our driver), but then the "u32 *irq" parameter becomes
useless and can be removed anyway.
M.