* [PATCH] media: cxd2841er: avoid too many status inquires
@ 2019-10-03 14:59 Mauro Carvalho Chehab
0 siblings, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2019-10-03 14:59 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Sergey Kozlov,
Abylay Ospan, stable
As reported at:
https://tvheadend.org/issues/5625
Retrieving certain status can cause discontinuity issues.
Prevent that by adding a timeout to each status logic.
Currently, the timeout is estimated based at the channel
bandwidth. There are other parameters that may also affect
the timeout, but that would require a per-delivery system
calculus and probably more information about how cxd2481er
works, with we don't have.
So, do a poor man's best guess.
Cc: stable@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
drivers/media/dvb-frontends/cxd2841er.c | 59 +++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
index 1b30cf570803..71428b0d2620 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -60,6 +60,13 @@ struct cxd2841er_priv {
enum cxd2841er_xtal xtal;
enum fe_caps caps;
u32 flags;
+
+ unsigned long ber_interval;
+ unsigned long ucb_interval;
+
+ unsigned long ber_time;
+ unsigned long ucb_time;
+ unsigned long snr_time;
};
static const struct cxd2841er_cnr_data s_cn_data[] = {
@@ -1941,12 +1948,20 @@ static void cxd2841er_read_ber(struct dvb_frontend *fe)
struct cxd2841er_priv *priv = fe->demodulator_priv;
u32 ret, bit_error = 0, bit_count = 0;
+ if (priv->ber_time &&
+ (!time_after(jiffies, priv->ber_time)))
+ return;
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
case SYS_DVBC_ANNEX_B:
case SYS_DVBC_ANNEX_C:
ret = cxd2841er_read_ber_c(priv, &bit_error, &bit_count);
+
+ if (!priv->ber_interval && p->symbol_rate)
+ priv->ber_interval = (10000000) / (p->symbol_rate / 1000);
+
break;
case SYS_ISDBT:
ret = cxd2841er_read_ber_i(priv, &bit_error, &bit_count);
@@ -1978,6 +1993,18 @@ static void cxd2841er_read_ber(struct dvb_frontend *fe)
p->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
p->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
}
+
+ /*
+ * If the per-delivery system doesn't specify, set a default timeout
+ * that will wait for ~12.5 seconds for 8MHz channels
+ */
+ if (!priv->ber_interval && p->bandwidth_hz)
+ priv->ber_interval = (100000000) / (p->bandwidth_hz / 1000);
+
+ if (priv->ber_interval < 1000)
+ priv->ber_interval = 1000;
+
+ priv->ber_time = jiffies + msecs_to_jiffies(priv->ber_interval);
}
static void cxd2841er_read_signal_strength(struct dvb_frontend *fe)
@@ -2037,6 +2064,13 @@ static void cxd2841er_read_snr(struct dvb_frontend *fe)
struct dtv_frontend_properties *p = &fe->dtv_property_cache;
struct cxd2841er_priv *priv = fe->demodulator_priv;
+ if (priv->snr_time &&
+ (!time_after(jiffies, priv->snr_time)))
+ return;
+
+ /* Preventing asking SNR more than once per second */
+ priv->snr_time = jiffies + msecs_to_jiffies(1000);
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
@@ -2081,12 +2115,18 @@ static void cxd2841er_read_ucblocks(struct dvb_frontend *fe)
struct cxd2841er_priv *priv = fe->demodulator_priv;
u32 ucblocks = 0;
+ if (priv->ucb_time &&
+ (!time_after(jiffies, priv->ucb_time)))
+ return;
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
case SYS_DVBC_ANNEX_B:
case SYS_DVBC_ANNEX_C:
cxd2841er_read_packet_errors_c(priv, &ucblocks);
+ if (!priv->ucb_interval && p->symbol_rate)
+ priv->ucb_interval = (100 * 204 * 1000 * 8) / p->symbol_rate;
break;
case SYS_DVBT:
cxd2841er_read_packet_errors_t(priv, &ucblocks);
@@ -2105,6 +2145,18 @@ static void cxd2841er_read_ucblocks(struct dvb_frontend *fe)
p->block_error.stat[0].scale = FE_SCALE_COUNTER;
p->block_error.stat[0].uvalue = ucblocks;
+
+ /*
+ * If the per-delivery system doesn't specify, set a default timeout
+ * that will wait 20-30 seconds
+ */
+ if (!priv->ucb_interval && p->bandwidth_hz)
+ priv->ucb_interval = (100 * 204 * 1000 * 8) / p->bandwidth_hz;
+
+ if (priv->ucb_interval < 1000)
+ priv->ucb_interval = 1000;
+
+ priv->ucb_time = jiffies + msecs_to_jiffies(priv->ucb_interval);
}
static int cxd2841er_dvbt2_set_profile(
@@ -3360,6 +3412,13 @@ static int cxd2841er_set_frontend_s(struct dvb_frontend *fe)
p->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
p->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+ /* Reset the wait for jiffies logic */
+ priv->ber_interval = 0;
+ priv->ucb_interval = 0;
+ priv->ber_time = 0;
+ priv->ucb_time = 0;
+ priv->snr_time = 0;
+
return ret;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] media: cxd2841er: avoid too many status inquires
@ 2019-10-06 12:48 Mauro Carvalho Chehab
0 siblings, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2019-10-06 12:48 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Sergey Kozlov,
Abylay Ospan
I2C ops are expensive, as the I2C bus typical speed is 100kbps.
Also, stats reading take some time, as it requires to retrieve a
certain number of packets to complete.
While we don't know the minimal for CXD2841er, trying to do it
too quickly is still a very bad idea.
So, add some sanity logic there, preventing to retrieve stats
faster than one second.
This shouldn't cause any issues with well behavior apps, as they
usually take stats on a polling rate slower than 1 second.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
drivers/media/dvb-frontends/cxd2841er.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
index 1b30cf570803..758c95bc3b11 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -60,6 +60,7 @@ struct cxd2841er_priv {
enum cxd2841er_xtal xtal;
enum fe_caps caps;
u32 flags;
+ unsigned long stats_time;
};
static const struct cxd2841er_cnr_data s_cn_data[] = {
@@ -3279,9 +3280,15 @@ static int cxd2841er_get_frontend(struct dvb_frontend *fe,
p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
if (status & FE_HAS_LOCK) {
+ if (priv->stats_time &&
+ (!time_after(jiffies, priv->stats_time)))
+ return 0;
+
+ /* Prevent retrieving stats faster than once per second */
+ priv->stats_time = jiffies + msecs_to_jiffies(1000);
+
cxd2841er_read_snr(fe);
cxd2841er_read_ucblocks(fe);
-
cxd2841er_read_ber(fe);
} else {
p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
@@ -3360,6 +3367,9 @@ static int cxd2841er_set_frontend_s(struct dvb_frontend *fe)
p->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
p->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+ /* Reset the wait for jiffies logic */
+ priv->stats_time = 0;
+
return ret;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] media: cxd2841er: avoid too many status inquires
@ 2019-10-03 14:37 Mauro Carvalho Chehab
0 siblings, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2019-10-03 14:37 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Sergey Kozlov,
Abylay Ospan
As reported at:
https://tvheadend.org/issues/5625
Retrieving certain status can cause discontinuity issues.
Prevent that by adding a timeout to each status logic.
Currently, the timeout is estimated based at the channel
bandwidth. There are other parameters that may also affect
the timeout, but that would require a per-delivery system
calculus and probably more information about how cxd2481er
works, with we don't have.
So, do a poor man's best guess.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
drivers/media/dvb-frontends/cxd2841er.c | 54 +++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
index 1b30cf570803..c75e63b9cfa7 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -60,6 +60,13 @@ struct cxd2841er_priv {
enum cxd2841er_xtal xtal;
enum fe_caps caps;
u32 flags;
+
+ unsigned long ber_interval;
+ unsigned long ucb_interval;
+
+ unsigned long ber_time;
+ unsigned long ucb_time;
+ unsigned long snr_time;
};
static const struct cxd2841er_cnr_data s_cn_data[] = {
@@ -1941,6 +1948,10 @@ static void cxd2841er_read_ber(struct dvb_frontend *fe)
struct cxd2841er_priv *priv = fe->demodulator_priv;
u32 ret, bit_error = 0, bit_count = 0;
+ if (priv->ber_time &&
+ (!time_after(jiffies, priv->ber_time)))
+ return;
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
@@ -1978,6 +1989,19 @@ static void cxd2841er_read_ber(struct dvb_frontend *fe)
p->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
p->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
}
+
+ /*
+ * If the per-delivery system doesn't specify, set a default timeout
+ * that will wait for 10^7 bits or 1 second
+ */
+ if (!priv->ber_interval && p->bandwidth_hz) {
+ priv->ber_interval = (10000000) / (p->bandwidth_hz / 1000);
+ }
+
+ if (priv->ber_interval < 1000)
+ priv->ber_interval = 1000;
+
+ priv->ber_time = jiffies + msecs_to_jiffies(priv->ber_interval);
}
static void cxd2841er_read_signal_strength(struct dvb_frontend *fe)
@@ -2037,6 +2061,13 @@ static void cxd2841er_read_snr(struct dvb_frontend *fe)
struct dtv_frontend_properties *p = &fe->dtv_property_cache;
struct cxd2841er_priv *priv = fe->demodulator_priv;
+ if (priv->snr_time &&
+ (!time_after(jiffies, priv->snr_time)))
+ return;
+
+ /* Preventing asking SNR more than once per second */
+ priv->snr_time = jiffies + msecs_to_jiffies(1000);
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
@@ -2081,6 +2112,10 @@ static void cxd2841er_read_ucblocks(struct dvb_frontend *fe)
struct cxd2841er_priv *priv = fe->demodulator_priv;
u32 ucblocks = 0;
+ if (priv->ucb_time &&
+ (!time_after(jiffies, priv->ucb_time)))
+ return;
+
dev_dbg(&priv->i2c->dev, "%s()\n", __func__);
switch (p->delivery_system) {
case SYS_DVBC_ANNEX_A:
@@ -2105,6 +2140,18 @@ static void cxd2841er_read_ucblocks(struct dvb_frontend *fe)
p->block_error.stat[0].scale = FE_SCALE_COUNTER;
p->block_error.stat[0].uvalue = ucblocks;
+
+ /*
+ * If the per-delivery system doesn't specify, set a default timeout
+ * that will wait for 100 packets or 1 second
+ */
+ if (!priv->ucb_interval && p->bandwidth_hz)
+ priv->ucb_interval = (100 * 204 * 1000 * 8) / p->bandwidth_hz;
+
+ if (priv->ucb_interval < 1000)
+ priv->ucb_interval = 1000;
+
+ priv->ucb_time = jiffies + msecs_to_jiffies(priv->ucb_interval);
}
static int cxd2841er_dvbt2_set_profile(
@@ -3360,6 +3407,13 @@ static int cxd2841er_set_frontend_s(struct dvb_frontend *fe)
p->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
p->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+ /* Reset the wait for jiffies logic */
+ priv->ber_interval = 0;
+ priv->ucb_interval = 0;
+ priv->ber_time = 0;
+ priv->ucb_time = 0;
+ priv->snr_time = 0;
+
return ret;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-06 12:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 14:59 [PATCH] media: cxd2841er: avoid too many status inquires Mauro Carvalho Chehab
-- strict thread matches above, loose matches on Subject: below --
2019-10-06 12:48 Mauro Carvalho Chehab
2019-10-03 14:37 Mauro Carvalho Chehab
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.