[linux-dvb] [PATCH] XC5000 tuner improvement/clean up

Chaogui Zhang czhang1974 at gmail.com
Sun Jan 27 06:08:37 CET 2008


Hi, all,

This patch improves the performance of the xc5000 tuner driver.

1. Now, only one xc5000_priv struct is used for each physical tuner
   instead of one for each attach (analog/digital). This avoids potential
   inconsistence such as the status of the firmware loading changed by
   one instance of the driver and the other instance not being aware of it.
   It is based on the tuner-xc2028.c code.

2. Removed a layer of function call for xc_reset().

3. Replaced all calls to xc_wait() by a direct msleep() call (which is all
   xc_wait does).

4. Changed the xc_wait(100) in xc_write_reg() to msleep(5). The comment
   says wait 5 ms, was the 100 a typo? In any case, this improved the
   tuner performace greatly (much faster lock).

5. Added a module parameter "allow_shutdown", which is defaulted to 0.
   If enabled, the xc5000_sleep() function will indeed shutdown the
   tuner device. On the 800i, this is fine with analog, but digital
   still cannot get a lock after shutdown and reinitialized, with no
   apparent error message. Please test this on other cards.

6. Some minor clean up.

The patch is tested on the Pinnacle 800i. Before the patch, tvtime and
azap works with analog/digital tv, but with very slow tuning and
locking. Mythtv had problems with analog tv frequently (with the "errors
are encountered during video play back" message). Most likely, that was
due to the slow tuning of the xc5000.

After the patch, both analog and digital tv saw great improvements.
Mythtv can consistently lock in both analog and digital channels. I
haven't run into the video play back errors yet so far after torturing
mythtv with channel changing and switching between different input
source/multiple cards several dozen times.

I still could not get audio (analog) in mythtv, but I suspect that is a
problem with my mythtv setup rather than the driver.

Comments are welcome and please test and post your experience. Thanks!

Best,

Signed-off-by: Chaogui Zhang <czhang1974 at gmail.com>

diff -r a7f8e825c09b linux/drivers/media/dvb/frontends/xc5000.c
--- a/linux/drivers/media/dvb/frontends/xc5000.c	Thu Jan 24 07:59:20 2008 -0200
+++ b/linux/drivers/media/dvb/frontends/xc5000.c	Sat Jan 26 20:44:14 2008 -0500
@@ -3,6 +3,7 @@
  *
  *  Copyright (c) 2007 Xceive Corporation
  *  Copyright (c) 2007 Steven Toth <stoth at hauppauge.com>
+ *  Copyright (c) 2007, 2008 Chaogui Zhang <czhang1974 at gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -38,6 +39,13 @@ MODULE_PARM_DESC(debug, "Turn on/off deb
 
 #define dprintk(level,fmt, arg...) if (debug >= level) \
 	printk(KERN_INFO "%s: " fmt, "xc5000", ## arg)
+
+static int allow_shutdown;
+module_param(allow_shutdown, int, 0644);
+MODULE_PARM_DESC(allow_shutdown, "Allow the XC5000 tuner to be shutdown (default: no).");
+
+static LIST_HEAD(xc5000_list);
+static DEFINE_MUTEX(xc5000_list_lock);
 
 #define XC5000_DEFAULT_FIRMWARE "dvb-fe-xc5000-1.1.fw"
 #define XC5000_DEFAULT_FIRMWARE_SIZE 12332
@@ -179,7 +187,6 @@ XC_TV_STANDARD XC5000_Standard[MAX_TV_ST
 
 static int  xc5000_writeregs(struct xc5000_priv *priv, u8 *buf, u8 len);
 static int  xc5000_readregs(struct xc5000_priv *priv, u8 *buf, u8 len);
-static void xc5000_TunerReset(struct dvb_frontend *fe);
 
 static int xc_send_i2c_data(struct xc5000_priv *priv, u8 *buf, int len)
 {
@@ -195,29 +202,21 @@ static int xc_read_i2c_data(struct xc500
 
 static int xc_reset(struct dvb_frontend *fe)
 {
-	xc5000_TunerReset(fe);
-	return XC_RESULT_SUCCESS;
-}
-
-static void xc_wait(int wait_ms)
-{
-	msleep(wait_ms);
-}
-
-static void xc5000_TunerReset(struct dvb_frontend *fe)
-{
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret;
 
 	dprintk(1, "%s()\n", __FUNCTION__);
 
-	if (priv->cfg->tuner_callback) {
-		ret = priv->cfg->tuner_callback(priv->cfg->priv,
-						XC5000_TUNER_RESET, 0);
-		if (ret)
-			printk(KERN_ERR "xc5000: reset failed\n");
-	} else
-		printk(KERN_ERR "xc5000: no tuner reset callback function, fatal\n");
+	if (!priv->cfg->tuner_callback) {
+		printk(KERN_ERR
+			"xc5000: no tuner reset callback function, fatal\n");
+		return XC_RESULT_RESET_FAILURE;
+	}
+
+	ret = priv->cfg->tuner_callback(priv->cfg->priv,
+					XC5000_TUNER_RESET, 0);
+	if (ret) printk(KERN_ERR "xc5000: reset failed\n");
+	return ret;
 }
 
 static int xc_write_reg(struct xc5000_priv *priv, u16 regAddr, u16 i2cData)
@@ -245,7 +244,7 @@ static int xc_write_reg(struct xc5000_pr
 						/* busy flag cleared */
 					break;
 					} else {
-						xc_wait(100); /* wait 5 ms */
+						msleep(5); /* wait 5 ms */
 						WatchDogTimer--;
 					}
 				}
@@ -296,7 +295,7 @@ static int xc_load_i2c_sequence(struct d
 				return result;
 		} else if (len & 0x8000) {
 			/* WAIT command */
-			xc_wait(len & 0x7FFF);
+			msleep(len & 0x7FFF);
 			index += 2;
 		} else {
 			/* Send i2c data whilst ensuring individual transactions
@@ -352,11 +351,10 @@ static int xc_SetTVStandard(struct xc500
 
 static int xc_shutdown(struct xc5000_priv *priv)
 {
-	return 0;
-	/* Fixme: cannot bring tuner back alive once shutdown
-	 *        without reloading the driver modules.
-	 *    return xc_write_reg(priv, XREG_POWER_DOWN, 0);
-	 */
+	if(allow_shutdown)
+		return xc_write_reg(priv, XREG_POWER_DOWN, 0);
+	else
+		return 0;
 }
 
 static int xc_SetSignalSource(struct xc5000_priv *priv, u16 rf_mode)
@@ -496,7 +494,7 @@ static u16 WaitForLock(struct xc5000_pri
 	while ((lockState == 0) && (watchDogCount > 0)) {
 		xc_get_lock_status(priv, &lockState);
 		if (lockState != 1) {
-			xc_wait(5);
+			msleep(5);
 			watchDogCount--;
 		}
 	}
@@ -577,6 +575,7 @@ static int xc5000_fwupload(struct dvb_fr
 	if (ret) {
 		printk(KERN_ERR "xc5000: Upload failed. (file not found?)\n");
 		ret = XC_RESULT_RESET_FAILURE;
+		goto out;
 	} else {
 		printk(KERN_INFO "xc5000: firmware read %Zu bytes.\n",
 		       fw->size);
@@ -591,6 +590,7 @@ static int xc5000_fwupload(struct dvb_fr
 		ret = xc_load_i2c_sequence(fe,  fw->data );
 	}
 
+out:
 	release_firmware(fw);
 	return ret;
 }
@@ -610,7 +610,7 @@ static void xc_debug_dump(struct xc5000_
 	 * Frame Lines needs two frame times after initial lock
 	 * before it is valid.
 	 */
-	xc_wait(100);
+	msleep(100);
 
 	xc_get_ADC_Envelope(priv,  &adc_envelope);
 	dprintk(1, "*** ADC envelope (0-1023) = %d\n", adc_envelope);
@@ -638,13 +638,32 @@ static void xc_debug_dump(struct xc5000_
 	dprintk(1, "*** Quality (0:<8dB, 7:>56dB) = %d\n", quality);
 }
 
+static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe);
+
 static int xc5000_set_params(struct dvb_frontend *fe,
 	struct dvb_frontend_parameters *params)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
-	int ret;
+	int ret=0;
 
 	dprintk(1, "%s() frequency=%d (Hz)\n", __FUNCTION__, params->frequency);
+
+	mutex_lock(&priv->lock);
+
+	if(priv->fwloaded == 0) {
+		ret = xc_load_fw_and_init_tuner(fe);
+	}
+#if 0
+	else {
+		ret = xc_initialize(priv);
+		msleep(100);
+	}
+#endif
+	if(ret != XC_RESULT_SUCCESS) {
+		printk(KERN_ERR "xc5000: Unable to initialise tuner\n");
+		mutex_unlock(&priv->lock);
+		return -EREMOTEIO;
+	}
 
 	switch(params->u.vsb.modulation) {
 	case VSB_8:
@@ -665,6 +684,7 @@ static int xc5000_set_params(struct dvb_
 		priv->video_standard = DTV6;
 		break;
 	default:
+		mutex_unlock(&priv->lock);
 		return -EINVAL;
 	}
 
@@ -676,6 +696,7 @@ static int xc5000_set_params(struct dvb_
 		printk(KERN_ERR
 			"xc5000: xc_SetSignalSource(%d) failed\n",
 			priv->rf_mode);
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 
@@ -684,6 +705,7 @@ static int xc5000_set_params(struct dvb_
 		XC5000_Standard[priv->video_standard].AudioMode);
 	if (ret != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR "xc5000: xc_SetTVStandard failed\n");
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 
@@ -691,6 +713,7 @@ static int xc5000_set_params(struct dvb_
 	if (ret != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR "xc5000: xc_Set_IF_frequency(%d) failed\n",
 			priv->cfg->if_khz);
+		mutex_unlock(&priv->lock);
 		return -EIO;
 	}
 
@@ -699,22 +722,36 @@ static int xc5000_set_params(struct dvb_
 	if (debug)
 		xc_debug_dump(priv);
 
-	return 0;
-}
-
-static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe);
+	mutex_unlock(&priv->lock);
+	return 0;
+}
 
 static int xc5000_set_analog_params(struct dvb_frontend *fe,
 	struct analog_parameters *params)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
-	int ret;
-
-	if(priv->fwloaded == 0)
-		xc_load_fw_and_init_tuner(fe);
+	int ret=0;
 
 	dprintk(1, "%s() frequency=%d (in units of 62.5khz)\n",
 		__FUNCTION__, params->frequency);
+
+	mutex_lock(&priv->lock);
+
+	if(priv->fwloaded == 0) {
+		ret = xc_load_fw_and_init_tuner(fe);
+	}
+#if 0
+	else {
+		ret = xc_initialize(priv);
+		msleep(100);
+	}
+#endif
+
+	if(ret != XC_RESULT_SUCCESS) {
+		printk(KERN_ERR "xc5000: Unable to initialise tuner\n");
+		mutex_unlock(&priv->lock);
+		return -EREMOTEIO;
+	}
 
 	priv->rf_mode = XC_RF_MODE_CABLE; /* Fix me: it could be air. */
 
@@ -767,9 +804,10 @@ tune_channel:
 tune_channel:
 	ret = xc_SetSignalSource(priv, priv->rf_mode);
 	if (ret != XC_RESULT_SUCCESS) {
-	printk(KERN_ERR
+		printk(KERN_ERR
 			"xc5000: xc_SetSignalSource(%d) failed\n",
 			priv->rf_mode);
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 
@@ -778,6 +816,7 @@ tune_channel:
 		XC5000_Standard[priv->video_standard].AudioMode);
 	if (ret != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR "xc5000: xc_SetTVStandard failed\n");
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 
@@ -786,6 +825,7 @@ tune_channel:
 	if (debug)
 		xc_debug_dump(priv);
 
+	mutex_unlock(&priv->lock);
 	return 0;
 }
 
@@ -825,12 +865,11 @@ static int xc_load_fw_and_init_tuner(str
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret = 0;
 
-	if (priv->fwloaded == 0) {
-		ret = xc5000_fwupload(fe);
-		if (ret != XC_RESULT_SUCCESS)
-			return ret;
-		priv->fwloaded = 1;
-	}
+	ret = xc5000_fwupload(fe);
+	if (ret != XC_RESULT_SUCCESS) {
+		return ret;
+	}
+	priv->fwloaded = 1;
 
 	/* Start the tuner self-calibration process */
 	ret |= xc_initialize(priv);
@@ -840,7 +879,7 @@ static int xc_load_fw_and_init_tuner(str
 	 * I2C transactions until calibration is complete.  This way we
 	 * don't have to rely on clock stretching working.
 	 */
-	xc_wait( 100 );
+	msleep( 100 );
 
 	/* Default to "CABLE" mode */
 	ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
@@ -855,21 +894,20 @@ static int xc5000_sleep(struct dvb_front
 
 	dprintk(1, "%s()\n", __FUNCTION__);
 
-	/* On Pinnacle PCTV HD 800i, the tuner cannot be reinitialized
-	 * once shutdown without reloading the driver. Maybe I am not
-	 * doing something right.
-	 *
-	 */
+	mutex_lock(&priv->lock);
 
 	ret = xc_shutdown(priv);
 	if(ret != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR
 			"xc5000: %s() unable to shutdown tuner\n",
 			__FUNCTION__);
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 	else {
-		/* priv->fwloaded = 0; */
+		if(allow_shutdown) 
+			priv->fwloaded = 0; /* was indeed shutdown */
+		mutex_unlock(&priv->lock);
 		return XC_RESULT_SUCCESS;
 	}
 }
@@ -877,24 +915,51 @@ static int xc5000_init(struct dvb_fronte
 static int xc5000_init(struct dvb_frontend *fe)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
+	int ret;
+
 	dprintk(1, "%s()\n", __FUNCTION__);
 
-	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+	mutex_lock(&priv->lock);
+
+	if(priv->fwloaded == 0) {
+		ret = xc_load_fw_and_init_tuner(fe);
+	}
+	else {	/* Firmware has been loaded previously, just initialize */
+		ret = xc_initialize(priv);
+		msleep(100);
+		ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
+	}
+
+	if(ret != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR "xc5000: Unable to initialise tuner\n");
+		mutex_unlock(&priv->lock);
 		return -EREMOTEIO;
 	}
 
 	if (debug)
 		xc_debug_dump(priv);
 
+	mutex_unlock(&priv->lock);
 	return 0;
 }
 
 static int xc5000_release(struct dvb_frontend *fe)
 {
+	struct xc5000_priv *priv = fe->tuner_priv;
+
 	dprintk(1, "%s()\n", __FUNCTION__);
-	kfree(fe->tuner_priv);
+
+	mutex_lock(&xc5000_list_lock);
+	
+	priv->count--;
+	if(priv->count == 0) {
+		list_del(&priv->xc5000_list);
+		kfree(priv);
+	}
 	fe->tuner_priv = NULL;
+
+	mutex_unlock(&xc5000_list_lock);
+
 	return 0;
 }
 
@@ -922,23 +987,49 @@ struct dvb_frontend * xc5000_attach(stru
 	struct xc5000_config *cfg)
 {
 	struct xc5000_priv *priv = NULL;
+	void 		   *cfg_priv;
 	u16 id = 0;
 
 	dprintk(1, "%s()\n", __FUNCTION__);
 
-	priv = kzalloc(sizeof(struct xc5000_priv), GFP_KERNEL);
-	if (priv == NULL)
+	if (NULL == cfg || NULL == cfg->priv || NULL == fe)
 		return NULL;
 
-	priv->cfg = cfg;
-	priv->bandwidth = BANDWIDTH_6_MHZ;
-	priv->i2c = i2c;
+	cfg_priv = cfg->priv;
+
+	mutex_lock(&xc5000_list_lock);
+
+	list_for_each_entry(priv, &xc5000_list, xc5000_list) {
+		if (priv->cfg->priv == cfg->priv) {
+			cfg_priv = NULL;
+			break;
+		}
+	}
+
+	if(cfg_priv) {
+		priv = kzalloc(sizeof(struct xc5000_priv), GFP_KERNEL);
+		if (priv == NULL) {
+			mutex_unlock(&xc5000_list_lock);
+			return NULL;
+		}
+
+		priv->cfg = cfg;
+		priv->bandwidth = BANDWIDTH_6_MHZ;
+		priv->i2c = i2c;
+		priv->fwloaded = 0;
+		priv->count = 0;
+
+		mutex_init(&priv->lock);
+		list_add_tail(&priv->xc5000_list, &xc5000_list);
+	}
+
 
 	/* Check if firmware has been loaded. It is possible that another
 	   instance of the driver has loaded the firmware.
 	 */
 	if (xc5000_readreg(priv, XREG_PRODUCT_ID, &id) != 0) {
 		kfree(priv);
+		mutex_unlock(&xc5000_list_lock);
 		return NULL;
 	}
 
@@ -964,6 +1055,7 @@ struct dvb_frontend * xc5000_attach(stru
 			"xc5000: Device not found at addr 0x%02x (0x%x)\n",
 			cfg->i2c_address, id);
 		kfree(priv);
+		mutex_unlock(&xc5000_list_lock);
 		return NULL;
 	}
 
@@ -971,11 +1063,14 @@ struct dvb_frontend * xc5000_attach(stru
 		sizeof(struct dvb_tuner_ops));
 
 	fe->tuner_priv = priv;
-
+	priv->count++;
+
+	mutex_unlock(&xc5000_list_lock);
 	return fe;
 }
 EXPORT_SYMBOL(xc5000_attach);
 
 MODULE_AUTHOR("Steven Toth");
+MODULE_AUTHOR("Chaogui Zhang");
 MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
 MODULE_LICENSE("GPL");
diff -r a7f8e825c09b linux/drivers/media/dvb/frontends/xc5000_priv.h
--- a/linux/drivers/media/dvb/frontends/xc5000_priv.h	Thu Jan 24 07:59:20 2008 -0200
+++ b/linux/drivers/media/dvb/frontends/xc5000_priv.h	Fri Jan 25 11:46:34 2008 -0500
@@ -23,6 +23,7 @@
 #define XC5000_PRIV_H
 
 struct xc5000_priv {
+	struct list_head     xc5000_list;
 	struct xc5000_config *cfg;
 	struct i2c_adapter   *i2c;
 
@@ -31,6 +32,14 @@ struct xc5000_priv {
 	u8  video_standard;
 	u8  rf_mode;
 	u8  fwloaded;
+
+	int count;
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 16)
+	struct mutex lock;
+#else
+	struct semaphore lock;
+#endif
 };
 
 #endif



More information about the linux-dvb mailing list