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@xxxxxxxxx wrote: > > From: Bingbu Cao <bingbu.cao@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > --- > > 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. -- Kind regards, Sakari Ailus
