mbox series

[v4,00/19] Intel IPU6 and IPU6 input system drivers

Message ID 20240416201105.781496-1-sakari.ailus@linux.intel.com (mailing list archive)
Headers show
Series Intel IPU6 and IPU6 input system drivers | expand

Message

Sakari Ailus April 16, 2024, 8:10 p.m. UTC
Hello everyone,

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.

I can now capture images from ov01a10 and ov2740 sensors (including
metadata from the latter).

The series applies on top of the metadata patchset
<URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.

---
since v3:

- Prepend the series with IPU bridge changes for more reliable
  IPU bridge initialisation.

- Split off the IPU6 PCI device ID table (due to the former change).

- Documentation improvements (mostly non-technical).

- Update copyright year.

- Remove unused struct ipu6_buttress_constraint and a few other unused
  definitions.

- Miscellaneous cleanups.

- Make functions static if they can be so.

- Merge watermark setup error handling bugfix from Hongju.

- Use media_pad_remote_pad_unique() instead of
  media_pad_remote_pad_first() in figuring out the remote sub-device.

- Determine the number of queues by counting the video nodes in the graph.

- Make the links from the external sub-devices to CSI-2 receivers enabled
  and immutable.

- Simplify determining CSI-2 port control base port offset.

- Create only as many links as needed between CSI-2 receivers and video
  nodes (one per video node).

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 (16):
  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: input system video nodes and buffer queues
  media: intel/ipu6: add the main input system driver
  media: intel/ipu6: add Kconfig and Makefile
  MAINTAINERS: add maintainers for Intel IPU6 input system driver
  media: intel/ipu6: support line-based metadata capture support
  Documentation: add Intel IPU6 ISYS driver admin-guide doc
  Documentation: add documentation of Intel IPU6 driver and hardware
    overview

Sakari Ailus (3):
  media: ipu6: Add PCI device table header
  media: ivsc: csi: Use IPU bridge
  media: Kconfig: Select MEDIA_CONTROLLER for VIDEO_V4L2_SUBDEV_API

 Documentation/admin-guide/media/ipu6-isys.rst |  161 ++
 .../admin-guide/media/ipu6_isys_graph.svg     |  548 +++++++
 .../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  |   92 ++
 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   |  568 +++++++
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c |  667 ++++++++
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h |   82 +
 .../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    |  824 ++++++++++
 .../media/pci/intel/ipu6/ipu6-isys-queue.h    |   78 +
 .../media/pci/intel/ipu6/ipu6-isys-subdev.c   |  408 +++++
 .../media/pci/intel/ipu6/ipu6-isys-subdev.h   |   59 +
 .../media/pci/intel/ipu6/ipu6-isys-video.c    | 1401 +++++++++++++++++
 .../media/pci/intel/ipu6/ipu6-isys-video.h    |  137 ++
 drivers/media/pci/intel/ipu6/ipu6-isys.c      | 1368 ++++++++++++++++
 drivers/media/pci/intel/ipu6/ipu6-isys.h      |  206 +++
 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  |  226 +++
 .../intel/ipu6/ipu6-platform-isys-csi2-reg.h  |  172 ++
 .../media/pci/intel/ipu6/ipu6-platform-regs.h |  179 +++
 drivers/media/pci/intel/ipu6/ipu6.c           |  895 +++++++++++
 drivers/media/pci/intel/ipu6/ipu6.h           |  342 ++++
 drivers/media/pci/intel/ivsc/mei_csi.c        |   20 +-
 drivers/media/v4l2-core/Kconfig               |    3 +-
 include/media/ipu6-pci-table.h                |   28 +
 45 files changed, 14206 insertions(+), 3 deletions(-)
 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
 create mode 100644 include/media/ipu6-pci-table.h

Comments

Hans de Goede April 17, 2024, 7:56 a.m. UTC | #1
Hi Sakari,
On 4/16/24 10:10 PM, Sakari Ailus wrote:
> Hello everyone,
> 
> 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.
> 
> I can now capture images from ov01a10 and ov2740 sensors (including
> metadata from the latter).
> 
> The series applies on top of the metadata patchset
> <URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.

Thank you for the new version!

I assume that the posting of this new version means that you have solved
the stability issues where the laptop would freeze after sttreaming from
an ov2740 sensor with metadata once ?

What about the unrelated ov2740 driver issue where the sensor would not
always start streaming for which you temporarily disabled runtime pm
for the sensor as a workaround any progress on that ?

Do you have a git branch available with the metadata + this series
somewhere for easy testing ?  I would like to give this a test run on
my own IPU6 + ov2740 laptop.

Regards,

Hans






> 
> ---
> since v3:
> 
> - Prepend the series with IPU bridge changes for more reliable
>   IPU bridge initialisation.
> 
> - Split off the IPU6 PCI device ID table (due to the former change).
> 
> - Documentation improvements (mostly non-technical).
> 
> - Update copyright year.
> 
> - Remove unused struct ipu6_buttress_constraint and a few other unused
>   definitions.
> 
> - Miscellaneous cleanups.
> 
> - Make functions static if they can be so.
> 
> - Merge watermark setup error handling bugfix from Hongju.
> 
> - Use media_pad_remote_pad_unique() instead of
>   media_pad_remote_pad_first() in figuring out the remote sub-device.
> 
> - Determine the number of queues by counting the video nodes in the graph.
> 
> - Make the links from the external sub-devices to CSI-2 receivers enabled
>   and immutable.
> 
> - Simplify determining CSI-2 port control base port offset.
> 
> - Create only as many links as needed between CSI-2 receivers and video
>   nodes (one per video node).
> 
> 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 (16):
>   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: input system video nodes and buffer queues
>   media: intel/ipu6: add the main input system driver
>   media: intel/ipu6: add Kconfig and Makefile
>   MAINTAINERS: add maintainers for Intel IPU6 input system driver
>   media: intel/ipu6: support line-based metadata capture support
>   Documentation: add Intel IPU6 ISYS driver admin-guide doc
>   Documentation: add documentation of Intel IPU6 driver and hardware
>     overview
> 
> Sakari Ailus (3):
>   media: ipu6: Add PCI device table header
>   media: ivsc: csi: Use IPU bridge
>   media: Kconfig: Select MEDIA_CONTROLLER for VIDEO_V4L2_SUBDEV_API
> 
>  Documentation/admin-guide/media/ipu6-isys.rst |  161 ++
>  .../admin-guide/media/ipu6_isys_graph.svg     |  548 +++++++
>  .../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  |   92 ++
>  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   |  568 +++++++
>  drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c |  667 ++++++++
>  drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h |   82 +
>  .../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    |  824 ++++++++++
>  .../media/pci/intel/ipu6/ipu6-isys-queue.h    |   78 +
>  .../media/pci/intel/ipu6/ipu6-isys-subdev.c   |  408 +++++
>  .../media/pci/intel/ipu6/ipu6-isys-subdev.h   |   59 +
>  .../media/pci/intel/ipu6/ipu6-isys-video.c    | 1401 +++++++++++++++++
>  .../media/pci/intel/ipu6/ipu6-isys-video.h    |  137 ++
>  drivers/media/pci/intel/ipu6/ipu6-isys.c      | 1368 ++++++++++++++++
>  drivers/media/pci/intel/ipu6/ipu6-isys.h      |  206 +++
>  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  |  226 +++
>  .../intel/ipu6/ipu6-platform-isys-csi2-reg.h  |  172 ++
>  .../media/pci/intel/ipu6/ipu6-platform-regs.h |  179 +++
>  drivers/media/pci/intel/ipu6/ipu6.c           |  895 +++++++++++
>  drivers/media/pci/intel/ipu6/ipu6.h           |  342 ++++
>  drivers/media/pci/intel/ivsc/mei_csi.c        |   20 +-
>  drivers/media/v4l2-core/Kconfig               |    3 +-
>  include/media/ipu6-pci-table.h                |   28 +
>  45 files changed, 14206 insertions(+), 3 deletions(-)
>  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
>  create mode 100644 include/media/ipu6-pci-table.h
>
Sakari Ailus April 17, 2024, 8:34 a.m. UTC | #2
Hi Hans,

On Wed, Apr 17, 2024 at 09:56:40AM +0200, Hans de Goede wrote:
> Hi Sakari,
> On 4/16/24 10:10 PM, Sakari Ailus wrote:
> > Hello everyone,
> > 
> > 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.
> > 
> > I can now capture images from ov01a10 and ov2740 sensors (including
> > metadata from the latter).
> > 
> > The series applies on top of the metadata patchset
> > <URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.
> 
> Thank you for the new version!
> 
> I assume that the posting of this new version means that you have solved
> the stability issues where the laptop would freeze after sttreaming from
> an ov2740 sensor with metadata once ?
> 
> What about the unrelated ov2740 driver issue where the sensor would not
> always start streaming for which you temporarily disabled runtime pm
> for the sensor as a workaround any progress on that ?

I'm afraid these issues remain.

> 
> Do you have a git branch available with the metadata + this series
> somewhere for easy testing ?  I would like to give this a test run on
> my own IPU6 + ov2740 laptop.

Both of the sets can be found here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ipu6>
Hans de Goede April 17, 2024, 8:59 a.m. UTC | #3
Hi Sakari,

On 4/17/24 10:34 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Apr 17, 2024 at 09:56:40AM +0200, Hans de Goede wrote:
>> Hi Sakari,
>> On 4/16/24 10:10 PM, Sakari Ailus wrote:
>>> Hello everyone,
>>>
>>> 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.
>>>
>>> I can now capture images from ov01a10 and ov2740 sensors (including
>>> metadata from the latter).
>>>
>>> The series applies on top of the metadata patchset
>>> <URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.
>>
>> Thank you for the new version!
>>
>> I assume that the posting of this new version means that you have solved
>> the stability issues where the laptop would freeze after sttreaming from
>> an ov2740 sensor with metadata once ?
>>
>> What about the unrelated ov2740 driver issue where the sensor would not
>> always start streaming for which you temporarily disabled runtime pm
>> for the sensor as a workaround any progress on that ?
> 
> I'm afraid these issues remain.

You mean both issues remain? I'm not that worried about the runtime-pm ov2740
issue, but if the lockup after streaming issue also remains that is a lot
more worrying.

I've been running an older version of this series without the metadata
support and that is pretty rock solid, so this seems to be caused by
enabling metadata support.

AFAIK the current out of tree solution with partly closed-source
userspace stack does not use metadata right /

Do you know if the Windows stack uses metadata capture from the sensor?

If neither the existing out of tree Linux stack nor the Windows stack
is using metadata capture then chances are we are actually hitting
hw/firmware bugs here. This would not be the first time that the Linux
community tries to enable a hw-feature not used by the factory installed
OS for the hw and ends up failing miserably because the feature was
never fully tested and turns out to be full of bugs.

IMHO if we cannot get the stability issue fixed real soon it would
be best to move forward with this patch series without adding
the metadata support. So basically drop patch 17/19 .

>> Do you have a git branch available with the metadata + this series
>> somewhere for easy testing ?  I would like to give this a test run on
>> my own IPU6 + ov2740 laptop.
> 
> Both of the sets can be found here:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ipu6>

Thank you.

Regards,

Hans
Bingbu Cao April 17, 2024, 11:33 a.m. UTC | #4
Sakari,

Thanks for posting this new version.

On 4/17/24 4:59 PM, Hans de Goede wrote:
> Hi Sakari,
> 
> On 4/17/24 10:34 AM, Sakari Ailus wrote:
>> Hi Hans,
>>
>> On Wed, Apr 17, 2024 at 09:56:40AM +0200, Hans de Goede wrote:
>>> Hi Sakari,
>>> On 4/16/24 10:10 PM, Sakari Ailus wrote:
>>>> Hello everyone,
>>>>
>>>> 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.
>>>>
>>>> I can now capture images from ov01a10 and ov2740 sensors (including
>>>> metadata from the latter).
>>>>
>>>> The series applies on top of the metadata patchset
>>>> <URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.
>>>
>>> Thank you for the new version!
>>>
>>> I assume that the posting of this new version means that you have solved
>>> the stability issues where the laptop would freeze after sttreaming from
>>> an ov2740 sensor with metadata once ?
>>>
>>> What about the unrelated ov2740 driver issue where the sensor would not
>>> always start streaming for which you temporarily disabled runtime pm
>>> for the sensor as a workaround any progress on that ?
>>
>> I'm afraid these issues remain.
> 
> You mean both issues remain? I'm not that worried about the runtime-pm ov2740
> issue, but if the lockup after streaming issue also remains that is a lot
> more worrying.

Hans,

We did not observe this issue on the X1 Carbon Gen7 even the metadata
is enabled. However, the 1st frame captured by IPU6 ISYS is randomly
blank.

> 
> I've been running an older version of this series without the metadata
> support and that is pretty rock solid, so this seems to be caused by
> enabling metadata support.
> 
> AFAIK the current out of tree solution with partly closed-source
> userspace stack does not use metadata right /
> 
> Do you know if the Windows stack uses metadata capture from the sensor?

AFAIK, Windows stack doesn't enable the metadata for ov2740.
Very rare camera sensors enabled metadata both on Windows, Linux and
Chrome so far. However, I am trying to find another sensor instead of
ov2740 to test.


> 
> If neither the existing out of tree Linux stack nor the Windows stack
> is using metadata capture then chances are we are actually hitting
> hw/firmware bugs here. This would not be the first time that the Linux
> community tries to enable a hw-feature not used by the factory installed
> OS for the hw and ends up failing miserably because the feature was
> never fully tested and turns out to be full of bugs.
> 
> IMHO if we cannot get the stability issue fixed real soon it would
> be best to move forward with this patch series without adding
> the metadata support. So basically drop patch 17/19 .
> 
>>> Do you have a git branch available with the metadata + this series
>>> somewhere for easy testing ?  I would like to give this a test run on
>>> my own IPU6 + ov2740 laptop.

Thanks!

>>
>> Both of the sets can be found here:
>>
>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ipu6>
> 
> Thank you.
> 
> Regards,
> 
> Hans
> 
> 
> 
>
Sakari Ailus April 17, 2024, 12:29 p.m. UTC | #5
Hi Hans,

On Wed, Apr 17, 2024 at 10:59:37AM +0200, Hans de Goede wrote:
> Hi Sakari,
> 
> On 4/17/24 10:34 AM, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Wed, Apr 17, 2024 at 09:56:40AM +0200, Hans de Goede wrote:
> >> Hi Sakari,
> >> On 4/16/24 10:10 PM, Sakari Ailus wrote:
> >>> Hello everyone,
> >>>
> >>> 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.
> >>>
> >>> I can now capture images from ov01a10 and ov2740 sensors (including
> >>> metadata from the latter).
> >>>
> >>> The series applies on top of the metadata patchset
> >>> <URL:https://lore.kernel.org/linux-media/20240416193319.778192-1-sakari.ailus@linux.intel.com/T/#t>.
> >>
> >> Thank you for the new version!
> >>
> >> I assume that the posting of this new version means that you have solved
> >> the stability issues where the laptop would freeze after sttreaming from
> >> an ov2740 sensor with metadata once ?
> >>
> >> What about the unrelated ov2740 driver issue where the sensor would not
> >> always start streaming for which you temporarily disabled runtime pm
> >> for the sensor as a workaround any progress on that ?
> > 
> > I'm afraid these issues remain.
> 
> You mean both issues remain? I'm not that worried about the runtime-pm ov2740
> issue, but if the lockup after streaming issue also remains that is a lot
> more worrying.
> 
> I've been running an older version of this series without the metadata
> support and that is pretty rock solid, so this seems to be caused by
> enabling metadata support.
> 
> AFAIK the current out of tree solution with partly closed-source
> userspace stack does not use metadata right /
> 
> Do you know if the Windows stack uses metadata capture from the sensor?
> 
> If neither the existing out of tree Linux stack nor the Windows stack
> is using metadata capture then chances are we are actually hitting
> hw/firmware bugs here. This would not be the first time that the Linux

Interestingly, the same sensor works differently on different systems with
IPU6 and the same sensor.

We'll look into this but I don't know when I have more information.

> community tries to enable a hw-feature not used by the factory installed
> OS for the hw and ends up failing miserably because the feature was
> never fully tested and turns out to be full of bugs.
> 
> IMHO if we cannot get the stability issue fixed real soon it would
> be best to move forward with this patch series without adding
> the metadata support. So basically drop patch 17/19 .
> 
> >> Do you have a git branch available with the metadata + this series
> >> somewhere for easy testing ?  I would like to give this a test run on
> >> my own IPU6 + ov2740 laptop.
> > 
> > Both of the sets can be found here:
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ipu6>
> 
> Thank you.