[PATCH 0/4] dmaengine: xilinx: dpdma: Fix freeze after 64k frames

[PATCH 0/4] dmaengine: xilinx: dpdma: Fix freeze after 64k frames

[Laurent Pinchart]

Hello,

This patch series addresses an issue in the Xilixn ZynqMP DPDMA driver
that causes a display freeze after 65536 frames. The first three patches
include a small compilation breakage issue (1/4) and enhancements to the
messages logged by the driver (2/4 and 3/4). The last patch fixes the
freeze bug. Please see individual patches for details.

Laurent Pinchart (4):
  dmaengine: xilinx: dpdma: Add missing dependencies to Kconfig
  dmaengine: xilinx: dpdma: Print channel number in kernel log messages
  dmaengine: xilinx: dpdma: Print debug message when losing vsync race
  dmaengine: xilinx: dpdma: Limit descriptor IDs to 16 bits

 drivers/dma/Kconfig               |  1 +
 drivers/dma/xilinx/xilinx_dpdma.c | 45 ++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 15 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/4] dmaengine: xilinx: dpdma: Add missing dependencies to Kconfig

[Laurent Pinchart]

The driver depends on both OF and IOMEM support, express those
dependencies in Kconfig. This fixes a build failure on S390 reported by
the 0day bot.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/dma/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6ab9d9a488a6..e47d4efbe7c5 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -701,6 +701,7 @@ config XILINX_ZYNQMP_DMA
 
 config XILINX_ZYNQMP_DPDMA
 	tristate "Xilinx DPDMA Engine"
+	depends on HAS_IOMEM && OF
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help
-- 
Regards,

Laurent Pinchart

[PATCH 2/4] dmaengine: xilinx: dpdma: Print channel number in kernel log messages

[Laurent Pinchart]

To ease debugging, add the channel number to all kernel log messages
related to a particular channel.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/dma/xilinx/xilinx_dpdma.c | 32 +++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index 70b29bd079c9..ea56c3b35782 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -702,8 +702,9 @@ xilinx_dpdma_chan_prep_interleaved_dma(struct xilinx_dpdma_chan *chan,
 	size_t stride = hsize + xt->sgl[0].icg;
 
 	if (!IS_ALIGNED(xt->src_start, XILINX_DPDMA_ALIGN_BYTES)) {
-		dev_err(chan->xdev->dev, "buffer should be aligned at %d B\n",
-			XILINX_DPDMA_ALIGN_BYTES);
+		dev_err(chan->xdev->dev,
+			"chan%u: buffer should be aligned at %d B\n",
+			chan->id, XILINX_DPDMA_ALIGN_BYTES);
 		return NULL;
 	}
 
@@ -934,7 +935,9 @@ static int xilinx_dpdma_chan_notify_no_ostand(struct xilinx_dpdma_chan *chan)
 
 	cnt = xilinx_dpdma_chan_ostand(chan);
 	if (cnt) {
-		dev_dbg(chan->xdev->dev, "%d outstanding transactions\n", cnt);
+		dev_dbg(chan->xdev->dev,
+			"chan%u: %d outstanding transactions\n",
+			chan->id, cnt);
 		return -EWOULDBLOCK;
 	}
 
@@ -970,8 +973,8 @@ static int xilinx_dpdma_chan_wait_no_ostand(struct xilinx_dpdma_chan *chan)
 		return 0;
 	}
 
-	dev_err(chan->xdev->dev, "not ready to stop: %d trans\n",
-		xilinx_dpdma_chan_ostand(chan));
+	dev_err(chan->xdev->dev, "chan%u: not ready to stop: %d trans\n",
+		chan->id, xilinx_dpdma_chan_ostand(chan));
 
 	if (ret == 0)
 		return -ETIMEDOUT;
@@ -1005,8 +1008,8 @@ static int xilinx_dpdma_chan_poll_no_ostand(struct xilinx_dpdma_chan *chan)
 		return 0;
 	}
 
-	dev_err(chan->xdev->dev, "not ready to stop: %d trans\n",
-		xilinx_dpdma_chan_ostand(chan));
+	dev_err(chan->xdev->dev, "chan%u: not ready to stop: %d trans\n",
+		chan->id, xilinx_dpdma_chan_ostand(chan));
 
 	return -ETIMEDOUT;
 }
