Skip to contentSkip to conversation

STV0367/DDB DVBv5 signal statistics

PATCH Superseded Daniel Scheller 2 participants 8 messages Jun 21, 2017 – Jun 24, 2017 Media (V4L/DVB)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover STV0367/DDB DVBv5 signal statistics

From: Daniel Scheller <[email protected]>

This series adds DVBv5 statistics support to the new DDB codepath of the stv0367 demodulator driver.

The changes utilise already existing functionality (in form of register readouts), but wraps the reads in separate functions so the existing relative scale reporting can be kept as-is, while adding the v5 stats in dB scale where appropriate.

Quoted 1 line

From my own testing: Reported values look approx. the same as those

reported by the cxd2841er driver for both -C and -T.

Changes from v1 to v2:
- INTLOG10X100() macro for QAM SNR calculation removed and replaced by directly utilising intlog2 plus a div
- factored statistics collection into *_read_status()
- prevent a possible division by zero (though requires ridiculously good SNR to trigger)
- _read_status() doesn't return -EINVAL anymore if no demod state is set, prevents falsely reported errors from inquiries of userspace tools

Daniel Scheller (4):
[media] dvb-frontends/stv0367: initial DDB DVBv5 stats, implement
ucblocks
[media] dvb-frontends/stv0367: split SNR determination into functions [media] dvb-frontends/stv0367: SNR DVBv5 statistics for DVB-C and T [media] dvb-frontends/stv0367: DVB-C signal strength statistics

 drivers/media/dvb-frontends/stv0367.c | 178 ++++++++++++++++++++++++++++------
 1 file changed, 148 insertions(+), 30 deletions(-)
Signature · 1 line
2.13.0
#1 lore raw

PATCH v2 1/4 dvb-frontends/stv0367: initial DDB DVBv5 stats, implement ucblocks

From: Daniel Scheller <[email protected]>

This adds the basics to stv0367ddb_read_status() to be able to properly provide signal statistics in DVBv5 format. Also adds UCB readout and provides those values. Also, don't return -EINVAL in ddb_read_status() if active_demod_state indicates no delivery system.

---
 drivers/media/dvb-frontends/stv0367.c | 54 ++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)
drivers/media/dvb-frontends/stv0367.c +50 -4
index e726c2e00460..8ba15dc339f8 100644
@@ -2980,21 +2980,59 @@ static int stv0367ddb_set_frontend(struct dvb_frontend *fe)
 	return -EINVAL;
 }
 
