cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sat Mar 11 05:00:20 CET 2017
media-tree git hash:700ea5e0e0dd70420a04e703ff264cc133834cba
media_build git hash:   bc4c2a205c087c8deff3cd14ed663c4767dd2016
v4l-utils git hash: ca6a0c099399cc51ecfacff7ef938be6ef73b8b3
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-i686: OK
linux-4.10.1-i686: OK
linux-4.11-rc1-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9-x86_64: WARNINGS
linux-4.10.1-x86_64: WARNINGS
linux-4.11-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

[PATCH V2] Staging: bcm2835: Fixed style of block comments

[Derek Robson]

Fixed style of block comments across whole driver
Found using checkpatch

Signed-off-by: Derek Robson 
---
Version #1 had ugly long subject name.

 .../media/platform/bcm2835/bcm2835-camera.c| 24 ++
 drivers/staging/media/platform/bcm2835/controls.c  | 22 +++-
 .../staging/media/platform/bcm2835/mmal-msg-port.h |  6 --
 drivers/staging/media/platform/bcm2835/mmal-msg.h  | 18 
 .../staging/media/platform/bcm2835/mmal-vchiq.c|  3 ++-
 .../staging/media/platform/bcm2835/mmal-vchiq.h|  4 ++--
 6 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index ca15a698e018..25beca62a8a9 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -239,8 +239,9 @@ static struct mmal_fmt *get_format(struct v4l2_format *f)
 }
 
 /* --
-   Videobuf queue operations
-   --*/
+ * Videobuf queue operations
+ * --
+ */
 
 static int queue_setup(struct vb2_queue *vq,
   unsigned int *nbuffers, unsigned int *nplanes,
@@ -668,8 +669,9 @@ static struct vb2_ops bm2835_mmal_video_qops = {
 };
 
 /* --
-   IOCTL operations
-   --*/
+ * IOCTL operations
+ * --
+ */
 
 static int set_overlay_params(struct bm2835_mmal_dev *dev,
  struct vchiq_mmal_port *port)
@@ -834,7 +836,8 @@ static int vidioc_g_fbuf(struct file *file, void *fh,
 struct v4l2_framebuffer *a)
 {
/* The video overlay must stay within the framebuffer and can't be
-  positioned independently. */
+* positioned independently.
+*/
struct bm2835_mmal_dev *dev = video_drvdata(file);
struct vchiq_mmal_port *preview_port =
>component[MMAL_COMPONENT_CAMERA]->
@@ -1291,7 +1294,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
*priv,
}
 
/* If the format is unsupported v4l2 says we should switch to
-* a supported one and not return an error. */
+* a supported one and not return an error.
+*/
mfmt = get_format(f);
if (!mfmt) {
v4l2_dbg(1, bcm2835_v4l2_debug, >v4l2_dev,
@@ -1485,7 +1489,8 @@ static const struct v4l2_ioctl_ops 
camera0_ioctl_ops_gstreamer = {
.vidioc_qbuf = vb2_ioctl_qbuf,
.vidioc_dqbuf = vb2_ioctl_dqbuf,
/* Remove this function ptr to fix gstreamer bug
-   .vidioc_enum_framesizes = vidioc_enum_framesizes, */
+* .vidioc_enum_framesizes = vidioc_enum_framesizes,
+*/
.vidioc_enum_frameintervals = vidioc_enum_frameintervals,
.vidioc_g_parm= vidioc_g_parm,
.vidioc_s_parm= vidioc_s_parm,
@@ -1498,8 +1503,9 @@ static const struct v4l2_ioctl_ops 
camera0_ioctl_ops_gstreamer = {
 };
 
 /* --
-   Driver init/finalise
-   --*/
+ * Driver init/finalise
+ * --
+ */
 
 static const struct v4l2_file_operations camera0_fops = {
.owner = THIS_MODULE,
diff --git a/drivers/staging/media/platform/bcm2835/controls.c 
b/drivers/staging/media/platform/bcm2835/controls.c
index a40987b2e75d..16fa40c904e7 100644
--- a/drivers/staging/media/platform/bcm2835/controls.c
+++ b/drivers/staging/media/platform/bcm2835/controls.c
@@ -90,7 +90,8 @@ struct bm2835_mmal_v4l2_ctrl {
u32 id; /* v4l2 control identifier */
enum bm2835_mmal_ctrl_type type;
/* control minimum value or
-* mask for MMAL_CONTROL_TYPE_STD_MENU */
+* mask for MMAL_CONTROL_TYPE_STD_MENU
+*/
s32 min;
s32 max; /* maximum value of control */
s32 def;  /* default value of control */
@@ -398,10 +399,10 @@ static int ctrl_set_metering_mode(struct bm2835_mmal_dev 
*dev,
break;
 
/* todo matrix weighting not added to Linux API till 3.9
-   case V4L2_EXPOSURE_METERING_MATRIX:
-   dev->metering_mode = MMAL_PARAM_EXPOSUREMETERINGMODE_MATRIX;
-   break;
-   */
+* case V4L2_EXPOSURE_METERING_MATRIX:
+*  dev->metering_mode = MMAL_PARAM_EXPOSUREMETERINGMODE_MATRIX;
+*  break;
+*/
}
 
if (dev->scene_mode == V4L2_SCENE_MODE_NONE) {
@@ -982,8 +983,9 @@ static const struct 

Re: [PATCH] libv4lconvert: by default, offer the original format to the client

[Troy Kisky]

On 3/10/2017 12:53 PM, Mauro Carvalho Chehab wrote:
> The libv4lconvert part of libv4l was meant to provide a common
> place to handle weird proprietary formats. With time, we also
> added support to other standard formats, in order to help
> V4L2 applications that are not performance sensitive to support
> all V4L2 formats.
> 
> Yet, the hole idea is to let userspace to decide to implement
> their own format conversion code when it needs either more
> performance or more quality than what libv4lconvert provides.
> 
> In other words, applications should have the right to decide
> between using a libv4lconvert emulated format or to implement
> the decoding themselves for non-proprietary formats,
> as this may have significative performance impact.
> 
> At the application side, deciding between them is just a matter
> of looking at the V4L2_FMT_FLAG_EMULATED flag.
> 
> Yet, we don't want to have a miriad of format converters
> everywhere for the proprietary formats, like V4L2_PIX_FMT_KONICA420,
> V4L2_PIX_FMT_SPCA501, etc. So, let's offer only the emulated
> variant for those weird stuff.
> 
> So, this patch changes the libv4lconvert behavior, for
> all non-proprietary formats, including Bayer, to offer both
> the original format and the emulated ones.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  lib/libv4lconvert/libv4lconvert.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libv4lconvert/libv4lconvert.c 
> b/lib/libv4lconvert/libv4lconvert.c
> index da718918b030..d87d6b91a838 100644
> --- a/lib/libv4lconvert/libv4lconvert.c
> +++ b/lib/libv4lconvert/libv4lconvert.c
> @@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt 
> supported_src_pixfmts[] = {
>   { V4L2_PIX_FMT_OV511,0,  7,  7, 1 },
>   { V4L2_PIX_FMT_OV518,0,  7,  7, 1 },
>   /* uncompressed bayer */
> - { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 1 },
> + { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 0 },
>   { V4L2_PIX_FMT_STV0680,  8,  8,  8, 1 },
>   /* compressed bayer */
>   { V4L2_PIX_FMT_SPCA561,  0,  9,  9, 1 },
> @@ -178,7 +178,7 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   /* This keeps tracks of devices which have only formats for which apps
>  most likely will need conversion and we can thus safely add software
>  processing controls without a performance impact. */


The bottom part seems unwanted.
Before, if any source format did not need conversion then 
always_needs_conversion = 0
else always_needs_conversion = 1

Now
if any source format needs conversion then always_needs_conversion = 1
else always_needs_conversion = 0


Seems like it should be 2 separate patches even if I'm wrong.



> - int always_needs_conversion = 1;
> + int always_needs_conversion = 0;
>  
>   if (!data) {
>   fprintf(stderr, "libv4lconvert: error: out of memory!\n");
> @@ -208,10 +208,9 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   if (j < ARRAY_SIZE(supported_src_pixfmts)) {
>   data->supported_src_formats |= 1ULL << j;
>   v4lconvert_get_framesizes(data, fmt.pixelformat, j);
> - if (!supported_src_pixfmts[j].needs_conversion)
> - always_needs_conversion = 0;
> - } else
> - always_needs_conversion = 0;
> + if (supported_src_pixfmts[j].needs_conversion)
> + always_needs_conversion = 1;
> + }
>   }
>  
>   data->no_formats = i;
> 

Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

[Steve Longerbeam]

On 03/10/2017 03:30 PM, Pavel Machek wrote:

On Fri 2017-03-10 10:37:21, Steve Longerbeam wrote:

Hi Hans,

On 03/10/2017 04:03 AM, Hans Verkuil wrote:

On 10/03/17 05:52, Steve Longerbeam wrote:

Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
output device has measured an interval between the reception or transmit
completion of two consecutive frames of video that is outside the nominal
frame interval by some tolerance value.


Reading back what was said on this I agree with Sakari that this doesn't
belong here.

Userspace can detect this just as easily (if not easier) with a timeout.




Unfortunately measuring frame intervals from userland is not accurate
enough for i.MX6.

The issue here is that the IPUv3, specifically the CSI unit, can
permanently lose vertical sync if there are truncated frames sent
on the bt.656 bus. We have seen a single missing line of video cause
loss of vertical sync. The only way to correct this is to shutdown
the IPU capture hardware and restart, which can be accomplished
simply by restarting streaming from userland.

There are no other indicators from the sensor about these short
frame events (believe me, we've exhausted all avenues with the ADV718x).
And the IPUv3 DMA engine has no status indicators for short frames
either. So the only way to detect them is by measuring frame intervals.

The intervals have to be able to resolve a single line of missing video.
With a PAL video source that requires better than 58 usec accuracy.

There is too much uncertainty to resolve this at user level. The
driver is able to resolve this by measuring intervals between hardware
interrupts as long as interrupt latency is reasonably low, and we
have another method using the i.MX6 hardware input capture support
that can measure these intervals very accurately with no errors
introduced by interrupt latency.


Requiring < 58 usec interrupt latency for correct operation is a
little too optimistic, no?



No it's not too optimistic, from experience the imx6 kernel has irq
latency less than 10 usec under normal system load. False events can be
generated if the latency gets bad, it's true, and that's why there is
the imx6 timer input capture approach.

Steve

Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

[Pavel Machek]

On Fri 2017-03-10 10:37:21, Steve Longerbeam wrote:
> Hi Hans,
> 
> On 03/10/2017 04:03 AM, Hans Verkuil wrote:
> >On 10/03/17 05:52, Steve Longerbeam wrote:
> >>Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
> >>output device has measured an interval between the reception or transmit
> >>completion of two consecutive frames of video that is outside the nominal
> >>frame interval by some tolerance value.
> >
> >Reading back what was said on this I agree with Sakari that this doesn't
> >belong here.
> >
> >Userspace can detect this just as easily (if not easier) with a timeout.
> >
> 
> 
> Unfortunately measuring frame intervals from userland is not accurate
> enough for i.MX6.
> 
> The issue here is that the IPUv3, specifically the CSI unit, can
> permanently lose vertical sync if there are truncated frames sent
> on the bt.656 bus. We have seen a single missing line of video cause
> loss of vertical sync. The only way to correct this is to shutdown
> the IPU capture hardware and restart, which can be accomplished
> simply by restarting streaming from userland.
> 
> There are no other indicators from the sensor about these short
> frame events (believe me, we've exhausted all avenues with the ADV718x).
> And the IPUv3 DMA engine has no status indicators for short frames
> either. So the only way to detect them is by measuring frame intervals.
> 
> The intervals have to be able to resolve a single line of missing video.
> With a PAL video source that requires better than 58 usec accuracy.
> 
> There is too much uncertainty to resolve this at user level. The
> driver is able to resolve this by measuring intervals between hardware
> interrupts as long as interrupt latency is reasonably low, and we
> have another method using the i.MX6 hardware input capture support
> that can measure these intervals very accurately with no errors
> introduced by interrupt latency.

Requiring < 58 usec interrupt latency for correct operation is a
little too optimistic, no?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v5 22/39] media: Add userspace header file for i.MX

[Pavel Machek]

Hi!

> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> index aafaa5a..fa78958 100644
> --- a/include/uapi/media/Kbuild
> +++ b/include/uapi/media/Kbuild
> @@ -1 +1,2 @@
>  # UAPI Header export list
> +header-y += imx.h
> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> new file mode 100644
> index 000..f573de4
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version
> + */
> +
> +#ifndef __UAPI_MEDIA_IMX_H__
> +#define __UAPI_MEDIA_IMX_H__
> +
> +enum imx_ctrl_id {
> + V4L2_CID_IMX_FIM_ENABLE = (V4L2_CID_USER_IMX_BASE + 0),
> + V4L2_CID_IMX_FIM_NUM,
> + V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> + V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> + V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +

Should this #include something so that if userland includes it, it
will not get compile error?

Should there be some documentation of userland API?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v5 00/39] i.MX Media Driver

[Steve Longerbeam]

On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:

Version 5 gives me no v4l2 controls exposed through the video device
interface.

Just like with version 4, version 5 is completely useless with IMX219:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110

So, like v4, I can't do any further testing.



Is the imx219 placing the csi-2 bus in LP-11 state on exit
from s_power(ON)?

I realize that probably means bringing the chip up to a
completely operational state and then setting it to stream
OFF in the s_power() op.

The same had to be done for the OV5640.

Steve

Re: [PATCH] v4l2-fwnode: Fix clock lane parsing

[Sakari Ailus]

On Mon, Mar 06, 2017 at 08:23:24AM +0100, Pavel Machek wrote:
> Fix clock lane parsing in v4l2-fwnode.
> 
> Signed-off-by: Pavel Machek 
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index dd3..44036b8 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -167,7 +167,7 @@ void v4l2_fwnode_endpoint_parse_csi1_bus(struct 
> fwnode_handle *fwn,
> bus->data_lane = v;
>  
> if (!fwnode_property_read_u32(fwn, "clock-lanes", ))
> -   bus->data_lane = v;
> +   bus->clock_lane = v;

Oh my. Did I really write it like that?

I'll merge your fix next Monday. Thanks!

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Sakari Ailus]

Hi Mauro (and others),

On Fri, Mar 10, 2017 at 12:53:42PM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 10 Mar 2017 15:20:48 +0100
> Hans Verkuil  escreveu:
> 
> > 
> > > As I've already mentioned, from talking about this with Mauro, it seems
> > > Mauro is in agreement with permitting the control inheritence... I wish
> > > Mauro would comment for himself, as I can't quote our private discussion
> > > on the subject.  
> > 
> > I can't comment either, not having seen his mail and reasoning.
> 
> The rationale is that we should support the simplest use cases first.
> 
> In the case of the first MC-based driver (and several subsequent
> ones), the simplest use case required MC, as it was meant to suport
> a custom-made sophisticated application that required fine control
> on each component of the pipeline and to allow their advanced
> proprietary AAA userspace-based algorithms to work.

The first MC based driver (omap3isp) supports what the hardware can do, it
does not support applications as such.

Adding support to drivers for different "operation modes" --- this is
essentially what is being asked for --- is not an approach which could serve
either purpose (some functionality with simple interface vs. fully support
what the hardware can do, with interfaces allowing that) adequately in the
short or the long run.

If we are missing pieces in the puzzle --- in this case the missing pieces
in the puzzle are a generic pipeline configuration library and another
library that, with the help of pipeline autoconfiguration would implement
"best effort" service for regular V4L2 on top of the MC + V4L2 subdev + V4L2
--- then these pieces need to be impelemented. The solution is
*not* to attempt to support different types of applications in each driver
separately. That will make writing drivers painful, error prone and is
unlikely ever deliver what either purpose requires.

So let's continue to implement the functionality that the hardware supports.
Making a different choice here is bound to create a lasting conflict between
having to change kernel interface behaviour and the requirement of
supporting new functionality that hasn't been previously thought of, pushing
away SoC vendors from V4L2 ecosystem. This is what we all do want to avoid.

As far as i.MX6 driver goes, it is always possible to implement i.MX6 plugin
for libv4l to perform this. This should be much easier than getting the
automatic pipe configuration library and the rest working, and as it is
custom for i.MX6, the resulting plugin may make informed technical choices
for better functionality. Jacek has been working on such a plugin for
Samsung Exynos hardware, but I don't think he has quite finished it yey.

The original plan was and continues to be sound, it's just that there have
always been too few hands to implement it. :-(

> 
> That's not true, for example, for the UVC driver. There, MC
> is optional, as it should be.

UVC is different. The device simply provides additional information through
MC to the user but MC (or V4L2 sub-device interface) is not used for
controlling the device.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Fabio Estevam]

On Fri, Mar 10, 2017 at 6:57 PM, Pavel Machek  wrote:

> And it should not depend on configuration. Hardware vendor should be
> able to ship board with working device tree...

We are talking about pin conflict here. Please read the commit log of
this patch for details.

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Pavel Machek]

On Fri 2017-03-10 16:17:28, Fabio Estevam wrote:
> On Fri, Mar 10, 2017 at 3:59 PM, Troy Kisky
>  wrote:
> > On 3/9/2017 8:52 PM, Steve Longerbeam wrote:
> >> There is a pin conflict with GPIO_6. This pin functions as a power
> >> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
> >> workaround for erratum ERR006687, to wake-up the ARM cores on normal
> >> RX and TX packet done events. So we need to remove the h/w workaround
> >> to support the OV5642. The result is that the CPUidle driver will no
> >> longer allow entering the deep idle states on the sabrelite.
> >>
> >> This is a partial revert of
> >>
> >> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
> >>   interrupt.")
> >> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW 
> >> workaround
> >>   for ERR006687")
> >>
> >> Signed-off-by: Steve Longerbeam 
> >> ---
> >>  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 4 
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
> >> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> >> index 8413179..89dce27 100644
> >> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> >> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> >> @@ -270,9 +270,6 @@
> >>   txd1-skew-ps = <0>;
> >>   txd2-skew-ps = <0>;
> >>   txd3-skew-ps = <0>;
> >
> > How about
> >
> > +#if !IS_ENABLED(CONFIG_VIDEO_OV5642)

dts is supposed to be hardware description.

> Or maybe just create a new device tree for using the camera, like
> imx6q-sabrelite-camera.dts.

And it should not depend on configuration. Hardware vendor should be
able to ship board with working device tree...

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] libv4lconvert: by default, offer the original format to the client

[Sakari Ailus]

Hi Mauro,

On Fri, Mar 10, 2017 at 05:53:26PM -0300, Mauro Carvalho Chehab wrote:
> The libv4lconvert part of libv4l was meant to provide a common
> place to handle weird proprietary formats. With time, we also
> added support to other standard formats, in order to help
> V4L2 applications that are not performance sensitive to support
> all V4L2 formats.
> 
> Yet, the hole idea is to let userspace to decide to implement
> their own format conversion code when it needs either more
> performance or more quality than what libv4lconvert provides.
> 
> In other words, applications should have the right to decide
> between using a libv4lconvert emulated format or to implement
> the decoding themselves for non-proprietary formats,
> as this may have significative performance impact.
> 
> At the application side, deciding between them is just a matter
> of looking at the V4L2_FMT_FLAG_EMULATED flag.
> 
> Yet, we don't want to have a miriad of format converters

   ^ myriad

> everywhere for the proprietary formats, like V4L2_PIX_FMT_KONICA420,
> V4L2_PIX_FMT_SPCA501, etc. So, let's offer only the emulated
> variant for those weird stuff.
> 
> So, this patch changes the libv4lconvert behavior, for
> all non-proprietary formats, including Bayer, to offer both
> the original format and the emulated ones.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  lib/libv4lconvert/libv4lconvert.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libv4lconvert/libv4lconvert.c 
> b/lib/libv4lconvert/libv4lconvert.c
> index da718918b030..d87d6b91a838 100644
> --- a/lib/libv4lconvert/libv4lconvert.c
> +++ b/lib/libv4lconvert/libv4lconvert.c
> @@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt 
> supported_src_pixfmts[] = {
>   { V4L2_PIX_FMT_OV511,0,  7,  7, 1 },
>   { V4L2_PIX_FMT_OV518,0,  7,  7, 1 },
>   /* uncompressed bayer */
> - { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 1 },
> - { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 1 },
> + { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 0 },
> + { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 0 },
>   { V4L2_PIX_FMT_STV0680,  8,  8,  8, 1 },
>   /* compressed bayer */
>   { V4L2_PIX_FMT_SPCA561,  0,  9,  9, 1 },
> @@ -178,7 +178,7 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   /* This keeps tracks of devices which have only formats for which apps
>  most likely will need conversion and we can thus safely add software
>  processing controls without a performance impact. */

Does the comment require updating?

With these,

Acked-by: Sakari Ailus 

> - int always_needs_conversion = 1;
> + int always_needs_conversion = 0;
>  
>   if (!data) {
>   fprintf(stderr, "libv4lconvert: error: out of memory!\n");
> @@ -208,10 +208,9 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   if (j < ARRAY_SIZE(supported_src_pixfmts)) {
>   data->supported_src_formats |= 1ULL << j;
>   v4lconvert_get_framesizes(data, fmt.pixelformat, j);
> - if (!supported_src_pixfmts[j].needs_conversion)
> - always_needs_conversion = 0;
> - } else
> - always_needs_conversion = 0;
> + if (supported_src_pixfmts[j].needs_conversion)
> + always_needs_conversion = 1;
> + }
>   }
>  
>   data->no_formats = i;

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Pavel Machek]

Hi!

> Argh! that is indeed a bug at libv4l (and maybe at gstreamer).
> 
> I guess that the always_needs_conversion logic was meant to be used to
> really odd proprietary formats, e. g:
>   
> /*  Vendor-specific formats   */
> #define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> #define V4L2_PIX_FMT_WNVA v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw 
> compress */
> #define V4L2_PIX_FMT_SN9C10X  v4l2_fourcc('S', '9', '1', '0') /* SN9C10x 
> compression */
> ...
> 
> I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's
> likely why nobody reported this bug before (that I know of).
> 
> In any case, for non-proprietary formats, the default should be to
> always offer both the emulated format and the original one.
> 
> I suspect that the enclosed patch should fix the issue with bayer formats.

...
> @@ -178,7 +178,7 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   /* This keeps tracks of devices which have only formats for which apps
>  most likely will need conversion and we can thus safely add software
>  processing controls without a performance impact. */
> - int always_needs_conversion = 1;
> + int always_needs_conversion = 0;
>  
>   if (!data) {
>   fprintf(stderr, "libv4lconvert: error: out of memory!\n");
> @@ -208,8 +208,8 @@ struct v4lconvert_data 
> *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>   if (j < ARRAY_SIZE(supported_src_pixfmts)) {
>   data->supported_src_formats |= 1ULL << j;
>   v4lconvert_get_framesizes(data, fmt.pixelformat, j);
> - if (!supported_src_pixfmts[j].needs_conversion)
> - always_needs_conversion = 0;
> + if (supported_src_pixfmts[j].needs_conversion)
> + always_needs_conversion = 1;
>   } else
>   always_needs_conversion = 0;
>   }

Is the else still needed? You changed default to 0...


Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] libv4lconvert: by default, offer the original format to the client

[Mauro Carvalho Chehab]

The libv4lconvert part of libv4l was meant to provide a common
place to handle weird proprietary formats. With time, we also
added support to other standard formats, in order to help
V4L2 applications that are not performance sensitive to support
all V4L2 formats.

Yet, the hole idea is to let userspace to decide to implement
their own format conversion code when it needs either more
performance or more quality than what libv4lconvert provides.

In other words, applications should have the right to decide
between using a libv4lconvert emulated format or to implement
the decoding themselves for non-proprietary formats,
as this may have significative performance impact.

At the application side, deciding between them is just a matter
of looking at the V4L2_FMT_FLAG_EMULATED flag.

Yet, we don't want to have a miriad of format converters
everywhere for the proprietary formats, like V4L2_PIX_FMT_KONICA420,
V4L2_PIX_FMT_SPCA501, etc. So, let's offer only the emulated
variant for those weird stuff.

So, this patch changes the libv4lconvert behavior, for
all non-proprietary formats, including Bayer, to offer both
the original format and the emulated ones.

Signed-off-by: Mauro Carvalho Chehab 
---
 lib/libv4lconvert/libv4lconvert.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/libv4lconvert/libv4lconvert.c 
b/lib/libv4lconvert/libv4lconvert.c
index da718918b030..d87d6b91a838 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt 
supported_src_pixfmts[] = {
{ V4L2_PIX_FMT_OV511,0,  7,  7, 1 },
{ V4L2_PIX_FMT_OV518,0,  7,  7, 1 },
/* uncompressed bayer */
-   { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 1 },
-   { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 1 },
-   { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 1 },
-   { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 1 },
+   { V4L2_PIX_FMT_SBGGR8,   8,  8,  8, 0 },
+   { V4L2_PIX_FMT_SGBRG8,   8,  8,  8, 0 },
+   { V4L2_PIX_FMT_SGRBG8,   8,  8,  8, 0 },
+   { V4L2_PIX_FMT_SRGGB8,   8,  8,  8, 0 },
{ V4L2_PIX_FMT_STV0680,  8,  8,  8, 1 },
/* compressed bayer */
{ V4L2_PIX_FMT_SPCA561,  0,  9,  9, 1 },
@@ -178,7 +178,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int 
fd, void *dev_ops_pri
/* This keeps tracks of devices which have only formats for which apps
   most likely will need conversion and we can thus safely add software
   processing controls without a performance impact. */
-   int always_needs_conversion = 1;
+   int always_needs_conversion = 0;
 
if (!data) {
fprintf(stderr, "libv4lconvert: error: out of memory!\n");
@@ -208,10 +208,9 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int 
fd, void *dev_ops_pri
if (j < ARRAY_SIZE(supported_src_pixfmts)) {
data->supported_src_formats |= 1ULL << j;
v4lconvert_get_framesizes(data, fmt.pixelformat, j);
-   if (!supported_src_pixfmts[j].needs_conversion)
-   always_needs_conversion = 0;
-   } else
-   always_needs_conversion = 0;
+   if (supported_src_pixfmts[j].needs_conversion)
+   always_needs_conversion = 1;
+   }
}
 
data->no_formats = i;
-- 
2.9.3

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Fri, 10 Mar 2017 15:57:09 +
Russell King - ARM Linux  escreveu:

> On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote:
> > Hi Russell,
> > 
> > Em Fri, 10 Mar 2017 13:07:33 +
> > Russell King - ARM Linux  escreveu:
> >   
> > > The idea that the v4l libraries should intercept the format negotiation
> > > between the application and kernel is a particularly painful one - the
> > > default gstreamer build detects the v4l libraries, and links against it.
> > > That much is fine.
> > > 
> > > However, the problem comes when you're trying to use bayer formats. The
> > > v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> > > negotiation, and decide that they'll invoke v4lconvert to convert the
> > > bayer to RGB for you, whether you want them to do that or not.
> > > 
> > > v4lconvert may not be the most efficient way to convert, or even what
> > > is desired (eg, you may want to receive the raw bayer image.)  However,
> > > since the v4l libraries/v4lconvert gives you no option but to have its
> > > conversion forced into the pipeline, other options (such as using the
> > > gstreamer neon accelerated de-bayer plugin) isn't an option   
> > 
> > That's not true. There is an special flag, used only by libv4l2
> > emulated formats, that indicates when a video format is handled
> > via v4lconvert:  
> 
> I'm afraid that my statement comes from trying to use gstreamer with
> libv4l2 and _not_ being able to use the 8-bit bayer formats there at
> all - they are simply not passed across to the application through
> libv4l2/v4lconvert.
> 
> Instead, the formats that are passed across are the emulated formats.
> As I said above, that forces applications to use only the v4lconvert
> formats, the raw formats are not available.
> 
> So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite
> meaningless if you can't even enumerate the non-converted formats.
> 
> The problem comes from the "always needs conversion" stuff in
> v4lconvert coupled with the way this subdev stuff works - since it
> requires manual configuration of all the pads within the kernel
> media pipeline, the kernel ends up only advertising _one_ format
> to userspace - in my case, that's RGGB8.
> 
> When v4lconvert_create_with_dev_ops() enumerates the formats from
> the kernel, it gets only RGGB8.  That causes always_needs_conversion
> in there to remain true, so the special v4l control which enables/
> disables conversion gets created with a default value of "true".
> The RGGB8 bit is also set in data->supported_src_formats.
> 
> This causes v4lconvert_supported_dst_fmt_only() to return true.
> 
> What this all means is that v4lconvert_enum_fmt() will _not_ return
> any of the kernel formats, only the faked formats.
> 
> Ergo, the RGGB8 format from the kernel is completely hidden from the
> application, and only the emulated format is made available.  As I
> said above, this forces v4lconvert's debayering on the application,
> whether you want it or not.
> 
> In the gstreamer case, it knows nothing about this special control,
> which means that trying to use this gstreamer pipeline:
> 
> $ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink
> 
> is completely impossible without first rebuilding gstreamer _without_
> libv4l support.  Build gstreamer without libv4l support, and the above
> works.
> 
> Enabling debug output in gstreamer's v4l2src plugin confirms that
> the kernel's bayer format are totally hidden from gstreamer when
> linked with libv4l2, but are present when it isn't linked with
> libv4l2.

Argh! that is indeed a bug at libv4l (and maybe at gstreamer).

I guess that the always_needs_conversion logic was meant to be used to
really odd proprietary formats, e. g:

/*  Vendor-specific formats   */
#define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
#define V4L2_PIX_FMT_WNVA v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw 
compress */
#define V4L2_PIX_FMT_SN9C10X  v4l2_fourcc('S', '9', '1', '0') /* SN9C10x 
compression */
...

I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's
likely why nobody reported this bug before (that I know of).

In any case, for non-proprietary formats, the default should be to
always offer both the emulated format and the original one.

I suspect that the enclosed patch should fix the issue with bayer formats.

> 

Thanks,
Mauro

[PATCH RFC] libv4lconvert: by default, offer the original format to the client

Applications should have the right to decide between using a
libv4lconvert emulated format or to implement the decoding themselves,
as this may have significative performance impact.

So, change the default to always show both formats.

Change also the default for Bayer encoded formats, as userspace
likely will want to handle it directly.

Signed-off-by: Mauro Carvalho Chehab 


diff --git 

Re: [PATCH v5 00/39] i.MX Media Driver

[Russell King - ARM Linux]

Version 5 gives me no v4l2 controls exposed through the video device
interface.

Just like with version 4, version 5 is completely useless with IMX219:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110

So, like v4, I can't do any further testing.

On Thu, Mar 09, 2017 at 08:52:40PM -0800, Steve Longerbeam wrote:
> In version 5:
> 
> - ov5640: renamed "pwdn-gpios" to "powerdown-gpios"
> 
> - ov5640: add mutex lock around the subdev op entry points.
> 
> - ov5640: don't attempt to program the new mode in ov5640_set_fmt().
>   Instead set a new flag, pending_mode_change, and program the new
>   mode at s_stream() if flag is set.
> 
> - ov5640: implement [gs]_frame_interval. As part of that, create
>   ov5640_try_frame_interval(), which is used by both [gs]_frame_interval
>   and [gs]_parm.
> 
> - ov5640: don't attempt to set controls in ov5640_s_ctrl(), or at
>   mode change, do it instead after first power-up.
> 
> - video-multiplexer: include link_validate in media_entity_operations.
> 
> - video-multiplexer: enforce that output pad frame interval must match
>   input pad frame interval in vidsw_s_frame_interval().
> 
> - video-multiplexer: initialize frame interval to a default 30 fps.
> 
> - mipi csi-2: renamed "cfg" clock name property to "ref". This is the
>   27 MHz mipi csi-2 PLL reference clock.
> 
> - mipi csi-2: create a hsfreq_map[] table based on
>   https://community.nxp.com/docs/DOC-94312. Use it to select
>   a hsfreqrange_sel value when programming the D-PHY, based on
>   a max Mbps per lane. This is computed from the source subdev
>   via V4L2_CID_LINK_FREQ control, and if the subdev doesn't implement
>   that control, use a default hard-coded max Mbps per lane.
> 
> - added required ports property description to imx-media binding doc.
> 
> - removed event V4L2_EVENT_FRAME_TIMEOUT. On a frame timeout, which
>   is always unrecoverable, call vb2_queue_error() instead.
> 
> - export the remaining custom events to V4L2_EVENT_FRAME_INTERVAL_ERROR
>   and V4L2_EVENT_NEW_FRAME_BEFORE_EOF.
> 
> - vdic: use V4L2_CID_DEINTERLACING_MODE for motion compensation control
>   instead of a custom control.
> 
> - add v4l2_subdev_link_validate_frame_interval(). Call this in the
>   link_validate imx-media subdev callbacks and video-multiplexer.
> 
> - fix subdev event registration: implementation of subscribe_event()
>   and unsubscribe_event() subdev ops were missing.
> 
> - all calls from the pipeline to the sensor subdev have been removed.
>   Only the CSI subdev still refers to a sensor, and only to retrieve
>   its media bus config, which is necessary to setup the CSI interface.
> 
> - add mutex locks around the imx-media subdev op entry points.
> 
> - completed the propagation of all pad format parameters from sink
>   pads to source pads within every imx-media subdev.
> 
> - implement [gs]_frame_interval in all the imx-media subdevs.
> 
> - imx-ic-prpencvf: there isn't necessarily a CSI subdev in the pipeline
>   in the future, so make sure this is optional when calling the CSI's
>   FIM.
> 
> - the source pads that attach to capture device nodes now require the
>   IPU internal pixel codes. The capture device translates these to
>   v4l2 fourcc memory formats.
> 
> - fix control inheritance to the capture device. When the pipeline
>   was modified, the inherited controls were not being refreshed.
>   v4l2_pipeline_inherit_controls() is now called only in imx-media
>   link_notify() callback when a pipelink link is disabled or modified.
>   imx_media_find_pipeline_video_device() is created to locate the
>   capture device in the pipeline.
> 
> - fix a possible race when propagating formats to the capture device.
>   The subdevs and capture device use different mutex locks when setting
>   formats. imx_media_capture_device_set_format() is created which acquires
>   the capture device mutex when updating the capture device format.
> 
> - verify all subdevs were bound in the async completion callback.
>  
> 
> Philipp Zabel (7):
>   [media] dt-bindings: Add bindings for video-multiplexer device
>   ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their
> connections
>   add mux and video interface bridge entity functions
>   platform: add video-multiplexer subdevice driver
>   media: imx: csi: fix crop rectangle changes in set_fmt
>   media: imx: csi: add frame skipping support
>   media: imx: csi: fix crop rectangle reset in sink set_fmt
> 
> Russell King (4):
>   media: imx: add support for bayer formats
>   media: imx: csi: add support for bayer formats
>   media: imx: mipi-csi2: enable setting and getting of frame rates
>   media: imx: csi/fim: add support for frame intervals
> 
> Steve Longerbeam (28):
>   [media] dt-bindings: Add bindings 

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Fabio Estevam]

On Fri, Mar 10, 2017 at 3:59 PM, Troy Kisky
 wrote:
> On 3/9/2017 8:52 PM, Steve Longerbeam wrote:
>> There is a pin conflict with GPIO_6. This pin functions as a power
>> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
>> workaround for erratum ERR006687, to wake-up the ARM cores on normal
>> RX and TX packet done events. So we need to remove the h/w workaround
>> to support the OV5642. The result is that the CPUidle driver will no
>> longer allow entering the deep idle states on the sabrelite.
>>
>> This is a partial revert of
>>
>> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
>>   interrupt.")
>> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW workaround
>>   for ERR006687")
>>
>> Signed-off-by: Steve Longerbeam 
>> ---
>>  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 4 
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
>> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> index 8413179..89dce27 100644
>> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> @@ -270,9 +270,6 @@
>>   txd1-skew-ps = <0>;
>>   txd2-skew-ps = <0>;
>>   txd3-skew-ps = <0>;
>
> How about
>
> +#if !IS_ENABLED(CONFIG_VIDEO_OV5642)

Or maybe just create a new device tree for using the camera, like
imx6q-sabrelite-camera.dts.

This way we can keep the FEC erratum for the existing sabrelite dtb's.

Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround

[Troy Kisky]

On 3/9/2017 8:52 PM, Steve Longerbeam wrote:
> There is a pin conflict with GPIO_6. This pin functions as a power
> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
> workaround for erratum ERR006687, to wake-up the ARM cores on normal
> RX and TX packet done events. So we need to remove the h/w workaround
> to support the OV5642. The result is that the CPUidle driver will no
> longer allow entering the deep idle states on the sabrelite.
> 
> This is a partial revert of
> 
> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
>   interrupt.")
> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW workaround
>   for ERR006687")
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> index 8413179..89dce27 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> @@ -270,9 +270,6 @@
>   txd1-skew-ps = <0>;
>   txd2-skew-ps = <0>;
>   txd3-skew-ps = <0>;

How about

+#if !IS_ENABLED(CONFIG_VIDEO_OV5642)

> - interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
> -   < 0 119 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,err006687-workaround-present;

+#endif

Is that allowed ?


>   status = "okay";
>  };
>  
> @@ -373,7 +370,6 @@
>   MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL   0x1b030
>   /* Phy reset */
>   MX6QDL_PAD_EIM_D23__GPIO3_IO23  0x000b0
> - MX6QDL_PAD_GPIO_6__ENET_IRQ 0x000b1
>   >;
>   };
>  
> 

Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

[Steve Longerbeam]

Hi Hans,

On 03/10/2017 04:03 AM, Hans Verkuil wrote:

On 10/03/17 05:52, Steve Longerbeam wrote:

Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
output device has measured an interval between the reception or transmit
completion of two consecutive frames of video that is outside the nominal
frame interval by some tolerance value.


Reading back what was said on this I agree with Sakari that this doesn't
belong here.

Userspace can detect this just as easily (if not easier) with a timeout.




Unfortunately measuring frame intervals from userland is not accurate
enough for i.MX6.

The issue here is that the IPUv3, specifically the CSI unit, can
permanently lose vertical sync if there are truncated frames sent
on the bt.656 bus. We have seen a single missing line of video cause
loss of vertical sync. The only way to correct this is to shutdown
the IPU capture hardware and restart, which can be accomplished
simply by restarting streaming from userland.

There are no other indicators from the sensor about these short
frame events (believe me, we've exhausted all avenues with the ADV718x).
And the IPUv3 DMA engine has no status indicators for short frames
either. So the only way to detect them is by measuring frame intervals.

The intervals have to be able to resolve a single line of missing video.
With a PAL video source that requires better than 58 usec accuracy.

There is too much uncertainty to resolve this at user level. The
driver is able to resolve this by measuring intervals between hardware
interrupts as long as interrupt latency is reasonably low, and we
have another method using the i.MX6 hardware input capture support
that can measure these intervals very accurately with no errors
introduced by interrupt latency.

I made this event a private event to imx-media driver in a previous
iteration, so I can return it to a private event, but this can't be
done at user level.

Steve






Signed-off-by: Steve Longerbeam 
---
 Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++
 Documentation/media/videodev2.h.rst.exceptions  | 1 +
 include/uapi/linux/videodev2.h  | 1 +
 3 files changed, 8 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
index 8d663a7..dc77363 100644
--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
@@ -197,6 +197,12 @@ call.
the regions changes. This event has a struct
:c:type:`v4l2_event_motion_det`
associated with it.
+* - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
+  - 7
+  - This event is triggered when the video capture or output device
+   has measured an interval between the reception or transmit
+   completion of two consecutive frames of video that is outside
+   the nominal frame interval by some tolerance value.
 * - ``V4L2_EVENT_PRIVATE_START``
   - 0x0800
   - Base event number for driver-private events.
diff --git a/Documentation/media/videodev2.h.rst.exceptions 
b/Documentation/media/videodev2.h.rst.exceptions
index e11a0d0..c7d8fad 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
 replace define V4L2_EVENT_FRAME_SYNC event-type
 replace define V4L2_EVENT_SOURCE_CHANGE event-type
 replace define V4L2_EVENT_MOTION_DET event-type
+replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
 replace define V4L2_EVENT_PRIVATE_START event-type

 replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45184a2..cf5a0d0 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
 #define V4L2_EVENT_FRAME_SYNC  4
 #define V4L2_EVENT_SOURCE_CHANGE   5
 #define V4L2_EVENT_MOTION_DET  6
+#define V4L2_EVENT_FRAME_INTERVAL_ERROR7
 #define V4L2_EVENT_PRIVATE_START   0x0800

 /* Payload for V4L2_EVENT_VSYNC */

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Fri, Mar 10, 2017 at 03:57:09PM +, Russell King - ARM Linux wrote:
> Enabling debug output in gstreamer's v4l2src plugin confirms that
> the kernel's bayer format are totally hidden from gstreamer when
> linked with libv4l2, but are present when it isn't linked with
> libv4l2.

Here's the information to back up my claims:

root@hbi2ex:~# v4l2-ctl -d 6 --list-formats-ext
ioctl: VIDIOC_ENUM_FMT
Index   : 0
Type: Video Capture
Pixel Format: 'RGGB'
Name: 8-bit Bayer RGRG/GBGB

root@hbi2ex:~# DISPLAY=:0 GST_DEBUG_NO_COLOR=1 GST_DEBUG=v4l2:9 gst-launch-1.0 
v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink > gst-v4l2-1.log 2>&1
root@hbi2ex:~# cut -b65- gst-v4l2-1.log|less
v4l2_calls.c:519:gst_v4l2_open: Trying to open device /dev/video6
v4l2_calls.c:69:gst_v4l2_get_capabilities: getting capabilities
v4l2_calls.c:77:gst_v4l2_get_capabilities: driver:  
'imx-media-camif'
v4l2_calls.c:78:gst_v4l2_get_capabilities: card:
'imx-media-camif'
v4l2_calls.c:79:gst_v4l2_get_capabilities: bus_info:''
v4l2_calls.c:80:gst_v4l2_get_capabilities: version: 00040a00
v4l2_calls.c:81:gst_v4l2_get_capabilities: capabilites: 8521
...
v4l2_calls.c:258:gst_v4l2_fill_lists:   controls+menus
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 
v4l2_calls.c:319:gst_v4l2_fill_lists: starting control class 'User 
Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 00980001
v4l2_calls.c:389:gst_v4l2_fill_lists: Adding ControlID 
white_balance_automatic (98090c)
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 0098090c
v4l2_calls.c:389:gst_v4l2_fill_lists: Adding ControlID gamma (980910)
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 00980910
v4l2_calls.c:389:gst_v4l2_fill_lists: Adding ControlID gain (980913)
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 00980913
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 00980914
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 00980915
v4l2_calls.c:319:gst_v4l2_fill_lists: starting control class 'Camera 
Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009a0001
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID 
exposure_time_absolute (9a0902) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009a0902
v4l2_calls.c:319:gst_v4l2_fill_lists: starting control class 'Image 
Source Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0001
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID vertical_blanking 
(9e0901) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0901
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID horizontal_blanking 
(9e0902) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0902
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID analogue_gain 
(9e0903) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0903
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID red_pixel_value 
(9e0904) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0904
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID green_red_pixel_value 
(9e0905) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0905
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID blue_pixel_value 
(9e0906) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0906
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID 
green_blue_pixel_value (9e0907) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009e0907
v4l2_calls.c:319:gst_v4l2_fill_lists: starting control class 'Image 
Processing Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009f0001
v4l2_calls.c:340:gst_v4l2_fill_lists: Control type for 'Pixel Rate' 
not suppored for extra controls.
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID Pixel Rate (9f0902) 
unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009f0902
v4l2_calls.c:382:gst_v4l2_fill_lists: ControlID test_pattern (9f0903) 
unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists: checking control 009f0903
v4l2_calls.c:284:gst_v4l2_fill_lists: controls finished
v4l2_calls.c:451:gst_v4l2_fill_lists: done
v4l2_calls.c:587:gst_v4l2_open: Opened device 'imx-media-camif' 
(/dev/video6) successfully
gstv4l2object.c:804:gst_v4l2_set_defaults: tv_norm=0x0, norm=(nil)
v4l2_calls.c:734:gst_v4l2_get_norm: getting norm
v4l2_calls.c:1021:gst_v4l2_get_input: trying to get input
v4l2_calls.c:1031:gst_v4l2_get_input: input: 0

gstv4l2object.c:1106:gst_v4l2_object_fill_format_list: getting src 
format enumerations
gstv4l2object.c:1124:gst_v4l2_object_fill_format_list: index:   0
gstv4l2object.c:1125:gst_v4l2_object_fill_format_list: type:1
gstv4l2object.c:1126:gst_v4l2_object_fill_format_list: flags:   
0002
gstv4l2object.c:1128:gst_v4l2_object_fill_format_list: description: 
'RGB3'
gstv4l2object.c:1130:gst_v4l2_object_fill_format_list: 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/10/2017 06:27 AM, Brian Starkey wrote:
> On Fri, Mar 10, 2017 at 11:46:42AM +, Robin Murphy wrote:
>> On 10/03/17 10:31, Brian Starkey wrote:
>>> Hi,
>>>
>>> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
 On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>>
>>> [snip]
>>>
>
> For me those patches are going in the right direction.
>
> I still have few questions:
> - since alignment management has been remove from ion-core, should it
> be also removed from ioctl structure ?

 Yes, I think I'm going to go with the suggestion to fixup the ABI
 so we don't need the compat layer and as part of that I'm also
 dropping the align argument.

>>>
>>> Is the only motivation for removing the alignment parameter that
>>> no-one got around to using it for something useful yet?
>>> The original comment was true - different devices do have different
>>> alignment requirements.
>>>
>>> Better alignment can help SMMUs use larger blocks when mapping,
>>> reducing TLB pressure and the chance of a page table walk causing
>>> display underruns.
>>
>> For that use-case, though, alignment alone doesn't necessarily help -
>> you need the whole allocation granularity to match your block size (i.e.
>> given a 1MB block size, asking for 17KB and getting back 17KB starting
>> at a 1MB boundary doesn't help much - that whole 1MB needs to be
>> allocated and everyone needs to know it to ensure that the whole lot can
>> be mapped safely). Now, whether it's down to the callers or the heap
>> implementations to decide and enforce that granularity is another
>> question, but provided allocations are at least naturally aligned to
>> whatever the granularity is (which is a reasonable assumption to bake
>> in) then it's all good.
>>
>> Robin.
> 
> Agreed, alignment alone isn't enough. But lets assume that an app
> knows what a "good" granularity is, and always asks for allocation
> sizes which are suitably rounded to allow blocks to be used. Currently
> it looks like a "standard" ION_HEAP_TYPE_CARVEOUT heap would give me
> back just a PAGE_SIZE aligned buffer. So even *if* the caller knows
> its desired block size, there's no way for it to get guaranteed better
> alignment, which wouldn't be a bad feature to have.
> 
> Anyway as Daniel and Rob say, if the interface is designed properly
> this kind of extension would be possible later, or you can have a
> special heap with a larger granule.
> 
> I suppose it makes sense to remove it while there's no-one actually
> implementing it, in case an alternate method proves more usable.
> 
> -Brian

Part of the reason I want to remove it is to avoid confusion over
callers thinking it will do anything on most heaps. I agree being
able to specify a larger granularity would be beneficial but I
don't think a dedicated field in the ABI is the right approach.

Thanks,
Laura

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Anton Sviridenko]

On Thu, Mar 09, 2017 at 06:58:45PM -0300, Ismael Luceno wrote:
> On 08/Mar/2017 21:59, Andrey Utkin wrote:
> > Signed-off-by: Andrey Utkin 
> > Signed-off-by: Andrey Utkin 
> > 
> > Please welcome Anton who is now in charge of solo6x10 and tw5864 support
> > and development in Bluecherry company, I have sent out to him the
> > hardware samples I possessed. (We will prepare the patch updating
> > MAINTAINERS file soon.)
> > 
> > If anybody has any outstanding complains, concerns or tasks regarding
> > solo6x10 and tw5864 drivers, I think now is good occasion to let us know
> > about it.
> 
> Welcome Anton!
> 
> The first issue that comes to mind is that quantization matrices
> need to be adjusted since forever. Also it needs more realistic
> buffer sizes.

Hi Ismael,

can you provide more details about quantization matrices issue and
buffer sizes?

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote:
> Hi Russell,
> 
> Em Fri, 10 Mar 2017 13:07:33 +
> Russell King - ARM Linux  escreveu:
> 
> > The idea that the v4l libraries should intercept the format negotiation
> > between the application and kernel is a particularly painful one - the
> > default gstreamer build detects the v4l libraries, and links against it.
> > That much is fine.
> > 
> > However, the problem comes when you're trying to use bayer formats. The
> > v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> > negotiation, and decide that they'll invoke v4lconvert to convert the
> > bayer to RGB for you, whether you want them to do that or not.
> > 
> > v4lconvert may not be the most efficient way to convert, or even what
> > is desired (eg, you may want to receive the raw bayer image.)  However,
> > since the v4l libraries/v4lconvert gives you no option but to have its
> > conversion forced into the pipeline, other options (such as using the
> > gstreamer neon accelerated de-bayer plugin) isn't an option 
> 
> That's not true. There is an special flag, used only by libv4l2
> emulated formats, that indicates when a video format is handled
> via v4lconvert:

I'm afraid that my statement comes from trying to use gstreamer with
libv4l2 and _not_ being able to use the 8-bit bayer formats there at
all - they are simply not passed across to the application through
libv4l2/v4lconvert.

Instead, the formats that are passed across are the emulated formats.
As I said above, that forces applications to use only the v4lconvert
formats, the raw formats are not available.

So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite
meaningless if you can't even enumerate the non-converted formats.

The problem comes from the "always needs conversion" stuff in
v4lconvert coupled with the way this subdev stuff works - since it
requires manual configuration of all the pads within the kernel
media pipeline, the kernel ends up only advertising _one_ format
to userspace - in my case, that's RGGB8.

When v4lconvert_create_with_dev_ops() enumerates the formats from
the kernel, it gets only RGGB8.  That causes always_needs_conversion
in there to remain true, so the special v4l control which enables/
disables conversion gets created with a default value of "true".
The RGGB8 bit is also set in data->supported_src_formats.

This causes v4lconvert_supported_dst_fmt_only() to return true.

What this all means is that v4lconvert_enum_fmt() will _not_ return
any of the kernel formats, only the faked formats.

Ergo, the RGGB8 format from the kernel is completely hidden from the
application, and only the emulated format is made available.  As I
said above, this forces v4lconvert's debayering on the application,
whether you want it or not.

In the gstreamer case, it knows nothing about this special control,
which means that trying to use this gstreamer pipeline:

$ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink

is completely impossible without first rebuilding gstreamer _without_
libv4l support.  Build gstreamer without libv4l support, and the above
works.

Enabling debug output in gstreamer's v4l2src plugin confirms that
the kernel's bayer format are totally hidden from gstreamer when
linked with libv4l2, but are present when it isn't linked with
libv4l2.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Fri, 10 Mar 2017 15:20:48 +0100
Hans Verkuil  escreveu:

> 
> > As I've already mentioned, from talking about this with Mauro, it seems
> > Mauro is in agreement with permitting the control inheritence... I wish
> > Mauro would comment for himself, as I can't quote our private discussion
> > on the subject.  
> 
> I can't comment either, not having seen his mail and reasoning.

The rationale is that we should support the simplest use cases first.

In the case of the first MC-based driver (and several subsequent
ones), the simplest use case required MC, as it was meant to suport
a custom-made sophisticated application that required fine control
on each component of the pipeline and to allow their advanced
proprietary AAA userspace-based algorithms to work.

That's not true, for example, for the UVC driver. There, MC
is optional, as it should be.

> > Right now, my view is that v4l2 is currently being screwed up by people
> > with different opinions - there is no unified concensus on how any of
> > this stuff is supposed to work, everyone is pulling in different
> > directions.  That needs solving _really_ quickly, so I suggest that
> > v4l2 people urgently talk to each other and thrash out some of the
> > issues that Steve's patch set has brought up, and settle on a way
> > forward, rather than what is seemingly happening today - which is
> > everyone working in isolation of everyone else with their own bias on
> > how things should be done.  
> 
> The simple fact is that to my knowledge no other MC applications inherit
> controls from subdevs. Suddenly doing something different here seems very
> wrong to me and needs very good reasons.

That's because it was not needed before, as other subdev-based drivers
are meant to be used only on complex scenarios with custom-made apps.

Thanks,
Mauro

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Hi Russell,

Em Fri, 10 Mar 2017 13:07:33 +
Russell King - ARM Linux  escreveu:

> The idea that the v4l libraries should intercept the format negotiation
> between the application and kernel is a particularly painful one - the
> default gstreamer build detects the v4l libraries, and links against it.
> That much is fine.
> 
> However, the problem comes when you're trying to use bayer formats. The
> v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> negotiation, and decide that they'll invoke v4lconvert to convert the
> bayer to RGB for you, whether you want them to do that or not.
> 
> v4lconvert may not be the most efficient way to convert, or even what
> is desired (eg, you may want to receive the raw bayer image.)  However,
> since the v4l libraries/v4lconvert gives you no option but to have its
> conversion forced into the pipeline, other options (such as using the
> gstreamer neon accelerated de-bayer plugin) isn't an option 

That's not true. There is an special flag, used only by libv4l2
emulated formats, that indicates when a video format is handled
via v4lconvert:

* - ``V4L2_FMT_FLAG_EMULATED``
  - 0x0002
  - This format is not native to the device but emulated through
software (usually libv4l2), where possible try to use a native
format instead for better performance.

Using this flag, if the application supports a video format directly
supported by the hardware, it can use their own video format decoder.
If not, it is still possible to use the V4L2 hardware, by using
v4lconvert.

Unfortunately, very few applications currently check it.

I wrote a patch for zbar (a multi-format barcode reader) in the past,
adding a logic there that gives a high priority to hardware formats,
and a low priority to emulated ones:

https://lists.fedoraproject.org/pipermail/scm-commits/2010-December/537428.html

> without
> rebuilding gstreamer _without_ linking against the v4l libraries.

I guess it wouldn't be complex to add a logic similar to that at
gstreamer.

AFAIKT, there's another problem that would prevent to make
libv4l the default on gstreamer: right now, libv4l doesn't support
DMABUF. As gstreamer is being used on embedded hardware, I'd say
that DMABUF support should be default there.

Thanks,
Mauro

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Fri, 10 Mar 2017 13:54:28 +0100
Hans Verkuil  escreveu:

> > Devices that have complex pipeline that do essentially require using the
> > Media controller interface to configure them are out of that scope.
> >   
> 
> Way too much of how the MC devices should be used is in the minds of 
> developers.
> There is a major lack for good detailed documentation, utilities, compliance
> test (really needed!) and libv4l plugins.

Unfortunately, we merged an incomplete MC support at the Kernel. We knew
all the problems with MC-based drivers and V4L2 applications by the time
it was developed, and we requested Nokia developers (with was sponsoring MC
develoment, on that time) to work on a solution to allow standard V4L2
applications to work with MC based boards.

Unfortunately, we took the decision to merge MC without that, because 
Nokia was giving up on Linux development, and we didn't want to lose the
2 years of discussions and work around it, as Nokia employers were leaving
the company. Also, on that time, there was already some patches floating
around adding backward support via libv4l. Unfortunately, those patches
were never finished.

The net result is that MC was merged with some huge gaps, including
the lack of a proper solution for a generic V4L2 program to work
with V4L2 devices that use the subdev API.

That was not that bad by then, as MC was used only on cell phones
that run custom-made applications. 

The reallity changed, as now, we have lots of low cost SoC based
boards, used for all sort of purposes. So, we need a quick solution
for it.

In other words, while that would be acceptable support special apps
on really embedded systems, it is *not OK* for general purpose SoC
harware[1].

[1] I'm calling "general purpose SoC harware" those ARM boards
like Raspberry Pi that are shipped to the mass and used by a wide
range of hobbyists and other people that just wants to run Linux on
ARM. It is possible to buy such boards for a very cheap price,
making them to be used not only on special projects, where a custom
made application could be interesting, but also for a lot of
users that just want to run Linux on a low cost ARM board, while
keeping using standard V4L2 apps, like "camorama".

That's perhaps one of the reasons why it took a long time for us to
start receiving drivers upstream for such hardware: it is quite 
intimidating and not logical to require developers to implement
on their drivers 2 complex APIs (MC, subdev) for those
hardware that most users won't care. From user's perspective,
being able to support generic applications like "camorama" and
"zbar" is all they want.

In summary, I'm pretty sure we need to support standard V4L2 
applications on boards like Raspberry Pi and those low-cost 
SoC-based boards that are shipped to end users.

> Anyway, regarding this specific patch and for this MC-aware driver: no, you
> shouldn't inherit controls from subdevs. It defeats the purpose.

Sorry, but I don't agree with that. The subdev API is an optional API
(and even the MC API can be optional).

I see the rationale for using MC and subdev APIs on cell phones,
ISV and other embedded hardware, as it will allow fine-tuning
the driver's support to allow providing the required quality for
certain custom-made applications. but on general SoC hardware,
supporting standard V4L2 applications is a need.

Ok, perhaps supporting both subdev API and V4L2 API at the same
time doesn't make much sense. We could disable one in favor of the
other, either at compilation time or at runtime.

This way, if the subdev API is disabled, the driver will be
functional for V4L2-based applications that don't support neither
MC or subdev APIs.

> As mentioned, I will attempt to try and get some time to work on this
> later this year. Fingers crossed.

That will be good, and, once we have a solution that works, we can
work on cleanup the code, but, until then, drivers for arm-based boards
sold to end consumers should work out of the box with standard V4L2 apps.

While we don't have that, I'm OK to merge patches adding such support
upstream.

Thanks,
Mauro

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Brian Starkey]

On Fri, Mar 10, 2017 at 11:46:42AM +, Robin Murphy wrote:

On 10/03/17 10:31, Brian Starkey wrote:

Hi,

On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:


[snip]



For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?


Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.



Is the only motivation for removing the alignment parameter that
no-one got around to using it for something useful yet?
The original comment was true - different devices do have different
alignment requirements.

Better alignment can help SMMUs use larger blocks when mapping,
reducing TLB pressure and the chance of a page table walk causing
display underruns.


For that use-case, though, alignment alone doesn't necessarily help -
you need the whole allocation granularity to match your block size (i.e.
given a 1MB block size, asking for 17KB and getting back 17KB starting
at a 1MB boundary doesn't help much - that whole 1MB needs to be
allocated and everyone needs to know it to ensure that the whole lot can
be mapped safely). Now, whether it's down to the callers or the heap
implementations to decide and enforce that granularity is another
question, but provided allocations are at least naturally aligned to
whatever the granularity is (which is a reasonable assumption to bake
in) then it's all good.

Robin.


Agreed, alignment alone isn't enough. But lets assume that an app
knows what a "good" granularity is, and always asks for allocation
sizes which are suitably rounded to allow blocks to be used. Currently
it looks like a "standard" ION_HEAP_TYPE_CARVEOUT heap would give me
back just a PAGE_SIZE aligned buffer. So even *if* the caller knows
its desired block size, there's no way for it to get guaranteed better
alignment, which wouldn't be a bad feature to have.

Anyway as Daniel and Rob say, if the interface is designed properly
this kind of extension would be possible later, or you can have a
special heap with a larger granule.

I suppose it makes sense to remove it while there's no-one actually
implementing it, in case an alternate method proves more usable.

-Brian





-Brian

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 10/03/17 15:01, Russell King - ARM Linux wrote:
> On Fri, Mar 10, 2017 at 02:22:29PM +0100, Hans Verkuil wrote:
>> And nobody of the media core developers has the time to work on the docs,
>> utilities and libraries you need to make this all work cleanly and reliably.
> 
> Well, talking about docs, and in connection to control inheritence,
> this is already documented in at least three separate places:
> 
> Documentation/media/uapi/v4l/dev-subdev.rst:
> 
>   Controls
>   
>   ...
>   Depending on the driver, those controls might also be exposed through
>   one (or several) V4L2 device nodes.
> 
> Documentation/media/kapi/v4l2-subdev.rst:
> 
>   ``VIDIOC_QUERYCTRL``,
>   ``VIDIOC_QUERYMENU``,
>   ``VIDIOC_G_CTRL``,
>   ``VIDIOC_S_CTRL``,
>   ``VIDIOC_G_EXT_CTRLS``,
>   ``VIDIOC_S_EXT_CTRLS`` and
>   ``VIDIOC_TRY_EXT_CTRLS``:
>   
>   The controls ioctls are identical to the ones defined in V4L2. They
>   behave identically, with the only exception that they deal only with
>   controls implemented in the sub-device. Depending on the driver, 
> those
>   controls can be also be accessed through one (or several) V4L2 
> device
>   nodes.
> 
> Then there's Documentation/media/kapi/v4l2-controls.rst, which gives a
> step by step approach to the main video device inheriting controls from
> its subdevices, and it says:
> 
>   Inheriting Controls
>   ---
>   
>   When a sub-device is registered with a V4L2 driver by calling
>   v4l2_device_register_subdev() and the ctrl_handler fields of both 
> v4l2_subdev
>   and v4l2_device are set, then the controls of the subdev will become
>   automatically available in the V4L2 driver as well. If the subdev driver
>   contains controls that already exist in the V4L2 driver, then those will be
>   skipped (so a V4L2 driver can always override a subdev control).
>   
>   What happens here is that v4l2_device_register_subdev() calls
>   v4l2_ctrl_add_handler() adding the controls of the subdev to the controls
>   of v4l2_device.
> 
> So, either the docs are wrong, or the advice being mentioned in emails
> about subdev control inheritence is misleading.  Whatever, the two are
> currently inconsistent.

These docs were written for non-MC drivers, and for those the documentation
is correct. Unfortunately, this was never updated for MC drivers.

> As I've already mentioned, from talking about this with Mauro, it seems
> Mauro is in agreement with permitting the control inheritence... I wish
> Mauro would comment for himself, as I can't quote our private discussion
> on the subject.

I can't comment either, not having seen his mail and reasoning.

> Right now, my view is that v4l2 is currently being screwed up by people
> with different opinions - there is no unified concensus on how any of
> this stuff is supposed to work, everyone is pulling in different
> directions.  That needs solving _really_ quickly, so I suggest that
> v4l2 people urgently talk to each other and thrash out some of the
> issues that Steve's patch set has brought up, and settle on a way
> forward, rather than what is seemingly happening today - which is
> everyone working in isolation of everyone else with their own bias on
> how things should be done.

The simple fact is that to my knowledge no other MC applications inherit
controls from subdevs. Suddenly doing something different here seems very
wrong to me and needs very good reasons.

But yes, the current situation sucks. Yelling doesn't help though if nobody
has time and there are several other high-prio projects that need our
attention as well.

If you know a good kernel developer who has a few months to spare, please
point him/her in our direction!

Regards,

Hans

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Fri, Mar 10, 2017 at 02:22:29PM +0100, Hans Verkuil wrote:
> And nobody of the media core developers has the time to work on the docs,
> utilities and libraries you need to make this all work cleanly and reliably.

Well, talking about docs, and in connection to control inheritence,
this is already documented in at least three separate places:

Documentation/media/uapi/v4l/dev-subdev.rst:

  Controls
  
  ...
  Depending on the driver, those controls might also be exposed through
  one (or several) V4L2 device nodes.

Documentation/media/kapi/v4l2-subdev.rst:

  ``VIDIOC_QUERYCTRL``,
  ``VIDIOC_QUERYMENU``,
  ``VIDIOC_G_CTRL``,
  ``VIDIOC_S_CTRL``,
  ``VIDIOC_G_EXT_CTRLS``,
  ``VIDIOC_S_EXT_CTRLS`` and
  ``VIDIOC_TRY_EXT_CTRLS``:
  
  The controls ioctls are identical to the ones defined in V4L2. They
  behave identically, with the only exception that they deal only with
  controls implemented in the sub-device. Depending on the driver, those
  controls can be also be accessed through one (or several) V4L2 device
  nodes.

Then there's Documentation/media/kapi/v4l2-controls.rst, which gives a
step by step approach to the main video device inheriting controls from
its subdevices, and it says:

  Inheriting Controls
  ---
  
  When a sub-device is registered with a V4L2 driver by calling
  v4l2_device_register_subdev() and the ctrl_handler fields of both v4l2_subdev
  and v4l2_device are set, then the controls of the subdev will become
  automatically available in the V4L2 driver as well. If the subdev driver
  contains controls that already exist in the V4L2 driver, then those will be
  skipped (so a V4L2 driver can always override a subdev control).
  
  What happens here is that v4l2_device_register_subdev() calls
  v4l2_ctrl_add_handler() adding the controls of the subdev to the controls
  of v4l2_device.

So, either the docs are wrong, or the advice being mentioned in emails
about subdev control inheritence is misleading.  Whatever, the two are
currently inconsistent.

As I've already mentioned, from talking about this with Mauro, it seems
Mauro is in agreement with permitting the control inheritence... I wish
Mauro would comment for himself, as I can't quote our private discussion
on the subject.

Right now, my view is that v4l2 is currently being screwed up by people
with different opinions - there is no unified concensus on how any of
this stuff is supposed to work, everyone is pulling in different
directions.  That needs solving _really_ quickly, so I suggest that
v4l2 people urgently talk to each other and thrash out some of the
issues that Steve's patch set has brought up, and settle on a way
forward, rather than what is seemingly happening today - which is
everyone working in isolation of everyone else with their own bias on
how things should be done.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Rob Clark]

On Fri, Mar 10, 2017 at 7:40 AM, Daniel Vetter  wrote:
> On Fri, Mar 10, 2017 at 10:31:13AM +, Brian Starkey wrote:
>> Hi,
>>
>> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
>> > On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>
>> [snip]
>>
>> > >
>> > > For me those patches are going in the right direction.
>> > >
>> > > I still have few questions:
>> > > - since alignment management has been remove from ion-core, should it
>> > > be also removed from ioctl structure ?
>> >
>> > Yes, I think I'm going to go with the suggestion to fixup the ABI
>> > so we don't need the compat layer and as part of that I'm also
>> > dropping the align argument.
>> >
>>
>> Is the only motivation for removing the alignment parameter that
>> no-one got around to using it for something useful yet?
>> The original comment was true - different devices do have different
>> alignment requirements.
>>
>> Better alignment can help SMMUs use larger blocks when mapping,
>> reducing TLB pressure and the chance of a page table walk causing
>> display underruns.
>
> Extending ioctl uapi is easy, trying to get rid of bad uapi is much
> harder. Given that right now we don't have an ion allocator that does
> alignment I think removing it makes sense. And if we go with lots of
> heaps, we might as well have an ion heap per alignment that your hw needs,
> so there's different ways to implement this in the future.

slight correction:  if you plan ahead (and do things like zero init if
userspace passes in a smaller ioctl struct like drm_ioctl does),
extending ioctl uapi is easy.. might be something worth fixing from
the get-go..

BR,
-R

> At least from the unix device memory allocator pov it's probably simpler
> to encode stuff like this into the heap name, instead of having to pass
> heap + list of additional properties/constraints.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6] [media] vimc: Virtual Media Controller core, capture and sensor

[Helen Koike]

Hi Hans,

On 2017-03-10 10:08 AM, Hans Verkuil wrote:

Hi Helen,

On 11/01/17 02:30, Helen Koike wrote:
>
> Thank you for the review, I'll update the patch accordingly and re-submit it.
>
> Helen

Do you know when you have a v7 ready?


Thanks for your interest. I don't have the next version entirely ready yet but 
I'll work on it to send this next version asap and the other patches I have on 
top of it as well.



We really need a vimc driver so people without hardware can actually fiddle 
around
with the MC.

See also my rant here (not directed at you!):

https://www.spinics.net/lists/kernel/msg2462009.html

Regards,

Hans


Helen

Re: [RFC] omap3isp: add support for CSI1 bus

[Pavel Machek]

On Mon 2017-03-06 08:56:59, Pavel Machek wrote:
> omap3isp: add rest of CSI1 support
> 
> CSI1 needs one more bit to be set up. Do just that.
> 
> Signed-off-by: Pavel Machek 
> 
> ---
> 
> Hmm. Looking at that... num_data_lanes probably should be modified in
> local variable, not globally like this. Should I do that?
> 
> Anything else that needs fixing?

Ping? Feedback here would be nice. This is last "interesting" piece of
the hardware support...

Best regards,
Pavel

> index 24a9fc5..6feba36 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -21,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "isp.h"
>  #include "ispreg.h"
> @@ -1149,6 +1152,7 @@ int omap3isp_ccp2_init(struct isp_device *isp)
>   "Could not get regulator vdds_csib\n");
>   ccp2->vdds_csib = NULL;
>   }
> + ccp2->phy = >isp_csiphy2;
>   } else if (isp->revision == ISP_REVISION_15_0) {
>   ccp2->phy = >isp_csiphy1;
>   }
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c 
> b/drivers/media/platform/omap3isp/ispcsiphy.c
> index 50c0f64..cd6351b 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -197,9 +200,10 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
>   }
>  
>   if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
> - || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2)
> + || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) {
>   lanes = >bus.ccp2.lanecfg;
> - else
> + phy->num_data_lanes = 1;
> + } else
>   lanes = >bus.csi2.lanecfg;
>  
>   /* Clock and data lanes verification */
> @@ -302,13 +306,16 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
>   if (rval < 0)
>   goto done;
>  
> - rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON);
> - if (rval) {
> - regulator_disable(phy->vdd);
> - goto done;
> + if (phy->isp->revision == ISP_REVISION_15_0) {
> + rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON);
> + if (rval) {
> + regulator_disable(phy->vdd);
> + goto done;
> + }
> + 
> + csiphy_power_autoswitch_enable(phy, true);  
>   }
>  
> - csiphy_power_autoswitch_enable(phy, true);
>   phy->phy_in_use = 1;
>  
>  done:
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH v1] staging: media: Remove unused function atomisp_set_stop_timeout()

[simran singhal]

The function atomisp_set_stop_timeout on being called, simply returns
back. The function hasn't been mentioned in the TODO and doesn't have
FIXME code around. Hence, atomisp_set_stop_timeout and its calls have been
removed.

This was done using Coccinelle.

@@
identifier f;
@@

void f(...) {

-return;

}

Signed-off-by: simran singhal 
---
 v1:
   -Change Subject to include name of function
   -change commit message to include the coccinelle script
   
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c  | 1 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat.h   | 1 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c | 5 -
 3 files changed, 7 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d9a5c24..9720756 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1692,7 +1692,6 @@ void atomisp_wdt_work(struct work_struct *work)
}
}
 #endif
-   atomisp_set_stop_timeout(ATOMISP_CSS_STOP_TIMEOUT_US);
dev_err(isp->dev, "timeout recovery handling done\n");
atomic_set(>wdt_work_queued, 0);
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat.h 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat.h
index e6b0cce..fb8b8fa 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat.h
@@ -660,7 +660,6 @@ int atomisp_css_set_acc_parameters(struct atomisp_acc_fw 
*acc_fw);
 int atomisp_css_isr_thread(struct atomisp_device *isp,
   bool *frame_done_found,
   bool *css_pipe_done);
-void atomisp_set_stop_timeout(unsigned int timeout);
 
 bool atomisp_css_valid_sof(struct atomisp_device *isp);
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 6697d72..cfa0ad4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4699,11 +4699,6 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
return 0;
 }
 
-void atomisp_set_stop_timeout(unsigned int timeout)
-{
-   return;
-}
-
 bool atomisp_css_valid_sof(struct atomisp_device *isp)
 {
unsigned int i, j;
-- 
2.7.4

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 10/03/17 14:07, Russell King - ARM Linux wrote:
> On Fri, Mar 10, 2017 at 01:54:28PM +0100, Hans Verkuil wrote:
>> But there was always meant to be a layer (libv4l plugin) that could be
>> used to setup a 'default scenario' that existing applications could use,
>> but that was never enforced, sadly.
> 
> However, there's other painful issues lurking in userspace, particularly
> to do with the v4l libraries.
> 
> The idea that the v4l libraries should intercept the format negotiation
> between the application and kernel is a particularly painful one - the
> default gstreamer build detects the v4l libraries, and links against it.
> That much is fine.
> 
> However, the problem comes when you're trying to use bayer formats. The
> v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> negotiation, and decide that they'll invoke v4lconvert to convert the
> bayer to RGB for you, whether you want them to do that or not.
> 
> v4lconvert may not be the most efficient way to convert, or even what
> is desired (eg, you may want to receive the raw bayer image.)  However,
> since the v4l libraries/v4lconvert gives you no option but to have its
> conversion forced into the pipeline, other options (such as using the
> gstreamer neon accelerated de-bayer plugin) isn't an option without
> rebuilding gstreamer _without_ linking against the v4l libraries.
> 
> At that point, saying "this should be done in a libv4l plugin" becomes
> a total nonsense, because if you need to avoid libv4l due to its
> stupidities, you don't get the benefit of subdevs, and it yet again
> _forces_ people down the route of custom applications.
> 
> So, I really don't agree with pushing this into a userspace library
> plugin - at least not with the current state there.
> 
> _At least_ the debayering in the v4l libraries needs to become optional.
> 

I *thought* that when a plugin is used the format conversion code was disabled.
But I'm not sure.

The whole problem is that we still don't have a decent plugin for an MC driver.
There is one for the exynos4 floating around, but it's still not accepted.

Companies write the driver, but the plugin isn't really needed since their
customers won't use it anyway since they make their own embedded driver.

And nobody of the media core developers has the time to work on the docs,
utilities and libraries you need to make this all work cleanly and reliably.

As mentioned, I will attempt to try and get some time to work on this
later this year. Fingers crossed.

We also have a virtual MC driver floating around. I've pinged the author if
she can fix the last round of review comments and post a new version. Having
a virtual driver makes life much easier when writing docs, utilities, etc.
since you don't need real hardware which can be hard to obtain and run.

Again, I agree completely with you. But we don't have many core developers
who can do something like this, and it's even harder for them to find the time.

Solutions on a postcard...

BTW, Steve: this has nothing to do with your work, it's a problem in our 
subsystem.

Regards,

Hans

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Fri, Mar 10, 2017 at 01:54:28PM +0100, Hans Verkuil wrote:
> But there was always meant to be a layer (libv4l plugin) that could be
> used to setup a 'default scenario' that existing applications could use,
> but that was never enforced, sadly.

However, there's other painful issues lurking in userspace, particularly
to do with the v4l libraries.

The idea that the v4l libraries should intercept the format negotiation
between the application and kernel is a particularly painful one - the
default gstreamer build detects the v4l libraries, and links against it.
That much is fine.

However, the problem comes when you're trying to use bayer formats. The
v4l libraries "helpfully" (or rather unhelpfully) intercept the format
negotiation, and decide that they'll invoke v4lconvert to convert the
bayer to RGB for you, whether you want them to do that or not.

v4lconvert may not be the most efficient way to convert, or even what
is desired (eg, you may want to receive the raw bayer image.)  However,
since the v4l libraries/v4lconvert gives you no option but to have its
conversion forced into the pipeline, other options (such as using the
gstreamer neon accelerated de-bayer plugin) isn't an option without
rebuilding gstreamer _without_ linking against the v4l libraries.

At that point, saying "this should be done in a libv4l plugin" becomes
a total nonsense, because if you need to avoid libv4l due to its
stupidities, you don't get the benefit of subdevs, and it yet again
_forces_ people down the route of custom applications.

So, I really don't agree with pushing this into a userspace library
plugin - at least not with the current state there.

_At least_ the debayering in the v4l libraries needs to become optional.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v6] [media] vimc: Virtual Media Controller core, capture and sensor

[Hans Verkuil]

Hi Helen,

On 11/01/17 02:30, Helen Koike wrote:
> 
> Thank you for the review, I'll update the patch accordingly and re-submit it.
> 
> Helen

Do you know when you have a v7 ready?

We really need a vimc driver so people without hardware can actually fiddle 
around
with the MC.

See also my rant here (not directed at you!):

https://www.spinics.net/lists/kernel/msg2462009.html

Regards,

Hans

[PATCH 0/2] staging: media: Remove parentheses from return arguments

[simran singhal]

This patch-series removes unnecessary parantheses from return arguments.

simran singhal (2):
  staging: css2400/sh_css: Remove parentheses from return arguments
  staging: sh_css_firmware: Remove parentheses from return arguments

 .../media/atomisp/pci/atomisp2/css2400/sh_css.c  | 20 ++--
 .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c   |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 2/2] staging: sh_css_firmware: Remove parentheses from return arguments

[simran singhal]

The sematic patch used for this is:
@@
identifier i;
constant c;
@@
return
- (
\(i\|-i\|i(...)\|c\)
- )
  ;

Signed-off-by: simran singhal 
Acked-by: Julia Lawall 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
index b294e6d..0d7e8cd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
@@ -74,7 +74,7 @@ static struct fw_param *fw_minibuffer;
 
 char *sh_css_get_fw_version(void)
 {
-   return(FW_rel_ver_name);
+   return FW_rel_ver_name;
 }
 
 
-- 
2.7.4

[PATCH 1/2] staging: css2400/sh_css: Remove parentheses from return arguments

[simran singhal]

The sematic patch used for this is:
@@
identifier i;
constant c;
@@
return
- (
\(i\|-i\|i(...)\|c\)
- )
  ;

Signed-off-by: simran singhal 
Acked-by: Julia Lawall 
---
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c  | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
index 0a1544d..c442d22 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
@@ -1989,7 +1989,7 @@ enum ia_css_err ia_css_suspend(void)
for(i=0;i after 1: seed %d 
(%p)\n", i, my_css_save.stream_seeds[i].stream);
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_suspend() leave\n");
-   return(IA_CSS_SUCCESS);
+   return IA_CSS_SUCCESS;
 }
 
 enum ia_css_err
@@ -2001,10 +2001,10 @@ ia_css_resume(void)
 
err = ia_css_init(&(my_css_save.driver_env), my_css_save.loaded_fw, 
my_css_save.mmu_base, my_css_save.irq_type);
if (err != IA_CSS_SUCCESS)
-   return(err);
+   return err;
err = ia_css_start_sp();
if (err != IA_CSS_SUCCESS)
-   return(err);
+   return err;
my_css_save.mode = sh_css_mode_resume;
for(i=0;i

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 04/03/17 14:13, Sakari Ailus wrote:
> Hi Russell,
> 
> On Fri, Mar 03, 2017 at 11:06:45PM +, Russell King - ARM Linux wrote:
>> On Thu, Mar 02, 2017 at 06:02:57PM +0200, Sakari Ailus wrote:
>>> Hi Steve,
>>>
>>> On Wed, Feb 15, 2017 at 06:19:16PM -0800, Steve Longerbeam wrote:
 v4l2_pipeline_inherit_controls() will add the v4l2 controls from
 all subdev entities in a pipeline to a given video device.

 Signed-off-by: Steve Longerbeam 
 ---
  drivers/media/v4l2-core/v4l2-mc.c | 48 
 +++
  include/media/v4l2-mc.h   | 25 
  2 files changed, 73 insertions(+)

 diff --git a/drivers/media/v4l2-core/v4l2-mc.c 
 b/drivers/media/v4l2-core/v4l2-mc.c
 index 303980b..09d4d97 100644
 --- a/drivers/media/v4l2-core/v4l2-mc.c
 +++ b/drivers/media/v4l2-core/v4l2-mc.c
 @@ -22,6 +22,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -238,6 +239,53 @@ int v4l_vb2q_enable_media_source(struct vb2_queue *q)
  }
  EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
  
 +int __v4l2_pipeline_inherit_controls(struct video_device *vfd,
 +   struct media_entity *start_entity)
>>>
>>> I have a few concerns / questions:
>>>
>>> - What's the purpose of this patch? Why not to access the sub-device node
>>>   directly?
>>
>> What tools are in existance _today_ to provide access to these controls
>> via the sub-device nodes?
> 
> yavta, for instance:
> 
> 
> 
> VIDIOC_QUERYCAP isn't supported on sub-devices and v4l2-ctl appears to be
> checking for that. That check should be removed (with possible other
> implications taken into account).

No, the subdev API should get a similar QUERYCAP ioctl. There isn't a single
ioctl that is guaranteed to be available for all subdev devices. I've made
proposals for this in the past, and those have all been shot down.

Add that, and I'll add support for subdevs in v4l2-ctl.

> 
>>
>> v4l-tools doesn't last time I looked - in fact, the only tool in v4l-tools
>> which is capable of accessing the subdevices is media-ctl, and that only
>> provides functionality for configuring the pipeline.
>>
>> So, pointing people at vapourware userspace is really quite rediculous.
> 
> Do bear in mind that there are other programs that can make use of these
> interfaces. It's not just the test programs, or a test program you attempted
> to use.
> 
>>
>> The established way to control video capture is through the main video
>> capture device, not through the sub-devices.  Yes, the controls are
>> exposed through sub-devices too, but that does not mean that is the
>> correct way to access them.
> 
> It is. That's the very purpose of the sub-devices: to provide access to the
> hardware independently of how the links are configured.
> 
>>
>> The v4l2 documentation (Documentation/media/kapi/v4l2-controls.rst)
>> even disagrees with your statements.  That talks about control
>> inheritence from sub-devices to the main video device, and the core
>> v4l2 code provides _automatic_ support for this - see
>> v4l2_device_register_subdev():
>>
>> /* This just returns 0 if either of the two args is NULL */
>> err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, 
>> sd->ctrl_handler, NULL);
>>
>> which merges the subdev's controls into the main device's control
>> handler.
> 
> That's done on different kind of devices: those that provide plain V4L2 API
> to control the entire device. V4L2 sub-device interface is used *in kernel*
> as an interface to control sub-devices that do not need to be exposed to the
> user space.
> 
> Devices that have complex pipeline that do essentially require using the
> Media controller interface to configure them are out of that scope.
> 

Way too much of how the MC devices should be used is in the minds of developers.
There is a major lack for good detailed documentation, utilities, compliance
test (really needed!) and libv4l plugins.

Russell's comments are spot on and it is a thorn in my side that this still
hasn't been addressed.

I want to see if I can get time from my boss to work on this this summer, but
there is no guarantee.

The main reason this hasn't been a much bigger problem is that most end-users
make custom applications for this hardware. It makes sense, if you need full
control over everything you make the application yourself, that's the whole 
point.

But there was always meant to be a layer (libv4l plugin) that could be used to
setup a 'default scenario' that existing applications could use, but that was
never enforced, sadly.

Anyway, regarding this specific patch and for this MC-aware driver: no, you
shouldn't inherit controls from subdevs. It defeats the purpose.

Regards,

Hans

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Fri, Mar 10, 2017 at 10:31:13AM +, Brian Starkey wrote:
> Hi,
> 
> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
> > On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 
> [snip]
> 
> > > 
> > > For me those patches are going in the right direction.
> > > 
> > > I still have few questions:
> > > - since alignment management has been remove from ion-core, should it
> > > be also removed from ioctl structure ?
> > 
> > Yes, I think I'm going to go with the suggestion to fixup the ABI
> > so we don't need the compat layer and as part of that I'm also
> > dropping the align argument.
> > 
> 
> Is the only motivation for removing the alignment parameter that
> no-one got around to using it for something useful yet?
> The original comment was true - different devices do have different
> alignment requirements.
> 
> Better alignment can help SMMUs use larger blocks when mapping,
> reducing TLB pressure and the chance of a page table walk causing
> display underruns.

Extending ioctl uapi is easy, trying to get rid of bad uapi is much
harder. Given that right now we don't have an ion allocator that does
alignment I think removing it makes sense. And if we go with lots of
heaps, we might as well have an ion heap per alignment that your hw needs,
so there's different ways to implement this in the future.

At least from the unix device memory allocator pov it's probably simpler
to encode stuff like this into the heap name, instead of having to pass
heap + list of additional properties/constraints.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v5 16/39] [media] v4l2: add a new-frame before end-of-frame event

[Hans Verkuil]

On 10/03/17 05:52, Steve Longerbeam wrote:
> Add a NEW_FRAME_BEFORE_EOF event to signal that a video capture or
> output device has signaled a new frame is ready before a previous
> frame has completed reception or transmission. This usually indicates
> a DMA read/write channel is having trouble gaining bus access.

This too is a weird event. Based on what you describe this basically means
that the previous frame is incomplete, in which case you would typically
return the buffer with the V4L2_BUF_FLAG_ERROR bit set.

Using an event for this is not a good idea.

Regards,

Hans

> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++
>  Documentation/media/videodev2.h.rst.exceptions  | 1 +
>  include/uapi/linux/videodev2.h  | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
> b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> index dc77363..54bc7ae 100644
> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> @@ -203,6 +203,12 @@ call.
>   has measured an interval between the reception or transmit
>   completion of two consecutive frames of video that is outside
>   the nominal frame interval by some tolerance value.
> +* - ``V4L2_EVENT_NEW_FRAME_BEFORE_EOF``
> +  - 8
> +  - This event is triggered when the video capture or output device
> + has signaled a new frame is ready before a previous frame has
> + completed reception or transmission. This usually indicates a
> + DMA read/write channel is having trouble gaining bus access.
>  * - ``V4L2_EVENT_PRIVATE_START``
>- 0x0800
>- Base event number for driver-private events.
> diff --git a/Documentation/media/videodev2.h.rst.exceptions 
> b/Documentation/media/videodev2.h.rst.exceptions
> index c7d8fad..be6f332 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -460,6 +460,7 @@ replace define V4L2_EVENT_FRAME_SYNC event-type
>  replace define V4L2_EVENT_SOURCE_CHANGE event-type
>  replace define V4L2_EVENT_MOTION_DET event-type
>  replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
> +replace define V4L2_EVENT_NEW_FRAME_BEFORE_EOF event-type
>  replace define V4L2_EVENT_PRIVATE_START event-type
>  
>  replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index cf5a0d0..f54a82a 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2132,6 +2132,7 @@ struct v4l2_streamparm {
>  #define V4L2_EVENT_SOURCE_CHANGE 5
>  #define V4L2_EVENT_MOTION_DET6
>  #define V4L2_EVENT_FRAME_INTERVAL_ERROR  7
> +#define V4L2_EVENT_NEW_FRAME_BEFORE_EOF  8
>  #define V4L2_EVENT_PRIVATE_START 0x0800
>  
>  /* Payload for V4L2_EVENT_VSYNC */
> 

Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

[Hans Verkuil]

On 10/03/17 05:52, Steve Longerbeam wrote:
> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
> output device has measured an interval between the reception or transmit
> completion of two consecutive frames of video that is outside the nominal
> frame interval by some tolerance value.

Reading back what was said on this I agree with Sakari that this doesn't
belong here.

Userspace can detect this just as easily (if not easier) with a timeout.

Regards,

Hans

> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++
>  Documentation/media/videodev2.h.rst.exceptions  | 1 +
>  include/uapi/linux/videodev2.h  | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
> b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> index 8d663a7..dc77363 100644
> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> @@ -197,6 +197,12 @@ call.
>   the regions changes. This event has a struct
>   :c:type:`v4l2_event_motion_det`
>   associated with it.
> +* - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
> +  - 7
> +  - This event is triggered when the video capture or output device
> + has measured an interval between the reception or transmit
> + completion of two consecutive frames of video that is outside
> + the nominal frame interval by some tolerance value.
>  * - ``V4L2_EVENT_PRIVATE_START``
>- 0x0800
>- Base event number for driver-private events.
> diff --git a/Documentation/media/videodev2.h.rst.exceptions 
> b/Documentation/media/videodev2.h.rst.exceptions
> index e11a0d0..c7d8fad 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
>  replace define V4L2_EVENT_FRAME_SYNC event-type
>  replace define V4L2_EVENT_SOURCE_CHANGE event-type
>  replace define V4L2_EVENT_MOTION_DET event-type
> +replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
>  replace define V4L2_EVENT_PRIVATE_START event-type
>  
>  replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45184a2..cf5a0d0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
>  #define V4L2_EVENT_FRAME_SYNC4
>  #define V4L2_EVENT_SOURCE_CHANGE 5
>  #define V4L2_EVENT_MOTION_DET6
> +#define V4L2_EVENT_FRAME_INTERVAL_ERROR  7
>  #define V4L2_EVENT_PRIVATE_START 0x0800
>  
>  /* Payload for V4L2_EVENT_VSYNC */
> 

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 12:25:54PM +0100, Hans Verkuil wrote:
> Slight problem with this. If I make this change, then the of_node_put below
> changes as well:
> 
> @@ -1193,7 +1176,7 @@ static int isi_graph_init(struct atmel_isi *isi)
>  done:
> if (ret < 0) {
> v4l2_async_notifier_unregister(>notifier);
> -   of_node_put(isi->entity.node);
> +   of_node_put(isi->entity.asd.match.of.node);
> }
> 
> And I get this compiler warning:
> 
>   CC [M]  drivers/media/platform/atmel/atmel-isi.o
> drivers/media/platform/atmel/atmel-isi.c: In function ‘isi_graph_init’:
> drivers/media/platform/atmel/atmel-isi.c:1179:15: warning: passing argument 1 
> of ‘of_node_put’ discards ‘const’ qualifier from pointer target type 
> [-Wdiscarded-qualifiers]
>of_node_put(isi->entity.asd.match.of.node);
>^~~
> In file included from drivers/media/platform/atmel/atmel-isi.c:25:0:
> ./include/linux/of.h:130:20: note: expected ‘struct device_node *’ but 
> argument is of type ‘const struct device_node *’
>  static inline void of_node_put(struct device_node *node) { }
> ^~~
> 
> 
> Any suggestions? Just keep the entity.node after all?

Yeah, makes sense, I didn't come to think of that.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v5 22/39] media: Add userspace header file for i.MX

[Hans Verkuil]

On 10/03/17 05:53, Steve Longerbeam wrote:
> This adds a header file for use by userspace programs wanting to interact
> with the i.MX media driver. It defines custom v4l2 controls for the
> i.MX v4l2 subdevices.
> 
> Signed-off-by: Steve Longerbeam 

I would not export this while the imx driver is in staging.

Also, traditionally exported media headers are in linux, not media.

I would like to have a discussion about that before deciding where to
place this header.

For the record: I am not opposed to placing this in media.

Regards,

Hans

> ---
>  include/uapi/media/Kbuild |  1 +
>  include/uapi/media/imx.h  | 21 +
>  2 files changed, 22 insertions(+)
>  create mode 100644 include/uapi/media/imx.h
> 
> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> index aafaa5a..fa78958 100644
> --- a/include/uapi/media/Kbuild
> +++ b/include/uapi/media/Kbuild
> @@ -1 +1,2 @@
>  # UAPI Header export list
> +header-y += imx.h
> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> new file mode 100644
> index 000..f573de4
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version
> + */
> +
> +#ifndef __UAPI_MEDIA_IMX_H__
> +#define __UAPI_MEDIA_IMX_H__
> +
> +enum imx_ctrl_id {
> + V4L2_CID_IMX_FIM_ENABLE = (V4L2_CID_USER_IMX_BASE + 0),
> + V4L2_CID_IMX_FIM_NUM,
> + V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> + V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> + V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +
> +#endif
> 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Robin Murphy]

On 10/03/17 10:31, Brian Starkey wrote:
> Hi,
> 
> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 
> [snip]
> 
>>>
>>> For me those patches are going in the right direction.
>>>
>>> I still have few questions:
>>> - since alignment management has been remove from ion-core, should it
>>> be also removed from ioctl structure ?
>>
>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>> so we don't need the compat layer and as part of that I'm also
>> dropping the align argument.
>>
> 
> Is the only motivation for removing the alignment parameter that
> no-one got around to using it for something useful yet?
> The original comment was true - different devices do have different
> alignment requirements.
> 
> Better alignment can help SMMUs use larger blocks when mapping,
> reducing TLB pressure and the chance of a page table walk causing
> display underruns.

For that use-case, though, alignment alone doesn't necessarily help -
you need the whole allocation granularity to match your block size (i.e.
given a 1MB block size, asking for 17KB and getting back 17KB starting
at a 1MB boundary doesn't help much - that whole 1MB needs to be
allocated and everyone needs to know it to ensure that the whole lot can
be mapped safely). Now, whether it's down to the callers or the heap
implementations to decide and enforce that granularity is another
question, but provided allocations are at least naturally aligned to
whatever the granularity is (which is a reasonable assumption to bake
in) then it's all good.

Robin.

> 
> -Brian
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 17/39] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 10/03/17 05:52, Steve Longerbeam wrote:
> v4l2_pipeline_inherit_controls() will add the v4l2 controls from
> all subdev entities in a pipeline to a given video device.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/media/v4l2-core/v4l2-mc.c | 48 
> +++
>  include/media/v4l2-mc.h   | 25 
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c 
> b/drivers/media/v4l2-core/v4l2-mc.c
> index 303980b..09d4d97 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -238,6 +239,53 @@ int v4l_vb2q_enable_media_source(struct vb2_queue *q)
>  }
>  EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
>  
> +int __v4l2_pipeline_inherit_controls(struct video_device *vfd,
> +  struct media_entity *start_entity)
> +{
> + struct media_device *mdev = start_entity->graph_obj.mdev;
> + struct media_entity *entity;
> + struct media_graph graph;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + ret = media_graph_walk_init(, mdev);
> + if (ret)
> + return ret;
> +
> + media_graph_walk_start(, start_entity);
> +
> + while ((entity = media_graph_walk_next())) {
> + if (!is_media_entity_v4l2_subdev(entity))
> + continue;
> +
> + sd = media_entity_to_v4l2_subdev(entity);
> +
> + ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
> + sd->ctrl_handler,
> + NULL);
> + if (ret)
> + break;
> + }
> +
> + media_graph_walk_cleanup();
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(__v4l2_pipeline_inherit_controls);
> +
> +int v4l2_pipeline_inherit_controls(struct video_device *vfd,
> +struct media_entity *start_entity)
> +{
> + struct media_device *mdev = start_entity->graph_obj.mdev;
> + int ret;
> +
> + mutex_lock(>graph_mutex);
> + ret = __v4l2_pipeline_inherit_controls(vfd, start_entity);
> + mutex_unlock(>graph_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_pipeline_inherit_controls);
> +
>  /* 
> -
>   * Pipeline power management
>   *
> diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> index 2634d9d..9848e77 100644
> --- a/include/media/v4l2-mc.h
> +++ b/include/media/v4l2-mc.h
> @@ -171,6 +171,17 @@ void v4l_disable_media_source(struct video_device *vdev);
>   */
>  int v4l_vb2q_enable_media_source(struct vb2_queue *q);
>  
> +/**
> + * v4l2_pipeline_inherit_controls - Add the v4l2 controls from all
> + *   subdev entities in a pipeline to
> + *   the given video device.
> + * @vfd: the video device
> + * @start_entity: Starting entity
> + */
> +int __v4l2_pipeline_inherit_controls(struct video_device *vfd,
> +  struct media_entity *start_entity);
> +int v4l2_pipeline_inherit_controls(struct video_device *vfd,
> +struct media_entity *start_entity);

Please document which is the unlocked variant and which lock the locked
variant takes.

Regards,

Hans

>  
>  /**
>   * v4l2_pipeline_pm_use - Update the use count of an entity
> @@ -231,6 +242,20 @@ static inline int v4l_vb2q_enable_media_source(struct 
> vb2_queue *q)
>   return 0;
>  }
>  
> +static inline int __v4l2_pipeline_inherit_controls(
> + struct video_device *vfd,
> + struct media_entity *start_entity)
> +{
> + return 0;
> +}
> +
> +static inline int v4l2_pipeline_inherit_controls(
> + struct video_device *vfd,
> + struct media_entity *start_entity)
> +{
> + return 0;
> +}
> +
>  static inline int v4l2_pipeline_pm_use(struct media_entity *entity, int use)
>  {
>   return 0;
> 

[PATCH 8/8] atomisp: remove FPGA defines

[Alan Cox]

These are not relevant to an upstream kernel driver.

Signed-off-by: Alan Cox 
---
 .../host/hive_isp_css_hrt_modified.h   |   16 
 1 file changed, 16 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/hive_isp_css_hrt_modified.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/hive_isp_css_hrt_modified.h
index 603ef1d..342553d 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/hive_isp_css_hrt_modified.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/hive_isp_css_hrt_modified.h
@@ -30,21 +30,6 @@
 #include 
 #include 
 #include 
-#ifdef _HIVE_ISP_CSS_FPGA_SYSTEM
-  #include 
-  #include 
-  #include 
-  #include 
-  #include 
-  #include 
-#define hrt_gdc_slave_port(gdc_id)HRTCAT(gdc_id,_sl_in)
-  #include 
-  #include 
-  #include "isp_css_dev_flash_hrt.h"
-  #include "isp_css_dev_display_hrt.h"
-  #include "isp_css_dev_i2c_hrt.h"
-  #include "isp_css_dev_tb.h"
-#else /* CSS ASIC system */
   #include 
 //  #include 
 //  #include 
@@ -63,7 +48,6 @@
 #error "hive_isp_css_hrt_modified.h: SYSTEM must be one of 
{2400_MAMOIADA_SYSTEM, 2401_MAMOIADA_SYSTEM}"
 #endif
   #endif
-#endif /* _HIVE_ISP_CSS_FPGA_SYSTEM */
 #include 
 #include 
 #include 

[PATCH 6/8] atomisp: tidy firmware loading code a little

[Alan Cox]

The FWNAME define is never used so can be removed. The option to skip firmware
loading isn't really Cherrytrail specific so remove this and complete the
merging of the two driver versions for this file.

Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  |   10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index 97103b4..755c27c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -51,12 +51,10 @@
 /* G-Min addition: pull this in from intel_mid_pm.h */
 #define CSTATE_EXIT_LATENCY_C1  1
 
-#ifdef ISP2401
 static uint skip_fwload = 0;
 module_param(skip_fwload, uint, 0644);
-MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load for COS");
+MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load");
 
-#endif
 /* set reserved memory pool size in page */
 unsigned int repool_pgnr;
 module_param(repool_pgnr, uint, 0644);
@@ -1088,14 +1086,9 @@ atomisp_load_firmware(struct atomisp_device *isp)
int rc;
char *fw_path = NULL;
 
-#ifdef ISP2401
if (skip_fwload)
return NULL;
 
-#endif
-#if defined(ATOMISP_FWNAME)
-   fw_path = ATOMISP_FWNAME;
-#else
if (isp->media_dev.driver_version == ATOMISP_CSS_VERSION_21) {
if (isp->media_dev.hw_revision ==
((ATOMISP_HW_REVISION_ISP2401 << ATOMISP_HW_REVISION_SHIFT)
@@ -1112,7 +1105,6 @@ atomisp_load_firmware(struct atomisp_device *isp)
 | ATOMISP_HW_STEPPING_B0))
fw_path = "shisp_2400b0_v21.bin";
}
-#endif
 
if (!fw_path) {
dev_err(isp->dev,

[PATCH 5/8] atomisp: eliminate intel_mid_pm.h

[Alan Cox]

We can do this because the only thing it is used for is identifying the
platform for power management purposes. The driver only supports Baytrail
and Cherrytrail and both of those always need the IPU to be power managed
directly not via PCI D3 states.

Signed-off-by: Alan Cox 
---
 .../media/atomisp/include/linux/intel_mid_pm.h |  233 
 .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  |   33 +--
 2 files changed, 10 insertions(+), 256 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/include/linux/intel_mid_pm.h

diff --git a/drivers/staging/media/atomisp/include/linux/intel_mid_pm.h 
b/drivers/staging/media/atomisp/include/linux/intel_mid_pm.h
deleted file mode 100644
index 82f8b57..000
--- a/drivers/staging/media/atomisp/include/linux/intel_mid_pm.h
+++ /dev/null
@@ -1,233 +0,0 @@
-/*
- * intel_mid_pm.h
- * Copyright (c) 2010, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- */
-#include 
-
-#ifndef INTEL_MID_PM_H
-#define INTEL_MID_PM_H
-
-#include "../asm/intel-mid.h"
-#include 
-#include 
-
-
-/* Chip ID of Intel Atom SOC*/
-#define INTEL_ATOM_MRST 0x26
-#define INTEL_ATOM_MFLD 0x27
-#define INTEL_ATOM_CLV 0x35
-#define INTEL_ATOM_MRFLD 0x4a
-#define INTEL_ATOM_BYT 0x37
-#define INTEL_ATOM_MOORFLD 0x5a
-#define INTEL_ATOM_CHT 0x4c
-
-static inline int platform_is(u8 model)
-{
-   return boot_cpu_data.x86_model == model;
-}
-
-/* Register Type definitions */
-#define OSPM_REG_TYPE  0x0
-#define APM_REG_TYPE   0x1
-#define OSPM_MAX_POWER_ISLANDS 16
-#define OSPM_ISLAND_UP 0x0
-#define OSPM_ISLAND_DOWN   0x1
-/*Soft reset*/
-#define OSPM_ISLAND_SR 0x2
-
-/* North complex power islands definitions for APM block*/
-#define APM_GRAPHICS_ISLAND0x1
-#define APM_VIDEO_DEC_ISLAND   0x2
-#define APM_VIDEO_ENC_ISLAND   0x4
-#define APM_GL3_CACHE_ISLAND   0x8
-#define APM_ISP_ISLAND 0x10
-#define APM_IPH_ISLAND 0x20
-
-/* North complex power islands definitions for OSPM block*/
-#define OSPM_DISPLAY_A_ISLAND  0x2
-#define OSPM_DISPLAY_B_ISLAND  0x80
-#define OSPM_DISPLAY_C_ISLAND  0x100
-#define OSPM_MIPI_ISLAND   0x200
-
-/* North Complex power islands definitions for Tangier */
-#define TNG_ISP_ISLAND 0x1
-/* North Complex Register definitions for Tangier */
-#defineISP_SS_PM0  0x39
-
-#define C4_STATE_IDX   3
-#define C6_STATE_IDX   4
-#define S0I1_STATE_IDX  5
-#define LPMP3_STATE_IDX 6
-#define S0I3_STATE_IDX  7
-
-#define C4_HINT(0x30)
-#define C6_HINT(0x52)
-
-#define CSTATE_EXIT_LATENCY_C1  1
-#define CSTATE_EXIT_LATENCY_C2  20
-#define CSTATE_EXIT_LATENCY_C4  100
-#define CSTATE_EXIT_LATENCY_C6  140
-#define CSTATE_EXIT_LATENCY_C7  1200
-
-/* Since entry latency is substantial
- * put exit_latency = entry+exit latency
- */
-#ifdef CONFIG_REMOVEME_INTEL_ATOM_MRFLD_POWER
-#define CSTATE_EXIT_LATENCY_S0i1 1200
-#define CSTATE_EXIT_LATENCY_S0i2 2000
-#define CSTATE_EXIT_LATENCY_S0i3 1
-#else
-#define CSTATE_EXIT_LATENCY_LPMP3 1040
-#define CSTATE_EXIT_LATENCY_S0i1 1040
-#define CSTATE_EXIT_LATENCY_S0i3 2800
-#endif
-#define BYT_S0I1_STATE 0x60
-#define BYT_S0I2_STATE 0x62
-#define BYT_LPMP3_STATE0x62
-#define BYT_S0I3_STATE 0x64
-
-enum s3_parts {
-   PROC_FRZ,
-   DEV_SUS,
-   NB_CPU_OFF,
-   NB_CPU_ON,
-   DEV_RES,
-   PROC_UNFRZ,
-   MAX_S3_PARTS
-};
-
-#ifdef CONFIG_ATOM_SOC_POWER
-#define LOG_PMU_EVENTS
-
-/* Error codes for pmu */
-#definePMU_SUCCESS 0
-#define PMU_FAILED -1
-#define PMU_BUSY_STATUS0
-#define PMU_MODE_ID1
-#defineSET_MODE1
-#defineSET_AOAC_S0i1   2
-#defineSET_AOAC_S0i3   3
-#defineSET_LPAUDIO 4
-#defineSET_AOAC_S0i2   7
-
-#ifdef CONFIG_REMOVEME_INTEL_ATOM_MRFLD_POWER
-#define MID_S0I1_STATE 0x60
-#define MID_S0I2_STATE 0x62
-#define MID_LPMP3_STATE0x62
-#define MID_S0I3_STATE 0x64
-#else
-#define MID_S0I1_STATE 0x1
-#define MID_LPMP3_STATE0x3
-#define MID_S0I2_STATE 0x7
-#define MID_S0I3_STATE 0x7
-#endif
-
-#define MID_S0IX_STATE 0xf
-#define MID_S3_STATE   0x1f
-#define MID_FAST_ON_OFF_STATE  0x3f
-
-/* combinations */
-#define MID_LPI1_STATE 0x1f
-#define MID_LPI3_STATE 0x7f
-#define MID_I1I3_STATE 

[PATCH 7/8] atomisp: remove pdaf kernel

[Alan Cox]

This is not used on either Baytrail or Cherrytrail so can simply be deleted


Signed-off-by: Alan Cox 
---
 .../staging/media/atomisp/pci/atomisp2/Makefile|2 
 .../css2400/isp/kernels/pdaf/ia_css_pdaf.host.c|   84 
 .../css2400/isp/kernels/pdaf/ia_css_pdaf.host.h|   37 -
 .../css2400/isp/kernels/pdaf/ia_css_pdaf_param.h   |   62 ---
 .../css2400/isp/kernels/pdaf/ia_css_pdaf_types.h   |   54 -
 5 files changed, 239 deletions(-)
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf.host.c
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf.host.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf_param.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf_types.h

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/Makefile 
b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
index ccde4f3..b8a841e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
@@ -97,7 +97,6 @@ atomisp-objs += \
./css2400/isp/kernels/crop/crop_1.0/ia_css_crop.host.o \
./css2400/isp/kernels/io_ls/bayer_io_ls/ia_css_bayer_io.host.o \
./css2400/isp/kernels/aa/aa_2/ia_css_aa2.host.o \
-   ./css2400/isp/kernels/pdaf/ia_css_pdaf.host.o \

./css2400/isp/kernels/copy_output/copy_output_1.0/ia_css_copy_output.host.o \
./css2400/isp/kernels/ob/ob_1.0/ia_css_ob.host.o \
./css2400/isp/kernels/ob/ob2/ia_css_ob2.host.o \
@@ -300,7 +299,6 @@ INCLUDES += \
-I$(atomisp)/css2400/isp/kernels/ob/ob2/ \
-I$(atomisp)/css2400/isp/kernels/output/ \
-I$(atomisp)/css2400/isp/kernels/output/output_1.0/ \
-   -I$(atomisp)/css2400/isp/kernels/pdaf/ \
-I$(atomisp)/css2400/isp/kernels/qplane/ \
-I$(atomisp)/css2400/isp/kernels/qplane/qplane_2/ \
-I$(atomisp)/css2400/isp/kernels/raw_aa_binning/ \
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf.host.c
deleted file mode 100644
index 79ddef6..000
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/pdaf/ia_css_pdaf.host.c
+++ /dev/null
@@ -1,84 +0,0 @@
-#ifdef ISP2600
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-#include "ia_css_types.h"
-#include "sh_css_defs.h"
-#include "sh_css_frac.h"
-#include "ia_css_pdaf.host.h"
-
-const struct ia_css_pdaf_config default_pdaf_config;
-
-void
-ia_css_pdaf_dmem_encode(
-   struct isp_pdaf_dmem_params *to,
-   const struct ia_css_pdaf_config *from,
-   unsigned size)
-{
-   (void)size;
-   to->frm_length = from->frm_length;
-   to->frm_width  = from->frm_width;
-
-   to->ext_cfg_l_data.num_valid_patterns = 
from->ext_cfg_data.l_pixel_grid.num_valid_patterns;
-
-   to->ext_cfg_r_data.num_valid_patterns = 
from->ext_cfg_data.r_pixel_grid.num_valid_patterns;
-
-   to->stats_calc_data.num_valid_elm = 
from->stats_calc_cfg_data.num_valid_elm;
-}
-
-void
-ia_css_pdaf_vmem_encode(
-   struct isp_pdaf_vmem_params *to,
-   const struct ia_css_pdaf_config *from,
-   unsigned size)
-{
-
-   unsigned int i;
-   (void)size;
-   /* Initialize left pixel grid */
-   for ( i=0 ; i < from->ext_cfg_data.l_pixel_grid.num_valid_patterns ; 
i++) {
-
-   to->ext_cfg_l_data.y_offset[0][i] = 
from->ext_cfg_data.l_pixel_grid.y_offset[i];
-   to->ext_cfg_l_data.x_offset[0][i] = 
from->ext_cfg_data.l_pixel_grid.x_offset[i];
-   to->ext_cfg_l_data.y_step_size[0][i] = 
from->ext_cfg_data.l_pixel_grid.y_step_size[i];
-   to->ext_cfg_l_data.x_step_size[0][i] = 
from->ext_cfg_data.l_pixel_grid.x_step_size[i];
-   }
-
-   for ( ; i < ISP_NWAY ; i++) {
-
-   to->ext_cfg_l_data.y_offset[0][i] = PDAF_INVALID_VAL;
-   to->ext_cfg_l_data.x_offset[0][i] = PDAF_INVALID_VAL;
-   to->ext_cfg_l_data.y_step_size[0][i] = PDAF_INVALID_VAL;
-   to->ext_cfg_l_data.x_step_size[0][i] = PDAF_INVALID_VAL;
-   }
-
-   /* Initialize left pixel grid */
-
-   for ( i=0 ; i < 

[PATCH 3/8] atomisp: remove HIVECC

[Alan Cox]

We are only going to be building for Linux with gcc, so we can lose bits of
material related to other build targets.

Signed-off-by: Alan Cox 
---
 .../css2400/hive_isp_css_common/dma_global.h   |   52 
 .../css2400/hive_isp_css_include/assert_support.h  |9 ---
 .../css2400/hive_isp_css_include/error_support.h   |7 ---
 .../css2400/hive_isp_css_include/storage_class.h   |2 -
 .../css2400/hive_isp_css_include/type_support.h|   11 
 .../pci/atomisp2/css2400/ia_css_device_access.h|6 --
 .../css2400/isp/modes/interface/isp_const.h|8 ---
 .../css2400/runtime/debug/src/ia_css_debug.c   |2 -
 .../atomisp/pci/atomisp2/css2400/sh_css_internal.h |2 -
 9 files changed, 1 insertion(+), 98 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/dma_global.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/dma_global.h
index dff6110..60d6de1 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/dma_global.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/dma_global.h
@@ -93,38 +93,6 @@ typedef enum {
DMA_PACK_WIDTH_B(width_b) | \
DMA_PACK_HEIGHT(height))
 
-#ifdef __HIVECC
-#define hive_dma_move_data(dma_id, read, channel, addr_a, addr_b, to_is_var, 
from_is_var) \
-{ \
-  /*hive_dma_snd(dma_id, 
DMA_PACK_CMD_CHANNEL(read?_DMA_V2_MOVE_B2A_COMMAND:_DMA_V2_MOVE_A2B_COMMAND, 
channel)); \
-  hive_dma_snd(dma_id, read?(unsigned)(addr_b):(unsigned)(addr_a)); \
-  hive_dma_snd(dma_id, read?(unsigned)(addr_a):(unsigned)(addr_b)); */\
-  if (read) { \
-hive_dma_snd(dma_id, DMA_PACK_CMD_CHANNEL(_DMA_V2_MOVE_B2A_COMMAND, 
channel)); \
-hive_dma_snd(dma_id, (unsigned)(addr_b)); \
-hive_dma_snd(dma_id, (unsigned)(addr_a)); \
-  } else { \
-hive_dma_snd(dma_id, DMA_PACK_CMD_CHANNEL(_DMA_V2_MOVE_A2B_COMMAND, 
channel)); \
-hive_dma_snd(dma_id, (unsigned)(addr_a)); \
-hive_dma_snd(dma_id, (unsigned)(addr_b)); \
-  } \
-}
-#define hive_dma_move_data_no_ack(dma_id, read, channel, addr_a, addr_b, 
to_is_var, from_is_var) \
-{ \
-  /*hive_dma_snd(dma_id, 
DMA_PACK_CMD_CHANNEL(read?_DMA_V2_NO_ACK_MOVE_B2A_NO_SYNC_CHK_COMMAND:_DMA_V2_NO_ACK_MOVE_A2B_NO_SYNC_CHK_COMMAND,
 channel)); \
-  hive_dma_snd(dma_id, read?(unsigned)(addr_b):(unsigned)(addr_a)); \
-  hive_dma_snd(dma_id, read?(unsigned)(addr_a):(unsigned)(addr_b)); */\
-  if (read) { \
-hive_dma_snd(dma_id, 
DMA_PACK_CMD_CHANNEL(_DMA_V2_NO_ACK_MOVE_B2A_NO_SYNC_CHK_COMMAND, channel)); \
-hive_dma_snd(dma_id, (unsigned)(addr_b)); \
-hive_dma_snd(dma_id, (unsigned)(addr_a)); \
-  } else { \
-hive_dma_snd(dma_id, 
DMA_PACK_CMD_CHANNEL(_DMA_V2_NO_ACK_MOVE_A2B_NO_SYNC_CHK_COMMAND, channel)); \
-hive_dma_snd(dma_id, (unsigned)(addr_a)); \
-hive_dma_snd(dma_id, (unsigned)(addr_b)); \
-  } \
-}
-#else
 #define hive_dma_move_data(dma_id, read, channel, addr_a, addr_b, to_is_var, 
from_is_var) \
 { \
   hive_dma_snd(dma_id, DMA_PACK(_DMA_V2_SET_CRUN_COMMAND, CMD)); \
@@ -143,7 +111,6 @@ typedef enum {
   hive_dma_snd(dma_id, to_is_var); \
   hive_dma_snd(dma_id, from_is_var); \
 }
-#endif
 
 #define hive_dma_move_b2a_data(dma_id, channel, to_addr, from_addr, to_is_var, 
from_is_var) \
 { \
@@ -155,14 +122,6 @@ typedef enum {
   hive_dma_move_data(dma_id, false, channel, from_addr, to_addr, from_is_var, 
to_is_var) \
 }
 
-#ifdef __HIVECC
-#define hive_dma_set_data(dma_id, channel, address, value, is_var) \
-{ \
-  hive_dma_snd(dma_id, _DMA_V2_PACK_CHANNEL_CMD(_DMA_V2_INIT_A_COMMAND, 
channel)); \
-  hive_dma_snd(dma_id, value); \
-  hive_dma_snd(dma_id, address); \
-}
-#else
 #define hive_dma_set_data(dma_id, channel, address, value, is_var) \
 { \
   hive_dma_snd(dma_id, DMA_PACK(_DMA_V2_SET_CRUN_COMMAND, CMD)); \
@@ -171,7 +130,6 @@ typedef enum {
   hive_dma_snd(dma_id, address); \
   hive_dma_snd(dma_id, is_var); \
 }
-#endif
 
 #define hive_dma_clear_data(dma_id, channel, address, is_var) 
hive_dma_set_data(dma_id, channel, address, 0, is_var)
 
@@ -190,15 +148,6 @@ typedef enum {
   hive_dma_snd(dma_id, height); \
 }
 
-#ifdef __HIVECC
-/* If the command is "set" the 5th argument holds the value */
-#define hive_dma_execute(dma_id, channel, cmd, to_addr, from_addr_value, 
to_is_var, from_is_var) \
-{ \
-  hive_dma_snd(dma_id, DMA_PACK_CMD_CHANNEL(cmd, channel)); \
-  hive_dma_snd(dma_id, to_addr); \
-  hive_dma_snd(dma_id, from_addr_value); \
-}
-#else
 #define hive_dma_execute(dma_id, channel, cmd, to_addr, from_addr_value, 
to_is_var, from_is_var) \
 { \
   hive_dma_snd(dma_id, DMA_PACK(_DMA_V2_SET_CRUN_COMMAND, CMD)); \
@@ -210,7 +159,6 @@ typedef enum {
hive_dma_snd(dma_id, from_is_var); \
   } \
 }
-#endif
 
 #define hive_dma_configure_fast(dma_id, channel, connection, extension, 
elems_A, elems_B) \
 { \
diff --git 

[PATCH 4/8] atomisp: remove C_RUN define and code

[Alan Cox]

We are not going to be building for anything but Linux so the code bracketed
by C_RUN is not used and not needed.

Signed-off-by: Alan Cox 
---
 .../css_2400_system/hrt/isp2400_mamoiada_params.h  |4 --
 .../pci/atomisp2/css2400/css_2400_system/hrt/var.h |   25 --
 .../css2400/hive_isp_css_common/host/isp.c |   19 ---
 .../css2400/hive_isp_css_common/host/isp_private.h |   20 ++-
 .../css2400/hive_isp_css_common/host/sp_local.h|   23 -
 .../css2400/hive_isp_css_common/host/sp_private.h  |   35 
 .../css2400/hive_isp_css_common/host/vmem.c|   21 
 .../css2400/hive_isp_css_common/vmem_global.h  |4 --
 .../css2400/hive_isp_css_include/assert_support.h  |4 --
 .../pci/atomisp2/css2400/runtime/bufq/src/bufq.c   |4 --
 .../css2400/runtime/debug/src/ia_css_debug.c   |4 +-
 .../atomisp2/css2400/runtime/spctrl/src/spctrl.c   |2 +
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c|8 ++---
 .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c |2 +
 .../media/atomisp/pci/atomisp2/hrt/device_access.c |   11 --
 15 files changed, 12 insertions(+), 174 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/isp2400_mamoiada_params.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/isp2400_mamoiada_params.h
index c33d241..669060d 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/isp2400_mamoiada_params.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/isp2400_mamoiada_params.h
@@ -177,11 +177,7 @@
 
 #define _isp_ceil_div(a,b) (((a)+(b)-1)/(b))
 
-#ifdef C_RUN
-#define ISP_VEC_ALIGN  (_isp_ceil_div(ISP_VEC_WIDTH, 
64)*8)
-#else
 #define ISP_VEC_ALIGN  ISP_VMEM_ALIGN
-#endif
 
 /* HRT specific vector support */
 #define isp2400_mamoiada_vector_alignment ISP_VEC_ALIGN
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/var.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/var.h
index 19b19ef..5bc0ad3 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/var.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/var.h
@@ -44,29 +44,6 @@
 #define HRT_HOST_TYPE(cell_type)   HRTCAT(hrt_host_type_of_, cell_type)
 #define HRT_INT_TYPE(type) HRTCAT(hrt_int_type_of_, type)
 
-#ifdef C_RUN
-
-#ifdef C_RUN_DYNAMIC_LINK_PROGRAMS
-extern void *csim_processor_get_crun_symbol(hive_proc_id p, const char *sym);
-#define _hrt_cell_get_crun_symbol(cell,sym)  
csim_processor_get_crun_symbol(cell,HRTSTR(sym))
-#define _hrt_cell_get_crun_indexed_symbol(cell,sym)  
csim_processor_get_crun_symbol(cell,HRTSTR(sym))
-#else
-#define _hrt_cell_get_crun_symbol(cell,sym) ()
-#define _hrt_cell_get_crun_indexed_symbol(cell,sym) (sym)
-#endif //  C_RUN_DYNAMIC_LINK_PROGRAMS
-
-#define hrt_scalar_store(cell, type, var, data) \
-   ((*(HRT_HOST_TYPE(type)*)_hrt_cell_get_crun_symbol(cell,var)) = (data))
-#define hrt_scalar_load(cell, type, var) \
-   ((*(HRT_HOST_TYPE(type)*)_hrt_cell_get_crun_symbol(cell,var)))
-
-#define hrt_indexed_store(cell, type, array, index, data) \
-   
HRT_HOST_TYPE(type)*)_hrt_cell_get_crun_indexed_symbol(cell,array))[index]) 
= (data))
-#define hrt_indexed_load(cell, type, array, index) \
-   
(((HRT_HOST_TYPE(type)*)_hrt_cell_get_crun_indexed_symbol(cell,array))[index])
-
-#else /* C_RUN */
-
 #define hrt_scalar_store(cell, type, var, data) \
   HRTCAT(hrt_mem_store_,HRT_TYPE_BITS(cell, type))(\
   cell, \
@@ -93,7 +70,5 @@ extern void *csim_processor_get_crun_symbol(hive_proc_id p, 
const char *sym);
   HRTCAT(HIVE_MEM_,array), \
   (HRTCAT(HIVE_ADDR_,array))+((index)*HRT_TYPE_BYTES(cell, type
 
-#endif /* C_RUN */
-
 #endif /* _HRT_VAR_H */
 #endif
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/isp.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/isp.c
index 211c760..47c21e4 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/isp.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/isp.c
@@ -98,51 +98,32 @@ return;
 }
 
 /* ISP functions to control the ISP state from the host, even in crun. */
-#ifdef C_RUN
-volatile uint32_t isp_sleeping[N_ISP_ID] = { 0 }; /* Sleeping state per ISP */
-volatile uint32_t isp_ready   [N_ISP_ID] = { 1 }; /* Ready state per ISP */
-#endif
 
 /* Inspect readiness of an ISP indexed by ID */
 unsigned isp_is_ready(isp_ID_t ID)
 {
assert (ID < N_ISP_ID);
-#ifdef C_RUN
-   return isp_ready[ID];
-#else
return isp_ctrl_getbit(ID, ISP_SC_REG, ISP_IDLE_BIT);
-#endif
 }
 
 /* Inspect 

[PATCH 2/8] atomisp: remove unused code and unify a header

[Alan Cox]

KLOCWORK is never defined so we can remove the workarounds for this in the
code.

Signed-off-by: Alan Cox 
---
 .../css2400/hive_isp_css_include/assert_support.h  |   11 ---
 .../atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c  |   32 +---
 2 files changed, 1 insertion(+), 42 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/assert_support.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/assert_support.h
index 95f3892..4d68405 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/assert_support.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/assert_support.h
@@ -17,17 +17,6 @@
 
 #include "storage_class.h"
 
-#ifdef __KLOCWORK__
-/* Klocwork does not see that assert will lead to abortion
- * as there is no good way to tell this to KW and the code
- * should not depend on assert to function (actually the assert
- * could be disabled in a release build) it was decided to
- * disable the assert for KW scans (by defining NDEBUG)
- * see also: 
http://www.klocwork.com/products/documentation/current/Tuning_C/C%2B%2B_analysis#Assertions
- */
-#define NDEBUG
-#endif /* __KLOCWORK__ */
-
 /**
  * The following macro can help to test the size of a struct at compile
  * time rather than at run-time. It does not work for all compilers; see
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c
index dc30e7c..3aafc0a 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c
@@ -1,7 +1,6 @@
-#ifndef ISP2401
 /*
  * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
+ * Copyright (c) 2010-2015, Intel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -12,21 +11,6 @@
  * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
  * more details.
  */
-#else
-/**
-Support for Intel Camera Imaging ISP subsystem.
-Copyright (c) 2010 - 2015, Intel Corporation.
-
-This program is free software; you can redistribute it and/or modify it
-under the terms and conditions of the GNU General Public License,
-version 2, as published by the Free Software Foundation.
-
-This program is distributed in the hope it will be useful, but WITHOUT
-ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-more details.
-*/
-#endif
 
 #include "ia_css_rmgr.h"
 
@@ -279,21 +263,7 @@ void rmgr_pop_handle(struct ia_css_rmgr_vbuf_pool *pool,
 void ia_css_rmgr_acq_vbuf(struct ia_css_rmgr_vbuf_pool *pool,
  struct ia_css_rmgr_vbuf_handle **handle)
 {
-#ifdef __KLOCWORK__
-   /* KW sees the *handle = h; assignment about 20 lines down
-  and thinks that we are assigning a local to a global.
-  What it does not see is that in ia_css_i_host_rmgr_pop_handle
-  a new value is assigned to handle.
-  So this is a false positive KW issue.
-  To fix that we make the struct static for KW so it will
-  think that h remains alive; we do not want this in our
-  production code though as it breaks reentrancy of the code
-*/
-
-   static struct ia_css_rmgr_vbuf_handle h;
-#else /* __KLOCWORK__ */
struct ia_css_rmgr_vbuf_handle h;
-#endif /* __KLOCWORK__ */
 
if ((pool == NULL) || (handle == NULL) || (*handle == NULL)) {
IA_CSS_LOG("Invalid inputs");

[PATCH 1/8] atomisp: remove dead code for SSSE3

[Alan Cox]

This is another define which is never used and references a header that doesn't
exist, so consider it dead. Our memcpy is pretty smart anyway.

Signed-off-by: Alan Cox 
---
 .../staging/media/atomisp/pci/atomisp2/hmm/hmm.c   |   32 +---
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index 358d340..b041e56 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -39,10 +39,6 @@
 #include "mmu/isp_mmu.h"
 #include "mmu/sh_mmu_mrfld.h"
 
-#ifdef USE_SSSE3
-#include 
-#endif
-
 struct hmm_bo_device bo_device;
 struct hmm_pooldynamic_pool;
 struct hmm_poolreserved_pool;
@@ -53,9 +49,7 @@ const char *hmm_bo_type_strings[HMM_BO_LAST] = {
"p", /* private */
"s", /* shared */
"u", /* user */
-#ifdef CONFIG_ION
"i", /* ion */
-#endif
 };
 
 static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
@@ -356,12 +350,7 @@ static int load_and_flush_by_kmap(ia_css_ptr virt, void 
*data, unsigned int byte
virt += len;/* update virt for next loop */
 
if (des) {
-
-#ifdef USE_SSSE3
-   _ssse3_memcpy(des, src, len);
-#else
memcpy(des, src, len);
-#endif
des += len;
}
 
@@ -388,11 +377,7 @@ static int load_and_flush(ia_css_ptr virt, void *data, 
unsigned int bytes)
void *src = bo->vmap_addr;
 
src += (virt - bo->start);
-#ifdef USE_SSSE3
-   _ssse3_memcpy(data, src, bytes);
-#else
memcpy(data, src, bytes);
-#endif
if (bo->status & HMM_BO_VMAPED_CACHED)
clflush_cache_range(src, bytes);
} else {
@@ -404,11 +389,7 @@ static int load_and_flush(ia_css_ptr virt, void *data, 
unsigned int bytes)
else
vptr = vptr + (virt - bo->start);
 
-#ifdef USE_SSSE3
-   _ssse3_memcpy(data, vptr, bytes);
-#else
memcpy(data, vptr, bytes);
-#endif
clflush_cache_range(vptr, bytes);
hmm_bo_vunmap(bo);
}
@@ -450,11 +431,7 @@ int hmm_store(ia_css_ptr virt, const void *data, unsigned 
int bytes)
void *dst = bo->vmap_addr;
 
dst += (virt - bo->start);
-#ifdef USE_SSSE3
-   _ssse3_memcpy(dst, data, bytes);
-#else
memcpy(dst, data, bytes);
-#endif
if (bo->status & HMM_BO_VMAPED_CACHED)
clflush_cache_range(dst, bytes);
} else {
@@ -464,11 +441,7 @@ int hmm_store(ia_css_ptr virt, const void *data, unsigned 
int bytes)
if (vptr) {
vptr = vptr + (virt - bo->start);
 
-#ifdef USE_SSSE3
-   _ssse3_memcpy(vptr, data, bytes);
-#else
memcpy(vptr, data, bytes);
-#endif
clflush_cache_range(vptr, bytes);
hmm_bo_vunmap(bo);
return 0;
@@ -504,11 +477,8 @@ int hmm_store(ia_css_ptr virt, const void *data, unsigned 
int bytes)
 
virt += len;
 
-#ifdef USE_SSSE3
-   _ssse3_memcpy(des, src, len);
-#else
memcpy(des, src, len);
-#endif
+
src += len;
 
clflush_cache_range(des, len);

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 10/03/17 11:39, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks! It's very nice to see one more driver converted to stand-alone one!
> 
> Speaking of which --- this seems to be the second last one. The only
> remaining one is sh_mobile_ceu_camera.c!
> 
> On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This patch converts the atmel-isi driver from a soc-camera driver to a driver
>> that is stand-alone.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/soc_camera/ov2640.c |   23 +-
>>  drivers/media/platform/soc_camera/Kconfig |3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 
>> +++--
>>  3 files changed, 735 insertions(+), 514 deletions(-)
>>



>> +static int isi_graph_parse(struct atmel_isi *isi,
>> +struct device_node *node)
> 
> Fits on a single line.
> 
>> +{
>> +struct device_node *remote;
>> +struct device_node *ep = NULL;
>> +struct device_node *next;
>> +int ret = 0;
>> +
>> +while (1) {
>> +next = of_graph_get_next_endpoint(node, ep);
>> +if (!next)
>> +break;
> 
> You could make this a while loop condition.
> 
>> +
>> +of_node_put(ep);
> 
> ep is put by of_graph_get_next_endpoint(), no need to do it manually here.
> 
>> +ep = next;
>> +
>> +remote = of_graph_get_remote_port_parent(ep);
>> +if (!remote) {
> 
> of_node_put(ep);
> 
>> +ret = -EINVAL;
>> +break;
>> +}
>> +
>> +/* Skip entities that we have already processed. */
>> +if (remote == isi->dev->of_node) {
>> +of_node_put(remote);
>> +continue;
>> +}
>> +
>> +/* Remote node to connect */
>> +if (!isi->entity.node) {
> 
> There would have to be something wrong about the DT graph for the two above
> to happen. I guess one could just return an error if something is terribly
> wrong.
> 
> I'm thinking this from the point of view of making this function generic,
> and moving it to the framework as most drivers to something very similar,
> but I'd prefer to get the fwnode patches in first.
> 
>> +isi->entity.node = remote;
> 
> You could use entity.asd.match.of.node instead, and drop the node field.

Slight problem with this. If I make this change, then the of_node_put below
changes as well:

@@ -1193,7 +1176,7 @@ static int isi_graph_init(struct atmel_isi *isi)
 done:
if (ret < 0) {
v4l2_async_notifier_unregister(>notifier);
-   of_node_put(isi->entity.node);
+   of_node_put(isi->entity.asd.match.of.node);
}

And I get this compiler warning:

  CC [M]  drivers/media/platform/atmel/atmel-isi.o
drivers/media/platform/atmel/atmel-isi.c: In function ‘isi_graph_init’:
drivers/media/platform/atmel/atmel-isi.c:1179:15: warning: passing argument 1 
of ‘of_node_put’ discards ‘const’ qualifier from pointer target type 
[-Wdiscarded-qualifiers]
   of_node_put(isi->entity.asd.match.of.node);
   ^~~
In file included from drivers/media/platform/atmel/atmel-isi.c:25:0:
./include/linux/of.h:130:20: note: expected ‘struct device_node *’ but argument 
is of type ‘const struct device_node *’
 static inline void of_node_put(struct device_node *node) { }
^~~


Any suggestions? Just keep the entity.node after all?

Regards,

Hans

> 
>> +isi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
>> +isi->entity.asd.match.of.node = remote;
>> +ret++;
>> +}
>> +}
>> +
>> +of_node_put(ep);
>> +
>> +return ret;
>> +}

Re: [PATCHv3 06/15] atmel-isi: document device tree bindings

[Sakari Ailus]

Hi Hans,

On Mon, Mar 06, 2017 at 03:56:07PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Document the device tree bindings for this hardware.
> 
> Mostly copied from the atmel-isc bindings.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  .../devicetree/bindings/media/atmel-isi.txt| 90 
> +-
>  1 file changed, 55 insertions(+), 35 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
> b/Documentation/devicetree/bindings/media/atmel-isi.txt
> index 251f008f220c..d6e93e8216af 100644
> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> @@ -1,51 +1,71 @@
> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> ---
> +Atmel Image Sensor Interface (ISI)
> +--
>  
> -Required properties:
> -- compatible: must be "atmel,at91sam9g45-isi"
> -- reg: physical base address and length of the registers set for the device;
> -- interrupts: should contain IRQ line for the ISI;
> -- clocks: list of clock specifiers, corresponding to entries in
> -  the clock-names property;
> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
> +Required properties for ISI:
> +- compatible: must be "atmel,at91sam9g45-isi".
> +- reg: physical base address and length of the registers set for the device.
> +- interrupts: should contain IRQ line for the ISI.
> +- clocks: list of clock specifiers, corresponding to entries in the 
> clock-names
> + property; please refer to clock-bindings.txt.
> +- clock-names: required elements: "isi_clk".
> +- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
>  
>  ISI supports a single port node with parallel bus. It should contain one

s/should/shall/ ?

>  'port' child node with child 'endpoint' node. Please refer to the bindings
>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>  
> -Example:
> - isi: isi@f0034000 {
> - compatible = "atmel,at91sam9g45-isi";
> - reg = <0xf0034000 0x4000>;
> - interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> -
> - clocks = <_clk>;
> - clock-names = "isi_clk";
> +Endpoint node properties
> +
>  
> - pinctrl-names = "default";
> - pinctrl-0 = <_isi>;
> +- bus-width: <8> or <10> (mandatory)
> +- hsync-active
> +- vsync-active
> +- pclk-sample
> +- remote-endpoint: A phandle to the bus receiver's endpoint node.

Are these required or are they optional?

The remote-endpoint is at least mandatory, isn't it?

>  
> - port {
> - #address-cells = <1>;
> - #size-cells = <0>;
> +Example:
>  
> - isi_0: endpoint {
> - remote-endpoint = <_0>;
> - bus-width = <8>;
> - };
> +isi: isi@f0034000 {
> + compatible = "atmel,at91sam9g45-isi";
> + reg = <0xf0034000 0x4000>;
> + interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_isi_data_0_7>;
> + clocks = <_clk>;
> + clock-names = "isi_clk";
> + status = "ok";
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + isi_0: endpoint {
> + remote-endpoint = <_0>;
> + bus-width = <8>;
> + vsync-active = <1>;
> + hsync-active = <1>;
>   };
>   };
> +};
> +
> +i2c1: i2c@f0018000 {
> + status = "okay";
>  
> - i2c1: i2c@f0018000 {
> - ov2640: camera@0x30 {
> - compatible = "ovti,ov2640";
> - reg = <0x30>;
> + ov2640: camera@30 {
> + compatible = "ovti,ov2640";
> + reg = <0x30>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
> _sensor_reset>;
> + resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
> + pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
> + clocks = <>;
> + clock-names = "xvclk";
> + assigned-clocks = <>;
> + assigned-clock-rates = <2500>;
>  
> - port {
> - ov2640_0: endpoint {
> - remote-endpoint = <_0>;
> - bus-width = <8>;
> - };
> + port {
> + ov2640_0: endpoint {
> + remote-endpoint = <_0>;
> + bus-width = <8>;
>   };
>   };
>   };
> +};

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv4 12/15] ov2640: add MC support

[Hans Verkuil]

On 10/03/17 12:15, Sakari Ailus wrote:
> Hi Hans,
> 
> On Fri, Mar 10, 2017 at 11:26:11AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> The MC support is needed by the em28xx driver.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/ov2640.c | 21 +++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
>> index 0445963c5fae..00df042fd6f1 100644
>> --- a/drivers/media/i2c/ov2640.c
>> +++ b/drivers/media/i2c/ov2640.c
>> @@ -282,6 +282,9 @@ struct ov2640_win_size {
>>  
>>  struct ov2640_priv {
>>  struct v4l2_subdev  subdev;
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +struct media_pad pad;
>> +#endif
>>  struct v4l2_ctrl_handlerhdl;
>>  u32 cfmt_code;
>>  struct clk  *clk;
>> @@ -1073,19 +1076,30 @@ static int ov2640_probe(struct i2c_client *client,
>>  ret = priv->hdl.error;
>>  goto err_hdl;
>>  }
> 
> As this is a sensor the format etc. should be accessible from the user
> space, shouldn't you have V4L2_SUBDEV_FL_HAS_DEVNODE flag for the
> sub-device as well?
> 
> The device node isn't exposed unless v4l2_device_register_subdev_nodes() is
> called anyway.

Can do this for both ov2640 and ov7670.

> 
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +priv->pad.flags = MEDIA_PAD_FL_SOURCE;
>> +priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>> +ret = media_entity_pads_init(>subdev.entity, 1, >pad);
>> +if (ret < 0)
>> +goto err_hdl;
>> +#endif
>>  
>>  ret = ov2640_video_probe(client);
>>  if (ret < 0)
>> -goto err_hdl;
>> +goto err_videoprobe;
>>  
>>  ret = v4l2_async_register_subdev(>subdev);
>>  if (ret < 0)
>> -goto err_hdl;
>> +goto err_videoprobe;
>>  
>>  dev_info(>dev, "OV2640 Probed\n");
>>  
>>  return 0;
>>  
>> +err_videoprobe:
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +media_entity_cleanup(>subdev.entity);
> 
> I believe you can call media_entity_cleanup() as the function does nothing
> right now. In general, we should provide nop variants if MC is disabled.

You mean that I don't have to check CONFIG_MEDIA_CONTROLLER for this, right?

Makes sense.

> 
>> +#endif
>>  err_hdl:
>>  v4l2_ctrl_handler_free(>hdl);
>>  err_clk:
>> @@ -1099,6 +1113,9 @@ static int ov2640_remove(struct i2c_client *client)
>>  
>>  v4l2_async_unregister_subdev(>subdev);
>>  v4l2_ctrl_handler_free(>hdl);
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +media_entity_cleanup(>subdev.entity);
>> +#endif
>>  v4l2_device_unregister_subdev(>subdev);
>>  clk_disable_unprepare(priv->clk);
>>  return 0;
> 

Regards,

Hans

Re: [PATCHv4 09/15] ov2640: update bindings

[Sakari Ailus]

On Fri, Mar 10, 2017 at 11:26:08AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Update the bindings for this device based on a working DT example.
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Rob Herring 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv4 12/15] ov2640: add MC support

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 11:26:11AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The MC support is needed by the em28xx driver.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/i2c/ov2640.c | 21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 0445963c5fae..00df042fd6f1 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -282,6 +282,9 @@ struct ov2640_win_size {
>  
>  struct ov2640_priv {
>   struct v4l2_subdev  subdev;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + struct media_pad pad;
> +#endif
>   struct v4l2_ctrl_handlerhdl;
>   u32 cfmt_code;
>   struct clk  *clk;
> @@ -1073,19 +1076,30 @@ static int ov2640_probe(struct i2c_client *client,
>   ret = priv->hdl.error;
>   goto err_hdl;
>   }

As this is a sensor the format etc. should be accessible from the user
space, shouldn't you have V4L2_SUBDEV_FL_HAS_DEVNODE flag for the
sub-device as well?

The device node isn't exposed unless v4l2_device_register_subdev_nodes() is
called anyway.

> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + priv->pad.flags = MEDIA_PAD_FL_SOURCE;
> + priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ret = media_entity_pads_init(>subdev.entity, 1, >pad);
> + if (ret < 0)
> + goto err_hdl;
> +#endif
>  
>   ret = ov2640_video_probe(client);
>   if (ret < 0)
> - goto err_hdl;
> + goto err_videoprobe;
>  
>   ret = v4l2_async_register_subdev(>subdev);
>   if (ret < 0)
> - goto err_hdl;
> + goto err_videoprobe;
>  
>   dev_info(>dev, "OV2640 Probed\n");
>  
>   return 0;
>  
> +err_videoprobe:
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + media_entity_cleanup(>subdev.entity);

I believe you can call media_entity_cleanup() as the function does nothing
right now. In general, we should provide nop variants if MC is disabled.

> +#endif
>  err_hdl:
>   v4l2_ctrl_handler_free(>hdl);
>  err_clk:
> @@ -1099,6 +1113,9 @@ static int ov2640_remove(struct i2c_client *client)
>  
>   v4l2_async_unregister_subdev(>subdev);
>   v4l2_ctrl_handler_free(>hdl);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + media_entity_cleanup(>subdev.entity);
> +#endif
>   v4l2_device_unregister_subdev(>subdev);
>   clk_disable_unprepare(priv->clk);
>   return 0;

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 10/03/17 11:39, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks! It's very nice to see one more driver converted to stand-alone one!
> 
> Speaking of which --- this seems to be the second last one. The only
> remaining one is sh_mobile_ceu_camera.c!
> 
> On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This patch converts the atmel-isi driver from a soc-camera driver to a driver
>> that is stand-alone.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/soc_camera/ov2640.c |   23 +-
>>  drivers/media/platform/soc_camera/Kconfig |3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 
>> +++--
>>  3 files changed, 735 insertions(+), 514 deletions(-)
>>
>> diff --git a/drivers/media/i2c/soc_camera/ov2640.c 
>> b/drivers/media/i2c/soc_camera/ov2640.c
>> index 56de18263359..b9a0069f5b33 100644
>> --- a/drivers/media/i2c/soc_camera/ov2640.c
>> +++ b/drivers/media/i2c/soc_camera/ov2640.c
> 
> Should this part go to a different patch?

Oops, yes. How did this end up here?

Split this off as a separate ov2640 patch.

> 
>> @@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client 
>> *client, u32 *width, u32 *height,
>>  dev_dbg(>dev, "%s: Selected cfmt YUYV (YUV422)", 
>> __func__);
>>  selected_cfmt_regs = ov2640_yuyv_regs;
>>  break;
>> -default:
>>  case MEDIA_BUS_FMT_UYVY8_2X8:
>> +default:
>>  dev_dbg(>dev, "%s: Selected cfmt UYVY", __func__);
>>  selected_cfmt_regs = ov2640_uyvy_regs;
>> +break;
>>  }
>>  
>>  /* reset hardware */
>> @@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
>>  mf->width   = priv->win->width;
>>  mf->height  = priv->win->height;
>>  mf->code= priv->cfmt_code;
>> -
>> -switch (mf->code) {
>> -case MEDIA_BUS_FMT_RGB565_2X8_BE:
>> -case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> -mf->colorspace = V4L2_COLORSPACE_SRGB;
>> -break;
>> -default:
>> -case MEDIA_BUS_FMT_YUYV8_2X8:
>> -case MEDIA_BUS_FMT_UYVY8_2X8:
>> -mf->colorspace = V4L2_COLORSPACE_JPEG;
>> -}
>> +mf->colorspace  = V4L2_COLORSPACE_SRGB;
>>  mf->field   = V4L2_FIELD_NONE;
>>  
>>  return 0;
>> @@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>>  ov2640_select_win(>width, >height);
>>  
>>  mf->field   = V4L2_FIELD_NONE;
>> +mf->colorspace  = V4L2_COLORSPACE_SRGB;
>>  
>>  switch (mf->code) {
>>  case MEDIA_BUS_FMT_RGB565_2X8_BE:
>>  case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> -mf->colorspace = V4L2_COLORSPACE_SRGB;
>> +case MEDIA_BUS_FMT_YUYV8_2X8:
>> +case MEDIA_BUS_FMT_UYVY8_2X8:
>>  break;
>>  default:
>>  mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> -case MEDIA_BUS_FMT_YUYV8_2X8:
>> -case MEDIA_BUS_FMT_UYVY8_2X8:
>> -mf->colorspace = V4L2_COLORSPACE_JPEG;
>> +break;
>>  }
>>  
>>  if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> diff --git a/drivers/media/platform/soc_camera/Kconfig 
>> b/drivers/media/platform/soc_camera/Kconfig
>> index 86d74788544f..a37ec91b026e 100644
>> --- a/drivers/media/platform/soc_camera/Kconfig
>> +++ b/drivers/media/platform/soc_camera/Kconfig
>> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>>  
>>  config VIDEO_ATMEL_ISI
>>  tristate "ATMEL Image Sensor Interface (ISI) support"
>> -depends on VIDEO_DEV && SOC_CAMERA
>> +depends on VIDEO_V4L2 && OF && HAS_DMA
>>  depends on ARCH_AT91 || COMPILE_TEST
>> -depends on HAS_DMA
>>  select VIDEOBUF2_DMA_CONTIG
>>  ---help---
>>This module makes the ATMEL Image Sensor Interface available
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
>> b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 46de657c3e6d..a837b94281ef 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> ...
>> @@ -457,60 +431,83 @@ static void buffer_queue(struct vb2_buffer *vb)
>>  if (vb2_is_streaming(vb->vb2_queue))
>>  start_dma(isi, buf);
>>  }
>> -spin_unlock_irqrestore(>lock, flags);
>> +spin_unlock_irqrestore(>irqlock, flags);
>>  }
>>  
>>  static int start_streaming(struct vb2_queue *vq, unsigned int count)
>>  {
>> -struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
>> -struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>> -struct atmel_isi *isi = ici->priv;
>> +struct atmel_isi *isi = vb2_get_drv_priv(vq);
>> +struct frame_buffer *buf, *node;
>>  int ret;
>>  
>> -pm_runtime_get_sync(ici->v4l2_dev.dev);
>> +/* Enable stream on the sub device */
>> +ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
>> +if (ret && ret != -ENOIOCTLCMD) {
>> +

Re: [PATCHv4 11/15] ov2640: use standard clk and enable it.

[Sakari Ailus]

On Fri, Mar 10, 2017 at 11:26:10AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Convert v4l2_clk to normal clk and enable the clock.
> 
> Signed-off-by: Hans Verkuil 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv4 10/15] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 11:26:09AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
> uses this driver, so it can safely be converted.
> 
> Note: the s_power op has been dropped: this never worked. When the last open()
> is closed, then the power is turned off, and when it is opened again the power
> is turned on again, but the old state isn't restored.
> 
> Someone else can figure out in the future how to get this working correctly,
> but I don't want to spend more time on this.
> 
> Signed-off-by: Hans Verkuil 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv4 00/15] atmel-isi/ov7670/ov2640: convert to standalone drivers

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 11:25:59AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This patch series converts the soc-camera atmel-isi to a standalone V4L2
> driver.
> 
> The same is done for the ov7670 and ov2640 sensor drivers: the ov7670 was
> used to test the atmel-isi driver. The ov2640 is needed because the em28xx
> driver has a soc_camera include dependency. Both ov7670 and ov2640 sensors
> have been tested with the atmel-isi driver.
> 
> The first 5 patches improve the ov7670 sensor driver, mostly adding modern
> features such as DT support.
> 
> The next three convert the atmel-isi and move it out of soc_camera.
> 
> The following 5 patches convert ov2640 and drop the soc_camera dependency
> in em28xx. I have tested that this works with my 'SpeedLink Vicious And
> Divine Laplace webcam'.
> 
> The last two patches add isi support to the DT: the first for the ov7670
> sensor, the second modifies it for the ov2640 sensor.
> 
> These two final patches are for demonstration purposes only, I do not plan
> on merging them.
> 
> Tested with my sama5d3-Xplained board, the ov2640 sensor and two ov7670
> sensors: one with and one without reset/pwdn pins. Also tested with my
> em28xx-based webcam.
> 
> I'd like to get this in for 4.12. Fingers crossed.

Patches 1, 4, 5, 6 and 8: 

Acked-by: Sakari Ailus 

I'll try to check the rest pretty soon.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v6 2/2] [media] s5p-mfc: Handle 'v4l2_pix_format:field' in try_fmt and g_fmt

[Hans Verkuil]

On 01/03/17 12:51, Thibault Saunier wrote:
> It is required by the standard that the field order is set by the
> driver, default to NONE in case any is provided, but we can basically
> accept any value provided by the userspace as we will anyway not
> be able to do any deinterlacing.
> 
> In this patch we also make sure to pass the interlacing mode provided
> by userspace from the output to the capture side of the device so
> that the information is given back to userspace. This way it can
> handle it and potentially deinterlace afterward.
> 
> Signed-off-by: Thibault Saunier 
> 
> ---
> 
> Changes in v6:
> - Pass user output field value to the capture as the device is not
>   doing any deinterlacing and thus decoded content will still be
>   interlaced on the output.
> 
> Changes in v5:
> - Just adapt the field and never error out.
> 
> Changes in v4: None
> Changes in v3:
> - Do not check values in the g_fmt functions as Andrzej explained in previous 
> review
> 
> Changes in v2:
> - Fix a silly build error that slipped in while rebasing the patches
> 
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 ++
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 6 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index ab23236aa942..3816a37de4bc 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -652,6 +652,8 @@ struct s5p_mfc_ctx {
>   size_t me_buffer_size;
>   size_t tmv_buffer_size;
>  
> + enum v4l2_field field;
> +
>   enum v4l2_mpeg_mfc51_video_force_frame_type force_frame_type;
>  
>   struct list_head ref_queue;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index 367ef8e8dbf0..6e5ca86fb331 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -345,7 +345,7 @@ static int vidioc_g_fmt(struct file *file, void *priv, 
> struct v4l2_format *f)
>  rectangle. */
>   pix_mp->width = ctx->buf_width;
>   pix_mp->height = ctx->buf_height;
> - pix_mp->field = V4L2_FIELD_NONE;
> + pix_mp->field = ctx->field;
>   pix_mp->num_planes = 2;
>   /* Set pixelformat to the format in which MFC
>  outputs the decoded frame */
> @@ -380,6 +380,9 @@ static int vidioc_try_fmt(struct file *file, void *priv, 
> struct v4l2_format *f)
>   struct s5p_mfc_dev *dev = video_drvdata(file);
>   struct s5p_mfc_fmt *fmt;
>  
> + if (f->fmt.pix.field == V4L2_FIELD_ANY)
> + f->fmt.pix.field = V4L2_FIELD_NONE;
> +
>   mfc_debug(2, "Type is %d\n", f->type);
>   if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>   fmt = find_format(f, MFC_FMT_DEC);
> @@ -436,6 +439,7 @@ static int vidioc_s_fmt(struct file *file, void *priv, 
> struct v4l2_format *f)
>   goto out;
>   } else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>   /* src_fmt is validated by call to vidioc_try_fmt */
> + ctx->field = f->fmt.pix.field;

Doing this means that you also allow for V4L2_FIELD_ALTERNATE. If you do that, 
then there
are additional requirements when you queue up buffers in that the 'field' has 
to be set to
TOP or BOTTOM in struct v4l2_buffer. I am sure that is a requirement for the 
encoder, how
this would work for a decoder I am not sure.

Also, values like V4L2_FIELD_SEQ_TB/BT would not behave well if you pass them 
through a
decoder.

Frankly I think the only 'reasonable' values would be FIELD_NONE, 
FIELD_INTERLACED and
possibly FIELD_ALTERNATE. I don't know enough about how codecs handle 
interlaced formats,
so I can't tell how much sense FIELD_ALTERNATE would make.

Properly supporting interlaced formats should only be done if you actually 
tested it and
know how it works and what exactly is supported.

Regards,

Hans

>   ctx->src_fmt = find_format(f, MFC_FMT_DEC);
>   ctx->codec_mode = ctx->src_fmt->codec_mode;
>   mfc_debug(2, "The codec number is: %d\n", ctx->codec_mode);
> 

Re: [PATCHv3 13/15] em28xx: drop last soc_camera link

[Sakari Ailus]

On Mon, Mar 06, 2017 at 03:56:14PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The em28xx driver still used the soc_camera.h header for the ov2640
> driver. Since this driver no longer uses soc_camera, that include can
> be removed.
> 
> Signed-off-by: Hans Verkuil 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 08/15] atmel-isi: move out of soc_camera to atmel

[Sakari Ailus]

On Mon, Mar 06, 2017 at 03:56:09PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Move this out of the soc_camera directory into the atmel directory
> where it belongs.
> 
> Signed-off-by: Hans Verkuil 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 05/15] ov7670: add devicetree support

[Sakari Ailus]

On Fri, Mar 10, 2017 at 10:32:41AM +0100, Hans Verkuil wrote:
> On 09/03/17 21:45, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Mar 06, 2017 at 03:56:06PM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Add DT support. Use it to get the reset and pwdn pins (if there are any).
> >> Tested with one sensor requiring reset/pwdn and one sensor that doesn't
> >> have reset/pwdn pins.
> > 
> > If I read the datasheet right, lifting the reset signal up will reset the
> > sensor but the patch doesn't make use of that, it only ensures the reset
> > signal stays low. Should you lift it up for a while as well? The datasheet
> > doesn't say for how long that should be done, but that it should be usable
> > after 1 ms since pulling reset down.
> 
> There does not seem to be any need for that. This sensor also comes in two
> models: one with separate pwdn and reset pins, and one where it is just 
> hardwired.
> 
> If the hardwired variant doesn't need a reset pulse, then neither does the
> variant with pins. It works, and I am not really willing to experiment with 
> this.

Fine with me.

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 07/15] atmel-isi: remove dependency of the soc-camera framework

[Sakari Ailus]

Hi Hans,

Thanks! It's very nice to see one more driver converted to stand-alone one!

Speaking of which --- this seems to be the second last one. The only
remaining one is sh_mobile_ceu_camera.c!

On Mon, Mar 06, 2017 at 03:56:08PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This patch converts the atmel-isi driver from a soc-camera driver to a driver
> that is stand-alone.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/i2c/soc_camera/ov2640.c |   23 +-
>  drivers/media/platform/soc_camera/Kconfig |3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1223 
> +++--
>  3 files changed, 735 insertions(+), 514 deletions(-)
> 
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c 
> b/drivers/media/i2c/soc_camera/ov2640.c
> index 56de18263359..b9a0069f5b33 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/soc_camera/ov2640.c

Should this part go to a different patch?

> @@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client *client, 
> u32 *width, u32 *height,
>   dev_dbg(>dev, "%s: Selected cfmt YUYV (YUV422)", 
> __func__);
>   selected_cfmt_regs = ov2640_yuyv_regs;
>   break;
> - default:
>   case MEDIA_BUS_FMT_UYVY8_2X8:
> + default:
>   dev_dbg(>dev, "%s: Selected cfmt UYVY", __func__);
>   selected_cfmt_regs = ov2640_uyvy_regs;
> + break;
>   }
>  
>   /* reset hardware */
> @@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
>   mf->width   = priv->win->width;
>   mf->height  = priv->win->height;
>   mf->code= priv->cfmt_code;
> -
> - switch (mf->code) {
> - case MEDIA_BUS_FMT_RGB565_2X8_BE:
> - case MEDIA_BUS_FMT_RGB565_2X8_LE:
> - mf->colorspace = V4L2_COLORSPACE_SRGB;
> - break;
> - default:
> - case MEDIA_BUS_FMT_YUYV8_2X8:
> - case MEDIA_BUS_FMT_UYVY8_2X8:
> - mf->colorspace = V4L2_COLORSPACE_JPEG;
> - }
> + mf->colorspace  = V4L2_COLORSPACE_SRGB;
>   mf->field   = V4L2_FIELD_NONE;
>  
>   return 0;
> @@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>   ov2640_select_win(>width, >height);
>  
>   mf->field   = V4L2_FIELD_NONE;
> + mf->colorspace  = V4L2_COLORSPACE_SRGB;
>  
>   switch (mf->code) {
>   case MEDIA_BUS_FMT_RGB565_2X8_BE:
>   case MEDIA_BUS_FMT_RGB565_2X8_LE:
> - mf->colorspace = V4L2_COLORSPACE_SRGB;
> + case MEDIA_BUS_FMT_YUYV8_2X8:
> + case MEDIA_BUS_FMT_UYVY8_2X8:
>   break;
>   default:
>   mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
> - case MEDIA_BUS_FMT_YUYV8_2X8:
> - case MEDIA_BUS_FMT_UYVY8_2X8:
> - mf->colorspace = V4L2_COLORSPACE_JPEG;
> + break;
>   }
>  
>   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> diff --git a/drivers/media/platform/soc_camera/Kconfig 
> b/drivers/media/platform/soc_camera/Kconfig
> index 86d74788544f..a37ec91b026e 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>  
>  config VIDEO_ATMEL_ISI
>   tristate "ATMEL Image Sensor Interface (ISI) support"
> - depends on VIDEO_DEV && SOC_CAMERA
> + depends on VIDEO_V4L2 && OF && HAS_DMA
>   depends on ARCH_AT91 || COMPILE_TEST
> - depends on HAS_DMA
>   select VIDEOBUF2_DMA_CONTIG
>   ---help---
> This module makes the ATMEL Image Sensor Interface available
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
> b/drivers/media/platform/soc_camera/atmel-isi.c
> index 46de657c3e6d..a837b94281ef 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
...
> @@ -457,60 +431,83 @@ static void buffer_queue(struct vb2_buffer *vb)
>   if (vb2_is_streaming(vb->vb2_queue))
>   start_dma(isi, buf);
>   }
> - spin_unlock_irqrestore(>lock, flags);
> + spin_unlock_irqrestore(>irqlock, flags);
>  }
>  
>  static int start_streaming(struct vb2_queue *vq, unsigned int count)
>  {
> - struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> - struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> - struct atmel_isi *isi = ici->priv;
> + struct atmel_isi *isi = vb2_get_drv_priv(vq);
> + struct frame_buffer *buf, *node;
>   int ret;
>  
> - pm_runtime_get_sync(ici->v4l2_dev.dev);
> + /* Enable stream on the sub device */
> + ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
> + if (ret && ret != -ENOIOCTLCMD) {
> + v4l2_err(>v4l2_dev, "stream on failed in subdev\n");

You mostly use dev_*() functions to print infromation in the driver. How
about using them consistently? There are few other cases of 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Brian Starkey]

Hi,

On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:


[snip]



For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?


Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.



Is the only motivation for removing the alignment parameter that
no-one got around to using it for something useful yet?
The original comment was true - different devices do have different
alignment requirements.

Better alignment can help SMMUs use larger blocks when mapping,
reducing TLB pressure and the chance of a page table walk causing
display underruns.

-Brian

Re: [PATCH v6 1/2] [media] exynos-gsc: Use user configured colorspace if provided

[Hans Verkuil]

On 01/03/17 12:51, Thibault Saunier wrote:
> Use colorspace provided by the user as we are only doing scaling and
> color encoding conversion, we won't be able to transform the colorspace
> itself and the colorspace won't mater in that operation.
> 
> Also always use output colorspace on the capture side.
> 
> If the user does not provide a colorspace do no make it up, we might
> later while processing need to figure out the colorspace, which
> is possible depending on the frame size but do not ever guess and
> leak that guess to the userspace.
> 
> Signed-off-by: Javier Martinez Canillas 
> Signed-off-by: Thibault Saunier 
> Reviewed-by: Andrzej Hajda 
> 
> ---
> 
> Changes in v6:
> - Do not ever guess colorspace
> 
> Changes in v5:
> - Squash commit to always use output colorspace on the capture side
>   inside this one
> - Fix typo in commit message
> 
> Changes in v4:
> - Reword commit message to better back our assumptions on specifications
> 
> Changes in v3:
> - Do not check values in the g_fmt functions as Andrzej explained in previous 
> review
> - Added 'Reviewed-by: Andrzej Hajda '
> 
> Changes in v2: None
> 
>  drivers/media/platform/exynos-gsc/gsc-core.c | 9 -
>  drivers/media/platform/exynos-gsc/gsc-core.h | 1 +
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 59a634201830..0241168c85af 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -454,6 +454,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct 
> v4l2_format *f)
>   } else {
>   min_w = variant->pix_min->target_rot_dis_w;
>   min_h = variant->pix_min->target_rot_dis_h;
> + pix_mp->colorspace = ctx->out_colorspace;
>   }
>  
>   pr_debug("mod_x: %d, mod_y: %d, max_w: %d, max_h = %d",
> @@ -472,10 +473,8 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct 
> v4l2_format *f)
>  
>   pix_mp->num_planes = fmt->num_planes;
>  
> - if (pix_mp->width >= 1280) /* HD */
> - pix_mp->colorspace = V4L2_COLORSPACE_REC709;
> - else /* SD */
> - pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
> + if (V4L2_TYPE_IS_OUTPUT(f->type))
> + ctx->out_colorspace = pix_mp->colorspace;
>  
>   for (i = 0; i < pix_mp->num_planes; ++i) {
>   struct v4l2_plane_pix_format *plane_fmt = _mp->plane_fmt[i];
> @@ -519,8 +518,8 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct 
> v4l2_format *f)
>   pix_mp->height  = frame->f_height;
>   pix_mp->field   = V4L2_FIELD_NONE;
>   pix_mp->pixelformat = frame->fmt->pixelformat;
> - pix_mp->colorspace  = V4L2_COLORSPACE_REC709;
>   pix_mp->num_planes  = frame->fmt->num_planes;
> + pix_mp->colorspace = ctx->out_colorspace;

You need to do the same for the ycbcr_enc, xfer_func and quantization fields.
With that in place it is fully 'colorspace compliant' :-)

Regards,

Hans

>  
>   for (i = 0; i < pix_mp->num_planes; ++i) {
>   pix_mp->plane_fmt[i].bytesperline = (frame->f_width *
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h 
> b/drivers/media/platform/exynos-gsc/gsc-core.h
> index 696217e9af66..715d9c9d8d30 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -376,6 +376,7 @@ struct gsc_ctx {
>   struct v4l2_ctrl_handler ctrl_handler;
>   struct gsc_ctrlsgsc_ctrls;
>   boolctrls_rdy;
> + enum v4l2_colorspace out_colorspace;
>  };
>  
>  void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm);
> 

[PATCHv4 09/15] ov2640: update bindings

[Hans Verkuil]

From: Hans Verkuil 

Update the bindings for this device based on a working DT example.

Signed-off-by: Hans Verkuil 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/media/i2c/ov2640.txt   | 23 +-
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2640.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2640.txt
index c429b5bdcaa0..989ce6cb6ac3 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov2640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov2640.txt
@@ -1,8 +1,8 @@
 * Omnivision OV2640 CMOS sensor
 
-The Omnivision OV2640 sensor support multiple resolutions output, such as
-CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB
-output format.
+The Omnivision OV2640 sensor supports multiple resolutions output, such as
+CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
+output formats.
 
 Required Properties:
 - compatible: should be "ovti,ov2640"
@@ -20,26 +20,21 @@ 
Documentation/devicetree/bindings/media/video-interfaces.txt.
 Example:
 
i2c1: i2c@f0018000 {
-   ov2640: camera@0x30 {
+   ov2640: camera@30 {
compatible = "ovti,ov2640";
reg = <0x30>;
-
pinctrl-names = "default";
-   pinctrl-0 = <_pck1 _ov2640_pwdn 
_ov2640_resetb>;
-
-   resetb-gpios = < 24 GPIO_ACTIVE_LOW>;
-   pwdn-gpios = < 29 GPIO_ACTIVE_HIGH>;
-
-   clocks = <>;
+   pinctrl-0 = <_pck0_as_isi_mck 
_sensor_power _sensor_reset>;
+   resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
+   pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
clock-names = "xvclk";
-
-   assigned-clocks = <>;
+   assigned-clocks = <>;
assigned-clock-rates = <2500>;
 
port {
ov2640_0: endpoint {
remote-endpoint = <_0>;
-   bus-width = <8>;
};
};
};
-- 
2.11.0

[PATCHv4 11/15] ov2640: use standard clk and enable it.

[Hans Verkuil]

From: Hans Verkuil 

Convert v4l2_clk to normal clk and enable the clock.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/ov2640.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 83f88efbce69..0445963c5fae 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -24,7 +25,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -284,7 +284,7 @@ struct ov2640_priv {
struct v4l2_subdev  subdev;
struct v4l2_ctrl_handlerhdl;
u32 cfmt_code;
-   struct v4l2_clk *clk;
+   struct clk  *clk;
const struct ov2640_win_size*win;
 
struct gpio_desc *resetb_gpio;
@@ -1051,14 +1051,11 @@ static int ov2640_probe(struct i2c_client *client,
return -ENOMEM;
}
 
-   priv->clk = v4l2_clk_get(>dev, "xvclk");
-   if (IS_ERR(priv->clk))
-   return -EPROBE_DEFER;
-
-   if (!client->dev.of_node) {
-   dev_err(>dev, "Missing platform_data for driver\n");
-   ret = -EINVAL;
-   goto err_clk;
+   if (client->dev.of_node) {
+   priv->clk = devm_clk_get(>dev, "xvclk");
+   if (IS_ERR(priv->clk))
+   return -EPROBE_DEFER;
+   clk_prepare_enable(priv->clk);
}
 
ret = ov2640_probe_dt(client, priv);
@@ -1074,25 +1071,25 @@ static int ov2640_probe(struct i2c_client *client,
priv->subdev.ctrl_handler = >hdl;
if (priv->hdl.error) {
ret = priv->hdl.error;
-   goto err_clk;
+   goto err_hdl;
}
 
ret = ov2640_video_probe(client);
if (ret < 0)
-   goto err_videoprobe;
+   goto err_hdl;
 
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
-   goto err_videoprobe;
+   goto err_hdl;
 
dev_info(>dev, "OV2640 Probed\n");
 
return 0;
 
-err_videoprobe:
+err_hdl:
v4l2_ctrl_handler_free(>hdl);
 err_clk:
-   v4l2_clk_put(priv->clk);
+   clk_disable_unprepare(priv->clk);
return ret;
 }
 
@@ -1101,9 +1098,9 @@ static int ov2640_remove(struct i2c_client *client)
struct ov2640_priv   *priv = to_ov2640(client);
 
v4l2_async_unregister_subdev(>subdev);
-   v4l2_clk_put(priv->clk);
-   v4l2_device_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
+   v4l2_device_unregister_subdev(>subdev);
+   clk_disable_unprepare(priv->clk);
return 0;
 }
 
-- 
2.11.0

[PATCHv4 10/15] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Hans Verkuil]

From: Hans Verkuil 

Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
uses this driver, so it can safely be converted.

Note: the s_power op has been dropped: this never worked. When the last open()
is closed, then the power is turned off, and when it is opened again the power
is turned on again, but the old state isn't restored.

Someone else can figure out in the future how to get this working correctly,
but I don't want to spend more time on this.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/Kconfig   | 11 
 drivers/media/i2c/Makefile  |  1 +
 drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +
 drivers/media/i2c/soc_camera/Kconfig|  6 --
 drivers/media/i2c/soc_camera/Makefile   |  1 -
 5 files changed, 27 insertions(+), 81 deletions(-)
 rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cee1dae6e014..db2c63f592c5 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_OV2640
+   tristate "OmniVision OV2640 sensor support"
+   depends on VIDEO_V4L2 && I2C && GPIOLIB
+   depends on MEDIA_CAMERA_SUPPORT
+   help
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2640 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2640.
+
 config VIDEO_OV2659
tristate "OmniVision OV2659 sensor support"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 5bc7bbeb5499..50af1e11c85a 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
 obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
similarity index 94%
rename from drivers/media/i2c/soc_camera/ov2640.c
rename to drivers/media/i2c/ov2640.c
index b9a0069f5b33..83f88efbce69 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -24,8 +24,8 @@
 #include 
 #include 
 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -287,7 +287,6 @@ struct ov2640_priv {
struct v4l2_clk *clk;
const struct ov2640_win_size*win;
 
-   struct soc_camera_subdev_desc   ssdd_dt;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
 };
@@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
 }
 
 /*
- * soc_camera_ops functions
+ * functions
  */
-static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
-{
-   return 0;
-}
-
 static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
 {
struct v4l2_subdev *sd =
@@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
 static int ov2640_s_power(struct v4l2_subdev *sd, int on)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
-   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
struct ov2640_priv *priv = to_ov2640(client);
 
-   return soc_camera_set_power(>dev, ssdd, priv->clk, on);
+   gpiod_direction_output(priv->pwdn_gpio, !on);
+   if (on && priv->resetb_gpio) {
+   /* Active the resetb pin to perform a reset pulse */
+   gpiod_direction_output(priv->resetb_gpio, 1);
+   usleep_range(3000, 5000);
+   gpiod_direction_output(priv->resetb_gpio, 0);
+   }
+   return 0;
 }
 
 /* Select the nearest higher resolution for capture */
@@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops 
= {
.s_power= ov2640_s_power,
 };
 
-static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
-   struct v4l2_mbus_config *cfg)
-{
-   struct i2c_client *client = v4l2_get_subdevdata(sd);
-   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
-
-   cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
-   V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
-   V4L2_MBUS_DATA_ACTIVE_HIGH;
-   cfg->type = V4L2_MBUS_PARALLEL;
-   cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
-
-   return 0;
-}
-
-static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
-   .s_stream   = ov2640_s_stream,
-   .g_mbus_config  = ov2640_g_mbus_config,
-};
-
 static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
   

[PATCHv4 07/15] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

From: Hans Verkuil 

This patch converts the atmel-isi driver from a soc-camera driver to a driver
that is stand-alone.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/soc_camera/ov2640.c |   23 +-
 drivers/media/platform/soc_camera/Kconfig |3 +-
 drivers/media/platform/soc_camera/atmel-isi.c | 1223 +++--
 3 files changed, 735 insertions(+), 514 deletions(-)

diff --git a/drivers/media/i2c/soc_camera/ov2640.c 
b/drivers/media/i2c/soc_camera/ov2640.c
index 56de18263359..b9a0069f5b33 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -794,10 +794,11 @@ static int ov2640_set_params(struct i2c_client *client, 
u32 *width, u32 *height,
dev_dbg(>dev, "%s: Selected cfmt YUYV (YUV422)", 
__func__);
selected_cfmt_regs = ov2640_yuyv_regs;
break;
-   default:
case MEDIA_BUS_FMT_UYVY8_2X8:
+   default:
dev_dbg(>dev, "%s: Selected cfmt UYVY", __func__);
selected_cfmt_regs = ov2640_uyvy_regs;
+   break;
}
 
/* reset hardware */
@@ -865,17 +866,7 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
mf->width   = priv->win->width;
mf->height  = priv->win->height;
mf->code= priv->cfmt_code;
-
-   switch (mf->code) {
-   case MEDIA_BUS_FMT_RGB565_2X8_BE:
-   case MEDIA_BUS_FMT_RGB565_2X8_LE:
-   mf->colorspace = V4L2_COLORSPACE_SRGB;
-   break;
-   default:
-   case MEDIA_BUS_FMT_YUYV8_2X8:
-   case MEDIA_BUS_FMT_UYVY8_2X8:
-   mf->colorspace = V4L2_COLORSPACE_JPEG;
-   }
+   mf->colorspace  = V4L2_COLORSPACE_SRGB;
mf->field   = V4L2_FIELD_NONE;
 
return 0;
@@ -897,17 +888,17 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
ov2640_select_win(>width, >height);
 
mf->field   = V4L2_FIELD_NONE;
+   mf->colorspace  = V4L2_COLORSPACE_SRGB;
 
switch (mf->code) {
case MEDIA_BUS_FMT_RGB565_2X8_BE:
case MEDIA_BUS_FMT_RGB565_2X8_LE:
-   mf->colorspace = V4L2_COLORSPACE_SRGB;
+   case MEDIA_BUS_FMT_YUYV8_2X8:
+   case MEDIA_BUS_FMT_UYVY8_2X8:
break;
default:
mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
-   case MEDIA_BUS_FMT_YUYV8_2X8:
-   case MEDIA_BUS_FMT_UYVY8_2X8:
-   mf->colorspace = V4L2_COLORSPACE_JPEG;
+   break;
}
 
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
diff --git a/drivers/media/platform/soc_camera/Kconfig 
b/drivers/media/platform/soc_camera/Kconfig
index 86d74788544f..a37ec91b026e 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
 
 config VIDEO_ATMEL_ISI
tristate "ATMEL Image Sensor Interface (ISI) support"
-   depends on VIDEO_DEV && SOC_CAMERA
+   depends on VIDEO_V4L2 && OF && HAS_DMA
depends on ARCH_AT91 || COMPILE_TEST
-   depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
---help---
  This module makes the ATMEL Image Sensor Interface available
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
b/drivers/media/platform/soc_camera/atmel-isi.c
index 46de657c3e6d..a837b94281ef 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -22,18 +22,22 @@
 #include 
 #include 
 #include 
-
-#include 
-#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
 
 #include "atmel-isi.h"
 
-#define MAX_BUFFER_NUM 32
 #define MAX_SUPPORT_WIDTH  2048
 #define MAX_SUPPORT_HEIGHT 2048
-#define VID_LIMIT_BYTES(16 * 1024 * 1024)
 #define MIN_FRAME_RATE 15
 #define FRAME_INTERVAL_MILLI_SEC   (1000 / MIN_FRAME_RATE)
 
@@ -65,9 +69,37 @@ struct frame_buffer {
struct list_head list;
 };
 
+struct isi_graph_entity {
+   struct device_node *node;
+
+   struct v4l2_async_subdev asd;
+   struct v4l2_subdev *subdev;
+};
+
+/*
+ * struct isi_format - ISI media bus format information
+ * @fourcc:Fourcc code for this format
+ * @mbus_code: V4L2 media bus format code.
+ * @bpp:   Bytes per pixel (when stored in memory)
+ * @swap:  Byte swap configuration value
+ * @support:   Indicates format supported by subdev
+ * @skip:  Skip duplicate format supported by subdev
+ */
+struct isi_format {
+   u32 fourcc;
+   u32 mbus_code;
+   u8  bpp;
+   u32 swap;
+
+   boolsupport;
+   boolskip;
+};
+
+
 struct atmel_isi {
/* Protects the access of variables shared with the ISR */
-   spinlock_t  lock;
+  

[PATCHv4 12/15] ov2640: add MC support

[Hans Verkuil]

From: Hans Verkuil 

The MC support is needed by the em28xx driver.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/ov2640.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 0445963c5fae..00df042fd6f1 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -282,6 +282,9 @@ struct ov2640_win_size {
 
 struct ov2640_priv {
struct v4l2_subdev  subdev;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_pad pad;
+#endif
struct v4l2_ctrl_handlerhdl;
u32 cfmt_code;
struct clk  *clk;
@@ -1073,19 +1076,30 @@ static int ov2640_probe(struct i2c_client *client,
ret = priv->hdl.error;
goto err_hdl;
}
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   priv->pad.flags = MEDIA_PAD_FL_SOURCE;
+   priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   ret = media_entity_pads_init(>subdev.entity, 1, >pad);
+   if (ret < 0)
+   goto err_hdl;
+#endif
 
ret = ov2640_video_probe(client);
if (ret < 0)
-   goto err_hdl;
+   goto err_videoprobe;
 
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
-   goto err_hdl;
+   goto err_videoprobe;
 
dev_info(>dev, "OV2640 Probed\n");
 
return 0;
 
+err_videoprobe:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>subdev.entity);
+#endif
 err_hdl:
v4l2_ctrl_handler_free(>hdl);
 err_clk:
@@ -1099,6 +1113,9 @@ static int ov2640_remove(struct i2c_client *client)
 
v4l2_async_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>subdev.entity);
+#endif
v4l2_device_unregister_subdev(>subdev);
clk_disable_unprepare(priv->clk);
return 0;
-- 
2.11.0

[PATCHv4 15/15] at91-sama5d3_xplained.dts: select ov2640

[Hans Verkuil]

From: Hans Verkuil 

This patch replaces the ov7670 with the ov2640. This patch is not
meant to be merged but is for demonstration purposes only.

Signed-off-by: Hans Verkuil 
---
 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts 
b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index c6b07f83578b..6951bd6a11d7 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -71,7 +71,7 @@
#address-cells = <1>;
#size-cells = <0>;
isi_0: endpoint {
-   remote-endpoint = <_0>;
+   remote-endpoint = <_0>;
bus-width = <8>;
vsync-active = <1>;
hsync-active = <1>;
@@ -87,20 +87,20 @@
i2c1: i2c@f0018000 {
status = "okay";
 
-   ov7670: camera@21 {
-   compatible = "ovti,ov7670";
-   reg = <0x21>;
+   ov2640: camera@30 {
+   compatible = "ovti,ov2640";
+   reg = <0x30>;
pinctrl-names = "default";
pinctrl-0 = <_pck0_as_isi_mck 
_sensor_power _sensor_reset>;
-   reset-gpios = < 11 
GPIO_ACTIVE_LOW>;
-   powerdown-gpios = < 13 
GPIO_ACTIVE_HIGH>;
+   resetb-gpios = < 11 
GPIO_ACTIVE_LOW>;
+   pwdn-gpios = < 13 
GPIO_ACTIVE_HIGH>;
clocks = <>;
-   clock-names = "xclk";
+   clock-names = "xvclk";
assigned-clocks = <>;
assigned-clock-rates = <2500>;
 
port {
-   ov7670_0: endpoint {
+   ov2640_0: endpoint {
remote-endpoint = 
<_0>;
};
};
-- 
2.11.0

[PATCHv4 01/15] ov7670: document device tree bindings

[Hans Verkuil]

From: Hans Verkuil 

Add binding documentation and add that file to the MAINTAINERS entry.

Signed-off-by: Hans Verkuil 
---
 .../devicetree/bindings/media/i2c/ov7670.txt   | 43 ++
 MAINTAINERS|  1 +
 2 files changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
new file mode 100644
index ..826b6563b009
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
@@ -0,0 +1,43 @@
+* Omnivision OV7670 CMOS sensor
+
+The Omnivision OV7670 sensor supports multiple resolutions output, such as
+CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
+output formats.
+
+Required Properties:
+- compatible: should be "ovti,ov7670"
+- clocks: reference to the xclk input clock.
+- clock-names: should be "xclk".
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+  Active is low.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+  Active is high.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+   i2c1: i2c@f0018000 {
+   ov7670: camera@21 {
+   compatible = "ovti,ov7670";
+   reg = <0x21>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pck0_as_isi_mck 
_sensor_power _sensor_reset>;
+   reset-gpios = < 11 GPIO_ACTIVE_LOW>;
+   powerdown-gpios = < 13 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+   clock-names = "xclk";
+   assigned-clocks = <>;
+   assigned-clock-rates = <2500>;
+
+   port {
+   ov7670_0: endpoint {
+   remote-endpoint = <_0>;
+   };
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 83a42ef1d1a7..93500928ca4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9273,6 +9273,7 @@ L:linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/ov7670.c
+F: Documentation/devicetree/bindings/media/i2c/ov7670.txt
 
 ONENAND FLASH DRIVER
 M: Kyungmin Park 
-- 
2.11.0

[PATCHv4 03/15] ov7670: fix g/s_parm

[Hans Verkuil]

From: Hans Verkuil 

Drop unnecesary memset. Drop the unnecessary extendedmode check and
set the V4L2_CAP_TIMEPERFRAME capability.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/i2c/ov7670.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 9af8d3b8f848..50e4466a2b37 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1046,7 +1046,6 @@ static int ov7670_g_parm(struct v4l2_subdev *sd, struct 
v4l2_streamparm *parms)
if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
-   memset(cp, 0, sizeof(struct v4l2_captureparm));
cp->capability = V4L2_CAP_TIMEPERFRAME;
info->devtype->get_framerate(sd, >timeperframe);
 
@@ -1061,9 +1060,8 @@ static int ov7670_s_parm(struct v4l2_subdev *sd, struct 
v4l2_streamparm *parms)
 
if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
-   if (cp->extendedmode != 0)
-   return -EINVAL;
 
+   cp->capability = V4L2_CAP_TIMEPERFRAME;
return info->devtype->set_framerate(sd, tpf);
 }
 
-- 
2.11.0

[PATCHv4 04/15] ov7670: get xclk

[Hans Verkuil]

From: Hans Verkuil 

Get the clock for this sensor.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/ov7670.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 50e4466a2b37..912ff09c6100 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -10,6 +10,7 @@
  * This file may be distributed under the terms of the GNU General
  * Public License, version 2.
  */
+#include 
 #include 
 #include 
 #include 
@@ -227,6 +228,7 @@ struct ov7670_info {
struct v4l2_ctrl *hue;
};
struct ov7670_format_struct *fmt;  /* Current format */
+   struct clk *clk;
int min_width;  /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
@@ -1587,13 +1589,24 @@ static int ov7670_probe(struct i2c_client *client,
info->pclk_hb_disable = true;
}
 
+   info->clk = devm_clk_get(>dev, "xclk");
+   if (IS_ERR(info->clk))
+   return -EPROBE_DEFER;
+   clk_prepare_enable(info->clk);
+
+   info->clock_speed = clk_get_rate(info->clk) / 100;
+   if (info->clock_speed < 10 || info->clock_speed > 48) {
+   ret = -EINVAL;
+   goto clk_disable;
+   }
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
v4l_dbg(1, debug, client,
"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
client->addr << 1, client->adapter->name);
-   return ret;
+   goto clk_disable;
}
v4l_info(client, "chip found @ 0x%02x (%s)\n",
client->addr << 1, client->adapter->name);
@@ -1656,6 +1669,8 @@ static int ov7670_probe(struct i2c_client *client,
 
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
+clk_disable:
+   clk_disable_unprepare(info->clk);
return ret;
 }
 
@@ -1667,6 +1682,7 @@ static int ov7670_remove(struct i2c_client *client)
 
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
+   clk_disable_unprepare(info->clk);
return 0;
 }
 
-- 
2.11.0

[PATCHv4 08/15] atmel-isi: move out of soc_camera to atmel

[Hans Verkuil]

From: Hans Verkuil 

Move this out of the soc_camera directory into the atmel directory
where it belongs.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/Makefile  |  1 +
 drivers/media/platform/atmel/Kconfig | 11 ++-
 drivers/media/platform/atmel/Makefile|  1 +
 drivers/media/platform/{soc_camera => atmel}/atmel-isi.c |  0
 drivers/media/platform/{soc_camera => atmel}/atmel-isi.h |  0
 drivers/media/platform/soc_camera/Kconfig| 10 --
 drivers/media/platform/soc_camera/Makefile   |  1 -
 7 files changed, 12 insertions(+), 12 deletions(-)
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.c (100%)
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.h (100%)

diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 8959f6e6692a..c491731f5909 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_XILINX)+= xilinx/
 obj-$(CONFIG_VIDEO_RCAR_VIN)   += rcar-vin/
 
 obj-$(CONFIG_VIDEO_ATMEL_ISC)  += atmel/
+obj-$(CONFIG_VIDEO_ATMEL_ISI)  += atmel/
 
 ccflags-y += -I$(srctree)/drivers/media/i2c
 
diff --git a/drivers/media/platform/atmel/Kconfig 
b/drivers/media/platform/atmel/Kconfig
index 867dca22a473..9bd0f19b127f 100644
--- a/drivers/media/platform/atmel/Kconfig
+++ b/drivers/media/platform/atmel/Kconfig
@@ -6,4 +6,13 @@ config VIDEO_ATMEL_ISC
select REGMAP_MMIO
help
   This module makes the ATMEL Image Sensor Controller available
-  as a v4l2 device.
\ No newline at end of file
+  as a v4l2 device.
+
+config VIDEO_ATMEL_ISI
+   tristate "ATMEL Image Sensor Interface (ISI) support"
+   depends on VIDEO_V4L2 && OF && HAS_DMA
+   depends on ARCH_AT91 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   ---help---
+ This module makes the ATMEL Image Sensor Interface available
+ as a v4l2 device.
diff --git a/drivers/media/platform/atmel/Makefile 
b/drivers/media/platform/atmel/Makefile
index 9d7c999d434d..27000d099a5e 100644
--- a/drivers/media/platform/atmel/Makefile
+++ b/drivers/media/platform/atmel/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
+obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
similarity index 100%
rename from drivers/media/platform/soc_camera/atmel-isi.c
rename to drivers/media/platform/atmel/atmel-isi.c
diff --git a/drivers/media/platform/soc_camera/atmel-isi.h 
b/drivers/media/platform/atmel/atmel-isi.h
similarity index 100%
rename from drivers/media/platform/soc_camera/atmel-isi.h
rename to drivers/media/platform/atmel/atmel-isi.h
diff --git a/drivers/media/platform/soc_camera/Kconfig 
b/drivers/media/platform/soc_camera/Kconfig
index a37ec91b026e..0c581aa1b18a 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -26,13 +26,3 @@ config VIDEO_SH_MOBILE_CEU
select SOC_CAMERA_SCALE_CROP
---help---
  This is a v4l2 driver for the SuperH Mobile CEU Interface
-
-config VIDEO_ATMEL_ISI
-   tristate "ATMEL Image Sensor Interface (ISI) support"
-   depends on VIDEO_V4L2 && OF && HAS_DMA
-   depends on ARCH_AT91 || COMPILE_TEST
-   select VIDEOBUF2_DMA_CONTIG
-   ---help---
- This module makes the ATMEL Image Sensor Interface available
- as a v4l2 device.
-
diff --git a/drivers/media/platform/soc_camera/Makefile 
b/drivers/media/platform/soc_camera/Makefile
index 7633a0f2f66f..07a451e8b228 100644
--- a/drivers/media/platform/soc_camera/Makefile
+++ b/drivers/media/platform/soc_camera/Makefile
@@ -6,5 +6,4 @@ obj-$(CONFIG_SOC_CAMERA_SCALE_CROP) += soc_scale_crop.o
 obj-$(CONFIG_SOC_CAMERA_PLATFORM)  += soc_camera_platform.o
 
 # soc-camera host drivers have to be linked after camera drivers
-obj-$(CONFIG_VIDEO_ATMEL_ISI)  += atmel-isi.o
 obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)  += sh_mobile_ceu_camera.o
-- 
2.11.0

[PATCHv4 05/15] ov7670: add devicetree support

[Hans Verkuil]

From: Hans Verkuil 

Add DT support. Use it to get the reset and pwdn pins (if there are any).
Tested with one sensor requiring reset/pwdn and one sensor that doesn't
have reset/pwdn pins.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/ov7670.c | 40 ++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 912ff09c6100..7270c68ed18a 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -17,6 +17,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -229,6 +231,8 @@ struct ov7670_info {
};
struct ov7670_format_struct *fmt;  /* Current format */
struct clk *clk;
+   struct gpio_desc *resetb_gpio;
+   struct gpio_desc *pwdn_gpio;
int min_width;  /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
@@ -591,8 +595,6 @@ static int ov7670_init(struct v4l2_subdev *sd, u32 val)
return ov7670_write_array(sd, ov7670_default_regs);
 }
 
-
-
 static int ov7670_detect(struct v4l2_subdev *sd)
 {
unsigned char v;
@@ -1549,6 +1551,27 @@ static const struct ov7670_devtype ov7670_devdata[] = {
},
 };
 
+static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info 
*info)
+{
+   info->pwdn_gpio = devm_gpiod_get_optional(>dev, "powerdown",
+   GPIOD_OUT_LOW);
+   if (IS_ERR(info->pwdn_gpio)) {
+   dev_info(>dev, "can't get %s GPIO\n", "powerdown");
+   return PTR_ERR(info->pwdn_gpio);
+   }
+
+   info->resetb_gpio = devm_gpiod_get_optional(>dev, "reset",
+   GPIOD_OUT_LOW);
+   if (IS_ERR(info->resetb_gpio)) {
+   dev_info(>dev, "can't get %s GPIO\n", "reset");
+   return PTR_ERR(info->resetb_gpio);
+   }
+
+   usleep_range(3000, 5000);
+
+   return 0;
+}
+
 static int ov7670_probe(struct i2c_client *client,
const struct i2c_device_id *id)
 {
@@ -1594,6 +1617,10 @@ static int ov7670_probe(struct i2c_client *client,
return -EPROBE_DEFER;
clk_prepare_enable(info->clk);
 
+   ret = ov7670_init_gpio(client, info);
+   if (ret)
+   goto clk_disable;
+
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
@@ -1693,9 +1720,18 @@ static const struct i2c_device_id ov7670_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ov7670_id);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id ov7670_of_match[] = {
+   { .compatible = "ovti,ov7670", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ov7670_of_match);
+#endif
+
 static struct i2c_driver ov7670_driver = {
.driver = {
.name   = "ov7670",
+   .of_match_table = of_match_ptr(ov7670_of_match),
},
.probe  = ov7670_probe,
.remove = ov7670_remove,
-- 
2.11.0

[PATCHv4 06/15] atmel-isi: document device tree bindings

[Hans Verkuil]

From: Hans Verkuil 

Document the device tree bindings for this hardware.

Mostly copied from the atmel-isc bindings.

Signed-off-by: Hans Verkuil 
---
 .../devicetree/bindings/media/atmel-isi.txt| 96 +-
 1 file changed, 58 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
b/Documentation/devicetree/bindings/media/atmel-isi.txt
index 251f008f220c..65249bbd5c00 100644
--- a/Documentation/devicetree/bindings/media/atmel-isi.txt
+++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
@@ -1,51 +1,71 @@
-Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
---
-
-Required properties:
-- compatible: must be "atmel,at91sam9g45-isi"
-- reg: physical base address and length of the registers set for the device;
-- interrupts: should contain IRQ line for the ISI;
-- clocks: list of clock specifiers, corresponding to entries in
-  the clock-names property;
-- clock-names: must contain "isi_clk", which is the isi peripherial clock.
-
-ISI supports a single port node with parallel bus. It should contain one
+Atmel Image Sensor Interface (ISI)
+--
+
+Required properties for ISI:
+- compatible: must be "atmel,at91sam9g45-isi".
+- reg: physical base address and length of the registers set for the device.
+- interrupts: should contain IRQ line for the ISI.
+- clocks: list of clock specifiers, corresponding to entries in the clock-names
+   property; please refer to clock-bindings.txt.
+- clock-names: required elements: "isi_clk".
+- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
+
+ISI supports a single port node with parallel bus. It shall contain one
 'port' child node with child 'endpoint' node. Please refer to the bindings
 defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
 
-Example:
-   isi: isi@f0034000 {
-   compatible = "atmel,at91sam9g45-isi";
-   reg = <0xf0034000 0x4000>;
-   interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+Endpoint node properties
+
 
-   clocks = <_clk>;
-   clock-names = "isi_clk";
+- bus-width: <8> or <10> (mandatory)
+- hsync-active (default: active high)
+- vsync-active (default: active high)
+- pclk-sample (default: sample on falling edge)
+- remote-endpoint: A phandle to the bus receiver's endpoint node (mandatory).
 
-   pinctrl-names = "default";
-   pinctrl-0 = <_isi>;
-
-   port {
-   #address-cells = <1>;
-   #size-cells = <0>;
+Example:
 
-   isi_0: endpoint {
-   remote-endpoint = <_0>;
-   bus-width = <8>;
-   };
+isi: isi@f0034000 {
+   compatible = "atmel,at91sam9g45-isi";
+   reg = <0xf0034000 0x4000>;
+   interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_isi_data_0_7>;
+   clocks = <_clk>;
+   clock-names = "isi_clk";
+   status = "ok";
+   port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   isi_0: endpoint {
+   remote-endpoint = <_0>;
+   bus-width = <8>;
+   vsync-active = <1>;
+   hsync-active = <1>;
};
};
+};
+
+i2c1: i2c@f0018000 {
+   status = "okay";
 
-   i2c1: i2c@f0018000 {
-   ov2640: camera@0x30 {
-   compatible = "ovti,ov2640";
-   reg = <0x30>;
+   ov2640: camera@30 {
+   compatible = "ovti,ov2640";
+   reg = <0x30>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
_sensor_reset>;
+   resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
+   pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+   clock-names = "xvclk";
+   assigned-clocks = <>;
+   assigned-clock-rates = <2500>;
 
-   port {
-   ov2640_0: endpoint {
-   remote-endpoint = <_0>;
-   bus-width = <8>;
-   };
+   port {
+   ov2640_0: endpoint {
+   remote-endpoint = <_0>;
+   bus-width = <8>;
};
};
};
+};
-- 
2.11.0

[PATCHv4 13/15] em28xx: drop last soc_camera link

[Hans Verkuil]

From: Hans Verkuil 

The em28xx driver still used the soc_camera.h header for the ov2640
driver. Since this driver no longer uses soc_camera, that include can
be removed.

Signed-off-by: Hans Verkuil 
---
 drivers/media/usb/em28xx/em28xx-camera.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c 
b/drivers/media/usb/em28xx/em28xx-camera.c
index 89c890ba7dd6..63aaa577a742 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -23,7 +23,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -43,13 +42,6 @@ static unsigned short omnivision_sensor_addrs[] = {
I2C_CLIENT_END
 };
 
-static struct soc_camera_link camlink = {
-   .bus_id = 0,
-   .flags = 0,
-   .module_name = "em28xx",
-   .unbalanced_power = true,
-};
-
 /* FIXME: Should be replaced by a proper mt9m111 driver */
 static int em28xx_initialize_mt9m111(struct em28xx *dev)
 {
@@ -421,7 +413,6 @@ int em28xx_init_camera(struct em28xx *dev)
.type = "ov2640",
.flags = I2C_CLIENT_SCCB,
.addr = client->addr,
-   .platform_data = ,
};
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-- 
2.11.0

[PATCHv4 00/15] atmel-isi/ov7670/ov2640: convert to standalone drivers

[Hans Verkuil]

From: Hans Verkuil 

This patch series converts the soc-camera atmel-isi to a standalone V4L2
driver.

The same is done for the ov7670 and ov2640 sensor drivers: the ov7670 was
used to test the atmel-isi driver. The ov2640 is needed because the em28xx
driver has a soc_camera include dependency. Both ov7670 and ov2640 sensors
have been tested with the atmel-isi driver.

The first 5 patches improve the ov7670 sensor driver, mostly adding modern
features such as DT support.

The next three convert the atmel-isi and move it out of soc_camera.

The following 5 patches convert ov2640 and drop the soc_camera dependency
in em28xx. I have tested that this works with my 'SpeedLink Vicious And
Divine Laplace webcam'.

The last two patches add isi support to the DT: the first for the ov7670
sensor, the second modifies it for the ov2640 sensor.

These two final patches are for demonstration purposes only, I do not plan
on merging them.

Tested with my sama5d3-Xplained board, the ov2640 sensor and two ov7670
sensors: one with and one without reset/pwdn pins. Also tested with my
em28xx-based webcam.

I'd like to get this in for 4.12. Fingers crossed.

Regards,

Hans

Changes since v3:
- ov2640/ov7670: call clk_disable_unprepare where needed. I assumed this was
  done by the devm_clk_get cleanup, but that wasn't the case.
- bindings: be even more explicit about which properties are mandatory.
- ov2640/ov7670: drop unused bus-width from the dts binding examples and from
  the actual dts patches.

Changes since v2:
- Incorporated Sakari's and Rob's device tree bindings comments.
- ov2640: dropped the reset/power changes. These actually broke the em28xx
  and there was really nothing wrong with it.
- merged the "ov2640: allow use inside em28xx" into patches 10 and 11.
  It really shouldn't have been a separate patch in the first place.
- rebased on top of 4.11-rc1.

Changes since v1:

- Dropped MC support from atmel-isi and ov7670: not needed to make this
  work. Only for the ov2640 was it kept since the em28xx driver requires it.
- Use devm_clk_get instead of clk_get.
- The ov7670 lower limit of the clock speed is 10 MHz instead of 12. Adjust
  accordingly.

Hans Verkuil (15):
  ov7670: document device tree bindings
  ov7670: call v4l2_async_register_subdev
  ov7670: fix g/s_parm
  ov7670: get xclk
  ov7670: add devicetree support
  atmel-isi: document device tree bindings
  atmel-isi: remove dependency of the soc-camera framework
  atmel-isi: move out of soc_camera to atmel
  ov2640: update bindings
  ov2640: convert from soc-camera to a standard subdev sensor driver.
  ov2640: use standard clk and enable it.
  ov2640: add MC support
  em28xx: drop last soc_camera link
  sama5d3 dts: enable atmel-isi
  at91-sama5d3_xplained.dts: select ov2640

 .../devicetree/bindings/media/atmel-isi.txt|   96 +-
 .../devicetree/bindings/media/i2c/ov2640.txt   |   23 +-
 .../devicetree/bindings/media/i2c/ov7670.txt   |   43 +
 MAINTAINERS|1 +
 arch/arm/boot/dts/at91-sama5d3_xplained.dts|   59 +-
 arch/arm/boot/dts/sama5d3.dtsi |4 +-
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/{soc_camera => }/ov2640.c|  152 +--
 drivers/media/i2c/ov7670.c |   75 +-
 drivers/media/i2c/soc_camera/Kconfig   |6 -
 drivers/media/i2c/soc_camera/Makefile  |1 -
 drivers/media/platform/Makefile|1 +
 drivers/media/platform/atmel/Kconfig   |   11 +-
 drivers/media/platform/atmel/Makefile  |1 +
 drivers/media/platform/atmel/atmel-isi.c   | 1398 
 .../platform/{soc_camera => atmel}/atmel-isi.h |0
 drivers/media/platform/soc_camera/Kconfig  |   11 -
 drivers/media/platform/soc_camera/Makefile |1 -
 drivers/media/platform/soc_camera/atmel-isi.c  | 1167 
 drivers/media/usb/em28xx/em28xx-camera.c   |9 -
 21 files changed, 1704 insertions(+), 1367 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
 rename drivers/media/i2c/{soc_camera => }/ov2640.c (92%)
 create mode 100644 drivers/media/platform/atmel/atmel-isi.c
 rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.h (100%)
 delete mode 100644 drivers/media/platform/soc_camera/atmel-isi.c

-- 
2.11.0

[PATCHv4 14/15] sama5d3 dts: enable atmel-isi

[Hans Verkuil]

From: Hans Verkuil 

This illustrates the changes needed to the dts in order to hook up the
ov7670. I don't plan on merging this.

Signed-off-by: Hans Verkuil 
---
 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 59 ++---
 arch/arm/boot/dts/sama5d3.dtsi  |  4 +-
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts 
b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index c51fc652f6c7..c6b07f83578b 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -65,18 +65,51 @@
status = "okay";
};
 
+   isi0: isi@f0034000 {
+   status = "okay";
+   port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   isi_0: endpoint {
+   remote-endpoint = <_0>;
+   bus-width = <8>;
+   vsync-active = <1>;
+   hsync-active = <1>;
+   };
+   };
+   };
+
i2c0: i2c@f0014000 {
pinctrl-0 = <_i2c0_pu>;
-   status = "okay";
+   status = "disabled";
};
 
i2c1: i2c@f0018000 {
status = "okay";
 
+   ov7670: camera@21 {
+   compatible = "ovti,ov7670";
+   reg = <0x21>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pck0_as_isi_mck 
_sensor_power _sensor_reset>;
+   reset-gpios = < 11 
GPIO_ACTIVE_LOW>;
+   powerdown-gpios = < 13 
GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+   clock-names = "xclk";
+   assigned-clocks = <>;
+   assigned-clock-rates = <2500>;
+
+   port {
+   ov7670_0: endpoint {
+   remote-endpoint = 
<_0>;
+   };
+   };
+   };
+
pmic: act8865@5b {
compatible = "active-semi,act8865";
reg = <0x5b>;
-   status = "disabled";
+   status = "okay";
 
regulators {
vcc_1v8_reg: DCDC_REG1 {
@@ -130,7 +163,7 @@
pwm0: pwm@f002c000 {
pinctrl-names = "default";
pinctrl-0 = <_pwm0_pwmh0_0 
_pwm0_pwmh1_0>;
-   status = "okay";
+   status = "disabled";
};
 
usart0: serial@f001c000 {
@@ -143,7 +176,7 @@
};
 
uart0: serial@f0024000 {
-   status = "okay";
+   status = "disabled";
};
 
mmc1: mmc@f800 {
@@ -181,7 +214,7 @@
i2c2: i2c@f801c000 {
dmas = <0>, <0>;/* Do not use DMA for 
i2c2 */
pinctrl-0 = <_i2c2_pu>;
-   status = "okay";
+   status = "disabled";
};
 
macb1: ethernet@f802c000 {
@@ -200,6 +233,22 @@
};
 
pinctrl@f200 {
+   camera_sensor {
+   pinctrl_pck0_as_isi_mck: 
pck0_as_isi_mck-0 {
+   atmel,pins =
+   ; /* ISI_MCK */
+   };
+
+   pinctrl_sensor_power: sensor_power-0 {
+   atmel,pins =
+   ;
+   };
+
+  

[PATCHv4 02/15] ov7670: call v4l2_async_register_subdev

[Hans Verkuil]

From: Hans Verkuil 

Add v4l2-async support for this driver.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/i2c/ov7670.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 56cfb5ca9c95..9af8d3b8f848 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1636,10 +1636,9 @@ static int ov7670_probe(struct i2c_client *client,
V4L2_EXPOSURE_AUTO);
sd->ctrl_handler = >hdl;
if (info->hdl.error) {
-   int err = info->hdl.error;
+   ret = info->hdl.error;
 
-   v4l2_ctrl_handler_free(>hdl);
-   return err;
+   goto hdl_free;
}
/*
 * We have checked empirically that hw allows to read back the gain
@@ -1651,7 +1650,15 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_cluster(2, >saturation);
v4l2_ctrl_handler_setup(>hdl);
 
+   ret = v4l2_async_register_subdev(>sd);
+   if (ret < 0)
+   goto hdl_free;
+
return 0;
+
+hdl_free:
+   v4l2_ctrl_handler_free(>hdl);
+   return ret;
 }
 
 
-- 
2.11.0

Re: [PATCHv3 05/15] ov7670: add devicetree support

[Hans Verkuil]

On 09/03/17 21:45, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:06PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Add DT support. Use it to get the reset and pwdn pins (if there are any).
>> Tested with one sensor requiring reset/pwdn and one sensor that doesn't
>> have reset/pwdn pins.
> 
> If I read the datasheet right, lifting the reset signal up will reset the
> sensor but the patch doesn't make use of that, it only ensures the reset
> signal stays low. Should you lift it up for a while as well? The datasheet
> doesn't say for how long that should be done, but that it should be usable
> after 1 ms since pulling reset down.

There does not seem to be any need for that. This sensor also comes in two
models: one with separate pwdn and reset pins, and one where it is just 
hardwired.

If the hardwired variant doesn't need a reset pulse, then neither does the
variant with pins. It works, and I am not really willing to experiment with 
this.

Regards,

Hans

> 
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/ov7670.c | 40 ++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index da0843617a49..f9d977430dcf 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -17,6 +17,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -229,6 +231,8 @@ struct ov7670_info {
>>  };
>>  struct ov7670_format_struct *fmt;  /* Current format */
>>  struct clk *clk;
>> +struct gpio_desc *resetb_gpio;
>> +struct gpio_desc *pwdn_gpio;
>>  int min_width;  /* Filter out smaller sizes */
>>  int min_height; /* Filter out smaller sizes */
>>  int clock_speed;/* External clock speed (MHz) */
>> @@ -591,8 +595,6 @@ static int ov7670_init(struct v4l2_subdev *sd, u32 val)
>>  return ov7670_write_array(sd, ov7670_default_regs);
>>  }
>>  
>> -
>> -
>>  static int ov7670_detect(struct v4l2_subdev *sd)
>>  {
>>  unsigned char v;
>> @@ -1549,6 +1551,27 @@ static const struct ov7670_devtype ov7670_devdata[] = 
>> {
>>  },
>>  };
>>  
>> +static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info 
>> *info)
>> +{
>> +info->pwdn_gpio = devm_gpiod_get_optional(>dev, "powerdown",
>> +GPIOD_OUT_LOW);
>> +if (IS_ERR(info->pwdn_gpio)) {
>> +dev_info(>dev, "can't get %s GPIO\n", "powerdown");
>> +return PTR_ERR(info->pwdn_gpio);
>> +}
>> +
>> +info->resetb_gpio = devm_gpiod_get_optional(>dev, "reset",
>> +GPIOD_OUT_LOW);
>> +if (IS_ERR(info->resetb_gpio)) {
>> +dev_info(>dev, "can't get %s GPIO\n", "reset");
>> +return PTR_ERR(info->resetb_gpio);
>> +}
>> +
>> +usleep_range(3000, 5000);
>> +
>> +return 0;
>> +}
>> +
>>  static int ov7670_probe(struct i2c_client *client,
>>  const struct i2c_device_id *id)
>>  {
>> @@ -1594,6 +1617,10 @@ static int ov7670_probe(struct i2c_client *client,
>>  return -EPROBE_DEFER;
>>  clk_prepare_enable(info->clk);
>>  
>> +ret = ov7670_init_gpio(client, info);
>> +if (ret)
>> +return ret;
>> +
>>  info->clock_speed = clk_get_rate(info->clk) / 100;
>>  if (info->clock_speed < 10 || info->clock_speed > 48)
>>  return -EINVAL;
>> @@ -1689,9 +1716,18 @@ static const struct i2c_device_id ov7670_id[] = {
>>  };
>>  MODULE_DEVICE_TABLE(i2c, ov7670_id);
>>  
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id ov7670_of_match[] = {
>> +{ .compatible = "ovti,ov7670", },
>> +{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ov7670_of_match);
>> +#endif
>> +
>>  static struct i2c_driver ov7670_driver = {
>>  .driver = {
>>  .name   = "ov7670",
>> +.of_match_table = of_match_ptr(ov7670_of_match),
>>  },
>>  .probe  = ov7670_probe,
>>  .remove = ov7670_remove,
> 

Re: [PATCH v4 13/36] [media] v4l2: add a frame timeout event

[Russell King - ARM Linux]

On Thu, Mar 09, 2017 at 06:38:18PM -0800, Steve Longerbeam wrote:
> On 03/05/2017 02:41 PM, Russell King - ARM Linux wrote:
> >I'm not sure that statement is entirely accurate.  With the IMX219
> >camera, I _could_ (with previous iterations of the iMX capture code)
> >stop it streaming, wait a while, and restart it, and everything
> >continues to work.
> 
> Hi Russell, did you see the "EOF timeout" kernel error message when you
> stopped the IMX219 from streaming? Only a "EOF timeout" message
> indicates the unrecoverable case.

I really couldn't tell you anymore - I can't go back and test at all,
because:

(a) your v4 patch set never worked for me
(b) I've now moved forward to v4.11-rc1, which conflicts with your v4
and older patch sets.

In any case, I'm in complete disagreement with many of the points that
Sakari has been bringing up, and I'm finding the direction that things
are progressing to be abhorrent.

I've discussed with Mauro about (eg) the application interface, and
unsurprisingly to me, Mauro immediately said about control inheritence
to the main v4l2 device, contradicting what Sakari has been saying.
The subdev API is supposed to be there to allow for finer control, it's
not a "one or the other" thing.  The controls are still supposed to be
exposed through the main v4l2 subdev device.

Since the v4l2 stuff is becoming abhorrent, and I've also come to
realise that I'm going to have to write an entirely new userspace
application to capture, debayer and encode efficiently, I'm finding
that I've little motivation to take much of a further interest in
iMX6 capture, or indeed continue my reverse engineering efforts of
the IMX219 sensor.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] [media] coda: fix warnings when compiling with 64 bits

[Hans Verkuil]

On 09/03/17 21:17, Mauro Carvalho Chehab wrote:
> drivers/media/platform/coda/coda-common.c: In function ‘coda_alloc_aux_buf’:
> ./include/linux/kern_levels.h:4:18: warning: format ‘%u’ expects argument of 
> type ‘unsigned int’, but argument 4 has type ‘size_t {aka long unsigned int}’ 
> [-Wformat=]
>  #define KERN_SOH "\001"  /* ASCII Start Of Header */
>   ^
> ./include/media/v4l2-common.h:69:9: note: in definition of macro ‘v4l2_printk’
>   printk(level "%s: " fmt, (dev)->name , ## arg)
>  ^
> ./include/linux/kern_levels.h:10:18: note: in expansion of macro ‘KERN_SOH’
>  #define KERN_ERR KERN_SOH "3" /* error conditions */
>   ^~~~
> ./include/media/v4l2-common.h:72:14: note: in expansion of macro ‘KERN_ERR’
>   v4l2_printk(KERN_ERR, dev, fmt , ## arg)
>   ^~~~
> drivers/media/platform/coda/coda-common.c:1341:3: note: in expansion of macro 
> ‘v4l2_err’
>v4l2_err(>v4l2_dev,
>^~~~

Yes, this too is in my pull request for coda :-)

Regards,

Hans

> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/coda/coda-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c 
> b/drivers/media/platform/coda/coda-common.c
> index cb76c96759b9..dc51ae2050cc 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1339,7 +1339,7 @@ int coda_alloc_aux_buf(struct coda_dev *dev, struct 
> coda_aux_buf *buf,
>   GFP_KERNEL);
>   if (!buf->vaddr) {
>   v4l2_err(>v4l2_dev,
> -  "Failed to allocate %s buffer of size %u\n",
> +  "Failed to allocate %s buffer of size %zd\n",
>name, size);
>   return -ENOMEM;
>   }
> 

Re: [PATCHv3 04/15] ov7670: get xclk

[Hans Verkuil]

On 10/03/17 10:11, Sakari Ailus wrote:
> Hi Hans,
> 
> On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
>> On 09/03/17 21:38, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
 From: Hans Verkuil 

 Get the clock for this sensor.

 Signed-off-by: Hans Verkuil 
 ---
  drivers/media/i2c/ov7670.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
 index 50e4466a2b37..da0843617a49 100644
 --- a/drivers/media/i2c/ov7670.c
 +++ b/drivers/media/i2c/ov7670.c
 @@ -10,6 +10,7 @@
   * This file may be distributed under the terms of the GNU General
   * Public License, version 2.
   */
 +#include 
  #include 
  #include 
  #include 
 @@ -227,6 +228,7 @@ struct ov7670_info {
struct v4l2_ctrl *hue;
};
struct ov7670_format_struct *fmt;  /* Current format */
 +  struct clk *clk;
int min_width;  /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
 @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
info->pclk_hb_disable = true;
}
  
 +  info->clk = devm_clk_get(>dev, "xclk");
 +  if (IS_ERR(info->clk))
 +  return -EPROBE_DEFER;
 +  clk_prepare_enable(info->clk);
 +
 +  info->clock_speed = clk_get_rate(info->clk) / 100;
 +  if (info->clock_speed < 10 || info->clock_speed > 48)
 +  return -EINVAL;
>>>
>>> clk_disable_unprepare() before return?
>>
>> It is my understanding that devm_clk_get will call that for you.
>>
>> Correct me if I am wrong.
> 
> devm_clk_get() obtained clock is released using devm_clk_release() which is
> just calling clk_put(). Which caller prepared or enabled the clock is not
> tracked. It's the responsibility of the caller.
> 

You're right. Then I need to check for this.

Hmm, I'm pretty sure I looked at what other drivers do. I think I'm not the
only one who forgets to call clk_disable_unprepare.

Regards,

Hans

Re: [PATCHv3 01/15] ov7670: document device tree bindings

[Hans Verkuil]

On 09/03/17 19:01, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:02PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Add binding documentation and add that file to the MAINTAINERS entry.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  .../devicetree/bindings/media/i2c/ov7670.txt   | 44 
>> ++
>>  MAINTAINERS|  1 +
>>  2 files changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
>> b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> new file mode 100644
>> index ..6d9c90dff7a7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> @@ -0,0 +1,44 @@
>> +* Omnivision OV7670 CMOS sensor
>> +
>> +The Omnivision OV7670 sensor supports multiple resolutions output, such as
>> +CIF, SVGA, UXGA. It also can support the YUV422/420, RGB565/555 or raw RGB
>> +output formats.
>> +
>> +Required Properties:
>> +- compatible: should be "ovti,ov7670"
>> +- clocks: reference to the xclk input clock.
>> +- clock-names: should be "xclk".
>> +
>> +Optional Properties:
>> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>> +  Active is low.
>> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
>> +  Active is high.
>> +
>> +The device node must contain one 'port' child node for its digital output
>> +video port, in accordance with the video interface bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example:
>> +
>> +i2c1: i2c@f0018000 {
>> +ov7670: camera@21 {
>> +compatible = "ovti,ov7670";
>> +reg = <0x21>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_pck0_as_isi_mck 
>> _sensor_power _sensor_reset>;
>> +reset-gpios = < 11 GPIO_ACTIVE_LOW>;
>> +powerdown-gpios = < 13 GPIO_ACTIVE_HIGH>;
>> +clocks = <>;
>> +clock-names = "xclk";
>> +assigned-clocks = <>;
>> +assigned-clock-rates = <2500>;
>> +
>> +port {
>> +ov7670_0: endpoint {
>> +remote-endpoint = <_0>;
>> +bus-width = <8>;
> 
> Didn't I previously request to specify which of the standardised properties
> are relevant for the device (and which ones are required and which are
> optional)? If I didn't, I'm doing that now. :-)
> 
> E.g. the omap3isp driver documentation looks like this:
> 
> Documentation/devicetree/bindings/media/ti,omap3isp.txt

I did document this in the atmel-isi bindings.

The real issue here is that bus-width is simply ignored by the ov7670 driver,
so it likely can be removed from the example in this document.

I clearly state the required and optional properties in this document, it is
just the example that is out-dated.

Regards,

Hans

> 
>> +};
>> +};
>> +};
>> +};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 83a42ef1d1a7..93500928ca4f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9273,6 +9273,7 @@ L: linux-media@vger.kernel.org
>>  T:  git git://linuxtv.org/media_tree.git
>>  S:  Maintained
>>  F:  drivers/media/i2c/ov7670.c
>> +F:  Documentation/devicetree/bindings/media/i2c/ov7670.txt
>>  
>>  ONENAND FLASH DRIVER
>>  M:  Kyungmin Park 
> 

Re: [PATCHv3 04/15] ov7670: get xclk

[Sakari Ailus]

Hi Hans,

On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
> On 09/03/17 21:38, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Get the clock for this sensor.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> ---
> >>  drivers/media/i2c/ov7670.c | 12 
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> >> index 50e4466a2b37..da0843617a49 100644
> >> --- a/drivers/media/i2c/ov7670.c
> >> +++ b/drivers/media/i2c/ov7670.c
> >> @@ -10,6 +10,7 @@
> >>   * This file may be distributed under the terms of the GNU General
> >>   * Public License, version 2.
> >>   */
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -227,6 +228,7 @@ struct ov7670_info {
> >>struct v4l2_ctrl *hue;
> >>};
> >>struct ov7670_format_struct *fmt;  /* Current format */
> >> +  struct clk *clk;
> >>int min_width;  /* Filter out smaller sizes */
> >>int min_height; /* Filter out smaller sizes */
> >>int clock_speed;/* External clock speed (MHz) */
> >> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
> >>info->pclk_hb_disable = true;
> >>}
> >>  
> >> +  info->clk = devm_clk_get(>dev, "xclk");
> >> +  if (IS_ERR(info->clk))
> >> +  return -EPROBE_DEFER;
> >> +  clk_prepare_enable(info->clk);
> >> +
> >> +  info->clock_speed = clk_get_rate(info->clk) / 100;
> >> +  if (info->clock_speed < 10 || info->clock_speed > 48)
> >> +  return -EINVAL;
> > 
> > clk_disable_unprepare() before return?
> 
> It is my understanding that devm_clk_get will call that for you.
> 
> Correct me if I am wrong.

devm_clk_get() obtained clock is released using devm_clk_release() which is
just calling clk_put(). Which caller prepared or enabled the clock is not
tracked. It's the responsibility of the caller.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 04/15] ov7670: get xclk

[Hans Verkuil]

On 09/03/17 21:38, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Get the clock for this sensor.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/ov7670.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index 50e4466a2b37..da0843617a49 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -10,6 +10,7 @@
>>   * This file may be distributed under the terms of the GNU General
>>   * Public License, version 2.
>>   */
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -227,6 +228,7 @@ struct ov7670_info {
>>  struct v4l2_ctrl *hue;
>>  };
>>  struct ov7670_format_struct *fmt;  /* Current format */
>> +struct clk *clk;
>>  int min_width;  /* Filter out smaller sizes */
>>  int min_height; /* Filter out smaller sizes */
>>  int clock_speed;/* External clock speed (MHz) */
>> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>>  info->pclk_hb_disable = true;
>>  }
>>  
>> +info->clk = devm_clk_get(>dev, "xclk");
>> +if (IS_ERR(info->clk))
>> +return -EPROBE_DEFER;
>> +clk_prepare_enable(info->clk);
>> +
>> +info->clock_speed = clk_get_rate(info->clk) / 100;
>> +if (info->clock_speed < 10 || info->clock_speed > 48)
>> +return -EINVAL;
> 
> clk_disable_unprepare() before return?

It is my understanding that devm_clk_get will call that for you.

Correct me if I am wrong.

Regards,

Hans

> 
>> +
>>  /* Make sure it's an ov7670 */
>>  ret = ov7670_detect(sd);
>>  if (ret) {
>> @@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
>>  
>>  v4l2_device_unregister_subdev(sd);
>>  v4l2_ctrl_handler_free(>hdl);
>> +clk_disable_unprepare(info->clk);
>>  return 0;
>>  }
>>  
> 

Re: [RFC PATCH 08/12] cma: Store a name in the cma structure

[Sumit Semwal]

Hi Laura,

Thanks for the patch.

On 3 March 2017 at 03:14, Laura Abbott  wrote:
>
> Frameworks that may want to enumerate CMA heaps (e.g. Ion) will find it
> useful to have an explicit name attached to each region. Store the name
> in each CMA structure.
>
> Signed-off-by: Laura Abbott 
> ---
>  drivers/base/dma-contiguous.c |  5 +++--
>  include/linux/cma.h   |  4 +++-
>  mm/cma.c  | 11 +--
>  mm/cma.h  |  1 +
>  mm/cma_debug.c|  2 +-
>  5 files changed, 17 insertions(+), 6 deletions(-)
>

> +const char *cma_get_name(const struct cma *cma)
> +{
> +   return cma->name ? cma->name : "(undefined)";
> +}
> +
Would it make sense to perhaps have the idx stored as the name,
instead of 'undefined'? That would make sure that the various cma
names are still unique.

>  static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
>  int align_order)
>  {
> @@ -168,6 +173,7 @@ core_initcall(cma_init_reserved_areas);
>   */
>  int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
>  unsigned int order_per_bit,
> +const char *name,
>  struct cma **res_cma)
>  {

Best regards,
Sumit.

Re: [PATCH 1/3] [media] platform: compile VIDEO_CODA with COMPILE_TEST

[Hans Verkuil]

On 03/09/2017 09:08 PM, Mauro Carvalho Chehab wrote:
> Currently, IMX_VDOA and VIDEO_CODA only builds on ARCH_MXC.
> 
> That prevented me to build-test the driver, causing a bad patch
> to be applied, and to see other warnings on this driver.

Huh, must be the time of year. I just made a similar patch this week and
it is in one of my git pull requests I posted with lots of other coda
fixes.

Regards,

Hans

> 
> Reported-by: Russell King 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index c9106e105bab..6d0bba271a8d 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -151,7 +151,7 @@ if V4L_MEM2MEM_DRIVERS
>  
>  config VIDEO_CODA
>   tristate "Chips Coda multi-standard codec IP"
> - depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_MXC
> + depends on VIDEO_DEV && VIDEO_V4L2 && (ARCH_MXC || COMPILE_TEST)
>   depends on HAS_DMA
>   select SRAM
>   select VIDEOBUF2_DMA_CONTIG
>