On 11/5/07, Steven Toth <stoth@xxxxxxxxxxx> wrote:
> 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.
>
Hi Steven,
just have a look at the i2c-dev driver it passes a variable length to
the kernel.
Markus
> 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
> linux-dvb@xxxxxxxxxxx
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
>
--
http://www.couchsurfing.com/people/mrec
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
