diff mbox

af9033: implement ber and ucb functions

Message ID 201204032259.43658.hfvogt@gmx.net (mailing list archive)
State Superseded, archived
Headers

Commit Message

Hans-Frieder Vogt April 3, 2012, 8:59 p.m. UTC 
  af9033: implement read_ber and read_ucblocks functions.

Signed-off-by: Hans-Frieder Vogt <hfvogt@gmx.net>

 drivers/media/dvb/frontends/af9033.c |   62 +++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)


Hans-Frieder Vogt                       e-mail: hfvogt <at> gmx .dot. net
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Antti Palosaari April 3, 2012, 10:30 p.m. UTC | #1 
On 03.04.2012 23:59, Hans-Frieder Vogt wrote:
> af9033: implement read_ber and read_ucblocks functions.
>
> Signed-off-by: Hans-Frieder Vogt<hfvogt@gmx.net>

For my quick test UCB counter seems to reset every query. That is 
violation of API. See http://www.kernel.org/doc/htmldocs/media.html

Do you have attenuator you can run tests yourself? It is very cheap and 
useful when coding that kind of signal statistics.

Current API does not even define anymore units for BER and UCB, so those 
calculations are not necessary. Anyhow, you can add some calculations if 
you wish.


regards
Antti
Hans-Frieder Vogt April 6, 2012, 8:34 a.m. UTC | #2 
Am Mittwoch, 4. April 2012 schrieb Antti Palosaari:
> On 03.04.2012 23:59, Hans-Frieder Vogt wrote:
> > af9033: implement read_ber and read_ucblocks functions.
> > 
> > Signed-off-by: Hans-Frieder Vogt<hfvogt@gmx.net>
> 
> For my quick test UCB counter seems to reset every query. That is
> violation of API. See http://www.kernel.org/doc/htmldocs/media.html> 

Indeed, interesting.
I quickly checked the behaviour with a dibcom based stick (dib7000p 
demodulator) and the uncorrected block number reduces there as well. It seems, 
other demodulator drivers ignore this detail as well. But that's not meant to 
be an excuse.....

$ tzap -r sixx
using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
reading channels from file '/home/xxxx/.tzap/channels.conf'
tuning to 754000000 Hz
video pid 0x0111, audio pid 0x0112
status 1f | signal a7f5 | snr 007c | ber 001fffff | unc 000001f2 | FE_HAS_LOCK
status 1f | signal a539 | snr 0080 | ber 00091280 | unc 00001071 | FE_HAS_LOCK
status 1f | signal a5eb | snr 0084 | ber 000a06e0 | unc 00000c6c | FE_HAS_LOCK
status 1f | signal a620 | snr 0087 | ber 0009f4b0 | unc 00000d78 | FE_HAS_LOCK
status 1f | signal a60c | snr 0080 | ber 000a5af0 | unc 00000df3 | FE_HAS_LOCK
status 1f | signal a68f | snr 0082 | ber 0009bfa0 | unc 00000e6f | FE_HAS_LOCK
status 1f | signal a678 | snr 007e | ber 000a17b0 | unc 00000cb8 | FE_HAS_LOCK
status 1f | signal a679 | snr 0085 | ber 000ad900 | unc 000009ea | FE_HAS_LOCK
status 1f | signal a6ee | snr 0082 | ber 000b0fa0 | unc 0000075c | FE_HAS_LOCK


> Do you have attenuator you can run tests yourself? It is very cheap and
> useful when coding that kind of signal statistics.

I haven't got an attenuator, but some very weak signals (see above), which 
should serve the same purpose.

> 
> Current API does not even define anymore units for BER and UCB, so those
> calculations are not necessary. Anyhow, you can add some calculations if
> you wish.
> 
> 
> regards
> Antti
cheers,

Hans-Frieder Vogt                       e-mail: hfvogt <at> gmx .dot. net
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Antti Palosaari April 6, 2012, 9:28 a.m. UTC | #3 
On 06.04.2012 11:34, Hans-Frieder Vogt wrote:
> Am Mittwoch, 4. April 2012 schrieb Antti Palosaari:
>> On 03.04.2012 23:59, Hans-Frieder Vogt wrote:
>>> af9033: implement read_ber and read_ucblocks functions.
>>>
>>> Signed-off-by: Hans-Frieder Vogt<hfvogt@gmx.net>
>>
>> For my quick test UCB counter seems to reset every query. That is
>> violation of API. See http://www.kernel.org/doc/htmldocs/media.html>
>
> Indeed, interesting.
> I quickly checked the behaviour with a dibcom based stick (dib7000p
> demodulator) and the uncorrected block number reduces there as well. It seems,
> other demodulator drivers ignore this detail as well. But that's not meant to
> be an excuse.....

Some demod drivers follows API better than others. There is also some 
problems with current API which makes implementations in practise a 
little bit different than API.
At least all demod drivers I have written should follow API on case of 
UCB counter reset. For a quick test here zl10353 demod seems to follow 
also. You can likely found out those who are following API just looking 
whether those stores UCB counter value to state and then increase it 
every query.

>> Current API does not even define anymore units for BER and UCB, so those
>> calculations are not necessary. Anyhow, you can add some calculations if
>> you wish.

I tried to calculate what you were doing there but did not understood. 
Looks like those values read from register are packets and then after 
all you finally converted those as a bits. 204 * 8 looks like that, 204 
is common MPEG TS packet size including parity (without parity 188), and 
8 is bits in one byte. But why abort_cnt, which is UCB I think, goes 
multiplied to 8 * 8 = 64 (in order to convert to bits?)?

And the resulting value is some amount of bits, unit is?

And current IT9035 and AF9013 seems to contain very simply UCB & BER 
implementation.

regards
Antti
diff mbox

Patch

diff -Nupr a/drivers/media/dvb/frontends/af9033.c b/drivers/media/dvb/frontends/af9033.c
--- a/drivers/media/dvb/frontends/af9033.c	2012-04-03 19:07:50.991367619 +0200
+++ b/drivers/media/dvb/frontends/af9033.c	2012-04-03 22:47:34.152121451 +0200
@@ -29,6 +29,10 @@  struct af9033_state {
 	u32 bandwidth_hz;
 	bool ts_mode_parallel;
 	bool ts_mode_serial;
+
+	u32 ber;
+	u32 ucb;
+	unsigned long last_stat_check;
 };
 
 /* write multiple registers */
@@ -645,16 +649,70 @@  err:
 	return ret;
 }
 
+static int af9033_update_ch_stat(struct af9033_state *state)
+{
+	int ret = 0;
+	u32 post_err_cnt, post_bit_cnt;
+	u16 abort_cnt;
+	u8 buf[7];
+
+	/* only update data every half second */
+	if (time_after(jiffies, state->last_stat_check + msecs_to_jiffies(500))) {
+		ret = af9033_rd_regs(state, 0x800032, buf, sizeof(buf));
+		if (ret < 0)
+			goto err;
+		abort_cnt = (buf[1] << 8) + buf[0];
+		post_err_cnt = (buf[4] << 16) + (buf[3] << 8) + buf[2];
+		post_bit_cnt = (buf[6] << 8) + buf[5];
+		if (post_bit_cnt == 0) {
+			abort_cnt = 1000;
+			post_err_cnt = 1;
+			post_bit_cnt = 2;
+		} else {
+			post_bit_cnt -= (u32)abort_cnt;
+			if (post_bit_cnt == 0) {
+				post_err_cnt = 1;
+				post_bit_cnt = 2;
+			} else {
+				post_err_cnt -= (u32)abort_cnt * 8 * 8;
+				post_bit_cnt *= 204 * 8;
+			}
+		}
+		state->ber = post_err_cnt * (0xffffffff / post_bit_cnt);
+		state->ucb = abort_cnt;
+		state->last_stat_check = jiffies;
+	}
+
+	return 0;
+err:
+	pr_debug("%s: failed=%d\n", __func__, ret);
+	return ret;
+}
+
 static int af9033_read_ber(struct dvb_frontend *fe, u32 *ber)
 {
-	*ber = 0;
+	struct af9033_state *state = fe->demodulator_priv;
+	int ret;
+
+	ret = af9033_update_ch_stat(state);
+	if (ret < 0)
+		return ret;
+
+	*ber = state->ber;
 
 	return 0;
 }
 
 static int af9033_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
 {
-	*ucblocks = 0;
+	struct af9033_state *state = fe->demodulator_priv;
+	int ret;
+
+	ret = af9033_update_ch_stat(state);
+	if (ret < 0)
+		return ret;
+
+	*ucblocks = state->ucb;
 
 	return 0;
 }