Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Chris Lee
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  wrote:
> On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread VDR User
On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Chris Lee
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Manu Abraham
On Wed, Jul 24, 2013 at 2:57 AM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Chris Lee
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Manu Abraham
On Tue, Jul 23, 2013 at 10:17 PM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Chris Lee
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  wrote:
> On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Manu Abraham
On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposed modifications to dvb_frontend_ops

2013-07-23 Thread Mauro Carvalho Chehab
Em Mon, 22 Jul 2013 19:28:55 -0600
Chris Lee  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
>  wrote:
> > Hi Chris,
> >
> > Em Fri, 19 Jul 2013 14:27:09 -0600
> > Chris Lee  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 
> >>
> >> 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,
> >>   }
> >> 

Re: Proposed modifications to dvb_frontend_ops

2013-07-22 Thread Chris Lee
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
 wrote:
> Hi Chris,
>
> Em Fri, 19 Jul 2013 14:27:09 -0600
> Chris Lee  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 
>>
>> 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->frequenc

Re: Proposed modifications to dvb_frontend_ops

2013-07-22 Thread Mauro Carvalho Chehab
Hi Chris,

Em Fri, 19 Jul 2013 14:27:09 -0600
Chris Lee  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 
> 
> 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-frontend