[PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[Anusha Arun Nandi]

The new (non-legacy) csiphy_stream_on() introduced by the PHY API
devices support only built a MIPI D-PHY configuration and
unconditionally called phy_set_mode(PHY_MODE_MIPI_DPHY). For a C-PHY
sensor this makes phy_configure() dispatch to the C-PHY handler with a
D-PHY opts union, which fails validation.

Branch on the recorded PHY type: for C-PHY build the opts via
phy_mipi_cphy_get_default_config_for_hsclk(), populate the per-trio
lane positions and polarities and select PHY_MODE_MIPI_CPHY; 
keep the existing D-PHY path otherwise. The shared
phy_configure()/phy_power_on() tail handles both.

Depends-on: https://lore.kernel.org/all/20261017035221.3816464-1-anusha.nandi@oss.qualcomm.com/

Signed-off-by: Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-csiphy.c  | 61 +++++++++++++------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 60d58becc7c5..5ee07b969005 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -329,16 +329,15 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
 	u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
 				csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
 	struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
-	struct phy_configure_opts_mipi_dphy *dphy_cfg;
-	union phy_configure_opts dphy_opts = { 0 };
+	union phy_configure_opts phy_opts = { 0 };
 	struct device *dev = csiphy->camss->dev;
 	u8 num_lanes = lncfg->num_data;
+	const bool cphy = (lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY);
 	s64 link_freq;
+	int i;
 	int ret;
 
-	dphy_cfg = &dphy_opts.mipi_dphy;
-
-	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, false);
+	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, cphy);
 
 	if (link_freq <= 0) {
 		dev_err(dev,
@@ -346,23 +345,49 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
 		return -EINVAL;
 	}
 
-	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
-							 dphy_cfg);
-	if (ret < 0) {
-		dev_err(dev, "Invalid config for link frequency %lld\n",
-			link_freq);
-		return ret;
-	}
+	if (cphy) {
+		struct phy_configure_opts_mipi_cphy *cphy_cfg = &phy_opts.mipi_cphy;
 
-	ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
-	if (ret) {
-		dev_err(dev, "Failed to set phy mode\n");
-		return ret;
+		ret = phy_mipi_cphy_get_default_config_for_hsclk(link_freq, num_lanes,
+								 cphy_cfg);
+		if (ret < 0) {
+			dev_err(dev, "Invalid config for link frequency %lld\n",
+				link_freq);
+			return ret;
+		}
+
+		/* Set data trio lane positions and polarities (no clock lane) */
+		for (i = 0; i < num_lanes; i++) {
+			cphy_cfg->lane_positions[i] = lncfg->data[i].pos;
+			cphy_cfg->lane_polarities[i] = lncfg->data[i].pol;
+		}
+
+		ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_CPHY);
+		if (ret) {
+			dev_err(dev, "Failed to set phy mode\n");
+			return ret;
+		}
+	} else {
+		struct phy_configure_opts_mipi_dphy *dphy_cfg = &phy_opts.mipi_dphy;
+
+		ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
+								 dphy_cfg);
+		if (ret < 0) {
+			dev_err(dev, "Invalid config for link frequency %lld\n",
+				link_freq);
+			return ret;
+		}
+
+		ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
+		if (ret) {
+			dev_err(dev, "Failed to set phy mode\n");
+			return ret;
+		}
 	}
 
-	ret = phy_configure(csiphy->phy, &dphy_opts);
+	ret = phy_configure(csiphy->phy, &phy_opts);
 	if (ret) {
-		dev_err(dev, "failed to configure MIPI D-PHY\n");
+		dev_err(dev, "failed to configure MIPI %s\n", cphy ? "C-PHY" : "D-PHY");
 		goto error;
 	}

--  
Best regards,
-- 
Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>
-- 
2.34.1

Re: [PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[Krzysztof Kozlowski]

On 17/10/2026 06:05, Anusha Arun Nandi wrote:
> The new (non-legacy) csiphy_stream_on() introduced by the PHY API
> devices support only built a MIPI D-PHY configuration and
> unconditionally called phy_set_mode(PHY_MODE_MIPI_DPHY). For a C-PHY
> sensor this makes phy_configure() dispatch to the C-PHY handler with a
> D-PHY opts union, which fails validation.
> 
> Branch on the recorded PHY type: for C-PHY build the opts via
> phy_mipi_cphy_get_default_config_for_hsclk(), populate the per-trio
> lane positions and polarities and select PHY_MODE_MIPI_CPHY; 
> keep the existing D-PHY path otherwise. The shared
> phy_configure()/phy_power_on() tail handles both.

I received multiple patchsets from you. CC list looks really random,
half of the addresses are some internal ones?

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

Best regards,
Krzysztof

Re: [PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[George Neacsu]

Hi Anusha,

One units issue showed up while testing this on X1E80100.

camss_get_link_freq(..., cphy=true) returns the V4L2 C-PHY link
frequency. The media documentation defines link frequency as the bus
frequency, explicitly not the symbol rate; its C-PHY pixel-rate formula
also has the factor of two that makes the C-PHY symbol rate twice the
V4L2 link frequency.

For the SP11 IMX681, the exact Windows/QTI sensor mode programs PLL2
for a 2.400 Gsym/s C-PHY rate (19.2 MHz * 375 / 3). The same 68-write
mode programming was also observed live under KD on this machine.
camss_get_link_freq() returns the corresponding 1.200 GHz V4L2 link
frequency.

phy_mipi_cphy_get_default_config_for_hsclk() takes the high-speed symbol
rate, so passing link_freq directly here fills hs_clk_rate with
1.2 Gsym/s rather than 2.4 Gsym/s. I think this wants:

    phy_mipi_cphy_get_default_config_for_hsclk((u64)link_freq * 2,
                                                num_lanes, cphy_cfg);

I instrumented that path on X1E and get link=1200000000 Hz and
symbol=2400000000 sym/s. Capture succeeds with those values.

The qcom provider then needs to keep those units distinct too; I have a
separate comment on that patch.

Thanks,
George

Re: [PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[Bryan O'Donoghue]

On 17/10/2026 05:05, Anusha Arun Nandi wrote:
> +	const bool cphy = (lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY);

suggest is_cphy

>   	s64 link_freq;
> +	int i;
>   	int ret;
>   
> -	dphy_cfg = &dphy_opts.mipi_dphy;
> -
> -	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, false);
> +	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, cphy);
>   
>   	if (link_freq <= 0) {
>   		dev_err(dev,
> @@ -346,23 +345,49 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   		return -EINVAL;
>   	}
>   
> -	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
> -							 dphy_cfg);
> -	if (ret < 0) {
> -		dev_err(dev, "Invalid config for link frequency %lld\n",
> -			link_freq);
> -		return ret;
> -	}
> +	if (cphy) {
> +		struct phy_configure_opts_mipi_cphy *cphy_cfg = &phy_opts.mipi_cphy;
>   
> -	ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
> -	if (ret) {
> -		dev_err(dev, "Failed to set phy mode\n");
> -		return ret;
> +		ret = phy_mipi_cphy_get_default_config_for_hsclk(link_freq, num_lanes,
> +								 cphy_cfg);
> +		if (ret < 0) {
> +			dev_err(dev, "Invalid config for link frequency %lld\n",
> +				link_freq);
> +			return ret;
> +		}
> +
> +		/* Set data trio lane positions and polarities (no clock lane) */
> +		for (i = 0; i < num_lanes; i++) {
> +			cphy_cfg->lane_positions[i] = lncfg->data[i].pos;
> +			cphy_cfg->lane_polarities[i] = lncfg->data[i].pol;
> +		}
> +
> +		ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_CPHY);
> +		if (ret) {
> +			dev_err(dev, "Failed to set phy mode\n");
> +			return ret;

We already derive the lane positions and polarities in the PHY driver. 
There's no reason for the CSID block to know or care what the polarities 
or positions are.

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c::phy_qcom_mipi_csi2_parse_routing()

I guess the PHY already knows the polarities and positions - or at least 
knows them for the DPHY case.

         /* lane-polarities: optional, up to num_data_lanes + 1 entries */
         memset(lane_polarities, 0x00, sizeof(lane_polarities));
         num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
         if (num_polarities > 0) {
                 if (num_polarities != stream_cfg->num_data_lanes + 1) {
                         ret = -EINVAL;
                         dev_err(dev, "clock+data-lane %d/polarities %d 
mismatch\n",
                                 stream_cfg->num_data_lanes + 1, 
num_polarities);
                         goto out_put;
                 }

                 ret = fwnode_property_read_u32_array(ep, 
"lane-polarities", lane_polarities,
                                                      num_polarities);
                 if (ret) {
                         dev_err(dev, "Failed to read lane-polarities: 
%d\n", ret);
                         goto out_put;
                 }
         }

         csi2phy->stream_cfg.lane_cfg.clk.pos = CSI2_DEFAULT_CLK_LANE;
         csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];

         for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
                 if (data_lanes[i] < 1 || data_lanes[i] > 
CSI2_MAX_DATA_LANES) {
                         dev_err(dev, "Invalid lane %d\n", data_lanes[i]);
                         ret = -EINVAL;
                         goto out_put;
                 }

                 /* Convert data-lanes = <1 2 3 4> to bit positions */
                 csi2phy->stream_cfg.lane_cfg.data[i].pos = 
data_lanes[i] - 1;
                 csi2phy->stream_cfg.lane_cfg.data[i].pol = 
lane_polarities[i + 1];
         }

---
bod