2022-11-24 10:32:09

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 0/6] Xilinx DMA enhancements and optimization

Some background about the patch series: Xilinx Axi Ethernet device driver
(xilinx_axienet_main.c) currently has axi-dma code inside it. The goal is
to refactor axiethernet driver and use existing AXI DMA driver using
DMAEngine API.

This patchset does feature addition and optimization to support axidma
integration with axiethernet network driver. Once axidma version is
accepted mcdma specific changes will be added in followup version.

Changes in V2:
1) Moved xlnx,axistream-connected optional property to under AXI DMA.
2) Removed Acked-by: Rob Herringi for PATCH 1/6.
3) New patch(6/6).

Comments, suggestions are very welcome!

Radhey Shyam Pandey (6):
dt-bindings: dmaengine: xilinx_dma:Add xlnx,axistream-connected
property
dt-bindings: dmaengine: xilinx_dma: Add xlnx,irq-delay property
dmaengine: xilinx_dma: Pass AXI4-Stream control words to dma client
dmaengine: xilinx_dma: Increase AXI DMA transaction segment count
dmaengine: xilinx_dma: Use tasklet_hi_schedule for timing critical
usecase
dmaengine: xilinx_dma: Program interrupt delay timeout

.../bindings/dma/xilinx/xilinx_dma.txt | 6 ++
drivers/dma/xilinx/xilinx_dma.c | 61 +++++++++++++++----
2 files changed, 56 insertions(+), 11 deletions(-)

--
2.25.1


2022-11-24 10:32:59

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 4/6] dmaengine: xilinx_dma: Increase AXI DMA transaction segment count

From: Radhey Shyam Pandey <[email protected]>

Increase AXI DMA transaction segments count to ensure that even in
high load we always get a free segment in prepare descriptor for a
DMA_SLAVE transaction.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
drivers/dma/xilinx/xilinx_dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index f783ba86cb09..6780b1c21414 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -178,7 +178,7 @@
#define XILINX_DMA_BD_SOP BIT(27)
#define XILINX_DMA_BD_EOP BIT(26)
#define XILINX_DMA_COALESCE_MAX 255
-#define XILINX_DMA_NUM_DESCS 255
+#define XILINX_DMA_NUM_DESCS 512
#define XILINX_DMA_NUM_APP_WORDS 5

/* AXI CDMA Specific Registers/Offsets */
--
2.25.1

2022-11-24 10:34:30

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 1/6] dt-bindings: dmaengine: xilinx_dma:Add xlnx,axistream-connected property

From: Radhey Shyam Pandey <[email protected]>

Add an optional AXI DMA property 'xlnx,axistream-connected'. This
can be specified to indicate that DMA is connected to a streaming IP
in the hardware design and dma driver needs to do some additional
handling i.e pass metadata and perform streaming IP specific
configuration.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
---
Changes in V2:
1) Moved xlnx,axistream-connected optional property to under AXI DMA.
2) Removed Acked-by: Rob Herring.
---
Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index d1700a5c36bf..fea5b09a439d 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -49,6 +49,10 @@ Optional properties for AXI DMA and MCDMA:
register as configured in h/w. Takes values {8...26}. If the property
is missing or invalid then the default value 23 is used. This is the
maximum value that is supported by all IP versions.
+
+Optional properties for AXI DMA:
+- xlnx,axistream-connected: Tells whether DMA is connected to AXI stream IP.
+
Optional properties for VDMA:
- xlnx,flush-fsync: Tells which channel to Flush on Frame sync.
It takes following values:
--
2.25.1

2022-11-24 10:47:30

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 5/6] dmaengine: xilinx_dma: Use tasklet_hi_schedule for timing critical usecase

From: Radhey Shyam Pandey <[email protected]>

Schedule tasklet with high priority to ensure that callback processing
is prioritized. It improves throughput for netdev dma clients.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
drivers/dma/xilinx/xilinx_dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 6780b1c21414..ce0c151d8f61 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1839,7 +1839,7 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
spin_unlock(&chan->lock);
}

