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
_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
