[v3,00/15] Intel IPU6 and IPU6 input system drivers

Message ID	20240111065531.2418836-1-bingbu.cao@intel.com (mailing list archive)
Headers	From: bingbu.cao@intel.com To: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com, laurent.pinchart@ideasonboard.com, andriy.shevchenko@linux.intel.com, hdegoede@redhat.com Cc: ilpo.jarvinen@linux.intel.com, claus.stovgaard@gmail.com, tomi.valkeinen@ideasonboard.com, tfiga@chromium.org, senozhatsky@chromium.org, andreaskleist@gmail.com, bingbu.cao@intel.com, bingbu.cao@linux.intel.com, tian.shu.qiu@intel.com, hongju.wang@intel.com Subject: [PATCH v3 00/15] Intel IPU6 and IPU6 input system drivers Date: Thu, 11 Jan 2024 14:55:14 +0800 Message-ID: <20240111065531.2418836-1-bingbu.cao@intel.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit

Message ID

20240111065531.2418836-1-bingbu.cao@intel.com (mailing list archive)

Headers

From: bingbu.cao@intel.com
To: linux-media@vger.kernel.org,
	sakari.ailus@linux.intel.com,
	laurent.pinchart@ideasonboard.com,
	andriy.shevchenko@linux.intel.com,
	hdegoede@redhat.com
Cc: ilpo.jarvinen@linux.intel.com,
	claus.stovgaard@gmail.com,
	tomi.valkeinen@ideasonboard.com,
	tfiga@chromium.org,
	senozhatsky@chromium.org,
	andreaskleist@gmail.com,
	bingbu.cao@intel.com,
	bingbu.cao@linux.intel.com,
	tian.shu.qiu@intel.com,
	hongju.wang@intel.com
Subject: [PATCH v3 00/15] Intel IPU6 and IPU6 input system drivers
Date: Thu, 11 Jan 2024 14:55:14 +0800
Message-ID: <20240111065531.2418836-1-bingbu.cao@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Message

Bingbu Cao Jan. 11, 2024, 6:55 a.m. UTC

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

v1 -> v2:
  - Add multiplexed streams support
  - Use auxiliary bus to register IPU6 devices
  - Add IPU6 hardware and driver overview documentation
  - Updata IPU6 admin-guide documentation
  - Update number of source pads and video nodes to support
    multiplexed streams

TODOs:
  - Add firmware CSI2 lanes configuration verification

Bingbu Cao (17):
  media: intel/ipu6: add Intel IPU6 PCI device driver
  media: intel/ipu6: add IPU auxiliary devices
  media: intel/ipu6: add IPU6 buttress interface driver
  media: intel/ipu6: CPD parsing for get firmware components
  media: intel/ipu6: add IPU6 DMA mapping API and MMU table
  media: intel/ipu6: add syscom interfaces between firmware and driver
  media: intel/ipu6: input system ABI between firmware and driver
  media: intel/ipu6: add IPU6 CSI2 receiver v4l2 sub-device
  media: intel/ipu6: add the CSI2 DPHY implementation
  media: intel/ipu6: add input system driver
  media: intel/ipu6: input system video capture nodes
  media: add Kconfig and Makefile for IPU6
  MAINTAINERS: add maintainers for Intel IPU6 input system driver
  Documentation: add Intel IPU6 ISYS driver admin-guide doc
  Documentation: add documentation of Intel IPU6 driver and hardware
    overview
  media: ipu6/isys: support line-based metadata capture support
  media: ipu6/isys: support new v4l2 subdev state APIs

 Documentation/admin-guide/media/ipu6-isys.rst |  158 ++
 .../admin-guide/media/ipu6_isys_graph.svg     |  174 ++
 .../admin-guide/media/v4l-drivers.rst         |    1 +
 .../driver-api/media/drivers/index.rst        |    1 +
 .../driver-api/media/drivers/ipu6.rst         |  205 +++
 MAINTAINERS                                   |   10 +
 drivers/media/pci/intel/Kconfig               |    1 +
 drivers/media/pci/intel/Makefile              |    1 +
 drivers/media/pci/intel/ipu6/Kconfig          |   17 +
 drivers/media/pci/intel/ipu6/Makefile         |   23 +
 drivers/media/pci/intel/ipu6/ipu6-bus.c       |  165 ++
 drivers/media/pci/intel/ipu6/ipu6-bus.h       |   58 +
 drivers/media/pci/intel/ipu6/ipu6-buttress.c  |  912 +++++++++++
 drivers/media/pci/intel/ipu6/ipu6-buttress.h  |  102 ++
 drivers/media/pci/intel/ipu6/ipu6-cpd.c       |  362 +++++
 drivers/media/pci/intel/ipu6/ipu6-cpd.h       |  105 ++
 drivers/media/pci/intel/ipu6/ipu6-dma.c       |  502 ++++++
 drivers/media/pci/intel/ipu6/ipu6-dma.h       |   19 +
 drivers/media/pci/intel/ipu6/ipu6-fw-com.c    |  413 +++++
 drivers/media/pci/intel/ipu6/ipu6-fw-com.h    |   47 +
 drivers/media/pci/intel/ipu6/ipu6-fw-isys.c   |  487 ++++++
 drivers/media/pci/intel/ipu6/ipu6-fw-isys.h   |  573 +++++++
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c |  669 ++++++++
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h |   81 +
 .../media/pci/intel/ipu6/ipu6-isys-dwc-phy.c  |  536 +++++++
 .../media/pci/intel/ipu6/ipu6-isys-jsl-phy.c  |  242 +++
 .../media/pci/intel/ipu6/ipu6-isys-mcd-phy.c  |  720 +++++++++
 .../media/pci/intel/ipu6/ipu6-isys-queue.c    |  833 ++++++++++
 .../media/pci/intel/ipu6/ipu6-isys-queue.h    |   76 +
 .../media/pci/intel/ipu6/ipu6-isys-subdev.c   |  389 +++++
 .../media/pci/intel/ipu6/ipu6-isys-subdev.h   |   59 +
 .../media/pci/intel/ipu6/ipu6-isys-video.c    | 1399 +++++++++++++++++
 .../media/pci/intel/ipu6/ipu6-isys-video.h    |  141 ++
 drivers/media/pci/intel/ipu6/ipu6-isys.c      | 1353 ++++++++++++++++
 drivers/media/pci/intel/ipu6/ipu6-isys.h      |  207 +++
 drivers/media/pci/intel/ipu6/ipu6-mmu.c       |  845 ++++++++++
 drivers/media/pci/intel/ipu6/ipu6-mmu.h       |   73 +
 .../intel/ipu6/ipu6-platform-buttress-regs.h  |  232 +++
 .../intel/ipu6/ipu6-platform-isys-csi2-reg.h  |  189 +++
 .../media/pci/intel/ipu6/ipu6-platform-regs.h |  179 +++
 drivers/media/pci/intel/ipu6/ipu6.c           |  966 ++++++++++++
 drivers/media/pci/intel/ipu6/ipu6.h           |  356 +++++
 42 files changed, 13881 insertions(+)
 create mode 100644 Documentation/admin-guide/media/ipu6-isys.rst
 create mode 100644 Documentation/admin-guide/media/ipu6_isys_graph.svg
 create mode 100644 Documentation/driver-api/media/drivers/ipu6.rst
 create mode 100644 drivers/media/pci/intel/ipu6/Kconfig
 create mode 100644 drivers/media/pci/intel/ipu6/Makefile
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-bus.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-bus.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-buttress.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-buttress.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-cpd.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-cpd.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-dma.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-dma.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-isys.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-jsl-phy.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-mcd-phy.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-video.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-video.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-mmu.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-mmu.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-buttress-regs.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-isys-csi2-reg.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-regs.h
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6.c
 create mode 100644 drivers/media/pci/intel/ipu6/ipu6.h

