Details about DVB frontend API

Details about DVB frontend API

[Jean Delvare]

Hi folks,

I am looking for details regarding the DVB frontend API. I've read
linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
commands return, however it does not give any information about how the
returned values should be interpreted (or, seen from the other end, how
the frontend kernel drivers should encode these values.) If there
documentation available that would explain this?

For example, the signal strength. All I know so far is that this is a
16-bit value. But then what? Do greater values represent stronger
signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
returned value meaningful even when FE_HAS_SIGNAL is 0? When
FE_HAS_LOCK is 0? Is the scale linear, or do some values have
well-defined meanings, or is it arbitrary and each driver can have its
own scale? What are the typical use cases by user-space application for
this value?

That's the kind of details I'd like to know, not only for the signal
strength, but also for the SNR, BER and UB. Without this information,
it seems a little difficult to have consistent frontend drivers.

Thanks,
-- 
Jean Delvare

Re: Details about DVB frontend API

[Devin Heitmueller]

On Thu, Oct 22, 2009 at 3:13 PM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi folks,
>
> I am looking for details regarding the DVB frontend API. I've read
> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> commands return, however it does not give any information about how the
> returned values should be interpreted (or, seen from the other end, how
> the frontend kernel drivers should encode these values.) If there
> documentation available that would explain this?
>
> For example, the signal strength. All I know so far is that this is a
> 16-bit value. But then what? Do greater values represent stronger
> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> well-defined meanings, or is it arbitrary and each driver can have its
> own scale? What are the typical use cases by user-space application for
> this value?
>
> That's the kind of details I'd like to know, not only for the signal
> strength, but also for the SNR, BER and UB. Without this information,
> it seems a little difficult to have consistent frontend drivers.
>
> Thanks,
> --
> Jean Delvare

Hi Jean,

I try to raise this every six months or so.  Check the mailing list
archive for "SNR" in the subject line.

Yes, it's all screwed up and inconsistent across demods.  I took a
crack at fixing it a few months ago by proposing a standard (and even
offering to fix up all the demods to be consistent), and those efforts
were derailed by some individuals who wanted what I would consider a
"perfect interface" at the cost of something that worked for 98% of
the userbase (I'm not going to point any fingers).  And what did we
get as a result?  Nothing.

I could have had this problem solved six months ago for 98% of the
community, and instead we are right where we have been since the
beginning of the project.

/me stops thinking about this and goes and gets some coffee....

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[VDR User]

On Thu, Oct 22, 2009 at 12:27 PM, Devin Heitmueller
<dheitmueller@kernellabs.com> wrote:
> I could have had this problem solved six months ago for 98% of the
> community, and instead we are right where we have been since the
> beginning of the project.

This is really a shame too considering the enormous amount of people,
both users & devs, who would really like to see this happen.  You've
got to start somewhere and build/improve from there.  Simply sitting
back and doing nothing is of absolutely no benefit what-so-ever.
Maybe you should release your patch(es) and when/if enough people use
them, there will be some pressure to actually have progress.  Then
again, the same ugly monster that is linux dvb politics might prevent
that progress from ever happening.

Regards,
Derek

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Em Thu, 22 Oct 2009 21:13:30 +0200
Jean Delvare <khali@linux-fr.org> escreveu:

> Hi folks,
> 
> I am looking for details regarding the DVB frontend API. I've read
> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> commands return, however it does not give any information about how the
> returned values should be interpreted (or, seen from the other end, how
> the frontend kernel drivers should encode these values.) If there
> documentation available that would explain this?
> 
> For example, the signal strength. All I know so far is that this is a
> 16-bit value. But then what? Do greater values represent stronger
> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> well-defined meanings, or is it arbitrary and each driver can have its
> own scale? What are the typical use cases by user-space application for
> this value?
> 
> That's the kind of details I'd like to know, not only for the signal
> strength, but also for the SNR, BER and UB. Without this information,
> it seems a little difficult to have consistent frontend drivers.

We all want to know about that ;)

Seriously, the lack of a description of the meaning of the ranges for those
read values were already widely discussed at LMML and at the legacy dvb ML.
We should return this discussion again and decide what would be the better
way to describe those values.

My suggestion is that someone summarize the proposals we had and give some time
for people vote. After that, we just commit the most voted one, and commit the
patches for it. A pending question that should also be discussed is what we will
do with those dvb devices where we simply don't know what scale it uses. There
are several of them.

Btw, the new official documentation is the media infrastructure docbook that
can be found at the Kernel and at:
	http://linuxtv.org/downloads/v4l-dvb-apis

This covers both DVB and V4L API's.

Cheers,
Mauro

Re: Details about DVB frontend API

[Manu Abraham]

[-- Attachment #1: Type: text/plain, Size: 2364 bytes --]

On Fri, Oct 23, 2009 at 12:10 AM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> Em Thu, 22 Oct 2009 21:13:30 +0200
> Jean Delvare <khali@linux-fr.org> escreveu:
>
>> Hi folks,
>>
>> I am looking for details regarding the DVB frontend API. I've read
>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
>> commands return, however it does not give any information about how the
>> returned values should be interpreted (or, seen from the other end, how
>> the frontend kernel drivers should encode these values.) If there
>> documentation available that would explain this?
>>
>> For example, the signal strength. All I know so far is that this is a
>> 16-bit value. But then what? Do greater values represent stronger
>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>> well-defined meanings, or is it arbitrary and each driver can have its
>> own scale? What are the typical use cases by user-space application for
>> this value?
>>
>> That's the kind of details I'd like to know, not only for the signal
>> strength, but also for the SNR, BER and UB. Without this information,
>> it seems a little difficult to have consistent frontend drivers.
>
> We all want to know about that ;)
>
> Seriously, the lack of a description of the meaning of the ranges for those
> read values were already widely discussed at LMML and at the legacy dvb ML.
> We should return this discussion again and decide what would be the better
> way to describe those values.
>
> My suggestion is that someone summarize the proposals we had and give some time
> for people vote. After that, we just commit the most voted one, and commit the
> patches for it. A pending question that should also be discussed is what we will
> do with those dvb devices where we simply don't know what scale it uses. There
> are several of them.


Sometime back, (some time in April) i proposed a patch which addressed
the issue to scale "even those devices which have a weird scale or
none". Though based on an older tree of mine, here is the patch again.
If it looks good enough, i can port the patch to accomodate other
devices as well.


Regards,
Manu

[-- Attachment #2: statistics.patch --]
[-- Type: text/x-patch, Size: 8005 bytes --]

diff -r b5505a985f24 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Sat Feb 21 01:12:09 2009 +0400
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Tue Apr 07 18:19:22 2009 +0400
@@ -1004,8 +1004,8 @@
 	 */
 	/* Legacy	*/
 	if (fe->legacy) {
-		if ((fepriv->state & FESTATE_LOSTLOCK) && 
-		    (fe->ops.info.caps & FE_CAN_RECOVER) && 
+		if ((fepriv->state & FESTATE_LOSTLOCK) &&
+		    (fe->ops.info.caps & FE_CAN_RECOVER) &&
 		    (fepriv->max_drift == 0)) {
 
 			dvb_frontend_swzigzag_update_delay(fepriv, s & FE_HAS_LOCK);
@@ -1487,6 +1487,13 @@
 		break;
 	}
 
+	case FE_STATISTICS_CAPS: {
+		struct fecap_statistics *stats_cap = parg;
+		memcpy(stats_cap, &fe->ops.statistics_caps, sizeof (struct fecap_statistics));
+		err = 0;
+		break;
+	}
+
 	case FE_READ_STATUS: {
 		fe_status_t* status = parg;
 
@@ -1502,6 +1509,17 @@
 			err = fe->ops.read_status(fe, status);
 		break;
 	}
+
+	case FE_SIGNAL_LEVEL:
+		if (fe->ops.read_level)
+			err = fe->ops.read_level(fe, (__u32 *) parg);
+		break;
+
+	case FE_SIGNAL_STATS:
+		if (fe->ops.read_stats)
+			err = fe->ops.read_stats(fe, (struct fesignal_stat *) parg);
+		break;
+
 	case FE_READ_BER:
 		if (fe->ops.read_ber)
 			err = fe->ops.read_ber(fe, (__u32*) parg);
@@ -1645,7 +1663,7 @@
 			break;
 		}
 
-		memcpy(&fepriv->parameters, parg, sizeof (struct dvb_frontend_parameters));		
+		memcpy(&fepriv->parameters, parg, sizeof (struct dvb_frontend_parameters));
 		memset(&fetunesettings, 0, sizeof(struct dvb_frontend_tune_settings));
 		memcpy(&fetunesettings.parameters, parg, sizeof (struct dvb_frontend_parameters));
 
diff -r b5505a985f24 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Sat Feb 21 01:12:09 2009 +0400
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Tue Apr 07 18:19:22 2009 +0400
@@ -72,7 +72,7 @@
 	unsigned int audmode;
 	u64 std;
 };
-	
+
 enum dvbfe_modcod {
 	DVBFE_MODCOD_DUMMY_PLFRAME	= 0,
 	DVBFE_MODCOD_QPSK_1_4,
@@ -250,6 +250,7 @@
 struct dvb_frontend_ops {
 
 	struct dvb_frontend_info info;
+	struct fecap_statistics statistics_caps;
 
 	void (*release)(struct dvb_frontend* fe);
 	void (*release_sec)(struct dvb_frontend* fe);
@@ -304,6 +305,9 @@
 	enum dvbfe_search (*search)(struct dvb_frontend *fe, struct dvbfe_params *fe_params);
 	int (*track)(struct dvb_frontend *fe, struct dvbfe_params *fe_params, int *delay);
 
+	int (*read_level)(struct dvb_frontend *fe, u32 *signal); /* Raw AGC level */
+	int (*read_stats)(struct dvb_frontend *fe, struct fesignal_stat *stat);
+
 	struct dvb_tuner_ops tuner_ops;
 	struct analog_demod_ops analog_ops;
 };
diff -r b5505a985f24 linux/drivers/media/dvb/frontends/stb0899_drv.c
--- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Sat Feb 21 01:12:09 2009 +0400
+++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Tue Apr 07 18:19:22 2009 +0400
@@ -1225,6 +1225,29 @@
 	return 0;
 }
 
+static int stb0899_read_level(struct dvb_frontend *fe, u32 *signal)
+{
+	/* TODO! */
+	return 0;
+}
+
+static int stb0899_read_stats(struct dvb_frontend *fe, struct fesignal_stat *stats)
+{
+	u16 snr, strength;
+	u32 ber;
+
+	stb0899_read_snr(fe, &snr);
+	stb0899_read_signal_strength(fe, &strength);
+	stb0899_read_ber(fe, &ber);
+
+	stats->quality = snr;
+	stats->strength = strength;
+	stats->error = ber;
+	stats->unc = 0;
+
+	return 0;
+}
+
 static int stb0899_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage)
 {
 	struct stb0899_state *state = fe->demodulator_priv;
@@ -2039,6 +2062,26 @@
 		.type			= FE_QPSK, /* with old API */
 	},
 
+	.statistics_caps = {
+		.quality = {
+			.params         = FE_QUALITY_CNR,
+			.scale          = FE_SCALE_dB,
+			.exponent       = -4,
+		},
+
+		.strength = {
+			.params         = FE_SCALE_dB,
+			.exponent       = -4,
+		},
+
+		.error = {
+			.params         = FE_ERROR_BER,
+			.exponent       = 7,
+		},
+
+		.unc = FE_UNC_UNKNOWN,
+	},
+
 	.release			= stb0899_release,
 	.init				= stb0899_init,
 	.sleep				= stb0899_sleep,
@@ -2059,6 +2102,9 @@
 	.read_signal_strength		= stb0899_read_signal_strength,
 	.read_status			= stb0899_read_status,
 	.read_ber			= stb0899_read_ber,
+
+	.read_level                     = stb0899_read_level,
+	.read_stats                     = stb0899_read_stats,
 
 	.set_voltage			= stb0899_set_voltage,
 	.set_tone			= stb0899_set_tone,
diff -r b5505a985f24 linux/include/linux/dvb/frontend.h
--- a/linux/include/linux/dvb/frontend.h	Sat Feb 21 01:12:09 2009 +0400
+++ b/linux/include/linux/dvb/frontend.h	Tue Apr 07 18:19:22 2009 +0400
@@ -645,4 +645,118 @@
 };
 #define DVBFE_GET_EVENT			_IOR('o', 86, struct dvbfe_event)
 
+/* Frontend General Statistics
+ * General parameters
+ * FE_*_UNKNOWN:
+ *	Parameter is unknown to the frontend and doesn't really
+ *	make any sense for an application.
+ *
+ * FE_*_RELATIVE:
+ *	Parameter is relative on the basis of a ceil - floor basis
+ *	Format is based on empirical test to determine
+ *	the floor and ceiling values. This format is exactly the
+ *	same format as the existing statistics implementation.
+ */
+enum fecap_quality_params {
+	FE_QUALITY_UNKNOWN		= 0,
+	FE_QUALITY_SNR			= (1 <<  0),
+	FE_QUALITY_CNR			= (1 <<  1),
+	FE_QUALITY_EsNo			= (1 <<  2),
+	FE_QUALITY_EbNo			= (1 <<  3),
+	FE_QUALITY_RELATIVE		= (1 << 31),
+};
+
+enum fecap_scale_params {
+	FE_SCALE_UNKNOWN		= 0,
+	FE_SCALE_dB			= (1 <<  0),
+	FE_SCALE_RELATIVE		= (1 << 31),
+};
+
+enum fecap_error_params {
+	FE_ERROR_UNKNOWN		= 0,
+	FE_ERROR_BER			= (1 <<  0),
+	FE_ERROR_PER			= (1 <<  1),
+	FE_ERROR_RELATIVE		= (1 << 31),
+};
+
+enum fecap_unc_params {
+	FE_UNC_UNKNOWN			= 0,
+	FE_UNC_RELATIVE			= (1 << 31),
+};
+
+/* General parameters
+ * width:
+ * 	Specifies the width of the field
+ *
+ * exponent:
+ *	Specifies the multiplier for the respective field
+ *	MSB:1bit indicates the signdness of the parameter
+ */
+struct fecap_quality {
+	enum fecap_quality_params	params;
+	enum fecap_scale_params		scale;
+
+	__u32				width;
+	__s32				exponent;
+};
+
+struct fecap_strength {
+	enum fecap_scale_params		params;
+	__u32				width;
+	__s32				exponent;
+};
+
+struct fecap_error {
+	enum fecap_error_params		params;
+	__u32 				width;
+	__s32 				exponent;
+};
+
+struct fecap_statistics {
+	struct fecap_quality		quality;
+	struct fecap_strength		strength;
+	struct fecap_error		error;
+	enum fecap_unc_params		unc;
+};
+
+/* FE_STATISTICS_CAPS
+ * Userspace query for frontend signal statistics capabilities
+ */
+#define FE_STATISTICS_CAPS		_IOR('o', 84, struct fecap_statistics)
+
+
+/* FE_SIGNAL_LEVEL
+ * This system call provides a direct monitor of the signal, without
+ * passing through the relevant processing chains. In many cases, it
+ * is simply considered as direct AGC1 scaled values. This parameter
+ * can generally be used to position an antenna to while looking at
+ * a peak of this value. This parameter can be read back, even when
+ * a frontend LOCK has not been achieved. Some microntroller based
+ * demodulators do not provide a direct access to the AGC on the
+ * demodulator, hence this parameter will be Unsupported for such
+ * devices.
+ */
+#define FE_SIGNAL_LEVEL			_IOR('o', 85, __u32)
+
+
+struct fesignal_stat {
+	__u32 quality;
+	__u32 strength;
+	__u32 error;
+	__u32 unc;
+};
+
+/* FE_SIGNAL_STATS
+ * This system call provides a snapshot of all the receiver system
+ * at any given point of time. System signal statistics are always
+ * computed with respect to time and is best obtained the nearest
+ * to each of the individual parameters in a time domain.
+ * Signal statistics are assumed, "at any given instance of time".
+ * It is not possible to get a snapshot at the exact single instance
+ * and hence we look at the nearest instance, in the time domain.
+ * The statistics are described by the FE_STATISTICS_CAPS ioctl,
+ * ie. based on the device capabilities.
+ */
+#define FE_SIGNAL_STATS			_IOR('o', 86, struct fesignal_stat)
+
 #endif /*_DVBFRONTEND_H_*/

