LinuxLists.cc - [PATCH v11 0/4] dmaengine: dw-edma: Add support for native HDMA

2023-05-20 05:19:51

Subject: [PATCH v11 0/4] dmaengine: dw-edma: Add support for native HDMA

2023-05-20 05:21:26

Subject: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

Add support for HDMA NATIVE, as long the IP design has set
the compatible register map parameter-HDMA_NATIVE,
which allows compatibility for native HDMA register configuration.

The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
And the native HDMA registers are different from eDMA, so this patch
add support for HDMA NATIVE mode.

HDMA write and read channels operate independently to maximize
the performance of the HDMA read and write data transfer over
the link When you configure the HDMA with multiple read channels,
then it uses a round robin (RR) arbitration scheme to select
the next read channel to be serviced.The same applies when you
have multiple write channels.

The native HDMA driver also supports a maximum of 16 independent
channels (8 write + 8 read), which can run simultaneously.
Both SAR (Source Address Register) and DAR (Destination Address Register)
are aligned to byte.

Signed-off-by: Cai Huoqing <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Serge Semin <[email protected]>
---
v10->v11: Using single name in commit log.

v10 link:
https://lore.kernel.org/lkml/[email protected]/

drivers/dma/dw-edma/Makefile | 5 +-
drivers/dma/dw-edma/dw-edma-core.c | 6 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 294 ++++++++++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 129 +++++++++++
include/linux/dma/edma.h | 3 +-
6 files changed, 450 insertions(+), 4 deletions(-)
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h

diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index 8d45c0d5689d..b1c91ef2c63d 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -2,6 +2,7 @@

obj-$(CONFIG_DW_EDMA) += dw-edma.o
dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
-dw-edma-objs := dw-edma-core.o \
- dw-edma-v0-core.o $(dw-edma-y)
+dw-edma-objs := dw-edma-core.o \
+ dw-edma-v0-core.o \
+ dw-hdma-v0-core.o $(dw-edma-y)
obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index f17207c66c19..68236247059d 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -18,6 +18,7 @@

#include "dw-edma-core.h"
#include "dw-edma-v0-core.h"
+#include "dw-hdma-v0-core.h"
#include "../dmaengine.h"
#include "../virt-dma.h"

@@ -922,7 +923,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)

dw->chip = chip;

- dw_edma_v0_core_register(dw);
+ if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
+ dw_hdma_v0_core_register(dw);
+ else
+ dw_edma_v0_core_register(dw);

raw_spin_lock_init(&dw->lock);

diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
new file mode 100644
index 000000000000..22b7b0410deb
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ */
+
+#include <linux/bitfield.h>
+#include <linux/irqreturn.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+
+#include "dw-edma-core.h"
+#include "dw-hdma-v0-core.h"
+#include "dw-hdma-v0-regs.h"
+
+enum dw_hdma_control {
+ DW_HDMA_V0_CB = BIT(0),
+ DW_HDMA_V0_TCB = BIT(1),
+ DW_HDMA_V0_LLP = BIT(2),
+ DW_HDMA_V0_LIE = BIT(3),
+ DW_HDMA_V0_RIE = BIT(4),
+ DW_HDMA_V0_CCS = BIT(8),
+ DW_HDMA_V0_LLE = BIT(9),
+};
+
+static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
+{
+ return dw->chip->reg_base;
+}
+
+static inline struct dw_hdma_v0_ch_regs __iomem *
+__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
+{
+ if (dir == EDMA_DIR_WRITE)
+ return &(__dw_regs(dw)->ch[ch].wr);
+ else
+ return &(__dw_regs(dw)->ch[ch].rd);
+}
+
+#define SET_CH_32(dw, dir, ch, name, value) \
+ writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
+
+#define GET_CH_32(dw, dir, ch, name) \
+ readl(&(__dw_ch_regs(dw, dir, ch)->name))
+
+#define SET_BOTH_CH_32(dw, ch, name, value) \
+ do { \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
+ } while (0)
+
+/* HDMA management callbacks */
+static void dw_hdma_v0_core_off(struct dw_edma *dw)
+{
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ SET_BOTH_CH_32(dw, id, int_setup,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, int_clear,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, ch_en, 0);
+ }
+}
+
+static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+ u32 num_ch = 0;
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
+ num_ch++;
+ }
+
+ if (num_ch > HDMA_V0_MAX_NR_CH)
+ num_ch = HDMA_V0_MAX_NR_CH;
+
+ return (u16)num_ch;
+}
+
+static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
+ GET_CH_32(dw, chan->id, chan->dir, ch_stat));
+
+ if (tmp == 1)
+ return DMA_IN_PROGRESS;
+ else if (tmp == 3)
+ return DMA_COMPLETE;
+ else
+ return DMA_ERROR;
+}
+
+static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
+}
+
+static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
+}
+
+static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ return GET_CH_32(dw, chan->dir, chan->id, int_stat);
+}
+
+static irqreturn_t
+dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
+ dw_edma_handler_t done, dw_edma_handler_t abort)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ irqreturn_t ret = IRQ_NONE;
+ struct dw_edma_chan *chan;
+ unsigned long off, mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ for_each_set_bit(pos, &mask, total) {
+ chan = &dw->chan[pos + off];
+
+ val = dw_hdma_v0_core_status_int(chan);
+ if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
+ dw_hdma_v0_core_clear_done_int(chan);
+ done(chan);
+
+ ret = IRQ_HANDLED;
+ }
+
+ if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
+ dw_hdma_v0_core_clear_abort_int(chan);
+ abort(chan);
+
+ ret = IRQ_HANDLED;
+ }
+ }
+
+ return ret;
+}
+
+static void dw_hdma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
+ u32 control, u32 size, u64 sar, u64 dar)
+{
+ ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
+
+ if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_hdma_v0_lli *lli = chunk->ll_region.vaddr.mem + ofs;
+
+ lli->control = control;
+ lli->transfer_size = size;
+ lli->sar.reg = sar;
+ lli->dar.reg = dar;
+ } else {
+ struct dw_hdma_v0_lli __iomem *lli = chunk->ll_region.vaddr.io + ofs;
+
+ writel(control, &lli->control);
+ writel(size, &lli->transfer_size);
+ writeq(sar, &lli->sar.reg);
+ writeq(dar, &lli->dar.reg);
+ }
+}
+
+static void dw_hdma_v0_write_ll_link(struct dw_edma_chunk *chunk,
+ int i, u32 control, u64 pointer)
+{
+ ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
+
+ if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_hdma_v0_llp *llp = chunk->ll_region.vaddr.mem + ofs;
+
+ llp->control = control;
+ llp->llp.reg = pointer;
+ } else {
+ struct dw_hdma_v0_llp __iomem *llp = chunk->ll_region.vaddr.io + ofs;
+
+ writel(control, &llp->control);
+ writeq(pointer, &llp->llp.reg);
+ }
+}
+
+static void dw_hdma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
+{
+ struct dw_edma_burst *child;
+ struct dw_edma_chan *chan = chunk->chan;
+ u32 control = 0, i = 0;
+ int j;
+
+ if (chunk->cb)
+ control = DW_HDMA_V0_CB;
+
+ j = chunk->bursts_alloc;
+ list_for_each_entry(child, &chunk->burst->list, list) {
+ j--;
+ if (!j) {
+ control |= DW_HDMA_V0_LIE;
+ if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+ control |= DW_HDMA_V0_RIE;
+ }
+
+ dw_hdma_v0_write_ll_data(chunk, i++, control, child->sz,
+ child->sar, child->dar);
+ }
+
+ control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
+ if (!chunk->cb)
+ control |= DW_HDMA_V0_CB;
+
+ dw_hdma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
+}
+
+static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+{
+ struct dw_edma_chan *chan = chunk->chan;
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ dw_hdma_v0_core_write_chunk(chunk);
+
+ if (first) {
+ /* Enable engine */
+ SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
+ /* Interrupt enable&unmask - done, abort */
+ tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
+ HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
+ SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
+ /* Channel control */
+ SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
+ /* Linked list */
+ /* llp is not aligned on 64bit -> keep 32bit accesses */
+ SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
+ lower_32_bits(chunk->ll_region.paddr));
+ SET_CH_32(dw, chan->dir, chan->id, llp.msb,
+ upper_32_bits(chunk->ll_region.paddr));
+ }
+ /* Set consumer cycle */
+ SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
+ HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
+ /* Doorbell */
+ SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
+}
+
+static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ /* MSI done addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
+ /* MSI abort addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
+ /* config MSI data */
+ SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
+}
+
+/* HDMA debugfs callbacks */
+static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
+{
+}
+
+static const struct dw_edma_core_ops dw_hdma_v0_core = {
+ .off = dw_hdma_v0_core_off,
+ .ch_count = dw_hdma_v0_core_ch_count,
+ .ch_status = dw_hdma_v0_core_ch_status,
+ .handle_int = dw_hdma_v0_core_handle_int,
+ .start = dw_hdma_v0_core_start,
+ .ch_config = dw_hdma_v0_core_ch_config,
+ .debugfs_on = dw_hdma_v0_core_debugfs_on,
+};
+
+void dw_hdma_v0_core_register(struct dw_edma *dw)
+{
+ dw->core = &dw_hdma_v0_core;
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
new file mode 100644
index 000000000000..c373b4f0bd8a
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_CORE_H
+#define _DW_HDMA_V0_CORE_H
+
+#include <linux/dma/edma.h>
+
+/* HDMA core register */
+void dw_hdma_v0_core_register(struct dw_edma *dw);
+
+#endif /* _DW_HDMA_V0_CORE_H */
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
new file mode 100644
index 000000000000..a974abdf8aaf
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -0,0 +1,129 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 reg
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_REGS_H
+#define _DW_HDMA_V0_REGS_H
+
+#include <linux/dmaengine.h>
+
+#define HDMA_V0_MAX_NR_CH 8
+#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
+#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
+#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
+#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
+#define HDMA_V0_ABORT_INT_MASK BIT(2)
+#define HDMA_V0_STOP_INT_MASK BIT(0)
+#define HDMA_V0_LINKLIST_EN BIT(0)
+#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
+#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
+#define HDMA_V0_DOORBELL_START BIT(0)
+#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
+
+struct dw_hdma_v0_ch_regs {
+ u32 ch_en; /* 0x0000 */
+ u32 doorbell; /* 0x0004 */
+ u32 prefetch; /* 0x0008 */
+ u32 handshake; /* 0x000c */
+ union {
+ u64 reg; /* 0x0010..0x0014 */
+ struct {
+ u32 lsb; /* 0x0010 */
+ u32 msb; /* 0x0014 */
+ };
+ } llp;
+ u32 cycle_sync; /* 0x0018 */
+ u32 transfer_size; /* 0x001c */
+ union {
+ u64 reg; /* 0x0020..0x0024 */
+ struct {
+ u32 lsb; /* 0x0020 */
+ u32 msb; /* 0x0024 */
+ };
+ } sar;
+ union {
+ u64 reg; /* 0x0028..0x002c */
+ struct {
+ u32 lsb; /* 0x0028 */
+ u32 msb; /* 0x002c */
+ };
+ } dar;
+ u32 watermark_en; /* 0x0030 */
+ u32 control1; /* 0x0034 */
+ u32 func_num; /* 0x0038 */
+ u32 qos; /* 0x003c */
+ u32 padding_1[16]; /* 0x0040..0x007c */
+ u32 ch_stat; /* 0x0080 */
+ u32 int_stat; /* 0x0084 */
+ u32 int_setup; /* 0x0088 */
+ u32 int_clear; /* 0x008c */
+ union {
+ u64 reg; /* 0x0090..0x0094 */
+ struct {
+ u32 lsb; /* 0x0090 */
+ u32 msb; /* 0x0094 */
+ };
+ } msi_stop;
+ union {
+ u64 reg; /* 0x0098..0x009c */
+ struct {
+ u32 lsb; /* 0x0098 */
+ u32 msb; /* 0x009c */
+ };
+ } msi_watermark;
+ union {
+ u64 reg; /* 0x00a0..0x00a4 */
+ struct {
+ u32 lsb; /* 0x00a0 */
+ u32 msb; /* 0x00a4 */
+ };
+ } msi_abort;
+ u32 msi_msgdata; /* 0x00a8 */
+ u32 padding_2[21]; /* 0x00ac..0x00fc */
+} __packed;
+
+struct dw_hdma_v0_ch {
+ struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
+ struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
+} __packed;
+
+struct dw_hdma_v0_regs {
+ struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
+} __packed;
+
+struct dw_hdma_v0_lli {
+ u32 control;
+ u32 transfer_size;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } sar;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } dar;
+} __packed;
+
+struct dw_hdma_v0_llp {
+ u32 control;
+ u32 reserved;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } llp;
+} __packed;
+
+#endif /* _DW_HDMA_V0_REGS_H */
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index ed401c965a87..3080747689f6 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -48,7 +48,8 @@ struct dw_edma_plat_ops {
enum dw_edma_map_format {
EDMA_MF_EDMA_LEGACY = 0x0,
EDMA_MF_EDMA_UNROLL = 0x1,
- EDMA_MF_HDMA_COMPAT = 0x5
+ EDMA_MF_HDMA_COMPAT = 0x5,
+ EDMA_MF_HDMA_NATIVE = 0x7,
};

/**
--
2.34.1

2023-05-20 05:22:21

by Cai Huoqing

[permalink] [raw]

Subject: [PATCH v11 4/4] dmaengine: dw-edma: Add HDMA DebugFS support

Add HDMA DebugFS support to show registers content

Signed-off-by: Cai Huoqing <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Serge Semin <[email protected]>
---
v10->v11: Using single name in commit log.

v10 link:
https://lore.kernel.org/lkml/[email protected]/

drivers/dma/dw-edma/Makefile | 3 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 2 +
drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 170 +++++++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 +++
4 files changed, 196 insertions(+), 1 deletion(-)
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h

diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index b1c91ef2c63d..83ab58f87760 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0

obj-$(CONFIG_DW_EDMA) += dw-edma.o
-dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
+dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
+ dw-hdma-v0-debugfs.o
dw-edma-objs := dw-edma-core.o \
dw-edma-v0-core.o \
dw-hdma-v0-core.o $(dw-edma-y)
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 22b7b0410deb..00b735a0202a 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -11,6 +11,7 @@
#include "dw-edma-core.h"
#include "dw-hdma-v0-core.h"
#include "dw-hdma-v0-regs.h"
+#include "dw-hdma-v0-debugfs.h"

enum dw_hdma_control {
DW_HDMA_V0_CB = BIT(0),
@@ -276,6 +277,7 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
/* HDMA debugfs callbacks */
static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
{
+ dw_hdma_v0_debugfs_on(dw);
}

static const struct dw_edma_core_ops dw_hdma_v0_core = {
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
new file mode 100644
index 000000000000..520c81978b08
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/bitfield.h>
+
+#include "dw-hdma-v0-debugfs.h"
+#include "dw-hdma-v0-regs.h"
+#include "dw-edma-core.h"
+
+#define REGS_ADDR(dw, name) \
+ ({ \
+ struct dw_hdma_v0_regs __iomem *__regs = (dw)->chip->reg_base; \
+ \
+ (void __iomem *)&__regs->name; \
+ })
+
+#define REGS_CH_ADDR(dw, name, _dir, _ch) \
+ ({ \
+ struct dw_hdma_v0_ch_regs __iomem *__ch_regs; \
+ \
+ if (_dir == EDMA_DIR_READ) \
+ __ch_regs = REGS_ADDR(dw, ch[_ch].rd); \
+ else \
+ __ch_regs = REGS_ADDR(dw, ch[_ch].wr); \
+ \
+ (void __iomem *)&__ch_regs->name; \
+ })
+
+#define CTX_REGISTER(dw, name, dir, ch) \
+ {#name, REGS_CH_ADDR(dw, name, dir, ch)}
+
+#define WRITE_STR "write"
+#define READ_STR "read"
+#define CHANNEL_STR "channel"
+#define REGISTERS_STR "registers"
+
+struct dw_hdma_debugfs_entry {
+ const char *name;
+ void __iomem *reg;
+};
+
+static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
+{
+ struct dw_hdma_debugfs_entry *entry = data;
+ void __iomem *reg = entry->reg;
+
+ *val = readl(reg);
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
+
+static void dw_hdma_debugfs_create_x32(struct dw_edma *dw,
+ const struct dw_hdma_debugfs_entry ini[],
+ int nr_entries, struct dentry *dent)
+{
+ struct dw_hdma_debugfs_entry *entries;
+ int i;
+
+ entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
+ GFP_KERNEL);
+ if (!entries)
+ return;
+
+ for (i = 0; i < nr_entries; i++) {
+ entries[i] = ini[i];
+
+ debugfs_create_file_unsafe(entries[i].name, 0444, dent,
+ &entries[i], &fops_x32);
+ }
+}
+
+static void dw_hdma_debugfs_regs_ch(struct dw_edma *dw, enum dw_edma_dir dir,
+ u16 ch, struct dentry *dent)
+{
+ const struct dw_hdma_debugfs_entry debugfs_regs[] = {
+ CTX_REGISTER(dw, ch_en, dir, ch),
+ CTX_REGISTER(dw, doorbell, dir, ch),
+ CTX_REGISTER(dw, prefetch, dir, ch),
+ CTX_REGISTER(dw, handshake, dir, ch),
+ CTX_REGISTER(dw, llp.lsb, dir, ch),
+ CTX_REGISTER(dw, llp.msb, dir, ch),
+ CTX_REGISTER(dw, cycle_sync, dir, ch),
+ CTX_REGISTER(dw, transfer_size, dir, ch),
+ CTX_REGISTER(dw, sar.lsb, dir, ch),
+ CTX_REGISTER(dw, sar.msb, dir, ch),
+ CTX_REGISTER(dw, dar.lsb, dir, ch),
+ CTX_REGISTER(dw, dar.msb, dir, ch),
+ CTX_REGISTER(dw, watermark_en, dir, ch),
+ CTX_REGISTER(dw, control1, dir, ch),
+ CTX_REGISTER(dw, func_num, dir, ch),
+ CTX_REGISTER(dw, qos, dir, ch),
+ CTX_REGISTER(dw, ch_stat, dir, ch),
+ CTX_REGISTER(dw, int_stat, dir, ch),
+ CTX_REGISTER(dw, int_setup, dir, ch),
+ CTX_REGISTER(dw, int_clear, dir, ch),
+ CTX_REGISTER(dw, msi_stop.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_stop.msb, dir, ch),
+ CTX_REGISTER(dw, msi_watermark.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_watermark.msb, dir, ch),
+ CTX_REGISTER(dw, msi_abort.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_abort.msb, dir, ch),
+ CTX_REGISTER(dw, msi_msgdata, dir, ch),
+ };
+ int nr_entries = ARRAY_SIZE(debugfs_regs);
+
+ dw_hdma_debugfs_create_x32(dw, debugfs_regs, nr_entries, dent);
+}
+
+static void dw_hdma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)
+{
+ struct dentry *regs_dent, *ch_dent;
+ char name[16];
+ int i;
+
+ regs_dent = debugfs_create_dir(WRITE_STR, dent);
+
+ for (i = 0; i < dw->wr_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dent = debugfs_create_dir(name, regs_dent);
+
+ dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_WRITE, i, ch_dent);
+ }
+}
+
+static void dw_hdma_debugfs_regs_rd(struct dw_edma *dw, struct dentry *dent)
+{
+ struct dentry *regs_dent, *ch_dent;
+ char name[16];
+ int i;
+
+ regs_dent = debugfs_create_dir(READ_STR, dent);
+
+ for (i = 0; i < dw->rd_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dent = debugfs_create_dir(name, regs_dent);
+
+ dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_READ, i, ch_dent);
+ }
+}
+
+static void dw_hdma_debugfs_regs(struct dw_edma *dw)
+{
+ struct dentry *regs_dent;
+
+ regs_dent = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
+
+ dw_hdma_debugfs_regs_wr(dw, regs_dent);
+ dw_hdma_debugfs_regs_rd(dw, regs_dent);
+}
+
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+ if (!debugfs_initialized())
+ return;
+
+ debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
+ debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
+ debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
+
+ dw_hdma_debugfs_regs(dw);
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
new file mode 100644
index 000000000000..e6842c83777d
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_DEBUG_FS_H
+#define _DW_HDMA_V0_DEBUG_FS_H
+
+#include <linux/dma/edma.h>
+
+#ifdef CONFIG_DEBUG_FS
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
+#else
+static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
+#endif /* _DW_HDMA_V0_DEBUG_FS_H */
--
2.34.1

