Re: [PATCH] [media] uvcvideo: Enable VIDIOC_CREATE_BUFS

2014-02-04 Thread Hans Verkuil
On 02/05/2014 12:04 AM, Sylwester Nawrocki wrote:
> Hi,
> 
> On 02/03/2014 10:03 AM, Hans Verkuil wrote:
>> Hi Philipp, Laurent,
>>
>> On 02/02/2014 02:04 PM, Philipp Zabel wrote:
>>> On Sun, Feb 02, 2014 at 11:21:13AM +0100, Laurent Pinchart wrote:
 Hi Hans,

 On Friday 31 January 2014 09:43:00 Hans Verkuil wrote:
> I think you might want to add a check in uvc_queue_setup to verify the
> fmt that create_bufs passes. The spec says that: "Unsupported formats
> will result in an error". In this case I guess that the format basically
> should match the current selected format.
>
> I'm unhappy with the current implementations of create_bufs (see also this
> patch:
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg70796.html).
>
> Nobody is actually checking the format today, which isn't good.
>
> The fact that the spec says that the fmt field isn't changed by the driver
> isn't helping as it invalidated my patch from above, although that can be
> fixed.
>
> I need to think about this some more, but for this particular case you can
> just do a memcmp of the v4l2_pix_format against the currently selected
> format and return an error if they differ. Unless you want to support
> different buffer sizes as well?

 Isn't the whole point of VIDIOC_CREATE_BUFS being able to create buffers of
 different resolutions than the current active resolution ?
>>
>> Or just additional buffers with the same resolution (or really, the same 
>> size).
>>
>>> For that to work the driver in question would need to keep track of 
>>> per-buffer
>>> format and resolution, and not only of per-queue format and resolution.
>>>
>>> For now, would something like the following be enough? Unfortunately the uvc
>>> driver doesn't keep a v4l2_format around that we could just memcmp against:
>>>
>>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
>>> b/drivers/media/usb/uvc/uvc_v4l2.c
>>> index fa58131..7fa469b 100644
>>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>>> @@ -1003,10 +1003,26 @@ static long uvc_v4l2_do_ioctl(struct file *file, 
>>> unsigned int cmd, void *arg)
>>> case VIDIOC_CREATE_BUFS:
>>> {
>>> struct v4l2_create_buffers *cb = arg;
>>> +   struct v4l2_pix_format *pix;
>>> +   struct uvc_format *format;
>>> +   struct uvc_frame *frame;
>>>
>>> if (!uvc_has_privileges(handle))
>>> return -EBUSY;
>>>
>>> +   format = stream->cur_format;
>>> +   frame = stream->cur_frame;
>>> +   pix =&cb->format.fmt.pix;
>>> +
>>> +   if (pix->pixelformat != format->fcc ||
>>> +   pix->width != frame->wWidth ||
>>> +   pix->height != frame->wHeight ||
>>> +   pix->field != V4L2_FIELD_NONE ||
>>> +   pix->bytesperline != format->bpp * frame->wWidth / 8 ||
>>> +   pix->sizeimage != stream->ctrl.dwMaxVideoFrameSize ||
>>> +   pix->colorspace != format->colorspace)
>>
>> I would drop the field and colorspace checks (those do not really affect
>> any size calculations), other than that it looks good.
> 
> That seems completely wrong to me, AFAICT the VIDIOC_CREATE_BUFS was 
> designed
> so that the driver is supposed to allow any format which is supported by the
> hardware.
> What has currently selected format to do with the format passed to
> VIDIOC_CREATE_BUFS ? It should be allowed to create buffers of any size
> (implied by the passed v4l2_pix_format). It is supposed to be checked if
> a buffer meets constraints of current configuration of
> the hardware at QBUF, not at VIDIOC_CREATE_BUFS time. User space may well
> allocate buffers when one image format is set, keep them aside and then
> just before queueing them to the driver may set the format to a different
> one, so the hardware set up matches buffers allocated with 
> VIDIOC_CREATE_BUFS.
> 
> What's the point of having VIDIOC_CREATE_BUFS when you are doing checks
> like above ? Unless I'm missing something that is completely wrong. :)
> Adjusting cb->format.fmt.pix as in VIDIOC_TRY_FORMAT seems more appropriate
> thing to do.

OK, I agree that the code above is wrong. So ignore that.

What should CREATE_BUFS do when it is called?

Should I go back to this patch: 
http://www.spinics.net/lists/linux-media/msg72171.html

It will at least ensure that the fmt is consistent. It is however not quite
according to the spec since invalid formats are generally 'reformatted' by
TRY_FMT to something valid, and the spec says invalid formats should return
an error. It is possible to do something more advanced here, though: you
could make a copy of v4l2_format, call TRY_FMT on it, and check if there
are any differences with what was passed in. If there are, return an
error.

It's a bit of work, but probably better to do it in the core rather than
depend on drivers to do it (since they wo

Re: [PATCH] e4000: implement controls via v4l2 control framework

2014-02-04 Thread Hans Verkuil
Hi Antti,

Hmm, it's a bit ugly this code. I have some suggestions below...

On 02/05/2014 01:05 AM, Antti Palosaari wrote:
> Implement gain and bandwidth controls using v4l2 control framework.
> Pointer to control handler is provided by exported symbol.
> 
> Cc: Mauro Carvalho Chehab 
> Cc: Hans Verkuil 
> Signed-off-by: Antti Palosaari 
> ---
>  drivers/media/tuners/e4000.c  | 210 
> +-
>  drivers/media/tuners/e4000.h  |  14 +++
>  drivers/media/tuners/e4000_priv.h |  12 +++
>  3 files changed, 235 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> index 9187190..77318e9 100644
> --- a/drivers/media/tuners/e4000.c
> +++ b/drivers/media/tuners/e4000.c
> @@ -448,6 +448,178 @@ err:
>   return ret;
>  }
>  
> +static int e4000_set_lna_gain(struct dvb_frontend *fe)

I would change this to:

 e4000_set_lna_if_gain(struct dvb_frontend *fe, bool lna_auto, bool if_auto, 
bool set_lna)

> +{
> + struct e4000_priv *priv = fe->tuner_priv;
> + int ret;
> + u8 u8tmp;

General comment: always add a newline after variable declarations.

> + dev_dbg(&priv->client->dev, "%s: lna auto=%d->%d val=%d->%d\n",
> + __func__, priv->lna_gain_auto->cur.val,
> + priv->lna_gain_auto->val, priv->lna_gain->cur.val,
> + priv->lna_gain->val);
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 1);
> +
> + if (priv->lna_gain_auto->val && priv->if_gain_auto->cur.val)
> + u8tmp = 0x17;
> + else if (priv->lna_gain_auto->val)
> + u8tmp = 0x19;
> + else if (priv->if_gain_auto->cur.val)
> + u8tmp = 0x16;
> + else
> + u8tmp = 0x10;
> +
> + ret = e4000_wr_reg(priv, 0x1a, u8tmp);
> + if (ret)
> + goto err;
> +
> + if (priv->lna_gain_auto->val == false) {
> + ret = e4000_wr_reg(priv, 0x14, priv->lna_gain->val);
> + if (ret)
> + goto err;
> + }

Set lna gain if set_lna is true and lna_auto is false, set if gain if
set_lna is false and if_gain is false.

> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +
> + return 0;

I would remove the 4 lines above, and instead just...

> +err:
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +

...add this:

if (ret)

> + dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
> + return ret;
> +}
> +
> +static int e4000_set_mixer_gain(struct dvb_frontend *fe)
> +{
> + struct e4000_priv *priv = fe->tuner_priv;
> + int ret;
> + u8 u8tmp;
> + dev_dbg(&priv->client->dev, "%s: mixer auto=%d->%d val=%d->%d\n",
> + __func__, priv->mixer_gain_auto->cur.val,
> + priv->mixer_gain_auto->val, priv->mixer_gain->cur.val,
> + priv->mixer_gain->val);
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 1);
> +
> + if (priv->mixer_gain_auto->val)
> + u8tmp = 0x15;
> + else
> + u8tmp = 0x14;
> +
> + ret = e4000_wr_reg(priv, 0x20, u8tmp);
> + if (ret)
> + goto err;
> +
> + if (priv->mixer_gain_auto->val == false) {
> + ret = e4000_wr_reg(priv, 0x15, priv->mixer_gain->val);
> + if (ret)
> + goto err;
> + }
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +
> + return 0;
> +err:
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +
> + dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
> + return ret;
> +}
> +
> +static int e4000_set_if_gain(struct dvb_frontend *fe)
> +{
> + struct e4000_priv *priv = fe->tuner_priv;
> + int ret;
> + u8 buf[2];
> + u8 u8tmp;
> + dev_dbg(&priv->client->dev, "%s: if auto=%d->%d val=%d->%d\n",
> + __func__, priv->if_gain_auto->cur.val,
> + priv->if_gain_auto->val, priv->if_gain->cur.val,
> + priv->if_gain->val);
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 1);
> +
> + if (priv->if_gain_auto->val && priv->lna_gain_auto->cur.val)
> + u8tmp = 0x17;
> + else if (priv->lna_gain_auto->cur.val)
> + u8tmp = 0x19;
> + else if (priv->if_gain_auto->val)
> + u8tmp = 0x16;
> + else
> + u8tmp = 0x10;
> +
> + ret = e4000_wr_reg(priv, 0x1a, u8tmp);
> + if (ret)
> + goto err;
> +
> + if (priv->if_gain_auto->val == false) {
> + buf[0] = e4000_if_gain_lut[priv->if_gain->val].reg16_val;
> + buf[1] = e4000_if_gain_lut[priv->if_gain->val].reg17_val;
> + ret = e4000_wr_regs(priv, 0x16, buf, 2);
> + if (ret)
> + goto err;
> + 

Re: [PATCH] s2255drv: file handle cleanup

2014-02-04 Thread Hans Verkuil
On 02/05/2014 12:10 AM, Dean Anderson wrote:
> Hi Hans,
> 
> Please ignore and reject this patch. videobuf_queue_vmalloc_init needs 
> to be in probe, not in open.
> 
> Let me know your thoughts on doing videobuf2 before s2255_fh removal so 
> we don't have to work around or fix videobuf version one's deficiencies.

What I have done in the past w.r.t. vb2 conversions is to move fields
out of the fh struct where it is possible without breaking videobuf, then
when I do the final vb2 conversion I drop the fh struct completely.

As you mentioned before, the resources field can't really be dropped
until the vb2 conversion, but it simplifies matters if the rest is moved
out first. The end result of a vb2 conversion is great, but the actual
patch is a pain to review :-)

Now that I am adding streaming tests in v4l2-compliance I am hoping that
it will be easier to verify correctness in the future, something that
really hasn't been possible.

Regards,

Hans

> 
> Thanks,
> 
> 
> 
> 
> On 2014-02-04 16:36, Dean Anderson wrote:
>> Removes most parameters from s2255_fh.  These elements belong in 
>> s2255_ch.
>> In the future, s2255_fh will be removed when videobuf2 is used. 
>> videobuf2
>> has convenient and safe functions for locking streaming resources.
>>
>> The removal of s2255_fh (and s2255_fh->resources) was not done now to
>> avoid using videobuf_queue_is_busy.
>>
>> videobuf_queue_is busy may be unsafe as noted by the following comment
>> in videobuf-core.c:
>> "/* Locking: Only usage in bttv unsafe find way to remove */"
>>
>> Signed-off-by: Dean Anderson 
>> ---

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] s2255drv: file handle cleanup

2014-02-04 Thread Hans Verkuil
On 02/04/2014 11:36 PM, Dean Anderson wrote:
> Removes most parameters from s2255_fh.  These elements belong in s2255_ch.
> In the future, s2255_fh will be removed when videobuf2 is used. videobuf2
> has convenient and safe functions for locking streaming resources.
> 
> The removal of s2255_fh (and s2255_fh->resources) was not done now to
> avoid using videobuf_queue_is_busy.
> 
> videobuf_queue_is busy may be unsafe as noted by the following comment 
> in videobuf-core.c:
> "/* Locking: Only usage in bttv unsafe find way to remove */"
> 
> Signed-off-by: Dean Anderson 
> ---
>  drivers/media/usb/s2255/s2255drv.c |  224 
> +---
>  1 file changed, 105 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/media/usb/s2255/s2255drv.c 
> b/drivers/media/usb/s2255/s2255drv.c
> index 2e24aee..3ea1bd5e 100644
> --- a/drivers/media/usb/s2255/s2255drv.c
> +++ b/drivers/media/usb/s2255/s2255drv.c
> @@ -251,6 +251,8 @@ struct s2255_vc {
>   unsigned intheight;
>   const struct s2255_fmt  *fmt;
>   int idx; /* channel number on device, 0-3 */
> + struct videobuf_queue   vb_vidq;
> + enum v4l2_buf_type  type;

The whole type field can be dropped completely. This driver only support the
VIDEO_CAPTURE type anyway.

>  };

Thank you for splitting up the large patch into smaller pieces. I plan to
review them Friday or Monday.

Regards,

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/1] v4l: subdev: Allow 32-bit compat IOCTLs

2014-02-04 Thread Hans Verkuil
On 02/04/2014 09:10 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 31 Jan 2014 18:35:12 +0100
> Hans Verkuil  escreveu:
> 
>> Hi Sakari,
>>
>> On 01/31/2014 05:15 PM, Sakari Ailus wrote:
>>> I thought this was already working but apparently not. Allow 32-bit compat
>>> IOCTLs on 64-bit systems.
>>>
>>> Signed-off-by: Sakari Ailus 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 12 
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
>>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> index 8f7a6a4..1fce944 100644
>>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> @@ -1087,6 +1087,18 @@ long v4l2_compat_ioctl32(struct file *file, unsigned 
>>> int cmd, unsigned long arg)
>>> case VIDIOC_QUERY_DV_TIMINGS:
>>> case VIDIOC_DV_TIMINGS_CAP:
>>> case VIDIOC_ENUM_FREQ_BANDS:
>>> +   /* Sub-device IOCTLs */
>>> +   case VIDIOC_SUBDEV_G_FMT:
>>> +   case VIDIOC_SUBDEV_S_FMT:
>>> +   case VIDIOC_SUBDEV_G_FRAME_INTERVAL:
>>> +   case VIDIOC_SUBDEV_S_FRAME_INTERVAL:
>>> +   case VIDIOC_SUBDEV_ENUM_MBUS_CODE:
>>> +   case VIDIOC_SUBDEV_ENUM_FRAME_SIZE:
>>> +   case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL:
>>> +   case VIDIOC_SUBDEV_G_CROP:
>>> +   case VIDIOC_SUBDEV_S_CROP:
>>> +   case VIDIOC_SUBDEV_G_SELECTION:
>>> +   case VIDIOC_SUBDEV_S_SELECTION:
>>> case VIDIOC_SUBDEV_G_EDID32:
>>> case VIDIOC_SUBDEV_S_EDID32:
>>> ret = do_video_ioctl(file, cmd, arg);
>>>
>>
>> Can you test with contrib/test/ioctl-test? Compile with:
>>
>> gcc -o ioctl-test -m32 -I ../../include/ ioctl-test.c
>>
>> Make sure you use the latest v4l-utils version and run autoreconf -vfi
>> and configure first.
>>
>> BTW, I noticed that VIDIOC_DBG_G_CHIP_INFO is missing as well.
>>
>> Hmm, this is just asking for problems. 
>>
>> How about this patch:
>>
>> Signed-off-by: Hans Verkuil 
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> index 8f7a6a4..cd9da4ce 100644
>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> @@ -1001,108 +1001,19 @@ static long do_video_ioctl(struct file *file, 
>> unsigned int cmd, unsigned long ar
>>  long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long 
>> arg)
>>  {
>>  struct video_device *vdev = video_devdata(file);
>> -long ret = -ENOIOCTLCMD;
>> +long ret = -ENOTTY;
>>  
>>  if (!file->f_op->unlocked_ioctl)
>>  return ret;
>>  
>> -switch (cmd) {
>> -case VIDIOC_QUERYCAP:
>> -case VIDIOC_RESERVED:
>> -case VIDIOC_ENUM_FMT:
>> -case VIDIOC_G_FMT32:
>> -case VIDIOC_S_FMT32:
>> -case VIDIOC_REQBUFS:
>> -case VIDIOC_QUERYBUF32:
>> -case VIDIOC_G_FBUF32:
>> -case VIDIOC_S_FBUF32:
>> -case VIDIOC_OVERLAY32:
>> -case VIDIOC_QBUF32:
>> -case VIDIOC_EXPBUF:
>> -case VIDIOC_DQBUF32:
>> -case VIDIOC_STREAMON32:
>> -case VIDIOC_STREAMOFF32:
>> -case VIDIOC_G_PARM:
>> -case VIDIOC_S_PARM:
>> -case VIDIOC_G_STD:
>> -case VIDIOC_S_STD:
>> -case VIDIOC_ENUMSTD32:
>> -case VIDIOC_ENUMINPUT32:
>> -case VIDIOC_G_CTRL:
>> -case VIDIOC_S_CTRL:
>> -case VIDIOC_G_TUNER:
>> -case VIDIOC_S_TUNER:
>> -case VIDIOC_G_AUDIO:
>> -case VIDIOC_S_AUDIO:
>> -case VIDIOC_QUERYCTRL:
>> -case VIDIOC_QUERYMENU:
>> -case VIDIOC_G_INPUT32:
>> -case VIDIOC_S_INPUT32:
>> -case VIDIOC_G_OUTPUT32:
>> -case VIDIOC_S_OUTPUT32:
>> -case VIDIOC_ENUMOUTPUT:
>> -case VIDIOC_G_AUDOUT:
>> -case VIDIOC_S_AUDOUT:
>> -case VIDIOC_G_MODULATOR:
>> -case VIDIOC_S_MODULATOR:
>> -case VIDIOC_S_FREQUENCY:
>> -case VIDIOC_G_FREQUENCY:
>> -case VIDIOC_CROPCAP:
>> -case VIDIOC_G_CROP:
>> -case VIDIOC_S_CROP:
>> -case VIDIOC_G_SELECTION:
>> -case VIDIOC_S_SELECTION:
>> -case VIDIOC_G_JPEGCOMP:
>> -case VIDIOC_S_JPEGCOMP:
>> -case VIDIOC_QUERYSTD:
>> -case VIDIOC_TRY_FMT32:
>> -case VIDIOC_ENUMAUDIO:
>> -case VIDIOC_ENUMAUDOUT:
>> -case VIDIOC_G_PRIORITY:
>> -case VIDIOC_S_PRIORITY:
>> -case VIDIOC_G_SLICED_VBI_CAP:
>> -case VIDIOC_LOG_STATUS:
>> -case VIDIOC_G_EXT_CTRLS32:
>> -case VIDIOC_S_EXT_CTRLS32:
>> -case VIDIOC_TRY_EXT_CTRLS32:
>> -case VIDIOC_ENUM_FRAMESIZES:
>> -case VIDIOC_ENUM_FRAMEINTERVALS:
>> -case VIDIOC_G_ENC_INDEX:
>> -case VIDIOC_ENCODER_CMD:
>> -case VIDIOC_TRY_ENCODER_CMD:
>> -case VIDIOC_DECODER_CMD:
>> -case VIDIOC_TRY_DECODER_CMD:
>> -case VIDIOC_DBG_S_REGISTER:
>> -case VIDIOC_DBG_G_REGISTER:
>> -case VIDIOC_S_HW_FREQ_SEEK:
>> -case VIDIOC_S_DV_TIMINGS:
>> -case VIDIOC_G_DV_TIMINGS:
>> -case VIDIOC_DQEVENT:
>> -case VIDIOC_DQEVENT32:
>> -case VIDIOC_SUBSCRIBE_EVENT:
>> -case VIDIOC_UNSUBSCRIBE_EVENT:
>> -

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

2014-02-04 Thread Antti Seppälä
On 4 February 2014 19:54, Mauro Carvalho Chehab  wrote:
> Em Thu, 23 Jan 2014 21:11:09 +0200
> Antti Seppälä  escreveu:
>
>> On 23 January 2014 00:01, Mauro Carvalho Chehab  wrote:
>> > Not sure if you saw it, but there's already another patchset proposing
>> > that, that got submitted before this changeset:
>> > https://patchwork.linuxtv.org/patch/21625/
>>
>> I actually didn't notice that until now. Seems quite a similar kind of
>> approach with even more advanced features than what I had in mind
>> (namely the scancode filtering and masking).
>>
>> However it looks like that patchset has the same drawback about not
>> knowing which protocol to use for the wakeup scancode as was pointed
>> from my patch.
>
> Well, the protocol is important only if there are two or more active
> RCs that produce the same IR code on different protocols.
>
> Also, from the sysfs description made by James, it seems clear to me
> that the protocol to be used is the current protocol.
>
> I think is an unlikely border case to have some hardware that supports
> more than one IR protocols enabled for the wakeup to work, so James
> patch looks ok on my eyes.
>
> Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node
> to allow to filter the wakeup scancode to a protocol set different than
> the one(s) currently enabled.
>
>> I think I'll try to come up with a new patch addressing the comments
>> I've seen so far.
>
> I guess I'll merge James approach, as there are a pile of other patches
> depending on it. If we need latter to distinguish between current_protocol
> and the wakeup one, as I said, a latter patch can add a
> "wakeup_filter_protocol" sysfs node to specify it.
>
> Regards,
> Mauro

Hi Mauro.

How do you want to proceed with the nuvoton wakeup I initially set out to do?

To wake up with nuvoton-cir we need to program several raw ir
pulse/space lengths to the hardware and not a scancode. James's
approach doesn't support this.

To keep things simple maybe module parameters in my first patch
(http://www.mail-archive.com/linux-media@vger.kernel.org/msg69782.html)
are then the best thing to do for nuvoton? Other drivers can probably
adapt to the suggested filter api.

-Antti
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


au0828 errors and mangled video with Hauppauge 950Q

2014-02-04 Thread Connor Behan
Ccing Devin. I'm pretty sure the analog side has a problem at the driver
level.

On most days, one cannot pickup an ATSC signal where I am, so I am
trying to capture analog video with a Hauppauge WinTV HVR 950Q. Whether
I use Television, Composite or S-Video, I see the same corrupted video
such as this: http://imgur.com/c398F4v

It is supposed to be a hockey game. When the broadcast cuts to a
commercial, I still see frames of the game for about 5 seconds, then the
fuzzy stuff on the screen contains some images from the game and some
images from the commercial, then it fully switches over to the
commercial. Frames that are supposed to be seen at different times are
being mixed together.

When I enable debugging for all of the media modules, my dmesg is
dominated by au0828 calls to print_err_status:
http://pastebin.ca/2628011 When I enable debugging on all modules except
the au0828, what I see is:

Insert tuner: http://pastebin.ca/2628012
mplayer tv:///2: http://pastebin.ca/2628017

Does this shed light on the problem? I'd be happy to test patches.



signature.asc
Description: OpenPGP digital signature


cron job: media_tree daily build: ERRORS

2014-02-04 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Wed Feb  5 04:00:30 CET 2014
git branch: test
git hash:   261cb200e7227820cd0056435d7c1a3a9c476766
gcc version:i686-linux-gcc (GCC) 4.8.2
sparse version: 0.4.5-rc1
host hardware:  x86_64
host os:3.12-6.slh.2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: ERRORS
linux-git-arm-exynos: WARNINGS
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: ERRORS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.14-i686: OK
linux-2.6.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12-i686: OK
linux-3.13-i686: OK
linux-2.6.31.14-x86_64: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12-x86_64: OK
linux-3.13-x86_64: OK
apps: OK
spec-git: OK
sparse version: 0.4.5-rc1
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] DVB tuner use v4l2 controls

2014-02-04 Thread Antti Palosaari
Split .s_ctrl logic according to Hans comments.

regards
Antti

Antti Palosaari (1):
  e4000: implement controls via v4l2 control framework

 drivers/media/tuners/e4000.c  | 210 +-
 drivers/media/tuners/e4000.h  |  14 +++
 drivers/media/tuners/e4000_priv.h |  12 +++
 3 files changed, 235 insertions(+), 1 deletion(-)

-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] e4000: implement controls via v4l2 control framework

2014-02-04 Thread Antti Palosaari
Implement gain and bandwidth controls using v4l2 control framework.
Pointer to control handler is provided by exported symbol.

Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Signed-off-by: Antti Palosaari 
---
 drivers/media/tuners/e4000.c  | 210 +-
 drivers/media/tuners/e4000.h  |  14 +++
 drivers/media/tuners/e4000_priv.h |  12 +++
 3 files changed, 235 insertions(+), 1 deletion(-)

diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 9187190..77318e9 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -448,6 +448,178 @@ err:
return ret;
 }
 
+static int e4000_set_lna_gain(struct dvb_frontend *fe)
+{
+   struct e4000_priv *priv = fe->tuner_priv;
+   int ret;
+   u8 u8tmp;
+   dev_dbg(&priv->client->dev, "%s: lna auto=%d->%d val=%d->%d\n",
+   __func__, priv->lna_gain_auto->cur.val,
+   priv->lna_gain_auto->val, priv->lna_gain->cur.val,
+   priv->lna_gain->val);
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 1);
+
+   if (priv->lna_gain_auto->val && priv->if_gain_auto->cur.val)
+   u8tmp = 0x17;
+   else if (priv->lna_gain_auto->val)
+   u8tmp = 0x19;
+   else if (priv->if_gain_auto->cur.val)
+   u8tmp = 0x16;
+   else
+   u8tmp = 0x10;
+
+   ret = e4000_wr_reg(priv, 0x1a, u8tmp);
+   if (ret)
+   goto err;
+
+   if (priv->lna_gain_auto->val == false) {
+   ret = e4000_wr_reg(priv, 0x14, priv->lna_gain->val);
+   if (ret)
+   goto err;
+   }
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   return 0;
+err:
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
+   return ret;
+}
+
+static int e4000_set_mixer_gain(struct dvb_frontend *fe)
+{
+   struct e4000_priv *priv = fe->tuner_priv;
+   int ret;
+   u8 u8tmp;
+   dev_dbg(&priv->client->dev, "%s: mixer auto=%d->%d val=%d->%d\n",
+   __func__, priv->mixer_gain_auto->cur.val,
+   priv->mixer_gain_auto->val, priv->mixer_gain->cur.val,
+   priv->mixer_gain->val);
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 1);
+
+   if (priv->mixer_gain_auto->val)
+   u8tmp = 0x15;
+   else
+   u8tmp = 0x14;
+
+   ret = e4000_wr_reg(priv, 0x20, u8tmp);
+   if (ret)
+   goto err;
+
+   if (priv->mixer_gain_auto->val == false) {
+   ret = e4000_wr_reg(priv, 0x15, priv->mixer_gain->val);
+   if (ret)
+   goto err;
+   }
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   return 0;
+err:
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
+   return ret;
+}
+
+static int e4000_set_if_gain(struct dvb_frontend *fe)
+{
+   struct e4000_priv *priv = fe->tuner_priv;
+   int ret;
+   u8 buf[2];
+   u8 u8tmp;
+   dev_dbg(&priv->client->dev, "%s: if auto=%d->%d val=%d->%d\n",
+   __func__, priv->if_gain_auto->cur.val,
+   priv->if_gain_auto->val, priv->if_gain->cur.val,
+   priv->if_gain->val);
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 1);
+
+   if (priv->if_gain_auto->val && priv->lna_gain_auto->cur.val)
+   u8tmp = 0x17;
+   else if (priv->lna_gain_auto->cur.val)
+   u8tmp = 0x19;
+   else if (priv->if_gain_auto->val)
+   u8tmp = 0x16;
+   else
+   u8tmp = 0x10;
+
+   ret = e4000_wr_reg(priv, 0x1a, u8tmp);
+   if (ret)
+   goto err;
+
+   if (priv->if_gain_auto->val == false) {
+   buf[0] = e4000_if_gain_lut[priv->if_gain->val].reg16_val;
+   buf[1] = e4000_if_gain_lut[priv->if_gain->val].reg17_val;
+   ret = e4000_wr_regs(priv, 0x16, buf, 2);
+   if (ret)
+   goto err;
+   }
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   return 0;
+err:
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
+   return ret;
+}
+
+static int e4000_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct e4000_priv *priv =
+   container_of(ctrl->handler, struct e4000_priv, hdl);
+   struct dvb_frontend *fe = priv->fe;
+   struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+   int ret;
+   dev_dbg(&priv->cli

Re: [PATCH] s2255drv: file handle cleanup

2014-02-04 Thread Dean Anderson

Hi Hans,

Please ignore and reject this patch. videobuf_queue_vmalloc_init needs 
to be in probe, not in open.


Let me know your thoughts on doing videobuf2 before s2255_fh removal so 
we don't have to work around or fix videobuf version one's deficiencies.


Thanks,




On 2014-02-04 16:36, Dean Anderson wrote:
Removes most parameters from s2255_fh.  These elements belong in 
s2255_ch.
In the future, s2255_fh will be removed when videobuf2 is used. 
videobuf2

has convenient and safe functions for locking streaming resources.

The removal of s2255_fh (and s2255_fh->resources) was not done now to
avoid using videobuf_queue_is_busy.

videobuf_queue_is busy may be unsafe as noted by the following comment
in videobuf-core.c:
"/* Locking: Only usage in bttv unsafe find way to remove */"

Signed-off-by: Dean Anderson 
---

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [media] uvcvideo: Enable VIDIOC_CREATE_BUFS

2014-02-04 Thread Sylwester Nawrocki

Hi,

On 02/03/2014 10:03 AM, Hans Verkuil wrote:

Hi Philipp, Laurent,

On 02/02/2014 02:04 PM, Philipp Zabel wrote:

On Sun, Feb 02, 2014 at 11:21:13AM +0100, Laurent Pinchart wrote:

Hi Hans,

On Friday 31 January 2014 09:43:00 Hans Verkuil wrote:

I think you might want to add a check in uvc_queue_setup to verify the
fmt that create_bufs passes. The spec says that: "Unsupported formats
will result in an error". In this case I guess that the format basically
should match the current selected format.

I'm unhappy with the current implementations of create_bufs (see also this
patch:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg70796.html).

Nobody is actually checking the format today, which isn't good.

The fact that the spec says that the fmt field isn't changed by the driver
isn't helping as it invalidated my patch from above, although that can be
fixed.

I need to think about this some more, but for this particular case you can
just do a memcmp of the v4l2_pix_format against the currently selected
format and return an error if they differ. Unless you want to support
different buffer sizes as well?


Isn't the whole point of VIDIOC_CREATE_BUFS being able to create buffers of
different resolutions than the current active resolution ?


Or just additional buffers with the same resolution (or really, the same size).


For that to work the driver in question would need to keep track of per-buffer
format and resolution, and not only of per-queue format and resolution.

For now, would something like the following be enough? Unfortunately the uvc
driver doesn't keep a v4l2_format around that we could just memcmp against:

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index fa58131..7fa469b 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1003,10 +1003,26 @@ static long uvc_v4l2_do_ioctl(struct file *file, 
unsigned int cmd, void *arg)
case VIDIOC_CREATE_BUFS:
{
struct v4l2_create_buffers *cb = arg;
+   struct v4l2_pix_format *pix;
+   struct uvc_format *format;
+   struct uvc_frame *frame;

if (!uvc_has_privileges(handle))
return -EBUSY;

+   format = stream->cur_format;
+   frame = stream->cur_frame;
+   pix =&cb->format.fmt.pix;
+
+   if (pix->pixelformat != format->fcc ||
+   pix->width != frame->wWidth ||
+   pix->height != frame->wHeight ||
+   pix->field != V4L2_FIELD_NONE ||
+   pix->bytesperline != format->bpp * frame->wWidth / 8 ||
+   pix->sizeimage != stream->ctrl.dwMaxVideoFrameSize ||
+   pix->colorspace != format->colorspace)


I would drop the field and colorspace checks (those do not really affect
any size calculations), other than that it looks good.


That seems completely wrong to me, AFAICT the VIDIOC_CREATE_BUFS was 
designed

so that the driver is supposed to allow any format which is supported by the
hardware.
What has currently selected format to do with the format passed to
VIDIOC_CREATE_BUFS ? It should be allowed to create buffers of any size
(implied by the passed v4l2_pix_format). It is supposed to be checked if
a buffer meets constraints of current configuration of
the hardware at QBUF, not at VIDIOC_CREATE_BUFS time. User space may well
allocate buffers when one image format is set, keep them aside and then
just before queueing them to the driver may set the format to a different
one, so the hardware set up matches buffers allocated with 
VIDIOC_CREATE_BUFS.


What's the point of having VIDIOC_CREATE_BUFS when you are doing checks
like above ? Unless I'm missing something that is completely wrong. :)
Adjusting cb->format.fmt.pix as in VIDIOC_TRY_FORMAT seems more appropriate
thing to do.

Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] s2255drv: file handle cleanup

2014-02-04 Thread Dean Anderson
Removes most parameters from s2255_fh.  These elements belong in s2255_ch.
In the future, s2255_fh will be removed when videobuf2 is used. videobuf2
has convenient and safe functions for locking streaming resources.

The removal of s2255_fh (and s2255_fh->resources) was not done now to
avoid using videobuf_queue_is_busy.

videobuf_queue_is busy may be unsafe as noted by the following comment 
in videobuf-core.c:
"/* Locking: Only usage in bttv unsafe find way to remove */"

Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c |  224 +---
 1 file changed, 105 insertions(+), 119 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 2e24aee..3ea1bd5e 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -251,6 +251,8 @@ struct s2255_vc {
unsigned intheight;
const struct s2255_fmt  *fmt;
int idx; /* channel number on device, 0-3 */
+   struct videobuf_queue   vb_vidq;
+   enum v4l2_buf_type  type;
 };
 
 
