Message ID | 20230413100429.919622-13-bingbu.cao@intel.com |
---|---|
State | New |
Headers | show |
Series | Intel IPU6 and IPU6 input system drivers | expand |
Hi Bingbu, Thank you for the patch. I'm happy to see this series landing on the mailing list. I'm starting the review with the easy pieces :-) On Thu, Apr 13, 2023 at 06:04:27PM +0800, bingbu.cao@intel.com wrote: > From: Bingbu Cao <bingbu.cao@intel.com> > > Add IPU6 support in Kconfig and Makefile, with this patch you can > build the Intel IPU6 and input system modules by select the > CONFIG_VIDEO_INTEL_IPU6 in config. > > Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> > --- > drivers/media/pci/Kconfig | 1 + > drivers/media/pci/intel/Makefile | 3 ++- > drivers/media/pci/intel/ipu6/Kconfig | 15 +++++++++++++++ > drivers/media/pci/intel/ipu6/Makefile | 23 +++++++++++++++++++++++ > 4 files changed, 41 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/pci/intel/ipu6/Kconfig > create mode 100644 drivers/media/pci/intel/ipu6/Makefile > > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig > index 480194543d05..38fb484f5c8e 100644 > --- a/drivers/media/pci/Kconfig > +++ b/drivers/media/pci/Kconfig > @@ -74,6 +74,7 @@ config VIDEO_PCI_SKELETON > when developing new drivers. > > source "drivers/media/pci/intel/ipu3/Kconfig" > +source "drivers/media/pci/intel/ipu6/Kconfig" > > endif #MEDIA_PCI_SUPPORT > endif #PCI > diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile > index 0b4236c4db49..de2b73fef890 100644 > --- a/drivers/media/pci/intel/Makefile > +++ b/drivers/media/pci/intel/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > # > -# Makefile for the IPU3 cio2 and ImGU drivers > +# Makefile for the Intel IPU drivers > # > > obj-y += ipu3/ > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += ipu6/ > diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig > new file mode 100644 > index 000000000000..c88ef2f40f6d > --- /dev/null > +++ b/drivers/media/pci/intel/ipu6/Kconfig > @@ -0,0 +1,15 @@ > +config VIDEO_INTEL_IPU6 As there will (hopefully) be a driver for the processing system in the future, should this Kconfig symbol be named with a reference to the input system already (and the help text be updated accordingly) ? The name "IPU6" covers both. Or do you envision that we would have a single Kconfig symbol to select both drivers ? > + tristate "Intel IPU6 driver" > + depends on ACPI || COMPILE_TEST > + depends on MEDIA_SUPPORT > + depends on MEDIA_PCI_SUPPORT > + depends on X86_64 Do you use any x86-64 API, or could this be depends on X86_64 || COMPILE_TEST ? > + select IOMMU_IOVA > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_FWNODE > + help > + This is the 6th Gen Intel Image Processing Unit, found in Intel SoCs > + and used for capturing images and video from camera sensors. > + > + To compile this driver, say Y here! It contains 2 modules - > + intel_ipu6 and intel_ipu6_isys. > diff --git a/drivers/media/pci/intel/ipu6/Makefile b/drivers/media/pci/intel/ipu6/Makefile > new file mode 100644 > index 000000000000..6a6339c84ef4 > --- /dev/null > +++ b/drivers/media/pci/intel/ipu6/Makefile > @@ -0,0 +1,23 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +intel-ipu6-objs += ipu6.o \ > + ipu6-bus.o \ > + ipu6-dma.o \ > + ipu6-mmu.o \ > + ipu6-buttress.o \ > + ipu6-cpd.o \ > + ipu6-fw-com.o > + > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6.o > + > +intel-ipu6-isys-objs += ipu6-isys.o \ > + ipu6-isys-csi2.o \ > + ipu6-fw-isys.o \ > + ipu6-isys-video.o \ > + ipu6-isys-queue.o \ > + ipu6-isys-subdev.o \ > + ipu6-isys-mcd-phy.o \ > + ipu6-isys-jsl-phy.o \ > + ipu6-isys-dwc-phy.o > + > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6-isys.o
Hi Laurent, On Thu, Apr 20, 2023 at 05:23:20PM +0300, Laurent Pinchart wrote: > Hi Bingbu, > > Thank you for the patch. I'm happy to see this series landing on the > mailing list. I'm starting the review with the easy pieces :-) > > On Thu, Apr 13, 2023 at 06:04:27PM +0800, bingbu.cao@intel.com wrote: > > From: Bingbu Cao <bingbu.cao@intel.com> > > > > Add IPU6 support in Kconfig and Makefile, with this patch you can > > build the Intel IPU6 and input system modules by select the > > CONFIG_VIDEO_INTEL_IPU6 in config. > > > > Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> > > --- > > drivers/media/pci/Kconfig | 1 + > > drivers/media/pci/intel/Makefile | 3 ++- > > drivers/media/pci/intel/ipu6/Kconfig | 15 +++++++++++++++ > > drivers/media/pci/intel/ipu6/Makefile | 23 +++++++++++++++++++++++ > > 4 files changed, 41 insertions(+), 1 deletion(-) > > create mode 100644 drivers/media/pci/intel/ipu6/Kconfig > > create mode 100644 drivers/media/pci/intel/ipu6/Makefile > > > > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig > > index 480194543d05..38fb484f5c8e 100644 > > --- a/drivers/media/pci/Kconfig > > +++ b/drivers/media/pci/Kconfig > > @@ -74,6 +74,7 @@ config VIDEO_PCI_SKELETON > > when developing new drivers. > > > > source "drivers/media/pci/intel/ipu3/Kconfig" > > +source "drivers/media/pci/intel/ipu6/Kconfig" > > > > endif #MEDIA_PCI_SUPPORT > > endif #PCI > > diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile > > index 0b4236c4db49..de2b73fef890 100644 > > --- a/drivers/media/pci/intel/Makefile > > +++ b/drivers/media/pci/intel/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > # > > -# Makefile for the IPU3 cio2 and ImGU drivers > > +# Makefile for the Intel IPU drivers > > # > > > > obj-y += ipu3/ > > +obj-$(CONFIG_VIDEO_INTEL_IPU6) += ipu6/ > > diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig > > new file mode 100644 > > index 000000000000..c88ef2f40f6d > > --- /dev/null > > +++ b/drivers/media/pci/intel/ipu6/Kconfig > > @@ -0,0 +1,15 @@ > > +config VIDEO_INTEL_IPU6 > > As there will (hopefully) be a driver for the processing system in the > future, should this Kconfig symbol be named with a reference to the > input system already (and the help text be updated accordingly) ? The > name "IPU6" covers both. Or do you envision that we would have a single > Kconfig symbol to select both drivers ? I'm fine with just one option. Much of the code is shared by both drivers. I don't object adding an option for both (starting with ISYS) though. > > > + tristate "Intel IPU6 driver" > > + depends on ACPI || COMPILE_TEST > > + depends on MEDIA_SUPPORT > > + depends on MEDIA_PCI_SUPPORT > > + depends on X86_64 > > Do you use any x86-64 API, or could this be > > depends on X86_64 || COMPILE_TEST > > ? The driver uses cache related APIs that are available on x86 only. I think in the long run we should have better non-coherent memory support for x86 API-wise but I don't think this driver should wait for that. But once we do, this dependency can be removed.
diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig index 480194543d05..38fb484f5c8e 100644 --- a/drivers/media/pci/Kconfig +++ b/drivers/media/pci/Kconfig @@ -74,6 +74,7 @@ config VIDEO_PCI_SKELETON when developing new drivers. source "drivers/media/pci/intel/ipu3/Kconfig" +source "drivers/media/pci/intel/ipu6/Kconfig" endif #MEDIA_PCI_SUPPORT endif #PCI diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile index 0b4236c4db49..de2b73fef890 100644 --- a/drivers/media/pci/intel/Makefile +++ b/drivers/media/pci/intel/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only # -# Makefile for the IPU3 cio2 and ImGU drivers +# Makefile for the Intel IPU drivers # obj-y += ipu3/ +obj-$(CONFIG_VIDEO_INTEL_IPU6) += ipu6/ diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig new file mode 100644 index 000000000000..c88ef2f40f6d --- /dev/null +++ b/drivers/media/pci/intel/ipu6/Kconfig @@ -0,0 +1,15 @@ +config VIDEO_INTEL_IPU6 + tristate "Intel IPU6 driver" + depends on ACPI || COMPILE_TEST + depends on MEDIA_SUPPORT + depends on MEDIA_PCI_SUPPORT + depends on X86_64 + select IOMMU_IOVA + select VIDEOBUF2_DMA_CONTIG + select V4L2_FWNODE + help + This is the 6th Gen Intel Image Processing Unit, found in Intel SoCs + and used for capturing images and video from camera sensors. + + To compile this driver, say Y here! It contains 2 modules - + intel_ipu6 and intel_ipu6_isys. diff --git a/drivers/media/pci/intel/ipu6/Makefile b/drivers/media/pci/intel/ipu6/Makefile new file mode 100644 index 000000000000..6a6339c84ef4 --- /dev/null +++ b/drivers/media/pci/intel/ipu6/Makefile @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: GPL-2.0-only + +intel-ipu6-objs += ipu6.o \ + ipu6-bus.o \ + ipu6-dma.o \ + ipu6-mmu.o \ + ipu6-buttress.o \ + ipu6-cpd.o \ + ipu6-fw-com.o + +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6.o + +intel-ipu6-isys-objs += ipu6-isys.o \ + ipu6-isys-csi2.o \ + ipu6-fw-isys.o \ + ipu6-isys-video.o \ + ipu6-isys-queue.o \ + ipu6-isys-subdev.o \ + ipu6-isys-mcd-phy.o \ + ipu6-isys-jsl-phy.o \ + ipu6-isys-dwc-phy.o + +obj-$(CONFIG_VIDEO_INTEL_IPU6) += intel-ipu6-isys.o