On 8/5/2024 11:39 AM, Lizhi Hou wrote:
AMD AI Engine forms the core of AMD NPU and can be used for accelerating
machine learning applications.
Add the driver to support AI Engine integrated to AMD CPU.
Only very basic functionalities are added.
- module and PCI device initialization
- firmware load
- power up
- low level hardware initialization
Co-developed-by: Narendra Gutta <VenkataNarendraKumar.Gutta@xxxxxxx>
Signed-off-by: Narendra Gutta <VenkataNarendraKumar.Gutta@xxxxxxx>
Co-developed-by: George Yang <George.Yang@xxxxxxx>
Signed-off-by: George Yang <George.Yang@xxxxxxx>
Co-developed-by: Min Ma <min.ma@xxxxxxx>
Signed-off-by: Min Ma <min.ma@xxxxxxx>
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
---
MAINTAINERS | 8 ++
drivers/accel/Kconfig | 1 +
drivers/accel/Makefile | 1 +
drivers/accel/amdxdna/Kconfig | 15 ++
drivers/accel/amdxdna/Makefile | 14 ++
drivers/accel/amdxdna/TODO | 4 +
drivers/accel/amdxdna/aie2_pci.c | 182 ++++++++++++++++++++++++
drivers/accel/amdxdna/aie2_pci.h | 144 +++++++++++++++++++
drivers/accel/amdxdna/aie2_psp.c | 137 ++++++++++++++++++
drivers/accel/amdxdna/aie2_smu.c | 112 +++++++++++++++
drivers/accel/amdxdna/amdxdna_drm.c | 20 +++
drivers/accel/amdxdna/amdxdna_drm.h | 65 +++++++++
drivers/accel/amdxdna/amdxdna_pci_drv.c | 118 +++++++++++++++
drivers/accel/amdxdna/amdxdna_pci_drv.h | 31 ++++
drivers/accel/amdxdna/amdxdna_sysfs.c | 47 ++++++
drivers/accel/amdxdna/npu1_regs.c | 94 ++++++++++++
drivers/accel/amdxdna/npu2_regs.c | 111 +++++++++++++++
drivers/accel/amdxdna/npu4_regs.c | 111 +++++++++++++++
drivers/accel/amdxdna/npu5_regs.c | 111 +++++++++++++++
include/uapi/drm/amdxdna_accel.h | 27 ++++
20 files changed, 1353 insertions(+)
create mode 100644 drivers/accel/amdxdna/Kconfig
create mode 100644 drivers/accel/amdxdna/Makefile
create mode 100644 drivers/accel/amdxdna/TODO
create mode 100644 drivers/accel/amdxdna/aie2_pci.c
create mode 100644 drivers/accel/amdxdna/aie2_pci.h
create mode 100644 drivers/accel/amdxdna/aie2_psp.c
create mode 100644 drivers/accel/amdxdna/aie2_smu.c
create mode 100644 drivers/accel/amdxdna/amdxdna_drm.c
create mode 100644 drivers/accel/amdxdna/amdxdna_drm.h
create mode 100644 drivers/accel/amdxdna/amdxdna_pci_drv.c
create mode 100644 drivers/accel/amdxdna/amdxdna_pci_drv.h
create mode 100644 drivers/accel/amdxdna/amdxdna_sysfs.c
create mode 100644 drivers/accel/amdxdna/npu1_regs.c
create mode 100644 drivers/accel/amdxdna/npu2_regs.c
create mode 100644 drivers/accel/amdxdna/npu4_regs.c
create mode 100644 drivers/accel/amdxdna/npu5_regs.c
create mode 100644 include/uapi/drm/amdxdna_accel.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 36d66b141352..3d2b9f1f1a07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1146,6 +1146,14 @@ M: Sanjay R Mehta <sanju.mehta@xxxxxxx>
S: Maintained
F: drivers/spi/spi-amd.c
+AMD XDNA DRIVER
+M: Min Ma <min.ma@xxxxxxx>
+M: Lizhi Hou <lizhi.hou@xxxxxxx>
+L: dri-devel@xxxxxxxxxxxxxxxxxxxxx
+S: Supported
+F: drivers/accel/amdxdna
Trailing "/"?
+F: include/uapi/drm/amdxdna_accel.h
T: ?
+
AMD XGBE DRIVER
M: "Shyam Sundar S K" <Shyam-sundar.S-k@xxxxxxx>
L: netdev@xxxxxxxxxxxxxxx
diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
new file mode 100644
index 000000000000..3660967c00e6
--- /dev/null
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/amd-iommu.h>
+#include <linux/errno.h>
+#include <linux/firmware.h>
+#include <linux/iommu.h>
You are clearly missing linux/pci.h and I suspect many more.
+
+#include "aie2_pci.h"
+
+static void aie2_hw_stop(struct amdxdna_dev *xdna)
+{
+ struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+ struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
+
+ aie2_psp_stop(ndev->psp_hdl);
+ aie2_smu_fini(ndev);
+ pci_clear_master(pdev);
+ pci_disable_device(pdev);
pci_clear_master() is redundant with pci_disable_device(), no?
+}
+
+static int aie2_hw_start(struct amdxdna_dev *xdna)
+{
+ struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+ struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
+ int ret;
+
+ ret = pci_enable_device(pdev);
+ if (ret) {
+ XDNA_ERR(xdna, "failed to enable device, ret %d", ret);
+ return ret;
+ }
+ pci_set_master(pdev);
+
+ ret = aie2_smu_init(ndev);
+ if (ret) {
+ XDNA_ERR(xdna, "failed to init smu, ret %d", ret);
+ goto disable_dev;
+ }
+
+ ret = aie2_psp_start(ndev->psp_hdl);
+ if (ret) {
+ XDNA_ERR(xdna, "failed to start psp, ret %d", ret);
+ goto fini_smu;
+ }
+
+ return 0;
+
+fini_smu:
+ aie2_smu_fini(ndev);
+disable_dev:
+ pci_disable_device(pdev);
+ pci_clear_master(pdev);
Again, clear_master appears redundant
+
+ return ret;
+}
+
+static int aie2_init(struct amdxdna_dev *xdna)
+{
+ struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+ struct amdxdna_dev_hdl *ndev;
+ struct psp_config psp_conf;
+ const struct firmware *fw;
+ void __iomem * const *tbl;
Is there an extra "*" here?
+ int i, bars, nvec, ret;
+
+ ndev = devm_kzalloc(&pdev->dev, sizeof(*ndev), GFP_KERNEL);
+ if (!ndev)
+ return -ENOMEM;
+
+ ndev->priv = xdna->dev_info->dev_priv;
+ ndev->xdna = xdna;
+
+ ret = request_firmware(&fw, ndev->priv->fw_path, &pdev->dev);
+ if (ret) {
+ XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
+ ndev->priv->fw_path, ret);
+ return ret;
+ }
+
+ ret = pcim_enable_device(pdev);
+ if (ret) {
+ XDNA_ERR(xdna, "pcim enable device failed, ret %d", ret);
+ goto release_fw;
+ }
+
+ bars = pci_select_bars(pdev, IORESOURCE_MEM);
+ for (i = 0; i < PSP_MAX_REGS; i++) {
+ if (!(BIT(PSP_REG_BAR(ndev, i)) && bars)) {
+ XDNA_ERR(xdna, "does not get pci bar%d",
+ PSP_REG_BAR(ndev, i));
+ ret = -EINVAL;
+ goto release_fw;
+ }
+ }
+
+ ret = pcim_iomap_regions(pdev, bars, "amdxdna-npu");
+ if (ret) {
+ XDNA_ERR(xdna, "map regions failed, ret %d", ret);
+ goto release_fw;
+ }
+
+ tbl = pcim_iomap_table(pdev);
+ if (!tbl) {
+ XDNA_ERR(xdna, "Cannot get iomap table");
+ ret = -ENOMEM;
+ goto release_fw;
+ }
+ ndev->sram_base = tbl[xdna->dev_info->sram_bar];
+ ndev->smu_base = tbl[xdna->dev_info->smu_bar];
Doesn't this throw a sparse error from directly accessing memory marked
iomem?
+
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+ if (ret) {
+ XDNA_ERR(xdna, "Failed to set DMA mask: %d", ret);
+ goto release_fw;
+ }
+
+ nvec = pci_msix_vec_count(pdev);
This feels weird. Can your device advertise variable number of MSI-X
vectors? It only works if all of the vectors are used?
+ if (nvec <= 0) {
+ XDNA_ERR(xdna, "does not get number of interrupt vector");
+ ret = -EINVAL;
+ goto release_fw;
+ }
+
+ ret = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX);
+ if (ret < 0) {
+ XDNA_ERR(xdna, "failed to alloc irq vectors, ret %d", ret);
+ goto release_fw;
+ }
+
+ ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+ if (ret) {
+ XDNA_ERR(xdna, "Enable PASID failed, ret %d", ret);
+ goto free_irq;
+ }
+
+ psp_conf.fw_size = fw->size;
+ psp_conf.fw_buf = fw->data;
+ for (i = 0; i < PSP_MAX_REGS; i++)
+ psp_conf.psp_regs[i] = tbl[PSP_REG_BAR(ndev, i)] + PSP_REG_OFF(ndev, i);
+ ndev->psp_hdl = aie2m_psp_create(&pdev->dev, &psp_conf);
+ if (!ndev->psp_hdl) {
+ XDNA_ERR(xdna, "failed to create psp");
+ ret = -ENOMEM;
+ goto disable_sva;
+ }
+ xdna->dev_handle = ndev;
+
+ ret = aie2_hw_start(xdna);
+ if (ret) {
+ XDNA_ERR(xdna, "start npu failed, ret %d", ret);
+ goto disable_sva;
+ }
+
+ release_firmware(fw);
+ return 0;
+
+disable_sva:
+ iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+free_irq:
+ pci_free_irq_vectors(pdev);
+release_fw:
+ release_firmware(fw);
+
+ return ret;
+}
+
+static void aie2_fini(struct amdxdna_dev *xdna)
+{
+ struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+
+ aie2_hw_stop(xdna);
+ iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+ pci_free_irq_vectors(pdev);
+}
+
+const struct amdxdna_dev_ops aie2_ops = {
+ .init = aie2_init,
+ .fini = aie2_fini,
+};
diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
new file mode 100644
index 000000000000..984bf65b7a2b
--- /dev/null
+++ b/drivers/accel/amdxdna/aie2_pci.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _AIE2_PCI_H_
+#define _AIE2_PCI_H_
+
+#include <linux/device.h>
+#include <linux/iopoll.h>
+#include <linux/io.h>
+
+#include "amdxdna_pci_drv.h"
+
+#define AIE2_INTERVAL 20000 /* us */
+#define AIE2_TIMEOUT 1000000 /* us */
+
+/* Firmware determines device memory base address and size */
+#define AIE2_DEVM_BASE 0x4000000
+#define AIE2_DEVM_SIZE (48 * 1024 * 1024)
I'd expect to see some SZ_ macros used here
+
+#define NDEV2PDEV(ndev) \
+ (to_pci_dev((ndev)->xdna->ddev.dev))
+
+#define AIE2_SRAM_OFF(ndev, addr) \
+ ((addr) - (ndev)->priv->sram_dev_addr)
+#define AIE2_MBOX_OFF(ndev, addr) \
+ ((addr) - (ndev)->priv->mbox_dev_addr)
+
+#define PSP_REG_BAR(ndev, idx) \
+ ((ndev)->priv->psp_regs_off[(idx)].bar_idx)
+#define PSP_REG_OFF(ndev, idx) \
+ ((ndev)->priv->psp_regs_off[(idx)].offset)
+#define SRAM_REG_OFF(ndev, idx) \
+ ((ndev)->priv->sram_offs[(idx)].offset)
Really looks like these 6 macros can all fit on a single line, why split
them up?
+
+#define SMU_REG(ndev, idx) \
+({ \
+ typeof(ndev) _ndev = ndev; \
+ ((_ndev)->smu_base + (_ndev)->priv->smu_regs_off[(idx)].offset); \
+})
+#define SRAM_GET_ADDR(ndev, idx) \
+({ \
+ typeof(ndev) _ndev = ndev; \
+ ((_ndev)->sram_base + SRAM_REG_OFF((_ndev), (idx))); \
+})
+
+#define SMU_MPNPUCLK_FREQ_MAX(ndev) \
+ ((ndev)->priv->smu_mpnpuclk_freq_max)
+#define SMU_HCLK_FREQ_MAX(ndev) \
+ ((ndev)->priv->smu_hclk_freq_max)
+
+enum aie2_smu_reg_idx {
+ SMU_CMD_REG = 0,
+ SMU_ARG_REG,
+ SMU_INTR_REG,
+ SMU_RESP_REG,
+ SMU_OUT_REG,
+ SMU_MAX_REGS /* Kepp this at the end */
"keep"
diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
new file mode 100644
index 000000000000..c24207c252a1
--- /dev/null
+++ b/drivers/accel/amdxdna/aie2_psp.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/firmware.h>
+#include <linux/iopoll.h>
+#include <linux/slab.h>
+#include "aie2_pci.h"
+
+#define PSP_STATUS_READY BIT(31)
+
+/* PSP commands */
+#define PSP_VALIDATE 1
+#define PSP_START 2
+#define PSP_RELEASE_TMR 3
+
+/* PSP special arguments */
+#define PSP_START_COPY_FW 1
+
+/* PSP response error code */
+#define PSP_ERROR_CANCEL 0xFFFF0002
+#define PSP_ERROR_BAD_STATE 0xFFFF0007
+
+#define PSP_FW_ALIGN 0x10000
+#define PSP_POLL_INTERVAL 20000 /* us */
+#define PSP_POLL_TIMEOUT 1000000 /* us */
+
+#define PSP_REG(p, reg) \
+ ((p)->psp_regs[reg])
This is not valid with __iomem
+struct psp_device *aie2m_psp_create(struct device *dev, struct psp_config *conf)
+{
+ struct psp_device *psp;
+ u64 offset;
+
+ psp = devm_kzalloc(dev, sizeof(*psp), GFP_KERNEL);
+ if (!psp)
+ return NULL;
+
+ psp->dev = dev;
+ memcpy(psp->psp_regs, conf->psp_regs, sizeof(psp->psp_regs));
+
+ psp->fw_buf_sz = ALIGN(conf->fw_size, PSP_FW_ALIGN) + PSP_FW_ALIGN;
+ psp->fw_buffer = devm_kmalloc(psp->dev, psp->fw_buf_sz, GFP_KERNEL);
Feels like this (and a bunch of other instances I haven't commented on)
should be drmm_* allocs.
+ if (!psp->fw_buffer) {
+ dev_err(psp->dev, "no memory for fw buffer");
+ return NULL;
+ }
+
+ psp->fw_paddr = virt_to_phys(psp->fw_buffer);
I'm pretty sure virt_to_phys() is always wrong
+ offset = ALIGN(psp->fw_paddr, PSP_FW_ALIGN) - psp->fw_paddr;
+ psp->fw_paddr += offset;
+ memcpy(psp->fw_buffer + offset, conf->fw_buf, conf->fw_size);
+
+ return psp;
+}
diff --git a/drivers/accel/amdxdna/amdxdna_drm.c b/drivers/accel/amdxdna/amdxdna_drm.c
new file mode 100644
index 000000000000..91e4f9c9dac9
--- /dev/null
+++ b/drivers/accel/amdxdna/amdxdna_drm.c
What is the point of this file? Seems like all of this could just be in
amdxdna_pci_drv.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
+ */
+
+#include <drm/drm_ioctl.h>
+#include <drm/drm_accel.h>
+
+#include "amdxdna_drm.h"
+
+DEFINE_DRM_ACCEL_FOPS(amdxdna_fops);
+
+const struct drm_driver amdxdna_drm_drv = {
+ .driver_features = DRIVER_GEM | DRIVER_COMPUTE_ACCEL,
+ .fops = &amdxdna_fops,
+ .name = "amdxdna_accel_driver",
+ .desc = "AMD XDNA DRM implementation",
+ .major = AMDXDNA_DRIVER_MAJOR,
+ .minor = AMDXDNA_DRIVER_MINOR,
These are deprecated. You should drop them
+};
diff --git a/drivers/accel/amdxdna/amdxdna_drm.h b/drivers/accel/amdxdna/amdxdna_drm.h
new file mode 100644
index 000000000000..2b18bcbdc23e
--- /dev/null
+++ b/drivers/accel/amdxdna/amdxdna_drm.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _AMDXDNA_DRM_H_
+#define _AMDXDNA_DRM_H_
+
+#include <drm/amdxdna_accel.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_print.h>
+#include <drm/drm_file.h>
+
+#define XDNA_INFO(xdna, fmt, args...) drm_info(&(xdna)->ddev, fmt, ##args)
+#define XDNA_WARN(xdna, fmt, args...) drm_warn(&(xdna)->ddev, "%s: "fmt, __func__, ##args)
+#define XDNA_ERR(xdna, fmt, args...) drm_err(&(xdna)->ddev, "%s: "fmt, __func__, ##args)
+#define XDNA_DBG(xdna, fmt, args...) drm_dbg(&(xdna)->ddev, fmt, ##args)
+#define XDNA_INFO_ONCE(xdna, fmt, args...) drm_info_once(&(xdna)->ddev, fmt, ##args)
+
+#define to_xdna_dev(drm_dev) \
+ ((struct amdxdna_dev *)container_of(drm_dev, struct amdxdna_dev, ddev))
+
+extern const struct drm_driver amdxdna_drm_drv;
+
+struct amdxdna_dev;
+
+/*
+ * struct amdxdna_dev_ops - Device hardware operation callbacks
+ */
+struct amdxdna_dev_ops {
+ int (*init)(struct amdxdna_dev *xdna);
+ void (*fini)(struct amdxdna_dev *xdna);
+};
+
+/*
+ * struct amdxdna_dev_info - Device hardware information
+ * Record device static information, like reg, mbox, PSP, SMU bar index,
The last "," is weird
+ */
+struct amdxdna_dev_info {
+ int reg_bar;
+ int mbox_bar;
+ int sram_bar;
+ int psp_bar;
+ int smu_bar;
+ int device_type;
+ int first_col;
+ u32 dev_mem_buf_shift;
+ u64 dev_mem_base;
+ size_t dev_mem_size;
+ char *vbnv;
+ const struct amdxdna_dev_priv *dev_priv;
+ const struct amdxdna_dev_ops *ops;
+};
+
+struct amdxdna_dev {
+ struct drm_device ddev;
+ struct amdxdna_dev_hdl *dev_handle;
+ const struct amdxdna_dev_info *dev_info;
+
+ struct mutex dev_lock; /* per device lock */
+};
+
+#endif /* _AMDXDNA_DRM_H_ */
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
new file mode 100644
index 000000000000..7d0cfd918b0e
--- /dev/null
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/module.h>
+
+#include "amdxdna_pci_drv.h"
+
+/*
+ * There are platforms which share the same PCI device ID
+ * but have different PCI revision IDs. So, let the PCI class
+ * determine the probe and later use the (device_id, rev_id)
+ * pair as a key to select the devices.
+ */
Huh? So, VID == AMD, DID == 0x17f0, rev == 0x1 is a completely
different device? That feels like a PCIe spec violation...
+static const struct pci_device_id pci_ids[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_ANY_ID),
+ .class = PCI_CLASS_SP_OTHER << 8,
Weird. I would have expected the Accelerator class to be used
+ .class_mask = 0xFFFF00,
+ },
+ {0}
+};
+
+MODULE_DEVICE_TABLE(pci, pci_ids);
+
+static const struct amdxdna_device_id amdxdna_ids[] = {
+ { 0x1502, 0x0, &dev_npu1_info },
+ { 0x17f0, 0x0, &dev_npu2_info },
+ { 0x17f0, 0x10, &dev_npu4_info },
+ { 0x17f0, 0x11, &dev_npu5_info },
+ {0}
+};
+
+module_pci_driver(amdxdna_pci_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("XRT Team <runtimeca39d@xxxxxxx>");
+MODULE_VERSION("0.1");
This is redundant with the kernel version, no? How are you going to
have different module versions for the same kernel version?
+MODULE_DESCRIPTION("amdxdna driver");
diff --git a/include/uapi/drm/amdxdna_accel.h b/include/uapi/drm/amdxdna_accel.h
new file mode 100644
index 000000000000..1b699464150e
--- /dev/null
+++ b/include/uapi/drm/amdxdna_accel.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _UAPI_AMDXDNA_ACCEL_H_
+#define _UAPI_AMDXDNA_ACCEL_H_
+
+#include "drm.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define AMDXDNA_DRIVER_MAJOR 1
+#define AMDXDNA_DRIVER_MINOR 0
Drop these.
+
+enum amdxdna_device_type {
+ AMDXDNA_DEV_TYPE_UNKNOWN = -1,
+ AMDXDNA_DEV_TYPE_KMQ,
+};
+
+#if defined(__cplusplus)
+} /* extern c end */
+#endif
+
+#endif /* _UAPI_AMDXDNA_ACCEL_H_ */