- tasklet_schedule(&chan->tasklet);
+ tasklet_hi_schedule(&chan->tasklet);
return IRQ_HANDLED;
}

--
2.25.1

2022-11-24 10:48:12

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt delay timeout

From: Radhey Shyam Pandey <[email protected]>

Program IRQDelay for AXI DMA. The interrupt timeout mechanism causes
the DMA engine to generate an interrupt after the delay time period
has expired. It enables dmaengine to respond in real-time even though
interrupt coalescing is configured. It also remove the placeholder
for delay interrupt and merge it with frame completion interrupt.
Since by default interrupt delay timeout is disabled this feature
addition has no functional impact on VDMA and CDMA IP's.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
---
drivers/dma/xilinx/xilinx_dma.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index ce0c151d8f61..333d68ee3559 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -173,8 +173,10 @@
#define XILINX_DMA_MAX_TRANS_LEN_MAX 23
#define XILINX_DMA_V2_MAX_TRANS_LEN_MAX 26
#define XILINX_DMA_CR_COALESCE_MAX GENMASK(23, 16)
+#define XILINX_DMA_CR_DELAY_MAX GENMASK(31, 24)
#define XILINX_DMA_CR_CYCLIC_BD_EN_MASK BIT(4)
#define XILINX_DMA_CR_COALESCE_SHIFT 16
+#define XILINX_DMA_CR_DELAY_SHIFT 24
#define XILINX_DMA_BD_SOP BIT(27)
#define XILINX_DMA_BD_EOP BIT(26)
#define XILINX_DMA_COALESCE_MAX 255
@@ -410,6 +412,7 @@ struct xilinx_dma_tx_descriptor {
* @stop_transfer: Differentiate b/w DMA IP's quiesce
* @tdest: TDEST value for mcdma
* @has_vflip: S2MM vertical flip
+ * @irq_delay: Interrupt delay timeout
*/
struct xilinx_dma_chan {
struct xilinx_dma_device *xdev;
@@ -448,6 +451,7 @@ struct xilinx_dma_chan {
int (*stop_transfer)(struct xilinx_dma_chan *chan);
u16 tdest;
bool has_vflip;
+ u8 irq_delay;
};

/**
@@ -1560,6 +1564,9 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->has_sg)
xilinx_write(chan, XILINX_DMA_REG_CURDESC,
head_desc->async_tx.phys);
+ reg &= ~XILINX_DMA_CR_DELAY_MAX;
+ reg |= chan->irq_delay << XILINX_DMA_CR_DELAY_SHIFT;
+ dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);

xilinx_dma_start(chan);

@@ -1887,15 +1894,8 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
}
}

- if (status & XILINX_DMA_DMASR_DLY_CNT_IRQ) {
- /*
- * Device takes too long to do the transfer when user requires
- * responsiveness.
- */
- dev_dbg(chan->dev, "Inter-packet latency too long\n");
- }
-
- if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
+ if (status & (XILINX_DMA_DMASR_FRM_CNT_IRQ |
+ XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
spin_lock(&chan->lock);
xilinx_dma_complete_descriptor(chan);
chan->idle = true;
@@ -2822,6 +2822,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
/* Retrieve the channel properties from the device tree */
has_dre = of_property_read_bool(node, "xlnx,include-dre");

+ of_property_read_u8(node, "xlnx,irq-delay", &chan->irq_delay);
+
chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");

err = of_property_read_u32(node, "xlnx,datawidth", &value);
--
2.25.1

2022-11-24 11:03:02

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 3/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to dma client

From: Radhey Shyam Pandey <[email protected]>

Read DT property to check if AXI DMA is connected to streaming IP
i.e axiethernet. If connected pass AXI4-Stream control words to
dma client using metadata_ops dmaengine API.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
drivers/dma/xilinx/xilinx_dma.c | 37 +++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8cd4e69dc7b4..f783ba86cb09 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -493,6 +493,7 @@ struct xilinx_dma_config {
* @s2mm_chan_id: DMA s2mm channel identifier
* @mm2s_chan_id: DMA mm2s channel identifier
* @max_buffer_len: Max buffer length
+ * @has_axistream_connected: AXI DMA connected to AXI Stream IP
*/
struct xilinx_dma_device {
void __iomem *regs;
@@ -511,6 +512,7 @@ struct xilinx_dma_device {
u32 s2mm_chan_id;
u32 mm2s_chan_id;
u32 max_buffer_len;
+ bool has_axistream_connected;
};

/* Macros */
@@ -623,6 +625,29 @@ static inline void xilinx_aximcdma_buf(struct xilinx_dma_chan *chan,
}
}

+/**
+ * xilinx_dma_get_metadata_ptr- Populate metadata pointer and payload length
+ * @tx: async transaction descriptor
+ * @payload_len: metadata payload length
+ * @max_len: metadata max length
+ * Return: The app field pointer.
+ */
+static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
+ size_t *payload_len, size_t *max_len)
+{
+ struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
+ struct xilinx_axidma_tx_segment *seg;
+
+ *max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
+ seg = list_first_entry(&desc->segments,
+ struct xilinx_axidma_tx_segment, node);
+ return seg->hw.app;
+}
+
+static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
+ .get_ptr = xilinx_dma_get_metadata_ptr,
+};
+
/* -----------------------------------------------------------------------------
* Descriptors and segments alloc and free
*/
@@ -2219,6 +2244,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
segment->hw.control |= XILINX_DMA_BD_EOP;
}

+ if (chan->xdev->has_axistream_connected)
+ desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
+
return &desc->async_tx;

error:
@@ -3065,6 +3093,11 @@ static int xilinx_dma_probe(struct platform_device *pdev)
}
}

+ if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
+ xdev->has_axistream_connected =
+ of_property_read_bool(node, "xlnx,axistream-connected");
+ }
+
if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
err = of_property_read_u32(node, "xlnx,num-fstores",
&num_frames);
@@ -3090,6 +3123,10 @@ static int xilinx_dma_probe(struct platform_device *pdev)
else
xdev->ext_addr = false;

+ /* Set metadata mode */
+ if (xdev->has_axistream_connected)
+ xdev->common.desc_metadata_modes = DESC_METADATA_ENGINE;
+
/* Set the dma mask bits */
err = dma_set_mask_and_coherent(xdev->dev, DMA_BIT_MASK(addr_width));
if (err < 0) {
--
2.25.1

2022-11-24 11:08:40

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: [PATCH V2 2/6] dt-bindings: dmaengine: xilinx_dma: Add xlnx,irq-delay property

From: Radhey Shyam Pandey <[email protected]>

Add an optional AXI DMA property 'xlnx,irq-delay'. It specifies interrupt
timeout value and causes the DMA engine to generate an interrupt after the
delay time period has expired. Timer begins counting at the end of a packet
and resets with receipt of a new packet or a timeout event occurs.

This property is useful when AXI DMA is connected to the streaming IP i.e
axiethernet where inter packet latency is critical while still taking the
benefit of interrupt coalescing.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
---
Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index fea5b09a439d..590d1948f202 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -52,7 +52,9 @@ Optional properties for AXI DMA and MCDMA:

Optional properties for AXI DMA:
- xlnx,axistream-connected: Tells whether DMA is connected to AXI stream IP.
-
+- xlnx,irq-delay: Tells the interrupt delay timeout value. Valid range is from
+ 0-255. Setting this value to zero disables the delay timer interrupt.
+ 1 timeout interval = 125 * clock period of SG clock.
Optional properties for VDMA:
- xlnx,flush-fsync: Tells which channel to Flush on Frame sync.
It takes following values:
--
2.25.1

2022-11-26 15:53:52

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V2 1/6] dt-bindings: dmaengine: xilinx_dma:Add xlnx,axistream-connected property

