[linux-dvb] DVB-S2 API vs. HVR4000: When?

Steven Toth stoth at linuxtv.org
Mon Nov 5 17:32:34 CET 2007


Johannes Stezenbach wrote:
> On Fri, Nov 02, 2007, Steven Toth wrote:
>> The design goals for me are:
>>
>> 1) We stop trying to predict what the API will need in 5 years, and focus 
>> on building a very simplistic ioctl API for getting and setting generic 
>> frontend properties, it should be basic enough to deal with any new 
>> modulation type (or advanced tuner) that we know of today.
> 
> good one
> 
>> 2) We change the tuning mechanism from being (mostly) atomic (SET_FRONTEND) 
>> to a looser command/sequence definition. (Thereby stepping away from fixed 
>> structs that define the ABI). With two new ioctls 
>> (get_property/set_property) and a simple property struct which specifies 
>> how generic types are passed to/from the kernel.
> 
> no so good
> 
>> 3) A userland library _may_ offer a number of helper functions to simplify 
>> in terms of applications development, turning this:
> 
> many people seem to hate ALSA, there's a lesson to be learned here
> 
>> command.id = BEGIN_TRANSACTION
>> ioctl(SET_PROPERTY, &command);
>>
>> command.id = SET_MODULATION
>> command.arg = 8VSB
>> ioctl(SET_PROPERTY, &command);
>>
>> command.id = SET_FREQUENCY
>> command.arg = 591250000
>> ioctl(SET_PROPERTY, &command);
>>
>> command.id = VALIDATE_TRANSACTION
>> ioctl(SET_PROPERTY, &command);
>>
>> command.id = END_TRANSACTION
>> ioctl(SET_PROPERTY, &command);
>>
>> Into a more human readable form like this:
>>
>> int tune_8vsb(frequency);
>>
>> It's a basic approach, we trade off multiple ioctls (at a very low rate) 
>> entering the kernel, for a very flexible tuning approach to frontend 
>> control.
> 
> Once you have those tag/value pairs, you could batch them
> together in an array and pass them down in one ioctl. This
> avoids the ugly transaction stuff and keeps it atomic.
> And I think it wouldn't look too ugly in user code, e.g.:
> 
> 	struct dvb_tuning_param p[3] = {
> 		{ .id = MODULATION, .val = MOD_8VSB },
> 		{ .id = FREQUENCY,  .val = 591250000 },
> 		{ .id = 0 }
> 	};
> 	ioctl(fd, DVB_TUNE, p);


You can't reliably pass in variably length arrays into the kernel, or 
none of the other kernel wide drivers do, they need to be fixed length 
for sanity reasons. That being said, I have a newly defined type which 
represents an array enabling 16 messages to be passed in a single 
transaction.

IE. It's no limited by array size. ioctls can be chained together to 
form atomic tuning operations and are not bound by length, should any 
future hardware require radically different parameters/operations.

Here's the message set I've using with QAM for example:

tv_properties_t _qam_cmdseq = {
	{ .cmd = TV_SEQ_START },
	{ .cmd = TV_SET_FREQUENCY,  .u.data = 561000000 },
	{ .cmd = TV_SET_MODULATION, .u.data = QAM_AUTO },
	{ .cmd = TV_SET_INVERSION,  .u.data = INVERSION_AUTO },
	{ .cmd = TV_SET_BANDWIDTH,  .u.data = BANDWIDTH_AUTO },
	{ .cmd = TV_SEQ_COMPLETE },
	{ .cmd = 0 },
};

... and here's the ioctl structure in question from frontend.h

typedef struct {
         __u32 cmd;
         union {
                 __u32 data;
                 struct {
                         __u8 data[32];
                         __u32 len;
                 } buffer;
         } u;
} tv_property_t;

/* No more than 16 properties during any given ioctl */
typedef tv_property_t tv_properties_t[16];

#define FE_SET_PROPERTY            _IOW('o', 82, tv_properties_t)

I have other changes to frontend.h that I'll describe below, in answer 
to your other points.

Unrelated: It's important to note that the new API allows messages to be 
atomic individually, or grouped into transactions for a single atomic 
operation. One example is a single message to control DiSEqC, which in 
the current published API occurs immediately. The current API is 
completely atomic, and offers nothing for future hardware which may need 
a finer grain of control. (Think analog)

Hence, this API supports both mechanisms.


> 
> 
> But there's more work to do:
> - need for .val being variable lenght or a union of different types?

See the current union. It supports a generic u32 for passing enums or 
other values, and also a generic byte buffer (which can be chained with 
multiple messages and multiple ioctls via transactions grouping). This 
generic buffer is large enough to store the current DiSEqC message 
mechanisms, but scalable to be able to support any number of future 
messages with any message lengths, literally into multi megabytes if 
that's what is required.


> - extend existing enums or define new ones for MODULATION etc?

A mixture of both.

Where appropriate the current enums have been extended to support new 
modulation types, FECs etc. In more cases than not, this is the case.

Where appropriate new enums/type have been defined to support new user 
facing attributes (rolloff/pilot) being a classic case (although both 
current drivers detect Pilot automatically). This doesn't impact the 
ABI, but mechanisms like this (for future devices) show how flexible the 
new API can be.

The ioctls messages themselves (TV_SET_FREQUENCY etc) are also new 
enums, but the impact is small to frontend.h and retains full backwards 
compatibility for existing users. No ABI changes.


> - how to do FE_GET_FRONTEND

Interesting. I haven't looked into this mainly because my focus this 
weekend (on short notice) was to prove out basic tuning. Assuming the 
direction that's being taken so far is sound, then I'll look into this next.

With the new patches I have, QAM and ATSC are working cleanly and 100% 
reliably. These patches allow the old or new API to be used to tune and 
to query some basic tuner metrics (locked/sync etc).

Importantly, because the new API works independently of the current API, 
it can be used in combination as a stepping stone for applications that 
do not want to migrate overnight. That's an unwritten goal.

Here's the basic tuning mechanism, in snippet form:

tv_properties_t _qam_cmdseq = {
         { .cmd = TV_SEQ_START },
         { .cmd = TV_SET_FREQUENCY,  .u.data = 561000000 },
         { .cmd = TV_SET_MODULATION, .u.data = QAM_AUTO },
         { .cmd = TV_SET_INVERSION,  .u.data = INVERSION_AUTO },
         { .cmd = TV_SET_BANDWIDTH,  .u.data = BANDWIDTH_AUTO },
         { .cmd = TV_SEQ_COMPLETE },
         { .cmd = 0 },
};
ioctl(fefd, FE_SET_PROPERTY, &_qam_cmdseq));

As such FE_GET_FRONTEND continues to work 100% reliably for existing 
drivers, but I would also expect to have a TV_GET_FRONTEND style message 
to return a superset of data in a cleaner non-fixed-structured way.



> - etc.
> 
> I'm sure we could have endless debates over the details ;-)
> 
> I hope many others will comment which approach they like better,
> or which one they think will be ready for prime time sooner.

All feedback is appreciated.

> 
> Personally I don't care either way. I spent a lot of time discussing
> the "multiproto" API and I believe it could be ready soon with
> a few minor tweaks. OTOH this tag/value approach seems to be
> more flexible, but who knows how much work it'll take to complete.

Clearly, tuning for 8PSK would look something like this:

tv_properties_t _8psk_cmdseq = {
         { .cmd = TV_SEQ_START },
         { .cmd = TV_SET_FREQUENCY,  .u.data = 12051000 },
         { .cmd = TV_SET_MODULATION, .u.data = _8PSK },
         { .cmd = TV_SET_SYMBOLRATE, .u.data = 27500 },
         { .cmd = TV_SET_INNERFEC,   .u.data = FEC_AUTO },
         { .cmd = TV_SET_VOLTAGE,    .u.data = SEC_VOLTAGE_13 },
         { .cmd = TV_SET_INVERSION,  .u.data = INVERSION_AUTO },
         { .cmd = TV_SET_PILOT,      .u.data = PILOT_AUTO },
         { .cmd = TV_SET_ROLLOFF,    .u.data = ROLLOFF_AUTO },
         { .cmd = TV_SEQ_COMPLETE },
         { .cmd = 0 },
};
ioctl(fefd, FE_SET_PROPERTY, &_qam_cmdseq));

-- with some of these messages being optional, I have them here for 
reading purposes.

Lastly, a few things I want to mention.

1) I've made minor changes to dvb_core to interpret these messages and 
handle the ioctl. No changes have been made to the frontend drivers.

2) Adding support for s2 _inside_ the kernel will mean adding a single 
method to dvb_frontend_ops which allows the dvb_core to notify a new S2 
driver. The goal here is to minimize these changes also. I haven't 
demonstrated that code here, becuse I felt it was more important to show 
the impact to the userland API/ABI, knowing that DVB-S2 and other 
advanced types (including analog) would naturally fit well within this 
model.

3) This applies to all devs. I welcome all feedback, for sure the sytax 
might need some cleanup, but please don't shoot the idea down when it's 
only had 6-8 hours work of engineering behind it. It's proof of concept. 
It doesn't contain any of Manu's code so I don't feel bound politically 
or technically. I think given another 4-5 hours I could show the HVR4000 
working, and demonstrate many of the userland signal/statistic API's.

At this stage I'm looking for a "Yes, in principle we like the idea, but 
show me how you do feature XYZ" from other devs. At which point I'll 
flush out more code this would probably lead to an RFC for your approval.

- Steve











More information about the linux-dvb mailing list