cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Thu Mar  1 05:00:11 CET 2018
media-tree git hash:e3e389f931a14ddf43089c7db92fc5d74edf93a4
media_build git hash:   c3a4fa1a633e24b4a607a78ad11a61598ee177b6
v4l-utils git hash: 200338d272a908be4d98c0127765a8e1611be639
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.98-i686: ERRORS
linux-3.2.98-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-i686: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.53-i686: ERRORS
linux-3.16.53-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.93-i686: ERRORS
linux-3.18.93-x86_64: ERRORS
linux-3.19-i686: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.49-i686: ERRORS
linux-4.1.49-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.115-i686: OK
linux-4.4.115-x86_64: OK
linux-4.5.7-i686: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-i686: OK
linux-4.7.5-x86_64: WARNINGS
linux-4.8-i686: OK
linux-4.8-x86_64: WARNINGS
linux-4.9.80-i686: OK
linux-4.9.80-x86_64: OK
linux-4.10.14-i686: OK
linux-4.10.14-x86_64: WARNINGS
linux-4.11-i686: OK
linux-4.11-x86_64: WARNINGS
linux-4.12.1-i686: OK
linux-4.12.1-x86_64: WARNINGS
linux-4.13-i686: OK
linux-4.13-x86_64: OK
linux-4.14.17-i686: OK
linux-4.14.17-x86_64: OK
linux-4.15.2-i686: OK
linux-4.15.2-x86_64: OK
linux-4.16-rc1-i686: OK
linux-4.16-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v4 2/2] media: video-i2c: add video-i2c driver

[kbuild test robot]

Hi Matt,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.16-rc3 next-20180228]
[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/Matt-Ranostay/media-video-i2c-add-video-i2c-driver-support/20180301-111038
base:   git://linuxtv.org/media_tree.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 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
make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   drivers/media//i2c/video-i2c.c: In function 'video_i2c_probe':
>> drivers/media//i2c/video-i2c.c:456:13: warning: cast from pointer to integer 
>> of different size [-Wpointer-to-int-cast]
  chip_id = (int) of_device_get_match_data(>dev);
^

vim +456 drivers/media//i2c/video-i2c.c

   442  
   443  static int video_i2c_probe(struct i2c_client *client,
   444   const struct i2c_device_id *id)
   445  {
   446  struct video_i2c_data *data;
   447  struct v4l2_device *v4l2_dev;
   448  struct vb2_queue *queue;
   449  int chip_id, ret;
   450  
   451  data = kzalloc(sizeof(*data), GFP_KERNEL);
   452  if (!data)
   453  return -ENOMEM;
   454  
   455  if (client->dev.of_node)
 > 456  chip_id = (int) of_device_get_match_data(>dev);
   457  else
   458  chip_id = id->driver_data;
   459  
   460  data->chip = _i2c_chip[chip_id];
   461  data->client = client;
   462  v4l2_dev = >v4l2_dev;
   463  strlcpy(v4l2_dev->name, VIDEO_I2C_DRIVER, 
sizeof(v4l2_dev->name));
   464  
   465  ret = v4l2_device_register(>dev, v4l2_dev);
   466  if (ret < 0)
   467  goto error_free_device;
   468  
   469  mutex_init(>lock);
   470  mutex_init(>queue_lock);
   471  
   472  queue = >vb_vidq;
   473  queue->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   474  queue->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR | 
VB2_READ;
   475  queue->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
   476  queue->drv_priv = data;
   477  queue->buf_struct_size = sizeof(struct video_i2c_buffer);
   478  queue->min_buffers_needed = 1;
   479  queue->ops = _i2c_video_qops;
   480  queue->mem_ops = _vmalloc_memops;
   481  
   482  ret = vb2_queue_init(queue);
   483  if (ret < 0)
   484  goto error_unregister_device;
   485  
   486  data->vdev.queue = queue;
   487  data->vdev.queue->lock = >queue_lock;
   488  
   489  snprintf(data->vdev.name, sizeof(data->vdev.name),
   490   "I2C %d-%d Transport Video",
   491   client->adapter->nr, client->addr);
   492  
   493  data->vdev.v4l2_dev = v4l2_dev;
   494  data->vdev.fops = _i2c_fops;
   495  data->vdev.lock = >lock;
   496  data->vdev.ioctl_ops = _i2c_ioctl_ops;
   497  data->vdev.release = video_i2c_release;
   498  data->vdev.device_caps = V4L2_CAP_VIDEO_CAPTURE |
   499   V4L2_CAP_READWRITE | 
V4L2_CAP_STREAMING;
   500  
   501  spin_lock_init(>slock);
   502  INIT_LIST_HEAD(>vid_cap_active);
   503  
   504  video_set_drvdata(>vdev, data);
   505  i2c_set_clientdata(client, data);
   506  
   507  ret = video_register_device(>vdev, VFL_TYPE_GRABBER, -1);
   508  if (ret < 0)
   509  goto error_unregister_device;
   510  
   511  return 0;
   512  
   513  error_unregister_device:
   514  v4l2_device_unregister(v4l2_dev);
   515  
   516  error_free_device:
   517  kfree(data);
   518  
   519  return ret;
   520  }
   521  

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


.config.gz
Description: application/gzip

[PATCH] staging/imx: Fix inconsistent IS_ERR and PTR_ERR

[Gustavo A. R. Silva]

Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.

This issue was detected with the help of Coccinelle.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..4f290a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
-   ret = PTR_ERR(priv->vdev);
+   ret = PTR_ERR(pinctrl);
goto free;
}
 
-- 
2.7.4

[PATCH] media: renesas-ceu: mark PM functions as __maybe_unused

[Arnd Bergmann]

The PM runtime operations are unused when CONFIG_PM is disabled,
leading to a harmless warning:

drivers/media/platform/renesas-ceu.c:1003:12: error: 'ceu_runtime_suspend' 
defined but not used [-Werror=unused-function]
 static int ceu_runtime_suspend(struct device *dev)
^~~
drivers/media/platform/renesas-ceu.c:987:12: error: 'ceu_runtime_resume' 
defined but not used [-Werror=unused-function]
 static int ceu_runtime_resume(struct device *dev)
^~

This adds a __maybe_unused annotation to shut up the warning.

Fixes: 32e5a70dc8f4 ("media: platform: Add Renesas CEU driver")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/renesas-ceu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
index 22330c0c2f6a..eccd60a7ebec 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -984,7 +984,7 @@ static int ceu_init_mbus_fmt(struct ceu_device *ceudev)
 /*
  * ceu_runtime_resume() - soft-reset the interface and turn sensor power on.
  */
-static int ceu_runtime_resume(struct device *dev)
+static int __maybe_unused ceu_runtime_resume(struct device *dev)
 {
struct ceu_device *ceudev = dev_get_drvdata(dev);
struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
@@ -1000,7 +1000,7 @@ static int ceu_runtime_resume(struct device *dev)
  * ceu_runtime_suspend() - disable capture and interrupts and soft-reset.
  *Turn sensor power off.
  */
-static int ceu_runtime_suspend(struct device *dev)
+static int __maybe_unused ceu_runtime_suspend(struct device *dev)
 {
struct ceu_device *ceudev = dev_get_drvdata(dev);
struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
-- 
2.9.0

[PATCH 3/3] media: vb2-core: vb2_ops: document non-interrupt-cantext calling

[Luca Ceresoli]

Driver writers can benefit in knowing if/when callbacks are called in
interrupt context. But it is not completely obvious here, so document it.

Signed-off-by: Luca Ceresoli 
Cc: Laurent Pinchart 
Cc: Pawel Osciak 
Cc: Marek Szyprowski 
Cc: Kyungmin Park 
Cc: Mauro Carvalho Chehab 
---
 include/media/videobuf2-core.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f2887d3c..f6818f732f34 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -296,6 +296,9 @@ struct vb2_buffer {
 /**
  * struct vb2_ops - driver-specific callbacks.
  *
+ * These operations are not called from interrupt context except where
+ * mentioned specifically.
+ *
  * @queue_setup:   called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
  * handlers before memory allocation. It can be called
  * twice: if the original number of requested buffers
-- 
2.7.4

[PATCH 2/3] media: vb2-core: document the REQUEUEING state

[Luca Ceresoli]

VB2_BUF_STATE_REQUEUEING is accepted by vb2_buffer_done() but not
documented, so add it along with notes about calls in interrupt context.

Signed-off-by: Luca Ceresoli 
Cc: Laurent Pinchart 
Cc: Pawel Osciak 
Cc: Marek Szyprowski 
Cc: Kyungmin Park 
Cc: Mauro Carvalho Chehab 
---
 include/media/videobuf2-core.h | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f1a479060f9e..f2887d3c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -358,12 +358,12 @@ struct vb2_buffer {
  * driver can return an error if hardware fails, in that
  * case all buffers that have been already given by
  * the @buf_queue callback are to be returned by the driver
- * by calling vb2_buffer_done() with %VB2_BUF_STATE_QUEUED.
- * If you need a minimum number of buffers before you can
- * start streaming, then set
- * _queue->min_buffers_needed. If that is non-zero then
- * @start_streaming won't be called until at least that
- * many buffers have been queued up by userspace.
+ * by calling vb2_buffer_done() with %VB2_BUF_STATE_QUEUED
+ * or %VB2_BUF_STATE_REQUEUEING. If you need a minimum
+ * number of buffers before you can start streaming, then
+ * set _queue->min_buffers_needed. If that is non-zero
+ * then @start_streaming won't be called until at least
+ * that many buffers have been queued up by userspace.
  * @stop_streaming:called when 'streaming' state must be disabled; driver
  * should stop any DMA transactions or wait until they
  * finish and give back all buffers it got from _queue
@@ -601,8 +601,9 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int 
plane_no);
  * @state: state of the buffer, as defined by  vb2_buffer_state.
  * Either %VB2_BUF_STATE_DONE if the operation finished
  * successfully, %VB2_BUF_STATE_ERROR if the operation finished
- * with an error or %VB2_BUF_STATE_QUEUED if the driver wants to
- * requeue buffers.
+ * with an error or any of %VB2_BUF_STATE_QUEUED or
+ * %VB2_BUF_STATE_REQUEUEING if the driver wants to
+ * requeue buffers (see below).
  *
  * This function should be called by the driver after a hardware operation on
  * a buffer is finished and the buffer may be returned to userspace. The driver
@@ -613,7 +614,12 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int 
plane_no);
  * While streaming a buffer can only be returned in state DONE or ERROR.
  * The _ops->start_streaming op can also return them in case the DMA engine
  * cannot be started for some reason. In that case the buffers should be
- * returned with state QUEUED to put them back into the queue.
+ * returned with state QUEUED or REQUEUEING to put them back into the queue.
+ *
+ * %VB2_BUF_STATE_REQUEUEING is like %VB2_BUF_STATE_QUEUED, but it also calls
+ * _ops->buf_queue to queue buffers back to the driver. Note that calling
+ * vb2_buffer_done(..., VB2_BUF_STATE_REQUEUEING) from interrupt context will
+ * result in _ops->buf_queue being called in interrupt context as well.
  */
 void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state);
 
-- 
2.7.4

[PATCH 1/3] media: vb2-core: vb2_buffer_done: consolidate docs

[Luca Ceresoli]

Documentation about what start_streaming() should do on failure are
scattered in two places and mostly duplicated, so consolidate them in
one of the two places.

Signed-off-by: Luca Ceresoli 
Cc: Laurent Pinchart 
Cc: Pawel Osciak 
Cc: Marek Szyprowski 
Cc: Kyungmin Park 
Cc: Mauro Carvalho Chehab 
---
 include/media/videobuf2-core.h | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5b6c541e4e1b..f1a479060f9e 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -602,9 +602,7 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int 
plane_no);
  * Either %VB2_BUF_STATE_DONE if the operation finished
  * successfully, %VB2_BUF_STATE_ERROR if the operation finished
  * with an error or %VB2_BUF_STATE_QUEUED if the driver wants to
- * requeue buffers. If start_streaming fails then it should return
- * buffers with state %VB2_BUF_STATE_QUEUED to put them back into
- * the queue.
+ * requeue buffers.
  *
  * This function should be called by the driver after a hardware operation on
  * a buffer is finished and the buffer may be returned to userspace. The driver
@@ -613,9 +611,9 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int 
plane_no);
  * to the driver by _ops->buf_queue can be passed to this function.
  *
  * While streaming a buffer can only be returned in state DONE or ERROR.
- * The start_streaming op can also return them in case the DMA engine cannot
- * be started for some reason. In that case the buffers should be returned with
- * state QUEUED.
+ * The _ops->start_streaming op can also return them in case the DMA engine
+ * cannot be started for some reason. In that case the buffers should be
+ * returned with state QUEUED to put them back into the queue.
  */
 void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state);
 
-- 
2.7.4

Re: [PATCH v2 5/8] v4l: vsp1: Refactor display list configure operations

[Laurent Pinchart]

Hi Kieran,

On Wednesday, 28 February 2018 18:41:31 EET Kieran Bingham wrote:
> Hi Laurent,
> 
> This series has a pending question below:
> 
> On 17/11/17 15:07, Kieran Bingham wrote:
> > Hi Laurent,
> > 
> > Just a query on your bikeshedding here.
> > 
> > Choose your colours wisely :)
> > 
> > On 12/09/17 20:19, Laurent Pinchart wrote:
> >> On Tuesday, 12 September 2017 00:16:50 EEST Kieran Bingham wrote:
> >>> On 17/08/17 19:13, Laurent Pinchart wrote:
>  On Monday 14 Aug 2017 16:13:28 Kieran Bingham wrote:
> > The entities provide a single .configure operation which configures
> > the object into the target display list, based on the
> > vsp1_entity_params selection.
> > 
> > This restricts us to a single function prototype for both static
> > configuration (the pre-stream INIT stage) and the dynamic runtime
> > stages for both each frame - and each partition therein.
> > 
> > Split the configure function into two parts, '.prepare()' and
> > '.configure()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
> > VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
> > .configure(). The configuration for individual partitions is handled
> > by passing the partition number to the configure call, and processing
> > any runtime stage actions on the first partition only.
> > 
> > Signed-off-by: Kieran Bingham
> > 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
> >  drivers/media/platform/vsp1/vsp1_clu.c|  43 +--
> >  drivers/media/platform/vsp1/vsp1_drm.c|  11 +-
> >  drivers/media/platform/vsp1/vsp1_entity.c |  15 +-
> >  drivers/media/platform/vsp1/vsp1_entity.h |  27 +--
> >  drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
> >  drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
> >  drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
> >  drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
> >  drivers/media/platform/vsp1/vsp1_lut.c|  24 +-
> >  drivers/media/platform/vsp1/vsp1_rpf.c| 162 ++---
> >  drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
> >  drivers/media/platform/vsp1/vsp1_uds.c|  55 ++--
> >  drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
> >  drivers/media/platform/vsp1/vsp1_wpf.c| 297 +++--
> >  15 files changed, 359 insertions(+), 371 deletions(-)
>  
>  [snip]
>  
> > diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> > b/drivers/media/platform/vsp1/vsp1_clu.c index
> > 175717018e11..5f65ce3ad97f
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_clu.c
> > +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> > @@ -213,37 +213,37 @@ static const struct v4l2_subdev_ops clu_ops = {
> >  /* --
> >   * VSP1 Entity Operations
> >   */
> > +static void clu_prepare(struct vsp1_entity *entity,
> > +   struct vsp1_pipeline *pipe,
> > +   struct vsp1_dl_list *dl)
> > +{
> > +   struct vsp1_clu *clu = to_clu(>subdev);
> > +
> > +   /*
> > +* The format can't be changed during streaming, only verify it
> > +* at setup time and store the information internally for future
> > +* runtime configuration calls.
> > +*/
>  
>  I know you're just moving the comment around, but let's fix it at the
>  same time. There's no verification here (and no "setup time" either).
>  I'd write it as
>  
>   /*
>    * The format can't be changed during streaming. Cache it internally
>    * for future runtime configuration calls.
>    */
> >>> 
> >>> I think I'm ok with that and I've updated the patch - but I'm not sure
> >>> we are really caching the 'format' here, as much as the yuv_mode ...
> >> 
> >> Yes, it's the YUV mode we're caching, feel free to update the comment.
> > 
> > Done.
> > 
> >>> I'll ponder ...
> >>> 
> > +   struct v4l2_mbus_framefmt *format;
> > +
> > +   format = vsp1_entity_get_pad_format(>entity,
> > +   clu->entity.config,
> > +   CLU_PAD_SINK);
> > +   clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
> > +}
>  
>  [snip]
>  
> > diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
> > b/drivers/media/platform/vsp1/vsp1_entity.h index
> > 408602ebeb97..2f33e343ccc6 100644
> > --- a/drivers/media/platform/vsp1/vsp1_entity.h
> > +++ b/drivers/media/platform/vsp1/vsp1_entity.h
>  
>  [snip]
>  
> > @@ -80,8 +68,10 @@ struct vsp1_route {
> >  /**
> >   * struct vsp1_entity_operations - Entity operations
> >   * @destroy:   Destroy the entity.
> > - * @configure: Setup 

[PATCH v6 1/9] v4l: vsp1: Reword uses of 'fragment' as 'body'

[Kieran Bingham]

Throughout the codebase, the term 'fragment' is used to represent a
display list body. This term duplicates the 'body' which is already in
use.

The datasheet references these objects as a body, therefore replace all
mentions of a fragment with a body, along with the corresponding
pluralised terms.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_clu.c |  10 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 107 --
 drivers/media/platform/vsp1/vsp1_dl.h  |  14 +--
 drivers/media/platform/vsp1/vsp1_lut.c |   8 +-
 4 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index f2fb26e5ab4e..9621afa3658c 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -47,19 +47,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_fragment_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
if (!dlb)
return -ENOMEM;
 
-   vsp1_dl_fragment_write(dlb, VI6_CLU_ADDR, 0);
+   vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
for (i = 0; i < 17 * 17 * 17; ++i)
-   vsp1_dl_fragment_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
+   vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_fragment_free(dlb);
+   vsp1_dl_body_free(dlb);
return 0;
 }
 
@@ -256,7 +256,7 @@ static void clu_configure(struct vsp1_entity *entity,
spin_unlock_irqrestore(>lock, flags);
 
if (dlb)
-   vsp1_dl_list_add_fragment(dl, dlb);
+   vsp1_dl_list_add_body(dl, dlb);
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0b86ed01e85d..0c1bd17f9281 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -69,7 +69,7 @@ struct vsp1_dl_body {
  * @header: display list header, NULL for headerless lists
  * @dma: DMA address for the header
  * @body0: first display list body
- * @fragments: list of extra display list bodies
+ * @bodies: list of extra display list bodies
  * @has_chain: if true, indicates that there's a partition chain
  * @chain: entry in the display list partition chain
  */
@@ -81,7 +81,7 @@ struct vsp1_dl_list {
dma_addr_t dma;
 
struct vsp1_dl_body body0;
-   struct list_head fragments;
+   struct list_head bodies;
 
bool has_chain;
struct list_head chain;
@@ -98,13 +98,13 @@ enum vsp1_dl_mode {
  * @mode: display list operation mode (header or headerless)
  * @singleshot: execute the display list in single-shot mode
  * @vsp1: the VSP1 device
- * @lock: protects the free, active, queued, pending and gc_fragments lists
+ * @lock: protects the free, active, queued, pending and gc_bodies lists
  * @free: array of all free display lists
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
- * @gc_work: fragments garbage collector work struct
- * @gc_fragments: array of display list fragments waiting to be freed
+ * @gc_work: bodies garbage collector work struct
+ * @gc_bodies: array of display list bodies waiting to be freed
  */
 struct vsp1_dl_manager {
unsigned int index;
@@ -119,7 +119,7 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *pending;
 
struct work_struct gc_work;
-   struct list_head gc_fragments;
+   struct list_head gc_bodies;
 };
 
 /* 
-
@@ -157,17 +157,16 @@ static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 }
 
 /**
- * vsp1_dl_fragment_alloc - Allocate a display list fragment
+ * vsp1_dl_body_alloc - Allocate a display list body
  * @vsp1: The VSP1 device
- * @num_entries: The maximum number of entries that the fragment can contain
+ * @num_entries: The maximum number of entries that the body can contain
  *
- * Allocate a display list fragment with enough memory to contain the requested
+ * Allocate a display list body with enough memory to contain the requested
  * number of entries.
  *
- * Return a pointer to a fragment on success or NULL if memory can't be
- * allocated.
+ * Return a pointer to a body on success or NULL if memory can't be allocated.
  */
-struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1,
+struct vsp1_dl_body *vsp1_dl_body_alloc(struct vsp1_device *vsp1,
unsigned int num_entries)
 {

[PATCH v6 5/9] v4l: vsp1: Use reference counting for bodies

[Kieran Bingham]

Extend the display list body with a reference count, allowing bodies to
be kept as long as a reference is maintained. This provides the ability
to keep a cached copy of bodies which will not change, so that they can
be re-applied to multiple display lists.

Signed-off-by: Kieran Bingham 

---
This could be squashed into the body update code, but it's not a
straightforward squash as the refcounts will affect both:
  v4l: vsp1: Provide a body pool
and
  v4l: vsp1: Convert display lists to use new body pool
therefore, I have kept this separate to prevent breaking bisectability
of the vsp-tests.

v3:
 - 's/fragment/body/'

v4:
 - Fix up reference handling comments.

 drivers/media/platform/vsp1/vsp1_clu.c |  7 ++-
 drivers/media/platform/vsp1/vsp1_dl.c  | 15 ++-
 drivers/media/platform/vsp1/vsp1_lut.c |  7 ++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 2018144470c5..b2a39a6ef7e4 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -257,8 +257,13 @@ static void clu_configure(struct vsp1_entity *entity,
clu->clu = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index a069c845..0f87e0bb21c1 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -58,6 +59,8 @@ struct vsp1_dl_body {
struct list_head list;
struct list_head free;
 
+   refcount_t refcnt;
+
struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
@@ -259,6 +262,7 @@ struct vsp1_dl_body *vsp1_dl_body_get(struct 
vsp1_dl_body_pool *pool)
if (!list_empty(>free)) {
dlb = list_first_entry(>free, struct vsp1_dl_body, free);
list_del(>free);
+   refcount_set(>refcnt, 1);
}
 
spin_unlock_irqrestore(>lock, flags);
@@ -279,6 +283,9 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb)
if (!dlb)
return;
 
+   if (!refcount_dec_and_test(>refcnt))
+   return;
+
dlb->num_entries = 0;
 
spin_lock_irqsave(>pool->lock, flags);
@@ -465,7 +472,11 @@ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, 
u32 data)
  * in the order in which bodies are added.
  *
  * Adding a body to a display list passes ownership of the body to the list. 
The
- * caller must not touch the body after this call.
+ * caller retains its reference to the fragment when adding it to the display
+ * list, but is not allowed to add new entries to the body.
+ *
+ * The reference must be explicitly released by a call to vsp1_dl_body_put()
+ * when the body isn't needed anymore.
  *
  * Additional bodies are only usable for display lists in header mode.
  * Attempting to add a body to a header-less display list will return an error.
@@ -477,6 +488,8 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl,
if (dl->dlm->mode != VSP1_DL_MODE_HEADER)
return -EINVAL;
 
+   refcount_inc(>refcnt);
+
list_add_tail(>list, >bodies);
return 0;
 }
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c 
b/drivers/media/platform/vsp1/vsp1_lut.c
index 262cb72139d6..77cf7137a0f2 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -213,8 +213,13 @@ static void lut_configure(struct vsp1_entity *entity,
lut->lut = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
-- 
git-series 0.9.1

[PATCH v6 3/9] v4l: vsp1: Provide a body pool

[Kieran Bingham]

Each display list allocates a body to store register values in a dma
accessible buffer from a dma_alloc_wc() allocation. Each of these
results in an entry in the TLB, and a large number of display list
allocations adds pressure to this resource.

Reduce TLB pressure on the IPMMUs by allocating multiple display list
bodies in a single allocation, and providing these to the display list
through a 'body pool'. A pool can be allocated by the display list
manager or entities which require their own body allocations.

Signed-off-by: Kieran Bingham 

---
v4:
 - Provide comment explaining extra allocation on body pool
   highlighting area for optimisation later.

v3:
 - s/fragment/body/, s/fragments/bodies/
 - qty -> num_bodies
 - indentation fix
 - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/'
 - Add kerneldoc to non-static functions

v2:
 - assign dlb->dma correctly

 drivers/media/platform/vsp1/vsp1_dl.c | 163 +++-
 drivers/media/platform/vsp1/vsp1_dl.h |   8 +-
 2 files changed, 171 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 59fe80bf6e9d..87bc4acf8c9e 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -45,6 +45,8 @@ struct vsp1_dl_entry {
 /**
  * struct vsp1_dl_body - Display list body
  * @list: entry in the display list list of bodies
+ * @free: entry in the pool free body list
+ * @pool: pool to which this body belongs
  * @vsp1: the VSP1 device
  * @entries: array of entries
  * @dma: DMA address of the entries
@@ -54,6 +56,9 @@ struct vsp1_dl_entry {
  */
 struct vsp1_dl_body {
struct list_head list;
+   struct list_head free;
+
+   struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
struct vsp1_dl_entry *entries;
@@ -65,6 +70,30 @@ struct vsp1_dl_body {
 };
 
 /**
+ * struct vsp1_dl_body_pool - display list body pool
+ * @dma: DMA address of the entries
+ * @size: size of the full DMA memory pool in bytes
+ * @mem: CPU memory pointer for the pool
+ * @bodies: Array of DLB structures for the pool
+ * @free: List of free DLB entries
+ * @lock: Protects the pool and free list
+ * @vsp1: the VSP1 device
+ */
+struct vsp1_dl_body_pool {
+   /* DMA allocation */
+   dma_addr_t dma;
+   size_t size;
+   void *mem;
+
+   /* Body management */
+   struct vsp1_dl_body *bodies;
+   struct list_head free;
+   spinlock_t lock;
+
+   struct vsp1_device *vsp1;
+};
+
+/**
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
@@ -105,6 +134,7 @@ enum vsp1_dl_mode {
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
+ * @pool: body pool for the display list bodies
  * @gc_work: bodies garbage collector work struct
  * @gc_bodies: array of display list bodies waiting to be freed
  */
@@ -120,6 +150,8 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *queued;
struct vsp1_dl_list *pending;
 
+   struct vsp1_dl_body_pool *pool;
+
struct work_struct gc_work;
struct list_head gc_bodies;
 };
@@ -128,6 +160,137 @@ struct vsp1_dl_manager {
  * Display List Body Management
  */
 
+/**
+ * vsp1_dl_body_pool_create - Create a pool of bodies from a single allocation
+ * @vsp1: The VSP1 device
+ * @num_bodies: The quantity of bodies to allocate
+ * @num_entries: The maximum number of entries that the body can contain
+ * @extra_size: Extra allocation provided for the bodies
+ *
+ * Allocate a pool of display list bodies each with enough memory to contain 
the
+ * requested number of entries.
+ *
+ * Return a pointer to a pool on success or NULL if memory can't be allocated.
+ */
+struct vsp1_dl_body_pool *
+vsp1_dl_body_pool_create(struct vsp1_device *vsp1, unsigned int num_bodies,
+unsigned int num_entries, size_t extra_size)
+{
+   struct vsp1_dl_body_pool *pool;
+   size_t dlb_size;
+   unsigned int i;
+
+   pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+   if (!pool)
+   return NULL;
+
+   pool->vsp1 = vsp1;
+
+   /*
+* Todo: 'extra_size' is only used by vsp1_dlm_create(), to allocate
+* extra memory for the display list header. We need only one header per
+* display list, not per display list body, thus this allocation is
+* extraneous and should be reworked in the future.
+*/
+   dlb_size = num_entries * sizeof(struct vsp1_dl_entry) + extra_size;
+   pool->size = dlb_size * num_bodies;
+
+   pool->bodies = kcalloc(num_bodies, sizeof(*pool->bodies), GFP_KERNEL);
+   if (!pool->bodies) {
+   kfree(pool);
+   return NULL;
+   }
+
+   pool->mem = 

[PATCH v6 9/9] v4l: vsp1: Reduce display list body size

[Kieran Bingham]

The display list originally allocated a body of 256 entries to store all
of the register lists required for each frame.

This has now been separated into fragments for constant stream setup, and
runtime updates.

Empirical testing shows that the body0 now uses a maximum of 41
registers for each frame, for both DRM and Video API pipelines thus a
rounded 64 entries provides a suitable allocation.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index a762e840d147..6b5743a431a2 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -21,7 +21,7 @@
 #include "vsp1.h"
 #include "vsp1_dl.h"
 
-#define VSP1_DL_NUM_ENTRIES256
+#define VSP1_DL_NUM_ENTRIES64
 
 #define VSP1_DLH_INT_ENABLE(1 << 1)
 #define VSP1_DLH_AUTO_START(1 << 0)
-- 
git-series 0.9.1

[PATCH v6 7/9] v4l: vsp1: Adapt entities to configure into a body

[Kieran Bingham]

Currently the entities store their configurations into a display list.
Adapt this such that the code can be configured into a body directly,
allowing greater flexibility and control of the content.

All users of vsp1_dl_list_write() are removed in this process, thus it
too is removed.

A helper, vsp1_dl_list_get_body0() is provided to access the internal body0
from the display list.

Signed-off-by: Kieran Bingham 

---

v4:
 - Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0()
   The similarities between vsp1_dl_list_get_body and
   vsp1_dl_list_body_get() were too close

 - body0 could be removed later when the default body is no longer
   needed.

v5:
 - Support DRM/UIF changes

v6:
 - Remove DRM/UIF changes

 drivers/media/platform/vsp1/vsp1_bru.c| 22 ++--
 drivers/media/platform/vsp1/vsp1_clu.c| 22 ++--
 drivers/media/platform/vsp1/vsp1_dl.c | 12 ++-
 drivers/media/platform/vsp1/vsp1_dl.h |  2 +-
 drivers/media/platform/vsp1/vsp1_drm.c| 22 +++-
 drivers/media/platform/vsp1/vsp1_entity.c | 16 -
 drivers/media/platform/vsp1/vsp1_entity.h | 12 ---
 drivers/media/platform/vsp1/vsp1_hgo.c| 16 -
 drivers/media/platform/vsp1/vsp1_hgt.c| 18 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   | 10 +++---
 drivers/media/platform/vsp1/vsp1_lif.c| 15 
 drivers/media/platform/vsp1/vsp1_lut.c| 21 ++--
 drivers/media/platform/vsp1/vsp1_pipe.c   |  4 +-
 drivers/media/platform/vsp1/vsp1_pipe.h   |  3 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 43 +++-
 drivers/media/platform/vsp1/vsp1_sru.c| 14 
 drivers/media/platform/vsp1/vsp1_uds.c| 24 +++--
 drivers/media/platform/vsp1/vsp1_uds.h|  2 +-
 drivers/media/platform/vsp1/vsp1_video.c  | 11 --
 drivers/media/platform/vsp1/vsp1_wpf.c| 42 ---
 20 files changed, 174 insertions(+), 157 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index b9ff96f76b3e..60d449d7b135 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -30,10 +30,10 @@
  * Device Access
  */
 
-static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
*dl,
- u32 reg, u32 data)
+static inline void vsp1_bru_write(struct vsp1_bru *bru,
+ struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
-   vsp1_dl_list_write(dl, bru->base + reg, data);
+   vsp1_dl_body_write(dlb, bru->base + reg, data);
 }
 
 /* 
-
@@ -287,7 +287,7 @@ static const struct v4l2_subdev_ops bru_ops = {
 
 static void bru_prepare(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
-   struct vsp1_dl_list *dl)
+   struct vsp1_dl_body *dlb)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
@@ -309,7 +309,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * format at the pipeline output is premultiplied.
 */
flags = pipe->output ? pipe->output->format.flags : 0;
-   vsp1_bru_write(bru, dl, VI6_BRU_INCTRL,
+   vsp1_bru_write(bru, dlb, VI6_BRU_INCTRL,
   flags & V4L2_PIX_FMT_FLAG_PREMUL_ALPHA ?
   0 : VI6_BRU_INCTRL_NRM);
 
@@ -317,12 +317,12 @@ static void bru_prepare(struct vsp1_entity *entity,
 * Set the background position to cover the whole output image and
 * configure its color.
 */
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_SIZE,
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_SIZE,
   (format->width << VI6_BRU_VIRRPF_SIZE_HSIZE_SHIFT) |
   (format->height << VI6_BRU_VIRRPF_SIZE_VSIZE_SHIFT));
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_LOC, 0);
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_LOC, 0);
 
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_COL, bru->bgcolor |
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_COL, bru->bgcolor |
   (0xff << VI6_BRU_VIRRPF_COL_A_SHIFT));
 
/*
@@ -332,7 +332,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * unit.
 */
if (entity->type == VSP1_ENTITY_BRU)
-   vsp1_bru_write(bru, dl, VI6_BRU_ROP,
+   vsp1_bru_write(bru, dlb, VI6_BRU_ROP,
   VI6_BRU_ROP_DSTSEL_BRUIN(1) |
   VI6_BRU_ROP_CROP(VI6_ROP_NOP) |
   VI6_BRU_ROP_AROP(VI6_ROP_NOP));
@@ -374,7 +374,7 @@ static void bru_prepare(struct vsp1_entity *entity,
if (!(entity->type == VSP1_ENTITY_BRU && i == 1))
ctrl |= VI6_BRU_CTRL_SRCSEL_BRUIN(i);
 
-   vsp1_bru_write(bru, dl, VI6_BRU_CTRL(i), 

[PATCH v6 0/9] vsp1: TLB optimisation and DL caching

[Kieran Bingham]

Each display list currently allocates an area of DMA memory to store register
settings for the VSP1 to process. Each of these allocations adds pressure to
the IPMMU TLB entries.

We can reduce the pressure by pre-allocating larger areas and dividing the area
across multiple bodies represented as a pool.

With this reconfiguration of bodies, we can adapt the configuration code to
separate out constant hardware configuration and cache it for re-use.

The patches provided in this series can be found at:
  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git  
tags/vsp1/tlb-optimise/v6

Changelog:
--

v6:
 - Rebased on to linux-media/master (v4.16-rc1)
 - Removed DRM/UIF (DISCOM/ColorKey) updates

v5:
 - Rebased on to renesas-drivers-2018-01-09-v4.15-rc7 to fix conflicts
   with DRM and UIF updates on VSP1 driver

v4:
 - Rebased to v4.14
 * v4l: vsp1: Use reference counting for bodies
   - Fix up reference handling comments

 * v4l: vsp1: Provide a body pool
   - Provide comment explaining extra allocation on body pool
 highlighting area for optimisation later.

 * v4l: vsp1: Refactor display list configure operations
   - Fix up comment to describe yuv_mode caching rather than format

 * vsp1: Adapt entities to configure into a body
   - Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0()

 * v4l: vsp1: Move video configuration to a cached dlb
   - Adjust pipe configured flag to be reset on resume rather than suspend
   - rename dl_child, dl_next

Testing:

The VSP unit tests have been run on this patch set with the following results:

--- Test loop 1 ---
- vsp-unit-test-.sh
Test Conditions:
  Platform  Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+
  Kernel release4.16.0-rc1-arm64-renesas-00166-ge0ad4e839fff
  convert   /usr/bin/convert
  compare   /usr/bin/compare
  killall   /usr/bin/killall
  raw2rgbpnm/usr/bin/raw2rgbpnm
  stress/usr/bin/stress
  yavta /usr/bin/yavta
- vsp-unit-test-0001.sh
Testing WPF packing in RGB332: pass
Testing WPF packing in ARGB555: pass
Testing WPF packing in XRGB555: pass
Testing WPF packing in RGB565: pass
Testing WPF packing in BGR24: pass
Testing WPF packing in RGB24: pass
Testing WPF packing in ABGR32: pass
Testing WPF packing in ARGB32: pass
Testing WPF packing in XBGR32: pass
Testing WPF packing in XRGB32: pass
- vsp-unit-test-0002.sh
Testing WPF packing in NV12M: pass
Testing WPF packing in NV16M: pass
Testing WPF packing in NV21M: pass
Testing WPF packing in NV61M: pass
Testing WPF packing in UYVY: pass
Testing WPF packing in VYUY: skip
Testing WPF packing in YUV420M: pass
Testing WPF packing in YUV422M: pass
Testing WPF packing in YUV444M: pass
Testing WPF packing in YVU420M: pass
Testing WPF packing in YVU422M: pass
Testing WPF packing in YVU444M: pass
Testing WPF packing in YUYV: pass
Testing WPF packing in YVYU: pass
- vsp-unit-test-0003.sh
Testing scaling from 640x640 to 640x480 in RGB24: pass
Testing scaling from 1024x768 to 640x480 in RGB24: pass
Testing scaling from 640x480 to 1024x768 in RGB24: pass
Testing scaling from 640x640 to 640x480 in YUV444M: pass
Testing scaling from 1024x768 to 640x480 in YUV444M: pass
Testing scaling from 640x480 to 1024x768 in YUV444M: pass
- vsp-unit-test-0004.sh
Testing histogram in RGB24: pass
Testing histogram in YUV444M: pass
- vsp-unit-test-0005.sh
Testing RPF.0: pass
Testing RPF.1: pass
Testing RPF.2: pass
Testing RPF.3: pass
Testing RPF.4: pass
- vsp-unit-test-0006.sh
Testing invalid pipeline with no RPF: pass
Testing invalid pipeline with no WPF: pass
- vsp-unit-test-0007.sh
Testing BRU in RGB24 with 1 inputs: pass
Testing BRU in RGB24 with 2 inputs: pass
Testing BRU in RGB24 with 3 inputs: pass
Testing BRU in RGB24 with 4 inputs: pass
Testing BRU in RGB24 with 5 inputs: pass
Testing BRU in YUV444M with 1 inputs: pass
Testing BRU in YUV444M with 2 inputs: pass
Testing BRU in YUV444M with 3 inputs: pass
Testing BRU in YUV444M with 4 inputs: pass
Testing BRU in YUV444M with 5 inputs: pass
- vsp-unit-test-0008.sh
Test requires unavailable feature set `bru rpf.0 uds wpf.0': skipped
- vsp-unit-test-0009.sh
Test requires unavailable feature set `rpf.0 wpf.0 wpf.1': skipped
- vsp-unit-test-0010.sh
Testing CLU in RGB24 with zero configuration: pass
Testing CLU in RGB24 with identity configuration: pass
Testing CLU in RGB24 with wave configuration: pass
Testing CLU in YUV444M with zero configuration: pass
Testing CLU in YUV444M with identity configuration: pass
Testing CLU in YUV444M with wave configuration: pass
Testing LUT in RGB24 with zero configuration: pass
Testing LUT in RGB24 with identity configuration: pass
Testing LUT in RGB24 with gamma configuration: pass
Testing LUT in YUV444M with zero configuration: pass
Testing LUT in YUV444M with identity configuration: pass
Testing LUT in YUV444M with gamma configuration: pass
- vsp-unit-test-0011.sh
Testing  hflip=0 vflip=0 rotate=0: pass
Testing  hflip=1 

[PATCH v6 8/9] v4l: vsp1: Move video configuration to a cached dlb

[Kieran Bingham]

We are now able to configure a pipeline directly into a local display
list body. Take advantage of this fact, and create a cacheable body to
store the configuration of the pipeline in the video object.

vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
Convert this function to use the cached video->config body and obtain a
local display list reference.

Attach the video->config body to the display list when needed before
committing to hardware.

The pipe object is marked as un-configured when resuming from a suspend.
This ensures that when the hardware is reset - our cached configuration
will be re-attached to the next committed DL.

Signed-off-by: Kieran Bingham 
---

v3:
 - 's/fragment/body/', 's/fragments/bodies/'
 - video dlb cache allocation increased from 2 to 3 dlbs

Our video DL usage now looks like the below output:

dl->body0 contains our disposable runtime configuration. Max 41.
dl_child->body0 is our partition specific configuration. Max 12.
dl->bodies shows our constant configuration and LUTs.

  These two are LUT/CLU:
 * dl->bodies[x]->num_entries 256 / max 256
 * dl->bodies[x]->num_entries 4914 / max 4914

Which shows that our 'constant' configuration cache is currently
utilised to a maximum of 64 entries.

trace-cmd report | \
grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;

  dl->body0->num_entries 13 / max 128
  dl->body0->num_entries 14 / max 128
  dl->body0->num_entries 16 / max 128
  dl->body0->num_entries 20 / max 128
  dl->body0->num_entries 27 / max 128
  dl->body0->num_entries 34 / max 128
  dl->body0->num_entries 41 / max 128
  dl_child->body0->num_entries 10 / max 128
  dl_child->body0->num_entries 12 / max 128
  dl->bodies[x]->num_entries 15 / max 128
  dl->bodies[x]->num_entries 16 / max 128
  dl->bodies[x]->num_entries 17 / max 128
  dl->bodies[x]->num_entries 18 / max 128
  dl->bodies[x]->num_entries 20 / max 128
  dl->bodies[x]->num_entries 21 / max 128
  dl->bodies[x]->num_entries 256 / max 256
  dl->bodies[x]->num_entries 31 / max 128
  dl->bodies[x]->num_entries 32 / max 128
  dl->bodies[x]->num_entries 39 / max 128
  dl->bodies[x]->num_entries 40 / max 128
  dl->bodies[x]->num_entries 47 / max 128
  dl->bodies[x]->num_entries 48 / max 128
  dl->bodies[x]->num_entries 4914 / max 4914
  dl->bodies[x]->num_entries 55 / max 128
  dl->bodies[x]->num_entries 56 / max 128
  dl->bodies[x]->num_entries 63 / max 128
  dl->bodies[x]->num_entries 64 / max 128

v4:
 - Adjust pipe configured flag to be reset on resume rather than suspend
 - rename dl_child, dl_next

 drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
 drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
 drivers/media/platform/vsp1/vsp1_video.c | 67 -
 drivers/media/platform/vsp1/vsp1_video.h |  2 +-
 4 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 5012643583b6..fa445b1a2e38 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
   VI6_CMD_STRCMD);
pipe->state = VSP1_PIPELINE_RUNNING;
+   pipe->configured = true;
}
 
pipe->buffers_ready = 0;
@@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
continue;
 
spin_lock_irqsave(>irqlock, flags);
+   /*
+* The hardware may have been reset during a suspend and will
+* need a full reconfiguration
+*/
+   pipe->configured = false;
+
if (vsp1_pipeline_ready(pipe))
vsp1_pipeline_run(pipe);
spin_unlock_irqrestore(>irqlock, flags);
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h 
b/drivers/media/platform/vsp1/vsp1_pipe.h
index 90d29492b9b9..e7ad6211b4d0 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -90,6 +90,7 @@ struct vsp1_partition {
  * @irqlock: protects the pipeline state
  * @state: current state
  * @wq: wait queue to wait for state change completion
+ * @configured: flag determining if the hardware has run since reset
  * @frame_end: frame end interrupt handler
  * @lock: protects the pipeline use count and stream count
  * @kref: pipeline reference count
@@ -117,6 +118,7 @@ struct vsp1_pipeline {
spinlock_t irqlock;
enum vsp1_pipeline_state state;
wait_queue_head_t wq;
+   bool configured;
 
void (*frame_end)(struct vsp1_pipeline *pipe, bool completed);
 
@@ -143,8 +145,6 @@ struct vsp1_pipeline {
 */
struct list_head entities;
 
-   struct vsp1_dl_list *dl;
-
unsigned int partitions;
struct 

[PATCH v6 2/9] v4l: vsp1: Protect bodies against overflow

[Kieran Bingham]

The body write function relies on the code never asking it to write more
than the entries available in the list.

Currently with each list body containing 256 entries, this is fine, but
we can reduce this number greatly saving memory. In preparation of this
add a level of protection to catch any buffer overflows.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 

---

v3:
 - adapt for new 'body' terminology
 - simplify WARN_ON macro usage

 drivers/media/platform/vsp1/vsp1_dl.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0c1bd17f9281..59fe80bf6e9d 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -50,6 +50,7 @@ struct vsp1_dl_entry {
  * @dma: DMA address of the entries
  * @size: size of the DMA memory in bytes
  * @num_entries: number of stored entries
+ * @max_entries: number of entries available
  */
 struct vsp1_dl_body {
struct list_head list;
@@ -60,6 +61,7 @@ struct vsp1_dl_body {
size_t size;
 
unsigned int num_entries;
+   unsigned int max_entries;
 };
 
 /**
@@ -139,6 +141,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
 
dlb->vsp1 = vsp1;
dlb->size = size;
+   dlb->max_entries = num_entries;
 
dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, >dma,
GFP_KERNEL);
@@ -220,6 +223,10 @@ void vsp1_dl_body_free(struct vsp1_dl_body *dlb)
  */
 void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
+   if (WARN_ONCE(dlb->num_entries >= dlb->max_entries,
+ "DLB size exceeded (max %u)", dlb->max_entries))
+   return;
+
dlb->entries[dlb->num_entries].addr = reg;
dlb->entries[dlb->num_entries].data = data;
dlb->num_entries++;
-- 
git-series 0.9.1

[PATCH v6 4/9] v4l: vsp1: Convert display lists to use new body pool

[Kieran Bingham]

Adapt the dl->body0 object to use an object from the body pool. This
greatly reduces the pressure on the TLB for IPMMU use cases, as all of
the lists use a single allocation for the main body.

The CLU and LUT objects pre-allocate a pool containing three bodies,
allowing a userspace update before the hardware has committed a previous
set of tables.

Bodies are no longer 'freed' in interrupt context, but instead released
back to their respective pools. This allows us to remove the garbage
collector in the DLM.

Signed-off-by: Kieran Bingham 

---
v3:
 - 's/fragment/body', 's/fragments/bodies/'
 - CLU/LUT now allocate 3 bodies
 - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put

v2:
 - Use dl->body0->max_entries to determine header offset, instead of the
   global constant VSP1_DL_NUM_ENTRIES which is incorrect.
 - squash updates for LUT, CLU, and fragment cleanup into single patch.
   (Not fully bisectable when separated)

 drivers/media/platform/vsp1/vsp1_clu.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_clu.h |   1 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 223 ++
 drivers/media/platform/vsp1/vsp1_dl.h  |   3 +-
 drivers/media/platform/vsp1/vsp1_lut.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
 6 files changed, 101 insertions(+), 181 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 9621afa3658c..2018144470c5 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -23,6 +23,8 @@
 #define CLU_MIN_SIZE   4U
 #define CLU_MAX_SIZE   8190U
 
+#define CLU_SIZE   (17 * 17 * 17)
+
 /* 
-
  * Device Access
  */
@@ -47,19 +49,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_get(clu->pool);
if (!dlb)
return -ENOMEM;
 
vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
-   for (i = 0; i < 17 * 17 * 17; ++i)
+   for (i = 0; i < CLU_SIZE; ++i)
vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_body_free(dlb);
+   vsp1_dl_body_put(dlb);
return 0;
 }
 
@@ -261,8 +263,16 @@ static void clu_configure(struct vsp1_entity *entity,
}
 }
 
+static void clu_destroy(struct vsp1_entity *entity)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   vsp1_dl_body_pool_destroy(clu->pool);
+}
+
 static const struct vsp1_entity_operations clu_entity_ops = {
.configure = clu_configure,
+   .destroy = clu_destroy,
 };
 
 /* 
-
@@ -288,6 +298,17 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
if (ret < 0)
return ERR_PTR(ret);
 
+   /*
+* Pre-allocate a body pool, with 3 bodies allowing a userspace update
+* before the hardware has committed a previous set of tables, handling
+* both the queued and pending dl entries. One extra entry is added to
+* the CLU_SIZE to allow for the VI6_CLU_ADDR header.
+*/
+   clu->pool = vsp1_dl_body_pool_create(clu->entity.vsp1, 3, CLU_SIZE + 1,
+0);
+   if (!clu->pool)
+   return ERR_PTR(-ENOMEM);
+
/* Initialize the control handler. */
v4l2_ctrl_handler_init(>ctrls, 2);
v4l2_ctrl_new_custom(>ctrls, _table_control, NULL);
diff --git a/drivers/media/platform/vsp1/vsp1_clu.h 
b/drivers/media/platform/vsp1/vsp1_clu.h
index 036e0a2f1a42..fa3fe856725b 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.h
+++ b/drivers/media/platform/vsp1/vsp1_clu.h
@@ -36,6 +36,7 @@ struct vsp1_clu {
spinlock_t lock;
unsigned int mode;
struct vsp1_dl_body *clu;
+   struct vsp1_dl_body_pool *pool;
 };
 
 static inline struct vsp1_clu *to_clu(struct v4l2_subdev *subdev)
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 87bc4acf8c9e..a069c845 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -111,7 +111,7 @@ struct vsp1_dl_list {
struct vsp1_dl_header *header;
dma_addr_t dma;
 
-   struct vsp1_dl_body body0;
+   struct vsp1_dl_body *body0;
struct list_head bodies;
 
bool has_chain;
@@ -135,8 +135,6 @@ enum vsp1_dl_mode {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
- * 

[PATCH v6 6/9] v4l: vsp1: Refactor display list configure operations

[Kieran Bingham]

The entities provide a single .configure operation which configures the
object into the target display list, based on the vsp1_entity_params
selection.

This restricts us to a single function prototype for both static
configuration (the pre-stream INIT stage) and the dynamic runtime stages
for both each frame - and each partition therein.

Split the configure function into two parts, '.prepare()' and
'.configure()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
.configure(). The configuration for individual partitions is handled by
passing the partition number to the configure call, and processing any
runtime stage actions on the first partition only.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_clu.c|  42 +--
 drivers/media/platform/vsp1/vsp1_dl.h |   1 +-
 drivers/media/platform/vsp1/vsp1_drm.c|  21 +--
 drivers/media/platform/vsp1/vsp1_entity.c |  15 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  27 +--
 drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
 drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
 drivers/media/platform/vsp1/vsp1_lut.c|  24 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 162 ++---
 drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_uds.c|  55 ++--
 drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
 drivers/media/platform/vsp1/vsp1_wpf.c| 297 ---
 16 files changed, 360 insertions(+), 380 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index e8fd2ae3b3eb..b9ff96f76b3e 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -285,19 +285,15 @@ static const struct v4l2_subdev_ops bru_ops = {
  * VSP1 Entity Operations
  */
 
-static void bru_configure(struct vsp1_entity *entity,
- struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+static void bru_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
unsigned int flags;
unsigned int i;
 
-   if (params != VSP1_ENTITY_PARAMS_INIT)
-   return;
-
format = vsp1_entity_get_pad_format(>entity, bru->entity.config,
bru->entity.source_pad);
 
@@ -404,7 +400,7 @@ static void bru_configure(struct vsp1_entity *entity,
 }
 
 static const struct vsp1_entity_operations bru_entity_ops = {
-   .configure = bru_configure,
+   .prepare = bru_prepare,
 };
 
 /* 
-
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index b2a39a6ef7e4..be4d7e493746 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -213,37 +213,36 @@ static const struct v4l2_subdev_ops clu_ops = {
 /* 
-
  * VSP1 Entity Operations
  */
+static void clu_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   /*
+* The yuv_mode can't be changed during streaming. Cache it internally
+* for future runtime configuration calls.
+*/
+   struct v4l2_mbus_framefmt *format;
+
+   format = vsp1_entity_get_pad_format(>entity,
+   clu->entity.config,
+   CLU_PAD_SINK);
+   clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
+}
 
 static void clu_configure(struct vsp1_entity *entity,
  struct vsp1_pipeline *pipe,
  struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+ unsigned int partition)
 {
struct vsp1_clu *clu = to_clu(>subdev);
struct vsp1_dl_body *dlb;
unsigned long flags;
u32 ctrl = VI6_CLU_CTRL_AAI | VI6_CLU_CTRL_MVS | VI6_CLU_CTRL_EN;
 
-   switch (params) {
-   case VSP1_ENTITY_PARAMS_INIT: {
-   /*
-* The format can't be changed during streaming, only verify it
-* at setup time and store the information internally for future
-* runtime configuration calls.
-*/
-   struct v4l2_mbus_framefmt *format;
-
-   

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Jani Nikula]

On Wed, 28 Feb 2018, Thierry Reding  wrote:
> Anyone that needs something other than normal mode should use the new
> atomic PWM API.

At the risk of revealing my true ignorance, what is the new atomic PWM
API? Where? Examples of how one would convert old code over to the new
API?

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Thierry Reding]

On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
> 
> Signed-off-by: Claudiu Beznea 
> ---
>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>  drivers/bus/ts-nbus.c|  2 +-
>  drivers/clk/clk-pwm.c|  3 ++-
>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>  drivers/hwmon/pwm-fan.c  |  2 +-
>  drivers/input/misc/max77693-haptic.c |  2 +-
>  drivers/input/misc/max8997_haptic.c  |  6 +-
>  drivers/leds/leds-pwm.c  |  5 -
>  drivers/media/rc/ir-rx51.c   |  5 -
>  drivers/media/rc/pwm-ir-tx.c |  5 -
>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>  drivers/video/backlight/lp8788_bl.c  |  5 -
>  drivers/video/backlight/pwm_bl.c | 11 +--
>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>  include/linux/pwm.h  |  6 --
>  16 files changed, 70 insertions(+), 21 deletions(-)

I don't think it makes sense to leak mode support into the legacy API.
The pwm_config() function is considered legacy and should eventually go
away. As such it doesn't make sense to integrate a new feature such as
PWM modes into it. All users of pwm_config() assume normal mode, and
that's what pwm_config() should provide.

Anyone that needs something other than normal mode should use the new
atomic PWM API.

Thierry


signature.asc
Description: PGP signature

dvb: New unsupported version of Astrometa DVB-T2

[Stanislav Brabec]

Hi.

I just purchased a new DVB-T2 USB dongle on Ebay[1].

This dongle reports itself as Astrometa DVB-T2, but it does not work
with the current v4l-dvb kernel (tested with 2942273).

After a teardown[2], I realized, that it has a different (and unknown,
as the manufacturer removed the label) tuner chip, MN88473 and RTL8232P.

w-scan is able to find some multiplexes, but it is not able to tune and
decode, so the tuner seems to be at least partially supported ("Info: no
data from PAT after 2 seconds", see the log in [3]).

MN88473 is not detected, and firmware is not loaded (even if the
previous supported version contains the same chip).

The bundled CD contains Windows drivers.[3]

Could anybody provide me a hint, how to debug it or make it working?

References:
[1] item to purchase: https://www.ebay.com/itm/152240047586 (Wish)
[2] teardown photos: https://photos.app.goo.gl/kWze7I03ksZWNL2C3
[3] files: 
https://drive.google.com/drive/folders/1N1H8KjpZHz3ruLOc37lSLpMU4fpPRnla?usp=sharing

[   66.521783] usb 1-1.3: new high-speed USB device number 4 using ehci-pci
[   66.642755] usb 1-1.3: New USB device found, idVendor=15f4, idProduct=0131
[   66.642758] usb 1-1.3: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[   66.642760] usb 1-1.3: Product: dvbt2
[   66.642761] usb 1-1.3: Manufacturer: astrometadvbt2
[   66.642763] usb 1-1.3: SerialNumber: 0
[   66.669636] rc_core: IR Remote Control driver registered, major 243
[   66.676299] usb 1-1.3: dvb_usb_v2: found a 'Astrometa DVB-T2' in warm state
[   66.736331] usb 1-1.3: dvb_usb_v2: will pass the complete MPEG2 transport 
stream to the software demuxer
[   66.736337] dvbdev: DVB: registering new adapter (Astrometa DVB-T2)
[   66.738086] i2c i2c-10: Added multiplexed i2c bus 11
[   66.738088] rtl2832 10-0010: Realtek RTL2832 successfully attached
[   66.738102] usb 1-1.3: DVB: registering adapter 0 frontend 0 (Realtek 
RTL2832 (DVB-T))...
[   66.739090] r820t 11-001a: creating new instance
[   66.746288] r820t 11-001a: Rafael Micro r820t successfully identified
[   66.751321] Linux video capture interface: v2.00
[   66.754254] rtl2832_sdr rtl2832_sdr.1.auto: Registered as swradio0
[   66.754256] rtl2832_sdr rtl2832_sdr.1.auto: Realtek RTL2832 SDR attached
[   66.754257] rtl2832_sdr rtl2832_sdr.1.auto: SDR API is still slightly 
experimental and functionality changes may follow
[   66.765294] Registered IR keymap rc-empty
[   66.765315] rc rc0: Astrometa DVB-T2 as 
/devices/pci:00/:00:1a.0/usb1/1-1/1-1.3/rc/rc0
[   66.765338] input: Astrometa DVB-T2 as 
/devices/pci:00/:00:1a.0/usb1/1-1/1-1.3/rc/rc0/input10
[   66.765387] rc rc0: lirc_dev: driver dvb_usb_rtl28xxu registered at minor = 0
[   66.765411] usb 1-1.3: dvb_usb_v2: schedule remote query interval to 200 
msecs
[   66.774571] usb 1-1.3: dvb_usb_v2: 'Astrometa DVB-T2' successfully 
initialized and connected
[   66.774622] usbcore: registered new interface driver dvb_usb_rtl28xxu

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
-
SUSE LINUX, s. r. o. e-mail: sbra...@suse.com
Křižíkova 148/34 (Corso IIa)  tel: +49 911 7405384547
186 00 Praha 8-Karlín  fax:  +420 284 084 001
Czech Republichttp://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

[PATCH v2] media: stm32-dcmi: add JPEG support

[Hugues Fruchet]

Add DCMI JPEG support.

Signed-off-by: Hugues Fruchet 
---
version 2:
  - Removed V4L2_FMT_FLAG_COMPRESSED flag setting already set by V4L2 core
See https://www.mail-archive.com/linux-media@vger.kernel.org/msg126825.html

 drivers/media/platform/stm32/stm32-dcmi.c | 193 ++
 1 file changed, 146 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 269e963..36f9a77 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -93,6 +93,11 @@ enum state {
 #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 TIMEOUT_MS 1000
 
 struct dcmi_graph_entity {
@@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 reg, 
u32 mask)
 
 static int dcmi_start_capture(struct stm32_dcmi *dcmi);
 
+static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
+struct dcmi_buf *buf,
+size_t bytesused,
+int err)
+{
+   struct vb2_v4l2_buffer *vbuf;
+
+   if (!buf)
+   return;
+
+   vbuf = >vb;
+
+   vbuf->sequence = dcmi->sequence++;
+   vbuf->field = V4L2_FIELD_NONE;
+   vbuf->vb2_buf.timestamp = ktime_get_ns();
+   vb2_set_plane_payload(>vb2_buf, 0, bytesused);
+   vb2_buffer_done(>vb2_buf,
+   err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
+   dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
+   vbuf->vb2_buf.index, vbuf->sequence, bytesused);
+
+   dcmi->buffers_count++;
+   dcmi->active = NULL;
+}
+
+static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
+{
+   spin_lock_irq(>irqlock);
+
+   if (dcmi->state != RUNNING) {
+   spin_unlock_irq(>irqlock);
+   return -EINVAL;
+   }
+
+   /* 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++;
+   dcmi->active = NULL;
+
+   spin_unlock_irq(>irqlock);
+   return -EINVAL;
+   }
+
+   dcmi->active = list_entry(dcmi->buffers.next,
+ struct dcmi_buf, list);
+   list_del_init(>active->list);
+
+   spin_unlock_irq(>irqlock);
+
+   return dcmi_start_capture(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;
-
-   spin_lock_irq(>irqlock);
+   struct dcmi_buf *buf = dcmi->active;
 
/* Check DMA status */
status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
@@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
case DMA_COMPLETE:
dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
 
-   if (dcmi->active) {
-   struct dcmi_buf *buf = dcmi->active;
-   struct vb2_v4l2_buffer *vbuf = >active->vb;
-
-   vbuf->sequence = dcmi->sequence++;
-   vbuf->field = V4L2_FIELD_NONE;
-   vbuf->vb2_buf.timestamp = ktime_get_ns();
-   vb2_set_plane_payload(>vb2_buf, 0, buf->size);
-   vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
-   dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
-   vbuf->vb2_buf.index, vbuf->sequence);
-
-   dcmi->buffers_count++;
-   dcmi->active = NULL;
-   }
-
-   /* Restart a new DMA transfer with next buffer */
-   if (dcmi->state == RUNNING) {
-   if (list_empty(>buffers)) {
-   dev_err(dcmi->dev, "%s: No more buffer queued, 
cannot capture buffer\n",
-   __func__);
-   dcmi->errors_count++;
-   dcmi->active = NULL;
-
-   spin_unlock_irq(>irqlock);
-   return;
-   }
-
-   dcmi->active = list_entry(dcmi->buffers.next,
- struct dcmi_buf, list);
-
-   list_del_init(>active->list);
-
-   spin_unlock_irq(>irqlock);
-   if (dcmi_start_capture(dcmi))
-   dev_err(dcmi->dev, "%s: Cannot restart capture 
on DMA complete\n",
-  

Re: [PATCH] media: stm32-dcmi: add JPEG support

[Hugues FRUCHET]

Hi Hans,

Yes depends on 'fix lock scheme', I'll send a v2 of this jpeg commit for 
unneeded V4L2_FMT_FLAG_COMPRESSED.

Best regards,
Hugues.

On 02/26/2018 03:17 PM, Hans Verkuil wrote:
> On 02/22/2018 10:51 AM, Hugues Fruchet wrote:
>> Add DCMI JPEG support.
> 
> Does this patch depend on the 'fix lock scheme' patch?
> 
> It looks good to me except for one small thing (see below), but I think this
> depends on the 'fix lock scheme' patch, right?
> 
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   drivers/media/platform/stm32/stm32-dcmi.c | 195 
>> +++---
>>   1 file changed, 148 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 269e963..7eaaf7c 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -93,6 +93,11 @@ enum state {
>>   #define MIN_HEIGHT 16U
>>   #define MAX_HEIGHT 2048U
>>   
>> +#define MIN_JPEG_WIDTH  16U
>> +#define MAX_JPEG_WIDTH  2592U
>> +#define MIN_JPEG_HEIGHT 16U
>> +#define MAX_JPEG_HEIGHT 2592U
>> +
>>   #define TIMEOUT_MS 1000
>>   
>>   struct dcmi_graph_entity {
>> @@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 
>> reg, u32 mask)
>>   
>>   static int dcmi_start_capture(struct stm32_dcmi *dcmi);
>>   
>> +static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
>> + struct dcmi_buf *buf,
>> + size_t bytesused,
>> + int err)
>> +{
>> +struct vb2_v4l2_buffer *vbuf;
>> +
>> +if (!buf)
>> +return;
>> +
>> +vbuf = >vb;
>> +
>> +vbuf->sequence = dcmi->sequence++;
>> +vbuf->field = V4L2_FIELD_NONE;
>> +vbuf->vb2_buf.timestamp = ktime_get_ns();
>> +vb2_set_plane_payload(>vb2_buf, 0, bytesused);
>> +vb2_buffer_done(>vb2_buf,
>> +err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
>> +dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
>> +vbuf->vb2_buf.index, vbuf->sequence, bytesused);
>> +
>> +dcmi->buffers_count++;
>> +dcmi->active = NULL;
>> +}
>> +
>> +static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
>> +{
>> +spin_lock_irq(>irqlock);
>> +
>> +if (dcmi->state != RUNNING) {
>> +spin_unlock_irq(>irqlock);
>> +return -EINVAL;
>> +}
>> +
>> +/* 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++;
>> +dcmi->active = NULL;
>> +
>> +spin_unlock_irq(>irqlock);
>> +return -EINVAL;
>> +}
>> +
>> +dcmi->active = list_entry(dcmi->buffers.next,
>> +  struct dcmi_buf, list);
>> +list_del_init(>active->list);
>> +
>> +spin_unlock_irq(>irqlock);
>> +
>> +return dcmi_start_capture(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;
>> -
>> -spin_lock_irq(>irqlock);
>> +struct dcmi_buf *buf = dcmi->active;
>>   
>>  /* Check DMA status */
>>  status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
>> @@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
>>  case DMA_COMPLETE:
>>  dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
>>   
>> -if (dcmi->active) {
>> -struct dcmi_buf *buf = dcmi->active;
>> -struct vb2_v4l2_buffer *vbuf = >active->vb;
>> -
>> -vbuf->sequence = dcmi->sequence++;
>> -vbuf->field = V4L2_FIELD_NONE;
>> -vbuf->vb2_buf.timestamp = ktime_get_ns();
>> -vb2_set_plane_payload(>vb2_buf, 0, buf->size);
>> -vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
>> -dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
>> -vbuf->vb2_buf.index, vbuf->sequence);
>> -
>> -dcmi->buffers_count++;
>> -dcmi->active = NULL;
>> -}
>> -
>> -/* Restart a new DMA transfer with next buffer */
>> -if (dcmi->state == RUNNING) {
>> -if (list_empty(>buffers)) {
>> -dev_err(dcmi->dev, "%s: No more buffer queued, 
>> cannot capture buffer\n",
>> -__func__);
>> -dcmi->errors_count++;
>> -dcmi->active = NULL;
>> -
>> -spin_unlock_irq(>irqlock);
>> -return;
>> -}

Re: [PATCH v2] media: stm32-dcmi: fix lock scheme

[Hugues FRUCHET]

Hi Hans,

Problem was that lock was held while calling dmaengine APIs, which 
causes double lock issue when dma callback was called under context of 
dma_async_issue_pending() (see dcmi_start_dma() & below comments).
Rest of changes are around spin_lock() changed to spin_lock_irq() for 
safer lock management.

On 02/26/2018 02:56 PM, Hans Verkuil wrote:
> On 02/22/2018 10:49 AM, Hugues Fruchet wrote:
>> Fix lock scheme leading to spurious freeze.
> 
> Can you elaborate a bit more? It's hard to review since you don't
> describe what was wrong and why this fixes the problem.
> 
> Regards,
> 
>   Hans
> 
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>> version 2:
>>- dcmi_buf_queue() refactor to avoid to have "else" after "return"
>>  (warning detected by checkpatch.pl --strict -f stm32-dcmi.c)
>>
>>   drivers/media/platform/stm32/stm32-dcmi.c | 57 
>> +--
>>   1 file changed, 24 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 2fd8bed..5de18ad 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -197,7 +197,7 @@ static void dcmi_dma_callback(void *param)
>>  struct dma_tx_state state;
>>  enum dma_status status;
>>   
>> -spin_lock(>irqlock);
>> +spin_lock_irq(>irqlock);
>>   
>>  /* Check DMA status */
>>  status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
>> @@ -239,7 +239,7 @@ static void dcmi_dma_callback(void *param)
>>  dcmi->errors_count++;
>>  dcmi->active = NULL;
>>   
>> -spin_unlock(>irqlock);
>> +spin_unlock_irq(>irqlock);
>>  return;
>>  }
>>   
>> @@ -248,13 +248,11 @@ static void dcmi_dma_callback(void *param)
>>   
>>  list_del_init(>active->list);
>>   
>> -if (dcmi_start_capture(dcmi)) {
>> +spin_unlock_irq(>irqlock);

Lock is released here before calling dcmi_start_capture() which calls 
dma_async_issue_pending()

>> +if (dcmi_start_capture(dcmi))
>>  dev_err(dcmi->dev, "%s: Cannot restart capture 
>> on DMA complete\n",
>>  __func__);
>> -
>> -spin_unlock(>irqlock);
>> -return;
>> -}
>> +return;
>>  }
>>   
>>  break;
>> @@ -263,7 +261,7 @@ static void dcmi_dma_callback(void *param)
>>  break;
>>  }
>>   
>> -spin_unlock(>irqlock);
>> +spin_unlock_irq(>irqlock);
>>   }
>>   
>>   static int dcmi_start_dma(struct stm32_dcmi *dcmi,
>> @@ -360,7 +358,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>>   {
>>  struct stm32_dcmi *dcmi = arg;
>>   
>> -spin_lock(>irqlock);
>> +spin_lock_irq(>irqlock);
>>   
>>  /* Stop capture is required */
>>  if (dcmi->state == STOPPING) {
>> @@ -370,7 +368,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>>   
>>  complete(>complete);
>>   
>> -spin_unlock(>irqlock);
>> +spin_unlock_irq(>irqlock);
>>  return IRQ_HANDLED;
>>  }
>>   
>> @@ -383,35 +381,34 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>>   __func__);
>>   
>>  dcmi->errors_count++;
>> -dmaengine_terminate_all(dcmi->dma_chan);
>> -
>>  dev_dbg(dcmi->dev, "Restarting capture after DCMI error\n");
>>   
>> -if (dcmi_start_capture(dcmi)) {
>> +spin_unlock_irq(>irqlock);
Lock is released here before calling dma APIs (terminate then 
async_issue_pending)

>> +dmaengine_terminate_all(dcmi->dma_chan);
>> +
>> +if (dcmi_start_capture(dcmi))
>>  dev_err(dcmi->dev, "%s: Cannot restart capture on 
>> overflow or error\n",
>>  __func__);
>> -
>> -spin_unlock(>irqlock);
>> -return IRQ_HANDLED;
>> -}
>> +return IRQ_HANDLED;
>>  }
>>   
>> -spin_unlock(>irqlock);
>> +spin_unlock_irq(>irqlock);
>>  return IRQ_HANDLED;
>>   }
>>   
>>   static irqreturn_t dcmi_irq_callback(int irq, void *arg)
>>   {
>>  struct stm32_dcmi *dcmi = arg;
>> +unsigned long flags;
>>   
>> -spin_lock(>irqlock);
>> +spin_lock_irqsave(>irqlock, flags);
>>   
>>  dcmi->misr = reg_read(dcmi->regs, DCMI_MIS);
>>   
>>  /* Clear interrupt */
>>  reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR);
>>   
>> -spin_unlock(>irqlock);
>> +spin_unlock_irqrestore(>irqlock, flags);
>>   
>>  return IRQ_WAKE_THREAD;
>>   }
>> @@ -490,9 +487,8 @@ static void dcmi_buf_queue(struct 

Re: [PATCH v2 5/8] v4l: vsp1: Refactor display list configure operations

[Kieran Bingham]

Hi Laurent,

This series has a pending question below:

On 17/11/17 15:07, Kieran Bingham wrote:
> Hi Laurent,
> 
> Just a query on your bikeshedding here.
> 
> Choose your colours wisely :)
> 
> --
> Kieran
> 
> On 12/09/17 20:19, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> On Tuesday, 12 September 2017 00:16:50 EEST Kieran Bingham wrote:
>>> On 17/08/17 19:13, Laurent Pinchart wrote:
 On Monday 14 Aug 2017 16:13:28 Kieran Bingham wrote:
> The entities provide a single .configure operation which configures the
> object into the target display list, based on the vsp1_entity_params
> selection.
>
> This restricts us to a single function prototype for both static
> configuration (the pre-stream INIT stage) and the dynamic runtime stages
> for both each frame - and each partition therein.
>
> Split the configure function into two parts, '.prepare()' and
> '.configure()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
> VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
> .configure(). The configuration for individual partitions is handled by
> passing the partition number to the configure call, and processing any
> runtime stage actions on the first partition only.
>
> Signed-off-by: Kieran Bingham 
> ---
>
>  drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
>  drivers/media/platform/vsp1/vsp1_clu.c|  43 +--
>  drivers/media/platform/vsp1/vsp1_drm.c|  11 +-
>  drivers/media/platform/vsp1/vsp1_entity.c |  15 +-
>  drivers/media/platform/vsp1/vsp1_entity.h |  27 +--
>  drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
>  drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
>  drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
>  drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
>  drivers/media/platform/vsp1/vsp1_lut.c|  24 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c| 162 ++---
>  drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
>  drivers/media/platform/vsp1/vsp1_uds.c|  55 ++--
>  drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
>  drivers/media/platform/vsp1/vsp1_wpf.c| 297 ---
>  15 files changed, 359 insertions(+), 371 deletions(-)

 [snip]

> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> b/drivers/media/platform/vsp1/vsp1_clu.c index 175717018e11..5f65ce3ad97f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -213,37 +213,37 @@ static const struct v4l2_subdev_ops clu_ops = {
>
>  /*
>  ---
>  -
>  
>   * VSP1 Entity Operations
>   */
>
> +static void clu_prepare(struct vsp1_entity *entity,
> + struct vsp1_pipeline *pipe,
> + struct vsp1_dl_list *dl)
> +{
> + struct vsp1_clu *clu = to_clu(>subdev);
> +
> + /*
> +  * The format can't be changed during streaming, only verify it
> +  * at setup time and store the information internally for future
> +  * runtime configuration calls.
> +  */

 I know you're just moving the comment around, but let's fix it at the same
 time. There's no verification here (and no "setup time" either). I'd write
 it as

/*

 * The format can't be changed during streaming. Cache it internally
 * for future runtime configuration calls.
 */
>>>
>>> I think I'm ok with that and I've updated the patch - but I'm not sure we
>>> are really caching the 'format' here, as much as the yuv_mode ...
>>
>> Yes, it's the YUV mode we're caching, feel free to update the comment.
> 
> Done.
> 
>>
>>> I'll ponder ...
>>>
> + struct v4l2_mbus_framefmt *format;
> +
> + format = vsp1_entity_get_pad_format(>entity,
> + clu->entity.config,
> + CLU_PAD_SINK);
> + clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
> +}

 [snip]

> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
> b/drivers/media/platform/vsp1/vsp1_entity.h index
> 408602ebeb97..2f33e343ccc6 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h

 [snip]

> @@ -80,8 +68,10 @@ struct vsp1_route {
>
>  /**
>  
>   * struct vsp1_entity_operations - Entity operations
>   * @destroy: Destroy the entity.
>
> - * @configure:   Setup the hardware based on the entity state
> (pipeline, formats,
> - *   selection rectangles, ...)
> + * @prepare: Setup the initial hardware parameters for the stream
> (pipeline,
> + *   formats)
> + * @configure:   Configure the runtime 

Re: Patch review

[Guennadi Liakhovetski]

Hi Laurent,

Yes, that's correct.

Thanks
Guennadi

On Wed, 28 Feb 2018, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Wednesday, 28 February 2018 17:07:00 EET Guennadi Liakhovetski wrote:
> > Hi,
> > 
> > I know the "development process and responsibilities" was the main topic
> > during the last media summit. Unfortunately I haven't attended it, from
> > the etherpad notes I also cannot quite conclude what decisions have been
> > made. Have any measures been discussed and agreed upon for cases, when
> > patches don't get reviewed for many months, adding up to more than a year
> > (in this specific case the first version submitted in June 2016)?
> 
> I assume you're talking about the "[PATCH 0/2 v6] uvcvideo: asynchronous 
> controls" series, is that correct ?
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

[PATCH v2 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui

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

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 4 files changed, 1147 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

[PATCH v2 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..78f2be27a8f5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   boolis_enabled;
+   bool  

Re: Patch review

[Laurent Pinchart]

Hi Guennadi,

On Wednesday, 28 February 2018 17:07:00 EET Guennadi Liakhovetski wrote:
> Hi,
> 
> I know the "development process and responsibilities" was the main topic
> during the last media summit. Unfortunately I haven't attended it, from
> the etherpad notes I also cannot quite conclude what decisions have been
> made. Have any measures been discussed and agreed upon for cases, when
> patches don't get reviewed for many months, adding up to more than a year
> (in this specific case the first version submitted in June 2016)?

I assume you're talking about the "[PATCH 0/2 v6] uvcvideo: asynchronous 
controls" series, is that correct ?

-- 
Regards,

Laurent Pinchart

Patch review

[Guennadi Liakhovetski]

Hi,

I know the "development process and responsibilities" was the main topic 
during the last media summit. Unfortunately I haven't attended it, from 
the etherpad notes I also cannot quite conclude what decisions have been 
made. Have any measures been discussed and agreed upon for cases, when 
patches don't get reviewed for many months, adding up to more than a year 
(in this specific case the first version submitted in June 2016)?

Thanks
Guennadi

Re: [PATCH RFC] media: em28xx: don't use coherent buffer for DMA transfers

[Devin Heitmueller]

On Tue, Feb 27, 2018 at 12:42 PM, Mauro Carvalho Chehab
 wrote:
> While coherent memory is cheap on x86, it has problems on
> arm. So, stop using it.
>
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>
> I wrote this patch in order to check if this would make things better
> for ISOCH transfers on Raspberry Pi3. It didn't. Yet, keep using
> coherent memory at USB drivers seem an overkill.
>
> So, I'm actually not sure if we should either go ahead and merge it
> or not.
>
> Comments? Tests?

For what it's worth, while I haven't tested this patch you're
proposing, I've been running what is essentially the same change in a
private tree for several years in order for the device to work better
with several TI Davinci SOC platforms.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Sakari Ailus]

Hi Rob,

Thanks for the review.

On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote:
> On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh  wrote:
> > From: Alan Chiang 
> >
> > Dongwoon DW9807 is a voice coil lens driver.
> >
> > Also add a vendor prefix for Dongwoon for one did not exist previously.
> 
> Where's that?

Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't
relevant indeed and should be removed.

> 
> >
> > Signed-off-by: Andy Yeh 
> > ---
> >  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 
> > +
> 
> DACs generally go in bindings/iio/dac/

We have quite a few lens voice coil drivers under bindings/media/i2c now. I
don't really object to putting this one to bindings/iio/dac but then the
rest should be moved as well.

The camera LED flash drivers are under bindings/leds so this would actually
be analoguous to that. The lens voice coil drivers are perhaps still a bit
more bound to the domain (camera) than the LED flash drivers.

I can send a patch if you think the existing bindings should be moved; let
me know.

-- 
Kind regards,

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

[GIT FIXES FOR 4.16] Remove LIRC_CAN_SEND_SCANCODE

[Sean Young]

Hi Mauro,

Here is a fix to the documentation warning. LIRC_CAN_SEND_SCANCODE was
introduced in v4.16 to signify that kernel IR encoders could be used
for IR Tx; this lirc feature bit was found to trip up the lirc daemon,
so it was removed again but not from the lirc header.

See https://git.linuxtv.org/v4l-utils.git/tree/utils/ir-ctl/ir-ctl.c#n763
how IR Tx using scancodes can be done.

Thanks

Sean

The following changes since commit 7dbdd16a79a9d27d7dca0a49029fc8966dcfecc5:

  media: vb2: Makefile: place vb2-trace together with vb2-core (2018-02-26 
11:39:04 -0500)

are available in the Git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.16d

for you to fetch changes up to 1ddde79437bae627a7b0cadde6b88c544cbca285:

  media: rc: lirc does not use LIRC_CAN_SEND_SCANCODE feature (2018-02-28 
12:07:48 +)


Sean Young (1):
  media: rc: lirc does not use LIRC_CAN_SEND_SCANCODE feature

 include/uapi/linux/lirc.h | 1 -
 1 file changed, 1 deletion(-)

[PATCH for v4.16] tegra-cec: reset rx_buf_cnt when start bit detected

[Hans Verkuil]

If a start bit is detected, then reset the receive buffer counter to 0.

This ensures that no stale data is in the buffer if a message is
broken off midstream due to e.g. a Low Drive condition and then
retransmitted.

The only Rx interrupts we need to listen to are RX_REGISTER_FULL (i.e.
a valid byte was received) and RX_START_BIT_DETECTED (i.e. a new
message starts and we need to reset the counter).

Signed-off-by: Hans Verkuil 
Cc:   # for v4.15 and up
---
 drivers/media/platform/tegra-cec/tegra_cec.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c 
b/drivers/media/platform/tegra-cec/tegra_cec.c
index 92f93a880015..aba488cd0e64 100644
--- a/drivers/media/platform/tegra-cec/tegra_cec.c
+++ b/drivers/media/platform/tegra-cec/tegra_cec.c
@@ -172,16 +172,13 @@ static irqreturn_t tegra_cec_irq_handler(int irq, void 
*data)
}
}

-   if (status & (TEGRA_CEC_INT_STAT_RX_REGISTER_OVERRUN |
- TEGRA_CEC_INT_STAT_RX_BUS_ANOMALY_DETECTED |
- TEGRA_CEC_INT_STAT_RX_START_BIT_DETECTED |
- TEGRA_CEC_INT_STAT_RX_BUS_ERROR_DETECTED)) {
+   if (status & TEGRA_CEC_INT_STAT_RX_START_BIT_DETECTED) {
cec_write(cec, TEGRA_CEC_INT_STAT,
- (TEGRA_CEC_INT_STAT_RX_REGISTER_OVERRUN |
-  TEGRA_CEC_INT_STAT_RX_BUS_ANOMALY_DETECTED |
-  TEGRA_CEC_INT_STAT_RX_START_BIT_DETECTED |
-  TEGRA_CEC_INT_STAT_RX_BUS_ERROR_DETECTED));
-   } else if (status & TEGRA_CEC_INT_STAT_RX_REGISTER_FULL) {
+ TEGRA_CEC_INT_STAT_RX_START_BIT_DETECTED);
+   cec->rx_done = false;
+   cec->rx_buf_cnt = 0;
+   }
+   if (status & TEGRA_CEC_INT_STAT_RX_REGISTER_FULL) {
u32 v;

cec_write(cec, TEGRA_CEC_INT_STAT,
@@ -255,7 +252,7 @@ static int tegra_cec_adap_enable(struct cec_adapter *adap, 
bool enable)
  TEGRA_CEC_INT_MASK_TX_BUS_ANOMALY_DETECTED |
  TEGRA_CEC_INT_MASK_TX_FRAME_TRANSMITTED |
  TEGRA_CEC_INT_MASK_RX_REGISTER_FULL |
- TEGRA_CEC_INT_MASK_RX_REGISTER_OVERRUN);
+ TEGRA_CEC_INT_MASK_RX_START_BIT_DETECTED);

cec_write(cec, TEGRA_CEC_HW_CONTROL, TEGRA_CEC_HWCTRL_TX_RX_MODE);
return 0;
-- 
2.14.1

Re: [v2] [media] Use common error handling code in 20 functions

[Laurent Pinchart]

Hello,

On Wednesday, 28 February 2018 10:45:21 EET SF Markus Elfring wrote:
> >> +put_isp:
> >> +  omap3isp_put(video->isp);
> >> +delete_fh:
> >> +  v4l2_fh_del(>vfh);
> >> +  v4l2_fh_exit(>vfh);
> >> +  kfree(handle);
> > 
> > Please prefix the error labels with error_.
> 
> How often do you really need such an extra prefix?
> 
> >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> >> @@ -994,10 +994,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
> >> void *fh, struct v4l2_queryctrl qc = { .id = ctrl->id };
> >> 
> >>ret = uvc_query_v4l2_ctrl(chain, );
> >> 
> >> -  if (ret < 0) {
> >> -  ctrls->error_idx = i;
> >> -  return ret;
> >> -  }
> >> +  if (ret < 0)
> >> +  goto set_index;
> >> 
> >>ctrl->value = qc.default_value;
> >>
> >>}
> >> 
> >> @@ -1013,14 +1011,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file
> >> *file,
> >> void *fh, ret = uvc_ctrl_get(chain, ctrl);
> >> 
> >>if (ret < 0) {
> >>
> >>uvc_ctrl_rollback(handle);
> >> 
> >> -  ctrls->error_idx = i;
> >> -  return ret;
> >> +  goto set_index;
> >> 
> >>}
> >>
> >>}
> >>
> >>ctrls->error_idx = 0;
> >>
> >>return uvc_ctrl_rollback(handle);
> >> 
> >> +
> >> +set_index:
> >> +  ctrls->error_idx = i;
> >> +  return ret;
> >> 
> >>  }
> > 
> > For uvcvideo I find this to hinder readability
> 
> I got an other development view.
> 
> > without adding much added value.
> 
> There can be a small effect for such a function implementation.
> 
> > Please drop the uvcvideo change from this patch.
> 
> Would it be nice if this source code adjustment could be integrated also?

Just for the record, and to avoid merging this patch by mistake,

Nacked-by: Laurent Pinchart 

at least until the requested changes are implemented.

-- 
Regards,

Laurent Pinchart

Re: [v2] [media] Use common error handling code in 20 functions

[SF Markus Elfring]

>> +put_isp:
>> +omap3isp_put(video->isp);
>> +delete_fh:
>> +v4l2_fh_del(>vfh);
>> +v4l2_fh_exit(>vfh);
>> +kfree(handle);
> 
> Please prefix the error labels with error_.

How often do you really need such an extra prefix?


>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -994,10 +994,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
>> void *fh, struct v4l2_queryctrl qc = { .id = ctrl->id };
>>
>>  ret = uvc_query_v4l2_ctrl(chain, );
>> -if (ret < 0) {
>> -ctrls->error_idx = i;
>> -return ret;
>> -}
>> +if (ret < 0)
>> +goto set_index;
>>
>>  ctrl->value = qc.default_value;
>>  }
>> @@ -1013,14 +1011,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
>> void *fh, ret = uvc_ctrl_get(chain, ctrl);
>>  if (ret < 0) {
>>  uvc_ctrl_rollback(handle);
>> -ctrls->error_idx = i;
>> -return ret;
>> +goto set_index;
>>  }
>>  }
>>
>>  ctrls->error_idx = 0;
>>
>>  return uvc_ctrl_rollback(handle);
>> +
>> +set_index:
>> +ctrls->error_idx = i;
>> +return ret;
>>  }
> 
> For uvcvideo I find this to hinder readability

I got an other development view.


> without adding much added value.

There can be a small effect for such a function implementation.


> Please drop the uvcvideo change from this patch.

Would it be nice if this source code adjustment could be integrated also?

Regards,
Markus