@@ -1060,7 +1063,8 @@ static void xilinx_dpdma_chan_done_irq(struct xilinx_dpdma_chan *chan)
 		vchan_cyclic_callback(&active->vdesc);
 	else
 		dev_warn(chan->xdev->dev,
-			 "DONE IRQ with no active descriptor!\n");
+			 "chan%u: DONE IRQ with no active descriptor!\n",
+			 chan->id);
 
 	spin_unlock_irqrestore(&chan->lock, flags);
 }
@@ -1148,10 +1152,12 @@ static void xilinx_dpdma_chan_handle_err(struct xilinx_dpdma_chan *chan)
 
 	spin_lock_irqsave(&chan->lock, flags);
 
-	dev_dbg(xdev->dev, "cur desc addr = 0x%04x%08x\n",
+	dev_dbg(xdev->dev, "chan%u: cur desc addr = 0x%04x%08x\n",
+		chan->id,
 		dpdma_read(chan->reg, XILINX_DPDMA_CH_DESC_START_ADDRE),
 		dpdma_read(chan->reg, XILINX_DPDMA_CH_DESC_START_ADDR));
-	dev_dbg(xdev->dev, "cur payload addr = 0x%04x%08x\n",
+	dev_dbg(xdev->dev, "chan%u: cur payload addr = 0x%04x%08x\n",
+		chan->id,
 		dpdma_read(chan->reg, XILINX_DPDMA_CH_PYLD_CUR_ADDRE),
 		dpdma_read(chan->reg, XILINX_DPDMA_CH_PYLD_CUR_ADDR));
 
@@ -1167,7 +1173,8 @@ static void xilinx_dpdma_chan_handle_err(struct xilinx_dpdma_chan *chan)
 	xilinx_dpdma_chan_dump_tx_desc(chan, active);
 
 	if (active->error)
-		dev_dbg(xdev->dev, "repeated error on desc\n");
+		dev_dbg(xdev->dev, "chan%u: repeated error on desc\n",
+			chan->id);
 
 	/* Reschedule if there's no new descriptor */
 	if (!chan->desc.pending &&
@@ -1232,7 +1239,8 @@ static int xilinx_dpdma_alloc_chan_resources(struct dma_chan *dchan)
 					  align, 0);
 	if (!chan->desc_pool) {
 		dev_err(chan->xdev->dev,
-			"failed to allocate a descriptor pool\n");
+			"chan%u: failed to allocate a descriptor pool\n",
+			chan->id);
 		return -ENOMEM;
 	}
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/4] dmaengine: xilinx: dpdma: Print debug message when losing vsync race

[Laurent Pinchart]

The hardware retrigger is inherently racy with the vsync interrupt. This
isn't an issue as the hardware provides a way to detect a race loss and
handle it correctly. When debugging issues related to this, it's useful
to get a notification of the race loss. Add a debug message to do so.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/dma/xilinx/xilinx_dpdma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index ea56c3b35782..5834f8614a58 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -1095,8 +1095,12 @@ static void xilinx_dpdma_chan_vsync_irq(struct  xilinx_dpdma_chan *chan)
 	/* If the retrigger raced with vsync, retry at the next frame. */
 	sw_desc = list_first_entry(&pending->descriptors,
 				   struct xilinx_dpdma_sw_desc, node);
-	if (sw_desc->hw.desc_id != desc_id)
+	if (sw_desc->hw.desc_id != desc_id) {
+		dev_dbg(chan->xdev->dev,
+			"chan%u: vsync race lost (%u != %u), retrying\n",
+			chan->id, sw_desc->hw.desc_id, desc_id);
 		goto out;
+	}
 
 	/*
 	 * Complete the active descriptor, if any, promote the pending
-- 
Regards,

Laurent Pinchart

[PATCH 4/4] dmaengine: xilinx: dpdma: Limit descriptor IDs to 16 bits

[Laurent Pinchart]

While the descriptor ID is stored in a 32-bit field in the hardware
descriptor, only 16 bits are used by the hardware and are reported
through the XILINX_DPDMA_CH_DESC_ID register. Failure to handle the
wrap-around results in a descriptor ID mismatch after 65536 frames. Fix
it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/dma/xilinx/xilinx_dpdma.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index 5834f8614a58..6a8bb977fc7a 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -113,6 +113,7 @@
 #define XILINX_DPDMA_CH_VDO				0x020
 #define XILINX_DPDMA_CH_PYLD_SZ				0x024
 #define XILINX_DPDMA_CH_DESC_ID				0x028
+#define XILINX_DPDMA_CH_DESC_ID_MASK			GENMASK(15, 0)
 
 /* DPDMA descriptor fields */
 #define XILINX_DPDMA_DESC_CONTROL_PREEMBLE		0xa5
@@ -867,7 +868,8 @@ static void xilinx_dpdma_chan_queue_transfer(struct xilinx_dpdma_chan *chan)
 	 * will be used, but it should be enough.
 	 */
 	list_for_each_entry(sw_desc, &desc->descriptors, node)
-		sw_desc->hw.desc_id = desc->vdesc.tx.cookie;
+		sw_desc->hw.desc_id = desc->vdesc.tx.cookie
+				    & XILINX_DPDMA_CH_DESC_ID_MASK;
 
 	sw_desc = list_first_entry(&desc->descriptors,
 				   struct xilinx_dpdma_sw_desc, node);
@@ -1090,7 +1092,8 @@ static void xilinx_dpdma_chan_vsync_irq(struct  xilinx_dpdma_chan *chan)
 	if (!chan->running || !pending)
 		goto out;
 
-	desc_id = dpdma_read(chan->reg, XILINX_DPDMA_CH_DESC_ID);
+	desc_id = dpdma_read(chan->reg, XILINX_DPDMA_CH_DESC_ID)
+		& XILINX_DPDMA_CH_DESC_ID_MASK;
 
 	/* If the retrigger raced with vsync, retry at the next frame. */
 	sw_desc = list_first_entry(&pending->descriptors,
-- 
Regards,

Laurent Pinchart

RE: [PATCH 0/4] dmaengine: xilinx: dpdma: Fix freeze after 64k frames

[Jianqiang Chen]

Tested-by: Jianqiang Chen <jianqiang.chen@xilinx.com>
Reviewed-by: Jianqiang Chen <jianqiang.chen@xilinx.com>

Thanks,
Jason

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Thursday, May 20, 2021 8:24 AM
> To: dmaengine@vger.kernel.org
> Cc: Vinod Koul <vkoul@kernel.org>; Michal Simek <michals@xilinx.com>;
> Jianqiang Chen <jianqian@xilinx.com>; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH 0/4] dmaengine: xilinx: dpdma: Fix freeze after 64k frames
> 
> Hello,
> 
> This patch series addresses an issue in the Xilixn ZynqMP DPDMA driver that
> causes a display freeze after 65536 frames. The first three patches include a
> small compilation breakage issue (1/4) and enhancements to the messages
> logged by the driver (2/4 and 3/4). The last patch fixes the freeze bug. Please
> see individual patches for details.
> 
> Laurent Pinchart (4):
>   dmaengine: xilinx: dpdma: Add missing dependencies to Kconfig
>   dmaengine: xilinx: dpdma: Print channel number in kernel log messages
>   dmaengine: xilinx: dpdma: Print debug message when losing vsync race
>   dmaengine: xilinx: dpdma: Limit descriptor IDs to 16 bits
> 
>  drivers/dma/Kconfig               |  1 +
>  drivers/dma/xilinx/xilinx_dpdma.c | 45 ++++++++++++++++++++-----------
>  2 files changed, 31 insertions(+), 15 deletions(-)
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 0/4] dmaengine: xilinx: dpdma: Fix freeze after 64k frames

[Vinod Koul]

On 20-05-21, 18:24, Laurent Pinchart wrote:
> Hello,
> 
> This patch series addresses an issue in the Xilixn ZynqMP DPDMA driver
> that causes a display freeze after 65536 frames. The first three patches
> include a small compilation breakage issue (1/4) and enhancements to the
> messages logged by the driver (2/4 and 3/4). The last patch fixes the
> freeze bug. Please see individual patches for details.

Applied, thanks

While at it please keep fixes at the top of the series, easier to apply
to fixes rather than next

Thanks
-- 
~Vinod