From patchwork Fri Apr 20 09:59:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 10352343 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 041EA60365 for ; Fri, 20 Apr 2018 09:59:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E80C6286DF for ; Fri, 20 Apr 2018 09:59:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DCC31286F8; Fri, 20 Apr 2018 09:59:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 32847286DF for ; Fri, 20 Apr 2018 09:59:42 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 07A236E1AD; Fri, 20 Apr 2018 09:59:42 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6883C6E1AD; Fri, 20 Apr 2018 09:59:40 +0000 (UTC) Received: by mail-wr0-x242.google.com with SMTP id p18-v6so2223025wrm.1; Fri, 20 Apr 2018 02:59:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=mGpIWRMOmRVpOwj4pE9+9x4BzsOHN8CIXVu8VcAYTV4=; b=Gtq9KDiOTvzhFRAWs3cKwk/TQraT+94A6tkFjPkFrpkrMvOuG0+2MHRYeWHemA+eDm wIvGOiAy6YcMMgh0OTeYkVa/cfRDGMT1Q2GLAzUrIDzoMRwlda4wHCJj7KFeSbJ4CoTy pQ7QK72ftJTSHNdhY5nvOn2hxFA660yWR/qZqpNJLgjOd/wE0rPhp5li/aaxh6FwXkXj uyDBOC7AdC2jxGYMHIpfzInlvB3AqoyI4w0H0Ea1Hmio31NI7Un/SUuHtD+yVBiSEYTD 8wlJhm/wNmUzHJNU4juntURr/m2zXDzxK1BExRg+pMR3h6oxNdFihnCiD2ITr2WR+mvb l6HQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=mGpIWRMOmRVpOwj4pE9+9x4BzsOHN8CIXVu8VcAYTV4=; b=Ea8qmpJM6eIB8xS5QiQpRZv4CF98g48c+LKE/rSmTFkwuJ+TuaixdKV8mZmLhUx+iQ itfBLC4PyhuqWuhCh2wJ44G4NVOWBprew4B6LoX8PvQmjHN+UuB3hDsbno7ygSuG0Drx ZlTpBLKvU43/r2aTQuT2ATSti7u2D08gnK3RswXDrR/+xDfAKlIfAT6q3Mc6KaWlCYs8 y/pxkglSY5h93cQIuI9oBPA2VzO6Rbuigb4LN1UWxyg7V7FG9IEbr4HCAbP2TIyc6v/E wgxVstBiBW+gtq0cX7E6F+cLirxrX60H0ol7MU5lJA19ImgttrfAC/swHyiJK++Ja71J 0Q3g== X-Gm-Message-State: ALQs6tCTVYJ/Ok3cZ5qg7Bm2ZTfxRnsYq5FkAfx2CzdxDfhBKRKQLP9E vStpMnq0y2VMdpTh4qrBo5oF6zwj X-Google-Smtp-Source: AIpwx49BrTtljyCKhscfBO5kFdp/uqpu4WaMOFmeJ1exeEkOv0hsb/EOCpmG5ziWZmYrD9p3A4o0yw== X-Received: by 10.80.214.73 with SMTP id c9mr13050414edj.80.1524218378913; Fri, 20 Apr 2018 02:59:38 -0700 (PDT) Received: from shalem.localdomain (546A5441.cm-12-3b.dynamic.ziggo.nl. [84.106.84.65]) by smtp.gmail.com with ESMTPSA id a16sm3504866edd.28.2018.04.20.02.59.37 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 20 Apr 2018 02:59:38 -0700 (PDT) From: Hans de Goede X-Google-Original-From: Hans de Goede To: Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= , Daniel Vetter Date: Fri, 20 Apr 2018 11:59:33 +0200 Message-Id: <20180420095933.16442-1-hdegoede@redhat.com> X-Mailer: git-send-email 2.17.0 Subject: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers v2 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hans de Goede , intel-gfx , dri-devel@lists.freedesktop.org MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) 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; }