+static void stv0367ddb_read_ucblocks(struct dvb_frontend *fe)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	u32 ucblocks = 0;
+
+	switch (state->activedemod) {
+	case demod_ter:
+		stv0367ter_read_ucblocks(fe, &ucblocks);
+		break;
+	case demod_cab:
+		stv0367cab_read_ucblcks(fe, &ucblocks);
+		break;
+	default:
+		p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return;
+	}
+
+	p->block_error.stat[0].scale = FE_SCALE_COUNTER;
+	p->block_error.stat[0].uvalue = ucblocks;
+}
+
 static int stv0367ddb_read_status(struct dvb_frontend *fe,
 				  enum fe_status *status)
 {
 	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	int ret;
 
 	switch (state->activedemod) {
 	case demod_ter:
-		return stv0367ter_read_status(fe, status);
+		ret = stv0367ter_read_status(fe, status);
+		break;
 	case demod_cab:
-		return stv0367cab_read_status(fe, status);
-	default:
+		ret = stv0367cab_read_status(fe, status);
 		break;
+	default:
+		return 0;
 	}
 
-	return -EINVAL;
+	/* stop and report on *_read_status failure */
+	if (ret)
+		return ret;
+
+	/* stop if demod isn't locked */
+	if (!(*status & FE_HAS_LOCK)) {
+		p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return ret;
+	}
+
+	stv0367ddb_read_ucblocks(fe);
+
+	return 0;
 }
 
 static int stv0367ddb_get_frontend(struct dvb_frontend *fe,
@@ -3035,6 +3073,7 @@ static int stv0367ddb_sleep(struct dvb_frontend *fe)
 static int stv0367ddb_init(struct stv0367_state *state)
 {
 	struct stv0367ter_state *ter_state = state->ter_state;
+	struct dtv_frontend_properties *p = &state->fe.dtv_property_cache;
 
 	stv0367_writereg(state, R367TER_TOPCTRL, 0x10);
 
@@ -3109,6 +3148,13 @@ static int stv0367ddb_init(struct stv0367_state *state)
 	ter_state->first_lock = 0;
 	ter_state->unlock_counter = 2;
 
+	p->strength.len = 1;
+	p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	p->cnr.len = 1;
+	p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	p->block_error.len = 1;
+	p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+
 	return 0;
 }
 
Signature · 1 line
2.13.0
#2 lore raw

PATCH v2 2/4 dvb-frontends/stv0367: split SNR determination into functions

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

The read_snr() functions currently do some magic to return relative scale values when called. Split out register readouts into separate functions so the functionality can be reused in some other way.

---
 drivers/media/dvb-frontends/stv0367.c | 68 +++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 26 deletions(-)
drivers/media/dvb-frontends/stv0367.c +42 -26
index 8ba15dc339f8..f8e9cceed04e 100644
@@ -1437,7 +1437,7 @@ static int stv0367ter_get_frontend(struct dvb_frontend *fe,
 	return 0;
 }
 
-static int stv0367ter_read_snr(struct dvb_frontend *fe, u16 *snr)
+static u32 stv0367ter_snr_readreg(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;
 	u32 snru32 = 0;
@@ -1453,10 +1453,16 @@ static int stv0367ter_read_snr(struct dvb_frontend *fe, u16 *snr)
 
 		cpt++;
 	}
-
 	snru32 /= 10;/*average on 10 values*/
 
-	*snr = snru32 / 1000;
+	return snru32;
+}
+
+static int stv0367ter_read_snr(struct dvb_frontend *fe, u16 *snr)
+{
+	u32 snrval = stv0367ter_snr_readreg(fe);
+
+	*snr = snrval / 1000;
 
 	return 0;
 }
@@ -2702,51 +2708,61 @@ static int stv0367cab_read_strength(struct dvb_frontend *fe, u16 *strength)
 	return 0;
 }
 
-static int stv0367cab_read_snr(struct dvb_frontend *fe, u16 *snr)
+static int stv0367cab_snr_power(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;
-	u32 noisepercentage;
 	enum stv0367cab_mod QAMSize;
-	u32 regval = 0, temp = 0;
-	int power, i;
 
 	QAMSize = stv0367_readbits(state, F367CAB_QAM_MODE);
 	switch (QAMSize) {
 	case FE_CAB_MOD_QAM4:
-		power = 21904;
-		break;
+		return 21904;
 	case FE_CAB_MOD_QAM16:
-		power = 20480;
-		break;
+		return 20480;
 	case FE_CAB_MOD_QAM32:
-		power = 23040;
-		break;
+		return 23040;
 	case FE_CAB_MOD_QAM64:
-		power = 21504;
-		break;
+		return 21504;
 	case FE_CAB_MOD_QAM128:
-		power = 23616;
-		break;
+		return 23616;
 	case FE_CAB_MOD_QAM256:
-		power = 21760;
-		break;
-	case FE_CAB_MOD_QAM512:
-		power = 1;
-		break;
+		return 21760;
 	case FE_CAB_MOD_QAM1024:
-		power = 21280;
-		break;
+		return 21280;
 	default:
-		power = 1;
 		break;
 	}
 
+	return 1;
+}
+
+static int stv0367cab_snr_readreg(struct dvb_frontend *fe, int avgdiv)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	u32 regval = 0;
+	int i;
+
 	for (i = 0; i < 10; i++) {
 		regval += (stv0367_readbits(state, F367CAB_SNR_LO)
 			+ 256 * stv0367_readbits(state, F367CAB_SNR_HI));
 	}
 
-	regval /= 10; /*for average over 10 times in for loop above*/
+	if (avgdiv)
+		regval /= 10;
+
+	return regval;
+}
+
+static int stv0367cab_read_snr(struct dvb_frontend *fe, u16 *snr)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	u32 noisepercentage;
+	u32 regval = 0, temp = 0;
+	int power;
+
+	power = stv0367cab_snr_power(fe);
+	regval = stv0367cab_snr_readreg(fe, 1);
+
 	if (regval != 0) {
 		temp = power
 			* (1 << (3 + stv0367_readbits(state, F367CAB_SNR_PER)));
Signature · 1 line
2.13.0
#3 lore raw

PATCH v2 3/4 dvb-frontends/stv0367: SNR DVBv5 statistics for DVB-C and T

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

Add signal-to-noise-ratio as provided by the demodulator in decibel scale. QAM/DVB-C needs some intlog calculation to have usable dB values, OFDM/ DVB-T values from the demod look alright already and are provided as-is.

---
 drivers/media/dvb-frontends/stv0367.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/media/dvb-frontends/stv0367.c +35 -0
index f8e9cceed04e..d3be25bc1002 100644
@@ -25,6 +25,8 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 
+#include "dvb_math.h"
+
 #include "stv0367.h"
 #include "stv0367_defs.h"
 #include "stv0367_regs.h"
@@ -2996,6 +2998,37 @@ static int stv0367ddb_set_frontend(struct dvb_frontend *fe)
 	return -EINVAL;
 }
 
+static void stv0367ddb_read_snr(struct dvb_frontend *fe)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	int cab_pwr;
+	u32 regval, tmpval, snrval = 0;
+
+	switch (state->activedemod) {
+	case demod_ter:
+		snrval = stv0367ter_snr_readreg(fe);
+		break;
+	case demod_cab:
+		cab_pwr = stv0367cab_snr_power(fe);
+		regval = stv0367cab_snr_readreg(fe, 0);
+
+		/* prevent division by zero */
+		if (!regval)
+			snrval = 0;
+
+		tmpval = (cab_pwr * 320) / regval;
+		snrval = ((tmpval != 0) ? (intlog2(tmpval) / 5581) : 0);
+		break;
+	default:
+		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return;
+	}
+
+	p->cnr.stat[0].scale = FE_SCALE_DECIBEL;
+	p->cnr.stat[0].uvalue = snrval;
+}
+
 static void stv0367ddb_read_ucblocks(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;
@@ -3042,10 +3075,12 @@ static int stv0367ddb_read_status(struct dvb_frontend *fe,
 
 	/* stop if demod isn't locked */
 	if (!(*status & FE_HAS_LOCK)) {
+		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		return ret;
 	}
 
+	stv0367ddb_read_snr(fe);
 	stv0367ddb_read_ucblocks(fe);
 
 	return 0;
Signature · 1 line
2.13.0
#4 lore raw

Re: [PATCH v2 3/4] [media] dvb-frontends/stv0367: SNR DVBv5 statistics for DVB-C and T

Em Wed, 21 Jun 2017 21:45:43 +0200
Daniel Scheller <[email protected]> escreveu:

Quoted 63 lines

From: Daniel Scheller <[email protected]>

Add signal-to-noise-ratio as provided by the demodulator in decibel scale. QAM/DVB-C needs some intlog calculation to have usable dB values, OFDM/ DVB-T values from the demod look alright already and are provided as-is.

---
 drivers/media/dvb-frontends/stv0367.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/media/dvb-frontends/stv0367.c +33 -0
index f8e9cceed04e..d3be25bc1002 100644
@@ -25,6 +25,8 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 
+#include "dvb_math.h"
+
 #include "stv0367.h"
 #include "stv0367_defs.h"
 #include "stv0367_regs.h"
@@ -2996,6 +2998,37 @@ static int stv0367ddb_set_frontend(struct dvb_frontend *fe)
 	return -EINVAL;
 }
 
+static void stv0367ddb_read_snr(struct dvb_frontend *fe)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	int cab_pwr;
+	u32 regval, tmpval, snrval = 0;
+
+	switch (state->activedemod) {
+	case demod_ter:
+		snrval = stv0367ter_snr_readreg(fe);
+		break;
+	case demod_cab:
+		cab_pwr = stv0367cab_snr_power(fe);
+		regval = stv0367cab_snr_readreg(fe, 0);
+
+		/* prevent division by zero */
+		if (!regval)
+			snrval = 0;
+
+		tmpval = (cab_pwr * 320) / regval;
+		snrval = ((tmpval != 0) ? (intlog2(tmpval) / 5581) : 0);
+		break;
+	default:
+		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return;
+	}
+
+	p->cnr.stat[0].scale = FE_SCALE_DECIBEL;
+	p->cnr.stat[0].uvalue = snrval;
+}
+
 static void stv0367ddb_read_ucblocks(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;

drivers/media/dvb-frontends/stv0367.c static int stv0367ddb_read_status(struct dvb_frontend *fe, · line 3078

Quoted 4 lines
drivers/media/dvb-frontends/stv0367.c +1 -0
@@ -3042,10 +3075,12 @@ static int stv0367ddb_read_status(struct dvb_frontend *fe,
 
 	/* stop if demod isn't locked */
 	if (!(*status & FE_HAS_LOCK)) {
+		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;

It sounds weird that CNR is only available with full lock.

On a very quick look at the driver, for SYS_DVBC_ANNEX_A, I suspect that you will get CNR earlier. Probably when
cab_state->state == FE_CAB_CARRIEROK.

For SYS_DVBT, it seems that the algorithm doesn't have an intermediate step. So, only after ter_state->state = FE_TER_LOCKOK you have full lock.

So, you should likely apply something like the enclosed (untested) patch to get more status from the frontend, at least for DVB-C.

Thanks,
Mauro

media: stv0367: Improve DVB-C frontend status

The stv0367 driver provide a lot of status on its state machine. Change the logic to provide more information about frontend locking status.

drivers/media/dvb-frontends/stv0367.c +14 -1
index f266c18c574c..9e5432b761b5 100644
@@ -1507,7 +1507,8 @@ static int stv0367ter_read_status(struct dvb_frontend *fe,
 	*status = 0;
 
 	if (stv0367_readbits(state, F367TER_LK)) {
-		*status |= FE_HAS_LOCK;
+		*status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI
+			  | FE_HAS_SYNC | FE_HAS_LOCK;
 		dprintk("%s: stv0367 has locked\n", __func__);
 	}
 
@@ -2155,6 +2156,18 @@ static int stv0367cab_read_status(struct dvb_frontend *fe,
 
 	*status = 0;
 
+	if (state->cab_state->state > FE_CAB_NOSIGNAL)
+		*status |= FE_HAS_SIGNAL;
+
+	if (state->cab_state->state > FE_CAB_NOCARRIER)
+		*status |= FE_HAS_CARRIER;
+
+	if (state->cab_state->state >= FE_CAB_DEMODOK)
+		*status |= FE_HAS_VITERBI;
+
+	if (state->cab_state->state >= FE_CAB_DATAOK)
+		*status |= FE_HAS_SYNC;
+
 	if (stv0367_readbits(state, (state->cab_state->qamfec_status_reg ?
 		state->cab_state->qamfec_status_reg : F367CAB_QAMFEC_LOCK))) {
 		*status |= FE_HAS_LOCK;
#5 lore raw

PATCH v2 4/4 dvb-frontends/stv0367: DVB-C signal strength statistics

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

Provide QAM/DVB-C signal strength in decibel scale. Values returned from stv0367cab_get_rf_lvl() are good but need to be multiplied as they're in 1dBm precision.

---
 drivers/media/dvb-frontends/stv0367.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
drivers/media/dvb-frontends/stv0367.c +21 -0
index d3be25bc1002..bac6707957a3 100644
@@ -2998,6 +2998,25 @@ static int stv0367ddb_set_frontend(struct dvb_frontend *fe)
 	return -EINVAL;
 }
 
+static void stv0367ddb_read_signal_strength(struct dvb_frontend *fe)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	s32 signalstrength;
+
+	switch (state->activedemod) {
+	case demod_cab:
+		signalstrength = stv0367cab_get_rf_lvl(state) * 1000;
+		break;
+	default:
+		p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return;
+	}
+
+	p->strength.stat[0].scale = FE_SCALE_DECIBEL;
+	p->strength.stat[0].uvalue = signalstrength;
+}
+
 static void stv0367ddb_read_snr(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;
@@ -3075,12 +3094,14 @@ static int stv0367ddb_read_status(struct dvb_frontend *fe,
 
 	/* stop if demod isn't locked */
 	if (!(*status & FE_HAS_LOCK)) {
+		p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		return ret;
 	}
 
 	stv0367ddb_read_snr(fe);
+	stv0367ddb_read_signal_strength(fe);
 	stv0367ddb_read_ucblocks(fe);
 
 	return 0;
Signature · 1 line
2.13.0
#6 lore raw

Re: [PATCH v2 4/4] [media] dvb-frontends/stv0367: DVB-C signal strength statistics

Em Wed, 21 Jun 2017 21:45:44 +0200
Daniel Scheller <[email protected]> escreveu:

Quoted 42 lines

From: Daniel Scheller <[email protected]>

Provide QAM/DVB-C signal strength in decibel scale. Values returned from stv0367cab_get_rf_lvl() are good but need to be multiplied as they're in 1dBm precision.

---
 drivers/media/dvb-frontends/stv0367.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
drivers/media/dvb-frontends/stv0367.c +19 -0
index d3be25bc1002..bac6707957a3 100644
@@ -2998,6 +2998,25 @@ static int stv0367ddb_set_frontend(struct dvb_frontend *fe)
 	return -EINVAL;
 }
 
+static void stv0367ddb_read_signal_strength(struct dvb_frontend *fe)
+{
+	struct stv0367_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	s32 signalstrength;
+
+	switch (state->activedemod) {
+	case demod_cab:
+		signalstrength = stv0367cab_get_rf_lvl(state) * 1000;
+		break;
+	default:
+		p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		return;
+	}
+
+	p->strength.stat[0].scale = FE_SCALE_DECIBEL;
+	p->strength.stat[0].uvalue = signalstrength;
+}
+
 static void stv0367ddb_read_snr(struct dvb_frontend *fe)
 {
 	struct stv0367_state *state = fe->demodulator_priv;

drivers/media/dvb-frontends/stv0367.c static int stv0367ddb_read_status(struct dvb_frontend *fe, · line 3097

Quoted 4 lines
drivers/media/dvb-frontends/stv0367.c +1 -0
@@ -3075,12 +3094,14 @@ static int stv0367ddb_read_status(struct dvb_frontend *fe,
 
 	/* stop if demod isn't locked */
 	if (!(*status & FE_HAS_LOCK)) {
+		p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;

Requiring full lock for signal strength sounds really wrong.

Are you sure that it won't work without locks?

Regards,
Mauro

drivers/media/dvb-frontends/stv0367.c static int stv0367ddb_read_status(struct dvb_frontend *fe, · line 3107

Quoted 10 lines
drivers/media/dvb-frontends/stv0367.c +1 -0
 		p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 		return ret;
 	}
 
 	stv0367ddb_read_snr(fe);
+	stv0367ddb_read_signal_strength(fe);
 	stv0367ddb_read_ucblocks(fe);
 
 	return 0;

Thanks,
Mauro

#7 lore raw
In reply to Daniel Scheller

Am Wed, 21 Jun 2017 21:45:40 +0200
schrieb Daniel Scheller <[email protected]>:

Quoted 13 lines

From: Daniel Scheller <[email protected]>

This series adds DVBv5 statistics support to the new DDB codepath of the stv0367 demodulator driver.

The changes utilise already existing functionality (in form of register readouts), but wraps the reads in separate functions so the existing relative scale reporting can be kept as-is, while adding the v5 stats in dB scale where appropriate.

From my own testing: Reported values look approx. the same as those reported by the cxd2841er driver for both -C and -T.

Quoted 8 lines

Changes from v1 to v2:
- INTLOG10X100() macro for QAM SNR calculation removed and replaced by directly utilising intlog2 plus a div
- factored statistics collection into *_read_status()
- prevent a possible division by zero (though requires ridiculously good SNR to trigger)
- _read_status() doesn't return -EINVAL anymore if no demod state is set, prevents falsely reported errors from inquiries of userspace tools

Antti, FYI: statistics inquiry now lives in read_status(), didn't see any functional differences or problems due to this, so if your're comfortable with this, this variant works fine for me. Also, INTLOG10X100 is history, this even gave an improvement of two decimals of precision in dtv_property_cache, which will definitely work for everyone - we're still no scientific pro-measurement-gear. Readout of stats are still limited to FE_HAS_LOCK though, since, as explained, this is the only bit we'll receive at the moment.

Best regards,
Daniel Scheller

Signature · 1 line
https://github.com/herrnst