Johannes Stezenbach wrote:
Hi Manu,
How does this proposal relate to the DVB-S2 proposal by mws?
Does it supersede it?
I think Marcel (mws) also like this patch, apart from the comments you
have. I had been working on the stb0899 + stb6100 (a multi standard
device that supports DVB-S/S2 and DSS with a silicon tuner. It is now a
lot done. hence i took a hand at the patch. It should be complete soon.
diff -Naurp multi-proto-orig/linux/drivers/media/dvb/dvb-core/dvb_frontend.c multi-proto2/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
--- multi-proto-orig/linux/drivers/media/dvb/dvb-core/dvb_frontend.c 2006-04-20 20:56:39.000000000 +0400
+++ multi-proto2/linux/drivers/media/dvb/dvb-core/dvb_frontend.c 2006-04-25 03:46:55.000000000 +0400
@@ -94,6 +94,8 @@ struct dvb_frontend_private {
/* thread/frontend values */
struct dvb_device *dvbdev;
struct dvb_frontend_parameters parameters;
+ struct dvb_frontend_params params;
Bad naming -- make it clear which one is new / legacy.
I will add in some comments.
+struct modcod_table dvbs2_modcod_lookup[] = {
+ { MOD_QPSK_1_4, FE_MOD_QPSK, FE_FECRATE_1_4 },
I see this cruft is actually in the DVB-S2 standard. Ugh!
It is not just in the standard alone, but in fact these values are
directly returned from the STB0899 DSP itself. We can straight away get
the modulation/code rates directly from it.
+int decode_dvbs2_modcod(struct dvb_frontend *fe, enum fe_modcod modcod)
+{
+ int i;
+
+ struct modcod_table *table;
+ struct dvb_frontend_private *fepriv = fe->frontend_priv;
+ struct dvb_frontend_params *params = &fepriv->params;
+
+ if (params->delivery & FE_DELSYS_DVBS2) {
+ for (i = 0, table = dvbs2_modcod_lookup;
+ i < ARRAY_SIZE(dvbs2_modcod_lookup);
+ i++, table++) {
+
+ if (table->modcod == modcod) {
+ params->delsys.dvbs2.modulation = table->modulation;
+ params->delsys.dvbs2.fecrate = table->fecrate;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(decode_dvbs2_modcod);
What's the loop for? Why not just make modcod an index into the array?
yeah, right.
diff -Naurp multi-proto-orig/linux/drivers/media/dvb/dvb-core/dvb_frontend.h multi-proto2/linux/drivers/media/dvb/dvb-core/dvb_frontend.h
--- multi-proto-orig/linux/drivers/media/dvb/dvb-core/dvb_frontend.h 2006-04-20 20:56:39.000000000 +0400
+++ multi-proto2/linux/drivers/media/dvb/dvb-core/dvb_frontend.h 2006-04-25 03:30:38.000000000 +0400
@@ -61,6 +61,12 @@ struct dvb_tuner_info {
u32 bandwidth_step;
};
+struct modcod_table {
+ u32 modcod;
+ u32 modulation;
+ u32 fecrate;
+};
+
Is used in implementation only -> move to .c file
ok
diff -Naurp multi-proto-orig/linux/include/linux/dvb/frontend.h multi-proto2/linux/include/linux/dvb/frontend.h
--- multi-proto-orig/linux/include/linux/dvb/frontend.h 2006-04-20 20:56:40.000000000 +0400
+++ multi-proto2/linux/include/linux/dvb/frontend.h 2006-04-25 03:47:10.000000000 +0400
@@ -274,4 +274,273 @@ struct dvb_frontend_event {
#define FE_DISHNETWORK_SEND_LEGACY_CMD _IO('o', 80) /* unsigned int */
+/**
+ * Supported delivery systems
+ * some frontends are capable of supporting
+ * multiple delivery systems
+ */
+enum fe_delsys {
+ FE_DELSYS_QUERY = 0x00000000,
How is QUERY used?
Initially we had only one MODCOD table from where everything was
decoded, at that time Ralph suggested that we can use "0" to query to
identify between multi standard devices, which was a very cool idea.
Then someone came up with the thought that there wouldn't be enough
space in the structure for the future, and hence now we have split up
the table. I think it would be better if we replace QUERY with IGNORE,
ie FE_DELSYS_IGNORE etc, we can ask GET_PARAMS/CAPS not to retrieve that
value. this will be helpful to avoid unnecessary I2C communication,
which results in faster results.
+ FE_DELSYS_DVBS = 0x00000001,
+ FE_DELSYS_DVBS2 = 0x00000002,
+ FE_DELSYS_DSS = 0x00000004,
+ FE_DELSYS_DVBC = 0x00000008,
+ FE_DELSYS_DVBT = 0x00000010,
+ FE_DELSYS_DVBH = 0x00000020,
+ FE_DELSYS_ATSC = 0x00000040,
+ FE_DELSYS_AUTO = 0x10000000
What does AUTO mean here?
Did you mean to use 0x80000000?
Ah, yes. Human error.
Generally, I prefer to write (1 << 31) etc., but that's
a matter of taste.
Ah, yes, since you pointed it out, will go that way, using the bitmasks
only for the STB0899
+};
+
+enum fe_matype {
+ FE_MATYPE_QUERY = 0x00000000,
+ FE_TRANSPORT = 0x00000001,
+ FE_GENERIC_PACKET = 0x00000002,
+ FE_GENERIC_CONTINUOUS = 0x00000004,
+ FE_RESERVED = 0x00000008,
+ FE_MATYPE_AUTO = 0x10000000
+};
I'm not sure if it makes sense to include this. Let's stick
with MPEG2 TS untils someone can demonstrate the other
modes are useful.
Well, the Nyquist rolloff rates are based on these parameters, ie, a =
0.20, a =0.25, a = 0.35
at the STB0899 side i have something like this , which maps to this like
this ..
enum stb0899_rrc_alpha {
STB0899_RRC_20 = 0x00000000,
STB0899_RRC_25,
STB0899_RRC_35
};
+struct dvb_frontend_cap {
+ enum fe_delsys delivery;
+ enum fe_inversion inversion;
+
+ union {
+ struct dvbs_params dvbs;
+ struct dvbs2_params dvbs2;
+ struct dss_params dss;
+ struct dvbc_params dvbc;
+ struct dvbt_params dvbt;
+ struct dvbh_params dvbh;
+ struct atsc_params atsc;
+ } delsys;
+};
if this doesn't report capabilities then _cap is the wrong name
I noticed it a little while back only. In fact the IOCTL naming is
wrong. :-(
I will fix this.
+struct dvb_frontend_params {
+ __u32 frequency;
+ enum fe_delsys delivery;
+ enum fe_inversion inversion;
+
+ union {
+ struct dvbs_params dvbs;
+ struct dvbs2_params dvbs2;
+ struct dss_params dss;
+ struct dvbc_params dvbc;
+ struct dvbt_params dvbt;
+ struct dvbh_params dvbh;
+ struct atsc_params atsc;
+ } delsys;
The kernel now requires gcc-3.x, so you could use anonymous unions if
you want.
But you repeat the same mistake which we already made: No room
for future binary compitable extension :-(
What's your thought on removing the size field in the IOCTL, so that the
size won't be an issue ?
That way it will be binary compatible. :-) Or any other thoughts you
have ? We can retain the size for now, but we can remove it as and when
need arises ..
+#define FE_SET_PARAMS _IOW('o', 81, struct dvb_frontend_params)
+#define FE_GET_PARAMS _IOWR('o', 82, struct dvb_frontend_cap)
Again, the name doesn't make it clear which is new / legacy.
It wouldn't hurt to add a short comment, too.
Ah, i will update it up with the changes. Thanks for the comments.
Thanks,
Manu
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
