Skip to contentSkip to conversation

Add AMD MDB Endpoint and non-LL mode Support

PATCH In review Devendra K Verma 2 participants 12 messages Sep 5, 2025 – Sep 12, 2025

Subsystems (3)
PCI dmaengine DesignWare edma core IP driver
lore
Share & tools
lore

Devendra K Verma

author
#0 lore raw

cover Add AMD MDB Endpoint and non-LL mode Support

This series of patch support the following:

- AMD MDB Endpoint Support, as part of this patch following are added:
o AMD supported device ID and vendor ID (Xilinx)
o AMD MDB specific driver data
o AMD specific VSEC capabilities to retrieve the base of phys address of MDB side DDR

- Addition of non-LL mode
o The IP supported non-LL mode functions
o Flexibility to choose non-LL mode via dma_slave_config param peripheral_config, by the client
o Allow IP utilization if LL mode is not available

Devendra K Verma (2):
dmaengine: dw-edma: Add AMD MDB Endpoint Support
dmaengine: dw-edma: Add non-LL mode

 drivers/dma/dw-edma/dw-edma-core.c    |  38 ++++++++++--
 drivers/dma/dw-edma/dw-edma-core.h    |   1 +
 drivers/dma/dw-edma/dw-edma-pcie.c    | 112 +++++++++++++++++++++++++++++++---
 drivers/dma/dw-edma/dw-hdma-v0-core.c |  62 ++++++++++++++++++-
 include/linux/dma/edma.h              |   1 +
 include/linux/pci_ids.h               |   1 +
 6 files changed, 201 insertions(+), 14 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 1/2 dmaengine: dw-edma: Add AMD MDB Endpoint Support

AMD MDB PCIe endpoint support. For AMD specific support added the following
- AMD supported PCIe Device IDs and Vendor ID (Xilinx).
- AMD MDB specific driver data
- AMD MDB specific VSEC capability to retrieve the device DDR base address.

Signed-off-by Devendra K Verma <[email protected]>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 83 +++++++++++++++++++++++++++++++++++++-
 include/linux/pci_ids.h            |  1 +
 2 files changed, 82 insertions(+), 2 deletions(-)
drivers/dma/dw-edma/dw-edma-pcie.c +81 -2
index 3371e0a7..749067b 100644
@@ -18,10 +18,12 @@
 #include "dw-edma-core.h"
 
 #define DW_PCIE_VSEC_DMA_ID			0x6
+#define DW_PCIE_AMD_MDB_VSEC_ID			0x20
 #define DW_PCIE_VSEC_DMA_BAR			GENMASK(10, 8)
 #define DW_PCIE_VSEC_DMA_MAP			GENMASK(2, 0)
 #define DW_PCIE_VSEC_DMA_WR_CH			GENMASK(9, 0)
 #define DW_PCIE_VSEC_DMA_RD_CH			GENMASK(25, 16)
+#define DW_PCIE_AMD_MDB_INVALID_ADDR		(~0ULL)
 
 #define DW_BLOCK(a, b, c) \
 	{ \

@@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
 	u8				irqs;
 	u16				wr_ch_cnt;
 	u16				rd_ch_cnt;
+	u64				phys_addr;
 };
 
 static const struct dw_edma_pcie_data snps_edda_data = {

@@ -90,6 +93,44 @@ struct dw_edma_pcie_data {
 	.rd_ch_cnt			= 2,
 };
 
+static const struct dw_edma_pcie_data amd_mdb_data = {
+	/* MDB registers location */
+	.rg.bar				= BAR_0,
+	.rg.off				= 0x00001000,	/*  4 Kbytes */
+	.rg.sz				= 0x00002000,	/*  8 Kbytes */
+	/* MDB memory linked list location */
+	.ll_wr = {
+		/* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
+	},
+	.ll_rd = {
+		/* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
+	},
+	/* MDB memory data location */
+	.dt_wr = {
+		/* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
+	},
+	.dt_rd = {
+		/* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
+	},
+	/* Other */
+	.mf				= EDMA_MF_HDMA_NATIVE,
+	.irqs				= 1,
+	.wr_ch_cnt			= 2,
+	.rd_ch_cnt			= 2,
+};
+
 static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
 {
 	return pci_irq_vector(to_pci_dev(dev), nr);

@@ -120,9 +161,14 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	u32 val, map;
 	u16 vsec;
 	u64 off;
+	u16 vendor;
 
-	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
-					DW_PCIE_VSEC_DMA_ID);
+	vendor = pdev->vendor;
+	if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
+	    vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor, DW_PCIE_VSEC_DMA_ID);
 	if (!vsec)
 		return;
 

@@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	off <<= 32;
 	off |= val;
 	pdata->rg.off = off;
+
+	/* AMD specific VSEC capability */
+	if (vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor,
+					DW_PCIE_AMD_MDB_VSEC_ID);
+	if (!vsec)
+		return;
+
+	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
+	if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
+	    PCI_VNDR_HEADER_REV(val) != 0x1)
+		return;
+
+	pci_read_config_dword(pdev, vsec + 0xc, &val);
+	off = val;
+	pci_read_config_dword(pdev, vsec + 0x8, &val);
+	off <<= 32;
+	off |= val;
+	pdata->phys_addr = off;
 }
 
 static int dw_edma_pcie_probe(struct pci_dev *pdev,

@@ -179,6 +246,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	}
 
 	memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
+	vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;
 
 	/*
 	 * Tries to find if exists a PCIe Vendor-Specific Extended Capability

@@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	 */
 	dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
 
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
+		/*
+		 * There is no valid address found for the LL memory
+		 * space on the device side.
+		 */
+		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
+			return -EINVAL;
+	}
+
 	/* Mapping PCI BAR regions */
 	mask = BIT(vsec_data->rg.bar);
 	for (i = 0; i < vsec_data->wr_ch_cnt; i++) {

@@ -367,6 +444,8 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
 
 static const struct pci_device_id dw_edma_pcie_id_table[] = {
 	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+	{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
+	  (kernel_ulong_t)&amd_mdb_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
include/linux/pci_ids.h +1 -0
index 92ffc43..c15607d 100644
@@ -636,6 +636,7 @@
 #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS		0x780b
 #define PCI_DEVICE_ID_AMD_HUDSON2_IDE		0x780c
 #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
+#define PCI_DEVICE_ID_AMD_MDB_B054	0xb054
 
 #define PCI_VENDOR_ID_TRIDENT		0x1023
 #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX	0x2000
Signature · 1 line
1.8.3.1
#2 lore raw

Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

Quoted 87 lines · Devendra K Verma

AMD MDB PCIe endpoint support. For AMD specific support added the following
- AMD supported PCIe Device IDs and Vendor ID (Xilinx).
- AMD MDB specific driver data
- AMD MDB specific VSEC capability to retrieve the device DDR base address.

Signed-off-by Devendra K Verma <[email protected]>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 83 +++++++++++++++++++++++++++++++++++++-
 include/linux/pci_ids.h            |  1 +
 2 files changed, 82 insertions(+), 2 deletions(-)
drivers/dma/dw-edma/dw-edma-pcie.c +41 -0
index 3371e0a7..749067b 100644
@@ -18,10 +18,12 @@
 #include "dw-edma-core.h"
 
 #define DW_PCIE_VSEC_DMA_ID			0x6
+#define DW_PCIE_AMD_MDB_VSEC_ID			0x20
 #define DW_PCIE_VSEC_DMA_BAR			GENMASK(10, 8)
 #define DW_PCIE_VSEC_DMA_MAP			GENMASK(2, 0)
 #define DW_PCIE_VSEC_DMA_WR_CH			GENMASK(9, 0)
 #define DW_PCIE_VSEC_DMA_RD_CH			GENMASK(25, 16)
+#define DW_PCIE_AMD_MDB_INVALID_ADDR		(~0ULL)
 
 #define DW_BLOCK(a, b, c) \
 	{ \

@@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
 	u8				irqs;
 	u16				wr_ch_cnt;
 	u16				rd_ch_cnt;
+	u64				phys_addr;
 };
 
 static const struct dw_edma_pcie_data snps_edda_data = {

@@ -90,6 +93,44 @@ struct dw_edma_pcie_data {
 	.rd_ch_cnt			= 2,
 };
 
+static const struct dw_edma_pcie_data amd_mdb_data = {
+	/* MDB registers location */
+	.rg.bar				= BAR_0,
+	.rg.off				= 0x00001000,	/*  4 Kbytes */
+	.rg.sz				= 0x00002000,	/*  8 Kbytes */
+	/* MDB memory linked list location */
+	.ll_wr = {
+		/* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
+	},
+	.ll_rd = {
+		/* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
+	},
+	/* MDB memory data location */
+	.dt_wr = {
+		/* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
+	},
+	.dt_rd = {
+		/* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
+	},
+	/* Other */
+	.mf				= EDMA_MF_HDMA_NATIVE,
+	.irqs				= 1,
+	.wr_ch_cnt			= 2,
+	.rd_ch_cnt			= 2,
+};
+
 static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
 {
 	return pci_irq_vector(to_pci_dev(dev), nr);

@@ -120,9 +161,14 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	u32 val, map;
 	u16 vsec;
 	u64 off;

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 171

Quoted 10 lines
drivers/dma/dw-edma/dw-edma-pcie.c +7 -2
+	u16 vendor;
 
-	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
-					DW_PCIE_VSEC_DMA_ID);
+	vendor = pdev->vendor;
+	if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
+	    vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor, DW_PCIE_VSEC_DMA_ID);

The vendor of a device assigns VSEC IDs and determines what each ID means, so the semantics of a VSEC Capability are determined by the tuple of (device Vendor ID, VSEC ID), where the Vendor ID is the value at 0x00 in config space.

This code assumes that Synopsys and Xilinx agreed on the same VSEC ID
(6) and semantics of that Capability.

I assume you know this is true in this particular case, but it is not safe for in general because even if other vendors incorporate this same IP into their devices, they may choose different VSEC IDs because they may have already assigned the VSEC ID 6 for something else.

So you should add a comment to the effect that "Synopsys and Xilinx happened to use the same VSEC ID and semantics. This may vary for other vendors."

The DVSEC Capability is a more generic solution to this problem. The VSEC ID namespace is determined by the Vendor ID of the *device*.

By contrast, the DVSEC ID namespace is determined by a Vendor ID in the DVSEC Capability itself, not by the Vendor ID of the device.

So AMD could define a DVSEC ID, e.g., 6, and define the semantics of that DVSEC. Then devices from *any* vendor could include a DVSEC Capability with (PCI_VENDOR_ID_AMD, 6), and generic code could look for that DVSEC independent of what is at 0x00 in config space.

Quoted 4 lines
drivers/dma/dw-edma/dw-edma-pcie.c +0 -0
 	if (!vsec)
 		return;

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 210

Quoted 10 lines
drivers/dma/dw-edma/dw-edma-pcie.c +7 -0
@@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	off <<= 32;
 	off |= val;
 	pdata->rg.off = off;
+
+	/* AMD specific VSEC capability */
+	if (vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor,
+					DW_PCIE_AMD_MDB_VSEC_ID);

pci_find_vsec_capability() finds a Vendor-Specific Extended Capability defined by the vendor of the device (Xilinx in this case).

pci_find_vsec_capability() already checks that dev->vendor matches the vendor argument so you don't need the "vendor != PCI_VENDOR_ID_XILINX" check above.

DW_PCIE_AMD_MDB_VSEC_ID should include "XILINX" because this ID is in the namespace created by PCI_VENDOR_ID_XILINX, i.e., it's somewhere in the (PCI_VENDOR_ID_XILINX, x) space.

This code should look something like this (you could add "MDB" or something if it makes sense):

  #define PCI_VSEC_ID_XILINX_DMA_DATA               0x20

vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,

                                  PCI_VSEC_ID_XILINX_DMA_DATA);
Quoted 55 lines
drivers/dma/dw-edma/dw-edma-pcie.c +26 -0
+	if (!vsec)
+		return;
+
+	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
+	if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
+	    PCI_VNDR_HEADER_REV(val) != 0x1)
+		return;
+
+	pci_read_config_dword(pdev, vsec + 0xc, &val);
+	off = val;
+	pci_read_config_dword(pdev, vsec + 0x8, &val);
+	off <<= 32;
+	off |= val;
+	pdata->phys_addr = off;
 }
 
 static int dw_edma_pcie_probe(struct pci_dev *pdev,

@@ -179,6 +246,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	}
 
 	memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
+	vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;
 
 	/*
 	 * Tries to find if exists a PCIe Vendor-Specific Extended Capability

@@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	 */
 	dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
 
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
+		/*
+		 * There is no valid address found for the LL memory
+		 * space on the device side.
+		 */
+		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
+			return -EINVAL;
+	}
+
 	/* Mapping PCI BAR regions */
 	mask = BIT(vsec_data->rg.bar);
 	for (i = 0; i < vsec_data->wr_ch_cnt; i++) {

@@ -367,6 +444,8 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
 
 static const struct pci_device_id dw_edma_pcie_id_table[] = {
 	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+	{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
+	  (kernel_ulong_t)&amd_mdb_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
include/linux/pci_ids.h +0 -0
index 92ffc43..c15607d 100644

include/linux/pci_ids.h line 639

Quoted 4 lines
include/linux/pci_ids.h +1 -0
@@ -636,6 +636,7 @@
 #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS		0x780b
 #define PCI_DEVICE_ID_AMD_HUDSON2_IDE		0x780c
 #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
+#define PCI_DEVICE_ID_AMD_MDB_B054	0xb054

Unless PCI_DEVICE_ID_AMD_MDB_B054 is used in more than one driver, move the #define into the file that uses it. See the note at the top of pci_ids.h.

author
#3 lore raw

RE: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you, Bjorn for reviewing the code!
Please check my response inline.

Regards,
Devendra

Quoted 105 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Friday, September 5, 2025 22:24
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Quoted 10 lines · Devendra K Verma

AMD MDB PCIe endpoint support. For AMD specific support added the following
- AMD supported PCIe Device IDs and Vendor ID (Xilinx).
- AMD MDB specific driver data
- AMD MDB specific VSEC capability to retrieve the device DDR base address.

Signed-off-by Devendra K Verma <[email protected]>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 83

+++++++++++++++++++++++++++++++++++++-

Quoted 80 lines
 include/linux/pci_ids.h            |  1 +
 2 files changed, 82 insertions(+), 2 deletions(-)
drivers/dma/dw-edma/dw-edma-pcie.c +42 -0
index 3371e0a7..749067b 100644
@@ -18,10 +18,12 @@
 #include "dw-edma-core.h"
 
 #define DW_PCIE_VSEC_DMA_ID                  0x6
+#define DW_PCIE_AMD_MDB_VSEC_ID                      0x20
 #define DW_PCIE_VSEC_DMA_BAR                 GENMASK(10, 8)
 #define DW_PCIE_VSEC_DMA_MAP                 GENMASK(2, 0)
 #define DW_PCIE_VSEC_DMA_WR_CH                       GENMASK(9, 0)
 #define DW_PCIE_VSEC_DMA_RD_CH                       GENMASK(25, 16)
+#define DW_PCIE_AMD_MDB_INVALID_ADDR         (~0ULL)
 
 #define DW_BLOCK(a, b, c) \
      { \

@@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
      u8                              irqs;
      u16                             wr_ch_cnt;
      u16                             rd_ch_cnt;
+     u64                             phys_addr;
 };
 
 static const struct dw_edma_pcie_data snps_edda_data = { @@ -90,6
+93,44 @@ struct dw_edma_pcie_data {
      .rd_ch_cnt                      = 2,
 };
 
+static const struct dw_edma_pcie_data amd_mdb_data = {
+     /* MDB registers location */
+     .rg.bar                         = BAR_0,
+     .rg.off                         = 0x00001000,   /*  4 Kbytes */
+     .rg.sz                          = 0x00002000,   /*  8 Kbytes */
+     /* MDB memory linked list location */
+     .ll_wr = {
+             /* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
+             /* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
+     },
+     .ll_rd = {
+             /* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
+             /* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
+     },
+     /* MDB memory data location */
+     .dt_wr = {
+             /* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
+             /* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
+     },
+     .dt_rd = {
+             /* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
+             /* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
+             DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
+     },
+     /* Other */
+     .mf                             = EDMA_MF_HDMA_NATIVE,
+     .irqs                           = 1,
+     .wr_ch_cnt                      = 2,
+     .rd_ch_cnt                      = 2,
+};
+
 static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int

nr)  {
      return pci_irq_vector(to_pci_dev(dev), nr); @@ -120,9 +161,14 @@
static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
      u32 val, map;
      u16 vsec;
      u64 off;

+ u16 vendor;

Quoted 14 lines

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,

Quoted 9 lines
drivers/dma/dw-edma/dw-edma-pcie.c +7 -2
-     vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
-                                     DW_PCIE_VSEC_DMA_ID);
+     vendor = pdev->vendor;
+     if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
+         vendor != PCI_VENDOR_ID_XILINX)
+             return;
+
+     vsec = pci_find_vsec_capability(pdev, vendor,
+ DW_PCIE_VSEC_DMA_ID);

The vendor of a device assigns VSEC IDs and determines what each ID means, so the semantics of a VSEC Capability are determined by the tuple of (device Vendor ID, VSEC ID), where the Vendor ID is the value at 0x00 in config space.

As AMD is a vendor for this device, it is determined as VSEC capability to support some of the functionality not supported by the other vendor Synopsys.

Quoted 11 lines

This code assumes that Synopsys and Xilinx agreed on the same VSEC ID
(6) and semantics of that Capability.

I assume you know this is true in this particular case, but it is not safe for in general because even if other vendors incorporate this same IP into their devices, they may choose different VSEC IDs because they may have already assigned the VSEC ID 6 for something else.

So you should add a comment to the effect that "Synopsys and Xilinx happened to use the same VSEC ID and semantics. This may vary for other vendors."

Agree with the above suggestion. Will add this in next review.

Quoted 11 lines

The DVSEC Capability is a more generic solution to this problem. The VSEC ID namespace is determined by the Vendor ID of the *device*.

By contrast, the DVSEC ID namespace is determined by a Vendor ID in the DVSEC Capability itself, not by the Vendor ID of the device.

So AMD could define a DVSEC ID, e.g., 6, and define the semantics of that DVSEC. Then devices from *any* vendor could include a DVSEC Capability with (PCI_VENDOR_ID_AMD, 6), and generic code could look for that DVSEC independent of what is at 0x00 in config space.

As AMD itself becomes the vendor for this device, VSEC capability is chosen to support the functionality missing in the code.

Quoted 23 lines

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct

Quoted 4 lines
      if (!vsec)
              return;
drivers/dma/dw-edma/dw-edma-pcie.c +0 -0
@@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct

pci_dev *pdev,

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 210

Quoted 10 lines
drivers/dma/dw-edma/dw-edma-pcie.c +7 -0
      off <<= 32;
      off |= val;
      pdata->rg.off = off;
+
+     /* AMD specific VSEC capability */
+     if (vendor != PCI_VENDOR_ID_XILINX)
+             return;
+
+     vsec = pci_find_vsec_capability(pdev, vendor,
+                                     DW_PCIE_AMD_MDB_VSEC_ID);

pci_find_vsec_capability() finds a Vendor-Specific Extended Capability defined by the vendor of the device (Xilinx in this case).

pci_find_vsec_capability() already checks that dev->vendor matches the vendor argument so you don't need the "vendor != PCI_VENDOR_ID_XILINX" check above.

Quoted 9 lines

DW_PCIE_AMD_MDB_VSEC_ID should include "XILINX" because this ID is in the namespace created by PCI_VENDOR_ID_XILINX, i.e., it's somewhere in the (PCI_VENDOR_ID_XILINX, x) space.

This code should look something like this (you could add "MDB" or something if it makes sense):

  #define PCI_VSEC_ID_XILINX_DMA_DATA               0x20

Thank you for the suggestion, will go with the DW_PCIE_XILINX_MDB_VSEC_ID similar to the format already been used in the code.

Quoted 58 lines

vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,

                                  PCI_VSEC_ID_XILINX_DMA_DATA);
Quoted 18 lines
drivers/dma/dw-edma/dw-edma-pcie.c +14 -0
+     if (!vsec)
+             return;
+
+     pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
+     if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
+         PCI_VNDR_HEADER_REV(val) != 0x1)
+             return;
+
+     pci_read_config_dword(pdev, vsec + 0xc, &val);
+     off = val;
+     pci_read_config_dword(pdev, vsec + 0x8, &val);
+     off <<= 32;
+     off |= val;
+     pdata->phys_addr = off;
 }
 
 static int dw_edma_pcie_probe(struct pci_dev *pdev, @@ -179,6 +246,7

@@ static int dw_edma_pcie_probe(struct pci_dev *pdev,

Quoted 8 lines
      }

      memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));

+ vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;

/*
* Tries to find if exists a PCIe Vendor-Specific Extended
Capability @@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev

*pdev,

Quoted 28 lines
       */
      dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
drivers/dma/dw-edma/dw-edma-pcie.c +9 -0
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
+             /*
+              * There is no valid address found for the LL memory
+              * space on the device side.
+              */
+             if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
+                     return -EINVAL;
+     }
+
      /* Mapping PCI BAR regions */
      mask = BIT(vsec_data->rg.bar);
      for (i = 0; i < vsec_data->wr_ch_cnt; i++) { @@ -367,6 +444,8 @@

static void dw_edma_pcie_remove(struct pci_dev *pdev)

static const struct pci_device_id dw_edma_pcie_id_table[] = {
{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },

drivers/dma/dw-edma/dw-edma-pcie.c +2 -0
+     { PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
+       (kernel_ulong_t)&amd_mdb_data },
      { }
 };
 MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table); diff --git

