Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver

2015-11-27 Thread Daniel Thompson

Hi Tiffany/Andrew

This review is a rather more superficial than my previous one. Mostly 
I'm just commenting on some of the bits I spotted whilst trying to find 
my way around the patchset.


I hope to another more detailed review for v2 (and feel free to add me 
to Cc:).



On 17/11/15 12:54, Tiffany Lin wrote:
> Signed-off-by: Tiffany Lin 

Signed-off-by: Andrew-CT Chen 


There is no description of what this patch does. Its not enough to have 
it on the cover letter (because that won't end up in version control). 
You need something here.




diff --git a/drivers/media/platform/mtk-vcodec/Kconfig 
b/drivers/media/platform/mtk-vcodec/Kconfig
new file mode 100644
index 000..1c0b935
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/Kconfig
@@ -0,0 +1,5 @@
+config MEDIATEK_VPU
+   bool
+   ---help---
+ This driver provides downloading firmware vpu (video processor unit)
+ and communicating with vpu.


Haven't I seen this before (in patch 3)? Why is it being added to 
another Kconfig file?




diff --git a/drivers/media/platform/mtk-vcodec/Makefile 
b/drivers/media/platform/mtk-vcodec/Makefile
new file mode 100644
index 000..c7f7174
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/Makefile
@@ -0,0 +1,12 @@
+obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk_vcodec_intr.o \
+  mtk_vcodec_util.o \
+  mtk_vcodec_enc_drv.o \
+  mtk_vcodec_enc.o \
+  mtk_vcodec_enc_pm.o
+
+obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/
+
+ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
+-I$(srctree)/drivers/media/platform/mtk-vcodec \
+-I$(srctree)/drivers/media/platform/mtk-vpu


Seems like there's a lot of directories here. Are these files 
(framework, common, vcodec, etc) so unrelated they really need to live 
in separate directories?


Why not just drivers/media/platform/mediatek?



diff --git a/drivers/media/platform/mtk-vcodec/common/Makefile 
b/drivers/media/platform/mtk-vcodec/common/Makefile
new file mode 100644
index 000..477ab80
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/common/Makefile
@@ -0,0 +1,8 @@
+obj-y += \
+venc_drv_if.o
+
+ccflags-y += \
+-I$(srctree)/include/ \
+-I$(srctree)/drivers/media/platform/mtk-vcodec \
+-I$(srctree)/drivers/media/platform/mtk-vcodec/include \
+-I$(srctree)/drivers/media/platform/mtk-vpu


As above, this appears to be a directory to hold just one file.


> diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c 
b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c

> new file mode 100644
> index 000..9b3f025
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> @@ -0,0 +1,152 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Daniel Hsiao 
> + * Jungchang Tsao 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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 
> +
> +#include "mtk_vcodec_drv.h"
> +#include "mtk_vcodec_enc.h"
> +#include "mtk_vcodec_pm.h"
> +#include "mtk_vcodec_util.h"
> +#include "mtk_vpu_core.h"
> +
> +#include "venc_drv_if.h"
> +#include "venc_drv_base.h"
> +
> +
> +int venc_if_create(void *ctx, unsigned int fourcc, unsigned long 
*handle)

> +{
> +  struct venc_handle *h;
> +  char str[10];
> +
> +  mtk_vcodec_fmt2str(fourcc, str);
> +
> +  h = kzalloc(sizeof(*h), GFP_KERNEL);
> +  if (!h)
> +  return -ENOMEM;
> +
> +  h->fourcc = fourcc;
> +  h->ctx = ctx;
> +  mtk_vcodec_debug(h, "fmt = %s handle = %p", str, h);
> +
> +  switch (fourcc) {
> +  default:
> +  mtk_vcodec_err(h, "invalid format %s", str);
> +  goto err_out;
> +  }
> +
> +  *handle = (unsigned long)h;
> +  return 0;
> +
> +err_out:
> +  kfree(h);
> +  return -EINVAL;
> +}
> +
> +int venc_if_init(unsigned long handle)
> +{
> +  int ret = 0;
> +  struct venc_handle *h = (struct venc_handle *)handle;
> +
> +  mtk_vcodec_debug_enter(h);
> +
> +  mtk_venc_lock(h->ctx);
> +  mtk_vcodec_enc_clock_on();
> +  vpu_enable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> +  ret = h->enc_if->init(h->ctx, (unsigned long *)>drv_handle);
> +  vpu_disable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> +  mtk_vcodec_enc_clock_off();
> +  mtk_venc_unlock(h->ctx);
> +
> +  return ret;
> +}

To me this looks more like an obfuscation layer rather 

Re: ->poll() instances shouldn't be indefinitely blocking

2015-11-27 Thread Linus Torvalds
On Fri, Nov 27, 2015 at 7:18 AM, Mauro Carvalho Chehab
 wrote:
> Al Viro  escreveu:
>
>> Take a look at this:
>> static unsigned int gsc_m2m_poll(struct file *file,
>> struct poll_table_struct *wait)
>> {
>> struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
>> struct gsc_dev *gsc = ctx->gsc_dev;
>> int ret;
>>
>> if (mutex_lock_interruptible(>lock))
>> return -ERESTARTSYS;
>>
>> ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
>> mutex_unlock(>lock);
>>
>> return ret;
>> }
>>
>> a) ->poll() should not return -E...; callers expect just a bitmap of
>> POLL... values.
>
> Yeah. We fixed issues like that on other drivers along the time. I guess
> this is a some bad code that people just cut-and-paste from legacy drivers
> without looking into it.

