linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor
@ 2019-05-18  1:07 Niklas Söderlund
  2019-05-18  1:07 ` [PATCH 1/3] vimc: Add usage count to subdevices Niklas Söderlund
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Niklas Söderlund @ 2019-05-18  1:07 UTC (permalink / raw)
  To: Helen Koike, linux-media; +Cc: libcamera-devel, Niklas Söderlund

Hi,

This series adds support for two (or more) capture devices to be 
connected to the same senors and run simultaneously. Each capture device 
can be started and stopped independent of each other.

Patch 1/3 and 2/3 deals with solving the issues that arises once two 
capture devices can be part of the same pipeline. While 3/3 allows for 
two capture devices to be part of the same pipeline and thus allows for 
simultaneously use.

The series is based on the latest media-tree and it functionality can be 
demonstrated with the following test.

>>> begin test <<<
mdev=/dev/media0

media-ctl -d $mdev -l "'Debayer A':1 -> 'Scaler':0 [1]"
media-ctl -d $mdev -l "'Debayer B':1 -> 'Scaler':0 [0]"

media-ctl -d $mdev -V "'Debayer A':0 [fmt:RGB888_1X24/640x480 field:none]"
media-ctl -d /dev/media0 -V "'Sensor A':0 [fmt:SRGGB8_1X8/640x480 field:none]"

yavta -f RGB24 -s 1920x1440 --field none /dev/video2
yavta -f SRGGB8 -s 640x480 --field none /dev/video0

yavta -f RGB24 -s 1920x1440 --field none /dev/video2 --capture=100 &
yavta -f SRGGB8 -s 640x480 --field none /dev/video0 --capture=100
wait
>>> end test <<<

In addition to testing with this test the series is tested with multiple 
qv4l2 instances controlling different capture devices connected to the 
same sensor.

Niklas Söderlund (3):
  vimc: Add usage count to subdevices
  vimc: Serialize vimc_streamer_s_stream()
  vimc: Join pipeline if one already exists

 drivers/media/platform/vimc/vimc-capture.c  | 35 ++++++++++++++++++++-
 drivers/media/platform/vimc/vimc-debayer.c  |  8 +++++
 drivers/media/platform/vimc/vimc-scaler.c   |  8 +++++
 drivers/media/platform/vimc/vimc-sensor.c   |  7 +++++
 drivers/media/platform/vimc/vimc-streamer.c | 23 +++++++++-----
 5 files changed, 73 insertions(+), 8 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] vimc: Add usage count to subdevices
  2019-05-18  1:07 [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Niklas Söderlund
@ 2019-05-18  1:07 ` Niklas Söderlund
  2020-03-16 19:40   ` Helen Koike
  2019-05-18  1:07 ` [PATCH 2/3] vimc: Serialize vimc_streamer_s_stream() Niklas Söderlund
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2019-05-18  1:07 UTC (permalink / raw)
  To: Helen Koike, linux-media
  Cc: libcamera-devel, Niklas Söderlund, Niklas Söderlund

Prepare for multiple video streams from the same sensor by adding a use
counter to each subdevice. The counter is increase for every s_stream(1)
and decremented for every s_stream(0) call.

The subdevice stream is not started or stopped unless the usage count go
from 0 to 1 (started) or from 1 to 0 (stopped). This allow for multiple
s_stream() calls to try to either start or stop the device while only
the first/last call will actually effect the state of the device.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 drivers/media/platform/vimc/vimc-debayer.c | 8 ++++++++
 drivers/media/platform/vimc/vimc-scaler.c  | 8 ++++++++
 drivers/media/platform/vimc/vimc-sensor.c  | 7 +++++++
 3 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 281f9c1a7249ad1d..624fc23ce3077d40 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -56,6 +56,7 @@ struct vimc_deb_device {
 	struct vimc_ent_device ved;
 	struct v4l2_subdev sd;
 	struct device *dev;
+	atomic_t use_count;
 	/* The active format */
 	struct v4l2_mbus_framefmt sink_fmt;
 	u32 src_code;
@@ -337,6 +338,9 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
 		const struct v4l2_format_info *pix_info;
 		unsigned int frame_size;
 
+		if (atomic_inc_return(&vdeb->use_count) != 1)
+			return 0;
+
 		if (vdeb->src_frame)
 			return 0;
 
@@ -374,6 +378,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;
 
@@ -562,6 +569,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
 	vdeb->ved.process_frame = vimc_deb_process_frame;
 	dev_set_drvdata(comp, &vdeb->ved);
 	vdeb->dev = comp;
+	atomic_set(&vdeb->use_count, 0);
 
 	/* Initialize the frame format */
 	vdeb->sink_fmt = sink_fmt_default;
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index 8aecf8e920310608..37d2020d987a7d80 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -45,6 +45,7 @@ struct vimc_sca_device {
 	struct vimc_ent_device ved;
 	struct v4l2_subdev sd;
 	struct device *dev;
+	atomic_t use_count;
 	/* NOTE: the source fmt is the same as the sink
 	 * with the width and hight multiplied by mult
 	 */
@@ -213,6 +214,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
 		const struct v4l2_format_info *pix_info;
 		unsigned int frame_size;
 
+		if (atomic_inc_return(&vsca->use_count) != 1)
+			return 0;
+
 		if (vsca->src_frame)
 			return 0;
 
@@ -242,6 +246,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;
 
@@ -396,6 +403,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
 	vsca->ved.process_frame = vimc_sca_process_frame;
 	dev_set_drvdata(comp, &vsca->ved);
 	vsca->dev = comp;
+	atomic_set(&vsca->use_count, 0);
 
 	/* Initialize the frame format */
 	vsca->sink_fmt = sink_fmt_default;
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index baca9ca67ce0af0b..36c3cea85a185f4b 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -34,6 +34,7 @@ struct vimc_sen_device {
 	struct vimc_ent_device ved;
 	struct v4l2_subdev sd;
 	struct device *dev;
+	atomic_t use_count;
 	struct tpg_data tpg;
 	struct task_struct *kthread_sen;
 	u8 *frame;
@@ -197,6 +198,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
 		const struct v4l2_format_info *pix_info;
 		unsigned int frame_size;
 
+		if (atomic_inc_return(&vsen->use_count) != 1)
+			return 0;
+
 		if (vsen->kthread_sen)
 			/* tpg is already executing */
 			return 0;
@@ -218,6 +222,8 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
 		vimc_sen_tpg_s_format(vsen);
 
 	} else {
+		if (atomic_dec_return(&vsen->use_count) != 0)
+			return 0;
 
 		vfree(vsen->frame);
 		vsen->frame = NULL;
@@ -367,6 +373,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
 	vsen->ved.process_frame = vimc_sen_process_frame;
 	dev_set_drvdata(comp, &vsen->ved);
 	vsen->dev = comp;
+	atomic_set(&vsen->use_count, 0);
 
 	/* Initialize the frame format */
 	vsen->mbus_format = fmt_default;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] vimc: Serialize vimc_streamer_s_stream()
  2019-05-18  1:07 [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Niklas Söderlund
  2019-05-18  1:07 ` [PATCH 1/3] vimc: Add usage count to subdevices Niklas Söderlund
@ 2019-05-18  1:07 ` Niklas Söderlund
  2019-05-18  1:07 ` [PATCH 3/3] vimc: Join pipeline if one already exists Niklas Söderlund
  2019-10-23  7:01 ` [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Hans Verkuil
  3 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2019-05-18  1:07 UTC (permalink / raw)
  To: Helen Koike, linux-media
  Cc: libcamera-devel, Niklas Söderlund, Niklas Söderlund

Prepare for multiple video streams from the same sensor by serializing
vimc_streamer_s_stream(). Multiple streams will allow for multiple
concurrent calls to this function that could involve the same
subdevices.

If that happens the internal state of the involved subdevices could go
out of sync as they are being started and stopped at the same time,
prevent this by serializing starting and stopping of the vimc streamer.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 drivers/media/platform/vimc/vimc-streamer.c | 23 ++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 26b6742594890b16..514690dca3b1187b 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -153,39 +153,48 @@ 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))
-			return PTR_ERR(stream->kthread);
+		if (IS_ERR(stream->kthread)) {
+			ret = PTR_ERR(stream->kthread);
+			goto out;
+		}
 
 	} else {
 		if (!stream->kthread)
-			return 0;
+			goto out;
 
 		ret = kthread_stop(stream->kthread);
 		if (ret)
-			return ret;
+			goto out;
 
 		stream->kthread = NULL;
 
 		vimc_streamer_pipeline_terminate(stream);
 	}
+out:
+	mutex_unlock(&vimc_streamer_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(vimc_streamer_s_stream);
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] vimc: Join pipeline if one already exists
  2019-05-18  1:07 [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Niklas Söderlund
  2019-05-18  1:07 ` [PATCH 1/3] vimc: Add usage count to subdevices Niklas Söderlund
  2019-05-18  1:07 ` [PATCH 2/3] vimc: Serialize vimc_streamer_s_stream() Niklas Söderlund
@ 2019-05-18  1:07 ` Niklas Söderlund
  2019-07-09 18:24   ` Helen Koike
  2019-10-23  7:01 ` [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Hans Verkuil
  3 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2019-05-18  1:07 UTC (permalink / raw)
  To: Helen Koike, linux-media
  Cc: libcamera-devel, Niklas Söderlund, Niklas Söderlund

A sensor which is running is already part of a pipeline and trying to
start a new pipeline is not possible. This prevents two capture devices
connected to the same sensor from running at the same time.

Instead of failing to start the second capture device allow it to join
the already running pipeline by looking it up at the sensor. This allows
two (or more) capture devices to independently be started and stopped
while still being connected to the same sensor.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 drivers/media/platform/vimc/vimc-capture.c | 35 +++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index e7d0fc2228a6f0c1..f9eb1e327e311b4a 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -264,16 +264,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;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] vimc: Join pipeline if one already exists
  2019-05-18  1:07 ` [PATCH 3/3] vimc: Join pipeline if one already exists Niklas Söderlund
@ 2019-07-09 18:24   ` Helen Koike
  2019-07-11  4:35     ` Niklas Söderlund
  0 siblings, 1 reply; 10+ messages in thread
From: Helen Koike @ 2019-07-09 18:24 UTC (permalink / raw)
  To: Niklas Söderlund, Helen Koike, linux-media
  Cc: libcamera-devel, Niklas Söderlund

Hi Niklas,

Thanks for the patch (and sorry for my late reply).

On 5/17/19 10:07 PM, Niklas Söderlund wrote:
> A sensor which is running is already part of a pipeline and trying to
> start a new pipeline is not possible. This prevents two capture devices
> connected to the same sensor from running at the same time.
> 
> Instead of failing to start the second capture device allow it to join
> the already running pipeline by looking it up at the sensor. This allows
> two (or more) capture devices to independently be started and stopped
> while still being connected to the same sensor.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  drivers/media/platform/vimc/vimc-capture.c | 35 +++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index e7d0fc2228a6f0c1..f9eb1e327e311b4a 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -264,16 +264,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;

I was wondering if it should search up to the sensor, or if it could just search the first entity with a pipe object, what do you think?
Like this it should work with an output device instead of a sensor.

Regards,
Helen

> +
> +	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;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] vimc: Join pipeline if one already exists
  2019-07-09 18:24   ` Helen Koike
@ 2019-07-11  4:35     ` Niklas Söderlund
  0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2019-07-11  4:35 UTC (permalink / raw)
  To: Helen Koike; +Cc: Helen Koike, linux-media, libcamera-devel

Hi Helen,

Thanks for your feedback.

On 2019-07-09 15:24:10 -0300, Helen Koike wrote:
> Hi Niklas,
> 
> Thanks for the patch (and sorry for my late reply).
> 
> On 5/17/19 10:07 PM, Niklas Söderlund wrote:
> > A sensor which is running is already part of a pipeline and trying to
> > start a new pipeline is not possible. This prevents two capture devices
> > connected to the same sensor from running at the same time.
> > 
> > Instead of failing to start the second capture device allow it to join
> > the already running pipeline by looking it up at the sensor. This allows
> > two (or more) capture devices to independently be started and stopped
> > while still being connected to the same sensor.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  drivers/media/platform/vimc/vimc-capture.c | 35 +++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> > index e7d0fc2228a6f0c1..f9eb1e327e311b4a 100644
> > --- a/drivers/media/platform/vimc/vimc-capture.c
> > +++ b/drivers/media/platform/vimc/vimc-capture.c
> > @@ -264,16 +264,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;
> 
> I was wondering if it should search up to the sensor, or if it could just search the first entity with a pipe object, what do you think?
> Like this it should work with an output device instead of a sensor.

I think that might be a good idea, I will see what I can do for v2.

> 
> Regards,
> Helen
> 
> > +
> > +	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;
> > 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor
  2019-05-18  1:07 [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Niklas Söderlund
                   ` (2 preceding siblings ...)
  2019-05-18  1:07 ` [PATCH 3/3] vimc: Join pipeline if one already exists Niklas Söderlund
@ 2019-10-23  7:01 ` Hans Verkuil
  2019-10-23 10:22   ` Niklas Söderlund
  3 siblings, 1 reply; 10+ messages in thread
From: Hans Verkuil @ 2019-10-23  7:01 UTC (permalink / raw)
  To: Niklas Söderlund, Helen Koike, linux-media; +Cc: libcamera-devel

Hi Niklas,

For one reason or another this series was never reviewed/picked up and
it now no longer applies.

Combined with the big switch to a monolithic driver I am sure that this
series needs to be redone. So I am marking it as "Changes Requested" and
it is up to you to decide whether to rebase/rework this series.

Regards,

	Hans

On 5/18/19 3:07 AM, Niklas Söderlund wrote:
> Hi,
> 
> This series adds support for two (or more) capture devices to be 
> connected to the same senors and run simultaneously. Each capture device 
> can be started and stopped independent of each other.
> 
> Patch 1/3 and 2/3 deals with solving the issues that arises once two 
> capture devices can be part of the same pipeline. While 3/3 allows for 
> two capture devices to be part of the same pipeline and thus allows for 
> simultaneously use.
> 
> The series is based on the latest media-tree and it functionality can be 
> demonstrated with the following test.
> 
>>>> begin test <<<
> mdev=/dev/media0
> 
> media-ctl -d $mdev -l "'Debayer A':1 -> 'Scaler':0 [1]"
> media-ctl -d $mdev -l "'Debayer B':1 -> 'Scaler':0 [0]"
> 
> media-ctl -d $mdev -V "'Debayer A':0 [fmt:RGB888_1X24/640x480 field:none]"
> media-ctl -d /dev/media0 -V "'Sensor A':0 [fmt:SRGGB8_1X8/640x480 field:none]"
> 
> yavta -f RGB24 -s 1920x1440 --field none /dev/video2
> yavta -f SRGGB8 -s 640x480 --field none /dev/video0
> 
> yavta -f RGB24 -s 1920x1440 --field none /dev/video2 --capture=100 &
> yavta -f SRGGB8 -s 640x480 --field none /dev/video0 --capture=100
> wait
>>>> end test <<<
> 
> In addition to testing with this test the series is tested with multiple 
> qv4l2 instances controlling different capture devices connected to the 
> same sensor.
> 
> Niklas Söderlund (3):
>   vimc: Add usage count to subdevices
>   vimc: Serialize vimc_streamer_s_stream()
>   vimc: Join pipeline if one already exists
> 
>  drivers/media/platform/vimc/vimc-capture.c  | 35 ++++++++++++++++++++-
>  drivers/media/platform/vimc/vimc-debayer.c  |  8 +++++
>  drivers/media/platform/vimc/vimc-scaler.c   |  8 +++++
>  drivers/media/platform/vimc/vimc-sensor.c   |  7 +++++
>  drivers/media/platform/vimc/vimc-streamer.c | 23 +++++++++-----
>  5 files changed, 73 insertions(+), 8 deletions(-)
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor
  2019-10-23  7:01 ` [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Hans Verkuil
@ 2019-10-23 10:22   ` Niklas Söderlund
  0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2019-10-23 10:22 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Helen Koike, linux-media, libcamera-devel

Hi Hans,

On 2019-10-23 09:01:40 +0200, Hans Verkuil wrote:
> Hi Niklas,
> 
> For one reason or another this series was never reviewed/picked up and
> it now no longer applies.
> 
> Combined with the big switch to a monolithic driver I am sure that this
> series needs to be redone. So I am marking it as "Changes Requested" and
> it is up to you to decide whether to rebase/rework this series.

I talked to Helen about this series and she thought it could be a nice 
candidate for a student workshop/class so I'm not planing to respin this 
myself.

> 
> Regards,
> 
> 	Hans
> 
> On 5/18/19 3:07 AM, Niklas Söderlund wrote:
> > Hi,
> > 
> > This series adds support for two (or more) capture devices to be 
> > connected to the same senors and run simultaneously. Each capture device 
> > can be started and stopped independent of each other.
> > 
> > Patch 1/3 and 2/3 deals with solving the issues that arises once two 
> > capture devices can be part of the same pipeline. While 3/3 allows for 
> > two capture devices to be part of the same pipeline and thus allows for 
> > simultaneously use.
> > 
> > The series is based on the latest media-tree and it functionality can be 
> > demonstrated with the following test.
> > 
> >>>> begin test <<<
> > mdev=/dev/media0
> > 
> > media-ctl -d $mdev -l "'Debayer A':1 -> 'Scaler':0 [1]"
> > media-ctl -d $mdev -l "'Debayer B':1 -> 'Scaler':0 [0]"
> > 
> > media-ctl -d $mdev -V "'Debayer A':0 [fmt:RGB888_1X24/640x480 field:none]"
> > media-ctl -d /dev/media0 -V "'Sensor A':0 [fmt:SRGGB8_1X8/640x480 field:none]"
> > 
> > yavta -f RGB24 -s 1920x1440 --field none /dev/video2
> > yavta -f SRGGB8 -s 640x480 --field none /dev/video0
> > 
> > yavta -f RGB24 -s 1920x1440 --field none /dev/video2 --capture=100 &
> > yavta -f SRGGB8 -s 640x480 --field none /dev/video0 --capture=100
> > wait
> >>>> end test <<<
> > 
> > In addition to testing with this test the series is tested with multiple 
> > qv4l2 instances controlling different capture devices connected to the 
> > same sensor.
> > 
> > Niklas Söderlund (3):
> >   vimc: Add usage count to subdevices
> >   vimc: Serialize vimc_streamer_s_stream()
> >   vimc: Join pipeline if one already exists
> > 
> >  drivers/media/platform/vimc/vimc-capture.c  | 35 ++++++++++++++++++++-
> >  drivers/media/platform/vimc/vimc-debayer.c  |  8 +++++
> >  drivers/media/platform/vimc/vimc-scaler.c   |  8 +++++
> >  drivers/media/platform/vimc/vimc-sensor.c   |  7 +++++
> >  drivers/media/platform/vimc/vimc-streamer.c | 23 +++++++++-----
> >  5 files changed, 73 insertions(+), 8 deletions(-)
> > 
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] vimc: Add usage count to subdevices
  2019-05-18  1:07 ` [PATCH 1/3] vimc: Add usage count to subdevices Niklas Söderlund
@ 2020-03-16 19:40   ` Helen Koike
  2020-03-16 20:32     ` Niklas Söderlund
  0 siblings, 1 reply; 10+ messages in thread
From: Helen Koike @ 2020-03-16 19:40 UTC (permalink / raw)
  To: Niklas Söderlund, linux-media
  Cc: libcamera-devel, Niklas Söderlund, Dafna Hirschfeld

Hi Niklas,

On 5/17/19 10:07 PM, Niklas Söderlund wrote:
> Prepare for multiple video streams from the same sensor by adding a use
> counter to each subdevice. The counter is increase for every s_stream(1)
> and decremented for every s_stream(0) call.
> 
> The subdevice stream is not started or stopped unless the usage count go
> from 0 to 1 (started) or from 1 to 0 (stopped). This allow for multiple
> s_stream() calls to try to either start or stop the device while only
> the first/last call will actually effect the state of the device.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

This patch won't be required if the following series gets accepted
https://patchwork.linuxtv.org/cover/62233/

Since helpers takes care that s_stream(true) won't be called twice without
s_stream(false) (and the opposite is also true).

Regards,
Helen

> ---
>  drivers/media/platform/vimc/vimc-debayer.c | 8 ++++++++
>  drivers/media/platform/vimc/vimc-scaler.c  | 8 ++++++++
>  drivers/media/platform/vimc/vimc-sensor.c  | 7 +++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 281f9c1a7249ad1d..624fc23ce3077d40 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -56,6 +56,7 @@ struct vimc_deb_device {
>  	struct vimc_ent_device ved;
>  	struct v4l2_subdev sd;
>  	struct device *dev;
> +	atomic_t use_count;
>  	/* The active format */
>  	struct v4l2_mbus_framefmt sink_fmt;
>  	u32 src_code;
> @@ -337,6 +338,9 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
>  		const struct v4l2_format_info *pix_info;
>  		unsigned int frame_size;
>  
> +		if (atomic_inc_return(&vdeb->use_count) != 1)
> +			return 0;
> +
>  		if (vdeb->src_frame)
>  			return 0;
>  
> @@ -374,6 +378,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;
>  
> @@ -562,6 +569,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
>  	vdeb->ved.process_frame = vimc_deb_process_frame;
>  	dev_set_drvdata(comp, &vdeb->ved);
>  	vdeb->dev = comp;
> +	atomic_set(&vdeb->use_count, 0);
>  
>  	/* Initialize the frame format */
>  	vdeb->sink_fmt = sink_fmt_default;
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index 8aecf8e920310608..37d2020d987a7d80 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -45,6 +45,7 @@ struct vimc_sca_device {
>  	struct vimc_ent_device ved;
>  	struct v4l2_subdev sd;
>  	struct device *dev;
> +	atomic_t use_count;
>  	/* NOTE: the source fmt is the same as the sink
>  	 * with the width and hight multiplied by mult
>  	 */
> @@ -213,6 +214,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
>  		const struct v4l2_format_info *pix_info;
>  		unsigned int frame_size;
>  
> +		if (atomic_inc_return(&vsca->use_count) != 1)
> +			return 0;
> +
>  		if (vsca->src_frame)
>  			return 0;
>  
> @@ -242,6 +246,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;
>  
> @@ -396,6 +403,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
>  	vsca->ved.process_frame = vimc_sca_process_frame;
>  	dev_set_drvdata(comp, &vsca->ved);
>  	vsca->dev = comp;
> +	atomic_set(&vsca->use_count, 0);
>  
>  	/* Initialize the frame format */
>  	vsca->sink_fmt = sink_fmt_default;
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index baca9ca67ce0af0b..36c3cea85a185f4b 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -34,6 +34,7 @@ struct vimc_sen_device {
>  	struct vimc_ent_device ved;
>  	struct v4l2_subdev sd;
>  	struct device *dev;
> +	atomic_t use_count;
>  	struct tpg_data tpg;
>  	struct task_struct *kthread_sen;
>  	u8 *frame;
> @@ -197,6 +198,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
>  		const struct v4l2_format_info *pix_info;
>  		unsigned int frame_size;
>  
> +		if (atomic_inc_return(&vsen->use_count) != 1)
> +			return 0;
> +
>  		if (vsen->kthread_sen)
>  			/* tpg is already executing */
>  			return 0;
> @@ -218,6 +222,8 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
>  		vimc_sen_tpg_s_format(vsen);
>  
>  	} else {
> +		if (atomic_dec_return(&vsen->use_count) != 0)
> +			return 0;
>  
>  		vfree(vsen->frame);
>  		vsen->frame = NULL;
> @@ -367,6 +373,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
>  	vsen->ved.process_frame = vimc_sen_process_frame;
>  	dev_set_drvdata(comp, &vsen->ved);
>  	vsen->dev = comp;
> +	atomic_set(&vsen->use_count, 0);
>  
>  	/* Initialize the frame format */
>  	vsen->mbus_format = fmt_default;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] vimc: Add usage count to subdevices
  2020-03-16 19:40   ` Helen Koike
@ 2020-03-16 20:32     ` Niklas Söderlund
  0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2020-03-16 20:32 UTC (permalink / raw)
  To: Helen Koike; +Cc: linux-media, libcamera-devel, Dafna Hirschfeld

Hi Helen,

Thanks for your notification.

On 2020-03-16 16:40:54 -0300, Helen Koike wrote:
> Hi Niklas,
> 
> On 5/17/19 10:07 PM, Niklas Söderlund wrote:
> > Prepare for multiple video streams from the same sensor by adding a use
> > counter to each subdevice. The counter is increase for every s_stream(1)
> > and decremented for every s_stream(0) call.
> > 
> > The subdevice stream is not started or stopped unless the usage count go
> > from 0 to 1 (started) or from 1 to 0 (stopped). This allow for multiple
> > s_stream() calls to try to either start or stop the device while only
> > the first/last call will actually effect the state of the device.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> This patch won't be required if the following series gets accepted
> https://patchwork.linuxtv.org/cover/62233/
> 
> Since helpers takes care that s_stream(true) won't be called twice without
> s_stream(false) (and the opposite is also true).

Sweet, it will be nice to get multiple stream support upstream for VIMC!

> 
> Regards,
> Helen
> 
> > ---
> >  drivers/media/platform/vimc/vimc-debayer.c | 8 ++++++++
> >  drivers/media/platform/vimc/vimc-scaler.c  | 8 ++++++++
> >  drivers/media/platform/vimc/vimc-sensor.c  | 7 +++++++
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> > index 281f9c1a7249ad1d..624fc23ce3077d40 100644
> > --- a/drivers/media/platform/vimc/vimc-debayer.c
> > +++ b/drivers/media/platform/vimc/vimc-debayer.c
> > @@ -56,6 +56,7 @@ struct vimc_deb_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> >  	struct device *dev;
> > +	atomic_t use_count;
> >  	/* The active format */
> >  	struct v4l2_mbus_framefmt sink_fmt;
> >  	u32 src_code;
> > @@ -337,6 +338,9 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct v4l2_format_info *pix_info;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vdeb->use_count) != 1)
> > +			return 0;
> > +
> >  		if (vdeb->src_frame)
> >  			return 0;
> >  
> > @@ -374,6 +378,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;
> >  
> > @@ -562,6 +569,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
> >  	vdeb->ved.process_frame = vimc_deb_process_frame;
> >  	dev_set_drvdata(comp, &vdeb->ved);
> >  	vdeb->dev = comp;
> > +	atomic_set(&vdeb->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vdeb->sink_fmt = sink_fmt_default;
> > diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> > index 8aecf8e920310608..37d2020d987a7d80 100644
> > --- a/drivers/media/platform/vimc/vimc-scaler.c
> > +++ b/drivers/media/platform/vimc/vimc-scaler.c
> > @@ -45,6 +45,7 @@ struct vimc_sca_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> >  	struct device *dev;
> > +	atomic_t use_count;
> >  	/* NOTE: the source fmt is the same as the sink
> >  	 * with the width and hight multiplied by mult
> >  	 */
> > @@ -213,6 +214,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct v4l2_format_info *pix_info;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vsca->use_count) != 1)
> > +			return 0;
> > +
> >  		if (vsca->src_frame)
> >  			return 0;
> >  
> > @@ -242,6 +246,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;
> >  
> > @@ -396,6 +403,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
> >  	vsca->ved.process_frame = vimc_sca_process_frame;
> >  	dev_set_drvdata(comp, &vsca->ved);
> >  	vsca->dev = comp;
> > +	atomic_set(&vsca->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vsca->sink_fmt = sink_fmt_default;
> > diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> > index baca9ca67ce0af0b..36c3cea85a185f4b 100644
> > --- a/drivers/media/platform/vimc/vimc-sensor.c
> > +++ b/drivers/media/platform/vimc/vimc-sensor.c
> > @@ -34,6 +34,7 @@ struct vimc_sen_device {
> >  	struct vimc_ent_device ved;
> >  	struct v4l2_subdev sd;
> >  	struct device *dev;
> > +	atomic_t use_count;
> >  	struct tpg_data tpg;
> >  	struct task_struct *kthread_sen;
> >  	u8 *frame;
> > @@ -197,6 +198,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> >  		const struct v4l2_format_info *pix_info;
> >  		unsigned int frame_size;
> >  
> > +		if (atomic_inc_return(&vsen->use_count) != 1)
> > +			return 0;
> > +
> >  		if (vsen->kthread_sen)
> >  			/* tpg is already executing */
> >  			return 0;
> > @@ -218,6 +222,8 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> >  		vimc_sen_tpg_s_format(vsen);
> >  
> >  	} else {
> > +		if (atomic_dec_return(&vsen->use_count) != 0)
> > +			return 0;
> >  
> >  		vfree(vsen->frame);
> >  		vsen->frame = NULL;
> > @@ -367,6 +373,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
> >  	vsen->ved.process_frame = vimc_sen_process_frame;
> >  	dev_set_drvdata(comp, &vsen->ved);
> >  	vsen->dev = comp;
> > +	atomic_set(&vsen->use_count, 0);
> >  
> >  	/* Initialize the frame format */
> >  	vsen->mbus_format = fmt_default;
> > 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-16 20:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-18  1:07 [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Niklas Söderlund
2019-05-18  1:07 ` [PATCH 1/3] vimc: Add usage count to subdevices Niklas Söderlund
2020-03-16 19:40   ` Helen Koike
2020-03-16 20:32     ` Niklas Söderlund
2019-05-18  1:07 ` [PATCH 2/3] vimc: Serialize vimc_streamer_s_stream() Niklas Söderlund
2019-05-18  1:07 ` [PATCH 3/3] vimc: Join pipeline if one already exists Niklas Söderlund
2019-07-09 18:24   ` Helen Koike
2019-07-11  4:35     ` Niklas Söderlund
2019-10-23  7:01 ` [PATCH 0/3] vimc: Allow multiple capture devices to use the same sensor Hans Verkuil
2019-10-23 10:22   ` Niklas Söderlund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).