Intel Vision Processing Unit base enabling part 1

From: mark gross <[email protected]>

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Mark Gross <[email protected]>

---
 Documentation/index.rst                  |   3 +-
 Documentation/vpu/index.rst              |  16 ++
 Documentation/vpu/vpu-stack-overview.rst | 267 +++++++++++++++++++++++
 3 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/vpu/index.rst
 create mode 100644 Documentation/vpu/vpu-stack-overview.rst

Documentation/index.rst +0 -0

index 57719744774c..0a2cc0204e8f 100644

Documentation/index.rst +1 -0

 .. The Linux Kernel documentation master file, created by

@@ -137,6 +137,7 @@ needed).
    misc-devices/index
    scheduler/index
    mhi/index
+   vpu/index
 
 Architecture-agnostic documentation
 -----------------------------------

Documentation/vpu/index.rst new file +0 -0

new file mode 100644
index 000000000000..7e290e048910

Documentation/vpu/index.rst new file +15 -0

+
+============================================
+Vision Processor Unit Documentation
+============================================
+
+This documentation contains information for the Intel VPU stack.
+
+.. class:: toc-title
+
+	   Table of contents
+
+.. toctree::
+   :maxdepth: 2
+
+   vpu-stack-overview

Documentation/vpu/vpu-stack-overview.rst new file +0 -0

new file mode 100644
index 000000000000..53c06a7d9a52

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+products.
+
+Keem Bay (KMB) is a Computer Vision AI processing SoC based on ARM A53 CPU that
+provides Edge neural network acceleration (inference) and includes a Vision
+Processing Unit (VPU) hardware.  The ARM CPU SubSystem (CPUSS) interfaces
+locally to the VPU and enables integration/interfacing with a remote host over
+PCIe or USB or Ethernet interfaces. The interface between the CPUSS and the VPU
+is implemented with HW FIFOs (Control) and coherent memory mapping (Data) such
+that zero copy processing can happen within the VPU.
+

Documentation/vpu/vpu-stack-overview.rst new file +6 -0

+from 0 to 1023.
+
+Channels are divided in two categories:
+
+* High-Speed (HS) channels, having IDs in the 0-9 range.
+* General-Purpose (GP) channels, having IDs in the 10-1023 range.

Documentation/vpu/vpu-stack-overview.rst new file +26 -0

+
+An IPC channel is full duplex: a pending operation from a certain channel does
+not block other operations on the same channel, regardless of their operation
+mode (blocking or non-blocking).
+
+Operation mode is individually selectable for each channel, per operation
+direction (read or write). All operations for that direction comply to
+selection.
+
+
+Keem Bay-VPU-IPC
+================
+
+This is the MMIO driver of the VPU IP block inside the SOC. It is a control
+driver mapping IPC channel communication to Xlink virtual channels.
+
+This driver provides the following functionality to other drivers in the
+communication stack:
+
+* VPU IP execution control (firmware load, start, reset)
+* VPU IP event notifications (device connected, device disconnected, WDT event)
+* VPU IP device status query (OFF, BUSY, READY, ERROR, RECOVERY)
+* Communication via the IPC protocol (wrapping the Keem Bay IPC driver and
+  exposing it to higher level Xlink layer)
+
+In addition to the above, the driver exposes SoC information (like stepping,

Documentation/vpu/vpu-stack-overview.rst new file +24 -0

+IDs to HW FIFO entries, using the Keem Bay VPU IPC driver.
+
+Some of the main functions this driver provides:
+
+* establishing a connection with an IPC device
+* obtaining a list with the available devices
+* obtaining the status for a device
+* booting a device
+* resetting a device
+* opening and closing channels
+* issuing read and write operations
+
+Xlink-core
+==========
+
+This component implements an abstracted set of control and communication APIs
+based on channel identification. It is intended to support VPU technology both
+at SoC level as well as at IP level, over multiple interfaces.
+
+It provides symmetrical services, where the producer and the consumer have
+the same privileges.
+
+Xlink driver has the ability to abstract several types of communication
+channels underneath, allowing the usage of different interfaces with the same

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+  Vision platform
+
+
+VPUMGR (VPU Manager)
+====================
+
+Bridges firmware on VPU side and applications on CPU user-space, it assists
+firmware on VPU side serving multiple user space application processes on CPU
+side concurrently while also performing necessary data buffer management on
+behalf of VPU IP.

Hi--

Quoted 5 lines · On 12/1/20 2:34 PM, [email protected] wrote:

From: mark gross <[email protected]>

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Mark Gross <[email protected]>

My reading of submitting-patches.rst seems to indicate that the Reviewer and Submitter are probably not the same person.

Are you sure that you reviewed it?

Quoted 13 lines

---
 Documentation/index.rst                  |   3 +-
 Documentation/vpu/index.rst              |  16 ++
 Documentation/vpu/vpu-stack-overview.rst | 267 +++++++++++++++++++++++
 3 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/vpu/index.rst
 create mode 100644 Documentation/vpu/vpu-stack-overview.rst

Documentation/index.rst +0 -0

index 57719744774c..0a2cc0204e8f 100644

Documentation/index.rst line 1

Quoted 2 lines

Documentation/index.rst +1 -1

@@ -1,4 +1,4 @@
-.. SPDX-License-Identifier: GPL-2.0
+.. SPDX-License-Identifier: GPL-2.0-only

That looks both inappropriate for this patch and incorrect AFAICT.

Quoted 17 lines

Documentation/index.rst +1 -0

 .. The Linux Kernel documentation master file, created by

@@ -137,6 +137,7 @@ needed).
    misc-devices/index
    scheduler/index
    mhi/index
+   vpu/index
 
 Architecture-agnostic documentation
 -----------------------------------

Documentation/vpu/index.rst new file +0 -0

new file mode 100644
index 000000000000..7e290e048910

Documentation/vpu/index.rst line 1

Quoted 1 line

Documentation/vpu/index.rst new file +1 -0

@@ -0,0 +1,16 @@
+.. SPDX-License-Identifier: GPL-2.0-only

license-rules.rst says:

For 'GNU General Public License (GPL) version 2 only' use:
SPDX-License-Identifier: GPL-2.0

Quoted 21 lines

Documentation/vpu/index.rst new file +15 -0

+
+============================================
+Vision Processor Unit Documentation
+============================================
+
+This documentation contains information for the Intel VPU stack.
+
+.. class:: toc-title
+
+	   Table of contents
+
+.. toctree::
+   :maxdepth: 2
+
+   vpu-stack-overview

Documentation/vpu/vpu-stack-overview.rst new file +0 -0

new file mode 100644
index 000000000000..53c06a7d9a52

Documentation/vpu/vpu-stack-overview.rst line 1

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

@@ -0,0 +1,267 @@
+.. SPDX-License-Identifier: GPL-2.0-only

Nope.

Documentation/vpu/vpu-stack-overview.rst line 11

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+
+======================
+Intel VPU architecture
+======================
+
+Overview
+========
+
+The Intel Movidius acquisition has developed a Vision Processing Unit (VPU)
+roadmap of products starting with Keem Bay (KMB).  The HW configurations the

s/HW/hardware/

Documentation/vpu/vpu-stack-overview.rst line 14

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+VPU can support include:
+
+1. Standalone smart camera that does local CV processing in camera

Tell us what CV is before using it.

Documentation/vpu/vpu-stack-overview.rst line 15

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+2. Standalone appliance or SBC device connected to a network and tethered

Tell us what SBC is before using it. (yeah, I know)

Documentation/vpu/vpu-stack-overview.rst line 18

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+   cameras doing local CV processing
+3. Embedded in a USB dongle or M.2 as an CV accelerator.
+4. Multiple VPU enabled SOC's on a PCIE card as a CV accelerator in a larger IA

                                      PCIe (?)

Documentation/vpu/vpu-stack-overview.rst line 22

Quoted 4 lines

Documentation/vpu/vpu-stack-overview.rst new file +4 -0

+   box or server.
+
+Keem Bay is the first instance of this family of products. This document
+provides an architectural overview of the SW stack supporting the VPU enabled

s/SW/software/

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+products.
+
+Keem Bay (KMB) is a Computer Vision AI processing SoC based on ARM A53 CPU that
+provides Edge neural network acceleration (inference) and includes a Vision
+Processing Unit (VPU) hardware.  The ARM CPU SubSystem (CPUSS) interfaces
+locally to the VPU and enables integration/interfacing with a remote host over
+PCIe or USB or Ethernet interfaces. The interface between the CPUSS and the VPU
+is implemented with HW FIFOs (Control) and coherent memory mapping (Data) such
+that zero copy processing can happen within the VPU.
+

Documentation/vpu/vpu-stack-overview.rst line 42

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+The KMB can be used in all 4 of the above classes of designs.
+
+We refer to the 'local host' as being the ARM part of the SoC, while the
+'remote host' as the IA system hosting the KMB device(s).  The KMB SoC boots
+from an eMMC via uBoot and ARM Linux compatible device tree interface with an
+expectation to fully boot within hundreds of milliseconds.  There is also
+support for downloading the kernel and root file system image from a remote
+host.
+
+The eMMC can be updated with standard mender update process.

Mender

Documentation/vpu/vpu-stack-overview.rst line 46

Quoted 4 lines

Documentation/vpu/vpu-stack-overview.rst new file +4 -0

+See https://github.com/mendersoftware/mender
+
+The VPU is started and controlled from the A53 local host.  Its firmware image
+is loaded using the drive FW helper KAPI's.

s/FW/firmware/

Documentation/vpu/vpu-stack-overview.rst line 51

Quoted 5 lines

Documentation/vpu/vpu-stack-overview.rst new file +5 -0

+
+The VPU IP FW payload consists of a SPARC ISA RTEMS bootloader and/or
+application binary.
+
+The interface allowing (remote or local) host clients  to access VPU IP

^^drop one space

Documentation/vpu/vpu-stack-overview.rst line 54

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+capabilities is realized through an abstracted programming model, which
+provides Remote Proxy APIs for a host CPU application to dynamically create and
+execute CV and NN workloads on the VPU. All frameworks exposed through

Tell us what NN is.

Documentation/vpu/vpu-stack-overview.rst line 67

Quoted 13 lines

Documentation/vpu/vpu-stack-overview.rst new file +13 -0

+programming model’s APIs are contained in the pre-compiled standard firmware
+image.
+
+There is a significant SW stack built up to support KMB and the use cases.  The
+rest of this documentation provides an overview of the components of the stack.
+
+Keem Bay IPC
+============
+
+Directly interfaces with the KMB HW FIFOs to provide zero copy processing from
+the VPU.  It implements the lowest level protocol for interacting with the VPU.
+
+The Keem Bay IPC mechanism is based on shared memory and hardware FIFOs,

FIFOs.

Documentation/vpu/vpu-stack-overview.rst line 68

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+specifically there are:

Specifically

Documentation/vpu/vpu-stack-overview.rst line 71

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+
+* Two 128-entry HW FIFOs, one for the CPU and one for the VPU.
+* Two shared memory regions, used as memory pool for allocating IPC buffers

end with a period since the previous line did that.

Documentation/vpu/vpu-stack-overview.rst line 74

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+
+An IPC channel is a software abstraction allowing communication multiplexing,
+so that multiple applications / users can concurrently communicated to the VPU.

communicate

Documentation/vpu/vpu-stack-overview.rst line 77

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+IPC channels area conceptually similar to socket ports.
+
+There is a total of 1024 channels, each one identified by a channel ID, ranging

are

Quoted 6 lines

Documentation/vpu/vpu-stack-overview.rst new file +6 -0

+from 0 to 1023.
+
+Channels are divided in two categories:
+
+* High-Speed (HS) channels, having IDs in the 0-9 range.
+* General-Purpose (GP) channels, having IDs in the 10-1023 range.

Documentation/vpu/vpu-stack-overview.rst line 93

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+
+HS channels have higher priority over GP channels and can be used by
+applications requiring higher throughput or lower latency.
+
+Since all the channels share the same HW resources (i.e., the HW FIFOs and the
+IPC memory pools), the Keem Bay IPC driver uses software queues to give a
+higher priority to HS channels.
+
+The driver supports a build-time configurable number of communication channels
+defined in a so called Channel Mapping Table.

so-called

Quoted 26 lines

Documentation/vpu/vpu-stack-overview.rst new file +26 -0

+
+An IPC channel is full duplex: a pending operation from a certain channel does
+not block other operations on the same channel, regardless of their operation
+mode (blocking or non-blocking).
+
+Operation mode is individually selectable for each channel, per operation
+direction (read or write). All operations for that direction comply to
+selection.
+
+
+Keem Bay-VPU-IPC
+================
+
+This is the MMIO driver of the VPU IP block inside the SOC. It is a control
+driver mapping IPC channel communication to Xlink virtual channels.
+
+This driver provides the following functionality to other drivers in the
+communication stack:
+
+* VPU IP execution control (firmware load, start, reset)
+* VPU IP event notifications (device connected, device disconnected, WDT event)
+* VPU IP device status query (OFF, BUSY, READY, ERROR, RECOVERY)
+* Communication via the IPC protocol (wrapping the Keem Bay IPC driver and
+  exposing it to higher level Xlink layer)
+
+In addition to the above, the driver exposes SoC information (like stepping,

Documentation/vpu/vpu-stack-overview.rst line 129

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+device ID, etc.) to user-space via sysfs.
+
+This driver depends on the 'Keem Bay IPC' driver, which enables the Keem Bay
+IPC communication protocol.
+
+The driver uses the Firmware API to load the VPU firmware from user-space.
+
+Xlink-IPC
+=========
+This component is implementing the IPC specific Xlink protocol. It maps channel

component implements IPC-specific

Quoted 24 lines

Documentation/vpu/vpu-stack-overview.rst new file +24 -0

+IDs to HW FIFO entries, using the Keem Bay VPU IPC driver.
+
+Some of the main functions this driver provides:
+
+* establishing a connection with an IPC device
+* obtaining a list with the available devices
+* obtaining the status for a device
+* booting a device
+* resetting a device
+* opening and closing channels
+* issuing read and write operations
+
+Xlink-core
+==========
+
+This component implements an abstracted set of control and communication APIs
+based on channel identification. It is intended to support VPU technology both
+at SoC level as well as at IP level, over multiple interfaces.
+
+It provides symmetrical services, where the producer and the consumer have
+the same privileges.
+
+Xlink driver has the ability to abstract several types of communication
+channels underneath, allowing the usage of different interfaces with the same

Documentation/vpu/vpu-stack-overview.rst line 163

Quoted 10 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+function calls.
+
+Xlink services are available to both kernel and user space clients and include:
+
+* interface abstract control and communication API
+* multi device support
+* concurrent communication across 4096 communication channels (from 0 to
+  0xFFF), with customizable properties
+* full duplex channels with multiprocess and multithread support
+* channel IDs can be mapped to desired physical interface (PCIE, USB, ETH, IPC)

PCIe

Documentation/vpu/vpu-stack-overview.rst line 165

Quoted 2 lines

Documentation/vpu/vpu-stack-overview.rst new file +2 -0

+  via a Channel Mapping Table
+* asynchronous fast pass through mode: remote host data packets are directly

passthrough

Documentation/vpu/vpu-stack-overview.rst line 173

Quoted 8 lines

Documentation/vpu/vpu-stack-overview.rst new file +8 -0

+  dispatched using interrupt systems running on local host to IPC calls for low
+  overhead
+* channel handshaking mechanism for peer to peer communication, without the
+  need of static channel preallocation
+* channel resource management
+* asynchronous data and device notifications to subscribers
+
+Xlink transports: PCIe, USB, ETH, IPCXLink-PCIe

IPC,

Documentation/vpu/vpu-stack-overview.rst line 175

Quoted 2 lines

Documentation/vpu/vpu-stack-overview.rst new file +2 -0

+
+XLink-PCIE

PCIe

Documentation/vpu/vpu-stack-overview.rst line 177

Quoted 2 lines

Documentation/vpu/vpu-stack-overview.rst new file +2 -0

+==========
+This is an endpoint driver that is mapping Xlink channel IDs to PCIE channels.

that maps PCIe

Documentation/vpu/vpu-stack-overview.rst line 180

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+
+This component ensures (remote)host-to-(local)host communication, and VPU IP
+communication via an asynchronous pass through mode, where PCIE data loads are

passthrough PCIe

Documentation/vpu/vpu-stack-overview.rst line 183

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+directly dispatched to Xlink-IPC.
+
+The component builds and advertises Device IDs that can are used by local host

that are used

Documentation/vpu/vpu-stack-overview.rst line 188

Quoted 5 lines

Documentation/vpu/vpu-stack-overview.rst new file +5 -0

+application in case of multi device scenarios.
+
+XLink-USB
+==========
+This is an endpoint driver that is mapping Xlink channel IDs to bidirectional

that maps

Documentation/vpu/vpu-stack-overview.rst line 189

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+USB endpoints and supports CDC USB class protocol. More than one Xlink channels

channel

Documentation/vpu/vpu-stack-overview.rst line 193

Quoted 4 lines

Documentation/vpu/vpu-stack-overview.rst new file +4 -0

+can be mapped to a single USB endpoint.
+
+This component ensures host-to-host communication, and, as well, asynchronous
+pass through communication, where USB transfer packets are directly dispatched

passthrough

Documentation/vpu/vpu-stack-overview.rst line 196

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+to Xlink-IPC.
+
+The component builds and advertises Device IDs that can are used by local host

that are used

Documentation/vpu/vpu-stack-overview.rst line 202

Quoted 6 lines

Documentation/vpu/vpu-stack-overview.rst new file +6 -0

+application in case of multi device scenarios.
+
+XLink-ETH
+=========
+
+This is an endpoint driver that is mapping Xlink channel IDs to Ethernet

that maps

Documentation/vpu/vpu-stack-overview.rst line 206

Quoted 4 lines

Documentation/vpu/vpu-stack-overview.rst new file +4 -0

+sockets.
+
+This component ensures host-to-host communication, and, as well, asynchronous
+pass through communication, where Ethernet data loads are directly dispatched to

passthrough

Documentation/vpu/vpu-stack-overview.rst line 209

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+Xlink-IPC.
+
+The component builds and advertises Device IDs that can are used by local host

that are used

Documentation/vpu/vpu-stack-overview.rst line 218

Quoted 9 lines

Documentation/vpu/vpu-stack-overview.rst new file +9 -0

+application in case of multi device scenarios.
+
+Assorted drivers that depend on this stack:
+
+Xlink-SMB
+=========
+The Intel Edge.AI Computer Vision platforms have to be monitored using platform
+devices like sensors, fan controller, IO expander etc. Some of these devices
+are memory mapped and some are i2c based. Either of these devices are not

I2C-based. None of these devices is

Documentation/vpu/vpu-stack-overview.rst line 224

Quoted 6 lines

Documentation/vpu/vpu-stack-overview.rst new file +6 -0

+directly accessible to the host.
+
+The host here refers to the server to which the vision accelerators are
+connected over PCIe Interface. The Host needs to do a consolidated action based
+on the parameters of platform devices. In general, most of the standard devices
+(includes sensors, fan controller, IO expander etc) are I2C/SMBus based and are

SMBus-based

Documentation/vpu/vpu-stack-overview.rst line 226

Quoted 2 lines

Documentation/vpu/vpu-stack-overview.rst new file +2 -0

+used to provide the status of the accelerator. Standard drivers for these
+devices are available based on i2c/smbus APIs.

I2C/SMBus

Documentation/vpu/vpu-stack-overview.rst line 229

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+
+Instead of changing the sensor drivers to adapt to PCIe interface, a generic
+i2c adapter "xlink-smbus" which underneath uses xlink as physical medium is

I2C Xlink

Documentation/vpu/vpu-stack-overview.rst line 230

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+used. With xlink-smbus, the drivers for the platform devices doesn't need to

don't

Documentation/vpu/vpu-stack-overview.rst line 237

Quoted 7 lines

Documentation/vpu/vpu-stack-overview.rst new file +7 -0

+undergo any interface change.
+
+TSEN
+====
+
+Thermal sensor driver for exporting thermal events to the local Arm64 host as
+well as to the remote X86 host if in the PCIe add in CV accelerator

add-in

Documentation/vpu/vpu-stack-overview.rst line 240

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+configuration.
+
+The driver receiving the junction temperature from different heating points

receives

Documentation/vpu/vpu-stack-overview.rst line 241

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+inside the SOC. The driver will receive the temperature on SMBUS connection and

SMBus

Documentation/vpu/vpu-stack-overview.rst line 244

Quoted 3 lines

Documentation/vpu/vpu-stack-overview.rst new file +3 -0

+forward over xlink-smb when in a remote host configuration.
+
+In Keem Bay, the four thermal junction temperature points are, Media Subsystem

^no comma

Documentation/vpu/vpu-stack-overview.rst line 245

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+(mss), NN subsystem (nce), Compute subsystem (cse) and SOC(Maximum of mss, nce

and SOC (maximum of mss, nce

Documentation/vpu/vpu-stack-overview.rst line 246

Quoted 1 line

Documentation/vpu/vpu-stack-overview.rst new file +1 -0

+and cse)

       cse).

Documentation/vpu/vpu-stack-overview.rst line 257

Quoted 11 lines

Documentation/vpu/vpu-stack-overview.rst new file +11 -0

+
+HDDL
+====
+
+- Exports details of temperature sensor, current sensor and fan controller
+  present in Intel Edge.AI Computer Vision platforms to IA host.
+- Enable Time sync of Intel Edge.AI Computer Vision platform with IA host.
+- Handles device connect and disconnect events.
+- Receives slave address from the IA host for memory mapped thermal sensors
+  present in SoC (Documentation/hwmon/intel_tsens_sensors.rst).
+- Registers i2c slave device for slaves present in Intel Edge.AI Computer

               I2C

Quoted 11 lines

Documentation/vpu/vpu-stack-overview.rst new file +10 -0

+  Vision platform
+
+
+VPUMGR (VPU Manager)
+====================
+
+Bridges firmware on VPU side and applications on CPU user-space, it assists
+firmware on VPU side serving multiple user space application processes on CPU
+side concurrently while also performing necessary data buffer management on
+behalf of VPU IP.

Signature · 1 line

~Randy

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Daniele Alessandrelli <[email protected]>

---
 .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +6 -0

new file mode 100644
index 000000000000..6e21c54d8f34
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 Intel Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +47 -0

+
+properties:
+  compatible:
+    const: intel,keembay-ipc
+
+  reg:
+    items:
+      - description: The CSS (CPU) FIFO registers
+      - description: The MSS (VPU) FIFO registers
+
+  reg-names:
+    items:
+      - const: css_fifo
+      - const: mss_fifo
+
+  interrupts:
+    items:
+      - description: CSS FIFO not-empty interrupt
+
+  interrupt-controller: true
+
+  memory-region:
+    items:
+      - description: Reserved memory region used for CSS IPC buffers
+      - description: Reserved memory region used for MSS IPC buffers
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    ipc@203300f0 {
+          compatible = "intel,keembay-ipc";
+          reg = <0x203300f0 0x310>, /* CPU TIM FIFO */
+                <0x208200f0 0x310>; /* VPU TIM FIFO */
+          reg-names = "css_fifo", "mss_fifo";
+          interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+          memory-region = <&css_ipc_reserved>, <&mss_ipc_reserved>;
+    };

Signature · 1 line

2.17.1

Quoted 31 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Daniele Alessandrelli <[email protected]>

---
 .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +10 -0

new file mode 100644
index 000000000000..6e21c54d8f34
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 Intel Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Keem Bay IPC
+
+maintainers:

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 6 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +6 -0

+  - Daniele Alessandrelli <[email protected]>
+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

What's the relationship between this and the xlink thing?

Quoted 7 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +5 -0

+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

What's the relationship between this and the xlink thing?

Hi Rob,

Thanks for the feedback.

Quoted 8 lines · Rob Herring

Quoted 7 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

[cut]

Quoted 7 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +5 -0

+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

We did consider using the mailbox framework, but eventually decided against it; mainly because of the following two reasons:

