cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Aug 16 05:00:24 CEST 2018 media-tree git hash:da2048b7348a0be92f706ac019e022139e29495e media_build git hash: baf45935ffad914f33faf751ad9f4d0dd276c021 v4l-utils git hash: 5ca5d0e3d0b920e5115eff51972ff77463591aeb edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-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.102-i686: OK linux-3.2.102-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.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.115-i686: OK linux-3.18.115-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.18-i686: OK linux-4.18-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCHv2 5/5] adv7842: enable reduced fps detection
Hi Hans,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18 next-20180815]
[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/Hans-Verkuil/Handling-of-reduced-FPS-in-V4L2/20180816-051924
base: git://linuxtv.org/media_tree.git master
config: i386-randconfig-a0-201832 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/media/i2c/adv7842.o: In function `adv7842_query_dv_timings':
>> drivers/media/i2c/adv7842.c:1602: undefined reference to `__udivdi3'
vim +1602 drivers/media/i2c/adv7842.c
1530
1531 static int adv7842_query_dv_timings(struct v4l2_subdev *sd,
1532 struct v4l2_dv_timings *timings)
1533 {
1534 struct adv7842_state *state = to_state(sd);
1535 struct v4l2_bt_timings *bt = >bt;
1536 struct stdi_readback stdi = { 0 };
1537
1538 v4l2_dbg(1, debug, sd, "%s:\n", __func__);
1539
1540 memset(timings, 0, sizeof(struct v4l2_dv_timings));
1541
1542 /* SDP block */
1543 if (state->mode == ADV7842_MODE_SDP)
1544 return -ENODATA;
1545
1546 /* read STDI */
1547 if (read_stdi(sd, )) {
1548 state->restart_stdi_once = true;
1549 v4l2_dbg(1, debug, sd, "%s: no valid signal\n",
__func__);
1550 return -ENOLINK;
1551 }
1552 bt->interlaced = stdi.interlaced ?
1553 V4L2_DV_INTERLACED : V4L2_DV_PROGRESSIVE;
1554 bt->standards = V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
1555 V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT;
1556
1557 if (is_digital_input(sd)) {
1558 u32 freq;
1559
1560 timings->type = V4L2_DV_BT_656_1120;
1561
1562 bt->width = (hdmi_read(sd, 0x07) & 0x0f) * 256 +
hdmi_read(sd, 0x08);
1563 bt->height = (hdmi_read(sd, 0x09) & 0x0f) * 256 +
hdmi_read(sd, 0x0a);
1564 freq = ((hdmi_read(sd, 0x51) << 1) + (hdmi_read(sd,
0x52) >> 7)) * 100;
1565 freq += ((hdmi_read(sd, 0x52) & 0x7f) * 7813);
1566 if (is_hdmi(sd)) {
1567 /* adjust for deep color mode */
1568 freq = freq * 8 / (((hdmi_read(sd, 0x0b) &
0xc0) >> 6) * 2 + 8);
1569 }
1570 bt->pixelclock = freq;
1571 bt->hfrontporch = (hdmi_read(sd, 0x20) & 0x03) * 256 +
1572 hdmi_read(sd, 0x21);
1573 bt->hsync = (hdmi_read(sd, 0x22) & 0x03) * 256 +
1574 hdmi_read(sd, 0x23);
1575 bt->hbackporch = (hdmi_read(sd, 0x24) & 0x03) * 256 +
1576 hdmi_read(sd, 0x25);
1577 bt->vfrontporch = ((hdmi_read(sd, 0x2a) & 0x1f) * 256 +
1578 hdmi_read(sd, 0x2b)) / 2;
1579 bt->vsync = ((hdmi_read(sd, 0x2e) & 0x1f) * 256 +
1580 hdmi_read(sd, 0x2f)) / 2;
1581 bt->vbackporch = ((hdmi_read(sd, 0x32) & 0x1f) * 256 +
1582 hdmi_read(sd, 0x33)) / 2;
1583 bt->polarities = ((hdmi_read(sd, 0x05) & 0x10) ?
V4L2_DV_VSYNC_POS_POL : 0) |
1584 ((hdmi_read(sd, 0x05) & 0x20) ?
V4L2_DV_HSYNC_POS_POL : 0);
1585 if (bt->interlaced == V4L2_DV_INTERLACED) {
1586 bt->height += (hdmi_read(sd, 0x0b) & 0x0f) *
256 +
1587 hdmi_read(sd, 0x0c);
1588 bt->il_vfrontporch = ((hdmi_read(sd, 0x2c) &
0x1f) * 256 +
1589 hdmi_read(sd, 0x2d)) / 2;
1590 bt->il_vsync = ((hdmi_read(sd, 0x30) & 0x1f) *
256 +
1591 hdmi_read(sd, 0x31)) / 2;
1592 bt->il_vbackporch = ((hdmi_read(sd, 0x34) &
0x1f) * 256 +
1593 hdmi_read(sd, 0x35)) / 2;
1594 } else {
1595 bt->il_vfrontporch = 0;
1596 bt->il_vsync = 0;
1597 bt->il_vbackporch = 0;
1598 }
1599 adv7842_fill_optional_dv_timings_fields(sd, timings);
1600
Re: [PATCHv18 01/35] Documentation: v4l: document request API
Hi Hans, Thank you for the patch. On Tuesday, 14 August 2018 17:20:13 EEST Hans Verkuil wrote: > From: Alexandre Courbot > > Document the request API for V4L2 devices, and amend the documentation > of system calls influenced by it. > > Signed-off-by: Alexandre Courbot > Signed-off-by: Hans Verkuil > --- > .../media/uapi/mediactl/media-controller.rst | 1 + > .../media/uapi/mediactl/media-funcs.rst | 6 + > .../uapi/mediactl/media-ioc-request-alloc.rst | 65 + > .../uapi/mediactl/media-request-ioc-queue.rst | 82 ++ > .../mediactl/media-request-ioc-reinit.rst | 51 > .../media/uapi/mediactl/request-api.rst | 245 ++ > .../uapi/mediactl/request-func-close.rst | 48 > .../uapi/mediactl/request-func-ioctl.rst | 67 + > .../media/uapi/mediactl/request-func-poll.rst | 77 ++ > Documentation/media/uapi/v4l/buffer.rst | 21 +- > .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 53 +++- > Documentation/media/uapi/v4l/vidioc-qbuf.rst | 32 ++- > .../media/videodev2.h.rst.exceptions | 1 + > 13 files changed, 739 insertions(+), 10 deletions(-) > create mode 100644 > Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst create mode > 100644 Documentation/media/uapi/mediactl/media-request-ioc-queue.rst create > mode 100644 Documentation/media/uapi/mediactl/media-request-ioc-reinit.rst > create mode 100644 Documentation/media/uapi/mediactl/request-api.rst create > mode 100644 Documentation/media/uapi/mediactl/request-func-close.rst create > mode 100644 Documentation/media/uapi/mediactl/request-func-ioctl.rst create > mode 100644 Documentation/media/uapi/mediactl/request-func-poll.rst > > diff --git a/Documentation/media/uapi/mediactl/media-controller.rst > b/Documentation/media/uapi/mediactl/media-controller.rst index > 0eea4f9a07d5..66aff38cd499 100644 > --- a/Documentation/media/uapi/mediactl/media-controller.rst > +++ b/Documentation/media/uapi/mediactl/media-controller.rst > @@ -21,6 +21,7 @@ Part IV - Media Controller API > media-controller-intro > media-controller-model > media-types > +request-api > media-funcs > media-header > > diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst > b/Documentation/media/uapi/mediactl/media-funcs.rst index > 076856501cdb..260f9dcadcde 100644 > --- a/Documentation/media/uapi/mediactl/media-funcs.rst > +++ b/Documentation/media/uapi/mediactl/media-funcs.rst > @@ -16,3 +16,9 @@ Function Reference > media-ioc-enum-entities > media-ioc-enum-links > media-ioc-setup-link > +media-ioc-request-alloc > +request-func-close > +request-func-ioctl > +request-func-poll > +media-request-ioc-queue > +media-request-ioc-reinit > diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst new file > mode 100644 > index ..34434e2b3918 > --- /dev/null > +++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > @@ -0,0 +1,65 @@ > +.. SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-or-later WITH > no-invariant-sections + > +.. _media_ioc_request_alloc: > + > +* > +ioctl MEDIA_IOC_REQUEST_ALLOC > +* > + > +Name > + > + > +MEDIA_IOC_REQUEST_ALLOC - Allocate a request > + > + > +Synopsis > + > + > +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_ALLOC, int *argp ) > +:name: MEDIA_IOC_REQUEST_ALLOC > + > + > +Arguments > += > + > +``fd`` > +File descriptor returned by :ref:`open() `. > + > +``argp`` > +Pointer to an integer. > + > + > +Description > +=== > + > +If the media device supports :ref:`requests `, then > +this ioctl can be used to allocate a request. If it is not supported, then > +``errno`` is set to ``ENOTTY``. A request is accessed through a file > descriptor +that is returned in ``*argp``. > + > +If the request was successfully allocated, then the request file descriptor > +can be passed to the :ref:`VIDIOC_QBUF `, > +:ref:`VIDIOC_G_EXT_CTRLS `, > +:ref:`VIDIOC_S_EXT_CTRLS ` and > +:ref:`VIDIOC_TRY_EXT_CTRLS ` ioctls. As with the media controller API, the request API isn't V4L2-specific, but can be used by subsystems that use the media controller API, such as V4L2. The above paragraph is correct, but I wouldn't list V4L2-specific ioctls in the MC API documentation. Instead, you could state that the request file descriptor can be passed to subsystem APIs that are request-aware, without detailing which ones. The documentation of the above V4L2 ioctls will contain detailed information. > +In addition, the request can be queued by calling > +:ref:`MEDIA_REQUEST_IOC_QUEUE` and re-initialized by calling > +:ref:`MEDIA_REQUEST_IOC_REINIT`. > + > +Finally, the file descriptor can be :ref:`polled ` to > wait > +for the request to complete. > + > +The request will remain allocated until all the file
Re: [RFC] Request API and V4L2 capabilities
Le mercredi 15 août 2018 à 09:11 -0300, Mauro Carvalho Chehab a écrit : > Em Sat, 4 Aug 2018 15:50:04 +0200 > Hans Verkuil escreveu: > > > Hi all, > > > > While the Request API patch series addresses all the core API > > issues, there > > are some high-level considerations as well: > > > > 1) How can the application tell that the Request API is supported > > and for > >which buffer types (capture/output) and pixel formats? > > > > 2) How can the application tell if the Request API is required as > > opposed to being > >optional? > > Huh? Why would it be mandatory? > > > > > 3) Some controls may be required in each request, how to let > > userspace know this? > >Is it even necessary to inform userspace? > > Again, why would it need to have a set of mandatory controls for > requests > to work? If this is really required, it should have a way to send > such > list to userspace. > > > > > 4) (For bonus points): How to let the application know which > > streaming I/O modes > >are available? That's never been possible before, but it would > > be very nice > >indeed if that's made explicit. > > > > Since the Request API associates data with frame buffers it makes > > sense to expose > > this as a new capability field in struct v4l2_requestbuffers and > > struct v4l2_create_buffers. > > > > The first struct has 2 reserved fields, the second has 8, so it's > > not a problem to > > take one for a capability field. Both structs also have a buffer > > type, so we know > > if this is requested for a capture or output buffer type. The pixel > > format is known > > in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I > > doubt we'll have > > drivers where the request caps would actually depend on the pixel > > format, but it > > theoretically possible. For both ioctls you can call them with > > count=0 at the start > > of the application. REQBUFS has of course the side-effect of > > deleting all buffers, > > but at the start of your application you don't have any yet. > > CREATE_BUFS has no > > side-effects. > > > > I propose adding these capabilities: > > > > #define V4L2_BUF_CAP_HAS_REQUESTS 0x0001 > > I'm OK with that. > > > #define V4L2_BUF_CAP_REQUIRES_REQUESTS 0x0002 > > But I'm not ok with breaking even more userspace support by forcing > requests. This is not breaking userspace, not in regard to state-less CODEC. Stateless CODECs uses a set of new pixel formats specifically designed for driving an accelerator rather then a full CODEC. The controls are needed to provide a state to the accelerator, so the accelerator knows what to do. Though, because of the nature of CODECs, queuing multiple buffers is strictly needed. Without the request, there is no way to figure-out which CID changes goes with which picture. There is no way an existing userspace will break as there is no way it can support these drivers as a) the formats aren't defined yet b) the CIDs didn't exist. > > > #define V4L2_BUF_CAP_HAS_MMAP 0x0100 > > #define V4L2_BUF_CAP_HAS_USERPTR0x0200 > > #define V4L2_BUF_CAP_HAS_DMABUF 0x0400 > > Those sounds ok to me too. > > > > > If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. > > > > At this time I think that REQUIRES_REQUESTS would only need to be > > set for the > > output queue of stateless codecs. > > Same as before: I don't see the need of support a request-only > driver. > > > > > If capabilities is 0, then it's from an old kernel and all you know > > is that > > requests are certainly not supported, and that MMAP is supported. > > Whether USERPTR > > or DMABUF are supported isn't known in that case (just try it :-) > > ). > > > > Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF > > caps, but it is very > > easy to add if we create a new capability field anyway, and it has > > always annoyed > > the hell out of me that we didn't have a good way to let userspace > > know what > > streaming I/O modes we support. And with vb2 it's easy to > > implement. > > Yeah, that sounds a bonus to me too. > > > Regarding point 3: I think this should be documented next to the > > pixel format. I.e. > > the MPEG-2 Slice format used by the stateless cedrus codec requires > > the request API > > and that two MPEG-2 controls (slice params and quantization > > matrices) must be present > > in each request. > > Makes sense to document with the pixel format... > > > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) > > is needed here. > > but it sounds worth to also have a flag. > > > It's really implied by the fact that you use a stateless codec. It > > doesn't help > > generic applications like v4l2-ctl or qv4l2 either since in order > > to support > > stateless codecs they will have to know about the details of these > > controls anyway. > > Yeah, but they could skip enum those ioctls if they see one marked > with > V4L2_CTRL_FLAG_REQUIRED_IN_REQ
Re: [RFC] Request API and V4L2 capabilities
Le samedi 04 août 2018 à 15:50 +0200, Hans Verkuil a écrit : > Hi all, > > While the Request API patch series addresses all the core API issues, there > are some high-level considerations as well: > > 1) How can the application tell that the Request API is supported and for >which buffer types (capture/output) and pixel formats? > > 2) How can the application tell if the Request API is required as opposed to > being >optional? > > 3) Some controls may be required in each request, how to let userspace know > this? >Is it even necessary to inform userspace? For state-less codec, there is a very strict set of controls that must be supported / filled. The data format pretty much dictate this. For complex camera's and video transformation m2m devices, there is a gap indeed. Duplicating the formats for this case does not seem like the right approach. > > 4) (For bonus points): How to let the application know which streaming I/O > modes >are available? That's never been possible before, but it would be very nice >indeed if that's made explicit. In GStreamer, we call REQBUFS(type, count=0) for each types we support. This call should never fail, unless the type is not supported. We build a list of supported I/O mode this way. It's also a no-op, because we didn't allocate any buffers yet. > > Since the Request API associates data with frame buffers it makes sense to > expose > this as a new capability field in struct v4l2_requestbuffers and struct > v4l2_create_buffers. > > The first struct has 2 reserved fields, the second has 8, so it's not a > problem to > take one for a capability field. Both structs also have a buffer type, so we > know > if this is requested for a capture or output buffer type. The pixel format is > known > in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I doubt > we'll have > drivers where the request caps would actually depend on the pixel format, but > it > theoretically possible. For both ioctls you can call them with count=0 at the > start > of the application. REQBUFS has of course the side-effect of deleting all > buffers, > but at the start of your application you don't have any yet. CREATE_BUFS has > no > side-effects. > > I propose adding these capabilities: > > #define V4L2_BUF_CAP_HAS_REQUESTS 0x0001 > #define V4L2_BUF_CAP_REQUIRES_REQUESTS0x0002 > #define V4L2_BUF_CAP_HAS_MMAP 0x0100 > #define V4L2_BUF_CAP_HAS_USERPTR 0x0200 > #define V4L2_BUF_CAP_HAS_DMABUF 0x0400 Looks similar to the bit map we create inside GStreamer using the described technique. Though we also add HAS_CREATE_BUFS to the lot. My main concern is in userspace like GStreamer, the difficulty is to sort drivers that we support, from the ones that we don't. So if we don't support requests yet, we would like to detect this early. As CODEC don't really have an initial format, I believe that before S_FMT, any kind of call to REQBUFS might fail at the moment. So detection would be very late. Thoughm be aware this is a totally artificial issue in the short term since state-less CODEC uses dedicated formats. > > If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. > > At this time I think that REQUIRES_REQUESTS would only need to be set for the > output queue of stateless codecs. > > If capabilities is 0, then it's from an old kernel and all you know is that > requests are certainly not supported, and that MMAP is supported. Whether > USERPTR > or DMABUF are supported isn't known in that case (just try it :-) ). Just a clarification, the doc is pretty clear the MMAP is supported if the device capability have STREAMING in it. > > Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF caps, but it > is very > easy to add if we create a new capability field anyway, and it has always > annoyed > the hell out of me that we didn't have a good way to let userspace know what > streaming I/O modes we support. And with vb2 it's easy to implement. > > Regarding point 3: I think this should be documented next to the pixel > format. I.e. > the MPEG-2 Slice format used by the stateless cedrus codec requires the > request API > and that two MPEG-2 controls (slice params and quantization matrices) must be > present > in each request. > > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed > here. > It's really implied by the fact that you use a stateless codec. It doesn't > help > generic applications like v4l2-ctl or qv4l2 either since in order to support > stateless codecs they will have to know about the details of these controls > anyway. Right, I don't think this is needed in the short term, as we target only stateless CODEC. But this is important use case for let's say request to cameras. When we get there, we will need a mechnism to list all the controls that can be included in a request, and also all the only the must be present (if any). > >
Re: [RFC] Request API and V4L2 capabilities
On 15/08/18 14:11, Mauro Carvalho Chehab wrote: > Em Sat, 4 Aug 2018 15:50:04 +0200 > Hans Verkuil escreveu: > >> Hi all, >> >> While the Request API patch series addresses all the core API issues, there >> are some high-level considerations as well: >> >> 1) How can the application tell that the Request API is supported and for >>which buffer types (capture/output) and pixel formats? >> >> 2) How can the application tell if the Request API is required as opposed to >> being >>optional? > > Huh? Why would it be mandatory? It is mandatory for stateless codecs: you can't use them without the Request API since each frame needs the state as well. If you could make a driver for a stateless codec without the Request API we wouldn't have had to spend ages on developing it in the first place, would we? :-) > >> >> 3) Some controls may be required in each request, how to let userspace know >> this? >>Is it even necessary to inform userspace? > > Again, why would it need to have a set of mandatory controls for requests > to work? If this is really required, it should have a way to send such > list to userspace. Also for stateless codecs: each frame needs the state information. This is done through one or more controls (see the cedrus driver implementation). Which controls those are is standardized, but it will differ depending on the HW codec (MPEG, H264, HEVC will almost certainly each require different state information). Obviously this will have to be documented as part of the pixel format for each HW codec. > >> >> 4) (For bonus points): How to let the application know which streaming I/O >> modes >>are available? That's never been possible before, but it would be very >> nice >>indeed if that's made explicit. >> >> Since the Request API associates data with frame buffers it makes sense to >> expose >> this as a new capability field in struct v4l2_requestbuffers and struct >> v4l2_create_buffers. >> >> The first struct has 2 reserved fields, the second has 8, so it's not a >> problem to >> take one for a capability field. Both structs also have a buffer type, so we >> know >> if this is requested for a capture or output buffer type. The pixel format >> is known >> in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I doubt >> we'll have >> drivers where the request caps would actually depend on the pixel format, >> but it >> theoretically possible. For both ioctls you can call them with count=0 at >> the start >> of the application. REQBUFS has of course the side-effect of deleting all >> buffers, >> but at the start of your application you don't have any yet. CREATE_BUFS has >> no >> side-effects. >> >> I propose adding these capabilities: >> >> #define V4L2_BUF_CAP_HAS_REQUESTS0x0001 > > I'm OK with that. > >> #define V4L2_BUF_CAP_REQUIRES_REQUESTS 0x0002 > > But I'm not ok with breaking even more userspace support by forcing > requests. You don't break userspace. This is for stateless codecs, which didn't exist before precisely because they require the Request API. > >> #define V4L2_BUF_CAP_HAS_MMAP0x0100 >> #define V4L2_BUF_CAP_HAS_USERPTR 0x0200 >> #define V4L2_BUF_CAP_HAS_DMABUF 0x0400 > > Those sounds ok to me too. > >> >> If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. >> >> At this time I think that REQUIRES_REQUESTS would only need to be set for the >> output queue of stateless codecs. > > Same as before: I don't see the need of support a request-only driver. > >> >> If capabilities is 0, then it's from an old kernel and all you know is that >> requests are certainly not supported, and that MMAP is supported. Whether >> USERPTR >> or DMABUF are supported isn't known in that case (just try it :-) ). >> >> Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF caps, but it >> is very >> easy to add if we create a new capability field anyway, and it has always >> annoyed >> the hell out of me that we didn't have a good way to let userspace know what >> streaming I/O modes we support. And with vb2 it's easy to implement. > > Yeah, that sounds a bonus to me too. > >> Regarding point 3: I think this should be documented next to the pixel >> format. I.e. >> the MPEG-2 Slice format used by the stateless cedrus codec requires the >> request API >> and that two MPEG-2 controls (slice params and quantization matrices) must >> be present >> in each request. > > Makes sense to document with the pixel format... > >> I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed >> here. > > but it sounds worth to also have a flag. I'll wait to get some more feedback. I don't have a very strong opinion on this. > >> It's really implied by the fact that you use a stateless codec. It doesn't >> help >> generic applications like v4l2-ctl or qv4l2 either since in order to support >> stateless codecs they will have to know about the
Re: [RFC] Request API and V4L2 capabilities
On 06/08/18 10:16, Paul Kocialkowski wrote: > Hi Hans and all, > > On Sat, 2018-08-04 at 15:50 +0200, Hans Verkuil wrote: >> Hi all, >> >> While the Request API patch series addresses all the core API issues, there >> are some high-level considerations as well: >> >> 1) How can the application tell that the Request API is supported and for >>which buffer types (capture/output) and pixel formats? >> >> 2) How can the application tell if the Request API is required as opposed to >> being >>optional? >> >> 3) Some controls may be required in each request, how to let userspace know >> this? >>Is it even necessary to inform userspace? >> >> 4) (For bonus points): How to let the application know which streaming I/O >> modes >>are available? That's never been possible before, but it would be very >> nice >>indeed if that's made explicit. > > Thanks for bringing up these considerations and questions, which perhaps > cover the last missing bits for streamlined use of the request API by > userspace. I would suggest another item, related to 3): > > 5) How can applications tell whether the driver supports a specific > codec profile/level, not only for encoding but also for decoding? It's > common for low-end embedded hardware to not support the most advanced > profiles (e.g. H264 high profile). Just to point out that this is unrelated to the Request API. I see this as a separate topic that is probably worth an RFC by itself. Regards, Hans
Re: [RFC] Request API and V4L2 capabilities
Le lundi 06 août 2018 à 10:16 +0200, Paul Kocialkowski a écrit : > Hi Hans and all, > > On Sat, 2018-08-04 at 15:50 +0200, Hans Verkuil wrote: > > Hi all, > > > > While the Request API patch series addresses all the core API issues, there > > are some high-level considerations as well: > > > > 1) How can the application tell that the Request API is supported and for > >which buffer types (capture/output) and pixel formats? > > > > 2) How can the application tell if the Request API is required as opposed > > to being > >optional? > > > > 3) Some controls may be required in each request, how to let userspace know > > this? > >Is it even necessary to inform userspace? > > > > 4) (For bonus points): How to let the application know which streaming I/O > > modes > >are available? That's never been possible before, but it would be very > > nice > >indeed if that's made explicit. > > Thanks for bringing up these considerations and questions, which perhaps > cover the last missing bits for streamlined use of the request API by > userspace. I would suggest another item, related to 3): > > 5) How can applications tell whether the driver supports a specific > codec profile/level, not only for encoding but also for decoding? It's > common for low-end embedded hardware to not support the most advanced > profiles (e.g. H264 high profile). Hi Paul, after some discussion with Philip, he sent a proposal patch that enables profile/level extended CID support to decoders too. The control is made read-only, the point is not really the CID get/set but that the controls allow enumerating the supported values. This seems quite straightforward and easy to use. This enumeration is already provided this way some of the existing sate-full encoders. > > > Since the Request API associates data with frame buffers it makes sense to > > expose > > this as a new capability field in struct v4l2_requestbuffers and struct > > v4l2_create_buffers. > > > > The first struct has 2 reserved fields, the second has 8, so it's not a > > problem to > > take one for a capability field. Both structs also have a buffer type, so > > we know > > if this is requested for a capture or output buffer type. The pixel format > > is known > > in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I doubt > > we'll have > > drivers where the request caps would actually depend on the pixel format, > > but it > > theoretically possible. For both ioctls you can call them with count=0 at > > the start > > of the application. REQBUFS has of course the side-effect of deleting all > > buffers, > > but at the start of your application you don't have any yet. CREATE_BUFS > > has no > > side-effects. > > My initial thoughts on this point were to have flags exposed in > v4l2_capability, but now that you're saying it, it does make sense for > the flag to be associated with a buffer rather than the global device. > > In addition, I've heard of cases (IIRC it was some Rockchip platforms) > where the platform has both stateless and stateful VPUs (I think it was > stateless up to H264 and stateful for H265). This would allow supporting > these two hardware blocks under the same video device (if that makes > sense anyway). And even if there's no immediate need, it's always good > to have this level of granularity (with little drawbacks). > > > I propose adding these capabilities: > > > > #define V4L2_BUF_CAP_HAS_REQUESTS 0x0001 > > #define V4L2_BUF_CAP_REQUIRES_REQUESTS 0x0002 > > #define V4L2_BUF_CAP_HAS_MMAP 0x0100 > > #define V4L2_BUF_CAP_HAS_USERPTR0x0200 > > #define V4L2_BUF_CAP_HAS_DMABUF 0x0400 > > > > If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. > > > > At this time I think that REQUIRES_REQUESTS would only need to be set for > > the > > output queue of stateless codecs. > > > > If capabilities is 0, then it's from an old kernel and all you know is that > > requests are certainly not supported, and that MMAP is supported. Whether > > USERPTR > > or DMABUF are supported isn't known in that case (just try it :-) ). > > Sounds good to me! > > > Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF caps, but it > > is very > > easy to add if we create a new capability field anyway, and it has always > > annoyed > > the hell out of me that we didn't have a good way to let userspace know what > > streaming I/O modes we support. And with vb2 it's easy to implement. > > I totally agree here, it would be very nice to take the occasion to > expose to userspace what I/O modes are available. The current try-and- > see approach works, but this feels much better indeed. > > > Regarding point 3: I think this should be documented next to the pixel > > format. I.e. > > the MPEG-2 Slice format used by the stateless cedrus codec requires the > > request API > > and that two MPEG-2 controls (slice params and quantization matrices) must >
Re: [BUG, RFC] media: Wrong module gets acquired
Dne 14.8.2018 v 15:03 Mauro Carvalho Chehab napsal(a): > Em Tue, 14 Aug 2018 11:35:01 +0300 > Sakari Ailus escreveu: > >> Hi Pert, >> >> On Mon, Aug 13, 2018 at 06:33:12PM +0200, petrcve...@gmail.com wrote: >>> From: Petr Cvek >>> >>> When transferring a media sensor driver from the soc_camera I've found >>> the controller module can get removed (which will cause a stack dump >>> because the sensor driver depends on resources from the controller driver). >>> >> >> There may be a kernel oops if a resource used by another driver goes away. >> But the right fix isn't to prevent unloading that module. Instead, one way >> to address the problem would be to have persistent clock objects that would >> not be dependent on the driver that provides them. >> >>> >>> When I've tried to remove the driver module of the sensor it said the >>> resource was busy (without a reference name) though is should be >>> possible to remove the sensor driver because it is at the end of >>> the dependency list and not to remove the controller driver. >> >> That might be one day possible but it is not today. >> >> You'll still need to acquire the sensor module as well as it registers a >> media entity as well as a sub-device. > > Let put my 2 cents here. > > Usually, the same problem of removing modules happen if you just > ask the driver's core to unbind a module (with can be done via > sysfs). > > Removing/unbinding a driver that uses media controller should work, > if the unbinding code at the driver (e. g. i2c_driver::remove field) > would delete the media controller entities, and the caller driver > doesn't cache it. > Yeah I assume it would be same as removing my sensor module from v4l2_async and unregistering v4l2 clocks. Petr
[PATCHv2 2/5] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS
From: Jose Abreu
Add a new flag to UAPI for DV timings which, whenever set,
indicates that hardware can detect the difference between
regular FPS and 1000/1001 FPS.
This is specific to HDMI receivers. Also, it is only valid
when V4L2_DV_FL_CAN_REDUCE_FPS is set.
Signed-off-by: Jose Abreu
Cc: Carlos Palminha
Signed-off-by: Hans Verkuil
---
include/uapi/linux/videodev2.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5d1a3685bea9..622f0479d668 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1400,6 +1400,13 @@ struct v4l2_bt_timings {
* InfoFrame).
*/
#define V4L2_DV_FL_HAS_HDMI_VIC(1 << 8)
+/*
+ * CEA-861 specific: only valid for video receivers.
+ * If set, then HW can detect the difference between regular FPS and
+ * 1000/1001 FPS. Note: This flag is only valid for HDMI VIC codes with
+ * the V4L2_DV_FL_CAN_REDUCE_FPS flag set.
+ */
+#define V4L2_DV_FL_CAN_DETECT_REDUCED_FPS (1 << 9)
/* A few useful defines to calculate the total blanking and frame sizes */
#define V4L2_DV_BT_BLANKING_WIDTH(bt) \
--
2.18.0
[PATCHv2 5/5] adv7842: enable reduced fps detection
From: Hans Verkuil
The pixelclock detection of the adv7842 is precise enough to detect
if the framerate is 60 Hz or 59.94 Hz (aka "reduced fps").
Implement this detection.
Signed-off-by: Hans Verkuil
---
drivers/media/i2c/adv7842.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 4f8fbdd00e35..999d621f5667 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -1525,6 +1525,7 @@ static void
adv7842_fill_optional_dv_timings_fields(struct v4l2_subdev *sd,
v4l2_find_dv_timings_cap(timings, adv7842_get_dv_timings_cap(sd),
is_digital_input(sd) ? 25 : 100,
adv7842_check_dv_timings, NULL);
+ timings->bt.flags |= V4L2_DV_FL_CAN_DETECT_REDUCED_FPS;
}
static int adv7842_query_dv_timings(struct v4l2_subdev *sd,
@@ -1596,6 +1597,14 @@ static int adv7842_query_dv_timings(struct v4l2_subdev
*sd,
bt->il_vbackporch = 0;
}
adv7842_fill_optional_dv_timings_fields(sd, timings);
+ if ((timings->bt.flags & V4L2_DV_FL_CAN_REDUCE_FPS) &&
+ freq < bt->pixelclock) {
+ u32 reduced_freq = (bt->pixelclock / 1001) * 1000;
+ u32 delta_freq = abs(freq - reduced_freq);
+
+ if (delta_freq < (bt->pixelclock - reduced_freq) / 2)
+ timings->bt.flags |= V4L2_DV_FL_REDUCED_FPS;
+ }
} else {
/* find format
* Since LCVS values are inaccurate [REF_03, p. 339-340],
--
2.18.0
[PATCHv2 3/5] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper
From: Jose Abreu
A new helper function was introduced to facilitate the calculation
of time per frame value whenever we have access to the full
v4l2_dv_timings structure.
This should be used only for receivers and only when there is
enough accuracy in the measured pixel clock value as well as in
the horizontal/vertical values.
Signed-off-by: Jose Abreu
Cc: Carlos Palminha
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++
include/media/v4l2-dv-timings.h | 11 +++
2 files changed, 50 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c
b/drivers/media/v4l2-core/v4l2-dv-timings.c
index c81faea96fba..8f52353b0881 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -373,6 +373,45 @@ struct v4l2_fract v4l2_dv_timings_aspect_ratio(const
struct v4l2_dv_timings *t)
}
EXPORT_SYMBOL_GPL(v4l2_dv_timings_aspect_ratio);
+/** v4l2_calc_timeperframe - helper function to calculate timeperframe based
+ * v4l2_dv_timings fields.
+ * @t - Timings for the video mode.
+ *
+ * Calculates the expected timeperframe using the pixel clock value and
+ * horizontal/vertical measures. This means that v4l2_dv_timings structure
+ * must be correctly and fully filled.
+ */
+struct v4l2_fract v4l2_calc_timeperframe(const struct v4l2_dv_timings *t)
+{
+ const struct v4l2_bt_timings *bt = >bt;
+ struct v4l2_fract fps_fract = { 1, 1 };
+ unsigned long n, d;
+ u32 htot, vtot, fps;
+ u64 pclk;
+
+ if (t->type != V4L2_DV_BT_656_1120)
+ return fps_fract;
+
+ htot = V4L2_DV_BT_FRAME_WIDTH(bt);
+ vtot = V4L2_DV_BT_FRAME_HEIGHT(bt);
+ pclk = bt->pixelclock;
+
+ if ((bt->flags & V4L2_DV_FL_CAN_DETECT_REDUCED_FPS) &&
+ (bt->flags & V4L2_DV_FL_REDUCED_FPS))
+ pclk = div_u64(pclk * 1000ULL, 1001);
+
+ fps = (htot * vtot) > 0 ? div_u64((100 * pclk), (htot * vtot)) : 0;
+ if (!fps)
+ return fps_fract;
+
+ rational_best_approximation(fps, 100, fps, 100, , );
+
+ fps_fract.numerator = d;
+ fps_fract.denominator = n;
+ return fps_fract;
+}
+EXPORT_SYMBOL_GPL(v4l2_calc_timeperframe);
+
/*
* CVT defines
* Based on Coordinated Video Timings Standard
diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
index 17cb27df1b81..fb355d9577a4 100644
--- a/include/media/v4l2-dv-timings.h
+++ b/include/media/v4l2-dv-timings.h
@@ -10,6 +10,17 @@
#include
+/**
+ * v4l2_calc_timeperframe - helper function to calculate timeperframe based
+ * v4l2_dv_timings fields.
+ * @t: Timings for the video mode.
+ *
+ * Calculates the expected timeperframe using the pixel clock value and
+ * horizontal/vertical measures. This means that v4l2_dv_timings structure
+ * must be correctly and fully filled.
+ */
+struct v4l2_fract v4l2_calc_timeperframe(const struct v4l2_dv_timings *t);
+
/*
* v4l2_dv_timings_presets: list of all dv_timings presets.
*/
--
2.18.0
[PATCHv2 1/5] vidioc-g-dv-timings.rst: document V4L2_DV_FL_CAN_DETECT_REDUCED_FPS
From: Hans Verkuil Document the new V4L2_DV_FL_CAN_DETECT_REDUCED_FPS flag and update the V4L2_DV_FL_REDUCED_FPS description since it can now also be used with receivers. Signed-off-by: Hans Verkuil --- .../media/uapi/v4l/vidioc-g-dv-timings.rst| 27 +-- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst b/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst index 1a034e825161..35cba2c8d459 100644 --- a/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst +++ b/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst @@ -257,14 +257,19 @@ EBUSY will also be cleared. This is a read-only flag, applications must not set this. * - ``V4L2_DV_FL_REDUCED_FPS`` - - CEA-861 specific: only valid for video transmitters, the flag is - cleared by receivers. It is also only valid for formats with the - ``V4L2_DV_FL_CAN_REDUCE_FPS`` flag set, for other formats the - flag will be cleared by the driver. If the application sets this - flag, then the pixelclock used to set up the transmitter is - divided by 1.001 to make it compatible with NTSC framerates. If - the transmitter can't generate such frequencies, then the flag - will also be cleared. + - CEA-861 specific: only valid for video transmitters or video +receivers that have the ``V4L2_DV_FL_CAN_DETECT_REDUCED_FPS`` + set. This flag is cleared otherwise. It is also only valid for + formats with the ``V4L2_DV_FL_CAN_REDUCE_FPS`` flag set, for other + formats the flag will be cleared by the driver. + + If the application sets this flag for a transmitter, then the + pixelclock used to set up the transmitter is divided by 1.001 to + make it compatible with NTSC framerates. If the transmitter can't + generate such frequencies, then the flag will be cleared. + + If a video receiver detects that the format uses a reduced framerate, + then it will set this flag to signal this to the application. * - ``V4L2_DV_FL_HALF_LINE`` - Specific to interlaced formats: if set, then the vertical backporch of field 1 (aka the odd field) is really one half-line @@ -294,3 +299,9 @@ EBUSY - If set, then the hdmi_vic field is valid and contains the Video Identification Code as per the HDMI standard (HDMI Vendor Specific InfoFrame). +* - ``V4L2_DV_FL_CAN_DETECT_REDUCED_FPS`` + - CEA-861 specific: only valid for video receivers, the flag is +cleared by transmitters. +If set, then the hardware can detect the difference between + regular framerates and framerates reduced by 1000/1001. E.g.: + 60 vs 59.94 Hz, 30 vs 29.97 Hz or 24 vs 23.976 Hz. -- 2.18.0
[PATCHv2 0/5] Handling of reduced FPS in V4L2
From: Hans Verkuil This is a v2 patch series of an old patch series from Jose: https://www.mail-archive.com/linux-media@vger.kernel.org/msg110275.html It's rebased, I updated the documentation and implemented this for the adv7842 which is one of the very few receivers that can detect the received pixelclock frequency with sufficient precision to tell the difference between 60 and 59.94 Hz. I also verified that the adv7604 definitely can NOT do this. Back in March 2017 I promised Jose that I would test this on my setup with a cobalt driver and an adv7842 receiver 'within a week'. Hmmm... Let's just say: better late than never. Regards, Hans Hans Verkuil (2): vidioc-g-dv-timings.rst: document V4L2_DV_FL_CAN_DETECT_REDUCED_FPS adv7842: enable reduced fps detection Jose Abreu (3): videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper cobalt: Use v4l2_calc_timeperframe helper .../media/uapi/v4l/vidioc-g-dv-timings.rst| 27 + drivers/media/i2c/adv7842.c | 9 + drivers/media/pci/cobalt/cobalt-v4l2.c| 9 - drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++ include/media/v4l2-dv-timings.h | 11 ++ include/uapi/linux/videodev2.h| 7 6 files changed, 92 insertions(+), 10 deletions(-) -- 2.18.0
[PATCHv2 4/5] cobalt: Use v4l2_calc_timeperframe helper
From: Jose Abreu
Currently, cobalt driver always returns 60fps in g_parm.
This patch uses the new v4l2_calc_timeperframe helper to
calculate the time per frame value.
Signed-off-by: Jose Abreu
Cc: Carlos Palminha
Tested-by: Hans Verkuil
Signed-off-by: Hans Verkuil
---
drivers/media/pci/cobalt/cobalt-v4l2.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c
b/drivers/media/pci/cobalt/cobalt-v4l2.c
index e2a4c705d353..c8fd2d075f43 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -1064,10 +1064,15 @@ static int cobalt_subscribe_event(struct v4l2_fh *fh,
static int cobalt_g_parm(struct file *file, void *fh, struct v4l2_streamparm
*a)
{
+ struct cobalt_stream *s = video_drvdata(file);
+ struct v4l2_fract fps;
+
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
- a->parm.capture.timeperframe.numerator = 1;
- a->parm.capture.timeperframe.denominator = 60;
+
+ fps = v4l2_calc_timeperframe(>timings);
+ a->parm.capture.timeperframe.numerator = fps.numerator;
+ a->parm.capture.timeperframe.denominator = fps.denominator;
a->parm.capture.readbuffers = 3;
return 0;
}
--
2.18.0
Re: [PATCH v2 1/4] media: soc_camera: ov9640: move ov9640 out of soc_camera
BTW from the v1 discussion: Dne 10.8.2018 v 09:32 jacopo mondi napsal(a): > When I've been recently doing the same for ov772x and other sensor > driver I've been suggested to first copy the driver into > drivers/media/i2c/ and leave the original soc_camera one there, so > they can be bulk removed or moved to staging. I'll let Hans confirm > this, as he's about to take care of this process. I would rather used git mv for preserve the git history, but if a simple copy is fine then I'm fine too ;-). Petr
[PATCH v2 3/4] media: i2c: ov9640: add missing SPDX identifiers
From: Petr Cvek Add missing SPDX identifiers to .c and .h files of the sensor driver. Signed-off-by: Petr Cvek --- drivers/media/i2c/ov9640.c | 5 + drivers/media/i2c/ov9640.h | 5 + 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c index ae55d13233f0..158959193453 100644 --- a/drivers/media/i2c/ov9640.c +++ b/drivers/media/i2c/ov9640.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * OmniVision OV96xx Camera Driver * @@ -14,10 +15,6 @@ * Copyright 2006-7 Jonathan Corbet * Copyright (C) 2008 Magnus Damm * Copyright (C) 2008, Guennadi Liakhovetski - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include diff --git a/drivers/media/i2c/ov9640.h b/drivers/media/i2c/ov9640.h index be5e4b29ac69..a8ed6992c1a8 100644 --- a/drivers/media/i2c/ov9640.h +++ b/drivers/media/i2c/ov9640.h @@ -1,11 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * OmniVision OV96xx Camera Header File * * Copyright (C) 2009 Marek Vasut - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef__DRIVERS_MEDIA_VIDEO_OV9640_H__ -- 2.18.0
[PATCH v2 2/4] media: i2c: ov9640: drop soc_camera code and switch to v4l2_async
From: Petr Cvek
This patch removes the dependency on an obsoleted soc_camera from ov9640
driver and changes the code to be a standalone v4l2 async subdevice.
It also adds GPIO allocations for power and reset signals (as they are not
handled by soc_camera now).
The values for waiting on GPIOs (reset and power) settling down were taken
from the datasheet (> 1 ms after HW/SW reset). The upper limit was chosen
as an arbitrary value. Also one occurrence of mdelay() was changed to
msleep(). The delays were successfully tested on a real hardware.
The patch makes ov9640 sensor again compatible with the pxa_camera driver.
Signed-off-by: Petr Cvek
---
drivers/media/i2c/ov9640.c | 76 ++
drivers/media/i2c/ov9640.h | 2 +
2 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
index c63948989688..ae55d13233f0 100644
--- a/drivers/media/i2c/ov9640.c
+++ b/drivers/media/i2c/ov9640.c
@@ -9,6 +9,7 @@
* Kuninori Morimoto
*
* Based on ov7670 and soc_camera_platform driver,
+ * transition from soc_camera to pxa_camera based on mt9m111
*
* Copyright 2006-7 Jonathan Corbet
* Copyright (C) 2008 Magnus Damm
@@ -27,10 +28,14 @@
#include
#include
-#include
+#include
#include
#include
#include
+#include
+#include
+
+#include
#include "ov9640.h"
@@ -323,11 +328,23 @@ static int ov9640_set_register(struct v4l2_subdev *sd,
static int ov9640_s_power(struct v4l2_subdev *sd, int on)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
struct ov9640_priv *priv = to_ov9640_sensor(sd);
-
- return soc_camera_set_power(>dev, ssdd, priv->clk, on);
+ int ret = 0;
+
+ if (on) {
+ gpiod_set_value(priv->gpio_power, 1);
+ usleep_range(1000, 2000);
+ ret = v4l2_clk_enable(priv->clk);
+ usleep_range(1000, 2000);
+ gpiod_set_value(priv->gpio_reset, 0);
+ } else {
+ gpiod_set_value(priv->gpio_reset, 1);
+ usleep_range(1000, 2000);
+ v4l2_clk_disable(priv->clk);
+ usleep_range(1000, 2000);
+ gpiod_set_value(priv->gpio_power, 0);
+ }
+ return ret;
}
/* select nearest higher resolution for capture */
@@ -475,7 +492,7 @@ static int ov9640_prog_dflt(struct i2c_client *client)
}
/* wait for the changes to actually happen, 140ms are not enough yet */
- mdelay(150);
+ msleep(150);
return 0;
}
@@ -631,14 +648,10 @@ static const struct v4l2_subdev_core_ops ov9640_core_ops
= {
static int ov9640_g_mbus_config(struct v4l2_subdev *sd,
struct v4l2_mbus_config *cfg)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
-
cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_PARALLEL;
- cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
return 0;
}
@@ -667,18 +680,27 @@ static int ov9640_probe(struct i2c_client *client,
const struct i2c_device_id *did)
{
struct ov9640_priv *priv;
- struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
int ret;
- if (!ssdd) {
- dev_err(>dev, "Missing platform_data for driver\n");
- return -EINVAL;
- }
-
- priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(>dev, sizeof(*priv),
+ GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ priv->gpio_power = devm_gpiod_get(>dev, "Camera power",
+ GPIOD_OUT_LOW);
+ if (IS_ERR_OR_NULL(priv->gpio_power)) {
+ ret = PTR_ERR(priv->gpio_power);
+ return ret;
+ }
+
+ priv->gpio_reset = devm_gpiod_get(>dev, "Camera reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR_OR_NULL(priv->gpio_reset)) {
+ ret = PTR_ERR(priv->gpio_reset);
+ return ret;
+ }
+
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 2);
@@ -697,12 +719,20 @@ static int ov9640_probe(struct i2c_client *client,
}
ret = ov9640_video_probe(client);
- if (ret) {
- v4l2_clk_put(priv->clk);
-eclkget:
- v4l2_ctrl_handler_free(>hdl);
- }
+ if (ret)
+ goto eprobe;
+ priv->subdev.dev = >dev;
+ ret = v4l2_async_register_subdev(>subdev);
+ if (ret)
+ goto eprobe;
+
+ return 0;
+
+eprobe:
+
[PATCH v2 4/4] MAINTAINERS: Add Petr Cvek as a maintainer for the ov9640 driver
From: Petr Cvek The soc_camera drivers are marked as orphaned. Add Petr Cvek as a new maintainer for ov9640 driver after its switch from the soc_camera. Signed-off-by: Petr Cvek --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 40d5ec9292ca..cab3fa4ccb37 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10627,6 +10627,12 @@ S: Maintained F: drivers/media/i2c/ov7740.c F: Documentation/devicetree/bindings/media/i2c/ov7740.txt +OMNIVISION OV9640 SENSOR DRIVER +M: Petr Cvek +L: linux-media@vger.kernel.org +S: Maintained +F: drivers/media/i2c/ov9640.* + OMNIVISION OV9650 SENSOR DRIVER M: Sakari Ailus R: Akinobu Mita -- 2.18.0
[PATCH v2 1/4] media: soc_camera: ov9640: move ov9640 out of soc_camera
From: Petr Cvek
Initial part of ov9640 transition from soc_camera subsystem to a standalone
v4l2 subdevice. The soc_camera version seems to be used only in Palm Zire72
and in (the future) HTC Magician. On these two devices the support is
broken as pxa_camera driver doesn't use soc_camera anymore. The other
mentions from git grep are "TODOs" (in board-osk.c) or chip names for
unsupported sensors on HW which doesn't use soc_camera at all (irelevant).
Copy the driver files from soc_camera and mark the original ones in the
Kconfig description as obsoleted.
Add config option VIDEO_OV9640 to the build files in drivers/media/i2c.
Signed-off-by: Petr Cvek
---
drivers/media/i2c/Kconfig| 7 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/ov9640.c | 739 +++
drivers/media/i2c/ov9640.h | 208
drivers/media/i2c/soc_camera/Kconfig | 6 +-
5 files changed, 959 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/i2c/ov9640.c
create mode 100644 drivers/media/i2c/ov9640.h
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 439f6be08b95..c948b163a567 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -771,6 +771,13 @@ config VIDEO_OV7740
This is a Video4Linux2 sensor driver for the OmniVision
OV7740 VGA camera sensor.
+config VIDEO_OV9640
+ tristate "OmniVision OV9640 sensor support"
+ depends on I2C && VIDEO_V4L2
+ help
+ This is a Video4Linux2 sensor driver for the OmniVision
+ OV9640 camera sensor.
+
config VIDEO_OV9650
tristate "OmniVision OV9650/OV9652 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 837c428339df..9cc951f9c041 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
+obj-$(CONFIG_VIDEO_OV9640) += ov9640.o
obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
new file mode 100644
index ..c63948989688
--- /dev/null
+++ b/drivers/media/i2c/ov9640.c
@@ -0,0 +1,739 @@
+/*
+ * OmniVision OV96xx Camera Driver
+ *
+ * Copyright (C) 2009 Marek Vasut
+ *
+ * Based on ov772x camera driver:
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "ov9640.h"
+
+#define to_ov9640_sensor(sd) container_of(sd, struct ov9640_priv, subdev)
+
+/* default register setup */
+static const struct ov9640_reg ov9640_regs_dflt[] = {
+ { OV9640_COM5, OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
+ { OV9640_COM6, OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
+ OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
+ { OV9640_PSHFT, OV9640_PSHFT_VAL(0x01) },
+ { OV9640_ACOM, OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
+ { OV9640_TSLB, OV9640_TSLB_YUYV_UYVY },
+ { OV9640_COM16, OV9640_COM16_RB_AVG },
+
+ /* Gamma curve P */
+ { 0x6c, 0x40 }, { 0x6d, 0x30 }, { 0x6e, 0x4b }, { 0x6f, 0x60 },
+ { 0x70, 0x70 }, { 0x71, 0x70 }, { 0x72, 0x70 }, { 0x73, 0x70 },
+ { 0x74, 0x60 }, { 0x75, 0x60 }, { 0x76, 0x50 }, { 0x77, 0x48 },
+ { 0x78, 0x3a }, { 0x79, 0x2e }, { 0x7a, 0x28 }, { 0x7b, 0x22 },
+
+ /* Gamma curve T */
+ { 0x7c, 0x04 }, { 0x7d, 0x07 }, { 0x7e, 0x10 }, { 0x7f, 0x28 },
+ { 0x80, 0x36 }, { 0x81, 0x44 }, { 0x82, 0x52 }, { 0x83, 0x60 },
+ { 0x84, 0x6c }, { 0x85, 0x78 }, { 0x86, 0x8c }, { 0x87, 0x9e },
+ { 0x88, 0xbb }, { 0x89, 0xd2 }, { 0x8a, 0xe6 },
+};
+
+/* Configurations
+ * NOTE: for YUV, alter the following registers:
+ * COM12 |= OV9640_COM12_YUV_AVG
+ *
+ * for RGB, alter the following registers:
+ * COM7 |= OV9640_COM7_RGB
+ * COM13 |= OV9640_COM13_RGB_AVG
+ * COM15 |= proper RGB color encoding mode
+ */
+static const struct ov9640_reg ov9640_regs_qqcif[] = {
+ { OV9640_CLKRC, OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x0f) },
+ { OV9640_COM1, OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
+ { OV9640_COM4, OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
+ {
[PATCH v2 0/4] media: soc_camera: ov9640: switch driver to v4l2_async
From: Petr Cvek This patch series transfer the ov9640 driver from the soc_camera subsystem into a standalone v4l2 driver. There is no changes except the required v4l2_async calls, GPIO allocation, deletion of now unused variables, a change from mdelay() to msleep() and an addition of SPDX identifiers (as suggested in the v1 version RFC). The config symbol has been changed from CONFIG_SOC_CAMERA_OV9640 to CONFIG_VIDEO_OV9640. Also as the drivers of the soc_camera seems to be orphaned I'm volunteering as a maintainer of the driver (I own the hardware). I've found the ov9640 seems to be used at least in (the future) HTC Magician and Palm Zire72. These will need to define power and reset GPIOs and remove the soc_camera definitions. I'm debugging it on magician now (ov9640 was unusable on them since the pxa_camera switched from the soc_camera). Petr Cvek (4): media: soc_camera: ov9640: move ov9640 out of soc_camera media: i2c: ov9640: drop soc_camera code and switch to v4l2_async media: i2c: ov9640: add missing SPDX identifiers MAINTAINERS: Add Petr Cvek as a maintainer for the ov9640 driver MAINTAINERS | 6 + drivers/media/i2c/Kconfig| 7 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov9640.c | 766 +++ drivers/media/i2c/ov9640.h | 207 drivers/media/i2c/soc_camera/Kconfig | 6 +- 6 files changed, 991 insertions(+), 2 deletions(-) create mode 100644 drivers/media/i2c/ov9640.c create mode 100644 drivers/media/i2c/ov9640.h -- 2.18.0
Re: [RFC] Request API and V4L2 capabilities
2018-08-06 11:13 GMT+02:00 Paul Kocialkowski : > Hi, > > On Mon, 2018-08-06 at 10:32 +0200, Hans Verkuil wrote: >> On 08/06/2018 10:16 AM, Paul Kocialkowski wrote: >> > On Sat, 2018-08-04 at 15:50 +0200, Hans Verkuil wrote: >> > > Regarding point 3: I think this should be documented next to the pixel >> > > format. I.e. >> > > the MPEG-2 Slice format used by the stateless cedrus codec requires the >> > > request API >> > > and that two MPEG-2 controls (slice params and quantization matrices) >> > > must be present >> > > in each request. >> > > >> > > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is >> > > needed here. >> > > It's really implied by the fact that you use a stateless codec. It >> > > doesn't help >> > > generic applications like v4l2-ctl or qv4l2 either since in order to >> > > support >> > > stateless codecs they will have to know about the details of these >> > > controls anyway. >> > > >> > > So I am inclined to say that it is not necessary to expose this >> > > information in >> > > the API, but it has to be documented together with the pixel format >> > > documentation. >> > >> > I think this is affected by considerations about codec profile/level >> > support. More specifically, some controls will only be required for >> > supporting advanced codec profiles/levels, so they can only be >> > explicitly marked with appropriate flags by the driver when the target >> > profile/level is known. And I don't think it would be sane for userspace >> > to explicitly set what profile/level it's aiming at. As a result, I >> > don't think we can explicitly mark controls as required or optional. >> > >> > I also like the idea that it should instead be implicit and that the >> > documentation should detail which specific stateless metadata controls >> > are required for a given profile/level. >> > >> > As for controls validation, the approach followed in the Cedrus driver >> > is to check that the most basic controls are filled and allow having >> > missing controls for those that match advanced profiles. >> > >> > Since this approach feels somewhat generic enough to be applied to all >> > stateless VPU drivers, maybe this should be made a helper in the >> > framework? >> >> Sounds reasonable. Not sure if it will be in the first version, but it is >> easy to add later. > > Definitely, I don't think this is such a high priority for now either. > >> > In addition, I see a need for exposing the maximum profile/level that >> > the driver supports for decoding. I would suggest reusing the already- >> > existing dedicated controls used for encoding for this purpose. For >> > decoders, they would be used to expose the (read-only) maximum >> > profile/level that is supported by the hardware and keep using them as a >> > settable value in a range (matching the level of support) for encoders. >> > >> > This is necessary for userspace to determine whether a given video can >> > be decoded in hardware or not. Instead of half-way decoding the video >> > (ending up in funky results), this would easily allow skipping hardware >> > decoding and e.g. falling back on software decoding. >> >> I think it might be better to expose this through new read-only bitmask >> controls: i.e. a bitmask containing the supported profiles and levels. > > It seems that this is more or less what the coda driver is doing for > decoding actually, although it uses a menu control between min/max > supported profile/levels, with a mask to "blacklist" the unsupported > values. Then, the V4L2_CTRL_FLAG_READ_ONLY flag is set to keep the > control read-only. > >> Reusing the existing controls for a decoder is odd since there is not >> really a concept of a 'current' value since you just want to report what >> is supported. And I am not sure if all decoders can report the profile >> or level that they detect. > > Is that really a problem when the READ_ONLY flag is set? I thought it > was designed to fit this specific case, when the driver reports a value > that userspace cannot affect. > > Otherwise, I agree that having a bitmask type would be a better fit, but > I think it would be beneficial to keep the already-defined control and > associated values, which implies using the menu control type for both > encoders and decoders. > > If this is not an option, I would be in favour of adding per-codec read- > only bitmask controls (e.g. for H264 something like > V4L2_CID_MPEG_VIDEO_H264_PROFILE_SUPPORT) that expose the already- > existing profile/level definitions as bit identifiers (a bit like coda > is using them to craft a mask for the menu items to blacklist) for > decoding only. > > What do you think? Hi Paul, I think we need to go deeper than just exposing the supported profiles/levels and also include a way to query the CAPTURE pixel formats that are supported for each profile. Maybe HEVC Main produces yuv420p but HEVC Main10 gives you yuv420p10le. Maybe H.264 HiP produces NV12 but H.264 Hi422P produces YUYV
Re: [PATCHv18 19/35] vb2: store userspace data in vb2_v4l2_buffer
On 15/08/18 14:28, Mauro Carvalho Chehab wrote:
> Em Wed, 15 Aug 2018 13:54:53 +0200
> Hans Verkuil escreveu:
>
>> On 14/08/18 21:47, Mauro Carvalho Chehab wrote:
>>> Em Tue, 14 Aug 2018 16:20:31 +0200
>>> Hans Verkuil escreveu:
>>
>>
>>
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 57848ddc584f..360dc4e7d413 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -154,17 +154,11 @@ static void vb2_warn_zero_bytesused(struct
vb2_buffer *vb)
pr_warn("use the actual size instead.\n");
}
-/*
- * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
- * v4l2_buffer by the userspace. It also verifies that struct
- * v4l2_buffer has a valid number of planes.
- */
-static int __fill_vb2_buffer(struct vb2_buffer *vb,
- const void *pb, struct vb2_plane *planes)
+static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct
v4l2_buffer *b)
{
struct vb2_queue *q = vb->vb2_queue;
- const struct v4l2_buffer *b = pb;
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct vb2_plane *planes = vbuf->planes;
unsigned int plane;
int ret;
@@ -186,7 +180,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
dprintk(1, "the field is incorrectly set to ALTERNATE for an
output buffer\n");
return -EINVAL;
}
- vb->timestamp = 0;
>>>
>>> See my note below about this removal. On a quick look, I guess we may have
>>> a regression here for output buffers (non-m2m).
>>
>> Note that this is no longer the __fill_vb2_buffer() callback, and the
>> timestamp
>> is not handled in vb2_fill_vb2_v4l2_buffer(). That's why it is removed here.
>>
>> It is handled in the new __fill_vb2_buffer function, see below for comments.
>>
>>>
vbuf->sequence = 0;
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
@@ -208,6 +201,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
}
break;
default:
+ for (plane = 0; plane < vb->num_planes; ++plane) {
+ planes[plane].m.offset =
+ vb->planes[plane].m.offset;
+ planes[plane].length =
+ vb->planes[plane].length;
+ }
break;
}
@@ -269,9 +268,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
planes[0].length = b->length;
break;
default:
+ planes[0].m.offset = vb->planes[0].m.offset;
+ planes[0].length = vb->planes[0].length;
break;
}
+ planes[0].data_offset = 0;
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
if (b->bytesused == 0)
vb2_warn_zero_bytesused(vb);
@@ -286,7 +288,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
}
- /* Zero flags that the vb2 core handles */
+ /* Zero flags that we handle */
vbuf->flags = b->flags & ~V4L2_BUFFER_MASK_FLAGS;
if (!vb->vb2_queue->copy_timestamp || !V4L2_TYPE_IS_OUTPUT(b->type)) {
/*
@@ -319,6 +321,10 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct
v4l2_buffer *b,
const char *opname)
{
+ struct vb2_v4l2_buffer *vbuf;
+ struct vb2_buffer *vb;
+ int ret;
+
if (b->type != q->type) {
dprintk(1, "%s: invalid buffer type\n", opname);
return -EINVAL;
@@ -340,7 +346,15 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue
*q, struct v4l2_buffer *b,
return -EINVAL;
}
- return __verify_planes_array(q->bufs[b->index], b);
+ vb = q->bufs[b->index];
+ vbuf = to_vb2_v4l2_buffer(vb);
+ ret = __verify_planes_array(vb, b);
+ if (ret)
+ return ret;
+
+ /* Copy relevant information provided by the userspace */
+ memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes);
+ return vb2_fill_vb2_v4l2_buffer(vb, b);
}
/*
@@ -448,6 +462,30 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
void *pb)
q->last_buffer_dequeued = true;
}
+/*
+ * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
+ * v4l2_buffer by the userspace. It also verifies that struct
+ *
Re: [PATCHv18 19/35] vb2: store userspace data in vb2_v4l2_buffer
Em Wed, 15 Aug 2018 13:54:53 +0200
Hans Verkuil escreveu:
> On 14/08/18 21:47, Mauro Carvalho Chehab wrote:
> > Em Tue, 14 Aug 2018 16:20:31 +0200
> > Hans Verkuil escreveu:
>
>
>
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> >> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> >> index 57848ddc584f..360dc4e7d413 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> >> @@ -154,17 +154,11 @@ static void vb2_warn_zero_bytesused(struct
> >> vb2_buffer *vb)
> >>pr_warn("use the actual size instead.\n");
> >> }
> >>
> >> -/*
> >> - * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
> >> - * v4l2_buffer by the userspace. It also verifies that struct
> >> - * v4l2_buffer has a valid number of planes.
> >> - */
> >> -static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >> - const void *pb, struct vb2_plane *planes)
> >> +static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct
> >> v4l2_buffer *b)
> >> {
> >>struct vb2_queue *q = vb->vb2_queue;
> >> - const struct v4l2_buffer *b = pb;
> >>struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >> + struct vb2_plane *planes = vbuf->planes;
> >>unsigned int plane;
> >>int ret;
> >>
> >> @@ -186,7 +180,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >>dprintk(1, "the field is incorrectly set to ALTERNATE for an
> >> output buffer\n");
> >>return -EINVAL;
> >>}
> >> - vb->timestamp = 0;
> >
> > See my note below about this removal. On a quick look, I guess we may have
> > a regression here for output buffers (non-m2m).
>
> Note that this is no longer the __fill_vb2_buffer() callback, and the
> timestamp
> is not handled in vb2_fill_vb2_v4l2_buffer(). That's why it is removed here.
>
> It is handled in the new __fill_vb2_buffer function, see below for comments.
>
> >
> >>vbuf->sequence = 0;
> >>
> >>if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> >> @@ -208,6 +201,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >>}
> >>break;
> >>default:
> >> + for (plane = 0; plane < vb->num_planes; ++plane) {
> >> + planes[plane].m.offset =
> >> + vb->planes[plane].m.offset;
> >> + planes[plane].length =
> >> + vb->planes[plane].length;
> >> + }
> >>break;
> >>}
> >>
> >> @@ -269,9 +268,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >>planes[0].length = b->length;
> >>break;
> >>default:
> >> + planes[0].m.offset = vb->planes[0].m.offset;
> >> + planes[0].length = vb->planes[0].length;
> >>break;
> >>}
> >>
> >> + planes[0].data_offset = 0;
> >>if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> >>if (b->bytesused == 0)
> >>vb2_warn_zero_bytesused(vb);
> >> @@ -286,7 +288,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >>
> >>}
> >>
> >> - /* Zero flags that the vb2 core handles */
> >> + /* Zero flags that we handle */
> >>vbuf->flags = b->flags & ~V4L2_BUFFER_MASK_FLAGS;
> >>if (!vb->vb2_queue->copy_timestamp || !V4L2_TYPE_IS_OUTPUT(b->type)) {
> >>/*
> >> @@ -319,6 +321,10 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> >> static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct
> >> v4l2_buffer *b,
> >>const char *opname)
> >> {
> >> + struct vb2_v4l2_buffer *vbuf;
> >> + struct vb2_buffer *vb;
> >> + int ret;
> >> +
> >>if (b->type != q->type) {
> >>dprintk(1, "%s: invalid buffer type\n", opname);
> >>return -EINVAL;
> >> @@ -340,7 +346,15 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue
> >> *q, struct v4l2_buffer *b,
> >>return -EINVAL;
> >>}
> >>
> >> - return __verify_planes_array(q->bufs[b->index], b);
> >> + vb = q->bufs[b->index];
> >> + vbuf = to_vb2_v4l2_buffer(vb);
> >> + ret = __verify_planes_array(vb, b);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Copy relevant information provided by the userspace */
> >> + memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes);
> >> + return vb2_fill_vb2_v4l2_buffer(vb, b);
> >> }
> >>
> >> /*
> >> @@ -448,6 +462,30 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
> >> void *pb)
> >>q->last_buffer_dequeued = true;
> >> }
> >>
> >> +/*
> >> + * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
> >> + * v4l2_buffer by the userspace. It also verifies that struct
> >> + * v4l2_buffer has a valid number of planes.
> >> + */
> >> +static
Re: [RFC] Request API and V4L2 capabilities
Em Sat, 4 Aug 2018 15:50:04 +0200 Hans Verkuil escreveu: > Hi all, > > While the Request API patch series addresses all the core API issues, there > are some high-level considerations as well: > > 1) How can the application tell that the Request API is supported and for >which buffer types (capture/output) and pixel formats? > > 2) How can the application tell if the Request API is required as opposed to > being >optional? Huh? Why would it be mandatory? > > 3) Some controls may be required in each request, how to let userspace know > this? >Is it even necessary to inform userspace? Again, why would it need to have a set of mandatory controls for requests to work? If this is really required, it should have a way to send such list to userspace. > > 4) (For bonus points): How to let the application know which streaming I/O > modes >are available? That's never been possible before, but it would be very nice >indeed if that's made explicit. > > Since the Request API associates data with frame buffers it makes sense to > expose > this as a new capability field in struct v4l2_requestbuffers and struct > v4l2_create_buffers. > > The first struct has 2 reserved fields, the second has 8, so it's not a > problem to > take one for a capability field. Both structs also have a buffer type, so we > know > if this is requested for a capture or output buffer type. The pixel format is > known > in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I doubt > we'll have > drivers where the request caps would actually depend on the pixel format, but > it > theoretically possible. For both ioctls you can call them with count=0 at the > start > of the application. REQBUFS has of course the side-effect of deleting all > buffers, > but at the start of your application you don't have any yet. CREATE_BUFS has > no > side-effects. > > I propose adding these capabilities: > > #define V4L2_BUF_CAP_HAS_REQUESTS 0x0001 I'm OK with that. > #define V4L2_BUF_CAP_REQUIRES_REQUESTS0x0002 But I'm not ok with breaking even more userspace support by forcing requests. > #define V4L2_BUF_CAP_HAS_MMAP 0x0100 > #define V4L2_BUF_CAP_HAS_USERPTR 0x0200 > #define V4L2_BUF_CAP_HAS_DMABUF 0x0400 Those sounds ok to me too. > > If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. > > At this time I think that REQUIRES_REQUESTS would only need to be set for the > output queue of stateless codecs. Same as before: I don't see the need of support a request-only driver. > > If capabilities is 0, then it's from an old kernel and all you know is that > requests are certainly not supported, and that MMAP is supported. Whether > USERPTR > or DMABUF are supported isn't known in that case (just try it :-) ). > > Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF caps, but it > is very > easy to add if we create a new capability field anyway, and it has always > annoyed > the hell out of me that we didn't have a good way to let userspace know what > streaming I/O modes we support. And with vb2 it's easy to implement. Yeah, that sounds a bonus to me too. > Regarding point 3: I think this should be documented next to the pixel > format. I.e. > the MPEG-2 Slice format used by the stateless cedrus codec requires the > request API > and that two MPEG-2 controls (slice params and quantization matrices) must be > present > in each request. Makes sense to document with the pixel format... > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed > here. but it sounds worth to also have a flag. > It's really implied by the fact that you use a stateless codec. It doesn't > help > generic applications like v4l2-ctl or qv4l2 either since in order to support > stateless codecs they will have to know about the details of these controls > anyway. Yeah, but they could skip enum those ioctls if they see one marked with V4L2_CTRL_FLAG_REQUIRED_IN_REQ and don't know how to use. Then, default to not use request API. Then, the driver would use a default that would work (even not providing the best possible compression). > So I am inclined to say that it is not necessary to expose this information in > the API, but it has to be documented together with the pixel format > documentation. > > Comments? Ideas? > > Regards, > > Hans Thanks, Mauro
Re: [PATCHv18 19/35] vb2: store userspace data in vb2_v4l2_buffer
On 14/08/18 21:47, Mauro Carvalho Chehab wrote:
> Em Tue, 14 Aug 2018 16:20:31 +0200
> Hans Verkuil escreveu:
>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> index 57848ddc584f..360dc4e7d413 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> @@ -154,17 +154,11 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer
>> *vb)
>> pr_warn("use the actual size instead.\n");
>> }
>>
>> -/*
>> - * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
>> - * v4l2_buffer by the userspace. It also verifies that struct
>> - * v4l2_buffer has a valid number of planes.
>> - */
>> -static int __fill_vb2_buffer(struct vb2_buffer *vb,
>> -const void *pb, struct vb2_plane *planes)
>> +static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct
>> v4l2_buffer *b)
>> {
>> struct vb2_queue *q = vb->vb2_queue;
>> -const struct v4l2_buffer *b = pb;
>> struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +struct vb2_plane *planes = vbuf->planes;
>> unsigned int plane;
>> int ret;
>>
>> @@ -186,7 +180,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>> dprintk(1, "the field is incorrectly set to ALTERNATE for an
>> output buffer\n");
>> return -EINVAL;
>> }
>> -vb->timestamp = 0;
>
> See my note below about this removal. On a quick look, I guess we may have
> a regression here for output buffers (non-m2m).
Note that this is no longer the __fill_vb2_buffer() callback, and the timestamp
is not handled in vb2_fill_vb2_v4l2_buffer(). That's why it is removed here.
It is handled in the new __fill_vb2_buffer function, see below for comments.
>
>> vbuf->sequence = 0;
>>
>> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>> @@ -208,6 +201,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>> }
>> break;
>> default:
>> +for (plane = 0; plane < vb->num_planes; ++plane) {
>> +planes[plane].m.offset =
>> +vb->planes[plane].m.offset;
>> +planes[plane].length =
>> +vb->planes[plane].length;
>> +}
>> break;
>> }
>>
>> @@ -269,9 +268,12 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>> planes[0].length = b->length;
>> break;
>> default:
>> +planes[0].m.offset = vb->planes[0].m.offset;
>> +planes[0].length = vb->planes[0].length;
>> break;
>> }
>>
>> +planes[0].data_offset = 0;
>> if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> if (b->bytesused == 0)
>> vb2_warn_zero_bytesused(vb);
>> @@ -286,7 +288,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>>
>> }
>>
>> -/* Zero flags that the vb2 core handles */
>> +/* Zero flags that we handle */
>> vbuf->flags = b->flags & ~V4L2_BUFFER_MASK_FLAGS;
>> if (!vb->vb2_queue->copy_timestamp || !V4L2_TYPE_IS_OUTPUT(b->type)) {
>> /*
>> @@ -319,6 +321,10 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>> static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer
>> *b,
>> const char *opname)
>> {
>> +struct vb2_v4l2_buffer *vbuf;
>> +struct vb2_buffer *vb;
>> +int ret;
>> +
>> if (b->type != q->type) {
>> dprintk(1, "%s: invalid buffer type\n", opname);
>> return -EINVAL;
>> @@ -340,7 +346,15 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue
>> *q, struct v4l2_buffer *b,
>> return -EINVAL;
>> }
>>
>> -return __verify_planes_array(q->bufs[b->index], b);
>> +vb = q->bufs[b->index];
>> +vbuf = to_vb2_v4l2_buffer(vb);
>> +ret = __verify_planes_array(vb, b);
>> +if (ret)
>> +return ret;
>> +
>> +/* Copy relevant information provided by the userspace */
>> +memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes);
>> +return vb2_fill_vb2_v4l2_buffer(vb, b);
>> }
>>
>> /*
>> @@ -448,6 +462,30 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
>> void *pb)
>> q->last_buffer_dequeued = true;
>> }
>>
>> +/*
>> + * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
>> + * v4l2_buffer by the userspace. It also verifies that struct
>> + * v4l2_buffer has a valid number of planes.
>> + */
>> +static int __fill_vb2_buffer(struct vb2_buffer *vb, struct vb2_plane
>> *planes)
>> +{
>> +struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +unsigned int plane;
>> +
>> +if
Re: [PATCHv18 18/35] videobuf2-v4l2: replace if by switch in __fill_vb2_buffer()
Em Tue, 14 Aug 2018 16:20:30 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Replace 'if' statements by a switch in __fill_vb2_buffer()
> in preparation of the next patch.
>
> No other changes.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> .../media/common/videobuf2/videobuf2-v4l2.c | 21 ---
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 408fd7ce9c09..57848ddc584f 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -190,21 +190,25 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> vbuf->sequence = 0;
>
> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> - if (b->memory == VB2_MEMORY_USERPTR) {
> + switch (b->memory) {
> + case VB2_MEMORY_USERPTR:
> for (plane = 0; plane < vb->num_planes; ++plane) {
> planes[plane].m.userptr =
> b->m.planes[plane].m.userptr;
> planes[plane].length =
> b->m.planes[plane].length;
> }
> - }
> - if (b->memory == VB2_MEMORY_DMABUF) {
> + break;
> + case VB2_MEMORY_DMABUF:
> for (plane = 0; plane < vb->num_planes; ++plane) {
> planes[plane].m.fd =
> b->m.planes[plane].m.fd;
> planes[plane].length =
> b->m.planes[plane].length;
> }
> + break;
> + default:
> + break;
> }
>
> /* Fill in driver-provided information for OUTPUT types */
> @@ -255,14 +259,17 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
>* the driver should use the allow_zero_bytesused flag to keep
>* old userspace applications working.
>*/
> - if (b->memory == VB2_MEMORY_USERPTR) {
> + switch (b->memory) {
> + case VB2_MEMORY_USERPTR:
> planes[0].m.userptr = b->m.userptr;
> planes[0].length = b->length;
> - }
> -
> - if (b->memory == VB2_MEMORY_DMABUF) {
> + break;
> + case VB2_MEMORY_DMABUF:
> planes[0].m.fd = b->m.fd;
> planes[0].length = b->length;
> + break;
> + default:
> + break;
> }
>
> if (V4L2_TYPE_IS_OUTPUT(b->type)) {
Thanks,
Mauro
[PATCH v3 2/2] media: ov5640: Fix timings setup code
From: Jacopo Mondi
As of:
commit 476dec012f4c ("media: ov5640: Add horizontal and vertical totals")
the timings parameters gets programmed separately from the static register
values array.
When changing capture mode, the vertical and horizontal totals gets inspected
by the set_mode_exposure_calc() functions, and only later programmed with the
new values. This means exposure, light banding filter and shutter gain are
calculated using the previous timings, and are thus not correct.
Fix this by programming timings right after the static register value table
has been sent to the sensor in the ov5640_load_regs() function.
Fixes: 476dec012f4c ("media: ov5640: Add horizontal and vertical totals")
Tested-by: Steve Longerbeam
on i.MX6q SabreSD with MIPI CSI-2 OV5640 module
Signed-off-by: Samuel Bobrowicz
Signed-off-by: Maxime Ripard
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/ov5640.c | 50 +++---
1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 7bbd1d7..c81a2a7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -908,6 +908,26 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16
reg,
}
/* download ov5640 settings to sensor through i2c */
+static int ov5640_set_timings(struct ov5640_dev *sensor,
+ const struct ov5640_mode_info *mode)
+{
+ int ret;
+
+ ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, mode->hact);
+ if (ret < 0)
+ return ret;
+
+ ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, mode->vact);
+ if (ret < 0)
+ return ret;
+
+ ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, mode->htot);
+ if (ret < 0)
+ return ret;
+
+ return ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS, mode->vtot);
+}
+
static int ov5640_load_regs(struct ov5640_dev *sensor,
const struct ov5640_mode_info *mode)
{
@@ -935,7 +955,7 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
usleep_range(1000 * delay_ms, 1000 * delay_ms + 100);
}
- return ret;
+ return ov5640_set_timings(sensor, mode);
}
/* read exposure, in number of line periods */
@@ -1391,30 +1411,6 @@ static int ov5640_set_virtual_channel(struct ov5640_dev
*sensor)
return ov5640_write_reg(sensor, OV5640_REG_DEBUG_MODE, temp);
}
-static int ov5640_set_timings(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *mode)
-{
- int ret;
-
- ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, mode->hact);
- if (ret < 0)
- return ret;
-
- ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, mode->vact);
- if (ret < 0)
- return ret;
-
- ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, mode->htot);
- if (ret < 0)
- return ret;
-
- ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS, mode->vtot);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-
static const struct ov5640_mode_info *
ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
int width, int height, bool nearest)
@@ -1658,10 +1654,6 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret < 0)
return ret;
- ret = ov5640_set_timings(sensor, mode);
- if (ret < 0)
- return ret;
-
ret = ov5640_set_binning(sensor, dn_mode != SCALING);
if (ret < 0)
return ret;
--
2.7.4
[PATCH v3 1/2] media: ov5640: Re-work MIPI startup sequence
From: Jacopo Mondi
Rework the MIPI interface startup sequence with the following changes:
- Remove MIPI bus initialization from the initial settings blob
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At s_stream time enable/disable the MIPI interface output.
- Restore default settings at set_power(0) time.
Before this commit the sensor MIPI interface was initialized with settings
that require a start/stop sequence at power-up time in order to force lanes
into LP11 state, as they were initialized in LP00 when in 'sleep mode',
which is assumed to be the sensor manual definition for the D-PHY defined
stop mode.
The stream start/stop was performed by enabling disabling clock gating,
and had the side effect to change the lanes sleep mode configuration when
stream was stopped.
Clock gating/ungating:
- ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
- if (ret)
Set lanes in LP11 when in 'sleep mode':
- ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
- on ? 0x00 : 0x70);
This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that
prevents the host interface from powering up correctly:
https://lkml.org/lkml/2018/6/1/38
It also improves MIPI capture operations stability on my testing platform
where MIPI capture often failed and returned all-purple frames.
fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel interface")
Tested-by: Steve Longerbeam
on i.MX6q SabreSD with MIPI CSI-2 OV5640 module
Reported-by: Jagan Teki
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/ov5640.c | 91 --
1 file changed, 71 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..7bbd1d7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -286,10 +286,10 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0},
{0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0},
- {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
+ {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
{0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
{0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0},
+ {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
{0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
{0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
{0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
@@ -1102,12 +1102,18 @@ static int ov5640_set_stream_mipi(struct ov5640_dev
*sensor, bool on)
{
int ret;
- ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
- if (ret)
- return ret;
- ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
- on ? 0x00 : 0x70);
+ /*
+* Enable/disable the MIPI interface
+*
+* 0x300e = on ? 0x45 : 0x40
+* [7:5] = 001 : 2 data lanes mode
+* [4] = 0 : Power up MIPI HS Tx
+* [3] = 0 : Power up MIPI LS Rx
+* [2] = 1/0: MIPI interface enable/disable
+* [1:0] = 01/00: FIXME: 'debug'
+*/
+ ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
+ on ? 0x45 : 0x40);
if (ret)
return ret;
@@ -1786,23 +1792,68 @@ static int ov5640_set_power(struct ov5640_dev *sensor,
bool on)
if (ret)
goto power_off;
+ /* We're done here for DVP bus, while CSI-2 needs setup. */
+ if (sensor->ep.bus_type != V4L2_MBUS_CSI2)
+ return 0;
+
+ /*
+* Power up MIPI HS Tx and LS Rx; 2 data lanes mode
+*
+* 0x300e = 0x40
+* [7:5] = 001 : 2 data lanes mode
+* [4] = 0 : Power up MIPI HS Tx
+* [3] = 0 : Power up MIPI LS Rx
+* [2] = 0 : MIPI interface disabled
+*/
+ ret = ov5640_write_reg(sensor,
+ OV5640_REG_IO_MIPI_CTRL00, 0x40);
+ if (ret)
+ goto power_off;
+
+ /*
+* Gate clock and set LP11 in 'no packets mode' (idle)
+*
+* 0x4800 = 0x24
+* [5] = 1 : Gate clock when 'no packets'
+* [2] = 1 : MIPI bus in
[PATCH v3 0/2] media: i2c: ov5640: Re-work MIPI startup sequence
Hello ov5640 people, this driver has received a lot of attention recently, and this series aims to fix the CSI-2 interface startup on i.Mx6Q platforms. Please refer to the v2 cover letters for more background informations: https://www.mail-archive.com/linux-media@vger.kernel.org/msg133420.html This two patches alone allows the MIPI interface to startup properly, but in order to capture good images (good as in 'not completely black') exposure and gain handling should be fixed too. Hugues Fruchet has a series in review that fixes that issues: [PATCH v2 0/5] Fix OV5640 exposure & gain And I have re-based it on top of this two fixes here: git://jmondi.org/linux ov5640/timings_exposure Steve Longerbeam tested that branch on his I.MX6q SabreSD board and confirms he can now capture frames (I added his Tested-by tag to this patches). I have verified the same on Engicam iCore I.MX6q and an Intel Atom based board. Ideally I would like to have these two fixes merged, and Hugues' ones then applied on top. Of course, more testing on other platforms using CSI-2 is very welcome. Thanks j v2 -> v3: - patch [2/2] was originally sent in a different series, compared to v2 it removes entries from the blob array instead of adding more. Jacopo Mondi (2): media: ov5640: Re-work MIPI startup sequence media: ov5640: Fix timings setup code drivers/media/i2c/ov5640.c | 141 + 1 file changed, 92 insertions(+), 49 deletions(-) -- 2.7.4
[PATCH 1/2] Remove unnecessary if
Signed-off-by: Sean Young
---
utils/ir-ctl/ir-ctl.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index 59555a13..ddd93068 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -175,19 +175,17 @@ static bool strtoscancode(const char *p, unsigned *ret)
static unsigned parse_emitters(char *p)
{
unsigned emit = 0;
- const char *sep = " ,;:";
+ static const char *sep = " ,;:";
char *saveptr, *q;
q = strtok_r(p, sep, );
while (q) {
- if (*q) {
- char *endptr;
- long e = strtol(q, , 10);
- if ((endptr && *endptr) || e <= 0 || e > 32)
- return 0;
+ char *endptr;
+ long e = strtol(q, , 10);
+ if ((endptr && *endptr) || e <= 0 || e > 32)
+ return 0;
- emit |= 1 << (e - 1);
- }
+ emit |= 1 << (e - 1);
q = strtok_r(NULL, sep, );
}
@@ -200,7 +198,7 @@ static struct file *read_file(struct arguments *args, const
char *fname)
int lineno = 0, lastspace = 0;
char line[1024];
int len = 0;
- const char *whitespace = " \n\r\t";
+ static const char *whitespace = " \n\r\t";
struct file *f;
FILE *input = fopen(fname, "r");
--
2.17.1
[PATCH 2/2] ir-ctl: different drivers have different default timeouts
A driver might not support setting the timeout either, in addition, if a device does not support measuring the carrier, or has no wideband receiver, this command will also produce an error. Signed-off-by: Sean Young --- utils/ir-ctl/ir-ctl.1.in | 4 1 file changed, 4 deletions(-) diff --git a/utils/ir-ctl/ir-ctl.1.in b/utils/ir-ctl/ir-ctl.1.in index f42d8da0..2a148c70 100644 --- a/utils/ir-ctl/ir-ctl.1.in +++ b/utils/ir-ctl/ir-ctl.1.in @@ -224,10 +224,6 @@ To send the pulse and space file \fBplay\fR on emitter 3: To send the rc-5 hauppauge '1' scancode: .br \fBir\-ctl \-S rc5:0x1e01 -.PP -To restore the IR receiver on /dev/lirc2 to the default state: -.br - \fBir\-ctl \-Mn \-\-timeout 125000 \-\-device=/dev/lirc2\fR .SH BUGS Report bugs to \fBLinux Media Mailing List \fR .SH COPYRIGHT -- 2.17.1
Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence
Hi Steve, On Tue, Aug 14, 2018 at 04:53:26PM -0700, Steve Longerbeam wrote: > > > On 08/14/2018 10:38 AM, jacopo mondi wrote: > >Hi Steve, > > > >On Tue, Aug 14, 2018 at 09:51:04AM -0700, Steve Longerbeam wrote: > >>Hi Jacopo, > >> > >> > >>On 08/14/2018 08:35 AM, jacopo mondi wrote: > >>>Hi Steve, > >>>sorry for resurecting this. > >>> > >>> > >I'm sorry I'm not sur I'm following. Does this mean that with that bug > >you are referring to up here fixed by my last patch you have capture > >working? > No, capture still not working for me on SabreSD, even after fixing > the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals", > by either using your patchset, or by running version 476dec0 of ov5640.c > with the call to ov5640_set_timings() moved to the correct places as > described below. > > >>>I've been reported a bug on exposure handling that makes the first > >>>captured frames all black. Both me and Hugues have tried to fix the > >>>issue (him with a more complete series, but that's another topic). > >>>See [1] and [2] > >>> > >>>It might be possible that you're getting blank frames with this series > >>>applied? I never seen them as I'm skipping the first frames when > >>>capturing, but I've now tested and without the exposure fixes (either > >>>[1] or [2]) I actually have blank frames. > >>> > >>>If that's the case for you too (which I hope so much) would you be > >>>available to test again this series with exposure fixes on top? > >>>On my platform that actually makes all frames correct. > >>> > >>>Thanks > >>>j > >>> > >>>[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure > >>>[2] [PATCH v2 0/5] Fix OV5640 exposure & gain > >>> > >>It's not clear to me which patch sets you would like me to test. > >>Just [1] and [2], or [1], [2], and "media: i2c: ov5640: Re-work MIPI startup > >>sequence"? > >> > >I have tested on my board the following: > >v4.18-rc2 + MIPI Fix + Timings + Hugues' exposure fix > > > >Without Hugues' patches I get blank frames (the first ones at least) > >Without MIPI startup reowkr and timings I get the LP-11 error on the > >CSI-2 bus. > > > >As Hugues' series has to be rebased on mine, I have prepared a branch > >here for you if you feel like testing it: > >git://jmondi.org/linux ov5640/timings_exposure > > Hi Jacopo, that branch works on SabreSD! > YEAH! I'm so happy about this (and not to having asked you to test yet-another-broken-patchset for no reason :) > Feel free to add > > Tested-by: Steve Longerbeam > on i.MX6q SabreSD with MIPI CSI-2 OV5640 module > > to whichever ov5640 patches are appropriate. Great, I'll send a v3 now collecting your tags and the most recent version of the timings fix that was not included in v2 (but in the branch you have tested). My ideal plan would be to have these two patches merged, then Hugues' series to fix exposure handling merged on top. This would, in my mind make capture on CSI-2 work properly. Of course, more testing is very welcome. Thanks again j > > Steve > > signature.asc Description: PGP signature
Re: [PATCHv17 01/34] Documentation: v4l: document request API
Hi! > Document the request API for V4L2 devices, and amend the documentation > of system calls influenced by it. > > Signed-off-by: Alexandre Courbot > Signed-off-by: Hans Verkuil Cc documentation people? > +Synopsis > + > + > +.. c:function:: int ioctl( int request_fd, MEDIA_REQUEST_IOC_QUEUE ) > +:name: MEDIA_REQUEST_IOC_QUEUE > + > + > +Arguments > += > + > +``request_fd`` > +File descriptor returned by :ref:`MEDIA_IOC_REQUEST_ALLOC`. > + > + > +Description > +=== > + > +If the media device supports :ref:`requests `, then > +this request ioctl can be used to queue a previously allocated request. > + > +If the request was successfully queued, then the file descriptor can be > +:ref:`polled ` to wait for the request to complete. > + > +If the request was already queued before, then ``EBUSY`` is returned. I'd expect -1 to be returned and errno set to EBUSY? > + > + > +On success 0 is returned, on error -1 and the ``errno`` variable is set > +appropriately. The generic error codes are described at the > +:ref:`Generic Error Codes ` chapter. > + > +EBUSY > +The request was already queued. > +EPERM > +The application queued the first buffer directly, but later attempted > +to use a request. It is not permitted to mix the two APIs. > +ENOENT > +The request did not contain any buffers. All requests are required > +to have at least one buffer. This can also be returned if required > +controls are missing. > +ENOMEM > +Out of memory when allocating internal data structures for this > +request. > +EINVAL > +The request has invalid data. > +EIO > +The hardware is in a bad state. To recover, the application needs to > +stop streaming to reset the hardware state and then try to restart > +streaming. Pavel
