On Sat, Jun 22, 2013 at 03:22:59PM +0100, Chris Wilson wrote:

On Fri, Jun 21, 2013 at 10:03:43PM -0400, Konrad Rzeszutek Wilk wrote:

...

On Fri, Jun 21, 2013 at 03:28:28PM -0400, Konrad Rzeszutek Wilk wrote:

...

Hey,

CC-ing Imre,

Imre, your patch 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4 ("drm/i915: create compact dma scatter lists for gem objects") is the cause of the regression.

If I revert your patch it boots fine without any trouble.

I am not entirely sure why that is - as I added some debug code in lib/swiotlb.c to trigger when it can't find 3MB area (which is what I thought initially was the issue) - but none of the debug code seems to be hit.

Any thoughts?

You should be hitting drivers/iommu/intel-iommu.c for the dma translation. It looks like the contiguous 3MiB segment will be special as it is the first sg that __domain_mapping() will attempt to allocate a superpage (2MiB) for. What goes wrong at point, I am not sure, but I would suggest peppering intel-iommu.c with printk to track down the error.

I figured it out. The issue was that I am backed by the SWIOTLB which can only allocate up to 128*4K chunks of contingous bounce buffer (see IO_TLB_SEGSIZE) - which means it can only setup up to 512kB buffers. While one of the SG entries tries to give it one past that size (3MB).

The change Imre introduced assume that the CPU addresses (virtual) are the same as the bus addresses. That is correct in most platforms, but some (for example when booting a Xen PV guest with i915 as PCI passthrough) the virt_to_phys() values != bus address. Which means that the nice check of:

if (!i || page_to_pfn(page) != last_pfn + 1) { if (i) sg = sg_next(sg); st->nents++; sg_set_page(sg, page, PAGE_SIZE, 0); } else { sg->length += PAGE_SIZE; }

is too simplistic. What it ought to do is consult the DMA API whether the next PFN (page_to_pfn(page)) is really contingous in the DMA space. And also that it does got past the DMA mask for said device.

Unfortunatly such calls do not exist. Those checks are all done when dma_map_page_* is done (which is where it failed for me).

The best (for an rc7 stage) fix I came up with is to revert just a bit of the old behavior and still retain the sg compact code behavior. See following patch:

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 970ad17..9edd2eb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1801,7 +1801,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD; gfp &= ~(__GFP_IO | __GFP_WAIT); } - + if (swiotlb_nr_tbl()) { + st->nents++; + sg_set_page(sg, page, PAGE_SIZE, 0); + sg = sg_next(sg); + continue; + } if (!i || page_to_pfn(page) != last_pfn + 1) { if (i) sg = sg_next(sg); @@ -1813,7 +1818,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) last_pfn = page_to_pfn(page); }

- sg_mark_end(sg); + if (!swiotlb_nr_tbl()) + sg_mark_end(sg); obj->pages = st;

if (i915_gem_object_needs_bit17_swizzle(obj))

On Tue, Jun 25, 2013 at 11:03:01AM -0400, Jerome Glisse wrote:

On Fri, Jun 21, 2013 at 3:28 PM, Konrad Rzeszutek Wilk konrad.wilk@oracle.com wrote:

...

Hey,

I am using an ThinkPad X230 with an Intel HD 4000. With a stock Fedora 18 (3.9.6) I can get it to boot and work just fine with Xen. If I use v3.10-rc6 I found that i915 would halt with a

[drm:intel_pipe_set_base] *ERROR* pin & fence failed [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28

after a bit of debugging (see patch below) I traced it down to the fact that the scatter list that is provided at the end has a huge (3MB) page. I am wondering if anybody knows what patch might have introduced it to grab such a large memory segment?

The other thing I am wondering is if there are some fallbacks when the underlaying IOMMU can't deal with a request for contingous regions that are more than 2MB?

There is no fallback afaik, but most gpu have their own mmu so they don't really need contiguous iommu mapping, all they need is at the very least being able to access all page of large object. Probably something we should take a look at.

They can see. It is just that the underlaying DMA was not capable of dealing with a large chunk of the scatter gather.

Cheers, Jerome

...

Thanks.

From a681a4adb4738c32cb1acdf6f5161bf877816b01 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk konrad.wilk@oracle.com Date: Fri, 21 Jun 2013 11:17:55 -0400 Subject: [PATCH] dbug: print scatterlist.

------------[ cut here ]------------ WARNING: at drivers/gpu/drm/i915/i915_gem_gtt.c:418 i915_gem_gtt_prepare_object+0x180/0x200() 10 but got 0 Modules linked in: crc32_pclmul sdhci_pci crc32c_intel sdhci mmc_core ghash_clmulni_intel CPU: 0 PID: 216 Comm: plymouthd Not tainted 3.10.0-rc6+ #16 Hardware name: LENOVO 2325DV4/2325DV4, BIOS G2ET86WW (2.06 ) 11/13/2012 0000000000000009 ffff8801fa42d958 ffffffff816e6d89 ffff8801fa42d998 ffffffff8105d2b0 ffff8801fa42d988 0000000000000000 ffff8801fb0f4d80 ffffffff81c172e0 ffff8801fa76f000 000000000000000a ffff8801fa42d9f8 Call Trace: [<ffffffff816e6d89>] dump_stack+0x19/0x1b [<ffffffff8105d2b0>] warn_slowpath_common+0x70/0xa0 [<ffffffff8105d396>] warn_slowpath_fmt+0x46/0x50 [<ffffffff8142a740>] i915_gem_gtt_prepare_object+0x180/0x200 [<ffffffff81423581>] i915_gem_object_pin+0x321/0x670 [<ffffffff81423951>] i915_gem_object_pin_to_display_plane+0x81/0x190 [<ffffffff814381b5>] intel_pin_and_fence_fb_obj+0x85/0x1a0 [<ffffffff8143999c>] intel_pipe_set_base+0x7c/0x220 [<ffffffff814409be>] intel_crtc_set_config+0x89e/0x990 [<ffffffff813ffdae>] drm_mode_set_config_internal+0x2e/0x60 [<ffffffff814023ab>] drm_mode_setcrtc+0xfb/0x620 [<ffffffff811850f9>] ? kmem_cache_alloc_trace+0x39/0x1f0 [<ffffffff813f9767>] ? drm_vm_open_locked+0x57/0x90 [<ffffffff813f2e39>] drm_ioctl+0x549/0x680 [<ffffffff814022b0>] ? drm_mode_setplane+0x3b0/0x3b0 [<ffffffff811aef77>] do_vfs_ioctl+0x97/0x580 [<ffffffff81295dca>] ? inode_has_perm.isra.32.constprop.62+0x2a/0x30 [<ffffffff81297397>] ? file_has_perm+0x97/0xb0 [<ffffffff811af4f1>] SyS_ioctl+0x91/0xb0 [<ffffffff816f63e7>] tracesys+0xdd/0xe2 ---[ end trace 7b6adc5450d9a9e1 ]--- i915 0000:00:02.0: i915_gem_gtt_prepare_object: Mapping 10 pages, mapped: 0 [0] virT:ffff8801fd37c000 dma: 1fd37c000, size:4096 [1] virT:ffff8801fd37b000 dma: 1fd37b000, size:4096 [2] virT:ffff8801fd37a000 dma: 1fd37a000, size:4096 [3] virT:ffff8801fd378000 dma: 1fd378000, size:4096 [4] virT:ffff8801fd131000 dma: 1fd131000, size:4096 [5] virT:ffff880200c36000 dma: 200c36000, size:4096 [6] virT:ffff8801fd1a4000 dma: 1fd1a4000, size:69632 [7] virT:ffff8801fd3bb000 dma: 1fd3bb000, size:4096 [8] virT:ffff8801fd3c0000 dma: 1fd3c0000, size:262144 [9] virT:ffff8801f9400000 dma: 1f9400000, size:3866624 [drm] 3011: ret:-28 [drm] 3540: ret:-28 [drm] 3364: ret:-28 [drm:intel_pin_and_fence_fb_obj] *ERROR* i915_gem_object_pin_to_display_plane failed: -28 [drm:intel_pipe_set_base] *ERROR* pin & fence failed [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28

Signed-off-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

drivers/gpu/drm/i915/i915_drv.c | 6 ++++++ drivers/gpu/drm/i915/i915_gem_gtt.c | 28 +++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8411942..141c6fb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -133,6 +133,12 @@ module_param_named(coherent, use_coherent, int, 0600); MODULE_PARM_DESC(use_coherent, "Use coherent DMA API calls (default: false)");

+int i915_sgl __read_mostly = 0; +module_param_named(sgl, i915_sgl, int, 0600); +MODULE_PARM_DESC(sgl,

•            "Print scatterlist SG's when DMA mapping them (default: false)");
  

static struct drm_driver driver; extern int intel_agp_enabled;

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index acb3b3f..292179c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -28,6 +28,7 @@ #include "i915_trace.h" #include "intel_drv.h"

+extern int use_coherent; typedef uint32_t gen6_gtt_pte_t;

/* PPGTT stuff */ @@ -403,15 +404,36 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)

    i915_gem_chipset_flush(dev);

}

+extern int i915_sgl; int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj) {

•   int elem;
    if (obj->has_dma_mapping)
            return 0;
  
  

•   if (!dma_map_sg(&obj->base.dev->pdev->dev,
  

•   elem = dma_map_sg(&obj->base.dev->pdev->dev,
                    obj->pages->sgl, obj->pages->nents,
  

•                   PCI_DMA_BIDIRECTIONAL))
  

•                   PCI_DMA_BIDIRECTIONAL);
  

•   WARN(elem == 0, "%d but got %d", obj->pages->nents, elem);
  

•   if (i915_sgl && obj->pages && obj->pages->sgl) {
  

•           int i;
  

•           struct scatterlist *s;
  

•           if (obj->base.dev) {
  

•                   dev_info(&obj->base.dev->pdev->dev, "%s: Mapping %d pages, mapped: %d\n",
  

•                           __func__,obj->pages->nents, elem);
  

•           } else
  

•                   printk(KERN_INFO "%s: no dev, %d pages, mapped: %d\n",__func__,
  

•                           obj->pages->nents, elem);
  

•           for_each_sg(obj->pages->sgl, s, obj->pages->nents, i) {
  

•                   printk(KERN_INFO "[%d] virT:%lx dma: %lx, size:%d\n", i,
  

•                           (unsigned long)sg_virt(s), (unsigned long)sg_phys(s), s->length);
  

•           }
  

•   }
  

•   if (!elem)
            return -ENOSPC;
  
    return 0;
  

-- 1.8.1.4

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel