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:   Thu Jan  5 05:00:10 CET 2017
media-tree git hash:40eca140c404505c09773d1c6685d818cb55ab1a
media_build git hash:   1606032398b1d79149c1507be2029e1a00d8dff0
v4l-utils git hash: 951c4878a93f4722146f8bc6515a47fba6470bb3
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.8.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: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9-x86_64: ERRORS
apps: WARNINGS
spec-git: ERRORS
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] drm_fourcc: Add new P010, P016 video format

[Ayaka]

從我的 iPad 傳送

> Daniel Stone  於 2017年1月5日 上午1:02 寫道：
> 
> Hi Randy,
> 
>> On 4 January 2017 at 16:29, Randy Li  wrote:
>> index 90d2cc8..23c8e99 100644
>> --- a/drivers/gpu/drm/drm_fourcc.c
>> +++ b/drivers/gpu/drm/drm_fourcc.c
>> @@ -165,6 +165,9 @@ const struct drm_format_info *__drm_format_info(u32 
>> format)
>>{ .format = DRM_FORMAT_UYVY,.depth = 0,  
>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
>>{ .format = DRM_FORMAT_VYUY,.depth = 0,  
>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
>>{ .format = DRM_FORMAT_AYUV,.depth = 0,  
>> .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
>> +   /* FIXME a pixel in Y for P010 is 10 bits */
>> +   { .format = DRM_FORMAT_P010,.depth = 0,  
>> .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> 
> It seems like P010 stores each Y component in a 16-bit value, with the
> bottom 6 bits ignored. So I think cpp here should be 2.
No, the rest bits are used to store the next pixel. The P010 is just a 10 bits 
color depth pixel format.
> 
> Cheers,
> Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] drm_fourcc: Add new P010, P016 video format

[Daniel Stone]

Hi Randy,

On 4 January 2017 at 16:29, Randy Li  wrote:
> index 90d2cc8..23c8e99 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -165,6 +165,9 @@ const struct drm_format_info *__drm_format_info(u32 
> format)
> { .format = DRM_FORMAT_UYVY,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_VYUY,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_AYUV,.depth = 0,  
> .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +   /* FIXME a pixel in Y for P010 is 10 bits */
> +   { .format = DRM_FORMAT_P010,.depth = 0,  
> .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },

It seems like P010 stores each Y component in a 16-bit value, with the
bottom 6 bits ignored. So I think cpp here should be 2.

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] drm_fourcc: Add new P010 video format

[Ville Syrjälä]

On Thu, Jan 05, 2017 at 12:31:27AM +0800, ayaka wrote:
>
>
> On 01/04/2017 11:56 PM, Ville Syrjälä wrote:
> > On Mon, Jan 02, 2017 at 04:50:03PM +0800, Randy Li wrote:
> >> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> >> per channel video format. Rockchip's vop support this
> >> video format(little endian only) as the input video format.
> >>
> >> Signed-off-by: Randy Li 
> >> ---
> >>   include/uapi/drm/drm_fourcc.h | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index 9e1bb7f..d2721da 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -119,6 +119,7 @@ extern "C" {
> >>   #define DRM_FORMAT_NV61  fourcc_code('N', 'V', '6', '1') /* 2x1 
> >> subsampled Cb:Cr plane */
> >>   #define DRM_FORMAT_NV24  fourcc_code('N', 'V', '2', '4') /* 
> >> non-subsampled Cr:Cb plane */
> >>   #define DRM_FORMAT_NV42  fourcc_code('N', 'V', '4', '2') /* 
> >> non-subsampled Cb:Cr plane */
> >> +#define DRM_FORMAT_P010   fourcc_code('P', '0', '1', '0') /* 2x2 
> >> subsampled Cr:Cb plane 10 bits per channel */
> > We could use a better description of the format here. IIRC there is
> > 10bits of actual data contained in each 16bits. So there should be a
> > proper comment explaning in which way the bits are stored.
> It is a little hard to describe P010,

/*
 * 2 plane YCbCr
 * index 0 = Y plane, [15:0] Y:X 10:6 little-endian
 * index 1 = Cr:Cb plane, [31:0] Cr:X:Cb:X 10:6:10:6 little-endian
 */

/*
 * 2 plane YCbCr
 * index 0 = Y plane, [15:0] Y 16 little-endian
 * index 1 = Cr:Cb plane, [31:0] Cr:Cb 16:16 little-endian
 */

or something like that (not 100% sure I got the order of bits and
whatnot correct).

-- 
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] drm_fourcc: Add new P010 video format

[ayaka]

On 01/04/2017 11:56 PM, Ville Syrjälä wrote:

On Mon, Jan 02, 2017 at 04:50:03PM +0800, Randy Li wrote:

P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
per channel video format. Rockchip's vop support this
video format(little endian only) as the input video format.

Signed-off-by: Randy Li 
---
  include/uapi/drm/drm_fourcc.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 9e1bb7f..d2721da 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -119,6 +119,7 @@ extern "C" {
  #define DRM_FORMAT_NV61   fourcc_code('N', 'V', '6', '1') /* 2x1 
subsampled Cb:Cr plane */
  #define DRM_FORMAT_NV24   fourcc_code('N', 'V', '2', '4') /* 
non-subsampled Cr:Cb plane */
  #define DRM_FORMAT_NV42   fourcc_code('N', 'V', '4', '2') /* 
non-subsampled Cb:Cr plane */
+#define DRM_FORMAT_P010fourcc_code('P', '0', '1', '0') /* 2x2 
subsampled Cr:Cb plane 10 bits per channel */

We could use a better description of the format here. IIRC there is
10bits of actual data contained in each 16bits. So there should be a
proper comment explaning in which way the bits are stored.
It is a little hard to describe P010, which leaves a problem cpp 
information in the new patches.
Also I have no idea how to draw the byte-order table the rst document 
for v4l2.


  
  /*

   * 3 plane YCbCr
--
2.7.4

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


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] Add pixel formats for 10/16 bits YUV video

[Randy Li]

Those pixel formats mainly comes from Gstreamer and ffmpeg. Currently,
the VOP(video output mixer) found on RK3288 and future support
those 10 bits pixel formats are input. Also the decoder on RK3288
could use them as output.

The fourcc is not enough to store the endian information for those
pixel formats in v4l2, as it doesn't have a flag like drm does.

I have not met the usage of those 16 bits per-channel format,
it is a very large color range, even the DSLR only use 12 bits.

Randy Li (2):
  drm_fourcc: Add new P010, P016 video format
  [media] v4l: Add 10/16-bits per channel YUV pixel formats

 Documentation/media/uapi/v4l/pixfmt-p010.rst  |  86 
 Documentation/media/uapi/v4l/pixfmt-p010m.rst |  94 ++
 Documentation/media/uapi/v4l/pixfmt-p016.rst  | 126 
 Documentation/media/uapi/v4l/pixfmt-p016m.rst | 136 ++
 drivers/gpu/drm/drm_fourcc.c  |   3 +
 include/uapi/drm/drm_fourcc.h |   2 +
 include/uapi/linux/videodev2.h|   4 +
 7 files changed, 451 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p010.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p010m.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p016.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p016m.rst

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] [media] v4l: Add 10/16-bits per channel YUV pixel formats

[Randy Li]

The formats added by this patch are:
V4L2_PIX_FMT_P010
V4L2_PIX_FMT_P010M
V4L2_PIX_FMT_P016
V4L2_PIX_FMT_P016M
Currently, none of driver uses those format, but some video device
has been confirmed with could as those format for video output.
The Rockchip's new decoder has supported those 10 bits format for
profile_10 HEVC/AVC video.

Signed-off-by: Randy Li 

v4l2
---
 Documentation/media/uapi/v4l/pixfmt-p010.rst  |  86 
 Documentation/media/uapi/v4l/pixfmt-p010m.rst |  94 ++
 Documentation/media/uapi/v4l/pixfmt-p016.rst  | 126 
 Documentation/media/uapi/v4l/pixfmt-p016m.rst | 136 ++
 include/uapi/linux/videodev2.h|   4 +
 5 files changed, 446 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p010.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p010m.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p016.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-p016m.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-p010.rst 
b/Documentation/media/uapi/v4l/pixfmt-p010.rst
new file mode 100644
index 000..82b300c
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-p010.rst
@@ -0,0 +1,86 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-P010:
+
+**
+V4L2_PIX_FMT_P010 ('P010')
+**
+
+
+V4L2_PIX_FMT_P010
+Formats with ½ horizontal and vertical chroma resolution. One luminance and
+one chrominance plane with alternating
+chroma samples as simliar to ``V4L2_PIX_FMT_NV12``
+
+
+Description
+===
+
+It is a two-plane versions of the YUV 4:2:0 format. The three
+components are separated into two sub-images or planes. The Y plane is
+first. The Y plane has 10 bits per pixel. For ``V4L2_PIX_FMT_P010``, a
+combined CbCr plane immediately follows the Y plane in memory. The CbCr
+plane is the same width, in bytes, as the Y plane (and of the image),
+but is half as tall in pixels. Each CbCr pair belongs to four pixels.
+For example, Cb\ :sub:`0`/Cr\ :sub:`0` belongs to Y'\ :sub:`00`,
+Y'\ :sub:`01`, Y'\ :sub:`10`, Y'\ :sub:`11`.
+If the Y plane has pad bytes after each row, then the CbCr plane has as
+many pad bytes after its rows.
+
+
+**Color Sample Location..**
+
+.. flat-table::
+:header-rows:  0
+:stub-columns: 0
+
+* -
+  - 0
+  -
+  - 1
+  - 2
+  -
+  - 3
+* - 0
+  - Y
+  -
+  - Y
+  - Y
+  -
+  - Y
+* -
+  -
+  - C
+  -
+  -
+  - C
+  -
+* - 1
+  - Y
+  -
+  - Y
+  - Y
+  -
+  - Y
+* -
+* - 2
+  - Y
+  -
+  - Y
+  - Y
+  -
+  - Y
+* -
+  -
+  - C
+  -
+  -
+  - C
+  -
+* - 3
+  - Y
+  -
+  - Y
+  - Y
+  -
+  - Y
diff --git a/Documentation/media/uapi/v4l/pixfmt-p010m.rst 
b/Documentation/media/uapi/v4l/pixfmt-p010m.rst
new file mode 100644
index 000..80194a1
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-p010m.rst
@@ -0,0 +1,94 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-P010M:
+
+***
+V4L2_PIX_FMT_P010 ('P010')
+***
+
+
+V4L2_PIX_FMT_P010M
+Variation of ``V4L2_PIX_FMT_P010`` with planes non contiguous in memory.
+
+
+Description
+===
+
+This is a multi-planar, two-plane version of the YUV 4:2:0 format. The
+three components are separated into two sub-images or planes.
+``V4L2_PIX_FMT_P010M`` differs from ``V4L2_PIX_FMT_P010`` in that the
+two planes are non-contiguous in memory, i.e. the chroma plane do not
+necessarily immediately follows the luma plane. The luminance data
+occupies the first plane. The Y plane has one byte per pixel. In the
+second plane there is a chrominance data with alternating chroma
+samples. The CbCr plane is the same width, in bytes, as the Y plane (and
+of the image), but is half as tall in pixels. Each CbCr pair belongs to
+four pixels. For example, Cb\ :sub:`0`/Cr\ :sub:`0` belongs to
+Y'\ :sub:`00`, Y'\ :sub:`01`, Y'\ :sub:`10`, Y'\ :sub:`11`.
+
+``V4L2_PIX_FMT_P010M`` is intended to be used only in drivers and
+applications that support the multi-planar API, described in
+:ref:`planar-apis`.
+
+If the Y plane has pad bytes after each row, then the CbCr plane has as
+many pad bytes after its rows.
+
+**Color Sample Location..**
+
+
+
+.. flat-table::
+:header-rows:  0
+:stub-columns: 0
+
+* -
+  - 0
+  -
+  - 1
+  - 2
+  -
+  - 3
+* - 0
+  - Y
+  -
+  - Y
+  - Y
+  -
+  - Y
+* -
+  -
+  - C
+  -
+  -
+  - C
+  -
+* - 1
+  - Y
+  -
+  - Y
+  - Y
+  -
+

[PATCH v2 1/2] drm_fourcc: Add new P010, P016 video format

[Randy Li]

P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
per channel video format. Rockchip's vop support this
video format(little endian only) as the input video format.

P016 is a planar 4:2:0 YUV with interleaved UV plane, 16 bits
per channel video format.

Signed-off-by: Randy Li 

drm
---
 drivers/gpu/drm/drm_fourcc.c  | 3 +++
 include/uapi/drm/drm_fourcc.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 90d2cc8..23c8e99 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -165,6 +165,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_VYUY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_AYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+   /* FIXME a pixel in Y for P010 is 10 bits */
+   { .format = DRM_FORMAT_P010,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
+   { .format = DRM_FORMAT_P016,.depth = 0,  
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
};
 
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 9e1bb7f..ecc2e05e5 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -119,6 +119,8 @@ extern "C" {
 #define DRM_FORMAT_NV61fourcc_code('N', 'V', '6', '1') /* 2x1 
subsampled Cb:Cr plane */
 #define DRM_FORMAT_NV24fourcc_code('N', 'V', '2', '4') /* 
non-subsampled Cr:Cb plane */
 #define DRM_FORMAT_NV42fourcc_code('N', 'V', '4', '2') /* 
non-subsampled Cb:Cr plane */
+#define DRM_FORMAT_P010fourcc_code('P', '0', '1', '0') /* 2x2 
subsampled Cr:Cb plane 10 bits per channel */
+#define DRM_FORMAT_P016fourcc_code('P', '0', '1', '6') /* 2x2 
subsampled Cr:Cb plane 16 bits per channel */
 
 /*
  * 3 plane YCbCr
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] drm_fourcc: Add new P010 video format

[Ville Syrjälä]

On Mon, Jan 02, 2017 at 04:50:03PM +0800, Randy Li wrote:
> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> per channel video format. Rockchip's vop support this
> video format(little endian only) as the input video format.
> 
> Signed-off-by: Randy Li 
> ---
>  include/uapi/drm/drm_fourcc.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 9e1bb7f..d2721da 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -119,6 +119,7 @@ extern "C" {
>  #define DRM_FORMAT_NV61  fourcc_code('N', 'V', '6', '1') /* 2x1 
> subsampled Cb:Cr plane */
>  #define DRM_FORMAT_NV24  fourcc_code('N', 'V', '2', '4') /* 
> non-subsampled Cr:Cb plane */
>  #define DRM_FORMAT_NV42  fourcc_code('N', 'V', '4', '2') /* 
> non-subsampled Cb:Cr plane */
> +#define DRM_FORMAT_P010  fourcc_code('P', '0', '1', '0') /* 2x2 
> subsampled Cr:Cb plane 10 bits per channel */

We could use a better description of the format here. IIRC there is
10bits of actual data contained in each 16bits. So there should be a
proper comment explaning in which way the bits are stored.

>  
>  /*
>   * 3 plane YCbCr
> -- 
> 2.7.4
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors

[Fabio Estevam]

On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeam  wrote:

> +   camera: ov5642@3c {
> +   compatible = "ovti,ov5642";
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ov5642>;
> +   clocks = < IMX6QDL_CLK_CKO>;
> +   clock-names = "xclk";
> +   reg = <0x3c>;
> +   xclk = <2400>;
> +   DOVDD-supply = <_reg>; /* 1.8v */
> +   AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> +   rev B board is VGEN5 */

Please use vgen3 so that by default we have the valid AVDD-supply for
revC boards which is more recent and more the users have access to.

> +   mipi_camera: ov5640@3c {
> +   compatible = "ovti,ov5640_mipi";
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ov5640>;
> +   reg = <0x3c>;
> +   clocks = < IMX6QDL_CLK_CKO>;
> +   clock-names = "xclk";
> +   xclk = <2400>;
> +   DOVDD-supply = <_reg>; /* 1.8v */
> +   AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> +   rev B board is VGEN5 */

Same here.

> +   pinctrl_ov5640: ov5640grp {
> +   fsl,pins = <
> +   MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000
> +   MX6QDL_PAD_SD1_CLK__GPIO1_IO20  0x8000

Please avoid all the 0x8000 IOMUX settings and replace them by
their real values.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile|   1 +
>  drivers/staging/media/imx/imx-mipi-csi2.c | 509 
> ++
>  2 files changed, 510 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
> b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 000..84df16e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,509 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort the list of headers alphabetically.

> +#include 

Why do you need to include this header?

> +#include 
> +#include "imx-media.h"
> +

[snip]

> +static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> + int i, ret = 0;
> +
> + if (!csi2->src_sd)
> + return -EPIPE;
> + for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
> + if (csi2->sink_sd[i])
> + break;
> + }
> + if (i >= CSI2_NUM_SRC_PADS)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !csi2->stream_on) {
> + clk_prepare_enable(csi2->pix_clk);

It can complicate the design for you, but in general clk_prepare_enable()
can return an error.

> + ret = imxcsi2_dphy_wait(csi2);
> + if (ret)
> + clk_disable_unprepare(csi2->pix_clk);
> + } else if (!enable && csi2->stream_on) {
> + clk_disable_unprepare(csi2->pix_clk);
> + }
> +
> + if (!ret)
> + csi2->stream_on = enable;
> + return ret;
> +}
> +

[snip]

> +
> +static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
> +{
> + struct device_node *node = csi2->dev->of_node;
> + struct device_node *epnode;
> + struct v4l2_of_endpoint ep;
> + int ret = 0;
> +
> + epnode = of_graph_get_next_endpoint(node, NULL);
> + if (!epnode) {
> + v4l2_err(>sd, "failed to get endpoint node\n");
> + return -EINVAL;
> + }
> +
> + v4l2_of_parse_endpoint(epnode, );

Do of_node_put(epnode) here and remove 'out' goto label.

> + if (ep.bus_type != V4L2_MBUS_CSI2) {
> + v4l2_err(>sd, "invalid bus type, must be MIPI CSI2\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + csi2->bus = ep.bus.mipi_csi2;
> +
> + v4l2_info(>sd, "data lanes: %d\n", csi2->bus.num_data_lanes);
> + v4l2_info(>sd, "flags: 0x%08x\n", csi2->bus.flags);
> +out:
> + of_node_put(epnode);
> + return ret;
> +}
> +

[snip]

> +static const struct of_device_id imxcsi2_dt_ids[] = {
> + { .compatible = "fsl,imx-mipi-csi2", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
> +
> +static struct platform_driver imxcsi2_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,

Please drop .owner assignment.

> + .of_match_table = imxcsi2_dt_ids,
> + },
> + .probe = imxcsi2_probe,
> + .remove = imxcsi2_remove,
> +};
> +
> +module_platform_driver(imxcsi2_driver);
> +
> +MODULE_DESCRIPTION("i.MX5/6 MIPI CSI-2 Receiver driver");
> +MODULE_AUTHOR("Steve Longerbeam ");
> +MODULE_LICENSE("GPL");
> +
> 

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 14/19] media: imx: Add Camera Interface subdev driver

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is the camera interface driver that provides the v4l2
> user interface. Frames can be received from various sources:
> 
> - directly from SMFC for capturing unconverted images directly from
>   camera sensors.
> 
> - from the IC pre-process encode task.
> 
> - from the IC pre-process viewfinder task.
> 
> - from the IC post-process task.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile|2 +-
>  drivers/staging/media/imx/imx-camif.c | 1010 
> +
>  2 files changed, 1011 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/staging/media/imx/imx-camif.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index d2a962c..fe9e992 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -8,4 +8,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>  
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
> -
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o

obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o imx-csi.o imx-smfc.o

as an option.

> diff --git a/drivers/staging/media/imx/imx-camif.c 
> b/drivers/staging/media/imx/imx-camif.c
> new file mode 100644
> index 000..3cf167e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-camif.c
> @@ -0,0 +1,1010 @@
> +/*
> + * Video Camera Capture Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort the list of headers alphabetically.

> +#include 
> +#include 
> +#include "imx-media.h"
> +
> +#define DEVICE_NAME "imx-media-camif"

I would propose to drop this macro.

> +
> +#define CAMIF_NUM_PADS 2
> +
> +#define CAMIF_DQ_TIMEOUT5000

Add a comment about time unit?

> +
> +struct camif_priv;
> +

This is a leftover apparently.

> +struct camif_priv {
> + struct device *dev;
> + struct video_devicevfd;
> + struct media_pipeline  mp;
> + struct imx_media_dev  *md;

[snip]

> +static int camif_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct camif_priv *priv;
> + struct video_device *vfd;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> + priv->dev = >dev;
> +
> + pdata = priv->dev->platform_data;
> +
> + mutex_init(>mutex);
> + spin_lock_init(>q_lock);
> +
> + v4l2_subdev_init(>sd, _subdev_ops);
> + v4l2_set_subdevdata(>sd, priv);
> + priv->sd.internal_ops = _internal_ops;
> + priv->sd.entity.ops = _entity_ops;
> + priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->sd.dev = >dev;
> + priv->sd.owner = THIS_MODULE;
> + /* get our group id and camif id */
> + priv->sd.grp_id = pdata->grp_id;
> + priv->id = (pdata->grp_id >> IMX_MEDIA_GRP_ID_CAMIF_BIT) - 1;
> + strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
> + snprintf(camif_videodev.name, sizeof(camif_videodev.name),
> +  "%s devnode", pdata->sd_name);
> +
> + priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> +
> + vfd = >vfd;
> + *vfd = camif_videodev;
> + vfd->lock = >mutex;
> + vfd->queue = >buffer_queue;
> +
> + video_set_drvdata(vfd, priv);
> +
> + v4l2_ctrl_handler_init(>ctrl_hdlr, 0);
> +
> + ret = v4l2_async_register_subdev(>sd);
> + if (ret)
> + goto free_ctrls;
> +
> + return 0;
> +free_ctrls:
> + v4l2_ctrl_handler_free(>ctrl_hdlr);
> + return ret;

A shorter version:

if (ret)
v4l2_ctrl_handler_free(>ctrl_hdlr);

return ret;

> +}
> +
> +static int camif_remove(struct platform_device *pdev)
> +{
> + struct camif_priv *priv =
> + (struct camif_priv *)platform_get_drvdata(pdev);
> +
> + v4l2_ctrl_handler_free(>ctrl_hdlr);
> + v4l2_async_unregister_subdev(>sd);
> + media_entity_cleanup(>sd.entity);
> + v4l2_device_unregister_subdev(>sd);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id camif_ids[] = {
> + { .name = DEVICE_NAME },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, camif_ids);
> +
> +static struct platform_driver imx_camif_driver = {
> + .probe  = camif_probe,
> + 

Re: [PATCH v2 13/19] media: imx: Add IC subdev drivers

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a set of three media entity subdevice drivers for the i.MX
> Image Converter. The i.MX IC module contains three independent
> "tasks":
> 
> - Pre-processing Encode task: video frames are routed directly from
>   the CSI and can be scaled, color-space converted, and rotated.
>   Scaled output is limited to 1024x1024 resolution. Output frames
>   are routed to the camera interface entities (camif).
> 
> - Pre-processing Viewfinder task: this task can perform the same
>   conversions as the pre-process encode task, but in addition can
>   be used for hardware motion compensated deinterlacing. Frames can
>   come either directly from the CSI or from the SMFC entities (memory
>   buffers via IDMAC channels). Scaled output is limited to 1024x1024
>   resolution. Output frames can be routed to various sinks including
>   the post-processing task entities.
> 
> - Post-processing task: same conversions as pre-process encode. However
>   this entity sends frames to the i.MX IPU image converter which supports
>   image tiling, which allows scaled output up to 4096x4096 resolution.
>   Output frames can be routed to the camera interfaces.
> 
> Signed-off-by: Steve Longerbeam 
> ---

[snip]

> +static int imx_ic_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct imx_ic_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, >sd);
> + priv->dev = >dev;
> +
> + /* get our ipu_id, grp_id and IC task id */
> + pdata = priv->dev->platform_data;
> + priv->ipu_id = pdata->ipu_id;
> + switch (pdata->grp_id) {
> + case IMX_MEDIA_GRP_ID_IC_PRPENC:
> + priv->task_id = IC_TASK_ENCODER;
> + break;
> + case IMX_MEDIA_GRP_ID_IC_PRPVF:
> + priv->task_id = IC_TASK_VIEWFINDER;
> + break;
> + case IMX_MEDIA_GRP_ID_IC_PP0...IMX_MEDIA_GRP_ID_IC_PP3:
> + priv->task_id = IC_TASK_POST_PROCESSOR;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + v4l2_subdev_init(>sd, ic_ops[priv->task_id]->subdev_ops);
> + v4l2_set_subdevdata(>sd, priv);
> + priv->sd.internal_ops = ic_ops[priv->task_id]->internal_ops;
> + priv->sd.entity.ops = ic_ops[priv->task_id]->entity_ops;
> + priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
> + priv->sd.dev = >dev;
> + priv->sd.owner = THIS_MODULE;
> + priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + priv->sd.grp_id = pdata->grp_id;
> + strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
> +
> + ret = ic_ops[priv->task_id]->init(priv);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_async_register_subdev(>sd);
> + if (ret)
> + goto remove;
> +
> + return 0;
> +remove:
> + ic_ops[priv->task_id]->remove(priv);
> + return ret;

if (ret)
ic_ops[priv->task_id]->remove(priv);

return ret;

as an alternative.

[snip]

> +static const struct platform_device_id imx_ic_ids[] = {
> + { .name = "imx-ipuv3-ic" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, imx_ic_ids);
> +
> +static struct platform_driver imx_ic_driver = {
> + .probe = imx_ic_probe,
> + .remove = imx_ic_remove,
> + .id_table = imx_ic_ids,
> + .driver = {
> + .name = "imx-ipuv3-ic",
> + .owner = THIS_MODULE,

Please drop .owner assignment.

> + },
> +};
> +module_platform_driver(imx_ic_driver);
> +
> +MODULE_DESCRIPTION("i.MX IC subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam ");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx-ipuv3-ic");
> diff --git a/drivers/staging/media/imx/imx-ic-pp.c 
> b/drivers/staging/media/imx/imx-ic-pp.c
> new file mode 100644
> index 000..5ef0581
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-ic-pp.c
> @@ -0,0 +1,636 @@
> +/*
> + * V4L2 IC Post-Processor Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2014-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort the list of headers alphabetically.

> +#include 
> +#include 
> +#include "imx-media.h"
> +#include "imx-ic.h"
> +

[snip]

> +
> +static int pp_start(struct pp_priv *priv)
> +{
> + struct imx_ic_priv *ic_priv = priv->ic_priv;
> + struct ipu_image image_in, 

Re: [PATCH v2 12/19] media: imx: Add SMFC subdev driver

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a media entity subdevice driver for the i.MX Sensor Multi-FIFO
> Controller module. Video frames are received from the CSI and can
> be routed to various sinks including the i.MX Image Converter for
> scaling, color-space conversion, motion compensated deinterlacing,
> and image rotation.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile   |   1 +
>  drivers/staging/media/imx/imx-smfc.c | 739 
> +++
>  2 files changed, 740 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-smfc.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 133672a..3559d7b 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -5,4 +5,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
>  
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o

May be

obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o imx-smfc.o

>  
> diff --git a/drivers/staging/media/imx/imx-smfc.c 
> b/drivers/staging/media/imx/imx-smfc.c
> new file mode 100644
> index 000..565048c
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-smfc.c
> @@ -0,0 +1,739 @@
> +/*
> + * V4L2 Capture SMFC Subdev for Freescale i.MX5/6 SOC
> + *
> + * This subdevice handles capture of raw/unconverted video frames
> + * from the CSI, directly to memory via the Sensor Multi-FIFO Controller.
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort the list of headers alphabetically.

> +#include 
> +#include "imx-media.h"
> +

[snip]

> +static irqreturn_t imx_smfc_eof_interrupt(int irq, void *dev_id)
> +{
> + struct imx_smfc_priv *priv = dev_id;
> + struct imx_media_dma_buf *done, *next;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>irqlock, flags);

spin_lock(>irqlock) should be sufficient.

> +
> + if (priv->last_eof) {
> + complete(>last_eof_comp);
> + priv->last_eof = false;
> + goto unlock;
> + }
> +
> + /* inform CSI of this EOF so it can monitor frame intervals */
> + v4l2_subdev_call(priv->src_sd, core, interrupt_service_routine,
> +  0, NULL);
> +
> + done = imx_media_dma_buf_get_active(priv->out_ring);
> + /* give the completed buffer to the sink  */
> + if (!WARN_ON(!done))
> + imx_media_dma_buf_done(done, IMX_MEDIA_BUF_STATUS_DONE);
> +
> + /* priv->next buffer is now the active one */
> + imx_media_dma_buf_set_active(priv->next);
> +
> + /* bump the EOF timeout timer */
> + mod_timer(>eof_timeout_timer,
> +   jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
> +
> + if (ipu_idmac_buffer_is_ready(priv->smfc_ch, priv->ipu_buf_num))
> + ipu_idmac_clear_buffer(priv->smfc_ch, priv->ipu_buf_num);
> +
> + /* get next queued buffer */
> + next = imx_media_dma_buf_get_next_queued(priv->out_ring);
> +
> + ipu_cpmem_set_buffer(priv->smfc_ch, priv->ipu_buf_num, next->phys);
> + ipu_idmac_select_buffer(priv->smfc_ch, priv->ipu_buf_num);
> +
> + /* toggle IPU double-buffer index */
> + priv->ipu_buf_num ^= 1;
> + priv->next = next;
> +
> +unlock:
> + spin_unlock_irqrestore(>irqlock, flags);
> + return IRQ_HANDLED;
> +}
> +

[snip]

> +
> +static const struct platform_device_id imx_smfc_ids[] = {
> + { .name = "imx-ipuv3-smfc" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, imx_smfc_ids);
> +
> +static struct platform_driver imx_smfc_driver = {
> + .probe = imx_smfc_probe,
> + .remove = imx_smfc_remove,
> + .id_table = imx_smfc_ids,
> + .driver = {
> + .name = "imx-ipuv3-smfc",
> + .owner = THIS_MODULE,

You can drop owner assignment.

> + },
> +};
> +module_platform_driver(imx_smfc_driver);
> +
> +MODULE_DESCRIPTION("i.MX SMFC subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam ");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx-ipuv3-smfc");
> 

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] v4l2-subdev.h: fix v4l2_subdev_pad_config documentation

[Baruch Siach]

The fields of v4l2_subdev_pad_config are not pointers.

Fixes: 21c29de1d09 ("[media] v4l2-subdev.h: Improve documentation")
Cc: Mauro Carvalho Chehab 
Signed-off-by: Baruch Siach 
---
 include/media/v4l2-subdev.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index cf778c5dca18..0ab1c5df6fac 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -592,9 +592,9 @@ struct v4l2_subdev_ir_ops {
 /**
  * struct v4l2_subdev_pad_config - Used for storing subdev pad information.
  *
- * @try_fmt: pointer to  v4l2_mbus_framefmt
- * @try_crop: pointer to  v4l2_rect to be used for crop
- * @try_compose: pointer to  v4l2_rect to be used for compose
+ * @try_fmt:  v4l2_mbus_framefmt
+ * @try_crop:  v4l2_rect to be used for crop
+ * @try_compose:  v4l2_rect to be used for compose
  *
  * This structure only needs to be passed to the pad op if the 'which' field
  * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 11/19] media: imx: Add CSI subdev driver

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a media entity subdevice for the i.MX Camera
> Serial Interface module.
> 
> Signed-off-by: Steve Longerbeam 
> ---

[snip]

> diff --git a/drivers/staging/media/imx/imx-csi.c 
> b/drivers/staging/media/imx/imx-csi.c
> new file mode 100644
> index 000..975eafb
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-csi.c
> @@ -0,0 +1,638 @@
> +/*
> + * V4L2 Capture CSI Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2014-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please add the headers alphabetically ordered.

> +#include 
> +#include "imx-media.h"
> +
> +#define CSI_NUM_PADS 2
> +
> +struct csi_priv {
> + struct device *dev;
> + struct ipu_soc *ipu;
> + struct imx_media_dev *md;
> + struct v4l2_subdev sd;
> + struct media_pad pad[CSI_NUM_PADS];
> + struct v4l2_mbus_framefmt format_mbus[CSI_NUM_PADS];
> + struct v4l2_mbus_config sensor_mbus_cfg;
> + struct v4l2_rect crop;
> + struct ipu_csi *csi;
> + int csi_id;
> + int input_pad;
> + int output_pad;
> + bool power_on;  /* power is on */
> + bool stream_on; /* streaming is on */
> +
> + /* the sink for the captured frames */
> + struct v4l2_subdev *sink_sd;
> + enum ipu_csi_dest dest;
> + struct v4l2_subdev *src_sd;
> +
> + struct v4l2_ctrl_handler ctrl_hdlr;
> + struct imx_media_fim *fim;
> +
> + /* the attached sensor at stream on */
> + struct imx_media_subdev *sensor;
> +};
> +
> +static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct csi_priv, sd);
> +}
> +
> +/* Update the CSI whole sensor and active windows */
> +static int csi_setup(struct csi_priv *priv)
> +{
> + struct v4l2_mbus_framefmt infmt;
> +
> + ipu_csi_set_window(priv->csi, >crop);
> +
> + /*
> +  * the ipu-csi doesn't understand ALTERNATE, but it only
> +  * needs to know whether the stream is interlaced, so set
> +  * to INTERLACED if infmt field is ALTERNATE.
> +  */
> + infmt = priv->format_mbus[priv->input_pad];
> + if (infmt.field == V4L2_FIELD_ALTERNATE)
> + infmt.field = V4L2_FIELD_INTERLACED;
> +
> + ipu_csi_init_interface(priv->csi, >sensor_mbus_cfg, );
> +
> + ipu_csi_set_dest(priv->csi, priv->dest);
> +
> + ipu_csi_dump(priv->csi);
> +
> + return 0;
> +}
> +
> +static int csi_start(struct csi_priv *priv)
> +{
> + int ret;
> +
> + if (!priv->sensor) {
> + v4l2_err(>sd, "no sensor attached\n");
> + return -EINVAL;
> + }
> +
> + ret = csi_setup(priv);
> + if (ret)
> + return ret;
> +
> + /* start the frame interval monitor */
> + ret = imx_media_fim_set_stream(priv->fim, priv->sensor, true);
> + if (ret)
> + return ret;
> +
> + ret = ipu_csi_enable(priv->csi);
> + if (ret) {
> + v4l2_err(>sd, "CSI enable error: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;

if (ret)
v4l2_err(>sd, "CSI enable error: %d\n", ret);

return ret;

> +}
> +
> +static void csi_stop(struct csi_priv *priv)
> +{
> + /* stop the frame interval monitor */
> + imx_media_fim_set_stream(priv->fim, priv->sensor, false);
> +
> + ipu_csi_disable(priv->csi);
> +}
> +
> +static int csi_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + if (!priv->src_sd || !priv->sink_sd)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !priv->stream_on)
> + ret = csi_start(priv);
> + else if (!enable && priv->stream_on)
> + csi_stop(priv);
> +
> + if (!ret)
> + priv->stream_on = enable;
> + return ret;
> +}
> +
> +static int csi_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + v4l2_info(sd, "power %s\n", on ? "ON" : "OFF");
> +
> + if (on != priv->power_on)
> + ret = imx_media_fim_set_power(priv->fim, on);
> +
> + if (!ret)
> + priv->power_on = on;
> + return ret;
> +}
> +
> +static int csi_link_setup(struct media_entity *entity,
> +   const struct media_pad *local,
> +   const struct media_pad *remote, u32 flags)
> +{
> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> + struct csi_priv *priv = v4l2_get_subdevdata(sd);
> + struct v4l2_subdev 

Re: [PATCH v2 10/19] media: Add i.MX media core driver

[Vladimir Zapolskiy]

Hi Steve,

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Add the core media driver for i.MX SOC.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt   | 205 +

v2 was sent before getting Rob's review comments, but still they
should be addressed in v3.

Also I would suggest to separate device tree binding documentation
change and place it as the first patch in the series, this should
make the following DTS changes valid.

>  Documentation/media/v4l-drivers/imx.rst   | 430 ++
>  drivers/staging/media/Kconfig |   2 +
>  drivers/staging/media/Makefile|   1 +
>  drivers/staging/media/imx/Kconfig |   8 +
>  drivers/staging/media/imx/Makefile|   6 +
>  drivers/staging/media/imx/TODO|  18 +
>  drivers/staging/media/imx/imx-media-common.c  | 985 
> ++
>  drivers/staging/media/imx/imx-media-dev.c | 479 +++
>  drivers/staging/media/imx/imx-media-fim.c | 509 +++
>  drivers/staging/media/imx/imx-media-internal-sd.c | 457 ++
>  drivers/staging/media/imx/imx-media-of.c  | 291 +++
>  drivers/staging/media/imx/imx-media-of.h  |  25 +
>  drivers/staging/media/imx/imx-media.h | 299 +++
>  include/media/imx.h   |  15 +
>  include/uapi/Kbuild   |   1 +
>  include/uapi/linux/v4l2-controls.h|   4 +
>  include/uapi/media/Kbuild |   2 +
>  include/uapi/media/imx.h  |  30 +

Probably Greg should ack the UAPI changes, you may consider
to split them into a separate patch.

>  19 files changed, 3767 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
>  create mode 100644 Documentation/media/v4l-drivers/imx.rst
>  create mode 100644 drivers/staging/media/imx/Kconfig
>  create mode 100644 drivers/staging/media/imx/Makefile
>  create mode 100644 drivers/staging/media/imx/TODO
>  create mode 100644 drivers/staging/media/imx/imx-media-common.c
>  create mode 100644 drivers/staging/media/imx/imx-media-dev.c
>  create mode 100644 drivers/staging/media/imx/imx-media-fim.c
>  create mode 100644 drivers/staging/media/imx/imx-media-internal-sd.c
>  create mode 100644 drivers/staging/media/imx/imx-media-of.c
>  create mode 100644 drivers/staging/media/imx/imx-media-of.h
>  create mode 100644 drivers/staging/media/imx/imx-media.h
>  create mode 100644 include/media/imx.h
>  create mode 100644 include/uapi/media/Kbuild
>  create mode 100644 include/uapi/media/imx.h
> 

[snip]

> +
> +struct imx_media_subdev *
> +imx_media_find_subdev_by_sd(struct imx_media_dev *imxmd,
> + struct v4l2_subdev *sd)
> +{
> + struct imx_media_subdev *imxsd;
> + int i, ret = -ENODEV;
> +
> + for (i = 0; i < imxmd->num_subdevs; i++) {
> + imxsd = >subdev[i];
> + if (sd == imxsd->sd) {

This can be simplifed:

...

if (sd == imxsd->sd)
return imxsd;
}

return ERR_PTR(-ENODEV);

> + ret = 0;
> + break;
> + }
> + }
> +
> + return ret ? ERR_PTR(ret) : imxsd;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_sd);
> +
> +struct imx_media_subdev *
> +imx_media_find_subdev_by_id(struct imx_media_dev *imxmd, u32 grp_id)
> +{
> + struct imx_media_subdev *imxsd;
> + int i, ret = -ENODEV;
> +
> + for (i = 0; i < imxmd->num_subdevs; i++) {
> + imxsd = >subdev[i];
> + if (imxsd->sd && imxsd->sd->grp_id == grp_id) {
> + ret = 0;
> + break;

This can be simplifed:

...

if (imxsd->sd && imxsd->sd->grp_id == grp_i)
return imxsd;
}

return ERR_PTR(-ENODEV);

> + }
> + }
> +
> + return ret ? ERR_PTR(ret) : imxsd;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_id);
> +

[snip]

> diff --git a/drivers/staging/media/imx/imx-media-dev.c 
> b/drivers/staging/media/imx/imx-media-dev.c
> new file mode 100644
> index 000..8d22730
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -0,0 +1,479 @@
> +/*
> + * V4L2 Media Controller Driver for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort out the list of headers alphabetically.

> +#include 
> +#include 
> +#include "imx-media.h"
> +#include 

Re: [PATCH v2 09/19] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Enables the ADV7180 decoder sensor. The ADV7180 connects to the
> parallel-bus mux input on ipu1_csi0_mux.
> 
> On the sabreauto, two analog video inputs are routed to the ADV7180,
> composite on Ain1, and composite on Ain3. Those inputs are defined
> via inputs and input-names under the ADV7180 node. The ADV7180 power
> pin is via max7310_b port expander.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 56 
> 
>  1 file changed, 56 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi 
> b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 83ac2ff..30ee378 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -147,10 +147,42 @@
>   gpio-controller;
>   #gpio-cells = <2>;
>   };
> +
> + camera: adv7180@21 {

adv7180: camera@21

> + compatible = "adi,adv7180";
> + reg = <0x21>;
> + powerdown-gpios = <_b 2 
> GPIO_ACTIVE_LOW>;
> + interrupt-parent = <>;
> + interrupts = <27 0x8>;
> + inputs = <0x00 0x02>;
> + input-names = "ADV7180 Composite on Ain1",
> + "ADV7180 Composite on Ain3";
> +
> + port {
> + adv7180_to_ipu1_csi0_mux: endpoint {
> + remote-endpoint = 
> <_csi0_mux_from_parallel_sensor>;
> + bus-width = <8>;
> + };
> + };
> + };
>   };
>   };
>  };
>  
> +_csi0_from_ipu1_csi0_mux {
> + bus-width = <8>;
> +};
> +
> +_csi0_mux_from_parallel_sensor {
> + remote-endpoint = <_to_ipu1_csi0_mux>;
> + bus-width = <8>;
> +};
> +
> +_csi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_ipu1_csi0>;
> +};
> +
>   {
>   assigned-clocks = < IMX6QDL_PLL4_BYPASS_SRC>,
> < IMX6QDL_PLL4_BYPASS>,
> @@ -451,6 +483,30 @@
>   >;
>   };
>  
> + pinctrl_ipu1_csi0: ipu1grp-csi0 {

Please rename node name to ipu1csi0grp.

> + fsl,pins = <
> + MX6QDL_PAD_CSI0_DAT4__IPU1_CSI0_DATA04   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT5__IPU1_CSI0_DATA05   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT6__IPU1_CSI0_DATA06   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT7__IPU1_CSI0_DATA07   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT8__IPU1_CSI0_DATA08   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT9__IPU1_CSI0_DATA09   
> 0x8000
> + MX6QDL_PAD_CSI0_DAT10__IPU1_CSI0_DATA10  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT11__IPU1_CSI0_DATA11  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18  
> 0x8000
> + MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19  
> 0x8000
> + MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK 
> 0x8000
> + MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC
> 0x8000
> + MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC   
> 0x8000
> + >;
> + };
> +
>   pinctrl_pwm3: pwm1grp {
>   fsl,pins = <
>   MX6QDL_PAD_SD4_DAT1__PWM3_OUT   0x1b0b1
> 

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors

[Vladimir Zapolskiy]

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
> 
> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
> 
> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> mipi_csi. It is set to transmit over MIPI virtual channel 1.
> 
> Until the OV5652 sensor module compatible with the SabreSD becomes
> available for testing, the ov5642 node is currently disabled.
> 
> Signed-off-by: Steve Longerbeam 
> ---

[snip]

> +
> + camera: ov5642@3c {

ov5642: camera@3c

> + compatible = "ovti,ov5642";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ov5642>;
> + clocks = < IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + reg = <0x3c>;
> + xclk = <2400>;
> + DOVDD-supply = <_reg>; /* 1.8v */
> + AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
> + DVDD-supply = <_reg>;  /* 1.5v*/
> + pwdn-gpios = < 16 GPIO_ACTIVE_HIGH>; /* SD1_DAT0 */
> + reset-gpios = < 17 GPIO_ACTIVE_LOW>; /* SD1_DAT1 */

Comments about SD1_* pad names are redundant.

> + status = "disabled";

Why is it disabled here?

> +
> + port {
> + ov5642_to_ipu1_csi0_mux: endpoint {
> + remote-endpoint = 
> <_csi0_mux_from_parallel_sensor>;
> + bus-width = <8>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + };
> + };
> + };
>  };
>  
>   {
> @@ -322,6 +376,34 @@
>   };
>   };
>   };
> +
> + mipi_camera: ov5640@3c {

ov5640: camera@3c

> + compatible = "ovti,ov5640_mipi";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ov5640>;
> + reg = <0x3c>;
> + clocks = < IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + xclk = <2400>;
> + DOVDD-supply = <_reg>; /* 1.8v */
> + AVDD-supply = <_reg>;  /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
> + DVDD-supply = <_reg>;  /* 1.5v*/
> + pwdn-gpios = < 19 GPIO_ACTIVE_HIGH>; /* SD1_DAT2 */
> + reset-gpios = < 20 GPIO_ACTIVE_LOW>; /* SD1_CLK */

Comments about SD1_* pad names are redundant.

> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640_to_mipi_csi: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <_csi_from_mipi_sensor>;
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> + };
> + };
> + };
>  };
>  
>   {
> @@ -426,6 +508,36 @@
>   >;
>   };
>  
> + pinctrl_ov5640: ov5640grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000
> + MX6QDL_PAD_SD1_CLK__GPIO1_IO20  0x8000
> + >;
> + };
> +
> + pinctrl_ov5642: ov5642grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT0__GPIO1_IO16 0x8000
> + MX6QDL_PAD_SD1_DAT1__GPIO1_IO17 0x8000
> + >;
> + };
> +
> + pinctrl_ipu1_csi0: ipu1grp-csi0 {

Please rename the node name to ipu1csi0grp.

Please add new pin control groups preserving the alphanimerical order.

> + fsl,pins = <
> + MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12
> 0x8000
> + MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13
> 0x8000
> + MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14
> 0x8000
> + MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15
> 0x8000
> + MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16
> 0x8000
> + MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17
> 0x8000
> + MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18
> 0x8000
> + MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19
> 0x8000
> + MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK   
> 0x8000
> + MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC  
> 0x8000
> + MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC 
> 0x8000
> + >;
> + };
> +
>   pinctrl_pcie: pciegrp {
> 

Re: [PATCH v2 04/19] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Vladimir Zapolskiy]

