Re: Testers wanted (was: Re: PATCH for SAA7146 DMA buffer overflow in budget cards (updated))

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Oliver Endriss wrote:
> Ingo Schneider wrote:
> > Hello again !
> > 
> > Here comes a Patch which does the following changes for all ttpci budget 
> > cards:
> > - Issue a warning when more than 80% of the DMA buffer is being used 
> > (probably due to bad IRQ latency)
> >  Warnings are limited to the first 100 warnings and after that one 
> > warning for every 100 buffer overruns.
> > - Introduce a new parameter "bufsize" (in k) which increases the default 
> > DMA buffer of 188k up to 4 MB
> >  A buffer size of 470k does it for me even at high I/O load conditions.
> > - Now the patch doesn't break budget-patch anymore
> > 
> > Signed-off-by: Ingo Schneider <mail at ingo-schneider.de>
> > 
> > Oliver, can you please integrate this patch ?
> 
> Sorry for the delay.
> 
> This weekend I will do some tests with different buffer settings.
> Unfortunately, I can only test Activy GR and old-style Nova hardware
> (budget driver).
> 
> I'd appreciate if more people would test the patch with different
> hardware (budget-ci, budget-av, budget-patch driver).
> Please report _any_ problems. Thanks.
> 
> Anyway, I still don't like the way buffer warnings are logged.
> syslog might be flooded with 100 buffer warnings in worst case.
> 
> I will make error logging rate-limited, i.e. no more than one message
> every 30 seconds will be logged. The error counter will be cleared after
> logging the message. That should be sufficient to detect any problems.

Ok, my proposal based on Ingo's patch:
- simplified calculation of dma buffer size:
  never change buffer_width, buffer size limited to 1410K (Activy 564K)
- not more than one buffer warning every 30 seconds
- removed unused struct member 'tsf'
- whitespace clean-up

All buffer sizes have been tested with Nova-S and Activy GR.

@Ingo:
If this patch is ok for you I will commit it.

CU
Oliver

P.S.:
Could someone please test the budget-patch driver with this patch?

-- 
--------------------------------------------------------
VDR Remote Plugin available at
http://www.escape-edv.de/endriss/vdr/
--------------------------------------------------------
diff -r e2c1aba7de46 linux/drivers/media/dvb/ttpci/budget-core.c
--- a/linux/drivers/media/dvb/ttpci/budget-core.c	Fri Mar 17 06:48:33 2006 -0300
+++ b/linux/drivers/media/dvb/ttpci/budget-core.c	Sat Mar 18 20:30:03 2006 +0100
@@ -39,9 +39,21 @@
 #include "budget.h"
 #include "ttpci-eeprom.h"
 
+#define TS_WIDTH		(2 * TS_SIZE)
+#define TS_WIDTH_ACTIVY		TS_SIZE
+#define TS_HEIGHT_MASK		0xf00
+#define TS_HEIGHT_MASK_ACTIVY	0xc00
+#define TS_MIN_BUFSIZE_K	188
+#define TS_MAX_BUFSIZE_K	1410
+#define TS_MAX_BUFSIZE_K_ACTIVY	564
+#define BUFFER_WARNING_WAIT	(30*HZ)
+
 int budget_debug;
+static int dma_buffer_size = TS_MIN_BUFSIZE_K;
 module_param_named(debug, budget_debug, int, 0644);
+module_param_named(bufsize, dma_buffer_size, int, 0444);
 MODULE_PARM_DESC(debug, "Turn on/off budget debugging (default:off).");
