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: Wed Dec 5 05:00:10 CET 2018 media-tree git hash:9b90dc85c718443a3e573a0ccf55900ff4fa73ae media_build git hash: 47bf46ff21f75d1fe4ae3275a8692cb6ff77b6e8 v4l-utils git hash: cff58fcfbdf75381d5351f5ea8e7846f59cb7905 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.0-2-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.57-i686: ERRORS linux-3.16.57-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.123-i686: ERRORS linux-3.18.123-x86_64: ERRORS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.52-i686: ERRORS linux-4.1.52-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.159-i686: ERRORS linux-4.4.159-x86_64: ERRORS linux-4.5.7-i686: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-i686: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.10-i686: ERRORS linux-4.7.10-x86_64: ERRORS linux-4.8.17-i686: ERRORS linux-4.8.17-x86_64: ERRORS linux-4.9.131-i686: ERRORS linux-4.9.131-x86_64: ERRORS linux-4.10.17-i686: ERRORS linux-4.10.17-x86_64: ERRORS linux-4.11.12-i686: ERRORS linux-4.11.12-x86_64: ERRORS linux-4.12.14-i686: ERRORS linux-4.12.14-x86_64: ERRORS linux-4.13.16-i686: ERRORS linux-4.13.16-x86_64: ERRORS linux-4.14.74-i686: ERRORS linux-4.14.74-x86_64: ERRORS linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19.1-i686: OK linux-4.19.1-x86_64: OK linux-4.20-rc1-i686: OK linux-4.20-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS 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/index.html
Re: [PATCH v5] media: imx: add mem2mem device
Hi Hans, Philipp,
One comment on my side...
On 12/3/18 7:21 AM, Hans Verkuil wrote:
+void imx_media_mem2mem_device_unregister(struct imx_media_video_dev *vdev)
+{
+ struct mem2mem_priv *priv = to_mem2mem_priv(vdev);
+ struct video_device *vfd = priv->vdev.vfd;
+
+ mutex_lock(>mutex);
+
+ if (video_is_registered(vfd)) {
+ video_unregister_device(vfd);
+ media_entity_cleanup(>entity);
Is this needed?
If this is to be part of the media controller, then I expect to see a call
to v4l2_m2m_register_media_controller() somewhere.
Yes, I agree there should be a call to
v4l2_m2m_register_media_controller(). This driver does not connect with
any of the imx-media entities, but calling it will at least make the
mem2mem output/capture device entities (and processing entity) visible
in the media graph.
Philipp, can you pick/squash the following from my media-tree github fork?
6fa05f5170 ("media: imx: mem2mem: Add missing media-device header")
d355bf8b15 ("media: imx: Add missing unregister and remove of mem2mem
device")
6787a50cdc ("media: imx: mem2mem: Register with media control")
Steve
RE: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Laurent, > > Hi Rajmohan, > > > > On Tuesday, 4 December 2018 18:07:16 EET Mani, Rajmohan wrote: > > > >> On Thursday, 29 November 2018 21:51:32 EET Tomasz Figa wrote: > > > >>> On Thu, Nov 29, 2018 at 6:43 AM Laurent Pinchart wrote: > > > On Tuesday, 30 October 2018 00:22:54 EET Yong Zhi wrote: > > > >> > > > >> [snip] > > > >> > > > > 1. Link pad flag of video nodes (i.e. ipu3-imgu 0 output) need > > > > to be enabled prior to the test. > > > > 2. Stream tests are not performed since it requires > > > > pre-configuration for each case. > > > > > > And that's a bit of an issue. I've tested the driver with a > > > small script based on media-ctl to configure links and yavta to > > > interface with the video nodes, and got the following oops: > > > > [snip] > > > > > The script can be found at > > > https://lists.libcamera.org/pipermail/libcamera-devel/2018-Nove > > > mb > > > e > > > r/40.html. > > > > > > I may be doing something wrong (and I probably am), but in any > > > case, the driver shouldn't crash. Could you please have a look ? > > > >>> > > > >>> It looks like the driver doesn't have the default state > > > >>> initialized correctly somewhere and it ends up using 0 as the > > > >>> divisor in some calculation? Something to fix indeed. > > > >> > > > >> That's probably the case. I'll trust Intel to fix that in v8 :-) > > > > > > > > Ack. > > > > > > Thanks for catching this. > > > I was able to reproduce this error and I see that error handling is > > > missing, leading to the panic. > > > > > > https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media > > > /pci/intel/ipu3/ipu3-css-params.c?h=ipu3-v7= > > > 19cee7329ca2d0156043cac6afcde535e93310af#n433 > > > > > > is where the -EINVAL is returned. > > > > > > Setting the return type as int for the following function and all > > > its callers to use the return value properly to error out, makes the > > > panic go away. > > > > I assume that I will still not be able to process frames through the > > pipe then, as I'll get an error :-) Could you tell me why the > > configuration is incorrect and how I can fix it ? > > > > :) > Let me look into this more and get back. > Thanks for your patience. I can see a couple of steps missing in the script below. (https://lists.libcamera.org/pipermail/libcamera-devel/2018-November/40.html) >From patch 02 of this v7 series "doc-rst: Add Intel IPU3 documentation", under >section "Configuring ImgU V4L2 subdev for image processing"... 1. The pipe mode needs to be configured for the V4L2 subdev. Also the pipe mode of the corresponding V4L2 subdev should be set as desired (e.g 0 for video mode or 1 for still mode) through the control id 0x009819a1 as below. e.g v4l2n -d /dev/v4l-subdev7 --ctrl=0x009819A1=1 2. ImgU pipeline needs to be configured for image processing as below. RAW bayer frames go through the following ISP pipeline HW blocks to have the processed image output to the DDR memory. RAW bayer frame -> Input Feeder -> Bayer Down Scaling (BDS) -> Geometric Distortion Correction (GDC) -> DDR The ImgU V4L2 subdev has to be configured with the supported resolutions in all the above HW blocks, for a given input resolution. For a given supported resolution for an input frame, the Input Feeder, Bayer Down Scaling and GDC blocks should be configured with the supported resolutions. This information can be obtained by looking at the following IPU3 ISP configuration table for ov5670 sensor. https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master /baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss /graph_settings_ov5670.xml For the ov5670 example, for an input frame with a resolution of 2592x1944 (which is input to the ImgU subdev pad 0), the corresponding resolutions for input feeder, BDS and GDC are 2592x1944, 2592x1944 and 2560x1920 respectively. The following steps prepare the ImgU ISP pipeline for the image processing. 1. The ImgU V4L2 subdev data format should be set by using the VIDIOC_SUBDEV_S_FMT on pad 0, using the GDC width and height obtained above. 2. The ImgU V4L2 subdev cropping should be set by using the VIDIOC_SUBDEV_S_SELECTION on pad 0, with V4L2_SEL_TGT_CROP as the target, using the input feeder height and width. 3. The ImgU V4L2 subdev composing should be set by using the VIDIOC_SUBDEV_S_SELECTION on pad 0, with V4L2_SEL_TGT_COMPOSE as the target, using the BDS height and width. Once these 2 steps are done, the raw bayer frames can be input to the ImgU V4L2 subdev for processing. > Raj > > > > ipu3_css_osys_calc_frame_and_stripe_params() > > > > > > Will include the fix in v8. > > > > Thank you. > > > > > Thanks for catching this. > > > > You're welcome. > > > > -- > > Regards, > > > > Laurent Pinchart > > > >
[PATCHv2] Add ir-rcmm-driver
Fix typo RC_PROTO_BIT_RCMM issue.
[PATCH] [PATCHv2] Add ir-rcmm-driver
From: Patrick LERDA
---
drivers/media/rc/Kconfig | 10 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/ir-rcmm-decoder.c | 185 +
drivers/media/rc/rc-core-priv.h| 5 +
drivers/media/rc/rc-main.c | 3 +
include/media/rc-map.h | 6 +-
include/uapi/linux/lirc.h | 1 +
tools/include/uapi/linux/lirc.h| 1 +
8 files changed, 210 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/rc/ir-rcmm-decoder.c
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 1021c08a9ba4..b7e08324b874 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -133,6 +133,16 @@ config IR_IMON_DECODER
remote control and you would like to use it with a raw IR
receiver, or if you wish to use an encoder to transmit this IR.
+config IR_RCMM_DECODER
+ tristate "Enable IR raw decoder for the RC-MM protocol"
+ depends on RC_CORE
+ select BITREVERSE
+ default y
+
+ ---help---
+ Enable this option if you have IR with RC-MM protocol, and
+ if the IR is decoded in software
+
endif #RC_DECODERS
menuconfig RC_DEVICES
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index e0340d043fe8..fc4058013234 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_IR_SHARP_DECODER) += ir-sharp-decoder.o
obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
obj-$(CONFIG_IR_IMON_DECODER) += ir-imon-decoder.o
+obj-$(CONFIG_IR_RCMM_DECODER) += ir-rcmm-decoder.o
# stand-alone IR receivers/transmitters
obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
diff --git a/drivers/media/rc/ir-rcmm-decoder.c
b/drivers/media/rc/ir-rcmm-decoder.c
new file mode 100644
index ..94d5b70f7a0f
--- /dev/null
+++ b/drivers/media/rc/ir-rcmm-decoder.c
@@ -0,0 +1,185 @@
+/* ir-rcmm-decoder.c - A decoder for the RCMM IR protocol
+ *
+ * Copyright (C) 2016 by Patrick Lerda
+ *
+ * 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 "rc-core-priv.h"
+#include
+#include
+
+
+#define RCMM_UNIT 17 /* nanosecs */
+#define RCMM_0_NBITS 64
+#define RCMM_PREFIX_PULSE 41 /* 16.6*2.5 */
+#define RCMM_PULSE_027 /* 16.6*(1+2/3) */
+#define RCMM_PULSE_144 /* 16.6*(2+2/3) */
+#define RCMM_PULSE_261 /* 16.6*(3+2/3) */
+#define RCMM_PULSE_378 /* 16.6*(4+2/3) */
+#define RCMM_MODE_MASK 0x
+
+enum rcmm_state {
+ STATE_INACTIVE,
+ STATE_LOW,
+ STATE_BUMP,
+ STATE_VALUE,
+ STATE_FINISHED,
+};
+
+static bool rcmm_mode(struct rcmm_dec *data)
+{
+return !((0x000c & data->bits) == 0x000c);
+}
+
+/**
+ * ir_rcmm_decode() - Decode one RCMM pulse or space
+ * @dev: the struct rc_dev descriptor of the device
+ * @ev:the struct ir_raw_event descriptor of the pulse/space
+ *
+ * This function returns -EINVAL if the pulse violates the state machine
+ */
+static int ir_rcmm_decode(struct rc_dev *dev, struct ir_raw_event ev)
+{
+ struct rcmm_dec *data = >raw->rcmm;
+ u32 scancode;
+ u8 toggle;
+
+ if (!(dev->enabled_protocols & RC_PROTO_BIT_RCMM))
+ return 0;
+
+ if (!is_timing_event(ev)) {
+ if (ev.reset)
+ data->state = STATE_INACTIVE;
+ return 0;
+ }
+
+ if (ev.duration > RCMM_PULSE_3 + RCMM_UNIT)
+ goto out;
+
+ switch (data->state) {
+
+ case STATE_INACTIVE:
+ if (!ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+ adjust to the signal */
+ if (!eq_margin(ev.duration, RCMM_PREFIX_PULSE, RCMM_UNIT/2))
+ break;
+
+ data->state = STATE_LOW;
+ data->count = 0;
+ data->bits = 0;
+ return 0;
+
+ case STATE_LOW:
+ if (ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+ adjust to the signal */
+ if (!eq_margin(ev.duration, RCMM_PULSE_0,
[ragnatech:media-tree] BUILD INCOMPLETE 9b90dc85c718443a3e573a0ccf55900ff4fa73ae
tree/branch: git://git.ragnatech.se/linux media-tree branch HEAD: 9b90dc85c718443a3e573a0ccf55900ff4fa73ae media: seco-cec: add missing header file to fix build TIMEOUT after 1554m Sorry we cannot finish the testset for your branch within a reasonable time. It's our fault -- either some build server is down or some build worker is busy doing bisects for _other_ trees. The branch will get more complete coverage and possible error reports when our build infrastructure is restored or catches up. There will be no more build success notification for this branch head, but you can expect reasonably good test coverage after waiting for 1 day. configs timed out: 151 alphaallmodconfig alphaallyesconfig alpha defconfig arm allnoconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm sunxi_defconfig arm64 allnoconfig arm64 defconfig i386 allmodconfig i386randconfig-a0 i386randconfig-a1 i386randconfig-a2 i386randconfig-a3 i386randconfig-f0 i386randconfig-f1 i386randconfig-f2 i386randconfig-f3 i386randconfig-i0 i386randconfig-i1 i386randconfig-i2 i386randconfig-i3 i386randconfig-j0 i386randconfig-j1 i386randconfig-j2 i386randconfig-j3 i386randconfig-k0 i386randconfig-k1 i386randconfig-k2 i386randconfig-k3 i386randconfig-n0 i386randconfig-n1 i386randconfig-n2 i386randconfig-n3 i386randconfig-s0 i386randconfig-s1 i386randconfig-s2 i386randconfig-s3 i386 randconfig-x010 i386 randconfig-x011 i386 randconfig-x012 i386 randconfig-x013 i386 randconfig-x014 i386 randconfig-x015 i386 randconfig-x016 i386 randconfig-x017 i386 randconfig-x018 i386 randconfig-x019 i386 tinyconfig ia64 allmodconfig ia64 allyesconfig m68k allmodconfig m68k allyesconfig m68k m5475evb_defconfig m68k multi_defconfig m68k sun3_defconfig microblaze mmu_defconfig microblazenommu_defconfig mips 32r2_defconfig mips 64r6el_defconfig mips allmodconfig mips allyesconfig mips jz4740 mips txx9 nds32allmodconfig nds32allyesconfig openriscor1ksim_defconfig parisc allmodconfig pariscallnoconfig parisc allyesconfig parisc b180_defconfig pariscc3000_defconfig parisc defconfig powerpc allmodconfig powerpc allnoconfig powerpc allyesconfig powerpc defconfig powerpc ppc64_defconfig riscvallmodconfig riscvallyesconfig riscv tinyconfig s390 allmodconfig s390 allyesconfig s390default_defconfig sh allmodconfig shallnoconfig sh allyesconfig sh rsk7269_defconfig sh sh7785lcr_32bit_defconfig shtitan_defconfig
Re: [PATCH] Add ir-rcmm-driver
The toggle flag is well handled with this code and at the kernel level;
This adds reliability.
Patrick.
On 04/12/2018 22:06, patrick9...@free.fr wrote:
We have so many decoders at the linux kernel level now. I'm not sure
of the best option.
Patrick.
On 04/12/2018 21:57, Sean Young wrote:
On Tue, Dec 04, 2018 at 09:20:25PM +0100, patrick9...@free.fr wrote:
From: Patrick LERDA
---
drivers/media/rc/Kconfig | 10 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/ir-rcmm-decoder.c | 185
+
drivers/media/rc/rc-core-priv.h| 5 +
drivers/media/rc/rc-main.c | 3 +
include/media/rc-map.h | 6 +-
include/uapi/linux/lirc.h | 1 +
tools/include/uapi/linux/lirc.h| 1 +
8 files changed, 210 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/rc/ir-rcmm-decoder.c
We have a rc-mm decoder written in BPF, see:
https://git.linuxtv.org/v4l-utils.git/tree/utils/keytable/bpf_protocols/rc_mm.c
This is in v4l-utils 1.16 and higher.
Any reason to have it in the kernel rather than in BPF?
Sean
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 1021c08a9ba4..b7e08324b874 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -133,6 +133,16 @@ config IR_IMON_DECODER
remote control and you would like to use it with a raw IR
receiver, or if you wish to use an encoder to transmit this IR.
+config IR_RCMM_DECODER
+ tristate "Enable IR raw decoder for the RC-MM protocol"
+ depends on RC_CORE
+ select BITREVERSE
+ default y
+
+ ---help---
+ Enable this option if you have IR with RC-MM protocol, and
+ if the IR is decoded in software
+
endif #RC_DECODERS
menuconfig RC_DEVICES
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index e0340d043fe8..fc4058013234 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_IR_SHARP_DECODER) +=
ir-sharp-decoder.o
obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
obj-$(CONFIG_IR_IMON_DECODER) += ir-imon-decoder.o
+obj-$(CONFIG_IR_RCMM_DECODER) += ir-rcmm-decoder.o
# stand-alone IR receivers/transmitters
obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
diff --git a/drivers/media/rc/ir-rcmm-decoder.c
b/drivers/media/rc/ir-rcmm-decoder.c
new file mode 100644
index ..b05063f0a552
--- /dev/null
+++ b/drivers/media/rc/ir-rcmm-decoder.c
@@ -0,0 +1,185 @@
+/* ir-rcmm-decoder.c - A decoder for the RCMM IR protocol
+ *
+ * Copyright (C) 2016 by Patrick Lerda
+ *
+ * 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 "rc-core-priv.h"
+#include
+#include
+
+
+#define RCMM_UNIT 17 /* nanosecs */
+#define RCMM_0_NBITS 64
+#define RCMM_PREFIX_PULSE 41 /* 16.6*2.5 */
+#define RCMM_PULSE_027 /* 16.6*(1+2/3)
*/
+#define RCMM_PULSE_144 /* 16.6*(2+2/3)
*/
+#define RCMM_PULSE_261 /* 16.6*(3+2/3)
*/
+#define RCMM_PULSE_378 /* 16.6*(4+2/3)
*/
+#define RCMM_MODE_MASK 0x
+
+enum rcmm_state {
+ STATE_INACTIVE,
+ STATE_LOW,
+ STATE_BUMP,
+ STATE_VALUE,
+ STATE_FINISHED,
+};
+
+static bool rcmm_mode(struct rcmm_dec *data)
+{
+return !((0x000c & data->bits) == 0x000c);
+}
+
+/**
+ * ir_rcmm_decode() - Decode one RCMM pulse or space
+ * @dev: the struct rc_dev descriptor of the device
+ * @ev:the struct ir_raw_event descriptor of the pulse/space
+ *
+ * This function returns -EINVAL if the pulse violates the state
machine
+ */
+static int ir_rcmm_decode(struct rc_dev *dev, struct ir_raw_event
ev)
+{
+ struct rcmm_dec *data = >raw->rcmm;
+ u32 scancode;
+ u8 toggle;
+
+ if (!(dev->enabled_protocols & RC_PROTO_RCMM))
+ return 0;
+
+ if (!is_timing_event(ev)) {
+ if (ev.reset)
+ data->state = STATE_INACTIVE;
+ return 0;
+ }
+
+ if (ev.duration > RCMM_PULSE_3 + RCMM_UNIT)
+ goto out;
+
+ switch (data->state) {
+
+ case STATE_INACTIVE:
+ if (!ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+
Re: [PATCH] Add ir-rcmm-driver
We have so many decoders at the linux kernel level now. I'm not sure of
the best option.
Patrick.
On 04/12/2018 21:57, Sean Young wrote:
On Tue, Dec 04, 2018 at 09:20:25PM +0100, patrick9...@free.fr wrote:
From: Patrick LERDA
---
drivers/media/rc/Kconfig | 10 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/ir-rcmm-decoder.c | 185
+
drivers/media/rc/rc-core-priv.h| 5 +
drivers/media/rc/rc-main.c | 3 +
include/media/rc-map.h | 6 +-
include/uapi/linux/lirc.h | 1 +
tools/include/uapi/linux/lirc.h| 1 +
8 files changed, 210 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/rc/ir-rcmm-decoder.c
We have a rc-mm decoder written in BPF, see:
https://git.linuxtv.org/v4l-utils.git/tree/utils/keytable/bpf_protocols/rc_mm.c
This is in v4l-utils 1.16 and higher.
Any reason to have it in the kernel rather than in BPF?
Sean
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 1021c08a9ba4..b7e08324b874 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -133,6 +133,16 @@ config IR_IMON_DECODER
remote control and you would like to use it with a raw IR
receiver, or if you wish to use an encoder to transmit this IR.
+config IR_RCMM_DECODER
+ tristate "Enable IR raw decoder for the RC-MM protocol"
+ depends on RC_CORE
+ select BITREVERSE
+ default y
+
+ ---help---
+ Enable this option if you have IR with RC-MM protocol, and
+ if the IR is decoded in software
+
endif #RC_DECODERS
menuconfig RC_DEVICES
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index e0340d043fe8..fc4058013234 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_IR_SHARP_DECODER) += ir-sharp-decoder.o
obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
obj-$(CONFIG_IR_IMON_DECODER) += ir-imon-decoder.o
+obj-$(CONFIG_IR_RCMM_DECODER) += ir-rcmm-decoder.o
# stand-alone IR receivers/transmitters
obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
diff --git a/drivers/media/rc/ir-rcmm-decoder.c
b/drivers/media/rc/ir-rcmm-decoder.c
new file mode 100644
index ..b05063f0a552
--- /dev/null
+++ b/drivers/media/rc/ir-rcmm-decoder.c
@@ -0,0 +1,185 @@
+/* ir-rcmm-decoder.c - A decoder for the RCMM IR protocol
+ *
+ * Copyright (C) 2016 by Patrick Lerda
+ *
+ * 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 "rc-core-priv.h"
+#include
+#include
+
+
+#define RCMM_UNIT 17 /* nanosecs */
+#define RCMM_0_NBITS 64
+#define RCMM_PREFIX_PULSE 41 /* 16.6*2.5 */
+#define RCMM_PULSE_027 /* 16.6*(1+2/3)
*/
+#define RCMM_PULSE_144 /* 16.6*(2+2/3)
*/
+#define RCMM_PULSE_261 /* 16.6*(3+2/3)
*/
+#define RCMM_PULSE_378 /* 16.6*(4+2/3)
*/
+#define RCMM_MODE_MASK 0x
+
+enum rcmm_state {
+ STATE_INACTIVE,
+ STATE_LOW,
+ STATE_BUMP,
+ STATE_VALUE,
+ STATE_FINISHED,
+};
+
+static bool rcmm_mode(struct rcmm_dec *data)
+{
+return !((0x000c & data->bits) == 0x000c);
+}
+
+/**
+ * ir_rcmm_decode() - Decode one RCMM pulse or space
+ * @dev: the struct rc_dev descriptor of the device
+ * @ev:the struct ir_raw_event descriptor of the pulse/space
+ *
+ * This function returns -EINVAL if the pulse violates the state
machine
+ */
+static int ir_rcmm_decode(struct rc_dev *dev, struct ir_raw_event ev)
+{
+ struct rcmm_dec *data = >raw->rcmm;
+ u32 scancode;
+ u8 toggle;
+
+ if (!(dev->enabled_protocols & RC_PROTO_RCMM))
+ return 0;
+
+ if (!is_timing_event(ev)) {
+ if (ev.reset)
+ data->state = STATE_INACTIVE;
+ return 0;
+ }
+
+ if (ev.duration > RCMM_PULSE_3 + RCMM_UNIT)
+ goto out;
+
+ switch (data->state) {
+
+ case STATE_INACTIVE:
+ if (!ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+ adjust to the signal */
+ if (!eq_margin(ev.duration, RCMM_PREFIX_PULSE, RCMM_UNIT/2))
+ break;
+
+ data->state
Re: [PATCH] Add ir-rcmm-driver
On Tue, Dec 04, 2018 at 09:20:25PM +0100, patrick9...@free.fr wrote:
> From: Patrick LERDA
>
> ---
> drivers/media/rc/Kconfig | 10 ++
> drivers/media/rc/Makefile | 1 +
> drivers/media/rc/ir-rcmm-decoder.c | 185 +
> drivers/media/rc/rc-core-priv.h| 5 +
> drivers/media/rc/rc-main.c | 3 +
> include/media/rc-map.h | 6 +-
> include/uapi/linux/lirc.h | 1 +
> tools/include/uapi/linux/lirc.h| 1 +
> 8 files changed, 210 insertions(+), 2 deletions(-)
> create mode 100644 drivers/media/rc/ir-rcmm-decoder.c
We have a rc-mm decoder written in BPF, see:
https://git.linuxtv.org/v4l-utils.git/tree/utils/keytable/bpf_protocols/rc_mm.c
This is in v4l-utils 1.16 and higher.
Any reason to have it in the kernel rather than in BPF?
Sean
>
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 1021c08a9ba4..b7e08324b874 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -133,6 +133,16 @@ config IR_IMON_DECODER
> remote control and you would like to use it with a raw IR
> receiver, or if you wish to use an encoder to transmit this IR.
>
> +config IR_RCMM_DECODER
> + tristate "Enable IR raw decoder for the RC-MM protocol"
> + depends on RC_CORE
> + select BITREVERSE
> + default y
> +
> + ---help---
> +Enable this option if you have IR with RC-MM protocol, and
> +if the IR is decoded in software
> +
> endif #RC_DECODERS
>
> menuconfig RC_DEVICES
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index e0340d043fe8..fc4058013234 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_IR_SHARP_DECODER) += ir-sharp-decoder.o
> obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
> obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
> obj-$(CONFIG_IR_IMON_DECODER) += ir-imon-decoder.o
> +obj-$(CONFIG_IR_RCMM_DECODER) += ir-rcmm-decoder.o
>
> # stand-alone IR receivers/transmitters
> obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
> diff --git a/drivers/media/rc/ir-rcmm-decoder.c
> b/drivers/media/rc/ir-rcmm-decoder.c
> new file mode 100644
> index ..b05063f0a552
> --- /dev/null
> +++ b/drivers/media/rc/ir-rcmm-decoder.c
> @@ -0,0 +1,185 @@
> +/* ir-rcmm-decoder.c - A decoder for the RCMM IR protocol
> + *
> + * Copyright (C) 2016 by Patrick Lerda
> + *
> + * 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 "rc-core-priv.h"
> +#include
> +#include
> +
> +
> +#define RCMM_UNIT17 /* nanosecs */
> +#define RCMM_0_NBITS 64
> +#define RCMM_PREFIX_PULSE41 /* 16.6*2.5 */
> +#define RCMM_PULSE_027 /* 16.6*(1+2/3) */
> +#define RCMM_PULSE_144 /* 16.6*(2+2/3) */
> +#define RCMM_PULSE_261 /* 16.6*(3+2/3) */
> +#define RCMM_PULSE_378 /* 16.6*(4+2/3) */
> +#define RCMM_MODE_MASK 0x
> +
> +enum rcmm_state {
> + STATE_INACTIVE,
> + STATE_LOW,
> + STATE_BUMP,
> + STATE_VALUE,
> + STATE_FINISHED,
> +};
> +
> +static bool rcmm_mode(struct rcmm_dec *data)
> +{
> +return !((0x000c & data->bits) == 0x000c);
> +}
> +
> +/**
> + * ir_rcmm_decode() - Decode one RCMM pulse or space
> + * @dev: the struct rc_dev descriptor of the device
> + * @ev: the struct ir_raw_event descriptor of the pulse/space
> + *
> + * This function returns -EINVAL if the pulse violates the state machine
> + */
> +static int ir_rcmm_decode(struct rc_dev *dev, struct ir_raw_event ev)
> +{
> + struct rcmm_dec *data = >raw->rcmm;
> + u32 scancode;
> + u8 toggle;
> +
> + if (!(dev->enabled_protocols & RC_PROTO_RCMM))
> + return 0;
> +
> + if (!is_timing_event(ev)) {
> + if (ev.reset)
> + data->state = STATE_INACTIVE;
> + return 0;
> + }
> +
> + if (ev.duration > RCMM_PULSE_3 + RCMM_UNIT)
> + goto out;
> +
> + switch (data->state) {
> +
> + case STATE_INACTIVE:
> + if (!ev.pulse)
> + break;
> +
> + /* Note: larger margin on first pulse since each RCMM_UNIT
> +is quite short and some hardware takes some time to
> +adjust to the signal */
> + if (!eq_margin(ev.duration, RCMM_PREFIX_PULSE, RCMM_UNIT/2))
> + break;
>
[PATCH] media: vicodec: Change variable names
Change variables names in vicodec-core.c to *_src *_dst
to improve readability
Signed-off-by: Dafna Hirschfeld
---
drivers/media/platform/vicodec/vicodec-core.c | 94 ++-
1 file changed, 48 insertions(+), 46 deletions(-)
diff --git a/drivers/media/platform/vicodec/vicodec-core.c
b/drivers/media/platform/vicodec/vicodec-core.c
index b7bdfe97215b..78c6009a53c3 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -151,52 +151,52 @@ static struct vicodec_q_data *get_q_data(struct
vicodec_ctx *ctx,
}
static int device_process(struct vicodec_ctx *ctx,
- struct vb2_v4l2_buffer *in_vb,
- struct vb2_v4l2_buffer *out_vb)
+ struct vb2_v4l2_buffer *src_vb,
+ struct vb2_v4l2_buffer *dst_vb)
{
struct vicodec_dev *dev = ctx->dev;
- struct vicodec_q_data *q_cap;
+ struct vicodec_q_data *q_dst;
struct v4l2_fwht_state *state = >state;
- u8 *p_in, *p_out;
+ u8 *p_src, *p_dst;
int ret;
- q_cap = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
if (ctx->is_enc)
- p_in = vb2_plane_vaddr(_vb->vb2_buf, 0);
+ p_src = vb2_plane_vaddr(_vb->vb2_buf, 0);
else
- p_in = state->compressed_frame;
- p_out = vb2_plane_vaddr(_vb->vb2_buf, 0);
- if (!p_in || !p_out) {
+ p_src = state->compressed_frame;
+ p_dst = vb2_plane_vaddr(_vb->vb2_buf, 0);
+ if (!p_src || !p_dst) {
v4l2_err(>v4l2_dev,
"Acquiring kernel pointers to buffers failed\n");
return -EFAULT;
}
if (ctx->is_enc) {
- struct vicodec_q_data *q_out;
+ struct vicodec_q_data *q_src;
- q_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
- state->info = q_out->info;
- ret = v4l2_fwht_encode(state, p_in, p_out);
+ q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+ state->info = q_src->info;
+ ret = v4l2_fwht_encode(state, p_src, p_dst);
if (ret < 0)
return ret;
- vb2_set_plane_payload(_vb->vb2_buf, 0, ret);
+ vb2_set_plane_payload(_vb->vb2_buf, 0, ret);
} else {
- state->info = q_cap->info;
- ret = v4l2_fwht_decode(state, p_in, p_out);
+ state->info = q_dst->info;
+ ret = v4l2_fwht_decode(state, p_src, p_dst);
if (ret < 0)
return ret;
- vb2_set_plane_payload(_vb->vb2_buf, 0, q_cap->sizeimage);
+ vb2_set_plane_payload(_vb->vb2_buf, 0, q_dst->sizeimage);
}
- out_vb->sequence = q_cap->sequence++;
- out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp;
+ dst_vb->sequence = q_dst->sequence++;
+ dst_vb->vb2_buf.timestamp = src_vb->vb2_buf.timestamp;
- if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE)
- out_vb->timecode = in_vb->timecode;
- out_vb->field = in_vb->field;
- out_vb->flags &= ~V4L2_BUF_FLAG_LAST;
- out_vb->flags |= in_vb->flags &
+ if (src_vb->flags & V4L2_BUF_FLAG_TIMECODE)
+ dst_vb->timecode = src_vb->timecode;
+ dst_vb->field = src_vb->field;
+ dst_vb->flags &= ~V4L2_BUF_FLAG_LAST;
+ dst_vb->flags |= src_vb->flags &
(V4L2_BUF_FLAG_TIMECODE |
V4L2_BUF_FLAG_KEYFRAME |
V4L2_BUF_FLAG_PFRAME |
@@ -219,12 +219,12 @@ static void device_run(void *priv)
struct vicodec_ctx *ctx = priv;
struct vicodec_dev *dev = ctx->dev;
struct vb2_v4l2_buffer *src_buf, *dst_buf;
- struct vicodec_q_data *q_out;
+ struct vicodec_q_data *q_src;
u32 state;
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
- q_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+ q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
state = VB2_BUF_STATE_DONE;
if (device_process(ctx, src_buf, dst_buf))
@@ -237,11 +237,11 @@ static void device_run(void *priv)
v4l2_event_queue_fh(>fh, _event);
}
if (ctx->is_enc) {
- src_buf->sequence = q_out->sequence++;
+ src_buf->sequence = q_src->sequence++;
src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
v4l2_m2m_buf_done(src_buf, state);
} else if (vb2_get_plane_payload(_buf->vb2_buf, 0) ==
ctx->cur_buf_offset) {
- src_buf->sequence = q_out->sequence++;
+ src_buf->sequence = q_src->sequence++;
src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
[PATCH] Add ir-rcmm-driver
From: Patrick LERDA
---
drivers/media/rc/Kconfig | 10 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/ir-rcmm-decoder.c | 185 +
drivers/media/rc/rc-core-priv.h| 5 +
drivers/media/rc/rc-main.c | 3 +
include/media/rc-map.h | 6 +-
include/uapi/linux/lirc.h | 1 +
tools/include/uapi/linux/lirc.h| 1 +
8 files changed, 210 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/rc/ir-rcmm-decoder.c
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 1021c08a9ba4..b7e08324b874 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -133,6 +133,16 @@ config IR_IMON_DECODER
remote control and you would like to use it with a raw IR
receiver, or if you wish to use an encoder to transmit this IR.
+config IR_RCMM_DECODER
+ tristate "Enable IR raw decoder for the RC-MM protocol"
+ depends on RC_CORE
+ select BITREVERSE
+ default y
+
+ ---help---
+ Enable this option if you have IR with RC-MM protocol, and
+ if the IR is decoded in software
+
endif #RC_DECODERS
menuconfig RC_DEVICES
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index e0340d043fe8..fc4058013234 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_IR_SHARP_DECODER) += ir-sharp-decoder.o
obj-$(CONFIG_IR_MCE_KBD_DECODER) += ir-mce_kbd-decoder.o
obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
obj-$(CONFIG_IR_IMON_DECODER) += ir-imon-decoder.o
+obj-$(CONFIG_IR_RCMM_DECODER) += ir-rcmm-decoder.o
# stand-alone IR receivers/transmitters
obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
diff --git a/drivers/media/rc/ir-rcmm-decoder.c
b/drivers/media/rc/ir-rcmm-decoder.c
new file mode 100644
index ..b05063f0a552
--- /dev/null
+++ b/drivers/media/rc/ir-rcmm-decoder.c
@@ -0,0 +1,185 @@
+/* ir-rcmm-decoder.c - A decoder for the RCMM IR protocol
+ *
+ * Copyright (C) 2016 by Patrick Lerda
+ *
+ * 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 "rc-core-priv.h"
+#include
+#include
+
+
+#define RCMM_UNIT 17 /* nanosecs */
+#define RCMM_0_NBITS 64
+#define RCMM_PREFIX_PULSE 41 /* 16.6*2.5 */
+#define RCMM_PULSE_027 /* 16.6*(1+2/3) */
+#define RCMM_PULSE_144 /* 16.6*(2+2/3) */
+#define RCMM_PULSE_261 /* 16.6*(3+2/3) */
+#define RCMM_PULSE_378 /* 16.6*(4+2/3) */
+#define RCMM_MODE_MASK 0x
+
+enum rcmm_state {
+ STATE_INACTIVE,
+ STATE_LOW,
+ STATE_BUMP,
+ STATE_VALUE,
+ STATE_FINISHED,
+};
+
+static bool rcmm_mode(struct rcmm_dec *data)
+{
+return !((0x000c & data->bits) == 0x000c);
+}
+
+/**
+ * ir_rcmm_decode() - Decode one RCMM pulse or space
+ * @dev: the struct rc_dev descriptor of the device
+ * @ev:the struct ir_raw_event descriptor of the pulse/space
+ *
+ * This function returns -EINVAL if the pulse violates the state machine
+ */
+static int ir_rcmm_decode(struct rc_dev *dev, struct ir_raw_event ev)
+{
+ struct rcmm_dec *data = >raw->rcmm;
+ u32 scancode;
+ u8 toggle;
+
+ if (!(dev->enabled_protocols & RC_PROTO_RCMM))
+ return 0;
+
+ if (!is_timing_event(ev)) {
+ if (ev.reset)
+ data->state = STATE_INACTIVE;
+ return 0;
+ }
+
+ if (ev.duration > RCMM_PULSE_3 + RCMM_UNIT)
+ goto out;
+
+ switch (data->state) {
+
+ case STATE_INACTIVE:
+ if (!ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+ adjust to the signal */
+ if (!eq_margin(ev.duration, RCMM_PREFIX_PULSE, RCMM_UNIT/2))
+ break;
+
+ data->state = STATE_LOW;
+ data->count = 0;
+ data->bits = 0;
+ return 0;
+
+ case STATE_LOW:
+ if (ev.pulse)
+ break;
+
+ /* Note: larger margin on first pulse since each RCMM_UNIT
+ is quite short and some hardware takes some time to
+ adjust to the signal */
+ if (!eq_margin(ev.duration, RCMM_PULSE_0, RCMM_UNIT/2))
[PATCH] Add ir-rcmm-driver
Add support for RCMM infrared remote controls.
[GIT PULL FOR v4.21] more dvb fixes
Hi Mauro, I think these are the all the outstanding dvb patches for the kernel. Thanks, Sean The following changes since commit 9b90dc85c718443a3e573a0ccf55900ff4fa73ae: media: seco-cec: add missing header file to fix build (2018-12-03 15:11:00 -0500) are available in the Git repository at: git://linuxtv.org/syoung/media_tree.git for-v4.21c for you to fetch changes up to 669c1d49153a4db43320ba6dcae16a321f5f4ed4: media: usb: dvb-usb: remove old friio driver (2018-12-04 19:23:20 +) Corentin Labbe (1): media: usb: dvb-usb: remove old friio driver Malcolm Priestley (2): media: lmedm04: Move usb buffer to lme2510_state. media: lmedm04: use dvb_usbv2_generic_rw_locked Nikita Gerasimov (1): media: rtl28xxu: add support for Sony CXD2837ER slave demod Sean Young (1): media: dib7000p: Remove dead code drivers/media/dvb-frontends/dib7000p.c | 7 +- drivers/media/usb/dvb-usb-v2/Kconfig| 1 + drivers/media/usb/dvb-usb-v2/lmedm04.c | 73 + drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 40 ++- drivers/media/usb/dvb-usb-v2/rtl28xxu.h | 4 +- drivers/media/usb/dvb-usb/friio-fe.c| 440 --- drivers/media/usb/dvb-usb/friio.c | 522 drivers/media/usb/dvb-usb/friio.h | 99 -- 8 files changed, 60 insertions(+), 1126 deletions(-) delete mode 100644 drivers/media/usb/dvb-usb/friio-fe.c delete mode 100644 drivers/media/usb/dvb-usb/friio.c delete mode 100644 drivers/media/usb/dvb-usb/friio.h
Re: [PATCH 3/4] media: lmedm04: Move interrupt buffer to priv buffer.
On Thu, Nov 29, 2018 at 10:30:15PM +, Malcolm Priestley wrote:
> Interrupt is always present throught life time of
> there is no dma element move this buffer to private
> area of driver.
>
> Signed-off-by: Malcolm Priestley
> ---
> drivers/media/usb/dvb-usb-v2/lmedm04.c | 26 +-
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> index 8fb53b83c914..7b1aaed259db 100644
> --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> @@ -134,7 +134,7 @@ struct lme2510_state {
> u8 stream_on;
> u8 pid_size;
> u8 pid_off;
> - void *buffer;
> + u8 int_buffer[128];
> struct urb *lme_urb;
> u8 usb_buffer[64];
> /* Frontend original calls */
> @@ -408,20 +408,14 @@ static int lme2510_int_read(struct dvb_usb_adapter
> *adap)
> if (lme_int->lme_urb == NULL)
> return -ENOMEM;
>
> - lme_int->buffer = usb_alloc_coherent(d->udev, 128, GFP_ATOMIC,
> - _int->lme_urb->transfer_dma);
> -
The buffer was allocated with usb_alloc_coherent, however now it is
allocated with kmalloc.
> - if (lme_int->buffer == NULL)
> - return -ENOMEM;
> -
> usb_fill_int_urb(lme_int->lme_urb,
> - d->udev,
> - usb_rcvintpipe(d->udev, 0xa),
> - lme_int->buffer,
> - 128,
> - lme2510_int_response,
> - adap,
> - 8);
> + d->udev,
> + usb_rcvintpipe(d->udev, 0xa),
> + lme_int->int_buffer,
> + sizeof(lme_int->int_buffer),
> + lme2510_int_response,
> + adap,
> + 8);
>
> /* Quirk of pipe reporting PIPE_BULK but behaves as interrupt */
> ep = usb_pipe_endpoint(d->udev, lme_int->lme_urb->pipe);
On line 408:
lme_int->lme_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
This requires usb_alloc_coherent().
> @@ -1245,11 +1239,9 @@ static void lme2510_exit(struct dvb_usb_device *d)
> lme2510_kill_urb(>stream);
> }
>
> - if (st->lme_urb != NULL) {
> + if (st->lme_urb) {
> usb_kill_urb(st->lme_urb);
> usb_free_urb(st->lme_urb);
> - usb_free_coherent(d->udev, 128, st->buffer,
> - st->lme_urb->transfer_dma);
> info("Interrupt Service Stopped");
> }
> }
> --
> 2.19.1
Re: [PATCH 1/4] media: lmedm04: Add missing usb_free_urb to free, interrupt urb
On Thu, Nov 29, 2018 at 10:29:31PM +, Malcolm Priestley wrote:
> The interrupt urb is killed but never freed add the function
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Malcolm Priestley
> ---
> drivers/media/usb/dvb-usb-v2/lmedm04.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> index f109c04f05ae..8b405e131439 100644
> --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> @@ -1264,6 +1264,7 @@ static void *lme2510_exit_int(struct dvb_usb_device *d)
>
> if (st->lme_urb != NULL) {
> usb_kill_urb(st->lme_urb);
> + usb_free_urb(st->lme_urb);
Now st->lme_urb is a stale pointer.
> usb_free_coherent(d->udev, 128, st->buffer,
> st->lme_urb->transfer_dma);
And now you're following it.
> info("Interrupt Service Stopped");
> --
> 2.19.1
RE: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Laurent, > -Original Message- > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] > Sent: Tuesday, December 04, 2018 8:42 AM > To: Mani, Rajmohan > Cc: Tomasz Figa ; Zhi, Yong ; > Linux Media Mailing List ; Sakari Ailus > ; Mauro Carvalho Chehab > ; Hans Verkuil ; Zheng, Jian > Xu ; Hu, Jerry W ; > Toivonen, Tuukka ; Qiu, Tian Shu > ; Cao, Bingbu > Subject: Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset > > Hi Rajmohan, > > On Tuesday, 4 December 2018 18:07:16 EET Mani, Rajmohan wrote: > > >> On Thursday, 29 November 2018 21:51:32 EET Tomasz Figa wrote: > > >>> On Thu, Nov 29, 2018 at 6:43 AM Laurent Pinchart wrote: > > On Tuesday, 30 October 2018 00:22:54 EET Yong Zhi wrote: > > >> > > >> [snip] > > >> > > > 1. Link pad flag of video nodes (i.e. ipu3-imgu 0 output) need > > > to be enabled prior to the test. > > > 2. Stream tests are not performed since it requires > > > pre-configuration for each case. > > > > And that's a bit of an issue. I've tested the driver with a small > > script based on media-ctl to configure links and yavta to > > interface with the video nodes, and got the following oops: > > [snip] > > > The script can be found at > > https://lists.libcamera.org/pipermail/libcamera-devel/2018-Novemb > > e > > r/40.html. > > > > I may be doing something wrong (and I probably am), but in any > > case, the driver shouldn't crash. Could you please have a look ? > > >>> > > >>> It looks like the driver doesn't have the default state > > >>> initialized correctly somewhere and it ends up using 0 as the > > >>> divisor in some calculation? Something to fix indeed. > > >> > > >> That's probably the case. I'll trust Intel to fix that in v8 :-) > > > > > > Ack. > > > > Thanks for catching this. > > I was able to reproduce this error and I see that error handling is > > missing, leading to the panic. > > > > https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media > > /pci/intel/ipu3/ipu3-css-params.c?h=ipu3-v7= > > 19cee7329ca2d0156043cac6afcde535e93310af#n433 > > > > is where the -EINVAL is returned. > > > > Setting the return type as int for the following function and all its > > callers to use the return value properly to error out, makes the panic > > go away. > > I assume that I will still not be able to process frames through the pipe > then, as > I'll get an error :-) Could you tell me why the configuration is incorrect > and how > I can fix it ? > :) Let me look into this more and get back. Thanks for your patience. Raj > > ipu3_css_osys_calc_frame_and_stripe_params() > > > > Will include the fix in v8. > > Thank you. > > > Thanks for catching this. > > You're welcome. > > -- > Regards, > > Laurent Pinchart > >
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Rajmohan, On Tuesday, 4 December 2018 18:07:16 EET Mani, Rajmohan wrote: > >> On Thursday, 29 November 2018 21:51:32 EET Tomasz Figa wrote: > >>> On Thu, Nov 29, 2018 at 6:43 AM Laurent Pinchart wrote: > On Tuesday, 30 October 2018 00:22:54 EET Yong Zhi wrote: > >> > >> [snip] > >> > > 1. Link pad flag of video nodes (i.e. ipu3-imgu 0 output) need to > > be enabled prior to the test. > > 2. Stream tests are not performed since it requires > > pre-configuration for each case. > > And that's a bit of an issue. I've tested the driver with a small > script based on media-ctl to configure links and yavta to > interface with the video nodes, and got the following oops: [snip] > The script can be found at > https://lists.libcamera.org/pipermail/libcamera-devel/2018-Novembe > r/40.html. > > I may be doing something wrong (and I probably am), but in any > case, the driver shouldn't crash. Could you please have a look ? > >>> > >>> It looks like the driver doesn't have the default state initialized > >>> correctly somewhere and it ends up using 0 as the divisor in some > >>> calculation? Something to fix indeed. > >> > >> That's probably the case. I'll trust Intel to fix that in v8 :-) > > > > Ack. > > Thanks for catching this. > I was able to reproduce this error and I see that error handling > is missing, leading to the panic. > > https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media > /pci/intel/ipu3/ipu3-css-params.c?h=ipu3-v7= > 19cee7329ca2d0156043cac6afcde535e93310af#n433 > > is where the -EINVAL is returned. > > Setting the return type as int for the following function and all > its callers to use the return value properly to error out, makes > the panic go away. I assume that I will still not be able to process frames through the pipe then, as I'll get an error :-) Could you tell me why the configuration is incorrect and how I can fix it ? > ipu3_css_osys_calc_frame_and_stripe_params() > > Will include the fix in v8. Thank you. > Thanks for catching this. You're welcome. -- Regards, Laurent Pinchart
RE: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Laurent, Tomasz, > > Thanks for the reviews. > > > Subject: Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset > > > > Hi Tomasz, > > > > On Thursday, 29 November 2018 21:51:32 EET Tomasz Figa wrote: > > > On Thu, Nov 29, 2018 at 6:43 AM Laurent Pinchart wrote: > > > > On Tuesday, 30 October 2018 00:22:54 EET Yong Zhi wrote: > > > > [snip] > > > > > >> 1. Link pad flag of video nodes (i.e. ipu3-imgu 0 output) need to > > > >> be enabled prior to the test. > > > >> 2. Stream tests are not performed since it requires > > > >> pre-configuration for each case. > > > > > > > > And that's a bit of an issue. I've tested the driver with a small > > > > script based on media-ctl to configure links and yavta to > > > > interface with the video nodes, and got the following oops: > > > > > > > > [ 136.927788] divide error: [#1] PREEMPT SMP PTI [ > > > > 136.927801] CPU: 2 PID: 2069 Comm: yavta Not tainted 4.20.0-rc1+ > > > > #9 [ 136.927806] Hardware name: HP Soraka/Soraka, BIOS > > > > 08/30/2018 [ 136.927820] RIP: 0010:ipu3_css_osys_calc+0xc54/0xe14 > > > > [ipu3_imgu] [ 136.927825] Code: 89 44 24 28 42 8b 44 86 6c f7 54 > > > > 24 04 81 64 24 28 > > > > 00 fd ff ff 81 64 24 04 00 03 00 00 8d 44 03 ff 81 44 24 28 80 03 > > > > 00 > > > > 00 99 fb 0f af c3 bb 20 00 00 00 99 f7 fb 8b 5c 24 40 83 fd > > > > 01 > > > > 19 d2 [ 136.927830] RSP: 0018:9af2c0b837c8 EFLAGS: 00010202 [ > > > > 136.927835] RAX: RBX: RCX: > > > > 9af2c3e353c0 > > > > [ 136.927839] RDX: RSI: 9af2c0b838e0 RDI: > > > > 9af2c3e353c0 > > > > [ 136.927843] RBP: 0001 R08: R09: > > > > 9af2c0b83880 > > > > [ 136.927846] R10: 9af2c3e353c0 R11: 9af2c3e357c0 R12: > > > > 03a0 > > > > [ 136.927849] R13: 00025a0a R14: R15: > > > > > > > > [ 136.927854] FS: 7f1eca167700() > > > > GS:8c19fab0() > > > > knlGS: > > > > > > > > [ 136.927858] CS: 0010 DS: ES: CR0: 80050033 [ > > > > 136.927862] CR2: 7f1ec776c000 CR3: 0001312a4003 CR4: > > > > 003606e0 > > > > [ 136.927865] Call Trace: > > > > [ 136.927884] ? __accumulate_pelt_segments+0x29/0x3a > > > > [ 136.927892] ? __switch_to_asm+0x40/0x70 [ 136.927899] ? > > > > alloc_vmap_area+0x78/0x2f6 [ 136.927903] ? > > > > __switch_to_asm+0x40/0x70 [ 136.927907] ? > > > > __switch_to_asm+0x34/0x70 [ 136.927911] ? > > > > __switch_to_asm+0x40/0x70 [ 136.927915] ? > > > > __switch_to_asm+0x34/0x70 [ 136.927923] ? > > > > __inc_numa_state+0x28/0x70 [ 136.927929] ? > > > > preempt_latency_start+0x1e/0x3d [ 136.927936] ? > > > > get_page_from_freelist+0x821/0xb62 > > > > [ 136.927943] ? slab_pre_alloc_hook+0x12/0x3b [ 136.927948] ? > > > > kmem_cache_alloc_node_trace+0xf6/0x108 > > > > [ 136.927954] ? alloc_vmap_area+0x78/0x2f6 > > > > > > Is it just me or the backtrace above doesn't seem to make sense? I > > > don't see any allocations inside ipu3_css_cfg_acc(). > > > > I suppose that's why it's prefixed with '?' :-) > > > > > > [ 136.927965] ipu3_css_cfg_acc+0xa0/0x1b5f [ipu3_imgu] [ > > > > 136.927981] ipu3_css_set_parameters+0x286/0x6e7 [ipu3_imgu] [ > > > > 136.927995] ipu3_css_start_streaming+0x1230/0x130a [ipu3_imgu] [ > > > > 136.928010] imgu_s_stream+0x104/0x2f7 [ipu3_imgu] [ 136.928022] > > > > ipu3_vb2_start_streaming+0x168/0x1bd [ipu3_imgu] [ 136.928034] > > > > vb2_start_streaming+0x6c/0xf2 [videobuf2_common] [ 136.928044] > > > > vb2_core_streamon+0xcf/0x109 [videobuf2_common] [ 136.928061] > > > > __video_do_ioctl+0x239/0x388 [videodev] [ 136.928081] > > > > video_usercopy+0x25d/0x47a [videodev] [ 136.928097] ? > > > > copy_overflow+0x14/0x14 [videodev] [ 136.928115] > > > > v4l2_ioctl+0x4d/0x58 [videodev] [ 136.928123] > > > > vfs_ioctl+0x1b/0x28 [ 136.928130] do_vfs_ioctl+0x4de/0x566 [ > > > > 136.928139] > > > > ksys_ioctl+0x50/0x70 [ 136.928146] __x64_sys_ioctl+0x16/0x19 [ > > > > 136.928152] do_syscall_64+0x4d/0x5a [ 136.928158] > > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > [ 136.928164] RIP: 0033:0x7f1ec9a84f47 [ 136.928169] Code: 00 00 > > > > 00 48 8b 05 51 6f 2c 00 64 c7 00 26 00 00 00 48 > > > > c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 > > > > 0f > > > > 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 21 6f 2c 00 f7 d8 64 89 > > > > 01 > > > > 48 [ 136.928173] RSP: 002b:7ffe279e6188 EFLAGS: 0246 > > ORIG_RAX: > > > > 0010 > > > > [ 136.928178] RAX: ffda RBX: 0007 RCX: > > > > 7f1ec9a84f47 > > > > [ 136.928181] RDX: 7ffe279e6194 RSI: 40045612 RDI: > > > > 0003 > > > > [ 136.928184] RBP: R08: 7f1ec776d000 R09: > > > > > > > > [ 136.928188] R10: 0020 R11: 0246 R12: > > > > 7ffe279e6360 > > > > [ 136.928191] R13: 0004 R14:
Greetings...
Get back to us, it is highly important. Mr.Ahmed Yaki
Re: [PATCH v2 1/1] media: Use common test pattern menu entries
Em Tue, 4 Dec 2018 15:40:42 +0200
Sakari Ailus escreveu:
> While the test pattern menu itself is not standardised, many devices
> support the same test patterns. Aligning the menu entries helps the user
> space to use the interface, and adding macros for the menu entry strings
> helps to keep them aligned.
>
> Signed-off-by: Sakari Ailus
> ---
> since v1:
>
> - Fix indentation of menu strings
> - Remove "8" from the macro names
>
> drivers/media/i2c/imx258.c | 10 +-
> drivers/media/i2c/imx319.c | 10 +-
> drivers/media/i2c/imx355.c | 10 +-
> drivers/media/i2c/ov2640.c | 4 ++--
> drivers/media/i2c/smiapp/smiapp-core.c | 10 +-
> include/uapi/linux/v4l2-controls.h | 5 +
> 6 files changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index f86ae18bc104..df5f016cebd9 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -498,11 +498,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
> };
>
> static const char * const imx258_test_pattern_menu[] = {
> - "Disabled",
> - "Solid Colour",
> - "Eight Vertical Colour Bars",
> - "Colour Bars With Fade to Grey",
> - "Pseudorandom Sequence (PN9)",
> + V4L2_TEST_PATTERN_DISABLED,
> + V4L2_TEST_PATTERN_SOLID_COLOUR,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
> + V4L2_TEST_PATTERN_PN9,
> };
>
> /* Configurations for supported link frequencies */
> diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
> index 17c2e4b41221..d9d4176b9d37 100644
> --- a/drivers/media/i2c/imx319.c
> +++ b/drivers/media/i2c/imx319.c
> @@ -1647,11 +1647,11 @@ static const struct imx319_reg mode_1280x720_regs[] =
> {
> };
>
> static const char * const imx319_test_pattern_menu[] = {
> - "Disabled",
> - "Solid Colour",
> - "Eight Vertical Colour Bars",
> - "Colour Bars With Fade to Grey",
> - "Pseudorandom Sequence (PN9)",
> + V4L2_TEST_PATTERN_DISABLED,
> + V4L2_TEST_PATTERN_SOLID_COLOUR,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
> + V4L2_TEST_PATTERN_PN9,
> };
>
> /* supported link frequencies */
> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index bed293b60e50..99138a291cb8 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c
> @@ -875,11 +875,11 @@ static const struct imx355_reg mode_820x616_regs[] = {
> };
>
> static const char * const imx355_test_pattern_menu[] = {
> - "Disabled",
> - "Solid Colour",
> - "Eight Vertical Colour Bars",
> - "Colour Bars With Fade to Grey",
> - "Pseudorandom Sequence (PN9)",
> + V4L2_TEST_PATTERN_DISABLED,
> + V4L2_TEST_PATTERN_SOLID_COLOUR,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
> + V4L2_TEST_PATTERN_PN9,
> };
>
> /* supported link frequencies */
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 5d2d6735cc78..65058d9a5d51 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -707,8 +707,8 @@ static int ov2640_reset(struct i2c_client *client)
> }
>
> static const char * const ov2640_test_pattern_menu[] = {
> - "Disabled",
> - "Eight Vertical Colour Bars",
> + V4L2_TEST_PATTERN_DISABLED,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
> };
>
> /*
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index 58a45c353e27..5c9bcc9438ec 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -409,11 +409,11 @@ static void smiapp_update_mbus_formats(struct
> smiapp_sensor *sensor)
> }
>
> static const char * const smiapp_test_patterns[] = {
> - "Disabled",
> - "Solid Colour",
> - "Eight Vertical Colour Bars",
> - "Colour Bars With Fade to Grey",
> - "Pseudorandom Sequence (PN9)",
> + V4L2_TEST_PATTERN_DISABLED,
> + V4L2_TEST_PATTERN_SOLID_COLOUR,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
> + V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
> + V4L2_TEST_PATTERN_PN9,
> };
>
> static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h
> index 998983a6e6b7..acb2a57fa5d6 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
> #define V4L2_CID_LINK_FREQ (V4L2_CID_IMAGE_PROC_CLASS_BASE
> + 1)
> #define V4L2_CID_PIXEL_RATE (V4L2_CID_IMAGE_PROC_CLASS_BASE
> + 2)
> #define V4L2_CID_TEST_PATTERN
> (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> +#define
Re: [PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic
Em Tue, 4 Dec 2018 12:32:32 -0200
Mauro Carvalho Chehab escreveu:
> Em Tue, 4 Dec 2018 15:45:06 +0200
> Sakari Ailus escreveu:
>
> > The test pattern could contain a different number of colour bars than
> > eight, make the entry more useful by removing "Eight " from the name.
> >
> > Signed-off-by: Sakari Ailus
> > ---
> > include/uapi/linux/v4l2-controls.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/v4l2-controls.h
> > b/include/uapi/linux/v4l2-controls.h
> > index acb2a57fa5d6..88f2759c2ce4 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
> > #define V4L2_CID_TEST_PATTERN
> > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> > #define V4L2_TEST_PATTERN_DISABLED "Disabled"
> > #define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
> > -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
> > +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Vertical Colour Bars"
>
> No, we can't do that. This is on an uAPI file.
>
> We should, instead, create another #define for non-eight vertical
> color bars.
>
> Before you say, yeah, I agree that we messed this one, as the defined
> name doesn't mention 8 bars...
>
> I would, instead, do something like:
>
> -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_N_COLOUR_BARS "Vertical Colour Bars"
> +
> + /* Kept for backward-compatibility */
> +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS
> V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS
>
> And, of course, update the documentation accordingly.
Please ignore this comment. I didn't realize that those definitions
don't exist yet at the uAPI file, and that this is a follow up for
another patch:
Subject: [PATCH v2 1/1] media: Use common test pattern menu entries
Next time, please put them into a patch series, as it makes easier for
reviewers.
Thanks,
Mauro
Re: [PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic
Em Tue, 4 Dec 2018 15:45:06 +0200
Sakari Ailus escreveu:
> The test pattern could contain a different number of colour bars than
> eight, make the entry more useful by removing "Eight " from the name.
>
> Signed-off-by: Sakari Ailus
> ---
> include/uapi/linux/v4l2-controls.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h
> index acb2a57fa5d6..88f2759c2ce4 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
> #define V4L2_CID_TEST_PATTERN
> (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> #define V4L2_TEST_PATTERN_DISABLED "Disabled"
> #define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
> -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Vertical Colour Bars"
No, we can't do that. This is on an uAPI file.
We should, instead, create another #define for non-eight vertical
color bars.
Before you say, yeah, I agree that we messed this one, as the defined
name doesn't mention 8 bars...
I would, instead, do something like:
-#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_N_COLOUR_BARS "Vertical Colour Bars"
+
+ /* Kept for backward-compatibility */
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS
V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS
And, of course, update the documentation accordingly.
> #define V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With
> Fade to Grey"
> #define V4L2_TEST_PATTERN_PN9"Pseudorandom Sequence
> (PN9)"
> #define V4L2_CID_DEINTERLACING_MODE (V4L2_CID_IMAGE_PROC_CLASS_BASE
> + 4)
Thanks,
Mauro
[PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic
The test pattern could contain a different number of colour bars than
eight, make the entry more useful by removing "Eight " from the name.
Signed-off-by: Sakari Ailus
---
include/uapi/linux/v4l2-controls.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/v4l2-controls.h
b/include/uapi/linux/v4l2-controls.h
index acb2a57fa5d6..88f2759c2ce4 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
#define V4L2_CID_TEST_PATTERN (V4L2_CID_IMAGE_PROC_CLASS_BASE
+ 3)
#define V4L2_TEST_PATTERN_DISABLED "Disabled"
#define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
-#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Vertical Colour Bars"
#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With Fade
to Grey"
#define V4L2_TEST_PATTERN_PN9 "Pseudorandom Sequence (PN9)"
#define V4L2_CID_DEINTERLACING_MODE(V4L2_CID_IMAGE_PROC_CLASS_BASE
+ 4)
--
2.11.0
Re: [PATCH 1/1] media: Use common test pattern menu entries
On Tue, Nov 27, 2018 at 07:19:52PM +0800, Bingbu Cao wrote:
>
> On 11/27/2018 05:33 PM, Sakari Ailus wrote:
> > diff --git a/include/uapi/linux/v4l2-controls.h
> > b/include/uapi/linux/v4l2-controls.h
> > index 998983a6e6b7..a74ff6f1ac88 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
> > #define V4L2_CID_LINK_FREQ
> > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
> > #define V4L2_CID_PIXEL_RATE
> > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
> > #define V4L2_CID_TEST_PATTERN
> > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> > +#define V4L2_TEST_PATTERN_DISABLED "Disabled"
> > +#define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
> > +#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS "Eight Vertical
> > Colour Bars"
> > +#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars
> > With Fade to Grey"
> > +#define V4L2_TEST_PATTERN_PN9 "Pseudorandom Sequence
> > (PN9)"
> More padding here for alignment?
Fixed in v2.
--
Sakari Ailus
[PATCH v2 1/1] media: Use common test pattern menu entries
While the test pattern menu itself is not standardised, many devices
support the same test patterns. Aligning the menu entries helps the user
space to use the interface, and adding macros for the menu entry strings
helps to keep them aligned.
Signed-off-by: Sakari Ailus
---
since v1:
- Fix indentation of menu strings
- Remove "8" from the macro names
drivers/media/i2c/imx258.c | 10 +-
drivers/media/i2c/imx319.c | 10 +-
drivers/media/i2c/imx355.c | 10 +-
drivers/media/i2c/ov2640.c | 4 ++--
drivers/media/i2c/smiapp/smiapp-core.c | 10 +-
include/uapi/linux/v4l2-controls.h | 5 +
6 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index f86ae18bc104..df5f016cebd9 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -498,11 +498,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
};
static const char * const imx258_test_pattern_menu[] = {
- "Disabled",
- "Solid Colour",
- "Eight Vertical Colour Bars",
- "Colour Bars With Fade to Grey",
- "Pseudorandom Sequence (PN9)",
+ V4L2_TEST_PATTERN_DISABLED,
+ V4L2_TEST_PATTERN_SOLID_COLOUR,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
+ V4L2_TEST_PATTERN_PN9,
};
/* Configurations for supported link frequencies */
diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
index 17c2e4b41221..d9d4176b9d37 100644
--- a/drivers/media/i2c/imx319.c
+++ b/drivers/media/i2c/imx319.c
@@ -1647,11 +1647,11 @@ static const struct imx319_reg mode_1280x720_regs[] = {
};
static const char * const imx319_test_pattern_menu[] = {
- "Disabled",
- "Solid Colour",
- "Eight Vertical Colour Bars",
- "Colour Bars With Fade to Grey",
- "Pseudorandom Sequence (PN9)",
+ V4L2_TEST_PATTERN_DISABLED,
+ V4L2_TEST_PATTERN_SOLID_COLOUR,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
+ V4L2_TEST_PATTERN_PN9,
};
/* supported link frequencies */
diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index bed293b60e50..99138a291cb8 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -875,11 +875,11 @@ static const struct imx355_reg mode_820x616_regs[] = {
};
static const char * const imx355_test_pattern_menu[] = {
- "Disabled",
- "Solid Colour",
- "Eight Vertical Colour Bars",
- "Colour Bars With Fade to Grey",
- "Pseudorandom Sequence (PN9)",
+ V4L2_TEST_PATTERN_DISABLED,
+ V4L2_TEST_PATTERN_SOLID_COLOUR,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
+ V4L2_TEST_PATTERN_PN9,
};
/* supported link frequencies */
diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 5d2d6735cc78..65058d9a5d51 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -707,8 +707,8 @@ static int ov2640_reset(struct i2c_client *client)
}
static const char * const ov2640_test_pattern_menu[] = {
- "Disabled",
- "Eight Vertical Colour Bars",
+ V4L2_TEST_PATTERN_DISABLED,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
};
/*
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
b/drivers/media/i2c/smiapp/smiapp-core.c
index 58a45c353e27..5c9bcc9438ec 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -409,11 +409,11 @@ static void smiapp_update_mbus_formats(struct
smiapp_sensor *sensor)
}
static const char * const smiapp_test_patterns[] = {
- "Disabled",
- "Solid Colour",
- "Eight Vertical Colour Bars",
- "Colour Bars With Fade to Grey",
- "Pseudorandom Sequence (PN9)",
+ V4L2_TEST_PATTERN_DISABLED,
+ V4L2_TEST_PATTERN_SOLID_COLOUR,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS,
+ V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY,
+ V4L2_TEST_PATTERN_PN9,
};
static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/include/uapi/linux/v4l2-controls.h
b/include/uapi/linux/v4l2-controls.h
index 998983a6e6b7..acb2a57fa5d6 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
#define V4L2_CID_LINK_FREQ (V4L2_CID_IMAGE_PROC_CLASS_BASE
+ 1)
#define V4L2_CID_PIXEL_RATE(V4L2_CID_IMAGE_PROC_CLASS_BASE
+ 2)
#define V4L2_CID_TEST_PATTERN (V4L2_CID_IMAGE_PROC_CLASS_BASE
+ 3)
+#define V4L2_TEST_PATTERN_DISABLED "Disabled"
+#define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With
[GIT PULL FOR v4.21] uvcvideo updates, part 2
Hi Mauro,
The following changes since commit 9b90dc85c718443a3e573a0ccf55900ff4fa73ae:
media: seco-cec: add missing header file to fix build (2018-12-03 15:11:00
-0500)
are available in the Git repository at:
git://linuxtv.org/pinchartl/media.git uvc/async
for you to fetch changes up to e1b1c84504a76ea48ac0f459c5efb64935dc3dde:
media: uvcvideo: Utilise for_each_uvc_urb iterator (2018-12-04 15:11:15
+0200)
Kieran Bingham (10):
media: uvcvideo: Refactor URB descriptors
media: uvcvideo: Convert decode functions to use new context structure
media: uvcvideo: Protect queue internals with helper
media: uvcvideo: queue: Simplify spin-lock usage
media: uvcvideo: queue: Support asynchronous buffer handling
media: uvcvideo: Abstract streaming object lifetime
media: uvcvideo: Move decode processing to process context
media: uvcvideo: Split uvc_video_enable into two
media: uvcvideo: Rename uvc_{un,}init_video()
media: uvcvideo: Utilise for_each_uvc_urb iterator
drivers/media/usb/uvc/uvc_driver.c | 65 ---
drivers/media/usb/uvc/uvc_isight.c | 6 +-
drivers/media/usb/uvc/uvc_queue.c | 110 ++
drivers/media/usb/uvc/uvc_video.c | 274 ++-
drivers/media/usb/uvc/uvcvideo.h | 65 +--
5 files changed, 369 insertions(+), 151 deletions(-)
--
Regards,
Laurent Pinchart
Re: [PATCH v2] v4l2-ioctl: Zero v4l2_plane_pix_format reserved fields
On Tue, Nov 27, 2018 at 07:07:56PM -0300, Ezequiel Garcia wrote:
> Make the core set the reserved fields to zero in
> vv4l2_pix_format_mplane.4l2_plane_pix_format,
> for _MPLANE queue types.
>
> Moving this to the core avoids having to do so in each
> and every driver.
>
> Suggested-by: Tomasz Figa
> Signed-off-by: Ezequiel Garcia
> --
> v2:
> * Drop unneeded clear in g_fmt.
> The sturct was already being cleared here.
> * Only zero plane_fmt reserved fields.
> * Use CLEAR_FIELD_MACRO.
>
> drivers/media/v4l2-core/v4l2-ioctl.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index e384142d2826..2506b602481f 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1512,6 +1512,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> struct v4l2_format *p = arg;
> struct video_device *vfd = video_devdata(file);
> int ret = check_fmt(file, p->type);
> + int i;
unsigned int; same below.
With that,
Acked-by: Sakari Ailus
>
> if (ret)
> return ret;
> @@ -1536,6 +1537,8 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_s_fmt_vid_cap_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> + for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> + CLEAR_AFTER_FIELD(p,
> fmt.pix_mp.plane_fmt[i].bytesperline);
> return ops->vidioc_s_fmt_vid_cap_mplane(file, fh, arg);
> case V4L2_BUF_TYPE_VIDEO_OVERLAY:
> if (unlikely(!ops->vidioc_s_fmt_vid_overlay))
> @@ -1564,6 +1567,8 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_s_fmt_vid_out_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> + for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> + CLEAR_AFTER_FIELD(p,
> fmt.pix_mp.plane_fmt[i].bytesperline);
> return ops->vidioc_s_fmt_vid_out_mplane(file, fh, arg);
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
> if (unlikely(!ops->vidioc_s_fmt_vid_out_overlay))
> @@ -1604,6 +1609,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
> {
> struct v4l2_format *p = arg;
> int ret = check_fmt(file, p->type);
> + int i;
>
> if (ret)
> return ret;
> @@ -1623,6 +1629,8 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_try_fmt_vid_cap_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> + for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> + CLEAR_AFTER_FIELD(p,
> fmt.pix_mp.plane_fmt[i].bytesperline);
> return ops->vidioc_try_fmt_vid_cap_mplane(file, fh, arg);
> case V4L2_BUF_TYPE_VIDEO_OVERLAY:
> if (unlikely(!ops->vidioc_try_fmt_vid_overlay))
> @@ -1651,6 +1659,8 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_try_fmt_vid_out_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> + for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> + CLEAR_AFTER_FIELD(p,
> fmt.pix_mp.plane_fmt[i].bytesperline);
> return ops->vidioc_try_fmt_vid_out_mplane(file, fh, arg);
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
> if (unlikely(!ops->vidioc_try_fmt_vid_out_overlay))
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v6 09/10] media: uvcvideo: Rename uvc_{un,}init_video()
Hi Kieran,
Thank you for the patch.
On Friday, 9 November 2018 19:15:42 EET Kieran Bingham wrote:
> Hi Laurent,
>
> I'm sorry - I didn't update the commit message on this one.
>
> On 09/11/2018 17:05, Kieran Bingham wrote:
> > We have both uvc_init_video() and uvc_video_init() calls which can be
> > quite confusing to determine the process for each. Now that video
> > uvc_video_enable() has been renamed to uvc_video_start_streaming(),
> > adapt these calls to suit the new flow.
> >
> > Rename uvc_init_video() to uvc_video_start() and uvc_uninit_video() to
> > uvc_video_stop().
>
> Could you s/uvc_video_start/uvc_video_start_transfer/ and
> s/uvc_video_stop/uvc_video_stop_transfer/ when applying please?
>
> (Assuming you get to apply and don't find something else :D)
Will do.
Reviewed-by: Laurent Pinchart
> > Signed-off-by: Kieran Bingham
> >
> > ---
> >
> > v6:
> > - Append _transfer to {_stop,_start}
> >
> > drivers/media/usb/uvc/uvc_video.c | 20 +++-
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > b/drivers/media/usb/uvc/uvc_video.c index cd67506dc696..a81012c65280
> > 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1641,7 +1641,8 @@ static int uvc_alloc_urb_buffers(struct
> > uvc_streaming *stream,
> > /*
> > * Uninitialize isochronous/bulk URBs and free transfer buffers.
> > */
> > -static void uvc_uninit_video(struct uvc_streaming *stream, int
> > free_buffers)
> > +static void uvc_video_stop_transfer(struct uvc_streaming *stream,
> > + int free_buffers)
> > {
> > struct uvc_urb *uvc_urb;
> >
> > @@ -1718,7 +1719,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
> > *stream,
> >
> > urb = usb_alloc_urb(npackets, gfp_flags);
> > if (urb == NULL) {
> > - uvc_uninit_video(stream, 1);
> > + uvc_video_stop_transfer(stream, 1);
> > return -ENOMEM;
> > }
> >
> > @@ -1786,7 +1787,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
> > *stream,
> >
> > urb = usb_alloc_urb(0, gfp_flags);
> > if (urb == NULL) {
> > - uvc_uninit_video(stream, 1);
> > + uvc_video_stop_transfer(stream, 1);
> > return -ENOMEM;
> > }
> >
> > @@ -1806,7 +1807,8 @@ static int uvc_init_video_bulk(struct uvc_streaming
> > *stream,
> > /*
> > * Initialize isochronous/bulk URBs and allocate transfer buffers.
> > */
> > -static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
> > +static int uvc_video_start_transfer(struct uvc_streaming *stream,
> > + gfp_t gfp_flags)
> {
> > struct usb_interface *intf = stream->intf;
> > struct usb_host_endpoint *ep;
> > @@ -1894,7 +1896,7 @@ static int uvc_init_video(struct uvc_streaming
> > *stream, gfp_t gfp_flags)
> >
> > if (ret < 0) {
> > uvc_printk(KERN_ERR, "Failed to submit URB %u "
> > "(%d).\n", i, ret);
> > - uvc_uninit_video(stream, 1);
> > + uvc_video_stop_transfer(stream, 1);
> > return ret;
> }
> > }
> > @@ -1925,7 +1927,7 @@ int uvc_video_suspend(struct uvc_streaming *stream)
> > return 0;
> >
> > stream->frozen = 1;
> > - uvc_uninit_video(stream, 0);
> > + uvc_video_stop_transfer(stream, 0);
> > usb_set_interface(stream->dev->udev, stream->intfnum, 0);
> > return 0;
> > }
> > @@ -1961,7 +1963,7 @@ int uvc_video_resume(struct uvc_streaming *stream,
> > int reset)
> >
> > if (ret < 0)
> > return ret;
> >
> > - return uvc_init_video(stream, GFP_NOIO);
> > + return uvc_video_start_transfer(stream, GFP_NOIO);
> > }
> >
> > /* --
> > @@ -2089,7 +2091,7 @@ int uvc_video_start_streaming(struct uvc_streaming
> > *stream)
> >
> > if (ret < 0)
> > goto error_commit;
> >
> > - ret = uvc_init_video(stream, GFP_KERNEL);
> > + ret = uvc_video_start_transfer(stream, GFP_KERNEL);
> > if (ret < 0)
> > goto error_video;
> >
> > @@ -2105,7 +2107,7 @@ int uvc_video_start_streaming(struct uvc_streaming
> > *stream)
> >
> > int uvc_video_stop_streaming(struct uvc_streaming *stream)
> > {
> > - uvc_uninit_video(stream, 1);
> > + uvc_video_stop_transfer(stream, 1);
> >
> > if (stream->intf->num_altsetting > 1) {
> > usb_set_interface(stream->dev->udev,
> > stream->intfnum, 0);
--
Regards,
Laurent Pinchart
Re: [PATCH v6 08/10] media: uvcvideo: Split uvc_video_enable into two
Hi Kieran,
On Sunday, 11 November 2018 00:02:39 EET Kieran Bingham wrote:
> Hi Laurent,
>
> I see that you made changes to this patch before accepting it last time.
>
> I'm afraid I haven't made those changes here, so please just cherry-pick
> your previous incarnation. There are no changes here from me between v5,
> and v6.
OK, I will do so. The two changes I made are both in
uvc_video_stop_streaming(), turning the function into a void function and
avoiding an unnecessarily line wrap.
> On 09/11/2018 17:05, Kieran Bingham wrote:
> > uvc_video_enable() is used both to start and stop the video stream
> > object, however the single function entry point shares no code between
> > the two operations.
> >
> > Split the function into two distinct calls, and rename to
> > uvc_video_start_streaming() and uvc_video_stop_streaming() as
> > appropriate.
> >
> > Signed-off-by: Kieran Bingham
> > ---
> >
> > drivers/media/usb/uvc/uvc_queue.c | 4 +-
> > drivers/media/usb/uvc/uvc_video.c | 56 +++-
> > drivers/media/usb/uvc/uvcvideo.h | 3 +-
> > 3 files changed, 31 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > b/drivers/media/usb/uvc/uvc_queue.c index 2752e386f1e8..d09b0c882938
> > 100644
> > --- a/drivers/media/usb/uvc/uvc_queue.c
> > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > @@ -176,7 +176,7 @@ static int uvc_start_streaming(struct vb2_queue *vq,
> > unsigned int count)>
> > queue->buf_used = 0;
> >
> > - ret = uvc_video_enable(stream, 1);
> > + ret = uvc_video_start_streaming(stream);
> >
> > if (ret == 0)
> >
> > return 0;
> >
> > @@ -194,7 +194,7 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
> >
> > lockdep_assert_irqs_enabled();
> >
> > if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
> >
> > - uvc_video_enable(uvc_queue_to_stream(queue), 0);
> > + uvc_video_stop_streaming(uvc_queue_to_stream(queue));
> >
> > spin_lock_irq(>irqlock);
> > uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > b/drivers/media/usb/uvc/uvc_video.c index e19bdf089cc4..cd67506dc696
> > 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -2076,38 +2076,10 @@ int uvc_video_init(struct uvc_streaming *stream)
> >
> > return 0;
> >
> > }
> >
> > -/*
> > - * Enable or disable the video stream.
> > - */
> > -int uvc_video_enable(struct uvc_streaming *stream, int enable)
> > +int uvc_video_start_streaming(struct uvc_streaming *stream)
> >
> > {
> >
> > int ret;
> >
> > - if (!enable) {
> > - uvc_uninit_video(stream, 1);
> > - if (stream->intf->num_altsetting > 1) {
> > - usb_set_interface(stream->dev->udev,
> > - stream->intfnum, 0);
> > - } else {
> > - /* UVC doesn't specify how to inform a bulk-based device
> > -* when the video stream is stopped. Windows sends a
> > -* CLEAR_FEATURE(HALT) request to the video streaming
> > -* bulk endpoint, mimic the same behaviour.
> > -*/
> > - unsigned int epnum = stream->header.bEndpointAddress
> > - & USB_ENDPOINT_NUMBER_MASK;
> > - unsigned int dir = stream->header.bEndpointAddress
> > -& USB_ENDPOINT_DIR_MASK;
> > - unsigned int pipe;
> > -
> > - pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
> > - usb_clear_halt(stream->dev->udev, pipe);
> > - }
> > -
> > - uvc_video_clock_cleanup(stream);
> > - return 0;
> > - }
> > -
> >
> > ret = uvc_video_clock_init(stream);
> > if (ret < 0)
> >
> > return ret;
> >
> > @@ -2130,3 +2102,29 @@ int uvc_video_enable(struct uvc_streaming *stream,
> > int enable)>
> > return ret;
> >
> > }
> >
> > +
> > +int uvc_video_stop_streaming(struct uvc_streaming *stream)
> > +{
> > + uvc_uninit_video(stream, 1);
> > + if (stream->intf->num_altsetting > 1) {
> > + usb_set_interface(stream->dev->udev,
> > + stream->intfnum, 0);
> > + } else {
> > + /* UVC doesn't specify how to inform a bulk-based device
> > +* when the video stream is stopped. Windows sends a
> > +* CLEAR_FEATURE(HALT) request to the video streaming
> > +* bulk endpoint, mimic the same behaviour.
> > +*/
> > + unsigned int epnum = stream->header.bEndpointAddress
> > + & USB_ENDPOINT_NUMBER_MASK;
> > + unsigned int dir = stream->header.bEndpointAddress
> > +& USB_ENDPOINT_DIR_MASK;
> > + unsigned int pipe;
> > +
> >
Re: [PATCH v6 07/10] media: uvcvideo: Move decode processing to process context
Hi Kieran,
Thank you for the patch.
On Friday, 9 November 2018 19:05:30 EET Kieran Bingham wrote:
> Newer high definition cameras, and cameras with multiple lenses such as
> the range of stereo-vision cameras now available have ever increasing
> data rates.
>
> The inclusion of a variable length packet header in URB packets mean
> that we must memcpy the frame data out to our destination 'manually'.
> This can result in data rates of up to 2 gigabits per second being
> processed.
>
> To improve efficiency, and maximise throughput, handle the URB decode
> processing through a work queue to move it from interrupt context, and
> allow multiple processors to work on URBs in parallel.
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
>
> ---
> v2:
> - Lock full critical section of usb_submit_urb()
>
> v3:
> - Fix race on submitting uvc_video_decode_data_work() to work queue.
> - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode)
> - Rename decodes -> copy_operations
> - Don't queue work if there is no async task
> - obtain copy op structure directly in uvc_video_decode_data()
> - uvc_video_decode_data_work() -> uvc_video_copy_data_work()
>
> v4:
> - Provide for_each_uvc_urb()
> - Simplify fix for shutdown race to flush queue before freeing URBs
> - Rebase to v4.16-rc4 (linux-media/master) adjusting for metadata
>conflicts.
>
> v5:
> - Rebase to media/v4.20-2
> - Use GFP_KERNEL allocation in uvc_video_copy_data_work()
> - Fix function documentation for uvc_video_copy_data_work()
> - Add periods to the end of sentences
> - Rename 'decode' variable to 'op' in uvc_video_decode_data()
> - Move uvc_urb->async_operations initialisation to before use
> - Move async workqueue to match uvc_streaming lifetime instead of
>streamon/streamoff
>
> v6:
> - Utilise the new streaming object lifetime functions to perform
>allocation and destruction of the async workqueue.
>
> drivers/media/usb/uvc/uvc_driver.c | 11 +++-
> drivers/media/usb/uvc/uvc_video.c | 104 +++---
> drivers/media/usb/uvc/uvcvideo.h | 28 -
> 3 files changed, 119 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index afb44d1c9d04..b62cbd800111
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -401,6 +401,9 @@ static struct uvc_streaming *uvc_stream_by_id(struct
> uvc_device *dev, int id)
>
> static void uvc_stream_delete(struct uvc_streaming *stream)
> {
> + if (stream->async_wq)
> + destroy_workqueue(stream->async_wq);
> +
> mutex_destroy(>mutex);
>
> usb_put_intf(stream->intf);
> @@ -425,6 +428,14 @@ static struct uvc_streaming *uvc_stream_new(struct
> uvc_device *dev, stream->intf = usb_get_intf(intf);
> stream->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
>
> + /* Allocate a stream specific work queue for asynchronous tasks. */
> + stream->async_wq = alloc_workqueue("uvcvideo", WQ_UNBOUND | WQ_HIGHPRI,
> +0);
> + if (!stream->async_wq) {
> + uvc_stream_delete(stream);
> + return NULL;
> + }
> +
> return stream;
> }
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 7a7779e1b466..e19bdf089cc4 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1094,21 +1094,54 @@ static int uvc_video_decode_start(struct
> uvc_streaming *stream, return data[0];
> }
>
> -static void uvc_video_decode_data(struct uvc_streaming *stream,
> +/*
> + * uvc_video_decode_data_work: Asynchronous memcpy processing
> + *
> + * Copy URB data to video buffers in process context, releasing buffer
> + * references and requeuing the URB when done.
> + */
> +static void uvc_video_copy_data_work(struct work_struct *work)
> +{
> + struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < uvc_urb->async_operations; i++) {
> + struct uvc_copy_op *op = _urb->copy_operations[i];
> +
> + memcpy(op->dst, op->src, op->len);
> +
> + /* Release reference taken on this buffer. */
> + uvc_queue_buffer_release(op->buf);
> + }
> +
> + ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
> + if (ret < 0)
> + uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
> +ret);
> +}
> +
> +static void uvc_video_decode_data(struct uvc_urb *uvc_urb,
> struct uvc_buffer *buf, const u8 *data, int len)
> {
> - unsigned int maxlen, nbytes;
> - void *mem;
> + unsigned int active_op = uvc_urb->async_operations;
> + struct uvc_copy_op *op = _urb->copy_operations[active_op];
> + unsigned int maxlen;
>
> if (len <= 0)
> return;
>
> -
Re: [PATCH v6 06/10] media: uvcvideo: Abstract streaming object lifetime
Hi Kieran,
Thank you for the patch.
On Friday, 9 November 2018 19:05:29 EET Kieran Bingham wrote:
> The streaming object is a key part of handling the UVC device. Although
> not critical, we are currently missing a call to destroy the mutex on
> clean up paths, and we are due to extend the objects complexity in the
> near future.
>
> Facilitate easy management of a stream object by creating a pair of
> functions to handle creating and destroying the allocation. The new
> uvc_stream_delete() function also performs the missing mutex_destroy()
> operation.
>
> Previously a failed streaming object allocation would cause
> uvc_parse_streaming() to return -EINVAL, which is inappropriate. If the
> constructor failes, we will instead return -ENOMEM.
>
> While we're here, fix the trivial spelling error in the function banner
> of uvc_delete().
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/usb/uvc/uvc_driver.c | 54 +--
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 67bd58c6f397..afb44d1c9d04
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -396,6 +396,39 @@ static struct uvc_streaming *uvc_stream_by_id(struct
> uvc_device *dev, int id) }
>
> /*
> + * Streaming Object Management
> + */
> +
> +static void uvc_stream_delete(struct uvc_streaming *stream)
> +{
> + mutex_destroy(>mutex);
> +
> + usb_put_intf(stream->intf);
> +
> + kfree(stream->format);
> + kfree(stream->header.bmaControls);
> + kfree(stream);
> +}
> +
> +static struct uvc_streaming *uvc_stream_new(struct uvc_device *dev,
> + struct usb_interface *intf)
> +{
> + struct uvc_streaming *stream;
> +
> + stream = kzalloc(sizeof(*stream), GFP_KERNEL);
> + if (stream == NULL)
> + return NULL;
> +
> + mutex_init(>mutex);
> +
> + stream->dev = dev;
> + stream->intf = usb_get_intf(intf);
> + stream->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +
> + return stream;
> +}
> +
> +/*
> * Descriptors parsing
> */
>
> @@ -687,17 +720,12 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> return -EINVAL;
> }
>
> - streaming = kzalloc(sizeof(*streaming), GFP_KERNEL);
> + streaming = uvc_stream_new(dev, intf);
> if (streaming == NULL) {
> usb_driver_release_interface(_driver.driver, intf);
> - return -EINVAL;
> + return -ENOMEM;
> }
>
> - mutex_init(>mutex);
> - streaming->dev = dev;
> - streaming->intf = usb_get_intf(intf);
> - streaming->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> -
> /* The Pico iMage webcam has its class-specific interface descriptors
>* after the endpoint descriptors.
>*/
> @@ -904,10 +932,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
>
> error:
> usb_driver_release_interface(_driver.driver, intf);
> - usb_put_intf(intf);
> - kfree(streaming->format);
> - kfree(streaming->header.bmaControls);
> - kfree(streaming);
> + uvc_stream_delete(streaming);
> return ret;
> }
>
> @@ -1815,7 +1840,7 @@ static int uvc_scan_device(struct uvc_device *dev)
> * is released.
> *
> * As this function is called after or during disconnect(), all URBs have
> - * already been canceled by the USB core. There is no need to kill the
> + * already been cancelled by the USB core. There is no need to kill the
> * interrupt URB manually.
> */
> static void uvc_delete(struct kref *kref)
> @@ -1853,10 +1878,7 @@ static void uvc_delete(struct kref *kref)
> streaming = list_entry(p, struct uvc_streaming, list);
> usb_driver_release_interface(_driver.driver,
> streaming->intf);
> - usb_put_intf(streaming->intf);
> - kfree(streaming->format);
> - kfree(streaming->header.bmaControls);
> - kfree(streaming);
> + uvc_stream_delete(streaming);
> }
>
> kfree(dev);
--
Regards,
Laurent Pinchart
Re: [PATCH dvb v1 3/4] media: dvb-usb-v2: remove __func__ from dev_dbg()
Hi Sean,
Thanks for taking time to review those patches.
On Tue, Nov 27, 2018 at 10:32:44AM +, Sean Young wrote:
> On Tue, Oct 30, 2018 at 05:14:50PM +0100, Victor Toso wrote:
> > From: Victor Toso
> >
> > As dynamic debug can be instructed to add the function name to the
> > debug output using +f switch, we can remove __func__ from all
> > dev_dbg() calls. If not, a user that sets +f in dynamic debug would
> > get duplicated function name.
> >
> > Signed-off-by: Victor Toso
> > ---
> > drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 105 ++--
> > drivers/media/usb/dvb-usb-v2/dvb_usb_urb.c | 7 +-
> > 2 files changed, 55 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> > b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> > index d55ef016d418..ad554668cc86 100644
> > --- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> > +++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> > @@ -37,7 +37,7 @@ static int dvb_usbv2_download_firmware(struct
> > dvb_usb_device *d,
> > {
> > int ret;
> > const struct firmware *fw;
> > - dev_dbg(>udev->dev, "%s:\n", __func__);
> > + dev_dbg(>udev->dev, "\n");
>
> How about "downloading firmware", or maybe deleting the line
> completely?
Ok
> Without dynamic debug enabled, you end up with a pretty useless
> debug message now. I think it would be better to convert these
> debug lines to useful messages, rather than "executing this
> line of code". Some of them should probably be deleted.
Yes, some of those debug lines are not useful without dynamic
debug. Some messages can be improved.
> > if (!d->props->download_firmware) {
> > ret = -EINVAL;
> > @@ -62,14 +62,14 @@ static int dvb_usbv2_download_firmware(struct
> > dvb_usb_device *d,
> >
> > return ret;
> > err:
> > - dev_dbg(>udev->dev, "%s: failed=%d\n", __func__, ret);
> > + dev_dbg(>udev->dev, "failed=%d\n", ret);
>
> Again, just say what failed here. Ideally debug messages should
> be useful and not just "hit this line of code".
>
> Sean
I'll go over all of them again and send a new version.
Cheers,
Victor
> > return ret;
> > }
> >
> > static int dvb_usbv2_i2c_init(struct dvb_usb_device *d)
> > {
> > int ret;
> > - dev_dbg(>udev->dev, "%s:\n", __func__);
> > + dev_dbg(>udev->dev, "\n");
> >
> > if (!d->props->i2c_algo)
> > return 0;
> > @@ -87,13 +87,13 @@ static int dvb_usbv2_i2c_init(struct dvb_usb_device *d)
> >
> > return 0;
> > err:
> > - dev_dbg(>udev->dev, "%s: failed=%d\n", __func__, ret);
> > + dev_dbg(>udev->dev, "failed=%d\n", ret);
> > return ret;
> > }
> >
> > static int dvb_usbv2_i2c_exit(struct dvb_usb_device *d)
> > {
> > - dev_dbg(>udev->dev, "%s:\n", __func__);
> > + dev_dbg(>udev->dev, "\n");
> >
> > if (d->i2c_adap.algo)
> > i2c_del_adapter(>i2c_adap);
> > @@ -133,7 +133,7 @@ static int dvb_usbv2_remote_init(struct dvb_usb_device
> > *d)
> > {
> > int ret;
> > struct rc_dev *dev;
> > - dev_dbg(>udev->dev, "%s:\n", __func__);
> > + dev_dbg(>udev->dev, "\n");
> >
> > if (dvb_usbv2_disable_rc_polling || !d->props->get_rc_config)
> > return 0;
> > @@ -188,13 +188,13 @@ static int dvb_usbv2_remote_init(struct
> > dvb_usb_device *d)
> >
> > return 0;
> > err:
> > - dev_dbg(>udev->dev, "%s: failed=%d\n", __func__, ret);
> > + dev_dbg(>udev->dev, "failed=%d\n", ret);
> > return ret;
> > }
> >
> > static int dvb_usbv2_remote_exit(struct dvb_usb_device *d)
> > {
> > - dev_dbg(>udev->dev, "%s:\n", __func__);
> > + dev_dbg(>udev->dev, "\n");
> >
> > if (d->rc_dev) {
> > cancel_delayed_work_sync(>rc_query_work);
> > @@ -232,7 +232,7 @@ static void dvb_usb_data_complete_raw(struct
> > usb_data_stream *stream, u8 *buf,
> >
> > static int dvb_usbv2_adapter_stream_init(struct dvb_usb_adapter *adap)
> > {
> > - dev_dbg(_to_d(adap)->udev->dev, "%s: adap=%d\n", __func__,
> > + dev_dbg(_to_d(adap)->udev->dev, "adap=%d\n",
> > adap->id);
> >
> > adap->stream.udev = adap_to_d(adap)->udev;
> > @@ -244,7 +244,7 @@ static int dvb_usbv2_adapter_stream_init(struct
> > dvb_usb_adapter *adap)
> >
> > static int dvb_usbv2_adapter_stream_exit(struct dvb_usb_adapter *adap)
> > {
> > - dev_dbg(_to_d(adap)->udev->dev, "%s: adap=%d\n", __func__,
> > + dev_dbg(_to_d(adap)->udev->dev, "adap=%d\n",
> > adap->id);
> >
> > return usb_urb_exitv2(>stream);
> > @@ -257,8 +257,8 @@ static int dvb_usb_start_feed(struct dvb_demux_feed
> > *dvbdmxfeed)
> > int ret = 0;
> > struct usb_data_stream_properties stream_props;
> > dev_dbg(>udev->dev,
> > - "%s: adap=%d active_fe=%d feed_type=%d setting pid
> > [%s]: %04x (%04d) at index %d\n",
> > - __func__, adap->id, adap->active_fe, dvbdmxfeed->type,
> > + "adap=%d active_fe=%d feed_type=%d