Hi Steve,

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
> Both hang off the same i2c2 bus, so they require different (and non-
> default) i2c slave addresses.
> 
> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
> 
> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> mipi_csi. It is set to transmit over MIPI virtual channel 1.
> 
> Note there is a pin conflict with GPIO6. This pin functions as a power
> input pin to the OV5642, but ENET uses it as the h/w workaround for
> erratum ERR006687, to wake-up the ARM cores on normal RX and TX packet
> done events (see 6261c4c8). So workaround 6261c4c8 is reverted here to
> support the OV5642, and the "fsl,err006687-workaround-present" boolean
> also must be removed. The result is that the CPUidle driver will no longer
> allow entering the deep idle states on the sabrelite.

For me it sounds like a candidate of its own separate change.

> 
> Signed-off-by: Steve Longerbeam 
> ---

[snip]

>   {
>   pinctrl-names = "default";
>   pinctrl-0 = <_audmux>;
> @@ -271,9 +298,6 @@
>   txd1-skew-ps = <0>;
>   txd2-skew-ps = <0>;
>   txd3-skew-ps = <0>;
> - interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
> -   < 0 119 IRQ_TYPE_LEVEL_HIGH>;

Like you say in the commit message this is a partial revert of 6261c4c8f13e
("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC interrupt.")

> - fsl,err006687-workaround-present;

This is a partial revert of a28eeb43ee57 ("ARM: dts: imx6: tag boards that
have the HW workaround for ERR006687").

The change should be split and reviewed separately in my opinion, also
cc Gary Bisson  for SabreLite changes.

>   status = "okay";
>  };
>  
> @@ -302,6 +326,52 @@
>   pinctrl-names = "default";
>   pinctrl-0 = <_i2c2>;
>   status = "okay";
> +
> + camera: ov5642@42 {
> + compatible = "ovti,ov5642";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ov5642>;
> + clocks = < IMX6QDL_CLK_CKO2>;
> + clock-names = "xclk";
> + reg = <0x42>;
> + xclk = <2400>;
> + reset-gpios = < 8 GPIO_ACTIVE_LOW>;
> + pwdn-gpios = < 6 GPIO_ACTIVE_HIGH>;
> + gp-gpios = < 16 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov5642_to_ipu1_csi0_mux: endpoint {
> + remote-endpoint = 
> <_csi0_mux_from_parallel_sensor>;
> + bus-width = <8>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + };
> + };
> + };
> +
> + mipi_camera: ov5640@40 {

Please reorder device nodes by address value, also according to ePAPR
node names should be generic, labels can be specific:

ov5640: camera@40 {
...
};

ov5642: camera@42 {
...
};

> + compatible = "ovti,ov5640_mipi";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ov5640>;
> + clocks = <_xclk>;
> + clock-names = "xclk";
> + reg = <0x40>;
> + xclk = <2200>;
> + reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* NANDF_D5 */
> + pwdn-gpios = < 9 GPIO_ACTIVE_HIGH>; /* NANDF_WP_B */
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640_to_mipi_csi: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <_csi_from_mipi_sensor>;
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> + };
> + };
> + };
>  };
>  
>   {
> @@ -374,7 +444,6 @@
>   MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL   0x1b030
>   /* Phy reset */
>   MX6QDL_PAD_EIM_D23__GPIO3_IO23  0x000b0
> - MX6QDL_PAD_GPIO_6__ENET_IRQ 0x000b1

Yep.

>   >;
>   };
>  
> @@ -449,6 +518,39 @@
>   >;
>   };
>  
> + pinctrl_ov5642: ov5642grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT0__GPIO1_IO16 0x8000
> + MX6QDL_PAD_GPIO_6__GPIO1_IO06   0x8000
> + MX6QDL_PAD_GPIO_8__GPIO1_IO08   0x8000
> + MX6QDL_PAD_GPIO_3__CCM_CLKO20x8000
> + >;
> + };
> +
> + pinctrl_ipu1_csi0: ipu1grp-csi0 {

Please rename node name to ipu1csi0grp.

> + fsl,pins = <
> +   

Re: [PATCH v6 0/5] davinci: VPIF: add DT support

[Sekhar Nori]

+ Mauro

On Tuesday 03 January 2017 02:42 PM, Laurent Pinchart wrote:
> Hi Sekhar,
> 
> On Tuesday 03 Jan 2017 14:33:00 Sekhar Nori wrote:
>> On Friday 16 December 2016 03:17 PM, Hans Verkuil wrote:
>>> On 07/12/16 19:30, Kevin Hilman wrote:
 Prepare the groundwork for adding DT support for davinci VPIF drivers.
 This series does some fixups/cleanups and then adds the DT binding and
 DT compatible string matching for DT probing.

 The controversial part from previous versions around async subdev
 parsing, and specifically hard-coding the input/output routing of
 subdevs, has been left out of this series.  That part can be done as a
 follow-on step after agreement has been reached on the path forward.
 With this version, platforms can still use the VPIF capture/display
 drivers, but must provide platform_data for the subdevs and subdev
 routing.

 Tested video capture to memory on da850-lcdk board using composite
 input.
>>>
>>> Other than the comment for the first patch this series looks good.
>>>
>>> So once that's addressed I'll queue it up for 4.11.
>>
>> Can you provide an immutable commit (as it will reach v4.11) with with
>> this series applied? I have some platform changes to queue for v4.11
>> that depend on the driver updates.
> 
> I don't think that's possible, given that Mauro rewrites all patches when 
> handling pull requests to prepend [media] to the subject line and to add his 
> SoB. Only Mauro can thus provide a stable branch, Hans can't.

Hi Mauro, once Hans sends you these patches, can you host these patches
on a stable branch, which I can merge into my pull request to ARM-SoC. I
have some platform updates that depend on these driver changes.

Ideally the branch has only these patches over an early v4.10-rc so I
include as little of media stuff as possible in my pull request.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] uapi: use wildcards to list files

[Nicolas Dichtel]

Le 03/01/2017 à 22:37, Arnd Bergmann a écrit :
> On Tuesday, January 3, 2017 3:35:44 PM CET Nicolas Dichtel wrote:
>> Regularly, when a new header is created in include/uapi/, the developer
>> forgets to add it in the corresponding Kbuild file. This error is usually
>> detected after the release is out.
>>
>> In fact, all headers under include/uapi/ should be exported, so let's
>> use wildcards.
> 
> I think the idea makes a lot of sense: if a header is in uapi, we should
> really export it. However, using a wildcard expression seems a bit
> backwards here, I think we should make this implicit and not have the
> Kbuild file at all.
> 
> The "header-y" syntax was originally added back when the uapi headers
> were mixed with the internal headers in the same directory. After
> David Howells introduced the separate directory for uapi, it has
> become a bit redundant.
Ok, thank you for the explanation, I was wondering why those Kbuild files were
needed.

> 
> Can you try to modify scripts/Makefile.headersinst instead so we
> can simply remove the Kbuild files entirely?
I will try something.


Regards,
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: entity: Catch unbalanced media_pipeline_stop calls

[Sakari Ailus]

Hi Kieran,

Thanks for the patch!

On Tue, Jan 03, 2017 at 05:05:58PM +, Kieran Bingham wrote:
> On 03/01/17 13:36, Laurent Pinchart wrote:
> > Hi Kieran,
> > 
> > Thank you for the patch.
> > 
> > On Tuesday 03 Jan 2017 13:12:11 Kieran Bingham wrote:
> >> Drivers must not perform unbalanced calls to stop the entity pipeline,
> >> however if they do they will fault in the core media code, as the
> >> entity->pipe will be set as NULL. We handle this gracefully in the core
> >> with a WARN for the developer.
> >>
> >> Replace the erroneous check on zero streaming counts, with a check on
> >> NULL pipe elements instead, as this is the symptom of unbalanced
> >> media_pipeline_stop calls.
> >>
> >> Signed-off-by: Kieran Bingham 
> > 
> > This looks good to me,
> > 
> > Acked-by: Laurent Pinchart 
> > 
> > I'll let Sakari review and merge the patch.
> 
> Ahh, yes - I forgot to mention, although perhaps it will be obvious for
> Sakari - but this patch is based on top of Sakari's pending media
> pipeline and graph walk cleanup series :D