On 24/11/2022 11:27, Sarath Babu Naidu Gaddam wrote:
> From: Radhey Shyam Pandey <[email protected]>
>
> Add an optional AXI DMA property 'xlnx,axistream-connected'. This
> can be specified to indicate that DMA is connected to a streaming IP
> in the hardware design and dma driver needs to do some additional
> handling i.e pass metadata and perform streaming IP specific
> configuration.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
> ---
> Changes in V2:
> 1) Moved xlnx,axistream-connected optional property to under AXI DMA.
> 2) Removed Acked-by: Rob Herring.
> ---

You already add two properties here. Convert to DT schema and then add.

Best regards,
Krzysztof

2022-11-30 22:00:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 1/6] dt-bindings: dmaengine: xilinx_dma:Add xlnx,axistream-connected property

On Sat, Nov 26, 2022 at 03:32:23PM +0100, Krzysztof Kozlowski wrote:
> On 24/11/2022 11:27, Sarath Babu Naidu Gaddam wrote:
> > From: Radhey Shyam Pandey <[email protected]>
> >
> > Add an optional AXI DMA property 'xlnx,axistream-connected'. This
> > can be specified to indicate that DMA is connected to a streaming IP
> > in the hardware design and dma driver needs to do some additional
> > handling i.e pass metadata and perform streaming IP specific
> > configuration.
> >
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
> > Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
> > ---
> > Changes in V2:
> > 1) Moved xlnx,axistream-connected optional property to under AXI DMA.
> > 2) Removed Acked-by: Rob Herring.
> > ---
>
> You already add two properties here. Convert to DT schema and then add.

That would be better, but I did ack it before. Though that was a year
and half ago...

Rob

2022-11-30 22:08:30

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 1/6] dt-bindings: dmaengine: xilinx_dma:Add xlnx,axistream-connected property


On Thu, 24 Nov 2022 15:57:40 +0530, Sarath Babu Naidu Gaddam wrote:
> From: Radhey Shyam Pandey <[email protected]>
>
> Add an optional AXI DMA property 'xlnx,axistream-connected'. This
> can be specified to indicate that DMA is connected to a streaming IP
> in the hardware design and dma driver needs to do some additional
> handling i.e pass metadata and perform streaming IP specific
> configuration.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
> ---
> Changes in V2:
> 1) Moved xlnx,axistream-connected optional property to under AXI DMA.
> 2) Removed Acked-by: Rob Herring.
> ---
> Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2022-12-28 11:26:21

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V2 3/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to dma client

On 24-11-22, 15:57, Sarath Babu Naidu Gaddam wrote:
> From: Radhey Shyam Pandey <[email protected]>
>
> Read DT property to check if AXI DMA is connected to streaming IP
> i.e axiethernet. If connected pass AXI4-Stream control words to
> dma client using metadata_ops dmaengine API.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> ---
> drivers/dma/xilinx/xilinx_dma.c | 37 +++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 8cd4e69dc7b4..f783ba86cb09 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -493,6 +493,7 @@ struct xilinx_dma_config {
> * @s2mm_chan_id: DMA s2mm channel identifier
> * @mm2s_chan_id: DMA mm2s channel identifier
> * @max_buffer_len: Max buffer length
> + * @has_axistream_connected: AXI DMA connected to AXI Stream IP
> */
> struct xilinx_dma_device {
> void __iomem *regs;
> @@ -511,6 +512,7 @@ struct xilinx_dma_device {
> u32 s2mm_chan_id;
> u32 mm2s_chan_id;
> u32 max_buffer_len;
> + bool has_axistream_connected;
> };
>
> /* Macros */
> @@ -623,6 +625,29 @@ static inline void xilinx_aximcdma_buf(struct xilinx_dma_chan *chan,
> }
> }
>
> +/**
> + * xilinx_dma_get_metadata_ptr- Populate metadata pointer and payload length
> + * @tx: async transaction descriptor
> + * @payload_len: metadata payload length
> + * @max_len: metadata max length
> + * Return: The app field pointer.
> + */
> +static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
> + size_t *payload_len, size_t *max_len)
> +{
> + struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
> + struct xilinx_axidma_tx_segment *seg;
> +
> + *max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> + seg = list_first_entry(&desc->segments,
> + struct xilinx_axidma_tx_segment, node);
> + return seg->hw.app;
> +}
> +
> +static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
> + .get_ptr = xilinx_dma_get_metadata_ptr,
> +};
> +
> /* -----------------------------------------------------------------------------
> * Descriptors and segments alloc and free
> */
> @@ -2219,6 +2244,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> segment->hw.control |= XILINX_DMA_BD_EOP;
> }
>
> + if (chan->xdev->has_axistream_connected)
> + desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;

