+ 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@xxxxxxxxxxx> Sent: Tuesday, February 21, 2023 12:44 AM To: bryan.odonoghue@xxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; Azam Sadiq Pasha Kapatrala Syed <akapatra@xxxxxxxxxxx>; Jigarkumar Zala <jzala@xxxxxxxxxxx>; todor.too@xxxxxxxxx Cc: agross@xxxxxxxxxx; konrad.dybcio@xxxxxxxxxxxxxx; mchehab@xxxxxxxxxx; Chandan Gera <cgera@xxxxxxxxxxxxxxxx>; Guru Chinnabhandar <gchinnab@xxxxxxxxxxx>; Alireza Yasan <ayasan@xxxxxxxxxxxxxxxx>; laurent.pinchart@xxxxxxxxxxxxxxxx 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@xxxxxxxxxxx wrote: >>>> From: Milen Mitkov <quic_mmitkov@xxxxxxxxxxx> >>>> >>>> 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