I've applied this on top of the other patches.

It's always good to mention dependencies to other patches, that's very
relevant for reviewers.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt: bindings: Add support for CSI1 bus

[Sakari Ailus]

Hi Rob,

Thanks for the review.

On Tue, Jan 03, 2017 at 02:38:54PM -0600, Rob Herring wrote:
> On Wed, Dec 28, 2016 at 07:30:36PM +0100, Pavel Machek wrote:
> > From: Sakari Ailus 
> > 
> > In the vast majority of cases the bus type is known to the driver(s)
> > since a receiver or transmitter can only support a single one. There
> > are cases however where different options are possible.
> 
> What cases specifically?

The existing V4L2 OF support tries to figure out the bus type and parse the
bus parameters based on that. This does not scale too well as there are
multiple serial busses that share common properties.

Some hardware also supports multiple types of busses on the same interfaces.

> 
> > Document the CSI1/CCP2 properties strobe_clk_inv and strobe_clock
> > properties. The former tells whether the strobe/clock signal is
> > inverted, while the latter signifies the clock or strobe mode.
> > 
> > Signed-off-by: Sakari Ailus 
> > Signed-off-by: Ivaylo Dimitrov 
> > Signed-off-by: Pavel Machek 
> > 
> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> > b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > index 9cd2a36..f0523f7 100644
> > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > @@ -76,6 +76,10 @@ Optional endpoint properties
> >mode horizontal and vertical synchronization signals are provided to the
> >slave device (data source) by the master device (data sink). In the 
> > master
> >mode the data source device is also the source of the synchronization 
> > signals.
> > +- bus-type: data bus type. Possible values are:
> > +  0 - CSI2
> 
> As in MIPI CSI2?