Re: Details about DVB frontend API

[Markus Rechberger]

On Thu, Oct 22, 2009 at 10:29 PM, Manu Abraham <abraham.manu@gmail.com> wrote:
> On Fri, Oct 23, 2009 at 12:10 AM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> Em Thu, 22 Oct 2009 21:13:30 +0200
>> Jean Delvare <khali@linux-fr.org> escreveu:
>>
>>> Hi folks,
>>>
>>> I am looking for details regarding the DVB frontend API. I've read
>>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
>>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
>>> commands return, however it does not give any information about how the
>>> returned values should be interpreted (or, seen from the other end, how
>>> the frontend kernel drivers should encode these values.) If there
>>> documentation available that would explain this?
>>>
>>> For example, the signal strength. All I know so far is that this is a
>>> 16-bit value. But then what? Do greater values represent stronger
>>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>>> well-defined meanings, or is it arbitrary and each driver can have its
>>> own scale? What are the typical use cases by user-space application for
>>> this value?
>>>
>>> That's the kind of details I'd like to know, not only for the signal
>>> strength, but also for the SNR, BER and UB. Without this information,
>>> it seems a little difficult to have consistent frontend drivers.
>>
>> We all want to know about that ;)
>>
>> Seriously, the lack of a description of the meaning of the ranges for those
>> read values were already widely discussed at LMML and at the legacy dvb ML.
>> We should return this discussion again and decide what would be the better
>> way to describe those values.
>>
>> My suggestion is that someone summarize the proposals we had and give some time
>> for people vote. After that, we just commit the most voted one, and commit the
>> patches for it. A pending question that should also be discussed is what we will
>> do with those dvb devices where we simply don't know what scale it uses. There
>> are several of them.
>
>
> Sometime back, (some time in April) i proposed a patch which addressed
> the issue to scale "even those devices which have a weird scale or
> none". Though based on an older tree of mine, here is the patch again.
> If it looks good enough, i can port the patch to accomodate other
> devices as well.
>

A few of our customers were requiring additional statistic
information, so we added follwing
command to our DVB API:

FE_GET_SIGQUALITY

struct media_sigquality {
   uint16_t MER;                  /**< in steps of 0.1 dB
          */
   uint32_t preViterbiBER ;       /**< in steps of 1/scaleFactorBER
          */
   uint32_t postViterbiBER ;      /**< in steps of 1/scaleFactorBER
          */
   uint32_t scaleFactorBER;       /**< scale factor for BER
          */
   uint32_t packetError ;         /**< number of packet errors
          */
   uint32_t postReedSolomonBER ;  /**< in steps of 1/scaleFactorBER
          */
   uint32_t indicator;            /**< indicative signal quality
low=0..100=high */
}

It's a one shot request.
it might be good to standardize this, although we can live with that
additional command too.

Best Regards,
Markus

Re: Details about DVB frontend API

[Manu Abraham]

On Fri, Oct 23, 2009 at 4:12 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Thu, Oct 22, 2009 at 10:29 PM, Manu Abraham <abraham.manu@gmail.com> wrote:
>> On Fri, Oct 23, 2009 at 12:10 AM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>>> Em Thu, 22 Oct 2009 21:13:30 +0200
>>> Jean Delvare <khali@linux-fr.org> escreveu:
>>>
>>>> Hi folks,
>>>>
>>>> I am looking for details regarding the DVB frontend API. I've read
>>>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
>>>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
>>>> commands return, however it does not give any information about how the
>>>> returned values should be interpreted (or, seen from the other end, how
>>>> the frontend kernel drivers should encode these values.) If there
>>>> documentation available that would explain this?
>>>>
>>>> For example, the signal strength. All I know so far is that this is a
>>>> 16-bit value. But then what? Do greater values represent stronger
>>>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>>>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>>>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>>>> well-defined meanings, or is it arbitrary and each driver can have its
>>>> own scale? What are the typical use cases by user-space application for
>>>> this value?
>>>>
>>>> That's the kind of details I'd like to know, not only for the signal
>>>> strength, but also for the SNR, BER and UB. Without this information,
>>>> it seems a little difficult to have consistent frontend drivers.
>>>
>>> We all want to know about that ;)
>>>
>>> Seriously, the lack of a description of the meaning of the ranges for those
>>> read values were already widely discussed at LMML and at the legacy dvb ML.
>>> We should return this discussion again and decide what would be the better
>>> way to describe those values.
>>>
>>> My suggestion is that someone summarize the proposals we had and give some time
>>> for people vote. After that, we just commit the most voted one, and commit the
>>> patches for it. A pending question that should also be discussed is what we will
>>> do with those dvb devices where we simply don't know what scale it uses. There
>>> are several of them.
>>
>>
>> Sometime back, (some time in April) i proposed a patch which addressed
>> the issue to scale "even those devices which have a weird scale or
>> none". Though based on an older tree of mine, here is the patch again.
>> If it looks good enough, i can port the patch to accomodate other
>> devices as well.
>>
>
> A few of our customers were requiring additional statistic
> information, so we added follwing
> command to our DVB API:
>
> FE_GET_SIGQUALITY
>
> struct media_sigquality {
>   uint16_t MER;                  /**< in steps of 0.1 dB
>          */
>   uint32_t preViterbiBER ;       /**< in steps of 1/scaleFactorBER
>          */
>   uint32_t postViterbiBER ;      /**< in steps of 1/scaleFactorBER
>          */
>   uint32_t scaleFactorBER;       /**< scale factor for BER
>          */
>   uint32_t packetError ;         /**< number of packet errors
>          */
>   uint32_t postReedSolomonBER ;  /**< in steps of 1/scaleFactorBER
>          */
>   uint32_t indicator;            /**< indicative signal quality
> low=0..100=high */
> }
>
> It's a one shot request.
> it might be good to standardize this, although we can live with that
> additional command too.


Based on the above data structure, since UNC is calculated from pre -
post viterbi, any good reason why you need it explicit ? Which
otherwise is redundant. Still when simplified, it looks exactly the
same as the version 3.2 frontend statistic information alone except
for the BER scale, isn't it ?

Regards,
Manu

Re: Details about DVB frontend API

[Mike Booth]

On Friday 23 October 2009 06:13:30 Jean Delvare wrote:
> Hi folks,
>
> I am looking for details regarding the DVB frontend API. I've read
> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> commands return, however it does not give any information about how the
> returned values should be interpreted (or, seen from the other end, how
> the frontend kernel drivers should encode these values.) If there
> documentation available that would explain this?
>
> For example, the signal strength. All I know so far is that this is a
> 16-bit value. But then what? Do greater values represent stronger
> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> well-defined meanings, or is it arbitrary and each driver can have its
> own scale? What are the typical use cases by user-space application for
> this value?
>
> That's the kind of details I'd like to know, not only for the signal
> strength, but also for the SNR, BER and UB. Without this information,
> it seems a little difficult to have consistent frontend drivers.
>
> Thanks,

I have tried on two occasions to engage the the author of my particular driver 
as to how to implement a patch and use femon with no response.

Its good that there is some movement at last which might get a result.

I've said before I don't really care too much about spot on accuracy
but rather a scale that increases as you get closer to a lock. I can imagine 
there are loads of users out there who rely on the output of things like 
femon and vdr-rotor to tune their equipment and with S2 cards like both of 
mine they are knackered so to speak.


Mike

Re: Details about DVB frontend API

[Jean Delvare]

Hi Devin,

On Thu, 22 Oct 2009 15:27:20 -0400, Devin Heitmueller wrote:
> On Thu, Oct 22, 2009 at 3:13 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > Hi folks,
> >
> > I am looking for details regarding the DVB frontend API. I've read
> > linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> > FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> > commands return, however it does not give any information about how the
> > returned values should be interpreted (or, seen from the other end, how
> > the frontend kernel drivers should encode these values.) If there
> > documentation available that would explain this?
> >
> > For example, the signal strength. All I know so far is that this is a
> > 16-bit value. But then what? Do greater values represent stronger
> > signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> > returned value meaningful even when FE_HAS_SIGNAL is 0? When
> > FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> > well-defined meanings, or is it arbitrary and each driver can have its
> > own scale? What are the typical use cases by user-space application for
> > this value?
> >
> > That's the kind of details I'd like to know, not only for the signal
> > strength, but also for the SNR, BER and UB. Without this information,
> > it seems a little difficult to have consistent frontend drivers.
> >
> > Thanks,
> > --
> > Jean Delvare
> 
> I try to raise this every six months or so.  Check the mailing list
> archive for "SNR" in the subject line.
>
> Yes, it's all screwed up and inconsistent across demods.  I took a
> crack at fixing it a few months ago by proposing a standard (and even
> offering to fix up all the demods to be consistent), and those efforts
> were derailed by some individuals who wanted what I would consider a
> "perfect interface" at the cost of something that worked for 98% of
> the userbase (I'm not going to point any fingers).  And what did we
> get as a result?  Nothing.
> 
> I could have had this problem solved six months ago for 98% of the
> community, and instead we are right where we have been since the
> beginning of the project.
> 
> /me stops thinking about this and goes and gets some coffee....

Sorry, I didn't mean to restart a war. I really expected a standard to
exist but be possibly undocumented. I did not expect all these values
to not be standardized at all :( Thanks to all who answered anyway. I
sincerely hope that we can improve the situation in a near future.

I am not too familiar with DVB driver development, but I believe that
even loose standards would be much better than no standards at all. And
strict standards might not be possible to implement properly anyway, in
case we do not have detailed specifications of the hardware (which is
relatively frequent as I understand it.)

Taking the example of the signal strength, I think I would be fine with
the following description: 16-bit value, 0 means weakest signal
(possibly no signal at all), 0xffff means strongest signal. I suspect
user-space applications would already be able to do something about
this.

Thanks,
-- 
Jean Delvare

Re: Details about DVB frontend API

[Jean Delvare]

On Fri, 23 Oct 2009 05:10:25 +0900, Mauro Carvalho Chehab wrote:
> Em Thu, 22 Oct 2009 21:13:30 +0200
> Jean Delvare <khali@linux-fr.org> escreveu:
> 
> > Hi folks,
> > 
> > I am looking for details regarding the DVB frontend API. I've read
> > linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> > FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> > commands return, however it does not give any information about how the
> > returned values should be interpreted (or, seen from the other end, how
> > the frontend kernel drivers should encode these values.) If there
> > documentation available that would explain this?
> > 
> > For example, the signal strength. All I know so far is that this is a
> > 16-bit value. But then what? Do greater values represent stronger
> > signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> > returned value meaningful even when FE_HAS_SIGNAL is 0? When
> > FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> > well-defined meanings, or is it arbitrary and each driver can have its
> > own scale? What are the typical use cases by user-space application for
> > this value?
> > 
> > That's the kind of details I'd like to know, not only for the signal
> > strength, but also for the SNR, BER and UB. Without this information,
> > it seems a little difficult to have consistent frontend drivers.
> 
> We all want to know about that ;)
> 
> Seriously, the lack of a description of the meaning of the ranges for those
> read values were already widely discussed at LMML and at the legacy dvb ML.
> We should return this discussion again and decide what would be the better
> way to describe those values.
> 
> My suggestion is that someone summarize the proposals we had and give some time
> for people vote. After that, we just commit the most voted one, and commit the
> patches for it. A pending question that should also be discussed is what we will
> do with those dvb devices where we simply don't know what scale it uses. There
> are several of them.
> 
> Btw, the new official documentation is the media infrastructure docbook that
> can be found at the Kernel and at:
> 	http://linuxtv.org/downloads/v4l-dvb-apis
> 
> This covers both DVB and V4L API's.

Ah, thank you. So I was reading old documentation. Too bad that
googling for "linux dvb api" points to the old documents. Maybe the new
document needs some more keywords so that search engines index it
properly?

Also, on http://linuxtv.org/docs.php, I can see links to
http://linuxtv.org/downloads/linux-dvb-api-1.0.0.pdf for DVB and
http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html
for V4L... but no link to http://linuxtv.org/downloads/v4l-dvb-apis .
Shouldn't the links be updated?

-- 
Jean Delvare

Re: Details about DVB frontend API

[Jean Delvare]

On Thu, 22 Oct 2009 21:13:30 +0200, Jean Delvare wrote:
> For example, the signal strength. All I know so far is that this is a
> 16-bit value. But then what? Do greater values represent stronger
> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> well-defined meanings, or is it arbitrary and each driver can have its
> own scale? What are the typical use cases by user-space application for
> this value?

To close the chapter on signal strength... I understand now that we
don't have strict rules about the exact values. But do we have at least
a common agreement that greater values mean stronger signal? I am
asking because the DVB-T adapter model I have here behaves very
strangely in this respect. I get values of:
* 0xffff when there's no signal at all
* 0x2828 to 0x2e2e when signal is OK
* greater values as signal weakens (I have an amplified antenna with
  manual gain control) up to 0x7272

I would have expected it the other way around: 0x0000 for no signal and
greater values as signal strengthens. I think the frontend driver
(cx22702) needs to be fixed.

-- 
Jean Delvare

Re: Details about DVB frontend API

[Michael Krufky]

On Fri, Oct 23, 2009 at 11:47 AM, Jean Delvare <khali@linux-fr.org> wrote:
> On Thu, 22 Oct 2009 21:13:30 +0200, Jean Delvare wrote:
>> For example, the signal strength. All I know so far is that this is a
>> 16-bit value. But then what? Do greater values represent stronger
>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>> well-defined meanings, or is it arbitrary and each driver can have its
>> own scale? What are the typical use cases by user-space application for
>> this value?
>
> To close the chapter on signal strength... I understand now that we
> don't have strict rules about the exact values. But do we have at least
> a common agreement that greater values mean stronger signal? I am
> asking because the DVB-T adapter model I have here behaves very
> strangely in this respect. I get values of:
> * 0xffff when there's no signal at all
> * 0x2828 to 0x2e2e when signal is OK
> * greater values as signal weakens (I have an amplified antenna with
>  manual gain control) up to 0x7272
>
> I would have expected it the other way around: 0x0000 for no signal and
> greater values as signal strengthens. I think the frontend driver
> (cx22702) needs to be fixed.
>
> --
> Jean Delvare
> --
> 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
>



I have a solution for this across the entire DVB subsystem, but I
haven't had time to write up a formal explanation.

I will follow up with better info when I have time.

Regards,

Mike Krufky

Re: Details about DVB frontend API

[VDR User]

On Thu, Oct 22, 2009 at 1:29 PM, Manu Abraham <abraham.manu@gmail.com> wrote:
> Sometime back, (some time in April) i proposed a patch which addressed
> the issue to scale "even those devices which have a weird scale or
> none". Though based on an older tree of mine, here is the patch again.
> If it looks good enough, i can port the patch to accomodate other
> devices as well.

Thanks for posting your patch.  How about people get some discussion
going about the pros/cons instead of ignoring it?  The best method for
the most devices needs to be the deciding factor here, not
accepting/disregarding based on who "your" friends are.  For once it
would be nice if the childish politics could get throw out and what's
best for v4l be the highest priority.

And a thanks to anyone else that would like to submit a recommendation
on how to deal with this.  Hopefully there will be a few "solid"
proposals to consider.

Regards.

Re: Details about DVB frontend API

[Markus Rechberger]

On Fri, Oct 23, 2009 at 9:02 PM, VDR User <user.vdr@gmail.com> wrote:
> On Thu, Oct 22, 2009 at 1:29 PM, Manu Abraham <abraham.manu@gmail.com> wrote:
>> Sometime back, (some time in April) i proposed a patch which addressed
>> the issue to scale "even those devices which have a weird scale or
>> none". Though based on an older tree of mine, here is the patch again.
>> If it looks good enough, i can port the patch to accomodate other
>> devices as well.
>
> Thanks for posting your patch.  How about people get some discussion
> going about the pros/cons instead of ignoring it?  The best method for
> the most devices needs to be the deciding factor here, not
> accepting/disregarding based on who "your" friends are.  For once it
> would be nice if the childish politics could get throw out and what's
> best for v4l be the highest priority.
>
> And a thanks to anyone else that would like to submit a recommendation
> on how to deal with this.  Hopefully there will be a few "solid"
> proposals to consider.
>

it would be good to have a table what statistics can be returned, obviously
the API implementation doesn't matter so much.

Manu, Mike can you start a wiki site on linuxtv.org addressing this topic?
Please work out a proper API definition first, afterwards think about
the implementation.

The correct implementation in the enduser applications is by far more important.

Best Regards,
Markus

Re: Details about DVB frontend API

[David T. L. Wong]

Jean Delvare wrote:
> Hi folks,
> 
> I am looking for details regarding the DVB frontend API. I've read
> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> commands return, however it does not give any information about how the
> returned values should be interpreted (or, seen from the other end, how
> the frontend kernel drivers should encode these values.) If there
> documentation available that would explain this?
> 
> For example, the signal strength. All I know so far is that this is a
> 16-bit value. But then what? Do greater values represent stronger
> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> well-defined meanings, or is it arbitrary and each driver can have its
> own scale? What are the typical use cases by user-space application for
> this value?
> 
> That's the kind of details I'd like to know, not only for the signal
> strength, but also for the SNR, BER and UB. Without this information,
> it seems a little difficult to have consistent frontend drivers.
> 
> Thanks,

Hi all,

   I am a bit late in this discussion.

   I just want to raise out a problem of the current architecture of FE 
+ tuner.

   Indeed, the actual "Signal Strength" can only be get from tuner. 
Tuner has amplifier internally and AGC. So demod can never know the 
accurate signal strength. Demod only roughly knows signal-to-noise ratio.

   Correct me if I am wrong that I found FE == Demod in current code.
Thus, asking FE to report the signal strength is not appropriate.

   To achieve reporting actual signal strength, in commercial 
proprietary code, it is a combination of readings from tuner + demod. 
Which in turn,
should sit in card/dongle specific code.

Regards,
David T.L. Wong

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Manu Abraham escreveu:
> On Fri, Oct 23, 2009 at 12:10 AM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> Em Thu, 22 Oct 2009 21:13:30 +0200
>> Jean Delvare <khali@linux-fr.org> escreveu:
>>
>>> Hi folks,
>>>
>>> I am looking for details regarding the DVB frontend API. I've read
>>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
>>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
>>> commands return, however it does not give any information about how the
>>> returned values should be interpreted (or, seen from the other end, how
>>> the frontend kernel drivers should encode these values.) If there
>>> documentation available that would explain this?
>>>
>>> For example, the signal strength. All I know so far is that this is a
>>> 16-bit value. But then what? Do greater values represent stronger
>>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>>> well-defined meanings, or is it arbitrary and each driver can have its
>>> own scale? What are the typical use cases by user-space application for
>>> this value?
>>>
>>> That's the kind of details I'd like to know, not only for the signal
>>> strength, but also for the SNR, BER and UB. Without this information,
>>> it seems a little difficult to have consistent frontend drivers.
>> We all want to know about that ;)
>>
>> Seriously, the lack of a description of the meaning of the ranges for those
>> read values were already widely discussed at LMML and at the legacy dvb ML.
>> We should return this discussion again and decide what would be the better
>> way to describe those values.
>>
>> My suggestion is that someone summarize the proposals we had and give some time
>> for people vote. After that, we just commit the most voted one, and commit the
>> patches for it. A pending question that should also be discussed is what we will
>> do with those dvb devices where we simply don't know what scale it uses. There
>> are several of them.
> 
> 
> Sometime back, (some time in April) i proposed a patch which addressed
> the issue to scale "even those devices which have a weird scale or
> none". Though based on an older tree of mine, here is the patch again.
> If it looks good enough, i can port the patch to accomodate other
> devices as well.
> 
> 
> Regards,
> Manu
> 