2023-05-20 05:23:05

by Cai Huoqing

[permalink] [raw]

Subject: [PATCH v11 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

The dw_edma_core_ops structure contains a set of the operations:
device IRQ numbers getter, CPU/PCI address translation. Based on the
functions semantics the structure name "dw_edma_plat_ops" looks more
descriptive since indeed the operations are platform-specific. The
"dw_edma_core_ops" name shall be used for a structure with the IP-core
specific set of callbacks in order to abstract out DW eDMA and DW HDMA
setups. Such structure will be added in one of the next commit in the
framework of the set of changes adding the DW HDMA device support.

Anyway the renaming was necessary to distinguish two types of
the implementation callbacks:
1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
setups in one or another aspect of the DMA-engine initialization.
2. DW eDMA/hDMA platform specific operations: the DMA device
environment configs like IRQs, address translation, etc.

Signed-off-by: Cai Huoqing <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Serge Semin <[email protected]>
---
v10->v11: Using single name in commit log.

v10 link:
https://lore.kernel.org/lkml/[email protected]/

drivers/dma/dw-edma/dw-edma-pcie.c | 4 ++--
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
include/linux/dma/edma.h | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 2b40f2b44f5e..1c6043751dc9 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
return region.start;
}

-static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
+static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
.irq_vector = dw_edma_pcie_irq_vector,
.pci_address = dw_edma_pcie_address,
};
@@ -225,7 +225,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_core_ops;
+ chip->ops = &dw_edma_pcie_plat_ops;