a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 92ffc43..c15607d 100644

include/linux/pci_ids.h +0 -0
Quoted 8 lines

include/linux/pci_ids.h line 639

Quoted 5 lines
include/linux/pci_ids.h +1 -0
@@ -636,6 +636,7 @@
 #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS              0x780b
 #define PCI_DEVICE_ID_AMD_HUDSON2_IDE                0x780c
 #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
+#define PCI_DEVICE_ID_AMD_MDB_B054   0xb054

Unless PCI_DEVICE_ID_AMD_MDB_B054 is used in more than one driver, move the #define into the file that uses it. See the note at the top of pci_ids.h.

Agreed. Will move this to the file where it is being used as no other driver is using it now.

#4 lore raw

Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

Quoted 1 line · Devendra
Quoted 1 line

From: Bjorn Helgaas <[email protected]>

[redundant quoted headers removed]

Quoted 24 lines
Quoted 24 lines · Devendra K Verma
Quoted 22 lines

AMD MDB PCIe endpoint support. For AMD specific support added the following
- AMD supported PCIe Device IDs and Vendor ID (Xilinx).
- AMD MDB specific driver data
- AMD MDB specific VSEC capability to retrieve the device DDR base address.

static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,

      u32 val, map;
      u16 vsec;
      u64 off;
