[PATCH] media: imx258: remove test pattern map from driver
From: "Chen, JasonX Z"
Test Pattern mode be picked at HAL instead of driver.
do a FLIP when userspace use test pattern mode.
add entity_ops for validating imx258 link.
Signed-off-by: Chen, JasonX Z
---
drivers/media/i2c/imx258.c | 28
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 31a1e22..71f9875 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -62,11 +62,6 @@
/* Test Pattern Control */
#define IMX258_REG_TEST_PATTERN0x0600
-#define IMX258_TEST_PATTERN_DISABLE0
-#define IMX258_TEST_PATTERN_SOLID_COLOR1
-#define IMX258_TEST_PATTERN_COLOR_BARS 2
-#define IMX258_TEST_PATTERN_GREY_COLOR 3
-#define IMX258_TEST_PATTERN_PN94
/* Orientation */
#define REG_MIRROR_FLIP_CONTROL0x0101
@@ -504,20 +499,12 @@ struct imx258_mode {
static const char * const imx258_test_pattern_menu[] = {
"Disabled",
- "Color Bars",
"Solid Color",
+ "Color Bars",
"Grey Color Bars",
"PN9"
};
-static const int imx258_test_pattern_val[] = {
- IMX258_TEST_PATTERN_DISABLE,
- IMX258_TEST_PATTERN_COLOR_BARS,
- IMX258_TEST_PATTERN_SOLID_COLOR,
- IMX258_TEST_PATTERN_GREY_COLOR,
- IMX258_TEST_PATTERN_PN9,
-};
-
/* Configurations for supported link frequencies */
#define IMX258_LINK_FREQ_634MHZ63360ULL
#define IMX258_LINK_FREQ_320MHZ32000ULL
@@ -752,7 +739,6 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl->handler, struct imx258, ctrl_handler);
struct i2c_client *client = v4l2_get_subdevdata(>sd);
int ret = 0;
-
/*
* Applying V4L2 control value only happens
* when power is up for streaming
@@ -778,13 +764,10 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEST_PATTERN:
ret = imx258_write_reg(imx258, IMX258_REG_TEST_PATTERN,
IMX258_REG_VALUE_16BIT,
- imx258_test_pattern_val[ctrl->val]);
-
+ ctrl->val);
ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
IMX258_REG_VALUE_08BIT,
- ctrl->val == imx258_test_pattern_val
- [IMX258_TEST_PATTERN_DISABLE] ?
- REG_CONFIG_MIRROR_FLIP :
+ !ctrl->val?REG_CONFIG_MIRROR_FLIP :
REG_CONFIG_FLIP_TEST_PATTERN);
break;
default:
@@ -1105,6 +1088,10 @@ static int imx258_identify_module(struct imx258 *imx258)
.pad = _pad_ops,
};
+static const struct media_entity_operations imx258_subdev_entity_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+};
+
static const struct v4l2_subdev_internal_ops imx258_internal_ops = {
.open = imx258_open,
};
@@ -1250,6 +1237,7 @@ static int imx258_probe(struct i2c_client *client)
/* Initialize subdev */
imx258->sd.internal_ops = _internal_ops;
+ imx258->sd.entity.ops = _subdev_entity_ops;
imx258->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
--
1.9.1
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Nov 7 05:00:11 CET 2018 media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708 media_build git hash: 0c8bb27f3aaa682b9548b656f77505c3d1f11e71 v4l-utils git hash: aa9d15a24894b884a39701a16998cdb43ecd7e01 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.0-2-amd64 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: WARNINGS linux-git-arm-multi: WARNINGS linux-git-arm-pxa: WARNINGS linux-git-arm-stm32: WARNINGS linux-git-arm64: WARNINGS linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-sh: OK linux-git-x86_64: WARNINGS Check COMPILE_TEST: OK linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.57-i686: ERRORS linux-3.16.57-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.123-i686: ERRORS linux-3.18.123-x86_64: ERRORS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.52-i686: ERRORS linux-4.1.52-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.159-i686: ERRORS linux-4.4.159-x86_64: ERRORS linux-4.5.7-i686: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-i686: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.10-i686: ERRORS linux-4.7.10-x86_64: ERRORS linux-4.8.17-i686: ERRORS linux-4.8.17-x86_64: ERRORS linux-4.9.131-i686: ERRORS linux-4.9.131-x86_64: ERRORS linux-4.10.17-i686: ERRORS linux-4.10.17-x86_64: ERRORS linux-4.11.12-i686: ERRORS linux-4.11.12-x86_64: ERRORS linux-4.12.14-i686: ERRORS linux-4.12.14-x86_64: ERRORS linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19.1-i686: OK linux-4.19.1-x86_64: OK linux-4.20-rc1-i686: OK linux-4.20-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Logs weren't copied as they are too large (1752 kB) The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
On 11/01/2018 08:03 PM, Sakari Ailus wrote: > Hi Yong, > > Thanks for the update! > > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: >> Hi, >> >> This series adds support for the Intel IPU3 (Image Processing Unit) >> ImgU which is essentially a modern memory-to-memory ISP. It implements >> raw Bayer to YUV image format conversion as well as a large number of >> other pixel processing algorithms for improving the image quality. >> >> Meta data formats are defined for image statistics (3A, i.e. automatic >> white balance, exposure and focus, histogram and local area contrast >> enhancement) as well as for the pixel processing algorithm parameters. >> The documentation for these formats is currently not included in the >> patchset but will be added in a future version of this set. >> >> The algorithm parameters need to be considered specific to a given frame >> and typically a large number of these parameters change on frame to frame >> basis. Additionally, the parameters are highly structured (and not a flat >> space of independent configuration primitives). They also reflect the >> data structures used by the firmware and the hardware. On top of that, >> the algorithms require highly specialized user space to make meaningful >> use of them. For these reasons it has been chosen video buffers to pass >> the parameters to the device. >> >> On individual patches: >> >> The heart of ImgU is the CSS, or Camera Subsystem, which contains the >> image processors and HW accelerators. >> >> The 3A statistics and other firmware parameter computation related >> functions are implemented in patch 11. >> >> All IPU3 pipeline default settings can be found in patch 10. >> >> To access DDR via ImgU's own memory space, IPU3 is also equipped with >> its own MMU unit, the driver is implemented in patch 6. >> >> Patch 7 uses above driver for DMA mapping operation. >> >> The communication between IPU3 firmware and driver is implemented with >> circular >> queues in patch 8. >> >> Patch 9 provide some utility functions and manage IPU3 fw download and >> install. >> >> The firmware which is called ipu3-fw.bin can be downloaded from: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git >> (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) >> >> Firmware ABI is defined in patches 4 and 5. >> >> Patches 12 and 13 are of the same file, the former contains all h/w >> programming >> related code, the latter implements interface functions for access fw & hw >> capabilities. >> >> Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: >> >> https://patchwork.kernel.org/patch/9976295/> > I've pushed the latest set here: > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > > You can just say the entire set depends on those going forward; the > documentation is needed, too. > >> Patch 15 represents the top level that glues all of the other components >> together, >> passing arguments between the components. >> >> Patch 16 is a recent effort to extend v6 for advanced camera features like >> Continuous View Finder (CVF) and Snapshot During Video(SDV) support. >> >> Link to user space implementation: >> >> git clone https://chromium.googlesource.com/chromiumos/platform/arc-camera >> >> ImgU media topology print: >> >> # media-ctl -d /dev/media0 -p >> Media controller API version 4.19.0 >> >> Media device information >> >> driver ipu3-imgu >> model ipu3-imgu >> serial >> bus infoPCI::00:05.0 >> hw revision 0x80862015 >> driver version 4.19.0 >> >> Device topology >> - entity 1: ipu3-imgu 0 (5 pads, 5 links) >> type V4L2 subdev subtype Unknown flags 0 >> device node name /dev/v4l-subdev0 >> pad0: Sink >> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > This doesn't seem right. Which formats can be enumerated from the pad? > >> crop:(0,0)/1920x1080 >> compose:(0,0)/1920x1080] > Does the compose rectangle affect the scaling on all outputs? Sakari, driver use crop and compose targets to help set input-feeder and BDS output resolutions which are 2 key block of whole imaging pipeline, not the actual ending output, but they will impact the final output. > >> <- "ipu3-imgu 0 input":0 [] > Are there links that have no useful link configuration? If so, you should > set them enabled and immutable in the driver. The enabled status of input pads is used to get which pipe that user is trying to enable (ipu3_link_setup()), so it could not been set as immutable. > >> pad1: Sink >> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > I'd suggest to use MEDIA_BUS_FMT_FIXED here. > >> <- "ipu3-imgu 0 parameters":0 [] >> pad2: Source >> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] >> -> "ipu3-imgu 0 output":0 [] >> pad3: Source >>
Re: [PATCH v5 0/9] Asynchronous UVC
Hi Kieran,
Thank you for the patches.
On Tuesday, 6 November 2018 23:27:11 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> The Linux UVC driver has long provided adequate performance capabilities for
> web-cams and low data rate video devices in Linux while resolutions were
> low.
>
> Modern USB cameras are now capable of high data rates thanks to USB3 with
> 1080p, and even 4k capture resolutions supported.
>
> Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO
> (isochronous transfers) can generate more data than an embedded ARM core is
> able to process on a single core, resulting in frame loss.
>
> A large part of this performance impact is from the requirement to
> ‘memcpy’ frames out from URB packets to destination frames. This unfortunate
> requirement is due to the UVC protocol allowing a variable length header,
> and thus it is not possible to provide the target frame buffers directly.
>
> Extra throughput is possible by moving the actual memcpy actions to a work
> queue, and moving the memcpy out of interrupt context thus allowing work
> tasks to be scheduled across multiple cores.
>
> This series has been tested on both the ZED and BRIO cameras on arm64
> platforms, and with thanks to Randy Dunlap, a Dynex 1.3MP Webcam, a Sonix
> USB2 Camera, and a built in Toshiba Laptop camera, and with thanks to
> Philipp Zabel for testing on a Lite-On internal Laptop Webcam, Logitech
> C910 (USB2 isoc), Oculus Sensor (USB3 isoc), and Microsoft HoloLens Sensors
> (USB3 bulk).
>
> As far as I am aware iSight devices, and devices which use UVC to encode
> data (output device) have not yet been tested - but should find no ill
> effect (at least not until they are tested of course :D )
:-D
I'm not sure whether anyone is still using those devices with Linux. I
wouldn't be surprised if we realized down the road that they already don't
work.
> Tested-by: Randy Dunlap
> Tested-by: Philipp Zabel
>
> v2:
> - Fix race reported by Guennadi
>
> v3:
> - Fix similar race reported by Laurent
> - Only queue work if required (encode/isight do not queue work)
> - Refactor/Rename variables for clarity
>
> v4:
> - (Yet another) Rework of the uninitialise path.
>This time to hopefully clean up the shutdown races for good.
>use usb_poison_urb() to halt all URBs, then flush the work queue
>before freeing.
> - Rebase to latest linux-media/master
>
> v5:
> - Provide lockdep validation
> - rename uvc_queue_requeue -> uvc_queue_buffer_requeue()
> - Fix comments and periods throughout
> - Rebase to media/v4.20-2
> - Use GFP_KERNEL allocation in uvc_video_copy_data_work()
> - Fix function documentation for uvc_video_copy_data_work()
> - Add periods to the end of sentences
> - Rename 'decode' variable to 'op' in uvc_video_decode_data()
> - Move uvc_urb->async_operations initialisation to before use
> - Move async workqueue to match uvc_streaming lifetime instead of
>streamon/streamoff
> - bracket the for_each_uvc_urb() macro
>
> - New patches added to series:
> media: uvcvideo: Split uvc_video_enable into two
> media: uvcvideo: Rename uvc_{un,}init_video()
> media: uvcvideo: Utilise for_each_uvc_urb iterator
>
> Kieran Bingham (9):
> media: uvcvideo: Refactor URB descriptors
> media: uvcvideo: Convert decode functions to use new context structure
> media: uvcvideo: Protect queue internals with helper
> media: uvcvideo: queue: Simplify spin-lock usage
> media: uvcvideo: queue: Support asynchronous buffer handling
> media: uvcvideo: Move decode processing to process context
> media: uvcvideo: Split uvc_video_enable into two
I've taken the above patches in my tree.
> media: uvcvideo: Rename uvc_{un,}init_video()
> media: uvcvideo: Utilise for_each_uvc_urb iterator
And I've sent review comments for these two.
> drivers/media/usb/uvc/uvc_driver.c | 2 +-
> drivers/media/usb/uvc/uvc_isight.c | 6 +-
> drivers/media/usb/uvc/uvc_queue.c | 110 +---
> drivers/media/usb/uvc/uvc_video.c | 282 +++---
> drivers/media/usb/uvc/uvcvideo.h | 65 ++-
> 5 files changed, 331 insertions(+), 134 deletions(-)
>
> base-commit: dafb7f9aef2fd44991ff1691721ff765a23be27b
--
Regards,
Laurent Pinchart
RE: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Sakari,
> Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
>
> Hi Raj,
>
> On Mon, Nov 05, 2018 at 07:05:53PM +, Mani, Rajmohan wrote:
> > Hi Sakari,
> >
> > > Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> > >
> > > Hi Yong,
> > >
> > > On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > > > This add all the structs of IPU3 firmware ABI.
> > > >
> > > > Signed-off-by: Yong Zhi
> > > > Signed-off-by: Rajmohan Mani
> > >
> > > ...
> > >
> > > > +struct imgu_abi_shd_intra_frame_operations_data {
> > > > + struct imgu_abi_acc_operation
> > > > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> > > __attribute__((aligned(32)));
> > > > + struct imgu_abi_acc_process_lines_cmd_data
> > > > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> > > __attribute__((aligned(32)));
> > > > + struct imgu_abi_shd_transfer_luts_set_data
> > > > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > > > +__attribute__((aligned(32)));
> > >
> > > Could you replace this wth __aligned(32), please? The same for the
> > > rest of the header.
> > >
> >
> > Using __aligned(32) in the uAPI header resulted in compilation errors
> > in user space / camera HAL code.
> >
> > e.g
> > ../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error:
> > expected ';'
> > at end of declaration list
> > __u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE]
> > __aligned(32);
> >
> > So we ended up using __attribute__((aligned(32))) format in uAPI
> > header and to be consistent, we followed the same format in ABI header as
> well.
> >
> > Let us know if it's okay to deviate between uAPI and ABI header for
> > this alignment qualifier.
>
> There's a reason for using __attribute__((aligned(32))) in the uAPI header,
> but
> not in the in-kernel headers where __aligned(32) is preferred.
>
> I have a patch for addressing this for the uAPI headers as well so
> __aligned(32) could be used there, too; I'll submit it soon. Let's see...
> there are kerneldoc issues still in this area.
>
Great. Thanks for the patch to address this need.
> --
> Regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
Hi Mauro,
Thanks for the reviews.
> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
>
> Hi Mauro,
>
> On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
> wrote:
> >
> > Em Mon, 29 Oct 2018 15:22:57 -0700
> > Yong Zhi escreveu:
> [snip]
> > > +struct ipu3_uapi_awb_config_s {
> > > + __u16 rgbs_thr_gr;
> > > + __u16 rgbs_thr_r;
> > > + __u16 rgbs_thr_gb;
> > > + __u16 rgbs_thr_b;
> > > + struct ipu3_uapi_grid_config grid; }
> > > +__attribute__((aligned(32))) __packed;
> >
> > Hmm... Kernel defines a macro for aligned attribute:
> >
> > include/linux/compiler_types.h:#define __aligned(x)
> __attribute__((aligned(x)))
> >
>
> First, thanks for review!
>
> Maybe I missed something, but last time I checked, it wasn't accessible from
> UAPI headers in userspace.
Ack. We see that's still the case.
>
> > I'm not a gcc expert, but it sounds weird to first ask it to align
> > with 32 bits and then have __packed (with means that pads should be
> > removed).
> >
> > In other words, I *guess* is it should either be __packed or
> > __aligned(32).
> >
> > Not that it would do any difference, in practice, as this specific
> > struct has a size with is multiple of 32 bits, but let's do the right
> > annotation here, not mixing two incompatible alignment requirements.
> >
>
> My understanding was that __packed makes the compiler not insert any
> alignment between particular fields of the struct, while __aligned makes the
> whole struct be aligned at given boundary, if placed in another struct. If I
> didn't miss anything, having both should make perfect sense here.
Ack
I also recall that as part of addressing review comments (from Hans and
Sakari),
on earlier versions of this patch series, we added __packed attribute to all
structs
to ensure the size of the structs remains the same between 32 and 64 bit builds.
The addition of structure members of the name padding[x] in some of the structs
ensures that respective members are aligned at 32 byte boundaries, while the
overall size of the structs remain the same between 32 and 64 bit builds.
Thanks
Raj
>
> Best regards,
> Tomasz
Re: [PATCH v5 9/9] media: uvcvideo: Utilise for_each_uvc_urb iterator
Hi Kieran,
Thank you for the patch.
On Tuesday, 6 November 2018 23:27:20 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> A new iterator is available for processing UVC URB structures. This
> simplifies the processing of the internal stream data.
>
> Convert the manual loop iterators to the new helper, adding an index
> helper to keep the existing debug print.
>
> Signed-off-by: Kieran Bingham
>
> ---
> This patch converts to using the iterator which for most hunks makes
> sense. The only one with uncertainty is in uvc_alloc_urb_buffers() where
> the loop index is used to determine if all the buffers were successfully
> allocated.
>
> As the loop index is not incremented by the loops, we can obtain the
> buffer index - but then we are offset and out-by-one.
>
> Adjusting this in the code is fine - but at that point it feels like
> it's not adding much value. I've left that hunk in for this patch - but
> that part could be reverted if desired - unless anyone has a better
> rework of the buffer check?
>
> drivers/media/usb/uvc/uvc_video.c | 51
> drivers/media/usb/uvc/uvcvideo.h | 3 ++-
> 2 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 020022e6ade4..f6e5db7ea50e 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1556,20 +1556,19 @@ static void uvc_video_complete(struct urb *urb)
> */
> static void uvc_free_urb_buffers(struct uvc_streaming *stream)
> {
> - unsigned int i;
> + struct uvc_urb *uvc_urb;
>
> - for (i = 0; i < UVC_URBS; ++i) {
> - struct uvc_urb *uvc_urb = >uvc_urb[i];
> + for_each_uvc_urb(uvc_urb, stream) {
> + if (!uvc_urb->buffer)
> + continue;
>
> - if (uvc_urb->buffer) {
> #ifndef CONFIG_DMA_NONCOHERENT
> - usb_free_coherent(stream->dev->udev, stream->urb_size,
> - uvc_urb->buffer, uvc_urb->dma);
> + usb_free_coherent(stream->dev->udev, stream->urb_size,
> + uvc_urb->buffer, uvc_urb->dma);
> #else
> - kfree(uvc_urb->buffer);
> + kfree(uvc_urb->buffer);
> #endif
> - uvc_urb->buffer = NULL;
> - }
> + uvc_urb->buffer = NULL;
> }
>
> stream->urb_size = 0;
> @@ -1589,8 +1588,9 @@ static void uvc_free_urb_buffers(struct uvc_streaming
> *stream) static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
> unsigned int size, unsigned int psize, gfp_t gfp_flags)
> {
> + struct uvc_urb *uvc_urb;
> unsigned int npackets;
> - unsigned int i;
> + unsigned int i = 0;
>
> /* Buffers are already allocated, bail out. */
> if (stream->urb_size)
> @@ -1605,8 +1605,12 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
> *stream,
>
> /* Retry allocations until one succeed. */
> for (; npackets > 1; npackets /= 2) {
> - for (i = 0; i < UVC_URBS; ++i) {
> - struct uvc_urb *uvc_urb = >uvc_urb[i];
> + for_each_uvc_urb(uvc_urb, stream) {
> + /*
> + * Track how many URBs we allocate, adding one to the
> + * index to account for our zero index.
> + */
> + i = uvc_urb_index(uvc_urb) + 1;
That's a bit ugly indeed, I think we could keep the existing loop;
> stream->urb_size = psize * npackets;
> #ifndef CONFIG_DMA_NONCOHERENT
> @@ -1700,7 +1704,8 @@ static int uvc_init_video_isoc(struct uvc_streaming
> *stream, struct usb_host_endpoint *ep, gfp_t gfp_flags)
> {
> struct urb *urb;
> - unsigned int npackets, i, j;
> + struct uvc_urb *uvc_urb;
> + unsigned int npackets, j;
j without i seems weird, could you rename it ?
> u16 psize;
> u32 size;
>
> @@ -1713,9 +1718,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
> *stream,
>
> size = npackets * psize;
>
> - for (i = 0; i < UVC_URBS; ++i) {
> - struct uvc_urb *uvc_urb = >uvc_urb[i];
> -
> + for_each_uvc_urb(uvc_urb, stream) {
> urb = usb_alloc_urb(npackets, gfp_flags);
> if (urb == NULL) {
> uvc_video_stop(stream, 1);
> @@ -1757,7 +1760,8 @@ static int uvc_init_video_bulk(struct uvc_streaming
> *stream, struct usb_host_endpoint *ep, gfp_t gfp_flags)
> {
> struct urb *urb;
> - unsigned int npackets, pipe, i;
> + struct uvc_urb *uvc_urb;
> + unsigned int npackets, pipe;
> u16 psize;
> u32 size;
>
> @@ -1781,9 +1785,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
> *stream, if (stream->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> size = 0;
>
> - for (i = 0; i < UVC_URBS; ++i) {
> - struct uvc_urb *uvc_urb =
Re: [PATCH v5 8/9] media: uvcvideo: Rename uvc_{un,}init_video()
Hi Kieran,
Thank you for the patch.
On Tuesday, 6 November 2018 23:27:19 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> We have both uvc_init_video() and uvc_video_init() calls which can be
> quite confusing to determine the process for each. Now that video
> uvc_video_enable() has been renamed to uvc_video_start_streaming(),
> adapt these calls to suit the new flow.
>
> Rename uvc_init_video() to uvc_video_start() and uvc_uninit_video() to
> uvc_video_stop().
I agree that these functions are badly named and should be renamed. We are
however entering the nitpicking territory :-) The two functions do more that
starting and stopping, they also allocate and free URBs and the associated
buffers. It could also be argued that they don't actually start and stop
anything, as beyond URB management, they just queue the URBs initially and
kill them. I thus wonder if we could come up with better names.
> Signed-off-by: Kieran Bingham
> ---
> drivers/media/usb/uvc/uvc_video.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 0d35e933856a..020022e6ade4 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1641,7 +1641,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
> *stream, /*
> * Uninitialize isochronous/bulk URBs and free transfer buffers.
> */
> -static void uvc_uninit_video(struct uvc_streaming *stream, int
> free_buffers) +static void uvc_video_stop(struct uvc_streaming *stream, int
> free_buffers) {
> struct uvc_urb *uvc_urb;
>
> @@ -1718,7 +1718,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
> *stream,
>
> urb = usb_alloc_urb(npackets, gfp_flags);
> if (urb == NULL) {
> - uvc_uninit_video(stream, 1);
> + uvc_video_stop(stream, 1);
> return -ENOMEM;
> }
>
> @@ -1786,7 +1786,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
> *stream,
>
> urb = usb_alloc_urb(0, gfp_flags);
> if (urb == NULL) {
> - uvc_uninit_video(stream, 1);
> + uvc_video_stop(stream, 1);
> return -ENOMEM;
> }
>
> @@ -1806,7 +1806,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
> *stream, /*
> * Initialize isochronous/bulk URBs and allocate transfer buffers.
> */
> -static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
> +static int uvc_video_start(struct uvc_streaming *stream, gfp_t gfp_flags)
> {
> struct usb_interface *intf = stream->intf;
> struct usb_host_endpoint *ep;
> @@ -1894,7 +1894,7 @@ static int uvc_init_video(struct uvc_streaming
> *stream, gfp_t gfp_flags) if (ret < 0) {
> uvc_printk(KERN_ERR, "Failed to submit URB %u "
> "(%d).\n", i, ret);
> - uvc_uninit_video(stream, 1);
> + uvc_video_stop(stream, 1);
> return ret;
> }
> }
> @@ -1925,7 +1925,7 @@ int uvc_video_suspend(struct uvc_streaming *stream)
> return 0;
>
> stream->frozen = 1;
> - uvc_uninit_video(stream, 0);
> + uvc_video_stop(stream, 0);
> usb_set_interface(stream->dev->udev, stream->intfnum, 0);
> return 0;
> }
> @@ -1961,7 +1961,7 @@ int uvc_video_resume(struct uvc_streaming *stream, int
> reset) if (ret < 0)
> return ret;
>
> - return uvc_init_video(stream, GFP_NOIO);
> + return uvc_video_start(stream, GFP_NOIO);
> }
>
> /*
> @@ -2095,7 +2095,7 @@ int uvc_video_start_streaming(struct uvc_streaming
> *stream) if (ret < 0)
> goto error_commit;
>
> - ret = uvc_init_video(stream, GFP_KERNEL);
> + ret = uvc_video_start(stream, GFP_KERNEL);
> if (ret < 0)
> goto error_video;
>
> @@ -2111,7 +2111,7 @@ int uvc_video_start_streaming(struct uvc_streaming
> *stream)
>
> int uvc_video_stop_streaming(struct uvc_streaming *stream)
> {
> - uvc_uninit_video(stream, 1);
> + uvc_video_stop(stream, 1);
> if (stream->intf->num_altsetting > 1) {
> usb_set_interface(stream->dev->udev,
> stream->intfnum, 0);
--
Regards,
Laurent Pinchart
Re: [PATCH v5 7/9] media: uvcvideo: Split uvc_video_enable into two
Hi Kieran,
Thank you for the patch.
On Tuesday, 6 November 2018 23:27:18 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> uvc_video_enable() is used both to start and stop the video stream
> object, however the single function entry point shares no code between
> the two operations.
>
> Split the function into two distinct calls, and rename to
> uvc_video_start_streaming() and uvc_video_stop_streaming() as
> appropriate.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/media/usb/uvc/uvc_queue.c | 4 +-
> drivers/media/usb/uvc/uvc_video.c | 56 +++-
> drivers/media/usb/uvc/uvcvideo.h | 3 +-
> 3 files changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index cd8c03341de0..682698ec1118 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -176,7 +176,7 @@ static int uvc_start_streaming(struct vb2_queue *vq,
> unsigned int count)
>
> queue->buf_used = 0;
>
> - ret = uvc_video_enable(stream, 1);
> + ret = uvc_video_start_streaming(stream);
> if (ret == 0)
> return 0;
>
> @@ -194,7 +194,7 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
> lockdep_assert_irqs_enabled();
>
> if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
> - uvc_video_enable(uvc_queue_to_stream(queue), 0);
> + uvc_video_stop_streaming(uvc_queue_to_stream(queue));
>
> spin_lock_irq(>irqlock);
> uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index ce9e40444507..0d35e933856a 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -2082,38 +2082,10 @@ int uvc_video_init(struct uvc_streaming *stream)
> return 0;
> }
>
> -/*
> - * Enable or disable the video stream.
> - */
> -int uvc_video_enable(struct uvc_streaming *stream, int enable)
> +int uvc_video_start_streaming(struct uvc_streaming *stream)
> {
> int ret;
>
> - if (!enable) {
> - uvc_uninit_video(stream, 1);
> - if (stream->intf->num_altsetting > 1) {
> - usb_set_interface(stream->dev->udev,
> - stream->intfnum, 0);
> - } else {
> - /* UVC doesn't specify how to inform a bulk-based device
> - * when the video stream is stopped. Windows sends a
> - * CLEAR_FEATURE(HALT) request to the video streaming
> - * bulk endpoint, mimic the same behaviour.
> - */
> - unsigned int epnum = stream->header.bEndpointAddress
> -& USB_ENDPOINT_NUMBER_MASK;
> - unsigned int dir = stream->header.bEndpointAddress
> - & USB_ENDPOINT_DIR_MASK;
> - unsigned int pipe;
> -
> - pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
> - usb_clear_halt(stream->dev->udev, pipe);
> - }
> -
> - uvc_video_clock_cleanup(stream);
> - return 0;
> - }
> -
> ret = uvc_video_clock_init(stream);
> if (ret < 0)
> return ret;
> @@ -2136,3 +2108,29 @@ int uvc_video_enable(struct uvc_streaming *stream,
> int enable)
>
> return ret;
> }
> +
> +int uvc_video_stop_streaming(struct uvc_streaming *stream)
> +{
> + uvc_uninit_video(stream, 1);
> + if (stream->intf->num_altsetting > 1) {
> + usb_set_interface(stream->dev->udev,
> + stream->intfnum, 0);
This now holds on a single line.
> + } else {
> + /* UVC doesn't specify how to inform a bulk-based device
Let's fix the checkpatch.pl warning here.
> + * when the video stream is stopped. Windows sends a
> + * CLEAR_FEATURE(HALT) request to the video streaming
> + * bulk endpoint, mimic the same behaviour.
> + */
> + unsigned int epnum = stream->header.bEndpointAddress
> +& USB_ENDPOINT_NUMBER_MASK;
> + unsigned int dir = stream->header.bEndpointAddress
> + & USB_ENDPOINT_DIR_MASK;
> + unsigned int pipe;
> +
> + pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
> + usb_clear_halt(stream->dev->udev, pipe);
> + }
> +
> + uvc_video_clock_cleanup(stream);
> + return 0;
As this always return 0 you can make it a void function.
Apart from that,
Reviewed-by: Laurent Pinchart
I'll take the patch in my tree with the above changes.
> +}
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 0953e2e59a79..c0a120496a1f 100644
> ---
Re: [PATCH v5 6/9] media: uvcvideo: Move decode processing to process context
Hi Kieran,
Thank you for the patch.
On Tuesday, 6 November 2018 23:27:17 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> Newer high definition cameras, and cameras with multiple lenses such as
> the range of stereo-vision cameras now available have ever increasing
> data rates.
>
> The inclusion of a variable length packet header in URB packets mean
> that we must memcpy the frame data out to our destination 'manually'.
> This can result in data rates of up to 2 gigabits per second being
> processed.
>
> To improve efficiency, and maximise throughput, handle the URB decode
> processing through a work queue to move it from interrupt context, and
> allow multiple processors to work on URBs in parallel.
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
I wonder if we shouldn't, as a future improvement, only queue async work when
the quantity of data to be copied is above a certain threshold.
> ---
> v2:
> - Lock full critical section of usb_submit_urb()
>
> v3:
> - Fix race on submitting uvc_video_decode_data_work() to work queue.
> - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode)
> - Rename decodes -> copy_operations
> - Don't queue work if there is no async task
> - obtain copy op structure directly in uvc_video_decode_data()
> - uvc_video_decode_data_work() -> uvc_video_copy_data_work()
>
> v4:
> - Provide for_each_uvc_urb()
> - Simplify fix for shutdown race to flush queue before freeing URBs
> - Rebase to v4.16-rc4 (linux-media/master) adjusting for metadata
>conflicts.
>
> v5:
> - Rebase to media/v4.20-2
> - Use GFP_KERNEL allocation in uvc_video_copy_data_work()
> - Fix function documentation for uvc_video_copy_data_work()
> - Add periods to the end of sentences
> - Rename 'decode' variable to 'op' in uvc_video_decode_data()
> - Move uvc_urb->async_operations initialisation to before use
> - Move async workqueue to match uvc_streaming lifetime instead of
>streamon/streamoff
>
> drivers/media/usb/uvc/uvc_driver.c | 2 +-
> drivers/media/usb/uvc/uvc_video.c | 110 +++---
> drivers/media/usb/uvc/uvcvideo.h | 28 -
> 3 files changed, 116 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index bc369a0934a3..e61a6d26e812
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1883,6 +1883,8 @@ static void uvc_unregister_video(struct uvc_device
> *dev) video_unregister_device(>vdev);
> video_unregister_device(>meta.vdev);
>
> + destroy_workqueue(stream->async_wq);
> +
> uvc_debugfs_cleanup_stream(stream);
> }
> }
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 7a7779e1b466..ce9e40444507 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1094,21 +1094,54 @@ static int uvc_video_decode_start(struct
> uvc_streaming *stream, return data[0];
> }
>
> -static void uvc_video_decode_data(struct uvc_streaming *stream,
> +/*
> + * uvc_video_decode_data_work: Asynchronous memcpy processing
> + *
> + * Copy URB data to video buffers in process context, releasing buffer
> + * references and requeuing the URB when done.
> + */
> +static void uvc_video_copy_data_work(struct work_struct *work)
> +{
> + struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < uvc_urb->async_operations; i++) {
> + struct uvc_copy_op *op = _urb->copy_operations[i];
> +
> + memcpy(op->dst, op->src, op->len);
> +
> + /* Release reference taken on this buffer. */
> + uvc_queue_buffer_release(op->buf);
> + }
> +
> + ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
> + if (ret < 0)
> + uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
> +ret);
> +}
> +
> +static void uvc_video_decode_data(struct uvc_urb *uvc_urb,
> struct uvc_buffer *buf, const u8 *data, int len)
> {
> - unsigned int maxlen, nbytes;
> - void *mem;
> + unsigned int active_op = uvc_urb->async_operations;
> + struct uvc_copy_op *op = _urb->copy_operations[active_op];
> + unsigned int maxlen;
>
> if (len <= 0)
> return;
>
> - /* Copy the video data to the buffer. */
> maxlen = buf->length - buf->bytesused;
> - mem = buf->mem + buf->bytesused;
> - nbytes = min((unsigned int)len, maxlen);
> - memcpy(mem, data, nbytes);
> - buf->bytesused += nbytes;
> +
> + /* Take a buffer reference for async work. */
> + kref_get(>ref);
> +
> + op->buf = buf;
> + op->src = data;
> + op->dst = buf->mem + buf->bytesused;
> + op->len = min_t(unsigned int, len, maxlen);
> +
> + buf->bytesused += op->len;
>
> /* Complete the current frame
Re: [PATCH v5 5/9] media: uvcvideo: queue: Support asynchronous buffer handling
Hi Kieran,
Thank you for the patch.
On Tuesday, 6 November 2018 23:27:16 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> The buffer queue interface currently operates sequentially, processing
> buffers after they have fully completed.
>
> In preparation for supporting parallel tasks operating on the buffers,
> we will need to support buffers being processed on multiple CPUs.
>
> Adapt the uvc_queue_next_buffer() such that a reference count tracks the
> active use of the buffer, returning the buffer to the VB2 stack at
> completion.
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
>
> v5:
> - uvc_queue_requeue() -> uvc_queue_buffer_requeue()
> - Fix comment
> ---
> drivers/media/usb/uvc/uvc_queue.c | 61 ++--
> drivers/media/usb/uvc/uvcvideo.h | 4 ++-
> 2 files changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index bebf2415d9de..cd8c03341de0 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -142,6 +142,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
>
> spin_lock_irqsave(>irqlock, flags);
> if (likely(!(queue->flags & UVC_QUEUE_DISCONNECTED))) {
> + kref_init(>ref);
> list_add_tail(>queue, >irqqueue);
> } else {
> /* If the device is disconnected return the buffer to userspace
> @@ -459,28 +460,66 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct
> uvc_video_queue *queue) return nextbuf;
> }
>
> -struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> +/*
> + * uvc_queue_buffer_requeue: Requeue a buffer on our internal irqqueue
> + *
> + * Reuse a buffer through our internal queue without the need to 'prepare'.
> + * The buffer will be returned to userspace through the uvc_buffer_queue
> call if + * the device has been disconnected.
> + */
> +static void uvc_queue_buffer_requeue(struct uvc_video_queue *queue,
> struct uvc_buffer *buf)
> {
> - struct uvc_buffer *nextbuf;
> - unsigned long flags;
> + buf->error = 0;
> + buf->state = UVC_BUF_STATE_QUEUED;
> + buf->bytesused = 0;
> + vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
> +
> + uvc_buffer_queue(>buf.vb2_buf);
> +}
> +
> +static void uvc_queue_buffer_complete(struct kref *ref)
> +{
> + struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref);
> + struct vb2_buffer *vb = >buf.vb2_buf;
> + struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>
> if ((queue->flags & UVC_QUEUE_DROP_CORRUPTED) && buf->error) {
> - buf->error = 0;
> - buf->state = UVC_BUF_STATE_QUEUED;
> - buf->bytesused = 0;
> - vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
> - return buf;
> + uvc_queue_buffer_requeue(queue, buf);
> + return;
> }
>
> + buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
> + vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused);
> + vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
> +}
> +
> +/*
> + * Release a reference on the buffer. Complete the buffer when the last
> + * reference is released.
> + */
> +void uvc_queue_buffer_release(struct uvc_buffer *buf)
> +{
> + kref_put(>ref, uvc_queue_buffer_complete);
> +}
> +
> +/*
> + * Remove this buffer from the queue. Lifetime will persist while async
> actions + * are still running (if any), and uvc_queue_buffer_release will
> give the buffer + * back to VB2 when all users have completed.
> + */
> +struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> + struct uvc_buffer *buf)
> +{
> + struct uvc_buffer *nextbuf;
> + unsigned long flags;
> +
> spin_lock_irqsave(>irqlock, flags);
> list_del(>queue);
> nextbuf = __uvc_queue_get_current_buffer(queue);
> spin_unlock_irqrestore(>irqlock, flags);
>
> - buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
> - vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused);
> - vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
> + uvc_queue_buffer_release(buf);
>
> return nextbuf;
> }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index bdb6d8daedab..1bc17da7f3d4 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -410,6 +410,9 @@ struct uvc_buffer {
> unsigned int bytesused;
>
> u32 pts;
> +
> + /* Asynchronous buffer handling. */
> + struct kref ref;
> };
>
> #define UVC_QUEUE_DISCONNECTED (1 << 0)
> @@ -726,6 +729,7 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int
> disconnect); struct uvc_buffer *uvc_queue_next_buffer(struct
> uvc_video_queue *queue, struct uvc_buffer *buf);
> struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue
>
Hello
Limes Asset Ltd is a Registered UK company currently delivering business financing and financing. We are currently funding start-up, Growth stage companies and Individual funding. Do you need to pay for our services? Website: limesassetltd.co.uk Email: i...@limesassetltd.co.uk John Pratt Limes Asset LTD Company Reg No: 06784897
[PATCH v4l-utils] Add missing linux/bpf_common.h
File needed by linux/bpf.h, add copy from linux-4.19.1 (and add to sync-with-kernel Makefile target). Signed-off-by: Peter Seiderer --- Changes v1 -> v2: - add linux/bpf_common.h to sync-with-kernel target --- Makefile.am| 4 ++- include/linux/bpf_common.h | 57 ++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 include/linux/bpf_common.h diff --git a/Makefile.am b/Makefile.am index 52f8e4c2..b0b8a098 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,7 +26,8 @@ sync-with-kernel: ! -f $(KERNEL_DIR)/usr/include/linux/dvb/frontend.h -o \ ! -f $(KERNEL_DIR)/usr/include/linux/dvb/dmx.h -o \ ! -f $(KERNEL_DIR)/usr/include/linux/lirc.h -o \ - ! -f $(KERNEL_DIR)/usr/include/linux/bpf.h ]; then \ + ! -f $(KERNEL_DIR)/usr/include/linux/bpf.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/bpf_common.h ]; then \ echo "Error you must set KERNEL_DIR to point to an extracted kernel source dir"; \ echo "and run 'make headers_install' in \$$KERNEL_DIR."; \ exit 1; \ @@ -45,6 +46,7 @@ sync-with-kernel: cp -a $(KERNEL_DIR)/usr/include/linux/dvb/dmx.h $(top_srcdir)/include/linux/dvb cp -a $(KERNEL_DIR)/usr/include/linux/lirc.h $(top_srcdir)/include/linux cp -a $(KERNEL_DIR)/usr/include/linux/bpf.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/bpf_common.h $(top_srcdir)/include/linux cp -a $(KERNEL_DIR)/usr/include/linux/cec.h $(top_srcdir)/include/linux cp -a $(KERNEL_DIR)/usr/include/linux/cec-funcs.h $(top_srcdir)/include/linux cp -a $(KERNEL_DIR)/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c $(top_srcdir)/utils/common diff --git a/include/linux/bpf_common.h b/include/linux/bpf_common.h new file mode 100644 index ..f0fe1394 --- /dev/null +++ b/include/linux/bpf_common.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __LINUX_BPF_COMMON_H__ +#define __LINUX_BPF_COMMON_H__ + +/* Instruction classes */ +#define BPF_CLASS(code) ((code) & 0x07) +#defineBPF_LD 0x00 +#defineBPF_LDX 0x01 +#defineBPF_ST 0x02 +#defineBPF_STX 0x03 +#defineBPF_ALU 0x04 +#defineBPF_JMP 0x05 +#defineBPF_RET 0x06 +#defineBPF_MISC0x07 + +/* ld/ldx fields */ +#define BPF_SIZE(code) ((code) & 0x18) +#defineBPF_W 0x00 /* 32-bit */ +#defineBPF_H 0x08 /* 16-bit */ +#defineBPF_B 0x10 /* 8-bit */ +/* eBPFBPF_DW 0x1864-bit */ +#define BPF_MODE(code) ((code) & 0xe0) +#defineBPF_IMM 0x00 +#defineBPF_ABS 0x20 +#defineBPF_IND 0x40 +#defineBPF_MEM 0x60 +#defineBPF_LEN 0x80 +#defineBPF_MSH 0xa0 + +/* alu/jmp fields */ +#define BPF_OP(code)((code) & 0xf0) +#defineBPF_ADD 0x00 +#defineBPF_SUB 0x10 +#defineBPF_MUL 0x20 +#defineBPF_DIV 0x30 +#defineBPF_OR 0x40 +#defineBPF_AND 0x50 +#defineBPF_LSH 0x60 +#defineBPF_RSH 0x70 +#defineBPF_NEG 0x80 +#defineBPF_MOD 0x90 +#defineBPF_XOR 0xa0 + +#defineBPF_JA 0x00 +#defineBPF_JEQ 0x10 +#defineBPF_JGT 0x20 +#defineBPF_JGE 0x30 +#defineBPF_JSET0x40 +#define BPF_SRC(code) ((code) & 0x08) +#defineBPF_K 0x00 +#defineBPF_X 0x08 + +#ifndef BPF_MAXINSNS +#define BPF_MAXINSNS 4096 +#endif + +#endif /* __LINUX_BPF_COMMON_H__ */ -- 2.19.1
Re: [PATCH v4l-utils] Add missing linux/bpf_common.h
Hello Sean, On Tue, 6 Nov 2018 10:38:56 +, Sean Young wrote: > On Mon, Nov 05, 2018 at 09:30:47PM +0100, Peter Seiderer wrote: > > Copy from [1], needed by bpf.h. > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/include/uapi/linux/bpf_common.h?h=v4.19 > > So bpf.h does include this file, but we don't use anything from it in > v4l-utils. > Maybe alternative fix is to remove the include (or not if your want the headers to be in sync with the kernel ones, but then they should be complete enough to be used for compile)? > This include file is for the original BPF, which has been around for a > long time. So why is this include file missing, i.e. what problem are you > trying to solve? A buildroot autobuild failure (see [1] for details) with older toolchains not providing this header... > > Lastely, the file should be included in the sync-with-kernel target so > it does not get out of sync -- should it really be necessary to add the > file. O.k, can do it on next patch iteration... Regards, Peter [1] http://lists.busybox.net/pipermail/buildroot/2018-November/234840.html > > > Sean > > > > > Signed-off-by: Peter Seiderer > > --- > > include/linux/bpf_common.h | 57 ++ > > 1 file changed, 57 insertions(+) > > create mode 100644 include/linux/bpf_common.h > > > > diff --git a/include/linux/bpf_common.h b/include/linux/bpf_common.h > > new file mode 100644 > > index ..ee97668b > > --- /dev/null > > +++ b/include/linux/bpf_common.h > > @@ -0,0 +1,57 @@ > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > +#ifndef _UAPI__LINUX_BPF_COMMON_H__ > > +#define _UAPI__LINUX_BPF_COMMON_H__ > > + > > +/* Instruction classes */ > > +#define BPF_CLASS(code) ((code) & 0x07) > > +#defineBPF_LD 0x00 > > +#defineBPF_LDX 0x01 > > +#defineBPF_ST 0x02 > > +#defineBPF_STX 0x03 > > +#defineBPF_ALU 0x04 > > +#defineBPF_JMP 0x05 > > +#defineBPF_RET 0x06 > > +#defineBPF_MISC0x07 > > + > > +/* ld/ldx fields */ > > +#define BPF_SIZE(code) ((code) & 0x18) > > +#defineBPF_W 0x00 /* 32-bit */ > > +#defineBPF_H 0x08 /* 16-bit */ > > +#defineBPF_B 0x10 /* 8-bit */ > > +/* eBPFBPF_DW 0x1864-bit */ > > +#define BPF_MODE(code) ((code) & 0xe0) > > +#defineBPF_IMM 0x00 > > +#defineBPF_ABS 0x20 > > +#defineBPF_IND 0x40 > > +#defineBPF_MEM 0x60 > > +#defineBPF_LEN 0x80 > > +#defineBPF_MSH 0xa0 > > + > > +/* alu/jmp fields */ > > +#define BPF_OP(code)((code) & 0xf0) > > +#defineBPF_ADD 0x00 > > +#defineBPF_SUB 0x10 > > +#defineBPF_MUL 0x20 > > +#defineBPF_DIV 0x30 > > +#defineBPF_OR 0x40 > > +#defineBPF_AND 0x50 > > +#defineBPF_LSH 0x60 > > +#defineBPF_RSH 0x70 > > +#defineBPF_NEG 0x80 > > +#defineBPF_MOD 0x90 > > +#defineBPF_XOR 0xa0 > > + > > +#defineBPF_JA 0x00 > > +#defineBPF_JEQ 0x10 > > +#defineBPF_JGT 0x20 > > +#defineBPF_JGE 0x30 > > +#defineBPF_JSET0x40 > > +#define BPF_SRC(code) ((code) & 0x08) > > +#defineBPF_K 0x00 > > +#defineBPF_X 0x08 > > + > > +#ifndef BPF_MAXINSNS > > +#define BPF_MAXINSNS 4096 > > +#endif > > + > > +#endif /* _UAPI__LINUX_BPF_COMMON_H__ */ > > -- > > 2.19.1
[GIT PULL FOR v4.21] rc changes
Hi Mauro,
A new driver for the usb IR receiver for the original XBox, and a few
minor fixes.
Thanks,
Sean
The following changes since commit ef86eaf97acd6d82cd3fd40f997b1c8c4895a443:
media: Rename vb2_m2m_request_queue -> v4l2_m2m_request_queue (2018-11-06
05:24:22 -0500)
are available in the Git repository at:
git://linuxtv.org/syoung/media_tree.git for-v4.21a
for you to fetch changes up to ca8dc4056dcff7c0cfcb0daaf0b630bcfa34c932:
media: rc: ensure close() is called on rc_unregister_device (2018-11-06
10:55:53 +)
Benjamin Valentin (1):
media: rc: add driver for Xbox DVD Movie Playback Kit
Brad Love (1):
mceusb: Include three Hauppauge USB dvb device with IR rx
Mauro Carvalho Chehab (1):
media: rc: imon: replace strcpy() by strscpy()
Sean Young (6):
media: rc: XBox DVD Remote uses 12 bits scancodes
media: rc: imon_raw: use fls rather than loop per bit
media: saa7134: rc device does not need 'saa7134 IR (' prefix
media: saa7134: hvr1110 can decode rc6
media: rc: cec devices do not have a lirc chardev
media: rc: ensure close() is called on rc_unregister_device
MAINTAINERS | 6 +
drivers/media/pci/saa7134/saa7134-input.c | 47 +
drivers/media/pci/saa7134/saa7134.h | 1 -
drivers/media/rc/Kconfig | 12 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/imon.c | 4 +-
drivers/media/rc/imon_raw.c | 47 +++--
drivers/media/rc/keymaps/Makefile | 1 +
drivers/media/rc/keymaps/rc-xbox-dvd.c| 63 ++
drivers/media/rc/mceusb.c | 9 +
drivers/media/rc/rc-main.c| 8 +-
drivers/media/rc/xbox_remote.c| 306 ++
include/media/rc-map.h| 1 +
13 files changed, 435 insertions(+), 71 deletions(-)
create mode 100644 drivers/media/rc/keymaps/rc-xbox-dvd.c
create mode 100644 drivers/media/rc/xbox_remote.c
[PATCH v5 7/9] media: uvcvideo: Split uvc_video_enable into two
From: Kieran Bingham
uvc_video_enable() is used both to start and stop the video stream
object, however the single function entry point shares no code between
the two operations.
Split the function into two distinct calls, and rename to
uvc_video_start_streaming() and uvc_video_stop_streaming() as
appropriate.
Signed-off-by: Kieran Bingham
---
drivers/media/usb/uvc/uvc_queue.c | 4 +-
drivers/media/usb/uvc/uvc_video.c | 56 +++-
drivers/media/usb/uvc/uvcvideo.h | 3 +-
3 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index cd8c03341de0..682698ec1118 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -176,7 +176,7 @@ static int uvc_start_streaming(struct vb2_queue *vq,
unsigned int count)
queue->buf_used = 0;
- ret = uvc_video_enable(stream, 1);
+ ret = uvc_video_start_streaming(stream);
if (ret == 0)
return 0;
@@ -194,7 +194,7 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
lockdep_assert_irqs_enabled();
if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
- uvc_video_enable(uvc_queue_to_stream(queue), 0);
+ uvc_video_stop_streaming(uvc_queue_to_stream(queue));
spin_lock_irq(>irqlock);
uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index ce9e40444507..0d35e933856a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -2082,38 +2082,10 @@ int uvc_video_init(struct uvc_streaming *stream)
return 0;
}
-/*
- * Enable or disable the video stream.
- */
-int uvc_video_enable(struct uvc_streaming *stream, int enable)
+int uvc_video_start_streaming(struct uvc_streaming *stream)
{
int ret;
- if (!enable) {
- uvc_uninit_video(stream, 1);
- if (stream->intf->num_altsetting > 1) {
- usb_set_interface(stream->dev->udev,
- stream->intfnum, 0);
- } else {
- /* UVC doesn't specify how to inform a bulk-based device
-* when the video stream is stopped. Windows sends a
-* CLEAR_FEATURE(HALT) request to the video streaming
-* bulk endpoint, mimic the same behaviour.
-*/
- unsigned int epnum = stream->header.bEndpointAddress
- & USB_ENDPOINT_NUMBER_MASK;
- unsigned int dir = stream->header.bEndpointAddress
-& USB_ENDPOINT_DIR_MASK;
- unsigned int pipe;
-
- pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
- usb_clear_halt(stream->dev->udev, pipe);
- }
-
- uvc_video_clock_cleanup(stream);
- return 0;
- }
-
ret = uvc_video_clock_init(stream);
if (ret < 0)
return ret;
@@ -2136,3 +2108,29 @@ int uvc_video_enable(struct uvc_streaming *stream, int
enable)
return ret;
}
+
+int uvc_video_stop_streaming(struct uvc_streaming *stream)
+{
+ uvc_uninit_video(stream, 1);
+ if (stream->intf->num_altsetting > 1) {
+ usb_set_interface(stream->dev->udev,
+ stream->intfnum, 0);
+ } else {
+ /* UVC doesn't specify how to inform a bulk-based device
+* when the video stream is stopped. Windows sends a
+* CLEAR_FEATURE(HALT) request to the video streaming
+* bulk endpoint, mimic the same behaviour.
+*/
+ unsigned int epnum = stream->header.bEndpointAddress
+ & USB_ENDPOINT_NUMBER_MASK;
+ unsigned int dir = stream->header.bEndpointAddress
+& USB_ENDPOINT_DIR_MASK;
+ unsigned int pipe;
+
+ pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
+ usb_clear_halt(stream->dev->udev, pipe);
+ }
+
+ uvc_video_clock_cleanup(stream);
+ return 0;
+}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 0953e2e59a79..c0a120496a1f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -784,7 +784,8 @@ void uvc_mc_cleanup_entity(struct uvc_entity *entity);
int uvc_video_init(struct uvc_streaming *stream);
int uvc_video_suspend(struct uvc_streaming *stream);
int uvc_video_resume(struct uvc_streaming *stream, int reset);
-int uvc_video_enable(struct uvc_streaming *stream, int enable);
+int uvc_video_start_streaming(struct uvc_streaming *stream);
+int uvc_video_stop_streaming(struct
[PATCH v5 9/9] media: uvcvideo: Utilise for_each_uvc_urb iterator
From: Kieran Bingham
A new iterator is available for processing UVC URB structures. This
simplifies the processing of the internal stream data.
Convert the manual loop iterators to the new helper, adding an index
helper to keep the existing debug print.
Signed-off-by: Kieran Bingham
---
This patch converts to using the iterator which for most hunks makes
sense. The only one with uncertainty is in uvc_alloc_urb_buffers() where
the loop index is used to determine if all the buffers were successfully
allocated.
As the loop index is not incremented by the loops, we can obtain the
buffer index - but then we are offset and out-by-one.
Adjusting this in the code is fine - but at that point it feels like
it's not adding much value. I've left that hunk in for this patch - but
that part could be reverted if desired - unless anyone has a better
rework of the buffer check?
drivers/media/usb/uvc/uvc_video.c | 51
drivers/media/usb/uvc/uvcvideo.h | 3 ++-
2 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 020022e6ade4..f6e5db7ea50e 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1556,20 +1556,19 @@ static void uvc_video_complete(struct urb *urb)
*/
static void uvc_free_urb_buffers(struct uvc_streaming *stream)
{
- unsigned int i;
+ struct uvc_urb *uvc_urb;
- for (i = 0; i < UVC_URBS; ++i) {
- struct uvc_urb *uvc_urb = >uvc_urb[i];
+ for_each_uvc_urb(uvc_urb, stream) {
+ if (!uvc_urb->buffer)
+ continue;
- if (uvc_urb->buffer) {
#ifndef CONFIG_DMA_NONCOHERENT
- usb_free_coherent(stream->dev->udev, stream->urb_size,
- uvc_urb->buffer, uvc_urb->dma);
+ usb_free_coherent(stream->dev->udev, stream->urb_size,
+ uvc_urb->buffer, uvc_urb->dma);
#else
- kfree(uvc_urb->buffer);
+ kfree(uvc_urb->buffer);
#endif
- uvc_urb->buffer = NULL;
- }
+ uvc_urb->buffer = NULL;
}
stream->urb_size = 0;
@@ -1589,8 +1588,9 @@ static void uvc_free_urb_buffers(struct uvc_streaming
*stream)
static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
unsigned int size, unsigned int psize, gfp_t gfp_flags)
{
+ struct uvc_urb *uvc_urb;
unsigned int npackets;
- unsigned int i;
+ unsigned int i = 0;
/* Buffers are already allocated, bail out. */
if (stream->urb_size)
@@ -1605,8 +1605,12 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
*stream,
/* Retry allocations until one succeed. */
for (; npackets > 1; npackets /= 2) {
- for (i = 0; i < UVC_URBS; ++i) {
- struct uvc_urb *uvc_urb = >uvc_urb[i];
+ for_each_uvc_urb(uvc_urb, stream) {
+ /*
+* Track how many URBs we allocate, adding one to the
+* index to account for our zero index.
+*/
+ i = uvc_urb_index(uvc_urb) + 1;
stream->urb_size = psize * npackets;
#ifndef CONFIG_DMA_NONCOHERENT
@@ -1700,7 +1704,8 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
struct usb_host_endpoint *ep, gfp_t gfp_flags)
{
struct urb *urb;
- unsigned int npackets, i, j;
+ struct uvc_urb *uvc_urb;
+ unsigned int npackets, j;
u16 psize;
u32 size;
@@ -1713,9 +1718,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
size = npackets * psize;
- for (i = 0; i < UVC_URBS; ++i) {
- struct uvc_urb *uvc_urb = >uvc_urb[i];
-
+ for_each_uvc_urb(uvc_urb, stream) {
urb = usb_alloc_urb(npackets, gfp_flags);
if (urb == NULL) {
uvc_video_stop(stream, 1);
@@ -1757,7 +1760,8 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
struct usb_host_endpoint *ep, gfp_t gfp_flags)
{
struct urb *urb;
- unsigned int npackets, pipe, i;
+ struct uvc_urb *uvc_urb;
+ unsigned int npackets, pipe;
u16 psize;
u32 size;
@@ -1781,9 +1785,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
if (stream->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
size = 0;
- for (i = 0; i < UVC_URBS; ++i) {
- struct uvc_urb *uvc_urb = >uvc_urb[i];
-
+ for_each_uvc_urb(uvc_urb, stream) {
urb = usb_alloc_urb(0, gfp_flags);
if (urb == NULL) {
uvc_video_stop(stream, 1);
@@ -1810,6 +1812,7 @@ static int uvc_video_start(struct uvc_streaming *stream,
gfp_t gfp_flags)
{
struct
[PATCH v5 8/9] media: uvcvideo: Rename uvc_{un,}init_video()
From: Kieran Bingham
We have both uvc_init_video() and uvc_video_init() calls which can be
quite confusing to determine the process for each. Now that video
uvc_video_enable() has been renamed to uvc_video_start_streaming(),
adapt these calls to suit the new flow.
Rename uvc_init_video() to uvc_video_start() and uvc_uninit_video() to
uvc_video_stop().
Signed-off-by: Kieran Bingham
---
drivers/media/usb/uvc/uvc_video.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 0d35e933856a..020022e6ade4 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1641,7 +1641,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
*stream,
/*
* Uninitialize isochronous/bulk URBs and free transfer buffers.
*/
-static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers)
+static void uvc_video_stop(struct uvc_streaming *stream, int free_buffers)
{
struct uvc_urb *uvc_urb;
@@ -1718,7 +1718,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
urb = usb_alloc_urb(npackets, gfp_flags);
if (urb == NULL) {
- uvc_uninit_video(stream, 1);
+ uvc_video_stop(stream, 1);
return -ENOMEM;
}
@@ -1786,7 +1786,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
urb = usb_alloc_urb(0, gfp_flags);
if (urb == NULL) {
- uvc_uninit_video(stream, 1);
+ uvc_video_stop(stream, 1);
return -ENOMEM;
}
@@ -1806,7 +1806,7 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
/*
* Initialize isochronous/bulk URBs and allocate transfer buffers.
*/
-static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
+static int uvc_video_start(struct uvc_streaming *stream, gfp_t gfp_flags)
{
struct usb_interface *intf = stream->intf;
struct usb_host_endpoint *ep;
@@ -1894,7 +1894,7 @@ static int uvc_init_video(struct uvc_streaming *stream,
gfp_t gfp_flags)
if (ret < 0) {
uvc_printk(KERN_ERR, "Failed to submit URB %u "
"(%d).\n", i, ret);
- uvc_uninit_video(stream, 1);
+ uvc_video_stop(stream, 1);
return ret;
}
}
@@ -1925,7 +1925,7 @@ int uvc_video_suspend(struct uvc_streaming *stream)
return 0;
stream->frozen = 1;
- uvc_uninit_video(stream, 0);
+ uvc_video_stop(stream, 0);
usb_set_interface(stream->dev->udev, stream->intfnum, 0);
return 0;
}
@@ -1961,7 +1961,7 @@ int uvc_video_resume(struct uvc_streaming *stream, int
reset)
if (ret < 0)
return ret;
- return uvc_init_video(stream, GFP_NOIO);
+ return uvc_video_start(stream, GFP_NOIO);
}
/*
@@ -2095,7 +2095,7 @@ int uvc_video_start_streaming(struct uvc_streaming
*stream)
if (ret < 0)
goto error_commit;
- ret = uvc_init_video(stream, GFP_KERNEL);
+ ret = uvc_video_start(stream, GFP_KERNEL);
if (ret < 0)
goto error_video;
@@ -2111,7 +2111,7 @@ int uvc_video_start_streaming(struct uvc_streaming
*stream)
int uvc_video_stop_streaming(struct uvc_streaming *stream)
{
- uvc_uninit_video(stream, 1);
+ uvc_video_stop(stream, 1);
if (stream->intf->num_altsetting > 1) {
usb_set_interface(stream->dev->udev,
stream->intfnum, 0);
--
git-series 0.9.1
[PATCH v5 0/9] Asynchronous UVC
From: Kieran Bingham
The Linux UVC driver has long provided adequate performance capabilities for
web-cams and low data rate video devices in Linux while resolutions were low.
Modern USB cameras are now capable of high data rates thanks to USB3 with
1080p, and even 4k capture resolutions supported.
Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO
(isochronous transfers) can generate more data than an embedded ARM core is
able to process on a single core, resulting in frame loss.
A large part of this performance impact is from the requirement to
‘memcpy’ frames out from URB packets to destination frames. This unfortunate
requirement is due to the UVC protocol allowing a variable length header, and
thus it is not possible to provide the target frame buffers directly.
Extra throughput is possible by moving the actual memcpy actions to a work
queue, and moving the memcpy out of interrupt context thus allowing work tasks
to be scheduled across multiple cores.
This series has been tested on both the ZED and BRIO cameras on arm64
platforms, and with thanks to Randy Dunlap, a Dynex 1.3MP Webcam, a Sonix USB2
Camera, and a built in Toshiba Laptop camera, and with thanks to Philipp Zabel
for testing on a Lite-On internal Laptop Webcam, Logitech C910 (USB2 isoc),
Oculus Sensor (USB3 isoc), and Microsoft HoloLens Sensors (USB3 bulk).
As far as I am aware iSight devices, and devices which use UVC to encode data
(output device) have not yet been tested - but should find no ill effect (at
least not until they are tested of course :D )
Tested-by: Randy Dunlap
Tested-by: Philipp Zabel
v2:
- Fix race reported by Guennadi
v3:
- Fix similar race reported by Laurent
- Only queue work if required (encode/isight do not queue work)
- Refactor/Rename variables for clarity
v4:
- (Yet another) Rework of the uninitialise path.
This time to hopefully clean up the shutdown races for good.
use usb_poison_urb() to halt all URBs, then flush the work queue
before freeing.
- Rebase to latest linux-media/master
v5:
- Provide lockdep validation
- rename uvc_queue_requeue -> uvc_queue_buffer_requeue()
- Fix comments and periods throughout
- Rebase to media/v4.20-2
- Use GFP_KERNEL allocation in uvc_video_copy_data_work()
- Fix function documentation for uvc_video_copy_data_work()
- Add periods to the end of sentences
- Rename 'decode' variable to 'op' in uvc_video_decode_data()
- Move uvc_urb->async_operations initialisation to before use
- Move async workqueue to match uvc_streaming lifetime instead of
streamon/streamoff
- bracket the for_each_uvc_urb() macro
- New patches added to series:
media: uvcvideo: Split uvc_video_enable into two
media: uvcvideo: Rename uvc_{un,}init_video()
media: uvcvideo: Utilise for_each_uvc_urb iterator
Kieran Bingham (9):
media: uvcvideo: Refactor URB descriptors
media: uvcvideo: Convert decode functions to use new context structure
media: uvcvideo: Protect queue internals with helper
media: uvcvideo: queue: Simplify spin-lock usage
media: uvcvideo: queue: Support asynchronous buffer handling
media: uvcvideo: Move decode processing to process context
media: uvcvideo: Split uvc_video_enable into two
media: uvcvideo: Rename uvc_{un,}init_video()
media: uvcvideo: Utilise for_each_uvc_urb iterator
drivers/media/usb/uvc/uvc_driver.c | 2 +-
drivers/media/usb/uvc/uvc_isight.c | 6 +-
drivers/media/usb/uvc/uvc_queue.c | 110 +---
drivers/media/usb/uvc/uvc_video.c | 282 +++---
drivers/media/usb/uvc/uvcvideo.h | 65 ++-
5 files changed, 331 insertions(+), 134 deletions(-)
base-commit: dafb7f9aef2fd44991ff1691721ff765a23be27b
--
git-series 0.9.1
[PATCH v5 2/9] media: uvcvideo: Convert decode functions to use new context structure
From: Kieran Bingham
The URB completion handlers currently reference the stream context.
Now that each URB has its own context structure, convert the decode (and
one encode) functions to utilise this context for URB management.
Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
---
v2:
- fix checkpatch warning (pre-existing in code)
v3: (none)
v4:
- Rebase on top of linux-media/master (v4.16-rc4, metadata additions)
drivers/media/usb/uvc/uvc_isight.c | 6 --
drivers/media/usb/uvc/uvc_video.c | 26 ++
drivers/media/usb/uvc/uvcvideo.h | 8 +---
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_isight.c
b/drivers/media/usb/uvc/uvc_isight.c
index 81e6f2187bfb..39a4e4482b23 100644
--- a/drivers/media/usb/uvc/uvc_isight.c
+++ b/drivers/media/usb/uvc/uvc_isight.c
@@ -99,9 +99,11 @@ static int isight_decode(struct uvc_video_queue *queue,
struct uvc_buffer *buf,
return 0;
}
-void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
- struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
+void uvc_video_decode_isight(struct uvc_urb *uvc_urb, struct uvc_buffer *buf,
+ struct uvc_buffer *meta_buf)
{
+ struct urb *urb = uvc_urb->urb;
+ struct uvc_streaming *stream = uvc_urb->stream;
int ret, i;
for (i = 0; i < urb->number_of_packets; ++i) {
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 113881bed2a4..6d4384695964 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1291,9 +1291,11 @@ static void uvc_video_next_buffers(struct uvc_streaming
*stream,
*video_buf = uvc_queue_next_buffer(>queue, *video_buf);
}
-static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
*stream,
+static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
{
+ struct urb *urb = uvc_urb->urb;
+ struct uvc_streaming *stream = uvc_urb->stream;
u8 *mem;
int ret, i;
@@ -1334,9 +1336,11 @@ static void uvc_video_decode_isoc(struct urb *urb,
struct uvc_streaming *stream,
}
}
-static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming
*stream,
+static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
{
+ struct urb *urb = uvc_urb->urb;
+ struct uvc_streaming *stream = uvc_urb->stream;
u8 *mem;
int len, ret;
@@ -1402,9 +1406,12 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream,
}
}
-static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming
*stream,
+static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb,
struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
{
+ struct urb *urb = uvc_urb->urb;
+ struct uvc_streaming *stream = uvc_urb->stream;
+
u8 *mem = urb->transfer_buffer;
int len = stream->urb_size, ret;
@@ -1447,7 +1454,8 @@ static void uvc_video_encode_bulk(struct urb *urb, struct
uvc_streaming *stream,
static void uvc_video_complete(struct urb *urb)
{
- struct uvc_streaming *stream = urb->context;
+ struct uvc_urb *uvc_urb = urb->context;
+ struct uvc_streaming *stream = uvc_urb->stream;
struct uvc_video_queue *queue = >queue;
struct uvc_video_queue *qmeta = >meta.queue;
struct vb2_queue *vb2_qmeta = stream->meta.vdev.queue;
@@ -1490,7 +1498,7 @@ static void uvc_video_complete(struct urb *urb)
spin_unlock_irqrestore(>irqlock, flags);
}
- stream->decode(urb, stream, buf, buf_meta);
+ stream->decode(uvc_urb, buf, buf_meta);
if ((ret = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
@@ -1568,6 +1576,8 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
*stream,
uvc_free_urb_buffers(stream);
break;
}
+
+ uvc_urb->stream = stream;
}
if (i == UVC_URBS) {
@@ -1666,7 +1676,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
}
urb->dev = stream->dev->udev;
- urb->context = stream;
+ urb->context = uvc_urb;
urb->pipe = usb_rcvisocpipe(stream->dev->udev,
ep->desc.bEndpointAddress);
#ifndef CONFIG_DMA_NONCOHERENT
@@ -1733,8 +1743,8 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
return -ENOMEM;
}
- usb_fill_bulk_urb(urb, stream->dev->udev, pipe, uvc_urb->buffer,
- size,
[PATCH v5 1/9] media: uvcvideo: Refactor URB descriptors
From: Kieran Bingham
We currently store three separate arrays for each URB reference we hold.
Objectify the data needed to track URBs into a single uvc_urb structure,
allowing better object management and tracking of the URB.
All accesses to the data pointers through stream, are converted to use a
uvc_urb pointer for consistency.
Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
---
v2:
- Re-describe URB context structure
- Re-name uvc_urb->{urb_buffer,urb_dma}{buffer,dma}
v3:
- No change
v4:
- Rebase on top of linux-media/master (v4.16-rc4, metadata additions)
drivers/media/usb/uvc/uvc_video.c | 49 +++-
drivers/media/usb/uvc/uvcvideo.h | 18 ++--
2 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 86a99f461fd8..113881bed2a4 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1506,14 +1506,16 @@ static void uvc_free_urb_buffers(struct uvc_streaming
*stream)
unsigned int i;
for (i = 0; i < UVC_URBS; ++i) {
- if (stream->urb_buffer[i]) {
+ struct uvc_urb *uvc_urb = >uvc_urb[i];
+
+ if (uvc_urb->buffer) {
#ifndef CONFIG_DMA_NONCOHERENT
usb_free_coherent(stream->dev->udev, stream->urb_size,
- stream->urb_buffer[i], stream->urb_dma[i]);
+ uvc_urb->buffer, uvc_urb->dma);
#else
- kfree(stream->urb_buffer[i]);
+ kfree(uvc_urb->buffer);
#endif
- stream->urb_buffer[i] = NULL;
+ uvc_urb->buffer = NULL;
}
}
@@ -1551,16 +1553,18 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
*stream,
/* Retry allocations until one succeed. */
for (; npackets > 1; npackets /= 2) {
for (i = 0; i < UVC_URBS; ++i) {
+ struct uvc_urb *uvc_urb = >uvc_urb[i];
+
stream->urb_size = psize * npackets;
#ifndef CONFIG_DMA_NONCOHERENT
- stream->urb_buffer[i] = usb_alloc_coherent(
+ uvc_urb->buffer = usb_alloc_coherent(
stream->dev->udev, stream->urb_size,
- gfp_flags | __GFP_NOWARN, >urb_dma[i]);
+ gfp_flags | __GFP_NOWARN, _urb->dma);
#else
- stream->urb_buffer[i] =
+ uvc_urb->buffer =
kmalloc(stream->urb_size, gfp_flags | __GFP_NOWARN);
#endif
- if (!stream->urb_buffer[i]) {
+ if (!uvc_urb->buffer) {
uvc_free_urb_buffers(stream);
break;
}
@@ -1590,13 +1594,15 @@ static void uvc_uninit_video(struct uvc_streaming
*stream, int free_buffers)
uvc_video_stats_stop(stream);
for (i = 0; i < UVC_URBS; ++i) {
- urb = stream->urb[i];
+ struct uvc_urb *uvc_urb = >uvc_urb[i];
+
+ urb = uvc_urb->urb;
if (urb == NULL)
continue;
usb_kill_urb(urb);
usb_free_urb(urb);
- stream->urb[i] = NULL;
+ uvc_urb->urb = NULL;
}
if (free_buffers)
@@ -1651,6 +1657,8 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
size = npackets * psize;
for (i = 0; i < UVC_URBS; ++i) {
+ struct uvc_urb *uvc_urb = >uvc_urb[i];
+
urb = usb_alloc_urb(npackets, gfp_flags);
if (urb == NULL) {
uvc_uninit_video(stream, 1);
@@ -1663,12 +1671,12 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
ep->desc.bEndpointAddress);
#ifndef CONFIG_DMA_NONCOHERENT
urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
- urb->transfer_dma = stream->urb_dma[i];
+ urb->transfer_dma = uvc_urb->dma;
#else
urb->transfer_flags = URB_ISO_ASAP;
#endif
urb->interval = ep->desc.bInterval;
- urb->transfer_buffer = stream->urb_buffer[i];
+ urb->transfer_buffer = uvc_urb->buffer;
urb->complete = uvc_video_complete;
urb->number_of_packets = npackets;
urb->transfer_buffer_length = size;
@@ -1678,7 +1686,7 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
urb->iso_frame_desc[j].length = psize;
}
- stream->urb[i] = urb;
+ uvc_urb->urb = urb;
}
return 0;
@@ -1717,21 +1725,22 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
size = 0;
[PATCH v5 4/9] media: uvcvideo: queue: Simplify spin-lock usage
From: Kieran Bingham
Both uvc_start_streaming(), and uvc_stop_streaming() are called from
userspace context, with interrupts enabled. As such, they do not need to
save the IRQ state, and can use spin_lock_irq() and spin_unlock_irq()
respectively.
Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
---
v4:
- Rebase to v4.16 (linux-media/master)
v5:
- Provide lockdep validation
drivers/media/usb/uvc/uvc_queue.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 74f9483911d0..bebf2415d9de 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -169,18 +169,19 @@ static int uvc_start_streaming(struct vb2_queue *vq,
unsigned int count)
{
struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
struct uvc_streaming *stream = uvc_queue_to_stream(queue);
- unsigned long flags;
int ret;
+ lockdep_assert_irqs_enabled();
+
queue->buf_used = 0;
ret = uvc_video_enable(stream, 1);
if (ret == 0)
return 0;
- spin_lock_irqsave(>irqlock, flags);
+ spin_lock_irq(>irqlock);
uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED);
- spin_unlock_irqrestore(>irqlock, flags);
+ spin_unlock_irq(>irqlock);
return ret;
}
@@ -188,14 +189,15 @@ static int uvc_start_streaming(struct vb2_queue *vq,
unsigned int count)
static void uvc_stop_streaming(struct vb2_queue *vq)
{
struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
- unsigned long flags;
+
+ lockdep_assert_irqs_enabled();
if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
uvc_video_enable(uvc_queue_to_stream(queue), 0);
- spin_lock_irqsave(>irqlock, flags);
+ spin_lock_irq(>irqlock);
uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
- spin_unlock_irqrestore(>irqlock, flags);
+ spin_unlock_irq(>irqlock);
}
static const struct vb2_ops uvc_queue_qops = {
--
git-series 0.9.1
[PATCH v5 3/9] media: uvcvideo: Protect queue internals with helper
From: Kieran Bingham
The URB completion operation obtains the current buffer by reading
directly into the queue internal interface.
Protect this queue abstraction by providing a helper
uvc_queue_get_current_buffer() which can be used by both the decode
task, and the uvc_queue_next_buffer() functions.
Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
---
v2:
- Fix coding style of conditional statements
v3:
- No change
v4:
- Rebase on top of linux-media/master (v4.16-rc4, metadata additions)
drivers/media/usb/uvc/uvc_queue.c | 33 +++-
drivers/media/usb/uvc/uvc_video.c | 6 +-
drivers/media/usb/uvc/uvcvideo.h | 1 +-
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 8964e16f2b22..74f9483911d0 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -430,6 +430,33 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int
disconnect)
spin_unlock_irqrestore(>irqlock, flags);
}
+/*
+ * uvc_queue_get_current_buffer: Obtain the current working output buffer
+ *
+ * Buffers may span multiple packets, and even URBs, therefore the active
buffer
+ * remains on the queue until the EOF marker.
+ */
+static struct uvc_buffer *
+__uvc_queue_get_current_buffer(struct uvc_video_queue *queue)
+{
+ if (list_empty(>irqqueue))
+ return NULL;
+
+ return list_first_entry(>irqqueue, struct uvc_buffer, queue);
+}
+
+struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue)
+{
+ struct uvc_buffer *nextbuf;
+ unsigned long flags;
+
+ spin_lock_irqsave(>irqlock, flags);
+ nextbuf = __uvc_queue_get_current_buffer(queue);
+ spin_unlock_irqrestore(>irqlock, flags);
+
+ return nextbuf;
+}
+
struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
struct uvc_buffer *buf)
{
@@ -446,11 +473,7 @@ struct uvc_buffer *uvc_queue_next_buffer(struct
uvc_video_queue *queue,
spin_lock_irqsave(>irqlock, flags);
list_del(>queue);
- if (!list_empty(>irqqueue))
- nextbuf = list_first_entry(>irqqueue, struct uvc_buffer,
- queue);
- else
- nextbuf = NULL;
+ nextbuf = __uvc_queue_get_current_buffer(queue);
spin_unlock_irqrestore(>irqlock, flags);
buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 6d4384695964..7a7779e1b466 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1484,11 +1484,7 @@ static void uvc_video_complete(struct urb *urb)
return;
}
- spin_lock_irqsave(>irqlock, flags);
- if (!list_empty(>irqqueue))
- buf = list_first_entry(>irqqueue, struct uvc_buffer,
- queue);
- spin_unlock_irqrestore(>irqlock, flags);
+ buf = uvc_queue_get_current_buffer(queue);
if (vb2_qmeta) {
spin_lock_irqsave(>irqlock, flags);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index f7f8db6fc91a..bdb6d8daedab 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -725,6 +725,7 @@ int uvc_queue_streamoff(struct uvc_video_queue *queue, enum
v4l2_buf_type type);
void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect);
struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
struct uvc_buffer *buf);
+struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue);
int uvc_queue_mmap(struct uvc_video_queue *queue,
struct vm_area_struct *vma);
__poll_t uvc_queue_poll(struct uvc_video_queue *queue, struct file *file,
--
git-series 0.9.1
[PATCH v5 6/9] media: uvcvideo: Move decode processing to process context
From: Kieran Bingham
Newer high definition cameras, and cameras with multiple lenses such as
the range of stereo-vision cameras now available have ever increasing
data rates.
The inclusion of a variable length packet header in URB packets mean
that we must memcpy the frame data out to our destination 'manually'.
This can result in data rates of up to 2 gigabits per second being
processed.
To improve efficiency, and maximise throughput, handle the URB decode
processing through a work queue to move it from interrupt context, and
allow multiple processors to work on URBs in parallel.
Signed-off-by: Kieran Bingham
---
v2:
- Lock full critical section of usb_submit_urb()
v3:
- Fix race on submitting uvc_video_decode_data_work() to work queue.
- Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode)
- Rename decodes -> copy_operations
- Don't queue work if there is no async task
- obtain copy op structure directly in uvc_video_decode_data()
- uvc_video_decode_data_work() -> uvc_video_copy_data_work()
v4:
- Provide for_each_uvc_urb()
- Simplify fix for shutdown race to flush queue before freeing URBs
- Rebase to v4.16-rc4 (linux-media/master) adjusting for metadata
conflicts.
v5:
- Rebase to media/v4.20-2
- Use GFP_KERNEL allocation in uvc_video_copy_data_work()
- Fix function documentation for uvc_video_copy_data_work()
- Add periods to the end of sentences
- Rename 'decode' variable to 'op' in uvc_video_decode_data()
- Move uvc_urb->async_operations initialisation to before use
- Move async workqueue to match uvc_streaming lifetime instead of
streamon/streamoff
drivers/media/usb/uvc/uvc_driver.c | 2 +-
drivers/media/usb/uvc/uvc_video.c | 110 +++---
drivers/media/usb/uvc/uvcvideo.h | 28 -
3 files changed, 116 insertions(+), 24 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index bc369a0934a3..e61a6d26e812 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1883,6 +1883,8 @@ static void uvc_unregister_video(struct uvc_device *dev)
video_unregister_device(>vdev);
video_unregister_device(>meta.vdev);
+ destroy_workqueue(stream->async_wq);
+
uvc_debugfs_cleanup_stream(stream);
}
}
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 7a7779e1b466..ce9e40444507 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1094,21 +1094,54 @@ static int uvc_video_decode_start(struct uvc_streaming
*stream,
return data[0];
}
-static void uvc_video_decode_data(struct uvc_streaming *stream,
+/*
+ * uvc_video_decode_data_work: Asynchronous memcpy processing
+ *
+ * Copy URB data to video buffers in process context, releasing buffer
+ * references and requeuing the URB when done.
+ */
+static void uvc_video_copy_data_work(struct work_struct *work)
+{
+ struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < uvc_urb->async_operations; i++) {
+ struct uvc_copy_op *op = _urb->copy_operations[i];
+
+ memcpy(op->dst, op->src, op->len);
+
+ /* Release reference taken on this buffer. */
+ uvc_queue_buffer_release(op->buf);
+ }
+
+ ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
+ if (ret < 0)
+ uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
+ ret);
+}
+
+static void uvc_video_decode_data(struct uvc_urb *uvc_urb,
struct uvc_buffer *buf, const u8 *data, int len)
{
- unsigned int maxlen, nbytes;
- void *mem;
+ unsigned int active_op = uvc_urb->async_operations;
+ struct uvc_copy_op *op = _urb->copy_operations[active_op];
+ unsigned int maxlen;
if (len <= 0)
return;
- /* Copy the video data to the buffer. */
maxlen = buf->length - buf->bytesused;
- mem = buf->mem + buf->bytesused;
- nbytes = min((unsigned int)len, maxlen);
- memcpy(mem, data, nbytes);
- buf->bytesused += nbytes;
+
+ /* Take a buffer reference for async work. */
+ kref_get(>ref);
+
+ op->buf = buf;
+ op->src = data;
+ op->dst = buf->mem + buf->bytesused;
+ op->len = min_t(unsigned int, len, maxlen);
+
+ buf->bytesused += op->len;
/* Complete the current frame if the buffer size was exceeded. */
if (len > maxlen) {
@@ -1116,6 +1149,8 @@ static void uvc_video_decode_data(struct uvc_streaming
*stream,
buf->error = 1;
buf->state = UVC_BUF_STATE_READY;
}
+
+ uvc_urb->async_operations++;
}
static void uvc_video_decode_end(struct uvc_streaming *stream,
@@ -1324,7 +1359,7 @@ static void uvc_video_decode_isoc(struct uvc_urb
[PATCH v5 5/9] media: uvcvideo: queue: Support asynchronous buffer handling
From: Kieran Bingham
The buffer queue interface currently operates sequentially, processing
buffers after they have fully completed.
In preparation for supporting parallel tasks operating on the buffers,
we will need to support buffers being processed on multiple CPUs.
Adapt the uvc_queue_next_buffer() such that a reference count tracks the
active use of the buffer, returning the buffer to the VB2 stack at
completion.
Signed-off-by: Kieran Bingham
v5:
- uvc_queue_requeue() -> uvc_queue_buffer_requeue()
- Fix comment
---
drivers/media/usb/uvc/uvc_queue.c | 61 ++--
drivers/media/usb/uvc/uvcvideo.h | 4 ++-
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index bebf2415d9de..cd8c03341de0 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -142,6 +142,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
spin_lock_irqsave(>irqlock, flags);
if (likely(!(queue->flags & UVC_QUEUE_DISCONNECTED))) {
+ kref_init(>ref);
list_add_tail(>queue, >irqqueue);
} else {
/* If the device is disconnected return the buffer to userspace
@@ -459,28 +460,66 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct
uvc_video_queue *queue)
return nextbuf;
}
-struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
+/*
+ * uvc_queue_buffer_requeue: Requeue a buffer on our internal irqqueue
+ *
+ * Reuse a buffer through our internal queue without the need to 'prepare'.
+ * The buffer will be returned to userspace through the uvc_buffer_queue call
if
+ * the device has been disconnected.
+ */
+static void uvc_queue_buffer_requeue(struct uvc_video_queue *queue,
struct uvc_buffer *buf)
{
- struct uvc_buffer *nextbuf;
- unsigned long flags;
+ buf->error = 0;
+ buf->state = UVC_BUF_STATE_QUEUED;
+ buf->bytesused = 0;
+ vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
+
+ uvc_buffer_queue(>buf.vb2_buf);
+}
+
+static void uvc_queue_buffer_complete(struct kref *ref)
+{
+ struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref);
+ struct vb2_buffer *vb = >buf.vb2_buf;
+ struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
if ((queue->flags & UVC_QUEUE_DROP_CORRUPTED) && buf->error) {
- buf->error = 0;
- buf->state = UVC_BUF_STATE_QUEUED;
- buf->bytesused = 0;
- vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
- return buf;
+ uvc_queue_buffer_requeue(queue, buf);
+ return;
}
+ buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
+ vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused);
+ vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
+}
+
+/*
+ * Release a reference on the buffer. Complete the buffer when the last
+ * reference is released.
+ */
+void uvc_queue_buffer_release(struct uvc_buffer *buf)
+{
+ kref_put(>ref, uvc_queue_buffer_complete);
+}
+
+/*
+ * Remove this buffer from the queue. Lifetime will persist while async actions
+ * are still running (if any), and uvc_queue_buffer_release will give the
buffer
+ * back to VB2 when all users have completed.
+ */
+struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
+ struct uvc_buffer *buf)
+{
+ struct uvc_buffer *nextbuf;
+ unsigned long flags;
+
spin_lock_irqsave(>irqlock, flags);
list_del(>queue);
nextbuf = __uvc_queue_get_current_buffer(queue);
spin_unlock_irqrestore(>irqlock, flags);
- buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
- vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused);
- vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
+ uvc_queue_buffer_release(buf);
return nextbuf;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index bdb6d8daedab..1bc17da7f3d4 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -410,6 +410,9 @@ struct uvc_buffer {
unsigned int bytesused;
u32 pts;
+
+ /* Asynchronous buffer handling. */
+ struct kref ref;
};
#define UVC_QUEUE_DISCONNECTED (1 << 0)
@@ -726,6 +729,7 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int
disconnect);
struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
struct uvc_buffer *buf);
struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue);
+void uvc_queue_buffer_release(struct uvc_buffer *buf);
int uvc_queue_mmap(struct uvc_video_queue *queue,
struct vm_area_struct *vma);
__poll_t uvc_queue_poll(struct uvc_video_queue *queue, struct file *file,
--
git-series
Re: [RFC] Create test script(s?) for regression testing
Hi Hans, On Tuesday, 6 November 2018 15:56:34 EET Hans Verkuil wrote: > On 11/06/18 14:12, Laurent Pinchart wrote: > > On Tuesday, 6 November 2018 13:36:55 EET Sakari Ailus wrote: > >> On Tue, Nov 06, 2018 at 09:37:07AM +0100, Hans Verkuil wrote: > >>> Hi all, > >>> > >>> After the media summit (heavy on test discussions) and the V4L2 event > >>> regression we just found it is clear we need to do a better job with > >>> testing. > >>> > >>> All the pieces are in place, so what is needed is to combine it and > >>> create a script that anyone of us as core developers can run to check > >>> for regressions. The same script can be run as part of the kernelci > >>> regression testing. > >> > >> I'd say that *some* pieces are in place. Of course, the more there is, > >> the better. > >> > >> The more there are tests, the more important it would be they're > >> automated, preferrably without the developer having to run them on his/ > >> her own machine. > > > > From my experience with testing, it's important to have both a core set of > > tests (a.k.a. smoke tests) that can easily be run on developers' machines, > > and extended tests that can be offloaded to a shared testing > > infrastructure (but possibly also run locally if desired). > > That was my idea as well for the longer term. First step is to do the basic > smoke tests (i.e. run compliance tests, do some (limited) streaming test). > > There are more extensive (and longer running) tests that can be done, but > that's something to look at later. > > >>> We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last > >>> one is IMHO not quite good enough yet for testing: it is not fully > >>> compliant to the upcoming stateful codec spec. Work for that is planned > >>> as part of an Outreachy project. > >>> > >>> My idea is to create a script that is maintained as part of v4l-utils > >>> that loads the drivers and runs v4l2-compliance and possibly other tests > >>> against the virtual drivers. > >> > >> How about spending a little time to pick a suitable framework for running > >> the tests? It could be useful to get more informative reports than just > >> pass / fail. > > > > We should keep in mind that other tests will be added later, and the test > > framework should make that easy. > > Since we want to be able to run this on kernelci.org, I think it makes sense > to let the kernelci folks (Hi Ezequiel!) decide this. KernelCI isn't the only test infrastructure out there, so let's not forget about the other ones. > As a developer all I need is a script to run smoke tests so I can catch most > regressions (you never catch all). > > I'm happy to work with them to make any changes to compliance tools and > scripts so they fit better into their test framework. > > The one key requirement to all this is that you should be able to run these > tests without dependencies to all sorts of external packages/libraries. v4l-utils already has a set of dependencies, but those are largely manageable. For v4l2-compliance we'll install libv4l, which depends on libjpeg. > > Regarding the test output, many formats exist (see > > https://testanything.org/ and > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/ > > json_test_results_format.md for instance), we should pick one of the > > leading industry standards (what those standards are still needs to be > > researched :-)). > > > >> Do note that for different hardware the tests would be likely different > >> as well although there are classes of devices for which the exact same > >> tests would be applicable. > > > > See http://git.ideasonboard.com/renesas/vsp-tests.git for an example of > > device-specific tests. I think some of that could be generalized. > > > >>> It should be simple to use and require very little in the way of > >>> dependencies. Ideally no dependencies other than what is in v4l-utils so > >>> it can easily be run on an embedded system as well. > >>> > >>> For a 64-bit kernel it should run the tests both with 32-bit and 64-bit > >>> applications. > >>> > >>> It should also test with both single and multiplanar modes where > >>> available. > >>> > >>> Since vivid emulates CEC as well, it should run CEC tests too. > >>> > >>> As core developers we should have an environment where we can easily > >>> test our patches with this script (I use a VM for that). > >>> > >>> I think maintaining the script (or perhaps scripts) in v4l-utils is best > >>> since that keeps it in sync with the latest kernel and v4l-utils > >>> developments. > >> > >> Makes sense --- and that can be always changed later on if there's a need > >> to. > > > > I wonder whether that would be best going forward, especially if we want > > to add more tests. Wouldn't a v4l-tests project make sense ? > > Let's see what happens. The more repos you have, the harder it becomes to > keep everything in sync with the latest kernel code. Why is that ? How would a v4l-tests repository
RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
Hi, Mauro,
Thanks for your review.
> -Original Message-
> From: Mauro Carvalho Chehab [mailto:mchehab+sams...@kernel.org]
> Sent: Friday, November 2, 2018 6:49 AM
> To: Zhi, Yong
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com;
> tf...@chromium.org; hans.verk...@cisco.com;
> laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> ; Zheng, Jian Xu ; Hu,
> Jerry W ; Toivonen, Tuukka
> ; Qiu, Tian Shu ; Cao,
> Bingbu ; Li, Chao C
> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
>
> Em Mon, 29 Oct 2018 15:22:57 -0700
> Yong Zhi escreveu:
>
> > These meta formats are used on Intel IPU3 ImgU video queues
> > to carry 3A statistics and ISP pipeline parameters.
>
> Just minor things. See below.
>
> >
> > V4L2_META_FMT_IPU3_3A
> > V4L2_META_FMT_IPU3_PARAMS
> >
> > Signed-off-by: Yong Zhi
> > Signed-off-by: Chao C Li
> > Signed-off-by: Rajmohan Mani
> > ---
> > Documentation/media/uapi/v4l/meta-formats.rst |1 +
> > .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst | 181 ++
>
> I would actually prefer to have those two changes merged together with
> patch 1, as it makes easier for review.
>
> > include/uapi/linux/intel-ipu3.h| 2819
> >
>
> This one makes sense to have a separate patch.
>
Ack, will re-group the three files as suggested.
> > 3 files changed, 3001 insertions(+)
> > create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-
> ipu3.rst
> > create mode 100644 include/uapi/linux/intel-ipu3.h
> >
> > diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst
> > index cf971d5..eafc534 100644
> > --- a/Documentation/media/uapi/v4l/meta-formats.rst
> > +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> > @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata`
> interface only.
> > .. toctree::
> > :maxdepth: 1
> >
> > +pixfmt-meta-intel-ipu3
> > pixfmt-meta-d4xx
> > pixfmt-meta-uvc
> > pixfmt-meta-vsp1-hgo
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > new file mode 100644
> > index 000..23b945b
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > @@ -0,0 +1,181 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _intel-ipu3:
> > +
> >
> +***
> ***
> > +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A
> ('ip3s')
> >
> +***
> ***
> > +
> > +.. c:type:: ipu3_uapi_stats_3a
> > +
> > +3A statistics
> > +=
> > +
> > +For IPU3 ImgU, the 3A statistics accelerators collect different statistics
> > over
> > +an input bayer frame. Those statistics, defined in data struct
> > +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu
> 3a stat"
> > +video node, which are then passed to user space for statistics analysis
> > +using :c:type:`v4l2_meta_format` interface.
> > +
> > +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green,
> Blue and
> > +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter
> response,
> > +and AE (Auto-exposure) histogram.
> > +
> > +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all
> above.
> > +
> > +
> > +.. code-block:: c
> > +
> > +
> > + struct ipu3_uapi_stats_3a {
> > + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> > +__attribute__((aligned(32)));
> > + struct ipu3_uapi_ae_raw_buffer_aligned
> > + ae_raw_buffer[IPU3_UAPI_MAX_STRIPES];
> > + struct ipu3_uapi_af_raw_buffer af_raw_buffer;
> > + struct ipu3_uapi_awb_fr_raw_buffer awb_fr_raw_buffer;
> > + struct ipu3_uapi_4a_config stats_4a_config;
> > + __u32 ae_join_buffers;
> > + __u8 padding[28];
> > + struct ipu3_uapi_stats_3a_bubble_info_per_stripe
> > + stats_3a_bubble_per_stripe;
> > + struct ipu3_uapi_ff_status stats_3a_status;
> > + } __packed;
> > +
> > +
> > +.. c:type:: ipu3_uapi_params
> > +
> > +Pipeline parameters
> > +===
> > +
> > +IPU3 pipeline has a number of image processing stages, each of which
> takes a
> > +set of parameters as input. The major stages of pipelines are shown here:
> > +
> > +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> > +
> > +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> > +
> > +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> > +
> > +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> > +
> > +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> > +
> > +Correction -> XNR3 -> TNR -> DDR
> > +
> > +The table below presents a description of the above algorithms.
> > +
> > +
> ===
> >
[PATCH] [v4l-utils] libv4l2subdev: Add MEDIA_BUS_FMT_FIXED to mbus_formats[]
Also add V4L2_COLORSPACE_RAW to the colorspaces[].
Signed-off-by: Yong Zhi
---
utils/media-ctl/libv4l2subdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index a989efb..46668eb 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -855,6 +855,7 @@ static const struct {
enum v4l2_mbus_pixelcode code;
} mbus_formats[] = {
#include "media-bus-format-names.h"
+ { "FIXED", MEDIA_BUS_FMT_FIXED},
{ "Y8", MEDIA_BUS_FMT_Y8_1X8},
{ "Y10", MEDIA_BUS_FMT_Y10_1X10 },
{ "Y12", MEDIA_BUS_FMT_Y12_1X12 },
@@ -965,7 +966,9 @@ static struct {
{ "srgb", V4L2_COLORSPACE_SRGB },
{ "oprgb", V4L2_COLORSPACE_OPRGB },
{ "bt2020", V4L2_COLORSPACE_BT2020 },
+ { "raw", V4L2_COLORSPACE_RAW },
{ "dcip3", V4L2_COLORSPACE_DCI_P3 },
+
};
const char *v4l2_subdev_colorspace_to_string(enum v4l2_colorspace colorspace)
--
2.7.4
[GIT PULL FOR v4.21] cec: add new SECO driver
This pull request adds CEC support to SECO devices, in particular UDOO X86. The pull request is identical to the v4 of this series: https://lkml.org/lkml/2018/10/21/143 Just rebased. Regards, Hans The following changes since commit fbe57dde7126d1b2712ab5ea93fb9d15f89de708: media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure (2018-11-06 07:17:02 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-seco for you to fetch changes up to 6877d7bd0438784fef213beb545f9a09eea80c4a: seco-cec: add Consumer-IR support (2018-11-06 15:49:33 +0100) Tag branch Ettore Chimenti (2): media: add SECO cec driver seco-cec: add Consumer-IR support MAINTAINERS| 6 + drivers/media/platform/Kconfig | 22 ++ drivers/media/platform/Makefile| 2 + drivers/media/platform/seco-cec/Makefile | 1 + drivers/media/platform/seco-cec/seco-cec.c | 795 + drivers/media/platform/seco-cec/seco-cec.h | 141 6 files changed, 967 insertions(+) create mode 100644 drivers/media/platform/seco-cec/Makefile create mode 100644 drivers/media/platform/seco-cec/seco-cec.c create mode 100644 drivers/media/platform/seco-cec/seco-cec.h
Re: [PATCH 1/3] media: imx: add capture compose rectangle
Hi Sakari,
On Tue, 2018-11-06 at 16:01 +0200, Sakari Ailus wrote:
[...]
> @@ -290,6 +294,35 @@ static int capture_s_std(struct file *file, void *fh,
> v4l2_std_id std)
> > return v4l2_subdev_call(priv->src_sd, video, s_std, std);
> > }
> >
> > +static int capture_g_selection(struct file *file, void *fh,
> > + struct v4l2_selection *s)
> > +{
> > + struct capture_priv *priv = video_drvdata(file);
> > +
> > + switch (s->target) {
> > + case V4L2_SEL_TGT_CROP:
> > + case V4L2_SEL_TGT_CROP_DEFAULT:
> > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > + case V4L2_SEL_TGT_NATIVE_SIZE:
>
> The NATIVE_SIZE is for devices such as sensors. It doesn't make sense here.
Should this be documented in Documentation/media/uapi/v4l/v4l2-
selection-targets.rst ? There it only mentions when to make it
writeable.
> With that removed,
>
> Acked-by: Sakari Ailus
Thank you, I'll remove that line.
regards
Philipp
[PATCH] vivid: fill in media_device bus_info
If you create multiple vivid instances, each with their own media device, then there was no way to tell them apart. Fill in the bus_info so each instance has a unique bus_info string. Signed-off-by: Hans Verkuil --- diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index bc7307183b1d..c1b5976af3e6 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -670,6 +670,8 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) /* Initialize media device */ strlcpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model)); + snprintf(dev->mdev.bus_info, sizeof(dev->mdev.bus_info), +"platform:%s-%03d", VIVID_MODULE_NAME, inst); dev->mdev.dev = >dev; media_device_init(>mdev); dev->mdev.ops = _media_ops;
Re: [PATCH 1/3] media: imx: add capture compose rectangle
Hi Philipp,
On Mon, Nov 05, 2018 at 04:20:53PM +0100, Philipp Zabel wrote:
> Allowing to compose captured images into larger memory buffers
> will let us lift alignment restrictions on CSI crop width.
>
> Signed-off-by: Philipp Zabel
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 3 +-
> drivers/staging/media/imx/imx-media-capture.c | 38 +++
> drivers/staging/media/imx/imx-media-csi.c | 3 +-
> drivers/staging/media/imx/imx-media-vdic.c| 4 +-
> drivers/staging/media/imx/imx-media.h | 2 +
> 5 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
> b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 28f41caba05d..fe5a77baa592 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -366,8 +366,7 @@ static int prp_setup_channel(struct prp_priv *priv,
>
> memset(, 0, sizeof(image));
> image.pix = vdev->fmt.fmt.pix;
> - image.rect.width = image.pix.width;
> - image.rect.height = image.pix.height;
> + image.rect = vdev->compose;
>
> if (rot_swap_width_height) {
> swap(image.pix.width, image.pix.height);
> diff --git a/drivers/staging/media/imx/imx-media-capture.c
> b/drivers/staging/media/imx/imx-media-capture.c
> index b37e1186eb2f..cace8a51aca8 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -262,6 +262,10 @@ static int capture_s_fmt_vid_cap(struct file *file, void
> *fh,
> priv->vdev.fmt.fmt.pix = f->fmt.pix;
> priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
> CS_SEL_ANY, true);
> + priv->vdev.compose.left = 0;
> + priv->vdev.compose.top = 0;
> + priv->vdev.compose.width = f->fmt.fmt.pix.width;
> + priv->vdev.compose.height = f->fmt.fmt.pix.height;
>
> return 0;
> }
> @@ -290,6 +294,35 @@ static int capture_s_std(struct file *file, void *fh,
> v4l2_std_id std)
> return v4l2_subdev_call(priv->src_sd, video, s_std, std);
> }
>
> +static int capture_g_selection(struct file *file, void *fh,
> +struct v4l2_selection *s)
> +{
> + struct capture_priv *priv = video_drvdata(file);
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_NATIVE_SIZE:
The NATIVE_SIZE is for devices such as sensors. It doesn't make sense here.
With that removed,
Acked-by: Sakari Ailus
> + case V4L2_SEL_TGT_COMPOSE:
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_COMPOSE_PADDED:
> + s->r = priv->vdev.compose;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int capture_s_selection(struct file *file, void *fh,
> +struct v4l2_selection *s)
> +{
> + return capture_g_selection(file, fh, s);
> +}
> +
> static int capture_g_parm(struct file *file, void *fh,
> struct v4l2_streamparm *a)
> {
> @@ -350,6 +383,9 @@ static const struct v4l2_ioctl_ops capture_ioctl_ops = {
> .vidioc_g_std = capture_g_std,
> .vidioc_s_std = capture_s_std,
>
> + .vidioc_g_selection = capture_g_selection,
> + .vidioc_s_selection = capture_s_selection,
> +
> .vidioc_g_parm = capture_g_parm,
> .vidioc_s_parm = capture_s_parm,
>
> @@ -687,6 +723,8 @@ int imx_media_capture_device_register(struct
> imx_media_video_dev *vdev)
> vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> imx_media_mbus_fmt_to_pix_fmt(>fmt.fmt.pix,
> _src.format, NULL);
> + vdev->compose.width = fmt_src.format.width;
> + vdev->compose.height = fmt_src.format.height;
> vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
>CS_SEL_ANY, false);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c
> b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..c4523afe7b48 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -413,8 +413,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>
> memset(, 0, sizeof(image));
> image.pix = vdev->fmt.fmt.pix;
> - image.rect.width = image.pix.width;
> - image.rect.height = image.pix.height;
> + image.rect = vdev->compose;
>
> csi_idmac_setup_vb2_buf(priv, phys);
>
> diff --git a/drivers/staging/media/imx/imx-media-vdic.c
> b/drivers/staging/media/imx/imx-media-vdic.c
> index 482250d47e7c..e08d296cf4eb 100644
> --- a/drivers/staging/media/imx/imx-media-vdic.c
> +++ b/drivers/staging/media/imx/imx-media-vdic.c
> @@
Re: [RFC] Create test script(s?) for regression testing
On 11/06/18 14:12, Laurent Pinchart wrote: > Hello, > > On Tuesday, 6 November 2018 13:36:55 EET Sakari Ailus wrote: >> On Tue, Nov 06, 2018 at 09:37:07AM +0100, Hans Verkuil wrote: >>> Hi all, >>> >>> After the media summit (heavy on test discussions) and the V4L2 event >>> regression we just found it is clear we need to do a better job with >>> testing. >>> >>> All the pieces are in place, so what is needed is to combine it and create >>> a script that anyone of us as core developers can run to check for >>> regressions. The same script can be run as part of the kernelci >>> regression testing. >> >> I'd say that *some* pieces are in place. Of course, the more there is, the >> better. >> >> The more there are tests, the more important it would be they're automated, >> preferrably without the developer having to run them on his/her own >> machine. > > From my experience with testing, it's important to have both a core set of > tests (a.k.a. smoke tests) that can easily be run on developers' machines, > and > extended tests that can be offloaded to a shared testing infrastructure (but > possibly also run locally if desired). That was my idea as well for the longer term. First step is to do the basic smoke tests (i.e. run compliance tests, do some (limited) streaming test). There are more extensive (and longer running) tests that can be done, but that's something to look at later. >>> We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last one >>> is IMHO not quite good enough yet for testing: it is not fully compliant >>> to the upcoming stateful codec spec. Work for that is planned as part of >>> an Outreachy project. >>> >>> My idea is to create a script that is maintained as part of v4l-utils that >>> loads the drivers and runs v4l2-compliance and possibly other tests >>> against the virtual drivers. >> >> How about spending a little time to pick a suitable framework for running >> the tests? It could be useful to get more informative reports than just >> pass / fail. > > We should keep in mind that other tests will be added later, and the test > framework should make that easy. Since we want to be able to run this on kernelci.org, I think it makes sense to let the kernelci folks (Hi Ezequiel!) decide this. As a developer all I need is a script to run smoke tests so I can catch most regressions (you never catch all). I'm happy to work with them to make any changes to compliance tools and scripts so they fit better into their test framework. The one key requirement to all this is that you should be able to run these tests without dependencies to all sorts of external packages/libraries. > Regarding the test output, many formats exist (see https://testanything.org/ > and https://chromium.googlesource.com/chromium/src/+/master/docs/testing/ > json_test_results_format.md for instance), we should pick one of the leading > industry standards (what those standards are still needs to be researched > :-)). > >> Do note that for different hardware the tests would be likely different as >> well although there are classes of devices for which the exact same tests >> would be applicable. > > See http://git.ideasonboard.com/renesas/vsp-tests.git for an example of > device-specific tests. I think some of that could be generalized. > >>> It should be simple to use and require very little in the way of >>> dependencies. Ideally no dependencies other than what is in v4l-utils so >>> it can easily be run on an embedded system as well. >>> >>> For a 64-bit kernel it should run the tests both with 32-bit and 64-bit >>> applications. >>> >>> It should also test with both single and multiplanar modes where >>> available. >>> >>> Since vivid emulates CEC as well, it should run CEC tests too. >>> >>> As core developers we should have an environment where we can easily test >>> our patches with this script (I use a VM for that). >>> >>> I think maintaining the script (or perhaps scripts) in v4l-utils is best >>> since that keeps it in sync with the latest kernel and v4l-utils >>> developments. >> >> Makes sense --- and that can be always changed later on if there's a need >> to. > > I wonder whether that would be best going forward, especially if we want to > add more tests. Wouldn't a v4l-tests project make sense ? > Let's see what happens. The more repos you have, the harder it becomes to keep everything in sync with the latest kernel code. My experience is that if you want to have good tests, then writing tests should be as easy as possible. Keep dependencies at an absolute minimum. Let's be honest, we (well, mainly me) are doing these tests as a side job, it's not our main focus. Anything that makes writing tests more painful is bad and just gets in the way. Regards, Hans
Re: [RFC] Create test script(s?) for regression testing
On Tue, 2018-11-06 at 09:37 +0100, Hans Verkuil wrote: > Hi all, > > After the media summit (heavy on test discussions) and the V4L2 event > regression > we just found it is clear we need to do a better job with testing. > > All the pieces are in place, so what is needed is to combine it and create a > script that anyone of us as core developers can run to check for regressions. > The same script can be run as part of the kernelci regression testing. > > We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last one > is IMHO not quite good enough yet for testing: it is not fully compliant to > the > upcoming stateful codec spec. Work for that is planned as part of an Outreachy > project. > > My idea is to create a script that is maintained as part of v4l-utils that > loads the drivers and runs v4l2-compliance and possibly other tests against > the virtual drivers. > > It should be simple to use and require very little in the way of dependencies. > Ideally no dependencies other than what is in v4l-utils so it can easily be > run > on an embedded system as well. > > For a 64-bit kernel it should run the tests both with 32-bit and 64-bit > applications. > > It should also test with both single and multiplanar modes where available. > > Since vivid emulates CEC as well, it should run CEC tests too. > > As core developers we should have an environment where we can easily test > our patches with this script (I use a VM for that). > It's quite trivial to setup a qemu environment for this, e.g. you can use virtme [1] and set it up so that it runs a script after booting. > I think maintaining the script (or perhaps scripts) in v4l-utils is best since > that keeps it in sync with the latest kernel and v4l-utils developments. > > Comments? Ideas? > Sounds great. I think it makes a lot of sense to have a script for CIs and developers to run. I guess we can start simple, with just a bash script? > Regards, > > Hans [1] https://www.collabora.com/news-and-blog/blog/2018/09/18/virtme-the-kernel-developers-best-friend/
[GIT PULL FOR v4.21] Convert last remaining g/s_crop/cropcap drivers
This pull request converts the last remaining drivers that use g/s_crop and cropcap to g/s_selection. The first two patches do some minor code cleanup. The third patch adds a new video_device flag to indicate that the driver inverts the normal usage of g/s_crop/cropcap. This applies to the old Samsung drivers that predate the Selection API and that abused the existing crop API. The next three patches do some code cleanup and prepare drivers for the removal of g/s_crop and ensure that cropcap only returns the pixelaspect. The next three patches convert the remaining Samsung drivers and set the QUIRK flag for all three. The final two patches remove vidioc_g/s_crop and rename vidioc_cropcap to vidioc_g_pixelaspect. This pull request is identical to the original RFC patch series: https://www.mail-archive.com/linux-media@vger.kernel.org/msg135494.html I was waiting for Samsung to test these changes, and Sylwester just did that. Thank you very much, Sylwester! It's really nice to have a single driver API for cropping and composing instead of having to deal with two. Regards, Hans The following changes since commit ef86eaf97acd6d82cd3fd40f997b1c8c4895a443: media: Rename vb2_m2m_request_queue -> v4l2_m2m_request_queue (2018-11-06 05:24:22 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-crop2sel-v2 for you to fetch changes up to 588488146f93467815fd2ffdbabe7bb37d10e54c: vidioc_cropcap -> vidioc_g_pixelaspect (2018-11-06 13:05:45 +0100) Tag branch Hans Verkuil (11): v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE v4l2-common.h: put backwards compat defines under #ifndef __KERNEL__ v4l2-ioctl: add QUIRK_INVERTED_CROP davinci/vpbe: drop unused g_cropcap cropcap/g_selection split exynos-gsc: replace v4l2_crop by v4l2_selection s5p_mfc_dec.c: convert g_crop to g_selection exynos4-is: convert g/s_crop to g/s_selection s5p-g2d: convert g/s_crop to g/s_selection v4l2-ioctl: remove unused vidioc_g/s_crop vidioc_cropcap -> vidioc_g_pixelaspect drivers/media/pci/bt8xx/bttv-driver.c | 12 +++--- drivers/media/pci/cobalt/cobalt-v4l2.c| 48 +++ drivers/media/pci/cx18/cx18-ioctl.c | 13 --- drivers/media/pci/cx23885/cx23885-video.c | 40 +-- drivers/media/pci/ivtv/ivtv-ioctl.c | 17 drivers/media/pci/saa7134/saa7134-video.c | 21 +- drivers/media/platform/am437x/am437x-vpfe.c | 31 --- drivers/media/platform/davinci/vpbe.c | 23 --- drivers/media/platform/davinci/vpbe_display.c | 10 ++--- drivers/media/platform/davinci/vpfe_capture.c | 12 +++--- drivers/media/platform/exynos-gsc/gsc-core.c | 57 +++ drivers/media/platform/exynos-gsc/gsc-core.h | 3 +- drivers/media/platform/exynos-gsc/gsc-m2m.c | 23 +-- drivers/media/platform/exynos4-is/fimc-core.h | 6 ++- drivers/media/platform/exynos4-is/fimc-m2m.c | 130 -- drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 ++--- drivers/media/platform/s5p-g2d/g2d.c | 102 ++-- drivers/media/platform/s5p-mfc/s5p_mfc.c | 1 + drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 49 +++ drivers/media/platform/vivid/vivid-core.c | 9 +++-- drivers/media/platform/vivid/vivid-vid-cap.c | 18 - drivers/media/platform/vivid/vivid-vid-cap.h | 2 +- drivers/media/platform/vivid/vivid-vid-out.c | 18 - drivers/media/platform/vivid/vivid-vid-out.h | 2 +- drivers/media/usb/au0828/au0828-video.c | 38 -- drivers/media/usb/cpia2/cpia2_v4l.c | 31 +++ drivers/media/usb/cx231xx/cx231xx-417.c | 41 ++-- drivers/media/usb/cx231xx/cx231xx-video.c | 41 ++-- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 13 --- drivers/media/v4l2-core/v4l2-dev.c| 8 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 44 + include/media/davinci/vpbe.h | 4 -- include/media/v4l2-dev.h | 13 ++- include/media/v4l2-ioctl.h| 16 ++-- include/uapi/linux/v4l2-common.h | 28 +++--- 35 files changed, 537 insertions(+), 397 deletions(-)
Re: [RFC] Create test script(s?) for regression testing
Hello, On Tuesday, 6 November 2018 13:36:55 EET Sakari Ailus wrote: > On Tue, Nov 06, 2018 at 09:37:07AM +0100, Hans Verkuil wrote: > > Hi all, > > > > After the media summit (heavy on test discussions) and the V4L2 event > > regression we just found it is clear we need to do a better job with > > testing. > > > > All the pieces are in place, so what is needed is to combine it and create > > a script that anyone of us as core developers can run to check for > > regressions. The same script can be run as part of the kernelci > > regression testing. > > I'd say that *some* pieces are in place. Of course, the more there is, the > better. > > The more there are tests, the more important it would be they're automated, > preferrably without the developer having to run them on his/her own > machine. >From my experience with testing, it's important to have both a core set of tests (a.k.a. smoke tests) that can easily be run on developers' machines, and extended tests that can be offloaded to a shared testing infrastructure (but possibly also run locally if desired). > > We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last one > > is IMHO not quite good enough yet for testing: it is not fully compliant > > to the upcoming stateful codec spec. Work for that is planned as part of > > an Outreachy project. > > > > My idea is to create a script that is maintained as part of v4l-utils that > > loads the drivers and runs v4l2-compliance and possibly other tests > > against the virtual drivers. > > How about spending a little time to pick a suitable framework for running > the tests? It could be useful to get more informative reports than just > pass / fail. We should keep in mind that other tests will be added later, and the test framework should make that easy. Regarding the test output, many formats exist (see https://testanything.org/ and https://chromium.googlesource.com/chromium/src/+/master/docs/testing/ json_test_results_format.md for instance), we should pick one of the leading industry standards (what those standards are still needs to be researched :-)). > Do note that for different hardware the tests would be likely different as > well although there are classes of devices for which the exact same tests > would be applicable. See http://git.ideasonboard.com/renesas/vsp-tests.git for an example of device-specific tests. I think some of that could be generalized. > > It should be simple to use and require very little in the way of > > dependencies. Ideally no dependencies other than what is in v4l-utils so > > it can easily be run on an embedded system as well. > > > > For a 64-bit kernel it should run the tests both with 32-bit and 64-bit > > applications. > > > > It should also test with both single and multiplanar modes where > > available. > > > > Since vivid emulates CEC as well, it should run CEC tests too. > > > > As core developers we should have an environment where we can easily test > > our patches with this script (I use a VM for that). > > > > I think maintaining the script (or perhaps scripts) in v4l-utils is best > > since that keeps it in sync with the latest kernel and v4l-utils > > developments. > > Makes sense --- and that can be always changed later on if there's a need > to. I wonder whether that would be best going forward, especially if we want to add more tests. Wouldn't a v4l-tests project make sense ? -- Regards, Laurent Pinchart
Re: [PATCH] cedrus: check if kzalloc() fails
On Tue, Nov 06, 2018 at 06:21:29AM -0500, Mauro Carvalho Chehab wrote: > As warned by static code analizer checkers: > drivers/staging/media/sunxi/cedrus/cedrus.c: > drivers/staging/media/sunxi/cedrus/cedrus.c:93 cedrus_init_ctrls() error: > potential null dereference 'ctx->ctrls'. (kzalloc returns null) > > The problem is that it assumes that kzalloc() will always > succeed. > > Signed-off-by: Mauro Carvalho Chehab Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH] media: dm365_ipipeif: better annotate a fall though
On 11/06/18 11:55, Mauro Carvalho Chehab wrote:
> Shut up this warning:
>
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function
> 'ipipeif_hw_setup':
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this
> statement may fall through [-Wimplicit-fallthrough=]
> switch (isif_port_if) {
> ^~
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
> case IPIPEIF_SDRAM_YUV:
> ^~~~
>
> By annotating a fall though case at the right place.
>
> Signed-off-by: Mauro Carvalho Chehab
Acked-by: Hans Verkuil
Thanks!
Hans
> ---
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> index a53231b08d30..e3425bf082ae 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> @@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
> ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
> break;
> }
> + /* fall through */
>
> case IPIPEIF_SDRAM_YUV:
> /* Set clock divider */
>
[GIT PULL for 4.20] Sensor and ISP driver patches for 4.21
Hi Mauro, Here are a few sensor and ISP driver patches for 4.21, plus a documentation fix. The noteworthy change in the sea of bugfixes is the imx214 driver. Please pull. The following changes since commit dafb7f9aef2fd44991ff1691721ff765a23be27b: v4l2-controls: add a missing include (2018-11-02 06:36:32 -0400) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git tags/for-4.21-1-sign for you to fetch changes up to aaa886f8404b6ae39aad984c8b826c092ebe0092: media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure (2018-11-06 13:33:19 +0200) Patches or 4.21 Chiranjeevi Rapolu (1): media: ov13858: Check for possible null pointer Julia Lawall (2): media: ov5645: constify v4l2_ctrl_ops structure media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure Rajmohan Mani (1): media: intel-ipu3: cio2: Remove redundant definitions Ricardo Ribalda Delgado (2): imx214: device tree binding imx214: Add imx214 camera sensor driver Sakari Ailus (4): media: docs: Document metadata format in struct v4l2_format omap3isp: Unregister media device as first ipu3-cio2: Unregister device nodes first, then release resources ipu3-cio2: Use cio2_queues_exit .../devicetree/bindings/media/i2c/sony,imx214.txt | 53 + Documentation/media/uapi/v4l/dev-meta.rst |2 +- Documentation/media/uapi/v4l/vidioc-g-fmt.rst |5 + MAINTAINERS|8 + drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/imx214.c | 1118 drivers/media/i2c/ov13858.c|6 +- drivers/media/i2c/ov5645.c |2 +- drivers/media/i2c/ov7740.c |4 +- drivers/media/pci/intel/ipu3/ipu3-cio2.c |6 +- drivers/media/pci/intel/ipu3/ipu3-cio2.h |2 - drivers/media/platform/omap3isp/isp.c |3 +- 13 files changed, 1209 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx214.txt create mode 100644 drivers/media/i2c/imx214.c -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [RFC] Create test script(s?) for regression testing
Hi Hans, On Tue, Nov 06, 2018 at 09:37:07AM +0100, Hans Verkuil wrote: > Hi all, > > After the media summit (heavy on test discussions) and the V4L2 event > regression > we just found it is clear we need to do a better job with testing. > > All the pieces are in place, so what is needed is to combine it and create a > script that anyone of us as core developers can run to check for regressions. > The same script can be run as part of the kernelci regression testing. I'd say that *some* pieces are in place. Of course, the more there is, the better. The more there are tests, the more important it would be they're automated, preferrably without the developer having to run them on his/her own machine. > > We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last one > is IMHO not quite good enough yet for testing: it is not fully compliant to > the > upcoming stateful codec spec. Work for that is planned as part of an Outreachy > project. > > My idea is to create a script that is maintained as part of v4l-utils that > loads the drivers and runs v4l2-compliance and possibly other tests against > the virtual drivers. How about spending a little time to pick a suitable framework for running the tests? It could be useful to get more informative reports than just pass / fail. Do note that for different hardware the tests would be likely different as well although there are classes of devices for which the exact same tests would be applicable. > > It should be simple to use and require very little in the way of dependencies. > Ideally no dependencies other than what is in v4l-utils so it can easily be > run > on an embedded system as well. > > For a 64-bit kernel it should run the tests both with 32-bit and 64-bit > applications. > > It should also test with both single and multiplanar modes where available. > > Since vivid emulates CEC as well, it should run CEC tests too. > > As core developers we should have an environment where we can easily test > our patches with this script (I use a VM for that). > > I think maintaining the script (or perhaps scripts) in v4l-utils is best since > that keeps it in sync with the latest kernel and v4l-utils developments. Makes sense --- and that can be always changed later on if there's a need to. -- Regards, Sakari Ailus sakari.ai...@linux.intel.com
[PATCH] cedrus: check if kzalloc() fails
As warned by static code analizer checkers:
drivers/staging/media/sunxi/cedrus/cedrus.c:
drivers/staging/media/sunxi/cedrus/cedrus.c:93 cedrus_init_ctrls() error:
potential null dereference 'ctx->ctrls'. (kzalloc returns null)
The problem is that it assumes that kzalloc() will always
succeed.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/media/sunxi/cedrus/cedrus.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index dd121f66fa2d..6a73a7841303 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -72,6 +72,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct
cedrus_ctx *ctx)
ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;
ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
+ if (!ctx->ctrls)
+ return -ENOMEM;
memset(ctx->ctrls, 0, ctrl_size);
for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
--
2.19.1
[PATCH] media: dm365_ipipeif: better annotate a fall though
Shut up this warning:
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function
'ipipeif_hw_setup':
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this
statement may fall through [-Wimplicit-fallthrough=]
switch (isif_port_if) {
^~
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
case IPIPEIF_SDRAM_YUV:
^~~~
By annotating a fall though case at the right place.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
index a53231b08d30..e3425bf082ae 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
@@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}
+ /* fall through */
case IPIPEIF_SDRAM_YUV:
/* Set clock divider */
--
2.19.1
[GIT PULL FOR 4.20] Fix first event delivery
Hi Mauro, There turns out to have been an issue in the event subscription fix; in particular the first control event is missed due to a subtle bug in the patch. This patch fixes it. Once it's in, I'll submit the corresponding patches to the stable kernels. Please pull. The following changes since commit dafb7f9aef2fd44991ff1691721ff765a23be27b: v4l2-controls: add a missing include (2018-11-02 06:36:32 -0400) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git tags/event-sub-fix-sign for you to fetch changes up to cbafeff167c91243f336e1703d7f86aa019b973e: v4l: event: Add subscription to list before calling "add" operation (2018-11-06 10:57:34 +0200) fix event subscription Sakari Ailus (1): v4l: event: Add subscription to list before calling "add" operation drivers/media/v4l2-core/v4l2-event.c | 43 1 file changed, 24 insertions(+), 19 deletions(-) -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
[PATCH v2] media: coda: fix memory corruption in case more than 32 instances are opened
The ffz() return value is undefined if the instance mask does not
contain any zeros. If it returned 32, the following set_bit would
corrupt the debugfs_root pointer.
Switch to IDA for context index allocation. This also removes the
artificial 32 instance limit for all except CodaDx6.
Signed-off-by: Philipp Zabel
---
Changes since v1:
- #include explicitly where struct ida or ida_*
functions are used, reported by Ian Arkver
---
drivers/media/platform/coda/coda-common.c | 26 +--
drivers/media/platform/coda/coda.h| 3 ++-
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 2848ea5f464d..547acf80c89d 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -2099,17 +2100,6 @@ int coda_decoder_queue_init(void *priv, struct vb2_queue
*src_vq,
return coda_queue_init(priv, dst_vq);
}
-static int coda_next_free_instance(struct coda_dev *dev)
-{
- int idx = ffz(dev->instance_mask);
-
- if ((idx < 0) ||
- (dev->devtype->product == CODA_DX6 && idx > CODADX6_MAX_INSTANCES))
- return -EBUSY;
-
- return idx;
-}
-
/*
* File operations
*/
@@ -2118,7 +2108,8 @@ static int coda_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct coda_dev *dev = video_get_drvdata(vdev);
- struct coda_ctx *ctx = NULL;
+ struct coda_ctx *ctx;
+ unsigned int max = ~0;
char *name;
int ret;
int idx;
@@ -2127,12 +2118,13 @@ static int coda_open(struct file *file)
if (!ctx)
return -ENOMEM;
- idx = coda_next_free_instance(dev);
+ if (dev->devtype->product == CODA_DX6)
+ max = CODADX6_MAX_INSTANCES - 1;
+ idx = ida_alloc_max(>ida, max, GFP_KERNEL);
if (idx < 0) {
ret = idx;
goto err_coda_max;
}
- set_bit(idx, >instance_mask);
name = kasprintf(GFP_KERNEL, "context%d", idx);
if (!name) {
@@ -2241,8 +2233,8 @@ static int coda_open(struct file *file)
err_pm_get:
v4l2_fh_del(>fh);
v4l2_fh_exit(>fh);
- clear_bit(ctx->idx, >instance_mask);
err_coda_name_init:
+ ida_free(>ida, ctx->idx);
err_coda_max:
kfree(ctx);
return ret;
@@ -2284,7 +2276,7 @@ static int coda_release(struct file *file)
pm_runtime_put_sync(>plat_dev->dev);
v4l2_fh_del(>fh);
v4l2_fh_exit(>fh);
- clear_bit(ctx->idx, >instance_mask);
+ ida_free(>ida, ctx->idx);
if (ctx->ops->release)
ctx->ops->release(ctx);
debugfs_remove_recursive(ctx->debugfs_entry);
@@ -2745,6 +2737,7 @@ static int coda_probe(struct platform_device *pdev)
mutex_init(>dev_mutex);
mutex_init(>coda_mutex);
+ ida_init(>ida);
dev->debugfs_root = debugfs_create_dir("coda", NULL);
if (!dev->debugfs_root)
@@ -2832,6 +2825,7 @@ static int coda_remove(struct platform_device *pdev)
coda_free_aux_buf(dev, >tempbuf);
coda_free_aux_buf(dev, >workbuf);
debugfs_remove_recursive(dev->debugfs_root);
+ ida_destroy(>ida);
return 0;
}
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 19ac0b9dc6eb..680c7035c9d4 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -16,6 +16,7 @@
#define __CODA_H__
#include
+#include
#include
#include
#include
@@ -95,7 +96,7 @@ struct coda_dev {
struct workqueue_struct *workqueue;
struct v4l2_m2m_dev *m2m_dev;
struct list_headinstances;
- unsigned long instance_mask;
+ struct ida ida;
struct dentry *debugfs_root;
};
--
2.19.1
Re: [PATCH 01/15] media: coda: fix memory corruption in case more than 32 instances are opened
On Mon, 2018-11-05 at 16:32 +, Ian Arkver wrote:
> Hi Philipp,
>
> On 05/11/2018 15:24, Philipp Zabel wrote:
> > The ffz() return value is undefined if the instance mask does not
> > contain any zeros. If it returned 32, the following set_bit would
> > corrupt the debugfs_root pointer.
> > Switch to IDA for context index allocation. This also removes the
> > artificial 32 instance limit for all except CodaDx6.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/media/platform/coda/coda-common.c | 25 ---
> > drivers/media/platform/coda/coda.h| 2 +-
> > 2 files changed, 10 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c
> > b/drivers/media/platform/coda/coda-common.c
> > index 2848ea5f464d..cbb59c2f3a82 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -2099,17 +2099,6 @@ int coda_decoder_queue_init(void *priv, struct
> > vb2_queue *src_vq,
> > return coda_queue_init(priv, dst_vq);
> > }
> >
> > -static int coda_next_free_instance(struct coda_dev *dev)
> > -{
> > - int idx = ffz(dev->instance_mask);
> > -
> > - if ((idx < 0) ||
> > - (dev->devtype->product == CODA_DX6 && idx > CODADX6_MAX_INSTANCES))
> > - return -EBUSY;
> > -
> > - return idx;
> > -}
> > -
> > /*
> >* File operations
> >*/
> > @@ -2118,7 +2107,8 @@ static int coda_open(struct file *file)
> > {
> > struct video_device *vdev = video_devdata(file);
> > struct coda_dev *dev = video_get_drvdata(vdev);
> > - struct coda_ctx *ctx = NULL;
> > + struct coda_ctx *ctx;
> > + unsigned int max = ~0;
> > char *name;
> > int ret;
> > int idx;
> > @@ -2127,12 +2117,13 @@ static int coda_open(struct file *file)
> > if (!ctx)
> > return -ENOMEM;
> >
> > - idx = coda_next_free_instance(dev);
> > + if (dev->devtype->product == CODA_DX6)
> > + max = CODADX6_MAX_INSTANCES - 1;
> > + idx = ida_alloc_max(>ida, max, GFP_KERNEL);
> > if (idx < 0) {
> > ret = idx;
> > goto err_coda_max;
> > }
> > - set_bit(idx, >instance_mask);
> >
> > name = kasprintf(GFP_KERNEL, "context%d", idx);
> > if (!name) {
> > @@ -2241,8 +2232,8 @@ static int coda_open(struct file *file)
> > err_pm_get:
> > v4l2_fh_del(>fh);
> > v4l2_fh_exit(>fh);
> > - clear_bit(ctx->idx, >instance_mask);
> > err_coda_name_init:
> > + ida_free(>ida, ctx->idx);
> > err_coda_max:
> > kfree(ctx);
> > return ret;
> > @@ -2284,7 +2275,7 @@ static int coda_release(struct file *file)
> > pm_runtime_put_sync(>plat_dev->dev);
> > v4l2_fh_del(>fh);
> > v4l2_fh_exit(>fh);
> > - clear_bit(ctx->idx, >instance_mask);
> > + ida_free(>ida, ctx->idx);
> > if (ctx->ops->release)
> > ctx->ops->release(ctx);
> > debugfs_remove_recursive(ctx->debugfs_entry);
> > @@ -2745,6 +2736,7 @@ static int coda_probe(struct platform_device *pdev)
> >
> > mutex_init(>dev_mutex);
> > mutex_init(>coda_mutex);
> > + ida_init(>ida);
> >
> > dev->debugfs_root = debugfs_create_dir("coda", NULL);
> > if (!dev->debugfs_root)
> > @@ -2832,6 +2824,7 @@ static int coda_remove(struct platform_device *pdev)
> > coda_free_aux_buf(dev, >tempbuf);
> > coda_free_aux_buf(dev, >workbuf);
> > debugfs_remove_recursive(dev->debugfs_root);
> > + ida_destroy(>ida);
> > return 0;
> > }
> >
> > diff --git a/drivers/media/platform/coda/coda.h
> > b/drivers/media/platform/coda/coda.h
> > index 19ac0b9dc6eb..b6cd14ee91ea 100644
> > --- a/drivers/media/platform/coda/coda.h
> > +++ b/drivers/media/platform/coda/coda.h
>
> Should you add:
> #include
> to this header?
Yes, thanks. It currently is pulled in indirectly. I'll send a v2 with
the #include added for the first patch.
regards
Philipp
Re: [PATCH v4l-utils] Add missing linux/bpf_common.h
On Mon, Nov 05, 2018 at 09:30:47PM +0100, Peter Seiderer wrote: > Copy from [1], needed by bpf.h. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/include/uapi/linux/bpf_common.h?h=v4.19 So bpf.h does include this file, but we don't use anything from it in v4l-utils. This include file is for the original BPF, which has been around for a long time. So why is this include file missing, i.e. what problem are you trying to solve? Lastely, the file should be included in the sync-with-kernel target so it does not get out of sync -- should it really be necessary to add the file. Sean > > Signed-off-by: Peter Seiderer > --- > include/linux/bpf_common.h | 57 ++ > 1 file changed, 57 insertions(+) > create mode 100644 include/linux/bpf_common.h > > diff --git a/include/linux/bpf_common.h b/include/linux/bpf_common.h > new file mode 100644 > index ..ee97668b > --- /dev/null > +++ b/include/linux/bpf_common.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +#ifndef _UAPI__LINUX_BPF_COMMON_H__ > +#define _UAPI__LINUX_BPF_COMMON_H__ > + > +/* Instruction classes */ > +#define BPF_CLASS(code) ((code) & 0x07) > +#define BPF_LD 0x00 > +#define BPF_LDX 0x01 > +#define BPF_ST 0x02 > +#define BPF_STX 0x03 > +#define BPF_ALU 0x04 > +#define BPF_JMP 0x05 > +#define BPF_RET 0x06 > +#define BPF_MISC0x07 > + > +/* ld/ldx fields */ > +#define BPF_SIZE(code) ((code) & 0x18) > +#define BPF_W 0x00 /* 32-bit */ > +#define BPF_H 0x08 /* 16-bit */ > +#define BPF_B 0x10 /* 8-bit */ > +/* eBPF BPF_DW 0x1864-bit */ > +#define BPF_MODE(code) ((code) & 0xe0) > +#define BPF_IMM 0x00 > +#define BPF_ABS 0x20 > +#define BPF_IND 0x40 > +#define BPF_MEM 0x60 > +#define BPF_LEN 0x80 > +#define BPF_MSH 0xa0 > + > +/* alu/jmp fields */ > +#define BPF_OP(code)((code) & 0xf0) > +#define BPF_ADD 0x00 > +#define BPF_SUB 0x10 > +#define BPF_MUL 0x20 > +#define BPF_DIV 0x30 > +#define BPF_OR 0x40 > +#define BPF_AND 0x50 > +#define BPF_LSH 0x60 > +#define BPF_RSH 0x70 > +#define BPF_NEG 0x80 > +#define BPF_MOD 0x90 > +#define BPF_XOR 0xa0 > + > +#define BPF_JA 0x00 > +#define BPF_JEQ 0x10 > +#define BPF_JGT 0x20 > +#define BPF_JGE 0x30 > +#define BPF_JSET0x40 > +#define BPF_SRC(code) ((code) & 0x08) > +#define BPF_K 0x00 > +#define BPF_X 0x08 > + > +#ifndef BPF_MAXINSNS > +#define BPF_MAXINSNS 4096 > +#endif > + > +#endif /* _UAPI__LINUX_BPF_COMMON_H__ */ > -- > 2.19.1
Re: [PATCH] davinci_vpfe: add a missing break
On 11/06/18 11:15, Mauro Carvalho Chehab wrote:
> As warned by gcc:
>
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function
> 'ipipeif_hw_setup':
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this
> statement may fall through [-Wimplicit-fallthrough=]
>switch (isif_port_if) {
>^~
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
> case IPIPEIF_SDRAM_YUV:
> ^~~~
>
> There is a missing break for the raw format.
>
> Signed-off-by: Mauro Carvalho Chehab
Nacked-by: Hans Verkuil
It really should fall through: see this comment:
/* fall through for SDRAM YUV mode */
/* configure CFG2 */
val = ipipeif_read(ipipeif_base_addr, IPIPEIF_CFG2);
switch (isif_port_if) {
case MEDIA_BUS_FMT_YUYV8_1X16:
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_Y8_1X8:
RESETBIT(val, IPIPEIF_CFG2_YUV8_SHIFT);
SETBIT(val, IPIPEIF_CFG2_YUV16_SHIFT);
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
default:
RESETBIT(val, IPIPEIF_CFG2_YUV8_SHIFT);
RESETBIT(val, IPIPEIF_CFG2_YUV16_SHIFT);
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}
case IPIPEIF_SDRAM_YUV:
So we need a proper /* fall through */ comment instead of a break.
In the SDRAM_YUV case the SDRAM clock divider is configured, and that needs
to be done for both SDRAM_* cases.
Regards,
Hans
> ---
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> index a53231b08d30..975272bcf8ca 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> @@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
> ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
> break;
> }
> + break;
>
> case IPIPEIF_SDRAM_YUV:
> /* Set clock divider */
>
[PATCH] configure: build without BPF support in ir-keytable
It currently does not build on mips and some platforms do not have BPF support yet (risc-v, for example). Signed-off-by: Sean Young --- configure.ac | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 387f8539..4100db06 100644 --- a/configure.ac +++ b/configure.ac @@ -170,7 +170,14 @@ AC_SUBST([X11_CFLAGS]) AC_SUBST([X11_LIBS]) AM_CONDITIONAL([HAVE_X11], [test x$x11_pkgconfig = xyes]) -PKG_CHECK_MODULES([LIBELF], [libelf], [libelf_pkgconfig=yes], [libelf_pkgconfig=no]) +AC_ARG_WITH([bpf], +AS_HELP_STRING([--without-bpf], + [Do not build with BPF IR decoder support]), +[], +[with_bpf=yes]) + +AS_IF([test "x$with_bpf" != xno], + PKG_CHECK_MODULES([LIBELF], [libelf], [libelf_pkgconfig=yes], [libelf_pkgconfig=no]), [libelf_pkgconfig=no]) AC_SUBST([LIBELF_CFLAGS]) AC_SUBST([LIBELF_LIBS]) AM_CONDITIONAL([HAVE_LIBELF], [test x$libelf_pkgconfig = xyes]) -- 2.17.2
Re: [PATCH v3 1/1] v4l: event: Add subscription to list before calling "add" operation
On Tue, 6 Nov 2018 at 08:00, Sakari Ailus wrote:
>
> Patch ad608fbcf166 changed how events were subscribed to address an issue
> elsewhere. As a side effect of that change, the "add" callback was called
> before the event subscription was added to the list of subscribed events,
> causing the first event queued by the add callback (and possibly other
> events arriving soon afterwards) to be lost.
>
> Fix this by adding the subscription to the list before calling the "add"
> callback, and clean up afterwards if that fails.
>
> Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
> while accessed")
>
> Reported-by: Dave Stevenson
> Signed-off-by: Sakari Ailus
Tested-By: Dave Stevenson
Tested with 3 bcm2835 drivers (camera driver in staging, CSI2
receiver, and codec M2M driver) via v4l2-compliance on 4.19.0. All 3
failed in the same way prior to this patch.
> ---
> since v2:
>
> - More accurate commit message. No other changes.
>
> drivers/media/v4l2-core/v4l2-event.c | 43
>
> 1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c
> b/drivers/media/v4l2-core/v4l2-event.c
> index a3ef1f50a4b3..481e3c65cf97 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -193,6 +193,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
> }
> EXPORT_SYMBOL_GPL(v4l2_event_pending);
>
> +static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
> +{
> + struct v4l2_fh *fh = sev->fh;
> + unsigned int i;
> +
> + lockdep_assert_held(>subscribe_lock);
> + assert_spin_locked(>vdev->fh_lock);
> +
> + /* Remove any pending events for this subscription */
> + for (i = 0; i < sev->in_use; i++) {
> + list_del(>events[sev_pos(sev, i)].list);
> + fh->navailable--;
> + }
> + list_del(>list);
> +}
> +
> int v4l2_event_subscribe(struct v4l2_fh *fh,
> const struct v4l2_event_subscription *sub, unsigned
> elems,
> const struct v4l2_subscribed_event_ops *ops)
> @@ -224,27 +240,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
>
> spin_lock_irqsave(>vdev->fh_lock, flags);
> found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
> + if (!found_ev)
> + list_add(>list, >subscribed);
> spin_unlock_irqrestore(>vdev->fh_lock, flags);
>
> if (found_ev) {
> /* Already listening */
> kvfree(sev);
> - goto out_unlock;
> - }
> -
> - if (sev->ops && sev->ops->add) {
> + } else if (sev->ops && sev->ops->add) {
> ret = sev->ops->add(sev, elems);
> if (ret) {
> + spin_lock_irqsave(>vdev->fh_lock, flags);
> + __v4l2_event_unsubscribe(sev);
> + spin_unlock_irqrestore(>vdev->fh_lock, flags);
> kvfree(sev);
> - goto out_unlock;
> }
> }
>
> - spin_lock_irqsave(>vdev->fh_lock, flags);
> - list_add(>list, >subscribed);
> - spin_unlock_irqrestore(>vdev->fh_lock, flags);
> -
> -out_unlock:
> mutex_unlock(>subscribe_lock);
>
> return ret;
> @@ -279,7 +291,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
> {
> struct v4l2_subscribed_event *sev;
> unsigned long flags;
> - int i;
>
> if (sub->type == V4L2_EVENT_ALL) {
> v4l2_event_unsubscribe_all(fh);
> @@ -291,14 +302,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
> spin_lock_irqsave(>vdev->fh_lock, flags);
>
> sev = v4l2_event_subscribed(fh, sub->type, sub->id);
> - if (sev != NULL) {
> - /* Remove any pending events for this subscription */
> - for (i = 0; i < sev->in_use; i++) {
> - list_del(>events[sev_pos(sev, i)].list);
> - fh->navailable--;
> - }
> - list_del(>list);
> - }
> + if (sev != NULL)
> + __v4l2_event_unsubscribe(sev);
>
> spin_unlock_irqrestore(>vdev->fh_lock, flags);
>
> --
> 2.11.0
>
[PATCH] davinci_vpfe: add a missing break
As warned by gcc:
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c: In function
'ipipeif_hw_setup':
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:298:3: warning: this
statement may fall through [-Wimplicit-fallthrough=]
switch (isif_port_if) {
^~
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:314:2: note: here
case IPIPEIF_SDRAM_YUV:
^~~~
There is a missing break for the raw format.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
index a53231b08d30..975272bcf8ca 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
@@ -310,6 +310,7 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
ipipeif_write(val, ipipeif_base_addr, IPIPEIF_CFG2);
break;
}
+ break;
case IPIPEIF_SDRAM_YUV:
/* Set clock divider */
--
2.19.1
[PATCH v3 2/3] media: imx-pxp: Check for pxp_soft_reset() error
pxp_soft_reset() may fail with a timeout, so it is better to propagate
the error in this case.
Signed-off-by: Fabio Estevam
Reviewed-by: Philipp Zabel
---
Changes since v2:
- Jump to err_clck when pxp_soft_reset() fails. (Philipp)
drivers/media/platform/imx-pxp.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 27780f1..986764d 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
.job_abort = pxp_job_abort,
};
-static void pxp_soft_reset(struct pxp_dev *dev)
+static int pxp_soft_reset(struct pxp_dev *dev)
{
int ret;
u32 val;
@@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev)
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
val & BM_PXP_CTRL_CLKGATE, 0, 100);
- if (ret < 0)
+ if (ret < 0) {
pr_err("PXP reset timeout\n");
+ return ret;
+ }
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
+
+ return 0;
}
static int pxp_probe(struct platform_device *pdev)
@@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- pxp_soft_reset(dev);
+ ret = pxp_soft_reset(dev);
+ if (ret < 0)
+ goto err_clk;
spin_lock_init(>irqlock);
--
2.7.4
[PATCH v3 1/3] media: imx-pxp: Check the return value from clk_prepare_enable()
clk_prepare_enable() may fail, so we should better check its return value and propagate it in the case of error. Signed-off-by: Fabio Estevam Reviewed-by: Philipp Zabel --- Changes since v2: - None drivers/media/platform/imx-pxp.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c index b76cd0e..27780f1 100644 --- a/drivers/media/platform/imx-pxp.c +++ b/drivers/media/platform/imx-pxp.c @@ -1666,7 +1666,10 @@ static int pxp_probe(struct platform_device *pdev) return ret; } - clk_prepare_enable(dev->clk); + ret = clk_prepare_enable(dev->clk); + if (ret < 0) + return ret; + pxp_soft_reset(dev); spin_lock_init(>irqlock); -- 2.7.4
[PATCH v3 3/3] media: imx-pxp: Improve pxp_soft_reset() error message
Improve the pxp_soft_reset() error message by moving it to the
caller function, associating it with a proper device and also
by displaying the error code.
Signed-off-by: Fabio Estevam
Reviewed-by: Philipp Zabel
---
Changes since v2:
- None (only rebased against the change made in 2/3)
drivers/media/platform/imx-pxp.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
index 986764d..b80d206 100644
--- a/drivers/media/platform/imx-pxp.c
+++ b/drivers/media/platform/imx-pxp.c
@@ -1619,10 +1619,8 @@ static int pxp_soft_reset(struct pxp_dev *dev)
ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
val & BM_PXP_CTRL_CLKGATE, 0, 100);
- if (ret < 0) {
- pr_err("PXP reset timeout\n");
+ if (ret < 0)
return ret;
- }
writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
@@ -1675,8 +1673,10 @@ static int pxp_probe(struct platform_device *pdev)
return ret;
ret = pxp_soft_reset(dev);
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(>dev, "PXP reset timeout: %d\n", ret);
goto err_clk;
+ }
spin_lock_init(>irqlock);
--
2.7.4
Re: [PATCH v2 3/3] media: imx-pxp: Improve pxp_soft_reset() error message
On Mon, 2018-11-05 at 18:45 -0200, Fabio Estevam wrote:
> Improve the pxp_soft_reset() error message by moving it to the
> caller function, associating it with a proper device and also
> by displaying the error code.
>
> Signed-off-by: Fabio Estevam
> ---
> Changes since v1:
> - Newly introduced in this version
>
> drivers/media/platform/imx-pxp.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/imx-pxp.c
> b/drivers/media/platform/imx-pxp.c
> index b3700b8..1b765c9 100644
> --- a/drivers/media/platform/imx-pxp.c
> +++ b/drivers/media/platform/imx-pxp.c
> @@ -1619,10 +1619,8 @@ static int pxp_soft_reset(struct pxp_dev *dev)
>
> ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
>val & BM_PXP_CTRL_CLKGATE, 0, 100);
> - if (ret < 0) {
> - pr_err("PXP reset timeout\n");
> + if (ret < 0)
> return ret;
> - }
>
> writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
> writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
> @@ -1675,8 +1673,10 @@ static int pxp_probe(struct platform_device *pdev)
> return ret;
>
> ret = pxp_soft_reset(dev);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err(>dev, "PXP reset timeout: %d\n", ret);
> return ret;
> + }
>
> spin_lock_init(>irqlock);
This should be rebased onto the fixed 2/2 or squashed into it,
but otherwise
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v2 2/3] media: imx-pxp: Check for pxp_soft_reset() error
On Mon, 2018-11-05 at 18:45 -0200, Fabio Estevam wrote:
> pxp_soft_reset() may fail with a timeout, so it is better to propagate
> the error in this case.
>
> Signed-off-by: Fabio Estevam
> ---
> Changes since v1:
> - None
>
> drivers/media/platform/imx-pxp.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/imx-pxp.c
> b/drivers/media/platform/imx-pxp.c
> index 27780f1..b3700b8 100644
> --- a/drivers/media/platform/imx-pxp.c
> +++ b/drivers/media/platform/imx-pxp.c
> @@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
> .job_abort = pxp_job_abort,
> };
>
> -static void pxp_soft_reset(struct pxp_dev *dev)
> +static int pxp_soft_reset(struct pxp_dev *dev)
> {
> int ret;
> u32 val;
> @@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev)
>
> ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val,
>val & BM_PXP_CTRL_CLKGATE, 0, 100);
> - if (ret < 0)
> + if (ret < 0) {
> pr_err("PXP reset timeout\n");
> + return ret;
> + }
>
> writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR);
I'm not sure if we should clear SFTRST again after a timeout. It
probably doesn't matter as something went wrong anyway and the next
probe will try to clear it again.
> writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR);
Clearing CLKGATE if it was not set by the SFTRST in time should have no
effect, so we could do this unconditionally as well.
> +
> + return 0;
So you could just "return ret;" here instead of breaking out above.
I have no preference either way.
> }
>
> static int pxp_probe(struct platform_device *pdev)
> @@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - pxp_soft_reset(dev);
> + ret = pxp_soft_reset(dev);
> + if (ret < 0)
> + return ret;
This should "goto err_clk;" instead, though. With that changed,
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v2 1/3] media: imx-pxp: Check the return value from clk_prepare_enable()
Hi Fabio, thank you for the fixes! On Mon, 2018-11-05 at 18:45 -0200, Fabio Estevam wrote: > clk_prepare_enable() may fail, so we should better check its return value > and propagate it in the case of error. > > Signed-off-by: Fabio Estevam > --- > Changes since v1: > - Properly enumerate the series > > drivers/media/platform/imx-pxp.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/imx-pxp.c > b/drivers/media/platform/imx-pxp.c > index b76cd0e..27780f1 100644 > --- a/drivers/media/platform/imx-pxp.c > +++ b/drivers/media/platform/imx-pxp.c > @@ -1666,7 +1666,10 @@ static int pxp_probe(struct platform_device *pdev) > return ret; > } > > - clk_prepare_enable(dev->clk); > + ret = clk_prepare_enable(dev->clk); > + if (ret < 0) > + return ret; > + > pxp_soft_reset(dev); > > spin_lock_init(>irqlock); Reviewed-by: Philipp Zabel regards Philipp
[RFC] Create test script(s?) for regression testing
Hi all, After the media summit (heavy on test discussions) and the V4L2 event regression we just found it is clear we need to do a better job with testing. All the pieces are in place, so what is needed is to combine it and create a script that anyone of us as core developers can run to check for regressions. The same script can be run as part of the kernelci regression testing. We have four virtual drivers: vivid, vim2m, vimc and vicodec. The last one is IMHO not quite good enough yet for testing: it is not fully compliant to the upcoming stateful codec spec. Work for that is planned as part of an Outreachy project. My idea is to create a script that is maintained as part of v4l-utils that loads the drivers and runs v4l2-compliance and possibly other tests against the virtual drivers. It should be simple to use and require very little in the way of dependencies. Ideally no dependencies other than what is in v4l-utils so it can easily be run on an embedded system as well. For a 64-bit kernel it should run the tests both with 32-bit and 64-bit applications. It should also test with both single and multiplanar modes where available. Since vivid emulates CEC as well, it should run CEC tests too. As core developers we should have an environment where we can easily test our patches with this script (I use a VM for that). I think maintaining the script (or perhaps scripts) in v4l-utils is best since that keeps it in sync with the latest kernel and v4l-utils developments. Comments? Ideas? Regards, Hans
Re: [PATCH v3 1/1] v4l: event: Add subscription to list before calling "add" operation
On 11/06/2018 09:00 AM, Sakari Ailus wrote:
> Patch ad608fbcf166 changed how events were subscribed to address an issue
> elsewhere. As a side effect of that change, the "add" callback was called
> before the event subscription was added to the list of subscribed events,
> causing the first event queued by the add callback (and possibly other
> events arriving soon afterwards) to be lost.
>
> Fix this by adding the subscription to the list before calling the "add"
> callback, and clean up afterwards if that fails.
>
> Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
> while accessed")
>
> Reported-by: Dave Stevenson
> Signed-off-by: Sakari Ailus
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Tested with vivid & v4l2-compliance.
Regards,
Hans
> ---
> since v2:
>
> - More accurate commit message. No other changes.
>
> drivers/media/v4l2-core/v4l2-event.c | 43
>
> 1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c
> b/drivers/media/v4l2-core/v4l2-event.c
> index a3ef1f50a4b3..481e3c65cf97 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -193,6 +193,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
> }
> EXPORT_SYMBOL_GPL(v4l2_event_pending);
>
> +static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
> +{
> + struct v4l2_fh *fh = sev->fh;
> + unsigned int i;
> +
> + lockdep_assert_held(>subscribe_lock);
> + assert_spin_locked(>vdev->fh_lock);
> +
> + /* Remove any pending events for this subscription */
> + for (i = 0; i < sev->in_use; i++) {
> + list_del(>events[sev_pos(sev, i)].list);
> + fh->navailable--;
> + }
> + list_del(>list);
> +}
> +
> int v4l2_event_subscribe(struct v4l2_fh *fh,
>const struct v4l2_event_subscription *sub, unsigned
> elems,
>const struct v4l2_subscribed_event_ops *ops)
> @@ -224,27 +240,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
>
> spin_lock_irqsave(>vdev->fh_lock, flags);
> found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
> + if (!found_ev)
> + list_add(>list, >subscribed);
> spin_unlock_irqrestore(>vdev->fh_lock, flags);
>
> if (found_ev) {
> /* Already listening */
> kvfree(sev);
> - goto out_unlock;
> - }
> -
> - if (sev->ops && sev->ops->add) {
> + } else if (sev->ops && sev->ops->add) {
> ret = sev->ops->add(sev, elems);
> if (ret) {
> + spin_lock_irqsave(>vdev->fh_lock, flags);
> + __v4l2_event_unsubscribe(sev);
> + spin_unlock_irqrestore(>vdev->fh_lock, flags);
> kvfree(sev);
> - goto out_unlock;
> }
> }
>
> - spin_lock_irqsave(>vdev->fh_lock, flags);
> - list_add(>list, >subscribed);
> - spin_unlock_irqrestore(>vdev->fh_lock, flags);
> -
> -out_unlock:
> mutex_unlock(>subscribe_lock);
>
> return ret;
> @@ -279,7 +291,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
> {
> struct v4l2_subscribed_event *sev;
> unsigned long flags;
> - int i;
>
> if (sub->type == V4L2_EVENT_ALL) {
> v4l2_event_unsubscribe_all(fh);
> @@ -291,14 +302,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
> spin_lock_irqsave(>vdev->fh_lock, flags);
>
> sev = v4l2_event_subscribed(fh, sub->type, sub->id);
> - if (sev != NULL) {
> - /* Remove any pending events for this subscription */
> - for (i = 0; i < sev->in_use; i++) {
> - list_del(>events[sev_pos(sev, i)].list);
> - fh->navailable--;
> - }
> - list_del(>list);
> - }
> + if (sev != NULL)
> + __v4l2_event_unsubscribe(sev);
>
> spin_unlock_irqrestore(>vdev->fh_lock, flags);
>
>
Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Raj,
On Mon, Nov 05, 2018 at 07:05:53PM +, Mani, Rajmohan wrote:
> Hi Sakari,
>
> > -Original Message-
> > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> > Sent: Monday, November 05, 2018 12:28 AM
> > To: Zhi, Yong
> > Cc: linux-media@vger.kernel.org; tf...@chromium.org; mche...@kernel.org;
> > hans.verk...@cisco.com; laurent.pinch...@ideasonboard.com; Mani,
> > Rajmohan ; Zheng, Jian Xu
> > ; Hu, Jerry W ; Toivonen,
> > Tuukka ; Qiu, Tian Shu
> > ; Cao, Bingbu
> > Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> >
> > Hi Yong,
> >
> > On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > > This add all the structs of IPU3 firmware ABI.
> > >
> > > Signed-off-by: Yong Zhi
> > > Signed-off-by: Rajmohan Mani
> >
> > ...
> >
> > > +struct imgu_abi_shd_intra_frame_operations_data {
> > > + struct imgu_abi_acc_operation
> > > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> > __attribute__((aligned(32)));
> > > + struct imgu_abi_acc_process_lines_cmd_data
> > > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> > __attribute__((aligned(32)));
> > > + struct imgu_abi_shd_transfer_luts_set_data
> > > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > > +__attribute__((aligned(32)));
> >
> > Could you replace this wth __aligned(32), please? The same for the rest of
> > the
> > header.
> >
>
> Using __aligned(32) in the uAPI header resulted in compilation errors in
> user space / camera HAL code.
>
> e.g
> ../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error:
> expected ';'
> at end of declaration list
> __u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE] __aligned(32);
>
> So we ended up using __attribute__((aligned(32))) format in uAPI header and
> to be consistent, we followed the same format in ABI header as well.
>
> Let us know if it's okay to deviate between uAPI and ABI header for this
> alignment qualifier.
There's a reason for using __attribute__((aligned(32))) in the uAPI header,
but not in the in-kernel headers where __aligned(32) is preferred.
I have a patch for addressing this for the uAPI headers as well so
__aligned(32) could be used there, too; I'll submit it soon. Let's see...
there are kerneldoc issues still in this area.
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
[PATCH v3 1/1] v4l: event: Add subscription to list before calling "add" operation
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
---
since v2:
- More accurate commit message. No other changes.
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index a3ef1f50a4b3..481e3c65cf97 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -193,6 +193,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(>subscribe_lock);
+ assert_spin_locked(>vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(>events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(>list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -224,27 +240,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(>list, >subscribed);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kvfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(>vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(>vdev->fh_lock, flags);
kvfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(>vdev->fh_lock, flags);
- list_add(>list, >subscribed);
- spin_unlock_irqrestore(>vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(>subscribe_lock);
return ret;
@@ -279,7 +291,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -291,14 +302,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(>events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(>list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
--
2.11.0
