Re: RFC: merge dvb_tuner_ops & tuner-core second try

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/27/07, Marcel Siegert <mws@xxxxxxxxxxx> wrote:
On Tuesday 27 February 2007, Markus Rechberger wrote:
> Hi guys,
>
> since I really want to get that xc3028 story done I propose to add
> some changes to the dvb framework which would allow to support loading
> tuner modules from v4l and dvb without having a real dependency of
> each other.
> The interface I aim at uses almost the same structure as dvb_tuner_ops.
>
> So to point it out more clearly:
>
> I'd like to replace or modify:
> struct dvb_tuner_ops {
>
>         struct dvb_tuner_info info;
>
>         int (*release)(struct dvb_frontend *fe);
>         int (*init)(struct dvb_frontend *fe);
>         int (*sleep)(struct dvb_frontend *fe);
>
>         /** This is for simple PLLs - set all parameters in one go. */
>         int (*set_params)(struct dvb_frontend *fe, struct
> dvb_frontend_parameters *p);
>
>         /** This is support for demods like the mt352 - fills out the
> supplied buffer with what to write. */
>         int (*calc_regs)(struct dvb_frontend *fe, struct
> dvb_frontend_parameters *p, u8 *buf, int buf_len);
>
>         int (*get_frequency)(struct dvb_frontend *fe, u32 *frequency);
>         int (*get_bandwidth)(struct dvb_frontend *fe, u32 *bandwidth);
>
> #define TUNER_STATUS_LOCKED 1
>         int (*get_status)(struct dvb_frontend *fe, u32 *status);
>
>         /** These are provided seperately from set_params in order to
> facilitate silicon
>          * tuners which require sophisticated tuning loops,
> controlling each parameter seperately. */
>         int (*set_frequency)(struct dvb_frontend *fe, u32 frequency);
>         int (*set_bandwidth)(struct dvb_frontend *fe, u32 bandwidth);
> };
>
> with:
> struct v4l_dvb_tuner {
>         /* wrapper */
>         void *priv; /* some privat data for internal use */
>         void *dev; /* v4l private data for tuner-core */
>         struct dvb_frontend *fe; /* dvb_frontend, for dvb only
> drivers, internal use */
>
>         int (*ioctl)(struct v4l_dvb_tuner *dev, int cmd, int arg);
>         struct tuner_info info;
>         int (*release)(struct v4l_dvb_tuner *dev);
>         int (*init)(struct v4l_dvb_tuner *dev);
>         int (*sleep)(struct v4l_dvb_tuner *dev);
>
>         /** This is for simple PLLs - set all parameters in one go. */
>         int (*set_params)(struct v4l_dvb_tuner *dev, struct
> tuner_parameters *p);
>
>         /** This is support for demods like the mt352 - fills out the
> supplied buffer with what to write. */
>         int (*calc_regs)(struct v4l_dvb_tuner *dev, struct
> tuner_parameters *p, u8 *buf, int buf_len);
>
>         int (*get_frequency)(struct v4l_dvb_tuner *dev, u32 *frequency);
>         int (*get_bandwidth)(struct v4l_dvb_tuner *dev, u32 *bandwidth);
>
> #define TUNER_STATUS_LOCKED 1
>         int (*get_status)(struct v4l_dvb_tuner *dev, u32 *status);
>
>         /** These are provided seperately from set_params in order to
> facilitate silicon
>          * tuners which require sophisticated tuning loops,
> controlling each parameter seperately. */
>         int (*set_frequency)(struct v4l_dvb_tuner *dev, u32 frequency);
>         int (*set_bandwidth)(struct v4l_dvb_tuner *dev, u32 bandwidth);
>
>         int  (*set_mode)(struct v4l_dvb_tuner *dev, struct
> tuner_parameters *params);
> };
>
> tuner_parameters is an extended version of dvb_frontend_params
>
> struct dvb_frontend_parameters {
>         __u32 frequency;     /* (absolute) frequency in Hz for
QAM/OFDM/ATSC */
>                              /* intermediate frequency in kHz for QPSK */
>         fe_spectral_inversion_t inversion;
>         union {
>                 struct dvb_qpsk_parameters qpsk;
>                 struct dvb_qam_parameters  qam;
>                 struct dvb_ofdm_parameters ofdm;
>                 struct dvb_vsb_parameters vsb;
>         } u;
> };
>
> vs:
>
> struct tuner_parameters {
>         __u32 frequency;     /* (absolute) frequency in Hz for
QAM/OFDM/ATSC */
>                              /* intermediate frequency in kHz for QPSK */
>         enum v4l2_tuner_type    type;
>         v4l2_std_id             std;
>         fe_spectral_inversion_t inversion;
>         union {
>                 struct dvb_qpsk_parameters qpsk;
>                 struct dvb_qam_parameters  qam;
>                 struct dvb_ofdm_parameters ofdm;
>                 struct dvb_vsb_parameters vsb;
>         } u;
> };
>
> for not breaking userspace we can use an internal converter for that
> format just like:
> #define V4L_OPS(i) ({ \
>         struct tuner_parameters __o; \
>         __o.frequency = i->frequency; \
>         __o.inversion = i->inversion; \
>         ....
>         &__o; \
> })
>
> this seems to be a good approach for hybrid devices
> dvb only silicon tuners could still access dvb_frontend internally,
> hybrid tuners have to avoid this since the structure since it wouldn't
> be fully initialized.
> So what do you guys think about that?
>
> I already have some code which does this and it works fine.
> So I'm looking for some feedback/suggestions here
>
> thanks,
> Markus
>
> _______________________________________________
> linux-dvb mailing list
> linux-dvb@xxxxxxxxxxx
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
>
hi,

as already told on irc (summary of one hour talk)

i am against this patch as this sort of thing could be implemented in the
same way
like e.g. saa7134

this implements your changes to be unneccessary.

best regards
marcel



Marcel you never had a look at the saa7134 so please only write about
things you really know about.

You mean lines like:
       /* setup tuner buffer */
       tuner_buf[0] = (tuner_frequency >> 8) & 0x7f;
       tuner_buf[1] = tuner_frequency & 0xff;
       tuner_buf[2] = 0xca;
       tuner_buf[3] = (cp << 5) | (filter << 3) | band;

       if (fe->ops.i2c_gate_ctrl)
               fe->ops.i2c_gate_ctrl(fe, 1);
       if (i2c_transfer(&dev->i2c_adap, &tuner_msg, 1) != 1) {
               printk("%s/dvb: could not write to tuner at addr:
0x%02x\n",dev->name, addr << 1);
               return -EIO;
       }
       msleep(1);
       return 0;
}

in saa7134-dvb.c .. then I say no thanks.
thanks,
Markus

_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux