[3/6] media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init

From: George Chan <gchan9527@gmail.com>

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

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

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
 1 file changed, 120 insertions(+)

On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
> Gen 2 version 1.2.2 CSI-2 PHY.
> 
> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
> mode. This configuration supports two-phase D-PHY mode.
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
>   1 file changed, 120 insertions(+)
> 
> 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 df7e93a5a4f6..181bb7f7c300 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
> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
>   	},
>   };
>   
> +/* GEN2 1.2.2 2PH */

This is the init sequence for 1_2_1 not 1_2_2

https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h

https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

Please fix.

---
bod

On 21.06.2024 1:25 PM, Bryan O'Donoghue wrote:
> On 21/06/2024 10:40, George Chan via B4 Relay wrote:
>> From: George Chan <gchan9527@gmail.com>
>>
>> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
>> Gen 2 version 1.2.2 CSI-2 PHY.
>>
>> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
>> mode. This configuration supports two-phase D-PHY mode.
>>
>> Signed-off-by: George Chan <gchan9527@gmail.com>
>> ---
>>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
>>   1 file changed, 120 insertions(+)
>>
>> 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 df7e93a5a4f6..181bb7f7c300 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
>> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
>>       },
>>   };
>>   +/* GEN2 1.2.2 2PH */
> 
> This is the init sequence for 1_2_1 not 1_2_2
> 
> https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h
> 
> https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

FWIW 1.2.2 seems to be the desired one: [1]

Konrad

[1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22

resend with plain text

On Sat, Jun 22, 2024 at 7:20 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 21.06.2024 1:25 PM, Bryan O'Donoghue wrote:
> > On 21/06/2024 10:40, George Chan via B4 Relay wrote:
> >> From: George Chan <gchan9527@gmail.com>
> >>
> >> Add a PHY configuration sequence for the sc7180 which uses a Qualcomm
> >> Gen 2 version 1.2.2 CSI-2 PHY.
> >>
> >> The PHY can be configured as two phase or three phase in C-PHY or D-PHY
> >> mode. This configuration supports two-phase D-PHY mode.
> >>
> >> Signed-off-by: George Chan <gchan9527@gmail.com>
> >> ---
> >>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 +++++++++++++++++++++
> >>   1 file changed, 120 insertions(+)
> >>
> >> 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 df7e93a5a4f6..181bb7f7c300 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
> >> @@ -348,6 +348,121 @@ csiphy_reg_t lane_regs_sm8250[5][20] = {
> >>       },
> >>   };
> >>   +/* GEN2 1.2.2 2PH */
> >
> > This is the init sequence for 1_2_1 not 1_2_2

Yes, undesirable copy-n-paste result.

