cron job: media_tree daily build: OK

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Tue Jun 12 05:00:17 CEST 2018
media-tree git hash:f2809d20b9250c675fca8268a0f6274277cca7ff
media_build git hash:   464ef972618cc9f845f07c1a4e8957ce2270cf91
v4l-utils git hash: c3b46c2c53d7d815a53c902cfb2ddd96c3732c5b
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.16.0-1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.101-i686: OK
linux-3.2.101-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.56-i686: OK
linux-3.16.56-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.102-i686: OK
linux-3.18.102-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.51-i686: OK
linux-4.1.51-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.109-i686: OK
linux-4.4.109-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.91-i686: OK
linux-4.9.91-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.42-i686: OK
linux-4.14.42-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
linux-4.16.8-i686: OK
linux-4.16.8-x86_64: OK
linux-4.17-i686: OK
linux-4.17-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH] media: tm6000: add error handling for dvb_register_adapter

[Alexandre-Xavier Labonté-Lamoureux]

"adater" should be "adapter".

Have a nice day,
Alexandre-Xavier

On Mon, Jun 11, 2018 at 12:39 AM, Zhouyang Jia  wrote:
> When dvb_register_adapter fails, the lack of error-handling code may
> cause unexpected results.
>
> This patch adds error-handling code after calling dvb_register_adapter.
>
> Signed-off-by: Zhouyang Jia 
> ---
>  drivers/media/usb/tm6000/tm6000-dvb.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/usb/tm6000/tm6000-dvb.c 
> b/drivers/media/usb/tm6000/tm6000-dvb.c
> index c811fc6..ff35d4b 100644
> --- a/drivers/media/usb/tm6000/tm6000-dvb.c
> +++ b/drivers/media/usb/tm6000/tm6000-dvb.c
> @@ -266,6 +266,11 @@ static int register_dvb(struct tm6000_core *dev)
>
> ret = dvb_register_adapter(>adapter, "Trident TVMaster 6000 
> DVB-T",
> THIS_MODULE, >udev->dev, 
> adapter_nr);
> +   if (ret < 0) {
> +   printk(KERN_ERR "tm6000: couldn't register the adater!\n");
> +   goto err;
> +   }
> +
> dvb->adapter.priv = dev;
>
> if (dvb->frontend) {
> --
> 2.7.4
>

Re: [PATCH] gpu: ipu-v3: Allow negative offsets for interlaced scanning

[Steve Longerbeam]

On 06/11/2018 02:19 AM, Philipp Zabel wrote:

Hi Steve,

On Sun, 2018-06-10 at 17:08 -0700, Steve Longerbeam wrote:

Hi Philipp,

On 06/01/2018 06:13 AM, Philipp Zabel wrote:

The IPU also supports interlaced buffers that start with the bottom field.
To achieve this, the the base address EBA has to be increased by a stride
length and the interlace offset ILO has to be set to the negative stride.

Signed-off-by: Philipp Zabel 
---
   drivers/gpu/ipu-v3/ipu-cpmem.c | 8 +++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index 9f2d9ec42add..c1028f38c553 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -269,8 +269,14 @@ EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
   
   void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)

   {
+   u32 ilo;
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
-   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, stride / 8);
+   if (stride >= 0)
+   ilo = stride / 8;
+   else
+   ilo = 0x10 - (-stride / 8);
+   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, (stride * 2) - 1);

This should be "(-stride * 2) - 1" for SLY when stride is negative.

After fixing that, interweaving seq-bt -> interlaced-bt works fine for
packed pixel formats,

Ouch, thanks.


  but there is still some problem for planar.
I haven't tracked down the issue with planar yet,

Just with the negative stride line?


Correct, planar is broken (bad colors in captured frames) when
negative stride is enabled for interweave. Planar works fine otherwise.


  Only for YUV420 or also for NV12?


I tested YV12 (three planes YUV420). I can't remember if I tested
NV12 (two planes). I'm currently not able to test as I'm away from
the hardware but I will try on Wednesday.


The problem could be that we also have to change UBO/VBO for planar
formats with a chroma stride (SLUV) that is half the luma stride (SLY)
when we move both Y and U/V frame start by a line length.


Right, and I think I am doing that, by setting image.rect.top = 1
before calling ipu_cpmem_set_image(). And when dequeuing a
new v4l2_buffer, I am adding one luma stride to the buffer address
when calling ipu_cpmem_set_buffer() (grep for interweave_offset).




  but the corresponding
changes to imx-media that allow interweaving with line swapping are at

e9dd81da20 ("media: imx: Allow interweave with top/bottom lines swapped")

in branch fix-csi-interlaced.3 in my media-tree fork on github. Please
have a
look and let me know if you see anything obvious.

In commit a19126a80d13 ("gpu: ipu-csi: Swap fields according to
input/output field types"), swap_fields = true is only set for
seq-bt -> seq-tb and seq-tb -> seq-bt. I think it should also be
enabled for alternate -> seq-bt (PAL) and alternate -> seq-tb (NTSC).


It is, see ipu_csi_translate_field() -- it will translate alternate
to seq-bt or seq-tb before determining swap_fields.




I've been made aware [1] that recently V4L2_FIELD_ALTERNATE has been
clarified [2] to specify that v4l2_mbus_fmt.height should contain the
number of lines per field, not per frame:


Yep! That was nagging at me as well. I noticed at least one other
platform (omap3isp) that doubles the source pad frame height
when the sensor reports ALTERNATE field mode, to capture a
whole frame. Makes sense. I think the crop height will need to
be doubled from the sink height in imx-media-csi.c to support
ALTERNATE. That also means sink height can't be used to
guess at input video standard (480 becomes 240 for NTSC).

Steve



[1] https://patchwork.linuxtv.org/patch/50166/
[2] 0018147c964e ("media: v4l: doc: Clarify v4l2_mbus_fmt height
 definition")

regards
Philipp

Re: [Xen-devel] [PATCH v2 5/9] xen/gntdev: Allow mappings for DMA buffers

[Stefano Stabellini]

On Mon, 11 Jun 2018, Oleksandr Andrushchenko wrote:
> On 06/08/2018 10:21 PM, Boris Ostrovsky wrote:
> > On 06/08/2018 01:59 PM, Stefano Stabellini wrote:
> > > > > > > > > >      @@ -325,6 +401,14 @@ static int map_grant_pages(struct
> > > > > > > > > > grant_map
> > > > > > > > > > *map)
> > > > > > > > > >      map->unmap_ops[i].handle =
> > > > > > > > > > map->map_ops[i].handle;
> > > > > > > > > >      if (use_ptemod)
> > > > > > > > > >      map->kunmap_ops[i].handle =
> > > > > > > > > > map->kmap_ops[i].handle;
> > > > > > > > > > +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
> > > > > > > > > > +    else if (map->dma_vaddr) {
> > > > > > > > > > +    unsigned long mfn;
> > > > > > > > > > +
> > > > > > > > > > +    mfn = __pfn_to_mfn(page_to_pfn(map->pages[i]));
> > > > > > > > > Not pfn_to_mfn()?
> > > > > > > > I'd love to, but pfn_to_mfn is only defined for x86, not ARM:
> > > > > > > > [1]
> > > > > > > > and [2]
> > > > > > > > Thus,
> > > > > > > > 
> > > > > > > > drivers/xen/gntdev.c:408:10: error: implicit declaration of
> > > > > > > > function
> > > > > > > > ‘pfn_to_mfn’ [-Werror=implicit-function-declaration]
> > > > > > > >    mfn = pfn_to_mfn(page_to_pfn(map->pages[i]));
> > > > > > > > 
> > > > > > > > So, I'll keep __pfn_to_mfn
> > > > > > > How will this work on non-PV x86?
> > > > > > So, you mean I need:
> > > > > > #ifdef CONFIG_X86
> > > > > > mfn = pfn_to_mfn(page_to_pfn(map->pages[i]));
> > > > > > #else
> > > > > > mfn = __pfn_to_mfn(page_to_pfn(map->pages[i]));
> > > > > > #endif
> > > > > > 
> > > > > I'd rather fix it in ARM code. Stefano, why does ARM uses the
> > > > > underscored version?
> > > > Do you want me to add one more patch for ARM to wrap __pfn_to_mfn
> > > > with static inline for ARM? e.g.
> > > > static inline ...pfn_to_mfn(...)
> > > > {
> > > >      __pfn_to_mfn();
> > > > }
> > > A Xen on ARM guest doesn't actually know the mfns behind its own
> > > pseudo-physical pages. This is why we stopped using pfn_to_mfn and
> > > started using pfn_to_bfn instead, which will generally return "pfn",
> > > unless the page is a foreign grant. See include/xen/arm/page.h.
> > > pfn_to_bfn was also introduced on x86. For example, see the usage of
> > > pfn_to_bfn in drivers/xen/swiotlb-xen.c. Otherwise, if you don't care
> > > about other mapped grants, you can just use pfn_to_gfn, that always
> > > returns pfn.
> > 
> > I think then this code needs to use pfn_to_bfn().
> Ok
> > 
> > 
> > > Also, for your information, we support different page granularities in
> > > Linux as a Xen guest, see the comment at include/xen/arm/page.h:
> > > 
> > >/*
> > > * The pseudo-physical frame (pfn) used in all the helpers is always
> > > based
> > > * on Xen page granularity (i.e 4KB).
> > > *
> > > * A Linux page may be split across multiple non-contiguous Xen page so
> > > we
> > > * have to keep track with frame based on 4KB page granularity.
> > > *
> > > * PV drivers should never make a direct usage of those helpers
> > > (particularly
> > > * pfn_to_gfn and gfn_to_pfn).
> > > */
> > > 
> > > A Linux page could be 64K, but a Xen page is always 4K. A granted page
> > > is also 4K. We have helpers to take into account the offsets to map
> > > multiple Xen grants in a single Linux page, see for example
> > > drivers/xen/grant-table.c:gnttab_foreach_grant. Most PV drivers have
> > > been converted to be able to work with 64K pages correctly, but if I
> > > remember correctly gntdev.c is the only remaining driver that doesn't
> > > support 64K pages yet, so you don't have to deal with it if you don't
> > > want to.
> > 
> > I believe somewhere in this series there is a test for PAGE_SIZE vs.
> > XEN_PAGE_SIZE. Right, Oleksandr?
> Not in gntdev. You might have seen this in xen-drmfront/xen-sndfront,
> but I didn't touch gntdev for that. Do you want me to add yet another patch
> in the series to check for that?

gntdev.c is already not capable of handling PAGE_SIZE != XEN_PAGE_SIZE,
so you are not going to break anything that is not already broken :-) If
your new gntdev.c code relies on PAGE_SIZE == XEN_PAGE_SIZE, it might be
good to add an in-code comment about it, just to make it easier to fix
the whole of gntdev.c in the future.



> > Thanks for the explanation.

Re: [PATCH 2/2] media: i2c: Add driver for Aptina MT9V111

[kbuild test robot]

Hi Jacopo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-mt9v111-sensor-driver/20180611-233038
base:   git://linuxtv.org/media_tree.git master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/media/i2c/mt9v111.c: In function 'mt9v111_set_format':
>> drivers/media/i2c/mt9v111.c:929:15: warning: 'idx' may be used uninitialized 
>> in this function [-Wmaybe-uninitialized]
 unsigned int idx;
  ^~~

vim +/idx +929 drivers/media/i2c/mt9v111.c

   920  
   921  static int mt9v111_set_format(struct v4l2_subdev *subdev,
   922struct v4l2_subdev_pad_config *cfg,
   923struct v4l2_subdev_format *format)
   924  {
   925  struct mt9v111_dev *mt9v111 = sd_to_mt9v111(subdev);
   926  struct v4l2_mbus_framefmt new_fmt;
   927  struct v4l2_mbus_framefmt *__fmt;
   928  unsigned int best_fit = ~0L;
 > 929  unsigned int idx;
   930  unsigned int i;
   931  
   932  mutex_lock(>stream_mutex);
   933  if (mt9v111->streaming) {
   934  mutex_unlock(>stream_mutex);
   935  return -EBUSY;
   936  }
   937  mutex_unlock(>stream_mutex);
   938  
   939  if (format->pad)
   940  return -EINVAL;
   941  
   942  /* Update mbus format code and sizes. */
   943  for (i = 0; i < ARRAY_SIZE(mt9v111_formats); i++) {
   944  if (format->format.code == mt9v111_formats[i].code) {
   945  new_fmt.code = mt9v111_formats[i].code;
   946  break;
   947  }
   948  }
   949  if (i == ARRAY_SIZE(mt9v111_formats))
   950  new_fmt.code = mt9v111_formats[0].code;
   951  
   952  for (i = 0; i < ARRAY_SIZE(mt9v111_frame_sizes); i++) {
   953  unsigned int fit = abs(mt9v111_frame_sizes[i].width -
   954 format->format.width) +
   955 abs(mt9v111_frame_sizes[i].height -
   956 format->format.height);
   957  if (fit < best_fit) {
   958  best_fit = fit;
   959  idx = i;
   960  
   961  if (fit == 0)
   962  break;
   963  }
   964  }
   965  new_fmt.width = mt9v111_frame_sizes[idx].width;
   966  new_fmt.height = mt9v111_frame_sizes[idx].height;
   967  
   968  /* Update the device (or pad) format if it has changed. */
   969  __fmt = __mt9v111_get_pad_format(mt9v111, cfg, format->pad,
   970   format->which);
   971  
   972  /* Format hasn't changed, stop here. */
   973  if (__fmt->code == new_fmt.code &&
   974  __fmt->width == new_fmt.width &&
   975  __fmt->height == new_fmt.height)
   976  goto done;
   977  
   978  /* Update the format and sizes, then  mark changes as pending. 
*/
   979  __fmt->code = new_fmt.code;
   980  __fmt->width = new_fmt.width;
   981  __fmt->height = new_fmt.height;
   982  
   983  if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
   984  mt9v111->pending = true;
   985  
   986  dev_info(mt9v111->dev, "%s: mbus_code: %x - (%ux%u)\n",
   987   __func__, __fmt->code, __fmt->width, __fmt->height);
   988  
   989  done:
   990  format->format = *__fmt;
   991  
   992  return 0;
   993  }
   994  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] media: soc_camera: ov772x: correct setting of banding filter

[jacopo mondi]

Hi Mita-san,

On Mon, Jun 11, 2018 at 12:42:26AM +0900, Akinobu Mita wrote:
> The banding filter ON/OFF is controlled via bit 5 of COM8 register.  It
> is attempted to be enabled in ov772x_set_params() by the following line.
>
>   ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
>
> But this unexpectedly results disabling the banding filter, because the
> mask and set bits are exclusive.
>
> On the other hand, ov772x_s_ctrl() correctly sets the bit by:
>
>   ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
>
> The same fix was already applied to non-soc_camera version of ov772x
> driver in the commit commit a024ee14cd36 ("media: ov772x: correct setting
> of banding filter")

This driver is aimed for removal, but until it's there, as per the
patch you mentioned:
Acked-by: Jacopo Mondi 

Thanks
   j

>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
>  drivers/media/i2c/soc_camera/ov772x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/soc_camera/ov772x.c 
> b/drivers/media/i2c/soc_camera/ov772x.c
> index 8063835..14377af 100644
> --- a/drivers/media/i2c/soc_camera/ov772x.c
> +++ b/drivers/media/i2c/soc_camera/ov772x.c
> @@ -834,7 +834,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>* set COM8
>*/
>   if (priv->band_filter) {
> - ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
> + ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
>   if (!ret)
>   ret = ov772x_mask_set(client, BDBASE,
> 0xff, 256 - priv->band_filter);
> --
> 2.7.4
>


signature.asc
Description: PGP signature

[PATCH 0/2] media: i2c: mt9v111 sensor driver

[Jacopo Mondi]

Hello,
   this is a sensor level driver for Aptina MT9V111.

The driver supports YUYV_2X8 output formats and VGA,QVGA,QQVGA,CIF and QCIF
resolution.

The driver allows control of frame rate through s_frame_interval or
V4L2_CID_H/VBLANK control. In order to allow manual frame control, the chip
is initialized with auto-exposure control, auto white balancing and flickering
control disabled.

Tested VGA, QVGA QQVGA resolutions at 5, 10, 15 and 30 frame per second.

Thanks
   j

Jacopo Mondi (2):
  dt-bindings: media: i2c: Document MT9V111 bindings
  media: i2c: Add driver for Aptina MT9V111

 .../bindings/media/i2c/aptina,mt9v111.txt  |   46 +
 MAINTAINERS|8 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/mt9v111.c| 1297 
 5 files changed, 1364 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt
 create mode 100644 drivers/media/i2c/mt9v111.c

--
2.7.4

[PATCH 1/2] dt-bindings: media: i2c: Document MT9V111 bindings

[Jacopo Mondi]

Add documentation for Aptina MT9V111 image sensor.

Signed-off-by: Jacopo Mondi 
---
 .../bindings/media/i2c/aptina,mt9v111.txt  | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt 
b/Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt
new file mode 100644
index 000..bac4bf0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt
@@ -0,0 +1,46 @@
+* Aptina MT9V111 CMOS sensor
+
+
+The Aptina MT9V111 is a 1/4-Inch VGA-format digital image sensor with a core
+based on Aptina MT9V011 sensor and an integrated Image Flow Processor (IFP).
+
+The sensor has an active pixel array of 649x489 pixels and can output a number
+of image resolution and formats controllable through a simple two-wires
+interface.
+
+Required properties:
+
+
+- compatible: shall be "aptina,mt9v111".
+- clocks: reference to the system clock input provider.
+
+Optional properties:
+
+
+- enable-gpios: output enable signal, pin name "OE#". Active low.
+- standby-gpios: low power state control signal, pin name "STANDBY".
+  Active high.
+- reset-gpios: chip reset signal, pin name "RESET#". Active low.
+
+The device node must contain one 'port' child node with one 'endpoint' child
+sub-node for its digital output video port, in accordance with the video
+interface bindings defined in:
+Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+
+ {
+camera@48 {
+compatible = "aptina,mt9v111";
+reg = <0x48>;
+
+clocks = <_clk>;
+
+port {
+mt9v111_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+};
+};
--
2.7.4

[PATCH 2/2] media: i2c: Add driver for Aptina MT9V111

[Jacopo Mondi]

Add V4L2 sensor driver for Aptina MT9V111 CMOS image sensor.

The MT9V111 is a 1/4-Inch CMOS image sensor based on MT9V011 with an
integrated Image Flow Processor.

Signed-off-by: Jacopo Mondi 
---
 MAINTAINERS |8 +
 drivers/media/i2c/Kconfig   |   12 +
 drivers/media/i2c/Makefile  |1 +
 drivers/media/i2c/mt9v111.c | 1297 +++
 4 files changed, 1318 insertions(+)
 create mode 100644 drivers/media/i2c/mt9v111.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a38e24a..2c2fe60 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9523,6 +9523,14 @@ F:   
Documentation/devicetree/bindings/media/i2c/mt9v032.txt
 F: drivers/media/i2c/mt9v032.c
 F: include/media/i2c/mt9v032.h
 
+MT9V111 APTINA CAMERA SENSOR
+M: Jacopo Mondi 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt
+F: drivers/media/i2c/mt9v111.c
+
 MULTIFUNCTION DEVICES (MFD)
 M: Lee Jones 
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452f..0bd867d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -841,6 +841,18 @@ config VIDEO_MT9V032
  This is a Video4Linux2 sensor-level driver for the Micron
  MT9V032 752x480 CMOS sensor.
 
+config VIDEO_MT9V111
+   tristate "Aptina MT9V111 sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   depends on OF
+   help
+ This is a Video4Linux2 sensor-level driver for the Aptina/Micron
+ MT9V111 sensor.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mt9v111.
+
 config VIDEO_SR030PC30
tristate "Siliconfile SR030PC30 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57..f0e8618 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o
 obj-$(CONFIG_VIDEO_MT9T112) += mt9t112.o
 obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
 obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
+obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
 obj-$(CONFIG_VIDEO_SR030PC30)  += sr030pc30.o
 obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
 obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
new file mode 100644
index 000..36e7424
--- /dev/null
+++ b/drivers/media/i2c/mt9v111.c
@@ -0,0 +1,1297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 sensor driver for Aptina MT9V111 image sensor
+ * Copyright (C) 2018 Jacopo Mondi 
+ *
+ * Based on mt9v032 driver
+ * Copyright (C) 2010, Laurent Pinchart 
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * Base on mt9v011 driver
+ * Copyright (c) 2009 Mauro Carvalho Chehab 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * MT9V111 is a 1/4-Inch CMOS digital image sensor with an integrated
+ * Image Flow Processing (IFP) engine and a sensor core loosely based on
+ * MT9V011.
+ *
+ * The IFP can produce several output image formats from the sensor core
+ * output, this driver currently supports only YUYV format permutations.
+ *
+ * As the auto exposure algorithms controls exposure time and modifies the
+ * frame rate, this driver disables auto exposure control, auto white balancing
+ * and auto flickering avoidance to allow manual frame rate control through
+ * s_frame_interval subdev operation or V4L2_CID_V/HBLANK controls.
+ *
+ * While it seems possible to instruct the auto-exposure control algorithm to
+ * respect a programmed frame rate when adjusting the pixel integration time,
+ * registers controlling this feature are not documented in the public
+ * available sensor manual used to develop this driver (09005aef80e90084,
+ * MT9V111_1.fm - Rev. G 1/05 EN).
+ */
+
+#define MT9V111_CHIP_ID_HIGH   0x82
+#define MT9V111_CHIP_ID_LOW0x3a
+
+#define MT9V111_R01_ADDR_SPACE 0x01
+#define MT9V111_R01_IFP0x01
+#define MT9V111_R01_CORE   0x04
+
+#define MT9V111_IFP_R06_OPMODE_CTRL0x06
+#defineMT9V111_IFP_R06_OPMODE_CTRL_AWB_EN  BIT(1)
+#defineMT9V111_IFP_R06_OPMODE_CTRL_AE_EN   BIT(14)
+#define MT9V111_IFP_R07_IFP_RESET  0x07
+#defineMT9V111_IFP_R07_IFP_RESET_MASK  BIT(0)
+#define MT9V111_IFP_R08_OUTFMT_CTRL0x08
+#defineMT9V111_IFP_R08_OUTFMT_CTRL_FLICKER BIT(11)
+#defineMT9V111_IFP_R08_OUTFMT_CTRL_PCLKBIT(5)

[PATCH] [media] cx18: remove redundant zero check on retval

[Colin King]

From: Colin Ian King 

The check for a zero retval is redundant as all paths that lead to
this point have set retval to an error return value that is non-zero.
Remove the redundant check.

Detected by CoverityScan, CID#102589 ("Logically dead code")

Signed-off-by: Colin Ian King 
---
 drivers/media/firewire/firedtv-fw.c  | 3 +++
 drivers/media/pci/cx18/cx18-driver.c | 2 --
 drivers/regulator/vctrl-regulator.c  | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/firewire/firedtv-fw.c 
b/drivers/media/firewire/firedtv-fw.c
index 92f4112d2e37..eed55be21836 100644
--- a/drivers/media/firewire/firedtv-fw.c
+++ b/drivers/media/firewire/firedtv-fw.c
@@ -271,6 +271,9 @@ static int node_probe(struct fw_unit *unit, const struct 
ieee1394_device_id *id)
 
name_len = fw_csr_string(unit->directory, CSR_MODEL,
 name, sizeof(name));
+   if (name_len < 0)
+   return name_len;
+
for (i = ARRAY_SIZE(model_names); --i; )
if (strlen(model_names[i]) <= name_len &&
strncmp(name, model_names[i], name_len) == 0)
diff --git a/drivers/media/pci/cx18/cx18-driver.c 
b/drivers/media/pci/cx18/cx18-driver.c
index 8f314ca320c7..0c389a3fb4e5 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -1134,8 +1134,6 @@ static int cx18_probe(struct pci_dev *pci_dev,
 free_workqueues:
destroy_workqueue(cx->in_work_queue);
 err:
-   if (retval == 0)
-   retval = -ENODEV;
CX18_ERR("Error %d on initialization\n", retval);
 
v4l2_device_unregister(>v4l2_dev);
diff --git a/drivers/regulator/vctrl-regulator.c 
b/drivers/regulator/vctrl-regulator.c
index 78de002037c7..044e5a5ca163 100644
--- a/drivers/regulator/vctrl-regulator.c
+++ b/drivers/regulator/vctrl-regulator.c
@@ -340,7 +340,7 @@ static int vctrl_init_vtable(struct platform_device *pdev)
}
}
 
-   if (rdesc->n_voltages == 0) {
+   if (rdesc->n_voltages <= 0) {
dev_err(>dev, "invalid configuration\n");
return -EINVAL;
}
-- 
2.17.0

Re: [PATCH] media: stm32-dcmi: add power saving support

[kbuild test robot]

Hi Hugues,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/media-stm32-dcmi-add-power-saving-support/20180611-174016
base:   git://linuxtv.org/media_tree.git master
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_suspend':
   drivers/media//platform/stm32/stm32-dcmi.c:1886:2: error: implicit 
declaration of function 'pinctrl_pm_select_sleep_state' 
[-Werror=implicit-function-declaration]
 pinctrl_pm_select_sleep_state(dev);
 ^
   drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_resume':
>> drivers/media//platform/stm32/stm32-dcmi.c:1894:2: error: implicit 
>> declaration of function 'pinctrl_pm_select_default_state'; did you mean 
>> 'irq_set_default_host'? [-Werror=implicit-function-declaration]
 pinctrl_pm_select_default_state(dev);
 ^~~
 irq_set_default_host
   cc1: some warnings being treated as errors

vim +1894 drivers/media//platform/stm32/stm32-dcmi.c

  1879  
  1880  static __maybe_unused int dcmi_suspend(struct device *dev)
  1881  {
  1882  /* disable clock */
  1883  pm_runtime_force_suspend(dev);
  1884  
  1885  /* change pinctrl state */
> 1886  pinctrl_pm_select_sleep_state(dev);
  1887  
  1888  return 0;
  1889  }
  1890  
  1891  static __maybe_unused int dcmi_resume(struct device *dev)
  1892  {
  1893  /* restore pinctl default state */
> 1894  pinctrl_pm_select_default_state(dev);
  1895  
  1896  /* clock enable */
  1897  pm_runtime_force_resume(dev);
  1898  
  1899  return 0;
  1900  }
  1901  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [Xen-devel] [PATCH v2 5/9] xen/gntdev: Allow mappings for DMA buffers

[Oleksandr Andrushchenko]

On 06/08/2018 10:21 PM, Boris Ostrovsky wrote:

On 06/08/2018 01:59 PM, Stefano Stabellini wrote:

     @@ -325,6 +401,14 @@ static int map_grant_pages(struct
grant_map
*map)
     map->unmap_ops[i].handle = map->map_ops[i].handle;
     if (use_ptemod)
     map->kunmap_ops[i].handle =
map->kmap_ops[i].handle;
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+    else if (map->dma_vaddr) {
+    unsigned long mfn;
+
+    mfn = __pfn_to_mfn(page_to_pfn(map->pages[i]));

Not pfn_to_mfn()?

I'd love to, but pfn_to_mfn is only defined for x86, not ARM: [1]
and [2]
Thus,

drivers/xen/gntdev.c:408:10: error: implicit declaration of function
‘pfn_to_mfn’ [-Werror=implicit-function-declaration]
   mfn = pfn_to_mfn(page_to_pfn(map->pages[i]));

So, I'll keep __pfn_to_mfn

How will this work on non-PV x86?

So, you mean I need:
#ifdef CONFIG_X86
mfn = pfn_to_mfn(page_to_pfn(map->pages[i]));
#else
mfn = __pfn_to_mfn(page_to_pfn(map->pages[i]));
#endif


I'd rather fix it in ARM code. Stefano, why does ARM uses the
underscored version?

Do you want me to add one more patch for ARM to wrap __pfn_to_mfn
with static inline for ARM? e.g.
static inline ...pfn_to_mfn(...)
{
     __pfn_to_mfn();
}

A Xen on ARM guest doesn't actually know the mfns behind its own
pseudo-physical pages. This is why we stopped using pfn_to_mfn and
started using pfn_to_bfn instead, which will generally return "pfn",
unless the page is a foreign grant. See include/xen/arm/page.h.
pfn_to_bfn was also introduced on x86. For example, see the usage of
pfn_to_bfn in drivers/xen/swiotlb-xen.c. Otherwise, if you don't care
about other mapped grants, you can just use pfn_to_gfn, that always
returns pfn.


I think then this code needs to use pfn_to_bfn().

Ok




Also, for your information, we support different page granularities in
Linux as a Xen guest, see the comment at include/xen/arm/page.h:

   /*
* The pseudo-physical frame (pfn) used in all the helpers is always based
* on Xen page granularity (i.e 4KB).
*
* A Linux page may be split across multiple non-contiguous Xen page so we
* have to keep track with frame based on 4KB page granularity.
*
* PV drivers should never make a direct usage of those helpers (particularly
* pfn_to_gfn and gfn_to_pfn).
*/

A Linux page could be 64K, but a Xen page is always 4K. A granted page
is also 4K. We have helpers to take into account the offsets to map
multiple Xen grants in a single Linux page, see for example
drivers/xen/grant-table.c:gnttab_foreach_grant. Most PV drivers have
been converted to be able to work with 64K pages correctly, but if I
remember correctly gntdev.c is the only remaining driver that doesn't
support 64K pages yet, so you don't have to deal with it if you don't
want to.


I believe somewhere in this series there is a test for PAGE_SIZE vs.
XEN_PAGE_SIZE. Right, Oleksandr?

Not in gntdev. You might have seen this in xen-drmfront/xen-sndfront,
but I didn't touch gntdev for that. Do you want me to add yet another patch
in the series to check for that?

Thanks for the explanation.

-boris

Thank you,
Oleksandr

[GIT PULL for 4.19] Sensor, lens and ISP driver patches

[Sakari Ailus]

Hi Mauro,

Here's the usual bunch of sensor, lens and ISP driver patches. In
particular, there are new drivers for ov2680 sensor and dw9807 VCM lens
controller part.

Please pull.


The following changes since commit f2809d20b9250c675fca8268a0f6274277cca7ff:

  media: omap2: fix compile-testing with FB_OMAP2=m (2018-06-05 09:56:56 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git for-4.19-1

for you to fetch changes up to 4fdefee7567a2e1f6ae09ca3e184851062bc3fa5:

  media: soc_camera: ov772x: correct setting of banding filter (2018-06-11 
15:45:57 +0300)


Akinobu Mita (14):
  media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
  media: ov772x: add checks for register read errors
  media: ov772x: add media controller support
  media: ov772x: use generic names for reset and powerdown gpios
  media: ov772x: omit consumer ID when getting clock reference
  media: ov772x: support device tree probing
  media: ov772x: handle nested s_power() calls
  media: ov772x: reconstruct s_frame_interval()
  media: ov772x: use v4l2_ctrl to get current control value
  media: ov772x: avoid accessing registers under power saving mode
  media: ov772x: make set_fmt() and s_frame_interval() return -EBUSY while 
streaming
  media: ov772x: create subdevice device node
  media: s3c-camif: ignore -ENOIOCTLCMD from v4l2_subdev_call for s_power
  media: soc_camera: ov772x: correct setting of banding filter

Alan Chiang (2):
  media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil
  media: dw9807: Add dw9807 vcm driver

Arnd Bergmann (3):
  media: omap3isp: fix warning for !CONFIG_PM
  media: v4l: cadence: include linux/slab.h
  media: v4l: cadence: add VIDEO_V4L2 dependency

Javier Martinez Canillas (1):
  media: omap3isp: zero-initialize the isp cam_xclk{a,b} initial data

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

Yong Zhi (1):
  MAINTAINERS: Update entry for Intel IPU3 cio2 driver

 .../bindings/media/i2c/dongwoon,dw9807.txt |9 +
 .../devicetree/bindings/media/i2c/ov2680.txt   |   46 +
 MAINTAINERS|   10 +
 arch/sh/boards/mach-migor/setup.c  |7 +-
 drivers/media/i2c/Kconfig  |   22 +
 drivers/media/i2c/Makefile |2 +
 drivers/media/i2c/dw9807.c |  329 ++
 drivers/media/i2c/ov2680.c | 1169 
 drivers/media/i2c/ov772x.c |  353 --
 drivers/media/i2c/soc_camera/ov772x.c  |2 +-
 drivers/media/platform/cadence/Kconfig |2 +
 drivers/media/platform/cadence/cdns-csi2rx.c   |1 +
 drivers/media/platform/cadence/cdns-csi2tx.c   |1 +
 drivers/media/platform/omap3isp/isp.c  |6 +-
 drivers/media/platform/s3c-camif/camif-capture.c   |2 +
 15 files changed, 1861 insertions(+), 100 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/dw9807.c
 create mode 100644 drivers/media/i2c/ov2680.c

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

[GIT PULL for 4.19] Add "rotation" property for sensors, use it

[Sakari Ailus]

Hi Mauro,

This pull request adds the "rotation" property already used for display
panels for sensors. Support for the property is added to the smiapp and
ov5640 drivers.

Please pull.


The following changes since commit f2809d20b9250c675fca8268a0f6274277cca7ff:

  media: omap2: fix compile-testing with FB_OMAP2=m (2018-06-05 09:56:56 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git v4l2-rotation

for you to fetch changes up to 8cb64987a0db499b61de3c76732ff34823fb5dc7:

  media: ov5640: add support of module orientation (2018-06-11 15:45:23 +0300)


Hugues Fruchet (2):
  media: ov5640: add HFLIP/VFLIP controls support
  media: ov5640: add support of module orientation

Sakari Ailus (2):
  dt-bindings: media: Define "rotation" property for sensors
  smiapp: Support the "rotation" property

 .../devicetree/bindings/media/i2c/nokia,smia.txt   |   2 +
 .../devicetree/bindings/media/i2c/ov5640.txt   |   3 +
 .../devicetree/bindings/media/video-interfaces.txt |   4 +
 drivers/media/i2c/ov5640.c | 127 ++---
 drivers/media/i2c/smiapp/smiapp-core.c |  16 +++
 5 files changed, 134 insertions(+), 18 deletions(-)

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

Re: [PATCH v4 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[Sakari Ailus]

On Thu, Jun 07, 2018 at 11:50:32AM +0800, bingbu@intel.com wrote:
> From: Bingbu Cao 
> 
> Add device tree bindings for AKM ak7375 voice coil lens
> driver. This chip is used to drive a lens in a camera module.
> 
> Signed-off-by: Tianshu Qiu 
> Signed-off-by: Bingbu Cao 

Please remember to add acks you've received in the future.

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

Re: [PATCH] media: stm32-dcmi: add power saving support

[kbuild test robot]

Hi Hugues,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/media-stm32-dcmi-add-power-saving-support/20180611-174016
base:   git://linuxtv.org/media_tree.git master
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/media/platform/stm32/stm32-dcmi.c: In function 'dcmi_suspend':
>> drivers/media/platform/stm32/stm32-dcmi.c:1886:2: error: implicit 
>> declaration of function 'pinctrl_pm_select_sleep_state' 
>> [-Werror=implicit-function-declaration]
 pinctrl_pm_select_sleep_state(dev);
 ^
   drivers/media/platform/stm32/stm32-dcmi.c: In function 'dcmi_resume':
>> drivers/media/platform/stm32/stm32-dcmi.c:1894:2: error: implicit 
>> declaration of function 'pinctrl_pm_select_default_state' 
>> [-Werror=implicit-function-declaration]
 pinctrl_pm_select_default_state(dev);
 ^~~
   cc1: some warnings being treated as errors

vim +/pinctrl_pm_select_sleep_state +1886 
drivers/media/platform/stm32/stm32-dcmi.c

  1879  
  1880  static __maybe_unused int dcmi_suspend(struct device *dev)
  1881  {
  1882  /* disable clock */
  1883  pm_runtime_force_suspend(dev);
  1884  
  1885  /* change pinctrl state */
> 1886  pinctrl_pm_select_sleep_state(dev);
  1887  
  1888  return 0;
  1889  }
  1890  
  1891  static __maybe_unused int dcmi_resume(struct device *dev)
  1892  {
  1893  /* restore pinctl default state */
> 1894  pinctrl_pm_select_default_state(dev);
  1895  
  1896  /* clock enable */
  1897  pm_runtime_force_resume(dev);
  1898  
  1899  return 0;
  1900  }
  1901  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Hi Ville,

I finally found some time to dig deeper into when a CEC device should be 
registered.

Since it's been a long while since we discussed this let me just recap the 
situation
and then propose three solutions:
CEC is implemented for DP-to-HDMI branch devices through DPCD CEC registers. 
When
HPD is high we can read these registers and check if CEC is supported or not.

If an external USB-C/DP/mini-DP to HDMI adapter is used, then when the HPD goes 
low
you lose access to the DPCD registers since that is (I think) powered by the 
HPD.

If an integrated branch device is used (such as for the HDMI connector on the 
NUC)
the DPCD registers will remain available even if the display is disconnected.

The problem is with external adapters since if the HPD goes low you do not know
if the adapter has been disconnected, or if the display just pulled the HPD low 
for a
short time (updating the EDID, HDCP changes). In the latter case you do not 
want to
unregister and reregister the cec device.

I see three options:

1) register a cec device when a connector is registered and keep it for the 
life time
of the connector. If HPD goes low, or the branch device doesn't support CEC, 
then just
set the physical address of the CEC adapter to f.f.f.f.

This is simple, but the disadvantage is that there is a CEC device around, even 
if the
branch device doesn't support CEC.

2) register a cec device when HPD goes high and if a branch device that 
supports CEC is
detected. Unregister the cec device when the HPD goes low and stays low for 
more than
a second. This prevents a cec device from disappearing whenever the display 
toggles
the HPD. It does require a delayed workqueue to handle this delay.

It would be nice if there is a way to avoid a delayed workqueue, but I didn't 
see
anything in the i915 code.

3) A hybrid option where the cec device is only registered when a CEC capable 
branch
device is detected, but then we keep it for the remaining lifetime of the 
connector.
This avoids the delayed workqueue, and avoids creating cec devices if the branch
device doesn't support CEC. But once it is created it won't be removed until the
connector is also unregistered.

I'm leaning towards the second or third option.

Opinions? Other ideas?

Regards,

Hans

[PATCH v4 7/8] media: imx274: fix typo

[Luca Ceresoli]

pd -> pad

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing
---
 drivers/media/i2c/imx274.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index e5ba19b97083..2c13961e9764 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -544,7 +544,7 @@ struct imx274_ctrls {
 /*
  * struct stim274 - imx274 device structure
  * @sd: V4L2 subdevice structure
- * @pd: Media pad structure
+ * @pad: Media pad structure
  * @client: Pointer to I2C client
  * @ctrls: imx274 control structure
  * @format: V4L2 media bus frame format structure
-- 
2.7.4

[PATCH v4 0/8] media: imx274: cleanups, improvements and SELECTION API support

[Luca Ceresoli]

Hi,

this patchset introduces cropping support for the Sony IMX274 sensor
using the SELECTION API.

With respect to v3, this version uses the SELECTION API with taget
V4L2_SEL_TGT_COMPOSE to change the output resolution. This is the
recommended API for cropping + downscaling. However for backward
compatibility the set_format callback is still supported and is
equivalent to setting the compose rect as far as resolutions are
concerned.

Patches 1-5 are overall improvements and restructuring, mostly useful
to implement the SELECTION API in a clean way.

Patch 6 introduces a helper to allow setting many registers computed
at runtime in a straightforward way. This would not have been very
useful before because all long register write sequences came from
const tables, but it's definitely a must for the cropping code where
several register values are computed at runtime.

Patch 7 is new in this series, it's a trivial typo fix that can be
applied independently.

Patch 8 implements the set_selection pad operation for cropping
(V4L2_SEL_TGT_CROP) and binning (V4L2_SEL_TGT_COMPOSE). The most
tricky part was respecting all the device constraints on the
horizontal crop.

Usage examples:

 * Capture the entire 4K area, downscaled to 1080p with 2:1 binning:
   media-ctl -V '"IMX274":0[crop:(0,0)/3840x2160]'
   media-ctl -V '"IMX274":0[compose:(0,0)/1920x1080]'

 * Capture the central 1080p area (no binning):
   media-ctl -V '"IMX274":0[crop:(960,540)/1920x1080]'
   (this also sets the compose and fmt rects to 1920x1080)

Regards,
Luca


Luca Ceresoli (8):
  media: imx274: initialize format before v4l2 controls
  media: imx274: consolidate per-mode data in imx274_frmfmt
  media: imx274: get rid of mode_index
  media: imx274: actually use IMX274_DEFAULT_MODE
  media: imx274: simplify imx274_write_table()
  media: imx274: add helper function to fill a reg_8 table chunk
  media: imx274: fix typo
  media: imx274: add SELECTION support for cropping

 drivers/media/i2c/imx274.c | 686 ++---
 1 file changed, 461 insertions(+), 225 deletions(-)

-- 
2.7.4

[PATCH v4 3/8] media: imx274: get rid of mode_index

[Luca Ceresoli]

After restructuring struct imx274_frmfmt, the mode_index field is
still in use only for two dev_dbg() calls in imx274_s_stream(). Let's
remove it and avoid duplicated information.

Replacing the first usage requires some rather annoying but trivial
pointer math. The other one can be removed entirely since it would
print the same value anyway.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3:
 - really fix dev_dbg() format mismatch warning for both 32 and 64 bit

Changed v1 -> v2:
 - add "media: " prefix to commit message
 - fix dev_dbg() format mismatch warning
   ("warning: format ‘%ld’ expects argument of type ‘long int’, but argument 6 
has type ‘int’")
 - slightly improve commit message
---
 drivers/media/i2c/imx274.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 2ec31ae4e60d..f075715ffced 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -553,8 +553,6 @@ struct imx274_ctrls {
  * @reset_gpio: Pointer to reset gpio
  * @lock: Mutex structure
  * @mode: Parameters for the selected readout mode
- *(points to imx274_formats[mode_index])
- * @mode_index: Resolution mode index
  */
 struct stimx274 {
struct v4l2_subdev sd;
@@ -567,7 +565,6 @@ struct stimx274 {
struct gpio_desc *reset_gpio;
struct mutex lock; /* mutex lock for operations */
const struct imx274_frmfmt *mode;
-   u32 mode_index;
 };
 
 /*
@@ -880,7 +877,6 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
index = 0;
}
 
-   imx274->mode_index = index;
imx274->mode = _formats[index];
 
if (fmt->width > IMX274_MAX_WIDTH)
@@ -1028,8 +1024,9 @@ static int imx274_s_stream(struct v4l2_subdev *sd, int on)
struct stimx274 *imx274 = to_imx274(sd);
int ret = 0;
 
-   dev_dbg(>client->dev, "%s : %s, mode index = %d\n", __func__,
-   on ? "Stream Start" : "Stream Stop", imx274->mode_index);
+   dev_dbg(>client->dev, "%s : %s, mode index = %td\n", __func__,
+   on ? "Stream Start" : "Stream Stop",
+   imx274->mode - _formats[0]);
 
mutex_lock(>lock);
 
@@ -1068,8 +1065,7 @@ static int imx274_s_stream(struct v4l2_subdev *sd, int on)
}
 
mutex_unlock(>lock);
-   dev_dbg(>client->dev,
-   "%s : Done: mode = %d\n", __func__, imx274->mode_index);
+   dev_dbg(>client->dev, "%s : Done\n", __func__);
return 0;
 
 fail:
@@ -1625,8 +1621,7 @@ static int imx274_probe(struct i2c_client *client,
mutex_init(>lock);
 
/* initialize format */
-   imx274->mode_index = IMX274_MODE_3840X2160;
-   imx274->mode = _formats[imx274->mode_index];
+   imx274->mode = _formats[IMX274_MODE_3840X2160];
imx274->format.width = imx274->mode->size.width;
imx274->format.height = imx274->mode->size.height;
imx274->format.field = V4L2_FIELD_NONE;
-- 
2.7.4

[PATCH v4 1/8] media: imx274: initialize format before v4l2 controls

[Luca Ceresoli]

The current probe function calls v4l2_ctrl_handler_setup() before
initializing the format info. This triggers call paths such as:
imx274_probe -> v4l2_ctrl_handler_setup -> imx274_s_ctrl ->
imx274_set_exposure, where priv->mode_index is accessed before being
assigned.

This is wrong but does not trigger a visible bug because priv is
zero-initialized and 0 is the default value for priv->mode_index. But
this would become a crash in follow-up commits when mode_index is
replaced by a pointer that must always be valid.

Fix the bug before it shows up by initializing struct members early.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3: nothing

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 63fb94e7da37..8a8a11b8d75d 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1632,6 +1632,16 @@ static int imx274_probe(struct i2c_client *client,
 
mutex_init(>lock);
 
+   /* initialize format */
+   imx274->mode_index = IMX274_MODE_3840X2160;
+   imx274->format.width = imx274_formats[0].size.width;
+   imx274->format.height = imx274_formats[0].size.height;
+   imx274->format.field = V4L2_FIELD_NONE;
+   imx274->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
+   imx274->format.colorspace = V4L2_COLORSPACE_SRGB;
+   imx274->frame_interval.numerator = 1;
+   imx274->frame_interval.denominator = IMX274_DEF_FRAME_RATE;
+
/* initialize regmap */
imx274->regmap = devm_regmap_init_i2c(client, _regmap_config);
if (IS_ERR(imx274->regmap)) {
@@ -1720,16 +1730,6 @@ static int imx274_probe(struct i2c_client *client,
goto err_ctrls;
}
 
-   /* initialize format */
-   imx274->mode_index = IMX274_MODE_3840X2160;
-   imx274->format.width = imx274_formats[0].size.width;
-   imx274->format.height = imx274_formats[0].size.height;
-   imx274->format.field = V4L2_FIELD_NONE;
-   imx274->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
-   imx274->format.colorspace = V4L2_COLORSPACE_SRGB;
-   imx274->frame_interval.numerator = 1;
-   imx274->frame_interval.denominator = IMX274_DEF_FRAME_RATE;
-
/* load default control values */
ret = imx274_load_default(imx274);
if (ret) {
-- 
2.7.4

[PATCH v4 5/8] media: imx274: simplify imx274_write_table()

[Luca Ceresoli]

imx274_write_table() is a mere wrapper (and the only user) to
imx274_regmap_util_write_table_8(). Remove this useless indirection by
merging the two functions into one.

Also get rid of the wait_ms_addr and end_addr parameters since it does
not make any sense to give them any values other than
IMX274_TABLE_WAIT_MS and IMX274_TABLE_END.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3: nothing

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index ceeec97cd330..48343c2ade83 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -597,20 +597,18 @@ static inline struct stimx274 *to_imx274(struct 
v4l2_subdev *sd)
 }
 
 /*
- * imx274_regmap_util_write_table_8 - Function for writing register table
- * @regmap: Pointer to device reg map structure
- * @table: Table containing register values
- * @wait_ms_addr: Flag for performing delay
- * @end_addr: Flag for incating end of table
+ * Writing a register table
+ *
+ * @priv: Pointer to device
+ * @table: Table containing register values (with optional delays)
  *
  * This is used to write register table into sensor's reg map.
  *
  * Return: 0 on success, errors otherwise
  */
-static int imx274_regmap_util_write_table_8(struct regmap *regmap,
-   const struct reg_8 table[],
-   u16 wait_ms_addr, u16 end_addr)
+static int imx274_write_table(struct stimx274 *priv, const struct reg_8 
table[])
 {
+   struct regmap *regmap = priv->regmap;
int err = 0;
const struct reg_8 *next;
u8 val;
@@ -622,8 +620,8 @@ static int imx274_regmap_util_write_table_8(struct regmap 
*regmap,
 
for (next = table;; next++) {
if ((next->addr != range_start + range_count) ||
-   (next->addr == end_addr) ||
-   (next->addr == wait_ms_addr) ||
+   (next->addr == IMX274_TABLE_END) ||
+   (next->addr == IMX274_TABLE_WAIT_MS) ||
(range_count == max_range_vals)) {
if (range_count == 1)
err = regmap_write(regmap,
@@ -642,10 +640,10 @@ static int imx274_regmap_util_write_table_8(struct regmap 
*regmap,
range_count = 0;
 
/* Handle special address values */
-   if (next->addr == end_addr)
+   if (next->addr == IMX274_TABLE_END)
break;
 
-   if (next->addr == wait_ms_addr) {
+   if (next->addr == IMX274_TABLE_WAIT_MS) {
msleep_range(next->val);
continue;
}
@@ -692,12 +690,6 @@ static inline int imx274_write_reg(struct stimx274 *priv, 
u16 addr, u8 val)
return err;
 }
 
-static int imx274_write_table(struct stimx274 *priv, const struct reg_8 
table[])
-{
-   return imx274_regmap_util_write_table_8(priv->regmap,
-   table, IMX274_TABLE_WAIT_MS, IMX274_TABLE_END);
-}
-
 /*
  * Set mode registers to start stream.
  * @priv: Pointer to device structure
-- 
2.7.4

[PATCH v4 6/8] media: imx274: add helper function to fill a reg_8 table chunk

[Luca Ceresoli]

Tables of struct reg_8 are used to simplify multi-byte register
assignment. However filling these snippets with values computed at
runtime is currently implemented by very similar functions doing the
needed shift & mask manipulation.

Replace all those functions with a unique helper function to fill
reg_8 tables in a simple and clean way.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3:
 - minor reformatting in prepare_reg() documentation

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 90 --
 1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 48343c2ade83..e5ba19b97083 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -597,6 +597,58 @@ static inline struct stimx274 *to_imx274(struct 
v4l2_subdev *sd)
 }
 
 /*
+ * Fill consecutive reg_8 items in order to set a multibyte register.
+ *
+ * The sensor has many 2-bytes registers and a 3-byte register. This
+ * simplifies code to set them by filling consecutive entries of a
+ * struct reg_8 table with the data to set a register.
+ *
+ * The number of table entries that is filled is the minimum needed
+ * for the given number of bits (i.e. nbits / 8, rounded up). If nbits
+ * is not a multiple of 8, extra bits in the most significant byte are
+ * zeroed.
+ *
+ * Example:
+ *   Calling prepare_reg([10], 0x3000, 0xcafe, 12) will set:
+ *   regs[10] = { .addr = 0x3000, .val = 0xfe }
+ *   regs[11] = { .addr = 0x3001, .val = 0x0a }
+ *
+ * @table_base: Pointer to the first reg_8 struct to be filled.  The
+ *  following entries will also be set, make sure they are
+ *  properly allocated!
+ * @addr_lsb: Address of the LSB register.  Other registers must be
+ *consecutive, least-to-most significant.
+ * @value: Value to be written to the register.
+ * @nbits: Number of bits to write (range: [1..24])
+ */
+static void prepare_reg(struct reg_8 *table_base,
+   u16 addr_lsb,
+   u32 value,
+   size_t nbits)
+{
+   struct reg_8 *cmd = table_base;
+   u16 addr = addr_lsb;
+   size_t bits; /* how many bits at this round */
+
+   WARN_ON(nbits > 24);
+
+   if (nbits > 24)
+   nbits = 24;
+
+   while (nbits > 0) {
+   bits = min_t(size_t, 8, nbits);
+
+   cmd->addr = addr;
+   cmd->val = value & ((1 << bits) - 1);
+
+   nbits -= bits;
+   value >>= 8;
+   addr++;
+   cmd++;
+   }
+}
+
+/*
  * Writing a register table
  *
  * @priv: Pointer to device
@@ -1163,15 +1215,6 @@ static int imx274_set_digital_gain(struct stimx274 
*priv, u32 dgain)
reg_val & IMX274_MASK_LSB_4_BITS);
 }
 
-static inline void imx274_calculate_gain_regs(struct reg_8 regs[2], u16 gain)
-{
-   regs->addr = IMX274_ANALOG_GAIN_ADDR_MSB;
-   regs->val = (gain >> IMX274_SHIFT_8_BITS) & IMX274_MASK_LSB_3_BITS;
-
-   (regs + 1)->addr = IMX274_ANALOG_GAIN_ADDR_LSB;
-   (regs + 1)->val = (gain) & IMX274_MASK_LSB_8_BITS;
-}
-
 /*
  * imx274_set_gain - Function called when setting gain
  * @priv: Pointer to device structure
@@ -1229,7 +1272,7 @@ static int imx274_set_gain(struct stimx274 *priv, struct 
v4l2_ctrl *ctrl)
if (gain_reg > IMX274_GAIN_REG_MAX)
gain_reg = IMX274_GAIN_REG_MAX;
 
-   imx274_calculate_gain_regs(reg_list, (u16)gain_reg);
+   prepare_reg(reg_list, IMX274_ANALOG_GAIN_ADDR_LSB, gain_reg, 11);
 
for (i = 0; i < ARRAY_SIZE(reg_list); i++) {
err = imx274_write_reg(priv, reg_list[i].addr,
@@ -1258,16 +1301,6 @@ static int imx274_set_gain(struct stimx274 *priv, struct 
v4l2_ctrl *ctrl)
return err;
 }
 
-static inline void imx274_calculate_coarse_time_regs(struct reg_8 regs[2],
-u32 coarse_time)
-{
-   regs->addr = IMX274_SHR_REG_MSB;
-   regs->val = (coarse_time >> IMX274_SHIFT_8_BITS)
-   & IMX274_MASK_LSB_8_BITS;
-   (regs + 1)->addr = IMX274_SHR_REG_LSB;
-   (regs + 1)->val = (coarse_time) & IMX274_MASK_LSB_8_BITS;
-}
-
 /*
  * imx274_set_coarse_time - Function called when setting SHR value
  * @priv: Pointer to device structure
@@ -1292,7 +1325,7 @@ static int imx274_set_coarse_time(struct stimx274 *priv, 
u32 *val)
goto fail;
 
/* prepare SHR registers */
-   imx274_calculate_coarse_time_regs(reg_list, coarse_time);
+   prepare_reg(reg_list, IMX274_SHR_REG_LSB, coarse_time, 16);
 
/* write to SHR registers */
for (i = 0; i < ARRAY_SIZE(reg_list); i++) {
@@ -1429,19 +1462,6 @@ static int imx274_set_test_pattern(struct stimx274 
*priv, int val)
return err;
 }
 
-static inline void 

[PATCH v4 8/8] media: imx274: add SELECTION support for cropping

[Luca Ceresoli]

Currently this driver does not support cropping. The supported modes
are the following, all capturing the entire area:

 - 3840x2160, 1:1 binning (native sensor resolution)
 - 1920x1080, 2:1 binning
 - 1280x720,  3:1 binning

The VIDIOC_SUBDEV_S_FMT ioctl chooses among these 3 configurations the
one that matches the requested format.

Add cropping support via VIDIOC_SUBDEV_S_SELECTION: with target
V4L2_SEL_TGT_CROP to choose the captured area, with
V4L2_SEL_TGT_COMPOSE to choose the output resolution.

To maintain backward compatibility we also allow setting the compose
format via VIDIOC_SUBDEV_S_FMT. To obtain this, compose rect and
output format are computed in the common helper function
__imx274_change_compose(), which sets both to the same width/height
values.

Cropping also calls __imx274_change_compose() whenever cropping rect
size changes in order to reset the compose rect (and output format
size) for 1:1 binning.

Also change the names in enum imx274_mode from being resolution-based
to being binning-based (e.g. from IMX274_MODE_1920X1080 to
IMX274_MODE_BINNING_2_1). Without cropping, the two naming are
equivalent. With cropping, the resolution could be different,
e.g. using 2:1 binning mode to crop 1200x960 and output a 600x480
format. Using binning in the names avoids any misunderstanding.  For
the same reason, replace the 'size' field in struct imx274_frmfmt with
'bin_ratio'.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4:
 - Set the binning via the SELECTION API (COMPOSE rect), but still
   allow using VIDIOC_SUBDEV_S_FMT for backward compatibility.
 - rename imx274_set_trimming -> imx274_apply_trimming for clarity

Changed v2 -> v3:
 - Remove hard-coded HMAX reg setting from all modes, not only the
   first one. HMAX is always computed and set in s_stream now.
 - Reword commit log message.

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 416 +++--
 1 file changed, 326 insertions(+), 90 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 2c13961e9764..8bfc20a46b3d 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -5,6 +5,7 @@
  *
  * Leon Luo 
  * Edwin Zou 
+ * Luca Ceresoli 
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,6 +20,7 @@
  * along with this program.  If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -74,7 +76,7 @@
  */
 #define IMX274_MIN_EXPOSURE_TIME   (4 * 260 / 72)
 
-#define IMX274_DEFAULT_MODEIMX274_MODE_3840X2160
+#define IMX274_DEFAULT_MODEIMX274_MODE_BINNING_OFF
 #define IMX274_MAX_WIDTH   (3840)
 #define IMX274_MAX_HEIGHT  (2160)
 #define IMX274_MAX_FRAME_RATE  (120)
@@ -111,6 +113,20 @@
 #define IMX274_SHR_REG_LSB 0x300C /* SHR */
 #define IMX274_SVR_REG_MSB 0x300F /* SVR */
 #define IMX274_SVR_REG_LSB 0x300E /* SVR */
+#define IMX274_HTRIM_EN_REG0x3037
+#define IMX274_HTRIM_START_REG_LSB 0x3038
+#define IMX274_HTRIM_START_REG_MSB 0x3039
+#define IMX274_HTRIM_END_REG_LSB   0x303A
+#define IMX274_HTRIM_END_REG_MSB   0x303B
+#define IMX274_VWIDCUTEN_REG   0x30DD
+#define IMX274_VWIDCUT_REG_LSB 0x30DE
+#define IMX274_VWIDCUT_REG_MSB 0x30DF
+#define IMX274_VWINPOS_REG_LSB 0x30E0
+#define IMX274_VWINPOS_REG_MSB 0x30E1
+#define IMX274_WRITE_VSIZE_REG_LSB 0x3130
+#define IMX274_WRITE_VSIZE_REG_MSB 0x3131
+#define IMX274_Y_OUT_SIZE_REG_LSB  0x3132
+#define IMX274_Y_OUT_SIZE_REG_MSB  0x3133
 #define IMX274_VMAX_REG_1  0x30FA /* VMAX, MSB */
 #define IMX274_VMAX_REG_2  0x30F9 /* VMAX */
 #define IMX274_VMAX_REG_3  0x30F8 /* VMAX, LSB */
@@ -141,9 +157,9 @@ static const struct regmap_config imx274_regmap_config = {
 };
 
 enum imx274_mode {
-   IMX274_MODE_3840X2160,
-   IMX274_MODE_1920X1080,
-   IMX274_MODE_1280X720,
+   IMX274_MODE_BINNING_OFF,
+   IMX274_MODE_BINNING_2_1,
+   IMX274_MODE_BINNING_3_1,
 };
 
 /*
@@ -152,8 +168,8 @@ enum imx274_mode {
  * These are the values to configure the sensor in one of the
  * implemented modes.
  *
- * @size: recommended recording pixels
  * @init_regs: registers to initialize the mode
+ * @bin_ratio: downscale factor (e.g. 3 for 3:1 binning)
  * @min_frame_len: Minimum frame length for each mode (see "Frame Rate
  * Adjustment (CSI-2)" in the datasheet)
  * @min_SHR: Minimum SHR register value (see "Shutter Setting (CSI-2)" in the
@@ -163,8 +179,8 @@ enum 

[PATCH v4 2/8] media: imx274: consolidate per-mode data in imx274_frmfmt

[Luca Ceresoli]

Data about the implemented readout modes is partially stored in
imx274_formats[], the rest is scattered in several arrays. The latter
are then accessed using the mode index, e.g.:

  min_frame_len[priv->mode_index]

Consolidate all these data in imx274_formats[], and store a pointer to
the selected mode (i.e. imx274_formats[priv->mode_index]) in the main
device struct. This way code to use these data becomes more readable,
e.g.:

  priv->mode->min_frame_len

This removes lots of scaffolding code and keeps data about each mode
in a unique place.

Also remove a parameter to imx274_mode_regs() that is now unused.

While this adds the mode pointer to the device struct, it does not
remove the mode_index from it because mode_index is still used in two
dev_dbg() calls.  This will be handled in a follow-up commit.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3: nothing

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 139 +
 1 file changed, 66 insertions(+), 73 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 8a8a11b8d75d..2ec31ae4e60d 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -147,10 +147,28 @@ enum imx274_mode {
 };
 
 /*
- * imx274 format related structure
+ * Parameters for each imx274 readout mode.
+ *
+ * These are the values to configure the sensor in one of the
+ * implemented modes.
+ *
+ * @size: recommended recording pixels
+ * @init_regs: registers to initialize the mode
+ * @min_frame_len: Minimum frame length for each mode (see "Frame Rate
+ * Adjustment (CSI-2)" in the datasheet)
+ * @min_SHR: Minimum SHR register value (see "Shutter Setting (CSI-2)" in the
+ *   datasheet)
+ * @max_fps: Maximum frames per second
+ * @nocpiop: Number of clocks per internal offset period (see "Integration Time
+ *   in Each Readout Drive Mode (CSI-2)" in the datasheet)
  */
 struct imx274_frmfmt {
struct v4l2_frmsize_discrete size;
+   const struct reg_8 *init_regs;
+   int min_frame_len;
+   int min_SHR;
+   int max_fps;
+   int nocpiop;
 };
 
 /*
@@ -476,58 +494,35 @@ static const struct reg_8 imx274_tp_regs[] = {
{IMX274_TABLE_END, 0x00}
 };
 
-static const struct reg_8 *mode_table[] = {
-   [IMX274_MODE_3840X2160] = imx274_mode1_3840x2160_raw10,
-   [IMX274_MODE_1920X1080] = imx274_mode3_1920x1080_raw10,
-   [IMX274_MODE_1280X720]  = imx274_mode5_1280x720_raw10,
-};
-
-/*
- * imx274 format related structure
- */
+/* nocpiop happens to be the same number for the implemented modes */
 static const struct imx274_frmfmt imx274_formats[] = {
-   { {3840, 2160} },
-   { {1920, 1080} },
-   { {1280,  720} },
-};
-
-/*
- * minimal frame length for each mode
- * refer to datasheet section "Frame Rate Adjustment (CSI-2)"
- */
-static const int min_frame_len[] = {
-   4550, /* mode 1, 4K */
-   2310, /* mode 3, 1080p */
-   2310 /* mode 5, 720p */
-};
-
-/*
- * minimal numbers of SHR register
- * refer to datasheet table "Shutter Setting (CSI-2)"
- */
-static const int min_SHR[] = {
-   12, /* mode 1, 4K */
-   8, /* mode 3, 1080p */
-   8 /* mode 5, 720p */
-};
-
-static const int max_frame_rate[] = {
-   60, /* mode 1 , 4K */
-   120, /* mode 3, 1080p */
-   120 /* mode 5, 720p */
-};
-
-/*
- * Number of clocks per internal offset period
- * a constant based on mode
- * refer to section "Integration Time in Each Readout Drive Mode (CSI-2)"
- * in the datasheet
- * for the implemented 3 modes, it happens to be the same number
- */
-static const int nocpiop[] = {
-   112, /* mode 1 , 4K */
-   112, /* mode 3, 1080p */
-   112 /* mode 5, 720p */
+   {
+   /* mode 1, 4K */
+   .size = {3840, 2160},
+   .init_regs = imx274_mode1_3840x2160_raw10,
+   .min_frame_len = 4550,
+   .min_SHR = 12,
+   .max_fps = 60,
+   .nocpiop = 112,
+   },
+   {
+   /* mode 3, 1080p */
+   .size = {1920, 1080},
+   .init_regs = imx274_mode3_1920x1080_raw10,
+   .min_frame_len = 2310,
+   .min_SHR = 8,
+   .max_fps = 120,
+   .nocpiop = 112,
+   },
+   {
+   /* mode 5, 720p */
+   .size = {1280, 720},
+   .init_regs = imx274_mode5_1280x720_raw10,
+   .min_frame_len = 2310,
+   .min_SHR = 8,
+   .max_fps = 120,
+   .nocpiop = 112,
+   },
 };
 
 /*
@@ -557,6 +552,8 @@ struct imx274_ctrls {
  * @regmap: Pointer to regmap structure
  * @reset_gpio: Pointer to reset gpio
  * @lock: Mutex structure
+ * @mode: Parameters for the selected readout mode
+ *(points to imx274_formats[mode_index])
  * 

[PATCH v4 4/8] media: imx274: actually use IMX274_DEFAULT_MODE

[Luca Ceresoli]

IMX274_DEFAULT_MODE is defined but not used. Start using it, so the
default can be more easily changed without digging into the code.

Signed-off-by: Luca Ceresoli 
Cc: Sakari Ailus 

---
Changed v3 -> v4: nothing

Changed v2 -> v3: nothing

Changed v1 -> v2:
 - add "media: " prefix to commit message
---
 drivers/media/i2c/imx274.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index f075715ffced..ceeec97cd330 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1621,7 +1621,7 @@ static int imx274_probe(struct i2c_client *client,
mutex_init(>lock);
 
/* initialize format */
-   imx274->mode = _formats[IMX274_MODE_3840X2160];
+   imx274->mode = _formats[IMX274_DEFAULT_MODE];
imx274->format.width = imx274->mode->size.width;
imx274->format.height = imx274->mode->size.height;
imx274->format.field = V4L2_FIELD_NONE;
-- 
2.7.4

Re: [PATCH 2/2] media: ov5640: add support of module orientation

[Hugues FRUCHET]

Hi Sakari,

I'm fine with the change to dev_fwnode(>dev).

Many thanks Sakari,

Hugues.

On 06/11/2018 12:10 PM, Sakari Ailus wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>> Add support of module being physically mounted upside down.
>> In this case, mirror and flip are enabled to fix captured images
>> orientation.
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   .../devicetree/bindings/media/i2c/ov5640.txt   |  3 +++
>>   drivers/media/i2c/ov5640.c | 28 
>> --
>>   2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
>> b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> index 8e36da0..f76eb7e 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> @@ -13,6 +13,8 @@ Optional Properties:
>> This is an active low signal to the OV5640.
>>   - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
>> if any. This is an active high signal to the OV5640.
>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>> +and 180 (sensor mounted upside down).
>>   
>>   The device node must contain one 'port' child node for its digital output
>>   video port, in accordance with the video interface bindings defined in
>> @@ -51,6 +53,7 @@ Examples:
>>  DVDD-supply = <_reg>;  /* 1.5v */
>>  powerdown-gpios = < 19 GPIO_ACTIVE_HIGH>;
>>  reset-gpios = < 20 GPIO_ACTIVE_LOW>;
>> +rotation = <180>;
>>   
>>  port {
>>  /* MIPI CSI-2 bus endpoint */
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index 41039e5..5529b14 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -215,6 +215,7 @@ struct ov5640_dev {
>>  struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
>>  struct gpio_desc *reset_gpio;
>>  struct gpio_desc *pwdn_gpio;
>> +bool   upside_down;
>>   
>>  /* lock to protect all members below */
>>  struct mutex lock;
>> @@ -,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct 
>> ov5640_dev *sensor, int value)
>>   static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>>   {
>>  /*
>> + * If sensor is mounted upside down, mirror logic is inversed.
>> + *
>>   * Sensor is a BSI (Back Side Illuminated) one,
>>   * so image captured is physically mirrored.
>>   * This is why mirror logic is inversed in
>> @@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev 
>> *sensor, int value)
>>   */
>>  return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
>>BIT(2) | BIT(1),
>> -  (!value) ? (BIT(2) | BIT(1)) : 0);
>> +  (!(value ^ sensor->upside_down)) ?
>> +  (BIT(2) | BIT(1)) : 0);
>>   }
>>   
>>   static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>>   {
>> +/* If sensor is mounted upside down, flip logic is inversed */
>> +
>>  /*
>>   * TIMING TC REG20:
>>   * - [2]:   ISP vflip
>> @@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev 
>> *sensor, int value)
>>   */
>>  return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
>>BIT(2) | BIT(1),
>> -  value ? (BIT(2) | BIT(1)) : 0);
>> +  (value ^ sensor->upside_down) ?
>> +  (BIT(2) | BIT(1)) : 0);
>>   }
>>   
>>   static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> @@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
>>  struct fwnode_handle *endpoint;
>>  struct ov5640_dev *sensor;
>>  struct v4l2_mbus_framefmt *fmt;
>> +u32 rotation;
>>  int ret;
>>   
>>  sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>> @@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
>>   
>>  sensor->ae_target = 52;
>>   
>> +/* optional indication of physical rotation of sensor */
>> +ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
> 
> Instead of of_fwnode_handle(), please use dev_fwnode(>dev) --- as the
> driver already does elsewhere.
> 
> I can make the change if you're happy with that; the patches seem fine
> otherwise.
> 
>> +   "rotation", );
>> +if (!ret) {
>> +switch (rotation) {
>> +case 180:
>> +sensor->upside_down = true;
>> +/* fall through */
>> +case 0:
>> +break;
>> +default:
>> +dev_warn(dev, "%u degrees rotation is not supported, 
>> ignoring...\n",
>> + 

Re: [PATCH v7 0/6] Add ChromeOS EC CEC Support

[Hans Verkuil]

On 11/06/18 10:56, Neil Armstrong wrote:
> Hi Lee,
> 
> On 11/06/2018 08:03, Lee Jones wrote:
>> On Fri, 08 Jun 2018, Hans Verkuil wrote:
>>> On 08/06/18 10:17, Neil Armstrong wrote:
 On 08/06/2018 09:53, Hans Verkuil wrote:
> On 06/01/2018 10:19 AM, Neil Armstrong wrote:
>> Hi All,
>>
>> The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support
>> through it's Embedded Controller, to enable the Linux CEC Core to 
>> communicate
>> with it and get the CEC Physical Address from the correct HDMI 
>> Connector, the
>> following must be added/changed:
>> - Add the CEC sub-device registration in the ChromeOS EC MFD Driver
>> - Add the CEC related commands and events definitions into the EC MFD 
>> driver
>> - Add a way to get a CEC notifier with it's (optional) connector name
>> - Add the CEC notifier to the i915 HDMI driver
>> - Add the proper ChromeOS EC CEC Driver
>>
>> The CEC notifier with the connector name is the tricky point, since even 
>> on
>> Device-Tree platforms, there is no way to distinguish between multiple 
>> HDMI
>> connectors from the same DRM driver. The solution I implemented is pretty
>> simple and only adds an optional connector name to eventually distinguish
>> an HDMI connector notifier from another if they share the same device.
>
> This looks good to me, which brings me to the next question: how to merge
> this?
>
> It touches on three subsystems (media, drm, mfd), so that makes this
> tricky.
>
> I think there are two options: either the whole series goes through the
> media tree, or patches 1+2 go through drm and 3-6 through media. If there
> is a high chance of conflicts in the mfd code, then it is also an option 
> to
> have patches 3-6 go through the mfd subsystem.

 I think patches 3-6 should go in the mfd tree, Lee is used to handle this,
 then I think the rest could go in the media tree.

 Lee, do you think it would be possible to have an immutable branch with 
 patches 3-6 ?

 Could we have an immutable branch from media tree with patch 1 to be 
 merged in
 the i915 tree for patch 2 ?

 Or patch 1+2 could me merged into the i915 tree and generate an immutable 
 branch
>>>
>>> I think patches 1+2 can just go to the i915 tree. The i915 driver changes 
>>> often,
>>> so going through that tree makes sense. The cec-notifier code is unlikely 
>>> to change,
>>> and I am fine with that patch going through i915.
>>>
 for media to merge the mfd branch + patch 7 ?
>>>
>>> Patch 7? I only count 6?
>>>
>>> If 1+2 go through drm and 3-6 go through mfd, then media isn't affected at 
>>> all.
>>> There is chance of a conflict when this is eventually pushed to mainline for
>>> the media Kconfig, but that's all.
>>
>> What are the *build* dependencies within the set?
> 
> Here are the hard the build dependency :
> 
> Patch 2 depends on Patch 1
> Patch 5 depends on Patch 4
> Patch 6 depends on Patches 1 & 4

Ah, I missed the dependency of patch 6 on patch 1. So the whole series needs
to be merged as a single unit.

> 
>>
>> I'd be happy to send out a pull-request for either all of the patches,
>> or just the MFD changes once I've had chance to review them.
>>
> 
> Great, thanks !
> 
> Neil
> 

I'm OK if this goes through the mfd tree.

Regards,

Hans

Re: [PATCH 2/2] media: ov5640: add support of module orientation

[Sakari Ailus]

On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> Add support of module being physically mounted upside down.
> In this case, mirror and flip are enabled to fix captured images
> orientation.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   |  3 +++
>  drivers/media/i2c/ov5640.c | 28 
> --
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0..f76eb7e 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> @@ -13,6 +13,8 @@ Optional Properties:
>  This is an active low signal to the OV5640.
>  - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
>  if any. This is an active high signal to the OV5640.
> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).
>  
>  The device node must contain one 'port' child node for its digital output
>  video port, in accordance with the video interface bindings defined in
> @@ -51,6 +53,7 @@ Examples:
>   DVDD-supply = <_reg>;  /* 1.5v */
>   powerdown-gpios = < 19 GPIO_ACTIVE_HIGH>;
>   reset-gpios = < 20 GPIO_ACTIVE_LOW>;
> + rotation = <180>;
>  
>   port {
>   /* MIPI CSI-2 bus endpoint */
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 41039e5..5529b14 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -215,6 +215,7 @@ struct ov5640_dev {
>   struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
>   struct gpio_desc *reset_gpio;
>   struct gpio_desc *pwdn_gpio;
> + bool   upside_down;
>  
>   /* lock to protect all members below */
>   struct mutex lock;
> @@ -,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev 
> *sensor, int value)
>  static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>  {
>   /*
> +  * If sensor is mounted upside down, mirror logic is inversed.
> +  *
>* Sensor is a BSI (Back Side Illuminated) one,
>* so image captured is physically mirrored.
>* This is why mirror logic is inversed in
> @@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev 
> *sensor, int value)
>*/
>   return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
> BIT(2) | BIT(1),
> -   (!value) ? (BIT(2) | BIT(1)) : 0);
> +   (!(value ^ sensor->upside_down)) ?
> +   (BIT(2) | BIT(1)) : 0);
>  }
>  
>  static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>  {
> + /* If sensor is mounted upside down, flip logic is inversed */
> +
>   /*
>* TIMING TC REG20:
>* - [2]:   ISP vflip
> @@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev 
> *sensor, int value)
>*/
>   return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
> BIT(2) | BIT(1),
> -   value ? (BIT(2) | BIT(1)) : 0);
> +   (value ^ sensor->upside_down) ?
> +   (BIT(2) | BIT(1)) : 0);
>  }
>  
>  static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> @@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
>   struct fwnode_handle *endpoint;
>   struct ov5640_dev *sensor;
>   struct v4l2_mbus_framefmt *fmt;
> + u32 rotation;
>   int ret;
>  
>   sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> @@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
>  
>   sensor->ae_target = 52;
>  
> + /* optional indication of physical rotation of sensor */
> + ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),

Instead of of_fwnode_handle(), please use dev_fwnode(>dev) --- as the
driver already does elsewhere.

I can make the change if you're happy with that; the patches seem fine
otherwise.

> +"rotation", );
> + if (!ret) {
> + switch (rotation) {
> + case 180:
> + sensor->upside_down = true;
> + /* fall through */
> + case 0:
> + break;
> + default:
> + dev_warn(dev, "%u degrees rotation is not supported, 
> ignoring...\n",
> +  rotation);
> + }
> + }
> +
>   endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(>dev),
> NULL);
>   if (!endpoint) {

-- 
Kind regards,

Sakari 

[PATCH] media: stm32-dcmi: revisit stop streaming ops

[Hugues Fruchet]

Do not wait for interrupt completion when stopping streaming,
stopping sensor and disabling interruptions are enough.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 29 +
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 581ded0..f0134a6 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -87,7 +87,6 @@ enum state {
STOPPED = 0,
WAIT_FOR_BUFFER,
RUNNING,
-   STOPPING,
 };
 
 #define MIN_WIDTH  16U
@@ -432,18 +431,6 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
 
spin_lock_irq(>irqlock);
 
-   /* Stop capture is required */
-   if (dcmi->state == STOPPING) {
-   reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
-
-   dcmi->state = STOPPED;
-
-   complete(>complete);
-
-   spin_unlock_irq(>irqlock);
-   return IRQ_HANDLED;
-   }
-
if ((dcmi->misr & IT_OVR) || (dcmi->misr & IT_ERR)) {
dcmi->errors_count++;
if (dcmi->misr & IT_OVR)
@@ -701,8 +688,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
 {
struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
struct dcmi_buf *buf, *node;
-   unsigned long time_ms = msecs_to_jiffies(TIMEOUT_MS);
-   long timeout;
int ret;
 
/* Disable stream on the sub device */
@@ -712,13 +697,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
__func__, ret);
 
spin_lock_irq(>irqlock);
-   dcmi->state = STOPPING;
-   spin_unlock_irq(>irqlock);
-
-   timeout = wait_for_completion_interruptible_timeout(>complete,
-   time_ms);
-
-   spin_lock_irq(>irqlock);
 
/* Disable interruptions */
reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
@@ -726,12 +704,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
/* Disable DCMI */
reg_clear(dcmi->regs, DCMI_CR, CR_ENABLE);
 
-   if (!timeout) {
-   dev_err(dcmi->dev, "%s: Timeout during stop streaming\n",
-   __func__);
-   dcmi->state = STOPPED;
-   }
-
/* Return all queued buffers to vb2 in ERROR state */
list_for_each_entry_safe(buf, node, >buffers, list) {
list_del_init(>list);
@@ -739,6 +711,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
}
 
dcmi->active = NULL;
+   dcmi->state = STOPPED;
 
spin_unlock_irq(>irqlock);
 
-- 
1.9.1

[PATCH 3/4] media: stm32-dcmi: clarify state logic on buffer starvation

[Hugues Fruchet]

Introduce WAIT_FOR_BUFFER state instead of "active" field checking
to manage buffer starvation case.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 6ccf195..93bb03a 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -85,6 +85,7 @@
 
 enum state {
STOPPED = 0,
+   WAIT_FOR_BUFFER,
RUNNING,
STOPPING,
 };
@@ -230,6 +231,7 @@ static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
if (list_empty(>buffers)) {
dev_dbg(dcmi->dev, "Capture restart is deferred to next buffer 
queueing\n");
dcmi->active = NULL;
+   dcmi->state = WAIT_FOR_BUFFER;
spin_unlock_irq(>irqlock);
return 0;
}
@@ -548,9 +550,11 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
 
spin_lock_irq(>irqlock);
 
-   if (dcmi->state == RUNNING && !dcmi->active) {
dcmi->active = buf;
 
+   if (dcmi->state == WAIT_FOR_BUFFER) {
+   dcmi->state = RUNNING;
+
dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n",
buf->vb.vb2_buf.index);
 
@@ -630,8 +634,6 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
/* Enable dcmi */
reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
 
-   dcmi->state = RUNNING;
-
dcmi->sequence = 0;
dcmi->errors_count = 0;
dcmi->overrun_count = 0;
@@ -644,6 +646,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 */
if (list_empty(>buffers)) {
dev_dbg(dcmi->dev, "Start streaming is deferred to next buffer 
queueing\n");
+   dcmi->state = WAIT_FOR_BUFFER;
spin_unlock_irq(>irqlock);
return 0;
}
@@ -653,6 +656,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 
dev_dbg(dcmi->dev, "Start streaming, starting capture\n");
 
+   dcmi->state = RUNNING;
+
spin_unlock_irq(>irqlock);
ret = dcmi_start_capture(dcmi);
if (ret) {
-- 
1.9.1

[PATCH] media: stm32-dcmi: code cleanup

[Hugues Fruchet]

Minor non-functional fixes around comments, variable namings
and trace point enhancement.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index c55e6b5..b796334 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -249,13 +249,12 @@ static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
 static void dcmi_dma_callback(void *param)
 {
struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
-   struct dma_chan *chan = dcmi->dma_chan;
struct dma_tx_state state;
enum dma_status status;
struct dcmi_buf *buf = dcmi->active;
 
/* Check DMA status */
-   status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
+   status = dmaengine_tx_status(dcmi->dma_chan, dcmi->dma_cookie, );
 
switch (status) {
case DMA_IN_PROGRESS:
@@ -309,10 +308,11 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
/* Prepare a DMA transaction */
desc = dmaengine_prep_slave_single(dcmi->dma_chan, buf->paddr,
   buf->size,
-  DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+  DMA_DEV_TO_MEM,
+  DMA_PREP_INTERRUPT);
if (!desc) {
-   dev_err(dcmi->dev, "%s: DMA dmaengine_prep_slave_single failed 
for buffer size %zu\n",
-   __func__, buf->size);
+   dev_err(dcmi->dev, "%s: DMA dmaengine_prep_slave_single failed 
for buffer phy=%pad size=%zu\n",
+   __func__, >paddr, buf->size);
return -EINVAL;
}
 
@@ -378,7 +378,6 @@ static void dcmi_process_jpeg(struct stm32_dcmi *dcmi)
 {
struct dma_tx_state state;
enum dma_status status;
-   struct dma_chan *chan = dcmi->dma_chan;
struct dcmi_buf *buf = dcmi->active;
 
if (!buf)
@@ -386,8 +385,7 @@ static void dcmi_process_jpeg(struct stm32_dcmi *dcmi)
 
/*
 * Because of variable JPEG buffer size sent by sensor,
-* DMA transfer never completes due to transfer size
-* never reached.
+* DMA transfer never completes due to transfer size never reached.
 * In order to ensure that all the JPEG data are transferred
 * in active buffer memory, DMA is drained.
 * Then DMA tx status gives the amount of data transferred
@@ -396,10 +394,10 @@ static void dcmi_process_jpeg(struct stm32_dcmi *dcmi)
 */
 
/* Drain DMA */
-   dmaengine_synchronize(chan);
+   dmaengine_synchronize(dcmi->dma_chan);
 
/* Get DMA residue to get JPEG size */
-   status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
+   status = dmaengine_tx_status(dcmi->dma_chan, dcmi->dma_cookie, );
if (status != DMA_ERROR && state.residue < buf->size) {
/* Return JPEG buffer to V4L2 with received JPEG buffer size */
dcmi_buffer_done(dcmi, buf, buf->size - state.residue, 0);
-- 
1.9.1

[PATCH 0/4] Revisit and fix DCMI buffers handling

[Hugues Fruchet]

Revisit and fix DCMI buffers handling.

Hugues Fruchet (4):
  media: stm32-dcmi: do not fall into error on buffer starvation
  media: stm32-dcmi: return buffer in error state on dma error
  media: stm32-dcmi: clarify state logic on buffer starvation
  media: stm32-dcmi: revisit buffer list management

 drivers/media/platform/stm32/stm32-dcmi.c | 80 ---
 1 file changed, 41 insertions(+), 39 deletions(-)

-- 
1.9.1

[PATCH 1/4] media: stm32-dcmi: do not fall into error on buffer starvation

[Hugues Fruchet]

Return silently instead of falling into error when running
out of available buffers when restarting capture.
Capture will be restarted when new buffers will be
provided by V4L2 client.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index b796334..a3fbfac 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -228,13 +228,10 @@ static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
 
/* Restart a new DMA transfer with next buffer */
if (list_empty(>buffers)) {
-   dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture 
buffer\n",
-   __func__);
-   dcmi->errors_count++;
+   dev_dbg(dcmi->dev, "Capture restart is deferred to next buffer 
queueing\n");
dcmi->active = NULL;
-
spin_unlock_irq(>irqlock);
-   return -EINVAL;
+   return 0;
}
 
dcmi->active = list_entry(dcmi->buffers.next,
-- 
1.9.1

[PATCH 2/4] media: stm32-dcmi: return buffer in error state on dma error

[Hugues Fruchet]

Return buffer to V4L2 in error state if DMA error occurs.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index a3fbfac..6ccf195 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -262,6 +262,9 @@ static void dcmi_dma_callback(void *param)
break;
case DMA_ERROR:
dev_err(dcmi->dev, "%s: Received DMA_ERROR\n", __func__);
+
+   /* Return buffer to V4L2 in error state */
+   dcmi_buffer_done(dcmi, buf, 0, -EIO);
break;
case DMA_COMPLETE:
dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
-- 
1.9.1

[PATCH 4/4] media: stm32-dcmi: revisit buffer list management

[Hugues Fruchet]

Cleanup "active" field usage and enhance list management
to avoid exceptions when releasing buffers on error or
stopping streaming.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 65 +++
 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 93bb03a..581ded0 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -191,7 +191,7 @@ static inline void reg_clear(void __iomem *base, u32 reg, 
u32 mask)
reg_write(base, reg, reg_read(base, reg) & ~mask);
 }
 
-static int dcmi_start_capture(struct stm32_dcmi *dcmi);
+static int dcmi_start_capture(struct stm32_dcmi *dcmi, struct dcmi_buf *buf);
 
 static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
 struct dcmi_buf *buf,
@@ -203,6 +203,8 @@ static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
if (!buf)
return;
 
+   list_del_init(>list);
+
vbuf = >vb;
 
vbuf->sequence = dcmi->sequence++;
@@ -220,6 +222,8 @@ static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
 
 static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
 {
+   struct dcmi_buf *buf;
+
spin_lock_irq(>irqlock);
 
if (dcmi->state != RUNNING) {
@@ -230,19 +234,16 @@ static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
/* Restart a new DMA transfer with next buffer */
if (list_empty(>buffers)) {
dev_dbg(dcmi->dev, "Capture restart is deferred to next buffer 
queueing\n");
-   dcmi->active = NULL;
dcmi->state = WAIT_FOR_BUFFER;
spin_unlock_irq(>irqlock);
return 0;
}
-
-   dcmi->active = list_entry(dcmi->buffers.next,
- struct dcmi_buf, list);
-   list_del_init(>active->list);
+   buf = list_entry(dcmi->buffers.next, struct dcmi_buf, list);
+   dcmi->active = buf;
 
spin_unlock_irq(>irqlock);
 
-   return dcmi_start_capture(dcmi);
+   return dcmi_start_capture(dcmi, buf);
 }
 
 static void dcmi_dma_callback(void *param)
@@ -252,6 +253,8 @@ static void dcmi_dma_callback(void *param)
enum dma_status status;
struct dcmi_buf *buf = dcmi->active;
 
+   spin_lock_irq(>irqlock);
+
/* Check DMA status */
status = dmaengine_tx_status(dcmi->dma_chan, dcmi->dma_cookie, );
 
@@ -274,15 +277,19 @@ static void dcmi_dma_callback(void *param)
/* Return buffer to V4L2 */
dcmi_buffer_done(dcmi, buf, buf->size, 0);
 
+   spin_unlock_irq(>irqlock);
+
/* Restart capture */
if (dcmi_restart_capture(dcmi))
dev_err(dcmi->dev, "%s: Cannot restart capture on DMA 
complete\n",
__func__);
-   break;
+   return;
default:
dev_err(dcmi->dev, "%s: Received unknown status\n", __func__);
break;
}
+
+   spin_unlock_irq(>irqlock);
 }
 
 static int dcmi_start_dma(struct stm32_dcmi *dcmi,
@@ -334,10 +341,9 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
return 0;
 }
 
-static int dcmi_start_capture(struct stm32_dcmi *dcmi)
+static int dcmi_start_capture(struct stm32_dcmi *dcmi, struct dcmi_buf *buf)
 {
int ret;
-   struct dcmi_buf *buf = dcmi->active;
 
if (!buf)
return -EINVAL;
@@ -491,8 +497,6 @@ static int dcmi_queue_setup(struct vb2_queue *vq,
*nplanes = 1;
sizes[0] = size;
 
-   dcmi->active = NULL;
-
dev_dbg(dcmi->dev, "Setup queue, count=%d, size=%d\n",
*nbuffers, size);
 
@@ -550,23 +554,24 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
 
spin_lock_irq(>irqlock);
 
-   dcmi->active = buf;
+   /* Enqueue to video buffers list */
+   list_add_tail(>list, >buffers);
 
if (dcmi->state == WAIT_FOR_BUFFER) {
dcmi->state = RUNNING;
+   dcmi->active = buf;
 
dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n",
buf->vb.vb2_buf.index);
 
spin_unlock_irq(>irqlock);
-   if (dcmi_start_capture(dcmi))
+   if (dcmi_start_capture(dcmi, buf))
dev_err(dcmi->dev, "%s: Cannot restart capture on 
overflow or error\n",
__func__);
-   } else {
-   /* Enqueue to video buffers list */
-   list_add_tail(>list, >buffers);
-   spin_unlock_irq(>irqlock);
+   return;
}
+
+   spin_unlock_irq(>irqlock);
 }
 
 static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
@@ -638,7 +643,6 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)

[PATCH] media: stm32-dcmi: increase max width/height to 2592

[Hugues Fruchet]

DCMI can capture 5Mp raw frames, increase limit accordingly.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 68da9ec..c55e6b5 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -90,14 +90,9 @@ enum state {
 };
 
 #define MIN_WIDTH  16U
-#define MAX_WIDTH  2048U
+#define MAX_WIDTH  2592U
 #define MIN_HEIGHT 16U
-#define MAX_HEIGHT 2048U
-
-#define MIN_JPEG_WIDTH 16U
-#define MAX_JPEG_WIDTH 2592U
-#define MIN_JPEG_HEIGHT16U
-#define MAX_JPEG_HEIGHT2592U
+#define MAX_HEIGHT 2592U
 
 #define TIMEOUT_MS 1000
 
@@ -844,14 +839,8 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct 
v4l2_format *f,
}
 
/* Limit to hardware capabilities */
-   if (pix->pixelformat == V4L2_PIX_FMT_JPEG) {
-   pix->width = clamp(pix->width, MIN_JPEG_WIDTH, MAX_JPEG_WIDTH);
-   pix->height =
-   clamp(pix->height, MIN_JPEG_HEIGHT, MAX_JPEG_HEIGHT);
-   } else {
-   pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH);
-   pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT);
-   }
+   pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH);
+   pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT);
 
/* No crop if JPEG is requested */
do_crop = dcmi->do_crop && (pix->pixelformat != V4L2_PIX_FMT_JPEG);
-- 
1.9.1

[PATCH] media: stm32-dcmi: add power saving support

[Hugues Fruchet]

Implements runtime & system sleep power management ops.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 80 ---
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 2e1933d..68da9ec 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
@@ -578,9 +579,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
u32 val = 0;
int ret;
 
-   ret = clk_enable(dcmi->mclk);
+   ret = pm_runtime_get_sync(dcmi->dev);
if (ret) {
-   dev_err(dcmi->dev, "%s: Failed to start streaming, cannot 
enable clock\n",
+   dev_err(dcmi->dev, "%s: Failed to start streaming, cannot get 
sync\n",
__func__);
goto err_release_buffers;
}
@@ -590,7 +591,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
if (ret && ret != -ENOIOCTLCMD) {
dev_err(dcmi->dev, "%s: Failed to start streaming, subdev 
streamon error",
__func__);
-   goto err_disable_clock;
+   goto err_pm_put;
}
 
spin_lock_irq(>irqlock);
@@ -675,8 +676,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 err_subdev_streamoff:
v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
 
-err_disable_clock:
-   clk_disable(dcmi->mclk);
+err_pm_put:
+   pm_runtime_put(dcmi->dev);
 
 err_release_buffers:
spin_lock_irq(>irqlock);
@@ -749,7 +750,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
/* Stop all pending DMA operations */
dmaengine_terminate_all(dcmi->dma_chan);
 
-   clk_disable(dcmi->mclk);
+   pm_runtime_put(dcmi->dev);
 
if (dcmi->errors_count)
dev_warn(dcmi->dev, "Some errors found while streaming: 
errors=%d (overrun=%d), buffers=%d\n",
@@ -1751,12 +1752,6 @@ static int dcmi_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
 
-   ret = clk_prepare(mclk);
-   if (ret) {
-   dev_err(>dev, "Unable to prepare mclk %p\n", mclk);
-   goto err_dma_release;
-   }
-
spin_lock_init(>irqlock);
mutex_init(>lock);
init_completion(>complete);
@@ -1772,7 +1767,7 @@ static int dcmi_probe(struct platform_device *pdev)
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
-   goto err_clk_unprepare;
+   goto err_dma_release;
 
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1832,14 +1827,15 @@ static int dcmi_probe(struct platform_device *pdev)
dev_info(>dev, "Probe done\n");
 
platform_set_drvdata(pdev, dcmi);
+
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_device_release:
video_device_release(dcmi->vdev);
 err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_clk_unprepare:
-   clk_unprepare(dcmi->mclk);
 err_dma_release:
dma_release_channel(dcmi->dma_chan);
 
@@ -1850,20 +1846,72 @@ static int dcmi_remove(struct platform_device *pdev)
 {
struct stm32_dcmi *dcmi = platform_get_drvdata(pdev);
 
+   pm_runtime_disable(>dev);
+
v4l2_async_notifier_unregister(>notifier);
v4l2_device_unregister(>v4l2_dev);
-   clk_unprepare(dcmi->mclk);
+
dma_release_channel(dcmi->dma_chan);
 
return 0;
 }
 
+static __maybe_unused int dcmi_runtime_suspend(struct device *dev)
+{
+   struct stm32_dcmi *dcmi = dev_get_drvdata(dev);
+
+   clk_disable_unprepare(dcmi->mclk);
+
+   return 0;
+}
+
+static __maybe_unused int dcmi_runtime_resume(struct device *dev)
+{
+   struct stm32_dcmi *dcmi = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_prepare_enable(dcmi->mclk);
+   if (ret)
+   dev_err(dev, "%s: Failed to prepare_enable clock\n", __func__);
+
+   return ret;
+}
+
+static __maybe_unused int dcmi_suspend(struct device *dev)
+{
+   /* disable clock */
+   pm_runtime_force_suspend(dev);
+
+   /* change pinctrl state */
+   pinctrl_pm_select_sleep_state(dev);
+
+   return 0;
+}
+
+static __maybe_unused int dcmi_resume(struct device *dev)
+{
+   /* restore pinctl default state */
+   pinctrl_pm_select_default_state(dev);
+
+   /* clock enable */
+   pm_runtime_force_resume(dev);
+
+   return 0;
+}
+
+static const struct dev_pm_ops dcmi_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(dcmi_suspend, dcmi_resume)
+   SET_RUNTIME_PM_OPS(dcmi_runtime_suspend,
+  dcmi_runtime_resume, NULL)
+};
+
 static struct platform_driver 

[PATCH 2/2] media: ov5640: add support of module orientation

[Hugues Fruchet]

Add support of module being physically mounted upside down.
In this case, mirror and flip are enabled to fix captured images
orientation.

Signed-off-by: Hugues Fruchet 
---
 .../devicetree/bindings/media/i2c/ov5640.txt   |  3 +++
 drivers/media/i2c/ov5640.c | 28 --
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 8e36da0..f76eb7e 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -13,6 +13,8 @@ Optional Properties:
   This is an active low signal to the OV5640.
 - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
   if any. This is an active high signal to the OV5640.
+- rotation: integer property; valid values are 0 (sensor mounted upright)
+   and 180 (sensor mounted upside down).
 
 The device node must contain one 'port' child node for its digital output
 video port, in accordance with the video interface bindings defined in
@@ -51,6 +53,7 @@ Examples:
DVDD-supply = <_reg>;  /* 1.5v */
powerdown-gpios = < 19 GPIO_ACTIVE_HIGH>;
reset-gpios = < 20 GPIO_ACTIVE_LOW>;
+   rotation = <180>;
 
port {
/* MIPI CSI-2 bus endpoint */
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 41039e5..5529b14 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -215,6 +215,7 @@ struct ov5640_dev {
struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
struct gpio_desc *reset_gpio;
struct gpio_desc *pwdn_gpio;
+   bool   upside_down;
 
/* lock to protect all members below */
struct mutex lock;
@@ -,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev 
*sensor, int value)
 static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
 {
/*
+* If sensor is mounted upside down, mirror logic is inversed.
+*
 * Sensor is a BSI (Back Side Illuminated) one,
 * so image captured is physically mirrored.
 * This is why mirror logic is inversed in
@@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev 
*sensor, int value)
 */
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
  BIT(2) | BIT(1),
- (!value) ? (BIT(2) | BIT(1)) : 0);
+ (!(value ^ sensor->upside_down)) ?
+ (BIT(2) | BIT(1)) : 0);
 }
 
 static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
 {
+   /* If sensor is mounted upside down, flip logic is inversed */
+
/*
 * TIMING TC REG20:
 * - [2]:   ISP vflip
@@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev 
*sensor, int value)
 */
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
  BIT(2) | BIT(1),
- value ? (BIT(2) | BIT(1)) : 0);
+ (value ^ sensor->upside_down) ?
+ (BIT(2) | BIT(1)) : 0);
 }
 
 static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
struct v4l2_mbus_framefmt *fmt;
+   u32 rotation;
int ret;
 
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
 
sensor->ae_target = 52;
 
+   /* optional indication of physical rotation of sensor */
+   ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
+  "rotation", );
+   if (!ret) {
+   switch (rotation) {
+   case 180:
+   sensor->upside_down = true;
+   /* fall through */
+   case 0:
+   break;
+   default:
+   dev_warn(dev, "%u degrees rotation is not supported, 
ignoring...\n",
+rotation);
+   }
+   }
+
endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(>dev),
  NULL);
if (!endpoint) {
-- 
1.9.1

[PATCH 0/2] OV5640 hflip, vflip and module orientation support

[Hugues Fruchet]

This patch serie relates to flip and mirror at sensor level through
registers "TIMING TC REG20/REG21".

The first commit implements HFLIP and VFLIP V4L2 controls
allowing V4L2 client to change horizontal and vertical flip
before or during streaming.

The second commit allows to inform driver of the physical
orientation of the sensor module through devicetree "rotation"
optional properties as defined by Sakari in media/video-interfaces.txt:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg132345.html

Hugues Fruchet (2):
  media: ov5640: add HFLIP/VFLIP controls support
  media: ov5640: add support of module orientation

 .../devicetree/bindings/media/i2c/ov5640.txt   |   3 +
 drivers/media/i2c/ov5640.c | 127 ++---
 2 files changed, 112 insertions(+), 18 deletions(-)

-- 
1.9.1

[PATCH 1/2] media: ov5640: add HFLIP/VFLIP controls support

[Hugues Fruchet]

Add HFLIP/VFLIP controls support by setting registers REG21/REG20.
Useless values in hardcoded mode sequences are removed and
remaining binning values are now set after mode sequence being set.
Note that due to BSI (Back Side Illuminated) technology, image capture
is physically mirrored, mirror logic is so inversed in REG21 register
to cancel this effect.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 103 +
 1 file changed, 85 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f6e40cc..41039e5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -64,6 +64,7 @@
 #define OV5640_REG_TIMING_DVPVO0x380a
 #define OV5640_REG_TIMING_HTS  0x380c
 #define OV5640_REG_TIMING_VTS  0x380e
+#define OV5640_REG_TIMING_TC_REG20 0x3820
 #define OV5640_REG_TIMING_TC_REG21 0x3821
 #define OV5640_REG_AEC_CTRL00  0x3a00
 #define OV5640_REG_AEC_B50_STEP0x3a08
@@ -199,6 +200,8 @@ struct ov5640_ctrls {
struct v4l2_ctrl *contrast;
struct v4l2_ctrl *hue;
struct v4l2_ctrl *test_pattern;
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
 };
 
 struct ov5640_dev {
@@ -341,7 +344,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -360,7 +363,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -379,7 +382,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -399,7 +402,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -418,7 +421,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -437,7 +440,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+   {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 

Re: [PATCH] gpu: ipu-v3: Allow negative offsets for interlaced scanning

[Philipp Zabel]

Hi Steve,

On Sun, 2018-06-10 at 17:08 -0700, Steve Longerbeam wrote:
> Hi Philipp,
> 
> On 06/01/2018 06:13 AM, Philipp Zabel wrote:
> > The IPU also supports interlaced buffers that start with the bottom field.
> > To achieve this, the the base address EBA has to be increased by a stride
> > length and the interlace offset ILO has to be set to the negative stride.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >   drivers/gpu/ipu-v3/ipu-cpmem.c | 8 +++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
> > index 9f2d9ec42add..c1028f38c553 100644
> > --- a/drivers/gpu/ipu-v3/ipu-cpmem.c
> > +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
> > @@ -269,8 +269,14 @@ EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
> >   
> >   void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
> >   {
> > +   u32 ilo;
> > +
> > ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
> > -   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, stride / 8);
> > +   if (stride >= 0)
> > +   ilo = stride / 8;
> > +   else
> > +   ilo = 0x10 - (-stride / 8);
> > +   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
> > ipu_ch_param_write_field(ch, IPU_FIELD_SLY, (stride * 2) - 1);
> 
> This should be "(-stride * 2) - 1" for SLY when stride is negative.
> 
> After fixing that, interweaving seq-bt -> interlaced-bt works fine for
> packed pixel formats,

Ouch, thanks.

>  but there is still some problem for planar.
> I haven't tracked down the issue with planar yet,

Just with the negative stride line? Only for YUV420 or also for NV12?
The problem could be that we also have to change UBO/VBO for planar
formats with a chroma stride (SLUV) that is half the luma stride (SLY)
when we move both Y and U/V frame start by a line length.

>  but the corresponding
> changes to imx-media that allow interweaving with line swapping are at
> 
> e9dd81da20 ("media: imx: Allow interweave with top/bottom lines swapped")
> 
> in branch fix-csi-interlaced.3 in my media-tree fork on github. Please 
> have a
> look and let me know if you see anything obvious.

In commit a19126a80d13 ("gpu: ipu-csi: Swap fields according to
input/output field types"), swap_fields = true is only set for
seq-bt -> seq-tb and seq-tb -> seq-bt. I think it should also be
enabled for alternate -> seq-bt (PAL) and alternate -> seq-tb (NTSC).

I've been made aware [1] that recently V4L2_FIELD_ALTERNATE has been
clarified [2] to specify that v4l2_mbus_fmt.height should contain the
number of lines per field, not per frame:

[1] https://patchwork.linuxtv.org/patch/50166/
[2] 0018147c964e ("media: v4l: doc: Clarify v4l2_mbus_fmt height
definition")

regards
Philipp

Re: [PATCH v7 0/6] Add ChromeOS EC CEC Support

[Neil Armstrong]

Hi Lee,

On 11/06/2018 08:03, Lee Jones wrote:
> On Fri, 08 Jun 2018, Hans Verkuil wrote:
>> On 08/06/18 10:17, Neil Armstrong wrote:
>>> On 08/06/2018 09:53, Hans Verkuil wrote:
 On 06/01/2018 10:19 AM, Neil Armstrong wrote:
> Hi All,
>
> The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support
> through it's Embedded Controller, to enable the Linux CEC Core to 
> communicate
> with it and get the CEC Physical Address from the correct HDMI Connector, 
> the
> following must be added/changed:
> - Add the CEC sub-device registration in the ChromeOS EC MFD Driver
> - Add the CEC related commands and events definitions into the EC MFD 
> driver
> - Add a way to get a CEC notifier with it's (optional) connector name
> - Add the CEC notifier to the i915 HDMI driver
> - Add the proper ChromeOS EC CEC Driver
>
> The CEC notifier with the connector name is the tricky point, since even 
> on
> Device-Tree platforms, there is no way to distinguish between multiple 
> HDMI
> connectors from the same DRM driver. The solution I implemented is pretty
> simple and only adds an optional connector name to eventually distinguish
> an HDMI connector notifier from another if they share the same device.

 This looks good to me, which brings me to the next question: how to merge
 this?

 It touches on three subsystems (media, drm, mfd), so that makes this
 tricky.

 I think there are two options: either the whole series goes through the
 media tree, or patches 1+2 go through drm and 3-6 through media. If there
 is a high chance of conflicts in the mfd code, then it is also an option to
 have patches 3-6 go through the mfd subsystem.
>>>
>>> I think patches 3-6 should go in the mfd tree, Lee is used to handle this,
>>> then I think the rest could go in the media tree.
>>>
>>> Lee, do you think it would be possible to have an immutable branch with 
>>> patches 3-6 ?
>>>
>>> Could we have an immutable branch from media tree with patch 1 to be merged 
>>> in
>>> the i915 tree for patch 2 ?
>>>
>>> Or patch 1+2 could me merged into the i915 tree and generate an immutable 
>>> branch
>>
>> I think patches 1+2 can just go to the i915 tree. The i915 driver changes 
>> often,
>> so going through that tree makes sense. The cec-notifier code is unlikely to 
>> change,
>> and I am fine with that patch going through i915.
>>
>>> for media to merge the mfd branch + patch 7 ?
>>
>> Patch 7? I only count 6?
>>
>> If 1+2 go through drm and 3-6 go through mfd, then media isn't affected at 
>> all.
>> There is chance of a conflict when this is eventually pushed to mainline for
>> the media Kconfig, but that's all.
> 
> What are the *build* dependencies within the set?

Here are the hard the build dependency :

Patch 2 depends on Patch 1
Patch 5 depends on Patch 4
Patch 6 depends on Patches 1 & 4

> 
> I'd be happy to send out a pull-request for either all of the patches,
> or just the MFD changes once I've had chance to review them.
> 

Great, thanks !

Neil

[PATCH] media: bt8xx: bttv: fix spelling mistake: "culpit" -> "culprit"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in pr_notice message text

Signed-off-by: Colin Ian King 
---
 drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c 
b/drivers/media/pci/bt8xx/bttv-driver.c
index de3f44b8dec6..cf05e11da01b 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3511,7 +3511,7 @@ static void bttv_irq_debug_low_latency(struct bttv *btv, 
u32 rc)
}
pr_notice("%d: Uhm. Looks like we have unusual high IRQ latencies\n",
  btv->c.nr);
-   pr_notice("%d: Lets try to catch the culpit red-handed ...\n",
+   pr_notice("%d: Lets try to catch the culprit red-handed ...\n",
  btv->c.nr);
dump_stack();
 }
-- 
2.17.0

Re: [RFC PATCH 2/2] media: docs-rst: Add encoder UAPI specification to Codec Interfaces

[Tomasz Figa]

Hi Hans,

On Thu, Jun 7, 2018 at 6:21 PM Hans Verkuil  wrote:
>
> On 06/05/2018 12:33 PM, Tomasz Figa wrote:
[snip]
> > +Initialization
> > +--
> > +
> > +1. (optional) Enumerate supported formats and resolutions. See
> > +   capability enumeration.
> > +
> > +2. Set a coded format on the CAPTURE queue via :c:func:`VIDIOC_S_FMT`
> > +
> > +   a. Required fields:
> > +
> > +  i.  type = CAPTURE
> > +
> > +  ii. fmt.pix_mp.pixelformat set to a coded format to be produced
> > +
> > +   b. Return values:
> > +
> > +  i.  EINVAL: unsupported format.
>
> I'm still not sure about returning an error in this case.
>
> And what should TRY_FMT do?
>
> Do you know what current codecs do? Return EINVAL or replace with a supported 
> format?
>

s5p-mfc returns -EINVAL, while mtk-vcodec and coda seem to fall back
to current format.

> It would be nice to standardize on one rule or another.
>
> The spec says that it should always return a valid format, but not all 
> drivers adhere
> to that. Perhaps we need to add a flag to let the driver signal the behavior 
> of S_FMT
> to userspace.
>
> This is a long-standing issue with S_FMT, actually.
>

Agreed. I'd personally prefer agreeing on one pattern to simplify
things. I generally don't like the "negotiation hell" that the
fallback introduces, but with the general documentation clearly
stating such behavior, I'd be worried that returning an error actually
breaks some userspace.

[snip]
> > +Encoding
> > +
> > +
> > +This state is reached after a successful initialization sequence. In
> > +this state, client queues and dequeues buffers to both queues via
> > +:c:func:`VIDIOC_QBUF` and :c:func:`VIDIOC_DQBUF`, as per spec.
> > +
> > +Both queues operate independently. The client may queue and dequeue
> > +buffers to queues in any order and at any rate, also at a rate different
> > +for each queue. The client may queue buffers within the same queue in
> > +any order (V4L2 index-wise).
>
> I'd drop the whole 'in any order' in the text above. This has always been
> the case, and I think it is only confusing. I think what you really want
> to say is that both queues operate independently and quite possibly at
> different rates. So clients should operate them independently as well.

I think there are at least 3 different "in any order" in play:

1) the order of buffers being dequeued not matching the order of
queuing the buffers in the same queue (e.g. encoder keeping some
buffers as reference framebuffers)

2) possible difference in order of queuing raw frames to encoder
OUTPUT and dequeuing encoded bitstream from encoder CAPTURE,

3) the order of handling the queue/dequeue operations on both queues,
i.e. dequeue OUTPUT, queue OUTPUT, dequeue CAPTURE, queue CAPTURE,

4) the order of queuing buffers (indices) within the queue being up to
the client - this has always been the case indeed. The extra bit here
is that this keeps being true, even with having 2 queues.

I believe the text above refers to 3) and 4). I guess we can drop 4)
indeed and we actually may want to clearly state 1) and 2).

By the way, do we already have some place in the documentation that
mentions the bits that have always been the case? We could refer to it
instead.

>
>  It is recommended for the client to operate
> > +the queues independently for best performance.
> > +
> > +Source OUTPUT buffers must contain full raw frames in the selected
> > +OUTPUT format, exactly one frame per buffer.
> > +
> > +Encoding parameter changes
> > +--
> > +
> > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > +parameters at any time. The driver must apply the new setting starting
> > +at the next frame queued to it.
> > +
> > +This specifically means that if the driver maintains a queue of buffers
> > +to be encoded and at the time of the call to :c:func:`VIDIOC_S_CTRL` not 
> > all the
> > +buffers in the queue are processed yet, the driver must not apply the
> > +change immediately, but schedule it for when the next buffer queued
> > +after the :c:func:`VIDIOC_S_CTRL` starts being processed.
>
> Is this what drivers do today? I thought it was applied immediately?
> This sounds like something for which you need the Request API.

mtk-vcodec seems to implement the above, while s5p-mfc, coda, venus
don't seem to be doing so.

> > +
> > +Flush
> > +-
> > +
> > +Flush is the process of draining the CAPTURE queue of any remaining
> > +buffers. After the flush sequence is complete, the client has received
> > +all encoded frames for all OUTPUT buffers queued before the sequence was
> > +started.
> > +
> > +1. Begin flush by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > +
> > +   a. Required fields:
> > +
> > +  i. cmd = ``V4L2_ENC_CMD_STOP``
> > +
> > +2. The driver must process and encode as normal all OUTPUT buffers
> > +   queued by the client before the :c:func:`VIDIOC_ENCODER_CMD` was issued.
>
> Note: TRY_ENCODER_CMD should also be 

[PATCH] media: cx88: add error handling for snd_ctl_add

[Zhouyang Jia]

When snd_ctl_add fails, the lack of error-handling code may
cause unexpected results.

This patch adds error-handling code after calling snd_ctl_add.

Signed-off-by: Zhouyang Jia 
---
 drivers/media/pci/cx88/cx88-alsa.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 8a28fda..d366793 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -962,8 +962,11 @@ static int cx88_audio_initdev(struct pci_dev *pci,
goto error;
 
/* If there's a wm8775 then add a Line-In ALC switch */
-   if (core->sd_wm8775)
-   snd_ctl_add(card, snd_ctl_new1(_cx88_alc_switch, chip));
+   if (core->sd_wm8775) {
+   err = snd_ctl_add(card, snd_ctl_new1(_cx88_alc_switch, 
chip));
+   if (err < 0)
+   goto error;
+   }
 
strcpy(card->driver, "CX88x");
sprintf(card->shortname, "Conexant CX%x", pci->device);
-- 
2.7.4

Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile

[Hans Verkuil]

On 11/06/18 08:44, Keiichi Watanabe wrote:
> Hi, Hans.
> Thank you for the review.
> Your idea sounds good.
> 
> However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
> breaks both of s5p-mfc and venus drivers.  This is because they call
> 'v4l2_ctrl_new_std' for it.  For menu controls,
> 'v4l2_ctrl_new_std_menu' must be used.
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133
> 
> I can fix them within the commit for adding VP8_PROFILE control, but
> cannot confirm that it works on real devices since I don't have them.
> What should I do?

Fix it in both drivers with a Cc to stanimir.varba...@linaro.org (venus) and
s.nawro...@samsung.com (s5p) to let them test/ack the patch.

Venus is no problem, the s5p driver can decide to keep the old INT control
since it has been in use for such a long time, but that is up to Sylwester to
decide.

Regards,

Hans

> 
> Best regards,
> Keiichi
> On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne  wrote:
>>
>>
>>
>> Le ven. 8 juin 2018 08:56, Stanimir Varbanov  
>> a écrit :
>>>
>>> Hi Hans,
>>>
>>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
 On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> or decoder.
>
> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> used for querying since it is not a menu control but an integer
> control, which cannot return an arbitrary set of supported profiles.
>
> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> with controls for other codec profiles. (e.g. H264)

 Please ignore my reply to patch 2/2. I looked at this a bit more and what 
 you
 should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.

 All other codec profile controls are all enums, so the fact that 
 VPX_PROFILE
 isn't is a bug. Changing the type should not cause any problems since the 
 same
 control value is used when you set the control.

 Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
 as an integer type control, so the s5p-mfc driver should not be affected by
 changing the type of this control.

 Stanimir: this will slightly change the venus driver, but since it is a 
 very
 recent driver I think we can get away with changing the core type of the
 VPX_PROFILE control. I think that's better than ending up with two controls
 that do the same thing.
>>>
>>> I agree. Actually the changes shouldn't be so much in venus driver.
>>
>>
>> Also fine on userspace side, since profiles enumeration isn't implemented 
>> yet in FFMPEG, GStreamer, Chrome
>>
>>
>>>
>>> --
>>> regards,
>>> Stan

Re: [PATCH -next] media/platform/cadence: add to fix build error

[Maxime Ripard]

On Fri, Jun 08, 2018 at 02:19:06PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> Add #include  to fix build errors.
> This driver uses kzalloc() and kfree() so it needs to #include
> the appropriate header file for those interfaces.
> 
> Fixes these build errors:
> 
> ../drivers/media/platform/cadence/cdns-csi2rx.c: In function 'csi2rx_probe':
> ../drivers/media/platform/cadence/cdns-csi2rx.c:421:2: error: implicit 
> declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
>   csi2rx = kzalloc(sizeof(*csi2rx), GFP_KERNEL);
> ../drivers/media/platform/cadence/cdns-csi2rx.c:421:9: warning: assignment 
> makes pointer from integer without a cast [enabled by default]
>   csi2rx = kzalloc(sizeof(*csi2rx), GFP_KERNEL);
> ../drivers/media/platform/cadence/cdns-csi2rx.c:466:2: error: implicit 
> declaration of function 'kfree' [-Werror=implicit-function-declaration]
>   kfree(csi2rx);
> 
> Signed-off-by: Randy Dunlap 
> Cc: Maxime Ripard 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile

[Keiichi Watanabe]

Hi, Hans.
Thank you for the review.
Your idea sounds good.

However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
breaks both of s5p-mfc and venus drivers.  This is because they call
'v4l2_ctrl_new_std' for it.  For menu controls,
'v4l2_ctrl_new_std_menu' must be used.

https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133

I can fix them within the commit for adding VP8_PROFILE control, but
cannot confirm that it works on real devices since I don't have them.
What should I do?

Best regards,
Keiichi
On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne  wrote:
>
>
>
> Le ven. 8 juin 2018 08:56, Stanimir Varbanov  a 
> écrit :
>>
>> Hi Hans,
>>
>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
>> > On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>> >> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>> >> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>> >> or decoder.
>> >>
>> >> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>> >> used for querying since it is not a menu control but an integer
>> >> control, which cannot return an arbitrary set of supported profiles.
>> >>
>> >> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>> >> with controls for other codec profiles. (e.g. H264)
>> >
>> > Please ignore my reply to patch 2/2. I looked at this a bit more and what 
>> > you
>> > should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>> >
>> > All other codec profile controls are all enums, so the fact that 
>> > VPX_PROFILE
>> > isn't is a bug. Changing the type should not cause any problems since the 
>> > same
>> > control value is used when you set the control.
>> >
>> > Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> > as an integer type control, so the s5p-mfc driver should not be affected by
>> > changing the type of this control.
>> >
>> > Stanimir: this will slightly change the venus driver, but since it is a 
>> > very
>> > recent driver I think we can get away with changing the core type of the
>> > VPX_PROFILE control. I think that's better than ending up with two controls
>> > that do the same thing.
>>
>> I agree. Actually the changes shouldn't be so much in venus driver.
>
>
> Also fine on userspace side, since profiles enumeration isn't implemented yet 
> in FFMPEG, GStreamer, Chrome
>
>
>>
>> --
>> regards,
>> Stan

Re: [PATCH v7 0/6] Add ChromeOS EC CEC Support

[Lee Jones]

On Fri, 08 Jun 2018, Hans Verkuil wrote:
> On 08/06/18 10:17, Neil Armstrong wrote:
> > On 08/06/2018 09:53, Hans Verkuil wrote:
> >> On 06/01/2018 10:19 AM, Neil Armstrong wrote:
> >>> Hi All,
> >>>
> >>> The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support
> >>> through it's Embedded Controller, to enable the Linux CEC Core to 
> >>> communicate
> >>> with it and get the CEC Physical Address from the correct HDMI Connector, 
> >>> the
> >>> following must be added/changed:
> >>> - Add the CEC sub-device registration in the ChromeOS EC MFD Driver
> >>> - Add the CEC related commands and events definitions into the EC MFD 
> >>> driver
> >>> - Add a way to get a CEC notifier with it's (optional) connector name
> >>> - Add the CEC notifier to the i915 HDMI driver
> >>> - Add the proper ChromeOS EC CEC Driver
> >>>
> >>> The CEC notifier with the connector name is the tricky point, since even 
> >>> on
> >>> Device-Tree platforms, there is no way to distinguish between multiple 
> >>> HDMI
> >>> connectors from the same DRM driver. The solution I implemented is pretty
> >>> simple and only adds an optional connector name to eventually distinguish
> >>> an HDMI connector notifier from another if they share the same device.
> >>
> >> This looks good to me, which brings me to the next question: how to merge
> >> this?
> >>
> >> It touches on three subsystems (media, drm, mfd), so that makes this
> >> tricky.
> >>
> >> I think there are two options: either the whole series goes through the
> >> media tree, or patches 1+2 go through drm and 3-6 through media. If there
> >> is a high chance of conflicts in the mfd code, then it is also an option to
> >> have patches 3-6 go through the mfd subsystem.
> > 
> > I think patches 3-6 should go in the mfd tree, Lee is used to handle this,
> > then I think the rest could go in the media tree.
> > 
> > Lee, do you think it would be possible to have an immutable branch with 
> > patches 3-6 ?
> > 
> > Could we have an immutable branch from media tree with patch 1 to be merged 
> > in
> > the i915 tree for patch 2 ?
> > 
> > Or patch 1+2 could me merged into the i915 tree and generate an immutable 
> > branch
> 
> I think patches 1+2 can just go to the i915 tree. The i915 driver changes 
> often,
> so going through that tree makes sense. The cec-notifier code is unlikely to 
> change,
> and I am fine with that patch going through i915.
> 
> > for media to merge the mfd branch + patch 7 ?
> 
> Patch 7? I only count 6?
> 
> If 1+2 go through drm and 3-6 go through mfd, then media isn't affected at 
> all.
> There is chance of a conflict when this is eventually pushed to mainline for
> the media Kconfig, but that's all.

What are the *build* dependencies within the set?

I'd be happy to send out a pull-request for either all of the patches,
or just the MFD changes once I've had chance to review them.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog