cron job: media_tree daily build: OK

[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:   Wed Apr  4 05:00:17 CEST 2018
media-tree git hash:6ccd228e0cfce2a4f44558422d25c60fcb1a6710
media_build git hash:   541653bb52fcf7c34b8b83a4c8cc66b09c68ac37
v4l-utils git hash: 65d4f9242cede9f6cdc0ee86dabae31cee552dee
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: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.101-i686: OK
linux-3.2.101-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.56-i686: OK
linux-3.16.56-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.102-i686: OK
linux-3.18.102-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.51-i686: OK
linux-4.1.51-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.109-i686: OK
linux-4.4.109-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.91-i686: OK
linux-4.9.91-x86_64: OK
linux-4.14.31-i686: OK
linux-4.14.31-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset

[Mani, Rajmohan]

Hi Mauro and all,

> > Subject: RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset
> >
> > Hi Mauro,
> >
> > > > > Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset
> > > > >
> > > > > Hi,
> > > > >
> > > > > Em Fri, 17 Nov 2017 02:58:56 + "Mani, Rajmohan"
> > > > >  escreveu:
> > > > >
> > > > > > Here is an update on the IPU3 documentation that we are
> > > > > > currently working
> > > > > on.
> > > > > >
> > > > > > Image processing in IPU3 relies on the following.
> > > > > >
> > > > > > 1) HW configuration to enable ISP and
> > > > > > 2) setting customer specific 3A Tuning / Algorithm Parameters
> > > > > > to achieve
> > > > > desired image quality.
> > > > > >
> > > > > > We intend to provide documentation on ImgU driver programming
> > > > > > interface
> > > > > to help users of this driver to configure and enable ISP HW to
> > > > > meet their needs.  This documentation will include details on
> > > > > complete
> > > > > V4L2 Kernel driver interface and IO-Control parameters, except
> > > > > for the ISP internal algorithm and its parameters (which is
> > > > > Intel proprietary
> > IP).
> > > > >
> > > > > Sakari asked me to take a look on this thread, specifically on
> > > > > this email. I took a look on the other e-mails from this thread
> > > > > that are discussing about this IP block.
> > > > >
> > > > > I understand that Intel wants to keep their internal 3A
> > > > > algorithm protected, just like other vendors protect their own
> > > > > algos. It was never a requirement to open whatever algorithm are
> > > > > used inside a hardware (or firmware). The only requirement is
> > > > > that firmwares should be licensed with redistribution
> > > > > permission, ideally merged at linux-firmware
> > > > git tree.
> > > > >
> > > > > Yet, what I don't understand is why Intel also wants to also
> > > > > protect the interface for such 3A hardware/firmware algorithm.
> > > > > The parameters that are passed from an userspace application to
> > > > > Intel ISP logic doesn't contain the algorithm itself. What's the
> > > > > issue of documenting the meaning of each parameter?
> > > > >
> > > >
> > > > Thanks for looking into this.
> > > >
> > > > To achieve improved image quality using IPU3, 3A (Auto White
> > > > balance, Auto Focus and Auto Exposure) Tuning parameters specific
> > > > to a given camera sensor module, are converted to Intel ISP
> > > > algorithm parameters in user space camera HAL using AIC (Automatic
> > > > ISP
> > Configuration) library.
> > > >
> > > > As a unique design of Intel ISP, it exposes very detailed
> > > > algorithm parameters (~ 1 parameters) to configure ISP's image
> > > > processing algorithm per each image fame in runtime. Typical
> > > > Camera SW developers (including those at
> > > > Intel) are not expected to fully understand and directly set these
> > > > parameters to configure the ISP algorithm blocks. Due to the
> > > > above, a user space AIC library (in binary form) is provided to
> > > > generate ISP Algorithm specific parameters, for a given set of 3A
> > > > tuning parameters. It significantly reduces the efforts of SW
> > > > development in ISP HW
> > > configuration.
> > > >
> > > > On the other hand, the ISP algorithm details could be deduced
> > > > readily through these detailed parameters by other ISP experts
> > > > outside
> > Intel.
> > > > This is the reason that we want to keep these parameter
> > > > definitions as Intel
> > > proprietary IP.
> > > >
> > > > We are fully aware of your concerns on how to enable open source
> > > > developers to use Intel ISP through up-streamed Kernel Driver.
> > > > Internally, we are working on the license for this AIC library
> > > > release now (as Hans said NDA license is not acceptable). We
> > > > believe this will be more efficient way to help open source developers.
> > > >
> > > > This AIC library release would be a binary-only release. This AIC
> > > > library does not use any kernel uAPIs directly. The user space
> > > > Camera HAL that uses kernel uAPIs is available at
> > > > https://chromium.googlesource.com/chromiumos/platform/arc-
> > > > camera/+/master
> > > >
> >
> > The AIC library (in binary form) is available here.
> > https://storage.googleapis.com/chromeos-localmirror/distfiles/intel-3a
> > -libs-
> > bin-2017.09.27.tbz2
> >
> > Licensing information can be found in ./LICENSE.intel_3a_library file
> > after unzipping the tar file.
> >
> > >
> > > Just pinging to know your thoughts on this.
> > >

To follow up further on this, we have posted the preliminary documentation on
bnr (bayer noise reduction) parameters as below.

https://www.mail-archive.com/linux-media@vger.kernel.org/msg128683.html

We would like hear your early feedback and apply the same towards the
documentation of remaining IPU3 parameters.

Thanks
Raj

[RFC PATCH]: intel-ipu3: Add uAPI documentation

[Yong Zhi]

This is a preliminary effort to add documentation for the
following BNR(bayer noise reduction) structs:

ipu3_uapi_bnr_static_config_wb_gains_config
ipu3_uapi_bnr_static_config_wb_gains_thr_config
ipu3_uapi_bnr_static_config_thr_coeffs_config
ipu3_uapi_bnr_static_config_thr_ctrl_shd_config
ipu3_uapi_bnr_static_config_opt_center_sqr_config
ipu3_uapi_bnr_static_config_bp_ctrl_config
ipu3_uapi_bnr_static_config_dn_detect_ctrl_config
ipu3_uapi_bnr_static_config_opt_center_sqr_config
ipu3_uapi_bnr_static_config_green_disparity

The feedback on this patch will be used towards the
documentation of reset of the uAPI in intel-ipu3.h.

Link to v6 IPU3 ImgU patchset:



Signed-off-by: Yong Zhi 
Signed-off-by: Chao C Li 
---
 Documentation/media/media_uapi.rst  |1 +
 Documentation/media/uapi/intel-ipu3.rst |8 +
 include/uapi/linux/intel-ipu3.h | 1520 +++
 3 files changed, 1529 insertions(+)
 create mode 100644 Documentation/media/uapi/intel-ipu3.rst
 create mode 100644 include/uapi/linux/intel-ipu3.h

diff --git a/Documentation/media/media_uapi.rst 
b/Documentation/media/media_uapi.rst
index 28eb35a1f965..edfa674b49c3 100644
--- a/Documentation/media/media_uapi.rst
+++ b/Documentation/media/media_uapi.rst
@@ -31,3 +31,4 @@ License".
 uapi/cec/cec-api
 uapi/gen-errors
 uapi/fdl-appendix
+uapi/intel-ipu3
diff --git a/Documentation/media/uapi/intel-ipu3.rst 
b/Documentation/media/uapi/intel-ipu3.rst
new file mode 100644
index ..d4d9b2806fe9
--- /dev/null
+++ b/Documentation/media/uapi/intel-ipu3.rst
@@ -0,0 +1,8 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _intel-ipu3:
+
+Intel IPU3 ImgU uAPI headers
+
+
+.. kernel-doc:: 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 ..34b071524beb
--- /dev/null
+++ b/include/uapi/linux/intel-ipu3.h
@@ -0,0 +1,1520 @@
+/* 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];
+} 

Re: [PATCH v13 17/33] rcar-vin: cache video standard

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:40 EEST Niklas Söderlund wrote:
> At stream on time the driver should not query the subdevice for which
> standard are used. Instead it should be cached when userspace sets the
> standard and used at stream on time.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c |  6 ++
>  drivers/media/platform/rcar-vin/rcar-dma.c  |  7 ++-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 --
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  2 ++
>  4 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> be49d8968f0a0cef..8c251687e81b345b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -96,6 +96,12 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> *vin, if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>   return ret;
> 
> + /* Read standard */
> + vin->std = V4L2_STD_UNKNOWN;
> + ret = v4l2_subdev_call(subdev, video, g_std, >std);
> + if (ret < 0 && ret != -ENOIOCTLCMD)
> + return ret;
> +
>   /* Add the controls */
>   ret = v4l2_ctrl_handler_init(>ctrl_handler, 16);
>   if (ret < 0)
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 9233924e5b52de5f..79f4074b931b5aeb 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -592,7 +592,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
>  static int rvin_setup(struct rvin_dev *vin)
>  {
>   u32 vnmc, dmr, dmr2, interrupts;
> - v4l2_std_id std;
>   bool progressive = false, output_is_yuv = false, input_is_yuv = false;
> 
>   switch (vin->format.field) {
> @@ -606,10 +605,8 @@ static int rvin_setup(struct rvin_dev *vin)
>   /* Default to TB */
>   vnmc = VNMC_IM_FULL;
>   /* Use BT if video standard can be read and is 60 Hz format */
> - if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, )) {
> - if (std & V4L2_STD_525_60)
> - vnmc = VNMC_IM_FULL | VNMC_FOC;
> - }
> + if (vin->std & V4L2_STD_525_60)
> + vnmc = VNMC_IM_FULL | VNMC_FOC;
>   break;
>   case V4L2_FIELD_INTERLACED_TB:
>   vnmc = VNMC_IM_FULL;
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> c4be0bcb8b16f941..43370c57d4b6239a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -477,6 +477,8 @@ static int rvin_s_std(struct file *file, void *priv,
> v4l2_std_id a) if (ret < 0)
>   return ret;
> 
> + vin->std = a;
> +
>   /* Changing the standard will change the width/height */
>   return rvin_reset_format(vin);
>  }
> @@ -484,9 +486,13 @@ static int rvin_s_std(struct file *file, void *priv,
> v4l2_std_id a) static int rvin_g_std(struct file *file, void *priv,
> v4l2_std_id *a) {
>   struct rvin_dev *vin = video_drvdata(file);
> - struct v4l2_subdev *sd = vin_to_source(vin);
> 
> - return v4l2_subdev_call(sd, video, g_std, a);
> + if (v4l2_subdev_has_op(vin_to_source(vin), pad, dv_timings_cap))
> + return -ENOIOCTLCMD;
> +
> + *a = vin->std;
> +
> + return 0;
>  }
> 
>  static int rvin_subscribe_event(struct v4l2_fh *fh,
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> e940366d7e8d0e76..06cec4f8e5ffaf2b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -118,6 +118,7 @@ struct rvin_info {
>   * @crop:active cropping
>   * @compose: active composing
>   * @source:  active size of the video source
> + * @std: active video standard of the video source
>   */
>  struct rvin_dev {
>   struct device *dev;
> @@ -146,6 +147,7 @@ struct rvin_dev {
>   struct v4l2_rect crop;
>   struct v4l2_rect compose;
>   struct v4l2_rect source;
> + v4l2_std_id std;
>  };
> 
>  #define vin_to_source(vin)   ((vin)->digital->subdev)


-- 
Regards,

Laurent Pinchart

Re: [PATCH v13 16/33] rcar-vin: simplify how formats are set and reset

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:39 EEST Niklas Söderlund wrote:
> With the recent cleanup of the format code to prepare for Gen3 it's
> possible to simplify the Gen2 format code path as well. Clean up the
> process by defining two functions to handle the set format and reset of
> format when the standard is changed.
> 
> While at it replace the driver local struct rvin_source_fmt with a
> struct v4l2_rect as all it's used for is keep track of the source
> dimensions.

I wonder whether we should introduce v4l2_size (or ) when all we need is width and height, as v4l2_rect stores the top and 
left offsets too. This doesn't have to be fixed here though.

> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 
> 
> ---
> 
> * Changes since v11
> - This patch where 'rcar-vin: read subdevice format for crop only when
> needed'
> - Keep caching the source dimensions and drop all changes to
> rvin_g_selection() and rvin_s_selection().
> - Inline rvin_get_vin_format_from_source() into rvin_reset_format()
> which now is the only user left.
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 116 +++--
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  14 +---
>  2 files changed, 52 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> c39891386576afb8..c4be0bcb8b16f941 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -138,67 +138,60 @@ static int rvin_format_align(struct rvin_dev *vin,
> struct v4l2_pix_format *pix) * V4L2
>   */
> 
> -static void rvin_reset_crop_compose(struct rvin_dev *vin)
> -{
> - vin->crop.top = vin->crop.left = 0;
> - vin->crop.width = vin->source.width;
> - vin->crop.height = vin->source.height;
> -
> - vin->compose.top = vin->compose.left = 0;
> - vin->compose.width = vin->format.width;
> - vin->compose.height = vin->format.height;
> -}
> -
>  static int rvin_reset_format(struct rvin_dev *vin)
>  {
>   struct v4l2_subdev_format fmt = {
>   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = vin->digital->source_pad,
>   };
> - struct v4l2_mbus_framefmt *mf = 
>   int ret;
> 
> - fmt.pad = vin->digital->source_pad;
> -
>   ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
>   if (ret)
>   return ret;
> 
> - vin->format.width   = mf->width;
> - vin->format.height  = mf->height;
> - vin->format.colorspace  = mf->colorspace;
> - vin->format.field   = mf->field;
> + v4l2_fill_pix_format(>format, );
> 
> - rvin_reset_crop_compose(vin);
> + ret = rvin_format_align(vin, >format);
> + if (ret)
> + return ret;

rvin_format_align() always returns 0 so you can remove the error check. You 
can actually turn the function into a void function.

> - vin->format.bytesperline = rvin_format_bytesperline(>format);
> - vin->format.sizeimage = rvin_format_sizeimage(>format);
> + vin->source.top = vin->crop.top = 0;
> + vin->source.left = vin->crop.left = 0;
> + vin->source.width = vin->crop.width = vin->format.width;
> + vin->source.height = vin->crop.height = vin->format.height;

I find multiple assignations on the same line hard to read. How about

vin->source.top = 0;
vin->source.left = 0;
vin->source.width = vin->format.width;
vin->source.height = vin->format.height;

vin->crop = vin->source;
vin->compose = vin->source;

> + vin->compose.top = vin->compose.left = 0;
> + vin->compose.width = vin->format.width;
> + vin->compose.height = vin->format.height;
> 
>   return 0;
>  }

I like the new rvin_reset_format(), it's much simpler.

> -static int __rvin_try_format_source(struct rvin_dev *vin,
> - u32 which,
> - struct v4l2_pix_format *pix,
> - struct rvin_source_fmt *source)
> +static int rvin_try_format(struct rvin_dev *vin, u32 which,
> +struct v4l2_pix_format *pix,
> +struct v4l2_rect *crop, struct v4l2_rect *compose)
>  {
> - struct v4l2_subdev *sd;
> + struct v4l2_subdev *sd = vin_to_source(vin);
>   struct v4l2_subdev_pad_config *pad_cfg;
>   struct v4l2_subdev_format format = {
>   .which = which,
> + .pad = vin->digital->source_pad,
>   };
>   enum v4l2_field field;
>   u32 width, height;
>   int ret;
> 
> - sd = vin_to_source(vin);
> -
> - v4l2_fill_mbus_format(, pix, vin->digital->code);
> -
>   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
>   if (pad_cfg == NULL)
>   return -ENOMEM;
> 
> - format.pad = vin->digital->source_pad;
> + if 

Re: [PATCH v13 12/33] rcar-vin: fix handling of single field frames (top, bottom and alternate fields)

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:35 EEST Niklas Söderlund wrote:
> There was never proper support in the VIN driver to deliver ALTERNATING
> field format to user-space, remove this field option. The problem is
> that ALTERNATING field order requires the sequence numbers of buffers
> returned to userspace to reflect if fields were dropped or not,
> something which is not possible with the VIN drivers capture logic.
> 
> The VIN driver can still capture from a video source which delivers
> frames in ALTERNATING field order, but needs to combine them using the
> VIN hardware into INTERLACED field order. Before this change if a source
> was delivering fields using ALTERNATE the driver would default to
> combining them using this hardware feature. Only if the user explicitly
> requested ALTERNATE field order would incorrect frames be delivered.
> 
> The height should not be cut in half for the format for TOP or BOTTOM
> fields settings. This was a mistake and it was made visible by the
> scaling refactoring. Correct behavior is that the user should request a
> frame size that fits the half height frame reflected in the field
> setting. If not the VIN will do its best to scale the top or bottom to
> the requested format and cropping and scaling do not work as expected.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 
> 
> ---
> 
> * Changes since v12
> - Spelling where -> were.
> - Add review tag from Hans.
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 15 +--
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 40 +++---
>  2 files changed, 10 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 4f48575f2008fe34..9233924e5b52de5f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -617,7 +617,6 @@ static int rvin_setup(struct rvin_dev *vin)
>   case V4L2_FIELD_INTERLACED_BT:
>   vnmc = VNMC_IM_FULL | VNMC_FOC;
>   break;
> - case V4L2_FIELD_ALTERNATE:
>   case V4L2_FIELD_NONE:
>   vnmc = VNMC_IM_ODD_EVEN;
>   progressive = true;
> @@ -745,18 +744,6 @@ static bool rvin_capture_active(struct rvin_dev *vin)
>   return rvin_read(vin, VNMS_REG) & VNMS_CA;
>  }
> 
> -static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32
> vnms)
> -{
> - if (vin->format.field == V4L2_FIELD_ALTERNATE) {
> - /* If FS is set it's a Even field */
> - if (vnms & VNMS_FS)
> - return V4L2_FIELD_BOTTOM;
> - return V4L2_FIELD_TOP;
> - }
> -
> - return vin->format.field;
> -}
> -
>  static void rvin_set_slot_addr(struct rvin_dev *vin, int slot, dma_addr_t
> addr) {
>   const struct rvin_video_format *fmt;
> @@ -892,7 +879,7 @@ static irqreturn_t rvin_irq(int irq, void *data)
> 
>   /* Capture frame */
>   if (vin->queue_buf[slot]) {
> - vin->queue_buf[slot]->field = rvin_get_active_field(vin, vnms);
> + vin->queue_buf[slot]->field = vin->format.field;
>   vin->queue_buf[slot]->sequence = vin->sequence;
>   vin->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
>   vb2_buffer_done(>queue_buf[slot]->vb2_buf,
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 16e895657c3f51c5..e956a385cc13f5f3 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -121,33 +121,6 @@ static int rvin_reset_format(struct rvin_dev *vin)
>   vin->format.colorspace  = mf->colorspace;
>   vin->format.field   = mf->field;
> 
> - /*
> -  * If the subdevice uses ALTERNATE field mode and G_STD is
> -  * implemented use the VIN HW to combine the two fields to
> -  * one INTERLACED frame. The ALTERNATE field mode can still
> -  * be requested in S_FMT and be respected, this is just the
> -  * default which is applied at probing or when S_STD is called.
> -  */
> - if (vin->format.field == V4L2_FIELD_ALTERNATE &&
> - v4l2_subdev_has_op(vin_to_source(vin), video, g_std))
> - vin->format.field = V4L2_FIELD_INTERLACED;
> -
> - switch (vin->format.field) {
> - case V4L2_FIELD_TOP:
> - case V4L2_FIELD_BOTTOM:
> - case V4L2_FIELD_ALTERNATE:
> - vin->format.height /= 2;
> - break;
> - case V4L2_FIELD_NONE:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> - case V4L2_FIELD_INTERLACED:
> - break;
> - default:
> - vin->format.field = RVIN_DEFAULT_FIELD;
> - break;
> - }
> -
>   rvin_reset_crop_compose(vin);
> 
>   vin->format.bytesperline = 

Re: [PATCH v13 15/33] rcar-vin: break out format alignment and checking

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:38 EEST Niklas Söderlund wrote:
> Part of the format alignment and checking can be shared with the Gen3
> format handling. Break that part out to a separate function.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 85 +++--
>  1 file changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 8dbd764883976ad1..c39891386576afb8 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -87,6 +87,53 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format
> *pix) return pix->bytesperline * pix->height;
>  }
> 
> +static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format
> *pix)
> +{
> + u32 walign;
> +
> + if (!rvin_format_from_pixel(pix->pixelformat) ||
> + (vin->info->model == RCAR_M1 &&
> +  pix->pixelformat == V4L2_PIX_FMT_XBGR32))
> + pix->pixelformat = RVIN_DEFAULT_FORMAT;
> +
> + switch (pix->field) {
> + case V4L2_FIELD_TOP:
> + case V4L2_FIELD_BOTTOM:
> + case V4L2_FIELD_NONE:
> + case V4L2_FIELD_INTERLACED_TB:
> + case V4L2_FIELD_INTERLACED_BT:
> + case V4L2_FIELD_INTERLACED:
> + break;
> + case V4L2_FIELD_ALTERNATE:
> + /*
> +  * Driver dose not (yet) support outputting ALTERNATE to a

s/dose/does/

Apart from that,

Reviewed-by: Laurent Pinchart 

> +  * userspace. It does support outputting INTERLACED so use
> +  * the VIN hardware to combine the two fields.
> +  */
> + pix->field = V4L2_FIELD_INTERLACED;
> + pix->height *= 2;
> + break;
> + default:
> + pix->field = RVIN_DEFAULT_FIELD;
> + break;
> + }
> +
> + /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> + walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> +
> + /* Limit to VIN capabilities */
> + v4l_bound_align_image(>width, 2, vin->info->max_width, walign,
> +   >height, 4, vin->info->max_height, 2, 0);
> +
> + pix->bytesperline = rvin_format_bytesperline(pix);
> + pix->sizeimage = rvin_format_sizeimage(pix);
> +
> + vin_dbg(vin, "Format %ux%u bpl: %u size: %u\n",
> + pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> +
> + return 0;
> +}
> +
>  /*
> ---
> -- * V4L2
>   */
> @@ -186,7 +233,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
>struct v4l2_pix_format *pix,
>struct rvin_source_fmt *source)
>  {
> - u32 walign;
>   int ret;
> 
>   if (!rvin_format_from_pixel(pix->pixelformat) ||
> @@ -199,42 +245,7 @@ static int __rvin_try_format(struct rvin_dev *vin,
>   if (ret)
>   return ret;
> 
> - switch (pix->field) {
> - case V4L2_FIELD_TOP:
> - case V4L2_FIELD_BOTTOM:
> - case V4L2_FIELD_NONE:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> - case V4L2_FIELD_INTERLACED:
> - break;
> - case V4L2_FIELD_ALTERNATE:
> - /*
> -  * Driver dose not (yet) support outputting ALTERNATE to a
> -  * userspace. It does support outputting INTERLACED so use
> -  * the VIN hardware to combine the two fields.
> -  */
> - pix->field = V4L2_FIELD_INTERLACED;
> - pix->height *= 2;
> - break;
> - default:
> - pix->field = RVIN_DEFAULT_FIELD;
> - break;
> - }
> -
> - /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> - walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> -
> - /* Limit to VIN capabilities */
> - v4l_bound_align_image(>width, 2, vin->info->max_width, walign,
> -   >height, 4, vin->info->max_height, 2, 0);
> -
> - pix->bytesperline = rvin_format_bytesperline(pix);
> - pix->sizeimage = rvin_format_sizeimage(pix);
> -
> - vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> - pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> -
> - return 0;
> + return rvin_format_align(vin, pix);
>  }
> 
>  static int rvin_querycap(struct file *file, void *priv,


-- 
Regards,

Laurent Pinchart

Re: [PATCH v13 22/33] rcar-vin: use different v4l2 operations in media controller mode

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:45 EEST Niklas Söderlund wrote:
> When the driver runs in media controller mode it should not directly
> control the subdevice instead userspace will be responsible for
> configuring the pipeline. To be able to run in this mode a different set
> of v4l2 operations needs to be used.
> 
> Add a new set of v4l2 operations to support operation without directly
> interacting with the source subdevice.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> 
> ---
> 
> * Changes since v11
> - Fixed error labels name in rvin_mc_open().
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c  |   2 +-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 161
> +++- 2 files changed, 159 insertions(+), 4
> deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 1809f5c0190eafb6..a93772c10baaa003 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -627,7 +627,7 @@ static int rvin_setup(struct rvin_dev *vin)
>   /* Default to TB */
>   vnmc = VNMC_IM_FULL;
>   /* Use BT if video standard can be read and is 60 Hz format */
> - if (vin->std & V4L2_STD_525_60)
> + if (!vin->info->use_mc && vin->std & V4L2_STD_525_60)
>   vnmc = VNMC_IM_FULL | VNMC_FOC;
>   break;
>   case V4L2_FIELD_INTERLACED_TB:
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> dd835be0f9cbcc05..2280535ca981993f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -18,12 +18,16 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  #include "rcar-vin.h"
> 
>  #define RVIN_DEFAULT_FORMAT  V4L2_PIX_FMT_YUYV
> +#define RVIN_DEFAULT_WIDTH   800
> +#define RVIN_DEFAULT_HEIGHT  600
>  #define RVIN_DEFAULT_FIELD   V4L2_FIELD_NONE
> +#define RVIN_DEFAULT_COLORSPACE  V4L2_COLORSPACE_SRGB
> 
>  /*
> ---
> -- * Format Conversions
> @@ -656,6 +660,74 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>   .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
>  };
> 
> +/*
> ---
> -- + * V4L2 Media Controller
> + */
> +
> +static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv,
> +struct v4l2_format *f)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> +
> + return rvin_format_align(vin, >fmt.pix);
> +}
> +
> +static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
> +  struct v4l2_format *f)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + int ret;
> +
> + if (vb2_is_busy(>queue))
> + return -EBUSY;
> +
> + ret = rvin_format_align(vin, >fmt.pix);
> + if (ret)
> + return ret;
> +
> + vin->format = f->fmt.pix;
> +
> + return 0;
> +}
> +
> +static int rvin_mc_enum_input(struct file *file, void *priv,
> +   struct v4l2_input *i)
> +{
> + if (i->index != 0)
> + return -EINVAL;
> +
> + i->type = V4L2_INPUT_TYPE_CAMERA;
> + strlcpy(i->name, "Camera", sizeof(i->name));
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
> + .vidioc_querycap= rvin_querycap,
> + .vidioc_try_fmt_vid_cap = rvin_mc_try_fmt_vid_cap,
> + .vidioc_g_fmt_vid_cap   = rvin_g_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap   = rvin_mc_s_fmt_vid_cap,
> + .vidioc_enum_fmt_vid_cap= rvin_enum_fmt_vid_cap,
> +
> + .vidioc_enum_input  = rvin_mc_enum_input,
> + .vidioc_g_input = rvin_g_input,
> + .vidioc_s_input = rvin_s_input,
> +
> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
> + .vidioc_querybuf= vb2_ioctl_querybuf,
> + .vidioc_qbuf= vb2_ioctl_qbuf,
> + .vidioc_dqbuf   = vb2_ioctl_dqbuf,
> + .vidioc_expbuf  = vb2_ioctl_expbuf,
> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> + .vidioc_streamon= vb2_ioctl_streamon,
> + .vidioc_streamoff   = vb2_ioctl_streamoff,
> +
> + .vidioc_log_status  = v4l2_ctrl_log_status,
> + .vidioc_subscribe_event = rvin_subscribe_event,
> + .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
> +};
> +
>  /*
> 

Re: [PATCH v13 23/33] rcar-vin: force default colorspace for media centric mode

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:46 EEST Niklas Söderlund wrote:
> The V4L2 specification clearly documents the colorspace fields as being
> set by drivers for capture devices. Using the values supplied by
> userspace thus wouldn't comply with the API. Until the API is updated to
> allow for userspace to set these Hans wants the fields to be set by the
> driver to fixed values.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 2280535ca981993f..ea0759a645e49490 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -664,12 +664,29 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>   * V4L2 Media Controller
>   */
> 
> +static int rvin_mc_try_format(struct rvin_dev *vin, struct v4l2_pix_format
> *pix) +{
> + /*
> +  * The V4L2 specification clearly documents the colorspace fields
> +  * as being set by drivers for capture devices. Using the values
> +  * supplied by userspace thus wouldn't comply with the API. Until
> +  * the API is updated force fixed vaules.
> +  */
> + pix->colorspace = RVIN_DEFAULT_COLORSPACE;
> + pix->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(pix->colorspace);
> + pix->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(pix->colorspace);
> + pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
> +   pix->ycbcr_enc);
> +
> + return rvin_format_align(vin, pix);
> +}
> +
>  static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv,
>  struct v4l2_format *f)
>  {
>   struct rvin_dev *vin = video_drvdata(file);
> 
> - return rvin_format_align(vin, >fmt.pix);
> + return rvin_mc_try_format(vin, >fmt.pix);
>  }
> 
>  static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
> @@ -681,7 +698,7 @@ static int rvin_mc_s_fmt_vid_cap(struct file *file, void
> *priv, if (vb2_is_busy(>queue))
>   return -EBUSY;
> 
> - ret = rvin_format_align(vin, >fmt.pix);
> + ret = rvin_mc_try_format(vin, >fmt.pix);
>   if (ret)
>   return ret;


-- 
Regards,

Laurent Pinchart

Re: [PATCH v13 27/33] rcar-vin: add chsel information to rvin_info

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:50 EEST Niklas Söderlund wrote:
> Each Gen3 SoC has a limited set of predefined routing possibilities for
> which CSI-2 device and channel can be routed to which VIN instance.
> Prepare to store this information in the struct rvin_info.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> ---
> 
> * Changes since v11
> - Fixed spelling.
> - Reorderd filed order in struct rvin_group_route.
> - Renamed chan to channel in struct rvin_group_route.
> ---
>  drivers/media/platform/rcar-vin/rcar-vin.h | 42 +++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> cf5c467d45e10847..93eb40856b866117 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -43,6 +43,14 @@ enum model_id {
>   RCAR_GEN3,
>  };
> 
> +enum rvin_csi_id {
> + RVIN_CSI20,
> + RVIN_CSI21,
> + RVIN_CSI40,
> + RVIN_CSI41,
> + RVIN_CSI_MAX,
> +};
> +
>  /**
>   * STOPPED  - No operation in progress
>   * RUNNING  - Operation in progress have buffers
> @@ -79,12 +87,45 @@ struct rvin_graph_entity {
>   unsigned int sink_pad;
>  };
> 
> +/**
> + * struct rvin_group_route - describes a route from a channel of a
> + *   CSI-2 receiver to a VIN
> + *
> + * @csi: CSI-2 receiver ID.
> + * @channel: Output channel of the CSI-2 receiver.
> + * @vin: VIN ID.
> + * @mask:Bitmask of the different CHSEL register values that
> + *   allow for a route from @csi + @chan to @vin.
> + *
> + * .. note::
> + *   Each R-Car CSI-2 receiver has four output channels facing the VIN
> + *   devices, each channel can carry one CSI-2 Virtual Channel (VC).
> + *   There is no correlation between channel number and CSI-2 VC. It's
> + *   up to the CSI-2 receiver driver to configure which VC is output
> + *   on which channel, the VIN devices only care about output channels.
> + *
> + *   There are in some cases multiple CHSEL register settings which would
> + *   allow for the same route from @csi + @channel to @vin. For example
> + *   on R-Car H3 both the CHSEL values 0 and 3 allow for a route from
> + *   CSI40/VC0 to VIN0. All possible CHSEL values for a route need to be
> + *   recorded as a bitmask in @mask, in this example bit 0 and 3 should
> + *   be set.
> + */
> +struct rvin_group_route {
> + enum rvin_csi_id csi;
> + unsigned int channel;
> + unsigned int vin;
> + unsigned int mask;
> +};
> +
>  /**
>   * struct rvin_info - Information about the particular VIN implementation
>   * @model:   VIN model
>   * @use_mc:  use media controller instead of controlling subdevice
>   * @max_width:   max input width the VIN supports
>   * @max_height:  max input height the VIN supports
> + * @routes:  list of possible routes from the CSI-2 recivers to
> + *   all VINs. The list mush be NULL terminated.
>   */
>  struct rvin_info {
>   enum model_id model;
> @@ -92,6 +133,7 @@ struct rvin_info {
> 
>   unsigned int max_width;
>   unsigned int max_height;
> + const struct rvin_group_route *routes;
>  };
> 
>  /**


-- 
Regards,

Laurent Pinchart

Re: [PATCH v13 29/33] rcar-vin: add link notify for Gen3

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday, 27 March 2018 00:44:52 EEST Niklas Söderlund wrote:
> Add the ability to process media device link change requests. Link
> enabling is a bit complicated on Gen3, whether or not it's possible to
> enable a link depends on what other links already are enabled. On Gen3
> the 8 VINs are split into two subgroup's (VIN0-3 and VIN4-7) and from a
> routing perspective these two groups are independent of each other.
> Each subgroup's routing is controlled by the subgroup VIN master
> instance (VIN0 and VIN4).
> 
> There are a limited number of possible route setups available for each
> subgroup and the configuration of each setup is dictated by the
> hardware. On H3 for example there are 6 possible route setups for each
> subgroup to choose from.
> 
> This leads to the media device link notification code being rather large
> since it will find the best routing configuration to try and accommodate
> as many links as possible. When it's not possible to enable a new link
> due to hardware constrains the link_notifier callback will return
> -EMLINK.
> 
> Signed-off-by: Niklas Söderlund 
> Reviewed-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> 
> ---
> 
> * Changes since v11
> - Fixed spelling
> - Updated comment to clarify the intent that no link can be enabled if
> any video node is open.
> - Use container_of() instead of a loop to find struct vin_dev from the
> video device.
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 147
>  1 file changed, 147 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> 99f6301a778046df..0cc76d73115e9277 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -24,6 +24,7 @@
> 
>  #include 
>  #include 
> +#include 
> 
>  #include "rcar-vin.h"
> 
> @@ -44,6 +45,151 @@
>   */
>  #define rvin_group_id_to_master(vin) ((vin) < 4 ? 0 : 4)
> 
> +/*
> ---
> -- + * Media Controller link notification
> + */
> +
> +/* group lock should be held when calling this function. */
> +static int rvin_group_entity_to_csi_id(struct rvin_group *group,
> +struct media_entity *entity)
> +{
> + struct v4l2_subdev *sd;
> + unsigned int i;
> +
> + sd = media_entity_to_v4l2_subdev(entity);
> +
> + for (i = 0; i < RVIN_CSI_MAX; i++)
> + if (group->csi[i].subdev == sd)
> + return i;
> +
> + return -ENODEV;
> +}
> +
> +static unsigned int rvin_group_get_mask(struct rvin_dev *vin,
> + enum rvin_csi_id csi_id,
> + unsigned char channel)
> +{
> + const struct rvin_group_route *route;
> + unsigned int mask = 0;
> +
> + for (route = vin->info->routes; route->mask; route++) {
> + if (route->vin == vin->id &&
> + route->csi == csi_id &&
> + route->channel == channel) {
> + vin_dbg(vin,
> + "Adding route: vin: %d csi: %d channel: %d\n",
> + route->vin, route->csi, route->channel);
> + mask |= route->mask;
> + }
> + }
> +
> + return mask;
> +}
> +
> +/*
> + * Link setup for the links between a VIN and a CSI-2 receiver is a bit
> + * complex. The reason for this is that the register controlling routing
> + * is not present in each VIN instance. There are special VINs which
> + * control routing for themselves and other VINs. There are not many
> + * different possible links combinations that can be enabled at the same
> + * time, therefor all already enabled links which are controlled by a
> + * master VIN need to be taken into account when making the decision
> + * if a new link can be enabled or not.
> + *
> + * 1. Find out which VIN the link the user tries to enable is connected to.
> + * 2. Lookup which master VIN controls the links for this VIN.
> + * 3. Start with a bitmask with all bits set.
> + * 4. For each previously enabled link from the master VIN bitwise AND its
> + *route mask (see documentation for mask in struct rvin_group_route)
> + *with the bitmask.
> + * 5. Bitwise AND the mask for the link the user tries to enable to the
> bitmask. + * 6. If the bitmask is not empty at this point the new link can
> be enabled + *while keeping all previous links enabled. Update the
> CHSEL value of the + *master VIN and inform the user that the link
> could be enabled. + *
> + * Please note that no link can be enabled if any VIN in the group is
> + * currently open.
> + */
> +static int rvin_group_link_notify(struct media_link *link, u32 flags,
> +   unsigned 

[PATCH 2/5] media: docs: clarify relationship between crop and selection APIs

[Luca Ceresoli]

Having two somewhat similar and largely overlapping APIs is confusing,
especially since the older one appears in the docs before the newer
and most featureful counterpart.

Clarify all of this in several ways:
 - swap the two sections
 - give a name to the two APIs in the section names
 - add a note at the beginning of the CROP API section

Also remove a note that is incorrect (correct wording is in
vidioc-cropcap.rst).

Signed-off-by: Luca Ceresoli 
Based on info from: Hans Verkuil 
Cc: Hans Verkuil 
---
 Documentation/media/uapi/v4l/common.rst|  2 +-
 Documentation/media/uapi/v4l/crop.rst  | 21 -
 Documentation/media/uapi/v4l/selection-api-005.rst |  2 ++
 Documentation/media/uapi/v4l/selection-api.rst |  4 ++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/Documentation/media/uapi/v4l/common.rst 
b/Documentation/media/uapi/v4l/common.rst
index 13f2ed3fc5a6..5f93e71122ef 100644
--- a/Documentation/media/uapi/v4l/common.rst
+++ b/Documentation/media/uapi/v4l/common.rst
@@ -41,6 +41,6 @@ applicable to all devices.
 extended-controls
 format
 planar-apis
-crop
 selection-api
+crop
 streaming-par
diff --git a/Documentation/media/uapi/v4l/crop.rst 
b/Documentation/media/uapi/v4l/crop.rst
index 182565b9ace4..83fa16eb347e 100644
--- a/Documentation/media/uapi/v4l/crop.rst
+++ b/Documentation/media/uapi/v4l/crop.rst
@@ -2,9 +2,18 @@
 
 .. _crop:
 
-*
-Image Cropping, Insertion and Scaling
-*
+*
+Image Cropping, Insertion and Scaling -- the CROP API
+*
+
+.. note::
+
+   The CROP API is mostly superseded by the newer :ref:`SELECTION API
+   `. The new API should be preferred in most cases,
+   with the exception of pixel aspect ratio detection, which is
+   implemented by :ref:`VIDIOC_CROPCAP ` and has no
+   equivalent in the SELECTION API. See :ref:`selection-vs-crop` for a
+   comparison of the two APIs.
 
 Some video capture devices can sample a subsection of the picture and
 shrink or enlarge it to an image of arbitrary size. We call these
@@ -40,12 +49,6 @@ support scaling or the :ref:`VIDIOC_G_CROP ` 
and
 :ref:`VIDIOC_S_CROP ` ioctls. Their size (and position
 where applicable) will be fixed in this case.
 
-.. note::
-
-   All capture and output devices must support the
-   :ref:`VIDIOC_CROPCAP ` ioctl such that applications
-   can determine if scaling takes place.
-
 
 Cropping Structures
 ===
diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst 
b/Documentation/media/uapi/v4l/selection-api-005.rst
index 5b47a28ac6d7..2ad30a49184f 100644
--- a/Documentation/media/uapi/v4l/selection-api-005.rst
+++ b/Documentation/media/uapi/v4l/selection-api-005.rst
@@ -1,5 +1,7 @@
 .. -*- coding: utf-8; mode: rst -*-
 
+.. _selection-vs-crop:
+
 
 Comparison with old cropping API
 
diff --git a/Documentation/media/uapi/v4l/selection-api.rst 
b/Documentation/media/uapi/v4l/selection-api.rst
index 81ea52d785b9..e4e623824b30 100644
--- a/Documentation/media/uapi/v4l/selection-api.rst
+++ b/Documentation/media/uapi/v4l/selection-api.rst
@@ -2,8 +2,8 @@
 
 .. _selection-api:
 
-API for cropping, composing and scaling
-===
+Cropping, composing and scaling -- the SELECTION API
+
 
 
 .. toctree::
-- 
2.7.4

[PATCH 3/5] media: docs: selection: rename files to something meaningful

[Luca Ceresoli]

These files have an automatically-generated numbering. Replaname them
to something that suggests their meaning.

Reported-by: Hans Verkuil 
Cc: Hans Verkuil 
Signed-off-by: Luca Ceresoli 
---
 .../{selection-api-004.rst => selection-api-configuration.rst} |  0
 .../v4l/{selection-api-006.rst => selection-api-examples.rst}  |  0
 .../v4l/{selection-api-002.rst => selection-api-intro.rst} |  0
 .../v4l/{selection-api-003.rst => selection-api-targets.rst}   |  0
 .../{selection-api-005.rst => selection-api-vs-crop-api.rst}   |  0
 Documentation/media/uapi/v4l/selection-api.rst | 10 +-
 6 files changed, 5 insertions(+), 5 deletions(-)
 rename Documentation/media/uapi/v4l/{selection-api-004.rst => 
selection-api-configuration.rst} (100%)
 rename Documentation/media/uapi/v4l/{selection-api-006.rst => 
selection-api-examples.rst} (100%)
 rename Documentation/media/uapi/v4l/{selection-api-002.rst => 
selection-api-intro.rst} (100%)
 rename Documentation/media/uapi/v4l/{selection-api-003.rst => 
selection-api-targets.rst} (100%)
 rename Documentation/media/uapi/v4l/{selection-api-005.rst => 
selection-api-vs-crop-api.rst} (100%)

diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst 
b/Documentation/media/uapi/v4l/selection-api-configuration.rst
similarity index 100%
rename from Documentation/media/uapi/v4l/selection-api-004.rst
rename to Documentation/media/uapi/v4l/selection-api-configuration.rst
diff --git a/Documentation/media/uapi/v4l/selection-api-006.rst 
b/Documentation/media/uapi/v4l/selection-api-examples.rst
similarity index 100%
rename from Documentation/media/uapi/v4l/selection-api-006.rst
rename to Documentation/media/uapi/v4l/selection-api-examples.rst
diff --git a/Documentation/media/uapi/v4l/selection-api-002.rst 
b/Documentation/media/uapi/v4l/selection-api-intro.rst
similarity index 100%
rename from Documentation/media/uapi/v4l/selection-api-002.rst
rename to Documentation/media/uapi/v4l/selection-api-intro.rst
diff --git a/Documentation/media/uapi/v4l/selection-api-003.rst 
b/Documentation/media/uapi/v4l/selection-api-targets.rst
similarity index 100%
rename from Documentation/media/uapi/v4l/selection-api-003.rst
rename to Documentation/media/uapi/v4l/selection-api-targets.rst
diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst 
b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
similarity index 100%
rename from Documentation/media/uapi/v4l/selection-api-005.rst
rename to Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
diff --git a/Documentation/media/uapi/v4l/selection-api.rst 
b/Documentation/media/uapi/v4l/selection-api.rst
index e4e623824b30..390233f704a3 100644
--- a/Documentation/media/uapi/v4l/selection-api.rst
+++ b/Documentation/media/uapi/v4l/selection-api.rst
@@ -9,8 +9,8 @@ Cropping, composing and scaling -- the SELECTION API
 .. toctree::
 :maxdepth: 1
 
-selection-api-002
-selection-api-003
-selection-api-004
-selection-api-005
-selection-api-006
+selection-api-intro.rst
+selection-api-targets.rst
+selection-api-configuration.rst
+selection-api-vs-crop-api.rst
+selection-api-examples.rst
-- 
2.7.4

[PATCH 1/5] media: docs: selection: fix typos

[Luca Ceresoli]

Cc: Hans Verkuil 
Signed-off-by: Luca Ceresoli 
---
 Documentation/media/uapi/v4l/selection-api-004.rst | 2 +-
 Documentation/media/uapi/v4l/selection.svg | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst 
b/Documentation/media/uapi/v4l/selection-api-004.rst
index d782cd5b2117..0a4ddc2d71db 100644
--- a/Documentation/media/uapi/v4l/selection-api-004.rst
+++ b/Documentation/media/uapi/v4l/selection-api-004.rst
@@ -41,7 +41,7 @@ The driver may further adjust the requested size and/or 
position
 according to hardware limitations.
 
 Each capture device has a default source rectangle, given by the
-``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall over what the
+``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall cover what the
 driver writer considers the complete picture. Drivers shall set the
 active crop rectangle to the default when the driver is first loaded,
 but not later.
diff --git a/Documentation/media/uapi/v4l/selection.svg 
b/Documentation/media/uapi/v4l/selection.svg
index a93e3b59786d..911062bd2844 100644
--- a/Documentation/media/uapi/v4l/selection.svg
+++ b/Documentation/media/uapi/v4l/selection.svg
@@ -1128,11 +1128,11 @@

   
   
-   COMPOSE_BONDS
+   COMPOSE_BOUNDS
   
   

-CROP_BONDS
+CROP_BOUNDS


 overscan area
-- 
2.7.4

[PATCH 4/5] media: docs: selection: improve formatting

[Luca Ceresoli]

Split section "Comparison with old cropping API" in paragraphs for
easier reading and improve visible links text.

Cc: Hans Verkuil 
Signed-off-by: Luca Ceresoli 
---
 .../media/uapi/v4l/selection-api-vs-crop-api.rst   | 55 --
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst 
b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
index 2ad30a49184f..ba1064a244a0 100644
--- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
+++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
@@ -6,31 +6,34 @@
 Comparison with old cropping API
 
 
-The selection API was introduced to cope with deficiencies of previous
-:ref:`API `, that was designed to control simple capture
-devices. Later the cropping API was adopted by video output drivers. The
-ioctls are used to select a part of the display were the video signal is
-inserted. It should be considered as an API abuse because the described
-operation is actually the composing. The selection API makes a clear
-distinction between composing and cropping operations by setting the
-appropriate targets. The V4L2 API lacks any support for composing to and
-cropping from an image inside a memory buffer. The application could
-configure a capture device to fill only a part of an image by abusing
-V4L2 API. Cropping a smaller image from a larger one is achieved by
-setting the field ``bytesperline`` at struct
-:c:type:`v4l2_pix_format`.
-Introducing an image offsets could be done by modifying field ``m_userptr``
-at struct
-:c:type:`v4l2_buffer` before calling
-:ref:`VIDIOC_QBUF`. Those operations should be avoided because they are not
-portable (endianness), and do not work for macroblock and Bayer formats
-and mmap buffers. The selection API deals with configuration of buffer
+The selection API was introduced to cope with deficiencies of the
+older :ref:`CROP API `, that was designed to control simple
+capture devices. Later the cropping API was adopted by video output
+drivers. The ioctls are used to select a part of the display were the
+video signal is inserted. It should be considered as an API abuse
+because the described operation is actually the composing. The
+selection API makes a clear distinction between composing and cropping
+operations by setting the appropriate targets.
+
+The V4L2 API lacks any support for composing to and cropping from an
+image inside a memory buffer. The application could configure a
+capture device to fill only a part of an image by abusing V4L2
+API. Cropping a smaller image from a larger one is achieved by setting
+the field ``bytesperline`` at struct :c:type:`v4l2_pix_format`.
+Introducing an image offsets could be done by modifying field
+``m_userptr`` at struct :c:type:`v4l2_buffer` before calling
+:ref:`VIDIOC_QBUF `. Those operations should be avoided
+because they are not portable (endianness), and do not work for
+macroblock and Bayer formats and mmap buffers.
+
+The selection API deals with configuration of buffer
 cropping/composing in a clear, intuitive and portable way. Next, with
 the selection API the concepts of the padded target and constraints
-flags are introduced. Finally, struct :c:type:`v4l2_crop`
-and struct :c:type:`v4l2_cropcap` have no reserved
-fields. Therefore there is no way to extend their functionality. The new
-struct :c:type:`v4l2_selection` provides a lot of place
-for future extensions. Driver developers are encouraged to implement
-only selection API. The former cropping API would be simulated using the
-new one.
+flags are introduced. Finally, struct :c:type:`v4l2_crop` and struct
+:c:type:`v4l2_cropcap` have no reserved fields. Therefore there is no
+way to extend their functionality. The new struct
+:c:type:`v4l2_selection` provides a lot of place for future
+extensions.
+
+Driver developers are encouraged to implement only selection API. The
+former cropping API would be simulated using the new one.
-- 
2.7.4

[PATCH 5/5] media: docs: selection: fix misleading sentence about the CROP API

[Luca Ceresoli]

The API limitation described here is about the CROP API, not about the
entire V4L2.

Cc: Hans Verkuil 
Signed-off-by: Luca Ceresoli 
---
 Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst 
b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
index ba1064a244a0..e7455fb1e572 100644
--- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
+++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst
@@ -15,7 +15,7 @@ because the described operation is actually the composing. The
 selection API makes a clear distinction between composing and cropping
 operations by setting the appropriate targets.
 
-The V4L2 API lacks any support for composing to and cropping from an
+The CROP API lacks any support for composing to and cropping from an
 image inside a memory buffer. The application could configure a
 capture device to fill only a part of an image by abusing V4L2
 API. Cropping a smaller image from a larger one is achieved by setting
-- 
2.7.4

Re: [PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[kbuild test robot]

Hi Rui,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.16]
[cannot apply to linuxtv-media/master next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180404-032903
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All errors (new ones prefixed by >>):

   drivers/media//i2c/ov2680.c: In function 'ov2680_set_fmt':
>> drivers/media//i2c/ov2680.c:692:9: error: implicit declaration of function 
>> 'v4l2_find_nearest_size'; did you mean 'v4l2_find_nearest_format'? 
>> [-Werror=implicit-function-declaration]
 mode = v4l2_find_nearest_size(ov2680_mode_data,
^~
v4l2_find_nearest_format
>> drivers/media//i2c/ov2680.c:693:41: error: 'width' undeclared (first use in 
>> this function)
  ARRAY_SIZE(ov2680_mode_data), width,
^
   drivers/media//i2c/ov2680.c:693:41: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/media//i2c/ov2680.c:694:11: error: 'height' undeclared (first use in 
>> this function); did you mean 'hweight8'?
  height, fmt->width, fmt->height);
  ^~
  hweight8
   cc1: some warnings being treated as errors

vim +692 drivers/media//i2c/ov2680.c

   672  
   673  static int ov2680_set_fmt(struct v4l2_subdev *sd,
   674struct v4l2_subdev_pad_config *cfg,
   675struct v4l2_subdev_format *format)
   676  {
   677  struct ov2680_dev *sensor = to_ov2680_dev(sd);
   678  struct v4l2_mbus_framefmt *fmt = >format;
   679  const struct ov2680_mode_info *mode;
   680  int ret = 0;
   681  
   682  if (format->pad != 0)
   683  return -EINVAL;
   684  
   685  mutex_lock(>lock);
   686  
   687  if (sensor->is_streaming) {
   688  ret = -EBUSY;
   689  goto unlock;
   690  }
   691  
 > 692  mode = v4l2_find_nearest_size(ov2680_mode_data,
 > 693ARRAY_SIZE(ov2680_mode_data), 
 > width,
 > 694height, fmt->width, fmt->height);
   695  if (!mode) {
   696  ret = -EINVAL;
   697  goto unlock;
   698  }
   699  
   700  if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
   701  fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
   702  
   703  *fmt = format->format;
   704  goto unlock;
   705  }
   706  
   707  fmt->width = mode->width;
   708  fmt->height = mode->height;
   709  fmt->code = sensor->fmt.code;
   710  fmt->colorspace = sensor->fmt.colorspace;
   711  
   712  sensor->current_mode = mode;
   713  sensor->fmt = format->format;
   714  sensor->mode_pending_changes = true;
   715  
   716  unlock:
   717  mutex_unlock(>lock);
   718  
   719  return ret;
   720  }
   721  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH] media: platform: video-mux: propagate format from sink to source

