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:   Fri Mar 30 05:00:10 CEST 2018
media-tree git hash:6ccd228e0cfce2a4f44558422d25c60fcb1a6710
media_build git hash:   d16c7406ed6cc5fc20c87c3711741c43039275d2
v4l-utils git hash: 3dc9af2b54eddb531823b99e77f3f212bdcc9cca
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.101-i686: WARNINGS
linux-3.0.101-x86_64: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.100-i686: WARNINGS
linux-3.2.100-x86_64: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.113-i686: WARNINGS
linux-3.4.113-x86_64: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.10-i686: WARNINGS
linux-3.7.10-x86_64: WARNINGS
linux-3.8.13-i686: WARNINGS
linux-3.8.13-x86_64: WARNINGS
linux-3.9.11-i686: WARNINGS
linux-3.9.11-x86_64: WARNINGS
linux-3.10.108-i686: WARNINGS
linux-3.10.108-x86_64: WARNINGS
linux-3.11.10-i686: WARNINGS
linux-3.11.10-x86_64: WARNINGS
linux-3.12.74-i686: WARNINGS
linux-3.12.74-x86_64: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.79-i686: WARNINGS
linux-3.14.79-x86_64: WARNINGS
linux-3.15.10-i686: WARNINGS
linux-3.15.10-x86_64: WARNINGS
linux-3.16.55-i686: WARNINGS
linux-3.16.55-x86_64: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.100-i686: WARNINGS
linux-3.18.100-x86_64: WARNINGS
linux-3.19.8-i686: WARNINGS
linux-3.19.8-x86_64: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.50-i686: WARNINGS
linux-4.1.50-x86_64: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.99-i686: OK
linux-4.4.99-x86_64: OK
linux-4.5.7-i686: WARNINGS
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: WARNINGS
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.87-i686: OK
linux-4.9.87-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.27-i686: OK
linux-4.14.27-x86_64: OK
linux-4.15.10-i686: OK
linux-4.15.10-x86_64: OK
linux-4.16-rc5-i686: OK
linux-4.16-rc5-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

DVB-T2 support for TVR801 USB stick (Astrometa DVB-T2)

[TPClmml]

Could anyone clarify what is supported on this stick for me?  According to 
https://www.linuxtv.org/wiki/index.php/Astrometa_DVB-T2 DVB-T2 has been 
supported since kernel 4.6.  I tried using a 4.9.9 kernel about a year ago 
and again today using 4.14.30, in both cases with the development tree 
code from git://linuxtv.org/media_build.git.


In both cases I don't get a second adapter exposed for the MN88473 demuxer 
just the /dev/dvb/adapter0 tree for the RTL2832 which only supports DVB-T. 
Here http://www.mklab.rhul.ac.uk/~tom/TVR801-dmesg.txt is the full dmesg 
O/P it got using kernel 4.14.30.



A year ago, for fun, I tried hacking the code a little, without really 
knowing what I was doing but managed to get the following from dmesg,



dvbdev: DVB: registering new adapter (Astrometa DVB-T2)
usb 1-8: media controller created
dvbdev: dvb_create_media_entity: media entity 'dvb-demux' registered.
i2c i2c-13: Added multiplexed i2c bus 14
rtl2832 13-0010: Realtek RTL2832 successfully attached
mn88473 13-0018: Panasonic MN88473 successfully identified
usb 1-8: DVB: registering adapter 0 frontend 0 (Realtek RTL2832 (DVB-T))...
dvbdev: dvb_create_media_entity: media entity 'Realtek RTL2832 (DVB-T)' 
registered.
usb 1-8: DVB: registering adapter 0 frontend 1 (Panasonic MN88473)...
dvbdev: dvb_create_media_entity: media entity 'Panasonic MN88473' registered.


The MN88473 interface I got was recognised by w_scan but was not usable. 
It complained the frequency limits were missing.  Here is an example,



w_scan -X -a /dev/dvb/adapter0/frontend1
w_scan version 20161022 (compiled for DVB API 5.10)
WARNING: could not guess your country. Falling back to 'DE'
guessing country 'DE', use -c  to override
using settings for GERMANY
DVB aerial
DVB-T Europe
scan type TERRESTRIAL, channellist 4
output format czap/tzap/szap/xine
WARNING: could not guess your codepage. Falling back to 'UTF-8'
output charset 'UTF-8', use -C  to override
-_-_-_-_ Getting frontend capabilities-_-_-_-_
Using DVB API 5.10
frontend 'Panasonic MN88473' supports
DVB-T2
INVERSION_AUTO
QAM_AUTO
TRANSMISSION_MODE_AUTO
GUARD_INTERVAL_AUTO
HIERARCHY_AUTO
FEC_AUTO
BANDWIDTH_AUTO not supported, trying 6/7/8 MHz.
This dvb driver is *buggy*: the frequency limits are undefined - please 
report to linuxtv.org

-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
Scanning DVB-T...
Scanning 8MHz frequencies...
474000: (time: 00:00.633)
482000: (time: 00:02.641)
49: (time: 00:04.650)
498000: (time: 00:06.661)
[cut]
842000: (time: 01:32.955)
85: (time: 01:34.960)
858000: (time: 01:36.969)
Scanning DVB-T2...
474000: (time: 01:38.978)
482000: (time: 01:40.989)
49: (time: 01:43.000)
498000: (time: 01:45.010)
[cut]
85: (time: 03:13.285)
858000: (time: 03:15.291)

ERROR: Sorry - i couldn't get any working frequency/transponder
 Nothing to scan!!



After that I gave up.  Any thoughts?

Many thanks
Tom Crane

--
Tom Crane, Dept. Physics, Royal Holloway, University of London, Egham Hill,
Egham, Surrey, TW20 0EX, England.
Email:  t.cr...@rhul.ac.uk

[PATCH v6 11/12] intel-ipu3: Add v4l2 driver based on media framework

[Yong Zhi]

Implement video driver that utilizes v4l2, vb2 queue support
and media controller APIs. The driver exposes single
subdevice and six nodes.

Signed-off-by: Yong Zhi 
Signed-off-by: Ramya Vijaykumar 
---
 drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1089 ++
 1 file changed, 1089 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c

diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c 
b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
new file mode 100644
index ..1095ef8a29a5
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
@@ -0,0 +1,1089 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+
+#include 
+
+#include "ipu3.h"
+#include "ipu3-dmamap.h"
+
+/ v4l2_subdev_ops /
+
+static int ipu3_subdev_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+   struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
+   struct v4l2_rect try_crop = {
+   .top = 0,
+   .left = 0,
+   .height = imgu->nodes[IMGU_NODE_IN].vdev_fmt.fmt.pix_mp.height,
+   .width = imgu->nodes[IMGU_NODE_IN].vdev_fmt.fmt.pix_mp.width,
+   };
+   unsigned int i;
+
+   /* Initialize try_fmt */
+   for (i = 0; i < IMGU_NODE_NUM; i++)
+   *v4l2_subdev_get_try_format(sd, fh->pad, i) =
+   imgu->nodes[i].pad_fmt;
+
+   *v4l2_subdev_get_try_crop(sd, fh->pad, IMGU_NODE_IN) = try_crop;
+
+   return 0;
+}
+
+static int ipu3_subdev_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
+   int r = 0;
+
+   r = imgu_s_stream(imgu, enable);
+   if (!r)
+   imgu->streaming = enable;
+
+   return r;
+}
+
+static int ipu3_subdev_get_fmt(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_format *fmt)
+{
+   struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
+   struct v4l2_mbus_framefmt *mf;
+   u32 pad = fmt->pad;
+
+   if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+   fmt->format = imgu->nodes[pad].pad_fmt;
+   } else {
+   mf = v4l2_subdev_get_try_format(sd, cfg, pad);
+   fmt->format = *mf;
+   }
+
+   return 0;
+}
+
+static int ipu3_subdev_set_fmt(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_format *fmt)
+{
+   struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
+   struct v4l2_mbus_framefmt *mf;
+   u32 pad = fmt->pad;
+
+   if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+   mf = v4l2_subdev_get_try_format(sd, cfg, pad);
+   else
+   mf = >nodes[pad].pad_fmt;
+
+   fmt->format.code = mf->code;
+   /* Clamp the w and h based on the hardware capabilities */
+   if (imgu->subdev_pads[pad].flags & MEDIA_PAD_FL_SOURCE) {
+   fmt->format.width = clamp(fmt->format.width,
+ IPU3_OUTPUT_MIN_WIDTH,
+ IPU3_OUTPUT_MAX_WIDTH);
+   fmt->format.height = clamp(fmt->format.height,
+  IPU3_OUTPUT_MIN_HEIGHT,
+  IPU3_OUTPUT_MAX_HEIGHT);
+   } else {
+   fmt->format.width = clamp(fmt->format.width,
+ IPU3_INPUT_MIN_WIDTH,
+ IPU3_INPUT_MAX_WIDTH);
+   fmt->format.height = clamp(fmt->format.height,
+  IPU3_INPUT_MIN_HEIGHT,
+  IPU3_INPUT_MAX_HEIGHT);
+   }
+
+   *mf = fmt->format;
+
+   return 0;
+}
+
+static int ipu3_subdev_get_selection(struct v4l2_subdev *sd,
+struct v4l2_subdev_pad_config *cfg,
+struct v4l2_subdev_selection *sel)
+{
+   struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
+   struct v4l2_rect *try_sel, *r;
+
+   if (sel->pad != IMGU_NODE_IN)
+   return -EINVAL;
+
+   switch (sel->target) {
+   case V4L2_SEL_TGT_CROP:
+   try_sel = v4l2_subdev_get_try_crop(sd, cfg, sel->pad);
+   r = >rect.eff;
+   break;
+   case V4L2_SEL_TGT_COMPOSE:
+   try_sel = v4l2_subdev_get_try_compose(sd, cfg, sel->pad);
+   r = >rect.bds;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
+   sel->r = *try_sel;
+   else
+   

[PATCH v6 05/12] intel-ipu3: css: Add dma buff pool utility functions

[Yong Zhi]

The pools are used to store previous parameters set by
user with the parameter queue. Due to pipelining,
there needs to be multiple sets (up to four)
of parameters which are queued in a host-to-sp queue.

Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-css-pool.c | 131 +++
 1 file changed, 131 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.c

diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.c 
b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c
new file mode 100644
index ..84f6a7801a01
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+
+#include "ipu3-css-pool.h"
+#include "ipu3-dmamap.h"
+
+int ipu3_css_dma_buffer_resize(struct device *dev, struct ipu3_css_map *map,
+  size_t size)
+{
+   if (map->size < size && map->vaddr) {
+   dev_warn(dev, "dma buffer is resized from %zu to %zu",
+map->size, size);
+
+   ipu3_dmamap_free(dev, map);
+   if (!ipu3_dmamap_alloc(dev, map, size))
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+void ipu3_css_pool_cleanup(struct device *dev, struct ipu3_css_pool *pool)
+{
+   unsigned int i;
+
+   for (i = 0; i < IPU3_CSS_POOL_SIZE; i++)
+   ipu3_dmamap_free(dev, >entry[i].param);
+}
+
+int ipu3_css_pool_init(struct device *dev, struct ipu3_css_pool *pool,
+  size_t size)
+{
+   unsigned int i;
+
+   for (i = 0; i < IPU3_CSS_POOL_SIZE; i++) {
+   /*
+* entry[i].framenum is initialized to INT_MIN so that
+* ipu3_css_pool_check() can treat it as usesable slot.
+*/
+   pool->entry[i].framenum = INT_MIN;
+
+   if (size == 0) {
+   pool->entry[i].param.vaddr = NULL;
+   continue;
+   }
+
+   if (!ipu3_dmamap_alloc(dev, >entry[i].param, size))
+   goto fail;
+   }
+
+   pool->last = IPU3_CSS_POOL_SIZE;
+
+   return 0;
+
+fail:
+   ipu3_css_pool_cleanup(dev, pool);
+   return -ENOMEM;
+}
+
+/*
+ * Check that the following call to pool_get succeeds.
+ * Return negative on error.
+ */
+static int ipu3_css_pool_check(struct ipu3_css_pool *pool, long framenum)
+{
+   /* Get the oldest entry */
+   int n = (pool->last + 1) % IPU3_CSS_POOL_SIZE;
+   long diff = framenum - pool->entry[n].framenum;
+
+   /* if framenum wraps around and becomes smaller than entry n */
+   if (diff < 0)
+   diff += LONG_MAX;
+
+   /*
+* pool->entry[n].framenum stores the frame number where that
+* entry was allocated. If that was allocated more than POOL_SIZE
+* frames back, it is old enough that we know it is no more in
+* use by firmware.
+*/
+   if (diff > IPU3_CSS_POOL_SIZE)
+   return n;
+
+   return -ENOSPC;
+}
+
+/*
+ * Allocate a new parameter from pool at frame number `framenum'.
+ * Release the oldest entry in the pool to make space for the new entry.
+ * Return negative on error.
+ */
+int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum)
+{
+   int n = ipu3_css_pool_check(pool, framenum);
+
+   if (n < 0)
+   return n;
+
+   pool->entry[n].framenum = framenum;
+   pool->last = n;
+
+   return n;
+}
+
+/*
+ * Undo, for all practical purposes, the effect of pool_get().
+ */
+void ipu3_css_pool_put(struct ipu3_css_pool *pool)
+{
+   pool->entry[pool->last].framenum = INT_MIN;
+   pool->last = (pool->last + IPU3_CSS_POOL_SIZE - 1) % IPU3_CSS_POOL_SIZE;
+}
+
+/*
+ * Return the nth entry from last, if that entry has no frame stored,
+ * return a null map instead to indicate frame not available for the entry.
+ */
+const struct ipu3_css_map *
+ipu3_css_pool_last(struct ipu3_css_pool *pool, unsigned int n)
+{
+   static const struct ipu3_css_map null_map = { 0 };
+   int i = (pool->last + IPU3_CSS_POOL_SIZE - n) % IPU3_CSS_POOL_SIZE;
+
+   WARN_ON(n >= IPU3_CSS_POOL_SIZE);
+
+   if (pool->entry[i].framenum < 0)
+   return _map;
+
+   return >entry[i].param;
+}
-- 
2.7.4

[PATCH v6 02/12] intel-ipu3: Add user space API definitions

[Yong Zhi]

Define the structures and macros to be used by public.

Signed-off-by: Yong Zhi 
Signed-off-by: Rajmohan Mani 
---
 include/uapi/linux/intel-ipu3.h | 1403 +++
 1 file changed, 1403 insertions(+)
 create mode 100644 include/uapi/linux/intel-ipu3.h

diff --git a/include/uapi/linux/intel-ipu3.h b/include/uapi/linux/intel-ipu3.h
new file mode 100644
index ..694ef0c8d7a7
--- /dev/null
+++ b/include/uapi/linux/intel-ipu3.h
@@ -0,0 +1,1403 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Intel Corporation */
+
+#ifndef __IPU3_UAPI_H
+#define __IPU3_UAPI_H
+
+#include 
+
+#define IPU3_UAPI_ISP_VEC_ELEMS64
+
+#define IMGU_ABI_PAD   __aligned(IPU3_UAPI_ISP_WORD_BYTES)
+#define IPU3_ALIGN __attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES)))
+
+#define IPU3_UAPI_ISP_WORD_BYTES   32
+#define IPU3_UAPI_MAX_STRIPES  2
+
+/*** ipu3_uapi_stats_3a ***/
+
+#define IPU3_UAPI_MAX_BUBBLE_SIZE  10
+
+#define IPU3_UAPI_AE_COLORS4
+#define IPU3_UAPI_AE_BINS  256
+
+#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8
+#define IPU3_UAPI_AWB_MAX_SETS 60
+#define IPU3_UAPI_AWB_SET_SIZE 0x500
+#define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \
+   (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
+IPU3_UAPI_AWB_MD_ITEM_SIZE)
+#define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
+   (IPU3_UAPI_AWB_MAX_SETS * \
+(IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES))
+
+#define IPU3_UAPI_AF_MAX_SETS  24
+#define IPU3_UAPI_AF_MD_ITEM_SIZE  4
+#define IPU3_UAPI_AF_SPARE_FOR_BUBBLES \
+   (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
+IPU3_UAPI_AF_MD_ITEM_SIZE)
+#define IPU3_UAPI_AF_Y_TABLE_SET_SIZE  0x80
+#define IPU3_UAPI_AF_Y_TABLE_MAX_SIZE \
+   (IPU3_UAPI_AF_MAX_SETS * \
+(IPU3_UAPI_AF_Y_TABLE_SET_SIZE + IPU3_UAPI_AF_SPARE_FOR_BUBBLES) * \
+IPU3_UAPI_MAX_STRIPES)
+
+#define IPU3_UAPI_AWB_FR_MAX_SETS  24
+#define IPU3_UAPI_AWB_FR_MD_ITEM_SIZE  8
+#define IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE0x100
+#define IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES \
+   (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
+IPU3_UAPI_AWB_FR_MD_ITEM_SIZE)
+#define IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE \
+   (IPU3_UAPI_AWB_FR_MAX_SETS * \
+   (IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE + \
+IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES) * IPU3_UAPI_MAX_STRIPES)
+
+struct ipu3_uapi_grid_config {
+   __u8 width; /* 6 or 7 (rgbs_grd_cfg) bits */
+   __u8 height;
+   __u16 block_width_log2:3;
+   __u16 block_height_log2:3;
+   __u16 height_per_slice:8;   /* default value 1 */
+   __u16 x_start;  /* 12 bits */
+   __u16 y_start;
+#define IPU3_UAPI_GRID_START_MASK  ((1 << 12) - 1)
+#define IPU3_UAPI_GRID_Y_START_EN  (1 << 15)
+   __u16 x_end;/* 12 bits */
+   __u16 y_end;
+} __packed;
+
+struct ipu3_uapi_awb_meta_data {
+   __u8 meta_data_buffer[IPU3_UAPI_AWB_MAX_BUFFER_SIZE];
+} __packed;
+
+struct ipu3_uapi_awb_raw_buffer {
+   struct ipu3_uapi_awb_meta_data meta_data;
+} __packed;
+
+struct IPU3_ALIGN ipu3_uapi_awb_config_s {
+   __u16 rgbs_thr_gr;
+   __u16 rgbs_thr_r;
+   __u16 rgbs_thr_gb;
+   __u16 rgbs_thr_b;
+/* controls generation of meta_data (like FF enable/disable) */
+#define IPU3_UAPI_AWB_RGBS_THR_B_EN(1 << 14)
+#define IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT  (1 << 15)
+
+   struct ipu3_uapi_grid_config grid;
+} __packed;
+
+struct ipu3_uapi_ae_raw_buffer {
+   __u32 vals[IPU3_UAPI_AE_BINS * IPU3_UAPI_AE_COLORS];
+} __packed;
+
+struct ipu3_uapi_ae_raw_buffer_aligned {
+   struct ipu3_uapi_ae_raw_buffer buff IPU3_ALIGN;
+} __packed;
+
+struct ipu3_uapi_ae_grid_config {
+   __u8 width;
+   __u8 height;
+   __u8 block_width_log2:4;
+   __u8 block_height_log2:4;
+   __u8 __reserved0:5;
+   __u8 ae_en:1;
+   __u8 rst_hist_array:1;
+   __u8 done_rst_hist_array:1;
+   __u16 x_start;  /* 12 bits */
+   __u16 y_start;
+   __u16 x_end;
+   __u16 y_end;
+} __packed;
+
+struct ipu3_uapi_af_filter_config {
+   struct {
+   __u8 a1;
+   __u8 a2;
+   __u8 a3;
+   __u8 a4;
+   } y1_coeff_0;
+   struct {
+   __u8 a5;
+   __u8 a6;
+   __u8 a7;
+   __u8 a8;
+   } y1_coeff_1;
+   struct {
+   __u8 a9;
+   __u8 a10;
+   __u8 a11;
+   __u8 

[PATCH v6 04/12] intel-ipu3: Implement DMA mapping functions

[Yong Zhi]

From: Tomasz Figa 

This driver uses IOVA space for buffer mapping through IPU3 MMU
to transfer data between imaging pipelines and system DDR.

Signed-off-by: Tomasz Figa 
Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-css-pool.h |  36 
 drivers/media/pci/intel/ipu3/ipu3-dmamap.c   | 280 +++
 drivers/media/pci/intel/ipu3/ipu3-dmamap.h   |  22 +++
 drivers/media/pci/intel/ipu3/ipu3.h  | 151 +++
 4 files changed, 489 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.h
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3.h

diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.h 
b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
new file mode 100644
index ..4b22e0856232
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Intel Corporation */
+
+#ifndef __IPU3_UTIL_H
+#define __IPU3_UTIL_H
+
+struct device;
+
+#define IPU3_CSS_POOL_SIZE 4
+
+struct ipu3_css_map {
+   size_t size;
+   void *vaddr;
+   dma_addr_t daddr;
+   struct vm_struct *vma;
+};
+
+struct ipu3_css_pool {
+   struct {
+   struct ipu3_css_map param;
+   long framenum;
+   } entry[IPU3_CSS_POOL_SIZE];
+   unsigned int last; /* Latest entry */
+};
+
+int ipu3_css_dma_buffer_resize(struct device *dev, struct ipu3_css_map *map,
+  size_t size);
+void ipu3_css_pool_cleanup(struct device *dev, struct ipu3_css_pool *pool);
+int ipu3_css_pool_init(struct device *dev, struct ipu3_css_pool *pool,
+  size_t size);
+int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum);
+void ipu3_css_pool_put(struct ipu3_css_pool *pool);
+const struct ipu3_css_map *ipu3_css_pool_last(struct ipu3_css_pool *pool,
+ unsigned int last);
+
+#endif
diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c 
b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
new file mode 100644
index ..b2bc5d00debc
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
@@ -0,0 +1,280 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018 Google, Inc.
+ *
+ * Author: Tomasz Figa 
+ * Author: Yong Zhi 
+ */
+
+#include 
+
+#include "ipu3.h"
+#include "ipu3-css-pool.h"
+#include "ipu3-mmu.h"
+
+/*
+ * Free a buffer allocated by ipu3_dmamap_alloc_buffer()
+ */
+static void ipu3_dmamap_free_buffer(struct page **pages,
+   size_t size)
+{
+   int count = size >> PAGE_SHIFT;
+
+   while (count--)
+   __free_page(pages[count]);
+   kvfree(pages);
+}
+
+/*
+ * Based on the implementation of __iommu_dma_alloc_pages()
+ * defined in drivers/iommu/dma-iommu.c
+ */
+static struct page **ipu3_dmamap_alloc_buffer(size_t size,
+ unsigned long order_mask,
+ gfp_t gfp)
+{
+   struct page **pages;
+   unsigned int i = 0, count = size >> PAGE_SHIFT;
+   const gfp_t high_order_gfp = __GFP_NOWARN | __GFP_NORETRY;
+
+   /* Allocate mem for array of page ptrs */
+   pages = kvmalloc_array(count, sizeof(struct page *), GFP_KERNEL);
+
+   if (!pages)
+   return NULL;
+
+   order_mask &= (2U << MAX_ORDER) - 1;
+   if (!order_mask)
+   return NULL;
+
+   gfp |= __GFP_HIGHMEM | __GFP_ZERO;
+
+   while (count) {
+   struct page *page = NULL;
+   unsigned int order_size;
+
+   for (order_mask &= (2U << __fls(count)) - 1;
+order_mask; order_mask &= ~order_size) {
+   unsigned int order = __fls(order_mask);
+
+   order_size = 1U << order;
+   page = alloc_pages((order_mask - order_size) ?
+  gfp | high_order_gfp : gfp, order);
+   if (!page)
+   continue;
+   if (!order)
+   break;
+   if (!PageCompound(page)) {
+   split_page(page, order);
+   break;
+   }
+
+   __free_pages(page, order);
+   }
+   if (!page) {
+   ipu3_dmamap_free_buffer(pages, i << PAGE_SHIFT);
+   return NULL;
+   }
+   count -= order_size;
+   while (order_size--)
+   pages[i++] = page++;
+   }
+
+   return pages;
+}
+

[PATCH v6 01/12] v4l: Add Intel IPU3 meta buffer formats

[Yong Zhi]

Add IPU3-specific meta formats for parameter
processing and 3A statistics:

  V4L2_META_FMT_IPU3_PARAMS
  V4L2_META_FMT_IPU3_STAT_3A

Signed-off-by: Yong Zhi 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
 include/uapi/linux/videodev2.h   | 4 
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index ec98b93b2a55..5360b3915af7 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1257,6 +1257,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_META_FMT_VSP1_HGO:descr = "R-Car VSP1 1-D Histogram"; 
break;
case V4L2_META_FMT_VSP1_HGT:descr = "R-Car VSP1 2-D Histogram"; 
break;
case V4L2_META_FMT_UVC: descr = "UVC payload header metadata"; 
break;
+   case V4L2_META_FMT_IPU3_PARAMS: descr = "IPU3 processing parameters"; 
break;
+   case V4L2_META_FMT_IPU3_STAT_3A:descr = "IPU3 3A statistics"; 
break;
 
default:
/* Compressed formats */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e20d10df75c1..83ef5a0fbdc2 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -698,6 +698,10 @@ struct v4l2_pix_format {
 #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car 
VSP1 2-D Histogram */
 #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC 
Payload Header metadata */
 
+/* Vendor specific - used for IPU3 camera sub-system */
+#define V4L2_META_FMT_IPU3_PARAMS  v4l2_fourcc('i', 'p', '3', 'p') /* IPU3 
params */
+#define V4L2_META_FMT_IPU3_STAT_3A v4l2_fourcc('i', 'p', '3', 's') /* IPU3 
3A statistics */
+
 /* priv field value to indicates that subsequent fields are valid. */
 #define V4L2_PIX_FMT_PRIV_MAGIC0xfeedcafe
 
-- 
2.7.4

[PATCH v6 03/12] intel-ipu3: mmu: Implement driver

[Yong Zhi]

From: Tomasz Figa 

This driver translates IO virtual address to physical
address based on two levels page tables.

Signed-off-by: Tomasz Figa 
Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-mmu.c | 560 
 drivers/media/pci/intel/ipu3/ipu3-mmu.h |  28 ++
 2 files changed, 588 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h

diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c 
b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
new file mode 100644
index ..a4b3e1680bbb
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
@@ -0,0 +1,560 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Intel Corporation.
+ * Copyright (C) 2018 Google, Inc.
+ *
+ * Author: Tuukka Toivonen 
+ * Author: Sakari Ailus 
+ * Author: Samu Onkalo 
+ * Author: Tomasz Figa 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "ipu3-mmu.h"
+
+#define IPU3_PAGE_SHIFT12
+#define IPU3_PAGE_SIZE (1UL << IPU3_PAGE_SHIFT)
+
+#define IPU3_PT_BITS   10
+#define IPU3_PT_PTES   (1UL << IPU3_PT_BITS)
+#define IPU3_PT_SIZE   (IPU3_PT_PTES << 2)
+#define IPU3_PT_ORDER  (IPU3_PT_SIZE >> PAGE_SHIFT)
+
+#define IPU3_ADDR2PTE(addr)((addr) >> IPU3_PAGE_SHIFT)
+#define IPU3_PTE2ADDR(pte) ((phys_addr_t)(pte) << IPU3_PAGE_SHIFT)
+
+#define IPU3_L2PT_SHIFTIPU3_PT_BITS
+#define IPU3_L2PT_MASK ((1UL << IPU3_L2PT_SHIFT) - 1)
+
+#define IPU3_L1PT_SHIFTIPU3_PT_BITS
+#define IPU3_L1PT_MASK ((1UL << IPU3_L1PT_SHIFT) - 1)
+
+#define IPU3_MMU_ADDRESS_BITS  (IPU3_PAGE_SHIFT + \
+IPU3_L2PT_SHIFT + \
+IPU3_L1PT_SHIFT)
+
+#define IMGU_REG_BASE  0x4000
+#define REG_TLB_INVALIDATE (IMGU_REG_BASE + 0x300)
+#define TLB_INVALIDATE 1
+#define REG_L1_PHYS(IMGU_REG_BASE + 0x304) /* 27-bit pfn */
+#define REG_GP_HALT(IMGU_REG_BASE + 0x5dc)
+#define REG_GP_HALTED  (IMGU_REG_BASE + 0x5e0)
+
+struct ipu3_mmu {
+   struct device *dev;
+   void __iomem *base;
+   /* protect access to l2pts, l1pt */
+   spinlock_t lock;
+
+   void *dummy_page;
+   u32 dummy_page_pteval;
+
+   u32 *dummy_l2pt;
+   u32 dummy_l2pt_pteval;
+
+   u32 **l2pts;
+   u32 *l1pt;
+
+   struct ipu3_mmu_info geometry;
+};
+
+static inline struct ipu3_mmu *to_ipu3_mmu(struct ipu3_mmu_info *info)
+{
+   return container_of(info, struct ipu3_mmu, geometry);
+}
+
+/**
+ * ipu3_mmu_tlb_invalidate - invalidate translation look-aside buffer
+ * @mmu: MMU to perform the invalidate operation on
+ *
+ * This function invalidates the whole TLB. Must be called when the hardware
+ * is powered on.
+ */
+static void ipu3_mmu_tlb_invalidate(struct ipu3_mmu *mmu)
+{
+   writel(TLB_INVALIDATE, mmu->base + REG_TLB_INVALIDATE);
+}
+
+static void call_if_ipu3_is_powered(struct ipu3_mmu *mmu,
+   void (*func)(struct ipu3_mmu *mmu))
+{
+   pm_runtime_get_noresume(mmu->dev);
+   if (pm_runtime_active(mmu->dev))
+   func(mmu);
+   pm_runtime_put(mmu->dev);
+}
+
+/**
+ * ipu3_mmu_set_halt - set CIO gate halt bit
+ * @mmu: MMU to set the CIO gate bit in.
+ * @halt: Desired state of the gate bit.
+ *
+ * This function sets the CIO gate bit that controls whether external memory
+ * accesses are allowed. Must be called when the hardware is powered on.
+ */
+static void ipu3_mmu_set_halt(struct ipu3_mmu *mmu, bool halt)
+{
+   int ret;
+   u32 val;
+
+   writel(halt, mmu->base + REG_GP_HALT);
+   ret = readl_poll_timeout(mmu->base + REG_GP_HALTED,
+val, (val & 1) == halt, 1000, 10);
+
+   if (ret)
+   dev_err(mmu->dev, "failed to %s CIO gate halt\n",
+   halt ? "set" : "clear");
+}
+
+/**
+ * ipu3_mmu_alloc_page_table - allocate a pre-filled page table
+ * @pteval: Value to initialize for page table entries with.
+ *
+ * Return: Pointer to allocated page table or NULL on failure.
+ */
+static u32 *ipu3_mmu_alloc_page_table(u32 pteval)
+{
+   u32 *pt;
+   int pte;
+
+   pt = (u32 *)__get_free_page(GFP_KERNEL);
+   if (!pt)
+   return NULL;
+
+   for (pte = 0; pte < IPU3_PT_PTES; pte++)
+   pt[pte] = pteval;
+
+   set_memory_uc((unsigned long int)pt, IPU3_PT_ORDER);
+
+   return pt;
+}
+
+/**
+ * ipu3_mmu_free_page_table - free page table
+ * @pt: Page table to free.
+ */
+static void ipu3_mmu_free_page_table(u32 *pt)
+{
+   set_memory_wb((unsigned long int)pt, IPU3_PT_ORDER);
+   free_page((unsigned long)pt);
+}
+

[PATCH v6 06/12] intel-ipu3: css: Add support for firmware management

[Yong Zhi]

Introduce functions to load and install ImgU FW blobs.

Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-abi.h| 1888 
 drivers/media/pci/intel/ipu3/ipu3-css-fw.c |  261 
 drivers/media/pci/intel/ipu3/ipu3-css-fw.h |  198 +++
 3 files changed, 2347 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h

diff --git a/drivers/media/pci/intel/ipu3/ipu3-abi.h 
b/drivers/media/pci/intel/ipu3/ipu3-abi.h
new file mode 100644
index ..24102647a89e
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-abi.h
@@ -0,0 +1,1888 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Intel Corporation */
+
+#ifndef __IPU3_ABI_H
+#define __IPU3_ABI_H
+
+#include 
+
+/*** IMGU Hardware information ***/
+
+typedef __u32 imgu_addr_t;
+
+#define IMGU_ISP_VMEM_ALIGN128
+#define IMGU_DVS_BLOCK_W   64
+#define IMGU_DVS_BLOCK_H   32
+#define IMGU_GDC_BUF_X (2 * IMGU_DVS_BLOCK_W)
+#define IMGU_GDC_BUF_Y IMGU_DVS_BLOCK_H
+/* n = 0..1 */
+#define IMGU_SP_PMEM_BASE(n)   (0x2 + (n) * 0x4000)
+#define IMGU_MAX_BQ_GRID_WIDTH 80
+#define IMGU_MAX_BQ_GRID_HEIGHT60
+#define IMGU_OBGRID_TILE_SIZE  16
+#define IMGU_PIXELS_PER_WORD   50
+#define IMGU_BYTES_PER_WORD64
+#define IMGU_STRIPE_FIXED_HALF_OVERLAP 2
+#define IMGU_SHD_SETS  3
+#define IMGU_BDS_MIN_CLIP_VAL  0
+#define IMGU_BDS_MAX_CLIP_VAL  2
+
+#define IMGU_ABI_AWB_MAX_CELLS_PER_SET 160
+#define IMGU_ABI_AF_MAX_CELLS_PER_SET  32
+#define IMGU_ABI_AWB_FR_MAX_CELLS_PER_SET  32
+
+#define IMGU_ABI_ACC_OP_IDLE   0
+#define IMGU_ABI_ACC_OP_END_OF_ACK 1
+#define IMGU_ABI_ACC_OP_END_OF_OPS 2
+#define IMGU_ABI_ACC_OP_NO_OPS 3
+
+#define IMGU_ABI_ACC_OPTYPE_PROCESS_LINES  0
+#define IMGU_ABI_ACC_OPTYPE_TRANSFER_DATA  1
+
+/* Register definitions */
+
+/* PM_CTRL_0_5_0_IMGHMMADR */
+#define IMGU_REG_PM_CTRL   0x0
+#define IMGU_PM_CTRL_START BIT(0)
+#define IMGU_PM_CTRL_CFG_DONE  BIT(1)
+#define IMGU_PM_CTRL_RACE_TO_HALT  BIT(2)
+#define IMGU_PM_CTRL_NACK_ALL  BIT(3)
+#define IMGU_PM_CTRL_CSS_PWRDN BIT(4)
+#define IMGU_PM_CTRL_RST_AT_EOFBIT(5)
+#define IMGU_PM_CTRL_FORCE_HALTBIT(6)
+#define IMGU_PM_CTRL_FORCE_UNHALT  BIT(7)
+#define IMGU_PM_CTRL_FORCE_PWRDN   BIT(8)
+#define IMGU_PM_CTRL_FORCE_RESET   BIT(9)
+
+/* SYSTEM_REQ_0_5_0_IMGHMMADR */
+#define IMGU_REG_SYSTEM_REQ0x18
+#define IMGU_SYSTEM_REQ_FREQ_MASK  0x3f
+#define IMGU_SYSTEM_REQ_FREQ_DIVIDER   25
+#define IMGU_REG_INT_STATUS0x30
+#define IMGU_REG_INT_ENABLE0x34
+#define IMGU_REG_INT_CSS_IRQ   (1 << 31)
+/* STATE_0_5_0_IMGHMMADR */
+#define IMGU_REG_STATE 0x130
+#define IMGU_STATE_HALT_STSBIT(0)
+#define IMGU_STATE_IDLE_STSBIT(1)
+#define IMGU_STATE_POWER_UPBIT(2)
+#define IMGU_STATE_POWER_DOWN  BIT(3)
+#define IMGU_STATE_CSS_BUSY_MASK   0xc0
+#define IMGU_STATE_PM_FSM_MASK 0x180
+#define IMGU_STATE_PWRDNM_FSM_MASK 0x1E0
+/* PM_STS_0_5_0_IMGHMMADR */
+#define IMGU_REG_PM_STS0x140
+
+#define IMGU_REG_BASE  0x4000
+
+#define IMGU_REG_ISP_CTRL  (IMGU_REG_BASE + 0x00)
+#define IMGU_CTRL_RST  BIT(0)
+#define IMGU_CTRL_STARTBIT(1)
+#define IMGU_CTRL_BREAKBIT(2)
+#define IMGU_CTRL_RUN  BIT(3)
+#define IMGU_CTRL_BROKEN   BIT(4)
+#define IMGU_CTRL_IDLE BIT(5)
+#define IMGU_CTRL_SLEEPING BIT(6)
+#define IMGU_CTRL_STALLING BIT(7)
+#define IMGU_CTRL_IRQ_CLEARBIT(8)
+#define IMGU_CTRL_IRQ_READYBIT(10)
+#define IMGU_CTRL_IRQ_SLEEPING BIT(11)
+#define IMGU_CTRL_ICACHE_INV   BIT(12)
+#define IMGU_CTRL_IPREFETCH_EN BIT(13)
+#define IMGU_REG_ISP_START_ADDR(IMGU_REG_BASE + 0x04)
+#define IMGU_REG_ISP_ICACHE_ADDR   (IMGU_REG_BASE + 0x10)
+#define IMGU_REG_ISP_PC(IMGU_REG_BASE + 0x1c)
+
+/* SP 

[PATCH v6 00/12] Intel IPU3 ImgU patchset

[Yong Zhi]

Hi,

This series adds support for the Intel IPU3 (Image Processing Unit)
ImgU which is essentially a modern memory-to-memory ISP. It implements
raw Bayer to YUV image format conversion as well as a large number of
other pixel processing algorithms for improving the image quality.

Meta data formats are defined for image statistics (3A, i.e. automatic
white balance, exposure and focus, histogram and local area contrast
enhancement) as well as for the pixel processing algorithm parameters.
The documentation for these formats is currently not included in the
patchset but will be added in a future version of this set.

The algorithm parameters need to be considered specific to a given frame
and typically a large number of these parameters change on frame to frame
basis. Additionally, the parameters are highly structured (and not a flat
space of independent configuration primitives). They also reflect the
data structures used by the firmware and the hardware. On top of that,
the algorithms require highly specialized user space to make meaningful
use of them. For these reasons it has been chosen video buffers to pass
the parameters to the device.

On individual patches:

The heart of ImgU is the CSS, or Camera Subsystem, which contains the
image processors and HW accelerators.

The 3A statistics and other firmware parameter computation related
functions are implemented in patch 8.

All h/w programming related code can be found in patch 9.

To access DDR via ImgU's own memory space, IPU3 is also equipped with
its own MMU unit, the driver is implemented in patch 3.

Patch 4 uses above driver for DMA mapping operation.

5 and 6 provide some utility functions and manage IPU3 fw request and
install.

The firmware which is called ipu3-fw.bin can be downloaded from:

git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
(commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f)

Patch 7-10 are basically IPU3 CSS specific implementations:

9 and 10 are of the same file, the latter implements interface
functions for access fw & hw capabilities.

Patch 11 is the v4l2 driver that has a dependency on Sakari's
V4L2_BUF_TYPE_META_OUTPUT support:



Patch 12 implements the PCI driver and glues everything together.

Patch 2 is the uAPI header file, link to user space implementation:

git clone https://chromium.googlesource.com/chromiumos/platform/arc-camera

ImgU media topology print:

media-ctl -d /dev/media0 -p
Media controller API version 4.16.0

Media device information

driver  ipu3-imgu
model   ipu3-imgu
serial  
bus info:00:05.0
hw revision 0x0
driver version  4.16.0

Device topology
- entity 1: ipu3-imgu (6 pads, 6 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown
 crop:(0,0)/1920x1080
 compose:(0,0)/1920x1080]
<- "ipu3-imgu input":0 [ENABLED,IMMUTABLE]
pad1: Sink
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
<- "ipu3-imgu parameters":0 []
pad2: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "ipu3-imgu output":0 []
pad3: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "ipu3-imgu viewfinder":0 []
pad4: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "ipu3-imgu postview":0 []
pad5: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "ipu3-imgu 3a stat":0 []

- entity 10: ipu3-imgu input (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video0
pad0: Source
-> "ipu3-imgu":0 [ENABLED,IMMUTABLE]

- entity 16: ipu3-imgu parameters (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video1
pad0: Source
-> "ipu3-imgu":1 []

- entity 22: ipu3-imgu output (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video2
pad0: Sink
<- "ipu3-imgu":2 []

- entity 28: ipu3-imgu viewfinder (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video3
pad0: Sink
<- "ipu3-imgu":3 []

- entity 34: ipu3-imgu postview (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video4
pad0: Sink
<- "ipu3-imgu":4 []

- entity 40: ipu3-imgu 3a stat (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video5
pad0: Sink
<- "ipu3-imgu":5 []

v4l2-compliance utility is compiled with 

[PATCH 08/12] intel-ipu3: css: Compute and program ccs

[Yong Zhi]

A collection of routines that are mainly used
to calculate the parameters for accelerator cluster.

Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-css-params.c | 2890 
 drivers/media/pci/intel/ipu3/ipu3-css-params.h |   25 +
 drivers/media/pci/intel/ipu3/ipu3-css.h|  205 ++
 3 files changed, 3120 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-params.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-params.h
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.h

diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-params.c 
b/drivers/media/pci/intel/ipu3/ipu3-css-params.c
new file mode 100644
index ..0492a87868ec
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-css-params.c
@@ -0,0 +1,2890 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+
+#include "ipu3-css.h"
+#include "ipu3-css-fw.h"
+#include "ipu3-tables.h"
+
+#define sqr(x) ((x) * (x))
+#define DIV_ROUND_CLOSEST_DOWN(a, b)   (((a) + ((b) / 2) - 1) / (b))
+#define roundclosest_down(a, b)(DIV_ROUND_CLOSEST_DOWN(a, b) * 
(b))
+
+struct ipu3_css_scaler_info {
+   unsigned int phase_step;/* Same for luma/chroma */
+   int exp_shift;
+
+   unsigned int phase_init;/* luma/chroma dependent */
+   int pad_left;
+   int pad_right;
+   int crop_left;
+   int crop_top;
+};
+
+static unsigned int ipu3_css_scaler_get_exp(unsigned int counter,
+   unsigned int divider)
+{
+   int i = fls(divider) - fls(counter);
+
+   if (i <= 0)
+   return 0;
+
+   if (divider >> i < counter)
+   i = i - 1;
+
+   return i;
+}
+
+/* Set up the CSS scaler look up table */
+static void
+ipu3_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
+ unsigned int output_width, int phase_step_correction,
+ const int *coeffs, unsigned int coeffs_size,
+ s8 coeff_lut[], struct ipu3_css_scaler_info *info)
+{
+   int tap, phase, phase_sum_left, phase_sum_right;
+   int exponent = ipu3_css_scaler_get_exp(output_width, input_width);
+   int mantissa = (1 << exponent) * output_width;
+   unsigned int phase_step;
+
+   if (input_width == output_width) {
+   for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
+   for (tap = 0; tap < taps; tap++) {
+   coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap]
+   = 0;
+   }
+   }
+
+   info->phase_step = IMGU_SCALER_PHASES *
+   (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF);
+   info->exp_shift = 0;
+   info->pad_left = 0;
+   info->pad_right = 0;
+   info->phase_init = 0;
+   info->crop_left = 0;
+   info->crop_top = 0;
+   return;
+   }
+
+   for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
+   for (tap = 0; tap < taps; tap++) {
+   /* flip table to for convolution reverse indexing */
+   s64 coeff = coeffs[coeffs_size -
+   ((tap * (coeffs_size / taps)) + phase) - 1];
+   coeff *= mantissa;
+   coeff = div64_long(coeff, input_width);
+
+   /* Add +"0.5" */
+   coeff += 1 << (IMGU_SCALER_COEFF_BITS - 1);
+   coeff >>= IMGU_SCALER_COEFF_BITS;
+
+   coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap] =
+   coeff;
+   }
+   }
+
+   phase_step = IMGU_SCALER_PHASES *
+   (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF) *
+   output_width / input_width;
+   phase_step += phase_step_correction;
+   phase_sum_left = (taps / 2 * IMGU_SCALER_PHASES *
+   (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF)) -
+   (1 << (IMGU_SCALER_PHASE_COUNTER_PREC_REF - 1));
+   phase_sum_right = (taps / 2 * IMGU_SCALER_PHASES *
+   (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF)) +
+   (1 << (IMGU_SCALER_PHASE_COUNTER_PREC_REF - 1));
+
+   info->exp_shift = IMGU_SCALER_MAX_EXPONENT_SHIFT - exponent;
+   info->pad_left = (phase_sum_left % phase_step == 0) ?
+   phase_sum_left / phase_step - 1 : phase_sum_left / phase_step;
+   info->pad_right = (phase_sum_right % phase_step == 0) ?
+   phase_sum_right / phase_step - 1 : phase_sum_right / phase_step;
+   info->phase_init = phase_sum_left - phase_step * info->pad_left;
+   info->phase_step = phase_step;
+   info->crop_left = taps - 1;
+   

[PATCH v6 10/12] intel-ipu3: Add css pipeline programming

[Yong Zhi]

This provides helper library to be used by
v4l2 level to program imaging pipelines and
control the streaming.

Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-css.c | 1752 +++
 1 file changed, 1752 insertions(+)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-css.c 
b/drivers/media/pci/intel/ipu3/ipu3-css.c
index 164830fc91ad..48fbc037c4a3 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-css.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-css.c
@@ -16,6 +16,173 @@
 IMGU_IRQCTRL_IRQ_SW_PIN(0) | \
 IMGU_IRQCTRL_IRQ_SW_PIN(1))
 
+#define IPU3_CSS_FORMAT_BPP_DEN50  /* Denominator */
+
+/* Some sane limits for resolutions */
+#define IPU3_CSS_MIN_RES   32
+#define IPU3_CSS_MAX_H 3136
+#define IPU3_CSS_MAX_W 4224
+
+/* filter size from graph settings is fixed as 4 */
+#define FILTER_SIZE 4
+#define MIN_ENVELOPE8
+
+/*
+ * pre-allocated buffer size for CSS ABI, auxiliary frames
+ * after BDS and before GDC. Those values should be tuned
+ * to big enough to avoid buffer re-allocation when
+ * streaming to lower streaming latency.
+ */
+#define CSS_ABI_SIZE136
+#define CSS_BDS_SIZE(4480 * 3200 * 3)
+#define CSS_GDC_SIZE(4224 * 3200 * 12 / 8)
+
+#define IPU3_CSS_QUEUE_TO_FLAGS(q) (1 << (q))
+#define IPU3_CSS_FORMAT_FL_IN  \
+   IPU3_CSS_QUEUE_TO_FLAGS(IPU3_CSS_QUEUE_IN)
+#define IPU3_CSS_FORMAT_FL_OUT \
+   IPU3_CSS_QUEUE_TO_FLAGS(IPU3_CSS_QUEUE_OUT)
+#define IPU3_CSS_FORMAT_FL_VF  \
+   IPU3_CSS_QUEUE_TO_FLAGS(IPU3_CSS_QUEUE_VF)
+
+/* Formats supported by IPU3 Camera Sub System */
+static const struct ipu3_css_format ipu3_css_formats[] = {
+   {
+   .pixelformat = V4L2_PIX_FMT_NV12,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+   .frame_format = IMGU_ABI_FRAME_FORMAT_NV12,
+   .osys_format = IMGU_ABI_OSYS_FORMAT_NV12,
+   .osys_tiling = IMGU_ABI_OSYS_TILING_NONE,
+   .bytesperpixel_num = 1 * IPU3_CSS_FORMAT_BPP_DEN,
+   .chroma_decim = 4,
+   .width_align = IPU3_UAPI_ISP_VEC_ELEMS,
+   .flags = IPU3_CSS_FORMAT_FL_OUT | IPU3_CSS_FORMAT_FL_VF,
+   }, {
+   /* Each 32 bytes contains 25 10-bit pixels */
+   .pixelformat = V4L2_PIX_FMT_IPU3_SBGGR10,
+   .colorspace = V4L2_COLORSPACE_RAW,
+   .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
+   .bayer_order = IMGU_ABI_BAYER_ORDER_BGGR,
+   .bit_depth = 10,
+   .bytesperpixel_num = 64,
+   .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
+   .flags = IPU3_CSS_FORMAT_FL_IN,
+   }, {
+   .pixelformat = V4L2_PIX_FMT_IPU3_SGBRG10,
+   .colorspace = V4L2_COLORSPACE_RAW,
+   .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
+   .bayer_order = IMGU_ABI_BAYER_ORDER_GBRG,
+   .bit_depth = 10,
+   .bytesperpixel_num = 64,
+   .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
+   .flags = IPU3_CSS_FORMAT_FL_IN,
+   }, {
+   .pixelformat = V4L2_PIX_FMT_IPU3_SGRBG10,
+   .colorspace = V4L2_COLORSPACE_RAW,
+   .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
+   .bayer_order = IMGU_ABI_BAYER_ORDER_GRBG,
+   .bit_depth = 10,
+   .bytesperpixel_num = 64,
+   .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
+   .flags = IPU3_CSS_FORMAT_FL_IN,
+   }, {
+   .pixelformat = V4L2_PIX_FMT_IPU3_SRGGB10,
+   .colorspace = V4L2_COLORSPACE_RAW,
+   .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
+   .bayer_order = IMGU_ABI_BAYER_ORDER_RGGB,
+   .bit_depth = 10,
+   .bytesperpixel_num = 64,
+   .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
+   .flags = IPU3_CSS_FORMAT_FL_IN,
+   },
+};
+
+static const struct {
+   enum imgu_abi_queue_id qid;
+   size_t ptr_ofs;
+} ipu3_css_queues[IPU3_CSS_QUEUES] = {
+   [IPU3_CSS_QUEUE_IN] = {
+   IMGU_ABI_QUEUE_C_ID,
+   offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
+   },
+   [IPU3_CSS_QUEUE_OUT] = {
+   IMGU_ABI_QUEUE_D_ID,
+   offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
+   },
+   [IPU3_CSS_QUEUE_VF] = {
+   IMGU_ABI_QUEUE_E_ID,
+   offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
+   },
+   [IPU3_CSS_QUEUE_STAT_3A] = {
+   IMGU_ABI_QUEUE_F_ID,
+   offsetof(struct imgu_abi_buffer, payload.s3a.data_ptr)
+   },
+};
+
+/* Initialize queue based on given format, adjust format as needed */
+static int 

[PATCH v6 09/12] intel-ipu3: css: Initialize css hardware

[Yong Zhi]

This patch implements the functions to initialize
and configure IPU3 h/w such as clock, irq and power.

Signed-off-by: Yong Zhi 
Signed-off-by: Tomasz Figa 
---
 drivers/media/pci/intel/ipu3/ipu3-css.c | 537 
 1 file changed, 537 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.c

diff --git a/drivers/media/pci/intel/ipu3/ipu3-css.c 
b/drivers/media/pci/intel/ipu3/ipu3-css.c
new file mode 100644
index ..164830fc91ad
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-css.c
@@ -0,0 +1,537 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+
+#include "ipu3-css.h"
+#include "ipu3-css-fw.h"
+#include "ipu3-css-params.h"
+#include "ipu3-dmamap.h"
+#include "ipu3-tables.h"
+
+/* IRQ configuration */
+#define IMGU_IRQCTRL_IRQ_MASK  (IMGU_IRQCTRL_IRQ_SP1 | \
+IMGU_IRQCTRL_IRQ_SP2 | \
+IMGU_IRQCTRL_IRQ_SW_PIN(0) | \
+IMGU_IRQCTRL_IRQ_SW_PIN(1))
+
+/*** css hw ***/
+
+/* In the style of writesl() defined in include/asm-generic/io.h */
+static inline void writes(const void *mem, ssize_t count, void __iomem *addr)
+{
+   if (count >= 4) {
+   const u32 *buf = mem;
+
+   count /= 4;
+   do {
+   writel(*buf++, addr);
+   addr += 4;
+   } while (--count);
+   }
+}
+
+/* Wait until register `reg', masked with `mask', becomes `cmp' */
+static int ipu3_hw_wait(void __iomem *base, int reg, u32 mask, u32 cmp)
+{
+   u32 val;
+
+   return readl_poll_timeout(base + reg, val, (val & mask) == cmp,
+ 1000, 100 * 1000);
+}
+
+/* Initialize the IPU3 CSS hardware and associated h/w blocks */
+
+int ipu3_css_set_powerup(struct device *dev, void __iomem *base)
+{
+   static const unsigned int freq = 450;
+   u32 pm_ctrl, state, val;
+
+   dev_dbg(dev, "%s\n", __func__);
+   /* Clear the CSS busy signal */
+   readl(base + IMGU_REG_GP_BUSY);
+   writel(0, base + IMGU_REG_GP_BUSY);
+
+   /* Wait for idle signal */
+   if (ipu3_hw_wait(base, IMGU_REG_STATE, IMGU_STATE_IDLE_STS,
+IMGU_STATE_IDLE_STS)) {
+   dev_err(dev, "failed to set CSS idle\n");
+   goto fail;
+   }
+
+   /* Reset the css */
+   writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_FORCE_RESET,
+  base + IMGU_REG_PM_CTRL);
+
+   usleep_range(200, 300);
+
+   /** Prepare CSS */
+
+   pm_ctrl = readl(base + IMGU_REG_PM_CTRL);
+   state = readl(base + IMGU_REG_STATE);
+
+   dev_dbg(dev, "CSS pm_ctrl 0x%x state 0x%x (power %s)\n",
+   pm_ctrl, state, state & IMGU_STATE_POWER_DOWN ? "down" : "up");
+
+   /* Power up CSS using wrapper */
+   if (state & IMGU_STATE_POWER_DOWN) {
+   writel(IMGU_PM_CTRL_RACE_TO_HALT | IMGU_PM_CTRL_START,
+  base + IMGU_REG_PM_CTRL);
+   if (ipu3_hw_wait(base, IMGU_REG_PM_CTRL,
+IMGU_PM_CTRL_START, 0)) {
+   dev_err(dev, "failed to power up CSS\n");
+   goto fail;
+   }
+   usleep_range(2000, 3000);
+   } else {
+   writel(IMGU_PM_CTRL_RACE_TO_HALT, base + IMGU_REG_PM_CTRL);
+   }
+
+   /* Set the busy bit */
+   writel(readl(base + IMGU_REG_GP_BUSY) | 1, base + IMGU_REG_GP_BUSY);
+
+   /* Set CSS clock frequency */
+   pm_ctrl = readl(base + IMGU_REG_PM_CTRL);
+   val = pm_ctrl & ~(IMGU_PM_CTRL_CSS_PWRDN | IMGU_PM_CTRL_RST_AT_EOF);
+   writel(val, base + IMGU_REG_PM_CTRL);
+   writel(0, base + IMGU_REG_GP_BUSY);
+   if (ipu3_hw_wait(base, IMGU_REG_STATE,
+IMGU_STATE_PWRDNM_FSM_MASK, 0)) {
+   dev_err(dev, "failed to pwrdn CSS\n");
+   goto fail;
+   }
+   val = (freq / IMGU_SYSTEM_REQ_FREQ_DIVIDER) & IMGU_SYSTEM_REQ_FREQ_MASK;
+   writel(val, base + IMGU_REG_SYSTEM_REQ);
+   writel(1, base + IMGU_REG_GP_BUSY);
+   writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_FORCE_HALT,
+  base + IMGU_REG_PM_CTRL);
+   if (ipu3_hw_wait(base, IMGU_REG_STATE, IMGU_STATE_HALT_STS,
+IMGU_STATE_HALT_STS)) {
+   dev_err(dev, "failed to halt CSS\n");
+   goto fail;
+   }
+
+   writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_START,
+  base + IMGU_REG_PM_CTRL);
+   if (ipu3_hw_wait(base, IMGU_REG_PM_CTRL, IMGU_PM_CTRL_START, 0)) {
+   dev_err(dev, "failed to start CSS\n");
+   goto fail;
+   }
+   writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_FORCE_UNHALT,
+  base + IMGU_REG_PM_CTRL);
+
+   val = 

[PATCH v6 12/12] intel-ipu3: Add imgu top level pci device driver

[Yong Zhi]

This patch adds support for the Intel IPU v3 as found
on Skylake and Kaby Lake SoCs.

The driver glues v4l2, css(camera sub system) and other
pieces together to perform its functions, it also loads
the IPU3 firmware binary as part of its initialization.

Signed-off-by: Yong Zhi 
Signed-off-by: Tomasz Figa 
---
 drivers/media/pci/intel/ipu3/Kconfig  |  14 +
 drivers/media/pci/intel/ipu3/Makefile |  11 +
 drivers/media/pci/intel/ipu3/ipu3.c   | 849 ++
 3 files changed, 874 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3.c

diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
b/drivers/media/pci/intel/ipu3/Kconfig
index a82d3fe277d2..a844735cab2e 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -17,3 +17,17 @@ config VIDEO_IPU3_CIO2
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.
+
+config VIDEO_IPU3_IMGU
+   tristate "Intel ipu3-imgu driver"
+   depends on PCI && VIDEO_V4L2
+   depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
+   select IOMMU_IOVA
+   select VIDEOBUF2_DMA_SG
+   ---help---
+   This is the video4linux2 driver for Intel IPU3 image processing unit,
+   found in Intel Skylake and Kaby Lake SoCs and used for processing
+   images and video.
+
+   Say Y or M here if you have a Skylake/Kaby Lake SoC with a MIPI
+   camera. The module will be called ipu3-imgu.
diff --git a/drivers/media/pci/intel/ipu3/Makefile 
b/drivers/media/pci/intel/ipu3/Makefile
index 20186e3ff2ae..dbfe37db3412 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -1 +1,12 @@
+#
+# Makefile for the IPU3 cio2 and ImgU drivers
+#
+
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
+
+ipu3-imgu-objs += ipu3-mmu.o ipu3-dmamap.o \
+   ipu3-tables.o ipu3-css-pool.o \
+   ipu3-css-fw.o ipu3-css-params.o \
+   ipu3-css.o ipu3-v4l2.o ipu3.o
+
+obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3-imgu.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3.c 
b/drivers/media/pci/intel/ipu3/ipu3.c
new file mode 100644
index ..c2fb0ab3bd6d
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3.c
@@ -0,0 +1,849 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 Intel Corporation
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * Based on Intel IPU4 driver.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "ipu3.h"
+#include "ipu3-dmamap.h"
+#include "ipu3-mmu.h"
+
+#define IMGU_PCI_ID0x1919
+#define IMGU_PCI_BAR   0
+#define IMGU_DMA_MASK  DMA_BIT_MASK(39)
+#define IMGU_MAX_QUEUE_DEPTH   (2 + 2)
+
+/*
+ * pre-allocated buffer size for IMGU dummy buffers. Those
+ * values should be tuned to big enough to avoid buffer
+ * re-allocation when streaming to lower streaming latency.
+ */
+#define CSS_QUEUE_IN_BUF_SIZE  0
+#define CSS_QUEUE_PARAMS_BUF_SIZE  0
+#define CSS_QUEUE_OUT_BUF_SIZE (4160 * 3120 * 12 / 8)
+#define CSS_QUEUE_VF_BUF_SIZE  (1920 * 1080 * 12 / 8)
+#define CSS_QUEUE_STAT_3A_BUF_SIZE 125664
+
+static const size_t css_queue_buf_size_map[IPU3_CSS_QUEUES] = {
+   [IPU3_CSS_QUEUE_IN] = CSS_QUEUE_IN_BUF_SIZE,
+   [IPU3_CSS_QUEUE_PARAMS] = CSS_QUEUE_PARAMS_BUF_SIZE,
+   [IPU3_CSS_QUEUE_OUT] = CSS_QUEUE_OUT_BUF_SIZE,
+   [IPU3_CSS_QUEUE_VF] = CSS_QUEUE_VF_BUF_SIZE,
+   [IPU3_CSS_QUEUE_STAT_3A] = CSS_QUEUE_STAT_3A_BUF_SIZE,
+};
+
+static const struct imgu_node_mapping imgu_node_map[IMGU_NODE_NUM] = {
+   [IMGU_NODE_IN] = {IPU3_CSS_QUEUE_IN, "input"},
+   [IMGU_NODE_PARAMS] = {IPU3_CSS_QUEUE_PARAMS, "parameters"},
+   [IMGU_NODE_OUT] = {IPU3_CSS_QUEUE_OUT, "output"},
+   [IMGU_NODE_VF] = {IPU3_CSS_QUEUE_VF, "viewfinder"},
+   [IMGU_NODE_PV] = {IPU3_CSS_QUEUE_VF, "postview"},
+   [IMGU_NODE_STAT_3A] = {IPU3_CSS_QUEUE_STAT_3A, "3a stat"},
+};
+
+unsigned int imgu_node_to_queue(unsigned int node)
+{
+   return imgu_node_map[node].css_queue;
+}
+
+unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue)
+{
+   unsigned int i;
+
+   if (css_queue == IPU3_CSS_QUEUE_VF)
+   return imgu->nodes[IMGU_NODE_VF].enabled ?
+   IMGU_NODE_VF : IMGU_NODE_PV;
+
+   for (i = 0; i < IMGU_NODE_NUM; i++)
+   if (imgu_node_map[i].css_queue == css_queue)
+   break;
+
+   return i;
+}
+
+/ Dummy buffers /
+
+static void imgu_dummybufs_cleanup(struct imgu_device *imgu)
+{
+   unsigned int i;
+
+   for (i = 0; i < IPU3_CSS_QUEUES; i++)
+   ipu3_dmamap_free(>pci_dev->dev, >queues[i].dmap);
+}
+
+static int imgu_dummybufs_preallocate(struct imgu_device *imgu)
+{
+   unsigned int i;
+   size_t size;
+
+   

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Jerome Glisse]

On Thu, Mar 29, 2018 at 10:25:52AM -0600, Logan Gunthorpe wrote:
> 
> 
> On 29/03/18 10:10 AM, Christian König wrote:
> > Why not? I mean the dma_map_resource() function is for P2P while other 
> > dma_map_* functions are only for system memory.
> 
> Oh, hmm, I wasn't aware dma_map_resource was exclusively for mapping
> P2P. Though it's a bit odd seeing we've been working under the
> assumption that PCI P2P is different as it has to translate the PCI bus
> address. Where as P2P for devices on other buses is a big unknown.

dma_map_resource() is the right API (thought its current implementation
is fill with x86 assumptions). So i would argue that arch can decide to
implement it or simply return dma error address which trigger fallback
path into the caller (at least for GPU drivers). SG variant can be added
on top.

dma_map_resource() is the right API because it has all the necessary
informations. It use the CPU physical address as the common address
"language" with CPU physical address of PCIE bar to map to another
device you can find the corresponding bus address from the IOMMU code
(NOP on x86). So dma_map_resource() knows both the source device which
export its PCIE bar and the destination devices.


So i don't think Christian need to base his patchset on yours. This
is orthogonal. Christian is using existing upstream API, if it is
broken on some arch it is up to those arch to fix it, or return error
if it is not fixable. In fact i would assume that you would need to
base your patchset on top of dma_map_resource() too ...

Moreover i doubt Christian want to have struct page for this. For
nouveau there will be HMM struct page and this would conflict.

AFAICT you need struct page because the API you are trying to expose
to user space rely on "regular" filesystem/RDMA umem API which all
assume struct page. GPU drivers do not have this requirement and it
should not be impose on them.


So from my point of view Christian patchset is good as it is. Modulo
better commit message.


Bottom line i think we need common PCIE helper for P2P and the current
dma_map_resource() is the right kind from POV. What you are doing with
struct page is specific to your sub-system and should not be impose on
others.

Cheers,
Jérôme

RE: [PATCH v3 7/9] v4l: xilinx: dma: Add multi-planar support

[Satish Kumar Nagireddy]

Hi Hyun,

Thanks a lot for the comments. I will fix them in v4 patch-set.

Regards,
Satish