> >
> > https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_1_hwreg.h
> >
> > https://review.lineageos.org/c/LineageOS/android_kernel_xiaomi_sm8250/+/311931/10/techpack/camera/drivers/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
>
> FWIW 1.2.2 seems to be the desired one: [1]
>
> Konrad
>
> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22

Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
[  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000

I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees,
and concluded below version:
(1)atoll, sdm845[1]
(2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]

I was tempted to use (1)atoll one but it looked like (2) is newer. Is
it worthy to create CAMSS_7125 specially for SM7125. Please give me
some advice about it.

Regards,
George

[1] https://github.com/LineageOS/android_kernel_xiaomi_sm6250/blob/lineage-21/drivers/media/platform/msm/camera/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
[2] https://github.com/LineageOS/android_kernel_xiaomi_surya/blob/lineage-21/drivers/media/platform/msm/camera/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h
[3] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-trogdor-13443.B-chromeos-5.4/drivers/media/platform/camx/cam_sensor_module/cam_csiphy/include/cam_csiphy_1_2_2_hwreg.h

On 22/06/2024 14:43, george chan wrote:
>     FWIW 1.2.2 seems to be the desired one: [1]
> 
>     Konrad
> 
>     [1]
>     https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22 <https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22>
> 
> 
> Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
> [  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000
> 
> I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees, and 
> concluded below version:
> (1)atoll, sdm845[1]
> (2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]
> 
> I was tempted to use (1)atoll one but it looked like (2) is newer. Is it 
> worthy to create CAMSS_7125 specially for SM7125. Please give me some 
> advice about it.

So, which have you tested with as verified and working ?

My assumption here is that this series has been tested and is proven to 
work.

Version 1.2.1 and version 1.2.2 don't indicate different versions of the 
init sequence but different versions of the PHY.

For example - the CSI decoder is "just" digital logic, the "source code" 
for the at logic can be "recompiled" for a different process node.

But the PHYs translate analogue signals into the digital domain and 
therefore will vary with different process nodes - 3nm v 4nm v 28nm.

So it is virtually impossible - or highly improbable that init sequence 
1.2.1 and init sequence 1.2.2 will work on the same piece of hardware.

So its not a question of choosing the newer version - only one version 
will work - the version that is specifically tuned to the PHY for the 
given process node and RTL version.

Err, so TL;DR you _have_ tested this and gotten data delivered to you in 
user-space - right ?

---
bod

On Sun, Jun 23, 2024 at 7:17 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 22/06/2024 14:43, george chan wrote:
> >     FWIW 1.2.2 seems to be the desired one: [1]
> >
> >     Konrad
> >
> >     [1]
> >     https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22 <https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/UC.UM.1.0.r1-02500-sa8155.0/arch/arm64/boot/dts/qcom/atoll-camera.dtsi#L22>
> >
> >
> > Here is the log from sm7125 joyeuse phone, not sure if it helps or not.
> > [  204.034767] qcom-camss acb3000.camss: CSIPHY 3PH HW Version = 0x01000000
> >
> > I carefully looked into this csiphy_2ph_v1_2_2_reg of various trees, and
> > concluded below version:
> > (1)atoll, sdm845[1]
> > (2)surya[2], sa8155, factory-trogdor-13443.B-chromeos-5.4[3]
> >
> > I was tempted to use (1)atoll one but it looked like (2) is newer. Is it
> > worthy to create CAMSS_7125 specially for SM7125. Please give me some
> > advice about it.
>
> So, which have you tested with as verified and working ?
>

Tests show me, under my sm7125 test phone test case, no matter v1.2.1,
or atoll's v1.2.2 even surya and trogdor tree v1.2.2 are all
surprisingly works. Thanks for telling me or I won't be able to spot
this out. These results are quite funny :-)

> My assumption here is that this series has been tested and is proven to
> work.
>
> Version 1.2.1 and version 1.2.2 don't indicate different versions of the
> init sequence but different versions of the PHY.
>
> For example - the CSI decoder is "just" digital logic, the "source code"
> for the at logic can be "recompiled" for a different process node.
>
> But the PHYs translate analogue signals into the digital domain and
> therefore will vary with different process nodes - 3nm v 4nm v 28nm.
>
> So it is virtually impossible - or highly improbable that init sequence
> 1.2.1 and init sequence 1.2.2 will work on the same piece of hardware.
>

Yes, agreed. I also have the feeling of sc7180(10nm) vs sm7125(8nm) fab.

> So its not a question of choosing the newer version - only one version
> will work - the version that is specifically tuned to the PHY for the
> given process node and RTL version.
>
> Err, so TL;DR you _have_ tested this and gotten data delivered to you in
> user-space - right ?

User-space tool can't tell so I made some guesses.

A side note, atoll's reg sequence is a bit shorter and, unlike
trogdor, it does not write upto 0x9xx reg. That seemed to me, using
atoll's init sequence for sc7180 is nothing wrong initially. Later on
today, I am wondering if writing those extra regs(> 0x9xx) is to
stabilize the phy, so applying trogdor init sequence to atoll might be
more desirable.

As the trogdor tree with kernel 5.4 so this is a side proof that
trogdor init sequence is newer than atoll's. So I will use the
trodgor's init sequence for CAMSS_7180.

So here is the plan. Let's treat CAMSS_7180 to both sm7125 and sc7180
SOCs, and apply the idea to all others sharing v1.2.2 phy atm.

If somebody knowledgeable could confirm some real difference, then I
can prepare another CAMSS_7125 patch-set afterward.

>
> ---
> bod

On 23/06/2024 22:37, george chan wrote:
> User-space tool can't tell so I made some guesses.

So how are you testing ?

Libcamera on your target rootfs ?

# example 1
cam -c 1 --capture=10 --file

Should deliver up ten frames to userpsace.

For me working means either

1. Sensor data delivered to user-space or
2. Minimum test pattern generator (TPG) data delivered to userspace

Here's an example of the TPG on the rb3/sdm845

# example 2
media-ctl --reset
yavta --no-query -w '0x009f0903 9' /dev/v4l-subdev4
yavta --list /dev/v4l-subdev4
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f SGRBG10P -s 3280x2464 
--file=TPG-SGRBG10-3280x2464-000-#.bin /dev/video2

If you can't use libcamera to do the v4l pipeline setup you can do so 
yourself manually again here's rb3 setting up the pipeline and reading 
from ov8856.

# example 3
media-ctl --reset
media-ctl -d /dev/media0 -V '"ov8856 
'16-0010'":0[fmt:SGRBG10_1X10/3280x2464 field:none]'
media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -l 
'"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f SGRBG10P -s 3280x2464 
--file=ov8856-SGRBG10-3280x2464-000-#.bin /dev/video0

Maybe its an obvious question but, are you currently able to read from 
either

1. The sensor - thus proving the PHY init sequence you have or
2. The TPG ?

as illustrated with one of the examples [1, 2, 3] above ?

---
bod

On Mon, Jun 24, 2024 at 6:13 AM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 23/06/2024 22:37, george chan wrote:
> > User-space tool can't tell so I made some guesses.

Sorry for misleading, actually i mean user-space too can't tell the
difference. As all 3 kinds of init sequences are working, I can't get
a strong conclusion of "correct" init sequence between atoll's and
trodger's.

> So how are you testing ?
>
> Libcamera on your target rootfs ?

Yes, a similar test was carried out early days with the "wrong" v1.2.1
init sequence, on pmOS qcam installed into xiaomi redmi note 9 pro
(sm7125). It showed nice output. And I was excited so I took a video
recording too:
https://www.youtube.com/watch?v=U_do11pSf1s

After your indication, I replaced the v1.2.1 init sequence with
atoll's and trodger's and carried some simple test with below cmd and
both are outputting files.

media-ctl --reset
media-ctl -V '"msm_csid0":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
v4l2-ctl -d /dev/v4l-subdev4 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev6 -c test_pattern=0
v4l2-ctl -d /dev/v4l-subdev19 -c test_pattern=$1

media-ctl -V '"s5k5e9 13-002d":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/2592x1944 field:none]'
media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
yavta -B capture-mplane --capture=3 -n 3 -f SRGGB10P -s 2592x1944 /dev/video0 -F

As you can see the cmos named s5k5e9. and this time simply do yavta
dump, no pmOS qcam test.

Since this test is carried out in sm7125 SOC, in theory, it is better
to test with sc7180 (less likely form-factor available in the market)
so I will send out v2 with trogdor init sequence for other dev have
sc7180 board to have a test.

Stay tuned.

On 24/06/2024 00:16, george chan wrote:
>> So how are you testing ?
>>
>> Libcamera on your target rootfs ?
> Yes, a similar test was carried out early days with the "wrong" v1.2.1
> init sequence, on pmOS qcam installed into xiaomi redmi note 9 pro
> (sm7125). It showed nice output. And I was excited so I took a video
> recording too:
> https://www.youtube.com/watch?v=U_do11pSf1s

Grand so.

---
bod