| Message ID | 20221209094037.1148-1-quic_mmitkov@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: camss: sm8250: Virtual channels support for SM8250 | expand |
On 09/12/2022 09:40, quic_mmitkov@quicinc.com wrote: > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > For v7: > - Fix an issue with output state for different versions of the IFE > hardware (for platforms different from QRB5, e.g. QRB3). > Yep. Working for me on rb3 now and thank you for updating the git commit in patch #4. Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> for the series. --- bod
On 09/12/2022 18:17, Bryan O'Donoghue wrote: > On 09/12/2022 09:40, quic_mmitkov@quicinc.com wrote: >> From: Milen Mitkov <quic_mmitkov@quicinc.com> >> >> For v7: >> - Fix an issue with output state for different versions of the IFE >> hardware (for platforms different from QRB5, e.g. QRB3). >> > > Yep. > > Working for me on rb3 now and thank you for updating the git commit in > patch #4. > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > for the series. > > --- > bod Hi Bryan, Robert, Hans and others, Happy New Year! Is there anything else I can/need to do to speed up the merging process of this series? Thanks, Milen
On 05/01/2023 08:37, Milen Mitkov (Consultant) wrote: > On 09/12/2022 18:17, Bryan O'Donoghue wrote: >> On 09/12/2022 09:40, quic_mmitkov@quicinc.com wrote: >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>> >>> For v7: >>> - Fix an issue with output state for different versions of the IFE >>> hardware (for platforms different from QRB5, e.g. QRB3). >>> >> >> Yep. >> >> Working for me on rb3 now and thank you for updating the git commit in >> patch #4. >> >> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> >> for the series. >> >> --- >> bod > > > Hi Bryan, Robert, Hans and others, > > > Happy New Year! > > Is there anything else I can/need to do to speed up the merging process > of this series? > > > Thanks, > > Milen > I don't think so. Is everything still working on linux-next ? e45fb347b630e...cc3c08b41a9c9 master -> linux-next/master
On 05/01/2023 20:43, Bryan O'Donoghue wrote: > On 05/01/2023 08:37, Milen Mitkov (Consultant) wrote: >> On 09/12/2022 18:17, Bryan O'Donoghue wrote: >>> On 09/12/2022 09:40, quic_mmitkov@quicinc.com wrote: >>>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>>> >>>> For v7: >>>> - Fix an issue with output state for different versions of the IFE >>>> hardware (for platforms different from QRB5, e.g. QRB3). >>>> >>> >>> Yep. >>> >>> Working for me on rb3 now and thank you for updating the git commit >>> in patch #4. >>> >>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>> >>> for the series. >>> >>> --- >>> bod >> >> >> Hi Bryan, Robert, Hans and others, >> >> >> Happy New Year! >> >> Is there anything else I can/need to do to speed up the merging >> process of this series? >> >> >> Thanks, >> >> Milen >> > > I don't think so. > > Is everything still working on linux-next ? > > e45fb347b630e...cc3c08b41a9c9 master -> linux-next/master > Hi Bryan, Yes, I took the sm8250_config from git.linaro.org/people/bryan.odonoghue/kernel.git, put it on most recent master of git.linaro.org/kernel-org/linux-next.git and build with it, virtual channels work as expected. Regard, Milen
Hi Milen, On Fri, Dec 09, 2022 at 11:40:33AM +0200, quic_mmitkov@quicinc.com wrote: > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > For v7: > - Fix an issue with output state for different versions of the IFE > hardware (for platforms different from QRB5, e.g. QRB3). > > For v6: > - Fix for a potential race condition in csid > - More detailed description on how to use/test this feature in > user-space in the last patch. > > 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. I'd like to get a high-level view of the result. Could you provide the media graph with this series applied (both -p and --print-dot) ? > 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-170.c | 4 +- > .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- > .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- > 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 +- > 9 files changed, 138 insertions(+), 64 deletions(-)
Hi Milen, On Fri, Jan 06, 2023 at 11:49:23AM +0200, Milen Mitkov (Consultant) wrote: > On 05/01/2023 20:43, Bryan O'Donoghue wrote: > > On 05/01/2023 08:37, Milen Mitkov (Consultant) wrote: > >> On 09/12/2022 18:17, Bryan O'Donoghue wrote: > >>> On 09/12/2022 09:40, quic_mmitkov@quicinc.com wrote: > >>>> From: Milen Mitkov <quic_mmitkov@quicinc.com> > >>>> > >>>> For v7: > >>>> - Fix an issue with output state for different versions of the IFE > >>>> hardware (for platforms different from QRB5, e.g. QRB3). > >>>> > >>> > >>> Yep. > >>> > >>> Working for me on rb3 now and thank you for updating the git commit > >>> in patch #4. > >>> > >>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > >>> > >>> for the series. > >>> > >>> --- > >>> bod > >> > >> Hi Bryan, Robert, Hans and others, > >> > >> Happy New Year! > >> > >> Is there anything else I can/need to do to speed up the merging > >> process of this series? > >> > >> Thanks, > >> > >> Milen > > > > I don't think so. > > > > Is everything still working on linux-next ? > > > > e45fb347b630e...cc3c08b41a9c9 master -> linux-next/master > > Hi Bryan, > > Yes, I took the sm8250_config from > git.linaro.org/people/bryan.odonoghue/kernel.git, put it on most recent > master of git.linaro.org/kernel-org/linux-next.git and build with it, > virtual channels work as expected. I haven't had much time to review the series so far, sorry about that. Spare review time is limited, and I've recently focussed on the streams API to get it merged in v6.3. I'll try to review your patches in that context.
On Sun, Jan 08, 2023 at 04:51:22AM +0200, Laurent Pinchart wrote: > Hi Milen, > > On Fri, Dec 09, 2022 at 11:40:33AM +0200, quic_mmitkov@quicinc.com wrote: > > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > > > For v7: > > - Fix an issue with output state for different versions of the IFE > > hardware (for platforms different from QRB5, e.g. QRB3). > > > > For v6: > > - Fix for a potential race condition in csid > > - More detailed description on how to use/test this feature in > > user-space in the last patch. > > > > 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. Another question, how does this work when demultiplexing different data types for the same virtual channel ? > > 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. > > I'd like to get a high-level view of the result. Could you provide the > media graph with this series applied (both -p and --print-dot) ? > > > 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-170.c | 4 +- > > .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- > > .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- > > 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 +- > > 9 files changed, 138 insertions(+), 64 deletions(-)
On 08/01/2023 04:54, Laurent Pinchart wrote: > On Sun, Jan 08, 2023 at 04:51:22AM +0200, Laurent Pinchart wrote: >> Hi Milen, >> >> On Fri, Dec 09, 2022 at 11:40:33AM +0200, quic_mmitkov@quicinc.com wrote: >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>> >>> For v7: >>> - Fix an issue with output state for different versions of the IFE >>> hardware (for platforms different from QRB5, e.g. QRB3). >>> >>> For v6: >>> - Fix for a potential race condition in csid >>> - More detailed description on how to use/test this feature in >>> user-space in the last patch. >>> >>> 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. > Another question, how does this work when demultiplexing different data > types for the same virtual channel ? > >>> 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. >> I'd like to get a high-level view of the result. Could you provide the >> media graph with this series applied (both -p and --print-dot) ? >> >>> 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-170.c | 4 +- >>> .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- >>> .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- >>> 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 +- >>> 9 files changed, 138 insertions(+), 64 deletions(-) Hi Laurent, Happy New Year! Would be good if you can help us merge this series in the upcoming merge window. I will answer your 3 questions here. Q: I'd like to get a high-level view of the result. Could you provide the media graph with this series applied (both -p and --print-dot) ? A: As I am not sure what is the policy with regards to mail attachments on the kernel mailing lists I've base64 encoded a small archive that has 4 different media-ctl dumps: -p before and after media links setup (for 2 virtual channels demuxing on CSID0) and --print-dot before and after media links setup. It can be found here: https://pastes.io/sp0pkgrgfh Copy/paste the content to a file and: base64 -d <file> | tar -gz p.s. if there's another way to share files here please mention it :) Q: How does this work when demultiplexing different data types for the same virtual channel ? A: Our open-source customers defined the scope of work and they needed virtual channels demuxing support only. Demuxing via datatype is not supported currently. Q: I was wondering if Qualcomm had a spare RB5 vision development kit that I could use for testing, both for this series, and for future development (including the libcamera side). A: We have to check internally as there's a limited supply of RB5 boards. We'll get back to you. Thanks, Milen
On 09/12/2022 11:40, quic_mmitkov@quicinc.com wrote: > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > For v7: > - Fix an issue with output state for different versions of the IFE > hardware (for platforms different from QRB5, e.g. QRB3). > > For v6: > - Fix for a potential race condition in csid > - More detailed description on how to use/test this feature in > user-space in the last patch. > > 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-170.c | 4 +- > .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- > .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- > 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 +- > 9 files changed, 138 insertions(+), 64 deletions(-) Hi guys, just a ping for this series. Laurent, I sent you an email with answers to the questions you requested. I read your reply that you'll review these changes in the context of the multi-stream API, but this series doesn't really use the multi-stream API, just a note. Cheers, Milen
On 31/01/2023 11:00, Milen Mitkov (Consultant) wrote: > On 09/12/2022 11:40, quic_mmitkov@quicinc.com wrote: >> From: Milen Mitkov <quic_mmitkov@quicinc.com> >> >> For v7: >> - Fix an issue with output state for different versions of the IFE >> hardware (for platforms different from QRB5, e.g. QRB3). >> >> For v6: >> - Fix for a potential race condition in csid >> - More detailed description on how to use/test this feature in >> user-space in the last patch. >> >> 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-170.c | 4 +- >> .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- >> .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- >> 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 +- >> 9 files changed, 138 insertions(+), 64 deletions(-) > > Hi guys, > > just a ping for this series. > > Laurent, I sent you an email with answers to the questions you > requested. I read your reply that you'll review these changes in the > context of the multi-stream API, but this series doesn't really use > the multi-stream API, just a note. > > Cheers, > > Milen > Hi there, Just another ping..:) Please let me know if there's anything I could do (improve/fix/code differently/etc.) to help get this series merged. Best Regards, Milen
On 20/02/2023 12:18, Milen Mitkov (Consultant) wrote: > On 31/01/2023 11:00, Milen Mitkov (Consultant) wrote: >> On 09/12/2022 11:40, quic_mmitkov@quicinc.com wrote: >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>> >>> For v7: >>> - Fix an issue with output state for different versions of the IFE >>> hardware (for platforms different from QRB5, e.g. QRB3). >>> >>> For v6: >>> - Fix for a potential race condition in csid >>> - More detailed description on how to use/test this feature in >>> user-space in the last patch. >>> >>> 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-170.c | 4 +- >>> .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- >>> .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- >>> 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 +- >>> 9 files changed, 138 insertions(+), 64 deletions(-) >> >> Hi guys, >> >> just a ping for this series. >> >> Laurent, I sent you an email with answers to the questions you >> requested. I read your reply that you'll review these changes in the >> context of the multi-stream API, but this series doesn't really use >> the multi-stream API, just a note. >> >> Cheers, >> >> Milen >> > Hi there, > > Just another ping..:) > > Please let me know if there's anything I could do (improve/fix/code > differently/etc.) to help get this series merged. > > > Best Regards, > > Milen > > > Well, we need to re-verify it works on linux-next. Other than that it seems OK to me. --- bod
On 20/02/2023 14:26, Bryan O'Donoghue wrote: > On 20/02/2023 12:18, Milen Mitkov (Consultant) wrote: >> On 31/01/2023 11:00, Milen Mitkov (Consultant) wrote: >>> On 09/12/2022 11:40, quic_mmitkov@quicinc.com wrote: >>>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>>> >>>> For v7: >>>> - Fix an issue with output state for different versions of the IFE >>>> hardware (for platforms different from QRB5, e.g. QRB3). >>>> >>>> For v6: >>>> - Fix for a potential race condition in csid >>>> - More detailed description on how to use/test this feature in >>>> user-space in the last patch. >>>> >>>> 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-170.c | 4 +- >>>> .../media/platform/qcom/camss/camss-vfe-480.c | 61 >>>> ++++++++++++------- >>>> .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- >>>> 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 +- >>>> 9 files changed, 138 insertions(+), 64 deletions(-) >>> >>> Hi guys, >>> >>> just a ping for this series. >>> >>> Laurent, I sent you an email with answers to the questions you >>> requested. I read your reply that you'll review these changes in the >>> context of the multi-stream API, but this series doesn't really use >>> the multi-stream API, just a note. >>> >>> Cheers, >>> >>> Milen >>> >> Hi there, >> >> Just another ping..:) >> >> Please let me know if there's anything I could do (improve/fix/code >> differently/etc.) to help get this series merged. >> >> >> Best Regards, >> >> Milen >> >> >> > > Well, we need to re-verify it works on linux-next. > > Other than that it seems OK to me. > > --- > bod Thanks Bryan, I just re-tested on latest linux-next (next-20230221 tag) and the set applies and, judging by the standard set of tests I did, works as expected and doesn't break anything. Best Regards, Milen
+ Nico (Linaro) Hi Team Would like to know if anything is pending form our end as we want to get the patches mainlined? Thanks, Azam -----Original Message----- From: Milen Mitkov (Consultant) (QUIC) <quic_mmitkov@quicinc.com> Sent: Tuesday, February 21, 2023 12:44 AM To: bryan.odonoghue@linaro.org; linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org; Azam Sadiq Pasha Kapatrala Syed <akapatra@quicinc.com>; Jigarkumar Zala <jzala@quicinc.com>; todor.too@gmail.com Cc: agross@kernel.org; konrad.dybcio@somainline.org; mchehab@kernel.org; Chandan Gera <cgera@qti.qualcomm.com>; Guru Chinnabhandar <gchinnab@quicinc.com>; Alireza Yasan <ayasan@qti.qualcomm.com>; laurent.pinchart@ideasonboard.com Subject: Re: [PATCH v7 0/4] media: camss: sm8250: Virtual channels support for SM8250 On 20/02/2023 14:26, Bryan O'Donoghue wrote: > On 20/02/2023 12:18, Milen Mitkov (Consultant) wrote: >> On 31/01/2023 11:00, Milen Mitkov (Consultant) wrote: >>> On 09/12/2022 11:40, quic_mmitkov@quicinc.com wrote: >>>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>>> >>>> For v7: >>>> - Fix an issue with output state for different versions of the IFE >>>> hardware (for platforms different from QRB5, e.g. QRB3). >>>> >>>> For v6: >>>> - Fix for a potential race condition in csid >>>> - More detailed description on how to use/test this feature in >>>> user-space in the last patch. >>>> >>>> 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-170.c | 4 +- >>>> .../media/platform/qcom/camss/camss-vfe-480.c | 61 >>>> ++++++++++++------- >>>> .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- >>>> 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 +- >>>> 9 files changed, 138 insertions(+), 64 deletions(-) >>> >>> Hi guys, >>> >>> just a ping for this series. >>> >>> Laurent, I sent you an email with answers to the questions you >>> requested. I read your reply that you'll review these changes in the >>> context of the multi-stream API, but this series doesn't really use >>> the multi-stream API, just a note. >>> >>> Cheers, >>> >>> Milen >>> >> Hi there, >> >> Just another ping..:) >> >> Please let me know if there's anything I could do (improve/fix/code >> differently/etc.) to help get this series merged. >> >> >> Best Regards, >> >> Milen >> >> >> > > Well, we need to re-verify it works on linux-next. > > Other than that it seems OK to me. > > --- > bod Thanks Bryan, I just re-tested on latest linux-next (next-20230221 tag) and the set applies and, judging by the standard set of tests I did, works as expected and doesn't break anything. Best Regards, Milen
On 31/03/2023 07:20, Azam Sadiq Pasha Kapatrala Syed wrote: > + Nico (Linaro) > Hi Team > > Would like to know if anything is pending form our end as we want to get the patches mainlined? > > Thanks, > Azam I'd like to get a clearer picture on this [ 90.535909] qcom-camss ac6a000.camss: VFE HW Version = 2.0.1 [ 90.545756] qcom-camss ac6a000.camss: CSIPHY 3PH HW Version = 0x40010000 [ 90.546358] qcom-camss ac6a000.camss: CSID HW Version = 2.0.1 [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID virtual channels mask 0x1 [ 90.547675] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID virtual channels mask 0x0 Using the IMX577 sensor on the RB5 we get his pretty odd virtual channels mask. If userspace is sending this in, the question I have is why. Surely with a sensor that doesn't have a VC there should be no impact on user-space. --- bod
On 03/04/2023 09:38, Milen Mitkov (Consultant) wrote: > On 31/03/2023 11:07, Bryan O'Donoghue wrote: >> On 31/03/2023 07:20, Azam Sadiq Pasha Kapatrala Syed wrote: >>> + Nico (Linaro) >>> Hi Team >>> >>> Would like to know if anything is pending form our end as we want to >>> get the patches mainlined? >>> >>> Thanks, >>> Azam >> >> I'd like to get a clearer picture on this >> >> [ 90.535909] qcom-camss ac6a000.camss: VFE HW Version = 2.0.1 >> [ 90.545756] qcom-camss ac6a000.camss: CSIPHY 3PH HW Version = >> 0x40010000 >> [ 90.546358] qcom-camss ac6a000.camss: CSID HW Version = 2.0.1 >> [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID >> virtual channels mask 0x1 >> [ 90.547675] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID >> virtual channels mask 0x0 >> >> Using the IMX577 sensor on the RB5 we get his pretty odd virtual >> channels mask. >> >> If userspace is sending this in, the question I have is why. Surely >> with a sensor that doesn't have a VC there should be no impact on >> user-space. >> >> --- >> bod > > Hey Bryan, > > what media-ctl commands are you using? I can't recreate this on my side. > I am using this set of commands to test (with the default imx577 driver > without any multiple virtual channels outputs) and am getting only the > first message (virtual channels mask 0x1): > > media-ctl --reset > media-ctl -v -d /dev/media0 -V '"imx577 > '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]' > media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]' > media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' > media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' > media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]' > media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > > yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0 > > > Thanks, > > Milen Its a dev_dbg() so "#define DEBUG 1" in drivers/media/platform/qcom/camss/camss-csid.c --- bod
On 03/04/2023 12:16, Bryan O'Donoghue wrote: > On 03/04/2023 09:38, Milen Mitkov (Consultant) wrote: >> On 31/03/2023 11:07, Bryan O'Donoghue wrote: >>> On 31/03/2023 07:20, Azam Sadiq Pasha Kapatrala Syed wrote: >>>> + Nico (Linaro) >>>> Hi Team >>>> >>>> Would like to know if anything is pending form our end as we want >>>> to get the patches mainlined? >>>> >>>> Thanks, >>>> Azam >>> >>> I'd like to get a clearer picture on this >>> >>> [ 90.535909] qcom-camss ac6a000.camss: VFE HW Version = 2.0.1 >>> [ 90.545756] qcom-camss ac6a000.camss: CSIPHY 3PH HW Version = >>> 0x40010000 >>> [ 90.546358] qcom-camss ac6a000.camss: CSID HW Version = 2.0.1 >>> [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>> CSID virtual channels mask 0x1 >>> [ 90.547675] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>> CSID virtual channels mask 0x0 >>> >>> Using the IMX577 sensor on the RB5 we get his pretty odd virtual >>> channels mask. >>> >>> If userspace is sending this in, the question I have is why. Surely >>> with a sensor that doesn't have a VC there should be no impact on >>> user-space. >>> >>> --- >>> bod >> >> Hey Bryan, >> >> what media-ctl commands are you using? I can't recreate this on my >> side. I am using this set of commands to test (with the default >> imx577 driver without any multiple virtual channels outputs) and am >> getting only the first message (virtual channels mask 0x1): >> >> media-ctl --reset >> media-ctl -v -d /dev/media0 -V '"imx577 >> '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]' >> media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]' >> media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]' >> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> >> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F >> /dev/video0 >> >> >> Thanks, >> >> Milen > > Its a dev_dbg() so "#define DEBUG 1" in > drivers/media/platform/qcom/camss/camss-csid.c > > --- > bod > Yes, I enabled the dev_dbg(). I just see only one message with regards to the channels mask. Just this one: [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID virtual channels mask 0x1 so I wonder if you're testing with a different set of media-ctl/yavta commands. Regards, Milen
On 03/04/2023 10:20, Milen Mitkov (Consultant) wrote: > > On 03/04/2023 12:16, Bryan O'Donoghue wrote: >> On 03/04/2023 09:38, Milen Mitkov (Consultant) wrote: >>> On 31/03/2023 11:07, Bryan O'Donoghue wrote: >>>> On 31/03/2023 07:20, Azam Sadiq Pasha Kapatrala Syed wrote: >>>>> + Nico (Linaro) >>>>> Hi Team >>>>> >>>>> Would like to know if anything is pending form our end as we want >>>>> to get the patches mainlined? >>>>> >>>>> Thanks, >>>>> Azam >>>> >>>> I'd like to get a clearer picture on this >>>> >>>> [ 90.535909] qcom-camss ac6a000.camss: VFE HW Version = 2.0.1 >>>> [ 90.545756] qcom-camss ac6a000.camss: CSIPHY 3PH HW Version = >>>> 0x40010000 >>>> [ 90.546358] qcom-camss ac6a000.camss: CSID HW Version = 2.0.1 >>>> [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>>> CSID virtual channels mask 0x1 >>>> [ 90.547675] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>>> CSID virtual channels mask 0x0 >>>> >>>> Using the IMX577 sensor on the RB5 we get his pretty odd virtual >>>> channels mask. >>>> >>>> If userspace is sending this in, the question I have is why. Surely >>>> with a sensor that doesn't have a VC there should be no impact on >>>> user-space. >>>> >>>> --- >>>> bod >>> >>> Hey Bryan, >>> >>> what media-ctl commands are you using? I can't recreate this on my >>> side. I am using this set of commands to test (with the default >>> imx577 driver without any multiple virtual channels outputs) and am >>> getting only the first message (virtual channels mask 0x1): >>> >>> media-ctl --reset >>> media-ctl -v -d /dev/media0 -V '"imx577 >>> '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]' >>> media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]' >>> media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >>> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >>> media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]' >>> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >>> >>> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F >>> /dev/video0 >>> >>> >>> Thanks, >>> >>> Milen >> >> Its a dev_dbg() so "#define DEBUG 1" in >> drivers/media/platform/qcom/camss/camss-csid.c >> >> --- >> bod >> > Yes, I enabled the dev_dbg(). I just see only one message with regards > to the channels mask. Just this one: > > [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled CSID > virtual channels mask 0x1 > > so I wonder if you're testing with a different set of media-ctl/yavta > commands. No. I'm asking about how this _used_ to be. If the iteration through the masks went away - then why did it go away ? libcamera version, a change you have made since - V5 I think it was when we discussed this last ? --- bod
On 03/04/2023 12:36, Bryan O'Donoghue wrote: > On 03/04/2023 10:20, Milen Mitkov (Consultant) wrote: >> >> On 03/04/2023 12:16, Bryan O'Donoghue wrote: >>> On 03/04/2023 09:38, Milen Mitkov (Consultant) wrote: >>>> On 31/03/2023 11:07, Bryan O'Donoghue wrote: >>>>> On 31/03/2023 07:20, Azam Sadiq Pasha Kapatrala Syed wrote: >>>>>> + Nico (Linaro) >>>>>> Hi Team >>>>>> >>>>>> Would like to know if anything is pending form our end as we want >>>>>> to get the patches mainlined? >>>>>> >>>>>> Thanks, >>>>>> Azam >>>>> >>>>> I'd like to get a clearer picture on this >>>>> >>>>> [ 90.535909] qcom-camss ac6a000.camss: VFE HW Version = 2.0.1 >>>>> [ 90.545756] qcom-camss ac6a000.camss: CSIPHY 3PH HW Version = >>>>> 0x40010000 >>>>> [ 90.546358] qcom-camss ac6a000.camss: CSID HW Version = 2.0.1 >>>>> [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>>>> CSID virtual channels mask 0x1 >>>>> [ 90.547675] qcom-camss ac6a000.camss: csid_link_setup: Enabled >>>>> CSID virtual channels mask 0x0 >>>>> >>>>> Using the IMX577 sensor on the RB5 we get his pretty odd virtual >>>>> channels mask. >>>>> >>>>> If userspace is sending this in, the question I have is why. >>>>> Surely with a sensor that doesn't have a VC there should be no >>>>> impact on user-space. >>>>> >>>>> --- >>>>> bod >>>> >>>> Hey Bryan, >>>> >>>> what media-ctl commands are you using? I can't recreate this on my >>>> side. I am using this set of commands to test (with the default >>>> imx577 driver without any multiple virtual channels outputs) and am >>>> getting only the first message (virtual channels mask 0x1): >>>> >>>> media-ctl --reset >>>> media-ctl -v -d /dev/media0 -V '"imx577 >>>> '22-001a'":0[fmt:SRGGB10/4056x3040 field:none]' >>>> media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]' >>>> media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >>>> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >>>> media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]' >>>> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >>>> >>>> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F >>>> /dev/video0 >>>> >>>> >>>> Thanks, >>>> >>>> Milen >>> >>> Its a dev_dbg() so "#define DEBUG 1" in >>> drivers/media/platform/qcom/camss/camss-csid.c >>> >>> --- >>> bod >>> >> Yes, I enabled the dev_dbg(). I just see only one message with >> regards to the channels mask. Just this one: >> >> [ 90.546365] qcom-camss ac6a000.camss: csid_link_setup: Enabled >> CSID virtual channels mask 0x1 >> >> so I wonder if you're testing with a different set of media-ctl/yavta >> commands. > > No. I'm asking about how this _used_ to be. > > If the iteration through the masks went away - then why did it go away > ? libcamera version, a change you have made since - V5 I think it was > when we discussed this last ? > > --- > bod > Hi Bryan, no, the iteration through the mask didn't go away? The print shows up when the csid entity's source pad(s) enables the link to the ife sink pad(s). Maybe the client (libcamera?) decides to disable this link for some reason? Regards, Milen
On 03/04/2023 12:01, Milen Mitkov (Consultant) wrote: > Hi Bryan, > > no, the iteration through the mask didn't go away? The print shows up > when the csid entity's source pad(s) enables the link to the ife sink > pad(s). Maybe the client (libcamera?) decides to disable this link for > some reason? > > Regards, > > Milen So previously we had one CSI device in user-space and after your change we have one CSI device per VC, correct ? --- bod
On 03/04/2023 15:10, Bryan O'Donoghue wrote: > On 03/04/2023 12:01, Milen Mitkov (Consultant) wrote: >> Hi Bryan, >> >> no, the iteration through the mask didn't go away? The print shows up >> when the csid entity's source pad(s) enables the link to the ife sink >> pad(s). Maybe the client (libcamera?) decides to disable this link >> for some reason? >> >> Regards, >> >> Milen > > So previously we had one CSI device in user-space and after your > change we have one CSI device per VC, correct ? > > --- > bod With these changes there's still one CSID device/media entity, but it has more source pads (4 vs 1 previously). Regards, Milen
On 03/04/2023 13:16, Milen Mitkov (Consultant) wrote: > > On 03/04/2023 15:10, Bryan O'Donoghue wrote: >> On 03/04/2023 12:01, Milen Mitkov (Consultant) wrote: >>> Hi Bryan, >>> >>> no, the iteration through the mask didn't go away? The print shows up >>> when the csid entity's source pad(s) enables the link to the ife sink >>> pad(s). Maybe the client (libcamera?) decides to disable this link >>> for some reason? >>> >>> Regards, >>> >>> Milen >> >> So previously we had one CSI device in user-space and after your >> change we have one CSI device per VC, correct ? >> >> --- >> bod > > With these changes there's still one CSID device/media entity, but it > has more source pads (4 vs 1 previously). > > Regards, > > Milen > OK. I took the time to apply this series to my development tree. Before: - entity 19: msm_csid0 (2 pads, 10 links) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev6 pad0: Sink [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] <- "msm_csiphy0":1 [] <- "msm_csiphy1":1 [] <- "msm_csiphy2":1 [] <- "msm_csiphy3":1 [] <- "msm_csiphy4":1 [] <- "msm_csiphy5":1 [] pad1: Source [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] -> "msm_vfe0_rdi0":0 [] -> "msm_vfe1_rdi0":0 [] -> "msm_vfe2_rdi0":0 [] -> "msm_vfe3_rdi0":0 [] After: - entity 19: msm_csid0 (5 pads, 22 links) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev6 pad0: Sink [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] <- "msm_csiphy0":1 [] <- "msm_csiphy1":1 [] <- "msm_csiphy2":1 [ENABLED] <- "msm_csiphy3":1 [] <- "msm_csiphy4":1 [] <- "msm_csiphy5":1 [] pad1: Source [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] -> "msm_vfe0_rdi0":0 [ENABLED] -> "msm_vfe1_rdi0":0 [] -> "msm_vfe2_rdi0":0 [] -> "msm_vfe3_rdi0":0 [] pad2: Source [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] -> "msm_vfe0_rdi1":0 [] -> "msm_vfe1_rdi1":0 [] -> "msm_vfe2_rdi1":0 [] -> "msm_vfe3_rdi1":0 [] pad3: Source [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] -> "msm_vfe0_rdi2":0 [] -> "msm_vfe1_rdi2":0 [] -> "msm_vfe2_rdi2":0 [] -> "msm_vfe3_rdi2":0 [] pad4: Source [fmt:UYVY8_2X8/1920x1080 field:none colorspace:srgb] -> "msm_vfe0_pix":0 [] -> "msm_vfe1_pix":0 [] -> "msm_vfe2_pix":0 [] -> "msm_vfe3_pix":0 [] So that's consistent and this worked for me. media-ctl --reset media-ctl -v -d /dev/media0 -V '"imx577 '20-001a'":0[fmt:SRGGB10/4056x3040 field:none]' media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' build/yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0 For the series. Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
From: Milen Mitkov <quic_mmitkov@quicinc.com> For v7: - Fix an issue with output state for different versions of the IFE hardware (for platforms different from QRB5, e.g. QRB3). For v6: - Fix for a potential race condition in csid - More detailed description on how to use/test this feature in user-space in the last patch. 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-170.c | 4 +- .../media/platform/qcom/camss/camss-vfe-480.c | 61 ++++++++++++------- .../platform/qcom/camss/camss-vfe-gen1.c | 4 +- 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 +- 9 files changed, 138 insertions(+), 64 deletions(-)