diff mbox series

[v6,4/4] media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels

Message ID 20221205152450.1099-5-quic_mmitkov@quicinc.com (mailing list archive)
State New, archived
Headers show
Series media: camss: sm8250: Virtual channels support for SM8250 | expand

Commit Message

Milen Mitkov (Consultant) Dec. 5, 2022, 3:24 p.m. UTC 
From: Milen Mitkov <quic_mmitkov@quicinc.com>

Use the multistream series function video_device_pipeline_alloc_start
to allows multiple clients of the same pipeline.

If the VFE entity is used by another instance of the pipeline,
the pipeline won't be stopped. This allows for stopping and starting
streams at any point without disrupting the other running streams.

To prepare and start multiple virtual channels each CSID source pad
corresponding to a virtual channel must be linked to the corresponding
IFE entity. Note that on SM8250 each CSID is connected, at the
hardware level, to only one IFE. Thus, you must use CSID1 with IFE1
and you can't use it with IFE0 for example.

Example: the following media controller setup expects multiplexed 
sensor data on CSIPHY2. Data will be passed on to CSID0, which will 
demux it to 2 streams - for RDI0 and RD1 ports of IFE0.:

media-ctl -v -d /dev/media0 -V '"imx577 '22-001a'":0[fmt:SRGGB10/3840x2160 field:none]'
media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_csid0":0[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_csid0":1[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_csid0":2[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/3840x2160]'
media-ctl -V '"msm_vfe0_rdi1":0[fmt:SRGGB10/3840x2160]'
media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]'

Note: CSID's entity pad 0 is a sink pad, pads 1..4 are source pads
To start streaming a v4l2 client must open the corresponding
/dev/videoN node. For example, with yavta:

yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 3840x2160 -F /dev/video0
yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 3840x2160 -F /dev/video1

Note that IFEs (vfe0, vfe1) on SM8250 have 3 RDI ports and a single 
PIX port and IFELites (vfe2, vfe3) have 4 RDI ports and no PIX port.

Signed-off-by: Milen Mitkov <quic_mmitkov@quicinc.com>
Reviewed-by: Robert Foss <robert.foss@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Robert Foss <robert.foss@linaro.org>
---
 .../media/platform/qcom/camss/camss-video.c   | 21 ++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Bryan O'Donoghue Dec. 5, 2022, 4:43 p.m. UTC | #1 
On 05/12/2022 15:24, quic_mmitkov@quicinc.com wrote:
> media-ctl -v -d /dev/media0 -V '"imx577 '22-001a'":0[fmt:SRGGB10/3840x2160 field:none]'

I really like the improved commit log, thank you for that.

SRGGB10/3840x2160 drivers/media/i2c/imx412.c that's not a supported 
resolution.

media-ctl -v -d /dev/media0 -V '"imx577 
'22-001a'":0[fmt:SRGGB10/4056x3040 field:none]'

?

---
bod
Milen Mitkov (Consultant) Dec. 5, 2022, 4:49 p.m. UTC | #2 
On 05/12/2022 18:43, Bryan O'Donoghue wrote:
> On 05/12/2022 15:24, quic_mmitkov@quicinc.com wrote:
>> media-ctl -v -d /dev/media0 -V '"imx577 
>> '22-001a'":0[fmt:SRGGB10/3840x2160 field:none]'
>
> I really like the improved commit log, thank you for that.
>
> SRGGB10/3840x2160 drivers/media/i2c/imx412.c that's not a supported 
> resolution.
>
> media-ctl -v -d /dev/media0 -V '"imx577 
> '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]'
>
> ?
>
> ---
> bod


Hi Bryan,

it's not a supported resolution with the current imx412/577 driver in 
upstream. We tested with a different driver (with proprietary Sony 
registers)

that uses this resolution that generates 2 multiplexed streams. It would 
be impossible for pure upstream solution to test this driver with imx412 
right now.

