Re: [GIT PULL for v4.14-rc6] media fixes

[Mauro Carvalho Chehab]

Hi Linus,

Em Mon, 16 Oct 2017 20:15:33 -0400
Linus Torvalds  escreveu:

> On Mon, Oct 16, 2017 at 4:31 PM, Mauro Carvalho Chehab
>  wrote:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
> > media/v4.14-2  
> 
> No such tag.
> 
> Did you forget to push out? The latest tag I see is v4.14-1.

Yes, I forgot to push, sorry[1]!

I just pushed it manually. You should now be able to see the
media/v4.14-2 tag there on my tree.

[1] Actually, my scripts were supposed to push it. I'm out of town for
two weeks (this week in US, next week in EU), so, I'm handling it from
my notebook, instead of using my usual machine. Maybe some setup
differences caused a flaw to it, or maybe the local copy of the script
is outdated. I'll double-check tomorrow to be sure that, if I need 
to do another push before returning home, everything would be just fine.

> I do see the branch (v4l_for_linus) that contains the commit you
> mention, but no tag..

Yep, the branch is identical to what's referenced by the tag.

Thanks,
Mauro


pgpKO5ma4s98B.pgp
Description: Assinatura digital OpenPGP

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Tue Oct 17 05:00:16 CEST 2017
media-tree git hash:7571358dd22dc91dea560f0dde62ce23c033b6b6
media_build git hash:   8a4a9a7064317bd4a687d35ce0537cb5984330e4
v4l-utils git hash: 01c04f7c8ad1a91af33e20621eba9200f447737e
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

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-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: WARNINGS
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: [GIT PULL for v4.14-rc6] media fixes

[Linus Torvalds]

On Mon, Oct 16, 2017 at 4:31 PM, Mauro Carvalho Chehab
 wrote:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
> media/v4.14-2

No such tag.

Did you forget to push out? The latest tag I see is v4.14-1.

I do see the branch (v4l_for_linus) that contains the commit you
mention, but no tag..

  Linus

Re: [GIT PULL for 4.15] Atomisp cleanups

[Mauro Carvalho Chehab]

Em Sat, 14 Oct 2017 01:33:55 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> Here's the usual set of atomisp cleanups.
> 
> Please pull.
> 
> 
> The following changes since commit 8382e556b1a2f30c4bf866f021b33577a64f9ebf:
> 
>   Simplify major/minor non-dynamic logic (2017-10-11 15:32:11 -0400)
> 
> are available in the git repository at:
> 
>   ssh://linuxtv.org/git/sailus/media_tree.git atomisp
> 
> for you to fetch changes up to 9c55caa37197e80b14250ff5709ce49737ed812c:
> 
>   staging: atomisp: use ARRAY_SIZE (2017-10-14 00:55:45 +0300)
> 
> 
> Jérémy Lefaure (1):
>   staging: atomisp: use ARRAY_SIZE
> 
> Muhammad Falak R Wani (1):
>   staging/atomisp: make six local functions static to appease sparse
> 
> Sakari Ailus (3):
>   staging: media: MAINTAINERS: Add entry for atomisp driver

I'd like to see Alan's SOB on this patch.


Cheers,
Mauro

Re: [GIT PULL for 4.15] More sensor driver patches

[Mauro Carvalho Chehab]

Em Sat, 14 Oct 2017 01:23:45 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> Here's the second set of sensor driver patches for 4.15.
> 
> Please pull.
> 
> 
> The following changes since commit 8382e556b1a2f30c4bf866f021b33577a64f9ebf:
> 
>   Simplify major/minor non-dynamic logic (2017-10-11 15:32:11 -0400)
> 
> are available in the git repository at:
> 
>   ssh://linuxtv.org/git/sailus/media_tree.git for-4.15-2
> 
> for you to fetch changes up to 5164fc93c2d8c2e9a2de1461bfba9d6b2911ce9e:
> 
>   imx274: V4l2 driver for Sony imx274 CMOS sensor (2017-10-14 01:06:10 +0300)
> 
> 
> Leon Luo (2):
>   imx274: device tree binding file
>   imx274: V4l2 driver for Sony imx274 CMOS sensor

As checkpatch complained:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#69: 
new file mode 100644

Who will maintain this driver?

Regards,
Mauro



Cheers,
Mauro

[PATCH] media: rga: make some functions static

[Mauro Carvalho Chehab]

drivers/media/platform/rockchip/rga/rga-hw.c:383:6: warning: no previous 
prototype for 'rga_cmd_set' [-Wmissing-prototypes]
 void rga_cmd_set(struct rga_ctx *ctx)
  ^~~
drivers/media/platform/rockchip/rga/rga.c:359:17: warning: no previous 
prototype for 'rga_fmt_find' [-Wmissing-prototypes]
 struct rga_fmt *rga_fmt_find(struct v4l2_format *f)
 ^~~~

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 2 +-
 drivers/media/platform/rockchip/rga/rga.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c 
b/drivers/media/platform/rockchip/rga/rga-hw.c
index 0645481c9a5e..96d1b1b3fe8e 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -380,7 +380,7 @@ static void rga_cmd_set_mode(struct rga_ctx *ctx)
dest[(RGA_MODE_CTRL - RGA_MODE_BASE_REG) >> 2] = mode.val;
 }
 
-void rga_cmd_set(struct rga_ctx *ctx)
+static void rga_cmd_set(struct rga_ctx *ctx)
 {
struct rockchip_rga *rga = ctx->rga;
 
diff --git a/drivers/media/platform/rockchip/rga/rga.c 
b/drivers/media/platform/rockchip/rga/rga.c
index 2cf3bb29a2b3..e7d1b34baf1c 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -356,7 +356,7 @@ struct rga_fmt formats[] = {
 
 #define NUM_FORMATS ARRAY_SIZE(formats)
 
-struct rga_fmt *rga_fmt_find(struct v4l2_format *f)
+static struct rga_fmt *rga_fmt_find(struct v4l2_format *f)
 {
unsigned int i;
 
-- 
2.13.6

[PATCH] staging/atomisp: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab 
Cc: Greg Kroah-Hartman 
Cc: Alan Cox 
Cc: Daeseok Youn 
Cc: Arnd Bergmann 
Cc: linux-media@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Kees Cook 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c  | 13 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h  |  6 +-
 .../media/atomisp/pci/atomisp2/atomisp_compat_css20.c |  2 +-
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 15 +--
 4 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index b0c647f4d250..f3cf4ecba630 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1661,20 +1661,15 @@ void atomisp_css_flush(struct atomisp_device *isp)
dev_dbg(isp->dev, "atomisp css flush done\n");
 }
 
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr)
-#else
-void atomisp_wdt(unsigned long pipe_addr)
-#endif
+void atomisp_wdt(struct timer_list *t)
 {
 #ifndef ISP2401
-   struct atomisp_device *isp = (struct atomisp_device *)isp_addr;
+   struct atomisp_sub_device *asd = from_timer(asd, t, wdt);
 #else
-   struct atomisp_video_pipe *pipe =
-   (struct atomisp_video_pipe *)pipe_addr;
+   struct atomisp_video_pipe *pipe = from_timer(pipe, t, wdt);
struct atomisp_sub_device *asd = pipe->asd;
-   struct atomisp_device *isp = asd->isp;
 #endif
+   struct atomisp_device *isp = asd->isp;
 
 #ifdef ISP2401
atomic_inc(>wdt_count);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
index 31ba4e613d13..4bb83864da2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
@@ -85,11 +85,7 @@ static inline void __iomem 
*atomisp_get_io_virt_addr(unsigned int address)
 void atomisp_msi_irq_init(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_msi_irq_uninit(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_wdt_work(struct work_struct *work);
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr);
-#else
-void atomisp_wdt(unsigned long pipe_addr);
-#endif
+void atomisp_wdt(struct timer_list *t);
 void atomisp_setup_flash(struct atomisp_sub_device *asd);
 irqreturn_t atomisp_isr(int irq, void *dev);
 irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 5027fd20d966..b190f9958066 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4526,7 +4526,7 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
for (i = 0; i < isp->num_of_streams; i++)
atomisp_wdt_stop(>asd[i], 0);
 #ifndef ISP2401
-   atomisp_wdt((unsigned long)isp);
+   atomisp_wdt(>asd[0].wdt);
 #else
queue_work(isp->wdt_work_queue, >wdt_work);
 #endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index e85b3819bffa..20aff7faf44a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1150,17 +1150,12 @@ static int init_atomisp_wdts(struct atomisp_device *isp)
struct atomisp_sub_device *asd = >asd[i];
asd = >asd[i];
 #ifndef ISP2401
-   setup_timer(>wdt, atomisp_wdt, (unsigned long)isp);
+   timer_setup(>wdt, atomisp_wdt, 0);
 #else
-   setup_timer(>video_out_capture.wdt,
-   atomisp_wdt, (unsigned long)>video_out_capture);
-   setup_timer(>video_out_preview.wdt,
-   atomisp_wdt, (unsigned long)>video_out_preview);
-   setup_timer(>video_out_vf.wdt,
-   atomisp_wdt, (unsigned long)>video_out_vf);
-   setup_timer(>video_out_video_capture.wdt,
-   atomisp_wdt,
-   (unsigned long)>video_out_video_capture);
+   timer_setup(>video_out_capture.wdt, atomisp_wdt, 0);
+   timer_setup(>video_out_preview.wdt, atomisp_wdt, 0);
+   timer_setup(>video_out_vf.wdt, atomisp_wdt, 0);
+

[PATCH] media: input: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to
perform its own repeat management. However, there is no path back to the
existing status variable, so add a generic one to the input structure and
use that instead.

Cc: Dmitry Torokhov 
Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Cc: Sakari Ailus 
Cc: Geliang Tang 
Cc: linux-in...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
Acked-by: Pali Rohár 
---
 drivers/input/input.c   | 12 ++--
 drivers/media/pci/ttpci/av7110.h|  1 -
 drivers/media/pci/ttpci/av7110_ir.c | 16 
 include/linux/input.h   |  2 ++
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..497ad2dcb699 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int 
code)
 {
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
-   dev->timer.data) {
+   dev->timer.function) {
dev->repeat_key = code;
mod_timer(>timer,
  jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
  * dev->event_lock here to avoid racing with input_event
  * which may cause keys get "stuck".
  */
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
 {
-   struct input_dev *dev = (void *) data;
+   struct input_dev *dev = from_timer(dev, t, timer);
unsigned long flags;
 
spin_lock_irqsave(>event_lock, flags);
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(>dev);
mutex_init(>mutex);
spin_lock_init(>event_lock);
-   init_timer(>timer);
+   timer_setup(>timer, NULL, 0);
INIT_LIST_HEAD(>h_list);
INIT_LIST_HEAD(>node);
 
@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device 
*dev, void *res)
  */
 void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
 {
-   dev->timer.data = (unsigned long) dev;
-   dev->timer.function = input_repeat_key;
+   dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+   dev->timer_data = 0;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
 }
diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h
index 347827925c14..b98a4f3006df 100644
--- a/drivers/media/pci/ttpci/av7110.h
+++ b/drivers/media/pci/ttpci/av7110.h
@@ -93,7 +93,6 @@ struct infrared {
u8  inversion;
u16 last_key;
u16 last_toggle;
-   u8  delay_timer_finished;
 };
 
 
diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
b/drivers/media/pci/ttpci/av7110_ir.c
index ca05198de2c2..a883caa6488c 100644
--- a/drivers/media/pci/ttpci/av7110_ir.c
+++ b/drivers/media/pci/ttpci/av7110_ir.c
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
if (timer_pending(>keyup_timer)) {
del_timer(>keyup_timer);
if (ir->last_key != keycode || toggle != ir->last_toggle) {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
-   } else if (ir->delay_timer_finished) {
+   } else if (ir->input_dev->timer_data) {
input_event(ir->input_dev, EV_KEY, keycode, 2);
input_sync(ir->input_dev);
}
} else {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
}
@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
 
 
 /* called by the input driver after rep[REP_DELAY] ms */
-static void input_repeat_key(unsigned long parm)
+static void input_repeat_key(struct timer_list *t)
 {
-   struct infrared *ir = (struct infrared *) parm;
+   struct input_dev *dev = from_timer(dev, t, timer);
 
-   ir->delay_timer_finished = 1;
+   /* Key repeat started */
+   

[PATCH] media: serial_ir: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Sean Young 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/rc/serial_ir.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c
index 8b66926bc16a..8bf5637b3a69 100644
--- a/drivers/media/rc/serial_ir.c
+++ b/drivers/media/rc/serial_ir.c
@@ -470,7 +470,7 @@ static int hardware_init_port(void)
return 0;
 }
 
-static void serial_ir_timeout(unsigned long arg)
+static void serial_ir_timeout(struct timer_list *unused)
 {
DEFINE_IR_RAW_EVENT(ev);
 
@@ -540,8 +540,7 @@ static int serial_ir_probe(struct platform_device *dev)
 
serial_ir.rcdev = rcdev;
 
-   setup_timer(_ir.timeout_timer, serial_ir_timeout,
-   (unsigned long)_ir);
+   timer_setup(_ir.timeout_timer, serial_ir_timeout, 0);
 
result = devm_request_irq(>dev, irq, serial_ir_irq_handler,
  share_irq ? IRQF_SHARED : 0,
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH] media: tc358743: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mats Randgaard 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/i2c/tc358743.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a9355032076f..359f63d7dfca 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1481,9 +1481,9 @@ static irqreturn_t tc358743_irq_handler(int irq, void 
*dev_id)
return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
-static void tc358743_irq_poll_timer(unsigned long arg)
+static void tc358743_irq_poll_timer(struct timer_list *t)
 {
-   struct tc358743_state *state = (struct tc358743_state *)arg;
+   struct tc358743_state *state = from_timer(state, t, timer);
unsigned int msecs;
 
schedule_work(>work_i2c_poll);
@@ -2147,8 +2147,7 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(>work_i2c_poll,
  tc358743_work_i2c_poll);
-   setup_timer(>timer, tc358743_irq_poll_timer,
-   (unsigned long)state);
+   timer_setup(>timer, tc358743_irq_poll_timer, 0);
state->timer.expires = jiffies +
   msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(>timer);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH] media: saa7134: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab 
Cc: Sakari Ailus 
Cc: Sean Young 
Cc: Geliang Tang 
Cc: Hans Verkuil 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/pci/saa7134/saa7134-core.c  | 6 +++---
 drivers/media/pci/saa7134/saa7134-input.c | 9 -
 drivers/media/pci/saa7134/saa7134-ts.c| 3 +--
 drivers/media/pci/saa7134/saa7134-vbi.c   | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c | 3 +--
 drivers/media/pci/saa7134/saa7134.h   | 2 +-
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c 
b/drivers/media/pci/saa7134/saa7134-core.c
index 7976c5a12ca8..9e76de2411ae 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -338,9 +338,9 @@ void saa7134_buffer_next(struct saa7134_dev *dev,
}
 }
 
-void saa7134_buffer_timeout(unsigned long data)
+void saa7134_buffer_timeout(struct timer_list *t)
 {
-   struct saa7134_dmaqueue *q = (struct saa7134_dmaqueue *)data;
+   struct saa7134_dmaqueue *q = from_timer(q, t, timeout);
struct saa7134_dev *dev = q->dev;
unsigned long flags;
 
@@ -378,7 +378,7 @@ void saa7134_stop_streaming(struct saa7134_dev *dev, struct 
saa7134_dmaqueue *q)
}
}
spin_unlock_irqrestore(>slock, flags);
-   saa7134_buffer_timeout((unsigned long)q); /* also calls 
del_timer(>timeout) */
+   saa7134_buffer_timeout(>timeout); /* also calls 
del_timer(>timeout) */
 }
 EXPORT_SYMBOL_GPL(saa7134_stop_streaming);
 
diff --git a/drivers/media/pci/saa7134/saa7134-input.c 
b/drivers/media/pci/saa7134/saa7134-input.c
index 9337e4615519..2d5abeddc079 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -447,10 +447,10 @@ void saa7134_input_irq(struct saa7134_dev *dev)
}
 }
 
-static void saa7134_input_timer(unsigned long data)
+static void saa7134_input_timer(struct timer_list *t)
 {
-   struct saa7134_dev *dev = (struct saa7134_dev *)data;
-   struct saa7134_card_ir *ir = dev->remote;
+   struct saa7134_card_ir *ir = from_timer(ir, t, timer);
+   struct saa7134_dev *dev = ir->dev->priv;
 
build_key(dev);
mod_timer(>timer, jiffies + msecs_to_jiffies(ir->polling));
@@ -507,8 +507,7 @@ static int __saa7134_ir_start(void *priv)
ir->running = true;
 
if (ir->polling) {
-   setup_timer(>timer, saa7134_input_timer,
-   (unsigned long)dev);
+   timer_setup(>timer, saa7134_input_timer, 0);
ir->timer.expires = jiffies + HZ;
add_timer(>timer);
}
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c 
b/drivers/media/pci/saa7134/saa7134-ts.c
index 7414878af9e0..2be703617e29 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -223,8 +223,7 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
dev->ts.nr_packets = ts_nr_packets;
 
INIT_LIST_HEAD(>ts_q.queue);
-   setup_timer(>ts_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>ts_q));
+   timer_setup(>ts_q.timeout, saa7134_buffer_timeout, 0);
dev->ts_q.dev  = dev;
dev->ts_q.need_two = 1;
dev->ts_started= 0;
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c 
b/drivers/media/pci/saa7134/saa7134-vbi.c
index bcad9b2d9bb3..af494c6147f1 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -181,8 +181,7 @@ struct vb2_ops saa7134_vbi_qops = {
 int saa7134_vbi_init1(struct saa7134_dev *dev)
 {
INIT_LIST_HEAD(>vbi_q.queue);
-   setup_timer(>vbi_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>vbi_q));
+   timer_setup(>vbi_q.timeout, saa7134_buffer_timeout, 0);
dev->vbi_q.dev  = dev;
 
if (vbibufs < 2)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..82d2a24644e4 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2145,8 +2145,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
dev->automute   = 0;
 
INIT_LIST_HEAD(>video_q.queue);
-   setup_timer(>video_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>video_q));
+   timer_setup(>video_q.timeout, saa7134_buffer_timeout, 0);
dev->video_q.dev  = dev;
dev->fmt = format_by_fourcc(V4L2_PIX_FMT_BGR24);
dev->width= 720;
diff --git 

[PATCH] media: dvb-core: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab 
Cc: devendra sharma 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/dvb-core/dmxdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 18e4230865be..3ddd44e1ee77 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -329,9 +329,9 @@ static int dvb_dmxdev_set_buffer_size(struct dmxdev_filter 
*dmxdevfilter,
return 0;
 }
 
-static void dvb_dmxdev_filter_timeout(unsigned long data)
+static void dvb_dmxdev_filter_timeout(struct timer_list *t)
 {
-   struct dmxdev_filter *dmxdevfilter = (struct dmxdev_filter *)data;
+   struct dmxdev_filter *dmxdevfilter = from_timer(dmxdevfilter, t, timer);
 
dmxdevfilter->buffer.error = -ETIMEDOUT;
spin_lock_irq(>dev->lock);
@@ -346,8 +346,6 @@ static void dvb_dmxdev_filter_timer(struct dmxdev_filter 
*dmxdevfilter)
 
del_timer(>timer);
if (para->timeout) {
-   dmxdevfilter->timer.function = dvb_dmxdev_filter_timeout;
-   dmxdevfilter->timer.data = (unsigned long)dmxdevfilter;
dmxdevfilter->timer.expires =
jiffies + 1 + (HZ / 2 + HZ * para->timeout) / 1000;
add_timer(>timer);
@@ -754,7 +752,7 @@ static int dvb_demux_open(struct inode *inode, struct file 
*file)
dvb_ringbuffer_init(>buffer, NULL, 8192);
dmxdevfilter->type = DMXDEV_TYPE_NONE;
dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
-   init_timer(>timer);
+   timer_setup(>timer, dvb_dmxdev_filter_timeout, 0);
 
dvbdev->users++;
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH] media: saa7146: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Hans Verkuil 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/common/saa7146/saa7146_fops.c  | 4 ++--
 drivers/media/common/saa7146/saa7146_vbi.c   | 3 +--
 drivers/media/common/saa7146/saa7146_video.c | 3 +--
 include/media/drv-intf/saa7146_vv.h  | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index c4664f0da874..8c87d6837c49 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -163,9 +163,9 @@ void saa7146_buffer_next(struct saa7146_dev *dev,
}
 }
 
-void saa7146_buffer_timeout(unsigned long data)
+void saa7146_buffer_timeout(struct timer_list *t)
 {
-   struct saa7146_dmaqueue *q = (struct saa7146_dmaqueue*)data;
+   struct saa7146_dmaqueue *q = from_timer(q, t, timeout);
struct saa7146_dev *dev = q->dev;
unsigned long flags;
 
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index ae66c2325228..7fa3147c2d7e 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -366,8 +366,7 @@ static void vbi_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 
INIT_LIST_HEAD(>vbi_dmaq.queue);
 
-   setup_timer(>vbi_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>vbi_dmaq));
+   timer_setup(>vbi_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->vbi_dmaq.dev  = dev;
 
init_waitqueue_head(>vbi_wq);
diff --git a/drivers/media/common/saa7146/saa7146_video.c 
b/drivers/media/common/saa7146/saa7146_video.c
index 51eeed830de4..2b631eaa65b3 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1201,8 +1201,7 @@ static void video_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 {
INIT_LIST_HEAD(>video_dmaq.queue);
 
-   setup_timer(>video_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>video_dmaq));
+   timer_setup(>video_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->video_dmaq.dev  = dev;
 
/* set some default values */
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 926c5b145279..17bbe3851d75 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -184,7 +184,7 @@ int saa7146_unregister_device(struct video_device *vid, 
struct saa7146_dev *dev)
 void saa7146_buffer_finish(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q, int state);
 void saa7146_buffer_next(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q,int vbi);
 int saa7146_buffer_queue(struct saa7146_dev *dev, struct saa7146_dmaqueue *q, 
struct saa7146_buf *buf);
-void saa7146_buffer_timeout(unsigned long data);
+void saa7146_buffer_timeout(struct timer_list *t);
 void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
struct saa7146_buf *buf);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH] media: tvaudio: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/i2c/tvaudio.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index ce86534450ac..16a1e08ce06c 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -300,9 +300,9 @@ static int chip_cmd(struct CHIPSTATE *chip, char *name, 
audiocmd *cmd)
  *   if available, ...
  */
 
-static void chip_thread_wake(unsigned long data)
+static void chip_thread_wake(struct timer_list *t)
 {
-   struct CHIPSTATE *chip = (struct CHIPSTATE*)data;
+   struct CHIPSTATE *chip = from_timer(chip, t, wt);
wake_up_process(chip->thread);
 }
 
@@ -1995,7 +1995,7 @@ static int tvaudio_probe(struct i2c_client *client, const 
struct i2c_device_id *
v4l2_ctrl_handler_setup(>hdl);
 
chip->thread = NULL;
-   init_timer(>wt);
+   timer_setup(>wt, chip_thread_wake, 0);
if (desc->flags & CHIP_NEED_CHECKMODE) {
if (!desc->getrxsubchans || !desc->setaudmode) {
/* This shouldn't be happen. Warn user, but keep working
@@ -2005,8 +2005,6 @@ static int tvaudio_probe(struct i2c_client *client, const 
struct i2c_device_id *
return 0;
}
/* start async thread */
-   chip->wt.function = chip_thread_wake;
-   chip->wt.data = (unsigned long)chip;
chip->thread = kthread_run(chip_thread, chip, "%s",
   client->name);
if (IS_ERR(chip->thread)) {
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH] media/saa7146: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer to
hold the timer's target file, as there won't be a way to pass this in the
future.

Cc: Hans Verkuil 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/common/saa7146/saa7146_fops.c | 2 +-
 drivers/media/common/saa7146/saa7146_vbi.c  | 9 +
 include/media/drv-intf/saa7146_vv.h | 1 +
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index 930d2c94d5d3..c4664f0da874 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -559,7 +559,7 @@ int saa7146_vv_init(struct saa7146_dev* dev, struct 
saa7146_ext_vv *ext_vv)
vbi->start[1] = 312;
vbi->count[1] = 16;
 
-   init_timer(>vbi_read_timeout);
+   timer_setup(>vbi_read_timeout, NULL, 0);
 
vv->ov_fb.capability = V4L2_FBUF_CAP_LIST_CLIPPING;
vv->ov_fb.flags = V4L2_FBUF_FLAG_PRIMARY;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index 69525ca4f52c..ae66c2325228 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -348,9 +348,10 @@ static void vbi_stop(struct saa7146_fh *fh, struct file 
*file)
spin_unlock_irqrestore(>slock, flags);
 }
 
-static void vbi_read_timeout(unsigned long data)
+static void vbi_read_timeout(struct timer_list *t)
 {
-   struct file *file = (struct file*)data;
+   struct saa7146_vv *vv = from_timer(vv, t, vbi_read_timeout);
+   struct file *file = vv->vbi_read_timeout_file;
struct saa7146_fh *fh = file->private_data;
struct saa7146_dev *dev = fh->dev;
 
@@ -401,8 +402,8 @@ static int vbi_open(struct saa7146_dev *dev, struct file 
*file)
sizeof(struct saa7146_buf),
file, >v4l2_lock);
 
-   vv->vbi_read_timeout.function = vbi_read_timeout;
-   vv->vbi_read_timeout.data = (unsigned long)file;
+   vv->vbi_read_timeout.function = (TIMER_FUNC_TYPE)vbi_read_timeout;
+   vv->vbi_read_timeout_file = file;
 
/* initialize the brs */
if ( 0 != (SAA7146_USE_PORT_B_FOR_VBI & dev->ext_vv_data->flags)) {
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 736f4f2d8290..926c5b145279 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -107,6 +107,7 @@ struct saa7146_vv
struct saa7146_dmaqueue vbi_dmaq;
struct v4l2_vbi_format  vbi_fmt;
struct timer_list   vbi_read_timeout;
+   struct file *vbi_read_timeout_file;
/* vbi workaround interrupt queue */
wait_queue_head_t   vbi_wq;
int vbi_fieldcount;
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] stagin: atomisp: Fix oops by unbalanced clk enable/disable call

[Tobin C. Harding]

On Mon, Oct 16, 2017 at 02:34:48PM +0200, Hans de Goede wrote:
> The common-clk core expects clk consumers to always call enable/disable
> in a balanced manner. The atomisp driver does not call gmin_flisclk_ctrl()
> in a balanced manner, so add a clock_on bool and skip redundant calls.
> 
> This fixes kernel oops like this one:
> 
> [   19.811613] gc0310_s_config S
> [   19.811655] [ cut here ]
> [   19.811664] WARNING: CPU: 1 PID: 720 at drivers/clk/clk.c:594 
> clk_core_disabl
> [   19.811666] Modules linked in: tpm_crb(+) snd_soc_sst_atom_hifi2_platform 
> tpm
> [   19.811744] CPU: 1 PID: 720 Comm: systemd-udevd Tainted: G C OE   
> 4.1
> [   19.811746] Hardware name: Insyde T701/T701, BIOS BYT70A.YNCHENG.WIN.007 
> 08/2
> [   19.811749] task: 988df7ab2500 task.stack: ac1400474000
> [   19.811752] RIP: 0010:clk_core_disable+0xc0/0x130
> ...
> [   19.811775] Call Trace:
> [   19.811783]  clk_core_disable_lock+0x1f/0x30
> [   19.811788]  clk_disable+0x1f/0x30
> [   19.811794]  gmin_flisclk_ctrl+0x87/0xf0
> [   19.811801]  0xc0528512
> [   19.811805]  0xc05295e2
> [   19.811811]  ? acpi_device_wakeup_disable+0x50/0x60
> [   19.811815]  ? acpi_dev_pm_attach+0x8e/0xd0
> [   19.811818]  ? 0xc05294d0
> [   19.811823]  i2c_device_probe+0x1cd/0x280
> [   19.811828]  driver_probe_device+0x2ff/0x450
> 
> Fixes: "staging: atomisp: use clock framework for camera clocks"
> Signed-off-by: Hans de Goede 
> ---
>  .../media/atomisp/platform/intel-mid/atomisp_gmin_platform.c   | 7 
> +++
>  1 file changed, 7 insertions(+)
> 
> diff --git 
> a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
> b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> index 828fe5abd832..6671ebe4ecc9 100644
> --- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> @@ -29,6 +29,7 @@ struct gmin_subdev {
>   struct v4l2_subdev *subdev;
>   int clock_num;
>   int clock_src;
> + bool clock_on;
>   struct clk *pmc_clk;
>   struct gpio_desc *gpio0;
>   struct gpio_desc *gpio1;
> @@ -583,6 +584,9 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev *subdev, 
> int on)
>   struct gmin_subdev *gs = find_gmin_subdev(subdev);
>   struct i2c_client *client = v4l2_get_subdevdata(subdev);
>  
> + if (gs->clock_on == !!on)
> + return 0;
> +
>   if (on) {
>   ret = clk_set_rate(gs->pmc_clk, gs->clock_src);

Which tree [and branch] are you working off please? In the staging-next branch 
of Greg's staging
tree this function does not appear as it is in this patch.

thanks,
Tobin.

[GIT PULL for v4.14-rc6] media fixes

[Mauro Carvalho Chehab]

Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
media/v4.14-2

For the following media core fixes:

   - cec: Respond to unregistered initiators, when applicable
   - dvb_frontend: only use kref after initialized

and the following driver-specific fixes:

   - qcom, camss: Make function vfe_set_selection static
   - qcom: VIDEO_QCOM_CAMSS should depend on HAS_DMA
   - s5p-cec: add NACK detection support
   - media: staging/imx: Fix uninitialized variable warning
   - dib3000mc: i2c transfers over usb cannot be done from stack
   - venus: init registered list on streamoff

Thanks!
Mauro

-

The following changes since commit 1efdf1776e2253b77413c997bed862410e4b6aaf:

  media: leds: as3645a: add V4L2_FLASH_LED_CLASS dependency (2017-09-05 
16:32:45 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
media/v4.14-2

for you to fetch changes up to ead666000a5fe34bdc82d61838e4df2d416ea15e:

  media: dvb_frontend: only use kref after initialized (2017-10-11 12:47:36 
-0400)


media fixes for v4.14-rc6


Colin Ian King (1):
  media: qcom: camss: Make function vfe_set_selection static

Geert Uytterhoeven (1):
  media: platform: VIDEO_QCOM_CAMSS should depend on HAS_DMA

Hans Verkuil (1):
  media: s5p-cec: add NACK detection support

Jose Abreu (1):
  media: cec: Respond to unregistered initiators, when applicable

Laurent Pinchart (1):
  media: staging/imx: Fix uninitialized variable warning

Mauro Carvalho Chehab (1):
  media: dvb_frontend: only use kref after initialized

Sean Young (1):
  media: dvb: i2c transfers over usb cannot be done from stack

Stanimir Varbanov (1):
  media: venus: init registered list on streamoff

 drivers/media/cec/cec-adap.c   | 13 +++--
 drivers/media/dvb-core/dvb_frontend.c  | 25 +++--
 drivers/media/dvb-frontends/dib3000mc.c| 50 ++
 drivers/media/dvb-frontends/dvb-pll.c  | 22 ++--
 drivers/media/platform/Kconfig |  2 +-
 drivers/media/platform/qcom/camss-8x16/camss-vfe.c |  2 +-
 drivers/media/platform/qcom/venus/helpers.c|  1 +
 .../media/platform/s5p-cec/exynos_hdmi_cecctrl.c   |  3 +-
 drivers/media/platform/s5p-cec/s5p_cec.c   | 11 +++-
 drivers/media/platform/s5p-cec/s5p_cec.h   |  2 +
 drivers/media/tuners/mt2060.c  | 59 +-
 drivers/staging/media/imx/imx-media-dev.c  |  4 +-
 12 files changed, 153 insertions(+), 41 deletions(-)


-- 

Cheers,
Mauro

Re: Exynos MFC issues on 4.14-rc4

[Shuah Khan]

On Mon, Oct 16, 2017 at 7:11 AM, Marek Szyprowski
 wrote:
> Hi Marian,
>
> On 2017-10-12 02:49, Marian Mihailescu wrote:
>>
>> I've been testing 4.14-rc4 on Odroid-XU4, and here's a kernel
>> complaint when running:
>>
>> gst-launch-1.0 filesrc location=bunny_trailer_1080p.mov ! parsebin !
>> v4l2video4dec capture-io-mode=dmabuf ! v4l2video6convert
>> output-io-mode=dmabuf-import capture-io-mode=dmabuf ! kmssink
>>
>> http://paste.debian.net/990353/
>
>
> This is rather harmless and it happens on v4.14-rcX, because LOCKDEP has
> been enabled by default in the exynos_defconfig. For more information
> see https://lkml.org/lkml/2017/10/13/974
>
>> PS: on kernel 4.9 patched with MFC & GSC updates (almost up to date
>> with 4.14 I think) there was no "Wrong buffer/video queue type (1)"
>> message either
>
>
> I will check it and let you know if this is something we should worry about.

I am seeing this messages. It appears to be harmless. However, it
would be good to look into this. It doesn't appear to cause any
problems for capture/output.

thanks,
-- Shuah

Re: [PATCH v2 2/2] media: s5p-mfc: fix lockdep warning

[Hans Verkuil]

On 10/16/2017 05:16 PM, Shuah Khan wrote:
> The driver mmap functions shouldn't take lock when calling vb2_mmap().
> Fix it to not take the lock.
> 
> Reference: commit log for f035eb4e976ef5a059e30bc91cfd310ff030a7d3
> and e752577ed7bf55c81e10343fced8b378cda2b63b
> 
> The following lockdep warning is fixed with this change.
> 
> [ 2106.181412] ==
> [ 2106.187563] WARNING: possible circular locking dependency detected
> [ 2106.193718] 4.14.0-rc2-2-gfab205f-dirty #4 Not tainted
> [ 2106.199175] --
> [ 2106.205328] qtdemux0:sink/2614 is trying to acquire lock:
> [ 2106.210701]  (>mfc_mutex){+.+.}, at: [] 
> s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [ 2106.218672]
> [ 2106.218672] but task is already holding lock:
> [ 2106.224477]  (>mmap_sem){}, at: [] 
> vm_mmap_pgoff+0x44/0xb8
> [ 2106.231497]
> [ 2106.231497] which lock already depends on the new lock.
> [ 2106.231497]
> [ 2106.239642]
> [ 2106.239642] the existing dependency chain (in reverse order) is:
> [ 2106.247095]
> [ 2106.247095] -> #1 (>mmap_sem){}:
> [ 2106.252473]__might_fault+0x80/0xb0
> [ 2106.256567]video_usercopy+0x1cc/0x510 [videodev]
> [ 2106.261845]v4l2_ioctl+0xa4/0xdc [videodev]
> [ 2106.266596]do_vfs_ioctl+0xa0/0xa18
> [ 2106.270667]SyS_ioctl+0x34/0x5c
> [ 2106.274395]ret_fast_syscall+0x0/0x28
> [ 2106.278637]
> [ 2106.278637] -> #0 (>mfc_mutex){+.+.}:
> [ 2106.284186]lock_acquire+0x6c/0x88
> [ 2106.288173]__mutex_lock+0x68/0xa34
> [ 2106.292244]mutex_lock_interruptible_nested+0x1c/0x24
> [ 2106.297893]s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [ 2106.302747]v4l2_mmap+0x54/0x88 [videodev]
> [ 2106.307409]mmap_region+0x3a8/0x638
> [ 2106.311480]do_mmap+0x330/0x3a4
> [ 2106.315207]vm_mmap_pgoff+0x90/0xb8
> [ 2106.319279]SyS_mmap_pgoff+0x90/0xc0
> [ 2106.323439]ret_fast_syscall+0x0/0x28
> [ 2106.327683]
> [ 2106.327683] other info that might help us debug this:
> [ 2106.327683]
> [ 2106.335656]  Possible unsafe locking scenario:
> [ 2106.335656]
> [ 2106.341548]CPU0CPU1
> [ 2106.346053]
> [ 2106.350559]   lock(>mmap_sem);
> [ 2106.353939]lock(>mfc_mutex);
> [ 2106.353939]lock(>mfc_mutex);
> [ 2106.365897]   lock(>mfc_mutex);
> [ 2106.369450]
> [ 2106.369450]  *** DEADLOCK ***
> [ 2106.369450]
> [ 2106.375344] 1 lock held by qtdemux0:sink/2614:
> [ 2106.379762]  #0:  (>mmap_sem){}, at: [] 
> vm_mmap_pgoff+0x44/0xb8
> [ 2106.387214]
> [ 2106.387214] stack backtrace:
> [ 2106.391550] CPU: 7 PID: 2614 Comm: qtdemux0:sink Not tainted 
> 4.14.0-rc2-2-gfab205f-dirty #4
> [ 2106.400213] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 2106.406285] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [ 2106.413995] [] (show_stack) from [] 
> (dump_stack+0x98/0xc4)
> [ 2106.421187] [] (dump_stack) from [] 
> (print_circular_bug+0x254/0x410)
> [ 2106.429245] [] (print_circular_bug) from [] 
> (check_prev_add+0x468/0x938)
> [ 2106.437651] [] (check_prev_add) from [] 
> (__lock_acquire+0x1314/0x14fc)
> [ 2106.445883] [] (__lock_acquire) from [] 
> (lock_acquire+0x6c/0x88)
> [ 2106.453596] [] (lock_acquire) from [] 
> (__mutex_lock+0x68/0xa34)
> [ 2106.461221] [] (__mutex_lock) from [] 
> (mutex_lock_interruptible_nested+0x1c/0x24)
> [ 2106.470425] [] (mutex_lock_interruptible_nested) from 
> [] (s5p_mfc_mmap+0x28/0xd4 [s5p_mfc])
> [ 2106.480494] [] (s5p_mfc_mmap [s5p_mfc]) from [] 
> (v4l2_mmap+0x54/0x88 [videodev])
> [ 2106.489575] [] (v4l2_mmap [videodev]) from [] 
> (mmap_region+0x3a8/0x638)
> [ 2106.497875] [] (mmap_region) from [] 
> (do_mmap+0x330/0x3a4)
> [ 2106.505068] [] (do_mmap) from [] 
> (vm_mmap_pgoff+0x90/0xb8)
> [ 2106.512260] [] (vm_mmap_pgoff) from [] 
> (SyS_mmap_pgoff+0x90/0xc0)
> [ 2106.520059] [] (SyS_mmap_pgoff) from [] 
> (ret_fast_syscall+0x0/0x28)
> 
> Signed-off-by: Shuah Khan 
> Suggested-by: Hans Verkuil 
> Acked-by: Marek Szyprowski 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 4c253fb..40c18b4 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1047,8 +1047,6 @@ static int s5p_mfc_mmap(struct file *file, struct 
> vm_area_struct *vma)
>   unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
>   int ret;
>  
> - if (mutex_lock_interruptible(>mfc_mutex))
> - return -ERESTARTSYS;
>   if (offset < DST_QUEUE_OFF_BASE) {
>   mfc_debug(2, "mmaping source\n");
>

Re: [PATCH v2 1/2] media: exynos-gsc: fix lockdep warning

[Hans Verkuil]

On 10/16/2017 05:16 PM, Shuah Khan wrote:
> The driver mmap functions shouldn't take lock when calling vb2_mmap().
> Fix it to not take the lock.
> 
> Reference: commit log for f035eb4e976ef5a059e30bc91cfd310ff030a7d3
> and e752577ed7bf55c81e10343fced8b378cda2b63b
> 
> The following lockdep warning is fixed with this change.
> 
> [ 1990.972058] ==
> [ 1990.978172] WARNING: possible circular locking dependency detected
> [ 1990.984327] 4.14.0-rc2-2-gfab205f-dirty #4 Not tainted
> [ 1990.989783] --
> [ 1990.995937] qtdemux0:sink/2765 is trying to acquire lock:
> [ 1991.001309]  (>lock){+.+.}, at: [] gsc_m2m_mmap+0x24/0x5c 
> [exynos_gsc]
> [ 1991.009108]
>but task is already holding lock:
> [ 1991.014913]  (>mmap_sem){}, at: [] 
> vm_mmap_pgoff+0x44/0xb8
> [ 1991.021932]
>which lock already depends on the new lock.
> 
> [ 1991.030078]
>the existing dependency chain (in reverse order) is:
> [ 1991.037530]
>-> #1 (>mmap_sem){}:
> [ 1991.042913]__might_fault+0x80/0xb0
> [ 1991.047096]video_usercopy+0x1cc/0x510 [videodev]
> [ 1991.052297]v4l2_ioctl+0xa4/0xdc [videodev]
> [ 1991.057036]do_vfs_ioctl+0xa0/0xa18
> [ 1991.061102]SyS_ioctl+0x34/0x5c
> [ 1991.064834]ret_fast_syscall+0x0/0x28
> [ 1991.069072]
>-> #0 (>lock){+.+.}:
> [ 1991.074193]lock_acquire+0x6c/0x88
> [ 1991.078179]__mutex_lock+0x68/0xa34
> [ 1991.082247]mutex_lock_interruptible_nested+0x1c/0x24
> [ 1991.087888]gsc_m2m_mmap+0x24/0x5c [exynos_gsc]
> [ 1991.093029]v4l2_mmap+0x54/0x88 [videodev]
> [ 1991.097673]mmap_region+0x3a8/0x638
> [ 1991.101743]do_mmap+0x330/0x3a4
> [ 1991.105470]vm_mmap_pgoff+0x90/0xb8
> [ 1991.109542]SyS_mmap_pgoff+0x90/0xc0
> [ 1991.113702]ret_fast_syscall+0x0/0x28
> [ 1991.117945]
>other info that might help us debug this:
> 
> [ 1991.125918]  Possible unsafe locking scenario:
> 
> [ 1991.131810]CPU0CPU1
> [ 1991.136315]
> [ 1991.140821]   lock(>mmap_sem);
> [ 1991.144201]lock(>lock);
> [ 1991.149833]lock(>mmap_sem);
> [ 1991.155725]   lock(>lock);
> [ 1991.158845]
> *** DEADLOCK ***
> 
> [ 1991.164740] 1 lock held by qtdemux0:sink/2765:
> [ 1991.169157]  #0:  (>mmap_sem){}, at: [] 
> vm_mmap_pgoff+0x44/0xb8
> [ 1991.176609]
>stack backtrace:
> [ 1991.180946] CPU: 2 PID: 2765 Comm: qtdemux0:sink Not tainted 
> 4.14.0-rc2-2-gfab205f-dirty #4
> [ 1991.189608] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 1991.195686] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [ 1991.203393] [] (show_stack) from [] 
> (dump_stack+0x98/0xc4)
> [ 1991.210586] [] (dump_stack) from [] 
> (print_circular_bug+0x254/0x410)
> [ 1991.218644] [] (print_circular_bug) from [] 
> (check_prev_add+0x468/0x938)
> [ 1991.227049] [] (check_prev_add) from [] 
> (__lock_acquire+0x1314/0x14fc)
> [ 1991.235281] [] (__lock_acquire) from [] 
> (lock_acquire+0x6c/0x88)
> [ 1991.242993] [] (lock_acquire) from [] 
> (__mutex_lock+0x68/0xa34)
> [ 1991.250620] [] (__mutex_lock) from [] 
> (mutex_lock_interruptible_nested+0x1c/0x24)
> [ 1991.259812] [] (mutex_lock_interruptible_nested) from 
> [] (gsc_m2m_mmap+0x24/0x5c [exynos_gsc])
> [ 1991.270159] [] (gsc_m2m_mmap [exynos_gsc]) from [] 
> (v4l2_mmap+0x54/0x88 [videodev])
> [ 1991.279510] [] (v4l2_mmap [videodev]) from [] 
> (mmap_region+0x3a8/0x638)
> [ 1991.287792] [] (mmap_region) from [] 
> (do_mmap+0x330/0x3a4)
> [ 1991.294986] [] (do_mmap) from [] 
> (vm_mmap_pgoff+0x90/0xb8)
> [ 1991.302178] [] (vm_mmap_pgoff) from [] 
> (SyS_mmap_pgoff+0x90/0xc0)
> [ 1991.309977] [] (SyS_mmap_pgoff) from [] 
> (ret_fast_syscall+0x0/0x28)
> 
> Signed-off-by: Shuah Khan 
> Suggested-by: Hans Verkuil 
> Acked-by: Marek Szyprowski 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c 
> b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> index 2a2994e..722d7c4 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> @@ -726,14 +726,9 @@ static unsigned int gsc_m2m_poll(struct file *file,
>  static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>   struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> - struct gsc_dev *gsc = ctx->gsc_dev;
>   int ret;
>  
> - if (mutex_lock_interruptible(>lock))
> - return -ERESTARTSYS;
> -
>   ret = v4l2_m2m_mmap(file, 

Re: [PATCH v4 04/21] doc: media/v4l-drivers: Add Qualcomm Camera Subsystem driver document

[Daniel Mack]

Hi,

On 28.08.2017 09:10, Todor Tomov wrote:
> On 25.08.2017 17:10, Daniel Mack wrote:
>> Could you explain how ISPIF, CSID and CSIPHY are related?
>>
>> I have a userspace test setup that works fine for USB webcams, but when
>> operating on any of the video devices exposed by this driver, the
>> lowlevel functions such as .s_power of the ISPIF, CSID, CSIPHY and the
>> sensor driver layers aren't called into.
> 
> Have you activated the media controller links? The s_power is called
> when the subdev is part of a pipeline in which the video device node
> is opened. You can see example configurations for the Qualcomm CAMSS
> driver on:
> https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/Guides/CameraModule.md
> This will probably answer most of your questions.

It did in fact, yes. Thanks again for the pointer.

I am however struggling getting a 4-lane OV13855 camera to work with
this camss driver, and I'd be happy to hear about similar setups that work.

In short, here's what my setup looks like:

1. I wrote a driver for the OV13855 sensor, based on the one for OV13858
but with updated register values. It announces
MEDIA_BUS_FMT_SBGGR10_1X10 as bus format which is what the sensor should
be sending, if I understand the specs correctly.


2. The DTS snippet for the endpoint connection look like this:

_i2c6 {
cam0: ov13855@16 {
/* ... */
port {
cam0_ep: endpoint {
clock-lanes = <1>;
data-lanes = <0 2 3 4>;
remote-endpoint = <_ep>;
};
};
};
};

 {
ports {
port@0 {
reg = <0>;
csiphy0_ep: endpoint {
clock-lanes = <1>;
data-lanes = <0 2 3 4>;
remote-endpoint = <_ep>;
};
};
};
};

There are also no lane swaps or any intermediate components in hardware.
We've checked the electrical bits many times, and that end seems alright.


3. The pads and links are set up like this:

# media-ctl -d /dev/media0 -l
'"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":1->"msm_ispif0":0[1],"msm_ispif0":1->"msm_vfe0_rdi0":0[1]'

# media-ctl -d /dev/media0 -V '"ov13855
1-0010":0[fmt:SBGGR10_1X10/4224x3136
field:none],"msm_csiphy0":0[fmt:SBGGR10_1X10/4224x3136
field:none],"msm_csid0":0[fmt:SBGGR10_1X10/4224x3136
field:none],"msm_ispif0":0[fmt:SBGGR10_1X10/4224x3136
field:none],"msm_vfe0_rdi0":0[fmt:SBGGR10_1X10/4224x3136 field:none]'

Both commands succeed.


4. When streaming is started, the power consumption of the device goes
up, all necessary external clocks and voltages are provided and are
stable, and I can see a continuous stream of data on all 4 MIPI lanes
using an oscilloscope.


5. Capturing frames with the following yavta command doesn't work
though. The task is mostly stuck in the buffer dequeing ioctl:

# yavta -B capture-mplane -c10 -I -n 5 -f SBGGR10P -s 4224x3136 /dev/video0

vfe_isr() does fire sometimes with VFE_0_IRQ_STATUS_1_RDIn_SOF(0) set,
but very occasionally only, and the frames do not contain data.

FWIW, an ov6540 is connected to port 1 of the camss, and this sensor
works fine.

I'd be grateful for any pointer about what I could investigate on.


Thanks,
Daniel

Re: [PATCH resend] [media] uvcvideo: zero seq number when disabling stream

[Laurent Pinchart]

Hi Hans,

(CC'ing Guennadi Liakhovetski)

Thank you for the patch.

On Friday, 15 September 2017 09:27:51 EEST Hans Yang wrote:
> For bulk-based devices, when disabling the video stream,
> in addition to issue CLEAR_FEATURE(HALT), it is better to set
> alternate setting 0 as well or the sequnce number in host
> side will probably not reset to zero.

The USB 2.0 specificatin states in the description of the SET_INTERFACE 
request that "If a device only supports a default setting for the specified 
interface, then a STALL may be returned in the Status stage of the request".

The Linux implementation of usb_set_interface() deals with this by handling 
STALL conditions and manually clearing HALT for all endpoints in the 
interface, but I'm still concerned that this change could break existing bulk-
based cameras. Do you know what other operating systems do when disabling the 
stream on bulk cameras ? According to a comment in the driver Windows calls 
CLEAR_FEATURE(HALT), but the situation might have changed since that was 
tested.

Guennadi, how do your bulk-based cameras handle this ?

> Then in next time video stream start, the device will expect
> host starts packet from 0 sequence number but host actually
> continue the sequence number from last transaction and this
> causes transaction errors.

Do you mean the DATA0/DATA1 toggle ? Why does the host continue toggling the 
PID from the last transation ? The usb_clear_halt() function calls 
usb_reset_endpoint() which should reset the DATA PID toggle.

> This commit fixes this by adding set alternate setting 0 back
> as what isoch-based devices do.
> 
> Below error message will also be eliminated for some devices:
> uvcvideo: Non-zero status (-71) in video completion handler.
> 
> Signed-off-by: Hans Yang 
> ---
>  drivers/media/usb/uvc/uvc_video.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index fb86d6af398d..ad80c2a6da6a 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1862,10 +1862,9 @@ int uvc_video_enable(struct uvc_streaming *stream,
> int enable)
> 
>   if (!enable) {
>   uvc_uninit_video(stream, 1);
> - if (stream->intf->num_altsetting > 1) {
> - usb_set_interface(stream->dev->udev,
> + usb_set_interface(stream->dev->udev,
> stream->intfnum, 0);
> - } else {
> + if (stream->intf->num_altsetting == 1) {
>   /* UVC doesn't specify how to inform a bulk-based device
>* when the video stream is stopped. Windows sends a
>* CLEAR_FEATURE(HALT) request to the video streaming

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] fix lockdep warnings in s5p_mfc and exynos-gsc vb2 drivers

[Shuah Khan]

Hi Marek,

On 10/16/2017 06:48 AM, Marek Szyprowski wrote:
> Hi Shuah,
> 
> On 2017-10-14 01:13, Shuah Khan wrote:
>> Driver mmap functions shouldn't hold lock when calling vb2_mmap(). The
>> vb2_mmap() function has its own lock that it uses to protect the critical
>> section.
>>
>> Reference: commit log for f035eb4e976ef5a059e30bc91cfd310ff030a7d3
> 
> It would make sense to add the information about the reference commit to each
> commit message and also point to commit 
> e752577ed7bf55c81e10343fced8b378cda2b63b,
> as it is exactly the same case here. Anyway:

I think It does make sense to add the commit information to each commit message.
I can do that send v2.

> 
> Acked-by: Marek Szyprowski 

Thanks.

> 
> I wonder if makes sense to send those patches also to sta...@vget.kernel.org
> (maybe v4.3+, like the mentioned above commit, if they really apply?).

I don't believe they will apply as is. I can work back-porting once these get 
into
the mainline.

> 
>> Shuah Khan (2):
>>media: exynos-gsc: fix lockdep warning
>>media: s5p-mfc: fix lockdep warning
>>
>>   drivers/media/platform/exynos-gsc/gsc-m2m.c | 5 -
>>   drivers/media/platform/s5p-mfc/s5p_mfc.c| 3 ---
>>   2 files changed, 8 deletions(-)
>>
> 
> Best regards

thanks,
-- Shuah

Re: [PATCH v3 1/2] media: i2c: OV5647: ensure clock lane in LP-11 state before streaming on

[Sakari Ailus]

On Mon, Oct 16, 2017 at 03:19:09PM +0100, Luis Oliveira wrote:
> Hi all,
> 
> Sorry for the delay and thank you for the fix.
> I just checked the databook and the changes makes sense.
> 
> cheers,
> Luis
> 
> On 16-Oct-17 13:23, Sakari Ailus wrote:
> > Luis,
> > 
> > Any comment on these?
> > 
> > On Sun, Oct 01, 2017 at 06:22:37PM +0800, Jacob Chen wrote:
> >> When I was supporting Rpi Camera Module on the ASUS Tinker board,
> >> I found this driver have some issues with rockchip's mipi-csi driver.
> >> It didn't place clock lane in LP-11 state before performing
> >> D-PHY initialisation.
> >>
> >> From our experience, on some OV sensors,
> >> LP-11 state is not achieved while BIT(5)-0x4800 is cleared.
> >>
> >> So let's set BIT(5) and BIT(0) both while not streaming, in order to
> >> coax the clock lane into LP-11 state.
> >>
> >> 0x4800 : MIPI CTRL 00
> >>BIT(5) : clock lane gate enable
> >>0: continuous
> >>1: none-continuous
> >>BIT(0) : manually set clock lane
> >>0: Not used
> >>1: used
> >>
> >> Signed-off-by: Jacob Chen 
> >> ---
> >>  drivers/media/i2c/ov5647.c | 13 -
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> >> index 95ce90fdb876..247302d01f53 100644
> >> --- a/drivers/media/i2c/ov5647.c
> >> +++ b/drivers/media/i2c/ov5647.c
> >> @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> >>  {
> >>int ret;
> >>  
> >> +  ret = ov5647_write(sd, 0x4800, 0x04);
> >> +  if (ret < 0)
> >> +  return ret;
> >> +
> >>ret = ov5647_write(sd, 0x4202, 0x00);
> >>if (ret < 0)
> >>return ret;
> >> @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
> >>  {
> >>int ret;
> >>  
> >> +  ret = ov5647_write(sd, 0x4800, 0x25);
> >> +  if (ret < 0)
> >> +  return ret;
> >> +
> >>ret = ov5647_write(sd, 0x4202, 0x0f);
> >>if (ret < 0)
> >>return ret;
> >> @@ -320,7 +328,10 @@ static int __sensor_init(struct v4l2_subdev *sd)
> >>return ret;
> >>}
> >>  
> >> -  return ov5647_write(sd, 0x4800, 0x04);
> >> +  /*
> >> +   * stream off to make the clock lane into LP-11 state.
> >> +   */
> >> +  return ov5647_stream_off(sd);
> >>  }
> >>  
> >>  static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
> >> -- 
> >> 2.14.1
> >>
> > 
> Reviewed-by: Luis Oliveira 

Thanks. I'll apply the patches then.

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

Re: [PATCH v1] [media] uvcvideo: mark buffer error where overflow

[Laurent Pinchart]

Hi Baoyou,

Thank you for the patch.

On Thursday, 7 September 2017 05:59:48 EEST Baoyou Xie wrote:
> Some cameras post inaccurate frame where next frame data overlap
> it. this results in screen flicker, and it need to be prevented.
> 
> So this patch marks the buffer error to discard the frame where
> buffer overflow.

I've thought about this before and I wasn't sure how to handle this case. As 
such an overflow might not signal an erroneous buffer, as the buffer could 
contain a valid image. However, if you have seen erroneous buffer contents in 
this case, and given that overflows should not occur, I think we could decide 
to stay on the safe side and set the error flag.

> Signed-off-by: Baoyou Xie 

Reviewed-by: Laurent Pinchart 

I'll apply the patch to my tree.

> ---
>  drivers/media/usb/uvc/uvc_video.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index fb86d6a..81a3530 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1077,6 +1077,7 @@ static void uvc_video_decode_data(struct uvc_streaming
> *stream, /* Complete the current frame if the buffer size was exceeded. */
> if (len > maxlen) {
>   uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow).\n");
> + buf->error = 1;
>   buf->state = UVC_BUF_STATE_READY;
>   }
>  }


-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/2] media: i2c: OV5647: ensure clock lane in LP-11 state before streaming on

[Luis Oliveira]

Hi all,

Sorry for the delay and thank you for the fix.
I just checked the databook and the changes makes sense.

cheers,
Luis

On 16-Oct-17 13:23, Sakari Ailus wrote:
> Luis,
> 
> Any comment on these?
> 
> On Sun, Oct 01, 2017 at 06:22:37PM +0800, Jacob Chen wrote:
>> When I was supporting Rpi Camera Module on the ASUS Tinker board,
>> I found this driver have some issues with rockchip's mipi-csi driver.
>> It didn't place clock lane in LP-11 state before performing
>> D-PHY initialisation.
>>
>> From our experience, on some OV sensors,
>> LP-11 state is not achieved while BIT(5)-0x4800 is cleared.
>>
>> So let's set BIT(5) and BIT(0) both while not streaming, in order to
>> coax the clock lane into LP-11 state.
>>
>> 0x4800 : MIPI CTRL 00
>>  BIT(5) : clock lane gate enable
>>  0: continuous
>>  1: none-continuous
>>  BIT(0) : manually set clock lane
>>  0: Not used
>>  1: used
>>
>> Signed-off-by: Jacob Chen 
>> ---
>>  drivers/media/i2c/ov5647.c | 13 -
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
>> index 95ce90fdb876..247302d01f53 100644
>> --- a/drivers/media/i2c/ov5647.c
>> +++ b/drivers/media/i2c/ov5647.c
>> @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
>>  {
>>  int ret;
>>  
>> +ret = ov5647_write(sd, 0x4800, 0x04);
>> +if (ret < 0)
>> +return ret;
>> +
>>  ret = ov5647_write(sd, 0x4202, 0x00);
>>  if (ret < 0)
>>  return ret;
>> @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
>>  {
>>  int ret;
>>  
>> +ret = ov5647_write(sd, 0x4800, 0x25);
>> +if (ret < 0)
>> +return ret;
>> +
>>  ret = ov5647_write(sd, 0x4202, 0x0f);
>>  if (ret < 0)
>>  return ret;
>> @@ -320,7 +328,10 @@ static int __sensor_init(struct v4l2_subdev *sd)
>>  return ret;
>>  }
>>  
>> -return ov5647_write(sd, 0x4800, 0x04);
>> +/*
>> + * stream off to make the clock lane into LP-11 state.
>> + */
>> +return ov5647_stream_off(sd);
>>  }
>>  
>>  static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
>> -- 
>> 2.14.1
>>
> 
Reviewed-by: Luis Oliveira 

Re: [PATCH] [media] stk-webcam: Fix use after free on disconnect

[Sakari Ailus]

On Fri, Sep 22, 2017 at 04:48:41PM +0300, Dan Carpenter wrote:
> We free the stk_camera device too early.  It's allocate first in probe
> and it should be freed last in stk_camera_disconnect().
> 
> Reported-by: Andrey Konovalov 
> Signed-off-by: Dan Carpenter 
> ---
> Not tested but these bug reports seem surprisingly straight forward.
> Thanks Andrey!

Acked-by: Sakari Ailus 

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

Re: Exynos MFC issues on 4.14-rc4

[Marek Szyprowski]

Hi Marian,

On 2017-10-12 02:49, Marian Mihailescu wrote:

I've been testing 4.14-rc4 on Odroid-XU4, and here's a kernel
complaint when running:

gst-launch-1.0 filesrc location=bunny_trailer_1080p.mov ! parsebin !
v4l2video4dec capture-io-mode=dmabuf ! v4l2video6convert
output-io-mode=dmabuf-import capture-io-mode=dmabuf ! kmssink

http://paste.debian.net/990353/


This is rather harmless and it happens on v4.14-rcX, because LOCKDEP has
been enabled by default in the exynos_defconfig. For more information
see https://lkml.org/lkml/2017/10/13/974


PS: on kernel 4.9 patched with MFC & GSC updates (almost up to date
with 4.14 I think) there was no "Wrong buffer/video queue type (1)"
message either


I will check it and let you know if this is something we should worry about.

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

Re: [PATCH v14 20/28] v4l: fwnode: Add a helper function to obtain device / integer references

[Sakari Ailus]

Hi Hans,

On Tue, Oct 10, 2017 at 03:07:29PM +0200, Hans Verkuil wrote:
> On 10/10/2017 01:27 PM, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Oct 09, 2017 at 02:06:55PM +0200, Hans Verkuil wrote:
> >> Hi Sakari,
> >>
> >> My reply here is also valid for v15.
> >>
> >> On 26/09/17 13:30, Sakari Ailus wrote:
> >>> Hi Hans,
> >>>
> >>> Thanks for the review.
> >>>
> >>> On Tue, Sep 26, 2017 at 10:47:40AM +0200, Hans Verkuil wrote:
>  On 26/09/17 00:25, Sakari Ailus wrote:
> > v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that 
> > under
> > the device's own fwnode, it will follow child fwnodes with the given
> > property-value pair and return the resulting fwnode.
> >
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 201 
> > ++
> >  1 file changed, 201 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index f739dfd16cf7..f93049c361e4 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -578,6 +578,207 @@ static int v4l2_fwnode_reference_parse(
> > return ret;
> >  }
> >  
> > +/*
> > + * v4l2_fwnode_reference_get_int_prop - parse a reference with integer
> > + * arguments
> > + * @dev: struct device pointer
> > + * @notifier: notifier for @dev
> > + * @prop: the name of the property
> > + * @index: the index of the reference to get
> > + * @props: the array of integer property names
> > + * @nprops: the number of integer property names in @nprops
> > + *
> > + * Find fwnodes referred to by a property @prop, then under that
> > + * iteratively, @nprops times, follow each child node which has a
> > + * property in @props array at a given child index the value of which
> > + * matches the integer argument at an index.
> 
>  "at an index". Still makes no sense to me. Which index?
> >>>
> >>> How about this:
> >>>
> >>> First find an fwnode referred to by the reference at @index in @prop.
> >>>
> >>> Then under that fwnode, @nprops times, for each property in @props,
> >>> iteratively follow child nodes starting from fwnode such that they have 
> >>> the
> >>> property in @props array at the index of the child node distance from the
> >>
> >> distance? You mean 'instance'?
> > 
> > No. It's a tree structure: this is the distance between a node in the tree
> > and the root node (i.e. device's fwnode).
> > 
> >>
> >>> root node and the value of that property matching with the integer 
> >>> argument of
> >>> the reference, at the same index.
> >>
> >> You've completely lost me. About halfway through this sentence my brain 
> >> crashed :-)
> > 
> > :-D
> > 
> > Did keeping distance have any effect?
> 
> No :-)
> 
> "the index of the child node distance from the root node": I have absolutely
> no idea how to interpret that.

This index is referring to the properties array and its value is the same
as the distance of the child node from the device's root node.

> 
> > 
> >>
> >>>
> 
> > + *
> > + * For example, if this function was called with arguments and values
> > + * @dev corresponding to device "SEN", @prop == "flash-leds", @index
> > + * == 1, @props == { "led" }, @nprops == 1, with the ASL snippet below
> > + * it would return the node marked with THISONE. The @dev argument in
> > + * the ASL below.
> 
>  I know I asked for this before, but can you change the example to one 
>  where
>  nprops = 2? I think that will help understanding this.
> >>>
> >>> I could do that but then the example no longer corresponds to any actual
> >>> case that exists at the moment. LED nodes will use a single integer
> >>> argument and lens-focus nodes none.
> >>
> >> So? The example is here to understand the code and it doesn't have to be
> >> related to actual hardware for a mainlined driver.
> > 
> > This isn't about hardware, the definitions being parsed currently aren't
> > specific to any single piece of hardware. I could add an example which does
> > not exist, that's certainly possible. But I fail to see how it'd help
> > while the contrary could well be the case.
> 
> It helps to relate the code (and the comments for that matter) to what is in
> the ACPI. In fact, if you can make such an example, then I can see if I can
> come up with a better description.

Hmm. I thought about the example, and figured out the graph data structure
could be parsed using this function as well. From
Documentation/acpi/dsd/graph.txt:

Scope (\_SB.PCI0.I2C2)
{
Device (CAM0)
{
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () { 

Re: [PATCH 0/2] fix lockdep warnings in s5p_mfc and exynos-gsc vb2 drivers

[Marek Szyprowski]

Hi Shuah,

On 2017-10-14 01:13, Shuah Khan wrote:

Driver mmap functions shouldn't hold lock when calling vb2_mmap(). The
vb2_mmap() function has its own lock that it uses to protect the critical
section.

Reference: commit log for f035eb4e976ef5a059e30bc91cfd310ff030a7d3


It would make sense to add the information about the reference commit to 
each
commit message and also point to commit 
e752577ed7bf55c81e10343fced8b378cda2b63b,

as it is exactly the same case here. Anyway:

Acked-by: Marek Szyprowski 

I wonder if makes sense to send those patches also to sta...@vget.kernel.org
(maybe v4.3+, like the mentioned above commit, if they really apply?).


Shuah Khan (2):
   media: exynos-gsc: fix lockdep warning
   media: s5p-mfc: fix lockdep warning

  drivers/media/platform/exynos-gsc/gsc-m2m.c | 5 -
  drivers/media/platform/s5p-mfc/s5p_mfc.c| 3 ---
  2 files changed, 8 deletions(-)



Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

[PATCH] stagin: atomisp: Fix oops by unbalanced clk enable/disable call

[Hans de Goede]

The common-clk core expects clk consumers to always call enable/disable
in a balanced manner. The atomisp driver does not call gmin_flisclk_ctrl()
in a balanced manner, so add a clock_on bool and skip redundant calls.

This fixes kernel oops like this one:

[   19.811613] gc0310_s_config S
[   19.811655] [ cut here ]
[   19.811664] WARNING: CPU: 1 PID: 720 at drivers/clk/clk.c:594 clk_core_disabl
[   19.811666] Modules linked in: tpm_crb(+) snd_soc_sst_atom_hifi2_platform tpm
[   19.811744] CPU: 1 PID: 720 Comm: systemd-udevd Tainted: G C OE   4.1
[   19.811746] Hardware name: Insyde T701/T701, BIOS BYT70A.YNCHENG.WIN.007 08/2
[   19.811749] task: 988df7ab2500 task.stack: ac1400474000
[   19.811752] RIP: 0010:clk_core_disable+0xc0/0x130
...
[   19.811775] Call Trace:
[   19.811783]  clk_core_disable_lock+0x1f/0x30
[   19.811788]  clk_disable+0x1f/0x30
[   19.811794]  gmin_flisclk_ctrl+0x87/0xf0
[   19.811801]  0xc0528512
[   19.811805]  0xc05295e2
[   19.811811]  ? acpi_device_wakeup_disable+0x50/0x60
[   19.811815]  ? acpi_dev_pm_attach+0x8e/0xd0
[   19.811818]  ? 0xc05294d0
[   19.811823]  i2c_device_probe+0x1cd/0x280
[   19.811828]  driver_probe_device+0x2ff/0x450

Fixes: "staging: atomisp: use clock framework for camera clocks"
Signed-off-by: Hans de Goede 
---
 .../media/atomisp/platform/intel-mid/atomisp_gmin_platform.c   | 7 +++
 1 file changed, 7 insertions(+)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 828fe5abd832..6671ebe4ecc9 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -29,6 +29,7 @@ struct gmin_subdev {
struct v4l2_subdev *subdev;
int clock_num;
int clock_src;
+   bool clock_on;
struct clk *pmc_clk;
struct gpio_desc *gpio0;
struct gpio_desc *gpio1;
@@ -583,6 +584,9 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev *subdev, 
int on)
struct gmin_subdev *gs = find_gmin_subdev(subdev);
struct i2c_client *client = v4l2_get_subdevdata(subdev);
 
+   if (gs->clock_on == !!on)
+   return 0;
+
if (on) {
ret = clk_set_rate(gs->pmc_clk, gs->clock_src);
 
@@ -591,8 +595,11 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev *subdev, 
int on)
gs->clock_src);
 
ret = clk_prepare_enable(gs->pmc_clk);
+   if (ret == 0)
+   gs->clock_on = true;
} else {
clk_disable_unprepare(gs->pmc_clk);
+   gs->clock_on = false;
}
 
return ret;
-- 
2.14.2

Re: [PATCH v3 1/2] media: i2c: OV5647: ensure clock lane in LP-11 state before streaming on

[Sakari Ailus]

Luis,

Any comment on these?

On Sun, Oct 01, 2017 at 06:22:37PM +0800, Jacob Chen wrote:
> When I was supporting Rpi Camera Module on the ASUS Tinker board,
> I found this driver have some issues with rockchip's mipi-csi driver.
> It didn't place clock lane in LP-11 state before performing
> D-PHY initialisation.
> 
> From our experience, on some OV sensors,
> LP-11 state is not achieved while BIT(5)-0x4800 is cleared.
> 
> So let's set BIT(5) and BIT(0) both while not streaming, in order to
> coax the clock lane into LP-11 state.
> 
> 0x4800 : MIPI CTRL 00
>   BIT(5) : clock lane gate enable
>   0: continuous
>   1: none-continuous
>   BIT(0) : manually set clock lane
>   0: Not used
>   1: used
> 
> Signed-off-by: Jacob Chen 
> ---
>  drivers/media/i2c/ov5647.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 95ce90fdb876..247302d01f53 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
>  {
>   int ret;
>  
> + ret = ov5647_write(sd, 0x4800, 0x04);
> + if (ret < 0)
> + return ret;
> +
>   ret = ov5647_write(sd, 0x4202, 0x00);
>   if (ret < 0)
>   return ret;
> @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
>  {
>   int ret;
>  
> + ret = ov5647_write(sd, 0x4800, 0x25);
> + if (ret < 0)
> + return ret;
> +
>   ret = ov5647_write(sd, 0x4202, 0x0f);
>   if (ret < 0)
>   return ret;
> @@ -320,7 +328,10 @@ static int __sensor_init(struct v4l2_subdev *sd)
>   return ret;
>   }
>  
> - return ov5647_write(sd, 0x4800, 0x04);
> + /*
> +  * stream off to make the clock lane into LP-11 state.
> +  */
> + return ov5647_stream_off(sd);
>  }
>  
>  static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
> -- 
> 2.14.1
> 

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

Re: [PATCH] media: vb2: unify calling of set_page_dirty_lock

[Sakari Ailus]

On Sun, Oct 15, 2017 at 07:09:24PM -0400, Nicolas Dufresne wrote:
> Le dimanche 15 octobre 2017 à 23:40 +0300, Sakari Ailus a écrit :
> > Hi Nicolas,
> > 
> > On Tue, Oct 10, 2017 at 11:40:10AM -0400, Nicolas Dufresne wrote:
> > > Le mardi 29 août 2017 à 14:26 +0300, Stanimir Varbanov a écrit :
> > > > Currently videobuf2-dma-sg checks for dma direction for
> > > > every single page and videobuf2-dc lacks any dma direction
> > > > checks and calls set_page_dirty_lock unconditionally.
> > > > 
> > > > Thus unify and align the invocations of set_page_dirty_lock
> > > > for videobuf2-dc, videobuf2-sg  memory allocators with
> > > > videobuf2-vmalloc, i.e. the pattern used in vmalloc has been
> > > > copied to dc and dma-sg.
> > > 
> > > Just before we go too far in "doing like vmalloc", I would like to
> > > share this small video that display coherency issues when rendering
> > > vmalloc backed DMABuf over various KMS/DRM driver. I can reproduce
> > > this
> > > easily with Intel and MSM display drivers using UVC or Vivid as
> > > source.
> > > 
> > > The following is an HDMI capture of the following GStreamer
> > > pipeline
> > > running on Dragonboard 410c.
> > > 
> > > gst-launch-1.0 -v v4l2src device=/dev/video2 ! video/x-
> > > raw,format=NV16,width=1280,height=720 ! kmssink
> > > https://people.collabora.com/~nicolas/vmalloc-issue.mov
> > > 
> > > Feedback on this issue would be more then welcome. It's not clear
> > > to me
> > > who's bug is this (v4l2, drm or iommu). The software is unlikely to
> > > be
> > > blamed as this same pipeline works fine with non-vmalloc based
> > > sources.
> > 
> > Could you elaborate this a little bit more? Which Intel CPU do you
> > have
> > there?
> 
> I have tested with Skylake and Ivy Bridge and on Dragonboard 410c
> (Qualcomm APQ8016 SoC) (same visual artefact)

Do you still have both corrupted and uncompressed frames around? Those
would be interesting to look at.

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

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Hans Verkuil]

Hi Alexandre,

Thank you very much for working on this. Much appreciated!

I only did a very high-level review of the patch series: there is not much
point IMHO of doing a detailed review given the upcoming discussions in
Prague. It's better to wait until we agree with the high-level API.

Regarding the public API: the ioctls seem sane. It's all very similar to the
other implementations we've seen.

I'm still not sure about the name 'job', but this is 'just' a naming issue.

The part where I have more doubts is the need to create a new device node.

For the upcoming meeting I would like to discuss whether this cannot be added
to the media API.

Originally the plan was that the media API would be subsystem-agnostic and could
also be used by ALSA/DRM/etc. This never happened and I also am not aware of any
movement in that area.

I am wondering whether we should just be realistic and abandon the 'subsystem
agnostic' part and be willing to add e.g. the job support to the media API.

We can also consider controlling sub-devices via the media device node instead
of creating separate v4l-subdev device nodes.

This does not necessarily preclude the use of the media device by other
subsystems, but it certainly ties it much closer to the media subsystem. On the
other hand, it does make life easier for us (and I like easy!).

This is just a brainstorm at the moment, but I am interested what others think
of making /dev/mediaX specific to the media subsystem (at least we chose the
right name for that device node!).

Obviously, if we go into that direction, then that will have an obvious impact
on the jobs API, especially if we want to be able to control subdevs through the
media device instead of through the v4l-subdev devices.

Regards,

Hans

On 09/28/2017 11:50 AM, Alexandre Courbot wrote:
> Hi everyone,
> 
> Here is a new attempt at the "request" (which I propose to rename "jobs") API
> for V4L2, hopefully in a manner that can converge to something that will be
> merged. The core ideas should be easy to grasp for those familiar with the
> previous attemps, yet there are a few important differences.
> 
> Most notably, user-space does not need to explicitly allocate and manage
> requests/jobs (but still can if this makes sense). We noticed that only 
> specific
> use-cases require such an explicit management, and opted for a jobs queue that
> controls the flow of work over a set of opened devices. This should simplify
> user-space code quite a bit, while still retaining the ability to manage 
> states
> explicitly like the previous request API proposals allowed to do.
> 
> The jobs API defines a few new concepts that user-space can use to control the
> workflow on a set of opened V4L2 devices:
> 
> A JOB QUEUE can be created from a set of opened FDs that are part of a 
> pipeline
> and need to cooperate (be it capture, m2m, or media controller devices).
> 
> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
> then submitted to the job queue. Once the job queue schedules the job, its
> parameters (controls, etc) are applied to the devices of the queue, and itsd
> buffers are processed. Immediately after a job is submitted, the next job is
> ready to be set up without further user action.
> 
> Once a job completes, it must be dequeued and user-space can then read back 
> its
> properties (notably controls) at completion time.
> 
> Internally, the state of jobs is managed through STATE HANDLERS. Each driver
> supporting the jobs API needs to specify an implementation of a state handler.
> Fortunately, most drivers can rely on the generic state handler implementation
> that simply records and replays a job's parameter using standard V4L2 
> functions.
> Thanks to this, adding jobs API support to a driver relying on the control
> framework and vb2 only requires a dozen lines of codes.
> 
> Drivers with specific needs or opportunities for optimization can however
> provide their own implementation of a state handler. This may in particular be
> beneficial for hardware that supports configuration or command buffers 
> (thinking
> about VSP1 here).
> 
> This is still very early work, and focus has been on the following points:
> 
> * Provide something that anybody can test (currently using vim2m and vivid),
> * Reuse the current V4L2 APIs as much as possible,
> * Remain flexible enough to accomodate the inevitable changes that will be
>   requested,
> * Keep line count low, even if functionality is missing at the moment.
> 
> Please keep this in mind while going through the patches. In particular, at 
> the
> moment the parameters of a job are limited to integer controls. I know that 
> much
> more is expected, but V4L2 has quite a learning curve and I preferred to focus
> on the general concepts for now. More is coming though! :)
> 
> I have written two small example programs that demonstrate the use of this 
> API:
> 
> - With a codec device (vim2m): 
> 

Re: [RFC PATCH 2/9] [media] v4l2-core: add core jobs API support

[Hans Verkuil]

On 09/28/2017 11:50 AM, Alexandre Courbot wrote:
> Add core support code for jobs API. This manages the life cycle of jobs
> and creation of a jobs queue, as well as the interface for job states.
> 
> It also exposes the user-space jobs API.
> 
> Signed-off-by: Alexandre Courbot 
> ---
>  drivers/media/v4l2-core/Makefile|   3 +-
>  drivers/media/v4l2-core/v4l2-dev.c  |   6 +
>  drivers/media/v4l2-core/v4l2-jobqueue-dev.c | 173 +++
>  drivers/media/v4l2-core/v4l2-jobqueue.c | 764 
> 
>  include/media/v4l2-dev.h|   4 +
>  include/media/v4l2-fh.h |   4 +
>  include/media/v4l2-job-state.h  |  75 +++
>  include/media/v4l2-jobqueue-dev.h   |  24 +
>  include/media/v4l2-jobqueue.h   |  54 ++
>  include/uapi/linux/v4l2-jobs.h  |  40 ++
>  include/uapi/linux/videodev2.h  |   2 +
>  11 files changed, 1148 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
>  create mode 100644 include/media/v4l2-job-state.h
>  create mode 100644 include/media/v4l2-jobqueue-dev.h
>  create mode 100644 include/media/v4l2-jobqueue.h
>  create mode 100644 include/uapi/linux/v4l2-jobs.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile 
> b/drivers/media/v4l2-core/Makefile
> index 098ad5fd5231..a717bb8f1a25 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -6,7 +6,8 @@ tuner-objs:=  tuner-core.o
>  
>  videodev-objs:=  v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o 
> \
>   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> - v4l2-async.o
> + v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
> +
>  ifeq ($(CONFIG_COMPAT),y)
>videodev-objs += v4l2-compat-ioctl32.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
> b/drivers/media/v4l2-core/v4l2-dev.c
> index 5a7063886c93..fb229b671b9d 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define VIDEO_NUM_DEVICES256
>  #define VIDEO_NAME  "video4linux"
> @@ -1058,6 +1059,10 @@ static int __init videodev_init(void)
>   return -EIO;
>   }
>  
> + ret = v4l2_jobqueue_device_init();
> + if (ret < 0)
> + printk(KERN_WARNING "video_dev: channel initialization 
> failed\n");
> +
>   return 0;
>  }
>  
> @@ -1065,6 +1070,7 @@ static void __exit videodev_exit(void)
>  {
>   dev_t dev = MKDEV(VIDEO_MAJOR, 0);
>  
> + v4l2_jobqueue_device_exit();
>   class_unregister(_class);
>   unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
>  }
> diff --git a/drivers/media/v4l2-core/v4l2-jobqueue-dev.c 
> b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
> new file mode 100644
> index ..688c4ba275a6
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
> @@ -0,0 +1,173 @@
> +/*
> +V4L2 job queue device
> +
> +Copyright (C) 2017  The Chromium project
> +
> +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.
> +
> +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.
> +
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CLASS_NAME "v4l2_jobqueue"
> +#define DEVICE_NAME "v4l2_jobqueue"
> +
> +static int major;
> +static struct class *jobqueue_class;
> +static struct device *jobqueue_device;
> +
> +static int v4l2_jobqueue_device_open(struct inode *inode, struct file *filp)
> +{
> + struct v4l2_jobqueue *jq;
> +
> + jq = v4l2_jobqueue_new();
> + if (IS_ERR(jq))
> + return PTR_ERR(jq);
> +
> + filp->private_data = jq;
> +
> + return 0;
> +}
> +
> +static int v4l2_jobqueue_device_release(struct inode *inode, struct file 
> *filp)
> +{
> + struct v4l2_jobqueue *jq = filp->private_data;
> +
> + return v4l2_jobqueue_del(jq);
> +}
> +
> +static long v4l2_jobqueue_ioctl_init(struct file *filp, void *arg)
> +{
> + struct v4l2_jobqueue *jq = filp->private_data;
> + struct v4l2_jobqueue_init *cinit = arg;
> +
> + return v4l2_jobqueue_init(jq, cinit);
> +}
> +
> +static long v4l2_jobqueue_device_ioctl_qjob(struct file *filp, void *arg)
> +{
> + struct v4l2_jobqueue *jq = filp->private_data;
> +
> + return v4l2_jobqueue_qjob(jq);
> +}
> +
> +static long