Re: Regression: DVBSky S960 USB tuner doesn't work in 4.10 or newer
I did test the patch and while it doesn't seem to introduce any
negative side effects it does not provide a remedy for the original
problem that was seen after introducing
9d659ae14b545c4296e812c70493bfdc999b5c1c (you probably did not expect
that either).
Cheers,
-olli
On 27 April 2018 at 18:33, Mauro Carvalho Chehab
wrote:
> Em Fri, 27 Apr 2018 16:25:08 +0200
> Olli Salonen escreveu:
>
>> Thanks for the suggestion Antti.
>>
>> I've tried to add a delay in various places, but haven't seen any
>> improvement. However, what I did saw was that if I added an msleep
>> after the lock:
>>
>> static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
>> u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen)
>> {
>> int ret;
>> struct dvbsky_state *state = d_to_priv(d);
>>
>> mutex_lock(&d->usb_mutex);
>> msleep(20);
>>
>> The error was seen very within a minute. If I increased the msleep to
>> 50, it failed within seconds. This doesn't seem to make sense to me.
>> This is the only function where usb_mutex is used. If the mutex is
>> held for a longer time, the next attempt to lock the mutex should just
>> be delayed a bit, no?
>
> I don't like the idea of having two mutexes there to protect reading/writing
> to data one for "generic" r/w ops, and another one just for streaming
> control, with ends by calling the "generic" mutex.
>
> IMHO, I would get rid of one of the mutexes, e. g. something like the
> patch below (untested).
>
> Regards,
> Mauro
>
> media: dvbsky: use just one mutex for serializing device R/W ops
>
> Right now, there are two mutexes serializing r/w ops: one "generic"
> and another one specifically for stream on/off.
>
> Clean it a little bit, getting rid of one of the mutexes.
>
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c
> b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> index 43eb82884555..50553975c39d 100644
> --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
> +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> @@ -31,7 +31,6 @@ MODULE_PARM_DESC(disable_rc, "Disable inbuilt IR
> receiver.");
> DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
>
> struct dvbsky_state {
> - struct mutex stream_mutex;
> u8 ibuf[DVBSKY_BUF_LEN];
> u8 obuf[DVBSKY_BUF_LEN];
> u8 last_lock;
> @@ -68,18 +67,17 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
>
> static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
> {
> - struct dvbsky_state *state = d_to_priv(d);
> int ret;
> - u8 obuf_pre[3] = { 0x37, 0, 0 };
> - u8 obuf_post[3] = { 0x36, 3, 0 };
> + static u8 obuf_pre[3] = { 0x37, 0, 0 };
> + static u8 obuf_post[3] = { 0x36, 3, 0 };
>
> - mutex_lock(&state->stream_mutex);
> - ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
> + mutex_lock(&d->usb_mutex);
> + ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
> if (!ret && onoff) {
> msleep(20);
> - ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
> + ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
> }
> - mutex_unlock(&state->stream_mutex);
> + mutex_unlock(&d->usb_mutex);
> return ret;
> }
>
> @@ -744,8 +742,6 @@ static int dvbsky_init(struct dvb_usb_device *d)
> if (ret)
> return ret;
> */
> - mutex_init(&state->stream_mutex);
> -
> state->last_lock = 0;
>
> return 0;
>
>
>
> Thanks,
> Mauro
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: Sat Apr 28 05:00:11 CEST 2018 media-tree git hash:a2b2eff6ac2716f499defa590a6ec4ba379d765e media_build git hash: 2945d108c680b3c09c9843e001e84a9797d7f379 v4l-utils git hash: 03e763fd4b361b2082019032fc315b7606669335 gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: 0.5.2-RC1 smatch version: 0.5.1 host hardware: x86_64 host os:4.15.0-3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.14.31-i686: OK linux-4.14.31-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16-i686: OK linux-4.16-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: FROM: "MRS PATRICIA WILLIAMS" linux-media@vger.kernel.org
FYI: About My Previous Message Hi, Am Mrs Patricia William, i just want to know if you receive my previous email i sent to you last three (3) days ago. Is your email still Active? If YES; please can you email me back, i have something very important to discuss with you. Awaits your reply soon.. Best Regard Mrs. Patricia Williams
Re: [PATCH] v4l: vsp1: Fix vsp1_regs.h license header
On Sat, Apr 28, 2018 at 12:46:47AM +0300, Laurent Pinchart wrote: > All source files of the vsp1 driver are licensed under the GPLv2+ except > for vsp1_regs.h which is licensed under GPLv2. This is caused by a bad > copy&paste that dates back from the initial version of the driver. Fix > it. > > Cc: Nobuhiro Iwamatsu > Cc: Kieran Bingham > Cc: Sergei Shtylyov > Cc: Niklas Söderlund > Cc: Wolfram Sang > Signed-off-by: Laurent Pinchart Acked-by: Wolfram Sang signature.asc Description: PGP signature
Re: [PATCH] v4l: vsp1: Fix vsp1_regs.h license header
On 27/04/18 22:46, Laurent Pinchart wrote: > All source files of the vsp1 driver are licensed under the GPLv2+ except > for vsp1_regs.h which is licensed under GPLv2. This is caused by a bad > copy&paste that dates back from the initial version of the driver. Fix > it. > > Cc: Nobuhiro Iwamatsu > Cc: Kieran Bingham > Cc: Sergei Shtylyov > Cc: Niklas Söderlund > Cc: Wolfram Sang > Signed-off-by: Laurent Pinchart Acked-by: Kieran Bingham > --- > Iwamatsu-san, Kieran, Sergei, Niklas, Wolfram, > > While working on the VSP1 driver I noticed that all source files are > licensed under the GPLv2+ except for vsp1_regs.h which is licensed under > GPLv2. I'd like to fix this inconsistency. As you have all contributed > to that file, could you please provide your explicit ack if you agree to > this change ? > --- > drivers/media/platform/vsp1/vsp1_regs.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_regs.h > b/drivers/media/platform/vsp1/vsp1_regs.h > index 0d249ff9f564..e82661216c1d 100644 > --- a/drivers/media/platform/vsp1/vsp1_regs.h > +++ b/drivers/media/platform/vsp1/vsp1_regs.h > @@ -1,4 +1,4 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +/* SPDX-License-Identifier: GPL-2.0+ */ > /* > * vsp1_regs.h -- R-Car VSP1 Registers Definitions > * >
Re: [PATCH v2 1/8] v4l: vsp1: Use SPDX license headers
Hi Kieran, On Saturday, 28 April 2018 00:25:51 EEST Kieran Bingham wrote: > Hi Laurent, > > Thank you for the patch, and going through the whole driver for this update. > On 22/04/18 23:34, Laurent Pinchart wrote: > > Adopt the SPDX license identifier headers to ease license compliance > > management. All files in the driver are licensed under the GPLv2+ except > > for the vsp1_regs.h file which is licensed under the GPLv2. This is > > likely an oversight, but fixing this requires contacting the copyright > > owners and is out of scope for this patch. > > I agree that's out of scope for this patch, but it's not too exhaustive a > list to correct at a later date: > > git shortlog -e -n -s -- ./drivers/media/platform/vsp1/vsp1_regs.h > 19 Laurent Pinchart > 5 Nobuhiro Iwamatsu > 3 Kieran Bingham > 2 Geert Uytterhoeven > 2 Sergei Shtylyov > 1 Linus Torvalds > 1 Niklas Söderlund > 1 Wolfram Sang > > (Both Geert and Linus are merge commits there) I agree with you, I've sent a separate patch. > > While at it fix the file descriptions to match file names where copy and > > paste error occurred. > > > > Signed-off-by: Laurent Pinchart > > > > It's crazy that we have two types of comment style for the SPDX identifier - > but that's not a fault in this patch, so: > > Reviewed-by: Kieran Bingham > > > --- > > > > drivers/media/platform/vsp1/vsp1.h| 6 +- > > drivers/media/platform/vsp1/vsp1_brx.c| 6 +- > > drivers/media/platform/vsp1/vsp1_brx.h| 6 +- > > drivers/media/platform/vsp1/vsp1_clu.c| 6 +- > > drivers/media/platform/vsp1/vsp1_clu.h| 6 +- > > drivers/media/platform/vsp1/vsp1_dl.c | 8 ++-- > > drivers/media/platform/vsp1/vsp1_dl.h | 6 +- > > drivers/media/platform/vsp1/vsp1_drm.c| 8 ++-- > > drivers/media/platform/vsp1/vsp1_drm.h| 6 +- > > drivers/media/platform/vsp1/vsp1_drv.c| 6 +- > > drivers/media/platform/vsp1/vsp1_entity.c | 6 +- > > drivers/media/platform/vsp1/vsp1_entity.h | 6 +- > > drivers/media/platform/vsp1/vsp1_hgo.c| 6 +- > > drivers/media/platform/vsp1/vsp1_hgo.h| 6 +- > > drivers/media/platform/vsp1/vsp1_hgt.c| 6 +- > > drivers/media/platform/vsp1/vsp1_hgt.h| 6 +- > > drivers/media/platform/vsp1/vsp1_histo.c | 6 +- > > drivers/media/platform/vsp1/vsp1_histo.h | 6 +- > > drivers/media/platform/vsp1/vsp1_hsit.c | 6 +- > > drivers/media/platform/vsp1/vsp1_hsit.h | 6 +- > > drivers/media/platform/vsp1/vsp1_lif.c| 6 +- > > drivers/media/platform/vsp1/vsp1_lif.h| 6 +- > > drivers/media/platform/vsp1/vsp1_lut.c| 6 +- > > drivers/media/platform/vsp1/vsp1_lut.h| 6 +- > > drivers/media/platform/vsp1/vsp1_pipe.c | 6 +- > > drivers/media/platform/vsp1/vsp1_pipe.h | 6 +- > > drivers/media/platform/vsp1/vsp1_regs.h | 5 + > > drivers/media/platform/vsp1/vsp1_rpf.c| 6 +- > > drivers/media/platform/vsp1/vsp1_rwpf.c | 6 +- > > drivers/media/platform/vsp1/vsp1_rwpf.h | 6 +- > > drivers/media/platform/vsp1/vsp1_sru.c| 6 +- > > drivers/media/platform/vsp1/vsp1_sru.h| 6 +- > > drivers/media/platform/vsp1/vsp1_uds.c| 6 +- > > drivers/media/platform/vsp1/vsp1_uds.h| 6 +- > > drivers/media/platform/vsp1/vsp1_video.c | 6 +- > > drivers/media/platform/vsp1/vsp1_video.h | 6 +- > > drivers/media/platform/vsp1/vsp1_wpf.c| 6 +- > > 37 files changed, 39 insertions(+), 186 deletions(-) -- Regards, Laurent Pinchart
[PATCH] v4l: vsp1: Fix vsp1_regs.h license header
All source files of the vsp1 driver are licensed under the GPLv2+ except for vsp1_regs.h which is licensed under GPLv2. This is caused by a bad copy&paste that dates back from the initial version of the driver. Fix it. Cc: Nobuhiro Iwamatsu Cc: Kieran Bingham Cc: Sergei Shtylyov Cc: Niklas Söderlund Cc: Wolfram Sang Signed-off-by: Laurent Pinchart --- Iwamatsu-san, Kieran, Sergei, Niklas, Wolfram, While working on the VSP1 driver I noticed that all source files are licensed under the GPLv2+ except for vsp1_regs.h which is licensed under GPLv2. I'd like to fix this inconsistency. As you have all contributed to that file, could you please provide your explicit ack if you agree to this change ? --- drivers/media/platform/vsp1/vsp1_regs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h index 0d249ff9f564..e82661216c1d 100644 --- a/drivers/media/platform/vsp1/vsp1_regs.h +++ b/drivers/media/platform/vsp1/vsp1_regs.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +/* SPDX-License-Identifier: GPL-2.0+ */ /* * vsp1_regs.h -- R-Car VSP1 Registers Definitions * -- Regards, Laurent Pinchart
Re: [PATCH v2 1/8] v4l: vsp1: Use SPDX license headers
Hi Laurent, Thank you for the patch, and going through the whole driver for this update. On 22/04/18 23:34, Laurent Pinchart wrote: > Adopt the SPDX license identifier headers to ease license compliance > management. All files in the driver are licensed under the GPLv2+ except > for the vsp1_regs.h file which is licensed under the GPLv2. This is > likely an oversight, but fixing this requires contacting the copyright > owners and is out of scope for this patch. I agree that's out of scope for this patch, but it's not too exhaustive a list to correct at a later date: git shortlog -e -n -s -- ./drivers/media/platform/vsp1/vsp1_regs.h 19 Laurent Pinchart 5 Nobuhiro Iwamatsu 3 Kieran Bingham 2 Geert Uytterhoeven 2 Sergei Shtylyov 1 Linus Torvalds 1 Niklas Söderlund 1 Wolfram Sang (Both Geert and Linus are merge commits there) > While at it fix the file descriptions to match file names where copy and > paste error occurred. > > Signed-off-by: Laurent Pinchart It's crazy that we have two types of comment style for the SPDX identifier - but that's not a fault in this patch, so: Reviewed-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1.h| 6 +- > drivers/media/platform/vsp1/vsp1_brx.c| 6 +- > drivers/media/platform/vsp1/vsp1_brx.h| 6 +- > drivers/media/platform/vsp1/vsp1_clu.c| 6 +- > drivers/media/platform/vsp1/vsp1_clu.h| 6 +- > drivers/media/platform/vsp1/vsp1_dl.c | 8 ++-- > drivers/media/platform/vsp1/vsp1_dl.h | 6 +- > drivers/media/platform/vsp1/vsp1_drm.c| 8 ++-- > drivers/media/platform/vsp1/vsp1_drm.h| 6 +- > drivers/media/platform/vsp1/vsp1_drv.c| 6 +- > drivers/media/platform/vsp1/vsp1_entity.c | 6 +- > drivers/media/platform/vsp1/vsp1_entity.h | 6 +- > drivers/media/platform/vsp1/vsp1_hgo.c| 6 +- > drivers/media/platform/vsp1/vsp1_hgo.h| 6 +- > drivers/media/platform/vsp1/vsp1_hgt.c| 6 +- > drivers/media/platform/vsp1/vsp1_hgt.h| 6 +- > drivers/media/platform/vsp1/vsp1_histo.c | 6 +- > drivers/media/platform/vsp1/vsp1_histo.h | 6 +- > drivers/media/platform/vsp1/vsp1_hsit.c | 6 +- > drivers/media/platform/vsp1/vsp1_hsit.h | 6 +- > drivers/media/platform/vsp1/vsp1_lif.c| 6 +- > drivers/media/platform/vsp1/vsp1_lif.h| 6 +- > drivers/media/platform/vsp1/vsp1_lut.c| 6 +- > drivers/media/platform/vsp1/vsp1_lut.h| 6 +- > drivers/media/platform/vsp1/vsp1_pipe.c | 6 +- > drivers/media/platform/vsp1/vsp1_pipe.h | 6 +- > drivers/media/platform/vsp1/vsp1_regs.h | 5 + > drivers/media/platform/vsp1/vsp1_rpf.c| 6 +- > drivers/media/platform/vsp1/vsp1_rwpf.c | 6 +- > drivers/media/platform/vsp1/vsp1_rwpf.h | 6 +- > drivers/media/platform/vsp1/vsp1_sru.c| 6 +- > drivers/media/platform/vsp1/vsp1_sru.h| 6 +- > drivers/media/platform/vsp1/vsp1_uds.c| 6 +- > drivers/media/platform/vsp1/vsp1_uds.h| 6 +- > drivers/media/platform/vsp1/vsp1_video.c | 6 +- > drivers/media/platform/vsp1/vsp1_video.h | 6 +- > drivers/media/platform/vsp1/vsp1_wpf.c| 6 +- > 37 files changed, 39 insertions(+), 186 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1.h > b/drivers/media/platform/vsp1/vsp1.h > index 894cc725c2d4..9cf4e1c4b036 100644 > --- a/drivers/media/platform/vsp1/vsp1.h > +++ b/drivers/media/platform/vsp1/vsp1.h > @@ -1,14 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > /* > * vsp1.h -- R-Car VSP1 Driver > * > * Copyright (C) 2013-2014 Renesas Electronics Corporation > * > * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com) > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > */ > #ifndef __VSP1_H__ > #define __VSP1_H__ > diff --git a/drivers/media/platform/vsp1/vsp1_brx.c > b/drivers/media/platform/vsp1/vsp1_brx.c > index b4af1d546022..3beec18fd863 100644 > --- a/drivers/media/platform/vsp1/vsp1_brx.c > +++ b/drivers/media/platform/vsp1/vsp1_brx.c > @@ -1,14 +1,10 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * vsp1_brx.c -- R-Car VSP1 Blend ROP Unit (BRU and BRS) > * > * Copyright (C) 2013 Renesas Corporation > * > * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com) > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > */ > > #include > diff --git a/drivers/media/platform/vsp1/vsp1_brx.h > b/drivers/media/platform/vsp1/vsp1_brx.h > index 927
Re: [PATCH v2 1/2] dt-bindings: media: renesas-ceu: Add R-Mobile R8A7740
On Thu, Apr 26, 2018 at 08:24:42PM +0200, Jacopo Mondi wrote: > Add R-Mobile A1 R8A7740 SoC to the list of compatible values for the CEU > unit. > > Signed-off-by: Jacopo Mondi > --- > Documentation/devicetree/bindings/media/renesas,ceu.txt | 7 --- > drivers/media/platform/renesas-ceu.c| 1 + > 2 files changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Rob Herring
[PATCH] media: media-device: fix ioctl function types
This change fixes function types for media device ioctls to avoid
indirect call mismatches with Control-Flow Integrity checking.
Signed-off-by: Sami Tolvanen
---
drivers/media/media-device.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 35e81f7c0d2f..bc5c024906e6 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -54,9 +54,10 @@ static int media_device_close(struct file *filp)
return 0;
}
-static int media_device_get_info(struct media_device *dev,
-struct media_device_info *info)
+static long media_device_get_info(struct media_device *dev, void *arg)
{
+ struct media_device_info *info = (struct media_device_info *)arg;
+
memset(info, 0, sizeof(*info));
if (dev->driver_name[0])
@@ -93,9 +94,9 @@ static struct media_entity *find_entity(struct media_device
*mdev, u32 id)
return NULL;
}
-static long media_device_enum_entities(struct media_device *mdev,
- struct media_entity_desc *entd)
+static long media_device_enum_entities(struct media_device *mdev, void *arg)
{
+ struct media_entity_desc *entd = (struct media_entity_desc *)arg;
struct media_entity *ent;
ent = find_entity(mdev, entd->id);
@@ -146,9 +147,9 @@ static void media_device_kpad_to_upad(const struct
media_pad *kpad,
upad->flags = kpad->flags;
}
-static long media_device_enum_links(struct media_device *mdev,
- struct media_links_enum *links)
+static long media_device_enum_links(struct media_device *mdev, void *arg)
{
+ struct media_links_enum *links = (struct media_links_enum *)arg;
struct media_entity *entity;
entity = find_entity(mdev, links->entity);
@@ -195,9 +196,9 @@ static long media_device_enum_links(struct media_device
*mdev,
return 0;
}
-static long media_device_setup_link(struct media_device *mdev,
- struct media_link_desc *linkd)
+static long media_device_setup_link(struct media_device *mdev, void *arg)
{
+ struct media_link_desc *linkd = (struct media_link_desc *)arg;
struct media_link *link = NULL;
struct media_entity *source;
struct media_entity *sink;
@@ -225,9 +226,9 @@ static long media_device_setup_link(struct media_device
*mdev,
return __media_entity_setup_link(link, linkd->flags);
}
-static long media_device_get_topology(struct media_device *mdev,
- struct media_v2_topology *topo)
+static long media_device_get_topology(struct media_device *mdev, void *arg)
{
+ struct media_v2_topology *topo = (struct media_v2_topology *)arg;
struct media_entity *entity;
struct media_interface *intf;
struct media_pad *pad;
--
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH v2 08/15] media: dt-bindings: add bindings for i.MX7 media driver
On Mon, Apr 23, 2018 at 02:47:43PM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
>
> Signed-off-by: Rui Miguel Silva
> ---
> .../devicetree/bindings/media/imx7.txt| 158 ++
> 1 file changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/imx7.txt
>
> diff --git a/Documentation/devicetree/bindings/media/imx7.txt
> b/Documentation/devicetree/bindings/media/imx7.txt
> new file mode 100644
> index ..7e058ea25102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx7.txt
> @@ -0,0 +1,158 @@
> +Freescale i.MX7 Media Video Device
> +==
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx7-capture-subsystem";
> +- ports : Should contain a list of phandles pointing to camera
> + sensor interface port of CSI
> +
> +example:
> +
> +capture-subsystem {
> + compatible = "fsl,imx7-capture-subsystem";
> + ports = <&csi>;
Why do you need this node? Just have the driver match on the CSI node.
> +};
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> +compatible with previous version of Samsung D-phy.
Compatible with Samsung?
> +
> +Required properties:
> +
> +- compatible: "fsl,imx7-mipi-csi2";
> +- reg : base address and length of the register set for the device;
> +- interrupts: should contain MIPI CSIS interrupt;
> +- clocks: list of clock specifiers, see
> +Documentation/devicetree/bindings/clock/clock-bindings.txt for
> details;
> +- clock-names : must contain "mipi" and "phy" entries, matching entries in
> the
> + clock property;
> +- power-domains : a phandle to the power domain, see
> + Documentation/devicetree/bindings/power/power_domain.txt for
> details.
> +- reset-names : should include following entry "mrst";
> +- resets: a list of phandle, should contain reset entry of
> + reset-names;
> +- phy-supply: from the generic phy bindings, a phandle to a regulator
> that
> + provides power to VBUS;
VBUS? Copy-n-paste from USB something?
> +- bus-width : maximum number of data lanes supported (SoC specific);
Don't we have a standard lanes property for CSI (or DSI)? bus-width is
for parallel buses and goes in endpoint nodes. (But maybe it got used
here too).
> +
> +Optional properties:
> +
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> + value when this property is not specified is 166 MHz;
> +
> +port node
> +-
> +
> +- reg : (required) can take the values 0 or 1, where 0 is
> the
> + related sink port and port 1 should be the source one;
> +
> +endpoint node
> +-
> +
> +- data-lanes: (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;
Ah yes, like this. :) So why do you need bus-width too?
> +
> +- csis-hs-settle : (optional) differential receiver (HS-RX) settle time;
units?
> +- csis-clk-settle : (optional) D-PHY control register;
> +- csis-wclk : CSI-2 wrapper clock selection. If this property is present
> + external clock from CMU will be used, or the bus clock if
> + if it's not specified.
boolean?
These 3 need vendor properties.
> +
> +example:
> +
> + mipi_csi: mipi-csi@3075 {
> +clock-frequency = <16600>;
> +status = "okay";
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> + compatible = "fsl,imx7-mipi-csi2";
> + reg = <0x3075 0x1>;
> + interrupts = ;
> + clocks = <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> + <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> + clock-names = "mipi", "phy";
> + power-domains = <&pgc_mipi_phy>;
> + phy-supply = <®_1p0d>;
> + resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> + reset-names = "mrst";
> + bus-width = <4>;
> + status = "disabled";
> +
> +port@0 {
> +reg = <0>;
> +
> +mipi_from_sensor: endpoint {
> +remote-endpoint = <&ov2680_to_mipi>;
> +data-lanes = <1>;
> +csis-hs-settle = <3>;
> +csis-clk-settle = <0>;
> +
[PATCH] media: v4l2-ioctl: fix function types for IOCTL_INFO_STD
This change fixes indirect call mismatches with Control-Flow Integrity
checking, which are caused by calling standard ioctls using a function
pointer that doesn't match the type of the actual function.
Signed-off-by: Sami Tolvanen
---
drivers/media/v4l2-core/v4l2-ioctl.c | 72 ++--
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index f48c505550e0..d50a06ab3509 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2489,11 +2489,8 @@ struct v4l2_ioctl_info {
unsigned int ioctl;
u32 flags;
const char * const name;
- union {
- u32 offset;
- int (*func)(const struct v4l2_ioctl_ops *ops,
- struct file *file, void *fh, void *p);
- } u;
+ int (*func)(const struct v4l2_ioctl_ops *ops, struct file *file,
+ void *fh, void *p);
void (*debug)(const void *arg, bool write_only);
};
@@ -2501,27 +2498,24 @@ struct v4l2_ioctl_info {
#define INFO_FL_PRIO (1 << 0)
/* This control can be valid if the filehandle passes a control handler. */
#define INFO_FL_CTRL (1 << 1)
-/* This is a standard ioctl, no need for special code */
-#define INFO_FL_STD(1 << 2)
/* This is ioctl has its own function */
-#define INFO_FL_FUNC (1 << 3)
+#define INFO_FL_FUNC (1 << 2)
/* Queuing ioctl */
-#define INFO_FL_QUEUE (1 << 4)
+#define INFO_FL_QUEUE (1 << 3)
/* Always copy back result, even on error */
-#define INFO_FL_ALWAYS_COPY(1 << 5)
+#define INFO_FL_ALWAYS_COPY(1 << 4)
/* Zero struct from after the field to the end */
#define INFO_FL_CLEAR(v4l2_struct, field) \
((offsetof(struct v4l2_struct, field) + \
sizeof(((struct v4l2_struct *)0)->field)) << 16)
#define INFO_FL_CLEAR_MASK (_IOC_SIZEMASK << 16)
-#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)
\
- [_IOC_NR(_ioctl)] = { \
- .ioctl = _ioctl,\
- .flags = _flags | INFO_FL_STD, \
- .name = #_ioctl,\
- .u.offset = offsetof(struct v4l2_ioctl_ops, _vidioc), \
- .debug = _debug,\
+#define DEFINE_IOCTL_STD_FNC(_vidioc) \
+ static int __v4l_ ## _vidioc ## _fnc( \
+ const struct v4l2_ioctl_ops *ops, \
+ struct file *file, void *fh, void *p) \
+ { \
+ return ops->_vidioc(file, fh, p); \
}
#define IOCTL_INFO_FNC(_ioctl, _func, _debug, _flags) \
@@ -2529,10 +2523,42 @@ struct v4l2_ioctl_info {
.ioctl = _ioctl,\
.flags = _flags | INFO_FL_FUNC, \
.name = #_ioctl,\
- .u.func = _func,\
+ .func = _func, \
.debug = _debug,\
}
+#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)\
+ IOCTL_INFO_FNC(_ioctl, __v4l_ ## _vidioc ## _fnc, _debug, _flags)
+
+DEFINE_IOCTL_STD_FNC(vidioc_g_fbuf)
+DEFINE_IOCTL_STD_FNC(vidioc_s_fbuf)
+DEFINE_IOCTL_STD_FNC(vidioc_expbuf)
+DEFINE_IOCTL_STD_FNC(vidioc_g_std)
+DEFINE_IOCTL_STD_FNC(vidioc_g_audio)
+DEFINE_IOCTL_STD_FNC(vidioc_s_audio)
+DEFINE_IOCTL_STD_FNC(vidioc_g_input)
+DEFINE_IOCTL_STD_FNC(vidioc_g_edid)
+DEFINE_IOCTL_STD_FNC(vidioc_s_edid)
+DEFINE_IOCTL_STD_FNC(vidioc_g_output)
+DEFINE_IOCTL_STD_FNC(vidioc_g_audout)
+DEFINE_IOCTL_STD_FNC(vidioc_s_audout)
+DEFINE_IOCTL_STD_FNC(vidioc_g_jpegcomp)
+DEFINE_IOCTL_STD_FNC(vidioc_s_jpegcomp)
+DEFINE_IOCTL_STD_FNC(vidioc_enumaudio)
+DEFINE_IOCTL_STD_FNC(vidioc_enumaudout)
+DEFINE_IOCTL_STD_FNC(vidioc_enum_framesizes)
+DEFINE_IOCTL_STD_FNC(vidioc_enum_frameintervals)
+DEFINE_IOCTL_STD_FNC(vidioc_g_enc_index)
+DEFINE_IOCTL_STD_FNC(vidioc_encoder_cmd)
+DEFINE_IOCTL_STD_FNC(vidioc_try_encoder_cmd)
+DEFINE_IOCTL_STD_FNC(vidioc_decoder_cmd)
+DEFINE_IOCTL_STD_FNC(vidioc_try_decoder_cmd)
+DEFINE_IOCTL_STD_FNC(vidioc_s_dv_timings)
+DEFINE_IOCTL_STD_FNC(vidioc_g_dv_timings)
+DEFINE_IOCTL_STD_FNC(vidioc_enum_dv_timings)
+DEFINE_IOCTL_STD_FNC(vidioc_query_dv_timings)
+DEFINE_IOCTL_STD_FNC(vidioc_dv_timings_cap)
+
static struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO_FNC(VIDIOC_QUERYCAP, v4l_querycap, v4l_print_querycap, 0),
IOCTL_INFO_FNC(VIDIOC_
Re: [PATCH] media: Revert cleanup ktime_set() usage
Em Fri, 27 Apr 2018 20:15:00 +0200
"Jasmin J." escreveu:
> Hello Mauro!
>
> > This patch looks fine, but not for the above-mentioned.
> So I shall reword the commit message?
Yes.
>
> > The thing is that it is not consistent to have some places with
> > things like:
> > timeout = ktime_set(1, ir->polling * 100);
> >
> > and others with:
> > timeout = ir->polling * 100;
> With my patch applied I can find ONLY two "ir->polling * 100" and both
> are used
> together with ktime_set.
>
> $ grep -r "ir->polling" *
> media/i2c/ir-kbd-i2c.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling_interval));
> media/i2c/ir-kbd-i2c.c: ir->polling_interval = DEFAULT_POLLING_INTERVAL;
> media/i2c/ir-kbd-i2c.c: ir->polling_interval =
> init_data->polling_interval;
> media/usb/em28xx/em28xx-input.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling));
> media/usb/em28xx/em28xx-input.c: ir->polling = 100; /* ms */
> media/usb/em28xx/em28xx-input.c:
> schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
> media/usb/tm6000/tm6000-input.c: if (!ir->polling)
> media/usb/tm6000/tm6000-input.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling));
> media/usb/tm6000/tm6000-input.c: ir->polling = 50;
> media/usb/tm6000/tm6000-input.c: if (!ir->polling)
> media/usb/au0828/au0828-input.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling));
> media/usb/au0828/au0828-input.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling));
> media/usb/au0828/au0828-input.c: ir->polling = 100; /* ms */
> media/usb/au0828/au0828-input.c: schedule_delayed_work(&ir->work,
> msecs_to_jiffies(ir->polling));
> media/pci/saa7134/saa7134-input.c:if (ir->polling) {
> media/pci/saa7134/saa7134-input.c:if (ir->polling) {
> media/pci/saa7134/saa7134-input.c:if (!ir->polling && !ir->raw_decode) {
> media/pci/saa7134/saa7134-input.c:mod_timer(&ir->timer, jiffies +
> msecs_to_jiffies(ir->polling));
> media/pci/saa7134/saa7134-input.c:if (ir->polling) {
> media/pci/saa7134/saa7134-input.c:if (ir->polling)
> media/pci/saa7134/saa7134-input.c:ir->polling = polling;
> media/pci/cx88/cx88-input.c: if (ir->polling) {
> media/pci/cx88/cx88-input.c: ir->polling ? "poll" : "irq",
> media/pci/cx88/cx88-input.c: ktime_set(0,
> ir->polling * 100));
> media/pci/cx88/cx88-input.c: if (ir->polling) {
> media/pci/cx88/cx88-input.c:ktime_set(0, ir->polling
> * 100),
> media/pci/cx88/cx88-input.c: if (ir->polling)
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 1; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 5; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 10; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 1; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 1; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 1; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 50; /* ms */
> media/pci/cx88/cx88-input.c: ir->polling = 100; /* ms */
> media/pci/bt8xx/bttv-input.c: if (ir->polling) {
> media/pci/bt8xx/bttv-input.c: ir->polling ? "poll" :
> "irq",
> media/pci/bt8xx/bttv-input.c: else if (!ir->polling)
> media/pci/bt8xx/bttv-input.c: mod_timer(&ir->timer, jiffies +
> msecs_to_jiffies(ir->polling));
> media/pci/bt8xx/bttv-input.c: if (ir->polling) {
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; /* ms */
> media/pci/bt8xx/bttv-input.c: ir->polling = 50; /* ms */
> media/pci/bt8xx/bttv-input.c: ir->polling = 1; /* ms */
>
> Currently I am using the exact same patch in media-build. media-build would
> not be able to compile without error for all Kernels, if I would have
> overlook a setting of ktime_t without ktime_set.
> Please point me where I can find the "timeout = ir->polling * 100;"
> you mentioned.
I don't really care on how many places it is used. The point is that,
at least on media, we should either call ktime_set() e
Re: [PATCH] media: Revert cleanup ktime_set() usage
Hello Mauro!
> This patch looks fine, but not for the above-mentioned.
So I shall reword the commit message?
> The thing is that it is not consistent to have some places with
> things like:
> timeout = ktime_set(1, ir->polling * 100);
>
> and others with:
> timeout = ir->polling * 100;
With my patch applied I can find ONLY two "ir->polling * 100" and both are
used
together with ktime_set.
$ grep -r "ir->polling" *
media/i2c/ir-kbd-i2c.c: schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling_interval));
media/i2c/ir-kbd-i2c.c: ir->polling_interval = DEFAULT_POLLING_INTERVAL;
media/i2c/ir-kbd-i2c.c: ir->polling_interval =
init_data->polling_interval;
media/usb/em28xx/em28xx-input.c:schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling));
media/usb/em28xx/em28xx-input.c:ir->polling = 100; /* ms */
media/usb/em28xx/em28xx-input.c:
schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
media/usb/tm6000/tm6000-input.c:if (!ir->polling)
media/usb/tm6000/tm6000-input.c:schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling));
media/usb/tm6000/tm6000-input.c:ir->polling = 50;
media/usb/tm6000/tm6000-input.c:if (!ir->polling)
media/usb/au0828/au0828-input.c:schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling));
media/usb/au0828/au0828-input.c:schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling));
media/usb/au0828/au0828-input.c:ir->polling = 100; /* ms */
media/usb/au0828/au0828-input.c:schedule_delayed_work(&ir->work,
msecs_to_jiffies(ir->polling));
media/pci/saa7134/saa7134-input.c: if (ir->polling) {
media/pci/saa7134/saa7134-input.c: if (ir->polling) {
media/pci/saa7134/saa7134-input.c: if (!ir->polling && !ir->raw_decode) {
media/pci/saa7134/saa7134-input.c: mod_timer(&ir->timer, jiffies +
msecs_to_jiffies(ir->polling));
media/pci/saa7134/saa7134-input.c: if (ir->polling) {
media/pci/saa7134/saa7134-input.c: if (ir->polling)
media/pci/saa7134/saa7134-input.c: ir->polling = polling;
media/pci/cx88/cx88-input.c:if (ir->polling) {
media/pci/cx88/cx88-input.c: ir->polling ? "poll" : "irq",
media/pci/cx88/cx88-input.c: ktime_set(0,
ir->polling * 100));
media/pci/cx88/cx88-input.c:if (ir->polling) {
media/pci/cx88/cx88-input.c: ktime_set(0, ir->polling
* 100),
media/pci/cx88/cx88-input.c:if (ir->polling)
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 1; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 5; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 10; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 1; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 1; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 1; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 50; /* ms */
media/pci/cx88/cx88-input.c:ir->polling = 100; /* ms */
media/pci/bt8xx/bttv-input.c: if (ir->polling) {
media/pci/bt8xx/bttv-input.c: ir->polling ? "poll" :
"irq",
media/pci/bt8xx/bttv-input.c: else if (!ir->polling)
media/pci/bt8xx/bttv-input.c: mod_timer(&ir->timer, jiffies +
msecs_to_jiffies(ir->polling));
media/pci/bt8xx/bttv-input.c: if (ir->polling) {
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; // ms
media/pci/bt8xx/bttv-input.c: ir->polling = 50; /* ms */
media/pci/bt8xx/bttv-input.c: ir->polling = 50; /* ms */
media/pci/bt8xx/bttv-input.c: ir->polling = 1; /* ms */
Currently I am using the exact same patch in media-build. media-build would
not be able to compile without error for all Kernels, if I would have
overlook a setting of ktime_t without ktime_set.
Please point me where I can find the "timeout = ir->polling * 100;"
you mentioned.
> My preference is to keep using it, as it makes it better documented.
This is also my preference.
> The fact that it makes maintainership of the media_build backport
> tree easier is just a plus, but it makes no sense upstream.
We fixed all those issues in the past in media-tree.
Re: [PATCH v3 07/11] media: ov772x: handle nested s_power() calls
2018-04-23 17:35 GMT+09:00 jacopo mondi :
> Hi Akinobu,
>thanks for v3
>
> On Mon, Apr 23, 2018 at 12:56:13AM +0900, Akinobu Mita wrote:
>> Depending on the v4l2 driver, calling s_power() could be nested. So the
>> actual transitions between power saving mode and normal operation mode
>> should only happen at the first power on and the last power off.
>>
>> This adds an s_power() nesting counter and updates the power state if the
>> counter is modified from 0 to != 0 or from != 0 to 0.
>>
>> Cc: Jacopo Mondi
>> Cc: Laurent Pinchart
>> Cc: Hans Verkuil
>> Cc: Sakari Ailus
>> Cc: Mauro Carvalho Chehab
>> Signed-off-by: Akinobu Mita
>> ---
>> * v3
>> - Rename mutex name from power_lock to lock
>> - Add warning for duplicated s_power call
>>
>> drivers/media/i2c/ov772x.c | 34 ++
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index 95c1c95d..8c0b850 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -424,6 +424,9 @@ struct ov772x_priv {
>> /* band_filter = COM8[5] ? 256 - BDBASE : 0 */
>> unsigned shortband_filter;
>> unsigned int fps;
>> + /* lock to protect power_count */
>> + struct mutex lock;
>> + int power_count;
>> #ifdef CONFIG_MEDIA_CONTROLLER
>> struct media_pad pad;
>> #endif
>> @@ -871,9 +874,26 @@ static int ov772x_power_off(struct ov772x_priv *priv)
>> static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>> {
>> struct ov772x_priv *priv = to_ov772x(sd);
>> + int ret = 0;
>> +
>> + mutex_lock(&priv->lock);
>> +
>> + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
>> + * update the power state.
>> + */
>> + if (priv->power_count == !on)
>> + ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
>> +
>> + if (!ret) {
>> + /* Update the power count. */
>> + priv->power_count += on ? 1 : -1;
>> + WARN(priv->power_count < 0, "Unbalanced power count\n");
>> + WARN(priv->power_count > 1, "Duplicated s_power call\n");
>
>
> The first time you receive a power on (on == 1, power_count == 0)
> you'll have:
>
> if (0 == !1)
> ret = ov772x_power_on()
> if (!0)
> power_count += 1;
>
> If power management is 'nested' you could receive a second,
> unexpected, power on (on == 1, power_count == 1)
> if (1 == !1)
> if (!0)
> power_count += 1;
>
> Now power_count == 2, if you now receive a power off, you're going to
> ignore it:
>
> if (2 == !0)
> ret = ov772x_power_off();
> if (!0)
> power_count += -1;
>
> Again you now receive a new power off, and you're going to issue that
> one.
>
> if (1 == !0)
> ret = ov772x_power_off();
> if (!0)
> power_count += -1;
>
> So this seems correct to me!
>
> If I were you, I would simplify this as:
>
> power_count += on ? 1 : -1;
> if (power_count == !!on)
> return on ? ov772x_power_on(priv) :
> ov772x_power_off(priv);
> WARN_ON(power_count < 0 || power_count > 1);
>
> return 0;
The only drawback is that the power_count is updated even when the
ov772x_power_on/off failed.
> which, if I'm not wrong expands to:
>
> power_count = 0 ; on = 1
> power_count += 1;
> if (1 == !!1)
> return ov772x_power_on(priv);
>
> power_count = 1 ; on = 1
> power_count += 1;
> if (2 == !!1)
> WARN_ON(2 > 1)
>
> power_count = 2 ; on = 0
> power_count -= 1;
> if (1 == !!0)
> if (1 > 1 || 1 < 0)
>
> power_count = 1; on = 0
> power_count -= 1;
> if (0 == !!0)
> return ov772_power_off(priv);
>
> power_count = 0 ; on = 0
> power_count -= 1;
> if (-1 == !!0)
> WARN_ON(-1 < 0)
>
> But if you have seen that pattern in other drivers, and maybe already
> tested your one, I'm fine with either of them.
For now, I leave this patch unchanged. If we can guarantee that s_power()
is not nested, we can simplify it by removing power_count or moving to
common field.
> Reviewed-by: Jacopo Mondi
>
> Thanks
>j
>
>> + }
>> +
>> + mutex_unlock(&priv->lock);
>>
>> - return on ? ov772x_power_on(priv) :
>> - ov772x_power_off(priv);
>> + return ret;
>> }
>>
>> static const struct ov772x_win_size *ov772x_select_win(u32 width, u32
>> height)
>> @@ -1303,6 +1323,7 @@ static int ov772x_probe(struct i2c_client *client,
>> return -ENOMEM;
>>
>> priv->info = client->dev.platform_data;
>> + mutex_init(&priv->lock);
>>
>> v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subde
Re: [PATCH v3 01/11] media: dt-bindings: ov772x: add device tree binding
2018-04-27 6:34 GMT+09:00 Laurent Pinchart :
> Hi Mita-san,
>
> On Thursday, 26 April 2018 19:17:55 EEST Akinobu Mita wrote:
>> 2018-04-26 7:40 GMT+09:00 Laurent Pinchart:
>> > On Wednesday, 25 April 2018 19:19:11 EEST Akinobu Mita wrote:
>> >> 2018-04-24 0:54 GMT+09:00 Akinobu Mita :
>> >> > 2018-04-23 18:17 GMT+09:00 Laurent Pinchart:
>> >> >> On Sunday, 22 April 2018 18:56:07 EEST Akinobu Mita wrote:
>> >> >>> This adds a device tree binding documentation for OV7720/OV7725
>> >> >>> sensor.
>> >> >>>
>> >> >>> Cc: Jacopo Mondi
>> >> >>> Cc: Laurent Pinchart
>> >> >>> Cc: Hans Verkuil
>> >> >>> Cc: Sakari Ailus
>> >> >>> Cc: Mauro Carvalho Chehab
>> >> >>> Cc: Rob Herring
>> >> >>> Reviewed-by: Rob Herring
>> >> >>> Reviewed-by: Jacopo Mondi
>> >> >>> Signed-off-by: Akinobu Mita
>> >> >>> ---
>> >> >>> * v3
>> >> >>> - Add Reviewed-by: lines
>> >> >>>
>> >> >>> .../devicetree/bindings/media/i2c/ov772x.txt | 42
>> >> >>> +++
>> >> >>> MAINTAINERS| 1 +
>> >> >>> 2 files changed, 43 insertions(+)
>> >> >>> create mode 100644
>> >> >>> Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>>
>> >> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>> b/Documentation/devicetree/bindings/media/i2c/ov772x.txt new file
>> >> >>> mode
>> >> >>> 100644
>> >> >>> index 000..b045503
>> >> >>> --- /dev/null
>> >> >>> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>> @@ -0,0 +1,42 @@
>> >> >>> +* Omnivision OV7720/OV7725 CMOS sensor
>> >> >>> +
>> >> >>> +The Omnivision OV7720/OV7725 sensor supports multiple resolutions
>> >> >>> output,
>> >> >>> +such as VGA, QVGA, and any size scaling down from CIF to 40x30. It
>> >> >>> also
>> >> >>> can +support the YUV422, RGB565/555/444, GRB422 or raw RGB output
>> >> >>> formats. +
>> >> >>> +Required Properties:
>> >> >>> +- compatible: shall be one of
>> >> >>> + "ovti,ov7720"
>> >> >>> + "ovti,ov7725"
>> >> >>> +- clocks: reference to the xclk input clock.
>> >> >>> +- clock-names: shall be "xclk".
>> >> >>
>> >> >> As there's a single clock we could omit clock-names, couldn't we ?
>> >> >
>> >> > Sounds good.
>> >> >
>> >> > I'll prepare another patch that replaces the clock consumer ID argument
>> >> > of clk_get() from "xclk" to NULL, and remove the above line in this
>> >> > bindings.
>> >>
>> >> I thought it's easy to do. However, there is a non-DT user
>> >> (arch/sh/boards/mach-migor/setup.c) that defines a clock with "xclk" ID.
>> >>
>> >> This can be resolved by retrying clk_get() with NULL if no entry
>> >> with "xclk". But should we do so or leave as is?
>> >
>> > How about patching the board code to register the clock alias with
>> >
>> > clk_add_alias(NULL, "0-0021", "video_clk", NULL);
>>
>> Sounds good.
>>
>> But I'm a bit worried about whether clk_add_alias() can be called with
>> alias == NULL. I couldn't find such use case.
>
> There aren't many occurrences, but
>
> $ find . -type f -exec grep -l 'clk_add_alias(NULL' {} \;
> /drivers/clk/ti/clk.c
> /drivers/clk/ti/fixed-factor.c
> /drivers/clk/ti/clk-dra7-atl.c
> /drivers/clk/ti/composite.c
>
> A quick code analysis also shows me that this should be supported.
This hits ti_clk_add_alias() only. The function name is very similar,
but the first argument is struct device *.
Anyway, I'll add the change you suggested as Jacopo tested it.
Re: Regression: DVBSky S960 USB tuner doesn't work in 4.10 or newer
Em Fri, 27 Apr 2018 16:25:08 +0200
Olli Salonen escreveu:
> Thanks for the suggestion Antti.
>
> I've tried to add a delay in various places, but haven't seen any
> improvement. However, what I did saw was that if I added an msleep
> after the lock:
>
> static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
> u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen)
> {
> int ret;
> struct dvbsky_state *state = d_to_priv(d);
>
> mutex_lock(&d->usb_mutex);
> msleep(20);
>
> The error was seen very within a minute. If I increased the msleep to
> 50, it failed within seconds. This doesn't seem to make sense to me.
> This is the only function where usb_mutex is used. If the mutex is
> held for a longer time, the next attempt to lock the mutex should just
> be delayed a bit, no?
I don't like the idea of having two mutexes there to protect reading/writing
to data one for "generic" r/w ops, and another one just for streaming
control, with ends by calling the "generic" mutex.
IMHO, I would get rid of one of the mutexes, e. g. something like the
patch below (untested).
Regards,
Mauro
media: dvbsky: use just one mutex for serializing device R/W ops
Right now, there are two mutexes serializing r/w ops: one "generic"
and another one specifically for stream on/off.
Clean it a little bit, getting rid of one of the mutexes.
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c
b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 43eb82884555..50553975c39d 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -31,7 +31,6 @@ MODULE_PARM_DESC(disable_rc, "Disable inbuilt IR receiver.");
DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
struct dvbsky_state {
- struct mutex stream_mutex;
u8 ibuf[DVBSKY_BUF_LEN];
u8 obuf[DVBSKY_BUF_LEN];
u8 last_lock;
@@ -68,18 +67,17 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
{
- struct dvbsky_state *state = d_to_priv(d);
int ret;
- u8 obuf_pre[3] = { 0x37, 0, 0 };
- u8 obuf_post[3] = { 0x36, 3, 0 };
+ static u8 obuf_pre[3] = { 0x37, 0, 0 };
+ static u8 obuf_post[3] = { 0x36, 3, 0 };
- mutex_lock(&state->stream_mutex);
- ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
+ mutex_lock(&d->usb_mutex);
+ ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
if (!ret && onoff) {
msleep(20);
- ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
+ ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
}
- mutex_unlock(&state->stream_mutex);
+ mutex_unlock(&d->usb_mutex);
return ret;
}
@@ -744,8 +742,6 @@ static int dvbsky_init(struct dvb_usb_device *d)
if (ret)
return ret;
*/
- mutex_init(&state->stream_mutex);
-
state->last_lock = 0;
return 0;
Thanks,
Mauro
[PATCH v2 1/3] media: coda: reuse coda_s_fmt_vid_cap to propagate format in coda_s_fmt_vid_out
Instead of duplicating the same code, call into coda_s_fmt_vid_out to
propagate the output format to the capture queue.
Signed-off-by: Philipp Zabel
---
Changes since v1 [1]:
- split out into a separate patch
[1] https://patchwork.linuxtv.org/patch/48266/
This is what coda_s_fmt_vid_cap currently looks like:
static int coda_s_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
struct coda_ctx *ctx = fh_to_ctx(priv);
struct coda_q_data *q_data_src;
struct v4l2_rect r;
int ret;
ret = coda_try_fmt_vid_cap(file, priv, f);
if (ret)
return ret;
q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
r.left = 0;
r.top = 0;
r.width = q_data_src->width;
r.height = q_data_src->height;
return coda_s_fmt(ctx, f, &r);
}
---
drivers/media/platform/coda/coda-common.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 04e35d70ce2e..40632df9e3e8 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -786,9 +786,7 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
struct v4l2_format *f)
{
struct coda_ctx *ctx = fh_to_ctx(priv);
- struct coda_q_data *q_data_src;
struct v4l2_format f_cap;
- struct v4l2_rect r;
int ret;
ret = coda_try_fmt_vid_out(file, priv, f);
@@ -810,17 +808,7 @@ static int coda_s_fmt_vid_out(struct file *file, void
*priv,
f_cap.fmt.pix.width = f->fmt.pix.width;
f_cap.fmt.pix.height = f->fmt.pix.height;
- ret = coda_try_fmt_vid_cap(file, priv, &f_cap);
- if (ret)
- return ret;
-
- q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
- r.left = 0;
- r.top = 0;
- r.width = q_data_src->width;
- r.height = q_data_src->height;
-
- return coda_s_fmt(ctx, &f_cap, &r);
+ return coda_s_fmt_vid_cap(file, priv, &f_cap);
}
static int coda_reqbufs(struct file *file, void *priv,
--
2.17.0
[PATCH v2 3/3] media: coda: set colorimetry on coded queue
Do not set context colorimetry on the raw (OUTPUT) queue for encoders. Always set colorimetry on the coded queue (CAPTURE for encoders, OUTPUT for decoders). This also skips propagation of capture queue format and selection rectangle on S_FMT(OUTPUT) to the CAPTURE queue for encoders. Signed-off-by: Philipp Zabel --- Added patch since v1 [1]: - remove automatic format propagation on S_FMT(OUT) for encoders - adding S_FMT(CAP) propagation from capture to output queue for encoders is left for a later time, this isn't even documented yet. [1] https://patchwork.linuxtv.org/patch/48266/ --- drivers/media/platform/coda/coda-common.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index d3e22c14fad4..c7631e117dd3 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -779,7 +779,19 @@ static int coda_s_fmt_vid_cap(struct file *file, void *priv, r.width = q_data_src->width; r.height = q_data_src->height; - return coda_s_fmt(ctx, f, &r); + ret = coda_s_fmt(ctx, f, &r); + if (ret) + return ret; + + if (ctx->inst_type != CODA_INST_ENCODER) + return 0; + + ctx->colorspace = f->fmt.pix.colorspace; + ctx->xfer_func = f->fmt.pix.xfer_func; + ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc; + ctx->quantization = f->fmt.pix.quantization; + + return 0; } static int coda_s_fmt_vid_out(struct file *file, void *priv, @@ -798,6 +810,9 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv, if (ret) return ret; + if (ctx->inst_type != CODA_INST_DECODER) + return 0; + ctx->colorspace = f->fmt.pix.colorspace; ctx->xfer_func = f->fmt.pix.xfer_func; ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc; -- 2.17.0
[PATCH v2 2/3] media: coda: do not try to propagate format if capture queue busy
The driver helpfully resets the capture queue format and selection
rectangle whenever output format is changed. This only works while
the capture queue is not busy.
Signed-off-by: Philipp Zabel
---
Changes since v1 [1]:
- split out into a separate patch
[1] https://patchwork.linuxtv.org/patch/48266/
---
drivers/media/platform/coda/coda-common.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 40632df9e3e8..d3e22c14fad4 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -787,6 +787,7 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
{
struct coda_ctx *ctx = fh_to_ctx(priv);
struct v4l2_format f_cap;
+ struct vb2_queue *dst_vq;
int ret;
ret = coda_try_fmt_vid_out(file, priv, f);
@@ -802,6 +803,19 @@ static int coda_s_fmt_vid_out(struct file *file, void
*priv,
ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
ctx->quantization = f->fmt.pix.quantization;
+ dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ if (!dst_vq)
+ return -EINVAL;
+
+ /*
+* Setting the capture queue format is not possible while the capture
+* queue is still busy. This is not an error, but the user will have to
+* make sure themselves that the capture format is set correctly before
+* starting the output queue again.
+*/
+ if (vb2_is_busy(dst_vq))
+ return 0;
+
memset(&f_cap, 0, sizeof(f_cap));
f_cap.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
coda_g_fmt(file, priv, &f_cap);
--
2.17.0
Re: Regression: DVBSky S960 USB tuner doesn't work in 4.10 or newer
Thanks for the suggestion Antti.
I've tried to add a delay in various places, but haven't seen any
improvement. However, what I did saw was that if I added an msleep
after the lock:
static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen)
{
int ret;
struct dvbsky_state *state = d_to_priv(d);
mutex_lock(&d->usb_mutex);
msleep(20);
The error was seen very within a minute. If I increased the msleep to
50, it failed within seconds. This doesn't seem to make sense to me.
This is the only function where usb_mutex is used. If the mutex is
held for a longer time, the next attempt to lock the mutex should just
be delayed a bit, no?
Cheers,
-olli
On 18 April 2018 at 10:49, Antti Palosaari wrote:
>
> On 04/18/2018 07:49 AM, Olli Salonen wrote:
>>
>> Thank you for your response Peter!
>>
>> Indeed, it seems strange. dvbsky.c driver seems to use mutex_lock in
>> very much the same way as many other drivers. I've now confirmed that
>> I can get a 4.10 kernel with working DVBSky S960 by reverting the
>> following 4 patches:
>>
>> 549bdd3 Revert "locking/mutex: Add lock handoff to avoid starvation"
>> 3210f31 Revert "locking/mutex: Restructure wait loop"
>> 418a170 Revert "locking/mutex: Simplify some ww_mutex code in
>> __mutex_lock_common()"
>> 0b1fb8f Revert "locking/mutex: Enable optimistic spinning of woken waiter"
>> c470abd Linux 4.10
>
>
> These kind of issues tend to be timing issues very often. Just add some
> sleeps to i2c adapter algo / usb control messages and test.
>
> regards
> Antti
>
>
>
>
> --
> http://palosaari.fi/
[PATCH] media: rc: probe zilog transmitter when zilog receiver is found
I found a Hauppauge WinTV 44981 (bt878) with a Zilog Z8F0811. The
transmitter was not probed. Most likely there are others like this
(e.g. HVR1110).
Signed-off-by: Sean Young
---
drivers/media/i2c/ir-kbd-i2c.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index a7e23bcf845c..a14a74e6b986 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -739,6 +739,7 @@ static int ir_probe(struct i2c_client *client, const struct
i2c_device_id *id)
struct rc_dev *rc = NULL;
struct i2c_adapter *adap = client->adapter;
unsigned short addr = client->addr;
+ bool probe_tx = (id->driver_data & FLAG_TX) != 0;
int err;
if ((id->driver_data & FLAG_HDPVR) && !enable_hdpvr) {
@@ -800,6 +801,7 @@ static int ir_probe(struct i2c_client *client, const struct
i2c_device_id *id)
rc_proto= RC_PROTO_BIT_RC5 | RC_PROTO_BIT_RC6_MCE |
RC_PROTO_BIT_RC6_6A_32;
ir_codes= RC_MAP_HAUPPAUGE;
+ probe_tx = true;
break;
}
@@ -892,7 +894,7 @@ static int ir_probe(struct i2c_client *client, const struct
i2c_device_id *id)
INIT_DELAYED_WORK(&ir->work, ir_work);
- if (id->driver_data & FLAG_TX) {
+ if (probe_tx) {
ir->tx_c = i2c_new_dummy(client->adapter, 0x70);
if (!ir->tx_c) {
dev_err(&client->dev, "failed to setup tx i2c address");
--
2.14.3
Re: [PATCH 04/12] intel-ipu3: Add user space ABI definitions
On Thu, Jun 08, 2017 at 01:22:59AM +0300, Sakari Ailus wrote:
> Hi Hans,
>
> On Tue, Jun 06, 2017 at 10:28:26AM +0200, Hans Verkuil wrote:
> > On 05/06/17 22:39, Yong Zhi wrote:
> >
> > Commit message missing.
> >
> > > Signed-off-by: Yong Zhi
> > > ---
> > > include/uapi/linux/intel-ipu3.h | 2182
> > > +++
> > > 1 file changed, 2182 insertions(+)
> > > create mode 100644 include/uapi/linux/intel-ipu3.h
> > >
> > > diff --git a/include/uapi/linux/intel-ipu3.h
> > > b/include/uapi/linux/intel-ipu3.h
> > > new file mode 100644
> > > index 000..9e90aec
> > > --- /dev/null
> > > +++ b/include/uapi/linux/intel-ipu3.h
> > > @@ -0,0 +1,2182 @@
> > > +/*
> > > + * Copyright (c) 2017 Intel Corporation.
> > > + *
> > > + * 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.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#ifndef __IPU3_UAPI_H
> > > +#define __IPU3_UAPI_H
> > > +
> > > +#include
> > > +
> > > +#define IPU3_UAPI_ISP_VEC_ELEMS 64
> > > +
> > > +#define IMGU_ABI_PAD __aligned(IPU3_UAPI_ISP_WORD_BYTES)
> > > +#define IPU3_ALIGN
> > > __attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES)))
> > > +
> > > +#define IPU3_UAPI_ISP_WORD_BYTES 32
> > > +#define IPU3_UAPI_MAX_STRIPES2
> > > +
> > > +/*** ipu3_uapi_stats_3a ***/
> > > +
> > > +#define IPU3_UAPI_MAX_BUBBLE_SIZE10
> > > +
> > > +#define IPU3_UAPI_AE_COLORS 4
> > > +#define IPU3_UAPI_AE_BINS256
> > > +
> > > +#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8
> > > +#define IPU3_UAPI_AWB_MAX_SETS 60
> > > +#define IPU3_UAPI_AWB_SET_SIZE 0x500
> > > +#define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \
> > > + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> > > + IPU3_UAPI_AWB_MD_ITEM_SIZE)
> > > +#define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
> > > + (IPU3_UAPI_AWB_MAX_SETS * \
> > > + (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES))
> > > +
> > > +#define IPU3_UAPI_AF_MAX_SETS24
> > > +#define IPU3_UAPI_AF_MD_ITEM_SIZE4
> > > +#define IPU3_UAPI_AF_SPARE_FOR_BUBBLES \
> > > + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> > > + IPU3_UAPI_AF_MD_ITEM_SIZE)
> > > +#define IPU3_UAPI_AF_Y_TABLE_SET_SIZE0x80
> > > +#define IPU3_UAPI_AF_Y_TABLE_MAX_SIZE \
> > > + (IPU3_UAPI_AF_MAX_SETS * \
> > > + (IPU3_UAPI_AF_Y_TABLE_SET_SIZE + IPU3_UAPI_AF_SPARE_FOR_BUBBLES) * \
> > > + IPU3_UAPI_MAX_STRIPES)
> > > +
> > > +#define IPU3_UAPI_AWB_FR_MAX_SETS24
> > > +#define IPU3_UAPI_AWB_FR_MD_ITEM_SIZE8
> > > +#define IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE 0x100
> > > +#define IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES \
> > > + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> > > + IPU3_UAPI_AWB_FR_MD_ITEM_SIZE)
> > > +#define IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE \
> > > + (IPU3_UAPI_AWB_FR_MAX_SETS * \
> > > + (IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE + \
> > > + IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES) * \
> > > + IPU3_UAPI_MAX_STRIPES)
> > > +
> > > +struct ipu3_uapi_grid_config {
> > > + __u8 width; /* 6 or 7 (rgbs_grd_cfg) bits */
> > > + __u8 height;
> > > + __u16 block_width_log2:3;
> > > + __u16 block_height_log2:3;
> > > + __u16 height_per_slice:8; /* default value 1 */
> > > + __u16 x_start; /* 12 bits */
> > > + __u16 y_start;
> > > +#define IPU3_UAPI_GRID_START_MASK((1 << 12) - 1)
> > > +#define IPU3_UAPI_GRID_Y_START_EN(1 << 15)
> > > + __u16 x_end;/* 12 bits */
> > > + __u16 y_end;
> > > +};
> >
> > You can't use bitfields in a public API. It is up to the compiler to
> > decide how to place bitfields, so this is not a stable API.
>
> We-ell --- it's ABI dependent, yes. The sheer number of structs with bit
> fields in the header will make using the definitions rather cumbersome if
> not unwieldy. Therefore it'd be very nice to continue using bit fields.
>
> There are certainly caveats with bit fields but using them in the user
> space interface is certainly not unforeseen. Just grep under
> /usr/include/linux .
>
> Endianness is a major factor here. That said, the Intel x86 / x86-64
> systems this dr
[PATCH 0/2] Use gpio_is_valid()
Replace the manual validity checks for the GPIO with the gpio_is_valid(). Arvind Yadav (2): [PATCH 1/2] [media] platform: Use gpio_is_valid() [PATCH 2/2] [media] sta2x11: Use gpio_is_valid() and remove unnecessary check drivers/media/pci/sta2x11/sta2x11_vip.c | 31 +++ drivers/media/platform/via-camera.c | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) -- 1.9.1
[PATCH 2/2] [media] sta2x11: Use gpio_is_valid() and remove unnecessary check
Replace the manual validity checks for the GPIO with the
gpio_is_valid().
In vip_gpio_reserve(), Error checking for gpio pin is not correct.
If pwr_pin = -1, It will return 0. This should be return an error.
In sta2x11_vip_init_one(), Error checking for gpio 'reset_pin'
is unnecessary. Because vip_gpio_reserve() is also checking for
valid gpio pin. So removed extra error checking for gpio 'reset_pin'.
Signed-off-by: Arvind Yadav
---
drivers/media/pci/sta2x11/sta2x11_vip.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index dd199bf..069c4a8 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -908,10 +908,10 @@ static int sta2x11_vip_init_controls(struct sta2x11_vip
*vip)
static int vip_gpio_reserve(struct device *dev, int pin, int dir,
const char *name)
{
- int ret;
+ int ret = -ENODEV;
- if (pin == -1)
- return 0;
+ if (!gpio_is_valid(pin))
+ return ret;
ret = gpio_request(pin, name);
if (ret) {
@@ -946,7 +946,7 @@ static int vip_gpio_reserve(struct device *dev, int pin,
int dir,
*/
static void vip_gpio_release(struct device *dev, int pin, const char *name)
{
- if (pin != -1) {
+ if (gpio_is_valid(pin)) {
dev_dbg(dev, "releasing pin %d (%s)\n", pin, name);
gpio_unexport(pin);
gpio_free(pin);
@@ -1003,25 +1003,24 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev,
if (ret)
goto disable;
- if (config->reset_pin >= 0) {
- ret = vip_gpio_reserve(&pdev->dev, config->reset_pin, 0,
- config->reset_name);
- if (ret) {
- vip_gpio_release(&pdev->dev, config->pwr_pin,
-config->pwr_name);
- goto disable;
- }
+ ret = vip_gpio_reserve(&pdev->dev, config->reset_pin, 0,
+ config->reset_name);
+ if (ret) {
+ vip_gpio_release(&pdev->dev, config->pwr_pin,
+config->pwr_name);
+ goto disable;
}
- if (config->pwr_pin != -1) {
+
+ if (gpio_is_valid(config->pwr_pin)) {
/* Datasheet says 5ms between PWR and RST */
usleep_range(5000, 25000);
- ret = gpio_direction_output(config->pwr_pin, 1);
+ gpio_direction_output(config->pwr_pin, 1);
}
- if (config->reset_pin != -1) {
+ if (gpio_is_valid(config->reset_pin)) {
/* Datasheet says 5ms between PWR and RST */
usleep_range(5000, 25000);
- ret = gpio_direction_output(config->reset_pin, 1);
+ gpio_direction_output(config->reset_pin, 1);
}
usleep_range(5000, 25000);
--
1.9.1
[PATCH 1/2] [media] platform: Use gpio_is_valid()
Replace the manual validity checks for the GPIO with the
gpio_is_valid().
Signed-off-by: Arvind Yadav
---
drivers/media/platform/via-camera.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/via-camera.c
b/drivers/media/platform/via-camera.c
index e9a0263..f01c3e8 100644
--- a/drivers/media/platform/via-camera.c
+++ b/drivers/media/platform/via-camera.c
@@ -178,7 +178,7 @@ static int via_sensor_power_setup(struct via_camera *cam)
cam->power_gpio = viafb_gpio_lookup("VGPIO3");
cam->reset_gpio = viafb_gpio_lookup("VGPIO2");
- if (cam->power_gpio < 0 || cam->reset_gpio < 0) {
+ if (!gpio_is_valid(cam->power_gpio) || !gpio_is_valid(cam->reset_gpio))
{
dev_err(&cam->platdev->dev, "Unable to find GPIO lines\n");
return -EINVAL;
}
--
1.9.1
[PATCH 1/2] media: v4l: Add new 2X8 10-bit grayscale media bus code
The code will be called MEDIA_BUS_FMT_Y10_2X8_PADHI_LE. It is similar to MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE but MEDIA_BUS_FMT_Y10_2X8_PADHI_LE describes grayscale data. Signed-off-by: Todor Tomov --- Documentation/media/uapi/v4l/subdev-formats.rst | 72 + include/uapi/linux/media-bus-format.h | 3 +- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst b/Documentation/media/uapi/v4l/subdev-formats.rst index 9fcabe7..c4fb0bf 100644 --- a/Documentation/media/uapi/v4l/subdev-formats.rst +++ b/Documentation/media/uapi/v4l/subdev-formats.rst @@ -4315,6 +4315,78 @@ the following codes. - y\ :sub:`2` - y\ :sub:`1` - y\ :sub:`0` +* .. _MEDIA-BUS-FMT-Y10-2X8-PADHI_LE: + + - MEDIA_BUS_FMT_Y10_2X8_PADHI_LE + - 0x202c + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - y\ :sub:`7` + - y\ :sub:`6` + - y\ :sub:`5` + - y\ :sub:`4` + - y\ :sub:`3` + - y\ :sub:`2` + - y\ :sub:`1` + - y\ :sub:`0` +* - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - 0 + - 0 + - 0 + - 0 + - 0 + - 0 + - y\ :sub:`9` + - y\ :sub:`8` * .. _MEDIA-BUS-FMT-UYVY10-2X10: - MEDIA_BUS_FMT_UYVY10_2X10 diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 9e35117..d6a5a3b 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -62,7 +62,7 @@ #define MEDIA_BUS_FMT_RGB121212_1X36 0x1019 #define MEDIA_BUS_FMT_RGB161616_1X48 0x101a -/* YUV (including grey) - next is 0x202c */ +/* YUV (including grey) - next is 0x202d */ #define MEDIA_BUS_FMT_Y8_1X8 0x2001 #define MEDIA_BUS_FMT_UV8_1X8 0x2015 #define MEDIA_BUS_FMT_UYVY8_1_5X8 0x2002 @@ -74,6 +74,7 @@ #define MEDIA_BUS_FMT_YUYV8_2X80x2008 #define MEDIA_BUS_FMT_YVYU8_2X80x2009 #define MEDIA_BUS_FMT_Y10_1X10 0x200a +#define MEDIA_BUS_FMT_Y10_2X8_PADHI_LE 0x202c #define MEDIA_BUS_FMT_UYVY10_2X10 0x2018 #define MEDIA_BUS_FMT_VYUY10_2X10 0x2019 #define MEDIA_BUS_FMT_YUYV10_2X10 0x200b -- 2.7.4
[PATCH 2/2] media: v4l: Add new 10-bit packed grayscale format
The new format will be called V4L2_PIX_FMT_Y10P.
It is similar to the V4L2_PIX_FMT_SBGGR10P family formats
but V4L2_PIX_FMT_Y10P is a grayscale format.
Signed-off-by: Todor Tomov
---
Documentation/media/uapi/v4l/pixfmt-y10p.rst | 31
Documentation/media/uapi/v4l/yuv-formats.rst | 1 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 1 +
4 files changed, 34 insertions(+)
create mode 100644 Documentation/media/uapi/v4l/pixfmt-y10p.rst
diff --git a/Documentation/media/uapi/v4l/pixfmt-y10p.rst
b/Documentation/media/uapi/v4l/pixfmt-y10p.rst
new file mode 100644
index 000..0018fe7
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-y10p.rst
@@ -0,0 +1,31 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-Y10P:
+
+**
+V4L2_PIX_FMT_Y10P ('Y10P')
+**
+
+Grey-scale image as a MIPI RAW10 packed array
+
+
+Description
+===
+
+This is a packed grey-scale image format with a depth of 10 bits per
+pixel. Every four consecutive pixels are packed into 5 bytes. Each of
+the first 4 bytes contain the 8 high order bits of the pixels, and
+the 5th byte contains the 2 least significants bits of each pixel,
+in the same order.
+
+**Bit-packed representation.**
+
+.. flat-table::
+:header-rows: 0
+:stub-columns: 0
+
+* - Y'\ :sub:`00[9:2]`
+ - Y'\ :sub:`01[9:2]`
+ - Y'\ :sub:`02[9:2]`
+ - Y'\ :sub:`03[9:2]`
+ - Y'\ :sub:`03[1:0]`\ Y'\ :sub:`02[1:0]`\ Y'\ :sub:`01[1:0]`\ Y'\
:sub:`00[1:0]`
diff --git a/Documentation/media/uapi/v4l/yuv-formats.rst
b/Documentation/media/uapi/v4l/yuv-formats.rst
index 3334ea4..9ab0592 100644
--- a/Documentation/media/uapi/v4l/yuv-formats.rst
+++ b/Documentation/media/uapi/v4l/yuv-formats.rst
@@ -29,6 +29,7 @@ to brightness information.
pixfmt-y10
pixfmt-y12
pixfmt-y10b
+pixfmt-y10p
pixfmt-y16
pixfmt-y16-be
pixfmt-y8i
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index f48c505..bdf2399 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1147,6 +1147,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_Y16: descr = "16-bit Greyscale"; break;
case V4L2_PIX_FMT_Y16_BE: descr = "16-bit Greyscale BE"; break;
case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)";
break;
+ case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI
Packed)"; break;
case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale";
break;
case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale";
break;
case V4L2_PIX_FMT_Z16: descr = "16-bit Depth"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877b..b24ab720 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -522,6 +522,7 @@ struct v4l2_pix_format {
/* Grey bit-packed formats */
#define V4L2_PIX_FMT_Y10BPACKv4l2_fourcc('Y', '1', '0', 'B') /* 10
Greyscale bit-packed */
+#define V4L2_PIX_FMT_Y10Pv4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale,
MIPI RAW10 packed */
/* Palette formats */
#define V4L2_PIX_FMT_PAL8v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit
palette */
--
2.7.4
[PATCH 0/2] Add new grayscale formats
The following two patches add one new media bus code and one new pixel format. Both are for 10bit grayscale data. They will be used by the future version of the QComm CAMSS driver when receiving the frame data from the OV7251 grayscale camera sensor. Todor Tomov (2): media: v4l: Add new 2X8 10-bit grayscale media bus code media: v4l: Add new 10-bit packed grayscale format Documentation/media/uapi/v4l/pixfmt-y10p.rst| 31 +++ Documentation/media/uapi/v4l/subdev-formats.rst | 72 + Documentation/media/uapi/v4l/yuv-formats.rst| 1 + drivers/media/v4l2-core/v4l2-ioctl.c| 1 + include/uapi/linux/media-bus-format.h | 3 +- include/uapi/linux/videodev2.h | 1 + 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 Documentation/media/uapi/v4l/pixfmt-y10p.rst -- 2.7.4
Re: [PATCH v2 00/12] media: ov5640: Misc cleanup and improvements
Hi Sam, On Wednesday, 25 April 2018 01:11:19 EEST Sam Bobrowicz wrote: > FYI, still hard at work on this. Did some more experiments last week > that seemed to corroborate the clock tree in the spreadsheet. It also > seems that the output of the P divider cell, SCLK cell and MIPI Rate > cell in the spreadsheet must have a ratio of 2x:1x:8x (respectively) > in order for the sensor to work properly on my platform, and that the > SCLK value must be close to the "rate" variable that you calculate and > pass to set_mipi_pclk. Unfortunately, I've only got the sensor working > well for 1080p@15Hz and 720p@30Hz, both with a SCLK of 42MHz (aka > 84:42:336). I'm running experiments now trying to adjust the htot and > vtot values to create different required rates, and also to try to get > faster Mipi rates working. Any information you have on the > requirements of the htot and vtot values with respect to vact and hact > values would likely be helpful. > > I'm also keeping an eye on the scaler clock, which I think may be > affecting certain resolutions, but haven't been able to see it make a > difference yet (see register 0x3824 and 0x460c) > > I plan on pushing a set of patches once I get this figured out, we can > discuss what I should base them on when I get closer to that point. > I'm new to this process :) I'm also interested in getting the ov5640 driver working with MIPI CSI-2. Studying the datasheet and the driver code, I found the stream on sequence to be a bit weird. In particular the configuration of OV5640_REG_IO_MIPI_CTRL00, OV5640_REG_PAD_OUTPUT00 and OV5640_REG_MIPI_CTRL00 caught my attention. OV5640_REG_IO_MIPI_CTRL00 (0x300e) is set to 0x45 in the large array of init mode data and never touched for MIPI CSI-2 (the register is only touched in ov5640_set_stream_dvp). The value means - mipi_lane_mode: 010 is documented as "debug mode", I would have expected 000 for one lane or 001 for two lanes. - MIPI TX PHY power down: 0 is documented as "debug mode" and 1 as "Power down PHY HS TX", so I suppose 0 is correct. - MIPI RX PHY power down: 0 is documented as "debug mode" and 1 as "Power down PHY LP RX module", so I suppose 0 is correct. I however wonder why there's a RX PHY, it could be a typo. - mipi_en: 1 means MIPI enable, which should be correct. - BIT(0) is undocumented. OV5640_REG_PAD_OUTPUT00 (0x3019) isn't initialized explicitly and thus retains its default value of 0x00, and is controlled when starting and stopping the stream where it's set to 0x00 and 0x70 respectively. Bits 6:4 control the "sleep mode" state of lane 2, lane 1 and clock lane respectively, and should be LP11 in my opinion (that's the PHY stop state). However, setting them to 0x00 when starting the stream mean that LP00 is selected as the sleep state at stream start, and LP11 when stopping the stream. Maybe "sleep mode" means LPDT, but I would expect that to be controlled by the idle status bit in register 0x4800. OV5640_REG_MIPI_CTRL00 (0x4800) is set to 0x04 in the large array of init mode data, and BIT(5) is then cleared at stream on time and set at stream off time. This means: - Clock lane gate enable: Clock lane is free running - Line sync enable: Do not send line short packets for each line (I assume that's LS/LE) - Lane select: Use lane1 as default data lane. - Idle status: MIPI bus will be LP11 when no packet to transmit. I would have expected the idle status to correspond to LPDT, and thus be LP00 (as opposed to the stop state that should be LP11, which I believe is named "sleep mode" in the datasheet and controlled in register 0x3019). BIT(5) is the clock lane gate enable, so at stream on time the clock is set to free running, and at stream off time to "Gate clock lane when no packet to transmit". Couldn't we always enable clock gating ? Do you have any insight on this ? Have you modified the MIPI CSI-2 configuration to get the CSI-2 output working ? -- Regards, Laurent Pinchart