> -Original Message-
> From: Hyun Kwon [mailto:hyun.k...@xilinx.com]
> Sent: Friday, February 16, 2018 9:05 AM
> To: Satish Kumar Nagireddy 
> Cc: linux-media@vger.kernel.org; laurent.pinch...@ideasonboard.com;
> michal.si...@xilinx.com; Hyun Kwon ; Satish Kumar
> Nagireddy 
> Subject: Re: [PATCH v3 7/9] v4l: xilinx: dma: Add multi-planar support
> 
> Hi Satish,
> 
> Thanks for the patch.
> 
> On Wed, 2018-02-14 at 22:42:43 -0800, Satish Kumar Nagireddy wrote:
> > The current v4l driver supports single plane formats. This patch will
> > add support to handle multi-planar formats. Updated driver
> > capabilities to multi-planar, where it can handle both single and
> > multi-planar formats
> >
> > Signed-off-by: Satish Kumar Nagireddy 
> > ---
> >  drivers/media/platform/xilinx/xilinx-dma.c  | 341
> +++-
> >  drivers/media/platform/xilinx/xilinx-dma.h  |   2 +-
> >  drivers/media/platform/xilinx/xilinx-vipp.c |  22 +-
> >  3 files changed, 307 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
> > b/drivers/media/platform/xilinx/xilinx-dma.c
> > index cb20ada..664981b 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -63,6 +63,7 @@ static int xvip_dma_verify_format(struct xvip_dma
> *dma)
> > struct v4l2_subdev_format fmt;
> > struct v4l2_subdev *subdev;
> > int ret;
> > +   int width, height;
> >
> > subdev = xvip_dma_remote_subdev(>pad, );
> > if (subdev == NULL)
> > @@ -73,9 +74,18 @@ static int xvip_dma_verify_format(struct xvip_dma
> *dma)
> > if (ret < 0)
> > return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> >
> > -   if (dma->fmtinfo->code != fmt.format.code ||
> > -   dma->format.height != fmt.format.height ||
> > -   dma->format.width != fmt.format.width)
> > +   if (dma->fmtinfo->code != fmt.format.code)
> > +   return -EINVAL;
> > +
> > +   if (V4L2_TYPE_IS_MULTIPLANAR(dma->format.type)) {
> 
> As discussed, let's plan to remove this check. :-) I think now it's safe to
> assume there's no backward compatibility issue.
> 
> > +   width = dma->format.fmt.pix_mp.width;
> > +   height = dma->format.fmt.pix_mp.height;
> > +   } else {
> > +   width = dma->format.fmt.pix.width;
> > +   height = dma->format.fmt.pix.height;
> > +   }
> > +
> > +   if (width != fmt.format.width || height != fmt.format.height)
> > return -EINVAL;
> >
> > return 0;
> > @@ -302,6 +312,8 @@ static void xvip_dma_complete(void *param)  {
> > struct xvip_dma_buffer *buf = param;
> > struct xvip_dma *dma = buf->dma;
> > +   u8 num_planes, i;
> > +   int sizeimage;
> >
> > spin_lock(>queued_lock);
> > list_del(>queue);
> > @@ -310,7 +322,28 @@ static void xvip_dma_complete(void *param)
> > buf->buf.field = V4L2_FIELD_NONE;
> > buf->buf.sequence = dma->sequence++;
> > buf->buf.vb2_buf.timestamp = ktime_get_ns();
> > -   vb2_set_plane_payload(>buf.vb2_buf, 0, dma-
> >format.sizeimage);
> > +
> > +   if (V4L2_TYPE_IS_MULTIPLANAR(dma->format.type)) {
> > +   /* Handling contiguous data with mplanes */
> > +   if (dma->fmtinfo->buffers == 1) {
> > +   sizeimage =
> > +   dma-
> >format.fmt.pix_mp.plane_fmt[0].sizeimage;
> > +   vb2_set_plane_payload(>buf.vb2_buf, 0,
> sizeimage);
> > +   } else {
> > +   /* Handling non-contiguous data with mplanes */
> > +   num_planes = dma-
> >format.fmt.pix_mp.num_planes;
> > +   for (i = 0; i < num_planes; i++) {
> > +   sizeimage =
> > +dma-
> >format.fmt.pix_mp.plane_fmt[i].sizeimage;
> > +   vb2_set_plane_payload(>buf.vb2_buf,
> i,
> > + sizeimage);
> > +   }
> > +   }
> 
> Can this be done in a single loop with number of buffers?
> 
> > +   } else {
> > +   sizeimage = dma->format.fmt.pix.sizeimage;
> > +   vb2_set_plane_payload(>buf.vb2_buf, 0, sizeimage);
> > +   }
> > +
> > vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);  }
> >
> > @@ -320,13 +353,48 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
> >  unsigned int sizes[], struct device *alloc_devs[])  {
> > struct xvip_dma *dma = vb2_get_drv_priv(vq);
> > +   u8 i;
> > +   int sizeimage;
> > +
> > +   /* Multi planar case: Make sure the image size is large enough */
> > +   if (V4L2_TYPE_IS_MULTIPLANAR(dma->format.type)) {
> > +   if (*nplanes) {
> > +   if (*nplanes != dma-
> >format.fmt.pix_mp.num_planes)
> > +   return -EINVAL;
> > +
> > + 

[lwn:docs-next 73/74] htmldocs: include/net/mac80211.h:950: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info'

[kbuild test robot]

tree:   git://git.lwn.net/linux-2.6 docs-next
head:   86afad7d87f535ebb1a0e978bc32a8c58ac99268
commit: d404d57955a6f67365423f9d0b89ad1881799087 [73/74] docs: kernel-doc: fix 
parsing of arrays
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick 
(https://www.imagemagick.org)
   include/linux/crypto.h:477: warning: Function parameter or member 
'cra_u.ablkcipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 
'cra_u.blkcipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 
'cra_u.cipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 
'cra_u.compress' not described in 'crypto_alg'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.ibss' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.connect' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.keys' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie' 
not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.ie_len' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.bssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.ssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.default_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.default_mgmt_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 
'wext.prev_bssid_valid' not described in 'wireless_dev'
   include/net/mac80211.h:2259: warning: Function parameter or member 
'radiotap_timestamp.units_pos' not described in 'ieee80211_hw'
   include/net/mac80211.h:2259: warning: Function parameter or member 
'radiotap_timestamp.accuracy' not described in 'ieee80211_hw'
>> include/net/mac80211.h:950: warning: Function parameter or member 
>> 'control.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.use_rts' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.use_cts_prot' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.short_preamble' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.skip_table' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.jiffies' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.vif' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.hw_key' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.flags' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'control.enqueue_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'ack' not 
described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'ack.cookie' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.ampdu_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.antenna' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'status.tx_time' not described in 'ieee80211_tx_info'
>> include/net/mac80211.h:950: warning: Function parameter or member 
>> 'status.status_driver_data' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 
'driver_rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or 

Re: [PATCH] media: i2c: tvp5150: fix color burst lock instability on some hardware

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 22:37:57 +0430
Nasser  escreveu:

> On Thu, Mar 29, 2018 at 12:02:40PM -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 29 Mar 2018 19:04:35 +0430
> > Nasser  escreveu:
> >   
> > > On Tue, Mar 27, 2018 at 02:59:21AM +0430, Nasser wrote:
> > > Hi Mauro,
> > > 
> > > Thank you for taking time to review my patch.
> > > 
> > > May be I should rephrase the commit message to something like:
> > >   Use the default register values as suggested in TVP5150AM1 datasheet
> > > 
> > > As this is not a hardware-dependent issue. Am I missing something?  
> > 
> > It is not a matter of rephasing, but, instead, to be sure that it won't
> > cause regressions on existing hardware.
> > 
> > Yet, it would worth if you could describe at the patch what hardware
> > did you test it, and if VBI was tested too.
> >   
> 
> Does this means that I should resend the patch with this additional info?
> Sorry for not being clear about that. This was a custom board based on
> ARM. The VBI was not used.

That's what I suspected when you send your patch :-)

The tvp5150 is used by several USB and PCI devices for analog TV and
for video stream capture, with may have different requirements from what
you're doing on your ARM board.

Changing a register setting global wide without enough care will
very likely break support for existing boards.

That's why I said that the right thing to do is to pass a parameter to
the driver specifying what "extra" features are needed.

Without looking at tvp5150/tvp5151/tvp5150am datasheets nor testing,
I'd risk to say that, at the specific case of your patch, touching at
VBLK settings have a potential risk of affecting VBI handling.

> > Anyway, I'll try to find some time to run some tests on the hardware
> > I have with tvp5150 too.  
> 
> It sounds great.

Yes, but I won't be able to do it on the next couple of weeks. We're
approaching the merge window for Kernel 4.17.

Thanks,
Mauro

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Christian König]

Am 29.03.2018 um 18:25 schrieb Logan Gunthorpe:


On 29/03/18 10:10 AM, Christian König wrote:

Why not? I mean the dma_map_resource() function is for P2P while other
dma_map_* functions are only for system memory.

Oh, hmm, I wasn't aware dma_map_resource was exclusively for mapping
P2P. Though it's a bit odd seeing we've been working under the
assumption that PCI P2P is different as it has to translate the PCI bus
address. Where as P2P for devices on other buses is a big unknown.


Yeah, completely agree. On my TODO list (but rather far down) is 
actually supporting P2P with USB devices.


And no, I don't have the slightest idea how to do this at the moment.


And this is necessary to
check if the DMA ops in use support it or not. We can't have the
dma_map_X() functions do the wrong thing because they don't support it yet.

Well that sounds like we should just return an error from
dma_map_resources() when an architecture doesn't support P2P yet as Alex
suggested.

Yes, well except in our patch-set we can't easily use
dma_map_resources() as we either have SGLs to deal with or we need to
create whole new interfaces to a number of subsystems.


Agree as well. I was also in clear favor of extending the SGLs to have a 
flag for this instead of the dma_map_resource() interface, but for some 
reason that didn't made it into the kernel.



You don't seem to understand the implications: The devices do have a
common upstream bridge! In other words your code would currently claim
that P2P is supported, but in practice it doesn't work.

Do they? They don't on any of the Intel machines I'm looking at. The
previous version of the patchset not only required a common upstream
bridge but two layers of upstream bridges on both devices which would
effectively limit transfers to PCIe switches only. But Bjorn did not
like this.


At least to me that sounds like a good idea, it would at least disable 
(the incorrect) auto detection of P2P for such devices.



You need to include both drivers which participate in the P2P
transaction to make sure that both supports this and give them
opportunity to chicken out and in the case of AMD APUs even redirect the
request to another location (e.g. participate in the DMA translation).

I don't think it's the drivers responsibility to reject P2P . The
topology is what governs support or not. The discussions we had with
Bjorn settled on if the devices are all behind the same bridge they can
communicate with each other. This is essentially guaranteed by the PCI spec.


Well it is not only rejecting P2P, see the devices I need to worry about 
are essentially part of the CPU. Their resources looks like a PCI BAR to 
the BIOS and OS, but are actually backed by stolen system memory.


So as crazy as it sounds what you get is an operation which starts as 
P2P, but then the GPU drivers sees it and says: Hey please don't write 
that to my PCIe BAR, but rather system memory location X.



DMA-buf fortunately seems to handle all this already, that's why we
choose it as base for our implementation.

Well, unfortunately DMA-buf doesn't help for the drivers we are working
with as neither the block layer nor the RDMA subsystem have any
interfaces for it.


A fact that gives me quite some sleepless nights as well. I think we 
sooner or later need to extend those interfaces to work with DMA-bufs as 
well.


I will try to give your patch set a review when I'm back from vacation 
and rebase my DMA-buf work on top of that.


Regards,
Christian.



Logan

Re: [PATCH] media: i2c: tvp5150: fix color burst lock instability on some hardware

[Nasser]

On Thu, Mar 29, 2018 at 12:02:40PM -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 29 Mar 2018 19:04:35 +0430
> Nasser  escreveu:
> 
> > On Tue, Mar 27, 2018 at 02:59:21AM +0430, Nasser wrote:
> > Hi Mauro,
> > 
> > Thank you for taking time to review my patch.
> > 
> > May be I should rephrase the commit message to something like:
> > Use the default register values as suggested in TVP5150AM1 datasheet
> > 
> > As this is not a hardware-dependent issue. Am I missing something?
> 
> It is not a matter of rephasing, but, instead, to be sure that it won't
> cause regressions on existing hardware.
> 
> Yet, it would worth if you could describe at the patch what hardware
> did you test it, and if VBI was tested too.
> 

Does this means that I should resend the patch with this additional info?
Sorry for not being clear about that. This was a custom board based on
ARM. The VBI was not used.

> Anyway, I'll try to find some time to run some tests on the hardware
> I have with tvp5150 too.

It sounds great.

> 
> Regards,
> Mauro
> 
> > 
> > > On Mon, Mar 26, 2018 at 06:43:53AM -0300, Mauro Carvalho Chehab wrote:  
> > > > Hi Nasser,
> > > > 
> > > > Em Mon, 26 Mar 2018 03:26:33 +0430
> > > > Nasser Afshin  escreveu:
> > > >   
> > > > > According to the datasheet, INTREQ/GPCL/VBLK should have a 
> > > > > pull-up/down
> > > > > resistor if it's been disabled. On hardware that does not have such
> > > > > resistor, we should use the default output enable value.
> > > > > This prevents the color burst lock instability problem.  
> > > >  
> > > 
> > > Color burst lock instability is just a side effect of not using the
> > > recommended value for this bit. If we use the recommended setting, we
> > > will support more hardware while not breaking anything.
> > >   
> > > > If this is hardware-dependent, you should instead store it at
> > > > OF (for SoC) or pass via platform_data (for PCI/USB devices).
> > > >  
> > > 
> > > We have used the recommended value for this bit (as the datasheet
> > > suggests) while we are in tvp5150_init_enable but in tvp5150_s_stream
> > > we are using the wrong value.
> > > 
> > > Also we have this comment at line 319:
> > > /* Default values as sugested at TVP5150AM1 datasheet */
> > > But as you see, TVP5150_MISC_CTL is not set to its suggested default
> > > value.
> > >

Any way the assignment to tvp5150_init_default after the above comment
seems not to be correct according to the "3.21.4 Miscellaneous Controls
Register" part in the datasheet. While following the same comment phrase
on line 455, we see the correct assignment as the default values to 
tvp5150_init_enable.

Sorry this is so lengthy.

Thank you,
Nasser

> > > > > 
> > > > > Signed-off-by: Nasser Afshin 
> > > > > ---
> > > > >  drivers/media/i2c/tvp5150.c | 5 +++--
> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > > > index 2476d812f669..0e9713814816 100644
> > > > > --- a/drivers/media/i2c/tvp5150.c
> > > > > +++ b/drivers/media/i2c/tvp5150.c
> > > > > @@ -328,7 +328,7 @@ static const struct i2c_reg_value 
> > > > > tvp5150_init_default[] = {
> > > > >   TVP5150_OP_MODE_CTL,0x00
> > > > >   },
> > > > >   { /* 0x03 */
> > > > > - TVP5150_MISC_CTL,0x01
> > > > > + TVP5150_MISC_CTL,0x21
> > > > >   },
> > > > >   { /* 0x06 */
> > > > >   TVP5150_COLOR_KIL_THSH_CTL,0x10
> > > > > @@ -1072,7 +1072,8 @@ static int tvp5150_s_stream(struct v4l2_subdev 
> > > > > *sd, int enable)
> > > > >* Enable the YCbCr and clock outputs. In discrete sync 
> > > > > mode
> > > > >* (non-BT.656) additionally enable the the sync 
> > > > > outputs.
> > > > >*/
> > > > > - val |= TVP5150_MISC_CTL_YCBCR_OE | 
> > > > > TVP5150_MISC_CTL_CLOCK_OE;
> > > > > + val |= TVP5150_MISC_CTL_YCBCR_OE | 
> > > > > TVP5150_MISC_CTL_CLOCK_OE |
> > > > > + TVP5150_MISC_CTL_INTREQ_OE;
> > > > >   if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> > > > >   val |= TVP5150_MISC_CTL_SYNC_OE;
> > > > >   }  
> > > > 
> > > > 
> > > > 
> > > > Thanks,
> > > > Mauro  
> > 
> > Thanks,
> > Nasser
> 
> 
> 
> Thanks,
> Mauro

Re: [linux-sunxi] [PATCH v9 0/2] Initial Allwinner V3s CSI Support

[Martin Kelly]

On 03/28/2018 06:02 PM, Yong wrote:


AFAIK, there is no document about MIPI CSI-2. You can take a look at the
source code in BSP:
https://github.com/friendlyarm/h3_lichee/tree/master/linux-3.4/drivers/media/video/sunxi-vfe/mipi_csi
And try to port it to mainline.



Yep, I see there's lots of magic constants in that code. I might try to 
forward-port it, but it won't be very maintainable or easy to change 
without a datasheet. That's too bad.


Thanks for the information, Yong.

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Logan Gunthorpe]

On 29/03/18 10:10 AM, Christian König wrote:
> Why not? I mean the dma_map_resource() function is for P2P while other 
> dma_map_* functions are only for system memory.

Oh, hmm, I wasn't aware dma_map_resource was exclusively for mapping
P2P. Though it's a bit odd seeing we've been working under the
assumption that PCI P2P is different as it has to translate the PCI bus
address. Where as P2P for devices on other buses is a big unknown.

>> And this is necessary to
>> check if the DMA ops in use support it or not. We can't have the
>> dma_map_X() functions do the wrong thing because they don't support it yet.
> 
> Well that sounds like we should just return an error from 
> dma_map_resources() when an architecture doesn't support P2P yet as Alex 
> suggested.

Yes, well except in our patch-set we can't easily use
dma_map_resources() as we either have SGLs to deal with or we need to
create whole new interfaces to a number of subsystems.

> You don't seem to understand the implications: The devices do have a 
> common upstream bridge! In other words your code would currently claim 
> that P2P is supported, but in practice it doesn't work.

Do they? They don't on any of the Intel machines I'm looking at. The
previous version of the patchset not only required a common upstream
bridge but two layers of upstream bridges on both devices which would
effectively limit transfers to PCIe switches only. But Bjorn did not
like this.

> You need to include both drivers which participate in the P2P 
> transaction to make sure that both supports this and give them 
> opportunity to chicken out and in the case of AMD APUs even redirect the 
> request to another location (e.g. participate in the DMA translation).

I don't think it's the drivers responsibility to reject P2P . The
topology is what governs support or not. The discussions we had with
Bjorn settled on if the devices are all behind the same bridge they can
communicate with each other. This is essentially guaranteed by the PCI spec.

> DMA-buf fortunately seems to handle all this already, that's why we 
> choose it as base for our implementation.

Well, unfortunately DMA-buf doesn't help for the drivers we are working
with as neither the block layer nor the RDMA subsystem have any
interfaces for it.

Logan

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Christian König]

Am 29.03.2018 um 17:45 schrieb Logan Gunthorpe:


On 29/03/18 05:44 AM, Christian König wrote:

Am 28.03.2018 um 21:53 schrieb Logan Gunthorpe:

On 28/03/18 01:44 PM, Christian König wrote:

Well, isn't that exactly what dma_map_resource() is good for? As far as
I can see it makes sure IOMMU is aware of the access route and
translates a CPU address into a PCI Bus address.
I'm using that with the AMD IOMMU driver and at least there it works
perfectly fine.

Yes, it would be nice, but no arch has implemented this yet. We are just
lucky in the x86 case because that arch is simple and doesn't need to do
anything for P2P (partially due to the Bus and CPU addresses being the
same). But in the general case, you can't rely on it.

Well, that an arch hasn't implemented it doesn't mean that we don't have
the right interface to do it.

Yes, but right now we don't have a performant way to check if we are
doing P2P or not in the dma_map_X() wrappers.


Why not? I mean the dma_map_resource() function is for P2P while other 
dma_map_* functions are only for system memory.



And this is necessary to
check if the DMA ops in use support it or not. We can't have the
dma_map_X() functions do the wrong thing because they don't support it yet.


Well that sounds like we should just return an error from 
dma_map_resources() when an architecture doesn't support P2P yet as Alex 
suggested.



Devices integrated in the CPU usually only "claim" to be PCIe devices.
In reality their memory request path go directly through the integrated
north bridge. The reason for this is simple better throughput/latency.

These are just more reasons why our patchset restricts to devices behind
a switch. And more mess for someone to deal with if they need to relax
that restriction.


You don't seem to understand the implications: The devices do have a 
common upstream bridge! In other words your code would currently claim 
that P2P is supported, but in practice it doesn't work.


You need to include both drivers which participate in the P2P 
transaction to make sure that both supports this and give them 
opportunity to chicken out and in the case of AMD APUs even redirect the 
request to another location (e.g. participate in the DMA translation).


DMA-buf fortunately seems to handle all this already, that's why we 
choose it as base for our implementation.


Regards,
Christian.

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Logan Gunthorpe]

On 29/03/18 05:44 AM, Christian König wrote:
> Am 28.03.2018 um 21:53 schrieb Logan Gunthorpe:
>>
>> On 28/03/18 01:44 PM, Christian König wrote:
>>> Well, isn't that exactly what dma_map_resource() is good for? As far as
>>> I can see it makes sure IOMMU is aware of the access route and
>>> translates a CPU address into a PCI Bus address.
>>> I'm using that with the AMD IOMMU driver and at least there it works
>>> perfectly fine.
>> Yes, it would be nice, but no arch has implemented this yet. We are just
>> lucky in the x86 case because that arch is simple and doesn't need to do
>> anything for P2P (partially due to the Bus and CPU addresses being the
>> same). But in the general case, you can't rely on it.
> 
> Well, that an arch hasn't implemented it doesn't mean that we don't have 
> the right interface to do it.

Yes, but right now we don't have a performant way to check if we are
doing P2P or not in the dma_map_X() wrappers. And this is necessary to
check if the DMA ops in use support it or not. We can't have the
dma_map_X() functions do the wrong thing because they don't support it yet.

> Devices integrated in the CPU usually only "claim" to be PCIe devices. 
> In reality their memory request path go directly through the integrated 
> north bridge. The reason for this is simple better throughput/latency.

These are just more reasons why our patchset restricts to devices behind
a switch. And more mess for someone to deal with if they need to relax
that restriction.

Logan

Re: [PATCH] media: i2c: tvp5150: fix color burst lock instability on some hardware

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 19:04:35 +0430
Nasser  escreveu:

> On Tue, Mar 27, 2018 at 02:59:21AM +0430, Nasser wrote:
> Hi Mauro,
> 
> Thank you for taking time to review my patch.
> 
> May be I should rephrase the commit message to something like:
>   Use the default register values as suggested in TVP5150AM1 datasheet
> 
> As this is not a hardware-dependent issue. Am I missing something?

It is not a matter of rephasing, but, instead, to be sure that it won't
cause regressions on existing hardware.

Yet, it would worth if you could describe at the patch what hardware
did you test it, and if VBI was tested too.

Anyway, I'll try to find some time to run some tests on the hardware
I have with tvp5150 too.

Regards,
Mauro

> 
> > On Mon, Mar 26, 2018 at 06:43:53AM -0300, Mauro Carvalho Chehab wrote:  
> > > Hi Nasser,
> > > 
> > > Em Mon, 26 Mar 2018 03:26:33 +0430
> > > Nasser Afshin  escreveu:
> > >   
> > > > According to the datasheet, INTREQ/GPCL/VBLK should have a pull-up/down
> > > > resistor if it's been disabled. On hardware that does not have such
> > > > resistor, we should use the default output enable value.
> > > > This prevents the color burst lock instability problem.  
> > >  
> > 
> > Color burst lock instability is just a side effect of not using the
> > recommended value for this bit. If we use the recommended setting, we
> > will support more hardware while not breaking anything.
> >   
> > > If this is hardware-dependent, you should instead store it at
> > > OF (for SoC) or pass via platform_data (for PCI/USB devices).
> > >  
> > 
> > We have used the recommended value for this bit (as the datasheet
> > suggests) while we are in tvp5150_init_enable but in tvp5150_s_stream
> > we are using the wrong value.
> > 
> > Also we have this comment at line 319:
> > /* Default values as sugested at TVP5150AM1 datasheet */
> > But as you see, TVP5150_MISC_CTL is not set to its suggested default
> > value.
> >
> > > > 
> > > > Signed-off-by: Nasser Afshin 
> > > > ---
> > > >  drivers/media/i2c/tvp5150.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > > index 2476d812f669..0e9713814816 100644
> > > > --- a/drivers/media/i2c/tvp5150.c
> > > > +++ b/drivers/media/i2c/tvp5150.c
> > > > @@ -328,7 +328,7 @@ static const struct i2c_reg_value 
> > > > tvp5150_init_default[] = {
> > > > TVP5150_OP_MODE_CTL,0x00
> > > > },
> > > > { /* 0x03 */
> > > > -   TVP5150_MISC_CTL,0x01
> > > > +   TVP5150_MISC_CTL,0x21
> > > > },
> > > > { /* 0x06 */
> > > > TVP5150_COLOR_KIL_THSH_CTL,0x10
> > > > @@ -1072,7 +1072,8 @@ static int tvp5150_s_stream(struct v4l2_subdev 
> > > > *sd, int enable)
> > > >  * Enable the YCbCr and clock outputs. In discrete sync 
> > > > mode
> > > >  * (non-BT.656) additionally enable the the sync 
> > > > outputs.
> > > >  */
> > > > -   val |= TVP5150_MISC_CTL_YCBCR_OE | 
> > > > TVP5150_MISC_CTL_CLOCK_OE;
> > > > +   val |= TVP5150_MISC_CTL_YCBCR_OE | 
> > > > TVP5150_MISC_CTL_CLOCK_OE |
> > > > +   TVP5150_MISC_CTL_INTREQ_OE;
> > > > if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> > > > val |= TVP5150_MISC_CTL_SYNC_OE;
> > > > }  
> > > 
> > > 
> > > 
> > > Thanks,
> > > Mauro  
> 
> Thanks,
> Nasser



Thanks,
Mauro

[RESEND PATCH] media: i2c: ov5640: Add pixel clock support

[Manivannan Sadhasivam]

Some of the camera subsystems like camss in Qualcommm MSM chipsets
require pixel clock support in camera sensor drivers. So, this commit
adds a default pixel clock rate of 96MHz to OV5640 camera sensor driver.

According to the datasheet, 96MHz can be used as a pixel clock rate for
most of the modes.

Signed-off-by: Manivannan Sadhasivam 
---
 drivers/media/i2c/ov5640.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 39a2269..7152c84 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -162,6 +162,7 @@ struct ov5640_ctrls {
struct v4l2_ctrl *auto_gain;
struct v4l2_ctrl *gain;
};
+   struct v4l2_ctrl *pixel_clock;
struct v4l2_ctrl *brightness;
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *contrast;
@@ -2009,6 +2010,9 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN,
0, 1023, 1, 0);
 
+   /* Pixel clock (default of 96MHz) */
+   ctrls->pixel_clock = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE,
+   1, INT_MAX, 1, 9600);
ctrls->saturation = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION,
  0, 255, 1, 64);
ctrls->hue = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HUE,
-- 
2.7.4

[RESEND PATCH] Add pixel clock support to OV5640 camera sensor

[Manivannan Sadhasivam]

Some of the camera subsystems like camss in Qualcommm MSM chipsets
require pixel clock support in camera sensor drivers for proper functioning.
So, add a default pixel clock rate of 96MHz to OV5640 camera sensor driver.

According to the datasheet, 96MHz can be used as a pixel clock rate for
most of the modes.

This patch has been validated on Dragonboard410c with OV5640 connected
using D3 Camera Mezzanine.

Manivannan Sadhasivam (1):
  media: i2c: ov5640: Add pixel clock support

 drivers/media/i2c/ov5640.c | 4 
 1 file changed, 4 insertions(+)

-- 
2.7.4

[no subject]

[Rainer Keller]

unsubscribe linux-media

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Alex Deucher]

Sorry, didn't mean to drop the lists here. re-adding.

On Wed, Mar 28, 2018 at 4:05 PM, Alex Deucher  wrote:
> On Wed, Mar 28, 2018 at 3:53 PM, Logan Gunthorpe  wrote:
>>
>>
>> On 28/03/18 01:44 PM, Christian König wrote:
>>> Well, isn't that exactly what dma_map_resource() is good for? As far as
>>> I can see it makes sure IOMMU is aware of the access route and
>>> translates a CPU address into a PCI Bus address.
>>
>>> I'm using that with the AMD IOMMU driver and at least there it works
>>> perfectly fine.
>>
>> Yes, it would be nice, but no arch has implemented this yet. We are just
>> lucky in the x86 case because that arch is simple and doesn't need to do
>> anything for P2P (partially due to the Bus and CPU addresses being the
>> same). But in the general case, you can't rely on it.
>
> Could we do something for the arches where it works?  I feel like peer
> to peer has dragged out for years because everyone is trying to boil
> the ocean for all arches.  There are a huge number of use cases for
> peer to peer on these "simple" architectures which actually represent
> a good deal of the users that want this.
>
> Alex
>
>>
> Yeah, but not for ours. See if you want to do real peer 2 peer you need
> to keep both the operation as well as the direction into account.
 Not sure what you are saying here... I'm pretty sure we are doing "real"
 peer 2 peer...

> For example when you can do writes between A and B that doesn't mean
> that writes between B and A work. And reads are generally less likely to
> work than writes. etc...
 If both devices are behind a switch then the PCI spec guarantees that A
 can both read and write B and vice versa.
>>>
>>> Sorry to say that, but I know a whole bunch of PCI devices which
>>> horrible ignores that.
>>
>> Can you elaborate? As far as the device is concerned it shouldn't know
>> whether a request comes from a peer or from the host. If it does do
>> crazy stuff like that it's well out of spec. It's up to the switch (or
>> root complex if good support exists) to route the request to the device
>> and it's the root complex that tends to be what drops the load requests
>> which causes the asymmetries.
>>
>> Logan
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] media: i2c: tvp5150: fix color burst lock instability on some hardware

[Nasser]

On Tue, Mar 27, 2018 at 02:59:21AM +0430, Nasser wrote:
Hi Mauro,

Thank you for taking time to review my patch.

May be I should rephrase the commit message to something like:
Use the default register values as suggested in TVP5150AM1 datasheet

As this is not a hardware-dependent issue. Am I missing something?

> On Mon, Mar 26, 2018 at 06:43:53AM -0300, Mauro Carvalho Chehab wrote:
> > Hi Nasser,
> > 
> > Em Mon, 26 Mar 2018 03:26:33 +0430
> > Nasser Afshin  escreveu:
> > 
> > > According to the datasheet, INTREQ/GPCL/VBLK should have a pull-up/down
> > > resistor if it's been disabled. On hardware that does not have such
> > > resistor, we should use the default output enable value.
> > > This prevents the color burst lock instability problem.
> >
> 
> Color burst lock instability is just a side effect of not using the
> recommended value for this bit. If we use the recommended setting, we
> will support more hardware while not breaking anything.
> 
> > If this is hardware-dependent, you should instead store it at
> > OF (for SoC) or pass via platform_data (for PCI/USB devices).
> >
> 
> We have used the recommended value for this bit (as the datasheet
> suggests) while we are in tvp5150_init_enable but in tvp5150_s_stream
> we are using the wrong value.
> 
> Also we have this comment at line 319:
> /* Default values as sugested at TVP5150AM1 datasheet */
> But as you see, TVP5150_MISC_CTL is not set to its suggested default
> value.
>  
> > > 
> > > Signed-off-by: Nasser Afshin 
> > > ---
> > >  drivers/media/i2c/tvp5150.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > index 2476d812f669..0e9713814816 100644
> > > --- a/drivers/media/i2c/tvp5150.c
> > > +++ b/drivers/media/i2c/tvp5150.c
> > > @@ -328,7 +328,7 @@ static const struct i2c_reg_value 
> > > tvp5150_init_default[] = {
> > >   TVP5150_OP_MODE_CTL,0x00
> > >   },
> > >   { /* 0x03 */
> > > - TVP5150_MISC_CTL,0x01
> > > + TVP5150_MISC_CTL,0x21
> > >   },
> > >   { /* 0x06 */
> > >   TVP5150_COLOR_KIL_THSH_CTL,0x10
> > > @@ -1072,7 +1072,8 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, 
> > > int enable)
> > >* Enable the YCbCr and clock outputs. In discrete sync mode
> > >* (non-BT.656) additionally enable the the sync outputs.
> > >*/
> > > - val |= TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_CLOCK_OE;
> > > + val |= TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_CLOCK_OE |
> > > + TVP5150_MISC_CTL_INTREQ_OE;
> > >   if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> > >   val |= TVP5150_MISC_CTL_SYNC_OE;
> > >   }
> > 
> > 
> > 
> > Thanks,
> > Mauro

Thanks,
Nasser

Re: [PATCH for v3.18 00/18] Backport CVE-2017-13166 fixes to Kernel 3.18

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 08:22:08 +0900
Inki Dae  escreveu:

> Hi Mauro,
> 
> 2018년 03월 29일 03:12에 Mauro Carvalho Chehab 이(가) 쓴 글:
> > Hi Greg,
> > 
> > Those are the backports meant to solve CVE-2017-13166 on Kernel 3.18.
> > 
> > It contains two v4l2-ctrls fixes that are required to avoid crashes
> > at the test application.
> > 
> > I wrote two patches myself for Kernel 3.18 in order to solve some
> > issues specific for Kernel 3.18 with aren't needed upstream.
> > one is actually a one-line change backport. The other one makes
> > sure that both 32-bits and 64-bits version of some ioctl calls
> > will return the same value for a reserved field.
> > 
> > I noticed an extra bug while testing it, but the bug also hits upstream,
> > and should be backported all the way down all stable/LTS versions.
> > So, I'll send it the usual way, after merging upsream.  
> 
> Really thanks for doing this. :) There would be many users who use Linux-3.18 
> for their products yet.

Anytime!

Please let me know if you find any issues with those backports.

Regards,
Mauro

Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver

[Niklas Söderlund]

Hi Maxime,

Thanks for your patch.

On 2018-02-15 14:33:35 +0100, Maxime Ripard wrote:
> The Cadence CSI-2 RX Controller is an hardware block meant to be used as a
> bridge between a CSI-2 bus and pixel grabbers.
> 
> It supports operating with internal or external D-PHY, with up to 4 lanes,
> or without any D-PHY. The current code only supports the latter case.
> 
> It also support dynamic mapping of the CSI-2 virtual channels to the
> associated pixel grabbers, but that isn't allowed at the moment either.
> 
> Acked-by: Sakari Ailus 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/media/platform/Kconfig   |   1 +
>  drivers/media/platform/Makefile  |   2 +
>  drivers/media/platform/cadence/Kconfig   |  17 +
>  drivers/media/platform/cadence/Makefile  |   1 +
>  drivers/media/platform/cadence/cdns-csi2rx.c | 499 
> +++
>  5 files changed, 520 insertions(+)
>  create mode 100644 drivers/media/platform/cadence/Kconfig
>  create mode 100644 drivers/media/platform/cadence/Makefile
>  create mode 100644 drivers/media/platform/cadence/cdns-csi2rx.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 614fbef08ddc..c001b646d441 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -26,6 +26,7 @@ config VIDEO_VIA_CAMERA
>  #
>  # Platform multimedia device configuration
>  #
> +source "drivers/media/platform/cadence/Kconfig"
>  
>  source "drivers/media/platform/davinci/Kconfig"
>  
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 7f3080437be6..3d29082fcf72 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -3,6 +3,8 @@
>  # Makefile for the video capture/playback device drivers.
>  #
>  
> +obj-$(CONFIG_VIDEO_CADENCE)  += cadence/
> +
>  obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o
>  
>  obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
> diff --git a/drivers/media/platform/cadence/Kconfig 
> b/drivers/media/platform/cadence/Kconfig
> new file mode 100644
> index ..18f061e5cbd1
> --- /dev/null
> +++ b/drivers/media/platform/cadence/Kconfig
> @@ -0,0 +1,17 @@
> +config VIDEO_CADENCE
> + bool "Cadence Video Devices"

I'm no expert on Kconfig best practices so if nothing else I might learn 
something. There is no need to add a description to this option as it 
only groups the Cadence drivers?

> +
> +if VIDEO_CADENCE
> +
> +config VIDEO_CADENCE_CSI2RX
> + tristate "Cadence MIPI-CSI2 RX Controller"
> + depends on MEDIA_CONTROLLER
> + depends on VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
> + help
> +   Support for the Cadence MIPI CSI2 Receiver controller.
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called cdns-csi2rx.
> +
> +endif
> diff --git a/drivers/media/platform/cadence/Makefile 
> b/drivers/media/platform/cadence/Makefile
> new file mode 100644
> index ..99a4086b7448
> --- /dev/null
> +++ b/drivers/media/platform/cadence/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_CADENCE_CSI2RX)   += cdns-csi2rx.o
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c 
> b/drivers/media/platform/cadence/cdns-csi2rx.c
> new file mode 100644
> index ..99662e1a536b
> --- /dev/null
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -0,0 +1,499 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Cadence MIPI-CSI2 RX Controller v1.3
> + *
> + * Copyright (C) 2017 Cadence Design Systems Inc.
> + */
> +
> +#include 

No longer need as you now use a mutex, right? Same comment for TX 
patch-set.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

I think you also can drop slab.h, same comment for TX patch-set.

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CSI2RX_DEVICE_CFG_REG0x000
> +
> +#define CSI2RX_SOFT_RESET_REG0x004
> +#define CSI2RX_SOFT_RESET_PROTOCOL   BIT(1)
> +#define CSI2RX_SOFT_RESET_FRONT  BIT(0)
> +
> +#define CSI2RX_STATIC_CFG_REG0x008
> +#define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane)((plane) << (16 + 
> (llane) * 4))
> +#define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8)
> +
> +#define CSI2RX_STREAM_BASE(n)(((n) + 1) * 0x100)
> +
> +#define CSI2RX_STREAM_CTRL_REG(n)(CSI2RX_STREAM_BASE(n) + 0x000)
> +#define CSI2RX_STREAM_CTRL_START BIT(0)
> +
> +#define CSI2RX_STREAM_DATA_CFG_REG(n)(CSI2RX_STREAM_BASE(n) 
> + 0x008)
> +#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT  BIT(31)
> +#define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n)  BIT((n) + 16)
> +
> +#define CSI2RX_STREAM_CFG_REG(n) 

Wrong use of GFP_DMA32 in drivers/media/platform/vivid/vivid-osd.c

[Hans de Goede]

Hi Hans, et. al.,

While debugging another GFP_DMA32 problem I did a quick
grep for GFP_DMA32 on the kernel, this result stood out:

drivers/media/platform/vivid/vivid-osd.c
373:dev->video_vbase = kzalloc(dev->video_buffer_size, GFP_KERNEL | 
GFP_DMA32);

Because it is making the same mistake as I was, you cannot use
GDP_DMA32 with kmalloc and friends, it will end up being
ignored. If you need memory below 4G you must call alloc_pages
for get_free_pages with GFP_DMA32 to get it.

Regards,

Hans

Re: [PATCH v8 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings

[Niklas Söderlund]

Hi Maxime,

Thanks for your patch.

On 2018-02-15 14:33:34 +0100, Maxime Ripard wrote:
> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to
> 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on
> the hardware implementation.
> 
> It can operate with an external D-PHY, an internal one or no D-PHY at all
> in some configurations.
> 
> Acked-by: Rob Herring 
> Acked-by: Benoit Parrot 
> Acked-by: Sakari Ailus 
> Reviewed-by: Laurent Pinchart 
> Signed-off-by: Maxime Ripard 

Shall this file be added to MAINTAINERS? This can be done in a follow up 
patch, but if you respin please consider it.

Reviewed-by: Niklas Söderlund 

> ---
>  .../devicetree/bindings/media/cdns,csi2rx.txt  | 100 
> +
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.txt 
> b/Documentation/devicetree/bindings/media/cdns,csi2rx.txt
> new file mode 100644
> index ..6b02a0657ad9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.txt
> @@ -0,0 +1,100 @@
> +Cadence MIPI-CSI2 RX controller
> +===
> +
> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 CSI
> +lanes in input, and 4 different pixel streams in output.
> +
> +Required properties:
> +  - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible
> +  - reg: base address and size of the memory mapped region
> +  - clocks: phandles to the clocks driving the controller
> +  - clock-names: must contain:
> +* sys_clk: main clock
> +* p_clk: register bank clock
> +* pixel_if[0-3]_clk: pixel stream output clock, one for each stream
> + implemented in hardware, between 0 and 3
> +
> +Optional properties:
> +  - phys: phandle to the external D-PHY, phy-names must be provided
> +  - phy-names: must contain "dphy", if the implementation uses an
> +   external D-PHY
> +
> +Required subnodes:
> +  - ports: A ports node with one port child node per device input and output
> +   port, in accordance with the video interface bindings defined in
> +   Documentation/devicetree/bindings/media/video-interfaces.txt. The
> +   port nodes are numbered as follows:
> +
> +   Port Description
> +   -
> +   0CSI-2 input
> +   1Stream 0 output
> +   2Stream 1 output
> +   3Stream 2 output
> +   4Stream 3 output
> +
> +   The stream output port nodes are optional if they are not
> +   connected to anything at the hardware level or implemented
> +   in the design.Since there is only one endpoint per port,
> +   the endpoints are not numbered.
> +
> +
> +Example:
> +
> +csi2rx: csi-bridge@0d06 {
> + compatible = "cdns,csi2rx";
> + reg = <0x0d06 0x1000>;
> + clocks = <>, <>
> +  <>, <>,
> +  <>, <>;
> + clock-names = "sys_clk", "p_clk",
> +   "pixel_if0_clk", "pixel_if1_clk",
> +   "pixel_if2_clk", "pixel_if3_clk";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + csi2rx_in_sensor: endpoint {
> + remote-endpoint = <_out_csi2rx>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + csi2rx_out_grabber0: endpoint {
> + remote-endpoint = <_in_csi2rx>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> +
> + csi2rx_out_grabber1: endpoint {
> + remote-endpoint = <_in_csi2rx>;
> + };
> + };
> +
> + port@3 {
> + reg = <3>;
> +
> + csi2rx_out_grabber2: endpoint {
> + remote-endpoint = <_in_csi2rx>;
> + };
> + };
> +
> + port@4 {
> + reg = <4>;
> +
> + csi2rx_out_grabber3: endpoint {
> + remote-endpoint = <_in_csi2rx>;
> + };
> + };
> + };
> +};
> -- 
> 2.14.3
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Hans Verkuil]

On 29/03/18 15:00, Mauro Carvalho Chehab wrote:
> Em Thu, 29 Mar 2018 10:40:23 +0200
> Hans Verkuil  escreveu:
> 
>> Hi Mauro,
>>
>> On 28/03/18 19:59, Mauro Carvalho Chehab wrote:
>>> At put_v4l2_window32(), it tries to access kp->clips. However,
>>> kp points to an userspace pointer. So, it should be obtained
>>> via get_user(), otherwise it can OOPS:
>>>   
>>
>> 
>>
>>>
>>> cc: sta...@vger.kernel.org
>>> Signed-off-by: Mauro Carvalho Chehab 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
>>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> index 5198c9eeb348..4312935f1dfc 100644
>>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>> @@ -101,7 +101,7 @@ static int get_v4l2_window32(struct v4l2_window __user 
>>> *kp,
>>>  static int put_v4l2_window32(struct v4l2_window __user *kp,
>>>  struct v4l2_window32 __user *up)
>>>  {
>>> -   struct v4l2_clip __user *kclips = kp->clips;
>>> +   struct v4l2_clip __user *kclips;
>>> struct v4l2_clip32 __user *uclips;
>>> compat_caddr_t p;
>>> u32 clipcount;
>>> @@ -116,6 +116,8 @@ static int put_v4l2_window32(struct v4l2_window __user 
>>> *kp,
>>> if (!clipcount)
>>> return 0;
>>>  
>>> +   if (get_user(kclips, >clips))
>>> +   return -EFAULT;
>>> if (get_user(p, >clips))
>>> return -EFAULT;
>>> uclips = compat_ptr(p);
>>>   
>>
>> Reviewed-by: Hans Verkuil 
>>
>> I have no idea why I didn't find this when I tested this with 
>> v4l2-compliance,
>> but the code was certainly wrong.
> 
> I built 4.16-rc4 with KASAN enabled. Perhaps, it won't OOPS without
> it. Yet, I doubt it would work without this fix.

I definitely did not have KASAN enabled when I tested this.

Regards,

Hans

> 
>>
>> Thank you for debugging this!
> 
> Anytime.
> 
> Thanks,
> Mauro
> 

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 10:40:23 +0200
Hans Verkuil  escreveu:

> Hi Mauro,
> 
> On 28/03/18 19:59, Mauro Carvalho Chehab wrote:
> > At put_v4l2_window32(), it tries to access kp->clips. However,
> > kp points to an userspace pointer. So, it should be obtained
> > via get_user(), otherwise it can OOPS:
> >   
> 
> 
> 
> > 
> > cc: sta...@vger.kernel.org
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > index 5198c9eeb348..4312935f1dfc 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -101,7 +101,7 @@ static int get_v4l2_window32(struct v4l2_window __user 
> > *kp,
> >  static int put_v4l2_window32(struct v4l2_window __user *kp,
> >  struct v4l2_window32 __user *up)
> >  {
> > -   struct v4l2_clip __user *kclips = kp->clips;
> > +   struct v4l2_clip __user *kclips;
> > struct v4l2_clip32 __user *uclips;
> > compat_caddr_t p;
> > u32 clipcount;
> > @@ -116,6 +116,8 @@ static int put_v4l2_window32(struct v4l2_window __user 
> > *kp,
> > if (!clipcount)
> > return 0;
> >  
> > +   if (get_user(kclips, >clips))
> > +   return -EFAULT;
> > if (get_user(p, >clips))
> > return -EFAULT;
> > uclips = compat_ptr(p);
> >   
> 
> Reviewed-by: Hans Verkuil 
> 
> I have no idea why I didn't find this when I tested this with v4l2-compliance,
> but the code was certainly wrong.

I built 4.16-rc4 with KASAN enabled. Perhaps, it won't OOPS without
it. Yet, I doubt it would work without this fix.

> 
> Thank you for debugging this!

Anytime.

Thanks,
Mauro

Re: [PATCH v7 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Niklas Söderlund]

Hi Maxime,

Thanks for your patch.

On 2018-03-26 15:34:56 +0200, Maxime Ripard wrote:
> The Cadence MIPI-CSI2 TX controller is an hardware block meant to be used
> as a bridge between pixel interfaces and a CSI-2 bus.
> 
> It supports operating with an internal or external D-PHY, with up to 4
> lanes, or without any D-PHY. The current code only supports the latter
> case.
> 
> While the virtual channel input on the pixel interface can be directly
> mapped to CSI2, the datatype input is actually a selection signal (3-bits)
> mapping to a table of up to 8 preconfigured datatypes/formats (programmed
> at start-up)
> 
> The block supports up to 8 input datatypes.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/media/platform/cadence/Kconfig   |  11 +
>  drivers/media/platform/cadence/Makefile  |   1 +
>  drivers/media/platform/cadence/cdns-csi2tx.c | 531 
> +++
>  3 files changed, 543 insertions(+)
>  create mode 100644 drivers/media/platform/cadence/cdns-csi2tx.c
> 
> diff --git a/drivers/media/platform/cadence/Kconfig 
> b/drivers/media/platform/cadence/Kconfig
> index 18f061e5cbd1..83dcf2b1814b 100644
> --- a/drivers/media/platform/cadence/Kconfig
> +++ b/drivers/media/platform/cadence/Kconfig
> @@ -14,4 +14,15 @@ config VIDEO_CADENCE_CSI2RX
> To compile this driver as a module, choose M here: the module will be
> called cdns-csi2rx.
>  
> +config VIDEO_CADENCE_CSI2TX
> + tristate "Cadence MIPI-CSI2 TX Controller"
> + depends on MEDIA_CONTROLLER
> + depends on VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
> + help
> +   Support for the Cadence MIPI CSI2 Transceiver controller.
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called cdns-csi2tx.
> +
>  endif
> diff --git a/drivers/media/platform/cadence/Makefile 
> b/drivers/media/platform/cadence/Makefile
> index 99a4086b7448..7fe992273162 100644
> --- a/drivers/media/platform/cadence/Makefile
> +++ b/drivers/media/platform/cadence/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_VIDEO_CADENCE_CSI2RX)   += cdns-csi2rx.o
> +obj-$(CONFIG_VIDEO_CADENCE_CSI2TX)   += cdns-csi2tx.o
> diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c 
> b/drivers/media/platform/cadence/cdns-csi2tx.c
> new file mode 100644
> index ..95b12d22241f
> --- /dev/null
> +++ b/drivers/media/platform/cadence/cdns-csi2tx.c
> @@ -0,0 +1,531 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Cadence MIPI-CSI2 TX Controller
> + *
> + * Copyright (C) 2017 Cadence Design Systems Inc.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CSI2TX_DEVICE_CONFIG_REG 0x00
> +
> +#define CSI2TX_CONFIG_REG0x20
> +#define CSI2TX_CONFIG_CFG_REQBIT(2)
> +#define CSI2TX_CONFIG_SRST_REQ   BIT(1)
> +
> +#define CSI2TX_DPHY_CFG_REG  0x28
> +#define CSI2TX_DPHY_CFG_CLK_RESETBIT(16)
> +#define CSI2TX_DPHY_CFG_LANE_RESET(n)BIT((n) + 12)
> +#define CSI2TX_DPHY_CFG_MODE_MASKGENMASK(9, 8)
> +#define CSI2TX_DPHY_CFG_MODE_LPDT(2 << 8)
> +#define CSI2TX_DPHY_CFG_MODE_HS  (1 << 8)
> +#define CSI2TX_DPHY_CFG_MODE_ULPS(0 << 8)
> +#define CSI2TX_DPHY_CFG_CLK_ENABLE   BIT(4)
> +#define CSI2TX_DPHY_CFG_LANE_ENABLE(n)   BIT(n)
> +
> +#define CSI2TX_DPHY_CLK_WAKEUP_REG   0x2c
> +#define CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(n)((n) & 0x)
> +
> +#define CSI2TX_DT_CFG_REG(n) (0x80 + (n) * 8)
> +#define CSI2TX_DT_CFG_DT(n)  (((n) & 0x3f) << 2)
> +
> +#define CSI2TX_DT_FORMAT_REG(n)  (0x84 + (n) * 8)
> +#define CSI2TX_DT_FORMAT_BYTES_PER_LINE(n)   (((n) & 0x) << 16)
> +#define CSI2TX_DT_FORMAT_MAX_LINE_NUM(n) ((n) & 0x)
> +
> +#define CSI2TX_STREAM_IF_CFG_REG(n)  (0x100 + (n) * 4)
> +#define CSI2TX_STREAM_IF_CFG_FILL_LEVEL(n)   ((n) & 0x1f)
> +
> +#define CSI2TX_LANES_MAX 4
> +#define CSI2TX_STREAMS_MAX   4
> +
> +enum csi2tx_pads {
> + CSI2TX_PAD_SOURCE,
> + CSI2TX_PAD_SINK_STREAM0,
> + CSI2TX_PAD_SINK_STREAM1,
> + CSI2TX_PAD_SINK_STREAM2,
> + CSI2TX_PAD_SINK_STREAM3,
> + CSI2TX_PAD_MAX,
> +};
> +
> +struct csi2tx_fmt {
> + u32 mbus;
> + u32 dt;
> + u32 bpp;
> +};
> +
> +struct csi2tx_priv {
> + struct device   *dev;
> + unsigned intcount;
> +
> + /*
> +  * Used to prevent race conditions between multiple,
> +  * concurrent calls to start and stop.
> +  */
> + struct mutexlock;
> +
> + void __iomem*base;
> +
> + struct clk  *esc_clk;
> + struct clk  *p_clk;
> + struct clk 

Re: [PATCH v2 2/2] media: Add a driver for the ov7251 camera sensor

[Sakari Ailus]

Hi Todor,

Thanks for the patch. A few quick comments below...

On Fri, Mar 23, 2018 at 12:14:20PM +0800, Todor Tomov wrote:
> The ov7251 sensor is a 1/7.5-Inch B VGA (640x480) CMOS Digital Image
> Sensor from Omnivision.
> 
> The driver supports the following modes:
> - 640x480 30fps
> - 640x480 60fps
> - 640x480 90fps
> 
> Output format is MIPI RAW 10.
> 
> The driver supports configuration via user controls for:
> - exposure and gain;
> - horizontal and vertical flip;
> - test pattern.
> 
> Signed-off-by: Todor Tomov 
> ---
>  drivers/media/i2c/Kconfig  |   13 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov7251.c | 1494 
> 
>  3 files changed, 1508 insertions(+)
>  create mode 100644 drivers/media/i2c/ov7251.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28..89aecb3 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -688,6 +688,19 @@ config VIDEO_OV5695
> To compile this driver as a module, choose M here: the
> module will be called ov5695.
>  
> +config VIDEO_OV7251
> + tristate "OmniVision OV7251 sensor support"
> + depends on OF
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + select V4L2_FWNODE
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV7251 camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov7251.
> +
>  config VIDEO_OV772X
>   tristate "OmniVision OV772x sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee..c8585b1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV5695) += ov5695.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV7251) += ov7251.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
> new file mode 100644
> index 000..7b401a9
> --- /dev/null
> +++ b/drivers/media/i2c/ov7251.c
> @@ -0,0 +1,1494 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the OV7251 camera sensor.
> + *
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017-2018, Linaro Ltd.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define OV7251_VOLTAGE_ANALOG   280
> +#define OV7251_VOLTAGE_DIGITAL_CORE 150
> +#define OV7251_VOLTAGE_DIGITAL_IO   180
> +
> +#define OV7251_SC_MODE_SELECT0x0100
> +#define OV7251_SC_MODE_SELECT_SW_STANDBY 0x0
> +#define OV7251_SC_MODE_SELECT_STREAMING  0x1
> +
> +#define OV7251_CHIP_ID_HIGH  0x300a
> +#define OV7251_CHIP_ID_HIGH_BYTE 0x77
> +#define OV7251_CHIP_ID_LOW   0x300b
> +#define OV7251_CHIP_ID_LOW_BYTE  0x50
> +#define OV7251_SC_GP_IO_IN1  0x3029
> +#define OV7251_AEC_EXPO_00x3500
> +#define OV7251_AEC_EXPO_10x3501
> +#define OV7251_AEC_EXPO_20x3502
> +#define OV7251_AEC_AGC_ADJ_0 0x350a
> +#define OV7251_AEC_AGC_ADJ_1 0x350b
> +#define OV7251_TIMING_FORMAT10x3820
> +#define OV7251_TIMING_FORMAT1_VFLIP  BIT(2)
> +#define OV7251_TIMING_FORMAT20x3821
> +#define OV7251_TIMING_FORMAT2_MIRROR BIT(2)
> +#define OV7251_PRE_ISP_000x5e00
> +#define OV7251_PRE_ISP_00_TEST_PATTERN   BIT(7)
> +
> +struct reg_value {
> + u16 reg;
> + u8 val;
> +};
> +
> +struct ov7251_mode_info {
> + u32 width;
> + u32 height;
> + const struct reg_value *data;
> + u32 data_size;
> + u32 pixel_clock;
> + u32 link_freq;
> + u16 exposure_max;
> + u16 exposure_def;
> + struct v4l2_fract timeperframe;
> +};
> +
> +struct ov7251 {
> + struct i2c_client *i2c_client;
> + struct device *dev;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct v4l2_fwnode_endpoint ep;
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_rect crop;
> + struct clk *xclk;
> +
> + struct regulator *io_regulator;
> + struct regulator *core_regulator;
> + struct regulator *analog_regulator;
> +
> + const struct ov7251_mode_info *current_mode;
> +
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *pixel_clock;
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *gain;
> +
> + /* Cached 

Re: [PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function

[Kieran Bingham]

Hi Laurent,

Thank you for another patch :D

On 26/02/18 21:45, Laurent Pinchart wrote:
> In order to make the vsp1_du_setup_lif() easier to read, and for
> symmetry with the DRM pipeline input setup, move the pipeline output
> setup code to a separate function.
> 
> Signed-off-by: Laurent Pinchart 

Just an easy code move. And I agree it improves things.

Small question below, but otherwise:

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 107 
> +++--
>  1 file changed, 61 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index 00ce99bd1605..1c8adda47440 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -276,6 +276,66 @@ static int vsp1_du_pipeline_setup_input(struct 
> vsp1_device *vsp1,
>   return 0;
>  }
>  
> +/* Setup the output side of the pipeline (WPF and LIF). */
> +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
> +  struct vsp1_pipeline *pipe)
> +{
> + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,

Why do you initialise this .which here, but all the other member variables 
below.

Wouldn't it make more sense to group all of this initialisation together? or is
there a distinction in keeping the .which separate.

(Perhaps this is just a way to initialise the rest of the structure to 0,
without using the memset?)


> + };
> + int ret;
> +
> + format.pad = RWPF_PAD_SINK;
> + format.format.width = drm_pipe->width;
> + format.format.height = drm_pipe->height;
> + format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
> + format.format.field = V4L2_FIELD_NONE;
> +
> + ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
> +);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n",
> + __func__, format.format.width, format.format.height,
> + format.format.code, pipe->output->entity.index);
> +
> + format.pad = RWPF_PAD_SOURCE;
> + ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL,
> +);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n",
> + __func__, format.format.width, format.format.height,
> + format.format.code, pipe->output->entity.index);
> +
> + format.pad = LIF_PAD_SINK;
> + ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL,
> +);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n",
> + __func__, format.format.width, format.format.height,
> + format.format.code, pipe->lif->index);
> +
> + /*
> +  * Verify that the format at the output of the pipeline matches the
> +  * requested frame size and media bus code.
> +  */
> + if (format.format.width != drm_pipe->width ||
> + format.format.height != drm_pipe->height ||
> + format.format.code != MEDIA_BUS_FMT_ARGB_1X32) {
> + dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__,
> + pipe->lif->index);
> + return -EPIPE;
> + }
> +
> + return 0;
> +}
> +
>  /* Configure all entities in the pipeline. */
>  static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
>  {
> @@ -356,7 +416,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
> pipe_index,
>   struct vsp1_drm_pipeline *drm_pipe;
>   struct vsp1_pipeline *pipe;
>   struct vsp1_bru *bru;
> - struct v4l2_subdev_format format;
>   unsigned long flags;
>   unsigned int i;
>   int ret;
> @@ -417,54 +476,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
> pipe_index,
>   if (ret < 0)
>   return ret;
>  
> - memset(, 0, sizeof(format));
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - format.pad = RWPF_PAD_SINK;
> - format.format.width = cfg->width;
> - format.format.height = cfg->height;
> - format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
> - format.format.field = V4L2_FIELD_NONE;
> -
> - ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
> -);
> + ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
>   if (ret < 0)
>   return ret;
>  
> - dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n",
> - __func__, format.format.width, format.format.height,
> - format.format.code, 

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Christian König]

Am 28.03.2018 um 21:53 schrieb Logan Gunthorpe:


On 28/03/18 01:44 PM, Christian König wrote:

Well, isn't that exactly what dma_map_resource() is good for? As far as
I can see it makes sure IOMMU is aware of the access route and
translates a CPU address into a PCI Bus address.
I'm using that with the AMD IOMMU driver and at least there it works
perfectly fine.

Yes, it would be nice, but no arch has implemented this yet. We are just
lucky in the x86 case because that arch is simple and doesn't need to do
anything for P2P (partially due to the Bus and CPU addresses being the
same). But in the general case, you can't rely on it.


Well, that an arch hasn't implemented it doesn't mean that we don't have 
the right interface to do it.



Yeah, but not for ours. See if you want to do real peer 2 peer you need
to keep both the operation as well as the direction into account.

Not sure what you are saying here... I'm pretty sure we are doing "real"
peer 2 peer...


For example when you can do writes between A and B that doesn't mean
that writes between B and A work. And reads are generally less likely to
work than writes. etc...

If both devices are behind a switch then the PCI spec guarantees that A
can both read and write B and vice versa.

Sorry to say that, but I know a whole bunch of PCI devices which
horrible ignores that.

Can you elaborate? As far as the device is concerned it shouldn't know
whether a request comes from a peer or from the host. If it does do
crazy stuff like that it's well out of spec. It's up to the switch (or
root complex if good support exists) to route the request to the device
and it's the root complex that tends to be what drops the load requests
which causes the asymmetries.


Devices integrated in the CPU usually only "claim" to be PCIe devices. 
In reality their memory request path go directly through the integrated 
north bridge. The reason for this is simple better throughput/latency.


That is hidden from the software, for example the BIOS just allocates 
address space for the BARs as if it's a normal PCIe device.


The only crux is when you then do peer2peer your request simply go into 
nirvana and are not handled by anything because the BARs are only 
visible from the CPU side of the northbridge.


Regards,
Christian.



Logan
___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 12/15] v4l: vsp1: Generalize detection of entity removal from DRM pipeline

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 26/02/18 21:45, Laurent Pinchart wrote:
> When disabling a DRM plane, the corresponding RPF is only marked as
> removed from the pipeline in the atomic update handler, with the actual
> removal happening when configuring the pipeline at atomic commit time.
> This is required as the RPF has to be disabled in the hardware, which
> can't be done from the atomic update handler.
> 
> The current implementation is RPF-specific. Make it independent of the
> entity type by using the entity's pipe pointer to mark removal from the
> pipeline. This will allow using the mechanism to remove BRU instances.

Nice improvement ...

> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index d705a6e9fa1d..6c60b72b6f50 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -346,13 +346,12 @@ static void vsp1_du_pipeline_configure(struct 
> vsp1_pipeline *pipe)
>   dl = vsp1_dl_list_get(pipe->output->dlm);
>  
>   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
> - /* Disconnect unused RPFs from the pipeline. */
> - if (entity->type == VSP1_ENTITY_RPF &&
> - !pipe->inputs[entity->index]) {
> + /* Disconnect unused entities from the pipeline. */
> + if (!entity->pipe) {
>   vsp1_dl_list_write(dl, entity->route->reg,
>  VI6_DPR_NODE_UNUSED);

I don't think it's a problem, as we don't unset the entity->pipe for arbitrary
entities, but what happens if we set an HGO/HGT entity to NULL (these currently
have 0 as the route->reg. This would risk writing VI6_DPR_NODE_UNUSED to the
VI6_CMD(0) register?

In fact any entity in the pipeline with a null pipe pointer could cause this...
so we'd best be sure that they are right. Otherwise this could cause some crazy
bugs manifesting with the hardware doing something unexpected.

Assuming that won't be a problem,

Reviewed-by: Kieran Bingham 

>  
> - entity->pipe = NULL;
> + entity->sink = NULL;
>   list_del(>list_pipe);
>  
>   continue;
> @@ -569,10 +568,11 @@ int vsp1_du_atomic_update(struct device *dev, unsigned 
> int pipe_index,
>   rpf_index);
>  
>   /*
> -  * Remove the RPF from the pipe's inputs. The atomic flush
> -  * handler will disable the input and remove the entity from the
> -  * pipe's entities list.
> +  * Remove the RPF from the pipeline's inputs. Keep it in the
> +  * pipeline's entity list to let vsp1_du_pipeline_configure()
> +  * remove it from the hardware pipeline.
>*/
> + rpf->entity.pipe = NULL;
>   drm_pipe->pipe.inputs[rpf_index] = NULL;
>   return 0;
>   }
> 

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christian König]

Am 29.03.2018 um 08:57 schrieb Daniel Vetter:

On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote:

Add a peer2peer flag noting that the importer can deal with device
resources which are not backed by pages.

Signed-off-by: Christian König 

Um strictly speaking they all should, but ttm never bothered to use the
real interfaces but just hacked around the provided sg list, grabbing the
underlying struct pages, then rebuilding the sg list again.


Actually that isn't correct. TTM converts them to a dma address array 
because drivers need it like this (at least nouveau, radeon and amdgpu).


I've fixed radeon and amdgpu to be able to deal without it and mailed 
with Ben about nouveau, but the outcome is they don't really know.


TTM itself doesn't have any need for the pages on imported BOs (you 
can't mmap them anyway), the real underlying problem is that sg tables 
doesn't provide what drivers need.


I think we could rather easily fix sg tables, but that is a totally 
separate task.



The entire point of using sg lists was exactly to allow this use case of
peer2peer dma (or well in general have special exporters which managed
memory/IO ranges not backed by struct page). So essentially you're having
a "I'm totally not broken flag" here.


No, independent of needed struct page pointers we need to note if the 
exporter can handle peer2peer stuff from the hardware side in general.


So what I've did is just to set peer2peer allowed on the importer 
because of the driver needs and clear it in the exporter if the hardware 
can't handle that.



I think a better approach would be if we add a requires_struct_page or so,
and annotate the current importers accordingly. Or we just fix them up (it
is all in shared ttm code after all, I think everyone else got this
right).


I would rather not bed on that.

Christian.


-Daniel


---
  drivers/dma-buf/dma-buf.c | 1 +
  include/linux/dma-buf.h   | 4 
  2 files changed, 5 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ffaa2f9a9c2c..f420225f93c6 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -565,6 +565,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct 
dma_buf_attach_info *info
  
  	attach->dev = info->dev;

attach->dmabuf = dmabuf;
+   attach->peer2peer = info->peer2peer;
attach->priv = info->priv;
attach->invalidate = info->invalidate;
  
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h

index 15dd8598bff1..1ef50bd9bc5b 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -313,6 +313,7 @@ struct dma_buf {
   * @dmabuf: buffer for this attachment.
   * @dev: device attached to the buffer.
   * @node: list of dma_buf_attachment.
+ * @peer2peer: true if the importer can handle peer resources without pages.
   * @priv: exporter specific attachment data.
   *
   * This structure holds the attachment information between the dma_buf buffer
@@ -328,6 +329,7 @@ struct dma_buf_attachment {
struct dma_buf *dmabuf;
struct device *dev;
struct list_head node;
+   bool peer2peer;
void *priv;
  
  	/**

@@ -392,6 +394,7 @@ struct dma_buf_export_info {
   * @dmabuf:   the exported dma_buf
   * @dev:  the device which wants to import the attachment
   * @priv: private data of importer to this attachment
+ * @peer2peer: true if the importer can handle peer resources without pages
   * @invalidate:   callback to use for invalidating mappings
   *
   * This structure holds the information required to attach to a buffer. Used
@@ -401,6 +404,7 @@ struct dma_buf_attach_info {
struct dma_buf *dmabuf;
struct device *dev;
void *priv;
+   bool peer2peer;
void (*invalidate)(struct dma_buf_attachment *attach);
  };
  
--

2.14.1

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v13 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Maxime Ripard]

Hi Niklas,

On Tue, Feb 13, 2018 at 12:01:32AM +0100, Niklas Söderlund wrote:
> + switch (priv->lanes) {
> + case 1:
> + phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_0;
> + break;
> + case 2:
> + phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> + break;
> + case 4:
> + phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2 |
> + PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
> + break;
> + default:
> + return -EINVAL;
> + }

I guess you could have a simpler construct here using this:

phycnt = PHYCNT_ENABLECLK;

switch (priv->lanes) {
case 4:
phycnt |= PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2;
case 2:
phycnt |= PHYCNT_ENABLE_1;
case 1:
phycnt |= PHYCNT_ENABLE_0;
break;

default:
return -EINVAL;
}

But that's really up to you.

> +static int rcar_csi2_probe(struct platform_device *pdev)
> +{
> + const struct soc_device_attribute *attr;
> + struct rcar_csi2 *priv;
> + unsigned int i;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->info = of_device_get_match_data(>dev);
> +
> + /* r8a7795 ES1.x behaves different then ES2.0+ but no own compat */
> + attr = soc_device_match(r8a7795es1);
> + if (attr)
> + priv->info = attr->data;
> +
> + priv->dev = >dev;
> +
> + mutex_init(>lock);
> + priv->stream_count = 0;
> +
> + ret = rcar_csi2_probe_resources(priv, pdev);
> + if (ret) {
> + dev_err(priv->dev, "Failed to get resources\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = rcar_csi2_parse_dt(priv);
> + if (ret)
> + return ret;
> +
> + priv->subdev.owner = THIS_MODULE;
> + priv->subdev.dev = >dev;
> + v4l2_subdev_init(>subdev, _csi2_subdev_ops);
> + v4l2_set_subdevdata(>subdev, >dev);
> + snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> +  KBUILD_MODNAME, dev_name(>dev));
> + priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->subdev.entity.ops = _csi2_entity_ops;
> +
> + priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> + for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> + priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(>subdev.entity, NR_OF_RCAR_CSI2_PAD,
> +  priv->pads);
> + if (ret)
> + goto error;
> +
> + pm_runtime_enable(>dev);

Is CONFIG_PM mandatory on Renesas SoCs? If not, you end up with the
device uninitialised at probe, and pm_runtime_get_sync will not
initialise it either if CONFIG_PM is not enabled. I guess you could
call your runtime_resume function unconditionally, and mark the device
as active in runtime_pm using pm_runtime_set_active.

Looks good otherwise, once fixed (and if relevant):
Reviewed-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Hans Verkuil]

On 29/03/18 11:52, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, Mar 29, 2018 at 11:16:45AM +0200, Hans Verkuil wrote:
>> On 29/03/18 11:01, Sakari Ailus wrote:
>>> On Thu, Mar 29, 2018 at 10:57:44AM +0200, Hans Verkuil wrote:
 On 29/03/18 10:45, Sakari Ailus wrote:
> Hi Hans,
>
> On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
> ...
>> @@ -88,6 +96,8 @@ struct media_device_ops {
>>   * @disable_source: Disable Source Handler function pointer
>>   *
>>   * @ops:Operation handler callbacks
>> + * @req_lock:   Serialise access to requests
>> + * @req_queue_mutex: Serialise validating and queueing requests
>
> s/validating and//
>
> As there's no more a separate validation step. Then,

 Well, you validate before queuing. It's not a separate step, but
 part of the queue operation. See patch 23 where this is implemented
 in the vb2_request_helper function.
>>>
>>> Works for me. I think we'll need the validate op sooner or later anyway.
>>>
>>
>>
>> There is one. Request objects have prepare, unprepare and queue ops.
>>
>> The request req_queue op will prepare all objects, and only queue if the
>> prepare (aka validate) step succeeds for all objects. If not, then the
> 
> You can't validate the objects in a request separately from other objects
> in it, the validation needs to happen at the request level. There are two
> reasons for this:
> 
> - The objects in a request must be compatible with all other objects in the
>   request and
> 
> - The request must contain the required objects in order to be valid ---
>   e.g. for a device producing multiple capture buffers from one output
>   buffer, the output buffer is mandatory whereas one or more of the capture
>   buffers are not.
> 
> The latter could presumably be implemented separately for each object but
> then the driver needs to go fishing for the related objects which may not
> be very efficient.
> 
> What I'm proposing is to put this at the level of a request, at least for
> now.
> 

The driver implements req_queue. It can do whatever it wants there, it has
full control.

However, as part of that process objects still have to be prepared (for
buffers that literally means calling buf_prepare). That step does the
validation on an object level and also possible memory/resource allocations
that can fail.

The helper function in patch 23 is however sufficient for simple drivers
like vim2m, vivid, codec drivers, etc. that do not need to validate the
request as a whole.

Drivers that do need to validate the request as a whole will not use that
helper function, they will do this themselves.

Regards,

Hans

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

[jacopo mondi]

Hi Todor,

On Thu, Mar 29, 2018 at 10:50:10AM +0300, Todor Tomov wrote:
> Hi Jacopo,
>
>
> >
> > With the above nits clarified, and as you addressed my v1 comments:
> >
> > Reviewed-by: Jacopo Mondi 
>
> Would you like to see the corrections or I can add the tag before sending 
> them?
>

I don't have any more comment, feel free to add it :)

Thanks
   j


signature.asc
Description: PGP signature

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

[Sakari Ailus]

Hi Todor,

On Thu, Mar 29, 2018 at 01:09:18PM +0300, Todor Tomov wrote:
> > There's another change needed, too, which is not using of_match_ptr
> > macro, but instead assigning the of_match_table unconditionally.
> 
> In that case the MODULE_DEVICE_TABLE(i2c, ...) is again not needed?
> And matching will be again via of_match_table?

Correct.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [RFCv9 PATCH 05/29] media-request: add request ioctls

[Sakari Ailus]

Hi Hans,

I think it'd be easier to review the code if it was a part of the previous
patch. It's so closely related to what's being done there.

On Wed, Mar 28, 2018 at 03:50:06PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Implement the MEDIA_REQUEST_IOC_QUEUE and MEDIA_REQUEST_IOC_REINIT
> ioctls.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/media-request.c | 80 
> +--
>  1 file changed, 78 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
> index 8135d9d32af9..3ee3b27fd644 100644
> --- a/drivers/media/media-request.c
> +++ b/drivers/media/media-request.c
> @@ -105,10 +105,86 @@ static unsigned int media_request_poll(struct file 
> *filp,
>   return 0;
>  }
>  
> +static long media_request_ioctl_queue(struct media_request *req)
> +{
> + struct media_device *mdev = req->mdev;
> + unsigned long flags;
> + int ret = 0;
> +
> + dev_dbg(mdev->dev, "request: queue %s\n", req->debug_str);
> +
> + spin_lock_irqsave(>lock, flags);
> + if (req->state != MEDIA_REQUEST_STATE_IDLE) {
> + dev_dbg(mdev->dev,
> + "request: unable to queue %s, request in state %s\n",
> + req->debug_str, media_request_state_str(req->state));

You could print the message after releasing the spinlock. Maybe store the
request state locally first?

> + spin_unlock_irqrestore(>lock, flags);
> + return -EINVAL;
> + }
> + req->state = MEDIA_REQUEST_STATE_QUEUEING;
> +
> + spin_unlock_irqrestore(>lock, flags);
> +
> + /*
> +  * Ensure the request that is validated will be the one that gets queued
> +  * next by serialising the queueing process.
> +  */
> + mutex_lock(>req_queue_mutex);
> +
> + ret = mdev->ops->req_queue(req);
> + spin_lock_irqsave(>lock, flags);
> + req->state = ret ? MEDIA_REQUEST_STATE_IDLE : 
> MEDIA_REQUEST_STATE_QUEUED;
> + spin_unlock_irqrestore(>lock, flags);
> + mutex_unlock(>req_queue_mutex);
> +
> + if (ret) {
> + dev_dbg(mdev->dev, "request: can't queue %s (%d)\n",
> + req->debug_str, ret);
> + } else {
> + media_request_get(req);
> + }
> +
> + return ret;
> +}
> +
> +static long media_request_ioctl_reinit(struct media_request *req)
> +{
> + struct media_device *mdev = req->mdev;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>lock, flags);
> + if (req->state != MEDIA_REQUEST_STATE_IDLE &&
> + req->state != MEDIA_REQUEST_STATE_COMPLETE) {
> + dev_dbg(mdev->dev,
> + "request: %s not in idle or complete state, cannot 
> reinit\n",
> + req->debug_str);
> + spin_unlock_irqrestore(>lock, flags);
> + return -EINVAL;
> + }
> + req->state = MEDIA_REQUEST_STATE_CLEANING;
> + spin_unlock_irqrestore(>lock, flags);
> +
> + media_request_clean(req);
> +
> + spin_lock_irqsave(>lock, flags);
> + req->state = MEDIA_REQUEST_STATE_IDLE;
> + spin_unlock_irqrestore(>lock, flags);

Newline?

> + return 0;
> +}
> +
>  static long media_request_ioctl(struct file *filp, unsigned int cmd,
> - unsigned long __arg)
> + unsigned long arg)

This belongs to the patch adding media_request_ioctl().

>  {
> - return -ENOIOCTLCMD;
> + struct media_request *req = filp->private_data;
> +
> + switch (cmd) {
> + case MEDIA_REQUEST_IOC_QUEUE:
> + return media_request_ioctl_queue(req);
> + case MEDIA_REQUEST_IOC_REINIT:
> + return media_request_ioctl_reinit(req);
> + default:
> + return -ENOIOCTLCMD;
> + }
>  }
>  
>  static const struct file_operations request_fops = {

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

[Todor Tomov]

Hi Sakari

On 29.03.2018 11:29, Sakari Ailus wrote:
> Hi Todor and Jacopo,
> 
> On Thu, Mar 29, 2018 at 10:50:10AM +0300, Todor Tomov wrote:
> ...
 +static const struct of_device_id ov7251_of_match[] = {
 +  { .compatible = "ovti,ov7251" },
 +  { /* sentinel */ }
 +};
 +MODULE_DEVICE_TABLE(of, ov7251_of_match);
 +
 +static struct i2c_driver ov7251_i2c_driver = {
 +  .driver = {
 +  .of_match_table = of_match_ptr(ov7251_of_match),
 +  .name  = "ov7251",
 +  },
 +  .probe  = ov7251_probe,
 +  .remove = ov7251_remove,
 +  .id_table = ov7251_id,
>>>
>>> As this driver depends on CONFIG_OF, I've been suggested to use probe_new 
>>> and
>>> get rid of i2c id_tables.
>>
>> Yes, I'll do that.
> 
> The proposal sounds good to me but rather than adding CONFIG_OF dependency,
> I'd instead suggest changing the of_property_read_u32 to
> fwnode_property_read_u32; then the driver may work on ACPI based systems as
> well. 

Ok.

> There's another change needed, too, which is not using of_match_ptr
> macro, but instead assigning the of_match_table unconditionally.

In that case the MODULE_DEVICE_TABLE(i2c, ...) is again not needed?
And matching will be again via of_match_table?

> 
> Up to you.
> 

Thank you for your help!
 
Best regards,
Todor Tomov

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Sakari Ailus]

Hi Hans,

On Thu, Mar 29, 2018 at 11:16:45AM +0200, Hans Verkuil wrote:
> On 29/03/18 11:01, Sakari Ailus wrote:
> > On Thu, Mar 29, 2018 at 10:57:44AM +0200, Hans Verkuil wrote:
> >> On 29/03/18 10:45, Sakari Ailus wrote:
> >>> Hi Hans,
> >>>
> >>> On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
> >>> ...
>  @@ -88,6 +96,8 @@ struct media_device_ops {
>    * @disable_source: Disable Source Handler function pointer
>    *
>    * @ops:Operation handler callbacks
>  + * @req_lock:   Serialise access to requests
>  + * @req_queue_mutex: Serialise validating and queueing requests
> >>>
> >>> s/validating and//
> >>>
> >>> As there's no more a separate validation step. Then,
> >>
> >> Well, you validate before queuing. It's not a separate step, but
> >> part of the queue operation. See patch 23 where this is implemented
> >> in the vb2_request_helper function.
> > 
> > Works for me. I think we'll need the validate op sooner or later anyway.
> > 
> 
> 
> There is one. Request objects have prepare, unprepare and queue ops.
> 
> The request req_queue op will prepare all objects, and only queue if the
> prepare (aka validate) step succeeds for all objects. If not, then the

You can't validate the objects in a request separately from other objects
in it, the validation needs to happen at the request level. There are two
reasons for this:

- The objects in a request must be compatible with all other objects in the
  request and

- The request must contain the required objects in order to be valid ---
  e.g. for a device producing multiple capture buffers from one output
  buffer, the output buffer is mandatory whereas one or more of the capture
  buffers are not.

The latter could presumably be implemented separately for each object but
then the driver needs to go fishing for the related objects which may not
be very efficient.

What I'm proposing is to put this at the level of a request, at least for
now.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH for v3.18 00/18] Backport CVE-2017-13166 fixes to Kernel 3.18

[Inki Dae]

2018년 03월 29일 16:00에 Greg KH 이(가) 쓴 글:
> On Thu, Mar 29, 2018 at 03:39:54PM +0900, Inki Dae wrote:
>> 2018년 03월 29일 13:25에 Greg KH 이(가) 쓴 글:
>>> On Thu, Mar 29, 2018 at 08:22:08AM +0900, Inki Dae wrote:
 Really thanks for doing this. :) There would be many users who use
 Linux-3.18 for their products yet.
>>>
>>> For new products?  They really should not be.  The kernel is officially
>>
>> Really no. Old products would still be using Linux-3.18 kernel without
>> kernel upgrade. For new product, most of SoC vendors will use
>> Linux-4.x including us.
>> Actually, we are preparing for kernel upgrade for some devices even
>> some old devices (to Linux-4.14-LTS) and almost done.
> 
> That is great to hear.
> 
>>> What is keeping you on 3.18.y and not allowing you to move to a newer
>>> kernel version?
>>
>> We also want to move to latest kernel version. However, there is a case that 
>> we cannot upgrade the kernel.
>> In case that SoC vendor never share firmwares and relevant data
>> sheets, we cannot upgrade the kernel. However, we have to resolve the
>> security issues for users of this device.
> 
> It sounds like you need to be getting those security updates from those
> SoC vendors, as they are the ones you are paying for support for that

It's true but some open source developers like me who use vendor kernel without 
vendor's support will never get the security updates from them.
So if you merge CVE patches even through this kernel is already EOL then many 
open source developers would be glad. :)

Thanks,
Inki Dae

> kernel version that they are forcing you to stay on.
> 
> good luck!
> 
> greg k-h
> 
> 
> 

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Hans Verkuil]

