Re: [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port
From: Bryan O'Donoghue
Date: Tue Apr 07 2026 - 03:41:58 EST
On 07/04/2026 03:24, Hangxiang Ma wrote:
On 4/7/2026 5:55 AM, bod@xxxxxxxxxx wrote:
From: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
The en_vc mask has always also been an en_port mask. Name the variable for
what it does a bitmask of ports. When implementing v4l2 subdev streams it
probably makes more sense to have tuples for port/vc mappings. Such a
change right now feels like putting the cart before the horse.
Sanitise the name in the interregnum.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
drivers/media/platform/qcom/camss/camss-csid-340.c | 2 +-
drivers/media/platform/qcom/camss/camss-csid-680.c | 2 +-
drivers/media/platform/qcom/camss/camss-csid-gen2.c | 4 ++--
drivers/media/platform/qcom/camss/camss-csid-gen3.c | 6 +++---
drivers/media/platform/qcom/camss/camss-csid.c | 10 +++++-----
drivers/media/platform/qcom/camss/camss-csid.h | 2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-340.c b/drivers/media/platform/qcom/camss/camss-csid-340.c
index 5a7271785ec7a..da5e03b340bb7 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-340.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-340.c
@@ -119,7 +119,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
__csid_configure_rx(csid, &csid->phy);
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_ctrl_rdi(csid, enable, i);
}
diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 35a6bb209f97c..80d8bcd6e0854 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -292,7 +292,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
/* Loop through all enabled VCs and configure stream for each */
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_configure_rx(csid, &csid->phy, 0);
__csid_ctrl_rdi(csid, enable, i);
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index 331feed199094..e2d14b25f8c85 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -328,7 +328,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
u8 i;
/* Loop through all enabled VCs and configure stream for each */
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
if (tg->enabled)
__csid_configure_testgen(csid, enable, i, 0);
@@ -369,7 +369,7 @@ static irqreturn_t csid_isr(int irq, void *dev)
/* Read and clear IRQ status for each enabled RDI channel */
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
}
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 73504c349fd0b..b92234ba84efc 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -215,7 +215,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
/* Loop through all enabled VCs and configure stream for each */
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_configure_rx(csid, &csid->phy, 0);
__csid_ctrl_rdi(csid, enable, i);
@@ -263,7 +263,7 @@ static irqreturn_t csid_isr(int irq, void *dev)
/* Read and clear IRQ status for each enabled RDI channel */
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
@@ -309,7 +309,7 @@ static int csid_reset(struct csid_device *csid)
writel(1, csid->base + CSID_TOP_IRQ_MASK);
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
- if (csid->phy.en_vc & BIT(i)) {
+ if (csid->phy.en_port & BIT(i)) {
writel(BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i),
csid->base + CSID_BUF_DONE_IRQ_CLEAR);
writel(IRQ_CMD_CLEAR, csid->base + CSID_IRQ_CMD);
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index ed1820488c987..71a40c2cb350b 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -1278,21 +1278,21 @@ static int csid_link_setup(struct media_entity *entity,
csid->phy.lane_cnt = lane_cfg->num_data;
csid->phy.lane_assign = csid_get_lane_assign(lane_cfg);
}
- /* Decide which virtual channels to enable based on which source pads are enabled */
+ /* Decide which ports to enable based on which source pads are enabled */
if (local->flags & MEDIA_PAD_FL_SOURCE) {
struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
struct csid_device *csid = v4l2_get_subdevdata(sd);
struct device *dev = csid->camss->dev;
if (flags & MEDIA_LNK_FL_ENABLED)
- csid->phy.en_vc |= BIT(local->index - 1);
+ csid->phy.en_port |= BIT(local->index - 1);
else
- csid->phy.en_vc &= ~BIT(local->index - 1);
+ csid->phy.en_port &= ~BIT(local->index - 1);
csid->phy.need_vc_update = true;
- dev_dbg(dev, "%s: Enabled CSID virtual channels mask 0x%x\n",
- __func__, csid->phy.en_vc);
+ dev_dbg(dev, "%s: Enabled CSID ports mask 0x%x\n",
+ __func__, csid->phy.en_port);
}
return 0;
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index aedc96ed84b2f..b227923ca5c15 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -68,7 +68,7 @@ struct csid_phy_config {
u8 csiphy_id;
u8 lane_cnt;
u32 lane_assign;
- u32 en_vc;
+ u32 en_port;
u8 need_vc_update;
};
I want to suggest a feasible way to solve this issue. The v4l2 framework
provide a standard interface 'get_frame_desc' to acquire vc/dt, stream,
format, etc. The vc/dt is determined by sensor in fact. Could we use
this inteface to populate such information from sensor side and use them
in csid? If sensor driver doesn't provide the desc, we can use the
default vc0.
I have made some tests locally, the only gap is we need to determine the
vc binding order and ask the user to follow it if multiple streams are
configured simultaneously. Maybe we can handle this more flexible if we
come up with new ideas.
Something like below
1. Configure RDI0 first then RDI1 will get
RDI0 -> vc0
RDI1 -> vc1
2. Configure RDI1 first then RDI0 will get
RDI1 -> vc0
RDI0 -> vc1
Best regards,
Hangxiang
I think there is a solution upstream we could reuse instead.
If you're interested in fixing this, I think the way to go is the set-route/set-routing syntax
- Add V4L2_SUBDEV_FL_STREAMS to CSID
- Support the routing syntax
- Continue to support the non-routing syntax
i.e. when not doing set route, default to VC0 and not requiring
the routing syntax to understand that VC0 is default
Let's pretend ov08x40 supports two VCs we want the existing syntax for the non-VC case to continue as-is
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]'
media-ctl -d /dev/media0 -p
And then the routing syntax - something like the following
media-ctl --reset
# Sensor format
media-ctl -V '"ov08x40 2-0036":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
# CSID sink format
media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
# Route VC0 to pad1 (RDI0), VC1 to pad2 (RDI1)
media-ctl -R '"msm_csid0" [0/0->1/0[1], 0/1->2/0[1]]'
# Set format on each routed source pad
media-ctl -V '"msm_csid0":1[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csid0":2[fmt:SGRBG10/3856x2416]'
# VFE sink formats
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_vfe0_rdi1":0[fmt:SGRBG10/3856x2416]'
# Links
media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -l '"msm_csid0":2->"msm_vfe0_rdi1":0[1]'
# Capture VC0 on /dev/video2, VC1 on /dev/video3
yavta -B capture-mplane -c -n 1000 -f SGRBG10P -s 3856x2416 -F /dev/video2 &
yavta -B capture-mplane -c -n 1000 -f SGRBG10P -s 3856x2416 -F /dev/video3 &
---
bod