drivers/media/platform/qcom/camss/camss-csid-340.c | 17 ++++---- drivers/media/platform/qcom/camss/camss-csid-680.c | 28 ++++++------- .../media/platform/qcom/camss/camss-csid-gen2.c | 48 +++++++++++----------- .../media/platform/qcom/camss/camss-csid-gen3.c | 32 +++++++-------- drivers/media/platform/qcom/camss/camss-csid.c | 10 ++--- drivers/media/platform/qcom/camss/camss-csid.h | 2 +- 6 files changed, 69 insertions(+), 68 deletions(-)
A serious bug has been copy/pasted from CSID 170 into various different
implementations of the CSID.
In simple terms we have a broken model of enabling virtual channels which
needs to be rewritten.
Taking the CSID 680 as an example. The CSID can output ports RDI0, RDI1,
RDI2 and PIX.
Each CSIPHY can connect to any of the CSIDs. Each CSID has four output
ports RDI0, RDI1, RDI2 and PIX. To get Bayer statistics going we need the
CSID to write on the PIX port.
Each of the RDI/PIX ports can process any valid virtual channel.
When adding virtual channels a spurious association was made between
virtual channel and the above mentioned ports. In simple terms
vfeN_rdi0 will always be fixed to VCO
vfeN_rdi1 will always be fixed to VC1
vfeN_rdi2 will always be fixed to VC2
vfeN_pix will always be fixed to VC3
What this means in practice is that it is impossible to route a sensor
driving VC:0 to the PIX/Bayer path in upstream.
Given we have now gotten a mutli-stream support in the kernel upstream we
should move to that API in CAMSS.
First up though is to remove the breakage of invalid VC constrains and make
those available to stable.
Here's the practical example on CSID680.
This works:
media-ctl --reset
media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
media-ctl -l '"msm_csiphy4":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 SGRBG10P -s 3856x2416 -F /dev/video3
This doesn't:
media-ctl --reset
media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_vfe0_rdi2":0[fmt:SGRBG10/3856x2416]'
media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":3->"msm_vfe0_rdi2":0[1]'
yavta -B capture-mplane -c -I -n 5 -f SGRBG10P -s 3856x2416 -F /dev/video5
The PIX path - Bayer stats requires more work still but in simple terms it
needs a pipeline like this:
media-ctl --reset
media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_vfe0_pix":0[fmt:SGRBG10/3856x2416]'
media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":3->"msm_vfe0_pix":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane -c -I -n 5 -f SGRBG10P -s 3856x2416 -F /dev/video6
But that can't work with the above sensor because that sensor puts out VC0
not VC3.
Constraining the CSID/VFE ports to individual VCs was never the intention
and since we have v4l2 subdev streams we should align CAMSS to that which
will ultimately allow us to map any VC to any port without _requiring_ a VC
to enable a particular port.
Signed-off-by: Bryan O'Donoghue <bod@kernel.org>
---
Bryan O'Donoghue (5):
media: qcom: camss: Fix RDI streaming for CSID 680
media: qcom: camss: Fix RDI streaming for CSID 340
media: qcom: camss: Fix RDI streaming for CSID GEN2
media: qcom: camss: Fix RDI streaming for CSID GEN3
media: qcom: camss: csid: Rename en_vc to en_port
drivers/media/platform/qcom/camss/camss-csid-340.c | 17 ++++----
drivers/media/platform/qcom/camss/camss-csid-680.c | 28 ++++++-------
.../media/platform/qcom/camss/camss-csid-gen2.c | 48 +++++++++++-----------
.../media/platform/qcom/camss/camss-csid-gen3.c | 32 +++++++--------
drivers/media/platform/qcom/camss/camss-csid.c | 10 ++---
drivers/media/platform/qcom/camss/camss-csid.h | 2 +-
6 files changed, 69 insertions(+), 68 deletions(-)
---
base-commit: 2211e826bd69c041534093735241182013dde7bc
change-id: 20260406-camss-rdi-fix-ddd769490ec2
Best regards,
--
Bryan O'Donoghue <bod@kernel.org>
Hi Bryan, On Mon, Apr 6, 2026 at 11:55 PM <bod@kernel.org> wrote: > > A serious bug has been copy/pasted from CSID 170 into various different > implementations of the CSID. > > In simple terms we have a broken model of enabling virtual channels which > needs to be rewritten. > > Taking the CSID 680 as an example. The CSID can output ports RDI0, RDI1, > RDI2 and PIX. > > Each CSIPHY can connect to any of the CSIDs. Each CSID has four output > ports RDI0, RDI1, RDI2 and PIX. To get Bayer statistics going we need the > CSID to write on the PIX port. > > Each of the RDI/PIX ports can process any valid virtual channel. > > When adding virtual channels a spurious association was made between > virtual channel and the above mentioned ports. In simple terms > > vfeN_rdi0 will always be fixed to VCO > vfeN_rdi1 will always be fixed to VC1 > vfeN_rdi2 will always be fixed to VC2 > vfeN_pix will always be fixed to VC3 > > What this means in practice is that it is impossible to route a sensor > driving VC:0 to the PIX/Bayer path in upstream. > > Given we have now gotten a mutli-stream support in the kernel upstream we > should move to that API in CAMSS. > > First up though is to remove the breakage of invalid VC constrains and make > those available to stable. I agree with the observation and conclusion that proper PORT and VC support is needed. However, as things stand today, this mechanism is also a convenient API for leveraging different virtual channels. Concretely, if you want to receive data from both VC0 and VC1, you can simply use RDI0 and RDI1. Changing this behavior would effectively break that usage model, leaving us only able to retrieve VC0 data, which feels like a regression to me. The more compelling use case, in my view, is the ability to stream different VCs in parallel, rather than streaming VC0 multiple times? This then brings us to the Pix interface, where streaming something like VC3 does not really make sense. In the current csid-340 series [1], I therefore took a simpler approach/workaround of forcing the main channel (VC0) for the Pix interface. [1] https://lore.kernel.org/linux-media/20260313131750.187518-4-loic.poulain@oss.qualcomm.com Regards, Loic
On 07/04/2026 09:16, Loic Poulain wrote: > I agree with the observation and conclusion that proper PORT and VC > support is needed. However, as things stand today, this mechanism is > also a convenient API for leveraging different virtual channels. > Concretely, if you want to receive data from both VC0 and VC1, you can > simply use RDI0 and RDI1. Changing this behavior would effectively > break that usage model, leaving us only able to retrieve VC0 data, > which feels like a regression to me. The more compelling use case, in > my view, is the ability to stream different VCs in parallel, rather > than streaming VC0 multiple times? > > This then brings us to the Pix interface, where streaming something > like VC3 does not really make sense. In the current csid-340 series > [1], I therefore took a simpler approach/workaround of forcing the > main channel (VC0) for the Pix interface. > > [1]https://lore.kernel.org/linux-media/20260313131750.187518-4- > loic.poulain@oss.qualcomm.com I thought about that however, there are no upstream sensors driving more than once VC right now. So this really is a bugfix. You can even see it in the original commit message for this feature, imx412 was used in the example but imx412 doesn't support multiple VCs. This is a pure bugfix and now that you draw my attention to it, I think you should update your series. I guess this explains the indexing stuff I was nagging you about. --- bod
On Tue, Apr 7, 2026 at 10:36 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > > On 07/04/2026 09:16, Loic Poulain wrote: > > I agree with the observation and conclusion that proper PORT and VC > > support is needed. However, as things stand today, this mechanism is > > also a convenient API for leveraging different virtual channels. > > Concretely, if you want to receive data from both VC0 and VC1, you can > > simply use RDI0 and RDI1. Changing this behavior would effectively > > break that usage model, leaving us only able to retrieve VC0 data, > > which feels like a regression to me. The more compelling use case, in > > my view, is the ability to stream different VCs in parallel, rather > > than streaming VC0 multiple times? > > > > This then brings us to the Pix interface, where streaming something > > like VC3 does not really make sense. In the current csid-340 series > > [1], I therefore took a simpler approach/workaround of forcing the > > main channel (VC0) for the Pix interface. > > > > [1]https://lore.kernel.org/linux-media/20260313131750.187518-4- > > loic.poulain@oss.qualcomm.com > > I thought about that however, there are no upstream sensors driving more > than once VC right now. > > So this really is a bugfix. You can even see it in the original commit > message for this feature, imx412 was used in the example but imx412 > doesn't support multiple VCs. Okay, then that does reduce the usefulness somewhat... Another point I hadn’t initially considered is that we may also want to support different data types on the same VC. For example, metadata, stats, and image data could be transmitted over the same VC/stream? That seems like a valid use case enabled by your fix, and it might be worth explicitly mentioning it. > > This is a pure bugfix and now that you draw my attention to it, I think > you should update your series. Yes, I'll consider this in the next version. Regards, Loic
On 07/04/2026 09:49, Loic Poulain wrote: > Okay, then that does reduce the usefulness somewhat... Another point I > hadn’t initially considered is that we may also want to support > different data types on the same VC. For example, metadata, stats, and > image data could be transmitted over the same VC/stream? That seems > like a valid use case enabled by your fix, and it might be worth > explicitly mentioning it. Yes true you can also separate out based on DT. We could have different data-types on the same VC but of course you'd see the DT field in data-stream change and then we'd route that to a different rdi on the VFE. Should be possible to interpret that data from the streams API and route but it is indeed VC+DT to vfe_rdiX where both determine X not just the VC. --- bod
© 2016 - 2026 Red Hat, Inc.