chip->ll_wr_cnt = vsec_data.wr_ch_cnt;
chip->ll_rd_cnt = vsec_data.rd_ch_cnt;
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 8e33e6e59e68..1f2ee71da4da 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -828,7 +828,7 @@ static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
return platform_get_irq_byname_optional(pdev, name);
}

-static struct dw_edma_core_ops dw_pcie_edma_ops = {
+static struct dw_edma_plat_ops dw_pcie_edma_ops = {
.irq_vector = dw_pcie_edma_irq_vector,
};

diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index d2638d9259dc..ed401c965a87 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -40,7 +40,7 @@ struct dw_edma_region {
* iATU windows. That will be done by the controller
* automatically.
*/
-struct dw_edma_core_ops {
+struct dw_edma_plat_ops {
int (*irq_vector)(struct device *dev, unsigned int nr);
u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
};
@@ -80,7 +80,7 @@ enum dw_edma_chip_flags {
struct dw_edma_chip {
struct device *dev;
int nr_irqs;
- const struct dw_edma_core_ops *ops;
+ const struct dw_edma_plat_ops *ops;
u32 flags;

void __iomem *reg_base;
--
2.34.1

2023-05-24 07:03:55

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH v11 0/4] dmaengine: dw-edma: Add support for native HDMA

On 20-05-23, 13:08, Cai Huoqing wrote:
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.The same applies when
> youhave multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are aligned to byte.

Applied, thanks

--
~Vinod

2023-06-07 08:13:52

by Kory Maincent

[permalink] [raw]

Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

On Sat, 20 May 2023 13:08:51 +0800
Cai Huoqing <[email protected]> wrote:

> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.

I know the patch has been merged in dmaengine tree but something seems weird on
my side.

The akida_dw_edma_probe function is selecting the minimum read and write
channels by doing the minimum between ll_wr_cnt and the ch_count callback.
The hdma ch_count callback is counting the number of channels enabled by reading
the number of ch_en registers set. At probe time there is no channels registers
that has been set as it is done later in the dw_hdma_v0_core_start function.
Then the dw_hdma_v0_core_ch_count will always return 0 at probe time and the
number of channels will be set to 0 which is not what we want.
Could I miss something?

See the functions bellow:

> int akida_dw_edma_probe(struct dw_edma_chip *chip)
> {
...
> dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
>
> dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
>
> if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> return -EINVAL;
...
}

> +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + u32 num_ch = 0;
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> + num_ch++;
> + }
> +
> + if (num_ch > HDMA_V0_MAX_NR_CH)
> + num_ch = HDMA_V0_MAX_NR_CH;
> +
> + return (u16)num_ch;
> +}

> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> + struct dw_edma_chan *chan = chunk->chan;
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + dw_hdma_v0_core_write_chunk(chunk);
> +
> + if (first) {
> + /* Enable engine */
> + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
...
> +}

2023-06-07 12:09:16

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

Hi K?ry

On Wed, Jun 07, 2023 at 09:58:32AM +0200, K?ry Maincent wrote:
> On Sat, 20 May 2023 13:08:51 +0800
> Cai Huoqing <[email protected]> wrote:
>
> > Add support for HDMA NATIVE, as long the IP design has set
> > the compatible register map parameter-HDMA_NATIVE,
> > which allows compatibility for native HDMA register configuration.
>
> I know the patch has been merged in dmaengine tree but something seems weird on
> my side.
>

> The akida_dw_edma_probe function is selecting the minimum read and write

First of all. What is akida platform you are referring to? I failed to
find any mention in the mainline kernel repo.

> channels by doing the minimum between ll_wr_cnt and the ch_count callback.
> The hdma ch_count callback is counting the number of channels enabled by reading
> the number of ch_en registers set. At probe time there is no channels registers
> that has been set as it is done later in the dw_hdma_v0_core_start function.
> Then the dw_hdma_v0_core_ch_count will always return 0 at probe time and the
> number of channels will be set to 0 which is not what we want.
> Could I miss something?

Based on the HDMA patches content you are right. The channels must be
pre-enabled in order to have the dw_hdma_v0_core_ch_count() procedure
to work correctly otherwise you'll indeed end up with an empty list of
channels. I don't have any device with the HDMA engine embedded so I
couldn't have possibly tracked that peculiarity on review. Anyway
AFAICS Cai just implemented a method which seemed to work for his
hardware setup.

If you think it doesn't work correctly or it isn't portable enough
then you are free to provide your own implementation of the method and
submit a patch. I hope Cai will be willing to test it out to make sure
that it works correctly for you and his platforms.

As for me if I were on your place I would have implemented a loop
which would walk over all possible HDMA channels (HDMA_V0_MAX_NR_CH)
and counted all channels which could be enabled. If the ch_en CSR is
writable (that is the channel could be enabled) then it shall be
considered as existent. Of course before that I would have made sure
that the non-existent channels had non-writable ch_en CSR.

-Serge(y)

>
> See the functions bellow:
>
> > int akida_dw_edma_probe(struct dw_edma_chip *chip)
> > {
> ...
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> >
> > dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> >
> > if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> > return -EINVAL;
> ...
> }
>
>
> > +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +{
> > + u32 num_ch = 0;
> > + int id;
> > +
> > + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> > + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> > + num_ch++;
> > + }
> > +
> > + if (num_ch > HDMA_V0_MAX_NR_CH)
> > + num_ch = HDMA_V0_MAX_NR_CH;
> > +
> > + return (u16)num_ch;
> > +}
>
>
>
> > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +{
> > + struct dw_edma_chan *chan = chunk->chan;
> > + struct dw_edma *dw = chan->dw;
> > + u32 tmp;
> > +
> > + dw_hdma_v0_core_write_chunk(chunk);
> > +
> > + if (first) {
> > + /* Enable engine */
> > + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> ...
> > +}
>

2023-06-07 12:47:34

by Kory Maincent

[permalink] [raw]

Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

On Wed, 7 Jun 2023 14:49:50 +0300
Serge Semin <[email protected]> wrote:

> Hi Köry
> First of all. What is akida platform you are referring to? I failed to
> find any mention in the mainline kernel repo.

Yes, sorry akida is the project prefix I am currently working on.
It is simply a prefix for the symbol export to be different than mainline,
don't take it into account.

> > channels by doing the minimum between ll_wr_cnt and the ch_count callback.
> > The hdma ch_count callback is counting the number of channels enabled by
> > reading the number of ch_en registers set. At probe time there is no
> > channels registers that has been set as it is done later in the
> > dw_hdma_v0_core_start function. Then the dw_hdma_v0_core_ch_count will
> > always return 0 at probe time and the number of channels will be set to 0
> > which is not what we want. Could I miss something?
>
> Based on the HDMA patches content you are right. The channels must be
> pre-enabled in order to have the dw_hdma_v0_core_ch_count() procedure
> to work correctly otherwise you'll indeed end up with an empty list of
> channels. I don't have any device with the HDMA engine embedded so I
> couldn't have possibly tracked that peculiarity on review. Anyway
> AFAICS Cai just implemented a method which seemed to work for his
> hardware setup.

Alright, on my side I have a board using this FPGA implementation and it
indeed doesn't work as is.

> If you think it doesn't work correctly or it isn't portable enough
> then you are free to provide your own implementation of the method and
> submit a patch. I hope Cai will be willing to test it out to make sure
> that it works correctly for you and his platforms.
>
> As for me if I were on your place I would have implemented a loop
> which would walk over all possible HDMA channels (HDMA_V0_MAX_NR_CH)
> and counted all channels which could be enabled. If the ch_en CSR is
> writable (that is the channel could be enabled) then it shall be
> considered as existent. Of course before that I would have made sure
> that the non-existent channels had non-writable ch_en CSR.

This could be a nice idea but it doesn't work, non-existent channels seems to
be writable. The datasheet of the HDMA IP doesn't have any register to find out
the maximum existent channel.
As there is no way to know this, the dw_hdma_v0_core_ch_count will simply
return HDMA_V0_MAX_NR_CH.
Cai how does it works on your side? does the ch_en register already enabled by
the implementation?

2023-06-07 12:47:41

by Cai Huoqing

[permalink] [raw]

Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

On 07 6月 23 09:58:32, Köry Maincent wrote:
> On Sat, 20 May 2023 13:08:51 +0800
> Cai Huoqing <[email protected]> wrote:
>
> > Add support for HDMA NATIVE, as long the IP design has set
> > the compatible register map parameter-HDMA_NATIVE,
> > which allows compatibility for native HDMA register configuration.
>
> I know the patch has been merged in dmaengine tree but something seems weird on
> my side.
>
> The akida_dw_edma_probe function is selecting the minimum read and write
> channels by doing the minimum between ll_wr_cnt and the ch_count callback.
> The hdma ch_count callback is counting the number of channels enabled by reading
> the number of ch_en registers set. At probe time there is no channels registers
> that has been set as it is done later in the dw_hdma_v0_core_start function.
> Then the dw_hdma_v0_core_ch_count will always return 0 at probe time and the
> number of channels will be set to 0 which is not what we want.

will check it

Thanks,
Cai-
> Could I miss something?
>
> See the functions bellow:
>
> > int akida_dw_edma_probe(struct dw_edma_chip *chip)
> > {
> ...
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> >
> > dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> >
> > if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> > return -EINVAL;
> ...
> }
>
>
> > +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +{
> > + u32 num_ch = 0;
> > + int id;
> > +
> > + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> > + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> > + num_ch++;
> > + }
> > +
> > + if (num_ch > HDMA_V0_MAX_NR_CH)
> > + num_ch = HDMA_V0_MAX_NR_CH;
> > +
> > + return (u16)num_ch;
> > +}
>
>
>
> > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +{
> > + struct dw_edma_chan *chan = chunk->chan;
> > + struct dw_edma *dw = chan->dw;
> > + u32 tmp;
> > +
> > + dw_hdma_v0_core_write_chunk(chunk);
> > +
> > + if (first) {
> > + /* Enable engine */
> > + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> ...
> > +}
>

2023-06-07 13:20:33

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

On Wed, Jun 07, 2023 at 02:40:14PM +0200, K?ry Maincent wrote:
> On Wed, 7 Jun 2023 14:49:50 +0300
> Serge Semin <[email protected]> wrote:
>
> > Hi K?ry
> > First of all. What is akida platform you are referring to? I failed to
> > find any mention in the mainline kernel repo.
>
> Yes, sorry akida is the project prefix I am currently working on.
> It is simply a prefix for the symbol export to be different than mainline,
> don't take it into account.
>
> > > channels by doing the minimum between ll_wr_cnt and the ch_count callback.
> > > The hdma ch_count callback is counting the number of channels enabled by
> > > reading the number of ch_en registers set. At probe time there is no
> > > channels registers that has been set as it is done later in the
> > > dw_hdma_v0_core_start function. Then the dw_hdma_v0_core_ch_count will
> > > always return 0 at probe time and the number of channels will be set to 0
> > > which is not what we want. Could I miss something?
> >
> > Based on the HDMA patches content you are right. The channels must be
> > pre-enabled in order to have the dw_hdma_v0_core_ch_count() procedure
> > to work correctly otherwise you'll indeed end up with an empty list of
> > channels. I don't have any device with the HDMA engine embedded so I
> > couldn't have possibly tracked that peculiarity on review. Anyway
> > AFAICS Cai just implemented a method which seemed to work for his
> > hardware setup.
>
> Alright, on my side I have a board using this FPGA implementation and it
> indeed doesn't work as is.
>
> > If you think it doesn't work correctly or it isn't portable enough
> > then you are free to provide your own implementation of the method and
> > submit a patch. I hope Cai will be willing to test it out to make sure
> > that it works correctly for you and his platforms.
> >
> > As for me if I were on your place I would have implemented a loop
> > which would walk over all possible HDMA channels (HDMA_V0_MAX_NR_CH)
> > and counted all channels which could be enabled. If the ch_en CSR is
> > writable (that is the channel could be enabled) then it shall be
> > considered as existent. Of course before that I would have made sure
> > that the non-existent channels had non-writable ch_en CSR.
>

> This could be a nice idea but it doesn't work, non-existent channels seems to
> be writable. The datasheet of the HDMA IP doesn't have any register to find out
> the maximum existent channel.
> As there is no way to know this, the dw_hdma_v0_core_ch_count will simply
> return HDMA_V0_MAX_NR_CH.

What about some other CSRs (llp, etc)? Are they all writable even if
the respective channel doesn't exist?

If it isn't possible to auto-detect a number of channels then indeed
we need to come up with a strategy to rely on the platform settings
instead. Your suggestion seems to be the best choice seeing there is a
commit 16b90dd94d3f ("dmaengine: dw-edma: Improve number of channels
check") which converts the rely-on-platform-settings pattern to the
current rely-on-min-between-plat-and-hw-capability.

Cai?

-Serge(y)

> Cai how does it works on your side? does the ch_en register already enabled by
> the implementation?