On 29/03/18 11:01, Sakari Ailus wrote:
> On Thu, Mar 29, 2018 at 10:57:44AM +0200, Hans Verkuil wrote:
>> On 29/03/18 10:45, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
>>> ...
 @@ -88,6 +96,8 @@ struct media_device_ops {
   * @disable_source: Disable Source Handler function pointer
   *
   * @ops:  Operation handler callbacks
 + * @req_lock: Serialise access to requests
 + * @req_queue_mutex: Serialise validating and queueing requests
>>>
>>> s/validating and//
>>>
>>> As there's no more a separate validation step. Then,
>>
>> Well, you validate before queuing. It's not a separate step, but
>> part of the queue operation. See patch 23 where this is implemented
>> in the vb2_request_helper function.
> 
> Works for me. I think we'll need the validate op sooner or later anyway.
> 


There is one. Request objects have prepare, unprepare and queue ops.

The request req_queue op will prepare all objects, and only queue if the
prepare (aka validate) step succeeds for all objects. If not, then the
objects that were prepared will be rolled back with the unprepare op
and req_queue returns an error.

Note that the object ops are not documented at the moment, it's one of
my TODOs.

Regards,

Hans

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Sakari Ailus]

On Thu, Mar 29, 2018 at 10:57:44AM +0200, Hans Verkuil wrote:
> On 29/03/18 10:45, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
> > ...
> >> @@ -88,6 +96,8 @@ struct media_device_ops {
> >>   * @disable_source: Disable Source Handler function pointer
> >>   *
> >>   * @ops:  Operation handler callbacks
> >> + * @req_lock: Serialise access to requests
> >> + * @req_queue_mutex: Serialise validating and queueing requests
> > 
> > s/validating and//
> > 
> > As there's no more a separate validation step. Then,
> 
> Well, you validate before queuing. It's not a separate step, but
> part of the queue operation. See patch 23 where this is implemented
> in the vb2_request_helper function.

Works for me. I think we'll need the validate op sooner or later anyway.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Hans Verkuil]

On 29/03/18 10:45, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
> ...
>> @@ -88,6 +96,8 @@ struct media_device_ops {
>>   * @disable_source: Disable Source Handler function pointer
>>   *
>>   * @ops:Operation handler callbacks
>> + * @req_lock:   Serialise access to requests
>> + * @req_queue_mutex: Serialise validating and queueing requests
> 
> s/validating and//
> 
> As there's no more a separate validation step. Then,

Well, you validate before queuing. It's not a separate step, but
part of the queue operation. See patch 23 where this is implemented
in the vb2_request_helper function.

Regards,

Hans

> 
> Acked-by: Sakari Ailus 
> 

Re: [RFCv9 PATCH 03/29] media-request: allocate media requests

[Sakari Ailus]

Hi Hans,

On Wed, Mar 28, 2018 at 03:50:04PM +0200, Hans Verkuil wrote:
...
> @@ -88,6 +96,8 @@ struct media_device_ops {
>   * @disable_source: Disable Source Handler function pointer
>   *
>   * @ops: Operation handler callbacks
> + * @req_lock:Serialise access to requests
> + * @req_queue_mutex: Serialise validating and queueing requests

s/validating and//

As there's no more a separate validation step. Then,

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [RFCv9 PATCH 02/29] uapi/linux/media.h: add request API

[Sakari Ailus]

On Wed, Mar 28, 2018 at 03:50:03PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Define the public request API.
> 
> This adds the new MEDIA_IOC_REQUEST_ALLOC ioctl to allocate a request
> and two ioctls that operate on a request in order to queue the
> contents of the request to the driver and to re-initialize the
> request.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  include/uapi/linux/media.h | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index c7e9a5cba24e..f8769e74f847 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -342,11 +342,19 @@ struct media_v2_topology {
>  
>  /* ioctls */
>  
> +struct __attribute__ ((packed)) media_request_alloc {
> + __s32 fd;
> +};
> +
>  #define MEDIA_IOC_DEVICE_INFO_IOWR('|', 0x00, struct 
> media_device_info)
>  #define MEDIA_IOC_ENUM_ENTITIES  _IOWR('|', 0x01, struct 
> media_entity_desc)
>  #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
>  #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
>  #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
> +#define MEDIA_IOC_REQUEST_ALLOC  _IOWR('|', 0x05, struct 
> media_request_alloc)
> +
> +#define MEDIA_REQUEST_IOC_QUEUE  _IO('|',  0x80)
> +#define MEDIA_REQUEST_IOC_REINIT _IO('|',  0x81)
>  
>  #if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API)
>  

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [RFCv9 PATCH 01/29] v4l2-device.h: always expose mdev

[Sakari Ailus]

On Wed, Mar 28, 2018 at 03:50:02PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The mdev field is only present if CONFIG_MEDIA_CONTROLLER is set.
> But since we will need to pass the media_device to vb2 snd the
> control framework it is very convenient to just make this field
> available all the time. If CONFIG_MEDIA_CONTROLLER is not set,
> then it will just be NULL.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  include/media/v4l2-device.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
> index 0c9e4da55499..b330e4a08a6b 100644
> --- a/include/media/v4l2-device.h
> +++ b/include/media/v4l2-device.h
> @@ -33,7 +33,7 @@ struct v4l2_ctrl_handler;
>   * struct v4l2_device - main struct to for V4L2 device drivers
>   *
>   * @dev: pointer to struct device.
> - * @mdev: pointer to struct media_device
> + * @mdev: pointer to struct media_device, may be NULL.
>   * @subdevs: used to keep track of the registered subdevs
>   * @lock: lock this struct; can be used by the driver as well
>   *   if this struct is embedded into a larger struct.
> @@ -58,9 +58,7 @@ struct v4l2_ctrl_handler;
>   */
>  struct v4l2_device {
>   struct device *dev;
> -#if defined(CONFIG_MEDIA_CONTROLLER)
>   struct media_device *mdev;
> -#endif
>   struct list_head subdevs;
>   spinlock_t lock;
>   char name[V4L2_DEVICE_NAME_SIZE];

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Hans Verkuil]

Hi Mauro,

On 28/03/18 19:59, Mauro Carvalho Chehab wrote:
> At put_v4l2_window32(), it tries to access kp->clips. However,
> kp points to an userspace pointer. So, it should be obtained
> via get_user(), otherwise it can OOPS:
> 



> 
> cc: sta...@vger.kernel.org
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 5198c9eeb348..4312935f1dfc 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -101,7 +101,7 @@ static int get_v4l2_window32(struct v4l2_window __user 
> *kp,
>  static int put_v4l2_window32(struct v4l2_window __user *kp,
>struct v4l2_window32 __user *up)
>  {
> - struct v4l2_clip __user *kclips = kp->clips;
> + struct v4l2_clip __user *kclips;
>   struct v4l2_clip32 __user *uclips;
>   compat_caddr_t p;
>   u32 clipcount;
> @@ -116,6 +116,8 @@ static int put_v4l2_window32(struct v4l2_window __user 
> *kp,
>   if (!clipcount)
>   return 0;
>  
> + if (get_user(kclips, >clips))
> + return -EFAULT;
>   if (get_user(p, >clips))
>   return -EFAULT;
>   uclips = compat_ptr(p);
> 

Reviewed-by: Hans Verkuil 

I have no idea why I didn't find this when I tested this with v4l2-compliance,
but the code was certainly wrong.

Thank you for debugging this!

Regards,

Hans

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

[Sakari Ailus]

Hi Todor and Jacopo,

On Thu, Mar 29, 2018 at 10:50:10AM +0300, Todor Tomov wrote:
...
> >> +static const struct of_device_id ov7251_of_match[] = {
> >> +  { .compatible = "ovti,ov7251" },
> >> +  { /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, ov7251_of_match);
> >> +
> >> +static struct i2c_driver ov7251_i2c_driver = {
> >> +  .driver = {
> >> +  .of_match_table = of_match_ptr(ov7251_of_match),
> >> +  .name  = "ov7251",
> >> +  },
> >> +  .probe  = ov7251_probe,
> >> +  .remove = ov7251_remove,
> >> +  .id_table = ov7251_id,
> > 
> > As this driver depends on CONFIG_OF, I've been suggested to use probe_new 
> > and
> > get rid of i2c id_tables.
> 
> Yes, I'll do that.

The proposal sounds good to me but rather than adding CONFIG_OF dependency,
I'd instead suggest changing the of_property_read_u32 to
fwnode_property_read_u32; then the driver may work on ACPI based systems as
well. There's another change needed, too, which is not using of_match_ptr
macro, but instead assigning the of_match_table unconditionally.

Up to you.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v2 1/2] dt-bindings: media: Binding document for OV7251 camera sensor

[Sakari Ailus]

On Fri, Mar 23, 2018 at 12:14:19PM +0800, Todor Tomov wrote:
> Add the document for ov7251 device tree binding.
> 
> CC: Rob Herring 
> CC: Mark Rutland 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Todor Tomov 
> Reviewed-by: Rob Herring 
> ---
>  .../devicetree/bindings/media/i2c/ov7251.txt   | 51 
> ++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7251.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7251.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> new file mode 100644
> index 000..4ee6888
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> @@ -0,0 +1,51 @@
> +* Omnivision 1/7.5-Inch B VGA CMOS Digital Image Sensor
> +
> +The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor 
> with
> +an active array size of 640H x 480V. It is programmable through a serial I2C
> +interface.
> +
> +Required Properties:
> +- compatible: Value should be "ovti,ov7251".
> +- clocks: Reference to the xclk clock.
> +- clock-names: Should be "xclk".
> +- clock-frequency: Frequency of the xclk clock.
> +- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This 
> corresponds
> +  to the hardware pin XSHUTDOWN which is physically active low.
> +- vdddo-supply: Chip digital IO regulator.
> +- vdda-supply: Chip analog regulator.
> +- vddd-supply: Chip digital core regulator.
> +
> +The device node must contain one 'port' child node for its digital output

Could you add there shall be a single endpoint node in the port node as
well?

> +video port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> +  {
> + ...
> +
> + ov7251: camera-sensor@60 {
> + compatible = "ovti,ov7251";
> + reg = <0x60>;
> +
> + enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_bw_default>;
> +
> + clocks = < 200>;
> + clock-names = "xclk";
> + clock-frequency = <2400>;
> +
> + vdddo-supply = <_dovdd_1v8>;
> + vdda-supply = <_avdd_2v8>;
> + vddd-supply = <_dvdd_1v2>;
> +
> + port {
> + ov7251_ep: endpoint {
> + clock-lanes = <1>;
> + data-lanes = <0>;
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> + };
> -- 
> 2.7.4
> 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

[Todor Tomov]

Hi Jacopo,

Thank you for your prompt review.

On 23.03.2018 15:40, jacopo mondi wrote:
> Hi Todor,
>thanks for the patch.
> 
> When running checkpatch --strict I see a few warning you can easily
> close (braces indentation mostly, and one additional empty line at
> line 1048).

Thank you for pointing me to the --strict mode. There are a few CHECKs for
braces alignment for which the alignment is still better as it is now
I think. However there were also a few reasonable points and I have
updated the code according to them.

> 
> A few more nits below.
> 
> On Fri, Mar 23, 2018 at 12:14:20PM +0800, Todor Tomov wrote:
>> The ov7251 sensor is a 1/7.5-Inch B VGA (640x480) CMOS Digital Image
>> Sensor from Omnivision.
>>
>> The driver supports the following modes:
>> - 640x480 30fps
>> - 640x480 60fps
>> - 640x480 90fps
>>
>> Output format is MIPI RAW 10.
>>
>> The driver supports configuration via user controls for:
>> - exposure and gain;
>> - horizontal and vertical flip;
>> - test pattern.
>>
>> Signed-off-by: Todor Tomov 
>> ---
>>  drivers/media/i2c/Kconfig  |   13 +
>>  drivers/media/i2c/Makefile |1 +
>>  drivers/media/i2c/ov7251.c | 1494 
>> 
>>  3 files changed, 1508 insertions(+)
>>  create mode 100644 drivers/media/i2c/ov7251.c
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 541f0d28..89aecb3 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -688,6 +688,19 @@ config VIDEO_OV5695
>>To compile this driver as a module, choose M here: the
>>module will be called ov5695.
>>
>> +config VIDEO_OV7251
>> +tristate "OmniVision OV7251 sensor support"
>> +depends on OF
>> +depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> +depends on MEDIA_CAMERA_SUPPORT
>> +select V4L2_FWNODE
>> +---help---
>> +  This is a Video4Linux2 sensor-level driver for the OmniVision
>> +  OV7251 camera.
>> +
>> +  To compile this driver as a module, choose M here: the
>> +  module will be called ov7251.
>> +
>>  config VIDEO_OV772X
>>  tristate "OmniVision OV772x sensor support"
>>  depends on I2C && VIDEO_V4L2
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index ea34aee..c8585b1 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>>  obj-$(CONFIG_VIDEO_OV5695) += ov5695.o
>>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
>> +obj-$(CONFIG_VIDEO_OV7251) += ov7251.o
>>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>>  obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
>> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
>> new file mode 100644
>> index 000..7b401a9
>> --- /dev/null
>> +++ b/drivers/media/i2c/ov7251.c
>> @@ -0,0 +1,1494 @@



>> +static int ov7251_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> +struct ov7251 *ov7251 = to_ov7251(sd);
>> +int ret = 0;
>> +
>> +mutex_lock(>power_lock);
>> +
>> +/*
>> + * If the power state is modified from 0 to 1 or from 1 to 0,
>> + * update the power state.
>> + */
>> +if (ov7251->power_on == !on) {
> 
> if (ov7251->power_on == !!on) {
> mutex_unlock(>power_lock);
> return 0;
> }
> 
> And you can save one indentation level and remove ret initialization.
> 

Good hint, I'd rather save one indentation level by:

if (ov7251->power_on == !!on)
goto exit;

> 
>> +if (on) {
>> +ret = ov7251_set_power_on(ov7251);
>> +if (ret < 0)
>> +goto exit;
>> +
>> +ret = ov7251_set_register_array(ov7251,
>> +ov7251_global_init_setting,
>> +ARRAY_SIZE(ov7251_global_init_setting));
>> +if (ret < 0) {
>> +dev_err(ov7251->dev,
>> +"could not set init registers\n");
>> +ov7251_set_power_off(ov7251);
>> +goto exit;
>> +}
>> +
>> +ov7251->power_on = true;
>> +} else {
>> +ov7251_set_power_off(ov7251);
>> +ov7251->power_on = false;
>> +}
>> +}
>> +
>> +exit:
>> +mutex_unlock(>power_lock);
>> +
>> +return ret;
>> +}
>> +



>> +
>> +static int ov7251_enum_frame_size(struct v4l2_subdev *subdev,
>> +  struct v4l2_subdev_pad_config *cfg,
>> +  struct v4l2_subdev_frame_size_enum *fse)
>> +{
> 
> Shouldn't you check for (pad != 0) in all subdev pad operations?
> I see other driver with a single pad doing this...

I looked up now and I can 

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Laurent Pinchart]

Hi Sakari,

On Thursday, 29 March 2018 10:35:49 EEST Sakari Ailus wrote:
> On Thu, Mar 29, 2018 at 09:19:43AM +0300, Laurent Pinchart wrote:
> > On Wednesday, 28 March 2018 23:16:08 EEST Sakari Ailus wrote:
> > > On Wed, Mar 28, 2018 at 02:59:22PM -0300, Mauro Carvalho Chehab wrote:
> > > > At put_v4l2_window32(), it tries to access kp->clips. However,
> > > > kp points to an userspace pointer. So, it should be obtained
> > > > 
> > > > via get_user(), otherwise it can OOPS:
> > > >  vivid-000: ==  END STATUS  ==
> > > >  BUG: unable to handle kernel paging request at fffb18e0
> > > >  IP: [] __put_v4l2_format32+0x169/0x220 [videodev]
> > > >  PGD 3f5776067 PUD 3f576f067 PMD 3f5769067 PTE 80042548f067
> > > >  Oops: 0001 [#1] SMP
> > > >  Modules linked in: vivid videobuf2_vmalloc videobuf2_memops
> > > >  v4l2_dv_timings videobuf2_core v4l2_common videodev media xt_CHECKSUM
> > > >  iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat
> > > >  nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack
> > > >  nf_conntrack tun bridge stp llc ebtable_filter ebtables
> > > >  ip6table_filter
> > > >  ip6_tables bluetooth rfkill binfmt_misc snd_hda_codec_hdmi i915
> > > >  snd_hda_intel snd_hda_controller snd_hda_codec intel_rapl
> > > >  x86_pkg_temp_thermal snd_hwdep intel_powerclamp snd_pcm coretemp
> > > >  snd_seq_midi kvm_intel kvm snd_seq_midi_event snd_rawmidi
> > > >  i2c_algo_bit
> > > >  drm_kms_helper snd_seq drm crct10dif_pclmul e1000e snd_seq_device
> > > >  crc32_pclmul snd_timer ghash_clmulni_intel snd mei_me mei ptp
> > > >  pps_core
> > > >  soundcore lpc_ich video crc32c_intel [last unloaded: media] CPU: 2
> > > >  PID:
> > > >  
> > > >  28332 Comm: v4l2-compliance Not tainted 3.18.102+ #107 Hardware name:
> > > >/NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> > > >  
> > > >  05/11/2017 task: 8804293f8000 ti: 8803f564 task.ti:
> > > >  8803f564 RIP: 0010:[]  []
> > > >  __put_v4l2_format32+0x169/0x220 [videodev] RSP: 0018:8803f5643e28
> > > >  EFLAGS: 00010246
> > > >  RAX:  RBX:  RCX: fffb1ab4
> > > >  RDX: fffb1a68 RSI: fffb18d8 RDI: fffb1aa8
> > > >  RBP: 8803f5643e48 R08: 0001 R09: 8803f54b0378
> > > >  R10:  R11: 0168 R12: fffb18c0
> > > >  R13: fffb1a94 R14: fffb18c8 R15: 
> > > >  FS:  () GS:880456d0(0063)
> > > >  knlGS:f7100980 CS:  0010 DS: 002b ES: 002b CR0:
> > > >  80050033
> > > >  CR2: fffb18e0 CR3: 0003f552b000 CR4: 003407e0
> > > >  
> > > >  Stack:
> > > >   fffb1a94 c0cc5640 0056 8804274f3600
> > > >   8803f5643ed0 c0547e16 0003 8803f5643eb0
> > > >   81301460 88009db44b01 880441942520 8800c0d05640
> > > >  
> > > >  Call Trace:
> > > >   [] v4l2_compat_ioctl32+0x12d6/0x1b1d [videodev]
> > > >   [] ? file_has_perm+0x70/0xc0
> > > >   [] compat_SyS_ioctl+0xec/0x1200
> > > >   [] sysenter_dispatch+0x7/0x21
> > > >  
> > > >  Code: 00 00 48 8b 80 48 c0 ff ff 48 83 e8 38 49 39 c6 0f 87 2b ff ff
> > > >  ff
> > > >  49 8d 45 1c e8 a3 ce e3 c0 85 c0 0f 85 1a ff ff ff 41 8d 40 ff <4d>
> > > >  8b
> > > >  64 24 20 41 89 d5 48 8d 44 40 03 4d 8d 34 c4 eb 15 0f 1f RIP
> > > >  [] __put_v4l2_format32+0x169/0x220 [videodev] RSP
> > > >  
> > > >  CR2: fffb18e0
> > > > 
> > > > Tested with vivid driver on Kernel v3.18.102.
> > > > 
> > > > Same bug happens upstream too:
> > > >  BUG: KASAN: user-memory-access in __put_v4l2_format32+0x98/0x4d0
> > > >  [videodev]
> > > >  Read of size 8 at addr ffe48400 by task v4l2-compliance/8713
> > > >  
> > > >  CPU: 0 PID: 8713 Comm: v4l2-compliance Not tainted 4.16.0-rc4+ #108
> > > >  Hardware name:  /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> > > >  05/11/2017>
> > > >  
> > > >  Call Trace:
> > > >   dump_stack+0x5c/0x7c
> > > >   kasan_report+0x164/0x380
> > > >   ? __put_v4l2_format32+0x98/0x4d0 [videodev]
> > > >   __put_v4l2_format32+0x98/0x4d0 [videodev]
> > > >   v4l2_compat_ioctl32+0x1aec/0x27a0 [videodev]
> > > >   ? __fsnotify_inode_delete+0x20/0x20
> > > >   ? __put_v4l2_format32+0x4d0/0x4d0 [videodev]
> > > >   compat_SyS_ioctl+0x646/0x14d0
> > > >   ? do_ioctl+0x30/0x30
> > > >   do_fast_syscall_32+0x191/0x3f4
> > > >   entry_SYSENTER_compat+0x6b/0x7a
> > > >  
> > > >  ==
> > > >  Disabling lock debugging due to kernel taint
> > > >  BUG: unable to handle kernel paging request at ffe48400
> > > >  IP: __put_v4l2_format32+0x98/0x4d0 [videodev]
> > > >  PGD 3a22fb067 P4D 3a22fb067 PUD 39b6f0067 PMD 39b6f1067 PTE
> > > >  8003256af067 Oops: 0001 [#1] SMP KASAN
> > > >  Modules linked in: vivid videobuf2_vmalloc videobuf2_dma_contig
> > 

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Sakari Ailus]

On Thu, Mar 29, 2018 at 09:19:43AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> On Wednesday, 28 March 2018 23:16:08 EEST Sakari Ailus wrote:
> > On Wed, Mar 28, 2018 at 02:59:22PM -0300, Mauro Carvalho Chehab wrote:
> > > At put_v4l2_window32(), it tries to access kp->clips. However,
> > > kp points to an userspace pointer. So, it should be obtained
> > > 
> > > via get_user(), otherwise it can OOPS:
> > >  vivid-000: ==  END STATUS  ==
> > >  BUG: unable to handle kernel paging request at fffb18e0
> > >  IP: [] __put_v4l2_format32+0x169/0x220 [videodev]
> > >  PGD 3f5776067 PUD 3f576f067 PMD 3f5769067 PTE 80042548f067
> > >  Oops: 0001 [#1] SMP
> > >  Modules linked in: vivid videobuf2_vmalloc videobuf2_memops
> > >  v4l2_dv_timings videobuf2_core v4l2_common videodev media xt_CHECKSUM
> > >  iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat
> > >  nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack
> > >  nf_conntrack tun bridge stp llc ebtable_filter ebtables ip6table_filter
> > >  ip6_tables bluetooth rfkill binfmt_misc snd_hda_codec_hdmi i915
> > >  snd_hda_intel snd_hda_controller snd_hda_codec intel_rapl
> > >  x86_pkg_temp_thermal snd_hwdep intel_powerclamp snd_pcm coretemp
> > >  snd_seq_midi kvm_intel kvm snd_seq_midi_event snd_rawmidi i2c_algo_bit
> > >  drm_kms_helper snd_seq drm crct10dif_pclmul e1000e snd_seq_device
> > >  crc32_pclmul snd_timer ghash_clmulni_intel snd mei_me mei ptp pps_core
> > >  soundcore lpc_ich video crc32c_intel [last unloaded: media] CPU: 2 PID:
> > >  28332 Comm: v4l2-compliance Not tainted 3.18.102+ #107 Hardware name:   
> > >/NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> > >  05/11/2017 task: 8804293f8000 ti: 8803f564 task.ti:
> > >  8803f564 RIP: 0010:[]  []
> > >  __put_v4l2_format32+0x169/0x220 [videodev] RSP: 0018:8803f5643e28 
> > >  EFLAGS: 00010246
> > >  RAX:  RBX:  RCX: fffb1ab4
> > >  RDX: fffb1a68 RSI: fffb18d8 RDI: fffb1aa8
> > >  RBP: 8803f5643e48 R08: 0001 R09: 8803f54b0378
> > >  R10:  R11: 0168 R12: fffb18c0
> > >  R13: fffb1a94 R14: fffb18c8 R15: 
> > >  FS:  () GS:880456d0(0063)
> > >  knlGS:f7100980 CS:  0010 DS: 002b ES: 002b CR0: 80050033
> > >  CR2: fffb18e0 CR3: 0003f552b000 CR4: 003407e0
> > >  
> > >  Stack:
> > >   fffb1a94 c0cc5640 0056 8804274f3600
> > >   8803f5643ed0 c0547e16 0003 8803f5643eb0
> > >   81301460 88009db44b01 880441942520 8800c0d05640
> > >  
> > >  Call Trace:
> > >   [] v4l2_compat_ioctl32+0x12d6/0x1b1d [videodev]
> > >   [] ? file_has_perm+0x70/0xc0
> > >   [] compat_SyS_ioctl+0xec/0x1200
> > >   [] sysenter_dispatch+0x7/0x21
> > >  
> > >  Code: 00 00 48 8b 80 48 c0 ff ff 48 83 e8 38 49 39 c6 0f 87 2b ff ff ff
> > >  49 8d 45 1c e8 a3 ce e3 c0 85 c0 0f 85 1a ff ff ff 41 8d 40 ff <4d> 8b
> > >  64 24 20 41 89 d5 48 8d 44 40 03 4d 8d 34 c4 eb 15 0f 1f RIP 
> > >  [] __put_v4l2_format32+0x169/0x220 [videodev] RSP
> > >  
> > >  CR2: fffb18e0
> > > 
> > > Tested with vivid driver on Kernel v3.18.102.
> > > 
> > > Same bug happens upstream too:
> > >  BUG: KASAN: user-memory-access in __put_v4l2_format32+0x98/0x4d0
> > >  [videodev]
> > >  Read of size 8 at addr ffe48400 by task v4l2-compliance/8713
> > >  
> > >  CPU: 0 PID: 8713 Comm: v4l2-compliance Not tainted 4.16.0-rc4+ #108
> > >  Hardware name:  /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> > >  05/11/2017>  
> > >  Call Trace:
> > >   dump_stack+0x5c/0x7c
> > >   kasan_report+0x164/0x380
> > >   ? __put_v4l2_format32+0x98/0x4d0 [videodev]
> > >   __put_v4l2_format32+0x98/0x4d0 [videodev]
> > >   v4l2_compat_ioctl32+0x1aec/0x27a0 [videodev]
> > >   ? __fsnotify_inode_delete+0x20/0x20
> > >   ? __put_v4l2_format32+0x4d0/0x4d0 [videodev]
> > >   compat_SyS_ioctl+0x646/0x14d0
> > >   ? do_ioctl+0x30/0x30
> > >   do_fast_syscall_32+0x191/0x3f4
> > >   entry_SYSENTER_compat+0x6b/0x7a
> > >  
> > >  ==
> > >  Disabling lock debugging due to kernel taint
> > >  BUG: unable to handle kernel paging request at ffe48400
> > >  IP: __put_v4l2_format32+0x98/0x4d0 [videodev]
> > >  PGD 3a22fb067 P4D 3a22fb067 PUD 39b6f0067 PMD 39b6f1067 PTE
> > >  8003256af067 Oops: 0001 [#1] SMP KASAN
> > >  Modules linked in: vivid videobuf2_vmalloc videobuf2_dma_contig
> > >  videobuf2_memops v4l2_tpg v4l2_dv_timings videobuf2_v4l2
> > >  videobuf2_common v4l2_common videodev xt_CHECKSUM iptable_mangle
> > >  ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat
> > >  nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack libcrc32c tun
> > >  bridge stp llc 

Re: [PATCH 07/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function

[Laurent Pinchart]

Hi Kieran,

On Wednesday, 28 March 2018 17:43:13 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > The DRM pipeline setup code used at atomic commit time is similar to the
> > setup code used when enabling the pipeline. Move it to a separate
> > function in order to share it.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Assuming no hidden secret code addition in this code move that I haven't
> seen..
> 
> Only a minor nit below asking if the function should be pluralised (_inputs,
> rather than _input)

I'll fix that in v2, thanks.

> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_drm.c | 347
> >  + 1 file changed, 180 insertions(+), 167
> >  deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index 9a043a915c0b..7bf697ba7969
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct
> > vsp1_pipeline *pipe,> 
> >   * Pipeline Configuration
> >   */
> > 
> > +/* Setup one RPF and the connected BRU sink pad. */
> > +static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
> > + struct vsp1_pipeline *pipe,
> > + struct vsp1_rwpf *rpf,
> > + unsigned int bru_input)
> > +{
> > +   struct v4l2_subdev_selection sel;
> > +   struct v4l2_subdev_format format;
> > +   const struct v4l2_rect *crop;
> > +   int ret;
> > +
> > +   /*
> > +* Configure the format on the RPF sink pad and propagate it up to the
> > +* BRU sink pad.
> > +*/
> > +   crop = >drm->inputs[rpf->entity.index].crop;
> > +
> > +   memset(, 0, sizeof(format));
> > +   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +   format.pad = RWPF_PAD_SINK;
> > +   format.format.width = crop->width + crop->left;
> > +   format.format.height = crop->height + crop->top;
> > +   format.format.code = rpf->fmtinfo->mbus;
> > +   format.format.field = V4L2_FIELD_NONE;
> > +
> > +   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   dev_dbg(vsp1->dev,
> > +   "%s: set format %ux%u (%x) on RPF%u sink\n",
> > +   __func__, format.format.width, format.format.height,
> > +   format.format.code, rpf->entity.index);
> > +
> > +   memset(, 0, sizeof(sel));
> > +   sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +   sel.pad = RWPF_PAD_SINK;
> > +   sel.target = V4L2_SEL_TGT_CROP;
> > +   sel.r = *crop;
> > +
> > +   ret = v4l2_subdev_call(>entity.subdev, pad, set_selection, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   dev_dbg(vsp1->dev,
> > +   "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n",
> > +   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
> > +   rpf->entity.index);
> > +
> > +   /*
> > +* RPF source, hardcode the format to ARGB to turn on format
> > +* conversion if needed.
> > +*/
> > +   format.pad = RWPF_PAD_SOURCE;
> > +
> > +   ret = v4l2_subdev_call(>entity.subdev, pad, get_fmt, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   dev_dbg(vsp1->dev,
> > +   "%s: got format %ux%u (%x) on RPF%u source\n",
> > +   __func__, format.format.width, format.format.height,
> > +   format.format.code, rpf->entity.index);
> > +
> > +   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
> > +
> > +   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   /* BRU sink, propagate the format from the RPF source. */
> > +   format.pad = bru_input;
> > +
> > +   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
> > +   __func__, format.format.width, format.format.height,
> > +   format.format.code, BRU_NAME(pipe->bru), format.pad);
> > +
> > +   sel.pad = bru_input;
> > +   sel.target = V4L2_SEL_TGT_COMPOSE;
> > +   sel.r = vsp1->drm->inputs[rpf->entity.index].compose;
> > +
> > +   ret = v4l2_subdev_call(>bru->subdev, pad, set_selection, NULL,
> > +  );
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",
> > +   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
> > +   BRU_NAME(pipe->bru), sel.pad);
> > +
> > +   return 0;
> > +}
> > +
> > +static unsigned int rpf_zpos(struct vsp1_device 

Re: [PATCH for v3.18 00/18] Backport CVE-2017-13166 fixes to Kernel 3.18

[Greg KH]

On Thu, Mar 29, 2018 at 03:39:54PM +0900, Inki Dae wrote:
> 2018년 03월 29일 13:25에 Greg KH 이(가) 쓴 글:
> > On Thu, Mar 29, 2018 at 08:22:08AM +0900, Inki Dae wrote:
> >> Really thanks for doing this. :) There would be many users who use
> >> Linux-3.18 for their products yet.
> > 
> > For new products?  They really should not be.  The kernel is officially
> 
> Really no. Old products would still be using Linux-3.18 kernel without
> kernel upgrade. For new product, most of SoC vendors will use
> Linux-4.x including us.
> Actually, we are preparing for kernel upgrade for some devices even
> some old devices (to Linux-4.14-LTS) and almost done.

That is great to hear.

> > What is keeping you on 3.18.y and not allowing you to move to a newer
> > kernel version?
> 
> We also want to move to latest kernel version. However, there is a case that 
> we cannot upgrade the kernel.
> In case that SoC vendor never share firmwares and relevant data
> sheets, we cannot upgrade the kernel. However, we have to resolve the
> security issues for users of this device.

It sounds like you need to be getting those security updates from those
SoC vendors, as they are the ones you are paying for support for that
kernel version that they are forcing you to stay on.

good luck!

greg k-h

Re: [PATCH 05/15] v4l: vsp1: Use vsp1_entity.pipe to check if entity belongs to a pipeline

[Laurent Pinchart]

Hi Kieran,

On Wednesday, 28 March 2018 17:10:10 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > The DRM pipeline handling code uses the entity's pipe list head to check
> > whether the entity is already included in a pipeline. This method is a
> > bit fragile in the sense that it uses list_empty() on a list_head that
> > is a list member. Replace it by a simpler check for the entity pipe
> > pointer.
> 
> Yes, excellent.
> 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_drm.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index a7ad85ab0b08..e210917fdc3f
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -119,9 +119,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> > pipe_index,> 
> >  * Remove the RPF from the pipe and the list of BRU
> >  * inputs.
> >  */
> > 
> > -   WARN_ON(list_empty(>entity.list_pipe));
> > +   WARN_ON(!rpf->entity.pipe);
> 
> Does this WARN_ON() have much value any more ?
> 
> I think it could probably be removed... unless there is a race between
> potential calls through vsp1_du_atomic_flush() and vsp1_du_setup_lif() -
> but I would be very surprised if that wasn't protected at the DRM levels.

It should indeed be protected at the DRM level. The purpose of the WARN_ON() 
is twofold, it catches both bugs in the VSP1 driver (but I don't expect any 
bug here, so from that point of view the WARN_ON isn't needed), but also 
misbehaviours in the callers. There hasn't been any so far though, so maybe we 
could indeed remove the WARN_ON(). It just makes me feel a bit safer but 
probably not in any rational way :-)

>  (Removing it if chosen doesn't need to be in this patch though)
> 
> > rpf->entity.pipe = NULL;
> > 
> > -   list_del_init(>entity.list_pipe);
> > +   list_del(>entity.list_pipe);
> > 
> > pipe->inputs[i] = NULL;
> > 
> > bru->inputs[rpf->bru_input].rpf = NULL;
> > 
> > @@ -537,7 +537,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned
> > int pipe_index)> 
> > continue;
> > 
> > }
> > 
> > -   if (list_empty(>entity.list_pipe)) {
> > +   if (!rpf->entity.pipe) {
> > 
> > rpf->entity.pipe = pipe;
> > list_add_tail(>entity.list_pipe, >entities);
> > 
> > }
> > 
> > @@ -566,7 +566,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned
> > int pipe_index)> 
> >VI6_DPR_NODE_UNUSED);
> > 
> > entity->pipe = NULL;
> > 
> > -   list_del_init(>list_pipe);
> > +   list_del(>list_pipe);
> > 
> > continue;
> > 
> > }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Daniel Vetter]

On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote:
> Add a peer2peer flag noting that the importer can deal with device
> resources which are not backed by pages.
> 
> Signed-off-by: Christian König 

Um strictly speaking they all should, but ttm never bothered to use the
real interfaces but just hacked around the provided sg list, grabbing the
underlying struct pages, then rebuilding the sg list again.

The entire point of using sg lists was exactly to allow this use case of
peer2peer dma (or well in general have special exporters which managed
memory/IO ranges not backed by struct page). So essentially you're having
a "I'm totally not broken flag" here.

I think a better approach would be if we add a requires_struct_page or so,
and annotate the current importers accordingly. Or we just fix them up (it
is all in shared ttm code after all, I think everyone else got this
right).
-Daniel

> ---
>  drivers/dma-buf/dma-buf.c | 1 +
>  include/linux/dma-buf.h   | 4 
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ffaa2f9a9c2c..f420225f93c6 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -565,6 +565,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct 
> dma_buf_attach_info *info
>  
>   attach->dev = info->dev;
>   attach->dmabuf = dmabuf;
> + attach->peer2peer = info->peer2peer;
>   attach->priv = info->priv;
>   attach->invalidate = info->invalidate;
>  
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 15dd8598bff1..1ef50bd9bc5b 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -313,6 +313,7 @@ struct dma_buf {
>   * @dmabuf: buffer for this attachment.
>   * @dev: device attached to the buffer.
>   * @node: list of dma_buf_attachment.
> + * @peer2peer: true if the importer can handle peer resources without pages.
>   * @priv: exporter specific attachment data.
>   *
>   * This structure holds the attachment information between the dma_buf buffer
> @@ -328,6 +329,7 @@ struct dma_buf_attachment {
>   struct dma_buf *dmabuf;
>   struct device *dev;
>   struct list_head node;
> + bool peer2peer;
>   void *priv;
>  
>   /**
> @@ -392,6 +394,7 @@ struct dma_buf_export_info {
>   * @dmabuf:  the exported dma_buf
>   * @dev: the device which wants to import the attachment
>   * @priv:private data of importer to this attachment
> + * @peer2peer:   true if the importer can handle peer resources without 
> pages
>   * @invalidate:  callback to use for invalidating mappings
>   *
>   * This structure holds the information required to attach to a buffer. Used
> @@ -401,6 +404,7 @@ struct dma_buf_attach_info {
>   struct dma_buf *dmabuf;
>   struct device *dev;
>   void *priv;
> + bool peer2peer;
>   void (*invalidate)(struct dma_buf_attachment *attach);
>  };
>  
> -- 
> 2.14.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 02/15] v4l: vsp1: Remove outdated comment

[Laurent Pinchart]

Hi Kieran,

On Wednesday, 28 March 2018 22:04:49 EEST Kieran Bingham wrote:
> On 28/03/18 13:27, Kieran Bingham wrote:
> > On 26/02/18 21:45, Laurent Pinchart wrote:
> >> The entities in the pipeline are all started when the LIF is setup.
> >> Remove the outdated comment that state otherwise.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> 
> > 
> > I'll start with the easy ones :-)
> 
> In fact, couldn't this patch be squashed into [PATCH 01/15] in this series ?

I suppose it could, I'll do so.

> > Reviewed-by: Kieran Bingham 
> > 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1_drm.c | 6 +-
> >>  1 file changed, 1 insertion(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> >> b/drivers/media/platform/vsp1/vsp1_drm.c index
> >> e31fb371eaf9..a1f2ba044092 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> >> @@ -221,11 +221,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned
> >> int pipe_index,>> 
> >>return -EPIPE;
> >>
> >>}
> >> 
> >> -  /*
> >> -   * Enable the VSP1. We don't start the entities themselves right at
> >> this
> >> -   * point as there's no plane configured yet, so we can't start
> >> -   * processing buffers.
> >> -   */
> >> +  /* Enable the VSP1. */
> >> 
> >>ret = vsp1_device_get(vsp1);
> >>if (ret < 0)
> >>
> >>return ret;


-- 
Regards,

Laurent Pinchart

Re: [PATCH for v3.18 00/18] Backport CVE-2017-13166 fixes to Kernel 3.18

[Inki Dae]

2018년 03월 29일 13:25에 Greg KH 이(가) 쓴 글:
> On Thu, Mar 29, 2018 at 08:22:08AM +0900, Inki Dae wrote:
>> Really thanks for doing this. :) There would be many users who use
>> Linux-3.18 for their products yet.
> 
> For new products?  They really should not be.  The kernel is officially

Really no. Old products would still be using Linux-3.18 kernel without kernel 
upgrade. For new product, most of SoC vendors will use Linux-4.x including us.
Actually, we are preparing for kernel upgrade for some devices even some old 
devices (to Linux-4.14-LTS) and almost done.

> end-of-life, but I'm keeping it alive for a short while longer just
> because too many people seem to still be using it.  However, they are
> not actually updating the kernel in their devices, so I don't think I
> will be doing many more new 3.18.y releases.
> 
> It's a problem when people ask for support, and then don't use the
> releases given to them :(
> 
> What is keeping you on 3.18.y and not allowing you to move to a newer
> kernel version?

We also want to move to latest kernel version. However, there is a case that we 
cannot upgrade the kernel.
In case that SoC vendor never share firmwares and relevant data sheets, we 
cannot upgrade the kernel. However, we have to resolve the security issues for 
users of this device.

Thanks,
Inki Dae

> 
> thanks,
> 
> greg k-h
> 
> 
> 

Re: [PATCH] media: v4l2-compat-ioctl32: don't oops on overlay

[Laurent Pinchart]

Hello,

On Wednesday, 28 March 2018 23:16:08 EEST Sakari Ailus wrote:
> On Wed, Mar 28, 2018 at 02:59:22PM -0300, Mauro Carvalho Chehab wrote:
> > At put_v4l2_window32(), it tries to access kp->clips. However,
> > kp points to an userspace pointer. So, it should be obtained
> > 
> > via get_user(), otherwise it can OOPS:
> >  vivid-000: ==  END STATUS  ==
> >  BUG: unable to handle kernel paging request at fffb18e0
> >  IP: [] __put_v4l2_format32+0x169/0x220 [videodev]
> >  PGD 3f5776067 PUD 3f576f067 PMD 3f5769067 PTE 80042548f067
> >  Oops: 0001 [#1] SMP
> >  Modules linked in: vivid videobuf2_vmalloc videobuf2_memops
> >  v4l2_dv_timings videobuf2_core v4l2_common videodev media xt_CHECKSUM
> >  iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat
> >  nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack
> >  nf_conntrack tun bridge stp llc ebtable_filter ebtables ip6table_filter
> >  ip6_tables bluetooth rfkill binfmt_misc snd_hda_codec_hdmi i915
> >  snd_hda_intel snd_hda_controller snd_hda_codec intel_rapl
> >  x86_pkg_temp_thermal snd_hwdep intel_powerclamp snd_pcm coretemp
> >  snd_seq_midi kvm_intel kvm snd_seq_midi_event snd_rawmidi i2c_algo_bit
> >  drm_kms_helper snd_seq drm crct10dif_pclmul e1000e snd_seq_device
> >  crc32_pclmul snd_timer ghash_clmulni_intel snd mei_me mei ptp pps_core
> >  soundcore lpc_ich video crc32c_intel [last unloaded: media] CPU: 2 PID:
> >  28332 Comm: v4l2-compliance Not tainted 3.18.102+ #107 Hardware name:   
> >/NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> >  05/11/2017 task: 8804293f8000 ti: 8803f564 task.ti:
> >  8803f564 RIP: 0010:[]  []
> >  __put_v4l2_format32+0x169/0x220 [videodev] RSP: 0018:8803f5643e28 
> >  EFLAGS: 00010246
> >  RAX:  RBX:  RCX: fffb1ab4
> >  RDX: fffb1a68 RSI: fffb18d8 RDI: fffb1aa8
> >  RBP: 8803f5643e48 R08: 0001 R09: 8803f54b0378
> >  R10:  R11: 0168 R12: fffb18c0
> >  R13: fffb1a94 R14: fffb18c8 R15: 
> >  FS:  () GS:880456d0(0063)
> >  knlGS:f7100980 CS:  0010 DS: 002b ES: 002b CR0: 80050033
> >  CR2: fffb18e0 CR3: 0003f552b000 CR4: 003407e0
> >  
> >  Stack:
> >   fffb1a94 c0cc5640 0056 8804274f3600
> >   8803f5643ed0 c0547e16 0003 8803f5643eb0
> >   81301460 88009db44b01 880441942520 8800c0d05640
> >  
> >  Call Trace:
> >   [] v4l2_compat_ioctl32+0x12d6/0x1b1d [videodev]
> >   [] ? file_has_perm+0x70/0xc0
> >   [] compat_SyS_ioctl+0xec/0x1200
> >   [] sysenter_dispatch+0x7/0x21
> >  
> >  Code: 00 00 48 8b 80 48 c0 ff ff 48 83 e8 38 49 39 c6 0f 87 2b ff ff ff
> >  49 8d 45 1c e8 a3 ce e3 c0 85 c0 0f 85 1a ff ff ff 41 8d 40 ff <4d> 8b
> >  64 24 20 41 89 d5 48 8d 44 40 03 4d 8d 34 c4 eb 15 0f 1f RIP 
> >  [] __put_v4l2_format32+0x169/0x220 [videodev] RSP
> >  
> >  CR2: fffb18e0
> > 
> > Tested with vivid driver on Kernel v3.18.102.
> > 
> > Same bug happens upstream too:
> >  BUG: KASAN: user-memory-access in __put_v4l2_format32+0x98/0x4d0
> >  [videodev]
> >  Read of size 8 at addr ffe48400 by task v4l2-compliance/8713
> >  
> >  CPU: 0 PID: 8713 Comm: v4l2-compliance Not tainted 4.16.0-rc4+ #108
> >  Hardware name:  /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949
> >  05/11/2017>  
> >  Call Trace:
> >   dump_stack+0x5c/0x7c
> >   kasan_report+0x164/0x380
> >   ? __put_v4l2_format32+0x98/0x4d0 [videodev]
> >   __put_v4l2_format32+0x98/0x4d0 [videodev]
> >   v4l2_compat_ioctl32+0x1aec/0x27a0 [videodev]
> >   ? __fsnotify_inode_delete+0x20/0x20
> >   ? __put_v4l2_format32+0x4d0/0x4d0 [videodev]
> >   compat_SyS_ioctl+0x646/0x14d0
> >   ? do_ioctl+0x30/0x30
> >   do_fast_syscall_32+0x191/0x3f4
> >   entry_SYSENTER_compat+0x6b/0x7a
> >  
> >  ==
> >  Disabling lock debugging due to kernel taint
> >  BUG: unable to handle kernel paging request at ffe48400
> >  IP: __put_v4l2_format32+0x98/0x4d0 [videodev]
> >  PGD 3a22fb067 P4D 3a22fb067 PUD 39b6f0067 PMD 39b6f1067 PTE
> >  8003256af067 Oops: 0001 [#1] SMP KASAN
> >  Modules linked in: vivid videobuf2_vmalloc videobuf2_dma_contig
> >  videobuf2_memops v4l2_tpg v4l2_dv_timings videobuf2_v4l2
> >  videobuf2_common v4l2_common videodev xt_CHECKSUM iptable_mangle
> >  ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat
> >  nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack libcrc32c tun
> >  bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables
> >  bluetooth rfkill ecdh_generic binfmt_misc snd_hda_codec_hdmi intel_rapl
> >  x86_pkg_temp_thermal intel_powerclamp i915 coretemp snd_hda_intel
> >  snd_hda_codec kvm_intel snd_hwdep snd_hda_core kvm snd_pcm