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

[Hans Verkuil <hverkuil@xs4all.nl>]

On 11/03/2021 19:09, 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.
> 
> 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
>  - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v4
>  - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v5
>  - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v6
>  - https://git.linaro.org/people/robert.foss/linux.git/log/?h=camss_sdm845_v7
> 

Please check the patches in this series with 'scripts/checkpatch.pl --strict'.
I got a bunch of warnings, most seemed trivial to fix.

When I compiled this series I also got these sparse warnings:

drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-4-1.c:959:30: warning: symbol
'vfe_ops_gen1_4_1' was not declared. Should it be static?
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-4-1.c:959:30: warning: symbol
'vfe_ops_gen1_4_1' was not declared. Should it be static?
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-1.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-7.c drivers/media/platform/qcom/camss/camss-vfe-4-7.c:1151:30: warning: symbol
'vfe_ops_gen1_4_7' was not declared. Should it be static?
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-7.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-7.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-7.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-7.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-170.c drivers/media/platform/qcom/camss/camss-vfe-170.c:770:26: warning: symbol
'vfe_isr_ops_170' was not declared. Should it be static?
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-8.c drivers/media/platform/qcom/camss/camss-vfe-4-8.c:1140:30: warning: symbol
'vfe_ops_gen1_4_8' was not declared. Should it be static?
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-8.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-8.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-8.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
SPARSE:drivers/media/platform/qcom/camss/camss-vfe-4-8.c drivers/media/platform/qcom/camss/camss-vfe-gen1.h:69:31: error: marked inline, but
without a definition
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]

And these smatch warnings (a lot of overlap with sparse):

drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-csid.h:66:27: warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=]
drivers/media/platform/qcom/camss/camss-vfe-170.c:422 vfe_halt() warn: ignoring unreachable code.
drivers/media/platform/qcom/camss/camss-vfe-gen1.c:26 vfe_gen1_halt() warn: ignoring unreachable code.

Regards,

	Hans

> 
> 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: Refactor 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     |  371 +++++
>  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      |  601 +++++++++
>  .../platform/qcom/camss/camss-csid-4-1.c      |  329 +++++
>  .../platform/qcom/camss/camss-csid-4-7.c      |  405 ++++++
>  .../platform/qcom/camss/camss-csid-gen1.h     |   27 +
>  .../platform/qcom/camss/camss-csid-gen2.h     |   39 +
>  .../media/platform/qcom/camss/camss-csid.c    |  621 +--------
>  .../media/platform/qcom/camss/camss-csid.h    |  164 ++-
>  .../qcom/camss/camss-csiphy-3ph-1-0.c         |  179 ++-
>  .../media/platform/qcom/camss/camss-csiphy.c  |   66 +-
>  .../media/platform/qcom/camss/camss-ispif.c   |  119 +-
>  .../media/platform/qcom/camss/camss-ispif.h   |    3 +-
>  .../media/platform/qcom/camss/camss-vfe-170.c |  806 +++++++++++
>  .../media/platform/qcom/camss/camss-vfe-4-1.c |  144 +-
>  .../media/platform/qcom/camss/camss-vfe-4-7.c |  277 ++--
>  .../media/platform/qcom/camss/camss-vfe-4-8.c | 1200 +++++++++++++++++
>  .../platform/qcom/camss/camss-vfe-gen1.c      |  763 +++++++++++
>  .../platform/qcom/camss/camss-vfe-gen1.h      |  110 ++
>  drivers/media/platform/qcom/camss/camss-vfe.c |  847 +-----------
>  drivers/media/platform/qcom/camss/camss-vfe.h |  129 +-
>  .../media/platform/qcom/camss/camss-video.c   |   52 +
>  drivers/media/platform/qcom/camss/camss.c     |  407 +++++-
>  drivers/media/platform/qcom/camss/camss.h     |   15 +-
>  31 files changed, 7069 insertions(+), 2048 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-csid-gen1.h
>  create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen2.h
>  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
>