cron job: media_tree daily build: OK
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 Oct 11 05:00:11 CEST 2018 media-tree git hash:8caec72e8cbff65afa38928197bea5a393b67975 media_build git hash: 9f419c414672676f63e85a61ea99df0ddcd6e9a7 v4l-utils git hash: 9d7d01f24b5e8ac73fbed783cffd5c0f5f6e8a87 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.11-marune 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-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.123-i686: OK linux-3.18.123-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.159-i686: OK linux-4.4.159-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.131-i686: OK linux-4.9.131-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19-rc6-i686: OK linux-4.19-rc6-x86_64: OK apps: OK 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: [PATCH v2 0/2] Add SECO Boards CEC device driver
On Wed, Oct 10, 2018 at 03:45:35PM +0200, Hans Verkuil wrote: > On 10/10/18 14:09, ektor5 wrote: > > Hi Hans, > > > > On Sat, Oct 06, 2018 at 11:54:38AM +0200, Hans Verkuil wrote: > >> Hi Ettore, > >> > >> On 10/05/2018 07:33 PM, ektor5 wrote: > >>> This series of patches aims to add CEC functionalities to SECO > >>> devices, in particular UDOO X86. > >>> > >>> The communication is achieved via Braswell SMBus (i2c-i801) to the > >>> onboard STM32 microcontroller that handles the CEC signals. The driver > >>> use direct access to the PCI addresses, due to the limitations of the > >>> specific driver in presence of ACPI calls. > >>> > >>> The basic functionalities are tested with success with cec-ctl and > >>> cec-compliance. > >> > >> This series looks good to me. But can you do one more test: > >> > >> Update your kernel to the latest media_tree master and also update your > >> v4l-utils repo to the latest master code. > >> > >> With all that in place please run: > >> > >> cec-compliance -A > >> > >> (have the HDMI output connected to a CEC-capable TV when running this > >> test). > >> > >> Please report back the output of cec-compliance. > >> > >> A bunch of CEC bug fixes and improvements were merged yesterday, and the > >> cec-compliance adapter test is improved to check for issues that were hard > >> to find in the past. > >> > >> So it will be good to have a final check of this driver. > > > > Here it is, compiled media-tree and latest v4l-utils: > > > > udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ uname -a > > Linux udoo-UDOO-x86 4.19.0-041900rc7-generic #201810071631+cec SMP Tue Oct > > 9 17:36:11 CEST 2018 x86_64 x86_64 x86_64 GNU/Linux > > udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ ./cec-compliance -A > > cec-compliance SHA : > > 06ad469e966aafaf39c1cc76e6e0953ec7d4f9c9 > > Driver Info: > > Driver Name: secocec > > Adapter Name : CEC1:00 > > Capabilities : 0x000e > > Logical Addresses > > Transmit > > Passthrough > > Driver version : 4.19.0 > > Available Logical Addresses: 1 > > Physical Address : 3.0.0.0 > > Logical Address Mask : 0x0010 > > CEC Version: 2.0 > > Vendor ID : 0x000c03 (HDMI) > > OSD Name : Playback > > Logical Addresses : 1 > > > > Logical Address : 4 (Playback Device 1) > > Primary Device Type: Playback > > Logical Address Type : Playback > > All Device Types : Playback > > RC TV Profile : None > > Device Features: > > None > > > > Compliance test for device /dev/cec0: > > > > The test results mean the following: > > OK Supported correctly by the device. > > OK (Not Supported) Not supported and not mandatory for the device. > > OK (Presumed) Presumably supported. Manually check to > > confirm. > > OK (Unexpected) Supported correctly but is not expected to be > > supported for this device. > > OK (Refused)Supported by the device, but was refused. > > FAILFailed and was expected to be supported by this > > device. > > > > Find remote devices: > > Polling: OK > > > > CEC API: > > CEC_ADAP_G_CAPS: OK > > CEC_DQEVENT: OK > > CEC_ADAP_G/S_PHYS_ADDR: OK > > CEC_ADAP_G/S_LOG_ADDRS: OK > > CEC_TRANSMIT: OK > > CEC_RECEIVE: OK > > CEC_TRANSMIT/RECEIVE (non-blocking): OK (Presumed) > > CEC_G/S_MODE: OK > > fail: cec-test-adapter.cpp(1042): There were 142 pending > > messages for 83 transmitted messages > > That's not good. > > If you look in the kernel log, do you see 'timed out' cec messages? No, actually I see a lot of 'cec_transmit_msg_fh: transmit queue full' messages. > > Note: that message is a warning since commit > 7ec2b3b941a666a942859684281b5f6460a0c234. > Before that you first need to enable debugging: > > echo 1 >/sys/module/cec/parameters/debug > > My guess is that this might be a fw bug. Does the firmware handle Signal Free > Time > correctly? My guess is that it doesn't do that and that this test causes what > is > effectively a 'denial of service' situation: the transmitter gets blocked > waiting > for sufficient signal free time. > > I have updated cec-compliance to give better information about what was > received, > so can you update cec-compliance and run again? Yes, I've recompiled the two, in attachment there are both outputs plus the relative kernel messages. > > Also, run 'cec-ctl -m >cec.log' at the same time and mail me that log. > > There are two types of CEC adapters: those that handle retransmits > automatically > (and they determine the Signal Free Time themselves) and those that don't do > automatic retransmits, and there you normally
Re: [PATCH v2 0/2] Add SECO Boards CEC device driver
On 10/10/18 14:09, ektor5 wrote: > Hi Hans, > > On Sat, Oct 06, 2018 at 11:54:38AM +0200, Hans Verkuil wrote: >> Hi Ettore, >> >> On 10/05/2018 07:33 PM, ektor5 wrote: >>> This series of patches aims to add CEC functionalities to SECO >>> devices, in particular UDOO X86. >>> >>> The communication is achieved via Braswell SMBus (i2c-i801) to the >>> onboard STM32 microcontroller that handles the CEC signals. The driver >>> use direct access to the PCI addresses, due to the limitations of the >>> specific driver in presence of ACPI calls. >>> >>> The basic functionalities are tested with success with cec-ctl and >>> cec-compliance. >> >> This series looks good to me. But can you do one more test: >> >> Update your kernel to the latest media_tree master and also update your >> v4l-utils repo to the latest master code. >> >> With all that in place please run: >> >> cec-compliance -A >> >> (have the HDMI output connected to a CEC-capable TV when running this test). >> >> Please report back the output of cec-compliance. >> >> A bunch of CEC bug fixes and improvements were merged yesterday, and the >> cec-compliance adapter test is improved to check for issues that were hard >> to find in the past. >> >> So it will be good to have a final check of this driver. > > Here it is, compiled media-tree and latest v4l-utils: > > udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ uname -a > Linux udoo-UDOO-x86 4.19.0-041900rc7-generic #201810071631+cec SMP Tue Oct 9 > 17:36:11 CEST 2018 x86_64 x86_64 x86_64 GNU/Linux > udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ ./cec-compliance -A > cec-compliance SHA : 06ad469e966aafaf39c1cc76e6e0953ec7d4f9c9 > Driver Info: > Driver Name: secocec > Adapter Name : CEC1:00 > Capabilities : 0x000e > Logical Addresses > Transmit > Passthrough > Driver version : 4.19.0 > Available Logical Addresses: 1 > Physical Address : 3.0.0.0 > Logical Address Mask : 0x0010 > CEC Version: 2.0 > Vendor ID : 0x000c03 (HDMI) > OSD Name : Playback > Logical Addresses : 1 > > Logical Address : 4 (Playback Device 1) > Primary Device Type: Playback > Logical Address Type : Playback > All Device Types : Playback > RC TV Profile : None > Device Features: > None > > Compliance test for device /dev/cec0: > > The test results mean the following: > OK Supported correctly by the device. > OK (Not Supported) Not supported and not mandatory for the device. > OK (Presumed) Presumably supported. Manually check to confirm. > OK (Unexpected) Supported correctly but is not expected to be > supported for this device. > OK (Refused)Supported by the device, but was refused. > FAILFailed and was expected to be supported by this > device. > > Find remote devices: > Polling: OK > > CEC API: > CEC_ADAP_G_CAPS: OK > CEC_DQEVENT: OK > CEC_ADAP_G/S_PHYS_ADDR: OK > CEC_ADAP_G/S_LOG_ADDRS: OK > CEC_TRANSMIT: OK > CEC_RECEIVE: OK > CEC_TRANSMIT/RECEIVE (non-blocking): OK (Presumed) > CEC_G/S_MODE: OK > fail: cec-test-adapter.cpp(1042): There were 142 pending > messages for 83 transmitted messages That's not good. If you look in the kernel log, do you see 'timed out' cec messages? Note: that message is a warning since commit 7ec2b3b941a666a942859684281b5f6460a0c234. Before that you first need to enable debugging: echo 1 >/sys/module/cec/parameters/debug My guess is that this might be a fw bug. Does the firmware handle Signal Free Time correctly? My guess is that it doesn't do that and that this test causes what is effectively a 'denial of service' situation: the transmitter gets blocked waiting for sufficient signal free time. I have updated cec-compliance to give better information about what was received, so can you update cec-compliance and run again? Also, run 'cec-ctl -m >cec.log' at the same time and mail me that log. There are two types of CEC adapters: those that handle retransmits automatically (and they determine the Signal Free Time themselves) and those that don't do automatic retransmits, and there you normally need to program the Signal Free Time before starting the transmit. This driver falls in the second category, but the SFT isn't set anywhere. This particular adapter test actually tests this, and I have seen this symptom before if the SFT wasn't set correctly. Regards, Hans > CEC_EVENT_LOST_MSGS: FAIL > > Network topology: > System Information for device 0 (TV) from device 4 (Playback Device 1): > CEC
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Jacopo,
On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
> Hi Sam,
>thanks for the patch, I see the same issue you reported, but I
> think this patch can be improved.
>
> (expanding the Cc list to all people involved in recent ov5640
> developemts, not just for this patch, but for the whole series to look
> at. Copying names from another series cover letter, hope it is
> complete.)
>
> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> > set_fmt was not properly triggering a mode change when
> > a new mode was set that happened to have the same format
> > as the previous mode (for example, when only changing the
> > frame dimensions). Fix this.
> >
> > Signed-off-by: Sam Bobrowicz
> > ---
> >
> > drivers/media/i2c/ov5640.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index eaefdb5..5031aab 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> >
> > goto out;
> >
> > }
> >
> > - if (new_mode != sensor->current_mode) {
> > +
> > + if (new_mode != sensor->current_mode ||
> > + mbus_fmt->code != sensor->fmt.code) {
> > + sensor->fmt = *mbus_fmt;
> >
> > sensor->current_mode = new_mode;
> > sensor->pending_mode_change = true;
> >
> > - }
> > - if (mbus_fmt->code != sensor->fmt.code) {
> > - sensor->fmt = *mbus_fmt;
> >
> > sensor->pending_fmt_change = true;
> >
> > }
>
> How I did reproduce the issue:
>
> # Set 1024x768 on ov5640 without changing the image format
> # (default image size at startup is 640x480)
> $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
> sensor->pending_mode_change = true; //verified this flag gets set
>
> # Start streaming, after having configured the whole pipeline to work
> # with 1024x768
> $ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
>Unable to start streaming: Broken pipe (32).
>
> # Inspect which part of pipeline validation went wrong
> # Turns out the sensor->fmt field is not updated, and when get_fmt()
> # is called, the old one is returned.
> $ media-ctl -e "ov5640 2-003c" -p
> ...
> [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ycbcr:601
> quantization:full-range] ^^^ ^^^
>
> So yes, sensor->fmt is not udapted as it should be when only image
> resolution is changed.
>
> Although I still see value in having two separate flags for the
> 'mode_change' (which in ov5640 lingo is resolution) and 'fmt_change' (which
> in ov5640 lingo is the image format), and write their configuration to
> registers only when they get actually changed.
>
> For this reasons I would like to propse the following patch which I
> have tested by:
> 1) changing resolution only
> 2) changing format only
> 3) change both
>
> What do you and others think?
I think that the format setting code should be completely rewritten, it's
pretty much unmaintainable as-is.
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..e392b9d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2020,6 +2020,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> const struct ov5640_mode_info *new_mode;
> struct v4l2_mbus_framefmt *mbus_fmt = >format;
> + struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> if (format->pad != 0)
> @@ -2037,22 +2038,19 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> if (ret)
> goto out;
>
> - if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> - struct v4l2_mbus_framefmt *fmt =
> - v4l2_subdev_get_try_format(sd, cfg, 0);
> + if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> + fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> + else
> + fmt = >fmt;
>
> - *fmt = *mbus_fmt;
> - goto out;
> - }
> + *fmt = *mbus_fmt;
>
> if (new_mode != sensor->current_mode) {
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> }
> - if (mbus_fmt->code != sensor->fmt.code) {
> - sensor->fmt = *mbus_fmt;
> + if (mbus_fmt->code != sensor->fmt.code)
> sensor->pending_fmt_change = true;
> - }
> out:
> mutex_unlock(>lock);
> return ret;
>
> > out:
> > --
> > 2.7.4
--
Regards,
Laurent Pinchart
Re: [PATCH v2 0/2] Add SECO Boards CEC device driver
Hi Hans, On Sat, Oct 06, 2018 at 11:54:38AM +0200, Hans Verkuil wrote: > Hi Ettore, > > On 10/05/2018 07:33 PM, ektor5 wrote: > > This series of patches aims to add CEC functionalities to SECO > > devices, in particular UDOO X86. > > > > The communication is achieved via Braswell SMBus (i2c-i801) to the > > onboard STM32 microcontroller that handles the CEC signals. The driver > > use direct access to the PCI addresses, due to the limitations of the > > specific driver in presence of ACPI calls. > > > > The basic functionalities are tested with success with cec-ctl and > > cec-compliance. > > This series looks good to me. But can you do one more test: > > Update your kernel to the latest media_tree master and also update your > v4l-utils repo to the latest master code. > > With all that in place please run: > > cec-compliance -A > > (have the HDMI output connected to a CEC-capable TV when running this test). > > Please report back the output of cec-compliance. > > A bunch of CEC bug fixes and improvements were merged yesterday, and the > cec-compliance adapter test is improved to check for issues that were hard > to find in the past. > > So it will be good to have a final check of this driver. Here it is, compiled media-tree and latest v4l-utils: udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ uname -a Linux udoo-UDOO-x86 4.19.0-041900rc7-generic #201810071631+cec SMP Tue Oct 9 17:36:11 CEST 2018 x86_64 x86_64 x86_64 GNU/Linux udoo@udoo-UDOO-x86:~/v4l-utils/utils/cec-compliance$ ./cec-compliance -A cec-compliance SHA : 06ad469e966aafaf39c1cc76e6e0953ec7d4f9c9 Driver Info: Driver Name: secocec Adapter Name : CEC1:00 Capabilities : 0x000e Logical Addresses Transmit Passthrough Driver version : 4.19.0 Available Logical Addresses: 1 Physical Address : 3.0.0.0 Logical Address Mask : 0x0010 CEC Version: 2.0 Vendor ID : 0x000c03 (HDMI) OSD Name : Playback Logical Addresses : 1 Logical Address : 4 (Playback Device 1) Primary Device Type: Playback Logical Address Type : Playback All Device Types : Playback RC TV Profile : None Device Features: None Compliance test for device /dev/cec0: The test results mean the following: OK Supported correctly by the device. OK (Not Supported) Not supported and not mandatory for the device. OK (Presumed) Presumably supported. Manually check to confirm. OK (Unexpected) Supported correctly but is not expected to be supported for this device. OK (Refused)Supported by the device, but was refused. FAILFailed and was expected to be supported by this device. Find remote devices: Polling: OK CEC API: CEC_ADAP_G_CAPS: OK CEC_DQEVENT: OK CEC_ADAP_G/S_PHYS_ADDR: OK CEC_ADAP_G/S_LOG_ADDRS: OK CEC_TRANSMIT: OK CEC_RECEIVE: OK CEC_TRANSMIT/RECEIVE (non-blocking): OK (Presumed) CEC_G/S_MODE: OK fail: cec-test-adapter.cpp(1042): There were 142 pending messages for 83 transmitted messages CEC_EVENT_LOST_MSGS: FAIL Network topology: System Information for device 0 (TV) from device 4 (Playback Device 1): CEC Version: 1.4 Physical Address : 0.0.0.0 Primary Device Type: TV Vendor ID : 0x00e091 OSD Name : Tx, OK, Rx, Timeout Menu Language : kor Power Status : On Total: 10, Succeeded: 9, Failed: 1, Warnings: 0 Thanks, Ettore > > Regards, > > Hans > > > > > v2: > > - Removed useless debug prints > > - Added DMI && PCI to dependences > > - Removed useless ifdefs > > - Renamed all irda references to ir > > - Fixed SPDX clause > > - Several style fixes > > > > Ettore Chimenti (2): > > media: add SECO cec driver > > seco-cec: add Consumer-IR support > > > > MAINTAINERS| 6 + > > drivers/media/platform/Kconfig | 22 + > > drivers/media/platform/Makefile| 2 + > > drivers/media/platform/seco-cec/Makefile | 1 + > > drivers/media/platform/seco-cec/seco-cec.c | 829 + > > drivers/media/platform/seco-cec/seco-cec.h | 141 > > 6 files changed, 1001 insertions(+) > > create mode 100644 drivers/media/platform/seco-cec/Makefile > > create mode 100644 drivers/media/platform/seco-cec/seco-cec.c > > create mode 100644
[PATCH] cec: check for non-OK/NACK conditions while claiming a LA
During the configuration phase of a CEC adapter it is trying to claim a
free logical address by polling.
However, the code doesn't check if there were errors other than OK or NACK,
those are just treated as if the poll was NACKed.
Instead check for such errors and retry the poll. And if the problem persists
then don't claim this LA since there is something weird going on.
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 0c0d9107383e..7b7139991445 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -1178,6 +1178,8 @@ static int cec_config_log_addr(struct cec_adapter *adap,
{
struct cec_log_addrs *las = >log_addrs;
struct cec_msg msg = { };
+ const unsigned int max_retries = 2;
+ unsigned int i;
int err;
if (cec_has_log_addr(adap, log_addr))
@@ -1186,19 +1188,44 @@ static int cec_config_log_addr(struct cec_adapter *adap,
/* Send poll message */
msg.len = 1;
msg.msg[0] = (log_addr << 4) | log_addr;
- err = cec_transmit_msg_fh(adap, , NULL, true);
- /*
-* While trying to poll the physical address was reset
-* and the adapter was unconfigured, so bail out.
-*/
- if (!adap->is_configuring)
- return -EINTR;
+ for (i = 0; i < max_retries; i++) {
+ err = cec_transmit_msg_fh(adap, , NULL, true);
- if (err)
- return err;
+ /*
+* While trying to poll the physical address was reset
+* and the adapter was unconfigured, so bail out.
+*/
+ if (!adap->is_configuring)
+ return -EINTR;
+
+ if (err)
+ return err;
- if (msg.tx_status & CEC_TX_STATUS_OK)
+ /*
+* The message was aborted due to a disconnect or
+* unconfigure, just bail out.
+*/
+ if (msg.tx_status & CEC_TX_STATUS_ABORTED)
+ return -EINTR;
+ if (msg.tx_status & CEC_TX_STATUS_OK)
+ return 0;
+ if (msg.tx_status & CEC_TX_STATUS_NACK)
+ break;
+ /*
+* Retry up to max_retries times if the message was neither
+* OKed or NACKed. This can happen due to e.g. a Lost
+* Arbitration condition.
+*/
+ }
+
+ /*
+* If we are unable to get an OK or a NACK after max_retries attempts
+* (and note that each attempt already consists of four polls), then
+* then we assume that something is really weird and that it is not a
+* good idea to try and claim this logical address.
+*/
+ if (i == max_retries)
return 0;
/*
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Sam,
thanks for the patch, I see the same issue you reported, but I
think this patch can be improved.
(expanding the Cc list to all people involved in recent ov5640
developemts, not just for this patch, but for the whole series to look
at. Copying names from another series cover letter, hope it is
complete.)
On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> set_fmt was not properly triggering a mode change when
> a new mode was set that happened to have the same format
> as the previous mode (for example, when only changing the
> frame dimensions). Fix this.
>
> Signed-off-by: Sam Bobrowicz
> ---
> drivers/media/i2c/ov5640.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..5031aab 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> goto out;
> }
>
> - if (new_mode != sensor->current_mode) {
> +
> + if (new_mode != sensor->current_mode ||
> + mbus_fmt->code != sensor->fmt.code) {
> + sensor->fmt = *mbus_fmt;
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> - }
> - if (mbus_fmt->code != sensor->fmt.code) {
> - sensor->fmt = *mbus_fmt;
> sensor->pending_fmt_change = true;
> }
How I did reproduce the issue:
# Set 1024x768 on ov5640 without changing the image format
# (default image size at startup is 640x480)
$ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
sensor->pending_mode_change = true; //verified this flag gets set
# Start streaming, after having configured the whole pipeline to work
# with 1024x768
$ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
Unable to start streaming: Broken pipe (32).
# Inspect which part of pipeline validation went wrong
# Turns out the sensor->fmt field is not updated, and when get_fmt()
# is called, the old one is returned.
$ media-ctl -e "ov5640 2-003c" -p
...
[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ycbcr:601
quantization:full-range]
^^^ ^^^
So yes, sensor->fmt is not udapted as it should be when only image
resolution is changed.
Although I still see value in having two separate flags for the
'mode_change' (which in ov5640 lingo is resolution) and 'fmt_change' (which
in ov5640 lingo is the image format), and write their configuration to
registers only when they get actually changed.
For this reasons I would like to propse the following patch which I
have tested by:
1) changing resolution only
2) changing format only
3) change both
What do you and others think?
Thanks
j
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index eaefdb5..e392b9d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2020,6 +2020,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *new_mode;
struct v4l2_mbus_framefmt *mbus_fmt = >format;
+ struct v4l2_mbus_framefmt *fmt;
int ret;
if (format->pad != 0)
@@ -2037,22 +2038,19 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
if (ret)
goto out;
- if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- struct v4l2_mbus_framefmt *fmt =
- v4l2_subdev_get_try_format(sd, cfg, 0);
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+ fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+ else
+ fmt = >fmt;
- *fmt = *mbus_fmt;
- goto out;
- }
+ *fmt = *mbus_fmt;
if (new_mode != sensor->current_mode) {
sensor->current_mode = new_mode;
sensor->pending_mode_change = true;
}
- if (mbus_fmt->code != sensor->fmt.code) {
- sensor->fmt = *mbus_fmt;
+ if (mbus_fmt->code != sensor->fmt.code)
sensor->pending_fmt_change = true;
- }
out:
mutex_unlock(>lock);
return ret;
> out:
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH] vicodec: lower minimum height to 360
Hi Hans, Thank you for the patch, On 10/10/18 08:03, Hans Verkuil wrote: > Lower the minimum height to 360 to be consistent with the webcam input of > vivid. > > The 480 was rather arbitrary but it made it harder to use vivid as a source > for > encoding since the default resolution when you load vivid is 640x360. As this is a virtual codec, is the minimum width and height really so 'large' ? What about 320x240 or such? (or even 32x32...) Or is the aim to provide minimum frame sizes and a means to verify userspace correctly handles the minimum frame sizes too ? I could certainly acknowledge it's worth providing a means for a userspace app to test that it handles minimum sizes correctly. > Signed-off-by: Hans Verkuil If the minimum is desired: Reviewed-by: Kieran Bingham > --- > diff --git a/drivers/media/platform/vicodec/vicodec-core.c > b/drivers/media/platform/vicodec/vicodec-core.c > index 1eb9132bfc85..b292cff26c86 100644 > --- a/drivers/media/platform/vicodec/vicodec-core.c > +++ b/drivers/media/platform/vicodec/vicodec-core.c > @@ -42,7 +42,7 @@ MODULE_PARM_DESC(debug, " activates debug info"); > #define MAX_WIDTH4096U > #define MIN_WIDTH640U > #define MAX_HEIGHT 2160U > -#define MIN_HEIGHT 480U > +#define MIN_HEIGHT 360U > > #define dprintk(dev, fmt, arg...) \ > v4l2_dbg(1, debug, >v4l2_dev, "%s: " fmt, __func__, ## arg) >
Re: [PATCH 2/4] media: ov5640: fix get_light_freq on auto
Hi Sam,
On Mon, Oct 08, 2018 at 11:48:00PM -0700, Sam Bobrowicz wrote:
> Light frequency was not properly returned when in auto
> mode and the detected frequency was 60Hz. Fix this.
>
> Signed-off-by: Sam Bobrowicz
This is indeed a bugfix
Acked-by: Jacopo Mondi
Thanks
j
> ---
> drivers/media/i2c/ov5640.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 5031aab..f183222 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1295,6 +1295,7 @@ static int ov5640_get_light_freq(struct ov5640_dev
> *sensor)
> light_freq = 50;
> } else {
> /* 60Hz */
> + light_freq = 60;
> }
> }
>
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH 3/4] media: ov5640: Don't access ctrl regs when off
Hi Sam,
thanks for the patch.
On Mon, Oct 08, 2018 at 11:48:01PM -0700, Sam Bobrowicz wrote:
> Add a check to g_volatile_ctrl to prevent trying to read
> registers when the sensor is not powered.
>
> Signed-off-by: Sam Bobrowicz
I've been carrying a similar patch in my tree. I found it is required
when the sensor control handler is add to the receiver driver control
handler, and thus g_voltaile_ctrl can be called when the sensor is
powered off.
Please add my:
Acked-by: Jacopo Mondi
Thanks
j
> ---
> drivers/media/i2c/ov5640.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index f183222..a50d884 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2336,6 +2336,13 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl
> *ctrl)
>
> /* v4l2_ctrl_lock() locks our own mutex */
>
> + /*
> + * If the sensor is not powered up by the host driver, do
> + * not try to access it to update the volatile controls.
> + */
> + if (sensor->power_count == 0)
> + return 0;
> +
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> val = ov5640_get_gain(sensor);
> --
> 2.7.4
>
signature.asc
Description: PGP signature
[PATCH 1/2] ipu3-cio2: Unregister device nodes first, then release resources
While there are issues related to object lifetime management, unregister the media device first, followed immediately by other device nodes when the driver is being unbound. Only then the resources needed by the driver may be released. This is slightly safer. Signed-off-by: Sakari Ailus --- drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index 452eb9b42140..723022ef3662 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -1846,12 +1846,12 @@ static void cio2_pci_remove(struct pci_dev *pci_dev) struct cio2_device *cio2 = pci_get_drvdata(pci_dev); unsigned int i; + media_device_unregister(>media_dev); cio2_notifier_exit(cio2); - cio2_fbpt_exit_dummy(cio2); for (i = 0; i < CIO2_QUEUES; i++) cio2_queue_exit(cio2, >queue[i]); + cio2_fbpt_exit_dummy(cio2); v4l2_device_unregister(>v4l2_dev); - media_device_unregister(>media_dev); media_device_cleanup(>media_dev); mutex_destroy(>lock); } -- 2.11.0
[PATCH 2/2] ipu3-cio2: Use cio2_queues_exit
The ipu3-cio2 driver has a function to tear down video devices as well as
the associated video buffer queues. Use it.
Signed-off-by: Sakari Ailus
---
drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 723022ef3662..447baaebca44 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1844,12 +1844,10 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
static void cio2_pci_remove(struct pci_dev *pci_dev)
{
struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
- unsigned int i;
media_device_unregister(>media_dev);
cio2_notifier_exit(cio2);
- for (i = 0; i < CIO2_QUEUES; i++)
- cio2_queue_exit(cio2, >queue[i]);
+ cio2_queues_exit(cio2);
cio2_fbpt_exit_dummy(cio2);
v4l2_device_unregister(>v4l2_dev);
media_device_cleanup(>media_dev);
--
2.11.0
[PATCH 0/2] Trivial CIO2 patches
Hi, Here are two simple cio2 patches. Slightly safer module removal plus a cleanup. Sakari Ailus (2): ipu3-cio2: Unregister device nodes first, then release resources ipu3-cio2: Use cio2_queues_exit drivers/media/pci/intel/ipu3/ipu3-cio2.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) -- 2.11.0
[PATCH] vicodec: lower minimum height to 360
Lower the minimum height to 360 to be consistent with the webcam input of vivid. The 480 was rather arbitrary but it made it harder to use vivid as a source for encoding since the default resolution when you load vivid is 640x360. Signed-off-by: Hans Verkuil --- diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c index 1eb9132bfc85..b292cff26c86 100644 --- a/drivers/media/platform/vicodec/vicodec-core.c +++ b/drivers/media/platform/vicodec/vicodec-core.c @@ -42,7 +42,7 @@ MODULE_PARM_DESC(debug, " activates debug info"); #define MAX_WIDTH 4096U #define MIN_WIDTH 640U #define MAX_HEIGHT 2160U -#define MIN_HEIGHT 480U +#define MIN_HEIGHT 360U #define dprintk(dev, fmt, arg...) \ v4l2_dbg(1, debug, >v4l2_dev, "%s: " fmt, __func__, ## arg)
Re: [RFC] Informal meeting during ELCE to discuss userspace support for stateless codecs
On 10/08/2018 01:53 PM, Hans Verkuil wrote: > Hi all, > > I would like to meet up somewhere during the ELCE to discuss userspace support > for stateless (and perhaps stateful as well?) codecs. > > It is also planned as a topic during the summit, but I would prefer to prepare > for that in advance, esp. since I myself do not have any experience writing > userspace SW for such devices. > > Nicolas, it would be really great if you can participate in this meeting > since you probably have the most experience with this by far. > > Looking through the ELCE program I found two timeslots that are likely to work > for most of us (because the topics in the program appear to be boring for us > media types!): > > Tuesday from 10:50-15:50 Let's do this Tuesday. Let's meet at the Linux Foundation Registration Desk at 11:00. I'll try to figure out where we can sit the day before. Please check your email Tuesday morning for any last minute changes. Tomasz, it would be nice indeed if we can get you and Paul in as well using Hangouts on my laptop. I would very much appreciate it if those who have experience with the userspace support think about this beforehand and make some requirements list of what you would like to see. Regards, Hans > > or: > > Monday from 15:45 onward > > My guess is that we need 2-3 hours or so. Hard to predict. > > The basic question that I would like to have answered is what the userspace > component should look like? libv4l-like plugin or a library that userspace can > link with? Do we want more general support for stateful codecs as well that > deals > with resolution changes and the more complex parts of the codec API? > > I've mailed this directly to those that I expect are most interested in this, > but if someone want to join in let me know. > > I want to keep the group small though, so you need to bring relevant > experience > to the table. > > Regards, > > Hans >