This is an optional property which is added now, what will happen if you
are on a system with older DT? This wont work there..

> +
> return &desc->async_tx;
>
> error:
> @@ -3065,6 +3093,11 @@ static int xilinx_dma_probe(struct platform_device *pdev)
> }
> }
>
> + if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> + xdev->has_axistream_connected =
> + of_property_read_bool(node, "xlnx,axistream-connected");
> + }
> +
> if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> err = of_property_read_u32(node, "xlnx,num-fstores",
> &num_frames);
> @@ -3090,6 +3123,10 @@ static int xilinx_dma_probe(struct platform_device *pdev)
> else
> xdev->ext_addr = false;
>
> + /* Set metadata mode */
> + if (xdev->has_axistream_connected)
> + xdev->common.desc_metadata_modes = DESC_METADATA_ENGINE;
> +
> /* Set the dma mask bits */
> err = dma_set_mask_and_coherent(xdev->dev, DMA_BIT_MASK(addr_width));
> if (err < 0) {
> --
> 2.25.1

--
~Vinod

2022-12-28 12:00:52

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt delay timeout

On 24-11-22, 15:57, Sarath Babu Naidu Gaddam wrote:
> From: Radhey Shyam Pandey <[email protected]>
>
> Program IRQDelay for AXI DMA. The interrupt timeout mechanism causes
> the DMA engine to generate an interrupt after the delay time period
> has expired. It enables dmaengine to respond in real-time even though
> interrupt coalescing is configured. It also remove the placeholder
> for delay interrupt and merge it with frame completion interrupt.
> Since by default interrupt delay timeout is disabled this feature
> addition has no functional impact on VDMA and CDMA IP's.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
> ---
> drivers/dma/xilinx/xilinx_dma.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ce0c151d8f61..333d68ee3559 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -173,8 +173,10 @@
> #define XILINX_DMA_MAX_TRANS_LEN_MAX 23
> #define XILINX_DMA_V2_MAX_TRANS_LEN_MAX 26
> #define XILINX_DMA_CR_COALESCE_MAX GENMASK(23, 16)
> +#define XILINX_DMA_CR_DELAY_MAX GENMASK(31, 24)
> #define XILINX_DMA_CR_CYCLIC_BD_EN_MASK BIT(4)
> #define XILINX_DMA_CR_COALESCE_SHIFT 16
> +#define XILINX_DMA_CR_DELAY_SHIFT 24
> #define XILINX_DMA_BD_SOP BIT(27)
> #define XILINX_DMA_BD_EOP BIT(26)
> #define XILINX_DMA_COALESCE_MAX 255
> @@ -410,6 +412,7 @@ struct xilinx_dma_tx_descriptor {
> * @stop_transfer: Differentiate b/w DMA IP's quiesce
> * @tdest: TDEST value for mcdma
> * @has_vflip: S2MM vertical flip
> + * @irq_delay: Interrupt delay timeout
> */
> struct xilinx_dma_chan {
> struct xilinx_dma_device *xdev;
> @@ -448,6 +451,7 @@ struct xilinx_dma_chan {
> int (*stop_transfer)(struct xilinx_dma_chan *chan);
> u16 tdest;
> bool has_vflip;
> + u8 irq_delay;
> };
>
> /**
> @@ -1560,6 +1564,9 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
> if (chan->has_sg)
> xilinx_write(chan, XILINX_DMA_REG_CURDESC,
> head_desc->async_tx.phys);
> + reg &= ~XILINX_DMA_CR_DELAY_MAX;
> + reg |= chan->irq_delay << XILINX_DMA_CR_DELAY_SHIFT;
> + dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>
> xilinx_dma_start(chan);
>
> @@ -1887,15 +1894,8 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
> }
> }
>
> - if (status & XILINX_DMA_DMASR_DLY_CNT_IRQ) {
> - /*
> - * Device takes too long to do the transfer when user requires
> - * responsiveness.
> - */
> - dev_dbg(chan->dev, "Inter-packet latency too long\n");
> - }
> -
> - if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
> + if (status & (XILINX_DMA_DMASR_FRM_CNT_IRQ |
> + XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
> spin_lock(&chan->lock);
> xilinx_dma_complete_descriptor(chan);
> chan->idle = true;
> @@ -2822,6 +2822,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
> /* Retrieve the channel properties from the device tree */
> has_dre = of_property_read_bool(node, "xlnx,include-dre");
>
> + of_property_read_u8(node, "xlnx,irq-delay", &chan->irq_delay);

Same question here too

> +
> chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");
>
> err = of_property_read_u32(node, "xlnx,datawidth", &value);
> --
> 2.25.1

--
~Vinod

2023-02-02 07:39:20

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: RE: [PATCH V2 3/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to dma client



> -----Original Message-----
> From: Vinod Koul <[email protected]>
> Sent: Wednesday, December 28, 2022 4:30 PM
> To: Gaddam, Sarath Babu Naidu
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; Simek, Michal
> <[email protected]>; Pandey, Radhey Shyam
> <[email protected]>; Sarangi, Anirudha
> <[email protected]>; Katakam, Harini
> <[email protected]>; git (AMD-Xilinx) <[email protected]>
> Subject: Re: [PATCH V2 3/6] dmaengine: xilinx_dma: Pass AXI4-Stream
> control words to dma client
>
> On 24-11-22, 15:57, Sarath Babu Naidu Gaddam wrote:
> > From: Radhey Shyam Pandey <[email protected]>
> >
> > Read DT property to check if AXI DMA is connected to streaming IP i.e
> > axiethernet. If connected pass AXI4-Stream control words to dma client
> > using metadata_ops dmaengine API.
> >
> > Signed-off-by: Radhey Shyam Pandey
> <[email protected]>
> > ---
> > drivers/dma/xilinx/xilinx_dma.c | 37
> > +++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> > b/drivers/dma/xilinx/xilinx_dma.c index 8cd4e69dc7b4..f783ba86cb09
> > 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -493,6 +493,7 @@ struct xilinx_dma_config {
> > * @s2mm_chan_id: DMA s2mm channel identifier
> > * @mm2s_chan_id: DMA mm2s channel identifier
> > * @max_buffer_len: Max buffer length
> > + * @has_axistream_connected: AXI DMA connected to AXI Stream IP
> > */
> > struct xilinx_dma_device {
> > void __iomem *regs;
> > @@ -511,6 +512,7 @@ struct xilinx_dma_device {
> > u32 s2mm_chan_id;
> > u32 mm2s_chan_id;
> > u32 max_buffer_len;
> > + bool has_axistream_connected;
> > };
> >
> > /* Macros */
> > @@ -623,6 +625,29 @@ static inline void xilinx_aximcdma_buf(struct
> xilinx_dma_chan *chan,
> > }
> > }
> >
> > +/**
> > + * xilinx_dma_get_metadata_ptr- Populate metadata pointer and
> payload
> > +length
> > + * @tx: async transaction descriptor
> > + * @payload_len: metadata payload length
> > + * @max_len: metadata max length
> > + * Return: The app field pointer.
> > + */
> > +static void *xilinx_dma_get_metadata_ptr(struct
> dma_async_tx_descriptor *tx,
> > + size_t *payload_len, size_t
> *max_len) {
> > + struct xilinx_dma_tx_descriptor *desc =
> to_dma_tx_descriptor(tx);
> > + struct xilinx_axidma_tx_segment *seg;
> > +
> > + *max_len = *payload_len = sizeof(u32) *
> XILINX_DMA_NUM_APP_WORDS;
> > + seg = list_first_entry(&desc->segments,
> > + struct xilinx_axidma_tx_segment, node);
> > + return seg->hw.app;
> > +}
> > +
> > +static struct dma_descriptor_metadata_ops
> xilinx_dma_metadata_ops = {
> > + .get_ptr = xilinx_dma_get_metadata_ptr, };
> > +
> > /* -----------------------------------------------------------------------------
> > * Descriptors and segments alloc and free
> > */
> > @@ -2219,6 +2244,9 @@ static struct dma_async_tx_descriptor
> *xilinx_dma_prep_slave_sg(
> > segment->hw.control |= XILINX_DMA_BD_EOP;
> > }
> >
> > + if (chan->xdev->has_axistream_connected)
> > + desc->async_tx.metadata_ops =
> &xilinx_dma_metadata_ops;
>
> This is an optional property which is added now, what will happen if you
> are on a system with older DT? This wont work there..

Sorry, we missed this comment. If this optional property is not there,
then driver does not export "metadata_ops" APIs and it does not
break any existing functionality.

Thanks,
Sarath

>
> > +
> > return &desc->async_tx;
> >
> > error:
> > @@ -3065,6 +3093,11 @@ static int xilinx_dma_probe(struct
> platform_device *pdev)
> > }
> > }
> >
> > + if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> > + xdev->has_axistream_connected =
> > + of_property_read_bool(node, "xlnx,axistream-
> connected");
> > + }
> > +
> > if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> > err = of_property_read_u32(node, "xlnx,num-fstores",
> > &num_frames);
> > @@ -3090,6 +3123,10 @@ static int xilinx_dma_probe(struct
> platform_device *pdev)
> > else
> > xdev->ext_addr = false;
> >
> > + /* Set metadata mode */
> > + if (xdev->has_axistream_connected)
> > + xdev->common.desc_metadata_modes =
> DESC_METADATA_ENGINE;
> > +
> > /* Set the dma mask bits */
> > err = dma_set_mask_and_coherent(xdev->dev,
> DMA_BIT_MASK(addr_width));
> > if (err < 0) {
> > --
> > 2.25.1
>
> --
> ~Vinod

2023-02-02 07:52:09

by Sarath Babu Naidu Gaddam

[permalink] [raw]
Subject: RE: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt delay timeout



> -----Original Message-----
> From: Vinod Koul <[email protected]>
> Sent: Wednesday, December 28, 2022 4:31 PM
> To: Gaddam, Sarath Babu Naidu
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; Simek, Michal
> <[email protected]>; Pandey, Radhey Shyam
> <[email protected]>; Sarangi, Anirudha
> <[email protected]>; Katakam, Harini
> <[email protected]>; git (AMD-Xilinx) <[email protected]>
> Subject: Re: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt
> delay timeout
>
> On 24-11-22, 15:57, Sarath Babu Naidu Gaddam wrote:
> > From: Radhey Shyam Pandey <[email protected]>
> >
> > Program IRQDelay for AXI DMA. The interrupt timeout mechanism
> causes
> > the DMA engine to generate an interrupt after the delay time period
> > has expired. It enables dmaengine to respond in real-time even though
> > interrupt coalescing is configured. It also remove the placeholder for
> > delay interrupt and merge it with frame completion interrupt.
> > Since by default interrupt delay timeout is disabled this feature
> > addition has no functional impact on VDMA and CDMA IP's.
> >
> > Signed-off-by: Radhey Shyam Pandey
> <[email protected]>
> > Signed-off-by: Sarath Babu Naidu Gaddam
> > <[email protected]>
> > ---
> > drivers/dma/xilinx/xilinx_dma.c | 20 +++++++++++---------
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> > b/drivers/dma/xilinx/xilinx_dma.c index ce0c151d8f61..333d68ee3559
> > 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -173,8 +173,10 @@
> > #define XILINX_DMA_MAX_TRANS_LEN_MAX 23
> > #define XILINX_DMA_V2_MAX_TRANS_LEN_MAX 26
> > #define XILINX_DMA_CR_COALESCE_MAX GENMASK(23, 16)
> > +#define XILINX_DMA_CR_DELAY_MAX GENMASK(31, 24)
> > #define XILINX_DMA_CR_CYCLIC_BD_EN_MASK BIT(4)
> > #define XILINX_DMA_CR_COALESCE_SHIFT 16
> > +#define XILINX_DMA_CR_DELAY_SHIFT 24
> > #define XILINX_DMA_BD_SOP BIT(27)
> > #define XILINX_DMA_BD_EOP BIT(26)
> > #define XILINX_DMA_COALESCE_MAX 255
> > @@ -410,6 +412,7 @@ struct xilinx_dma_tx_descriptor {
> > * @stop_transfer: Differentiate b/w DMA IP's quiesce
> > * @tdest: TDEST value for mcdma
> > * @has_vflip: S2MM vertical flip
> > + * @irq_delay: Interrupt delay timeout
> > */
> > struct xilinx_dma_chan {
> > struct xilinx_dma_device *xdev;
> > @@ -448,6 +451,7 @@ struct xilinx_dma_chan {
> > int (*stop_transfer)(struct xilinx_dma_chan *chan);
> > u16 tdest;
> > bool has_vflip;
> > + u8 irq_delay;
> > };
> >
> > /**
> > @@ -1560,6 +1564,9 @@ static void xilinx_dma_start_transfer(struct
> xilinx_dma_chan *chan)
> > if (chan->has_sg)
> > xilinx_write(chan, XILINX_DMA_REG_CURDESC,
> > head_desc->async_tx.phys);
> > + reg &= ~XILINX_DMA_CR_DELAY_MAX;
> > + reg |= chan->irq_delay << XILINX_DMA_CR_DELAY_SHIFT;
> > + dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
> >
> > xilinx_dma_start(chan);
> >
> > @@ -1887,15 +1894,8 @@ static irqreturn_t
> xilinx_dma_irq_handler(int irq, void *data)
> > }
> > }
> >
> > - if (status & XILINX_DMA_DMASR_DLY_CNT_IRQ) {
> > - /*
> > - * Device takes too long to do the transfer when user
> requires
> > - * responsiveness.
> > - */
> > - dev_dbg(chan->dev, "Inter-packet latency too long\n");
> > - }
> > -
> > - if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
> > + if (status & (XILINX_DMA_DMASR_FRM_CNT_IRQ |
> > + XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
> > spin_lock(&chan->lock);
> > xilinx_dma_complete_descriptor(chan);
> > chan->idle = true;
> > @@ -2822,6 +2822,8 @@ static int xilinx_dma_chan_probe(struct
> xilinx_dma_device *xdev,
> > /* Retrieve the channel properties from the device tree */
> > has_dre = of_property_read_bool(node, "xlnx,include-dre");
> >
> > + of_property_read_u8(node, "xlnx,irq-delay", &chan->irq_delay);
>
> Same question here too

This is an optional property. If it is not there, then driver programs
this value as zero and it does not break existing functionality.

>
> > +
> > chan->genlock = of_property_read_bool(node, "xlnx,genlock-
> mode");
> >
> > err = of_property_read_u32(node, "xlnx,datawidth", &value);
> > --
> > 2.25.1
>
> --
> ~Vinod