[libcamera-devel] media: vimc: VIMC multiple streams

Kaaira Gupta kgupta at es.iitr.ac.in
Wed Jul 22 16:48:31 CEST 2020


On Wed, Jul 22, 2020 at 02:49:38PM +0100, Kieran Bingham wrote:
> Hi Kaaira,
> 
> On 22/07/2020 14:43, Kaaira Gupta wrote:
> > This is a patchset by Niklas (https://patchwork.kernel.org/cover/10948831/)
> > that I have rebased on the current media-tree, I have not added anything
> > to it and it is just a compressed version of his patchset into a single
> > patch, based on the latest master to aid testing multiple stream support
> > in the VIMC pipeline.
> > 
> > Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
> 
> Great, it's good to hear that Niklas' patches still work and are simple
> to rebase.
> 
> I think his patches would need to be kept separate though, and submitted

Yes agreed, but I sent it here just for it to become easier for others to
review and test the patchset of multiples streams in the pipeline. I did not
send it as a patch, rather an option to help implement multiple streams
support in the driver easily, and hence test the previous patchset :-D

> to the linux kernel through the linux-media mailing list, with any
> comments from the previous series addressed.
> 
> You can keep Niklas' patches and his authorship, but add your signed-off
> by tag, and document any changes you make in the commit message before
> resubmitting to linux-media.

I did not make any changes to the original patchset, as there are
some requirements (like if the counter for stream start/stop will be in
the driver or in v4l2-common or someplace else) of which I am not sure
yet.

> 
> 
> Then when we get to a point that the kernel can support multiple
> streams, we can then enable that support in libcamera (we might have to
> add a variable that enables multiple streams based on the kernel
> version, but lets see how it goes).
> 
> 
> --
> Kieran

Thanks,
Kaaira

> 
> 
> 
> > ---
> >  .../media/test-drivers/vimc/vimc-capture.c    | 35 ++++++++++++++++++-
> >  .../media/test-drivers/vimc/vimc-debayer.c    |  8 +++++
> >  drivers/media/test-drivers/vimc/vimc-scaler.c |  8 +++++
> >  drivers/media/test-drivers/vimc/vimc-sensor.c |  8 +++++
> >  .../media/test-drivers/vimc/vimc-streamer.c   | 22 +++++++-----
> >  5 files changed, 71 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/media/test-drivers/vimc/vimc-capture.c b/drivers/media/test-drivers/vimc/vimc-capture.c
> > index c63496b17b9a..5fc56582af83 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-capture.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-capture.c
> > @@ -237,16 +237,49 @@ static void vimc_cap_return_all_buffers(struct vimc_cap_device *vcap,
> >  	spin_unlock(&vcap->qlock);
> >  }
> >  
> > +static struct media_entity *vimc_cap_get_sensor(struct vimc_cap_device *vcap)
> > +{
> > +	struct media_entity *entity = &vcap->vdev.entity;
> > +	struct media_device *mdev = entity->graph_obj.mdev;
> > +	struct media_graph graph;
> > +
> > +	mutex_lock(&mdev->graph_mutex);
> > +	if (media_graph_walk_init(&graph, mdev)) {
> > +		mutex_unlock(&mdev->graph_mutex);
> > +		return NULL;
> > +	}
> > +
> > +	media_graph_walk_start(&graph, entity);
> > +
> > +	while ((entity = media_graph_walk_next(&graph)))
> > +		if (entity->function == MEDIA_ENT_F_CAM_SENSOR)
> > +			break;
> > +
> > +	mutex_unlock(&mdev->graph_mutex);
> > +
> > +	media_graph_walk_cleanup(&graph);
> > +
> > +	return entity;
> > +}
> > +
> >  static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
> >  {
> >  	struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
> >  	struct media_entity *entity = &vcap->vdev.entity;
> > +	struct media_pipeline *pipe = NULL;
> > +	struct media_entity *sensorent;
> >  	int ret;
> >  
> >  	vcap->sequence = 0;
> >  
> >  	/* Start the media pipeline */
> > -	ret = media_pipeline_start(entity, &vcap->stream.pipe);
> > +	sensorent = vimc_cap_get_sensor(vcap);
> > +	if (sensorent && sensorent->pipe)
> > +		pipe = sensorent->pipe;
> > +	else
> > +		pipe = &vcap->stream.pipe;
> > +
> > +	ret = media_pipeline_start(entity, pipe);
> >  	if (ret) {
> >  		vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
> >  		return ret;
> > diff --git a/drivers/media/test-drivers/vimc/vimc-debayer.c b/drivers/media/test-drivers/vimc/vimc-debayer.c
> > index c3f6fef34f68..93fe19d8d2b4 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-debayer.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-debayer.c
> > @@ -29,6 +29,7 @@ struct vimc_deb_pix_map {
> >  struct vimc_deb_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> > +	atomic_t use_count;
> >  	/* The active format */
> >  	struct v4l2_mbus_framefmt sink_fmt;
> >  	u32 src_code;
> > @@ -343,6 +344,9 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct vimc_pix_map *vpix;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vdeb->use_count) != 1)
> > +			return 0;
> > +
> >  		if (vdeb->src_frame)
> >  			return 0;
> >  
> > @@ -368,6 +372,9 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> >  			return -ENOMEM;
> >  
> >  	} else {
> > +		if (atomic_dec_return(&vdeb->use_count) != 0)
> > +			return 0;
> > +
> >  		if (!vdeb->src_frame)
> >  			return 0;
> >  
> > @@ -595,6 +602,7 @@ static struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> >  	vdeb->ved.process_frame = vimc_deb_process_frame;
> >  	vdeb->ved.dev = vimc->mdev.dev;
> >  	vdeb->mean_win_size = vimc_deb_ctrl_mean_win_size.def;
> > +	atomic_set(&vdeb->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vdeb->sink_fmt = sink_fmt_default;
> > diff --git a/drivers/media/test-drivers/vimc/vimc-scaler.c b/drivers/media/test-drivers/vimc/vimc-scaler.c
> > index 121fa7d62a2e..9b8458dbe57c 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-scaler.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-scaler.c
> > @@ -25,6 +25,7 @@ MODULE_PARM_DESC(sca_mult, " the image size multiplier");
> >  struct vimc_sca_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> > +	atomic_t use_count;
> >  	/* NOTE: the source fmt is the same as the sink
> >  	 * with the width and hight multiplied by mult
> >  	 */
> > @@ -340,6 +341,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct vimc_pix_map *vpix;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vsca->use_count) != 1)
> > +			return 0;
> > +
> >  		if (vsca->src_frame)
> >  			return 0;
> >  
> > @@ -363,6 +367,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
> >  			return -ENOMEM;
> >  
> >  	} else {
> > +		if (atomic_dec_return(&vsca->use_count) != 0)
> > +			return 0;
> > +
> >  		if (!vsca->src_frame)
> >  			return 0;
> >  
> > @@ -506,6 +513,7 @@ static struct vimc_ent_device *vimc_sca_add(struct vimc_device *vimc,
> >  
> >  	vsca->ved.process_frame = vimc_sca_process_frame;
> >  	vsca->ved.dev = vimc->mdev.dev;
> > +	atomic_set(&vsca->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vsca->sink_fmt = sink_fmt_default;
> > diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
> > index a2f09ac9a360..d68113886290 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
> > @@ -18,6 +18,7 @@ struct vimc_sen_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> >  	struct tpg_data tpg;
> > +	atomic_t use_count;
> >  	u8 *frame;
> >  	/* The active format */
> >  	struct v4l2_mbus_framefmt mbus_format;
> > @@ -201,6 +202,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct vimc_pix_map *vpix;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vsen->use_count) != 1)
> > +			return 0;
> > +
> >  		/* Calculate the frame size */
> >  		vpix = vimc_pix_map_by_code(vsen->mbus_format.code);
> >  		frame_size = vsen->mbus_format.width * vpix->bpp *
> > @@ -219,6 +223,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> >  
> >  	} else {
> >  
> > +		if (atomic_dec_return(&vsen->use_count) != 0)
> > +			return 0;
> > +
> >  		vfree(vsen->frame);
> >  		vsen->frame = NULL;
> >  	}
> > @@ -359,6 +366,7 @@ static struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
> >  
> >  	vsen->ved.process_frame = vimc_sen_process_frame;
> >  	vsen->ved.dev = vimc->mdev.dev;
> > +	atomic_set(&vsen->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vsen->mbus_format = fmt_default;
> > diff --git a/drivers/media/test-drivers/vimc/vimc-streamer.c b/drivers/media/test-drivers/vimc/vimc-streamer.c
> > index 451a32c0d034..d5b966a99f7b 100644
> > --- a/drivers/media/test-drivers/vimc/vimc-streamer.c
> > +++ b/drivers/media/test-drivers/vimc/vimc-streamer.c
> > @@ -192,33 +192,35 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
> >  			   struct vimc_ent_device *ved,
> >  			   int enable)
> >  {
> > +	static DEFINE_MUTEX(vimc_streamer_lock);
> >  	int ret;
> >  
> >  	if (!stream || !ved)
> >  		return -EINVAL;
> >  
> > +	ret = mutex_lock_interruptible(&vimc_streamer_lock);
> > +	if (ret)
> > +		return ret;
> > +
> >  	if (enable) {
> >  		if (stream->kthread)
> > -			return 0;
> > +			goto out;
> >  
> >  		ret = vimc_streamer_pipeline_init(stream, ved);
> >  		if (ret)
> > -			return ret;
> > +			goto out;
> >  
> >  		stream->kthread = kthread_run(vimc_streamer_thread, stream,
> >  					      "vimc-streamer thread");
> >  
> >  		if (IS_ERR(stream->kthread)) {
> >  			ret = PTR_ERR(stream->kthread);
> > -			dev_err(ved->dev, "kthread_run failed with %d\n", ret);
> > -			vimc_streamer_pipeline_terminate(stream);
> > -			stream->kthread = NULL;
> > -			return ret;
> > +			goto out;
> >  		}
> >  
> >  	} else {
> >  		if (!stream->kthread)
> > -			return 0;
> > +			goto out;
> >  
> >  		ret = kthread_stop(stream->kthread);
> >  		/*
> > @@ -228,12 +230,14 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
> >  		 * pipeline.
> >  		 */
> >  		if (ret)
> > -			dev_dbg(ved->dev, "kthread_stop returned '%d'\n", ret);
> > +			goto out;
> >  
> >  		stream->kthread = NULL;
> >  
> >  		vimc_streamer_pipeline_terminate(stream);
> >  	}
> > +out:
> > +	mutex_unlock(&vimc_streamer_lock);
> >  
> > -	return 0;
> > +	return ret;
> >  }
> > 
> 
> -- 
> Regards
> --
> Kieran


More information about the libcamera-devel mailing list