| Message ID | CAA9z4LY6cWEm+4ed7HM3ga0dohsg6LJ6Z4XSge9i4FguJR=FJw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Chris, Em Fri, 19 Jul 2013 14:27:09 -0600 Chris Lee <updatelee@gmail.com> escreveu: > In frontend.h we have a struct called dvb_frontend_ops, in there we > have an element called delsys to show the delivery systems supported > by the tuner, Id like to propose we add onto that with delmod and > delfec. > > Its not a perfect solution as sometimes a specific modulation or fec > is only availible on specific systems. But its better then what we > have now. The struct fe_caps isnt really suited for this, its missing > many systems, modulations, and fec's. Its just not expandable enough > to get all the supported sys/mod/fec a tuner supports in there. > > Expanding this would allow user land applications to poll the tuner to > determine more detailed information on the tuners capabilities. > > Here is the patch I propose, along with the au8522 driver modified to > utilize the new elements. Id like to hear comments on it. Does anyone > see a better way of doing this ? We had a discussion some time ago about it. Basically, a device that it is said to support, let's say, DVB-T, should support all possible modulations and FECs that are part of the system. So, in thesis, there shouldn't be any need to add a list of modulations and FECs. Also, frontends that support multiple delivery systems would need to enumerate the modulations and FECs after the selection of a given delivery system (as, typically, they only support a subset of them for each delsys). Ok, practice is different, as there are reverse-engineered drivers that may not support everything that the hardware supports. Also, a few hardware may have additional restrictions. Yet, on those cases, the userspace may detect if a given modulation type or FEC is supported, by trying to set it and check if the operation didn't fail, and if the cache got properly updated. So, at the end, it was decided to not add anything like that. Yet, it is good to see other opinions. It should be said that one of the hard parts of an approach like that, is that someone would need to dig into each driver and add the proper support for per-delsys modulation and FECs. Alternatively, the core could initialize it to the default value for each standard, and call some driver-specific function that would reset the modulation/FECs that aren't supported by some specific drivers. Regards, Mauro > > Chris Lee <updatelee@gmail.com> > > diff --git a/drivers/media/dvb-core/dvb_frontend.c > b/drivers/media/dvb-core/dvb_frontend.c > index 1f925e8..f5df08e 100644 > --- a/drivers/media/dvb-core/dvb_frontend.c > +++ b/drivers/media/dvb-core/dvb_frontend.c > @@ -1036,6 +1036,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = { > _DTV_CMD(DTV_API_VERSION, 0, 0), > > _DTV_CMD(DTV_ENUM_DELSYS, 0, 0), > + _DTV_CMD(DTV_ENUM_DELMOD, 0, 0), > + _DTV_CMD(DTV_ENUM_DELFEC, 0, 0), > > _DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0), > _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0), > @@ -1285,6 +1287,22 @@ static int dtv_property_process_get(struct > dvb_frontend *fe, > } > tvp->u.buffer.len = ncaps; > break; > + case DTV_ENUM_DELMOD: > + ncaps = 0; > + while (fe->ops.delmod[ncaps] && ncaps < MAX_DELMOD) { > + tvp->u.buffer.data[ncaps] = fe->ops.delmod[ncaps]; > + ncaps++; > + } > + tvp->u.buffer.len = ncaps; > + break; > + case DTV_ENUM_DELFEC: > + ncaps = 0; > + while (fe->ops.delfec[ncaps] && ncaps < MAX_DELFEC) { > + tvp->u.buffer.data[ncaps] = fe->ops.delfec[ncaps]; > + ncaps++; > + } > + tvp->u.buffer.len = ncaps; > + break; > case DTV_FREQUENCY: > tvp->u.data = c->frequency; > break; > diff --git a/drivers/media/dvb-core/dvb_frontend.h > b/drivers/media/dvb-core/dvb_frontend.h > index 371b6ca..4e96640 100644 > --- a/drivers/media/dvb-core/dvb_frontend.h > +++ b/drivers/media/dvb-core/dvb_frontend.h > @@ -47,6 +47,8 @@ > * should be smaller or equal to 32 > */ > #define MAX_DELSYS 8 > +#define MAX_DELMOD 8 > +#define MAX_DELFEC 32 > > struct dvb_frontend_tune_settings { > int min_delay_ms; > @@ -263,6 +265,8 @@ struct dvb_frontend_ops { > struct dvb_frontend_info info; > > u8 delsys[MAX_DELSYS]; > + u8 delmod[MAX_DELMOD]; > + u8 delfec[MAX_DELFEC]; > > void (*release)(struct dvb_frontend* fe); > void (*release_sec)(struct dvb_frontend* fe); > diff --git a/include/uapi/linux/dvb/frontend.h > b/include/uapi/linux/dvb/frontend.h > index c56d77c..be63d37 100644 > --- a/include/uapi/linux/dvb/frontend.h > +++ b/include/uapi/linux/dvb/frontend.h > @@ -375,7 +375,10 @@ struct dvb_frontend_event { > #define DTV_STAT_ERROR_BLOCK_COUNT 68 > #define DTV_STAT_TOTAL_BLOCK_COUNT 69 > > -#define DTV_MAX_COMMAND DTV_STAT_TOTAL_BLOCK_COUNT > +#define DTV_ENUM_DELMOD 70 > +#define DTV_ENUM_DELFEC 71 > + > +#define DTV_MAX_COMMAND DTV_ENUM_DELFEC > > typedef enum fe_pilot { > PILOT_ON, > diff --git a/drivers/media/dvb-frontends/au8522_dig.c > b/drivers/media/dvb-frontends/au8522_dig.c > index 6ee9028..1044c9d 100644 > --- a/drivers/media/dvb-frontends/au8522_dig.c > +++ b/drivers/media/dvb-frontends/au8522_dig.c > @@ -822,7 +822,9 @@ error: > EXPORT_SYMBOL(au8522_attach); > > static struct dvb_frontend_ops au8522_ops = { > - .delsys = { SYS_ATSC, SYS_DVBC_ANNEX_B }, > + .delsys = { SYS_DVBC_ANNEX_B, SYS_ATSC }, > + .delmod = { QAM_256, QAM_64, VSB_8 }, > + .delfec = { FEC_NONE }, > .info = { > .name = "Auvitek AU8522 QAM/8VSB Frontend", > .frequency_min = 54000000, > -- > 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
By using DTV_SET_PROPERTY I can run though a list of possible systems to determine what is supported and what isnt. I havent looked too far but I think it uses delsys to determine this information. Which I can already get from DTV_ENUM_DELSYS. This functionality could be expanded to delmod and delfec. But there is no way to pol modulations or fec using DTV_SET_PROPERTY. I understand there is FE_CAN_*** for modulations and fecs but its far from complete and not very expandable. If we only went on FE_CAN_ for fec we'd be missing about half the supported fec's. Ultimately Id like a solution that would have modulations per system, and fec per modulation as they are quite often different. The delsys/delmod/delfec is a simpler cleaner interface that is adding new functionality and wouldnt be required for drivers or userland to implement if they dont want to as the patch stands (if we implemented DTV_SET_PROPERTY checks against delmod/delfec then it would be required in drivers) So Id love to hear more comments on this subject, it would really be nice to clean some of the inadequacies up, imo userland should have the ability to pol the driver for supported systems/modulation/fec vs just trying everything and seeing what works and what doesnt. Thanks, Chris On Mon, Jul 22, 2013 at 5:21 AM, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > Hi Chris, > > Em Fri, 19 Jul 2013 14:27:09 -0600 > Chris Lee <updatelee@gmail.com> escreveu: > >> In frontend.h we have a struct called dvb_frontend_ops, in there we >> have an element called delsys to show the delivery systems supported >> by the tuner, Id like to propose we add onto that with delmod and >> delfec. >> >> Its not a perfect solution as sometimes a specific modulation or fec >> is only availible on specific systems. But its better then what we >> have now. The struct fe_caps isnt really suited for this, its missing >> many systems, modulations, and fec's. Its just not expandable enough >> to get all the supported sys/mod/fec a tuner supports in there. >> >> Expanding this would allow user land applications to poll the tuner to >> determine more detailed information on the tuners capabilities. >> >> Here is the patch I propose, along with the au8522 driver modified to >> utilize the new elements. Id like to hear comments on it. Does anyone >> see a better way of doing this ? > > We had a discussion some time ago about it. Basically, a device that > it is said to support, let's say, DVB-T, should support all possible > modulations and FECs that are part of the system. > > So, in thesis, there shouldn't be any need to add a list of modulations > and FECs. > > Also, frontends that support multiple delivery systems would need > to enumerate the modulations and FECs after the selection of a given > delivery system (as, typically, they only support a subset of them > for each delsys). > > Ok, practice is different, as there are reverse-engineered drivers > that may not support everything that the hardware supports. Also, > a few hardware may have additional restrictions. > > Yet, on those cases, the userspace may detect if a given modulation > type or FEC is supported, by trying to set it and check if the > operation didn't fail, and if the cache got properly updated. > > So, at the end, it was decided to not add anything like that. > > Yet, it is good to see other opinions. > > It should be said that one of the hard parts of an approach like > that, is that someone would need to dig into each driver and add > the proper support for per-delsys modulation and FECs. > > Alternatively, the core could initialize it to the default value > for each standard, and call some driver-specific function that > would reset the modulation/FECs that aren't supported by some > specific drivers. > > Regards, > Mauro > >> >> Chris Lee <updatelee@gmail.com> >> >> diff --git a/drivers/media/dvb-core/dvb_frontend.c >> b/drivers/media/dvb-core/dvb_frontend.c >> index 1f925e8..f5df08e 100644 >> --- a/drivers/media/dvb-core/dvb_frontend.c >> +++ b/drivers/media/dvb-core/dvb_frontend.c >> @@ -1036,6 +1036,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = { >> _DTV_CMD(DTV_API_VERSION, 0, 0), >> >> _DTV_CMD(DTV_ENUM_DELSYS, 0, 0), >> + _DTV_CMD(DTV_ENUM_DELMOD, 0, 0), >> + _DTV_CMD(DTV_ENUM_DELFEC, 0, 0), >> >> _DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0), >> _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0), >> @@ -1285,6 +1287,22 @@ static int dtv_property_process_get(struct >> dvb_frontend *fe, >> } >> tvp->u.buffer.len = ncaps; >> break; >> + case DTV_ENUM_DELMOD: >> + ncaps = 0; >> + while (fe->ops.delmod[ncaps] && ncaps < MAX_DELMOD) { >> + tvp->u.buffer.data[ncaps] = fe->ops.delmod[ncaps]; >> + ncaps++; >> + } >> + tvp->u.buffer.len = ncaps; >> + break; >> + case DTV_ENUM_DELFEC: >> + ncaps = 0; >> + while (fe->ops.delfec[ncaps] && ncaps < MAX_DELFEC) { >> + tvp->u.buffer.data[ncaps] = fe->ops.delfec[ncaps]; >> + ncaps++; >> + } >> + tvp->u.buffer.len = ncaps; >> + break; >> case DTV_FREQUENCY: >> tvp->u.data = c->frequency; >> break; >> diff --git a/drivers/media/dvb-core/dvb_frontend.h >> b/drivers/media/dvb-core/dvb_frontend.h >> index 371b6ca..4e96640 100644 >> --- a/drivers/media/dvb-core/dvb_frontend.h >> +++ b/drivers/media/dvb-core/dvb_frontend.h >> @@ -47,6 +47,8 @@ >> * should be smaller or equal to 32 >> */ >> #define MAX_DELSYS 8 >> +#define MAX_DELMOD 8 >> +#define MAX_DELFEC 32 >> >> struct dvb_frontend_tune_settings { >> int min_delay_ms; >> @@ -263,6 +265,8 @@ struct dvb_frontend_ops { >> struct dvb_frontend_info info; >> >> u8 delsys[MAX_DELSYS]; >> + u8 delmod[MAX_DELMOD]; >> + u8 delfec[MAX_DELFEC]; >> >> void (*release)(struct dvb_frontend* fe); >> void (*release_sec)(struct dvb_frontend* fe); >> diff --git a/include/uapi/linux/dvb/frontend.h >> b/include/uapi/linux/dvb/frontend.h >> index c56d77c..be63d37 100644 >> --- a/include/uapi/linux/dvb/frontend.h >> +++ b/include/uapi/linux/dvb/frontend.h >> @@ -375,7 +375,10 @@ struct dvb_frontend_event { >> #define DTV_STAT_ERROR_BLOCK_COUNT 68 >> #define DTV_STAT_TOTAL_BLOCK_COUNT 69 >> >> -#define DTV_MAX_COMMAND DTV_STAT_TOTAL_BLOCK_COUNT >> +#define DTV_ENUM_DELMOD 70 >> +#define DTV_ENUM_DELFEC 71 >> + >> +#define DTV_MAX_COMMAND DTV_ENUM_DELFEC >> >> typedef enum fe_pilot { >> PILOT_ON, >> diff --git a/drivers/media/dvb-frontends/au8522_dig.c >> b/drivers/media/dvb-frontends/au8522_dig.c >> index 6ee9028..1044c9d 100644 >> --- a/drivers/media/dvb-frontends/au8522_dig.c >> +++ b/drivers/media/dvb-frontends/au8522_dig.c >> @@ -822,7 +822,9 @@ error: >> EXPORT_SYMBOL(au8522_attach); >> >> static struct dvb_frontend_ops au8522_ops = { >> - .delsys = { SYS_ATSC, SYS_DVBC_ANNEX_B }, >> + .delsys = { SYS_DVBC_ANNEX_B, SYS_ATSC }, >> + .delmod = { QAM_256, QAM_64, VSB_8 }, >> + .delfec = { FEC_NONE }, >> .info = { >> .name = "Auvitek AU8522 QAM/8VSB Frontend", >> .frequency_min = 54000000, >> -- >> 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 > > > -- > > Cheers, > Mauro -- 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
Em Mon, 22 Jul 2013 19:28:55 -0600 Chris Lee <updatelee@gmail.com> escreveu: > By using DTV_SET_PROPERTY I can run though a list of possible systems > to determine what is supported and what isnt. I havent looked too far > but I think it uses delsys to determine this information. Which I can > already get from DTV_ENUM_DELSYS. This functionality could be expanded > to delmod and delfec. > > But there is no way to pol modulations or fec using DTV_SET_PROPERTY. > I understand there is FE_CAN_*** for modulations and fecs but its far > from complete and not very expandable. If we only went on FE_CAN_ for > fec we'd be missing about half the supported fec's. > > Ultimately Id like a solution that would have modulations per system, > and fec per modulation as they are quite often different. The > delsys/delmod/delfec is a simpler cleaner interface that is adding new > functionality and wouldnt be required for drivers or userland to > implement if they dont want to as the patch stands (if we implemented > DTV_SET_PROPERTY checks against delmod/delfec then it would be > required in drivers) > > So Id love to hear more comments on this subject, it would really be > nice to clean some of the inadequacies up, imo userland should have > the ability to pol the driver for supported systems/modulation/fec vs > just trying everything and seeing what works and what doesnt. Well, if we're going to properly implement it, then we need to deprecate the .caps field at the dvb structure, replacing it by the new DVBv5 equivalent, adding a DVBv3 backward code at dvb_frontend.c that will use the new DVBv5 "caps" to fill the DVBv3 caps. > > Thanks, > Chris > > > > On Mon, Jul 22, 2013 at 5:21 AM, Mauro Carvalho Chehab > <m.chehab@samsung.com> wrote: > > Hi Chris, > > > > Em Fri, 19 Jul 2013 14:27:09 -0600 > > Chris Lee <updatelee@gmail.com> escreveu: > > > >> In frontend.h we have a struct called dvb_frontend_ops, in there we > >> have an element called delsys to show the delivery systems supported > >> by the tuner, Id like to propose we add onto that with delmod and > >> delfec. > >> > >> Its not a perfect solution as sometimes a specific modulation or fec > >> is only availible on specific systems. But its better then what we > >> have now. The struct fe_caps isnt really suited for this, its missing > >> many systems, modulations, and fec's. Its just not expandable enough > >> to get all the supported sys/mod/fec a tuner supports in there. > >> > >> Expanding this would allow user land applications to poll the tuner to > >> determine more detailed information on the tuners capabilities. > >> > >> Here is the patch I propose, along with the au8522 driver modified to > >> utilize the new elements. Id like to hear comments on it. Does anyone > >> see a better way of doing this ? > > > > We had a discussion some time ago about it. Basically, a device that > > it is said to support, let's say, DVB-T, should support all possible > > modulations and FECs that are part of the system. > > > > So, in thesis, there shouldn't be any need to add a list of modulations > > and FECs. > > > > Also, frontends that support multiple delivery systems would need > > to enumerate the modulations and FECs after the selection of a given > > delivery system (as, typically, they only support a subset of them > > for each delsys). > > > > Ok, practice is different, as there are reverse-engineered drivers > > that may not support everything that the hardware supports. Also, > > a few hardware may have additional restrictions. > > > > Yet, on those cases, the userspace may detect if a given modulation > > type or FEC is supported, by trying to set it and check if the > > operation didn't fail, and if the cache got properly updated. > > > > So, at the end, it was decided to not add anything like that. > > > > Yet, it is good to see other opinions. > > > > It should be said that one of the hard parts of an approach like > > that, is that someone would need to dig into each driver and add > > the proper support for per-delsys modulation and FECs. > > > > Alternatively, the core could initialize it to the default value > > for each standard, and call some driver-specific function that > > would reset the modulation/FECs that aren't supported by some > > specific drivers. > > > > Regards, > > Mauro > > > >> > >> Chris Lee <updatelee@gmail.com> > >> > >> diff --git a/drivers/media/dvb-core/dvb_frontend.c > >> b/drivers/media/dvb-core/dvb_frontend.c > >> index 1f925e8..f5df08e 100644 > >> --- a/drivers/media/dvb-core/dvb_frontend.c > >> +++ b/drivers/media/dvb-core/dvb_frontend.c > >> @@ -1036,6 +1036,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = { > >> _DTV_CMD(DTV_API_VERSION, 0, 0), > >> > >> _DTV_CMD(DTV_ENUM_DELSYS, 0, 0), > >> + _DTV_CMD(DTV_ENUM_DELMOD, 0, 0), > >> + _DTV_CMD(DTV_ENUM_DELFEC, 0, 0), > >> > >> _DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0), > >> _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0), > >> @@ -1285,6 +1287,22 @@ static int dtv_property_process_get(struct > >> dvb_frontend *fe, > >> } > >> tvp->u.buffer.len = ncaps; > >> break; > >> + case DTV_ENUM_DELMOD: > >> + ncaps = 0; > >> + while (fe->ops.delmod[ncaps] && ncaps < MAX_DELMOD) { > >> + tvp->u.buffer.data[ncaps] = fe->ops.delmod[ncaps]; > >> + ncaps++; > >> + } > >> + tvp->u.buffer.len = ncaps; > >> + break; > >> + case DTV_ENUM_DELFEC: > >> + ncaps = 0; > >> + while (fe->ops.delfec[ncaps] && ncaps < MAX_DELFEC) { > >> + tvp->u.buffer.data[ncaps] = fe->ops.delfec[ncaps]; > >> + ncaps++; > >> + } > >> + tvp->u.buffer.len = ncaps; > >> + break; > >> case DTV_FREQUENCY: > >> tvp->u.data = c->frequency; > >> break; > >> diff --git a/drivers/media/dvb-core/dvb_frontend.h > >> b/drivers/media/dvb-core/dvb_frontend.h > >> index 371b6ca..4e96640 100644 > >> --- a/drivers/media/dvb-core/dvb_frontend.h > >> +++ b/drivers/media/dvb-core/dvb_frontend.h > >> @@ -47,6 +47,8 @@ > >> * should be smaller or equal to 32 > >> */ > >> #define MAX_DELSYS 8 > >> +#define MAX_DELMOD 8 > >> +#define MAX_DELFEC 32 > >> > >> struct dvb_frontend_tune_settings { > >> int min_delay_ms; > >> @@ -263,6 +265,8 @@ struct dvb_frontend_ops { > >> struct dvb_frontend_info info; > >> > >> u8 delsys[MAX_DELSYS]; > >> + u8 delmod[MAX_DELMOD]; > >> + u8 delfec[MAX_DELFEC]; > >> > >> void (*release)(struct dvb_frontend* fe); > >> void (*release_sec)(struct dvb_frontend* fe); > >> diff --git a/include/uapi/linux/dvb/frontend.h > >> b/include/uapi/linux/dvb/frontend.h > >> index c56d77c..be63d37 100644 > >> --- a/include/uapi/linux/dvb/frontend.h > >> +++ b/include/uapi/linux/dvb/frontend.h > >> @@ -375,7 +375,10 @@ struct dvb_frontend_event { > >> #define DTV_STAT_ERROR_BLOCK_COUNT 68 > >> #define DTV_STAT_TOTAL_BLOCK_COUNT 69 > >> > >> -#define DTV_MAX_COMMAND DTV_STAT_TOTAL_BLOCK_COUNT > >> +#define DTV_ENUM_DELMOD 70 > >> +#define DTV_ENUM_DELFEC 71 > >> + > >> +#define DTV_MAX_COMMAND DTV_ENUM_DELFEC > >> > >> typedef enum fe_pilot { > >> PILOT_ON, > >> diff --git a/drivers/media/dvb-frontends/au8522_dig.c > >> b/drivers/media/dvb-frontends/au8522_dig.c > >> index 6ee9028..1044c9d 100644 > >> --- a/drivers/media/dvb-frontends/au8522_dig.c > >> +++ b/drivers/media/dvb-frontends/au8522_dig.c > >> @@ -822,7 +822,9 @@ error: > >> EXPORT_SYMBOL(au8522_attach); > >> > >> static struct dvb_frontend_ops au8522_ops = { > >> - .delsys = { SYS_ATSC, SYS_DVBC_ANNEX_B }, > >> + .delsys = { SYS_DVBC_ANNEX_B, SYS_ATSC }, > >> + .delmod = { QAM_256, QAM_64, VSB_8 }, > >> + .delfec = { FEC_NONE }, > >> .info = { > >> .name = "Auvitek AU8522 QAM/8VSB Frontend", > >> .frequency_min = 54000000, > >> -- > >> 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 > > > > > > -- > > > > Cheers, > > Mauro
On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee <updatelee@gmail.com> wrote: > In frontend.h we have a struct called dvb_frontend_ops, in there we > have an element called delsys to show the delivery systems supported > by the tuner, Id like to propose we add onto that with delmod and > delfec. > > Its not a perfect solution as sometimes a specific modulation or fec > is only availible on specific systems. But its better then what we > have now. The struct fe_caps isnt really suited for this, its missing > many systems, modulations, and fec's. Its just not expandable enough > to get all the supported sys/mod/fec a tuner supports in there. Question > Why should an application know all the modulations and FEC's supported by a demodulator ? Aren't demodulators compliant to their respective delivery systems ? Or do you mean to state that, you are trying to work around some demodulator quirks ? Regards, Manu -- 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
Not all tuners support all fec's - genpix devices support an odd 5/11 fec for digicipher, pretty sure no one else does. - stv0899 supports 1/2, 2/3, 3/4, 5/6, 6/7, 7/8 - stv0900 supports 1/2, 3/5, 2/3, 3/4, 4/5, 5/6, 8/9, 9/10 Not all tuners support the entire range of fec's. I think this is more the norm then the exception. If the userland application can poll the driver for a list of supported fec it allows them to have a list of valid tuning options for the user to choose from, vs listing everything and hoping it doesnt fail. As stated Id much rather have a list made up from system -> modulation -> fec. ie genpix SYS_TURBO -> QPSK/8PSK SYS_TURBO.QPSK -> 1/2, 2/3, 3/4, 5/6, 7/8 SYS_TURBO.8PSK -> 2/3, 3/4, 5/6, 8/9 but that could get more complicated to implement pretty quickly Chris Lee On Tue, Jul 23, 2013 at 7:35 AM, Manu Abraham <abraham.manu@gmail.com> wrote: > On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee <updatelee@gmail.com> wrote: >> In frontend.h we have a struct called dvb_frontend_ops, in there we >> have an element called delsys to show the delivery systems supported >> by the tuner, Id like to propose we add onto that with delmod and >> delfec. >> >> Its not a perfect solution as sometimes a specific modulation or fec >> is only availible on specific systems. But its better then what we >> have now. The struct fe_caps isnt really suited for this, its missing >> many systems, modulations, and fec's. Its just not expandable enough >> to get all the supported sys/mod/fec a tuner supports in there. > > Question > Why should an application know all the modulations and > FEC's supported by a demodulator ? > > Aren't demodulators compliant to their respective delivery systems ? > > Or do you mean to state that, you are trying to work around some > demodulator quirks ? > > > Regards, > > Manu -- 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
On Tue, Jul 23, 2013 at 10:17 PM, Chris Lee <updatelee@gmail.com> wrote: > Not all tuners support all fec's Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ outputs to the demodulator. ;-) That said; Demods support all FEC's relevant to their delivery systems. It's just that some devices likely do support some additional states. > - genpix devices support an odd 5/11 fec for digicipher, pretty sure > no one else does. I think DCII FEC5/11 is standard, reading this URL http://rickcaylor.websitetoolbox.com/post/DCII-Valid-SRFECModulation-Combinations-5827500 Also, according to the BCM4201 datasheet: * DVB/DIRECTV/Digicipher II compliant FEC decoder 64 state viterbi decoder supports rates= 5/11, 1/2, 3/5, 2/3, 3/4. 4/5, 5/6, 6/7, 7/8 I would say, it is pretty much standard for DCII. Given that it is pretty much standard, I would say that for DCII; for the genpix all you need is a SYS_DCII and or a SYS_DSS addition to the genpix driver, rather than having a ton of delivery systems mixed with modulations as in your patch with DCII_QPSK, ... _OQPSK etc. Actually, those are a bit too superfluous. You shouldn't mix delivery systems and modulations. That was the whole reason why the delivery system flag was introduced to make things saner and proper for the frontend API. If I am not mistaken, the genpix hardware is a hardware wrapper around the BCM demodulator. So, it is quite likely that even if you don't set any FEC parameter, the device could still acquire lock as expected. I am not holding my breath on this. Maybe someone with a genpix device can prove me right or wrong. > - stv0899 supports 1/2, 2/3, 3/4, 5/6, 6/7, 7/8 > - stv0900 supports 1/2, 3/5, 2/3, 3/4, 4/5, 5/6, 8/9, 9/10 Ah.... Though these devices support additional modes, the STB0899 (I don't know whether you meant the STB0899 with stv0899, yet looking at the stb0899, since there doesn't seem to be other references) With the STB0899 driver, all you need to tune with it is Frequency, Symbol Rate and Delivery system With the STV090x driver all you need is Frequency and Symbol Rate. (It will auto detect delivery system) > > Not all tuners support the entire range of fec's. I think this is more > the norm then the exception. > I find it slightly hard to believe... ;-) > If the userland application can poll the driver for a list of > supported fec it allows them to have a list of valid tuning options > for the user to choose from, vs listing everything and hoping it > doesnt fail. When a driver is not accepting those parameters as inputs, why should the application/user burden himself with knowing parameters of no relevance to him ? > > As stated Id much rather have a list made up from system -> modulation -> fec. > > ie genpix > > SYS_TURBO -> QPSK/8PSK > SYS_TURBO.QPSK -> 1/2, 2/3, 3/4, 5/6, 7/8 > SYS_TURBO.8PSK -> 2/3, 3/4, 5/6, 8/9 > > but that could get more complicated to implement pretty quickly Actually with all those redundant FEC bits gone away from relevance, things are a bit more saner. Regards, Manu -- 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
> Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ > outputs to the demodulator. ;-) ya ya :) you knew what I meant, not what I said hehe > Demods support all FEC's relevant to their delivery systems. It's just that > some devices likely do support some additional states. This part I dont understand, what do you mean additional states ? and how would a userland application determine if a demod supports these additional states? > I think DCII FEC5/11 is standard, reading this URL > http://rickcaylor.websitetoolbox.com/post/DCII-Valid-SRFECModulation-Combinations-5827500 > I would say, it is pretty much standard for DCII. yes 5/11 is standard for DCII, but nothing else. > Given that it is pretty much standard, I would say that for DCII; for > the genpix > all you need is a SYS_DCII and or a SYS_DSS addition to the genpix driver, > rather than having a ton of delivery systems mixed with modulations as in > your patch with DCII_QPSK, ... _OQPSK etc. Actually, those are a bit too > superfluous. You shouldn't mix delivery systems and modulations. That was > the whole reason why the delivery system flag was introduced to make > things saner and proper for the frontend API. Yup fair enough, easy to change, I'll get on that and resubmit the patch. > If I am not mistaken, the genpix hardware is a hardware wrapper around the > BCM demodulator. So, it is quite likely that even if you don't set any FEC > parameter, the device could still acquire lock as expected. I am not holding > my breath on this. Maybe someone with a genpix device can prove me right > or wrong. FEC_AUTO works for all but turbo-qpsk on genpix devices. I still think its important to have all the fec supported in the driver though even if FEC_AUTO did work 100% else why even have it as an option at all. > With the STB0899 driver, all you need to tune with it is Frequency, > Symbol Rate and Delivery system > > > With the STV090x driver all you need is Frequency and Symbol Rate. > (It will auto detect delivery system) Same thing, I still think if we allow the user to send a fec value we should make sure its right, else why not just hard code all the drivers to fec-auto that support it and remove the option all together. I dont like that option. > When a driver is not accepting those parameters as inputs, why > should the application/user burden himself with knowing parameters > of no relevance to him ? But it will accept them as inputs. without complaint too. I can send DTV_INNER_FEC w/ FEC_5_11 to stv090x and it doesnt complain at all, even though it doesnt support it. It'll even acquire a lock just because the demod uses blind search. So the driver most definitely does accept fec that it cant use. > Actually with all those redundant FEC bits gone away from relevance, things are > a bit more saner. I dont understand this either. "gone away from relevance" are you meaning just how they really arent used much anymore or something? still though if the demod supports them I think we should too. Honestly I still think the .delsys .delmod .delfec is a cleaner approach then we have now which is ugly and mismatched (modulations mixed in with fec, and only some are defined) its not a perfect solution though so I really dont think its worth fighting for if others dont agree with me. Im just kinda surprised that everyone is perfectly happy with the .delsys / .caps method we use Chris -- 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
On Wed, Jul 24, 2013 at 2:57 AM, Chris Lee <updatelee@gmail.com> wrote: >> Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ >> outputs to the demodulator. ;-) > > ya ya :) you knew what I meant, not what I said hehe > >> Demods support all FEC's relevant to their delivery systems. It's just that >> some devices likely do support some additional states. > > This part I dont understand, what do you mean additional states ? and > how would a userland application determine if a demod supports these > additional states? Actually, the userland application shouldn't know about these. >> If I am not mistaken, the genpix hardware is a hardware wrapper around the >> BCM demodulator. So, it is quite likely that even if you don't set any FEC >> parameter, the device could still acquire lock as expected. I am not holding >> my breath on this. Maybe someone with a genpix device can prove me right >> or wrong. > > FEC_AUTO works for all but turbo-qpsk on genpix devices. > That was why the SYS_TURBO flag was introduced. IIRC, you needed one flag alone for the turbo mode. > I still think its important to have all the fec supported in the > driver though even if FEC_AUTO did work 100% else why even have it as > an option at all. Maybe, FEC_AUTO is broken for some very old hardware. If FEC_AUTO works just as expected, why would you have to take the gigantic effort of specifying parameters by hand which is error prone which you have mentioned later on ? I fail to understand your point. >> With the STB0899 driver, all you need to tune with it is Frequency, >> Symbol Rate and Delivery system >> >> >> With the STV090x driver all you need is Frequency and Symbol Rate. >> (It will auto detect delivery system) > > Same thing, I still think if we allow the user to send a fec value we > should make sure its right, else why not just hard code all the > drivers to fec-auto that support it and remove the option all > together. I dont like that option. This is why it was decided eventually that the FEC bits are redundant and we decided not to create large lists and enumerations causing insanity and not to mention ugliness. AFAIR, almost all drivers do FEC_AUTO, except for the ones which have some known issues. >> When a driver is not accepting those parameters as inputs, why >> should the application/user burden himself with knowing parameters >> of no relevance to him ? > > But it will accept them as inputs. without complaint too. I can send > DTV_INNER_FEC w/ FEC_5_11 to stv090x and it doesnt complain at all, > even though it doesnt support it. It'll even acquire a lock just > because the demod uses blind search. So the driver most definitely > does accept fec that it cant use. The driver will acquire a lock to the frequency/srate and "return" the relevant FEC value for the user/application. This avoids pitfalls and human errors in manually specifying FEC bits to tune configurations, as I described above. Because some legacy application does set a FEC value which might be wrong and the rest are correct, I wouldn't fail that request. >> Actually with all those redundant FEC bits gone away from relevance, things are >> a bit more saner. > > I dont understand this either. "gone away from relevance" are you > meaning just how they really arent used much anymore or something? > still though if the demod supports them I think we should too. Yeah, they aren't really used at all. They exist for compatibility reasons. Manu -- 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
The problems isnt for tuners where FEC_AUTO does work, its more for ones that dont work like the genpix. Im sure there are others too. I still think that userland applications should be able to poll that info and that the ability to poll the info is a good thing not a bad thing. oh well, lets let this patch die, and the idea can be revisited in the future if it warrants more of a pressing need. Chris -- 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
On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee <updatelee@gmail.com> wrote: > The problems isnt for tuners where FEC_AUTO does work, its more for > ones that dont work like the genpix. Im sure there are others too. If FEC_AUTO for turbo qpsk can be fixed in the Genpix firmware, maybe it's worth seeing if Genpix will have a look into it.? -- 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
He is, I talked to him last month about various things and he mentioned turbofec-qpsk FEC_AUTO is semi working and its in his plans. Chris On Tue, Jul 23, 2013 at 5:39 PM, VDR User <user.vdr@gmail.com> wrote: > On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee <updatelee@gmail.com> wrote: >> The problems isnt for tuners where FEC_AUTO does work, its more for >> ones that dont work like the genpix. Im sure there are others too. > > If FEC_AUTO for turbo qpsk can be fixed in the Genpix firmware, maybe > it's worth seeing if Genpix will have a look into it.? -- 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
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index 1f925e8..f5df08e 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -1036,6 +1036,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = { _DTV_CMD(DTV_API_VERSION, 0, 0), _DTV_CMD(DTV_ENUM_DELSYS, 0, 0), + _DTV_CMD(DTV_ENUM_DELMOD, 0, 0), + _DTV_CMD(DTV_ENUM_DELFEC, 0, 0), _DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0), _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0), @@ -1285,6 +1287,22 @@ static int dtv_property_process_get(struct dvb_frontend *fe, } tvp->u.buffer.len = ncaps; break; + case DTV_ENUM_DELMOD: + ncaps = 0; + while (fe->ops.delmod[ncaps] && ncaps < MAX_DELMOD) { + tvp->u.buffer.data[ncaps] = fe->ops.delmod[ncaps]; + ncaps++; + } + tvp->u.buffer.len = ncaps; + break; + case DTV_ENUM_DELFEC: + ncaps = 0; + while (fe->ops.delfec[ncaps] && ncaps < MAX_DELFEC) { + tvp->u.buffer.data[ncaps] = fe->ops.delfec[ncaps]; + ncaps++; + } + tvp->u.buffer.len = ncaps; + break; case DTV_FREQUENCY: tvp->u.data = c->frequency; break; diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h index 371b6ca..4e96640 100644 --- a/drivers/media/dvb-core/dvb_frontend.h +++ b/drivers/media/dvb-core/dvb_frontend.h @@ -47,6 +47,8 @@ * should be smaller or equal to 32 */ #define MAX_DELSYS 8 +#define MAX_DELMOD 8 +#define MAX_DELFEC 32 struct dvb_frontend_tune_settings { int min_delay_ms; @@ -263,6 +265,8 @@ struct dvb_frontend_ops { struct dvb_frontend_info info; u8 delsys[MAX_DELSYS]; + u8 delmod[MAX_DELMOD]; + u8 delfec[MAX_DELFEC]; void (*release)(struct dvb_frontend* fe); void (*release_sec)(struct dvb_frontend* fe); diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h index c56d77c..be63d37 100644 --- a/include/uapi/linux/dvb/frontend.h +++ b/include/uapi/linux/dvb/frontend.h @@ -375,7 +375,10 @@ struct dvb_frontend_event { #define DTV_STAT_ERROR_BLOCK_COUNT 68 #define DTV_STAT_TOTAL_BLOCK_COUNT 69 -#define DTV_MAX_COMMAND DTV_STAT_TOTAL_BLOCK_COUNT +#define DTV_ENUM_DELMOD 70 +#define DTV_ENUM_DELFEC 71 + +#define DTV_MAX_COMMAND DTV_ENUM_DELFEC typedef enum fe_pilot { PILOT_ON, diff --git a/drivers/media/dvb-frontends/au8522_dig.c b/drivers/media/dvb-frontends/au8522_dig.c index 6ee9028..1044c9d 100644 --- a/drivers/media/dvb-frontends/au8522_dig.c +++ b/drivers/media/dvb-frontends/au8522_dig.c @@ -822,7 +822,9 @@ error: EXPORT_SYMBOL(au8522_attach); static struct dvb_frontend_ops au8522_ops = { - .delsys = { SYS_ATSC, SYS_DVBC_ANNEX_B }, + .delsys = { SYS_DVBC_ANNEX_B, SYS_ATSC }, + .delmod = { QAM_256, QAM_64, VSB_8 }, + .delfec = { FEC_NONE }, .info = { .name = "Auvitek AU8522 QAM/8VSB Frontend", .frequency_min = 54000000,