I couldn't have used the upstream supported resolution either, because 
it's of a sensor mode with only 1 stream and that would create the 
impression that anybody can just take this imx412 sensor and use the 
upstream driver to test the solution, but this is in fact not possible 
without register changes to the sensor driver itself.

Regards,

Milen
Bryan O'Donoghue Dec. 5, 2022, 5:01 p.m. UTC | #3 
On 05/12/2022 16:49, Milen Mitkov (Consultant) wrote:
> 
> On 05/12/2022 18:43, Bryan O'Donoghue wrote:
>> On 05/12/2022 15:24, quic_mmitkov@quicinc.com wrote:
>>> media-ctl -v -d /dev/media0 -V '"imx577 
>>> '22-001a'":0[fmt:SRGGB10/3840x2160 field:none]'
>>
>> I really like the improved commit log, thank you for that.
>>
>> SRGGB10/3840x2160 drivers/media/i2c/imx412.c that's not a supported 
>> resolution.
>>
>> media-ctl -v -d /dev/media0 -V '"imx577 
>> '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]'
>>
>> ?
>>
>> ---
>> bod
> 
> 
> Hi Bryan,
> 
> it's not a supported resolution with the current imx412/577 driver in 
> upstream. We tested with a different driver (with proprietary Sony 
> registers)
> 
> that uses this resolution that generates 2 multiplexed streams. It would 
> be impossible for pure upstream solution to test this driver with imx412 
> right now.
> 
> I couldn't have used the upstream supported resolution either, because 
> it's of a sensor mode with only 1 stream and that would create the 
> impression that anybody can just take this imx412 sensor and use the 
> upstream driver to test the solution, but this is in fact not possible 
> without register changes to the sensor driver itself.
> 
> Regards,
> 
> Milen
> 

OK fair enough, I think you should make a more explicit statement about 
the mapping of VC to RDI

Here's the real bit of salient information for whomever is trying to get 
VCs working in the future

media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]'

The first VC msm_csid0":1 always maps to RDI0 the second VC always maps 
to RDI1

I think we should document that mapping clearly so that you don't have 
to read or understand the code to know

media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi1":0[1]' is an invalid mapping 
and why that is.

---
bod
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 41deda232e4a..12ac7d4d755e 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -351,6 +351,7 @@  static int video_get_subdev_format(struct camss_video *video,
 	if (subdev == NULL)
 		return -EPIPE;
 
+	memset(&fmt, 0, sizeof(fmt));
 	fmt.pad = pad;
 	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
 
@@ -493,9 +494,11 @@  static int video_start_streaming(struct vb2_queue *q, unsigned int count)
 	struct v4l2_subdev *subdev;
 	int ret;
 
-	ret = video_device_pipeline_start(vdev, &video->pipe);
-	if (ret < 0)
+	ret = video_device_pipeline_alloc_start(vdev);
+	if (ret < 0) {
+		dev_err(video->camss->dev, "Failed to start media pipeline: %d\n", ret);
 		goto flush_buffers;
+	}
 
 	ret = video_check_format(video);
 	if (ret < 0)
@@ -537,6 +540,7 @@  static void video_stop_streaming(struct vb2_queue *q)
 	struct media_entity *entity;
 	struct media_pad *pad;
 	struct v4l2_subdev *subdev;
+	int ret;
 
 	entity = &vdev->entity;
 	while (1) {
@@ -551,7 +555,18 @@  static void video_stop_streaming(struct vb2_queue *q)
 		entity = pad->entity;
 		subdev = media_entity_to_v4l2_subdev(entity);
 
-		v4l2_subdev_call(subdev, video, s_stream, 0);
+		ret = v4l2_subdev_call(subdev, video, s_stream, 0);
+
+		if (entity->use_count > 1) {
+			/* Don't stop if other instances of the pipeline are still running */
+			dev_dbg(video->camss->dev, "Video pipeline still used, don't stop streaming.\n");
+			return;
+		}
+
+		if (ret) {
+			dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
+			return;
+		}
 	}
 
 	video_device_pipeline_stop(vdev);