drivers/dma/dw-edma/dw-edma-pcie.c +8 -2
+     u16 vendor;
 
-     vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
-                                     DW_PCIE_VSEC_DMA_ID);
+     vendor = pdev->vendor;
+     if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
+         vendor != PCI_VENDOR_ID_XILINX)
+             return;
+
+     vsec = pci_find_vsec_capability(pdev, vendor,
+ DW_PCIE_VSEC_DMA_ID);
Quoted 8 lines
Quoted 4 lines

The vendor of a device assigns VSEC IDs and determines what each ID means, so the semantics of a VSEC Capability are determined by the tuple of (device Vendor ID, VSEC ID), where the Vendor ID is the value at 0x00 in config space.

As AMD is a vendor for this device, it is determined as VSEC capability to support some of the functionality not supported by the other vendor Synopsys.

Based on your code, the vendor of this device (the value at 0x00 in config space) is either PCI_VENDOR_ID_SYNOPSYS or PCI_VENDOR_ID_XILINX. Whoever controls those Vendor IDs also controls the VSEC ID allocations for those Vendor IDs.

Quoted 8 lines
Quoted 8 lines

The DVSEC Capability is a more generic solution to this problem. The VSEC ID namespace is determined by the Vendor ID of the *device*.

By contrast, the DVSEC ID namespace is determined by a Vendor ID in the DVSEC Capability itself, not by the Vendor ID of the device.

