cron job: media_tree daily build: ERRORS
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
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(&client->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(&client->dev); 457 else 458 chip_id = id->driver_data; 459 460 data->chip = &video_i2c_chip[chip_id]; 461 data->client = client; 462 v4l2_dev = &data->v4l2_dev; 463 strlcpy(v4l2_dev->name, VIDEO_I2C_DRIVER, sizeof(v4l2_dev->name)); 464 465 ret = v4l2_device_register(&client->dev, v4l2_dev); 466 if (ret < 0) 467 goto error_free_device; 468 469 mutex_init(&data->lock); 470 mutex_init(&data->queue_lock); 471 472 queue = &data->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 = &video_i2c_video_qops; 480 queue->mem_ops = &vb2_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 = &data->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 = &video_i2c_fops; 495 data->vdev.lock = &data->lock; 496 data->vdev.ioctl_ops = &video_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(&data->slock); 502 INIT_LIST_HEAD(&data->vid_cap_active); 503 504 video_set_drvdata(&data->vdev, data); 505 i2c_set_clientdata(client, data); 506 507 ret = video_register_device(&data->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
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
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
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
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 - * &vb2_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 &vb2_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 &buf_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 &enum 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 &vb2_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 + * &vb2_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 &vb2_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
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 &vb2_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 &vb2_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
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(&entity->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(&clu->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 ent
[PATCH v6 1/9] v4l: vsp1: Reword uses of 'fragment' as 'body'
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(&clu->lock); swap(clu->clu, dlb); spin_unlock_irq(&clu->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(&clu->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) { struct vsp1_dl_body *dlb; @@ -18
[PATCH v6 5/9] v4l: vsp1: Use reference counting for bodies
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(&clu->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(&pool->free)) { dlb = list_first_entry(&pool->free, struct vsp1_dl_body, free); list_del(&dlb->free); + refcount_set(&dlb->refcnt, 1); } spin_unlock_irqrestore(&pool->lock, flags); @@ -279,6 +283,9 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb) if (!dlb) return; + if (!refcount_dec_and_test(&dlb->refcnt)) + return; + dlb->num_entries = 0; spin_lock_irqsave(&dlb->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(&dlb->refcnt); + list_add_tail(&dlb->list, &dl->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(&lut->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
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 = dma_alloc_wc(vsp1->bus_master, pool->size, &pool->dma, +
[PATCH v6 9/9] v4l: vsp1: Reduce display list body size
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
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(&entity->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), ctrl); + vsp1_bru_wri
[PATCH v6 0/9] vsp1: TLB optimisation and DL caching
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 vf
[PATCH v6 8/9] v4l: vsp1: Move video configuration to a cached dlb
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(&pipe->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(&pipe->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 vsp1_partition *partition; s
[PATCH v6 2/9] v4l: vsp1: Protect bodies against overflow
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, &dlb->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
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(&clu->lock); swap(clu->clu, dlb); spin_unlock_irq(&clu->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(&entity->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(&clu->ctrls, 2); v4l2_ctrl_new_custom(&clu->ctrls, &clu_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 - * @gc_work: bo
[PATCH v6 6/9] v4l: vsp1: Refactor display list configure operations
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(&entity->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(&bru->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(&entity->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(&clu->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(&entity->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; - - format =
Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
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()
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
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
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 = &buf->vb; + + vbuf->sequence = dcmi->sequence++; + vbuf->field = V4L2_FIELD_NONE; + vbuf->vb2_buf.timestamp = ktime_get_ns(); + vb2_set_plane_payload(&vbuf->vb2_buf, 0, bytesused); + vb2_buffer_done(&vbuf->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(&dcmi->irqlock); + + if (dcmi->state != RUNNING) { + spin_unlock_irq(&dcmi->irqlock); + return -EINVAL; + } + + /* Restart a new DMA transfer with next buffer */ + if (list_empty(&dcmi->buffers)) { + dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture buffer\n", + __func__); + dcmi->errors_count++; + dcmi->active = NULL; + + spin_unlock_irq(&dcmi->irqlock); + return -EINVAL; + } + + dcmi->active = list_entry(dcmi->buffers.next, + struct dcmi_buf, list); + list_del_init(&dcmi->active->list); + + spin_unlock_irq(&dcmi->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(&dcmi->irqlock); + struct dcmi_buf *buf = dcmi->active; /* Check DMA status */ status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state); @@ -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 = &dcmi->active->vb; - - vbuf->sequence = dcmi->sequence++; - vbuf->field = V4L2_FIELD_NONE; - vbuf->vb2_buf.timestamp = ktime_get_ns(); - vb2_set_plane_payload(&vbuf->vb2_buf, 0, buf->size); - vb2_buffer_done(&vbuf->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(&dcmi->buffers)) { - dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture buffer\n", - __func__); - dcmi->errors_count++; - dcmi->active = NULL; - - spin_unlock_irq(&dcmi->irqlock); - return; - } - - dcmi->active = list_entry(dcmi->buffers.next, - struct dcmi_buf, list); - - list_del_init(&dcmi->active->list); - - spin_unlock_irq(&dcmi->irqlock); - if (dcmi_start_capture(dcmi)) - dev_err(dcmi->dev, "%s:
Re: [PATCH] media: stm32-dcmi: add JPEG support
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 = &buf->vb; >> + >> +vbuf->sequence = dcmi->sequence++; >> +vbuf->field = V4L2_FIELD_NONE; >> +vbuf->vb2_buf.timestamp = ktime_get_ns(); >> +vb2_set_plane_payload(&vbuf->vb2_buf, 0, bytesused); >> +vb2_buffer_done(&vbuf->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(&dcmi->irqlock); >> + >> +if (dcmi->state != RUNNING) { >> +spin_unlock_irq(&dcmi->irqlock); >> +return -EINVAL; >> +} >> + >> +/* Restart a new DMA transfer with next buffer */ >> +if (list_empty(&dcmi->buffers)) { >> +dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture >> buffer\n", >> +__func__); >> +dcmi->errors_count++; >> +dcmi->active = NULL; >> + >> +spin_unlock_irq(&dcmi->irqlock); >> +return -EINVAL; >> +} >> + >> +dcmi->active = list_entry(dcmi->buffers.next, >> + struct dcmi_buf, list); >> +list_del_init(&dcmi->active->list); >> + >> +spin_unlock_irq(&dcmi->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(&dcmi->irqlock); >> +struct dcmi_buf *buf = dcmi->active; >> >> /* Check DMA status */ >> status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state); >> @@ -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 = &dcmi->active->vb; >> - >> -vbuf->sequence = dcmi->sequence++; >> -vbuf->field = V4L2_FIELD_NONE; >> -vbuf->vb2_buf.timestamp = ktime_get_ns(); >> -vb2_set_plane_payload(&vbuf->vb2_buf, 0, buf->size); >> -vb2_buffer_done(&vbuf->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(&dcmi->buffers)) { >> -dev_err(dcmi->dev, "%s: No more buffer queued, >> cannot capture buffer\n", >> -__func__); >> -dcmi->errors_count++; >> -dcmi->active = NULL; >> - >> -spin_unlock_irq(&dcmi->irql
Re: [PATCH v2] media: stm32-dcmi: fix lock scheme
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(&dcmi->irqlock); >> +spin_lock_irq(&dcmi->irqlock); >> >> /* Check DMA status */ >> status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state); >> @@ -239,7 +239,7 @@ static void dcmi_dma_callback(void *param) >> dcmi->errors_count++; >> dcmi->active = NULL; >> >> -spin_unlock(&dcmi->irqlock); >> +spin_unlock_irq(&dcmi->irqlock); >> return; >> } >> >> @@ -248,13 +248,11 @@ static void dcmi_dma_callback(void *param) >> >> list_del_init(&dcmi->active->list); >> >> -if (dcmi_start_capture(dcmi)) { >> +spin_unlock_irq(&dcmi->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(&dcmi->irqlock); >> -return; >> -} >> +return; >> } >> >> break; >> @@ -263,7 +261,7 @@ static void dcmi_dma_callback(void *param) >> break; >> } >> >> -spin_unlock(&dcmi->irqlock); >> +spin_unlock_irq(&dcmi->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(&dcmi->irqlock); >> +spin_lock_irq(&dcmi->irqlock); >> >> /* Stop capture is required */ >> if (dcmi->state == STOPPING) { >> @@ -370,7 +368,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) >> >> complete(&dcmi->complete); >> >> -spin_unlock(&dcmi->irqlock); >> +spin_unlock_irq(&dcmi->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(&dcmi->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(&dcmi->irqlock); >> -return IRQ_HANDLED; >> -} >> +return IRQ_HANDLED; >> } >> >> -spin_unlock(&dcmi->irqlock); >> +spin_unlock_irq(&dcmi->irqlock); >> return IRQ_HANDLED; >> } >> >> static irqreturn_t dcmi_irq_callback(int irq, void *arg) >> { >> struct stm32_dcmi *dcmi = arg; >> +unsigned long flags; >> >> -spin_lock(&dcmi->irqlock); >> +spin_lock_irqsave(&dcmi->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(&dcmi->irqlock); >> +spin_unlock_irqrestore(&dcmi->irqlock,
Re: [PATCH v2 5/8] v4l: vsp1: Refactor display list configure operations
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(&entity->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(&clu->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 parameters for each partition
Re: Patch review
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
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
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: + +&i2c2 { + ov2680: camera-sensor@36 { + compatible = "ovti,ov2680"; + reg = <0x36>; + clocks = <&osc>; + clock-names = "xvclk"; + powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>; + + port { + ov2680_mipi_ep: endpoint { + remote-endpoint = <&mipi_sensor_ep>; + clock-lanes = <0>; + data-lanes = <1>; + }; + }; + }; +}; -- 2.16.2
[PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
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; + boolis_streaming; + +
Re: Patch review
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
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
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
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
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
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
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(&handle->vfh); > >> + v4l2_fh_exit(&handle->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, &qc); > >> > >> - 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
>> +put_isp: >> +omap3isp_put(video->isp); >> +delete_fh: >> +v4l2_fh_del(&handle->vfh); >> +v4l2_fh_exit(&handle->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, &qc); >> -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