| Message ID | 20221013121255.1977-1-quic_mmitkov@quicinc.com |
|---|---|
| Headers | show |
| Series | media: camss: sm8250: Virtual channels support for SM8250 | expand |
Hello Milen, Thank you for the patches. I'll try to give this an initial review soon. What hardware platform would you recommend, if any, if I wanted to test this series ? Ideally that would be an off-the-shelf development board that can easily be sourced, and that wouldn't require out-of-tree drivers (beside perhaps for the camera sensor if it's not in mainline yet) to test the camss driver (ethernet would be very appreciated, so would display support). On Thu, Oct 13, 2022 at 03:12:51PM +0300, quic_mmitkov@quicinc.com wrote: > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > 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 | 7 +++ > .../media/platform/qcom/camss/camss-video.c | 21 ++++++- > drivers/media/platform/qcom/camss/camss.c | 2 +- > 7 files changed, 140 insertions(+), 60 deletions(-)
On 17/10/2022 12:47, Milen Mitkov (Consultant) wrote: > > Hey Bryan, > > I don't see the second print (..."virtual channels mask 0x0") on my side > when testing with the standard imx577 driver. I also have imx577 on slot > 2 and ov9282 on slot 1. I am testing with: > > 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 > > > What is your testing procedure? > > > Regards, So for that I just do https://libcamera.org/getting-started.html apt install -y g++ meson libyaml-dev python3-yaml python3-ply python3-jinja2 libssl-dev libdw-dev libunwind-dev libudev-dev libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev libevent-dev git clone https://git.libcamera.org/libcamera/libcamera.git cd libcamera meson build ninja -C build cd build/src/cam ./cam -l Available cameras: 1: 'ov9282' (/base/soc@0/cci@ac4f000/i2c-bus@1/camera@60) 2: 'imx577' (/base/soc@0/cci@ac50000/i2c-bus@0/camera@1a) ./cam -c 2 --capture=10 --file Do you have the ov9282 probed ? To ssh://git.linaro.org/people/bryan.odonoghue/kernel.git * [new branch] HEAD -> next-20221013-sm8250-camss-vc --- bod
Quoting Bryan O'Donoghue (2022-10-17 15:22:10) > On 17/10/2022 14:06, Kieran Bingham wrote: > > Quoting Bryan O'Donoghue (2022-10-17 01:16:05) > >> On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: > >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> > >>> > >>> 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 | 7 +++ > >>> .../media/platform/qcom/camss/camss-video.c | 21 ++++++- > >>> drivers/media/platform/qcom/camss/camss.c | 2 +- > >>> 7 files changed, 140 insertions(+), 60 deletions(-) > >>> > >> > >> Hi Milen > >> > >> The set applies to next-20221013 including patch#4. > >> > >> I can confirm it doesn't break anything for me - though my sensor is a > >> bog-standard imx577 so it doesn't have any VC support. > > > > Interesting though - the IMX477 has the ability to convey metadata on a > > separate VC... That's actually the thing holding back the RPi IMX477 > > driver from mainline, as the way it was anticipated to support multiple > > data streams is with the new multiplexed streams API. > > > > And I think we inferred that the IMX577 and IMX477 are closely related, > > so should have similar capabilities for obtaining metadata channels? > > Hmm I was not aware of that. > > If we could import the rpi/imx477.c code into upstrea/imx412.c it might > be possible > > The core init is very similar > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx477.c#L167 > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx412.c#L160 > > Maybe it would be possible to apply the rest of the imx477 config on-top > as a POC > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx477.c#L479 > > The similary is born out by the shared init code I can see in Leopard > imaging's driver, I'm not sure if it supports virtual-channels - I'll > have a look, though. > > What's in the imx477 meta-data ? The exact exposure of the captured frame, exact gain, and frame length, and even the temperature of the sensor at the time of capture (not sure at /which/ time if this is a long exposure). https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/raspberrypi/cam_helper_imx477.cpp#n168 """ void CamHelperImx477::populateMetadata(const MdParser::RegisterMap ®isters, Metadata &metadata) const { DeviceStatus deviceStatus; deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg)); deviceStatus.analogueGain = gain(registers.at(gainHiReg) * 256 + registers.at(gainLoReg)); deviceStatus.frameLength = registers.at(frameLengthHiReg) * 256 + registers.at(frameLengthLoReg); deviceStatus.sensorTemperature = std::clamp<int8_t>(registers.at(temperatureReg), -20, 80); metadata.set("device.status", deviceStatus); } """ Having the embedded metadata from the sensor helps to ensure accurate handling in the control loops, so I believe we would always prefer to reference this when available, rather than what we 'think' we have programmed. (Which due to timing, or any other error - might not be as accurate as what the metadata will report). -- Kieran > > @Milen if you have the imx577 datasheet - I don't - perhaps we could > cherry-pick some of the code from imx477 and get the imx412.c->imx577 > dumping VC data out with the RB5 camera mezzanine. > > --- > bod
On 17/10/2022 17:22, Bryan O'Donoghue wrote: > On 17/10/2022 14:06, Kieran Bingham wrote: >> Quoting Bryan O'Donoghue (2022-10-17 01:16:05) >>> On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: >>>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>>> >>>> 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 | 7 +++ >>>> .../media/platform/qcom/camss/camss-video.c | 21 ++++++- >>>> drivers/media/platform/qcom/camss/camss.c | 2 +- >>>> 7 files changed, 140 insertions(+), 60 deletions(-) >>>> >>> >>> Hi Milen >>> >>> The set applies to next-20221013 including patch#4. >>> >>> I can confirm it doesn't break anything for me - though my sensor is a >>> bog-standard imx577 so it doesn't have any VC support. >> >> Interesting though - the IMX477 has the ability to convey metadata on a >> separate VC... That's actually the thing holding back the RPi IMX477 >> driver from mainline, as the way it was anticipated to support multiple >> data streams is with the new multiplexed streams API. >> >> And I think we inferred that the IMX577 and IMX477 are closely related, >> so should have similar capabilities for obtaining metadata channels? > > Hmm I was not aware of that. > > If we could import the rpi/imx477.c code into upstrea/imx412.c it > might be possible > > The core init is very similar > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx477.c#L167 > > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx412.c#L160 > > > Maybe it would be possible to apply the rest of the imx477 config > on-top as a POC > > https://github.com/raspberrypi/linux/blob/rpi-5.19.y/drivers/media/i2c/imx477.c#L479 > > > The similary is born out by the shared init code I can see in Leopard > imaging's driver, I'm not sure if it supports virtual-channels - I'll > have a look, though. > > What's in the imx477 meta-data ? > > @Milen if you have the imx577 datasheet - I don't - perhaps we could > cherry-pick some of the code from imx477 and get the imx412.c->imx577 > dumping VC data out with the RB5 camera mezzanine. > > --- > bod Bryan, contact Nicolas Dechesne (from Linaro) with regards to the imx577 datasheet. I don't have permissions. Regards, Milen
On 17/10/2022 16:06, Kieran Bingham wrote: > Quoting Bryan O'Donoghue (2022-10-17 01:16:05) >> On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>> >>> 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 | 7 +++ >>> .../media/platform/qcom/camss/camss-video.c | 21 ++++++- >>> drivers/media/platform/qcom/camss/camss.c | 2 +- >>> 7 files changed, 140 insertions(+), 60 deletions(-) >>> >> >> Hi Milen >> >> The set applies to next-20221013 including patch#4. >> >> I can confirm it doesn't break anything for me - though my sensor is a >> bog-standard imx577 so it doesn't have any VC support. > > Interesting though - the IMX477 has the ability to convey metadata on a > separate VC... That's actually the thing holding back the RPi IMX477 > driver from mainline, as the way it was anticipated to support multiple > data streams is with the new multiplexed streams API. Note that while the streams series enables the support for multiple streams, metadata support is not included. This mainly means the metadata mbus/v4l2 formats. We've had some discussions about this with Laurent and Sakari, and it sounds like a rather complex issue (although you can skip the problems by just adding a custom metadata format, which I'm doing in my work branch to test metadata). Tomi
On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > 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 | 7 +++ > .../media/platform/qcom/camss/camss-video.c | 21 ++++++- > drivers/media/platform/qcom/camss/camss.c | 2 +- > 7 files changed, 140 insertions(+), 60 deletions(-) > I've done some offline work with Milen on this. I'm happy enough to add my Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> to the series. I don't have - currently a VC enabled setup but for the simple case this set doesn't break anything on RB5 for me. --- bod
On Tue, 8 Nov 2022 at 01:12, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > > On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: > > From: Milen Mitkov <quic_mmitkov@quicinc.com> > > > > 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 | 7 +++ > > .../media/platform/qcom/camss/camss-video.c | 21 ++++++- > > drivers/media/platform/qcom/camss/camss.c | 2 +- > > 7 files changed, 140 insertions(+), 60 deletions(-) > > > > I've done some offline work with Milen on this. > > I'm happy enough to add my > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > to the series. I don't have - currently a VC enabled setup but for the > simple case this set doesn't break anything on RB5 for me. > > --- > bod This series has my ack. Acked-by: Robert Foss <robert.foss@linaro.org>
On 08/11/2022 12:35, Robert Foss wrote: > On Tue, 8 Nov 2022 at 01:12, Bryan O'Donoghue > <bryan.odonoghue@linaro.org> wrote: >> On 13/10/2022 13:12, quic_mmitkov@quicinc.com wrote: >>> From: Milen Mitkov <quic_mmitkov@quicinc.com> >>> >>> 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 | 7 +++ >>> .../media/platform/qcom/camss/camss-video.c | 21 ++++++- >>> drivers/media/platform/qcom/camss/camss.c | 2 +- >>> 7 files changed, 140 insertions(+), 60 deletions(-) >>> >> I've done some offline work with Milen on this. >> >> I'm happy enough to add my >> >> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> >> to the series. I don't have - currently a VC enabled setup but for the >> simple case this set doesn't break anything on RB5 for me. >> >> --- >> bod > This series has my ack. > > Acked-by: Robert Foss <robert.foss@linaro.org> Hi Robert, Bryan et al, Thanks for the Tested-by and Acked-by. Do I need to resubmit the patches with the added new tags? If this is not needed, is there something else I can do to help the merging process? Thank you, Milen
On 14/11/2022 17:18, Milen Mitkov (Consultant) wrote: > > Do I need to resubmit the patches with the added new tags? Not the committer can do that for you. > If this is not needed, is there something else I can do to help the > merging process? It looks like Hans has been merging most of this stuff on the linux-media side. + Hans --- bod
From: Milen Mitkov <quic_mmitkov@quicinc.com> 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 | 7 +++ .../media/platform/qcom/camss/camss-video.c | 21 ++++++- drivers/media/platform/qcom/camss/camss.c | 2 +- 7 files changed, 140 insertions(+), 60 deletions(-)