| Message ID | 20180420095933.16442-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 04/20/2018 03:52 PM, Patchwork wrote: > == Series Details == > > Series: drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers (rev3) > URL : https://patchwork.freedesktop.org/series/40929/ > State : failure > > == Summary == > > = CI Bug Log - changes from CI_DRM_4072_full -> Patchwork_8763_full = > > == Summary - FAILURE == > > Serious unknown changes coming with Patchwork_8763_full absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_8763_full, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: https://patchwork.freedesktop.org/api/1.0/series/40929/revisions/3/mbox/ > > == Possible new issues == > > Here are the unknown changes that may have been introduced in Patchwork_8763_full: > > === IGT changes === > > ==== Possible regressions ==== > > igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible: > shard-apl: PASS -> FAIL This seems to be a false positive, so unless someone objects I plan to push this change soonish. Regards, Hans > ==== Warnings ==== > > igt@gem_exec_schedule@deep-blt: > shard-kbl: PASS -> SKIP +1 > > igt@gem_exec_schedule@deep-bsd1: > shard-kbl: SKIP -> PASS +1 > > igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping: > shard-glk: PASS -> SKIP +95 > > igt@kms_vblank@pipe-b-wait-forked-busy-hang: > shard-glk: SKIP -> PASS +104 > > > == Known issues == > > Here are the changes found in Patchwork_8763_full that come from known issues: > > === IGT changes === > > ==== Issues hit ==== > > igt@kms_flip@2x-flip-vs-expired-vblank: > shard-hsw: PASS -> FAIL (fdo#102887) > > igt@kms_flip@2x-wf_vblank-ts-check: > shard-hsw: PASS -> FAIL (fdo#100368) > > igt@pm_rpm@drm-resources-equal: > shard-kbl: PASS -> DMESG-FAIL (fdo#103558, fdo#104767) > > ==== Possible fixes ==== > > igt@kms_flip@dpms-vs-vblank-race-interruptible: > shard-glk: FAIL (fdo#103060) -> PASS > > igt@kms_flip@plain-flip-fb-recreate: > shard-hsw: FAIL (fdo#100368) -> PASS > > igt@kms_flip@plain-flip-ts-check-interruptible: > shard-glk: FAIL (fdo#100368) -> PASS +1 > > igt@kms_setmode@basic: > shard-glk: FAIL (fdo#99912) -> PASS > shard-kbl: FAIL (fdo#99912) -> PASS > > igt@perf@blocking: > shard-hsw: FAIL (fdo#102252) -> PASS > > > fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 > fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 > fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 > fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 > fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 > fdo#104767 https://bugs.freedesktop.org/show_bug.cgi?id=104767 > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > > == Participating hosts (6 -> 5) == > > Missing (1): shard-glkb > > > == Build changes == > > * Linux: CI_DRM_4072 -> Patchwork_8763 > > CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_8763: 4aa49825b5adc90cab8a53d154aa779168348a6c @ git://anongit.freedesktop.org/gfx-ci/linux > piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8763/shards.html >
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index af915d041281..ad949cc30928 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -51,6 +51,10 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv, if (!drm_mm_initialized(&dev_priv->mm.stolen)) return -ENODEV; + /* WaSkipStolenMemoryFirstPage:bdw+ */ + if (INTEL_GEN(dev_priv) >= 8 && start < 4096) + start = 4096; + mutex_lock(&dev_priv->mm.stolen_lock); ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size, alignment, 0, @@ -343,7 +347,6 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv) { resource_size_t reserved_base, stolen_top; resource_size_t reserved_total, reserved_size; - resource_size_t stolen_usable_start; mutex_init(&dev_priv->mm.stolen_lock); @@ -435,17 +438,11 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv) (u64)resource_size(&dev_priv->dsm) >> 10, ((u64)resource_size(&dev_priv->dsm) - reserved_total) >> 10); - stolen_usable_start = 0; - /* WaSkipStolenMemoryFirstPage:bdw+ */ - if (INTEL_GEN(dev_priv) >= 8) - stolen_usable_start = 4096; - dev_priv->stolen_usable_size = - resource_size(&dev_priv->dsm) - reserved_total - stolen_usable_start; + resource_size(&dev_priv->dsm) - reserved_total; /* Basic memrange allocator for stolen space. */ - drm_mm_init(&dev_priv->mm.stolen, stolen_usable_start, - dev_priv->stolen_usable_size); + drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->stolen_usable_size); return 0; }
Before this commit the WaSkipStolenMemoryFirstPage workaround code was skipping the first 4k by passing 4096 as start of the address range passed to drm_mm_init(). This means that calling drm_mm_reserve_node() to try and reserve the firmware framebuffer so that we can inherit it would always fail, as the firmware framebuffer starts at address 0. Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory on everything >= gen8") says in its commit message: "This is confirmed to fix Skylake screen flickering issues (probably caused by the fact that we initialized a ring in the first page of stolen, but I didn't 100% confirm this theory)." Which suggests that it is safe to use the first page for a linear framebuffer as the firmware is doing (see note below). This commit always passes 0 as start to drm_mm_init() and works around WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range() by insuring the start address passed by to drm_mm_insert_node_in_range() is always 4k or more. All entry points to i915_gem_stolen.c go through i915_gem_stolen_insert_node_in_range(), so that any newly allocated objects such as ring-buffers will not be allocated in the first 4k. The one exception is i915_gem_object_create_stolen_for_preallocated() which directly calls drm_mm_reserve_node() which now will be able to use the first 4k. This fixes the i915 driver no longer being able to inherit the firmware framebuffer on gen8+, which fixes the video output changing from the vendor logo to a black screen as soon as the i915 driver is loaded (on systems without fbcon). Some notes about the mapping of the BIOS framebuffer: v1 led to some discussion if the assumption of the intel_display.c code that the firmware framebuffer is a linear mapping of the stolen memory starting at offset 0 is still correct, because that would mean that the GOP does not implement the WaSkipStolenMemoryFirstPage workaround. To verify this the following code was added at the end of i915_gem_object_create_stolen_for_preallocated() : pr_err("first ggtt entry before bind: 0x%016llx\n", readq(dev_priv->ggtt.gsm)); ret = i915_vma_bind(vma, HAS_LLC(dev_priv) ? I915_CACHE_LLC : I915_CACHE_NONE, PIN_UPDATE); pr_err("i915_vma_bind ret %d\n", ret); pr_err("first ggtt entry after bind: 0x%016llx\n", readq(dev_priv->ggtt.gsm)); Which prints the mapping of the first page, then does a vma_bind() to force update the mapping with our linear view of the framebuffer and then prints the mapping of the first page again. On an Asrock B150M Pro4S/D3 mainboard with i5-6500 CPU this prints: [ 1.651141] first ggtt entry before bind: 0x0000000078c00001 [ 1.651151] i915_vma_bind ret 0 [ 1.651152] first ggtt entry after bind: 0x0000000078c00083 And "sudo cat /proc/iomem | grep Stolen" gives: 78c00000-88bfffff : Graphics Stolen Memory There are no visual changes with this patch (BIOS vendor logo still stays in place when we inherit the BIOS framebuffer), so the vma_bind() does not impact which memory is being scanned out. The address of the first ggtt entry matches with the start of stolen and the i915_vma_bind call only changes the first gtt entry's flags, or-ing in _PAGE_RW (BIT(1)) and PPAT_CACHED (BIT(7)), which perfectly matches what we would expect based on gen8_pte_encode()'s behavior. So it seems that the GOP indeed does NOT implement the wa and the i915's code assuming a linear mapping at the start of stolen for the BIOS fb still holds true for gen8+. I've also tested this on a Cherry Trail based device (a GPD Win) with identical results (the flags are 0x1b after the vma_bind on CHT, which matches with I915_CACHE_NONE). Changed in v2: No code changes, extended the commit message with the verification that the intel_display.c BIOS framebuffer mapping is still correct. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)