[PATCH v1] media: ov5670: Use recommended black level and output bias

2017-08-24 Thread Chiranjeevi Rapolu
Previously, images were relatively darker due to non-optimal
settings for black target level and bias.

Now, use recommended settings for black target level and output bias
as default values. The same default settings apply to all the resolutions.
Given these recommeneded settings do not change dynamically, add these to
existing mode register settings.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/ov5670.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 6f7a1d6..759ca62 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -390,7 +390,10 @@ struct ov5670_mode {
{0x5792, 0x00},
{0x5793, 0x52},
{0x5794, 0xa3},
-   {0x3503, 0x00}
+   {0x3503, 0x00},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const struct ov5670_reg mode_1296x972_regs[] = {
@@ -653,7 +656,10 @@ struct ov5670_mode {
{0x5792, 0x00},
{0x5793, 0x52},
{0x5794, 0xa3},
-   {0x3503, 0x00}
+   {0x3503, 0x00},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const struct ov5670_reg mode_648x486_regs[] = {
@@ -916,7 +922,10 @@ struct ov5670_mode {
{0x5792, 0x00},
{0x5793, 0x52},
{0x5794, 0xa3},
-   {0x3503, 0x00}
+   {0x3503, 0x00},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const struct ov5670_reg mode_2560x1440_regs[] = {
@@ -1178,7 +1187,10 @@ struct ov5670_mode {
{0x5791, 0x06},
{0x5792, 0x00},
{0x5793, 0x52},
-   {0x5794, 0xa3}
+   {0x5794, 0xa3},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const struct ov5670_reg mode_1280x720_regs[] = {
@@ -1441,7 +1453,10 @@ struct ov5670_mode {
{0x5792, 0x00},
{0x5793, 0x52},
{0x5794, 0xa3},
-   {0x3503, 0x00}
+   {0x3503, 0x00},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const struct ov5670_reg mode_640x360_regs[] = {
@@ -1704,7 +1719,10 @@ struct ov5670_mode {
{0x5792, 0x00},
{0x5793, 0x52},
{0x5794, 0xa3},
-   {0x3503, 0x00}
+   {0x3503, 0x00},
+   {0x5045, 0x05},
+   {0x4003, 0x40},
+   {0x5048, 0x40}
 };
 
 static const char * const ov5670_test_pattern_menu[] = {
-- 
1.9.1



cron job: media_tree daily build: ERRORS

2017-08-24 Thread 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:   Fri Aug 25 05:00:17 CEST 2017
media-tree git hash:0779b8855c746c90b85bfe6e16d5dfa2a6a46655
media_build git hash:   4a73db0fa0115ef58537be61da6099c828f57d2b
v4l-utils git hash: 5a6e0c38468c629f3f6f4fb988acebb9e66e2917
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.11.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: WARNINGS
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: WARNINGS
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: WARNINGS
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: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-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: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
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: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


[PATCH] [media] au0828: fix RC_CORE dependency

2017-08-24 Thread Arnd Bergmann
When RC_CORE is a loadable module, and au0828 is built-in including
the RC support, we get a link error:

drivers/media/usb/au0828/au0828-input.o: In function `au0828_get_key_au8522':
au0828-input.c:(.text+0x474): undefined reference to `ir_raw_event_store'
drivers/media/usb/au0828/au0828-input.o: In function `au0828_rc_register':
au0828-input.c:(.text+0x7c8): undefined reference to `rc_allocate_device'
au0828-input.c:(.text+0x8f8): undefined reference to `rc_register_device'

This adds an additional dependency, similar to the one for em28xx,
to ensure the broken configuration is never used.

Fixes: 2fcfd317f66c ("[media] au0828: add support for IR on HVR-950Q")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/usb/au0828/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/usb/au0828/Kconfig b/drivers/media/usb/au0828/Kconfig
index 78b797e0b434..70521e0b4c53 100644
--- a/drivers/media/usb/au0828/Kconfig
+++ b/drivers/media/usb/au0828/Kconfig
@@ -31,6 +31,7 @@ config VIDEO_AU0828_V4L2
 config VIDEO_AU0828_RC
bool "AU0828 Remote Controller support"
depends on RC_CORE
+   depends on !(RC_CORE=m && VIDEO_AU0828=y)
depends on VIDEO_AU0828
---help---
   Enables Remote Controller support on au0828 driver.
-- 
2.9.0



[GIT PULL for 4.14] Still more atomisp cleanups and fixes

2017-08-24 Thread Sakari Ailus
Hi Mauro,

Here are the last atomisp cleanups and fixes for 4.14.

Please pull.


The following changes since commit 0779b8855c746c90b85bfe6e16d5dfa2a6a46655:

  media: ddbridge: fix semicolon.cocci warnings (2017-08-20 10:25:22 -0400)

are available in the git repository at:

  https://linuxtv.org/git/sailus/media_tree.git atomisp

for you to fetch changes up to fdb35c456dd1bcb792b107e1ebc9b46744037281:

  staging: atomisp: fix bounds checking in mt9m114_s_exposure_selection() 
(2017-08-25 00:20:05 +0300)


Arnd Bergmann (1):
  staging: atomisp: imx: remove dead code

Dan Carpenter (1):
  staging: atomisp: fix bounds checking in mt9m114_s_exposure_selection()

Harold Gomez (1):
  staging: media: atomisp: ap1302: Remove FSF postal address

 drivers/staging/media/atomisp/i2c/ap1302.c|  5 
 drivers/staging/media/atomisp/i2c/imx/ad5816g.c   | 11 +---
 drivers/staging/media/atomisp/i2c/imx/drv201.c| 11 +---
 drivers/staging/media/atomisp/i2c/imx/dw9714.c| 14 +-
 drivers/staging/media/atomisp/i2c/imx/dw9718.c|  5 
 drivers/staging/media/atomisp/i2c/imx/dw9719.c| 11 
 drivers/staging/media/atomisp/i2c/imx/imx.c   | 32 ---
 drivers/staging/media/atomisp/i2c/imx/imx.h   | 29 
 drivers/staging/media/atomisp/i2c/mt9m114.c   |  8 +++---
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.c |  2 +-
 drivers/staging/media/atomisp/i2c/ov8858.h|  3 ---
 drivers/staging/media/atomisp/i2c/ov8858_btns.h   |  3 ---
 12 files changed, 8 insertions(+), 126 deletions(-)

-- 
Regards,

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


Re: [PATCH v2 4/4] docs-rst: Allow Sphinx version 1.6

2017-08-24 Thread Jonathan Corbet
On Thu, 24 Aug 2017 13:26:28 -0600
Jonathan Corbet  wrote:

> > -   % To allow adjusting table sizes
> > -   \\usepackage{adjustbox}
> > -
> >   '''
> >  }  
> 
> So this change doesn't quite match the changelog...what's the story there?

Indeed, with that patch applied, I get this:

! LaTeX Error: Environment adjustbox undefined.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H   for immediate help.
 ...  
  
l.4108 \begin{adjustbox}
{width=\columnwidth}

...so methinks something isn't quite right...

jon


Re: [PATCH v2 4/4] docs-rst: Allow Sphinx version 1.6

2017-08-24 Thread Jonathan Corbet
On Wed, 23 Aug 2017 05:56:57 -0300
Mauro Carvalho Chehab  wrote:

> Now that the PDF building issues with Sphinx 1.6 got fixed,
> update the documentation and scripts accordingly.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/conf.py  | 3 ---
>  Documentation/doc-guide/sphinx.rst | 4 +---
>  scripts/sphinx-pre-install | 1 -
>  3 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 8e74d68037a5..0834a9933d69 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -331,9 +331,6 @@ latex_elements = {
>  \\setromanfont{DejaVu Sans}
>  \\setmonofont{DejaVu Sans Mono}
>  
> - % To allow adjusting table sizes
> - \\usepackage{adjustbox}
> -
>   '''
>  }

So this change doesn't quite match the changelog...what's the story there?

Thanks,

jon


[PATCH] [media_build] ddbridge: backport to enable_msi_block, require kernel 3.8

2017-08-24 Thread Daniel Scheller
From: Daniel Scheller 

Backport to pci_enable_msi_block for kernels <3.14 (picked from upstream
dddvb package). Also, ddbridge requires the PCI_DEVICE_SUB macro, which
was added in 3.8.

Signed-off-by: Daniel Scheller 
Tested-by: Jasmin Jessich 
---
This should finally silence all current media_build compile issues related
to the ddbridge pcie driver.

 backports/backports.txt   |  3 +++
 backports/v3.13_ddbridge_pcimsi.patch | 29 +
 v4l/versions.txt  |  2 ++
 3 files changed, 34 insertions(+)
 create mode 100644 backports/v3.13_ddbridge_pcimsi.patch

diff --git a/backports/backports.txt b/backports/backports.txt
index 873b2f5..87b9ee8 100644
--- a/backports/backports.txt
+++ b/backports/backports.txt
@@ -84,6 +84,9 @@ add v3.16_netdev.patch
 add v3.16_wait_on_bit.patch
 add v3.16_void_gpiochip_remove.patch
 
+[3.13.255]
+add v3.13_ddbridge_pcimsi.patch
+
 [3.12.255]
 add v3.12_kfifo_in.patch
 
diff --git a/backports/v3.13_ddbridge_pcimsi.patch 
b/backports/v3.13_ddbridge_pcimsi.patch
new file mode 100644
index 000..80b93a0
--- /dev/null
+++ b/backports/v3.13_ddbridge_pcimsi.patch
@@ -0,0 +1,29 @@
+diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c 
b/drivers/media/pci/ddbridge/ddbridge-main.c
+index 9ab4736..50c3b4f 100644
+--- a/drivers/media/pci/ddbridge/ddbridge-main.c
 b/drivers/media/pci/ddbridge/ddbridge-main.c
+@@ -129,13 +129,18 @@ static void ddb_irq_msi(struct ddb *dev, int nr)
+   int stat;
+ 
+   if (msi && pci_msi_enabled()) {
+-  stat = pci_enable_msi_range(dev->pdev, 1, nr);
+-  if (stat >= 1) {
+-  dev->msi = stat;
+-  dev_info(dev->dev, "using %d MSI interrupt(s)\n",
+-  dev->msi);
+-  } else
++  stat = pci_enable_msi_block(dev->pdev, nr);
++  if (stat == 0) {
++  dev->msi = nr;
++  } else if (stat == 1) {
++  stat = pci_enable_msi(dev->pdev);
++  dev->msi = 1;
++  }
++  if (stat < 0)
+   dev_info(dev->dev, "MSI not available.\n");
++  else
++  dev_info(dev->dev, "using %d MSI interrupts\n",
++   dev->msi);
+   }
+ }
+ #endif
diff --git a/v4l/versions.txt b/v4l/versions.txt
index 5f0b301..abf3b27 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -126,6 +126,8 @@ IR_SPI
 [3.8.0]
 # needs regmap lock/unlock ops
 DVB_TS2020
+# needs the PCI_DEVICE_SUB macro
+DVB_DDBRIDGE
 
 [3.7.0]
 # i2c_add_mux_adapter prototype change
-- 
2.13.0



Re: [PATCH 11/15] remoteproc: make device_type const

2017-08-24 Thread Bjorn Andersson
On Sat 19 Aug 01:22 PDT 2017, Bhumika Goyal wrote:

> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal 

Applied, thanks.

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c 
> b/drivers/remoteproc/remoteproc_core.c
> index 364ef28..48b2c5d 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1360,7 +1360,7 @@ static void rproc_type_release(struct device *dev)
>   kfree(rproc);
>  }
>  
> -static struct device_type rproc_type = {
> +static const struct device_type rproc_type = {
>   .name   = "remoteproc",
>   .release= rproc_type_release,
>  };
> -- 
> 1.9.1
> 


Re: [PATCH 03/15] [media] i2c: make device_type const

2017-08-24 Thread Guennadi Liakhovetski
On Sat, 19 Aug 2017, Bhumika Goyal wrote:

> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal 

Acked-by: Guennadi Liakhovetski 

Thanks
Guennadi

> ---
>  drivers/media/i2c/soc_camera/mt9t031.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/soc_camera/mt9t031.c 
> b/drivers/media/i2c/soc_camera/mt9t031.c
> index 714fb35..4802d30 100644
> --- a/drivers/media/i2c/soc_camera/mt9t031.c
> +++ b/drivers/media/i2c/soc_camera/mt9t031.c
> @@ -592,7 +592,7 @@ static int mt9t031_runtime_resume(struct device *dev)
>   .runtime_resume = mt9t031_runtime_resume,
>  };
>  
> -static struct device_type mt9t031_dev_type = {
> +static const struct device_type mt9t031_dev_type = {
>   .name   = "MT9T031",
>   .pm = &mt9t031_dev_pm_ops,
>  };
> -- 
> 1.9.1
> 


Re: [PATCH 08/15] PCI: make device_type const

2017-08-24 Thread Bjorn Helgaas
On Sat, Aug 19, 2017 at 01:52:19PM +0530, Bhumika Goyal wrote:
> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal 

Applied to pci/misc for v4.14, thanks!

> ---
>  drivers/pci/endpoint/pci-epf-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c 
> b/drivers/pci/endpoint/pci-epf-core.c
> index 6877d6a..9d0de12 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -27,7 +27,7 @@
>  #include 
>  
>  static struct bus_type pci_epf_bus_type;
> -static struct device_type pci_epf_type;
> +static const struct device_type pci_epf_type;
>  
>  /**
>   * pci_epf_linkup() - Notify the function driver that EPC device has
> @@ -275,7 +275,7 @@ static void pci_epf_dev_release(struct device *dev)
>   kfree(epf);
>  }
>  
> -static struct device_type pci_epf_type = {
> +static const struct device_type pci_epf_type = {
>   .release= pci_epf_dev_release,
>  };
>  
> -- 
> 1.9.1
> 


Re: [PATCH 2/4] v4l: async: abort if memory allocation fails when unregistering notifiers

2017-08-24 Thread Sakari Ailus
Hi Niklas,

On Mon, Jul 31, 2017 at 12:31:56AM +0200, Niklas Söderlund wrote:
> Instead of trying to cope with the failed memory allocation and still
> leaving the kernel in a semi-broken state (the subdevices will be
> released but never re-probed) simply abort. The kernel have already
> printed a warning about allocation failure but keep the error printout
> to ease pinpointing the problem if it happens.
> 
> By doing this we can increase the readability of this complex function
> which puts it in a better state to separate the v4l2 housekeeping tasks
> from the re-probing of devices. It also serves to prepare for adding
> subnotifers.

By the time the notifier has been removed from the notifier list, we're the
sole user of the notifier done list. There is thus no need to acquire the
list lock to serialise access to that list.

Is there something that prevents re-probing the devices directly off the
notifier's done list without gathering the device pointers first? As the
notifier has been removed from the notifier list already, there will be no
matches found.

> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 13 ++---
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 0acf288d7227ba97..67852f0f2d3000c9 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -215,6 +215,7 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   if (!dev) {
>   dev_err(notifier->v4l2_dev->dev,
>   "Failed to allocate device cache!\n");
> + return;
>   }
>  
>   mutex_lock(&list_lock);
> @@ -234,23 +235,13 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   /* If we handled USB devices, we'd have to lock the parent too 
> */
>   device_release_driver(d);
>  
> - /*
> -  * Store device at the device cache, in order to call
> -  * put_device() on the final step
> -  */
> - if (dev)
> - dev[i++] = d;
> - else
> - put_device(d);
> + dev[i++] = d;
>   }
>  
>   mutex_unlock(&list_lock);
>  
>   /*
>* Call device_attach() to reprobe devices
> -  *
> -  * NOTE: If dev allocation fails, i is 0, and the whole loop won't be
> -  * executed.
>*/
>   while (i--) {
>   struct device *d = dev[i];
> -- 
> 2.13.3
> 

-- 
Regards,

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


Re: [PATCH 4/4] v4l: async: add comment about re-probing to v4l2_async_notifier_unregister()

2017-08-24 Thread Sakari Ailus
Hi Hans,

On Thu, Aug 24, 2017 at 09:59:41AM +0200, Hans Verkuil wrote:
> On 08/23/17 21:03, Niklas Söderlund wrote:
> > Hi,
> > 
> > On 2017-08-18 15:42:37 +0200, Niklas Söderlund wrote:
> >> Hi Sakari and Laurent,
> >>
> >> Thanks for your feedback.
> >>
> >> On 2017-08-18 14:20:08 +0300, Laurent Pinchart wrote:
> >>> Hello,
> >>>
> >>> On Tuesday 15 Aug 2017 19:09:33 Sakari Ailus wrote:
>  On Mon, Jul 31, 2017 at 12:31:58AM +0200, Niklas Söderlund wrote:
> > The re-probing of subdevices when unregistering a notifier is tricky to
> > understand, and implemented somewhat as a hack. Add a comment trying to
> > explain why the re-probing is needed in the first place and why existing
> > helper functions can't be used in this situation.
> >
> > Signed-off-by: Niklas Söderlund 
> > ---
> >
> >  drivers/media/v4l2-core/v4l2-async.c | 17 +
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c index
> > d91ff0a33fd3eaff..a3c5a1f6d4d2ab03 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -234,6 +234,23 @@ void v4l2_async_notifier_unregister(struct
> > v4l2_async_notifier *notifier)> 
> > mutex_unlock(&list_lock);
> >
> > +   /*
> > +* Try to re-probe the subdevices which where part of the 
> > notifier.
> > +* This is done so subdevices which where part of the notifier 
> > will
> > +* be re-probed to a pristine state and put back on the global
> > +* list of subdevices so they can once more be found and 
> > associated
> > +* with a new notifier.
> 
>  Instead of tweaking the code trying to handle unhandleable error 
>  conditions
>  in notifier unregistration and adding lengthy stories on why this is done
>  the way it is, could we simply get rid of the driver re-probing?
> 
>  I can't see why drivers shouldn't simply cope with the current interfaces
>  without re-probing to which I've never seen any reasoned cause. When a
>  sub-device driver is unbound, simply return the sub-device node to the 
>  list
>  of async sub-devices.
> >>>
> >>> I agree, this is a hack that we should get rid of. Reprobing has been 
> >>> there 
> >>> from the very beginning, it's now 4 years and a half old, let's allow it 
> >>> to 
> >>> retire :-)
> >>
> >> I would also be happy to see this code go away :-)
> >>
> >>>
>  Or can someone come up with a valid reason why the re-probing code should
>  stay? :-)
> >>
> >> Hans kindly dug out the original reason talking about why this code was 
> >> added in the first place at
> >>
> >> http://lkml.iu.edu/hypermail/linux/kernel/1210.2/00713.html
> >>
> >> I would also like record here what Laurent stated about this after 
> >> reading the above on #v4l 
> >>
> >> 13:53  pinchartl : what could happen is this
> >> 13:53  pinchartl : the master could export resources used by the subdev
> >> 13:53  pinchartl : the omap3 isp driver, for instance, is a clock source
> >> 13:54  pinchartl : and the clock can be used by sensors
> >> 13:54  pinchartl : so if you remove the omap3 isp, the clock won't be 
> >>there anymore
> >> 13:54  pinchartl : and that's bad for the subdev

Re-probing never helped anything with omap3isp driver as the clock is
removed *after* unregistering async notifier. This means that the
re-probing sub-device driver will get the same clock which is about to be
removed and continues with that happily, only to find the clock gone in a
brief moment.

This could be fixed in the omap3isp driver but it is telling that _no-one
ever complained_.

> >>
> >>
> >> I don't claim I fully understand all the consequences of removing this 
> >> reprobing now. But maybe it's safer to lave the current behavior in for 
> >> now until the full problem is understood and move forward whit these 
> >> patches since at least they document the behavior and removes another 
> >> funky bit when trying to handle the situation where the memory 
> >> allocation fails? What do you guys think?
> > 
> > Any thoughts about how I can move forward with this? The reason I'm 
> > asking is that this is a dependency for the sub-notifier patches which 
> > in turn is dependency for the R-Car CSI-2 driver :-) If someone wants to 
> > think more about this that is fine I just don't want it to be forgotten.  
> > As I see it these are the options open to me, but as always I'm always 
> > open to other solutions which I'm to narrow minded to see :-)
> > 
> > - If after the latest discussions it feels the safest option is to keep 
> >   the re-probe logic but separating the v4l2 housekeeping from re-probe 
> >   logic move forward with this series as-is.
> 
> I prefer this. We can always remove the reprobe code later once we have
> a bet

Re: [PATCH RFC v2] media: open.rst: document devnode-centric and mc-centric types

2017-08-24 Thread Sakari Ailus
Hi Mauro,

Thanks for the update! A few comments below.

(Cc Hans and Laurent.)

On Thu, Aug 24, 2017 at 09:07:35AM -0300, Mauro Carvalho Chehab wrote:
> From: "mche...@s-opensource.com" 
> 
> When we added support for omap3, back in 2010, we added a new
> type of V4L2 devices that aren't fully controlled via the V4L2
> device node. Yet, we never made it clear, at the V4L2 spec,
> about the differences between both types.
> 
> Let's document them with the current implementation.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Signed-off-by: Mauro Carvalho Chehab 

Pick one. :-)

> ---
>  Documentation/media/uapi/v4l/open.rst | 47 
> +++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst 
> b/Documentation/media/uapi/v4l/open.rst
> index afd116edb40d..cf522d9bb53c 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -6,6 +6,53 @@
>  Opening and Closing Devices
>  ***
>  
> +Types of V4L2 device control
> +
> +
> +V4L2 devices are usually complex: they're implemented via a main driver and
> +often several additional drivers. The main driver always exposes one or
> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`). The other
> +devices are called **V4L2 sub-devices**. They are usually controlled via a
> +serial bus (I2C or SMBus).

Reading this paragraph again, I think this is meant as an introduction to
the reader. In this level of documentation (user space), it's fair to
describe hardware. As the V4L2 sub-device concept in the kernel and the
V4L2 sub-device API which is visible to the user space are not exactly the
same things, I think I'd avoid using V4L2 sub-device concept when referring
to hardware and how the concept is used in the kernel.

How about, to replace the two last sentences:

The other devices are typically I²C or SPI devices. Depending on the main
driver, these devices are controlled either implicitly through the main
driver or explicitly through the **V4L2 sub-device** interface.

(I'm not sure the second sentence is even needed here.)

> +
> +When V4L2 started, there was only one type of device control. The entire
> +device was controlled via the **V4L2 device nodes**. We refer to this
> +kind of control as **V4L2 device-centric** (or, simply, **device-centric**).

I'm still not quite happy with the naming. "Device" is too generic. How
about "V4L2-centric"? SDR, radio and video nodes are part of V4L2, so I
think that should convey the message as well as is factually correct.

> +
> +Since the end of 2010, a new type of V4L2 device control was added in order
> +to support complex devices that are common on embedded systems. Those
> +devices are controlled mainly via the media controller and sub-devices.
> +So, they're called: **media controller centric** (or, simply,
> +"**mc-centric**").

Looks good. I'd use capital "M" in Media controller.

> +
> +On **device-centric** control, the device and their corresponding hardware
> +pipelines are controlled via the **V4L2 device** node. They may optionally
> +expose the hardware pipelines via the
> +:ref:`media controller API `.

s/the hardware pipelines//

It could be that there is a media device, but still no pipeline. Think of
lens and flash devices, for instance.

> +
> +On a **mc-centric**, before using the V4L2 device, it is required to

In English, abbreviations are capitalised, i.e. "MC-centric".

> +set the hardware pipelines via the
> +:ref:`media controller API `. On those devices, the
> +sub-devices' configuration can be controlled via the
> +:ref:`sub-device API `, with creates one device node per sub device.
> +
> +In summary, on **mc-centric** devices:
> +
> +- The **V4L2 device** node is mainly responsible for controlling the
> +  streaming features;
> +- The **media controller device** is responsible to setup the pipelines
> +  and image settings (like size and format);

"image settings (like size and format)" go to the sub-device bullet below.

> +- The **V4L2 sub-devices** are responsible for sub-device
> +  specific settings.
> +
> +.. note::
> +
> +   It is forbidden for **device-centric** devices to expose V4L2
> +   sub-devices via :ref:`sub-device API `, although this
> +   might change in the future.

I believe we agree on the subject matter but we can still argue. :-D

Could you drop ", although this might change in the future" part? We've
established that there is no use case currently and we could well allow
things in the API that were not allowed before without a specific not in
the spec.

> +
> +
> +.. _v4l2_device_naming:
>  
>  Device Naming
>  =

-- 
Kind regards,

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


[GIT PULL for 4.14] LED flash class improvements, AS3645A flash support

2017-08-24 Thread Sakari Ailus
Hi Mauro,

This patchset includes the AS3645A LED driver as well as contents of the
previous pull request here of which the driver depends on:

https://patchwork.linuxtv.org/patch/43363/>

I've agreed with Jacek that the AS3645A patches in the pull request go
through the media tree. The same applies to the greybus light patch from
Rui.

Please pull.


The following changes since commit 0779b8855c746c90b85bfe6e16d5dfa2a6a46655:

  media: ddbridge: fix semicolon.cocci warnings (2017-08-20 10:25:22 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git as3645a-leds

for you to fetch changes up to 281410d1c163b7eaaa0128338e8717b401797555:

  arm: dts: omap3: N9/N950: Add AS3645A camera flash (2017-08-23 11:02:52 +0300)


Rui Miguel Silva (1):
  staging: greybus: light: fix memory leak in v4l2 register

Sakari Ailus (5):
  v4l2-flash-led-class: Create separate sub-devices for indicators
  v4l2-flash-led-class: Document v4l2_flash_init() references
  dt: bindings: Document DT bindings for Analog devices as3645a
  leds: as3645a: Add LED flash class driver
  arm: dts: omap3: N9/N950: Add AS3645A camera flash

 .../devicetree/bindings/leds/ams,as3645a.txt   |  71 ++
 MAINTAINERS|   6 +
 arch/arm/boot/dts/omap3-n950-n9.dtsi   |  14 +
 drivers/leds/Kconfig   |   8 +
 drivers/leds/Makefile  |   1 +
 drivers/leds/leds-aat1290.c|   4 +-
 drivers/leds/leds-as3645a.c| 763 +
 drivers/leds/leds-max77693.c   |   4 +-
 drivers/media/v4l2-core/v4l2-flash-led-class.c | 113 +--
 drivers/staging/greybus/light.c|  42 +-
 include/media/v4l2-flash-led-class.h   |  46 +-
 11 files changed, 990 insertions(+), 82 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
 create mode 100644 drivers/leds/leds-as3645a.c

-- 
Regards,

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


Re: DRM Format Modifiers in v4l2

2017-08-24 Thread Brian Starkey

On Thu, Aug 24, 2017 at 01:37:35PM +0200, Hans Verkuil wrote:

On 08/24/17 13:14, Brian Starkey wrote:

Hi Hans,

On Mon, Aug 21, 2017 at 06:36:29PM +0200, Hans Verkuil wrote:

On 08/21/2017 06:01 PM, Daniel Vetter wrote:

On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey  wrote:

Hi all,

I couldn't find this topic talked about elsewhere, but apologies if
it's a duplicate - I'll be glad to be steered in the direction of a
thread.

We'd like to support DRM format modifiers in v4l2 in order to share
the description of different (mostly proprietary) buffer formats
between e.g. a v4l2 device and a DRM device.

DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and
are a vendor-namespaced 64-bit value used to describe various
vendor-specific buffer layouts. They are combined with a (DRM) FourCC
code to give a complete description of the data contained in a buffer.

The same modifier definition is used in the Khronos EGL extension
EGL_EXT_image_dma_buf_import_modifiers, and is supported in the
Wayland linux-dmabuf protocol.


This buffer information could of course be described in the
vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the
information already defined in drm_fourcc.h. Additionally, there
would be quite a format explosion where a device supports a dozen or
more formats, all of which can use one or more different
layouts/compression schemes.

So, I'm wondering if anyone has views on how/whether this could be
incorporated?

I spoke briefly about this to Laurent at LPC last year, and he
suggested v4l2_control as one approach.

I also wondered if could be added in v4l2_pix_format_mplane - looks
like there's 8 bytes left before it exceeds the 200 bytes, or could go
in the reserved portion of v4l2_plane_pix_format.

Thanks for any thoughts,


One problem is that the modifers sometimes reference the DRM fourcc
codes. v4l has a different (and incompatible set) of fourcc codes,
whereas all the protocols and specs (you can add DRI3.1 for Xorg to
that list btw) use both drm fourcc and drm modifiers.

This might or might not make this proposal unworkable, but it's
something I'd at least review carefully.

Otherwise I think it'd be great if we could have one namespace for all
modifiers, that's pretty much why we have them. Please also note that
for drm_fourcc.h we don't require an in-kernel user for a new modifier
since a bunch of them might need to be allocated just for
userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example
for this would be compressed surfaces with fast-clearing, which is
planned for i915 (but current hw can't scan it out). And we really
want to have one namespace for everything.


Who sets these modifiers? Kernel or userspace? Or can it be set by both?
I assume any userspace code that sets/reads this is code specific for that
hardware?


I think normally the modifier would be set by userspace. However it
might not necessarily be device-specific code. In DRM the intention is
for userspace to query the set of modifiers which are supported, and
then use them without necessarily knowing exactly what they mean
(insofar as that is possible).

e.g. if I have two devices which support MODIFIER_FOO, I could attempt
to share a buffer between them which uses MODIFIER_FOO without
necessarily knowing exactly what it is/does.



I think Laurent's suggestion of using a 64 bit V4L2 control for this makes
the most sense.

Especially if you can assume that whoever sets this knows the hardware.

I think this only makes sense if you pass buffers from one HW device to another.

Because you cannot expect generic video capture code to be able to interpret
all the zillion different combinations of modifiers.


I don't quite follow this last bit. The control could report the set
of supported modifiers.


What I mean was: an application can use the modifier to give buffers from
one device to another without needing to understand it.

But a generic video capture application that processes the video itself
cannot be expected to know about the modifiers. It's a custom HW specific
format that you only use between two HW devices or with software written
for that hardware.



Yes, makes sense.



However, in DRM the API lets you get the supported formats for each
modifier as-well-as the modifier list itself. I'm not sure how exactly
to provide that in a control.


We have support for a 'menu' of 64 bit integers: V4L2_CTRL_TYPE_INTEGER_MENU.
You use VIDIOC_QUERYMENU to enumerate the available modifiers.

So enumerating these modifiers would work out-of-the-box.


Right. So I guess the supported set of formats could be somehow
enumerated in the menu item string. In DRM the pairs are (modifier +
bitmask) where bits represent formats in the supported formats list
(commit db1689aa61bd in drm-next). Printing a hex representation of
the bitmask would be functional but I concede not very pretty.

Cheers,
-Brian



Regards,

Hans


[PATCH RFC v2] media: open.rst: document devnode-centric and mc-centric types

2017-08-24 Thread Mauro Carvalho Chehab
From: "mche...@s-opensource.com" 

When we added support for omap3, back in 2010, we added a new
type of V4L2 devices that aren't fully controlled via the V4L2
device node. Yet, we never made it clear, at the V4L2 spec,
about the differences between both types.

Let's document them with the current implementation.

Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/uapi/v4l/open.rst | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst 
b/Documentation/media/uapi/v4l/open.rst
index afd116edb40d..cf522d9bb53c 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -6,6 +6,53 @@
 Opening and Closing Devices
 ***
 
+Types of V4L2 device control
+
+
+V4L2 devices are usually complex: they're implemented via a main driver and
+often several additional drivers. The main driver always exposes one or
+more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`). The other
+devices are called **V4L2 sub-devices**. They are usually controlled via a
+serial bus (I2C or SMBus).
+
+When V4L2 started, there was only one type of device control. The entire
+device was controlled via the **V4L2 device nodes**. We refer to this
+kind of control as **V4L2 device-centric** (or, simply, **device-centric**).
+
+Since the end of 2010, a new type of V4L2 device control was added in order
+to support complex devices that are common on embedded systems. Those
+devices are controlled mainly via the media controller and sub-devices.
+So, they're called: **media controller centric** (or, simply,
+"**mc-centric**").
+
+On **device-centric** control, the device and their corresponding hardware
+pipelines are controlled via the **V4L2 device** node. They may optionally
+expose the hardware pipelines via the
+:ref:`media controller API `.
+
+On a **mc-centric**, before using the V4L2 device, it is required to
+set the hardware pipelines via the
+:ref:`media controller API `. On those devices, the
+sub-devices' configuration can be controlled via the
+:ref:`sub-device API `, with creates one device node per sub device.
+
+In summary, on **mc-centric** devices:
+
+- The **V4L2 device** node is mainly responsible for controlling the
+  streaming features;
+- The **media controller device** is responsible to setup the pipelines
+  and image settings (like size and format);
+- The **V4L2 sub-devices** are responsible for sub-device
+  specific settings.
+
+.. note::
+
+   It is forbidden for **device-centric** devices to expose V4L2
+   sub-devices via :ref:`sub-device API `, although this
+   might change in the future.
+
+
+.. _v4l2_device_naming:
 
 Device Naming
 =
-- 
2.13.5



Re: DRM Format Modifiers in v4l2

2017-08-24 Thread Hans Verkuil
On 08/24/17 13:14, Brian Starkey wrote:
> Hi Hans,
> 
> On Mon, Aug 21, 2017 at 06:36:29PM +0200, Hans Verkuil wrote:
>> On 08/21/2017 06:01 PM, Daniel Vetter wrote:
>>> On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey  
>>> wrote:
 Hi all,

 I couldn't find this topic talked about elsewhere, but apologies if
 it's a duplicate - I'll be glad to be steered in the direction of a
 thread.

 We'd like to support DRM format modifiers in v4l2 in order to share
 the description of different (mostly proprietary) buffer formats
 between e.g. a v4l2 device and a DRM device.

 DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and
 are a vendor-namespaced 64-bit value used to describe various
 vendor-specific buffer layouts. They are combined with a (DRM) FourCC
 code to give a complete description of the data contained in a buffer.

 The same modifier definition is used in the Khronos EGL extension
 EGL_EXT_image_dma_buf_import_modifiers, and is supported in the
 Wayland linux-dmabuf protocol.


 This buffer information could of course be described in the
 vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the
 information already defined in drm_fourcc.h. Additionally, there
 would be quite a format explosion where a device supports a dozen or
 more formats, all of which can use one or more different
 layouts/compression schemes.

 So, I'm wondering if anyone has views on how/whether this could be
 incorporated?

 I spoke briefly about this to Laurent at LPC last year, and he
 suggested v4l2_control as one approach.

 I also wondered if could be added in v4l2_pix_format_mplane - looks
 like there's 8 bytes left before it exceeds the 200 bytes, or could go
 in the reserved portion of v4l2_plane_pix_format.

 Thanks for any thoughts,
>>>
>>> One problem is that the modifers sometimes reference the DRM fourcc
>>> codes. v4l has a different (and incompatible set) of fourcc codes,
>>> whereas all the protocols and specs (you can add DRI3.1 for Xorg to
>>> that list btw) use both drm fourcc and drm modifiers.
>>>
>>> This might or might not make this proposal unworkable, but it's
>>> something I'd at least review carefully.
>>>
>>> Otherwise I think it'd be great if we could have one namespace for all
>>> modifiers, that's pretty much why we have them. Please also note that
>>> for drm_fourcc.h we don't require an in-kernel user for a new modifier
>>> since a bunch of them might need to be allocated just for
>>> userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example
>>> for this would be compressed surfaces with fast-clearing, which is
>>> planned for i915 (but current hw can't scan it out). And we really
>>> want to have one namespace for everything.
>>
>> Who sets these modifiers? Kernel or userspace? Or can it be set by both?
>> I assume any userspace code that sets/reads this is code specific for that
>> hardware?
> 
> I think normally the modifier would be set by userspace. However it
> might not necessarily be device-specific code. In DRM the intention is
> for userspace to query the set of modifiers which are supported, and
> then use them without necessarily knowing exactly what they mean
> (insofar as that is possible).
> 
> e.g. if I have two devices which support MODIFIER_FOO, I could attempt
> to share a buffer between them which uses MODIFIER_FOO without
> necessarily knowing exactly what it is/does.
> 
>>
>> I think Laurent's suggestion of using a 64 bit V4L2 control for this makes
>> the most sense.
>>
>> Especially if you can assume that whoever sets this knows the hardware.
>>
>> I think this only makes sense if you pass buffers from one HW device to 
>> another.
>>
>> Because you cannot expect generic video capture code to be able to interpret
>> all the zillion different combinations of modifiers.
> 
> I don't quite follow this last bit. The control could report the set
> of supported modifiers.

What I mean was: an application can use the modifier to give buffers from
one device to another without needing to understand it.

But a generic video capture application that processes the video itself
cannot be expected to know about the modifiers. It's a custom HW specific
format that you only use between two HW devices or with software written
for that hardware.

> 
> However, in DRM the API lets you get the supported formats for each
> modifier as-well-as the modifier list itself. I'm not sure how exactly
> to provide that in a control.

We have support for a 'menu' of 64 bit integers: V4L2_CTRL_TYPE_INTEGER_MENU.
You use VIDIOC_QUERYMENU to enumerate the available modifiers.

So enumerating these modifiers would work out-of-the-box.

Regards,

Hans


Re: DRM Format Modifiers in v4l2

2017-08-24 Thread Brian Starkey

Hi Hans,

On Mon, Aug 21, 2017 at 06:36:29PM +0200, Hans Verkuil wrote:

On 08/21/2017 06:01 PM, Daniel Vetter wrote:

On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey  wrote:

Hi all,

I couldn't find this topic talked about elsewhere, but apologies if
it's a duplicate - I'll be glad to be steered in the direction of a
thread.

We'd like to support DRM format modifiers in v4l2 in order to share
the description of different (mostly proprietary) buffer formats
between e.g. a v4l2 device and a DRM device.

DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and
are a vendor-namespaced 64-bit value used to describe various
vendor-specific buffer layouts. They are combined with a (DRM) FourCC
code to give a complete description of the data contained in a buffer.

The same modifier definition is used in the Khronos EGL extension
EGL_EXT_image_dma_buf_import_modifiers, and is supported in the
Wayland linux-dmabuf protocol.


This buffer information could of course be described in the
vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the
information already defined in drm_fourcc.h. Additionally, there
would be quite a format explosion where a device supports a dozen or
more formats, all of which can use one or more different
layouts/compression schemes.

So, I'm wondering if anyone has views on how/whether this could be
incorporated?

I spoke briefly about this to Laurent at LPC last year, and he
suggested v4l2_control as one approach.

I also wondered if could be added in v4l2_pix_format_mplane - looks
like there's 8 bytes left before it exceeds the 200 bytes, or could go
in the reserved portion of v4l2_plane_pix_format.

Thanks for any thoughts,


One problem is that the modifers sometimes reference the DRM fourcc
codes. v4l has a different (and incompatible set) of fourcc codes,
whereas all the protocols and specs (you can add DRI3.1 for Xorg to
that list btw) use both drm fourcc and drm modifiers.

This might or might not make this proposal unworkable, but it's
something I'd at least review carefully.

Otherwise I think it'd be great if we could have one namespace for all
modifiers, that's pretty much why we have them. Please also note that
for drm_fourcc.h we don't require an in-kernel user for a new modifier
since a bunch of them might need to be allocated just for
userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example
for this would be compressed surfaces with fast-clearing, which is
planned for i915 (but current hw can't scan it out). And we really
want to have one namespace for everything.


Who sets these modifiers? Kernel or userspace? Or can it be set by both?
I assume any userspace code that sets/reads this is code specific for that
hardware?


I think normally the modifier would be set by userspace. However it
might not necessarily be device-specific code. In DRM the intention is
for userspace to query the set of modifiers which are supported, and
then use them without necessarily knowing exactly what they mean
(insofar as that is possible).

e.g. if I have two devices which support MODIFIER_FOO, I could attempt
to share a buffer between them which uses MODIFIER_FOO without
necessarily knowing exactly what it is/does.



I think Laurent's suggestion of using a 64 bit V4L2 control for this makes
the most sense.

Especially if you can assume that whoever sets this knows the hardware.

I think this only makes sense if you pass buffers from one HW device to another.

Because you cannot expect generic video capture code to be able to interpret
all the zillion different combinations of modifiers.


I don't quite follow this last bit. The control could report the set
of supported modifiers.

However, in DRM the API lets you get the supported formats for each
modifier as-well-as the modifier list itself. I'm not sure how exactly
to provide that in a control.

Thanks,
-Brian



Regards,

Hans


[PATCH v4] drm/bridge/sii8620: add remote control support

2017-08-24 Thread Maciej Purski
MHL specification defines Remote Control Protocol(RCP) to
send input events between MHL devices.
The driver now recognizes RCP messages and reacts to them
by reporting key events to input subsystem, allowing
a user to control a device using TV remote control.

Signed-off-by: Maciej Purski 
---

Changes in v2:
- use RC subsystem (including CEC keymap)
- RC device initialized in attach drm_bridge callback and
  removed in detach callback. This is necessary, because RC_CORE,
  which is needed during rc_dev init, is loaded after sii8620.
  DRM bridge is binded later which solves the problem.
- add RC_CORE dependency

Changes in v3:
- fix error handling in init_rcp and in attach callback

Changes in v4:
- usage of rc-core API compatible with upcoming changes
- fix error handling in init_rcp
- fix commit message
---
 drivers/gpu/drm/bridge/Kconfig   |  2 +-
 drivers/gpu/drm/bridge/sil-sii8620.c | 96 ++--
 include/drm/bridge/mhl.h |  4 ++
 3 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index adf9ae0..6ef901c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
 
 config DRM_SIL_SII8620
tristate "Silicon Image SII8620 HDMI/MHL bridge"
-   depends on OF
+   depends on OF && RC_CORE
select DRM_KMS_HELPER
help
  Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 2d51a22..ecb26c4 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -28,6 +28,8 @@
 #include 
 #include 
 
+#include 
+
 #include "sil-sii8620.h"
 
 #define SII8620_BURST_BUF_LEN 288
@@ -58,6 +60,7 @@ enum sii8620_mt_state {
 struct sii8620 {
struct drm_bridge bridge;
struct device *dev;
+   struct rc_dev *rc_dev;
struct clk *clk_xtal;
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_int;
@@ -431,6 +434,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 code)
sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
 }
 
+static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)
+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
+}
+
+static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
+}
+
 static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
struct sii8620_mt_msg *msg)
 {
@@ -1753,6 +1766,25 @@ static void sii8620_send_features(struct sii8620 *ctx)
sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
 }
 
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
+{
+   bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
+
+   scancode &= MHL_RCP_KEY_ID_MASK;
+
+   if (!ctx->rc_dev) {
+   dev_dbg(ctx->dev, "RCP input device not initialized\n");
+   return false;
+   }
+
+   if (pressed)
+   rc_keydown(ctx->rc_dev, RC_PROTO_CEC, scancode, 0);
+   else
+   rc_keyup(ctx->rc_dev);
+
+   return true;
+}
+
 static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
 {
u8 ints[MHL_INT_SIZE];
@@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
 
 static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
 {
-   struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
+   struct sii8620_mt_msg *msg;
u8 buf[2];
 
-   if (!msg)
-   return;
-
sii8620_read_buf(ctx, REG_MSC_MR_MSC_MSG_RCVD_1ST_DATA, buf, 2);
 
switch (buf[0]) {
case MHL_MSC_MSG_RAPK:
+   msg = sii8620_msc_msg_first(ctx);
+   if (!msg)
+   return;
msg->ret = buf[1];
ctx->mt_state = MT_STATE_DONE;
break;
+   case MHL_MSC_MSG_RCP:
+   if (!sii8620_rcp_consume(ctx, buf[1]))
+   sii8620_mt_rcpe(ctx,
+   MHL_RCPE_STATUS_INEFFECTIVE_KEY_CODE);
+   sii8620_mt_rcpk(ctx, buf[1]);
+   break;
default:
dev_err(ctx->dev, "%s message type %d,%d not supported",
__func__, buf[0], buf[1]);
@@ -2102,11 +2140,57 @@ static void sii8620_cable_in(struct sii8620 *ctx)
enable_irq(to_i2c_client(ctx->dev)->irq);
 }
 
+static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
+{
+   struct rc_dev *rc_dev;
+   int ret;
+
+   rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
+   if (!rc_dev) {
+   dev_err(ctx->dev, "Failed to allocate RC device\n");
+   ctx->error = -ENOMEM;
+   return;
+   }
+
+   rc_dev->input_phys = "sii8620/input0";
+   rc_dev->input_id.bustype = BUS_VIRTUAL;
+   rc_d

Re: [PATCH] [media_build] rc: Fix ktime erros in rc_ir_raw.c

2017-08-24 Thread Sean Young
On Thu, Aug 24, 2017 at 08:47:54AM +0200, Hans Verkuil wrote:
> On 08/24/2017 02:07 AM, Jasmin J. wrote:
> > Hi!
> > 
> > Just some notes on that patch.
> > 
> > I have *not* tested it due to the lack of an ir remote control. So someone
> > needs to test this on an <= 4.9 Kernel, if the ir core is still working as
> > expected.
> > 
> > Even if I fixed that in media_build, it may be better to apply this code 
> > change
> > in media_tree. This because the involved variables are all of type ktime_t 
> > and
> > there are accessor and converter functions available for that type, which
> > should have been used by the original author of 86fe1ac0d and 48b2de197 in 
> > my
> > opinion.
> 
> Sean,
> 
> I agree with Jasmin here. I noticed the same errors in the daily build and it
> is really caused by not using the correct functions. I just didn't have the
> time to follow up on it.
> 
> Can you take a look at Jasmin's patch and, if OK, make a pull request for
> it?

I hadn't taken media_build into account when I wrote this, so yes I'll
have a look and create a pull request.


Sean


Re: [PATCH v3 2/3] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-08-24 Thread Sakari Ailus
Hi Laurent,

On Wed, Aug 23, 2017 at 03:59:35PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday, 23 August 2017 12:01:24 EEST Sakari Ailus wrote:
> > On Tue, Aug 22, 2017 at 03:52:33PM +0300, Laurent Pinchart wrote:
> > > On Friday, 18 August 2017 14:23:16 EEST Sakari Ailus wrote:
> > >> The current practice is that drivers iterate over their endpoints and
> > >> parse each endpoint separately. This is very similar in a number of
> > >> drivers, implement a generic function for the job. Driver specific
> > >> matters can be taken into account in the driver specific callback.
> > >> 
> > >> Convert the omap3isp as an example.
> > > 
> > > It would be nice to convert at least two drivers to show that the code can
> > > indeed be shared between multiple drivers. Even better, you could convert
> > > all drivers.
> > 
> > That's the intent in the long run. There's still no definite need to do
> > this in a single, big, hot patchset.
> 
> You don't need to convert them all in a single patch, but I'd still prefer 
> converting them all in a single patch series (and I'd split this patch in two 
> to make backporting easier). Otherwise we'll likely end up with multiple 
> competing implementations.

I don't think that presumption is founded. The fact that there is a driver
doing something on its own does not prevent add a helper function to the
framework which could be used to remove driver code. We have, as an
example, the pipeline power management code in v4l2-mc.c. It was introduced
with the omap3isp driver and is now used by a number of drivers.

This is also related preparation for supporting associations that will be
needed for lens and flash devices.

> 
> > >> Signed-off-by: Sakari Ailus 
> > >> ---
> > >> drivers/media/platform/omap3isp/isp.c | 116 ++--
> > >> drivers/media/v4l2-core/v4l2-fwnode.c | 125 +
> > >> include/media/v4l2-async.h|   4 +-
> > >> include/media/v4l2-fwnode.h   |   9 +++
> > >> 4 files changed, 173 insertions(+), 81 deletions(-)
> > > 
> > > [snip]
> > > 
> > >> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > >> b/drivers/media/v4l2-core/v4l2-fwnode.c index 5cd2687310fe..cb0fc4b4e3bf
> > >> 100644
> > >> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > >> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > >> @@ -26,6 +26,7 @@
> > >>  #include 
> > >>  #include 
> > >> 
> > >> +#include 
> > >>  #include 
> > >>  
> > >>  enum v4l2_fwnode_bus_type {
> > >> @@ -383,6 +384,130 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link
> > >> *link) }
> > >> 
> > >>  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
> > >> 
> > >> +static int notifier_realloc(struct device *dev,
> > >> +struct v4l2_async_notifier *notifier,
> > >> +unsigned int max_subdevs)
> > > 
> > > It looks like you interpret the variable as an increment. You shouldn't
> > > call it max_subdevs in that case. I would however keep the name and pass
> > > the total number of subdevs instead of an increment, to mimic the realloc
> > > API.
> > 
> > Works for me.
> > 
> > >> +{
> > >> +struct v4l2_async_subdev **subdevs;
> > >> +unsigned int i;
> > >> +
> > >> +if (max_subdevs + notifier->num_subdevs <= 
> > >> notifier->max_subdevs)
> > >> +return 0;
> > >> +
> > >> +subdevs = devm_kcalloc(
> > >> +dev, max_subdevs + notifier->num_subdevs,
> > >> +sizeof(*notifier->subdevs), GFP_KERNEL);
> > > 
> > > We know that we'll have to move away from devm_* allocation to fix object
> > > lifetime management, so we could as well start now.
> > 
> > The memory is in practice allocated using devm_() interface in existing
> > drivers.
> 
> Yes, and that's bad :-)
> 
> > The fact that it's in a single location makes it much easier getting rid of
> > it.
> 
> Great, so let's get rid of it :-)
> 
> > I'd rather get rid of memory allocation here in the first place, to be
> > replaced by a linked list. But first the user of notifier->subdevs in
> > drivers need to go. The framework interface doesn't need to change as a
> > result.
> 
> How are you going to allocate and free the linked list entries ?

Thinking about it some more --- we can't actually use other objects (not
allocated here) to hold the list struct, so we'll keep the allocation.

I think we'll need v4l2_async_notifier_release() as a result. I'll add that
to v2. This could be used to release resources related to a notifier which
is not yet registered.

> 
> > >> +if (!subdevs)
> > >> +return -ENOMEM;
> > >> +
> > >> +if (notifier->subdevs) {
> > >> +for (i = 0; i < notifier->num_subdevs; i++)
> > >> +subdevs[i] = notifier->subdevs[i];
> > > 
> > > Is there a reason to use a loop here instead of a memcpy() covering the
> > > whole array ?
> > 
> > You could do that, yes, although I'd think this looks nice

Re: [PATCH 4/4] v4l: async: add comment about re-probing to v4l2_async_notifier_unregister()

2017-08-24 Thread Hans Verkuil
On 08/23/17 21:03, Niklas Söderlund wrote:
> Hi,
> 
> On 2017-08-18 15:42:37 +0200, Niklas Söderlund wrote:
>> Hi Sakari and Laurent,
>>
>> Thanks for your feedback.
>>
>> On 2017-08-18 14:20:08 +0300, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> On Tuesday 15 Aug 2017 19:09:33 Sakari Ailus wrote:
 On Mon, Jul 31, 2017 at 12:31:58AM +0200, Niklas Söderlund wrote:
> The re-probing of subdevices when unregistering a notifier is tricky to
> understand, and implemented somewhat as a hack. Add a comment trying to
> explain why the re-probing is needed in the first place and why existing
> helper functions can't be used in this situation.
>
> Signed-off-by: Niklas Söderlund 
> ---
>
>  drivers/media/v4l2-core/v4l2-async.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index
> d91ff0a33fd3eaff..a3c5a1f6d4d2ab03 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -234,6 +234,23 @@ void v4l2_async_notifier_unregister(struct
> v4l2_async_notifier *notifier)> 
>   mutex_unlock(&list_lock);
>
> + /*
> +  * Try to re-probe the subdevices which where part of the notifier.
> +  * This is done so subdevices which where part of the notifier will
> +  * be re-probed to a pristine state and put back on the global
> +  * list of subdevices so they can once more be found and associated
> +  * with a new notifier.

 Instead of tweaking the code trying to handle unhandleable error conditions
 in notifier unregistration and adding lengthy stories on why this is done
 the way it is, could we simply get rid of the driver re-probing?

 I can't see why drivers shouldn't simply cope with the current interfaces
 without re-probing to which I've never seen any reasoned cause. When a
 sub-device driver is unbound, simply return the sub-device node to the list
 of async sub-devices.
>>>
>>> I agree, this is a hack that we should get rid of. Reprobing has been there 
>>> from the very beginning, it's now 4 years and a half old, let's allow it to 
>>> retire :-)
>>
>> I would also be happy to see this code go away :-)
>>
>>>
 Or can someone come up with a valid reason why the re-probing code should
 stay? :-)
>>
>> Hans kindly dug out the original reason talking about why this code was 
>> added in the first place at
>>
>> http://lkml.iu.edu/hypermail/linux/kernel/1210.2/00713.html
>>
>> I would also like record here what Laurent stated about this after 
>> reading the above on #v4l 
>>
>> 13:53  pinchartl : what could happen is this
>> 13:53  pinchartl : the master could export resources used by the subdev
>> 13:53  pinchartl : the omap3 isp driver, for instance, is a clock source
>> 13:54  pinchartl : and the clock can be used by sensors
>> 13:54  pinchartl : so if you remove the omap3 isp, the clock won't be 
>>there anymore
>> 13:54  pinchartl : and that's bad for the subdev
>>
>>
>> I don't claim I fully understand all the consequences of removing this 
>> reprobing now. But maybe it's safer to lave the current behavior in for 
>> now until the full problem is understood and move forward whit these 
>> patches since at least they document the behavior and removes another 
>> funky bit when trying to handle the situation where the memory 
>> allocation fails? What do you guys think?
> 
> Any thoughts about how I can move forward with this? The reason I'm 
> asking is that this is a dependency for the sub-notifier patches which 
> in turn is dependency for the R-Car CSI-2 driver :-) If someone wants to 
> think more about this that is fine I just don't want it to be forgotten.  
> As I see it these are the options open to me, but as always I'm always 
> open to other solutions which I'm to narrow minded to see :-)
> 
> - If after the latest discussions it feels the safest option is to keep 
>   the re-probe logic but separating the v4l2 housekeeping from re-probe 
>   logic move forward with this series as-is.

I prefer this. We can always remove the reprobe code later once we have
a better understanding. I see no downside to this cleanup series and it
doesn't block any future development.

> - Post 1/4 separately and repost patch 2/4 -- 4/4 in a v2 to allow for 
>   more input on what is the right thing to do here.

I'm OK with this as well, we missed the 4.14 merge window anyway.

> - Post 1/4 separately, drop patch 2/4 -- 4/4 and create a new patch 
>   which removes all re-probe related code and post that as a new patch.  
>   I would feel a but uneasy about this without a consensus from all you 
>   guys since I don't understand all the ramifications in doing so.

I'm uneasy about this as well.

> - Post 1/4 separately, drop patch 2/4 -- 4/4 and try to rework the 
>   sub-notifier code to work the intertwined v4l2 and re-pr

Re: [PATCH] [media_build] rc: Fix ktime erros in rc_ir_raw.c

2017-08-24 Thread Jasmin J.
Hello Sean!

> I agree with Jasmin here. I noticed the same errors in the daily build and it
> is really caused by not using the correct functions. I just didn't have the
> time to follow up on it.
I started to fix also gpio-ir-tx.c, but stopped that because it was too late.
So I simply deactivated the driver:
   https://www.mail-archive.com/linux-media@vger.kernel.org/msg117607.html

Maybe you can fix gpio-ir-tx.c also by using the right functions to access
ktime_t, so that this driver would be available for older Kernels, too.
But there was another problem beside the ktime_t accessors, which I didn't
analyze (symbol missing).

BR,
   Jasmin


[GIT PULL for 4.14] omap3isp and smiapp fixes

2017-08-24 Thread Sakari Ailus
Hi Mauro,

Here are two fixes for the smiapp and omap3isp drivers.

Please pull.

The following changes since commit 0779b8855c746c90b85bfe6e16d5dfa2a6a46655:

  media: ddbridge: fix semicolon.cocci warnings (2017-08-20 10:25:22 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git for-4.14-5

for you to fetch changes up to ecee8b797cc74b35587fd2e8ab9e916a85f09e66:

  smiapp: check memory allocation failure (2017-08-24 09:53:46 +0300)


Arnd Bergmann (1):
  media: omap3isp: fix uninitialized variable use

Christophe JAILLET (1):
  smiapp: check memory allocation failure

 drivers/media/i2c/smiapp/smiapp-core.c | 2 ++
 drivers/media/platform/omap3isp/isp.c  | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

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