Manu,

Sorry for not answering earlier. Due to my travels, I had a very big backlog here
to handle.

I prefer a solution like you've proposed of creating a new set of API calls for
it, instead of re-defining the current calls.

Yet, I have a few comments:

diff -r b5505a985f24 linux/include/linux/dvb/frontend.h
--- a/linux/include/linux/dvb/frontend.h	Sat Feb 21 01:12:09 2009 +0400
+++ b/linux/include/linux/dvb/frontend.h	Tue Apr 07 18:19:22 2009 +0400
@@ -645,4 +645,118 @@
 };
 #define DVBFE_GET_EVENT			_IOR('o', 86, struct dvbfe_event)
 
+/* Frontend General Statistics
+ * General parameters
+ * FE_*_UNKNOWN:
+ *	Parameter is unknown to the frontend and doesn't really
+ *	make any sense for an application.
+ *
+ * FE_*_RELATIVE:
+ *	Parameter is relative on the basis of a ceil - floor basis
+ *	Format is based on empirical test to determine
+ *	the floor and ceiling values. This format is exactly the
+ *	same format as the existing statistics implementation.
+ */
+enum fecap_quality_params {
+	FE_QUALITY_UNKNOWN		= 0,
+	FE_QUALITY_SNR			= (1 <<  0),
+	FE_QUALITY_CNR			= (1 <<  1),
+	FE_QUALITY_EsNo			= (1 <<  2),
+	FE_QUALITY_EbNo			= (1 <<  3),
+	FE_QUALITY_RELATIVE		= (1 << 31),
+};
+
+enum fecap_scale_params {
+	FE_SCALE_UNKNOWN		= 0,
+	FE_SCALE_dB			= (1 <<  0),
+	FE_SCALE_RELATIVE		= (1 << 31),
+};
+
+enum fecap_error_params {
+	FE_ERROR_UNKNOWN		= 0,
+	FE_ERROR_BER			= (1 <<  0),
+	FE_ERROR_PER			= (1 <<  1),
+	FE_ERROR_RELATIVE		= (1 << 31),
+};
+
+enum fecap_unc_params {
+	FE_UNC_UNKNOWN			= 0,
+	FE_UNC_RELATIVE			= (1 << 31),
+};
+
+/* General parameters
+ * width:
+ * 	Specifies the width of the field
+ *
+ * exponent:
+ *	Specifies the multiplier for the respective field
+ *	MSB:1bit indicates the signdness of the parameter
+ */
+struct fecap_quality {
+	enum fecap_quality_params	params;
+	enum fecap_scale_params		scale;
+
+	__u32				width;
+	__s32				exponent;
+};
+
+struct fecap_strength {
+	enum fecap_scale_params		params;
+	__u32				width;
+	__s32				exponent;
+};
+
+struct fecap_error {
+	enum fecap_error_params		params;
+	__u32 				width;
+	__s32 				exponent;
+};
+
+struct fecap_statistics {
+	struct fecap_quality		quality;
+	struct fecap_strength		strength;
+	struct fecap_error		error;
+	enum fecap_unc_params		unc;
+};

I don't like the idea of creating structs grouping those parameters. While for
certain devices this may mean a more direct approach, for others, this may
not make sense, due to the way their API's were implemented (for example,
devices with firmware may need several calls to get all those info).

+#define FE_STATISTICS_CAPS		_IOR('o', 84, struct fecap_statistics)
+#define FE_SIGNAL_LEVEL			_IOR('o', 85, __u32)
+#define FE_SIGNAL_STATS			_IOR('o', 86, struct fesignal_stat)

Instead of defining 3 new ioctls, the better is to use the DVBS2API, maybe extending it to allow
receiving more than one parameter at the same time (with an approach similar to what we did
with V4L extended CTRLs API).

We are already redefining some existing ioctls there, so it would be clearer for the
userspace developers what would be the new way to retrieve frontend stats, as we can
simply say that DVBS2API features superseeds the equivalent DVB v3 ioctls.

Comments?

Cheers,
Mauro.

Re: Details about DVB frontend API

[Devin Heitmueller]

On Tue, Nov 17, 2009 at 2:46 PM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> I don't like the idea of creating structs grouping those parameters. While for
> certain devices this may mean a more direct approach, for others, this may
> not make sense, due to the way their API's were implemented (for example,
> devices with firmware may need several calls to get all those info).

There is some value in providing grouping the results in a single
request - in many cases the data is based off of the same internal
registers, and Manu's proposed approach allows for a more "atomic"
response.  The fact that we currently do the status, SNR, strength,
and UNC/BER in separate calls is one reason that people sometimes see
inconsistent results in the output of tools like zap.  As an example,
they can see lines in the zap output where the lock is lost but SNR
appears fine.

In the case where the driver servicing the query needs to do three
calls, it could be slightly more expensive, but only if we believe
that it is commonplace to ask for a subset of the stats.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[Jean Delvare]

On Tue, 17 Nov 2009 14:55:51 -0500, Devin Heitmueller wrote:
> On Tue, Nov 17, 2009 at 2:46 PM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
> > I don't like the idea of creating structs grouping those parameters. While for
> > certain devices this may mean a more direct approach, for others, this may
> > not make sense, due to the way their API's were implemented (for example,
> > devices with firmware may need several calls to get all those info).
> 
> There is some value in providing grouping the results in a single
> request - in many cases the data is based off of the same internal
> registers, and Manu's proposed approach allows for a more "atomic"
> response.  The fact that we currently do the status, SNR, strength,
> and UNC/BER in separate calls is one reason that people sometimes see
> inconsistent results in the output of tools like zap.  As an example,
> they can see lines in the zap output where the lock is lost but SNR
> appears fine.
> 
> In the case where the driver servicing the query needs to do three
> calls, it could be slightly more expensive, but only if we believe
> that it is commonplace to ask for a subset of the stats.

For what it's worth, we have solved this problem in hwmon driver the
following way: we cache related values (read from the same register or
set of registers) for ~1 second. When user-space requests the
information, if the cache is fresh it is used, otherwise the cache is
first refreshed. That way we ensure that data returned to nearby
user-space calls are taken from the same original register value. One
advantage is that we thus did not have to map the API to the hardware
register constraints and thus have the guarantee that all hardware
designs fit.

I don't know if a similar logic would help for DVB.

-- 
Jean Delvare

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Devin Heitmueller escreveu:
> On Tue, Nov 17, 2009 at 2:46 PM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> I don't like the idea of creating structs grouping those parameters. While for
>> certain devices this may mean a more direct approach, for others, this may
>> not make sense, due to the way their API's were implemented (for example,
>> devices with firmware may need several calls to get all those info).
> 
> There is some value in providing grouping the results in a single
> request - in many cases the data is based off of the same internal
> registers, and Manu's proposed approach allows for a more "atomic"
> response.  The fact that we currently do the status, SNR, strength,
> and UNC/BER in separate calls is one reason that people sometimes see
> inconsistent results in the output of tools like zap.  As an example,
> they can see lines in the zap output where the lock is lost but SNR
> appears fine.
> 
> In the case where the driver servicing the query needs to do three
> calls, it could be slightly more expensive, but only if we believe
> that it is commonplace to ask for a subset of the stats.

We shouldn't write API's thinking on some specific use case or aplication. 
If there's a problem with zap, the fix should be there, not at the kernel.

Also, the above mentioned problem can happen even if there's just one API
call from userspace to kernel or if the driver needs to do separate, 
serialized calls to firmware (or a serialized calculus) to get the
three measures.

> For what it's worth, we have solved this problem in hwmon driver the
> following way: we cache related values (read from the same register or
> set of registers) for ~1 second. When user-space requests the
> information, if the cache is fresh it is used, otherwise the cache is
> first refreshed. That way we ensure that data returned to nearby
> user-space calls are taken from the same original register value. One
> advantage is that we thus did not have to map the API to the hardware
> register constraints and thus have the guarantee that all hardware
> designs fit.
> 
> I don't know if a similar logic would help for DVB.

This could be an alternative, if implemented at the right way. However,
I suspect that it would be better to do such things at libdvb.

For example, caching measures for 1 second may be too much, if userspace is
doing a scan, while, when streaming, this timeout can be fine.

Cheers,
Mauro.

Re: Details about DVB frontend API

[Devin Heitmueller]

On Tue, Nov 17, 2009 at 5:53 PM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> We shouldn't write API's thinking on some specific use case or aplication.
> If there's a problem with zap, the fix should be there, not at the kernel.

Your response suggests I must have poorly described the problem.  Zap
is just one example where having an "inconsistent" view of the various
performance counters is easily visible.  If you're trying to write
something like an application to control antenna orientation, the fact
that you cannot ask for a single view of all counters can be a real
problem.  Having to make separate ioctl calls for each field can cause
real problems here.

I disagree strongly with your assertion that we should not considering
specific use cases when writing an API.  That's *EXACTLY* what you
want to do - when designing an API, you should be asking yourself what
use cases is it actually going to be used for, and strive to build an
API that accommodates all the use cases.  In this case, Manu's
approach provides the ability to get a single consistent view of all
the counters (for those drivers which can support it), which solves a
specific use case that cannot be accomplished with the existing API.
Building abstract APIs without considering all use cases is how we end
up with APIs that nobody uses because they don't actually work in the
real world.

The fact that the existing SNR and strength counters never had their
format explicitly defined is a really good example of how nobody must
have considered how applications would be expected to actually use the
API and represent the information to users in a useful manner.

On the other hand, this issue has been beaten to death so badly and
the existing API has been broken/useless for so long that I have
lowered my expectations to the point where I would accept just about
*any* proposal that actually provides a uniform representation of SNR
across drivers.

(/rant mode off)

> Also, the above mentioned problem can happen even if there's just one API
> call from userspace to kernel or if the driver needs to do separate,
> serialized calls to firmware (or a serialized calculus) to get the
> three measures.

True, the accuracy in which a given driver can provide accurate data
is tied to the quality of the hardware implementation.  However, for
well engineered hardware, Manu's proposed API affords the ability to
accurately report a consistent view of the information.  The existing
implementation restricts all drivers to working as well as the
worst-case hardware implementation.

>> For what it's worth, we have solved this problem in hwmon driver the
>> following way: we cache related values (read from the same register or
>> set of registers) for ~1 second. When user-space requests the
>> information, if the cache is fresh it is used, otherwise the cache is
>> first refreshed. That way we ensure that data returned to nearby
>> user-space calls are taken from the same original register value. One
>> advantage is that we thus did not have to map the API to the hardware
>> register constraints and thus have the guarantee that all hardware
>> designs fit.
>>
>> I don't know if a similar logic would help for DVB.
>
> This could be an alternative, if implemented at the right way. However,
> I suspect that it would be better to do such things at libdvb.
>
> For example, caching measures for 1 second may be too much, if userspace is
> doing a scan, while, when streaming, this timeout can be fine.

Jean's caching approach for hwmon is fine for something like the
chassis temperature, which doesn't change that rapidly.  However, it's
probably not appropriate for things like SNR and strength counters,
where near real-time feedback can be useful in things like controlling
a rotor.

One more point worth noting - the approach of returning all the
counters in one ioctl can actually be cheaper in terms of the number
of register read operations.  I've seen a number of drivers where we
hit the same register three or four times, since all of various fields
are based on the same register.  Having a single call actually allows
all the duplicate register reads to be eliminated in those cases, the
driver reads the register once and then populates all the fields in
one shot based on the result.

I was actually against Manu's proposal the last time it was put out
there, as I felt just normalizing the existing API would be *good
enough* for the vast majority of applications.  However, if we have
decided to give up on the existing API entirely and write a whole new
API, we might as well do it right this time and build an API that
satisfies all the people who plan to make use of it.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Devin Heitmueller wrote:
> On Tue, Nov 17, 2009 at 5:53 PM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> We shouldn't write API's thinking on some specific use case or aplication.
>> If there's a problem with zap, the fix should be there, not at the kernel.
> 
> Your response suggests I must have poorly described the problem.  Zap
> is just one example where having an "inconsistent" view of the various
> performance counters is easily visible.  If you're trying to write
> something like an application to control antenna orientation, the fact
> that you cannot ask for a single view of all counters can be a real
> problem.  Having to make separate ioctl calls for each field can cause
> real problems here.
> 
> I disagree strongly with your assertion that we should not considering
> specific use cases when writing an API. 

That's not what I've said (or maybe I haven't said it clear enough). 
I'm saying that we shouldn't look for one particular use case only.
In other words, the API should cover all use cases that makes sense.

>In this case, Manu's
> approach provides the ability to get a single consistent view of all
> the counters (for those drivers which can support it)

No. To get all counters, you'll need to call 3 ioctls. The status were
grouped around 3 groups of counters on his proposal. I'm sure Manu has some
explanation why he thinks that 3 is better then 2 or 4 calls, but the point
is: should we really group them on a fixed way?

Btw, by using S2API, we'll break it into different commands. Yet, we can
call all of them at once, if needed, as the API defines it as:

struct dtv_property {
        __u32 cmd;
        __u32 reserved[3];
        union {
                __u32 data;
                struct {
                        __u8 data[32];
                        __u32 len;
                        __u32 reserved1[3];
                        void *reserved2;
                } buffer;
        } u;
        int result;
} __attribute__ ((packed));

struct dtv_properties {
        __u32 num;
        struct dtv_property *props;
};

#define FE_SET_PROPERTY            _IOW('o', 82, struct dtv_properties)
#define FE_GET_PROPERTY            _IOR('o', 83, struct dtv_properties)

So, all needed commands can be grouped together to provide an atomic read.

>> Also, the above mentioned problem can happen even if there's just one API
>> call from userspace to kernel or if the driver needs to do separate,
>> serialized calls to firmware (or a serialized calculus) to get the
>> three measures.
> 
> True, the accuracy in which a given driver can provide accurate data
> is tied to the quality of the hardware implementation.  However, for
> well engineered hardware, Manu's proposed API affords the ability to
> accurately report a consistent view of the information.  The existing
> implementation restricts all drivers to working as well as the
> worst-case hardware implementation.

Imagining that some application will need to retrieve all quality indicators
at the same time, as they were grouped into 3 groups, even on a perfect
hardware, you won't be able to get all of them at the sime time,
since you'll need to call 3 ioctls.

>>> For what it's worth, we have solved this problem in hwmon driver the
>>> following way: we cache related values (read from the same register or
>>> set of registers) for ~1 second. When user-space requests the
>>> information, if the cache is fresh it is used, otherwise the cache is
>>> first refreshed. That way we ensure that data returned to nearby
>>> user-space calls are taken from the same original register value. One
>>> advantage is that we thus did not have to map the API to the hardware
>>> register constraints and thus have the guarantee that all hardware
>>> designs fit.
>>>
>>> I don't know if a similar logic would help for DVB.
>> This could be an alternative, if implemented at the right way. However,
>> I suspect that it would be better to do such things at libdvb.
>>
>> For example, caching measures for 1 second may be too much, if userspace is
>> doing a scan, while, when streaming, this timeout can be fine.
> 
> Jean's caching approach for hwmon is fine for something like the
> chassis temperature, which doesn't change that rapidly.  However, it's
> probably not appropriate for things like SNR and strength counters,
> where near real-time feedback can be useful in things like controlling
> a rotor.

I agree. 

Yet, you won't have this feedback in real-time, since calculating
QoS indicators require some time. So, after moving the rotor, you'll need
to wait for some time, in order to allow the frontend to recalculate the
QoS after the movement. There's a problem here, not addressed by none of
the proposals: when the QoS indicators will reflect the rotor movement?

If you just read the QoS indicator in a register, you're likely getting a
cached value from the last time the hardware did the calculus.

So, if you really need to know the QoS value after changing the antenna
position, you'll need to wait for some time before reading the QoS.

This time will likely depend on how the frontend chip QoS logic is
implemented internally. Also, some indicators are dependent of the bit rate
(like frame error counters).

Imagining the best case where the developer knows how to estimate this time,
we would need a way to report this time to userspace or to generate an event
after having the measure done.
 
> One more point worth noting - the approach of returning all the
> counters in one ioctl can actually be cheaper in terms of the number
> of register read operations.  I've seen a number of drivers where we
> hit the same register three or four times, since all of various fields
> are based on the same register.  Having a single call actually allows
> all the duplicate register reads to be eliminated in those cases, the
> driver reads the register once and then populates all the fields in
> one shot based on the result.

True.

> I was actually against Manu's proposal the last time it was put out
> there, as I felt just normalizing the existing API would be *good
> enough* for the vast majority of applications.  However, if we have
> decided to give up on the existing API entirely and write a whole new
> API, we might as well do it right this time and build an API that
> satisfies all the people who plan to make use of it.

In general, I don't like to deprecate userspace API's. This means too much
work for kernel and for userspace developers.

However, in this specific case, there are several cons against
normalizing the existing API is that:

	1) We will never know the scale used by some drivers. So, on some
drivers, it will keep representing data at some arbitrary scale;

	2) Reverse-engineered drivers will never have a proper scale, as
the developer has no way to check for sure what scale is reported by the
device;

	3) Existing applications might break, since we'll change the
	   scale of existing drivers;

	4) There are no way for applications written after the change to
	   check if that driver is providing the value at the proposed scale;

The beauty of Manu's approach is that it supports all devices, even those
were we'll never know what scale is used. So, all 4 cons are solved by
creating a new API.

However, as-is, it has some drawbacks:

	1) arbitrary groups of QoS measures;
	2) structs are fixed. So, if changes would be needed later, or if more
	   QoS indicators would be needed, we'll have troubles.
	3) due to (2), it is not future-proof, since it may need to be deprecated
	   if more QoS indicators would need to be added to a certain QoS group.

However, if we port his proposal to S2API approach, we'll have a future-proof
approach, as adding more QoS is as simple as creating another S2API command.

Cheers,
Mauro.
	

Re: Details about DVB frontend API

[Devin Heitmueller]

On Wed, Nov 18, 2009 at 9:04 AM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> Devin Heitmueller wrote:
>> On Tue, Nov 17, 2009 at 5:53 PM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>>> We shouldn't write API's thinking on some specific use case or aplication.
>>> If there's a problem with zap, the fix should be there, not at the kernel.
>>
>> Your response suggests I must have poorly described the problem.  Zap
>> is just one example where having an "inconsistent" view of the various
>> performance counters is easily visible.  If you're trying to write
>> something like an application to control antenna orientation, the fact
>> that you cannot ask for a single view of all counters can be a real
>> problem.  Having to make separate ioctl calls for each field can cause
>> real problems here.
>>
>> I disagree strongly with your assertion that we should not considering
>> specific use cases when writing an API.
>
> That's not what I've said (or maybe I haven't said it clear enough).
> I'm saying that we shouldn't look for one particular use case only.
> In other words, the API should cover all use cases that makes sense.
>
>>In this case, Manu's
>> approach provides the ability to get a single consistent view of all
>> the counters (for those drivers which can support it)
>
> No. To get all counters, you'll need to call 3 ioctls. The status were
> grouped around 3 groups of counters on his proposal. I'm sure Manu has some
> explanation why he thinks that 3 is better then 2 or 4 calls, but the point
> is: should we really group them on a fixed way?
>
> Btw, by using S2API, we'll break it into different commands. Yet, we can
> call all of them at once, if needed, as the API defines it as:
>
> struct dtv_property {
>        __u32 cmd;
>        __u32 reserved[3];
>        union {
>                __u32 data;
>                struct {
>                        __u8 data[32];
>                        __u32 len;
>                        __u32 reserved1[3];
>                        void *reserved2;
>                } buffer;
>        } u;
>        int result;
> } __attribute__ ((packed));
>
> struct dtv_properties {
>        __u32 num;
>        struct dtv_property *props;
> };
>
> #define FE_SET_PROPERTY            _IOW('o', 82, struct dtv_properties)
> #define FE_GET_PROPERTY            _IOR('o', 83, struct dtv_properties)
>
> So, all needed commands can be grouped together to provide an atomic read.
>
>>> Also, the above mentioned problem can happen even if there's just one API
>>> call from userspace to kernel or if the driver needs to do separate,
>>> serialized calls to firmware (or a serialized calculus) to get the
>>> three measures.
>>
>> True, the accuracy in which a given driver can provide accurate data
>> is tied to the quality of the hardware implementation.  However, for
>> well engineered hardware, Manu's proposed API affords the ability to
>> accurately report a consistent view of the information.  The existing
>> implementation restricts all drivers to working as well as the
>> worst-case hardware implementation.
>
> Imagining that some application will need to retrieve all quality indicators
> at the same time, as they were grouped into 3 groups, even on a perfect
> hardware, you won't be able to get all of them at the sime time,
> since you'll need to call 3 ioctls.
>
>>>> For what it's worth, we have solved this problem in hwmon driver the
>>>> following way: we cache related values (read from the same register or
>>>> set of registers) for ~1 second. When user-space requests the
>>>> information, if the cache is fresh it is used, otherwise the cache is
>>>> first refreshed. That way we ensure that data returned to nearby
>>>> user-space calls are taken from the same original register value. One
>>>> advantage is that we thus did not have to map the API to the hardware
>>>> register constraints and thus have the guarantee that all hardware
>>>> designs fit.
>>>>
>>>> I don't know if a similar logic would help for DVB.
>>> This could be an alternative, if implemented at the right way. However,
>>> I suspect that it would be better to do such things at libdvb.
>>>
>>> For example, caching measures for 1 second may be too much, if userspace is
>>> doing a scan, while, when streaming, this timeout can be fine.
>>
>> Jean's caching approach for hwmon is fine for something like the
>> chassis temperature, which doesn't change that rapidly.  However, it's
>> probably not appropriate for things like SNR and strength counters,
>> where near real-time feedback can be useful in things like controlling
>> a rotor.
>
> I agree.
>
> Yet, you won't have this feedback in real-time, since calculating
> QoS indicators require some time. So, after moving the rotor, you'll need
> to wait for some time, in order to allow the frontend to recalculate the
> QoS after the movement. There's a problem here, not addressed by none of
> the proposals: when the QoS indicators will reflect the rotor movement?
>
> If you just read the QoS indicator in a register, you're likely getting a
> cached value from the last time the hardware did the calculus.
>
> So, if you really need to know the QoS value after changing the antenna
> position, you'll need to wait for some time before reading the QoS.
>
> This time will likely depend on how the frontend chip QoS logic is
> implemented internally. Also, some indicators are dependent of the bit rate
> (like frame error counters).
>
> Imagining the best case where the developer knows how to estimate this time,
> we would need a way to report this time to userspace or to generate an event
> after having the measure done.
>
>> One more point worth noting - the approach of returning all the
>> counters in one ioctl can actually be cheaper in terms of the number
>> of register read operations.  I've seen a number of drivers where we
>> hit the same register three or four times, since all of various fields
>> are based on the same register.  Having a single call actually allows
>> all the duplicate register reads to be eliminated in those cases, the
>> driver reads the register once and then populates all the fields in
>> one shot based on the result.
>
> True.
>
>> I was actually against Manu's proposal the last time it was put out
>> there, as I felt just normalizing the existing API would be *good
>> enough* for the vast majority of applications.  However, if we have
>> decided to give up on the existing API entirely and write a whole new
>> API, we might as well do it right this time and build an API that
>> satisfies all the people who plan to make use of it.
>
> In general, I don't like to deprecate userspace API's. This means too much
> work for kernel and for userspace developers.
>
> However, in this specific case, there are several cons against
> normalizing the existing API is that:
>
>        1) We will never know the scale used by some drivers. So, on some
> drivers, it will keep representing data at some arbitrary scale;
>
>        2) Reverse-engineered drivers will never have a proper scale, as
> the developer has no way to check for sure what scale is reported by the
> device;
>
>        3) Existing applications might break, since we'll change the
>           scale of existing drivers;
>
>        4) There are no way for applications written after the change to
>           check if that driver is providing the value at the proposed scale;
>
> The beauty of Manu's approach is that it supports all devices, even those
> were we'll never know what scale is used. So, all 4 cons are solved by
> creating a new API.
>
> However, as-is, it has some drawbacks:
>
>        1) arbitrary groups of QoS measures;
>        2) structs are fixed. So, if changes would be needed later, or if more
>           QoS indicators would be needed, we'll have troubles.
>        3) due to (2), it is not future-proof, since it may need to be deprecated
>           if more QoS indicators would need to be added to a certain QoS group.
>
> However, if we port his proposal to S2API approach, we'll have a future-proof
> approach, as adding more QoS is as simple as creating another S2API command.
>
> Cheers,
> Mauro.

If this is an argument about whether to do it with s2api or the older
ioctl interface, I definitely do not have any objection to doing it
with s2api.  All the applications I care about have been ported over,
so the incremental cost of using the new call is minimal, and perhaps
having some reliable quality indicators will prompt some of the
applications that haven't ported to s2api to finally cut over.

Devin


-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[Michael Krufky]

On Wed, Nov 18, 2009 at 10:17 AM, Devin Heitmueller
<dheitmueller@kernellabs.com> wrote:
> On Wed, Nov 18, 2009 at 9:04 AM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> Devin Heitmueller wrote:
>>> On Tue, Nov 17, 2009 at 5:53 PM, Mauro Carvalho Chehab
>>> <mchehab@infradead.org> wrote:
>>>> We shouldn't write API's thinking on some specific use case or aplication.
>>>> If there's a problem with zap, the fix should be there, not at the kernel.
>>>
>>> Your response suggests I must have poorly described the problem.  Zap
>>> is just one example where having an "inconsistent" view of the various
>>> performance counters is easily visible.  If you're trying to write
>>> something like an application to control antenna orientation, the fact
>>> that you cannot ask for a single view of all counters can be a real
>>> problem.  Having to make separate ioctl calls for each field can cause
>>> real problems here.
>>>
>>> I disagree strongly with your assertion that we should not considering
>>> specific use cases when writing an API.
>>
>> That's not what I've said (or maybe I haven't said it clear enough).
>> I'm saying that we shouldn't look for one particular use case only.
>> In other words, the API should cover all use cases that makes sense.
>>
>>>In this case, Manu's
>>> approach provides the ability to get a single consistent view of all
>>> the counters (for those drivers which can support it)
>>
>> No. To get all counters, you'll need to call 3 ioctls. The status were
>> grouped around 3 groups of counters on his proposal. I'm sure Manu has some
>> explanation why he thinks that 3 is better then 2 or 4 calls, but the point
>> is: should we really group them on a fixed way?
>>
>> Btw, by using S2API, we'll break it into different commands. Yet, we can
>> call all of them at once, if needed, as the API defines it as:
>>
>> struct dtv_property {
>>        __u32 cmd;
>>        __u32 reserved[3];
>>        union {
>>                __u32 data;
>>                struct {
>>                        __u8 data[32];
>>                        __u32 len;
>>                        __u32 reserved1[3];
>>                        void *reserved2;
>>                } buffer;
>>        } u;
>>        int result;
>> } __attribute__ ((packed));
>>
>> struct dtv_properties {
>>        __u32 num;
>>        struct dtv_property *props;
>> };
>>
>> #define FE_SET_PROPERTY            _IOW('o', 82, struct dtv_properties)
>> #define FE_GET_PROPERTY            _IOR('o', 83, struct dtv_properties)
>>
>> So, all needed commands can be grouped together to provide an atomic read.
>>
>>>> Also, the above mentioned problem can happen even if there's just one API
>>>> call from userspace to kernel or if the driver needs to do separate,
>>>> serialized calls to firmware (or a serialized calculus) to get the
>>>> three measures.
>>>
>>> True, the accuracy in which a given driver can provide accurate data
>>> is tied to the quality of the hardware implementation.  However, for
>>> well engineered hardware, Manu's proposed API affords the ability to
>>> accurately report a consistent view of the information.  The existing
>>> implementation restricts all drivers to working as well as the
>>> worst-case hardware implementation.
>>
>> Imagining that some application will need to retrieve all quality indicators
>> at the same time, as they were grouped into 3 groups, even on a perfect
>> hardware, you won't be able to get all of them at the sime time,
>> since you'll need to call 3 ioctls.
>>
>>>>> For what it's worth, we have solved this problem in hwmon driver the
>>>>> following way: we cache related values (read from the same register or
>>>>> set of registers) for ~1 second. When user-space requests the
>>>>> information, if the cache is fresh it is used, otherwise the cache is
>>>>> first refreshed. That way we ensure that data returned to nearby
>>>>> user-space calls are taken from the same original register value. One
>>>>> advantage is that we thus did not have to map the API to the hardware
>>>>> register constraints and thus have the guarantee that all hardware
>>>>> designs fit.
>>>>>
>>>>> I don't know if a similar logic would help for DVB.
>>>> This could be an alternative, if implemented at the right way. However,
>>>> I suspect that it would be better to do such things at libdvb.
>>>>
>>>> For example, caching measures for 1 second may be too much, if userspace is
>>>> doing a scan, while, when streaming, this timeout can be fine.
>>>
>>> Jean's caching approach for hwmon is fine for something like the
>>> chassis temperature, which doesn't change that rapidly.  However, it's
>>> probably not appropriate for things like SNR and strength counters,
>>> where near real-time feedback can be useful in things like controlling
>>> a rotor.
>>
>> I agree.
>>
>> Yet, you won't have this feedback in real-time, since calculating
>> QoS indicators require some time. So, after moving the rotor, you'll need
>> to wait for some time, in order to allow the frontend to recalculate the
>> QoS after the movement. There's a problem here, not addressed by none of
>> the proposals: when the QoS indicators will reflect the rotor movement?
>>
>> If you just read the QoS indicator in a register, you're likely getting a
>> cached value from the last time the hardware did the calculus.
>>
>> So, if you really need to know the QoS value after changing the antenna
>> position, you'll need to wait for some time before reading the QoS.
>>
>> This time will likely depend on how the frontend chip QoS logic is
>> implemented internally. Also, some indicators are dependent of the bit rate
>> (like frame error counters).
>>
>> Imagining the best case where the developer knows how to estimate this time,
>> we would need a way to report this time to userspace or to generate an event
>> after having the measure done.
>>
>>> One more point worth noting - the approach of returning all the
>>> counters in one ioctl can actually be cheaper in terms of the number
>>> of register read operations.  I've seen a number of drivers where we
>>> hit the same register three or four times, since all of various fields
>>> are based on the same register.  Having a single call actually allows
>>> all the duplicate register reads to be eliminated in those cases, the
>>> driver reads the register once and then populates all the fields in
>>> one shot based on the result.
>>
>> True.
>>
>>> I was actually against Manu's proposal the last time it was put out
>>> there, as I felt just normalizing the existing API would be *good
>>> enough* for the vast majority of applications.  However, if we have
>>> decided to give up on the existing API entirely and write a whole new
>>> API, we might as well do it right this time and build an API that
>>> satisfies all the people who plan to make use of it.
>>
>> In general, I don't like to deprecate userspace API's. This means too much
>> work for kernel and for userspace developers.
>>
>> However, in this specific case, there are several cons against
>> normalizing the existing API is that:
>>
>>        1) We will never know the scale used by some drivers. So, on some
>> drivers, it will keep representing data at some arbitrary scale;
>>
>>        2) Reverse-engineered drivers will never have a proper scale, as
>> the developer has no way to check for sure what scale is reported by the
>> device;
>>
>>        3) Existing applications might break, since we'll change the
>>           scale of existing drivers;
>>
>>        4) There are no way for applications written after the change to
>>           check if that driver is providing the value at the proposed scale;
>>
>> The beauty of Manu's approach is that it supports all devices, even those
>> were we'll never know what scale is used. So, all 4 cons are solved by
>> creating a new API.
>>
>> However, as-is, it has some drawbacks:
>>
>>        1) arbitrary groups of QoS measures;
>>        2) structs are fixed. So, if changes would be needed later, or if more
>>           QoS indicators would be needed, we'll have troubles.
>>        3) due to (2), it is not future-proof, since it may need to be deprecated
>>           if more QoS indicators would need to be added to a certain QoS group.
>>
>> However, if we port his proposal to S2API approach, we'll have a future-proof
>> approach, as adding more QoS is as simple as creating another S2API command.
>>
>> Cheers,
>> Mauro.
>
> If this is an argument about whether to do it with s2api or the older
> ioctl interface, I definitely do not have any objection to doing it
> with s2api.  All the applications I care about have been ported over,
> so the incremental cost of using the new call is minimal, and perhaps
> having some reliable quality indicators will prompt some of the
> applications that haven't ported to s2api to finally cut over.
>
> Devin
>
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com


I, too, would prefer to see S2API used as the interface into the
kernel for these quality measurement functions, however, I feel that
Mauro is missing a large point:

These quality measurements are not necessarily as meaningful when
retrieved as standalone values--  It is most useful to retrieve some
of these values all at once, since qualityvalueX might depend on
qualityvalueY, and may mean something entirely different depending on
qualityvalueZ.  When retrieved separately, the application has no way
to identify the relationship between these values, and the information
can become much less meaningful.

For example, SNR means nothing in DVB-T without also knowing the
modulation and coderate.  If a single call retrieves the TPS
statistics, and a later call retrieves the SNR, we have no way to know
that TPS has remained the same -- the SNR value retrieved may be
associated to new TPS data, but the application will only have the
data returned from the previous call in memory.

That wasn't the best example, but I don't have a lot of time for this
email right now.

My point is, some of these quality measurements indeed belong
together, and these calls for quality statistics really should handle
multiple values at once.  The application can make less calls into the
kernel and only use the information that it needs.  Anything else
would hurt the accuracy of the quality reporting, and thus make this
entire effort worth a lot less.

I like Manu's proposal, I just haven't had time to provide any
feedback.  If I were to change anything in his proposal, it would be
to use the S2API get property method, rather than issuing new IOCTLS.

I feel the need to mention, however, that old applications will
continue to use the older SNR / strength / ber / unc values -- I would
still like to fix THESE readings to be uniform across the subsystem,
and we will need to support these for the sake of application
compatibility.  I would like to write a translation layer once Manu's
method is in place, (hopefully using S2API get property) ... and I
believe that it's very important for conformity across the existing
drivers.

Regards,

Mike Krufky

Re: Details about DVB frontend API

[Devin Heitmueller]

On Wed, Nov 18, 2009 at 9:04 AM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> Devin Heitmueller wrote:
>> On Tue, Nov 17, 2009 at 5:53 PM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>>> We shouldn't write API's thinking on some specific use case or aplication.
>>> If there's a problem with zap, the fix should be there, not at the kernel.
>>
>> Your response suggests I must have poorly described the problem.  Zap
>> is just one example where having an "inconsistent" view of the various
>> performance counters is easily visible.  If you're trying to write
>> something like an application to control antenna orientation, the fact
>> that you cannot ask for a single view of all counters can be a real
>> problem.  Having to make separate ioctl calls for each field can cause
>> real problems here.
>>
>> I disagree strongly with your assertion that we should not considering
>> specific use cases when writing an API.
>
> That's not what I've said (or maybe I haven't said it clear enough).
> I'm saying that we shouldn't look for one particular use case only.
> In other words, the API should cover all use cases that makes sense.
>
>>In this case, Manu's
>> approach provides the ability to get a single consistent view of all
>> the counters (for those drivers which can support it)
>
> No. To get all counters, you'll need to call 3 ioctls. The status were
> grouped around 3 groups of counters on his proposal. I'm sure Manu has some
> explanation why he thinks that 3 is better then 2 or 4 calls, but the point
> is: should we really group them on a fixed way?
>
> Btw, by using S2API, we'll break it into different commands. Yet, we can
> call all of them at once, if needed, as the API defines it as:
>
> struct dtv_property {
>        __u32 cmd;
>        __u32 reserved[3];
>        union {
>                __u32 data;
>                struct {
>                        __u8 data[32];
>                        __u32 len;
>                        __u32 reserved1[3];
>                        void *reserved2;
>                } buffer;
>        } u;
>        int result;
> } __attribute__ ((packed));
>
> struct dtv_properties {
>        __u32 num;
>        struct dtv_property *props;
> };
>
> #define FE_SET_PROPERTY            _IOW('o', 82, struct dtv_properties)
> #define FE_GET_PROPERTY            _IOR('o', 83, struct dtv_properties)
>
> So, all needed commands can be grouped together to provide an atomic read.
>
>>> Also, the above mentioned problem can happen even if there's just one API
>>> call from userspace to kernel or if the driver needs to do separate,
>>> serialized calls to firmware (or a serialized calculus) to get the
>>> three measures.
>>
>> True, the accuracy in which a given driver can provide accurate data
>> is tied to the quality of the hardware implementation.  However, for
>> well engineered hardware, Manu's proposed API affords the ability to
>> accurately report a consistent view of the information.  The existing
>> implementation restricts all drivers to working as well as the
>> worst-case hardware implementation.
>
> Imagining that some application will need to retrieve all quality indicators
> at the same time, as they were grouped into 3 groups, even on a perfect
> hardware, you won't be able to get all of them at the sime time,
> since you'll need to call 3 ioctls.
>
>>>> For what it's worth, we have solved this problem in hwmon driver the
>>>> following way: we cache related values (read from the same register or
>>>> set of registers) for ~1 second. When user-space requests the
>>>> information, if the cache is fresh it is used, otherwise the cache is
>>>> first refreshed. That way we ensure that data returned to nearby
>>>> user-space calls are taken from the same original register value. One
>>>> advantage is that we thus did not have to map the API to the hardware
>>>> register constraints and thus have the guarantee that all hardware
>>>> designs fit.
>>>>
>>>> I don't know if a similar logic would help for DVB.
>>> This could be an alternative, if implemented at the right way. However,
>>> I suspect that it would be better to do such things at libdvb.
>>>
>>> For example, caching measures for 1 second may be too much, if userspace is
>>> doing a scan, while, when streaming, this timeout can be fine.
>>
>> Jean's caching approach for hwmon is fine for something like the
>> chassis temperature, which doesn't change that rapidly.  However, it's
>> probably not appropriate for things like SNR and strength counters,
>> where near real-time feedback can be useful in things like controlling
>> a rotor.
>
> I agree.
>
> Yet, you won't have this feedback in real-time, since calculating
> QoS indicators require some time. So, after moving the rotor, you'll need
> to wait for some time, in order to allow the frontend to recalculate the
> QoS after the movement. There's a problem here, not addressed by none of
> the proposals: when the QoS indicators will reflect the rotor movement?
>
> If you just read the QoS indicator in a register, you're likely getting a
> cached value from the last time the hardware did the calculus.
>
> So, if you really need to know the QoS value after changing the antenna
> position, you'll need to wait for some time before reading the QoS.
>
> This time will likely depend on how the frontend chip QoS logic is
> implemented internally. Also, some indicators are dependent of the bit rate
> (like frame error counters).
>
> Imagining the best case where the developer knows how to estimate this time,
> we would need a way to report this time to userspace or to generate an event
> after having the measure done.
>
>> One more point worth noting - the approach of returning all the
>> counters in one ioctl can actually be cheaper in terms of the number
>> of register read operations.  I've seen a number of drivers where we
>> hit the same register three or four times, since all of various fields
>> are based on the same register.  Having a single call actually allows
>> all the duplicate register reads to be eliminated in those cases, the
>> driver reads the register once and then populates all the fields in
>> one shot based on the result.
>
> True.
>
>> I was actually against Manu's proposal the last time it was put out
>> there, as I felt just normalizing the existing API would be *good
>> enough* for the vast majority of applications.  However, if we have
>> decided to give up on the existing API entirely and write a whole new
>> API, we might as well do it right this time and build an API that
>> satisfies all the people who plan to make use of it.
>
> In general, I don't like to deprecate userspace API's. This means too much
> work for kernel and for userspace developers.
>
> However, in this specific case, there are several cons against
> normalizing the existing API is that:
>
>        1) We will never know the scale used by some drivers. So, on some
> drivers, it will keep representing data at some arbitrary scale;
>
>        2) Reverse-engineered drivers will never have a proper scale, as
> the developer has no way to check for sure what scale is reported by the
> device;
>
>        3) Existing applications might break, since we'll change the
>           scale of existing drivers;
>
>        4) There are no way for applications written after the change to
>           check if that driver is providing the value at the proposed scale;
>
> The beauty of Manu's approach is that it supports all devices, even those
> were we'll never know what scale is used. So, all 4 cons are solved by
> creating a new API.
>
> However, as-is, it has some drawbacks:
>
>        1) arbitrary groups of QoS measures;
>        2) structs are fixed. So, if changes would be needed later, or if more
>           QoS indicators would be needed, we'll have troubles.
>        3) due to (2), it is not future-proof, since it may need to be deprecated
>           if more QoS indicators would need to be added to a certain QoS group.
>
> However, if we port his proposal to S2API approach, we'll have a future-proof
> approach, as adding more QoS is as simple as creating another S2API command.
>
> Cheers,
> Mauro.

Two other points we should definitely consider in defining this API:

1.  We should provide some example raw values for different modulation
schemes, as well for the different scales and quality types.  This
will help application developers who may not have access to signals
for testing because of geography and want some confidence it will
still work.  It will also allow people adding support for new
demodulators to have some reference values that their new drivers
should be similar to.  I know I've had situations where I was adding a
new demod driver and wondered, "what should the values look like?".  I
think this also explains one reason we ended up with different
standards, since people likely copied the behavior of whatever other
device they happen to have that already worked.

2.  We should define the various possible error codes that can occur.
For example, in the DVBv3 API, demods are supposed to return ENOSIGNAL
when they are unable to provide an accurate reading (for example, many
demods cannot provide SNR data when there is no signal lock).  The API
should be explicit and we should be make sure that new drivers
conform.  For example, patches that have stub implementations for the
SNR and strength callbacks which just return zero should be rejected
and the submitter should fix the implementation to return ENOSYS.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[Manu Abraham]

On Tue, Nov 17, 2009 at 11:46 PM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
>
> Manu Abraham escreveu:
> > On Fri, Oct 23, 2009 at 12:10 AM, Mauro Carvalho Chehab
> > <mchehab@infradead.org> wrote:
> >> Em Thu, 22 Oct 2009 21:13:30 +0200
> >> Jean Delvare <khali@linux-fr.org> escreveu:
> >>
> >>> Hi folks,
> >>>
> >>> I am looking for details regarding the DVB frontend API. I've read
> >>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
> >>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
> >>> commands return, however it does not give any information about how the
> >>> returned values should be interpreted (or, seen from the other end, how
> >>> the frontend kernel drivers should encode these values.) If there
> >>> documentation available that would explain this?
> >>>
> >>> For example, the signal strength. All I know so far is that this is a
> >>> 16-bit value. But then what? Do greater values represent stronger
> >>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
> >>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
> >>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
> >>> well-defined meanings, or is it arbitrary and each driver can have its
> >>> own scale? What are the typical use cases by user-space application for
> >>> this value?
> >>>
> >>> That's the kind of details I'd like to know, not only for the signal
> >>> strength, but also for the SNR, BER and UB. Without this information,
> >>> it seems a little difficult to have consistent frontend drivers.
> >> We all want to know about that ;)
> >>
> >> Seriously, the lack of a description of the meaning of the ranges for those
> >> read values were already widely discussed at LMML and at the legacy dvb ML.
> >> We should return this discussion again and decide what would be the better
> >> way to describe those values.
> >>
> >> My suggestion is that someone summarize the proposals we had and give some time
> >> for people vote. After that, we just commit the most voted one, and commit the
> >> patches for it. A pending question that should also be discussed is what we will
> >> do with those dvb devices where we simply don't know what scale it uses. There
> >> are several of them.
> >
> >
> > Sometime back, (some time in April) i proposed a patch which addressed
> > the issue to scale "even those devices which have a weird scale or
> > none". Though based on an older tree of mine, here is the patch again.
> > If it looks good enough, i can port the patch to accomodate other
> > devices as well.
> >
> >
> > Regards,
> > Manu
> >
>
> Manu,
>
> Sorry for not answering earlier. Due to my travels, I had a very big backlog here
> to handle.
>
> I prefer a solution like you've proposed of creating a new set of API calls for
> it, instead of re-defining the current calls.