Quoted 8 lines
Quoted 5 lines

So AMD could define a DVSEC ID, e.g., 6, and define the semantics of that DVSEC. Then devices from *any* vendor could include a DVSEC Capability with (PCI_VENDOR_ID_AMD, 6), and generic code could look for that DVSEC independent of what is at 0x00 in config space.

As AMD itself becomes the vendor for this device, VSEC capability is chosen to support the functionality missing in the code.

For VSEC, it doesn't matter who *sells* the device or who supplies IP contained in the device. What matters is the Vendor ID at 0x00 in config space.

Bjorn

author
#5 lore raw

PATCH 2/2 dmaengine: dw-edma: Add non-LL mode

In reply to Devendra K Verma

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:
- When a valid physical base address is not configured via the Xilinx VSEC capability then the IP can still be used in non-LL mode. The default mode for all the DMA transactions and for all the DMA channels then is non-LL mode.
- When a valid physical base address is configured but the client wants to use the non-LL mode for DMA transactions then also the flexibility is provided via the peripheral_config struct member of dma_slave_config. In this case the channels can be individually configured in non-LL mode. This use case is desirable for single DMA transfer of a chunk, this saves the effort of preparing the Link List.

Signed-off-by Devendra K Verma <[email protected]>
---
 drivers/dma/dw-edma/dw-edma-core.c    | 38 ++++++++++++++++++---
 drivers/dma/dw-edma/dw-edma-core.h    |  1 +
 drivers/dma/dw-edma/dw-edma-pcie.c    | 33 ++++++++++++++-----
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 62 ++++++++++++++++++++++++++++++++++-
 include/linux/dma/edma.h              |  1 +
 5 files changed, 121 insertions(+), 14 deletions(-)
drivers/dma/dw-edma/dw-edma-core.c +33 -5
index b43255f..dbef571 100644
@@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	int nollp = 0;
+
+	if (WARN_ON(config->peripheral_config &&
+		    config->peripheral_size != sizeof(int)))
+		return -EINVAL;
 
 	memcpy(&chan->config, config, sizeof(*config));
+
+	/*
+	 * When there is no valid LLP base address available
+	 * then the default DMA ops will use the non-LL mode.
+	 * Cases where LL mode is enabled and client wants
+	 * to use the non-LL mode then also client can do
+	 * so via the providing the peripheral_config param.
+	 */
+	if (config->peripheral_config)
+		nollp = *(int *)config->peripheral_config;
+
+	chan->nollp = false;
+	if (chan->dw->chip->nollp || (!chan->dw->chip->nollp && nollp))
+		chan->nollp = true;
+
 	chan->configured = true;
 
 	return 0;

@@ -353,7 +373,7 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(xfer->dchan);
 	enum dma_transfer_direction dir = xfer->direction;
 	struct scatterlist *sg = NULL;
-	struct dw_edma_chunk *chunk;
+	struct dw_edma_chunk *chunk = NULL;
 	struct dw_edma_burst *burst;
 	struct dw_edma_desc *desc;
 	u64 src_addr, dst_addr;

@@ -419,9 +439,11 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 	if (unlikely(!desc))
 		goto err_alloc;
 
-	chunk = dw_edma_alloc_chunk(desc);
-	if (unlikely(!chunk))
-		goto err_alloc;
+	if (!chan->nollp) {
+		chunk = dw_edma_alloc_chunk(desc);
+		if (unlikely(!chunk))
+			goto err_alloc;
+	}
 
 	if (xfer->type == EDMA_XFER_INTERLEAVED) {
 		src_addr = xfer->xfer.il->src_start;

@@ -450,7 +472,13 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 		if (xfer->type == EDMA_XFER_SCATTER_GATHER && !sg)
 			break;
 
-		if (chunk->bursts_alloc == chan->ll_max) {
+		/*
+		 * For non-LL mode, only a single burst can be handled
+		 * in a single chunk unlike LL mode where multiple bursts
+		 * can be configured in a single chunk.
+		 */
+		if ((chunk && chunk->bursts_alloc == chan->ll_max) ||
+		    chan->nollp) {
 			chunk = dw_edma_alloc_chunk(desc);
 			if (unlikely(!chunk))
 				goto err_alloc;
drivers/dma/dw-edma/dw-edma-core.h +1 -0
index 71894b9..2a4ad45 100644
@@ -86,6 +86,7 @@ struct dw_edma_chan {
 	u8				configured;
 
 	struct dma_slave_config		config;
+	bool				nollp;
 };
 
 struct dw_edma_irq {
drivers/dma/dw-edma/dw-edma-pcie.c +25 -8
index 749067b..0d6254f 100644
@@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	pdata->phys_addr = off;
 }
 
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+				 struct dw_edma_pcie_data *pdata,
+				 enum pci_barno bar)
+{
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+		return pdata->phys_addr;
+	return pci_bus_address(pdev, bar);
+}
+
 static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			      const struct pci_device_id *pid)
 {

@@ -233,6 +242,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	struct dw_edma_chip *chip;
 	int err, nr_irqs;
 	int i, mask;
+	bool nollp = false;
 
 	vsec_data = kmalloc(sizeof(*vsec_data), GFP_KERNEL);
 	if (!vsec_data)

@@ -257,10 +267,12 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
 		/*
 		 * There is no valid address found for the LL memory
-		 * space on the device side.
+		 * space on the device side. In the absence of LL base
+		 * address use the non-LL mode or simple mode supported by
+		 * the HDMA IP.
 		 */
 		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
-			return -EINVAL;
+			nollp = true;
 	}
 
 	/* Mapping PCI BAR regions */

@@ -308,6 +320,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->mf = vsec_data->mf;
 	chip->nr_irqs = nr_irqs;
 	chip->ops = &dw_edma_pcie_plat_ops;
+	chip->nollp = nollp;
 
 	chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
 	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;

@@ -316,7 +329,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	if (!chip->reg_base)
 		return -ENOMEM;
 
-	for (i = 0; i < chip->ll_wr_cnt; i++) {
+	for (i = 0; i < chip->ll_wr_cnt && !nollp; i++) {
 		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
 		struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
 		struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];

@@ -327,7 +340,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		ll_region->vaddr.io += ll_block->off;
-		ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
+		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 ll_block->bar);
 		ll_region->paddr += ll_block->off;
 		ll_region->sz = ll_block->sz;
 

@@ -336,12 +350,13 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		dt_region->vaddr.io += dt_block->off;
-		dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
+		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 dt_block->bar);
 		dt_region->paddr += dt_block->off;
 		dt_region->sz = dt_block->sz;
 	}
 
-	for (i = 0; i < chip->ll_rd_cnt; i++) {
+	for (i = 0; i < chip->ll_rd_cnt && !nollp; i++) {
 		struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
 		struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
 		struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];

@@ -352,7 +367,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		ll_region->vaddr.io += ll_block->off;
-		ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
+		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 ll_block->bar);
 		ll_region->paddr += ll_block->off;
 		ll_region->sz = ll_block->sz;
 

@@ -361,7 +377,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		dt_region->vaddr.io += dt_block->off;
-		dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
+		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 dt_block->bar);
 		dt_region->paddr += dt_block->off;
 		dt_region->sz = dt_block->sz;
 	}
drivers/dma/dw-edma/dw-hdma-v0-core.c +61 -1

74 lines not on this page · raw message

include/linux/dma/edma.h +1 -0
index 3080747..e14e16f 100644
@@ -99,6 +99,7 @@ struct dw_edma_chip {
 	enum dw_edma_map_format	mf;
 
 	struct dw_edma		*dw;
+	bool			nollp;
 };
 
 /* Export to the platform drivers */
Signature · 1 line
1.8.3.1
#6 lore raw

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Quoted 25 lines · Devendra K Verma

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:
- When a valid physical base address is not configured via the Xilinx VSEC capability then the IP can still be used in non-LL mode. The default mode for all the DMA transactions and for all the DMA channels then is non-LL mode.
- When a valid physical base address is configured but the client wants to use the non-LL mode for DMA transactions then also the flexibility is provided via the peripheral_config struct member of dma_slave_config. In this case the channels can be individually configured in non-LL mode. This use case is desirable for single DMA transfer of a chunk, this saves the effort of preparing the Link List.

+++ b/drivers/dma/dw-edma/dw-edma-core.c

drivers/dma/dw-edma/dw-edma-core.c +4 -0
@@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	int nollp = 0;
+
+	if (WARN_ON(config->peripheral_config &&
+		    config->peripheral_size != sizeof(int)))

drivers/dma/dw-edma/dw-edma-core.c static int dw_edma_device_config(struct dma_chan *dchan, · line 239

Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +8 -0
+		return -EINVAL;
 
 	memcpy(&chan->config, config, sizeof(*config));
+
+	/*
+	 * When there is no valid LLP base address available
+	 * then the default DMA ops will use the non-LL mode.
+	 * Cases where LL mode is enabled and client wants
+	 * to use the non-LL mode then also client can do
+	 * so via the providing the peripheral_config param.

s/via the/via/

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 12 lines
drivers/dma/dw-edma/dw-edma-pcie.c +8 -0
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c

@@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	pdata->phys_addr = off;
 }
 
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+				 struct dw_edma_pcie_data *pdata,
+				 enum pci_barno bar)
+{
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+		return pdata->phys_addr;
+	return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

Bjorn

author
#7 lore raw

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you Bjorn for reviewing the patch.
Please check my response inline.

Regards,
Devendra

Quoted 69 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Saturday, September 6, 2025 00:36
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Review static int dw_edma_device_config(struct dma_chan

Quoted 18 lines · Devendra K Verma

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:
- When a valid physical base address is not configured via the Xilinx VSEC capability then the IP can still be used in non-LL mode. The default mode for all the DMA transactions and for all the DMA channels then is non-LL mode.
- When a valid physical base address is configured but the client wants to use the non-LL mode for DMA transactions then also the flexibility is provided via the peripheral_config struct member of dma_slave_config. In this case the channels can be individually configured in non-LL mode. This use case is desirable for single DMA transfer of a chunk, this saves the effort of preparing the Link List.

+++ b/drivers/dma/dw-edma/dw-edma-core.c

(quoted diff) +0 -0
@@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan

*dchan,

Quoted 6 lines
drivers/dma/dw-edma/dw-edma-core.c +4 -0
                               struct dma_slave_config *config)  {
      struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+     int nollp = 0;
+
+     if (WARN_ON(config->peripheral_config &&
+                 config->peripheral_size != sizeof(int)))

drivers/dma/dw-edma/dw-edma-core.c static int dw_edma_device_config(struct dma_chan *dchan, · line 239

Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +8 -0
+             return -EINVAL;
 
      memcpy(&chan->config, config, sizeof(*config));
+
+     /*
+      * When there is no valid LLP base address available
+      * then the default DMA ops will use the non-LL mode.
+      * Cases where LL mode is enabled and client wants
+      * to use the non-LL mode then also client can do
+      * so via the providing the peripheral_config param.

s/via the/via/

Review static void dw_edma_pcie_get_vsec_dma_data(struct

Quoted 2 lines
(quoted diff) +1 -0
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c

@@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct

pci_dev *pdev,

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 9 lines
drivers/dma/dw-edma/dw-edma-pcie.c +6 -0
      pdata->phys_addr = off;
 }
 
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+                              struct dw_edma_pcie_data *pdata,
+                              enum pci_barno bar) {
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+             return pdata->phys_addr;
+     return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

Quoted 8 lines

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

Bjorn

In case of AMD, it is not an address that is accessible from host via PCI, it is the device side DDR offset of physical address which is not known to host,that is why the VSEC capability is used to let know host of the DDR offset to correctly programming the LLP of DMA controller. Without programming the LLP controller will not know from where to start reading the LL for DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

#8 lore raw

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Quoted 1 line · Devendra
Quoted 1 line

From: Bjorn Helgaas <[email protected]>

[redundant headers removed]

Quoted 25 lines
Quoted 17 lines · Devendra K Verma

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 11 lines

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:

drivers/dma/dw-edma/dw-edma-pcie.c +6 -0
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+                              struct dw_edma_pcie_data *pdata,
+                              enum pci_barno bar) {
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+             return pdata->phys_addr;
+     return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

Quoted 7 lines

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

Quoted 8 lines

In case of AMD, it is not an address that is accessible from host via PCI, it is the device side DDR offset of physical address which is not known to host,that is why the VSEC capability is used to let know host of the DDR offset to correctly programming the LLP of DMA controller. Without programming the LLP controller will not know from where to start reading the LL for DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

I guess "device side DDR offset" means this Xilinx device has some DDR internal to the PCI device, and the CPU cannot access it via a BAR?

But you need addresses inside that device DDR even though the CPU can't access it, and the VSEC gives you the base address of the DDR?

This makes me wonder about how dw_edma_region is used elsewhere because some of those places seem like they assume the CPU *can* access this area.

dw_pcie_edma_ll_alloc() uses dmam_alloc_coherent(), which allocates RAM and gives you a CPU virtual address (ll->vaddr.mem) and a DMA address (ll->paddr). dw_edma_pcie_probe() will later overwrite the ll->paddr with the DDR offset based on VSEC.

But it sounds like ll->vaddr.mem is useless for Xilinx devices since the CPU can't access the DDR?

If the CPU can't use ll->vaddr.mem, what happens in places like dw_hdma_v0_write_ll_data() where we access it?

Bjorn

author
#9 lore raw

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you Bjorn for your inputs!
Please check my comments inline.

Regards,
Devendra

Quoted 51 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Wednesday, September 10, 2025 23:27
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Quoted 1 line · Devendra
Quoted 1 line

From: Bjorn Helgaas <[email protected]>

[redundant headers removed]

Quoted 25 lines
Quoted 17 lines · Devendra K Verma

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 11 lines

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:

drivers/dma/dw-edma/dw-edma-pcie.c +6 -0
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+                              struct dw_edma_pcie_data *pdata,
+                              enum pci_barno bar) {
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+             return pdata->phys_addr;
+     return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

Quoted 7 lines

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

Quoted 7 lines

In case of AMD, it is not an address that is accessible from host via PCI, it is the device side DDR offset of physical address which is not known to host,that is why the VSEC capability is used to let know host of the DDR offset to correctly programming the LLP of DMA controller. Without programming the LLP controller will not know from where to start reading the LL for DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

Quoted 3 lines

I guess "device side DDR offset" means this Xilinx device has some DDR internal to the PCI device, and the CPU cannot access it via a BAR?

The host can access the DDR internal to the PCI device via BAR, but it involves an iATU translation. The host can use the virtual / physical address to access that DDR. The issue is not with the host rather DMA controller which does not understand the physical address provided by the host, eg, the address returned
by pci_bus_addr(pdev, barno).

Quoted 11 lines

But you need addresses inside that device DDR even though the CPU can't access it, and the VSEC gives you the base address of the DDR?

This makes me wonder about how dw_edma_region is used elsewhere because some of those places seem like they assume the CPU *can* access this area.

dw_pcie_edma_ll_alloc() uses dmam_alloc_coherent(), which allocates RAM and gives you a CPU virtual address (ll->vaddr.mem) and a DMA address (ll->paddr). dw_edma_pcie_probe() will later overwrite the
ll->paddr with the DDR offset based on VSEC.

Quoted 7 lines

But it sounds like ll->vaddr.mem is useless for Xilinx devices since the CPU can't access the DDR?

If the CPU can't use ll->vaddr.mem, what happens in places like dw_hdma_v0_write_ll_data() where we access it?

Bjorn

As explained above, the host can access the device side internal DDR via one of the BARs but involves some translation (iATU) configuration on the device side. The issue is with the DMA controller which does not understand the host side virt / phys addresses thus need to be programmed correctly to the device side DDR offset which is a valid physical address on the device side. Host needs the Information of the device side DDR offset to program the LLP register using which DMA controller starts reading the DDR on its side.
Hope this clarifies.

#10 lore raw

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Quoted 45 lines · Devendra
Quoted 41 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>

Quoted 1 line · Devendra
Quoted 1 line

From: Bjorn Helgaas <[email protected]>

[redundant headers removed]

Quoted 25 lines
Quoted 17 lines · Devendra K Verma

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 11 lines

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:

drivers/dma/dw-edma/dw-edma-pcie.c +6 -0
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+                              struct dw_edma_pcie_data *pdata,
+                              enum pci_barno bar) {
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+             return pdata->phys_addr;
+     return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

Quoted 7 lines

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

Quoted 7 lines

In case of AMD, it is not an address that is accessible from host via PCI, it is the device side DDR offset of physical address which is not known to host,that is why the VSEC capability is used to let know host of the DDR offset to correctly programming the LLP of DMA controller. Without programming the LLP controller will not know from where to start reading the LL for DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

Quoted 3 lines

I guess "device side DDR offset" means this Xilinx device has some DDR internal to the PCI device, and the CPU cannot access it via a BAR?

Quoted 6 lines

The host can access the DDR internal to the PCI device via BAR, but it involves an iATU translation. The host can use the virtual / physical address to access that DDR. The issue is not with the host rather DMA controller which does not understand the physical address provided by the host, eg, the address returned by pci_bus_addr(pdev, barno).

Does this mean dw_edma_get_phys_addr() depends on iATU programming done by the PCI controller driver? Is it possible for that driver to change the iATU programming after dw_edma_get_phys_addr() in a way that breaks this?

Bjorn

author
#11 lore raw

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Bjorn

Please check my response inline.
Regards,
Devendra

Quoted 66 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Friday, September 12, 2025 02:14
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Quoted 30 lines · Devendra
Quoted 30 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>

Quoted 1 line · Devendra
Quoted 1 line

From: Bjorn Helgaas <[email protected]>

[redundant headers removed]

Quoted 22 lines
Quoted 22 lines · Devendra K Verma

drivers/dma/dw-edma/dw-edma-pcie.c static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev, · line 233

Quoted 11 lines

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode. The current code does not have the mechanisms to enable the DMA transactions using the non-LL mode. The following two cases are added with this patch:

drivers/dma/dw-edma/dw-edma-pcie.c +6 -0
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+                              struct dw_edma_pcie_data *pdata,
+                              enum pci_barno bar) {
+     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+             return pdata->phys_addr;
+     return pci_bus_address(pdev, bar);

This doesn't seem right. pci_bus_address() returns pci_bus_addr_t, so pdata->phys_addr should also be a pci_bus_addr_t, and the function should return pci_bus_addr_t.

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side of a PCI host bridge, which may be different than the CPU physical address on the CPU side of the bridge because of things like IOMMUs.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Seems like the struct dw_edma_region.paddr should be renamed to something like "bus_addr" and made into a pci_bus_addr_t.

In case of AMD, it is not an address that is accessible from host via PCI, it is the device side DDR offset of physical address which is not known to host,that is why the VSEC capability is used to let know host of the DDR offset to correctly programming the LLP of DMA

controller.

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

Without programming the LLP controller will not know from where to start reading the LL for DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

I guess "device side DDR offset" means this Xilinx device has some DDR internal to the PCI device, and the CPU cannot access it via a BAR?

Quoted 5 lines

The host can access the DDR internal to the PCI device via BAR, but it involves an iATU translation. The host can use the virtual / physical address to access that DDR. The issue is not with the host rather DMA controller which does not understand the physical address provided by the host, eg, the address returned by pci_bus_addr(pdev, barno).

Quoted 4 lines

Does this mean dw_edma_get_phys_addr() depends on iATU programming done by the PCI controller driver? Is it possible for that driver to change the iATU programming after dw_edma_get_phys_addr() in a way that breaks this?

No, driver can neither program nor modify the iATU. Once device is configured using AMD Programmable Device Image (PDI) which contains these configurations. Once programmed the configuration can not be changed by the driver.

Quoted 1 line

Bjorn