[PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init

David Heidelberg via B4 Relay posted 8 patches 2 months ago
There is a newer version of this series
[PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
Posted by David Heidelberg via B4 Relay 2 months ago
From: Casey Connolly <casey.connolly@linaro.org>

Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
Gen 2 version 1.1 CSI-2 PHY.

The PHY can be configured as two phase or three phase in C-PHY or D-PHY
mode. This configuration supports three-phase C-PHY mode.

Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 74 +++++++++++++++++++++-
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index 3d30cdce33f96..7121aa97a19c4 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -145,6 +145,7 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
 };
 
 /* GEN2 1.0 2PH */
+/* 5 entries: clock + 4 lanes */
 static const struct
 csiphy_lane_regs lane_regs_sdm845[] = {
 	{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
@@ -219,6 +220,69 @@ csiphy_lane_regs lane_regs_sdm845[] = {
 	{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
 };
 
+/* GEN2 1.0 3PH */
+/* 3 entries: 3 lanes (C-PHY) */
+static const struct
+csiphy_lane_regs lane_regs_sdm845_3ph[] = {
+	{0x015C, 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0168, 0xA0, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x016C, 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0104, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x010C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x0108, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+	{0x0114, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0150, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0118, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x011C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0120, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0124, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0128, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x012C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0144, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0160, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x01CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0164, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x01DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x035C, 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0368, 0xA0, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x036C, 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0304, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x030C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x0308, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+	{0x0314, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0350, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0318, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x031C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0320, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0324, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0328, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x032C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0344, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0360, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x03CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0364, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x03DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x055C, 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0568, 0xA0, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x056C, 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0504, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x050C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{0x0508, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+	{0x0514, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0550, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0518, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x051C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0520, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0524, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0528, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x052C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0544, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0560, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x05CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x0564, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+	{0x05DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+};
+
 /* GEN2 1.1 2PH */
 static const struct
 csiphy_lane_regs lane_regs_sc8280xp[] = {
@@ -1043,8 +1107,14 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 
 	switch (csiphy->camss->res->version) {
 	case CAMSS_845:
-		regs->lane_regs = &lane_regs_sdm845[0];
-		regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
+		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+			regs->lane_regs = &lane_regs_sdm845_3ph[0];
+			regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845_3ph);
+
+		} else { /* V4L2_MBUS_CSI2_DPHY */
+			regs->lane_regs = &lane_regs_sdm845[0];
+			regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
+		}
 		break;
 	case CAMSS_2290:
 		regs->lane_regs = &lane_regs_qcm2290[0];

-- 
2.51.0
Re: [PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
Posted by Konrad Dybcio 2 months ago
On 12/4/25 5:32 PM, David Heidelberg via B4 Relay wrote:
> From: Casey Connolly <casey.connolly@linaro.org>
> 
> Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
> Gen 2 version 1.1 CSI-2 PHY.
> 
> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
> mode. This configuration supports three-phase C-PHY mode.
> 
> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Co-developed-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 74 +++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> index 3d30cdce33f96..7121aa97a19c4 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> @@ -145,6 +145,7 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
>  };
>  
>  /* GEN2 1.0 2PH */
> +/* 5 entries: clock + 4 lanes */
>  static const struct
>  csiphy_lane_regs lane_regs_sdm845[] = {
>  	{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
> @@ -219,6 +220,69 @@ csiphy_lane_regs lane_regs_sdm845[] = {
>  	{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>  };
>  
> +/* GEN2 1.0 3PH */
> +/* 3 entries: 3 lanes (C-PHY) */
> +static const struct
> +csiphy_lane_regs lane_regs_sdm845_3ph[] = {

Here's a downstream snippet which seems to have more up-to-date settings
(checked against a doc and it seems to have changes made at the time of
the last edit of the doc)

You'll notice it's split into 3 arrays of register sets - that's because
it applies setting for each lane.. something to keep in mind we could
optimize upstream data storage for (they are identical per lane in this
instance) one day

struct csiphy_reg_t csiphy_3ph_v1_0_reg[MAX_LANES][MAX_SETTINGS_PER_LANE] = {
	{
		{0x015C, 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0168, 0xAC, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x016C, 0xA5, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0104, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x010C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
		{0x0108, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
		{0x0114, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0150, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0118, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x011C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0120, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0124, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0128, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x012C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0144, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0160, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x01CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0164, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x01DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
	},
	{
		{0x035C, 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0368, 0xAC, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x036C, 0xA5, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0304, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x030C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
		{0x0308, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
		{0x0314, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0350, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0318, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x031C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0320, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0324, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0328, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x032C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0344, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0360, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x03CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0364, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x03DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
	},
	{
		{0x055C, 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0568, 0xAC, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x056C, 0xA5, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0504, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x050C, 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
		{0x0508, 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
		{0x0514, 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0550, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0518, 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x051C, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0520, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0524, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0528, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x052C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0544, 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0560, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x05CC, 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x0564, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
		{0x05DC, 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
	},
};

Konrad
Re: [PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
Posted by David Heidelberg 2 months ago
On 05/12/2025 10:54, Konrad Dybcio wrote:
> On 12/4/25 5:32 PM, David Heidelberg via B4 Relay wrote:
>> From: Casey Connolly <casey.connolly@linaro.org>
>>
>> Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
>> Gen 2 version 1.1 CSI-2 PHY.
>>
>> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
>> mode. This configuration supports three-phase C-PHY mode.
>>
>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Co-developed-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 74 +++++++++++++++++++++-
>>   1 file changed, 72 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> index 3d30cdce33f96..7121aa97a19c4 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> @@ -145,6 +145,7 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
>>   };
>>   
>>   /* GEN2 1.0 2PH */
>> +/* 5 entries: clock + 4 lanes */
>>   static const struct
>>   csiphy_lane_regs lane_regs_sdm845[] = {
>>   	{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> @@ -219,6 +220,69 @@ csiphy_lane_regs lane_regs_sdm845[] = {
>>   	{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>>   };
>>   
>> +/* GEN2 1.0 3PH */
>> +/* 3 entries: 3 lanes (C-PHY) */
>> +static const struct
>> +csiphy_lane_regs lane_regs_sdm845_3ph[] = {
> 
> Here's a downstream snippet which seems to have more up-to-date settings
> (checked against a doc and it seems to have changes made at the time of
> the last edit of the doc)
> 
> You'll notice it's split into 3 arrays of register sets - that's because
> it applies setting for each lane.. something to keep in mind we could
> optimize upstream data storage for (they are identical per lane in this
> instance) one day

see 87c2c2716523 ("media: qcom: camss: csiphy-3ph: Remove redundant PHY 
init sequence control loop")

for the reason to flatten the regs array (thou outside the scope of this 
patchset).

Regarding to the different value, I can test them, can you point to docs 
regarding why these regs has been changed and what the values means?

I thought it's some default seq, but as you show there is multiple 
versions, it make sense to properly document what these regs do.

David
[...]
Re: [PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
Posted by Konrad Dybcio 2 months ago
On 12/5/25 12:56 PM, David Heidelberg wrote:
> On 05/12/2025 10:54, Konrad Dybcio wrote:
>> On 12/4/25 5:32 PM, David Heidelberg via B4 Relay wrote:
>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>
>>> Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
>>> Gen 2 version 1.1 CSI-2 PHY.
>>>
>>> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
>>> mode. This configuration supports three-phase C-PHY mode.
>>>
>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> Co-developed-by: David Heidelberg <david@ixit.cz>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 74 +++++++++++++++++++++-
>>>   1 file changed, 72 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>> index 3d30cdce33f96..7121aa97a19c4 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>> @@ -145,6 +145,7 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
>>>   };
>>>     /* GEN2 1.0 2PH */
>>> +/* 5 entries: clock + 4 lanes */
>>>   static const struct
>>>   csiphy_lane_regs lane_regs_sdm845[] = {
>>>       {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>>> @@ -219,6 +220,69 @@ csiphy_lane_regs lane_regs_sdm845[] = {
>>>       {0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>>>   };
>>>   +/* GEN2 1.0 3PH */
>>> +/* 3 entries: 3 lanes (C-PHY) */
>>> +static const struct
>>> +csiphy_lane_regs lane_regs_sdm845_3ph[] = {
>>
>> Here's a downstream snippet which seems to have more up-to-date settings
>> (checked against a doc and it seems to have changes made at the time of
>> the last edit of the doc)
>>
>> You'll notice it's split into 3 arrays of register sets - that's because
>> it applies setting for each lane.. something to keep in mind we could
>> optimize upstream data storage for (they are identical per lane in this
>> instance) one day
> 
> see 87c2c2716523 ("media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop")
> 
> for the reason to flatten the regs array (thou outside the scope of this patchset).
> 
> Regarding to the different value, I can test them, can you point to docs regarding why these regs has been changed and what the values means?
> 
> I thought it's some default seq, but as you show there is multiple versions, it make sense to properly document what these regs do.

I'll make that point to the relevant folks when they get around to refreshing
this driver, I'm not sure I can just tell you what all the magic sequences do..

The high-level description for all post-release PHY sequence updates is pretty
much always improves robustness as a result of "more better" electrical tuning.
It's also the case this time around.

Konrad
Re: [PATCH WIP v2 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
Posted by David Heidelberg 2 months ago
On 05/12/2025 13:00, Konrad Dybcio wrote:
> On 12/5/25 12:56 PM, David Heidelberg wrote:
>> On 05/12/2025 10:54, Konrad Dybcio wrote:
>>> On 12/4/25 5:32 PM, David Heidelberg via B4 Relay wrote:
>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>
>>>> Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
>>>> Gen 2 version 1.1 CSI-2 PHY.
>>>>
>>>> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
>>>> mode. This configuration supports three-phase C-PHY mode.
>>>>
>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> Co-developed-by: David Heidelberg <david@ixit.cz>
>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>>> ---
>>>>    .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 74 +++++++++++++++++++++-
>>>>    1 file changed, 72 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>>> index 3d30cdce33f96..7121aa97a19c4 100644
>>>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>>>> @@ -145,6 +145,7 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
>>>>    };
>>>>      /* GEN2 1.0 2PH */
>>>> +/* 5 entries: clock + 4 lanes */
>>>>    static const struct
>>>>    csiphy_lane_regs lane_regs_sdm845[] = {
>>>>        {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>>>> @@ -219,6 +220,69 @@ csiphy_lane_regs lane_regs_sdm845[] = {
>>>>        {0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>>>>    };
>>>>    +/* GEN2 1.0 3PH */
>>>> +/* 3 entries: 3 lanes (C-PHY) */
>>>> +static const struct
>>>> +csiphy_lane_regs lane_regs_sdm845_3ph[] = {
>>>
>>> Here's a downstream snippet which seems to have more up-to-date settings
>>> (checked against a doc and it seems to have changes made at the time of
>>> the last edit of the doc)
>>>
>>> You'll notice it's split into 3 arrays of register sets - that's because
>>> it applies setting for each lane.. something to keep in mind we could
>>> optimize upstream data storage for (they are identical per lane in this
>>> instance) one day
>>
>> see 87c2c2716523 ("media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop")
>>
>> for the reason to flatten the regs array (thou outside the scope of this patchset).
>>
>> Regarding to the different value, I can test them, can you point to docs regarding why these regs has been changed and what the values means?
>>
>> I thought it's some default seq, but as you show there is multiple versions, it make sense to properly document what these regs do.
> 
> I'll make that point to the relevant folks when they get around to refreshing
> this driver, I'm not sure I can just tell you what all the magic sequences do..
> 
> The high-level description for all post-release PHY sequence updates is pretty
> much always improves robustness as a result of "more better" electrical tuning.
> It's also the case this time around.

Since the original version works, but maybe it could work better (I'll 
do testing), I would do "upgrade" commit, so at least we know tho regs 
setups which works and someone can try deduct or document it in the future.

If you try to poke the right people, that would be awesome.

Thank you for noticing the new regs! :)

David

> 
> Konrad

-- 
David Heidelberg