I have been unable to respond earlier on this, being out of station.
Sorry, my previous mail failed to reach the Mailing list due to HTML
part being existant. Hopefully it is fixed this time.


> Yet, I have a few comments:
>
> diff -r b5505a985f24 linux/include/linux/dvb/frontend.h
> --- a/linux/include/linux/dvb/frontend.h        Sat Feb 21 01:12:09 2009 +0400
> +++ b/linux/include/linux/dvb/frontend.h        Tue Apr 07 18:19:22 2009 +0400
> @@ -645,4 +645,118 @@
>  };
>  #define DVBFE_GET_EVENT                        _IOR('o', 86, struct dvbfe_event)
>
> +/* Frontend General Statistics
> + * General parameters
> + * FE_*_UNKNOWN:
> + *     Parameter is unknown to the frontend and doesn't really
> + *     make any sense for an application.
> + *
> + * FE_*_RELATIVE:
> + *     Parameter is relative on the basis of a ceil - floor basis
> + *     Format is based on empirical test to determine
> + *     the floor and ceiling values. This format is exactly the
> + *     same format as the existing statistics implementation.
> + */
> +enum fecap_quality_params {
> +       FE_QUALITY_UNKNOWN              = 0,
> +       FE_QUALITY_SNR                  = (1 <<  0),
> +       FE_QUALITY_CNR                  = (1 <<  1),
> +       FE_QUALITY_EsNo                 = (1 <<  2),
> +       FE_QUALITY_EbNo                 = (1 <<  3),
> +       FE_QUALITY_RELATIVE             = (1 << 31),
> +};
> +
> +enum fecap_scale_params {
> +       FE_SCALE_UNKNOWN                = 0,
> +       FE_SCALE_dB                     = (1 <<  0),
> +       FE_SCALE_RELATIVE               = (1 << 31),
> +};
> +
> +enum fecap_error_params {
> +       FE_ERROR_UNKNOWN                = 0,
> +       FE_ERROR_BER                    = (1 <<  0),
> +       FE_ERROR_PER                    = (1 <<  1),
> +       FE_ERROR_RELATIVE               = (1 << 31),
> +};
> +
> +enum fecap_unc_params {
> +       FE_UNC_UNKNOWN                  = 0,
> +       FE_UNC_RELATIVE                 = (1 << 31),
> +};
> +
> +/* General parameters
> + * width:
> + *     Specifies the width of the field
> + *
> + * exponent:
> + *     Specifies the multiplier for the respective field
> + *     MSB:1bit indicates the signdness of the parameter
> + */
> +struct fecap_quality {
> +       enum fecap_quality_params       params;
> +       enum fecap_scale_params         scale;
> +
> +       __u32                           width;
> +       __s32                           exponent;
> +};
> +
> +struct fecap_strength {
> +       enum fecap_scale_params         params;
> +       __u32                           width;
> +       __s32                           exponent;
> +};
> +
> +struct fecap_error {
> +       enum fecap_error_params         params;
> +       __u32                           width;
> +       __s32                           exponent;
> +};
> +
> +struct fecap_statistics {
> +       struct fecap_quality            quality;
> +       struct fecap_strength           strength;
> +       struct fecap_error              error;
> +       enum fecap_unc_params           unc;
> +};
>
> I don't like the idea of creating structs grouping those parameters. While for
> certain devices this may mean a more direct approach, for others, this may
> not make sense, due to the way their API's were implemented (for example,
> devices with firmware may need several calls to get all those info).
>
> +#define FE_STATISTICS_CAPS             _IOR('o', 84, struct fecap_statistics)
> +#define FE_SIGNAL_LEVEL                        _IOR('o', 85, __u32)
> +#define FE_SIGNAL_STATS                        _IOR('o', 86, struct fesignal_stat)
>
> Instead of defining 3 new ioctls, the better is to use the DVBS2API, maybe extending it to allow
> receiving more than one parameter at the same time (with an approach similar to what we did
> with V4L extended CTRLs API).


Let me explain a bit. The current issue that persists as Devin explained in
another email explains things to a certain extend, but still i think it
might be better to detail further.

Generally a request can be classified to 3 basic types.

1. Request to acquire, retrieve acquisition details
2. Get information (device state, device info)
3. Get information (channel statistics)


The first one for example is a request to say tune to a channel, get tuned
channel details, or even other frontend specific commands such as SEC
operations. These operations are not very critical with regards on a time
scale, just that things could be shifted all along on the time scale, the
worser the implementation, the larger the time "taken to carry around a
larger set of information to handle the operation". Currently, the V3.x and
the V5 implementation handles this. The V3 implementation is small and fast,
while the V5 implementation is "sluggish".


The second one gets basic device information. The V3 implementation handled
this to a certain extend, but the V5 implementation hardly handles this and
the implementation is rather crude rather than being "sophisticated".


The third aspect which is basically needed in most cases for debugging the
channel properties. If all things were ideal, one wouldn't need to know the
details on what's going on inside. So being in the far from ideal thing, the
requisite to know what happens internally is very much a need in all cases.
Another point to be noted is that this category of information is very much
critical on a timescale as these parameters are valid for a very certain
instances of time only. The more this information gets out of sync, the more
these values are meaningless. Also another point to be noted is that all
these values when read back together at the very same instance only does
make sense. It is indeed very hard to achieve this very short timespan
between each of the values being monitored. The larger the bandwidth
associated, the larger the error introduced in the readback of the values
within the same timeframe in between the reads. So the timeframe has to be
the very least possible in between this operation to the device driver
internals too..


Although, i have pointed out with this patch what happens at the driver
level and at the userspace level, There needs additions to the libdvb parts
to handle whatever conversions from format x to y. This needs to be handled
since it might not be very easy to be handled consistsently by all
applications. So in this concept, the application can choose the format
conversion from the library as well, such that the application can provide
the statistics in either the the driver native format, or a unified format
(conversion done by the library) if the user prefers it.



> We are already redefining some existing ioctls there, so it would be clearer for the
> userspace developers what would be the new way to retrieve frontend stats, as we can
> simply say that DVBS2API features superseeds the equivalent DVB v3 ioctls.

As i have noted above, the statistics related calls have a meaning, if and
only if it is hanled very fast and properly with no caching. Having a
genarlized datastructure to handle this event, breaks up the whole meaning
of having this call itself in this position.

What an API generally does is to make things generalized. When one makes
things "the most generalized way" an overhead is also associated with it in
terms of performance. If things were possible, i would directly read from
memory from an application from the hardware itself for processing data in
such a scenario, rather than to make things very generalized. This is an
area where concepts like data caching can be ruled out completely. We are
already at a disadvantage with the kernel driver doing a copy_to_user
itself. Ideally, a memory mapped to userspace would have been the ideal
thing over here.

It is just the question of having yet another set of statistics calls that
handles the information properly or not.


Regards,
Manu

Re: Details about DVB frontend API

[Julian Scheel]

Am Freitag, 20. November 2009 10:29:34 schrieb Manu Abraham:
> Let me explain a bit. The current issue that persists as Devin explained in
> another email explains things to a certain extend, but still i think it
> might be better to detail further.
> 
> Generally a request can be classified to 3 basic types.
> 
> 1. Request to acquire, retrieve acquisition details
> 2. Get information (device state, device info)
> 3. Get information (channel statistics)
> 
> 
> The first one for example is a request to say tune to a channel, get tuned
> channel details, or even other frontend specific commands such as SEC
> operations. These operations are not very critical with regards on a time
> scale, just that things could be shifted all along on the time scale, the
> worser the implementation, the larger the time "taken to carry around a
> larger set of information to handle the operation". Currently, the V3.x and
> the V5 implementation handles this. The V3 implementation is small and
>  fast, while the V5 implementation is "sluggish".
> 
> 
> The second one gets basic device information. The V3 implementation handled
> this to a certain extend, but the V5 implementation hardly handles this and
> the implementation is rather crude rather than being "sophisticated".
> 
> 
> The third aspect which is basically needed in most cases for debugging the
> channel properties. If all things were ideal, one wouldn't need to know the
> details on what's going on inside. So being in the far from ideal thing,
>  the requisite to know what happens internally is very much a need in all
>  cases. Another point to be noted is that this category of information is
>  very much critical on a timescale as these parameters are valid for a very
>  certain instances of time only. The more this information gets out of
>  sync, the more these values are meaningless. Also another point to be
>  noted is that all these values when read back together at the very same
>  instance only does make sense. It is indeed very hard to achieve this very
>  short timespan between each of the values being monitored. The larger the
>  bandwidth associated, the larger the error introduced in the readback of
>  the values within the same timeframe in between the reads. So the
>  timeframe has to be the very least possible in between this operation to
>  the device driver internals too..
> 
> 
> Although, i have pointed out with this patch what happens at the driver
> level and at the userspace level, There needs additions to the libdvb parts
> to handle whatever conversions from format x to y. This needs to be handled
> since it might not be very easy to be handled consistsently by all
> applications. So in this concept, the application can choose the format
> conversion from the library as well, such that the application can provide
> the statistics in either the the driver native format, or a unified format
> (conversion done by the library) if the user prefers it.
> 
> > We are already redefining some existing ioctls there, so it would be
> > clearer for the userspace developers what would be the new way to
> > retrieve frontend stats, as we can simply say that DVBS2API features
> > superseeds the equivalent DVB v3 ioctls.
> 
> As i have noted above, the statistics related calls have a meaning, if and
> only if it is hanled very fast and properly with no caching. Having a
> genarlized datastructure to handle this event, breaks up the whole meaning
> of having this call itself in this position.
> 
> What an API generally does is to make things generalized. When one makes
> things "the most generalized way" an overhead is also associated with it in
> terms of performance. If things were possible, i would directly read from
> memory from an application from the hardware itself for processing data in
> such a scenario, rather than to make things very generalized. This is an
> area where concepts like data caching can be ruled out completely. We are
> already at a disadvantage with the kernel driver doing a copy_to_user
> itself. Ideally, a memory mapped to userspace would have been the ideal
> thing over here.
> 
> It is just the question of having yet another set of statistics calls that
> handles the information properly or not.

Hi Manu,

thanks for the detailed explanation of your point. Actually I am not 
completely familiar with how the S2API calls are handled internally. Still are 
there any proven measurements about the timeframe the calls are being executed 
within? - I absolutely see that reading signal statistics is a time critical 
process, at least it is important to be able to assign the data to a specific 
moment so it can be interpreted in conjunction with the data which is being 
received in that moment.
If this would be the only point where the delay is important one might be able 
to overcome this by adding timestamps into the retrieved data, although I am 
not sure if this would be feasible at all. Anyway having a very good and low-
delay statistics approach would allow realtime applications like sat-finders 
to be implemented (provided, the hardware delivers good enough data).
This would let me vote for a direct IOCTL approach to gain the best possible 
performance.
Treating signal statistics as a seperate approach from configuring the 
frontend would be tolerable for me, so that it could use a different API-
approach than the tuning, which goes with S2API.

I hope you can follow my thoughts and it might be a bit of help for getting 
this forward.

Best regards,
Julian

Re: Details about DVB frontend API

[Manu Abraham]

On Fri, Nov 20, 2009 at 3:37 PM, Julian Scheel <julian@jusst.de> wrote:
> Am Freitag, 20. November 2009 10:29:34 schrieb Manu Abraham:
>> Let me explain a bit. The current issue that persists as Devin explained in
>> another email explains things to a certain extend, but still i think it
>> might be better to detail further.
>>
>> Generally a request can be classified to 3 basic types.
>>
>> 1. Request to acquire, retrieve acquisition details
>> 2. Get information (device state, device info)
>> 3. Get information (channel statistics)
>>
>>
>> The first one for example is a request to say tune to a channel, get tuned
>> channel details, or even other frontend specific commands such as SEC
>> operations. These operations are not very critical with regards on a time
>> scale, just that things could be shifted all along on the time scale, the
>> worser the implementation, the larger the time "taken to carry around a
>> larger set of information to handle the operation". Currently, the V3.x and
>> the V5 implementation handles this. The V3 implementation is small and
>>  fast, while the V5 implementation is "sluggish".
>>
>>
>> The second one gets basic device information. The V3 implementation handled
>> this to a certain extend, but the V5 implementation hardly handles this and
>> the implementation is rather crude rather than being "sophisticated".
>>
>>
>> The third aspect which is basically needed in most cases for debugging the
>> channel properties. If all things were ideal, one wouldn't need to know the
>> details on what's going on inside. So being in the far from ideal thing,
>>  the requisite to know what happens internally is very much a need in all
>>  cases. Another point to be noted is that this category of information is
>>  very much critical on a timescale as these parameters are valid for a very
>>  certain instances of time only. The more this information gets out of
>>  sync, the more these values are meaningless. Also another point to be
>>  noted is that all these values when read back together at the very same
>>  instance only does make sense. It is indeed very hard to achieve this very
>>  short timespan between each of the values being monitored. The larger the
>>  bandwidth associated, the larger the error introduced in the readback of
>>  the values within the same timeframe in between the reads. So the
>>  timeframe has to be the very least possible in between this operation to
>>  the device driver internals too..
>>
>>
>> Although, i have pointed out with this patch what happens at the driver
>> level and at the userspace level, There needs additions to the libdvb parts
>> to handle whatever conversions from format x to y. This needs to be handled
>> since it might not be very easy to be handled consistsently by all
>> applications. So in this concept, the application can choose the format
>> conversion from the library as well, such that the application can provide
>> the statistics in either the the driver native format, or a unified format
>> (conversion done by the library) if the user prefers it.
>>
>> > We are already redefining some existing ioctls there, so it would be
>> > clearer for the userspace developers what would be the new way to
>> > retrieve frontend stats, as we can simply say that DVBS2API features
>> > superseeds the equivalent DVB v3 ioctls.
>>
>> As i have noted above, the statistics related calls have a meaning, if and
>> only if it is hanled very fast and properly with no caching. Having a
>> genarlized datastructure to handle this event, breaks up the whole meaning
>> of having this call itself in this position.
>>
>> What an API generally does is to make things generalized. When one makes
>> things "the most generalized way" an overhead is also associated with it in
>> terms of performance. If things were possible, i would directly read from
>> memory from an application from the hardware itself for processing data in
>> such a scenario, rather than to make things very generalized. This is an
>> area where concepts like data caching can be ruled out completely. We are
>> already at a disadvantage with the kernel driver doing a copy_to_user
>> itself. Ideally, a memory mapped to userspace would have been the ideal
>> thing over here.
>>
>> It is just the question of having yet another set of statistics calls that
>> handles the information properly or not.
>
> Hi Manu,
>
> thanks for the detailed explanation of your point. Actually I am not
> completely familiar with how the S2API calls are handled internally. Still are
> there any proven measurements about the timeframe the calls are being executed
> within? - I absolutely see that reading signal statistics is a time critical
> process, at least it is important to be able to assign the data to a specific
> moment so it can be interpreted in conjunction with the data which is being
> received in that moment.


Not only is it time critical, but it should also be "atomic", ie it
should be all in one go, ie one single snapshot of an event, not
events bunched together serially. Things wont seem that "atomic" on a
system with a large load. Latency will have a significant effect on
the statistics (values) read back, since it is again disjoint events.


> If this would be the only point where the delay is important one might be able
> to overcome this by adding timestamps into the retrieved data, although I am
> not sure if this would be feasible at all. Anyway having a very good and low-
> delay statistics approach would allow realtime applications like sat-finders
> to be implemented (provided, the hardware delivers good enough data).
> This would let me vote for a direct IOCTL approach to gain the best possible
> performance.

Time stamping would be helpful, prior to any processing by the library
such that the time overhead for the calculations is offset, but that
can be really useful within the same library alone. I can't imagine
how time stamping can be helpful to result a low latency.

If you don't have a low latency, Consider this (even when you are able
to ignore the statistics for any general processing, on the thought
that "i can always live with those errors and i always had"):

The error fedback into the loop for a sat positioner/rotor. The final
calculated position will never be the actual position that you wanted
the antenna to be at a certain location. The problem would be made
worser by the different rotor speeds as well, to add to the nightmare.

With the V5 operation, you bunch operations together in a serial
manner, it is atomic to the sense that it happens or doesn't happen,
but it is not atomic to the sense of any particular time frame. You
just keep your fingers crossed that the CPU executes the event in the
shortest frame. This won't hold good in all cases when there is a high
latency on the system when there is a significant load.

eg: You can imagine an IPTV headend streaming data, with a small CPU
with multiple tuners and trying to compensate the offset that's
introduced.

Still worser situation: imagine a gyro stabilized setup, where the
base itself is not that stationary.


> Treating signal statistics as a seperate approach from configuring the
> frontend would be tolerable for me, so that it could use a different API-
> approach than the tuning, which goes with S2API.

Some other points to be considered:

* As of now, the get/set interface is not used for any signal statistics

* Even if one prefers to normalize all parameters into one single
standard, even then you wouldn't land with a get/set interface.

* The generic get/set interface is good when you have an unknown set
of parameters, such that one can keep adding in parameters.  Eg: most
people favoured this approach when we had a larger set of modulations/
error correction and other parameters.

For the case what we have currently, we do not have such a varied set
of parameters to consider.


Regards,
Manu

Re: Details about DVB frontend API

[Julian Scheel]

Manu Abraham schrieb
> Not only is it time critical, but it should also be "atomic", ie it
> should be all in one go, ie one single snapshot of an event, not
> events bunched together serially. Things wont seem that "atomic" on a
> system with a large load. Latency will have a significant effect on
> the statistics (values) read back, since it is again disjoint events.
>   
Right, the values should be treatened as a unique unit...
> Time stamping would be helpful, prior to any processing by the library
> such that the time overhead for the calculations is offset, but that
> can be really useful within the same library alone. I can't imagine
> how time stamping can be helpful to result a low latency.
>   

No, timestamping would of course not be helpful for reducing the 
latency, it would just allow to correctly interpret values if they are 
delayed. So that you won't assume the values you receive can be taken as 
proven for the current moment.

> If you don't have a low latency, Consider this (even when you are able
> to ignore the statistics for any general processing, on the thought
> that "i can always live with those errors and i always had"):
>
> The error fedback into the loop for a sat positioner/rotor. The final
> calculated position will never be the actual position that you wanted
> the antenna to be at a certain location. The problem would be made
> worser by the different rotor speeds as well, to add to the nightmare.
>
> With the V5 operation, you bunch operations together in a serial
> manner, it is atomic to the sense that it happens or doesn't happen,
> but it is not atomic to the sense of any particular time frame. You
> just keep your fingers crossed that the CPU executes the event in the
> shortest frame. This won't hold good in all cases when there is a high
> latency on the system when there is a significant load.
>
> eg: You can imagine an IPTV headend streaming data, with a small CPU
> with multiple tuners and trying to compensate the offset that's
> introduced.
>
> Still worser situation: imagine a gyro stabilized setup, where the
> base itself is not that stationary.
>   

Ok, thanks for the details about how V5 API deals with this. Indeed this 
is a major issue one has to think of when talking about signal statistics.

> Some other points to be considered:
> * As of now, the get/set interface is not used for any signal statistics
>
> * Even if one prefers to normalize all parameters into one single
> standard, even then you wouldn't land with a get/set interface.
>
> * The generic get/set interface is good when you have an unknown set
> of parameters, such that one can keep adding in parameters.  Eg: most
> people favoured this approach when we had a larger set of modulations/
> error correction and other parameters.
>
> For the case what we have currently, we do not have such a varied set
> of parameters to consider.

Right, the situation about the parameters which will be requested in 
terms of signal statistics should not change for future frontend types, 
so it really should be a save approach to have a static API here. I have 
not yet done a very detailed look into your proposed patch, but I will 
do so tomorrow.
I really would like to see a reliable statistics API in v4l-dvb soon.

Regards,
Julian

Re: Details about DVB frontend API

[VDR User]

No activity in this thread for 2 weeks now.  Has there been any progress?

Regards.

Re: Details about DVB frontend API

[Michael Krufky]

On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
> No activity in this thread for 2 weeks now.  Has there been any progress?

I think I speak on behalf of most LinuxTV developers, when I say that
nobody wants to spend their free personal time working on something
that might get shot down with such controversy.

I have stated that I like Manu's proposal, but I would prefer that the
get_property (s2api) interface were used, because it totally provides
an interface that is sufficient for this feature.

Manu and I agree that these values should all be read at once.

I think we all (except Mauro) agree that the behavior within the
driver should fetch all statistics at once and return it to userspace
as a single structure with all the information as it all relates to
each other.

Furthermore, I think we all know that we cant just remove the current
structures, and we should do something to normalize the current
reporting values.

The longer this thread gets, the less likely anybody is to do anything about it.

Let me state my opinion again:

I would like to see a solution merged, and I think Manu's solution is
reasonable, although it may be complicated -- if all drivers are
updated to support it, then it will all be worth it.  The question is,
*will* all drivers update to support this?  I don't know.

We have the S2API's set / get property API -- In my opinion, we should
use this API to fetch statistic information and have it return a
single atomic structure.  Applications can use only the information
that they're interested in.

In the meanwhile, as a SEPARATE PROJECT, we should do something to
standardize the values reported by the CURRENT API across the entire
subsystem.  This should not be confused with Manu's initiative to
create a better API -- we cant remove the current API, but it should
be standardized.

I volunteer to work on the standardization of the CURRENT Api, and I
am all for seeing a new API introduced for better statistical
reporting, provided that the get property method is used as an
interface, rather than adding new ioctls.  However, if we add a new
API, we haev to make sure that all the current drivers are updated to
support it -- do we have all the information that we need for this?
Do we have the manpower and the drive to get it done?

My urge to do this work is a strong urge, but I have no desire to do
this if people want to continue arguing about it... In the meanwhile,
I am working on new drivers for new devices, and this is *much* more
interesting that worrying about how strong a signal is for a device
that already works.

When you folks stop talking about this, that's when I will push the
trees containing all the work that I've done already thus far -- we
need to standardize the current API, and that has nothing to do with
Manu's proposal.

We should not confuse standardization the current reporting units with
the introduction of a new API -- both should be done, but the more
arguing there is about it, the less of a chance that anybody will
volunteer their own time to work on it.

...and just to clarify -- I think I said it twice already, but
repeating again -- I (mostly) like Manu's proposal, but if we cant
update the drivers to support it, then is it worth the trouble?

Regards,

Mike Krufky

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Michael Krufky wrote:
> On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
> I have stated that I like Manu's proposal, but I would prefer that the
> get_property (s2api) interface were used, because it totally provides
> an interface that is sufficient for this feature.
> 
> Manu and I agree that these values should all be read at once.
> 
> I think we all (except Mauro) agree that the behavior within the
> driver should fetch all statistics at once and return it to userspace
> as a single structure with all the information as it all relates to
> each other.

You're contradicting yourself: by using S2API, the userspace API won't
be using a single structure, since S2API will break them into pairs of
attributes/values.

Nothing limits that the in-kernel API will group those values into a struct,
but the internal API should be smart enough to not return to userspace
the values that weren't requested by the call.

Cheers,
Mauro.

Re: Details about DVB frontend API

[Michael Krufky]

On Sat, Dec 5, 2009 at 12:30 PM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> Michael Krufky wrote:
>> On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
>> I have stated that I like Manu's proposal, but I would prefer that the
>> get_property (s2api) interface were used, because it totally provides
>> an interface that is sufficient for this feature.
>>
>> Manu and I agree that these values should all be read at once.
>>
>> I think we all (except Mauro) agree that the behavior within the
>> driver should fetch all statistics at once and return it to userspace
>> as a single structure with all the information as it all relates to
>> each other.
>
> You're contradicting yourself: by using S2API, the userspace API won't
> be using a single structure, since S2API will break them into pairs of
> attributes/values.

Incorrect.  Userspace would issue a get_property call and kernelspace
would return a block of key/value pairs.

> Nothing limits that the in-kernel API will group those values into a struct,
> but the internal API should be smart enough to not return to userspace
> the values that weren't requested by the call.

The call should be generic, something like get_property_signalstats
...  Kernelspace should return all related information, and userspace
should pick out what it needs.

-Mike

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Michael Krufky wrote:
> On Sat, Dec 5, 2009 at 12:30 PM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
>> Michael Krufky wrote:
>>> On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
>>> I have stated that I like Manu's proposal, but I would prefer that the
>>> get_property (s2api) interface were used, because it totally provides
>>> an interface that is sufficient for this feature.
>>>
>>> Manu and I agree that these values should all be read at once.
>>>
>>> I think we all (except Mauro) agree that the behavior within the
>>> driver should fetch all statistics at once and return it to userspace
>>> as a single structure with all the information as it all relates to
>>> each other.
>> You're contradicting yourself: by using S2API, the userspace API won't
>> be using a single structure, since S2API will break them into pairs of
>> attributes/values.
> 
> Incorrect.  Userspace would issue a get_property call and kernelspace
> would return a block of key/value pairs.

If userspace does a call with space for just one key/value pair, where do
you expect to store the other key/value pairs?

Mauro.

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Michael Krufky wrote:
> On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
>> No activity in this thread for 2 weeks now.  Has there been any progress?

> I have stated that I like Manu's proposal, but I would prefer that the
> get_property (s2api) interface were used, because it totally provides
> an interface that is sufficient for this feature.

I've ported Manu's proposal to S2API way of handling it. It is just compiled
only. I haven't test it yet on a real driver.

Comments?

---

Add support for frontend statistics via S2API

The current DVB V3 API to handle statistics has two issues:
	- Retrieving several values can't be done atomically;
	- There's no indication about scale information.

This patch solves those two issues by adding a group of S2API
that handles the needed statistics operations. It basically ports the
proposal of Manu Abraham <abraham.manu@gmail.com> To S2API.

As the original patch, both of the above issues were addressed.

In order to demonstrate the changes on an existing driver for the new API, I've
implemented it at the cx24123 driver.

There are some advantages of using this approach over using the static structs
of the original proposal:
	- userspace can select an arbitrary number of parameters on his get request;
	- the latency to retrieve just one parameter is lower than retrieving
several parameters. On the cx24123 example, if user wants just signal strength,
the latency is the same as reading one register via i2c bus. If using the original
proposal, the latency would be 6 times worse, since you would need to get 3 properties
at the same time;
	- the latency for reading all 3 parameters at the same time is equal to
the latency of the original proposal;
	- if newer statistics parameters will be needed in the future, it is just
a matter of adding additional S2API command/value pairs;
	- the DVB V3 calls can be easily implemented as a call to the new get_stats ops,
without adding extra latency time.

Thanks to Manu Abraham <abraham.manu@gmail.com> for his initial proposal.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>


diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -975,6 +975,16 @@ static struct dtv_cmds_h dtv_cmds[] = {
 	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
 	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
 	_DTV_CMD(DTV_HIERARCHY, 0, 0),
+
+	/* Statistics API */
+	_DTV_CMD(DTV_FE_QUALITY, 0, 0),
+	_DTV_CMD(DTV_FE_QUALITY_UNIT, 0, 0),
+	_DTV_CMD(DTV_FE_STRENGTH, 0, 0),
+	_DTV_CMD(DTV_FE_STRENGTH_UNIT, 0, 0),
+	_DTV_CMD(DTV_FE_ERROR, 0, 0),
+	_DTV_CMD(DTV_FE_ERROR_UNIT, 0, 0),
+	_DTV_CMD(DTV_FE_SIGNAL, 0, 0),
+	_DTV_CMD(DTV_FE_SIGNAL_UNIT, 0, 0),
 };
 
 static void dtv_property_dump(struct dtv_property *tvp)
@@ -1203,16 +1213,59 @@ static int dvb_frontend_ioctl_legacy(str
 static int dvb_frontend_ioctl_properties(struct inode *inode, struct file *file,
 			unsigned int cmd, void *parg);
 
+static int dtv_property_prepare_get_stats(struct dvb_frontend *fe,
+				    struct dtv_property *tvp,
+				    struct inode *inode, struct file *file)
+{
+	switch (tvp->cmd) {
+	case DTV_FE_QUALITY:
+		fe->dtv_property_cache.need_stats |= FE_NEED_QUALITY;
+		break;
+	case DTV_FE_QUALITY_UNIT:
+		fe->dtv_property_cache.need_stats |= FE_NEED_QUALITY_UNIT;
+		break;
+	case DTV_FE_STRENGTH:
+		fe->dtv_property_cache.need_stats |= FE_NEED_STRENGTH;
+		break;
+	case DTV_FE_STRENGTH_UNIT:
+		fe->dtv_property_cache.need_stats |= FE_NEED_STRENGTH_UNIT;
+		break;
+	case DTV_FE_ERROR:
+		fe->dtv_property_cache.need_stats |= FE_NEED_ERROR;
+		break;
+	case DTV_FE_ERROR_UNIT:
+		fe->dtv_property_cache.need_stats |= FE_NEED_ERROR_UNIT;
+		break;
+	case DTV_FE_SIGNAL:
+		fe->dtv_property_cache.need_stats |= FE_NEED_SIGNAL;
+		break;
+	case DTV_FE_SIGNAL_UNIT:
+		fe->dtv_property_cache.need_stats |= FE_NEED_SIGNAL_UNIT;
+		break;
+	case DTV_FE_UNC:
+		fe->dtv_property_cache.need_stats |= FE_NEED_SIGNAL;
+		break;
+	case DTV_FE_UNC_UNIT:
+		fe->dtv_property_cache.need_stats |= FE_NEED_SIGNAL_UNIT;
+		break;
+	default:
+		return 1;
+	};
+
+	return 0;
+}
+
 static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct dtv_property *tvp,
-				    struct inode *inode, struct file *file)
+				    struct inode *inode, struct file *file,
+				    int need_get_ops)
 {
 	int r = 0;
 
 	dtv_property_dump(tvp);
 
 	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.get_property)
+	if (fe->ops.get_property && need_get_ops)
 		r = fe->ops.get_property(fe, tvp);
 
 	if (r < 0)
@@ -1329,6 +1382,38 @@ static int dtv_property_process_get(stru
 	case DTV_ISDBS_TS_ID:
 		tvp->u.data = fe->dtv_property_cache.isdbs_ts_id;
 		break;
+
+	/* Quality measures */
+	case DTV_FE_QUALITY:
+		tvp->u.data = fe->dtv_property_cache.quality;
+		break;
+	case DTV_FE_QUALITY_UNIT:
+		tvp->u.data = fe->dtv_property_cache.quality_unit;
+		break;
+	case DTV_FE_STRENGTH:
+		tvp->u.data = fe->dtv_property_cache.strength;
+		break;
+	case DTV_FE_STRENGTH_UNIT:
+		tvp->u.data = fe->dtv_property_cache.strength_unit;
+		break;
+	case DTV_FE_ERROR:
+		tvp->u.data = fe->dtv_property_cache.error;
+		break;
+	case DTV_FE_ERROR_UNIT:
+		tvp->u.data = fe->dtv_property_cache.error_unit;
+		break;
+	case DTV_FE_SIGNAL:
+		tvp->u.data = fe->dtv_property_cache.signal;
+		break;
+	case DTV_FE_SIGNAL_UNIT:
+		tvp->u.data = fe->dtv_property_cache.signal_unit;
+		break;
+	case DTV_FE_UNC:
+		tvp->u.data = fe->dtv_property_cache.signal;
+		break;
+	case DTV_FE_UNC_UNIT:
+		tvp->u.data = fe->dtv_property_cache.signal_unit;
+		break;
 	default:
 		r = -1;
 	}
@@ -1527,7 +1612,7 @@ static int dvb_frontend_ioctl_properties
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_frontend *fe = dvbdev->priv;
-	int err = 0;
+	int err = 0, need_get_ops;
 
 	struct dtv_properties *tvps = NULL;
 	struct dtv_property *tvp = NULL;
@@ -1591,8 +1676,29 @@ static int dvb_frontend_ioctl_properties
 			goto out;
 		}
 
+		/*
+		* Do all get operations at once, instead of handling them
+		* individually
+		*/
+		need_get_ops = 0;
+		fe->dtv_property_cache.need_stats = 0;
+		for (i = 0; i < tvps->num; i++)
+			need_get_ops += dtv_property_prepare_get_stats(fe,
+							 tvp + i, inode, file);
+
+		if (!fe->dtv_property_cache.need_stats) {
+			need_get_ops++;
+		} else {
+			if (fe->ops.get_stats) {
+				err = fe->ops.get_stats(fe);
+				if (err < 0)
+					return err;
+			}
+		}
+
 		for (i = 0; i < tvps->num; i++) {
-			(tvp + i)->result = dtv_property_process_get(fe, tvp + i, inode, file);
+			(tvp + i)->result = dtv_property_process_get(fe,
+					tvp + i, inode, file, need_get_ops);
 			err |= (tvp + i)->result;
 		}
 
diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h
@@ -304,6 +304,7 @@ struct dvb_frontend_ops {
 
 	int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
 	int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
+	int (*get_stats)(struct dvb_frontend* fe);
 };
 
 #define MAX_EVENT 8
@@ -358,6 +359,32 @@ struct dtv_frontend_properties {
 
 	/* ISDB-T specifics */
 	u32			isdbs_ts_id;
+
+	/* Statistics group */
+
+#define FE_NEED_QUALITY		(1 << 0)
+#define FE_NEED_QUALITY_UNIT	(1 << 1)
+#define FE_NEED_STRENGTH	(1 << 2)
+#define FE_NEED_STRENGTH_UNIT	(1 << 3)
+#define FE_NEED_ERROR		(1 << 4)
+#define FE_NEED_ERROR_UNIT	(1 << 5)
+#define FE_NEED_UNC		(1 << 6)
+#define FE_NEED_UNC_UNIT	(1 << 7)
+#define FE_NEED_SIGNAL		(1 << 6)
+#define FE_NEED_SIGNAL_UNIT	(1 << 7)
+	int			need_stats;
+
+	u32			quality;
+	u32			strength;
+	u32			error;
+	u32			unc;
+	u32			signal;
+
+	enum fecap_quality_params	quality_unit;
+	enum fecap_scale_params		strength_unit;
+	enum fecap_error_params		error_unit;
+	enum fecap_unc_params		unc_unit;
+	enum fecap_scale_params		signal_unit;
 };
 
 struct dvb_frontend {
diff --git a/linux/drivers/media/dvb/frontends/cx24123.c b/linux/drivers/media/dvb/frontends/cx24123.c
--- a/linux/drivers/media/dvb/frontends/cx24123.c
+++ b/linux/drivers/media/dvb/frontends/cx24123.c
@@ -890,6 +890,66 @@ static int cx24123_read_status(struct dv
 	return 0;
 }
 
+static int cx24123_get_stats(struct dvb_frontend* fe)
+{
+	struct cx24123_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *prop = &fe->dtv_property_cache;
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_STRENGTH) {
+		/* larger = better */
+		prop->strength = cx24123_readreg(state, 0x3b) << 8;
+			dprintk("Signal strength = %d\n", prop->strength);
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_STRENGTH;
+	}
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_STRENGTH_UNIT) {
+		/* larger = better */
+		prop->strength_unit = FE_SCALE_UNKNOWN;
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_STRENGTH_UNIT;
+	}
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_ERROR) {
+		/* The true bit error rate is this value divided by
+		the window size (set as 256 * 255) */
+		prop->error = ((cx24123_readreg(state, 0x1c) & 0x3f) << 16) |
+			       (cx24123_readreg(state, 0x1d) << 8 |
+			       cx24123_readreg(state, 0x1e));
+
+		dprintk("BER = %d\n", prop->error);
+
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_ERROR;
+	}
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_ERROR_UNIT) {
+		/* larger = better */
+		prop->strength_unit = FE_ERROR_BER;
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_ERROR_UNIT;
+	}
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_QUALITY) {
+		/* Inverted raw Es/N0 count, totally bogus but better than the
+		   BER threshold. */
+		prop->quality = 65535 - (((u16)cx24123_readreg(state, 0x18) << 8) |
+					  (u16)cx24123_readreg(state, 0x19));
+
+		dprintk("read S/N index = %d\n", prop->quality);
+
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_QUALITY;
+	}
+
+	if (fe->dtv_property_cache.need_stats & FE_NEED_QUALITY_UNIT) {
+		/* larger = better */
+		prop->strength_unit = FE_QUALITY_EsNo;
+		fe->dtv_property_cache.need_stats &= ~FE_NEED_QUALITY_UNIT;
+	}
+
+	/* Check if userspace requested a parameter that we can't handle*/
+	if (fe->dtv_property_cache.need_stats)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * Configured to return the measurement of errors in blocks,
  * because no UCBLOCKS value is available, so this value doubles up
@@ -897,43 +957,30 @@ static int cx24123_read_status(struct dv
  */
 static int cx24123_read_ber(struct dvb_frontend *fe, u32 *ber)
 {
-	struct cx24123_state *state = fe->demodulator_priv;
+	fe->dtv_property_cache.need_stats = FE_NEED_ERROR;
+	cx24123_get_stats(fe);
 
-	/* The true bit error rate is this value divided by
-	   the window size (set as 256 * 255) */
-	*ber = ((cx24123_readreg(state, 0x1c) & 0x3f) << 16) |
-		(cx24123_readreg(state, 0x1d) << 8 |
-		 cx24123_readreg(state, 0x1e));
-
-	dprintk("BER = %d\n", *ber);
-
+	*ber = fe->dtv_property_cache.error;
 	return 0;
 }
 
 static int cx24123_read_signal_strength(struct dvb_frontend *fe,
 	u16 *signal_strength)
 {
-	struct cx24123_state *state = fe->demodulator_priv;
-
-	/* larger = better */
-	*signal_strength = cx24123_readreg(state, 0x3b) << 8;
-
-	dprintk("Signal strength = %d\n", *signal_strength);
-
+	fe->dtv_property_cache.need_stats = FE_NEED_STRENGTH;
+	cx24123_get_stats(fe);
+	*signal_strength = fe->dtv_property_cache.strength;
 	return 0;
 }
 
+
 static int cx24123_read_snr(struct dvb_frontend *fe, u16 *snr)
 {
-	struct cx24123_state *state = fe->demodulator_priv;
-
 	/* Inverted raw Es/N0 count, totally bogus but better than the
-	   BER threshold. */
-	*snr = 65535 - (((u16)cx24123_readreg(state, 0x18) << 8) |
-			 (u16)cx24123_readreg(state, 0x19));
-
-	dprintk("read S/N index = %d\n", *snr);
-
+		BER threshold. */
+	fe->dtv_property_cache.need_stats = FE_NEED_QUALITY;
+	cx24123_get_stats(fe);
+	*snr = fe->dtv_property_cache.quality;
 	return 0;
 }
 
@@ -1174,6 +1221,7 @@ static struct dvb_frontend_ops cx24123_o
 	.set_voltage = cx24123_set_voltage,
 	.tune = cx24123_tune,
 	.get_frontend_algo = cx24123_get_algo,
+	.get_stats = cx24123_get_stats,
 };
 
 MODULE_DESCRIPTION("DVB Frontend module for Conexant " \
diff --git a/linux/include/linux/dvb/frontend.h b/linux/include/linux/dvb/frontend.h
--- a/linux/include/linux/dvb/frontend.h
+++ b/linux/include/linux/dvb/frontend.h
@@ -304,7 +304,19 @@ struct dvb_frontend_event {
 
 #define DTV_ISDBS_TS_ID		42
 
-#define DTV_MAX_COMMAND				DTV_ISDBS_TS_ID
+/* Quality parameters */
+#define DTV_FE_QUALITY		43
+#define DTV_FE_QUALITY_UNIT	44
+#define DTV_FE_STRENGTH		45
+#define DTV_FE_STRENGTH_UNIT	46
+#define DTV_FE_ERROR		47
+#define DTV_FE_ERROR_UNIT	48
+#define DTV_FE_SIGNAL		49
+#define DTV_FE_SIGNAL_UNIT	50
+#define DTV_FE_UNC		51
+#define DTV_FE_UNC_UNIT		52
+
+#define DTV_MAX_COMMAND				DTV_FE_UNC_UNIT
 
 typedef enum fe_pilot {
 	PILOT_ON,
@@ -338,6 +350,46 @@ typedef enum fe_delivery_system {
 	SYS_DAB,
 } fe_delivery_system_t;
 
+/* Frontend General Statistics
+ * General parameters
+ * FE_*_UNKNOWN:
+ *	Parameter is unknown to the frontend and doesn't really
+ *	make any sense for an application.
+ *
+ * FE_*_RELATIVE:
+ *	Parameter is relative on the basis of a ceil - floor basis
+ *	Format is based on empirical test to determine
+ *	the floor and ceiling values. This format is exactly the
+ *	same format as the existing statistics implementation.
+ */
+
+enum fecap_quality_params {
+	FE_QUALITY_UNKNOWN		= 0,
+	FE_QUALITY_SNR			= (1 <<  0),
+	FE_QUALITY_CNR			= (1 <<  1),
+	FE_QUALITY_EsNo			= (1 <<  2),
+	FE_QUALITY_EbNo			= (1 <<  3),
+	FE_QUALITY_RELATIVE		= (1 << 31),
+};
+
+enum fecap_scale_params {
+	FE_SCALE_UNKNOWN		= 0,
+	FE_SCALE_dB			= (1 <<  0),
+	FE_SCALE_RELATIVE		= (1 << 31),
+};
+
+enum fecap_error_params {
+	FE_ERROR_UNKNOWN		= 0,
+	FE_ERROR_BER			= (1 <<  0),
+	FE_ERROR_PER			= (1 <<  1),
+	FE_ERROR_RELATIVE		= (1 << 31),
+};
+
+enum fecap_unc_params {
+	FE_UNC_UNKNOWN			= 0,
+	FE_UNC_RELATIVE			= (1 << 31),
+};
+
 struct dtv_cmds_h {
 	char	*name;		/* A display name for debugging purposes */
 

Re: Details about DVB frontend API

[Mauro Carvalho Chehab]

Mauro Carvalho Chehab wrote:
> Michael Krufky wrote:
>> On Fri, Dec 4, 2009 at 3:02 PM, VDR User <user.vdr@gmail.com> wrote:
>>> No activity in this thread for 2 weeks now.  Has there been any progress?
> 
>> I have stated that I like Manu's proposal, but I would prefer that the
>> get_property (s2api) interface were used, because it totally provides
>> an interface that is sufficient for this feature.
> 
> I've ported Manu's proposal to S2API way of handling it. It is just compiled
> only. I haven't test it yet on a real driver.
> 
> Comments?
> 
> ---
> 
> Add support for frontend statistics via S2API
> 
> The current DVB V3 API to handle statistics has two issues:
> 	- Retrieving several values can't be done atomically;
> 	- There's no indication about scale information.
> 
> This patch solves those two issues by adding a group of S2API
> that handles the needed statistics operations. It basically ports the
> proposal of Manu Abraham <abraham.manu@gmail.com> To S2API.
> 
> As the original patch, both of the above issues were addressed.
> 
> In order to demonstrate the changes on an existing driver for the new API, I've
> implemented it at the cx24123 driver.
> 
> There are some advantages of using this approach over using the static structs
> of the original proposal:
> 	- userspace can select an arbitrary number of parameters on his get request;
> 	- the latency to retrieve just one parameter is lower than retrieving
> several parameters. On the cx24123 example, if user wants just signal strength,
> the latency is the same as reading one register via i2c bus. If using the original
> proposal, the latency would be 6 times worse, since you would need to get 3 properties
> at the same time;
> 	- the latency for reading all 3 parameters at the same time is equal to
> the latency of the original proposal;
> 	- if newer statistics parameters will be needed in the future, it is just
> a matter of adding additional S2API command/value pairs;
> 	- the DVB V3 calls can be easily implemented as a call to the new get_stats ops,
> without adding extra latency time.

In time:

I only wrote the get callback. It could be interesting to implement also the set callback
for the DTV_FE*_UNIT parameters if there are some cases where the same driver can provide
a different set of units/parameters. This way, it is possible for userspace to
negotiate what parameter type he wants, on such drivers.
> 
> Thanks to Manu Abraham <abraham.manu@gmail.com> for his initial proposal.
> 

Cheers,
Mauro.

Re: Details about DVB frontend API

[Steven Toth]

> We did discuss this briefly during the v4l-dvb mini-summit and I know Mike
> Krufky knew what to do about this, but for the life of me I can't remember
> what it was. I should have made a note of it...
>
> Mike, can you refresh my memory?

You are correct Hans. Mike has some patches that begin to address this
for the ATSC products, standardizing some of the unit measurements.
Very clean, easy to review and merge.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: Details about DVB frontend API

[Hans Verkuil]

> Em Thu, 22 Oct 2009 21:13:30 +0200
> Jean Delvare <khali@linux-fr.org> escreveu:
>
>> Hi folks,
>>
>> I am looking for details regarding the DVB frontend API. I've read
>> linux-dvb-api-1.0.0.pdf, it roughly explains what the FE_READ_BER,
>> FE_READ_SNR, FE_READ_SIGNAL_STRENGTH and FE_READ_UNCORRECTED_BLOCKS
>> commands return, however it does not give any information about how the
>> returned values should be interpreted (or, seen from the other end, how
>> the frontend kernel drivers should encode these values.) If there
>> documentation available that would explain this?
>>
>> For example, the signal strength. All I know so far is that this is a
>> 16-bit value. But then what? Do greater values represent stronger
>> signal or weaker signal? Are 0x0000 and 0xffff special values? Is the
>> returned value meaningful even when FE_HAS_SIGNAL is 0? When
>> FE_HAS_LOCK is 0? Is the scale linear, or do some values have
>> well-defined meanings, or is it arbitrary and each driver can have its
>> own scale? What are the typical use cases by user-space application for
>> this value?
>>
>> That's the kind of details I'd like to know, not only for the signal
>> strength, but also for the SNR, BER and UB. Without this information,
>> it seems a little difficult to have consistent frontend drivers.
>
> We all want to know about that ;)
>
> Seriously, the lack of a description of the meaning of the ranges for
> those
> read values were already widely discussed at LMML and at the legacy dvb
> ML.
> We should return this discussion again and decide what would be the better
> way to describe those values.
>
> My suggestion is that someone summarize the proposals we had and give some
> time
> for people vote. After that, we just commit the most voted one, and commit
> the
> patches for it. A pending question that should also be discussed is what
> we will
> do with those dvb devices where we simply don't know what scale it uses.
> There
> are several of them.
>
> Btw, the new official documentation is the media infrastructure docbook
> that
> can be found at the Kernel and at:
> 	http://linuxtv.org/downloads/v4l-dvb-apis
>
> This covers both DVB and V4L API's.

We did discuss this briefly during the v4l-dvb mini-summit and I know Mike
Krufky knew what to do about this, but for the life of me I can't remember
what it was. I should have made a note of it...

Mike, can you refresh my memory?

Thanks,

         Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom