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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+ 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






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux