Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver
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 LinSigned-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
On Fri, Nov 27, 2015 at 7:18 AM, Mauro Carvalho Chehabwrote: > 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
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
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
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
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
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
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
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
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
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
Em Fri, 27 Nov 2015 05:00:26 + Al Viroescreveu: > 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
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
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