| Message ID | 20191114200408.28883-5-brad@nextdimension.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | si2157: Analog tuning and optimizations | expand |
On 11/14/19 10:03 PM, Brad Love wrote: > Include set_analog_params, get_frequency, and get_bandwidth. > > Tested with NTSC and PAL standards via ch3/4 generator. Other standards > are included, but are untested due to lack of generator. > > Signed-off-by: Brad Love <brad@nextdimension.cc> > --- > Changes since v1: > - remove __func__ from dev_dbg macros After all it looks pretty simply, but implementation is not done that simply. Crazy RF/IF offsets, impossible values and so. I think you need to study some tuner basics: * what IF frequency is, why, and so * IF vs. BW, what is relation, what are possible values * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but it is fundamental. So basics of integer-N and fractional-N PLL is always you must to know. * Filtering. Especially IF filtering, which is generally low-pass filtering. Think possible filters when selecting IF. regards Antti
Em Sun, 24 Nov 2019 07:09:07 +0200 Antti Palosaari <crope@iki.fi> escreveu: > On 11/14/19 10:03 PM, Brad Love wrote: > > Include set_analog_params, get_frequency, and get_bandwidth. > > > > Tested with NTSC and PAL standards via ch3/4 generator. Other standards > > are included, but are untested due to lack of generator. > > > > Signed-off-by: Brad Love <brad@nextdimension.cc> > > --- > > Changes since v1: > > - remove __func__ from dev_dbg macros > > After all it looks pretty simply, but implementation is not done that > simply. Crazy RF/IF offsets, impossible values and so. > > I think you need to study some tuner basics: > * what IF frequency is, why, and so > * IF vs. BW, what is relation, what are possible values > * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but > it is fundamental. So basics of integer-N and fractional-N PLL is always > you must to know. > * Filtering. Especially IF filtering, which is generally low-pass > filtering. Think possible filters when selecting IF. For me, the implementation seems to make sense. I mean, for analog TV, both channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard only. So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is a channel at VHF or at UHF range. So, this part of the patch sounds OK to me. The IF is actually a little trickier. Yet, if you take a lok on other tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to automatically set the IF that will work for each video standard: static void tda827x_set_std(struct dvb_frontend *fe, struct analog_parameters *params) { struct tda827x_priv *priv = fe->tuner_priv; char *mode; priv->lpsel = 0; if (params->std & V4L2_STD_MN) { priv->sgIF = 92; priv->lpsel = 1; mode = "MN"; } else if (params->std & V4L2_STD_B) { priv->sgIF = 108; mode = "B"; ... static int tda827xo_set_analog_params(struct dvb_frontend *fe, struct analog_parameters *params) { ... N = freq + priv->sgIF; In other words, for analog TV, the tuner will always receive the channel central frequency, with may vary depending on the video standard, and will adjust it to tune at the right channel, using the per-standard IF (if needed), as, on most tuner drivers, the tunning frequency should be either initial frequency or the main carrier frequency, and not the center frequency. Cheers, Mauro
On 12/19/19 3:13 PM, Mauro Carvalho Chehab wrote: > Em Sun, 24 Nov 2019 07:09:07 +0200 > Antti Palosaari <crope@iki.fi> escreveu: > >> On 11/14/19 10:03 PM, Brad Love wrote: >>> Include set_analog_params, get_frequency, and get_bandwidth. >>> >>> Tested with NTSC and PAL standards via ch3/4 generator. Other standards >>> are included, but are untested due to lack of generator. >>> >>> Signed-off-by: Brad Love <brad@nextdimension.cc> >>> --- >>> Changes since v1: >>> - remove __func__ from dev_dbg macros >> >> After all it looks pretty simply, but implementation is not done that >> simply. Crazy RF/IF offsets, impossible values and so. >> >> I think you need to study some tuner basics: >> * what IF frequency is, why, and so >> * IF vs. BW, what is relation, what are possible values >> * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but >> it is fundamental. So basics of integer-N and fractional-N PLL is always >> you must to know. >> * Filtering. Especially IF filtering, which is generally low-pass >> filtering. Think possible filters when selecting IF. > > For me, the implementation seems to make sense. I mean, for analog TV, both > channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard > only. > > So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it > may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is > a channel at VHF or at UHF range. > > So, this part of the patch sounds OK to me. > > The IF is actually a little trickier. Yet, if you take a lok on other > tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to > automatically set the IF that will work for each video standard: > > static void tda827x_set_std(struct dvb_frontend *fe, > struct analog_parameters *params) > { > struct tda827x_priv *priv = fe->tuner_priv; > char *mode; > > priv->lpsel = 0; > if (params->std & V4L2_STD_MN) { > priv->sgIF = 92; > priv->lpsel = 1; > mode = "MN"; > } else if (params->std & V4L2_STD_B) { > priv->sgIF = 108; > mode = "B"; > ... > > static int tda827xo_set_analog_params(struct dvb_frontend *fe, > struct analog_parameters *params) > { > > ... > > N = freq + priv->sgIF; > > In other words, for analog TV, the tuner will always receive the > channel central frequency, with may vary depending on the video > standard, and will adjust it to tune at the right channel, using the > per-standard IF (if needed), as, on most tuner drivers, the tunning > frequency should be either initial frequency or the main carrier > frequency, and not the center frequency. How the carrier central frequency can be vary tings like video standard? If you tune to some channel like 654.321MHz then that is central frequency no matter what kind of bandwidth is used. And as I pointed those strange offset, please tell me what is that 1250000 Hz offset? It simply looks nonsense. You first add 1250000 to RF frequency, then you compensate same from IF value - the carrier at IF will be just same. Also, there was something like 6/7/8MHz wide channel dropped to IF only 1.25MHz. How that wide channel could fit that small space? regards Antti
Hi Mauro, On 12/19/19 7:13 AM, Mauro Carvalho Chehab wrote: > Em Sun, 24 Nov 2019 07:09:07 +0200 > Antti Palosaari <crope@iki.fi> escreveu: > >> On 11/14/19 10:03 PM, Brad Love wrote: >>> Include set_analog_params, get_frequency, and get_bandwidth. >>> >>> Tested with NTSC and PAL standards via ch3/4 generator. Other standards >>> are included, but are untested due to lack of generator. >>> >>> Signed-off-by: Brad Love <brad@nextdimension.cc> >>> --- >>> Changes since v1: >>> - remove __func__ from dev_dbg macros >> After all it looks pretty simply, but implementation is not done that >> simply. Crazy RF/IF offsets, impossible values and so. >> >> I think you need to study some tuner basics: >> * what IF frequency is, why, and so >> * IF vs. BW, what is relation, what are possible values >> * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but >> it is fundamental. So basics of integer-N and fractional-N PLL is always >> you must to know. >> * Filtering. Especially IF filtering, which is generally low-pass >> filtering. Think possible filters when selecting IF. > For me, the implementation seems to make sense. I mean, for analog TV, both > channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard > only. > > So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it > may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is > a channel at VHF or at UHF range. > > So, this part of the patch sounds OK to me. > > The IF is actually a little trickier. Yet, if you take a lok on other > tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to > automatically set the IF that will work for each video standard: > > static void tda827x_set_std(struct dvb_frontend *fe, > struct analog_parameters *params) > { > struct tda827x_priv *priv = fe->tuner_priv; > char *mode; > > priv->lpsel = 0; > if (params->std & V4L2_STD_MN) { > priv->sgIF = 92; > priv->lpsel = 1; > mode = "MN"; > } else if (params->std & V4L2_STD_B) { > priv->sgIF = 108; > mode = "B"; > ... > > static int tda827xo_set_analog_params(struct dvb_frontend *fe, > struct analog_parameters *params) > { > > ... > > N = freq + priv->sgIF; > > In other words, for analog TV, the tuner will always receive the > channel central frequency, with may vary depending on the video > standard, and will adjust it to tune at the right channel, using the > per-standard IF (if needed), as, on most tuner drivers, the tunning > frequency should be either initial frequency or the main carrier > frequency, and not the center frequency. > > > Cheers, > Mauro This code has been widely tested by multiple Hauppauge customers across North America and Europe. This code has been in use by various parties for years, with zero issues reported. I am merely working to upstream all of the code we have generated over the years. It took a while for me to get tester and test bench time, but included below is results from using an analog generator and testing channels across the entire frequency range for ATSC and PAL-I analog TV channels. First lock and signal strength were verified, then video and audio signal decoding was verified. Decoding is flawless on every channel tried. Software used for validation is scantv, tvtime, vlc, and qv4l2. Cheers, Brad $ ######################### $ # USA ATSC TESTING $ ######################### $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 5 ( 77.25 MHz): ??? [unknown (5)] channel = 5 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 10 (193.25 MHz): ??? [unknown (10)] channel = 10 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 17 (489.25 MHz): ??? [unknown (17)] channel = 17 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 30 (567.25 MHz): ??? [unknown (30)] channel = 30 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 40 (627.25 MHz): ??? [unknown (40)] channel = 40 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 50 (687.25 MHz): ??? [unknown (50)] channel = 50 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 60 (747.25 MHz): ??? [unknown (60)] channel = 60 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 70 (807.25 MHz): ??? [unknown (70)] channel = 70 <...> $ $ scantv -n NTSC-M -f us-bcast vid-open-auto: using analog TV device /dev/video0 [global] freqtab = us-bcast [defaults] input = Television norm = NTSC-M scanning channel list us-bcast... <...> 83 (885.25 MHz): ??? [unknown (83)] channel = 83 $ ######################### $ # EUROPEAN PAL-I TESTING $ ######################### $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 21 (471.25 MHz): ??? [unknown (21)] channel = 21 <...> $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 30 (543.25 MHz): ??? [unknown (30)] channel = 30 <...> $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 40 (623.25 MHz): ??? [unknown (40)] channel = 40 <...> $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 50 (703.25 MHz): ??? [unknown (50)] channel = 50 <...> $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 60 (783.25 MHz): ??? [unknown (60)] channel = 60 <...> $ $ scantv -n PAL-I -f europe-west vid-open-auto: using analog TV device /dev/video0 [global] freqtab = europe-west [defaults] input = Television norm = PAL-I scanning channel list europe-west... <...> 68 (847.25 MHz): ??? [unknown (68)] channel = 68 <...> $ $ $
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index e1e23e78fd19..78a855df30da 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -371,7 +371,7 @@ static int si2157_set_params(struct dvb_frontend *fe) if (ret) goto err; - /* set if frequency if needed */ + /* set digital if frequency if needed */ if (if_frequency != dev->if_frequency) { memcpy(cmd.args, "\x14\x00\x06\x07", 4); cmd.args[4] = (if_frequency / 1000) & 0xff; @@ -385,7 +385,7 @@ static int si2157_set_params(struct dvb_frontend *fe) dev->if_frequency = if_frequency; } - /* set frequency */ + /* set digital frequency */ memcpy(cmd.args, "\x41\x00\x00\x00\x00\x00\x00\x00", 8); cmd.args[4] = (c->frequency >> 0) & 0xff; cmd.args[5] = (c->frequency >> 8) & 0xff; @@ -397,18 +397,239 @@ static int si2157_set_params(struct dvb_frontend *fe) if (ret) goto err; + dev->bandwidth = bandwidth; + dev->frequency = c->frequency; + return 0; err: + dev->bandwidth = 0; + dev->frequency = 0; + dev->if_frequency = 0; dev_dbg(&client->dev, "failed=%d\n", ret); return ret; } +static int si2157_set_analog_params(struct dvb_frontend *fe, + struct analog_parameters *params) +{ + struct i2c_client *client = fe->tuner_priv; + struct si2157_dev *dev = i2c_get_clientdata(client); + char *std; /* for debugging */ + int ret; + struct si2157_cmd cmd; + u32 bandwidth = 0; + u32 if_frequency = 0; + u32 freq = 0; + u64 tmp_lval = 0; + u8 system = 0; + u8 color = 0; /* 0=NTSC/PAL, 0x10=SECAM */ + u8 invert_analog = 1; /* analog tuner spectrum; 0=normal, 1=inverted */ + + if (dev->chiptype != SI2157_CHIPTYPE_SI2157) { + dev_info(&client->dev, "Analog tuning not supported for chiptype=%u\n", + dev->chiptype); + ret = -EINVAL; + goto err; + } + + if (!dev->active) + si2157_init(fe); + + if (!dev->active) { + ret = -EAGAIN; + goto err; + } + if (params->mode == V4L2_TUNER_RADIO) { + /* + * std = "fm"; + * bandwidth = 1700000; //best can do for FM, AGC will be a mess though + * if_frequency = 1250000; //HVR-225x(saa7164), HVR-12xx(cx23885) + * if_frequency = 6600000; //HVR-9xx(cx231xx) + * if_frequency = 5500000; //HVR-19xx(pvrusb2) + */ + dev_err(&client->dev, "si2157 does not currently support FM radio\n"); + ret = -EINVAL; + goto err; + } + tmp_lval = params->frequency * 625LL; + do_div(tmp_lval, 10); /* convert to HZ */ + freq = (u32)tmp_lval; + + if (freq < 1000000) /* is freq in KHz */ + freq = freq * 1000; + dev->frequency = freq; + + /* if_frequency values based on tda187271C2 */ + if (params->std & (V4L2_STD_B | V4L2_STD_GH)) { + if (freq >= 470000000) { + std = "palGH"; + bandwidth = 8000000; + if_frequency = 6000000; + system = 1; + if (params->std & (V4L2_STD_SECAM_G | V4L2_STD_SECAM_H)) { + std = "secamGH"; + color = 0x10; + } + } else { + std = "palB"; + bandwidth = 7000000; + if_frequency = 6000000; + system = 0; + if (params->std & V4L2_STD_SECAM_B) { + std = "secamB"; + color = 0x10; + } + } + } else if (params->std & V4L2_STD_MN) { + std = "MN"; + bandwidth = 6000000; + if_frequency = 5400000; + system = 2; + } else if (params->std & V4L2_STD_PAL_I) { + std = "palI"; + bandwidth = 8000000; + if_frequency = 7250000; /* TODO: does not work yet */ + system = 4; + } else if (params->std & V4L2_STD_DK) { + std = "palDK"; + bandwidth = 8000000; + if_frequency = 6900000; /* TODO: does not work yet */ + system = 5; + if (params->std & V4L2_STD_SECAM_DK) { + std = "secamDK"; + color = 0x10; + } + } else if (params->std & V4L2_STD_SECAM_L) { + std = "secamL"; + bandwidth = 8000000; + if_frequency = 6750000; /* TODO: untested */ + system = 6; + color = 0x10; + } else if (params->std & V4L2_STD_SECAM_LC) { + std = "secamL'"; + bandwidth = 7000000; + if_frequency = 1250000; /* TODO: untested */ + system = 7; + color = 0x10; + } else { + std = "unknown"; + } + /* calc channel center freq */ + freq = freq - 1250000 + (bandwidth / 2); + + dev_dbg(&client->dev, + "mode=%d system=%u std='%s' params->frequency=%u center freq=%u if=%u bandwidth=%u\n", + params->mode, system, std, params->frequency, + freq, if_frequency, bandwidth); + + /* set analog IF port */ + memcpy(cmd.args, "\x14\x00\x03\x06\x08\x02", 6); + /* in using dev->if_port, we assume analog and digital IF's */ + /* are always on different ports */ + /* assumes if_port definition is 0 or 1 for digital out */ + cmd.args[4] = (dev->if_port == 1) ? 8 : 10; + /* Analog AGC assumed external */ + cmd.args[5] = (dev->if_port == 1) ? 2 : 1; + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + /* set analog IF output config */ + memcpy(cmd.args, "\x14\x00\x0d\x06\x94\x64", 6); + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + /* make this distinct from a digital IF */ + dev->if_frequency = if_frequency | 1; + + /* calc and set tuner analog if center frequency */ + if_frequency = if_frequency + 1250000 - (bandwidth / 2); + dev_dbg(&client->dev, "IF Ctr freq=%d\n", if_frequency); + + memcpy(cmd.args, "\x14\x00\x0C\x06", 4); + cmd.args[4] = (if_frequency / 1000) & 0xff; + cmd.args[5] = ((if_frequency / 1000) >> 8) & 0xff; + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + /* set analog AGC config */ + memcpy(cmd.args, "\x14\x00\x07\x06\x32\xc8", 6); + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + /* set analog video mode */ + memcpy(cmd.args, "\x14\x00\x04\x06\x00\x00", 6); + cmd.args[4] = system | color; + /* can use dev->inversion if assumed applies to both digital/analog */ + if (invert_analog) + cmd.args[5] |= 0x02; + cmd.wlen = 6; + cmd.rlen = 1; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + /* set analog frequency */ + memcpy(cmd.args, "\x41\x01\x00\x00\x00\x00\x00\x00", 8); + cmd.args[4] = (freq >> 0) & 0xff; + cmd.args[5] = (freq >> 8) & 0xff; + cmd.args[6] = (freq >> 16) & 0xff; + cmd.args[7] = (freq >> 24) & 0xff; + cmd.wlen = 8; + cmd.rlen = 1; + ret = si2157_cmd_execute(client, &cmd); + if (ret) + goto err; + + dev->bandwidth = bandwidth; + + return 0; +err: + dev->bandwidth = 0; + dev->frequency = 0; + dev->if_frequency = 0; + dev_dbg(&client->dev, "failed=%d\n", ret); + return ret; +} + +static int si2157_get_frequency(struct dvb_frontend *fe, u32 *frequency) +{ + struct i2c_client *client = fe->tuner_priv; + struct si2157_dev *dev = i2c_get_clientdata(client); + + *frequency = dev->frequency; + dev_dbg(&client->dev, "freq=%u\n", dev->frequency); + return 0; +} + +static int si2157_get_bandwidth(struct dvb_frontend *fe, u32 *bandwidth) +{ + struct i2c_client *client = fe->tuner_priv; + struct si2157_dev *dev = i2c_get_clientdata(client); + + *bandwidth = dev->bandwidth; + dev_dbg(&client->dev, "bandwidth=%u\n", dev->bandwidth); + return 0; +} + static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency) { struct i2c_client *client = fe->tuner_priv; struct si2157_dev *dev = i2c_get_clientdata(client); - *frequency = dev->if_frequency; + *frequency = dev->if_frequency & ~1; /* strip analog IF indicator bit */ + dev_dbg(&client->dev, "if_frequency=%u\n", *frequency); return 0; } @@ -422,6 +643,9 @@ static const struct dvb_tuner_ops si2157_ops = { .init = si2157_init, .sleep = si2157_sleep, .set_params = si2157_set_params, + .set_analog_params = si2157_set_analog_params, + .get_frequency = si2157_get_frequency, + .get_bandwidth = si2157_get_bandwidth, .get_if_frequency = si2157_get_if_frequency, }; diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h index 778f81b39996..ef4796098931 100644 --- a/drivers/media/tuners/si2157_priv.h +++ b/drivers/media/tuners/si2157_priv.h @@ -29,6 +29,8 @@ struct si2157_dev { u8 chiptype; u8 if_port; u32 if_frequency; + u32 bandwidth; + u32 frequency; struct delayed_work stat_work; #if defined(CONFIG_MEDIA_CONTROLLER)
Include set_analog_params, get_frequency, and get_bandwidth. Tested with NTSC and PAL standards via ch3/4 generator. Other standards are included, but are untested due to lack of generator. Signed-off-by: Brad Love <brad@nextdimension.cc> --- Changes since v1: - remove __func__ from dev_dbg macros drivers/media/tuners/si2157.c | 230 ++++++++++++++++++++++++++++- drivers/media/tuners/si2157_priv.h | 2 + 2 files changed, 229 insertions(+), 3 deletions(-)