cron job: media_tree daily build: ERRORS

[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:   Thu May  4 05:00:26 CEST 2017
media-tree git hash:3622d3e77ecef090b5111e3c5423313f11711dfa
media_build git hash:   1af19680bde3e227d64d99ff5fdc43eb343a3b28
v4l-utils git hash: 3ece5b03eddcbfe2fae6861cee15755a65864111
gcc version:i686-linux-gcc (GCC) 6.3.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
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-i686: OK
linux-4.10.1-i686: OK
linux-4.11-rc1-i686: OK
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
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-x86_64: WARNINGS
linux-4.10.1-x86_64: WARNINGS
linux-4.11-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH] media: i2c: initialize scalar variables

[Gustavo A. R. Silva]

Initialize scalar variables _pid_ and _ver_ to avoid a possible misbehavior.

Addresses-Coverity-ID: 1324239
Addresses-Coverity-ID: 1324240
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/media/i2c/ov2659.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 6e63672..58615f8 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1308,7 +1308,8 @@ static const struct v4l2_subdev_internal_ops 
ov2659_subdev_internal_ops = {
 static int ov2659_detect(struct v4l2_subdev *sd)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
-   u8 pid, ver;
+   u8 pid = 0;
+   u8 ver = 0;
int ret;
 
dev_dbg(>dev, "%s:\n", __func__);
-- 
2.5.0

[PATCH] [media] v4l2: Add support for go2001 PCI codec driver

[Thierry Escande]

This patch adds support for the go2001 PCI codec driver. This hardware
is present on ChromeOS based devices like the Acer ChromeBox and Acer/LG
ChromeBase 24 devices.

This driver comes from the ChromeOS v3.18 kernel tree and has been
modified to support vb2_buffer restructuring introduced in Linux v4.4.

This driver is originally developed by:
 Pawel Osciak 
 Ville-Mikko Rautio 
 henryhsu 
 Wu-Cheng Li 

Signed-off-by: Thierry Escande 
---
 drivers/media/pci/Kconfig|2 +
 drivers/media/pci/Makefile   |1 +
 drivers/media/pci/go2001/Kconfig |   11 +
 drivers/media/pci/go2001/Makefile|2 +
 drivers/media/pci/go2001/go2001.h|  331 
 drivers/media/pci/go2001/go2001_driver.c | 2563 ++
 drivers/media/pci/go2001/go2001_hw.c | 1365 
 drivers/media/pci/go2001/go2001_hw.h |   55 +
 drivers/media/pci/go2001/go2001_proto.h  |  359 +
 9 files changed, 4689 insertions(+)
 create mode 100644 drivers/media/pci/go2001/Kconfig
 create mode 100644 drivers/media/pci/go2001/Makefile
 create mode 100644 drivers/media/pci/go2001/go2001.h
 create mode 100644 drivers/media/pci/go2001/go2001_driver.c
 create mode 100644 drivers/media/pci/go2001/go2001_hw.c
 create mode 100644 drivers/media/pci/go2001/go2001_hw.h
 create mode 100644 drivers/media/pci/go2001/go2001_proto.h

diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
index da28e68..837681e 100644
--- a/drivers/media/pci/Kconfig
+++ b/drivers/media/pci/Kconfig
@@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
 source "drivers/media/pci/netup_unidvb/Kconfig"
 endif
 
+source "drivers/media/pci/go2001/Kconfig"
+
 endif #MEDIA_PCI_SUPPORT
 endif #PCI
diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
index a7e8af0..58639b7 100644
--- a/drivers/media/pci/Makefile
+++ b/drivers/media/pci/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_STA2X11_VIP) += sta2x11/
 obj-$(CONFIG_VIDEO_SOLO6X10) += solo6x10/
 obj-$(CONFIG_VIDEO_COBALT) += cobalt/
 obj-$(CONFIG_VIDEO_TW5864) += tw5864/
+obj-$(CONFIG_VIDEO_GO2001) += go2001/
diff --git a/drivers/media/pci/go2001/Kconfig b/drivers/media/pci/go2001/Kconfig
new file mode 100644
index 000..c7b5149
--- /dev/null
+++ b/drivers/media/pci/go2001/Kconfig
@@ -0,0 +1,11 @@
+config VIDEO_GO2001
+   tristate "GO2001 codec driver"
+   depends on VIDEO_V4L2 && PCI
+   select VIDEOBUF2_DMA_SG
+   ---help---
+ This driver supports the GO2001 PCI hardware codec. This codec
+ is present on ChromeOS based devices like the Acer ChromeBox
+ and ChromeBase 24 and LG ChromeBase as well.
+
+ To compile this driver as a module, choose M here: the
+ module will be called go2001.
diff --git a/drivers/media/pci/go2001/Makefile 
b/drivers/media/pci/go2001/Makefile
new file mode 100644
index 000..20bad18
--- /dev/null
+++ b/drivers/media/pci/go2001/Makefile
@@ -0,0 +1,2 @@
+go2001-objs:= go2001_driver.o go2001_hw.o
+obj-$(CONFIG_VIDEO_GO2001) += go2001.o
diff --git a/drivers/media/pci/go2001/go2001.h 
b/drivers/media/pci/go2001/go2001.h
new file mode 100644
index 000..0e5ccfd
--- /dev/null
+++ b/drivers/media/pci/go2001/go2001.h
@@ -0,0 +1,331 @@
+/*
+ *  go2001 - GO2001 codec driver.
+ *
+ *  Author : Pawel Osciak 
+ *
+ *  Copyright (C) 2017 Google, Inc.
+ *
+ *  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.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see .
+ */
+#ifndef _MEDIA_PCI_GO2001_GO2001_H_
+#define _MEDIA_PCI_GO2001_GO2001_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include "go2001_proto.h"
+
+struct go2001_msg {
+   struct list_head list_entry;
+   struct go2001_msg_payload payload;
+};
+
+static inline struct go2001_msg_hdr *msg_to_hdr(struct go2001_msg *msg)
+{
+   return >payload.hdr;
+}
+
+static inline void *msg_to_param(struct go2001_msg *msg)
+{
+   return msg->payload.param;
+}
+
+struct go2001_msg_ring {
+   struct go2001_msg_ring_desc desc;
+   void __iomem *desc_iomem;
+   void __iomem *data_iomem;
+   spinlock_t lock;
+};
+
+struct go2001_hw_inst {
+   struct list_head inst_entry;
+
+   u32 session_id;
+   

Re: [PATCH] v4l2-async: add subnotifier registration for subdevices

[Niklas Söderlund]

Hej Sakari,

Tack för dina kommentarer.

On 2017-05-03 22:51:46 +0300, Sakari Ailus wrote:
> Hejssan!
> 
> On Fri, Apr 28, 2017 at 01:47:48PM +0200, Niklas Söderlund wrote:
> > On 2017-04-28 13:28:17 +0300, Sakari Ailus wrote:
> > > Hi Niklas,
> > > 
> > > Thank you for the patch.
> > > 
> > > Do you happen to have a driver that would use this, to see some example of
> > > how the code is to be used?
> > 
> > Yes, the latest R-Car CSI-2 series make use of this, see:
> > 
> > https://www.spinics.net/lists/linux-renesas-soc/msg13693.html
> 
> Ah, thanks. I'll take a look at that --- which should do for other reasons
> as well...
> 
> ...
> 
> > > > +
> > > > +   /*
> > > > +* This function can be called recursively so the list
> > > > +* might be modified in a recursive call. Start from the
> > > > +* top of the list each iteration.
> > > > +*/
> > > > +   found = 1;
> > > > +   while (found) {
> > > > +   found = 0;
> > > >  
> > > > -   list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > > > -   int ret;
> > > > +   list_for_each_entry_safe(sd, tmp, _list, 
> > > > async_list) {
> > > > +   int ret;
> > > >  
> > > > -   asd = v4l2_async_belongs(notifier, sd);
> > > > -   if (!asd)
> > > > -   continue;
> > > > +   asd = v4l2_async_belongs(notifier, sd);
> > > > +   if (!asd)
> > > > +   continue;
> > > >  
> > > > -   ret = v4l2_async_test_notify(notifier, sd, asd);
> > > > -   if (ret < 0) {
> > > > -   mutex_unlock(_lock);
> > > > -   return ret;
> > > > +   ret = v4l2_async_test_notify(notifier, sd, asd);
> > > > +   if (ret < 0) {
> > > > +   if (!subnotifier)
> > > > +   mutex_unlock(_lock);
> > > > +   return ret;
> > > > +   }
> > > > +
> > > > +   found = 1;
> > > > +   break;
> > > > }
> > > > }
> > > >  
> > > > /* Keep also completed notifiers on the list */
> > > > list_add(>list, _list);
> > > >  
> > > > -   mutex_unlock(_lock);
> > > > +   if (!subnotifier)
> > > > +   mutex_unlock(_lock);
> > > >  
> > > > return 0;
> > > >  }
> > > > +
> > > > +int v4l2_async_subnotifier_register(struct v4l2_subdev *sd,
> > > > +   struct v4l2_async_notifier 
> > > > *notifier)
> > > > +{
> > > > +   if (!sd->v4l2_dev) {
> > > > +   dev_err(sd->dev ? sd->dev : NULL,
> 
> sd->dev is enough.

Thanks, but I think i will drop the dev_err() all together and just 
return -EINVAL once i move this to what will be called 
v4l2_async_do_notifier_register() as you suggest bellow.

> 
> > > > +   "Can't register subnotifier for without 
> > > > v4l2_dev\n");
> > > > +   return -EINVAL;
> > > 
> > > When did this start happening? :-)
> > 
> > What do you mean? I'm not sure I understand this comment.
> 
> Uh, right. So the caller simply needs to specify v4l2_dev? The same applies
> to v4l2_async_notifier_register() which does not test that --- but it
> should.
> 
> How about adding this change in a separate patch to what will be called
> v4l2_async_do_notifier_register()?

I agree, I will do this in a separate patch before I add the 
v4l2_async_subnotifier_register().

> 
> > 
> > > 
> > > > +   }
> > > > +
> > > > +   return v4l2_async_do_notifier_register(sd->v4l2_dev, notifier, 
> > > > true);
> > > > +}
> > > > +EXPORT_SYMBOL(v4l2_async_subnotifier_register);
> > > > +
> > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > +struct v4l2_async_notifier *notifier)
> > > > +{
> > > > +   return v4l2_async_do_notifier_register(v4l2_dev, notifier, 
> > > > false);
> > > > +}
> > > >  EXPORT_SYMBOL(v4l2_async_notifier_register);
> > > >  
> > > > -void v4l2_async_notifier_unregister(struct v4l2_async_notifier 
> > > > *notifier)
> > > > +static void
> > > > +v4l2_async_do_notifier_unregister(struct v4l2_async_notifier *notifier,
> > > > + bool subnotifier)
> > > >  {
> > > > struct v4l2_subdev *sd, *tmp;
> > > > unsigned int notif_n_subdev = notifier->num_subdevs;
> > > > @@ -210,7 +248,8 @@ void v4l2_async_notifier_unregister(struct 
> > > > v4l2_async_notifier *notifier)
> > > > "Failed to allocate device cache!\n");
> > > > }
> > > >  
> > > > -   mutex_lock(_lock);
> > > > +   if (!subnotifier)
> > > > +   mutex_lock(_lock);
> > > >  
> > > > list_del(>list);
> > > >  
> > > > @@ -237,15 +276,20 @@ void 

Re: [PATCH] v4l2-async: add subnotifier registration for subdevices

[Niklas Söderlund]

Hi Kieran,

Thanks for your feedback.

