From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Date: Thu, 10 Dec 2015 18:36:04 +0200 Message-ID: <20151210163604.GC4437@intel.com> References: <20151124212049.GA26526@wunner.de> <1449245126-26158-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 mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id A4AFD6EAE0 for ; Thu, 10 Dec 2015 08:36:10 -0800 (PST) Content-Disposition: inline In-Reply-To: <1449245126-26158-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 Cc: Takashi Iwai , Daniel Vetter , intel-gfx@lists.freedesktop.org, "Goel, Akash" , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gRnJpLCBEZWMgMDQsIDIwMTUgYXQgMDQ6MDU6MjZQTSArMDAwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IEEgbG9uZyB0aW1lIGFnbyAoYmVmb3JlIDMuMTQpIHdlIHJlbGllZCBvbiBhIHBlcm1h bmVudCBwaW5uaW5nIG9mIHRoZQo+IGlmYmRldiB0byBsb2NrIHRoZSBmYiBpbiBwbGFjZSBpbnNp ZGUgdGhlIEdHVFQuIEhvd2V2ZXIsIHRoZQo+IGludHJvZHVjdGlvbiBvZiBzdGVhbGluZyB0aGUg QklPUyBmcmFtZWJ1ZmZlciBhbmQgcmV1c2luZyBpdHMgYWRkcmVzcyBpbgo+IHRoZSBHR1RUIGZv ciB0aGUgZmJkZXYgaGFzIG11ZGRpZWQgd2F0ZXJzIGFuZCB3ZSB1c2UgYW4gaW5oZXJpdGVkIGZi Lgo+IEhvd2V2ZXIsIHRoZSBpbmhlcml0ZWQgZmIgaXMgb25seSBwaW5uZWQgd2hpbHN0IGl0IGlz IGFjdGl2ZSBhbmQgd2Ugbm8KPiBsb25nZXIgaGF2ZSBhbiBleHBsaWNpdCBwaW4gZm9yIHRoZSBp bmZvLT5zeXN0ZW1fYmFzZSBtbWFwcGluZyB1c2VkIGJ5Cj4gdGhlIGZiZGV2LiBUaGUgcmVzdWx0 IGlzIHRoYXQgYWZ0ZXIgc29tZSBhcGVydHVyZSBwcmVzc3VyZSB0aGUgZmJkZXYgbWF5Cj4gYmUg ZXZpY3RlZCwgYnV0IHdlIGNvbnRpbnVlIHRvIHdyaXRlIHRoZSBmYmNvbiBpbnRvIHRoZSBzYW1l IEdHVFQKPiBhZGRyZXNzIC0gb3ZlcndyaXRpbmcgYW55dGhpbmcgZWxzZSB0aGF0IG1heSBiZSBw dXQgaW50byB0aGF0IG9mZnNldC4KPiBUaGUgZWZmZWN0IGlzIG1vc3QgcHJvbm91bmNlZCBhY3Jv c3Mgc3VzcGVuZC9yZXN1bWUgYXMKPiBpbnRlbF9mYmRldl9zZXRfc3VzcGVuZCgpIGRvZXMgYSBm dWxsIGNsZWFyIG92ZXIgdGhlIHdob2xlIHNjYW5vdXQuCj4gCj4gdjI6IE9ubHkgdW5waW4gdGhl IGludGVsX2ZiIGlzIHdlIGFsbG9jYXRlIGl0LiBJZiB3ZSBpbmhlcml0IHRoZSBmYiBmcm9tCj4g dGhlIEJJT1MsIHdlIGRvIG5vdCBvd24gdGhlIHBpbm5lZCB2bWEgKGV4Y2VwdCBmb3IgdGhlIHJl ZmVyZW5jZSB3ZSBhZGQKPiBpbiB0aGlzIHBhdGNoIGZvciBvdXIgYWNjZXNzIHZpYSBpbmZvLT5z Y3JlZW5fYmFzZSkuCj4gCj4gdjM6IEZpbmlzaCBiYWxhbmNpbmcgdGhlIHZtYSBwaW5uaW5nIGZv ciB0aGUgbm9ybWFsICFwcmVhbGxvY2F0ZWQgY2FzZS4KPiAKPiB2NDogVHJ5IHRvIHNpbXBsaWZ5 IHRoZSBwaW5uaW5nIGV2ZW4gZnVydGhlci4KPiB2NTogTGVhayB0aGUgVk1BIChjbGVhbmVkIHVw IGJ5IG9iamVjdC1mcmVlKSB0byBhdm9pZCBjb21wbGljYXRlZCBlcnJvciBwYXRocy4KPiAKPiBT aWduZWQtb2ZmLWJ5OiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdpbHNvbi5jby51az4KPiBD YzogIkdvZWwsIEFrYXNoIiA8YWthc2guZ29lbEBpbnRlbC5jb20+Cj4gQ2M6IERhbmllbCBWZXR0 ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gQ2M6IEplc3NlIEJhcm5lcyA8amJhcm5lc0B2 aXJ0dW91c2dlZWsub3JnPgo+IENjOiBMdWthcyBXdW5uZXIgPGx1a2FzQHd1bm5lci5kZT4KPiBD Yzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwoKVGhpcyBzZWVtcyB0byBoYXZlIGZpeGVkIG15IGdh cmJsZWQgdGV4dCtmYmNvbiBkZWFkIGFmdGVyCnN1c3BlbmQvaGliZXJuYXRlIGlzc3Vlcy4gV2Vs bCwgb25seSBoYWQgdGhlIHBhdGNoIGluIGZvciBhIGRheSBvciBzbywKYnV0IHNvIGZhciBzbyBn b29kLgoKVGVzdGVkLWJ5OiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGludXguaW50 ZWwuY29tPgoKVGFrYXNoaSwgZG9uJ3Qga25vdyBpZiB5b3UgYWxyZWFkeSBmb3VuZCB0aGlzIHBh dGNoLCBidXQgaXQncyBkZWZpbml0ZWx5CnNvbWV0aGluZyB5b3Ugc2hvdWxkIHRyeSBhcyB3ZWxs LgoKPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZmJkZXYuYyB8IDIwICsrKysr KysrKysrKystLS0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspLCA3IGRl bGV0aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9m YmRldi5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZmJkZXYuYwo+IGluZGV4IDdjY2Rl NThmOGM5OC4uYmVhNzVjYWZjNjIzIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1 L2ludGVsX2ZiZGV2LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9mYmRldi5j Cj4gQEAgLTE2MywxMyArMTYzLDYgQEAgc3RhdGljIGludCBpbnRlbGZiX2FsbG9jKHN0cnVjdCBk cm1fZmJfaGVscGVyICpoZWxwZXIsCj4gIAkJZ290byBvdXQ7Cj4gIAl9Cj4gIAo+IC0JLyogRmx1 c2ggZXZlcnl0aGluZyBvdXQsIHdlJ2xsIGJlIGRvaW5nIEdUVCBvbmx5IGZyb20gbm93IG9uICov Cj4gLQlyZXQgPSBpbnRlbF9waW5fYW5kX2ZlbmNlX2ZiX29iaihOVUxMLCBmYiwgTlVMTCk7Cj4g LQlpZiAocmV0KSB7Cj4gLQkJRFJNX0VSUk9SKCJmYWlsZWQgdG8gcGluIG9iajogJWRcbiIsIHJl dCk7Cj4gLQkJZ290byBvdXQ7Cj4gLQl9Cj4gLQo+ICAJbXV0ZXhfdW5sb2NrKCZkZXYtPnN0cnVj dF9tdXRleCk7Cj4gIAo+ICAJaWZiZGV2LT5mYiA9IHRvX2ludGVsX2ZyYW1lYnVmZmVyKGZiKTsK PiBAQCAtMjI1LDYgKzIxOCwxNCBAQCBzdGF0aWMgaW50IGludGVsZmJfY3JlYXRlKHN0cnVjdCBk cm1fZmJfaGVscGVyICpoZWxwZXIsCj4gIAo+ICAJbXV0ZXhfbG9jaygmZGV2LT5zdHJ1Y3RfbXV0 ZXgpOwo+ICAKPiArCS8qIFBpbiB0aGUgR0dUVCB2bWEgZm9yIG91ciBhY2Nlc3MgdmlhIGluZm8t PnNjcmVlbl9iYXNlLgo+ICsJICogVGhpcyBhbHNvIHZhbGlkYXRlcyB0aGF0IGFueSBleGlzdGlu ZyBmYiBpbmhlcml0ZWQgZnJvbSB0aGUKPiArCSAqIEJJT1MgaXMgc3VpdGFibGUgZm9yIG93biBh Y2Nlc3MuCj4gKwkgKi8KPiArCXJldCA9IGludGVsX3Bpbl9hbmRfZmVuY2VfZmJfb2JqKE5VTEws ICZpZmJkZXYtPmZiLT5iYXNlLCBOVUxMKTsKPiArCWlmIChyZXQpCj4gKwkJZ290byBvdXRfdW5s b2NrOwo+ICsKPiAgCWluZm8gPSBkcm1fZmJfaGVscGVyX2FsbG9jX2ZiaShoZWxwZXIpOwo+ICAJ aWYgKElTX0VSUihpbmZvKSkgewo+ICAJCURSTV9FUlJPUigiRmFpbGVkIHRvIGFsbG9jYXRlIGZi X2luZm9cbiIpOwo+IEBAIC0yODcsNiArMjg4LDcgQEAgb3V0X2Rlc3Ryb3lfZmJpOgo+ICAJZHJt X2ZiX2hlbHBlcl9yZWxlYXNlX2ZiaShoZWxwZXIpOwo+ICBvdXRfdW5waW46Cj4gIAlpOTE1X2dl bV9vYmplY3RfZ2d0dF91bnBpbihvYmopOwo+ICtvdXRfdW5sb2NrOgo+ICAJbXV0ZXhfdW5sb2Nr KCZkZXYtPnN0cnVjdF9tdXRleCk7Cj4gIAlyZXR1cm4gcmV0Owo+ICB9Cj4gQEAgLTUyNCw2ICs1 MjYsMTAgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fZmJfaGVscGVyX2Z1bmNzIGludGVsX2Zi X2hlbHBlcl9mdW5jcyA9IHsKPiAgc3RhdGljIHZvaWQgaW50ZWxfZmJkZXZfZGVzdHJveShzdHJ1 Y3QgZHJtX2RldmljZSAqZGV2LAo+ICAJCQkJc3RydWN0IGludGVsX2ZiZGV2ICppZmJkZXYpCj4g IHsKPiArCS8qIFdlIHJlbHkgb24gdGhlIG9iamVjdC1mcmVlIHRvIHJlbGVhc2UgdGhlIFZNQSBw aW5uaW5nIGZvcgo+ICsJICogdGhlIGluZm8tPnNjcmVlbl9iYXNlIG1tYXBpbmcuIExlYWtpbmcg dGhlIFZNQSBpcyBzaW1wbGVyIHRoYW4KPiArCSAqIHRyeWluZyB0byByZWN0aWZ5IGFsbCB0aGUg cG9zc2libGUgZXJyb3IgcGF0aHMgbGVhZGluZyBoZXJlLgo+ICsJICovCj4gIAo+ICAJZHJtX2Zi X2hlbHBlcl91bnJlZ2lzdGVyX2ZiaSgmaWZiZGV2LT5oZWxwZXIpOwo+ICAJZHJtX2ZiX2hlbHBl cl9yZWxlYXNlX2ZiaSgmaWZiZGV2LT5oZWxwZXIpOwo+IC0tIAo+IDIuNi4yCj4gCj4gX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiBJbnRlbC1nZnggbWFp bGluZyBsaXN0Cj4gSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+IGh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngKCi0tIApWaWxsZSBT eXJqw6Rsw6QKSW50ZWwgT1RDCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNr dG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50 ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:55167 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751996AbbLJQgL (ORCPT ); Thu, 10 Dec 2015 11:36:11 -0500 Date: Thu, 10 Dec 2015 18:36:04 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter , stable@vger.kernel.org, "Goel, Akash" , Takashi Iwai Subject: Re: [Intel-gfx] [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Message-ID: <20151210163604.GC4437@intel.com> References: <20151124212049.GA26526@wunner.de> <1449245126-26158-1-git-send-email-chris@chris-wilson.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1449245126-26158-1-git-send-email-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org List-ID: On Fri, Dec 04, 2015 at 04:05:26PM +0000, 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. > v5: Leak the VMA (cleaned up by object-free) to avoid complicated error paths. > > Signed-off-by: Chris Wilson > Cc: "Goel, Akash" > Cc: Daniel Vetter > Cc: Jesse Barnes > Cc: Lukas Wunner > Cc: stable@vger.kernel.org This seems to have fixed my garbled text+fbcon dead after suspend/hibernate issues. Well, only had the patch in for a day or so, but so far so good. Tested-by: Ville Syrj�l� Takashi, don't know if you already found this patch, but it's definitely something you should try as well. > --- > drivers/gpu/drm/i915/intel_fbdev.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 7ccde58f8c98..bea75cafc623 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,10 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { > static void intel_fbdev_destroy(struct drm_device *dev, > struct intel_fbdev *ifbdev) > { > + /* We rely on the object-free to release the VMA pinning for > + * the info->screen_base mmaping. Leaking the VMA is simpler than > + * trying to rectify all the possible error paths leading here. > + */ > > drm_fb_helper_unregister_fbi(&ifbdev->helper); > drm_fb_helper_release_fbi(&ifbdev->helper); > -- > 2.6.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj�l� Intel OTC