Re: [RFCv11 PATCH 03/29] media-request: allocate media requests
Hi Hans, On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuilwrote: [snip] > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > new file mode 100644 > index ..ead78613fdbe > --- /dev/null > +++ b/drivers/media/media-request.c > @@ -0,0 +1,23 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Media device request objects > + * > + * Copyright (C) 2018 Intel Corporation > + * Copyright (C) 2018, The Chromium OS Authors. All rights reserved. I'm not sure about the origin of this line, but it's not a correct copyright for kernel code produced as a part of Chrome OS project. It would normally be something like Copyright (C) 2018 Google, Inc. > + * > + * Author: Sakari Ailus > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +int media_request_alloc(struct media_device *mdev, > + struct media_request_alloc *alloc) > +{ > + return -ENOMEM; > +} > diff --git a/include/media/media-device.h b/include/media/media-device.h > index bcc6ec434f1f..07e323c57202 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -19,6 +19,7 @@ > #ifndef _MEDIA_DEVICE_H > #define _MEDIA_DEVICE_H > +#include What is the need for anon_inodes in this header? > #include > #include > @@ -27,6 +28,7 @@ > struct ida; > struct device; > +struct media_device; > /** >* struct media_entity_notify - Media Entity Notify > @@ -50,10 +52,16 @@ struct media_entity_notify { >* struct media_device_ops - Media device operations >* @link_notify: Link state change notification callback. This callback is >* called with the graph_mutex held. > + * @req_alloc: Allocate a request > + * @req_free: Free a request > + * @req_queue: Queue a request >*/ > struct media_device_ops { > int (*link_notify)(struct media_link *link, u32 flags, > unsigned int notification); > + struct media_request *(*req_alloc)(struct media_device *mdev); > + void (*req_free)(struct media_request *req); > + int (*req_queue)(struct media_request *req); > }; > /** > @@ -88,6 +96,8 @@ struct media_device_ops { >* @disable_source: Disable Source Handler function pointer >* >* @ops: Operation handler callbacks > + * @req_lock: Serialise access to requests > + * @req_queue_mutex: Serialise validating and queueing requests Let's bikeshed a bit! "access" sounds like a superset of "validating and queuing" to me. Perhaps it could make sense to be a bit more specific on what type of access the spinlock is used for? Best regards, Tomasz
Re: [RFCv11 PATCH 02/29] uapi/linux/media.h: add request API
Hi Hans, On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuilwrote: > From: Hans Verkuil > Define the public request API. > This adds the new MEDIA_IOC_REQUEST_ALLOC ioctl to allocate a request > and two ioctls that operate on a request in order to queue the > contents of the request to the driver and to re-initialize the > request. > Signed-off-by: Hans Verkuil > --- > include/uapi/linux/media.h | 8 > 1 file changed, 8 insertions(+) > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index c7e9a5cba24e..f8769e74f847 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -342,11 +342,19 @@ struct media_v2_topology { > /* ioctls */ > +struct __attribute__ ((packed)) media_request_alloc { > + __s32 fd; > +}; > + > #define MEDIA_IOC_DEVICE_INFO _IOWR('|', 0x00, struct media_device_info) > #define MEDIA_IOC_ENUM_ENTITIES_IOWR('|', 0x01, struct media_entity_desc) > #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum) > #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc) > #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology) > +#define MEDIA_IOC_REQUEST_ALLOC_IOWR('|', 0x05, struct media_request_alloc) > + > +#define MEDIA_REQUEST_IOC_QUEUE_IO('|', 0x80) > +#define MEDIA_REQUEST_IOC_REINIT _IO('|', 0x81) I wonder if it wouldn't make sense to add a comment here saying that these are called on request FD, as opposed to the others above, which are called on the media FD. Best regards, Tomasz
cron job: media_tree daily build: OK
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: Tue Apr 10 05:00:14 CEST 2018 media-tree git hash:17dec0a949153d9ac00760ba2f5b78cb583e995f media_build git hash: 541653bb52fcf7c34b8b83a4c8cc66b09c68ac37 v4l-utils git hash: 47d43b130dc6e9e0edc900759fb37649208371e4 gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: v0.5.2-rc1 smatch version: v0.5.0-4419-g3b5bf5c9 host hardware: x86_64 host os:4.14.0-3-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 linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.14.31-i686: OK linux-4.14.31-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16-i686: OK linux-4.16-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v3] usbtv: Enforce standard for color decoding
Le 10/04/2018 à 01:13, Hugo Grostabussiat a écrit : > if (!ret) { > /* Configure the decoder for the color standard */ > - u16 cfg[][2] = { > + const u16 cfg[][2] = { > { USBTV_BASE + 0x016f, usbtv_norm_to_16f_reg(norm) } > }; > ret = usbtv_set_regs(usbtv, cfg, ARRAY_SIZE(cfg)); This sums up the differences between the v2 and the v3 of this patch
[PATCH v3] usbtv: Enforce standard for color decoding
Depending on the chosen standard, configure the decoder to use the appropriate color encoding standard (PAL-like, NTSC-like or SECAM). Until now, the decoder was not configured for a specific color standard, making it autodetect the color encoding. While this may sound fine, it potentially causes the wrong image tuning parameters to be applied (e.g. tuning parameters for NTSC are applied to a PAL source), and may confuse users about what the actual standard is in use. This commit explicitly configures the color standard the decoder will use, making it visually obvious if a wrong standard was chosen. Signed-off-by: Hugo Grostabussiat--- drivers/media/usb/usbtv/usbtv-video.c | 45 --- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c index 6cad50d1e5f8..767fab1cc5cf 100644 --- a/drivers/media/usb/usbtv/usbtv-video.c +++ b/drivers/media/usb/usbtv/usbtv-video.c @@ -121,6 +121,25 @@ static int usbtv_select_input(struct usbtv *usbtv, int input) return ret; } +static uint16_t usbtv_norm_to_16f_reg(v4l2_std_id norm) +{ + /* NTSC M/M-JP/M-KR */ + if (norm & V4L2_STD_NTSC) + return 0x00b8; + /* PAL BG/DK/H/I */ + if (norm & V4L2_STD_PAL) + return 0x00ee; + /* SECAM B/D/G/H/K/K1/L/Lc */ + if (norm & V4L2_STD_SECAM) + return 0x00ff; + if (norm & V4L2_STD_NTSC_443) + return 0x00a8; + if (norm & (V4L2_STD_PAL_M | V4L2_STD_PAL_60)) + return 0x00bc; + /* Fallback to automatic detection for other standards */ + return 0x; +} + static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm) { int ret; @@ -154,7 +173,7 @@ static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm) { USBTV_BASE + 0x0263, 0x0017 }, { USBTV_BASE + 0x0266, 0x0016 }, { USBTV_BASE + 0x0267, 0x0036 }, - /* Epilog */ + /* End image tuning */ { USBTV_BASE + 0x024e, 0x0002 }, { USBTV_BASE + 0x024f, 0x0002 }, }; @@ -182,7 +201,7 @@ static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm) { USBTV_BASE + 0x0263, 0x001c }, { USBTV_BASE + 0x0266, 0x0011 }, { USBTV_BASE + 0x0267, 0x0005 }, - /* Epilog */ + /* End image tuning */ { USBTV_BASE + 0x024e, 0x0002 }, { USBTV_BASE + 0x024f, 0x0002 }, }; @@ -210,7 +229,7 @@ static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm) { USBTV_BASE + 0x0263, 0x0021 }, { USBTV_BASE + 0x0266, 0x0016 }, { USBTV_BASE + 0x0267, 0x0036 }, - /* Epilog */ + /* End image tuning */ { USBTV_BASE + 0x024e, 0x0002 }, { USBTV_BASE + 0x024f, 0x0002 }, }; @@ -218,12 +237,28 @@ static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm) ret = usbtv_configure_for_norm(usbtv, norm); if (!ret) { - if (norm & V4L2_STD_525_60) + /* Masks for norms using a NTSC or PAL color encoding. */ + static const v4l2_std_id ntsc_mask = + V4L2_STD_NTSC | V4L2_STD_NTSC_443; + static const v4l2_std_id pal_mask = + V4L2_STD_PAL | V4L2_STD_PAL_60 | V4L2_STD_PAL_M; + + if (norm & ntsc_mask) ret = usbtv_set_regs(usbtv, ntsc, ARRAY_SIZE(ntsc)); - else if (norm & V4L2_STD_PAL) + else if (norm & pal_mask) ret = usbtv_set_regs(usbtv, pal, ARRAY_SIZE(pal)); else if (norm & V4L2_STD_SECAM) ret = usbtv_set_regs(usbtv, secam, ARRAY_SIZE(secam)); + else + ret = -EINVAL; + } + + if (!ret) { + /* Configure the decoder for the color standard */ + const u16 cfg[][2] = { + { USBTV_BASE + 0x016f, usbtv_norm_to_16f_reg(norm) } + }; + ret = usbtv_set_regs(usbtv, cfg, ARRAY_SIZE(cfg)); } return ret; -- 2.17.0
Re: [PATCH] [media] v4l2-core: Rename array 'video_driver' to 'video_drivers'
Hi Simon, Thank you for the patch. On Monday, 9 April 2018 22:47:38 EEST Simon Que wrote: > Improves code clarity in two ways: > 1. The plural name makes it more clear that it is an array. > 2. The name of the array is now no longer identical to the struct type > name, so it is easier to find in the code. > > Signed-off-by: Simon QueI like this and agree with the two reasons you have given. Acked-by: Laurent Pinchart > --- > drivers/media/v4l2-core/v4l2-dev.c | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c > b/drivers/media/v4l2-core/v4l2-dev.c index 0301fe426a43..8d5f7cfe1695 > 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -91,7 +91,7 @@ ATTRIBUTE_GROUPS(video_device); > /* > * Active devices > */ > -static struct video_device *video_device[VIDEO_NUM_DEVICES]; > +static struct video_device *video_devices[VIDEO_NUM_DEVICES]; > static DEFINE_MUTEX(videodev_lock); > static DECLARE_BITMAP(devnode_nums[VFL_TYPE_MAX], VIDEO_NUM_DEVICES); > > @@ -173,14 +173,14 @@ static void v4l2_device_release(struct device *cd) > struct v4l2_device *v4l2_dev = vdev->v4l2_dev; > > mutex_lock(_lock); > - if (WARN_ON(video_device[vdev->minor] != vdev)) { > + if (WARN_ON(video_devices[vdev->minor] != vdev)) { > /* should not happen */ > mutex_unlock(_lock); > return; > } > > /* Free up this device for reuse */ > - video_device[vdev->minor] = NULL; > + video_devices[vdev->minor] = NULL; > > /* Delete the cdev on this minor as well */ > cdev_del(vdev->cdev); > @@ -229,7 +229,7 @@ static struct class video_class = { > > struct video_device *video_devdata(struct file *file) > { > - return video_device[iminor(file_inode(file))]; > + return video_devices[iminor(file_inode(file))]; > } > EXPORT_SYMBOL(video_devdata); > > @@ -493,9 +493,9 @@ static int get_index(struct video_device *vdev) > bitmap_zero(used, VIDEO_NUM_DEVICES); > > for (i = 0; i < VIDEO_NUM_DEVICES; i++) { > - if (video_device[i] != NULL && > - video_device[i]->v4l2_dev == vdev->v4l2_dev) { > - set_bit(video_device[i]->index, used); > + if (video_devices[i] != NULL && > + video_devices[i]->v4l2_dev == vdev->v4l2_dev) { > + set_bit(video_devices[i]->index, used); > } > } > > @@ -929,7 +929,7 @@ int __video_register_device(struct video_device *vdev, > /* The device node number and minor numbers are independent, so > we just find the first free minor number. */ > for (i = 0; i < VIDEO_NUM_DEVICES; i++) > - if (video_device[i] == NULL) > + if (video_devices[i] == NULL) > break; > if (i == VIDEO_NUM_DEVICES) { > mutex_unlock(_lock); > @@ -942,9 +942,9 @@ int __video_register_device(struct video_device *vdev, > devnode_set(vdev); > > /* Should not happen since we thought this minor was free */ > - WARN_ON(video_device[vdev->minor] != NULL); > + WARN_ON(video_devices[vdev->minor] != NULL); > vdev->index = get_index(vdev); > - video_device[vdev->minor] = vdev; > + video_devices[vdev->minor] = vdev; > mutex_unlock(_lock); > > if (vdev->ioctl_ops) > @@ -999,7 +999,7 @@ int __video_register_device(struct video_device *vdev, > mutex_lock(_lock); > if (vdev->cdev) > cdev_del(vdev->cdev); > - video_device[vdev->minor] = NULL; > + video_devices[vdev->minor] = NULL; > devnode_clear(vdev); > mutex_unlock(_lock); > /* Mark this video device as never having been registered. */ -- Regards, Laurent Pinchart
[PATCH] [media] v4l2-core: Rename array 'video_driver' to 'video_drivers'
Improves code clarity in two ways: 1. The plural name makes it more clear that it is an array. 2. The name of the array is now no longer identical to the struct type name, so it is easier to find in the code. Signed-off-by: Simon Que--- drivers/media/v4l2-core/v4l2-dev.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index 0301fe426a43..8d5f7cfe1695 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -91,7 +91,7 @@ ATTRIBUTE_GROUPS(video_device); /* * Active devices */ -static struct video_device *video_device[VIDEO_NUM_DEVICES]; +static struct video_device *video_devices[VIDEO_NUM_DEVICES]; static DEFINE_MUTEX(videodev_lock); static DECLARE_BITMAP(devnode_nums[VFL_TYPE_MAX], VIDEO_NUM_DEVICES); @@ -173,14 +173,14 @@ static void v4l2_device_release(struct device *cd) struct v4l2_device *v4l2_dev = vdev->v4l2_dev; mutex_lock(_lock); - if (WARN_ON(video_device[vdev->minor] != vdev)) { + if (WARN_ON(video_devices[vdev->minor] != vdev)) { /* should not happen */ mutex_unlock(_lock); return; } /* Free up this device for reuse */ - video_device[vdev->minor] = NULL; + video_devices[vdev->minor] = NULL; /* Delete the cdev on this minor as well */ cdev_del(vdev->cdev); @@ -229,7 +229,7 @@ static struct class video_class = { struct video_device *video_devdata(struct file *file) { - return video_device[iminor(file_inode(file))]; + return video_devices[iminor(file_inode(file))]; } EXPORT_SYMBOL(video_devdata); @@ -493,9 +493,9 @@ static int get_index(struct video_device *vdev) bitmap_zero(used, VIDEO_NUM_DEVICES); for (i = 0; i < VIDEO_NUM_DEVICES; i++) { - if (video_device[i] != NULL && - video_device[i]->v4l2_dev == vdev->v4l2_dev) { - set_bit(video_device[i]->index, used); + if (video_devices[i] != NULL && + video_devices[i]->v4l2_dev == vdev->v4l2_dev) { + set_bit(video_devices[i]->index, used); } } @@ -929,7 +929,7 @@ int __video_register_device(struct video_device *vdev, /* The device node number and minor numbers are independent, so we just find the first free minor number. */ for (i = 0; i < VIDEO_NUM_DEVICES; i++) - if (video_device[i] == NULL) + if (video_devices[i] == NULL) break; if (i == VIDEO_NUM_DEVICES) { mutex_unlock(_lock); @@ -942,9 +942,9 @@ int __video_register_device(struct video_device *vdev, devnode_set(vdev); /* Should not happen since we thought this minor was free */ - WARN_ON(video_device[vdev->minor] != NULL); + WARN_ON(video_devices[vdev->minor] != NULL); vdev->index = get_index(vdev); - video_device[vdev->minor] = vdev; + video_devices[vdev->minor] = vdev; mutex_unlock(_lock); if (vdev->ioctl_ops) @@ -999,7 +999,7 @@ int __video_register_device(struct video_device *vdev, mutex_lock(_lock); if (vdev->cdev) cdev_del(vdev->cdev); - video_device[vdev->minor] = NULL; + video_devices[vdev->minor] = NULL; devnode_clear(vdev); mutex_unlock(_lock); /* Mark this video device as never having been registered. */ -- 2.17.0.484.g0c8726318c-goog
[GIT PULL for v4.17-rc1] media fixes and sparse/smatch cleanups
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v4.17-2 For a series of media updates/fixes for Kernel 4.17-rc1. There are two important core fix patches in this series: - A regression fix on Kernel 4.16 with causes it to not work with some input devices that depend on media core; - A fix at compat32 bits with causes it to OOPS on overlay, and affects the Kernels where the CVE-2017-13166 was backported. The remaining ones are other random fixes at the documentation and on drivers. The biggest part of this series is a set of 18 patches for the Intel atomisp driver. Currently, it produces hundreds of warnings/errors on sparse/smatch, causing me to sometimes ignore new warnings on other drivers that are not so broken. This driver is on really poor state, even for staging standards: it has several layers of abstraction on it, and it supports two different hardware. Selecting between them require to add a define (there isn't even a Kconfig option for such purpose). Just on this smatch cleanup, I could easily get rid of 8 "do-nothing" files. So, I'm seriously considering its removal from upstream, if I don't see any real work on addressing the problems there along this year. Thanks! Mauro - The following changes since commit 17dec0a949153d9ac00760ba2f5b78cb583e995f: Merge branch 'userns-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace (2018-04-03 19:15:32 -0700) are available in the Git repository at: for you to fetch changes up to a95845ba184b854106972f5d8f50354c2d272c06: media: v4l2-core: fix size of devnode_nums[] bitarray (2018-04-05 06:41:30 -0400) media updates for v4.17-rc1 Akinobu Mita (2): media: ov5645: add missing of_node_put() in error path media: ov5640: add missing output pixel format setting Alexandre Courbot (1): media: venus: vdec: fix format enumeration Arnd Bergmann (1): media: imx: work around false-positive warning Brad Love (1): media: cx231xx: Increase USB bridge bandwidth Chiranjeevi Rapolu (2): media: ov5670: Update to SPDX identifier media: ov13858: Update to SPDX identifier Colin Ian King (1): media: staging: media: davinci_vpfe: fix spelling of resizer_configure_in_continious_mode Fabio Estevam (2): media: imx-media-csi: Do not propagate the error when pinctrl is not found media: ov2685: Remove owner assignment from i2c_driver Hans Verkuil (6): media: v4l2-tpg-core.c: add space after % media: pixfmt-v4l2-mplane.rst: fix types media: pixfmt-v4l2.rst: fix types media: media-ioc-g-topology.rst: fix 'reserved' sizes media: media-types.rst: rename media-entity-type to media-entity-functions media: extended-controls.rst: transmitter -> receiver Hugues Fruchet (1): media: ov5640: fix get_/set_fmt colorspace related fields Jasmin Jessich (1): media: cec-pin: Fixed ktime_t to ns conversion Katsuhiro Suzuki (1): media: dvb_frontend: fix wrong cast in compat_ioctl Kieran Bingham (1): media: vsp1: Fix BRx conditional path in WPF Luca Ceresoli (2): media: doc: fix ReST link syntax media: imx274: fix typo in error message Mauro Carvalho Chehab (20): media: r820t: don't crash if attach fails media: staging: atomisp: do some coding style improvements media: staging: atomisp: ia_css_output.host: don't use var before check media: staging: atomisp: declare static vars as such media: staging: atomisp: get rid of stupid statements media: staging: atomisp: add a missing include media: staging: atomisp: fix endianess issues media: staging: atomisp: remove unused set_pd_base() media: staging: atomisp: get rid of an unused function media: staging: atomisp: Get rid of *default.host.[ch] media: staging: atomisp: don't access a NULL var media: staging: atomisp: avoid a warning if 32 bits build media: staging: atomisp: remove an useless check media: staging: atomisp: use %p to print pointers media: staging: atomisp: get rid of some static warnings media: staging: atomisp: stop mixing enum types media: staging: atomisp: get rid of an unused var media: staging: atomisp: stop duplicating input format types media: v4l2-compat-ioctl32: don't oops on overlay media: v4l2-core: fix size of devnode_nums[] bitarray Niklas Söderlund (1): media: i2c: adv748x: afe: fix sparse warning Rajmohan Mani (1): media: dw9714: Update to SPDX license identifier Ryder Lee (1): media: vcodec: fix error return value from mtk_jpeg_clk_init() Sakari Ailus (1): media: v4l: Bring back array_size parameter to v4l2_find_nearest_size Todor Tomov (1): media: ov5645: Use v4l2_find_nearest_size
[PATCH v2 02/19] [media] dvb-frontends/stv0910: fix CNR reporting in read_snr()
From: Daniel SchellerThe CNR value determined in read_snr() is reported via the wrong variable. It uses FE_SCALE_DECIBEL, which implies the value to be reported in svalue instead of uvalue. Fix this accordingly. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/dvb-frontends/stv0910.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c index f5b5ce971c0c..1d96ae9f9f6e 100644 --- a/drivers/media/dvb-frontends/stv0910.c +++ b/drivers/media/dvb-frontends/stv0910.c @@ -1326,7 +1326,7 @@ static int read_snr(struct dvb_frontend *fe) if (!get_signal_to_noise(state, )) { p->cnr.stat[0].scale = FE_SCALE_DECIBEL; - p->cnr.stat[0].uvalue = 100 * snrval; /* fix scale */ + p->cnr.stat[0].svalue = 100 * snrval; /* fix scale */ } else { p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE; } -- 2.16.1
[PATCH v2 05/19] [media] ddbridge: move MSI IRQ cleanup to a helper function
From: Daniel SchellerIntroduce the ddb_msi_exit() helper to be used for cleaning up previously allocated MSI IRQ vectors. Deduplicates code and makes things look cleaner as for all cleanup work the CONFIG_PCI_MSI ifdeffery is only needed in the helper now. Also, replace the call to the deprecated pci_disable_msi() function with pci_free_irq_vectors(). Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-main.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index 7088162af9d3..77089081db1f 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -65,16 +65,20 @@ static void ddb_irq_disable(struct ddb *dev) ddbwritel(dev, 0, MSI1_ENABLE); } +static void ddb_msi_exit(struct ddb *dev) +{ +#ifdef CONFIG_PCI_MSI + if (dev->msi) + pci_free_irq_vectors(dev->pdev); +#endif +} + static void ddb_irq_exit(struct ddb *dev) { ddb_irq_disable(dev); if (dev->msi == 2) free_irq(dev->pdev->irq + 1, dev); free_irq(dev->pdev->irq, dev); -#ifdef CONFIG_PCI_MSI - if (dev->msi) - pci_disable_msi(dev->pdev); -#endif } static void ddb_remove(struct pci_dev *pdev) @@ -86,6 +90,7 @@ static void ddb_remove(struct pci_dev *pdev) ddb_i2c_release(dev); ddb_irq_exit(dev); + ddb_msi_exit(dev); ddb_ports_release(dev); ddb_buffers_free(dev); @@ -230,8 +235,7 @@ static int ddb_probe(struct pci_dev *pdev, ddb_irq_exit(dev); fail0: dev_err(>dev, "fail0\n"); - if (dev->msi) - pci_disable_msi(dev->pdev); + ddb_msi_exit(dev); fail: dev_err(>dev, "fail\n"); -- 2.16.1
[PATCH v2 17/19] [media] ddbridge: add hardware defs and PCI IDs for MCI cards
From: Daniel SchellerAdd PCI IDs and ddb_info for the new MCI-based MaxSX8 cards. Also add needed defines so the cards can be hooked up into ddbridge's probe and attach handling. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-hw.c | 11 +++ drivers/media/pci/ddbridge/ddbridge-main.c | 1 + drivers/media/pci/ddbridge/ddbridge.h | 11 +++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-hw.c b/drivers/media/pci/ddbridge/ddbridge-hw.c index c6d14925e2fc..1d3ee6accdd5 100644 --- a/drivers/media/pci/ddbridge/ddbridge-hw.c +++ b/drivers/media/pci/ddbridge/ddbridge-hw.c @@ -311,6 +311,16 @@ static const struct ddb_info ddb_s2_48 = { .tempmon_irq = 24, }; +static const struct ddb_info ddb_s2x_48 = { + .type = DDB_OCTOPUS_MCI, + .name = "Digital Devices MAX SX8", + .regmap = _map, + .port_num = 4, + .i2c_mask = 0x00, + .tempmon_irq = 24, + .mci = 4 +}; + // // // @@ -346,6 +356,7 @@ static const struct ddb_device_id ddb_device_ids[] = { DDB_DEVID(0x0008, 0x0036, ddb_isdbt_8), DDB_DEVID(0x0008, 0x0037, ddb_c2t2i_v0_8), DDB_DEVID(0x0008, 0x0038, ddb_c2t2i_8), + DDB_DEVID(0x0009, 0x0025, ddb_s2x_48), DDB_DEVID(0x0006, 0x0039, ddb_ctv7), DDB_DEVID(0x0011, 0x0040, ddb_ci), DDB_DEVID(0x0011, 0x0041, ddb_cis), diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index 6356b48b3874..f4748cfd904b 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -264,6 +264,7 @@ static const struct pci_device_id ddb_id_table[] = { DDB_DEVICE_ANY(0x0006), DDB_DEVICE_ANY(0x0007), DDB_DEVICE_ANY(0x0008), + DDB_DEVICE_ANY(0x0009), DDB_DEVICE_ANY(0x0011), DDB_DEVICE_ANY(0x0012), DDB_DEVICE_ANY(0x0013), diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index cb69021a3443..72fe33cb72b9 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -112,11 +112,12 @@ struct ddb_ids { struct ddb_info { int type; -#define DDB_NONE 0 -#define DDB_OCTOPUS 1 -#define DDB_OCTOPUS_CI 2 -#define DDB_OCTOPUS_MAX 5 +#define DDB_NONE0 +#define DDB_OCTOPUS 1 +#define DDB_OCTOPUS_CI 2 +#define DDB_OCTOPUS_MAX 5 #define DDB_OCTOPUS_MAX_CT 6 +#define DDB_OCTOPUS_MCI 9 char *name; u32 i2c_mask; u8port_num; @@ -133,6 +134,7 @@ struct ddb_info { #define TS_QUIRK_REVERSED 2 #define TS_QUIRK_ALT_OSC 8 u32 tempmon_irq; + u8mci; const struct ddb_regmap *regmap; }; @@ -253,6 +255,7 @@ struct ddb_port { #define DDB_CI_EXTERNAL_XO2_B13 #define DDB_TUNER_DVBS_STV0910_PR 14 #define DDB_TUNER_DVBC2T2I_SONY_P 15 +#define DDB_TUNER_MCI16 #define DDB_TUNER_XO232 #define DDB_TUNER_DVBS_STV0910 (DDB_TUNER_XO2 + 0) -- 2.16.1
[PATCH v2 07/19] [media] ddbridge: add helper for IRQ handler setup
From: Daniel SchellerIntroduce the ddb_irq_set() helper function (along with a matching prototype in ddbridge.h) to improve the set up of the IRQ handlers and handler_data, and rework storing this data into the ddb_link using a new ddb_irq struct. This also does the necessary rework of affected variables. And while at it, always do queue_work in input_handler() as there's not much of a difference to directly calling input_work if there's no ptr at input->redi, or queueing this call. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 53 +- drivers/media/pci/ddbridge/ddbridge-i2c.c | 5 ++- drivers/media/pci/ddbridge/ddbridge.h | 11 +-- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index fb9a2cb758e6..be6935bd0cb5 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -108,6 +108,16 @@ static struct ddb *ddbs[DDB_MAX_ADAPTER]; // // +struct ddb_irq *ddb_irq_set(struct ddb *dev, u32 link, u32 nr, + void (*handler)(void *), void *data) +{ + struct ddb_irq *irq = >link[link].irq[nr]; + + irq->handler = handler; + irq->data = data; + return irq; +} + static void ddb_set_dma_table(struct ddb_io *io) { struct ddb *dev = io->port->dev; @@ -2109,26 +2119,18 @@ static void input_work(struct work_struct *work) spin_unlock_irqrestore(>lock, flags); } -static void input_handler(unsigned long data) +static void input_handler(void *data) { struct ddb_input *input = (struct ddb_input *)data; struct ddb_dma *dma = input->dma; - /* -* If there is no input connected, input_tasklet() will -* just copy pointers and ACK. So, there is no need to go -* through the tasklet scheduler. -*/ - if (input->redi) - queue_work(ddb_wq, >work); - else - input_work(>work); + queue_work(ddb_wq, >work); } -static void output_handler(unsigned long data) +static void output_work(struct work_struct *work) { - struct ddb_output *output = (struct ddb_output *)data; - struct ddb_dma *dma = output->dma; + struct ddb_dma *dma = container_of(work, struct ddb_dma, work); + struct ddb_output *output = (struct ddb_output *)dma->io; struct ddb *dev = output->port->dev; spin_lock(>lock); @@ -2144,6 +2146,14 @@ static void output_handler(unsigned long data) spin_unlock(>lock); } +static void output_handler(void *data) +{ + struct ddb_output *output = (struct ddb_output *)data; + struct ddb_dma *dma = output->dma; + + queue_work(ddb_wq, >work); +} + // // @@ -2174,6 +2184,7 @@ static void ddb_dma_init(struct ddb_io *io, int nr, int out) spin_lock_init(>lock); init_waitqueue_head(>wq); if (out) { + INIT_WORK(>work, output_work); dma->regs = rm->odma->base + rm->odma->size * nr; dma->bufregs = rm->odma_buf->base + rm->odma_buf->size * nr; dma->num = OUTPUT_DMA_BUFS; @@ -2218,8 +2229,7 @@ static void ddb_input_init(struct ddb_port *port, int nr, int pnr, int anr) dev_dbg(dev->dev, "init link %u, input %u, handler %u\n", port->lnr, nr, dma_nr + base); - dev->handler[0][dma_nr + base] = input_handler; - dev->handler_data[0][dma_nr + base] = (unsigned long)input; + ddb_irq_set(dev, 0, dma_nr + base, _handler, input); ddb_dma_init(input, dma_nr, 0); } } @@ -2244,8 +2254,7 @@ static void ddb_output_init(struct ddb_port *port, int nr) const struct ddb_regmap *rm0 = io_regmap(output, 0); u32 base = rm0->irq_base_odma; - dev->handler[0][nr + base] = output_handler; - dev->handler_data[0][nr + base] = (unsigned long)output; + ddb_irq_set(dev, 0, nr + base, _handler, output); ddb_dma_init(output, nr, 1); } } @@ -2389,8 +2398,9 @@ void ddb_ports_release(struct ddb *dev) // #define IRQ_HANDLE(_nr) \ - do { if ((s & (1UL << ((_nr) & 0x1f))) && dev->handler[0][_nr]) \ - dev->handler[0][_nr](dev->handler_data[0][_nr]); } \ + do { if ((s & (1UL << ((_nr) & 0x1f))) && \ +
[PATCH v2 08/19] [media] ddbridge: add macros to handle IRQs in nibble and byte blocks
From: Daniel SchellerCurrently, each IRQ requires one IRQ_HANDLE() line to call each IRQ handler that was set up. Add a IRQ_HANDLE_NIBBLE() and IRQ_HANDLE_BYTE() macro to call all handlers in blocks of four (_NIBBLE) or eight (_BYTE) handlers at a time, to make this construct more compact. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 67 -- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index be6935bd0cb5..5fbb0996a12c 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -2403,54 +2403,41 @@ void ddb_ports_release(struct ddb *dev) dev->link[0].irq[_nr].handler(dev->link[0].irq[_nr].data); } \ while (0) +#define IRQ_HANDLE_NIBBLE(_shift) { \ + if (s & (0x000f << ((_shift) & 0x1f))) { \ + IRQ_HANDLE(0 + (_shift));\ + IRQ_HANDLE(1 + (_shift));\ + IRQ_HANDLE(2 + (_shift));\ + IRQ_HANDLE(3 + (_shift));\ + }\ +} + +#define IRQ_HANDLE_BYTE(_shift) { \ + if (s & (0x00ff << ((_shift) & 0x1f))) { \ + IRQ_HANDLE(0 + (_shift));\ + IRQ_HANDLE(1 + (_shift));\ + IRQ_HANDLE(2 + (_shift));\ + IRQ_HANDLE(3 + (_shift));\ + IRQ_HANDLE(4 + (_shift));\ + IRQ_HANDLE(5 + (_shift));\ + IRQ_HANDLE(6 + (_shift));\ + IRQ_HANDLE(7 + (_shift));\ + }\ +} + static void irq_handle_msg(struct ddb *dev, u32 s) { dev->i2c_irq++; - IRQ_HANDLE(0); - IRQ_HANDLE(1); - IRQ_HANDLE(2); - IRQ_HANDLE(3); + IRQ_HANDLE_NIBBLE(0); } static void irq_handle_io(struct ddb *dev, u32 s) { dev->ts_irq++; - if ((s & 0x00f0)) { - IRQ_HANDLE(4); - IRQ_HANDLE(5); - IRQ_HANDLE(6); - IRQ_HANDLE(7); - } - if ((s & 0xff00)) { - IRQ_HANDLE(8); - IRQ_HANDLE(9); - IRQ_HANDLE(10); - IRQ_HANDLE(11); - IRQ_HANDLE(12); - IRQ_HANDLE(13); - IRQ_HANDLE(14); - IRQ_HANDLE(15); - } - if ((s & 0x00ff)) { - IRQ_HANDLE(16); - IRQ_HANDLE(17); - IRQ_HANDLE(18); - IRQ_HANDLE(19); - IRQ_HANDLE(20); - IRQ_HANDLE(21); - IRQ_HANDLE(22); - IRQ_HANDLE(23); - } - if ((s & 0xff00)) { - IRQ_HANDLE(24); - IRQ_HANDLE(25); - IRQ_HANDLE(26); - IRQ_HANDLE(27); - IRQ_HANDLE(28); - IRQ_HANDLE(29); - IRQ_HANDLE(30); - IRQ_HANDLE(31); - } + IRQ_HANDLE_NIBBLE(4); + IRQ_HANDLE_BYTE(8); + IRQ_HANDLE_BYTE(16); + IRQ_HANDLE_BYTE(24); } irqreturn_t ddb_irq_handler0(int irq, void *dev_id) -- 2.16.1
[PATCH v2 12/19] [media] ddbridge: set devid entry for link 0
From: Daniel SchellerCurrently, /sys/class/ddbridgeX/devid always reports 0 due to devid not being set at all. Set the devid field alongside while storing all other hardware ID data. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index 008be9066814..6356b48b3874 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -198,6 +198,7 @@ static int ddb_probe(struct pci_dev *pdev, dev->link[0].ids.device = id->device; dev->link[0].ids.subvendor = id->subvendor; dev->link[0].ids.subdevice = pdev->subsystem_device; + dev->link[0].ids.devid = (id->device << 16) | id->vendor; dev->link[0].dev = dev; dev->link[0].info = get_ddb_info(id->vendor, id->device, -- 2.16.1
[PATCH v2 15/19] [media] ddbridge: initial support for MCI-based MaxSX8 cards
From: Daniel SchellerThis adds initial support for the new MCI-based (micro-code interface) DD cards, with the first one being the MaxSX8 eight-tuner DVB-S/S2/S2X PCIe card. The MCI is basically a generalized interface implemented in the card's FPGA firmware and usable for all kind of cards, without the need to implement any demod/tuner drivers as this interface "hides" any I2C interface to the actual ICs, in other words any required driver is implemented in the card firmware. At this stage, the MCI interface is quite rudimentary with things like signal statistics reporting missing, but is already working to serve DVB streams to DVB applications. Missing functionality will be enabled over time. This implements only the ddbridge-mci sub-object and hooks it up to the Makefile so the object gets build. The upcoming commits hook this module into all other ddbridge parts where required, including device IDs etc. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/Makefile | 2 +- drivers/media/pci/ddbridge/ddbridge-mci.c | 550 ++ drivers/media/pci/ddbridge/ddbridge-mci.h | 152 + 3 files changed, 703 insertions(+), 1 deletion(-) create mode 100644 drivers/media/pci/ddbridge/ddbridge-mci.c create mode 100644 drivers/media/pci/ddbridge/ddbridge-mci.h diff --git a/drivers/media/pci/ddbridge/Makefile b/drivers/media/pci/ddbridge/Makefile index 745b37d07558..9b9e35f171b7 100644 --- a/drivers/media/pci/ddbridge/Makefile +++ b/drivers/media/pci/ddbridge/Makefile @@ -4,7 +4,7 @@ # ddbridge-objs := ddbridge-main.o ddbridge-core.o ddbridge-ci.o \ - ddbridge-hw.o ddbridge-i2c.o ddbridge-max.o + ddbridge-hw.o ddbridge-i2c.o ddbridge-max.o ddbridge-mci.o obj-$(CONFIG_DVB_DDBRIDGE) += ddbridge.o diff --git a/drivers/media/pci/ddbridge/ddbridge-mci.c b/drivers/media/pci/ddbridge/ddbridge-mci.c new file mode 100644 index ..214b301f30a5 --- /dev/null +++ b/drivers/media/pci/ddbridge/ddbridge-mci.c @@ -0,0 +1,550 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ddbridge-mci.c: Digital Devices microcode interface + * + * Copyright (C) 2017 Digital Devices GmbH + *Ralph Metzler + *Marcus Metzler + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 only, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "ddbridge.h" +#include "ddbridge-io.h" +#include "ddbridge-mci.h" + +static LIST_HEAD(mci_list); + +static const u32 MCLK = (155000 / 12); +static const u32 MAX_DEMOD_LDPC_BITRATE = (155000 / 6); +static const u32 MAX_LDPC_BITRATE = (72000); + +struct mci_base { + struct list_head mci_list; + void*key; + struct ddb_link *link; + struct completioncompletion; + + struct device *dev; + struct mutex tuner_lock; /* concurrent tuner access lock */ + u8 adr; + struct mutex mci_lock; /* concurrent MCI access lock */ + int count; + + u8 tuner_use_count[4]; + u8 assigned_demod[8]; + u32 used_ldpc_bitrate[8]; + u8 demod_in_use[8]; + u32 iq_mode; +}; + +struct mci { + struct mci_base *base; + struct dvb_frontend fe; + int nr; + int demod; + int tuner; + int first_time_lock; + int started; + struct mci_resultsignal_info; + + u32 bb_mode; +}; + +static int mci_reset(struct mci *state) +{ + struct ddb_link *link = state->base->link; + u32 status = 0; + u32 timeout = 40; + + ddblwritel(link, MCI_CONTROL_RESET, MCI_CONTROL); + ddblwritel(link, 0, MCI_CONTROL + 4); /* 1= no internal init */ + msleep(300); + ddblwritel(link, 0, MCI_CONTROL); + + while (1) { + status = ddblreadl(link, MCI_CONTROL); + if ((status & MCI_CONTROL_READY) == MCI_CONTROL_READY) + break; + if (--timeout == 0) + break; + msleep(50); + } + if ((status & MCI_CONTROL_READY) == 0) + return -1; + if (link->ids.device == 0x0009) + ddblwritel(link, SX8_TSCONFIG_MODE_NORMAL, SX8_TSCONFIG); + return 0; +} + +static int
[PATCH v2 13/19] [media] ddbridge: make DMA buffer count and size modparam-configurable
From: Daniel SchellerMake the number of DMA buffers and their size configurable using module parameters. Being able to set these to a higher number might help on busy systems when handling overall high data rates without having to edit the driver sources and recompile things. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 30 -- drivers/media/pci/ddbridge/ddbridge.h | 12 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index e9c2e3e5d64b..8907551b02e4 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -96,6 +96,15 @@ static int stv0910_single; module_param(stv0910_single, int, 0444); MODULE_PARM_DESC(stv0910_single, "use stv0910 cards as single demods"); +static int dma_buf_num = 8; +module_param(dma_buf_num, int, 0444); +MODULE_PARM_DESC(dma_buf_num, "Number of DMA buffers, possible values: 8-32"); + +static int dma_buf_size = 21; +module_param(dma_buf_size, int, 0444); +MODULE_PARM_DESC(dma_buf_size, +"DMA buffer size as multiple of 128*47, possible values: 1-43"); + // static DEFINE_MUTEX(redirect_lock); @@ -2187,16 +2196,16 @@ static void ddb_dma_init(struct ddb_io *io, int nr, int out) INIT_WORK(>work, output_work); dma->regs = rm->odma->base + rm->odma->size * nr; dma->bufregs = rm->odma_buf->base + rm->odma_buf->size * nr; - dma->num = OUTPUT_DMA_BUFS; - dma->size = OUTPUT_DMA_SIZE; - dma->div = OUTPUT_DMA_IRQ_DIV; + dma->num = dma_buf_num; + dma->size = dma_buf_size * 128 * 47; + dma->div = 1; } else { INIT_WORK(>work, input_work); dma->regs = rm->idma->base + rm->idma->size * nr; dma->bufregs = rm->idma_buf->base + rm->idma_buf->size * nr; - dma->num = INPUT_DMA_BUFS; - dma->size = INPUT_DMA_SIZE; - dma->div = INPUT_DMA_IRQ_DIV; + dma->num = dma_buf_num; + dma->size = dma_buf_size * 128 * 47; + dma->div = 1; } ddbwritel(io->port->dev, 0, DMA_BUFFER_ACK(dma)); dev_dbg(io->port->dev->dev, "init link %u, io %u, dma %u, dmaregs %08x bufregs %08x\n", @@ -3353,6 +3362,15 @@ int ddb_exit_ddbridge(int stage, int error) int ddb_init_ddbridge(void) { + if (dma_buf_num < 8) + dma_buf_num = 8; + if (dma_buf_num > 32) + dma_buf_num = 32; + if (dma_buf_size < 1) + dma_buf_size = 1; + if (dma_buf_size > 43) + dma_buf_size = 43; + if (ddb_class_create() < 0) return -1; ddb_wq = alloc_workqueue("ddbridge", 0, 0); diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index de9ddf1068bf..86db6f19369a 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -136,20 +136,8 @@ struct ddb_info { const struct ddb_regmap *regmap; }; -/* DMA_SIZE MUST be smaller than 256k and - * MUST be divisible by 188 and 128 !!! - */ - #define DMA_MAX_BUFS 32 /* hardware table limit */ -#define INPUT_DMA_BUFS 8 -#define INPUT_DMA_SIZE (128 * 47 * 21) -#define INPUT_DMA_IRQ_DIV 1 - -#define OUTPUT_DMA_BUFS 8 -#define OUTPUT_DMA_SIZE (128 * 47 * 21) -#define OUTPUT_DMA_IRQ_DIV 1 - struct ddb; struct ddb_port; -- 2.16.1
[PATCH v2 14/19] [media] ddbridge: support dummy tuners with 125MByte/s dummy data stream
From: Daniel SchellerThe Octopus V3 and Octopus Mini devices support set up of a dummy tuner mode on port 0 that will deliver a continuous data stream of 125MBytes per second while raising IRQs and filling the DMA buffers, which comes handy for some stress, PCIe link and IRQ handling testing. The dummy frontend is registered using dvb_dummy_fe's QAM dummy frontend. Set ddbridge.dummy_tuner to 1 to enable this on the supported cards. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/Kconfig | 1 + drivers/media/pci/ddbridge/ddbridge-core.c | 36 ++ drivers/media/pci/ddbridge/ddbridge.h | 1 + 3 files changed, 38 insertions(+) diff --git a/drivers/media/pci/ddbridge/Kconfig b/drivers/media/pci/ddbridge/Kconfig index a422dde2f34a..16faef265e97 100644 --- a/drivers/media/pci/ddbridge/Kconfig +++ b/drivers/media/pci/ddbridge/Kconfig @@ -14,6 +14,7 @@ config DVB_DDBRIDGE select MEDIA_TUNER_TDA18212 if MEDIA_SUBDRV_AUTOSELECT select DVB_MXL5XX if MEDIA_SUBDRV_AUTOSELECT select DVB_CXD2099 if MEDIA_SUBDRV_AUTOSELECT + select DVB_DUMMY_FE if MEDIA_SUBDRV_AUTOSELECT ---help--- Support for cards with the Digital Devices PCI express bridge: - Octopus PCIe Bridge diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 8907551b02e4..59e137516003 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -54,6 +54,7 @@ #include "stv6111.h" #include "lnbh25.h" #include "cxd2099.h" +#include "dvb_dummy_fe.h" // @@ -105,6 +106,11 @@ module_param(dma_buf_size, int, 0444); MODULE_PARM_DESC(dma_buf_size, "DMA buffer size as multiple of 128*47, possible values: 1-43"); +static int dummy_tuner; +module_param(dummy_tuner, int, 0444); +MODULE_PARM_DESC(dummy_tuner, +"attach dummy tuner to port 0 on Octopus V3 or Octopus Mini cards"); + // static DEFINE_MUTEX(redirect_lock); @@ -548,6 +554,9 @@ static void ddb_input_start(struct ddb_input *input) ddbwritel(dev, 0x09, TS_CONTROL(input)); + if (input->port->type == DDB_TUNER_DUMMY) + ddbwritel(dev, 0x000fff01, TS_CONTROL2(input)); + if (input->dma) { input->dma->running = 1; spin_unlock_irq(>dma->lock); @@ -1255,6 +1264,20 @@ static int tuner_attach_stv6111(struct ddb_input *input, int type) return 0; } +static int demod_attach_dummy(struct ddb_input *input) +{ + struct ddb_dvb *dvb = >port->dvb[input->nr & 1]; + struct device *dev = input->port->dev->dev; + + dvb->fe = dvb_attach(dvb_dummy_fe_qam_attach); + if (!dvb->fe) { + dev_err(dev, "QAM dummy attach failed!\n"); + return -ENODEV; + } + + return 0; +} + static int start_feed(struct dvb_demux_feed *dvbdmxfeed) { struct dvb_demux *dvbdmx = dvbdmxfeed->demux; @@ -1547,6 +1570,10 @@ static int dvb_input_attach(struct ddb_input *input) if (tuner_attach_tda18212(input, port->type) < 0) goto err_tuner; break; + case DDB_TUNER_DUMMY: + if (demod_attach_dummy(input) < 0) + goto err_detach; + break; default: return 0; } @@ -1809,6 +1836,15 @@ static void ddb_port_probe(struct ddb_port *port) /* Handle missing ports and ports without I2C */ + if (dummy_tuner && !port->nr && + dev->link[0].ids.device == 0x0005) { + port->name = "DUMMY"; + port->class = DDB_PORT_TUNER; + port->type = DDB_TUNER_DUMMY; + port->type_name = "DUMMY"; + return; + } + if (port->nr == ts_loop) { port->name = "TS LOOP"; port->class = DDB_PORT_LOOP; diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index 86db6f19369a..cb69021a3443 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -236,6 +236,7 @@ struct ddb_port { char *name; char *type_name; u32 type; +#define DDB_TUNER_DUMMY 0x #define DDB_TUNER_NONE 0 #define DDB_TUNER_DVBS_ST1 #define DDB_TUNER_DVBS_ST_AA 2 -- 2.16.1
[PATCH v2 19/19] [media] ddbridge: set driver version to 0.9.33-integrated
From: Daniel SchellerSet DDBRIDGE_VERSION in ddbridge.h to 0.9.33-integrated to reflect the updated driver. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index 72fe33cb72b9..a66b1125cc74 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -63,7 +63,7 @@ #include #include -#define DDBRIDGE_VERSION "0.9.32-integrated" +#define DDBRIDGE_VERSION "0.9.33-integrated" #define DDB_MAX_I2C32 #define DDB_MAX_PORT 32 -- 2.16.1
[PATCH v2 18/19] [media] ddbridge: recognize and attach the MaxSX8 cards
From: Daniel SchellerAdd needed logic into dvb_input_attach(), ddb_port_probe() and ddb_ports_init() to initialize and support these new cards. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 59e137516003..4a2819d3e225 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -1574,6 +1574,10 @@ static int dvb_input_attach(struct ddb_input *input) if (demod_attach_dummy(input) < 0) goto err_detach; break; + case DDB_TUNER_MCI: + if (ddb_fe_attach_mci(input) < 0) + goto err_detach; + break; default: return 0; } @@ -1869,6 +1873,16 @@ static void ddb_port_probe(struct ddb_port *port) return; } + if (dev->link[l].info->type == DDB_OCTOPUS_MCI) { + if (port->nr >= dev->link[l].info->mci) + return; + port->name = "DUAL MCI"; + port->type_name = "MCI"; + port->class = DDB_PORT_TUNER; + port->type = DDB_TUNER_MCI; + return; + } + if (port->nr > 1 && dev->link[l].info->type == DDB_OCTOPUS_CI) { port->name = "CI internal"; port->type_name = "INTERNAL"; @@ -2411,6 +2425,7 @@ void ddb_ports_init(struct ddb *dev) break; case DDB_OCTOPUS_MAX: case DDB_OCTOPUS_MAX_CT: + case DDB_OCTOPUS_MCI: ddb_input_init(port, 2 * i, 0, 2 * p); ddb_input_init(port, 2 * i + 1, 1, 2 * p + 1); break; -- 2.16.1
[PATCH v2 10/19] [media] ddbridge: use spin_lock_irqsave() in output_work()
From: Daniel SchellerMake sure to save IRQ states before taking the dma lock, as already done in it's input_work() counterpart. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 9d91221dacc4..c22537eceee5 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -2132,18 +2132,18 @@ static void output_work(struct work_struct *work) struct ddb_dma *dma = container_of(work, struct ddb_dma, work); struct ddb_output *output = (struct ddb_output *)dma->io; struct ddb *dev = output->port->dev; + unsigned long flags; - spin_lock(>lock); - if (!dma->running) { - spin_unlock(>lock); - return; - } + spin_lock_irqsave(>lock, flags); + if (!dma->running) + goto unlock_exit; dma->stat = ddbreadl(dev, DMA_BUFFER_CURRENT(dma)); dma->ctrl = ddbreadl(dev, DMA_BUFFER_CONTROL(dma)); if (output->redi) output_ack_input(output, output->redi); wake_up(>wq); - spin_unlock(>lock); +unlock_exit: + spin_unlock_irqrestore(>lock, flags); } static void output_handler(void *data) -- 2.16.1
[PATCH v2 11/19] [media] ddbridge: fix output buffer check
From: Daniel SchellerA 188 byte gap has to be left between the writer and the consumer. This requires 2*188 bytes available to be able to write to the output buffers. So, change ddb_output_free() to report free bytes according to this rule. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index c22537eceee5..e9c2e3e5d64b 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -587,12 +587,12 @@ static u32 ddb_output_free(struct ddb_output *output) if (output->dma->cbuf != idx) { if output->dma->cbuf + 1) % output->dma->num) == idx) && - (output->dma->size - output->dma->coff <= 188)) + (output->dma->size - output->dma->coff <= (2 * 188))) return 0; return 188; } diff = off - output->dma->coff; - if (diff <= 0 || diff > 188) + if (diff <= 0 || diff > (2 * 188)) return 188; return 0; } -- 2.16.1
[PATCH v2 16/19] [media] ddbridge/max: implement MCI/MaxSX8 attach function
From: Daniel SchellerImplement frontend attachment as ddb_fe_attach_mci() into the ddbridge-max module. The MaxSX8 MCI cards are part of the Max card series and make use of the LNB controller driven by the already existing lnb functionality, so here's where this code belongs to. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-max.c | 42 +++ drivers/media/pci/ddbridge/ddbridge-max.h | 1 + 2 files changed, 43 insertions(+) diff --git a/drivers/media/pci/ddbridge/ddbridge-max.c b/drivers/media/pci/ddbridge/ddbridge-max.c index dc6b81488746..739e4b444cf4 100644 --- a/drivers/media/pci/ddbridge/ddbridge-max.c +++ b/drivers/media/pci/ddbridge/ddbridge-max.c @@ -33,6 +33,7 @@ #include "ddbridge.h" #include "ddbridge-regs.h" #include "ddbridge-io.h" +#include "ddbridge-mci.h" #include "ddbridge-max.h" #include "mxl5xx.h" @@ -452,3 +453,44 @@ int ddb_fe_attach_mxl5xx(struct ddb_input *input) dvb->input = tuner; return 0; } + +/**/ +/* MAX MCI related functions */ + +int ddb_fe_attach_mci(struct ddb_input *input) +{ + struct ddb *dev = input->port->dev; + struct ddb_dvb *dvb = >port->dvb[input->nr & 1]; + struct ddb_port *port = input->port; + struct ddb_link *link = >link[port->lnr]; + int demod, tuner; + + demod = input->nr; + tuner = demod & 3; + if (fmode == 3) + tuner = 0; + dvb->fe = ddb_mci_attach(input, 0, demod, >set_input); + if (!dvb->fe) { + dev_err(dev->dev, "No MAXSX8 found!\n"); + return -ENODEV; + } + if (!dvb->set_input) { + dev_err(dev->dev, "No MCI set_input function pointer!\n"); + return -ENODEV; + } + if (input->nr < 4) { + lnb_command(dev, port->lnr, input->nr, LNB_CMD_INIT); + lnb_set_voltage(dev, port->lnr, input->nr, SEC_VOLTAGE_OFF); + } + ddb_lnb_init_fmode(dev, link, fmode); + + dvb->fe->ops.set_voltage = max_set_voltage; + dvb->fe->ops.enable_high_lnb_voltage = max_enable_high_lnb_voltage; + dvb->fe->ops.set_tone = max_set_tone; + dvb->diseqc_send_master_cmd = dvb->fe->ops.diseqc_send_master_cmd; + dvb->fe->ops.diseqc_send_master_cmd = max_send_master_cmd; + dvb->fe->ops.diseqc_send_burst = max_send_burst; + dvb->fe->sec_priv = input; + dvb->input = tuner; + return 0; +} diff --git a/drivers/media/pci/ddbridge/ddbridge-max.h b/drivers/media/pci/ddbridge/ddbridge-max.h index bf8bf38739f6..82efc53baa94 100644 --- a/drivers/media/pci/ddbridge/ddbridge-max.h +++ b/drivers/media/pci/ddbridge/ddbridge-max.h @@ -25,5 +25,6 @@ int ddb_lnb_init_fmode(struct ddb *dev, struct ddb_link *link, u32 fm); int ddb_fe_attach_mxl5xx(struct ddb_input *input); +int ddb_fe_attach_mci(struct ddb_input *input); #endif /* _DDBRIDGE_MAX_H */ -- 2.16.1
[PATCH v2 03/19] [media] ddbridge: move modparams to ddbridge-core.c
From: Daniel SchellerBesides the 'msi' module option, all options are used from within ddbridge-core only, so move them over from ddbridge-main, and declare the associated variables static. Since the prototypes in ddbridge.h aren't necessary anymore now, remove them. As a side effect, this has the benefit of aligning things more with the dddvb upstream. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 28 drivers/media/pci/ddbridge/ddbridge-main.c | 28 drivers/media/pci/ddbridge/ddbridge.h | 6 -- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 90687eff5909..933046d03db5 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -68,6 +68,34 @@ module_param(adapter_alloc, int, 0444); MODULE_PARM_DESC(adapter_alloc, "0-one adapter per io, 1-one per tab with io, 2-one per tab, 3-one for all"); +static int ci_bitrate = 7; +module_param(ci_bitrate, int, 0444); +MODULE_PARM_DESC(ci_bitrate, " Bitrate in KHz for output to CI."); + +static int ts_loop = -1; +module_param(ts_loop, int, 0444); +MODULE_PARM_DESC(ts_loop, "TS in/out test loop on port ts_loop"); + +static int xo2_speed = 2; +module_param(xo2_speed, int, 0444); +MODULE_PARM_DESC(xo2_speed, "default transfer speed for xo2 based duoflex, 0=55,1=75,2=90,3=104 MBit/s, default=2, use attribute to change for individual cards"); + +#ifdef __arm__ +static int alt_dma = 1; +#else +static int alt_dma; +#endif +module_param(alt_dma, int, 0444); +MODULE_PARM_DESC(alt_dma, "use alternative DMA buffer handling"); + +static int no_init; +module_param(no_init, int, 0444); +MODULE_PARM_DESC(no_init, "do not initialize most devices"); + +static int stv0910_single; +module_param(stv0910_single, int, 0444); +MODULE_PARM_DESC(stv0910_single, "use stv0910 cards as single demods"); + // static DEFINE_MUTEX(redirect_lock); diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index 26497d6b1395..bde04dc39080 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -55,34 +55,6 @@ MODULE_PARM_DESC(msi, "Control MSI interrupts: 0-disable (default), 1-enable"); #endif #endif -int ci_bitrate = 7; -module_param(ci_bitrate, int, 0444); -MODULE_PARM_DESC(ci_bitrate, " Bitrate in KHz for output to CI."); - -int ts_loop = -1; -module_param(ts_loop, int, 0444); -MODULE_PARM_DESC(ts_loop, "TS in/out test loop on port ts_loop"); - -int xo2_speed = 2; -module_param(xo2_speed, int, 0444); -MODULE_PARM_DESC(xo2_speed, "default transfer speed for xo2 based duoflex, 0=55,1=75,2=90,3=104 MBit/s, default=2, use attribute to change for individual cards"); - -#ifdef __arm__ -int alt_dma = 1; -#else -int alt_dma; -#endif -module_param(alt_dma, int, 0444); -MODULE_PARM_DESC(alt_dma, "use alternative DMA buffer handling"); - -int no_init; -module_param(no_init, int, 0444); -MODULE_PARM_DESC(no_init, "do not initialize most devices"); - -int stv0910_single; -module_param(stv0910_single, int, 0444); -MODULE_PARM_DESC(stv0910_single, "use stv0910 cards as single demods"); - // // // diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index f223dc6c9963..e22e67d7e0fe 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -369,12 +369,6 @@ int ddbridge_flashread(struct ddb *dev, u32 link, u8 *buf, u32 addr, u32 len); // /* ddbridge-main.c (modparams) */ -extern int ci_bitrate; -extern int ts_loop; -extern int xo2_speed; -extern int alt_dma; -extern int no_init; -extern int stv0910_single; extern struct workqueue_struct *ddb_wq; /* ddbridge-core.c */ -- 2.16.1
[PATCH v2 09/19] [media] ddbridge: improve separated MSI IRQ handling
From: Daniel SchellerImprove IRQ handling in the separated MSG/I2C and IO/TSDATA handlers by applying a mask for recognized bits immediately upon reading the IRQ mask from the hardware, so only the bits/IRQs that actually were set will be acked. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 5fbb0996a12c..9d91221dacc4 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -2443,16 +2443,17 @@ static void irq_handle_io(struct ddb *dev, u32 s) irqreturn_t ddb_irq_handler0(int irq, void *dev_id) { struct ddb *dev = (struct ddb *)dev_id; - u32 s = ddbreadl(dev, INTERRUPT_STATUS); + u32 mask = 0x8f00; + u32 s = mask & ddbreadl(dev, INTERRUPT_STATUS); + if (!s) + return IRQ_NONE; do { if (s & 0x8000) return IRQ_NONE; - if (!(s & 0xf00)) - return IRQ_NONE; - ddbwritel(dev, s & 0xf00, INTERRUPT_ACK); + ddbwritel(dev, s, INTERRUPT_ACK); irq_handle_io(dev, s); - } while ((s = ddbreadl(dev, INTERRUPT_STATUS))); + } while ((s = mask & ddbreadl(dev, INTERRUPT_STATUS))); return IRQ_HANDLED; } @@ -2460,16 +2461,17 @@ irqreturn_t ddb_irq_handler0(int irq, void *dev_id) irqreturn_t ddb_irq_handler1(int irq, void *dev_id) { struct ddb *dev = (struct ddb *)dev_id; - u32 s = ddbreadl(dev, INTERRUPT_STATUS); + u32 mask = 0x800f; + u32 s = mask & ddbreadl(dev, INTERRUPT_STATUS); + if (!s) + return IRQ_NONE; do { if (s & 0x8000) return IRQ_NONE; - if (!(s & 0xf)) - return IRQ_NONE; - ddbwritel(dev, s & 0xf, INTERRUPT_ACK); + ddbwritel(dev, s, INTERRUPT_ACK); irq_handle_msg(dev, s); - } while ((s = ddbreadl(dev, INTERRUPT_STATUS))); + } while ((s = mask & ddbreadl(dev, INTERRUPT_STATUS))); return IRQ_HANDLED; } -- 2.16.1
[PATCH v2 06/19] [media] ddbridge: request/free_irq using pci_irq_vector, enable MSI-X
From: Daniel SchellerInstead of trying to manage IRQ numbers on itself, utilise the pci_irq_vector() function to do this, which will take care of correct IRQ numbering for MSI and non-MSI IRQs. While at it, request and enable MSI-X interrupts for hardware (boards and cards) that support this. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-main.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index 77089081db1f..008be9066814 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -77,8 +77,8 @@ static void ddb_irq_exit(struct ddb *dev) { ddb_irq_disable(dev); if (dev->msi == 2) - free_irq(dev->pdev->irq + 1, dev); - free_irq(dev->pdev->irq, dev); + free_irq(pci_irq_vector(dev->pdev, 1), dev); + free_irq(pci_irq_vector(dev->pdev, 0), dev); } static void ddb_remove(struct pci_dev *pdev) @@ -105,7 +105,8 @@ static void ddb_irq_msi(struct ddb *dev, int nr) int stat; if (msi && pci_msi_enabled()) { - stat = pci_alloc_irq_vectors(dev->pdev, 1, nr, PCI_IRQ_MSI); + stat = pci_alloc_irq_vectors(dev->pdev, 1, nr, +PCI_IRQ_MSI | PCI_IRQ_MSIX); if (stat >= 1) { dev->msi = stat; dev_info(dev->dev, "using %d MSI interrupt(s)\n", @@ -137,21 +138,24 @@ static int ddb_irq_init(struct ddb *dev) if (dev->msi) irq_flag = 0; if (dev->msi == 2) { - stat = request_irq(dev->pdev->irq, ddb_irq_handler0, - irq_flag, "ddbridge", (void *)dev); + stat = request_irq(pci_irq_vector(dev->pdev, 0), + ddb_irq_handler0, irq_flag, "ddbridge", + (void *)dev); if (stat < 0) return stat; - stat = request_irq(dev->pdev->irq + 1, ddb_irq_handler1, - irq_flag, "ddbridge", (void *)dev); + stat = request_irq(pci_irq_vector(dev->pdev, 1), + ddb_irq_handler1, irq_flag, "ddbridge", + (void *)dev); if (stat < 0) { - free_irq(dev->pdev->irq, dev); + free_irq(pci_irq_vector(dev->pdev, 0), dev); return stat; } } else #endif { - stat = request_irq(dev->pdev->irq, ddb_irq_handler, - irq_flag, "ddbridge", (void *)dev); + stat = request_irq(pci_irq_vector(dev->pdev, 0), + ddb_irq_handler, irq_flag, "ddbridge", + (void *)dev); if (stat < 0) return stat; } -- 2.16.1
[PATCH v2 01/19] [media] dvb-frontends/stv0910: add init values for TSINSDELM/L
From: Daniel SchellerThe TSINSDEL registers were lacking initialisation in the stv0910 demod driver. Initialise them (both demods) in the probe() function. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/dvb-frontends/stv0910.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c index 52355c14fd64..f5b5ce971c0c 100644 --- a/drivers/media/dvb-frontends/stv0910.c +++ b/drivers/media/dvb-frontends/stv0910.c @@ -1220,6 +1220,12 @@ static int probe(struct stv *state) write_reg(state, RSTV0910_P1_I2CRPT, state->i2crpt); write_reg(state, RSTV0910_P2_I2CRPT, state->i2crpt); + write_reg(state, RSTV0910_P1_TSINSDELM, 0x17); + write_reg(state, RSTV0910_P1_TSINSDELL, 0xff); + + write_reg(state, RSTV0910_P2_TSINSDELM, 0x17); + write_reg(state, RSTV0910_P2_TSINSDELL, 0xff); + init_diseqc(state); return 0; } -- 2.16.1
[PATCH v2 04/19] [media] ddbridge: move ddb_wq and the wq+class initialisation to -core
From: Daniel SchellerMove the ddbridge module initialisation and cleanup code to ddbridge-core and set up the ddb_wq workqueue there, and create and destroy the ddb device class there aswell. Due to this, the prototypes for ddb_wq, ddb_class_create() and ddb_class_destroy() aren't required in ddbridge.h anymore, so remove them. Also, declare ddb_wq and the ddb_class_*() functions static. Picked up from the upstream dddvb-0.9.33 release. Signed-off-by: Daniel Scheller --- drivers/media/pci/ddbridge/ddbridge-core.c | 32 +++--- drivers/media/pci/ddbridge/ddbridge-main.c | 21 +++- drivers/media/pci/ddbridge/ddbridge.h | 7 ++- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 933046d03db5..fb9a2cb758e6 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -100,7 +100,7 @@ MODULE_PARM_DESC(stv0910_single, "use stv0910 cards as single demods"); static DEFINE_MUTEX(redirect_lock); -struct workqueue_struct *ddb_wq; +static struct workqueue_struct *ddb_wq; static struct ddb *ddbs[DDB_MAX_ADAPTER]; @@ -3055,7 +3055,7 @@ static struct class ddb_class = { .devnode= ddb_devnode, }; -int ddb_class_create(void) +static int ddb_class_create(void) { ddb_major = register_chrdev(0, DDB_NAME, _fops); if (ddb_major < 0) @@ -3065,7 +3065,7 @@ int ddb_class_create(void) return 0; } -void ddb_class_destroy(void) +static void ddb_class_destroy(void) { class_unregister(_class); unregister_chrdev(ddb_major, DDB_NAME); @@ -3337,3 +3337,29 @@ void ddb_unmap(struct ddb *dev) iounmap(dev->regs); vfree(dev); } + +int ddb_exit_ddbridge(int stage, int error) +{ + switch (stage) { + default: + case 2: + destroy_workqueue(ddb_wq); + /* fall-through */ + case 1: + ddb_class_destroy(); + break; + } + + return error; +} + +int ddb_init_ddbridge(void) +{ + if (ddb_class_create() < 0) + return -1; + ddb_wq = alloc_workqueue("ddbridge", 0, 0); + if (!ddb_wq) + return ddb_exit_ddbridge(1, -1); + + return 0; +} diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c index bde04dc39080..7088162af9d3 100644 --- a/drivers/media/pci/ddbridge/ddbridge-main.c +++ b/drivers/media/pci/ddbridge/ddbridge-main.c @@ -282,32 +282,25 @@ static struct pci_driver ddb_pci_driver = { static __init int module_init_ddbridge(void) { - int stat = -1; + int stat; pr_info("Digital Devices PCIE bridge driver " DDBRIDGE_VERSION ", Copyright (C) 2010-17 Digital Devices GmbH\n"); - if (ddb_class_create() < 0) - return -1; - ddb_wq = create_workqueue("ddbridge"); - if (!ddb_wq) - goto exit1; + stat = ddb_init_ddbridge(); + if (stat < 0) + return stat; stat = pci_register_driver(_pci_driver); if (stat < 0) - goto exit2; - return stat; -exit2: - destroy_workqueue(ddb_wq); -exit1: - ddb_class_destroy(); + ddb_exit_ddbridge(0, stat); + return stat; } static __exit void module_exit_ddbridge(void) { pci_unregister_driver(_pci_driver); - destroy_workqueue(ddb_wq); - ddb_class_destroy(); + ddb_exit_ddbridge(0, 0); } module_init(module_init_ddbridge); diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index e22e67d7e0fe..dbd5f551ce76 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -368,9 +368,6 @@ int ddbridge_flashread(struct ddb *dev, u32 link, u8 *buf, u32 addr, u32 len); // -/* ddbridge-main.c (modparams) */ -extern struct workqueue_struct *ddb_wq; - /* ddbridge-core.c */ void ddb_ports_detach(struct ddb *dev); void ddb_ports_release(struct ddb *dev); @@ -383,9 +380,9 @@ void ddb_ports_init(struct ddb *dev); int ddb_buffers_alloc(struct ddb *dev); int ddb_ports_attach(struct ddb *dev); int ddb_device_create(struct ddb *dev); -int ddb_class_create(void); -void ddb_class_destroy(void); int ddb_init(struct ddb *dev); void ddb_unmap(struct ddb *dev); +int ddb_exit_ddbridge(int stage, int error); +int ddb_init_ddbridge(void); #endif /* DDBRIDGE_H */ -- 2.16.1
[PATCH v2 00/19] dddvb/ddbridge-0.9.33
From: Daniel SchellerThis series brings all relevant changes from the upstream dddvb-0.9.33 driver package to the in-kernel ddbridge and stv0910, though a few changes were picked up and merged previously already. Summary of changes: * stv0910: initialisation fixes and fixed CNR reporting (uvalue vs. svalue) * ddbridge: general code move, cleanups and fixups * ddbridge: fixes and improvements to the IRQ setup and handling, and MSI-X support * ddbridge: configurable DMA buffers (via modparam) * ddbridge: dummy tuner option, useful for debugging and stress testing purposes * ddbridge: support for the new MCI card types, and namely the new MaxSX8 cards Patches were build-tested in their order and are bisect safe. Besides the modparam move, everything is picked up from dddvb-0.9.33. The series adds the new ddbridge-mci.[c|h] files. Here, SPDX headers were already put in place, but until things have been fully sorted out, the original GPL boiler plate is kept in place for now. Changes since v1: * The "stv0910: increase parallel TS output speed" commit was deleted, as it causes non-working cineS2V7 cards with older card/FPGA firmware. Please pick up and merge. Daniel Scheller (19): [media] dvb-frontends/stv0910: add init values for TSINSDELM/L [media] dvb-frontends/stv0910: fix CNR reporting in read_snr() [media] ddbridge: move modparams to ddbridge-core.c [media] ddbridge: move ddb_wq and the wq+class initialisation to -core [media] ddbridge: move MSI IRQ cleanup to a helper function [media] ddbridge: request/free_irq using pci_irq_vector, enable MSI-X [media] ddbridge: add helper for IRQ handler setup [media] ddbridge: add macros to handle IRQs in nibble and byte blocks [media] ddbridge: improve separated MSI IRQ handling [media] ddbridge: use spin_lock_irqsave() in output_work() [media] ddbridge: fix output buffer check [media] ddbridge: set devid entry for link 0 [media] ddbridge: make DMA buffer count and size modparam-configurable [media] ddbridge: support dummy tuners with 125MByte/s dummy data stream [media] ddbridge: initial support for MCI-based MaxSX8 cards [media] ddbridge/max: implement MCI/MaxSX8 attach function [media] ddbridge: add hardware defs and PCI IDs for MCI cards [media] ddbridge: recognize and attach the MaxSX8 cards [media] ddbridge: set driver version to 0.9.33-integrated drivers/media/dvb-frontends/stv0910.c | 8 +- drivers/media/pci/ddbridge/Kconfig | 1 + drivers/media/pci/ddbridge/Makefile| 2 +- drivers/media/pci/ddbridge/ddbridge-core.c | 299 +++- drivers/media/pci/ddbridge/ddbridge-hw.c | 11 + drivers/media/pci/ddbridge/ddbridge-i2c.c | 5 +- drivers/media/pci/ddbridge/ddbridge-main.c | 91 ++--- drivers/media/pci/ddbridge/ddbridge-max.c | 42 +++ drivers/media/pci/ddbridge/ddbridge-max.h | 1 + drivers/media/pci/ddbridge/ddbridge-mci.c | 550 + drivers/media/pci/ddbridge/ddbridge-mci.h | 152 drivers/media/pci/ddbridge/ddbridge.h | 50 +-- 12 files changed, 1029 insertions(+), 183 deletions(-) create mode 100644 drivers/media/pci/ddbridge/ddbridge-mci.c create mode 100644 drivers/media/pci/ddbridge/ddbridge-mci.h -- 2.16.1
Re: [RFCv11 PATCH 17/29] vb2: store userspace data in vb2_v4l2_buffer
Hi Hans, Thank you for the patch series ! I'm looking forwards to finding some time to try out this work. Just briefly scanning through the series, and I saw the minor issue below. Regards Kieran On 09/04/18 15:20, Hans Verkuil wrote: > From: Hans Verkuil> > The userspace-provided plane data needs to be stored in > vb2_v4l2_buffer. Currently this information is applied by > __fill_vb2_buffer() which is called by the core prepare_buf > and qbuf functions, but when using requests these functions > aren't called yet since the buffer won't be prepared until > the media request is actually queued. > > In the meantime this information has to be stored somewhere > and vb2_v4l2_buffer is a good place for it. > > The __fill_vb2_buffer callback now just copies the relevant > information from vb2_v4l2_buffer into the planes array. > > Signed-off-by: Hans Verkuil > --- > drivers/media/common/videobuf2/videobuf2-core.c | 25 +- > drivers/media/common/videobuf2/videobuf2-v4l2.c | 324 > +--- > drivers/media/dvb-core/dvb_vb2.c| 3 +- > include/media/videobuf2-core.h | 3 +- > include/media/videobuf2-v4l2.h | 2 + > 5 files changed, 197 insertions(+), 160 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > b/drivers/media/common/videobuf2/videobuf2-core.c > index d3f7bb33a54d..3d436ccb61f8 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -968,9 +968,8 @@ static int __prepare_mmap(struct vb2_buffer *vb, const > void *pb) Now that pb is unused here, should it be removed from the function arguments ? > { > int ret = 0; > > - if (pb) > - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > - vb, pb, vb->planes); > + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > + vb, vb->planes); > return ret ? ret : call_vb_qop(vb, buf_prepare, vb); > } > > @@ -988,12 +987,10 @@ static int __prepare_userptr(struct vb2_buffer *vb, > const void *pb) Same comment here > > memset(planes, 0, sizeof(planes[0]) * vb->num_planes); > /* Copy relevant information provided by the userspace */ > - if (pb) { > - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > - vb, pb, planes); > - if (ret) > - return ret; > - } > + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > + vb, planes); > + if (ret) > + return ret; > > for (plane = 0; plane < vb->num_planes; ++plane) { > /* Skip the plane if already verified */ > @@ -1104,12 +1101,10 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, > const void *pb) And here :D > > memset(planes, 0, sizeof(planes[0]) * vb->num_planes); > /* Copy relevant information provided by the userspace */ > - if (pb) { > - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > - vb, pb, planes); > - if (ret) > - return ret; > - } > + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, > + vb, planes); > + if (ret) > + return ret; > > for (plane = 0; plane < vb->num_planes; ++plane) { > struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c > b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 4e9c77f21858..bf7a3ba9fed0 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -154,9 +154,177 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer > *vb) > pr_warn("use the actual size instead.\n"); > } > > +static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct > v4l2_buffer *b) > +{ > + struct vb2_queue *q = vb->vb2_queue; > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); > + struct vb2_plane *planes = vbuf->planes; > + unsigned int plane; > + int ret; > + > + ret = __verify_length(vb, b); > + if (ret < 0) { > + dprintk(1, "plane parameters verification failed: %d\n", ret); > + return ret; > + } > + if (b->field == V4L2_FIELD_ALTERNATE && q->is_output) { > + /* > + * If the format's field is ALTERNATE, then the buffer's field > + * should be either TOP or BOTTOM, not ALTERNATE since that > + * makes no sense. The driver has to know whether the > + * buffer represents a top or a bottom field in order to > + * program any DMA correctly. Using ALTERNATE is wrong, since > + * that just says that it is either a top or a bottom field, > + * but not which of
[RFCv11 PATCH 06/29] media-request: add media_request_find
From: Hans VerkuilAdd media_request_find() to find a request based on the file descriptor. The caller has to call media_request_put() for the returned request since this function increments the refcount. Signed-off-by: Hans Verkuil --- drivers/media/media-request.c | 47 +++ include/media/media-request.h | 10 + 2 files changed, 57 insertions(+) diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c index 27739ff7cb09..02b620c81de5 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -207,6 +207,53 @@ static const struct file_operations request_fops = { .release = media_request_close, }; +/** + * media_request_find - Find a request based on the file descriptor + * @mdev: The media device + * @request: The request file handle + * + * Find and return the request associated with the given file descriptor, or + * an error if no such request exists. + * + * When the function returns a request it increases its reference count. The + * caller is responsible for releasing the reference by calling + * media_request_put() on the request. + */ +struct media_request * +media_request_find(struct media_device *mdev, int request_fd) +{ + struct file *filp; + struct media_request *req; + + if (!mdev || !mdev->ops || !mdev->ops->req_queue) + return ERR_PTR(-EPERM); + + filp = fget(request_fd); + if (!filp) + return ERR_PTR(-ENOENT); + + if (filp->f_op != _fops) + goto err_fput; + req = filp->private_data; + media_request_get(req); + + if (req->mdev != mdev) + goto err_kref_put; + + fput(filp); + + return req; + +err_kref_put: + media_request_put(req); + +err_fput: + fput(filp); + + return ERR_PTR(-ENOENT); +} +EXPORT_SYMBOL_GPL(media_request_find); + int media_request_alloc(struct media_device *mdev, struct media_request_alloc *alloc) { diff --git a/include/media/media-request.h b/include/media/media-request.h index 082c3cae04ac..033697d493cd 100644 --- a/include/media/media-request.h +++ b/include/media/media-request.h @@ -59,6 +59,9 @@ static inline void media_request_get(struct media_request *req) void media_request_put(struct media_request *req); void media_request_cancel(struct media_request *req); +struct media_request * +media_request_find(struct media_device *mdev, int request_fd); + int media_request_alloc(struct media_device *mdev, struct media_request_alloc *alloc); #else @@ -74,6 +77,12 @@ static inline void media_request_cancel(struct media_request *req) { } +static inline struct media_request * +media_request_find(struct media_device *mdev, int request_fd) +{ + return ERR_PTR(-ENOENT); +} + #endif struct media_request_object_ops { @@ -173,6 +182,7 @@ static inline void media_request_object_unbind(struct media_request_object *obj) static inline void media_request_object_complete(struct media_request_object *obj) { } + #endif #endif -- 2.16.3
[RFCv11 PATCH 09/29] videodev2.h: add V4L2_CTRL_FLAG_IN_REQUEST
From: Hans VerkuilSigned-off-by: Hans Verkuil --- include/uapi/linux/videodev2.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 6f41baa53787..d6e5c245bdcf 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1679,6 +1679,7 @@ struct v4l2_querymenu { #define V4L2_CTRL_FLAG_HAS_PAYLOAD 0x0100 #define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE0x0200 #define V4L2_CTRL_FLAG_MODIFY_LAYOUT 0x0400 +#define V4L2_CTRL_FLAG_IN_REQUEST 0x0800 /* Query flags, to be ORed with the control ID */ #define V4L2_CTRL_FLAG_NEXT_CTRL 0x8000 -- 2.16.3
[RFCv11 PATCH 10/29] v4l2-ctrls: v4l2_ctrl_add_handler: add from_other_dev
From: Hans VerkuilAdd a 'bool from_other_dev' argument: set to true if the two handlers refer to different devices (e.g. it is true when inheriting controls from a subdev into a main v4l2 bridge driver). This will be used later when implementing support for the request API since we need to skip such controls. TODO: check drivers/staging/media/imx/imx-media-fim.c change. Signed-off-by: Hans Verkuil Signed-off-by: Alexandre Courbot --- drivers/media/dvb-frontends/rtl2832_sdr.c| 5 +-- drivers/media/pci/bt8xx/bttv-driver.c| 2 +- drivers/media/pci/cx23885/cx23885-417.c | 2 +- drivers/media/pci/cx88/cx88-blackbird.c | 2 +- drivers/media/pci/cx88/cx88-video.c | 2 +- drivers/media/pci/saa7134/saa7134-empress.c | 4 +-- drivers/media/pci/saa7134/saa7134-video.c| 2 +- drivers/media/platform/exynos4-is/fimc-capture.c | 2 +- drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 +- drivers/media/platform/rcar_drif.c | 2 +- drivers/media/platform/soc_camera/soc_camera.c | 3 +- drivers/media/platform/vivid/vivid-ctrls.c | 46 drivers/media/usb/cx231xx/cx231xx-417.c | 2 +- drivers/media/usb/cx231xx/cx231xx-video.c| 4 +-- drivers/media/usb/msi2500/msi2500.c | 2 +- drivers/media/usb/tm6000/tm6000-video.c | 2 +- drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++--- drivers/media/v4l2-core/v4l2-device.c| 3 +- drivers/staging/media/imx/imx-media-dev.c| 2 +- drivers/staging/media/imx/imx-media-fim.c| 2 +- include/media/v4l2-ctrls.h | 8 - 21 files changed, 62 insertions(+), 49 deletions(-) diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c index c6e78d870ccd..6064d28224e8 100644 --- a/drivers/media/dvb-frontends/rtl2832_sdr.c +++ b/drivers/media/dvb-frontends/rtl2832_sdr.c @@ -1394,7 +1394,8 @@ static int rtl2832_sdr_probe(struct platform_device *pdev) case RTL2832_SDR_TUNER_E4000: v4l2_ctrl_handler_init(>hdl, 9); if (subdev) - v4l2_ctrl_add_handler(>hdl, subdev->ctrl_handler, NULL); + v4l2_ctrl_add_handler(>hdl, subdev->ctrl_handler, + NULL, true); break; case RTL2832_SDR_TUNER_R820T: case RTL2832_SDR_TUNER_R828D: @@ -1423,7 +1424,7 @@ static int rtl2832_sdr_probe(struct platform_device *pdev) v4l2_ctrl_handler_init(>hdl, 2); if (subdev) v4l2_ctrl_add_handler(>hdl, subdev->ctrl_handler, - NULL); + NULL, true); break; default: v4l2_ctrl_handler_init(>hdl, 0); diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c index f697698fe38d..cdcb36d8c5c3 100644 --- a/drivers/media/pci/bt8xx/bttv-driver.c +++ b/drivers/media/pci/bt8xx/bttv-driver.c @@ -4211,7 +4211,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id) /* register video4linux + input */ if (!bttv_tvcards[btv->c.type].no_video) { v4l2_ctrl_add_handler(>radio_ctrl_handler, hdl, - v4l2_ctrl_radio_filter); + v4l2_ctrl_radio_filter, false); if (btv->radio_ctrl_handler.error) { result = btv->radio_ctrl_handler.error; goto fail2; diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c index a71f3c7569ce..762823871c78 100644 --- a/drivers/media/pci/cx23885/cx23885-417.c +++ b/drivers/media/pci/cx23885/cx23885-417.c @@ -1527,7 +1527,7 @@ int cx23885_417_register(struct cx23885_dev *dev) dev->cxhdl.priv = dev; dev->cxhdl.func = cx23885_api_func; cx2341x_handler_set_50hz(>cxhdl, tsport->height == 576); - v4l2_ctrl_add_handler(>ctrl_handler, >cxhdl.hdl, NULL); + v4l2_ctrl_add_handler(>ctrl_handler, >cxhdl.hdl, NULL, false); /* Allocate and initialize V4L video device */ dev->v4l_device = cx23885_video_dev_alloc(tsport, diff --git a/drivers/media/pci/cx88/cx88-blackbird.c b/drivers/media/pci/cx88/cx88-blackbird.c index 0e0952e60795..39f69d89a663 100644 --- a/drivers/media/pci/cx88/cx88-blackbird.c +++ b/drivers/media/pci/cx88/cx88-blackbird.c @@ -1183,7 +1183,7 @@ static int cx8802_blackbird_probe(struct cx8802_driver *drv) err = cx2341x_handler_init(>cxhdl, 36); if (err) goto fail_core; - v4l2_ctrl_add_handler(>cxhdl.hdl, >video_hdl, NULL); + v4l2_ctrl_add_handler(>cxhdl.hdl, >video_hdl, NULL, false); /*
[RFCv11 PATCH 08/29] videodev2.h: add request_fd field to v4l2_ext_controls
From: Alexandre CourbotIf which is V4L2_CTRL_WHICH_REQUEST, then the request_fd field can be used to specify a request for the G/S/TRY_EXT_CTRLS ioctls. Signed-off-by: Alexandre Courbot --- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 5 - drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++--- include/uapi/linux/videodev2.h| 4 +++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 5198c9eeb348..0782b3666deb 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -732,7 +732,8 @@ struct v4l2_ext_controls32 { __u32 which; __u32 count; __u32 error_idx; - __u32 reserved[2]; + __s32 request_fd; + __u32 reserved[1]; compat_caddr_t controls; /* actually struct v4l2_ext_control32 * */ }; @@ -807,6 +808,7 @@ static int get_v4l2_ext_controls32(struct file *file, get_user(count, >count) || put_user(count, >count) || assign_in_user(>error_idx, >error_idx) || + assign_in_user(>request_fd, >request_fd) || copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) return -EFAULT; @@ -865,6 +867,7 @@ static int put_v4l2_ext_controls32(struct file *file, get_user(count, >count) || put_user(count, >count) || assign_in_user(>error_idx, >error_idx) || + assign_in_user(>request_fd, >request_fd) || copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved)) || get_user(kcontrols, >controls)) return -EFAULT; diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index f48c505550e0..9ce23e23c5bf 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -553,8 +553,8 @@ static void v4l_print_ext_controls(const void *arg, bool write_only) const struct v4l2_ext_controls *p = arg; int i; - pr_cont("which=0x%x, count=%d, error_idx=%d", - p->which, p->count, p->error_idx); + pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d", + p->which, p->count, p->error_idx, p->request_fd); for (i = 0; i < p->count; i++) { if (!p->controls[i].size) pr_cont(", id/val=0x%x/0x%x", @@ -870,7 +870,7 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int allow_priv) __u32 i; /* zero the reserved fields */ - c->reserved[0] = c->reserved[1] = 0; + c->reserved[0] = 0; for (i = 0; i < c->count; i++) c->controls[i].reserved2[0] = 0; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 600877be5c22..6f41baa53787 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1592,7 +1592,8 @@ struct v4l2_ext_controls { }; __u32 count; __u32 error_idx; - __u32 reserved[2]; + __s32 request_fd; + __u32 reserved[1]; struct v4l2_ext_control *controls; }; @@ -1605,6 +1606,7 @@ struct v4l2_ext_controls { #define V4L2_CTRL_MAX_DIMS (4) #define V4L2_CTRL_WHICH_CUR_VAL 0 #define V4L2_CTRL_WHICH_DEF_VAL 0x0f00 +#define V4L2_CTRL_WHICH_REQUEST 0x0f01 enum v4l2_ctrl_type { V4L2_CTRL_TYPE_INTEGER = 1, -- 2.16.3
[RFCv11 PATCH 13/29] v4l2-ctrls: use ref in helper instead of ctrl
From: Hans VerkuilThe next patch needs the reference to a control instead of the control itself, so change struct v4l2_ctrl_helper accordingly. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 3c1b00baa8d0..da4cc1485dc4 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -37,8 +37,8 @@ struct v4l2_ctrl_helper { /* Pointer to the control reference of the master control */ struct v4l2_ctrl_ref *mref; - /* The control corresponding to the v4l2_ext_control ID field. */ - struct v4l2_ctrl *ctrl; + /* The control ref corresponding to the v4l2_ext_control ID field. */ + struct v4l2_ctrl_ref *ref; /* v4l2_ext_control index of the next control belonging to the same cluster, or 0 if there isn't any. */ u32 next; @@ -2887,6 +2887,7 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl, ref = find_ref_lock(hdl, id); if (ref == NULL) return -EINVAL; + h->ref = ref; ctrl = ref->ctrl; if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED) return -EINVAL; @@ -2909,7 +2910,6 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl, } /* Store the ref to the master control of the cluster */ h->mref = ref; - h->ctrl = ctrl; /* Initially set next to 0, meaning that there is no other control in this helper array belonging to the same cluster */ @@ -2994,7 +2994,7 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs cs->error_idx = cs->count; for (i = 0; !ret && i < cs->count; i++) - if (helpers[i].ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY) + if (helpers[i].ref->ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY) ret = -EACCES; for (i = 0; !ret && i < cs->count; i++) { @@ -3029,7 +3029,7 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs do { ret = ctrl_to_user(cs->controls + idx, - helpers[idx].ctrl); + helpers[idx].ref->ctrl); idx = helpers[idx].next; } while (!ret && idx); } @@ -3168,7 +3168,7 @@ static int validate_ctrls(struct v4l2_ext_controls *cs, cs->error_idx = cs->count; for (i = 0; i < cs->count; i++) { - struct v4l2_ctrl *ctrl = helpers[i].ctrl; + struct v4l2_ctrl *ctrl = helpers[i].ref->ctrl; union v4l2_ctrl_ptr p_new; cs->error_idx = i; @@ -3280,7 +3280,7 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, do { /* Check if the auto control is part of the list, and remember the new value. */ - if (helpers[tmp_idx].ctrl == master) + if (helpers[tmp_idx].ref->ctrl == master) new_auto_val = cs->controls[tmp_idx].value; tmp_idx = helpers[tmp_idx].next; } while (tmp_idx); @@ -3293,7 +3293,7 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, /* Copy the new caller-supplied control values. user_to_new() sets 'is_new' to 1. */ do { - struct v4l2_ctrl *ctrl = helpers[idx].ctrl; + struct v4l2_ctrl *ctrl = helpers[idx].ref->ctrl; ret = user_to_new(cs->controls + idx, ctrl); if (!ret && ctrl->is_ptr) @@ -3309,7 +3309,7 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, idx = i; do { ret = new_to_user(cs->controls + idx, - helpers[idx].ctrl); + helpers[idx].ref->ctrl); idx = helpers[idx].next; } while (!ret && idx); } -- 2.16.3
[RFCv11 PATCH 12/29] v4l2-ctrls: alloc memory for p_req
From: Hans VerkuilTo store request data the handler_new_ref() allocates memory for it if needed. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index d09f49530d9e..3c1b00baa8d0 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1997,13 +1997,18 @@ EXPORT_SYMBOL(v4l2_ctrl_find); /* Allocate a new v4l2_ctrl_ref and hook it into the handler. */ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, struct v4l2_ctrl *ctrl, - bool from_other_dev) + struct v4l2_ctrl_ref **ctrl_ref, + bool from_other_dev, bool allocate_req) { struct v4l2_ctrl_ref *ref; struct v4l2_ctrl_ref *new_ref; u32 id = ctrl->id; u32 class_ctrl = V4L2_CTRL_ID2WHICH(id) | 1; int bucket = id % hdl->nr_of_buckets; /* which bucket to use */ + unsigned int sz_extra = 0; + + if (ctrl_ref) + *ctrl_ref = NULL; /* * Automatically add the control class if it is not yet present and @@ -2017,11 +2022,16 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, if (hdl->error) return hdl->error; - new_ref = kzalloc(sizeof(*new_ref), GFP_KERNEL); + if (allocate_req) + sz_extra = ctrl->elems * ctrl->elem_size; + new_ref = kzalloc(sizeof(*new_ref) + sz_extra, GFP_KERNEL); if (!new_ref) return handler_set_err(hdl, -ENOMEM); new_ref->ctrl = ctrl; new_ref->from_other_dev = from_other_dev; + if (sz_extra) + new_ref->p_req.p = _ref[1]; + if (ctrl->handler == hdl) { /* By default each control starts in a cluster of its own. new_ref->ctrl is basically a cluster array with one @@ -2061,6 +2071,8 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, /* Insert the control node in the hash */ new_ref->next = hdl->buckets[bucket]; hdl->buckets[bucket] = new_ref; + if (ctrl_ref) + *ctrl_ref = new_ref; unlock: mutex_unlock(hdl->lock); @@ -2202,7 +2214,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, ctrl->type_ops->init(ctrl, idx, ctrl->p_new); } - if (handler_new_ref(hdl, ctrl, false)) { + if (handler_new_ref(hdl, ctrl, NULL, false, false)) { kvfree(ctrl); return NULL; } @@ -2395,7 +2407,7 @@ int v4l2_ctrl_add_handler(struct v4l2_ctrl_handler *hdl, /* Filter any unwanted controls */ if (filter && !filter(ctrl)) continue; - ret = handler_new_ref(hdl, ctrl, from_other_dev); + ret = handler_new_ref(hdl, ctrl, NULL, from_other_dev, false); if (ret) break; } -- 2.16.3
[RFCv11 PATCH 11/29] v4l2-ctrls: prepare internal structs for request API
From: Hans VerkuilEmbed and initialize a media_request_object in struct v4l2_ctrl_handler. Add a p_req field to struct v4l2_ctrl_ref that will store the request value. Signed-off-by: Hans Verkuil Signed-off-by: Alexandre Courbot --- drivers/media/v4l2-core/v4l2-ctrls.c | 1 + include/media/v4l2-ctrls.h | 7 +++ 2 files changed, 8 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index aa1dd2015e84..d09f49530d9e 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1880,6 +1880,7 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl, sizeof(hdl->buckets[0]), GFP_KERNEL | __GFP_ZERO); hdl->error = hdl->buckets ? 0 : -ENOMEM; + media_request_object_init(>req_obj); return hdl->error; } EXPORT_SYMBOL(v4l2_ctrl_handler_init_class); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index f8faa54b5e7e..89a985607126 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -20,6 +20,7 @@ #include #include #include +#include /* forward references */ struct file; @@ -249,6 +250,8 @@ struct v4l2_ctrl { * ``prepare_ext_ctrls`` function at ``v4l2-ctrl.c``. * @from_other_dev: If true, then @ctrl was defined in another * device then the v4l2_ctrl_handler. + * @p_req: The request value. Only used if the control handler + * is bound to a media request. * * Each control handler has a list of these refs. The list_head is used to * keep a sorted-by-control-ID list of all controls, while the next pointer @@ -260,6 +263,7 @@ struct v4l2_ctrl_ref { struct v4l2_ctrl *ctrl; struct v4l2_ctrl_helper *helper; bool from_other_dev; + union v4l2_ctrl_ptr p_req; }; /** @@ -283,6 +287,8 @@ struct v4l2_ctrl_ref { * @notify_priv: Passed as argument to the v4l2_ctrl notify callback. * @nr_of_buckets: Total number of buckets in the array. * @error: The error code of the first failed control addition. + * @req_obj: The media_request_object, used to link into a + * media_request. */ struct v4l2_ctrl_handler { struct mutex _lock; @@ -295,6 +301,7 @@ struct v4l2_ctrl_handler { void *notify_priv; u16 nr_of_buckets; int error; + struct media_request_object req_obj; }; /** -- 2.16.3
[RFCv11 PATCH 04/29] media-request: core request support
From: Hans VerkuilImplement the core of the media request processing. Drivers can bind request objects to a request. These objects can then be marked completed if the driver finished using them, or just be unbound if the results do not need to be kept (e.g. in the case of buffers). Once all objects that were added are either unbound or completed, the request is marked 'complete' and a POLLPRI signal is sent via poll. Both requests and request objects are refcounted. While a request is queued its refcount is incremented (since it is in use by a driver). Once it is completed the refcount is decremented. When the user closes the request file descriptor the refcount is also decremented. Once it reaches 0 all request objects in the request are unbound and put() and the request itself is freed. Signed-off-by: Hans Verkuil --- drivers/media/media-request.c | 284 +- include/media/media-request.h | 156 +++ 2 files changed, 439 insertions(+), 1 deletion(-) diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c index ead78613fdbe..dffc290e4ada 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -16,8 +16,290 @@ #include #include +static const char * const request_state[] = { + "idle", + "queueing", + "queued", + "complete", + "cleaning", +}; + +static const char * +media_request_state_str(enum media_request_state state) +{ + if (WARN_ON(state >= ARRAY_SIZE(request_state))) + return "unknown"; + return request_state[state]; +} + +static void media_request_clean(struct media_request *req) +{ + struct media_request_object *obj, *obj_safe; + + WARN_ON(req->state != MEDIA_REQUEST_STATE_CLEANING); + + list_for_each_entry_safe(obj, obj_safe, >objects, list) { + media_request_object_unbind(obj); + media_request_object_put(obj); + } + + req->num_incomplete_objects = 0; + wake_up_interruptible(>poll_wait); +} + +static void media_request_release(struct kref *kref) +{ + struct media_request *req = + container_of(kref, struct media_request, kref); + struct media_device *mdev = req->mdev; + unsigned long flags; + + dev_dbg(mdev->dev, "request: release %s\n", req->debug_str); + + spin_lock_irqsave(>lock, flags); + req->state = MEDIA_REQUEST_STATE_CLEANING; + spin_unlock_irqrestore(>lock, flags); + + media_request_clean(req); + + if (mdev->ops->req_free) + mdev->ops->req_free(req); + else + kfree(req); +} + +void media_request_put(struct media_request *req) +{ + kref_put(>kref, media_request_release); +} +EXPORT_SYMBOL_GPL(media_request_put); + +void media_request_cancel(struct media_request *req) +{ + struct media_request_object *obj, *obj_safe; + + if (req->state != MEDIA_REQUEST_STATE_QUEUED) + return; + + list_for_each_entry_safe(obj, obj_safe, >objects, list) + if (obj->ops->cancel) + obj->ops->cancel(obj); +} +EXPORT_SYMBOL_GPL(media_request_cancel); + +static int media_request_close(struct inode *inode, struct file *filp) +{ + struct media_request *req = filp->private_data; + + media_request_put(req); + return 0; +} + +static unsigned int media_request_poll(struct file *filp, + struct poll_table_struct *wait) +{ + struct media_request *req = filp->private_data; + unsigned long flags; + enum media_request_state state; + + if (!(poll_requested_events(wait) & POLLPRI)) + return 0; + + spin_lock_irqsave(>lock, flags); + state = req->state; + spin_unlock_irqrestore(>lock, flags); + + if (state == MEDIA_REQUEST_STATE_COMPLETE) + return POLLPRI; + if (state == MEDIA_REQUEST_STATE_IDLE) + return POLLERR; + + poll_wait(filp, >poll_wait, wait); + return 0; +} + +static long media_request_ioctl(struct file *filp, unsigned int cmd, + unsigned long __arg) +{ + return -ENOIOCTLCMD; +} + +static const struct file_operations request_fops = { + .owner = THIS_MODULE, + .poll = media_request_poll, + .unlocked_ioctl = media_request_ioctl, + .release = media_request_close, +}; + int media_request_alloc(struct media_device *mdev, struct media_request_alloc *alloc) { - return -ENOMEM; + struct media_request *req; + struct file *filp; + char comm[TASK_COMM_LEN]; + int fd; + int ret; + + fd = get_unused_fd_flags(O_CLOEXEC); + if (fd < 0) + return fd; + + filp = anon_inode_getfile("request", _fops, NULL, O_CLOEXEC); + if (IS_ERR(filp)) { + ret =
[RFCv11 PATCH 07/29] media-request: add media_request_object_find
From: Hans VerkuilAdd media_request_object_find to find a request object inside a request based on ops and/or priv values. Objects of the same type (vb2 buffer, control handler) will have the same ops value. And objects that refer to the same 'parent' object (e.g. the v4l2_ctrl_handler that has the current driver state) will have the same priv value. The caller has to call media_request_object_put() for the returned object since this function increments the refcount. Signed-off-by: Hans Verkuil --- drivers/media/media-request.c | 26 ++ include/media/media-request.h | 25 + 2 files changed, 51 insertions(+) diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c index 02b620c81de5..415f7e31019d 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -322,6 +322,32 @@ static void media_request_object_release(struct kref *kref) obj->ops->release(obj); } +struct media_request_object * +media_request_object_find(struct media_request *req, + const struct media_request_object_ops *ops, + void *priv) +{ + struct media_request_object *obj; + struct media_request_object *found = NULL; + unsigned long flags; + + if (!ops && !priv) + return NULL; + + spin_lock_irqsave(>lock, flags); + list_for_each_entry(obj, >objects, list) { + if ((!ops || obj->ops == ops) && + (!priv || obj->priv == priv)) { + media_request_object_get(obj); + found = obj; + break; + } + } + spin_unlock_irqrestore(>lock, flags); + return found; +} +EXPORT_SYMBOL_GPL(media_request_object_find); + void media_request_object_put(struct media_request_object *obj) { kref_put(>kref, media_request_object_release); diff --git a/include/media/media-request.h b/include/media/media-request.h index 033697d493cd..ea990c8f76bc 100644 --- a/include/media/media-request.h +++ b/include/media/media-request.h @@ -130,6 +130,23 @@ static inline void media_request_object_get(struct media_request_object *obj) */ void media_request_object_put(struct media_request_object *obj); +/** + * media_request_object_find - Find an object in a request + * + * @ops: Find an object with this ops value, may be NULL. + * @priv: Find an object with this priv value, may be NULL. + * + * At least one of @ops and @priv must be non-NULL. If one of + * these is NULL, then skip checking for that field. + * + * Returns NULL if not found or the object (the refcount is increased + * in that case). + */ +struct media_request_object * +media_request_object_find(struct media_request *req, + const struct media_request_object_ops *ops, + void *priv); + /** * media_request_object_init - Initialise a media request object * @@ -162,6 +179,14 @@ static inline void media_request_object_put(struct media_request_object *obj) { } +static inline struct media_request_object * +media_request_object_find(struct media_request *req, + const struct media_request_object_ops *ops, + void *priv) +{ + return NULL; +} + static inline void media_request_object_init(struct media_request_object *obj) { obj->ops = NULL; -- 2.16.3
[RFCv11 PATCH 05/29] media-request: add request ioctls
From: Hans VerkuilImplement the MEDIA_REQUEST_IOC_QUEUE and MEDIA_REQUEST_IOC_REINIT ioctls. Signed-off-by: Hans Verkuil --- drivers/media/media-request.c | 80 +-- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c index dffc290e4ada..27739ff7cb09 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -118,10 +118,86 @@ static unsigned int media_request_poll(struct file *filp, return 0; } +static long media_request_ioctl_queue(struct media_request *req) +{ + struct media_device *mdev = req->mdev; + unsigned long flags; + int ret = 0; + + dev_dbg(mdev->dev, "request: queue %s\n", req->debug_str); + + spin_lock_irqsave(>lock, flags); + if (req->state != MEDIA_REQUEST_STATE_IDLE) { + dev_dbg(mdev->dev, + "request: unable to queue %s, request in state %s\n", + req->debug_str, media_request_state_str(req->state)); + spin_unlock_irqrestore(>lock, flags); + return -EINVAL; + } + req->state = MEDIA_REQUEST_STATE_QUEUEING; + + spin_unlock_irqrestore(>lock, flags); + + /* +* Ensure the request that is validated will be the one that gets queued +* next by serialising the queueing process. +*/ + mutex_lock(>req_queue_mutex); + + ret = mdev->ops->req_queue(req); + spin_lock_irqsave(>lock, flags); + req->state = ret ? MEDIA_REQUEST_STATE_IDLE : MEDIA_REQUEST_STATE_QUEUED; + spin_unlock_irqrestore(>lock, flags); + mutex_unlock(>req_queue_mutex); + + if (ret) { + dev_dbg(mdev->dev, "request: can't queue %s (%d)\n", + req->debug_str, ret); + } else { + media_request_get(req); + } + + return ret; +} + +static long media_request_ioctl_reinit(struct media_request *req) +{ + struct media_device *mdev = req->mdev; + unsigned long flags; + + spin_lock_irqsave(>lock, flags); + if (req->state != MEDIA_REQUEST_STATE_IDLE && + req->state != MEDIA_REQUEST_STATE_COMPLETE) { + dev_dbg(mdev->dev, + "request: %s not in idle or complete state, cannot reinit\n", + req->debug_str); + spin_unlock_irqrestore(>lock, flags); + return -EINVAL; + } + req->state = MEDIA_REQUEST_STATE_CLEANING; + spin_unlock_irqrestore(>lock, flags); + + media_request_clean(req); + + spin_lock_irqsave(>lock, flags); + req->state = MEDIA_REQUEST_STATE_IDLE; + spin_unlock_irqrestore(>lock, flags); + return 0; +} + static long media_request_ioctl(struct file *filp, unsigned int cmd, - unsigned long __arg) + unsigned long arg) { - return -ENOIOCTLCMD; + struct media_request *req = filp->private_data; + + switch (cmd) { + case MEDIA_REQUEST_IOC_QUEUE: + return media_request_ioctl_queue(req); + case MEDIA_REQUEST_IOC_REINIT: + return media_request_ioctl_reinit(req); + default: + return -ENOIOCTLCMD; + } } static const struct file_operations request_fops = { -- 2.16.3
[RFCv11 PATCH 24/29] Documentation: v4l: document request API
From: Alexandre CourbotDocument the request API for V4L2 devices, and amend the documentation of system calls influenced by it. Signed-off-by: Alexandre Courbot --- Documentation/media/uapi/v4l/buffer.rst| 19 +- Documentation/media/uapi/v4l/common.rst| 1 + Documentation/media/uapi/v4l/request-api.rst | 199 + Documentation/media/uapi/v4l/user-func.rst | 1 + .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 22 ++- .../media/uapi/v4l/vidioc-new-request.rst | 64 +++ Documentation/media/uapi/v4l/vidioc-qbuf.rst | 8 + 7 files changed, 308 insertions(+), 6 deletions(-) create mode 100644 Documentation/media/uapi/v4l/request-api.rst create mode 100644 Documentation/media/uapi/v4l/vidioc-new-request.rst diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index e2c85ddc990b..e23eae12905c 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -306,10 +306,13 @@ struct v4l2_buffer - A place holder for future extensions. Drivers and applications must set this to 0. * - __u32 - - ``reserved`` + - ``request_fd`` - - - A place holder for future extensions. Drivers and applications - must set this to 0. + - The file descriptor of the request to queue the buffer to. If specified +and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will be + queued to that request. This is set by the user when calling + :ref:`VIDIOC_QBUF` and :ref:`VIDIOC_PREPARE_BUF` and ignored by other + ioctls. @@ -514,6 +517,11 @@ Buffer Flags streaming may continue as normal and the buffer may be reused normally. Drivers set this flag when the ``VIDIOC_DQBUF`` ioctl is called. +* .. _`V4L2-BUF-FLAG-IN-REQUEST`: + + - ``V4L2_BUF_FLAG_IN_REQUEST`` + - 0x0080 + - This buffer is part of a request the hasn't been queued yet. * .. _`V4L2-BUF-FLAG-KEYFRAME`: - ``V4L2_BUF_FLAG_KEYFRAME`` @@ -589,6 +597,11 @@ Buffer Flags the format. Any Any subsequent call to the :ref:`VIDIOC_DQBUF ` ioctl will not block anymore, but return an ``EPIPE`` error code. +* .. _`V4L2-BUF-FLAG-REQUEST-FD`: + + - ``V4L2_BUF_FLAG_REQUEST_FD`` + - 0x0080 + - The ``request_fd`` field contains a valid file descriptor. * .. _`V4L2-BUF-FLAG-TIMESTAMP-MASK`: - ``V4L2_BUF_FLAG_TIMESTAMP_MASK`` diff --git a/Documentation/media/uapi/v4l/common.rst b/Documentation/media/uapi/v4l/common.rst index 13f2ed3fc5a6..a4aa0059d45a 100644 --- a/Documentation/media/uapi/v4l/common.rst +++ b/Documentation/media/uapi/v4l/common.rst @@ -44,3 +44,4 @@ applicable to all devices. crop selection-api streaming-par +request-api diff --git a/Documentation/media/uapi/v4l/request-api.rst b/Documentation/media/uapi/v4l/request-api.rst new file mode 100644 index ..0c1f2896e197 --- /dev/null +++ b/Documentation/media/uapi/v4l/request-api.rst @@ -0,0 +1,199 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _media-request-api: + +Request API +=== + +The Request API has been designed to allow V4L2 to deal with requirements of +modern devices (stateless codecs, MIPI cameras, ...) and APIs (Android Codec +v2). One such requirement is the ability for devices belonging to the same +pipeline to reconfigure and collaborate closely on a per-frame basis. Another is +efficient support of stateless codecs, which need per-frame controls to be set +asynchronously in order to be efficiently used. + +Supporting these features without the Request API is possible but terribly +inefficient: user-space would have to flush all activity on the media pipeline, +reconfigure it for the next frame, queue the buffers to be processed with that +configuration, and wait until they are all available for dequeing before +considering the next frame. This defeats the purpose of having buffer queues +since in practice only one buffer would be queued at a time. + +The Request API allows a specific configuration of the pipeline (media +controller topology + controls for each device) to be associated with specific +buffers. The parameters are applied by each participating device as buffers +associated to a request flow in. This allows user-space to schedule several +tasks ("requests") with different parameters in advance, knowing that the +parameters will be applied when needed to get the expected result. Controls +values at the time of request completion are also available for reading. + +Usage += + +The Request API is used on top of standard media controller and V4L2 calls, +which are augmented with an extra ``request_fd`` parameter. Request themselves +are allocated from either a supporting V4L2 device node, or a supporting media +controller node. The origin of
[RFCv11 PATCH 23/29] videobuf2-v4l2: export request_fd
From: Hans VerkuilRequested by Sakari Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 -- include/media/videobuf2-v4l2.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 3d0c74bb4220..7b79149b7fae 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -184,6 +184,7 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b return -EINVAL; } vbuf->sequence = 0; + vbuf->request_fd = -1; if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { switch (b->memory) { @@ -391,6 +392,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md } *p_req = req; + vbuf->request_fd = b->request_fd; return 0; } @@ -496,9 +498,9 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) if (vb2_buffer_in_use(q, vb)) b->flags |= V4L2_BUF_FLAG_MAPPED; - if (vb->req_obj.req) { + if (vbuf->request_fd >= 0) { b->flags |= V4L2_BUF_FLAG_REQUEST_FD; - b->request_fd = -1; + b->request_fd = vbuf->request_fd; } if (!q->is_output && diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h index 0baa3023d7ad..d3ee1f28e197 100644 --- a/include/media/videobuf2-v4l2.h +++ b/include/media/videobuf2-v4l2.h @@ -32,6 +32,7 @@ * v4l2_field. * @timecode: frame timecode. * @sequence: sequence count of this frame. + * @request_fd:the request_fd associated with this buffer * @planes:plane information (userptr/fd, length, bytesused, data_offset). * * Should contain enough information to be able to cover all the fields @@ -44,6 +45,7 @@ struct vb2_v4l2_buffer { __u32 field; struct v4l2_timecodetimecode; __u32 sequence; + __s32 request_fd; struct vb2_planeplanes[VB2_MAX_PLANES]; }; -- 2.16.3
[RFCv11 PATCH 25/29] media: vim2m: add media device
From: Alexandre CourbotRequest API requires a media node. Add one to the vim2m driver so we can use requests with it. This probably needs a bit more work to correctly represent m2m hardware in the media topology. Signed-off-by: Alexandre Courbot --- drivers/media/platform/vim2m.c | 43 +- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c index 065483e62db4..ef970434af13 100644 --- a/drivers/media/platform/vim2m.c +++ b/drivers/media/platform/vim2m.c @@ -140,6 +140,10 @@ static struct vim2m_fmt *find_format(struct v4l2_format *f) struct vim2m_dev { struct v4l2_device v4l2_dev; struct video_device vfd; +#ifdef CONFIG_MEDIA_CONTROLLER + struct media_device mdev; + struct media_padpad[2]; +#endif atomic_tnum_inst; struct mutexdev_mutex; @@ -1000,11 +1004,6 @@ static int vim2m_probe(struct platform_device *pdev) return -ENOMEM; spin_lock_init(>irqlock); - - ret = v4l2_device_register(>dev, >v4l2_dev); - if (ret) - return ret; - atomic_set(>num_inst, 0); mutex_init(>dev_mutex); @@ -1013,6 +1012,22 @@ static int vim2m_probe(struct platform_device *pdev) vfd->lock = >dev_mutex; vfd->v4l2_dev = >v4l2_dev; +#ifdef CONFIG_MEDIA_CONTROLLER + dev->mdev.dev = >dev; + strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model)); + media_device_init(>mdev); + dev->v4l2_dev.mdev = >mdev; + dev->pad[0].flags = MEDIA_PAD_FL_SINK; + dev->pad[1].flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(>entity, 2, dev->pad); + if (ret) + return ret; +#endif + + ret = v4l2_device_register(>dev, >v4l2_dev); + if (ret) + goto unreg_media; + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); if (ret) { v4l2_err(>v4l2_dev, "Failed to register video device\n"); @@ -1034,6 +1049,13 @@ static int vim2m_probe(struct platform_device *pdev) goto err_m2m; } +#ifdef CONFIG_MEDIA_CONTROLLER + /* Register the media device node */ + ret = media_device_register(>mdev); + if (ret) + goto err_m2m; +#endif + return 0; err_m2m: @@ -1041,6 +1063,10 @@ static int vim2m_probe(struct platform_device *pdev) video_unregister_device(>vfd); unreg_dev: v4l2_device_unregister(>v4l2_dev); +unreg_media: +#ifdef CONFIG_MEDIA_CONTROLLER + media_device_unregister(>mdev); +#endif return ret; } @@ -1050,6 +1076,13 @@ static int vim2m_remove(struct platform_device *pdev) struct vim2m_dev *dev = platform_get_drvdata(pdev); v4l2_info(>v4l2_dev, "Removing " MEM2MEM_NAME); + +#ifdef CONFIG_MEDIA_CONTROLLER + if (media_devnode_is_registered(dev->mdev.devnode)) + media_device_unregister(>mdev); + media_device_cleanup(>mdev); +#endif + v4l2_m2m_release(dev->m2m_dev); del_timer_sync(>timer); video_unregister_device(>vfd); -- 2.16.3
[RFCv11 PATCH 20/29] videobuf2-v4l2: integrate with media requests
From: Hans VerkuilThis implements the V4L2 part of the request support. The main change is that vb2_qbuf and vb2_prepare_buf now have a new media_device pointer. This required changes to several drivers that did not use the vb2_ioctl_qbuf/prepare_buf helper functions. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 84 drivers/media/platform/omap3isp/ispvideo.c | 2 +- drivers/media/platform/s3c-camif/camif-capture.c | 4 +- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 4 +- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 4 +- drivers/media/platform/soc_camera/soc_camera.c | 4 +- drivers/media/usb/uvc/uvc_queue.c| 5 +- drivers/media/usb/uvc/uvc_v4l2.c | 3 +- drivers/media/usb/uvc/uvcvideo.h | 1 + drivers/media/v4l2-core/v4l2-mem2mem.c | 7 +- drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +- drivers/staging/media/omap4iss/iss_video.c | 3 +- drivers/usb/gadget/function/uvc_queue.c | 2 +- include/media/videobuf2-v4l2.h | 12 +++- 14 files changed, 106 insertions(+), 32 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index b8d370b97cca..73c1fd4da58a 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -40,10 +41,12 @@ module_param(debug, int, 0644); pr_info("vb2-v4l2: %s: " fmt, __func__, ## arg); \ } while (0) -/* Flags that are set by the vb2 core */ +/* Flags that are set by us */ #define V4L2_BUFFER_MASK_FLAGS (V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED | \ V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \ V4L2_BUF_FLAG_PREPARED | \ +V4L2_BUF_FLAG_IN_REQUEST | \ +V4L2_BUF_FLAG_REQUEST_FD | \ V4L2_BUF_FLAG_TIMESTAMP_MASK) /* Output buffer flags that should be passed on to the driver */ #define V4L2_BUFFER_OUT_FLAGS (V4L2_BUF_FLAG_PFRAME | V4L2_BUF_FLAG_BFRAME | \ @@ -318,13 +321,17 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b return 0; } -static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b, - const char *opname) +static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, + struct v4l2_buffer *b, + const char *opname, + struct media_request **p_req) { + struct media_request *req; struct vb2_v4l2_buffer *vbuf; struct vb2_buffer *vb; int ret; + *p_req = NULL; if (b->type != q->type) { dprintk(1, "%s: invalid buffer type\n", opname); return -EINVAL; @@ -354,7 +361,38 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b, /* Copy relevant information provided by the userspace */ memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes); - return vb2_fill_vb2_v4l2_buffer(vb, b); + ret = vb2_fill_vb2_v4l2_buffer(vb, b); + if (ret) + return ret; + + if (!(b->flags & V4L2_BUF_FLAG_REQUEST_FD)) + return 0; + + if (vb->state != VB2_BUF_STATE_DEQUEUED) { + dprintk(1, "%s: buffer is not in dequeued state\n", opname); + return -EINVAL; + } + + if (b->request_fd < 0) { + dprintk(1, "%s: request_fd < 0\n", opname); + return -EINVAL; + } + + req = media_request_find(mdev, b->request_fd); + if (IS_ERR(req)) { + dprintk(1, "%s: invalid request_fd\n", opname); + return PTR_ERR(req); + } + + if (req->state != MEDIA_REQUEST_STATE_IDLE) { + dprintk(1, "%s: request is not idle\n", opname); + media_request_put(req); + return -EBUSY; + } + + *p_req = req; + + return 0; } /* @@ -437,6 +475,9 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) case VB2_BUF_STATE_ACTIVE: b->flags |= V4L2_BUF_FLAG_QUEUED; break; + case VB2_BUF_STATE_IN_REQUEST: + b->flags |= V4L2_BUF_FLAG_IN_REQUEST; + break; case VB2_BUF_STATE_ERROR: b->flags |= V4L2_BUF_FLAG_ERROR; /* fall through */ @@ -455,6 +496,10 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) if (vb2_buffer_in_use(q, vb)) b->flags |=
[RFCv11 PATCH 01/29] v4l2-device.h: always expose mdev
From: Hans VerkuilThe mdev field is only present if CONFIG_MEDIA_CONTROLLER is set. But since we will need to pass the media_device to vb2 snd the control framework it is very convenient to just make this field available all the time. If CONFIG_MEDIA_CONTROLLER is not set, then it will just be NULL. Signed-off-by: Hans Verkuil --- include/media/v4l2-device.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h index 0c9e4da55499..b330e4a08a6b 100644 --- a/include/media/v4l2-device.h +++ b/include/media/v4l2-device.h @@ -33,7 +33,7 @@ struct v4l2_ctrl_handler; * struct v4l2_device - main struct to for V4L2 device drivers * * @dev: pointer to struct device. - * @mdev: pointer to struct media_device + * @mdev: pointer to struct media_device, may be NULL. * @subdevs: used to keep track of the registered subdevs * @lock: lock this struct; can be used by the driver as well * if this struct is embedded into a larger struct. @@ -58,9 +58,7 @@ struct v4l2_ctrl_handler; */ struct v4l2_device { struct device *dev; -#if defined(CONFIG_MEDIA_CONTROLLER) struct media_device *mdev; -#endif struct list_head subdevs; spinlock_t lock; char name[V4L2_DEVICE_NAME_SIZE]; -- 2.16.3
[RFCv11 PATCH 28/29] vivid: add mc
From: Hans VerkuilAdd support for the media_device to vivid. This is a prerequisite for request support. Signed-off-by: Hans Verkuil --- drivers/media/platform/vivid/vivid-core.c | 61 +++ drivers/media/platform/vivid/vivid-core.h | 8 2 files changed, 69 insertions(+) diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index 82ec216f2ad8..69386b26d5dd 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -657,6 +657,15 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) dev->inst = inst; +#ifdef CONFIG_MEDIA_CONTROLLER + dev->v4l2_dev.mdev = >mdev; + + /* Initialize media device */ + strlcpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model)); + dev->mdev.dev = >dev; + media_device_init(>mdev); +#endif + /* register v4l2_device */ snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s-%03d", VIVID_MODULE_NAME, inst); @@ -1173,6 +1182,13 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) vfd->lock = >mutex; video_set_drvdata(vfd, dev); +#ifdef CONFIG_MEDIA_CONTROLLER + dev->vid_cap_pad.flags = MEDIA_PAD_FL_SINK; + ret = media_entity_pads_init(>entity, 1, >vid_cap_pad); + if (ret) + goto unreg_dev; +#endif + #ifdef CONFIG_VIDEO_VIVID_CEC if (in_type_counter[HDMI]) { struct cec_adapter *adap; @@ -1225,6 +1241,13 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) vfd->lock = >mutex; video_set_drvdata(vfd, dev); +#ifdef CONFIG_MEDIA_CONTROLLER + dev->vid_out_pad.flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(>entity, 1, >vid_out_pad); + if (ret) + goto unreg_dev; +#endif + #ifdef CONFIG_VIDEO_VIVID_CEC for (i = 0; i < dev->num_outputs; i++) { struct cec_adapter *adap; @@ -1274,6 +1297,13 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) vfd->tvnorms = tvnorms_cap; video_set_drvdata(vfd, dev); +#ifdef CONFIG_MEDIA_CONTROLLER + dev->vbi_cap_pad.flags = MEDIA_PAD_FL_SINK; + ret = media_entity_pads_init(>entity, 1, >vbi_cap_pad); + if (ret) + goto unreg_dev; +#endif + ret = video_register_device(vfd, VFL_TYPE_VBI, vbi_cap_nr[inst]); if (ret < 0) goto unreg_dev; @@ -1299,6 +1329,13 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) vfd->tvnorms = tvnorms_out; video_set_drvdata(vfd, dev); +#ifdef CONFIG_MEDIA_CONTROLLER + dev->vbi_out_pad.flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(>entity, 1, >vbi_out_pad); + if (ret) + goto unreg_dev; +#endif + ret = video_register_device(vfd, VFL_TYPE_VBI, vbi_out_nr[inst]); if (ret < 0) goto unreg_dev; @@ -1322,6 +1359,13 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) vfd->lock = >mutex; video_set_drvdata(vfd, dev); +#ifdef CONFIG_MEDIA_CONTROLLER + dev->sdr_cap_pad.flags = MEDIA_PAD_FL_SINK; + ret = media_entity_pads_init(>entity, 1, >sdr_cap_pad); + if (ret) + goto unreg_dev; +#endif + ret = video_register_device(vfd, VFL_TYPE_SDR, sdr_cap_nr[inst]); if (ret < 0) goto unreg_dev; @@ -1368,12 +1412,25 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) video_device_node_name(vfd)); } +#ifdef CONFIG_MEDIA_CONTROLLER + /* Register the media device */ + ret = media_device_register(>mdev); + if (ret) { + dev_err(dev->mdev.dev, + "media device register failed (err=%d)\n", ret); + goto unreg_dev; + } +#endif + /* Now that everything is fine, let's add it to device list */ vivid_devs[inst] = dev; return 0; unreg_dev: +#ifdef CONFIG_MEDIA_CONTROLLER + media_device_unregister(>mdev); +#endif video_unregister_device(>radio_tx_dev); video_unregister_device(>radio_rx_dev); video_unregister_device(>sdr_cap_dev); @@ -1444,6 +1501,10 @@ static int vivid_remove(struct platform_device *pdev) if (!dev) continue; +#ifdef CONFIG_MEDIA_CONTROLLER +
[RFCv11 PATCH 22/29] videobuf2-v4l2: add vb2_request_queue helper
From: Hans VerkuilGeneric helper function that checks if there are buffers in the request and if so, prepares and queues all objects in the request. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 39 + include/media/videobuf2-v4l2.h | 3 ++ 2 files changed, 42 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 73c1fd4da58a..3d0c74bb4220 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -1061,6 +1061,45 @@ void vb2_ops_wait_finish(struct vb2_queue *vq) } EXPORT_SYMBOL_GPL(vb2_ops_wait_finish); +int vb2_request_queue(struct media_request *req) +{ + struct media_request_object *obj; + struct media_request_object *failed_obj = NULL; + int ret = 0; + + if (!vb2_core_request_has_buffers(req)) + return -ENOENT; + + list_for_each_entry(obj, >objects, list) { + if (!obj->ops->prepare) + continue; + + ret = obj->ops->prepare(obj); + + if (ret) { + failed_obj = obj; + break; + } + } + + if (ret) { + list_for_each_entry(obj, >objects, list) { + if (obj == failed_obj) + break; + if (obj->ops->unprepare) + obj->ops->unprepare(obj); + } + return ret; + } + + list_for_each_entry(obj, >objects, list) { + if (obj->ops->queue) + obj->ops->queue(obj); + } + return 0; +} +EXPORT_SYMBOL_GPL(vb2_request_queue); + MODULE_DESCRIPTION("Driver helper framework for Video for Linux 2"); MODULE_AUTHOR("Pawel Osciak , Marek Szyprowski"); MODULE_LICENSE("GPL"); diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h index cf312ab4e7e8..0baa3023d7ad 100644 --- a/include/media/videobuf2-v4l2.h +++ b/include/media/videobuf2-v4l2.h @@ -301,4 +301,7 @@ void vb2_ops_wait_prepare(struct vb2_queue *vq); */ void vb2_ops_wait_finish(struct vb2_queue *vq); +struct media_request; +int vb2_request_queue(struct media_request *req); + #endif /* _MEDIA_VIDEOBUF2_V4L2_H */ -- 2.16.3
[RFCv11 PATCH 17/29] vb2: store userspace data in vb2_v4l2_buffer
From: Hans VerkuilThe userspace-provided plane data needs to be stored in vb2_v4l2_buffer. Currently this information is applied by __fill_vb2_buffer() which is called by the core prepare_buf and qbuf functions, but when using requests these functions aren't called yet since the buffer won't be prepared until the media request is actually queued. In the meantime this information has to be stored somewhere and vb2_v4l2_buffer is a good place for it. The __fill_vb2_buffer callback now just copies the relevant information from vb2_v4l2_buffer into the planes array. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-core.c | 25 +- drivers/media/common/videobuf2/videobuf2-v4l2.c | 324 +--- drivers/media/dvb-core/dvb_vb2.c| 3 +- include/media/videobuf2-core.h | 3 +- include/media/videobuf2-v4l2.h | 2 + 5 files changed, 197 insertions(+), 160 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index d3f7bb33a54d..3d436ccb61f8 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -968,9 +968,8 @@ static int __prepare_mmap(struct vb2_buffer *vb, const void *pb) { int ret = 0; - if (pb) - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, -vb, pb, vb->planes); + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, +vb, vb->planes); return ret ? ret : call_vb_qop(vb, buf_prepare, vb); } @@ -988,12 +987,10 @@ static int __prepare_userptr(struct vb2_buffer *vb, const void *pb) memset(planes, 0, sizeof(planes[0]) * vb->num_planes); /* Copy relevant information provided by the userspace */ - if (pb) { - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, -vb, pb, planes); - if (ret) - return ret; - } + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, +vb, planes); + if (ret) + return ret; for (plane = 0; plane < vb->num_planes; ++plane) { /* Skip the plane if already verified */ @@ -1104,12 +1101,10 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb) memset(planes, 0, sizeof(planes[0]) * vb->num_planes); /* Copy relevant information provided by the userspace */ - if (pb) { - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, -vb, pb, planes); - if (ret) - return ret; - } + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, +vb, planes); + if (ret) + return ret; for (plane = 0; plane < vb->num_planes; ++plane) { struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd); diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 4e9c77f21858..bf7a3ba9fed0 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -154,9 +154,177 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer *vb) pr_warn("use the actual size instead.\n"); } +static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) +{ + struct vb2_queue *q = vb->vb2_queue; + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); + struct vb2_plane *planes = vbuf->planes; + unsigned int plane; + int ret; + + ret = __verify_length(vb, b); + if (ret < 0) { + dprintk(1, "plane parameters verification failed: %d\n", ret); + return ret; + } + if (b->field == V4L2_FIELD_ALTERNATE && q->is_output) { + /* +* If the format's field is ALTERNATE, then the buffer's field +* should be either TOP or BOTTOM, not ALTERNATE since that +* makes no sense. The driver has to know whether the +* buffer represents a top or a bottom field in order to +* program any DMA correctly. Using ALTERNATE is wrong, since +* that just says that it is either a top or a bottom field, +* but not which of the two it is. +*/ + dprintk(1, "the field is incorrectly set to ALTERNATE for an output buffer\n"); + return -EINVAL; + } + vbuf->sequence = 0; + + if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { + switch (b->memory) { + case VB2_MEMORY_USERPTR: + for (plane = 0; plane < vb->num_planes; ++plane) { + planes[plane].m.userptr = +
[RFCv11 PATCH 18/29] videobuf2-core: embed media_request_object
From: Hans VerkuilMake vb2_buffer a request object. Signed-off-by: Hans Verkuil --- include/media/videobuf2-core.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 224c4820a044..3d54654c3cd4 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -17,6 +17,7 @@ #include #include #include +#include #define VB2_MAX_FRAME (32) #define VB2_MAX_PLANES (8) @@ -238,6 +239,7 @@ struct vb2_queue; * @num_planes:number of planes in the buffer * on an internal driver queue. * @timestamp: frame timestamp in ns. + * @req_obj: used to bind this buffer to a request */ struct vb2_buffer { struct vb2_queue*vb2_queue; @@ -246,6 +248,7 @@ struct vb2_buffer { unsigned intmemory; unsigned intnum_planes; u64 timestamp; + struct media_request_object req_obj; /* private: internal use only * -- 2.16.3
[RFCv11 PATCH 29/29] vivid: add request support
From: Hans VerkuilAdd support for requests to vivid. Signed-off-by: Hans Verkuil --- drivers/media/platform/vivid/vivid-core.c| 7 +++ drivers/media/platform/vivid/vivid-kthread-cap.c | 12 drivers/media/platform/vivid/vivid-kthread-out.c | 12 drivers/media/platform/vivid/vivid-sdr-cap.c | 8 drivers/media/platform/vivid/vivid-vbi-cap.c | 2 ++ drivers/media/platform/vivid/vivid-vbi-out.c | 2 ++ drivers/media/platform/vivid/vivid-vid-cap.c | 2 ++ drivers/media/platform/vivid/vivid-vid-out.c | 2 ++ 8 files changed, 47 insertions(+) diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index 69386b26d5dd..20e74d36b673 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -627,6 +627,12 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev) kfree(dev); } +#ifdef CONFIG_MEDIA_CONTROLLER +static const struct media_device_ops vivid_media_ops = { + .req_queue = vb2_request_queue, +}; +#endif + static int vivid_create_instance(struct platform_device *pdev, int inst) { static const struct v4l2_dv_timings def_dv_timings = @@ -664,6 +670,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) strlcpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model)); dev->mdev.dev = >dev; media_device_init(>mdev); + dev->mdev.ops = _media_ops; #endif /* register v4l2_device */ diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c b/drivers/media/platform/vivid/vivid-kthread-cap.c index 3fdb280c36ca..c192b4b1b9de 100644 --- a/drivers/media/platform/vivid/vivid-kthread-cap.c +++ b/drivers/media/platform/vivid/vivid-kthread-cap.c @@ -703,6 +703,8 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs) goto update_mv; if (vid_cap_buf) { + v4l2_ctrl_request_setup(vid_cap_buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vid_cap); /* Fill buffer */ vivid_fillbuff(dev, vid_cap_buf); dprintk(dev, 1, "filled buffer %d\n", @@ -713,6 +715,8 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs) dev->fb_cap.fmt.pixelformat == dev->fmt_cap->fourcc) vivid_overlay(dev, vid_cap_buf); + v4l2_ctrl_request_complete(vid_cap_buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vid_cap); vb2_buffer_done(_cap_buf->vb.vb2_buf, dev->dqbuf_error ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); dprintk(dev, 2, "vid_cap buffer %d done\n", @@ -720,10 +724,14 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs) } if (vbi_cap_buf) { + v4l2_ctrl_request_setup(vbi_cap_buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vbi_cap); if (dev->stream_sliced_vbi_cap) vivid_sliced_vbi_cap_process(dev, vbi_cap_buf); else vivid_raw_vbi_cap_process(dev, vbi_cap_buf); + v4l2_ctrl_request_complete(vbi_cap_buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vbi_cap); vb2_buffer_done(_cap_buf->vb.vb2_buf, dev->dqbuf_error ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); dprintk(dev, 2, "vbi_cap %d done\n", @@ -891,6 +899,8 @@ void vivid_stop_generating_vid_cap(struct vivid_dev *dev, bool *pstreaming) buf = list_entry(dev->vid_cap_active.next, struct vivid_buffer, list); list_del(>list); + v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vid_cap); vb2_buffer_done(>vb.vb2_buf, VB2_BUF_STATE_ERROR); dprintk(dev, 2, "vid_cap buffer %d done\n", buf->vb.vb2_buf.index); @@ -904,6 +914,8 @@ void vivid_stop_generating_vid_cap(struct vivid_dev *dev, bool *pstreaming) buf = list_entry(dev->vbi_cap_active.next, struct vivid_buffer, list); list_del(>list); + v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req, + >ctrl_hdl_vbi_cap); vb2_buffer_done(>vb.vb2_buf, VB2_BUF_STATE_ERROR); dprintk(dev, 2, "vbi_cap buffer %d done\n", buf->vb.vb2_buf.index); diff --git
[RFCv11 PATCH 27/29] vim2m: support requests
From: Hans VerkuilAdd support for requests to vim2m. Signed-off-by: Hans Verkuil --- drivers/media/platform/vim2m.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c index 9b18b32c255d..2dcf0ea85705 100644 --- a/drivers/media/platform/vim2m.c +++ b/drivers/media/platform/vim2m.c @@ -387,8 +387,26 @@ static void device_run(void *priv) src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx); + /* Apply request if needed */ + if (src_buf->vb2_buf.req_obj.req) + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req, + >hdl); + if (dst_buf->vb2_buf.req_obj.req && + dst_buf->vb2_buf.req_obj.req != src_buf->vb2_buf.req_obj.req) + v4l2_ctrl_request_setup(dst_buf->vb2_buf.req_obj.req, + >hdl); + device_process(ctx, src_buf, dst_buf); + /* Complete request if needed */ + if (src_buf->vb2_buf.req_obj.req) + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, + >hdl); + if (dst_buf->vb2_buf.req_obj.req && + dst_buf->vb2_buf.req_obj.req != src_buf->vb2_buf.req_obj.req) + v4l2_ctrl_request_complete(dst_buf->vb2_buf.req_obj.req, + >hdl); + /* Run a timer, which simulates a hardware irq */ schedule_irq(dev, ctx->transtime); } @@ -823,6 +841,8 @@ static void vim2m_stop_streaming(struct vb2_queue *q) vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); if (vbuf == NULL) return; + v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, + >hdl); spin_lock_irqsave(>dev->irqlock, flags); v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR); spin_unlock_irqrestore(>dev->irqlock, flags); @@ -1003,6 +1023,10 @@ static const struct v4l2_m2m_ops m2m_ops = { .job_abort = job_abort, }; +static const struct media_device_ops m2m_media_ops = { + .req_queue = vb2_request_queue, +}; + static int vim2m_probe(struct platform_device *pdev) { struct vim2m_dev *dev; @@ -1027,6 +1051,7 @@ static int vim2m_probe(struct platform_device *pdev) dev->mdev.dev = >dev; strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model)); media_device_init(>mdev); + dev->mdev.ops = _media_ops; dev->v4l2_dev.mdev = >mdev; dev->pad[0].flags = MEDIA_PAD_FL_SINK; dev->pad[1].flags = MEDIA_PAD_FL_SOURCE; -- 2.16.3
[RFCv11 PATCH 26/29] vim2m: use workqueue
From: Hans Verkuilv4l2_ctrl uses mutexes, so we can't setup a ctrl_handler in interrupt context. Switch to a workqueue instead. Signed-off-by: Hans Verkuil --- drivers/media/platform/vim2m.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c index ef970434af13..9b18b32c255d 100644 --- a/drivers/media/platform/vim2m.c +++ b/drivers/media/platform/vim2m.c @@ -150,6 +150,7 @@ struct vim2m_dev { spinlock_t irqlock; struct timer_list timer; + struct work_struct work_run; struct v4l2_m2m_dev *m2m_dev; }; @@ -392,9 +393,10 @@ static void device_run(void *priv) schedule_irq(dev, ctx->transtime); } -static void device_isr(struct timer_list *t) +static void device_work(struct work_struct *w) { - struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t, timer); + struct vim2m_dev *vim2m_dev = + container_of(w, struct vim2m_dev, work_run); struct vim2m_ctx *curr_ctx; struct vb2_v4l2_buffer *src_vb, *dst_vb; unsigned long flags; @@ -426,6 +428,13 @@ static void device_isr(struct timer_list *t) } } +static void device_isr(struct timer_list *t) +{ + struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t, timer); + + schedule_work(_dev->work_run); +} + /* * video ioctls */ @@ -806,6 +815,7 @@ static void vim2m_stop_streaming(struct vb2_queue *q) struct vb2_v4l2_buffer *vbuf; unsigned long flags; + flush_scheduled_work(); for (;;) { if (V4L2_TYPE_IS_OUTPUT(q->type)) vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx); @@ -1011,6 +1021,7 @@ static int vim2m_probe(struct platform_device *pdev) vfd = >vfd; vfd->lock = >dev_mutex; vfd->v4l2_dev = >v4l2_dev; + INIT_WORK(>work_run, device_work); #ifdef CONFIG_MEDIA_CONTROLLER dev->mdev.dev = >dev; -- 2.16.3
[RFCv11 PATCH 03/29] media-request: allocate media requests
From: Hans VerkuilAdd support for allocating a new request. This is only supported if mdev->ops->req_queue is set, i.e. the driver indicates that it supports queueing requests. Signed-off-by: Hans Verkuil --- drivers/media/Makefile| 3 ++- drivers/media/media-device.c | 14 ++ drivers/media/media-request.c | 23 +++ include/media/media-device.h | 13 + include/media/media-request.h | 22 ++ 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 drivers/media/media-request.c create mode 100644 include/media/media-request.h diff --git a/drivers/media/Makefile b/drivers/media/Makefile index 594b462ddf0e..985d35ec6b29 100644 --- a/drivers/media/Makefile +++ b/drivers/media/Makefile @@ -3,7 +3,8 @@ # Makefile for the kernel multimedia device drivers. # -media-objs := media-device.o media-devnode.o media-entity.o +media-objs := media-device.o media-devnode.o media-entity.o \ + media-request.o # # I2C drivers should come before other drivers, otherwise they'll fail diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 35e81f7c0d2f..acb583c0eacd 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -32,6 +32,7 @@ #include #include #include +#include #ifdef CONFIG_MEDIA_CONTROLLER @@ -366,6 +367,15 @@ static long media_device_get_topology(struct media_device *mdev, return ret; } +static long media_device_request_alloc(struct media_device *mdev, + struct media_request_alloc *alloc) +{ + if (!mdev->ops || !mdev->ops->req_queue) + return -ENOTTY; + + return media_request_alloc(mdev, alloc); +} + static long copy_arg_from_user(void *karg, void __user *uarg, unsigned int cmd) { /* All media IOCTLs are _IOWR() */ @@ -414,6 +424,7 @@ static const struct media_ioctl_info ioctl_info[] = { MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX), MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX), MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX), + MEDIA_IOC(REQUEST_ALLOC, media_device_request_alloc, 0), }; static long media_device_ioctl(struct file *filp, unsigned int cmd, @@ -686,6 +697,9 @@ void media_device_init(struct media_device *mdev) INIT_LIST_HEAD(>pads); INIT_LIST_HEAD(>links); INIT_LIST_HEAD(>entity_notify); + + spin_lock_init(>req_lock); + mutex_init(>req_queue_mutex); mutex_init(>graph_mutex); ida_init(>entity_internal_idx); diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c new file mode 100644 index ..ead78613fdbe --- /dev/null +++ b/drivers/media/media-request.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Media device request objects + * + * Copyright (C) 2018 Intel Corporation + * Copyright (C) 2018, The Chromium OS Authors. All rights reserved. + * + * Author: Sakari Ailus + */ + +#include +#include +#include +#include + +#include +#include + +int media_request_alloc(struct media_device *mdev, + struct media_request_alloc *alloc) +{ + return -ENOMEM; +} diff --git a/include/media/media-device.h b/include/media/media-device.h index bcc6ec434f1f..07e323c57202 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -19,6 +19,7 @@ #ifndef _MEDIA_DEVICE_H #define _MEDIA_DEVICE_H +#include #include #include @@ -27,6 +28,7 @@ struct ida; struct device; +struct media_device; /** * struct media_entity_notify - Media Entity Notify @@ -50,10 +52,16 @@ struct media_entity_notify { * struct media_device_ops - Media device operations * @link_notify: Link state change notification callback. This callback is * called with the graph_mutex held. + * @req_alloc: Allocate a request + * @req_free: Free a request + * @req_queue: Queue a request */ struct media_device_ops { int (*link_notify)(struct media_link *link, u32 flags, unsigned int notification); + struct media_request *(*req_alloc)(struct media_device *mdev); + void (*req_free)(struct media_request *req); + int (*req_queue)(struct media_request *req); }; /** @@ -88,6 +96,8 @@ struct media_device_ops { * @disable_source: Disable Source Handler function pointer * * @ops: Operation handler callbacks + * @req_lock: Serialise access to requests + * @req_queue_mutex: Serialise validating and queueing requests * * This structure represents an abstract high-level media device. It allows easy * access to entities and provides basic media device-level support. The @@ -158,6 +168,9 @@ struct media_device { void (*disable_source)(struct
[RFCv11 PATCH 00/29] Request API
From: Hans VerkuilHi all, This is a cleaned up version of the v10 series (never posted to the list since it was messy). The main changes compared to v9 are in the control framework which is (hopefully!) now in sync with the RFC. Specifically, when queueing a request it will 'chain' itself correctly with the previously queued request. Control values that were not explicitly set in the request will get the value from the first request in the queue that sets it, or the hardware value if no request in the queue ever touches it. However, I have not yet had the opportunity to test this in v4l2-compliance! Sakari: I did not have the time to incorporate your comments. I'll probably wait until I have more feedback and then post a new series next week. Other changes: - various request state checks were missing (i.e. you could set a control in a queued request). - a new cancel op was added to handle the corner case where a request was queued but never reached the driver since STREAMOFF was called before the buffers were ever queued in the driver. - various random fixes. - added the patch "videobuf2-v4l2: export request_fd" as requested by Sakari. - changed some inconsistent error codes. This has been tested with vim2m and vivid using v4l2-compliance. The v4l-utils repo supporting requests is here: https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=request TODO/Remarks: 1) missing prototype documentation in media-requests.c/h. Some is documented, but not everything. 2) No VIDIOC_REQUEST_ALLOC 'shortcut' ioctl. Sorry, I just ran out of time. Alexandre, Tomasz, feel free to add it back (it should be quite easy to do) and post a patch. I'll add it to my patch series. As mentioned before: whether or not we actually want this has not been decided yet. 3) vim2m: the media topology is a bit bogus, this needs to be fixed (i.e. a proper HW entity should be added). But for now it is good enough for testing. 4) I think this should slide in fairly easy after the fence support is merged. I made sure the request API changes in public headers did not clash with the changes made by the fence API. 5) I did not verify the Request API documentation patch. I did update it with the new buffer flags and 'which' value, but it probably is out of date in places. 6) More testing. I think I have fairly good coverage in v4l2-compliance, but no doubt I've missed a few test cases. 7) debugfs: it would be really useful to expose the number of request and request objects in debugfs to simplify debugging. Esp. to make sure everything is freed correctly. 8) Review copyright/authorship lines. I'm not sure if everything is correct. Alexandre, Sakari, if you see something that is not right in this respect, just let me know! 9) The req_queue op should likely be split into a req_validate and a req_queue op (based on a discussion on the ML with Sakari). That avoids a race condition. Everything seemed to slip nicely into place while working on this, so I hope this is finally an implementation that we can proceed to upstream and build upon for complex camera pipelines in the future. This patch series is also available here: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=reqv11 Regards, Hans Alexandre Courbot (3): videodev2.h: add request_fd field to v4l2_ext_controls Documentation: v4l: document request API media: vim2m: add media device Hans Verkuil (26): v4l2-device.h: always expose mdev uapi/linux/media.h: add request API media-request: allocate media requests media-request: core request support media-request: add request ioctls media-request: add media_request_find media-request: add media_request_object_find videodev2.h: add V4L2_CTRL_FLAG_IN_REQUEST v4l2-ctrls: v4l2_ctrl_add_handler: add from_other_dev v4l2-ctrls: prepare internal structs for request API v4l2-ctrls: alloc memory for p_req v4l2-ctrls: use ref in helper instead of ctrl v4l2-ctrls: add core request support v4l2-ctrls: support g/s_ext_ctrls for requests videodev2.h: Add request_fd field to v4l2_buffer vb2: store userspace data in vb2_v4l2_buffer videobuf2-core: embed media_request_object videobuf2-core: integrate with media requests videobuf2-v4l2: integrate with media requests videobuf2-core: add vb2_core_request_has_buffers videobuf2-v4l2: add vb2_request_queue helper videobuf2-v4l2: export request_fd vim2m: use workqueue vim2m: support requests vivid: add mc vivid: add request support Documentation/media/uapi/v4l/buffer.rst| 19 +- Documentation/media/uapi/v4l/common.rst| 1 + Documentation/media/uapi/v4l/request-api.rst | 199 + Documentation/media/uapi/v4l/user-func.rst | 1 + .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 22 +- .../media/uapi/v4l/vidioc-new-request.rst | 64 +++ Documentation/media/uapi/v4l/vidioc-qbuf.rst
[RFCv11 PATCH 16/29] videodev2.h: Add request_fd field to v4l2_buffer
From: Hans VerkuilWhen queuing buffers allow for passing the request that should be associated with this buffer. If V4L2_BUF_FLAG_REQUEST_FD is set, then request_fd is used as the file descriptor. If a buffer is stored in a request, but not yet queued to the driver, then V4L2_BUF_FLAG_IN_REQUEST is set. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 +- drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 ++--- drivers/media/v4l2-core/v4l2-ioctl.c| 4 ++-- include/uapi/linux/videodev2.h | 10 +- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 886a2d8d5c6c..4e9c77f21858 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -204,7 +204,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) b->timecode = vbuf->timecode; b->sequence = vbuf->sequence; b->reserved2 = 0; - b->reserved = 0; + b->request_fd = 0; if (q->is_multiplanar) { /* diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c index 99f106b13280..13aee9f67d05 100644 --- a/drivers/media/usb/cpia2/cpia2_v4l.c +++ b/drivers/media/usb/cpia2/cpia2_v4l.c @@ -949,7 +949,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer; buf->length = cam->frame_size; buf->reserved2 = 0; - buf->reserved = 0; + buf->request_fd = 0; memset(>timecode, 0, sizeof(buf->timecode)); DBG("DQBUF #%d status:%d seq:%d length:%d\n", buf->index, diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 0782b3666deb..7e27d0feac94 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -387,7 +387,7 @@ struct v4l2_buffer32 { } m; __u32 length; __u32 reserved2; - __u32 reserved; + __s32 request_fd; }; static int get_v4l2_plane32(struct v4l2_plane __user *up, @@ -486,6 +486,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, { u32 type; u32 length; + s32 request_fd; enum v4l2_memory memory; struct v4l2_plane32 __user *uplane32; struct v4l2_plane __user *uplane; @@ -500,7 +501,9 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, get_user(memory, >memory) || put_user(memory, >memory) || get_user(length, >length) || - put_user(length, >length)) + put_user(length, >length) || + get_user(request_fd, >request_fd) || + put_user(request_fd, >request_fd)) return -EFAULT; if (V4L2_TYPE_IS_OUTPUT(type)) @@ -605,7 +608,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, copy_in_user(>timecode, >timecode, sizeof(kp->timecode)) || assign_in_user(>sequence, >sequence) || assign_in_user(>reserved2, >reserved2) || - assign_in_user(>reserved, >reserved) || + assign_in_user(>request_fd, >request_fd) || get_user(length, >length) || put_user(length, >length)) return -EFAULT; diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 56741c4a48fc..561a1fe3160b 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -437,13 +437,13 @@ static void v4l_print_buffer(const void *arg, bool write_only) const struct v4l2_plane *plane; int i; - pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, flags=0x%08x, field=%s, sequence=%d, memory=%s", + pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request_fd=%d, flags=0x%08x, field=%s, sequence=%d, memory=%s", p->timestamp.tv_sec / 3600, (int)(p->timestamp.tv_sec / 60) % 60, (int)(p->timestamp.tv_sec % 60), (long)p->timestamp.tv_usec, p->index, - prt_names(p->type, v4l2_type_names), + prt_names(p->type, v4l2_type_names), p->request_fd, p->flags, prt_names(p->field, v4l2_field_names), p->sequence, prt_names(p->memory, v4l2_memory_names)); diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index d6e5c245bdcf..9c65d890a5f2 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@
[RFCv11 PATCH 15/29] v4l2-ctrls: support g/s_ext_ctrls for requests
From: Hans VerkuilThe v4l2_g/s_ext_ctrls functions now support control handlers that represent requests. The v4l2_ctrls_find_req_obj() function is responsible for finding the request from the fd. Signed-off-by: Hans Verkuil --- drivers/media/platform/omap3isp/ispvideo.c | 2 +- drivers/media/v4l2-core/v4l2-ctrls.c | 113 +++-- drivers/media/v4l2-core/v4l2-ioctl.c | 12 +-- drivers/media/v4l2-core/v4l2-subdev.c | 9 ++- include/media/v4l2-ctrls.h | 7 +- 5 files changed, 124 insertions(+), 19 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c index a751c89a3ea8..bd564c2e767f 100644 --- a/drivers/media/platform/omap3isp/ispvideo.c +++ b/drivers/media/platform/omap3isp/ispvideo.c @@ -1028,7 +1028,7 @@ static int isp_video_check_external_subdevs(struct isp_video *video, ctrls.count = 1; ctrls.controls = - ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, ); + ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, NULL, ); if (ret < 0) { dev_warn(isp->dev, "no pixel rate control in subdev %s\n", pipe->external->name); diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 6e2c5e24734f..317a374b6ea6 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -3122,7 +3122,8 @@ static int class_check(struct v4l2_ctrl_handler *hdl, u32 which) } /* Get extended controls. Allocates the helpers array if needed. */ -int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs) +int __v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, + struct v4l2_ext_controls *cs) { struct v4l2_ctrl_helper helper[4]; struct v4l2_ctrl_helper *helpers = helper; @@ -3202,6 +3203,73 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs kvfree(helpers); return ret; } + +static struct media_request_object * +v4l2_ctrls_find_req_obj(struct v4l2_ctrl_handler *hdl, + struct media_device *mdev, s32 fd, bool set) +{ + struct media_request *req = media_request_find(mdev, fd); + struct media_request_object *obj; + struct v4l2_ctrl_handler *new_hdl; + int ret; + + if (IS_ERR(req)) + return ERR_CAST(req); + + if (set && req->state != MEDIA_REQUEST_STATE_IDLE) { + media_request_put(req); + return ERR_PTR(-EBUSY); + } + + obj = media_request_object_find(req, _ops, hdl); + if (obj) { + media_request_put(req); + return obj; + } + + new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL); + if (!new_hdl) { + ret = -ENOMEM; + goto put; + } + obj = _hdl->req_obj; + ret = v4l2_ctrl_handler_init(new_hdl, (hdl->nr_of_buckets - 1) * 8); + if (!ret) + ret = v4l2_ctrl_request_bind(req, new_hdl, hdl); + if (!ret) { + media_request_object_get(obj); + media_request_put(req); + return obj; + } + kfree(new_hdl); + +put: + media_request_put(req); + return ERR_PTR(ret); +} + +int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct media_device *mdev, +struct v4l2_ext_controls *cs) +{ + struct media_request_object *obj = NULL; + int ret; + + if (cs->which == V4L2_CTRL_WHICH_REQUEST) { + if (!mdev || cs->request_fd < 0) + return -EINVAL; + obj = v4l2_ctrls_find_req_obj(hdl, mdev, cs->request_fd, false); + if (IS_ERR(obj)) + return PTR_ERR(obj); + hdl = container_of(obj, struct v4l2_ctrl_handler, + req_obj); + } + + ret = __v4l2_g_ext_ctrls(hdl, cs); + + if (obj) + media_request_object_put(obj); + return ret; +} EXPORT_SYMBOL(v4l2_g_ext_ctrls); /* Helper function to get a single control */ @@ -3377,9 +3445,9 @@ static void update_from_auto_cluster(struct v4l2_ctrl *master) } /* Try or try-and-set controls */ -static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, -struct v4l2_ext_controls *cs, -bool set) +static int __try_set_ext_ctrls(struct v4l2_fh *fh, + struct v4l2_ctrl_handler *hdl, + struct v4l2_ext_controls *cs, bool set) { struct v4l2_ctrl_helper helper[4]; struct v4l2_ctrl_helper *helpers = helper; @@ -3492,16 +3560,45 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, return ret; } -int
[RFCv11 PATCH 21/29] videobuf2-core: add vb2_core_request_has_buffers
From: Hans VerkuilAdd a new helper function that returns true if a media_request contains buffers. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-core.c | 12 include/media/videobuf2-core.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 4f6c0b2d7d1a..13c9d9e243dd 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -1339,6 +1339,18 @@ static const struct media_request_object_ops vb2_core_req_ops = { .release = vb2_req_release, }; +bool vb2_core_request_has_buffers(struct media_request *req) +{ + struct media_request_object *obj; + + list_for_each_entry(obj, >objects, list) { + if (obj->ops == _core_req_ops) + return true; + } + return false; +} +EXPORT_SYMBOL_GPL(vb2_core_request_has_buffers); + int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb, struct media_request *req) { diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 72663c2a3ba3..e23dc028aee7 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -1158,4 +1158,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb); */ int vb2_verify_memory_type(struct vb2_queue *q, enum vb2_memory memory, unsigned int type); + +bool vb2_core_request_has_buffers(struct media_request *req); #endif /* _MEDIA_VIDEOBUF2_CORE_H */ -- 2.16.3
[RFCv11 PATCH 19/29] videobuf2-core: integrate with media requests
From: Hans VerkuilBuffers can now be prepared or queued for a request. A buffer is unbound from the request at vb2_buffer_done time or when the queue is cancelled. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-core.c | 115 +--- drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 +- drivers/media/dvb-core/dvb_vb2.c| 2 +- include/media/videobuf2-core.h | 17 +++- 4 files changed, 122 insertions(+), 16 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 3d436ccb61f8..4f6c0b2d7d1a 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -930,6 +930,14 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) vb->state = state; } atomic_dec(>owned_by_drv_count); + + if (vb->req_obj.req) { + /* This is not supported at the moment */ + WARN_ON(state == VB2_BUF_STATE_REQUEUEING); + media_request_object_unbind(>req_obj); + media_request_object_put(>req_obj); + } + spin_unlock_irqrestore(>done_lock, flags); trace_vb2_buf_done(q, vb); @@ -1276,11 +1284,66 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb) return 0; } -int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) +static int vb2_req_prepare(struct media_request_object *obj) { - struct vb2_buffer *vb; + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj); int ret; + if (WARN_ON(vb->state != VB2_BUF_STATE_IN_REQUEST)) + return -EINVAL; + + ret = __buf_prepare(vb, NULL); + if (ret) + vb->state = VB2_BUF_STATE_IN_REQUEST; + return ret; +} + +static void __vb2_dqbuf(struct vb2_buffer *vb); +static void vb2_req_unprepare(struct media_request_object *obj) +{ + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj); + + __vb2_dqbuf(vb); + vb->state = VB2_BUF_STATE_IN_REQUEST; + WARN_ON(!vb->req_obj.req); +} + +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, + struct media_request *req); + +static void vb2_req_queue(struct media_request_object *obj) +{ + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj); + + vb2_core_qbuf(vb->vb2_queue, vb->index, NULL, NULL); +} + +static void vb2_req_release(struct media_request_object *obj) +{ + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj); + + if (vb->state == VB2_BUF_STATE_IN_REQUEST) + vb->state = VB2_BUF_STATE_DEQUEUED; +} + +static void vb2_req_cancel(struct media_request_object *obj) +{ + /* Nothing to do here, buffers are cancelled in __vb2_queue_cancel() */ +} + +static const struct media_request_object_ops vb2_core_req_ops = { + .prepare = vb2_req_prepare, + .unprepare = vb2_req_unprepare, + .queue = vb2_req_queue, + .cancel = vb2_req_cancel, + .release = vb2_req_release, +}; + +int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb, +struct media_request *req) +{ + struct vb2_buffer *vb; + vb = q->bufs[index]; if (vb->state != VB2_BUF_STATE_DEQUEUED) { dprintk(1, "invalid buffer state %d\n", @@ -1288,16 +1351,24 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) return -EINVAL; } - ret = __buf_prepare(vb, pb); - if (ret) - return ret; + if (req) { + vb->state = VB2_BUF_STATE_IN_REQUEST; + media_request_object_init(>req_obj); + media_request_object_bind(req, _core_req_ops, + q, >req_obj); + } else { + int ret = __buf_prepare(vb, pb); + + if (ret) + return ret; + } /* Fill buffer information for the userspace */ call_void_bufop(q, fill_user_buffer, vb, pb); dprintk(2, "prepare of buffer %d succeeded\n", vb->index); - return ret; + return 0; } EXPORT_SYMBOL_GPL(vb2_core_prepare_buf); @@ -1364,13 +1435,27 @@ static int vb2_start_streaming(struct vb2_queue *q) return ret; } -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb) +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, + struct media_request *req) { struct vb2_buffer *vb; int ret; vb = q->bufs[index]; + if (vb->state == VB2_BUF_STATE_DEQUEUED && req) { + vb->state = VB2_BUF_STATE_IN_REQUEST; + media_request_object_init(>req_obj); +
[RFCv11 PATCH 14/29] v4l2-ctrls: add core request support
From: Hans VerkuilIntegrate the request support. This adds the v4l2_ctrl_request_complete and v4l2_ctrl_request_setup functions to complete a request and (as a helper function) to apply a request to the hardware. It takes care of queuing requests and correctly chaining control values in the request queue. Note that when a request is marked completed it will copy control values to the internal request state. This can be optimized in the future since this is sub-optimal when dealing with large compound and/or array controls. For the initial 'stateless codec' use-case the current implementation is sufficient. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls.c | 325 ++- include/media/v4l2-ctrls.h | 23 +++ 2 files changed, 342 insertions(+), 6 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index da4cc1485dc4..6e2c5e24734f 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1647,6 +1647,13 @@ static int new_to_user(struct v4l2_ext_control *c, return ptr_to_user(c, ctrl, ctrl->p_new); } +/* Helper function: copy the request value back to the caller */ +static int req_to_user(struct v4l2_ext_control *c, + struct v4l2_ctrl_ref *ref) +{ + return ptr_to_user(c, ref->ctrl, ref->p_req); +} + /* Helper function: copy the initial control value back to the caller */ static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl) { @@ -1766,6 +1773,26 @@ static void cur_to_new(struct v4l2_ctrl *ctrl) ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new); } +/* Copy the new value to the request value */ +static void new_to_req(struct v4l2_ctrl_ref *ref) +{ + if (!ref) + return; + ptr_to_ptr(ref->ctrl, ref->ctrl->p_new, ref->p_req); + ref->req = ref; +} + +/* Copy the request value to the new value */ +static void req_to_new(struct v4l2_ctrl_ref *ref) +{ + if (!ref) + return; + if (ref->req) + ptr_to_ptr(ref->ctrl, ref->req->p_req, ref->ctrl->p_new); + else + ptr_to_ptr(ref->ctrl, ref->ctrl->p_cur, ref->ctrl->p_new); +} + /* Return non-zero if one or more of the controls in the cluster has a new value that differs from the current value. */ static int cluster_changed(struct v4l2_ctrl *master) @@ -1875,6 +1902,9 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl, lockdep_set_class_and_name(hdl->lock, key, name); INIT_LIST_HEAD(>ctrls); INIT_LIST_HEAD(>ctrl_refs); + INIT_LIST_HEAD(>requests); + INIT_LIST_HEAD(>requests_queued); + hdl->request_is_queued = false; hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8; hdl->buckets = kvmalloc_array(hdl->nr_of_buckets, sizeof(hdl->buckets[0]), @@ -1895,6 +1925,14 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl) if (hdl == NULL || hdl->buckets == NULL) return; + if (!hdl->req_obj.req && !list_empty(>requests)) { + struct v4l2_ctrl_handler *req, *next_req; + + list_for_each_entry_safe(req, next_req, >requests, requests) { + media_request_object_unbind(>req_obj); + media_request_object_put(>req_obj); + } + } mutex_lock(hdl->lock); /* Free all nodes */ list_for_each_entry_safe(ref, next_ref, >ctrl_refs, node) { @@ -2816,6 +2854,126 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl, struct v4l2_querymenu *qm) } EXPORT_SYMBOL(v4l2_querymenu); +static int v4l2_ctrl_request_clone(struct v4l2_ctrl_handler *hdl, + const struct v4l2_ctrl_handler *from) +{ + struct v4l2_ctrl_ref *ref; + int err; + + if (WARN_ON(!hdl || hdl == from)) + return -EINVAL; + + if (hdl->error) + return hdl->error; + + WARN_ON(hdl->lock != >_lock); + + mutex_lock(from->lock); + list_for_each_entry(ref, >ctrl_refs, node) { + struct v4l2_ctrl *ctrl = ref->ctrl; + struct v4l2_ctrl_ref *new_ref; + + /* Skip refs inherited from other devices */ + if (ref->from_other_dev) + continue; + /* And buttons */ + if (ctrl->type == V4L2_CTRL_TYPE_BUTTON) + continue; + err = handler_new_ref(hdl, ctrl, _ref, false, true); + if (err) { + printk("%s: handler_new_ref on control %x (%s) returned %d\n", __func__, ctrl->id, ctrl->name, err); + err = 0; + continue; + } + if (err) + break; + } + mutex_unlock(from->lock); +
[RFCv11 PATCH 02/29] uapi/linux/media.h: add request API
From: Hans VerkuilDefine the public request API. This adds the new MEDIA_IOC_REQUEST_ALLOC ioctl to allocate a request and two ioctls that operate on a request in order to queue the contents of the request to the driver and to re-initialize the request. Signed-off-by: Hans Verkuil --- include/uapi/linux/media.h | 8 1 file changed, 8 insertions(+) diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index c7e9a5cba24e..f8769e74f847 100644 --- a/include/uapi/linux/media.h +++ b/include/uapi/linux/media.h @@ -342,11 +342,19 @@ struct media_v2_topology { /* ioctls */ +struct __attribute__ ((packed)) media_request_alloc { + __s32 fd; +}; + #define MEDIA_IOC_DEVICE_INFO _IOWR('|', 0x00, struct media_device_info) #define MEDIA_IOC_ENUM_ENTITIES_IOWR('|', 0x01, struct media_entity_desc) #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum) #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc) #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology) +#define MEDIA_IOC_REQUEST_ALLOC_IOWR('|', 0x05, struct media_request_alloc) + +#define MEDIA_REQUEST_IOC_QUEUE_IO('|', 0x80) +#define MEDIA_REQUEST_IOC_REINIT _IO('|', 0x81) #if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API) -- 2.16.3
Re: [PATCH] media: entity: fix spelling for media_entity_get_fwnode_pad()
On Sun, Apr 08, 2018 at 06:11:52PM +0200, Niklas Söderlund wrote: > From: Niklas Söderlund> > s/dose/does/ > > Fixes: d295c6a460cd2ac6 ("[media] media: entity: Add > media_entity_get_fwnode_pad() function") > Signed-off-by: Niklas Söderlund Reviewed-by: Simon Horman
[PATCH v3 1/7] drm/i2c: tda998x: move mutex/waitqueue/timer/work init early
Move the mutex, waitqueue, timer and detect work initialisation early in the driver's initialisation, rather than being after we've registered the CEC device. Acked-by: Hans VerkuilSigned-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index cd3f0873bbdd..83407159e957 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1475,7 +1475,11 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) u32 video; int rev_lo, rev_hi, ret; - mutex_init(>audio_mutex); /* Protect access from audio thread */ + mutex_init(>mutex); /* protect the page access */ + mutex_init(>audio_mutex); /* protect access from audio thread */ + init_waitqueue_head(>edid_delay_waitq); + timer_setup(>edid_delay_timer, tda998x_edid_delay_done, 0); + INIT_WORK(>detect_work, tda998x_detect_work); priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3); priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); @@ -1489,11 +1493,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) if (!priv->cec) return -ENODEV; - mutex_init(>mutex); /* protect the page access */ - init_waitqueue_head(>edid_delay_waitq); - timer_setup(>edid_delay_timer, tda998x_edid_delay_done, 0); - INIT_WORK(>detect_work, tda998x_detect_work); - /* wake up the device: */ cec_write(priv, REG_CEC_ENAMODS, CEC_ENAMODS_EN_RXSENS | CEC_ENAMODS_EN_HDMI); -- 2.7.4
[PATCH v3 5/7] drm/i2c: tda9950: add CEC driver
Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device, but is also integrated into HDMI transceivers such as the TDA9989 and TDA19989. The TDA9950 contains a command processor which handles retransmissions and the low level bus protocol. The driver just has to read and write the messages, and handle error conditions. Signed-off-by: Russell King--- drivers/gpu/drm/i2c/Kconfig | 5 + drivers/gpu/drm/i2c/Makefile | 1 + drivers/gpu/drm/i2c/tda9950.c | 509 ++ include/linux/platform_data/tda9950.h | 16 ++ 4 files changed, 531 insertions(+) create mode 100644 drivers/gpu/drm/i2c/tda9950.c create mode 100644 include/linux/platform_data/tda9950.h diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig index a6c92beb410a..3a232f5ff0a1 100644 --- a/drivers/gpu/drm/i2c/Kconfig +++ b/drivers/gpu/drm/i2c/Kconfig @@ -26,4 +26,9 @@ config DRM_I2C_NXP_TDA998X help Support for NXP Semiconductors TDA998X HDMI encoders. +config DRM_I2C_NXP_TDA9950 + tristate "NXP Semiconductors TDA9950/TDA998X HDMI CEC" + select CEC_NOTIFIER + select CEC_CORE + endmenu diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile index b20100c18ffb..a962f6f08568 100644 --- a/drivers/gpu/drm/i2c/Makefile +++ b/drivers/gpu/drm/i2c/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o tda998x-y := tda998x_drv.o obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o +obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c new file mode 100644 index ..3f7396caad48 --- /dev/null +++ b/drivers/gpu/drm/i2c/tda9950.c @@ -0,0 +1,509 @@ +/* + * TDA9950 Consumer Electronics Control driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * The NXP TDA9950 implements the HDMI Consumer Electronics Control + * interface. The host interface is similar to a mailbox: the data + * registers starting at REG_CDR0 are written to send a command to the + * internal CPU, and replies are read from these registers. + * + * As the data registers represent a mailbox, they must be accessed + * as a single I2C transaction. See the TDA9950 data sheet for details. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +enum { + REG_CSR = 0x00, + CSR_BUSY = BIT(7), + CSR_INT = BIT(6), + CSR_ERR = BIT(5), + + REG_CER = 0x01, + + REG_CVR = 0x02, + + REG_CCR = 0x03, + CCR_RESET = BIT(7), + CCR_ON= BIT(6), + + REG_ACKH = 0x04, + REG_ACKL = 0x05, + + REG_CCONR = 0x06, + CCONR_ENABLE_ERROR = BIT(4), + CCONR_RETRY_MASK = 7, + + REG_CDR0 = 0x07, + + CDR1_REQ = 0x00, + CDR1_CNF = 0x01, + CDR1_IND = 0x81, + CDR1_ERR = 0x82, + CDR1_IER = 0x83, + + CDR2_CNF_SUCCESS= 0x00, + CDR2_CNF_OFF_STATE = 0x80, + CDR2_CNF_BAD_REQ= 0x81, + CDR2_CNF_CEC_ACCESS = 0x82, + CDR2_CNF_ARB_ERROR = 0x83, + CDR2_CNF_BAD_TIMING = 0x84, + CDR2_CNF_NACK_ADDR = 0x85, + CDR2_CNF_NACK_DATA = 0x86, +}; + +struct tda9950_priv { + struct i2c_client *client; + struct device *hdmi; + struct cec_adapter *adap; + struct tda9950_glue *glue; + u16 addresses; + struct cec_msg rx_msg; + struct cec_notifier *notify; + bool open; +}; + +static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, int cnt) +{ + struct i2c_msg msg; + u8 buf[cnt + 1]; + int ret; + + buf[0] = addr; + memcpy(buf + 1, p, cnt); + + msg.addr = client->addr; + msg.flags = 0; + msg.len = cnt + 1; + msg.buf = buf; + + dev_dbg(>dev, "wr 0x%02x: %*ph\n", addr, cnt, p); + + ret = i2c_transfer(client->adapter, , 1); + if (ret < 0) + dev_err(>dev, "Error %d writing to cec:0x%x\n", ret, addr); + return ret < 0 ? ret : 0; +} + +static void tda9950_write(struct i2c_client *client, u8 addr, u8 val) +{ + tda9950_write_range(client, addr, , 1); +} + +static int tda9950_read_range(struct i2c_client *client, u8 addr, u8 *p, int cnt) +{ + struct i2c_msg msg[2]; + int ret; + + msg[0].addr = client->addr; + msg[0].flags = 0; + msg[0].len = 1; + msg[0].buf = + msg[1].addr = client->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = cnt; + msg[1].buf = p; + + ret = i2c_transfer(client->adapter, msg, 2); + if (ret < 0) + dev_err(>dev, "Error %d reading from cec:0x%x\n", ret, addr); + + dev_dbg(>dev, "rd 0x%02x: %*ph\n", addr, cnt, p); + + return ret; +} + +static u8
[PATCH v3 7/7] dt-bindings: tda998x: add the calibration gpio
Add the optional calibration gpio for integrated TDA9950 CEC support. This GPIO corresponds with the interrupt from the TDA998x, as the calibration requires driving the interrupt pin low. Reviewed-by: Rob HerringSigned-off-by: Russell King --- Documentation/devicetree/bindings/display/bridge/tda998x.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/tda998x.txt b/Documentation/devicetree/bindings/display/bridge/tda998x.txt index 24cc2466185a..1a4eaca40d94 100644 --- a/Documentation/devicetree/bindings/display/bridge/tda998x.txt +++ b/Documentation/devicetree/bindings/display/bridge/tda998x.txt @@ -27,6 +27,9 @@ Required properties; in question is used. The implementation allows one or two DAIs. If two DAIs are defined, they must be of different type. + - nxp,calib-gpios: calibration GPIO, which must correspond with the + gpio used for the TDA998x interrupt pin. + [1] Documentation/sound/alsa/soc/DAI.txt [2] include/dt-bindings/display/tda998x.h -- 2.7.4
[PATCH v3 4/7] drm/i2c: tda998x: always disable and clear interrupts at probe
Always disable and clear interrupts at probe time to ensure that the TDA998x is in a sane state. This ensures that the interrupt line, which is also the CEC clock calibration signal, is always deasserted. Acked-by: Hans VerkuilSigned-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 7f2762fab5c9..16e0439cad44 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1546,6 +1546,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) cec_write(priv, REG_CEC_FRO_IM_CLK_CTRL, CEC_FRO_IM_CLK_CTRL_GHOST_DIS | CEC_FRO_IM_CLK_CTRL_IMCLK_SEL); + /* ensure interrupts are disabled */ + cec_write(priv, REG_CEC_RXSHPDINTENA, 0); + + /* clear pending interrupts */ + cec_read(priv, REG_CEC_RXSHPDINT); + reg_read(priv, REG_INT_FLAGS_0); + reg_read(priv, REG_INT_FLAGS_1); + reg_read(priv, REG_INT_FLAGS_2); + /* initialize the optional IRQ */ if (client->irq) { unsigned long irq_flags; @@ -1553,11 +1562,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) /* init read EDID waitqueue and HDP work */ init_waitqueue_head(>wq_edid); - /* clear pending interrupts */ - reg_read(priv, REG_INT_FLAGS_0); - reg_read(priv, REG_INT_FLAGS_1); - reg_read(priv, REG_INT_FLAGS_2); - irq_flags = irqd_get_trigger_type(irq_get_irq_data(client->irq)); irq_flags |= IRQF_SHARED | IRQF_ONESHOT; -- 2.7.4
[PATCH v3 6/7] drm/i2c: tda998x: add CEC support
The TDA998x is a HDMI transmitter with a TDA9950 CEC engine integrated onto the same die. Add support for the TDA9950 CEC engine to the TDA998x driver. Signed-off-by: Russell King--- drivers/gpu/drm/i2c/Kconfig | 1 + drivers/gpu/drm/i2c/tda998x_drv.c | 195 -- 2 files changed, 187 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig index 3a232f5ff0a1..65d3acb61c03 100644 --- a/drivers/gpu/drm/i2c/Kconfig +++ b/drivers/gpu/drm/i2c/Kconfig @@ -22,6 +22,7 @@ config DRM_I2C_SIL164 config DRM_I2C_NXP_TDA998X tristate "NXP Semiconductors TDA998X HDMI encoder" default m if DRM_TILCDC + select CEC_CORE if CEC_NOTIFIER select SND_SOC_HDMI_CODEC if SND_SOC help Support for NXP Semiconductors TDA998X HDMI encoders. diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 16e0439cad44..eb9916bd84a4 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -16,8 +16,10 @@ */ #include +#include #include #include +#include #include #include #include @@ -29,6 +31,8 @@ #include #include +#include + #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__) struct tda998x_audio_port { @@ -55,6 +59,7 @@ struct tda998x_priv { struct platform_device *audio_pdev; struct mutex audio_mutex; + struct mutex edid_mutex; wait_queue_head_t wq_edid; volatile int wq_edid_wait; @@ -67,6 +72,9 @@ struct tda998x_priv { struct drm_connector connector; struct tda998x_audio_port audio_port[2]; + struct tda9950_glue cec_glue; + struct gpio_desc *calib; + struct cec_notifier *cec_notify; }; #define conn_to_tda998x_priv(x) \ @@ -345,6 +353,12 @@ struct tda998x_priv { #define REG_CEC_INTSTATUS0xee/* read */ # define CEC_INTSTATUS_CEC (1 << 0) # define CEC_INTSTATUS_HDMI (1 << 1) +#define REG_CEC_CAL_XOSC_CTRL10xf2 +# define CEC_CAL_XOSC_CTRL1_ENA_CALBIT(0) +#define REG_CEC_DES_FREQ2 0xf5 +# define CEC_DES_FREQ2_DIS_AUTOCAL BIT(7) +#define REG_CEC_CLK 0xf6 +# define CEC_CLK_FRO 0x11 #define REG_CEC_FRO_IM_CLK_CTRL 0xfb/* read/write */ # define CEC_FRO_IM_CLK_CTRL_GHOST_DIS (1 << 7) # define CEC_FRO_IM_CLK_CTRL_ENA_OTP (1 << 6) @@ -359,6 +373,7 @@ struct tda998x_priv { # define CEC_RXSHPDLEV_HPD(1 << 1) #define REG_CEC_ENAMODS 0xff/* read/write */ +# define CEC_ENAMODS_EN_CEC_CLK (1 << 7) # define CEC_ENAMODS_DIS_FRO (1 << 6) # define CEC_ENAMODS_DIS_CCLK (1 << 5) # define CEC_ENAMODS_EN_RXSENS(1 << 2) @@ -417,6 +432,114 @@ cec_read(struct tda998x_priv *priv, u8 addr) return val; } +static void cec_enamods(struct tda998x_priv *priv, u8 mods, bool enable) +{ + int val = cec_read(priv, REG_CEC_ENAMODS); + + if (val < 0) + return; + + if (enable) + val |= mods; + else + val &= ~mods; + + cec_write(priv, REG_CEC_ENAMODS, val); +} + +static void tda998x_cec_set_calibration(struct tda998x_priv *priv, bool enable) +{ + if (enable) { + u8 val; + + cec_write(priv, 0xf3, 0xc0); + cec_write(priv, 0xf4, 0xd4); + + /* Enable automatic calibration mode */ + val = cec_read(priv, REG_CEC_DES_FREQ2); + val &= ~CEC_DES_FREQ2_DIS_AUTOCAL; + cec_write(priv, REG_CEC_DES_FREQ2, val); + + /* Enable free running oscillator */ + cec_write(priv, REG_CEC_CLK, CEC_CLK_FRO); + cec_enamods(priv, CEC_ENAMODS_DIS_FRO, false); + + cec_write(priv, REG_CEC_CAL_XOSC_CTRL1, + CEC_CAL_XOSC_CTRL1_ENA_CAL); + } else { + cec_write(priv, REG_CEC_CAL_XOSC_CTRL1, 0); + } +} + +/* + * Calibration for the internal oscillator: we need to set calibration mode, + * and then pulse the IRQ line low for a 10ms ± 1% period. + */ +static void tda998x_cec_calibration(struct tda998x_priv *priv) +{ + struct gpio_desc *calib = priv->calib; + + mutex_lock(>edid_mutex); + if (priv->hdmi->irq > 0) + disable_irq(priv->hdmi->irq); + gpiod_direction_output(calib, 1); + tda998x_cec_set_calibration(priv, true); + + local_irq_disable(); + gpiod_set_value(calib, 0); + mdelay(10); + gpiod_set_value(calib, 1); + local_irq_enable(); + + tda998x_cec_set_calibration(priv, false); + gpiod_direction_input(calib); + if (priv->hdmi->irq > 0) + enable_irq(priv->hdmi->irq); + mutex_unlock(>edid_mutex); +} + +static int tda998x_cec_hook_init(void *data) +{ + struct tda998x_priv *priv = data; + struct gpio_desc
[PATCH v3 3/7] drm/i2c: tda998x: move CEC device initialisation later
We no longer use the CEC client to access the CEC part itself, so we can move this later in the initialisation sequence. Acked-by: Hans VerkuilSigned-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 2a99930f1bda..7f2762fab5c9 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1489,9 +1489,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->cec_addr = 0x34 + (client->addr & 0x03); priv->current_page = 0xff; priv->hdmi = client; - priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr); - if (!priv->cec) - return -ENODEV; /* wake up the device: */ cec_write(priv, REG_CEC_ENAMODS, @@ -1578,6 +1575,12 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); } + priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr); + if (!priv->cec) { + ret = -ENODEV; + goto fail; + } + /* enable EDID read irq: */ reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD); @@ -1594,14 +1597,14 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) ret = tda998x_get_audio_ports(priv, np); if (ret) - goto err_audio; + goto fail; if (priv->audio_port[0].format != AFMT_UNUSED) tda998x_audio_codec_init(priv, >dev); return 0; -err_audio: +fail: if (client->irq) free_irq(client->irq, priv); err_irq: -- 2.7.4
[PATCH v3 2/7] drm/i2c: tda998x: fix error cleanup paths
If tda998x_get_audio_ports() fails, and we requested the interrupt, we fail to free the interrupt before returning failure. Rework the failure cleanup code and exit paths so that we always clean up properly after an error, and always propagate the error code. Acked-by: Hans VerkuilSigned-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 83407159e957..2a99930f1bda 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1501,10 +1501,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) /* read version: */ rev_lo = reg_read(priv, REG_VERSION_LSB); + if (rev_lo < 0) { + dev_err(>dev, "failed to read version: %d\n", rev_lo); + return rev_lo; + } + rev_hi = reg_read(priv, REG_VERSION_MSB); - if (rev_lo < 0 || rev_hi < 0) { - ret = rev_lo < 0 ? rev_lo : rev_hi; - goto fail; + if (rev_hi < 0) { + dev_err(>dev, "failed to read version: %d\n", rev_hi); + return rev_hi; } priv->rev = rev_lo | rev_hi << 8; @@ -1528,7 +1533,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) default: dev_err(>dev, "found unsupported device: %04x\n", priv->rev); - goto fail; + return -ENXIO; } /* after reset, enable DDC: */ @@ -1566,7 +1571,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) dev_err(>dev, "failed to request IRQ#%u: %d\n", client->irq, ret); - goto fail; + goto err_irq; } /* enable HPD irq */ @@ -1589,19 +1594,19 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) ret = tda998x_get_audio_ports(priv, np); if (ret) - goto fail; + goto err_audio; if (priv->audio_port[0].format != AFMT_UNUSED) tda998x_audio_codec_init(priv, >dev); return 0; -fail: - /* if encoder_init fails, the encoder slave is never registered, -* so cleanup here: -*/ - if (priv->cec) - i2c_unregister_device(priv->cec); - return -ENXIO; + +err_audio: + if (client->irq) + free_irq(client->irq, priv); +err_irq: + i2c_unregister_device(priv->cec); + return ret; } static void tda998x_encoder_prepare(struct drm_encoder *encoder) -- 2.7.4
[PATCH v3 0/7] TDA998x CEC support
Hi, This patch series adds CEC support to the DRM TDA998x driver. The TDA998x family of devices integrate a TDA9950 CEC at a separate I2C address from the HDMI encoder. Implementation of the CEC part is separate to allow independent CEC implementations, or independent HDMI implementations (since the TDA9950 may be a separate device.) .../devicetree/bindings/display/bridge/tda998x.txt | 3 + drivers/gpu/drm/i2c/Kconfig| 6 + drivers/gpu/drm/i2c/Makefile | 1 + drivers/gpu/drm/i2c/tda9950.c | 509 + drivers/gpu/drm/i2c/tda998x_drv.c | 242 -- include/linux/platform_data/tda9950.h | 16 + 6 files changed, 750 insertions(+), 27 deletions(-) create mode 100644 drivers/gpu/drm/i2c/tda9950.c create mode 100644 include/linux/platform_data/tda9950.h v3: addressed most of Hans comments in v2 v2: updated DT property. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
Re: [PATCH 1/6] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
Hello, On Monday, 9 April 2018 09:58:12 EEST jacopo mondi wrote: > Hello Akinobu, > thank you for the patch. > > On which platform have you tested the series (just curious) ? > > On Sun, Apr 08, 2018 at 12:48:05AM +0900, Akinobu Mita wrote: > > The ov772x driver only works when the i2c controller have > > I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't > > support it. > > > > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that > > it doesn't support repeated starts. > > > > This change adds an alternative method for reading from ov772x register > > which uses two separated i2c messages for the i2c controllers without > > I2C_FUNC_PROTOCOL_MANGLING. > > Actually, and please correct me if I'm wrong, what I see here is that > an i2c_master_send+i2c_master_recv sequence is equivalent to a mangled > smbus transaction: > > i2c_smbus_read_byte_data | I2C_CLIENT_SCCB: > S Addr Wr [A] Comm [A] P S Addr Rd [A] [Data] NA P > > i2c_master_send() + i2c_master_recv(): > S Addr Wr [A] Data [A] P > S Addr Rd [A] [Data] NA P > > I wonder if it is not worth to ditch the existing read() function in > favour of your new proposed one completely. Given that this implementation is in no way specific to the ov772x driver, wouldn't it be better to handle this fallback in the I2C core instead ? > I have tested it on the Migo-R board where I have an ov772x installed > and it works fine. > > Although I would like to have a confirmation this is fine by people > how has seen more i2c adapters in action than me :) > > > Cc: Jacopo Mondi> > Cc: Laurent Pinchart > > Cc: Hans Verkuil > > Cc: Sakari Ailus > > Cc: Mauro Carvalho Chehab > > Signed-off-by: Akinobu Mita > > --- > > > > drivers/media/i2c/ov772x.c | 42 - > > 1 file changed, 33 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > > index b62860c..283ae2c 100644 > > --- a/drivers/media/i2c/ov772x.c > > +++ b/drivers/media/i2c/ov772x.c > > @@ -424,6 +424,7 @@ struct ov772x_priv { > > /* band_filter = COM8[5] ? 256 - BDBASE : 0 */ > > unsigned shortband_filter; > > unsigned int fps; > > + int (*reg_read)(struct i2c_client *client, u8 addr); > > }; > > > > /* > > @@ -542,11 +543,34 @@ static struct ov772x_priv *to_ov772x(struct > > v4l2_subdev *sd) > > return container_of(sd, struct ov772x_priv, subdev); > > } > > > > -static inline int ov772x_read(struct i2c_client *client, u8 addr) > > +static int ov772x_read(struct i2c_client *client, u8 addr) > > +{ > > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > > + struct ov772x_priv *priv = to_ov772x(sd); > > + > > + return priv->reg_read(client, addr); > > +} > > + > > +static int ov772x_reg_read(struct i2c_client *client, u8 addr) > > { > > return i2c_smbus_read_byte_data(client, addr); > > } > > > > +static int ov772x_reg_read_fallback(struct i2c_client *client, u8 addr) > > +{ > > + int ret; > > + u8 val; > > + > > + ret = i2c_master_send(client, , 1); > > + if (ret < 0) > > + return ret; > > + ret = i2c_master_recv(client, , 1); > > + if (ret < 0) > > + return ret; > > + > > + return val; > > +} > > + > > > > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 > > value) { > > return i2c_smbus_write_byte_data(client, addr, value); > > @@ -1255,20 +1279,20 @@ static int ov772x_probe(struct i2c_client *client, > > return -EINVAL; > > } > > > > - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > > - I2C_FUNC_PROTOCOL_MANGLING)) { > > - dev_err(>dev, > > - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or > > PROTOCOL_MANGLING \n"); > > - return -EIO; > > - } > > - client->flags |= I2C_CLIENT_SCCB; > > - > > priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL); > > if (!priv) > > return -ENOMEM; > > > > priv->info = client->dev.platform_data; > > > > + if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > > +I2C_FUNC_PROTOCOL_MANGLING)) > > + priv->reg_read = ov772x_reg_read; > > + else > > + priv->reg_read = ov772x_reg_read_fallback; > > + > > + client->flags |= I2C_CLIENT_SCCB; > > + > > v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); > > v4l2_ctrl_handler_init(>hdl, 3); > > v4l2_ctrl_new_std(>hdl, _ctrl_ops, -- Regards, Laurent Pinchart
Re: [PATCH 07/16] media: exymos4-is: allow compile test for EXYNOS FIMC-LITE
On 04/05/2018 07:54 PM, Mauro Carvalho Chehab wrote: > There's nothing that prevents building this driver with > COMPILE_TEST. So, enable it. > > While here, make the Kconfig dependency cleaner by removing > the unneeded if block. > > Signed-off-by: Mauro Carvalho ChehabWith s/exymos4-is/exynos4-is in the subject Acked-by: Sylwester Nawrocki -- Regards, Sylwester
Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST
HI Arnd, Em Mon, 9 Apr 2018 10:50:13 +0200 Arnd Bergmannescreveu: > >> > That hardly seems to be an arch-specific iommu solution, but, instead, > >> > some > >> > hack used by only three drivers or some legacy iommu binding. > >> > >> It's more complex than that. There are multiple IOMMU-related APIs on ARM, > >> so > >> more recent than others, with different feature sets. While I agree that > >> drivers should move away from arm_iommu_create_mapping(), doing so requires > >> coordination between the IOMMU driver and the bus master driver (for > >> instance > >> the omap3isp driver). It's not a trivial matter, but I'd love if someone > >> submitted patches :-) > > > > If someone steps up to do that, it would be really helpful, but we > > should not trust that this will happen. OMAP3 is an old hardware, > > and not many developers are working on improving its support. > > Considering its age, I still see a lot of changes on the arch/arm side of > it, so I wouldn't give up the hope yet. Yeah, someone might still work on such fix. > > Arnd, > > > > What do you think? > > I think including a foreign architecture header is worse than your > earlier patch, I'd rather see a local hack in the driver. > > I haven't tried it, but how about something simpler like what > I have below. Actually, another #ifdef was needed, before include arch-specifi header :-) > > Arnd > > (in case it works and you want to pick it up with a proper > changelog): > > Signed-off-by: Arnd Bergmann Sounds a reasonable approach. Instead of using CONFIG_ARM, I would, instead check for CONFIG_ARM_DMA_USE_IOMMU, with is the actual dependency for such code, as otherwise it could cause some compilation breakages on ARM with COMPILE_TEST and some randconfig. An advantage is that it properly annotates the part of the code that depends on ARM_DMA_USE_IOMMU. Thanks, Mauro From: Arnd Bergmann media: omap3isp: allow it to build with COMPILE_TEST There aren't much things required for it to build with COMPILE_TEST. It just needs to not compile the code that depends on arm-specific iommu implementation. Co-developed-by: Mauro Carvalho Chehab Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 1ee915b794c0..2757b621091c 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -63,12 +63,10 @@ config VIDEO_MUX config VIDEO_OMAP3 tristate "OMAP 3 Camera support" depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API - depends on ARCH_OMAP3 || COMPILE_TEST - depends on ARM + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST) depends on COMMON_CLK depends on HAS_DMA && OF - depends on OMAP_IOMMU - select ARM_DMA_USE_IOMMU + select ARM_DMA_USE_IOMMU if OMAP_IOMMU select VIDEOBUF2_DMA_CONTIG select MFD_SYSCON select V4L2_FWNODE diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 16c50099cccd..b8c8761a76b6 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -61,7 +61,9 @@ #include #include +#ifdef CONFIG_ARM_DMA_USE_IOMMU #include +#endif #include #include @@ -1938,12 +1940,15 @@ static int isp_initialize_modules(struct isp_device *isp) static void isp_detach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM_DMA_USE_IOMMU arm_iommu_release_mapping(isp->mapping); isp->mapping = NULL; +#endif } static int isp_attach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM_DMA_USE_IOMMU struct dma_iommu_mapping *mapping; int ret; @@ -1972,6 +1977,9 @@ static int isp_attach_iommu(struct isp_device *isp) error: isp_detach_iommu(isp); return ret; +#else + return -ENODEV; +#endif } /*
Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
Em Sat, 7 Apr 2018 14:40:08 +0300 Laurent Pinchartescreveu: > The omap3isp driver can't be compiled on non-ARM platforms but has no > compile-time dependency on OMAP. It however requires common clock > framework support, which isn't provided by all ARM platforms. > > Drop the OMAP dependency when COMPILE_TEST is set and add ARM and > COMMON_CLK dependencies. > > Signed-off-by: Laurent Pinchart > --- > drivers/media/platform/Kconfig | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > Hi Mauro, > > While we continue the discussions on whether the ARM IOMMU functions should be > stubbed in the omap3isp driver itself or not, I propose already merging this > patch that will extend build coverage for the omap3isp driver. Extending > compilation to non-ARM platforms can then be added on top, depending on the > result of the discussion. Ok, I'll add it before the patch with the approach proposed by Arnd. > You might have noticed the 0-day build bot report reporting that the driver > depends on the common clock framework (build failure on openrisc). The issue > affects ARM as well as not all ARM platforms use the common clock framework. > I've thus also added a dependency on COMMON_CLK. Note that this dependency can > prevent compilation on x86 platforms. Actually, it doesn't. COMMON_CLK can be selected on x86. It actually doesn't depend on anything. So, build failures due to that happens only with randconfigs. So, it is fine to make OMAP3ISP depending on COMMON_CLK. Thanks, Mauro
Re: [PATCH 6/6] media: ov772x: support device tree probing
Hi Akinobu, On Sun, Apr 08, 2018 at 12:48:10AM +0900, Akinobu Mita wrote: > The ov772x driver currently only supports legacy platform data probe. > This change enables device tree probing. > > Note that the platform data probe can select auto or manual edge control > mode, but the device tree probling can only select auto edge control mode > for now. > > Cc: Jacopo Mondi> Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > drivers/media/i2c/ov772x.c | 60 > ++ > 1 file changed, 40 insertions(+), 20 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 5e91fa1..e67ec37 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -763,13 +763,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_VFLIP: > val = ctrl->val ? VFLIP_IMG : 0x00; > priv->flag_vflip = ctrl->val; > - if (priv->info->flags & OV772X_FLAG_VFLIP) > + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP)) > val ^= VFLIP_IMG; > return ov772x_mask_set(client, COM3, VFLIP_IMG, val); > case V4L2_CID_HFLIP: > val = ctrl->val ? HFLIP_IMG : 0x00; > priv->flag_hflip = ctrl->val; > - if (priv->info->flags & OV772X_FLAG_HFLIP) > + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP)) > val ^= HFLIP_IMG; > return ov772x_mask_set(client, COM3, HFLIP_IMG, val); > case V4L2_CID_BAND_STOP_FILTER: > @@ -928,19 +928,14 @@ static void ov772x_select_params(const struct > v4l2_mbus_framefmt *mf, > *win = ov772x_select_win(mf->width, mf->height); > } > > -static int ov772x_set_params(struct ov772x_priv *priv, > - const struct ov772x_color_format *cfmt, > - const struct ov772x_win_size *win) > +static int ov772x_edgectrl(struct ov772x_priv *priv) > { > struct i2c_client *client = v4l2_get_subdevdata(>subdev); > - struct v4l2_fract tpf; > int ret; > - u8 val; > > - /* Reset hardware. */ > - ov772x_reset(client); > + if (!priv->info) > + return 0; > > - /* Edge Ctrl. */ > if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) { > /* >* Manual Edge Control Mode. > @@ -951,19 +946,19 @@ static int ov772x_set_params(struct ov772x_priv *priv, > > ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00); > if (ret < 0) > - goto ov772x_set_fmt_error; > + return ret; > > ret = ov772x_mask_set(client, > EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK, > priv->info->edgectrl.threshold); > if (ret < 0) > - goto ov772x_set_fmt_error; > + return ret; > > ret = ov772x_mask_set(client, > EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK, > priv->info->edgectrl.strength); > if (ret < 0) > - goto ov772x_set_fmt_error; > + return ret; > > } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) { > /* > @@ -975,15 +970,35 @@ static int ov772x_set_params(struct ov772x_priv *priv, > EDGE_UPPER, OV772X_EDGE_UPPER_MASK, > priv->info->edgectrl.upper); > if (ret < 0) > - goto ov772x_set_fmt_error; > + return ret; > > ret = ov772x_mask_set(client, > EDGE_LOWER, OV772X_EDGE_LOWER_MASK, > priv->info->edgectrl.lower); > if (ret < 0) > - goto ov772x_set_fmt_error; > + return ret; > } > > + return 0; > +} > + > +static int ov772x_set_params(struct ov772x_priv *priv, > + const struct ov772x_color_format *cfmt, > + const struct ov772x_win_size *win) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(>subdev); > + struct v4l2_fract tpf; > + int ret; > + u8 val; > + > + /* Reset hardware. */ > + ov772x_reset(client); > + > + /* Edge Ctrl. */ > + ret = ov772x_edgectrl(priv); > + if (ret < 0) > + goto ov772x_set_fmt_error; > + > /* Format and window size. */ > ret = ov772x_write(client, HSTART, win->rect.left >> 2); > if (ret < 0) > @@
Re: Regression: DVBSky S960 USB tuner doesn't work in 4.10 or newer
On Wed, Apr 04, 2018 at 02:41:51PM +0300, Olli Salonen wrote: > Hello Peter and Max, > > I noticed that when using kernel 4.10 or newer my DVBSky S960 and > S960CI satellite USB TV tuners stopped working properly. Basically, > they will fail at one point when tuning to a channel. This typically > takes less than 100 tuning attempts. For perspective, when performing > a full channel scan on my system, the tuner tunes at least 500 times. > After the tuner fails, I need to reboot the PC (probably unloading the > driver and loading it again would do). > > 2018-04-04 10:17:36.756 [ INFO] mpegts: 12149H in 4.8E - tuning on > Montage Technology M88DS3103 : DVB-S #0 > 2018-04-04 10:17:37.159 [ ERROR] diseqc: failed to send diseqc cmd > (e=Connection timed out) > 2018-04-04 10:17:37.160 [ INFO] mpegts: 12265H in 4.8E - tuning on > Montage Technology M88DS3103 : DVB-S #0 > 2018-04-04 10:17:37.535 [ ERROR] diseqc: failed to send diseqc cmd > (e=Connection timed out) > > I did a kernel bisect between 4.9 and 4.10. It seems the commit that > breaks my tuner is the following one: > > 9d659ae14b545c4296e812c70493bfdc999b5c1c is the first bad commit > commit 9d659ae14b545c4296e812c70493bfdc999b5c1c > Author: Peter Zijlstra> Date: Tue Aug 23 14:40:16 2016 +0200 > > locking/mutex: Add lock handoff to avoid starvation > > I couldn't easily revert that commit only. I can see that the > drivers/media/usb/dvb-usb-v2/dvbsky.c driver does use mutex_lock() and > mutex_lock_interruptible() in a few places. > > Do you guys see anything that's obviously wrong in the way the mutexes > are used in dvbsky.c or anything in that particular patch that could > cause this issue? Nothing, sorry.. really weird. That driver looks fairly straight forward with respect to mutex usage (although obviously I have less than 0 clue on the whole usb media thing). That it breaks that driver would suggest something funny with it though; because the kernel has loads and loads of mutexes in and they all appear to work well with that patch -- in fact, it fixed a reported starvation case. The only way for that patch to affect things is if there is contention on the mutex though; so who or what is also trying to acquire the mutex? The reported error is a timeout, suggesting that whoever is contending on the lock is keeping it held too long? I do notice that dvbsky_stream_ctrl() has an msleep() while holding a mutex. Do you have any idea which of the 3 (afaict) mutexes in that driver is failing? Going by the fact that it's failing to send, I'd hazard a guess it's the i2c mutex, but again, I have less than 0 clues about i2c.
Re: [PATCH 5/6] media: ov772x: add device tree binding
Hi Akinobu, On Sun, Apr 08, 2018 at 12:48:09AM +0900, Akinobu Mita wrote: > This adds a device tree binding documentation for OV7720/OV7725 sensor. Please use as patch subject media: dt-bindings: > > Cc: Jacopo Mondi> Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Cc: Rob Herring > Signed-off-by: Akinobu Mita > --- > .../devicetree/bindings/media/i2c/ov772x.txt | 36 > ++ > MAINTAINERS| 1 + > 2 files changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > new file mode 100644 > index 000..9b0df3b > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > @@ -0,0 +1,36 @@ > +* Omnivision OV7720/OV7725 CMOS sensor > + Could you please provide a brief description of the sensor (supported resolution and formats is ok) > +Required Properties: > +- compatible: shall be one of > + "ovti,ov7720" > + "ovti,ov7725" > +- clocks: reference to the xclk input clock. > +- clock-names: shall be "xclk". > + > +Optional Properties: > +- rstb-gpios: reference to the GPIO connected to the RSTB pin, if any. > +- pwdn-gpios: reference to the GPIO connected to the PWDN pin, if any. As a general note: This is debated, and I'm not enforcing it, but please consider using generic names for GPIOs with common functions. In this case "reset-gpios" and "powerdown-gpios". Also please indicate the GPIO active level in bindings description. For this specific driver: The probe routine already looks for a GPIO named 'pwdn', so I guess the DT bindings should use the same name. Unless you're willing to change it in the board files that register it (Migo-R only in mainline) and use the generic 'powerdown' name for both. Either is fine with me. There is no support for the reset GPIO in the driver code, it supports soft reset only. Either ditch it from bindings or add support for it in driver's code. Thanks j > + > +The device node shall contain one 'port' child node with one child 'endpoint' > +subnode for its digital output video port, in accordance with the video > +interface bindings defined in Documentation/devicetree/bindings/media/ > +video-interfaces.txt. > + > +Example: > + > + { > + ov772x: camera@21 { > + compatible = "ovti,ov7725"; > + reg = <0x21>; > + rstb-gpios = <_gpio_0 0 GPIO_ACTIVE_LOW>; > + pwdn-gpios = <_gpio_0 1 GPIO_ACTIVE_LOW>; > + clocks = <>; > + clock-names = "xclk"; > + > + port { > + ov772x_0: endpoint { > + remote-endpoint = <_in0>; > + }; > + }; > + }; > +}; > diff --git a/MAINTAINERS b/MAINTAINERS > index 7e48624..3e0224a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10295,6 +10295,7 @@ T:git git://linuxtv.org/media_tree.git > S: Odd fixes > F: drivers/media/i2c/ov772x.c > F: include/media/i2c/ov772x.h > +F: Documentation/devicetree/bindings/media/i2c/ov772x.txt > > OMNIVISION OV7740 SENSOR DRIVER > M: Wenyou Yang > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST
On Sat, Apr 7, 2018 at 3:14 PM, Mauro Carvalho Chehabwrote: > Em Sat, 07 Apr 2018 14:56:59 +0300 > Laurent Pinchart escreveu: >> On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote: >> > Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu: >> > > If you want to make drivers compile for all architectures, the APIs they >> > > use must be defined in linux/, not in asm/. They can be stubbed there >> > > when not implemented in a particular architecture, but not in the driver. >> > >> > In this specific case, the same approach taken here is already needed >> > by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside >> > drivers/iommu: >> > >> > drivers/iommu/ipmmu-vmsa.c >> >> The reason there is different, the driver is shared by ARM32 and ARM64 >> platforms. Furthermore, there's an effort (or at least there was) to move >> away >> from those APIs in the driver, but I think it has stalled. > > Anyway, the approach sticks at the driver. As this was accepted even > inside drivers/iommu, I fail to see why not doing the same on media, > specially since it really helps us to find bugs at omap3isp driver. > > Even without pushing upstream (where Coverity would analyze it), > we got lots of problems by simply letting omap3isp to use the > usual tools we use for all other drivers. > >> > Also, this API is used only by 3 drivers [1]: >> > >> > drivers/iommu/ipmmu-vmsa.c >> > drivers/iommu/mtk_iommu_v1.c >> > drivers/media/platform/omap3isp/isp.c >> > >> > [1] as blamed by >> > git grep -l arm_iommu_create_mapping >> >> The exynos driver also uses it. >> >> > That hardly seems to be an arch-specific iommu solution, but, instead, some >> > hack used by only three drivers or some legacy iommu binding. >> >> It's more complex than that. There are multiple IOMMU-related APIs on ARM, so >> more recent than others, with different feature sets. While I agree that >> drivers should move away from arm_iommu_create_mapping(), doing so requires >> coordination between the IOMMU driver and the bus master driver (for instance >> the omap3isp driver). It's not a trivial matter, but I'd love if someone >> submitted patches :-) > > If someone steps up to do that, it would be really helpful, but we > should not trust that this will happen. OMAP3 is an old hardware, > and not many developers are working on improving its support. Considering its age, I still see a lot of changes on the arch/arm side of it, so I wouldn't give up the hope yet. >> > The omap3isp is, btw, the only driver outside drivers/iommu that needs it. >> > >> > So, it sounds that other driver uses some other approach, but hardly >> > it would be worth to change this driver to use something else. >> > >> > So, better to stick with the same solution the Renesas driver used. >> >> I'm not responsible for that solution and I didn't think it was a good one at >> the time it was introduced, but in any case it is not meant at all to support >> COMPILE_TEST. I still don't think the omap3isp driver should declare stubs >> for >> these functions for the purpose of supporting compile-testing on x86. > > Well, there is another alternative. We could do add this to its Makefile: > > ifndef CONFIG_ARCH_ARM > ccflags-y += -I./arch/arm/include/ > endif > > And add those stubs to arch/arm/include/asm/dma-iommu.h, > in order to be used when CONFIG_IOMMU_DMA isn't defined: > > #define arm_iommu_create_mapping(...) NULL > #define arm_iommu_attach_device(...)-ENODEV > #define arm_iommu_release_mapping(...) do {} while (0) > #define arm_iommu_detach_device(...)do {} while (0) > > If done right, such solution could also be used to remove > the #ifdef inside drivers/iommu/ipmmu-vmsa.c. > > Yet, I think that the approach I proposed before is better, > but maybe arm maintainers may have a different opinion. > > Arnd, > > What do you think? I think including a foreign architecture header is worse than your earlier patch, I'd rather see a local hack in the driver. I haven't tried it, but how about something simpler like what I have below. Arnd (in case it works and you want to pick it up with a proper changelog): Signed-off-by: Arnd Bergmann diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..625f2e407929 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -1945,12 +1945,15 @@ static int isp_initialize_modules(struct isp_device *isp) static void isp_detach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM arm_iommu_release_mapping(isp->mapping); isp->mapping = NULL; +#endif } static int isp_attach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM struct dma_iommu_mapping *mapping; int ret; @@ -1979,6 +1982,9 @@ static int isp_attach_iommu(struct isp_device *isp) error:
Re: [PATCH 4/6] media: ov772x: add media controller support
Hi Akinobu, On Sun, Apr 08, 2018 at 12:48:08AM +0900, Akinobu Mita wrote: > Create a source pad and set the media controller type to the sensor. > > Cc: Jacopo Mondi> Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > drivers/media/i2c/ov772x.c | 22 -- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 4bb81ff..5e91fa1 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -425,6 +425,9 @@ struct ov772x_priv { > unsigned shortband_filter; > unsigned int fps; > int (*reg_read)(struct i2c_client *client, u8 addr); > +#ifdef CONFIG_MEDIA_CONTROLLER > + struct media_pad pad; > +#endif > }; > > /* > @@ -1328,9 +1331,17 @@ static int ov772x_probe(struct i2c_client *client, > goto error_clk_put; > } > > - ret = ov772x_video_probe(priv); > +#ifdef CONFIG_MEDIA_CONTROLLER > + priv->pad.flags = MEDIA_PAD_FL_SOURCE; > + priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + ret = media_entity_pads_init(>subdev.entity, 1, >pad); > if (ret < 0) > goto error_gpio_put; > +#endif > + > + ret = ov772x_video_probe(priv); > + if (ret < 0) > + goto error_entity_cleanup; If you remove the #ifdef around the media_entity_cleanup() below, I suggest moving video_probe() before the entity intialization so you don't have to #ifdef around the error_gpio_put: label, which otherwise the compiler complains for being defined but not used. > > priv->cfmt = _cfmts[0]; > priv->win = _win_sizes[0]; > @@ -1338,11 +1349,15 @@ static int ov772x_probe(struct i2c_client *client, > > ret = v4l2_async_register_subdev(>subdev); > if (ret) > - goto error_gpio_put; > + goto error_entity_cleanup; > > return 0; > > +error_entity_cleanup: > +#ifdef CONFIG_MEDIA_CONTROLLER > + media_entity_cleanup(>subdev.entity); media_entity_cleanup() resolves to a nop if CONFIG_MEDIA_CONTROLLER is not defined > error_gpio_put: > +#endif > if (priv->pwdn_gpio) > gpiod_put(priv->pwdn_gpio); > error_clk_put: > @@ -1357,6 +1372,9 @@ static int ov772x_remove(struct i2c_client *client) > { > struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client)); > > +#ifdef CONFIG_MEDIA_CONTROLLER > + media_entity_cleanup(>subdev.entity); ditto Thanks j > +#endif > clk_put(priv->clk); > if (priv->pwdn_gpio) > gpiod_put(priv->pwdn_gpio); > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH v2] Add udmabuf misc device
On Fri, Apr 06, 2018 at 02:24:46PM +0200, Christian König wrote: > Am 06.04.2018 um 11:33 schrieb Gerd Hoffmann: > >Hi, > > > > > The pages backing a DMA-buf are not allowed to move (at least not without > > > a > > > patch set I'm currently working on), but for certain MM operations to work > > > correctly you must be able to modify the page tables entries and move the > > > pages backing them around. > > > > > > For example try to use fork() with some copy on write pages with this > > > approach. You will find that you have only two options to correctly handle > > > this. > > The fork() issue should go away with shared memory pages (no cow). > > I guess this is the reason why vgem is internally backed by shmem. > > Yes, exactly that is also an approach which should work fine. Just don't try > to get this working with get_user_pages(). > > > > > Hmm. So I could try to limit the udmabuf driver to shmem too (i.e. > > have the ioctl take a shmem filehandle and offset instead of a virtual > > address). > > > > But maybe it is better then to just extend vgem, i.e. add support to > > create gem objects from existing shmem. > > > > Comments? > > Yes, extending vgem instead of creating something new sounds like a good > idea to me as well. +1 on adding a vgem "import from shmem/memfd" ioctl. Sounds like a good idea, and generally useful. We might want to limit to memfd though for semantic reasons: dma-buf have invariant size, shmem not so much. memfd can be locked down to not change their size anymore. And iirc the core mm page invalidation protocol around truncate() is about as bad as get_user_pages vs cow :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [RFCv9 PATCH 15/29] videodev2.h: add request_fd field to v4l2_ext_controls
Hi Hans, On Wed, Mar 28, 2018 at 03:50:16PM +0200, Hans Verkuil wrote: > From: Alexandre Courbot> > If which is V4L2_CTRL_WHICH_REQUEST, then the request_fd field can be > used to specify a request for the G/S/TRY_EXT_CTRLS ioctls. > > Signed-off-by: Alexandre Courbot > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 5 - > drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++--- > include/uapi/linux/videodev2.h| 4 +++- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index 5198c9eeb348..0782b3666deb 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -732,7 +732,8 @@ struct v4l2_ext_controls32 { > __u32 which; > __u32 count; > __u32 error_idx; > - __u32 reserved[2]; > + __s32 request_fd; > + __u32 reserved[1]; No need for an array anymore. > compat_caddr_t controls; /* actually struct v4l2_ext_control32 * */ > }; > > @@ -807,6 +808,7 @@ static int get_v4l2_ext_controls32(struct file *file, > get_user(count, >count) || > put_user(count, >count) || > assign_in_user(>error_idx, >error_idx) || > + assign_in_user(>request_fd, >request_fd) || > copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) And this can be assign_in_user(). > return -EFAULT; > > @@ -865,6 +867,7 @@ static int put_v4l2_ext_controls32(struct file *file, > get_user(count, >count) || > put_user(count, >count) || > assign_in_user(>error_idx, >error_idx) || > + assign_in_user(>request_fd, >request_fd) || Ditto. > copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved)) || > get_user(kcontrols, >controls)) > return -EFAULT; > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c > index a5dab16ff2d2..2c623da33155 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -553,8 +553,8 @@ static void v4l_print_ext_controls(const void *arg, bool > write_only) > const struct v4l2_ext_controls *p = arg; > int i; > > - pr_cont("which=0x%x, count=%d, error_idx=%d", > - p->which, p->count, p->error_idx); > + pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d", > + p->which, p->count, p->error_idx, p->request_fd); > for (i = 0; i < p->count; i++) { > if (!p->controls[i].size) > pr_cont(", id/val=0x%x/0x%x", > @@ -870,7 +870,7 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, > int allow_priv) > __u32 i; > > /* zero the reserved fields */ > - c->reserved[0] = c->reserved[1] = 0; > + c->reserved[0] = 0; > for (i = 0; i < c->count; i++) > c->controls[i].reserved2[0] = 0; > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 600877be5c22..6f41baa53787 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -1592,7 +1592,8 @@ struct v4l2_ext_controls { > }; > __u32 count; > __u32 error_idx; > - __u32 reserved[2]; > + __s32 request_fd; > + __u32 reserved[1]; > struct v4l2_ext_control *controls; > }; > > @@ -1605,6 +1606,7 @@ struct v4l2_ext_controls { > #define V4L2_CTRL_MAX_DIMS (4) > #define V4L2_CTRL_WHICH_CUR_VAL 0 > #define V4L2_CTRL_WHICH_DEF_VAL 0x0f00 > +#define V4L2_CTRL_WHICH_REQUEST 0x0f01 > > enum v4l2_ctrl_type { > V4L2_CTRL_TYPE_INTEGER = 1, -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [RfC PATCH] Add udmabuf misc device
On Thu, Apr 05, 2018 at 05:11:17PM -0700, Matt Roper wrote: > On Thu, Apr 05, 2018 at 10:32:04PM +0200, Daniel Vetter wrote: > > Pulling this out of the shadows again. > > > > We now also have xen-zcopy from Oleksandr and the hyper dmabuf stuff > > from Matt and Dongwong. > > > > At least from the intel side there seems to be the idea to just have 1 > > special device that can handle cross-gues/host sharing for all kinds > > of hypervisors, so I guess you all need to work together :-) > > > > Or we throw out the idea that hyper dmabuf will be cross-hypervisor > > (not sure how useful/reasonable that is, someone please convince me > > one way or the other). > > > > Cheers, Daniel > > Dongwon (DW) is the one doing all the real work on hyper_dmabuf, but I'm > familiar with the use cases he's trying to address, and I think there > are a couple high-level goals of his work that are worth calling out as > we discuss the various options for sharing buffers produced in one VM > with a consumer running in another VM: > > * We should try to keep the interface/usage separate from the >underlying hypervisor implementation details. I.e., in DW's design >the sink/source drivers that handle the actual buffer passing in the >two VM's should provide a generic interface that does not depend on a >specific hypervisor. Behind the scenes there could be various >implementations for specific hypervisors (Xen, KVM, ACRN, etc.), and >some of those backends may have additional restrictions, but it would >be best if userspace didn't have to know the specific hypervisor >running on the system and could just query the general capabilities >available to it. We've already got projects in flight that are >wanting this functionality on Xen and ACRN today. Two comments on this: - Just because it's in drivers/gpu doesn't mean you can't use it for anything else. E.g. the xen-zcopy driver can very much be used for any dma-buf, there's nothing gpu specific with it - well besides that it resuses some useful DRM ioctls, but if that annoys you just do a #define TOTALLY_GENERIC DRM and be done :-) - Especially the kvm memory and hypervisor model seems totally different from other hypervisors, e.g. no real use for guest-guest sharing (which doesn't go through the host) and other cases. So trying to make something 100% generic seems like a bad idea. Wrt making it generic: Just use generic interfaces - if you can somehow use xen-front for the display sharing, then a) no need for hyper-dmabuf and b) already fully generic since it looks like a normal drm device to the guest userspace. > * The general interface should be able to express sharing from any >guest:guest, not just guest:host. Arbitrary G:G sharing might be >something some hypervisors simply aren't able to support, but the >userspace API itself shouldn't make assumptions or restrict that. I >think ideally the sharing API would include some kind of >query_targets interface that would return a list of VM's that your >current OS is allowed to share with; that list would be depend on the >policy established by the system integrator, but obviously wouldn't >include targets that the hypervisor itself wouldn't be capable of >handling. Uh ... has a proper security architect analyzed this idea? > * A lot of the initial use cases are in the realm of graphics, but this >shouldn't be a graphics-specific API. Buffers might contain other >types of content as well (e.g., audio). Really the content producer >could potentially be any driver (or userspace) running in the VM that >knows how to import/export dma_buf's (or maybe just import given >danvet's suggestion that we should make the sink driver do all the >actual memory allocation for any buffers that may be shared). See above, just because it uses drm ioctls doesn't make it gfx specific. Otoh making it even more graphics specific might be even better, i.e. just sharing the backend tech (grant tables or whatever), but having dedicated front-ents for each use-case so there's less code to type. > * We need to be able to handle cross-VM coordination of buffer usage as >well, so I think we'd want to include fence forwarding support in the >API as well to signal back and forth about production/consumption >completion. And of course document really well what should happen >if, for example, the entire VM you're sharing with/from dies. Implicit fencing has been proven to be a bad idea. Please do explicit passing of dma_fences (plus assorted protocol). > * The sharing API could be used to share multiple kinds of content in a >single system. The sharing sink driver running in the content >producer's VM should accept some additional metadata that will be >passed over to the target VM as well. The sharing source driver >running in the content consumer's VM would then be able to use
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 12:54:22PM +0200, Gerd Hoffmann wrote: > On Fri, Apr 06, 2018 at 10:52:21AM +0100, Daniel Stone wrote: > > Hi Gerd, > > > > On 14 March 2018 at 08:03, Gerd Hoffmannwrote: > > >> Either mlock account (because it's mlocked defacto), and get_user_pages > > >> won't do that for you. > > >> > > >> Or you write the full-blown userptr implementation, including > > >> mmu_notifier > > >> support (see i915 or amdgpu), but that also requires Christian Königs > > >> latest ->invalidate_mapping RFC for dma-buf (since atm exporting userptr > > >> buffers is a no-go). > > > > > > I guess I'll look at mlock accounting for starters then. Easier for > > > now, and leaves the door open to switch to userptr later as this should > > > be transparent to userspace. > > > > Out of interest, do you have usecases for full userptr support? Maybe > > another way would be to allow creation of dmabufs from memfds. > > I have two things in mind. > > One is vga emulation. I have virtual pci memory bar for the virtual > vga. qemu backs vga memory with anonymous pages right now, switching > that to shmem should be easy though if that makes things easier. Guest > places the framebuffer somewhere in the pci bar, and I want export the > chunk which represents the framebuffer as dma-buf to display it on the > host without copying around data. Framebuffer is linear in guest > physical memory, so a single block only. That is the simpler case. > > The more difficuilt one is virtio-gpu ressources. virtio-gpu resources > live in host memory (guest has no direct access). The guest can > optionally specify guest memory pages as backing storage for the > resource. Guest backing storage is allowed to be scattered. Commands > exist to copy both ways between host storage and guest backing. > > With virgl (opengl acceleration) enabled the guest will send rendering > commands to fill the framebuffer ressource, so there is no need to copy > content to the framebuffer ressource. The guest may fill other > resources such as textures used for rendering with copy commands. > > Without acceleration the guest does software-rendering to the backing > storage, then sends a command to copy the framebuffer content from guest > backing storage to host ressource. > > Now it would be useful to allow a shared mapping, so no copying between > guest backing storage and host resource is needed, especially for the > software rendering case (i.e. dumb gem buffers). Being able to export > guest dumb buffers to other host processes would be useful too, for > example to display guest windows seamlessly on the host wayland server. > > So getting a dma-buf for the guest backing storage via udmabuf looked > like a useful approach. We can export the guest gem buffers to other > host processes that way. qemu itself could map it too, to get a linear > representation of the scattered guest backing storage. > > The other obvious approach would be to do it the other way around and > allow the guest map the host resource somehow. On the host side qemu > could use vgem to allocate resource memory, so it'll be a gem object > already. Mapping that into the guest isn't that straight-forward > though. The guest manages its physical address space, so the guest > would need to find a free spot and ask the host to place the resource > there. Then the guest needs page structs covering the mapped resource, > so it can work with it. Didn't investigate how difficuilt that is. Use > memory hotplug maybe? Can we easily unmap the resource then? Also I > think updating the guests physical memory layout (which we would need to > do on every resource map/unmap) isn't an exactly cheap operation ... Generally we try to cache mappings as much as possible. And wrt finding a slot: Create a sufficiently sized BAR on the virgl device, just for that? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 2/6] media: ov772x: add checks for register read errors
Hi Akinobu, On Sun, Apr 08, 2018 at 12:48:06AM +0900, Akinobu Mita wrote: > This change adds checks for register read errors and returns correct > error code. > I feel like error conditions are anyway captured by the switch() default case, but I understand there may be merits in returning the actual error code. > Cc: Jacopo Mondi> Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > drivers/media/i2c/ov772x.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 283ae2c..c56f910 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -1169,8 +1169,15 @@ static int ov772x_video_probe(struct ov772x_priv *priv) > return ret; > > /* Check and show product ID and manufacturer ID. */ > - pid = ov772x_read(client, PID); > - ver = ov772x_read(client, VER); > + ret = ov772x_read(client, PID); > + if (ret < 0) > + return ret; > + pid = ret; > + > + ret = ov772x_read(client, VER); > + if (ret < 0) > + return ret; > + ver = ret; You can assign the ov772x_read() return value to pid and ver directly and save two assignments. > > switch (VERSION(pid, ver)) { > case OV7720: If we want to check for return values here, which is always a good thing, could you do the same for MIDH and MIDL below? Nit: You can also fix the dev_info() parameters alignment to span to the whole line length while at there. Ie. dev_info(>dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n", devname, pid, ver, ov772x_read(client, MIDH), ov772x_read(client, MIDL)); Thanks j > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [RFCv9 PATCH 07/29] media-request: add media_request_object_find
Hi Hans, On Wed, Mar 28, 2018 at 03:50:08PM +0200, Hans Verkuil wrote: > From: Hans Verkuil> > Add media_request_object_find to find a request object inside a > request based on ops and/or priv values. > > Objects of the same type (vb2 buffer, control handler) will have > the same ops value. And objects that refer to the same 'parent' > object (e.g. the v4l2_ctrl_handler that has the current driver > state) will have the same priv value. > > The caller has to call media_request_object_put() for the returned > object since this function increments the refcount. > > Signed-off-by: Hans Verkuil > --- > drivers/media/media-request.c | 26 ++ > include/media/media-request.h | 25 + > 2 files changed, 51 insertions(+) > > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > index d54fd353d8a6..10a05dd7b571 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -309,6 +309,32 @@ static void media_request_object_release(struct kref > *kref) > obj->ops->release(obj); > } > > +struct media_request_object * > +media_request_object_find(struct media_request *req, > + const struct media_request_object_ops *ops, > + void *priv) > +{ > + struct media_request_object *obj; > + struct media_request_object *found = NULL; > + unsigned long flags; > + > + if (!ops && !priv) > + return NULL; > + > + spin_lock_irqsave(>lock, flags); > + list_for_each_entry(obj, >objects, list) { > + if ((!ops || obj->ops == ops) && > + (!priv || obj->priv == priv)) { Do you have a use case for having matching priv but mismatching ops? I think it'd be useful to require drivers to provide unique priv, considering that it's risky to let them use the same while the VB2 request object ops are always the same. > + media_request_object_get(obj); > + found = obj; > + break; > + } > + } > + spin_unlock_irqrestore(>lock, flags); > + return found; > +} > +EXPORT_SYMBOL_GPL(media_request_object_find); > + > void media_request_object_put(struct media_request_object *obj) > { > kref_put(>kref, media_request_object_release); > diff --git a/include/media/media-request.h b/include/media/media-request.h > index c01b05570a31..570f3a205776 100644 > --- a/include/media/media-request.h > +++ b/include/media/media-request.h > @@ -122,6 +122,23 @@ static inline void media_request_object_get(struct > media_request_object *obj) > */ > void media_request_object_put(struct media_request_object *obj); > > +/** > + * media_request_object_find - Find an object in a request > + * > + * @ops: Find an object with this ops value, may be NULL. > + * @priv: Find an object with this priv value, may be NULL. > + * > + * At least one of @ops and @priv must be non-NULL. If one of > + * these is NULL, then skip checking for that field. > + * > + * Returns NULL if not found or the object (the refcount is increased > + * in that case). > + */ > +struct media_request_object * > +media_request_object_find(struct media_request *req, > + const struct media_request_object_ops *ops, > + void *priv); > + > /** > * media_request_object_init - Initialise a media request object > * > @@ -154,6 +171,14 @@ static inline void media_request_object_put(struct > media_request_object *obj) > { > } > > +static inline struct media_request_object * > +media_request_object_find(struct media_request *req, > + const struct media_request_object_ops *ops, > + void *priv) > +{ > + return NULL; > +} > + > static inline void media_request_object_init(struct media_request_object > *obj) > { > obj->ops = NULL; > -- > 2.16.1 > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH 1/6] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
Hello Akinobu, thank you for the patch. On which platform have you tested the series (just curious) ? On Sun, Apr 08, 2018 at 12:48:05AM +0900, Akinobu Mita wrote: > The ov772x driver only works when the i2c controller have > I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't > support it. > > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that > it doesn't support repeated starts. > > This change adds an alternative method for reading from ov772x register > which uses two separated i2c messages for the i2c controllers without > I2C_FUNC_PROTOCOL_MANGLING. Actually, and please correct me if I'm wrong, what I see here is that an i2c_master_send+i2c_master_recv sequence is equivalent to a mangled smbus transaction: i2c_smbus_read_byte_data | I2C_CLIENT_SCCB: S Addr Wr [A] Comm [A] P S Addr Rd [A] [Data] NA P i2c_master_send() + i2c_master_recv(): S Addr Wr [A] Data [A] P S Addr Rd [A] [Data] NA P I wonder if it is not worth to ditch the existing read() function in favour of your new proposed one completely. I have tested it on the Migo-R board where I have an ov772x installed and it works fine. Although I would like to have a confirmation this is fine by people how has seen more i2c adapters in action than me :) Thanks j > > Cc: Jacopo Mondi> Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > drivers/media/i2c/ov772x.c | 42 +- > 1 file changed, 33 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index b62860c..283ae2c 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -424,6 +424,7 @@ struct ov772x_priv { > /* band_filter = COM8[5] ? 256 - BDBASE : 0 */ > unsigned shortband_filter; > unsigned int fps; > + int (*reg_read)(struct i2c_client *client, u8 addr); > }; > > /* > @@ -542,11 +543,34 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev > *sd) > return container_of(sd, struct ov772x_priv, subdev); > } > > -static inline int ov772x_read(struct i2c_client *client, u8 addr) > +static int ov772x_read(struct i2c_client *client, u8 addr) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct ov772x_priv *priv = to_ov772x(sd); > + > + return priv->reg_read(client, addr); > +} > + > +static int ov772x_reg_read(struct i2c_client *client, u8 addr) > { > return i2c_smbus_read_byte_data(client, addr); > } > > +static int ov772x_reg_read_fallback(struct i2c_client *client, u8 addr) > +{ > + int ret; > + u8 val; > + > + ret = i2c_master_send(client, , 1); > + if (ret < 0) > + return ret; > + ret = i2c_master_recv(client, , 1); > + if (ret < 0) > + return ret; > + > + return val; > +} > + > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value) > { > return i2c_smbus_write_byte_data(client, addr, value); > @@ -1255,20 +1279,20 @@ static int ov772x_probe(struct i2c_client *client, > return -EINVAL; > } > > - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > - I2C_FUNC_PROTOCOL_MANGLING)) { > - dev_err(>dev, > - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or > PROTOCOL_MANGLING\n"); > - return -EIO; > - } > - client->flags |= I2C_CLIENT_SCCB; > - > priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > > priv->info = client->dev.platform_data; > > + if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_PROTOCOL_MANGLING)) > + priv->reg_read = ov772x_reg_read; > + else > + priv->reg_read = ov772x_reg_read_fallback; > + > + client->flags |= I2C_CLIENT_SCCB; > + > v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); > v4l2_ctrl_handler_init(>hdl, 3); > v4l2_ctrl_new_std(>hdl, _ctrl_ops, > -- > 2.7.4 > signature.asc Description: PGP signature
Grüße von Frau Jordanka
Nachricht des Absenders: /Guten Morgen, Bitte schämen Sie sich nicht, diese Nachricht zu lesen, weil Sie der beabsichtigte Empfänger sind. Ich habe deine E-Mail-Adresse über ein Verzeichnis gesehen und ich habe dich kontaktiert, weil ich dringend deine Hilfe benötigt habe. Mein Name ist Frau Yordanka Velikova Nencheva. Ich bin 69 Jahre alt und bin rechtmäßig mit dem verstorbenen Herrn Peter Nencheva verheiratet. Ich und mein verstorbener Ehemann waren beide Bürger von Bulgarien, aber wir lebten hier in der Elfenbeinküste in Westafrika seit 28 Jahren. Mein verstorbener Ehemann arbeitete viele Jahre lang als Expatriate hier in der Elfenbeinküste, bevor er bei einem tödlichen Autounfall starb, bei dem er und sein Fahrer hier in der Elfenbeinküste ums Leben kamen. Ich und mein verstorbener Mann waren beide verheiratet, ohne ein eigenes leibliches Kind. Ich habe derzeit eine schwere Brustkrebskrankheit und schreibe diese Nachricht mit Hilfe eines Arztes hier im Krankenhaus in der Elfenbeinküste. Als mein verstorbener Ehemann noch am Leben war, hinterlegte er die Summe von einer Million fünfhunderttausend Euro (1,500,000.00 Euro) bei der Westra Wermlands Sparbank http://www.warbarbank.se/ in Schweden, die er mit der Bank eine Vereinbarung unterzeichnete die Mittel werden für wohltätige Zwecke verwendet, wie z. Hilfe für die mutterlosen Babys, Unterstützung der kirchlichen Aktivitäten und Unterstützung der nicht privilegierten Kinder, ihre Ausbildung fortzusetzen. Mein Mann hat diese Entscheidung für diese karitative Arbeit getroffen, weil wir kein Kind haben, das die Gelder in der schwedischen Bank erbt, und er wurde von einem Waisenhaus in Sofia, Bulgarien, erzogen, weil er keine Familie in Bulgarien hat. Ich kontaktierte Sie heute, weil mein Arzt mich gerade darüber informiert hat, dass meine Brustkrebskrankheit am schlimmsten geworden ist und sie nächste Woche eine schwere / tödliche Krebsoperation durchführen werden, die ich vielleicht nicht überleben würde, weil der Tod tief in meinem Körpersystem gefressen hat. Ich fürchte, wenn ich nächste Woche während meiner Operation sterbe, werden die Gelder in der schwedischen Bank vergessen, weil ich keinen anderen Verwandten habe, der behauptet, dass ich keine Familie meines verstorbenen Ehemannes in Bulgarien finden kann. Hinweis: Ich möchte, dass Sie mir helfen, das Geld auf Ihr eigenes Bankkonto zu überweisen und es auf Ihr Bankkonto zu überweisen. Sie werden 30% (450,000.00 Euro) des Gesamtbetrages für sich selbst als Entschädigung einstreichen und die restlichen 70% (1,50, 000,00 Euro) für die Wohltätigkeitsarbeit in Ihrem Land verwenden. Bitte antworten Sie mir dringend, wenn Sie daran interessiert sind, mir bei dieser Transaktion zu helfen, damit ich Ihnen weitere Einzelheiten zusenden kann, da ich möchte, dass die Mittel vor meiner bevorstehenden Operation nächste Woche überwiesen werden. Mit freundlichen Grüßen, Frau Yordanka Nencheva Velikova/ Parador de Alcañiz [1] [1] http://www.parador.es/de/paradores/parador-de-alcaniz