| Message ID | 20221128144210.1028-1-quic_mmitkov@quicinc.com |
|---|---|
| Headers | show |
| Series | media: camss: sm8250: Virtual channels support for SM8250 | expand |
On 28/11/2022 14:42, quic_mmitkov@quicinc.com wrote: > With these changes, the CSID entity has, as it did previously, a single > sink port (0), and always exposes 4 source ports (1, 2,3, 4). The > virtual channel configuration is determined by which of the source ports > are linked to an output VFE line. For example, the link below will > configure the CSID driver to enable vc 0 and vc 1: > > media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' Following your instructions here root@linaro-gnome:~# media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' root@linaro-gnome:~# media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' root@linaro-gnome:~# media-ctl -l '"msm_csid0":3->"msm_vfe0_rdi2":0[1]' root@linaro-gnome:~# media-ctl -l '"msm_csid0":4->"msm_vfe0_rdi3":0[1]' Unable to parse link: Invalid argument (22) ? --- bod
On 30/11/2022 17:50, Bryan O'Donoghue wrote: > On 28/11/2022 14:42, quic_mmitkov@quicinc.com wrote: >> With these changes, the CSID entity has, as it did previously, a single >> sink port (0), and always exposes 4 source ports (1, 2,3, 4). The >> virtual channel configuration is determined by which of the source ports >> are linked to an output VFE line. For example, the link below will >> configure the CSID driver to enable vc 0 and vc 1: >> >> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' > > Following your instructions here > > root@linaro-gnome:~# media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > root@linaro-gnome:~# media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' > root@linaro-gnome:~# media-ctl -l '"msm_csid0":3->"msm_vfe0_rdi2":0[1]' > root@linaro-gnome:~# media-ctl -l '"msm_csid0":4->"msm_vfe0_rdi3":0[1]' > Unable to parse link: Invalid argument (22) > > ? > > --- > bod Hi Bryan, Thanks for the feedback! There are the following IFEs on SM8250 (Titan 4.8): IFE0 -> has 3 RDI lines and 1 PIX line IFE1 -> has 3 RDI lines and 1 PIX line IFELite0 -> has 4 RDI lines IFELite1 -> has 4 RDI lines so there's no msm_vfe0_rdi3 port for IFE0 and IFE1 at all, only rdi0, rdi1, rdi2. In theory if you link the 4th CSID source port to msm_vfe0_pix the parsing works: media-ctl -l '"msm_csid0":4->"msm_vfe0_pix":0[1]' However, in practice, there's no PIX support in camss so even if the linking works, the hardware won't get configured properly. You can however use the IFELite's 4th RDI port. Due to omission in camss-vfe there isn't any separate logic for naming the sink ports of IFELite's devies so it's still called "pix", even though it's not a pix port. It should be a small code change to fix the naming of the port for IFELite, I could do it as part of this patch set or as a new patch after this patch set gets merged. Which is the preferred way? Regards, Milen
On 30/11/2022 17:04, Milen Mitkov (Consultant) wrote: > On 30/11/2022 17:50, Bryan O'Donoghue wrote: >> On 28/11/2022 14:42, quic_mmitkov@quicinc.com wrote: >>> With these changes, the CSID entity has, as it did previously, a single >>> sink port (0), and always exposes 4 source ports (1, 2,3, 4). The >>> virtual channel configuration is determined by which of the source ports >>> are linked to an output VFE line. For example, the link below will >>> configure the CSID driver to enable vc 0 and vc 1: >>> >>> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >>> media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' >> >> Following your instructions here >> >> root@linaro-gnome:~# media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> root@linaro-gnome:~# media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' >> root@linaro-gnome:~# media-ctl -l '"msm_csid0":3->"msm_vfe0_rdi2":0[1]' >> root@linaro-gnome:~# media-ctl -l '"msm_csid0":4->"msm_vfe0_rdi3":0[1]' >> Unable to parse link: Invalid argument (22) >> >> ? >> >> --- >> bod > > Hi Bryan, > > Thanks for the feedback! There are the following IFEs on SM8250 (Titan > 4.8): > > IFE0 -> has 3 RDI lines and 1 PIX line > > IFE1 -> has 3 RDI lines and 1 PIX line > > IFELite0 -> has 4 RDI lines > > IFELite1 -> has 4 RDI lines > > so there's no msm_vfe0_rdi3 port for IFE0 and IFE1 at all, only rdi0, > rdi1, rdi2. In theory if you link the 4th CSID source port to > msm_vfe0_pix the parsing works: > > media-ctl -l '"msm_csid0":4->"msm_vfe0_pix":0[1]' > > However, in practice, there's no PIX support in camss so even if the > linking works, the hardware won't get configured properly. > > You can however use the IFELite's 4th RDI port. Due to omission in > camss-vfe there isn't any separate logic for naming the sink ports of > IFELite's devies so it's still called "pix", even though it's not a pix > port. It should be a small code change to fix the naming of the port for > IFELite, I could do it as part of this patch set or as a new patch after > this patch set gets merged. Which is the preferred way? > > Regards, > > Milen > Hmm, OK the pix mapping works for me, thanks, the It would be good to fixup the name, maybe as an additional patch. Also the above description contains important information. IMO its worthwhile to include information like that in the git log. i.e. IFE <-> RDI mapping, IFELite <-> RDI mapping Also suggest adding example media-ctl commands for the VCs to your git log both for yourself to remember - I'm usually logging so that I can remind myself - but also for other people to more easily replicate/validate your work. --- bod
From: Milen Mitkov <quic_mmitkov@quicinc.com> For v5: - Use entity->use_count instead of s_stream subdev call ret code to check if another instance of the pipeline is running. Prevents an error on 6.1 and up, when stopping one of several simultaneous instances. - flush buffers instead of just returning if the pipeline didn't start. For v4: - fixes the warning reported by the kernel test robot - tiny code change to enable the vc functionality with the partially-applied multistream patches on linux-next (tested on tag:next-20221010) For v3: - setting the sink pad format on the CSID entity will now propagate the format to the source pads to keep the subdev in a valid internal state. - code syntax improvements For v2: - code syntax improvements - The info print for the enabled VCs was demoted to a dbg print. Can be enabled with dynamic debug, e.g.: echo "file drivers/media/platform/qcom/camss/* +p" > /sys/kernel/debug/dynamic_debug/control NOTE: These changes depend on the multistream series, that as of yet is still not merged upstream. However, part of the multistream patches are merged in linux-next (tested on tag:next-20221010), including the patch that introduces the video_device_pipeline_alloc_start() functionality. This allows applying and using this series on linux-next without applying the complete multistream set. The CSID hardware on SM8250 can demux the input data stream into maximum of 4 multiple streams depending on virtual channel (vc) or data type (dt) configuration. Situations in which demuxing is useful: - HDR sensors that produce a 2-frame HDR output, e.g. a light and a dark frame (the setup we used for testing, with the imx412 sensor), or a 3-frame HDR output - light, medium-lit, dark frame. - sensors with additional metadata that is streamed over a different virtual channel/datatype. - sensors that produce frames with multiple resolutions in the same pixel data stream With these changes, the CSID entity has, as it did previously, a single sink port (0), and always exposes 4 source ports (1, 2,3, 4). The virtual channel configuration is determined by which of the source ports are linked to an output VFE line. For example, the link below will configure the CSID driver to enable vc 0 and vc 1: media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]' which will be demuxed and propagated into /dev/video0 and /dev/video1 respectively. With this, the userspace can use any normal V4L2 client app to start/stop/queue/dequeue from these video nodes. Tested with the yavta app. The format of each RDI channel of the used VFE(s) (msm_vfe0_rdi0, msm_vfe0_rdi1,...) must also be configured explicitly. Note that in order to keep a valid internal subdevice state, setting the sink pad format of the CSID subdevice will propagate this format to the source pads. However, since the CSID hardware can demux the input stream into several streams each of which can be a different format, in that case each source pad's format must be set individually, e.g.: media-ctl -V '"msm_csid0":1[fmt:SRGGB10/3840x2160]' media-ctl -V '"msm_csid0":2[fmt:SRGGB10/960x540]' Milen Mitkov (4): media: camss: sm8250: Virtual channels for CSID media: camss: vfe: Reserve VFE lines on stream start and link to CSID media: camss: vfe-480: Multiple outputs support for SM8250 media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels .../platform/qcom/camss/camss-csid-gen2.c | 54 ++++++++++------ .../media/platform/qcom/camss/camss-csid.c | 44 +++++++++---- .../media/platform/qcom/camss/camss-csid.h | 11 +++- .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- drivers/media/platform/qcom/camss/camss-vfe.c | 1 + .../media/platform/qcom/camss/camss-video.c | 21 ++++++- drivers/media/platform/qcom/camss/camss.c | 2 +- 7 files changed, 134 insertions(+), 60 deletions(-)