[v2,1/5] dvb-core: add a new tuner ops to dvb_frontend for APIv5

  From: Akihiro Tsukada <tskd08@gmail.com>

fe->ops.tuner_ops.get_rf_strength() reports its result in u16,
while in DVB APIv5 it should be reported in s64 and by 0.001dBm.

Signed-off-by: Akihiro Tsukada <tskd08@gmail.com>
---
 drivers/media/dvb-core/dvb_frontend.h | 2 ++
 1 file changed, 2 insertions(+)
  

Moikka
I have feeling DVBv5 API is aimed to transfer data via property cached. 
I haven't done much driver for DVBv5 statistics, but recently I 
implemented CNR (DVBv5 stats) to Si2168 driver and it just writes all 
the values directly to property cache. I expect RF strength (RSSI) is 
just similar.

Antti



On 08/27/2014 06:29 PM, tskd08@gmail.com wrote:
> From: Akihiro Tsukada <tskd08@gmail.com>
>
> fe->ops.tuner_ops.get_rf_strength() reports its result in u16,
> while in DVB APIv5 it should be reported in s64 and by 0.001dBm.
>
> Signed-off-by: Akihiro Tsukada <tskd08@gmail.com>
> ---
>   drivers/media/dvb-core/dvb_frontend.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index 816269e..f6222b5 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -222,6 +222,8 @@ struct dvb_tuner_ops {
>   #define TUNER_STATUS_STEREO 2
>   	int (*get_status)(struct dvb_frontend *fe, u32 *status);
>   	int (*get_rf_strength)(struct dvb_frontend *fe, u16 *strength);
> +	/** get signal strengh in 0.001dBm, in accordance with APIv5 */
> +	int (*get_rf_strength_dbm)(struct dvb_frontend *fe, s64 *strength);
>   	int (*get_afc)(struct dvb_frontend *fe, s32 *afc);
>
>   	/** These are provided separately from set_params in order to facilitate silicon
>
  

moikka,
thanks for the comment.

> I have feeling DVBv5 API is aimed to transfer data via property cached.
> I haven't done much driver for DVBv5 statistics, but recently I
> implemented CNR (DVBv5 stats) to Si2168 driver and it just writes all
> the values directly to property cache. I expect RF strength (RSSI) is
> just similar.

Currently, the demod of PT3 card (tc90522) gets RSSI data from
the connected tuner (mxl301rf) via tuner_ops.get_signal_strength_dbm()
and sets property cache in fe->ops.get_frontend() (which is called
before returning property cache value by dvb_frontend_ioctl_properties()). 
If the tuner driver should set property cache directly,
when is the right timing to do so?
In fe->ops.tuner_ops.get_status() ?
or in the old fe->ops.tuner_ops.get_signal_strength()?
or Should I change get_signal_strength_dbm(fe, s64 *) to
update_signal_strength(fe) and let the tuner driver set property cache there?

--
akihiro


--
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
  

moikka!

On 08/28/2014 12:07 PM, Akihiro TSUKADA wrote:
> moikka,
> thanks for the comment.
>
>> I have feeling DVBv5 API is aimed to transfer data via property cached.
>> I haven't done much driver for DVBv5 statistics, but recently I
>> implemented CNR (DVBv5 stats) to Si2168 driver and it just writes all
>> the values directly to property cache. I expect RF strength (RSSI) is
>> just similar.
>
> Currently, the demod of PT3 card (tc90522) gets RSSI data from
> the connected tuner (mxl301rf) via tuner_ops.get_signal_strength_dbm()
> and sets property cache in fe->ops.get_frontend() (which is called
> before returning property cache value by dvb_frontend_ioctl_properties()).
> If the tuner driver should set property cache directly,
> when is the right timing to do so?
> In fe->ops.tuner_ops.get_status() ?
> or in the old fe->ops.tuner_ops.get_signal_strength()?
> or Should I change get_signal_strength_dbm(fe, s64 *) to
> update_signal_strength(fe) and let the tuner driver set property cache there?

I think tuner driver should set c->strength as own. Look 
drivers/media/dvb-core/dvb_frontend.c
	/* Fill quality measures */
	case DTV_STAT_SIGNAL_STRENGTH:
		tvp->u.st = c->strength;
		break;

So user-space just get info what is set to struct 
dtv_frontend_properties. That is similarly than CNR and all the other 
statistics.

Start polling thread, which polls once per 2 sec or so, which reads RSSI 
and writes value to struct dtv_frontend_properties. That it is, in my 
understanding. Same for all those DVBv5 stats. Mauro knows better as he 
designed that functionality.

regards
Antti
  

moikka,

> Start polling thread, which polls once per 2 sec or so, which reads RSSI
> and writes value to struct dtv_frontend_properties. That it is, in my
> understanding. Same for all those DVBv5 stats. Mauro knows better as he
> designed that functionality.

I understand that RSSI property should be set directly in the tuner driver,
but I'm afraid that creating a kthread just for updating RSSI would be
overkill and complicate matters.

Would you give me an advice? >> Mauro

regards,
akihiro
--
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
  

Moro!

On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
> moikka,
>
>> Start polling thread, which polls once per 2 sec or so, which reads RSSI
>> and writes value to struct dtv_frontend_properties. That it is, in my
>> understanding. Same for all those DVBv5 stats. Mauro knows better as he
>> designed that functionality.
>
> I understand that RSSI property should be set directly in the tuner driver,
> but I'm afraid that creating a kthread just for updating RSSI would be
> overkill and complicate matters.
>
> Would you give me an advice? >> Mauro

Now I know that as I implement it. I added kthread and it works 
correctly, just I though it is aimed to work. In my case signal strength 
is reported by demod, not tuner, because there is some logic in firmware 
to calculate it.

Here is patches you would like to look as a example:

af9033: implement DVBv5 statistic for signal strength
https://patchwork.linuxtv.org/patch/25748/

af9033: implement DVBv5 statistic for CNR
https://patchwork.linuxtv.org/patch/25744/

af9033: implement DVBv5 stat block counters
https://patchwork.linuxtv.org/patch/25749/

af9033: implement DVBv5 post-Viterbi BER
https://patchwork.linuxtv.org/patch/25750/

regards
Antti
  

Em Sat, 06 Sep 2014 05:09:55 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> Moro!
> 
> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
> > moikka,
> >
> >> Start polling thread, which polls once per 2 sec or so, which reads RSSI
> >> and writes value to struct dtv_frontend_properties. That it is, in my
> >> understanding. Same for all those DVBv5 stats. Mauro knows better as he
> >> designed that functionality.
> >
> > I understand that RSSI property should be set directly in the tuner driver,
> > but I'm afraid that creating a kthread just for updating RSSI would be
> > overkill and complicate matters.
> >
> > Would you give me an advice? >> Mauro
> 
> Now I know that as I implement it. I added kthread and it works 
> correctly, just I though it is aimed to work. In my case signal strength 
> is reported by demod, not tuner, because there is some logic in firmware 
> to calculate it.
> 
> Here is patches you would like to look as a example:
> 
> af9033: implement DVBv5 statistic for signal strength
> https://patchwork.linuxtv.org/patch/25748/

Actually, you don't need to add a separate kthread to collect the stats.
The DVB frontend core already has a thread that calls the frontend status
on every 3 seconds (the time can actually be different, depending on
the value for fepriv->delay. So, if the device doesn't have any issues
on getting stats on this period, it could just hook the DVBv5 stats logic
at ops.read_status().

From the last time I reviewed the code, the PT3 driver seems to be using
this approach already at the demod.

Getting this value at the tuner makes it a little more trickier,
as you need to use some tuner callback to update the demod cache.
The .get_rf_strength ops is meant to get the signal strength from the
tuner, but it doesn't allow the tuner to return a value in dBm.

It shouldn't be the demod's task to convert a raw value on a tuner client
into dBm. 

After reading this thread and its comments, I think that the best would be
to not add a new callback.

Instead, change the implementation at the .get_rf_strength callback in
a way that it will return an integer from 0 to 65535 that would represent
a "percentage" level, where 100% means the maximum signal that the device
can measure.

Inside the tuner driver (mxl301rf), a call to .get_rf_strength will
directly update the FE stats cache to reflect the signal measurements in
dBm.

So, from the bridge driver, it will just call .get_rf_strength() without
using the returned results. If, latter, we use this tuner on some other
configuration (for example, on an hybrid analog/digital or SDR/digital
board), the V4L2 part will use the "percentage" level, as the V4L2 API
doesn't support returning values in dBm.

Regards,
Mauro

> af9033: implement DVBv5 statistic for CNR
> https://patchwork.linuxtv.org/patch/25744/
> 
> af9033: implement DVBv5 stat block counters
> https://patchwork.linuxtv.org/patch/25749/
> 
> af9033: implement DVBv5 post-Viterbi BER
> https://patchwork.linuxtv.org/patch/25750/
> 
> regards
> Antti
> 
--
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
  

Em Fri, 5 Sep 2014 23:51:05 -0300
Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:

> Em Sat, 06 Sep 2014 05:09:55 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
> 
> > Moro!
> > 
> > On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
> > > moikka,
> > >
> > >> Start polling thread, which polls once per 2 sec or so, which reads RSSI
> > >> and writes value to struct dtv_frontend_properties. That it is, in my
> > >> understanding. Same for all those DVBv5 stats. Mauro knows better as he
> > >> designed that functionality.
> > >
> > > I understand that RSSI property should be set directly in the tuner driver,
> > > but I'm afraid that creating a kthread just for updating RSSI would be
> > > overkill and complicate matters.
> > >
> > > Would you give me an advice? >> Mauro
> > 
> > Now I know that as I implement it. I added kthread and it works 
> > correctly, just I though it is aimed to work. In my case signal strength 
> > is reported by demod, not tuner, because there is some logic in firmware 
> > to calculate it.
> > 
> > Here is patches you would like to look as a example:
> > 
> > af9033: implement DVBv5 statistic for signal strength
> > https://patchwork.linuxtv.org/patch/25748/
> 
> Actually, you don't need to add a separate kthread to collect the stats.
> The DVB frontend core already has a thread that calls the frontend status
> on every 3 seconds (the time can actually be different, depending on
> the value for fepriv->delay. So, if the device doesn't have any issues
> on getting stats on this period, it could just hook the DVBv5 stats logic
> at ops.read_status().

In time: not implementing its own thread has one drawback: the driver needs
to check if the minimal time needed to get a new stats were already archived.

Please see the mt86a20s driver and check for some examples on how to
properly do that.

There, we do things like:

static int mb86a20s_read_signal_strength(struct dvb_frontend *fe)
{
	struct mb86a20s_state *state = fe->demodulator_priv;
	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
	int rc;
	unsigned rf_max, rf_min, rf;

	if (state->get_strength_time &&
	   (!time_after(jiffies, state->get_strength_time)))
		return c->strength.stat[0].uvalue;

To prevent the stats to be called too fast.

Regards,
Mauro
--
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
  

On 09/06/2014 05:54 AM, Mauro Carvalho Chehab wrote:
> Em Fri, 5 Sep 2014 23:51:05 -0300
> Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
>
>> Em Sat, 06 Sep 2014 05:09:55 +0300
>> Antti Palosaari <crope@iki.fi> escreveu:
>>
>>> Moro!
>>>
>>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>>> moikka,
>>>>
>>>>> Start polling thread, which polls once per 2 sec or so, which reads RSSI
>>>>> and writes value to struct dtv_frontend_properties. That it is, in my
>>>>> understanding. Same for all those DVBv5 stats. Mauro knows better as he
>>>>> designed that functionality.
>>>>
>>>> I understand that RSSI property should be set directly in the tuner driver,
>>>> but I'm afraid that creating a kthread just for updating RSSI would be
>>>> overkill and complicate matters.
>>>>
>>>> Would you give me an advice? >> Mauro
>>>
>>> Now I know that as I implement it. I added kthread and it works
>>> correctly, just I though it is aimed to work. In my case signal strength
>>> is reported by demod, not tuner, because there is some logic in firmware
>>> to calculate it.
>>>
>>> Here is patches you would like to look as a example:
>>>
>>> af9033: implement DVBv5 statistic for signal strength
>>> https://patchwork.linuxtv.org/patch/25748/
>>
>> Actually, you don't need to add a separate kthread to collect the stats.
>> The DVB frontend core already has a thread that calls the frontend status
>> on every 3 seconds (the time can actually be different, depending on
>> the value for fepriv->delay. So, if the device doesn't have any issues
>> on getting stats on this period, it could just hook the DVBv5 stats logic
>> at ops.read_status().
>
> In time: not implementing its own thread has one drawback: the driver needs
> to check if the minimal time needed to get a new stats were already archived.
>
> Please see the mt86a20s driver and check for some examples on how to
> properly do that.
>
> There, we do things like:
>
> static int mb86a20s_read_signal_strength(struct dvb_frontend *fe)
> {
> 	struct mb86a20s_state *state = fe->demodulator_priv;
> 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> 	int rc;
> 	unsigned rf_max, rf_min, rf;
>
> 	if (state->get_strength_time &&
> 	   (!time_after(jiffies, state->get_strength_time)))
> 		return c->strength.stat[0].uvalue;
>
> To prevent the stats to be called too fast.

... I simply don't understand why you want hook that RF strength call 
via demod? The frontend cache is shared between demod and tuner. We use 
it for tuner driver as well demod driver. Let the tuner driver make RSSI 
calculation independently without any unneeded relation to demod driver.

regards
Antti
  

Em Sat, 06 Sep 2014 06:10:01 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> 
> 
> On 09/06/2014 05:54 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 5 Sep 2014 23:51:05 -0300
> > Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
> >
> >> Em Sat, 06 Sep 2014 05:09:55 +0300
> >> Antti Palosaari <crope@iki.fi> escreveu:
> >>
> >>> Moro!
> >>>
> >>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
> >>>> moikka,
> >>>>
> >>>>> Start polling thread, which polls once per 2 sec or so, which reads RSSI
> >>>>> and writes value to struct dtv_frontend_properties. That it is, in my
> >>>>> understanding. Same for all those DVBv5 stats. Mauro knows better as he
> >>>>> designed that functionality.
> >>>>
> >>>> I understand that RSSI property should be set directly in the tuner driver,
> >>>> but I'm afraid that creating a kthread just for updating RSSI would be
> >>>> overkill and complicate matters.
> >>>>
> >>>> Would you give me an advice? >> Mauro
> >>>
> >>> Now I know that as I implement it. I added kthread and it works
> >>> correctly, just I though it is aimed to work. In my case signal strength
> >>> is reported by demod, not tuner, because there is some logic in firmware
> >>> to calculate it.
> >>>
> >>> Here is patches you would like to look as a example:
> >>>
> >>> af9033: implement DVBv5 statistic for signal strength
> >>> https://patchwork.linuxtv.org/patch/25748/
> >>
> >> Actually, you don't need to add a separate kthread to collect the stats.
> >> The DVB frontend core already has a thread that calls the frontend status
> >> on every 3 seconds (the time can actually be different, depending on
> >> the value for fepriv->delay. So, if the device doesn't have any issues
> >> on getting stats on this period, it could just hook the DVBv5 stats logic
> >> at ops.read_status().
> >
> > In time: not implementing its own thread has one drawback: the driver needs
> > to check if the minimal time needed to get a new stats were already archived.
> >
> > Please see the mt86a20s driver and check for some examples on how to
> > properly do that.
> >
> > There, we do things like:
> >
> > static int mb86a20s_read_signal_strength(struct dvb_frontend *fe)
> > {
> > 	struct mb86a20s_state *state = fe->demodulator_priv;
> > 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> > 	int rc;
> > 	unsigned rf_max, rf_min, rf;
> >
> > 	if (state->get_strength_time &&
> > 	   (!time_after(jiffies, state->get_strength_time)))
> > 		return c->strength.stat[0].uvalue;
> >
> > To prevent the stats to be called too fast.
> 
> ... I simply don't understand why you want hook that RF strength call 
> via demod? The frontend cache is shared between demod and tuner. We use 
> it for tuner driver as well demod driver. Let the tuner driver make RSSI 
> calculation independently without any unneeded relation to demod driver.

Well, adding kthreads has a cost, with is a way higher than just
calling a callback function.

Also, it makes a way more complicated to implement several tasks.

For example, devices with kthreads need to stop them during suspend,
and restart them at resume time, or otherwise suspend and/or resume
may not work.

Also, the power consumption increases with kthread, as the CPU need
to be periodically waken.

I'm not saying we shouldn't use kthreads at driver level, but
the best is to avoid when there are some other simpler ways of doing it.

Regards,
Mauro
--
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
  

On 09/06/2014 06:17 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 06 Sep 2014 06:10:01 +0300
> Antti Palosaari <crope@iki.fi> escreveu:

>> ... I simply don't understand why you want hook that RF strength call
>> via demod? The frontend cache is shared between demod and tuner. We use
>> it for tuner driver as well demod driver. Let the tuner driver make RSSI
>> calculation independently without any unneeded relation to demod driver.
>
> Well, adding kthreads has a cost, with is a way higher than just
> calling a callback function.
>
> Also, it makes a way more complicated to implement several tasks.
>
> For example, devices with kthreads need to stop them during suspend,
> and restart them at resume time, or otherwise suspend and/or resume
> may not work.
>
> Also, the power consumption increases with kthread, as the CPU need
> to be periodically waken.
>
> I'm not saying we shouldn't use kthreads at driver level, but
> the best is to avoid when there are some other simpler ways of doing it.

And small reality check, how much you think one kthreads, that polls 
once per 2 second or so causes, in a case when you are *already 
streaming* 20-80 Mbit/sec data stream :) I think CPU does not need wake 
up to execute one thread as there is a lot of other interrupts happening 
in that use case anyway.

We have a remote controller which is polled often as 100ms and it 
happens even when device is "sleeping".

regards
Antti
  

Moikka!,
thanks for the comments and advices.

I had been updating my code and during that, I also found that
updating property cache in tuner_ops.get_signal_strength() was
simple and (seemed to me) better than using a kthread,
so the current implementation (under testing) is just like
what Mauro proposed, but,

> In time: not implementing its own thread has one drawback: the driver needs
> to check if the minimal time needed to get a new stats were already archived.

since I don't know the minimal time and
whether there's a limit in the first place,
I'd like to let users take the responsibility.


>> ... I simply don't understand why you want hook that RF strength call 
>> via demod? The frontend cache is shared between demod and tuner. We use 
>> it for tuner drivr as well demod driver. Let the tuner driver make RSSI 
>> calculation independently without any unneeded relation to demod driver.

I think the main reason for the hook is because the dvb-core calls
ops.get_frontend() everytime before reading of any property cache,
so it is already a nice place to trigger property updates,
and reading any property involves demod (FE as a whole) anyway.

regards,
akihiro
--
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
  

On 09/06/2014 07:08 AM, Akihiro TSUKADA wrote:
> Moikka!,
> thanks for the comments and advices.
>
> I had been updating my code and during that, I also found that
> updating property cache in tuner_ops.get_signal_strength() was
> simple and (seemed to me) better than using a kthread,
> so the current implementation (under testing) is just like
> what Mauro proposed, but,
>
>> In time: not implementing its own thread has one drawback: the driver needs
>> to check if the minimal time needed to get a new stats were already archived.
>
> since I don't know the minimal time and
> whether there's a limit in the first place,
> I'd like to let users take the responsibility.

You could add some simple jiffie (some kind of kernel global time) which 
limits calls to some reasonable level.

if (jiffies > jiffies_previous + 1 sec)
   return 0;
else
   jiffies_previous = jiffies;

... continue

>>> ... I simply don't understand why you want hook that RF strength call
>>> via demod? The frontend cache is shared between demod and tuner. We use
>>> it for tuner drivr as well demod driver. Let the tuner driver make RSSI
>>> calculation independently without any unneeded relation to demod driver.
>
> I think the main reason for the hook is because the dvb-core calls
> ops.get_frontend() everytime before reading of any property cache,
> so it is already a nice place to trigger property updates,
> and reading any property involves demod (FE as a whole) anyway.

That must be changed 'resently'. IIRC originally get_frontend() was 
called by dvb-core only once, just after demod lock was gained. Also 
userspace could call it using some IOCTL (GET_FRONTEND?).

But if it is not called periodically by dvb-core, you could not use it 
for bit error measurements, as you will usually need to start 
measurement, then wait complete, read values and return.

Signal strength and SNR are typically provided by chip without any waiting.

regards
Antti
  

Em Sat, 06 Sep 2014 06:34:33 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> On 09/06/2014 06:17 AM, Mauro Carvalho Chehab wrote:
> > Em Sat, 06 Sep 2014 06:10:01 +0300
> > Antti Palosaari <crope@iki.fi> escreveu:
> 
> >> ... I simply don't understand why you want hook that RF strength call
> >> via demod? The frontend cache is shared between demod and tuner. We use
> >> it for tuner driver as well demod driver. Let the tuner driver make RSSI
> >> calculation independently without any unneeded relation to demod driver.
> >
> > Well, adding kthreads has a cost, with is a way higher than just
> > calling a callback function.
> >
> > Also, it makes a way more complicated to implement several tasks.
> >
> > For example, devices with kthreads need to stop them during suspend,
> > and restart them at resume time, or otherwise suspend and/or resume
> > may not work.
> >
> > Also, the power consumption increases with kthread, as the CPU need
> > to be periodically waken.
> >
> > I'm not saying we shouldn't use kthreads at driver level, but
> > the best is to avoid when there are some other simpler ways of doing it.
> 
> And small reality check, how much you think one kthreads, that polls 
> once per 2 second or so causes, in a case when you are *already 
> streaming* 20-80 Mbit/sec data stream :) I think CPU does not need wake 
> up to execute one thread as there is a lot of other interrupts happening 
> in that use case anyway.

You can't assume that all streams received by a tuner uses 20-80Mbps. 

It could be a sound broadcasting stream, for example, with uses a much
lower bandwidth.

> We have a remote controller which is polled often as 100ms and it 
> happens even when device is "sleeping".

And lots of drivers have a modprobe parameter to disable it, because
it causes too much power consumption.

Regards,
Mauro
--
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
  

Em Sat, 06 Sep 2014 13:36:14 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> On 09/06/2014 07:08 AM, Akihiro TSUKADA wrote:
> > Moikka!,
> > thanks for the comments and advices.
> >
> > I had been updating my code and during that, I also found that
> > updating property cache in tuner_ops.get_signal_strength() was
> > simple and (seemed to me) better than using a kthread,
> > so the current implementation (under testing) is just like
> > what Mauro proposed, but,
> >
> >> In time: not implementing its own thread has one drawback: the driver needs
> >> to check if the minimal time needed to get a new stats were already archived.
> >
> > since I don't know the minimal time and
> > whether there's a limit in the first place,
> > I'd like to let users take the responsibility.

For RSSI measurements, in general there's no minimal time, but for measures
like BER, PER, UCB, you'll need to wait for a while before the stats to be
updated. So, you'll need to at least track those.

> You could add some simple jiffie (some kind of kernel global time) which 
> limits calls to some reasonable level.
> 
> if (jiffies > jiffies_previous + 1 sec)
>    return 0;
> else
>    jiffies_previous = jiffies;

Don't use jiffies like that. jiffies can be overflowed and the update
will never occur. The right way is to use the macros time_after() and
time_before(), or, alternatively, time_is_after_jiffies() and
time_is_before_jiffies().

> 
> ... continue
> 
> >>> ... I simply don't understand why you want hook that RF strength call
> >>> via demod? The frontend cache is shared between demod and tuner. We use
> >>> it for tuner drivr as well demod driver. Let the tuner driver make RSSI
> >>> calculation independently without any unneeded relation to demod driver.
> >
> > I think the main reason for the hook is because the dvb-core calls
> > ops.get_frontend() everytime before reading of any property cache,
> > so it is already a nice place to trigger property updates,
> > and reading any property involves demod (FE as a whole) anyway.
> 
> That must be changed 'resently'. IIRC originally get_frontend() was 
> called by dvb-core only once, just after demod lock was gained. Also 
> userspace could call it using some IOCTL (GET_FRONTEND?).

No, get_frontend() is not automatically called by the dvb kthread after
lock has gained. Just double-checked.

> But if it is not called periodically by dvb-core, you could not use it 
> for bit error measurements, as you will usually need to start 
> measurement, then wait complete, read values and return.

Probably, the application you're using for tests are calling it
periodically.

What the core calls periodically for sure is read_status(). That's why
most drivers that provide DVBv5 stats hook the cache update there.

> Signal strength and SNR are typically provided by chip without any waiting.
> 
> regards
> Antti
> 
--
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
  

On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
> Em Sat, 06 Sep 2014 05:09:55 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> Moro!
>>
>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>> moikka,
>>>
>>>> Start polling thread, which polls once per 2 sec or so, which reads RSSI
>>>> and writes value to struct dtv_frontend_properties. That it is, in my
>>>> understanding. Same for all those DVBv5 stats. Mauro knows better as he
>>>> designed that functionality.
>>>
>>> I understand that RSSI property should be set directly in the tuner driver,
>>> but I'm afraid that creating a kthread just for updating RSSI would be
>>> overkill and complicate matters.
>>>
>>> Would you give me an advice? >> Mauro
>>
>> Now I know that as I implement it. I added kthread and it works
>> correctly, just I though it is aimed to work. In my case signal strength
>> is reported by demod, not tuner, because there is some logic in firmware
>> to calculate it.
>>
>> Here is patches you would like to look as a example:
>>
>> af9033: implement DVBv5 statistic for signal strength
>> https://patchwork.linuxtv.org/patch/25748/
>
> Actually, you don't need to add a separate kthread to collect the stats.
> The DVB frontend core already has a thread that calls the frontend status
> on every 3 seconds (the time can actually be different, depending on
> the value for fepriv->delay. So, if the device doesn't have any issues
> on getting stats on this period, it could just hook the DVBv5 stats logic
> at ops.read_status().
>

Hmm, fepriv->delay missed that one, 3 seconds is far too long for lmedm04.

It would be good to hook stats on to this thread.

Regards


Malcolm






--
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
  

On 06/09/14 17:24, Malcolm Priestley wrote:
> On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
>> Em Sat, 06 Sep 2014 05:09:55 +0300
>> Antti Palosaari <crope@iki.fi> escreveu:
>>
>>> Moro!
>>>
>>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>>> moikka,
>>>>
>>>>> Start polling thread, which polls once per 2 sec or so, which reads
>>>>> RSSI
>>>>> and writes value to struct dtv_frontend_properties. That it is, in my
>>>>> understanding. Same for all those DVBv5 stats. Mauro knows better
>>>>> as he
>>>>> designed that functionality.
>>>>
>>>> I understand that RSSI property should be set directly in the tuner
>>>> driver,
>>>> but I'm afraid that creating a kthread just for updating RSSI would be
>>>> overkill and complicate matters.
>>>>
>>>> Would you give me an advice? >> Mauro
>>>
>>> Now I know that as I implement it. I added kthread and it works
>>> correctly, just I though it is aimed to work. In my case signal strength
>>> is reported by demod, not tuner, because there is some logic in firmware
>>> to calculate it.
>>>
>>> Here is patches you would like to look as a example:
>>>
>>> af9033: implement DVBv5 statistic for signal strength
>>> https://patchwork.linuxtv.org/patch/25748/
>>
>> Actually, you don't need to add a separate kthread to collect the stats.
>> The DVB frontend core already has a thread that calls the frontend status
>> on every 3 seconds (the time can actually be different, depending on
>> the value for fepriv->delay. So, if the device doesn't have any issues
>> on getting stats on this period, it could just hook the DVBv5 stats logic
>> at ops.read_status().
>>
>
> Hmm, fepriv->delay missed that one, 3 seconds is far too long for lmedm04.
>
> It would be good to hook stats on to this thread.
optional that is.
--
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
  

On 06/09/14 17:24, Malcolm Priestley wrote:
> On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
>> Em Sat, 06 Sep 2014 05:09:55 +0300
>> Antti Palosaari <crope@iki.fi> escreveu:
>>
>>> Moro!
>>>
>>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>>> moikka,
>>>>
>>>>> Start polling thread, which polls once per 2 sec or so, which reads
>>>>> RSSI
>>>>> and writes value to struct dtv_frontend_properties. That it is, in my
>>>>> understanding. Same for all those DVBv5 stats. Mauro knows better
>>>>> as he
>>>>> designed that functionality.
>>>>
>>>> I understand that RSSI property should be set directly in the tuner
>>>> driver,
>>>> but I'm afraid that creating a kthread just for updating RSSI would be
>>>> overkill and complicate matters.
>>>>
>>>> Would you give me an advice? >> Mauro
>>>
>>> Now I know that as I implement it. I added kthread and it works
>>> correctly, just I though it is aimed to work. In my case signal strength
>>> is reported by demod, not tuner, because there is some logic in firmware
>>> to calculate it.
>>>
>>> Here is patches you would like to look as a example:
>>>
>>> af9033: implement DVBv5 statistic for signal strength
>>> https://patchwork.linuxtv.org/patch/25748/
>>
>> Actually, you don't need to add a separate kthread to collect the stats.
>> The DVB frontend core already has a thread that calls the frontend status
>> on every 3 seconds (the time can actually be different, depending on
>> the value for fepriv->delay. So, if the device doesn't have any issues
>> on getting stats on this period, it could just hook the DVBv5 stats logic
>> at ops.read_status().
>>
>
> Hmm, fepriv->delay missed that one, 3 seconds is far too long for lmedm04.

The only way change this is by using algo DVBFE_ALGO_HW using the 
frontend ops tune.

As most frontends are using dvb_frontend_swzigzag it could be 
implemented by patching the frontend ops tune code at the lock
return in this function or in dvb_frontend_swzigzag_update_delay.

Regards

Malcolm
--
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
  

Em Sat, 06 Sep 2014 22:37:21 +0100
Malcolm Priestley <tvboxspy@gmail.com> escreveu:

> On 06/09/14 17:24, Malcolm Priestley wrote:
> > On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
> >> Em Sat, 06 Sep 2014 05:09:55 +0300
> >> Antti Palosaari <crope@iki.fi> escreveu:
> >>
> >>> Moro!
> >>>
> >>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
> >>>> moikka,
> >>>>
> >>>>> Start polling thread, which polls once per 2 sec or so, which reads
> >>>>> RSSI
> >>>>> and writes value to struct dtv_frontend_properties. That it is, in my
> >>>>> understanding. Same for all those DVBv5 stats. Mauro knows better
> >>>>> as he
> >>>>> designed that functionality.
> >>>>
> >>>> I understand that RSSI property should be set directly in the tuner
> >>>> driver,
> >>>> but I'm afraid that creating a kthread just for updating RSSI would be
> >>>> overkill and complicate matters.
> >>>>
> >>>> Would you give me an advice? >> Mauro
> >>>
> >>> Now I know that as I implement it. I added kthread and it works
> >>> correctly, just I though it is aimed to work. In my case signal strength
> >>> is reported by demod, not tuner, because there is some logic in firmware
> >>> to calculate it.
> >>>
> >>> Here is patches you would like to look as a example:
> >>>
> >>> af9033: implement DVBv5 statistic for signal strength
> >>> https://patchwork.linuxtv.org/patch/25748/
> >>
> >> Actually, you don't need to add a separate kthread to collect the stats.
> >> The DVB frontend core already has a thread that calls the frontend status
> >> on every 3 seconds (the time can actually be different, depending on
> >> the value for fepriv->delay. So, if the device doesn't have any issues
> >> on getting stats on this period, it could just hook the DVBv5 stats logic
> >> at ops.read_status().
> >>
> >
> > Hmm, fepriv->delay missed that one, 3 seconds is far too long for lmedm04.
> 
> The only way change this is by using algo DVBFE_ALGO_HW using the 
> frontend ops tune.
> 
> As most frontends are using dvb_frontend_swzigzag it could be 
> implemented by patching the frontend ops tune code at the lock
> return in this function or in dvb_frontend_swzigzag_update_delay.

Well, if a different value is needed, it shouldn't be hard to add a
way to customize it, letting the demod to specify it, in the same way
as fe->ops.info.frequency_stepsize (and other similar demot properties)
are passed through the core.

Regards,
Mauro
--
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
  

On 09/07/2014 01:37 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 06 Sep 2014 22:37:21 +0100
> Malcolm Priestley <tvboxspy@gmail.com> escreveu:
>
>> On 06/09/14 17:24, Malcolm Priestley wrote:
>>> On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
>>>> Em Sat, 06 Sep 2014 05:09:55 +0300
>>>> Antti Palosaari <crope@iki.fi> escreveu:
>>>>
>>>>> Moro!
>>>>>
>>>>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>>>>> moikka,
>>>>>>
>>>>>>> Start polling thread, which polls once per 2 sec or so, which reads
>>>>>>> RSSI
>>>>>>> and writes value to struct dtv_frontend_properties. That it is, in my
>>>>>>> understanding. Same for all those DVBv5 stats. Mauro knows better
>>>>>>> as he
>>>>>>> designed that functionality.
>>>>>>
>>>>>> I understand that RSSI property should be set directly in the tuner
>>>>>> driver,
>>>>>> but I'm afraid that creating a kthread just for updating RSSI would be
>>>>>> overkill and complicate matters.
>>>>>>
>>>>>> Would you give me an advice? >> Mauro
>>>>>
>>>>> Now I know that as I implement it. I added kthread and it works
>>>>> correctly, just I though it is aimed to work. In my case signal strength
>>>>> is reported by demod, not tuner, because there is some logic in firmware
>>>>> to calculate it.
>>>>>
>>>>> Here is patches you would like to look as a example:
>>>>>
>>>>> af9033: implement DVBv5 statistic for signal strength
>>>>> https://patchwork.linuxtv.org/patch/25748/
>>>>
>>>> Actually, you don't need to add a separate kthread to collect the stats.
>>>> The DVB frontend core already has a thread that calls the frontend status
>>>> on every 3 seconds (the time can actually be different, depending on
>>>> the value for fepriv->delay. So, if the device doesn't have any issues
>>>> on getting stats on this period, it could just hook the DVBv5 stats logic
>>>> at ops.read_status().
>>>>
>>>
>>> Hmm, fepriv->delay missed that one, 3 seconds is far too long for lmedm04.
>>
>> The only way change this is by using algo DVBFE_ALGO_HW using the
>> frontend ops tune.
>>
>> As most frontends are using dvb_frontend_swzigzag it could be
>> implemented by patching the frontend ops tune code at the lock
>> return in this function or in dvb_frontend_swzigzag_update_delay.
>
> Well, if a different value is needed, it shouldn't be hard to add a
> way to customize it, letting the demod to specify it, in the same way
> as fe->ops.info.frequency_stepsize (and other similar demot properties)
> are passed through the core.

DVBFE_ALGO_SW, which is used normally, polls read_status rather rapidly. 
For statics problem is that it is too rapid, not that it is too slow. If 
you want re-use that as a timer for statistics, you could simply make 
own ratelimit very easily using kernel jiffies.

Not going to implement details, but here is skeleton, which is almost as 
many lines of code as actual implementation:

if (jiffies < jiffies_prev + 2 sec)
   return 0; // limit on
else
   jiffies_prev = jiffies;

... statistics polling code here.

regards
Antti
  

On 07/09/14 00:38, Antti Palosaari wrote:
>
>
> On 09/07/2014 01:37 AM, Mauro Carvalho Chehab wrote:
>> Em Sat, 06 Sep 2014 22:37:21 +0100
>> Malcolm Priestley <tvboxspy@gmail.com> escreveu:
>>
>>> On 06/09/14 17:24, Malcolm Priestley wrote:
>>>> On 06/09/14 03:51, Mauro Carvalho Chehab wrote:
>>>>> Em Sat, 06 Sep 2014 05:09:55 +0300
>>>>> Antti Palosaari <crope@iki.fi> escreveu:
>>>>>
>>>>>> Moro!
>>>>>>
>>>>>> On 08/29/2014 01:45 PM, Akihiro TSUKADA wrote:
>>>>>>> moikka,
>>>>>>>
>>>>>>>> Start polling thread, which polls once per 2 sec or so, which reads
>>>>>>>> RSSI
>>>>>>>> and writes value to struct dtv_frontend_properties. That it is,
>>>>>>>> in my
>>>>>>>> understanding. Same for all those DVBv5 stats. Mauro knows better
>>>>>>>> as he
>>>>>>>> designed that functionality.
>>>>>>>
>>>>>>> I understand that RSSI property should be set directly in the tuner
>>>>>>> driver,
>>>>>>> but I'm afraid that creating a kthread just for updating RSSI
>>>>>>> would be
>>>>>>> overkill and complicate matters.
>>>>>>>
>>>>>>> Would you give me an advice? >> Mauro
>>>>>>
>>>>>> Now I know that as I implement it. I added kthread and it works
>>>>>> correctly, just I though it is aimed to work. In my case signal
>>>>>> strength
>>>>>> is reported by demod, not tuner, because there is some logic in
>>>>>> firmware
>>>>>> to calculate it.
>>>>>>
>>>>>> Here is patches you would like to look as a example:
>>>>>>
>>>>>> af9033: implement DVBv5 statistic for signal strength
>>>>>> https://patchwork.linuxtv.org/patch/25748/
>>>>>
>>>>> Actually, you don't need to add a separate kthread to collect the
>>>>> stats.
>>>>> The DVB frontend core already has a thread that calls the frontend
>>>>> status
>>>>> on every 3 seconds (the time can actually be different, depending on
>>>>> the value for fepriv->delay. So, if the device doesn't have any issues
>>>>> on getting stats on this period, it could just hook the DVBv5 stats
>>>>> logic
>>>>> at ops.read_status().
>>>>>
>>>>
>>>> Hmm, fepriv->delay missed that one, 3 seconds is far too long for
>>>> lmedm04.
>>>
>>> The only way change this is by using algo DVBFE_ALGO_HW using the
>>> frontend ops tune.
>>>
>>> As most frontends are using dvb_frontend_swzigzag it could be
>>> implemented by patching the frontend ops tune code at the lock
>>> return in this function or in dvb_frontend_swzigzag_update_delay.
>>
>> Well, if a different value is needed, it shouldn't be hard to add a
>> way to customize it, letting the demod to specify it, in the same way
>> as fe->ops.info.frequency_stepsize (and other similar demot properties)
>> are passed through the core.
>
> DVBFE_ALGO_SW, which is used normally, polls read_status rather rapidly.
> For statics problem is that it is too rapid, not that it is too slow. If
> you want re-use that as a timer for statistics, you could simply make
> own ratelimit very easily using kernel jiffies.
The default starts off at 50msec and gradually rises to around 950msec.

There is another way to set fepriv->delay that is in 
ops->get_tune_settings  dvb_frontend_tune_settings->min_delay_ms.

The delay increases by around 900msec on top of the value set there.

In the case of lmedm04/m88rs2000 that why I am seeing 3 sec.

Regards


Malcolm




--
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