1. The channel concept in the Mailbox framework is different than the channel concept in Keem Bay IPC:

a. My understanding is that Mailbox channels are meant to be SW
representation of physical HW channels, while in Keem Bay IPC channels are software abstractions to achieve communication multiplexing over a single HW link

b. Additionally, Keem Bay IPC has two different classes of channels
(high-speed channels and general-purpose channels) that need to access the same HW link with different priorities.

2. The blocking / non-blocking TX behavior of mailbox channels is defined at channel creation time (by the tx_block value of the mailbox client passed to mbox_request_channel();

my understanding
is that the tx_block value cannot be modified after the channel is created),

while in Keem Bay IPC the same channel can be used for
both blocking and non-blocking TX (behavior is controlled by the timeout argument passed to keembay_ipc_send()).

Having said that, I guess that it could be possible to create a Mailbox driver implementing the core communication mechanism used by the Keem Bay IPC and then build our API around it (basically having two drivers). But I'm not sure that would make the code simpler or easier to maintain. Any thoughts on this?

Quoted 28 lines · Daniele Alessandrelli

Hi Rob,

Thanks for the feedback.

Quoted 8 lines · Rob Herring

Quoted 7 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

[cut]

Quoted 7 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +5 -0

+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

We did consider using the mailbox framework, but eventually decided against it; mainly because of the following two reasons:

Quoted 8 lines

1. The channel concept in the Mailbox framework is different than the channel concept in Keem Bay IPC:

a. My understanding is that Mailbox channels are meant to be SW
representation of physical HW channels, while in Keem Bay IPC channels are software abstractions to achieve communication multiplexing over a single HW link

In mailbox api, that would be a physical channel shared between various clients.

Quoted 4 lines

b. Additionally, Keem Bay IPC has two different classes of channels
(high-speed channels and general-purpose channels) that need to access the same HW link with different priorities.

If the priorities are hard (programmed into some register), you could do that via dt during channel population.
If they are soft, that would be handled in the shared channel implementation.

Quoted 4 lines

2. The blocking / non-blocking TX behavior of mailbox channels is defined at channel creation time (by the tx_block value of the mailbox client passed to mbox_request_channel();

No, that is checked at mbox_send_message()

Quoted 4 lines

my understanding
is that the tx_block value cannot be modified after the channel is created),

Again no. If you don't queue more than one message at any time you can change it between transfers. To be safe you can always change it between channel release - request calls.

Quoted 4 lines

while in Keem Bay IPC the same channel can be used for
both blocking and non-blocking TX (behavior is controlled by the timeout argument passed to keembay_ipc_send()).

Quoted 6 lines

Having said that, I guess that it could be possible to create a Mailbox driver implementing the core communication mechanism used by the Keem Bay IPC and then build our API around it (basically having two drivers). But I'm not sure that would make the code simpler or easier to maintain. Any thoughts on this?

I think so. Most of KeemBay specific behaviour would be implemented in the shared channel above the mailbox api.

Quoted 72 lines · Jassi Brar

Quoted 28 lines · Daniele Alessandrelli

Hi Rob,

Thanks for the feedback.

Quoted 8 lines · Rob Herring

Quoted 7 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

[cut]

Quoted 7 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +5 -0

+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

We did consider using the mailbox framework, but eventually decided against it; mainly because of the following two reasons:

Quoted 8 lines

1. The channel concept in the Mailbox framework is different than the channel concept in Keem Bay IPC:

a. My understanding is that Mailbox channels are meant to be SW
representation of physical HW channels, while in Keem Bay IPC channels are software abstractions to achieve communication multiplexing over a single HW link

In mailbox api, that would be a physical channel shared between various clients.

Quoted 4 lines

b. Additionally, Keem Bay IPC has two different classes of channels
(high-speed channels and general-purpose channels) that need to access the same HW link with different priorities.

If the priorities are hard (programmed into some register), you could do that via dt during channel population.
If they are soft, that would be handled in the shared channel implementation.

Quoted 4 lines

2. The blocking / non-blocking TX behavior of mailbox channels is defined at channel creation time (by the tx_block value of the mailbox client passed to mbox_request_channel();

No, that is checked at mbox_send_message()

Quoted 4 lines

my understanding
is that the tx_block value cannot be modified after the channel is created),

Again no. If you don't queue more than one message at any time you can change it between transfers. To be safe you can always change it between channel release - request calls.

Quoted 10 lines

while in Keem Bay IPC the same channel can be used for
both blocking and non-blocking TX (behavior is controlled by the timeout argument passed to keembay_ipc_send()).

Having said that, I guess that it could be possible to create a Mailbox driver implementing the core communication mechanism used by the Keem Bay IPC and then build our API around it (basically having two drivers). But I'm not sure that would make the code simpler or easier to maintain. Any thoughts on this?

Quoted 2 lines

I think so. Most of KeemBay specific behaviour would be implemented in the shared channel above the mailbox api.

Quick question. By "I think so"

do you mean that you feel the keem bay IPC
code will be simpler and easier to maintain if we make yet another driver at the Keem Bay IPC driver sits on top off? Or, the current implementation would be simpler if we rework the implementation to use the mailbox api?

Hi Rob,

Thanks for the feedback.

Quoted 8 lines · Rob Herring

Quoted 7 lines · On Tue, Dec 01, 2020 at 02:34:51PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

Add DT binding documentation for the Intel Keem Bay IPC driver, which enables communication between the Computing Sub-System (CSS) and the Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem Bay.

[cut]

Quoted 7 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml new file +5 -0

+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.

Sounds like a mailbox.

We did consider using the mailbox framework, but eventually decided against it; mainly because of the following two reasons:

1. The channel concept in the Mailbox framework is different than the channel concept in Keem Bay IPC:

a. My understanding is that Mailbox channels are meant to be SW
representation of physical HW channels, while in Keem Bay IPC channels are software abstractions to achieve communication multiplexing over a single HW link

b. Additionally, Keem Bay IPC has two different classes of channels
(high-speed channels and general-purpose channels) that need to access the same HW link with different priorities.

2. The blocking / non-blocking TX behavior of mailbox channels is defined at channel creation time (by the tx_block value of the mailbox client passed to mbox_request_channel(); my understanding is that the tx_block value cannot be modified after the channel is created), while in Keem Bay IPC the same channel can be used for both blocking and non-blocking TX (behavior is controlled by the timeout argument passed to keembay_ipc_send()).

Having said that, I guess that it could be possible to create a Mailbox driver implementing the core communication mechanism used by the Keem Bay IPC and then build our API around it (basically having two drivers). But I'm not sure that would make the code simpler or easier to maintain. Any thoughts on this?

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 7 lines

MAINTAINERS +7 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

Does this mean you all will have to do it on your own time and not as part of your work at Intel? If so, why not just use your personal email addresses instead to make it a bit more obvious?

Quoted 7 lines · Greg KH

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

Quoted 11 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 7 lines

MAINTAINERS +7 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over time.

While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better.

Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over time.

Quoted 22 lines · mark gross

Quoted 7 lines · Greg KH

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

Quoted 11 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 7 lines

MAINTAINERS +7 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over time.

Quoted 7 lines

While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better.

Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over time.

I'm not objecting to your, or anyone else's name on this at all. I'm just asking about Intel's support for this new codebase being added. Having a new subsystem from a major company and not have someone paid to actually maintain it seems really odd to me.

That's all. If that's Intel's stance, that's fine, just wanted to clarify it is correct as I know some people at Intel have been confused recently about just what the S: field means.

Quoted 31 lines · Greg KH

Quoted 22 lines · mark gross

Quoted 7 lines · Greg KH

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

Quoted 11 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 7 lines

MAINTAINERS +7 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over time.

Quoted 7 lines

While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better.

Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over time.

Quoted 8 lines

I'm not objecting to your, or anyone else's name on this at all. I'm just asking about Intel's support for this new codebase being added. Having a new subsystem from a major company and not have someone paid to actually maintain it seems really odd to me.

That's all. If that's Intel's stance, that's fine, just wanted to clarify it is correct as I know some people at Intel have been confused recently about just what the S: field means.

I've been following up on whether the status field should be "Supported" or "Maintained" at this time. For this current instantiation of the VPU enabling under review here I think Maintained most appropriate. There are a good number of people who look after it.

However; I have learned that the instantiations of the VPU after keem bay and its follow on SoC will include an evolution of this stack and between now and when those get close to landing that evolved version will become "Supported".

Given this, would it be more appropriate to put this stack into staging for a while?

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Saturday, December 5, 2020 12:40 AM
To: mark gross <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Alessandrelli, Daniele <[email protected]>; Iyer, Sundar <[email protected]>
Subject: Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module

Quoted 10 lines · mark gross

Quoted 9 lines · Greg KH

Quoted 8 lines · mark gross

Quoted 7 lines · Greg KH

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

MAINTAINERS M: Deepak Saxena <[email protected]>

Quoted 5 lines

MAINTAINERS +5 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-

ipc.yaml

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 2 lines

MAINTAINERS +2 -0

+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over

time.

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better.

Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over

time.

Quoted 9 lines

Quoted 9 lines

I'm not objecting to your, or anyone else's name on this at all. I'm just asking about Intel's support for this new codebase being added. Having a new subsystem from a major company and not have someone paid to actually maintain it seems really odd to me.

That's all. If that's Intel's stance, that's fine, just wanted to clarify it is correct as I know some people at Intel have been confused recently about just what the S: field means.

Quoted 8 lines

I've been following up on whether the status field should be "Supported" or "Maintained" at this time. For this current instantiation of the VPU enabling under review here I think Maintained most appropriate. There are a good number of people who look after it.

However; I have learned that the instantiations of the VPU after keem bay and its follow on SoC will include an evolution of this stack and between now and when those get close to landing that evolved version will

become "Supported".

Quoted 1 line

Quoted 2 lines

Given this, would it be more appropriate to put this stack into staging for a while?

drivers/staging/ is for code that for some reason is not good enough to be merged to the "right" place in the kernel tree, and you need community help to get it cleaned up because you can not do it yourself.

Is that the case here? If not, then no, it should not go into drivers/staging/.

Quoted 71 lines

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Saturday, December 5, 2020 12:40 AM
To: mark gross <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Alessandrelli, Daniele <[email protected]>; Iyer, Sundar <[email protected]>
Subject: Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module

Quoted 10 lines · mark gross

Quoted 9 lines · Greg KH

Quoted 8 lines · mark gross

Quoted 7 lines · Greg KH

Quoted 6 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

MAINTAINERS +0 -0

@@ -8955,6 +8955,14 @@ M:	Deepak Saxena <[email protected]>
 S:	Maintained
 F:	drivers/char/hw_random/ixp4xx-rng.c
 

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8962

Quoted 5 lines

MAINTAINERS +5 -0

+INTEL KEEM BAY IPC DRIVER
+M:	Daniele Alessandrelli <[email protected]>
+M:	Mark Gross <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/intel/intel,keembay-

ipc.yaml

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

MAINTAINERS M: Deepak Saxena <[email protected]> · line 8964

Quoted 2 lines

MAINTAINERS +2 -0

+F:	drivers/soc/intel/keembay-ipc.c
+F:	include/linux/soc/intel/keembay-ipc.h

Sad that Intel is not going to actually pay you all to do this maintenance work for a brand new subsystem you are wanting to add to the tree :(

I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over

time.

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better.

Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over

time.

Quoted 9 lines

Quoted 9 lines

I'm not objecting to your, or anyone else's name on this at all. I'm just asking about Intel's support for this new codebase being added. Having a new subsystem from a major company and not have someone paid to actually maintain it seems really odd to me.

That's all. If that's Intel's stance, that's fine, just wanted to clarify it is correct as I know some people at Intel have been confused recently about just what the S: field means.

Quoted 8 lines

I've been following up on whether the status field should be "Supported" or "Maintained" at this time. For this current instantiation of the VPU enabling under review here I think Maintained most appropriate. There are a good number of people who look after it.

However; I have learned that the instantiations of the VPU after keem bay and its follow on SoC will include an evolution of this stack and between now and when those get close to landing that evolved version will

become "Supported".

Quoted 3 lines

Given this, would it be more appropriate to put this stack into staging for a while?

Quoted 5 lines

drivers/staging/ is for code that for some reason is not good enough to be merged to the "right" place in the kernel tree, and you need community help to get it cleaned up because you can not do it yourself.

Is that the case here? If not, then no, it should not go into drivers/staging/.

That is not the case here. Lets proceed as we are on this then.

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Keem Bay IPC mechanism is based on shared memory and hardware FIFOs. Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and the VPU. Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender can reuse the buffer).

Quoted 42 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Keem Bay IPC mechanism is based on shared memory and hardware FIFOs. Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and the VPU. Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Quoted 9 lines

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender can reuse the buffer).

Any reason you can't use the dmabuf interface for these memory buffers you are creating and having to manage "by hand"? I thought that was what the kernel was wanting to unify on such that individual drivers/subsystems didn't have to do this on their own.

Hi Greg,

Thanks for your feedback.

Quoted 53 lines · Greg KH

Quoted 42 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Keem Bay IPC mechanism is based on shared memory and hardware FIFOs. Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and the VPU. Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Quoted 9 lines

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender can reuse the buffer).

Quoted 4 lines

Any reason you can't use the dmabuf interface for these memory buffers you are creating and having to manage "by hand"? I thought that was what the kernel was wanting to unify on such that individual drivers/subsystems didn't have to do this on their own.

My understanding is that the dmabuf interface is used to share DMA buffers across different drivers, while these buffers are used only internally to the IPC driver (and exchanged only with the VPU firmware). They basically are packet headers that are sent to the VPU.

Quoted 63 lines · Daniele Alessandrelli

Hi Greg,

Thanks for your feedback.

Quoted 53 lines · Greg KH

Quoted 42 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Keem Bay IPC mechanism is based on shared memory and hardware FIFOs. Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and the VPU. Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Quoted 9 lines

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender can reuse the buffer).

Quoted 4 lines

Any reason you can't use the dmabuf interface for these memory buffers you are creating and having to manage "by hand"? I thought that was what the kernel was wanting to unify on such that individual drivers/subsystems didn't have to do this on their own.

Quoted 4 lines

My understanding is that the dmabuf interface is used to share DMA buffers across different drivers, while these buffers are used only internally to the IPC driver (and exchanged only with the VPU firmware). They basically are packet headers that are sent to the VPU.

There's no reason you couldn't use these to share your buffers "internally" as well, because you have the same lifetime rules and accounting and all other sorts of things you have to handle, right? Why rewrite something like this when you should take advantage of common code instead?

Quoted 69 lines · Greg KH

Quoted 63 lines · Daniele Alessandrelli

Hi Greg,

Thanks for your feedback.

Quoted 53 lines · Greg KH

Quoted 42 lines · On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected] wrote:

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Keem Bay IPC mechanism is based on shared memory and hardware FIFOs. Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and the VPU. Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Quoted 9 lines

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender can reuse the buffer).

Quoted 4 lines

Any reason you can't use the dmabuf interface for these memory buffers you are creating and having to manage "by hand"? I thought that was what the kernel was wanting to unify on such that individual drivers/subsystems didn't have to do this on their own.

Quoted 4 lines

My understanding is that the dmabuf interface is used to share DMA buffers across different drivers, while these buffers are used only internally to the IPC driver (and exchanged only with the VPU firmware). They basically are packet headers that are sent to the VPU.

Quoted 5 lines

There's no reason you couldn't use these to share your buffers "internally" as well, because you have the same lifetime rules and accounting and all other sorts of things you have to handle, right? Why rewrite something like this when you should take advantage of common code instead?

I looked at dma-buf again, but I'm still failing to see how we can use it in this driver :/

The problem I'm not able to solve is exactly how to match the lifetime of this IPC packets (IPC buffers is probably a misleading name, my bad for using it in the code) with the dma-buf lifetime rules.

Basically, these IPC packets are cache-aligned (64 bytes) and have the following fixed structure:

   struct kmb_ipc_buf {
          u32 data_addr;
          u32 data_size;
          u16 channel;
          u8  src_node;
          u8  dst_node;
          u8  status;
   } __packed __aligned(KMB_IPC_ALIGNMENT);

Now, let's ignore the first 5 fields and focus on the last one that controls the lifetime of the packet. The status field can be either FREE or ALLOCATED (perhaps should be renamed to IN_USE).

Basically, during probe, the driver allocates an array (pool) of these packets from its 'local' reserved memory region. This is done using dma_alloc_coherent(). This array remains allocated until remove() is called.

At run-time, every time the driver sends a message to the VPU, the following happens:

1. The IPC driver loops through the array of 'local' packets to find the first one marked as FREE
2. The IPC driver marks the packet as IN_USE and shares it with the VPU by putting its physical address into the VPU HW FIFO.
3. The VPU Firmware processes the packet and, when done with it, it marks it as FREE again so that the IPC driver can use it again.

Please note that the IPC driver is not notified by the VPU when the packet is marked as free (there is no IRQ or other async mechanism to do that). The driver will simply find the message free the next time it loops over the message pool again (i.e., when it is trying to send a new message).

Given that the free operation is silently done by the VPU, I'm failing to see how I can use dmabuf (or gen_pool or dma_pool). Is there anything that I'm missing?

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Friday, December 11, 2020 12:23 AM
To: Daniele Alessandrelli <[email protected]>
Cc: Alessandrelli, Daniele <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module

Quoted 8 lines · Daniele Alessandrelli

Quoted 7 lines · Greg KH

Quoted 6 lines · Daniele Alessandrelli

Hi Greg,

Thanks for your feedback.

Quoted 1 line · Greg KH

On Tue, Dec 01, 2020 at 02:34:52PM -0800, [email protected]

Quoted 27 lines · wrote:

Quoted 27 lines

Quoted 27 lines

Quoted 27 lines

Quoted 27 lines

From: Daniele Alessandrelli <[email protected]>

On the Intel Movidius SoC code named Keem Bay, communication between the Computing Sub-System (CSS), i.e., the CPU, and the Multimedia Sub-System (MSS), i.e., the VPU is enabled by the Keem Bay Inter-Processor Communication (IPC) mechanism.

Add the driver for using Keem Bay IPC from within the Linux Kernel.

Keem Bay IPC uses the following terminology:

- Node: A processing entity that can use the IPC to communicate;
currently, we just have two nodes, CPU (CSS) and VPU (MSS).

- Link: Two nodes that can communicate over IPC form an IPC link
(currently, we just have one link, the one between the CPU and VPU).

- Channel: An IPC link can provide multiple IPC channels. IPC channels allow communication multiplexing, i.e., the same IPC link can be used by different applications for different communications. Each channel is identified by a channel ID, which must be unique within a single IPC link. Channels are divided in two categories, High-Speed (HS) channels and General-Purpose (GP) channels. HS channels have higher priority over GP channels.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Keem Bay IPC mechanism is based on shared memory and hardware

FIFOs.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Both the CPU and the VPU have their own hardware FIFO. When the CPU wants to send an IPC message to the VPU, it writes to the VPU FIFO (MSS FIFO); similarly, when MSS wants to send an IPC message to the CPU, it writes to the CPU FIFO (CSS FIFO).

A FIFO entry is simply a pointer to an IPC buffer (aka IPC header) stored in a portion of memory shared between the CPU and

the VPU.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Specifically, the FIFO entry contains the (VPU) physical address of the IPC buffer being transferred.

In turn, the IPC buffer contains the (VPU) physical address of the payload (which must be located in shared memory too) as well as other information (payload size, IPC channel ID, etc.).

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Each IPC node instantiates a pool of IPC buffers from its own IPC buffer memory region. When instantiated, IPC buffers are marked as free. When the node needs to send an IPC message, it gets the first free buffer it finds (from its own pool), marks it as allocated (used), and puts its physical address into the IPC FIFO of the destination node. The destination node (which is notified by an interrupt when there are entries pending in its FIFO) extract the physical address from the FIFO and process the IPC buffer, marking it as free once done (so that the sender

can reuse the buffer).

Quoted 27 lines

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

Any reason you can't use the dmabuf interface for these memory buffers you are creating and having to manage "by hand"? I thought that was what the kernel was wanting to unify on such that individual drivers/subsystems didn't have to do this on their own.

My understanding is that the dmabuf interface is used to share DMA buffers across different drivers, while these buffers are used only internally to the IPC driver (and exchanged only with the VPU firmware). They basically are packet headers that are sent to the VPU.

Quoted 5 lines

There's no reason you couldn't use these to share your buffers "internally" as well, because you have the same lifetime rules and accounting and all other sorts of things you have to handle, right? Why rewrite something like this when you should take advantage of common code instead?

I looked at dma-buf again, but I'm still failing to see how we can use it in this driver :/

The problem I'm not able to solve is exactly how to match the lifetime of this IPC packets (IPC buffers is probably a misleading name, my bad for using it in the code) with the dma-buf lifetime rules.

Basically, these IPC packets are cache-aligned (64 bytes) and have the following fixed structure:

Quoted 8 lines

   struct kmb_ipc_buf {
          u32 data_addr;
          u32 data_size;
          u16 channel;
          u8  src_node;
          u8  dst_node;
          u8  status;
   } __packed __aligned(KMB_IPC_ALIGNMENT);

If that is shared out of the kernel, please use the proper data types for it.

And it isn't properly padded out? That feels like a hardware design mistake somewhere, anyway...

Quoted 12 lines

Now, let's ignore the first 5 fields and focus on the last one that controls the lifetime of the packet. The status field can be either FREE or ALLOCATED (perhaps should be renamed to IN_USE).

Basically, during probe, the driver allocates an array (pool) of these packets from its 'local' reserved memory region. This is done using dma_alloc_coherent(). This array remains allocated until remove() is called.

At run-time, every time the driver sends a message to the VPU, the following happens:

Quoted 6 lines

1. The IPC driver loops through the array of 'local' packets to find the first one marked as FREE
2. The IPC driver marks the packet as IN_USE and shares it with the VPU by putting its physical address into the VPU HW FIFO.
3. The VPU Firmware processes the packet and, when done with it, it marks it as FREE again so that the IPC driver can use it again.

How does the firmware "mark it as free"?

Quoted 9 lines

Please note that the IPC driver is not notified by the VPU when the packet is marked as free (there is no IRQ or other async mechanism to do that). The driver will simply find the message free the next time it loops over the message pool again (i.e., when it is trying to send a new message).

Given that the free operation is silently done by the VPU, I'm failing to see how I can use dmabuf (or gen_pool or dma_pool). Is there anything that I'm missing?

So maybe this isn't set up to use the dma buffers, but it still feels really odd. I recommend getting a review by the dmabuf authors before declaring that you really should be rolling your own buffer management logic here, as you can see that it feels like you are doing much the same thing, sharing memory outside of the kernel to something else.

thanks,

greg k-h

From: Paul Murphy <[email protected]>

Add DT bindings documentation for the Keem Bay VPU IPC driver.

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Paul Murphy <[email protected]>

Co-developed-by Daniele Alessandrelli <[email protected]>

Signed-off-by Daniele Alessandrelli <[email protected]>

---
 .../soc/intel/intel,keembay-vpu-ipc.yaml      | 151 ++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml

Quoted 9 lines · On Tue, 01 Dec 2020 14:34:53 -0800, [email protected] wrote:

From: Paul Murphy <[email protected]>

Add DT bindings documentation for the Keem Bay VPU IPC driver.

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Paul Murphy <[email protected]>

Co-developed-by Daniele Alessandrelli <[email protected]>

Signed-off-by Daniele Alessandrelli <[email protected]>

Quoted 5 lines

---
 .../soc/intel/intel,keembay-vpu-ipc.yaml      | 151 ++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: 'additionalProperties' is a required property /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml

See https://patchwork.ozlabs.org/patch/1409183

The base for the patch is generally the last rc1. Any dependencies should be noted.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

From: Srikanth Thokala <[email protected]>

Provide overview of XLink PCIe driver implementation

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Srikanth Thokala <[email protected]>

---
 Documentation/vpu/index.rst      |  1 +
 Documentation/vpu/xlink-pcie.rst | 91 ++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 Documentation/vpu/xlink-pcie.rst

Documentation/vpu/xlink-pcie.rst new file +4 -0

new file mode 100644
index 000000000000..bc64b566989d
@@ -0,0 +1,91 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver: xlink-pcie driver
+================================

Documentation/vpu/xlink-pcie.rst new file +35 -0

+
+------------------------
+High-level architecture:
+------------------------
+Remote Host: IA CPU
+Local Host: ARM CPU (Keem Bay)::
+
+        +------------------------------------------------------------------------+
+        |  Remote Host IA CPU              | | Local Host ARM CPU (Keem Bay) |   |
+        +==================================+=+===============================+===+
+        |  User App                        | | User App                      |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink UAPI                     | | XLink UAPI                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink Core                     | | XLink Core                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink PCIe                     | | XLink PCIe                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink-PCIe Remote Host driver  | | XLink-PCIe Local Host driver  |   |
+        +----------------------------------+-+-------------------------------+---+
+        |-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:|:|:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:|
+        +----------------------------------+-+-------------------------------+---+
+        |     PCIe Host Controller         | | PCIe Device Controller        | HW|
+        +----------------------------------+-+-------------------------------+---+
+               ^                                             ^
+               |                                             |
+               |------------- PCIe x2 Link  -----------------|
+
+This XLink PCIe driver comprises of two variants:
+* Local Host driver
+
+  * Intended for ARM CPU
+  * It is based on PCI Endpoint Framework
+  * Driver path: {tree}/drivers/misc/xlink-pcie/local_host
+

Quoted 4 lines · On 12/1/20 2:34 PM, [email protected] wrote:

From: Srikanth Thokala <[email protected]>

Provide overview of XLink PCIe driver implementation

Quoted 9 lines

Cc [email protected]

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Srikanth Thokala <[email protected]>

---
 Documentation/vpu/index.rst      |  1 +
 Documentation/vpu/xlink-pcie.rst | 91 ++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 Documentation/vpu/xlink-pcie.rst

Hi--

For document, chapter, section, etc., headings, please read & use Documentation/doc-guide/sphinx.rst:

* Please stick to this order of heading adornments:

1. ``=`` with overline for document title::

       ==============
       Document title
       ==============

2. ``=`` for chapters::

       Chapters
       ========

3. ``-`` for sections::

       Section
       -------

4. ``~`` for subsections::

       Subsection
       ~~~~~~~~~~

Quoted 11 lines

Documentation/vpu/xlink-pcie.rst new file +4 -0

new file mode 100644
index 000000000000..bc64b566989d
@@ -0,0 +1,91 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver: xlink-pcie driver
+================================

Documentation/vpu/xlink-pcie.rst line 14

Quoted 10 lines

Documentation/vpu/xlink-pcie.rst new file +10 -0

+Supported chips:
+  * Intel Edge.AI Computer Vision platforms: Keem Bay
+    Suffix: Bay
+    Slave address: 6240
+    Datasheet: Publicly available at Intel
+
+Author: Srikanth Thokala [email protected]
+
+-------------
+Introduction:

No colon at end of chapter/section headings.

Documentation/vpu/xlink-pcie.rst line 16

Quoted 2 lines

Documentation/vpu/xlink-pcie.rst new file +2 -0

+-------------
+The xlink-pcie driver in linux-5.4 provides transport layer implementation for

                            Linux 5.4 (?)

Documentation/vpu/xlink-pcie.rst line 17

Quoted 1 line

Documentation/vpu/xlink-pcie.rst new file +1 -0

+the data transfers to support xlink protocol subsystem communication with the

Xlink

Documentation/vpu/xlink-pcie.rst line 18

Quoted 1 line

Documentation/vpu/xlink-pcie.rst new file +1 -0

+peer device. i.e, between remote host system and the local Keem Bay device.

device, i.e., between the remote host system and

Documentation/vpu/xlink-pcie.rst line 20

Quoted 2 lines

Documentation/vpu/xlink-pcie.rst new file +2 -0

+
+The Keem Bay device is an ARM based SOC that includes a vision processing

ARM-based

Documentation/vpu/xlink-pcie.rst line 23

Quoted 3 lines

Documentation/vpu/xlink-pcie.rst new file +3 -0

+unit (VPU) and deep learning, neural network core in the hardware.
+The xlink-pcie driver exports a functional device endpoint to the Keem Bay device
+and supports two-way communication with peer device.

with the peer device.

Quoted 35 lines

Documentation/vpu/xlink-pcie.rst new file +35 -0

+
+------------------------
+High-level architecture:
+------------------------
+Remote Host: IA CPU
+Local Host: ARM CPU (Keem Bay)::
+
+        +------------------------------------------------------------------------+
+        |  Remote Host IA CPU              | | Local Host ARM CPU (Keem Bay) |   |
+        +==================================+=+===============================+===+
+        |  User App                        | | User App                      |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink UAPI                     | | XLink UAPI                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink Core                     | | XLink Core                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink PCIe                     | | XLink PCIe                    |   |
+        +----------------------------------+-+-------------------------------+---+
+        |   XLink-PCIe Remote Host driver  | | XLink-PCIe Local Host driver  |   |
+        +----------------------------------+-+-------------------------------+---+
+        |-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:|:|:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:|
+        +----------------------------------+-+-------------------------------+---+
+        |     PCIe Host Controller         | | PCIe Device Controller        | HW|
+        +----------------------------------+-+-------------------------------+---+
+               ^                                             ^
+               |                                             |
+               |------------- PCIe x2 Link  -----------------|
+
+This XLink PCIe driver comprises of two variants:
+* Local Host driver
+
+  * Intended for ARM CPU
+  * It is based on PCI Endpoint Framework
+  * Driver path: {tree}/drivers/misc/xlink-pcie/local_host
+

Documentation/vpu/xlink-pcie.rst line 68

Quoted 10 lines

Documentation/vpu/xlink-pcie.rst new file +10 -0

+* Remote Host driver
+
+       * Intended for IA CPU
+       * It is a PCIe endpoint driver
+       * Driver path: {tree}/drivers/misc/xlink-pcie/remote_host
+
+XLink PCIe communication between local host and remote host is achieved through
+ring buffer management and MSI/Doorbell interrupts.
+
+The xlink-pcie driver subsystem registers Keem Bay device as an endpoint driver

registers the

Documentation/vpu/xlink-pcie.rst line 69

Quoted 1 line

Documentation/vpu/xlink-pcie.rst new file +1 -0

+and provides standard linux pcie sysfs interface, # /sys/bus/pci/devices/xxxx:xx:xx.0/

Linux PCIe

Documentation/vpu/xlink-pcie.rst line 73

Quoted 4 lines

Documentation/vpu/xlink-pcie.rst new file +4 -0

+
+
+-------------------------
+XLink protocol subsystem:

No colon at end.

Documentation/vpu/xlink-pcie.rst line 75

Quoted 2 lines

Documentation/vpu/xlink-pcie.rst new file +2 -0

+-------------------------
+xlink is an abstracted control and communication subsystem based on channel

Xlink

Documentation/vpu/xlink-pcie.rst line 79

Quoted 4 lines

Documentation/vpu/xlink-pcie.rst new file +4 -0

+identification. It is intended to support VPU technology both at SoC level as
+well as at IP level, over multiple interfaces.
+
+- The xlink subsystem abstracts several types of communication channels

Xlink

Documentation/vpu/xlink-pcie.rst line 84

Quoted 5 lines

Documentation/vpu/xlink-pcie.rst new file +5 -0

+  underneath, allowing the usage of different interfaces with the
+  same function call interface.
+- The Communication channels are full-duplex protocol channels allowing
+  concurrent bidirectional communication.
+- The xlink subsystem also supports control operations to VPU either

Xlink

Documentation/vpu/xlink-pcie.rst line 87

Quoted 3 lines

Documentation/vpu/xlink-pcie.rst new file +3 -0

+  from standalone local system or from remote system based on communication
+  interface underneath.
+- The xlink subsystem supports following communication interfaces:

Xlink supports the following

Documentation/vpu/xlink-pcie.rst line 91

Quoted 5 lines

Documentation/vpu/xlink-pcie.rst new file +4 -0

+    * USB CDC
+    * Gigabit Ethernet
+    * PCIe
+    * IPC

cheers.

Signature · 1 line

~Randy

From: Srikanth Thokala <[email protected]>

Add logic to establish communication with the remote host which is through ring buffer management and MSI/Doorbell interrupts

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Srikanth Thokala <[email protected]>

---
 drivers/misc/xlink-pcie/local_host/Makefile |   2 +
 drivers/misc/xlink-pcie/local_host/core.c   | 894 ++++++++++++++++++++
 drivers/misc/xlink-pcie/local_host/core.h   | 247 ++++++
 drivers/misc/xlink-pcie/local_host/epf.c    | 116 ++-
 drivers/misc/xlink-pcie/local_host/epf.h    |  26 +
 drivers/misc/xlink-pcie/local_host/util.c   | 375 ++++++++
 drivers/misc/xlink-pcie/local_host/util.h   |  70 ++
 drivers/misc/xlink-pcie/local_host/xpcie.h  |  65 ++
 include/linux/xlink_drv_inf.h               |  60 ++
 9 files changed, 1847 insertions(+), 8 deletions(-)
 create mode 100644 drivers/misc/xlink-pcie/local_host/core.c
 create mode 100644 drivers/misc/xlink-pcie/local_host/core.h
 create mode 100644 drivers/misc/xlink-pcie/local_host/util.c
 create mode 100644 drivers/misc/xlink-pcie/local_host/util.h
 create mode 100644 include/linux/xlink_drv_inf.h

drivers/misc/xlink-pcie/local_host/Makefile +2 -0

index 54fc118e2dd1..28761751d43b 100644
@@ -1,3 +1,5 @@
 obj-$(CONFIG_XLINK_PCIE_LH_DRIVER) += mxlk_ep.o
 mxlk_ep-objs := epf.o
 mxlk_ep-objs += dma.o
+mxlk_ep-objs += core.o
+mxlk_ep-objs += util.o

drivers/misc/xlink-pcie/local_host/core.c new file +28 -0

new file mode 100644
index 000000000000..aecaaa783153
@@ -0,0 +1,894 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*****************************************************************************
+ *
+ * Intel Keem Bay XLink PCIe Driver
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ ****************************************************************************/
+
+#include <linux/of_reserved_mem.h>
+
+#include "epf.h"
+#include "core.h"
+#include "util.h"
+
+static struct xpcie *global_xpcie;
+
+static int rx_pool_size = SZ_32M;
+module_param(rx_pool_size, int, 0664);
+MODULE_PARM_DESC(rx_pool_size, "receiving pool size (default 32 MiB)");
+
+static int tx_pool_size = SZ_32M;
+module_param(tx_pool_size, int, 0664);
+MODULE_PARM_DESC(tx_pool_size, "transmitting pool size (default 32 MiB)");
+
+static int fragment_size = XPCIE_FRAGMENT_SIZE;
+module_param(fragment_size, int, 0664);
+MODULE_PARM_DESC(fragment_size, "transfer descriptor size (default 128 KiB)");

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Wednesday, December 2, 2020 11:48 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Thokala, Srikanth <[email protected]>
Subject: Re: [PATCH 09/22] misc: xlink-pcie: lh: Add core communication logic

Quoted 3 lines · On Tue, Dec 01, 2020 at 02:34:58PM -0800, [email protected] wrote:

From: Srikanth Thokala <[email protected]>

Add logic to establish communication with the remote host which is

through

Quoted 21 lines

ring buffer management and MSI/Doorbell interrupts

Reviewed-by Mark Gross <[email protected]>

Signed-off-by Srikanth Thokala <[email protected]>

---
 drivers/misc/xlink-pcie/local_host/Makefile |   2 +
 drivers/misc/xlink-pcie/local_host/core.c   | 894 ++++++++++++++++++++
 drivers/misc/xlink-pcie/local_host/core.h   | 247 ++++++
 drivers/misc/xlink-pcie/local_host/epf.c    | 116 ++-
 drivers/misc/xlink-pcie/local_host/epf.h    |  26 +
 drivers/misc/xlink-pcie/local_host/util.c   | 375 ++++++++
 drivers/misc/xlink-pcie/local_host/util.h   |  70 ++
 drivers/misc/xlink-pcie/local_host/xpcie.h  |  65 ++
 include/linux/xlink_drv_inf.h               |  60 ++
 9 files changed, 1847 insertions(+), 8 deletions(-)
 create mode 100644 drivers/misc/xlink-pcie/local_host/core.c
 create mode 100644 drivers/misc/xlink-pcie/local_host/core.h
 create mode 100644 drivers/misc/xlink-pcie/local_host/util.c
 create mode 100644 drivers/misc/xlink-pcie/local_host/util.h
 create mode 100644 include/linux/xlink_drv_inf.h

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/misc/xlink-pcie/local_host/Makefile

Review

Quoted 10 lines

index 54fc118e2dd1..28761751d43b 100644

drivers/misc/xlink-pcie/local_host/Makefile +2 -0

@@ -1,3 +1,5 @@
 obj-$(CONFIG_XLINK_PCIE_LH_DRIVER) += mxlk_ep.o
 mxlk_ep-objs := epf.o
 mxlk_ep-objs += dma.o
+mxlk_ep-objs += core.o
+mxlk_ep-objs += util.o

(quoted diff) +0 -0

b/drivers/misc/xlink-pcie/local_host/core.c

drivers/misc/xlink-pcie/local_host/core.c line 1

Quoted 7 lines

new file mode 100644
index 000000000000..aecaaa783153

drivers/misc/xlink-pcie/local_host/core.c new file +1 -0

@@ -0,0 +1,894 @@
+// SPDX-License-Identifier: GPL-2.0-only

+/************************************************************************
*****

drivers/misc/xlink-pcie/local_host/core.c line 9

Quoted 6 lines

drivers/misc/xlink-pcie/local_host/core.c new file +6 -0

+ *
+ * Intel Keem Bay XLink PCIe Driver
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+

**************************************************************************
**/

Quoted 6 lines

drivers/misc/xlink-pcie/local_host/core.c new file +6 -0

+
+#include <linux/of_reserved_mem.h>
+
+#include "epf.h"
+#include "core.h"
+#include "util.h"

drivers/misc/xlink-pcie/local_host/core.c

Quoted 10 lines

drivers/misc/xlink-pcie/local_host/core.c new file +10 -0

+
+static struct xpcie *global_xpcie;
+
+static int rx_pool_size = SZ_32M;
+module_param(rx_pool_size, int, 0664);
+MODULE_PARM_DESC(rx_pool_size, "receiving pool size (default 32 MiB)");
+
+static int tx_pool_size = SZ_32M;
+module_param(tx_pool_size, int, 0664);
+MODULE_PARM_DESC(tx_pool_size, "transmitting pool size (default 32

MiB)");

drivers/misc/xlink-pcie/local_host/core.c

Quoted 4 lines

drivers/misc/xlink-pcie/local_host/core.c new file +4 -0

+
+static int fragment_size = XPCIE_FRAGMENT_SIZE;
+module_param(fragment_size, int, 0664);
+MODULE_PARM_DESC(fragment_size, "transfer descriptor size (default 128

KiB)");

drivers/misc/xlink-pcie/local_host/core.c line 38

Quoted 10 lines

drivers/misc/xlink-pcie/local_host/core.c new file +10 -0

+
+static bool tx_pool_coherent = true;
+module_param(tx_pool_coherent, bool, 0664);
+MODULE_PARM_DESC(tx_pool_coherent,
+		 "transmitting pool using coherent memory (default true)");
+
+static bool rx_pool_coherent;
+module_param(rx_pool_coherent, bool, 0664);
+MODULE_PARM_DESC(rx_pool_coherent,
+		 "receiving pool using coherent memory (default false)");

This isn't the 1990's anymore. Please make these dynamic such that they are never needed (the code figures out the best values), or on some per-device basis using configfs or sysfs.

thanks,

greg k-h