cron job: media_tree daily build: OK

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Tue May  1 05:00:12 CEST 2018
media-tree git hash:a2b2eff6ac2716f499defa590a6ec4ba379d765e
media_build git hash:   2945d108c680b3c09c9843e001e84a9797d7f379
v4l-utils git hash: 03e763fd4b361b2082019032fc315b7606669335
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: 0.5.2-RC1
smatch version: 0.5.1
host hardware:  x86_64
host os:4.15.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.101-i686: OK
linux-3.2.101-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.56-i686: OK
linux-3.16.56-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.102-i686: OK
linux-3.18.102-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.51-i686: OK
linux-4.1.51-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.109-i686: OK
linux-4.4.109-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.91-i686: OK
linux-4.9.91-x86_64: OK
linux-4.14.31-i686: OK
linux-4.14.31-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
linux-4.16-i686: OK
linux-4.16-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: OK

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: [RESEND PATCH v8 2/2] media: dw9807: Add dw9807 vcm driver

[Tomasz Figa]

Hi Andy,

On Wed, Apr 25, 2018 at 11:04 AM Andy Yeh  wrote:

> From: Alan Chiang 

> DW9807 is a 10 bit DAC from Dongwoon, designed for linear
> control of voice coil motor.

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

> Signed-off-by: Andy Yeh 
> ---
> since v1:
> - changed author.
> since v2:
> - addressed outstanding comments.
> - enabled sequential write to update 2 registers in a single transaction.
> since v3:
> - addressed comments for v3.
> - Remove redundant codes and declare some variables as constant variable.
> - separate DT binding to another patch
> since v4:
> - sent patchset included DT binding with cover page
> since v6:
> - change the return code of i2c_check
> - fix long cols exceed 80 chars
> - remove #define DW9807_NAME since only used once
> since v7:
> - Remove some redundant type cast
> - Modify to meet the coding style
> - Replace a while loop by readx_poll_timeout function
> - Return the i2c error directly

Reviewed-by: Tomasz Figa 

Thanks for addressing the comments.

Best regards,
Tomasz

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

[Tomasz Figa]

On Fri, Apr 27, 2018 at 2:22 AM Zhi, Yong  wrote:

> Hi, Tomasz,

> Thanks for the review again.

> > -Original Message-
> > From: Tomasz Figa [mailto:tf...@chromium.org]
> > Sent: Thursday, April 26, 2018 12:15 AM
> > To: Zhi, Yong 
> > Cc: Linux Media Mailing List ; Sakari Ailus
> > ; Mani, Rajmohan
> > ; Toivonen, Tuukka
> > ; Hu, Jerry W ; Zheng,
> > Jian Xu 
> > Subject: Re: [PATCH v6 12/12] intel-ipu3: Add imgu top level pci device
> > driver
> >
> > Hi Yong,
> >
> > On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi  wrote:
> > [snip]
> > > +static int imgu_video_nodes_init(struct imgu_device *imgu) {
> > > +   struct v4l2_pix_format_mplane *fmts[IPU3_CSS_QUEUES] = { NULL
};
> > > +   struct v4l2_rect *rects[IPU3_CSS_RECTS] = { NULL };
> > > +   unsigned int i;
> > > +   int r;
> > > +
> > > +   imgu->buf_struct_size = sizeof(struct imgu_buffer);
> > > +
> > > +   for (i = 0; i < IMGU_NODE_NUM; i++) {
> > > +   imgu->nodes[i].name = imgu_node_map[i].name;
> > > +   imgu->nodes[i].output = i < IMGU_QUEUE_FIRST_INPUT;
> > > +   imgu->nodes[i].immutable = false;
> > > +   imgu->nodes[i].enabled = false;
> > > +
> > > +   if (i != IMGU_NODE_PARAMS && i != IMGU_NODE_STAT_3A)
> > > +   fmts[imgu_node_map[i].css_queue] =
> > > +   >nodes[i].vdev_fmt.fmt.pix_mp;
> > > +   atomic_set(>nodes[i].sequence, 0);
> > > +   }
> > > +
> > > +   /* Master queue is always enabled */
> > > +   imgu->nodes[IMGU_QUEUE_MASTER].immutable = true;
> > > +   imgu->nodes[IMGU_QUEUE_MASTER].enabled = true;
> > > +
> > > +   r = ipu3_v4l2_register(imgu);
> > > +   if (r)
> > > +   return r;
> > > +
> > > +   /* Set initial formats and initialize formats of video nodes
*/
> > > +   rects[IPU3_CSS_RECT_EFFECTIVE] = >rect.eff;
> > > +   rects[IPU3_CSS_RECT_BDS] = >rect.bds;
> > > +   ipu3_css_fmt_set(>css, fmts, rects);
> > > +
> > > +   /* Pre-allocate dummy buffers */
> > > +   r = imgu_dummybufs_preallocate(imgu);
> > > +   if (r) {
> > > +   dev_err(>pci_dev->dev,
> > > +   "failed to pre-allocate dummy buffers (%d)",
r);
> > > +   imgu_dummybufs_cleanup(imgu);
> >
> > No need to call ipu3_v4l2_unregister() here?
> >
> > (That's why I keep suggesting use of single return error path with
labels
> > named after the first cleanup step that needs to be done, as it makes it
> > easier to spot such mistakes.)
> >

> Good catch, suggestion taken :)

Thanks.


> Maybe I should move the imgu_dummybufs_preallocate() out from
imgu_video_nodes_init().

Either is fine for me. I don't see any big problem in
imgu_dummybufs_preallocate() being in imgu_video_nodes_init(). It must be
done before exposing video nodes to userspace (video_device_register()0, so
it's probably a good place.

Best regards,
Tomasz

[PATCH v4 06/10] media-bus: uapi: Add YCrCb 420 media bus format

[Satish Kumar Nagireddy]

This commit adds YUV 420 media bus format. VYYUYY8_1X24
is an approximate way to descrive the pixels sent over
the bus.

This patch also contain rst documentation for media bus format.

Signed-off-by: Satish Kumar Nagireddy 
---
 Documentation/media/uapi/v4l/subdev-formats.rst | 38 -
 include/uapi/linux/media-bus-format.h   |  3 +-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst 
b/Documentation/media/uapi/v4l/subdev-formats.rst
index 9fcabe7..904c52b 100644
--- a/Documentation/media/uapi/v4l/subdev-formats.rst
+++ b/Documentation/media/uapi/v4l/subdev-formats.rst
@@ -6640,6 +6640,43 @@ the following codes.
   - u\ :sub:`2`
   - u\ :sub:`1`
   - u\ :sub:`0`
+* .. _MEDIA-BUS-FMT-VYYUYY8-1X24:
+
+  - MEDIA_BUS_FMT_VYYUYY8_1X24
+  - 0x202c
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  - v\ :sub:`3`
+  - v\ :sub:`2`
+  - v\ :sub:`1`
+  - v\ :sub:`0`
+  - y\ :sub:`7`
+  - y\ :sub:`6`
+  - y\ :sub:`5`
+  - y\ :sub:`4`
+  - y\ :sub:`3`
+  - y\ :sub:`2`
+  - y\ :sub:`1`
+  - y\ :sub:`0`
+  - u\ :sub:`3`
+  - u\ :sub:`2`
+  - u\ :sub:`1`
+  - u\ :sub:`0`
+  - 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-YUV10-1X30:
 
   - MEDIA_BUS_FMT_YUV10_1X30
@@ -7287,7 +7324,6 @@ The following table list existing packed 48bit wide YUV 
formats.
   - y\ :sub:`1`
   - y\ :sub:`0`
 
-
 .. raw:: latex
 
\endgroup
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 9e35117..ade7e9d 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -62,7 +62,7 @@
 #define MEDIA_BUS_FMT_RGB121212_1X36   0x1019
 #define MEDIA_BUS_FMT_RGB161616_1X48   0x101a
 
-/* YUV (including grey) - next is  0x202c */
+/* YUV (including grey) - next is  0x202d */
 #define MEDIA_BUS_FMT_Y8_1X8   0x2001
 #define MEDIA_BUS_FMT_UV8_1X8  0x2015
 #define MEDIA_BUS_FMT_UYVY8_1_5X8  0x2002
@@ -106,6 +106,7 @@
 #define MEDIA_BUS_FMT_YUV12_1X36   0x2029
 #define MEDIA_BUS_FMT_YUV16_1X48   0x202a
 #define MEDIA_BUS_FMT_UYYVYY16_0_5X48  0x202b
+#define MEDIA_BUS_FMT_VYYUYY8_1X24 0x202c
 
 /* Bayer - next is 0x3021 */
 #define MEDIA_BUS_FMT_SBGGR8_1X8   0x3001
-- 
2.1.1

[PATCH v4 07/10] media: Add new dt-bindings/vf_codes for supported formats

[Satish Kumar Nagireddy]

From: Rohit Athavale 

This commit adds new entries to the exisiting vf_codes that are used
to describe the media bus formats in the DT bindings. The newly added
8-bit and 10-bit color depth related formats will need these updates.

Signed-off-by: Rohit Athavale 
Signed-off-by: Satish Kumar Nagireddy 
---
 include/dt-bindings/media/xilinx-vip.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/media/xilinx-vip.h 
b/include/dt-bindings/media/xilinx-vip.h
index 6298fec..fcd34d7 100644
--- a/include/dt-bindings/media/xilinx-vip.h
+++ b/include/dt-bindings/media/xilinx-vip.h
@@ -35,5 +35,7 @@
 #define XVIP_VF_CUSTOM213
 #define XVIP_VF_CUSTOM314
 #define XVIP_VF_CUSTOM415
+#define XVIP_VF_VUY_42216
+#define XVIP_VF_XBGR   17
 
 #endif /* __DT_BINDINGS_MEDIA_XILINX_VIP_H__ */
-- 
2.1.1

[PATCH v4 05/10] uapi: media: New fourcc codes needed by Xilinx Video IP

[Satish Kumar Nagireddy]

From: Jeffrey Mouroux 

The Xilinx Video Framebuffer DMA IP supports video memory formats
that are not represented in the current V4L2 fourcc library. This
patch adds those missing fourcc codes. This includes both new
8-bit and 10-bit pixel formats.

Signed-off-by: Jeffrey Mouroux 
Signed-off-by: Satish Kumar Nagireddy 
---
 include/uapi/linux/videodev2.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877b..97b6633 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -510,6 +510,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_RGB32   v4l2_fourcc('R', 'G', 'B', '4') /* 32  
RGB-8-8-8-8   */
 #define V4L2_PIX_FMT_ARGB32  v4l2_fourcc('B', 'A', '2', '4') /* 32  
ARGB-8-8-8-8  */
 #define V4L2_PIX_FMT_XRGB32  v4l2_fourcc('B', 'X', '2', '4') /* 32  
XRGB-8-8-8-8  */
+#define V4L2_PIX_FMT_BGRX32  v4l2_fourcc('X', 'B', 'G', 'R') /* 32  
XBGR-8-8-8-8 */
 
 /* Grey formats */
 #define V4L2_PIX_FMT_GREYv4l2_fourcc('G', 'R', 'E', 'Y') /*  8  Greyscale  
   */
@@ -537,6 +538,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VYUYv4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2  
   */
 #define V4L2_PIX_FMT_Y41Pv4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1  
   */
 #define V4L2_PIX_FMT_YUV444  v4l2_fourcc('Y', '4', '4', '4') /* 16   
 */
+#define V4L2_PIX_FMT_VUY24   v4l2_fourcc('V', 'U', '2', '4') /* 24  VUY 8:8:8 
*/
 #define V4L2_PIX_FMT_YUV555  v4l2_fourcc('Y', 'U', 'V', 'O') /* 16  YUV-5-5-5  
   */
 #define V4L2_PIX_FMT_YUV565  v4l2_fourcc('Y', 'U', 'V', 'P') /* 16  YUV-5-6-5  
   */
 #define V4L2_PIX_FMT_YUV32   v4l2_fourcc('Y', 'U', 'V', '4') /* 32  
YUV-8-8-8-8   */
@@ -551,6 +553,8 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_NV61v4l2_fourcc('N', 'V', '6', '1') /* 16  Y/CrCb 
4:2:2  */
 #define V4L2_PIX_FMT_NV24v4l2_fourcc('N', 'V', '2', '4') /* 24  Y/CbCr 
4:4:4  */
 #define V4L2_PIX_FMT_NV42v4l2_fourcc('N', 'V', '4', '2') /* 24  Y/CrCb 
4:4:4  */
+#define V4L2_PIX_FMT_XV20v4l2_fourcc('X', 'V', '2', '0') /* 32  XY/UV 
4:2:2 10-bit */
+#define V4L2_PIX_FMT_XV15v4l2_fourcc('X', 'V', '1', '5') /* 32  XY/UV 
4:2:0 10-bit */
 
 /* two non contiguous planes - one Y, one Cr + Cb interleaved  */
 #define V4L2_PIX_FMT_NV12M   v4l2_fourcc('N', 'M', '1', '2') /* 12  Y/CbCr 
4:2:0  */
-- 
2.1.1

[PATCH v4 10/10] v4l: xilinx: dma: Add support for 10 bit formats

[Satish Kumar Nagireddy]

This patch adds xvip_format_plane_width_bytes function to
calculate number of bytes for a macropixel formats and also
adds new 10 bit pixel formats to video descriptor table.

Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c |  5 ++--
 drivers/media/platform/xilinx/xilinx-vip.c | 43 +-
 drivers/media/platform/xilinx/xilinx-vip.h |  5 
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index a714057..b33e4b9 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -370,7 +370,8 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
}
 
dma->xt.frame_size = dma->fmtinfo->num_planes;
-   dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
+   dma->sgl[0].size = xvip_fmt_plane_width_bytes(dma->fmtinfo,
+ pix_mp->width);
dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
dma->xt.numf = pix_mp->height;
dma->sgl[0].dst_icg = 0;
@@ -596,7 +597,7 @@ __xvip_dma_try_format(struct xvip_dma *dma,
 * with the minimum in that case.
 */
max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
-   min_bpl = pix_mp->width * info->bpp[0];
+   min_bpl = xvip_fmt_plane_width_bytes(info, pix_mp->width);
min_bpl = roundup(min_bpl, align);
bpl = roundup(plane_fmt[0].bytesperline, align);
plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c 
b/drivers/media/platform/xilinx/xilinx-vip.c
index 81cc0d2..1825f5d 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -28,31 +28,31 @@
 
 static const struct xvip_video_format xvip_video_formats[] = {
{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
- {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
+ {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, 1, 1, "4:2:0, semi-planar, 
YUV" },
{ XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
- {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
+ {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, 4, 3, "4:2:0, 10-bit 2-plane 
cont" },
{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
- {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
+ {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, 1, 1, "4:2:2, packed, YUYV" },
{ XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
- {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
+ {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, 1, 1, "4:2:2, packed, UYVY" },
{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
- {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
+ {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, 1, 1, "4:2:2, semi-planar, 
YUV" },
{ XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
- {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
+ {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, 4, 3, "4:2:2, 10-bit 2-plane 
cont" },
{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
- {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
+ {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, 1, 1, "24-bit RGB" },
{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
- {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
+ {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, 1, 1, "24-bit RGB" },
{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
- {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
+ {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 1, "Greyscale 8-bit" },
{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
- {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
+ {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit RGGB" },
{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
- {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit GRBG" },
+ {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit GRBG" },
{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
- {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, "Bayer 8-bit GBRG" },
+ {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, 1, 1, "Bayer 8-bit GBRG" },
{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
- {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, "Bayer 8-bit BGGR" },
+ {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, 1, 1, "Bayer 8-bit BGGR" },
 };
 
 /**
@@ -102,6 +102,23 @@ const struct xvip_video_format 
*xvip_get_format_by_fourcc(u32 fourcc)
 EXPORT_SYMBOL_GPL(xvip_get_format_by_fourcc);
 
 /**
+ * 

[PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Satish Kumar Nagireddy]

The current v4l driver supports single plane formats. This patch
adds support to handle multi-planar formats. Driver can handle
both single and multi-planar formats.

Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++--
 drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
 drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
 3 files changed, 111 insertions(+), 68 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index 658586e..a714057 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
return ret == -ENOIOCTLCMD ? -EINVAL : ret;
 
if (dma->fmtinfo->code != fmt.format.code ||
-   dma->format.height != fmt.format.height ||
-   dma->format.width != fmt.format.width)
+   dma->format.fmt.pix_mp.width != fmt.format.width ||
+   dma->format.fmt.pix_mp.height != fmt.format.height)
return -EINVAL;
 
return 0;
@@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
buf->buf.field = V4L2_FIELD_NONE;
buf->buf.sequence = dma->sequence++;
buf->buf.vb2_buf.timestamp = ktime_get_ns();
-   vb2_set_plane_payload(>buf.vb2_buf, 0, dma->format.sizeimage);
+   vb2_set_plane_payload(>buf.vb2_buf, 0,
+ dma->format.fmt.pix_mp.plane_fmt[0].sizeimage);
vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
 }
 
@@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
 unsigned int sizes[], struct device *alloc_devs[])
 {
struct xvip_dma *dma = vb2_get_drv_priv(vq);
+   s32 sizeimage;
 
/* Make sure the image size is large enough. */
+   sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;
if (*nplanes)
-   return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
+   return sizes[0] < sizeimage ? -EINVAL : 0;
 
*nplanes = 1;
-   sizes[0] = dma->format.sizeimage;
+   sizes[0] = sizeimage;
 
return 0;
 }
@@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
struct dma_async_tx_descriptor *desc;
dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
u32 flags;
+   struct v4l2_pix_format_mplane *pix_mp = >format.fmt.pix_mp;
 
-   if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+   if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
dma->xt.dir = DMA_DEV_TO_MEM;
dma->xt.src_sgl = false;
dma->xt.dst_sgl = true;
dma->xt.dst_start = addr;
-   } else {
+   } else if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
dma->xt.dir = DMA_MEM_TO_DEV;
dma->xt.src_sgl = true;
@@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
dma->xt.src_start = addr;
}
 
-   dma->xt.frame_size = 1;
-   dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
-   dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
-   dma->xt.numf = dma->format.height;
+   dma->xt.frame_size = dma->fmtinfo->num_planes;
+   dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
+   dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
+   dma->xt.numf = pix_mp->height;
+   dma->sgl[0].dst_icg = 0;
 
desc = dmaengine_prep_interleaved_dma(dma->dma, >xt, flags);
if (!desc) {
@@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh, struct 
v4l2_capability *cap)
cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
  | dma->xdev->v4l2_caps;
 
-   if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
-   else
-   cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+   cap->device_caps = V4L2_CAP_STREAMING;
+   switch (dma->queue.type) {
+   case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+   cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
+   break;
+   case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+   cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
+   break;
+   }
 
strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
strlcpy(cap->card, dma->video.name, sizeof(cap->card));
@@ -524,7 +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct 
v4l2_fmtdesc *f)
if (f->index > 0)
return -EINVAL;
 
-   f->pixelformat = 

[PATCH v4 08/10] v4l: xilinx: dma: Update video format descriptor

[Satish Kumar Nagireddy]

This patch updates video format descriptor to help information
viz., number of planes per color format and chroma sub sampling
factors.

Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c | 12 ++--
 drivers/media/platform/xilinx/xilinx-vip.c | 28 +++-
 drivers/media/platform/xilinx/xilinx-vip.h |  8 +++-
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index 16aeb46..658586e 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -366,7 +366,7 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
}
 
dma->xt.frame_size = 1;
-   dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp;
+   dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
dma->xt.numf = dma->format.height;
 
@@ -569,12 +569,12 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct 
v4l2_pix_format *pix,
 * the minimum and maximum values, clamp the requested width and convert
 * it back to pixels.
 */
-   align = lcm(dma->align, info->bpp);
+   align = lcm(dma->align, info->bpp[0]);
min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
-   width = rounddown(pix->width * info->bpp, align);
+   width = rounddown(pix->width * info->bpp[0], align);
 
-   pix->width = clamp(width, min_width, max_width) / info->bpp;
+   pix->width = clamp(width, min_width, max_width) / info->bpp[0];
pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
XVIP_DMA_MAX_HEIGHT);
 
@@ -582,7 +582,7 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct 
v4l2_pix_format *pix,
 * line value is zero, the module doesn't support user configurable line
 * sizes. Override the requested value with the minimum in that case.
 */
-   min_bpl = pix->width * info->bpp;
+   min_bpl = pix->width * info->bpp[0];
max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
bpl = rounddown(pix->bytesperline, dma->align);
 
@@ -676,7 +676,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, 
struct xvip_dma *dma,
dma->format.field = V4L2_FIELD_NONE;
dma->format.width = XVIP_DMA_DEF_WIDTH;
dma->format.height = XVIP_DMA_DEF_HEIGHT;
-   dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp;
+   dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
 
/* Initialize the media entity... */
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c 
b/drivers/media/platform/xilinx/xilinx-vip.c
index 3112591..81cc0d2 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -27,22 +27,32 @@
  */
 
 static const struct xvip_video_format xvip_video_formats[] = {
+   { XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
+ {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
+   { XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
+ {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
- 2, V4L2_PIX_FMT_YUYV, "4:2:2, packed, YUYV" },
-   { XVIP_VF_YUV_444, 8, NULL, MEDIA_BUS_FMT_VUY8_1X24,
- 3, V4L2_PIX_FMT_YUV444, "4:4:4, packed, YUYV" },
+ {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
+   { XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+ {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
+   { XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+ {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
+   { XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+ {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
+   { XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
+ {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
- 3, 0, NULL },
+ {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
- 1, V4L2_PIX_FMT_GREY, "Greyscale 8-bit" },
+ {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
- 1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit RGGB" },
+ {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
- 1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit GRBG" },
+ {1, 

[PATCH v4 04/10] Documentation: uapi: media: v4l: New pixel format

[Satish Kumar Nagireddy]

From: Jeffrey Mouroux 

These descriptions are for YUV 420 and YUV 422 10 bit
formats.

Signed-off-by: Jeffrey Mouroux 
Signed-off-by: Satish Kumar Nagireddy 
---
 Documentation/media/uapi/v4l/pixfmt-xv15.rst | 135 ++
 Documentation/media/uapi/v4l/pixfmt-xv20.rst | 136 +++
 Documentation/media/uapi/v4l/yuv-formats.rst |   2 +
 3 files changed, 273 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv15.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv20.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-xv15.rst 
b/Documentation/media/uapi/v4l/pixfmt-xv15.rst
new file mode 100644
index 000..313d056
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-xv15.rst
@@ -0,0 +1,135 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-XV15:
+.. _V4L2-PIX-FMT-XV15M:
+
+***
+V4L2_PIX_FMT_XV15 ('XV15'), V4L2_PIX_FMT_XV15 ('XV15M')
+***
+
+Semi-planar YUV 420 10-bit
+
+
+Description
+===
+
+This is the 10-bit version of YUV 420 semi-planar format.
+XV15M differs from XV15 insofar as the chroma plane is not contiguous with the
+luma plane in memory.
+
+Each pixel of YUV 420 contains a single luma component of 10-bits in length.
+Three luma components are stored per word with the remaining two bits serving
+as padding.
+
+The chroma plane is subsampled and is only 1/2 the size of the luma plane.  A
+single chroma component serves two pixels on a given row and is re-used on the
+adjacent row of luma data.
+
+**Data Layout of Luma Plane**
+Each cell is one 32-bit word.
+
+.. flat-table::
+:header-rows:  0
+:stub-columns: 0
+
+* - word + 0:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`02 [29:20]`
+  - Y'\ :sub:`01 [19:10]`
+  - Y'\ :sub:`00 [09:00]`
+  -
+* - word + 1:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`05 [29:20]`
+  - Y'\ :sub:`04 [19:10]`
+  - Y'\ :sub:`03 [09:00]`
+  -
+* - word + 2:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`08 [29:20]`
+  - Y'\ :sub:`07 [19:10]`
+  - Y'\ :sub:`06 [09:00]`
+  -
+
+
+**Data Layout of Chroma Plane**
+
+.. flat-table::
+:header-rows:  0
+:stub-columns: 0
+
+* - word + 0:
+  - X'\ :sub:`[31:30]`
+  - U'\ :sub:`02 [29:20]`
+  - V'\ :sub:`01 [19:10]`
+  - U'\ :sub:`00 [09:00]`
+  -
+* - word + 1:
+  - X'\ :sub:`[31:30]`
+  - V'\ :sub:`05 [29:20]`
+  - U'\ :sub:`04 [19:10]`
+  - V'\ :sub:`03 [09:00]`
+  -
+* - word + 2:
+  - X'\ :sub:`[31:30]`
+  - U'\ :sub:`08 [29:20]`
+  - V'\ :sub:`07 [19:10]`
+  - U'\ :sub:`06 [09:00]`
+  -
+
+**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-xv20.rst 
b/Documentation/media/uapi/v4l/pixfmt-xv20.rst
new file mode 100644
index 000..fe9dac2
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-xv20.rst
@@ -0,0 +1,136 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-XV20:
+.. _V4L2-PIX-FMT-XV20M:
+
+***
+V4L2_PIX_FMT_XV20 ('XV20'), V4L2_PIX_FMT_XV20 ('XV20M')
+***
+
+Semi-planar YUV422 10-bit
+
+
+Description
+===
+
+This is the 10-bit version of YUV 422 semi-planar format.
+XV20M differs from XV20 insofar as the chroma plane is not contiquous with the
+luma plane in memory.
+
+
+Each pixel of YUV 422 contains a single luma component of 10-bits in length.
+Three luma components are stored per word with the remaining two bits serving
+as padding.
+
+The chroma plane is subsampled in and is the size of the luma plane.  A single
+chroma component (U or V) serves two pixels on a given row.
+
+**Data Layout of Luma Plane**
+Each cell is one 32-bit word.
+
+.. flat-table::
+:header-rows:  0
+:stub-columns: 0
+
+* - word + 0:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`02 [29:20]`
+  - Y'\ :sub:`01 [19:10]`
+  - Y'\ :sub:`00 [09:00]`
+  -
+* - word + 1:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`05 [29:20]`
+  - Y'\ :sub:`04 [19:10]`
+  - Y'\ :sub:`03 [09:00]`
+  -
+* - word + 2:
+  - X'\ :sub:`[31:30]`
+  - Y'\ :sub:`08 [29:20]`
+  - Y'\ :sub:`07 [19:10]`

[PATCH v4 01/10] v4l: xilinx: dma: Remove colorspace check in xvip_dma_verify_format

[Satish Kumar Nagireddy]

From: Radhey Shyam Pandey 

In current implementation driver only checks the colorspace
between the last subdev in the pipeline and the connected video node,
the pipeline could be configured with wrong colorspace information
until the very end. It thus makes little sense to check the
colorspace only at the video node. So check can be dropped until
we find a better solution to carry colorspace information
through pipelines and to userspace.

Signed-off-by: Radhey Shyam Pandey 
Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index 522cdfd..cb20ada 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -75,8 +75,7 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
 
if (dma->fmtinfo->code != fmt.format.code ||
dma->format.height != fmt.format.height ||
-   dma->format.width != fmt.format.width ||
-   dma->format.colorspace != fmt.format.colorspace)
+   dma->format.width != fmt.format.width)
return -EINVAL;
 
return 0;
-- 
2.1.1

[PATCH v4 02/10] xilinx: v4l: dma: Use the dmaengine_terminate_all() wrapper

[Satish Kumar Nagireddy]

From: Laurent Pinchart 

Calling dmaengine_device_control() to terminate transfers is an internal
API that will disappear, use the stable API wrapper instead.

Signed-off-by: Laurent Pinchart 
Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index cb20ada..a5bf345 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -434,6 +434,7 @@ static int xvip_dma_start_streaming(struct vb2_queue *vq, 
unsigned int count)
return 0;
 
 error_stop:
+   dmaengine_terminate_all(dma->dma);
media_pipeline_stop(>video.entity);
 
 error:
-- 
2.1.1

[PATCH v4 03/10] xilinx: v4l: dma: Update driver to allow for probe defer

[Satish Kumar Nagireddy]

From: Rohit Athavale 

Update xvip_dma_init() to use dma_request_chan(), enabling probe
deferral. Also update the cleanup routine to prevent dereferencing
an ERR_PTR().

Signed-off-by: Rohit Athavale 
Signed-off-by: Satish Kumar Nagireddy 
---
 drivers/media/platform/xilinx/xilinx-dma.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index a5bf345..16aeb46 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -730,10 +730,13 @@ int xvip_dma_init(struct xvip_composite_device *xdev, 
struct xvip_dma *dma,
 
/* ... and the DMA channel. */
snprintf(name, sizeof(name), "port%u", port);
-   dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
-   if (dma->dma == NULL) {
-   dev_err(dma->xdev->dev, "no VDMA channel found\n");
-   ret = -ENODEV;
+   dma->dma = dma_request_chan(dma->xdev->dev, name);
+   if (IS_ERR(dma->dma)) {
+   ret = PTR_ERR(dma->dma);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dma->xdev->dev,
+   "No Video DMA channel found");
+
goto error;
}
 
@@ -757,7 +760,7 @@ void xvip_dma_cleanup(struct xvip_dma *dma)
if (video_is_registered(>video))
video_unregister_device(>video);
 
-   if (dma->dma)
+   if (!IS_ERR(dma->dma))
dma_release_channel(dma->dma);
 
media_entity_cleanup(>video.entity);
-- 
2.1.1

[PATCH v4 00/10] Add support for multi-planar formats and 10 bit formats

[Satish Kumar Nagireddy]

 The patches are for xilinx v4l. The patcheset enable support to handle 
multiplanar
 formats and 10 bit formats. The implemenation has handling of single plane 
formats
 too for backward compatibility of some existing applications.

Changes in v4 (Thanks to Sakari Ailus, Hyun Kwon and Ian Arkver):
 - rst documentation is moved to 24 bit yuv formats group
 - Single plane implementation is removed as multi-plane supports both
 - num_buffers and bpl_factor parameters are removed to have clean
   implementation
 - macropixel concept is used to calculate number of bytes in a row
   for 10 bit formats
 - Video format descriptor table updated with 10 bit format information

Changes in v3:
 - Fixed table alignment issue in rst file. Ensured the output is proper uisng
   'make pdfdocs'

Changes in v2:
 - Added rst documentation for MEDIA_BUS_FMT_VYYUYY8_1X24

Jeffrey Mouroux (2):
  Documentation: uapi: media: v4l: New pixel format
  uapi: media: New fourcc codes needed by Xilinx Video IP

Laurent Pinchart (1):
  xilinx: v4l: dma: Use the dmaengine_terminate_all() wrapper

Radhey Shyam Pandey (1):
  v4l: xilinx: dma: Remove colorspace check in xvip_dma_verify_format

Rohit Athavale (2):
  xilinx: v4l: dma: Update driver to allow for probe defer
  media: Add new dt-bindings/vf_codes for supported formats

Satish Kumar Nagireddy (4):
  media-bus: uapi: Add YCrCb 420 media bus format
  v4l: xilinx: dma: Update video format descriptor
  v4l: xilinx: dma: Add multi-planar support
  v4l: xilinx: dma: Add support for 10 bit formats

 Documentation/media/uapi/v4l/pixfmt-xv15.rst| 135 ++
 Documentation/media/uapi/v4l/pixfmt-xv20.rst| 136 ++
 Documentation/media/uapi/v4l/subdev-formats.rst |  38 -
 Documentation/media/uapi/v4l/yuv-formats.rst|   2 +
 drivers/media/platform/xilinx/xilinx-dma.c  | 177 +++-
 drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
 drivers/media/platform/xilinx/xilinx-vip.c  |  45 --
 drivers/media/platform/xilinx/xilinx-vip.h  |  13 +-
 drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
 include/dt-bindings/media/xilinx-vip.h  |   2 +
 include/uapi/linux/media-bus-format.h   |   3 +-
 include/uapi/linux/videodev2.h  |   4 +
 12 files changed, 488 insertions(+), 87 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv15.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv20.rst

-- 
2.1.1

Re: [PATCH] media: v4l2-ioctl: fix function types for IOCTL_INFO_STD

[Kees Cook]

On Fri, Apr 27, 2018 at 11:59 AM, Sami Tolvanen  wrote:
> This change fixes indirect call mismatches with Control-Flow Integrity
> checking, which are caused by calling standard ioctls using a function
> pointer that doesn't match the type of the actual function.
>
> Signed-off-by: Sami Tolvanen 

I think this actually makes things much more readable in the end. Thanks!

Reviewed-by: Kees Cook 

-Kees


> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 72 ++--
>  1 file changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index f48c505550e0..d50a06ab3509 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2489,11 +2489,8 @@ struct v4l2_ioctl_info {
> unsigned int ioctl;
> u32 flags;
> const char * const name;
> -   union {
> -   u32 offset;
> -   int (*func)(const struct v4l2_ioctl_ops *ops,
> -   struct file *file, void *fh, void *p);
> -   } u;
> +   int (*func)(const struct v4l2_ioctl_ops *ops, struct file *file,
> +   void *fh, void *p);
> void (*debug)(const void *arg, bool write_only);
>  };
>
> @@ -2501,27 +2498,24 @@ struct v4l2_ioctl_info {
>  #define INFO_FL_PRIO   (1 << 0)
>  /* This control can be valid if the filehandle passes a control handler. */
>  #define INFO_FL_CTRL   (1 << 1)
> -/* This is a standard ioctl, no need for special code */
> -#define INFO_FL_STD(1 << 2)
>  /* This is ioctl has its own function */
> -#define INFO_FL_FUNC   (1 << 3)
> +#define INFO_FL_FUNC   (1 << 2)
>  /* Queuing ioctl */
> -#define INFO_FL_QUEUE  (1 << 4)
> +#define INFO_FL_QUEUE  (1 << 3)
>  /* Always copy back result, even on error */
> -#define INFO_FL_ALWAYS_COPY(1 << 5)
> +#define INFO_FL_ALWAYS_COPY(1 << 4)
>  /* Zero struct from after the field to the end */
>  #define INFO_FL_CLEAR(v4l2_struct, field)  \
> ((offsetof(struct v4l2_struct, field) + \
>   sizeof(((struct v4l2_struct *)0)->field)) << 16)
>  #define INFO_FL_CLEAR_MASK (_IOC_SIZEMASK << 16)
>
> -#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)  
>   \
> -   [_IOC_NR(_ioctl)] = {   \
> -   .ioctl = _ioctl,\
> -   .flags = _flags | INFO_FL_STD,  \
> -   .name = #_ioctl,\
> -   .u.offset = offsetof(struct v4l2_ioctl_ops, _vidioc),   \
> -   .debug = _debug,\
> +#define DEFINE_IOCTL_STD_FNC(_vidioc)  \
> +   static int __v4l_ ## _vidioc ## _fnc(   \
> +   const struct v4l2_ioctl_ops *ops,   \
> +   struct file *file, void *fh, void *p)   \
> +   {   \
> +   return ops->_vidioc(file, fh, p);   \
> }
>
>  #define IOCTL_INFO_FNC(_ioctl, _func, _debug, _flags)  \
> @@ -2529,10 +2523,42 @@ struct v4l2_ioctl_info {
> .ioctl = _ioctl,\
> .flags = _flags | INFO_FL_FUNC, \
> .name = #_ioctl,\
> -   .u.func = _func,\
> +   .func = _func,  \
> .debug = _debug,\
> }
>
> +#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)\
> +   IOCTL_INFO_FNC(_ioctl, __v4l_ ## _vidioc ## _fnc, _debug, _flags)
> +
> +DEFINE_IOCTL_STD_FNC(vidioc_g_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_expbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_std)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_input)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_output)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_jpegcomp)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_jpegcomp)
> +DEFINE_IOCTL_STD_FNC(vidioc_enumaudio)
> +DEFINE_IOCTL_STD_FNC(vidioc_enumaudout)
> +DEFINE_IOCTL_STD_FNC(vidioc_enum_framesizes)
> +DEFINE_IOCTL_STD_FNC(vidioc_enum_frameintervals)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_enc_index)
> +DEFINE_IOCTL_STD_FNC(vidioc_encoder_cmd)
> +DEFINE_IOCTL_STD_FNC(vidioc_try_encoder_cmd)
> +DEFINE_IOCTL_STD_FNC(vidioc_decoder_cmd)

Re: [PATCH] media: media-device: fix ioctl function types

[Kees Cook]

On Fri, Apr 27, 2018 at 12:54 PM, Sami Tolvanen  wrote:
> This change fixes function types for media device ioctls to avoid
> indirect call mismatches with Control-Flow Integrity checking.
>
> Signed-off-by: Sami Tolvanen 

Thanks for sending these!

Reviewed-by: Kees Cook 

-Kees

> ---
>  drivers/media/media-device.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 35e81f7c0d2f..bc5c024906e6 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -54,9 +54,10 @@ static int media_device_close(struct file *filp)
> return 0;
>  }
>
> -static int media_device_get_info(struct media_device *dev,
> -struct media_device_info *info)
> +static long media_device_get_info(struct media_device *dev, void *arg)
>  {
> +   struct media_device_info *info = (struct media_device_info *)arg;
> +
> memset(info, 0, sizeof(*info));
>
> if (dev->driver_name[0])
> @@ -93,9 +94,9 @@ static struct media_entity *find_entity(struct media_device 
> *mdev, u32 id)
> return NULL;
>  }
>
> -static long media_device_enum_entities(struct media_device *mdev,
> -  struct media_entity_desc *entd)
> +static long media_device_enum_entities(struct media_device *mdev, void *arg)
>  {
> +   struct media_entity_desc *entd = (struct media_entity_desc *)arg;
> struct media_entity *ent;
>
> ent = find_entity(mdev, entd->id);
> @@ -146,9 +147,9 @@ static void media_device_kpad_to_upad(const struct 
> media_pad *kpad,
> upad->flags = kpad->flags;
>  }
>
> -static long media_device_enum_links(struct media_device *mdev,
> -   struct media_links_enum *links)
> +static long media_device_enum_links(struct media_device *mdev, void *arg)
>  {
> +   struct media_links_enum *links = (struct media_links_enum *)arg;
> struct media_entity *entity;
>
> entity = find_entity(mdev, links->entity);
> @@ -195,9 +196,9 @@ static long media_device_enum_links(struct media_device 
> *mdev,
> return 0;
>  }
>
> -static long media_device_setup_link(struct media_device *mdev,
> -   struct media_link_desc *linkd)
> +static long media_device_setup_link(struct media_device *mdev, void *arg)
>  {
> +   struct media_link_desc *linkd = (struct media_link_desc *)arg;
> struct media_link *link = NULL;
> struct media_entity *source;
> struct media_entity *sink;
> @@ -225,9 +226,9 @@ static long media_device_setup_link(struct media_device 
> *mdev,
> return __media_entity_setup_link(link, linkd->flags);
>  }
>
> -static long media_device_get_topology(struct media_device *mdev,
> - struct media_v2_topology *topo)
> +static long media_device_get_topology(struct media_device *mdev, void *arg)
>  {
> +   struct media_v2_topology *topo = (struct media_v2_topology *)arg;
> struct media_entity *entity;
> struct media_interface *intf;
> struct media_pad *pad;
> --
> 2.17.0.441.gb46fe60e1d-goog
>



-- 
Kees Cook
Pixel Security

Re: [Intel-gfx] [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Eric Anholt]

Daniel Vetter  writes:
> + /**
> +  * @fill_driver_data:
> +  *
> +  * Callback to fill in free-form debug info Returns amount of bytes
> +  * filled, or negative error on failure.

Maybe this "Returns" should be on a new line?  Or at least a '.' in
between.

Other than that,

Reviewed-by: Eric Anholt 

Thanks!


signature.asc
Description: PGP signature

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

[Kieran Bingham]

Hi Laurent,

On 07/04/18 01:23, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 8 March 2018 02:05:31 EEST Kieran Bingham wrote:
>> We are now able to configure a pipeline directly into a local display
>> list body. Take advantage of this fact, and create a cacheable body to
>> store the configuration of the pipeline in the video object.
>>
>> vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
>> Convert this function to use the cached video->config body and obtain a
>> local display list reference.
>>
>> Attach the video->config body to the display list when needed before
>> committing to hardware.
>>
>> The pipe object is marked as un-configured when resuming from a suspend.
>> This ensures that when the hardware is reset - our cached configuration
>> will be re-attached to the next committed DL.
>>
>> Signed-off-by: Kieran Bingham 
>> ---
>>
>> v3:
>>  - 's/fragment/body/', 's/fragments/bodies/'
>>  - video dlb cache allocation increased from 2 to 3 dlbs
>>
>> Our video DL usage now looks like the below output:
>>
>> dl->body0 contains our disposable runtime configuration. Max 41.
>> dl_child->body0 is our partition specific configuration. Max 12.
>> dl->bodies shows our constant configuration and LUTs.
>>
>>   These two are LUT/CLU:
>>  * dl->bodies[x]->num_entries 256 / max 256
>>  * dl->bodies[x]->num_entries 4914 / max 4914
>>
>> Which shows that our 'constant' configuration cache is currently
>> utilised to a maximum of 64 entries.
>>
>> trace-cmd report | \
>> grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;
>>
>>   dl->body0->num_entries 13 / max 128
>>   dl->body0->num_entries 14 / max 128
>>   dl->body0->num_entries 16 / max 128
>>   dl->body0->num_entries 20 / max 128
>>   dl->body0->num_entries 27 / max 128
>>   dl->body0->num_entries 34 / max 128
>>   dl->body0->num_entries 41 / max 128
>>   dl_child->body0->num_entries 10 / max 128
>>   dl_child->body0->num_entries 12 / max 128
>>   dl->bodies[x]->num_entries 15 / max 128
>>   dl->bodies[x]->num_entries 16 / max 128
>>   dl->bodies[x]->num_entries 17 / max 128
>>   dl->bodies[x]->num_entries 18 / max 128
>>   dl->bodies[x]->num_entries 20 / max 128
>>   dl->bodies[x]->num_entries 21 / max 128
>>   dl->bodies[x]->num_entries 256 / max 256
>>   dl->bodies[x]->num_entries 31 / max 128
>>   dl->bodies[x]->num_entries 32 / max 128
>>   dl->bodies[x]->num_entries 39 / max 128
>>   dl->bodies[x]->num_entries 40 / max 128
>>   dl->bodies[x]->num_entries 47 / max 128
>>   dl->bodies[x]->num_entries 48 / max 128
>>   dl->bodies[x]->num_entries 4914 / max 4914
>>   dl->bodies[x]->num_entries 55 / max 128
>>   dl->bodies[x]->num_entries 56 / max 128
>>   dl->bodies[x]->num_entries 63 / max 128
>>   dl->bodies[x]->num_entries 64 / max 128
> 
> This might be useful to capture in the main part of the commit message.
> 
>> v4:
>>  - Adjust pipe configured flag to be reset on resume rather than suspend
>>  - rename dl_child, dl_next
>>
>>  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
>>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
>>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>>  4 files changed, 54 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>> b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..fa445b1a2e38
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>>  vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>> VI6_CMD_STRCMD);
>>  pipe->state = VSP1_PIPELINE_RUNNING;
>> +pipe->configured = true;
>>  }
>>
>>  pipe->buffers_ready = 0;
>> @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
>>  continue;
>>
>>  spin_lock_irqsave(>irqlock, flags);
>> +/*
>> + * The hardware may have been reset during a suspend and will
>> + * need a full reconfiguration
>> + */
> 
> s/reconfiguration/reconfiguration./
> 
>> +pipe->configured = false;
>> +
> 
> Where does that full reconfiguration occur, given that the 
> vsp1_pipeline_run() 
> right below sets pipe->configured to true without performing reconfiguration ?

It's magic isn't it :D

If the pipe->configured flag gets set to false, the next execution of
vsp1_pipeline_run() attaches the video->pipe_config (the cached configuration,
containing the route_setup() and the configure_stream() entries) to the display
list before configuring for the next frame.

This means that the hardware gets a full configuration written to it after a
suspend/resume action.

Perhaps the comment should say "The video object will write out it's cached pipe

[PATCH] media: videobuf-dma-sg: Fix dma_{sync,unmap}_sg() calls

[Robin Murphy]

This reverts commit fc7f8fd42c2b934ac348995e0c530c917fc277d5.

Whilst the rationale for the above commit was in general correct, i.e.
that users *consuming* the DMA addresses should rely on sglen rather
than num_pages, it has always been the case that the DMA API itself
still requires that dma_{sync,unmap}_sg() are called with the original
number of entries as passed to dma_map_sg(), not the number of mapped
entries it returned. Thus the particular changes made in that patch
were erroneous.

At worst this might lead to data loss at the tail end of mapped buffers
on non-coherent hardware, while at best it's an example of incorrect
DMA API usage which has proven to mislead readers.

Signed-off-by: Robin Murphy 
---
 drivers/media/v4l2-core/videobuf-dma-sg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index add2edb23eac..37550f81cc29 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -334,7 +334,7 @@ int videobuf_dma_unmap(struct device *dev, struct 
videobuf_dmabuf *dma)
if (!dma->sglen)
return 0;
 
-   dma_unmap_sg(dev, dma->sglist, dma->sglen, dma->direction);
+   dma_unmap_sg(dev, dma->sglist, dma->nr_pages, dma->direction);
 
vfree(dma->sglist);
dma->sglist = NULL;
@@ -581,7 +581,7 @@ static int __videobuf_sync(struct videobuf_queue *q,
MAGIC_CHECK(mem->dma.magic, MAGIC_DMABUF);
 
dma_sync_sg_for_cpu(q->dev, mem->dma.sglist,
-   mem->dma.sglen, mem->dma.direction);
+   mem->dma.nr_pages, mem->dma.direction);
 
return 0;
 }
-- 
2.17.0.dirty

Re: [PATCH v7 6/8] media: vsp1: Refactor display list configure operations

[Kieran Bingham]

Hi Laurent,

On 07/04/18 00:38, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 8 March 2018 02:05:29 EEST Kieran Bingham wrote:
>> The entities provide a single .configure operation which configures the
>> object into the target display list, based on the vsp1_entity_params
>> selection.
>>
>> This restricts us to a single function prototype for both static
>> configuration (the pre-stream INIT stage) and the dynamic runtime stages
>> for both each frame - and each partition therein.
>>
>> Split the configure function into two parts, '.configure_stream()' and
>> '.configure_frame()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
>> VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
>> .configure_frame(). The configuration for individual partitions is
>> handled by passing the partition number to the configure call, and
>> processing any runtime stage actions on the first partition only.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>> v7
>>  - Fix formatting and white space
>>  - s/prepare/configure_stream/
>>  - s/configure/configure_frame/
>>
>>  drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
>>  drivers/media/platform/vsp1/vsp1_clu.c|  50 +---
>>  drivers/media/platform/vsp1/vsp1_dl.h |   1 +-
>>  drivers/media/platform/vsp1/vsp1_drm.c|  21 +--
>>  drivers/media/platform/vsp1/vsp1_entity.c |  17 +-
>>  drivers/media/platform/vsp1/vsp1_entity.h |  33 +--
>>  drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
>>  drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
>>  drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
>>  drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
>>  drivers/media/platform/vsp1/vsp1_lut.c|  32 +-
>>  drivers/media/platform/vsp1/vsp1_rpf.c| 164 ++---
>>  drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
>>  drivers/media/platform/vsp1/vsp1_uds.c|  57 ++--
>>  drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
>>  drivers/media/platform/vsp1/vsp1_wpf.c| 299 ---
>>  16 files changed, 378 insertions(+), 392 deletions(-)
> 
> [snip]
> 
>> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
>> b/drivers/media/platform/vsp1/vsp1_clu.c index b2a39a6ef7e4..b8d8af6d4910
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_clu.c
>> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
>> @@ -213,37 +213,36 @@ static const struct v4l2_subdev_ops clu_ops = {
>>  /*
>> ---
>> -- * VSP1 Entity Operations
>>   */
>> +static void clu_configure_stream(struct vsp1_entity *entity,
>> + struct vsp1_pipeline *pipe,
>> + struct vsp1_dl_list *dl)
>> +{
>> +struct vsp1_clu *clu = to_clu(>subdev);
>> +
>> +/*
>> + * The yuv_mode can't be changed during streaming. Cache it internally
>> + * for future runtime configuration calls.
>> + */
> 
> I'd move this comment right before the vsp1_entity_get_pad_format() call to 
> keep all variable declarations together.

Agreed, Done.

> 
>> +struct v4l2_mbus_framefmt *format;
>> +
>> +format = vsp1_entity_get_pad_format(>entity,
>> +clu->entity.config,
>> +CLU_PAD_SINK);
>> +clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
>> +}
> 
> [snip]
> 
> 
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h
>> b/drivers/media/platform/vsp1/vsp1_dl.h index 7e820ac6865a..f45083251644
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.h
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
>> @@ -41,7 +41,6 @@ vsp1_dl_body_pool_create(struct vsp1_device *vsp1,
>> unsigned int num_bodies, void vsp1_dl_body_pool_destroy(struct
>> vsp1_dl_body_pool *pool);
>>  struct vsp1_dl_body *vsp1_dl_body_get(struct vsp1_dl_body_pool *pool);
>>  void vsp1_dl_body_put(struct vsp1_dl_body *dlb);
>> -
> 
> This is an unrelated change.
> 

Removed

>>  void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data);
>>  int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body
>> *dlb);
>>  int vsp1_dl_list_add_chain(struct vsp1_dl_list *head, struct vsp1_dl_list
>>  *dl);
> 
> [snip]
> 
>> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
>> b/drivers/media/platform/vsp1/vsp1_entity.h index
>> 408602ebeb97..b44ed5414fc3 100644
>> --- a/drivers/media/platform/vsp1/vsp1_entity.h
>> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> 
> [snip]
> 
>> @@ -80,8 +68,10 @@ struct vsp1_route {
>>  /**
>>   * struct vsp1_entity_operations - Entity operations
>>   * @destroy:Destroy the entity.
>> - * @configure:  Setup the hardware based on the entity state (pipeline,
>> formats,
>> - *  selection rectangles, ...)
>> + * @configure_stream:   Setup the initial hardware parameters for the 
> stream
>> + *  (pipeline, formats)
> 
> Instead of initial I would 

Re: [PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Daniel Vetter]

On Sun, Apr 29, 2018 at 09:11:31AM +0200, Christian König wrote:
> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> > When this was introduced in
> > 
> > commit a519435a96597d8cd96123246fea4ae5a6c90b02
> > Author: Christian König 
> > Date:   Tue Oct 20 16:34:16 2015 +0200
> > 
> >  dma-buf/fence: add fence_wait_any_timeout function v2
> > 
> > there was a restriction added that this only works if the dma-fence
> > uses the dma_fence_default_wait hook. Which works for amdgpu, which is
> > the only caller. Well, until you share some buffers with e.g. i915,
> > then you get an -EINVAL.
> > 
> > But there's really no reason for this, because all drivers must
> > support callbacks. The special ->wait hook is only as an optimization;
> > if the driver needs to create a worker thread for an active callback,
> > then it can avoid to do that if it knows that there's a process
> > context available already. So ->wait is just an optimization, just
> > using the logic in dma_fence_default_wait() should work for all
> > drivers.
> > 
> > Let's remove this restriction.
> 
> Mhm, that was intentional introduced because for radeon that is not only an
> optimization, but mandatory for correct operation.
> 
> On the other hand radeon isn't using this function, so it should be fine as
> long as the Intel driver can live with it.

Well dma-buf already requires that dma_fence_add_callback works correctly.
And so do various users of it as soon as you engage in a bit of buffer
sharing. I guess whomever cares about buffer sharing with radeon gets to
fix this (you need to spawn a kthread or whatever in ->enable_signaling
which does the same work as your optimized ->wait callback).

But yeah, I'm definitely not making things work with this series, just a
bit more obvious that there's a problem already.
-Daniel

> 
> Christian.
> 
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Sumit Semwal 
> > Cc: Gustavo Padovan 
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Cc: Christian König 
> > Cc: Alex Deucher 
> > ---
> >   drivers/dma-buf/dma-fence.c | 5 -
> >   1 file changed, 5 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 7b5b40d6b70e..59049375bd19 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, 
> > uint32_t count,
> > for (i = 0; i < count; ++i) {
> > struct dma_fence *fence = fences[i];
> > -   if (fence->ops->wait != dma_fence_default_wait) {
> > -   ret = -EINVAL;
> > -   goto fence_rm_cb;
> > -   }
> > -
> > cb[i].task = current;
> > if (dma_fence_add_callback(fence, [i].base,
> >dma_fence_default_wait_cb)) {
> 

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

Re: [PATCH v8 10/13] [media] vb2: add out-fence support to QBUF

[Ezequiel Garcia]

Hi guys,

I've a couple questions.

On 9 March 2018 at 14:49, Gustavo Padovan  wrote:
> From: Gustavo Padovan 
>
> If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
> an out_fence and send its fd to userspace on the fence_fd field as a
> return arg for the QBUF call.
>
> The fence is signaled on buffer_done(), when the job on the buffer is
> finished.
>
> v9: - remove in-fences changes from this patch (Alex Courbot)
> - improve fence context creation (Hans Verkuil)
> - clean up out fences if vb2_core_qbuf() fails (Hans Verkuil)
>
> v8:
> - return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
> - fix crash when checking not using fences in vb2_buffer_done()
>
> v7:
> - merge patch that add the infrastructure to out-fences into
> this one (Alex Courbot)
> - Do not install the fd if there is no fence. (Alex Courbot)
> - do not report error on requeueing, just WARN_ON_ONCE() (Hans)
>
> v6
> - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
> ordering in vb2 for queueing in the driver, so the event is not
> necessary anymore and the out_fence_fd is sent back to userspace
> on QBUF call return arg
> - do not allow requeueing with out-fences, instead mark the buffer
> with an error and wake up to userspace.
> - send the out_fence_fd back to userspace on the fence_fd field
>
> v5:
> - delay fd_install to DQ_EVENT (Hans)
> - if queue is fully ordered send OUT_FENCE event right away
> (Brian)
> - rename 'q->ordered' to 'q->ordered_in_driver'
> - merge change to implement OUT_FENCE event here
>
> v4:
> - return the out_fence_fd in the BUF_QUEUED event(Hans)
>
> v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
> - set the OUT_FENCE flag if there is a fence pending (Hans)
> - call fd_install() after vb2_core_qbuf() (Hans)
> - clean up fence if vb2_core_qbuf() fails (Hans)
> - add list to store sync_file and fence for the next queued buffer
>
> v2: check if the queue is ordered.
>
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 88 
> +
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 20 +-
>  include/media/videobuf2-core.h  | 25 +++
>  3 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 5de5e35cfc40..dd18a9f345c7 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -357,6 +358,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
> vb2_memory memory,
> vb->planes[plane].length = plane_sizes[plane];
> vb->planes[plane].min_length = plane_sizes[plane];
> }
> +   vb->out_fence_fd = -1;
> q->bufs[vb->index] = vb;
>
> /* Allocate video buffer memory for the MMAP type */
> @@ -934,10 +936,22 @@ static void vb2_process_buffer_done(struct vb2_buffer 
> *vb, enum vb2_buffer_state
> case VB2_BUF_STATE_QUEUED:
> return;
> case VB2_BUF_STATE_REQUEUEING:
> +   /* Requeuing with explicit synchronization, spit warning */
> +   WARN_ON_ONCE(vb->out_fence);
> +
> if (q->start_streaming_called)
> __enqueue_in_driver(vb);
> return;
> default:
> +   if (vb->out_fence) {
> +   if (state == VB2_BUF_STATE_ERROR)
> +   dma_fence_set_error(vb->out_fence, -EFAULT);
> +   dma_fence_signal(vb->out_fence);
> +   dma_fence_put(vb->out_fence);
> +   vb->out_fence = NULL;
> +   vb->out_fence_fd = -1;
> +   }
> +
> /* Inform any processes that may be waiting for buffers */
> wake_up(>done_wq);
> break;
> @@ -1353,6 +1367,62 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
> int index, void *pb)
>  }
>  EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
>
> +static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
> +{
> +   return "vb2_fence";
> +}
> +
> +static inline const char *vb2_fence_get_timeline_name(struct dma_fence 
> *fence)
> +{
> +   return "vb2_fence_timeline";
> +}
> +
> +static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
> +{
> +   return true;
> +}
> +
> +static const struct dma_fence_ops vb2_fence_ops = {
> +   .get_driver_name = 

Re: [PATCH][next] media: ispstat: don't dereference user_cfg before a null check

[Gustavo A. R. Silva]

Hi Sakari,

On 04/30/2018 10:15 AM, Sakari Ailus wrote:

Isn't there a guarantee that new_buf won't be NULL ? The new_buf pointer comes
from the parg variable in video_usercopy(), which should always point to a
valid buffer given that the ioctl number specifies a non-zero size.


Fair question. After looking at the code, I agree with you; there should be
no reason to perform the check in the first place. It may have been that
the function has been used differently in the past but the check should be
rather removed now.

I'll drop the patch.



Please, if the check isn't needed anymore, make sure it is removed.

This helps to reduce the number of false positives reported by static 
analyzers.


Thanks
--
Gustavo

Re: [PATCH][next] media: ispstat: don't dereference user_cfg before a null check

[Sakari Ailus]

On Thu, Apr 26, 2018 at 01:03:15PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Thursday, 26 April 2018 11:37:31 EEST Sakari Ailus wrote:
> > On Tue, Apr 24, 2018 at 02:06:18PM +0100, Colin King wrote:
> > > From: Colin Ian King 
> > > 
> > > The pointer user_cfg (a copy of new_conf) is dereference before
> > > new_conf is null checked, hence we may have a null pointer dereference
> > > on user_cfg when assigning buf_size from user_cfg->buf_size. Ensure
> > > this does not occur by moving the assignment of buf_size after the
> > > null check.
> > > 
> > > Detected by CoverityScan, CID#1468386 ("Dereference before null check")
> > > 
> > > Fixes: 68e342b3068c ("[media] omap3isp: Statistics")
> > > Signed-off-by: Colin Ian King 
> > 
> > Thanks for the patch.
> > 
> > Gustavo sent effectively the same patch a moment earlier, and that patch
> > got applied instead.
> 
> Isn't there a guarantee that new_buf won't be NULL ? The new_buf pointer 
> comes 
> from the parg variable in video_usercopy(), which should always point to a 
> valid buffer given that the ioctl number specifies a non-zero size.

Fair question. After looking at the code, I agree with you; there should be
no reason to perform the check in the first place. It may have been that
the function has been used differently in the past but the check should be
rather removed now.

I'll drop the patch.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Sakari Ailus]

On Mon, Apr 30, 2018 at 09:30:40AM -0500, Yong Zhi wrote:
> From: Bingbu Cao 
> 
> Interrupt behavior shows that some time the frame end and frame start
> of next frame is unstable and can range from several to hundreds of micro-sec.
> In the case of ~10us, isr may not clear next sof interrupt status in
> single handling, which prevents new interrupts from coming.
> 
> Fix this by handling all pending IRQs before exiting isr, so any abnormal
> behavior results from very short interrupt status changes is protected.
> 
> Signed-off-by: Bingbu Cao 
> Signed-off-by: Andy Yeh 
> Signed-off-by: Yong Zhi 
> ---
> Hi, Sakari,
> 
> Re-send with correct signed-off-by order.

Thanks, Yong. I've replaced the patch in my ipu3 branch, with the commit
message wrapped --- after adding four spaces to the left margin it exceeded
80 characters.

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

Re: [RFCv11 PATCH 26/29] vim2m: use workqueue

[Hans Verkuil]

On 12/04/18 11:02, Tomasz Figa wrote:
> Hi Hans,
> 
> On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil  wrote:
> 
>> From: Hans Verkuil 
> 
>> v4l2_ctrl uses mutexes, so we can't setup a ctrl_handler in
>> interrupt context. Switch to a workqueue instead.
> 
> Could it make more sense to just replace the old (non-hr) timer used in
> this driver with delayed work?

I agree. I'll change that once v12 is posted to the mailing list.

Regards,

Hans

> 
> Best regards,
> Tomasz
> 

Re: [RFCv11 PATCH 26/29] vim2m: use workqueue

[Hans Verkuil]

On 11/04/18 16:06, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2018-04-09 at 16:20 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> v4l2_ctrl uses mutexes, so we can't setup a ctrl_handler in
>> interrupt context. Switch to a workqueue instead.
> 
> See one comment below.
> 
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/platform/vim2m.c | 15 +--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/vim2m.c
>> b/drivers/media/platform/vim2m.c
>> index ef970434af13..9b18b32c255d 100644
>> --- a/drivers/media/platform/vim2m.c
>> +++ b/drivers/media/platform/vim2m.c
>> @@ -150,6 +150,7 @@ struct vim2m_dev {
>>  spinlock_t  irqlock;
>>  
>>  struct timer_list   timer;
>> +struct work_struct  work_run;
> 
> Wouldn't it make more sense to move this to vim2m_ctx instead (since
> this is heavily m2m-specific)?

The work is triggered by a timer which is m2m_dev specific. So it makes
no sense to move this to the per-filehandle vim2m_ctx IMHO.

Regards,

Hans

> 
>>  struct v4l2_m2m_dev *m2m_dev;
>>  };
>> @@ -392,9 +393,10 @@ static void device_run(void *priv)
>>  schedule_irq(dev, ctx->transtime);
>>  }
>>  
>> -static void device_isr(struct timer_list *t)
>> +static void device_work(struct work_struct *w)
>>  {
>> -struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t,
>> timer);
>> +struct vim2m_dev *vim2m_dev =
>> +container_of(w, struct vim2m_dev, work_run);
>>  struct vim2m_ctx *curr_ctx;
>>  struct vb2_v4l2_buffer *src_vb, *dst_vb;
>>  unsigned long flags;
>> @@ -426,6 +428,13 @@ static void device_isr(struct timer_list *t)
>>  }
>>  }
>>  
>> +static void device_isr(struct timer_list *t)
>> +{
>> +struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t,
>> timer);
>> +
>> +schedule_work(_dev->work_run);
>> +}
>> +
>>  /*
>>   * video ioctls
>>   */
>> @@ -806,6 +815,7 @@ static void vim2m_stop_streaming(struct vb2_queue
>> *q)
>>  struct vb2_v4l2_buffer *vbuf;
>>  unsigned long flags;
>>  
>> +flush_scheduled_work();
>>  for (;;) {
>>  if (V4L2_TYPE_IS_OUTPUT(q->type))
>>  vbuf = v4l2_m2m_src_buf_remove(ctx-
>>> fh.m2m_ctx);
>> @@ -1011,6 +1021,7 @@ static int vim2m_probe(struct platform_device
>> *pdev)
>>  vfd = >vfd;
>>  vfd->lock = >dev_mutex;
>>  vfd->v4l2_dev = >v4l2_dev;
>> +INIT_WORK(>work_run, device_work);
>>  
>>  #ifdef CONFIG_MEDIA_CONTROLLER
>>  dev->mdev.dev = >dev;

Re: [PATCH v7 4/8] media: vsp1: Convert display lists to use new body pool

[Kieran Bingham]

Hi Laurent,

On 06/04/18 23:55, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 8 March 2018 02:05:27 EEST Kieran Bingham wrote:
>> Adapt the dl->body0 object to use an object from the body pool. This
>> greatly reduces the pressure on the TLB for IPMMU use cases, as all of
>> the lists use a single allocation for the main body.
>>
>> The CLU and LUT objects pre-allocate a pool containing three bodies,
>> allowing a userspace update before the hardware has committed a previous
>> set of tables.
>>
>> Bodies are no longer 'freed' in interrupt context, but instead released
>> back to their respective pools. This allows us to remove the garbage
>> collector in the DLM.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>> v3:
>>  - 's/fragment/body', 's/fragments/bodies/'
>>  - CLU/LUT now allocate 3 bodies
>>  - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put
>>
>> v2:
>>  - Use dl->body0->max_entries to determine header offset, instead of the
>>global constant VSP1_DL_NUM_ENTRIES which is incorrect.
>>  - squash updates for LUT, CLU, and fragment cleanup into single patch.
>>(Not fully bisectable when separated)
>>
>>  drivers/media/platform/vsp1/vsp1_clu.c |  27 ++-
>>  drivers/media/platform/vsp1/vsp1_clu.h |   1 +-
>>  drivers/media/platform/vsp1/vsp1_dl.c  | 223 ++
>>  drivers/media/platform/vsp1/vsp1_dl.h  |   3 +-
>>  drivers/media/platform/vsp1/vsp1_lut.c |  27 ++-
>>  drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
>>  6 files changed, 101 insertions(+), 181 deletions(-)
> 
> Still a nice diffstart :-)
> 
> [snip]
> 
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index 0208e72cb356..74476726451c
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> 
> [snip]
> 
>> @@ -399,11 +311,10 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32
>> reg, u32 data) * Display List Transaction Management
>>   */
>>
>> -static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
>> +static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager
>> *dlm,
>> +   struct vsp1_dl_body_pool *pool)
> 
> Given that the only caller of this function passes dlm->pool as the second 
> argument, can't you remove the second argument ?

Hrm ... I thought there was going to be a use case where the pool will be
separated. But perhaps not.

So yes - Removing.

> 
>>  {
>>  struct vsp1_dl_list *dl;
>> -size_t header_size;
>> -int ret;
>>
>>  dl = kzalloc(sizeof(*dl), GFP_KERNEL);
>>  if (!dl)
>> @@ -412,41 +323,39 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct
>> vsp1_dl_manager *dlm) INIT_LIST_HEAD(>bodies);
>>  dl->dlm = dlm;
>>
>> -/*
>> - * Initialize the display list body and allocate DMA memory for the body
>> - * and the optional header. Both are allocated together to avoid memory
>> - * fragmentation, with the header located right after the body in
>> - * memory.
>> - */
>> -header_size = dlm->mode == VSP1_DL_MODE_HEADER
>> -? ALIGN(sizeof(struct vsp1_dl_header), 8)
>> -: 0;
>> -
>> -ret = vsp1_dl_body_init(dlm->vsp1, >body0, VSP1_DL_NUM_ENTRIES,
>> -header_size);
>> -if (ret < 0) {
>> -kfree(dl);
>> +/* Retrieve a body from our DLM body pool */
> 
> s/body pool/body pool./
> 
> (And I would have said "Get a body" but that's up to you)

I think that's evident by the function name "vsp1_dl_body_get()", thus I've
adapted this comment to be a bit more meaningful:
/* Get a default body for our list. */

But I'm not opposed to dropping the comment. Also at somepoint, I think there's
scope to remove the dl->body0 so it may not matter.

> 
>> +dl->body0 = vsp1_dl_body_get(pool);
>> +if (!dl->body0)
>>  return NULL;
>> -}
>> -
>>  if (dlm->mode == VSP1_DL_MODE_HEADER) {
>> -size_t header_offset = VSP1_DL_NUM_ENTRIES
>> - * sizeof(*dl->body0.entries);
>> +size_t header_offset = dl->body0->max_entries
>> + * sizeof(*dl->body0->entries);
>>
>> -dl->header = ((void *)dl->body0.entries) + header_offset;
>> -dl->dma = dl->body0.dma + header_offset;
>> +dl->header = ((void *)dl->body0->entries) + header_offset;
>> +dl->dma = dl->body0->dma + header_offset;
>>
>>  memset(dl->header, 0, sizeof(*dl->header));
>> -dl->header->lists[0].addr = dl->body0.dma;
>> +dl->header->lists[0].addr = dl->body0->dma;
>>  }
>>
>>  return dl;
>>  }
>>
>> +static void vsp1_dl_list_bodies_put(struct vsp1_dl_list *dl)
>> +{
>> +struct vsp1_dl_body *dlb, *tmp;
>> +
>> +list_for_each_entry_safe(dlb, tmp, >bodies, list) {
>> 

[PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Yong Zhi]

From: Bingbu Cao 

Interrupt behavior shows that some time the frame end and frame start
of next frame is unstable and can range from several to hundreds of micro-sec.
In the case of ~10us, isr may not clear next sof interrupt status in
single handling, which prevents new interrupts from coming.

Fix this by handling all pending IRQs before exiting isr, so any abnormal
behavior results from very short interrupt status changes is protected.

Signed-off-by: Bingbu Cao 
Signed-off-by: Andy Yeh 
Signed-off-by: Yong Zhi 
---
Hi, Sakari,

Re-send with correct signed-off-by order.
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 7d768ec0f824..29027159eced 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -640,18 +640,10 @@ static const char *const cio2_port_errs[] = {
"PKT2LONG",
 };
 
-static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+static void cio2_irq_handle_once(struct cio2_device *cio2, u32 int_status)
 {
-   struct cio2_device *cio2 = cio2_ptr;
void __iomem *const base = cio2->base;
struct device *dev = >pci_dev->dev;
-   u32 int_status, int_clear;
-
-   int_status = readl(base + CIO2_REG_INT_STS);
-   int_clear = int_status;
-
-   if (!int_status)
-   return IRQ_NONE;
 
if (int_status & CIO2_INT_IOOE) {
/*
@@ -770,9 +762,29 @@ static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
int_status &= ~(CIO2_INT_IOIE | CIO2_INT_IOIRQ);
}
 
-   writel(int_clear, base + CIO2_REG_INT_STS);
if (int_status)
dev_warn(dev, "unknown interrupt 0x%x on INT\n", int_status);
+}
+
+static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+{
+   struct cio2_device *cio2 = cio2_ptr;
+   void __iomem *const base = cio2->base;
+   struct device *dev = >pci_dev->dev;
+   u32 int_status;
+
+   int_status = readl(base + CIO2_REG_INT_STS);
+   dev_dbg(dev, "isr enter - interrupt status 0x%x\n", int_status);
+   if (!int_status)
+   return IRQ_NONE;
+
+   do {
+   writel(int_status, base + CIO2_REG_INT_STS);
+   cio2_irq_handle_once(cio2, int_status);
+   int_status = readl(base + CIO2_REG_INT_STS);
+   if (int_status)
+   dev_dbg(dev, "pending status 0x%x\n", int_status);
+   } while (int_status);
 
return IRQ_HANDLED;
 }
-- 
2.7.4

Re: [PATCH v7 3/8] media: vsp1: Provide a body pool

[Kieran Bingham]

Hi Laurent,

On 06/04/18 23:33, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 8 March 2018 02:05:26 EEST Kieran Bingham wrote:
>> Each display list allocates a body to store register values in a dma
>> accessible buffer from a dma_alloc_wc() allocation. Each of these
>> results in an entry in the TLB, and a large number of display list
> 
> I'd write it as "IOMMU TLB" to make it clear we're not concerned about CPU 
> MMU 
> TLB pressure.

Yes, of course.

> 
>> allocations adds pressure to this resource.
>>
>> Reduce TLB pressure on the IPMMUs by allocating multiple display list
>> bodies in a single allocation, and providing these to the display list
>> through a 'body pool'. A pool can be allocated by the display list
>> manager or entities which require their own body allocations.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>> v4:
>>  - Provide comment explaining extra allocation on body pool
>>highlighting area for optimisation later.
>>
>> v3:
>>  - s/fragment/body/, s/fragments/bodies/
>>  - qty -> num_bodies
>>  - indentation fix
>>  - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/'
>>  - Add kerneldoc to non-static functions
>>
>> v2:
>>  - assign dlb->dma correctly
>>
>>  drivers/media/platform/vsp1/vsp1_dl.c | 163 +++-
>>  drivers/media/platform/vsp1/vsp1_dl.h |   8 +-
>>  2 files changed, 171 insertions(+)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index 67cc16c1b8e3..0208e72cb356
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -45,6 +45,8 @@ struct vsp1_dl_entry {
>>  /**
>>   * struct vsp1_dl_body - Display list body
>>   * @list: entry in the display list list of bodies
>> + * @free: entry in the pool free body list
> 
> Could we reuse @list for this purpose ? Unless I'm mistaken, when a body is 
> in 
> a pool it doesn't belong to any particular display list, and when it is in a 
> display list it isn't in the pool anymore.
I've adapted the @list doc-string to read:

 * @list: entry in the display list list of bodies, or body pool free list


Actually, I think I'm tempted to leave this distinct and separate.

If we use the single @list field, then we have calls to vsp1_dl_body_get() which
'might not' add that body to a display list.

Consider the lut_set_table call, if it was called twice without a dl_commit() in
between.

The first call would get a dlb, and populate it with the LUT, setting it as the
active LUT in the entitiy, but not adding it to any list.

The call to vsp1_dl_body_get() *has* to remove it from the free list at that
point. But now the list structure '@list' is poisoned.


Now consider the general case for 'putting' the object back to the pool free
list when it's reference count is zero:

vsp1_dl_body_put() must delete the structure from any existing list and add it
to the pool-free list.

We can use list_move_tail for this:

@@ -286,7 +284,7 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb)
dlb->num_entries = 0;

spin_lock_irqsave(>pool->lock, flags);
-   list_add_tail(>free, >pool->free);
+   list_move_tail(>list, >pool->free);
spin_unlock_irqrestore(>pool->lock, flags);


However - this fails in the instance above where the dlb did not make it onto a
list, because the call to __list_del_entry() will segfault on the poisoned 
values.

I don't believe we can expect the callers of vsp1_dl_body_put() to guarantee
that the entity is not on a list either, as that is intrinsic to the refcounting
associated with the object.


I think we have the same issue with the cached body in the vsp1_video object 
too.

So - leaving this as it is.


> 
>> + * @pool: pool to which this body belongs
>>   * @vsp1: the VSP1 device
>>   * @entries: array of entries
>>   * @dma: DMA address of the entries
>> @@ -54,6 +56,9 @@ struct vsp1_dl_entry {
>>   */
>>  struct vsp1_dl_body {
>>  struct list_head list;
>> +struct list_head free;
>> +
>> +struct vsp1_dl_body_pool *pool;
>>  struct vsp1_device *vsp1;
>>
>>  struct vsp1_dl_entry *entries;
>> @@ -65,6 +70,30 @@ struct vsp1_dl_body {
>>  };
>>
>>  /**
>> + * struct vsp1_dl_body_pool - display list body pool
>> + * @dma: DMA address of the entries
>> + * @size: size of the full DMA memory pool in bytes
>> + * @mem: CPU memory pointer for the pool
>> + * @bodies: Array of DLB structures for the pool
>> + * @free: List of free DLB entries
>> + * @lock: Protects the pool and free list
> 
> The pool and free list ? As far as I can tell the lock only protects the free 
> list.

I've removed the reference to the pool for the lock. I don't think much else
needs protecting in the current code, so it should be fine.

> 
>> + * @vsp1: the VSP1 device
>> + */
>> +struct vsp1_dl_body_pool {
>> +/* DMA allocation */
>> +dma_addr_t dma;
>> +size_t 

Re: atomisp: drop from staging ?

[Sakari Ailus]

Hi Alan,

On Sun, Apr 29, 2018 at 01:18:37AM +0100, Alan Cox wrote:
> 
> I think this is going to be the best option. When I started cleaning up
> the atomisp code I had time to work on it. Then spectre/meltdown
> happened (which btw is why the updating suddenly and mysteriously stopped
> last summer).
> 
> I no longer have time to work on it and it's becoming evident that the
> world of speculative side channel is going to be mean that I am
> not going to get time in the forseeable future despite me trying to find
> space to get back into atomisp cleaning up. It sucks because we made some
> good initial progress but shit happens.
> 
> There are at this point (unsurprisngly ;)) no other volunteers I can
> find crazy enough to take this on.

The driver has been in the staging tree for quite some time now and is a
regular target of cleanup patches but little has been done to address the
growing list of entries in the associated TODO file to get it out of
staging. Beyond this, I don't have the hardware but as far as I understand,
the driver is not functional in its current state.

I agree with removing the driver. It can always be brought back if someone
wishes to continue working it.

I can send patches to remove it.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v2 1/2] dt-bindings: media: renesas-ceu: Add R-Mobile R8A7740

[Simon Horman]

On Thu, Apr 26, 2018 at 08:24:42PM +0200, Jacopo Mondi wrote:
> Add R-Mobile A1 R8A7740 SoC to the list of compatible values for the CEU
> unit.
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Simon Horman 

> ---
>  Documentation/devicetree/bindings/media/renesas,ceu.txt | 7 ---
>  drivers/media/platform/renesas-ceu.c| 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
> b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> index 3fc66df..8a7a616 100644
> --- a/Documentation/devicetree/bindings/media/renesas,ceu.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> @@ -2,14 +2,15 @@ Renesas Capture Engine Unit (CEU)
>  --
>  
>  The Capture Engine Unit is the image capture interface found in the Renesas
> -SH Mobile and RZ SoCs.
> +SH Mobile, R-Mobile and RZ SoCs.
>  
>  The interface supports a single parallel input with data bus width of 8 or 16
>  bits.
>  
>  Required properties:
> -- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1H
> -  and RZ/A1M SoCs.
> +- compatible: Shall be one of the following values:
> + "renesas,r7s72100-ceu" for CEU units found in RZ/A1H and RZ/A1M SoCs
> + "renesas,r8a7740-ceu" for CEU units found in R-Mobile A1 R8A7740 SoCs

Nit: I think you can drop R8A7740 as I believe that by adding it to
R-Mobile A1 you have constructed a tautology (I mean "R-Mobile A1" =
"R8A7740" as far as I know).

>  - reg: Registers address base and size.
>  - interrupts: The interrupt specifier.
>  
> diff --git a/drivers/media/platform/renesas-ceu.c 
> b/drivers/media/platform/renesas-ceu.c
> index 6599dba..c964a56 100644
> --- a/drivers/media/platform/renesas-ceu.c
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -1545,6 +1545,7 @@ static const struct ceu_data ceu_data_sh4 = {
>  #if IS_ENABLED(CONFIG_OF)
>  static const struct of_device_id ceu_of_match[] = {
>   { .compatible = "renesas,r7s72100-ceu", .data = _data_rz },
> + { .compatible = "renesas,r8a7740-ceu", .data = _data_rz },
>   { }
>  };
>  MODULE_DEVICE_TABLE(of, ceu_of_match);
> -- 
> 2.7.4
> 

Re: [PATCH] rcar-vin: fix null pointer dereference in rvin_group_get()

[Simon Horman]

On Thu, Apr 26, 2018 at 05:20:05PM +0200, Niklas Söderlund wrote:
> Hi Simon,
> 
> Thanks for your feedback.
> 
> On 2018-04-25 09:18:51 +0200, Simon Horman wrote:
> > On Wed, Apr 25, 2018 at 01:45:06AM +0200, Niklas Söderlund wrote:
> > > Store the group pointer before disassociating the VIN from the group.
> > > 
> > > Fixes: 3bb4c3bc85bf77a7 ("media: rcar-vin: add group allocator functions")
> > > Reported-by: Colin Ian King 
> > > Signed-off-by: Niklas Söderlund 
> > > ---
> > >  drivers/media/platform/rcar-vin/rcar-core.c | 12 +++-
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index 7bc2774a11232362..d3072e166a1ca24f 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -338,19 +338,21 @@ static int rvin_group_get(struct rvin_dev *vin)
> > >  
> > >  static void rvin_group_put(struct rvin_dev *vin)
> > >  {
> > > - mutex_lock(>group->lock);
> > > + struct rvin_group *group = vin->group;
> > > +
> > > + mutex_lock(>lock);
> > 
> > Hi Niklas, its not clear to me why moving the lock is safe.
> > Could you explain the locking scheme a little?
> 
> The lock here protects the members of the group struct and not any of 
> the members of the vin struct. The intent of the rvin_group_put() 
> function is:
> 
> 1. Disassociate the vin struct from the group struct. This is done by 
>removing the pointer to the vin from the group->vin array and 
>removing the pointer from vin->group to the group struct. Here the 
>lock is needed to protect access to the group->vin array.
> 
> 2. Decrease the refcount of the struct group and if we are the last one 
>out release the group.
> 
> The problem with the original code is that I first disassociate group 
> from the vin 'vin->group = NULL' but still use the pointer stored in the 
> vin struct when I try to disassociate the vin from the group 
> 'vin->group->vin[vin->id]'.
> 
> AFIK can tell the locking here is fine, the problem was that I pulled 
> the rug from under my own feet in how I access the lock in order to not 
> having to declare a variable to store the pointer in ;-)
> 
> Do this explanation help put you at ease?

Thanks, I am completely relaxed now :)

Reviewed-by: Simon Horman 

> > >   vin->group = NULL;
> > >   vin->v4l2_dev.mdev = NULL;
> > >  
> > > - if (WARN_ON(vin->group->vin[vin->id] != vin))
> > > + if (WARN_ON(group->vin[vin->id] != vin))
> > >   goto out;
> > >  
> > > - vin->group->vin[vin->id] = NULL;
> > > + group->vin[vin->id] = NULL;
> > >  out:
> > > - mutex_unlock(>group->lock);
> > > + mutex_unlock(>lock);
> > >  
> > > - kref_put(>group->refcount, rvin_group_release);
> > > + kref_put(>refcount, rvin_group_release);
> > >  }
> > >  
> > >  /* 
> > > -
> > > -- 
> > > 2.17.0
> > > 
> 
> -- 
> Regards,
> Niklas Söderlund
>