Actually, while returning -ERESTARTSYS is bogus, returning _zero_
would not be. The top-level poll() code will happily notice the
signal, and return -EINTR like poll should (unless something else is
pending, in which case it will return zero and the bits set for that
something else).

So having a driver with a ->poll() function that does that kind of
conditional locking is not wrong per se. It's just he return value
that is crap.

I also do wonder if we might not make the generic code a bit more
robust wrt things like this. The bitmask we use is only about the low
bits, so we *could* certainly allow the driver poll() functions to
return errors - possibly just ignoring them. Or perhaps have a
WARN_ON_OCNE() to find them.

Al, what do you think? The whole "generic code should be robust wrt
drivers making silly mistakes" just sounds like a good idea. Finding
these things through code inspection is all well and good, but having
a nice warning report from users might be even better.

Linus
--
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: Mt9v024 gettting image problem

2015-11-27 Thread Guennadi Liakhovetski
Hi,

The reason for a lower framerate must not necessarily be in the program, 
it can be the hardware - either the clock frequency, your camera is driven 
by, or the slow CPU, or the camera host driver, or your frame processing.

Thanks
Guennadi

On Thu, 26 Nov 2015, Ayhan KÃ~\Ã~GÃ~\KMANÄ°SA wrote:

> Hi,
> 
> 
> I'm trying to get image from aptina mt9v024 sensor using v4l2 library. In
> sensor datasheet that is defined that sensor has 60 fps capability at
> default. But with my code which is below i could get images at 20 fps. How
> can i reach 60 fps? Could you give me an advice that where is my fault?
> 
> Thanks.
> 
> 
>  CODE 
> 
> 
> char *dev_name = "/dev/video0";
> int fd = v4l2_open(dev_name, O_RDWR | O_NONBLOCK, 0);
> if (fd < 0) {
> perror("Cannot open device");
> exit(EXIT_FAILURE);
> }
> cout << "camera init" << endl;
> CLEAR(fmt);
> fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> fmt.fmt.pix.width   = 752;
> fmt.fmt.pix.height  = 480;
> fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_SBGGR16;
> fmt.fmt.pix.field   = V4L2_FIELD_ANY;
> xioctl(fd, VIDIOC_S_FMT, );
> 
> 
> CLEAR(req);
> req.count = 2;
> req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> req.memory = V4L2_MEMORY_MMAP;
> xioctl(fd, VIDIOC_REQBUFS, );
> 
> buffers = (buffer *)calloc(req.count, sizeof(*buffers));
> for (n_buffers = 0; n_buffers < req.count; ++n_buffers) {
> CLEAR(buf);
> 
> buf.type= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> buf.memory  = V4L2_MEMORY_MMAP;
> buf.index   = n_buffers;
> 
> xioctl(fd, VIDIOC_QUERYBUF, );
> 
> buffers[n_buffers].length = buf.length;
> buffers[n_buffers].start = v4l2_mmap(NULL, buf.length,
>   PROT_READ | PROT_WRITE, MAP_SHARED,
>   fd, buf.m.offset);
> 
> if (MAP_FAILED == buffers[n_buffers].start) {
> perror("mmap");
> exit(EXIT_FAILURE);
> }
> }
> 
> for (i = 0; i < n_buffers; ++i) {
> CLEAR(buf);
> buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> buf.memory = V4L2_MEMORY_MMAP;
> buf.index = i;
> xioctl(fd, VIDIOC_QBUF, );
> }
> type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> xioctl(fd, VIDIOC_STREAMON, );
> while(1)
> {
> do {
> FD_ZERO();
> FD_SET(fd, );
> 
> /* Timeout. */
> tv.tv_sec = 2;
> tv.tv_usec = 0;
> 
> r = select(fd + 1, , NULL, NULL, );
> } while ((r == -1 && (errno = EINTR)));
> if (r == -1) {
> perror("select");
> return errno;
> }
> 
> CLEAR(buf);
> buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> buf.memory = V4L2_MEMORY_MMAP;
> xioctl(fd, VIDIOC_DQBUF, );
> /* Getting Image */
> Mat bayer16Bit = Mat(fmt.fmt.pix.height, fmt.fmt.pix.width, CV_8UC1, (void
> *)buffers[buf.index].start);
> }
> 
> 
> 
> 
> ---
> Arş. Gör. Ayhan KÜÇÜKMANİSA
> Kocaeli Üniversitesi, GömÃŒlÃŒ Sistemler ve GörÃŒntÃŒleme Sistemleri
> Laboratuvarı
> 
> Res. Asst. Ayhan KÜÇÜKMANİSA
> Kocaeli University, Laboratory of Embedded and Vision Systems
> 
--
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: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU

2015-11-27 Thread andrew-ct chen
Thanks a lot for your review comments.

On Wed, 2015-11-25 at 16:11 +, Daniel Thompson wrote:
> On 17/11/15 12:54, Tiffany Lin wrote:
> > From: Andrew-CT Chen 
> >
> > The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs.
> > It is able to handle video decoding/encoding of in a range of formats.
> > The driver provides with VPU firmware download, memory management and
> > the communication interface between CPU and VPU.
> > For VPU initialization, it will create virtual memory for CPU access and
> > IOMMU address for vcodec hw device access. When a decode/encode instance
> > opens a device node, vpu driver will download vpu firmware to the device.
> > A decode/encode instant will decode/encode a frame using VPU
> > interface to interrupt vpu to handle decoding/encoding jobs.
> >
> > Signed-off-by: Andrew-CT Chen 
> > ---
> >   drivers/media/platform/Kconfig |6 +
> >   drivers/media/platform/Makefile|2 +
> >   drivers/media/platform/mtk-vpu/Makefile|1 +
> >   .../platform/mtk-vpu/h264_enc/venc_h264_vpu.h  |  127 +++
> >   .../media/platform/mtk-vpu/include/venc_ipi_msg.h  |  212 +
> >   drivers/media/platform/mtk-vpu/mtk_vpu_core.c  |  823 
> > 
> >   drivers/media/platform/mtk-vpu/mtk_vpu_core.h  |  161 
> >   .../media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h  |  119 +++
> >   8 files changed, 1451 insertions(+)
> >   create mode 100644 drivers/media/platform/mtk-vpu/Makefile
> >   create mode 100644 drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> >   create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index ccbc974..f98eb47 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -148,6 +148,12 @@ config VIDEO_CODA
> >Coda is a range of video codec IPs that supports
> >H.264, MPEG-4, and other video formats.
> >
> > +config MEDIATEK_VPU
> > +   bool "Mediatek Video Processor Unit"
> > +   ---help---
> > +   This driver provides downloading firmware vpu and
> > +   communicating with vpu.
> > +
> 
> This looks pretty broken.
> 
> Shouldn't this option be tristate? Why are there no depends-on or selects?
> 

Yes, this should be tristate and depends on VIDEO_DEV && VIDEO_V4L2 &&
ARCH_MEDIATEK

> Also I think the help text could be more descriptive here (and so does 
> checkpatch ;-) ).
> 

I'll put more descriptive. Thanks.

> 
> > diff --git a/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h 
> > b/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> > new file mode 100644
> > index 000..9c8ebdd
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> 
> Why is this file included in *this* patch? It is not included by 
> anything in the patch and defines functions that do not exist yet.

I'll move this to the h264 patch.Thanks

> 
> 
> > diff --git a/drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h 
> > b/drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h
> > new file mode 100644
> > index 000..a345b98
> 
> This file also is not included by anything and should, perhaps be 
> included in a different patch.
> 

I'll move this to the v4l2 encoder patch.

> 
> > diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu_core.c 
> > b/drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> > new file mode 100644
> > index 000..b524dbc
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> > @@ -0,0 +1,823 @@
> > +/*
> > +* Copyright (c) 2015 MediaTek Inc.
> > +* Author: Andrew-CT Chen 
> > +*
> > +* This program is free software; you can redistribute it and/or modify
> > +* it under the terms of the GNU General Public License version 2 as
> > +* published by the Free Software Foundation.
> > +*
> > +* 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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "mtk_vpu_core.h"
> > +
> > +/**
> > + * VPU (video processor unit) is a tiny processor controlling video 
> > hardware
> > + * related to video codec, scaling and color format converting.
> > + * VPU interfaces with other blocks by share memory and interrupt.
> > + **/
> > +#define MTK_VPU_DRV_NAME   

Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU

2015-11-27 Thread Daniel Thompson

On 27/11/15 12:10, andrew-ct chen wrote:

+
> >+  memcpy((void *)send_obj->share_buf, buf, len);
> >+  send_obj->len = len;
> >+  send_obj->id = id;
> >+  vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU);
> >+
> >+  /* Wait until VPU receives the command */
> >+  timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS);
> >+  do {
> >+  if (time_after(jiffies, timeout)) {
> >+  dev_err(vpu->dev, "vpu_ipi_send: IPI timeout!\n");
> >+  return -EIO;
> >+  }
> >+  } while (vpu_cfg_readl(vpu, HOST_TO_VPU));

>
>Do we need to busy wait every time we communicate with the co-processor?
>Couldn't we put this wait*before*  we write to HOST_TO_VPU above.
>
>That way we only spin when there is a need to.
>

Since the hardware VPU only allows that one client sends the command to
it each time.
We need the wait to make sure VPU accepted the command and cleared the
interrupt and then the next command would be served.


I understand that the VPU  can only have on message outstanding at once.

I just wonder why we busy wait *after* sending the first command rather 
than *before* sending the second one.


Streamed decode/encode typically ends up being rate controlled by 
capture or display meaning that in these cases we don't need to busy 
wait at all (because by the time we send the next frame the VPU has 
already accepted the previous message).



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


[GIT PULL FOR v4.5] Three patches

2015-11-27 Thread Hans Verkuil
The v4l2-dv-timings patch is identical to what I sent before, but it merges the
pci-skeleton patch and fixes the docbook warning (was a typo).

Regards,

Hans

The following changes since commit 10897dacea26943dd80bd6629117f4620fc320ef:

  Merge tag 'v4.4-rc2' into patchwork (2015-11-23 14:16:58 -0200)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.5b

for you to fetch changes up to 737e89f00682c11d25ae987e0776a13db645fcec:

  cx231xx: constify cx2341x_handler_ops structures (2015-11-27 14:44:31 +0100)


Hans Verkuil (1):
  v4l2-dv-timings: add new arg to v4l2_match_dv_timings

Julia Lawall (2):
  media, sound: tea575x: constify snd_tea575x_ops structures
  cx231xx: constify cx2341x_handler_ops structures

 Documentation/video4linux/v4l2-pci-skeleton.c | 2 +-
 drivers/media/i2c/adv7604.c   | 6 +++---
 drivers/media/i2c/adv7842.c   | 6 +++---
 drivers/media/i2c/tc358743.c  | 4 ++--
 drivers/media/pci/bt8xx/bttv-cards.c  | 2 +-
 drivers/media/pci/cobalt/cobalt-v4l2.c| 2 +-
 drivers/media/pci/cx18/cx18-controls.c| 2 +-
 drivers/media/pci/cx18/cx18-controls.h| 2 +-
 drivers/media/pci/ivtv/ivtv-controls.c| 2 +-
 drivers/media/pci/ivtv/ivtv-controls.h| 2 +-
 drivers/media/platform/s5p-tv/hdmi_drv.c  | 2 +-
 drivers/media/platform/vivid/vivid-vid-cap.c  | 2 +-
 drivers/media/platform/vivid/vivid-vid-out.c  | 2 +-
 drivers/media/radio/radio-maxiradio.c | 2 +-
 drivers/media/radio/radio-sf16fmr2.c  | 2 +-
 drivers/media/radio/radio-shark.c | 2 +-
 drivers/media/usb/cx231xx/cx231xx-417.c   | 2 +-
 drivers/media/usb/hdpvr/hdpvr-video.c | 2 +-
 drivers/media/v4l2-core/v4l2-dv-timings.c | 9 +++--
 include/media/drv-intf/tea575x.h  | 2 +-
 include/media/v4l2-dv-timings.h   | 4 +++-
 sound/pci/es1968.c| 2 +-
 sound/pci/fm801.c | 2 +-
 23 files changed, 36 insertions(+), 29 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


[GIT PULL FOR v4.5] New ti-cal driver

2015-11-27 Thread Hans Verkuil
The following changes since commit 10897dacea26943dd80bd6629117f4620fc320ef:

  Merge tag 'v4.4-rc2' into patchwork (2015-11-23 14:16:58 -0200)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git cal

for you to fetch changes up to 5df9a7909b737e9725c1005b3b39383e9c2490ca:

  media: v4l: ti-vpe: Document DRA72 CAL h/w module (2015-11-27 14:25:07 +0100)


Benoit Parrot (2):
  media: v4l: ti-vpe: Add CAL v4l2 camera capture driver
  media: v4l: ti-vpe: Document DRA72 CAL h/w module

 Documentation/devicetree/bindings/media/ti-cal.txt |   72 ++
 drivers/media/platform/Kconfig |   12 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/ti-vpe/Makefile |4 +
 drivers/media/platform/ti-vpe/cal.c| 2143 
++
 drivers/media/platform/ti-vpe/cal_regs.h   |  779 +
 6 files changed, 3012 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
 create mode 100644 drivers/media/platform/ti-vpe/cal.c
 create mode 100644 drivers/media/platform/ti-vpe/cal_regs.h
--
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] cx18: Fix VIDIOC_TRY_FMT to fill in sizeimage and bytesperline

2015-11-27 Thread Hans Verkuil
On 11/24/2015 07:09 PM, Simon Farnsworth wrote:
> I was having trouble capturing raw video from GStreamer; turns out that I
> now need VIDIOC_TRY_FMT to fill in sizeimage and bytesperline to make it work.
> 
> Signed-off-by: Simon Farnsworth 
> ---
> 
> I'm leaving ONELAN on Friday, so this is a drive-by patch being sent for the
> benefit of anyone else trying to use raw capture from a cx18 card. If it's
> not suitable for applying as-is, please feel free to just leave it in the
> archives so that someone else hitting the same problem can find my fix.
> 
>  drivers/media/pci/cx18/cx18-ioctl.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/pci/cx18/cx18-ioctl.c 
> b/drivers/media/pci/cx18/cx18-ioctl.c
> index 55525af..1c9924a 100644
> --- a/drivers/media/pci/cx18/cx18-ioctl.c
> +++ b/drivers/media/pci/cx18/cx18-ioctl.c
> @@ -234,6 +234,13 @@ static int cx18_try_fmt_vid_cap(struct file *file, void 
> *fh,
>  
>   fmt->fmt.pix.width = w;
>   fmt->fmt.pix.height = h;
> + if (fmt->fmt.pix.pixelformat == V4L2_PIX_FMT_HM12) {
> + fmt->fmt.pix.sizeimage = h * 720 * 3 / 2;
> + fmt->fmt.pix.bytesperline = 720; /* First plane */
> + } else {
> + fmt->fmt.pix.sizeimage = h * 720 * 2;
> + fmt->fmt.pix.bytesperline = 1440; /* Packed */
> + }

This isn't correct: for MPEG formats bytesperline should be 0 and sizeimage is 
fixed
at 128*1024.

I really have no time to make a proper patch, Andy is this something you can 
look at?

Hmm, ivtv does it a bit better but it will return the wrong sizeimage.

Regards,

Hans

>   return 0;
>  }
>  
> 

--
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


Can't access CIR wakeup registers of Nuvoton 6779D

2015-11-27 Thread Heiner Kallweit
I have a Zotac CI321 mini-PC which uses a Nuvoton 6779D (or compatible) Super-IO
for infrared remote control. The normal CIR part works flawlessly however I 
can't
access the wakeup registers.
Resource allocation works normal, /proc/ioports reports:

0220-022e : nuvoton-cir-wake
02e0-02ee : nuvoton-cir

Also configuring the logical device works normal as can be seen in the debug 
output.
However all reads to this ioport region fail.
I tried manually changing the ioport region but this didn't help either.

Meanwhile I'm wondering whether there might be stripped down versions of this 
chip
w/o the wake functionality as I'm running out of other ideas.
Appreciate any hint.

[ 7608.622878] nuvoton_cir: nuvoton-cir: Dump CIR logical device registers:
[ 7608.622949] nuvoton_cir:  * CR CIR ACTIVE :   0x1
[ 7608.623005] nuvoton_cir:  * CR CIR BASE ADDR: 0x2e0
[ 7608.623059] nuvoton_cir:  * CR CIR IRQ NUM:   0x3
[ 7608.623109] nuvoton_cir: nuvoton-cir: Dump CIR registers:
[ 7608.623166] nuvoton_cir:  * IRCON: 0x36
[ 7608.623210] nuvoton_cir:  * IRSTS: 0x0
[ 7608.623255] nuvoton_cir:  * IREN:  0x60
[ 7608.623299] nuvoton_cir:  * RXFCONT:   0x0
[ 7608.623344] nuvoton_cir:  * CP:0x0
[ 7608.623388] nuvoton_cir:  * CC:0x0
[ 7608.623432] nuvoton_cir:  * SLCH:  0x7
[ 7608.623478] nuvoton_cir:  * SLCL:  0xd0
[ 7608.623524] nuvoton_cir:  * FIFOCON:   0x23
[ 7608.623569] nuvoton_cir:  * IRFIFOSTS: 0x96
[ 7608.623614] nuvoton_cir:  * SRXFIFO:   0x84
[ 7608.623659] nuvoton_cir:  * TXFCONT:   0x0
[ 7608.623706] nuvoton_cir:  * STXFIFO:   0x0
[ 7608.623762] nuvoton_cir:  * FCCH:  0x0
[ 7608.623808] nuvoton_cir:  * FCCL:  0x0
[ 7608.623853] nuvoton_cir:  * IRFSM: 0x14
[ 7608.623904] nuvoton_cir: nuvoton-cir: Dump CIR WAKE logical device registers:
[ 7608.623979] nuvoton_cir:  * CR CIR WAKE ACTIVE :   0x1
[ 7608.624039] nuvoton_cir:  * CR CIR WAKE BASE ADDR: 0x220
[ 7608.624097] nuvoton_cir:  * CR CIR WAKE IRQ NUM:   0x3
[ 7608.624151] nuvoton_cir: nuvoton-cir: Dump CIR WAKE registers
[ 7608.624210] nuvoton_cir:  * IRCON:  0xff
[ 7608.624258] nuvoton_cir:  * IRSTS:  0xff
[ 7608.624308] nuvoton_cir:  * IREN:   0xff
[ 7608.624356] nuvoton_cir:  * FIFO CMP DEEP:  0xff
[ 7608.624404] nuvoton_cir:  * FIFO CMP TOL:   0xff
[ 7608.624453] nuvoton_cir:  * FIFO COUNT: 0xff
[ 7608.624502] nuvoton_cir:  * SLCH:   0xff
[ 7608.624550] nuvoton_cir:  * SLCL:   0xff
[ 7608.625897] nuvoton_cir:  * FIFOCON:0xff
[ 7608.627204] nuvoton_cir:  * SRXFSTS:0xff
[ 7608.628481] nuvoton_cir:  * SAMPLE RX FIFO: 0xff
[ 7608.629393] nuvoton_cir:  * WR FIFO DATA:   0xff
[ 7608.630157] nuvoton_cir:  * RD FIFO ONLY:   0xff
[ 7608.630906] nuvoton_cir:  * RD FIFO ONLY IDX: 0xff
[ 7608.631648] nuvoton_cir:  * FIFO IGNORE:0xff
[ 7608.632380] nuvoton_cir:  * IRFSM:  0xff
[ 7608.633085] nuvoton_cir: nuvoton-cir: Dump CIR WAKE FIFO (len 255)
--
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


Reply Me For Details

2015-11-27 Thread HS09912HH3
Good day,

I wish to contact you personally for an important proposal that might be 
of interest to you. I am sending this mail just to know if this email address
is functional.

I have something absolutely essential to discuss with you. Contact me for 
details through my private email: shu...@qq.com

Regards,
shu...@qq.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


cron job: media_tree daily build: ERRORS

2015-11-27 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:   Sat Nov 28 04:00:23 CET 2015
git branch: test
git hash:   10897dacea26943dd80bd6629117f4620fc320ef
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3202-g618e15b
host hardware:  x86_64
host os:4.2.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.32.27-i686: ERRORS
linux-2.6.33.7-i686: ERRORS
linux-2.6.34.7-i686: ERRORS
linux-2.6.35.9-i686: ERRORS
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
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: ERRORS
linux-3.5.7-i686: ERRORS
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.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-rc1-i686: OK
linux-2.6.32.27-x86_64: ERRORS
linux-2.6.33.7-x86_64: ERRORS
linux-2.6.34.7-x86_64: ERRORS
linux-2.6.35.9-x86_64: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
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: ERRORS
linux-3.5.7-x86_64: ERRORS
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.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: ERRORS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.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


Re: ->poll() instances shouldn't be indefinitely blocking

2015-11-27 Thread Mauro Carvalho Chehab
Em Fri, 27 Nov 2015 05:00:26 +
Al Viro  escreveu:

> Take a look at this:
> static unsigned int gsc_m2m_poll(struct file *file,
> struct poll_table_struct *wait)
> {
> struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> struct gsc_dev *gsc = ctx->gsc_dev;
> int ret;
> 
> if (mutex_lock_interruptible(>lock))
> return -ERESTARTSYS;
> 
> ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> mutex_unlock(>lock);
> 
> return ret;
> }
> 
> a) ->poll() should not return -E...; callers expect just a bitmap of
> POLL... values.

Yeah. We fixed issues like that on other drivers along the time. I guess
this is a some bad code that people just cut-and-paste from legacy drivers
without looking into it.

The same kind of crap were found (and fixed) on other drivers, like
the fix on this changeset: 45053edc05 ('[media] saa7164: fix poll bugs').

> b) sure, it's nice that if this thing hangs, we'll be able to kill it.
> However, if one's ->poll() can hang indefinitely, it means bad things
> for poll(2), select(2), etc. semantics.  What the hell had been intended
> there?

I guess there was no special intent. It is just a bad driver code that
was replicated from other drivers.

> c) a bunch of v4l2_m2m_poll() callers are also taking some kind of
> mutex; AFAICS, all of those appear bogus (the rest of them do not
> play wiht ERESTARTSYS, just plain mutex_lock() for those).
> 
> What's going on there?
--
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] media: rc: raw: improve FIFO handling

2015-11-27 Thread Heiner Kallweit
The FIFO is used for ir_raw_event records, however for some historic
reason the FIFO is used on a per byte basis. IMHO this adds unneeded
complexity. Therefore set up the FIFO for ir_raw_event records.

This also allows to define the FIFO statically as part of
ir_raw_event_ctrl instead of having to allocate the FIFO dynamically.
In addition:

- When writing into the FIFO and it's full return ENOSPC instead of
  ENOMEM thus making it easier to tell between "FIFO full" and
  "Dynamic memory allocation failed" when the error is propagated to
  a higher level.
  Also add an error message.

- When reading from the FIFO check whether it's empty.
  This is not strictly needed here but kfifo_out is annotated
  "must check" anyway.

Successfully tested it with the nuvoton-cir driver.

Signed-off-by: Heiner Kallweit 
---
 drivers/media/rc/rc-core-priv.h |  6 +-
 drivers/media/rc/rc-ir-raw.c| 23 ---
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 7359f3d..585d5e5 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -16,6 +16,9 @@
 #ifndef _RC_CORE_PRIV
 #define _RC_CORE_PRIV
 
+/* Define the max number of pulse/space transitions to buffer */
+#defineMAX_IR_EVENT_SIZE   512
+
 #include 
 #include 
 #include 
@@ -35,7 +38,8 @@ struct ir_raw_event_ctrl {
struct list_headlist;   /* to keep track of raw 
clients */
struct task_struct  *thread;
spinlock_t  lock;
-   struct kfifo_rec_ptr_1  kfifo;  /* fifo for the 
pulse/space durations */
+   /* fifo for the pulse/space durations */
+   DECLARE_KFIFO(kfifo, struct ir_raw_event, MAX_IR_EVENT_SIZE);
ktime_t last_event; /* when last event 
occurred */
enum raw_event_type last_type;  /* last event type */
struct rc_dev   *dev;   /* pointer to the 
parent rc_dev */
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index c69807f..144304c 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -20,9 +20,6 @@
 #include 
 #include "rc-core-priv.h"
 
-/* Define the max number of pulse/space transitions to buffer */
-#define MAX_IR_EVENT_SIZE  512
-
 /* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */
 static LIST_HEAD(ir_raw_client_list);
 
@@ -36,14 +33,12 @@ static int ir_raw_event_thread(void *data)
struct ir_raw_event ev;
struct ir_raw_handler *handler;
struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data;
-   int retval;
 
while (!kthread_should_stop()) {
 
spin_lock_irq(>lock);
-   retval = kfifo_len(>kfifo);
 
-   if (retval < sizeof(ev)) {
+   if (!kfifo_len(>kfifo)) {
set_current_state(TASK_INTERRUPTIBLE);
 
if (kthread_should_stop())
@@ -54,7 +49,8 @@ static int ir_raw_event_thread(void *data)
continue;
}
 
-   retval = kfifo_out(>kfifo, , sizeof(ev));
+   if(!kfifo_out(>kfifo, , 1))
+   dev_err(>dev->dev, "IR event FIFO is empty!\n");
spin_unlock_irq(>lock);
 
mutex_lock(_raw_handler_lock);
@@ -87,8 +83,10 @@ int ir_raw_event_store(struct rc_dev *dev, struct 
ir_raw_event *ev)
IR_dprintk(2, "sample: (%05dus %s)\n",
   TO_US(ev->duration), TO_STR(ev->pulse));
 
-   if (kfifo_in(>raw->kfifo, ev, sizeof(*ev)) != sizeof(*ev))
-   return -ENOMEM;
+   if (!kfifo_put(>raw->kfifo, *ev)) {
+   dev_err(>dev, "IR event FIFO is full!\n");
+   return -ENOSPC;
+   }
 
return 0;
 }
@@ -273,11 +271,7 @@ int ir_raw_event_register(struct rc_dev *dev)
 
dev->raw->dev = dev;
dev->change_protocol = change_protocol;
-   rc = kfifo_alloc(>raw->kfifo,
-sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
-GFP_KERNEL);
-   if (rc < 0)
-   goto out;
+   INIT_KFIFO(dev->raw->kfifo);
 
spin_lock_init(>raw->lock);
dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
@@ -319,7 +313,6 @@ void ir_raw_event_unregister(struct rc_dev *dev)
handler->raw_unregister(dev);
mutex_unlock(_raw_handler_lock);
 
-   kfifo_free(>raw->kfifo);
kfree(dev->raw);
dev->raw = NULL;
 }
-- 
2.6.2

--
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] media: rc: nuvoton: mark wakeup-related resources

2015-11-27 Thread Heiner Kallweit
When requesting resources use different names for the normal and
the wakeup part. This makes it easier to interpret the output
of e.g. /proc/interrupts and /proc/ioports.

Signed-off-by: Heiner Kallweit 
---
 drivers/media/rc/nuvoton-cir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index 18adf58..081435c 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -1079,12 +1079,12 @@ static int nvt_probe(struct pnp_dev *pdev, const struct 
pnp_device_id *dev_id)
goto exit_unregister_device;
 
if (!devm_request_region(>dev, nvt->cir_wake_addr,
-   CIR_IOREG_LENGTH, NVT_DRIVER_NAME))
+   CIR_IOREG_LENGTH, NVT_DRIVER_NAME "-wake"))
goto exit_unregister_device;
 
if (devm_request_irq(>dev, nvt->cir_wake_irq,
 nvt_cir_wake_isr, IRQF_SHARED,
-NVT_DRIVER_NAME, (void *)nvt))
+NVT_DRIVER_NAME "-wake", (void *)nvt))
goto exit_unregister_device;
 
device_init_wakeup(>dev, true);
-- 
2.6.2

--
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