New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 602675 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
I'm out of this project
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 641392



Sign in to add a comment

"Failed DMA_BUF_SYNC_END" error in chrome log file

Reported by tiago.vi...@intel.com, Apr 12 2016

Issue description


Some recent CrOS systems that have --enable-native-gpu-memory-buffers activated using the new dma-buf API (in particular the ioctls for cache flush, https://codereview.chromium.org/1841683003) are logging messages like the following:

[581:721:0412/133301:ERROR:client_native_pixmap_dmabuf.cc(51)] Failed DMA_BUF_SYNC_END: Invalid argument

Systems with kernel version 3.14 (and possible the other lower versions as well) emit this because they try to set an object back to the GTT space, when this object is unbound. I found out that CrOS systems with kernel 3.18 don't have such issue though because they have the following patchset applied: 

http://comments.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/51828
 
a couple of more thoughts:

0. DMA_BUF_SYNC_END ioctl sets back the obj into the gtt domain

1. the problem we're seeing is all about having an unbound obj in the gtt domain and old i915s don't actually have support for it.

2. Chromium error message seems fearless though. The driver looks resilient because after the obj gets actually bound, things seems to work like expected i.e. no artifacts whatsoever on the screen and DMA_BUF_SYNC_END returns fine in the subsequent calls in that same obj.

3. unbound obj in the gtt roughly means performance improvements for WC mmap (check "drm/i915: Support creation of unbound wc user mappings for objects"), 
which is not particularly our interest here cause we're using CPU mmap. It doesn't mean either that, by fixing this issue, the performance wouldn't be improved.

4. it's too risky to touch anything in CrOS 3.14's i915 driver: to get clflush and object domain right is quite tricky; you touch one thing, you break another thing. Besides, the commit diff between this old i915.ko driver (3.14) and the one that actually brings unbound obj support is huge. We're talking about over 1-year development difference here + hundreds of Chromium only patches (patches tagged with CHROMIUM and BACKPORT). Cherry-pick that fix obviously doesn't work.


Therefore, I'm inclined to simply shut up that Chromium error message. Comments?

Cc: ch...@chris-wilson.co.uk dan...@ffwll.ch
Chris, Daniel PTAL.
It's complicated issue.
Before we find good solution, we should use DPLOG(ERROR) instead of PLOG(ERROR) so that end users don't see the verbose error message.
@1: in principle we have nothing against backporting fixes to our older kernels, and we are ok with large changes (for example we have atomic support in our older kernels). If that's the right thing to do, I would vote for doing it, since that's usually what's best in the long term.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 19 2016

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6b1c085ae9cffc987b6d2e8258ff0304ec4e470f

commit 6b1c085ae9cffc987b6d2e8258ff0304ec4e470f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Apr 14 20:08:00 2016

BACKPORT: drm/i915: Broaden application of set-domain(GTT)

Previously, this was restricted to only operate on bound objects - to
make pointer access through the GTT to the object coherent with writes
to and from the GPU. A second usecase is drm_intel_bo_wait_rendering()
which at present does not function unless the object also happens to
be bound into the GGTT (on current systems that is becoming increasingly
rare, especially for the typical requests from mesa). A third usecase is
a future patch wishing to extend the coverage of the GTT domain to
include objects not bound into the GGTT but still in its coherent cache
domain. For the latter pair of requests, we need to operate on the
object regardless of its bind state.

v2: After discussion with Akash, we came to the conclusion that the
get-pages was required in order for accurate domain tracking in the
corner cases (like the shrinker) and also useful for ensuring memory
coherency with earlier cached CPU mmaps in case userspace uses exotic
cache bypass (non-temporal) instructions.

v3: Fix the inactive object check.

v4: Rebase to latest drm-intel-nightly codebase

BUG= chromium:602675 
TEST=build chromeos-kernel-3_14 on samus, open Chrome and browser around.
Checked also that "Failed DMA_BUF_SYNC_END" error in not anymore in chrome log
file. Besides I ran (upstream) igt `prime_mmap_coherency --run-subtest
ioctl-errors`, which was failing before this patch.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Akash Goel <akash.goel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit 43566dedde54f9729113f5f9fde77d53e75e61e9)
Signed-off-by: Marc Herbert <marc.herbert@intel.com>

Change-Id: I2bd115d87d6ba3bad0fbe352e91a647aaf4aafdc
Reviewed-on: https://chromium-review.googlesource.com/339030
Commit-Ready: Tiago Vignatti <tiago.vignatti@intel.com>
Tested-by: Tiago Vignatti <tiago.vignatti@intel.com>
Reviewed-by: Tiago Vignatti <tiago.vignatti@intel.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/6b1c085ae9cffc987b6d2e8258ff0304ec4e470f/drivers/gpu/drm/i915/i915_gem.c

Comment 6 by h...@chromium.org, Apr 23 2016

Re:#5: this patch seems to cause tons of warnings in i915_gem_obj_to_ggtt on samus. See  bug 605774 
Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 26 2016

Labels: merge-merged-release-R51-8172.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/93ae20e87133c3b7ea46507d273cd19371ca7e2d

commit 93ae20e87133c3b7ea46507d273cd19371ca7e2d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Apr 14 20:08:00 2016

BACKPORT: drm/i915: Broaden application of set-domain(GTT)

Previously, this was restricted to only operate on bound objects - to
make pointer access through the GTT to the object coherent with writes
to and from the GPU. A second usecase is drm_intel_bo_wait_rendering()
which at present does not function unless the object also happens to
be bound into the GGTT (on current systems that is becoming increasingly
rare, especially for the typical requests from mesa). A third usecase is
a future patch wishing to extend the coverage of the GTT domain to
include objects not bound into the GGTT but still in its coherent cache
domain. For the latter pair of requests, we need to operate on the
object regardless of its bind state.

v2: After discussion with Akash, we came to the conclusion that the
get-pages was required in order for accurate domain tracking in the
corner cases (like the shrinker) and also useful for ensuring memory
coherency with earlier cached CPU mmaps in case userspace uses exotic
cache bypass (non-temporal) instructions.

v3: Fix the inactive object check.

v4: Rebase to latest drm-intel-nightly codebase

BUG= chromium:602675 
TEST=build chromeos-kernel-3_14 on samus, open Chrome and browser around.
Checked also that "Failed DMA_BUF_SYNC_END" error in not anymore in chrome log
file. Besides I ran (upstream) igt `prime_mmap_coherency --run-subtest
ioctl-errors`, which was failing before this patch.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Akash Goel <akash.goel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit 43566dedde54f9729113f5f9fde77d53e75e61e9)
Signed-off-by: Marc Herbert <marc.herbert@intel.com>

Change-Id: I2bd115d87d6ba3bad0fbe352e91a647aaf4aafdc
Reviewed-on: https://chromium-review.googlesource.com/339030
Commit-Ready: Tiago Vignatti <tiago.vignatti@intel.com>
Tested-by: Tiago Vignatti <tiago.vignatti@intel.com>
Reviewed-by: Tiago Vignatti <tiago.vignatti@intel.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
(cherry picked from commit 6b1c085ae9cffc987b6d2e8258ff0304ec4e470f)
Signed-off-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/340840

[modify] https://crrev.com/93ae20e87133c3b7ea46507d273cd19371ca7e2d/drivers/gpu/drm/i915/i915_gem.c

Status: Verified (was: Fixed)
Bulk verified
Blocking: 641392

Sign in to add a comment