Comments

Hans de Goede Jan. 16, 2024, 4:12 p.m. UTC | #1

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 ?

I would be happy to test any patches to fix this.

Regards,

Hans

Laurent Pinchart Jan. 16, 2024, 4:18 p.m. UTC | #2

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).

> I would be happy to test any patches to fix this.

Hans de Goede Jan. 16, 2024, 4:38 p.m. UTC | #3

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'll just use media_stage/master + this series minus patch 16 for now.

Regards,

Hans


> 
>> I would be happy to test any patches to fix this.
>

Laurent Pinchart Jan. 16, 2024, 4:42 p.m. UTC | #4

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.

Hans de Goede Jan. 16, 2024, 4:43 p.m. UTC | #5

Hi,

On 1/16/24 17:42, Laurent Pinchart wrote:
> 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.

Sounds good, thanks.

Regards,

Hans

Sakari Ailus Jan. 16, 2024, 4:54 p.m. UTC | #6

Hi Hans,

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 ?

That's the intention when it comes to the kernel APIs indeed.

I wonder how the simple pipeline handler works with this, does it try to
configure both streams all the way from the internal source pad to the
video node? This will certainly fail without metadata support in the ISYS
driver. Just guessing the cause though. An extra stream from the source pad
won't fail pipeline validation.

I should be able to set up a system to test this, too.

> 
> I would be happy to test any patches to fix this.

Hans de Goede Jan. 16, 2024, 4:57 p.m. UTC | #7

Hi,

On 1/16/24 17:54, Sakari Ailus wrote:
> Hi Hans,
> 
> 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 ?
> 
> That's the intention when it comes to the kernel APIs indeed.
> 
> I wonder how the simple pipeline handler works with this, does it try to
> configure both streams all the way from the internal source pad to the
> video node? This will certainly fail without metadata support in the ISYS
> driver. Just guessing the cause though. An extra stream from the source pad
> won't fail pipeline validation.
> 
> I should be able to set up a system to test this, too.

I did include Bingbu's patch to add metadata to the isys driver for my
testing (and I also tried reverting just that patch). I think the issue
might be that libcamera already has some streams awareness based on
an older streams patch-set.

I'll retest when Laurent's branch for this is ready.

Regards,

Hans

Laurent Pinchart Jan. 16, 2024, 5:48 p.m. UTC | #8

On Tue, Jan 16, 2024 at 05:57:14PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 1/16/24 17:54, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > 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 ?
> > 
> > That's the intention when it comes to the kernel APIs indeed.
> > 
> > I wonder how the simple pipeline handler works with this, does it try to
> > configure both streams all the way from the internal source pad to the
> > video node? This will certainly fail without metadata support in the ISYS
> > driver. Just guessing the cause though. An extra stream from the source pad
> > won't fail pipeline validation.
> > 
> > I should be able to set up a system to test this, too.
> 
> I did include Bingbu's patch to add metadata to the isys driver for my
> testing (and I also tried reverting just that patch). I think the issue
> might be that libcamera already has some streams awareness based on
> an older streams patch-set.

Correct. It uses the G_ROUTING and S_ROUTING ioctls, and those have
chenged in the latest kernel patch series.

The simple pipeline handler doesn't configure routing as such, it gets
the default routes with G_ROUTING(TRY) if the device advertises routing
support, and calls S_ROUTING(ACTIVE) to reset the routes to the default.
It then uses the routes to walk the pipeline, but doesn't change them.

> I'll retest when Laurent's branch for this is ready.