+MODULE_PARM_DESC(bufsize, "DMA buffer size in KB, default: 188, min: 188, max: 1410 (Activy: 564)");
 
 /****************************************************************************
  * TT budget / WinTV Nova
@@ -70,11 +82,10 @@ static int start_ts_capture(struct budge
 
 	saa7146_write(dev, MC1, MASK_20);	// DMA3 off
 
-	memset(budget->grabbing, 0x00, TS_HEIGHT * TS_WIDTH);
+	memset(budget->grabbing, 0x00, budget->buffer_size);
 
 	saa7146_write(dev, PCI_BT_V1, 0x001c0000 | (saa7146_read(dev, PCI_BT_V1) & ~0x001f0000));
 
-	budget->tsf = 0xff;
 	budget->ttbp = 0;
 
 	/*
@@ -115,16 +126,12 @@ static int start_ts_capture(struct budge
 
 	saa7146_write(dev, BASE_ODD3, 0);
 	saa7146_write(dev, BASE_EVEN3, 0);
-	saa7146_write(dev, PROT_ADDR3, TS_WIDTH * TS_HEIGHT);
+	saa7146_write(dev, PROT_ADDR3, budget->buffer_size);
 	saa7146_write(dev, BASE_PAGE3, budget->pt.dma | ME1 | 0x90);
 
-	if (budget->card->type == BUDGET_FS_ACTIVY) {
-		saa7146_write(dev, PITCH3, TS_WIDTH / 2);
-		saa7146_write(dev, NUM_LINE_BYTE3, ((TS_HEIGHT * 2) << 16) | (TS_WIDTH / 2));
-	} else {
-		saa7146_write(dev, PITCH3, TS_WIDTH);
-		saa7146_write(dev, NUM_LINE_BYTE3, (TS_HEIGHT << 16) | TS_WIDTH);
-	}
+	saa7146_write(dev, PITCH3, budget->buffer_width);
+	saa7146_write(dev, NUM_LINE_BYTE3,
+			(budget->buffer_height << 16) | budget->buffer_width);
 
 	saa7146_write(dev, MC2, (MASK_04 | MASK_20));
 
@@ -141,11 +148,12 @@ static void vpeirq(unsigned long data)
 	u8 *mem = (u8 *) (budget->grabbing);
 	u32 olddma = budget->ttbp;
 	u32 newdma = saa7146_read(budget->dev, PCI_VDP3);
+	u32 count;
 
 	/* nearest lower position divisible by 188 */
 	newdma -= newdma % 188;
 
-	if (newdma >= TS_BUFLEN)
+	if (newdma >= budget->buffer_size)
 		return;
 
 	budget->ttbp = newdma;
@@ -154,10 +162,23 @@ static void vpeirq(unsigned long data)
 		return;
 
 	if (newdma > olddma) {	/* no wraparound, dump olddma..newdma */
-		dvb_dmx_swfilter_packets(&budget->demux, mem + olddma, (newdma - olddma) / 188);
+		count = newdma - olddma;
+		dvb_dmx_swfilter_packets(&budget->demux, mem + olddma, count / 188);
 	} else {		/* wraparound, dump olddma..buflen and 0..newdma */
-		dvb_dmx_swfilter_packets(&budget->demux, mem + olddma, (TS_BUFLEN - olddma) / 188);
+		count = budget->buffer_size - olddma;
+		dvb_dmx_swfilter_packets(&budget->demux, mem + olddma, count / 188);
+		count += newdma;
 		dvb_dmx_swfilter_packets(&budget->demux, mem, newdma / 188);
+	}
+
+	if (count > budget->buffer_warning_threshold)
+		budget->buffer_warnings++;
+
+	if (budget->buffer_warnings && time_after(jiffies, budget->buffer_warning_time)) {
+		printk("%s %s: used %d times >80%% of buffer (%u bytes now)\n",
+			budget->dev->name, __FUNCTION__, budget->buffer_warnings, count);
+		budget->buffer_warning_time = jiffies + BUFFER_WARNING_WAIT;
+		budget->buffer_warnings = 0;
 	}
 }
 
@@ -341,9 +362,10 @@ int ttpci_budget_init(struct budget *bud
 		      struct saa7146_pci_extension_data *info,
 		      struct module *owner)
 {
-	int length = TS_WIDTH * TS_HEIGHT;
 	int ret = 0;
 	struct budget_info *bi = info->ext_priv;
+	int max_bufsize;
+	int height_mask;
 
 	memset(budget, 0, sizeof(struct budget));
 
@@ -351,6 +373,32 @@ int ttpci_budget_init(struct budget *bud
 
 	budget->card = bi;
 	budget->dev = (struct saa7146_dev *) dev;
+
+	if (budget->card->type == BUDGET_FS_ACTIVY) {
+		budget->buffer_width = TS_WIDTH_ACTIVY;
+		max_bufsize = TS_MAX_BUFSIZE_K_ACTIVY;
+		height_mask = TS_HEIGHT_MASK_ACTIVY;
+	} else {
+		budget->buffer_width = TS_WIDTH;
+		max_bufsize = TS_MAX_BUFSIZE_K;
+		height_mask = TS_HEIGHT_MASK;
+	}
+
+	if (dma_buffer_size < TS_MIN_BUFSIZE_K)
+		dma_buffer_size = TS_MIN_BUFSIZE_K;
+	else if (dma_buffer_size > max_bufsize)
+		dma_buffer_size = max_bufsize;
+
+	budget->buffer_height = dma_buffer_size * 1024 / budget->buffer_width;
+	budget->buffer_height &= height_mask;
+	budget->buffer_size = budget->buffer_height * budget->buffer_width;
+	budget->buffer_warning_threshold = budget->buffer_size * 80/100;
+	budget->buffer_warnings = 0;
+	budget->buffer_warning_time = jiffies;
+
+	dprintk(2, "%s: width = %d, height = %d\n",
+		budget->dev->name, budget->buffer_width, budget->buffer_height);
+	printk("%s: dma buffer size %u\n", budget->dev->name, budget->buffer_size);
 
 	dvb_register_adapter(&budget->dvb_adapter, budget->card->name, owner);
 
@@ -392,7 +440,7 @@ int ttpci_budget_init(struct budget *bud
 	ttpci_eeprom_parse_mac(&budget->i2c_adap, budget->dvb_adapter.proposed_mac);
 
 	if (NULL ==
-	    (budget->grabbing = saa7146_vmalloc_build_pgtable(dev->pci, length, &budget->pt))) {
+	    (budget->grabbing = saa7146_vmalloc_build_pgtable(dev->pci, budget->buffer_size, &budget->pt))) {
 		ret = -ENOMEM;
 		goto err;
 	}
diff -r e2c1aba7de46 linux/drivers/media/dvb/ttpci/budget-patch.c
--- a/linux/drivers/media/dvb/ttpci/budget-patch.c	Fri Mar 17 06:48:33 2006 -0300
+++ b/linux/drivers/media/dvb/ttpci/budget-patch.c	Sat Mar 18 20:30:03 2006 +0100
@@ -577,6 +577,17 @@ static int budget_patch_attach (struct s
 	saa7146_setgpio(dev, 3, SAA7146_GPIO_OUTLO);
 	// Set RPS1 Address register to point to RPS code               (r108 p42)
 	saa7146_write(dev, RPS_ADDR1, dev->d_rps1.dma_handle);
+
+	if (!(budget = kmalloc (sizeof(struct budget_patch), GFP_KERNEL)))
+		return -ENOMEM;
+
+	dprintk(2, "budget: %p\n", budget);
+
+	if ((err = ttpci_budget_init (budget, dev, info, THIS_MODULE))) {
+		kfree (budget);
+		return err;
+	}
+
 	// Set Source Line Counter Threshold, using BRS                 (rCC p43)
 	// It generates HS event every TS_HEIGHT lines
 	// this is related to TS_WIDTH set in register
@@ -585,22 +596,11 @@ static int budget_patch_attach (struct s
 	//,then RPS_THRESH1
 	// should be set to trigger every TS_HEIGHT (512) lines.
 	//
-	saa7146_write(dev, RPS_THRESH1, (TS_HEIGHT*1) | MASK_12 );
+	saa7146_write(dev, RPS_THRESH1, budget->buffer_height | MASK_12 );
 
 	// saa7146_write(dev, RPS_THRESH0, ((TS_HEIGHT/2)<<16) |MASK_28| (TS_HEIGHT/2) |MASK_12 );
 	// Enable RPS1                                                  (rFC p33)
 	saa7146_write(dev, MC1, (MASK_13 | MASK_29));
-
-
-	if (!(budget = kmalloc (sizeof(struct budget_patch), GFP_KERNEL)))
-		return -ENOMEM;
-
-	dprintk(2, "budget: %p\n", budget);
-
-	if ((err = ttpci_budget_init (budget, dev, info, THIS_MODULE))) {
-		kfree (budget);
-		return err;
-	}
 
 
 	dev->ext_priv = budget;
diff -r e2c1aba7de46 linux/drivers/media/dvb/ttpci/budget.h
--- a/linux/drivers/media/dvb/ttpci/budget.h	Fri Mar 17 06:48:33 2006 -0300
+++ b/linux/drivers/media/dvb/ttpci/budget.h	Sat Mar 18 20:30:03 2006 +0100
@@ -70,7 +70,13 @@ struct budget {
 	int ci_present;
 	int video_port;
 
-	u8 tsf;
+	u32 buffer_width;
+	u32 buffer_height;
+	u32 buffer_size;
+	u32 buffer_warning_threshold;
+	u32 buffer_warnings;
+	unsigned long buffer_warning_time;
+
 	u32 ttbp;
 	int feeding;
 
@@ -90,11 +96,6 @@ static struct saa7146_pci_extension_data
 static struct saa7146_pci_extension_data x_var = { \
 	.ext_priv = &x_var ## _info, \
 	.ext = &budget_extension };
-
-#define TS_WIDTH  (376)
-#define TS_HEIGHT (512)
-#define TS_BUFLEN (TS_WIDTH*TS_HEIGHT)
-#define TS_MAX_PACKETS (TS_BUFLEN/TS_SIZE)
 
 #define BUDGET_TT		   0
 #define BUDGET_TT_HW_DISEQC	   1
_______________________________________________

linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux