Re: [git:v4l-dvb/for_v3.3] [media] dvb_frontend: Don't use ops-info.type anymore
On Wednesday 04 January 2012 20:29:36 Mauro Carvalho Chehab wrote:
drivers/media/dvb/dvb-core/dvb_frontend.c | 541
++---
1 files changed, 266 insertions(+), 275 deletions(-)
...
-static int dvb_frontend_check_parameters(struct dvb_frontend *fe,
- struct dvb_frontend_parameters *parms)
+static int dvb_frontend_check_parameters(struct dvb_frontend *fe)
{
...
- /* check for supported modulation */
- if (fe-ops.info.type == FE_QAM
- (parms-u.qam.modulation QAM_AUTO ||
- !((1 (parms-u.qam.modulation + 10)) fe-ops.info.caps))) {
- printk(KERN_WARNING DVB: adapter %i frontend %i modulation %u
not supported\n,
-fe-dvb-num, fe-id, parms-u.qam.modulation);
+ /*
+ * check for supported modulation
+ *
+ * This is currently hacky. Also, it only works for DVB-S friends,
+ * and not all modulations has FE_CAN flags
+ */
+ switch (c-delivery_system) {
+ case SYS_DVBS:
+ case SYS_DVBS2:
+ case SYS_TURBO:
+ if ((c-modulation QAM_AUTO ||
+ !((1 (c-modulation + 10)) fe-ops.info.caps))) {
^^
+ printk(KERN_WARNING
+DVB: adapter %i frontend %i modulation %u not
supported\n,
+fe-dvb-num, fe-id, c-modulation);
return -EINVAL;
+ }
+ break;
...
This code is completely bogus: I get tons of warnings, if vdr tries to
tune to DVB-S2 (modulation == 9 == PSK_8) on my stv090x.
PSK_8 == 9 is QAM_AUTO, and the shift operation does not make much
sense, except for modulation == 0 == QPSK.
The original version makes more sense for me.
CU
Oliver
--
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
--
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: [git:v4l-dvb/for_v3.3] [media] dvb_frontend: Don't use ops-info.type anymore
On 06-01-2012 22:36, Oliver Endriss wrote:
On Wednesday 04 January 2012 20:29:36 Mauro Carvalho Chehab wrote:
drivers/media/dvb/dvb-core/dvb_frontend.c | 541
++---
1 files changed, 266 insertions(+), 275 deletions(-)
...
-static int dvb_frontend_check_parameters(struct dvb_frontend *fe,
-struct dvb_frontend_parameters *parms)
+static int dvb_frontend_check_parameters(struct dvb_frontend *fe)
{
...
-/* check for supported modulation */
-if (fe-ops.info.type == FE_QAM
-(parms-u.qam.modulation QAM_AUTO ||
- !((1 (parms-u.qam.modulation + 10)) fe-ops.info.caps))) {
-printk(KERN_WARNING DVB: adapter %i frontend %i modulation %u
not supported\n,
- fe-dvb-num, fe-id, parms-u.qam.modulation);
+/*
+ * check for supported modulation
+ *
+ * This is currently hacky. Also, it only works for DVB-S friends,
+ * and not all modulations has FE_CAN flags
+ */
+switch (c-delivery_system) {
+case SYS_DVBS:
+case SYS_DVBS2:
+case SYS_TURBO:
+if ((c-modulation QAM_AUTO ||
+!((1 (c-modulation + 10)) fe-ops.info.caps))) {
^^
+printk(KERN_WARNING
+ DVB: adapter %i frontend %i modulation %u not
supported\n,
+ fe-dvb-num, fe-id, c-modulation);
return -EINVAL;
+}
+break;
...
This code is completely bogus: I get tons of warnings, if vdr tries to
tune to DVB-S2 (modulation == 9 == PSK_8) on my stv090x.
PSK_8 == 9 is QAM_AUTO, and the shift operation does not make much
sense, except for modulation == 0 == QPSK.
The original version makes more sense for me.
Oliver,
At least for DVBv3 calls, the old code will also generate bogus
warnings if you try to use a DVBv3 call to set PSK_8.
I almost removed this validation code during the conversion for several
reasons:
1) it does some magic by assuming that all QAM modulations are below
QAM_AUTO;
2) it checks modulation parameters only for DVB-S. IMO, or the core should
invalid parameters for all delivery systems, or should let the frontend
drivers do it;
3) frontend drivers should already be checking for invalid parameters
(most of them do it, anyway);
4) not all modulations are mapped at fe-ops.info.caps, so it is not
even possible to check for the valid modulations inside the core for
some delivery systems;
5) Why the core checks just the modulation, and doesn't check for other
types of invalid parameters, like FEC and bandwidth?
At the end, I decided to keep it, but added that note, as I really didn't
like that part of the code.
I can see two fixes for this:
a) just remove the validation, and let the frontend check what's
supported;
b) rewrite the code with a per-standard table of valid values.
I vote for removing the validation logic there.
Regards,
Mauro
--
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: [git:v4l-dvb/for_v3.3] [media] dvb_frontend: Don't use ops-info.type anymore
On Saturday 07 January 2012 03:05:40 Mauro Carvalho Chehab wrote:
On 06-01-2012 22:36, Oliver Endriss wrote:
On Wednesday 04 January 2012 20:29:36 Mauro Carvalho Chehab wrote:
drivers/media/dvb/dvb-core/dvb_frontend.c | 541
++---
1 files changed, 266 insertions(+), 275 deletions(-)
...
-static int dvb_frontend_check_parameters(struct dvb_frontend *fe,
- struct dvb_frontend_parameters *parms)
+static int dvb_frontend_check_parameters(struct dvb_frontend *fe)
{
...
- /* check for supported modulation */
- if (fe-ops.info.type == FE_QAM
- (parms-u.qam.modulation QAM_AUTO ||
- !((1 (parms-u.qam.modulation + 10)) fe-ops.info.caps))) {
- printk(KERN_WARNING DVB: adapter %i frontend %i modulation %u
not supported\n,
- fe-dvb-num, fe-id, parms-u.qam.modulation);
+ /*
+ * check for supported modulation
+ *
+ * This is currently hacky. Also, it only works for DVB-S friends,
+ * and not all modulations has FE_CAN flags
+ */
+ switch (c-delivery_system) {
+ case SYS_DVBS:
+ case SYS_DVBS2:
+ case SYS_TURBO:
+ if ((c-modulation QAM_AUTO ||
+ !((1 (c-modulation + 10)) fe-ops.info.caps))) {
^^
+ printk(KERN_WARNING
+ DVB: adapter %i frontend %i modulation %u not
supported\n,
+ fe-dvb-num, fe-id, c-modulation);
return -EINVAL;
+ }
+ break;
...
This code is completely bogus: I get tons of warnings, if vdr tries to
tune to DVB-S2 (modulation == 9 == PSK_8) on my stv090x.
PSK_8 == 9 is QAM_AUTO, and the shift operation does not make much
sense, except for modulation == 0 == QPSK.
The original version makes more sense for me.
Oliver,
At least for DVBv3 calls, the old code will also generate bogus
warnings if you try to use a DVBv3 call to set PSK_8.
No, since the checks were only performed for type==QAM, i.e. DVB-C.
So DVB-S2 was not affected before.
I almost removed this validation code during the conversion for several
reasons:
1) it does some magic by assuming that all QAM modulations are below
QAM_AUTO;
2) it checks modulation parameters only for DVB-S. IMO, or the core should
invalid parameters for all delivery systems, or should let the frontend
drivers do it;
3) frontend drivers should already be checking for invalid parameters
(most of them do it, anyway);
4) not all modulations are mapped at fe-ops.info.caps, so it is not
even possible to check for the valid modulations inside the core for
some delivery systems;
5) Why the core checks just the modulation, and doesn't check for other
types of invalid parameters, like FEC and bandwidth?
At the end, I decided to keep it, but added that note, as I really didn't
like that part of the code.
I can see two fixes for this:
a) just remove the validation, and let the frontend check what's
supported;
b) rewrite the code with a per-standard table of valid values.
I vote for removing the validation logic there.
Ack.
Atm the core could only do proper checks for frontends, which support a
single delivery system. For multi-delsys frontends some values of the
info struct might not apply to the currently selected delivery system.
To fix this, you need precise information, what is supported for a given
delivery system. In this case we need 'per delivery system' information.
Maybe it would make sense to add a callback, and let the driver do the
checks?
Furthermore, old API-5 applications do not set the delivery system!
For example: VDR checked the FE_CAN_2G_MODULATION flag and eventually
issues a tune call, no matter whether the current delsys is DVB-S or
DVB-S2. So it is difficult to do check parameters in a precise way,
while keeping backward compatibility.
CU
Oliver
--
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
--
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: [git:v4l-dvb/for_v3.3] [media] dvb_frontend: Don't use ops-info.type anymore
On 07-01-2012 03:18, Oliver Endriss wrote:
On Saturday 07 January 2012 03:05:40 Mauro Carvalho Chehab wrote:
On 06-01-2012 22:36, Oliver Endriss wrote:
On Wednesday 04 January 2012 20:29:36 Mauro Carvalho Chehab wrote:
drivers/media/dvb/dvb-core/dvb_frontend.c | 541
++---
1 files changed, 266 insertions(+), 275 deletions(-)
...
-static int dvb_frontend_check_parameters(struct dvb_frontend *fe,
- struct dvb_frontend_parameters *parms)
+static int dvb_frontend_check_parameters(struct dvb_frontend *fe)
{
...
- /* check for supported modulation */
- if (fe-ops.info.type == FE_QAM
- (parms-u.qam.modulation QAM_AUTO ||
- !((1 (parms-u.qam.modulation + 10)) fe-ops.info.caps))) {
- printk(KERN_WARNING DVB: adapter %i frontend %i modulation %u
not supported\n,
- fe-dvb-num, fe-id, parms-u.qam.modulation);
+ /*
+ * check for supported modulation
+ *
+ * This is currently hacky. Also, it only works for DVB-S friends,
+ * and not all modulations has FE_CAN flags
+ */
+ switch (c-delivery_system) {
+ case SYS_DVBS:
+ case SYS_DVBS2:
+ case SYS_TURBO:
+ if ((c-modulation QAM_AUTO ||
+ !((1 (c-modulation + 10)) fe-ops.info.caps))) {
^^
+ printk(KERN_WARNING
+ DVB: adapter %i frontend %i modulation %u not
supported\n,
+ fe-dvb-num, fe-id, c-modulation);
return -EINVAL;
+ }
+ break;
...
This code is completely bogus: I get tons of warnings, if vdr tries to
tune to DVB-S2 (modulation == 9 == PSK_8) on my stv090x.
PSK_8 == 9 is QAM_AUTO, and the shift operation does not make much
sense, except for modulation == 0 == QPSK.
The original version makes more sense for me.
Oliver,
At least for DVBv3 calls, the old code will also generate bogus
warnings if you try to use a DVBv3 call to set PSK_8.
No, since the checks were only performed for type==QAM, i.e. DVB-C.
So DVB-S2 was not affected before.
Sorry, I coded it wrong.
Anyway, when DVB-C2 will be added, the code will likely break again.
I almost removed this validation code during the conversion for several
reasons:
1) it does some magic by assuming that all QAM modulations are below
QAM_AUTO;
2) it checks modulation parameters only for DVB-S. IMO, or the core should
invalid parameters for all delivery systems, or should let the frontend
drivers do it;
3) frontend drivers should already be checking for invalid parameters
(most of them do it, anyway);
4) not all modulations are mapped at fe-ops.info.caps, so it is not
even possible to check for the valid modulations inside the core for
some delivery systems;
5) Why the core checks just the modulation, and doesn't check for other
types of invalid parameters, like FEC and bandwidth?
At the end, I decided to keep it, but added that note, as I really didn't
like that part of the code.
I can see two fixes for this:
a) just remove the validation, and let the frontend check what's
supported;
b) rewrite the code with a per-standard table of valid values.
I vote for removing the validation logic there.
Ack.
Atm the core could only do proper checks for frontends, which support a
single delivery system. For multi-delsys frontends some values of the
info struct might not apply to the currently selected delivery system.
To fix this, you need precise information, what is supported for a given
delivery system. In this case we need 'per delivery system' information.
Maybe it would make sense to add a callback, and let the driver do the
checks?
With the changes I made, all frontends are now filling ops.delsys with
the supported delivery system. The DVB core picks ops.delsys[0] during
register and on cache clear. So, no callback is needed, and the core can
quickly check the supported delivery systems.
Furthermore, old API-5 applications do not set the delivery system!
In this case, it will use ops.delsys[0]. On all frondends with 2G support,
the first delivery system is the 1 gen.
For example: VDR checked the FE_CAN_2G_MODULATION flag and eventually
issues a tune call, no matter whether the current delsys is DVB-S or
DVB-S2. So it is difficult to do check parameters in a precise way,
while keeping backward compatibility.
Yes. Still, the frontend may not do the right thing, as it may use different
checks for SYS_DVBS/SYS_DVBS2 internally.
I did not touch (at least not intentionally) on any of such checks, but a
quick grep for SYS_DVBS2 shows that setting the wrong delivery system will
cause troubles.
For example, on frontends/ds3000.c, ds3000_read_status() uses a different
register for DVB_S2 lock. So, an application that doesn't set the delivery
system to SYS_DVBS2 will (likely) not lock into DVB-S2 channels.
The
