Re: [PATCH 2/3] dmaengine: ae4dma: register AE4DMA controller as a DMA resource

From: Vinod Koul
Date: Wed Oct 04 2023 - 08:47:44 EST

Next message: Buddhabhatti, Jay: "RE: [PATCH v2 4/6] drivers: soc: xilinx: Fix error message on SGI registration failure"
Previous message: John Ogness: "Re: [PATCH printk] printk: flush consoles before checking progress"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 11-09-23, 14:25, Sanjay R Mehta wrote:
> From: Sanjay R Mehta <sanju.mehta@xxxxxxx>
>
> Register ae4dma queue's to Linux dmaengine framework as general-purpose
> DMA channels.
>
> Signed-off-by: Sanjay R Mehta <sanju.mehta@xxxxxxx>
> ---
> drivers/dma/ae4dma/Kconfig | 2 +
> drivers/dma/ae4dma/Makefile | 2 +-
> drivers/dma/ae4dma/ae4dma-dev.c | 17 ++
> drivers/dma/ae4dma/ae4dma-dmaengine.c | 425 ++++++++++++++++++++++++++++++++++
> drivers/dma/ae4dma/ae4dma.h | 28 +++
> 5 files changed, 473 insertions(+), 1 deletion(-)
> create mode 100644 drivers/dma/ae4dma/ae4dma-dmaengine.c
>
> diff --git a/drivers/dma/ae4dma/Kconfig b/drivers/dma/ae4dma/Kconfig
> index 1cda9de..3d1dbb7 100644
> --- a/drivers/dma/ae4dma/Kconfig
> +++ b/drivers/dma/ae4dma/Kconfig
> @@ -2,6 +2,8 @@
> config AMD_AE4DMA
> tristate "AMD AE4DMA Engine"
> depends on X86_64 && PCI
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> help
> Enabling this option provides support for the AMD AE4DMA controller,
> which offers DMA capabilities for high-bandwidth memory-to-memory and
> diff --git a/drivers/dma/ae4dma/Makefile b/drivers/dma/ae4dma/Makefile
> index db9cab1..b1e4318 100644
> --- a/drivers/dma/ae4dma/Makefile
> +++ b/drivers/dma/ae4dma/Makefile
> @@ -5,6 +5,6 @@
>
> obj-$(CONFIG_AMD_AE4DMA) += ae4dma.o
>
> -ae4dma-objs := ae4dma-dev.o
> +ae4dma-objs := ae4dma-dev.o ae4dma-dmaengine.o
>
> ae4dma-$(CONFIG_PCI) += ae4dma-pci.o
> diff --git a/drivers/dma/ae4dma/ae4dma-dev.c b/drivers/dma/ae4dma/ae4dma-dev.c
> index a3c50a2..af7e510 100644
> --- a/drivers/dma/ae4dma/ae4dma-dev.c
> +++ b/drivers/dma/ae4dma/ae4dma-dev.c
> @@ -25,6 +25,11 @@ static unsigned int max_hw_q = 1;
> module_param(max_hw_q, uint, 0444);
> MODULE_PARM_DESC(max_hw_q, "Max queues per engine (any non-zero value less than or equal to 16, default: 1)");
>
> +static inline struct ae4_dma_chan *to_ae4_chan(struct dma_chan *dma_chan)
> +{
> + return container_of(dma_chan, struct ae4_dma_chan, vc.chan);
> +}
> +
> /* Human-readable error strings */
> static char *ae4_error_codes[] = {
> "",
> @@ -273,8 +278,17 @@ int ae4_core_init(struct ae4_device *ae4)
> tasklet_setup(&cmd_q->irq_tasklet, ae4_do_cmd_complete);
> }
>
> + /* register to the DMA engine */
> + ret = ae4_dmaengine_register(ae4);
> + if (ret)
> + goto e_free_irq;
> +
> return 0;
>
> +e_free_irq:
> + for (i = 0; i < ae4->cmd_q_count; i++)
> + free_irq(ae4->ae4_irq[i], ae4);
> +
> e_free_dma:
> dma_free_coherent(dev, cmd_q->qsize, cmd_q->qbase, cmd_q->qbase_dma);
>
> @@ -292,6 +306,9 @@ void ae4_core_destroy(struct ae4_device *ae4)
> struct ae4_cmd *cmd;
> unsigned int i;
>
> + /* Unregister the DMA engine */
> + ae4_dmaengine_unregister(ae4);
> +
> for (i = 0; i < ae4->cmd_q_count; i++) {
> cmd_q = &ae4->cmd_q[i];
>
> diff --git a/drivers/dma/ae4dma/ae4dma-dmaengine.c b/drivers/dma/ae4dma/ae4dma-dmaengine.c
> new file mode 100644
> index 0000000..a50e4f5
> --- /dev/null
> +++ b/drivers/dma/ae4dma/ae4dma-dmaengine.c
> @@ -0,0 +1,425 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD AE4DMA device driver
> + * -- Based on the PTDMA driver

How different is it from PTDMA, why cant this be incorporated in ptdma
driver

> + *
> + * Copyright (C) 2023 Advanced Micro Devices, Inc.
> + *
> + * Author: Sanjay R Mehta <sanju.mehta@xxxxxxx>
> + */
> +#include <linux/delay.h>
> +#include "ae4dma.h"
> +#include "../dmaengine.h"
> +#include "../virt-dma.h"
> +
> +static inline struct ae4_dma_chan *to_ae4_chan(struct dma_chan *dma_chan)
> +{
> + return container_of(dma_chan, struct ae4_dma_chan, vc.chan);
> +}
> +
> +static inline struct ae4_dma_desc *to_ae4_desc(struct virt_dma_desc *vd)
> +{
> + return container_of(vd, struct ae4_dma_desc, vd);
> +}
> +
> +static void ae4_free_chan_resources(struct dma_chan *dma_chan)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> +
> + vchan_free_chan_resources(&chan->vc);
> +}
> +
> +static void ae4_synchronize(struct dma_chan *dma_chan)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> +
> + vchan_synchronize(&chan->vc);
> +}
> +
> +static void ae4_do_cleanup(struct virt_dma_desc *vd)
> +{
> + struct ae4_dma_desc *desc = to_ae4_desc(vd);
> + struct ae4_device *ae4 = desc->ae4;
> +
> + kmem_cache_free(ae4->dma_desc_cache, desc);
> +}
> +
> +static int ae4_dma_start_desc(struct ae4_dma_desc *desc, struct ae4_dma_chan *chan)
> +{
> + struct ae4_passthru_engine *ae4_engine;
> + struct ae4_device *ae4;
> + struct ae4_cmd *ae4_cmd;
> + struct ae4_cmd_queue *cmd_q;
> +
> + desc->issued_to_hw = 1;
> + list_del(&desc->vd.node);
> +
> + ae4_cmd = &desc->ae4_cmd;
> + ae4 = ae4_cmd->ae4;
> + cmd_q = chan->cmd_q;
> + ae4_engine = &ae4_cmd->passthru;
> +
> + ae4_cmd->qid = cmd_q->qidx;
> + cmd_q->tdata.cmd = ae4_cmd;
> +
> + /* Execute the command */
> + ae4_cmd->ret = ae4_core_perform_passthru(cmd_q, ae4_engine);
> +
> + return 0;
> +}
> +
> +static struct ae4_dma_desc *ae4_next_dma_desc(struct ae4_dma_chan *chan)
> +{
> + /* Get the next DMA descriptor on the active list */
> + struct virt_dma_desc *vd = vchan_next_desc(&chan->vc);
> +
> + return vd ? to_ae4_desc(vd) : NULL;
> +}
> +
> +static void ae4_cmd_callback_tasklet(void *data, int err)
> +{
> + struct ae4_dma_desc *desc = data;
> + struct dma_chan *dma_chan;
> + struct ae4_dma_chan *chan;
> + struct dma_async_tx_descriptor *tx_desc;
> + struct virt_dma_desc *vd;
> + unsigned long flags;
> +
> + dma_chan = desc->vd.tx.chan;
> + chan = to_ae4_chan(dma_chan);
> +
> + if (err == -EINPROGRESS)
> + return;
> +
> + tx_desc = &desc->vd.tx;
> + vd = &desc->vd;
> +
> + if (err)
> + desc->status = DMA_ERROR;
> +
> + spin_lock_irqsave(&chan->vc.lock, flags);
> + if (desc) {
> + if (desc->status != DMA_COMPLETE) {
> + if (desc->status != DMA_ERROR)
> + desc->status = DMA_COMPLETE;

So a DMA_IN_PROGRESS descriptor would be marked complete?

> +
> + dma_cookie_complete(tx_desc);
> + dma_descriptor_unmap(tx_desc);
> + } else {
> + /* Don't handle it twice */
> + tx_desc = NULL;
> + }
> + }
> + spin_unlock_irqrestore(&chan->vc.lock, flags);
> +
> + if (tx_desc) {
> + dmaengine_desc_get_callback_invoke(tx_desc, NULL);
> + dma_run_dependencies(tx_desc);
> + }
> +}
> +
> +static struct ae4_dma_desc *ae4_handle_active_desc(struct ae4_dma_chan *chan,
> + struct ae4_dma_desc *desc)
> +{
> + struct dma_async_tx_descriptor *tx_desc;
> + struct virt_dma_desc *vd;
> + unsigned long flags;
> +
> + /* Loop over descriptors until one is found with commands */
> + do {
> + if (desc) {
> + if (!desc->issued_to_hw) {
> + /* No errors, keep going */
> + if (desc->status != DMA_ERROR)
> + return desc;
> + }
> + tx_desc = &desc->vd.tx;
> + vd = &desc->vd;
> + } else {
> + tx_desc = NULL;
> + }
> + spin_lock_irqsave(&chan->vc.lock, flags);
> + desc = ae4_next_dma_desc(chan);
> + spin_unlock_irqrestore(&chan->vc.lock, flags);
> + } while (desc);
> +
> + return NULL;
> +}
> +
> +static void ae4_cmd_callback(void *data, int err)
> +{
> + struct ae4_dma_desc *desc = data;
> + struct dma_chan *dma_chan;
> + struct ae4_dma_chan *chan;
> + struct ae4_device *ae4;
> + int ret;
> +
> + if (err == -EINPROGRESS)
> + return;
> +
> + dma_chan = desc->vd.tx.chan;
> + chan = to_ae4_chan(dma_chan);
> + ae4 = chan->ae4;
> +
> + if (err)
> + desc->status = DMA_ERROR;
> +
> + while (true) {
> + /* Check for DMA descriptor completion */
> + desc = ae4_handle_active_desc(chan, desc);
> +
> + /* Don't submit cmd if no descriptor or DMA is paused */
> + if (!desc)
> + break;
> +
> + /* if queue is full dont submit to queue */
> + if (ae4_core_queue_full(ae4, chan->cmd_q)) {
> + usleep_range(100, 200);
> + continue;
> + }
> +
> + ret = ae4_dma_start_desc(desc, chan);
> + if (!ret)
> + break;
> +
> + desc->status = DMA_ERROR;
> + }
> +}
> +
> +static struct ae4_dma_desc *ae4_alloc_dma_desc(struct ae4_dma_chan *chan, unsigned long flags)
> +{
> + struct ae4_dma_desc *desc;
> + struct ae4_cmd_queue *cmd_q = chan->cmd_q;
> +
> + desc = kmem_cache_zalloc(chan->ae4->dma_desc_cache, GFP_NOWAIT);
> + if (!desc)
> + return NULL;
> +
> + vchan_tx_prep(&chan->vc, &desc->vd, flags);
> +
> + desc->ae4 = chan->ae4;
> + cmd_q->int_en = !!(flags & DMA_PREP_INTERRUPT);
> + desc->issued_to_hw = 0;
> + desc->status = DMA_IN_PROGRESS;
> +
> + return desc;
> +}
> +
> +static struct ae4_dma_desc *ae4_create_desc(struct dma_chan *dma_chan,
> + dma_addr_t dst,
> + dma_addr_t src,
> + unsigned int len,
> + unsigned long flags)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> + struct ae4_cmd_queue *cmd_q = chan->cmd_q;
> + struct ae4_passthru_engine *ae4_engine;
> + struct ae4_dma_desc *desc;
> + struct ae4_cmd *ae4_cmd;
> +
> + desc = ae4_alloc_dma_desc(chan, flags);
> + if (!desc)
> + return NULL;
> +
> + ae4_cmd = &desc->ae4_cmd;
> + ae4_cmd->ae4 = chan->ae4;
> + ae4_engine = &ae4_cmd->passthru;
> + ae4_cmd->engine = AE4_ENGINE_PASSTHRU;
> + ae4_engine->src_dma = src;
> + ae4_engine->dst_dma = dst;
> + ae4_engine->src_len = len;
> + ae4_cmd->ae4_cmd_callback = ae4_cmd_callback_tasklet;
> + ae4_cmd->data = desc;
> +
> + desc->len = len;
> +
> + spin_lock_irqsave(&cmd_q->cmd_lock, flags);
> + list_add_tail(&ae4_cmd->entry, &cmd_q->cmd);
> + spin_unlock_irqrestore(&cmd_q->cmd_lock, flags);
> +
> + return desc;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +ae4_prep_dma_memcpy(struct dma_chan *dma_chan, dma_addr_t dst,
> + dma_addr_t src, size_t len, unsigned long flags)
> +{
> + struct ae4_dma_desc *desc;
> +
> + desc = ae4_create_desc(dma_chan, dst, src, len, flags);
> + if (!desc)
> + return NULL;

I dont see any logic in splitting these into two!

> +
> + return &desc->vd.tx;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +ae4_prep_dma_interrupt(struct dma_chan *dma_chan, unsigned long flags)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> + struct ae4_dma_desc *desc;
> +
> + desc = ae4_alloc_dma_desc(chan, flags);
> + if (!desc)
> + return NULL;
> +
> + return &desc->vd.tx;
> +}
> +
> +static void ae4_issue_pending(struct dma_chan *dma_chan)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> + struct ae4_dma_desc *desc;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->vc.lock, flags);
> + vchan_issue_pending(&chan->vc);
> + desc = ae4_next_dma_desc(chan);
> + spin_unlock_irqrestore(&chan->vc.lock, flags);
> +
> + ae4_cmd_callback(desc, 0);
> +}
> +
> +static int ae4_pause(struct dma_chan *dma_chan)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->vc.lock, flags);
> + //TODO : implemnt pause

typo implemnt and drop this and add when you add support!
No dummy code please

> + spin_unlock_irqrestore(&chan->vc.lock, flags);
> +
> + return 0;
> +}
> +
> +static int ae4_resume(struct dma_chan *dma_chan)
> +{
> + struct ae4_dma_chan *chan = to_ae4_chan(dma_chan);
> + struct ae4_dma_desc *desc = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->vc.lock, flags);
> + ae4_start_queue(chan->cmd_q);
> + desc = ae4_next_dma_desc(chan);
> + spin_unlock_irqrestore(&chan->vc.lock, flags);
> +
> + /* If there was something active, re-start */
> + if (desc)
> + ae4_cmd_callback(desc, 0);

resume when no pause is supported..??
--
~Vinod

Next message: Buddhabhatti, Jay: "RE: [PATCH v2 4/6] drivers: soc: xilinx: Fix error message on SGI registration failure"
Previous message: John Ogness: "Re: [PATCH printk] printk: flush consoles before checking progress"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]