cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

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

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Steve Longerbeam]

On 03/20/2017 10:23 AM, Philipp Zabel wrote:

Hi Steve, Russell,

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:

On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:

The same document says:

  Scaling support is optional. When supported by a subdev, the crop
  rectangle on the subdev's sink pad is scaled to the size configured
  using the
  :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
  using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
  subdev supports scaling but not composing, the top and left values are
  not used and must always be set to zero.


Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.

I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.


That is what I believe having read these documents several times, but
we need v4l2 people to confirm.


What do you think of this:

--8<--
From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 20 Mar 2017 17:10:21 +0100
Subject: [PATCH] media: imx: csi: add sink selection rectangles

Move the crop rectangle to the sink pad and add a sink compose rectangle
to configure scaling. Also propagate rectangles from sink pad to crop
rectangle, to compose rectangle, and to the source pads both in ACTIVE
and TRY variants of set_fmt/selection, and initialize the default crop
and compose rectangles.



I haven't had a chance to look at this patch in detail yet, but on
first glance it looks good to me. I'll try to find the time tomorrow
to incorporate it and then fixup Russell's subsequent patches for
the enum frame sizes/intervals.

Steve


Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-csi.c | 216 +-
 1 file changed, 152 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 560da3abdd70b..b026a5d602ddf 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -79,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+   struct v4l2_rect compose;
const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];

/* active vb2 buffers to send to video dev sink */
@@ -574,8 +575,8 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_window(priv->csi, >crop);

ipu_csi_set_downsize(priv->csi,
-priv->crop.width == 2 * outfmt->width,
-priv->crop.height == 2 * outfmt->height);
+priv->crop.width == 2 * priv->compose.width,
+priv->crop.height == 2 * priv->compose.height);

ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt);

@@ -1001,6 +1002,27 @@ __csi_get_fmt(struct csi_priv *priv, struct 
v4l2_subdev_pad_config *cfg,
return >format_mbus[pad];
 }

+static struct v4l2_rect *
+__csi_get_crop(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+  enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_crop(>sd, cfg, CSI_SINK_PAD);
+   else
+   return >crop;
+}
+
+static struct v4l2_rect *
+__csi_get_compose(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_compose(>sd, cfg,
+  CSI_SINK_PAD);
+   else
+   return >compose;
+}
+
 static void csi_try_crop(struct csi_priv *priv,
 struct v4l2_rect *crop,
 struct v4l2_subdev_pad_config *cfg,
@@ -1159,6 +1181,7 @@ static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
+   struct v4l2_rect *compose,
const struct imx_media_pixfmt **cc)
 {
const struct imx_media_pixfmt *incc;
@@ -1173,15 +1196,8 @@ static void 

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[kbuild test robot]

Hi unknown,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.11-rc3 next-20170320]
[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/unknown/staging-radio-bcm2048-Fix-checkpatch-warnings-WARNING-Prefer-unsigned-int-to-bare-use-of-unsigned/20170321-105721
base:   git://linuxtv.org/media_tree.git master
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/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=xtensa 

All errors (new ones prefixed by >>):

   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_power_state_write':
>> drivers/staging/media/bcm2048/radio-bcm2048.c:2031:50: error: two or more 
>> data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0)
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2031:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_mute_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2032:43: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0)
  ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2032:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_audio_route_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2033:50: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0)
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2033:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_dac_output_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2034:49: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2034:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_hi_lo_injection_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2036:57: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2036:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_frequency_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2037:51: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0)
  ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2037:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_af_frequency_write':
   drivers/staging/media/bcm2048/radio-bcm2048

[PATCH 4/4 V2] staging: atomisp: remove redudant condition in if-statement

[Daeseok Youn]

The V4L2_FIELD_ANY is zero, so the (!field) is same meaning
with (field == V4L2_FIELD_ANY) in if-statement.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index f7c0705..943a7ae 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -5084,7 +5084,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
 
depth = get_pixel_depth(pixelformat);
 
-   if (!field || field == V4L2_FIELD_ANY)
+   if (field == V4L2_FIELD_ANY)
field = V4L2_FIELD_NONE;
else if (field != V4L2_FIELD_NONE) {
dev_err(isp->dev, "Wrong output field\n");
-- 
1.9.1

[PATCH 3/4 V2] staging: atomisp: remove useless condition in if-statements

[Daeseok Youn]

The css_pipe_id was checked with 'CSS_PIPE_ID_COPY' in previous if-
statement. In this case, if the css_pipe_id equals to 'CSS_PIPE_ID_COPY',
it could not enter the next if-statement. But the "next" if-statement
has the condition to check whether the css_pipe_id equals to
'CSS_PIPE_ID_COPY' or not. It should be removed.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 811331d..f7c0705 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -910,8 +910,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
/* For online video or SDV video pipe. */
if (css_pipe_id == CSS_PIPE_ID_VIDEO ||
-   css_pipe_id == CSS_PIPE_ID_COPY ||
-   css_pipe_id == CSS_PIPE_ID_YUVPP) {
+   css_pipe_id == CSS_PIPE_ID_COPY) {
if (buf_type == CSS_BUFFER_TYPE_OUTPUT_FRAME)
return >video_out_video_capture;
return >video_out_preview;
@@ -919,8 +918,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW) {
/* For online preview or ZSL preview pipe. */
if (css_pipe_id == CSS_PIPE_ID_PREVIEW ||
-   css_pipe_id == CSS_PIPE_ID_COPY ||
-   css_pipe_id == CSS_PIPE_ID_YUVPP)
+   css_pipe_id == CSS_PIPE_ID_COPY)
return >video_out_preview;
}
/* For capture pipe. */
-- 
1.9.1

[PATCH 2/4 V2] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[Daeseok Youn]

If v4l2_subdev_call() gets the global frame interval values,
it returned 0 and it could be checked whether numerator is zero or not.

If the numerator is not zero, the fps could be calculated in this function.
If not, it just returns 0.

Signed-off-by: Daeseok Youn 
---
V2: split error handling, the first check is for the result from 
v4l2_subdev_call(),
another check is for fi.interval.numerator > 0. it is more understandable 
and
simpler than before.

 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 24 ++
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 8bdb224..811331d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -153,21 +153,19 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
video_device *dev)
 
 static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
 {
-   struct v4l2_subdev_frame_interval frame_interval;
+   struct v4l2_subdev_frame_interval fi;
struct atomisp_device *isp = asd->isp;
-   unsigned short fps;
 
-   if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-   video, g_frame_interval, _interval)) {
-   fps = 0;
-   } else {
-   if (frame_interval.interval.numerator)
-   fps = frame_interval.interval.denominator /
-   frame_interval.interval.numerator;
-   else
-   fps = 0;
-   }
-   return fps;
+   int ret;
+
+   ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+  video, g_frame_interval, );
+
+   if (ret)
+   return 0;
+
+   return (fi.interval.numerator > 0) ?
+  (fi.interval.denominator / fi.interval.numerator) : 0;
 }
 
 /*
-- 
1.9.1

[PATCH 1/4 V2] staging: atomisp: remove else statement after return

[Daeseok Youn]

It doesn't need to have else statement after return.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d97a8df..8bdb224 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2958,11 +2958,11 @@ int atomisp_get_metadata(struct atomisp_sub_device 
*asd, int flag,
dev_err(isp->dev, "copy to user failed: copied %d bytes\n",
ret);
return -EFAULT;
-   } else {
-   list_del_init(_buf->list);
-   list_add_tail(_buf->list, >metadata[md_type]);
}
 
+   list_del_init(_buf->list);
+   list_add_tail(_buf->list, >metadata[md_type]);
+
dev_dbg(isp->dev, "%s: HAL de-queued metadata type %d with exp_id %d\n",
__func__, md_type, md->exp_id);
return 0;
-- 
1.9.1

Re: [PATCH 2/2] [media] vcodec: mediatek: mark pm functions as __maybe_unused

[Rick Chang]

On Mon, 2017-03-20 at 10:47 +0100, Arnd Bergmann wrote:
> When CONFIG_PM is disabled, we get a couple of unused functions:
> 
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c:927:13: error: 
> 'mtk_jpeg_clk_off' defined but not used [-Werror=unused-function]
>  static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg)
>  ^~~~
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c:916:13: error: 
> 'mtk_jpeg_clk_on' defined but not used [-Werror=unused-function]
>  static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> 
> Rather than adding more error-prone #ifdefs around those, this patch
> removes the existing #ifdef checks and marks the PM functions as 
> __maybe_unused
> to let gcc do the right thing.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)

Acked-by: Rick Chang 

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[DaeSeok Youn]

2017-03-20 22:11 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 13:51, schrieb DaeSeok Youn:
>> 2017-03-20 21:04 GMT+09:00 walter harms :
>>>
>>>
>>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
 If v4l2_subdev_call() gets the global frame interval values,
 it returned 0 and it could be checked whether numerator is zero or not.

 If the numerator is not zero, the fps could be calculated in this function.
 If not, it just returns 0.

 Signed-off-by: Daeseok Youn 
 ---
  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
 ++
  1 file changed, 10 insertions(+), 12 deletions(-)

 diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
 b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 index 8bdb224..6bdd19e 100644
 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
 video_device *dev)

  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device 
 *asd)
  {
 - struct v4l2_subdev_frame_interval frame_interval;
 + struct v4l2_subdev_frame_interval fi;
   struct atomisp_device *isp = asd->isp;
 - unsigned short fps;

 - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
 - video, g_frame_interval, _interval)) {
 - fps = 0;
 - } else {
 - if (frame_interval.interval.numerator)
 - fps = frame_interval.interval.denominator /
 - frame_interval.interval.numerator;
 - else
 - fps = 0;
 - }
 + unsigned short fps = 0;
 + int ret;
 +
 + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
 +video, g_frame_interval, );
 +
 + if (!ret && fi.interval.numerator)
 + fps = fi.interval.denominator / fi.interval.numerator;
 +
   return fps;
  }
>>>
>>>
>>>
>>> do you need to check ret at all ? if an error occurs can 
>>> fi.interval.numerator
>>> be something else than 0 ?
>> the return value from the v4l2_subdev_call() function is zero when it
>> is done without any error. and also I checked
>> the ret value whether is 0 or not. if the ret is 0 then the value of
>> numerator should be checked to avoid for dividing by 0.
>>>
>>> if ret is an ERRNO it would be wise to return ret not fps, but this may 
>>> require
>>> changes at other places also.
>> hmm.., yes, you are right. but I think it is ok because the
>> atomisp_get_sensor_fps() function is needed to get fps value.
>> (originally, zero or calculated fps value was returned.)
>
> maybe its better to divide this in:
> if (ret)
>return 0; // error case
>
> return (fi.interval.numerator>0)?fi.interval.denominator / 
> fi.interval.numerator:0;
>
> So there is a chance that someone will a) understand and b) fix the error 
> return.
yes, it looks better than mine. I will update it and resend it.

Thanks walter,
Regards,
Daeseok Youn.

>
> re,
>  wh
>
>>
>>>
>>> re,
>>>  wh
>>>

>>

Re: [PATCH 1/2] [media] vcodec: mediatek: add missing linux/slab.h include

[Rick Chang]

Hi Arnd,

Thank you for the patch, but someone has fixed the same problem.

Regards,
Rick

On Mon, 2017-03-20 at 10:47 +0100, Arnd Bergmann wrote:
> With the newly added driver, I have run into randconfig failures like:
> 
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c: In function 'mtk_jpeg_open':
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c:1017:8: error: implicit 
> declaration of function 'kzalloc';did you mean 'kvzalloc'? 
> [-Werror=implicit-function-declaration]
> 
> Including the header with the declaration solves the problem.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c 
> b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index b10183f7942b..f9bd58ce7d32 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 

Reply Urgently.

[JOJO AKPE]

I am Barr, Jojo Akpe,  esq, a personal attorney to your late relative.
I want to contact you to work with me in securing the transfer of
fund(US$10,000,000.00 M) legacy by your late relative. I solicit your
consent to enable me produce you as the Next of Kin to my deceased
client since both of you bear the same last name.

The funds will then be transferred to you as the beneficiary and
shared according to a proposed sharing pattern / ratio of 50:50
i.e.50% for me and 50% for you. For more details, send me your private
email address and do contact me with your data through my private
email address (barrjojoak...@gmail.com)

Please kindly call me on 0022897981669 after reading this message.
I wait for your urgent  response

Barr,Jojo Akpe
0022897981669

Re: [PATCH] [media] dvb-frontends/cxd2841er: define symbol_rate_min/max in T/C fe-ops

[Abylay Ospan]

Hello,

looks good for me.
Acked-by: Abylay Ospan 

2017-03-19 11:26 GMT-04:00 Daniel Scheller :
> From: Daniel Scheller 
>
> Fixes "w_scan -f c" complaining with
>
>   This dvb driver is *buggy*: the symbol rate limits are undefined - please
>   report to linuxtv.org)
>
> Signed-off-by: Daniel Scheller 
> ---
>  drivers/media/dvb-frontends/cxd2841er.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
> b/drivers/media/dvb-frontends/cxd2841er.c
> index 614bfb3..ce37dc2 100644
> --- a/drivers/media/dvb-frontends/cxd2841er.c
> +++ b/drivers/media/dvb-frontends/cxd2841er.c
> @@ -3852,7 +3852,9 @@ static struct dvb_frontend_ops cxd2841er_t_c_ops = {
> FE_CAN_MUTE_TS |
> FE_CAN_2G_MODULATION,
> .frequency_min = 4200,
> -   .frequency_max = 100200
> +   .frequency_max = 100200,
> +   .symbol_rate_min = 87,
> +   .symbol_rate_max = 1170
> },
> .init = cxd2841er_init_tc,
> .sleep = cxd2841er_sleep_tc,
> --
> 2.10.2
>



-- 
Abylay Ospan,
NetUP Inc.
http://www.netup.tv

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 06:23:24PM +0100, Philipp Zabel wrote:
> --8<--
> >From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
> From: Philipp Zabel 
> Date: Mon, 20 Mar 2017 17:10:21 +0100
> Subject: [PATCH] media: imx: csi: add sink selection rectangles
> 
> Move the crop rectangle to the sink pad and add a sink compose rectangle
> to configure scaling. Also propagate rectangles from sink pad to crop
> rectangle, to compose rectangle, and to the source pads both in ACTIVE
> and TRY variants of set_fmt/selection, and initialize the default crop
> and compose rectangles.

Looks fine for the most part.

> - /*
> -  * Modifying the crop rectangle always changes the format on the source
> -  * pad. If the KEEP_CONFIG flag is set, just return the current crop
> -  * rectangle.
> -  */
> - if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> - sel->r = priv->crop;
> - if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> - cfg->try_crop = sel->r;
> + infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sel->which);
> + crop = __csi_get_crop(priv, cfg, sel->which);
> + compose = __csi_get_compose(priv, cfg, sel->which);
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP:
> + /*
> +  * Modifying the crop rectangle always changes the format on
> +  * the source pads. If the KEEP_CONFIG flag is set, just return
> +  * the current crop rectangle.
> +  */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->crop;

My understanding of KEEP_CONFIG is that the only thing we're not
allowed to do is to propagate the change downstream.

Since downstream of the crop is the compose, that means the only
restriction here is that the width and height of the crop window must
be either equal to the compose width/height, or double the compose
width/height.  (Anything else would necessitate the compose window
changing.)

However, the crop window can move position within the crop bounds,
provided it's entirely contained within those crop bounds.

The problem is that this becomes rather more complex it deal with
(as I'm finding out in my imx219 camera driver) and I'm thinking
that some of this complexity should probably be in a helper in
generic v4l2 code.

I don't know whether this applies (I hope it doesn't) but there's a
pile of guidelines in Documentation/media/uapi/v4l/vidioc-g-selection.rst
which describe how a crop/compose rectangle should be adjusted.  As
I say, I hope they don't apply, because if they do, we would _really_
need helpers for this stuff, as I don't think having each driver
implement all these rules would be too successful!

> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + *crop = sel->r;
> + goto out;
> + }
> +
> + csi_try_crop(priv, >r, cfg, infmt, sensor);
> +
> + *crop = sel->r;
> +
> + /* Reset scaling to 1:1 */
> + compose->width = crop->width;
> + compose->height = crop->height;
> + break;
> + case V4L2_SEL_TGT_COMPOSE:
> + /*
> +  * Modifying the compose rectangle always changes the format on
> +  * the source pads. If the KEEP_CONFIG flag is set, just return
> +  * the current compose rectangle.
> +  */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->compose;

I think, with my understanding of how the KEEP_CONFIG flag works, this
should be:
sel->r = *compose;

because if we change the compose rectangle width/height, we would need
to propagate this to the source pad, and the KEEP_CONFIG description
says we're not allowed to do that.

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Steve Longerbeam]

On 03/20/2017 07:00 AM, Philipp Zabel wrote:

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:

On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:

The above paragraph suggests we skip any rectangles that are not
supported. In our case that would be 3. and 4., since the CSI can't
compose into a larger frame. I hadn't realised that the crop selection
currently happens on the source pad.

I'd recommend viewing the documentation in its post-processed version,
because then you get the examples as pictures, and they say that a
picture is worth 1000 words.  See

   https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html

There is almost an exact example of what we're trying to do - it's
figure 4.6.  Here, we have a sink pad with a cropping rectangle on
the input, which is then scaled to a composition rectangle (there's
no bounds rectangle, and it's specified that in such a case the
top,left of the composition rectangle will always be 0,0 - see quote
below).

Where it differs is that the example also supports source cropping
for two source pads.  We don't support that.

The same document says:

   Scaling support is optional. When supported by a subdev, the crop
   rectangle on the subdev's sink pad is scaled to the size configured
   using the
   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
   subdev supports scaling but not composing, the top and left values are
   not used and must always be set to zero.

Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.


Ok, this all makes consistent sense to me too. So:

- the CSI hardware cropping rectangle should be specified via the
  sink pad crop selection.

- the CSI hardware /2 downscaler should be specified via the
  sink pad compose selection.

- the final source pad rectangle is the same as the sink pad
  compose rectangle.

So that leaves only step 4 (source pad crop selection) as
unsupported.

Steve



I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.

regards
Philipp

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 06:40:21PM +0100, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> > I have tripped over a bug in media-ctl when specifying both a crop and
> > compose rectangle - the --help output suggests that "," should be used
> > to separate them.  media-ctl rejects that, telling me the character at
> > the "," should be "]".  Replacing the "," with " " allows media-ctl to
> > accept it and set both rectangles, so it sounds like a parser bug - I've
> > not looked into this any further yet.
> 
> I can confirm this. I don't see any place in
> v4l2_subdev_parse_pad_format that handles the "," separator. There's
> just whitespace skipping between the v4l2-properties.

Maybe this is the easiest solution:

 utils/media-ctl/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 83ca1ca..8b97874 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -65,7 +65,7 @@ static void usage(const char *argv0)
printf("\tentity  = entity-number | ( '\"' entity-name '\"' ) 
;\n");
printf("\n");
printf("\tv4l2= pad '[' v4l2-properties ']' ;\n");
-   printf("\tv4l2-properties = v4l2-property { ',' v4l2-property } ;\n");
+   printf("\tv4l2-properties = v4l2-property { ' '* v4l2-property } ;\n");
printf("\tv4l2-property   = v4l2-mbusfmt | v4l2-crop | 
v4l2-interval\n");
printf("\t| v4l2-compose | v4l2-interval ;\n");
printf("\tv4l2-mbusfmt= 'fmt:' fcc '/' size ; { 'field:' v4l2-field 
; } { 'colorspace:' v4l2-colorspace ; }\n");

;)

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.
> 
> Note that setting the format on 'ipu1_csi0':0 should already be done by
> the previous media-ctl command, so it should be possible to simplify
> that to:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Thanks, that works, too.

> I have tripped over a bug in media-ctl when specifying both a crop and
> compose rectangle - the --help output suggests that "," should be used
> to separate them.  media-ctl rejects that, telling me the character at
> the "," should be "]".  Replacing the "," with " " allows media-ctl to
> accept it and set both rectangles, so it sounds like a parser bug - I've
> not looked into this any further yet.

I can confirm this. I don't see any place in
v4l2_subdev_parse_pad_format that handles the "," separator. There's
just whitespace skipping between the v4l2-properties.

regards
Philipp

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

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 16:10:03 +
Russell King - ARM Linux  escreveu:

> On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 20 Mar 2017 14:24:25 +0100
> > Hans Verkuil  escreveu:  
> > > I don't think this control inheritance patch will magically prevent you
> > > from needed a plugin.  
> > 
> > Yeah, it is not just control inheritance. The driver needs to work
> > without subdev API, e. g. mbus settings should also be done via the
> > video devnode.
> > 
> > Btw, Sakari made a good point on IRC: what happens if some app 
> > try to change the pipeline or subdev settings while another
> > application is using the device? The driver should block such 
> > changes, maybe using the V4L2 priority mechanism.  
> 
> My understanding is that there are already mechanisms in place to
> prevent that, but it's driver dependent - certainly several of the
> imx driver subdevs check whether they have an active stream, and
> refuse (eg) all set_fmt calls with -EBUSY if that is so.
> 
> (That statement raises another question in my mind: if the subdev is
> streaming, should it refuse all set_fmt, even for the TRY stuff?)

Returning -EBUSY only when streaming is too late, as ioctl's
may be changing the pipeline configuration and/or buffer allocation,
while the application is sending other ioctls in order to prepare
for streaming.

V4L2 has a mechanism of blocking other apps to change such
parameters via VIDIOC_S_PRIORITY[1]. If an application sets
priority to V4L2_PRIORITY_RECORD, any other application attempting
to change the device via some other file descriptor will fail.
So, it is a sort of "exclusive write access".

On a quick look at V4L2 core, currently, sending a 
VIDIOC_S_PRIORITY ioctl to a /dev/video device doesn't seem to have
any effect at either MC or V4L2 subdev API for the subdevs connected
to it. We'll likely need to add some code at v4l2_prio_change()
for it to notify the subdevs for them to return -EBUSY if one
would try to change something there, while the device is priorized.

[1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-priority.html

> > In parallel, someone has to fix libv4l for it to be default on
> > applications like gstreamer, e. g. adding support for DMABUF
> > and fixing other issues that are preventing it to be used by
> > default.  
> 
> Hmm, not sure what you mean there - I've used dmabuf with gstreamer's
> v4l2src linked to libv4l2, importing the buffers into etnaviv using
> a custom plugin.  There are distros around (ubuntu) where the v4l2
> plugin is built against libv4l2.

Hmm... I guess some gst developer mentioned that there are/where
some restrictions at libv4l2 with regards to DMABUF. I may be
wrong.

> 
> > My understanding here is that there are developers wanting/needing
> > to have standard V4L2 apps support for (some) i.MX6 devices. Those are
> > the ones that may/will allocate some time for it to happen.  
> 
> Quite - but we need to first know what is acceptable to the v4l2
> community before we waste a lot of effort coding something up that
> may not be suitable.  Like everyone else, there's only a limited
> amount of effort available, so using it wisely is a very good idea.

Sure. That's why we're discussing here :-)

> Up until recently, it seemed that the only solution was to solve the
> userspace side of the media controller API via v4l2 plugins and the
> like.
> 
> Much of my time that I have available to look at the imx6 capture
> stuff at the moment is taken up by triping over UAPI issues with the
> current code (such as the ones about CSI scaling, colorimetry, etc)
> and trying to get concensus on what the right solution to fix those
> issues actually is, and at the moment I don't have spare time to
> start addressing any kind of v4l2 plugin for user controls nor any
> other solution.

I hear you. A solution via libv4l could be more elegant, but it
doesn't seem simple, as nobody did it before, and depends on the
libv4l plugin mechanism, with is currently unused.

Also, I think it is easier to provide a solution using DT and some
driver and/or core support for it, specially since, AFAICT,
currently there's no way request exclusive access to MC and subdevs.

It is probably not possible do to that exclusively in userspace.

> Eg, I spent much of this last weekend sorting out my IMX219 camera
> sensor driver for my new understanding about how scaling is supposed
> to work, the frame enumeration issue (which I've posted patches for)
> and the CSI scaling issue (which I've some half-baked patch for at the
> moment, but probably by the time I've finished sorting that, Philipp
> or Steve will already have a solution.)
> 
> That said, my new understanding of the scaling impacts the four patches
> I posted, and probably makes the frame size enumeration in CSI (due to
> its scaling) rather obsolete.

Yeah, when there's no scaler, it should report just the resolution(s)
supported by 

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

Hi Steve, Russell,

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.

What do you think of this:

--8<--
>From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 20 Mar 2017 17:10:21 +0100
Subject: [PATCH] media: imx: csi: add sink selection rectangles

Move the crop rectangle to the sink pad and add a sink compose rectangle
to configure scaling. Also propagate rectangles from sink pad to crop
rectangle, to compose rectangle, and to the source pads both in ACTIVE
and TRY variants of set_fmt/selection, and initialize the default crop
and compose rectangles.

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-csi.c | 216 +-
 1 file changed, 152 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 560da3abdd70b..b026a5d602ddf 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -79,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+   struct v4l2_rect compose;
const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];
 
/* active vb2 buffers to send to video dev sink */
@@ -574,8 +575,8 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_window(priv->csi, >crop);
 
ipu_csi_set_downsize(priv->csi,
-priv->crop.width == 2 * outfmt->width,
-priv->crop.height == 2 * outfmt->height);
+priv->crop.width == 2 * priv->compose.width,
+priv->crop.height == 2 * priv->compose.height);
 
ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt);
 
@@ -1001,6 +1002,27 @@ __csi_get_fmt(struct csi_priv *priv, struct 
v4l2_subdev_pad_config *cfg,
return >format_mbus[pad];
 }
 
+static struct v4l2_rect *
+__csi_get_crop(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+  enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_crop(>sd, cfg, CSI_SINK_PAD);
+   else
+   return >crop;
+}
+
+static struct v4l2_rect *
+__csi_get_compose(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_compose(>sd, cfg,
+  CSI_SINK_PAD);
+   else
+   return >compose;
+}
+
 static void csi_try_crop(struct csi_priv *priv,
 struct v4l2_rect *crop,
 struct v4l2_subdev_pad_config *cfg,
@@ -1159,6 +1181,7 @@ static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
+   struct v4l2_rect *compose,
const struct imx_media_pixfmt **cc)
 {
const struct imx_media_pixfmt *incc;
@@ -1173,15 +1196,8 @@ static void csi_try_fmt(struct csi_priv *priv,
incc = imx_media_find_mbus_format(infmt->code,
  CS_SEL_ANY, true);
 

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:17:05PM +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.
> 
> Note that setting the format on 'ipu1_csi0':0 should already be done by
> the previous media-ctl command, so it should be possible to simplify
> that to:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

There is a slightly puzzling bit in the documentation.  If we consider
the CSI, and the sink compose rectangle size has to always match the
the source output pad format size (since in hardware they are one of
the same), then:

  Inside subdevs, the order of image processing steps will always be from
  the sink pad towards the source pad. This is also reflected in the order
  in which the configuration must be performed by the user: the changes
  made will be propagated to any subsequent stages. If this behaviour is
  not desired, the user must set ``V4L2_SEL_FLAG_KEEP_CONFIG`` flag. This
 
  flag causes no propagation of the changes are allowed in any
  
  circumstances. This may also cause the accessed rectangle to be adjusted
  
  by the driver, depending on the properties of the underlying hardware.
  ^^

presumably, this means if we get a request to change the source compose
rectangle with V4L2_SEL_FLAG_KEEP_CONFIG set, we need to force its size
to be the current output format size.  I don't think we can do anything
else - because the above makes it very clear that any following stages
shall not be updated.  The last sentence appears to give permission to
do that.

This also has implications when changing the sink crop - the sink crop
(eg) must not be smaller than the sink compose, as we don't support
scaling up in CSI.

It seems to me that V4L2_SEL_FLAG_KEEP_CONFIG in practice changes the
way validation of the request works.  So, rather than validating the
request against the upstream rectangle and propagating downstream, it
needs to be validated against both the upstream and downstream
rectangles instead.

It seems there's many subtleties to this...

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

Re: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Rob Herring]

On Mon, Mar 20, 2017 at 11:49 AM, Hans Verkuil  wrote:
> On 03/20/2017 05:41 PM, Rob Herring wrote:
>> On Sat, Mar 11, 2017 at 12:23:18PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil 

>>> +ov2640: camera@30 {
>>> +compatible = "ovti,ov2640";
>>> +reg = <0x30>;
>>> +pinctrl-names = "default";
>>> +pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
>>> _sensor_reset>;
>>> +resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
>>
>> reset-gpios?
>>
>>> +pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
>>
>> powerdown-gpios?
>
> I can't change this: these two properties have been in use for a long time 
> for the ov2640
> driver.

NM. I was thinking this was the ov7670 you just added.

Rob

Re: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Hans Verkuil]

On 03/20/2017 05:41 PM, Rob Herring wrote:
> On Sat, Mar 11, 2017 at 12:23:18PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Document the device tree bindings for this hardware.
>>
>> Mostly copied from the atmel-isc bindings.
> 
> This commit message doesn't really reflect what you are doing and the 
> reformatting and fixes really make this a PIA to review.

I'll improve the commit log.

> 
>>
>> Signed-off-by: Hans Verkuil 
>> Acked-by: Sakari Ailus 
>> ---
>>  .../devicetree/bindings/media/atmel-isi.txt| 96 
>> +-
>>  1 file changed, 58 insertions(+), 38 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> index 251f008f220c..65249bbd5c00 100644
>> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -1,51 +1,71 @@
>> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> ---
>> -
>> -Required properties:
>> -- compatible: must be "atmel,at91sam9g45-isi"
>> -- reg: physical base address and length of the registers set for the device;
>> -- interrupts: should contain IRQ line for the ISI;
>> -- clocks: list of clock specifiers, corresponding to entries in
>> -  the clock-names property;
>> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> -
>> -ISI supports a single port node with parallel bus. It should contain one
>> +Atmel Image Sensor Interface (ISI)
>> +--
>> +
>> +Required properties for ISI:
>> +- compatible: must be "atmel,at91sam9g45-isi".
>> +- reg: physical base address and length of the registers set for the device.
>> +- interrupts: should contain IRQ line for the ISI.
>> +- clocks: list of clock specifiers, corresponding to entries in the 
>> clock-names
>> +property; please refer to clock-bindings.txt.
>> +- clock-names: required elements: "isi_clk".
>> +- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
>> +
>> +ISI supports a single port node with parallel bus. It shall contain one
>>  'port' child node with child 'endpoint' node. Please refer to the bindings
>>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>>  
>> -Example:
>> -isi: isi@f0034000 {
>> -compatible = "atmel,at91sam9g45-isi";
>> -reg = <0xf0034000 0x4000>;
>> -interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +Endpoint node properties
>> +
>>  
>> -clocks = <_clk>;
>> -clock-names = "isi_clk";
>> +- bus-width: <8> or <10> (mandatory)
>> +- hsync-active (default: active high)
>> +- vsync-active (default: active high)
>> +- pclk-sample (default: sample on falling edge)
>> +- remote-endpoint: A phandle to the bus receiver's endpoint node 
>> (mandatory).
>>  
>> -pinctrl-names = "default";
>> -pinctrl-0 = <_isi>;
>> -
>> -port {
>> -#address-cells = <1>;
>> -#size-cells = <0>;
>> +Example:
>>  
>> -isi_0: endpoint {
>> -remote-endpoint = <_0>;
>> -bus-width = <8>;
>> -};
>> +isi: isi@f0034000 {
>> +compatible = "atmel,at91sam9g45-isi";
>> +reg = <0xf0034000 0x4000>;
>> +interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_isi_data_0_7>;
>> +clocks = <_clk>;
>> +clock-names = "isi_clk";
>> +status = "ok";
> 
> Don't put status in examples.

Will remove.

> 
>> +port {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
> 
> These can be dropped.

Ack.

> 
>> +isi_0: endpoint {
>> +remote-endpoint = <_0>;
>> +bus-width = <8>;
>> +vsync-active = <1>;
>> +hsync-active = <1>;
>>  };
>>  };
>> +};
>> +
>> +i2c1: i2c@f0018000 {
>> +status = "okay";
>>  
>> -i2c1: i2c@f0018000 {
>> -ov2640: camera@0x30 {
>> -compatible = "ovti,ov2640";
>> -reg = <0x30>;
>> +ov2640: camera@30 {
>> +compatible = "ovti,ov2640";
>> +reg = <0x30>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
>> _sensor_reset>;
>> +resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
> 
> reset-gpios?
> 
>> +pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
> 
> powerdown-gpios?

I can't change this: these two properties have been in use for a long time for 
the ov2640
driver.

Regards,

Hans

> 
>> +clocks = <>;
>> +clock-names = "xvclk";
>> +assigned-clocks = <>;
>> +

Re: [PATCHv5 06/16] atmel-isi: document device tree bindings

[Rob Herring]

On Sat, Mar 11, 2017 at 12:23:18PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Document the device tree bindings for this hardware.
> 
> Mostly copied from the atmel-isc bindings.

This commit message doesn't really reflect what you are doing and the 
reformatting and fixes really make this a PIA to review.

>
> Signed-off-by: Hans Verkuil 
> Acked-by: Sakari Ailus 
> ---
>  .../devicetree/bindings/media/atmel-isi.txt| 96 
> +-
>  1 file changed, 58 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
> b/Documentation/devicetree/bindings/media/atmel-isi.txt
> index 251f008f220c..65249bbd5c00 100644
> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> @@ -1,51 +1,71 @@
> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> ---
> -
> -Required properties:
> -- compatible: must be "atmel,at91sam9g45-isi"
> -- reg: physical base address and length of the registers set for the device;
> -- interrupts: should contain IRQ line for the ISI;
> -- clocks: list of clock specifiers, corresponding to entries in
> -  the clock-names property;
> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
> -
> -ISI supports a single port node with parallel bus. It should contain one
> +Atmel Image Sensor Interface (ISI)
> +--
> +
> +Required properties for ISI:
> +- compatible: must be "atmel,at91sam9g45-isi".
> +- reg: physical base address and length of the registers set for the device.
> +- interrupts: should contain IRQ line for the ISI.
> +- clocks: list of clock specifiers, corresponding to entries in the 
> clock-names
> + property; please refer to clock-bindings.txt.
> +- clock-names: required elements: "isi_clk".
> +- pinctrl-names, pinctrl-0: please refer to pinctrl-bindings.txt.
> +
> +ISI supports a single port node with parallel bus. It shall contain one
>  'port' child node with child 'endpoint' node. Please refer to the bindings
>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>  
> -Example:
> - isi: isi@f0034000 {
> - compatible = "atmel,at91sam9g45-isi";
> - reg = <0xf0034000 0x4000>;
> - interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> +Endpoint node properties
> +
>  
> - clocks = <_clk>;
> - clock-names = "isi_clk";
> +- bus-width: <8> or <10> (mandatory)
> +- hsync-active (default: active high)
> +- vsync-active (default: active high)
> +- pclk-sample (default: sample on falling edge)
> +- remote-endpoint: A phandle to the bus receiver's endpoint node (mandatory).
>  
> - pinctrl-names = "default";
> - pinctrl-0 = <_isi>;
> -
> - port {
> - #address-cells = <1>;
> - #size-cells = <0>;
> +Example:
>  
> - isi_0: endpoint {
> - remote-endpoint = <_0>;
> - bus-width = <8>;
> - };
> +isi: isi@f0034000 {
> + compatible = "atmel,at91sam9g45-isi";
> + reg = <0xf0034000 0x4000>;
> + interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_isi_data_0_7>;
> + clocks = <_clk>;
> + clock-names = "isi_clk";
> + status = "ok";

Don't put status in examples.

> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;

These can be dropped.

> + isi_0: endpoint {
> + remote-endpoint = <_0>;
> + bus-width = <8>;
> + vsync-active = <1>;
> + hsync-active = <1>;
>   };
>   };
> +};
> +
> +i2c1: i2c@f0018000 {
> + status = "okay";
>  
> - i2c1: i2c@f0018000 {
> - ov2640: camera@0x30 {
> - compatible = "ovti,ov2640";
> - reg = <0x30>;
> + ov2640: camera@30 {
> + compatible = "ovti,ov2640";
> + reg = <0x30>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
> _sensor_reset>;
> + resetb-gpios = < 11 GPIO_ACTIVE_LOW>;

reset-gpios?

> + pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;

powerdown-gpios?

> + clocks = <>;
> + clock-names = "xvclk";
> + assigned-clocks = <>;
> + assigned-clock-rates = <2500>;
>  
> - port {
> - ov2640_0: endpoint {
> - remote-endpoint = <_0>;
> - bus-width = <8>;
> - };
> + port {
> + ov2640_0: 

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

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 05:29:07PM +0100, Philipp Zabel wrote:
> According to the documentation [1], you are doing the right thing:
> 
> The struct v4l2_subdev_frame_interval pad references a non-existing
> pad, or the pad doesn’t support frame intervals.
> 
> But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
> not implemented at all, which is turned into -ENOTTY by video_usercopy.
> 
> [1] 
> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value

Thanks for confirming.

> > Maybe something like the following would be a better idea?
> > 
> >  utils/media-ctl/media-ctl.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> > index f61963a..a50a559 100644
> > --- a/utils/media-ctl/media-ctl.c
> > +++ b/utils/media-ctl/media-ctl.c
> > @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct 
> > media_entity *entity,
> > struct v4l2_mbus_framefmt format;
> > struct v4l2_fract interval = { 0, 0 };
> > struct v4l2_rect rect;
> > -   int ret;
> > +   int ret, err_fi;
> >  
> > ret = v4l2_subdev_get_format(entity, , pad, which);
> > if (ret != 0)
> > return;
> >  
> > -   ret = v4l2_subdev_get_frame_interval(entity, , pad);
> > -   if (ret != 0 && ret != -ENOTTY)
> > -   return;
> > +   err_fi = v4l2_subdev_get_frame_interval(entity, , pad);
> 
> Not supporting frame intervals doesn't warrant a visible error message,
> I think -EINVAL should also be ignored above, if the spec is to be
> believed.
> 
> >  
> > printf("\t\t[fmt:%s/%ux%u",
> >v4l2_subdev_pixelcode_to_string(format.code),
> >format.width, format.height);
> >  
> > -   if (interval.numerator || interval.denominator)
> > +   if (err_fi == 0 && (interval.numerator || interval.denominator))
> > printf("@%u/%u", interval.numerator, interval.denominator);
> > +   else if (err_fi != -ENOTTY)
> > +   printf("@", strerror(-err_fi));
> 
> Or here.

I don't mind which - I could change this to:

else if (err_fi != -ENOTTY && err_fi != -EINVAL)

Or an alternative would be to print an error (ignoring ENOTTY and EINVAL)
to stderr at the "v4l2_subdev_get_frame_interval" callsite and continue
on (ensuring that interval is zeroed).

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

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

[Philipp Zabel]

On Mon, 2017-03-20 at 15:43 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> > To set and read colorimetry information:
> > https://patchwork.linuxtv.org/patch/39350/
> 
> Thanks, I've applied all four of your patches, but there's a side effect
> from that.  Old media-ctl (modified by me):
> 
> - entity 53: imx219 0-0010 (2 pads, 2 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8/816x616 field:none
>  frame_interval:1/25]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> [fmt:SRGGB10/3280x2464 field:none
>  crop.bounds:(0,0)/3280x2464
>  crop:(0,0)/3264x2464
>  compose.bounds:(0,0)/3264x2464
>  compose:(0,0)/816x616]
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
> 
> New media-ctl:
> 
> - entity 53: imx219 0-0010 (2 pads, 2 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb 
> xfer:srgb]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
> 
> It looks like we successfully retrieve the frame interval for pad 0
> and print it, but when we try to retrieve the frame interval for pad 1,
> we get EINVAL (because that's what I'm returning, but I'm wondering if
> that's the correct thing to do...) and that prevents _all_ format
> information being output.

According to the documentation [1], you are doing the right thing:

The struct v4l2_subdev_frame_interval pad references a non-existing
pad, or the pad doesn’t support frame intervals.

But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
not implemented at all, which is turned into -ENOTTY by video_usercopy.

[1] 
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value

> Maybe something like the following would be a better idea?
> 
>  utils/media-ctl/media-ctl.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index f61963a..a50a559 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity 
> *entity,
>   struct v4l2_mbus_framefmt format;
>   struct v4l2_fract interval = { 0, 0 };
>   struct v4l2_rect rect;
> - int ret;
> + int ret, err_fi;
>  
>   ret = v4l2_subdev_get_format(entity, , pad, which);
>   if (ret != 0)
>   return;
>  
> - ret = v4l2_subdev_get_frame_interval(entity, , pad);
> - if (ret != 0 && ret != -ENOTTY)
> - return;
> + err_fi = v4l2_subdev_get_frame_interval(entity, , pad);

Not supporting frame intervals doesn't warrant a visible error message,
I think -EINVAL should also be ignored above, if the spec is to be
believed.

>  
>   printf("\t\t[fmt:%s/%ux%u",
>  v4l2_subdev_pixelcode_to_string(format.code),
>  format.width, format.height);
>  
> - if (interval.numerator || interval.denominator)
> + if (err_fi == 0 && (interval.numerator || interval.denominator))
>   printf("@%u/%u", interval.numerator, interval.denominator);
> + else if (err_fi != -ENOTTY)
> + printf("@", strerror(-err_fi));

Or here.

>  
>   if (format.field)
>   printf(" field:%s", v4l2_subdev_field_to_string(format.field));
> 
> 

regards
Philipp

Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning

[Arnd Bergmann]

On Mon, Mar 20, 2017 at 4:05 PM, Stephen Hemminger
 wrote:
> On Mon, 20 Mar 2017 10:32:19 +0100
> Arnd Bergmann  wrote:
>
>> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
>> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
>>  {
> Why keep the comment?

The comment matches one later in the file when this function gets called.

I thought about cleaning up both at the same time, but couldn't figure out
how the comment ended up in there or why it was left behind in the first
place, so I ended up leaving both for another patch on top. If you prefer,
I could resend the patch and do both at once.

Arnd

Re: [PATCHv5 01/16] ov7670: document device tree bindings

[Rob Herring]

On Sat, Mar 11, 2017 at 12:23:13PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Add binding documentation and add that file to the MAINTAINERS entry.
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Sakari Ailus 
> ---
>  .../devicetree/bindings/media/i2c/ov7670.txt   | 43 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt

Acked-by: Rob Herring 

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

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 14:24:25 +0100
> Hans Verkuil  escreveu:
> > I don't think this control inheritance patch will magically prevent you
> > from needed a plugin.
> 
> Yeah, it is not just control inheritance. The driver needs to work
> without subdev API, e. g. mbus settings should also be done via the
> video devnode.
> 
> Btw, Sakari made a good point on IRC: what happens if some app 
> try to change the pipeline or subdev settings while another
> application is using the device? The driver should block such 
> changes, maybe using the V4L2 priority mechanism.

My understanding is that there are already mechanisms in place to
prevent that, but it's driver dependent - certainly several of the
imx driver subdevs check whether they have an active stream, and
refuse (eg) all set_fmt calls with -EBUSY if that is so.

(That statement raises another question in my mind: if the subdev is
streaming, should it refuse all set_fmt, even for the TRY stuff?)

> In parallel, someone has to fix libv4l for it to be default on
> applications like gstreamer, e. g. adding support for DMABUF
> and fixing other issues that are preventing it to be used by
> default.

Hmm, not sure what you mean there - I've used dmabuf with gstreamer's
v4l2src linked to libv4l2, importing the buffers into etnaviv using
a custom plugin.  There are distros around (ubuntu) where the v4l2
plugin is built against libv4l2.

> My understanding here is that there are developers wanting/needing
> to have standard V4L2 apps support for (some) i.MX6 devices. Those are
> the ones that may/will allocate some time for it to happen.

Quite - but we need to first know what is acceptable to the v4l2
community before we waste a lot of effort coding something up that
may not be suitable.  Like everyone else, there's only a limited
amount of effort available, so using it wisely is a very good idea.

Up until recently, it seemed that the only solution was to solve the
userspace side of the media controller API via v4l2 plugins and the
like.

Much of my time that I have available to look at the imx6 capture
stuff at the moment is taken up by triping over UAPI issues with the
current code (such as the ones about CSI scaling, colorimetry, etc)
and trying to get concensus on what the right solution to fix those
issues actually is, and at the moment I don't have spare time to
start addressing any kind of v4l2 plugin for user controls nor any
other solution.

Eg, I spent much of this last weekend sorting out my IMX219 camera
sensor driver for my new understanding about how scaling is supposed
to work, the frame enumeration issue (which I've posted patches for)
and the CSI scaling issue (which I've some half-baked patch for at the
moment, but probably by the time I've finished sorting that, Philipp
or Steve will already have a solution.)

That said, my new understanding of the scaling impacts the four patches
I posted, and probably makes the frame size enumeration in CSI (due to
its scaling) rather obsolete.

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

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Michael Zoran]

On Mon, 2017-03-20 at 12:33 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 08:11:41 -0700
> Michael Zoran  escreveu:
> 
> > On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> > > Em Mon, 20 Mar 2017 04:08:21 -0700
> > > Michael Zoran  escreveu:
> > >   
> > > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab
> > > > wrote:  
> > > > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > > > Mauro Carvalho Chehab  escreveu:
> > > > > 
> > > > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > > > Michael Zoran  escreveu:
> > > > > > 
> > > > > > > A working DT that I tried this morning with the current
> > > > > > > firmware
> > > > > > > is
> > > > > > > posted here:
> > > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/201
> > > > > > > 7-Ma
> > > > > > > rch/
> > > > > > > 005924
> > > > > > > .html
> > > > > > > 
> > > > > > > It even works with minecraft_pi!  
> > > > > 
> > > > > 
> > > > 
> > > > Hi, can you e-mail out your config.txt?  Do you have audio
> > > > enabled
> > > > in
> > > > config.txt?  
> > > 
> > > yes, I have this:
> > > 
> > > $ cat config.txt |grep -i audio
> > > # uncomment to force a HDMI mode rather than DVI. This can make
> > > audio
> > > work in
> > > # Enable audio (loads snd_bcm2835)
> > > dtparam=audio=on
> > > 
> > > Full config attached.
> > > 
> > > Thanks,
> > > Mauro
> > >   
> > 
> > Are you using Eric Anholt's HDMI Audio driver that's included in
> > VC4? 
> > That could well be incompatible with the firmware driver. Or are
> > you
> > using a half mode of VC4 for audio and VCHIQ for video?
> 
> I'm using vanilla staging Kernel, from Greg's tree:
>   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.
> git/commit/?h=staging-
> next=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c
> 
> Plus the DWC2 fixup I wrote and DT changes you pointed
> (see enclosed).
> 
> I can disable the audio overlay here, as I don't have anything 
> connected to audio inputs/outputs.
> 
> Regards,
> Mauro
> 

Why is the vchiq node in the tree twice? For me to even respond anymore
you you going to have to include your entire dtb(whatever you are
using) run through dtc -I dtb -O dts.  You are also going to have to
include your exact .config file you used for building, and exactly what
these DWC2 fixeups are.

You don't even state exactly what platform you are using, Is it even an
RPI of some kind.

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

[Hans Verkuil]

On 03/20/2017 03:11 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote:
>> On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
>>> It's what I have - remember, not everyone is happy to constantly replace
>>> their distro packages with random new stuff.
>>
>> This is a compliance test, which is continuously developed in tandem with
>> new kernel versions. If you are working with an upstream kernel, then you
>> should also use the corresponding v4l2-compliance test. What's the point
>> of using an old one?
>>
>> I will not support driver developers that use an old version of the
>> compliance test, that's a waste of my time.
> 
> The reason that people may _not_ wish to constantly update v4l-utils
> is that it changes the libraries installed on their systems.
> 
> So, the solution to that is not to complain about developers not using
> the latest version, but instead to de-couple it from the rest of the
> package, and provide it as a separate, stand-alone package that doesn't
> come with all the extra baggage.
> 
> Now, there's two possible answers to that:
> 
> 1. it depends on the libv4l2 version.  If that's so, then you are
>insisting that people constantly move to the latest libv4l2 because
>of API changes, and those API changes may upset applications they're
>using.  So this isn't really on.
> 
> 2. it doesn't depend on libv4l2 version, in which case there's no reason
>for it to be packaged with v4l-utils.

Run configure with --disable-v4l2-compliance-libv4l.

This avoids linking with libv4l and allows you to build it stand-alone.

Perhaps I should invert this option since in most cases you don't want to
run v4l2-compliance with libv4l (it's off by default unless you pass the
-w option to v4l2-compliance).

> 
> The reality is that v4l2-compliance links with libv4l2, so I'm not sure
> which it is.  What I am sure of is that I don't want to upgrade libv4l2
> on an ad-hoc basis, potentially causing issues with applications.
> 
 To test actual streaming you need to provide the -s option.

 Note: v4l2-compliance has been developed for 'regular' video devices,
 not MC devices. It may or may not work with the -s option.
>>>
>>> Right, and it exists to verify that the establised v4l2 API is correctly
>>> implemented.  If the v4l2 API is being offered to user applications,
>>> then it must be conformant, otherwise it's not offering the v4l2 API.
>>> (That's very much a definition statement in itself.)
>>>
>>> So, are we really going to say MC devices do not offer the v4l2 API to
>>> userspace, but something that might work?  We've already seen today
>>> one user say that they're not going to use mainline because of the
>>> crud surrounding MC.
>>>
>>
>> Actually, my understanding was that he was stuck on the old kernel code.
> 
> Err, sorry, I really don't follow.  Who is "he"?

"one user say that they're not going to use mainline because of the
crud surrounding MC."

> 
> _I_ was the one who reported the EXPBUF problem.  Your comment makes no
> sense.
> 
>> In the case of v4l2-compliance, I never had the time to make it work with
>> MC devices. Same for that matter of certain memory to memory devices.
>>
>> Just like MC devices these too behave differently. They are partially
>> supported in v4l2-compliance, but not fully.
> 
> It seems you saying that the API provided by /dev/video* for a MC device
> breaks the v4l2-compliance tests?

There may be tests in the compliance suite that do not apply for MC devices
and for which I never check. The compliance suite was never written with MC
devices in mind, and it certainly hasn't been tested much with such devices.

It's only very recent that I even got hardware that has MC support...

>From what I can tell from the feedback I got it seems to be OKish, but I
just can't guarantee that the compliance utility is correct for such devices.

In particular I doubt the streaming tests (-s, -f, etc.) will work. The -s
test *might* work if the pipeline is configured correctly before running
v4l2-compliance. I can't imagine that the -f option would work at all since
I would expect pipeline validation errors.

I've been gently pushing Helen Koike to finish her virtual MC driver
(https://patchwork.kernel.org/patch/9312783/) since having a virtual driver
makes writing compliance tests much easier.

> _No one_ has mentioned using v4l2-compliance on the subdevs.
> 
>> Complaining about this really won't help. We know it's a problem and unless
>> someone (me perhaps?) manages to get paid to work on this it's unlikely to
>> change for now.
> 
> Like the above comment, your comment makes no sense.  I'm not complaining,
> I'm trying to find out the details.

Must be me then, it did feel like complaining...

> Yes, MC stuff sucks big time right now, the documentation is poor, there's
> a lack of understanding on all sides of the issues (which can be seen by
> the different opinions that people hold.)  

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

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> To set and read colorimetry information:
> https://patchwork.linuxtv.org/patch/39350/

Thanks, I've applied all four of your patches, but there's a side effect
from that.  Old media-ctl (modified by me):

- entity 53: imx219 0-0010 (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8/816x616 field:none
 frame_interval:1/25]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
[fmt:SRGGB10/3280x2464 field:none
 crop.bounds:(0,0)/3280x2464
 crop:(0,0)/3264x2464
 compose.bounds:(0,0)/3264x2464
 compose:(0,0)/816x616]
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]

New media-ctl:

- entity 53: imx219 0-0010 (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb 
xfer:srgb]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]

It looks like we successfully retrieve the frame interval for pad 0
and print it, but when we try to retrieve the frame interval for pad 1,
we get EINVAL (because that's what I'm returning, but I'm wondering if
that's the correct thing to do...) and that prevents _all_ format
information being output.

Maybe something like the following would be a better idea?

 utils/media-ctl/media-ctl.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index f61963a..a50a559 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity 
*entity,
struct v4l2_mbus_framefmt format;
struct v4l2_fract interval = { 0, 0 };
struct v4l2_rect rect;
-   int ret;
+   int ret, err_fi;
 
ret = v4l2_subdev_get_format(entity, , pad, which);
if (ret != 0)
return;
 
-   ret = v4l2_subdev_get_frame_interval(entity, , pad);
-   if (ret != 0 && ret != -ENOTTY)
-   return;
+   err_fi = v4l2_subdev_get_frame_interval(entity, , pad);
 
printf("\t\t[fmt:%s/%ux%u",
   v4l2_subdev_pixelcode_to_string(format.code),
   format.width, format.height);
 
-   if (interval.numerator || interval.denominator)
+   if (err_fi == 0 && (interval.numerator || interval.denominator))
printf("@%u/%u", interval.numerator, interval.denominator);
+   else if (err_fi != -ENOTTY)
+   printf("@", strerror(-err_fi));
 
if (format.field)
printf(" field:%s", v4l2_subdev_field_to_string(format.field));


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

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 08:11:41 -0700
Michael Zoran  escreveu:

> On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 20 Mar 2017 04:08:21 -0700
> > Michael Zoran  escreveu:
> >   
> > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:  
> > > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > > Mauro Carvalho Chehab  escreveu:
> > > >     
> > > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > > Michael Zoran  escreveu:
> > > > >     
> > > > > > A working DT that I tried this morning with the current
> > > > > > firmware
> > > > > > is
> > > > > > posted here:
> > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma
> > > > > > rch/
> > > > > > 005924
> > > > > > .html
> > > > > > 
> > > > > > It even works with minecraft_pi!  
> > > > 
> > > >     
> > > 
> > > Hi, can you e-mail out your config.txt?  Do you have audio enabled
> > > in
> > > config.txt?  
> > 
> > yes, I have this:
> > 
> > $ cat config.txt |grep -i audio
> > # uncomment to force a HDMI mode rather than DVI. This can make audio
> > work in
> > # Enable audio (loads snd_bcm2835)
> > dtparam=audio=on
> > 
> > Full config attached.
> > 
> > Thanks,
> > Mauro
> >   
> 
> Are you using Eric Anholt's HDMI Audio driver that's included in VC4? 
> That could well be incompatible with the firmware driver. Or are you
> using a half mode of VC4 for audio and VCHIQ for video?

I'm using vanilla staging Kernel, from Greg's tree:

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-next=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c

Plus the DWC2 fixup I wrote and DT changes you pointed
(see enclosed).

I can disable the audio overlay here, as I don't have anything 
connected to audio inputs/outputs.

Regards,
Mauro

---


diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi 
b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 38e6050035bc..1f42190e8558 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -27,6 +27,14 @@
firmware = <>;
#power-domain-cells = <1>;
};
+
+   vchiq {
+   compatible = "brcm,bcm2835-vchiq";
+   reg = <0x7e00b840 0xf>;
+   interrupts = <0 2>;
+   cache-line-size = <32>;
+   firmware = <>;
+   };
};
 };

diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts 
b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
index c309633a1e87..7e8d42904022 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
@@ -17,6 +17,45 @@
gpios = < 47 0>;
};
};
+
+
+   soc {
+
+// hvs at 7e40 {
+// status = "disabled";
+// };
+
+// v3d: v3d at 7ec0 {
+// status = "disabled";
+// };
+
+   vc4: gpu {
+   status = "disabled";
+   };
+
+   fb: fb {
+   status = "disabled";
+   };
+
+   vchiq: vchiq {
+   compatible = "brcm,bcm2835-vchiq";
+   reg = <0x7e00b840 0xf>;
+   interrupts = <0 2>;
+   cache-line-size = <32>;
+   firmware = <>;
+   };
+
+   audio: audio {
+   compatible = "brcm,bcm2835-audio";
+   brcm,pwm-channels = <8>;
+   };
+
+   };
+
+   __overrides__ {
+   cache_line_size = <>, "cache-line-size:0";
+   };
+
 };
 

Thanks,
Mauro

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

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 14:24:25 +0100
Hans Verkuil  escreveu:

> On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote:
> > Em Tue, 14 Mar 2017 08:55:36 +0100
> > Hans Verkuil  escreveu:
> >   
> >> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote:  
> >>> Hi Sakari,
> >>>

> >> We're all very driver-development-driven, and userspace gets very little
> >> attention in general. So before just throwing in the towel we should take
> >> a good look at the reasons why there has been little or no development: is
> >> it because of fundamental design defects, or because nobody paid attention
> >> to it?  
> > 
> > No. We should look it the other way: basically, there are patches
> > for i.MX6 driver that sends control from videonode to subdevs. 
> > 
> > If we nack apply it, who will write the userspace plugin? When
> > such change will be merged upstream?
> > 
> > If we don't have answers to any of the above questions, we should not
> > nack it.
> > 
> > That's said, that doesn't prevent merging a libv4l plugin if/when
> > someone can find time/interest to develop it.  
> 
> I don't think this control inheritance patch will magically prevent you
> from needed a plugin.

Yeah, it is not just control inheritance. The driver needs to work
without subdev API, e. g. mbus settings should also be done via the
video devnode.

Btw, Sakari made a good point on IRC: what happens if some app 
try to change the pipeline or subdev settings while another
application is using the device? The driver should block such 
changes, maybe using the V4L2 priority mechanism.

> This is literally the first time we have to cater to a cheap devkit. We
> were always aware of this issue, but nobody really needed it.

That's true. Now that we have a real need for that, we need to
provide a solution.

> > I'd say that we should not care anymore on providing a solution for
> > generic applications to run on boards like OMAP3[1]. For hardware that
> > are currently available that have Kernel driver and boards developed
> > to be used as "cheap hobbyist devkit", I'd say we should implement
> > a Kernel solution that would allow them to be used without subdev
> > API, e. g. having all ioctls needed by generic applications to work
> > functional, after some external application sets the pipeline.  
> 
> I liked Russell's idea of having the DT set up an initial video path.
> This would (probably) make it much easier to provide a generic plugin since
> there is already an initial valid path when the driver is loaded, and it
> doesn't require custom code in the driver since this is left to the DT
> which really knows about the HW.

Setting the device via DT indeed makes easier (either for a kernel
or userspace solution), but things like resolution changes should
be possible without needing to change DT.

Also, as MC and subdev changes should be blocked while a V4L2 app
is using the device, using a mechanism like calling VIDIOC_S_PRIORITY
ioctl via the V4l2 interface, Kernel will require changes, anyway.

My suggestion is to touch on one driver to make it work with a
generic application. As we currently have efforts and needs for
the i.MX6 to do it, I'd say that the best would be to make it
work on such driver. Once the work is done, we can see if the
approach taken there would work at libv4l or not.

In parallel, someone has to fix libv4l for it to be default on
applications like gstreamer, e. g. adding support for DMABUF
and fixing other issues that are preventing it to be used by
default.

Nicolas,

Why libv4l is currently disabled at gstreamer's default settings?

> > [1] Yet, I might eventually do that for fun, an OMAP3 board with tvp5150
> > just arrived here last week. It would be nice to have xawtv3 running on it 
> > :-)
> > So, if I have a lot of spare time (with is very unlikely), I might 
> > eventually 
> > do something for it to work.
> >   
> >> I know it took me a very long time before I had a working omap3.  
> > 
> > My first OMAP3 board with working V4L2 source just arrived last week :-)
> >   
> >> So I am not at all surprised that little progress has been made.  
> > 
> > I'm not surprised, but I'm disappointed, as I tried to push toward a
> > solution for this problem since when we had our initial meetings about
> > it.  
> 
> So many things to do, so little time. Sounds corny, but really, that's what
> this is all about. There were always (and frankly, still are) more important
> things that needed to be done.

What's most important for some developer may not be so important for
another developer.

My understanding here is that there are developers wanting/needing
to have standard V4L2 apps support for (some) i.MX6 devices. Those are
the ones that may/will allocate some time for it to happen.

Thanks,
Mauro

Re: [PATCH v3 2/6] media: uapi: Add RGB and YUV bus formats for Synopsys HDMI TX Controller

[Hans Verkuil]

On 03/07/2017 05:42 PM, Neil Armstrong wrote:
> In order to describe the RGB and YUB bus formats used to feed the
> Synopsys DesignWare HDMI TX Controller, add missing formats to the
> list of Bus Formats.
> 
> Documentation for these formats is added in a separate patch.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  include/uapi/linux/media-bus-format.h | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/media-bus-format.h 
> b/include/uapi/linux/media-bus-format.h
> index 2168759..7cc820b 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -33,7 +33,7 @@
>  
>  #define MEDIA_BUS_FMT_FIXED  0x0001
>  
> -/* RGB - next is 0x1018 */
> +/* RGB - next is 0x101b */
>  #define MEDIA_BUS_FMT_RGB444_1X120x1016
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002
> @@ -57,8 +57,11 @@
>  #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA 0x1012
>  #define MEDIA_BUS_FMT_ARGB_1X32  0x100d
>  #define MEDIA_BUS_FMT_RGB888_1X32_PADHI  0x100f
> +#define MEDIA_BUS_FMT_RGB101010_1X30 0x1018
> +#define MEDIA_BUS_FMT_RGB121212_1X36 0x1019
> +#define MEDIA_BUS_FMT_RGB161616_1X48 0x101a
>  
> -/* YUV (including grey) - next is0x2026 */
> +/* YUV (including grey) - next is0x202c */
>  #define MEDIA_BUS_FMT_Y8_1X8 0x2001
>  #define MEDIA_BUS_FMT_UV8_1X80x2015
>  #define MEDIA_BUS_FMT_UYVY8_1_5X80x2002
> @@ -90,12 +93,18 @@
>  #define MEDIA_BUS_FMT_YVYU10_1X200x200e
>  #define MEDIA_BUS_FMT_VUY8_1X24  0x2024
>  #define MEDIA_BUS_FMT_YUV8_1X24  0x2025
> +#define MEDIA_BUS_FMT_UYVY8_1_1X24   0x2026
>  #define MEDIA_BUS_FMT_UYVY12_1X240x2020
>  #define MEDIA_BUS_FMT_VYUY12_1X240x2021
>  #define MEDIA_BUS_FMT_YUYV12_1X240x2022
>  #define MEDIA_BUS_FMT_YVYU12_1X240x2023
>  #define MEDIA_BUS_FMT_YUV10_1X30 0x2016
> +#define MEDIA_BUS_FMT_UYVY10_1_1X30  0x2027
>  #define MEDIA_BUS_FMT_AYUV8_1X32 0x2017
> +#define MEDIA_BUS_FMT_UYVY12_1_1X36  0x2028
> +#define MEDIA_BUS_FMT_YUV12_1X36 0x2029
> +#define MEDIA_BUS_FMT_YUV16_1X48 0x202a
> +#define MEDIA_BUS_FMT_UYVY16_1_1X48  0x202b
>  
>  /* Bayer - next is   0x3021 */
>  #define MEDIA_BUS_FMT_SBGGR8_1X8 0x3001
> 

These new bus formats:

#define MEDIA_BUS_FMT_UYVY8_1_1X24  0x2026
#define MEDIA_BUS_FMT_UYVY10_1_1X30 0x2027
#define MEDIA_BUS_FMT_UYVY12_1_1X36 0x2028
#define MEDIA_BUS_FMT_UYVY16_1_1X48 0x202b

are all 4:2:0, right?

I think these should all be renamed to:

#define MEDIA_BUS_FMT_UYYVYY8_1X24  0x2026
#define MEDIA_BUS_FMT_UYYVYY10_1X30 0x2027
#define MEDIA_BUS_FMT_UYYVYY12_1X36 0x2028
#define MEDIA_BUS_FMT_UYYVYY16_1X48 0x202b

Conform the other 4:2:0 format MEDIA_BUS_FMT_YDYUYDYV8_1X16 (except without the 
'D'!).

I am slightly unsure about the _1. Strictly speaking we're transferring two 
pixels per
bus-sample, so _0_5 might be more accurate. Sakari, what's your opinion about 
that?

Regards,

Hans

Re: [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver

[Rob Herring]

+Ramiro

On Thu, Mar 09, 2017 at 08:52:42PM -0800, Steve Longerbeam wrote:
> Add bindings documentation for the i.MX media driver.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt | 74 
> +
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..3059c06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,74 @@
> +Freescale i.MX Media Video Device
> +=
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx-capture-subsystem";
> +- ports  : Should contain a list of phandles pointing to camera
> + sensor interface ports of IPU devices
> +
> +example:
> +
> +capture-subsystem {
> + compatible = "fsl,imx-capture-subsystem";
> + ports = <_csi0>, <_csi1>;
> +};
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> +  specified as . The channel number
> +  must be 0 or 1. The flags can be
> +  IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> +  IRQ_TYPE_EDGE_BOTH, and specify which input
> +  capture signal edge will trigger the input
> +  capture event. If an input capture channel is
> +  specified, the FIM will use this method to
> +  measure frame intervals instead of via the EOF
> +  interrupt. The input capture method is much
> +  preferred over EOF as it is not subject to
> +  interrupt latency errors. However it requires
> +  routing the VSYNC or FIELD output signals of
> +  the camera sensor to one of the i.MX input
> +  capture pads (SD1_DAT0, SD1_DAT1), which also
> +  gives up support for SD1.
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";

Ramiro is also working on a binding for DW MIPI CSI2 block[1]. We need 1 
binding for that.

> +- reg   : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> +   (the D-PHY clock), video_27m (D-PHY PLL reference
> +   clock), and eim_podf;
> +- clock-names: must contain "dphy", "ref", "pix";
> +- port@*: five port nodes must exist, containing endpoints
> +   connecting to the source and sink devices according to
> +   of_graph bindings. The first port is an input port,
> +   connecting with a MIPI CSI-2 source, and ports 1
> +   through 4 are output ports connecting with parallel
> +   bus sink endpoint nodes and correspond to the four
> +   MIPI CSI-2 virtual channel outputs.
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> +   in order: 100 and 101;
> -- 
> 2.7.4
> 

[1] https://lkml.org/lkml/2017/3/7/395

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Michael Zoran]

On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 04:08:21 -0700
> Michael Zoran  escreveu:
> 
> > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:
> > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > Mauro Carvalho Chehab  escreveu:
> > >   
> > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > Michael Zoran  escreveu:
> > > >   
> > > > > A working DT that I tried this morning with the current
> > > > > firmware
> > > > > is
> > > > > posted here:
> > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma
> > > > > rch/
> > > > > 005924
> > > > > .html
> > > > > 
> > > > > It even works with minecraft_pi!
> > > 
> > >   
> > 
> > Hi, can you e-mail out your config.txt?  Do you have audio enabled
> > in
> > config.txt?
> 
> yes, I have this:
> 
> $ cat config.txt |grep -i audio
> # uncomment to force a HDMI mode rather than DVI. This can make audio
> work in
> # Enable audio (loads snd_bcm2835)
> dtparam=audio=on
> 
> Full config attached.
> 
> Thanks,
> Mauro
> 

Are you using Eric Anholt's HDMI Audio driver that's included in VC4? 
That could well be incompatible with the firmware driver. Or are you
using a half mode of VC4 for audio and VCHIQ for video?

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

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 14:10:30 +0100
Hans Verkuil  escreveu:

> On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote:
> > On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote:  
> >> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote:
> >> [...]  
> >>> The big question, waiting for an answer on the last 8 years is
> >>> who would do that? Such person would need to have several different
> >>> hardware from different vendors, in order to ensure that it has
> >>> a generic solution.
> >>>
> >>> It is a way more feasible that the Kernel developers that already 
> >>> have a certain hardware on their hands to add support inside the
> >>> driver to forward the controls through the pipeline and to setup
> >>> a "default" pipeline that would cover the common use cases at
> >>> driver's probe.  
> >>
> >> Actually, would setting pipeline via libv4l2 plugin and letting drivers
> >> provide a sane enabled default pipeline configuration be mutually
> >> exclusive? Not sure about the control forwarding, but at least a simple
> >> link setup and format forwarding would also be possible in the kernel
> >> without hindering userspace from doing it themselves later.  
> > 
> > I think this is the exact same problem as controls in ALSA.
> > 
> > When ALSA started off in life, the requirement was that all controls
> > shall default to minimum, and the user is expected to adjust controls
> > after the system is running.
> > 
> > After OSS, this gave quite a marked change in system behaviour, and
> > led to a lot of "why doesn't my sound work anymore" problems, because
> > people then had to figure out which combination of controls had to be
> > set to get sound out of their systems.
> > 
> > Now it seems to be much better, where install Linux on a platform, and
> > you have a working sound system (assuming that the drivers are all there
> > which is generally the case for x86.)
> > 
> > However, it's still possible to adjust all the controls from userspace.
> > All that's changed is the defaults.
> > 
> > Why am I mentioning this - because from what I understand Mauro saying,
> > it's no different from this situation.  Userspace will still have the
> > power to disable all links and setup its own.  The difference is that
> > there will be a default configuration that the kernel sets up at boot
> > time that will be functional, rather than the current default
> > configuration where the system is completely non-functional until
> > manually configured.
> > 
> > However, at the end of the day, I don't care _where_ the usability
> > problems are solved, only that there is some kind of solution.  It's not
> > the _where_ that's the real issue here, but the _how_, and discussion of
> > the _how_ is completely missing.
> > 
> > So, let's try kicking off a discussion about _how_ to do things.
> > 
> > _How_ do we setup a media controller system so that we end up with a
> > usable configuration - let's start with the obvious bit... which links
> > should be enabled.
> > 
> > I think the first pre-requisit is that we stop exposing capture devices
> > that can never be functional for the hardware that's present on the board,
> > so that there isn't this plentora of useless /dev/video* nodes and useless
> > subdevices.
> > 
> > One possible solution to finding a default path may be "find the shortest
> > path between the capture device and the sensor and enable intervening
> > links".
> > 
> > Then we need to try configuring that path with format/resolution
> > information.
> > 
> > However, what if something in the shortest path can't handle the format
> > that the sensor produces?  I think at that point, we'd need to drop that
> > subdev out of the path resolution, re-run the "find the shortest path"
> > algorithm, and try again.
> > 
> > Repeat until success or no path between the capture and sensor exists.
> > 
> > This works fine if you have just one sensor visible from a capture device,
> > but not if there's more than one (which I suspect is the case with the
> > Sabrelite board with its two cameras and video receiver.)  That breaks
> > the "find the shortest path" algorithm.
> > 
> > So, maybe it's a lot better to just let the board people provide via DT
> > a default setup for the connectivity of the modules somehow - certainly
> > one big step forward would be to disable in DT parts of the capture
> > system that can never be used (remembering that boards like the RPi /
> > Hummingboard may end up using DT overlays to describe this for different
> > cameras, so the capture setup may change after initial boot.)  
> 
> The MC was developed before the device tree came along. But now that the DT
> is here, I think this could be a sensible idea to let the DT provide an
> initial path.
> 
> Sakari, Laurent, Mauro: any opinions?

It makes perfect sense to me.

By setting the pipeline via DT on boards with simple configurations,
e. g. just one CSI physical interface, it can create just one

[GIT PULL for v4.12] RC fixes

[Sean Young]

Hi Mauro,

Various small RC fixes and documentation fixes. The most controversial is
the changing of the return code of lirc ioctls. I've tested lirc. If you
can think of anything else which needs testing, please let me know.

Thanks,
Sean

The following changes since commit 700ea5e0e0dd70420a04e703ff264cc133834cba:

  Merge tag 'v4.11-rc1' into patchwork (2017-03-06 06:49:34 -0300)

are available in the git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.12b

for you to fetch changes up to 3349540fb070a97d9cbd92a925235618490ec6d9:

  [media] rc: sunxi-cir: simplify optional reset handling (2017-03-20 12:08:28 
+)


Derek Robson (1):
  [media] staging: lirc: use octal instead of symbolic permission

Johan Hovold (1):
  [media] mceusb: fix NULL-deref at probe

Philipp Zabel (2):
  [media] st_rc: simplify optional reset handling
  [media] rc: sunxi-cir: simplify optional reset handling

Sean Young (11):
  [media] cxusb: dvico remotes are nec
  [media] lirc: document lirc modes better
  [media] lirc: return ENOTTY when ioctl is not supported
  [media] lirc: return ENOTTY when device does support ioctl
  [media] winbond: allow timeout to be set
  [media] gpio-ir: do not allow a timeout of 0
  [media] rc: lirc keymap no longer makes any sense
  [media] lirc: advertise LIRC_CAN_GET_REC_RESOLUTION and improve
  [media] mce_kbd: add encoder
  [media] serial_ir: iommap is a memory address, not bool
  [media] lirc: use refcounting for lirc devices

 Documentation/media/lirc.h.rst.exceptions  |   1 -
 Documentation/media/uapi/rc/lirc-dev-intro.rst |  53 +++--
 Documentation/media/uapi/rc/lirc-get-features.rst  |  13 ++-
 Documentation/media/uapi/rc/lirc-get-length.rst|   3 +-
 Documentation/media/uapi/rc/lirc-get-rec-mode.rst  |   4 +-
 Documentation/media/uapi/rc/lirc-get-send-mode.rst |   7 +-
 Documentation/media/uapi/rc/lirc-read.rst  |  16 +--
 .../media/uapi/rc/lirc-set-rec-carrier-range.rst   |   2 +-
 .../media/uapi/rc/lirc-set-rec-timeout-reports.rst |   2 +
 Documentation/media/uapi/rc/lirc-write.rst |  17 +--
 drivers/media/rc/gpio-ir-recv.c|   2 +-
 drivers/media/rc/igorplugusb.c |   2 +-
 drivers/media/rc/ir-lirc-codec.c   |  34 --
 drivers/media/rc/ir-mce_kbd-decoder.c  |  49 -
 drivers/media/rc/keymaps/Makefile  |   1 -
 drivers/media/rc/keymaps/rc-dvico-mce.c|  92 
 drivers/media/rc/keymaps/rc-dvico-portable.c   |  74 ++---
 drivers/media/rc/keymaps/rc-lirc.c |  42 ---
 drivers/media/rc/lirc_dev.c| 122 +
 drivers/media/rc/mceusb.c  |   4 +-
 drivers/media/rc/rc-core-priv.h|   2 +-
 drivers/media/rc/rc-ir-raw.c   |   6 +-
 drivers/media/rc/rc-main.c |   8 +-
 drivers/media/rc/serial_ir.c   |   4 +-
 drivers/media/rc/st_rc.c   |  15 ++-
 drivers/media/rc/sunxi-cir.c   |  21 ++--
 drivers/media/rc/winbond-cir.c |   4 +-
 drivers/media/usb/dvb-usb/cxusb.c  |  24 ++--
 drivers/staging/media/lirc/lirc_sasem.c|   2 +-
 drivers/staging/media/lirc/lirc_sir.c  |   8 +-
 include/media/rc-map.h |  79 ++---
 31 files changed, 378 insertions(+), 335 deletions(-)
 delete mode 100644 drivers/media/rc/keymaps/rc-lirc.c

Re: [PATCH v3 3/6] documentation: media: Add documentation for new RGB and YUV bus formats

[Hans Verkuil]

On 03/17/2017 05:11 PM, Neil Armstrong wrote:
> On 03/16/2017 06:01 PM, Archit Taneja wrote:
>>
>>
>> On 3/7/2017 10:12 PM, Neil Armstrong wrote:
>>> Add documentation for added Bus Formats to describe RGB and YUS formats used
>>
>> s/YUS/YUV
> 
> Thanks again
> 
>>
>>> as input to the Synopsys DesignWare HDMI TX Controller.
>>>
>>> Signed-off-by: Neil Armstrong 
>>> ---
>>>  Documentation/media/uapi/v4l/subdev-formats.rst | 4992 
>>> ++-
>>>  1 file changed, 3963 insertions(+), 1029 deletions(-)
>>
>> Do we know if there is a better way to add more columns without
>> adding so many lines?
> 
> It seems not, the reason is written in the commands.
> 
>> If not, one option could be to create a separate tables for
>> 48 bit RGB formats, 48 bit YUV formats etc.
> 
> It would be simple indeed, any V4L guys for an advice here ?

I would split up these large tables into separate tables, depending on the
number of bits each pixel uses: so an RGB table for 8 bits, 9-16 bits,
17-24, 25-32, 33-48.

This also avoids a major problem where the horizontal scrollbar is at the bottom
of the table, but the column numbering is at the top and out of sight if the
table is long.

It also prevents these large horizontal widths when they are not needed.

Regards,

Hans

Re: [PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640

[Rob Herring]

On Thu, Mar 09, 2017 at 08:52:43PM -0800, Steve Longerbeam wrote:
> Add device tree binding documentation for the OV5640 camera sensor.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   | 45 
> ++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt

Acked-by: Rob Herring 

Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning

[Stephen Hemminger]

On Mon, 20 Mar 2017 10:32:19 +0100
Arnd Bergmann  wrote:

> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
>  {
Why keep the comment?

[PATCH 08/24] atomisp/imx: Fix locking bug on error path

[Alan Cox]

This was reported by Dan Carpenter. When we error with an IMX 227 we don't 
release
the lock and the sensor would then hang.

Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/i2c/imx/imx.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/i2c/imx/imx.c 
b/drivers/staging/media/atomisp/i2c/imx/imx.c
index a73f902..408a7b9 100644
--- a/drivers/staging/media/atomisp/i2c/imx/imx.c
+++ b/drivers/staging/media/atomisp/i2c/imx/imx.c
@@ -454,8 +454,10 @@ static int imx_set_exposure_gain(struct v4l2_subdev *sd, 
u16 coarse_itg,
 
if (dev->sensor_id == IMX227_ID) {
ret = imx_write_reg_array(client, imx_param_hold);
-   if (ret)
+   if (ret) {
+   mutex_unlock(>input_lock);
return ret;
+   }
}
 
/* For imx175, setting gain must be delayed by one */

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 04:08:21 -0700
Michael Zoran  escreveu:

> On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:
> > Em Sun, 19 Mar 2017 22:11:07 -0300
> > Mauro Carvalho Chehab  escreveu:
> >   
> > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > Michael Zoran  escreveu:
> > >   
> > > > A working DT that I tried this morning with the current firmware
> > > > is
> > > > posted here:
> > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/
> > > > 005924
> > > > .html
> > > > 
> > > > It even works with minecraft_pi!    
> > 
> > With the new firmware, sometime after booting, I'm getting an oops,
> > caused
> > by bcm2835_audio/vchiq:
> > 
> > [  298.788995] Unable to handle kernel NULL pointer dereference at
> > virtual address 0034
> > [  298.821458] pgd = ed004000
> > [  298.832294] [0034] *pgd=2e5e9835, *pte=,
> > *ppte=
> > [  298.857450] Internal error: Oops: 17 [#1] SMP ARM
> > [  298.876273] Modules linked in: cfg80211 hid_logitech_hidpp
> > hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore
> > vchiq(C) uio_pdrv_genirq uio fuse
> > [  298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted:
> > G C  4.11.0-rc1+ #56
> > [  298.963774] Hardware name: Generic DT based system
> > [  298.982945] task: ef758580 task.stack: ee4c6000
> > [  299.001080] PC is at mutex_lock+0x14/0x3c
> > [  299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0
> > [vchiq]
> > [  299.042246] pc : []lr : []psr:
> > 4013
> > sp : ee4c7ca8  ip :   fp : ef709800
> > [  299.088240] r10:   r9 : ee3bffc0  r8 : 0034
> > [  299.109153] r7 : 0003  r6 :   r5 : ee4c7d00  r4 :
> > ee1d8c00
> > [  299.135291] r3 : ef758580  r2 :   r1 : ffc8  r0 :
> > 0034
> > [  299.161431] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA
> > ARM  Segment none
> > [  299.190008] Control: 10c5383d  Table: 2d00406a  DAC: 0051
> > [  299.213011] Process pulseaudio (pid: 847, stack limit =
> > 0xee4c6220)
> > [  299.238104] Stack: (0xee4c7ca8 to 0xee4c8000)
> > [  299.255539] 7ca0:   c1403d54 80400040 ff7f0600
> > ff7f0660 bf06b578 ee3bffc0
> > [  299.288301] 7cc0:  ee3afd00  ee4c7d00 
> > bf0640b4  bf066428
> > [  299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0
> > ee3af444 ee3bffc0 bf0662ec
> > [  299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408
> > ee3af440  bf0d7408
> > [  299.386587] 7d20: ee79bd80 bf0d5a04  ef709800 0020
> > 0002 0001 41554453
> > [  299.419349] 7d40:    bf0d559c ee3af440
> > 0001 0001 
> > [  299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00  ee79bd80
> > ee24ace8 0001 bf0d4dfc
> > [  299.484872] 7d80: 000b  ee4b8c3c  ee4c7dc8
> > ee4b8800 ee4b8c28 ee4c6000
> > [  299.517635] 7da0:  ee4b8c3c ed029e40 bf0c0404 ee4b8800
> > ee1c4a00 ee4b8800 ed029e40
> > [  299.550398] 7dc0:  bf0c0560 ee072340  ef758580
> > c0367b7c ee4b8c40 ee4b8c40
> > [  299.583161] 7de0:  ee4b8800 ed029e40 ee318f80 ed029e40
> > 0006 ee318f80 bf0c0748
> > [  299.615924] 7e00: bf0a3430 ee4f6180  c0428fe0 ee318f80
> > 21b0 0026 ed029e40
> > [  299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006
> > c0421cc0 ee4c7ed0 
> > [  299.681464] 7e40: 0802  ee4c7e94 0006 ee318f80
> > c0432c8c ee4c7f40 c0433bc0
> > [  299.714225] 7e60:  ed029e40  0041 
> > ed004000  ee4c6000
> > [  299.746987] 7e80: eec69808 0005  0002 ee318f80
> > ef0d2910 ee924908 bf0ba284
> > [  299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74
> > 0001 f000 c0308b04
> > [  299.812512] 7ec0: ee4c6000  bebb5710 c0434578 ef0d2910
> > ee924908 73541c18 0008
> > [  299.845274] 7ee0: ee4a7019   ee899bb0 ee318f80
> > 0101 0002 0084
> > [  299.878037] 7f00:    ee4c7f10 ee318df8
> > ed029840 40045532 bebb53c4
> > [  299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011
> > ee5eca00 0020 ee5eca18
> > [  299.943562] 7f40: ee4a7000  00080802 0002 ff9c
> > f000 0011 ff9c
> > [  299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840
> > 0802 ed02 0006
> > [  300.009086] 7f80: 0100 0001   0004
> > b189d000 0005 c0308b04
> > [  300.041848] 7fa0: ee4c6000 c0308940  0004 bebb550c
> > 00080802 bebb53c4 00084b58
> > [  300.074611] 7fc0:  0004 b189d000 0005 
> > bebb550c 00099448 bebb5710
> > [  300.107373] 7fe0:  bebb53c8 b6c40da4 b6c24334 8010
> > bebb550c 2fffd861 2fffdc61
> > [  300.140190] [] (mutex_lock) from []
> > (vchiq_add_service_internal+0x138/0x3a0 [vchiq])
> > [  300.178237] [] 

[PATCH 12/24] staging: media: atomisp: select REGMAP_I2C needed by ap1302.c

[Alan Cox]

From: Jérémy Lefaure 

REGMAP_I2C should be enabled to build the driver ap1302 because it uses
regmap functions.

Signed-off-by: Jérémy Lefaure 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/i2c/Kconfig |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
b/drivers/staging/media/atomisp/i2c/Kconfig
index 30b0bb9..b80d29d 100644
--- a/drivers/staging/media/atomisp/i2c/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/Kconfig
@@ -62,6 +62,7 @@ config VIDEO_MT9M114
 config VIDEO_AP1302
tristate "AP1302 external ISP support"
depends on I2C && VIDEO_V4L2
+   select REGMAP_I2C
---help---
  This is a Video4Linux2 sensor-level driver for the external
  ISP AP1302.

[PATCH 06/24] atomisp: kill another define

[Alan Cox]

We don't need an ifdef for the sake of 8-12 bytes. This undoes the ifdef added 
by
fde469701c7efabebf885e785edf367bfb1a8f3f. Instead turn it into a single const 
string
array at a fixed location thereby saving even more memory.

Signed-off-by: Alan Cox 
---
 .../staging/media/atomisp/pci/atomisp2/hmm/hmm.c   |   23 +---
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index e78f02f..1f07c7a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -1,7 +1,7 @@
 /*
  * Support for Medifield PNW Camera Imaging ISP subsystem.
  *
- * Copyright (c) 2010 Intel Corporation. All Rights Reserved.
+ * Copyright (c) 2010-2017 Intel Corporation. All Rights Reserved.
  *
  * Copyright (c) 2010 Silicon Hive www.siliconhive.com.
  *
@@ -45,14 +45,11 @@ struct hmm_pool reserved_pool;
 static ia_css_ptr dummy_ptr;
 struct _hmm_mem_stat hmm_mem_stat;
 
-const char *hmm_bo_type_strings[HMM_BO_LAST] = {
-   "p", /* private */
-   "s", /* shared */
-   "u", /* user */
-#ifdef CONFIG_ION
-   "i", /* ion */
-#endif
-};
+/* p: private
+   s: shared
+   u: user
+   i: ion */
+static const char hmm_bo_type_string[] = "psui";
 
 static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
char *buf, struct list_head *bo_list, bool active)
@@ -77,8 +74,8 @@ static ssize_t bo_show(struct device *dev, struct 
device_attribute *attr,
if ((active && (bo->status & HMM_BO_ALLOCED)) ||
(!active && !(bo->status & HMM_BO_ALLOCED))) {
ret = scnprintf(buf + index1, PAGE_SIZE - index1,
-   "%s %d\n",
-   hmm_bo_type_strings[bo->type], bo->pgnr);
+   "%c %d\n",
+   hmm_bo_type_string[bo->type], bo->pgnr);
 
total[bo->type] += bo->pgnr;
count[bo->type]++;
@@ -92,8 +89,8 @@ static ssize_t bo_show(struct device *dev, struct 
device_attribute *attr,
if (count[i]) {
ret = scnprintf(buf + index1 + index2,
PAGE_SIZE - index1 - index2,
-   "%ld %s buffer objects: %ld KB\n",
-   count[i], hmm_bo_type_strings[i], total[i] * 4);
+   "%ld %c buffer objects: %ld KB\n",
+   count[i], hmm_bo_type_string[i], total[i] * 4);
if (ret > 0)
index2 += ret;
}

[PATCH 07/24] ov5693: remove unused function

[Alan Cox]

It's commented out in the tree with a note saying to remove it. So let's remove 
it.

Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.c |   23 -
 1 file changed, 23 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c 
b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
index ac75982..5e9dafe 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
+++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
@@ -82,30 +82,7 @@ static int vcm_ad_i2c_wr8(struct i2c_client *client, u8 reg, 
u8 val)
}
return 0;
 }
-/*TODO: remove this unuseful i2c writer helper*/
-/*
-static int vcm_ad_i2c_wr16(struct i2c_client *client, u8 reg, u16 val)
-{
-   int err;
-   struct i2c_msg msg;
-   u8 buf[3];
-   buf[0] = reg;
-   buf[1] = (u8)(val >> 8);
-   buf[2] = (u8)(val & 0xff);
-   msg.addr = VCM_ADDR;
-   msg.flags = 0;
-   msg.len = 3;
-   msg.buf = [0];
 
-   err = i2c_transfer(client->adapter, , 1);
-   if (err != 1) {
-   dev_err(>dev, "%s: vcm i2c fail, err code = %d\n",
-   __func__, err);
-   return -EIO;
-   }
-   return 0;
-}
-*/
 static int ad5823_i2c_write(struct i2c_client *client, u8 reg, u8 val)
 {
struct i2c_msg msg;

[PATCH 24/24] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[Alan Cox]

From: Daeseok Youn 

If v4l2_subdev_call() gets the global frame interval values,
it returned 0 and it could be checked whether numerator is zero or not.

If the numerator is not zero, the fps could be calculated in this function.
If not, it just returns 0.

Signed-off-by: Daeseok Youn 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |   22 +---
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 0a2df3d..9d44a1d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
video_device *dev)
 
 static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
 {
-   struct v4l2_subdev_frame_interval frame_interval;
+   struct v4l2_subdev_frame_interval fi;
struct atomisp_device *isp = asd->isp;
-   unsigned short fps;
 
-   if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-   video, g_frame_interval, _interval)) {
-   fps = 0;
-   } else {
-   if (frame_interval.interval.numerator)
-   fps = frame_interval.interval.denominator /
-   frame_interval.interval.numerator;
-   else
-   fps = 0;
-   }
+   unsigned short fps = 0;
+   int ret;
+
+   ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+  video, g_frame_interval, );
+
+   if (!ret && fi.interval.numerator)
+   fps = fi.interval.denominator / fi.interval.numerator;
+
return fps;
 }
 

[PATCH 23/24] atomisp: remove a sysfs error message that can be used to log spam

[Alan Cox]

Instead of logging this just report ERANGE as an error, which will give 
something close to the
right user space report.

The others of these were already removed by Dan Carpenter's patch.

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

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_drvfs.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_drvfs.c
index 7f7c6d5..fcfe8d7 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_drvfs.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_drvfs.c
@@ -107,9 +107,7 @@ static ssize_t iunit_dbglvl_store(struct device_driver 
*drv, const char *buf,
if (kstrtouint(buf, 10, _debug.dbglvl)
|| iunit_debug.dbglvl < 1
|| iunit_debug.dbglvl > 9) {
-   dev_err(atomisp_dev, "%s setting %d value invalid, should be 
[1,9]\n",
-   __func__, iunit_debug.dbglvl);
-   return -EINVAL;
+   return -ERANGE;
}
atomisp_css_debug_set_dtrace_level(iunit_debug.dbglvl);
 

[PATCH 05/24] atomisp: ia_css_bh_hmem_encode is a no-op so remove it

[Alan Cox]

This is a do nothing function so we can replace it with nothing and eliminate 
it entirely.

Signed-off-by: Alan Cox 
---
 .../ia_css_isp_params.c|6 --
 .../ia_css_isp_params.c|6 --
 .../ia_css_isp_params.c|6 --
 .../css2400/isp/kernels/bh/bh_2/ia_css_bh.host.c   |   11 ---
 .../css2400/isp/kernels/bh/bh_2/ia_css_bh.host.h   |6 --
 5 files changed, 35 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
index 9620bc3..3246d99 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
@@ -176,15 +176,9 @@ size);
{
unsigned size   = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.size;
 
-   unsigned offset = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.offset;
-
if (size) {
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, 
"ia_css_process_bh() enter:\n");
 
-   ia_css_bh_hmem_encode((struct sh_css_isp_bh_hmem_params 
*)
-   
>binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_HMEM0].address[offset],
-   >s3a_config,
-size);
params->isp_params_changed = true;

params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_HMEM0] = 
true;
 
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hive_isp_css_2401_system_csi2p_generated/ia_css_isp_params.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hive_isp_css_2401_system_csi2p_generated/ia_css_isp_params.c
index 87a3308..4c79a31 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hive_isp_css_2401_system_csi2p_generated/ia_css_isp_params.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hive_isp_css_2401_system_csi2p_generated/ia_css_isp_params.c
@@ -175,15 +175,9 @@ size);
{
unsigned size   = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.size;
 
-   unsigned offset = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.offset;
-
if (size) {
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, 
"ia_css_process_bh() enter:\n");
 
-   ia_css_bh_hmem_encode((struct sh_css_isp_bh_hmem_params 
*)
-   
>binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_HMEM0].address[offset],
-   >s3a_config,
-size);
params->isp_params_changed = true;

params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_HMEM0] = 
true;
 
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hive_isp_css_2401_system_generated/ia_css_isp_params.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hive_isp_css_2401_system_generated/ia_css_isp_params.c
index 87a3308..4c79a31 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hive_isp_css_2401_system_generated/ia_css_isp_params.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hive_isp_css_2401_system_generated/ia_css_isp_params.c
@@ -175,15 +175,9 @@ size);
{
unsigned size   = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.size;
 
-   unsigned offset = 
stage->binary->info->mem_offsets.offsets.param->hmem0.bh.offset;
-
if (size) {
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, 
"ia_css_process_bh() enter:\n");
 
-   ia_css_bh_hmem_encode((struct sh_css_isp_bh_hmem_params 
*)
-   
>binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_HMEM0].address[offset],
-   >s3a_config,
-size);
params->isp_params_changed = true;

params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_HMEM0] = 
true;
 
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/bh/bh_2/ia_css_bh.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/bh/bh_2/ia_css_bh.host.c
index 0dcafad..99c80d2 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/bh/bh_2/ia_css_bh.host.c
+++ 

[PATCH 22/24] staging: atomisp: remove redudant condition in if-statement

[Alan Cox]

From: Daeseok Youn 

The V4L2_FIELD_ANY is zero, so the (!field) is same meaning
with (field == V4L2_FIELD_ANY) in if-statement.

Signed-off-by: Daeseok Youn 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 036413b..0a2df3d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -5081,7 +5081,7 @@ atomisp_try_fmt_file(struct atomisp_device *isp, struct 
v4l2_format *f)
 
depth = get_pixel_depth(pixelformat);
 
-   if (!field || field == V4L2_FIELD_ANY)
+   if (field == V4L2_FIELD_ANY)
field = V4L2_FIELD_NONE;
else if (field != V4L2_FIELD_NONE) {
dev_err(isp->dev, "Wrong output field\n");

[PATCH 21/24] staging: atomisp: remove else statement after return

[Alan Cox]

From: Daeseok Youn 

It doesn't need to have else statement after return.

Signed-off-by: Daeseok Youn 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 37d334e..036413b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2951,11 +2951,11 @@ int atomisp_get_metadata(struct atomisp_sub_device 
*asd, int flag,
dev_err(isp->dev, "copy to user failed: copied %d bytes\n",
ret);
return -EFAULT;
-   } else {
-   list_del_init(_buf->list);
-   list_add_tail(_buf->list, >metadata[md_type]);
}
 
+   list_del_init(_buf->list);
+   list_add_tail(_buf->list, >metadata[md_type]);
+
dev_dbg(isp->dev, "%s: HAL de-queued metadata type %d with exp_id %d\n",
__func__, md_type, md->exp_id);
return 0;

[PATCH 16/24] stating/atomisp: fix -Wold-style-definition warning

[Alan Cox]

From: Arnd Bergmann 

ia_css_dequeue_param_buffers does not have an arguement type, causing a warning:

drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c: In function 
'ia_css_dequeue_param_buffers':
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c:3728:6: 
error: old-style function definition [-Werror=old-style-definition]

This adds a 'void' keywork to silence the warning.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c   |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
index e4599f7..36a0c6b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
@@ -3723,7 +3723,7 @@ static void sh_css_update_isp_mem_params_to_ddr(
IA_CSS_LEAVE_PRIVATE("void");
 }
 
-void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
+void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
 {
unsigned int i;
hrt_vaddress cpy;

[PATCH 13/24] staging: media: atomisp: add missing dependencies in Kconfig

[Alan Cox]

From: Jérémy Lefaure 

Two dependencies were missing to build atomisp drivers:

_ MEDIA_CONTROLLER: to use the entity field of v4l2_subdev structure. Since
every atomisp driver needs MEDIA_CONTROLLER has a dependency, let's add it
to INTEL_ATOMISP

_ EFI: to use efivar_entry_get:
drivers/built-in.o: In function `gmin_get_config_var':
(.text+0xe062b): undefined reference to `efivar_entry_get'

Signed-off-by: Jérémy Lefaure 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/Kconfig|2 +-
 drivers/staging/media/atomisp/i2c/ov5693/Kconfig |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index f7d8a84..28615aa 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86
+depends on X86 && EFI && MEDIA_CONTROLLER
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig 
b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
index 3954b8c..9fb1bff 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
@@ -1,6 +1,6 @@
 config VIDEO_OV5693
tristate "Omnivision ov5693 sensor support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
---help---
  This is a Video4Linux2 sensor-level driver for the Micron
  ov5693 5 Mpixel camera.

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Hans Verkuil]

On 03/20/2017 03:27 PM, Russell King - ARM Linux wrote:

On Mon, Mar 20, 2017 at 02:26:16PM +, Russell King - ARM Linux wrote:

On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:

From: Hans Verkuil 

Add support for video hotplug detect and EDID/ELD notifiers, which is used
to convey information from video drivers to their CEC and audio counterparts.

Based on an earlier version from Russell King:

https://patchwork.kernel.org/patch/9277043/

The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
of a video device.


I thought we had decided to drop the ELD bits?


Ignore that - mailer wrapped around to the first message in my mailbox!



Just FYI: I've removed anything not needed for CEC in this git repo:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=exynos4-cec2

It compiles, but it's otherwise untested.

And I still need to think more about Daniel's comments. I hope to work on this
a bit more next week.

Regards,

Hans

[PATCH 17/24] staging/atomisp: remove sh_css_lace_stat code

[Alan Cox]

From: Arnd Bergmann 

I ran into a build warning on my randconfig build box:

drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c: In function 
'ia_css_lace_statistics_free':
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c:2845:64: 
error: parameter 'me' set but not used [-Werror=unused-but-set-parameter]

It turns out that not only the parameter is unused but the entire function has 
no
caller. Let's just remove it.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 .../staging/media/atomisp/pci/atomisp2/Makefile|1 -
 .../media/atomisp/pci/atomisp2/css2400/ia_css.h|1 -
 .../atomisp/pci/atomisp2/css2400/ia_css_buffer.h   |1 -
 .../pci/atomisp2/css2400/ia_css_lace_stat.h|   37 
 .../atomisp/pci/atomisp2/css2400/sh_css_internal.h |1 -
 .../pci/atomisp2/css2400/sh_css_lace_stat.c|   16 -
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c   |   15 
 7 files changed, 72 deletions(-)
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_lace_stat.c

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/Makefile 
b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
index 162bcbf..ab10fc0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
@@ -106,7 +106,6 @@ atomisp-objs += \
css2400/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.o \
css2400/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.o \
css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.o \
-   css2400/sh_css_lace_stat.o \
css2400/sh_css_pipe.o \
css2400/ia_css_device_access.o \
css2400/sh_css_host_data.o \
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
index f67626f..2458b37 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
@@ -42,7 +42,6 @@
 #include "ia_css_stream_format.h"
 #include "ia_css_stream_public.h"
 #include "ia_css_tpg.h"
-#include "ia_css_lace_stat.h"
 #include "ia_css_version.h"
 #include "ia_css_mmu.h"
 #include "ia_css_morph.h"
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
index 26b16f4..b2ecf36 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
@@ -60,7 +60,6 @@ struct ia_css_buffer {
struct ia_css_isp_3a_statistics  *stats_3a;/**< 3A 
statistics & optionally RGBY statistics. */
struct ia_css_isp_dvs_statistics *stats_dvs;   /**< DVS 
statistics. */
struct ia_css_isp_skc_dvs_statistics *stats_skc_dvs;  /**< SKC 
DVS statistics. */
-   struct ia_css_isp_lace_statistics *stats_lace; /**< LACE 
statistics. */
struct ia_css_frame  *frame;   /**< Frame 
buffer. */
struct ia_css_acc_param  *custom_data; /**< Custom 
buffer. */
struct ia_css_metadata   *metadata;/**< Sensor 
metadata. */
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
deleted file mode 100644
index 6fee1e2..000
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-#ifndef __IA_CSS_LACE_STAT_H
-#define __IA_CSS_LACE_STAT_H
-
-/** @file
- * This file contains types used for LACE statistics
- */
-
-struct ia_css_isp_lace_statistics;
-
-/** @brief Allocate mem for the LACE statistics on the ISP
- * @return Pointer to the allocated LACE statistics
- * buffer on the ISP
-*/
-struct ia_css_isp_lace_statistics *ia_css_lace_statistics_allocate(void);
-
-/** @brief Free the ACC LACE statistics memory on the isp
- * @param[in]  me Pointer to the LACE statistics buffer on the
- *   ISP.
- * @return None
-*/
-void 

[PATCH 09/24] atomisp: remove another pair of 2400/2401 differences

[Alan Cox]

The first of these checks the PCI identifier in order to decide what to do so 
needs no
ifdef. The other is simply a variation on what is dumped for debug - so favour 
dumping the
most.

Signed-off-by Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |7 ---
 .../atomisp/pci/atomisp2/atomisp_dfs_tables.h  |4 
 2 files changed, 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d97a8df..08da8ea 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -263,12 +263,10 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
return -EINVAL;
}
 
-#ifdef ISP2401
if ((isp->pdev->device & ATOMISP_PCI_DEVICE_SOC_MASK) ==
ATOMISP_PCI_DEVICE_SOC_CHT && ATOMISP_USE_YUVPP(asd))
isp->dfs = _config_cht_soc;
 
-#endif
if (isp->dfs->lowest_freq == 0 || isp->dfs->max_freq_at_vmin == 0 ||
isp->dfs->highest_freq == 0 || isp->dfs->dfs_table_size == 0 ||
!isp->dfs->dfs_table) {
@@ -654,13 +652,8 @@ irqreturn_t atomisp_isr(int irq, void *dev)
}
 
atomisp_eof_event(asd, eof_event.event.exp_id);
-#ifndef ISP2401
-   dev_dbg(isp->dev, "%s EOF exp_id %d\n", __func__,
-   eof_event.event.exp_id);
-#else
dev_dbg(isp->dev, "%s EOF exp_id %d, asd %d\n",
__func__, eof_event.event.exp_id, asd->index);
-#endif
}
 
irq_infos &= ~IA_CSS_IRQ_INFO_ISYS_EVENTS_READY;
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_dfs_tables.h 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_dfs_tables.h
index dfb94e6..204d941 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_dfs_tables.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_dfs_tables.h
@@ -340,7 +340,6 @@ static const struct atomisp_freq_scaling_rule 
dfs_rules_cht[] = {
.run_mode = ATOMISP_RUN_MODE_PREVIEW,
},
{
-#ifdef ISP2401
.width = 1280,
.height = 720,
.fps = ISP_FREQ_RULE_ANY,
@@ -386,7 +385,6 @@ static const struct atomisp_freq_scaling_rule 
dfs_rules_cht_soc[] = {
.run_mode = ATOMISP_RUN_MODE_PREVIEW,
},
{
-#endif
.width = ISP_FREQ_RULE_ANY,
.height = ISP_FREQ_RULE_ANY,
.fps = ISP_FREQ_RULE_ANY,
@@ -403,7 +401,6 @@ static const struct atomisp_dfs_config dfs_config_cht = {
.dfs_table_size = ARRAY_SIZE(dfs_rules_cht),
 };
 
-#ifdef ISP2401
 static const struct atomisp_dfs_config dfs_config_cht_soc = {
.lowest_freq = ISP_FREQ_100MHZ,
.max_freq_at_vmin = ISP_FREQ_356MHZ,
@@ -412,5 +409,4 @@ static const struct atomisp_dfs_config dfs_config_cht_soc = 
{
.dfs_table_size = ARRAY_SIZE(dfs_rules_cht_soc),
 };
 
-#endif
 #endif /* __ATOMISP_DFS_TABLES_H__ */

[PATCH 14/24] staging/atomisp: include linux/io.h where needed

[Alan Cox]

From: Arnd Bergmann 

The plat_clock implementation fails ot build in some configurations:

platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_set_clock_freq':
platform/clock/vlv2_plat_clock.c:88:2: error: implicit declaration of function 
'writel';did you mean 'wrmsrl'? [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:88:12: error: implicit declaration of function 
'readl' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_probe':
platform/clock/vlv2_plat_clock.c:193:13: error: implicit declaration of 
function 'ioremap_nocache' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:193:11: error: assignment makes pointer from 
integer without a cast [-Werror=int-conversion]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_remove':
platform/clock/vlv2_plat_clock.c:209:2: error: implicit declaration of function 
'iounmap' [-Werror=implicit-function-declaration]

This includes the required header file.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/platform/clock/vlv2_plat_clock.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c 
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
index a8ca93d..25e939c 100644
--- a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
+++ b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
@@ -20,6 +20,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include "../../include/linux/vlv2_plat_clock.h"

[PATCH 20/24] staging/atomisp: add ACPI dependency

[Alan Cox]

From: Arnd Bergmann 

Without ACPI, some of the code fails to build:

media/atomisp/platform/intel-mid/atomisp_gmin_platform.c: In function 
'atomisp_register_i2c_module':
media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:174:7: error: 
dereferencing pointer to incomplete type 'struct acpi_device'

We could work around that in the code, but since we already have a hard
dependency on x86, adding the ACPI dependency seems to be the easiest
solution.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index 8b172ed..8eb13c3 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && EFI && MEDIA_CONTROLLER && PCI
+depends on X86 && EFI && MEDIA_CONTROLLER && PCI && ACPI
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.

[PATCH 10/24] Staging: atomisp: fix locking in alloc_user_pages()

[Alan Cox]

From: Dan Carpenter 

We call this function with the lock held and should also return with the
lock held as well.  This one error path is not-consistent because we
should return without the lock held.

Signed-off-by: Dan Carpenter 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/hmm/hmm_bo.c|1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
index fd3bd5c..d1a609d2 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
@@ -1012,6 +1012,7 @@ static int alloc_user_pages(struct hmm_buffer_object *bo,
dev_err(atomisp_dev, "find_vma failed\n");
atomisp_kernel_free(bo->page_obj);
atomisp_kernel_free(pages);
+   mutex_lock(>mutex);
return -EFAULT;
}
mutex_lock(>mutex);

[PATCH 18/24] staging/atomisp: add VIDEO_V4L2_SUBDEV_API dependency

[Alan Cox]

From: Arnd Bergmann 

The driver fails to build if this is disabled, so we need an explicit
Kconfig dependency:

drivers/staging/media/atomisp/pci/atomisp2/./atomisp_cmd.c:6085:48: error: 
'struct v4l2_subdev_fh' has no member named 'pad'

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/pci/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/Kconfig 
b/drivers/staging/media/atomisp/pci/Kconfig
index e8f6783..a724214 100644
--- a/drivers/staging/media/atomisp/pci/Kconfig
+++ b/drivers/staging/media/atomisp/pci/Kconfig
@@ -4,7 +4,7 @@
 
 config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
-   depends on VIDEO_V4L2
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
select VIDEOBUF_VMALLOC
 ---help---
   Say Y here if your platform supports Intel Atom SoC

[PATCH 11/24] Staging: atomisp: fix an uninitialized variable bug

[Alan Cox]

From: Dan Carpenter 

There are some error paths in atomisp_css_frame_allocate() which don't
initialize "res" so it could lead us to try release random memory.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Dan Carpenter 
Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 08da8ea..37d334e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -4722,7 +4722,7 @@ static int
 atomisp_v4l2_framebuffer_to_css_frame(const struct v4l2_framebuffer *arg,
 struct atomisp_css_frame **result)
 {
-   struct atomisp_css_frame *res;
+   struct atomisp_css_frame *res = NULL;
unsigned int padded_width;
enum atomisp_css_frame_format sh_format;
char *tmp_buf = NULL;

[PATCH 15/24] staging/atomisp: fix empty-body warning

[Alan Cox]

From: Arnd Bergmann 

Defining a debug function to nothing causes a warning with an empty block
after if()/else():

drivers/staging/media/atomisp/i2c/ov2680.c: In function 'ov2680_s_stream':
drivers/staging/media/atomisp/i2c/ov2680.c:1208:55: error: suggest braces 
around empty body in an 'else' statement [-Werror=empty-body]

This changes the empty debug statement to dev_dbg(), which by default also
does nothing, but avoids this warning and also checks the format string.
As a side-effect, we can now use dynamic debugging to turn on the
output at runtime.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/i2c/ov2680.c |   37 ++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c 
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 58d2a07..c08dd0b 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -35,7 +35,6 @@
 
 #include "ov2680.h"
 
-#define ov2680_debug(...) //dev_err(__VA_ARGS__)
 static int h_flag = 0;
 static int v_flag = 0;
 static enum atomisp_bayer_order ov2680_bayer_order_mapping[] = {
@@ -99,7 +98,7 @@ static int ov2680_read_reg(struct i2c_client *client,
*val = be16_to_cpu(*(u16 *)[0]);
else
*val = be32_to_cpu(*(u32 *)[0]);
-   //ov2680_debug(>dev,  "i2c read adr%x = %x\n", reg,*val);
+   //dev_dbg(>dev,  "i2c read adr%x = %x\n", reg,*val);
return 0;
 }
 
@@ -114,7 +113,7 @@ static int ov2680_i2c_write(struct i2c_client *client, u16 
len, u8 *data)
msg.len = len;
msg.buf = data;
ret = i2c_transfer(client->adapter, , 1);
-   //ov2680_debug(>dev,  "+++i2c write reg=%x->%x\n", data[0]*256 
+data[1],data[2]);
+   //dev_dbg(>dev,  "+++i2c write reg=%x->%x\n", data[0]*256 
+data[1],data[2]);
return ret == num_msg ? 0 : -EIO;
 }
 
@@ -235,7 +234,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
const struct ov2680_reg *next = reglist;
struct ov2680_write_ctrl ctrl;
int err;
-   ov2680_debug(>dev,  "write reg array\n");
+   dev_dbg(>dev,  "write reg array\n");
ctrl.index = 0;
for (; next->type != OV2680_TOK_TERM; next++) {
switch (next->type & OV2680_TOK_MASK) {
@@ -250,7 +249,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
 * If next address is not consecutive, data needs to be
 * flushed before proceed.
 */
-ov2680_debug(>dev,  "+++ov2680_write_reg_array 
reg=%x->%x\n", next->reg,next->val);
+dev_dbg(>dev,  "+++ov2680_write_reg_array 
reg=%x->%x\n", next->reg,next->val);
if (!__ov2680_write_reg_is_consecutive(client, ,
next)) {
err = __ov2680_flush_reg_array(client, );
@@ -296,7 +295,8 @@ static int ov2680_g_fnumber_range(struct v4l2_subdev *sd, 
s32 *val)
 static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
 {
struct ov2680_device *dev = to_ov2680_sensor(sd);
-   ov2680_debug(dev,  "ov2680_g_bin_factor_x\n");
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   dev_dbg(>dev,  "ov2680_g_bin_factor_x\n");
*val = ov2680_res[dev->fmt_idx].bin_factor_x;
 
return 0;
@@ -305,9 +305,10 @@ static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, 
s32 *val)
 static int ov2680_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
 {
struct ov2680_device *dev = to_ov2680_sensor(sd);
+   struct i2c_client *client = v4l2_get_subdevdata(sd);

*val = ov2680_res[dev->fmt_idx].bin_factor_y;
-   ov2680_debug(dev,  "ov2680_g_bin_factor_y\n");
+   dev_dbg(>dev,  "ov2680_g_bin_factor_y\n");
return 0;
 }
 
@@ -322,7 +323,7 @@ static int ov2680_get_intg_factor(struct i2c_client *client,
unsigned int pix_clk_freq_hz;
u16 reg_val;
int ret;
-   ov2680_debug(dev,  "ov2680_get_intg_factor\n");
+   dev_dbg(>dev,  "ov2680_get_intg_factor\n");
if (!info)
return -EINVAL;
 
@@ -399,7 +400,7 @@ static long __ov2680_set_exposure(struct v4l2_subdev *sd, 
int coarse_itg,
u16 vts,hts;
int ret,exp_val;

-   ov2680_debug(dev, "+++__ov2680_set_exposure coarse_itg %d, gain %d, 
digitgain %d++\n",coarse_itg, gain, digitgain);
+   dev_dbg(>dev, "+++__ov2680_set_exposure coarse_itg %d, gain 
%d, digitgain %d++\n",coarse_itg, gain, digitgain);
 
hts = ov2680_res[dev->fmt_idx].pixels_per_line;
vts = ov2680_res[dev->fmt_idx].lines_per_frame;
@@ -605,7 +606,7 @@ static int 

[PATCH 19/24] staging/atomisp: add PCI dependency

[Alan Cox]

From: Arnd Bergmann 

Without CONFIG_PCI, config space reads never return any data,
leading to undefined behavior that gcc warns about:

platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32_raw':
platform/intel-mid/intel_mid_pcihelpers.c:66:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32_raw_ext':
platform/intel-mid/intel_mid_pcihelpers.c:84:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32':
platform/intel-mid/intel_mid_pcihelpers.c:137:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]

With a dependency on CONFIG_PCI, we don't get this warning. This seems
safe as PCI config space accessors should always return something
when PCI is enabled.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Alan Cox 
---
 drivers/staging/media/atomisp/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index 28615aa..8b172ed 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && EFI && MEDIA_CONTROLLER
+depends on X86 && EFI && MEDIA_CONTROLLER && PCI
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:26:16PM +, Russell King - ARM Linux wrote:
> On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> > From: Hans Verkuil 
> > 
> > Add support for video hotplug detect and EDID/ELD notifiers, which is used
> > to convey information from video drivers to their CEC and audio 
> > counterparts.
> > 
> > Based on an earlier version from Russell King:
> > 
> > https://patchwork.kernel.org/patch/9277043/
> > 
> > The hpd_notifier is a reference counted object containing the HPD/EDID/ELD 
> > state
> > of a video device.
> 
> I thought we had decided to drop the ELD bits?

Ignore that - mailer wrapped around to the first message in my mailbox!

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

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Russell King - ARM Linux]

On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
> 
> Based on an earlier version from Russell King:
> 
> https://patchwork.kernel.org/patch/9277043/
> 
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD 
> state
> of a video device.

I thought we had decided to drop the ELD bits?

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

Re: [PATCH v3 3/6] documentation: media: Add documentation for new RGB and YUV bus formats

[Hans Verkuil]

On 03/07/2017 05:42 PM, Neil Armstrong wrote:
> Add documentation for added Bus Formats to describe RGB and YUS formats used
> as input to the Synopsys DesignWare HDMI TX Controller.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  Documentation/media/uapi/v4l/subdev-formats.rst | 4992 
> ++-
>  1 file changed, 3963 insertions(+), 1029 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst 
> b/Documentation/media/uapi/v4l/subdev-formats.rst
> index d6152c9..feb55b5 100644
> --- a/Documentation/media/uapi/v4l/subdev-formats.rst
> +++ b/Documentation/media/uapi/v4l/subdev-formats.rst
> @@ -114,7 +114,7 @@ The following tables list existing packed RGB formats.
>  .. it switches to long table, and there's no way to override it.
>  
>  
> -.. tabularcolumns:: 
> |p{4.0cm}|p{0.7cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|
> +.. tabularcolumns:: 
> |p{4.0cm}|p{0.7cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|
>  
>  .. _v4l2-mbus-pixelcode-rgb:
>  



> +* .. _MEDIA-BUS-FMT-RGB161616-1X48:
> +
> +  - MEDIA_BUS_FMT_RGB161616_1X48
> +  - 0x10

0x10 -> 0x101a

> +  -
> +  - r\ :sub:`15`



> +* .. _MEDIA-BUS-FMT-YUV16-1X48:
> +
> +  - MEDIA_BUS_FMT_YUV16_1X48
> +  - 0x202a

Needs an extra line:

 -

The first cell is the 'Bit' column, which should be an empty cell.

If you look at the output, then you'll see that without this the bit 0 cell is 
empty.

Same for the other three 48 bit YUV formats.

> +  - y\ :sub:`15`
> +  - y\ :sub:`14`
> +  - y\ :sub:`13`
> +  - y\ :sub:`12`
>- y\ :sub:`11`
>- y\ :sub:`10`
> -  - y\ :sub:`9`
> +  - y\ :sub:`8`

Typo: should remain 9.

>- y\ :sub:`8`
>- y\ :sub:`7`
>- y\ :sub:`6`
> @@ -6203,6 +9124,26 @@ the following codes.
>- y\ :sub:`2`
>- y\ :sub:`1`
>- y\ :sub:`0`
> +  - u\ :sub:`15`
> +  - u\ :sub:`14`
> +  - u\ :sub:`13`
> +  - u\ :sub:`12`
> +  - u\ :sub:`11`
> +  - u\ :sub:`10`
> +  - u\ :sub:`9`
> +  - u\ :sub:`8`
> +  - u\ :sub:`7`
> +  - u\ :sub:`6`
> +  - u\ :sub:`5`
> +  - u\ :sub:`4`
> +  - u\ :sub:`3`
> +  - u\ :sub:`2`
> +  - u\ :sub:`1`
> +  - u\ :sub:`0`
> +  - v\ :sub:`15`
> +  - v\ :sub:`14`
> +  - v\ :sub:`13`
> +  - v\ :sub:`12`
>- v\ :sub:`11`
>- v\ :sub:`10`
>- v\ :sub:`9`
> @@ -6215,29 +9156,14 @@ the following codes.
>- v\ :sub:`2`
>- v\ :sub:`1`
>- v\ :sub:`0`
> -* -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  -
> -  - y\ :sub:`11`
> -  - y\ :sub:`10`
> -  - y\ :sub:`9`
> -  - y\ :sub:`8`
> -  - y\ :sub:`7`
> -  - y\ :sub:`6`
> -  - y\ :sub:`5`
> -  - y\ :sub:`4`
> -  - y\ :sub:`3`
> -  - y\ :sub:`2`
> -  - y\ :sub:`1`
> -  - y\ :sub:`0`
> +* .. _MEDIA-BUS-FMT-UYVY16-1-1X48:
> +
> +  - MEDIA_BUS_FMT_UYVY16_1_1X48
> +  - 0x202b
> +  - u\ :sub:`15`
> +  - u\ :sub:`14`
> +  - u\ :sub:`13`
> +  - u\ :sub:`12`
>- u\ :sub:`11`
>- u\ :sub:`10`
>- u\ :sub:`9`
> @@ -6250,13 +9176,12 @@ the following codes.
>- u\ :sub:`2`
>- u\ :sub:`1`
>- u\ :sub:`0`
> -* .. _MEDIA-BUS-FMT-YUV10-1X30:
> -
> -  - MEDIA_BUS_FMT_YUV10_1X30
> -  - 0x2016
> -  -
> -  -
> -  -
> +  - y\ :sub:`15`
> +  - y\ :sub:`14`
> +  - y\ :sub:`13`
> +  - y\ :sub:`12`
> +  - y\ :sub:`11`
> +  - y\ :sub:`10`
>- y\ :sub:`9`
>- y\ :sub:`8`
>- y\ :sub:`7`
> @@ -6267,16 +9192,30 @@ the following codes.
>- y\ :sub:`2`
>- y\ :sub:`1`
>- y\ :sub:`0`
> -  - u\ :sub:`9`
> -  - u\ :sub:`8`
> -  - u\ :sub:`7`
> -  - u\ :sub:`6`
> -  - u\ :sub:`5`
> -  - u\ :sub:`4`
> -  - u\ :sub:`3`
> -  - u\ :sub:`2`
> -  - u\ :sub:`1`
> -  - u\ :sub:`0`
> +  - y\ :sub:`15`
> +  - y\ :sub:`14`
> +  - y\ :sub:`13`
> +  - y\ :sub:`12`
> +  - y\ :sub:`11`
> +  - y\ :sub:`10`
> +  - y\ :sub:`8`

8 -> 9

> +  - y\ :sub:`8`
> +  - y\ 

[PATCH 04/24] atomisp: remove another layer of allocator indirection

[Alan Cox]

Our driver only ever uses one set of routines for the allocators used by the 
CSS layer to
manage memory and the memory management on the ISP. We can thus remove the 
function vectors
and simply call the intended routines directly.

These routines in turn are simply wrappers around another layer of code so 
remove this
second layer of wrappers and call the hrt methods directly. In time we can 
remove this layer
of indirection as well.

Signed-off-by: Alan Cox 
---
 .../atomisp/pci/atomisp2/atomisp_compat_css20.c|   66 
 .../ia_css_isp_states.h|1 
 .../ia_css_isp_states.h|1 
 .../ia_css_isp_states.h|1 
 .../atomisp/pci/atomisp2/css2400/ia_css_env.h  |   48 ---
 .../pci/atomisp2/css2400/ia_css_memory_access.c|   64 +++
 .../pci/atomisp2/css2400/ia_css_memory_access.h|   24 ---
 .../css2400/isp/kernels/bh/bh_2/ia_css_bh.host.c   |1 
 .../raw_aa_binning_1.0/ia_css_raa.host.c   |1 
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c|4 -
 10 files changed, 35 insertions(+), 176 deletions(-)
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_memory_access.h

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 760f06d..2e20a81 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -92,65 +92,6 @@ unsigned int atomisp_css_debug_get_dtrace_level(void)
return ia_css_debug_trace_level;
 }
 
-static ia_css_ptr atomisp_css2_mm_alloc(size_t bytes, uint32_t attr)
-{
-   if (attr & IA_CSS_MEM_ATTR_ZEROED) {
-   if (attr & IA_CSS_MEM_ATTR_CACHED) {
-   if (attr & IA_CSS_MEM_ATTR_CONTIGUOUS)
-   return (ia_css_ptr) 
hrt_isp_css_mm_calloc_contiguous(bytes);
-   else
-   return (ia_css_ptr) 
hrt_isp_css_mm_calloc_cached(bytes);
-   } else {
-   if (attr & IA_CSS_MEM_ATTR_CONTIGUOUS)
-   return (ia_css_ptr) 
hrt_isp_css_mm_calloc_contiguous(bytes);
-   else
-   return (ia_css_ptr) 
hrt_isp_css_mm_calloc(bytes);
-   }
-   } else {
-   if (attr & IA_CSS_MEM_ATTR_CACHED) {
-   if (attr & IA_CSS_MEM_ATTR_CONTIGUOUS)
-   return (ia_css_ptr) 
hrt_isp_css_mm_alloc_contiguous(bytes);
-   else
-   return (ia_css_ptr) 
hrt_isp_css_mm_alloc_cached(bytes);
-   } else {
-   if (attr & IA_CSS_MEM_ATTR_CONTIGUOUS)
-   return (ia_css_ptr) 
hrt_isp_css_mm_alloc_contiguous(bytes);
-   else
-   return (ia_css_ptr) hrt_isp_css_mm_alloc(bytes);
-   }
-   }
-}
-
-static void atomisp_css2_mm_free(ia_css_ptr ptr)
-{
-   hrt_isp_css_mm_free(ptr);
-}
-
-static int atomisp_css2_mm_load(ia_css_ptr ptr, void *data, size_t bytes)
-{
-   return hrt_isp_css_mm_load(ptr, data, bytes);
-}
-
-static int atomisp_css2_mm_store(ia_css_ptr ptr, const void *data, size_t 
bytes)
-{
-   return hrt_isp_css_mm_store(ptr, data, bytes);
-}
-
-static int atomisp_css2_mm_set(ia_css_ptr ptr, int c, size_t bytes)
-{
-   return hrt_isp_css_mm_set(ptr, c, bytes);
-}
-
-static ia_css_ptr atomisp_css2_mm_mmap(const void *ptr, const size_t size,
-  uint16_t attribute, void *context)
-{
-   struct hrt_userbuffer_attr *userbuffer_attr = context;
-   return hrt_isp_css_mm_alloc_user_ptr(
-   size, (void *)ptr, userbuffer_attr->pgnr,
-   userbuffer_attr->type,
-   attribute & HRT_BUF_FLAG_CACHED);
-}
-
 void atomisp_css2_hw_store_8(hrt_address addr, uint8_t data)
 {
unsigned long flags;
@@ -985,13 +926,6 @@ int atomisp_css_load_firmware(struct atomisp_device *isp)
isp->css_env.isp_css_env.cpu_mem_env.alloc = atomisp_kernel_zalloc;
isp->css_env.isp_css_env.cpu_mem_env.free = atomisp_kernel_free;
 
-   isp->css_env.isp_css_env.css_mem_env.alloc = atomisp_css2_mm_alloc;
-   isp->css_env.isp_css_env.css_mem_env.free = atomisp_css2_mm_free;
-   isp->css_env.isp_css_env.css_mem_env.load = atomisp_css2_mm_load;
-   isp->css_env.isp_css_env.css_mem_env.store = atomisp_css2_mm_store;
-   isp->css_env.isp_css_env.css_mem_env.set = atomisp_css2_mm_set;
-   isp->css_env.isp_css_env.css_mem_env.mmap = atomisp_css2_mm_mmap;
-
isp->css_env.isp_css_env.hw_access_env.store_8 =

[PATCH 03/24] atomisp: remove the unused debug wrapping from the mmgr layer

[Alan Cox]

We don't need this layer of indirection and the debugging information is not 
used. With
this removed we can then go on to try and remove the abstraction layer entirely.

Signed-off-by: Alan Cox 
---
 .../memory_access/memory_access.h  |   80 +++-
 .../pci/atomisp2/css2400/ia_css_memory_access.c|   30 ++--
 2 files changed, 20 insertions(+), 90 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/memory_access/memory_access.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/memory_access/memory_access.h
index e78d462..54ab3d9 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/memory_access/memory_access.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/memory_access/memory_access.h
@@ -1,6 +1,6 @@
 /*
  * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
+ * Copyright (c) 2015-2017, Intel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -74,7 +74,7 @@
  * within the allocation referencable from the
  * returned pointer/address.
  */
-#define MMGR_ATTRIBUTE_MASK0x000f
+#define MMGR_ATTRIBUTE_MASK0x000f
 #define MMGR_ATTRIBUTE_CACHED  0x0001
 #define MMGR_ATTRIBUTE_CONTIGUOUS  0x0002
 #define MMGR_ATTRIBUTE_PAGEALIGN   0x0004
@@ -87,78 +87,43 @@
 extern const hrt_vaddress  mmgr_NULL;
 extern const hrt_vaddress  mmgr_EXCEPTION;
 
-/*! Set the (sub)system virtual memory page table base address
-
- \parambase_addr[in]   The address where page table 0 is 
located
-
- \Note: The base_addr is an absolute system address, thus it is not
-relative to the DDR base address
-
- \return none,
- */
-extern void mmgr_set_base_address(
-   const sys_address   base_addr);
-
 /*! Return the address of an allocation in memory
 
- \paramsize[in]Size in bytes of the allocation
+ \paramsize[in]Size in bytes of the allocation
  \paramcaller_func[in] Caller function name
  \paramcaller_line[in] Caller function line number
 
  \return vaddress
  */
-#define mmgr_malloc(__size) mmgr_malloc_ex(__size, __func__, __LINE__)
-extern hrt_vaddress mmgr_malloc_ex(
-   const size_tsize,
-   const char  *caller_func,
-   int caller_line);
+extern hrt_vaddress mmgr_malloc(const size_t size);
 
 /*! Return the address of a zero initialised allocation in memory
 
  \paramN[in]   Horizontal dimension of array
  \paramsize[in]Vertical dimension of array  Total size 
is N*size
- \paramcaller_func[in] Caller function name
- \paramcaller_line[in] Caller function line number
 
  \return vaddress
  */
-#define mmgr_calloc(__N, __size) mmgr_calloc_ex(__N, __size, __func__, 
__LINE__)
-extern hrt_vaddress mmgr_calloc_ex(
-   const size_tN,
-   const size_tsize,
-   const char  *caller_func,
-   int caller_line);
+extern hrt_vaddress mmgr_calloc(const size_t N, const size_t size);
 
 /*! Free the memory allocation identified by the address
 
  \paramvaddr[in]   Address of the allocation
- \paramcaller_func[in] Caller function name
- \paramcaller_line[in] Caller function line number
 
  \return vaddress
  */
-#define mmgr_free(__vaddr) mmgr_free_ex(__vaddr, __func__, __LINE__)
-extern void mmgr_free_ex(
-   hrt_vaddressvaddr,
-   const char  *caller_func,
-   int caller_line);
+extern void mmgr_free(hrt_vaddress vaddr);
 
 /*! Return the address of an allocation in memory
 
  \paramsize[in]Size in bytes of the allocation
  \paramattribute[in]   Bit vector specifying the properties
of the allocation including zero initialisation
- \paramcaller_func[in] Caller function name
- \paramcaller_line[in] Caller function line number
 
  \return vaddress
  */
-#define mmgr_alloc_attr(__size, __attribute) mmgr_alloc_attr_ex(__size, 
__attribute, __func__, __LINE__)
-extern hrt_vaddress mmgr_alloc_attr_ex(
-   const size_tsize,
-   const uint16_t  attribute,
-   const char  *caller_func,
-   int caller_line);
+
+extern hrt_vaddress mmgr_alloc_attr(const 

[PATCH 02/24] atomisp: remove aa kernel wrappers

[Alan Cox]

The aa kernel is used but it consists of nothing more than a set of wrappers
for a memset and an assignment. Replace these at the calling points with the
memset and assignment.

Keep the structures for now - those should disappear as the next layer up
gets unwrapped.

Signed-off-by: Alan Cox 
---
 .../ia_css_isp_params.c|   29 +
 .../ia_css_isp_states.c|7 +---
 .../ia_css_isp_params.c|   27 
 .../ia_css_isp_states.c|8 +
 .../ia_css_isp_params.c|   27 
 .../ia_css_isp_states.c|8 +
 .../css2400/isp/kernels/aa/aa_2/ia_css_aa2.host.c  |   34 
 .../css2400/isp/kernels/aa/aa_2/ia_css_aa2.host.h  |   23 --
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c   |2 -
 9 files changed, 26 insertions(+), 139 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
index 8a35750..9620bc3 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
@@ -70,27 +70,16 @@ ia_css_process_aa(
const struct ia_css_pipeline_stage *stage,
struct ia_css_isp_parameters *params)
 {
-   assert(params != NULL);
-
-   {
-   unsigned size   = 
stage->binary->info->mem_offsets.offsets.param->dmem.aa.size;
-
-   unsigned offset = 
stage->binary->info->mem_offsets.offsets.param->dmem.aa.offset;
-
-   if (size) {
-   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, 
"ia_css_process_aa() enter:\n");
-
-   ia_css_aa_encode((struct sh_css_isp_aa_params *)
-   
>binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_DMEM].address[offset],
-   >aa_config,
-size);
-   params->isp_params_changed = true;
-   
params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_DMEM] = 
true;
-
-   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, 
"ia_css_process_aa() leave:\n");
-   }
+   unsigned size   = 
stage->binary->info->mem_offsets.offsets.param->dmem.aa.size;
+   unsigned offset = 
stage->binary->info->mem_offsets.offsets.param->dmem.aa.offset;
 
+   if (size) {
+   struct sh_css_isp_aa_params *t =  (struct sh_css_isp_aa_params 
*)
+   
>binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_DMEM].address[offset];
+   t->strength = params->aa_config.strength;
}
+   params->isp_params_changed = true;
+   
params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_DMEM] = 
true;
 }
 
 /* Code generated by genparam/gencode.c:gen_process_function() */
@@ -2214,7 +2203,6 @@ ia_css_get_aa_config(const struct ia_css_isp_parameters 
*params,
*config = params->aa_config;
 
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "ia_css_get_aa_config() 
leave\n");
-   ia_css_aa_debug_dtrace(config, IA_CSS_DEBUG_TRACE);
 }
 
 /* Code generated by genparam/gencode.c:gen_set_function() */
@@ -2228,7 +2216,6 @@ ia_css_set_aa_config(struct ia_css_isp_parameters *params,
 
assert(params != NULL);
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_set_aa_config() 
enter:\n");
-   ia_css_aa_debug_dtrace(config, IA_CSS_DEBUG_TRACE);
params->aa_config = *config;
params->config_changed[IA_CSS_AA_ID] = true;
 #ifndef ISP2401
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.c
index 471ceba..fb3ba08 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.c
@@ -31,11 +31,8 @@ ia_css_initialize_aa_state(
 
unsigned offset = 
binary->info->mem_offsets.offsets.state->vmem.aa.offset;
 
-   if (size) {
-   ia_css_init_aa_state(
-   
>mem_params.params[IA_CSS_PARAM_CLASS_STATE][IA_CSS_ISP_VMEM].address[offset],
-   size);
-   }
+   if (size)
+ 

[PATCH 01/24] atomisp: remove the iefd2 kernel

[Alan Cox]

While this is included and the headers pulled in nothing actually uses this
functionality in the driver, so remove it.

Signed-off-by: Alan Cox 
---
 .../staging/media/atomisp/pci/atomisp2/Makefile|3 
 .../ia_css_isp_params.c|1 
 .../ia_css_isp_states.h|1 
 .../ia_css_isp_params.c|1 
 .../ia_css_isp_states.h|1 
 .../ia_css_isp_params.c|1 
 .../ia_css_isp_states.h|1 
 .../isp/kernels/iefd2_6/ia_css_iefd2_6.host.c  |  200 
 .../isp/kernels/iefd2_6/ia_css_iefd2_6.host.h  |   46 -
 .../kernels/iefd2_6/ia_css_iefd2_6_default.host.c  |  144 --
 .../kernels/iefd2_6/ia_css_iefd2_6_default.host.h  |   23 --
 .../isp/kernels/iefd2_6/ia_css_iefd2_6_param.h |   83 
 .../isp/kernels/iefd2_6/ia_css_iefd2_6_state.h |   32 ---
 .../isp/kernels/iefd2_6/ia_css_iefd2_6_types.h |  164 
 14 files changed, 701 deletions(-)
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6.host.c
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6.host.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_default.host.c
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_default.host.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_param.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_state.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_types.h

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/Makefile 
b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
index f538e56..162bcbf 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
@@ -56,8 +56,6 @@ atomisp-objs += \
css2400/isp/kernels/macc/macc_1.0/ia_css_macc.host.o \
css2400/isp/kernels/macc/macc_1.0/ia_css_macc_table.host.o \
css2400/isp/kernels/csc/csc_1.0/ia_css_csc.host.o \
-   css2400/isp/kernels/iefd2_6/ia_css_iefd2_6_default.host.o \
-   css2400/isp/kernels/iefd2_6/ia_css_iefd2_6.host.o \
css2400/isp/kernels/bnr/bnr_1.0/ia_css_bnr.host.o \
css2400/isp/kernels/bnr/bnr2_2/ia_css_bnr2_2.host.o \
css2400/isp/kernels/dpc2/ia_css_dpc2.host.o \
@@ -274,7 +272,6 @@ INCLUDES += \
-I$(atomisp)/css2400/isp/kernels/gc/gc_1.0/ \
-I$(atomisp)/css2400/isp/kernels/gc/gc_2/ \
-I$(atomisp)/css2400/isp/kernels/hdr/ \
-   -I$(atomisp)/css2400/isp/kernels/iefd2_6/ \
-I$(atomisp)/css2400/isp/kernels/io_ls/ \
-I$(atomisp)/css2400/isp/kernels/io_ls/bayer_io_ls/ \
-I$(atomisp)/css2400/isp/kernels/io_ls/common/ \
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
index 744e56e..8a35750 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_params.c
@@ -51,7 +51,6 @@
 #include "isp/kernels/ynr/ynr_2/ia_css_ynr2.host.h"
 #include "isp/kernels/fc/fc_1.0/ia_css_formats.host.h"
 #include "isp/kernels/tdf/tdf_1.0/ia_css_tdf.host.h"
-#include "isp/kernels/iefd2_6/ia_css_iefd2_6.host.h"
 #include "isp/kernels/dpc2/ia_css_dpc2.host.h"
 #include "isp/kernels/eed1_8/ia_css_eed1_8.host.h"
 #include "isp/kernels/bnlm/ia_css_bnlm.host.h"
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.h
index d658a00..939dc36 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hive_isp_css_2400_system_generated/ia_css_isp_states.h
@@ -22,7 +22,6 @@
 #include "isp/kernels/ref/ref_1.0/ia_css_ref.host.h"
 #include "isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.h"
 #include "isp/kernels/ynr/ynr_1.0/ia_css_ynr.host.h"
-#include "isp/kernels/iefd2_6/ia_css_iefd2_6.host.h"
 #include "isp/kernels/dpc2/ia_css_dpc2.host.h"
 #include "isp/kernels/eed1_8/ia_css_eed1_8.host.h"
 /* Generated code: do not edit or 

Re: [PATCH v10 1/2] Documentation: DT: Add OV5647 bindings

[Vladimir Zapolskiy]

Hi Ramiro,

On 03/06/2017 01:16 PM, Ramiro Oliveira wrote:
> Create device tree bindings documentation.
> 
> Signed-off-by: Ramiro Oliveira 

The device tree binding description looks perfect from my perspective.

Reviewed-by: Vladimir Zapolskiy 

--
With best wishes,
Vladimir

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > The same document says:
> > 
> >   Scaling support is optional. When supported by a subdev, the crop
> >   rectangle on the subdev's sink pad is scaled to the size configured
> >   using the
> >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> >   subdev supports scaling but not composing, the top and left values are
> >   not used and must always be set to zero.
> 
> Right, this sentence does imply that when scaling is supported, there
> must be a sink compose rectangle, even when composing is not.
> 
> I have previously set up scaling like this:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> 
> Does this mean, it should work like this instead?
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 
> "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> 
> I suppose setting the source pad format should not be allowed to modify
> the sink compose rectangle.

That is what I believe having read these documents several times, but
we need v4l2 people to confirm.

Note that setting the format on 'ipu1_csi0':0 should already be done by
the previous media-ctl command, so it should be possible to simplify
that to:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I have tripped over a bug in media-ctl when specifying both a crop and
compose rectangle - the --help output suggests that "," should be used
to separate them.  media-ctl rejects that, telling me the character at
the "," should be "]".  Replacing the "," with " " allows media-ctl to
accept it and set both rectangles, so it sounds like a parser bug - I've
not looked into this any further yet.

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:
> > The above paragraph suggests we skip any rectangles that are not
> > supported. In our case that would be 3. and 4., since the CSI can't
> > compose into a larger frame. I hadn't realised that the crop selection
> > currently happens on the source pad.
> 
> I'd recommend viewing the documentation in its post-processed version,
> because then you get the examples as pictures, and they say that a
> picture is worth 1000 words.  See
> 
>   https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html
> 
> There is almost an exact example of what we're trying to do - it's
> figure 4.6.  Here, we have a sink pad with a cropping rectangle on
> the input, which is then scaled to a composition rectangle (there's
> no bounds rectangle, and it's specified that in such a case the
> top,left of the composition rectangle will always be 0,0 - see quote
> below).
> 
> Where it differs is that the example also supports source cropping
> for two source pads.  We don't support that.
>
> The same document says:
> 
>   Scaling support is optional. When supported by a subdev, the crop
>   rectangle on the subdev's sink pad is scaled to the size configured
>   using the
>   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
>   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
>   subdev supports scaling but not composing, the top and left values are
>   not used and must always be set to zero.

Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.

I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.

regards
Philipp

Re: [PATCH v10 2/2] media: i2c: Add support for OV5647 sensor.

[Vladimir Zapolskiy]

Hi Ramiro,

On 03/06/2017 01:16 PM, Ramiro Oliveira wrote:
> The OV5647 sensor from Omnivision supports up to 2592x1944 @ 15 fps, RAW 8
> and RAW 10 output formats, and MIPI CSI-2 interface.
> 
> The driver adds support for 640x480 RAW 8.
> 
> Signed-off-by: Ramiro Oliveira 

All updates are fine, thank you. Feel free to add my

Reviewed-by: Vladimir Zapolskiy 

> ---
>  MAINTAINERS|   7 +
>  drivers/media/i2c/Kconfig  |  11 +
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ov5647.c | 636 
> +

I see this version has 100 LoC less in comparison to v8, good result.

[snip]

> +
> +static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
> + .enum_mbus_code =   ov5647_enum_mbus_code,

Nitpicking, above it's better to swap tab and space symbols around '='.

> +};
> +
> +static const struct v4l2_subdev_ops ov5647_subdev_ops = {
> + .core   = _subdev_core_ops,
> + .video  = _subdev_video_ops,
> + .pad= _subdev_pad_ops,
> +};
> +

--
With best wishes,
Vladimir

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

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote:
> On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
> > It's what I have - remember, not everyone is happy to constantly replace
> > their distro packages with random new stuff.
> 
> This is a compliance test, which is continuously developed in tandem with
> new kernel versions. If you are working with an upstream kernel, then you
> should also use the corresponding v4l2-compliance test. What's the point
> of using an old one?
> 
> I will not support driver developers that use an old version of the
> compliance test, that's a waste of my time.

The reason that people may _not_ wish to constantly update v4l-utils
is that it changes the libraries installed on their systems.

So, the solution to that is not to complain about developers not using
the latest version, but instead to de-couple it from the rest of the
package, and provide it as a separate, stand-alone package that doesn't
come with all the extra baggage.

Now, there's two possible answers to that:

1. it depends on the libv4l2 version.  If that's so, then you are
   insisting that people constantly move to the latest libv4l2 because
   of API changes, and those API changes may upset applications they're
   using.  So this isn't really on.

2. it doesn't depend on libv4l2 version, in which case there's no reason
   for it to be packaged with v4l-utils.

The reality is that v4l2-compliance links with libv4l2, so I'm not sure
which it is.  What I am sure of is that I don't want to upgrade libv4l2
on an ad-hoc basis, potentially causing issues with applications.

> >> To test actual streaming you need to provide the -s option.
> >>
> >> Note: v4l2-compliance has been developed for 'regular' video devices,
> >> not MC devices. It may or may not work with the -s option.
> > 
> > Right, and it exists to verify that the establised v4l2 API is correctly
> > implemented.  If the v4l2 API is being offered to user applications,
> > then it must be conformant, otherwise it's not offering the v4l2 API.
> > (That's very much a definition statement in itself.)
> > 
> > So, are we really going to say MC devices do not offer the v4l2 API to
> > userspace, but something that might work?  We've already seen today
> > one user say that they're not going to use mainline because of the
> > crud surrounding MC.
> > 
> 
> Actually, my understanding was that he was stuck on the old kernel code.

Err, sorry, I really don't follow.  Who is "he"?

_I_ was the one who reported the EXPBUF problem.  Your comment makes no
sense.

> In the case of v4l2-compliance, I never had the time to make it work with
> MC devices. Same for that matter of certain memory to memory devices.
> 
> Just like MC devices these too behave differently. They are partially
> supported in v4l2-compliance, but not fully.

It seems you saying that the API provided by /dev/video* for a MC device
breaks the v4l2-compliance tests?

_No one_ has mentioned using v4l2-compliance on the subdevs.

> Complaining about this really won't help. We know it's a problem and unless
> someone (me perhaps?) manages to get paid to work on this it's unlikely to
> change for now.

Like the above comment, your comment makes no sense.  I'm not complaining,
I'm trying to find out the details.

Yes, MC stuff sucks big time right now, the documentation is poor, there's
a lack of understanding on all sides of the issues (which can be seen by
the different opinions that people hold.)  The only way to resolve these
differences is via discussion, and if you're going to start thinking that
everyone is complaining, then there's not going to be any forward progress.

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

Re: [bug report] staging/atomisp: Add support for the Intel IPU v2

[Alan Cox]

>    674  
>    675  /* To avoid owerflows when calling the efivar API */
>    676  if (*out_len > ULONG_MAX)
> 
> This is impossible.  Was UINT_MAX intended?

I am really not sure. The only case I can imagine is 32bit EFI on a
64bit system, but even then I don't see what the check is about at this
point.

Alan

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

[Hans Verkuil]

On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
>> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
>>>
>>>
>>> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
 What did you do with:

 ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, 
 memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
 ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>>
>> This is really a knock-on effect from an earlier issue where the compliance 
>> test
>> didn't detect support for MEMORY_MMAP.
> 
> So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
> With that fixed, I now get:
> 
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
> 
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
> 
> The reason is, if you look at the code, VIDIOC_G_FMT populates a list
> of possible buffer formats "node->valid_buftypes".  If the VIDIOC_G_FMT
> test fails, then node->valid_buftypes is zero.
> 
> This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
> and declare it conformant, without creating any buffers (it can't, it
> doesn't know which formats are supported.)
> 
> This causes node->valid_memorytype to be zero.

It should fail on this and return a more understandable error message.

> 
> We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
> that MMAP is not supported.  The reality is that it _is_ supported, but
> it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
> issue) causes the sequence of tests to fail.

Yeah, you're not the first to complain about this. I plan on fixing this this
week.

> 
>> Always build from the master repo. 1.10 is pretty old.
> 
> It's what I have - remember, not everyone is happy to constantly replace
> their distro packages with random new stuff.

This is a compliance test, which is continuously developed in tandem with
new kernel versions. If you are working with an upstream kernel, then you
should also use the corresponding v4l2-compliance test. What's the point
of using an old one?

I will not support driver developers that use an old version of the
compliance test, that's a waste of my time.

> 
 In any case, it doesn't look like the buffer management is being
 tested at all by v4l2-compliance - we know that gstreamer works, so
 buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
 so I also know that VIDIOC_EXPBUF works there.
>>
>> To test actual streaming you need to provide the -s option.
>>
>> Note: v4l2-compliance has been developed for 'regular' video devices,
>> not MC devices. It may or may not work with the -s option.
> 
> Right, and it exists to verify that the establised v4l2 API is correctly
> implemented.  If the v4l2 API is being offered to user applications,
> then it must be conformant, otherwise it's not offering the v4l2 API.
> (That's very much a definition statement in itself.)
> 
> So, are we really going to say MC devices do not offer the v4l2 API to
> userspace, but something that might work?  We've already seen today
> one user say that they're not going to use mainline because of the
> crud surrounding MC.
> 

Actually, my understanding was that he was stuck on the old kernel code.

In the case of v4l2-compliance, I never had the time to make it work with
MC devices. Same for that matter of certain memory to memory devices.

Just like MC devices these too behave differently. They are partially
supported in v4l2-compliance, but not fully.

Why? NO TIME.

Be glad there *is* a v4l2-compliance test at all! It's really, really useful
already, but it took *years* to develop, little bit by little bit. And yes,
I would really like to update it to fully support codecs and MC devices.
And with a bit of luck I hope to get permission from my boss to work on
this (among others) later in the year.

Complaining about this really won't help. We know it's a problem and unless
someone (me perhaps?) manages to get paid to work on this it's unlikely to
change for now.

Regards,

Hans

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

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
> > 
> > 
> > On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
> >> What did you do with:
> >>
> >> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, 
> >> memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
> >>  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> >> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
> >>  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
> 
> This is really a knock-on effect from an earlier issue where the compliance 
> test
> didn't detect support for MEMORY_MMAP.

So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
With that fixed, I now get:

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

The reason is, if you look at the code, VIDIOC_G_FMT populates a list
of possible buffer formats "node->valid_buftypes".  If the VIDIOC_G_FMT
test fails, then node->valid_buftypes is zero.

This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
and declare it conformant, without creating any buffers (it can't, it
doesn't know which formats are supported.)

This causes node->valid_memorytype to be zero.

We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
that MMAP is not supported.  The reality is that it _is_ supported, but
it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
issue) causes the sequence of tests to fail.

> Always build from the master repo. 1.10 is pretty old.

It's what I have - remember, not everyone is happy to constantly replace
their distro packages with random new stuff.

> >> In any case, it doesn't look like the buffer management is being
> >> tested at all by v4l2-compliance - we know that gstreamer works, so
> >> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
> >> so I also know that VIDIOC_EXPBUF works there.
> 
> To test actual streaming you need to provide the -s option.
> 
> Note: v4l2-compliance has been developed for 'regular' video devices,
> not MC devices. It may or may not work with the -s option.

Right, and it exists to verify that the establised v4l2 API is correctly
implemented.  If the v4l2 API is being offered to user applications,
then it must be conformant, otherwise it's not offering the v4l2 API.
(That's very much a definition statement in itself.)

So, are we really going to say MC devices do not offer the v4l2 API to
userspace, but something that might work?  We've already seen today
one user say that they're not going to use mainline because of the
crud surrounding MC.

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

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

[Hans Verkuil]

On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote:
> Em Tue, 14 Mar 2017 08:55:36 +0100
> Hans Verkuil  escreveu:
> 
>> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote:
>>> Hi Sakari,
>>>
>>> I started preparing a long argument about it, but gave up in favor of a
>>> simpler one.
>>>
>>> Em Mon, 13 Mar 2017 14:46:22 +0200
>>> Sakari Ailus  escreveu:
>>>   
 Drivers are written to support hardware, not particular use case.
>>>
>>> No, it is just the reverse: drivers and hardware are developed to
>>> support use cases.
>>>
>>> Btw, you should remember that the hardware is the full board, not just the
>>> SoC. In practice, the board do limit the use cases: several provide a
>>> single physical CSI connector, allowing just one sensor.
>>>   
> This situation is there since 2009. If I remember well, you tried to write
> such generic plugin in the past, but never finished it, apparently because
> it is too complex. Others tried too over the years. 

 I'd argue I know better what happened with that attempt than you do. I had 
 a
 prototype of a generic pipeline configuration library but due to various
 reasons I haven't been able to continue working on that since around 2012. 
  
>>>
>>> ...
>>>   
> The last trial was done by Jacek, trying to cover just the exynos4 
> driver. 
> Yet, even such limited scope plugin was not good enough, as it was never
> merged upstream. Currently, there's no such plugins upstream.
>
> If we can't even merge a plugin that solves it for just *one* driver,
> I have no hope that we'll be able to do it for the generic case.

 I believe Jacek ceased to work on that plugin in his day job; other than
 that, there are some matters left to be addressed in his latest patchset.  
>>>
>>> The two above basically summaries the issue: the task of doing a generic
>>> plugin on userspace, even for a single driver is complex enough to
>>> not cover within a reasonable timeline.
>>>
>>> From 2009 to 2012, you were working on it, but didn't finish it.
>>>
>>> Apparently, nobody worked on it between 2013-2014 (but I may be wrong, as
>>> I didn't check when the generic plugin interface was added to libv4l).
>>>
>>> In the case of Jacek's work, the first patch I was able to find was
>>> written in Oct, 2014:
>>> https://patchwork.kernel.org/patch/5098111/
>>> (not sure what happened with the version 1).
>>>
>>> The last e-mail about this subject was issued in Dec, 2016.
>>>
>>> In summary, you had this on your task for 3 years for an OMAP3
>>> plugin (where you have a good expertise), and Jacek for 2 years, 
>>> for Exynos 4, where he should also have a good knowledge.
>>>
>>> Yet, with all that efforts, no concrete results were achieved, as none
>>> of the plugins got merged.
>>>
>>> Even if they were merged, if we keep the same mean time to develop a
>>> libv4l plugin, that would mean that a plugin for i.MX6 could take 2-3
>>> years to be developed.
>>>
>>> There's a clear message on it:
>>> - we shouldn't keep pushing for a solution via libv4l.  
>>
>> Or:
>>
>>  - userspace plugin development had a very a low priority and
>>never got the attention it needed.
> 
> The end result is the same: we can't count on it.
> 
>>
>> I know that's *my* reason. I rarely if ever looked at it. I always assumed
>> Sakari and/or Laurent would look at it. If this reason is also valid for
>> Sakari and Laurent, then it is no wonder nothing has happened in all that
>> time.
>>
>> We're all very driver-development-driven, and userspace gets very little
>> attention in general. So before just throwing in the towel we should take
>> a good look at the reasons why there has been little or no development: is
>> it because of fundamental design defects, or because nobody paid attention
>> to it?
> 
> No. We should look it the other way: basically, there are patches
> for i.MX6 driver that sends control from videonode to subdevs. 
> 
> If we nack apply it, who will write the userspace plugin? When
> such change will be merged upstream?
> 
> If we don't have answers to any of the above questions, we should not
> nack it.
> 
> That's said, that doesn't prevent merging a libv4l plugin if/when
> someone can find time/interest to develop it.

I don't think this control inheritance patch will magically prevent you
from needed a plugin.

> 
>> I strongly suspect it is the latter.
>>
>> In addition, I suspect end-users of these complex devices don't really care
>> about a plugin: they want full control and won't typically use generic
>> applications. If they would need support for that, we'd have seen much more
>> interest. The main reason for having a plugin is to simplify testing and
>> if this is going to be used on cheap hobbyist devkits.
> 
> What are the needs for a cheap hobbyist devkit owner? Do we currently
> satisfy those needs? I'd say that having a functional 

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

[Philipp Zabel]

On Sun, 2017-03-19 at 12:14 +, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> > On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
> > >0:00:01.955927879 20954  0x15ffe90 INFOv4l2 
> > >gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: 
> > >video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; 
> > >video/x-raw, format=(string)I420, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
> > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, 
> > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
> > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; 
> > >video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
> > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, 
> > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
> > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
> > >
> > >despite the media pipeline actually being configured for 60fps.
> > >
> > >Forcing it by adjusting the pipeline only results in gstreamer
> > >failing, because it believes that v4l2 is unable to operate at
> > >60fps.
> > >
> > >Also note the complaints from v4l2src about the non-compliance...
> > 
> > Thanks, I've fixed most of v4l2-compliance issues, but this is not
> > done yet. Is that something you can help with?
> 
> I've looked at this, and IMHO it's yet another media control API mess.
> 
> - media-ctl itself allows setting the format on subdev pads via
>   struct v4l2_subdev_format.
> 
> - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt.
> 
> - struct v4l2_mbus_framefmt contains:
>   * @width:  frame width
>   * @height: frame height
>   * @code:   data format code (from enum v4l2_mbus_pixelcode)
>   * @field:  used interlacing type (from enum v4l2_field)
>   * @colorspace: colorspace of the data (from enum v4l2_colorspace)
>   * @ycbcr_enc:  YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
>   * @quantization: quantization of the data (from enum v4l2_quantization)
>   * @xfer_func:  transfer function of the data (from enum v4l2_xfer_func)
> 
> - media-ctl sets width, height, code and field, but nothing else.
> 
> We're already agreed that the fields that media-ctl are part of the
> format negotiation between the ultimate source, flowing down to the
> capture device.  However, there's no support in media-ctl to deal
> with these other fields - so media-ctl in itself is only half-
> implemented.

To set and read colorimetry information:
https://patchwork.linuxtv.org/patch/39350/

regards
Philipp

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[walter harms]

Am 20.03.2017 13:51, schrieb DaeSeok Youn:
> 2017-03-20 21:04 GMT+09:00 walter harms :
>>
>>
>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>>> If v4l2_subdev_call() gets the global frame interval values,
>>> it returned 0 and it could be checked whether numerator is zero or not.
>>>
>>> If the numerator is not zero, the fps could be calculated in this function.
>>> If not, it just returns 0.
>>>
>>> Signed-off-by: Daeseok Youn 
>>> ---
>>>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
>>> ++
>>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
>>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> index 8bdb224..6bdd19e 100644
>>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
>>> video_device *dev)
>>>
>>>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device 
>>> *asd)
>>>  {
>>> - struct v4l2_subdev_frame_interval frame_interval;
>>> + struct v4l2_subdev_frame_interval fi;
>>>   struct atomisp_device *isp = asd->isp;
>>> - unsigned short fps;
>>>
>>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> - video, g_frame_interval, _interval)) {
>>> - fps = 0;
>>> - } else {
>>> - if (frame_interval.interval.numerator)
>>> - fps = frame_interval.interval.denominator /
>>> - frame_interval.interval.numerator;
>>> - else
>>> - fps = 0;
>>> - }
>>> + unsigned short fps = 0;
>>> + int ret;
>>> +
>>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> +video, g_frame_interval, );
>>> +
>>> + if (!ret && fi.interval.numerator)
>>> + fps = fi.interval.denominator / fi.interval.numerator;
>>> +
>>>   return fps;
>>>  }
>>
>>
>>
>> do you need to check ret at all ? if an error occurs can 
>> fi.interval.numerator
>> be something else than 0 ?
> the return value from the v4l2_subdev_call() function is zero when it
> is done without any error. and also I checked
> the ret value whether is 0 or not. if the ret is 0 then the value of
> numerator should be checked to avoid for dividing by 0.
>>
>> if ret is an ERRNO it would be wise to return ret not fps, but this may 
>> require
>> changes at other places also.
> hmm.., yes, you are right. but I think it is ok because the
> atomisp_get_sensor_fps() function is needed to get fps value.
> (originally, zero or calculated fps value was returned.)

maybe its better to divide this in:
if (ret)
   return 0; // error case

return (fi.interval.numerator>0)?fi.interval.denominator / 
fi.interval.numerator:0;

So there is a chance that someone will a) understand and b) fix the error 
return.

re,
 wh

> 
>>
>> re,
>>  wh
>>
>>>
> 

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

[Hans Verkuil]

On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote:
> On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote:
>> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote:
>> [...]
>>> The big question, waiting for an answer on the last 8 years is
>>> who would do that? Such person would need to have several different
>>> hardware from different vendors, in order to ensure that it has
>>> a generic solution.
>>>
>>> It is a way more feasible that the Kernel developers that already 
>>> have a certain hardware on their hands to add support inside the
>>> driver to forward the controls through the pipeline and to setup
>>> a "default" pipeline that would cover the common use cases at
>>> driver's probe.
>>
>> Actually, would setting pipeline via libv4l2 plugin and letting drivers
>> provide a sane enabled default pipeline configuration be mutually
>> exclusive? Not sure about the control forwarding, but at least a simple
>> link setup and format forwarding would also be possible in the kernel
>> without hindering userspace from doing it themselves later.
> 
> I think this is the exact same problem as controls in ALSA.
> 
> When ALSA started off in life, the requirement was that all controls
> shall default to minimum, and the user is expected to adjust controls
> after the system is running.
> 
> After OSS, this gave quite a marked change in system behaviour, and
> led to a lot of "why doesn't my sound work anymore" problems, because
> people then had to figure out which combination of controls had to be
> set to get sound out of their systems.
> 
> Now it seems to be much better, where install Linux on a platform, and
> you have a working sound system (assuming that the drivers are all there
> which is generally the case for x86.)
> 
> However, it's still possible to adjust all the controls from userspace.
> All that's changed is the defaults.
> 
> Why am I mentioning this - because from what I understand Mauro saying,
> it's no different from this situation.  Userspace will still have the
> power to disable all links and setup its own.  The difference is that
> there will be a default configuration that the kernel sets up at boot
> time that will be functional, rather than the current default
> configuration where the system is completely non-functional until
> manually configured.
> 
> However, at the end of the day, I don't care _where_ the usability
> problems are solved, only that there is some kind of solution.  It's not
> the _where_ that's the real issue here, but the _how_, and discussion of
> the _how_ is completely missing.
> 
> So, let's try kicking off a discussion about _how_ to do things.
> 
> _How_ do we setup a media controller system so that we end up with a
> usable configuration - let's start with the obvious bit... which links
> should be enabled.
> 
> I think the first pre-requisit is that we stop exposing capture devices
> that can never be functional for the hardware that's present on the board,
> so that there isn't this plentora of useless /dev/video* nodes and useless
> subdevices.
> 
> One possible solution to finding a default path may be "find the shortest
> path between the capture device and the sensor and enable intervening
> links".
> 
> Then we need to try configuring that path with format/resolution
> information.
> 
> However, what if something in the shortest path can't handle the format
> that the sensor produces?  I think at that point, we'd need to drop that
> subdev out of the path resolution, re-run the "find the shortest path"
> algorithm, and try again.
> 
> Repeat until success or no path between the capture and sensor exists.
> 
> This works fine if you have just one sensor visible from a capture device,
> but not if there's more than one (which I suspect is the case with the
> Sabrelite board with its two cameras and video receiver.)  That breaks
> the "find the shortest path" algorithm.
> 
> So, maybe it's a lot better to just let the board people provide via DT
> a default setup for the connectivity of the modules somehow - certainly
> one big step forward would be to disable in DT parts of the capture
> system that can never be used (remembering that boards like the RPi /
> Hummingboard may end up using DT overlays to describe this for different
> cameras, so the capture setup may change after initial boot.)

The MC was developed before the device tree came along. But now that the DT
is here, I think this could be a sensible idea to let the DT provide an
initial path.

Sakari, Laurent, Mauro: any opinions?

Regards,

Hans

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

[Philipp Zabel]

On Sat, 2017-03-18 at 12:58 -0700, Steve Longerbeam wrote:
> 
> On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
> > Hi Steve,
> >
> > I've just been trying to get gstreamer to capture and h264 encode
> > video from my camera at various frame rates, and what I've discovered
> > does not look good.
> >
> > 1) when setting frame rates, media-ctl _always_ calls
> > VIDIOC_SUBDEV_S_FRAME_INTERVAL with pad=0.

To allow setting pad > 0:
https://patchwork.linuxtv.org/patch/39348/

> > 2) media-ctl never retrieves the frame interval information, so there's
> > no way to read it back with standard tools, and no indication that
> > this is going on...
> 
> I think Philipp Zabel submitted a patch which addresses these
> in media-ctl. Check with him.

To read back and propagate the frame interval:
https://patchwork.linuxtv.org/patch/39349/
https://patchwork.linuxtv.org/patch/39351/

regards
Philipp

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

[Hans Verkuil]

On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
> 
> 
> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
>> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>>> Right, imx-media-capture.c (the "standard" v4l2 user interface module)
>>> is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
>>> return the single frame size that the pipeline has configured (the mbus
>>> format of the attached source pad).
>> I now have a set of patches that enumerate the frame sizes and intervals
>> from the source pad of the first subdev (since you're setting the formats
>> etc there from the capture device, it seems sensible to return what it
>> can support.)  This means my patch set doesn't add to non-CSI subdevs.
>>
>>> Can you share your gstreamer pipeline? For now, until
>>> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
>>> does not attempt to specify a frame rate. I use the attached
>>> script for testing, which works for me.
>> Note that I'm not specifying a frame rate on gstreamer - I'm setting
>> the pipeline up for 60fps, but gstreamer in its wisdom is unable to
>> enumerate the frame sizes, and therefore is unable to enumerate the
>> frame intervals (frame intervals depend on frame sizes), so it
>> falls back to the "tvnorms" which are basically 25/1 and 3/1001.
>>
>> It sees 60fps via G_PARM, and then decides to set 3/1001 via S_PARM.
>> So, we end up with most of the pipeline operating at 60fps, with CSI
>> doing frame skipping to reduce the frame rate to 30fps.
>>
>> gstreamer doesn't complain, doesn't issue any warnings, the only way
>> you can spot this is to enable debugging and look through the copious
>> debug log, or use -v and check the pad capabilities.
>>
>> Testing using gstreamer, and only using "does it produce video" is a
>> good simple test, but it's just that - it's a simple test.  It doesn't
>> tell you that what you're seeing is what you intended to see (such as
>> video at the frame rate you expected) without more work.
>>
>>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>>> done yet. Is that something you can help with?
>> What did you do with:
>>
>> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, memory=0 
>> /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
>>  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
>>  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)

This is really a knock-on effect from an earlier issue where the compliance test
didn't detect support for MEMORY_MMAP.

>>  test VIDIOC_EXPBUF: FAIL
>>
>> To me, this looks like a bug in v4l2-compliance (I'm using 1.10.0).

Always build from the master repo. 1.10 is pretty old.

>> I'm not sure what buffer VIDIOC_EXPBUF is expected to export, since
>> afaics no buffers have been allocated, so of course it's going to fail.

It just tests if EXPBUF is supported.

I think I will modify v4l2-compliance to bail out if it doesn't find support
for MEMORY_MMAP. Even though in theory support for this is optional, in practice
all applications expect that it is supported. That should fix this
hard-to-understand error.

>> Either that, or the v4l2 core vb2 code is non-compliant with v4l2's
>> interface requirements.
>>
>> In any case, it doesn't look like the buffer management is being
>> tested at all by v4l2-compliance - we know that gstreamer works, so
>> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
>> so I also know that VIDIOC_EXPBUF works there.

To test actual streaming you need to provide the -s option.

Note: v4l2-compliance has been developed for 'regular' video devices,
not MC devices. It may or may not work with the -s option.

As I think I mentioned somewhere else, creating a compliance test for
MC devices would help enormously in verifying drivers. I'm not sure if
it is better to create a new test or integrate it in v4l2-compliance.

I'm leaning towards the latter since there is a lot of overlap.

>>
> 
> I wouldn't be surprised if you hit on a bug in v4l2-compliance. I 
> stopped with v4l2-compliance
> at a different test failure that also didn't make sense to me:
> 
> Streaming ioctls:
>  test read/write: OK (Not Supported)
>  Video Capture:
>  Buffer: 0 Sequence: 0 Field: Any Timestamp: 41.664259s
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(281): 
> !(g_flags() & (V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR))
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(610): 
> buf.check(q, last_seq)
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(883): 
> captureBufs(node, q, m2m_q, frame_count, false)
>  test MMAP: FAIL
>  test USERPTR: OK (Not Supported)
>  test DMABUF: Cannot test, specify --expbuf-device
> 
> Total: 42, Succeeded: 38, Failed: 

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[DaeSeok Youn]

2017-03-20 21:04 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>> If v4l2_subdev_call() gets the global frame interval values,
>> it returned 0 and it could be checked whether numerator is zero or not.
>>
>> If the numerator is not zero, the fps could be calculated in this function.
>> If not, it just returns 0.
>>
>> Signed-off-by: Daeseok Youn 
>> ---
>>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
>> ++
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> index 8bdb224..6bdd19e 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
>> video_device *dev)
>>
>>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>>  {
>> - struct v4l2_subdev_frame_interval frame_interval;
>> + struct v4l2_subdev_frame_interval fi;
>>   struct atomisp_device *isp = asd->isp;
>> - unsigned short fps;
>>
>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> - video, g_frame_interval, _interval)) {
>> - fps = 0;
>> - } else {
>> - if (frame_interval.interval.numerator)
>> - fps = frame_interval.interval.denominator /
>> - frame_interval.interval.numerator;
>> - else
>> - fps = 0;
>> - }
>> + unsigned short fps = 0;
>> + int ret;
>> +
>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> +video, g_frame_interval, );
>> +
>> + if (!ret && fi.interval.numerator)
>> + fps = fi.interval.denominator / fi.interval.numerator;
>> +
>>   return fps;
>>  }
>
>
>
> do you need to check ret at all ? if an error occurs can fi.interval.numerator
> be something else than 0 ?
the return value from the v4l2_subdev_call() function is zero when it
is done without any error. and also I checked
the ret value whether is 0 or not. if the ret is 0 then the value of
numerator should be checked to avoid for dividing by 0.
>
> if ret is an ERRNO it would be wise to return ret not fps, but this may 
> require
> changes at other places also.
hmm.., yes, you are right. but I think it is ok because the
atomisp_get_sensor_fps() function is needed to get fps value.
(originally, zero or calculated fps value was returned.)

>
> re,
>  wh
>
>>

[PATCH 6/9] staging/atomisp: add PCI dependency

[Arnd Bergmann]

Without CONFIG_PCI, config space reads never return any data,
leading to undefined behavior that gcc warns about:

platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32_raw':
platform/intel-mid/intel_mid_pcihelpers.c:66:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32_raw_ext':
platform/intel-mid/intel_mid_pcihelpers.c:84:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function 
'intel_mid_msgbus_read32':
platform/intel-mid/intel_mid_pcihelpers.c:137:9: error: 'data' is used 
uninitialized in this function [-Werror=uninitialized]

With a dependency on CONFIG_PCI, we don't get this warning. This seems
safe as PCI config space accessors should always return something
when PCI is enabled.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index f7d8a841c629..3af2acdc7e96 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86
+depends on X86 && PCI
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
-- 
2.9.0

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

[Hans Verkuil]

On 03/19/2017 01:14 PM, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>> On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
>>> 0:00:01.955927879 20954  0x15ffe90 INFOv4l2 
>>> gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: 
>>> video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; 
>>> video/x-raw, format=(string)I420, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
>>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, 
>>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
>>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; 
>>> video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
>>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, 
>>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
>>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
>>>
>>>despite the media pipeline actually being configured for 60fps.
>>>
>>>Forcing it by adjusting the pipeline only results in gstreamer
>>>failing, because it believes that v4l2 is unable to operate at
>>>60fps.
>>>
>>>Also note the complaints from v4l2src about the non-compliance...
>>
>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>> done yet. Is that something you can help with?
> 
> I've looked at this, and IMHO it's yet another media control API mess.
> 
> - media-ctl itself allows setting the format on subdev pads via
>   struct v4l2_subdev_format.
> 
> - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt.
> 
> - struct v4l2_mbus_framefmt contains:
>   * @width:  frame width
>   * @height: frame height
>   * @code:   data format code (from enum v4l2_mbus_pixelcode)
>   * @field:  used interlacing type (from enum v4l2_field)
>   * @colorspace: colorspace of the data (from enum v4l2_colorspace)
>   * @ycbcr_enc:  YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
>   * @quantization: quantization of the data (from enum v4l2_quantization)
>   * @xfer_func:  transfer function of the data (from enum v4l2_xfer_func)
> 
> - media-ctl sets width, height, code and field, but nothing else.
> 
> We're already agreed that the fields that media-ctl are part of the
> format negotiation between the ultimate source, flowing down to the
> capture device.  However, there's no support in media-ctl to deal
> with these other fields - so media-ctl in itself is only half-
> implemented.

Correct. The colorspace et al fields are in practice unimportant for sensors.
For HDMI/DP they are very important, though.

It's the reason why nobody worked on adding support for this to media-ctl,
it's almost exclusively used with sensors. Not saying that it is right that
it hasn't been added to media-ctl, just that it never had a high enough prio.

Regards,

Hans

> 
> From what I can tell, _we_ are doing the right thing in imx-media-capture.
> 
> However, I think part of the problem is the set_fmt implementation.
> When a source pad is configured via set_fmt(), any fields that can
> not be altered (eg, because the subdev doesn't support colorspace
> conversion) need to be preserved from the subdev's sink pad.
> 
> Right now, CSI doesn't do that - it only looks at the width, height,
> code, and field.
> 
> I think we've got other bugs though that haven't been picked up by any
> review - csi_try_fmt() adjusts the format using the _current_
> configuration of the sink pad, even when using V4L2_SUBDEV_FORMAT_TRY.
> This seems wrong according to the docs: the purpose of the try
> mechanism is to be able to setup the _entire_ pipeline using the TRY
> mechanism to work out whether the configuration works, before then
> setting for real.  If we're validating the TRY formats against the
> live configuration, then we're not doing that.
> 
> There's calls for:
> 
> v4l2_subdev_get_try_format
> v4l2_subdev_get_try_crop
> v4l2_subdev_get_try_compose
> 
> to get the try configuration - we hardly make use of all of these.  I
> would suggest that we change the approach to implementing the various
> subdevs such that:
> 
> 1) like __csi_get_fmt(), we have accessors that gets a pointer to the
>correct state for the TRY/live settings.
> 
> 2) everywhere we're asked to get or set parameters that can be TRY/live,
>we use these accessors to retrieve a pointer to the correct state to
>not only read, but also modify.
> 
> 3) when we're evaluating parameters against another pad, we use these
>accessors to obtain the other pad's configuration, rather than poking
>about in the state saved in the subdev's priv-> (which is irrelevant
>for the TRY variant.)
> 
> 4) ensure that all 

[PATCH 9/9] staging/atomisp: add EFI dependency

[Arnd Bergmann]

Without CONFIG_EFI, the driver fails to call efivar_entry_get:

drivers/staging/built-in.o: In function `gmin_get_config_var':
(.text+0x1e3b): undefined reference to `efivar_entry_get'

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index f24ae1c8cc90..e0ae0c93f800 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI && ACPI && MEDIA_CONTROLLER
+depends on X86 && PCI && ACPI && EFI && MEDIA_CONTROLLER
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
-- 
2.9.0

[PATCH 8/9] staging/atomisp: add MEDIA_CONTROLLER dependency globally

[Arnd Bergmann]

One i2c driver already gained a dependency, but the others are equally broken:

drivers/staging/media/atomisp/i2c/ap1302.c: In function 'ap1302_remove':
drivers/staging/media/atomisp/i2c/ap1302.c:1143:31: error: 'struct v4l2_subdev' 
has no member named 'entity'
drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_remove':
drivers/staging/media/atomisp/i2c/mt9m114.c:1850:31: error: 'struct 
v4l2_subdev' has no member named 'entity'
drivers/staging/media/atomisp/i2c/gc0310.c: In function 'gc0310_remove':
drivers/staging/media/atomisp/i2c/gc0310.c:1372:31: error: 'struct v4l2_subdev' 
has no member named 'entity'
drivers/staging/media/atomisp/i2c/gc0310.c: In function 'gc0310_probe':
drivers/staging/media/atomisp/i2c/gc0310.c:1422:9: error: 'struct v4l2_subdev' 
has no member named 'entity'
drivers/staging/media/atomisp/i2c/ov2722.c: In function 'ov2722_remove':
drivers/staging/media/atomisp/i2c/ov2722.c:1253:31: error: 'struct v4l2_subdev' 
has no member named 'entity'

Let's just require MEDIA_CONTROLLER for all of them.

Fixes: dd1c0f278b0e ("staging: media: atomisp: fix build error in ov5693 
driver")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/Kconfig| 2 +-
 drivers/staging/media/atomisp/i2c/ov5693/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index 97ffa2fc5384..f24ae1c8cc90 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI && ACPI
+depends on X86 && PCI && ACPI && MEDIA_CONTROLLER
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig 
b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
index 3954b8c65fd1..9fb1bffbe9b3 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
@@ -1,6 +1,6 @@
 config VIDEO_OV5693
tristate "Omnivision ov5693 sensor support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
---help---
  This is a Video4Linux2 sensor-level driver for the Micron
  ov5693 5 Mpixel camera.
-- 
2.9.0

[PATCH 5/9] staging/atomisp: add VIDEO_V4L2_SUBDEV_API dependency

[Arnd Bergmann]

The driver fails to build if this is disabled, so we need an explicit
Kconfig dependency:

drivers/staging/media/atomisp/pci/atomisp2/./atomisp_cmd.c:6085:48: error: 
'struct v4l2_subdev_fh' has no member named 'pad'

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/pci/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/Kconfig 
b/drivers/staging/media/atomisp/pci/Kconfig
index e8f67835d03d..a72421431c7a 100644
--- a/drivers/staging/media/atomisp/pci/Kconfig
+++ b/drivers/staging/media/atomisp/pci/Kconfig
@@ -4,7 +4,7 @@
 
 config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
-   depends on VIDEO_V4L2
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
select VIDEOBUF_VMALLOC
 ---help---
   Say Y here if your platform supports Intel Atom SoC
-- 
2.9.0

[PATCH 7/9] staging/atomisp: add ACPI dependency

[Arnd Bergmann]

Without ACPI, some of the code fails to build:

media/atomisp/platform/intel-mid/atomisp_gmin_platform.c: In function 
'atomisp_register_i2c_module':
media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:174:7: error: 
dereferencing pointer to incomplete type 'struct acpi_device'

We could work around that in the code, but since we already have a hard
dependency on x86, adding the ACPI dependency seems to be the easiest
solution.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index 3af2acdc7e96..97ffa2fc5384 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI
+depends on X86 && PCI && ACPI
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
-- 
2.9.0

[PATCH 4/9] staging/atomisp: remove sh_css_lace_stat code

[Arnd Bergmann]

I ran into a build warning on my randconfig build box:

drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c: In function 
'ia_css_lace_statistics_free':
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c:2845:64: 
error: parameter 'me' set but not used [-Werror=unused-but-set-parameter]

It turns out that not only the parameter is unused but the entire function has 
no
caller. Let's just remove it.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 .../staging/media/atomisp/pci/atomisp2/Makefile|  1 -
 .../media/atomisp/pci/atomisp2/css2400/ia_css.h|  1 -
 .../atomisp/pci/atomisp2/css2400/ia_css_buffer.h   |  1 -
 .../pci/atomisp2/css2400/ia_css_lace_stat.h| 37 --
 .../atomisp/pci/atomisp2/css2400/sh_css_internal.h |  1 -
 .../pci/atomisp2/css2400/sh_css_lace_stat.c| 16 --
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c   | 15 -
 7 files changed, 72 deletions(-)
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
 delete mode 100644 
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_lace_stat.c

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/Makefile 
b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
index f538e56ed1a7..463f84cca4d8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
@@ -108,7 +108,6 @@ atomisp-objs += \
css2400/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.o \
css2400/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.o \
css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.o \
-   css2400/sh_css_lace_stat.o \
css2400/sh_css_pipe.o \
css2400/ia_css_device_access.o \
css2400/sh_css_host_data.o \
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
index f67626f5258c..2458b3767c90 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
@@ -42,7 +42,6 @@
 #include "ia_css_stream_format.h"
 #include "ia_css_stream_public.h"
 #include "ia_css_tpg.h"
-#include "ia_css_lace_stat.h"
 #include "ia_css_version.h"
 #include "ia_css_mmu.h"
 #include "ia_css_morph.h"
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
index 26b16f469042..b2ecf3618c15 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
@@ -60,7 +60,6 @@ struct ia_css_buffer {
struct ia_css_isp_3a_statistics  *stats_3a;/**< 3A 
statistics & optionally RGBY statistics. */
struct ia_css_isp_dvs_statistics *stats_dvs;   /**< DVS 
statistics. */
struct ia_css_isp_skc_dvs_statistics *stats_skc_dvs;  /**< SKC 
DVS statistics. */
-   struct ia_css_isp_lace_statistics *stats_lace; /**< LACE 
statistics. */
struct ia_css_frame  *frame;   /**< Frame 
buffer. */
struct ia_css_acc_param  *custom_data; /**< Custom 
buffer. */
struct ia_css_metadata   *metadata;/**< Sensor 
metadata. */
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
deleted file mode 100644
index 6fee1e200a8a..
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-#ifndef __IA_CSS_LACE_STAT_H
-#define __IA_CSS_LACE_STAT_H
-
-/** @file
- * This file contains types used for LACE statistics
- */
-
-struct ia_css_isp_lace_statistics;
-
-/** @brief Allocate mem for the LACE statistics on the ISP
- * @return Pointer to the allocated LACE statistics
- * buffer on the ISP
-*/
-struct ia_css_isp_lace_statistics *ia_css_lace_statistics_allocate(void);
-
-/** @brief Free the ACC LACE statistics memory on the isp
- * @param[in]  me Pointer to the LACE statistics buffer on the
- *   ISP.
- * @return None
-*/
-void ia_css_lace_statistics_free(struct ia_css_isp_lace_statistics *me);
-

[PATCH 1/9] staging/atomisp: include linux/io.h where needed

[Arnd Bergmann]

The plat_clock implementation fails ot build in some configurations:

platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_set_clock_freq':
platform/clock/vlv2_plat_clock.c:88:2: error: implicit declaration of function 
'writel';did you mean 'wrmsrl'? [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:88:12: error: implicit declaration of function 
'readl' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_probe':
platform/clock/vlv2_plat_clock.c:193:13: error: implicit declaration of 
function 'ioremap_nocache' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:193:11: error: assignment makes pointer from 
integer without a cast [-Werror=int-conversion]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_remove':
platform/clock/vlv2_plat_clock.c:209:2: error: implicit declaration of function 
'iounmap' [-Werror=implicit-function-declaration]

This includes the required header file.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c 
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
index a8ca93dbfbb5..25e939c50aef 100644
--- a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
+++ b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
@@ -20,6 +20,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include "../../include/linux/vlv2_plat_clock.h"
-- 
2.9.0

[PATCH 2/9] staging/atomisp: fix empty-body warning

[Arnd Bergmann]

Defining a debug function to nothing causes a warning with an empty block
after if()/else():

drivers/staging/media/atomisp/i2c/ov2680.c: In function 'ov2680_s_stream':
drivers/staging/media/atomisp/i2c/ov2680.c:1208:55: error: suggest braces 
around empty body in an 'else' statement [-Werror=empty-body]

This changes the empty debug statement to dev_dbg(), which by default also
does nothing, but avoids this warning and also checks the format string.
As a side-effect, we can now use dynamic debugging to turn on the
output at runtime.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/atomisp/i2c/ov2680.c | 37 +++---
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c 
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 58d2a075d436..c08dd0b18fbb 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -35,7 +35,6 @@
 
 #include "ov2680.h"
 
-#define ov2680_debug(...) //dev_err(__VA_ARGS__)
 static int h_flag = 0;
 static int v_flag = 0;
 static enum atomisp_bayer_order ov2680_bayer_order_mapping[] = {
@@ -99,7 +98,7 @@ static int ov2680_read_reg(struct i2c_client *client,
*val = be16_to_cpu(*(u16 *)[0]);
else
*val = be32_to_cpu(*(u32 *)[0]);
-   //ov2680_debug(>dev,  "i2c read adr%x = %x\n", reg,*val);
+   //dev_dbg(>dev,  "i2c read adr%x = %x\n", reg,*val);
return 0;
 }
 
@@ -114,7 +113,7 @@ static int ov2680_i2c_write(struct i2c_client *client, u16 
len, u8 *data)
msg.len = len;
msg.buf = data;
ret = i2c_transfer(client->adapter, , 1);
-   //ov2680_debug(>dev,  "+++i2c write reg=%x->%x\n", data[0]*256 
+data[1],data[2]);
+   //dev_dbg(>dev,  "+++i2c write reg=%x->%x\n", data[0]*256 
+data[1],data[2]);
return ret == num_msg ? 0 : -EIO;
 }
 
@@ -235,7 +234,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
const struct ov2680_reg *next = reglist;
struct ov2680_write_ctrl ctrl;
int err;
-   ov2680_debug(>dev,  "write reg array\n");
+   dev_dbg(>dev,  "write reg array\n");
ctrl.index = 0;
for (; next->type != OV2680_TOK_TERM; next++) {
switch (next->type & OV2680_TOK_MASK) {
@@ -250,7 +249,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
 * If next address is not consecutive, data needs to be
 * flushed before proceed.
 */
-ov2680_debug(>dev,  "+++ov2680_write_reg_array 
reg=%x->%x\n", next->reg,next->val);
+dev_dbg(>dev,  "+++ov2680_write_reg_array 
reg=%x->%x\n", next->reg,next->val);
if (!__ov2680_write_reg_is_consecutive(client, ,
next)) {
err = __ov2680_flush_reg_array(client, );
@@ -296,7 +295,8 @@ static int ov2680_g_fnumber_range(struct v4l2_subdev *sd, 
s32 *val)
 static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
 {
struct ov2680_device *dev = to_ov2680_sensor(sd);
-   ov2680_debug(dev,  "ov2680_g_bin_factor_x\n");
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   dev_dbg(>dev,  "ov2680_g_bin_factor_x\n");
*val = ov2680_res[dev->fmt_idx].bin_factor_x;
 
return 0;
@@ -305,9 +305,10 @@ static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, 
s32 *val)
 static int ov2680_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
 {
struct ov2680_device *dev = to_ov2680_sensor(sd);
+   struct i2c_client *client = v4l2_get_subdevdata(sd);

*val = ov2680_res[dev->fmt_idx].bin_factor_y;
-   ov2680_debug(dev,  "ov2680_g_bin_factor_y\n");
+   dev_dbg(>dev,  "ov2680_g_bin_factor_y\n");
return 0;
 }
 
@@ -322,7 +323,7 @@ static int ov2680_get_intg_factor(struct i2c_client *client,
unsigned int pix_clk_freq_hz;
u16 reg_val;
int ret;
-   ov2680_debug(dev,  "ov2680_get_intg_factor\n");
+   dev_dbg(>dev,  "ov2680_get_intg_factor\n");
if (!info)
return -EINVAL;
 
@@ -399,7 +400,7 @@ static long __ov2680_set_exposure(struct v4l2_subdev *sd, 
int coarse_itg,
u16 vts,hts;
int ret,exp_val;

-   ov2680_debug(dev, "+++__ov2680_set_exposure coarse_itg %d, gain %d, 
digitgain %d++\n",coarse_itg, gain, digitgain);
+   dev_dbg(>dev, "+++__ov2680_set_exposure coarse_itg %d, gain 
%d, digitgain %d++\n",coarse_itg, gain, digitgain);
 
hts = ov2680_res[dev->fmt_idx].pixels_per_line;
vts = ov2680_res[dev->fmt_idx].lines_per_frame;
@@ -605,7 +606,7 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value)
int ret;
u16 

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:
> The above paragraph suggests we skip any rectangles that are not
> supported. In our case that would be 3. and 4., since the CSI can't
> compose into a larger frame. I hadn't realised that the crop selection
> currently happens on the source pad.

I'd recommend viewing the documentation in its post-processed version,
because then you get the examples as pictures, and they say that a
picture is worth 1000 words.  See

  https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html

There is almost an exact example of what we're trying to do - it's
figure 4.6.  Here, we have a sink pad with a cropping rectangle on
the input, which is then scaled to a composition rectangle (there's
no bounds rectangle, and it's specified that in such a case the
top,left of the composition rectangle will always be 0,0 - see quote
below).

Where it differs is that the example also supports source cropping
for two source pads.  We don't support that.

The same document says:

  Scaling support is optional. When supported by a subdev, the crop
  rectangle on the subdev's sink pad is scaled to the size configured
  using the
  :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
  using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
  subdev supports scaling but not composing, the top and left values are
  not used and must always be set to zero.

which in itself describes _exactly_ our hardware here as far as the
cropping and scaling the hardware supports.

> Except when composing is not supported. If the sink compose and source
> crop rectangles are not supported, the source pad format takes their
> place in determining the scaling output resolution. At least that's how
> I read the documentation.

This isn't how I read it.  Scaling is the relationship between the size
of the sink crop and sink compose rectangle.  Composition requires that
there be a composition bounds rectangle to define the composition space,
and the top,left of the composition rectangle be adjustable to place
the composition rectangle within that space.

The above quoted paragraph from the documentation backs up my view in
its final sentence - it doesn't say "if the subdev supports scaling
but not composing, there is no composition rectangle" but says that
there _is_ one but its top,left coordinates are always zero.

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

Re: mainline build: 208 builds: 0 failed, 208 passed, 422 warnings (v4.11-rc2-164-gdefc7d752265)

[Will Deacon]

On Wed, Mar 15, 2017 at 09:02:06PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 15, 2017 at 6:53 PM, kernelci.org bot  wrote:
> >
> > mainline build: 208 builds: 0 failed, 208 passed, 422 warnings 
> > (v4.11-rc2-164-gdefc7d752265)
> 
> The last build failure in mainline is gone now, though I don't know
> what fixed it.
> Let's hope this doesn't come back as the cause was apparently a race condition
> in Kbuild that might have stopped triggering.
> 
> > Warnings summary:
> > 409 :1325:2: warning: #warning syscall statx not implemented [-Wcpp]
> 
> The warning triggers for arm, arm64 and mips on every build. I saw a patch
> was posted for asm-generic, which takes care of arm64.
> 
> Catalin and Will: can you take this through the arm64 tree? I don't have
> anything else for asm-generic at the moment.

Yes, I'll pick that up.

Will

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[walter harms]

Am 20.03.2017 11:59, schrieb Daeseok Youn:
> If v4l2_subdev_call() gets the global frame interval values,
> it returned 0 and it could be checked whether numerator is zero or not.
> 
> If the numerator is not zero, the fps could be calculated in this function.
> If not, it just returns 0.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
> ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> index 8bdb224..6bdd19e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
> video_device *dev)
>  
>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>  {
> - struct v4l2_subdev_frame_interval frame_interval;
> + struct v4l2_subdev_frame_interval fi;
>   struct atomisp_device *isp = asd->isp;
> - unsigned short fps;
>  
> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - video, g_frame_interval, _interval)) {
> - fps = 0;
> - } else {
> - if (frame_interval.interval.numerator)
> - fps = frame_interval.interval.denominator /
> - frame_interval.interval.numerator;
> - else
> - fps = 0;
> - }
> + unsigned short fps = 0;
> + int ret;
> +
> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> +video, g_frame_interval, );
> +
> + if (!ret && fi.interval.numerator)
> + fps = fi.interval.denominator / fi.interval.numerator;
> +
>   return fps;
>  }



do you need to check ret at all ? if an error occurs can fi.interval.numerator
be something else than 0 ?

if ret is an ERRNO it would be wise to return ret not fps, but this may require
changes at other places also.

re,
 wh

>  

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[Dan Carpenter]

Also fix your From header.

regards,
dan carpenter

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[Dan Carpenter]

The subject is too long.

On Mon, Mar 20, 2017 at 10:02:33PM +1100, unknown wrote:
> Signed-off-by: Eddie Youseph 

You need to have a changelog.

regards,
dan carpenter

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Stefan Wahren]

Hi Mauro,


Am 20.03.2017 um 11:58 schrieb Mauro Carvalho Chehab:

Em Sun, 19 Mar 2017 22:11:07 -0300
Mauro Carvalho Chehab  escreveu:


Em Sun, 19 Mar 2017 10:04:28 -0700
Michael Zoran  escreveu:


A working DT that I tried this morning with the current firmware is
posted here:
http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/005924
.html

It even works with minecraft_pi!

With the new firmware, sometime after booting, I'm getting an oops, caused
by bcm2835_audio/vchiq:

[  298.788995] Unable to handle kernel NULL pointer dereference at virtual 
address 0034
[  298.821458] pgd = ed004000
[  298.832294] [0034] *pgd=2e5e9835, *pte=, *ppte=
[  298.857450] Internal error: Oops: 17 [#1] SMP ARM
[  298.876273] Modules linked in: cfg80211 hid_logitech_hidpp hid_logitech_dj 
snd_bcm2835(C) snd_pcm snd_timer snd soundcore vchiq(C) uio_pdrv_genirq uio fuse
[  298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: G C  
4.11.0-rc1+ #56
[  298.963774] Hardware name: Generic DT based system
[  298.982945] task: ef758580 task.stack: ee4c6000
[  299.001080] PC is at mutex_lock+0x14/0x3c
[  299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 [vchiq]
[  299.042246] pc : []lr : []psr: 4013
sp : ee4c7ca8  ip :   fp : ef709800
[  299.088240] r10:   r9 : ee3bffc0  r8 : 0034
[  299.109153] r7 : 0003  r6 :   r5 : ee4c7d00  r4 : ee1d8c00
[  299.135291] r3 : ef758580  r2 :   r1 : ffc8  r0 : 0034
[  299.161431] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  299.190008] Control: 10c5383d  Table: 2d00406a  DAC: 0051
[  299.213011] Process pulseaudio (pid: 847, stack limit = 0xee4c6220)
[  299.238104] Stack: (0xee4c7ca8 to 0xee4c8000)
[  299.255539] 7ca0:   c1403d54 80400040 ff7f0600 ff7f0660 
bf06b578 ee3bffc0
[  299.288301] 7cc0:  ee3afd00  ee4c7d00  bf0640b4 
 bf066428
[  299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 ee3af444 
ee3bffc0 bf0662ec
[  299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 ee3af440 
 bf0d7408
[  299.386587] 7d20: ee79bd80 bf0d5a04  ef709800 0020 0002 
0001 41554453
[  299.419349] 7d40:    bf0d559c ee3af440 0001 
0001 
[  299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00  ee79bd80 ee24ace8 
0001 bf0d4dfc
[  299.484872] 7d80: 000b  ee4b8c3c  ee4c7dc8 ee4b8800 
ee4b8c28 ee4c6000
[  299.517635] 7da0:  ee4b8c3c ed029e40 bf0c0404 ee4b8800 ee1c4a00 
ee4b8800 ed029e40
[  299.550398] 7dc0:  bf0c0560 ee072340  ef758580 c0367b7c 
ee4b8c40 ee4b8c40
[  299.583161] 7de0:  ee4b8800 ed029e40 ee318f80 ed029e40 0006 
ee318f80 bf0c0748
[  299.615924] 7e00: bf0a3430 ee4f6180  c0428fe0 ee318f80 21b0 
0026 ed029e40
[  299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 c0421cc0 
ee4c7ed0 
[  299.681464] 7e40: 0802  ee4c7e94 0006 ee318f80 c0432c8c 
ee4c7f40 c0433bc0
[  299.714225] 7e60:  ed029e40  0041  ed004000 
 ee4c6000
[  299.746987] 7e80: eec69808 0005  0002 ee318f80 ef0d2910 
ee924908 bf0ba284
[  299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 0001 
f000 c0308b04
[  299.812512] 7ec0: ee4c6000  bebb5710 c0434578 ef0d2910 ee924908 
73541c18 0008
[  299.845274] 7ee0: ee4a7019   ee899bb0 ee318f80 0101 
0002 0084
[  299.878037] 7f00:    ee4c7f10 ee318df8 ed029840 
40045532 bebb53c4
[  299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 ee5eca00 
0020 ee5eca18
[  299.943562] 7f40: ee4a7000  00080802 0002 ff9c f000 
0011 ff9c
[  299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 0802 
ed02 0006
[  300.009086] 7f80: 0100 0001   0004 b189d000 
0005 c0308b04
[  300.041848] 7fa0: ee4c6000 c0308940  0004 bebb550c 00080802 
bebb53c4 00084b58
[  300.074611] 7fc0:  0004 b189d000 0005  bebb550c 
00099448 bebb5710
[  300.107373] 7fe0:  bebb53c8 b6c40da4 b6c24334 8010 bebb550c 
2fffd861 2fffdc61
[  300.140190] [] (mutex_lock) from [] 
(vchiq_add_service_internal+0x138/0x3a0 [vchiq])
[  300.178237] [] (vchiq_add_service_internal [vchiq]) from 
[] (vchiq_open_service+0x58/0xf0 [vchiq])
[  300.221152] [] (vchiq_open_service [vchiq]) from [] 
(vchi_service_open+0x74/0xa8 [vchiq])
[  300.260919] [] (vchi_service_open [vchiq]) from [] 
(bcm2835_audio_open+0xe8/0x2d0 [snd_bcm2835])
[  300.303111] [] (bcm2835_audio_open [snd_bcm2835]) from 
[] (snd_bcm2835_playback_open_generic+0xc0/0x1c4 [snd_bcm2835])
[  300.352975] [] (snd_bcm2835_playback_open_generic [snd_bcm2835]) from 
[] (snd_pcm_open_substream+0x60/0x110 [snd_pcm])

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Sun, 2017-03-19 at 12:08 -0700, Steve Longerbeam wrote:
> 
> On 03/19/2017 08:22 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 09, 2017 at 08:53:18PM -0800, Steve Longerbeam wrote:
> >> From: Philipp Zabel 
> >>
> >> The csi_try_crop call in set_fmt should compare the cropping rectangle
> >> to the currently set input format, not to the previous input format.
> > Are we really sure that the cropping support is implemented correctly?
> >
> > I came across this while looking at what we're doing with the
> > V4L2_SEL_FLAG_KEEP_CONFIG flag.
> >
> > Documentation/media/uapi/v4l/dev-subdev.rst defines the behaviour of
> > the user API, and "Order of configuration and format propagation" says:
> >
> >The coordinates to a step always refer to the actual size of the
> >previous step. The exception to this rule is the source compose
> >rectangle, which refers to the sink compose bounds rectangle --- if it
> >is supported by the hardware.
> >
> >1. Sink pad format. The user configures the sink pad format. This format
> >   defines the parameters of the image the entity receives through the
> >   pad for further processing.
> >
> >2. Sink pad actual crop selection. The sink pad crop defines the crop
> >   performed to the sink pad format.
> >
> >3. Sink pad actual compose selection. The size of the sink pad compose
> >   rectangle defines the scaling ratio compared to the size of the sink
> >   pad crop rectangle. The location of the compose rectangle specifies
> >   the location of the actual sink compose rectangle in the sink compose
> >   bounds rectangle.
> >
> >4. Source pad actual crop selection. Crop on the source pad defines crop
> >   performed to the image in the sink compose bounds rectangle.
> >
> >5. Source pad format. The source pad format defines the output pixel
> >   format of the subdev, as well as the other parameters with the
> >   exception of the image width and height. Width and height are defined
> >   by the size of the source pad actual crop selection.
> >
> >Accessing any of the above rectangles not supported by the subdev will
> >return ``EINVAL``. Any rectangle referring to a previous unsupported
> >rectangle coordinates will instead refer to the previous supported
> >rectangle. For example, if sink crop is not supported, the compose
> >selection will refer to the sink pad format dimensions instead.
> >
> > Note step 3 above: scaling is defined by the ratio of the _sink_ crop
> > rectangle to the _sink_ compose rectangle.

The above paragraph suggests we skip any rectangles that are not
supported. In our case that would be 3. and 4., since the CSI can't
compose into a larger frame. I hadn't realised that the crop selection
currently happens on the source pad.
The hardware actually only supports cropping of the input (the crop
rectangle we write into the window registers are before downscaling). So
the crop rectangle should be moved to the sink pad.

> > So, lets say that the camera produces a 1280x720 image, and the sink
> > pad format is configured with 1280x720.  That's step 1.
> >
> > The sink crop operates within that rectangle, cropping it to an area.
> > Let's say we're only interested in its centre, so we'd chose 640x360
> > with the top-left as 320,180.  This is step 2.
>>
> > Then, if we want to down-scale by a factor of two, we'd set the sink
> > compose selection to 320x180.

Except when composing is not supported. If the sink compose and source
crop rectangles are not supported, the source pad format takes their
place in determining the scaling output resolution. At least that's how
I read the documentation.

> > This seems to be at odds with how the scaling is done in CSI at
> > present: the selection implementations all reject attempts to
> > configure the sink pad, instead only supporting crop rectangles on
> > the source,
> 
> Correct. Currently cropping is only supported at the source pad
> (step 4).
> 
> Initially the CSI didn't support down-scaling, so step 3 is not supported,
> so the sink pad format/crop selection rectangle/crop compose rectangle
> are collapsed into the same sink pad format rectangle.
> 
> Philipp later added support for /2 downscaling, but we didn't put this in
> the correct API, looks like this needs to move into the selection API at
> step 3 (sink pad compose rectangle).

I am not sure about this. Wouldn't moving the input crop to the sink pad
be enough? If we added support for the sink pad compose rectangle, that
wouldn't actually allow to compose the CSI output into a larger frame.
Since the subdevice can't compose, I'd leave the sink compose rectangle
disabled.

> >   and we use the source crop rectangle to define the
> > down-scaling.

We use the source pad format to define the downscaling relative to the
source crop rectangle (which is wrong, it should be relative to the sink
crop