Yeah, I guess it'd make sense to make this explicit.

> 
> > +  1 - parallel / Bt656
> > +  2 - CCP2
> >  - bus-width: number of data lines actively used, valid for the parallel 
> > busses.
> >  - data-shift: on the parallel data busses, if bus-width is used to specify 
> > the
> >number of data lines, data-shift can be used to specify which data lines 
> > are
> > @@ -110,9 +114,10 @@ Optional endpoint properties
> >lane and followed by the data lanes in the same order as in data-lanes.
> >Valid values are 0 (normal) and 1 (inverted). The length of the array
> >should be the combined length of data-lanes and clock-lanes properties.
> > -  If the lane-polarities property is omitted, the value must be interpreted
> > -  as 0 (normal). This property is valid for serial busses only.
> 
> Why is this removed?

Must have been by mistake. :-)

> 
> > -
> > +- clock-inv: Clock or strobe signal inversion.
> > +  Possible values: 0 -- not inverted; 1 -- inverted
> 
> "invert" assumes I know what is normal and I do not. Define what is 
> "normal" and name the property the opposite of that. If normal is data 
> shifted on clock rising edge, then call the the property 
> "clock-shift-falling-edge" for example..

The hardware documentation says this is the "strobe/clock inversion control
signal". I'm not entirely sure whether this is just signal polarity (it's a
differential signal) or inversion of an internal signal of the CCP2 block.