On 2017-05-03 18:07:47 +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> Small thing to ponder discovered below:
> 
> On 27/04/17 23:30, Niklas Söderlund wrote:
> > When registered() of v4l2_subdev_internal_ops is called the subdevice
> > have access to the master devices v4l2_dev and it's called with the
> > async frameworks list_lock held. In this context the subdevice can
> > register its own notifiers to allow for incremental discovery of
> > subdevices.
> > 
> > The master device registers the subdevices closest to itself in its
> > notifier while the subdevice(s) themself register notifiers for there
> > closest neighboring devices when they are registered. Using this
> > incremental approach two problems can be solved.
> > 
> > 1. The master device no longer have to care how many subdevices exist in
> >the pipeline. It only needs to care about its closest subdevice and
> >arbitrary long pipelines can be created without having to adapt the
> >master device for each case.
> > 
> > 2. Subdevices which are represented as a single DT node but register
> >more then one subdevice can use this to further the pipeline
> >discovery. Since the subdevice driver is the only one who knows which
> >of its subdevices is linked with which subdevice of a neighboring DT
> >node.
> > 
> > To enable subdevices to register/unregister notifiers from the
> > registered()/unregistered() callback v4l2_async_subnotifier_register()
> > and v4l2_async_subnotifier_unregister() are added. These new notifier
> > register functions are similar to the master device equivalent functions
> > but run without taking the v4l2-async list_lock which already are held
> > when he registered()/unregistered() callbacks are called.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 91 
> > +---
> >  include/media/v4l2-async.h   | 22 +
> >  2 files changed, 95 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 96cc733f35ef72b0..d4a676a2935eb058 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -136,12 +136,13 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> > sd->dev = NULL;
> >  }
> >  
> > -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > -struct v4l2_async_notifier *notifier)
> > +static int v4l2_async_do_notifier_register(struct v4l2_device *v4l2_dev,
> > +  struct v4l2_async_notifier *notifier,
> > +  bool subnotifier)
> >  {
> > struct v4l2_subdev *sd, *tmp;
> > struct v4l2_async_subdev *asd;
> > -   int i;
> > +   int found, i;
> >  
> > if (!notifier->num_subdevs || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> > return -EINVAL;
> > @@ -168,32 +169,69 @@ int v4l2_async_notifier_register(struct v4l2_device 
> > *v4l2_dev,
> > list_add_tail(>list, >waiting);
> > }
> >  
> > -   mutex_lock(_lock);
> > +   if (!subnotifier)
> > +   mutex_lock(_lock);
> > +
> > +   /*
> > +* This function can be called recursively so the list
> > +* might be modified in a recursive call. Start from the
> > +* top of the list each iteration.
> > +*/
> > +   found = 1;
> > +   while (found) {
> > +   found = 0;
> >  
> > -   list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > -   int ret;
> > +   list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > +   int ret;
> >  
> > -   asd = v4l2_async_belongs(notifier, sd);
> > -   if (!asd)
> > -   continue;
> > +   asd = v4l2_async_belongs(notifier, sd);
> > +   if (!asd)
> > +   continue;
> >  
> > -   ret = v4l2_async_test_notify(notifier, sd, asd);
> > -   if (ret < 0) {
> > -   mutex_unlock(_lock);
> > -   return ret;
> > +   ret = v4l2_async_test_notify(notifier, sd, asd);
> > +   if (ret < 0) {
> > +   if (!subnotifier)
> > +   mutex_unlock(_lock);
> > +   return ret;
> > +   }
> > +
> > +   found = 1;
> > +   break;
> > }
> > }
> >  
> > /* Keep also completed notifiers on the list */
> > list_add(>list, _list);
> >  
> > -   mutex_unlock(_lock);
> > +   if (!subnotifier)
> > +   mutex_unlock(_lock);
> >  
> > return 0;
> >  }
> > +
> > +int v4l2_async_subnotifier_register(struct v4l2_subdev *sd,
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   if (!sd->v4l2_dev) {

Re: [PATCH] libdvbv5: T2 delivery descriptor: fix wrong size of bandwidth field

[Mauro Carvalho Chehab]

Em Wed, 3 May 2017 09:53:03 -0300
Mauro Carvalho Chehab  escreveu:

> Hi Gregor,
> 
> Em Tue, 2 May 2017 22:30:29 +0200
> Gregor Jasny  escreveu:
> 
> > Hello Clemens,
> > 
> > On 4/1/17 5:50 PM, Clemens Ladisch wrote:  
> > > ETSI EN 300 468 V1.11.1 § 6.4.4.2 defines the bandwith field as having
> > > four bits.
> > 
> > I just used your patch and another to hopefully fix
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=859008
> > 
> > But I'm a little bit hesitant to merge it to v4l-utils git without
> > Mauros acknowledgement.  
> 
> Patches look correct, but the T2 parser has a more serious issue that
> will require breaking ABI/API compatibility.
> 
> Let me explain a little more about te T2 delivery descriptor.
> 
> Such descriptor is present on DVB-T2 streams, but the specs allow
> a "simplified" version of it, with just 4 bytes:
>   16 bytes for system ID;
>   16 bytes for bit field.
> 
> By the time this descriptor parser was written, the existing
> DVB-T2 streams I got access were using this simplified version.
> 
> After those 4 bytes, the DVB spec[1] allows a variable number of elements, 
> controlled by a C-like code, defined at the spec as[2]:
> 
>   for (i = 0; i < N, i++){
>   cell_id // 16 bits
>   if (tfs_flag == 1) {
>   frequency_loop_length   // 8 bits
>   for (j = 0; j < frequency_loop_length; j++) {
>   centre_frequency// 32 bis
>   }
>   } else {
>   centre_frequency// 32 bis
>   }
>   subcell_info_loop_length// 8 bits
>   for (k = 0; k < subcell_info_loop_length; k++) {
>   cell_id_extension   // 8 bits
>   transposer_frequency// 32 bits
>   }
>   }
> 
> where "N" is dynamically discovered, e. g. the logic checks if
> there is still bytes left inside the descriptor, it will run the
> loop. So, this is actually something like:
>   while (pos < size) {
>   // handle cell_ID logic
>   pos += number_of_bytes_parsed;
>   }
>   
> 
> [1] 
> https://www.dvb.org/resources/public/standards/a38_dvb-si_specification.pdf
> [2] The code is not an exact copy of what's at the spec, as, at spec, all
> loops use "N" instead of the name of the real variable that controls
> the loop.
> 
> There are two problems with the current code:
> 
> 1) This struct that stores the subcell data is wrong. It is currently
> defined as:
> 
>   struct dvb_desc_t2_delivery_subcell {
>   uint8_t cell_id_extension;
>   uint16_t transposer_frequency;
>   } __attribute__((packed));
> 
> However, the transposer frequency is actually 32 bits. From the specs:
> 
>   "transposer_frequency: This 32 bit field indicates the
>centre frequency that is used by a transposer in the sub-cell
>indicated. It is encoded in the same way as the centre_frequency
>field."
> 
> 2) Right now, the code assumes just one table of centre_frequency.
> According with the specs (at least v1.13.1 - with is the latest
> documentation), multiple tables can exist.
> 
> I remember I tested it some years after the initial version, with a
> DVB-T2 stream. On that time, there was just one frequency table,
> e. g. just one cell ID.
> 
> Yet, as now DVB-T2 is spreading, I won't doubt that we'll find some
> places that use multiple cell IDs.
> 
> At the end of the day, what really matters for a DVB scan program
> is that all center_frequency and transposer_frequency to be
> added to the frequencies that will be scanned.
> 
> So, I'm thinking on a way to make a patch that would be
> backward-compatible, e. g. adding both "centre_frequency" and
> "transposer_frequency" at the centre_frequency table, and not
> filling the subcell IDs, as the additional field there (the
> subcell ID) is useless without the cell ID, and its parsing is
> broken, anyway.
> 
> We may latter add a way to store the cell ID and subcell ID at the
> end of the structure.

Ok, I added the above logic and merged the corresponding patch
upstream.

Unfortunately, the only DVB-T2 signal I have is simple: it doesn't
have any subcel IDs. Yet, the first transport (4061) has two
cell IDs (I added an extra debug prints to identify it (as, currently,
we're not storing the cell ID anywhere):
CELL ID= 6101
freq = 6420
CELL ID= 0457
freq = 6500

So, except for the subcel parsing, patch looks OK:


NIT
| table_id 0x40
| section_length  135
| one 3
| zero1
| syntax  1
| transport_stream_id 12352
| current_next1
| version 9
| one23
| section_number  0
| last_section_number 0
| desc_length   45
|0x40: 

[PATCH] ov5670: Add Omnivision OV5670 5M sensor support

[Chiranjeevi Rapolu]

Provides single source pad with up to 2576x1936 pixels at 10-bit raw
bayer format over MIPI CSI2 two lanes at 640Mbps/lane.
Supports up to 30fps at 5M pixels, up to 60fps at 1080p.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov5670.c | 3890 
 3 files changed, 3902 insertions(+)
 create mode 100644 drivers/media/i2c/ov5670.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cee1dae..ded8485 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -531,6 +531,17 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV5670
+   tristate "OmniVision OV5670 sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV5670 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov5670.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 5bc7bbe..3efc61f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
 obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
new file mode 100644
index 000..394f8f2
--- /dev/null
+++ b/drivers/media/i2c/ov5670.c
@@ -0,0 +1,3890 @@
+/*
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define OV5670_CHIP_ID 0x005670
+
+#define OV5670_REG_VALUE_08BIT 1
+#define OV5670_REG_VALUE_16BIT 2
+#define OV5670_REG_VALUE_24BIT 3
+
+#define OV5670_REG_MODE_SELECT 0x0100
+#define OV5670_MODE_STANDBY0x00
+#define OV5670_MODE_STREAMING  0x01
+
+#define OV5670_REG_SOFTWARE_RST0x0103
+#define OV5670_SOFTWARE_RST0x01
+
+#define OV5670_REG_EXPOSURE0x3500
+#define OV5670_REG_ANALOG_GAIN 0x3508
+
+#define OV5670_REG_CHIPID  0x300a
+
+/* Supported link frequencies */
+#define OV5670_LINK_FREQ_840MBPS   84000
+#define OV5670_LINK_FREQ_840MBPS_INDEX 0
+
+/* Analog gain controls from sensor */
+#defineANALOG_GAIN_MIN 0
+#defineANALOG_GAIN_MAX 8191
+#defineANALOG_GAIN_STEP1
+#defineANALOG_GAIN_DEFAULT 128
+
+/* Exposure controls from sensor */
+#defineEXPOSURE_MIN0
+#defineEXPOSURE_MAX1048575
+#defineEXPOSURE_STEP   1
+#defineEXPOSURE_DEFAULT47232
+
+struct ov5670_reg {
+   u16 address;
+   u8 val;
+};
+
+struct ov5670_reg_list {
+   u32 num_of_regs;
+   const struct ov5670_reg *regs;
+};
+
+struct ov5670_link_freq_config {
+   s64 link_freq;
+   u32 pixel_rate;
+
+   const struct ov5670_reg_list reg_list;
+};
+
+struct ov5670_mode {
+   /* Frame width in pixels */
+   u32 width;
+
+   /* Frame height in pixels */
+   u32 height;
+
+   /* Initial number of frames to skip to avoid garbage in the frames */
+   u32 skip_frames;
+
+   /* Link frequency needed for this resolution */
+   u32 link_freq_index;
+
+   /* Sensor register settings for this resolution */
+   const struct ov5670_reg_list reg_list;
+};
+
+static const struct ov5670_reg mipi_data_rate_840mbps[] = {
+   {0x0300, 0x04},
+   {0x0301, 0x00},
+   {0x0302, 0x84},
+   {0x0303, 0x00},
+   {0x0304, 0x03},
+   {0x0305, 0x01},
+   {0x0306, 0x01},
+   {0x030a, 0x00},
+   {0x030b, 0x00},
+   {0x030c, 0x00},
+   {0x030d, 0x26},
+   {0x030e, 0x00},
+   {0x030f, 0x06},
+   {0x0312, 0x01},
+   {0x3031, 0x0a},
+};
+
+static const struct ov5670_reg mode_640x480_regs[] = {
+   {0x3000, 0x00},
+   {0x3002, 0x61},
+   {0x3005, 0xf0},
+ 

Re: [PATCH 11/25] [media] dvb-core: use pr_foo() instead of printk()

[Arnd Bergmann]

On Friday, October 14, 2016 2:45:49 PM CEST Mauro Carvalho Chehab wrote:
> 
> -#define dprintkif (debug) printk
> +#define dprintk(fmt, arg...) do {  \
> +   if (debug)  \
> +   printk(KERN_DEBUG pr_fmt("%s: " fmt),   \
> +   __func__, ##arg);   \
> +} while (0)
> 

Why not just use pr_debug() or dev_dbg() here? They already
have a way to control output at runtime (CONFIG_DYNAMIC_DEBUG).

Arnd

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Hans,

Hans Verkuil wrote:

Is it likely that there will be more Intel PCI drivers? If so, then we
can consider putting it in pci/intel/ipu3/.


Maybe. There is more hardware and they're called IPUs, too. I think this 
naming would be a good idea going forward.


--
Regards,

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

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Hans Verkuil]

On 04/30/2017 01:34 AM, Yong Zhi wrote:
> This patch adds CIO2 CSI-2 device driver for
> Intel's IPU3 camera sub-system support.
> 
> The V4L2 fwnode matching depends on the following work:
> 
> 
> 
> Signed-off-by: Yong Zhi 
> ---
>  drivers/media/pci/Kconfig  |2 +
>  drivers/media/pci/Makefile |3 +-
>  drivers/media/pci/ipu3/Kconfig |   17 +
>  drivers/media/pci/ipu3/Makefile|1 +
>  drivers/media/pci/ipu3/ipu3-cio2.c | 1813 
> 
>  drivers/media/pci/ipu3/ipu3-cio2.h |  425 +
>  6 files changed, 2260 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/pci/ipu3/Kconfig
>  create mode 100644 drivers/media/pci/ipu3/Makefile
>  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.c
>  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.h

Not a review (yet), just something I noticed: I would recommend calling
the directory intel-ipu3. The ipu3 name is a bit obscure.

Is it likely that there will be more Intel PCI drivers? If so, then we
can consider putting it in pci/intel/ipu3/.

Also, can you post as a reply to the cover letter of this patch series the
output of 'v4l2-compliance' and 'v4l2-compliance -f'. Make sure you use the
latest v4l2-compliance code from the master branch of the v4l-utils repository
(https://git.linuxtv.org/v4l-utils.git/).

Regards,

Hans

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Tuukka,

On Wed, May 03, 2017 at 02:06:23PM +0300, Tuukka Toivonen wrote:
> Hi Sakari,
> 
> On Wednesday, May 03, 2017 11:58:01 Sakari Ailus wrote:
> > Hi Yong,
> > 
> > A few more minor comments below...
> > 
> > On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
> > ...
> > > +/*** V4L2 sub-device asynchronous registration callbacks***/
> > > +
> > > +static struct cio2_queue *cio2_find_queue_by_sensor_node(struct 
> > > cio2_queue *q,
> > > + struct fwnode_handle *fwnode)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < CIO2_QUEUES; i++) {
> > > + if (q[i].sensor->fwnode == fwnode)
> > > + return [i];
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +/* The .bound() notifier callback when a match is found */
> > > +static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> > > +struct v4l2_subdev *sd,
> > > +struct v4l2_async_subdev *asd)
> > > +{
> > > + struct cio2_device *cio2 = container_of(notifier,
> > > + struct cio2_device, notifier);
> > > + struct sensor_async_subdev *s_asd = container_of(asd,
> > > + struct sensor_async_subdev, asd);
> > > + struct cio2_queue *q;
> > > + struct device *dev;
> > > + int i;
> > > +
> > > + dev = >pci_dev->dev;
> > > +
> > > + /* Find first free slot for the subdev */
> > > + for (i = 0; i < CIO2_QUEUES; i++)
> > > + if (!cio2->queue[i].sensor)
> > > + break;
> > > +
> > > + if (i >= CIO2_QUEUES) {
> > > + dev_err(dev, "too many subdevs\n");
> > > + return -ENOSPC;
> > > + }
> > > + q = >queue[i];
> > > +
> > > + q->csi2.port = s_asd->vfwn_endpt.base.port;
> > > + q->csi2.num_of_lanes = s_asd->vfwn_endpt.bus.mipi_csi2.num_data_lanes;
> > > + q->sensor = sd;
> > > + q->csi_rx_base = cio2->base + CIO2_REG_PIPE_BASE(q->csi2.port);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* The .unbind callback */
> > > +static void cio2_notifier_unbind(struct v4l2_async_notifier *notifier,
> > > +  struct v4l2_subdev *sd,
> > > +  struct v4l2_async_subdev *asd)
> > > +{
> > > + struct cio2_device *cio2 = container_of(notifier,
> > > + struct cio2_device, notifier);
> > > + unsigned int i;
> > > +
> > > + /* Note: sd may here point to unallocated memory. Do not access. */
> > 
> > When can this happen?
> 
> If v4l2_device_register_subdev() fails, it may lead to
> calling subdevice's remove function and the devres system
> freeing the subdevice before unbind is called. This did
> happen during driver development.

Ouch. Indeed, we do have object lifetime issues. :-( They will obviously
need to be fixed.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv2] omap3isp: add support for CSI1 bus

[Pavel Machek]

Hi!

> It seems they don't compile. Hmmm. Did I do something wrong? "struct
> fwnode_endpoint" seems to be only used in v4l2-fwnode.h; that can't be 
> right...?

Next problem is missing dev_fwnode; fixed. Next problem is

pavel@duo:/data/l/linux-n900$ git grep fwnode_graph_get_next_endpoint
.
drivers/media/i2c/smiapp/smiapp-core.c: ep =
fwnode_graph_get_next_endpoint(fwnode, NULL);
drivers/media/platform/omap3isp/isp.c:  while ((fwnode =
fwnode_graph_get_next_endpoint(dev_fwnode(dev),

So sorry, I guess I should wait for version that compiles ;-).
Pavel

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c458c63..f52a260 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -182,11 +182,6 @@ static int pset_prop_read_string(struct property_set *pset,
return 0;
 }
 
-static inline struct fwnode_handle *dev_fwnode(struct device *dev)
-{
-   return IS_ENABLED(CONFIG_OF) && dev->of_node ?
-   >of_node->fwnode : dev->fwnode;
-}
 
 /**
  * device_property_present - check if a property of a device is present
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 8bd28ce..9215e23 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -27,4 +27,10 @@ struct fwnode_handle {
struct fwnode_handle *secondary;
 };
 
+static inline struct fwnode_handle *dev_fwnode(struct device *dev)
+{
+   return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+   >of_node->fwnode : dev->fwnode;
+}
+
 #endif
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index d762a55..9e9cfbc 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -80,7 +80,7 @@ struct v4l2_fwnode_bus_mipi_csi1 {
  * @nr_of_link_frequencies: number of elements in link_frequenccies array
  */
 struct v4l2_fwnode_endpoint {
-   struct fwnode_endpoint base;
+   /*struct fwnode_endpoint base; */
/*
 * Fields below this line will be zeroed by
 * v4l2_fwnode_parse_endpoint()




-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Moi,

On Wed, May 03, 2017 at 01:57:31PM +0300, Tuukka Toivonen wrote:
> Hi Sakari,
> 
> Thanks for the comments.
> 
> On Tuesday, May 02, 2017 16:00:20 Sakari Ailus wrote:
> > Hi Yong,
> > 
> > Thanks for the patches! Some comments below.
> > 
> > On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
> > > This patch adds CIO2 CSI-2 device driver for
> > > Intel's IPU3 camera sub-system support.
> > > 
> > > The V4L2 fwnode matching depends on the following work:
> > > 
> > > 
> > > 
> > > Signed-off-by: Yong Zhi 
> > > ---
> > >  drivers/media/pci/Kconfig  |2 +
> > >  drivers/media/pci/Makefile |3 +-
> > >  drivers/media/pci/ipu3/Kconfig |   17 +
> > >  drivers/media/pci/ipu3/Makefile|1 +
> > >  drivers/media/pci/ipu3/ipu3-cio2.c | 1813 
> > > 
> > >  drivers/media/pci/ipu3/ipu3-cio2.h |  425 +
> > >  6 files changed, 2260 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/media/pci/ipu3/Kconfig
> > >  create mode 100644 drivers/media/pci/ipu3/Makefile
> > >  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.c
> > >  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.h
> > > 
> > > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
> > > index da28e68..63ece75 100644
> > > --- a/drivers/media/pci/Kconfig
> > > +++ b/drivers/media/pci/Kconfig
> > > @@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
> > >  source "drivers/media/pci/netup_unidvb/Kconfig"
> > >  endif
> > >  
> > > +source "drivers/media/pci/ipu3/Kconfig"
> > > +
> > >  endif #MEDIA_PCI_SUPPORT
> > >  endif #PCI
> > > diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
> > > index a7e8af0..8d5e8db 100644
> > > --- a/drivers/media/pci/Makefile
> > > +++ b/drivers/media/pci/Makefile
> > > @@ -13,7 +13,8 @@ obj-y+= ttpci/  \
> > >   ddbridge/   \
> > >   saa7146/\
> > >   smipcie/\
> > > - netup_unidvb/
> > > + netup_unidvb/   \
> > > + ipu3/
> > >  
> > >  obj-$(CONFIG_VIDEO_IVTV) += ivtv/
> > >  obj-$(CONFIG_VIDEO_ZORAN) += zoran/
> > > diff --git a/drivers/media/pci/ipu3/Kconfig 
> > > b/drivers/media/pci/ipu3/Kconfig
> > > new file mode 100644
> > > index 000..2a895d6
> > > --- /dev/null
> > > +++ b/drivers/media/pci/ipu3/Kconfig
> > > @@ -0,0 +1,17 @@
> > > +config VIDEO_IPU3_CIO2
> > > + tristate "Intel ipu3-cio2 driver"
> > > + depends on VIDEO_V4L2 && PCI
> > > + depends on MEDIA_CONTROLLER
> > > + depends on HAS_DMA
> > > + depends on ACPI
> > > + select V4L2_FWNODE
> > > + select VIDEOBUF2_DMA_SG
> > > +
> > > + ---help---
> > > + This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
> > > + Skylake and Kaby Lake SoCs and used for capturing images and
> > > + video from a camera sensor.
> > > +
> > > + Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> > > + connected camera.
> > > + The module will be called ipu3-cio2.
> > > diff --git a/drivers/media/pci/ipu3/Makefile 
> > > b/drivers/media/pci/ipu3/Makefile
> > > new file mode 100644
> > > index 000..20186e3
> > > --- /dev/null
> > > +++ b/drivers/media/pci/ipu3/Makefile
> > > @@ -0,0 +1 @@
> > > +obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> > > diff --git a/drivers/media/pci/ipu3/ipu3-cio2.c 
> > > b/drivers/media/pci/ipu3/ipu3-cio2.c
> > > new file mode 100644
> > > index 000..2b641ad
> > > --- /dev/null
> > > +++ b/drivers/media/pci/ipu3/ipu3-cio2.c
> > > @@ -0,0 +1,1813 @@
> > > +/*
> > > + * 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.
> > > + *
> > > + * Based partially on Intel IPU4 driver written by
> > > + *  Sakari Ailus 
> > > + *  Samu Onkalo 
> > > + *  Jouni Högander 
> > > + *  Jouni Ukkonen 
> > > + *  Antti Laakso 
> > > + * et al.
> > > + *
> > > + */
> > > +
> > > +#include 
> > 
> > I believe you shouldn't need this one.
> > 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > 
> > Alphabetical order, please.
> > 
> > > +#include "ipu3-cio2.h"
> > > +
> > > +MODULE_AUTHOR("Tianshu Qiu ");
> > > 

Re: [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2)

[Pavel Machek]

On Wed 2017-05-03 20:05:56, Russell King - ARM Linux wrote:
> On Wed, Apr 26, 2017 at 06:43:54PM +0300, Ivaylo Dimitrov wrote:
> > >+static int get_luminosity_bayer10(uint16_t *buf, const struct v4l2_format 
> > >*fmt)
> > >+{
> > >+  long long avg_lum = 0;
> > >+  int x, y;
> > >+  
> > >+  buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> > >+  fmt->fmt.pix.width / 4;
> > >+
> > >+  for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> > >+  for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> > 
> > That would take some time :). AIUI, we have NEON support in ARM kernels
> > (CONFIG_KERNEL_MODE_NEON), I wonder if it makes sense (me) to convert the
> > above loop to NEON-optimized when it comes to it? Are there any drawbacks in
> > using NEON code in kernel?
> 
> Using neon without the VFP state saved and restored corrupts userspace's
> FP state.  So, you have to save the entire VFP state to use neon in kernel
> mode.  There are helper functions for this: kernel_neon_begin() and
> kernel_neon_end().
...
> Given that, do we really want to be walking over multi-megabytes of image
> data in the kernel with preemption disabled - it sounds like a recipe for
> a very sluggish system.  I think this should (and can only sensibly be
> done) in userspace.

The patch was for libv4l2. (And I explained why we don't need to
overoptimize this.)
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] v4l2-async: add subnotifier registration for subdevices

[Sakari Ailus]

Hejssan!

On Fri, Apr 28, 2017 at 01:47:48PM +0200, Niklas Söderlund wrote:
> On 2017-04-28 13:28:17 +0300, Sakari Ailus wrote:
> > Hi Niklas,
> > 
> > Thank you for the patch.
> > 
> > Do you happen to have a driver that would use this, to see some example of
> > how the code is to be used?
> 
> Yes, the latest R-Car CSI-2 series make use of this, see:
> 
> https://www.spinics.net/lists/linux-renesas-soc/msg13693.html

Ah, thanks. I'll take a look at that --- which should do for other reasons
as well...

...

> > > +
> > > + /*
> > > +  * This function can be called recursively so the list
> > > +  * might be modified in a recursive call. Start from the
> > > +  * top of the list each iteration.
> > > +  */
> > > + found = 1;
> > > + while (found) {
> > > + found = 0;
> > >  
> > > - list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > > - int ret;
> > > + list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > > + int ret;
> > >  
> > > - asd = v4l2_async_belongs(notifier, sd);
> > > - if (!asd)
> > > - continue;
> > > + asd = v4l2_async_belongs(notifier, sd);
> > > + if (!asd)
> > > + continue;
> > >  
> > > - ret = v4l2_async_test_notify(notifier, sd, asd);
> > > - if (ret < 0) {
> > > - mutex_unlock(_lock);
> > > - return ret;
> > > + ret = v4l2_async_test_notify(notifier, sd, asd);
> > > + if (ret < 0) {
> > > + if (!subnotifier)
> > > + mutex_unlock(_lock);
> > > + return ret;
> > > + }
> > > +
> > > + found = 1;
> > > + break;
> > >   }
> > >   }
> > >  
> > >   /* Keep also completed notifiers on the list */
> > >   list_add(>list, _list);
> > >  
> > > - mutex_unlock(_lock);
> > > + if (!subnotifier)
> > > + mutex_unlock(_lock);
> > >  
> > >   return 0;
> > >  }
> > > +
> > > +int v4l2_async_subnotifier_register(struct v4l2_subdev *sd,
> > > + struct v4l2_async_notifier *notifier)
> > > +{
> > > + if (!sd->v4l2_dev) {
> > > + dev_err(sd->dev ? sd->dev : NULL,

sd->dev is enough.

> > > + "Can't register subnotifier for without v4l2_dev\n");
> > > + return -EINVAL;
> > 
> > When did this start happening? :-)
> 
> What do you mean? I'm not sure I understand this comment.

Uh, right. So the caller simply needs to specify v4l2_dev? The same applies
to v4l2_async_notifier_register() which does not test that --- but it
should.

How about adding this change in a separate patch to what will be called
v4l2_async_do_notifier_register()?

> 
> > 
> > > + }
> > > +
> > > + return v4l2_async_do_notifier_register(sd->v4l2_dev, notifier, true);
> > > +}
> > > +EXPORT_SYMBOL(v4l2_async_subnotifier_register);
> > > +
> > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > +  struct v4l2_async_notifier *notifier)
> > > +{
> > > + return v4l2_async_do_notifier_register(v4l2_dev, notifier, false);
> > > +}
> > >  EXPORT_SYMBOL(v4l2_async_notifier_register);
> > >  
> > > -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> > > +static void
> > > +v4l2_async_do_notifier_unregister(struct v4l2_async_notifier *notifier,
> > > +   bool subnotifier)
> > >  {
> > >   struct v4l2_subdev *sd, *tmp;
> > >   unsigned int notif_n_subdev = notifier->num_subdevs;
> > > @@ -210,7 +248,8 @@ void v4l2_async_notifier_unregister(struct 
> > > v4l2_async_notifier *notifier)
> > >   "Failed to allocate device cache!\n");
> > >   }
> > >  
> > > - mutex_lock(_lock);
> > > + if (!subnotifier)
> > > + mutex_lock(_lock);
> > >  
> > >   list_del(>list);
> > >  
> > > @@ -237,15 +276,20 @@ void v4l2_async_notifier_unregister(struct 
> > > v4l2_async_notifier *notifier)
> > >   put_device(d);
> > >   }
> > >  
> > > - mutex_unlock(_lock);
> > > + if (!subnotifier)
> > > + mutex_unlock(_lock);
> > >  
> > >   /*
> > >* Call device_attach() to reprobe devices
> > >*
> > >* NOTE: If dev allocation fails, i is 0, and the whole loop won't be
> > >* executed.
> > > +  * TODO: If we are unregistering a subdevice notifier we can't reprobe
> > > +  * since the lock_list is held by the master device and attaching that
> > > +  * device would call v4l2_async_register_subdev() and end in a deadlock
> > > +  * on list_lock.
> > >*/
> > > - while (i--) {
> > > + while (i-- && !subnotifier) {
> > 
> > Why is this not done for sub-notifiers?
> > 
> > That said, the code here looks really dubious. But that's out of scope of
> > the patchset.
> 
> I try to explain this in the comment above :-)
> 
> If this is called for sub-notifiers it will result in the probe function 
> of the 

Re: [PATCHv2] omap3isp: add support for CSI1 bus

[Pavel Machek]

Hi!

> > Could you try to two patches I've applied on the ccp2 branch (I'll remove
> > them if there are issues).
> > 
> > That's compile tested for now only.
> > 
> 
> I've updated the CCP2 patches here on top of the latest fwnode patches:
> 
> 
> 
> No even compile testing this time though. I'm afraid I haven't had the
> time to otherwise to work on the CCP2 support, so there are no other
> changes besides the rebase.

It seems they don't compile. Hmmm. Did I do something wrong? "struct
fwnode_endpoint" seems to be only used in v4l2-fwnode.h; that can't be right...?

  CC  drivers/media/i2c/smiapp/smiapp-core.o
  In file included from drivers/media/i2c/smiapp/smiapp-core.c:35:0:
  ./include/media/v4l2-fwnode.h:83:25: error: field 'base' has
  incomplete type
  drivers/media/i2c/smiapp/smiapp-core.c: In function
  'smiapp_get_hwconfig':
  drivers/media/i2c/smiapp/smiapp-core.c:2790:9: error: implicit
  declaration of function 'dev_fwnode'
  [-Werror=implicit-function-declaration]
  drivers/media/i2c/smiapp/smiapp-core.c:2790:33: warning:
  initialization makes pointer from integer without a cast [enabled by
  default]
  drivers/media/i2c/smiapp/smiapp-core.c:2797:2: error: implicit
  declaration of function 'fwnode_graph_get_next_endpoint'
  [-Werror=implicit-function-declaration]

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] [media] dt-bindings: Add bindings for video-multiplexer device

[Sakari Ailus]

Thanks, Philipp!

On Tue, May 02, 2017 at 05:09:12PM +0200, Philipp Zabel wrote:
> Add bindings documentation for the video multiplexer device.
> 
> Signed-off-by: Sascha Hauer 
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Steve Longerbeam 
> Acked-by: Peter Rosin 
> ---
> No changes since v1 [1].
> 
> This was previously sent as part of Steve's i.MX media series [2].
> 
> [1] https://patchwork.kernel.org/patch/9704789/
> [2] https://patchwork.kernel.org/patch/9647951/
> ---

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 2/2] [media] platform: add video-multiplexer subdevice driver

[Sakari Ailus]

Hi Philipp,

Thanks for continuing working on this!

I have some minor comments below...

On Tue, May 02, 2017 at 05:09:13PM +0200, Philipp Zabel wrote:
> This driver can handle SoC internal and external video bus multiplexers,
> controlled by mux controllers provided by the mux controller framework,
> such as MMIO register bitfields or GPIOs. The subdevice passes through
> the mbus configuration of the active input to the output side.
> 
> Signed-off-by: Sascha Hauer 
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Steve Longerbeam 
> ---
> Changes since v1 [1]:
>  - Protect vmux->active with a mutex in link_setup and set_format.
>s_stream does not need to be locked; it is called when the pipeline
>is started and thus the link setup can not be changed anymore.
>  - Remove the unused g_mbus_config.
> 
> This was previously sent as part of Steve's i.MX media series [2].
> 
> [1] https://patchwork.kernel.org/patch/9704791/
> [2] https://patchwork.kernel.org/patch/9647869/
> ---
>  drivers/media/platform/Kconfig |   6 +
>  drivers/media/platform/Makefile|   2 +
>  drivers/media/platform/video-mux.c | 312 
> +
>  3 files changed, 320 insertions(+)
>  create mode 100644 drivers/media/platform/video-mux.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index c9106e105baba..b046a6d39fee5 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -74,6 +74,12 @@ config VIDEO_M32R_AR_M64278
> To compile this driver as a module, choose M here: the
> module will be called arv.
>  
> +config VIDEO_MUX
> + tristate "Video Multiplexer"
> + depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER && 
> MULTIPLEXER
> + help
> +   This driver provides support for N:1 video bus multiplexers.
> +
>  config VIDEO_OMAP3
>   tristate "OMAP 3 Camera support"
>   depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 349ddf6a69da2..fd2735ca3ff75 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -27,6 +27,8 @@ obj-$(CONFIG_VIDEO_SH_VEU)  += sh_veu.o
>  
>  obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)  += m2m-deinterlace.o
>  
> +obj-$(CONFIG_VIDEO_MUX)  += video-mux.o
> +
>  obj-$(CONFIG_VIDEO_S3C_CAMIF)+= s3c-camif/
>  obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS)   += exynos4-is/
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
> diff --git a/drivers/media/platform/video-mux.c 
> b/drivers/media/platform/video-mux.c
> new file mode 100644
> index 0..a857f5e89deff
> --- /dev/null
> +++ b/drivers/media/platform/video-mux.c
> @@ -0,0 +1,312 @@
> +/*
> + * video stream multiplexer controlled via mux control
> + *
> + * Copyright (C) 2013 Pengutronix, Sascha Hauer 
> + * Copyright (C) 2016-2017 Pengutronix, Philipp Zabel 
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Could you rebase this on the V4L2 fwnode patchset here, please?



The conversion is rather simple, as shown here:



> +
> +struct video_mux {
> + struct v4l2_subdev subdev;
> + struct media_pad *pads;
> + struct v4l2_mbus_framefmt *format_mbus;
> + struct v4l2_of_endpoint *endpoint;
> + struct mux_control *mux;
> + struct mutex lock;
> + int active;
> +};
> +
> +static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev 
> *sd)
> +{
> + return container_of(sd, struct video_mux, subdev);
> +}
> +
> +static inline bool is_source_pad(struct video_mux *vmux, unsigned int pad)

It's a common practice to test pad flags rather than the pad number.
Although the pad number here implicitly tells this, too, testing pad flags
is cleaner.

The matter was discussed in the past and it was decided not to add helper
functions to the framework for the purpose as testing the flags is trivial.

> +{
> + return pad == vmux->subdev.entity.num_pads - 1;
> +}
> +

Re: [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2)

[Russell King - ARM Linux]

On Wed, Apr 26, 2017 at 06:43:54PM +0300, Ivaylo Dimitrov wrote:
> >+static int get_luminosity_bayer10(uint16_t *buf, const struct v4l2_format 
> >*fmt)
> >+{
> >+long long avg_lum = 0;
> >+int x, y;
> >+
> >+buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> >+fmt->fmt.pix.width / 4;
> >+
> >+for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> >+for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> 
> That would take some time :). AIUI, we have NEON support in ARM kernels
> (CONFIG_KERNEL_MODE_NEON), I wonder if it makes sense (me) to convert the
> above loop to NEON-optimized when it comes to it? Are there any drawbacks in
> using NEON code in kernel?

Using neon without the VFP state saved and restored corrupts userspace's
FP state.  So, you have to save the entire VFP state to use neon in kernel
mode.  There are helper functions for this: kernel_neon_begin() and
kernel_neon_end().

You can't build C code with the compiler believing that neon is available
as the compiler could emit neon instructions in unprotected kernel code.

Note that kernel_neon_begin() is only allowed to be called outside
interrupt context and with preemption disabled.

Given that, do we really want to be walking over multi-megabytes of image
data in the kernel with preemption disabled - it sounds like a recipe for
a very sluggish system.  I think this should (and can only sensibly be
done) in userspace.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] v4l2-async: add subnotifier registration for subdevices

[Kieran Bingham]

Hi Niklas,

Small thing to ponder discovered below:

On 27/04/17 23:30, Niklas Söderlund wrote:
> When registered() of v4l2_subdev_internal_ops is called the subdevice
> have access to the master devices v4l2_dev and it's called with the
> async frameworks list_lock held. In this context the subdevice can
> register its own notifiers to allow for incremental discovery of
> subdevices.
> 
> The master device registers the subdevices closest to itself in its
> notifier while the subdevice(s) themself register notifiers for there
> closest neighboring devices when they are registered. Using this
> incremental approach two problems can be solved.
> 
> 1. The master device no longer have to care how many subdevices exist in
>the pipeline. It only needs to care about its closest subdevice and
>arbitrary long pipelines can be created without having to adapt the
>master device for each case.
> 
> 2. Subdevices which are represented as a single DT node but register
>more then one subdevice can use this to further the pipeline
>discovery. Since the subdevice driver is the only one who knows which
>of its subdevices is linked with which subdevice of a neighboring DT
>node.
> 
> To enable subdevices to register/unregister notifiers from the
> registered()/unregistered() callback v4l2_async_subnotifier_register()
> and v4l2_async_subnotifier_unregister() are added. These new notifier
> register functions are similar to the master device equivalent functions
> but run without taking the v4l2-async list_lock which already are held
> when he registered()/unregistered() callbacks are called.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 91 
> +---
>  include/media/v4l2-async.h   | 22 +
>  2 files changed, 95 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 96cc733f35ef72b0..d4a676a2935eb058 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -136,12 +136,13 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
>   sd->dev = NULL;
>  }
>  
> -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> -  struct v4l2_async_notifier *notifier)
> +static int v4l2_async_do_notifier_register(struct v4l2_device *v4l2_dev,
> +struct v4l2_async_notifier *notifier,
> +bool subnotifier)
>  {
>   struct v4l2_subdev *sd, *tmp;
>   struct v4l2_async_subdev *asd;
> - int i;
> + int found, i;
>  
>   if (!notifier->num_subdevs || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
>   return -EINVAL;
> @@ -168,32 +169,69 @@ int v4l2_async_notifier_register(struct v4l2_device 
> *v4l2_dev,
>   list_add_tail(>list, >waiting);
>   }
>  
> - mutex_lock(_lock);
> + if (!subnotifier)
> + mutex_lock(_lock);
> +
> + /*
> +  * This function can be called recursively so the list
> +  * might be modified in a recursive call. Start from the
> +  * top of the list each iteration.
> +  */
> + found = 1;
> + while (found) {
> + found = 0;
>  
> - list_for_each_entry_safe(sd, tmp, _list, async_list) {
> - int ret;
> + list_for_each_entry_safe(sd, tmp, _list, async_list) {
> + int ret;
>  
> - asd = v4l2_async_belongs(notifier, sd);
> - if (!asd)
> - continue;
> + asd = v4l2_async_belongs(notifier, sd);
> + if (!asd)
> + continue;
>  
> - ret = v4l2_async_test_notify(notifier, sd, asd);
> - if (ret < 0) {
> - mutex_unlock(_lock);
> - return ret;
> + ret = v4l2_async_test_notify(notifier, sd, asd);
> + if (ret < 0) {
> + if (!subnotifier)
> + mutex_unlock(_lock);
> + return ret;
> + }
> +
> + found = 1;
> + break;
>   }
>   }
>  
>   /* Keep also completed notifiers on the list */
>   list_add(>list, _list);
>  
> - mutex_unlock(_lock);
> + if (!subnotifier)
> + mutex_unlock(_lock);
>  
>   return 0;
>  }
> +
> +int v4l2_async_subnotifier_register(struct v4l2_subdev *sd,
> + struct v4l2_async_notifier *notifier)
> +{
> + if (!sd->v4l2_dev) {
> + dev_err(sd->dev ? sd->dev : NULL,

Just came across this, and it seems a bit 'extraneous'

'If sd->dev is not null, use sd->dev, otherwise use NULL'

--
KB



> + "Can't register subnotifier 

Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

[Jordan Crouse]

On Tue, May 02, 2017 at 12:17:20PM +0300, Stanimir Varbanov wrote:
> Hi,
> 
> On 04/29/2017 11:22 PM, Bjorn Andersson wrote:
> > On Fri 28 Apr 15:02 PDT 2017, Jordan Crouse wrote:
> > 
> >> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> >>> +int venus_boot(struct device *parent, struct device *fw_dev)
> >>> +{
> >>> + const struct firmware *mdt;
> >>> + phys_addr_t mem_phys;
> >>> + ssize_t fw_size;
> >>> + size_t mem_size;
> >>> + void *mem_va;
> >>> + int ret;
> >>> +
> >>> + if (!qcom_scm_is_available())
> >>> + return -EPROBE_DEFER;
> >>> +
> >>> + fw_dev->parent = parent;
> >>> + fw_dev->release = device_release_dummy;
> >>> +
> >>> + ret = dev_set_name(fw_dev, "%s:%s", dev_name(parent), "firmware");
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + ret = device_register(fw_dev);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> +
> >>> + ret = of_reserved_mem_device_init_by_idx(fw_dev, parent->of_node, 0);
> >>> + if (ret)
> >>> + goto err_unreg_device;
> >>> +
> >>> + mem_size = VENUS_FW_MEM_SIZE;
> >>> +
> >>> + mem_va = dmam_alloc_coherent(fw_dev, mem_size, _phys, GFP_KERNEL);
> >>> + if (!mem_va) {
> >>> + ret = -ENOMEM;
> >>> + goto err_unreg_device;
> >>> + }
> >>> +
> >>> + ret = request_firmware(, VENUS_FIRMWARE_NAME, fw_dev);
> >>> + if (ret < 0)
> >>> + goto err_unreg_device;
> >>> +
> >>> + fw_size = qcom_mdt_get_size(mdt);
> >>> + if (fw_size < 0) {
> >>> + ret = fw_size;
> >>> + release_firmware(mdt);
> >>> + goto err_unreg_device;
> >>> + }
> >>> +
> >>> + ret = qcom_mdt_load(fw_dev, mdt, VENUS_FIRMWARE_NAME, VENUS_PAS_ID,
> >>> + mem_va, mem_phys, mem_size);
> >>> +
> >>> + release_firmware(mdt);
> >>> +
> >>> + if (ret)
> >>> + goto err_unreg_device;
> >>> +
> >>> + ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> >>> + if (ret)
> >>> + goto err_unreg_device;
> >>> +
> >>> + return 0;
> >>> +
> >>> +err_unreg_device:
> >>> + device_unregister(fw_dev);
> >>> + return ret;
> >>> +}
> >>
> >> Hey, this looks familiar - almost line for line identical to what we'll 
> >> need to
> >> do for GPU.
> >>
> >> Bjorn - Is this enough to qualify for generic status in the mdt_loader 
> >> code?
> >> I know its just two consumers, but it would save 50 or 60 lines of code 
> >> between
> >> the two drivers and be easier to maintain.
> >>
> > 
> > I think the code setting up the struct device for memory allocation
> > should be done during probe of the parent, so that I don't think should
> > be shared.
> > 
> > The part that allocates memory from a device, loads the mdt into that
> > memory and calls auth_and_reset() sounds like a useful thing to move to
> > the mdt_loader.c
> 
> I agree, who is going to do that?

I'll volunteer with the caveat that sometimes I'm not as fast on turning around
code as I would like to be given then organization I'm affiliated with. 

That said I do have a stack with my zap shader code pending so it would make
sense to stick it in there.

If the worst happens we can alway merge separately and then consolidate.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] media: dvb-frontends: drx39xyj: remove obsolete sign extend macros

[Martin Kepplinger]

DRX_S9TOS16 and DRX_S24TODRXFREQ are simply not used. Furthermore,
sign_extend32() should be used for sign extension. (Also, the comment
describing DRX_S24TODRXFREQ was wrong). So remove these macros.

Signed-off-by: Martin Kepplinger 
---
 drivers/media/dvb-frontends/drx39xyj/drx_driver.h | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h 
b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
index 4442e47..afa702c 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
+++ b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
@@ -450,19 +450,6 @@ MACROS
((u8)u16)x)>>8)&0xFF))
 
 /**
-* \brief Macro to sign extend signed 9 bit value to signed  16 bit value
-*/
-#define DRX_S9TOS16(x) u16)x)&0x100) ? ((s16)((u16)(x)|0xFF00)) : (x))
-
-/**
-* \brief Macro to sign extend signed 9 bit value to signed  16 bit value
-*/
-#define DRX_S24TODRXFREQ(x) u32) x) & 0x0080UL) ? \
-((s32) \
-   (((u32) x) | 0xFF00)) : \
-((s32) x))
-
-/**
 * \brief Macro to convert 16 bit register value to a s32
 */
 #define DRX_U16TODRXFREQ(x)   ((x & 0x8000) ? \
-- 
2.1.4

Re: [PATCH] libdvbv5: T2 delivery descriptor: fix wrong size of bandwidth field

[Mauro Carvalho Chehab]

Hi Gregor,

Em Tue, 2 May 2017 22:30:29 +0200
Gregor Jasny  escreveu:

> Hello Clemens,
> 
> On 4/1/17 5:50 PM, Clemens Ladisch wrote:
> > ETSI EN 300 468 V1.11.1 § 6.4.4.2 defines the bandwith field as having
> > four bits.  
> 
> I just used your patch and another to hopefully fix
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=859008
> 
> But I'm a little bit hesitant to merge it to v4l-utils git without
> Mauros acknowledgement.

Patches look correct, but the T2 parser has a more serious issue that
will require breaking ABI/API compatibility.

Let me explain a little more about te T2 delivery descriptor.

Such descriptor is present on DVB-T2 streams, but the specs allow
a "simplified" version of it, with just 4 bytes:
16 bytes for system ID;
16 bytes for bit field.

By the time this descriptor parser was written, the existing
DVB-T2 streams I got access were using this simplified version.

After those 4 bytes, the DVB spec[1] allows a variable number of elements, 
controlled by a C-like code, defined at the spec as[2]:

for (i = 0; i < N, i++){
cell_id // 16 bits
if (tfs_flag == 1) {
frequency_loop_length   // 8 bits
for (j = 0; j < frequency_loop_length; j++) {
centre_frequency// 32 bis
}
} else {
centre_frequency// 32 bis
}
subcell_info_loop_length// 8 bits
for (k = 0; k < subcell_info_loop_length; k++) {
cell_id_extension   // 8 bits
transposer_frequency// 32 bits
}
}

where "N" is dynamically discovered, e. g. the logic checks if
there is still bytes left inside the descriptor, it will run the
loop. So, this is actually something like:
while (pos < size) {
// handle cell_ID logic
pos += number_of_bytes_parsed;
}


[1] https://www.dvb.org/resources/public/standards/a38_dvb-si_specification.pdf
[2] The code is not an exact copy of what's at the spec, as, at spec, all
loops use "N" instead of the name of the real variable that controls
the loop.

There are two problems with the current code:

1) This struct that stores the subcell data is wrong. It is currently
defined as:

struct dvb_desc_t2_delivery_subcell {
uint8_t cell_id_extension;
uint16_t transposer_frequency;
} __attribute__((packed));

However, the transposer frequency is actually 32 bits. From the specs:

"transposer_frequency: This 32 bit field indicates the
 centre frequency that is used by a transposer in the sub-cell
 indicated. It is encoded in the same way as the centre_frequency
 field."

2) Right now, the code assumes just one table of centre_frequency.
According with the specs (at least v1.13.1 - with is the latest
documentation), multiple tables can exist.

I remember I tested it some years after the initial version, with a
DVB-T2 stream. On that time, there was just one frequency table,
e. g. just one cell ID.

Yet, as now DVB-T2 is spreading, I won't doubt that we'll find some
places that use multiple cell IDs.

At the end of the day, what really matters for a DVB scan program
is that all center_frequency and transposer_frequency to be
added to the frequencies that will be scanned.

So, I'm thinking on a way to make a patch that would be
backward-compatible, e. g. adding both "centre_frequency" and
"transposer_frequency" at the centre_frequency table, and not
filling the subcell IDs, as the additional field there (the
subcell ID) is useless without the cell ID, and its parsing is
broken, anyway.

We may latter add a way to store the cell ID and subcell ID at the
end of the structure.

Regards,
Mauro

Re: [PATCH 2/2] [media] platform: add video-multiplexer subdevice driver

[Peter Rosin]

On 2017-05-03 10:35, Philipp Zabel wrote:
> On Tue, 2017-05-02 at 19:42 +0200, Peter Rosin wrote:
>> On 2017-05-02 17:21, Philipp Zabel wrote:
>>> Thank you, I've resent a version with a mutex lock around vmux->active.
>>
>> I had a bunch of ifs in the above message, so I'm not sure it's needed.
>> I would expect there to be a lock outside somewhere in the media layer.
>> A cursory look gets me to media-entity.c and media_entity_setup_link()
>> which does have a mutex. But I'm no media expert, so maybe there are other
>> ways of getting to video_mux_link_setup that I'm not aware of?
> 
> link_setup is always called under the graph mutex of the /dev/media
> device. That is why I didn't think about locking too hard. In fact, I
> initially wrote this expecting mux_control_get_exclusive to exist and
> the mux select/deselect not to be locked at all.
> 
> But set_format is called from an unlocked ioctl on a /dev/v4l-subdev
> device. Until your comments I didn't notice that it would be possible to
> let link_setup set active = -1 in the middle of the set_format call,
> causing it to return garbage.

Obviously, now that you point it out. I completely missed set_format...

Cheers,
peda

Re: [RESEND v2 PATCH] v4l: omap_vout: vrfb: Convert to dmaengine

[Peter Ujfalusi]

On 2017-05-03 14:08, Peter Ujfalusi wrote:

The dmaengine driver for sDMA now have support for interleaved transfer.
This trasnfer type was open coded with the legacy omap-dma API, but now
we can move it to dmaengine.

Signed-off-by: Peter Ujfalusi <peter.ujfal...@ti.com>
---
Hi,

changes since RESEND (27.10.2016):
- rebased on next-20170503

I can not test it on real HW (still), but I have validated [1] that the change
is correct and should not cause any regression.

Laurent: can you verify the patch on a real hardware?


[1] 
https://github.com/omap-audio/linux-audio/blob/peter/linux-next-wip/drivers/misc/ovv_dmaengine.c




Regards,
Peter

 drivers/media/platform/omap/omap_vout_vrfb.c | 133 ---
 drivers/media/platform/omap/omap_voutdef.h   |   6 +-
 2 files changed, 83 insertions(+), 56 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c 
b/drivers/media/platform/omap/omap_vout_vrfb.c
index 92c4e1826356..45a553d4f5b2 100644
--- a/drivers/media/platform/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/omap/omap_vout_vrfb.c
@@ -16,7 +16,6 @@
 #include 
 #include 

-#include 
 #include 

 #include "omap_voutdef.h"
@@ -63,7 +62,7 @@ static int omap_vout_allocate_vrfb_buffers(struct 
omap_vout_device *vout,
 /*
  * Wakes up the application once the DMA transfer to VRFB space is completed.
  */
-static void omap_vout_vrfb_dma_tx_callback(int lch, u16 ch_status, void *data)
+static void omap_vout_vrfb_dma_tx_callback(void *data)
 {
struct vid_vrfb_dma *t = (struct vid_vrfb_dma *) data;

@@ -94,6 +93,7 @@ int omap_vout_setup_vrfb_bufs(struct platform_device *pdev, 
int vid_num,
int ret = 0, i, j;
struct omap_vout_device *vout;
struct video_device *vfd;
+   dma_cap_mask_t mask;
int image_width, image_height;
int vrfb_num_bufs = VRFB_NUM_BUFS;
struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
@@ -131,18 +131,27 @@ int omap_vout_setup_vrfb_bufs(struct platform_device 
*pdev, int vid_num,
/*
 * Request and Initialize DMA, for DMA based VRFB transfer
 */
-   vout->vrfb_dma_tx.dev_id = OMAP_DMA_NO_DEVICE;
-   vout->vrfb_dma_tx.dma_ch = -1;
-   vout->vrfb_dma_tx.req_status = DMA_CHAN_ALLOTED;
-   ret = omap_request_dma(vout->vrfb_dma_tx.dev_id, "VRFB DMA TX",
-   omap_vout_vrfb_dma_tx_callback,
-   (void *) >vrfb_dma_tx, >vrfb_dma_tx.dma_ch);
-   if (ret < 0) {
+   dma_cap_zero(mask);
+   dma_cap_set(DMA_INTERLEAVE, mask);
+   vout->vrfb_dma_tx.chan = dma_request_chan_by_mask();
+   if (IS_ERR(vout->vrfb_dma_tx.chan)) {
vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
+   } else {
+   size_t xt_size = sizeof(struct dma_interleaved_template) +
+sizeof(struct data_chunk);
+
+   vout->vrfb_dma_tx.xt = kzalloc(xt_size, GFP_KERNEL);
+   if (!vout->vrfb_dma_tx.xt) {
+   dma_release_channel(vout->vrfb_dma_tx.chan);
+   vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
+   }
+   }
+
+   if (vout->vrfb_dma_tx.req_status == DMA_CHAN_NOT_ALLOTED)
dev_info(>dev,
 ": failed to allocate DMA Channel for video%d\n",
 vfd->minor);
-   }
+
init_waitqueue_head(>vrfb_dma_tx.wait);

/* statically allocated the VRFB buffer is done through
@@ -177,7 +186,9 @@ void omap_vout_release_vrfb(struct omap_vout_device *vout)

if (vout->vrfb_dma_tx.req_status == DMA_CHAN_ALLOTED) {
vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
-   omap_free_dma(vout->vrfb_dma_tx.dma_ch);
+   kfree(vout->vrfb_dma_tx.xt);
+   dmaengine_terminate_sync(vout->vrfb_dma_tx.chan);
+   dma_release_channel(vout->vrfb_dma_tx.chan);
}
 }

@@ -219,70 +230,84 @@ int omap_vout_vrfb_buffer_setup(struct omap_vout_device 
*vout,
 }

 int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
-   struct videobuf_buffer *vb)
+  struct videobuf_buffer *vb)
 {
-   dma_addr_t dmabuf;
-   struct vid_vrfb_dma *tx;
+   struct dma_async_tx_descriptor *tx;
+   enum dma_ctrl_flags flags;
+   struct dma_chan *chan = vout->vrfb_dma_tx.chan;
+   struct dma_device *dmadev = chan->device;
+   struct dma_interleaved_template *xt = vout->vrfb_dma_tx.xt;
+   dma_cookie_t cookie;
+   enum dma_status status;
enum dss_rotation rotation;
-   u32 dest_frame_index = 0, src_element_index = 0;
-   u32 dest_element_index = 0, src_frame_index = 0;
-   u32 elem_count = 0, frame_count = 0, pixsize = 2;
+   size_t dst_icg;
+   u32 p

[RESEND v2 PATCH] v4l: omap_vout: vrfb: Convert to dmaengine

[Peter Ujfalusi]

The dmaengine driver for sDMA now have support for interleaved transfer.
This trasnfer type was open coded with the legacy omap-dma API, but now
we can move it to dmaengine.

Signed-off-by: Peter Ujfalusi <peter.ujfal...@ti.com>
---
Hi,

changes since RESEND (27.10.2016):
- rebased on next-20170503

I can not test it on real HW (still), but I have validated [1] that the change
is correct and should not cause any regression.

Laurent: can you verify the patch on a real hardware?

Regards,
Peter

 drivers/media/platform/omap/omap_vout_vrfb.c | 133 ---
 drivers/media/platform/omap/omap_voutdef.h   |   6 +-
 2 files changed, 83 insertions(+), 56 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c 
b/drivers/media/platform/omap/omap_vout_vrfb.c
index 92c4e1826356..45a553d4f5b2 100644
--- a/drivers/media/platform/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/omap/omap_vout_vrfb.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 
-#include 
 #include 
 
 #include "omap_voutdef.h"
@@ -63,7 +62,7 @@ static int omap_vout_allocate_vrfb_buffers(struct 
omap_vout_device *vout,
 /*
  * Wakes up the application once the DMA transfer to VRFB space is completed.
  */
-static void omap_vout_vrfb_dma_tx_callback(int lch, u16 ch_status, void *data)
+static void omap_vout_vrfb_dma_tx_callback(void *data)
 {
struct vid_vrfb_dma *t = (struct vid_vrfb_dma *) data;
 
@@ -94,6 +93,7 @@ int omap_vout_setup_vrfb_bufs(struct platform_device *pdev, 
int vid_num,
int ret = 0, i, j;
struct omap_vout_device *vout;
struct video_device *vfd;
+   dma_cap_mask_t mask;
int image_width, image_height;
int vrfb_num_bufs = VRFB_NUM_BUFS;
struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
@@ -131,18 +131,27 @@ int omap_vout_setup_vrfb_bufs(struct platform_device 
*pdev, int vid_num,
/*
 * Request and Initialize DMA, for DMA based VRFB transfer
 */
-   vout->vrfb_dma_tx.dev_id = OMAP_DMA_NO_DEVICE;
-   vout->vrfb_dma_tx.dma_ch = -1;
-   vout->vrfb_dma_tx.req_status = DMA_CHAN_ALLOTED;
-   ret = omap_request_dma(vout->vrfb_dma_tx.dev_id, "VRFB DMA TX",
-   omap_vout_vrfb_dma_tx_callback,
-   (void *) >vrfb_dma_tx, >vrfb_dma_tx.dma_ch);
-   if (ret < 0) {
+   dma_cap_zero(mask);
+   dma_cap_set(DMA_INTERLEAVE, mask);
+   vout->vrfb_dma_tx.chan = dma_request_chan_by_mask();
+   if (IS_ERR(vout->vrfb_dma_tx.chan)) {
vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
+   } else {
+   size_t xt_size = sizeof(struct dma_interleaved_template) +
+sizeof(struct data_chunk);
+
+   vout->vrfb_dma_tx.xt = kzalloc(xt_size, GFP_KERNEL);
+   if (!vout->vrfb_dma_tx.xt) {
+   dma_release_channel(vout->vrfb_dma_tx.chan);
+   vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
+   }
+   }
+
+   if (vout->vrfb_dma_tx.req_status == DMA_CHAN_NOT_ALLOTED)
dev_info(>dev,
 ": failed to allocate DMA Channel for video%d\n",
 vfd->minor);
-   }
+
init_waitqueue_head(>vrfb_dma_tx.wait);
 
/* statically allocated the VRFB buffer is done through
@@ -177,7 +186,9 @@ void omap_vout_release_vrfb(struct omap_vout_device *vout)
 
if (vout->vrfb_dma_tx.req_status == DMA_CHAN_ALLOTED) {
vout->vrfb_dma_tx.req_status = DMA_CHAN_NOT_ALLOTED;
-   omap_free_dma(vout->vrfb_dma_tx.dma_ch);
+   kfree(vout->vrfb_dma_tx.xt);
+   dmaengine_terminate_sync(vout->vrfb_dma_tx.chan);
+   dma_release_channel(vout->vrfb_dma_tx.chan);
}
 }
 
@@ -219,70 +230,84 @@ int omap_vout_vrfb_buffer_setup(struct omap_vout_device 
*vout,
 }
 
 int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
-   struct videobuf_buffer *vb)
+  struct videobuf_buffer *vb)
 {
-   dma_addr_t dmabuf;
-   struct vid_vrfb_dma *tx;
+   struct dma_async_tx_descriptor *tx;
+   enum dma_ctrl_flags flags;
+   struct dma_chan *chan = vout->vrfb_dma_tx.chan;
+   struct dma_device *dmadev = chan->device;
+   struct dma_interleaved_template *xt = vout->vrfb_dma_tx.xt;
+   dma_cookie_t cookie;
+   enum dma_status status;
enum dss_rotation rotation;
-   u32 dest_frame_index = 0, src_element_index = 0;
-   u32 dest_element_index = 0, src_frame_index = 0;
-   u32 elem_count = 0, frame_count = 0, pixsize = 2;
+   size_t dst_icg;
+   u32 pixsize;
 
if (!is_rotation_enabled(vout))
return 0;
 
-   dmabuf = vout->buf_phy_addr[vb->i];
/* If rotation is enabled, c

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Tuukka Toivonen]

Hi Sakari,

On Wednesday, May 03, 2017 11:58:01 Sakari Ailus wrote:
> Hi Yong,
> 
> A few more minor comments below...
> 
> On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
> ...
> > +/*** V4L2 sub-device asynchronous registration callbacks***/
> > +
> > +static struct cio2_queue *cio2_find_queue_by_sensor_node(struct cio2_queue 
> > *q,
> > +   struct fwnode_handle *fwnode)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < CIO2_QUEUES; i++) {
> > +   if (q[i].sensor->fwnode == fwnode)
> > +   return [i];
> > +   }
> > +
> > +   return NULL;
> > +}
> > +
> > +/* The .bound() notifier callback when a match is found */
> > +static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> > +  struct v4l2_subdev *sd,
> > +  struct v4l2_async_subdev *asd)
> > +{
> > +   struct cio2_device *cio2 = container_of(notifier,
> > +   struct cio2_device, notifier);
> > +   struct sensor_async_subdev *s_asd = container_of(asd,
> > +   struct sensor_async_subdev, asd);
> > +   struct cio2_queue *q;
> > +   struct device *dev;
> > +   int i;
> > +
> > +   dev = >pci_dev->dev;
> > +
> > +   /* Find first free slot for the subdev */
> > +   for (i = 0; i < CIO2_QUEUES; i++)
> > +   if (!cio2->queue[i].sensor)
> > +   break;
> > +
> > +   if (i >= CIO2_QUEUES) {
> > +   dev_err(dev, "too many subdevs\n");
> > +   return -ENOSPC;
> > +   }
> > +   q = >queue[i];
> > +
> > +   q->csi2.port = s_asd->vfwn_endpt.base.port;
> > +   q->csi2.num_of_lanes = s_asd->vfwn_endpt.bus.mipi_csi2.num_data_lanes;
> > +   q->sensor = sd;
> > +   q->csi_rx_base = cio2->base + CIO2_REG_PIPE_BASE(q->csi2.port);
> > +
> > +   return 0;
> > +}
> > +
> > +/* The .unbind callback */
> > +static void cio2_notifier_unbind(struct v4l2_async_notifier *notifier,
> > +struct v4l2_subdev *sd,
> > +struct v4l2_async_subdev *asd)
> > +{
> > +   struct cio2_device *cio2 = container_of(notifier,
> > +   struct cio2_device, notifier);
> > +   unsigned int i;
> > +
> > +   /* Note: sd may here point to unallocated memory. Do not access. */
> 
> When can this happen?

If v4l2_device_register_subdev() fails, it may lead to
calling subdevice's remove function and the devres system
freeing the subdevice before unbind is called. This did
happen during driver development.

> 
> > +   for (i = 0; i < CIO2_QUEUES; i++) {
> > +   if (cio2->queue[i].sensor == sd) {
> > +   cio2->queue[i].sensor = NULL;
> > +   return;
> > +   }
> > +   }
> > +}
> > +
> > +/* .complete() is called after all subdevices have been located */
> > +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> > +{
> > +   struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
> > +   notifier);
> > +   struct sensor_async_subdev *s_asd;
> > +   struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
> > +   struct cio2_queue *q;
> > +   struct fwnode_endpoint remote_endpt;
> > +   int i, ret;
> > +
> > +   for (i = 0; i < notifier->num_subdevs; i++) {
> > +   s_asd = container_of(cio2->notifier.subdevs[i],
> > +   struct sensor_async_subdev,
> > +   asd);
> 
> Fits on previous line. It's a good practice to align the further function
> arguments wrapped on the following lines to the character just right of the
> opening parenthesis, e.g.
> 
> ret = foo(bar,
> foobar);
> 
> > +
> > +   fwn_remote = s_asd->asd.match.fwnode.fwn;
> > +   fwn_endpt = (struct fwnode_handle *)
> > +   s_asd->vfwn_endpt.base.local_fwnode;
> > +   fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
> > +   if (!fwn_remote_endpt) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to get remote endpt %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = fwnode_graph_parse_endpoint(fwn_remote_endpt,
> > +   _endpt);
> > +   if (ret) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to parse remote endpt %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   q = cio2_find_queue_by_sensor_node(cio2->queue, fwn_remote);
> > +   if (!q) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to find cio2 queue %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = media_create_pad_link(
> > +   

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Tuukka Toivonen]

Hi Sakari,

Thanks for the comments.

On Tuesday, May 02, 2017 16:00:20 Sakari Ailus wrote:
> Hi Yong,
> 
> Thanks for the patches! Some comments below.
> 
> On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
> > This patch adds CIO2 CSI-2 device driver for
> > Intel's IPU3 camera sub-system support.
> > 
> > The V4L2 fwnode matching depends on the following work:
> > 
> > 
> > 
> > Signed-off-by: Yong Zhi 
> > ---
> >  drivers/media/pci/Kconfig  |2 +
> >  drivers/media/pci/Makefile |3 +-
> >  drivers/media/pci/ipu3/Kconfig |   17 +
> >  drivers/media/pci/ipu3/Makefile|1 +
> >  drivers/media/pci/ipu3/ipu3-cio2.c | 1813 
> > 
> >  drivers/media/pci/ipu3/ipu3-cio2.h |  425 +
> >  6 files changed, 2260 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/media/pci/ipu3/Kconfig
> >  create mode 100644 drivers/media/pci/ipu3/Makefile
> >  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.c
> >  create mode 100644 drivers/media/pci/ipu3/ipu3-cio2.h
> > 
> > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
> > index da28e68..63ece75 100644
> > --- a/drivers/media/pci/Kconfig
> > +++ b/drivers/media/pci/Kconfig
> > @@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
> >  source "drivers/media/pci/netup_unidvb/Kconfig"
> >  endif
> >  
> > +source "drivers/media/pci/ipu3/Kconfig"
> > +
> >  endif #MEDIA_PCI_SUPPORT
> >  endif #PCI
> > diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
> > index a7e8af0..8d5e8db 100644
> > --- a/drivers/media/pci/Makefile
> > +++ b/drivers/media/pci/Makefile
> > @@ -13,7 +13,8 @@ obj-y+=   ttpci/  \
> > ddbridge/   \
> > saa7146/\
> > smipcie/\
> > -   netup_unidvb/
> > +   netup_unidvb/   \
> > +   ipu3/
> >  
> >  obj-$(CONFIG_VIDEO_IVTV) += ivtv/
> >  obj-$(CONFIG_VIDEO_ZORAN) += zoran/
> > diff --git a/drivers/media/pci/ipu3/Kconfig b/drivers/media/pci/ipu3/Kconfig
> > new file mode 100644
> > index 000..2a895d6
> > --- /dev/null
> > +++ b/drivers/media/pci/ipu3/Kconfig
> > @@ -0,0 +1,17 @@
> > +config VIDEO_IPU3_CIO2
> > +   tristate "Intel ipu3-cio2 driver"
> > +   depends on VIDEO_V4L2 && PCI
> > +   depends on MEDIA_CONTROLLER
> > +   depends on HAS_DMA
> > +   depends on ACPI
> > +   select V4L2_FWNODE
> > +   select VIDEOBUF2_DMA_SG
> > +
> > +   ---help---
> > +   This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
> > +   Skylake and Kaby Lake SoCs and used for capturing images and
> > +   video from a camera sensor.
> > +
> > +   Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> > +   connected camera.
> > +   The module will be called ipu3-cio2.
> > diff --git a/drivers/media/pci/ipu3/Makefile 
> > b/drivers/media/pci/ipu3/Makefile
> > new file mode 100644
> > index 000..20186e3
> > --- /dev/null
> > +++ b/drivers/media/pci/ipu3/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> > diff --git a/drivers/media/pci/ipu3/ipu3-cio2.c 
> > b/drivers/media/pci/ipu3/ipu3-cio2.c
> > new file mode 100644
> > index 000..2b641ad
> > --- /dev/null
> > +++ b/drivers/media/pci/ipu3/ipu3-cio2.c
> > @@ -0,0 +1,1813 @@
> > +/*
> > + * 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.
> > + *
> > + * Based partially on Intel IPU4 driver written by
> > + *  Sakari Ailus 
> > + *  Samu Onkalo 
> > + *  Jouni Högander 
> > + *  Jouni Ukkonen 
> > + *  Antti Laakso 
> > + * et al.
> > + *
> > + */
> > +
> > +#include 
> 
> I believe you shouldn't need this one.
> 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> Alphabetical order, please.
> 
> > +#include "ipu3-cio2.h"
> > +
> > +MODULE_AUTHOR("Tianshu Qiu ");
> > +MODULE_AUTHOR("Jian Xu Zheng ");
> > +MODULE_AUTHOR("Yuning Pu ");
> > +MODULE_AUTHOR("Tuukka Toivonen ");
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("IPU3 CIO2 driver");
> > +
> > +/*
> > + * These are raw formats used in Intel's third generation 

[PATCH] rc-core: cleanup rc_register_device (v2)

[David Härdeman]

The device core infrastructure is based on the presumption that
once a driver calls device_add(), it must be ready to accept
userspace interaction.

This requires splitting rc_setup_rx_device() into two functions
and reorganizing rc_register_device() so that as much work
as possible is performed before calling device_add().

Version 2: switch the order in which rc_prepare_rx_device() and
ir_raw_event_prepare() gets called so that dev->change_protocol()
gets called before device_add().

Signed-off-by: David Härdeman 
---
 drivers/media/rc/rc-core-priv.h |2 +
 drivers/media/rc/rc-ir-raw.c|   34 --
 drivers/media/rc/rc-main.c  |   75 +--
 3 files changed, 73 insertions(+), 38 deletions(-)

diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 0455b273c2fc..b3e7cac2c3ee 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -263,7 +263,9 @@ int ir_raw_gen_pl(struct ir_raw_event **ev, unsigned int 
max,
  * Routines from rc-raw.c to be used internally and by decoders
  */
 u64 ir_raw_get_allowed_protocols(void);
+int ir_raw_event_prepare(struct rc_dev *dev);
 int ir_raw_event_register(struct rc_dev *dev);
+void ir_raw_event_free(struct rc_dev *dev);
 void ir_raw_event_unregister(struct rc_dev *dev);
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
 void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler);
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 90f66dc7c0d7..ae7785c4fbe7 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -486,14 +486,18 @@ EXPORT_SYMBOL(ir_raw_encode_scancode);
 /*
  * Used to (un)register raw event clients
  */
-int ir_raw_event_register(struct rc_dev *dev)
+int ir_raw_event_prepare(struct rc_dev *dev)
 {
-   int rc;
-   struct ir_raw_handler *handler;
+   static bool raw_init; /* 'false' default value, raw decoders loaded? */
 
if (!dev)
return -EINVAL;
 
+   if (!raw_init) {
+   request_module("ir-lirc-codec");
+   raw_init = true;
+   }
+
dev->raw = kzalloc(sizeof(*dev->raw), GFP_KERNEL);
if (!dev->raw)
return -ENOMEM;
@@ -502,6 +506,13 @@ int ir_raw_event_register(struct rc_dev *dev)
dev->change_protocol = change_protocol;
INIT_KFIFO(dev->raw->kfifo);
 
+   return 0;
+}
+
+int ir_raw_event_register(struct rc_dev *dev)
+{
+   struct ir_raw_handler *handler;
+
/*
 * raw transmitters do not need any event registration
 * because the event is coming from userspace
@@ -510,10 +521,8 @@ int ir_raw_event_register(struct rc_dev *dev)
dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
   "rc%u", dev->minor);
 
-   if (IS_ERR(dev->raw->thread)) {
-   rc = PTR_ERR(dev->raw->thread);
-   goto out;
-   }
+   if (IS_ERR(dev->raw->thread))
+   return PTR_ERR(dev->raw->thread);
}
 
mutex_lock(_raw_handler_lock);
@@ -524,11 +533,15 @@ int ir_raw_event_register(struct rc_dev *dev)
mutex_unlock(_raw_handler_lock);
 
return 0;
+}
+
+void ir_raw_event_free(struct rc_dev *dev)
+{
+   if (!dev)
+   return;
 
-out:
kfree(dev->raw);
dev->raw = NULL;
-   return rc;
 }
 
 void ir_raw_event_unregister(struct rc_dev *dev)
@@ -547,8 +560,7 @@ void ir_raw_event_unregister(struct rc_dev *dev)
handler->raw_unregister(dev);
mutex_unlock(_raw_handler_lock);
 
-   kfree(dev->raw);
-   dev->raw = NULL;
+   ir_raw_event_free(dev);
 }
 
 /*
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 802e559cc30e..f3bc9f4e2b96 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1663,7 +1663,7 @@ struct rc_dev *devm_rc_allocate_device(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_rc_allocate_device);
 
-static int rc_setup_rx_device(struct rc_dev *dev)
+static int rc_prepare_rx_device(struct rc_dev *dev)
 {
int rc;
struct rc_map *rc_map;
@@ -1708,10 +1708,22 @@ static int rc_setup_rx_device(struct rc_dev *dev)
dev->input_dev->phys = dev->input_phys;
dev->input_dev->name = dev->input_name;
 
+   return 0;
+
+out_table:
+   ir_free_table(>rc_map);
+
+   return rc;
+}
+
+static int rc_setup_rx_device(struct rc_dev *dev)
+{
+   int rc;
+
/* rc_open will be called here */
rc = input_register_device(dev->input_dev);
if (rc)
-   goto out_table;
+   return rc;
 
/*
 * Default delay of 250ms is too short for some protocols, especially
@@ -1729,27 +1741,23 @@ static int rc_setup_rx_device(struct rc_dev *dev)

Re: [PATCH 2/6] rc-core: cleanup rc_register_device

[David Härdeman]

On Tue, May 02, 2017 at 09:48:26PM +0100, Sean Young wrote:
>On Tue, May 02, 2017 at 08:53:02PM +0200, David Härdeman wrote:
>> On Mon, May 01, 2017 at 07:47:25PM +0200, David Härdeman wrote:
>> >On Mon, May 01, 2017 at 05:49:53PM +0100, Sean Young wrote:
>> >>On Thu, Apr 27, 2017 at 10:34:03PM +0200, David Härdeman wrote:
>> >>> The device core infrastructure is based on the presumption that
>> >>> once a driver calls device_add(), it must be ready to accept
>> >>> userspace interaction.
>> >>> 
>> >>> This requires splitting rc_setup_rx_device() into two functions
>> >>> and reorganizing rc_register_device() so that as much work
>> >>> as possible is performed before calling device_add().
>> >>> 
>> >>
>> >>With this patch applied, I'm no longer getting any scancodes from
>> >>my rc devices.
>> >>
>> >>David, please can you test your patches before submitting. I have to go
>> >>over them meticulously because I cannot assume you've tested them.
>> >
>> >I did test this patch and I just redid the tests, both with rc-loopback
>> >and with a mceusb receiver. I'm seeing scancodes on the input device as
>> >well as pulse-space readings on the lirc device in both tests.
>> >
>> >I did the tests with only this patch applied and the lirc-use-after-free
>> >(v3). What hardware did you test with?
>> >
>> >Meanwhile, I'll try rebasing my patches to the latest version of the
>> >media-master git tree and test again.
>> 
>> I rebased the patches onto media-master (commit
>> 3622d3e77ecef090b5111e3c5423313f11711dfa) and tested again. I still
>> can't reproduce the problems you're having :/
>
>The protocol is not set properly. In rc_prepare_rx_device(), 
>dev->change_protocol() is call if not null. At this point it still is
>null, since it will only be set up in ir_raw_event_prepare(), which
>is called afterwards.

Ah, good catch. Since ir_raw_event_prepare() only does a kmalloc() and
sets the change_protocol pointer it should be fine to call
ir_raw_event_prepare() before rc_prepare_rx_device(). I'll prepare a v2
of the patch.

>Presumably you have udev set up to execute ir-keytable, which sets the
>protocol up (again).

Well, kind of. In the automated testing I use rc-loopback which has the
"rc-empty" keytable so it doesn't set the protocols. In my manual
testing I used mceusb with a NEC remote so I anyway needed to set the
protocols manually and I missed the fact that "[rc-6]" was no longer set
in sysfs.

>There is another problem with your patches, you've introduced a race
>condition. When device_add() is called, the protocol is not set up yet.
>So anyone reading the protocols sysfs attribute early enough will get
>false information. Is it not possible to make sure that it is all setup
>correctly at the point of device_add()?

Isn't this the same problem? If dev->change_protocol() isn't NULL when
rc_prepare_rx_device() is called then the protocol will be set up (i.e.
dev->enabled_protcols will be set to the right value). Or did I
misunderstand you?

-- 
David Härdeman

Re: [PATCH v2 3/3] libv4l-codecparsers: add GStreamer mpeg2 parser

[Hugues FRUCHET]

Thanks Nicolas,
I will update configure.ac accordingly.
BR,
Hugues.

On 05/01/2017 07:37 PM, Nicolas Dufresne wrote:
> Le vendredi 28 avril 2017 à 17:02 +0200, Hugues Fruchet a écrit :
>> Add the mpeg2 codecparser backend glue which will
>> call the GStreamer parsing functions.
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>  configure.ac|  21 ++
>>  lib/libv4l-codecparsers/Makefile.am |  14 +-
>>  lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c | 375
>> 
>>  lib/libv4l-codecparsers/libv4l-cparsers.c   |   4 +
>>  4 files changed, 413 insertions(+), 1 deletion(-)
>>  create mode 100644 lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 9ce7392..ce43f18 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -273,6 +273,25 @@ fi
>>
>>  AC_SUBST([JPEG_LIBS])
>>
>> +# Check for GStreamer codecparsers
>> +
>> +gst_codecparsers_pkgconfig=false
>> +PKG_CHECK_MODULES([GST], [gstreamer-1.0 >= 1.8.0],
>> [gst_pkgconfig=true], [gst_pkgconfig=false])
>> +if test "x$gst_pkgconfig" = "xfalse"; then
>> +   AC_MSG_WARN(GStreamer library is not available)
>> +else
>> +   PKG_CHECK_MODULES([GST_BASE], [gstreamer-base-1.0 >= 1.8.0],
>> [gst_base_pkgconfig=true], [gst_base_pkgconfig=false])
>> +   if test "x$gst_base_pkgconfig" = "xfalse"; then
>> +  AC_MSG_WARN(GStreamer base library is not available)
>> +   else
>> +  PKG_CHECK_MODULES(GST_CODEC_PARSERS, [gstreamer-codecparsers-
>> 1.0 >= 1.8.0], [gst_codecparsers_pkgconfig=true],
>
> You should only check for the codecparser library. The rest are
> dependencies which will be pulled automatically by pkg-config. If for
> some reason you needed multiple libs, that don't depend on each others,
> notice the S in PKG_CHECK_MODULES. You can do a single invocation.
>
>> [gst_codecparsers_pkgconfig=false])
>> +  if test "x$gst_codecparsers_pkgconfig" = "xfalse"; then
>> + AC_MSG_WARN(GStreamer codecparser library is not available)
>> +  fi
>> +   fi
>> +fi
>> +AM_CONDITIONAL([HAVE_GST_CODEC_PARSERS], [test
>> x$gst_codecparsers_pkgconfig = xtrue])
>> +
>>  # Check for pthread
>>
>>  AS_IF([test x$enable_shared != xno],
>> @@ -477,6 +496,7 @@ AM_COND_IF([WITH_V4L2_CTL_LIBV4L],
>> [USE_V4L2_CTL="yes"], [USE_V4L2_CTL="no"])
>>  AM_COND_IF([WITH_V4L2_CTL_STREAM_TO], [USE_V4L2_CTL="yes"],
>> [USE_V4L2_CTL="no"])
>>  AM_COND_IF([WITH_V4L2_COMPLIANCE_LIBV4L],
>> [USE_V4L2_COMPLIANCE="yes"], [USE_V4L2_COMPLIANCE="no"])
>>  AS_IF([test "x$alsa_pkgconfig" = "xtrue"], [USE_ALSA="yes"],
>> [USE_ALSA="no"])
>> +AS_IF([test "x$gst_codecparsers_pkgconfig" = "xtrue"],
>> [USE_GST_CODECPARSERS="yes"], [USE_GST_CODECPARSERS="no"])
>>
>>  AC_OUTPUT
>>
>> @@ -497,6 +517,7 @@ compile time options summary
>>  pthread : $have_pthread
>>  QT version  : $QT_VERSION
>>  ALSA support: $USE_ALSA
>> +GST codecparsers: $USE_GST_CODECPARSERS
>>
>>  build dynamic libs  : $enable_shared
>>  build static libs   : $enable_static
>> diff --git a/lib/libv4l-codecparsers/Makefile.am b/lib/libv4l-
>> codecparsers/Makefile.am
>> index a9d6c8b..61f4730 100644
>> --- a/lib/libv4l-codecparsers/Makefile.am
>> +++ b/lib/libv4l-codecparsers/Makefile.am
>> @@ -1,9 +1,21 @@
>>  if WITH_V4L_PLUGINS
>> +if HAVE_GST_CODEC_PARSERS
>> +
>>  libv4l2plugin_LTLIBRARIES = libv4l-codecparsers.la
>> -endif
>>
>>  libv4l_codecparsers_la_SOURCES = libv4l-cparsers.c libv4l-cparsers.h
>>
>>  libv4l_codecparsers_la_CPPFLAGS = $(CFLAG_VISIBILITY)
>> -I$(top_srcdir)/lib/libv4l2/ -I$(top_srcdir)/lib/libv4lconvert/
>>  libv4l_codecparsers_la_LDFLAGS = -avoid-version -module -shared
>> -export-dynamic -lpthread
>>  libv4l_codecparsers_la_LIBADD = ../libv4l2/libv4l2.la
>> +
>> +# GStreamer codecparsers library
>> +libv4l_codecparsers_la_CFLAGS = $(GST_CFLAGS) -DGST_USE_UNSTABLE_API
>> +libv4l_codecparsers_la_LDFLAGS += $(GST_LIB_LDFLAGS)
>> +libv4l_codecparsers_la_LIBADD += $(GLIB_LIBS) $(GST_LIBS)
>> $(GST_BASE_LIBS) $(GST_CODEC_PARSERS_LIBS) $(NULL)
>> +
>> +# MPEG-2 parser back-end
>> +libv4l_codecparsers_la_SOURCES += libv4l-cparsers-mpeg2.c
>> +
>> +endif
>> +endif
>> diff --git a/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
>> b/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
>> new file mode 100644
>> index 000..3456b73
>> --- /dev/null
>> +++ b/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
>> @@ -0,0 +1,375 @@
>> +/*
>> + * libv4l-cparsers-mpeg2.c
>> + *
>> + * Copyright (C) STMicroelectronics SA 2017
>> + * Authors: Hugues Fruchet 
>> + *  Tifaine Inguere 
>> + *  for STMicroelectronics.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU Lesser General Public License as
>> published by
>> + * the Free Software Foundation; either version 2.1 of the License,
>> or
>> + * 

Re: [PATCH v5] media: platform: Renesas IMR driver

[Laurent Pinchart]

Hi Geert,

On Wednesday 03 May 2017 09:22:00 Geert Uytterhoeven wrote:
> On Tue, May 2, 2017 at 11:17 PM, Laurent Pinchart wrote:
> > On Wednesday 22 Mar 2017 10:34:16 Geert Uytterhoeven wrote:
> >> On Thu, Mar 9, 2017 at 9:08 PM, Sergei Shtylyov wrote:
> >>> --- /dev/null
> >>> +++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
> >>> @@ -0,0 +1,27 @@
> >>> +Renesas R-Car Image Renderer (Distortion Correction Engine)
> >>> +---
> >>> +
> >>> +The image renderer, or the distortion correction engine, is a drawing
> >>> processor
> >>> +with a simple instruction system capable of referencing video capture
> >>> data or
> >>> +data in an external memory as 2D texture data and performing texture
> >>> mapping
> >>> +and drawing with respect to any shape that is split into triangular
> >>> objects.
> >>> +
> >>> +Required properties:
> >>> +
> >>> +- compatible: "renesas,-imr-lx4", "renesas,imr-lx4" as a
> >>> fallback for
> >>> +  the image renderer light extended 4 (IMR-LX4) found in the R-Car
> >>> gen3 SoCs,
> >>> +  where the examples with  are:
> >>> +  - "renesas,r8a7795-imr-lx4" for R-Car H3,
> >>> +  - "renesas,r8a7796-imr-lx4" for R-Car M3-W.
> >> 
> >> Laurent: what do you think about the need for SoC-specific compatible
> >> values for the various IM* blocks?
> > 
> > There's no documented IP core version register, but when dumping all
> > configuration registers on H3 and M3-W I noticed that register 0x002c, not
> > documented in the datasheet, reads 0x14060514 on all four IMR instances in
> > H3, and 0x20150505 on both instances in M3-W.
> > 
> > This looks like a version register to me. If my assumption is correct, we
> > could do without any SoC-specific compatible string.
> 
> I read this assumed version registers on all R-Car SoCs, after writing
> zero to 0xe6150990 (SMSTPCR8).
> 
> IMR-X2 on R-Car H2: 0x12072009
> IMR-LSX2 on R-Car H2:   0x12072009
> IMR-LSX3 on R-Car V2H:  0x13052617
> IMR-LX2 on R-Car M2-W:  0x12072009
> IMR-LX2 on R-Car M2-N:  0x12072009
> IMR-LX2 on R-Car E2:0x13091909
> IMR-LX3 on R-Car V2H:   0x13052617
> 
> Note that several IDs are the same, but you know the type from the
> compatible value.
> 
> It would be good to get confirmation from the hardware team that this is
> indeed a version register.

Thank you for checking.

Morimoto-san, do you think there are still people alive in the Gen2 hardware 
team who could provide the information ? :-) If not, information restricted to 
Gen3 would still be useful.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] [media] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Sakari Ailus]

Hi Yong,

A few more minor comments below...

On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
...
> +/*** V4L2 sub-device asynchronous registration callbacks***/
> +
> +static struct cio2_queue *cio2_find_queue_by_sensor_node(struct cio2_queue 
> *q,
> + struct fwnode_handle *fwnode)
> +{
> + int i;
> +
> + for (i = 0; i < CIO2_QUEUES; i++) {
> + if (q[i].sensor->fwnode == fwnode)
> + return [i];
> + }
> +
> + return NULL;
> +}
> +
> +/* The .bound() notifier callback when a match is found */
> +static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> +struct v4l2_subdev *sd,
> +struct v4l2_async_subdev *asd)
> +{
> + struct cio2_device *cio2 = container_of(notifier,
> + struct cio2_device, notifier);
> + struct sensor_async_subdev *s_asd = container_of(asd,
> + struct sensor_async_subdev, asd);
> + struct cio2_queue *q;
> + struct device *dev;
> + int i;
> +
> + dev = >pci_dev->dev;
> +
> + /* Find first free slot for the subdev */
> + for (i = 0; i < CIO2_QUEUES; i++)
> + if (!cio2->queue[i].sensor)
> + break;
> +
> + if (i >= CIO2_QUEUES) {
> + dev_err(dev, "too many subdevs\n");
> + return -ENOSPC;
> + }
> + q = >queue[i];
> +
> + q->csi2.port = s_asd->vfwn_endpt.base.port;
> + q->csi2.num_of_lanes = s_asd->vfwn_endpt.bus.mipi_csi2.num_data_lanes;
> + q->sensor = sd;
> + q->csi_rx_base = cio2->base + CIO2_REG_PIPE_BASE(q->csi2.port);
> +
> + return 0;
> +}
> +
> +/* The .unbind callback */
> +static void cio2_notifier_unbind(struct v4l2_async_notifier *notifier,
> +  struct v4l2_subdev *sd,
> +  struct v4l2_async_subdev *asd)
> +{
> + struct cio2_device *cio2 = container_of(notifier,
> + struct cio2_device, notifier);
> + unsigned int i;
> +
> + /* Note: sd may here point to unallocated memory. Do not access. */

When can this happen?

> + for (i = 0; i < CIO2_QUEUES; i++) {
> + if (cio2->queue[i].sensor == sd) {
> + cio2->queue[i].sensor = NULL;
> + return;
> + }
> + }
> +}
> +
> +/* .complete() is called after all subdevices have been located */
> +static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
> + notifier);
> + struct sensor_async_subdev *s_asd;
> + struct fwnode_handle *fwn_remote, *fwn_endpt, *fwn_remote_endpt;
> + struct cio2_queue *q;
> + struct fwnode_endpoint remote_endpt;
> + int i, ret;
> +
> + for (i = 0; i < notifier->num_subdevs; i++) {
> + s_asd = container_of(cio2->notifier.subdevs[i],
> + struct sensor_async_subdev,
> + asd);

Fits on previous line. It's a good practice to align the further function
arguments wrapped on the following lines to the character just right of the
opening parenthesis, e.g.

ret = foo(bar,
  foobar);

> +
> + fwn_remote = s_asd->asd.match.fwnode.fwn;
> + fwn_endpt = (struct fwnode_handle *)
> + s_asd->vfwn_endpt.base.local_fwnode;
> + fwn_remote_endpt = fwnode_graph_get_remote_endpoint(fwn_endpt);
> + if (!fwn_remote_endpt) {
> + dev_err(>pci_dev->dev,
> + "failed to get remote endpt %d\n", ret);
> + return ret;
> + }
> +
> + ret = fwnode_graph_parse_endpoint(fwn_remote_endpt,
> + _endpt);
> + if (ret) {
> + dev_err(>pci_dev->dev,
> + "failed to parse remote endpt %d\n", ret);
> + return ret;
> + }
> +
> + q = cio2_find_queue_by_sensor_node(cio2->queue, fwn_remote);
> + if (!q) {
> + dev_err(>pci_dev->dev,
> + "failed to find cio2 queue %d\n", ret);
> + return ret;
> + }
> +
> + ret = media_create_pad_link(
> + >sensor->entity, remote_endpt.id,
> + >subdev.entity, s_asd->vfwn_endpt.base.id,
> + 0);
> + if (ret) {
> + dev_err(>pci_dev->dev,
> + "failed to create link for %s\n",
> + cio2->queue[i].sensor->name);
> +  

Re: [PATCH 2/2] [media] platform: add video-multiplexer subdevice driver

[Philipp Zabel]

On Tue, 2017-05-02 at 19:42 +0200, Peter Rosin wrote:
> On 2017-05-02 17:21, Philipp Zabel wrote:
> > On Sat, 2017-04-29 at 23:42 +0200, Peter Rosin wrote:
> >> On 2017-04-29 23:29, Peter Rosin wrote:
> >>> On 2017-04-28 16:13, Philipp Zabel wrote:
>  This driver can handle SoC internal and external video bus multiplexers,
>  controlled by mux controllers provided by the mux controller framework,
>  such as MMIO register bitfields or GPIOs. The subdevice passes through
>  the mbus configuration of the active input to the output side.
> 
>  Signed-off-by: Sascha Hauer 
>  Signed-off-by: Philipp Zabel 
>  Signed-off-by: Steve Longerbeam 
>  ---
>  This has been last sent as part of the i.MX media series.
> 
>  Changes since https://patchwork.kernel.org/patch/9647869/:
>   - Split out the actual mux operation to be provided by the mux 
>  controller
> framework [1]. GPIO and MMIO control can be provided by individual mux
> controller drivers [2][3].
> [1] https://patchwork.kernel.org/patch/9695837/
> [2] https://patchwork.kernel.org/patch/9695839/
> [3] https://patchwork.kernel.org/patch/9704509/
>   - Shortened 'video-multiplexer' to 'video-mux', replaced all instances 
>  of
> vidsw with video_mux.
>   - Made the mux inactive by default, only activated by user interaction.
>   - Added CONFIG_OF and CONFIG_MULTIPLEXER dependencies.
>   - Reuse subdev.entity.num_pads instead of keeping our own count.
>   - Removed implicit link disabling. Instead, trying to enable a second
> sink pad link yields -EBUSY.
>   - Merged _async_init into _probe.
>   - Removed superfluous pad index check from _set_format.
>   - Added is_source_pad helper to tell source and sink pads apart.
>   - Removed test for status property in endpoint nodes. Disable the remote
> device or sever the endpoint link to disable a sink pad.
>  ---
>   drivers/media/platform/Kconfig |   6 +
>   drivers/media/platform/Makefile|   2 +
>   drivers/media/platform/video-mux.c | 341 
>  +
>   3 files changed, 349 insertions(+)
>   create mode 100644 drivers/media/platform/video-mux.c
> 
>  diff --git a/drivers/media/platform/Kconfig 
>  b/drivers/media/platform/Kconfig
>  index c9106e105baba..b046a6d39fee5 100644
>  --- a/drivers/media/platform/Kconfig
>  +++ b/drivers/media/platform/Kconfig
>  @@ -74,6 +74,12 @@ config VIDEO_M32R_AR_M64278
> To compile this driver as a module, choose M here: the
> module will be called arv.
>   
>  +config VIDEO_MUX
>  +tristate "Video Multiplexer"
>  +depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER && 
>  MULTIPLEXER
>  +help
>  +  This driver provides support for N:1 video bus multiplexers.
>  +
>   config VIDEO_OMAP3
>   tristate "OMAP 3 Camera support"
>   depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && 
>  ARCH_OMAP3
>  diff --git a/drivers/media/platform/Makefile 
>  b/drivers/media/platform/Makefile
>  index 349ddf6a69da2..fd2735ca3ff75 100644
>  --- a/drivers/media/platform/Makefile
>  +++ b/drivers/media/platform/Makefile
>  @@ -27,6 +27,8 @@ obj-$(CONFIG_VIDEO_SH_VEU) += sh_veu.o
>   
>   obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE) += m2m-deinterlace.o
>   
>  +obj-$(CONFIG_VIDEO_MUX) += video-mux.o
>  +
>   obj-$(CONFIG_VIDEO_S3C_CAMIF)   += s3c-camif/
>   obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS)  += exynos4-is/
>   obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)+= s5p-jpeg/
>  diff --git a/drivers/media/platform/video-mux.c 
>  b/drivers/media/platform/video-mux.c
>  new file mode 100644
>  index 0..419541729f67e
>  --- /dev/null
>  +++ b/drivers/media/platform/video-mux.c
>  @@ -0,0 +1,341 @@
>  +/*
>  + * video stream multiplexer controlled via mux control
>  + *
>  + * Copyright (C) 2013 Pengutronix, Sascha Hauer 
>  + * Copyright (C) 2016 Pengutronix, Philipp Zabel 
> >>>
> >>> 2017?
> >>>
>  + *
>  + * 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 

Re: [PATCHv2] omap3isp: add support for CSI1 bus

[Sakari Ailus]

Hi Pavel,

On 03/04/17 15:03, Sakari Ailus wrote:
> Hi Pavel,
> 
> On Thu, Mar 02, 2017 at 01:38:48PM +0100, Pavel Machek wrote:
>> Hi!
>>
 Ok, how about this one?
 omap3isp: add rest of CSI1 support
 
 CSI1 needs one more bit to be set up. Do just that.
 
 It is not as straightforward as I'd like, see the comments in the code
 for explanation.
>> ...
 +  if (isp->phy_type == ISP_PHY_TYPE_3430) {
 +  struct media_pad *pad;
 +  struct v4l2_subdev *sensor;
 +  const struct isp_ccp2_cfg *buscfg;
 +
 +  pad = media_entity_remote_pad(>pads[CCP2_PAD_SINK]);
 +  sensor = media_entity_to_v4l2_subdev(pad->entity);
 +  /* Struct isp_bus_cfg has union inside */
 +  buscfg = &((struct isp_bus_cfg *)sensor->host_priv)->bus.ccp2;
 +
 +  csiphy_routing_cfg_3430(>isp_csiphy2,
 +  ISP_INTERFACE_CCP2B_PHY1,
 +  enable, !!buscfg->phy_layer,
 +  buscfg->strobe_clk_pol);
>>>
>>> You should do this through omap3isp_csiphy_acquire(), and not call
>>> csiphy_routing_cfg_3430() directly from here.
>>
>> Well, unfortunately omap3isp_csiphy_acquire() does have csi2
>> assumptions hard-coded :-(.
>>
>> This will probably fail.
>>
>>  rval = omap3isp_csi2_reset(phy->csi2);
>>  if (rval < 0)
>>  goto done;
> 
> Could you try to two patches I've applied on the ccp2 branch (I'll remove
> them if there are issues).
> 
> That's compile tested for now only.
> 

I've updated the CCP2 patches here on top of the latest fwnode patches:



No even compile testing this time though. I'm afraid I haven't had the
time to otherwise to work on the CCP2 support, so there are no other
changes besides the rebase.

I intend to send a pull request for the fwnode patches once we have the
next rc1 in media tree so then we can have the patches on plain media
tree master branch.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v5] media: platform: Renesas IMR driver

[Geert Uytterhoeven]

Hi Laurent,

On Tue, May 2, 2017 at 11:17 PM, Laurent Pinchart
 wrote:
> On Wednesday 22 Mar 2017 10:34:16 Geert Uytterhoeven wrote:
>> On Thu, Mar 9, 2017 at 9:08 PM, Sergei Shtylyov wrote:
>> > --- /dev/null
>> > +++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
>> > @@ -0,0 +1,27 @@
>> > +Renesas R-Car Image Renderer (Distortion Correction Engine)
>> > +---
>> > +
>> > +The image renderer, or the distortion correction engine, is a drawing
>> > processor
>> > +with a simple instruction system capable of referencing video capture
>> > data or
>> > +data in an external memory as 2D texture data and performing texture
>> > mapping
>> > +and drawing with respect to any shape that is split into triangular
>> > objects.
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: "renesas,-imr-lx4", "renesas,imr-lx4" as a
>> > fallback for
>> > +  the image renderer light extended 4 (IMR-LX4) found in the R-Car gen3
>> > SoCs,
>> > +  where the examples with  are:
>> > +  - "renesas,r8a7795-imr-lx4" for R-Car H3,
>> > +  - "renesas,r8a7796-imr-lx4" for R-Car M3-W.
>>
>> Laurent: what do you think about the need for SoC-specific compatible
>> values for the various IM* blocks?
>
> There's no documented IP core version register, but when dumping all
> configuration registers on H3 and M3-W I noticed that register 0x002c, not
> documented in the datasheet, reads 0x14060514 on all four IMR instances in H3,
> and 0x20150505 on both instances in M3-W.
>
> This looks like a version register to me. If my assumption is correct, we
> could do without any SoC-specific compatible string.

I read this assumed version registers on all R-Car SoCs, after writing
zero to 0xe6150990 (SMSTPCR8).

IMR-X2 on R-Car H2: 0x12072009
IMR-LSX2 on R-Car H2:   0x12072009
IMR-LSX3 on R-Car V2H:  0x13052617
IMR-LX2 on R-Car M2-W:  0x12072009
IMR-LX2 on R-Car M2-N:  0x12072009
IMR-LX2 on R-Car E2:0x13091909
IMR-LX3 on R-Car V2H:   0x13052617

Note that several IDs are the same, but you know the type from the
compatible value.

It would be good to get confirmation from the hardware team that this is
indeed a version register.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds