[PULL V3][SRU][Jammy] Support Intel IPU6 MIPI camera on Alder Lake platforms

Andrea Righi andrea.righi at canonical.com
Thu Mar 24 12:43:04 UTC 2022


Overall, this is quite a big patch... If we decide to accept this we
need to maintain it for many years. Good thing is that it's mostly
self-contained, it doesn't touch other subsystems in the kernel and the
code seems to be written pretty well.

I have a few concerns, comments below.

On Mon, Mar 14, 2022 at 10:36:30PM +0800, You-Sheng Yang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1955383
> 
> [Impact]
> 
> To support Intel IPU6 MIPI camera on Alder Lake platforms.
> 
> [Fix]
> 
> Initial support for Intel IPU6 MIPI camera on Tiger Lake platforms has
> been addressed by bug 1921345 and 1939539. They are backported from
> https://github.com/intel/ipu6-drivers.
> 
> Further works to enable IPU6 camera on Alder Lake platforms depend on a
> few more fixes from same ipu6-drivers repository, as well as an extra
> https://github.com/intel/ivsc-driver for Intel Vision Sensing
> Controller(IVSC).
> 
> [Test Case]
> 
> This depends on an integral of enablement components inclusive of the
> kernel drivers that are being proposed, firmware, updates for the
> userspace camera hardware abstration layer library and a gstreamer
> element as what we have for Tiger Lake platforms.
> 
> [Where problems could occur]
> 
> It's confirmed Intel IPU6 MIPI camera doesn't support suspend at
> streaming.
> 
> On Jammy, while intel_iommu is turned on in bug 1951440, it breaks
> current IPU6 driver implementation and can be worked-around with
> intel_iommu=off temporarily. A follow-up bug 1958004 has been filed.
> 
> Besides, since UBSAN is also on since Ubuntu 5.15 kernels, multiple
> warnings have been detected and filed as bug 1958006. No function
> impact observed so far.

UBSAN warnings can be problematic, in LP: #1958006 I see some
array-index-out-of-bounds, shift-out-of-bouds, .. these are scary, they
might be false positives, but if they're not they may introduce nasty
security-related bugs.

Do you think it's possible to get them fixed before applying this patch
set?

> 
> [Other Info]
> 
> Before this, we did not submit IPU6 driver for generic kernel due to
> the lack of a solid commitment from the vendor to upstream this
> before kernel camera API is out. There is still no such commitment,
> but during the process we've been able to address version migration
> issues and we would continue to do so. We're getting to have the
> confidence that it's possible to maintain and share the support for
> generic Ubuntu users.
> 
> Fixes for oem-5.14 have been merged, so here we nominate for Jammy
> only. This patchset is almost identical to that for oem-5.14 except
> one additioanl fix for 5.15 kernel API change in "UBUNTU: SAUCE: Fix
> build error for kernel 5.15".
> 
> V2: no change rebase only.
> V3: build only on amd64 and add config annotations

...

> diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile
> index 0b4236c4db49..608acd574921 100644
> --- a/drivers/media/pci/intel/Makefile
> +++ b/drivers/media/pci/intel/Makefile
> @@ -3,4 +3,13 @@
>  # Makefile for the IPU3 cio2 and ImGU drivers
>  #
>  
> -obj-y	+= ipu3/
> +# force check the compile warning to make sure zero warnings
> +# note we may have build issue when gcc upgraded.
> +subdir-ccflags-y := -Wall -Wextra
> +subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
> +subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
> +subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> +subdir-ccflags-$(CONFIG_VIDEO_INTEL_IPU_WERROR) += -Werror

Hm.. this looks bad. Why do we need to hide these warnings?

For the rest of the code it's pretty hard to do a full review, it's
really a lot, I'll take a quick look later and send more comments if I
spot any other potential issues.

For now I think the most critical issue that I see is about the UBSAN
warnings.

Thanks,
-Andrea



More information about the kernel-team mailing list