lening bieden

[SAFETY NET CREDIT]

Goede dag,

 Dit is het beveiligingsnetwerk van de kredietverstrekking.

  SAFETY NET CREDIT biedt flexibele en betaalbare leningen voor elk doel om u 
te helpen uw doelen te bereiken. We lenen tegen een lage rente van 3%. Hier 
zijn enkele belangrijke kenmerken van de persoonlijke lening aangeboden door 
SAFETY NET CREDIT. Hier zijn de Lening Factoren die we samenwerken met de 
toonaangevende Britse makelaars die toegang hebben tot topleners en kunnen de 
beste financiële oplossing tegen een betaalbare prijs vinden. Als u 
geïnteresseerd bent, neem dan contact met ons op via deze e-mail: 
safetynetcredi...@gmail.com

Na de reactie ontvangt u een aanvraag voor leningsvulling. Geen sociale 
zekerheid en geen credit check, 100% gegarandeerd.

Het zal onze eer zijn als u ons toelaat om tot uw dienst te zijn.


INFORMATIE NODIG

Jullie namen
Adres: ...
Telefoon: ...
Bedrag nodig: 
Looptijd: ...
Beroep: ...
Maandelijks Inkomen Niveau: 
Geslacht: ...
Geboortedatum: 
Staat: ..
Land: ..
Doel: .

Het voldoen aan uw financiële behoeften is onze trots.


Dr.John Mahama.

Unknown symbol put_vaddr_frames when using media_build

[Matthias Schwarzott]

Hi!

Whenever I compile the media drivers using media_build against a recent
kernel, I get this message when loading them:

[5.848537] media: Linux media interface: v0.10
[5.881440] Linux video capture interface: v2.00
[5.881441] WARNING: You are using an experimental version of the
media stack.
...
[6.166390] videobuf2_memops: Unknown symbol put_vaddr_frames (err 0)
[6.166394] videobuf2_memops: Unknown symbol get_vaddr_frames (err 0)
[6.166396] videobuf2_memops: Unknown symbol frame_vector_destroy (err 0)
[6.166398] videobuf2_memops: Unknown symbol frame_vector_create (err 0)

That means I am not able to load any drivers being based on
videobuf2_memops without manual actions.

I used kernel 4.11.0, but it does not matter which kernel version
exactly is used.

My solution for that has been to modify mm/Kconfig of my kernel like
this and then enable FRAME_VECTOR in .config

diff --git a/mm/Kconfig b/mm/Kconfig
index 9b8fccb969dc..cfa6a80d1a0a 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -701,7 +701,7 @@ config ZONE_DEVICE
  If FS_DAX is enabled, then say Y.

 config FRAME_VECTOR
-   bool
+   tristate "frame vector"

 config ARCH_USES_HIGH_VMA_FLAGS
bool

But I do not like that solution.
I would prefer one of these solutions:

1. Have media_build apply its fallback the same way as for older kernels
that do not even have the the FRAME_VECTOR support.

2. Get the above patch merged (plus description etc.).

What do you think?

Regards
Matthias

cron job: media_tree daily build: WARNINGS

[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:   Tue May  9 05:00:19 CEST 2017
media-tree git hash:3622d3e77ecef090b5111e3c5423313f11711dfa
media_build git hash:   ab988a3d089232ce9e1aec2f259e947c06983dbc
v4l-utils git hash: fba2bdde45aaf6b56608c3cf8b906d7e5ee0e21f
gcc version:i686-linux-gcc (GCC) 7.1.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: WARNINGS
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: WARNINGS
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: WARNINGS
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: WARNINGS
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: WARNINGS
linux-3.12.67-i686: WARNINGS
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: WARNINGS
linux-4.9.26-i686: WARNINGS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
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: WARNINGS
linux-3.12.67-x86_64: WARNINGS
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.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH 0/4] DMA-buf: Fine-tuning for four function implementations

[Sumit Semwal]

Hello Markus,

On 8 May 2017 at 14:40, SF Markus Elfring  wrote:
> From: Markus Elfring 
> Date: Mon, 8 May 2017 11:05:05 +0200
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (4):
>   Combine two function calls into one in dma_buf_debug_show()
>   Improve a size determination in dma_buf_attach()
>   Adjust a null pointer check in dma_buf_attach()
>   Use seq_putc() in two functions

All queued up in drm-misc-next now. Thanks!
>
>  drivers/dma-buf/dma-buf.c| 8 +++-
>  drivers/dma-buf/sync_debug.c | 6 +++---
>  2 files changed, 6 insertions(+), 8 deletions(-)
>
> --
> 2.12.2
>

Best regards,
Sumit.

Re: [PATCH 40/40] media: imx: set and propagate empty field, colorimetry params

[Steve Longerbeam]

On 05/08/2017 02:41 AM, Philipp Zabel wrote:

Hi Steve,

On Wed, 2017-04-12 at 17:45 -0700, Steve Longerbeam wrote:

This patch adds a call to imx_media_fill_empty_mbus_fields() in the
*_try_fmt() functions at the sink pads, to set empty field order and
colorimetry parameters.

If the field order is set to ANY, choose the currently set field order
at the sink pad. If the colorspace is set to DEFAULT, choose the
current colorspace at the sink pad.  If any of xfer_func, ycbcr_enc
or quantization are set to DEFAULT, either choose the current sink pad
setting, or the default setting for the new colorspace, if non-DEFAULT
colorspace was given.

Colorimetry is also propagated from sink to source pads anywhere
this has not already been done. The exception is ic-prpencvf at the
source pad, since the Image Converter outputs fixed quantization and
Y`CbCr encoding.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-ic-prp.c  |  5 ++-
 drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +++---
 drivers/staging/media/imx/imx-media-csi.c   | 12 +--
 drivers/staging/media/imx/imx-media-utils.c | 53 +
 drivers/staging/media/imx/imx-media-vdic.c  |  7 ++--
 drivers/staging/media/imx/imx-media.h   |  3 +-
 6 files changed, 95 insertions(+), 10 deletions(-)


[...]

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index 7b2f92d..b07d0ae 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -464,6 +464,59 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt 
*mbus,
 }
 EXPORT_SYMBOL_GPL(imx_media_init_mbus_fmt);

+/*
+ * Check whether the field or colorimetry params in tryfmt are
+ * uninitialized, and if so fill them with the values from fmt.
+ * The exception is when tryfmt->colorspace has been initialized,
+ * if so all the further default colorimetry params can be derived
+ * from tryfmt->colorspace.
+ */
+void imx_media_fill_empty_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
+ struct v4l2_mbus_framefmt *fmt)
+{
+   /* fill field if necessary */
+   if (tryfmt->field == V4L2_FIELD_ANY)
+   tryfmt->field = fmt->field;
+
+   /* fill colorimetry if necessary */
+   if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
+   tryfmt->colorspace = fmt->colorspace;
+   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
+   tryfmt->xfer_func = fmt->xfer_func;
+   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
+   tryfmt->quantization = fmt->quantization;


According to Hans' latest comments, this could be changed to:

--8<--
From cca3cda9effcaca0891eb8044a79137023fed1c2 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 8 May 2017 11:38:05 +0200
Subject: [PATCH] fixup! media: imx: set and propagate default field,
 colorimetry

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-utils.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index a8766489e8a18..ec2abd618cc44 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -497,12 +497,9 @@ void imx_media_fill_default_mbus_fields(struct 
v4l2_mbus_framefmt *tryfmt,
/* fill colorimetry if necessary */
if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
tryfmt->colorspace = fmt->colorspace;
-   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
-   tryfmt->xfer_func = fmt->xfer_func;
-   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
-   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
-   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
-   tryfmt->quantization = fmt->quantization;
+   tryfmt->xfer_func = fmt->xfer_func;
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   tryfmt->quantization = fmt->quantization;
} else {
if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT) {
tryfmt->xfer_func =



Hi Philipp, makes sense to me, I'll make the change in next revision.

Steve

[patch, libv4l]: fix integer overflow

[Pavel Machek]

Hi!

This bit me while trying to use absolute exposure time on Nokia N900:

Can someone apply it to libv4l2 tree? Could I get some feedback on the
other patches? Is this the way to submit patches to libv4l2?

Thanks,
Pavel

commit 0484e39ec05fdc644191e7c334a7ebfff9cb2ec5
Author: Pavel 
Date:   Mon May 8 21:52:02 2017 +0200

Fix integer overflow with EXPOSURE_ABSOLUTE.

diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index e795aee..189fc06 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -1776,7 +1776,7 @@ int v4l2_set_control(int fd, int cid, int value)
if (qctrl.type == V4L2_CTRL_TYPE_BOOLEAN)
ctrl.value = value ? 1 : 0;
else
-   ctrl.value = (value * (qctrl.maximum - qctrl.minimum) + 
32767) / 65535 +
+   ctrl.value = ((long long) value * (qctrl.maximum - 
qctrl.minimum) + 32767) / 65535 +
qctrl.minimum;
 
result = v4lconvert_vidioc_s_ctrl(devices[index].convert, 
);
@@ -1812,7 +1812,7 @@ int v4l2_get_control(int fd, int cid)
if (v4l2_propagate_ioctl(index, VIDIOC_G_CTRL, ))
return -1;
 
-   return ((ctrl.value - qctrl.minimum) * 65535 +
+   return (((long long) ctrl.value - qctrl.minimum) * 65535 +
(qctrl.maximum - qctrl.minimum) / 2) /
(qctrl.maximum - qctrl.minimum);
 }


-- 
(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] ATOMISP: Tidies up code warnings and errors in file

[Mark Railton]

Cleared up some errors and warnings in
drivers/staging/media/atomisp/i2c/ap1302.c

Signed-off-by: Mark Railton 
---
 drivers/staging/media/atomisp/i2c/ap1302.c | 83 ++
 1 file changed, 50 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ap1302.c 
b/drivers/staging/media/atomisp/i2c/ap1302.c
index bacffbe..8c33a35 100644
--- a/drivers/staging/media/atomisp/i2c/ap1302.c
+++ b/drivers/staging/media/atomisp/i2c/ap1302.c
@@ -11,11 +11,6 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
  */
 
 #include "../include/linux/atomisp.h"
@@ -162,9 +157,11 @@ static struct ap1302_context_info context_info[] = {
{CNTX_HINF_CTRL, AP1302_REG16, "hinf_ctrl"},
 };
 
-/* This array stores the description list for metadata.
-   The metadata contains exposure settings and face
-   detection results. */
+/*
+ *  This array stores the description list for metadata.
+ *  The metadata contains exposure settings and face
+ *  detection results.
+ */
 static u16 ap1302_ss_list[] = {
0xb01c, /* From 0x0186 with size 0x1C are exposure settings. */
0x0186,
@@ -213,6 +210,7 @@ static int ap1302_i2c_write_reg(struct v4l2_subdev *sd,
struct ap1302_device *dev = to_ap1302_device(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
int ret;
+
if (len == AP1302_REG16)
ret = regmap_write(dev->regmap16, reg, val);
else if (len == AP1302_REG32)
@@ -236,11 +234,13 @@ static u16
 ap1302_calculate_context_reg_addr(enum ap1302_contexts context, u16 offset)
 {
u16 reg_addr;
-   /* The register offset is defined according to preview/video registers.
-  Preview and video context have the same register definition.
-  But snapshot context does not have register S1_SENSOR_MODE.
-  When setting snapshot registers, if the offset exceeds
-  S1_SENSOR_MODE, the actual offset needs to minus 2. */
+   /*
+*  The register offset is defined according to preview/video registers.
+*  Preview and video context have the same register definition.
+*  But snapshot context does not have register S1_SENSOR_MODE.
+*  When setting snapshot registers, if the offset exceeds
+*  S1_SENSOR_MODE, the actual offset needs to minus 2.
+*/
if (context == CONTEXT_SNAPSHOT) {
if (offset == CNTX_S1_SENSOR_MODE)
return 0;
@@ -261,6 +261,7 @@ static int ap1302_read_context_reg(struct v4l2_subdev *sd,
 {
struct ap1302_device *dev = to_ap1302_device(sd);
u16 reg_addr = ap1302_calculate_context_reg_addr(context, offset);
+
if (reg_addr == 0)
return -EINVAL;
return ap1302_i2c_read_reg(sd, reg_addr, len,
@@ -272,6 +273,7 @@ static int ap1302_write_context_reg(struct v4l2_subdev *sd,
 {
struct ap1302_device *dev = to_ap1302_device(sd);
u16 reg_addr = ap1302_calculate_context_reg_addr(context, offset);
+
if (reg_addr == 0)
return -EINVAL;
return ap1302_i2c_write_reg(sd, reg_addr, len,
@@ -284,7 +286,9 @@ static int ap1302_dump_context_reg(struct v4l2_subdev *sd,
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ap1302_device *dev = to_ap1302_device(sd);
int i;
+
dev_dbg(>dev, "Dump registers for context[%d]:\n", context);
+
for (i = 0; i < ARRAY_SIZE(context_info); i++) {
struct ap1302_context_info *info = _info[i];
u8 *var = (u8 *)>cntx_config[context] + info->offset;
@@ -308,6 +312,7 @@ static int ap1302_request_firmware(struct v4l2_subdev *sd)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ap1302_device *dev = to_ap1302_device(sd);
int ret;
+
ret = request_firmware(>fw, "ap1302_fw.bin", >dev);
if (ret)
dev_err(>dev,
@@ -315,12 +320,14 @@ static int ap1302_request_firmware(struct v4l2_subdev *sd)
return ret;
 }
 
-/* When loading firmware, host writes firmware data from address 0x8000.
-   When the address reaches 0x9FFF, the next address should return to 0x8000.
-   This function handles this address window and load firmware data to AP1302.
-   win_pos indicates the offset within this window. Firmware loading procedure
-   may call this function several times. win_pos records the current position
-   that has been written to.*/
+/*
+ *  When loading firmware, host writes firmware data from address 0x8000.
+ *  When the address reaches 0x9FFF, the next address should return to 0x8000.
+ *  This function handles this address window and load 

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Hans,

On Mon, May 08, 2017 at 03:08:26PM +0200, Hans Verkuil wrote:
> On 05/08/2017 02:56 PM, Sakari Ailus wrote:
...
> >> The USERPTR mode is more dubious. Has this been tested? Can the DMA handle 
> >> partially
> >> filled pages? (I.e. there must be no requirements such as that the DMA has 
> >> to start
> >> at a page boundary, since that's not the case with USERPTR).
> > 
> > I rememeber this has been discussed before. :-)
> > 
> > Most hardware has some limitations on the granularity of the buffer start
> > address, and the drivers still support USERPTR memory. In practice the C
> > library allocated memory is always page aligned if the size is large enough,
> > which is in practice the case for video buffers.
> 
> That was not true the last time I checked. I can't remember what the exact
> alignment was, although I do remember that it was different for 32 and 64 bit.

Hmm. This just shows how little I really know about user space. :-I

I just tested this to see how it really works out, and there doesn't seem to
be much alignment at all. I believe my earlier recollection was likely
related to a non-GNU C library (huh!).

> 
> I am also pretty sure it was less than 64 bytes. It's been 2 years ago since
> I last looked at this, though.

As in here:

00:26:02 lanttu sailus [~]cat /tmp/foo.c
#include 
#include 
#include 

void main() {
unsigned int i;

for (i=0; i < 16; i++)
printf("%x\n",malloc(1024*768*i));
}
00:26:03 lanttu sailus [~]gcc -o /tmp/foo /tmp/foo.c
00:26:06 lanttu sailus [~]/tmp/foo
8604008
f7525008
f73a4008
f7163008
f6e62008
f6aa1008
f6620008
f60df008
f5ade008
f541d008
f4c9c008
f445b008
f3b5a008
f3199008
f2718008
f1bd7008

So posix_memalign() (or memalign()) is needed to allocate page aligned
memory. At least on GNU libc.

-- 
Regards,

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

Re: [PATCH v2] dw9714: Initial driver for dw9714 VCM

[Sakari Ailus]

On Mon, May 08, 2017 at 07:36:48AM -0700, Rajmohan Mani wrote:
> + dev_dbg(dev, "%s ret = %d\n", __func__, ret);

Please remove such debug prints.

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

Re: [PATCH] dw9714: Initial driver for dw9714 VCM

[Sakari Ailus]

Hi Rajmohan,

A few comments below...

On Sun, May 07, 2017 at 04:33:24AM -0700, rajmohan.m...@intel.com wrote:
> From: Rajmohan Mani 
> 
> DW9714 is a 10 bit DAC, designed for linear
> control of voice coil motor.
> 
> This driver creates a V4L2 subdevice and
> provides control to set the desired focus.
> 
> Signed-off-by: Rajmohan Mani 
> ---
>  drivers/media/i2c/Kconfig  |   9 ++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/dw9714.c | 333 
> +
>  3 files changed, 343 insertions(+)
>  create mode 100644 drivers/media/i2c/dw9714.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index fd181c9..4f0a7ad 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -300,6 +300,15 @@ config VIDEO_AD5820
> This is a driver for the AD5820 camera lens voice coil.
> It is used for example in Nokia N900 (RX-51).
>  
> +config VIDEO_DW9714
> + tristate "DW9714 lens voice coil support"
> + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER

You should also depend on VIDEO_V4L2_SUBDEV_API .

> + ---help---
> +   This is a driver for the DW9714 camera lens voice coil.
> +   DW9714 is a 10 bit DAC with 120mA output current sink
> +   capability. This is designed for linear control of
> +   voice coil motors, controlled via I2C serial interface.
> +
>  config VIDEO_SAA7110
>   tristate "Philips SAA7110 video decoder"
>   depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 62323ec..987bd1f 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
>  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
>  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
> +obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
>  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
>  obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
> diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
> new file mode 100644
> index 000..276d3f2
> --- /dev/null
> +++ b/drivers/media/i2c/dw9714.c
> @@ -0,0 +1,333 @@
> +/*
> + * Copyright (c) 2015--2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DW9714_NAME  "dw9714"
> +#define DW9714_MAX_FOCUS_POS 1023
> +#define DW9714_CTRL_STEPS16  /* Keep this value power of 2 */
> +#define DW9714_CTRL_DELAY_US 1000
> +/*
> + * S[3:2] = 0x00, codes per step for "Linear Slope Control"
> + * S[1:0] = 0x00, step period
> + */
> +#define DW9714_DEFAULT_S 0x0
> +#define DW9714_VAL(data, s) (u16)((data) << 4 | (s))
> +
> +/* dw9714 device structure */
> +struct dw9714_device {
> + struct i2c_client *client;
> + struct v4l2_ctrl_handler ctrls_vcm;
> + struct v4l2_subdev sd;
> + u16 current_val;
> +};
> +
> +#define to_dw9714_vcm(_ctrl) \
> + container_of(_ctrl->handler, struct dw9714_device, ctrls_vcm)
> +
> +static int dw9714_i2c_write(struct i2c_client *client, u16 data)
> +{
> + const int num_msg = 1;
> + int ret;
> + u16 val = cpu_to_be16(data);
> + struct i2c_msg msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .len = sizeof(val),
> + .buf = (u8 *) ,
> + };
> +
> + ret = i2c_transfer(client->adapter, , num_msg);
> +
> + /*One retry */
> + if (ret != num_msg)
> + ret = i2c_transfer(client->adapter, , num_msg);
> +
> + if (ret != num_msg) {
> + dev_err(>dev, "I2C write fail fail\n");
> + return -EIO;
> + } else {
> + return 0;
> + }
> +}
> +
> +static int dw9714_t_focus_vcm(struct dw9714_device *dw9714_dev, u16 val)
> +{
> + struct i2c_client *client = dw9714_dev->client;
> + int ret = -EINVAL;
> +
> + dev_dbg(>dev, "Setting new value VCM: %d\n", val);
> + dw9714_dev->current_val = val;
> +
> + ret = dw9714_i2c_write(client, DW9714_VAL(val, DW9714_DEFAULT_S));
> + return ret;
> +}
> +
> +static int dw9714_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9714_device *dev_vcm = to_dw9714_vcm(ctrl);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> + return dw9714_t_focus_vcm(dev_vcm, ctrl->val);
> + else
> + return -EINVAL;
> +}
> +
> +static const 

Re: [PATCH 3/4] v4l: vsp1: Calculate partition sizes at stream start.

[Kieran Bingham]

Hi Laurent,

Thanks for the review,

V2 to be posted after testing.

On 13/02/17 21:21, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Friday 04 Nov 2016 18:19:29 Kieran Bingham wrote:
>> Previously the active window and partition sizes for each partition is
> 
> s/is/were/

Fixed.

> 
>> calculated for each partition every frame. This data is constant and
>> only needs to be calculated once at the start of the stream.
>>
>> Extend the vsp1_pipe object to store the maximum number of partitions
>> possible and pre-calculate the partition sizes into this table.
>>
>> Signed-off-by: Kieran Bingham 
>> ---
>>  drivers/media/platform/vsp1/vsp1_pipe.h  | 6 ++
>>  drivers/media/platform/vsp1/vsp1_video.c | 8 ++--
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
>> b/drivers/media/platform/vsp1/vsp1_pipe.h index f181949824c9..3af96c4ea244
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
>> @@ -20,6 +20,9 @@
>>
>>  #include 
>>
>> +/* Max Video Width / Min Partition Size = 8190/128 */
>> +#define VSP1_PIPE_MAX_PARTITIONS 64
>> +

^ This define is removed with dynamic allocations.

>>  struct vsp1_dl_list;
>>  struct vsp1_rwpf;
>>
>> @@ -81,7 +84,9 @@ enum vsp1_pipeline_state {
>>   * @dl: display list associated with the pipeline
>>   * @div_size: The maximum allowed partition size for the pipeline
>>   * @partitions: The number of partitions used to process one frame
>> + * @partition: The current partition for configuration to process
>>   * @current_partition: The partition number currently being configured
>> + * @part_table: The pre-calculated partitions used by the pipeline
>>   */
>>  struct vsp1_pipeline {
>>  struct media_pipeline pipe;
>> @@ -116,6 +121,7 @@ struct vsp1_pipeline {
>>  unsigned int partitions;
>>  struct v4l2_rect partition;
>>  unsigned int current_partition;
>> +struct v4l2_rect part_table[VSP1_PIPE_MAX_PARTITIONS];
> 
> That's an extra 1kB or kmalloc'ed data. I'd prefer allocating it dynamically 
> as needed.

Ok, allocating with kcalloc in vsp1_video_pipeline_setup_partitions() (The
earliest opportunity to know how many partitions are needed)

Free'd in vsp1_video_stop_streaming()

>>  };
>>
>>  void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
>> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
>> b/drivers/media/platform/vsp1/vsp1_video.c index 6d43c02bbc56..c4a8c30df108
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_video.c
>> +++ b/drivers/media/platform/vsp1/vsp1_video.c
>> @@ -255,6 +255,7 @@ static void vsp1_video_pipeline_setup_partitions(struct
>> vsp1_pipeline *pipe) const struct v4l2_mbus_framefmt *format;
>>  struct vsp1_entity *entity;
>>  unsigned int div_size;
>> +int i;
> 
> i can never be negative, you can make it an unsigned int.

Fixed.

> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 

Thanks, I'll tentatively add this tag, but please check you are happy with the
dynamic allocation on the repost v2 :-) !

> 
>>  /*
>>   * Partitions are computed on the size before rotation, use the format
>> @@ -269,6 +270,7 @@ static void vsp1_video_pipeline_setup_partitions(struct
>> vsp1_pipeline *pipe) if (vsp1->info->gen == 2) {
>>  pipe->div_size = div_size;
>>  pipe->partitions = 1;
>> +pipe->part_table[0] = vsp1_video_partition(pipe, div_size, 0);
>>  return;
>>  }
>>
>> @@ -284,6 +286,9 @@ static void vsp1_video_pipeline_setup_partitions(struct
>> vsp1_pipeline *pipe)
>>
>>  pipe->div_size = div_size;
>>  pipe->partitions = DIV_ROUND_UP(format->width, div_size);
>> +
>> +for (i = 0; i < pipe->partitions; i++)
>> +pipe->part_table[i] = vsp1_video_partition(pipe, div_size, i);
>>  }
>>
>>  /* 
>> @@ -355,8 +360,7 @@ static void vsp1_video_pipeline_run_partition(struct
>> vsp1_pipeline *pipe, {
>>  struct vsp1_entity *entity;
>>
>> -pipe->partition = vsp1_video_partition(pipe, pipe->div_size,
>> -   pipe->current_partition);
>> +pipe->partition = pipe->part_table[pipe->current_partition];
>>
>>  list_for_each_entry(entity, >entities, list_pipe) {
>>  if (entity->ops->configure)
> 

Re: [PATCH v2 0/2] rcar-du, vsp1: rcar-gen3: Add support for colorkey alpha blending

[Daniel Vetter]

On Mon, May 08, 2017 at 09:33:37AM -0700, Eric Anholt wrote:
> Alexandru Gheorghe  writes:
> 
> > Currently, rcar-du supports colorkeying  only for rcar-gen2 and it uses 
> > some hw capability of the display unit(DU) which is not available on gen3.
> > In order to implement colorkeying for gen3 we need to use the colorkey
> > capability of the VSPD, hence the need to change both drivers rcar-du and
> > vsp1.
> >
> > This patchset had been developed and tested on top of v4.9/rcar-3.5.1 from
> > git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git
> 
> A few questions:
> 
> Are other drivers interested in supporting this property?  VC4 has the
> 24-bit RGB colorkey, but I don't see YCBCR support.  Should it be
> documented in a generic location?
> 
> Does your colorkey end up forcing alpha to 1 for the plane when it's not
> matched?

I think generic color-key for plane compositioning would be nice, but I'm
not sure that's possible due to differences in how the key works.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] staging: media: fix one code style problem

[Remco Verhoef]

this patch will fix one code style problem (ctx:WxE), space
prohibited before that

Signed-off-by: Remco Verhoef 
---
 .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a..b0f9188 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -51,7 +51,7 @@ struct gmin_subdev {
 
 static struct gmin_subdev gmin_subdevs[MAX_SUBDEVS];
 
-static enum { PMIC_UNSET = 0, PMIC_REGULATOR, PMIC_AXP, PMIC_TI ,
+static enum { PMIC_UNSET = 0, PMIC_REGULATOR, PMIC_AXP, PMIC_TI,
PMIC_CRYSTALCOVE } pmic_id;
 
 /* The atomisp uses type==0 for the end-of-list marker, so leave space. */
-- 
1.9.1

Re: [PATCH 01/11] [media] dvb-core/dvb_ca_en50221.c: Rename STATUSREG_??

[Jasmin J.]

Hello Mauro!

>> Rename STATUSREG_?? -> STATREG_?? to reduce the line length.
> That sounds a bad idea. We use "stat" on the DVB subsystem as an
> alias for statistics.
At the beginning of the style fixes, I thought it is a good idea to reduce
as much as possible to get more characters, but at the end this patch
doesn't save so much, so we can omit it.

What is then the right procedure now?
When I omit it in the first place, I can redo the whole work again and
this were a lot of hours. Would it be acceptable to make a patch no. 12 at
the end of the sequence, which renames it back?

BR,
   Jasmin

Re: [RFC v2 3/3] dt: bindings: Add a binding for referencing EEPROM from camera sensors

[Rob Herring]

On Fri, May 05, 2017 at 11:48:30AM +0300, Sakari Ailus wrote:
> Many camera sensor devices contain EEPROM chips that describe the
> properties of a given unit --- the data is specific to a given unit can
> thus is not stored e.g. in user space or the driver.
> 
> Some sensors embed the EEPROM chip and it can be accessed through the
> sensor's I2C interface. This property is to be used for devices where the
> EEPROM chip is accessed through a different I2C address than the sensor.

Different I2C address or bus? We already have i2c bindings for sub 
devices downstream of another I2C device. Either the upstream device 
passes thru the I2C transactions or itself is an I2C controller with a 
separate downstream bus. For those cases the EEPROM should be a child 
node. A phandle only makes sense if you have the sensor and eeprom 
connected to 2 entirely separate host buses.

Rob

Re: [RFC v2 2/3] dt: bindings: Add lens-focus binding for image sensors

[Rob Herring]

On Fri, May 05, 2017 at 11:48:29AM +0300, Sakari Ailus wrote:
> The lens-focus property contains a phandle to the lens voice coil driver
> that is associated to the sensor; typically both are contained in the same
> camera module.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Pavel Machek 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring 

Re: [RFC v2 1/3] dt: bindings: Add a binding for flash devices associated to a sensor

[Rob Herring]

On Fri, May 05, 2017 at 11:48:28AM +0300, Sakari Ailus wrote:
> Camera flash drivers (and LEDs) are separate from the sensor devices in
> DT. In order to make an association between the two, provide the
> association information to the software.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Pavel Machek 
> Reviewed-by: Sebastian Reichel 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 11 +++
>  1 file changed, 11 insertions(+)

Acked-by: Rob Herring 

> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 9cd2a36..dac764b 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -67,6 +67,17 @@ are required in a relevant parent node:
>   identifier, should be 1.
>   - #size-cells: should be zero.
>  
> +
> +Optional properties
> +---
> +
> +- flash: An array of phandles that refer to the flash light sources
> +  related to an image sensor. These could be e.g. LEDs. In case the LED
> +  driver (current sink or source chip for the LED(s)) drives more than a
> +  single LED, then the phandles here refer to the child nodes of the LED
> +  driver describing individual LEDs.
> +
> +
>  Optional endpoint properties
>  
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] staging: media: atomisp: Fix indentation to tabs

[Gideon Sheril]

Fixing following checkpatch warnnings/errors:
ERROR: code indent should use tabs where possible
WARNING: please, no spaces at the start of a line

Signed-off-by: Gideon Sheril 
---
 Changed from previuos version:
  - Fixed alignment issues
  - Verified no new warning or errors appear after patch

 .../platform/intel-mid/atomisp_gmin_platform.c | 150 ++---
 1 file changed, 75 insertions(+), 75 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a..4eb0c91 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -152,13 +152,13 @@ const struct camera_af_platform_data 
*camera_get_af_platform_data(void)
 EXPORT_SYMBOL_GPL(camera_get_af_platform_data);
 
 int atomisp_register_i2c_module(struct v4l2_subdev *subdev,
-struct camera_sensor_platform_data *plat_data,
-enum intel_v4l2_subdev_type type)
+   struct camera_sensor_platform_data *plat_data,
+   enum intel_v4l2_subdev_type type)
 {
int i;
struct i2c_board_info *bi;
struct gmin_subdev *gs;
-struct i2c_client *client = v4l2_get_subdevdata(subdev);
+   struct i2c_client *client = v4l2_get_subdevdata(subdev);
struct acpi_device *adev;
 
dev_info(>dev, "register atomisp i2c module type %d\n", type);
@@ -270,45 +270,45 @@ static const struct gmin_cfg_var t100_vars[] = {
 };
 
 static const struct gmin_cfg_var mrd7_vars[] = {
-{"INT33F8:00_CamType", "1"},
-{"INT33F8:00_CsiPort", "1"},
-{"INT33F8:00_CsiLanes","2"},
-{"INT33F8:00_CsiFmt","13"},
-{"INT33F8:00_CsiBayer", "0"},
-{"INT33F8:00_CamClk", "0"},
-{"INT33F9:00_CamType", "1"},
-{"INT33F9:00_CsiPort", "0"},
-{"INT33F9:00_CsiLanes","1"},
-{"INT33F9:00_CsiFmt","13"},
-{"INT33F9:00_CsiBayer", "0"},
-{"INT33F9:00_CamClk", "1"},
-{},
+   {"INT33F8:00_CamType", "1"},
+   {"INT33F8:00_CsiPort", "1"},
+   {"INT33F8:00_CsiLanes", "2"},
+   {"INT33F8:00_CsiFmt", "13"},
+   {"INT33F8:00_CsiBayer", "0"},
+   {"INT33F8:00_CamClk", "0"},
+   {"INT33F9:00_CamType", "1"},
+   {"INT33F9:00_CsiPort", "0"},
+   {"INT33F9:00_CsiLanes", "1"},
+   {"INT33F9:00_CsiFmt", "13"},
+   {"INT33F9:00_CsiBayer", "0"},
+   {"INT33F9:00_CamClk", "1"},
+   {},
 };
 
 static const struct gmin_cfg_var ecs7_vars[] = {
-{"INT33BE:00_CsiPort", "1"},
-{"INT33BE:00_CsiLanes","2"},
-{"INT33BE:00_CsiFmt","13"},
-{"INT33BE:00_CsiBayer", "2"},
-{"INT33BE:00_CamClk", "0"},
-{"INT33F0:00_CsiPort", "0"},
-{"INT33F0:00_CsiLanes","1"},
-{"INT33F0:00_CsiFmt","13"},
-{"INT33F0:00_CsiBayer", "0"},
-{"INT33F0:00_CamClk", "1"},
-{"gmin_V2P8GPIO","402"},
-{},
+   {"INT33BE:00_CsiPort", "1"},
+   {"INT33BE:00_CsiLanes", "2"},
+   {"INT33BE:00_CsiFmt", "13"},
+   {"INT33BE:00_CsiBayer", "2"},
+   {"INT33BE:00_CamClk", "0"},
+   {"INT33F0:00_CsiPort", "0"},
+   {"INT33F0:00_CsiLanes", "1"},
+   {"INT33F0:00_CsiFmt", "13"},
+   {"INT33F0:00_CsiBayer", "0"},
+   {"INT33F0:00_CamClk", "1"},
+   {"gmin_V2P8GPIO", "402"},
+   {},
 };
 
 
 static const struct gmin_cfg_var i8880_vars[] = {
-{"XXOV2680:00_CsiPort", "1"},
-{"XXOV2680:00_CsiLanes","1"},
-{"XXOV2680:00_CamClk","0"},
-{"XXGC0310:00_CsiPort", "0"},
-{"XXGC0310:00_CsiLanes", "1"},
-{"XXGC0310:00_CamClk", "1"},
-{},
+   {"XXOV2680:00_CsiPort", "1"},
+   {"XXOV2680:00_CsiLanes", "1"},
+   {"XXOV2680:00_CamClk", "0"},
+   {"XXGC0310:00_CsiPort", "0"},
+   {"XXGC0310:00_CsiLanes", "1"},
+   {"XXGC0310:00_CamClk", "1"},
+   {},
 };
 
 static const struct {
@@ -317,9 +317,9 @@ static const struct {
 } hard_vars[] = {
{ "BYT-T FFD8", ffrd8_vars },
{ "T100TA", t100_vars },
-{ "MRD7", mrd7_vars },
-{ "ST70408", ecs7_vars },
-{ "VTA0803", i8880_vars },
+   { "MRD7", mrd7_vars },
+   { "ST70408", ecs7_vars },
+   { "VTA0803", i8880_vars },
 };
 
 
@@ -343,7 +343,7 @@ static struct gmin_subdev *gmin_subdev_add(struct 
v4l2_subdev *subdev)
 {
int i, ret;
struct device *dev;
-struct i2c_client *client = v4l2_get_subdevdata(subdev);
+   struct i2c_client *client = v4l2_get_subdevdata(subdev);
 
if (!pmic_id) {
 
@@ -361,8 +361,8 @@ static struct gmin_subdev *gmin_subdev_add(struct 
v4l2_subdev *subdev)
return NULL;
 
dev_info(dev,
-   "gmin: initializing atomisp module subdev 

Re: [PATCH v2 0/2] rcar-du, vsp1: rcar-gen3: Add support for colorkey alpha blending

[Eric Anholt]

Alexandru Gheorghe  writes:

> Currently, rcar-du supports colorkeying  only for rcar-gen2 and it uses 
> some hw capability of the display unit(DU) which is not available on gen3.
> In order to implement colorkeying for gen3 we need to use the colorkey
> capability of the VSPD, hence the need to change both drivers rcar-du and
> vsp1.
>
> This patchset had been developed and tested on top of v4.9/rcar-3.5.1 from
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git

A few questions:

Are other drivers interested in supporting this property?  VC4 has the
24-bit RGB colorkey, but I don't see YCBCR support.  Should it be
documented in a generic location?

Does your colorkey end up forcing alpha to 1 for the plane when it's not
matched?


signature.asc
Description: PGP signature

Re: [PATCH v4 6/7] dt-bindings: media: Add Renesas R-Car DRIF binding

[Rob Herring]

On Tue, May 02, 2017 at 02:26:14PM +0100, Ramesh Shanmugasundaram wrote:
> Add binding documentation for Renesas R-Car Digital Radio Interface
> (DRIF) controller.
> 
> Signed-off-by: Ramesh Shanmugasundaram 
> 
> ---
> v4:
>  - port property description modified as suggested by Laurent.
>  - removed renesas vendor tag from sync-active end-point property (Rob)
> ---
>  .../devicetree/bindings/media/renesas,drif.txt | 187 
> +
>  1 file changed, 187 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt 
> b/Documentation/devicetree/bindings/media/renesas,drif.txt
> new file mode 100644
> index ..12428f969958
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,187 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +
> +
> +R-Car Gen3 DRIF is a SPI like receive only slave device. A general
> +representation of DRIF interfacing with a master device is shown below.
> +
> ++-++-+
> +| |-SCK--->|CLK  |
> +|   Master|-SS>|SYNC  DRIFn (slave)  |
> +| |-SD0--->|D0   |
> +| |-SD1--->|D1   |
> ++-++-+
> +
> +As per datasheet, each DRIF channel (drifn) is made up of two internal
> +channels (drifn0 & drifn1). These two internal channels share the common
> +CLK & SYNC. Each internal channel has its own dedicated resources like
> +irq, dma channels, address space & clock. This internal split is not
> +visible to the external master device.
> +
> +The device tree model represents each internal channel as a separate node.
> +The internal channels sharing the CLK & SYNC are tied together by their
> +phandles using a new property called "renesas,bonding". For the rest of
> +the documentation, unless explicitly stated, the word channel implies an
> +internal channel.
> +
> +When both internal channels are enabled they need to be managed together
> +as one (i.e.) they cannot operate alone as independent devices. Out of the
> +two, one of them needs to act as a primary device that accepts common
> +properties of both the internal channels. This channel is identified by a
> +new property called "renesas,primary-bond".

s/new property/property/

> +
> +To summarize,
> +   - When both the internal channels that are bonded together are enabled,
> + the zeroth channel is selected as primary-bond. This channels accepts
> + properties common to all the members of the bond.
> +   - When only one of the bonded channels need to be enabled, the property
> + "renesas,bonding" or "renesas,primary-bond" will have no effect. That
> + enabled channel can act alone as any other independent device.
> +
> +Required properties of an internal channel:
> +---
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of R8A7795 
> SoC.
> +   "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible 
> device.
> +   When compatible with the generic version, nodes must list the
> +   SoC-specific version corresponding to the platform first
> +   followed by the generic version.

Please format this such that it is easier to add the 2nd SoC with this 
block. 

> +- reg: offset and length of that channel.
> +- interrupts: associated with that channel.
> +- clocks: phandle and clock specifier of that channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channels.
> +- dma-names: names of the DMA channel: "rx".
> +- renesas,bonding: phandle to the other channel.
> +
> +Optional properties of an internal channel:
> +---
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of an internal channel when:
> + - It is the only enabled channel of the bond (or)
> + - If it acts as primary among enabled bonds
> +
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- renesas,primary-bond: empty property indicating the channel acts as primary
> + among the bonded channels.
> +- port: child port node corresponding to the data input, in accordance with
> + the video interface bindings defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt. The port
> + node must contain at least one endpoint.
> +
> +Optional endpoint property:
> +---
> +- sync-active: Indicates sync signal polarity, 0/1 for low/high 

[RFC v4 05/18] vb2: Anticipate queue specific DMA attributes for USERPTR buffers

[Sakari Ailus]

The DMA attributes were available for the memop implementation for MMAP
buffers but not for USERPTR buffers. Do the same for USERPTR. This patch
makes no functional changes.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c   | 2 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 ++-
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 3 ++-
 drivers/media/v4l2-core/videobuf2-vmalloc.c| 3 ++-
 include/media/videobuf2-core.h | 3 ++-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index e866115..c659b64 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1025,7 +1025,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const 
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
-   planes[plane].length, dma_dir);
+   planes[plane].length, dma_dir, q->dma_attrs);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace memory for plane 
%d\n",
plane);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index d29a07f..30082a4 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -475,7 +475,8 @@ static inline dma_addr_t vb2_dc_pfn_to_dma(struct device 
*dev, unsigned long pfn
 #endif
 
 static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
-   unsigned long size, enum dma_data_direction dma_dir)
+   unsigned long size, enum dma_data_direction dma_dir,
+   unsigned long attrs)
 {
struct vb2_dc_buf *buf;
struct frame_vector *vec;
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c 
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 29fde1a..102ddb2 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -220,7 +220,8 @@ static void vb2_dma_sg_finish(void *buf_priv)
 
 static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
unsigned long size,
-   enum dma_data_direction dma_dir)
+   enum dma_data_direction dma_dir,
+   unsigned long dma_attrs)
 {
struct vb2_dma_sg_buf *buf;
struct sg_table *sgt;
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c 
b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index f83253a..a4914fc 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -73,7 +73,8 @@ static void vb2_vmalloc_put(void *buf_priv)
 
 static void *vb2_vmalloc_get_userptr(struct device *dev, unsigned long vaddr,
 unsigned long size,
-enum dma_data_direction dma_dir)
+enum dma_data_direction dma_dir,
+unsigned long dma_attrs)
 {
struct vb2_vmalloc_buf *buf;
struct frame_vector *vec;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index cb97c22..4172f6e 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -122,7 +122,8 @@ struct vb2_mem_ops {
 
void*(*get_userptr)(struct device *dev, unsigned long vaddr,
unsigned long size,
-   enum dma_data_direction dma_dir);
+   enum dma_data_direction dma_dir,
+   unsigned long dma_attrs);
void(*put_userptr)(void *buf_priv);
 
void(*prepare)(void *buf_priv);
-- 
2.7.4

[RFC v4 14/18] vb2: Improve struct vb2_mem_ops documentation; alloc and put are for MMAP

[Sakari Ailus]

The alloc() and put() ops are for MMAP buffers only. Document it.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
Reviewed-by: Laurent Pinchart 
---
 include/media/videobuf2-core.h | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 08f1d0e..dd67ae6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -46,16 +46,16 @@ struct vb2_threadio_data;
 
 /**
  * struct vb2_mem_ops - memory handling/memory allocator operations
- * @alloc: allocate video memory and, optionally, allocator private data,
- * return ERR_PTR() on failure or a pointer to allocator private,
- * per-buffer data on success; the returned private structure
- * will then be passed as @buf_priv argument to other ops in this
- * structure. Additional gfp_flags to use when allocating the
- * are also passed to this operation. These flags are from the
- * gfp_flags field of vb2_queue.
- * @put:   inform the allocator that the buffer will no longer be used;
- * usually will result in the allocator freeing the buffer (if
- * no other users of this buffer are present); the @buf_priv
+ * @alloc: allocate video memory for an MMAP buffer and, optionally,
+ * allocator private data, return ERR_PTR() on failure or a pointer
+ * to allocator private, per-buffer data on success; the returned
+ * private structure will then be passed as @buf_priv argument to
+ * other ops in this structure. Additional gfp_flags to use when
+ * allocating the memory are also passed to this operation. These
+ * flags are from the gfp_flags field of vb2_queue.
+ * @put:   inform the allocator that the MMAP buffer will no longer be
+ * used; usually will result in the allocator freeing the buffer
+ * (if no other users of this buffer are present); the @buf_priv
  * argument is the allocator private per-buffer structure
  * previously returned from the alloc callback.
  * @get_dmabuf: acquire userspace memory for a hardware operation; used for
-- 
2.7.4

[RFC v4 12/18] vb2: dma-sg: Let drivers decide DMA attrs of MMAP and USERPTR bufs

[Sakari Ailus]

The desirable DMA attributes are not generic for all devices using
Videobuf2 scatter-gather DMA ops. Let the drivers decide.

As a result, also the DMA-BUF exporter must provide ops for synchronising
the cache. This adds begin_cpu_access and end_cpu_access ops to
vb2_dc_dmabuf_ops.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 81 +++---
 1 file changed, 62 insertions(+), 19 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c 
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 102ddb2..5662f00 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -38,6 +38,7 @@ struct vb2_dma_sg_buf {
struct frame_vector *vec;
int offset;
enum dma_data_direction dma_dir;
+   unsigned long   dma_attrs;
struct sg_table sg_table;
/*
 * This will point to sg_table when used with the MMAP or USERPTR
@@ -114,6 +115,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned 
long dma_attrs,
 
buf->vaddr = NULL;
buf->dma_dir = dma_dir;
+   buf->dma_attrs = dma_attrs;
buf->offset = 0;
buf->size = size;
/* size is already page aligned */
@@ -143,7 +145,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned 
long dma_attrs,
 * prepare() memop is called.
 */
sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ buf->dma_dir, dma_attrs);
if (!sgt->nents)
goto fail_map;
 
@@ -181,7 +183,7 @@ static void vb2_dma_sg_put(void *buf_priv)
dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
buf->num_pages);
dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
-  buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+  buf->dma_dir, buf->dma_attrs);
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->num_pages);
sg_free_table(buf->dma_sgt);
@@ -198,12 +200,13 @@ static void vb2_dma_sg_prepare(void *buf_priv)
struct vb2_dma_sg_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
 
-   /* DMABUF exporter will flush the cache for us */
-   if (buf->db_attach)
-   return;
-
-   dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
-  buf->dma_dir);
+   /*
+* DMABUF exporter will flush the cache for us; only USERPTR
+* and MMAP buffers with non-coherent memory will be flushed.
+*/
+   if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT)
+   dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
+  buf->dma_dir);
 }
 
 static void vb2_dma_sg_finish(void *buf_priv)
@@ -211,11 +214,13 @@ static void vb2_dma_sg_finish(void *buf_priv)
struct vb2_dma_sg_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
 
-   /* DMABUF exporter will flush the cache for us */
-   if (buf->db_attach)
-   return;
-
-   dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+   /*
+* DMABUF exporter will flush the cache for us; only USERPTR
+* and MMAP buffers with non-coherent memory will be flushed.
+*/
+   if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT)
+   dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
+   buf->dma_dir);
 }
 
 static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
@@ -237,6 +242,7 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, 
unsigned long vaddr,
buf->vaddr = NULL;
buf->dev = dev;
buf->dma_dir = dma_dir;
+   buf->dma_attrs = dma_attrs;
buf->offset = vaddr & ~PAGE_MASK;
buf->size = size;
buf->dma_sgt = >sg_table;
@@ -260,7 +266,7 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, 
unsigned long vaddr,
 * prepare() memop is called.
 */
sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ buf->dma_dir, dma_attrs);
if (!sgt->nents)
goto userptr_fail_map;
 
@@ -288,7 +294,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
   __func__, buf->num_pages);
dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir,
-  DMA_ATTR_SKIP_CPU_SYNC);
+  buf->dma_attrs);
if 

[RFC v4 01/18] vb2: Rename confusingly named internal buffer preparation functions

[Sakari Ailus]

Rename __qbuf_*() functions which are specific to a buffer type as
__prepare_*() which matches with what they do. The naming was there for
historical reasons; the purpose of the functions was changed without
renaming them.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/v4l2-core/videobuf2-core.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 94afbbf9..8df680d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -956,9 +956,9 @@ void vb2_discard_done(struct vb2_queue *q)
 EXPORT_SYMBOL_GPL(vb2_discard_done);
 
 /**
- * __qbuf_mmap() - handle qbuf of an MMAP buffer
+ * __prepare_mmap() - prepare an MMAP buffer
  */
-static int __qbuf_mmap(struct vb2_buffer *vb, const void *pb)
+static int __prepare_mmap(struct vb2_buffer *vb, const void *pb)
 {
int ret = 0;
 
@@ -969,9 +969,9 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const void 
*pb)
 }
 
 /**
- * __qbuf_userptr() - handle qbuf of a USERPTR buffer
+ * __prepare_userptr() - prepare a USERPTR buffer
  */
-static int __qbuf_userptr(struct vb2_buffer *vb, const void *pb)
+static int __prepare_userptr(struct vb2_buffer *vb, const void *pb)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1087,9 +1087,9 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const 
void *pb)
 }
 
 /**
- * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
+ * __prepare_dmabuf() - prepare a DMABUF buffer
  */
-static int __qbuf_dmabuf(struct vb2_buffer *vb, const void *pb)
+static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1255,13 +1255,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
 
switch (q->memory) {
case VB2_MEMORY_MMAP:
-   ret = __qbuf_mmap(vb, pb);
+   ret = __prepare_mmap(vb, pb);
break;
case VB2_MEMORY_USERPTR:
-   ret = __qbuf_userptr(vb, pb);
+   ret = __prepare_userptr(vb, pb);
break;
case VB2_MEMORY_DMABUF:
-   ret = __qbuf_dmabuf(vb, pb);
+   ret = __prepare_dmabuf(vb, pb);
break;
default:
WARN(1, "Invalid queue type\n");
-- 
2.7.4

[RFC v4 15/18] vb2: Dma direction is always DMA_TO_DEVICE in buffer preparation

[Sakari Ailus]

The patch changes the DMA direction from DMA_FROM_DEVICE to DMA_TO_DEVICE
for capture buffers.

The DMA API does not require that any synchronisation is done to the
buffer when it is passed to hardware for writing _but_ there's a caveat:
the user *must not* have written to the buffer.

The V4L2 API does however not require this. Instead, it requires that the
user does not access the buffer since it is queued to the device using
VIDIOC_QBUF IOCTL until the buffer is dequeued again using VIDIOC_DQBUF
IOCTL.

So in this case we want to ensure there will be no dirty cache lines that
could end up to memory possibly after the device has written to the same
memory area. What data gets written to the system memory from the cache is
not extremely important. Still, an for debugging purposes an application
capturing images could fill the buffer with a known pattern which will be
overwritten by the device, hence DMA_TO_DEVICE.

If an application can guarantee that it has not written to the buffer, it
can specify the V4L2_BUF_FLAG_NO_CACHE_SYNC flag to omit the sync
operation.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 2 +-
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index f572911..320e53a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -103,7 +103,7 @@ static void vb2_dc_prepare(void *buf_priv)
 */
if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
-  buf->dma_dir);
+  DMA_TO_DEVICE);
 }
 
 static void vb2_dc_finish(void *buf_priv)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c 
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 5662f00..88b2530 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -206,7 +206,7 @@ static void vb2_dma_sg_prepare(void *buf_priv)
 */
if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT)
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
-  buf->dma_dir);
+  DMA_TO_DEVICE);
 }
 
 static void vb2_dma_sg_finish(void *buf_priv)
-- 
2.7.4

[RFC v4 16/18] vb2: Do sync plane cache only for CAPTURE buffers in finish memop

[Sakari Ailus]

There is no need synchronise buffer cache for OUTPUT devices when the
buffer is dequeued as the hardware only reads from the buffer. Only sync
for capture buffers.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 5 +++--
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 320e53a..41d5782 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -115,9 +115,10 @@ static void vb2_dc_finish(void *buf_priv)
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt) &&
+   buf->dma_dir == DMA_FROM_DEVICE)
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
-   buf->dma_dir);
+   DMA_FROM_DEVICE);
 }
 
 /*/
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c 
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 88b2530..e30c869 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -218,9 +218,10 @@ static void vb2_dma_sg_finish(void *buf_priv)
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT)
+   if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT &&
+   buf->dma_dir == DMA_FROM_DEVICE)
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
-   buf->dma_dir);
+   DMA_FROM_DEVICE);
 }
 
 static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
-- 
2.7.4

[RFC v4 03/18] vb2: Move cache synchronisation from buffer done to dqbuf handler

[Sakari Ailus]

The cache synchronisation may be a time consuming operation and thus not
best performed in an interrupt which is a typical context for
vb2_buffer_done() calls. This may consume up to tens of ms on some
machines, depending on the buffer size.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-core.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 8bf3369..e866115 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -889,7 +889,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb->vb2_queue;
unsigned long flags;
-   unsigned int plane;
 
if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
return;
@@ -910,10 +909,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, "done processing on buffer %d, state: %d\n",
vb->index, state);
 
-   /* sync buffers */
-   for (plane = 0; plane < vb->num_planes; ++plane)
-   call_void_memop(vb, finish, vb->planes[plane].mem_priv);
-
spin_lock_irqsave(>done_lock, flags);
if (state == VB2_BUF_STATE_QUEUED ||
state == VB2_BUF_STATE_REQUEUEING) {
@@ -1573,6 +1568,10 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 
vb->state = VB2_BUF_STATE_DEQUEUED;
 
+   /* sync buffers */
+   for (i = 0; i < vb->num_planes; ++i)
+   call_void_memop(vb, finish, vb->planes[i].mem_priv);
+
/* unmap DMABUF buffer */
if (q->memory == VB2_MEMORY_DMABUF)
for (i = 0; i < vb->num_planes; ++i) {
-- 
2.7.4

[RFC v4 08/18] vb2: dma-contig: Don't warn on failure in obtaining scatterlist

[Sakari Ailus]

vb2_dc_get_base_sgt() which obtains the scatterlist already prints
information on why the scatterlist could not be obtained.

Also, remove the useless warning of a failed kmalloc().

Signed-off-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index ddbbcf0..22636cd 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -370,10 +370,8 @@ static struct sg_table *vb2_dc_get_base_sgt(struct 
vb2_dc_buf *buf)
struct sg_table *sgt;
 
sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
-   if (!sgt) {
-   dev_err(buf->dev, "failed to alloc sg table\n");
+   if (!sgt)
return NULL;
-   }
 
ret = dma_get_sgtable_attrs(buf->dev, sgt, buf->cookie, buf->dma_addr,
buf->size, buf->attrs);
@@ -400,7 +398,7 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, 
unsigned long flags)
if (!buf->dma_sgt)
buf->dma_sgt = vb2_dc_get_base_sgt(buf);
 
-   if (WARN_ON(!buf->dma_sgt))
+   if (!buf->dma_sgt)
return NULL;
 
dbuf = dma_buf_export(_info);
-- 
2.7.4

[RFC v4 10/18] vb2: dma-contig: Fix DMA attribute and cache management

[Sakari Ailus]

Patch ccc66e73 ("ARM: 8508/2: videobuf2-dc: Let drivers specify DMA
attrs") added support for driver specific DMA attributes to
videobuf2-dma-contig but it had several issues in it.

In particular,

- cache operations were only performed on USERPTR buffers,

- DMA attributes were set only for MMAP buffers and

- it did not provide begin_cpu_access() and end_cpu_access() dma_buf_ops
  callbacks for cache syncronisation on exported MMAP buffers.

This patch corrects these issues.

Also arrange the header files alphabetically.

Fixes: ccc66e73 ("ARM: 8508/2: videobuf2-dc: Let drivers specify DMA attrs")
Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 94 --
 1 file changed, 72 insertions(+), 22 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 0afc3da..8b0298a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -11,12 +11,12 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -97,12 +97,13 @@ static void vb2_dc_prepare(void *buf_priv)
struct vb2_dc_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
 
-   /* DMABUF exporter will flush the cache for us */
-   if (!buf->vec)
-   return;
-
-   dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
-  buf->dma_dir);
+   /*
+* DMABUF exporter will flush the cache for us; only USERPTR
+* and MMAP buffers with non-coherent memory will be flushed.
+*/
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
+  buf->dma_dir);
 }
 
 static void vb2_dc_finish(void *buf_priv)
@@ -110,11 +111,13 @@ static void vb2_dc_finish(void *buf_priv)
struct vb2_dc_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
 
-   /* DMABUF exporter will flush the cache for us */
-   if (!buf->vec)
-   return;
-
-   dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+   /*
+* DMABUF exporter will flush the cache for us; only USERPTR
+* and MMAP buffers with non-coherent memory will be flushed.
+*/
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
+   buf->dma_dir);
 }
 
 /*/
@@ -142,6 +145,7 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long 
attrs,
  gfp_t gfp_flags)
 {
struct vb2_dc_buf *buf;
+   int ret;
 
if (WARN_ON(!dev))
return ERR_PTR(-EINVAL);
@@ -152,9 +156,9 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long 
attrs,
 
buf->attrs = attrs;
buf->cookie = dma_alloc_attrs(dev, size, >dma_addr,
-   GFP_KERNEL | gfp_flags, buf->attrs);
+ GFP_KERNEL | gfp_flags, buf->attrs);
if (!buf->cookie) {
-   dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
+   dev_err(dev, "dma_alloc_attrs of size %ld failed\n", size);
kfree(buf);
return ERR_PTR(-ENOMEM);
}
@@ -167,6 +171,16 @@ static void *vb2_dc_alloc(struct device *dev, unsigned 
long attrs,
buf->size = size;
buf->dma_dir = dma_dir;
 
+   ret = dma_get_sgtable_attrs(buf->dev, >__dma_sgt, buf->cookie,
+   buf->dma_addr, buf->size, buf->attrs);
+   if (ret < 0) {
+   dma_free_attrs(dev, size, buf->cookie, buf->dma_addr,
+  buf->attrs);
+   put_device(dev);
+   return ERR_PTR(-ENOMEM);
+   }
+
+   buf->dma_sgt = >__dma_sgt;
buf->handler.refcount = >refcount;
buf->handler.put = vb2_dc_put;
buf->handler.arg = buf;
@@ -339,6 +353,40 @@ static void *vb2_dc_dmabuf_ops_kmap(struct dma_buf *dbuf, 
unsigned long pgnum)
return buf->vaddr ? buf->vaddr + pgnum * PAGE_SIZE : NULL;
 }
 
+static int vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf,
+ enum dma_data_direction direction)
+{
+   struct vb2_dc_buf *buf = dbuf->priv;
+   struct sg_table *sgt = buf->dma_sgt;
+
+   /*
+* DMABUF exporter will flush the cache for us; only USERPTR
+* and MMAP buffers with non-coherent memory will be flushed.
+*/
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents,
+   buf->dma_dir);
+
+   return 0;
+}
+
+static int 

[RFC v4 07/18] vb2: dma-contig: Remove redundant sgt_base field

[Sakari Ailus]

The struct vb2_dc_buf contains two struct sg_table fields: sgt_base and
dma_sgt. The former is used by DMA-BUF buffers whereas the latter is used
by USERPTR.

Unify the two, leaving dma_sgt.

MMAP buffers do not need cache flushing since they have been allocated
using dma_alloc_coherent().

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index a8a46a8..ddbbcf0 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -31,12 +31,13 @@ struct vb2_dc_buf {
unsigned long   attrs;
enum dma_data_direction dma_dir;
struct sg_table *dma_sgt;
-   struct frame_vector *vec;
 
/* MMAP related */
struct vb2_vmarea_handler   handler;
refcount_t  refcount;
-   struct sg_table *sgt_base;
+
+   /* USERPTR related */
+   struct frame_vector *vec;
 
/* DMABUF related */
struct dma_buf_attachment   *db_attach;
@@ -96,7 +97,7 @@ static void vb2_dc_prepare(void *buf_priv)
struct sg_table *sgt = buf->dma_sgt;
 
/* DMABUF exporter will flush the cache for us */
-   if (!sgt || buf->db_attach)
+   if (!buf->vec)
return;
 
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
@@ -109,7 +110,7 @@ static void vb2_dc_finish(void *buf_priv)
struct sg_table *sgt = buf->dma_sgt;
 
/* DMABUF exporter will flush the cache for us */
-   if (!sgt || buf->db_attach)
+   if (!buf->vec)
return;
 
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
@@ -126,9 +127,9 @@ static void vb2_dc_put(void *buf_priv)
if (!refcount_dec_and_test(>refcount))
return;
 
-   if (buf->sgt_base) {
-   sg_free_table(buf->sgt_base);
-   kfree(buf->sgt_base);
+   if (buf->dma_sgt) {
+   sg_free_table(buf->dma_sgt);
+   kfree(buf->dma_sgt);
}
dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr,
   buf->attrs);
@@ -239,13 +240,13 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, 
struct device *dev,
/* Copy the buf->base_sgt scatter list to the attachment, as we can't
 * map the same scatter list to multiple attachments at the same time.
 */
-   ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
+   ret = sg_alloc_table(sgt, buf->dma_sgt->orig_nents, GFP_KERNEL);
if (ret) {
kfree(attach);
return -ENOMEM;
}
 
-   rd = buf->sgt_base->sgl;
+   rd = buf->dma_sgt->sgl;
wr = sgt->sgl;
for (i = 0; i < sgt->orig_nents; ++i) {
sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
@@ -396,10 +397,10 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, 
unsigned long flags)
exp_info.flags = flags;
exp_info.priv = buf;
 
-   if (!buf->sgt_base)
-   buf->sgt_base = vb2_dc_get_base_sgt(buf);
+   if (!buf->dma_sgt)
+   buf->dma_sgt = vb2_dc_get_base_sgt(buf);
 
-   if (WARN_ON(!buf->sgt_base))
+   if (WARN_ON(!buf->dma_sgt))
return NULL;
 
dbuf = dma_buf_export(_info);
-- 
2.7.4

[RFC v4 09/18] vb2: dma-contig: Allocate sgt as part of struct vb2_dc_buf

[Sakari Ailus]

The scatterlist table is needed in the vast majority of the cases.
Allocate struct sg_table as part of the struct. This has the benefit of
making managing the buffer data structure allocation, setup and release
more simple.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 83 ++
 1 file changed, 32 insertions(+), 51 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 22636cd..0afc3da 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -30,6 +30,7 @@ struct vb2_dc_buf {
dma_addr_t  dma_addr;
unsigned long   attrs;
enum dma_data_direction dma_dir;
+   struct sg_table __dma_sgt;
struct sg_table *dma_sgt;
 
/* MMAP related */
@@ -127,10 +128,9 @@ static void vb2_dc_put(void *buf_priv)
if (!refcount_dec_and_test(>refcount))
return;
 
-   if (buf->dma_sgt) {
+   if (buf->dma_sgt)
sg_free_table(buf->dma_sgt);
-   kfree(buf->dma_sgt);
-   }
+
dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr,
   buf->attrs);
put_device(buf->dev);
@@ -364,26 +364,6 @@ static struct dma_buf_ops vb2_dc_dmabuf_ops = {
.release = vb2_dc_dmabuf_ops_release,
 };
 
-static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
-{
-   int ret;
-   struct sg_table *sgt;
-
-   sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
-   if (!sgt)
-   return NULL;
-
-   ret = dma_get_sgtable_attrs(buf->dev, sgt, buf->cookie, buf->dma_addr,
-   buf->size, buf->attrs);
-   if (ret < 0) {
-   dev_err(buf->dev, "failed to get scatterlist from DMA API\n");
-   kfree(sgt);
-   return NULL;
-   }
-
-   return sgt;
-}
-
 static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags)
 {
struct vb2_dc_buf *buf = buf_priv;
@@ -395,11 +375,19 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, 
unsigned long flags)
exp_info.flags = flags;
exp_info.priv = buf;
 
-   if (!buf->dma_sgt)
-   buf->dma_sgt = vb2_dc_get_base_sgt(buf);
+   if (!buf->dma_sgt) {
+   int ret;
 
-   if (!buf->dma_sgt)
-   return NULL;
+   ret = dma_get_sgtable_attrs(buf->dev, >__dma_sgt,
+   buf->cookie, buf->dma_addr,
+   buf->size, buf->attrs);
+   if (ret < 0) {
+   dev_err(buf->dev, "failed to get scatterlist from DMA 
API\n");
+   return NULL;
+   }
+
+   buf->dma_sgt = >__dma_sgt;
+   }
 
dbuf = dma_buf_export(_info);
if (IS_ERR(dbuf))
@@ -435,7 +423,6 @@ static void vb2_dc_put_userptr(void *buf_priv)
for (i = 0; i < frame_vector_count(buf->vec); i++)
set_page_dirty_lock(pages[i]);
sg_free_table(sgt);
-   kfree(sgt);
}
vb2_destroy_framevec(buf->vec);
kfree(buf);
@@ -481,7 +468,6 @@ static void *vb2_dc_get_userptr(struct device *dev, 
unsigned long vaddr,
unsigned long offset;
int n_pages, i;
int ret = 0;
-   struct sg_table *sgt;
unsigned long contig_size;
unsigned long dma_align = dma_get_cache_alignment();
 
@@ -529,33 +515,31 @@ static void *vb2_dc_get_userptr(struct device *dev, 
unsigned long vaddr,
goto out;
}
 
-   sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
-   if (!sgt) {
-   pr_err("failed to allocate sg table\n");
-   ret = -ENOMEM;
-   goto fail_pfnvec;
-   }
-
-   ret = sg_alloc_table_from_pages(sgt, frame_vector_pages(vec), n_pages,
-   offset, size, GFP_KERNEL);
+   ret = sg_alloc_table_from_pages(>__dma_sgt,
+   frame_vector_pages(vec), n_pages,
+   offset, size, GFP_KERNEL);
if (ret) {
pr_err("failed to initialize sg table\n");
-   goto fail_sgt;
+   goto fail_pfnvec;
}
 
+   buf->dma_sgt = >__dma_sgt;
+
/*
 * No need to sync to the device, this will happen later when the
 * prepare() memop is called.
 */
-   sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
-   if (sgt->nents <= 0) {
+   buf->dma_sgt->nents = dma_map_sg_attrs(buf->dev, buf->dma_sgt->sgl,
+  buf->dma_sgt->orig_nents,
+   

[RFC v4 06/18] vb2: dma-contig: Assign DMA attrs for a buffer unconditionally

[Sakari Ailus]

attrs used to be a pointer and the caller of vb2_dc_alloc() could
optionally provide it, or NULL. This was when struct dma_attrs was used
to describe DMA attributes rather than an unsigned long value. There is no
longer a need to maintain the condition, assign the value unconditionally.
There is no functional difference because the memory was initialised to
zero anyway.

Fixes: 00085f1e ("dma-mapping: use unsigned long for dma_attrs")
Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 30082a4..a8a46a8 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -149,8 +149,7 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long 
attrs,
if (!buf)
return ERR_PTR(-ENOMEM);
 
-   if (attrs)
-   buf->attrs = attrs;
+   buf->attrs = attrs;
buf->cookie = dma_alloc_attrs(dev, size, >dma_addr,
GFP_KERNEL | gfp_flags, buf->attrs);
if (!buf->cookie) {
-- 
2.7.4

[RFC v4 18/18] v4l: Use non-consistent DMA mappings for hardware that deserves it

[Sakari Ailus]

Set DMA_ATTR_NON_CONSISTENT for hardware that uses non-consistent memory.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/ispvideo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index 218e6d7..b7e 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1333,6 +1333,7 @@ static int isp_video_open(struct file *file)
queue->buf_struct_size = sizeof(struct isp_buffer);
queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
queue->dev = video->isp->dev;
+   queue->dma_attrs = DMA_ATTR_NON_CONSISTENT;
 
ret = vb2_queue_init(>queue);
if (ret < 0) {
-- 
2.7.4

[RFC v4 11/18] vb2: dma-contig: Add WARN_ON_ONCE() to check for potential bugs

[Sakari Ailus]

The scatterlist should always be present when the cache would need to be
flushed. Each buffer type has its own means to provide that. Add
WARN_ON_ONCE() to check the scatterist exists.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 8b0298a..f572911 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -101,7 +101,7 @@ static void vb2_dc_prepare(void *buf_priv)
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
   buf->dma_dir);
 }
@@ -115,7 +115,7 @@ static void vb2_dc_finish(void *buf_priv)
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
buf->dma_dir);
 }
@@ -363,7 +363,7 @@ static int vb2_dc_dmabuf_ops_begin_cpu_access(struct 
dma_buf *dbuf,
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents,
buf->dma_dir);
 
@@ -380,7 +380,7 @@ static int vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf 
*dbuf,
 * DMABUF exporter will flush the cache for us; only USERPTR
 * and MMAP buffers with non-coherent memory will be flushed.
 */
-   if (buf->attrs & DMA_ATTR_NON_CONSISTENT)
+   if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt))
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents,
   buf->dma_dir);
 
-- 
2.7.4

[RFC v4 17/18] docs-rst: Document precise V4L2_BUF_FLAG_NO_CACHE_SYNC flag behaviour

[Sakari Ailus]

Document when should the user specify V4L2_BUF_FLAG_NO_CACHE_SYNC flag.

Signed-off-by: Sakari Ailus 
---
 Documentation/media/uapi/v4l/buffer.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index 9eb42bd..e1f93dd 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -570,6 +570,27 @@ Buffer Flags
then read by another one, in which case the flag should be set in both
:ref:`VIDIOC_QBUF` and :ref:`VIDIOC_DQBUF` ioctls. This flag has no
effect on some devices / architectures.
+
+   More specifically, this flag causes cache synchronisation to
+   be skipped for OUTPUT buffers when the buffer is queued. The
+   flag can be used if the buffer has been previously written to
+   by hardware but has not been written to by the CPU.
+
+   Additionally, if this flag is specified for a CAPTURE buffer
+   when it is queued, cache synchronisation is skipped. This
+   signals that the application can guarantee that it has not
+   written to the buffer memory since it was last dequeued from
+   the device.
+
+   Specifying this flag for a CAPTURE buffer when
+   dequeueing a buffer will skip cache maintenance for the buffer
+   memory. An application may not access the buffer memory in
+   that case but it may well be passed onwards to another device
+   in the system.
+
+   Specifying this flag has no effect when dequeuing an OUTPUT
+   buffer.
+
 * .. _`V4L2-BUF-FLAG-LAST`:
 
   - ``V4L2_BUF_FLAG_LAST``
-- 
2.7.4

[RFC v4 13/18] vb2: Don't sync cache for a buffer if so requested

[Sakari Ailus]

From: Samu Onkalo 

The user may request to the driver (vb2) to skip the cache maintenance
operations in case the buffer does not need cache synchronisation, e.g. in
cases where the buffer is passed between hardware blocks without it being
touched by the CPU.

Also document that the prepare and finish vb2_mem_ops might not get called
every time the buffer ownership changes between the kernel and the user
space.

Signed-off-by: Samu Onkalo 
Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-core.c | 101 +--
 drivers/media/v4l2-core/videobuf2-v4l2.c |  14 -
 include/media/videobuf2-core.h   |  23 ---
 3 files changed, 97 insertions(+), 41 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index c659b64..fbe0db1 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -189,6 +189,28 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
 
 /**
+ * __mem_prepare_planes() - call finish mem op for all planes of the buffer
+ */
+static void __mem_prepare_planes(struct vb2_buffer *vb)
+{
+   unsigned int plane;
+
+   for (plane = 0; plane < vb->num_planes; ++plane)
+   call_void_memop(vb, prepare, vb->planes[plane].mem_priv);
+}
+
+/**
+ * __mem_finish_planes() - call finish mem op for all planes of the buffer
+ */
+static void __mem_finish_planes(struct vb2_buffer *vb)
+{
+   unsigned int plane;
+
+   for (plane = 0; plane < vb->num_planes; ++plane)
+   call_void_memop(vb, finish, vb->planes[plane].mem_priv);
+}
+
+/**
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
  */
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
@@ -953,20 +975,29 @@ EXPORT_SYMBOL_GPL(vb2_discard_done);
 /**
  * __prepare_mmap() - prepare an MMAP buffer
  */
-static int __prepare_mmap(struct vb2_buffer *vb, const void *pb)
+static int __prepare_mmap(struct vb2_buffer *vb, const void *pb,
+ bool no_cache_sync)
 {
-   int ret = 0;
+   int ret;
 
-   if (pb)
+   if (pb) {
ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
 vb, pb, vb->planes);
-   return ret ? ret : call_vb_qop(vb, buf_prepare, vb);
+   if (ret)
+   return ret;
+   }
+
+   if (!no_cache_sync)
+   __mem_prepare_planes(vb);
+
+   return call_vb_qop(vb, buf_prepare, vb);
 }
 
 /**
  * __prepare_userptr() - prepare a USERPTR buffer
  */
-static int __prepare_userptr(struct vb2_buffer *vb, const void *pb)
+static int __prepare_userptr(struct vb2_buffer *vb, const void *pb,
+bool no_cache_sync)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1057,6 +1088,11 @@ static int __prepare_userptr(struct vb2_buffer *vb, 
const void *pb)
dprintk(1, "buffer initialization failed\n");
goto err;
}
+
+   /* This is new buffer memory --- always synchronise cache. */
+   __mem_prepare_planes(vb);
+   } else if (!no_cache_sync) {
+   __mem_prepare_planes(vb);
}
 
ret = call_vb_qop(vb, buf_prepare, vb);
@@ -1084,7 +1120,8 @@ static int __prepare_userptr(struct vb2_buffer *vb, const 
void *pb)
 /**
  * __prepare_dmabuf() - prepare a DMABUF buffer
  */
-static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb)
+static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb,
+   bool no_cache_sync)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1199,6 +1236,11 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const 
void *pb)
dprintk(1, "buffer initialization failed\n");
goto err;
}
+
+   /* This is new buffer memory --- always synchronise cache. */
+   __mem_prepare_planes(vb);
+   } else if (!no_cache_sync) {
+   __mem_prepare_planes(vb);
}
 
ret = call_vb_qop(vb, buf_prepare, vb);
@@ -1231,10 +1273,10 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
call_void_vb_qop(vb, buf_queue, vb);
 }
 
-static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
+static int __buf_prepare(struct vb2_buffer *vb, const void *pb,
+bool no_cache_sync)
 {
struct vb2_queue *q = vb->vb2_queue;
-   unsigned int plane;
int ret;
 
if (q->error) {
@@ -1246,13 +1288,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
 
switch (q->memory) {
case 

[RFC v4 00/18] vb2: Handle user cache hints, allow drivers to choose cache coherency

[Sakari Ailus]

Hello,

This is a rebased and partially reworked version of the vb2 cache hints
support patch series posted by first myself, then Laurent and then myself
again.

I'm still posting this as RFC primarily because more testing and driver
changes will be needed. In particular, a lot of platform drivers assume
non-coherent memory but are not properly labelled as such.

Please see the end of the message for detailed changes.

This set unifies the cache coherency hint flags and corrects cache
management in videobuf2 dma-contig and dma-sg memtype implementation. The
support for non-coherent memory is completed: support for MMAP buffers is
added and begin_cpu_access and end_cpu_access functions are added to DMA
ops.

Comments are welcome.

changes since RFC v3:

- Document V4L2_BUF_FLAG_NO_CACHE_SYNC flag behaviour for QBUF and DQBUF
  for CAPTURE and OUTPUT buffers.

- Set queue dma_attrs DMA_ATTR_NON_CONSISTENT flag for omap3isp.

- Put dma_sgt in vb2_dc_buf in order to avoid allocating and releasing it
  separately. It's generally needed anyway.

- Buffer preparation DMA direction is generally DMA_TO_DEVICE for both
  CAPTURE and OUTPUT buffers: V4L2 does not guarantee that the user space
  could not write to capture buffers as well.

  Documentation/DMA-API.txt:

- Before reading values that have been written by DMA from the device
  (use the DMA_FROM_DEVICE direction)
- After writing values that will be written to the device using DMA
  (use the DMA_TO_DEVICE) direction
- before *and* after handing memory to the device if the memory is
  DMA_BIDIRECTIONAL

  Cache maintenance can also be skipped for OUTPUT buffers in buffer
  finish as the hardware did not write to the buffer.

changes since RFC v2:

- Nicer looking tests for the need for syncing.

- Also set DMA attributes for USERPTR buffers.

- Unconditionally assign buf->attrs for MMAP buffers.

- Don't call vb2_dc_get_base_sgt() until buf->dev is set.

- Provide {begin,end}_cpu_access() dmabuf ops for cache management.

- Make similar changes to dma-sg memops to support DMA attributes.


Sakari Ailus (13):
  vb2: Rename confusingly named internal buffer preparation functions
  vb2: Move buffer cache synchronisation to prepare from queue
  vb2: Move cache synchronisation from buffer done to dqbuf handler
  v4l: Unify cache management hint buffer flags
  vb2: Anticipate queue specific DMA attributes for USERPTR buffers
  vb2: dma-contig: Assign DMA attrs for a buffer unconditionally
  vb2: dma-contig: Remove redundant sgt_base field
  vb2: dma-contig: Don't warn on failure in obtaining scatterlist
  vb2: dma-contig: Move vb2_dc_get_base_sgt() up
  vb2: dma-contig: Fix DMA attribute and cache management
  vb2: dma-contig: Add WARN_ON_ONCE() to check for potential bugs
  vb2: dma-sg: Let drivers decide DMA attrs of MMAP and USERPTR bufs
  vb2: Improve struct vb2_mem_ops documentation; alloc and put are for
MMAP

Samu Onkalo (1):
  vb2: Don't sync cache for a buffer if so requested

 Documentation/media/uapi/v4l/buffer.rst|  24 ++--
 .../media/uapi/v4l/vidioc-prepare-buf.rst  |   5 +-
 drivers/media/v4l2-core/videobuf2-core.c   | 129 ++---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 120 ---
 drivers/media/v4l2-core/videobuf2-dma-sg.c |  47 ++--
 drivers/media/v4l2-core/videobuf2-v4l2.c   |  14 ++-
 drivers/media/v4l2-core/videobuf2-vmalloc.c|   3 +-
 include/media/videobuf2-core.h |  46 +---
 include/trace/events/v4l2.h|   3 +-
 include/uapi/linux/videodev2.h |   7 +-
 10 files changed, 263 insertions(+), 135 deletions(-)

-- 
Regards,
Sakari



Sakari Ailus (17):
  vb2: Rename confusingly named internal buffer preparation functions
  vb2: Move buffer cache synchronisation to prepare from queue
  vb2: Move cache synchronisation from buffer done to dqbuf handler
  v4l: Unify cache management hint buffer flags
  vb2: Anticipate queue specific DMA attributes for USERPTR buffers
  vb2: dma-contig: Assign DMA attrs for a buffer unconditionally
  vb2: dma-contig: Remove redundant sgt_base field
  vb2: dma-contig: Don't warn on failure in obtaining scatterlist
  vb2: dma-contig: Allocate sgt as part of struct vb2_dc_buf
  vb2: dma-contig: Fix DMA attribute and cache management
  vb2: dma-contig: Add WARN_ON_ONCE() to check for potential bugs
  vb2: dma-sg: Let drivers decide DMA attrs of MMAP and USERPTR bufs
  vb2: Improve struct vb2_mem_ops documentation; alloc and put are for
MMAP
  vb2: Dma direction is always DMA_TO_DEVICE in buffer preparation
  vb2: Do sync plane cache only for CAPTURE buffers in finish memop
  docs-rst: Document precise V4L2_BUF_FLAG_NO_CACHE_SYNC flag behaviour
  v4l: Use non-consistent DMA mappings for hardware that deserves it

Samu Onkalo (1):
  vb2: Don't sync cache for a buffer if so requested

 

[RFC v4 02/18] vb2: Move buffer cache synchronisation to prepare from queue

[Sakari Ailus]

The buffer cache should be synchronised in buffer preparation, not when
the buffer is queued to the device. Fix this.

Mmap buffers do not need cache synchronisation since they are always
coherent.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-core.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 8df680d..8bf3369 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1227,23 +1227,19 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, 
const void *pb)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
-   unsigned int plane;
 
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
trace_vb2_buf_queue(q, vb);
 
-   /* sync buffers */
-   for (plane = 0; plane < vb->num_planes; ++plane)
-   call_void_memop(vb, prepare, vb->planes[plane].mem_priv);
-
call_void_vb_qop(vb, buf_queue, vb);
 }
 
 static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
 {
struct vb2_queue *q = vb->vb2_queue;
+   unsigned int plane;
int ret;
 
if (q->error) {
@@ -1268,11 +1264,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
ret = -EINVAL;
}
 
-   if (ret)
+   if (ret) {
dprintk(1, "buffer preparation failed: %d\n", ret);
-   vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;
+   vb->state = VB2_BUF_STATE_DEQUEUED;
+   return ret;
+   }
 
-   return ret;
+   /* sync buffers */
+   for (plane = 0; plane < vb->num_planes; ++plane)
+   call_void_memop(vb, prepare, vb->planes[plane].mem_priv);
+
+   vb->state = VB2_BUF_STATE_PREPARED;
+
+   return 0;
 }
 
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
-- 
2.7.4

[RFC v4 04/18] v4l: Unify cache management hint buffer flags

[Sakari Ailus]

The V4L2_BUF_FLAG_NO_CACHE_INVALIDATE and V4L2_BUF_FLAG_NO_CACHE_CLEAN
buffer flags are currently not used by the kernel. Replace the definitions
by a single V4L2_BUF_FLAG_NO_CACHE_SYNC flag to be used by further
patches.

Different cache architectures should not be visible to the user space
which can make no meaningful use of the differences anyway. In case a
device can make use of non-coherent memory accesses, the necessary cache
operations depend on the CPU architecture and the buffer type, not the
requests of the user. The cache operation itself may be skipped on the
user's request which was the purpose of the two flags.

On ARM the invalidate and clean are separate operations whereas on
x86(-64) the two are a single operation (flush). Whether the hardware uses
the buffer for reading (V4L2_BUF_TYPE_*_OUTPUT*) or writing
(V4L2_BUF_TYPE_*CAPTURE*) already defines the required cache operation
(clean and invalidate, respectively). No user input is required.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/buffer.rst| 24 --
 .../media/uapi/v4l/vidioc-prepare-buf.rst  |  5 ++---
 include/trace/events/v4l2.h|  3 +--
 include/uapi/linux/videodev2.h |  7 +--
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index ae6ee73..9eb42bd 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -559,23 +559,17 @@ Buffer Flags
:ref:`VIDIOC_PREPARE_BUF `,
:ref:`VIDIOC_QBUF` or
:ref:`VIDIOC_DQBUF ` ioctl is called.
-* .. _`V4L2-BUF-FLAG-NO-CACHE-INVALIDATE`:
+* .. _`V4L2-BUF-FLAG-NO-CACHE-SYNC`:
 
-  - ``V4L2_BUF_FLAG_NO_CACHE_INVALIDATE``
+  - ``V4L2_BUF_FLAG_NO_CACHE_SYNC``
   - 0x0800
-  - Caches do not have to be invalidated for this buffer. Typically
-   applications shall use this flag if the data captured in the
-   buffer is not going to be touched by the CPU, instead the buffer
-   will, probably, be passed on to a DMA-capable hardware unit for
-   further processing or output.
-* .. _`V4L2-BUF-FLAG-NO-CACHE-CLEAN`:
-
-  - ``V4L2_BUF_FLAG_NO_CACHE_CLEAN``
-  - 0x1000
-  - Caches do not have to be cleaned for this buffer. Typically
-   applications shall use this flag for output buffers if the data in
-   this buffer has not been created by the CPU but by some
-   DMA-capable unit, in which case caches have not been used.
+  - Do not perform CPU cache synchronisation operations when the buffer is
+   queued or dequeued. The user is responsible for the correct use of
+   this flag. It should be only used when the buffer is not accessed
+   using the CPU, e.g. the buffer is written to by a hardware block and
+   then read by another one, in which case the flag should be set in both
+   :ref:`VIDIOC_QBUF` and :ref:`VIDIOC_DQBUF` ioctls. This flag has no
+   effect on some devices / architectures.
 * .. _`V4L2-BUF-FLAG-LAST`:
 
   - ``V4L2_BUF_FLAG_LAST``
diff --git a/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst 
b/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst
index bdcfd9f..80aeb7e 100644
--- a/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst
+++ b/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst
@@ -36,9 +36,8 @@ pass ownership of the buffer to the driver before actually 
enqueuing it,
 using the :ref:`VIDIOC_QBUF` ioctl, and to prepare it for future I/O. Such
 preparations may include cache invalidation or cleaning. Performing them
 in advance saves time during the actual I/O. In case such cache
-operations are not required, the application can use one of
-``V4L2_BUF_FLAG_NO_CACHE_INVALIDATE`` and
-``V4L2_BUF_FLAG_NO_CACHE_CLEAN`` flags to skip the respective step.
+operations are not required, the application can use the
+``V4L2_BUF_FLAG_NO_CACHE_SYNC`` flag to skip the cache synchronization step.
 
 The struct :c:type:`v4l2_buffer` structure is specified in
 :ref:`buffer`.
diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index b3a85b3..3d5897d 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -81,8 +81,7 @@ SHOW_FIELD
{ V4L2_BUF_FLAG_ERROR,   "ERROR" },   \
{ V4L2_BUF_FLAG_TIMECODE,"TIMECODE" },\
{ V4L2_BUF_FLAG_PREPARED,"PREPARED" },\
-   { V4L2_BUF_FLAG_NO_CACHE_INVALIDATE, "NO_CACHE_INVALIDATE" }, \
-   { V4L2_BUF_FLAG_NO_CACHE_CLEAN,  "NO_CACHE_CLEAN" },  \
+   { V4L2_BUF_FLAG_NO_CACHE_SYNC,   "NO_CACHE_SYNC" },   \
{ V4L2_BUF_FLAG_TIMESTAMP_MASK,  "TIMESTAMP_MASK" },  \
{ 

[PATCH v2] dw9714: Initial driver for dw9714 VCM

[Rajmohan Mani]

DW9714 is a 10 bit DAC, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Rajmohan Mani 
---
Changes in v2:
- Addressed review comments from Hans Verkuil
- Fixed a debug message typo
- Got rid of a return variable
---
 drivers/media/i2c/Kconfig  |   9 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9714.c | 320 +
 3 files changed, 330 insertions(+)
 create mode 100644 drivers/media/i2c/dw9714.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd181c9..4f0a7ad 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -300,6 +300,15 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_DW9714
+   tristate "DW9714 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   ---help---
+ This is a driver for the DW9714 camera lens voice coil.
+ DW9714 is a 10 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 62323ec..987bd1f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
new file mode 100644
index 000..cd6cde7
--- /dev/null
+++ b/drivers/media/i2c/dw9714.c
@@ -0,0 +1,320 @@
+/*
+ * Copyright (c) 2015--2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9714_NAME"dw9714"
+#define DW9714_MAX_FOCUS_POS   1023
+#define DW9714_CTRL_STEPS  16  /* Keep this value power of 2 */
+#define DW9714_CTRL_DELAY_US   1000
+/*
+ * S[3:2] = 0x00, codes per step for "Linear Slope Control"
+ * S[1:0] = 0x00, step period
+ */
+#define DW9714_DEFAULT_S 0x0
+#define DW9714_VAL(data, s) (u16)((data) << 4 | (s))
+
+/* dw9714 device structure */
+struct dw9714_device {
+   struct i2c_client *client;
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   u16 current_val;
+};
+
+#define to_dw9714_vcm(_ctrl)   \
+   container_of(_ctrl->handler, struct dw9714_device, ctrls_vcm)
+
+static int dw9714_i2c_write(struct i2c_client *client, u16 data)
+{
+   const int num_msg = 1;
+   int ret;
+   u16 val = cpu_to_be16(data);
+   struct i2c_msg msg = {
+   .addr = client->addr,
+   .flags = 0,
+   .len = sizeof(val),
+   .buf = (u8 *) ,
+   };
+
+   ret = i2c_transfer(client->adapter, , num_msg);
+
+   /*One retry */
+   if (ret != num_msg)
+   ret = i2c_transfer(client->adapter, , num_msg);
+
+   if (ret != num_msg) {
+   dev_err(>dev, "I2C write fail\n");
+   return -EIO;
+   }
+   return 0;
+}
+
+static int dw9714_t_focus_vcm(struct dw9714_device *dw9714_dev, u16 val)
+{
+   struct i2c_client *client = dw9714_dev->client;
+
+   dev_dbg(>dev, "Setting new value VCM: %d\n", val);
+   dw9714_dev->current_val = val;
+
+   return dw9714_i2c_write(client, DW9714_VAL(val, DW9714_DEFAULT_S));
+}
+
+static int dw9714_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct dw9714_device *dev_vcm = to_dw9714_vcm(ctrl);
+
+   if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+   return dw9714_t_focus_vcm(dev_vcm, ctrl->val);
+
+   return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops dw9714_vcm_ctrl_ops = {
+   .s_ctrl = dw9714_set_ctrl,
+};
+
+static int dw9714_init_controls(struct dw9714_device *dev_vcm)
+{
+   struct v4l2_ctrl_handler *hdl = _vcm->ctrls_vcm;
+   const struct v4l2_ctrl_ops *ops = _vcm_ctrl_ops;
+   struct i2c_client *client = dev_vcm->client;
+
+   

[RFC PATCH 2/2] media/uapi/v4l: clarify cropcap/crop/selection behavior

[Hans Verkuil]

From: Hans Verkuil 

Unfortunately the use of 'type' was inconsistent for multiplanar
buffer types. Starting with 4.12 both the normal and _MPLANE variants
are allowed, thus making it possible to write sensible code.

Yes, we messed up :-(

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/vidioc-cropcap.rst| 21 -
 Documentation/media/uapi/v4l/vidioc-g-crop.rst | 22 +-
 .../media/uapi/v4l/vidioc-g-selection.rst  | 22 --
 3 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-cropcap.rst 
b/Documentation/media/uapi/v4l/vidioc-cropcap.rst
index f21a69b554e1..d354216846e6 100644
--- a/Documentation/media/uapi/v4l/vidioc-cropcap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-cropcap.rst
@@ -39,16 +39,19 @@ structure. Drivers fill the rest of the structure. The 
results are
 constant except when switching the video standard. Remember this switch
 can occur implicit when switching the video input or output.
 
-Do not use the multiplanar buffer types. Use
-``V4L2_BUF_TYPE_VIDEO_CAPTURE`` instead of
-``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE`` and use
-``V4L2_BUF_TYPE_VIDEO_OUTPUT`` instead of
-``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
-
 This ioctl must be implemented for video capture or output devices that
 support cropping and/or scaling and/or have non-square pixels, and for
 overlay devices.
 
+.. note::
+   Unfortunately in the case of multiplanar buffer types
+   (``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE`` and 
``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``)
+   this API was messed up with regards to how the :c:type:`v4l2_cropcap` 
``type`` field
+   should be filled in. The Samsung Exynos drivers only accepted the
+   ``_MPLANE`` buffer type while other drivers only accepted a non-multiplanar
+   buffer type (i.e. without the ``_MPLANE`` at the end).
+
+   Starting with kernel 4.12 both variations are allowed.
 
 .. c:type:: v4l2_cropcap
 
@@ -62,9 +65,9 @@ overlay devices.
 * - __u32
   - ``type``
   - Type of the data stream, set by the application. Only these types
-   are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``,
-   ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` and
-   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type`.
+   are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``, 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``,
+   ``V4L2_BUF_TYPE_VIDEO_OUTPUT``, ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE`` 
and
+   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note above.
 * - struct :ref:`v4l2_rect `
   - ``bounds``
   - Defines the window within capturing or output is possible, this
diff --git a/Documentation/media/uapi/v4l/vidioc-g-crop.rst 
b/Documentation/media/uapi/v4l/vidioc-g-crop.rst
index 56a36340f565..8aabe33c8da7 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-crop.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-crop.rst
@@ -45,12 +45,6 @@ and struct :c:type:`v4l2_rect` substructure named ``c`` of a
 v4l2_crop structure and call the :ref:`VIDIOC_S_CROP ` ioctl 
with a pointer
 to this structure.
 
-Do not use the multiplanar buffer types. Use
-``V4L2_BUF_TYPE_VIDEO_CAPTURE`` instead of
-``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE`` and use
-``V4L2_BUF_TYPE_VIDEO_OUTPUT`` instead of
-``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
-
 The driver first adjusts the requested dimensions against hardware
 limits, i. e. the bounds given by the capture/output window, and it
 rounds to the closest possible values of horizontal and vertical offset,
@@ -74,6 +68,16 @@ been negotiated.
 When cropping is not supported then no parameters are changed and
 :ref:`VIDIOC_S_CROP ` returns the ``EINVAL`` error code.
 
+.. note::
+   Unfortunately in the case of multiplanar buffer types
+   (``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE`` and 
``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``)
+   this API was messed up with regards to how the :c:type:`v4l2_crop` ``type`` 
field
+   should be filled in. The Samsung Exynos drivers only accepted the
+   ``_MPLANE`` buffer type while other drivers only accepted a non-multiplanar
+   buffer type (i.e. without the ``_MPLANE`` at the end).
+
+   Starting with kernel 4.12 both variations are allowed.
+
 
 .. c:type:: v4l2_crop
 
@@ -87,9 +91,9 @@ When cropping is not supported then no parameters are changed 
and
 * - __u32
   - ``type``
   - Type of the data stream, set by the application. Only these types
-   are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``,
-   ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` and
-   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type`.
+   are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``, 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``,
+   ``V4L2_BUF_TYPE_VIDEO_OUTPUT``, ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE`` 
and
+   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note above.
 * - struct :c:type:`v4l2_rect`
   

[RFC PATCH 1/2] v4l2-ioctl/exynos: fix G/S_SELECTION's type handling

[Hans Verkuil]

From: Hans Verkuil 

The type field in struct v4l2_selection is supposed to never use the
_MPLANE variants. E.g. if the driver supports 
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
then userspace should still pass V4L2_BUF_TYPE_VIDEO_CAPTURE.

The reasons for this are lost in the mists of time, but it is really
annoying. In addition, the exynos drivers didn't follow this rule and
instead expected the _MPLANE type.

To fix that code is added to the v4l2 core that maps the _MPLANE buffer
types to their regular equivalents before calling the driver.

Effectively this allows for userspace to use either _MPLANE or the regular
buffer type. This keeps backwards compatibility while making things easier
for userspace.

Since drivers now never see the _MPLANE buffer types the exynos drivers
had to be adapted as well.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/exynos-gsc/gsc-core.c |  4 +-
 drivers/media/platform/exynos-gsc/gsc-m2m.c  |  8 ++--
 drivers/media/platform/exynos4-is/fimc-capture.c |  4 +-
 drivers/media/platform/exynos4-is/fimc-lite.c|  4 +-
 drivers/media/v4l2-core/v4l2-ioctl.c | 53 +---
 5 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 59a634201830..107faa04c947 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -569,9 +569,9 @@ int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
}
pr_debug("user put w: %d, h: %d", cr->c.width, cr->c.height);
 
-   if (cr->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (cr->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
f = >d_frame;
-   else if (cr->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+   else if (cr->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
f = >s_frame;
else
return -EINVAL;
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c 
b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 82505025d96c..33611a46ce35 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -460,8 +460,8 @@ static int gsc_m2m_g_selection(struct file *file, void *fh,
struct gsc_frame *frame;
struct gsc_ctx *ctx = fh_to_ctx(fh);
 
-   if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
-   (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
+   if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
+   (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
return -EINVAL;
 
frame = ctx_get_frame(ctx, s->type);
@@ -503,8 +503,8 @@ static int gsc_m2m_s_selection(struct file *file, void *fh,
cr.type = s->type;
cr.c = s->r;
 
-   if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
-   (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
+   if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
+   (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
return -EINVAL;
 
ret = gsc_try_crop(ctx, );
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c 
b/drivers/media/platform/exynos4-is/fimc-capture.c
index 8a7cd07dbe28..d876fc3e0ef7 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -1270,7 +1270,7 @@ static int fimc_cap_g_selection(struct file *file, void 
*fh,
struct fimc_ctx *ctx = fimc->vid_cap.ctx;
struct fimc_frame *f = >s_frame;
 
-   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
switch (s->target) {
@@ -1320,7 +1320,7 @@ static int fimc_cap_s_selection(struct file *file, void 
*fh,
struct fimc_frame *f;
unsigned long flags;
 
-   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
if (s->target == V4L2_SEL_TGT_COMPOSE)
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c 
b/drivers/media/platform/exynos4-is/fimc-lite.c
index b4c4a33784c4..7d3ec5cc6608 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -901,7 +901,7 @@ static int fimc_lite_g_selection(struct file *file, void 
*fh,
struct fimc_lite *fimc = video_drvdata(file);
struct flite_frame *f = >out_frame;
 
-   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
switch (sel->target) {
@@ -929,7 +929,7 @@ static int fimc_lite_s_selection(struct file *file, void 
*fh,
struct v4l2_rect rect = sel->r;
unsigned long flags;
 
-   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
+   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||

Re: [PATCH 4/4] dma-buf: Use seq_putc() in two functions

[Gustavo Padovan]

Hi Markus,

Thank for your patches.

2017-05-08 SF Markus Elfring :

> From: Markus Elfring 
> Date: Mon, 8 May 2017 10:55:42 +0200
> 
> Three single characters (line breaks) should be put into a sequence.
> Thus use the corresponding function "seq_putc".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/dma-buf/sync_debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Gustavo Padovan 

Gustavo

Re: [PATCH 3/4] dma-buf: Adjust a null pointer check in dma_buf_attach()

[Gustavo Padovan]

2017-05-08 SF Markus Elfring :

> From: Markus Elfring 
> Date: Mon, 8 May 2017 10:54:17 +0200
> 
> The script "checkpatch.pl" pointed information out like the following.
> 
> Comparison to NULL could be written "!attach"
> 
> Thus adjust this expression.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/dma-buf/dma-buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Gustavo Padovan 

Gustavo

Re: [PATCH 2/4] dma-buf: Improve a size determination in dma_buf_attach()

[Gustavo Padovan]

2017-05-08 SF Markus Elfring :

> From: Markus Elfring 
> Date: Mon, 8 May 2017 10:50:09 +0200
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/dma-buf/dma-buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Gustavo Padovan 

Gustavo
 

Re: [PATCH 1/4] dma-buf: Combine two function calls into one in dma_buf_debug_show()

[Gustavo Padovan]

2017-05-08 SF Markus Elfring :

> From: Markus Elfring 
> Date: Mon, 8 May 2017 10:32:44 +0200
> 
> A bit of data was put into a sequence by two separate function calls.
> Print the same data by a single function call instead.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/dma-buf/dma-buf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Gustavo Padovan 

Gustavo

RE: [PATCH] dw9714: Initial driver for dw9714 VCM

[Mani, Rajmohan]

Hi Hans,

Thanks for the quick review. Please see my comments inline...
I will post the new v2 patch soon.

> -Original Message-
> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> Sent: Monday, May 08, 2017 6:59 PM
> To: Mani, Rajmohan ; linux-
> me...@vger.kernel.org; mche...@kernel.org
> Subject: Re: [PATCH] dw9714: Initial driver for dw9714 VCM
> 
> Hi Rajmohan,
> 
> Thanks for the patch!
> 
> A quick code review:
> 
> On 05/07/2017 01:33 PM, rajmohan.m...@intel.com wrote:
> > From: Rajmohan Mani 
> >
> > DW9714 is a 10 bit DAC, designed for linear control of voice coil
> > motor.
> >
> > This driver creates a V4L2 subdevice and provides control to set the
> > desired focus.
> >
> > Signed-off-by: Rajmohan Mani 
> > ---
> >  drivers/media/i2c/Kconfig  |   9 ++
> >  drivers/media/i2c/Makefile |   1 +
> >  drivers/media/i2c/dw9714.c | 333
> > +
> >  3 files changed, 343 insertions(+)
> >  create mode 100644 drivers/media/i2c/dw9714.c
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index fd181c9..4f0a7ad 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -300,6 +300,15 @@ config VIDEO_AD5820
> >   This is a driver for the AD5820 camera lens voice coil.
> >   It is used for example in Nokia N900 (RX-51).
> >
> > +config VIDEO_DW9714
> > +   tristate "DW9714 lens voice coil support"
> > +   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> > +   ---help---
> > + This is a driver for the DW9714 camera lens voice coil.
> > + DW9714 is a 10 bit DAC with 120mA output current sink
> > + capability. This is designed for linear control of
> > + voice coil motors, controlled via I2C serial interface.
> > +
> >  config VIDEO_SAA7110
> > tristate "Philips SAA7110 video decoder"
> > depends on VIDEO_V4L2 && I2C
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index 62323ec..987bd1f 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
> >  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
> >  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
> >  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
> > +obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
> >  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> >  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> >  obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o diff --git
> > a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c new file
> > mode 100644 index 000..276d3f2
> > --- /dev/null
> > +++ b/drivers/media/i2c/dw9714.c
> > @@ -0,0 +1,333 @@
> > +/*
> > + * Copyright (c) 2015--2017 Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > +version
> > + * 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define DW9714_NAME"dw9714"
> > +#define DW9714_MAX_FOCUS_POS   1023
> > +#define DW9714_CTRL_STEPS  16  /* Keep this value power of 2
> */
> > +#define DW9714_CTRL_DELAY_US   1000
> > +/*
> > + * S[3:2] = 0x00, codes per step for "Linear Slope Control"
> > + * S[1:0] = 0x00, step period
> > + */
> > +#define DW9714_DEFAULT_S 0x0
> > +#define DW9714_VAL(data, s) (u16)((data) << 4 | (s))
> > +
> > +/* dw9714 device structure */
> > +struct dw9714_device {
> > +   struct i2c_client *client;
> > +   struct v4l2_ctrl_handler ctrls_vcm;
> > +   struct v4l2_subdev sd;
> > +   u16 current_val;
> > +};
> > +
> > +#define to_dw9714_vcm(_ctrl)   \
> > +   container_of(_ctrl->handler, struct dw9714_device, ctrls_vcm)
> > +
> > +static int dw9714_i2c_write(struct i2c_client *client, u16 data) {
> > +   const int num_msg = 1;
> > +   int ret;
> > +   u16 val = cpu_to_be16(data);
> > +   struct i2c_msg msg = {
> > +   .addr = client->addr,
> > +   .flags = 0,
> > +   .len = sizeof(val),
> > +   .buf = (u8 *) ,
> > +   };
> > +
> > +   ret = i2c_transfer(client->adapter, , num_msg);
> > +
> > +   /*One retry */
> > +   if (ret != num_msg)
> > +   ret = i2c_transfer(client->adapter, , num_msg);
> > +
> > +   if (ret != num_msg) {
> > +   dev_err(>dev, "I2C write fail fail\n");
> > +   return -EIO;
> > +   } else {
> 
> No need for the 'else' here, since the 'if' already returns.

Ack
I also removed the redundant " fail" in the dev_err() message.

> 
> > +   return 0;
> > +   }
> > +}
> > +
> > 

Re: [PATCH v4 1/2] [media] dt-bindings: Add bindings for video-multiplexer device

[Rob Herring]

On Thu, May 04, 2017 at 05:17:18PM +0200, Philipp Zabel wrote:
> Add bindings documentation for the video multiplexer device.
> 
> Signed-off-by: Sascha Hauer 
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Steve Longerbeam 
> Acked-by: Sakari Ailus 
> Reviewed-by: Sebastian Reichel 
> ---
> No changes since v3 [1].
> 
> This was previously sent as part of Steve's i.MX media series [2].
> 
> [1] https://patchwork.kernel.org/patch/9711997/
> [2] https://patchwork.kernel.org/patch/9647951/
> ---
>  .../devicetree/bindings/media/video-mux.txt| 60 
> ++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/video-mux.txt

Acked-by: Rob Herring 

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Hans Verkuil]

On 05/08/2017 02:56 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, May 08, 2017 at 02:32:22PM +0200, Hans Verkuil wrote:
>> On 05/08/2017 02:10 PM, Sakari Ailus wrote:
> 
> ...
> 
> +/* .complete() is called after all subdevices have been located */
> +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
> + notifier);
> + struct sensor_async_subdev *s_asd;
> + struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
> + struct cio2_queue *q;
> + struct fwnode_endpoint remote_endpt;
> + int i, ret;
> +
> + for (i = 0; i < notifier->num_subdevs; i++) {
> + s_asd = container_of(cio2->notifier.subdevs[i],
> + struct sensor_async_subdev,
> + asd);
> +
> + fwn_remote = s_asd->asd.match.fwnode.fwn;
> + fwn_endpt = (struct fwnode_handle *)
> + s_asd->vfwn_endpt.base.local_fwnode;
> + fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
> + if (!fwn_remote_endpt) {
> + dev_err(>pci_dev->dev,
> + "failed to get remote endpt %d\n", ret);
> + return ret;
> + }
> +
> + ret = fwnode_graph_parse_endpoint(fwn_remote_endpt,
> + _endpt);
> + if (ret) {
> + dev_err(>pci_dev->dev,
> + "failed to parse remote endpt %d\n", ret);
> + return ret;
> + }
> +
> + q = cio2_find_queue_by_sensor_node(cio2->queue, fwn_remote);
> + if (!q) {
> + dev_err(>pci_dev->dev,
> + "failed to find cio2 queue %d\n", ret);
> + return ret;
> + }
> +
> + ret = media_create_pad_link(
> + >sensor->entity, remote_endpt.id,
> + >subdev.entity, s_asd->vfwn_endpt.base.id,
> + 0);
> + if (ret) {
> + dev_err(>pci_dev->dev,
> + "failed to create link for %s\n",
> + cio2->queue[i].sensor->name);
> + return ret;
> + }
> + }
> +
> + return v4l2_device_register_subdev_nodes(>v4l2_dev);
> +}

 The current code only supports sensor subdevs, right? Not non-sensor 
 subdevs such as
 for focus etc. control (e.g. voice coil).
>>>
>>> The CIO2 driver only supports sensors (I presume you could attach e.g. a TV
>>> tuner, too) but then again I don't think it's its job to support voice coils
>>> either: they're related to the sensors instead and the CIO2 driver wouldn't
>>> have enough information on them to associate them to a particular sensor.
>>>
>>> There are DT binding patches for such devices + async sub-device notifier
>>> patches floating around to support these.
>>
>> Can you give me a pointer to those?
> 
> Certainly. Lens etc. DT bindings:
> 
> 
> 
> And async sub-device notifier support:
> 
> 
> 
> ...
> 
> +
> + q->pixelformat = V4L2_PIX_FMT_IPU3_SRGGB10;
> +
> + /* Initialize fbpt */
> + r = cio2_fbpt_init(cio2, q);
> + if (r)
> + goto fail_fbpt;
> +
> + /* Initialize media entities */
> + r = media_entity_pads_init(>entity, CIO2_PADS, q->subdev_pads);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed initialize subdev media entity (%d)\n", r);
> + goto fail_subdev_media_entity;
> + }
> + q->subdev_pads[CIO2_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
> + MEDIA_PAD_FL_MUST_CONNECT;
> + q->subdev_pads[CIO2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> + subdev->entity.ops = _media_ops;
> + r = media_entity_pads_init(>entity, 1, >vdev_pad);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed initialize videodev media entity (%d)\n", r);
> + goto fail_vdev_media_entity;
> + }
> + q->vdev_pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> + vdev->entity.ops = _media_ops;
> +
> + /* Initialize subdev */
> + v4l2_subdev_init(subdev, _subdev_ops);
> + subdev->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + subdev->owner = THIS_MODULE;
> + snprintf(subdev->name, sizeof(subdev->name),
> +  CIO2_ENTITY_NAME ":%li", q - cio2->queue);
> + v4l2_set_subdevdata(subdev, cio2);
> + r = 

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Hans,

On Mon, May 08, 2017 at 02:32:22PM +0200, Hans Verkuil wrote:
> On 05/08/2017 02:10 PM, Sakari Ailus wrote:

...

> >>> +/* .complete() is called after all subdevices have been located */
> >>> +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> >>> +{
> >>> + struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
> >>> + notifier);
> >>> + struct sensor_async_subdev *s_asd;
> >>> + struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
> >>> + struct cio2_queue *q;
> >>> + struct fwnode_endpoint remote_endpt;
> >>> + int i, ret;
> >>> +
> >>> + for (i = 0; i < notifier->num_subdevs; i++) {
> >>> + s_asd = container_of(cio2->notifier.subdevs[i],
> >>> + struct sensor_async_subdev,
> >>> + asd);
> >>> +
> >>> + fwn_remote = s_asd->asd.match.fwnode.fwn;
> >>> + fwn_endpt = (struct fwnode_handle *)
> >>> + s_asd->vfwn_endpt.base.local_fwnode;
> >>> + fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
> >>> + if (!fwn_remote_endpt) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed to get remote endpt %d\n", ret);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + ret = fwnode_graph_parse_endpoint(fwn_remote_endpt,
> >>> + _endpt);
> >>> + if (ret) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed to parse remote endpt %d\n", ret);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + q = cio2_find_queue_by_sensor_node(cio2->queue, fwn_remote);
> >>> + if (!q) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed to find cio2 queue %d\n", ret);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + ret = media_create_pad_link(
> >>> + >sensor->entity, remote_endpt.id,
> >>> + >subdev.entity, s_asd->vfwn_endpt.base.id,
> >>> + 0);
> >>> + if (ret) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed to create link for %s\n",
> >>> + cio2->queue[i].sensor->name);
> >>> + return ret;
> >>> + }
> >>> + }
> >>> +
> >>> + return v4l2_device_register_subdev_nodes(>v4l2_dev);
> >>> +}
> >>
> >> The current code only supports sensor subdevs, right? Not non-sensor 
> >> subdevs such as
> >> for focus etc. control (e.g. voice coil).
> > 
> > The CIO2 driver only supports sensors (I presume you could attach e.g. a TV
> > tuner, too) but then again I don't think it's its job to support voice coils
> > either: they're related to the sensors instead and the CIO2 driver wouldn't
> > have enough information on them to associate them to a particular sensor.
> > 
> > There are DT binding patches for such devices + async sub-device notifier
> > patches floating around to support these.
> 
> Can you give me a pointer to those?

Certainly. Lens etc. DT bindings:



And async sub-device notifier support:



...

> >>> +
> >>> + q->pixelformat = V4L2_PIX_FMT_IPU3_SRGGB10;
> >>> +
> >>> + /* Initialize fbpt */
> >>> + r = cio2_fbpt_init(cio2, q);
> >>> + if (r)
> >>> + goto fail_fbpt;
> >>> +
> >>> + /* Initialize media entities */
> >>> + r = media_entity_pads_init(>entity, CIO2_PADS, q->subdev_pads);
> >>> + if (r) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed initialize subdev media entity (%d)\n", r);
> >>> + goto fail_subdev_media_entity;
> >>> + }
> >>> + q->subdev_pads[CIO2_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
> >>> + MEDIA_PAD_FL_MUST_CONNECT;
> >>> + q->subdev_pads[CIO2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> >>> + subdev->entity.ops = _media_ops;
> >>> + r = media_entity_pads_init(>entity, 1, >vdev_pad);
> >>> + if (r) {
> >>> + dev_err(>pci_dev->dev,
> >>> + "failed initialize videodev media entity (%d)\n", r);
> >>> + goto fail_vdev_media_entity;
> >>> + }
> >>> + q->vdev_pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> >>> + vdev->entity.ops = _media_ops;
> >>> +
> >>> + /* Initialize subdev */
> >>> + v4l2_subdev_init(subdev, _subdev_ops);
> >>> + subdev->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> >>> + subdev->owner = THIS_MODULE;
> >>> + snprintf(subdev->name, sizeof(subdev->name),
> >>> +  CIO2_ENTITY_NAME ":%li", q - cio2->queue);
> >>> + v4l2_set_subdevdata(subdev, cio2);
> >>> + r = v4l2_device_register_subdev(>v4l2_dev, subdev);
> >>> + if (r) {
> >>> + dev_err(>pci_dev->dev,
> 

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Hans Verkuil]

On 05/08/2017 02:10 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, May 08, 2017 at 11:06:29AM +0200, Hans Verkuil wrote:
> ...
>>> +static void cio2_queue_event_sof(struct cio2_device *cio2, struct 
>>> cio2_queue *q)
>>> +{
>>> +   struct v4l2_event event = {
>>> +   .type = V4L2_EVENT_FRAME_SYNC,
>>> +   .u.frame_sync.frame_sequence =
>>> +   atomic_inc_return(>frame_sequence) - 1,
>>> +   };
>>> +
>>> +   v4l2_event_queue(q->subdev.devnode, );
>>
>> Out of curiosity: why do you need this event? I recommend that you document 
>> the
>> reasons for having this event somewhere.
> 
> For the user space camera control algorithms it is essential to know when
> the reception of a frame has begun. That's often the best timing information
> you get from the hardware, and not specific to this device --- the omap3isp
> driver does the same.

OK. Just document this somewhere in the driver. It's helpful background info.

> 
> ...
> 
>>> +static const struct v4l2_file_operations cio2_v4l2_fops = {
>>> +   .owner = THIS_MODULE,
>>> +   .unlocked_ioctl = video_ioctl2,
>>> +   .open = v4l2_fh_open,
>>> +   .release = vb2_fop_release,
>>> +   .poll = vb2_fop_poll,
>>> +   .mmap = vb2_fop_mmap,
>>
>> I suggest adding .read = vb2_fop_read as well. It's for free, and I never 
>> see any
>> reason not to do this (although opinions differ on that :-) ).
> 
> I wonder if any real applications use it. A number of drivers have never
> supported it either and frankly I'd be happy to rather see it disappear.
> Complexity is one of the biggest problems also in videobuf2.

Applications are unlikely to use it, but I use it occasionally when debugging,
just to get some raw video quickly. All the support is there, so it is all for
free. But it is up to you.

> 
> The support can be added later on but it cannot be reasonably removed going
> forward.
> 
>>
>>> +};
>>> +
>>> +static const struct v4l2_ioctl_ops cio2_v4l2_ioctl_ops = {
>>> +   .vidioc_querycap = cio2_v4l2_querycap,
>>> +   .vidioc_enum_fmt_vid_cap = cio2_v4l2_enum_fmt,
>>> +   .vidioc_g_fmt_vid_cap = cio2_v4l2_g_fmt,
>>> +   .vidioc_s_fmt_vid_cap = cio2_v4l2_s_fmt,
>>> +   .vidioc_try_fmt_vid_cap = cio2_v4l2_try_fmt,
>>> +   .vidioc_reqbufs = vb2_ioctl_reqbufs,
>>> +   .vidioc_create_bufs = vb2_ioctl_create_bufs,
>>> +   .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
>>> +   .vidioc_querybuf = vb2_ioctl_querybuf,
>>> +   .vidioc_qbuf = vb2_ioctl_qbuf,
>>> +   .vidioc_dqbuf = vb2_ioctl_dqbuf,
>>> +   .vidioc_streamon = vb2_ioctl_streamon,
>>> +   .vidioc_streamoff = vb2_ioctl_streamoff,
>>> +   .vidioc_expbuf = vb2_ioctl_expbuf,
>>> +};
>>> +
>>> +static int cio2_subdev_subscribe_event(struct v4l2_subdev *sd,
>>> +  struct v4l2_fh *fh,
>>> +  struct v4l2_event_subscription *sub)
>>> +{
>>> +   if (sub->type != V4L2_EVENT_FRAME_SYNC)
>>> +   return -EINVAL;
>>
>> You must also support V4L2_EVENT_CTRL.
>>
>>> +
>>> +   /* Line number. For now only zero accepted. */
>>> +   if (sub->id != 0)
>>> +   return -EINVAL;
>>> +
>>> +   return v4l2_event_subscribe(fh, sub, 0, NULL);
>>
>> You support room in the event queue for only one V4L2_EVENT_FRAME_SYNC 
>> event. Is
>> that what you want? If userspace can't keep up you will lose older 
>> frame_sync events.
>>
>> It's probably OK for this event, but I want to make sure you thought about 
>> this :-)
> 
> I think I'd like to have as many events as there can be buffers. I have to
> say I don't write much user space software, but I presume it'll be extra
> work to prepare for one more not-seen-often special case.

Given the use-case you have to wonder how useful it is to retain old events.
I leave it up to you.

> 
> ...
> 
>>> +/* .complete() is called after all subdevices have been located */
>>> +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
>>> +{
>>> +   struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
>>> +   notifier);
>>> +   struct sensor_async_subdev *s_asd;
>>> +   struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
>>> +   struct cio2_queue *q;
>>> +   struct fwnode_endpoint remote_endpt;
>>> +   int i, ret;
>>> +
>>> +   for (i = 0; i < notifier->num_subdevs; i++) {
>>> +   s_asd = container_of(cio2->notifier.subdevs[i],
>>> +   struct sensor_async_subdev,
>>> +   asd);
>>> +
>>> +   fwn_remote = s_asd->asd.match.fwnode.fwn;
>>> +   fwn_endpt = (struct fwnode_handle *)
>>> +   s_asd->vfwn_endpt.base.local_fwnode;
>>> +   fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
>>> +   if (!fwn_remote_endpt) {
>>> +   dev_err(>pci_dev->dev,
>>> +   "failed to get remote endpt %d\n", ret);
>>> +   return ret;

Re: [PATCH v2 0/7] [media]: vimc: Virtual Media Control VPU's

[Hans Verkuil]

Hi Helen,

On 04/08/2017 12:37 AM, Helen Koike wrote:
> This patch series improves the current video processing units in vimc
> (by adding more controls to the sensor and capture node, allowing the
> user to configure different frame formats) and also adds a debayer
> and a scaler node.
> The debayer transforms the bayer format image received in its sink pad
> to a bayer format by averaging the pixels within a mean window.
> The scaler only scales up the image for now.
> 
> This patch series depends on commit "[media] vimc: Virtual Media Controller 
> core, capture and sensor"
> and it is available at:
> https://github.com/helen-fornazier/opw-staging/tree/z/sent/vimc/vpu/v2

As you saw, I had some comments, but nothing major.

One thing I was wondering about: the since the sensor is using the tpg anyway,
we can in theory use the tpg as well to generate the output of the debayer
and scaler blocks.

It won't be quite the same since it is not actually processing the frame, but
it is efficient. It might need additional work in the tpg though. I'm not sure
how worthwhile this is, and it probably shouldn't be done now. But it is an
idea since I suspect doing a debayer + scaling of a 1920x1080 picture will be
very slow.

Regards,

Hans

> 
> Changes in v2:
> [media] vimc: sen: Integrate the tpg on the sensor
>   - Fix include location
>   - Select V4L2_TPG in Kconfig
>   - configure tpg on streamon only
>   - rm BUG_ON
>   - coding style
> [media] vimc: Add vimc_ent_sd_* helper functions
>   - Comments in vimc_ent_sd_init
>   - Update vimc_ent_sd_init with upstream code as media_entity_pads_init
>   (instead of media_entity_init), entity->function intead of entity->type
>   - Add missing vimc_pads_cleanup in vimc_ent_sd_cleanup
>   - remove subdevice v4l2_dev and dev fields
>   - change unregister order in vimc_ent_sd_cleanup
>   - rename vimc_ent_sd_{init,cleanup} to vimc_ent_sd_{register,unregister}
>   - remove struct vimc_ent_subdevice, use ved and sd directly
>   - don't impose struct vimc_sen_device to declare ved and sd struct first
>   - add kernel docs
> [media] vimc: Add vimc_pipeline_s_stream in the core
>   - Use is_media_entity_v4l2_subdev instead of comparing with the old
>   entity->type
>   - Fix comments style
>   - add kernel-docs
>   - call s_stream across all sink pads
> [media] vimc: sen: Support several image formats
>   - this is a new commit in the serie (the old one was splitted in two)
>   - add init_cfg to initialize try_fmt
>   - reorder code in vimc_sen_set_fmt
>   - allow user space to change all fields from struct v4l2_mbus_framefmt
> (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
>   - merge with patch for the enum_mbus_code and enum_frame_size
>   - change commit message
>   - add vimc_pix_map_by_index
>   - rename MIN/MAX macros
>   - check set_fmt default parameters for quantization, colorspace ...
> [media] vimc: cap: Support several image formats
>   - this is a new commit in the serie (the old one was splitted in two)
>   - allow user space to change all fields from struct v4l2_pix_format
> (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
>   - link_validate and try_fmt: also checks colospace, quantization, 
> field, xfer_func, ycbcr_enc
>   - add struct v4l2_pix_format fmt_default
>   - add enum_framesizes
>   - enum_fmt_vid_cap: enumerate all formats from vimc_pix_map_table
>   - add mode dev_dbg
> [media] vimc: deb: Add debayer filter
>   - Using MEDIA_ENT_F_ATV_DECODER in function
>   - remove v4l2_dev and dev from vimc_deb_device struct
>   - src fmt propagates from the sink
>   - coding style
>   - remove redundant else if statements
>   - check end of enum and remove BUG_ON
>   - enum frame size with min and max values
>   - set/try fmt
>   - remove unecessary include freezer.h
>   - check pad types on create
>   - return EBUSY when trying to set the format while stream is on
>   - remove vsd struct
>   - add IS_SRC and IS_SINK macros
>   - add deb_mean_win_size as a parameter of the module
>   - check set_fmt default parameters for quantization, colorspace ...
>   - add more dev_dbg
> [media] vimc: sca: Add scaler
>   - Add function MEDIA_ENT_F_IO_V4L
>   - remove v4l2_dev and dev
>   - s/sink_mbus_fmt/sink_fmt
>   - remove BUG_ON, remove redundant if else, rewrite TODO, check end of 
> enum
>   - rm src_width/height, enum fsize with min and max values
>   - set/try fmt
>   - remove unecessary include freezer.h
>   - core: add bayer boolean in pixel table
>   - coding style
>   - fix bug in enum frame size
>   - check pad types on create
>   - return EBUSY when trying to set the format while stream is on
>   - remove vsd struct
>   - add IS_SRC and IS_SINK 

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Hans,

On Mon, May 08, 2017 at 11:06:29AM +0200, Hans Verkuil wrote:
...
> > +static void cio2_queue_event_sof(struct cio2_device *cio2, struct 
> > cio2_queue *q)
> > +{
> > +   struct v4l2_event event = {
> > +   .type = V4L2_EVENT_FRAME_SYNC,
> > +   .u.frame_sync.frame_sequence =
> > +   atomic_inc_return(>frame_sequence) - 1,
> > +   };
> > +
> > +   v4l2_event_queue(q->subdev.devnode, );
> 
> Out of curiosity: why do you need this event? I recommend that you document 
> the
> reasons for having this event somewhere.

For the user space camera control algorithms it is essential to know when
the reception of a frame has begun. That's often the best timing information
you get from the hardware, and not specific to this device --- the omap3isp
driver does the same.

...

> > +static const struct v4l2_file_operations cio2_v4l2_fops = {
> > +   .owner = THIS_MODULE,
> > +   .unlocked_ioctl = video_ioctl2,
> > +   .open = v4l2_fh_open,
> > +   .release = vb2_fop_release,
> > +   .poll = vb2_fop_poll,
> > +   .mmap = vb2_fop_mmap,
> 
> I suggest adding .read = vb2_fop_read as well. It's for free, and I never see 
> any
> reason not to do this (although opinions differ on that :-) ).

I wonder if any real applications use it. A number of drivers have never
supported it either and frankly I'd be happy to rather see it disappear.
Complexity is one of the biggest problems also in videobuf2.

The support can be added later on but it cannot be reasonably removed going
forward.

> 
> > +};
> > +
> > +static const struct v4l2_ioctl_ops cio2_v4l2_ioctl_ops = {
> > +   .vidioc_querycap = cio2_v4l2_querycap,
> > +   .vidioc_enum_fmt_vid_cap = cio2_v4l2_enum_fmt,
> > +   .vidioc_g_fmt_vid_cap = cio2_v4l2_g_fmt,
> > +   .vidioc_s_fmt_vid_cap = cio2_v4l2_s_fmt,
> > +   .vidioc_try_fmt_vid_cap = cio2_v4l2_try_fmt,
> > +   .vidioc_reqbufs = vb2_ioctl_reqbufs,
> > +   .vidioc_create_bufs = vb2_ioctl_create_bufs,
> > +   .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> > +   .vidioc_querybuf = vb2_ioctl_querybuf,
> > +   .vidioc_qbuf = vb2_ioctl_qbuf,
> > +   .vidioc_dqbuf = vb2_ioctl_dqbuf,
> > +   .vidioc_streamon = vb2_ioctl_streamon,
> > +   .vidioc_streamoff = vb2_ioctl_streamoff,
> > +   .vidioc_expbuf = vb2_ioctl_expbuf,
> > +};
> > +
> > +static int cio2_subdev_subscribe_event(struct v4l2_subdev *sd,
> > +  struct v4l2_fh *fh,
> > +  struct v4l2_event_subscription *sub)
> > +{
> > +   if (sub->type != V4L2_EVENT_FRAME_SYNC)
> > +   return -EINVAL;
> 
> You must also support V4L2_EVENT_CTRL.
> 
> > +
> > +   /* Line number. For now only zero accepted. */
> > +   if (sub->id != 0)
> > +   return -EINVAL;
> > +
> > +   return v4l2_event_subscribe(fh, sub, 0, NULL);
> 
> You support room in the event queue for only one V4L2_EVENT_FRAME_SYNC event. 
> Is
> that what you want? If userspace can't keep up you will lose older frame_sync 
> events.
> 
> It's probably OK for this event, but I want to make sure you thought about 
> this :-)

I think I'd like to have as many events as there can be buffers. I have to
say I don't write much user space software, but I presume it'll be extra
work to prepare for one more not-seen-often special case.

...

> > +/* .complete() is called after all subdevices have been located */
> > +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> > +{
> > +   struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
> > +   notifier);
> > +   struct sensor_async_subdev *s_asd;
> > +   struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
> > +   struct cio2_queue *q;
> > +   struct fwnode_endpoint remote_endpt;
> > +   int i, ret;
> > +
> > +   for (i = 0; i < notifier->num_subdevs; i++) {
> > +   s_asd = container_of(cio2->notifier.subdevs[i],
> > +   struct sensor_async_subdev,
> > +   asd);
> > +
> > +   fwn_remote = s_asd->asd.match.fwnode.fwn;
> > +   fwn_endpt = (struct fwnode_handle *)
> > +   s_asd->vfwn_endpt.base.local_fwnode;
> > +   fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
> > +   if (!fwn_remote_endpt) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to get remote endpt %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = fwnode_graph_parse_endpoint(fwn_remote_endpt,
> > +   _endpt);
> > +   if (ret) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to parse remote endpt %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   q = cio2_find_queue_by_sensor_node(cio2->queue, fwn_remote);
> > +   if (!q) {
> > +   

Re: [PATCH v2 6/7] [media] vimc: deb: Add debayer filter

[Hans Verkuil]

On 04/08/2017 12:37 AM, Helen Koike wrote:
> Implement the debayer filter and integrate it with the core
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Changes in v2:
> [media] vimc: deb: Add debayer filter
>   - Using MEDIA_ENT_F_ATV_DECODER in function
>   - remove v4l2_dev and dev from vimc_deb_device struct
>   - src fmt propagates from the sink
>   - coding style
>   - remove redundant else if statements
>   - check end of enum and remove BUG_ON
>   - enum frame size with min and max values
>   - set/try fmt
>   - remove unecessary include freezer.h
>   - check pad types on create
>   - return EBUSY when trying to set the format while stream is on
>   - remove vsd struct
>   - add IS_SRC and IS_SINK macros
>   - add deb_mean_win_size as a parameter of the module
>   - check set_fmt default parameters for quantization, colorspace ...
>   - add more dev_dbg
> 
> 
> ---
>  drivers/media/platform/vimc/Makefile   |   2 +-
>  drivers/media/platform/vimc/vimc-core.c|   6 +-
>  drivers/media/platform/vimc/vimc-core.h|   2 +
>  drivers/media/platform/vimc/vimc-debayer.c | 573 
> +
>  drivers/media/platform/vimc/vimc-debayer.h |  28 ++
>  5 files changed, 609 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/media/platform/vimc/vimc-debayer.c
>  create mode 100644 drivers/media/platform/vimc/vimc-debayer.h
> 
> diff --git a/drivers/media/platform/vimc/Makefile 
> b/drivers/media/platform/vimc/Makefile
> index c45195e..a6708f9 100644
> --- a/drivers/media/platform/vimc/Makefile
> +++ b/drivers/media/platform/vimc/Makefile
> @@ -1,3 +1,3 @@
> -vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
> +vimc-objs := vimc-core.o vimc-capture.o vimc-debayer.o vimc-sensor.o
>  
>  obj-$(CONFIG_VIDEO_VIMC) += vimc.o
> diff --git a/drivers/media/platform/vimc/vimc-core.c 
> b/drivers/media/platform/vimc/vimc-core.c
> index bc4b1bb..51cbbf6 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -23,6 +23,7 @@
>  
>  #include "vimc-capture.h"
>  #include "vimc-core.h"
> +#include "vimc-debayer.h"
>  #include "vimc-sensor.h"
>  
>  #define VIMC_PDEV_NAME "vimc"
> @@ -637,9 +638,12 @@ static int vimc_device_register(struct vimc_device *vimc)
>   create_func = vimc_cap_create;
>   break;
>  
> + case VIMC_ENT_NODE_DEBAYER:
> + create_func = vimc_deb_create;
> + break;
> +
>   /* TODO: Instantiate the specific topology node */
>   case VIMC_ENT_NODE_INPUT:
> - case VIMC_ENT_NODE_DEBAYER:
>   case VIMC_ENT_NODE_SCALER:
>   default:
>   /*
> diff --git a/drivers/media/platform/vimc/vimc-core.h 
> b/drivers/media/platform/vimc/vimc-core.h
> index 2146672..2e621fe 100644
> --- a/drivers/media/platform/vimc/vimc-core.h
> +++ b/drivers/media/platform/vimc/vimc-core.h
> @@ -26,6 +26,8 @@
>  #define VIMC_FRAME_MIN_WIDTH 16
>  #define VIMC_FRAME_MIN_HEIGHT 16
>  
> +#define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp)
> +
>  /**
>   * struct vimc_pix_map - maps media bus code with v4l2 pixel format
>   *
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c 
> b/drivers/media/platform/vimc/vimc-debayer.c
> new file mode 100644
> index 000..24e5952
> --- /dev/null
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -0,0 +1,573 @@
> +/*
> + * vimc-debayer.c Virtual Media Controller Driver
> + *
> + * Copyright (C) 2015-2017 Helen Koike 
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "vimc-debayer.h"
> +
> +static unsigned int deb_mean_win_size = 3;
> +module_param(deb_mean_win_size, uint, );
> +MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the 
> mean.\n"
> + "NOTE: the window size need to be an odd number, as the main pixel "
> + "stays in the center of the window, otherwise the next odd number "
> + "is considered");
> +
> +#define IS_SINK(pad) (!pad)
> +#define IS_SRC(pad)  (pad)
> +
> +enum vimc_deb_rgb_colors {
> + VIMC_DEB_RED = 0,
> + VIMC_DEB_GREEN = 1,
> + VIMC_DEB_BLUE = 2,
> +};
> +
> +struct vimc_deb_pix_map {
> + u32 code;
> + enum vimc_deb_rgb_colors order[2][2];
> +};
> +
> +struct 

Re: [PATCH v2 5/7] [media] vimc: cap: Support several image formats

[Hans Verkuil]

On 04/08/2017 12:37 AM, Helen Koike wrote:
> Allow user space to change the image format as the frame size, the
> pixel format, colorspace, quantization, field YCbCr encoding
> and the transfer function
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Changes in v2:
> [media] vimc: cap: Support several image formats
>   - this is a new commit in the serie (the old one was splitted in two)
>   - allow user space to change all fields from struct v4l2_pix_format
> (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
>   - link_validate and try_fmt: also checks colospace, quantization, 
> field, xfer_func, ycbcr_enc
>   - add struct v4l2_pix_format fmt_default
>   - add enum_framesizes
>   - enum_fmt_vid_cap: enumerate all formats from vimc_pix_map_table
>   - add mode dev_dbg
> 
> 
> ---
>  drivers/media/platform/vimc/vimc-capture.c | 167 
> -
>  1 file changed, 139 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c 
> b/drivers/media/platform/vimc/vimc-capture.c
> index 93f6a09..a6441f7 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -40,6 +40,16 @@ struct vimc_cap_device {
>   struct media_pipeline pipe;
>  };
>  
> +static const struct v4l2_pix_format fmt_default = {
> + .width = 640,
> + .height = 480,
> + .pixelformat = V4L2_PIX_FMT_RGB24,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .quantization = V4L2_QUANTIZATION_FULL_RANGE,
> + .xfer_func = V4L2_XFER_FUNC_SRGB,

I actually think we should keep .quantization and .xfer_func to 0 (DEFAULT).
It's what most drivers will do. Same for the previous patch (I didn't mention
it there).

> +};
> +
>  struct vimc_cap_buffer {
>   /*
>* struct vb2_v4l2_buffer must be the first element
> @@ -64,7 +74,7 @@ static int vimc_cap_querycap(struct file *file, void *priv,
>   return 0;
>  }
>  
> -static int vimc_cap_fmt_vid_cap(struct file *file, void *priv,
> +static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_format *f)
>  {
>   struct vimc_cap_device *vcap = video_drvdata(file);
> @@ -74,16 +84,112 @@ static int vimc_cap_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> +static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
> + struct v4l2_format *f)
> +{
> + struct v4l2_pix_format *format = >fmt.pix;
> + const struct vimc_pix_map *vpix;
> +
> + format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
> + VIMC_FRAME_MAX_WIDTH);
> + format->height = clamp_t(u32, format->height, VIMC_FRAME_MIN_HEIGHT,
> +  VIMC_FRAME_MAX_HEIGHT);
> +
> + /* Don't accept a pixelformat that is not on the table */
> + vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
> + if (!vpix) {
> + format->pixelformat = fmt_default.pixelformat;
> + vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
> + }
> + /* TODO: Add support for custom bytesperline values */
> + format->bytesperline = format->width * vpix->bpp;
> + format->sizeimage = format->bytesperline * format->height;
> +
> + if (format->field == V4L2_FIELD_ANY)
> + format->field = fmt_default.field;
> +
> + /* Check if values are out of range */
> + if (format->colorspace == V4L2_COLORSPACE_DEFAULT
> + || format->colorspace > V4L2_COLORSPACE_DCI_P3)
> + format->colorspace = fmt_default.colorspace;
> + if (format->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT
> + || format->ycbcr_enc > V4L2_YCBCR_ENC_SMPTE240M)
> + format->ycbcr_enc = fmt_default.ycbcr_enc;
> + if (format->quantization == V4L2_QUANTIZATION_DEFAULT
> + || format->quantization > V4L2_QUANTIZATION_LIM_RANGE)
> + format->quantization = fmt_default.quantization;
> + if (format->xfer_func == V4L2_XFER_FUNC_DEFAULT
> + || format->xfer_func > V4L2_XFER_FUNC_SMPTE2084)
> + format->xfer_func = fmt_default.xfer_func;

Same comments in the previous patch regarding width/height and the colorspace
information apply here as well.

> +
> + return 0;
> +}
> +
> +static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
> +   struct v4l2_format *f)
> +{
> + struct vimc_cap_device *vcap = video_drvdata(file);
> +
> + /* Do not change the format while stream is on */
> + if (vb2_is_busy(>queue))
> + return -EBUSY;
> +
> + vimc_cap_try_fmt_vid_cap(file, priv, f);
> +
> + dev_dbg(vcap->vdev.v4l2_dev->dev, "%s: format update: "
> + "old:%dx%d (0x%x, %d, %d, %d, %d) "
> + "new:%dx%d (0x%x, %d, %d, %d, %d)\n", vcap->vdev.name,
> + /* old */

Re: [PATCH v2 4/7] [media] vimc: sen: Support several image formats

[Hans Verkuil]

On 04/08/2017 12:37 AM, Helen Koike wrote:
> Allow user space to change the image format as the frame size, the
> media bus pixel format, colorspace, quantization, field YCbCr encoding
> and the transfer function
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Changes in v2:
> [media] vimc: sen: Support several image formats
>   - this is a new commit in the serie (the old one was splitted in two)
>   - add init_cfg to initialize try_fmt
>   - reorder code in vimc_sen_set_fmt
>   - allow user space to change all fields from struct v4l2_mbus_framefmt
> (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
>   - merge with patch for the enum_mbus_code and enum_frame_size
>   - change commit message
>   - add vimc_pix_map_by_index
>   - rename MIN/MAX macros
>   - check set_fmt default parameters for quantization, colorspace ...
> 
> 
> ---
>  drivers/media/platform/vimc/vimc-core.c   |   8 ++
>  drivers/media/platform/vimc/vimc-core.h   |  12 +++
>  drivers/media/platform/vimc/vimc-sensor.c | 143 
> --
>  3 files changed, 134 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c 
> b/drivers/media/platform/vimc/vimc-core.c
> index 7c23735..bc4b1bb 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -324,6 +324,14 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>   },
>  };
>  
> +const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i)
> +{
> + if (i >= ARRAY_SIZE(vimc_pix_map_list))
> + return NULL;
> +
> + return _pix_map_list[i];
> +}
> +
>  const struct vimc_pix_map *vimc_pix_map_by_code(u32 code)
>  {
>   unsigned int i;
> diff --git a/drivers/media/platform/vimc/vimc-core.h 
> b/drivers/media/platform/vimc/vimc-core.h
> index 8c3d401..2146672 100644
> --- a/drivers/media/platform/vimc/vimc-core.h
> +++ b/drivers/media/platform/vimc/vimc-core.h
> @@ -21,6 +21,11 @@
>  #include 
>  #include 
>  
> +#define VIMC_FRAME_MAX_WIDTH 4096
> +#define VIMC_FRAME_MAX_HEIGHT 2160
> +#define VIMC_FRAME_MIN_WIDTH 16
> +#define VIMC_FRAME_MIN_HEIGHT 16
> +
>  /**
>   * struct vimc_pix_map - maps media bus code with v4l2 pixel format
>   *
> @@ -107,6 +112,13 @@ static inline void vimc_pads_cleanup(struct media_pad 
> *pads)
>  int vimc_pipeline_s_stream(struct media_entity *ent, int enable);
>  
>  /**
> + * vimc_pix_map_by_index - get vimc_pix_map struct by its index
> + *
> + * @i:   index of the vimc_pix_map struct in 
> vimc_pix_map_list
> + */
> +const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);
> +
> +/**
>   * vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
>   *
>   * @code:media bus format code defined by MEDIA_BUS_FMT_* macros
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
> b/drivers/media/platform/vimc/vimc-sensor.c
> index abb2172..c86b4e6 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -24,8 +24,6 @@
>  
>  #include "vimc-sensor.h"
>  
> -#define VIMC_SEN_FRAME_MAX_WIDTH 4096
> -
>  struct vimc_sen_device {
>   struct vimc_ent_device ved;
>   struct v4l2_subdev sd;
> @@ -37,18 +35,41 @@ struct vimc_sen_device {
>   int frame_size;
>  };
>  
> +static const struct v4l2_mbus_framefmt fmt_default = {
> + .width = 640,
> + .height = 480,
> + .code = MEDIA_BUS_FMT_RGB888_1X24,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .quantization = V4L2_QUANTIZATION_FULL_RANGE,
> + .xfer_func = V4L2_XFER_FUNC_SRGB,
> +};
> +
> +static int vimc_sen_init_cfg(struct v4l2_subdev *sd,
> +  struct v4l2_subdev_pad_config *cfg)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < sd->entity.num_pads; i++) {
> + struct v4l2_mbus_framefmt *mf;
> +
> + mf = v4l2_subdev_get_try_format(sd, cfg, i);
> + *mf = fmt_default;
> + }
> +
> + return 0;
> +}
> +
>  static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
>  struct v4l2_subdev_pad_config *cfg,
>  struct v4l2_subdev_mbus_code_enum *code)
>  {
> - struct vimc_sen_device *vsen =
> - container_of(sd, struct vimc_sen_device, sd);
> + const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
>  
> - /* TODO: Add support for other codes */
> - if (code->index)
> + if (!vpix)
>   return -EINVAL;
>  
> - code->code = vsen->mbus_format.code;
> + code->code = vpix->code;
>  
>   return 0;
>  }
> @@ -57,33 +78,34 @@ static int vimc_sen_enum_frame_size(struct v4l2_subdev 
> *sd,
>   struct v4l2_subdev_pad_config *cfg,
>   struct v4l2_subdev_frame_size_enum *fse)
>  

Re: [PATCH v2 2/7] [media] vimc: Add vimc_ent_sd_* helper functions

[Hans Verkuil]

On 04/08/2017 12:37 AM, Helen Koike wrote:
> As all the subdevices in the topology will be initialized in the same
> way, to avoid code repetition the vimc_ent_sd_{register, unregister}
> helper functions were created
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Changes in v2:
> [media] vimc: Add vimc_ent_sd_* helper functions
>   - Comments in vimc_ent_sd_init
>   - Update vimc_ent_sd_init with upstream code as media_entity_pads_init
>   (instead of media_entity_init), entity->function intead of entity->type
>   - Add missing vimc_pads_cleanup in vimc_ent_sd_cleanup
>   - remove subdevice v4l2_dev and dev fields
>   - change unregister order in vimc_ent_sd_cleanup
>   - rename vimc_ent_sd_{init,cleanup} to vimc_ent_sd_{register,unregister}
>   - remove struct vimc_ent_subdevice, use ved and sd directly
>   - don't impose struct vimc_sen_device to declare ved and sd struct first
>   - add kernel docs
> 
> 
> ---
>  drivers/media/platform/vimc/vimc-core.c   | 66 
> +++
>  drivers/media/platform/vimc/vimc-core.h   | 39 ++
>  drivers/media/platform/vimc/vimc-sensor.c | 58 +--
>  3 files changed, 114 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c 
> b/drivers/media/platform/vimc/vimc-core.c
> index bc107da..15fa311 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -416,6 +416,72 @@ struct media_pad *vimc_pads_init(u16 num_pads, const 
> unsigned long *pads_flag)
>   return pads;
>  }
>  
> +static const struct media_entity_operations vimc_ent_sd_mops = {
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +int vimc_ent_sd_register(struct vimc_ent_device *ved,
> +  struct v4l2_subdev *sd,
> +  struct v4l2_device *v4l2_dev,
> +  const char *const name,
> +  u32 function,
> +  u16 num_pads,
> +  const unsigned long *pads_flag,
> +  const struct v4l2_subdev_ops *sd_ops,
> +  void (*sd_destroy)(struct vimc_ent_device *))
> +{
> + int ret;
> +
> + /* Allocate the pads */
> + ved->pads = vimc_pads_init(num_pads, pads_flag);
> + if (IS_ERR(ved->pads))
> + return PTR_ERR(ved->pads);
> +
> + /* Fill the vimc_ent_device struct */
> + ved->destroy = sd_destroy;
> + ved->ent = >entity;
> +
> + /* Initialize the subdev */
> + v4l2_subdev_init(sd, sd_ops);
> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;

Huh? Shouldn't this be:

sd->entity.function = function;

> + sd->entity.ops = _ent_sd_mops;
> + sd->owner = THIS_MODULE;
> + strlcpy(sd->name, name, sizeof(sd->name));
> + v4l2_set_subdevdata(sd, ved);
> +
> + /* Expose this subdev to user space */
> + sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + /* Initialize the media entity */
> + ret = media_entity_pads_init(>entity, num_pads, ved->pads);
> + if (ret)
> + goto err_clean_pads;
> +
> + /* Register the subdev with the v4l2 and the media framework */
> + ret = v4l2_device_register_subdev(v4l2_dev, sd);
> + if (ret) {
> + dev_err(v4l2_dev->dev,
> + "%s: subdev register failed (err=%d)\n",
> + name, ret);
> + goto err_clean_m_ent;
> + }
> +
> + return 0;
> +
> +err_clean_m_ent:
> + media_entity_cleanup(>entity);
> +err_clean_pads:
> + vimc_pads_cleanup(ved->pads);
> + return ret;
> +}
> +
> +void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev 
> *sd)
> +{
> + v4l2_device_unregister_subdev(sd);
> + media_entity_cleanup(ved->ent);
> + vimc_pads_cleanup(ved->pads);
> +}
> +
>  /*
>   * TODO: remove this function when all the
>   * entities specific code are implemented
> diff --git a/drivers/media/platform/vimc/vimc-core.h 
> b/drivers/media/platform/vimc/vimc-core.h
> index 4525d23..92c4729 100644
> --- a/drivers/media/platform/vimc/vimc-core.h
> +++ b/drivers/media/platform/vimc/vimc-core.h
> @@ -109,4 +109,43 @@ const struct vimc_pix_map *vimc_pix_map_by_code(u32 
> code);
>   */
>  const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
>  
> +/**
> + * vimc_ent_sd_register - initialize and register a subdev node
> + *
> + * @ved: the vimc_ent_device struct to be initialize
> + * @sd:  the v4l2_subdev struct to be initialize and registered
> + * @v4l2_dev:the v4l2 device to register the v4l2_subdev
> + * @name:name of the sub-device. Please notice that the name must be
> + *   unique.
> + * @function:media entity function defined by MEDIA_ENT_F_* macros
> + * @num_pads:number of pads to initialize
> + * @pads_flag:   flags to use in each pad
> + * @sd_ops:  

Re: [PATCH v2 1/7] [media] vimc: sen: Integrate the tpg on the sensor

[Hans Verkuil]

Hi Helen,

A quick review:

On 04/08/2017 12:37 AM, Helen Koike wrote:
> Initialize the test pattern generator on the sensor
> Generate a colored bar image instead of a grey one
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Changes in v2:
> [media] vimc: sen: Integrate the tpg on the sensor
>   - Fix include location
>   - Select V4L2_TPG in Kconfig
>   - configure tpg on streamon only
>   - rm BUG_ON
>   - coding style
> 
> 
> ---
>  drivers/media/platform/vimc/Kconfig   |  1 +
>  drivers/media/platform/vimc/vimc-sensor.c | 43 
> +--
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/Kconfig 
> b/drivers/media/platform/vimc/Kconfig
> index dd285fa..df124d4 100644
> --- a/drivers/media/platform/vimc/Kconfig
> +++ b/drivers/media/platform/vimc/Kconfig
> @@ -2,6 +2,7 @@ config VIDEO_VIMC
>   tristate "Virtual Media Controller Driver (VIMC)"
>   depends on VIDEO_DEV && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>   select VIDEOBUF2_VMALLOC
> + select VIDEO_V4L2_TPG
>   default n
>   ---help---
> Skeleton driver for Virtual Media Controller
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
> b/drivers/media/platform/vimc/vimc-sensor.c
> index 591f6a4..9154322 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -20,12 +20,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "vimc-sensor.h"
>  
> +#define VIMC_SEN_FRAME_MAX_WIDTH 4096
> +
>  struct vimc_sen_device {
>   struct vimc_ent_device ved;
>   struct v4l2_subdev sd;
> + struct tpg_data tpg;
>   struct task_struct *kthread_sen;
>   u8 *frame;
>   /* The active format */
> @@ -84,6 +88,28 @@ static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
>   return 0;
>  }
>  
> +static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
> +{
> + const struct vimc_pix_map *vpix =
> + vimc_pix_map_by_code(vsen->mbus_format.code);
> +
> + tpg_reset_source(>tpg, vsen->mbus_format.width,
> +  vsen->mbus_format.height, vsen->mbus_format.field);
> + tpg_s_bytesperline(>tpg, 0,
> +vsen->mbus_format.width * vpix->bpp);
> + tpg_s_buf_height(>tpg, vsen->mbus_format.height);
> + tpg_s_fourcc(>tpg, vpix->pixelformat);
> + /* TODO: check why the tpg_s_field need this third argument if
> +  * it is already receiving the field
> +  */
> + tpg_s_field(>tpg, vsen->mbus_format.field,
> + vsen->mbus_format.field == V4L2_FIELD_ALTERNATE);

V4L2_FIELD_ALTERNATE means that the driver will produce alternating top and
bottom fields. The test pattern generator needs to know that, so the third
argument here tells the tpg that this is indeed an alternating pattern.

The second argument tells it whether to generate a top or a bottom field,
so this argument cannot be V4L2_FIELD_ALTERNATE.

Supporting FIELD_ALTERNATE is tricky. The best place to look at for how to
use it is the vivid driver.

When you start generating frames you start off with the TOP field first.

The only exception to that is 60 Hz SDTV formats, but we're not supporting
those in this vimc driver.

> + tpg_s_colorspace(>tpg, vsen->mbus_format.colorspace);
> + tpg_s_ycbcr_enc(>tpg, vsen->mbus_format.ycbcr_enc);
> + tpg_s_quantization(>tpg, vsen->mbus_format.quantization);
> + tpg_s_xfer_func(>tpg, vsen->mbus_format.xfer_func);
> +}
> +
>  static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
>   .enum_mbus_code = vimc_sen_enum_mbus_code,
>   .enum_frame_size= vimc_sen_enum_frame_size,
> @@ -110,7 +136,7 @@ static int vimc_thread_sen(void *data)
>   if (kthread_should_stop())
>   break;
>  
> - memset(vsen->frame, 100, vsen->frame_size);
> + tpg_fill_plane_buffer(>tpg, V4L2_STD_PAL, 0, vsen->frame);

Use 0 instead of V4L2_STD_PAL. We're not emulating a SDTV receiver here.

>  
>   /* Send the frame to all source pads */
>   for (i = 0; i < vsen->sd.entity.num_pads; i++)
> @@ -159,6 +185,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int 
> enable)
>   vsen->frame = NULL;
>   return PTR_ERR(vsen->kthread_sen);
>   }
> +
> + /* configure the test pattern generator */
> + vimc_sen_tpg_s_format(vsen);
>   } else {
>   if (!vsen->kthread_sen)
>   return -EINVAL;
> @@ -189,6 +218,7 @@ static void vimc_sen_destroy(struct vimc_ent_device *ved)
>   struct vimc_sen_device *vsen =
>   container_of(ved, struct vimc_sen_device, ved);
>  
> + tpg_free(>tpg);
>   v4l2_device_unregister_subdev(>sd);
>   media_entity_cleanup(ved->ent);
>   kfree(vsen);
> @@ 

Re: [PATCH] dw9714: Initial driver for dw9714 VCM

[Hans Verkuil]

Hi Rajmohan,

Thanks for the patch!

A quick code review:

On 05/07/2017 01:33 PM, rajmohan.m...@intel.com wrote:
> From: Rajmohan Mani 
> 
> DW9714 is a 10 bit DAC, designed for linear
> control of voice coil motor.
> 
> This driver creates a V4L2 subdevice and
> provides control to set the desired focus.
> 
> Signed-off-by: Rajmohan Mani 
> ---
>  drivers/media/i2c/Kconfig  |   9 ++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/dw9714.c | 333 
> +
>  3 files changed, 343 insertions(+)
>  create mode 100644 drivers/media/i2c/dw9714.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index fd181c9..4f0a7ad 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -300,6 +300,15 @@ config VIDEO_AD5820
> This is a driver for the AD5820 camera lens voice coil.
> It is used for example in Nokia N900 (RX-51).
>  
> +config VIDEO_DW9714
> + tristate "DW9714 lens voice coil support"
> + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> + ---help---
> +   This is a driver for the DW9714 camera lens voice coil.
> +   DW9714 is a 10 bit DAC with 120mA output current sink
> +   capability. This is designed for linear control of
> +   voice coil motors, controlled via I2C serial interface.
> +
>  config VIDEO_SAA7110
>   tristate "Philips SAA7110 video decoder"
>   depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 62323ec..987bd1f 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
>  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
>  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
> +obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
>  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
>  obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
> diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
> new file mode 100644
> index 000..276d3f2
> --- /dev/null
> +++ b/drivers/media/i2c/dw9714.c
> @@ -0,0 +1,333 @@
> +/*
> + * Copyright (c) 2015--2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DW9714_NAME  "dw9714"
> +#define DW9714_MAX_FOCUS_POS 1023
> +#define DW9714_CTRL_STEPS16  /* Keep this value power of 2 */
> +#define DW9714_CTRL_DELAY_US 1000
> +/*
> + * S[3:2] = 0x00, codes per step for "Linear Slope Control"
> + * S[1:0] = 0x00, step period
> + */
> +#define DW9714_DEFAULT_S 0x0
> +#define DW9714_VAL(data, s) (u16)((data) << 4 | (s))
> +
> +/* dw9714 device structure */
> +struct dw9714_device {
> + struct i2c_client *client;
> + struct v4l2_ctrl_handler ctrls_vcm;
> + struct v4l2_subdev sd;
> + u16 current_val;
> +};
> +
> +#define to_dw9714_vcm(_ctrl) \
> + container_of(_ctrl->handler, struct dw9714_device, ctrls_vcm)
> +
> +static int dw9714_i2c_write(struct i2c_client *client, u16 data)
> +{
> + const int num_msg = 1;
> + int ret;
> + u16 val = cpu_to_be16(data);
> + struct i2c_msg msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .len = sizeof(val),
> + .buf = (u8 *) ,
> + };
> +
> + ret = i2c_transfer(client->adapter, , num_msg);
> +
> + /*One retry */
> + if (ret != num_msg)
> + ret = i2c_transfer(client->adapter, , num_msg);
> +
> + if (ret != num_msg) {
> + dev_err(>dev, "I2C write fail fail\n");
> + return -EIO;
> + } else {

No need for the 'else' here, since the 'if' already returns.

> + return 0;
> + }
> +}
> +
> +static int dw9714_t_focus_vcm(struct dw9714_device *dw9714_dev, u16 val)
> +{
> + struct i2c_client *client = dw9714_dev->client;
> + int ret = -EINVAL;
> +
> + dev_dbg(>dev, "Setting new value VCM: %d\n", val);
> + dw9714_dev->current_val = val;
> +
> + ret = dw9714_i2c_write(client, DW9714_VAL(val, DW9714_DEFAULT_S));
> + return ret;

Just do return dw9714_i2c_write(client, DW9714_VAL(val, DW9714_DEFAULT_S));

Thus dropping the 'ret' variable.

> +}
> +
> +static int dw9714_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9714_device *dev_vcm = to_dw9714_vcm(ctrl);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> +   

Re: [PATCH 8/8] omapdrm: hdmi4: hook up the HDMI CEC support

[Hans Verkuil]

On 05/08/2017 12:26 PM, Tomi Valkeinen wrote:
> On 06/05/17 14:58, Hans Verkuil wrote:
> 
>> My assumption was that hdmi_display_disable() was called when the hotplug 
>> would go
>> away. But I discovered that that isn't the case, or at least not when X is 
>> running.
>> It seems that the actual HPD check is done in hdmic_detect() in
>> omapdrm/displays/connector-hdmi.c.
> 
> For some HW it's done there (in the case there's no IP handling the
> HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard),
> and in some cases it also could be done in the hdmi driver (if the HPD
> is handled by the HDMI IP, but at the moment we don't have this case
> supported in the SW).
> 
>> But there I have no access to hdmi.core (needed for the 
>> hdmi4_cec_set_phys_addr() call).
>>
>> Any idea how to solve this? I am not all that familiar with drm, let alone 
>> omapdrm,
>> so if you can point me in the right direction, then that would be very 
>> helpful.
> 
> Hmm, indeed, looks the the output is kept enabled even if HPD drops and
> the connector status is changed to disconnected.
> 
> I don't have a very good solution... I think we have to add a function
> to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can
> call when they detect a HPD change. That call would go to the HDMI IP
> driver.

Right, I was thinking the same, I just wasn't sure if that was the correct
solution.

> Peter is about to send hotplug-interrupt-handling series, I think the
> HPD function work should be done on top of that, as otherwise it'll just
> conflict horribly.

OK, I'll do that.

I'll get CEC supported on the omap4 eventually! :-)

Regards,

Hans

Re: [PATCH 2/4] media: platform: dwc: Support for DW CSI-2 Host

[Hans Verkuil]

Hi Ramiro,

My sincere apologies for the long delay in reviewing this. The good news is that
I should have more time for reviews going forward, so I hope I'll be a lot 
quicker
in the future.

On 03/07/2017 03:37 PM, Ramiro Oliveira wrote:
> Add support for the Synopsys DesignWare CSI-2 Host
> 
> Signed-off-by: Ramiro Oliveira 
> ---
>  MAINTAINERS  |   7 +
>  drivers/media/platform/Kconfig   |   1 +
>  drivers/media/platform/Makefile  |   2 +
>  drivers/media/platform/dwc/Kconfig   |   5 +
>  drivers/media/platform/dwc/Makefile  |   1 +
>  drivers/media/platform/dwc/dw_mipi_csi.c | 653 
> +++
>  drivers/media/platform/dwc/dw_mipi_csi.h | 181 +
>  include/media/dwc/csi_host_platform.h|  97 +
>  8 files changed, 947 insertions(+)
>  create mode 100644 drivers/media/platform/dwc/Kconfig
>  create mode 100644 drivers/media/platform/dwc/Makefile
>  create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.c
>  create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.h
>  create mode 100644 include/media/dwc/csi_host_platform.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5badfd33e51f..19673dad43b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11061,6 +11061,13 @@ S:   Maintained
>  F:   drivers/staging/media/st-cec/
>  F:   Documentation/devicetree/bindings/media/stih-cec.txt
>  
> +SYNOPSYS DESIGNWARE CSI-2 HOST VIDEO PLATFORM
> +M:   Ramiro Oliveira 
> +L:   linux-media@vger.kernel.org
> +T:   git git://linuxtv.org/media_tree.git
> +S:   Maintained
> +F:   drivers/media/platform/dwc/
> +
>  SHARED MEMORY COMMUNICATIONS (SMC) SOCKETS
>  M:   Ursula Braun 
>  L:   linux-s...@vger.kernel.org
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 53f6f12bff0d..4b6e00da763f 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -120,6 +120,7 @@ source "drivers/media/platform/am437x/Kconfig"
>  source "drivers/media/platform/xilinx/Kconfig"
>  source "drivers/media/platform/rcar-vin/Kconfig"
>  source "drivers/media/platform/atmel/Kconfig"
> +source "drivers/media/platform/dwc/Kconfig"
>  
>  config VIDEO_TI_CAL
>   tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 8959f6e6692a..95eae2772c1f 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -64,6 +64,8 @@ obj-$(CONFIG_VIDEO_RCAR_VIN)+= rcar-vin/
>  
>  obj-$(CONFIG_VIDEO_ATMEL_ISC)+= atmel/
>  
> +obj-$(CONFIG_CSI_VIDEO_PLATFORM) += dwc/
> +
>  ccflags-y += -I$(srctree)/drivers/media/i2c
>  
>  obj-$(CONFIG_VIDEO_MEDIATEK_VPU) += mtk-vpu/
> diff --git a/drivers/media/platform/dwc/Kconfig 
> b/drivers/media/platform/dwc/Kconfig
> new file mode 100644
> index ..2cd13d23f897
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Kconfig
> @@ -0,0 +1,5 @@
> +config DWC_MIPI_CSI2_HOST
> + tristate "SNPS DWC MIPI CSI2 Host"
> + select GENERIC_PHY
> + help
> +   This is a V4L2 driver for Synopsys Designware MIPI CSI-2 Host.
> diff --git a/drivers/media/platform/dwc/Makefile 
> b/drivers/media/platform/dwc/Makefile
> new file mode 100644
> index ..5eb076a55123
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_DWC_MIPI_CSI2_HOST) += dw_mipi_csi.o
> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.c 
> b/drivers/media/platform/dwc/dw_mipi_csi.c
> new file mode 100644
> index ..6905def40a07
> --- /dev/null
> +++ b/drivers/media/platform/dwc/dw_mipi_csi.c
> @@ -0,0 +1,653 @@
> +/*
> + * DWC MIPI CSI-2 Host device driver
> + *
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + * Author: Ramiro Oliveira 
> + *
> + * 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.
> + */
> +
> +#include "dw_mipi_csi.h"
> +
> +/**
> + * @short Video formats supported by the MIPI CSI-2
> + */
> +static const struct mipi_fmt dw_mipi_csi_formats[] = {
> + {
> + /* RAW 8 */
> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .depth = 8,
> + },
> + {
> + /* RAW 10 */
> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
> + .depth = 10,
> + },
> + {
> + /* RGB 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + .depth = 16,
> + },
> + {
> + /* BGR 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> + .depth = 16,
> + },
> + {
> + /* RGB 888 */
> + .code = 

Re: [PATCH 8/8] omapdrm: hdmi4: hook up the HDMI CEC support

[Tomi Valkeinen]

On 06/05/17 14:58, Hans Verkuil wrote:

> My assumption was that hdmi_display_disable() was called when the hotplug 
> would go
> away. But I discovered that that isn't the case, or at least not when X is 
> running.
> It seems that the actual HPD check is done in hdmic_detect() in
> omapdrm/displays/connector-hdmi.c.

For some HW it's done there (in the case there's no IP handling the
HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard),
and in some cases it also could be done in the hdmi driver (if the HPD
is handled by the HDMI IP, but at the moment we don't have this case
supported in the SW).

> But there I have no access to hdmi.core (needed for the 
> hdmi4_cec_set_phys_addr() call).
> 
> Any idea how to solve this? I am not all that familiar with drm, let alone 
> omapdrm,
> so if you can point me in the right direction, then that would be very 
> helpful.

Hmm, indeed, looks the the output is kept enabled even if HPD drops and
the connector status is changed to disconnected.

I don't have a very good solution... I think we have to add a function
to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can
call when they detect a HPD change. That call would go to the HDMI IP
driver.

Peter is about to send hotplug-interrupt-handling series, I think the
HPD function work should be done on top of that, as otherwise it'll just
conflict horribly.

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Hans Verkuil]

On 05/08/2017 11:36 AM, Philipp Zabel wrote:
> Hi Hans,
> 
> On Mon, 2017-05-08 at 10:27 +0200, Hans Verkuil wrote:
>> Hi Philipp,
>>
>> Sorry for the very long delay, but I finally had some time to think about 
>> this.
> 
> Thank you for your thoughts.
> 
>> On 04/06/2017 03:55 PM, Philipp Zabel wrote:
>>> If the the field order is set to ANY in set_fmt, choose the currently
>>> set field order. If the colorspace is set to DEFAULT, choose the current
>>> colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
>>> DEFAULT, either choose the current setting, or the default setting for the
>>> new colorspace, if non-DEFAULT colorspace was given.
>>>
>>> This allows to let field order and colorimetry settings be propagated
>>> from upstream by calling media-ctl on the upstream entity source pad,
>>> and then call media-ctl on the sink pad to manually set the input frame
>>> interval, without changing the already set field order and colorimetry
>>> information.
>>>
>>> Signed-off-by: Philipp Zabel 
>>> ---
>>> This is based on imx-media-staging-md-v14, and it is supposed to allow
>>> configuring the pipeline with media-ctl like this:
>>>
>>> 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
>>> 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
>>> 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
>>> 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
>>> 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
>>>
>>> Without having step 4) overwrite the colorspace and field order set on
>>> 'ipu1_csi0':0 by the propagation in step 3).
>>> ---
>>>  drivers/staging/media/imx/imx-media-csi.c | 34 
>>> +++
>>>  1 file changed, 34 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
>>> b/drivers/staging/media/imx/imx-media-csi.c
>>> index 64dc454f6b371..d94ce1de2bf05 100644
>>> --- a/drivers/staging/media/imx/imx-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>>> @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>>> csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
>>>  
>>> fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
>>> +
>>> +   /* Retain current field setting as default */
>>> +   if (sdformat->format.field == V4L2_FIELD_ANY)
>>> +   sdformat->format.field = fmt->field;
>>
>> This is OK.
> 
> Would this be a "may" or a "should"? As in,
> "If SUBDEV_S_FMT is called with field == ANY, the driver may/should set
> field to the previously configured interlacing field order".

To quote the FIELD_ANY documentation:

file:///home/hans/work/src/v4l/media-git/Documentation/output/html/uapi/v4l/field-order.html#field-order

"Drivers must never return V4L2_FIELD_ANY."

How a driver replaces FIELD_ANY is something that I am not sure should be 
explicitly
stated in the documentation. In many cases there is only one choice (usually 
FIELD_NONE).

In cases like this I think your proposed rule is a good recommendation. I would 
probably
phrase it like that.

> What would be the correct place to document this behaviour?
> Documentation/media/uapi/v4l/vidioc-subdev-g-fmt.rst?

Yes.

> 
>>> +   /* Retain current colorspace setting as default */
>>> +   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>>> +   sdformat->format.colorspace = fmt->colorspace;
>>> +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
>>> +   sdformat->format.xfer_func = fmt->xfer_func;
>>> +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
>>> +   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>>> +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
>>> +   sdformat->format.quantization = fmt->quantization;
>>
>> If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
>> copy
>> all four fields from fmt to sdformat->format. The other three fields are 
>> meaningless
>> when colorspace == V4L2_COLORSPACE_DEFAULT.
> 
> Ok, good. Ignoring the transfer function / YCbCr encoding / quantization
> range fields when colorspace is DEFAULT would simplify this part to:
> 
>   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>   sdformat->format.colorspace = fmt->colorspace;
>   sdformat->format.xfer_func = fmt->xfer_func;
>   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>   sdformat->format.quantization = fmt->quantization;
>   }
> 
> Is that expectation already written down somewhere? If not, should we
> add it to Documentation/media/uapi/v4l/pixfmt-006.rst?

I don't think it is written down. It would be a good idea to make this
explicit.

> 
>>> +   } else {
>>> +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
>>> +   sdformat->format.xfer_func =

Re: [PATCH 01/11] [media] dvb-core/dvb_ca_en50221.c: Rename STATUSREG_??

[Mauro Carvalho Chehab]

Em Sun,  7 May 2017 23:23:24 +0200
"Jasmin J."  escreveu:

> From: Jasmin Jessich 
> 
> Rename STATUSREG_?? -> STATREG_?? to reduce the line length.

That sounds a bad idea. We use "stat" on the DVB subsystem as an
alias for statistics.

> 
> Signed-off-by: Jasmin Jessich 
> ---
>  drivers/media/dvb-core/dvb_ca_en50221.c | 34 
> -
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
> b/drivers/media/dvb-core/dvb_ca_en50221.c
> index cc709c9..b978246 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -72,11 +72,11 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug 
> messages");
>  #define CMDREG_DAIE   0x80   /* Enable DA interrupt */
>  #define IRQEN (CMDREG_DAIE)
>  
> -#define STATUSREG_RE 1   /* read error */
> -#define STATUSREG_WE 2   /* write error */
> -#define STATUSREG_FR  0x40   /* module free */
> -#define STATUSREG_DA  0x80   /* data available */
> -#define STATUSREG_TXERR (STATUSREG_RE|STATUSREG_WE)  /* general transfer 
> error */
> +#define STATREG_RE 1 /* read error */
> +#define STATREG_WE 2 /* write error */
> +#define STATREG_FR  0x40 /* module free */
> +#define STATREG_DA  0x80 /* data available */
> +#define STATREG_TXERR (STATREG_RE|STATREG_WE)/* general transfer 
> error */
>  
>  
>  #define DVB_CA_SLOTSTATE_NONE   0
> @@ -347,7 +347,7 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
> *ca, int slot)
>   /* read the buffer size from the CAM */
>   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
> IRQEN | CMDREG_SR)) != 0)
>   return ret;
> - if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_DA, HZ)) 
> != 0)
> + if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATREG_DA, HZ)) != 
> 0)
>   return ret;
>   if ((ret = dvb_ca_en50221_read_data(ca, slot, buf, 2)) != 2)
>   return -EIO;
> @@ -366,7 +366,7 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
> *ca, int slot)
>   /* write the buffer size to the CAM */
>   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
> IRQEN | CMDREG_SW)) != 0)
>   return ret;
> - if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_FR, HZ / 
> 10)) != 0)
> + if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATREG_FR, HZ / 
> 10)) != 0)
>   return ret;
>   if ((ret = dvb_ca_en50221_write_data(ca, slot, buf, 2)) != 2)
>   return -EIO;
> @@ -661,7 +661,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
> *ca, int slot, u8 * eb
>   /* check if there is data available */
>   if ((status = ca->pub->read_cam_control(ca->pub, slot, 
> CTRLIF_STATUS)) < 0)
>   goto exit;
> - if (!(status & STATUSREG_DA)) {
> + if (!(status & STATREG_DA)) {
>   /* no data */
>   status = 0;
>   goto exit;
> @@ -713,7 +713,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
> *ca, int slot, u8 * eb
>   /* check for read error (RE should now be 0) */
>   if ((status = ca->pub->read_cam_control(ca->pub, slot, 
> CTRLIF_STATUS)) < 0)
>   goto exit;
> - if (status & STATUSREG_RE) {
> + if (status & STATREG_RE) {
>   ca->slot_info[slot].slot_state = 
> DVB_CA_SLOTSTATE_LINKINIT;
>   status = -EIO;
>   goto exit;
> @@ -778,8 +778,8 @@ static int dvb_ca_en50221_write_data(struct 
> dvb_ca_private *ca, int slot, u8 * b
>  process the data if necessary. */
>   if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) 
> < 0)
>   goto exitnowrite;
> - if (status & (STATUSREG_DA | STATUSREG_RE)) {
> - if (status & STATUSREG_DA)
> + if (status & (STATREG_DA | STATREG_RE)) {
> + if (status & STATREG_DA)
>   dvb_ca_en50221_thread_wakeup(ca);
>  
>   status = -EAGAIN;
> @@ -794,7 +794,7 @@ static int dvb_ca_en50221_write_data(struct 
> dvb_ca_private *ca, int slot, u8 * b
>   /* check if interface is still free */
>   if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) 
> < 0)
>   goto exit;
> - if (!(status & STATUSREG_FR)) {
> + if (!(status & STATREG_FR)) {
>   /* it wasn't free => try again later */
>   status = -EAGAIN;
>   goto exit;
> @@ -815,8 +815,8 @@ static int dvb_ca_en50221_write_data(struct 
> dvb_ca_private *ca, int slot, u8 * b
>   if (status < 0)
>   goto exit;
>  
> - if (status & (STATUSREG_DA | STATUSREG_RE)) {
> - if (status & STATUSREG_DA)
> +

Re: [PATCH 40/40] media: imx: set and propagate empty field, colorimetry params

[Philipp Zabel]

Hi Steve,

On Wed, 2017-04-12 at 17:45 -0700, Steve Longerbeam wrote:
> This patch adds a call to imx_media_fill_empty_mbus_fields() in the
> *_try_fmt() functions at the sink pads, to set empty field order and
> colorimetry parameters.
> 
> If the field order is set to ANY, choose the currently set field order
> at the sink pad. If the colorspace is set to DEFAULT, choose the
> current colorspace at the sink pad.  If any of xfer_func, ycbcr_enc
> or quantization are set to DEFAULT, either choose the current sink pad
> setting, or the default setting for the new colorspace, if non-DEFAULT
> colorspace was given.
> 
> Colorimetry is also propagated from sink to source pads anywhere
> this has not already been done. The exception is ic-prpencvf at the
> source pad, since the Image Converter outputs fixed quantization and
> Y`CbCr encoding.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-ic-prp.c  |  5 ++-
>  drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +++---
>  drivers/staging/media/imx/imx-media-csi.c   | 12 +--
>  drivers/staging/media/imx/imx-media-utils.c | 53 
> +
>  drivers/staging/media/imx/imx-media-vdic.c  |  7 ++--
>  drivers/staging/media/imx/imx-media.h   |  3 +-
>  6 files changed, 95 insertions(+), 10 deletions(-)
> 
[...]
> diff --git a/drivers/staging/media/imx/imx-media-utils.c 
> b/drivers/staging/media/imx/imx-media-utils.c
> index 7b2f92d..b07d0ae 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -464,6 +464,59 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt 
> *mbus,
>  }
>  EXPORT_SYMBOL_GPL(imx_media_init_mbus_fmt);
>  
> +/*
> + * Check whether the field or colorimetry params in tryfmt are
> + * uninitialized, and if so fill them with the values from fmt.
> + * The exception is when tryfmt->colorspace has been initialized,
> + * if so all the further default colorimetry params can be derived
> + * from tryfmt->colorspace.
> + */
> +void imx_media_fill_empty_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
> +   struct v4l2_mbus_framefmt *fmt)
> +{
> + /* fill field if necessary */
> + if (tryfmt->field == V4L2_FIELD_ANY)
> + tryfmt->field = fmt->field;
> +
> + /* fill colorimetry if necessary */
> + if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
> + tryfmt->colorspace = fmt->colorspace;
> + if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
> + tryfmt->xfer_func = fmt->xfer_func;
> + if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> + tryfmt->ycbcr_enc = fmt->ycbcr_enc;
> + if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
> + tryfmt->quantization = fmt->quantization;

According to Hans' latest comments, this could be changed to:

--8<--
>From cca3cda9effcaca0891eb8044a79137023fed1c2 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 8 May 2017 11:38:05 +0200
Subject: [PATCH] fixup! media: imx: set and propagate default field,
 colorimetry

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-utils.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index a8766489e8a18..ec2abd618cc44 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -497,12 +497,9 @@ void imx_media_fill_default_mbus_fields(struct 
v4l2_mbus_framefmt *tryfmt,
/* fill colorimetry if necessary */
if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
tryfmt->colorspace = fmt->colorspace;
-   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
-   tryfmt->xfer_func = fmt->xfer_func;
-   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
-   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
-   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
-   tryfmt->quantization = fmt->quantization;
+   tryfmt->xfer_func = fmt->xfer_func;
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   tryfmt->quantization = fmt->quantization;
} else {
if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT) {
tryfmt->xfer_func =
-- 
2.11.0
-->8--

> + } else {
> + const struct imx_media_pixfmt *cc;
> + bool is_rgb = false;
> +
> + cc = imx_media_find_mbus_format(tryfmt->code,
> + CS_SEL_ANY, false);
> + if (!cc)
> + cc = imx_media_find_ipu_format(tryfmt->code,
> +

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Philipp Zabel]

Hi Hans,

On Mon, 2017-05-08 at 10:27 +0200, Hans Verkuil wrote:
> Hi Philipp,
> 
> Sorry for the very long delay, but I finally had some time to think about 
> this.

Thank you for your thoughts.

> On 04/06/2017 03:55 PM, Philipp Zabel wrote:
> > If the the field order is set to ANY in set_fmt, choose the currently
> > set field order. If the colorspace is set to DEFAULT, choose the current
> > colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
> > DEFAULT, either choose the current setting, or the default setting for the
> > new colorspace, if non-DEFAULT colorspace was given.
> > 
> > This allows to let field order and colorimetry settings be propagated
> > from upstream by calling media-ctl on the upstream entity source pad,
> > and then call media-ctl on the sink pad to manually set the input frame
> > interval, without changing the already set field order and colorimetry
> > information.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> > This is based on imx-media-staging-md-v14, and it is supposed to allow
> > configuring the pipeline with media-ctl like this:
> > 
> > 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
> > 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
> > 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
> > 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
> > 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
> > 
> > Without having step 4) overwrite the colorspace and field order set on
> > 'ipu1_csi0':0 by the propagation in step 3).
> > ---
> >  drivers/staging/media/imx/imx-media-csi.c | 34 
> > +++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> > b/drivers/staging/media/imx/imx-media-csi.c
> > index 64dc454f6b371..d94ce1de2bf05 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
> > csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
> >  
> > fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
> > +
> > +   /* Retain current field setting as default */
> > +   if (sdformat->format.field == V4L2_FIELD_ANY)
> > +   sdformat->format.field = fmt->field;
> 
> This is OK.

Would this be a "may" or a "should"? As in,
"If SUBDEV_S_FMT is called with field == ANY, the driver may/should set
field to the previously configured interlacing field order".

What would be the correct place to document this behaviour?
Documentation/media/uapi/v4l/vidioc-subdev-g-fmt.rst?

> > +   /* Retain current colorspace setting as default */
> > +   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
> > +   sdformat->format.colorspace = fmt->colorspace;
> > +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
> > +   sdformat->format.xfer_func = fmt->xfer_func;
> > +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> > +   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
> > +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
> > +   sdformat->format.quantization = fmt->quantization;
> 
> If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
> copy
> all four fields from fmt to sdformat->format. The other three fields are 
> meaningless
> when colorspace == V4L2_COLORSPACE_DEFAULT.

Ok, good. Ignoring the transfer function / YCbCr encoding / quantization
range fields when colorspace is DEFAULT would simplify this part to:

if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
sdformat->format.colorspace = fmt->colorspace;
sdformat->format.xfer_func = fmt->xfer_func;
sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
sdformat->format.quantization = fmt->quantization;
}

Is that expectation already written down somewhere? If not, should we
add it to Documentation/media/uapi/v4l/pixfmt-006.rst?

> > +   } else {
> > +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
> > +   sdformat->format.xfer_func =
> > +   V4L2_MAP_XFER_FUNC_DEFAULT(
> > +   sdformat->format.colorspace);
> > +   }
> > +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
> > +   sdformat->format.ycbcr_enc =
> > +   V4L2_MAP_YCBCR_ENC_DEFAULT(
> > +   sdformat->format.colorspace);
> > +   }
> > +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) 
> > {
> > +   sdformat->format.quantization =
> > +   V4L2_MAP_QUANTIZATION_DEFAULT(
> > +   

[PATCH 4/4] dma-buf: Use seq_putc() in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 8 May 2017 10:55:42 +0200

Three single characters (line breaks) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/dma-buf/sync_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
index c769dc653b34..a0d780ab68c3 100644
--- a/drivers/dma-buf/sync_debug.c
+++ b/drivers/dma-buf/sync_debug.c
@@ -110,7 +110,7 @@ static void sync_print_fence(struct seq_file *s,
}
}
 
-   seq_puts(s, "\n");
+   seq_putc(s, '\n');
 }
 
 static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj)
