Hi, Erdi!
I have been testing the ring-buffer quite extensively since there is an option in the unichrome X driver to use it for 2D acceleration and for XvMC mpeg. I and other people have had some problems with short hangs and above all total screen confusion (sometimes recoverable) every once in a while.
The short hangs seems to be related to the fact that the command regulator stops short of the last buffer segment while the code not waiting long enough to detect the stop. This is a very rare occurence. I also added a DRMWRITEMEMORYBARRIER() here to make sure write-combined data was flushed to AGP memory. I'm not sure this is the right way to do it or whether this is even necessary.
The other problem is the rewind code. It seems to fail about every 200th rewind or something, but not consistently, and I can find no pattern in wether it fails when the jump command is issued using PCI or from the AGP buffer. However, I tried to modify the code so that the jump command was always issued from the AGP buffer and still got occasional fails. What seems to happen is that bad data is sent to the decoder or that a buffer segment is skipped. Finally I replaced the jump code with a via_cmdbuf_reset and via_cmdbuf_start call, and suddenly everything worked well. Unfortunately, this has to wait for command buffer completion.
So my question is how exactly does that jump command work? From the code I could see you modified the buffer end and then made a jump command to / (from ?) that end address.
Will the command regulator automatically wraparound when it reaches the buffer end? In that case it would be possible to alter the code to fill the buffer completely and never use the jump / rewind command.
I'll attach a patch for my latest stop / start version that seems to be working OK, but that suffers from that it has to stop DMA to restart it at buffer end.
Best regards Thomas.
Index: shared/via_dma.c
===================================================================
RCS file: /cvs/dri/drm/shared/via_dma.c,v
retrieving revision 1.11
diff -u -r1.11 via_dma.c
--- shared/via_dma.c 12 Oct 2004 18:46:26 -0000 1.11
+++ shared/via_dma.c 14 Oct 2004 17:33:29 -0000
@@ -7,6 +7,8 @@
*
* Copyright 2004 Digeo, Inc., Palo Alto, CA, U.S.A.
* All Rights Reserved.
+ *
+ * Copyright (2004) The Unichrome project, All Rights Reserved
*
**************************************************************************/
@@ -17,11 +19,20 @@
#include "via_drv.h"
#define VIA_2D_CMD 0xF0000000
+#define VIA_OUT_RING_QW(p1, p2) \
+ *vb++ = (p1); \
+ *vb++ = (p2); \
+ dev_priv->dma_low += 8;
+
+
+#define via_flush_write_combine() DRM_WRITEMEMORYBARRIER()
+
static void via_cmdbuf_start(drm_via_private_t * dev_priv);
static void via_cmdbuf_pause(drm_via_private_t * dev_priv);
static void via_cmdbuf_reset(drm_via_private_t * dev_priv);
static void via_cmdbuf_rewind(drm_via_private_t * dev_priv);
+static void via_cmdbuf_flush(drm_via_private_t * dev_priv, uint32_t cmd_type);
static int via_wait_idle(drm_via_private_t * dev_priv);
/*
@@ -82,13 +93,8 @@
uint32_t count;
hw_addr_ptr = dev_priv->hw_addr_ptr;
cur_addr = dev_priv->dma_low;
- /* At high resolution (i.e. 1280x1024) and with high workload within
- * a short commmand stream, the following test will fail. It may be
- * that the engine is too busy to update hw_addr. Therefore, add
- * a large 64KB window between buffer head and tail.
- */
- next_addr = cur_addr + size + 64 * 1024;
- count = 1000000; /* How long is this? */
+ next_addr = cur_addr + size;
+ count = 1000000;
do {
hw_addr = *hw_addr_ptr - agp_base;
if (count-- == 0) {
@@ -382,9 +388,7 @@
uint32_t * vb, int qw_count)
{
for (; qw_count > 0; --qw_count) {
- *vb++ = (0xcc000000 | (dev_priv->dma_low & 0xffffff));
- *vb++ = (0xdd400000 | via_swap_count);
- dev_priv->dma_low += 8;
+ VIA_OUT_RING_QW(HC_DUMMY,HC_DUMMY);
}
via_swap_count = (via_swap_count + 1) & 0xffff;
return vb;
@@ -409,216 +413,165 @@
return count;
}
-static inline void via_dummy_bitblt(drm_via_private_t * dev_priv)
-{
- uint32_t *vb = via_get_dma(dev_priv);
- /* GEDST */
- SetReg2DAGP(0x0C, (0 | (0 << 16)));
- /* GEWD */
- SetReg2DAGP(0x10, 0 | (0 << 16));
- /* BITBLT */
- SetReg2DAGP(0x0, 0x1 | 0x2000 | 0xAA000000);
-}
-
-static void via_cmdbuf_start(drm_via_private_t * dev_priv)
+/*
+ * Aligns the command buffer and adds a command cmd_type with associated
+ * address addr. If addr is 0, then it calculates it's own address at the
+ * end of the buffer - 8. This automatic calculation is useful
+ * for pause and stop commands.
+ */
+static uint32_t *via_align_and_cmd(drm_via_private_t * dev_priv, uint32_t cmd_type,
+ uint32_t addr, uint32_t * addr_hi, uint32_t
*addr_lo)
{
- uint32_t agp_base;
- uint32_t pause_addr, pause_addr_lo, pause_addr_hi;
- uint32_t start_addr, start_addr_lo;
- uint32_t end_addr, end_addr_lo;
- uint32_t qw_pad_count;
- uint32_t command;
uint32_t *vb;
+ uint32_t cmd_addr, cmd_addr_lo, cmd_addr_hi;
+ uint32_t qw_pad_count, agp_base;
- dev_priv->dma_low = 0;
+
+ via_cmdbuf_wait(dev_priv, 2*CMDBUF_ALIGNMENT_SIZE);
vb = via_get_dma(dev_priv);
+ VIA_OUT_RING_QW( HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
+ (VIA_REG_TRANSPACE >> 2),(HC_ParaType_PreCR << 16));
agp_base = dev_priv->dma_offset + (uint32_t) dev_priv->agpAddr;
- start_addr = agp_base;
- end_addr = agp_base + dev_priv->dma_high;
+ qw_pad_count = (CMDBUF_ALIGNMENT_SIZE >> 3) -
+ ((dev_priv->dma_low & CMDBUF_ALIGNMENT_MASK) >> 3);
- start_addr_lo = ((HC_SubA_HAGPBstL << 24) | (start_addr & 0xFFFFFF));
- end_addr_lo = ((HC_SubA_HAGPBendL << 24) | (end_addr & 0xFFFFFF));
- command = ((HC_SubA_HAGPCMNT << 24) | (start_addr >> 24) |
- ((end_addr & 0xff000000) >> 16));
-
- *vb++ = HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
- (VIA_REG_TRANSPACE >> 2);
- *vb++ = (HC_ParaType_PreCR << 16);
- dev_priv->dma_low += 8;
-
- qw_pad_count = (CMDBUF_ALIGNMENT_SIZE >> 3) -
- ((dev_priv->dma_low & CMDBUF_ALIGNMENT_MASK) >> 3);
-
- pause_addr = agp_base + dev_priv->dma_low - 8 + (qw_pad_count << 3);
- pause_addr_lo = ((HC_SubA_HAGPBpL << 24) |
- HC_HAGPBpID_PAUSE | (pause_addr & 0xffffff));
- pause_addr_hi = ((HC_SubA_HAGPBpH << 24) | (pause_addr >> 24));
+
+ cmd_addr = (addr) ? addr : agp_base + dev_priv->dma_low - 8+
+ (qw_pad_count << 3);
+ cmd_addr_lo = ((HC_SubA_HAGPBpL << 24) | cmd_type |
+ (cmd_addr & 0xffffff));
+ cmd_addr_hi = ((HC_SubA_HAGPBpH << 24) | (cmd_addr >> 24));
vb = via_align_buffer(dev_priv, vb, qw_pad_count - 1);
- *vb++ = pause_addr_hi;
- *vb++ = pause_addr_lo;
- dev_priv->dma_low += 8;
- dev_priv->last_pause_ptr = vb - 1;
-
- VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
- VIA_WRITE(VIA_REG_TRANSPACE, command);
- VIA_WRITE(VIA_REG_TRANSPACE, start_addr_lo);
- VIA_WRITE(VIA_REG_TRANSPACE, end_addr_lo);
-
- VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
- VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
-
- VIA_WRITE(VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK);
+ VIA_OUT_RING_QW(*addr_hi = cmd_addr_hi,*addr_lo = cmd_addr_lo);
+ return vb;
}
-static void via_cmdbuf_jump(drm_via_private_t * dev_priv)
+/*
+ * Hooks a segment of data into the tail of the ring-buffer by
+ * modifying the pause address stored in the buffer itself. If
+ * the regulator has already paused, restart it.
+ */
+static int via_hook_segment(drm_via_private_t *dev_priv,
+ uint32_t pause_addr_hi, uint32_t pause_addr_lo,
+ int no_pci_fire)
{
- uint32_t agp_base;
- uint32_t pause_addr, pause_addr_lo, pause_addr_hi;
- uint32_t start_addr;
- uint32_t end_addr, end_addr_lo;
- uint32_t *vb;
- uint32_t qw_pad_count;
- uint32_t command;
- uint32_t jump_addr, jump_addr_lo, jump_addr_hi;
+ int paused, count;
- /* Seems like Unichrome has bug that when the PAUSE register is
- * set in the AGP command stream immediately after a PCI write to
- * the same register, the command regulator goes into a looping
- * state. Prepending a BitBLT command to stall the command
- * regulator for a moment seems to solve the problem.
- */
-
- via_cmdbuf_wait(dev_priv, 48);
- via_dummy_bitblt(dev_priv);
-
- via_cmdbuf_wait(dev_priv, 2 * CMDBUF_ALIGNMENT_SIZE);
+ *dev_priv->last_pause_ptr = pause_addr_lo;
+ dev_priv->last_pause_ptr = via_get_dma(dev_priv) - 1;
+ via_flush_write_combine();
- /* At end of buffer, rewind with a JUMP command. */
- vb = via_get_dma(dev_priv);
+ paused = 0;
+ count = 5;
- *vb++ = HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
- (VIA_REG_TRANSPACE >> 2);
- *vb++ = (HC_ParaType_PreCR << 16);
- dev_priv->dma_low += 8;
+ while (!(paused = (VIA_READ(0x41c) & 0x80000000)) && count--);
- qw_pad_count = (CMDBUF_ALIGNMENT_SIZE >> 3) -
- ((dev_priv->dma_low & CMDBUF_ALIGNMENT_MASK) >> 3);
+ /*
+ * We need info from users on how low we can set "count".
+ * Ideally, we'd set it to 0, but that won't work. What happens is that
+ * the DMA engine has just missed our update of the pause address
+ * in the command buffer, but has not yet paused on the wrong address.
+ * Give it some time to do that.
+ * I have already seen cases with count=3 in the below message,
+ * so those are not of interest.
+ */
- agp_base = dev_priv->dma_offset + (uint32_t) dev_priv->agpAddr;
- start_addr = agp_base;
- end_addr = agp_base + dev_priv->dma_low - 8 + (qw_pad_count << 3);
+ if ((count < 3) && (count >= 0)) {
+ uint32_t rgtr, ptr;
+ rgtr = *(dev_priv->hw_addr_ptr);
+ ptr = ((char *)dev_priv->last_pause_ptr - dev_priv->dma_ptr) +
+ dev_priv->dma_offset + (uint32_t) dev_priv->agpAddr + 4;
+ if (rgtr != ptr) {
+ DRM_ERROR("Command regulator\npaused at count %d, address %x, "
+ "while current pause address is %x.\n"
+ "Please mail this message to "
+ "<[EMAIL PROTECTED]>\n",
+ count, rgtr, ptr);
+ }
+ }
+
+ if (paused && !no_pci_fire) {
+ VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
+ VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
+ VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
+ }
+ return paused;
+}
- jump_addr = end_addr;
- jump_addr_lo = ((HC_SubA_HAGPBpL << 24) | HC_HAGPBpID_JUMP |
- (jump_addr & 0xffffff));
- jump_addr_hi = ((HC_SubA_HAGPBpH << 24) | (jump_addr >> 24));
- end_addr_lo = ((HC_SubA_HAGPBendL << 24) | (end_addr & 0xFFFFFF));
- command = ((HC_SubA_HAGPCMNT << 24) | (start_addr >> 24) |
- ((end_addr & 0xff000000) >> 16));
- *vb++ = command;
- *vb++ = end_addr_lo;
- dev_priv->dma_low += 8;
+/*
+ * Starts up the ring-buffer DMA with an empty buffer.
+ */
- vb = via_align_buffer(dev_priv, vb, qw_pad_count - 1);
+static void via_cmdbuf_start(drm_via_private_t * dev_priv)
+{
+ uint32_t agp_base;
+ uint32_t pause_addr_lo, pause_addr_hi;
+ uint32_t start_addr, start_addr_lo;
+ uint32_t end_addr, end_addr_lo;
+ uint32_t command;
+ uint32_t *vb;
- /* Now at beginning of buffer, make sure engine will pause here. */
dev_priv->dma_low = 0;
- if (via_cmdbuf_wait(dev_priv, CMDBUF_ALIGNMENT_SIZE) != 0) {
- DRM_ERROR("via_cmdbuf_jump failed\n");
- }
- vb = via_get_dma(dev_priv);
+ agp_base = dev_priv->dma_offset + (uint32_t) dev_priv->agpAddr;
+ start_addr = agp_base;
end_addr = agp_base + dev_priv->dma_high;
+ start_addr_lo = ((HC_SubA_HAGPBstL << 24) | (start_addr & 0xFFFFFF));
end_addr_lo = ((HC_SubA_HAGPBendL << 24) | (end_addr & 0xFFFFFF));
command = ((HC_SubA_HAGPCMNT << 24) | (start_addr >> 24) |
((end_addr & 0xff000000) >> 16));
- qw_pad_count = (CMDBUF_ALIGNMENT_SIZE >> 3) -
- ((dev_priv->dma_low & CMDBUF_ALIGNMENT_MASK) >> 3);
+ vb = via_align_and_cmd( dev_priv, HC_HAGPBpID_PAUSE, 0,
+ &pause_addr_hi, &pause_addr_lo);
- pause_addr = agp_base + dev_priv->dma_low - 8 + (qw_pad_count << 3);
- pause_addr_lo = ((HC_SubA_HAGPBpL << 24) | HC_HAGPBpID_PAUSE |
- (pause_addr & 0xffffff));
- pause_addr_hi = ((HC_SubA_HAGPBpH << 24) | (pause_addr >> 24));
-
- *vb++ = HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
- (VIA_REG_TRANSPACE >> 2);
- *vb++ = (HC_ParaType_PreCR << 16);
- dev_priv->dma_low += 8;
+ dev_priv->last_pause_ptr = vb - 1;
- *vb++ = pause_addr_hi;
- *vb++ = pause_addr_lo;
- dev_priv->dma_low += 8;
+ via_flush_write_combine();
+ VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
+ VIA_WRITE(VIA_REG_TRANSPACE, command);
+ VIA_WRITE(VIA_REG_TRANSPACE, start_addr_lo);
+ VIA_WRITE(VIA_REG_TRANSPACE, end_addr_lo);
- *vb++ = command;
- *vb++ = end_addr_lo;
- dev_priv->dma_low += 8;
+ VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
+ VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
- vb = via_align_buffer(dev_priv, vb, qw_pad_count - 4);
+ VIA_WRITE(VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK);
+}
- *vb++ = pause_addr_hi;
- *vb++ = pause_addr_lo;
- dev_priv->dma_low += 8;
- *dev_priv->last_pause_ptr = jump_addr_lo;
- dev_priv->last_pause_ptr = vb - 1;
+/*
+ * Adds a jump command to the end of the current buffer and a pause command
+ * at the start of the physical buffer memory area. Also fires this data.
+ *
+ */
- if (VIA_READ(0x41c) & 0x80000000) {
- VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
- VIA_WRITE(VIA_REG_TRANSPACE, jump_addr_hi);
- VIA_WRITE(VIA_REG_TRANSPACE, jump_addr_lo);
- }
+static void via_cmdbuf_jump(drm_via_private_t * dev_priv)
+{
+ /* Temporarily removed */
}
+
static void via_cmdbuf_rewind(drm_via_private_t * dev_priv)
{
- via_cmdbuf_pause(dev_priv);
- via_cmdbuf_jump(dev_priv);
+ /* via_cmdbuf_jump(dev_priv); */
+ via_cmdbuf_reset( dev_priv );
+ via_cmdbuf_start( dev_priv );
}
static void via_cmdbuf_flush(drm_via_private_t * dev_priv, uint32_t cmd_type)
{
- uint32_t agp_base;
- uint32_t pause_addr, pause_addr_lo, pause_addr_hi;
- uint32_t *vb;
- uint32_t qw_pad_count;
+ uint32_t pause_addr_lo, pause_addr_hi;
- via_cmdbuf_wait(dev_priv, 0x200);
-
- vb = via_get_dma(dev_priv);
- *vb++ = HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
- (VIA_REG_TRANSPACE >> 2);
- *vb++ = (HC_ParaType_PreCR << 16);
- dev_priv->dma_low += 8;
-
- agp_base = dev_priv->dma_offset + (uint32_t) dev_priv->agpAddr;
- qw_pad_count = (CMDBUF_ALIGNMENT_SIZE >> 3) -
- ((dev_priv->dma_low & CMDBUF_ALIGNMENT_MASK) >> 3);
-
- pause_addr = agp_base + dev_priv->dma_low - 8 + (qw_pad_count << 3);
- pause_addr_lo = ((HC_SubA_HAGPBpL << 24) | cmd_type |
- (pause_addr & 0xffffff));
- pause_addr_hi = ((HC_SubA_HAGPBpH << 24) | (pause_addr >> 24));
-
- vb = via_align_buffer(dev_priv, vb, qw_pad_count - 1);
-
- *vb++ = pause_addr_hi;
- *vb++ = pause_addr_lo;
- dev_priv->dma_low += 8;
- *dev_priv->last_pause_ptr = pause_addr_lo;
- dev_priv->last_pause_ptr = vb - 1;
-
- if (VIA_READ(0x41c) & 0x80000000) {
- VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
- VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
- VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
- }
+ via_align_and_cmd(dev_priv, cmd_type, 0, &pause_addr_hi,
+ &pause_addr_lo);
+ via_hook_segment(dev_priv, pause_addr_hi, pause_addr_lo, 0);
+
}
static void via_cmdbuf_pause(drm_via_private_t * dev_priv)
