Re: [PATCH v7 0/4] media: camss: sm8250: Virtual channels support for SM8250

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

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