* PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS @ 2011-11-21 21:06 Manu Abraham 2011-11-21 21:21 ` Michael Krufky 2011-11-22 0:04 ` Andreas Oberritter 0 siblings, 2 replies; 7+ messages in thread From: Manu Abraham @ 2011-11-21 21:06 UTC (permalink / raw) To: Linux Media Mailing List Cc: Mauro Carvalho Chehab, Andreas Oberritter, Michael Krufky [-- Attachment #1: Type: text/plain, Size: 1 bytes --] [-- Attachment #2: 0004-TDA18271-Allow-frontend-to-set-DELSYS-rather-than-qu.patch --] [-- Type: text/x-patch, Size: 4008 bytes --] From 2ece38602678ae323450d0e35379147e6e086326 Mon Sep 17 00:00:00 2001 From: Manu Abraham <abraham.manu@gmail.com> Date: Sat, 19 Nov 2011 19:50:09 +0530 Subject: [PATCH 04/13] TDA18271: Allow frontend to set DELSYS, rather than querying fe->ops.info.type With any tuner that can tune to multiple delivery systems/standards, it does query fe->ops.info.type to determine frontend type and set the delivery system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK, FE_QAM, FE_OFDM and FE_ATSC. The change allows the tuner to be set to any delivery system specified in fe_delivery_system_t, thereby simplifying a lot of issues. Signed-off-by: Manu Abraham <abraham.manu@gmail.com> --- drivers/media/common/tuners/tda18271-fe.c | 80 +++++++++++++++++++++++++++ drivers/media/common/tuners/tda18271-priv.h | 2 + 2 files changed, 82 insertions(+), 0 deletions(-) diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c index 3347c5b..6e29faf 100644 --- a/drivers/media/common/tuners/tda18271-fe.c +++ b/drivers/media/common/tuners/tda18271-fe.c @@ -928,6 +928,85 @@ fail: /* ------------------------------------------------------------------ */ +static int tda18271_set_state(struct dvb_frontend *fe, + enum tuner_param param, + struct tuner_state *state) +{ + struct tda18271_priv *priv = fe->tuner_priv; + struct tuner_state *req = &priv->request; + struct tda18271_std_map *std_map = &priv->std; + struct tda18271_std_map_item *map; + int ret; + + BUG_ON(!priv); + if (param & DVBFE_TUNER_DELSYS) + req->delsys = state->delsys; + if (param & DVBFE_TUNER_FREQUENCY) + req->frequency = state->frequency; + if (param & DVBFE_TUNER_BANDWIDTH) + req->bandwidth = state->bandwidth; + + priv->mode = TDA18271_DIGITAL; + + switch (req->delsys) { + case SYS_ATSC: + map = &std_map->atsc_6; + req->bandwidth = 6000000; + break; + case SYS_DVBC_ANNEX_B: + map = &std_map->qam_6; + req->bandwidth = 6000000; + break; + case SYS_DVBT: + case SYS_DVBT2: + switch (req->bandwidth) { + case 6000000: + map = &std_map->dvbt_6; + break; + case 7000000: + map = &std_map->dvbt_7; + break; + case 8000000: + map = &std_map->dvbt_8; + break; + default: + ret = -EINVAL; + goto fail; + } + break; + case SYS_DVBC_ANNEX_AC: + map = &std_map->qam_8; + req->bandwidth = 8000000; + break; + default: + tda_warn("Invalid delivery system!\n"); + ret = -EINVAL; + goto fail; + } + tda_dbg("Trying to tune .. delsys=%d modulation=%d frequency=%d bandwidth=%d", + req->delsys, + req->modulation, + req->frequency, + req->bandwidth); + + /* When tuning digital, the analog demod must be tri-stated */ + if (fe->ops.analog_ops.standby) + fe->ops.analog_ops.standby(fe); + + ret = tda18271_tune(fe, map, req->frequency, req->bandwidth); + + if (tda_fail(ret)) + goto fail; + + priv->if_freq = map->if_freq; + priv->frequency = req->frequency; + priv->bandwidth = (req->delsys == SYS_DVBT || req->delsys == SYS_DVBT2) ? + req->bandwidth : 0; +fail: + return ret; +} + + static int tda18271_set_params(struct dvb_frontend *fe, struct dvb_frontend_parameters *params) { @@ -1249,6 +1328,7 @@ static const struct dvb_tuner_ops tda18271_tuner_ops = { .init = tda18271_init, .sleep = tda18271_sleep, .set_params = tda18271_set_params, + .set_state = tda18271_set_state, .set_analog_params = tda18271_set_analog_params, .release = tda18271_release, .set_config = tda18271_set_config, diff --git a/drivers/media/common/tuners/tda18271-priv.h b/drivers/media/common/tuners/tda18271-priv.h index 454c152..bd1bf58 100644 --- a/drivers/media/common/tuners/tda18271-priv.h +++ b/drivers/media/common/tuners/tda18271-priv.h @@ -126,6 +126,8 @@ struct tda18271_priv { u32 frequency; u32 bandwidth; + + struct tuner_state request; }; /*---------------------------------------------------------------------*/ -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-21 21:06 PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS Manu Abraham @ 2011-11-21 21:21 ` Michael Krufky 2011-11-21 21:28 ` Manu Abraham 2011-11-22 0:04 ` Andreas Oberritter 1 sibling, 1 reply; 7+ messages in thread From: Michael Krufky @ 2011-11-21 21:21 UTC (permalink / raw) To: Manu Abraham Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Andreas Oberritter Thank you, Manu... After the Linux Kernel Summit in Prague, I had intentions of solving this exact problem, but you did it first -- good job! I have reviewed the patch to the tda18271 driver, and the changes make good sense to me. I have one question, however: Perhaps my eyes have overlooked something -- I fail to see any code that defines the new "set_state" callback or any code that calls this new callback within dvb-core (assuming dvb_frontend.c) I also can't find the structure declaration of the "tuner_state" struct... ... is this patch missing from your series, or did I just overlook it? That missing patch is what interests me most. Once I can see that missing code, I'd like to begin discussion on whether we actually need the additional callback, or if it can simply be handled by the set_params call. Likewise, I'm not exactly sure why we need this affional "struct tuner_state" ... Perhaps the answer will be self-explanatory once I see the code - maybe no discussion is necessary :-P But this does look good to me so far. I'd be happy to provide my "reviewed-by" tag once I can see the missing code mentioned above. Best regards, Michael Krufky On Mon, Nov 21, 2011 at 4:06 PM, Manu Abraham <abraham.manu@gmail.com> wrote: > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-21 21:21 ` Michael Krufky @ 2011-11-21 21:28 ` Manu Abraham 2011-11-21 21:42 ` Michael Krufky 0 siblings, 1 reply; 7+ messages in thread From: Manu Abraham @ 2011-11-21 21:28 UTC (permalink / raw) To: Michael Krufky Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Andreas Oberritter On 11/22/11, Michael Krufky <mkrufky@linuxtv.org> wrote: > Thank you, Manu... After the Linux Kernel Summit in Prague, I had > intentions of solving this exact problem, but you did it first -- good > job! > > I have reviewed the patch to the tda18271 driver, and the changes make > good sense to me. I have one question, however: > > Perhaps my eyes have overlooked something -- I fail to see any code > that defines the new "set_state" callback or any code that calls this > new callback within dvb-core (assuming dvb_frontend.c) I also can't > find the structure declaration of the "tuner_state" struct... ... is > this patch missing from your series, or did I just overlook it? I guess more like that. The data structure existed for quite a long while in dvb_frontend.h and hence you don't find any new changes. Only delivery and modulation added to it. > > That missing patch is what interests me most. Once I can see that > missing code, I'd like to begin discussion on whether we actually need > the additional callback, or if it can simply be handled by the > set_params call. Likewise, I'm not exactly sure why we need this > affional "struct tuner_state" ... Perhaps the answer will be > self-explanatory once I see the code - maybe no discussion is > necessary :-P > > But this does look good to me so far. I'd be happy to provide my > "reviewed-by" tag once I can see the missing code mentioned above. The callback is used from within a demodulator context as usual and hence. eg: /* program tuner */ - if (fe->ops.tuner_ops.set_params) - fe->ops.tuner_ops.set_params(fe, params); + tstate.delsys = SYS_DVBC_ANNEX_AC; + tstate.frequency = c->frequency; + + if (fe->ops.tuner_ops.set_state) { + fe->ops.tuner_ops.set_state(fe, + DVBFE_TUNER_DELSYS | + DVBFE_TUNER_FREQUENCY, + &tstate); + } else { + if (fe->ops.tuner_ops.set_params) + fe->ops.tuner_ops.set_params(fe, params); + } Best Regards, Manu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-21 21:28 ` Manu Abraham @ 2011-11-21 21:42 ` Michael Krufky 0 siblings, 0 replies; 7+ messages in thread From: Michael Krufky @ 2011-11-21 21:42 UTC (permalink / raw) To: Manu Abraham Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Andreas Oberritter On Mon, Nov 21, 2011 at 4:28 PM, Manu Abraham <abraham.manu@gmail.com> wrote: > On 11/22/11, Michael Krufky <mkrufky@linuxtv.org> wrote: >> Thank you, Manu... After the Linux Kernel Summit in Prague, I had >> intentions of solving this exact problem, but you did it first -- good >> job! >> >> I have reviewed the patch to the tda18271 driver, and the changes make >> good sense to me. I have one question, however: >> >> Perhaps my eyes have overlooked something -- I fail to see any code >> that defines the new "set_state" callback or any code that calls this >> new callback within dvb-core (assuming dvb_frontend.c) I also can't >> find the structure declaration of the "tuner_state" struct... ... is >> this patch missing from your series, or did I just overlook it? > > I guess more like that. The data structure existed for quite a long > while in dvb_frontend.h and hence you don't find any new changes. Only > delivery and modulation added to it. > >> >> That missing patch is what interests me most. Once I can see that >> missing code, I'd like to begin discussion on whether we actually need >> the additional callback, or if it can simply be handled by the >> set_params call. Likewise, I'm not exactly sure why we need this >> affional "struct tuner_state" ... Perhaps the answer will be >> self-explanatory once I see the code - maybe no discussion is >> necessary :-P >> >> But this does look good to me so far. I'd be happy to provide my >> "reviewed-by" tag once I can see the missing code mentioned above. > > The callback is used from within a demodulator context as usual and hence. > eg: > > /* program tuner */ > - if (fe->ops.tuner_ops.set_params) > - fe->ops.tuner_ops.set_params(fe, params); > + tstate.delsys = SYS_DVBC_ANNEX_AC; > + tstate.frequency = c->frequency; > + > + if (fe->ops.tuner_ops.set_state) { > + fe->ops.tuner_ops.set_state(fe, > + DVBFE_TUNER_DELSYS | > + DVBFE_TUNER_FREQUENCY, > + &tstate); > + } else { > + if (fe->ops.tuner_ops.set_params) > + fe->ops.tuner_ops.set_params(fe, params); > + } > > > Best Regards, > Manu > Manu, Thank you for explaining -- I found that structure in dvb_frontend.h, now that you've pointed that out. I am on board with this change -- it is a positive move in the right direction. I believe that after this is merged, we may be able to obsolete and remove the set_params callback. In fact, we can even obsolete the set_analog_params callback as well, using set_state as the single entry point for setting the tuner. Of course, one step at a time -- this is great for now. We should consider the other optimizations after this has been merged and tested. :-) Reviewed-by: Michael Krufky <mkrufky@linuxtv.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-21 21:06 PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS Manu Abraham 2011-11-21 21:21 ` Michael Krufky @ 2011-11-22 0:04 ` Andreas Oberritter 2011-11-22 0:27 ` Manu Abraham 2011-11-24 23:40 ` Mauro Carvalho Chehab 1 sibling, 2 replies; 7+ messages in thread From: Andreas Oberritter @ 2011-11-22 0:04 UTC (permalink / raw) To: Manu Abraham Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Michael Krufky On 21.11.2011 22:06, Manu Abraham wrote: > > 0004-TDA18271-Allow-frontend-to-set-DELSYS-rather-than-qu.patch > > > From 2ece38602678ae323450d0e35379147e6e086326 Mon Sep 17 00:00:00 2001 > From: Manu Abraham <abraham.manu@gmail.com> > Date: Sat, 19 Nov 2011 19:50:09 +0530 > Subject: [PATCH 04/13] TDA18271: Allow frontend to set DELSYS, rather than querying fe->ops.info.type > > With any tuner that can tune to multiple delivery systems/standards, it does > query fe->ops.info.type to determine frontend type and set the delivery > system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK, > FE_QAM, FE_OFDM and FE_ATSC. > > The change allows the tuner to be set to any delivery system specified in > fe_delivery_system_t, thereby simplifying a lot of issues. > > Signed-off-by: Manu Abraham <abraham.manu@gmail.com> > --- > drivers/media/common/tuners/tda18271-fe.c | 80 +++++++++++++++++++++++++++ > drivers/media/common/tuners/tda18271-priv.h | 2 + > 2 files changed, 82 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c > index 3347c5b..6e29faf 100644 > --- a/drivers/media/common/tuners/tda18271-fe.c > +++ b/drivers/media/common/tuners/tda18271-fe.c > @@ -928,6 +928,85 @@ fail: > > /* ------------------------------------------------------------------ */ > > +static int tda18271_set_state(struct dvb_frontend *fe, > + enum tuner_param param, > + struct tuner_state *state) > +{ > + struct tda18271_priv *priv = fe->tuner_priv; > + struct tuner_state *req = &priv->request; > + struct tda18271_std_map *std_map = &priv->std; > + struct tda18271_std_map_item *map; > + int ret; > + > + BUG_ON(!priv); At this point priv has already been dereferenced. > + if (param & DVBFE_TUNER_DELSYS) > + req->delsys = state->delsys; > + if (param & DVBFE_TUNER_FREQUENCY) > + req->frequency = state->frequency; > + if (param & DVBFE_TUNER_BANDWIDTH) > + req->bandwidth = state->bandwidth; What happens if one of these flags is not set, when the function is called for the first time? priv->request doesn't seem to get initialized. Regards, Andreas > + > + priv->mode = TDA18271_DIGITAL; > + > + switch (req->delsys) { > + case SYS_ATSC: > + map = &std_map->atsc_6; > + req->bandwidth = 6000000; > + break; > + case SYS_DVBC_ANNEX_B: > + map = &std_map->qam_6; > + req->bandwidth = 6000000; > + break; > + case SYS_DVBT: > + case SYS_DVBT2: > + switch (req->bandwidth) { > + case 6000000: > + map = &std_map->dvbt_6; > + break; > + case 7000000: > + map = &std_map->dvbt_7; > + break; > + case 8000000: > + map = &std_map->dvbt_8; > + break; > + default: > + ret = -EINVAL; > + goto fail; > + } > + break; > + case SYS_DVBC_ANNEX_AC: > + map = &std_map->qam_8; > + req->bandwidth = 8000000; > + break; > + default: > + tda_warn("Invalid delivery system!\n"); > + ret = -EINVAL; > + goto fail; > + } > + tda_dbg("Trying to tune .. delsys=%d modulation=%d frequency=%d bandwidth=%d", > + req->delsys, > + req->modulation, > + req->frequency, > + req->bandwidth); > + > + /* When tuning digital, the analog demod must be tri-stated */ > + if (fe->ops.analog_ops.standby) > + fe->ops.analog_ops.standby(fe); > + > + ret = tda18271_tune(fe, map, req->frequency, req->bandwidth); > + > + if (tda_fail(ret)) > + goto fail; > + > + priv->if_freq = map->if_freq; > + priv->frequency = req->frequency; > + priv->bandwidth = (req->delsys == SYS_DVBT || req->delsys == SYS_DVBT2) ? > + req->bandwidth : 0; > +fail: > + return ret; > +} > + > + > static int tda18271_set_params(struct dvb_frontend *fe, > struct dvb_frontend_parameters *params) > { > @@ -1249,6 +1328,7 @@ static const struct dvb_tuner_ops tda18271_tuner_ops = { > .init = tda18271_init, > .sleep = tda18271_sleep, > .set_params = tda18271_set_params, > + .set_state = tda18271_set_state, > .set_analog_params = tda18271_set_analog_params, > .release = tda18271_release, > .set_config = tda18271_set_config, > diff --git a/drivers/media/common/tuners/tda18271-priv.h b/drivers/media/common/tuners/tda18271-priv.h > index 454c152..bd1bf58 100644 > --- a/drivers/media/common/tuners/tda18271-priv.h > +++ b/drivers/media/common/tuners/tda18271-priv.h > @@ -126,6 +126,8 @@ struct tda18271_priv { > > u32 frequency; > u32 bandwidth; > + > + struct tuner_state request; > }; > > /*---------------------------------------------------------------------*/ > -- 1.7.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-22 0:04 ` Andreas Oberritter @ 2011-11-22 0:27 ` Manu Abraham 2011-11-24 23:40 ` Mauro Carvalho Chehab 1 sibling, 0 replies; 7+ messages in thread From: Manu Abraham @ 2011-11-22 0:27 UTC (permalink / raw) To: Andreas Oberritter Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Michael Krufky On Tue, Nov 22, 2011 at 5:34 AM, Andreas Oberritter <obi@linuxtv.org> wrote: > On 21.11.2011 22:06, Manu Abraham wrote: >> >> 0004-TDA18271-Allow-frontend-to-set-DELSYS-rather-than-qu.patch >> >> >> From 2ece38602678ae323450d0e35379147e6e086326 Mon Sep 17 00:00:00 2001 >> From: Manu Abraham <abraham.manu@gmail.com> >> Date: Sat, 19 Nov 2011 19:50:09 +0530 >> Subject: [PATCH 04/13] TDA18271: Allow frontend to set DELSYS, rather than querying fe->ops.info.type >> >> With any tuner that can tune to multiple delivery systems/standards, it does >> query fe->ops.info.type to determine frontend type and set the delivery >> system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK, >> FE_QAM, FE_OFDM and FE_ATSC. >> >> The change allows the tuner to be set to any delivery system specified in >> fe_delivery_system_t, thereby simplifying a lot of issues. >> >> Signed-off-by: Manu Abraham <abraham.manu@gmail.com> >> --- >> drivers/media/common/tuners/tda18271-fe.c | 80 +++++++++++++++++++++++++++ >> drivers/media/common/tuners/tda18271-priv.h | 2 + >> 2 files changed, 82 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c >> index 3347c5b..6e29faf 100644 >> --- a/drivers/media/common/tuners/tda18271-fe.c >> +++ b/drivers/media/common/tuners/tda18271-fe.c >> @@ -928,6 +928,85 @@ fail: >> >> /* ------------------------------------------------------------------ */ >> >> +static int tda18271_set_state(struct dvb_frontend *fe, >> + enum tuner_param param, >> + struct tuner_state *state) >> +{ >> + struct tda18271_priv *priv = fe->tuner_priv; >> + struct tuner_state *req = &priv->request; >> + struct tda18271_std_map *std_map = &priv->std; >> + struct tda18271_std_map_item *map; >> + int ret; >> + >> + BUG_ON(!priv); > > At this point priv has already been dereferenced. True, that check is useless. > >> + if (param & DVBFE_TUNER_DELSYS) >> + req->delsys = state->delsys; >> + if (param & DVBFE_TUNER_FREQUENCY) >> + req->frequency = state->frequency; >> + if (param & DVBFE_TUNER_BANDWIDTH) >> + req->bandwidth = state->bandwidth; > > What happens if one of these flags is not set, when the function is > called for the first time? priv->request doesn't seem to get initialized. Request needs to be evaluated, whether it is a valid request for the tuner operation. Need to fix. Thanks, Manu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS 2011-11-22 0:04 ` Andreas Oberritter 2011-11-22 0:27 ` Manu Abraham @ 2011-11-24 23:40 ` Mauro Carvalho Chehab 1 sibling, 0 replies; 7+ messages in thread From: Mauro Carvalho Chehab @ 2011-11-24 23:40 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Manu Abraham, Linux Media Mailing List, Michael Krufky Em 21-11-2011 22:04, Andreas Oberritter escreveu: > On 21.11.2011 22:06, Manu Abraham wrote: >> >> 0004-TDA18271-Allow-frontend-to-set-DELSYS-rather-than-qu.patch >> >> >> From 2ece38602678ae323450d0e35379147e6e086326 Mon Sep 17 00:00:00 2001 >> From: Manu Abraham <abraham.manu@gmail.com> >> Date: Sat, 19 Nov 2011 19:50:09 +0530 >> Subject: [PATCH 04/13] TDA18271: Allow frontend to set DELSYS, rather than querying fe->ops.info.type >> >> With any tuner that can tune to multiple delivery systems/standards, it does >> query fe->ops.info.type to determine frontend type and set the delivery >> system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK, >> FE_QAM, FE_OFDM and FE_ATSC. >> >> The change allows the tuner to be set to any delivery system specified in >> fe_delivery_system_t, thereby simplifying a lot of issues. >> >> Signed-off-by: Manu Abraham <abraham.manu@gmail.com> >> --- >> drivers/media/common/tuners/tda18271-fe.c | 80 +++++++++++++++++++++++++++ >> drivers/media/common/tuners/tda18271-priv.h | 2 + >> 2 files changed, 82 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c >> index 3347c5b..6e29faf 100644 >> --- a/drivers/media/common/tuners/tda18271-fe.c >> +++ b/drivers/media/common/tuners/tda18271-fe.c >> @@ -928,6 +928,85 @@ fail: >> >> /* ------------------------------------------------------------------ */ >> >> +static int tda18271_set_state(struct dvb_frontend *fe, >> + enum tuner_param param, >> + struct tuner_state *state) >> +{ >> + struct tda18271_priv *priv = fe->tuner_priv; >> + struct tuner_state *req = &priv->request; >> + struct tda18271_std_map *std_map = &priv->std; >> + struct tda18271_std_map_item *map; >> + int ret; >> + >> + BUG_ON(!priv); > > At this point priv has already been dereferenced. > >> + if (param & DVBFE_TUNER_DELSYS) >> + req->delsys = state->delsys; >> + if (param & DVBFE_TUNER_FREQUENCY) >> + req->frequency = state->frequency; >> + if (param & DVBFE_TUNER_BANDWIDTH) >> + req->bandwidth = state->bandwidth; > > What happens if one of these flags is not set, when the function is > called for the first time? priv->request doesn't seem to get initialized. > > Regards, > Andreas > >> + >> + priv->mode = TDA18271_DIGITAL; >> + >> + switch (req->delsys) { >> + case SYS_ATSC: >> + map = &std_map->atsc_6; >> + req->bandwidth = 6000000; >> + break; >> + case SYS_DVBC_ANNEX_B: >> + map = &std_map->qam_6; >> + req->bandwidth = 6000000; >> + break; >> + case SYS_DVBT: >> + case SYS_DVBT2: >> + switch (req->bandwidth) { >> + case 6000000: >> + map = &std_map->dvbt_6; >> + break; >> + case 7000000: >> + map = &std_map->dvbt_7; >> + break; >> + case 8000000: >> + map = &std_map->dvbt_8; >> + break; >> + default: >> + ret = -EINVAL; >> + goto fail; >> + } >> + break; >> + case SYS_DVBC_ANNEX_AC: >> + map = &std_map->qam_8; >> + req->bandwidth = 8000000; >> + break; This premise is not correct, and causes tuning failure on places where channels are spaced with 6MHz. The bandwidth should be calculated as a function of the rolloff and symbol rate. I had to fix it on a few places, for devices to work with Net Digital Cable operator in Brazil (6MHz spaced channels, 5.217 KBauds per carrier, DVB-C Annex A). The correct way for doing it is: else if (fe->ops.info.type == FE_QAM) { /* * Using a higher bandwidth at the tuner filter may * allow inter-carrier interference. * So, determine the minimal channel spacing, in order * to better adjust the tuner filter. * According with ITU-T J.83, the bandwidth is given by: * bw = Simbol Rate * (1 + roll_off), where the roll_off * is equal to 0.15 for Annex A, and 0.13 for annex C */ if (fe->dtv_property_cache.rolloff == ROLLOFF_13) bw = (params->u.qam.symbol_rate * 13) / 10; else bw = (params->u.qam.symbol_rate * 15) / 10; if (bw <= 6000000) Standard = HF_DVBC_6MHZ; else if (bw <= 7000000) Standard = HF_DVBC_7MHZ; else Standard = HF_DVBC_8MHZ; (from drivers/media/dvb/frontends/tda18271c2dd.c) Where ROLLOFF_13 is used on Annex C, and ROLLOF_15 on Annex A. The same fix should be applied to all DVB-C capable tuners. Alternatively, we should put some ancillary function at the core, and let the core estimate the needed bandwidth for DVB-C. PS.: While I didn't test, I suspect that places using 7MHz-spaced channels will also increase the reception, as less inter-channel noise will be sent to the demod. >> + default: >> + tda_warn("Invalid delivery system!\n"); >> + ret = -EINVAL; >> + goto fail; >> + } >> + tda_dbg("Trying to tune .. delsys=%d modulation=%d frequency=%d bandwidth=%d", >> + req->delsys, >> + req->modulation, >> + req->frequency, >> + req->bandwidth); >> + >> + /* When tuning digital, the analog demod must be tri-stated */ >> + if (fe->ops.analog_ops.standby) >> + fe->ops.analog_ops.standby(fe); >> + >> + ret = tda18271_tune(fe, map, req->frequency, req->bandwidth); >> + >> + if (tda_fail(ret)) >> + goto fail; >> + >> + priv->if_freq = map->if_freq; >> + priv->frequency = req->frequency; >> + priv->bandwidth = (req->delsys == SYS_DVBT || req->delsys == SYS_DVBT2) ? >> + req->bandwidth : 0; >> +fail: >> + return ret; >> +} >> + >> + >> static int tda18271_set_params(struct dvb_frontend *fe, >> struct dvb_frontend_parameters *params) >> { >> @@ -1249,6 +1328,7 @@ static const struct dvb_tuner_ops tda18271_tuner_ops = { >> .init = tda18271_init, >> .sleep = tda18271_sleep, >> .set_params = tda18271_set_params, >> + .set_state = tda18271_set_state, >> .set_analog_params = tda18271_set_analog_params, >> .release = tda18271_release, >> .set_config = tda18271_set_config, >> diff --git a/drivers/media/common/tuners/tda18271-priv.h b/drivers/media/common/tuners/tda18271-priv.h >> index 454c152..bd1bf58 100644 >> --- a/drivers/media/common/tuners/tda18271-priv.h >> +++ b/drivers/media/common/tuners/tda18271-priv.h >> @@ -126,6 +126,8 @@ struct tda18271_priv { >> >> u32 frequency; >> u32 bandwidth; >> + >> + struct tuner_state request; >> }; >> >> /*---------------------------------------------------------------------*/ >> -- 1.7.1 >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-11-25 0:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-21 21:06 PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS Manu Abraham 2011-11-21 21:21 ` Michael Krufky 2011-11-21 21:28 ` Manu Abraham 2011-11-21 21:42 ` Michael Krufky 2011-11-22 0:04 ` Andreas Oberritter 2011-11-22 0:27 ` Manu Abraham 2011-11-24 23:40 ` Mauro Carvalho Chehab
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.