[linux-dvb] Re: Busy waiting in i2c_busy_rise_and_fall (was: Re: Re: full featured card without signal and required video memory investigation)

[Oliver Endriss <o.endriss@gmx.de>]

On Sunday 13 July 2003 08:53, Robert Schlabbach wrote:
> From: "Oliver Endriss" <o.endriss@gmx.de>
>
> > Well, I found the reason for the high cpu load:
> > i2c_busy_rise_and_fall (saa7146.c) uses mdelay calls, i.e.
> > busy-waiting. Normally this is no problem, but it consumes 30% of
> > the cpu power when the frontend thread is unable to tune...
>
> Hmm, I had mentioned this before. Try replacing the wait for the rise
> with an empty read of IICSTA. In my experience, only the first read
> of IICSTA will sometimes not indicate the correct busy states, but
> the second one is always correct.
>
> As to waiting for the transfer to complete, try increasing the I2C
> bus clock rate, 275kHz should work for all components and decrease
> the wait significantly. Then try if you can do entirely _without_ any
> delay or scheduler calls, i.e. a fully busy loop. This is preferrable
> for cases in which greater amounts of data need to be transferred.
> Such a case is e.g. the TDA10045H DSP firmware code upload, which
> takes only about 1 second when fully exploiting the 275kHz clock, but
> can take over a minute when scheduler calls are inserted...
>
> Aside from that, with a 275kHz clock a transfer finishes so fast that
> the overhead from rescheduling to other tasks might exceed the CPU
> load from the busy wait loop...

Thanks for your explanations. I modified the source code accordingly:

--- saa7146.c.org	Mon Nov 18 22:18:46 2002
+++ saa7146.c	Sun Jul 13 17:56:24 2003
@@ -1271,6 +1271,7 @@ int i2c_busy_rise_and_fall(struct saa714
 {
 	int i = 0;
 	u32 status = 0;
+	u32 endjiff;
 		
 	hprintk("saa7146: ==> i2c_busy_rise_and_fall\n");
 
@@ -1284,11 +1285,9 @@ int i2c_busy_rise_and_fall(struct saa714
 		/* check busy flag */
 		if ( 0 != (status & SAA7146_I2C_BUSY))
 			break;
-
-		/* see if anything can be done while we're waiting */
-		cond_resched ();
-		mdelay(1);
 	}
+	if (i == 0)
+		printk ("saa7146: i2c_busy_rise_and_fall: no busy flag!\n");
 
 	/* we don't check the i-value, since it does not matter
 	   if we missed the rise of the busy flag or the fall or
@@ -1296,9 +1295,9 @@ int i2c_busy_rise_and_fall(struct saa714
 	   after an i2c-command has been written out */	
 	
 	/* wait until busy-flag is inactive or error is reported */
-	for (i = timeout; i > 0; i--) {
+	for (endjiff = jiffies+timeout/10; jiffies <= endjiff; ) {
 	
-		hprintk("saa7146: i2c_busy_rise_and_fall; fall wait %d\n",i);
+		hprintk("saa7146: i2c_busy_rise_and_fall; fall wait\n");
 
 		status = i2c_status_check(saa);
 
@@ -1309,15 +1308,10 @@ int i2c_busy_rise_and_fall(struct saa714
 		/* check error flag */
 		if ( 0 != (status & SAA7146_I2C_ERR))
 			break;
-
-		/* see if anything can be done while we're waiting */
-		cond_resched ();
-
-		mdelay(1);
 	}
 	
 	/* did a timeout occur ? */
-	if ( 0 == i ) {
+	if (jiffies > endjiff) {
 		hprintk("saa7146: i2c_busy_rise_and_fall: timeout #2\n");
 		return -1;
 	}
--- saa7146_core.c.org	Sat Jun 28 01:42:53 2003
+++ saa7146_core.c	Sun Jul 13 17:54:36 2003
@@ -82,7 +82,7 @@ static struct saa7146_extension* saa7146
 #define SAA7146_I2C_TIMEOUT  100   /* in ms */
 #define SAA7146_I2C_RETRIES  6
 
-static u32 SAA7146_I2C_BBR = SAA7146_I2C_BUS_BIT_RATE_3200;
+static u32 SAA7146_I2C_BBR = SAA7146_I2C_BUS_BIT_RATE_120; /* 275 kHz */
 
 #define	__COMPILE_SAA7146_I2C__
 #define	__COMPILE_SAA7146_DEBI__


A quick test showed very nice results:
- cpu usage of the frontend process is almost zero if there is no signal.
- Tuning seems to be faster than before.

@all:
Please test and report if you encounter any problems with this patch.

Oliver



-- 
Info:
To unsubscribe send a mail to ecartis@linuxtv.org with "unsubscribe linux-dvb" as subject.