[V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134
[V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134 When Medion 7134 analog+DVB-T card is cold (after powerup) the tda9887 analog demodulator won't show on i2c bus. This results in no signal on analog TV. After loading driver for second time eeprom (required for tuner autodetection) read is corrupted, but tda9987 is detected properly and analog TV works when tuner model is forced. Fix tda9887 problem by moving its detect code after tuner setup which unhides it. The eeprom read issue is fixed by opening i2c gate in DVB-T demodulator. Tested on Medion 7134 and also tested for reference on Typhoon Cardbus Hybrid (which also uses saa7134 driver). Signed-off-by: Maciej Szmigiero m...@o2.pl --- a/drivers/media/video/saa7134/saa7134-cards.c 2010-08-02 00:11:14.0 +0200 +++ b/drivers/media/video/saa7134/saa7134-cards.c 2010-10-25 19:19:08.0 +0200 @@ -7249,12 +7249,37 @@ break; case SAA7134_BOARD_MD7134: { - u8 subaddr; + u8 subaddr, dmdregval; u8 data[3]; int ret, tuner_t; + struct i2c_msg i2cgatemsg_r[] = { {.addr = 0x08, .flags = 0, + .buf = subaddr, .len = 1}, + {.addr = 0x08, + .flags = I2C_M_RD, + .buf = dmdregval, .len = 1} }; + struct i2c_msg i2cgatemsg_w[] = { {.addr = 0x08, .flags = 0, + .buf = data, .len = 2} }; struct i2c_msg msg[] = {{.addr=0x50, .flags=0, .buf=subaddr, .len = 1}, {.addr=0x50, .flags=I2C_M_RD, .buf=data, .len = 3}}; + /* open i2c gate in DVB-T demod */ + /* so eeprom read won't be corrupted */ + subaddr = 0x7; + ret = i2c_transfer(dev-i2c_adap, i2cgatemsg_r, 2); + if ((ret == 2) (dmdregval 0x2)) { + printk(KERN_NOTICE %s DVB-T demod i2c gate was left +closed\n, dev-name); + printk(KERN_NOTICE %s previous informational +EEPROM read might have been +corrupted\n, dev-name); + + data[0] = 0x7; + data[1] = (dmdregval ~0x2); + if (i2c_transfer(dev-i2c_adap, i2cgatemsg_w, 1) != 1) + printk(KERN_ERR early i2c gate +open failure\n); + } + subaddr= 0x14; tuner_t = 0; @@ -7522,10 +7547,6 @@ v4l2_i2c_new_subdev(dev-v4l2_dev, dev-i2c_adap, tuner, tuner, dev-radio_addr, NULL); - if (has_demod) - v4l2_i2c_new_subdev(dev-v4l2_dev, - dev-i2c_adap, tuner, tuner, - 0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); if (dev-tuner_addr == ADDR_UNSET) { enum v4l2_i2c_tuner_type type = has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV; @@ -7542,6 +7563,15 @@ saa7134_tuner_setup(dev); + /* some cards (Medion 7134 for example) needs tuner to be setup */ + /* before tda9887 shows itself on i2c bus */ + if ((TUNER_ABSENT != dev-tuner_type) +(dev-tda9887_conf TDA9887_PRESENT)) { + v4l2_i2c_new_subdev(dev-v4l2_dev, + dev-i2c_adap, tuner, tuner, + 0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); + } + switch (dev-board) { case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM: case SAA7134_BOARD_AVERMEDIA_CARDBUS_501: -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134
W dniu 09.11.2010 11:53, Mauro Carvalho Chehab pisze: Em 25-10-2010 15:59, Maciej Szmigiero escreveu: +printk(KERN_NOTICE %s DVB-T demod i2c gate was left + closed\n, dev-name); +printk(KERN_NOTICE %s previous informational + EEPROM read might have been + corrupted\n, dev-name); hmm... I don't think we need those debug messages on normal cases. I added this message because when the gate was left closed the eeprom content (printed out unconditionally in saa7134_i2c_eeprom) looks garbled in dmesg, so it's better to inform user that he (or she) shouldn't be worried about this. The eeprom dump is called from saa7134_i2c_register, before card-specific code has opportunity to run. saa7134_tuner_setup(dev); +/* some cards (Medion 7134 for example) needs tuner to be setup */ +/* before tda9887 shows itself on i2c bus */ +if ((TUNER_ABSENT != dev-tuner_type) + (dev-tda9887_conf TDA9887_PRESENT)) { +v4l2_i2c_new_subdev(dev-v4l2_dev, +dev-i2c_adap, tuner, tuner, +0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); +} + switch (dev-board) { case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM: case SAA7134_BOARD_AVERMEDIA_CARDBUS_501: The order change for the demod probe will likely break support for other boards. If the problem is specific to Medion 7134, what you should do, instead, is to change the order just for MD7134 (so, inside the switch(dev-board)). It was done in that order (tuner first then tda9887) for a long time before it was changed. For example ( http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/media/video/saa7134/saa7134-core.c;hb=7a766f9ddd74b50d6069f054a3004ece0439f5c1 ) it used to look like that: 942 /* load i2c helpers */ 943 if (TUNER_ABSENT != dev-tuner_type) 944 request_module(tuner); 945 if (dev-tda9887_conf) 946 request_module(tda9887); 947 if (card_is_empress(dev)) { 948 request_module(saa6752hs); 949 request_module_depend(saa7134-empress,need_empress); 950 } But then the code for tda9887 was integrated into tuner module and later split out again, this time reversing the detection order (by accident I suppose). Cheers, Mauro. Best regards, Maciej Szmigiero -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[V4L2]decrement struct v4l2_device refcount on device urnegister
commit bedf8bcf6b4f90a6e31add3721a2e71877289381 introduced reference counting for struct v4l2_device. In v4l2_device_register() a call to kref_init() initializes reference count to 1, but in v4l2_device_unregister() there is no corresponding decrement. End result is that reference count never reaches zero and v4l2_device_release() is never called, not even on videodev module unload. Fix this by adding reference counter decrement to v4l2_device_unregister(). Signed-off-by: Maciej Szmigiero m...@o2.pl diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c index c72856c..eb39af9 100644 --- a/drivers/media/video/v4l2-device.c +++ b/drivers/media/video/v4l2-device.c @@ -131,6 +131,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev) } #endif } + +v4l2_device_put(v4l2_dev); } EXPORT_SYMBOL_GPL(v4l2_device_unregister); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[V4L2]decrement struct v4l2_device refcount on device unregister
commit bedf8bcf6b4f90a6e31add3721a2e71877289381 introduced reference counting for struct v4l2_device. In v4l2_device_register() a call to kref_init() initializes reference count to 1, but in v4l2_device_unregister() there is no corresponding decrement. End result is that reference count never reaches zero and v4l2_device_release() is never called, not even on videodev module unload. Fix this by adding reference counter decrement to v4l2_device_unregister(). Resending due to spurious newlines around the patch in previous message. Signed-off-by: Maciej Szmigiero m...@o2.pl diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c index c72856c..eb39af9 100644 --- a/drivers/media/video/v4l2-device.c +++ b/drivers/media/video/v4l2-device.c @@ -131,6 +131,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev) } #endif } + + v4l2_device_put(v4l2_dev); } EXPORT_SYMBOL_GPL(v4l2_device_unregister); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]Medion 95700 analog video support
W dniu 04.09.2011 21:05, Arnaud Lacombe pisze: Hi, On Sun, Sep 4, 2011 at 2:51 PM, Maciej Szmigiero m...@o2.pl wrote: This patch adds support for analog part of Medion 95700 in the cxusb driver. For what reason am I CC'ed on this ? Most of the relevant changes I ever made in the media tree were only mechanical. Thanks, - Arnaud Hi, get_maintainer.pl script output on this patch returned your name, thats why it was included. If thats a mistake I'm sorry for spamming you. Best regards, Maciej Szmigiero -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]Medion 95700 analog video support
W dniu 04.09.2011 21:46, Michael Krufky pisze: Maciej, I'm excited to see some success getting analog to work in the dvb-usb framework. Some people have been asking for this support in the cxusb driver for a long time. I have a device (DViCO FusionHDTV5 usb) that should work with this patch with some small modifications -- i will try it out. I see that this patch adds analog support to cxusb... have you thought at all about adding generic analog support callbacks to the dvb-usb framework? There are some other dvb-usb devices based on the dib0700 chipset that also also use the cx25840 decoder for analog -- it would be great if this can be done in a way to benefit both the dib0700 and cxusb drivers. I will let you know how things go after I try this code on my own device, here. Thanks for your patch. -Mike Krufky Hi and thanks for reply, I think whether the code should be moved to dvb-usb framework really depends on how much the devices have in common - Medion uses a generic Cypress FX2 USB bridge which is programmed (by firmware) to post an ISO buffer once there is exactly 1010 bytes received on its parallel interface. This parallel interface is 8-bit wide and has data inputs connected to cx25840 video interface data output and clock input to cx25840 pixel clock output. USB bridge does not modify this data in any way, nor it does any alignment. So we have a raw BT.656 (or VESA VIP) stream coming from the device. Because there are 3 such 1010-byte packets per (micro)frame the URB frame descriptor has to be 3030 bytes in length (or more) or data will be truncated, which will result in parts of field being all-green. If your device has similar characteristics then it is just matter of substituting a few commands in code (and maybe changing CXUSB_VIDEO_PKT_SIZE to different value if it does not use 3*1010 byte frames). Otherwise, for example if device outputs simple YUV data without any framing, the code will pretty much be different - other than a bit of V4L2 driver glue code which can be shared. Best regards and hope this helps, Maciej Szmigiero -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]Medion 95700 analog video support
W dniu 23.09.2011 23:06, Andy Walls pisze: Michael Krufky mkru...@linuxtv.org wrote: The cx25840 part of the patch breaks ivtv, IIRC. The patch really need to add board specific configuration and behavior to cx25840. I'll have time tomorrow late afternoon to properly reviw and comment. Regards, Andy Have you already narrowed it down which part of the cx25840 patch breaks ivtv - maybe it is setting the defaults at init or change to check for plain I2C instead of SMBus? Best regards, Maciej Szmigiero -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]Medion 95700 analog video support
W dniu 24.09.2011 22:21, Andy Walls pisze: Hi Maciej, I'll try and comment on the specific areas below, but overall the problem is this: 1. The default setup and behavior of the cx25840 module was written around hardware designs supported by the ivtv driver: i.e. interfacing to a CX23416 MPEG encoder. 2. The ivtv and pvrusb2 drivers rely on that default setup and behavior of the cx25840 module. 3. The PVR-150 and PVR-500 are very popular cards supported by ivtv that use a CX25843 and CX23416. Many MythTV users still have these cards in service. 4. The ivtv driver also supports other hardware designs that use different encoders, so trying fix ivtv to match new changes in the cx25840 will ripples along to other analog video decoder drivers. This would result in a lot of time to perform regression testing with as many different ivtv supported capture cards as possible. What I recommend is that you rework your changes so that the cx25840 module is provided information by the bridge driver as to the board model, and then have the cx25840 module behave appropriately based on the board information passed in by the bridge driver. 1. Add whatever data fields you think you need to the struct cx25840_platform_data structure in include/media/cx25840.h. Maybe something as simple as bool is_medion95700 2. In cxusb-analog.c you instantiate the cx25840 sub-device with v4l2_i2c_new_subdev_board() with the cx25840 platform data filled in as needed for the Medion 95700. Look at drivers/media/video/ivtv/ivtv-i2c.c:ivtv_i2c_register() for an example of how this is done for the cx25840 module. 3. Modify the cx25840 module to behave as you need it if the platform data indicates a Medion 95700; otherwise, leave the default cx25840 setup and behavior. Hi Andy, Thanks for you detailed explanation, I did not know that ivtv boards are that quirky with regard to VBI capture. I will do as you wrote above, make my changes to cx25840 driver conditional, so ivtv won't be affected. Any specific comments I have are in-line below: @@ -18,6 +18,9 @@ * CX2388[578] IRQ handling, IO Pin mux configuration and other small fixes are * Copyright (C) 2010 Andy Walls awa...@md.metrocast.net * + * CX2384x pin to pad mapping and output format configuration support are ^^^ CX2584x? if ((fmt-width * 16 Hsrc) || (Hsrc fmt-width) || (Vlines * 8 Vsrc) || (Vsrc Vlines)) { @@ -1403,6 +1695,112 @@ static void log_audio_status(struct i2c_client *client) } } +#define CX25480_VCONFIG_OPTION(option_mask) \ ^^ CX25840? +if (config_in option_mask) { \ +state-vid_config = ~(option_mask); \ +state-vid_config |= config_in option_mask; \ +} \ + +#define CX25480_VCONFIG_SET_BIT(optionmask, reg, bit, oneval) \ ^^ CX25840? You mean here that it should be named consistently either as CX2584x or CX25840? return set_input(client, input, state-aud_input); } @@ -1877,7 +2278,7 @@ static int cx25840_probe(struct i2c_client *client, u16 device_id; /* Check if the adapter supports the needed features */ -if (!i2c_check_functionality(client-adapter, I2C_FUNC_SMBUS_BYTE_DATA)) +if (!i2c_check_functionality(client-adapter, I2C_FUNC_I2C)) return -EIO; On the surface, this change doesn't appear to adversely affect the ivtv, pvrusb2, cx23885, and cx231xx bridge drivers. I would need to take a hard look at the CX2584[0123], CX2583[67], CX2388[578], and CX2310[12] datasheets to see why, and if, all the Mako core variants require I2C_FUNC_SMBUS_BYTE_DATA. However, if the cxusb bridge has a full I2C master, shouldn't the cxusb driver be specifying (I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL) as its functionality? See Documentation/i2c/functionality. Adding I2C_FUNC_SMBUS_EMUL flag to cxusb i2c host seems to be a right thing to do for now, but I would be very surprised if any of Conexant video decoders actually used SMBus instead of plain I2C. Best regards, Maciej Szmigiero -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html