[PATCH v4 12/12] intel-ipu3: imgu top level pci device

[Yong Zhi]

This patch adds support for the Intel IPU v3 as found
on Skylake and Kaby Lake SoCs. The driver has a dependency
on the firmware binary to function properly.

Signed-off-by: Yong Zhi 
Signed-off-by: Tomasz Figa 
---
 drivers/media/pci/intel/ipu3/Kconfig  |  17 +
 drivers/media/pci/intel/ipu3/Makefile |   6 +
 drivers/media/pci/intel/ipu3/ipu3.c   | 882 ++
 drivers/media/pci/intel/ipu3/ipu3.h   | 186 +++
 4 files changed, 1091 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3.h

diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
b/drivers/media/pci/intel/ipu3/Kconfig
index d7dab52dc881..344b57df2179 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -33,3 +33,20 @@ config INTEL_IPU3_DMAMAP
select IOMMU_IOVA
---help---
  This is IPU3 IOMMU domain specific DMA driver.
+
+config VIDEO_IPU3_IMGU
+   tristate "Intel ipu3-imgu driver"
+   depends on PCI && VIDEO_V4L2 && IOMMU_SUPPORT
+   depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
+   depends on X86 || COMPILE_TEST
+   select INTEL_IPU3_MMU
+   select INTEL_IPU3_DMAMAP
+   select VIDEOBUF2_DMA_SG
+
+   ---help---
+   This is the video4linux2 driver for Intel IPU3 image processing unit,
+   found in Intel Skylake and Kaby Lake SoCs and used for processing
+   images and video.
+
+   Say Y or M here if you have a Skylake/Kaby Lake SoC with a MIPI
+   camera. The module will be called ipu3-imgu.
diff --git a/drivers/media/pci/intel/ipu3/Makefile 
b/drivers/media/pci/intel/ipu3/Makefile
index 651773231496..c613f508ffe4 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -14,3 +14,9 @@
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
 obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
 obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o
+ipu3-imgu-objs += \
+   ipu3-tables.o ipu3-css-pool.o \
+   ipu3-css-fw.o ipu3-css-params.o \
+   ipu3-css.o ipu3-v4l2.o ipu3.o
+
+obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3-imgu.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3.c 
b/drivers/media/pci/intel/ipu3/ipu3.c
new file mode 100644
index ..5492af263ca0
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3.c
@@ -0,0 +1,882 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based on Intel IPU4 driver.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ipu3.h"
+#include "ipu3-mmu.h"
+#include "ipu3-dmamap.h"
+
+#define IMGU_NAME  "ipu3-imgu"
+#define IMGU_PCI_ID0x1919
+#define IMGU_PCI_BAR   0
+#define IMGU_DMA_MASK  DMA_BIT_MASK(39)
+#define IMGU_MAX_QUEUE_DEPTH   (2 + 2)
+
+static const struct imgu_node_mapping imgu_node_map[IMGU_NODE_NUM] = {
+   [IMGU_NODE_IN] = {IPU3_CSS_QUEUE_IN, "input"},
+   [IMGU_NODE_PARAMS] = {IPU3_CSS_QUEUE_PARAMS, "parameters"},
+   [IMGU_NODE_OUT] = {IPU3_CSS_QUEUE_OUT, "output"},
+   [IMGU_NODE_VF] = {IPU3_CSS_QUEUE_VF, "viewfinder"},
+   [IMGU_NODE_PV] = {IPU3_CSS_QUEUE_VF, "postview"},
+   [IMGU_NODE_STAT_3A] = {IPU3_CSS_QUEUE_STAT_3A, "3a stat"},
+   [IMGU_NODE_STAT_DVS] = {IPU3_CSS_QUEUE_STAT_DVS, "dvs stat"},
+   [IMGU_NODE_STAT_LACE] = {IPU3_CSS_QUEUE_STAT_LACE, "lace stat"},
+};
+
+int imgu_node_to_queue(int node)
+{
+   return imgu_node_map[node].css_queue;
+}
+
+int imgu_map_node(struct imgu_device *imgu, int css_queue)
+{
+   unsigned int i;
+
+   if (css_queue == IPU3_CSS_QUEUE_VF)
+   return imgu->mem2mem2.nodes[IMGU_NODE_VF].enabled ?
+   IMGU_NODE_VF : IMGU_NODE_PV;
+
+   for (i = 0; i < IMGU_NODE_NUM; i++)
+   if (imgu_node_map[i].css_queue == css_queue)
+   return i;
+
+   return -EINVAL;
+}
+
+/ Dummy buffers /
+
+static void imgu_dummybufs_cleanup(struct imgu_device *imgu)
+{
+   unsigned int i;
+
+   for (i = 0; i < IPU3_CSS_QUEUES; i++)
+   ipu3_dmamap_free(imgu, >queues[i].dmap);
+}
+
+static int imgu_dummybufs_init(struct imgu_device *imgu)
+{
+   const struct v4l2_pix_format_mplane *mpix;
+   const struct v4l2_meta_format   *meta;
+   size_t size;
+   unsigned int i, j;
+   int node;
+
+   /* Allocate a 

[PATCH v4 03/12] intel-ipu3: Add IOMMU based dmamap support

[Yong Zhi]

From: Tomasz Figa 

This patch adds driver to support IPU3-specific
MMU-aware memory alloc/free and sg mapping functions.

Signed-off-by: Tomasz Figa 
Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/Kconfig   |   7 +
 drivers/media/pci/intel/ipu3/Makefile  |   2 +-
 drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 342 +
 drivers/media/pci/intel/ipu3/ipu3-dmamap.h |  33 +++
 4 files changed, 383 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h

diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
b/drivers/media/pci/intel/ipu3/Kconfig
index 46ff138f3e50..d7dab52dc881 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -26,3 +26,10 @@ config INTEL_IPU3_MMU
---help---
  For IPU3, this option enables its MMU driver to translate its internal
  virtual address to 39 bits wide physical address for 64GBytes space 
access.
+
+config INTEL_IPU3_DMAMAP
+   tristate
+   default n
+   select IOMMU_IOVA
+   ---help---
+ This is IPU3 IOMMU domain specific DMA driver.
diff --git a/drivers/media/pci/intel/ipu3/Makefile 
b/drivers/media/pci/intel/ipu3/Makefile
index 91cac9cb7401..651773231496 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -13,4 +13,4 @@
 
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
 obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
-
+obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c 
b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
new file mode 100644
index ..e54bd9dfa302
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
@@ -0,0 +1,342 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ipu3-css-pool.h"
+#include "ipu3.h"
+
+/*
+ * Based on arch/arm64/mm/dma-mapping.c, with simplifications possible due
+ * to driver-specific character of this file.
+ */
+
+static int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+{
+   int prot = coherent ? IOMMU_CACHE : 0;
+
+   switch (dir) {
+   case DMA_BIDIRECTIONAL:
+   return prot | IOMMU_READ | IOMMU_WRITE;
+   case DMA_TO_DEVICE:
+   return prot | IOMMU_READ;
+   case DMA_FROM_DEVICE:
+   return prot | IOMMU_WRITE;
+   default:
+   return 0;
+   }
+}
+
+/*
+ * Free a buffer allocated by ipu3_dmamap_alloc_buffer()
+ */
+static void ipu3_dmamap_free_buffer(struct page **pages,
+   size_t size)
+{
+   int count = size >> PAGE_SHIFT;
+
+   while (count--)
+   __free_page(pages[count]);
+   kvfree(pages);
+}
+
+/*
+ * Based on the implementation of __iommu_dma_alloc_pages()
+ * defined in drivers/iommu/dma-iommu.c
+ */
+static struct page **ipu3_dmamap_alloc_buffer(size_t size,
+ unsigned long order_mask,
+ gfp_t gfp)
+{
+   struct page **pages;
+   unsigned int i = 0, count = size >> PAGE_SHIFT;
+   const gfp_t high_order_gfp = __GFP_NOWARN | __GFP_NORETRY;
+
+   /* Allocate mem for array of page ptrs */
+   pages = kvmalloc_array(count, sizeof(struct page *), GFP_KERNEL);
+
+   if (!pages)
+   return NULL;
+
+   order_mask &= (2U << MAX_ORDER) - 1;
+   if (!order_mask)
+   return NULL;
+
+   gfp |= __GFP_NOWARN | __GFP_HIGHMEM | __GFP_ZERO;
+
+   while (count) {
+   struct page *page = NULL;
+   unsigned int order_size;
+
+   for (order_mask &= (2U << __fls(count)) - 1;
+order_mask; order_mask &= ~order_size) {
+   unsigned int order = __fls(order_mask);
+
+   order_size = 1U << order;
+   page = alloc_pages((order_mask - order_size) ?
+  gfp | high_order_gfp : gfp, order);
+   if (!page)
+   continue;
+   if (!order)
+   break;
+   if (!PageCompound(page)) {
+   

[PATCH v4 02/12] intel-ipu3: Add mmu driver

[Yong Zhi]

From: Tomasz Figa 

IPU3 is capable to deal with a virtual address space with
a dedicated MMU. The driver supports address translation
from virtual(IPU3 internal) to 39 bit wide physical(system).

Build has dependency on exported symbols from:



Signed-off-by: Tomasz Figa 
Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/Kconfig|   9 +
 drivers/media/pci/intel/ipu3/Makefile   |  15 +
 drivers/media/pci/intel/ipu3/ipu3-mmu.c | 580 
 drivers/media/pci/intel/ipu3/ipu3-mmu.h |  26 ++
 4 files changed, 630 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h

diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
b/drivers/media/pci/intel/ipu3/Kconfig
index 0861077a4dae..46ff138f3e50 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -17,3 +17,12 @@ config VIDEO_IPU3_CIO2
Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
connected camera.
The module will be called ipu3-cio2.
+
+config INTEL_IPU3_MMU
+   tristate
+   default n
+   select IOMMU_API
+   select IOMMU_IOVA
+   ---help---
+ For IPU3, this option enables its MMU driver to translate its internal
+ virtual address to 39 bits wide physical address for 64GBytes space 
access.
diff --git a/drivers/media/pci/intel/ipu3/Makefile 
b/drivers/media/pci/intel/ipu3/Makefile
index 20186e3ff2ae..91cac9cb7401 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -1 +1,16 @@
+#
+#  Copyright (c) 2017, Intel Corporation.
+#
+#  This program is free software; you can redistribute it and/or modify it
+#  under the terms and conditions of the GNU General Public License,
+#  version 2, as published by the Free Software Foundation.
+#
+#  This program is distributed in the hope it will be useful, but WITHOUT
+#  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+#  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+#  more details.
+#
+
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
+obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
+
diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c 
b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
new file mode 100644
index ..05d001319aca
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
@@ -0,0 +1,580 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "ipu3-mmu.h"
+
+#define IMGU_DMA_NAME  "ipu3-imgu-dma"
+#define IMGU_DMA_BUS_NAME  "ipu3-imgu-bus"
+
+#define IPU3_PAGE_SHIFT12
+#define IPU3_PAGE_SIZE (1UL << IPU3_PAGE_SHIFT)
+
+#define IPU3_PT_BITS   10
+#define IPU3_PT_PTES   (1UL << IPU3_PT_BITS)
+#define IPU3_PT_SIZE   (IPU3_PT_PTES << 2)
+#define IPU3_PT_ORDER  (IPU3_PT_SIZE >> PAGE_SHIFT)
+
+#define IPU3_ADDR2PTE(addr)((addr) >> IPU3_PAGE_SHIFT)
+#define IPU3_PTE2ADDR(pte) ((phys_addr_t)(pte) << IPU3_PAGE_SHIFT)
+
+#define IPU3_L2PT_SHIFTIPU3_PT_BITS
+#define IPU3_L2PT_MASK ((1UL << IPU3_L2PT_SHIFT) - 1)
+
+#define IPU3_L1PT_SHIFTIPU3_PT_BITS
+#define IPU3_L1PT_MASK ((1UL << IPU3_L1PT_SHIFT) - 1)
+
+#define IPU3_MMU_ADDRESS_BITS  (IPU3_PAGE_SHIFT + \
+IPU3_L2PT_SHIFT + \
+IPU3_L1PT_SHIFT)
+
+#define IMGU_REG_BASE  0x4000
+#define REG_TLB_INVALIDATE (IMGU_REG_BASE + 0x300)
+#define TLB_INVALIDATE 1
+#define REG_L1_PHYS(IMGU_REG_BASE + 0x304) /* 27-bit pfn */
+#define REG_GP_HALT(IMGU_REG_BASE + 0x5dc)
+#define REG_GP_HALTED  (IMGU_REG_BASE + 0x5e0)
+
+struct ipu3_mmu_domain {
+   struct iommu_domain domain;
+   struct ipu3_mmu *mmu;
+};
+
+struct ipu3_mmu {
+   struct bus_type bus;
+   struct device *dev;
+   struct device dma_dev;
+   void __iomem *base;
+   struct iommu_group *group;
+   spinlock_t lock;
+
+   void *dummy_page;
+   u32 dummy_page_pteval;
+
+   u32 *dummy_l2pt;
+   u32 dummy_l2pt_pteval;
+
+   u32 **l2pts;
+   u32 *l1pt;
+};
+
+static inline struct ipu3_mmu_domain *
+to_ipu3_mmu_domain(struct 

[PATCH v4 00/12] Intel IPU3 ImgU patchset

[Yong Zhi]

This patchset adds support for the Intel IPU3 (Image Processing Unit)
ImgU which is essentially a modern memory-to-memory ISP. It implements
raw Bayer to YUV image format conversion as well as a large number of
other pixel processing algorithms for improving the image quality.

Meta data formats are defined for image statistics (3A, i.e. automatic
white balance, exposure and focus, histogram and local area contrast
enhancement) as well as for the pixel processing algorithm parameters.
The documentation for these formats is currently not included in the
patchset but will be added in a future version of this set.

The algorithm parameters need to be considered specific to a given frame
and typically a large number of these parameters change on frame to frame
basis. Additionally, the parameters are highly structured (and not a flat
space of independent configuration primitives). They also reflect the
data structures used by the firmware and the hardware. On top of that,
the algorithms require highly specialized user space to make meaningful
use of them. For these reasons it has been chosen video buffers to pass
the parameters to the device.

On individual patches:

The heart of ImgU is the CSS, or Camera Subsystem, which contains the
image processors and HW accelerators.

The 3A statistics and other firmware parameter computation related
functions are implemented in patch 8.

All h/w programming related code can be found in patch 9.

To access DDR via ImgU's own memory space, IPU3 is also equipped with
its own MMU unit, the driver is implemented in patch 2.

Currently, the MMU driver has dependency on two exported symbols
(iommu_group_ref_get and iommu_group_get_for_dev))to build as ko.

Patch 3 uses above IOMMU driver for DMA mem related functions.

Patch 5-10 are basically IPU3 CSS specific implementations:

6 and 7 provide some utility functions and manage IPU3 fw download and
install.

The firmware which is called ipu3-fw.bin can be downloaded from:

git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
(commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f)

Patch 9 and 10 are of the same file, the latter implements interface
functions for access fw & hw capabilities defined in patch 8.

Patch 11 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work:




Patch 12 uses Kconfig and Makefile created by IPU3 cio2 patch series.

Link to user space implementation:



Device topology:

./media-ctl -d /dev/media0 -p   
Media controller API version 4.14.0

Media device information

driver  ipu3-imgu
model   ipu3-imgu
serial  
bus info:00:05.0
hw revision 0x0
driver version  4.14.0

Device topology
- entity 1: ipu3-imgu:0 (8 pads, 8 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
<- "input":0 [ENABLED,IMMUTABLE]
pad1: Sink
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
<- "parameters":0 []
pad2: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "output":0 []
pad3: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "viewfinder":0 []
pad4: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "postview":0 []
pad5: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "3a stat":0 []
pad6: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "dvs stat":0 []
pad7: Source
[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown]
-> "lace stat":0 []

- entity 12: input (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video0
pad0: Source
-> "ipu3-imgu:0":0 [ENABLED,IMMUTABLE]

- entity 18: parameters (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video1
pad0: Source
-> "ipu3-imgu:0":1 []

- entity 24: output (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video2
pad0: Sink
<- "ipu3-imgu:0":2 []

- entity 30: viewfinder (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video3
pad0: Sink
<- "ipu3-imgu:0":3 []

- entity 36: postview (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video4
   

Re: [RFCv2 PATCH 12/36] dt-bindings: document stall and PASID properties for IOMMU masters

[Rob Herring]

On Mon, Oct 16, 2017 at 5:23 AM, Jean-Philippe Brucker
 wrote:
> On 13/10/17 20:10, Rob Herring wrote:
>> On Fri, Oct 06, 2017 at 02:31:39PM +0100, Jean-Philippe Brucker wrote:
>>> On ARM systems, some platform devices behind an IOMMU may support stall
>>> and PASID features. Stall is the ability to recover from page faults and
>>> PASID offers multiple process address spaces to the device. Together they
>>> allow to do paging with a device. Let the firmware tell us when a device
>>> supports stall and PASID.
>>
>> Can't these be implied by the compatible string of the devices?
>
> I think that PASID capacity can be deduced from the compatible string but
> don't know how these devices will be implemented (the only known example
> being a software model for testing). In any case implementing PASID based
> on the compatible string is tricky, because the IOMMU driver needs to know
> PASID capacity before the device driver has had time to probe the device.
> Maybe we could get away with a static table associating compatible string
> to PASID capacity, but it's not very nice.
>
> For stall it's a property of the integration between device and IOMMU,
> much like the "iommus" property, so it can't be deduced only from the
> compatible string. It's crucial that the firmware validates that stalling
> is safe, because we don't have any other way to discover it.

Okay, fair enough. I'll let others comment.

Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 5/9] PCI: host: brcmstb: add dma-ranges for inbound traffic

[Jim Quinlan]

On Tue, Oct 17, 2017 at 4:14 AM, Christoph Hellwig  wrote:
> Just took a quick look over this and I basically agree with the comments
> from Robin.
>
> What I don't understand is why you're even trying to do all these
> hacky things.
>
> It seems like the controller should simply set dma_pfn_offset for
> each device hanging off it, and all the supported architectures
> should be updated to obey that if they don't do so yet, and
> you're done without needing this giant mess.

My understanding is that dma_pfn_offset is that it is a single
constant offset from RAM, in our case, to map to PCIe space.  But in
my commit message I detail how our PCIe controller presents memory
with multiple regions with multiple different offsets. If an EP device
maps to a region on the host memory, yes we can set the dma_pfn_offset
for that device for that location within that range,.  But if the
device then unmaps and allocates from another region with a different
offset, it won't work.  If  I set dma_pfn_offset I have to assume that
the device is using only one region of memory only, not more than one,
and that it is not unmapping that region and mapping another (with a
different offset).  Can I make those assumptions?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu: Limit the IOVA page range to the specified addresses

[Gary R Hook]

From: amd 

The extent of pages specified when applying a reserved region should
include up to the last page of the range, but not the page following
the range.

Signed-off-by: Gary R Hook 
---
 drivers/iommu/amd_iommu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 27eb0d6fcdc2..bb3ef33e3784 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3271,7 +3271,7 @@ static void amd_iommu_apply_resv_region(struct device 
*dev,
unsigned long start, end;
 
start = IOVA_PFN(region->start);
-   end   = IOVA_PFN(region->start + region->length);
+   end   = IOVA_PFN(region->start + region->length - 1);
 
WARN_ON_ONCE(reserve_iova(_dom->iovad, start, end) == NULL);
 }

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH -next] iommu/ipmmu-vmsa: Fix return value check in ipmmu_find_group_dma()

[Wei Yongjun]

In case of error, the function iommu_group_get() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should be
replaced with NULL test.

Fixes: 3ae47292024f ("iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops")
Signed-off-by: Wei Yongjun 
---
 drivers/iommu/ipmmu-vmsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index af81400..00e88a8 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -871,7 +871,7 @@ static struct iommu_group *ipmmu_find_group_dma(struct 
device *dev)
sibling = ipmmu_find_sibling_device(dev);
if (sibling)
group = iommu_group_get(sibling);
-   if (!sibling || IS_ERR(group))
+   if (!sibling || !group)
group = generic_device_group(dev);
 
return group;

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] xhci: Set DMA parameters appropriately

[Marek Szyprowski]

Hi Robin,

On 2017-10-13 12:48, Robin Murphy wrote:

On 13/10/17 09:15, Marek Szyprowski wrote:

On 2017-10-11 15:56, Robin Murphy wrote:

xHCI requires that data buffers do not cross 64KB boundaries (and are
thus at most 64KB long as well) - whilst xhci_queue_{bulk,isoc}_tx()
already split their input buffers into individual TRBs as necessary,
it's still a good idea to advertise the limitations via the standard DMA
API mechanism, so that most producers like the block layer and the DMA
mapping implementations can lay things out correctly to begin with.

Signed-off-by: Robin Murphy 
---
   drivers/usb/host/xhci.c | 4 
   drivers/usb/host/xhci.h | 3 +++
   2 files changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 74b4500641c2..1e7e1e3d8c48 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4883,6 +4883,10 @@ int xhci_gen_setup(struct usb_hcd *hcd,
xhci_get_quirks_t get_quirks)
   dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
   }
   +    dev->dma_parms = >dma_parms;
+    dma_set_max_seg_size(dev, SZ_64K);
+    dma_set_seg_boundary(dev, SZ_64K - 1);
+
   xhci_dbg(xhci, "Calling HCD init\n");
   /* Initialize HCD and host controller data structures. */
   retval = xhci_init(hcd);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7ef69ea0b480..afcae4cc908d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1767,6 +1767,9 @@ struct xhci_hcd {
   struct dma_pool    *small_streams_pool;
   struct dma_pool    *medium_streams_pool;
   +    /* DMA alignment restrictions */
+    struct device_dma_parameters dma_parms;
+
   /* Host controller watchdog timer structures */
   unsigned int    xhc_state;
   

Are you sure that xhci_hcd life time is proper to keep dma_parms? It looks
that when driver gets removed and xhci_hcd is freed, the dma_parms will
point to freed memory. Maybe it would make sense to clear dev->dma_parms
somewhere or definitely change the way dma_parms are allocated?

AFAICS it lives until the last usb_put_hcd() call, which is pretty much
the last thing in the drivers' .remove paths, so it looks to follow the
standard paradigm evidenced by other dma_parms users. Any dangling
pointer after the driver has been unbound will be reinitialised by a
subsequent probe, and anyone using an unbound device for DMA API calls
is very wrong anyway.


Okay.


On the other hand 64K is the default segment size if no dma_parms are
provided, so there is very little value added by this patch.

I prefer to explicitly set the segment size for cleanliness and to
emphasize the difference between "whatever the default value is is fine"
and "we have a real hardware limit of 64K". What really matters here
though is the non-default segment boundary mask - that's the motiviation
for the patch.


Okay. What about other type of USB HCD (EHCI, OHCI, UHCI)?

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] drivers: Flag buses which demand DMA configuration

[Greg KH]

On Tue, Oct 17, 2017 at 10:17:01AM +0200, Christoph Hellwig wrote:
> Looks fine:
> 
> Reviewed-by: Christoph Hellwig 
> 
> Greg, do you want to take this or should I queue it up in the
> dma-mapping tree?

You can take it, thanks!

Acked-by: Greg Kroah-Hartman 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] drivers: Flag buses which demand DMA configuration

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig 

Greg, do you want to take this or should I queue it up in the
dma-mapping tree?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 5/9] PCI: host: brcmstb: add dma-ranges for inbound traffic

[Christoph Hellwig]

Just took a quick look over this and I basically agree with the comments
from Robin.

What I don't understand is why you're even trying to do all these
hacky things.

It seems like the controller should simply set dma_pfn_offset for
each device hanging off it, and all the supported architectures
should be updated to obey that if they don't do so yet, and
you're done without needing this giant mess.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] mmc: renesas_sdhi: fix swiotlb buffer is full

[Geert Uytterhoeven]

Hi Shimoda-san,

CC iommu

On Tue, Oct 17, 2017 at 9:30 AM, Yoshihiro Shimoda
 wrote:
> Since the commit de3ee99b097d ("mmc: Delete bounce buffer handling")
> deletes the bounce buffer handling, a request data size will be referred
> to max_{req,seg}_size instead of MMC_QUEUE_BOUNCESZ (64k bytes).
>
> In other hand, renesas_sdhi_internal_dmac.c will set very big value of
> max_{req,seg}_size because the max_blk_count is set to 0x.
> And then, "swiotlb buffer is full" happens because swiotlb can handle
> a memory size up to 256k bytes only (IO_TLB_SEGSIZE = 128 and
> IO_TLB_SHIFT = 11).
>
> So, this patch fixes the issue to set max_blk_count to 512. Then,
> the max_{req,seg}_size will be set to 256k bytes.
>
> Reported-by: Dirk Behme 
> Signed-off-by: Yoshihiro Shimoda 

Thanks for your patch!

> ---
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
> b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> index f905f23..6c9b4b2 100644
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -80,8 +80,9 @@
> .scc_offset = 0x1000,
> .taps   = rcar_gen3_scc_taps,
> .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
> -   /* Gen3 SDHI DMAC can handle 0x blk count, but seg = 1 */
> -   .max_blk_count  = 0x,
> +   /* The swiotlb can handle memory size up to 256 kbytes for now. */
> +   .max_blk_count  = 512,

Fixing this in the individual drivers feels like the wrong solution to me.

iommu: Is there a better (generic) way to handle this?

> +   /* Gen3 SDHI DMAC cannot handle scatter-gather. So, max_segs = 1 */
> .max_segs   = 1,
>  };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu