From patchwork Fri Jan 22 00:57:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 8086501 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: X-Original-To: patchwork-linux-renesas-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id D791DBEEE5 for ; Fri, 22 Jan 2016 00:58:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C76BF2034C for ; Fri, 22 Jan 2016 00:58:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AECAC20443 for ; Fri, 22 Jan 2016 00:58:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751648AbcAVA6G (ORCPT ); Thu, 21 Jan 2016 19:58:06 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:58730 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751652AbcAVA6F (ORCPT ); Thu, 21 Jan 2016 19:58:05 -0500 Received: from avalon.access.network (nblzone-211-213.nblnetworks.fi [83.145.211.213]) by galahad.ideasonboard.com (Postfix) with ESMTPSA id E66892005A for ; Fri, 22 Jan 2016 01:56:59 +0100 (CET) From: Laurent Pinchart To: linux-renesas-soc@vger.kernel.org Subject: [PATCH/RFC v2 56/56] v4l: vsp1: Allocate pipelines on demand Date: Fri, 22 Jan 2016 02:57:25 +0200 Message-Id: <1453424245-18782-57-git-send-email-laurent.pinchart+renesas@ideasonboard.com> X-Mailer: git-send-email 2.4.10 In-Reply-To: <1453424245-18782-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> References: <1453424245-18782-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Instead of embedding pipelines in the vsp1_video objects allocate them on demand when they are needed. This prepares for support of the request API where the pipeline can differ from request to request. As a side effects it fixes the streamon race condition where pipelines objects from different video nodes could be used for the same pipeline. Signed-off-by: Laurent Pinchart --- drivers/media/platform/vsp1/vsp1_bru.c | 1 + drivers/media/platform/vsp1/vsp1_drv.c | 1 + drivers/media/platform/vsp1/vsp1_pipe.c | 1 + drivers/media/platform/vsp1/vsp1_pipe.h | 5 +- drivers/media/platform/vsp1/vsp1_rpf.c | 1 + drivers/media/platform/vsp1/vsp1_video.c | 109 +++++++++++++++++-------------- drivers/media/platform/vsp1/vsp1_video.h | 2 - drivers/media/platform/vsp1/vsp1_wpf.c | 1 + 8 files changed, 68 insertions(+), 53 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c index f46fd79fc3be..41c970bf5f74 100644 --- a/drivers/media/platform/vsp1/vsp1_bru.c +++ b/drivers/media/platform/vsp1/vsp1_bru.c @@ -19,6 +19,7 @@ #include "vsp1.h" #include "vsp1_bru.h" #include "vsp1_dl.h" +#include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_video.h" diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index 0232057504d6..6b0e3e3382c3 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -30,6 +30,7 @@ #include "vsp1_hsit.h" #include "vsp1_lif.h" #include "vsp1_lut.h" +#include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_sru.h" #include "vsp1_uds.h" diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c index 6c590b857812..b661d8e76a3a 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.c +++ b/drivers/media/platform/vsp1/vsp1_pipe.c @@ -189,6 +189,7 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe) mutex_init(&pipe->lock); spin_lock_init(&pipe->irqlock); init_waitqueue_head(&pipe->wq); + kref_init(&pipe->kref); INIT_LIST_HEAD(&pipe->entities); pipe->state = VSP1_PIPELINE_STOPPED; diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index 5ef1a7f374d6..953c473dca8f 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -13,6 +13,7 @@ #ifndef __VSP1_PIPE_H__ #define __VSP1_PIPE_H__ +#include #include #include #include @@ -63,7 +64,7 @@ enum vsp1_pipeline_state { * @wq: work queue to wait for state change completion * @frame_end: frame end interrupt handler * @lock: protects the pipeline use count and stream count - * @use_count: number of video nodes using the pipeline + * @kref: pipeline reference count * @stream_count: number of streaming video nodes * @buffers_ready: bitmask of RPFs and WPFs with at least one buffer available * @num_inputs: number of RPFs @@ -88,7 +89,7 @@ struct vsp1_pipeline { void (*frame_end)(struct vsp1_pipeline *pipe); struct mutex lock; - unsigned int use_count; + struct kref kref; unsigned int stream_count; unsigned int buffers_ready; diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c index 13a853664853..0d6e6f89990f 100644 --- a/drivers/media/platform/vsp1/vsp1_rpf.c +++ b/drivers/media/platform/vsp1/vsp1_rpf.c @@ -17,6 +17,7 @@ #include "vsp1.h" #include "vsp1_dl.h" +#include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_video.h" diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index 7757ad6e9a61..3a4de7d5dbd5 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -377,12 +377,9 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, { struct media_entity_graph graph; struct media_entity *entity = &video->video.entity; - struct media_device *mdev = entity->parent; unsigned int i; int ret; - mutex_lock(&mdev->graph_mutex); - /* Walk the graph to locate the entities and video nodes. */ media_entity_graph_walk_start(&graph, entity); @@ -415,13 +412,9 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, } } - mutex_unlock(&mdev->graph_mutex); - /* We need one output and at least one input. */ - if (pipe->num_inputs == 0 || !pipe->output) { - ret = -EPIPE; - goto error; - } + if (pipe->num_inputs == 0 || !pipe->output) + return -EPIPE; /* Follow links downstream for each input and make sure the graph * contains no loop and that all branches end at the output WPF. @@ -434,47 +427,76 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, pipe->inputs[i].rpf, pipe->output); if (ret < 0) - goto error; + return ret; } return 0; - -error: - vsp1_pipeline_reset(pipe); - return ret; } static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe, struct vsp1_video *video) { + vsp1_pipeline_init(pipe); + + pipe->frame_end = vsp1_video_pipeline_frame_end; + + return vsp1_video_pipeline_build(pipe, video); +} + +static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video) +{ + struct media_device *mdev = &video->vsp1->media_dev; + struct vsp1_pipeline *pipe; int ret; - mutex_lock(&pipe->lock); + /* Get a pipeline object for the video node. If a pipeline has already + * been allocated just increment its reference count and return it. + * Otherwise allocate a new pipeline and initialize it, it will be freed + * when the last reference is released. + */ + mutex_lock(&mdev->graph_mutex); - /* If we're the first user build and validate the pipeline. */ - if (pipe->use_count == 0) { - ret = vsp1_video_pipeline_build(pipe, video); - if (ret < 0) + if (!video->rwpf->pipe) { + pipe = kzalloc(sizeof(*pipe), GFP_KERNEL); + if (!pipe) { + pipe = ERR_PTR(-ENOMEM); goto done; + } + + ret = vsp1_video_pipeline_init(pipe, video); + if (ret < 0) { + vsp1_pipeline_reset(pipe); + kfree(pipe); + pipe = ERR_PTR(ret); + goto done; + } + } else { + pipe = video->rwpf->pipe; + kref_get(&pipe->kref); } - pipe->use_count++; - ret = 0; + /* TODO: Make sure links can't be touched anymore. */ done: - mutex_unlock(&pipe->lock); - return ret; + mutex_unlock(&mdev->graph_mutex); + return pipe; } -static void vsp1_video_pipeline_cleanup(struct vsp1_pipeline *pipe) +static void vsp1_video_pipeline_release(struct kref *kref) { - mutex_lock(&pipe->lock); + struct vsp1_pipeline *pipe = container_of(kref, typeof(*pipe), kref); - /* If we're the last user clean up the pipeline. */ - if (--pipe->use_count == 0) - vsp1_pipeline_reset(pipe); + vsp1_pipeline_reset(pipe); + kfree(pipe); +} - mutex_unlock(&pipe->lock); +static void vsp1_video_pipeline_put(struct vsp1_pipeline *pipe) +{ + struct media_device *mdev = &pipe->output->entity.vsp1->media_dev; + + mutex_lock(&mdev->graph_mutex); + kref_put(&pipe->kref, vsp1_video_pipeline_release); + mutex_unlock(&mdev->graph_mutex); } /* ----------------------------------------------------------------------------- @@ -651,8 +673,8 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq) } mutex_unlock(&pipe->lock); - vsp1_video_pipeline_cleanup(pipe); media_entity_pipeline_stop(&video->video.entity); + vsp1_video_pipeline_put(pipe); /* Remove all buffers from the IRQ queue. */ spin_lock_irqsave(&video->irqlock, flags); @@ -770,20 +792,16 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type) video->sequence = 0; + pipe = vsp1_video_pipeline_get(video); + if (IS_ERR(pipe)) + return PTR_ERR(pipe); + /* Start streaming on the pipeline. No link touching an entity in the * pipeline can be activated or deactivated once streaming is started. - * - * Use the VSP1 pipeline object embedded in the first video object that - * starts streaming. - * - * FIXME: This is racy, the ioctl is only protected by the video node - * lock. */ - pipe = video->rwpf->pipe ? video->rwpf->pipe : &video->pipe; - ret = media_entity_pipeline_start(&video->video.entity, &pipe->pipe); if (ret < 0) - return ret; + goto err_pipe; /* Verify that the configured format matches the output of the connected * subdev. @@ -792,21 +810,17 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type) if (ret < 0) goto err_stop; - ret = vsp1_video_pipeline_init(pipe, video); - if (ret < 0) - goto err_stop; - /* Start the queue. */ ret = vb2_streamon(&video->queue, type); if (ret < 0) - goto err_cleanup; + goto err_stop; return 0; -err_cleanup: - vsp1_video_pipeline_cleanup(pipe); err_stop: media_entity_pipeline_stop(&video->video.entity); +err_pipe: + vsp1_video_pipeline_put(pipe); return ret; } @@ -922,9 +936,6 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1, spin_lock_init(&video->irqlock); INIT_LIST_HEAD(&video->irqqueue); - vsp1_pipeline_init(&video->pipe); - video->pipe.frame_end = vsp1_video_pipeline_frame_end; - /* Initialize the media entity... */ ret = media_entity_init(&video->video.entity, 1, &video->pad, 0); if (ret < 0) diff --git a/drivers/media/platform/vsp1/vsp1_video.h b/drivers/media/platform/vsp1/vsp1_video.h index 64abd39ee1e7..867b00807c46 100644 --- a/drivers/media/platform/vsp1/vsp1_video.h +++ b/drivers/media/platform/vsp1/vsp1_video.h @@ -18,7 +18,6 @@ #include -#include "vsp1_pipe.h" #include "vsp1_rwpf.h" struct vsp1_vb2_buffer { @@ -44,7 +43,6 @@ struct vsp1_video { struct mutex lock; - struct vsp1_pipeline pipe; unsigned int pipe_index; struct vb2_queue queue; diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c index a7d94a76032c..179f21035765 100644 --- a/drivers/media/platform/vsp1/vsp1_wpf.c +++ b/drivers/media/platform/vsp1/vsp1_wpf.c @@ -17,6 +17,7 @@ #include "vsp1.h" #include "vsp1_dl.h" +#include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_video.h"