Re: [PATCH 01/12] [media] vb2: add explicit fence user API
On Tue, Jul 4, 2017 at 2:57 PM, Tomasz Figa wrote: > Hi Gustavo, > > On Tue, Jun 27, 2017 at 12:39 AM, Gustavo Padovan wrote: >> 2017-06-18 kbuild test robot : >> >>> Hi Gustavo, >>> >>> [auto build test ERROR on linuxtv-media/master] >>> [also build test ERROR on v4.12-rc5 next-20170616] >>> [if your patch is applied to the wrong git tree, please drop us a note to >>> help improve the system] >>> >>> url: >>> https://github.com/0day-ci/linux/commits/Gustavo-Padovan/vb2-add-explicit-fence-user-API/20170618-210740 >>> base: git://linuxtv.org/media_tree.git master >>> config: x86_64-allmodconfig (attached as .config) >>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 >>> reproduce: >>> # save the attached .config to linux build tree >>> make ARCH=x86_64 >>> >>> All error/warnings (new ones prefixed by >>): >>> >>>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function >>> 'atomisp_qbuf': >>> >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1297:10: >>> >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you >>> >> mean 'reserved'? >>> (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { >>> ^~ >>>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1299:50: >>> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >>> 'reserved'? >>> pipe->frame_request_config_id[buf->index] = buf->reserved2 & >>> ^~ >>>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function >>> 'atomisp_dqbuf': >>>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1483:5: >>> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >>> 'reserved'? >>> buf->reserved2 = pipe->frame_config_id[buf->index]; >>> ^~ >>>In file included from include/linux/printk.h:329:0, >>> from include/linux/kernel.h:13, >>> from include/linux/delay.h:21, >>> from >>> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:24: >>>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1488:6: >>> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >>> 'reserved'? >>> buf->reserved2); >>> ^ >> >> Ouch! Seems the reserved2 was burned down by 2 drivers accessing it >> without any care for the uAPI. I'll change my patches to use the >> 'reserved' field instead. > > Given that a reserved field has a clear meaning of being reserved and > the driver in question is in staging. I'd rather vote for fixing the > driver. Same here. It seems like this use of reserved2 should not have been merged in the first place, thankfully it's only in staging.
Re: [PATCH 01/12] [media] vb2: add explicit fence user API
Hi Gustavo, On Tue, Jun 27, 2017 at 12:39 AM, Gustavo Padovan wrote: > 2017-06-18 kbuild test robot : > >> Hi Gustavo, >> >> [auto build test ERROR on linuxtv-media/master] >> [also build test ERROR on v4.12-rc5 next-20170616] >> [if your patch is applied to the wrong git tree, please drop us a note to >> help improve the system] >> >> url: >> https://github.com/0day-ci/linux/commits/Gustavo-Padovan/vb2-add-explicit-fence-user-API/20170618-210740 >> base: git://linuxtv.org/media_tree.git master >> config: x86_64-allmodconfig (attached as .config) >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 >> reproduce: >> # save the attached .config to linux build tree >> make ARCH=x86_64 >> >> All error/warnings (new ones prefixed by >>): >> >>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function >> 'atomisp_qbuf': >> >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1297:10: >> >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >> >> 'reserved'? >> (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { >> ^~ >>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1299:50: >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >> 'reserved'? >> pipe->frame_request_config_id[buf->index] = buf->reserved2 & >> ^~ >>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function >> 'atomisp_dqbuf': >>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1483:5: >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >> 'reserved'? >> buf->reserved2 = pipe->frame_config_id[buf->index]; >> ^~ >>In file included from include/linux/printk.h:329:0, >> from include/linux/kernel.h:13, >> from include/linux/delay.h:21, >> from >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:24: >>drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1488:6: >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >> 'reserved'? >> buf->reserved2); >> ^ > > Ouch! Seems the reserved2 was burned down by 2 drivers accessing it > without any care for the uAPI. I'll change my patches to use the > 'reserved' field instead. Given that a reserved field has a clear meaning of being reserved and the driver in question is in staging. I'd rather vote for fixing the driver. Best regards, Tomasz
Re: [PATCH 01/12] [media] vb2: add explicit fence user API
Em Fri, 16 Jun 2017 16:39:04 +0900 Gustavo Padovan escreveu: > From: Gustavo Padovan > > Turn the reserved2 field into fence_fd that we will use to send > an in-fence to the kernel and return an out-fence from the kernel to > userspace. > > Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used > when sending a fence to the kernel to be waited on, and > V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence. > > Signed-off-by: Gustavo Padovan > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++-- > drivers/media/v4l2-core/videobuf2-v4l2.c | 2 +- > include/uapi/linux/videodev2.h| 4 +++- > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index 6f52970..8f6ab85 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -367,7 +367,7 @@ struct v4l2_buffer32 { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + __s32 fence_fd; > __u32 reserved; > }; > > @@ -530,7 +530,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, > struct v4l2_buffer32 __user > put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) || > copy_to_user(&up->timecode, &kp->timecode, sizeof(struct > v4l2_timecode)) || > put_user(kp->sequence, &up->sequence) || > - put_user(kp->reserved2, &up->reserved2) || > + put_user(kp->fence_fd, &up->fence_fd) || > put_user(kp->reserved, &up->reserved) || > put_user(kp->length, &up->length)) > return -EFAULT; > diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c > b/drivers/media/v4l2-core/videobuf2-v4l2.c > index 0c06699..110fb45 100644 > --- a/drivers/media/v4l2-core/videobuf2-v4l2.c > +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c > @@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, > void *pb) > b->timestamp = ns_to_timeval(vb->timestamp); > b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > - b->reserved2 = 0; > + b->fence_fd = -1; > b->reserved = 0; > > if (q->is_multiplanar) { > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 2b8feb8..750d511 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -916,7 +916,7 @@ struct v4l2_buffer { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + __s32 fence_fd; > __u32 reserved; > }; > > @@ -953,6 +953,8 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE 0x0001 > /* mem2mem encoder/decoder */ > #define V4L2_BUF_FLAG_LAST 0x0010 > +#define V4L2_BUF_FLAG_IN_FENCE 0x0020 > +#define V4L2_BUF_FLAG_OUT_FENCE 0x0040 Please document them at Documentation/media/uapi/v4l/buffer.rst. We'll also need a new chapter at the documentation, describing the explicit fences mechanism. > > /** > * struct v4l2_exportbuffer - export of video buffer as DMABUF file > descriptor -- Thanks, Mauro
Re: [PATCH 01/12] [media] vb2: add explicit fence user API
2017-06-18 kbuild test robot : > Hi Gustavo, > > [auto build test ERROR on linuxtv-media/master] > [also build test ERROR on v4.12-rc5 next-20170616] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Gustavo-Padovan/vb2-add-explicit-fence-user-API/20170618-210740 > base: git://linuxtv.org/media_tree.git master > config: x86_64-allmodconfig (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All error/warnings (new ones prefixed by >>): > >drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function > 'atomisp_qbuf': > >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1297:10: > >> error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean > >> 'reserved'? > (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { > ^~ >drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1299:50: > error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean > 'reserved'? > pipe->frame_request_config_id[buf->index] = buf->reserved2 & > ^~ >drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function > 'atomisp_dqbuf': >drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1483:5: error: > 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? > buf->reserved2 = pipe->frame_config_id[buf->index]; > ^~ >In file included from include/linux/printk.h:329:0, > from include/linux/kernel.h:13, > from include/linux/delay.h:21, > from > drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:24: >drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1488:6: error: > 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? > buf->reserved2); > ^ Ouch! Seems the reserved2 was burned down by 2 drivers accessing it without any care for the uAPI. I'll change my patches to use the 'reserved' field instead. Gustavo
Re: [PATCH 01/12] [media] vb2: add explicit fence user API
Hi Gustavo, [auto build test ERROR on linuxtv-media/master] [also build test ERROR on v4.12-rc5 next-20170616] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gustavo-Padovan/vb2-add-explicit-fence-user-API/20170618-210740 base: git://linuxtv.org/media_tree.git master config: x86_64-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function 'atomisp_qbuf': >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1297:10: error: >> 'struct v4l2_buffer' has no member named 'reserved2'; did you mean >> 'reserved'? (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { ^~ drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1299:50: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? pipe->frame_request_config_id[buf->index] = buf->reserved2 & ^~ drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: In function 'atomisp_dqbuf': drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1483:5: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? buf->reserved2 = pipe->frame_config_id[buf->index]; ^~ In file included from include/linux/printk.h:329:0, from include/linux/kernel.h:13, from include/linux/delay.h:21, from drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:24: drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1488:6: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? buf->reserved2); ^ include/linux/dynamic_debug.h:135:9: note: in definition of macro 'dynamic_dev_dbg' ##__VA_ARGS__); \ ^~~ >> drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c:1486:2: note: in >> expansion of macro 'dev_dbg' dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d, isp_config_id %d\n", ^~~ drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c: At top level: >> cc1: warning: unrecognized command line option '-Wno-implicit-fallthrough' -- drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c: In function 'atomisp_qbuf': drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1297:10: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { ^~ drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1299:50: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? pipe->frame_request_config_id[buf->index] = buf->reserved2 & ^~ drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c: In function 'atomisp_dqbuf': drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1483:5: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? buf->reserved2 = pipe->frame_config_id[buf->index]; ^~ In file included from include/linux/printk.h:329:0, from include/linux/kernel.h:13, from include/linux/delay.h:21, from drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:24: drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1488:6: error: 'struct v4l2_buffer' has no member named 'reserved2'; did you mean 'reserved'? buf->reserved2); ^ include/linux/dynamic_debug.h:135:9: note: in definition of macro 'dynamic_dev_dbg' ##__VA_ARGS__); \ ^~~ drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1486:2: note: in expansion of macro 'dev_dbg' dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d, isp_config_id %d\n", ^~~ drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c: At top level: >> cc1: warning: unrecognized command line option '-Wno-implicit-fallthrough' vim +1297 drivers/staging/media//atomisp/pci/atomisp2/atomisp_ioctl.c a49d2536 Alan Cox 2017-02-17 1291 a49d2536 Alan Cox 2017-02-17 1292 done: a49d2536 Alan Cox 2017-02-17 1293 if (!((buf->flags & NOFLUSH_FLAGS) == NOFLUSH_FLAGS)) a49d2536 Alan Cox 2017-02-17 1294 wbinvd(); a49d2536 Alan Cox 2017-02-17 1295 a49d2536 Alan Cox 2017-02-17 1296 if (!atomisp_is_vf_pipe(pipe) && a49d2536 Alan Cox 2017-02-17 @1297 (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { a49d2536 Alan Cox 2017-02-17 1298 /* this buffer will have a per-f
Re: [PATCH 01/12] [media] vb2: add explicit fence user API
Hi Gustavo, [auto build test ERROR on linuxtv-media/master] [also build test ERROR on v4.12-rc5 next-20170616] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gustavo-Padovan/vb2-add-explicit-fence-user-API/20170618-210740 base: git://linuxtv.org/media_tree.git master config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sh All errors (new ones prefixed by >>): drivers/media//usb/cpia2/cpia2_v4l.c: In function 'cpia2_dqbuf': >> drivers/media//usb/cpia2/cpia2_v4l.c:951:5: error: 'struct v4l2_buffer' has >> no member named 'reserved2'; did you mean 'reserved'? buf->reserved2 = 0; ^~ vim +951 drivers/media//usb/cpia2/cpia2_v4l.c 1b18e7a0 drivers/media/usb/cpia2/cpia2_v4l.c Sakari Ailus 2012-10-22 945 | V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 946 buf->field = V4L2_FIELD_NONE; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 947 buf->timestamp = cam->buffers[buf->index].timestamp; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 948 buf->sequence = cam->buffers[buf->index].seq; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 949 buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 950 buf->length = cam->frame_size; 2b719d7b drivers/media/video/cpia2/cpia2_v4l.c Sakari Ailus 2012-05-02 @951 buf->reserved2 = 0; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 952 buf->reserved = 0; ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 953 memset(&buf->timecode, 0, sizeof(buf->timecode)); ab33d507 drivers/media/video/cpia2/cpia2_v4l.c Alan Cox 2006-02-27 954 :: The code at line 951 was first introduced by commit :: 2b719d7baf490e24ce7d817c6337b7c87fda84c1 [media] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT :: TO: Sakari Ailus :: CC: Mauro Carvalho Chehab --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip