Message-Id: <D5E93DI7GAS8.23YY5O5CTU9IJ@fairphone.com>
Date: Tue, 05 Nov 2024 13:40:36 +0100
From: "Luca Weiss" <luca.weiss@...rphone.com>
To: "Vikram Sharma" <quic_vikramsa@...cinc.com>, <rfoss@...nel.org>,
 <todor.too@...il.com>, <bryan.odonoghue@...aro.org>, <mchehab@...nel.org>,
 <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
 <akapatra@...cinc.com>, <hariramp@...cinc.com>, <andersson@...nel.org>,
 <konradybcio@...nel.org>, <hverkuil-cisco@...all.nl>,
 <cros-qcom-dts-watchers@...omium.org>, <catalin.marinas@....com>,
 <will@...nel.org>, <matti.lehtimaki@...il.com>
Cc: <bryan.odonoghue@...aro.org>, <linux-arm-kernel@...ts.infradead.org>,
 <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <kernel@...cinc.com>
Subject: Re: [PATCH v4 0/6] media: qcom: camss: Add sc7280 support

Hi Vikram,

On Mon Nov 4, 2024 at 10:08 AM CET, Vikram Sharma wrote:
> Hi Luca,
>
> Thanks for your review and your efforts to validate tpg for this
> Please find my comments inline.
>
> Regards,
> Vikram
>
> On 11/1/2024 8:57 PM, Luca Weiss wrote:
> > On Wed Oct 30, 2024 at 11:53 AM CET, Vikram Sharma wrote:
> >> SC7280 is a Qualcomm SoC. This series adds support to bring up the CSIPHY,
> >> CSID, VFE/RDI interfaces in SC7280.
> >>
> >> SC7280 provides
> >>
> >> - 3 x VFE, 3 RDI per VFE
> >> - 2 x VFE Lite, 4 RDI per VFE
> >> - 3 x CSID
> >> - 2 x CSID Lite
> >> - 5 x CSI PHY
> >>
> >> The changes are verified on SC7280 qcs6490-rb3gen2 board, with attached vision mezzanine
> >> the base dts for qcs6490-rb3gen2 is:
> >> https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@quicinc.com/
> > Hi Vikram!
> >
> > Two things:
> >
> > You use the property "power-domains-names" in both bindings and dtsi but
> > this property is never parsed in the kernel. This should be
> > "power-domain-names"
> Thanks for this catch. I will update this in next version.
> >
> > Second, I still can't get the test pattern to work on my QCM6490-based
> > phone (Fairphone 5). Could you please try if the commands as per [0]
> > work on your board?
> I verified TPG with this change. but only mode 8 and 9 are working for TPG.
> Rest of the modes are getting stuck. To debug this I have verified the 
> register dump for all the TPG registers in all the modes.
> as expected the register programming is similar for all the modes just 
> mode field is getting updated.
> We are discussing this issue with TPG hardware team. Will keep you posted.
>
> Steps I used to validate TPG:
>
> echo "* Reset pipeline"
> media-ctl-d**/dev/media0**--reset
> yavta --list /dev/v4l-subdev5
>
> # /dev/v4l-subdev5 represent CSID0 can be different as well.
> yavta --no-query -w '0x009f0903*8*' /dev/v4l-subdev5
>
> media-ctl -d/dev/media0*-*V '"msm_csid0":0[fmt:SRGGB10/4056x3040]'
>
> media-ctl-d/dev/media0**-V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]'
>
> media-ctl-d/dev/media0**-l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
>
> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0

Thanks for checking, I can confirm I see the same behavior on my board,
test pattern 8 and 9 work as expected.

This seems to be some issue with the clocks since taking the clock
definitions from older sc7280 camss patches (never sent to the list)
test patterns 1-7 work properly.

Please see attached patch on top of yours with which it works. I haven't
dug into it more to find out which exact clock is causing this, but this
should be a good starting point for you & the hardware team.

Regards
Luca

>
> >
> > [0]https://lore.kernel.org/linux-arm-msm/c912f2da-519c-4bdc-a5cb-e19c3aa63ea8@linaro.org/
> >
> > Regards
> > Luca
> >
> >> Changes in V4:
> >> - V3 had 8 patches and V4 is reduced to 6.
> >> - Removed [Patch v3 2/8] as binding change is not required for dtso.
> >> - Removed [Patch v3 3/8] as the fix is already taken care in latest
> >>    kernel tip.
> >> - Updated alignment for dtsi and dt-bindings.
> >> - Adding qcs6490-rb3gen2-vision-mezzanine as overlay.
> >> - Link to v3:https://lore.kernel.org/linux-arm-msm/20241011140932.1744124-1-quic_vikramsa@quicinc.com/
> >>
> >> Changes in V3:
> >> - Added missed subject line for cover letter of V2.
> >> - Updated Alignment, indentation and properties order.
> >> - edit commit text for [PATCH 02/10] and [PATCH 03/10].
> >> - Refactor camss_link_entities.
> >> - Removed camcc enablement changes as it already done.
> >> - Link to v2:https://lore.kernel.org/linux-arm-msm/20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-v1-0-b18ddcd7d9df@quicinc.com/
> >>
> >> Changes in V2:
> >> - Improved indentation/formatting.
> >> - Removed _src clocks and misleading code comments.
> >> - Added name fields for power domains and csid register offset in DTSI.
> >> - Dropped minItems field from YAML file.
> >> - Listed changes in alphabetical order.
> >> - Updated description and commit text to reflect changes
> >> - Changed the compatible string from imx412 to imx577.
> >> - Added board-specific enablement changes in the newly created vision
> >>    board DTSI file.
> >> - Fixed bug encountered during testing.
> >> - Moved logically independent changes to a new/seprate patch.
> >> - Removed cci0 as no sensor is on this port and MCLK2, which was a
> >>    copy-paste error from the RB5 board reference.
> >> - Added power rails, referencing the RB5 board.
> >> - Discarded Patch 5/6 completely (not required).
> >> - Removed unused enums.
> >> - Link to v1:https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com/
> >>
> >> Suresh Vankadara (1):
> >>    media: qcom: camss: Add support for camss driver on SC7280
> >>
> >> Vikram Sharma (5):
> >>    media: dt-bindings: media: camss: Add qcom,sc7280-camss binding
> >>    media: qcom: camss: Sort CAMSS version enums and compatible strings
> >>    media: qcom: camss: Restructure camss_link_entities
> >>    arm64: dts: qcom: sc7280: Add support for camss
> >>    arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Add vision
> >>      mezzanine
> >>
> >>   .../bindings/media/qcom,sc7280-camss.yaml     | 439 +++++++++++++++
> >>   arch/arm64/boot/dts/qcom/Makefile             |   4 +
> >>   .../qcs6490-rb3gen2-vision-mezzanine.dtso     |  73 +++
> >>   arch/arm64/boot/dts/qcom/sc7280.dtsi          | 208 ++++++++
> >>   .../media/platform/qcom/camss/camss-csid.c    |   1 -
> >>   .../qcom/camss/camss-csiphy-3ph-1-0.c         |  13 +-
> >>   .../media/platform/qcom/camss/camss-csiphy.c  |   5 +
> >>   .../media/platform/qcom/camss/camss-csiphy.h  |   1 +
> >>   drivers/media/platform/qcom/camss/camss-vfe.c |   8 +-
> >>   drivers/media/platform/qcom/camss/camss.c     | 500 ++++++++++++++++--
> >>   drivers/media/platform/qcom/camss/camss.h     |   1 +
> >>   11 files changed, 1190 insertions(+), 63 deletions(-)
> >>   create mode 100644 Documentation/devicetree/bindings/media/qcom,sc7280-camss.yaml
> >>   create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso



View attachment "sc7280-camss-tpg-clocks.diff" of type "text/x-patch" (12677 bytes)

Powered by blists - more mailing lists