[Chris Lesiak]

Propagate the v4l2_mbus_framefmt to the source pad when either a sink
pad is activated or when the format of the active sink pad changes.

Signed-off-by: Chris Lesiak 
---
 drivers/media/platform/video-mux.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/video-mux.c 
b/drivers/media/platform/video-mux.c
index ee89ad76bee2..1fb887293337 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -45,6 +45,7 @@ static int video_mux_link_setup(struct media_entity *entity,
 {
struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+   u16 source_pad = entity->num_pads - 1;
int ret = 0;
 
/*
@@ -74,6 +75,9 @@ static int video_mux_link_setup(struct media_entity *entity,
if (ret < 0)
goto out;
vmux->active = local->index;
+
+   /* Propagate the active format to the source */
+   vmux->format_mbus[source_pad] = vmux->format_mbus[vmux->active];
} else {
if (vmux->active != local->index)
goto out;
@@ -162,14 +166,20 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
struct v4l2_subdev_format *sdformat)
 {
struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
-   struct v4l2_mbus_framefmt *mbusformat;
+   struct v4l2_mbus_framefmt *mbusformat, *source_mbusformat;
struct media_pad *pad = >pads[sdformat->pad];
+   u16 source_pad = sd->entity.num_pads - 1;
 
mbusformat = __video_mux_get_pad_format(sd, cfg, sdformat->pad,
sdformat->which);
if (!mbusformat)
return -EINVAL;
 
+   source_mbusformat = __video_mux_get_pad_format(sd, cfg, source_pad,
+  sdformat->which);
+   if (!source_mbusformat)
+   return -EINVAL;
+
mutex_lock(>lock);
 
/* Source pad mirrors active sink pad, no limitations on sink pads */
@@ -178,6 +188,10 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
 
*mbusformat = sdformat->format;
 
+   /* Propagate the format from an active sink to source */
+   if ((pad->flags & MEDIA_PAD_FL_SINK) && (pad->index == vmux->active))
+   *source_mbusformat = sdformat->format;
+
mutex_unlock(>lock);
 
return 0;
-- 
2.14.3

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

[Daniel Vetter]

On Tue, Apr 03, 2018 at 01:06:45PM -0400, Jerome Glisse wrote:
> On Tue, Apr 03, 2018 at 11:09:09AM +0200, Daniel Vetter wrote:
> > On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote:
> > > 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.
> > 
> > Looking at patch 8, the sg table seems perfectly sufficient to convey the
> > right dma addresses to the importer. Ofcourse the exporter has to set up
> > the right kind of iommu mappings to make this work.
> > 
> > > > 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.
> > 
> > The only thing the importer seems to do is call the
> > pci_peer_traffic_supported, which the exporter could call too. What am I
> > missing (since the sturct_page stuff sounds like it's fixed already by
> > you)?
> > -Daniel
> 
> AFAIK Logan patchset require to register and initialize struct page
> for the device memory you want to map (export from exporter point of
> view).
> 
> With GPU this isn't something we want, struct page is >~= 2^6 so for
> 4GB GPU = 2^6*2^32/2^12 = 2^26 = 64MB of RAM
> 8GB GPU = 2^6*2^33/2^12 = 2^27 = 128MB of RAM
> 16GB GPU = 2^6*2^34/2^12 = 2^28 = 256MB of RAM
> 32GB GPU = 2^6*2^34/2^12 = 2^29 = 512MB of RAM
> 
> All this is mostly wasted as only a small sub-set (that can not be
> constraint to specific range) will ever be exported at any point in
> time. For GPU work load this is hardly justifiable, even for HMM i
> do not plan to register all those pages.
> 
> Hence why i argue that dma_map_resource() like use by Christian is
> good enough for us. People that care about SG can fix that but i
> rather not have to depend on that and waste system memory.

I did not mean you should dma_map_sg/page. I just meant that using
dma_map_resource to fill only the dma address part of the sg table seems
perfectly sufficient. And that's exactly why the importer gets an already
mapped sg table, so that it doesn't have to call dma_map_sg on something
that dma_map_sg can't handle.

Assuming you get an sg table that's been mapping by calling dma_map_sg was
always a bit a case of bending the abstraction to avoid typing code. The
only thing an importer ever should have done is look at the dma addresses
in that sg table, nothing else.

And p2p seems to perfectly fit into this (surprise, it was meant to).
That's why I suggested we annotate the broken importers who assume the sg
table is mapped using dma_map_sg or has a struct_page backing the memory
(but there doesn't seem to be any left it seems) instead of annotating the
ones that aren't broken with a flag that's confusing - you could also have
a dma-buf sgt that points at some other memory that doesn't have struct
pages backing it.

Aside: At least internally in i915 we've been using this forever for our
own private/stolen memory. Unfortunately no other device can access that
range of memory, which is why we don't allow it to be imported to anything
but i915 itself. But if that hw restriction doesn't exist, it'd would
work.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Jerome Glisse]

On Tue, Apr 03, 2018 at 11:09:09AM +0200, Daniel Vetter wrote:
> On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote:
> > 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.
> 
> Looking at patch 8, the sg table seems perfectly sufficient to convey the
> right dma addresses to the importer. Ofcourse the exporter has to set up
> the right kind of iommu mappings to make this work.
> 
> > > 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.
> 
> The only thing the importer seems to do is call the
> pci_peer_traffic_supported, which the exporter could call too. What am I
> missing (since the sturct_page stuff sounds like it's fixed already by
> you)?
> -Daniel

AFAIK Logan patchset require to register and initialize struct page
for the device memory you want to map (export from exporter point of
view).

With GPU this isn't something we want, struct page is >~= 2^6 so for
4GB GPU = 2^6*2^32/2^12 = 2^26 = 64MB of RAM
8GB GPU = 2^6*2^33/2^12 = 2^27 = 128MB of RAM
16GB GPU = 2^6*2^34/2^12 = 2^28 = 256MB of RAM
32GB GPU = 2^6*2^34/2^12 = 2^29 = 512MB of RAM

All this is mostly wasted as only a small sub-set (that can not be
constraint to specific range) will ever be exported at any point in
time. For GPU work load this is hardly justifiable, even for HMM i
do not plan to register all those pages.

Hence why i argue that dma_map_resource() like use by Christian is
good enough for us. People that care about SG can fix that but i
rather not have to depend on that and waste system memory.

Cheers,
Jérôme

Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Arnd Bergmann]

On Tue, Apr 3, 2018 at 5:18 PM, Robert Jarzmik  wrote:
> Arnd Bergmann  writes:
>
>>> +   { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> +   { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> +   { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>>
>> This one is interesting, as you are dealing with an off-chip device,
>> and the channel number is '-'1. How does this even work? Does it
>> mean
>
> This relies on pxa_dma, in which the "-1" for a requestor line means "no
> requestor" or said in another way "always requesting". As a consequence, as 
> soon
> as the DMA descriptors are queued, the transfer begins, and it is supposed
> implicitely that the FIFO output availability is at least as quick as the 
> system
> bus and the DMA size is perfectly fit for the FIFO available bytes.
>
> This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
> platforms, where the smc91x(s) are directly wired on the system bus (the same
> bus having DRAM, SRAM, IO-mapped devices).

Ok, I looked at the driver in more detail now and found the scary parts.
So it's using the async DMA interface to do synchronous DMA in
interrupt context in order to transfer the rx data faster than an readsl()
would, correct?

It still feels odd to me that there is an entry in the slave map for
a device that does not have a request line. However, it also seems
that the entire code in those two drivers that deals with DMA is specific
to PXA anyway, so maybe it can be done differently: instead of
calling dma_request_slave_channel_compat() or dma_request_chan()
with a fake request line, how about calling dma_request_channel()
with an NULL filter function and data, and have the driver handle
the empty data case the same way as the rq=-1 case today?

>>> +   /* PXA25x specific map */
>>> +   { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>>> +   { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>>> +   { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> +   { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> +   { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> +   { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> +   { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> +   { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> +   { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> +   { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> +
>>> +   /* PXA27x specific map */
>>> +   { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>>> +   { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>>> +   { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>>> +   { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>>> +   { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>>> +
>>> +   /* PXA3xx specific map */
>>> +   { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>>> +   { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>>> +   { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>>> +   { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>>> +   { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>>> +   { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>>> +   { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>>> +};
>>
>> Since more than half the entries in here are chip specific, maybe it would be
>> better to split that table into three and have a copy for each one in
>> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
> Mmmh, today the split is :
>  - 16 common entries
>  - 10 pxa25x specific entries
>  - 5 pxa27x specific entries
>  - 7 pxa3xx specific entries
>  => total of 38 lines
>
> After the split we'll have :
>  - 26 pxa25x specific entries
>  - 21 pxa27x specific entries
>  - 23 pxa3xx specific entries
>  => total of 70 lines
>
> That doubles the number of lines, not counting the declarations, and amending 
> of
> pxa2xx_set_dmac_info().
>
> If you think it's worth it, what is the driving benefit behind ?

It seems a bit cleaner to only register the tables for the dma lines that
are actually present on a given chip.

>> Does that mean it's actually a memory-to-memory transfer with a device being
>> on the external SRAM interface?
> I'm taking this is the follow up to the "-1" question :0

Right.

Arnd

Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Robert Jarzmik]

Arnd Bergmann  writes:

>> +   { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> +   { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> +   { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>
> This one is interesting, as you are dealing with an off-chip device,
> and the channel number is '-'1. How does this even work? Does it
> mean

This relies on pxa_dma, in which the "-1" for a requestor line means "no
requestor" or said in another way "always requesting". As a consequence, as soon
as the DMA descriptors are queued, the transfer begins, and it is supposed
implicitely that the FIFO output availability is at least as quick as the system
bus and the DMA size is perfectly fit for the FIFO available bytes.

This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
platforms, where the smc91x(s) are directly wired on the system bus (the same
bus having DRAM, SRAM, IO-mapped devices).

>
>> +   /* PXA25x specific map */
>> +   { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>> +   { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>> +   { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> +   { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> +   { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> +   { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> +   { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> +   { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> +   { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> +   { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> +
>> +   /* PXA27x specific map */
>> +   { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>> +   { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>> +   { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>> +   { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>> +   { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>> +
>> +   /* PXA3xx specific map */
>> +   { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>> +   { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>> +   { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>> +   { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>> +   { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>> +   { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>> +   { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>> +};
>
> Since more than half the entries in here are chip specific, maybe it would be
> better to split that table into three and have a copy for each one in
> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
Mmmh, today the split is :
 - 16 common entries
 - 10 pxa25x specific entries
 - 5 pxa27x specific entries
 - 7 pxa3xx specific entries
 => total of 38 lines

After the split we'll have :
 - 26 pxa25x specific entries
 - 21 pxa27x specific entries
 - 23 pxa3xx specific entries
 => total of 70 lines

That doubles the number of lines, not counting the declarations, and amending of
pxa2xx_set_dmac_info().

If you think it's worth it, what is the driving benefit behind ?

> Does that mean it's actually a memory-to-memory transfer with a device being
> on the external SRAM interface?
I'm taking this is the follow up to the "-1" question :0

Cheers.

-- 
Robert

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Ulf Hansson]

On 2 April 2018 at 16:26, Robert Jarzmik  wrote:
> Hi,
>
> This serie is aimed at removing the dmaengine slave compat use, and transfer
> knowledge of the DMA requestors into architecture code.
>
> This was discussed/advised by Arnd a couple of years back, it's almost time.
>
> The serie is divided in 3 phasees :
>  - phase 1 : patch 1/15 and patch 2/15
>=> this is the preparation work
>  - phase 2 : patches 3/15 .. 10/15
>=> this is the switch of all the drivers
>=> this one will require either an Ack of the maintainers or be taken by 
> them
>   once phase 1 is merged
>  - phase 3 : patches 11/15
>=> this is the last part, cleanup and removal of export of the DMA filter
>   function
>
> As this looks like a patch bomb, each maintainer expressing for his tree 
> either
> an Ack or "I want to take through my tree" will be spared in the next 
> iterations
> of this serie.

Perhaps an option is to send this hole series as PR for 3.17 rc1, that
would removed some churns and make this faster/easier? Well, if you
receive the needed acks of course.

For the mmc change:

Acked-by: Ulf Hansson 

Kind regards
Uffe

>
> Several of these changes have been tested on actual hardware, including :
>  - pxamci
>  - pxa_camera
>  - smc*
>  - ASoC and SSP
>
> Happy review.
>
> Robert Jarzmik (15):
>   dmaengine: pxa: use a dma slave map
>   ARM: pxa: add dma slave map
>   mmc: pxamci: remove the dmaengine compat need
>   media: pxa_camera: remove the dmaengine compat need
>   mtd: nand: pxa3xx: remove the dmaengine compat need
>   net: smc911x: remove the dmaengine compat need
>   net: smc91x: remove the dmaengine compat need
>   ASoC: pxa: remove the dmaengine compat need
>   net: irda: pxaficp_ir: remove the dmaengine compat need
>   ata: pata_pxa: remove the dmaengine compat need
>   dmaengine: pxa: document pxad_param
>   dmaengine: pxa: make the filter function internal
>   ARM: pxa: remove the DMA IO resources
>   ARM: pxa: change SSP devices allocation
>   ARM: pxa: change SSP DMA channels allocation
>
>  arch/arm/mach-pxa/devices.c   | 269 
> ++
>  arch/arm/mach-pxa/devices.h   |  14 +-
>  arch/arm/mach-pxa/include/mach/audio.h|  12 ++
>  arch/arm/mach-pxa/pxa25x.c|   4 +-
>  arch/arm/mach-pxa/pxa27x.c|   4 +-
>  arch/arm/mach-pxa/pxa3xx.c|   5 +-
>  arch/arm/plat-pxa/ssp.c   |  50 +-
>  drivers/ata/pata_pxa.c|  10 +-
>  drivers/dma/pxa_dma.c |  13 +-
>  drivers/media/platform/pxa_camera.c   |  22 +--
>  drivers/mmc/host/pxamci.c |  29 +---
>  drivers/mtd/nand/pxa3xx_nand.c|  10 +-
>  drivers/net/ethernet/smsc/smc911x.c   |  16 +-
>  drivers/net/ethernet/smsc/smc91x.c|  12 +-
>  drivers/net/ethernet/smsc/smc91x.h|   1 -
>  drivers/staging/irda/drivers/pxaficp_ir.c |  14 +-
>  include/linux/dma/pxa-dma.h   |  20 +--
>  include/linux/platform_data/mmp_dma.h |   4 +
>  include/linux/pxa2xx_ssp.h|   4 +-
>  sound/arm/pxa2xx-ac97.c   |  14 +-
>  sound/arm/pxa2xx-pcm-lib.c|   6 +-
>  sound/soc/pxa/pxa-ssp.c   |   5 +-
>  sound/soc/pxa/pxa2xx-ac97.c   |  32 +---
>  23 files changed, 196 insertions(+), 374 deletions(-)
>
> --
> 2.11.0
>

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

[Niklas Söderlund]

Hi Maxime,

On 2018-04-03 15:48:59 +0200, Maxime Ripard wrote:
> Hi Niklas,
> 
> On Thu, Mar 29, 2018 at 02:35:34PM +0200, Niklas Söderlund wrote:
> > > + /*
> > > +  * Create a static mapping between the CSI virtual channels
> > > +  * and the input streams.
> > > +  *
> > > +  * This should be enhanced, but v4l2 lacks the support for
> > > +  * changing that mapping dynamically at the moment.
> > > +  *
> > > +  * We're protected from the userspace setting up links at the
> > > +  * same time by the upper layer having called
> > > +  * media_pipeline_start().
> > > +  */
> > > + list_for_each_entry(link, >links, list) {
> > 
> > I wonder is this list_for_each_entry() really needed? Can't you simply 
> > iterate over all sink pads as with the loop bellow but drop the pad == 
> > link->sink check? Maybe I'm missing something.
> 
> This was a review made by Sakari here:
> https://patchwork.linuxtv.org/patch/44422/
> 
> The idea is that we need to know if the pad is enabled, and as far as
> I know this information is only stored at the link level, not at the
> pad level.

Ahh I see, you are correct.

> 
> > Apart from this and the small nit-picks (one more bellow) I think this 
> > patch is fine. Once I understand this I be happy to add my tag to this 
> > change, great work!
> 
> Is this a reviewed by? :)

Yes, I now understand what I did not before :-) Feel free to add my

Reviewed-by: Niklas Söderlund 

> 
> > I also think you shall consider to add a MAINTAINERS record for the RX 
> > and TX drivers. Maybe one entry for both drivers as they live in the 
> > same directory but I think one of the two should add it :-)
> 
> Right, I'll do it, thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
Regards,
Niklas Söderlund

[PATCH v4 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v3->v4:
Sakari Ailus:
   - remove auto_{exposure|gain}_enable and direct call the set functions
   - add separe control sets to gain and exposure
   - fix number of controls allocated
   - check the exact frequency that it is supported

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui


Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 4 files changed, 1164 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.3

[PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 3 files changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..1af6175e09d5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1, @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_VALUE 2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   boolis_enabled;
+   boolis_streaming;
+
+   

[PATCH v4 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.3

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

[Maxime Ripard]

Hi Niklas,

Thanks for your review,

On Thu, Mar 29, 2018 at 03:32:55PM +0200, Niklas Söderlund wrote:
> > 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?

You don't strictly need it, but you're right and one should be better,
I've added it.

Maxime

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


signature.asc
Description: PGP signature

Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller

[Laurent Pinchart]

Hi Hans,

On Tuesday, 3 April 2018 15:30:40 EEST Hans Verkuil wrote:
> On 26/03/18 23:44, Niklas Söderlund wrote:
> > Hi,
> > 
> > This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795,
> > r8a7796 and r8a77970. It is based on the media-tree and depends on
> > Fabrizio Castro patches as they touches the order of the compatible
> > strings in the documentation to reduce merge conflicts. The dependencies
> > are included in this series.
> 
> Laurent, Kieran,
> 
> Unless there are any objections I want to make a pull request for this
> series once 4.17-rc1 has been merged back into our master tree. It all
> looks good to me, and it will be nice to get this in (finally!).
> 
> I don't want to postpone the pull request for small improvements, they
> can be applied later. But if there are more serious concerns, then let
> us know.

There's just a handful of patchs I haven't acked (but I have reviewed previous 
version) and I don't think there's any blocker there, small problems can 
always be fixed later. I'll still try to review them, and if I notice any 
issue I'll ask Niklas to submit follow-up patches. v13 can be merged upstream 
from my point of view. Thank you for handling this.

> > The driver is tested on Renesas H3 (r8a7795, ES2.0),
> > M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and
> > not yet upstream) and the Salvator-X onboard ADV7482. It is also tested
> > on the V3M (r8a77970) on the Eagle board together with its expansion
> > board with a ADV7482 and out of tree patches for GMSL capture using the
> > max9286 and rdacm20 drivers.
> > 
> > It is possible to capture both CVBS and HDMI video streams,
> > v4l2-compliance passes with no errors and media-ctl can be used to
> > change the routing and formats for the different entities in the media
> > graph.
> > 
> > Gen2 compatibility is verified on Koelsch and no problems where found,
> > video can be captured just like before and v4l2-compliance passes
> > without errors or warnings just like before this series.
> > 
> > For convenience the series can be fetched from:
> >   git://git.ragnatech.se/linux rcar/vin/mc-v13
> > 
> > I have started on a very basic test suite for the VIN driver at:
> >   https://git.ragnatech.se/vin-tests
> > 
> > And as before the state of the driver and information about how to test
> > 
> > it can be found on the elinux wiki:
> >   http://elinux.org/R-Car/Tests:rcar-vin
> > 
> > * Changes from v12
> > - Rebase to latest media-tree/master changed a 'return ret' to a 'goto
> > 
> >   out' in rvin_start_streaming() to take recent changes to the VIN
> >   driver into account.
> > 
> > - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to
> > 
> >   fallback on' check from a later commit 'rcar-vin: simplify how formats
> >   are set and reset' in the series. This is to avoid ignoring the field
> >   returned from the sensor if FIELD_ANY was requested by the user. This
> >   was only a problem between this change and a few patches later, but
> >   better to fix it now. Reported by Hans, thanks for spotting this.
> > 
> > - Fix spelling.
> > - Add review tags from Hans.
> > 
> > * Changes since v11
> > - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default
> > 
> >   colorspace for media centric mode'. Also set fixed values for
> >   xfer_func, quantization and ycbcr_enc.
> > 
> > - Reorderd filed order in struct rvin_group_route.
> > - Renamed chan to channel in struct rvin_group_route.
> > - Rework 'rcar-vin: read subdevice format for crop only when
> > 
> >   needed' into 'rcar-vin: simplify how formats are set and reset'.
> > 
> > - Keep caching the source dimensions and drop all changes to
> > 
> >   rvin_g_selection() and rvin_s_selection().
> > 
> > - Inline rvin_get_vin_format_from_source() into rvin_reset_format()
> > 
> >   which now is the only user left.
> > 
> > - Add patch to cache the video standard instead of reading it at stream
> > 
> >   on.
> > 
> > - Fix error labels in rvin_mc_open().
> > - Fixed spelling in commit messages and comment, thanks Laurent!
> > - Added reviewed tags from Laurent, Thanks!
> > 
> > * Changes since v10
> > - Corrected spelling in comments and commit messages.
> > - Reworked 'rcar-vin: read subdevice format for crop only when needed'
> > 
> >   to only get the source format once per operation.
> > 
> > - Moved some patches around to make it easier to review, moved:
> > - rcar-vin: set a default field to fallback on
> > - rcar-vin: fix handling of single field frames (top, bottom and
> > alternate fields) - rcar-vin: update bytesperline and sizeimage
> > calculation
> > - rcar-vin: break out format alignment and checking
> > - rcar-vin: update pixelformat check for M1
> >   
> >   Before:
> > - rcar-vin: read subdevice format for crop only when needed
> > 
> > - Rename variable 'code' to 'mbus_code' in struct rvin_dev.
> > - Add comment describing no locking is needed in
> > 
> >  

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

[Maxime Ripard]

Hi Niklas,

On Thu, Mar 29, 2018 at 02:35:34PM +0200, Niklas Söderlund wrote:
> > +   /*
> > +* Create a static mapping between the CSI virtual channels
> > +* and the input streams.
> > +*
> > +* This should be enhanced, but v4l2 lacks the support for
> > +* changing that mapping dynamically at the moment.
> > +*
> > +* We're protected from the userspace setting up links at the
> > +* same time by the upper layer having called
> > +* media_pipeline_start().
> > +*/
> > +   list_for_each_entry(link, >links, list) {
> 
> I wonder is this list_for_each_entry() really needed? Can't you simply 
> iterate over all sink pads as with the loop bellow but drop the pad == 
> link->sink check? Maybe I'm missing something.

This was a review made by Sakari here:
https://patchwork.linuxtv.org/patch/44422/

The idea is that we need to know if the pad is enabled, and as far as
I know this information is only stored at the link level, not at the
pad level.

> Apart from this and the small nit-picks (one more bellow) I think this 
> patch is fine. Once I understand this I be happy to add my tag to this 
> change, great work!

Is this a reviewed by? :)

> I also think you shall consider to add a MAINTAINERS record for the RX 
> and TX drivers. Maybe one entry for both drivers as they live in the 
> same directory but I think one of the two should add it :-)

Right, I'll do it, thanks!
Maxime

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


signature.asc
Description: PGP signature

Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller

[Kieran Bingham]

Hi Hans,

On 03/04/18 13:30, Hans Verkuil wrote:
> On 26/03/18 23:44, Niklas Söderlund wrote:
>> Hi,
>>
>> This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795,
>> r8a7796 and r8a77970. It is based on the media-tree and depends on
>> Fabrizio Castro patches as they touches the order of the compatible
>> strings in the documentation to reduce merge conflicts. The dependencies
>> are included in this series.
> 
> Laurent, Kieran,
> 
> Unless there are any objections I want to make a pull request for this
> series once 4.17-rc1 has been merged back into our master tree. It all
> looks good to me, and it will be nice to get this in (finally!).
> 
> I don't want to postpone the pull request for small improvements, they
> can be applied later. But if there are more serious concerns, then let
> us know.

Certainly sounds good, and no objections from me.
Looking forward to getting this series in.

Thanks

Kieran


> Regards,
> 
>   Hans
> 
>>
>> The driver is tested on Renesas H3 (r8a7795, ES2.0),
>> M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and
>> not yet upstream) and the Salvator-X onboard ADV7482. It is also tested
>> on the V3M (r8a77970) on the Eagle board together with its expansion
>> board with a ADV7482 and out of tree patches for GMSL capture using the
>> max9286 and rdacm20 drivers.
>>
>> It is possible to capture both CVBS and HDMI video streams,
>> v4l2-compliance passes with no errors and media-ctl can be used to
>> change the routing and formats for the different entities in the media
>> graph.
>>
>> Gen2 compatibility is verified on Koelsch and no problems where found,
>> video can be captured just like before and v4l2-compliance passes
>> without errors or warnings just like before this series.
>>
>> For convenience the series can be fetched from:
>>
>>   git://git.ragnatech.se/linux rcar/vin/mc-v13
>>
>> I have started on a very basic test suite for the VIN driver at:
>>
>>   https://git.ragnatech.se/vin-tests
>>
>> And as before the state of the driver and information about how to test
>> it can be found on the elinux wiki:
>>
>>   http://elinux.org/R-Car/Tests:rcar-vin
>>
>> * Changes from v12
>> - Rebase to latest media-tree/master changed a 'return ret' to a 'goto
>>   out' in rvin_start_streaming() to take recent changes to the VIN 
>>   driver into account.
>> - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to  
>>   fallback on' check from a later commit 'rcar-vin: simplify how formats 
>>   are set and reset' in the series. This is to avoid ignoring the field 
>>   returned from the sensor if FIELD_ANY was requested by the user. This 
>>   was only a problem between this change and a few patches later, but 
>>   better to fix it now. Reported by Hans, thanks for spotting this.
>> - Fix spelling.
>> - Add review tags from Hans.
>>
>> * Changes since v11
>> - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default
>>   colorspace for media centric mode'. Also set fixed values for
>>   xfer_func, quantization and ycbcr_enc.
>> - Reorderd filed order in struct rvin_group_route.
>> - Renamed chan to channel in struct rvin_group_route.
>> - Rework 'rcar-vin: read subdevice format for crop only when
>>   needed' into 'rcar-vin: simplify how formats are set and reset'.
>> - Keep caching the source dimensions and drop all changes to
>>   rvin_g_selection() and rvin_s_selection().
>> - Inline rvin_get_vin_format_from_source() into rvin_reset_format()
>>   which now is the only user left.
>> - Add patch to cache the video standard instead of reading it at stream
>>   on.
>> - Fix error labels in rvin_mc_open().
>> - Fixed spelling in commit messages and comment, thanks Laurent!
>> - Added reviewed tags from Laurent, Thanks!
>>
>> * Changes since v10
>> - Corrected spelling in comments and commit messages.
>> - Reworked 'rcar-vin: read subdevice format for crop only when needed'
>>   to only get the source format once per operation.
>> - Moved some patches around to make it easier to review, moved:
>> - rcar-vin: set a default field to fallback on
>> - rcar-vin: fix handling of single field frames (top, bottom and 
>> alternate fields)
>> - rcar-vin: update bytesperline and sizeimage calculation
>> - rcar-vin: break out format alignment and checking
>> - rcar-vin: update pixelformat check for M1
>>   Before:
>> - rcar-vin: read subdevice format for crop only when needed
>> - Rename variable 'code' to 'mbus_code' in struct rvin_dev.
>> - Add comment describing no locking is needed in
>>   rvin_set_channel_routing().
>> - Check return value of pm_runtime_get_sync() in
>>   rvin_set_channel_routing().
>> - Rework 'rcar-vin: add check for colorspace' to not try to check the
>>   format, instead force a default format. This should be revisited once
>>   either v4l2-compliance or v4l2 framework changes are worked out to
>>   allow for MC centric drivers to validate user supplied colorspace.
>> - Add 

Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller

[Hans Verkuil]

On 26/03/18 23:44, Niklas Söderlund wrote:
> Hi,
> 
> This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795,
> r8a7796 and r8a77970. It is based on the media-tree and depends on
> Fabrizio Castro patches as they touches the order of the compatible
> strings in the documentation to reduce merge conflicts. The dependencies
> are included in this series.

Laurent, Kieran,

Unless there are any objections I want to make a pull request for this
series once 4.17-rc1 has been merged back into our master tree. It all
looks good to me, and it will be nice to get this in (finally!).

I don't want to postpone the pull request for small improvements, they
can be applied later. But if there are more serious concerns, then let
us know.

Regards,

Hans

> 
> The driver is tested on Renesas H3 (r8a7795, ES2.0),
> M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and
> not yet upstream) and the Salvator-X onboard ADV7482. It is also tested
> on the V3M (r8a77970) on the Eagle board together with its expansion
> board with a ADV7482 and out of tree patches for GMSL capture using the
> max9286 and rdacm20 drivers.
> 
> It is possible to capture both CVBS and HDMI video streams,
> v4l2-compliance passes with no errors and media-ctl can be used to
> change the routing and formats for the different entities in the media
> graph.
> 
> Gen2 compatibility is verified on Koelsch and no problems where found,
> video can be captured just like before and v4l2-compliance passes
> without errors or warnings just like before this series.
> 
> For convenience the series can be fetched from:
> 
>   git://git.ragnatech.se/linux rcar/vin/mc-v13
> 
> I have started on a very basic test suite for the VIN driver at:
> 
>   https://git.ragnatech.se/vin-tests
> 
> And as before the state of the driver and information about how to test
> it can be found on the elinux wiki:
> 
>   http://elinux.org/R-Car/Tests:rcar-vin
> 
> * Changes from v12
> - Rebase to latest media-tree/master changed a 'return ret' to a 'goto
>   out' in rvin_start_streaming() to take recent changes to the VIN 
>   driver into account.
> - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to  
>   fallback on' check from a later commit 'rcar-vin: simplify how formats 
>   are set and reset' in the series. This is to avoid ignoring the field 
>   returned from the sensor if FIELD_ANY was requested by the user. This 
>   was only a problem between this change and a few patches later, but 
>   better to fix it now. Reported by Hans, thanks for spotting this.
> - Fix spelling.
> - Add review tags from Hans.
> 
> * Changes since v11
> - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default
>   colorspace for media centric mode'. Also set fixed values for
>   xfer_func, quantization and ycbcr_enc.
> - Reorderd filed order in struct rvin_group_route.
> - Renamed chan to channel in struct rvin_group_route.
> - Rework 'rcar-vin: read subdevice format for crop only when
>   needed' into 'rcar-vin: simplify how formats are set and reset'.
> - Keep caching the source dimensions and drop all changes to
>   rvin_g_selection() and rvin_s_selection().
> - Inline rvin_get_vin_format_from_source() into rvin_reset_format()
>   which now is the only user left.
> - Add patch to cache the video standard instead of reading it at stream
>   on.
> - Fix error labels in rvin_mc_open().
> - Fixed spelling in commit messages and comment, thanks Laurent!
> - Added reviewed tags from Laurent, Thanks!
> 
> * Changes since v10
> - Corrected spelling in comments and commit messages.
> - Reworked 'rcar-vin: read subdevice format for crop only when needed'
>   to only get the source format once per operation.
> - Moved some patches around to make it easier to review, moved:
> - rcar-vin: set a default field to fallback on
> - rcar-vin: fix handling of single field frames (top, bottom and 
> alternate fields)
> - rcar-vin: update bytesperline and sizeimage calculation
> - rcar-vin: break out format alignment and checking
> - rcar-vin: update pixelformat check for M1
>   Before:
> - rcar-vin: read subdevice format for crop only when needed
> - Rename variable 'code' to 'mbus_code' in struct rvin_dev.
> - Add comment describing no locking is needed in
>   rvin_set_channel_routing().
> - Check return value of pm_runtime_get_sync() in
>   rvin_set_channel_routing().
> - Rework 'rcar-vin: add check for colorspace' to not try to check the
>   format, instead force a default format. This should be revisited once
>   either v4l2-compliance or v4l2 framework changes are worked out to
>   allow for MC centric drivers to validate user supplied colorspace.
> - Add error checking for pm_runtime_get_sync() and
>   v4l2_pipeline_pm_use().
> - Change mutex_lock() to mutex_lock_interruptible() in rvin_mc_open().
> - Rewrote documentation for struct rvin_group_route.
> - Rename rvin_mc_parse_v4l2() to rvin_mc_parse_of_endpoint().

I am waiting to hear from you soon,

[Mabel Raymond]

From
The international Investigation Financial Crime  Unit

Attention Dear!!
We need your urgent information .

I am writing to inform you that I am Staff In the international
investigation financial crime  unit and  I have discovered through our
network E-mail  system This Week  that your  Inheritance Claim  Fund
$6.5m usd have been trapped by Some Bank Officers and  who specialized
in collecting money from You.

There fore I would appreciate to inform you that there  
is hope for you to recover all what you have lost . Your  Inheritance
Fund is $6.5m usd  Have Been Removed From That Bank And  Deposited To
Security Financier Company This Morning In Burkina Faso.

You should send to us your complete  information to enable us
Submit Them   on your behalf  To The Security And Finance Company To
Enable The Finance Company Proceed Transferring Your Inheritance Fund
$6.5m usd to your country!  This will be completed within the  next
few  days .

1.Your complete name

2.Your Mobile phone number

3. Your nationality

 I am waiting to hear from you soon,

Thank you  .

Mrs Raymond  Mabel

Re: [PATCH v9 2/2] media: V3s: Add support for Allwinner CSI.

[Hans Verkuil]

On 06/03/18 03:16, Yong Deng wrote:
> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> interface and CSI1 is used for parallel interface. This is not
> documented in datasheet but by test and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Reviewed-by: Maxime Ripard 
> Tested-by: Maxime Ripard 
> Signed-off-by: Yong Deng 

Reviewed-by: Hans Verkuil 

Thanks!

Hans

[PATCH] media: gspca: fix Kconfig help info

[winton.liu]

Documentation/video4linux/gspca.txt is missing.
It has moved to Documentation/media/v4l-drivers/gspca-cardlist.rst

Signed-off-by: winton.liu <18502523...@163.com>
---
 drivers/media/usb/gspca/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/gspca/Kconfig b/drivers/media/usb/gspca/Kconfig
index d214a21..bc9a439 100644
--- a/drivers/media/usb/gspca/Kconfig
+++ b/drivers/media/usb/gspca/Kconfig
@@ -7,7 +7,7 @@ menuconfig USB_GSPCA
  Say Y here if you want to enable selecting webcams based
  on the GSPCA framework.
 
- See  for more info.
+ See  for 
more info.
 
  This driver uses the Video For Linux API. You must say Y or M to
  "Video For Linux" to use this driver.
-- 
2.7.4

[GIT PULL for v4.17-rc1] media updates

[Mauro Carvalho Chehab]

Hi Linus,

Please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.17-1

For:
  - New CEC pin injection code for testing purposes;
  - DVB frontend cxd2099 promoted from staging;
  - New platform driver for Sony cxd2880 DVB devices;
  - New sensor drivers: mt9t112, ov2685, ov5695, ov772x, tda1997x, tw9910.c
  - Removal of unused cx18 and ivtv alsa mixers;
  - The reneseas-ceu driver doesn't depend on soc_camera anymore and
moved from staging;
  - Removed the mantis_vp3028 driver, unused since 2009;
  - s5p-mfc: add support for version 10 of the MSP;
  - Added a decoder for imon protocol;
  - atomisp: lots of cleanups;
  - imx074 and mt9t031: don't depend on soc_camera anymore, being 
promoted from staging;
  - added helper functions to better support DVB I2C binding;
  - lots of driver improvements and cleanups.

The following changes since commit 661e50bc853209e41a5c14a290ca4decc43cbfd1:

  Linux 4.16-rc4 (2018-03-04 14:54:11 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.17-1

for you to fetch changes up to f8a695c4b43d02c89b8bba9ba6058fd5db1bc71d:

  media: v4l2-ioctl: rename a temp var that stores _IOC_SIZE(cmd) (2018-03-26 
06:58:47 -0400)


media updates for v4.17-rc1


A Sun (1):
  media: mceusb: add IR learning support features (IR carrier frequency 
measurement and wide-band/short-range receiver)

Akinobu Mita (3):
  media: MAINTAINERS: add entry for ov9650 driver
  media: ov9650: add device tree binding
  media: ov9650: support device tree probing

Alexandre Courbot (3):
  media: v4l: vidioc-prepare-buf.rst: fix link to VIDIOC_QBUF
  media: v4l2_fh.h: add missing kconfig.h include
  media: media-types.rst: fix typo

Alexey Khoroshilov (1):
  media: rc: ir-hix5hd2: fix error handling of clk_prepare_enable()

Alona Solntseva (1):
  media: drivers: staging: media: atomisp: pci: atomisp2: css2400: fix 
misspellings

Andi Kleen (1):
  media: rc: don't mark IR decoders default y

Antonio Cardace (2):
  media: em28xx: use %*phC to print small buffers
  media: gspca: dtcs033: use %*ph to print small buffer

Antti Palosaari (18):
  media: af9013: change lock detection slightly
  media: af9013: dvbv5 signal strength
  media: af9013: dvbv5 cnr
  media: af9013: dvbv5 ber and per
  media: af9013: wrap dvbv3 statistics via dvbv5
  media: af9015: fix logging
  media: af9013: convert inittabs suitable for regmap_update_bits
  media: af9013: add i2c mux adapter for tuner bus
  media: af9015: attach demod using i2c binding
  media: af9013: remove all legacy media attach releated stuff
  media: af9013: add pid filter support
  media: af9015: use af9013 demod pid filters
  media: af9015: refactor firmware download
  media: af9015: refactor copy firmware to slave demod
  media: af9015: enhance streaming config
  media: dvb-usb-v2: add probe/disconnect callbacks
  media: af9015: convert to regmap api
  media: af9015: correct some coding style issues

Arnd Bergmann (10):
  media: au0828: fix VIDEO_V4L2 dependency
  media: i2c: TDA1997x: add CONFIG_SND dependency
  media: ov5695: mark PM functions as __maybe_unused
  media: ov2685: mark PM functions as __maybe_unused
  media: em28xx: split up em28xx_dvb_init to reduce stack size
  media: s3c-camif: fix out-of-bounds array access
  media: renesas-ceu: mark PM functions as __maybe_unused
  media: staging: media: atomisp: remove pointless string copy
  media: v4l: omap_vout: vrfb: remove an unused variable
  media: ngene: avoid unused variable warning

Arushi Singhal (4):
  media: staging: media: Remove unnecessary semicolon
  media: staging: media: Replace "be be" with "be"
  media: staging: media: Replace "dont" with "don't"
  media: staging: media: Replace "cant" with "can't"

Benjamin Gaignard (1):
  media: platform: stm32: Adopt SPDX identifier

Brad Love (37):
  media: em28xx: Hauppauge DualHD second tuner functionality
  media: em28xx: Bulk transfer implementation fix
  media: em28xx: USB bulk packet size fix
  media: em28xx: Increase max em28xx boards to max dvb adapters
  media: em28xx: Add Hauppauge SoloHD/DualHD bulk models
  media: em28xx: Enable Hauppauge SoloHD rebranded 292e SE
  media: lgdt3306a: Set fe ops.release to NULL if probed
  media: lgdt3306a: QAM streaming improvement
  media: lgdt3306a: Add QAM AUTO support
  media: lgdt3306a: Fix module count mismatch on usb unplug
  media: lgdt3306a: Fix a double kfree on i2c device remove
  media: cx23885: Enable new Hauppauge PCIe ImpactVCBe variant
  media: cx23885: Add support for Hauppauge PCIe HVR1265 K4
  

Re: [PATCH] media: coda: do not try to propagate format if capture queue busy

[Tomasz Figa]

Hi Philipp,

On Thu, Mar 29, 2018 at 2:12 AM Philipp Zabel 
wrote:

> The driver helpfully resets the capture queue format and selection
> rectangle whenever output format is changed. This only works while
> the capture queue is not busy.

Is the code in question used only for decoder case? For encoder, CAPTURE
queue determines the codec and so things should work the other way around,
i.e. setting CAPTURE format should reset OUTPUT format and it should be
allowed only if OUTPUT queue is not busy.

Best regards,
Tomasz

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

[Daniel Vetter]

On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote:
> 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.

Looking at patch 8, the sg table seems perfectly sufficient to convey the
right dma addresses to the importer. Ofcourse the exporter has to set up
the right kind of iommu mappings to make this work.

> > 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.

The only thing the importer seems to do is call the
pci_peer_traffic_supported, which the exporter could call too. What am I
missing (since the sturct_page stuff sounds like it's fixed already by
you)?
-Daniel

> > 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: [PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Hans Verkuil]

On 27/03/18 00:25, Rob Herring wrote:
> On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Some boards have both a DesignWare and their own CEC controller.
>> The CEC pin is only hooked up to their own CEC controller and not
>> to the DW controller.
>>
>> Add the cec-disable property to disable the DW CEC controller.
>>
>> This particular situation happens on Amlogic boards that have their
>> own meson CEC controller.
> 
> Seems like we could avoid this by describing how the CEC line is hooked 
> up which could be needed for other reasons.

So there are three situations:

1) The cec pin is connected to the DW HDMI TX. That's already supported.
2) The cec pin is not connected at all, but the CEC IP is instantiated.
   We need the cec-disable property for that. This simply states that the
   CEC pin is not connected.
3) The cec pin is connected to an HDMI RX. We do not support this at the
   moment. If we want to support this, then we need a 'hdmi-rx' phandle
   that points to the HDMI receiver that the CEC pin is associated with.
   This will be similar to the already existing 'hdmi-phandle' property
   used to associate a CEC driver with an HDMI transmitter. In hindsight
   it would have been better if 'hdmi-phandle' was named 'hdmi-tx' :-(

I can make a binding proposal for 3, but I have no hardware to test it,
so I think it is better to add this only when someone has hardware. It
will require quite a few changes to the driver and likely also the CEC core.

Regards,

Hans

> 
>>
>> Signed-off-by: Hans Verkuil 
>> Acked-by: Neil Armstrong 
>> ---
>>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>>  1 file changed, 3 insertions(+)

Re: V4l2 Sensor driver and V4l2 ctrls

[Hans Verkuil]

On 03/04/18 08:32, asadpt iqroot wrote:
> Hi Hans,
> 
> Thanks for the reply.
> 
> In board, we have the HDMI connectors. Is it mandatory to use this ctrl
> V4L2_CID_DV_RX_POWER_PRESENT. Based on this v4l2 ctrl, what v4l2 framework
> will do? If I do not set any value to this ctrl, what will happen to
> video streaming?

This control isn't for the kernel, it's to help userspace. Userspace can
subscribe to this control and receive an event whenever it changes value
(i.e. whenever the driver detects a change in the 5V HDMI line).

It then knows that it looks like something has been connected to the receiver
and it can start waiting for a stable signal, inform the user or just keep
track of it internally for debug purposes.

What HDMI receivers do you use? Analog Devices? NXP? SoC-based?

> 
> For example, if I did not add this control in driver, what will
> happen? Whether this will
> affect the functionality of the video streaming? or not.

No, it is just to inform userspace that something was connected or
disconnected. And to aid in debugging problems.

> Why do we have the standard v4l2 ctrl like this?

See above.

Note: if you plan on upstreaming your HDMI receiver driver, then this control
must be present.

Regards,

Hans

> 
> Thanks for the inputs.
> 
> On 30 March 2018 at 12:58, Hans Verkuil  wrote:
>> On 30/03/18 09:23, asadpt iqroot wrote:
>>> Hi Hans,
>>>
>>> Thanks for the reply.
>>>
>>> In HDMI receivers, when we need to use this control. What scenario?
>>
>> https://www.linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/extended-controls.html#digital-video-control-reference
>>
>> "Detects whether the receiver receives power from the source (e.g. HDMI 
>> carries 5V on one of the pins)."
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> -Thanks.
>>>
>>>
>>> On 30 March 2018 at 12:13, Hans Verkuil  wrote:
 On 30/03/18 08:16, asadpt iqroot wrote:
> Hi All,
>
> In reference sensor drivers, they used the
> V4L2_CID_DV_RX_POWER_PRESENT v4l2 ctrl.
> It is a standard ctrl and created using v4l2_ctrl_new_std().
>
> The doubts are:
>
> 1. Whether in our sensor driver, we need to create this Control Id or
> not. How to take the decision on this. Since this is the standard
> ctrl. When we need to use these standard ctrls??

 No. This control is for HDMI receivers, not for sensors.

 Regards,

 Hans

>
> 2. In Sensor driver, the ctrls creation is anything depends on the
> bridge driver.
> Based on bridge driver, whether we need to create any ctrls in Sensor 
> driver.
>
> This question belongs to design of the sensor driver.
>
>
>
> Thanks & Regards
>

>>

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:
> Hi,
>
> This serie is aimed at removing the dmaengine slave compat use, and transfer
> knowledge of the DMA requestors into architecture code.
>
> This was discussed/advised by Arnd a couple of years back, it's almost time.
>
> The serie is divided in 3 phasees :
>  - phase 1 : patch 1/15 and patch 2/15
>=> this is the preparation work
>  - phase 2 : patches 3/15 .. 10/15
>=> this is the switch of all the drivers
>=> this one will require either an Ack of the maintainers or be taken by 
> them
>   once phase 1 is merged
>  - phase 3 : patches 11/15
>=> this is the last part, cleanup and removal of export of the DMA filter
>   function
>
> As this looks like a patch bomb, each maintainer expressing for his tree 
> either
> an Ack or "I want to take through my tree" will be spared in the next 
> iterations
> of this serie.
>
> Several of these changes have been tested on actual hardware, including :
>  - pxamci
>  - pxa_camera
>  - smc*
>  - ASoC and SSP
>
> Happy review.

This looks really great overall, thanks for cleaning this up!

The SSP part is still a bit weird, as I commented, but I'm sure we can
figure something out there.

Arnd

Re: [PATCH 12/15] dmaengine: pxa: make the filter function internal

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 6:35 PM, kbuild test robot  wrote:

>
>drivers/mtd/nand/marvell_nand.c:2621:17: sparse: undefined identifier 
> 'pxad_filter_fn'
>>> drivers/mtd/nand/marvell_nand.c:2621:17: sparse: call with no type!
>In file included from drivers/mtd/nand/marvell_nand.c:21:0:
>drivers/mtd/nand/marvell_nand.c: In function 'marvell_nfc_init_dma':
>drivers/mtd/nand/marvell_nand.c:2621:42: error: 'pxad_filter_fn' 
> undeclared (first use in this function); did you mean 'dma_filter_fn'?
>   dma_request_slave_channel_compat(mask, pxad_filter_fn,
>  ^
>include/linux/dmaengine.h:1408:46: note: in definition of macro 
> 'dma_request_slave_channel_compat'
>  __dma_request_slave_channel_compat(&(mask), x, y, dev, name)
>  ^
>drivers/mtd/nand/marvell_nand.c:2621:42: note: each undeclared identifier 
> is reported only once for each function it appears in
>   dma_request_slave_channel_compat(mask, pxad_filter_fn,
>  ^
>include/linux/dmaengine.h:1408:46: note: in definition of macro 
> 'dma_request_slave_channel_compat'
>  __dma_request_slave_channel_compat(&(mask), x, y, dev, name)

The driver is a replacement for the pxa3xx nand driver, so it now has
to get changed as well.

   Arnd

Re: [PATCH 14/15] ARM: pxa: change SSP devices allocation

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:

>
> +static struct pxa_ssp_info pxa_ssp_infos[] = {
> +   { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
> +   { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
> +   { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
> +   { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
> +   { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
> +   { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
> +   { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
> +   { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
> +};

This part looks odd to me, you're adding an extra level of indirection to
do two stages of lookups in some form of platform data.

Why can't you just always use "rx" and "tx" as the names here?

(also, I don't see why each line is duplicated, but I'm sure there's
an easy answer for that).

   Arnd

Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:
> +
> +static const struct dma_slave_map pxa_slave_map[] = {
> +   /* PXA25x, PXA27x and PXA3xx common entries */
> +   { "pxa-pcm-audio", "ac97_mic_mono", PDMA_FILTER_PARAM(LOWEST, 8) },
> +   { "pxa-pcm-audio", "ac97_aux_mono_in", PDMA_FILTER_PARAM(LOWEST, 9) },
> +   { "pxa-pcm-audio", "ac97_aux_mono_out", PDMA_FILTER_PARAM(LOWEST, 10) 
> },
> +   { "pxa-pcm-audio", "ac97_stereo_in", PDMA_FILTER_PARAM(LOWEST, 11) },
> +   { "pxa-pcm-audio", "ac97_stereo_out", PDMA_FILTER_PARAM(LOWEST, 12) },
> +   { "pxa-pcm-audio", "ssp1_rx", PDMA_FILTER_PARAM(LOWEST, 13) },
> +   { "pxa-pcm-audio", "ssp1_tx", PDMA_FILTER_PARAM(LOWEST, 14) },
> +   { "pxa-pcm-audio", "ssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa-pcm-audio", "ssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa2xx-ir", "rx", PDMA_FILTER_PARAM(LOWEST, 17) },
> +   { "pxa2xx-ir", "tx", PDMA_FILTER_PARAM(LOWEST, 18) },
> +   { "pxa2xx-mci.0", "rx", PDMA_FILTER_PARAM(LOWEST, 21) },
> +   { "pxa2xx-mci.0", "tx", PDMA_FILTER_PARAM(LOWEST, 22) },


> +   { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
> +   { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
> +   { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },

This one is interesting, as you are dealing with an off-chip device,
and the channel number is '-'1. How does this even work? Does it
mean

> +   /* PXA25x specific map */
> +   { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
> +   { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
> +   { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
> +   { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
> +   { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
> +   { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
> +
> +   /* PXA27x specific map */
> +   { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
> +   { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
> +   { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
> +   { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
> +   { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
> +
> +   /* PXA3xx specific map */
> +   { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
> +   { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
> +   { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
> +   { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
> +   { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
> +   { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
> +   { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
> +};

Since more than half the entries in here are chip specific, maybe it would be
better to split that table into three and have a copy for each one in
arch/arm/mach-pxa/pxa{25x.27x.3xx}.c? Does that mean it's actually
a memory-to-memory transfer with a device being on the external
SRAM interface?

   Arnd

Re: V4l2 Sensor driver and V4l2 ctrls

[asadpt iqroot]

Hi Hans,

Thanks for the reply.

In board, we have the HDMI connectors. Is it mandatory to use this ctrl
V4L2_CID_DV_RX_POWER_PRESENT. Based on this v4l2 ctrl, what v4l2 framework
will do? If I do not set any value to this ctrl, what will happen to
video streaming?

For example, if I did not add this control in driver, what will
happen? Whether this will
affect the functionality of the video streaming? or not.

Why do we have the standard v4l2 ctrl like this?

Thanks for the inputs.

On 30 March 2018 at 12:58, Hans Verkuil  wrote:
> On 30/03/18 09:23, asadpt iqroot wrote:
>> Hi Hans,
>>
>> Thanks for the reply.
>>
>> In HDMI receivers, when we need to use this control. What scenario?
>
> https://www.linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/extended-controls.html#digital-video-control-reference
>
> "Detects whether the receiver receives power from the source (e.g. HDMI 
> carries 5V on one of the pins)."
>
> Regards,
>
> Hans
>
>>
>> -Thanks.
>>
>>
>> On 30 March 2018 at 12:13, Hans Verkuil  wrote:
>>> On 30/03/18 08:16, asadpt iqroot wrote:
 Hi All,

 In reference sensor drivers, they used the
 V4L2_CID_DV_RX_POWER_PRESENT v4l2 ctrl.
 It is a standard ctrl and created using v4l2_ctrl_new_std().

 The doubts are:

 1. Whether in our sensor driver, we need to create this Control Id or
 not. How to take the decision on this. Since this is the standard
 ctrl. When we need to use these standard ctrls??
>>>
>>> No. This control is for HDMI receivers, not for sensors.
>>>
>>> Regards,
>>>
>>> Hans
>>>

 2. In Sensor driver, the ctrls creation is anything depends on the
 bridge driver.
 Based on bridge driver, whether we need to create any ctrls in Sensor 
 driver.

 This question belongs to design of the sensor driver.



 Thanks & Regards

>>>
>