| Message ID | 20170624160301.17710-5-d.scheller.oss@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Daniel Scheller writes: > From: Daniel Scheller <d.scheller@gmx.net> > > Original code at least has some signed/unsigned issues, resulting in > values like 32dBm. I will look into that. > Change signal strength readout to work without asking > the attached tuner, and use a lookup table instead of log calc. Values How can you determine the exact strength without knowing what the tuner did? At least the stv6111 does its own AGC which has to be added. > +struct SLookup padc_lookup[] = { > + { 0, 118000 }, /* PADC=+0dBm */ > + { -100, 93600 }, /* PADC=-1dBm */ > + { -200, 74500 }, /* PADC=-2dBm */ > + { -300, 59100 }, /* PADC=-3dBm */ > + { -400, 47000 }, /* PADC=-4dBm */ > + { -500, 37300 }, /* PADC=-5dBm */ > + { -600, 29650 }, /* PADC=-6dBm */ > + { -700, 23520 }, /* PADC=-7dBm */ > + { -900, 14850 }, /* PADC=-9dBm */ > + { -1100, 9380 }, /* PADC=-11dBm */ > + { -1300, 5910 }, /* PADC=-13dBm */ > + { -1500, 3730 }, /* PADC=-15dBm */ > + { -1700, 2354 }, /* PADC=-17dBm */ > + { -1900, 1485 }, /* PADC=-19dBm */ > + { -2000, 1179 }, /* PADC=-20dBm */ > + { -2100, 1000 }, /* PADC=-21dBm */ > +}; ... > - if (bbgain < (s32) *strength) > - *strength -= bbgain; > - else > - *strength = 0; > + padc = TableLookup(padc_lookup, ARRAY_SIZE(padc_lookup), Power) + 352; > Where does the padc_lookup table come from? I saw it before in CrazyCat github tree. Is he or you the original source/author or somebody else? Regards, Ralph
Em Mon, 26 Jun 2017 10:55:34 +0200 Ralph Metzler <rjkm@metzlerbros.de> escreveu: > Daniel Scheller writes: > > From: Daniel Scheller <d.scheller@gmx.net> > > > > Original code at least has some signed/unsigned issues, resulting in > > values like 32dBm. > > I will look into that. > > > Change signal strength readout to work without asking > > the attached tuner, and use a lookup table instead of log calc. Values > > How can you determine the exact strength without knowing what the tuner did? > At least the stv6111 does its own AGC which has to be added. I remember I had to solve this issue on some other driver[1][2][3]. What I did was to get the AGC gain from the tuner using a callback, then I added it to the main gain. [1] https://www.spinics.net/lists/linux-media/msg101836.html [2] https://www.spinics.net/lists/linux-media/msg101838.html [3] https://www.spinics.net/lists/linux-media/msg101842.html I don't remember why it was not merged upstream, though. Perhaps because I was in doubt about reporting it as "rf_attenuation" or as "agc gain". Anyway, with something like that, any demod could check for such callback. If defined, add it to its AGC own gain, in order to get the total AGC gain. > > +struct SLookup padc_lookup[] = { > > + { 0, 118000 }, /* PADC=+0dBm */ > > + { -100, 93600 }, /* PADC=-1dBm */ > > + { -200, 74500 }, /* PADC=-2dBm */ > > + { -300, 59100 }, /* PADC=-3dBm */ > > + { -400, 47000 }, /* PADC=-4dBm */ > > + { -500, 37300 }, /* PADC=-5dBm */ > > + { -600, 29650 }, /* PADC=-6dBm */ > > + { -700, 23520 }, /* PADC=-7dBm */ > > + { -900, 14850 }, /* PADC=-9dBm */ > > + { -1100, 9380 }, /* PADC=-11dBm */ > > + { -1300, 5910 }, /* PADC=-13dBm */ > > + { -1500, 3730 }, /* PADC=-15dBm */ > > + { -1700, 2354 }, /* PADC=-17dBm */ > > + { -1900, 1485 }, /* PADC=-19dBm */ > > + { -2000, 1179 }, /* PADC=-20dBm */ > > + { -2100, 1000 }, /* PADC=-21dBm */ > > +}; > ... > > - if (bbgain < (s32) *strength) > > - *strength -= bbgain; > > - else > > - *strength = 0; > > + padc = TableLookup(padc_lookup, ARRAY_SIZE(padc_lookup), Power) + 352; > > > > > Where does the padc_lookup table come from? > I saw it before in CrazyCat github tree. > Is he or you the original source/author or somebody else? > > > Regards, > Ralph Thanks, Mauro
Mauro Carvalho Chehab writes: > Em Mon, 26 Jun 2017 10:55:34 +0200 > Ralph Metzler <rjkm@metzlerbros.de> escreveu: > > > Daniel Scheller writes: > > > From: Daniel Scheller <d.scheller@gmx.net> > > > > > > Original code at least has some signed/unsigned issues, resulting in > > > values like 32dBm. > > > > I will look into that. > > > > > Change signal strength readout to work without asking > > > the attached tuner, and use a lookup table instead of log calc. Values > > > > How can you determine the exact strength without knowing what the tuner did? > > At least the stv6111 does its own AGC which has to be added. > > I remember I had to solve this issue on some other driver[1][2][3]. What I > did was to get the AGC gain from the tuner using a callback, > then I added it to the main gain. > > [1] https://www.spinics.net/lists/linux-media/msg101836.html > [2] https://www.spinics.net/lists/linux-media/msg101838.html > [3] https://www.spinics.net/lists/linux-media/msg101842.html > > I don't remember why it was not merged upstream, though. Perhaps because > I was in doubt about reporting it as "rf_attenuation" or as "agc gain". > > Anyway, with something like that, any demod could check for such > callback. If defined, add it to its AGC own gain, in order to get > the total AGC gain. I misused get_rf_strength for this in my versions of stv6111.c/stv0910.c. But get_rf_attenuation would be exactly what we need. Daniel now removed the get_rf_strength() call from stv0910.c and ignores the correction from the tuner. That was what I was commenting on. Regards, Ralph
Am Mon, 26 Jun 2017 10:55:34 +0200 schrieb Ralph Metzler <rjkm@metzlerbros.de>: > Daniel Scheller writes: > > From: Daniel Scheller <d.scheller@gmx.net> > > > > Original code at least has some signed/unsigned issues, resulting > > in values like 32dBm. > > I will look into that. > > > Change signal strength readout to work without asking > > the attached tuner, and use a lookup table instead of log calc. > > Values > > How can you determine the exact strength without knowing what the > tuner did? At least the stv6111 does its own AGC which has to be > added. Good to know. Though, from what I gathered, a lot of demod drivers are made this way, e.g. read out the AGC, do some math and have a signal strength as result. If there are ways to do this better and/or accurately, then by all means lets do this :) Re the 32dBm, this is from a user who reported even four different values on the same coax cable (as he claimed). A MaxS8 and some measuring gear reported something around -25dBm. With the stv0910, the initial port from dddvb to the kernel reported those 32dBm, "the other driver" (suspect he meant dddvb but he wasn't exact in what that "other driver" was) did report -9dBm, and this changed variant reported around -30dBm, which seems plausible wrt the MaxS8 and his gauge. > > +struct SLookup padc_lookup[] = { > > + { 0, 118000 }, /* PADC=+0dBm */ > > + { -100, 93600 }, /* PADC=-1dBm */ > > + { -200, 74500 }, /* PADC=-2dBm */ > > + { -300, 59100 }, /* PADC=-3dBm */ > > + { -400, 47000 }, /* PADC=-4dBm */ > > + { -500, 37300 }, /* PADC=-5dBm */ > > + { -600, 29650 }, /* PADC=-6dBm */ > > + { -700, 23520 }, /* PADC=-7dBm */ > > + { -900, 14850 }, /* PADC=-9dBm */ > > + { -1100, 9380 }, /* PADC=-11dBm */ > > + { -1300, 5910 }, /* PADC=-13dBm */ > > + { -1500, 3730 }, /* PADC=-15dBm */ > > + { -1700, 2354 }, /* PADC=-17dBm */ > > + { -1900, 1485 }, /* PADC=-19dBm */ > > + { -2000, 1179 }, /* PADC=-20dBm */ > > + { -2100, 1000 }, /* PADC=-21dBm */ > > +}; > ... > > - if (bbgain < (s32) *strength) > > - *strength -= bbgain; > > - else > > - *strength = 0; > > + padc = TableLookup(padc_lookup, ARRAY_SIZE(padc_lookup), > > Power) + 352; > > > Where does the padc_lookup table come from? > I saw it before in CrazyCat github tree. > Is he or you the original source/author or somebody else? Yes, this is picked from CrazyCat's GIT [1], more precisely, from the commit at [2], which imports an already modified version of your driver code, hidden behind the message "STV6120 tuner driver" (stv0910 is part of that commit). Honestly, no idea if he is the actual author of the table plus the math. As initially mentioned, if we can fix this and do it the real proper way, let's do this and drop this patch, but this needs your help. Best regards, Daniel Scheller [1] https://github.com/crazycat69/linux_media [2] https://github.com/crazycat69/linux_media/commit/9099babc397bb8bd9d0e33f39156643487378768
diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c index 999ee6a8ea23..c1875be01631 100644 --- a/drivers/media/dvb-frontends/stv0910.c +++ b/drivers/media/dvb-frontends/stv0910.c @@ -31,8 +31,6 @@ #include "stv0910.h" #include "stv0910_regs.h" -#define INTLOG10X100(x) ((u32) (((u64) intlog10(x) * 100) >> 24)) - #define EXT_CLOCK 30000000 #define TUNING_DELAY 200 #define BER_SRC_S 0x20 @@ -140,7 +138,7 @@ struct SInitTable { struct SLookup { s16 Value; - u16 RegValue; + u32 RegValue; }; static inline int i2c_write(struct i2c_adapter *adap, u8 adr, @@ -332,6 +330,25 @@ struct SLookup S2_SN_Lookup[] = { { 510, 463 }, /*C/N=51.0dB*/ }; +struct SLookup padc_lookup[] = { + { 0, 118000 }, /* PADC=+0dBm */ + { -100, 93600 }, /* PADC=-1dBm */ + { -200, 74500 }, /* PADC=-2dBm */ + { -300, 59100 }, /* PADC=-3dBm */ + { -400, 47000 }, /* PADC=-4dBm */ + { -500, 37300 }, /* PADC=-5dBm */ + { -600, 29650 }, /* PADC=-6dBm */ + { -700, 23520 }, /* PADC=-7dBm */ + { -900, 14850 }, /* PADC=-9dBm */ + { -1100, 9380 }, /* PADC=-11dBm */ + { -1300, 5910 }, /* PADC=-13dBm */ + { -1500, 3730 }, /* PADC=-15dBm */ + { -1700, 2354 }, /* PADC=-17dBm */ + { -1900, 1485 }, /* PADC=-19dBm */ + { -2000, 1179 }, /* PADC=-20dBm */ + { -2100, 1000 }, /* PADC=-21dBm */ +}; + /********************************************************************* * Tracking carrier loop carrier QPSK 1/4 to 8PSK 9/10 long Frame *********************************************************************/ @@ -572,7 +589,7 @@ static int TrackingOptimization(struct stv *state) } static s32 TableLookup(struct SLookup *Table, - int TableSize, u16 RegValue) + int TableSize, u32 RegValue) { s32 Value; int imin = 0; @@ -1300,17 +1317,18 @@ static int read_ber(struct dvb_frontend *fe, u32 *ber, u32 *n, u32 *d) return 0; } -static int read_signal_strength(struct dvb_frontend *fe, u16 *strength) +static int read_signal_strength(struct dvb_frontend *fe, s64 *strength) { struct stv *state = fe->demodulator_priv; u8 Reg[2]; - s32 bbgain; + u16 agc; + s32 padc; s32 Power = 0; int i; read_regs(state, RSTV0910_P2_AGCIQIN1 + state->regoff, Reg, 2); - *strength = (((u32) Reg[0]) << 8) | Reg[1]; + agc = (((u32) Reg[0]) << 8) | Reg[1]; for (i = 0; i < 5; i += 1) { read_regs(state, RSTV0910_P2_POWERI + state->regoff, Reg, 2); @@ -1320,20 +1338,9 @@ static int read_signal_strength(struct dvb_frontend *fe, u16 *strength) } Power /= 5; - bbgain = (465 - INTLOG10X100(Power)) * 10; - - if (fe->ops.tuner_ops.get_rf_strength) - fe->ops.tuner_ops.get_rf_strength(fe, strength); - else - *strength = 0; - - if (bbgain < (s32) *strength) - *strength -= bbgain; - else - *strength = 0; + padc = TableLookup(padc_lookup, ARRAY_SIZE(padc_lookup), Power) + 352; - if (*strength > 0) - *strength = 10 * (s64) (s16) *strength - 108750; + *strength = (padc - agc); return 0; } @@ -1463,7 +1470,8 @@ static int get_frontend(struct dvb_frontend *fe, { struct stv *state = fe->demodulator_priv; enum fe_status status; - u16 snr = 0, strength = 0; + u16 snr = 0; + s64 strength = 0; u32 ber = 0, bernom = 0, berdenom = 0; u8 tmp;