@@ -296,10 +298,6 @@ struct s2255_buffer {
 struct s2255_fh {
/* this must be the first field in this struct */
struct v4l2_fh  fh;
-   struct s2255_dev*dev;
-   struct videobuf_queue   vb_vidq;
-   enum v4l2_buf_type  type;
-   struct s2255_vc *vc;
int resources;
 };
 
@@ -674,8 +672,7 @@ static void s2255_fillbuff(struct s2255_vc *vc,
 static int buffer_setup(struct videobuf_queue *vq, unsigned int *count,
unsigned int *size)
 {
-   struct s2255_fh *fh = vq->priv_data;
-   struct s2255_vc *vc = fh->vc;
+   struct s2255_vc *vc = vq->priv_data;
*size = vc->width * vc->height * (vc->fmt->depth >> 3);
 
if (0 == *count)
@@ -696,13 +693,12 @@ static void free_buffer(struct videobuf_queue *vq, struct 
s2255_buffer *buf)
 static int buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer 
*vb,
  enum v4l2_field field)
 {
-   struct s2255_fh *fh = vq->priv_data;
-   struct s2255_vc *vc = fh->vc;
+   struct s2255_vc *vc = vq->priv_data;
struct s2255_buffer *buf = container_of(vb, struct s2255_buffer, vb);
int rc;
int w = vc->width;
int h = vc->height;
-   dprintk(fh->dev, 4, "%s, field=%d\n", __func__, field);
+   dprintk(vc->dev, 4, "%s, field=%d\n", __func__, field);
if (vc->fmt == NULL)
return -EINVAL;
 
@@ -710,12 +706,12 @@ static int buffer_prepare(struct videobuf_queue *vq, 
struct videobuf_buffer *vb,
(w > norm_maxw(vc)) ||
(h < norm_minh(vc)) ||
(h > norm_maxh(vc))) {
-   dprintk(fh->dev, 4, "invalid buffer prepare\n");
+   dprintk(vc->dev, 4, "invalid buffer prepare\n");
return -EINVAL;
}
buf->vb.size = w * h * (vc->fmt->depth >> 3);
if (0 != buf->vb.baddr && buf->vb.bsize < buf->vb.size) {
-   dprintk(fh->dev, 4, "invalid buffer prepare\n");
+   dprintk(vc->dev, 4, "invalid buffer prepare\n");
return -EINVAL;
}
 
@@ -740,9 +736,8 @@ fail:
 static void buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
 {
struct s2255_buffer *buf = container_of(vb, struct s2255_buffer, vb);
-   struct s2255_fh *fh = vq->priv_data;
-   struct s2255_vc *vc = fh->vc;
-   dprintk(fh->dev, 1, "%s\n", __func__);
+   struct s2255_vc *vc = vq->priv_data;
+   dprintk(vc->dev, 1, "%s\n", __func__);
buf->vb.state = VIDEOBUF_QUEUED;
list_add_tail(&buf->vb.queue, &vc->buf_list);
 }
@@ -751,8 +746,8 @@ static void buffer_release(struct videobuf_queue *vq,
   struct videobuf_buffer *vb)
 {
struct s2255_buffer *buf = container_of(vb, struct s2255_buffer, vb);
-   struct s2255_fh *fh = vq->priv_data;
-   dprintk(fh->dev, 4, "%s %d\n", __func__, fh->vc->idx);
+   struct s2255_vc *vc = vq->priv_data;
+   dprintk(vc->dev, 4, "%s %d\n", __func__, vc->idx);
free_buffer(vq, buf);
 }
 
@@ -764,22 +759,21 @@ static struct videobuf_queue_ops s2255_video_qops = {
 };
 
 
-static int res_get(struct s2255_fh *fh)
+static int res_get(struct s2255_fh *fh, struct s2255_vc *vc)
 {
-   struct s2255_vc *vc = fh->vc;
/* is it free? */
if (vc->resources)
return 0; /* no, someone else uses it */
/* it's free, grab it */
vc->resources = 1;
fh->resources = 1;
-   dprintk(fh->dev, 1, "s2255: res: get\n");
+   dprintk(vc->dev, 1, "s2255: res: get\n");
return 1;
 }
 
-static int res_locked(struct s2255_fh *fh)
+static int res_locked(struct s2255_vc *vc)
 {
-   return fh->vc->resources;
+   return vc->resources;
 }
 
 static int res_check(struct s2255_fh *fh)
@@ -788,9 +782,8 @@ static int res_check(stru

Re: [PATCH] [media] uvcvideo: Enable VIDIOC_CREATE_BUFS

2014-02-04 Thread Laurent Pinchart
Hi Hans,

On Monday 03 February 2014 10:03:39 Hans Verkuil wrote:
> On 02/02/2014 02:04 PM, Philipp Zabel wrote:
> > On Sun, Feb 02, 2014 at 11:21:13AM +0100, Laurent Pinchart wrote:
> >> On Friday 31 January 2014 09:43:00 Hans Verkuil wrote:
> >>> I think you might want to add a check in uvc_queue_setup to verify the
> >>> fmt that create_bufs passes. The spec says that: "Unsupported formats
> >>> will result in an error". In this case I guess that the format basically
> >>> should match the current selected format.
> >>> 
> >>> I'm unhappy with the current implementations of create_bufs (see also
> >>> this patch:
> >>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg70796.html).
> >>> 
> >>> Nobody is actually checking the format today, which isn't good.
> >>> 
> >>> The fact that the spec says that the fmt field isn't changed by the
> >>> driver isn't helping as it invalidated my patch from above, although
> >>> that can be fixed.
> >>> 
> >>> I need to think about this some more, but for this particular case you
> >>> can just do a memcmp of the v4l2_pix_format against the currently
> >>> selected format and return an error if they differ. Unless you want to
> >>> support different buffer sizes as well?
> >> 
> >> Isn't the whole point of VIDIOC_CREATE_BUFS being able to create buffers
> >> of different resolutions than the current active resolution ?
> 
> Or just additional buffers with the same resolution (or really, the same
> size).

Sure, that as well, but one use is to allocate larger buffers, shouldn't that 
be allowed ?

> > For that to work the driver in question would need to keep track of
> > per-buffer format and resolution, and not only of per-queue format and
> > resolution.
> > 
> > For now, would something like the following be enough? Unfortunately the
> > uvc driver doesn't keep a v4l2_format around that we could just memcmp
> > against:
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > b/drivers/media/usb/uvc/uvc_v4l2.c index fa58131..7fa469b 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -1003,10 +1003,26 @@ static long uvc_v4l2_do_ioctl(struct file *file,
> > unsigned int cmd, void *arg)> 
> > case VIDIOC_CREATE_BUFS:
> > {
> > 
> > struct v4l2_create_buffers *cb = arg;
> > 
> > +   struct v4l2_pix_format *pix;
> > +   struct uvc_format *format;
> > +   struct uvc_frame *frame;
> > 
> > if (!uvc_has_privileges(handle))
> > 
> > return -EBUSY;
> > 
> > +   format = stream->cur_format;
> > +   frame = stream->cur_frame;
> > +   pix = &cb->format.fmt.pix;
> > +
> > +   if (pix->pixelformat != format->fcc ||
> > +   pix->width != frame->wWidth ||
> > +   pix->height != frame->wHeight ||
> > +   pix->field != V4L2_FIELD_NONE ||
> > +   pix->bytesperline != format->bpp * frame->wWidth / 8 ||
> > +   pix->sizeimage != stream->ctrl.dwMaxVideoFrameSize ||
> > +   pix->colorspace != format->colorspace)
> 
> I would drop the field and colorspace checks (those do not really affect
> any size calculations), other than that it looks good.
> 
> Regards,
> 
>   Hans
> 
> > +   return -EINVAL;
> > +
> > return uvc_create_buffers(&stream->queue, cb);
> > 
> > }

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] s2255drv: refactoring s2255_channel to s2255_vc

2014-02-04 Thread Dean Anderson
Renaming s2255_channel and all instances of channel to vc (video channel).

Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c |  517 ++--
 1 file changed, 259 insertions(+), 258 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 5f09a56..2e24aee 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -212,7 +212,8 @@ struct s2255_pipeinfo {
 struct s2255_fmt; /*forward declaration */
 struct s2255_dev;
 
-struct s2255_channel {
+/* 2255 video channel */
+struct s2255_vc {
struct s2255_dev*dev;
struct video_device vdev;
struct v4l2_ctrl_handler hdl;
@@ -254,7 +255,7 @@ struct s2255_channel {
 
 
 struct s2255_dev {
-   struct s2255_channelchannel[MAX_CHANNELS];
+   struct s2255_vc vc[MAX_CHANNELS];
struct v4l2_device  v4l2_dev;
atomic_tnum_channels;
int frames;
@@ -298,7 +299,7 @@ struct s2255_fh {
struct s2255_dev*dev;
struct videobuf_queue   vb_vidq;
enum v4l2_buf_type  type;
-   struct s2255_channel*channel;
+   struct s2255_vc *vc;
int resources;
 };
 
@@ -351,11 +352,11 @@ static int debug;
 
 static int s2255_start_readpipe(struct s2255_dev *dev);
 static void s2255_stop_readpipe(struct s2255_dev *dev);
-static int s2255_start_acquire(struct s2255_channel *channel);
-static int s2255_stop_acquire(struct s2255_channel *channel);
-static void s2255_fillbuff(struct s2255_channel *chn, struct s2255_buffer *buf,
+static int s2255_start_acquire(struct s2255_vc *vc);
+static int s2255_stop_acquire(struct s2255_vc *vc);
+static void s2255_fillbuff(struct s2255_vc *vc, struct s2255_buffer *buf,
   int jpgsize);
-static int s2255_set_mode(struct s2255_channel *chan, struct s2255_mode *mode);
+static int s2255_set_mode(struct s2255_vc *vc, struct s2255_mode *mode);
 static int s2255_board_shutdown(struct s2255_dev *dev);
 static void s2255_fwload_start(struct s2255_dev *dev, int reset);
 static void s2255_destroy(struct s2255_dev *dev);
@@ -434,27 +435,27 @@ static const struct s2255_fmt formats[] = {
}
 };
 
-static int norm_maxw(struct s2255_channel *channel)
+static int norm_maxw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_4CIFS_NTSC : LINE_SZ_4CIFS_PAL;
 }
 
-static int norm_maxh(struct s2255_channel *channel)
+static int norm_maxh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC * 2) : (NUM_LINES_1CIFS_PAL * 2);
 }
 
-static int norm_minw(struct s2255_channel *channel)
+static int norm_minw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_1CIFS_NTSC : LINE_SZ_1CIFS_PAL;
 }
 
-static int norm_minh(struct s2255_channel *channel)
+static int norm_minh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC) : (NUM_LINES_1CIFS_PAL);
 }
 
@@ -567,23 +568,23 @@ static void s2255_fwchunk_complete(struct urb *urb)
 
 }
 
-static int s2255_got_frame(struct s2255_channel *channel, int jpgsize)
+static int s2255_got_frame(struct s2255_vc *vc, int jpgsize)
 {
struct s2255_buffer *buf;
-   struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
+   struct s2255_dev *dev = to_s2255_dev(vc->vdev.v4l2_dev);
unsigned long flags = 0;
int rc = 0;
spin_lock_irqsave(&dev->slock, flags);
-   if (list_empty(&channel->buf_list)) {
+   if (list_empty(&vc->buf_list)) {
dprintk(dev, 1, "No active queue to serve\n");
rc = -1;
goto unlock;
}
-   buf = list_entry(channel->buf_list.next,
+   buf = list_entry(vc->buf_list.next,
 struct s2255_buffer, vb.queue);
list_del(&buf->vb.queue);
v4l2_get_timestamp(&buf->vb.ts);
-   s2255_fillbuff(channel, buf, jpgsize);
+   s2255_fillbuff(vc, buf, jpgsize);
wake_up(&buf->vb.done);
dprintk(dev, 2, "%s: [buf/i] [%p/%d]\n", __func__, buf, buf->vb.i);
 unlock:
@@ -614,21 +615,21 @@ static const struct s2255_fmt *format_by_fourcc(int 
fourcc)
  *  http://v4l.videotechnology.com/
  *
  */
-static void s2255_fillbuff(struct s2255_channel *channel,
+static void s2255_fillbuff(struct s2255_vc *vc,
   struct s2255_buffer *buf, int jpgsize)
 {
int pos = 0;
const char *tmpbuf;
char *vbuf = videobuf_to_vmalloc(&buf->vb);
unsigned long last_frame;
-   struct s2255_dev *dev = channel->dev;
+   struct s2255_dev *de

Re: [PATCH] s2255drv: refactoring s2255_channel to s2255_vc

2014-02-04 Thread Dean Anderson
There are some missing changes.  It compiles, but please review v2 
instead.


On 2014-02-04 15:04, Dean Anderson wrote:
Renaming s2255_channel and all instances of channel to vc (video 
channel).


Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c |  469 
++--

 1 file changed, 235 insertions(+), 234 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index 5f09a56..e8b7096 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -212,7 +212,8 @@ struct s2255_pipeinfo {
 struct s2255_fmt; /*forward declaration */
 struct s2255_dev;

-struct s2255_channel {
+/* 2255 video channel */
+struct s2255_vc {
struct s2255_dev*dev;
struct video_device vdev;
struct v4l2_ctrl_handler hdl;
@@ -254,7 +255,7 @@ struct s2255_channel {


 struct s2255_dev {
-   struct s2255_channelchannel[MAX_CHANNELS];
+   struct s2255_vc vc[MAX_CHANNELS];
struct v4l2_device  v4l2_dev;
atomic_tnum_channels;
int frames;
@@ -298,7 +299,7 @@ struct s2255_fh {
struct s2255_dev*dev;
struct videobuf_queue   vb_vidq;
enum v4l2_buf_type  type;
-   struct s2255_channel*channel;
+   struct s2255_vc *vc;
int resources;
 };

@@ -351,11 +352,11 @@ static int debug;

 static int s2255_start_readpipe(struct s2255_dev *dev);
 static void s2255_stop_readpipe(struct s2255_dev *dev);
-static int s2255_start_acquire(struct s2255_channel *channel);
-static int s2255_stop_acquire(struct s2255_channel *channel);
-static void s2255_fillbuff(struct s2255_channel *chn, struct 
s2255_buffer *buf,

+static int s2255_start_acquire(struct s2255_vc *vc);
+static int s2255_stop_acquire(struct s2255_vc *vc);
+static void s2255_fillbuff(struct s2255_vc *chn, struct s2255_buffer 
*buf,

   int jpgsize);
-static int s2255_set_mode(struct s2255_channel *chan, struct 
s2255_mode *mode);
+static int s2255_set_mode(struct s2255_vc *chan, struct s2255_mode 
*mode);

 static int s2255_board_shutdown(struct s2255_dev *dev);
 static void s2255_fwload_start(struct s2255_dev *dev, int reset);
 static void s2255_destroy(struct s2255_dev *dev);
@@ -434,27 +435,27 @@ static const struct s2255_fmt formats[] = {
}
 };

-static int norm_maxw(struct s2255_channel *channel)
+static int norm_maxw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_4CIFS_NTSC : LINE_SZ_4CIFS_PAL;
 }

-static int norm_maxh(struct s2255_channel *channel)
+static int norm_maxh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC * 2) : (NUM_LINES_1CIFS_PAL * 2);
 }

-static int norm_minw(struct s2255_channel *channel)
+static int norm_minw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_1CIFS_NTSC : LINE_SZ_1CIFS_PAL;
 }

-static int norm_minh(struct s2255_channel *channel)
+static int norm_minh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC) : (NUM_LINES_1CIFS_PAL);
 }

@@ -567,23 +568,23 @@ static void s2255_fwchunk_complete(struct urb 
*urb)


 }

-static int s2255_got_frame(struct s2255_channel *channel, int 
jpgsize)

+static int s2255_got_frame(struct s2255_vc *vc, int jpgsize)
 {
struct s2255_buffer *buf;
-   struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
+   struct s2255_dev *dev = to_s2255_dev(vc->vdev.v4l2_dev);
unsigned long flags = 0;
int rc = 0;
spin_lock_irqsave(&dev->slock, flags);
-   if (list_empty(&channel->buf_list)) {
+   if (list_empty(&vc->buf_list)) {
dprintk(dev, 1, "No active queue to serve\n");
rc = -1;
goto unlock;
}
-   buf = list_entry(channel->buf_list.next,
+   buf = list_entry(vc->buf_list.next,
 struct s2255_buffer, vb.queue);
list_del(&buf->vb.queue);
v4l2_get_timestamp(&buf->vb.ts);
-   s2255_fillbuff(channel, buf, jpgsize);
+   s2255_fillbuff(vc, buf, jpgsize);
wake_up(&buf->vb.done);
dprintk(dev, 2, "%s: [buf/i] [%p/%d]\n", __func__, buf, buf->vb.i);
 unlock:
@@ -614,21 +615,21 @@ static const struct s2255_fmt
*format_by_fourcc(int fourcc)
  *  http://v4l.videotechnology.com/
  *
  */
-static void s2255_fillbuff(struct s2255_channel *channel,
+static void s2255_fillbuff(struct s2255_vc *vc,
   struct s2255_buffer *buf, int jpgsize)
 {
int pos = 0;
const char *tmpbuf;
char *vbuf = videobuf_to_vm

[PATCH] s2255drv: refactoring s2255_channel to s2255_vc

2014-02-04 Thread Dean Anderson
Renaming s2255_channel and all instances of channel to vc (video channel).

Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c |  469 ++--
 1 file changed, 235 insertions(+), 234 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 5f09a56..e8b7096 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -212,7 +212,8 @@ struct s2255_pipeinfo {
 struct s2255_fmt; /*forward declaration */
 struct s2255_dev;
 
-struct s2255_channel {
+/* 2255 video channel */
+struct s2255_vc {
struct s2255_dev*dev;
struct video_device vdev;
struct v4l2_ctrl_handler hdl;
@@ -254,7 +255,7 @@ struct s2255_channel {
 
 
 struct s2255_dev {
-   struct s2255_channelchannel[MAX_CHANNELS];
+   struct s2255_vc vc[MAX_CHANNELS];
struct v4l2_device  v4l2_dev;
atomic_tnum_channels;
int frames;
@@ -298,7 +299,7 @@ struct s2255_fh {
struct s2255_dev*dev;
struct videobuf_queue   vb_vidq;
enum v4l2_buf_type  type;
-   struct s2255_channel*channel;
+   struct s2255_vc *vc;
int resources;
 };
 
@@ -351,11 +352,11 @@ static int debug;
 
 static int s2255_start_readpipe(struct s2255_dev *dev);
 static void s2255_stop_readpipe(struct s2255_dev *dev);
-static int s2255_start_acquire(struct s2255_channel *channel);
-static int s2255_stop_acquire(struct s2255_channel *channel);
-static void s2255_fillbuff(struct s2255_channel *chn, struct s2255_buffer *buf,
+static int s2255_start_acquire(struct s2255_vc *vc);
+static int s2255_stop_acquire(struct s2255_vc *vc);
+static void s2255_fillbuff(struct s2255_vc *chn, struct s2255_buffer *buf,
   int jpgsize);
-static int s2255_set_mode(struct s2255_channel *chan, struct s2255_mode *mode);
+static int s2255_set_mode(struct s2255_vc *chan, struct s2255_mode *mode);
 static int s2255_board_shutdown(struct s2255_dev *dev);
 static void s2255_fwload_start(struct s2255_dev *dev, int reset);
 static void s2255_destroy(struct s2255_dev *dev);
@@ -434,27 +435,27 @@ static const struct s2255_fmt formats[] = {
}
 };
 
-static int norm_maxw(struct s2255_channel *channel)
+static int norm_maxw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_4CIFS_NTSC : LINE_SZ_4CIFS_PAL;
 }
 
-static int norm_maxh(struct s2255_channel *channel)
+static int norm_maxh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC * 2) : (NUM_LINES_1CIFS_PAL * 2);
 }
 
-static int norm_minw(struct s2255_channel *channel)
+static int norm_minw(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
LINE_SZ_1CIFS_NTSC : LINE_SZ_1CIFS_PAL;
 }
 
-static int norm_minh(struct s2255_channel *channel)
+static int norm_minh(struct s2255_vc *vc)
 {
-   return (channel->std & V4L2_STD_525_60) ?
+   return (vc->std & V4L2_STD_525_60) ?
(NUM_LINES_1CIFS_NTSC) : (NUM_LINES_1CIFS_PAL);
 }
 
@@ -567,23 +568,23 @@ static void s2255_fwchunk_complete(struct urb *urb)
 
 }
 
-static int s2255_got_frame(struct s2255_channel *channel, int jpgsize)
+static int s2255_got_frame(struct s2255_vc *vc, int jpgsize)
 {
struct s2255_buffer *buf;
-   struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
+   struct s2255_dev *dev = to_s2255_dev(vc->vdev.v4l2_dev);
unsigned long flags = 0;
int rc = 0;
spin_lock_irqsave(&dev->slock, flags);
-   if (list_empty(&channel->buf_list)) {
+   if (list_empty(&vc->buf_list)) {
dprintk(dev, 1, "No active queue to serve\n");
rc = -1;
goto unlock;
}
-   buf = list_entry(channel->buf_list.next,
+   buf = list_entry(vc->buf_list.next,
 struct s2255_buffer, vb.queue);
list_del(&buf->vb.queue);
v4l2_get_timestamp(&buf->vb.ts);
-   s2255_fillbuff(channel, buf, jpgsize);
+   s2255_fillbuff(vc, buf, jpgsize);
wake_up(&buf->vb.done);
dprintk(dev, 2, "%s: [buf/i] [%p/%d]\n", __func__, buf, buf->vb.i);
 unlock:
@@ -614,21 +615,21 @@ static const struct s2255_fmt *format_by_fourcc(int 
fourcc)
  *  http://v4l.videotechnology.com/
  *
  */
-static void s2255_fillbuff(struct s2255_channel *channel,
+static void s2255_fillbuff(struct s2255_vc *vc,
   struct s2255_buffer *buf, int jpgsize)
 {
int pos = 0;
const char *tmpbuf;
char *vbuf = videobuf_to_vmalloc(&buf->vb);
unsigned long last_frame;
-   struct s2255_dev *dev = channel->dev;
+   struct s2255_dev 

[PATCH] s2255drv: removal of s2255_dmaqueue structure

2014-02-04 Thread Dean Anderson
Removal of unused and unnecessary s2255dma_queue structure.

Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c |   29 ++---
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index c6bdccc..5f09a56 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -1,7 +1,7 @@
 /*
  *  s2255drv.c - a driver for the Sensoray 2255 USB video capture device
  *
- *   Copyright (C) 2007-2013 by Sensoray Company Inc.
+ *   Copyright (C) 2007-2014 by Sensoray Company Inc.
  *  Dean Anderson
  *
  * Some video buffer code based on vivi driver:
@@ -52,7 +52,7 @@
 #include 
 #include 
 
-#define S2255_VERSION  "1.23.1"
+#define S2255_VERSION  "1.24.1"
 #define FIRMWARE_FILE_NAME "f2255usb.bin"
 
 /* default JPEG quality */
@@ -178,11 +178,6 @@ struct s2255_bufferi {
DEF_FDEC, DEF_BRIGHT, DEF_CONTRAST, DEF_SATURATION, \
DEF_HUE, 0, DEF_USB_BLOCK, 0}
 
-struct s2255_dmaqueue {
-   struct list_headactive;
-   struct s2255_dev*dev;
-};
-
 /* for firmware loading, fw_state */
 #define S2255_FW_NOTLOADED 0
 #define S2255_FW_LOADED_DSPWAIT1
@@ -223,7 +218,7 @@ struct s2255_channel {
struct v4l2_ctrl_handler hdl;
struct v4l2_ctrl*jpegqual_ctrl;
int resources;
-   struct s2255_dmaqueue   vidq;
+   struct list_headbuf_list;
struct s2255_bufferibuffer;
struct s2255_mode   mode;
v4l2_std_id std;
@@ -574,18 +569,17 @@ static void s2255_fwchunk_complete(struct urb *urb)
 
 static int s2255_got_frame(struct s2255_channel *channel, int jpgsize)
 {
-   struct s2255_dmaqueue *dma_q = &channel->vidq;
struct s2255_buffer *buf;
struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
unsigned long flags = 0;
int rc = 0;
spin_lock_irqsave(&dev->slock, flags);
-   if (list_empty(&dma_q->active)) {
+   if (list_empty(&channel->buf_list)) {
dprintk(dev, 1, "No active queue to serve\n");
rc = -1;
goto unlock;
}
-   buf = list_entry(dma_q->active.next,
+   buf = list_entry(channel->buf_list.next,
 struct s2255_buffer, vb.queue);
list_del(&buf->vb.queue);
v4l2_get_timestamp(&buf->vb.ts);
@@ -747,10 +741,9 @@ static void buffer_queue(struct videobuf_queue *vq, struct 
videobuf_buffer *vb)
struct s2255_buffer *buf = container_of(vb, struct s2255_buffer, vb);
struct s2255_fh *fh = vq->priv_data;
struct s2255_channel *channel = fh->channel;
-   struct s2255_dmaqueue *vidq = &channel->vidq;
dprintk(fh->dev, 1, "%s\n", __func__);
buf->vb.state = VIDEOBUF_QUEUED;
-   list_add_tail(&buf->vb.queue, &vidq->active);
+   list_add_tail(&buf->vb.queue, &channel->buf_list);
 }
 
 static void buffer_release(struct videobuf_queue *vq,
@@ -1679,11 +1672,10 @@ static int __s2255_open(struct file *file)
}
dprintk(dev, 1, "%s: dev=%s type=%s\n", __func__,
video_device_node_name(vdev), v4l2_type_names[type]);
-   dprintk(dev, 2, "%s: fh=0x%08lx, dev=0x%08lx, vidq=0x%08lx\n", __func__,
-   (unsigned long)fh, (unsigned long)dev,
-   (unsigned long)&channel->vidq);
+   dprintk(dev, 2, "%s: fh=0x%08lx, dev=0x%08lx\n", __func__,
+   (unsigned long)fh, (unsigned long)dev);
dprintk(dev, 4, "%s: list_empty active=%d\n", __func__,
-   list_empty(&channel->vidq.active));
+   list_empty(&channel->buf_list));
videobuf_queue_vmalloc_init(&fh->vb_vidq, &s2255_video_qops,
NULL, &dev->slock,
fh->type,
@@ -1876,7 +1868,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
/* register 4 video devices */
for (i = 0; i < MAX_CHANNELS; i++) {
channel = &dev->channel[i];
-   INIT_LIST_HEAD(&channel->vidq.active);
+   INIT_LIST_HEAD(&channel->buf_list);
 
v4l2_ctrl_handler_init(&channel->hdl, 6);
v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
@@ -1901,7 +1893,6 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
dev_err(&dev->udev->dev, "couldn't register control\n");
break;
}
-   channel->vidq.dev = dev;
/* register 4 video devices */
channel->vdev = template;
channel->vdev.ctrl_handler = &channel->hdl;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majo

[PATCH 1/2] m88rs2000: add caps FE_CAN_INVERSION_AUTO

2014-02-04 Thread Malcolm Priestley
This frontend is always auto inversion.

Signed-off-by: Malcolm Priestley 
Cc: sta...@vger.kernel.org # v3.9+
---
 drivers/media/dvb-frontends/m88rs2000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/m88rs2000.c 
b/drivers/media/dvb-frontends/m88rs2000.c
index b235146..ee2fec8 100644
--- a/drivers/media/dvb-frontends/m88rs2000.c
+++ b/drivers/media/dvb-frontends/m88rs2000.c
@@ -746,7 +746,7 @@ static struct dvb_frontend_ops m88rs2000_ops = {
.symbol_rate_tolerance  = 500,  /* ppm */
.caps = FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 | FE_CAN_FEC_3_4 |
  FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 |
- FE_CAN_QPSK |
+ FE_CAN_QPSK | FE_CAN_INVERSION_AUTO |
  FE_CAN_FEC_AUTO
},
 
-- 
1.9.rc1




--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] m88rs2000: add m88rs2000_get_tune_settings

2014-02-04 Thread Malcolm Priestley
Add min delay of 2000 ms on symbol rate more than 300 and
delay of 3000ms less than this.

This prevents crashing the frontend on continuous transponder scans.

Otherwise other dvb_frontend_tune_settings are the same as default.

This makes very little time difference to good channel scans, but slows down
the set frontend where lock can never be achieved i.e. DVB-S2.

Signed-off-by: Malcolm Priestley 
Cc: sta...@vger.kernel.org # v3.9+
---
 drivers/media/dvb-frontends/m88rs2000.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/dvb-frontends/m88rs2000.c 
b/drivers/media/dvb-frontends/m88rs2000.c
index ee2fec8..32cffca 100644
--- a/drivers/media/dvb-frontends/m88rs2000.c
+++ b/drivers/media/dvb-frontends/m88rs2000.c
@@ -715,6 +715,22 @@ static int m88rs2000_get_frontend(struct dvb_frontend *fe)
return 0;
 }
 
+static int m88rs2000_get_tune_settings(struct dvb_frontend *fe,
+   struct dvb_frontend_tune_settings *tune)
+{
+   struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+
+   if (c->symbol_rate > 300)
+   tune->min_delay_ms = 2000;
+   else
+   tune->min_delay_ms = 3000;
+
+   tune->step_size = c->symbol_rate / 16000;
+   tune->max_drift = c->symbol_rate / 2000;
+
+   return 0;
+}
+
 static int m88rs2000_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
 {
struct m88rs2000_state *state = fe->demodulator_priv;
@@ -766,6 +782,7 @@ static struct dvb_frontend_ops m88rs2000_ops = {
 
.set_frontend = m88rs2000_set_frontend,
.get_frontend = m88rs2000_get_frontend,
+   .get_tune_settings = m88rs2000_get_tune_settings,
 };
 
 struct dvb_frontend *m88rs2000_attach(const struct m88rs2000_config *config,
-- 
1.9.rc1


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/1] v4l: subdev: Allow 32-bit compat IOCTLs

2014-02-04 Thread Mauro Carvalho Chehab
Em Fri, 31 Jan 2014 18:35:12 +0100
Hans Verkuil  escreveu:

> Hi Sakari,
> 
> On 01/31/2014 05:15 PM, Sakari Ailus wrote:
> > I thought this was already working but apparently not. Allow 32-bit compat
> > IOCTLs on 64-bit systems.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > index 8f7a6a4..1fce944 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -1087,6 +1087,18 @@ long v4l2_compat_ioctl32(struct file *file, unsigned 
> > int cmd, unsigned long arg)
> > case VIDIOC_QUERY_DV_TIMINGS:
> > case VIDIOC_DV_TIMINGS_CAP:
> > case VIDIOC_ENUM_FREQ_BANDS:
> > +   /* Sub-device IOCTLs */
> > +   case VIDIOC_SUBDEV_G_FMT:
> > +   case VIDIOC_SUBDEV_S_FMT:
> > +   case VIDIOC_SUBDEV_G_FRAME_INTERVAL:
> > +   case VIDIOC_SUBDEV_S_FRAME_INTERVAL:
> > +   case VIDIOC_SUBDEV_ENUM_MBUS_CODE:
> > +   case VIDIOC_SUBDEV_ENUM_FRAME_SIZE:
> > +   case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL:
> > +   case VIDIOC_SUBDEV_G_CROP:
> > +   case VIDIOC_SUBDEV_S_CROP:
> > +   case VIDIOC_SUBDEV_G_SELECTION:
> > +   case VIDIOC_SUBDEV_S_SELECTION:
> > case VIDIOC_SUBDEV_G_EDID32:
> > case VIDIOC_SUBDEV_S_EDID32:
> > ret = do_video_ioctl(file, cmd, arg);
> > 
> 
> Can you test with contrib/test/ioctl-test? Compile with:
> 
> gcc -o ioctl-test -m32 -I ../../include/ ioctl-test.c
> 
> Make sure you use the latest v4l-utils version and run autoreconf -vfi
> and configure first.
> 
> BTW, I noticed that VIDIOC_DBG_G_CHIP_INFO is missing as well.
> 
> Hmm, this is just asking for problems. 
> 
> How about this patch:
> 
> Signed-off-by: Hans Verkuil 
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 8f7a6a4..cd9da4ce 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -1001,108 +1001,19 @@ static long do_video_ioctl(struct file *file, 
> unsigned int cmd, unsigned long ar
>  long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long 
> arg)
>  {
>   struct video_device *vdev = video_devdata(file);
> - long ret = -ENOIOCTLCMD;
> + long ret = -ENOTTY;
>  
>   if (!file->f_op->unlocked_ioctl)
>   return ret;
>  
> - switch (cmd) {
> - case VIDIOC_QUERYCAP:
> - case VIDIOC_RESERVED:
> - case VIDIOC_ENUM_FMT:
> - case VIDIOC_G_FMT32:
> - case VIDIOC_S_FMT32:
> - case VIDIOC_REQBUFS:
> - case VIDIOC_QUERYBUF32:
> - case VIDIOC_G_FBUF32:
> - case VIDIOC_S_FBUF32:
> - case VIDIOC_OVERLAY32:
> - case VIDIOC_QBUF32:
> - case VIDIOC_EXPBUF:
> - case VIDIOC_DQBUF32:
> - case VIDIOC_STREAMON32:
> - case VIDIOC_STREAMOFF32:
> - case VIDIOC_G_PARM:
> - case VIDIOC_S_PARM:
> - case VIDIOC_G_STD:
> - case VIDIOC_S_STD:
> - case VIDIOC_ENUMSTD32:
> - case VIDIOC_ENUMINPUT32:
> - case VIDIOC_G_CTRL:
> - case VIDIOC_S_CTRL:
> - case VIDIOC_G_TUNER:
> - case VIDIOC_S_TUNER:
> - case VIDIOC_G_AUDIO:
> - case VIDIOC_S_AUDIO:
> - case VIDIOC_QUERYCTRL:
> - case VIDIOC_QUERYMENU:
> - case VIDIOC_G_INPUT32:
> - case VIDIOC_S_INPUT32:
> - case VIDIOC_G_OUTPUT32:
> - case VIDIOC_S_OUTPUT32:
> - case VIDIOC_ENUMOUTPUT:
> - case VIDIOC_G_AUDOUT:
> - case VIDIOC_S_AUDOUT:
> - case VIDIOC_G_MODULATOR:
> - case VIDIOC_S_MODULATOR:
> - case VIDIOC_S_FREQUENCY:
> - case VIDIOC_G_FREQUENCY:
> - case VIDIOC_CROPCAP:
> - case VIDIOC_G_CROP:
> - case VIDIOC_S_CROP:
> - case VIDIOC_G_SELECTION:
> - case VIDIOC_S_SELECTION:
> - case VIDIOC_G_JPEGCOMP:
> - case VIDIOC_S_JPEGCOMP:
> - case VIDIOC_QUERYSTD:
> - case VIDIOC_TRY_FMT32:
> - case VIDIOC_ENUMAUDIO:
> - case VIDIOC_ENUMAUDOUT:
> - case VIDIOC_G_PRIORITY:
> - case VIDIOC_S_PRIORITY:
> - case VIDIOC_G_SLICED_VBI_CAP:
> - case VIDIOC_LOG_STATUS:
> - case VIDIOC_G_EXT_CTRLS32:
> - case VIDIOC_S_EXT_CTRLS32:
> - case VIDIOC_TRY_EXT_CTRLS32:
> - case VIDIOC_ENUM_FRAMESIZES:
> - case VIDIOC_ENUM_FRAMEINTERVALS:
> - case VIDIOC_G_ENC_INDEX:
> - case VIDIOC_ENCODER_CMD:
> - case VIDIOC_TRY_ENCODER_CMD:
> - case VIDIOC_DECODER_CMD:
> - case VIDIOC_TRY_DECODER_CMD:
> - case VIDIOC_DBG_S_REGISTER:
> - case VIDIOC_DBG_G_REGISTER:
> - case VIDIOC_S_HW_FREQ_SEEK:
> - case VIDIOC_S_DV_TIMINGS:
> - case VIDIOC_G_DV_TIMINGS:
> - case VIDIOC_DQEVENT:
> - case VIDIOC_DQEVENT32:
> - case VIDIOC_SUBSCRIBE_EVENT:
> - case VIDIOC_UNSUBSCRIBE_EVENT:
> - case VIDIOC_CREATE_BUFS32:
> - case VIDIOC_PREPARE_BUF32:
> - case VID

Re: [PATCH 2/4] e4000: implement controls via v4l2 control framework

2014-02-04 Thread Antti Palosaari

Moi Hans

On 04.02.2014 20:43, Hans Verkuil wrote:

On 02/04/2014 02:39 AM, Antti Palosaari wrote:

Implement gain and bandwidth controls using v4l2 control framework.
Pointer to control handler is provided by exported symbol.

Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Signed-off-by: Antti Palosaari 
---
  drivers/media/tuners/e4000.c  | 142 +-
  drivers/media/tuners/e4000.h  |  14 
  drivers/media/tuners/e4000_priv.h |  12 
  3 files changed, 167 insertions(+), 1 deletion(-)



+static int e4000_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct e4000_priv *priv =
+   container_of(ctrl->handler, struct e4000_priv, hdl);
+   struct dvb_frontend *fe = priv->fe;
+   struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+   int ret;
+   dev_dbg(&priv->client->dev,
+   "%s: id=%d name=%s val=%d min=%d max=%d step=%d\n",
+   __func__, ctrl->id, ctrl->name, ctrl->val,
+   ctrl->minimum, ctrl->maximum, ctrl->step);
+
+   switch (ctrl->id) {
+   case V4L2_CID_BANDWIDTH_AUTO:
+   case V4L2_CID_BANDWIDTH:
+   c->bandwidth_hz = priv->bandwidth->val;
+   ret = e4000_set_params(priv->fe);
+   break;
+   case  V4L2_CID_LNA_GAIN_AUTO:
+   case  V4L2_CID_LNA_GAIN:
+   case  V4L2_CID_MIXER_GAIN_AUTO:
+   case  V4L2_CID_MIXER_GAIN:
+   case  V4L2_CID_IF_GAIN_AUTO:
+   case  V4L2_CID_IF_GAIN:
+   ret = e4000_set_gain(priv->fe);


That won't work. You need to handle each gain cluster separately. The control
framework processes the controls one cluster at a time and takes a lock on the
master control before calling s_ctrl. The ctrl->val field is only valid inside
s_ctrl for the controls in the cluster, not for other controls. For other
controls only the ctrl->cur.val field is valid.


hmm, actually it worked fine on my tests - but I think see your point. 
It likely woks as my app sets one control per call, but if you try to 
set multiple controls then it go out of sync I think.


I am going to split that gain function to three pieces then.

regards
Antti

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] [media] mceusb: remove redundant function and defines

2014-02-04 Thread Mauro Carvalho Chehab
Hi Sean,

Em Mon, 20 Jan 2014 22:10:43 +
Sean Young  escreveu:

Could you please provide a patch description? Even simple ones should have,
and this one is everything but trivial...

Also, you should likely break it into smaller changesets. For example, the
last hunk adding a usb_kill_urb() looks more like a bugfix than a pure
cleanup change.

Thanks!
Mauro

> Signed-off-by: Sean Young 
> ---
>  drivers/media/rc/mceusb.c | 92 
> +++
>  1 file changed, 28 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
> index a25bb15..3a4f95f 100644
> --- a/drivers/media/rc/mceusb.c
> +++ b/drivers/media/rc/mceusb.c
> @@ -166,15 +166,6 @@ static bool debug;
>   dev_info(dev, fmt, ## __VA_ARGS__); \
>   } while (0)
>  
> -/* general constants */
> -#define SEND_FLAG_IN_PROGRESS1
> -#define SEND_FLAG_COMPLETE   2
> -#define RECV_FLAG_IN_PROGRESS3
> -#define RECV_FLAG_COMPLETE   4
> -
> -#define MCEUSB_RX1
> -#define MCEUSB_TX2
> -
>  #define VENDOR_PHILIPS   0x0471
>  #define VENDOR_SMK   0x0609
>  #define VENDOR_TATUNG0x1460
> @@ -452,7 +443,6 @@ struct mceusb_dev {
>   } flags;
>  
>   /* transmit support */
> - int send_flags;
>   u32 carrier;
>   unsigned char tx_mask;
>  
> @@ -731,45 +721,29 @@ static void mce_async_callback(struct urb *urb)
>  
>  /* request incoming or send outgoing usb packet - used to initialize remote 
> */
>  static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
> -int size, int urb_type)
> +int size)
>  {
>   int res, pipe;
>   struct urb *async_urb;
>   struct device *dev = ir->dev;
>   unsigned char *async_buf;
>  
> - if (urb_type == MCEUSB_TX) {
> - async_urb = usb_alloc_urb(0, GFP_KERNEL);
> - if (unlikely(!async_urb)) {
> - dev_err(dev, "Error, couldn't allocate urb!\n");
> - return;
> - }
> -
> - async_buf = kzalloc(size, GFP_KERNEL);
> - if (!async_buf) {
> - dev_err(dev, "Error, couldn't allocate buf!\n");
> - usb_free_urb(async_urb);
> - return;
> - }
> + async_urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (unlikely(!async_urb))
> + return;
>  
> - /* outbound data */
> - pipe = usb_sndintpipe(ir->usbdev,
> -   ir->usb_ep_out->bEndpointAddress);
> - usb_fill_int_urb(async_urb, ir->usbdev, pipe,
> - async_buf, size, mce_async_callback,
> - ir, ir->usb_ep_out->bInterval);
> - memcpy(async_buf, data, size);
> -
> - } else if (urb_type == MCEUSB_RX) {
> - /* standard request */
> - async_urb = ir->urb_in;
> - ir->send_flags = RECV_FLAG_IN_PROGRESS;
> -
> - } else {
> - dev_err(dev, "Error! Unknown urb type %d\n", urb_type);
> + async_buf = kmalloc(size, GFP_KERNEL);
> + if (!async_buf) {
> + usb_free_urb(async_urb);
>   return;
>   }
>  
> + /* outbound data */
> + pipe = usb_sndintpipe(ir->usbdev, ir->usb_ep_out->bEndpointAddress);
> + usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf, size,
> + mce_async_callback, ir, ir->usb_ep_out->bInterval);
> + memcpy(async_buf, data, size);
> +
>   mce_dbg(dev, "receive request called (size=%#x)\n", size);
>  
>   async_urb->transfer_buffer_length = size;
> @@ -789,19 +763,14 @@ static void mce_async_out(struct mceusb_dev *ir, 
> unsigned char *data, int size)
>  
>   if (ir->need_reset) {
>   ir->need_reset = false;
> - mce_request_packet(ir, DEVICE_RESUME, rsize, MCEUSB_TX);
> + mce_request_packet(ir, DEVICE_RESUME, rsize);
>   msleep(10);
>   }
>  
> - mce_request_packet(ir, data, size, MCEUSB_TX);
> + mce_request_packet(ir, data, size);
>   msleep(10);
>  }
>  
> -static void mce_flush_rx_buffer(struct mceusb_dev *ir, int size)
> -{
> - mce_request_packet(ir, NULL, size, MCEUSB_RX);
> -}
> -
>  /* Send data out the IR blaster port(s) */
>  static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  {
> @@ -1040,7 +1009,6 @@ static void mceusb_process_ir_data(struct mceusb_dev 
> *ir, int buf_len)
>  static void mceusb_dev_recv(struct urb *urb)
>  {
>   struct mceusb_dev *ir;
> - int buf_len;
>  
>   if (!urb)
>   return;
> @@ -1051,18 +1019,10 @@ static void mceusb_dev_recv(struct urb *urb)
>   return;
>   }
>  
> - buf_len = urb->actual_length;
> -
> - if (ir->send_flags == RECV_FLAG_IN_PROGRESS) {
> - ir->send_flags = SEND_FLAG_COMPL

Re: [PATCH] s2255drv: port to videobuf2

2014-02-04 Thread Dean Anderson

On 2014-02-04 04:04, Hans Verkuil wrote:

Hi Dean,

On 02/03/14 18:06, Dean Anderson wrote:

On 2014-02-03 03:51, Hans Verkuil wrote:

Hi Dean,

Some specific comments below, but first two general comments:

It is easier to review if at least the removal of the old s2255_fh 
struct
was done as a separate patch. It's always good to try and keep the 
changes
in patches as small as possible. The actual vb2 conversion is always 
a
'big bang' patch, that's unavoidable, but it's easier if it isn't 
mixed in
with other changes that are not directly related to the vb2 
conversion.



I figured removal of s2255_fh was a natural part of the videobuf2 
conversion process, but I can break it up.


It's more like the first phase of a vb2 conversion. It really is wrong
for videobuf as well, so it makes sense to do that first.


Hans, if it's ok with you, I'd prefer to do it after the vb2 
conversion.  Right now, it's using SAA7134-style locking with resources 
in fh.  I could use "videobuf_queue_is_busy" from videobuf-core.c, but 
it has this comment "/* Locking: Only usage in bttv unsafe find way to 
remove */".


Thanks,

Dean






I also did change some formatting and naming changes (s2255_channel 
to s2255_vc) that can be postponed.


Just put it in a separate patch either before or after the patch that 
does

the vb2 conversion.





And did you also run the v4l2-compliance utility for this driver? 
That's

useful to check that everything it still correct.


Thanks for the comments.  I'll do a v2 soon with v4l2-compliance 
fully tested too.


Rather than the standard v4l2-compliance from v4l-utils, can you use 
this

from my own tree:

http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/streaming

I've started work to add tests for streaming to v4l2-compliance. While 
not
complete it should cover what the s2255 driver needs. I'm very 
interested

in what it finds (or, as the case might be, what it doesn't find).

In order to do the streaming tests you have to run it with option -s.

Regards,

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] em28xx-i2c: remove duplicate error printing code from em28xx_i2c_xfer()

2014-02-04 Thread Mauro Carvalho Chehab
Em Sun, 19 Jan 2014 22:48:37 +0100
Frank Schäfer  escreveu:

> Signed-off-by: Frank Schäfer 
> ---
>  drivers/media/usb/em28xx/em28xx-i2c.c |   11 +++
>  1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
> b/drivers/media/usb/em28xx/em28xx-i2c.c
> index a26d7d4..1a514ca 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -535,14 +535,9 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
>* This code is only called during device probe.
>*/
>   rc = i2c_check_for_device(i2c_bus, addr);
> - if (rc < 0) {
> - if (rc == -ENXIO) {
> - if (i2c_debug > 1)
> - printk(KERN_CONT " no 
> device\n");
> - } else {
> - if (i2c_debug > 1)
> - printk(KERN_CONT " ERROR: 
> %i\n", rc);
> - }
> + if (rc == -ENXIO) {
> + if (i2c_debug > 1)
> + printk(KERN_CONT " no device\n");

Even if the previous patch were accepted, this one is wrong, as -ENXIO
doesn't always mean that there's no device. Also, other return codes
may happen here (like -EIO).

>   rt_mutex_unlock(&dev->i2c_bus_lock);
>   return rc;
>   }


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] em28xx-i2c: do not map -ENXIO errors to -ENODEV for empty i2c transfers

2014-02-04 Thread Mauro Carvalho Chehab
Em Sun, 19 Jan 2014 22:48:36 +0100
Frank Schäfer  escreveu:

> Commit e63b009d6e "" changed the error codes i2c ACK errors from -ENODEV to 
> -ENXIO.
> But it also introduced a line that maps -ENXIO back to -ENODEV in case of 
> empty i2c
> messages, which makes no sense, because
> 1.) an ACK error is an ACK error no matter what the i2c message content is
> 2.) -ENXIO is perfectly suited for probing, too

I don't agree with this patch. 0-byte messages are only usin during device
probe.

> 3.) we are loosing the ability to distinguish USB device disconnects

Huh?

> 
> Signed-off-by: Frank Schäfer 
> ---
>  drivers/media/usb/em28xx/em28xx-i2c.c |1 -
>  1 Datei geändert, 1 Zeile entfernt(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
> b/drivers/media/usb/em28xx/em28xx-i2c.c
> index ba6433c..a26d7d4 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -539,7 +539,6 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
>   if (rc == -ENXIO) {
>   if (i2c_debug > 1)
>   printk(KERN_CONT " no 
> device\n");
> - rc = -ENODEV;
>   } else {
>   if (i2c_debug > 1)
>   printk(KERN_CONT " ERROR: 
> %i\n", rc);


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] e4000: implement controls via v4l2 control framework

2014-02-04 Thread Hans Verkuil
On 02/04/2014 02:39 AM, Antti Palosaari wrote:
> Implement gain and bandwidth controls using v4l2 control framework.
> Pointer to control handler is provided by exported symbol.
> 
> Cc: Mauro Carvalho Chehab 
> Cc: Hans Verkuil 
> Signed-off-by: Antti Palosaari 
> ---
>  drivers/media/tuners/e4000.c  | 142 
> +-
>  drivers/media/tuners/e4000.h  |  14 
>  drivers/media/tuners/e4000_priv.h |  12 
>  3 files changed, 167 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> index 9187190..11d31b0 100644
> --- a/drivers/media/tuners/e4000.c
> +++ b/drivers/media/tuners/e4000.c
> @@ -448,6 +448,110 @@ err:
>   return ret;
>  }
>  
> +static int e4000_set_gain(struct dvb_frontend *fe)
> +{
> + struct e4000_priv *priv = fe->tuner_priv;
> + int ret;
> + u8 buf[2];
> + u8 u8tmp;
> + dev_dbg(&priv->client->dev, "%s: lna=%d mixer=%d if=%d\n", __func__,
> + priv->lna_gain->val, priv->mixer_gain->val,
> + priv->if_gain->val);
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 1);
> +
> + if (priv->lna_gain_auto->val && priv->if_gain_auto->val)
> + u8tmp = 0x17;
> + else if (priv->lna_gain_auto->val)
> + u8tmp = 0x19;
> + else if (priv->if_gain_auto->val)
> + u8tmp = 0x16;
> + else
> + u8tmp = 0x10;
> +
> + ret = e4000_wr_reg(priv, 0x1a, u8tmp);
> + if (ret)
> + goto err;
> +
> + if (priv->mixer_gain_auto->val)
> + u8tmp = 0x15;
> + else
> + u8tmp = 0x14;
> +
> + ret = e4000_wr_reg(priv, 0x20, u8tmp);
> + if (ret)
> + goto err;
> +
> + if (priv->lna_gain_auto->val == false) {
> + ret = e4000_wr_reg(priv, 0x14, priv->lna_gain->val);
> + if (ret)
> + goto err;
> + }
> +
> + if (priv->mixer_gain_auto->val == false) {
> + ret = e4000_wr_reg(priv, 0x15, priv->mixer_gain->val);
> + if (ret)
> + goto err;
> + }
> +
> + if (priv->if_gain_auto->val == false) {
> + buf[0] = e4000_if_gain_lut[priv->if_gain->val].reg16_val;
> + buf[1] = e4000_if_gain_lut[priv->if_gain->val].reg17_val;
> + ret = e4000_wr_regs(priv, 0x16, buf, 2);
> + if (ret)
> + goto err;
> + }
> +
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +
> + return 0;
> +err:
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> +
> + dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
> + return ret;
> +}
> +
> +static int e4000_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct e4000_priv *priv =
> + container_of(ctrl->handler, struct e4000_priv, hdl);
> + struct dvb_frontend *fe = priv->fe;
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> + int ret;
> + dev_dbg(&priv->client->dev,
> + "%s: id=%d name=%s val=%d min=%d max=%d step=%d\n",
> + __func__, ctrl->id, ctrl->name, ctrl->val,
> + ctrl->minimum, ctrl->maximum, ctrl->step);
> +
> + switch (ctrl->id) {
> + case V4L2_CID_BANDWIDTH_AUTO:
> + case V4L2_CID_BANDWIDTH:
> + c->bandwidth_hz = priv->bandwidth->val;
> + ret = e4000_set_params(priv->fe);
> + break;
> + case  V4L2_CID_LNA_GAIN_AUTO:
> + case  V4L2_CID_LNA_GAIN:
> + case  V4L2_CID_MIXER_GAIN_AUTO:
> + case  V4L2_CID_MIXER_GAIN:
> + case  V4L2_CID_IF_GAIN_AUTO:
> + case  V4L2_CID_IF_GAIN:
> + ret = e4000_set_gain(priv->fe);

That won't work. You need to handle each gain cluster separately. The control
framework processes the controls one cluster at a time and takes a lock on the
master control before calling s_ctrl. The ctrl->val field is only valid inside
s_ctrl for the controls in the cluster, not for other controls. For other
controls only the ctrl->cur.val field is valid.

Regards,

Hans

> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops e4000_ctrl_ops = {
> + .s_ctrl = e4000_s_ctrl,
> +};
> +
>  static const struct dvb_tuner_ops e4000_tuner_ops = {
>   .info = {
>   .name   = "Elonics E4000",
> @@ -463,6 +567,13 @@ static const struct dvb_tuner_ops e4000_tuner_ops = {
>   .get_if_frequency = e4000_get_if_frequency,
>  };
>  
> +struct v4l2_ctrl_handler *e4000_get_ctrl_handler(struct dvb_frontend *fe)
> +{
> + struct e4000_priv *priv = fe->tuner_priv;
> + return &priv->hdl;
> +}
> +EXPORT_SYMBOL(e4000_get_ctrl_handler);
> +
>  static int e4000_probe(struct i2c_client *client,
>   const struct i2c_device_id *id)
>

[git:media_tree/master] [media] media: rc: add raw decoder for Sharp protocol

2014-02-04 Thread Mauro Carvalho Chehab
This is an automatic generated email to let you know that the following patch 
were queued at the 
http://git.linuxtv.org/media_tree.git tree:

Subject: [media] media: rc: add raw decoder for Sharp protocol
Author:  James Hogan 
Date:Fri Jan 17 10:58:48 2014 -0300

Add a raw decoder for the Sharp protocol. It uses a pulse distance
modulation with a pulse of 320us and a bit period of 2ms for a logical 1
and 1ms for a logical 0. The first part of the message consists of a
5-bit address, an 8-bit command, and two other bits, followed by a 40ms
gap before the echo message which is an inverted version of the main
message except for the address bits.

Signed-off-by: James Hogan 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab 

 drivers/media/rc/Kconfig|9 ++
 drivers/media/rc/Makefile   |1 +
 drivers/media/rc/ir-sharp-decoder.c |  200 +++
 drivers/media/rc/rc-core-priv.h |6 +
 4 files changed, 216 insertions(+), 0 deletions(-)

---

http://git.linuxtv.org/media_tree.git?a=commitdiff;h=1d184b0bc13d49108e571033fc00f3b5f32670e1

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 904f113..3b25887 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -106,6 +106,15 @@ config IR_SANYO_DECODER
   uses the Sanyo protocol (Sanyo, Aiwa, Chinon remotes),
   and you need software decoding support.
 
+config IR_SHARP_DECODER
+   tristate "Enable IR raw decoder for the Sharp protocol"
+   depends on RC_CORE
+   default y
+
+   ---help---
+  Enable this option if you have an infrared remote control which
+  uses the Sharp protocol, and you need software decoding support.
+
 config IR_MCE_KBD_DECODER
tristate "Enable IR raw decoder for the MCE keyboard/mouse protocol"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index f4eb32c..36dafed 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_IR_JVC_DECODER) += ir-jvc-decoder.o
 obj-$(CONFIG_IR_SONY_DECODER) += ir-sony-decoder.o
 obj-$(CONFIG_IR_RC5_SZ_DECODER) += ir-rc5-sz-decoder.o
 obj-$(CONFIG_IR_SANYO_DECODER) += ir-sanyo-decoder.o
+obj-$(CONFIG_IR_SHARP_DECODER) += ir-sharp-decoder.o
 obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
 obj-$(CONFIG_IR_LIRC_CODEC) += ir-lirc-codec.o
 
diff --git a/drivers/media/rc/ir-sharp-decoder.c 
b/drivers/media/rc/ir-sharp-decoder.c
new file mode 100644
index 000..4c17be5
--- /dev/null
+++ b/drivers/media/rc/ir-sharp-decoder.c
@@ -0,0 +1,200 @@
+/* ir-sharp-decoder.c - handle Sharp IR Pulse/Space protocol
+ *
+ * Copyright (C) 2013-2014 Imagination Technologies Ltd.
+ *
+ * Based on NEC decoder:
+ * Copyright (C) 2010 by Mauro Carvalho Chehab 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include "rc-core-priv.h"
+
+#define SHARP_NBITS15
+#define SHARP_UNIT 4  /* ns */
+#define SHARP_BIT_PULSE(8* SHARP_UNIT) /* 320us */
+#define SHARP_BIT_0_PERIOD (25   * SHARP_UNIT) /* 1ms (680us space) */
+#define SHARP_BIT_1_PERIOD (50   * SHARP_UNIT) /* 2ms (1680ms space) */
+#define SHARP_ECHO_SPACE   (1000 * SHARP_UNIT) /* 40 ms */
+#define SHARP_TRAILER_SPACE(125  * SHARP_UNIT) /* 5 ms (even longer) */
+
+enum sharp_state {
+   STATE_INACTIVE,
+   STATE_BIT_PULSE,
+   STATE_BIT_SPACE,
+   STATE_TRAILER_PULSE,
+   STATE_ECHO_SPACE,
+   STATE_TRAILER_SPACE,
+};
+
+/**
+ * ir_sharp_decode() - Decode one Sharp pulse or space
+ * @dev:   the struct rc_dev descriptor of the device
+ * @duration:  the struct ir_raw_event descriptor of the pulse/space
+ *
+ * This function returns -EINVAL if the pulse violates the state machine
+ */
+static int ir_sharp_decode(struct rc_dev *dev, struct ir_raw_event ev)
+{
+   struct sharp_dec *data = &dev->raw->sharp;
+   u32 msg, echo, address, command, scancode;
+
+   if (!(dev->enabled_protocols & RC_BIT_SHARP))
+   return 0;
+
+   if (!is_timing_event(ev)) {
+   if (ev.reset)
+   data->state = STATE_INACTIVE;
+   return 0;
+   }
+
+   IR_dprintk(2, "Sharp decode started at state %d (%uus %s)\n",
+  data->state, TO_US(ev.duration), TO_STR(ev.pulse));
+
+   switch (data->state) {
+
+   case STATE_INACTIVE:
+   if (!ev.pulse)
+   break;
+

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

2014-02-04 Thread Mauro Carvalho Chehab
Em Thu, 23 Jan 2014 21:11:09 +0200
Antti Seppälä  escreveu:

> On 23 January 2014 00:01, Mauro Carvalho Chehab  wrote:
> > Not sure if you saw it, but there's already another patchset proposing
> > that, that got submitted before this changeset:
> > https://patchwork.linuxtv.org/patch/21625/
> 
> I actually didn't notice that until now. Seems quite a similar kind of
> approach with even more advanced features than what I had in mind
> (namely the scancode filtering and masking).
> 
> However it looks like that patchset has the same drawback about not
> knowing which protocol to use for the wakeup scancode as was pointed
> from my patch.

Well, the protocol is important only if there are two or more active
RCs that produce the same IR code on different protocols.

Also, from the sysfs description made by James, it seems clear to me
that the protocol to be used is the current protocol.

I think is an unlikely border case to have some hardware that supports
more than one IR protocols enabled for the wakeup to work, so James
patch looks ok on my eyes.

Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node
to allow to filter the wakeup scancode to a protocol set different than
the one(s) currently enabled.

> I think I'll try to come up with a new patch addressing the comments
> I've seen so far.

I guess I'll merge James approach, as there are a pile of other patches
depending on it. If we need latter to distinguish between current_protocol
and the wakeup one, as I said, a latter patch can add a
"wakeup_filter_protocol" sysfs node to specify it.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: linux-next: Tree for Feb 4 (media/radio/si4713/radio-usb-si4713.c)

2014-02-04 Thread Randy Dunlap
On 02/03/2014 09:07 PM, Stephen Rothwell wrote:
> Hi all,
> 
> This tree fails (more than usual) the powerpc allyesconfig build.
> 
> Changes since 20140203:
> 

on i386:

# CONFIG_I2C is not set

  CC [M]  drivers/media/radio/si4713/radio-usb-si4713.o
drivers/media/radio/si4713/radio-usb-si4713.c: In function 
'usb_si4713_video_device_release':
drivers/media/radio/si4713/radio-usb-si4713.c:147:2: error: implicit 
declaration of function 'i2c_del_adapter' 
[-Werror=implicit-function-declaration]
drivers/media/radio/si4713/radio-usb-si4713.c: In function 
'si4713_register_i2c_adapter':
drivers/media/radio/si4713/radio-usb-si4713.c:424:2: error: implicit 
declaration of function 'i2c_add_adapter' 
[-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[5]: *** [drivers/media/radio/si4713/radio-usb-si4713.o] Error 1



-- 
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help wanted patching saa7164 driver to work with Compro E900F

2014-02-04 Thread Steven Toth
>> Where did you purchase the card?
>
> Hi Steve
>
> It is a pleasure, I'm finding it very interesting, and it would
> certainly be satisfying if I did get it working. Thankyou for writing
> the driver!

You are welcome!

>
> I bought it from either msy.com.au or skycomp.com.au quite a few years
> ago, but I don't see it listed anywhere now in Australia except here
> (http://www.foxcomp.com.au/ProductDetail.asp?ID=10591,
> http://www.ht.com.au/part/AF826-Compro-VideoMate-Vista-E900F-DVB-T-HDTV-receiver-analogue-TV-radio-tuner-video-input-adapter-PCIe/detail.hts).
> I can't see many tv tuners for sale anymore.

Crazy prices, a sign that its generally no longer available in quantity.

HVR2200 is still a good option and generally available worldwide.

I did find a E900F available in the UK (amazon - roughly $40 USD) but
they won't ship it overseas.

Grrr.

- Steve

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fw: [PATCH 34/52] devices.txt: add video4linux device for Software Defined Radio

2014-02-04 Thread Greg Kroah-Hartman
On Tue, Feb 04, 2014 at 11:19:03AM -0200, Mauro Carvalho Chehab wrote:
> Alan/Greg/Andrew/Rob,
> 
> Not sure who is currently maintaining Documentation/devices.txt.
> 
> We're needing to add support of a new type of V4L2 devices there.
> 
> Could you please ack with the following patch? If this one is ok, I intend
> to send via my tree together with the patch series that implements support
> for it, if you agree.
> 
> Thank you!
> Mauro
> 
> Forwarded message:
> 
> Date: Sat, 25 Jan 2014 19:10:28 +0200
> From: Antti Palosaari 
> To: linux-media@vger.kernel.org
> Cc: Antti Palosaari , Hans Verkuil 
> Subject: [PATCH 34/52] devices.txt: add video4linux device for Software 
> Defined Radio
> 
> 
> Add new video4linux device named /dev/swradio for Software Defined
> Radio use. V4L device minor numbers are allocated dynamically
> nowadays, but there is still configuration option for old fixed style.
> Add note to mention that configuration option too.
> 
> Cc: Hans Verkuil 
> Signed-off-by: Antti Palosaari 
> Acked-by: Hans Verkuil 

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help wanted patching saa7164 driver to work with Compro E900F

2014-02-04 Thread Daniel Playfair Cal
On Wed, Feb 5, 2014 at 12:32 AM, Steven Toth  wrote:
> On Tue, Feb 4, 2014 at 5:29 AM, Daniel Playfair Cal
.
>
> Hey Daniel,
>
> Thanks for showing an interest in the driver.
>
> I've been trying to source one of these cards for quite a while,
> occasionally checking amazon etc. I can't seem to find anything in
> retail on amazon or on ebay.
>
> Where did you purchase the card?

Hi Steve

It is a pleasure, I'm finding it very interesting, and it would
certainly be satisfying if I did get it working. Thankyou for writing
the driver!

I bought it from either msy.com.au or skycomp.com.au quite a few years
ago, but I don't see it listed anywhere now in Australia except here
(http://www.foxcomp.com.au/ProductDetail.asp?ID=10591,
http://www.ht.com.au/part/AF826-Compro-VideoMate-Vista-E900F-DVB-T-HDTV-receiver-analogue-TV-radio-tuner-video-input-adapter-PCIe/detail.hts).
I can't see many tv tuners for sale anymore.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help wanted patching saa7164 driver to work with Compro E900F

2014-02-04 Thread Steven Toth
On Tue, Feb 4, 2014 at 5:29 AM, Daniel Playfair Cal
 wrote:
> Hello all,
>
> I've been working on patching the saa7164 driver to work with the
> Compro Videomate Vista E900F. This is a PCI-e tuner card that seems to
> be almost identical to the Hauppauge HVR-2200, many versions of which
> are supported. I've had some success but there are a few problems

Hey Daniel,

Thanks for showing an interest in the driver.

I've been trying to source one of these cards for quite a while,
occasionally checking amazon etc. I can't seem to find anything in
retail on amazon or on ebay.

Where did you purchase the card?

- Steve

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 34/52] devices.txt: add video4linux device for Software Defined Radio

2014-02-04 Thread Alan Cox
On Tue, 04 Feb 2014 11:19:03 -0200
Mauro Carvalho Chehab  wrote:

> Alan/Greg/Andrew/Rob,
> 
> Not sure who is currently maintaining Documentation/devices.txt.
> 
> We're needing to add support of a new type of V4L2 devices there.
> 
> Could you please ack with the following patch? If this one is ok, I intend
> to send via my tree together with the patch series that implements support
> for it, if you agree.
> 
> Thank you!
> Mauro
> 
> Forwarded message:
> 
> Date: Sat, 25 Jan 2014 19:10:28 +0200
> From: Antti Palosaari 
> To: linux-media@vger.kernel.org
> Cc: Antti Palosaari , Hans Verkuil 
> Subject: [PATCH 34/52] devices.txt: add video4linux device for Software 
> Defined Radio
> 
> 
> Add new video4linux device named /dev/swradio for Software Defined
> Radio use. V4L device minor numbers are allocated dynamically
> nowadays, but there is still configuration option for old fixed style.
> Add note to mention that configuration option too.
> 
> Cc: Hans Verkuil 
> Signed-off-by: Antti Palosaari 
> Acked-by: Hans Verkuil 

Acked-by: Alan Cox 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Fw: [PATCH 34/52] devices.txt: add video4linux device for Software Defined Radio

2014-02-04 Thread Mauro Carvalho Chehab
Alan/Greg/Andrew/Rob,

Not sure who is currently maintaining Documentation/devices.txt.

We're needing to add support of a new type of V4L2 devices there.

Could you please ack with the following patch? If this one is ok, I intend
to send via my tree together with the patch series that implements support
for it, if you agree.

Thank you!
Mauro

Forwarded message:

Date: Sat, 25 Jan 2014 19:10:28 +0200
From: Antti Palosaari 
To: linux-media@vger.kernel.org
Cc: Antti Palosaari , Hans Verkuil 
Subject: [PATCH 34/52] devices.txt: add video4linux device for Software Defined 
Radio


Add new video4linux device named /dev/swradio for Software Defined
Radio use. V4L device minor numbers are allocated dynamically
nowadays, but there is still configuration option for old fixed style.
Add note to mention that configuration option too.

Cc: Hans Verkuil 
Signed-off-by: Antti Palosaari 
Acked-by: Hans Verkuil 
---
 Documentation/devices.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 80b7241..e852855 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -1490,10 +1490,17 @@ Your cooperation is appreciated.
 64 = /dev/radio0   Radio device
...
127 = /dev/radio63  Radio device
+   128 = /dev/swradio0 Software Defined Radio device
+   ...
+   191 = /dev/swradio63Software Defined Radio device
224 = /dev/vbi0 Vertical blank interrupt
...
255 = /dev/vbi31Vertical blank interrupt
 
+   Minor numbers are allocated dynamically unless
+   CONFIG_VIDEO_FIXED_MINOR_RANGES (default n)
+   configuration option is set.
+
  81 block  I2O hard disk
  0 = /dev/i2o/hdq  17th I2O hard disk, whole disk
 16 = /dev/i2o/hdr  18th I2O hard disk, whole disk
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[REVIEW PATCH] DocBook media: partial rewrite of "Opening and Closing Devices"

2014-02-04 Thread Hans Verkuil
This section was horribly out of date. A lot of references to old and
obsolete behavior have been dropped.

Same as https://patchwork.linuxtv.org/patch/21620/, but with the text
", with the exception of overlay support." dropped since 'overlay' is
not a function in that sense.

Signed-off-by: Hans Verkuil 
---
 Documentation/DocBook/media/v4l/common.xml | 191 +++--
 Documentation/DocBook/media/v4l/v4l2.xml   |   2 +-
 2 files changed, 70 insertions(+), 123 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/common.xml 
b/Documentation/DocBook/media/v4l/common.xml
index 0936dd2..71f6bf9 100644
--- a/Documentation/DocBook/media/v4l/common.xml
+++ b/Documentation/DocBook/media/v4l/common.xml
@@ -38,70 +38,41 @@ the basic concepts applicable to all devices.
 
   V4L2 drivers are implemented as kernel modules, loaded
 manually by the system administrator or automatically when a device is
-first opened. The driver modules plug into the "videodev" kernel
+first discovered. The driver modules plug into the "videodev" kernel
 module. It provides helper functions and a common application
 interface specified in this document.
 
   Each driver thus loaded registers one or more device nodes
-with major number 81 and a minor number between 0 and 255. Assigning
-minor numbers to V4L2 devices is entirely up to the system administrator,
-this is primarily intended to solve conflicts between devices.
- Access permissions are associated with character
-device special files, hence we must ensure device numbers cannot
-change with the module load order. To this end minor numbers are no
-longer automatically assigned by the "videodev" module as in V4L but
-requested by the driver. The defaults will suffice for most people
-unless two drivers compete for the same minor numbers.
-The module options to select minor numbers are named
-after the device special file with a "_nr" suffix. For example "video_nr"
-for /dev/video video capture devices. The number is
-an offset to the base minor number associated with the device type.
-
- In earlier versions of the V4L2 API the module options
-where named after the device special file with a "unit_" prefix, expressing
-the minor number itself, not an offset. Rationale for this change is unknown.
-Lastly the naming and semantics are just a convention among driver writers,
-the point to note is that minor numbers are not supposed to be hardcoded
-into drivers.
-When the driver supports multiple devices of the same
-type more than one minor number can be assigned, separated by commas:
-
+with major number 81 and a minor number between 0 and 255. Minor numbers
+are allocated dynamically unless the kernel is compiled with the kernel
+option CONFIG_VIDEO_FIXED_MINOR_RANGES. In that case minor numbers are
+allocated in ranges depending on the device node type (video, radio, 
etc.).
+
+  Many drivers support "video_nr", "radio_nr" or "vbi_nr"
+module options to select specific video/radio/vbi node numbers. This allows
+the user to request that the device node is named e.g. /dev/video5 instead
+of leaving it to chance. When the driver supports multiple devices of the same
+type more than one device node number can be assigned, separated by commas:
+   
  
-> insmod mydriver.o video_nr=0,1 radio_nr=0,1
+> modprobe mydriver video_nr=0,1 radio_nr=0,1

 
   In /etc/modules.conf this may be
 written as: 
  
-alias char-major-81-0 mydriver
-alias char-major-81-1 mydriver
-alias char-major-81-64 mydriver  
-options mydriver video_nr=0,1 radio_nr=0,1   
+options mydriver video_nr=0,1 radio_nr=0,1
  
- 
-   
- When an application attempts to open a device
-special file with major number 81 and minor number 0, 1, or 64, load
-"mydriver" (and the "videodev" module it depends upon).
-   
-   
- Register the first two video capture devices with
-minor number 0 and 1 (base number is 0), the first two radio device
-with minor number 64 and 65 (base 64).
-   
- 
-When no minor number is given as module
-option the driver supplies a default. 
-recommends the base minor numbers to be used for the various device
-types. Obviously minor numbers must be unique. When the number is
-already in use the offending device will not be
-registered. 
-
-  By convention system administrators create various
-character device special files with these major and minor numbers in
-the /dev directory. The names recommended for the
-different V4L2 device types are listed in .
+When no device node number is given as module
+option the driver supplies a default.
+
+  Normally udev will create the device nodes in /dev automatically
+for you. If udev is not installed, then you need to enable the
+CONFIG_VIDEO_FIXED_MINOR_RANGES kernel option in order to be able to correctly
+relate a minor number to a device node number

Re: [GIT PULL FOR v3.15] Updates for 3.15

2014-02-04 Thread Mauro Carvalho Chehab
Em Mon, 03 Feb 2014 11:35:20 +0100
Hans Verkuil  escreveu:

> Hi Mauro,
> 
> The usual list of updates. I also decided to add my DocBook changes to this
> pull request.
> 
> Regards,
> 
>   Hans
> 
> The following changes since commit 587d1b06e07b4a079453c74ba9edf17d21931049:
> 
>   [media] rc-core: reuse device numbers (2014-01-15 11:46:37 -0200)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/hverkuil/media_tree.git for-v3.15a
> 
> for you to fetch changes up to 5979412aac4c8342c4f7d12c642a2ae955b0c68f:
> 
>   DocBook media: add revision entry for 3.15. (2014-02-03 11:29:03 +0100)
> 
> 
> Hans Verkuil (11):
>   usbvision: drop unused define USBVISION_SAY_AND_WAIT
>   s3c-camif: Remove use of deprecated V4L2_CTRL_FLAG_DISABLED.
>   v4l2-dv-timings.h: add new 4K DMT resolutions.
>   v4l2-dv-timings: mention missing 'reduced blanking V2'
>   DocBook media: fix email addresses.
>   DocBook media: update copyright years and Introduction.
>   DocBook media: partial rewrite of "Opening and Closing Devices"
>   DocBook media: update four more sections
>   DocBook media: update three sections
>   DocBook media: drop the old incorrect packed RGB table.
>   DocBook media: add revision entry for 3.15.

Hmm... I didn't see this patch at the ML.

As I dropped the patch that changed the "Opening and Closing Devices" from
this series, I modified this one to reflect it.

> 
> Martin Bugge (4):
>   adv7842: adjust gain and offset for DVI-D signals
>   adv7842: pixelclock read-out
>   adv7842: log-status for Audio Video Info frames (AVI)
>   adv7842: platform-data for Hotplug Active (HPA) manual/auto
> 
> Sachin Kamat (1):
>   radio-keene: Use module_usb_driver
> 
> sensoray-dev (1):
>   s2255drv: checkpatch fix: coding style fix
> 
>  Documentation/DocBook/media/dvb/dvbapi.xml|   4 +-
>  Documentation/DocBook/media/v4l/common.xml| 413 
> +-
>  Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml | 513 
> +++
>  Documentation/DocBook/media/v4l/v4l2.xml  |  13 +-
>  Documentation/DocBook/media_api.tmpl  |  15 +-
>  drivers/media/i2c/adv7842.c   | 149 
> 
>  drivers/media/platform/s3c-camif/camif-capture.c  |  15 +-
>  drivers/media/radio/radio-keene.c |  19 +--
>  drivers/media/usb/s2255/s2255drv.c| 333 
> ---
>  drivers/media/usb/usbvision/usbvision.h   |   8 --
>  drivers/media/v4l2-core/v4l2-dv-timings.c |   4 +
>  include/media/adv7842.h   |   3 +
>  include/uapi/linux/v4l2-dv-timings.h  |  17 +++
>  13 files changed, 536 insertions(+), 970 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/6] DocBook media: partial rewrite of "Opening and Closing Devices"

2014-02-04 Thread Hans Verkuil
On 02/04/14 13:20, Mauro Carvalho Chehab wrote:
> Em Fri, 17 Jan 2014 10:24:11 +0100
> Hans Verkuil  escreveu:
> 
>>
>> I'll post the revised version of this patch next.
> 
> Weird, I'm not seeing the revised version of this patch posted.

https://patchwork.linuxtv.org/patch/21620/

> 
> Anyway, from your original patch:
> 
>> +  Today each device node supports just one function, with the
>> +exception of overlay support.
> 
> It is still mixing overlay with "function" (where "function" means
> VBI, radio, video).

Yes, I want to tackle that separately.

> 
> I'll drop this one from the series I'm applying. Please review and submit
> latter a new version of this one to the ML for easier review.

Please just take this as is. The 'function' terminology was there in the
original, so it is not something this patch has introduced. Yes, it should be
changed, but it's something I need to think about some more. This patch may
not solve everything, but at least it greatly improves it.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/6] DocBook media: partial rewrite of "Opening and Closing Devices"

2014-02-04 Thread Mauro Carvalho Chehab
Em Fri, 17 Jan 2014 10:24:11 +0100
Hans Verkuil  escreveu:

> Hi Mauro,
> 
> On 01/13/2014 06:23 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 13 Jan 2014 17:15:40 +0100
> > Hans Verkuil  escreveu:
> > 
> >> On 01/13/2014 04:20 PM, Mauro Carvalho Chehab wrote:
> >>> Em Tue,  7 Jan 2014 14:06:54 +0100
> >>> Hans Verkuil  escreveu:
> >>>
>  From: Hans Verkuil 
> 
>  This section was horribly out of date. A lot of references to old and
>  obsolete behavior have been dropped.
> > 
> > Forgot to mention, put patches 1 and 2 are ok. I'll review the patches 4-6
> > later this week.
> > 
> 
>  Signed-off-by: Hans Verkuil 
>  ---
>   Documentation/DocBook/media/v4l/common.xml | 188 
>  ++---
>   1 file changed, 67 insertions(+), 121 deletions(-)
> 
>  diff --git a/Documentation/DocBook/media/v4l/common.xml 
>  b/Documentation/DocBook/media/v4l/common.xml
>  index 1ddf354..da08df9 100644
>  --- a/Documentation/DocBook/media/v4l/common.xml
>  +++ b/Documentation/DocBook/media/v4l/common.xml
> 
> 
> 
>  +  Devices can support several functions. For example
>  +video capturing, VBI capturing and radio support.
> >>>
> >>> "function" seems to be a poor choice of word here. Ok, it comes from the
> >>> original text, but it is still not clear.
> >>>
> >>> I would use another word, like "broadcast type", in order to refer to
> >>> radio, software defined radio, VBI and video.
> >>
> >> I agree that it is not the best word, but neither is (IMHO) "broadcast 
> >> type".
> >> This would be something for a follow-up patch.
> > 
> > I think we should use the right word here on this fix. Other suggestions:
> > "stream type", "media type".
> > 
> > In any case, we should enumerate all those types here, maybe even putting
> > them into a table, in order to define precisely to what we're referring to.
> 
> I'm not going to do this now. I need to think about this some more, and this
> might also require changes in a lot of other places in the documentation.
> 
> So as far as I am concerned this is something for a future patch.
> 
> > 
>  +
>  +  The V4L2 API creates different nodes for each of these 
>  functions.
>  +One exception to this rule is the overlay function: this is shared
>  +with the video capture node (or video output node for output 
>  overlays).
> >>>
> >>> The mention to overlay here is completely out of context, and proofs
> >>> my point that "function" is a very bad choice: overlay is not a
> >>> broadcast type. It is just one of the ways to output the data. The same
> >>> device node can support multiple "delivery types":
> >>>   - overlay;
> >>>   - dma-buf;
> >>>   - mmap;
> >>>   - read or write.
> >>>
> >>> Let's not mix those two concepts in the new text.
> >>>
> >>> Also, the delivery type has nothing to do with "Opening and closing 
> >>> devices".
> >>
> >> I like the word "delivery type" in this context and I agree with you here.
> >> I'll see if I can improve this text.
> > 
> > Thanks!
> 
> I decided to just drop this paragraph. It doesn't belong here and it doesn't
> add anything useful.
> 
> >  
> >>>
>  +
>  +  The V4L2 API was designed with the idea that one device 
>  node could support
>  +all functions. However, in practice this never worked: this 'feature'
>  +was never used by applications and many drivers did not support it and 
>  if
>  +they did it was certainly never tested. In addition, switching a device
>  +node between different functions only works when using the streaming I/O
>  +API, not with the read()/write() API.
>  +
>  +  Today each device node supports just one function, with the
>  +exception of overlay support.
>   
> Besides video input or output the hardware may also
>   support audio sampling or playback. If so, these functions are
>  -implemented as OSS or ALSA PCM devices and eventually OSS or ALSA
>  -audio mixer. The V4L2 API makes no provisions yet to find these
>  -related devices. If you have an idea please write to the linux-media
>  -mailing list: &v4l-ml;.
>  +implemented as ALSA PCM devices with optional ALSA audio mixer
>  +devices.
>  +
>  +  One problem with all these devices is that the V4L2 API
>  +makes no provisions to find these related devices. Some really
>  +complex devices use the Media Controller (see   linkend="media_controller" />)
>  +which can be used for this purpose. But most drivers do not use it,
>  +and while some code exists that uses sysfs to discover related devices,
>  +there is no standard library yet. If you want to work on this please 
>  write
>  +to the linux-media mailing list: &v4l-ml;.
> >>>
> >>> Not true. It is there at v4l-utils. Ok, patches are always welcome.
> >>
> >> Well, sort of. That library only handles sysfs, not the mc.
> > 
> > Yes, but that c

RFC: how to set the colorspace for an output sub-device?

2014-02-04 Thread Hans Verkuil
Hi all,

We have the following situation: we use a ths8200 to send video out
over a VGA connector. It receives RGB video from a videoport DMA
engine. However, we want to be able to tell the ths8200 to convert
the RGB to YUV and send out YUV over the VGA connector instead of
RGB. Yeah, I know, it's odd, but it turns out that there are some
cases where that is needed (some cameras do that).

The question is, how do we tell the ths8200 to do that?

Currently in our repository we effectively implement it as another
output. So even though there is only one connector, the bridge driver
says that there are three, one RGB VGA and two YUV VGA using slightly
different YUV colorspaces. The bridge driver will call s_routing in
ths8200 and that sets up the colorspace conversion hardware.

There really is no way in the spec to explicitly define the colorspace
in a case like this. VIDIOC_S_FMT does has a colorspace field, but
that's for the video data in memory, and that remains RGB in this case.

In the subdev video ops you can also specify the colorspace of the
video going over a mediabus, but again that's RGB as well and we
don't have a way to specify this for the actual output.

I don't like the 'multiple outputs' idea. It's really a hack.

One option is to add a colorspace field to struct v4l2_bt_timings
(or to v4l2_dv_timings since it is not really specific to BT 656/1120).

If it is 0, then the default colorspace suitable for that connector
and format is used. If it is non-zero it can be used to specify which
colorspace the output should have (or, for receivers, which colorspace
the input has).

On the other hand, the colorspace doesn't really have anything to do
with the timings, so perhaps this isn't the best option.

Another alternative is to add RX and TX colorspace controls similar
to the RGB_RANGE controls that already exist today:
http://hverkuil.home.xs4all.nl/spec/media.html#dv-controls

E.g. V4L2_CID_TX_COLORSPACE

with options:

"Auto"
"SMPTE 170M"
"SMPTE 240M"
etc, etc. (same as colorspace defines)

One thing that is missing in the colorspaces are support for limited/full
range. During the Edinburgh summit we decided to add new colorspaces for
that. Adding this will make the TX/RX_RGB_RANGE controls obsolete since
those will be folded into the new COLORSPACE controls.

I think I like this approach better than the 'multiple outputs' or
the v4l2_dv_timings change.

Does anyone have other ideas or feedback?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Help wanted patching saa7164 driver to work with Compro E900F

2014-02-04 Thread Daniel Playfair Cal
Hello all,

I've been working on patching the saa7164 driver to work with the
Compro Videomate Vista E900F. This is a PCI-e tuner card that seems to
be almost identical to the Hauppauge HVR-2200, many versions of which
are supported. I've had some success but there are a few problems
which I'm not sure how to approach and I would appreciate any
help/advice. The most important point is number 2 under things that
are confusing.

CARD DETAILS:
 - Dual hybrid DVB-T/analogue TV tuner
 - hardware analogue source encoders
 - Various analogue video inputs
 - IR remote input
 - pci ID: 185b:e900
 - Main IC: phillips saa7164 rev2
 - Tuners: 2x NXP tda18271
 - DVB-T Demodulators: 2x NXP tda10048

WHAT I'VE DONE
My approach has been to modify the driver to make the kernel output
resemble the sample shown for the HVR-2200 here:
http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-HVR-2200#Sample_kernel_output
I patched saa7164-cards.c and saa7164.h to include the details of the
card (see https://gist.github.com/anonymous/8801027), and added the
card to the first case in saa7164_dvb_register() in saa7164-dvb.c (the
case covering the Hauppauge DVB-T cards). This results in the card
being recognised, the tuner/digital demodulator chips being detected,
and the DVB frontend devices being registered correctly. Kernel log of
this process is here: https://gist.github.com/anonymous/8799960. The
output of w_scan is here: https://gist.github.com/anonymous/881.
w_scan correctly finds frequencies on which there is a signal in my
area but seems to fail to get any data from those transmissions like
channel names etc. Kernel logs collected during that run of w_scan are
here: https://gist.github.com/anonymous/8800084. The behaviour is the
same on both adapters.

MISCELLANEOUS OBSERVATIONS/ACTIONS:
 - the card's firmware is different to any of the hauppauge firmwares.
The earlier two rev2 firmwares on Steven Toth's site boot correctly
and behave similarly to the stock firmware under linux but result in
the card failing to work at all under windows. The rev3 firmwares and
the newer/larger rev2 firmwares fail to boot. The firmware in the
windows driver has a different header structure to that in the
hauppauge firmware files, but I haven't bothered tackling that problem
yet since the firmware persists even through a full power cycle. I've
just been using the Compro firmware as flashed by the windows driver.
 - My best guess is that the two GPIO pins connected to the tda10048
demodulator reset pin are the same as on the HVR-2200. When I disabled
setting these pins on initialisation, the demodulators were not
correctly detected after a reboot, and strobing these pins as is done
for the HVR-2200 fixed the problem.

THINGS THAT ARE CONFUSING TO ME AND SEEM TO BE WRONG:
1. RF calibration for the tda18271 occurs only on the second of the
two tuners. When tda18271c2_rf_cal_init() is called from within
tda18271_attach(...) in tda18271-fe.c, cfg->rf_cal_on_startup is 0 for
the first chip and 1 for the second. I don't know why this is the
case, but I'm currently assuming its not a serious problem.
2. firmware is not loaded onto either of the tda10048's.
tda10048_init() seems to never be called. I don't understand how it is
meant to be called and why it isn't being called. Does the firmware on
this chip not persist through a reboot or reset, and would this
explain why w_scan is unable to see the channels on the frequencies?
What is the expected series of events that results in tda10048_init
being called to initialise the demodulator? At the moment I feel like
this is likely the biggest problem.
3. The firmware seems to object the the get firmware debug status
command sent from saa7164_api_set_debug() with a 0x9 status code
(SAA_ERR_BAD_PARAMETER), but not to the set debug status command, so I
don't see that this matters. I can see messages in the log starting
with FWMSG like what saa7164_api_collect_debug() would print, so it
seems that the firmware debugging is working in any case.
4. The output of saa7164_api_dump_subdevs() (see kernel log:
https://gist.github.com/anonymous/8799960) indicates that there is an
EEPROM with unitID 0x02. I have configured the card with this unitID
but the firmware returns 0x09 (SAA_ERR_BAD_PARAMETER) and the eeprom
is not read. I don't know why this is the case, and I am curious as to
why the particular read addresses are used in
saa7164_api_read_eeprom() in saa7164-api.c. ("u8 reg[] = { 0x0f, 0x00
};"). Might the correct address vary between cards? If so, how would I
got about finding the correct address? Am I correct in thinking that
the failure to read the EEPROM will not affect digital reception?
(btw, I also tried using all other unitIDs 0-0xff and none resulted in
a successful eeprom read)
5. Some of the configurations of hvr-2200 cards do not include analog
demodulators. I assume one must be present and configured in the card
struct to enable analog reception, and that the driver currently only
supports it 

Re: [PATCH 1/2] [media] v4l2: Add settings for Horizontal and Vertical MV Search Range

2014-02-04 Thread Hans Verkuil
Acked-by: Hans Verkuil 

Regards,

Hans

On 02/04/14 10:59, Amit Grover wrote:
> Adding V4L2 controls for horizontal and vertical search range in pixels
> for motion estimation module in video encoder.
> 
> Signed-off-by: Swami Nathan 
> Signed-off-by: Amit Grover 
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   20 
>  drivers/media/v4l2-core/v4l2-ctrls.c |6 ++
>  include/uapi/linux/v4l2-controls.h   |2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> b/Documentation/DocBook/media/v4l/controls.xml
> index a5a3188..0e1770c 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2258,6 +2258,26 @@ Applicable to the MPEG1, MPEG2, MPEG4 encoders.
>  VBV buffer control.
> 
>  
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE 
> + integer
> +   
> + Horizontal search range defines 
> maximum horizontal search area in pixels
> +to search and match for the present Macroblock (MB) in the reference 
> picture. This V4L2 control macro is used to set
> +horizontal search range for motion estimation module in video 
> encoder.
> +   
> +
> +  
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE 
> + integer
> +   
> + Vertical search range defines 
> maximum vertical search area in pixels
> +to search and match for the present Macroblock (MB) in the reference 
> picture. This V4L2 control macro is used to set
> +vertical search range for motion estimation module in video encoder.
> +   
> +
> 
> 
>spanname="id">V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 6ff002b..e9e12c4 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -735,6 +735,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_MPEG_VIDEO_DEC_PTS:   return "Video 
> Decoder PTS";
>   case V4L2_CID_MPEG_VIDEO_DEC_FRAME: return "Video 
> Decoder Frame Count";
>   case V4L2_CID_MPEG_VIDEO_VBV_DELAY: return "Initial 
> Delay for VBV Control";
> + case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: return 
> "Horizontal MV Search Range";
> + case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return 
> "Vertical MV Search Range";
>   case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat 
> Sequence Header";
>  
>   /* VPX controls */
> @@ -910,6 +912,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   *min = 0;
>   *max = *step = 1;
>   break;
> + case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
> + case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
> + *type = V4L2_CTRL_TYPE_INTEGER;
> + break;
>   case V4L2_CID_PAN_RESET:
>   case V4L2_CID_TILT_RESET:
>   case V4L2_CID_FLASH_STROBE:
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index 2cbe605..cda6fa0 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -376,6 +376,8 @@ enum v4l2_mpeg_video_multi_slice_mode {
>  #define V4L2_CID_MPEG_VIDEO_DEC_FRAME
> (V4L2_CID_MPEG_BASE+224)
>  #define V4L2_CID_MPEG_VIDEO_VBV_DELAY
> (V4L2_CID_MPEG_BASE+225)
>  #define V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER
> (V4L2_CID_MPEG_BASE+226)
> +#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE
> (V4L2_CID_MPEG_BASE+227)
> +#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE
> (V4L2_CID_MPEG_BASE+228)
>  
>  #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP  (V4L2_CID_MPEG_BASE+300)
>  #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP  (V4L2_CID_MPEG_BASE+301)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] s2255drv: port to videobuf2

2014-02-04 Thread Hans Verkuil
Hi Dean,

On 02/03/14 18:06, Dean Anderson wrote:
> On 2014-02-03 03:51, Hans Verkuil wrote:
>> Hi Dean,
>>
>> Some specific comments below, but first two general comments:
>>
>> It is easier to review if at least the removal of the old s2255_fh struct
>> was done as a separate patch. It's always good to try and keep the changes
>> in patches as small as possible. The actual vb2 conversion is always a
>> 'big bang' patch, that's unavoidable, but it's easier if it isn't mixed in
>> with other changes that are not directly related to the vb2 conversion.
> 
> 
> I figured removal of s2255_fh was a natural part of the videobuf2 conversion 
> process, but I can break it up.

It's more like the first phase of a vb2 conversion. It really is wrong
for videobuf as well, so it makes sense to do that first.

> I also did change some formatting and naming changes (s2255_channel to 
> s2255_vc) that can be postponed.

Just put it in a separate patch either before or after the patch that does
the vb2 conversion.

> 
>>
>> And did you also run the v4l2-compliance utility for this driver? That's
>> useful to check that everything it still correct.
> 
> Thanks for the comments.  I'll do a v2 soon with v4l2-compliance fully tested 
> too.

Rather than the standard v4l2-compliance from v4l-utils, can you use this
from my own tree:

http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/streaming

I've started work to add tests for streaming to v4l2-compliance. While not
complete it should cover what the s2255 driver needs. I'm very interested
in what it finds (or, as the case might be, what it doesn't find).

In order to do the streaming tests you have to run it with option -s.

Regards,

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] [media] v4l2: Add settings for Horizontal and Vertical MV Search Range

2014-02-04 Thread Amit Grover
Adding V4L2 controls for horizontal and vertical search range in pixels
for motion estimation module in video encoder.

Signed-off-by: Swami Nathan 
Signed-off-by: Amit Grover 
---
 Documentation/DocBook/media/v4l/controls.xml |   20 
 drivers/media/v4l2-core/v4l2-ctrls.c |6 ++
 include/uapi/linux/v4l2-controls.h   |2 ++
 3 files changed, 28 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index a5a3188..0e1770c 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2258,6 +2258,26 @@ Applicable to the MPEG1, MPEG2, MPEG4 encoders.
 VBV buffer control.
  
 
+ 
+ 
+   V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE 
+   integer
+ 
+   Horizontal search range defines 
maximum horizontal search area in pixels
+to search and match for the present Macroblock (MB) in the reference picture. 
This V4L2 control macro is used to set
+horizontal search range for motion estimation module in video encoder.
+ 
+
+
+ 
+   V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE 
+   integer
+ 
+   Vertical search range defines 
maximum vertical search area in pixels
+to search and match for the present Macroblock (MB) in the reference picture. 
This V4L2 control macro is used to set
+vertical search range for motion estimation module in video encoder.
+ 
+
  
  
V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE 
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 6ff002b..e9e12c4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -735,6 +735,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_DEC_PTS:   return "Video 
Decoder PTS";
case V4L2_CID_MPEG_VIDEO_DEC_FRAME: return "Video 
Decoder Frame Count";
case V4L2_CID_MPEG_VIDEO_VBV_DELAY: return "Initial 
Delay for VBV Control";
+   case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: return 
"Horizontal MV Search Range";
+   case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return 
"Vertical MV Search Range";
case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat 
Sequence Header";
 
/* VPX controls */
@@ -910,6 +912,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
v4l2_ctrl_type *type,
*min = 0;
*max = *step = 1;
break;
+   case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
+   case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
+   *type = V4L2_CTRL_TYPE_INTEGER;
+   break;
case V4L2_CID_PAN_RESET:
case V4L2_CID_TILT_RESET:
case V4L2_CID_FLASH_STROBE:
diff --git a/include/uapi/linux/v4l2-controls.h 
b/include/uapi/linux/v4l2-controls.h
index 2cbe605..cda6fa0 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -376,6 +376,8 @@ enum v4l2_mpeg_video_multi_slice_mode {
 #define V4L2_CID_MPEG_VIDEO_DEC_FRAME  (V4L2_CID_MPEG_BASE+224)
 #define V4L2_CID_MPEG_VIDEO_VBV_DELAY  (V4L2_CID_MPEG_BASE+225)
 #define V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER  (V4L2_CID_MPEG_BASE+226)
+#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE  (V4L2_CID_MPEG_BASE+227)
+#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE  (V4L2_CID_MPEG_BASE+228)
 
 #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP(V4L2_CID_MPEG_BASE+300)
 #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP(V4L2_CID_MPEG_BASE+301)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] drivers/media: Add controls for Horizontal and Vertical MV Search Range

2014-02-04 Thread Amit Grover
This is v3 version for the patch:
s5p-mfc: Add Horizontal and Vertical search range for Video Macro Blocks
(https://lkml.org/lkml/2013/12/30/83)

Changes from v1:
1) Splitted the patch into V4L2 and MFC driver patches.
2) Incorporated review comments on v1.

Changes from v2:
1) Removed the range restrictions from drivers/media/v4l2-core/v4l2-ctrls.c.
2) Rebased the patch to git://linuxtv.org/mchehab/media-next.git.
3) Added [media] tag in title.

Amit Grover (2):
  [media] v4l2: Add settings for Horizontal and Vertical MV Search
Range
  [media] s5p-mfc: Add Horizontal and Vertical MV Search Range

 Documentation/DocBook/media/v4l/controls.xml|   20 +++
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h|1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c|   24 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |8 ++--
 drivers/media/v4l2-core/v4l2-ctrls.c|6 ++
 include/uapi/linux/v4l2-controls.h  |2 ++
 7 files changed, 57 insertions(+), 6 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] [media] s5p-mfc: Add Horizontal and Vertical MV Search Range

2014-02-04 Thread Amit Grover
This patch adds Controls to set Horizontal and Vertical search range
for Motion Estimation block for Samsung MFC video Encoders.

Signed-off-by: Swami Nathan 
Signed-off-by: Amit Grover 
---
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h|1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c|   24 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |8 ++--
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h 
b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
index 2398cdf..8d0b686 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
@@ -229,6 +229,7 @@
 #define S5P_FIMV_E_PADDING_CTRL_V6 0xf7a4
 #define S5P_FIMV_E_MV_HOR_RANGE_V6 0xf7ac
 #define S5P_FIMV_E_MV_VER_RANGE_V6 0xf7b0
+#define S5P_FIMV_E_MV_RANGE_V6_MASK0x3fff
 
 #define S5P_FIMV_E_VBV_BUFFER_SIZE_V6  0xf84c
 #define S5P_FIMV_E_VBV_INIT_DELAY_V6   0xf850
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index f723f1f..5c28cc3 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -426,6 +426,8 @@ struct s5p_mfc_vp8_enc_params {
 struct s5p_mfc_enc_params {
u16 width;
u16 height;
+   u32 mv_h_range;
+   u32 mv_v_range;
 
u16 gop_size;
enum v4l2_mpeg_video_multi_slice_mode slice_mode;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 91b6e02..df83cd1 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -208,6 +208,24 @@ static struct mfc_control controls[] = {
.default_value = 0,
},
{
+   .id = V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE,
+   .type = V4L2_CTRL_TYPE_INTEGER,
+   .name = "Horizontal MV Search Range",
+   .minimum = 16,
+   .maximum = 128,
+   .step = 16,
+   .default_value = 32,
+   },
+   {
+   .id = V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE,
+   .type = V4L2_CTRL_TYPE_INTEGER,
+   .name = "Vertical MV Search Range",
+   .minimum = 16,
+   .maximum = 128,
+   .step = 16,
+   .default_value = 32,
+   },
+   {
.id = V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE,
.type = V4L2_CTRL_TYPE_INTEGER,
.minimum = 0,
@@ -1417,6 +1435,12 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MPEG_VIDEO_VBV_SIZE:
p->vbv_size = ctrl->val;
break;
+   case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
+   p->mv_h_range = ctrl->val;
+   break;
+   case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
+   p->mv_v_range = ctrl->val;
+   break;
case V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE:
p->codec.h264.cpb_size = ctrl->val;
break;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index f6ff2db..f64621a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -727,14 +727,10 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
WRITEL(reg, S5P_FIMV_E_RC_CONFIG_V6);
 
/* setting for MV range [16, 256] */
-   reg = 0;
-   reg &= ~(0x3FFF);
-   reg = 256;
+   reg = (p->mv_h_range & S5P_FIMV_E_MV_RANGE_V6_MASK);
WRITEL(reg, S5P_FIMV_E_MV_HOR_RANGE_V6);
 
-   reg = 0;
-   reg &= ~(0x3FFF);
-   reg = 256;
+   reg = (p->mv_v_range & S5P_FIMV_E_MV_RANGE_V6_MASK);
WRITEL(reg, S5P_FIMV_E_MV_VER_RANGE_V6);
 
WRITEL(0x0, S5P_FIMV_E_FRAME_INSERTION_V6);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[linuxtv-media:master 499/499] drivers/media/usb/usbtv/usbtv-core.c:119:22: sparse: symbol 'usbtv_id_table' was not declared. Should it be static?

2014-02-04 Thread kbuild test robot
tree:   git://linuxtv.org/media_tree.git master
head:   a3550ea665acd1922df8275379028c1634675629
commit: a3550ea665acd1922df8275379028c1634675629 [499/499] [media] usbtv: split 
core and video implementation
reproduce: make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/media/usb/usbtv/usbtv-core.c:119:22: sparse: symbol 'usbtv_id_table' 
>> was not declared. Should it be static?
>> drivers/media/usb/usbtv/usbtv-core.c:129:19: sparse: symbol 
>> 'usbtv_usb_driver' was not declared. Should it be static?
--
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:285:14: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:287:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:288:15: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:289:20: sparse: cast to restricted 
>> __be32
>> drivers/media/usb/usbtv/usbtv-video.c:565:23: sparse: symbol 
>> 'usbtv_ioctl_ops' was not declared. Should it be static?
>> drivers/media/usb/usbtv/usbtv-video.c:587:29: sparse: symbol 'usbtv_fops' 
>> was not declared. Should it be static?
>> drivers/media/usb/usbtv/usbtv-video.c:648:16: sparse: symbol 'usbtv_vb2_ops' 
>> was not declared. Should it be static?

Please consider folding the attached diff :-)

---
0-DAY kernel build testing backend  Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
From: Fengguang Wu 
Subject: [PATCH linuxtv-media] usbtv: usbtv_id_table[] can be static
TO: Federico Simoncelli 
CC: Mauro Carvalho Chehab 
CC: linux-media@vger.kernel.org
CC: linux-media@vger.kernel.org 
CC: linux-ker...@vger.kernel.org 

CC: Federico Simoncelli 
CC: Mauro Carvalho Chehab 
CC: linux-media@vger.kernel.org
Signed-off-by: Fengguang Wu 
---
 usbtv-core.c  |4 ++--
 usbtv-video.c |6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/usbtv/usbtv-core.c b/drivers/media/usb/usbtv/usbtv-core.c
index e89e48b..2dd47e0 100644
--- a/drivers/media/usb/usbtv/usbtv-core.c
+++ b/drivers/media/usb/usbtv/usbtv-core.c
@@ -116,7 +116,7 @@ static void usbtv_disconnect(struct usb_interface *intf)
 	v4l2_device_put(&usbtv->v4l2_dev);
 }
 
-struct usb_device_id usbtv_id_table[] = {
+static struct usb_device_id usbtv_id_table[] = {
 	{ USB_DEVICE(0x1b71, 0x3002) },
 	{}
 };
@@ -126,7 +126,7 @@ MODULE_AUTHOR("Lubomir Rintel");
 MODULE_DESCRIPTION("Fushicai USBTV007 Video Grabber Driver");
 MODULE_LICENSE("Dual BSD/GPL");
 
-struct usb_driver usbtv_usb_driver = {
+static struct usb_driver usbtv_usb_driver = {
 	.name = "usbtv",
 	.id_table = usbtv_id_table,
 	.probe = usbtv_probe,
diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index 496bc2e..029ea7c 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -562,7 +562,7 @@ static int usbtv_s_input(struct file *file, void *priv, unsigned int i)
 	return usbtv_select_input(usbtv, i);
 }
 
-struct v4l2_ioctl_ops usbtv_ioctl_ops = {
+static struct v4l2_ioctl_ops usbtv_ioctl_ops = {
 	.vidioc_querycap = usbtv_querycap,
 	.vidioc_enum_input = usbtv_enum_input,
 	.vidioc_enum_fmt_vid_cap = usbtv_e

[PATCH] usbtv: fix compiler error due to missing module.h

2014-02-04 Thread Hans Verkuil
usbtv-video.c needs module.h. So move the module.h include from usbtv-core.c to 
usbtv.h,
that way both core.c and video.c have it.

Signed-off-by: Hans Verkuil 

diff --git a/drivers/media/usb/usbtv/usbtv-core.c 
b/drivers/media/usb/usbtv/usbtv-core.c
index e89e48b..d543928 100644
--- a/drivers/media/usb/usbtv/usbtv-core.c
+++ b/drivers/media/usb/usbtv/usbtv-core.c
@@ -28,8 +28,6 @@
  * GNU General Public License ("GPL").
  */
 
-#include 
-
 #include "usbtv.h"
 
 int usbtv_set_regs(struct usbtv *usbtv, const u16 regs[][2], int size)
diff --git a/drivers/media/usb/usbtv/usbtv.h b/drivers/media/usb/usbtv/usbtv.h
index 536343d..cb1d388 100644
--- a/drivers/media/usb/usbtv/usbtv.h
+++ b/drivers/media/usb/usbtv/usbtv.h
@@ -19,6 +19,7 @@
  * GNU General Public License ("GPL").
  */
 
+#include 
 #include 
 #include 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFCv1 PATCH 7/9] vb2: add reinit_streaming op.

2014-02-04 Thread Hans Verkuil
On 01/30/2014 03:51 PM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This new op is called after stop_streaming() or after a failed call to
> start_streaming(). The driver needs to dequeue any pending active buffers
> it got from the buf_queue() callback.
> 
> The reason this op was added is that stop_streaming() traditionally dequeued
> any pending active buffers after stopping the DMA engine. However,
> stop_streaming() is never called if start_streaming() fails, even though any
> prequeued buffers have been passed on to the driver. In that case those
> pending active buffers may still be in the driver's active buffer list,
> which can cause all sorts of problems if they are not removed.
> 
> By splitting stop_streaming into stop_streaming (i.e. stop the DMA engine)
> and reinit_streaming (i.e. reinitialize the buffer lists) this problem is
> solved. After calling reinit_streaming() the vb2 core will also call
> vb2_buffer_done() for any remaining active buffers.

No need to review patches 7+8: this is going to change. I had a useful 
discussion
with Pawel regarding this and we came up with a better solution.

Regards,

Hans

> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 13 +++--
>  include/media/videobuf2-core.h   |  9 +++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index a3b4b4c..3030ef6 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -395,9 +395,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
> int buffers)
>   if (unbalanced || debug) {
>   pr_info("vb2: counters for queue %p:%s\n", q,
>   unbalanced ? " UNBALANCED!" : "");
> - pr_info("vb2: setup: %u start_streaming: %u 
> stop_streaming: %u\n",
> + pr_info("vb2: setup: %u start_streaming: %u 
> stop_streaming: %u reinit_streaming: %u\n",
>   q->cnt_queue_setup, q->cnt_start_streaming,
> - q->cnt_stop_streaming);
> + q->cnt_stop_streaming, q->cnt_reinit_streaming);
>   pr_info("vb2: wait_prepare: %u wait_finish: %u\n",
>   q->cnt_wait_prepare, q->cnt_wait_finish);
>   }
> @@ -406,6 +406,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
> int buffers)
>   q->cnt_wait_finish = 0;
>   q->cnt_start_streaming = 0;
>   q->cnt_stop_streaming = 0;
> + q->cnt_reinit_streaming = 0;
>   }
>   for (buffer = 0; buffer < q->num_buffers; ++buffer) {
>   struct vb2_buffer *vb = q->bufs[buffer];
> @@ -1900,7 +1901,15 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>*/
>   if (q->streaming)
>   call_qop(q, stop_streaming, q);
> +
>   q->streaming = 0;
> + if (atomic_read(&q->queued_count)) {
> + call_qop(q, reinit_streaming, q);
> +
> + for (i = 0; i < q->num_buffers; ++i)
> + if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
> + vb2_buffer_done(q->bufs[i], 
> VB2_BUF_STATE_ERROR);
> + }
>  
>   /*
>* Remove all buffers from videobuf's list...
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 82b7f0f..b40dfbc 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -294,8 +294,11 @@ struct vb2_buffer {
>   *   buffer is queued.
>   * @stop_streaming:  called when 'streaming' state must be disabled; driver
>   *   should stop any DMA transactions or wait until they
> - *   finish and give back all buffers it got from buf_queue()
> - *   callback; may use vb2_wait_for_all_buffers() function
> + *   finish; may use vb2_wait_for_all_buffers() function.
> + * @reinit_streaming:called after stop_streaming() or after a failed 
> call to
> + *   start_streaming(). The driver needs to dequeue any
> + *   pending active buffers it got from the buf_queue()
> + *   callback.
>   * @buf_queue:   passes buffer vb to the driver; driver may start
>   *   hardware operation on this buffer; driver should give
>   *   the buffer back by calling vb2_buffer_done() function;
> @@ -318,6 +321,7 @@ struct vb2_ops {
>  
>   int (*start_streaming)(struct vb2_queue *q, unsigned int count);
>   int (*stop_streaming)(struct vb2_queue *q);
> + void (*reinit_streaming)(struct vb2_queue *q);
>  
>   void (*buf_queue)(struct vb2_buffer *vb);
>  };
> @@ -408,6 +412,7 @@ struct vb2_queue {
>   u32  

Re: [RFCv1 PATCH 9/9] v4l2-ioctl: check CREATE_BUFS format via TRY_FMT.

2014-02-04 Thread Hans Verkuil
On 01/30/2014 03:51 PM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The format passed to VIDIOC_CREATE_BUFS is completely unchecked at
> the moment. So pass it to VIDIOC_TRY_FMT first.

Don't bother reviewing this. I'm going to change this anyway.

Regards,

Hans

> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 707aef7..7b9d59e 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1443,9 +1443,15 @@ static int v4l_dqbuf(const struct v4l2_ioctl_ops *ops,
>  static int v4l_create_bufs(const struct v4l2_ioctl_ops *ops,
>   struct file *file, void *fh, void *arg)
>  {
> + struct video_device *vfd = video_devdata(file);
>   struct v4l2_create_buffers *create = arg;
>   int ret = check_fmt(file, create->format.type);
>  
> + if (ret)
> + return ret;
> +
> + if (!WARN_ON(!is_valid_ioctl(vfd, VIDIOC_TRY_FMT)))
> + ret = v4l_try_fmt(ops, file, fh, &create->format);
>   return ret ? ret : ops->vidioc_create_bufs(file, fh, create);
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] usbtv: add audio support

2014-02-04 Thread Mauro Carvalho Chehab
Em Tue,  7 Jan 2014 23:13:22 +0100
Federico Simoncelli  escreveu:

> From: Federico Simoncelli 
> 
> Signed-off-by: Federico Simoncelli 
> Tested-by: Lubomir Rintel 
> ---
>  drivers/media/usb/usbtv/Makefile  |   3 +-
>  drivers/media/usb/usbtv/usbtv-audio.c | 384 
> ++
>  drivers/media/usb/usbtv/usbtv-core.c  |  16 +-
>  drivers/media/usb/usbtv/usbtv-video.c |   9 +-
>  drivers/media/usb/usbtv/usbtv.h   |  21 +-
>  5 files changed, 423 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/media/usb/usbtv/usbtv-audio.c
> 
> diff --git a/drivers/media/usb/usbtv/Makefile 
> b/drivers/media/usb/usbtv/Makefile
> index 775316a..f555cf8 100644
> --- a/drivers/media/usb/usbtv/Makefile
> +++ b/drivers/media/usb/usbtv/Makefile
> @@ -1,4 +1,5 @@
>  usbtv-y := usbtv-core.o \
> - usbtv-video.o
> + usbtv-video.o \
> + usbtv-audio.o
>  
>  obj-$(CONFIG_VIDEO_USBTV) += usbtv.o
> diff --git a/drivers/media/usb/usbtv/usbtv-audio.c 
> b/drivers/media/usb/usbtv/usbtv-audio.c
> new file mode 100644
> index 000..3acc52c
> --- /dev/null
> +++ b/drivers/media/usb/usbtv/usbtv-audio.c
> @@ -0,0 +1,384 @@
> +/*
> + * Fushicai USBTV007 Audio-Video Grabber Driver
> + *
> + * Product web site:
> + * 
> http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
> + *
> + * Copyright (c) 2013 Federico Simoncelli
> + * All rights reserved.
> + * No physical hardware was harmed running Windows during the
> + * reverse-engineering activity
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *notice, this list of conditions, and the following disclaimer,
> + *without modification.
> + * 2. The name of the author may not be used to endorse or promote products
> + *derived from this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL").
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "usbtv.h"
> +
> +static struct snd_pcm_hardware snd_usbtv_digital_hw = {
> + .info = SNDRV_PCM_INFO_BATCH |
> + SNDRV_PCM_INFO_MMAP |
> + SNDRV_PCM_INFO_INTERLEAVED |
> + SNDRV_PCM_INFO_BLOCK_TRANSFER |
> + SNDRV_PCM_INFO_MMAP_VALID,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_KNOT,

No, the above is wrong. It should be, instead:

.rates = SNDRV_PCM_RATE_48000,

> + .rate_min = 48000,
> + .rate_max = 48000,
> + .channels_min = 2,
> + .channels_max = 2,

> + .period_bytes_min = 64,
> + .period_bytes_max = 12544,

The above is likely wrong too, as it seems that you're using a fixed
number of URBs and a fixed URB size.

An invalid period size can cause bad audio artifacts. Basically, you need
to estimate/check how many URBs you're receiving per second, and what's
their size, in order to fill these.

I did such review on one of the drivers, at:

http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/em28xx

In particular, I suggest you to take a look on those patches:

http://git.linuxtv.org/mchehab/experimental.git/commitdiff/1b3fd2d342667005855deae74200195695433259

http://git.linuxtv.org/mchehab/experimental.git/commitdiff/49677aef90de7834e7bb4b0adf95c3342c2c8668

http://git.linuxtv.org/mchehab/experimental.git/commitdiff/a02b9c238b408f69fc78d528b549b85001df98b8

As it provides a way to dynamically fill it in runtime, showing the
calculus to estimate those values.

> + .periods_min = 2,
> + .periods_max = 98,
> + .buffer_bytes_max = 62720 * 8, /* value in usbaudio.c */
> +};
> +
> +static int snd_usbtv_pcm_open(struct snd_pcm_substream *substream)
> +{
> + struct usbtv *chip = snd_pcm_substream_chip(substream);
> + struct snd_pcm_runtime *runtime = substream->runtime;
> +
> + chip->snd_substream = substream;
> + runtime->hw = snd_usbtv_digital_hw;
> +
> + return 0;
> +}
> +
> +static int snd_usbtv_pcm_close(struct snd_pcm_substream *substream)
> +{
> + struct usbtv *chip = snd_pcm_substream_chip(substream);
> +
> + if (atomic_read(&chip->snd_stream)) {
> + atomic_set(&chip->snd_stream, 0);
> + schedule_work(&chip->snd_trigger);
> + }
> +
> + return 0;
> +}
> +
> +static int snd_usbtv_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *hw_params)
> +{
> + int rv;
> + struct usbtv *chip = snd_pcm_substream_chip(substream);
> +
> + rv = snd_pcm_lib_malloc_pages(substream,
> + params_buffer_bytes(hw_params));
> +
> + if (rv < 0) {
> + dev_warn(chip->dev, "pcm audio buffer allocation failure %