From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH v4] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Date: Fri, 20 Nov 2015 08:35:31 -0800 Message-ID: <564F4BD3.9080701@virtuousgeek.org> References: <564F4709.5070909@virtuousgeek.org> <1448036992-11749-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) by gabe.freedesktop.org (Postfix) with ESMTPS id E59096EAFC for ; Fri, 20 Nov 2015 08:34:51 -0800 (PST) Received: by pacdm15 with SMTP id dm15so121002481pac.3 for ; Fri, 20 Nov 2015 08:34:51 -0800 (PST) In-Reply-To: <1448036992-11749-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: Daniel Vetter , "Goel, Akash" , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gMTEvMjAvMjAxNSAwODoyOSBBTSwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+IEEgbG9uZyB0aW1l IGFnbyAoYmVmb3JlIDMuMTQpIHdlIHJlbGllZCBvbiBhIHBlcm1hbmVudCBwaW5uaW5nIG9mIHRo ZQo+IGlmYmRldiB0byBsb2NrIHRoZSBmYiBpbiBwbGFjZSBpbnNpZGUgdGhlIEdHVFQuIEhvd2V2 ZXIsIHRoZQo+IGludHJvZHVjdGlvbiBvZiBzdGVhbGluZyB0aGUgQklPUyBmcmFtZWJ1ZmZlciBh bmQgcmV1c2luZyBpdHMgYWRkcmVzcyBpbgo+IHRoZSBHR1RUIGZvciB0aGUgZmJkZXYgaGFzIG11 ZGRpZWQgd2F0ZXJzIGFuZCB3ZSB1c2UgYW4gaW5oZXJpdGVkIGZiLgo+IEhvd2V2ZXIsIHRoZSBp bmhlcml0ZWQgZmIgaXMgb25seSBwaW5uZWQgd2hpbHN0IGl0IGlzIGFjdGl2ZSBhbmQgd2Ugbm8K PiBsb25nZXIgaGF2ZSBhbiBleHBsaWNpdCBwaW4gZm9yIHRoZSBpbmZvLT5zeXN0ZW1fYmFzZSBt bWFwcGluZyB1c2VkIGJ5Cj4gdGhlIGZiZGV2LiBUaGUgcmVzdWx0IGlzIHRoYXQgYWZ0ZXIgc29t ZSBhcGVydHVyZSBwcmVzc3VyZSB0aGUgZmJkZXYgbWF5Cj4gYmUgZXZpY3RlZCwgYnV0IHdlIGNv bnRpbnVlIHRvIHdyaXRlIHRoZSBmYmNvbiBpbnRvIHRoZSBzYW1lIEdHVFQKPiBhZGRyZXNzIC0g b3ZlcndyaXRpbmcgYW55dGhpbmcgZWxzZSB0aGF0IG1heSBiZSBwdXQgaW50byB0aGF0IG9mZnNl dC4KPiBUaGUgZWZmZWN0IGlzIG1vc3QgcHJvbm91bmNlZCBhY3Jvc3Mgc3VzcGVuZC9yZXN1bWUg YXMKPiBpbnRlbF9mYmRldl9zZXRfc3VzcGVuZCgpIGRvZXMgYSBmdWxsIGNsZWFyIG92ZXIgdGhl IHdob2xlIHNjYW5vdXQuCj4gCj4gdjI6IE9ubHkgdW5waW4gdGhlIGludGVsX2ZiIGlzIHdlIGFs bG9jYXRlIGl0LiBJZiB3ZSBpbmhlcml0IHRoZSBmYiBmcm9tCj4gdGhlIEJJT1MsIHdlIGRvIG5v dCBvd24gdGhlIHBpbm5lZCB2bWEgKGV4Y2VwdCBmb3IgdGhlIHJlZmVyZW5jZSB3ZSBhZGQKPiBp biB0aGlzIHBhdGNoIGZvciBvdXIgYWNjZXNzIHZpYSBpbmZvLT5zY3JlZW5fYmFzZSkuCj4gCj4g djM6IEZpbmlzaCBiYWxhbmNpbmcgdGhlIHZtYSBwaW5uaW5nIGZvciB0aGUgbm9ybWFsICFwcmVh bGxvY2F0ZWQgY2FzZS4KPiAKPiB2NDogVHJ5IHRvIHNpbXBsaWZ5IHRoZSBwaW5uaW5nIGV2ZW4g ZnVydGhlci4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdp bHNvbi5jby51az4KPiBDYzogIkdvZWwsIEFrYXNoIiA8YWthc2guZ29lbEBpbnRlbC5jb20+Cj4g Q2M6IERhbmllbCBWZXR0ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gQ2M6IEplc3NlIEJh cm5lcyA8amJhcm5lc0B2aXJ0dW91c2dlZWsub3JnPgo+IENjOiBzdGFibGVAdmdlci5rZXJuZWwu b3JnCj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2ZiZGV2LmMgfCAxOCArKysr KysrKysrKy0tLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDExIGluc2VydGlvbnMoKyksIDcgZGVs ZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Zi ZGV2LmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9mYmRldi5jCj4gaW5kZXggN2NjZGU1 OGY4Yzk4Li43OWYwMmU3MmRhOGEgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfZmJkZXYuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2ZiZGV2LmMK PiBAQCAtMTYzLDEzICsxNjMsNiBAQCBzdGF0aWMgaW50IGludGVsZmJfYWxsb2Moc3RydWN0IGRy bV9mYl9oZWxwZXIgKmhlbHBlciwKPiAgCQlnb3RvIG91dDsKPiAgCX0KPiAgCj4gLQkvKiBGbHVz aCBldmVyeXRoaW5nIG91dCwgd2UnbGwgYmUgZG9pbmcgR1RUIG9ubHkgZnJvbSBub3cgb24gKi8K PiAtCXJldCA9IGludGVsX3Bpbl9hbmRfZmVuY2VfZmJfb2JqKE5VTEwsIGZiLCBOVUxMKTsKPiAt CWlmIChyZXQpIHsKPiAtCQlEUk1fRVJST1IoImZhaWxlZCB0byBwaW4gb2JqOiAlZFxuIiwgcmV0 KTsKPiAtCQlnb3RvIG91dDsKPiAtCX0KPiAtCj4gIAltdXRleF91bmxvY2soJmRldi0+c3RydWN0 X211dGV4KTsKPiAgCj4gIAlpZmJkZXYtPmZiID0gdG9faW50ZWxfZnJhbWVidWZmZXIoZmIpOwo+ IEBAIC0yMjUsNiArMjE4LDE0IEBAIHN0YXRpYyBpbnQgaW50ZWxmYl9jcmVhdGUoc3RydWN0IGRy bV9mYl9oZWxwZXIgKmhlbHBlciwKPiAgCj4gIAltdXRleF9sb2NrKCZkZXYtPnN0cnVjdF9tdXRl eCk7Cj4gIAo+ICsJLyogUGluIHRoZSBHR1RUIHZtYSBmb3Igb3VyIGFjY2VzcyB2aWEgaW5mby0+ c2NyZWVuX2Jhc2UuCj4gKwkgKiBUaGlzIGFsc28gdmFsaWRhdGVzIHRoYXQgYW55IGV4aXN0aW5n IGZiIGluaGVyaXRlZCBmcm9tIHRoZQo+ICsJICogQklPUyBpcyBzdWl0YWJsZSBmb3Igb3duIGFj Y2Vzcy4KPiArCSAqLwo+ICsJcmV0ID0gaW50ZWxfcGluX2FuZF9mZW5jZV9mYl9vYmooTlVMTCwg aWZiZGV2LT5mYi0+YmFzZSwgTlVMTCk7Cj4gKwlpZiAocmV0KQo+ICsJCWdvdG8gb3V0X3VubG9j azsKPiArCj4gIAlpbmZvID0gZHJtX2ZiX2hlbHBlcl9hbGxvY19mYmkoaGVscGVyKTsKPiAgCWlm IChJU19FUlIoaW5mbykpIHsKPiAgCQlEUk1fRVJST1IoIkZhaWxlZCB0byBhbGxvY2F0ZSBmYl9p bmZvXG4iKTsKPiBAQCAtMjg3LDYgKzI4OCw3IEBAIG91dF9kZXN0cm95X2ZiaToKPiAgCWRybV9m Yl9oZWxwZXJfcmVsZWFzZV9mYmkoaGVscGVyKTsKPiAgb3V0X3VucGluOgo+ICAJaTkxNV9nZW1f b2JqZWN0X2dndHRfdW5waW4ob2JqKTsKPiArb3V0X3VubG9jazoKPiAgCW11dGV4X3VubG9jaygm ZGV2LT5zdHJ1Y3RfbXV0ZXgpOwo+ICAJcmV0dXJuIHJldDsKPiAgfQo+IEBAIC01MjQsNiArNTI2 LDggQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fZmJfaGVscGVyX2Z1bmNzIGludGVsX2ZiX2hl bHBlcl9mdW5jcyA9IHsKPiAgc3RhdGljIHZvaWQgaW50ZWxfZmJkZXZfZGVzdHJveShzdHJ1Y3Qg ZHJtX2RldmljZSAqZGV2LAo+ICAJCQkJc3RydWN0IGludGVsX2ZiZGV2ICppZmJkZXYpCj4gIHsK PiArCS8qIFJlbGVhc2UgdGhlIHBpbm5pbmcgZm9yIHRoZSBpbmZvLT5zY3JlZW5fYmFzZSBtbWFw aW5nLiAqLwo+ICsJaTkxNV9nZW1fb2JqZWN0X2dndHRfdW5waW4oaWZiZGV2LT5mYi0+b2JqKTsK PiAgCj4gIAlkcm1fZmJfaGVscGVyX3VucmVnaXN0ZXJfZmJpKCZpZmJkZXYtPmhlbHBlcik7Cj4g IAlkcm1fZmJfaGVscGVyX3JlbGVhc2VfZmJpKCZpZmJkZXYtPmhlbHBlcik7Cj4gCgpBaCBldmVu IGJldHRlci4KClJldmlld2VkLWJ5OiBKZXNzZSBCYXJuZXMgPGpiYXJuZXNAdmlydHVvdXNnZWVr Lm9yZz4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50 ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6 Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f54.google.com ([209.85.220.54]:36408 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162191AbbKTQew (ORCPT ); Fri, 20 Nov 2015 11:34:52 -0500 Received: by pacdm15 with SMTP id dm15so121002480pac.3 for ; Fri, 20 Nov 2015 08:34:51 -0800 (PST) Subject: Re: [PATCH v4] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <564F4709.5070909@virtuousgeek.org> <1448036992-11749-1-git-send-email-chris@chris-wilson.co.uk> Cc: "Goel, Akash" , Daniel Vetter , stable@vger.kernel.org From: Jesse Barnes Message-ID: <564F4BD3.9080701@virtuousgeek.org> Date: Fri, 20 Nov 2015 08:35:31 -0800 MIME-Version: 1.0 In-Reply-To: <1448036992-11749-1-git-send-email-chris@chris-wilson.co.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 11/20/2015 08:29 AM, Chris Wilson wrote: > A long time ago (before 3.14) we relied on a permanent pinning of the > ifbdev to lock the fb in place inside the GGTT. However, the > introduction of stealing the BIOS framebuffer and reusing its address in > the GGTT for the fbdev has muddied waters and we use an inherited fb. > However, the inherited fb is only pinned whilst it is active and we no > longer have an explicit pin for the info->system_base mmapping used by > the fbdev. The result is that after some aperture pressure the fbdev may > be evicted, but we continue to write the fbcon into the same GGTT > address - overwriting anything else that may be put into that offset. > The effect is most pronounced across suspend/resume as > intel_fbdev_set_suspend() does a full clear over the whole scanout. > > v2: Only unpin the intel_fb is we allocate it. If we inherit the fb from > the BIOS, we do not own the pinned vma (except for the reference we add > in this patch for our access via info->screen_base). > > v3: Finish balancing the vma pinning for the normal !preallocated case. > > v4: Try to simplify the pinning even further. > > Signed-off-by: Chris Wilson > Cc: "Goel, Akash" > Cc: Daniel Vetter > Cc: Jesse Barnes > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/i915/intel_fbdev.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 7ccde58f8c98..79f02e72da8a 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -163,13 +163,6 @@ static int intelfb_alloc(struct drm_fb_helper *helper, > goto out; > } > > - /* Flush everything out, we'll be doing GTT only from now on */ > - ret = intel_pin_and_fence_fb_obj(NULL, fb, NULL); > - if (ret) { > - DRM_ERROR("failed to pin obj: %d\n", ret); > - goto out; > - } > - > mutex_unlock(&dev->struct_mutex); > > ifbdev->fb = to_intel_framebuffer(fb); > @@ -225,6 +218,14 @@ static int intelfb_create(struct drm_fb_helper *helper, > > mutex_lock(&dev->struct_mutex); > > + /* Pin the GGTT vma for our access via info->screen_base. > + * This also validates that any existing fb inherited from the > + * BIOS is suitable for own access. > + */ > + ret = intel_pin_and_fence_fb_obj(NULL, ifbdev->fb->base, NULL); > + if (ret) > + goto out_unlock; > + > info = drm_fb_helper_alloc_fbi(helper); > if (IS_ERR(info)) { > DRM_ERROR("Failed to allocate fb_info\n"); > @@ -287,6 +288,7 @@ out_destroy_fbi: > drm_fb_helper_release_fbi(helper); > out_unpin: > i915_gem_object_ggtt_unpin(obj); > +out_unlock: > mutex_unlock(&dev->struct_mutex); > return ret; > } > @@ -524,6 +526,8 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { > static void intel_fbdev_destroy(struct drm_device *dev, > struct intel_fbdev *ifbdev) > { > + /* Release the pinning for the info->screen_base mmaping. */ > + i915_gem_object_ggtt_unpin(ifbdev->fb->obj); > > drm_fb_helper_unregister_fbi(&ifbdev->helper); > drm_fb_helper_release_fbi(&ifbdev->helper); > Ah even better. Reviewed-by: Jesse Barnes