* [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
@ 2026-04-06 21:55 ` bod
2026-04-06 22:41 ` Vladimir Zapolskiy
2026-04-06 21:55 ` [PATCH 2/5] media: qcom: camss: Fix RDI streaming for CSID 340 bod
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: bod @ 2026-04-06 21:55 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
stable
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Fix streaming to RDI1 and RDI2. csid->phy.en_vc contains a bitmask of
enabled CSID ports not virtual channels.
We cycle through the number of available CSID ports and test this value
against the vc_en bitmask.
We then use the passed value both as an index to the port configuration
macros and as a virtual channel index.
This is a very broken pattern. Reviewing the initial introduction of VC
support it states that you can only map one CSID to one VFE. This is true
however each CSID has multiple sources which can sink inside of the VFE -
for example there is a "pixel" path for bayer stats which sources @
CSID(x):3 and sinks on VFE(x):pix.
That is CSID port # 3 should drive VFE port #3. With our current setup only
a sensor which drives virtual channel number #3 could possibly enable that
setup.
This is deeply wrong the virtual channel has no relevance to hooking CSID
to VFE, a fact that is proven after this patch is applied allowing
RDI0,RDI1 and RDI2 to function with VC0 whereas before only RDI1 worked.
Default the VC back to zero. A follow on series will implement subdev
streams to actually enable VCs without breaking CSID source to VFE sink.
Fixes: 253314b20408 ("media: qcom: camss: Add CSID 680 support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/camss-csid-680.c | 26 +++++++++++-----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 3ad3a174bcfb8..35a6bb209f97c 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -219,9 +219,9 @@ static void __csid_configure_top(struct csid_device *csid)
CSID_TOP_IO_PATH_CFG0(csid->id));
}
-static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
{
- struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
csid->res->formats->nformats,
input_format->code);
@@ -240,21 +240,21 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
* the four least significant bits of the five bit VC
* bitfield to generate an internal CID value.
*
- * CSID_RDI_CFG0(vc)
+ * CSID_RDI_CFG0(port)
* DT_ID : 28:27
* VC : 26:22
* DT : 21:16
*
* CID : VC 3:0 << 2 | DT_ID 1:0
*/
- dt_id = vc & 0x03;
+ dt_id = port & 0x03;
/* note: for non-RDI path, this should be format->decode_format */
val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
val |= format->data_type << RDI_CFG0_DATA_TYPE;
val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
val |= dt_id << RDI_CFG0_DT_ID;
- writel(val, csid->base + CSID_RDI_CFG0(vc));
+ writel(val, csid->base + CSID_RDI_CFG0(port));
val = RDI_CFG1_TIMESTAMP_STB_FRAME;
val |= RDI_CFG1_BYTE_CNTR_EN;
@@ -265,23 +265,23 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
val |= RDI_CFG1_CROP_V_EN;
val |= RDI_CFG1_PACKING_MIPI;
- writel(val, csid->base + CSID_RDI_CFG1(vc));
+ writel(val, csid->base + CSID_RDI_CFG1(port));
val = 0;
- writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
+ writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(port));
val = 1;
- writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
+ writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(port));
val = 0;
- writel(val, csid->base + CSID_RDI_CTRL(vc));
+ writel(val, csid->base + CSID_RDI_CTRL(port));
- val = readl(csid->base + CSID_RDI_CFG0(vc));
+ val = readl(csid->base + CSID_RDI_CFG0(port));
if (enable)
val |= RDI_CFG0_ENABLE;
else
val &= ~RDI_CFG0_ENABLE;
- writel(val, csid->base + CSID_RDI_CFG0(vc));
+ writel(val, csid->base + CSID_RDI_CFG0(port));
}
static void csid_configure_stream(struct csid_device *csid, u8 enable)
@@ -293,8 +293,8 @@ 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)) {
- __csid_configure_rdi_stream(csid, enable, i);
- __csid_configure_rx(csid, &csid->phy, i);
+ __csid_configure_rdi_stream(csid, enable, i, 0);
+ __csid_configure_rx(csid, &csid->phy, 0);
__csid_ctrl_rdi(csid, enable, i);
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680
2026-04-06 21:55 ` [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680 bod
@ 2026-04-06 22:41 ` Vladimir Zapolskiy
2026-04-07 7:43 ` Bryan O'Donoghue
0 siblings, 1 reply; 14+ messages in thread
From: Vladimir Zapolskiy @ 2026-04-06 22:41 UTC (permalink / raw)
To: bod, Robert Foss, Todor Tomov, Bryan O'Donoghue,
Mauro Carvalho Chehab, Hans Verkuil, Loic Poulain, Hans Verkuil,
Gjorgji Rosikopulos, Milen Mitkov, Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, stable
On 4/7/26 00:55, bod@kernel.org wrote:
> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> Fix streaming to RDI1 and RDI2. csid->phy.en_vc contains a bitmask of
> enabled CSID ports not virtual channels.
>
> We cycle through the number of available CSID ports and test this value
> against the vc_en bitmask.
>
> We then use the passed value both as an index to the port configuration
> macros and as a virtual channel index.
>
> This is a very broken pattern. Reviewing the initial introduction of VC
> support it states that you can only map one CSID to one VFE. This is true
> however each CSID has multiple sources which can sink inside of the VFE -
> for example there is a "pixel" path for bayer stats which sources @
> CSID(x):3 and sinks on VFE(x):pix.
>
> That is CSID port # 3 should drive VFE port #3. With our current setup only
> a sensor which drives virtual channel number #3 could possibly enable that
> setup.
>
> This is deeply wrong the virtual channel has no relevance to hooking CSID
> to VFE, a fact that is proven after this patch is applied allowing
> RDI0,RDI1 and RDI2 to function with VC0 whereas before only RDI1 worked.
>
> Default the VC back to zero. A follow on series will implement subdev
> streams to actually enable VCs without breaking CSID source to VFE sink.
>
> Fixes: 253314b20408 ("media: qcom: camss: Add CSID 680 support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/media/platform/qcom/camss/camss-csid-680.c | 26 +++++++++++-----------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
> index 3ad3a174bcfb8..35a6bb209f97c 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
> @@ -219,9 +219,9 @@ static void __csid_configure_top(struct csid_device *csid)
> CSID_TOP_IO_PATH_CFG0(csid->id));
> }
>
> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
> {
> - struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
> + struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
> const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> csid->res->formats->nformats,
> input_format->code);
> @@ -240,21 +240,21 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
> * the four least significant bits of the five bit VC
> * bitfield to generate an internal CID value.
> *
> - * CSID_RDI_CFG0(vc)
> + * CSID_RDI_CFG0(port)
> * DT_ID : 28:27
> * VC : 26:22
> * DT : 21:16
> *
> * CID : VC 3:0 << 2 | DT_ID 1:0
> */
> - dt_id = vc & 0x03;
> + dt_id = port & 0x03;
>
> /* note: for non-RDI path, this should be format->decode_format */
> val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
> val |= format->data_type << RDI_CFG0_DATA_TYPE;
> val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
> val |= dt_id << RDI_CFG0_DT_ID;
> - writel(val, csid->base + CSID_RDI_CFG0(vc));
> + writel(val, csid->base + CSID_RDI_CFG0(port));
>
> val = RDI_CFG1_TIMESTAMP_STB_FRAME;
> val |= RDI_CFG1_BYTE_CNTR_EN;
> @@ -265,23 +265,23 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
> val |= RDI_CFG1_CROP_V_EN;
> val |= RDI_CFG1_PACKING_MIPI;
>
> - writel(val, csid->base + CSID_RDI_CFG1(vc));
> + writel(val, csid->base + CSID_RDI_CFG1(port));
>
> val = 0;
> - writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
> + writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(port));
>
> val = 1;
> - writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
> + writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(port));
>
> val = 0;
> - writel(val, csid->base + CSID_RDI_CTRL(vc));
> + writel(val, csid->base + CSID_RDI_CTRL(port));
>
> - val = readl(csid->base + CSID_RDI_CFG0(vc));
> + val = readl(csid->base + CSID_RDI_CFG0(port));
> if (enable)
> val |= RDI_CFG0_ENABLE;
> else
> val &= ~RDI_CFG0_ENABLE;
> - writel(val, csid->base + CSID_RDI_CFG0(vc));
> + writel(val, csid->base + CSID_RDI_CFG0(port));
> }
>
> static void csid_configure_stream(struct csid_device *csid, u8 enable)
> @@ -293,8 +293,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
> /* Loop through all enabled VCs and configure stream for each */
The comment should get an update as well, this is applicable and should be done
for the changes in camss-csid-680.c, camss-csid-gen2.c and camss-csid-gen3.c.
> for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
> if (csid->phy.en_vc & BIT(i)) {
> - __csid_configure_rdi_stream(csid, enable, i);
> - __csid_configure_rx(csid, &csid->phy, i);
> + __csid_configure_rdi_stream(csid, enable, i, 0);
> + __csid_configure_rx(csid, &csid->phy, 0);
> __csid_ctrl_rdi(csid, enable, i);
> }
> }
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680
2026-04-06 22:41 ` Vladimir Zapolskiy
@ 2026-04-07 7:43 ` Bryan O'Donoghue
0 siblings, 0 replies; 14+ messages in thread
From: Bryan O'Donoghue @ 2026-04-07 7:43 UTC (permalink / raw)
To: Vladimir Zapolskiy, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, stable
On 06/04/2026 23:41, Vladimir Zapolskiy wrote:
>> /* Loop through all enabled VCs and configure stream for each */
> The comment should get an update as well, this is applicable and should be done
> for the changes in camss-csid-680.c, camss-csid-gen2.c and camss-csid-gen3.c.
I struggled with the comment myself, it didn't feel strictly necessary
for a backport but, I take your point, when you read the code the fix
and the comment should match.
---
bod
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] media: qcom: camss: Fix RDI streaming for CSID 340
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
2026-04-06 21:55 ` [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680 bod
@ 2026-04-06 21:55 ` bod
2026-04-06 21:55 ` [PATCH 3/5] media: qcom: camss: Fix RDI streaming for CSID GEN2 bod
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: bod @ 2026-04-06 21:55 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
stable
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Fix streaming from CSIDn RDI1 and RDI2 to VFEn RDI1 and RDI2. A pattern we
have replicated throughout CAMSS where we use the VC number to populate
both the VC fields and port fields of the CSID means that in practice only
VC = 0 on CSIDn:RDI0 to VFEn:RDI0 works.
Fix that for CSID 340 by separating VC and port. Fix to VC zero as a bugfix
we will look to properly populate the VC field with follow on patches
later.
Fixes: f0fc808a466a ("media: qcom: camss: Add CSID 340 support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/camss-csid-340.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-340.c b/drivers/media/platform/qcom/camss/camss-csid-340.c
index 2b50f9b96a34e..5a7271785ec7a 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-340.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-340.c
@@ -74,9 +74,9 @@ static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
writel_relaxed(!!enable, csid->base + CSID_RDI_CTRL(rdi));
}
-static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
{
- struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
csid->res->formats->nformats,
input_format->code);
@@ -95,7 +95,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
*
* CID : VC 3:0 << 2 | DT_ID 1:0
*/
- dt_id = vc & 0x03;
+ dt_id = port & 0x03;
val = CSID_RDI_CFG0_DECODE_FORMAT_NOP; /* only for RDI path */
val |= FIELD_PREP(CSID_RDI_CFG0_DT_MASK, format->data_type);
@@ -105,10 +105,11 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
if (enable)
val |= CSID_RDI_CFG0_ENABLE;
- dev_dbg(csid->camss->dev, "CSID%u: Stream %s (dt:0x%x vc=%u)\n",
- csid->id, enable ? "enable" : "disable", format->data_type, vc);
+ dev_dbg(csid->camss->dev, "CSID%u: Stream %s (dt:0x%x port=%u vc=%u)\n",
+ csid->id, enable ? "enable" : "disable", format->data_type,
+ port, vc);
- writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
}
static void csid_configure_stream(struct csid_device *csid, u8 enable)
@@ -119,7 +120,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
if (csid->phy.en_vc & BIT(i)) {
- __csid_configure_rdi_stream(csid, enable, i);
+ __csid_configure_rdi_stream(csid, enable, i, 0);
__csid_ctrl_rdi(csid, enable, i);
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/5] media: qcom: camss: Fix RDI streaming for CSID GEN2
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
2026-04-06 21:55 ` [PATCH 1/5] media: qcom: camss: Fix RDI streaming for CSID 680 bod
2026-04-06 21:55 ` [PATCH 2/5] media: qcom: camss: Fix RDI streaming for CSID 340 bod
@ 2026-04-06 21:55 ` bod
2026-04-06 21:55 ` [PATCH 4/5] media: qcom: camss: Fix RDI streaming for CSID GEN3 bod
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: bod @ 2026-04-06 21:55 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
stable
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Fix streaming from CSIDn RDI1 and RDI2 to VFEn RDI1 and RDI2. A pattern we
have replicated throughout CAMSS where we use the VC number to populate
both the VC fields and port fields of the CSID means that in practice only
VC = 0 on CSIDn:RDI0 to VFEn:RDI0 works.
Fix that for CSID gen2 by separating VC and port. Fix to VC zero as a
bugfix we will look to properly populate the VC field with follow on
patches later.
Fixes: 729fc005c8e2 ("media: qcom: camss: Split testgen, RDI and RX for CSID 170")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../media/platform/qcom/camss/camss-csid-gen2.c | 44 +++++++++++-----------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index 2a1746dcc1c5b..331feed199094 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -203,10 +203,10 @@ static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
writel_relaxed(val, csid->base + CSID_RDI_CTRL(rdi));
}
-static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 vc)
+static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 port, u8 vc)
{
struct csid_testgen_config *tg = &csid->testgen;
- struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
csid->res->formats->nformats,
input_format->code);
@@ -253,10 +253,10 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 vc)
writel_relaxed(val, csid->base + CSID_TPG_CTRL);
}
-static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
{
/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
- struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
csid->res->formats->nformats,
input_format->code);
@@ -267,14 +267,14 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
* the four least significant bits of the five bit VC
* bitfield to generate an internal CID value.
*
- * CSID_RDI_CFG0(vc)
+ * CSID_RDI_CFG0(port)
* DT_ID : 28:27
* VC : 26:22
* DT : 21:16
*
* CID : VC 3:0 << 2 | DT_ID 1:0
*/
- u8 dt_id = vc & 0x03;
+ u8 dt_id = port & 0x03;
val = 1 << RDI_CFG0_BYTE_CNTR_EN;
val |= 1 << RDI_CFG0_FORMAT_MEASURE_EN;
@@ -284,42 +284,42 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
val |= format->data_type << RDI_CFG0_DATA_TYPE;
val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
val |= dt_id << RDI_CFG0_DT_ID;
- writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
/* CSID_TIMESTAMP_STB_POST_IRQ */
val = 2 << RDI_CFG1_TIMESTAMP_STB_SEL;
- writel_relaxed(val, csid->base + CSID_RDI_CFG1(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_CFG1(port));
val = 1;
- writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(port));
val = 0;
- writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PATTERN(port));
val = 1;
- writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(port));
val = 0;
- writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(port));
val = 1;
- writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PERIOD(port));
val = 0;
- writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_RPP_PIX_DROP_PATTERN(port));
val = 1;
- writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PERIOD(port));
val = 0;
- writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_RPP_LINE_DROP_PATTERN(port));
val = 0;
- writel_relaxed(val, csid->base + CSID_RDI_CTRL(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_CTRL(port));
- val = readl_relaxed(csid->base + CSID_RDI_CFG0(vc));
+ val = readl_relaxed(csid->base + CSID_RDI_CFG0(port));
val |= enable << RDI_CFG0_ENABLE;
- writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+ writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
}
static void csid_configure_stream(struct csid_device *csid, u8 enable)
@@ -330,10 +330,10 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
if (csid->phy.en_vc & BIT(i)) {
if (tg->enabled)
- __csid_configure_testgen(csid, enable, i);
+ __csid_configure_testgen(csid, enable, i, 0);
- __csid_configure_rdi_stream(csid, enable, i);
- __csid_configure_rx(csid, &csid->phy, i);
+ __csid_configure_rdi_stream(csid, enable, i, 0);
+ __csid_configure_rx(csid, &csid->phy, 0);
__csid_ctrl_rdi(csid, enable, i);
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/5] media: qcom: camss: Fix RDI streaming for CSID GEN3
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
` (2 preceding siblings ...)
2026-04-06 21:55 ` [PATCH 3/5] media: qcom: camss: Fix RDI streaming for CSID GEN2 bod
@ 2026-04-06 21:55 ` bod
2026-04-06 21:55 ` [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port bod
2026-04-07 8:16 ` [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs Loic Poulain
5 siblings, 0 replies; 14+ messages in thread
From: bod @ 2026-04-06 21:55 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
stable
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Fix streaming from CSIDn RDI1 and RDI2 to VFEn RDI1 and RDI2. A pattern we
have replicated throughout CAMSS where we use the VC number to populate
both the VC fields and port fields of the CSID means that in practice only
VC = 0 on CSIDn:RDI0 to VFEn:RDI0 works.
Fix that for CSID gen3 by separating VC and port. Fix to VC zero as a
bugfix we will look to properly populate the VC field with follow on
patches later.
Fixes: d96fe1808dcc ("media: qcom: camss: Add CSID 780 support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../media/platform/qcom/camss/camss-csid-gen3.c | 26 +++++++++++-----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index bd059243790ed..73504c349fd0b 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -145,12 +145,12 @@ static void __csid_configure_wrapper(struct csid_device *csid)
writel(val, csid->camss->csid_wrapper_base + CSID_IO_PATH_CFG0(csid->id));
}
-static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
{
u32 val;
u8 lane_cnt = csid->phy.lane_cnt;
/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
- struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
csid->res->formats->nformats,
input_format->code);
@@ -163,14 +163,14 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
* the four least significant bits of the five bit VC
* bitfield to generate an internal CID value.
*
- * CSID_RDI_CFG0(vc)
+ * CSID_RDI_CFG0(port)
* DT_ID : 28:27
* VC : 26:22
* DT : 21:16
*
* CID : VC 3:0 << 2 | DT_ID 1:0
*/
- u8 dt_id = vc & 0x03;
+ u8 dt_id = port & 0x03;
val = RDI_CFG0_TIMESTAMP_EN;
val |= RDI_CFG0_TIMESTAMP_STB_SEL;
@@ -180,7 +180,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
val |= format->data_type << RDI_CFG0_DT;
val |= dt_id << RDI_CFG0_DT_ID;
- writel(val, csid->base + CSID_RDI_CFG0(vc));
+ writel(val, csid->base + CSID_RDI_CFG0(port));
val = RDI_CFG1_PACKING_FORMAT_MIPI;
val |= RDI_CFG1_PIX_STORE;
@@ -189,22 +189,22 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
val |= RDI_CFG1_CROP_H_EN;
val |= RDI_CFG1_CROP_V_EN;
- writel(val, csid->base + CSID_RDI_CFG1(vc));
+ writel(val, csid->base + CSID_RDI_CFG1(port));
val = 0;
- writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
+ writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(port));
val = 1;
- writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
+ writel(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(port));
val = 0;
- writel(val, csid->base + CSID_RDI_CTRL(vc));
+ writel(val, csid->base + CSID_RDI_CTRL(port));
- val = readl(csid->base + CSID_RDI_CFG0(vc));
+ val = readl(csid->base + CSID_RDI_CFG0(port));
if (enable)
val |= RDI_CFG0_EN;
- writel(val, csid->base + CSID_RDI_CFG0(vc));
+ writel(val, csid->base + CSID_RDI_CFG0(port));
}
static void csid_configure_stream(struct csid_device *csid, u8 enable)
@@ -216,8 +216,8 @@ 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)) {
- __csid_configure_rdi_stream(csid, enable, i);
- __csid_configure_rx(csid, &csid->phy, i);
+ __csid_configure_rdi_stream(csid, enable, i, 0);
+ __csid_configure_rx(csid, &csid->phy, 0);
__csid_ctrl_rdi(csid, enable, i);
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
` (3 preceding siblings ...)
2026-04-06 21:55 ` [PATCH 4/5] media: qcom: camss: Fix RDI streaming for CSID GEN3 bod
@ 2026-04-06 21:55 ` bod
2026-04-07 2:24 ` Hangxiang Ma
2026-04-07 8:16 ` [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs Loic Poulain
5 siblings, 1 reply; 14+ messages in thread
From: bod @ 2026-04-06 21:55 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li
Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
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@linaro.org>
---
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;
};
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port
2026-04-06 21:55 ` [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port bod
@ 2026-04-07 2:24 ` Hangxiang Ma
2026-04-07 7:41 ` Bryan O'Donoghue
0 siblings, 1 reply; 14+ messages in thread
From: Hangxiang Ma @ 2026-04-07 2:24 UTC (permalink / raw)
To: bod, Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li, Vijay Kumar Tumati
Cc: linux-media, linux-arm-msm, linux-kernel
On 4/7/2026 5:55 AM, bod@kernel.org wrote:
> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> 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@linaro.org>
> ---
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port
2026-04-07 2:24 ` Hangxiang Ma
@ 2026-04-07 7:41 ` Bryan O'Donoghue
0 siblings, 0 replies; 14+ messages in thread
From: Bryan O'Donoghue @ 2026-04-07 7:41 UTC (permalink / raw)
To: Hangxiang Ma, Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Loic Poulain, Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov,
Depeng Shao, Yongsheng Li, Vijay Kumar Tumati
Cc: linux-media, linux-arm-msm, linux-kernel
On 07/04/2026 03:24, Hangxiang Ma wrote:
> On 4/7/2026 5:55 AM, bod@kernel.org wrote:
>> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>
>> 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@linaro.org>
>> ---
>> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs
2026-04-06 21:55 [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs bod
` (4 preceding siblings ...)
2026-04-06 21:55 ` [PATCH 5/5] media: qcom: camss: csid: Rename en_vc to en_port bod
@ 2026-04-07 8:16 ` Loic Poulain
2026-04-07 8:36 ` Bryan O'Donoghue
5 siblings, 1 reply; 14+ messages in thread
From: Loic Poulain @ 2026-04-07 8:16 UTC (permalink / raw)
To: bod
Cc: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Hans Verkuil,
Hans Verkuil, Gjorgji Rosikopulos, Milen Mitkov, Depeng Shao,
Yongsheng Li, linux-media, linux-arm-msm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs
2026-04-07 8:16 ` [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs Loic Poulain
@ 2026-04-07 8:36 ` Bryan O'Donoghue
2026-04-07 8:49 ` Loic Poulain
0 siblings, 1 reply; 14+ messages in thread
From: Bryan O'Donoghue @ 2026-04-07 8:36 UTC (permalink / raw)
To: Loic Poulain, bod
Cc: Robert Foss, Todor Tomov, Vladimir Zapolskiy,
Mauro Carvalho Chehab, Hans Verkuil, Hans Verkuil,
Gjorgji Rosikopulos, Milen Mitkov, Depeng Shao, Yongsheng Li,
linux-media, linux-arm-msm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs
2026-04-07 8:36 ` Bryan O'Donoghue
@ 2026-04-07 8:49 ` Loic Poulain
2026-04-07 9:34 ` Bryan O'Donoghue
0 siblings, 1 reply; 14+ messages in thread
From: Loic Poulain @ 2026-04-07 8:49 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: bod, Robert Foss, Todor Tomov, Vladimir Zapolskiy,
Mauro Carvalho Chehab, Hans Verkuil, Hans Verkuil,
Gjorgji Rosikopulos, Milen Mitkov, Depeng Shao, Yongsheng Li,
linux-media, linux-arm-msm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] media: qcom: camss: Fix RDI streaming for various CSIDs
2026-04-07 8:49 ` Loic Poulain
@ 2026-04-07 9:34 ` Bryan O'Donoghue
0 siblings, 0 replies; 14+ messages in thread
From: Bryan O'Donoghue @ 2026-04-07 9:34 UTC (permalink / raw)
To: Loic Poulain, Bryan O'Donoghue
Cc: Robert Foss, Todor Tomov, Vladimir Zapolskiy,
Mauro Carvalho Chehab, Hans Verkuil, Hans Verkuil,
Gjorgji Rosikopulos, Milen Mitkov, Depeng Shao, Yongsheng Li,
linux-media, linux-arm-msm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread