| Message ID | 20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-v1-0-b18ddcd7d9df@quicinc.com |
|---|---|
| Headers | show |
| Series | (no cover subject) | expand |
On 04/09/2024 12:10, 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 Please RESEND with a subject !
On 04/09/2024 12:21, Bryan O'Donoghue wrote: > On 04/09/2024 12:10, 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 > > Please RESEND with a subject ! > And don't forget to include the V number V3 ? perhaps. --- bod
On 04/09/2024 13:10, Vikram Sharma wrote: > Add changes to support the camera subsystem on the SC7280. > > Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 175 +++++++++++++++++++++++++++++++++++ > 1 file changed, 175 insertions(+) > This does not follow DTS coding style. Best regards, Krzysztof
On 04/09/2024 13:10, Vikram Sharma wrote: > Enable the IMX577 camera sensor for the qcs6490-rb3gen2-vision-mezzanine > board. Explain what hardware are you adding here. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > .../dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dts | 61 ++++++++++++++++++++++ > 2 files changed, 62 insertions(+) Different boards need different compatibles. OTOH, mezzanine is addon, not a board, so I would expect overlay. This is confusing. Best regards, Krzysztof
On 04/09/2024 13:10, Vikram Sharma wrote:
> Enable the camera clock driver for SC7280.
What is SC7280? Why this should be enabled? Your commit msg must explain
to these questions.
Best regards,
Krzysztof
On 04/09/2024 13:10, Vikram Sharma wrote: > Add default and suspend states for GPIO 67 and 78 on the SC7280. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- Please stop sending the same buggy patches. You received comment that this is broken and what you must fix. You keep sending the same patches over and over and expect different feedback. NAK. Best regards, Krzysztof
On Wed, 04 Sep 2024 16:40:06 +0530, 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-vision board, > the base dts for qcs6490-rb3gen2 is: > https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@quicinc.com/ > > V1 for this series: https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com/ > > Changes in V2: > 1) Improved indentation/formatting. > 2) Removed _src clocks and misleading code comments. > 3) Added name fields for power domains and csid register offset in DTSI. > 4) Dropped minItems field from YAML file. > 5) Listed changes in alphabetical order. > 6) Updated description and commit text to reflect changes > 7) Changed the compatible string from imx412 to imx577. > 8) Added board-specific enablement changes in the newly created vision > board DTSI file. > 9) Fixed bug encountered during testing. > 10) Moved logically independent changes to a new/seprate patch. > 11) Removed cci0 as no sensor is on this port and MCLK2, which was a > copy-paste error from the RB5 board reference. > 12) Added power rails, referencing the RB5 board. > 13) Discarded Patch 5/6 completely (not required). > 14) Removed unused enums. > > To: Robert Foss <rfoss@kernel.org> > To: Todor Tomov <todor.too@gmail.com> > To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > To: Mauro Carvalho Chehab <mchehab@kernel.org> > To: Rob Herring <robh@kernel.org> > To: Krzysztof Kozlowski <krzk+dt@kernel.org> > To: Conor Dooley <conor+dt@kernel.org> > To: Kapatrala Syed <akapatra@quicinc.com> > To: Hariram Purushothaman <hariramp@quicinc.com> > To: Bjorn Andersson <andersson@kernel.org> > To: Konrad Dybcio <konradybcio@kernel.org> > To: Hans Verkuil <hverkuil-cisco@xs4all.nl> > To: cros-qcom-dts-watchers@chromium.org > To: Catalin Marinas <catalin.marinas@arm.com> > To: Will Deacon <will@kernel.org> > Cc: linux-arm-msm@vger.kernel.org > Cc: linux-media@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > > Test-by: Vikram Sharma <quic_vikramsa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > Suresh Vankadara (1): > media: qcom: camss: Add support for camss driver on SC7280 > > Vikram Sharma (9): > media: dt-bindings: media: camss: Add qcom,sc7280-camss binding > media: dt-bindings: media: qcs6490-rb3gen2-vision-mezzanine: Add dt bindings > media: qcom: camss: Fix potential crash if domain attach fails > media: qcom: camss: Sort CAMSS version enums and compatible strings > media: qcom: camss: Add camss_link_entities_v2 > arm64: dts: qcom: sc7280: Add support for camss > arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Enable IMX577 sensor > arm64: dts: qcom: sc7280: Add default and suspend states for GPIO > arm64: defconfig: Enable camcc driver for SC7280 > > Documentation/devicetree/bindings/arm/qcom.yaml | 1 + > .../bindings/media/qcom,sc7280-camss.yaml | 441 +++++++++++++++++++++ > arch/arm64/boot/dts/qcom/Makefile | 1 + > .../dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dts | 61 +++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 208 ++++++++++ > arch/arm64/configs/defconfig | 1 + > drivers/media/platform/qcom/camss/camss-csid.c | 1 - > .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 13 +- > drivers/media/platform/qcom/camss/camss-csiphy.c | 5 + > drivers/media/platform/qcom/camss/camss-csiphy.h | 1 + > drivers/media/platform/qcom/camss/camss-vfe.c | 8 +- > drivers/media/platform/qcom/camss/camss.c | 400 ++++++++++++++++++- > drivers/media/platform/qcom/camss/camss.h | 1 + > 13 files changed, 1131 insertions(+), 11 deletions(-) > --- > base-commit: fdadd93817f124fd0ea6ef251d4a1068b7feceba > change-id: 20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-15c195fb3f12 > > Best regards, > -- > Vikram Sharma <quic_vikramsa@quicinc.com> > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y qcom/qcs6490-rb3gen2-vision-mezzanine.dtb' for 20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-v1-0-b18ddcd7d9df@quicinc.com: arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: pcie@1c08000: interrupts: [[0, 307, 4], [0, 308, 4], [0, 309, 4], [0, 312, 4], [0, 313, 4], [0, 314, 4], [0, 374, 4], [0, 375, 4]] is too long from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml# arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: pcie@1c08000: interrupt-names:0: 'msi' was expected from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml# arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: pcie@1c08000: interrupt-names: ['msi0', 'msi1', 'msi2', 'msi3', 'msi4', 'msi5', 'msi6', 'msi7'] is too long from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml# arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: usb@8cf8800: interrupt-names: ['pwr_event', 'hs_phy_irq', 'dp_hs_phy_irq', 'dm_hs_phy_irq'] is too short from schema $id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml# arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: video-codec@aa00000: iommus: [[66, 8576, 32]] is too short from schema $id: http://devicetree.org/schemas/media/qcom,sc7280-venus.yaml# arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtb: pinctrl@f100000: Unevaluated properties are not allowed ('cam2-default', 'cam2-suspend' were unexpected) from schema $id: http://devicetree.org/schemas/pinctrl/qcom,sc7280-pinctrl.yaml#
On 04/09/2024 12:10, Vikram Sharma wrote: > Sort CAMSS version enums and compatible strings alphanumerically. > > Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 04/09/2024 12:10, Vikram Sharma wrote: > Add camss_link_entities_v2, derived from the camss_link_entities > function, to handle linking for targets without ISPIF. > > camss_link_entities -> Targets with ispif. > camss_link_entities_v2 -> Targets without ispif. > > Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > drivers/media/platform/qcom/camss/camss.c | 53 ++++++++++++++++++++++++++++++- > 1 file changed, 52 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 5e7235001239..516434686a27 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -2154,6 +2154,57 @@ static int camss_init_subdevices(struct camss *camss) > return 0; > } > > +/* > + * camss_link_entities_v2 - Register subdev nodes and create links > + * @camss: CAMSS device > + * > + * Return 0 on success or a negative error code on failure > + */ > +static int camss_link_entities_v2(struct camss *camss) > +{ > + int i, j; > + int ret; > + > + for (i = 0; i < camss->res->csiphy_num; i++) { > + for (j = 0; j < camss->res->csid_num; j++) { > + ret = media_create_pad_link(&camss->csiphy[i].subdev.entity, > + MSM_CSIPHY_PAD_SRC, > + &camss->csid[j].subdev.entity, > + MSM_CSID_PAD_SINK, > + 0); > + if (ret < 0) { > + dev_err(camss->dev, > + "Failed to link %s->%s entities: %d\n", > + camss->csiphy[i].subdev.entity.name, > + camss->csid[j].subdev.entity.name, > + ret); > + return ret; > + } > + } > + } Please reduce the above loop down into a common call that both camss_link_entities_ispif()[1] and camss_link_entities() can call. Because => functional decomposition and code reuse instead of code replication. > + for (i = 0; i < camss->res->csid_num; i++) > + for (j = 0; j < camss->vfe[i].res->line_num; j++) { > + struct v4l2_subdev *csid = &camss->csid[i].subdev; > + struct v4l2_subdev *vfe = &camss->vfe[i].line[j].subdev; > + > + ret = media_create_pad_link(&csid->entity, > + MSM_CSID_PAD_FIRST_SRC + j, > + &vfe->entity, > + MSM_VFE_PAD_SINK, > + 0); > + if (ret < 0) { > + dev_err(camss->dev, > + "Failed to link %s->%s entities: %d\n", > + csid->entity.name, > + vfe->entity.name, > + ret); > + return ret; > + } > + } > + return 0; > +} Having a tidy function for every non ispif SoC seems like nice code, approve. However the corollary is the ispif version of the code should then drop the if (camss->ispif) { //do stuff } else { // do other stuff } i.e. your function pointer now determines if ispif is true so you can drop the dead code on the else path in camss_link_entities_ispif() > + > /* > * camss_link_entities - Register subdev nodes and create links > * @camss: CAMSS device > @@ -2769,7 +2820,7 @@ static const struct camss_resources sc8280xp_resources = { > .csiphy_num = ARRAY_SIZE(csiphy_res_sc8280xp), > .csid_num = ARRAY_SIZE(csid_res_sc8280xp), > .vfe_num = ARRAY_SIZE(vfe_res_sc8280xp), > - .link_entities = camss_link_entities > + .link_entities = camss_link_entities_v2 > }; > > static const struct camss_resources sc7280_resources = { > This change isn't correct. There are six upstream camss entires grep camss arch/arm64/boot/dts/qcom/*.dtsi | grep compat | wc -l 6 Only three of which have ispif grep -m1 "ispif" arch/arm64/boot/dts/qcom/*.dtsi | wc -l 3 => You've missed 2/3 of the SoCs the change should apply to - 845 and 8250. [1] I also think the legacy function should be called camss_link_entities_ispif() with the default being camss_link_entities(); 1. Please change the name of the existing function to camss_link_entities_ispif() to reflect its function and dependency. 2. Functionally decompose the top part of the loop into some common method that both camss_link_entities_ispif() and camss_link_entities() can call. Because functionally decomposed and reused code is better code. 3. You need to make sure you hook the right set of SoCs with the default and legacy versions of the function -> msm8916/msm8996/sdm630 .link_entites = camss_link_entities_ispif() -> sdm845/sc7280/sc8280xp .link_entities = camss_link_entities() --- bod
On 4.09.2024 1:10 PM, Vikram Sharma wrote: > Fix a potential crash in camss by ensuring detach is skipped if attach > is unsuccessful. > > Fixes: d89751c61279 ("media: qcom: camss: Add support for named power-domains") > CC: stable@vger.kernel.org > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > drivers/media/platform/qcom/camss/camss.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index d64985ca6e88..447b89d07e8a 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -2132,7 +2132,7 @@ static int camss_configure_pd(struct camss *camss) > camss->res->pd_name); > if (IS_ERR(camss->genpd)) { > ret = PTR_ERR(camss->genpd); > - goto fail_pm; > + goto fail_pm_attach; > } > } > > @@ -2149,7 +2149,7 @@ static int camss_configure_pd(struct camss *camss) > ret = -ENODEV; > else > ret = PTR_ERR(camss->genpd); > - goto fail_pm; > + goto fail_pm_attach; > } > camss->genpd_link = device_link_add(camss->dev, camss->genpd, > DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | > @@ -2164,6 +2164,7 @@ static int camss_configure_pd(struct camss *camss) > fail_pm: > dev_pm_domain_detach(camss->genpd, true); > > +fail_pm_attach: > return ret; What's the point, just call return directly where you added the goto Konrad
On Wed Sep 4, 2024 at 1:10 PM CEST, 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 Hi Vikram, I tried this on my QCM6490 Fairphone 5 smartphone. Unfortunately I couldn't get e.g. CSID test pattern out of camss. I've tested this patchset on v6.11. These commands did work on an older sc7280 camss patchset (which was never sent to the lists). Can you please take a look? v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=1 media-ctl -d /dev/media0 -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -V '"msm_csid0":1[fmt:UYVY8_2X8/1920x1080 field:none],"msm_vfe0_rdi0":0[fmt:UYVY8_2X8/1920x1080 field:none]' gst-launch-1.0 v4l2src device=/dev/video0 num-buffers=1 ! 'video/x-raw,format=UYVY,width=1920,height=1080' ! jpegenc ! filesink location=image01.jpg The last command just hangs instead of producing a picture in image01.jpg. Can you please check if this works for you on your board? Regards Luca > > The changes are verified on SC7280 qcs6490-rb3gen2-vision board, > the base dts for qcs6490-rb3gen2 is: > https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@quicinc.com/ > > V1 for this series: https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com/ > > Changes in V2: > 1) Improved indentation/formatting. > 2) Removed _src clocks and misleading code comments. > 3) Added name fields for power domains and csid register offset in DTSI. > 4) Dropped minItems field from YAML file. > 5) Listed changes in alphabetical order. > 6) Updated description and commit text to reflect changes > 7) Changed the compatible string from imx412 to imx577. > 8) Added board-specific enablement changes in the newly created vision > board DTSI file. > 9) Fixed bug encountered during testing. > 10) Moved logically independent changes to a new/seprate patch. > 11) Removed cci0 as no sensor is on this port and MCLK2, which was a > copy-paste error from the RB5 board reference. > 12) Added power rails, referencing the RB5 board. > 13) Discarded Patch 5/6 completely (not required). > 14) Removed unused enums. > > To: Robert Foss <rfoss@kernel.org> > To: Todor Tomov <todor.too@gmail.com> > To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > To: Mauro Carvalho Chehab <mchehab@kernel.org> > To: Rob Herring <robh@kernel.org> > To: Krzysztof Kozlowski <krzk+dt@kernel.org> > To: Conor Dooley <conor+dt@kernel.org> > To: Kapatrala Syed <akapatra@quicinc.com> > To: Hariram Purushothaman <hariramp@quicinc.com> > To: Bjorn Andersson <andersson@kernel.org> > To: Konrad Dybcio <konradybcio@kernel.org> > To: Hans Verkuil <hverkuil-cisco@xs4all.nl> > To: cros-qcom-dts-watchers@chromium.org > To: Catalin Marinas <catalin.marinas@arm.com> > To: Will Deacon <will@kernel.org> > Cc: linux-arm-msm@vger.kernel.org > Cc: linux-media@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > > Test-by: Vikram Sharma <quic_vikramsa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > Suresh Vankadara (1): > media: qcom: camss: Add support for camss driver on SC7280 > > Vikram Sharma (9): > media: dt-bindings: media: camss: Add qcom,sc7280-camss binding > media: dt-bindings: media: qcs6490-rb3gen2-vision-mezzanine: Add dt bindings > media: qcom: camss: Fix potential crash if domain attach fails > media: qcom: camss: Sort CAMSS version enums and compatible strings > media: qcom: camss: Add camss_link_entities_v2 > arm64: dts: qcom: sc7280: Add support for camss > arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Enable IMX577 sensor > arm64: dts: qcom: sc7280: Add default and suspend states for GPIO > arm64: defconfig: Enable camcc driver for SC7280 > > Documentation/devicetree/bindings/arm/qcom.yaml | 1 + > .../bindings/media/qcom,sc7280-camss.yaml | 441 +++++++++++++++++++++ > arch/arm64/boot/dts/qcom/Makefile | 1 + > .../dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dts | 61 +++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 208 ++++++++++ > arch/arm64/configs/defconfig | 1 + > drivers/media/platform/qcom/camss/camss-csid.c | 1 - > .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 13 +- > drivers/media/platform/qcom/camss/camss-csiphy.c | 5 + > drivers/media/platform/qcom/camss/camss-csiphy.h | 1 + > drivers/media/platform/qcom/camss/camss-vfe.c | 8 +- > drivers/media/platform/qcom/camss/camss.c | 400 ++++++++++++++++++- > drivers/media/platform/qcom/camss/camss.h | 1 + > 13 files changed, 1131 insertions(+), 11 deletions(-) > --- > base-commit: fdadd93817f124fd0ea6ef251d4a1068b7feceba > change-id: 20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-15c195fb3f12 > > Best regards,
On 30/09/2024 11:52, Luca Weiss wrote: > On Wed Sep 4, 2024 at 1:10 PM CEST, 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 > > Hi Vikram, > > I tried this on my QCM6490 Fairphone 5 smartphone. > > Unfortunately I couldn't get e.g. CSID test pattern out of camss. I've > tested this patchset on v6.11. > > These commands did work on an older sc7280 camss patchset (which was > never sent to the lists). Can you please take a look? > > v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=1 > media-ctl -d /dev/media0 -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > media-ctl -d /dev/media0 -V '"msm_csid0":1[fmt:UYVY8_2X8/1920x1080 field:none],"msm_vfe0_rdi0":0[fmt:UYVY8_2X8/1920x1080 field:none]' > gst-launch-1.0 v4l2src device=/dev/video0 num-buffers=1 ! 'video/x-raw,format=UYVY,width=1920,height=1080' ! jpegenc ! filesink location=image01.jpg Here's what I have for rb5 # CSID0 TPG RB5 media-ctl --reset yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev6 yavta --list /dev/v4l-subdev6 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -p Maybe on FP5 ... media-ctl --reset yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 yavta --list /dev/v4l-subdev5 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -p ? --- bod
On Mon Sep 30, 2024 at 1:54 PM CEST, Bryan O'Donoghue wrote: > On 30/09/2024 11:52, Luca Weiss wrote: > > On Wed Sep 4, 2024 at 1:10 PM CEST, 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 > > > > Hi Vikram, > > > > I tried this on my QCM6490 Fairphone 5 smartphone. > > > > Unfortunately I couldn't get e.g. CSID test pattern out of camss. I've > > tested this patchset on v6.11. > > > > These commands did work on an older sc7280 camss patchset (which was > > never sent to the lists). Can you please take a look? > > > > v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=1 > > media-ctl -d /dev/media0 -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > > media-ctl -d /dev/media0 -V '"msm_csid0":1[fmt:UYVY8_2X8/1920x1080 field:none],"msm_vfe0_rdi0":0[fmt:UYVY8_2X8/1920x1080 field:none]' > > gst-launch-1.0 v4l2src device=/dev/video0 num-buffers=1 ! 'video/x-raw,format=UYVY,width=1920,height=1080' ! jpegenc ! filesink location=image01.jpg > > Here's what I have for rb5 > > # CSID0 TPG RB5 > media-ctl --reset > yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev6 > yavta --list /dev/v4l-subdev6 > media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' > media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' > media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > media-ctl -d /dev/media0 -p > > Maybe on FP5 ... > > media-ctl --reset > yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 > yavta --list /dev/v4l-subdev5 > media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' > media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' > media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > media-ctl -d /dev/media0 -p Hi Bryan! These commands are to set up the pipeline, and what then to grab an image from it? I tried this, but it also just hangs: $ yavta -B capture-mplane --capture=3 -n 3 -f SRGGB10P -s 4056x3040 /dev/video0 --file=foo-#.bin Device /dev/video0 opened. Device `Qualcomm Camera Subsystem' on `platform:acb3000.camss' (driver 'qcom-camss') supports video, capture, with mplanes. Video format set: SRGGB10P (41415270) 4056x3040 field none, 1 planes: * Stride 5072, buffer size 15418880 Video format: SRGGB10P (41415270) 4056x3040 field none, 1 planes: * Stride 5072, buffer size 15418880 3 buffers requested. length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 0/0 mapped at address 0xffffa0c00000. length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 1/0 mapped at address 0xffff9fc08000. length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 2/0 mapped at address 0xffff9ec10000. Regards Luca > > ? > > --- > bod
> On Mon Sep 30, 2024 at 1:54 PM CEST, Bryan O'Donoghue wrote: >>> On 30/09/2024 11:52, Weiss wrote: >> > On Wed Sep 4, 2024 at 1:10 PM CEST, 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 >> > >> > Hi Vikram, >> > >> > I tried this on my QCM6490 Fairphone 5 smartphone. >> > >> > Unfortunately I couldn't get e.g. CSID test pattern out of camss. >> > I've tested this patchset on v6.11. >> > >> > These commands did work on an older sc7280 camss patchset (which was >> > never sent to the lists). Can you please take a look? >> > >> > v4l2-ctl -d /dev/v4l-subdev5 -c test_pattern=1 media-ctl -d >> > /dev/media0 -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> > media-ctl -d /dev/media0 -V '"msm_csid0":1[fmt:UYVY8_2X8/1920x1080 field:none],"msm_vfe0_rdi0":0[fmt:UYVY8_2X8/1920x1080 field:none]' >> > gst-launch-1.0 v4l2src device=/dev/video0 num-buffers=1 ! >> > 'video/x-raw,format=UYVY,width=1920,height=1080' ! jpegenc ! >> > filesink location=image01.jpg >> >> Here's what I have for rb5 >> >> # CSID0 TPG RB5 >> media-ctl --reset >> yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev6 yavta --list >> /dev/v4l-subdev6 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> media-ctl -d /dev/media0 -p >> >> Maybe on FP5 ... >> >> media-ctl --reset >> yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 yavta --list >> /dev/v4l-subdev5 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> media-ctl -d /dev/media0 -p >Hi Bryan! > >These commands are to set up the pipeline, and what then to grab an image from it? > >I tried this, but it also just hangs: > >$ yavta -B capture-mplane --capture=3 -n 3 -f SRGGB10P -s 4056x3040 /dev/video0 --file=foo-#.bin Device /dev/video0 opened. >Device `Qualcomm Camera Subsystem' on `platform:acb3000.camss' (driver 'qcom-camss') supports video, capture, with mplanes. >Video format set: SRGGB10P (41415270) 4056x3040 field none, 1 planes: > * Stride 5072, buffer size 15418880 >Video format: SRGGB10P (41415270) 4056x3040 field none, 1 planes: > * Stride 5072, buffer size 15418880 >3 buffers requested. >length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 0/0 mapped at address 0xffffa0c00000. >length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 1/0 mapped at address 0xffff9fc08000. >length: 1 offset: 3326519176 timestamp type/source: mono/EoF Buffer 2/0 mapped at address 0xffff9ec10000. > >Regards >Luca Hi Luca, We will try to reproduce this internally and get back. Thanks, Vikram
On 01/10/2024 09:24, Luca Weiss wrote: >> media-ctl --reset >> yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 >> yavta --list /dev/v4l-subdev5 >> media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' >> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' >> media-ctl -d /dev/media0 -p > Hi Bryan! > > These commands are to set up the pipeline, and what then to grab an > image from it? > > I tried this, but it also just hangs: > > $ yavta -B capture-mplane --capture=3 -n 3 -f SRGGB10P -s 4056x3040 /dev/video0 --file=foo-#.bin > Device /dev/video0 opened. > Device `Qualcomm Camera Subsystem' on `platform:acb3000.camss' (driver 'qcom-camss') supports video, capture, with mplanes. > Video format set: SRGGB10P (41415270) 4056x3040 field none, 1 planes: > * Stride 5072, buffer size 15418880 > Video format: SRGGB10P (41415270) 4056x3040 field none, 1 planes: > * Stride 5072, buffer size 15418880 > 3 buffers requested. > length: 1 offset: 3326519176 timestamp type/source: mono/EoF > Buffer 0/0 mapped at address 0xffffa0c00000. > length: 1 offset: 3326519176 timestamp type/source: mono/EoF > Buffer 1/0 mapped at address 0xffff9fc08000. > length: 1 offset: 3326519176 timestamp type/source: mono/EoF > Buffer 2/0 mapped at address 0xffff9ec10000. No there's no CSIPHY in that case, it should be the TPG inside of CSID0 @ /dev/v4l-subdev5 which generates the data. Just for verification purposes do a `media-ctl -d /dev/media0 -p` and confirm that /dev/v4l-subdev5 == csid0 Rewrite the above as export csid0=v4l-subdevX media-ctl --reset yavta --no-query -w '0x009f0903 2' /dev/$csid0 yavta --list /dev/$csid0 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -p basically you have to make sure you've set the TPG on the correct subdev.. Something like in media-ctl subdev4 in my case. - entity 13: msm_csid0 (5 pads, 36 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev4 => export csid0=v4l-subdev4 media-ctl --reset yavta --no-query -w '0x009f0903 2' /dev/$csid0 yavta --list /dev/$csid0 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -p --- bod
On 01/10/2024 12:39, Luca Weiss wrote: > And v4l-subdev5 is msm_csid0 on my device. <snip> > > - entity 16: msm_csid0 (5 pads, 22 links, 0 routes) > type V4L2 subdev subtype Unknown flags 0 > device node name /dev/v4l-subdev5 > pad0: Sink > [stream:0 fmt:SRGGB10_1X10/4056x3040 field:none colorspace:srgb] > <- "msm_csiphy0":1 [] > <- "msm_csiphy1":1 [] > <- "msm_csiphy2":1 [] > <- "msm_csiphy3":1 [] > <- "msm_csiphy4":1 [] > pad1: Source > [stream:0 fmt:SRGGB10_1X10/4056x3040 field:none colorspace:srgb] > -> "msm_vfe0_rdi0":0 [ENABLED] > -> "msm_vfe1_rdi0":0 [] > -> "msm_vfe2_rdi0":0 [] > -> "msm_vfe3_rdi0":0 [] > -> "msm_vfe4_rdi0":0 [] <snip> media-ctl --reset yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 yavta --list /dev/v4l-subdev5 media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' media-ctl -d /dev/media0 -p That command list and this yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0 should work. I have to test Vladimir's two patches. I'll verify rb5 TPG while I'm at it, perhaps the error is not sdm670 specific. That said last time I tested it, it worked and no changes have gone in, in the meantime. --- bod
On Tue Oct 1, 2024 at 2:49 PM CEST, Bryan O'Donoghue wrote: > On 01/10/2024 12:39, Luca Weiss wrote: > > > And v4l-subdev5 is msm_csid0 on my device. > > <snip> > > > > > - entity 16: msm_csid0 (5 pads, 22 links, 0 routes) > > type V4L2 subdev subtype Unknown flags 0 > > device node name /dev/v4l-subdev5 > > pad0: Sink > > [stream:0 fmt:SRGGB10_1X10/4056x3040 field:none colorspace:srgb] > > <- "msm_csiphy0":1 [] > > <- "msm_csiphy1":1 [] > > <- "msm_csiphy2":1 [] > > <- "msm_csiphy3":1 [] > > <- "msm_csiphy4":1 [] > > pad1: Source > > [stream:0 fmt:SRGGB10_1X10/4056x3040 field:none colorspace:srgb] > > -> "msm_vfe0_rdi0":0 [ENABLED] > > -> "msm_vfe1_rdi0":0 [] > > -> "msm_vfe2_rdi0":0 [] > > -> "msm_vfe3_rdi0":0 [] > > -> "msm_vfe4_rdi0":0 [] > > <snip> > > media-ctl --reset > yavta --no-query -w '0x009f0903 2' /dev/v4l-subdev5 > yavta --list /dev/v4l-subdev5 > media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]' > media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]' > media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]' > media-ctl -d /dev/media0 -p > > That command list and this > > yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0 > > should work. Yeah, unfortunately this is still hanging... Let's also see what Vikram sees on their board. fairphone-fp5:~$ yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0 Device /dev/video0 opened. Device `Qualcomm Camera Subsystem' on `platform:acb3000.camss' (driver 'qcom-camss') supports video, capture, with mplanes. Video format set: SRGGB10P (41415270) 4056x3040 field none, 1 planes: * Stride 5072, buffer size 15418880 Video format: SRGGB10P (41415270) 4056x3040 field none, 1 planes: * Stride 5072, buffer size 15418880 5 buffers requested. length: 1 offset: 3442938648 timestamp type/source: mono/EoF Buffer 0/0 mapped at address 0xffff85e00000. length: 1 offset: 3442938648 timestamp type/source: mono/EoF Buffer 1/0 mapped at address 0xffff84e08000. length: 1 offset: 3442938648 timestamp type/source: mono/EoF Buffer 2/0 mapped at address 0xffff83e10000. length: 1 offset: 3442938648 timestamp type/source: mono/EoF Buffer 3/0 mapped at address 0xffff82e18000. length: 1 offset: 3442938648 timestamp type/source: mono/EoF Buffer 4/0 mapped at address 0xffff81e20000. > I have to test Vladimir's two patches. I'll verify rb5 TPG while I'm at > it, perhaps the error is not sdm670 specific. FWIW this is not sdm670 but sc7280/qcm6490 here :) But I didn't follow the sdm670 thread so maybe you mean something there. > > That said last time I tested it, it worked and no changes have gone in, > in the meantime. I also had the test pattern working on a 6.8-based kernel on this device with camss patches from Matti Lehtimäki. Regards Luca > > --- > bod
On 01/10/2024 15:22, Luca Weiss wrote: >> I have to test Vladimir's two patches. I'll verify rb5 TPG while I'm at >> it, perhaps the error is not sdm670 specific. > FWIW this is not sdm670 but sc7280/qcm6490 here 🙂 But I didn't follow > the sdm670 thread so maybe you mean something there. Yes I sc7280/sm8250. Freudian slip, when you type one thing but you mean your mother. --- bod
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-vision board, the base dts for qcs6490-rb3gen2 is: https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@quicinc.com/ V1 for this series: https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com/ Changes in V2: 1) Improved indentation/formatting. 2) Removed _src clocks and misleading code comments. 3) Added name fields for power domains and csid register offset in DTSI. 4) Dropped minItems field from YAML file. 5) Listed changes in alphabetical order. 6) Updated description and commit text to reflect changes 7) Changed the compatible string from imx412 to imx577. 8) Added board-specific enablement changes in the newly created vision board DTSI file. 9) Fixed bug encountered during testing. 10) Moved logically independent changes to a new/seprate patch. 11) Removed cci0 as no sensor is on this port and MCLK2, which was a copy-paste error from the RB5 board reference. 12) Added power rails, referencing the RB5 board. 13) Discarded Patch 5/6 completely (not required). 14) Removed unused enums. To: Robert Foss <rfoss@kernel.org> To: Todor Tomov <todor.too@gmail.com> To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> To: Mauro Carvalho Chehab <mchehab@kernel.org> To: Rob Herring <robh@kernel.org> To: Krzysztof Kozlowski <krzk+dt@kernel.org> To: Conor Dooley <conor+dt@kernel.org> To: Kapatrala Syed <akapatra@quicinc.com> To: Hariram Purushothaman <hariramp@quicinc.com> To: Bjorn Andersson <andersson@kernel.org> To: Konrad Dybcio <konradybcio@kernel.org> To: Hans Verkuil <hverkuil-cisco@xs4all.nl> To: cros-qcom-dts-watchers@chromium.org To: Catalin Marinas <catalin.marinas@arm.com> To: Will Deacon <will@kernel.org> Cc: linux-arm-msm@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Test-by: Vikram Sharma <quic_vikramsa@quicinc.com> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> --- Suresh Vankadara (1): media: qcom: camss: Add support for camss driver on SC7280 Vikram Sharma (9): media: dt-bindings: media: camss: Add qcom,sc7280-camss binding media: dt-bindings: media: qcs6490-rb3gen2-vision-mezzanine: Add dt bindings media: qcom: camss: Fix potential crash if domain attach fails media: qcom: camss: Sort CAMSS version enums and compatible strings media: qcom: camss: Add camss_link_entities_v2 arm64: dts: qcom: sc7280: Add support for camss arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Enable IMX577 sensor arm64: dts: qcom: sc7280: Add default and suspend states for GPIO arm64: defconfig: Enable camcc driver for SC7280 Documentation/devicetree/bindings/arm/qcom.yaml | 1 + .../bindings/media/qcom,sc7280-camss.yaml | 441 +++++++++++++++++++++ arch/arm64/boot/dts/qcom/Makefile | 1 + .../dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dts | 61 +++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 208 ++++++++++ arch/arm64/configs/defconfig | 1 + drivers/media/platform/qcom/camss/camss-csid.c | 1 - .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 13 +- drivers/media/platform/qcom/camss/camss-csiphy.c | 5 + drivers/media/platform/qcom/camss/camss-csiphy.h | 1 + drivers/media/platform/qcom/camss/camss-vfe.c | 8 +- drivers/media/platform/qcom/camss/camss.c | 400 ++++++++++++++++++- drivers/media/platform/qcom/camss/camss.h | 1 + 13 files changed, 1131 insertions(+), 11 deletions(-) --- base-commit: fdadd93817f124fd0ea6ef251d4a1068b7feceba change-id: 20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-15c195fb3f12 Best regards,