Re: [PATCH v4 00/22] Add support for the SDM845 Camera Subsystem

From: Robert Foss
Date: Wed Feb 17 2021 - 06:07:10 EST


On Wed, 10 Feb 2021 at 21:14, Andrey Konovalov
<andrey.konovalov@xxxxxxxxxx> wrote:
>
> Hi Robert,
>
> On 05.02.2021 13:43, Robert Foss wrote:
> > This series implements support for the camera subsystem found in
> > the SDM845 SOCs and the Titan 170 ISP. The support is partial
> > in that it implements CSIPHY, CSID, and partial VFE support.
> >
> > The Titan generation of the ISP diverges a fair amount from the
> > design of the previous architecture generation, CAMSS. As a result
> > some pretty invasive refactoring is done in this series. It also
> > means that at this time we're unable to implement support for all
> > of the IP blocks contained. This is due to a combination of legal
> > considerations with respect to the IP and its owner Qualcomm and
> > time & man hour constrains on the Linaro side.
> >
> > The CSIPHY (CSI Physical Layer) & CSID (CSI Decoder) support is
> > complete, but the VFE (Video Front End, which is referred to as IFE
> > (Image Front End) in the Titan generation of ISPs) only has support
> > for the RDI (Raw Dump Interface) which allows the raw output of
> > the CSID to be written to memory.
> >
> > The 2nd interface implemented in the VFE silicon is the PIX
> > interface, and camss does not support it for this generation of ISPs.
> > The reason for this is that the PIX interface is used for sending
> > image data to the BPS (Bayer Processing Section) & IPE (Image
> > Processing Engine), but both of these units are beyond the scope
> > of enabling basic ISP functionality for the SDM845.
>
> The problem is that for SDM845 the topology printed by media-ctl
> still has the PIX devices. That is even though the PIX interface is not
> supported for SDM845 in this driver, the msm_vfeN_pix subdevices
> and the corresponding msm_vfeN_video3 devices are still created.
> Your patchset is currently missing changes to the hardcoded:
>
> #define MSM_VFE_LINE_NUM 4
>
> struct vfe_device {
> ...
> struct vfe_line line[MSM_VFE_LINE_NUM];
> ...
> };
>
> in drivers/media/platform/qcom/camss/camss-vfe.h.

I had a look through the driver and made the line number variable for
the different versions of hardware. This required touching most of the
vfe related compilation units, but was a pretty mechanical change.

Thanks for spotting this issue.

>
>
> Thanks,
> Andrey
>
> > Since the Titan architecture generation diverges quite a bit from
> > the CAMSS generation, a lot of pretty major refactoring is carried
> > out in this series. Both the CSID & VFE core paths are made more
> > general and hardware version specific parts are broken out.
> > The CSIPHY didn't require quite as radical changes and therefore
> > keeps its current form.
> >
> > Tested on:
> > - Qcom RB3 / db845c + camera mezzanine, which is SDM845 based
> > - db410c + D3 Camera mezzanine, which is APQ8016 based
> >
> > Branch:
> > - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v1
> > - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v2
> > - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v3
> >
> >
> > Due to the dt-bindings supporting sdm660-camss, this series depends
> > the sdm660 clock driver being upstreamed. I've linked this series below.
> >
> > SDM630/660 Multimedia and GPU clock controllers
> > https://lkml.org/lkml/2020/9/26/166
> >
> >
> > Robert Foss (22):
> > media: camss: Fix vfe_isr_comp_done() documentation
> > media: camss: Fix vfe_isr comment typo
> > media: camss: Replace trace_printk() with dev_dbg()
> > media: camss: Add CAMSS_845 camss version
> > media: camss: Make ISPIF subdevice optional
> > media: camss: Refactor VFE HW version support
> > media: camss: Add support for VFE hardware version Titan 170
> > media: camss: Add missing format identifiers
> > media: camss: Refactor CSID HW version support
> > media: camss: Add support for CSID hardware version Titan 170
> > media: camss: Add support for CSIPHY hardware version Titan 170
> > media: camss: Remove per VFE power domain toggling
> > media: camss: Enable SDM845
> > dt-bindings: media: camss: Add qcom,msm8916-camss binding
> > dt-bindings: media: camss: Add qcom,msm8996-camss binding
> > dt-bindings: media: camss: Add qcom,sdm660-camss binding
> > dt-bindings: media: camss: Add qcom,sdm845-camss binding
> > MAINTAINERS: Change CAMSS documentation to use dtschema bindings
> > media: dt-bindings: media: Remove qcom,camss documentation
> > arm64: dts: sdm845: Add CAMSS ISP node
> > arm64: dts: sdm845-db845c: Configure regulators for camss node
> > arm64: dts: sdm845-db845c: Enable ov8856 sensor and connect to ISP
> >
> > .../devicetree/bindings/media/qcom,camss.txt | 236 ----
> > .../bindings/media/qcom,msm8916-camss.yaml | 256 ++++
> > .../bindings/media/qcom,msm8996-camss.yaml | 387 ++++++
> > .../bindings/media/qcom,sdm660-camss.yaml | 398 ++++++
> > .../bindings/media/qcom,sdm845-camss.yaml | 370 ++++++
> > MAINTAINERS | 2 +-
> > arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 23 +-
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 135 ++
> > drivers/media/platform/qcom/camss/Makefile | 6 +
> > .../platform/qcom/camss/camss-csid-170.c | 602 +++++++++
> > .../platform/qcom/camss/camss-csid-4-1.c | 338 +++++
> > .../platform/qcom/camss/camss-csid-4-7.c | 406 ++++++
> > .../media/platform/qcom/camss/camss-csid.c | 620 +--------
> > .../media/platform/qcom/camss/camss-csid.h | 178 ++-
> > .../qcom/camss/camss-csiphy-3ph-1-0.c | 182 ++-
> > .../media/platform/qcom/camss/camss-csiphy.c | 66 +-
> > .../media/platform/qcom/camss/camss-ispif.c | 117 +-
> > .../media/platform/qcom/camss/camss-ispif.h | 3 +-
> > .../media/platform/qcom/camss/camss-vfe-170.c | 804 ++++++++++++
> > .../media/platform/qcom/camss/camss-vfe-4-1.c | 123 +-
> > .../media/platform/qcom/camss/camss-vfe-4-7.c | 244 ++--
> > .../media/platform/qcom/camss/camss-vfe-4-8.c | 1164 +++++++++++++++++
> > .../platform/qcom/camss/camss-vfe-gen1.c | 763 +++++++++++
> > .../platform/qcom/camss/camss-vfe-gen1.h | 110 ++
> > drivers/media/platform/qcom/camss/camss-vfe.c | 840 +-----------
> > drivers/media/platform/qcom/camss/camss-vfe.h | 118 +-
> > .../media/platform/qcom/camss/camss-video.c | 100 ++
> > drivers/media/platform/qcom/camss/camss.c | 419 ++++--
> > drivers/media/platform/qcom/camss/camss.h | 17 +-
> > 29 files changed, 6965 insertions(+), 2062 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
> > create mode 100644 Documentation/devicetree/bindings/media/qcom,msm8916-camss.yaml
> > create mode 100644 Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
> > create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm660-camss.yaml
> > create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
> > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-170.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-1.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-7.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-170.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-4-8.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-gen1.c
> > create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-gen1.h
> >