@@ -161,7 +161,7 @@ static int sync_debugfs_show(struct seq_file *s, void 
*unused)
 sync_timeline_list);
 
sync_print_obj(s, obj);
-   seq_puts(s, "\n");
+   seq_putc(s, '\n');
}
spin_unlock_irqrestore(_timeline_list_lock, flags);
 
@@ -173,7 +173,7 @@ static int sync_debugfs_show(struct seq_file *s, void 
*unused)
container_of(pos, struct sync_file, sync_file_list);
 
sync_print_sync_file(s, sync_file);
-   seq_puts(s, "\n");
+   seq_putc(s, '\n');
}
spin_unlock_irqrestore(_file_list_lock, flags);
return 0;
-- 
2.12.2

[PATCH 3/4] dma-buf: Adjust a null pointer check in dma_buf_attach()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 8 May 2017 10:54:17 +0200

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written "!attach"

Thus adjust this expression.

Signed-off-by: Markus Elfring 
---
 drivers/dma-buf/dma-buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 9887d72cf804..4a038dcf5361 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -559,7 +559,7 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf 
*dmabuf,
return ERR_PTR(-EINVAL);
 
attach = kzalloc(sizeof(*attach), GFP_KERNEL);
-   if (attach == NULL)
+   if (!attach)
return ERR_PTR(-ENOMEM);
 
