Re: [PATCH v3 00/15] Intel IPU6 and IPU6 input system drivers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Jan 16, 2024 at 05:38:15PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 1/16/24 17:18, Laurent Pinchart wrote:
> > On Tue, Jan 16, 2024 at 05:12:50PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 1/11/24 07:55, bingbu.cao@intel.com wrote:
> >>> From: Bingbu Cao <bingbu.cao@intel.com>
> >>>
> >>> This patch series adds a driver for Intel IPU6 input system.
> >>> IPU6 is the sixth generation of Imaging Processing Unit, it is a PCI
> >>> device which can be found in some Intel Client Platforms. User can use
> >>> IPU6 to capture images from MIPI camera sensors.
> >>>
> >>> IPU6 has its own firmware which exposes ABIs to driver, and communicates
> >>> with CSE to do firmware authentication. IPU6 has its MMU hardware, so
> >>> the driver sets up a page table to allow IPU6 DMA to access the system
> >>> memory.
> >>>
> >>> IPU6 input system driver uses MC and V4L2 sub-device APIs besides V4L2.
> >>> ---
> >>> v2 -> v3:
> >>>   - Add line-based metadata capture support
> >>>   - Fix header files inclusion issues
> >>>   - Fix the CSI2 timing calculation
> >>>   - Fix crash when remove module during streaming
> >>>   - Remove some unused code
> >>>   - Use cross-referencing links in documentation
> >>>   - Update Makefile to use ":=" for objects
> >>>   - Fix several bugs and coding style issues
> >>
> >> So I've given this version a try on a Lenovo X1 yoga gen 8 with ov2740
> >> sensor using the ongoing libcamera SoftISP work + this small patch
> >> to enable the SoftISP on IPU6 :
> >>
> >> https://github.com/jwrdegoede/libcamera/commit/3172f3703cf7076390fbf86c3b43e388c2422b31
> >>
> >> things work fine when using patch 1-15 + 17 on top of 6.7, note
> >> I'm skipping patch 16 ("media: ipu6/isys: support line-based
> >> metadata capture support")" here!
> >>
> >> However when I instead apply the whole series on top of:
> >> https://git.linuxtv.org/sailus/media_tree.git/log/?h=metadata
> >>
> >> Then things stop working, with the following errors
> >> (I added extra error logging to figure out in which syscall
> >> resetRoutingTable() fails and made libcamera ignore the routing
> >> errors):
> >>
> >> [2:02:04.466310686] [8943] ERROR SimplePipeline simple.cpp:1443 GetRouting() failed -25
> >> [2:02:04.466315975] [8943] ERROR SimplePipeline simple.cpp:1574 Failed to reset routes for /dev/v4l-subdev1: Inappropriate ioctl for device
> >> [2:02:04.466366331] [8943] ERROR SimplePipeline simple.cpp:1443 GetRouting() failed -25
> >> [2:02:04.466370025] [8943] ERROR SimplePipeline simple.cpp:1574 Failed to reset routes for /dev/v4l-subdev4: Inappropriate ioctl for device
> >>  	[2:03:32.334708887] [8929]  INFO Camera camera.cpp:1183 configuring streams: (0) 1924x1088-BGR888
> >> [2:03:32.335129023] [8943] ERROR SimplePipeline simple.cpp:1205 Unable to configure capture in 1932x1092-BA10 (got 0x0-@...)
> >> Failed to configure camera.
> >>
> >> I was sorta assuming that the new metadata-stream support would
> >> be backwards compatible for userspace without support for this,
> >> so I think this is a bug ?
> > 
> > The ABI has changed, it's not stable in mainline yet (which is why it
> > requires a kernel code change to enable it, see
> > https://git.linuxtv.org/sailus/media_tree.git/commit/?h=metadata&id=cc8be69a5c04b71bbf92fa26633ece13305ca451).
> 
> Thanks. So I reverted this patch to have the streams subdev API disabled by default again,
> after this the resetRouting errors are gone, but things still fail with:
> 
> [2:03:32.335129023] [8943] ERROR SimplePipeline simple.cpp:1205 Unable to configure capture in 1932x1092-BA10 (got 0x0-@...)
> Failed to configure camera.
> 
> When using Sakari's metadata branch and including patch 16 from this series.

I'm working on updating libcamera to the latest routing API, I'll ask
you to retest with the pipeline handler if you can when I'll be done.

> I'll just use media_stage/master + this series minus patch 16 for now.
> 
> >> I would be happy to test any patches to fix this.

-- 
Regards,

Laurent Pinchart