[0/6] media: qcom: camss: Add sc7180 support

SM7125 is the SoC found in the Xiaomi Redmi Note 9 Pro(joyeuse) cellphone.
This series adds support to bring up the CSIPHY, CSID, VFE/RDI interfaces.

Since SM7125 is a low-speed variant of SC7180, SC7180 testers please
take a look and have a test as well.

sc7180 provides

- 2 x VFE
- 1 x VFE Lite
- 2 x CSID
- 1 x CSID Lite
- 4 x CSI PHY

The sc7180-camss binding should be comaptible with sdm845 yaml.
I've copied a new yaml from sdm845-camss.yaml, strip all _src clk and leave
original maintainer. If this is not desirable then i can add binding to
existing sdm845 yaml instead.

In addition, a bootable tree of sm7125/joyeuse is availble at:
https://github.com/99degree/linux/tree/camss
  

Signed-off-by: George Chan <gchan9527@gmail.com>
---
George Chan (6):
      media: dt-bindings: media: camss: Add qcom,sc7180-camss binding
      media: qcom: camss: Add CAMSS_SC7180 enum
      media: qcom: camss: csiphy-3ph: Add Gen2 v1.2.2 two-phase MIPI CSI-2 DPHY init
      media: qcom: camss: Add sc7180 support
      media: qcom: camss: Add sc7180 resources
      [RFT]arm64: dts: qcom: sc7180: Add support for camss subsys

 .../bindings/media/qcom,sc7180-camss.yaml          | 324 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7180.dtsi               | 134 +++++++++
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 120 ++++++++
 drivers/media/platform/qcom/camss/camss-csiphy.c   |   1 +
 drivers/media/platform/qcom/camss/camss-vfe.c      |   3 +
 drivers/media/platform/qcom/camss/camss-video.c    |   1 +
 drivers/media/platform/qcom/camss/camss.c          | 218 +++++++++++++-
 drivers/media/platform/qcom/camss/camss.h          |   1 +
 8 files changed, 801 insertions(+), 1 deletion(-)
---
base-commit: 2102cb0d050d34d50b9642a3a50861787527e922
change-id: 20240621-b4-sc7180-camss-cddc6b60a9b4

Best regards,

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 Fri, Jun 21, 2024 at 6:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> > From: George Chan <gchan9527@gmail.com>
> >
> > Add bindings for qcom,sc7180-camss in order to support the camera
> > subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.
> >
> > Signed-off-by: George Chan <gchan9527@gmail.com>
>
> Subject: just one media (first). No need to write media: media: ...
>
>
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

I found the cause of this, see if I can fix it next round.

> > +title: Qualcomm CAMSS ISP
>
> What is CAMSS?
>

No idea from an outsider, i can suggest one like "title: Qualcomm
Camera SubSystem"

> > +
> > +maintainers:
> > +  - Robert Foss <robert.foss@linaro.org>
>
> For sure this is not true. Robert does not work in Linaro and I doubt he
> cares that much about camss.
>
Well, i might suggest to be like below if no objection

 maintainers:
-  - Robert Foss <robert.foss@linaro.org>
+  - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
...
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.

How about this? I have no idea what this is, just modify it blindly below.
-description: |
+description:

Then drop all "minItems"
...
> > +required:
> > +  - clock-names
> > +  - clocks
> > +  - compatible
>
> Keep the list ordered, the same as list properties.

Sure for this "required" list
...
>
> Missed alignment with previous <.
>
Sure
...
> > +        reg = <0 0xacb3000 0 0x1000>,
>
> reg is always the second property. See DTS coding style.
>
...
> > +        reg-names = "csid0",
>
> So this will be the third property.
>
>
>
> Best regards,
> Krzysztof
>
Best regards,
George

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 Sat, Jun 22, 2024 at 7:18 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> although I wasn't able to find
> the matching clock rates
The clk rate is in the camcc clk driver, isn't it? ;-)

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:14 AM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
> Yes but what we are saying here is you should have two patches. One
> patch to add the resources and a separate patch to add extended error
> messaging.

Sure, v2 is available with split patches for sure.