| Message ID | 20230901155732.252436-1-alain.volmat@foss.st.com |
|---|---|
| Headers | show |
| Series | Add support for DCMIPP camera interface of STMicroelectronics STM32 SoC series | expand |
On Fri, Sep 22, 2023 at 06:02:27PM +0200, Alain Volmat wrote: > On Tue, Sep 05, 2023 at 12:02:58PM +0300, Laurent Pinchart wrote: > > On Fri, Sep 01, 2023 at 05:57:23PM +0200, Alain Volmat wrote: > > > From: Hugues Fruchet <hugues.fruchet@foss.st.com> > > > > > > Add dcmipp support to STM32MP135. > > > > > > Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com> > > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > > > --- > > > arch/arm/boot/dts/st/stm32mp135.dtsi | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/st/stm32mp135.dtsi b/arch/arm/boot/dts/st/stm32mp135.dtsi > > > index abf2acd37b4e..beee9ec7ed0d 100644 > > > --- a/arch/arm/boot/dts/st/stm32mp135.dtsi > > > +++ b/arch/arm/boot/dts/st/stm32mp135.dtsi > > > @@ -8,5 +8,13 @@ > > > > > > / { > > > soc { > > > + dcmipp: dcmipp@5a000000 { > > > + compatible = "st,stm32mp13-dcmipp"; > > > + reg = <0x5a000000 0x400>; > > > + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>; > > > + resets = <&rcc DCMIPP_R>; > > > + clocks = <&rcc DCMIPP_K>; > > > + status = "disabled"; > > > > This needs a port, as it's marked as required in the bindings. You can > > leave the endpoint out. > > I first agreed with your comment but, having done the check (make > CHECK_DTBS=y ...) this doesn't seem to be required because the dcmipp > node is kept disabled within our dtsi. Interesting. > (it is later on only enabled in dts file which as well have the port > property). > Indeed, to check this I changed it to okay and DTC_CHK complained about > missing port property. > > Hence, I'd think that port doesn't have to be added in this dtsi file. > Would you agree with that ? I still think the port belongs here, as it's an intrinsic property of the dcmipp, not a property of the board. Does it cause any issue to add a port in the .dtsi ? > > With this fixed, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + }; > > > }; > > > };
Hi Laurent, On Fri, Sep 22, 2023 at 07:08:18PM +0300, Laurent Pinchart wrote: > On Fri, Sep 22, 2023 at 06:02:27PM +0200, Alain Volmat wrote: > > On Tue, Sep 05, 2023 at 12:02:58PM +0300, Laurent Pinchart wrote: > > > On Fri, Sep 01, 2023 at 05:57:23PM +0200, Alain Volmat wrote: > > > > From: Hugues Fruchet <hugues.fruchet@foss.st.com> > > > > > > > > Add dcmipp support to STM32MP135. > > > > > > > > Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com> > > > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > > > > --- > > > > arch/arm/boot/dts/st/stm32mp135.dtsi | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/st/stm32mp135.dtsi b/arch/arm/boot/dts/st/stm32mp135.dtsi > > > > index abf2acd37b4e..beee9ec7ed0d 100644 > > > > --- a/arch/arm/boot/dts/st/stm32mp135.dtsi > > > > +++ b/arch/arm/boot/dts/st/stm32mp135.dtsi > > > > @@ -8,5 +8,13 @@ > > > > > > > > / { > > > > soc { > > > > + dcmipp: dcmipp@5a000000 { > > > > + compatible = "st,stm32mp13-dcmipp"; > > > > + reg = <0x5a000000 0x400>; > > > > + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>; > > > > + resets = <&rcc DCMIPP_R>; > > > > + clocks = <&rcc DCMIPP_K>; > > > > + status = "disabled"; > > > > > > This needs a port, as it's marked as required in the bindings. You can > > > leave the endpoint out. > > > > I first agreed with your comment but, having done the check (make > > CHECK_DTBS=y ...) this doesn't seem to be required because the dcmipp > > node is kept disabled within our dtsi. > > Interesting. > > > (it is later on only enabled in dts file which as well have the port > > property). > > Indeed, to check this I changed it to okay and DTC_CHK complained about > > missing port property. > > > > Hence, I'd think that port doesn't have to be added in this dtsi file. > > Would you agree with that ? > > I still think the port belongs here, as it's an intrinsic property of > the dcmipp, not a property of the board. Does it cause any issue to add > a port in the .dtsi ? I agree that the port refers more to the SoC (hence dtsi) rather than the board (hence dts), however I am wondering if this is really something usually done. I had a look at other dtsi with node related to similar kind of devices and it seems to me that there is no such case of a dtsi with a port having nothing in it. Did I missed something ? > > > > With this fixed, > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > > + }; > > > > }; > > > > }; > > -- > Regards, > > Laurent Pinchart
This patchset introduces support for Digital Camera Memory Interface Pixel Processor (DCMIPP) of STMicroelectronics STM32 SoC series. This initial support implements a single capture pipe allowing RGB565, YUV, Y, RAW8 and JPEG capture with frame skipping, prescaling and cropping. DCMIPP is exposed through 3 subdevices: - dcmipp_dump_parallel: parallel interface handling - dcmipp_dump_postproc: frame skipping, prescaling and cropping control - dcmipp_dump_capture: video device capture node v3: - Have same To & Cc in all patches emails of the serie so that everybody has coherent view of the serie - bindings: correct wording, clock-names & label removal - driver: replace of_graph call with fwnode_graph - driver: use defined bus-type when calling v4l2_fwnode_endpoint_parse - driver: remove clock name - dtsi: remove clock-names property v2: - removal of pclk-max-frequency from yaml example dts - codying-style fixes - correction in enum functions (format, mbus, frame_size ...) handling - drop of v4l2_pipeline_pm_ calls, and specific open/close handler of vdev - video dev s_stream handling updated to call s_stream of remote subdev instead of loop until sensor subdev - code update following media_pipeline & v4l2_async_ api changes since v1 - removal of IP reset call upon error - removal of link_validate handlers - addition of V4L2_CAP_IO_MC device_caps - removal of the frame skip control for the time being, will be added back in another commit once control method will be agreed - change byteproc entity type to MEDIA_ENT_F_PROC_VIDEO_SCALER - various fixes from Dan & Sakari remarks Alain Volmat (2): dt-bindings: media: add bindings for stm32 dcmipp media: MAINTAINERS: add entry for STM32 DCMIPP driver Hugues Fruchet (3): media: stm32-dcmipp: STM32 DCMIPP camera interface driver ARM: dts: stm32: add dcmipp support to stm32mp135 ARM: multi_v7_defconfig: enable STM32 DCMIPP media support .../bindings/media/st,stm32-dcmipp.yaml | 89 ++ MAINTAINERS | 9 + arch/arm/boot/dts/st/stm32mp135.dtsi | 8 + arch/arm/configs/multi_v7_defconfig | 1 + drivers/media/platform/st/stm32/Kconfig | 15 + drivers/media/platform/st/stm32/Makefile | 1 + .../platform/st/stm32/stm32-dcmipp/Makefile | 5 + .../st/stm32/stm32-dcmipp/dcmipp-bytecap.c | 1040 +++++++++++++++++ .../st/stm32/stm32-dcmipp/dcmipp-byteproc.c | 692 +++++++++++ .../st/stm32/stm32-dcmipp/dcmipp-common.c | 105 ++ .../st/stm32/stm32-dcmipp/dcmipp-common.h | 227 ++++ .../st/stm32/stm32-dcmipp/dcmipp-core.c | 675 +++++++++++ .../st/stm32/stm32-dcmipp/dcmipp-parallel.c | 523 +++++++++ 13 files changed, 3390 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/st,stm32-dcmipp.yaml create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/Makefile create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-bytecap.c create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-byteproc.c create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-common.c create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-common.h create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c create mode 100644 drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-parallel.c