attach->dev = dev;
-- 
2.12.2

[PATCH 2/4] dma-buf: Improve a size determination in dma_buf_attach()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 8 May 2017 10:50:09 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/dma-buf/dma-buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 53257c166f4d..9887d72cf804 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -558,7 +558,7 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf 
*dmabuf,
if (WARN_ON(!dmabuf || !dev))
return ERR_PTR(-EINVAL);
 
-   attach = kzalloc(sizeof(struct dma_buf_attachment), GFP_KERNEL);
+   attach = kzalloc(sizeof(*attach), GFP_KERNEL);
if (attach == NULL)
return ERR_PTR(-ENOMEM);
 
-- 
2.12.2

[PATCH 1/4] dma-buf: Combine two function calls into one in dma_buf_debug_show()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 8 May 2017 10:32:44 +0200

A bit of data was put into a sequence by two separate function calls.
Print the same data by a single function call instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/dma-buf/dma-buf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 512bdbc23bbb..53257c166f4d 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1122,9 +1122,7 @@ static int dma_buf_debug_show(struct seq_file *s, void 
*unused)
attach_count = 0;
 
list_for_each_entry(attach_obj, _obj->attachments, node) {
-   seq_puts(s, "\t");
-
-   seq_printf(s, "%s\n", dev_name(attach_obj->dev));
+   seq_printf(s, "\t%s\n", dev_name(attach_obj->dev));
attach_count++;
}
 
-- 
2.12.2

[PATCH 0/4] DMA-buf: Fine-tuning for four function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 8 May 2017 11:05:05 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Combine two function calls into one in dma_buf_debug_show()
  Improve a size determination in dma_buf_attach()
  Adjust a null pointer check in dma_buf_attach()
  Use seq_putc() in two functions

 drivers/dma-buf/dma-buf.c| 8 +++-
 drivers/dma-buf/sync_debug.c | 6 +++---
 2 files changed, 6 insertions(+), 8 deletions(-)

-- 
2.12.2

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Hans Verkuil]

Hi Yong Zhi,

See my review comments below. Nothing earth-shattering, you'll be pleased to 
hear :-)

On 04/30/2017 01:34 AM, Yong Zhi wrote:
> This patch adds CIO2 CSI-2 device driver for
> Intel's IPU3 camera sub-system support.
> 
> The V4L2 fwnode matching depends on the following work:
> 
> 
> 
> Signed-off-by: Yong Zhi 
> ---
>  drivers/media/pci/Kconfig  |2 +
>  drivers/media/pci/Makefile |3 +-
>  drivers/media/pci/ipu3/Kconfig |   17 +
>  drivers/media/pci/ipu3/Makefile|1 +
>  drivers/media/pci/ipu3/ipu3-cio2.c | 1813 
> 
>  drivers/media/pci/ipu3/ipu3-cio2.h |  425 +
>  6 files changed, 2260 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/pci/ipu3/Kconfig
>  create mode 100644 drivers/media/pci/ipu3/Makefile
>  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.c
>  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.h
> 
> diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
> index da28e68..63ece75 100644
> --- a/drivers/media/pci/Kconfig
> +++ b/drivers/media/pci/Kconfig
> @@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
>  source "drivers/media/pci/netup_unidvb/Kconfig"
>  endif
>  
> +source "drivers/media/pci/ipu3/Kconfig"
> +
>  endif #MEDIA_PCI_SUPPORT
>  endif #PCI
> diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
> index a7e8af0..8d5e8db 100644
> --- a/drivers/media/pci/Makefile
> +++ b/drivers/media/pci/Makefile
> @@ -13,7 +13,8 @@ obj-y+= ttpci/  \
>   ddbridge/   \
>   saa7146/\
>   smipcie/\
> - netup_unidvb/
> + netup_unidvb/   \
> + ipu3/
>  
>  obj-$(CONFIG_VIDEO_IVTV) += ivtv/
>  obj-$(CONFIG_VIDEO_ZORAN) += zoran/
> diff --git a/drivers/media/pci/ipu3/Kconfig b/drivers/media/pci/ipu3/Kconfig
> new file mode 100644
> index 000..2a895d6
> --- /dev/null
> +++ b/drivers/media/pci/ipu3/Kconfig
> @@ -0,0 +1,17 @@
> +config VIDEO_IPU3_CIO2
> + tristate "Intel ipu3-cio2 driver"
> + depends on VIDEO_V4L2 && PCI
> + depends on MEDIA_CONTROLLER
> + depends on HAS_DMA
> + depends on ACPI
> + select V4L2_FWNODE
> + select VIDEOBUF2_DMA_SG
> +
> + ---help---
> + This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
> + Skylake and Kaby Lake SoCs and used for capturing images and
> + video from a camera sensor.
> +
> + Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> + connected camera.
> + The module will be called ipu3-cio2.
> diff --git a/drivers/media/pci/ipu3/Makefile b/drivers/media/pci/ipu3/Makefile
> new file mode 100644
> index 000..20186e3
> --- /dev/null
> +++ b/drivers/media/pci/ipu3/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> diff --git a/drivers/media/pci/ipu3/ipu3-cio2.c 
> b/drivers/media/pci/ipu3/ipu3-cio2.c
> new file mode 100644
> index 000..2b641ad
> --- /dev/null
> +++ b/drivers/media/pci/ipu3/ipu3-cio2.c
> @@ -0,0 +1,1813 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Based partially on Intel IPU4 driver written by
> + *  Sakari Ailus 
> + *  Samu Onkalo 
> + *  Jouni Högander 
> + *  Jouni Ukkonen 
> + *  Antti Laakso 
> + * et al.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "ipu3-cio2.h"
> +
> +MODULE_AUTHOR("Tianshu Qiu ");
> +MODULE_AUTHOR("Jian Xu Zheng ");
> +MODULE_AUTHOR("Yuning Pu ");
> +MODULE_AUTHOR("Tuukka Toivonen ");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("IPU3 CIO2 driver");
> +
> +/*
> + * These are raw formats used in Intel's third generation of
> + * Image Processing Unit known as IPU3.
> + * 10bit raw bayer packed, 32 bytes for every 25 pixels, last 6 bits unused
> + */
> +static const u32 cio2_csi2_fmts[] = {
> + V4L2_PIX_FMT_IPU3_SRGGB10,
> + V4L2_PIX_FMT_IPU3_SGBRG10,
> + V4L2_PIX_FMT_IPU3_SGRBG10,
> + V4L2_PIX_FMT_IPU3_SBGGR10,
> +};
> +
> +static inline u32 cio2_bytesperline(const unsigned int 

Re: [PATCH 2/3] [media] doc-rst: add IPU3 raw10 bayer pixel format definitions

[Hans Verkuil]

On 04/30/2017 01:34 AM, Yong Zhi wrote:
> The formats added by this patch are:
> 
> V4L2_PIX_FMT_IPU3_SBGGR10
> V4L2_PIX_FMT_IPU3_SGBRG10
> V4L2_PIX_FMT_IPU3_SGRBG10
> V4L2_PIX_FMT_IPU3_SRGGB10
> 
> Signed-off-by: Yong Zhi 
> ---
>  Documentation/media/uapi/v4l/pixfmt-rgb.rst|  1 +
>  .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst | 61 
> ++
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
> 
> diff --git a/Documentation/media/uapi/v4l/pixfmt-rgb.rst 
> b/Documentation/media/uapi/v4l/pixfmt-rgb.rst
> index b0f3513..6900d5c 100644
> --- a/Documentation/media/uapi/v4l/pixfmt-rgb.rst
> +++ b/Documentation/media/uapi/v4l/pixfmt-rgb.rst
> @@ -16,5 +16,6 @@ RGB Formats
>  pixfmt-srggb10p
>  pixfmt-srggb10alaw8
>  pixfmt-srggb10dpcm8
> +pixfmt-srggb10-ipu3
>  pixfmt-srggb12
>  pixfmt-srggb16
> diff --git a/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst 
> b/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
> new file mode 100644
> index 000..8a82f30
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
> @@ -0,0 +1,61 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2_PIX_FMT_IPU3_SBGGR10:
> +.. _V4L2_PIX_FMT_IPU3_SGBRG10:
> +.. _V4L2_PIX_FMT_IPU3_SGRBG10:
> +.. _V4L2_PIX_FMT_IPU3_SRGGB10:
> +
> +**
> +V4L2_PIX_FMT_IPU3_SBGGR10 ('ip3b'), V4L2_PIX_FMT_IPU3_SGBRG10 ('ip3g'), 
> V4L2_PIX_FMT_IPU3_SGRBG10 ('ip3G'), V4L2_PIX_FMT_IPU3_SRGGB10 ('ip3r')
> +**
> +
> +10-bit Bayer formats
> +
> +Description
> +===
> +
> +These four pixel formats are raw sRGB / Bayer formats with 10 bits per
> +sample with every 25 pixels packed to 32 bytes leaving 6 most significant 
> +bits padding in the last byte. The format is little endian.

Make a quick mention that these formats are used by the Intel IPU3 driver.

Regards,

Hans

> +
> +In other respects this format is similar to :ref:`V4L2-PIX-FMT-SRGGB10`.
> +
> +**Byte Order.**
> +Each cell is one byte.
> +
> +.. raw:: latex
> +
> +\newline\newline\begin{adjustbox}{width=\columnwidth}
> +
> +.. tabularcolumns:: 
> |p{1.3cm}|p{1.0cm}|p{10.9cm}|p{10.9cm}|p{10.9cm}|p{1.0cm}|
> +
> +.. flat-table::
> +
> +* - start + 0:
> +  - B\ :sub:`00low`
> +  - G\ :sub:`01low` \ (bits 7--2) B\ :sub:`00high`\ (bits 1--0)
> +  - B\ :sub:`02low` \ (bits 7--4) G\ :sub:`01high`\ (bits 3--0)
> +  - G\ :sub:`03low` \ (bits 7--6) B\ :sub:`02high`\ (bits 5--0)
> +  - G\ :sub:`03high`
> +* - start + 5:
> +  - G\ :sub:`10low`
> +  - R\ :sub:`11low` \ (bits 7--2) G\ :sub:`10high`\ (bits 1--0)
> +  - G\ :sub:`12low` \ (bits 7--4) R\ :sub:`11high`\ (bits 3--0)
> +  - R\ :sub:`13low` \ (bits 7--6) G\ :sub:`12high`\ (bits 5--0)
> +  - R\ :sub:`13high`
> +* - start + 10:
> +  - B\ :sub:`20low`
> +  - G\ :sub:`21low` \ (bits 7--2) B\ :sub:`20high`\ (bits 1--0)
> +  - B\ :sub:`22low` \ (bits 7--4) G\ :sub:`21high`\ (bits 3--0)
> +  - G\ :sub:`23low` \ (bits 7--6) B\ :sub:`22high`\ (bits 5--0)
> +  - G\ :sub:`23high`
> +* - start + 15:
> +  - G\ :sub:`30low`
> +  - R\ :sub:`31low` \ (bits 7--2) G\ :sub:`30high`\ (bits 1--0)
> +  - G\ :sub:`32low` \ (bits 7--4) R\ :sub:`31high`\ (bits 3--0)
> +  - R\ :sub:`33low` \ (bits 7--6) G\ :sub:`32high`\ (bits 5--0)
> +  - R\ :sub:`33high`
> +
> +.. raw:: latex
> +
> +\end{adjustbox}\newline\newline
> 

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Hans Verkuil]

Hi Philipp,

Sorry for the very long delay, but I finally had some time to think about this.

On 04/06/2017 03:55 PM, Philipp Zabel wrote:
> If the the field order is set to ANY in set_fmt, choose the currently
> set field order. If the colorspace is set to DEFAULT, choose the current
> colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
> DEFAULT, either choose the current setting, or the default setting for the
> new colorspace, if non-DEFAULT colorspace was given.
> 
> This allows to let field order and colorimetry settings be propagated
> from upstream by calling media-ctl on the upstream entity source pad,
> and then call media-ctl on the sink pad to manually set the input frame
> interval, without changing the already set field order and colorimetry
> information.
> 
> Signed-off-by: Philipp Zabel 
> ---
> This is based on imx-media-staging-md-v14, and it is supposed to allow
> configuring the pipeline with media-ctl like this:
> 
> 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
> 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
> 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
> 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
> 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
> 
> Without having step 4) overwrite the colorspace and field order set on
> 'ipu1_csi0':0 by the propagation in step 3).
> ---
>  drivers/staging/media/imx/imx-media-csi.c | 34 
> +++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 64dc454f6b371..d94ce1de2bf05 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>   csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
>  
>   fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
> +
> + /* Retain current field setting as default */
> + if (sdformat->format.field == V4L2_FIELD_ANY)
> + sdformat->format.field = fmt->field;

This is OK.

> +
> + /* Retain current colorspace setting as default */
> + if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
> + sdformat->format.colorspace = fmt->colorspace;
> + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
> + sdformat->format.xfer_func = fmt->xfer_func;
> + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> + sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
> + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
> + sdformat->format.quantization = fmt->quantization;

If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
copy
all four fields from fmt to sdformat->format. The other three fields are 
meaningless
when colorspace == V4L2_COLORSPACE_DEFAULT.

> + } else {
> + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
> + sdformat->format.xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(
> + sdformat->format.colorspace);
> + }
> + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
> + sdformat->format.ycbcr_enc =
> + V4L2_MAP_YCBCR_ENC_DEFAULT(
> + sdformat->format.colorspace);
> + }
> + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) 
> {
> + sdformat->format.quantization =
> + V4L2_MAP_QUANTIZATION_DEFAULT(
> + cc->cs != IPUV3_COLORSPACE_YUV,
> + sdformat->format.colorspace,
> + sdformat->format.ycbcr_enc);
> + }

Is this needed for validation? Currently these fields play no role in the
default link validation. Which I think is actually the right thing to do,
unless the subdev can do actual colorspace conversion.

I would just drop the whole 'else' here.

Actually, wouldn't it be better to always just copy this information from
fmt? This subdev doesn't do any colorspace conversion, it just passes on
this information. I.e., you can't set it in any meaningful way.

Regards,

Hans

> + }
> +
>   *fmt = sdformat->format;
>  
>   if (sdformat->pad == CSI_SINK_PAD) {
>