It might make sense to make this a private property for the OMAP 3 ISP
instead. If it's seen elsewhere, then think about it again. I doubt it
would, as CCP2 is an old bus that's used on Nokia N9, N950 and N900.

As strobe is included, I'd add that to the name. Say, "ti,clock-strobe-inv".

> 
> > +- strobe: Whether the clock signal is used as clock or strobe. Used
> > +  with CCP2, for instance.
> >  
> >  Example
> >  ---
> > 
> > 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Andrzej Hajda]

On 03.01.2017 09:11, Hans Verkuil wrote:
> On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
>> Is there a reason to split registration into two steps?
>> Wouldn't be better to integrate hpd_notifier_get into
>> cec_register_hpd_notifier.
> One problem is that hpd_notifier_get can fail, whereas 
> cec_register_hpd_notifier can't.
> And I rather not have to register a CEC device only to unregister it again if 
> the
> hpd_notifier_get would fail.

hpd_notifier_get can fail only due to lack of memory for about 150 bytes
so if it happens whole system will probably fail anyway :)


>
> Another reason is that this keeps the responsibility of the hpd_notifier 
> life-time
> handling in the driver instead of hiding it in the CEC framework, which is 
> IMHO
> unexpected.

Notifier is used only by CEC framework, so IMHO it would be desirable to
put CEC specific things into CEC framework.
Drivers duty is just to find notifier device.
Leaving it as is will just put little more burden on drivers, so this is
not big deal, do as you wish :)

Regards
Andrzej

>
> I think I want to keep this as-is, at least for now.
>
> Regards,
>
>   Hans
>
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html