From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Wunner Subject: Re: [PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Date: Sun, 25 Oct 2015 15:53:49 +0100 Message-ID: <20151025145349.GA19581@wunner.de> References: <20151008090737.GJ27939@nuc-i3427.alporthouse.com> <1444337421-20784-1-git-send-email-wayne.boyer@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mailout2.hostsharing.net (mailout2.hostsharing.net [83.223.90.233]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4CCCF6E09E for ; Sun, 25 Oct 2015 07:53:52 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1444337421-20784-1-git-send-email-wayne.boyer@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Wayne Boyer Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, "Goel, Akash" , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org QW5vdGhlciBvYnNlcnZhdGlvbiB0aGF0IG9jY3VycmVkIHRvIG1lIG9ubHkgYWZ0ZXIgc2VuZGlu ZyBhd2F5IG15CnByZXZpb3VzIG1lc3NhZ2UgKHNvcnJ5KToKCk9uIFRodSwgT2N0IDA4LCAyMDE1 IGF0IDAxOjUwOjIxUE0gLTA3MDAsIFdheW5lIEJveWVyIHdyb3RlOgo+IEZyb206IENocmlzIFdp bHNvbiA8Y2hyaXNAY2hyaXMtd2lsc29uLmNvLnVrPgo+IAo+IEEgbG9uZyB0aW1lIGFnbyAoYmVm b3JlIDMuMTQpIHdlIHJlbGllZCBvbiBhIHBlcm1hbmVudCBwaW5uaW5nIG9mIHRoZQo+IGlmYmRl diB0byBsb2NrIHRoZSBmYiBpbiBwbGFjZSBpbnNpZGUgdGhlIEdHVFQuIEhvd2V2ZXIsIHRoZQo+ IGludHJvZHVjdGlvbiBvZiBzdGVhbGluZyB0aGUgQklPUyBmcmFtZWJ1ZmZlciBhbmQgcmV1c2lu ZyBpdHMgYWRkcmVzcyBpbgo+IHRoZSBHR1RUIGZvciB0aGUgZmJkZXYgaGFzIG11ZGRpZWQgd2F0 ZXJzIGFuZCB3ZSB1c2UgYW4gaW5oZXJpdGVkIGZiLgo+IEhvd2V2ZXIsIHRoZSBpbmhlcml0ZWQg ZmIgaXMgb25seSBwaW5uZWQgd2hpbHN0IGl0IGlzIGFjdGl2ZSBhbmQgd2Ugbm8KPiBsb25nZXIg aGF2ZSBhbiBleHBsaWNpdCBwaW4gZm9yIHRoZSBpbmZvLT5zeXN0ZW1fYmFzZSBtbWFwcGluZyB1 c2VkIGJ5Cj4gdGhlIGZiZGV2LiBUaGUgcmVzdWx0IGlzIHRoYXQgYWZ0ZXIgc29tZSBhcGVydHVy ZSBwcmVzc3VyZSB0aGUgZmJkZXYgbWF5Cj4gYmUgZXZpY3RlZCwgYnV0IHdlIGNvbnRpbnVlIHRv IHdyaXRlIHRoZSBmYmNvbiBpbnRvIHRoZSBzYW1lIEdHVFQKPiBhZGRyZXNzIC0gb3ZlcndyaXRp bmcgYW55dGhpbmcgZWxzZSB0aGF0IG1heSBiZSBwdXQgaW50byB0aGF0IG9mZnNldC4KPiBUaGUg ZWZmZWN0IGlzIG1vc3QgcHJvbm91bmNlZCBhY3Jvc3Mgc3VzcGVuZC9yZXN1bWUgYXMKPiBpbnRl bF9mYmRldl9zZXRfc3VzcGVuZCgpIGRvZXMgYSBmdWxsIGNsZWFyIG92ZXIgdGhlIHdob2xlIHNj YW5vdXQuCgpTaW5jZSB0aGlzIG9ubHkgY29uY2VybnMgdGhlIGNhc2Ugd2hlbiB0aGUgZmIgd2Fz IGluaGVyaXRlZCBmcm9tIEJJT1MsCndoeSBkb24ndCB5b3UgaW52b2tlIGk5MTVfZ2VtX29ial9n Z3R0X3BpbigpIG9ubHkgaW4gdGhpcyBwYXJ0aWN1bGFyIGNhc2U/Ckl0J3MgZGlzY2VybmlibGUg ZnJvbSB0aGUgcHJlYWxsb2MgdmFyaWFibGUuCgpJIHRoaW5rIHRoZW4geW91IGFsc28gZG9uJ3Qg bmVlZCB0byBjYWxsIGk5MTVfZ2VtX29iamVjdF9nZ3R0X3VucGluKCkgdHdpY2UuCgoKPiAKPiB2 MjogcmViYXNlZCBvbiBsYXRlc3QgbmlnaHRseSAoV2F5bmUpCj4gdjM6IGNoYW5nZWQgaTkxNV9n ZW1fb2JqZWN0X2dndHRfcGluKCkgdG8gaTkxNV9nZW1fb2JqX2dndHRfcGluKCkgYmFzZWQKPiBv biBDaHJpcycgcmV2aWV3LiAoV2F5bmUpCj4gCj4gU2lnbmVkLW9mZi1ieTogQ2hyaXMgV2lsc29u IDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+Cj4gQ2M6ICJHb2VsLCBBa2FzaCIgPGFrYXNoLmdv ZWxAaW50ZWwuY29tPgo+IENjOiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xsLmNo Pgo+IENjOiBKZXNzZSBCYXJuZXMgPGpiYXJuZXNAdmlydHVvdXNnZWVrLm9yZz4KPiBDYzogc3Rh YmxlQHZnZXIua2VybmVsLm9yZwo+IFJldmlld2VkLWJ5OiBEZWVwYWsgUyA8ZGVlcGFrLnNAbGlu dXguaW50ZWwuY29tPgo+IFNpZ25lZC1vZmYtYnk6IFdheW5lIEJveWVyIDx3YXluZS5ib3llckBp bnRlbC5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2ZiZGV2LmMgfCAx NSArKysrKysrKysrKysrKysKPiAgMSBmaWxlIGNoYW5nZWQsIDE1IGluc2VydGlvbnMoKykKPiAK PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZmJkZXYuYyBiL2RyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX2ZiZGV2LmMKPiBpbmRleCA2NTMyOTEyLi4wYWQ0NjUyMSAx MDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9mYmRldi5jCj4gKysrIGIv ZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZmJkZXYuYwo+IEBAIC0yMTUsNiArMjE1LDE2IEBA IHN0YXRpYyBpbnQgaW50ZWxmYl9jcmVhdGUoc3RydWN0IGRybV9mYl9oZWxwZXIgKmhlbHBlciwK PiAgCW9iaiA9IGludGVsX2ZiLT5vYmo7Cj4gIAlzaXplID0gb2JqLT5iYXNlLnNpemU7Cj4gIAo+ ICsJLyogVGhlIGZiIGNvbnN0cnVjdG9yIHdpbGwgaGF2ZSBhbHJlYWR5IHBpbm5lZCB1cyAob3Ig aW5oZXJpdGVkIGEKPiArCSAqIEdHVFQgcmVnaW9uIGZyb20gdGhlIEJJT1MpIHN1aXRhYmxlIGZv ciBhIHNjYW5vdXQsIHNvCj4gKwkgKiB0aGlzIHNob3VsZCBqdXN0IGJlIGEgbm8tb3AgYW5kIGlu Y3JlbWVudCB0aGUgcGluIGNvdW50IGZvciB0aGUKPiArCSAqIGZiZGV2IG1tYXBwaW5nLiBJdCBk b2VzIGhhdmUgYSB1c2VmdWwgc2lkZS1lZmZlY3Qgb2YgdmFsaWRhdGluZwo+ICsJICogdGhlIHBp biBmb3IgZmJkZXYncyB1c2UgdmlhIGEgR0dUVCBtbWFwcGluZy4KPiArCSAqLwo+ICsJcmV0ID0g aTkxNV9nZW1fb2JqX2dndHRfcGluKG9iaiwgMCwgUElOX01BUFBBQkxFKTsKPiArCWlmIChyZXQp Cj4gKwkJZ290byBvdXRfdW5sb2NrOwo+ICsKPiAgCWluZm8gPSBkcm1fZmJfaGVscGVyX2FsbG9j X2ZiaShoZWxwZXIpOwo+ICAJaWYgKElTX0VSUihpbmZvKSkgewo+ICAJCXJldCA9IFBUUl9FUlIo aW5mbyk7Cj4gQEAgLTI3NCw2ICsyODQsOSBAQCBzdGF0aWMgaW50IGludGVsZmJfY3JlYXRlKHN0 cnVjdCBkcm1fZmJfaGVscGVyICpoZWxwZXIsCj4gIG91dF9kZXN0cm95X2ZiaToKPiAgCWRybV9m Yl9oZWxwZXJfcmVsZWFzZV9mYmkoaGVscGVyKTsKPiAgb3V0X3VucGluOgo+ICsJLyogT25jZSBm b3IgaW5mby0+c2NyZWVuX2Jhc2UgbW1hcGluZy4uLiAqLwo+ICsJaTkxNV9nZW1fb2JqZWN0X2dn dHRfdW5waW4ob2JqKTsKPiArCS8qIC4uLmFuZCBvbmNlIGZvciB0aGUgaW50ZWxfZmIgKi8KPiAg CWk5MTVfZ2VtX29iamVjdF9nZ3R0X3VucGluKG9iaik7Cj4gIAlkcm1fZ2VtX29iamVjdF91bnJl ZmVyZW5jZSgmb2JqLT5iYXNlKTsKPiAgb3V0X3VubG9jazoKPiBAQCAtNTE0LDYgKzUyNyw4IEBA IHN0YXRpYyBjb25zdCBzdHJ1Y3QgZHJtX2ZiX2hlbHBlcl9mdW5jcyBpbnRlbF9mYl9oZWxwZXJf ZnVuY3MgPSB7Cj4gIHN0YXRpYyB2b2lkIGludGVsX2ZiZGV2X2Rlc3Ryb3koc3RydWN0IGRybV9k ZXZpY2UgKmRldiwKPiAgCQkJCXN0cnVjdCBpbnRlbF9mYmRldiAqaWZiZGV2KQo+ICB7Cj4gKwkv KiBSZWxlYXNlIHRoZSBwaW5uaW5nIGZvciB0aGUgaW5mby0+c2NyZWVuX2Jhc2UgbW1hcGluZy4g Ki8KPiArCWk5MTVfZ2VtX29iamVjdF9nZ3R0X3VucGluKGlmYmRldi0+ZmItPm9iaik7Cj4gIAo+ ICAJZHJtX2ZiX2hlbHBlcl91bnJlZ2lzdGVyX2ZiaSgmaWZiZGV2LT5oZWxwZXIpOwo+ICAJZHJt X2ZiX2hlbHBlcl9yZWxlYXNlX2ZiaSgmaWZiZGV2LT5oZWxwZXIpOwo+IC0tIAo+IDEuOS4xCj4g Cj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiBJbnRl bC1nZnggbWFpbGluZyBsaXN0Cj4gSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+IGh0 dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngKX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1h aWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.hostsharing.net ([83.223.90.233]:52176 "EHLO mailout2.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752436AbbJYO7S (ORCPT ); Sun, 25 Oct 2015 10:59:18 -0400 Date: Sun, 25 Oct 2015 15:53:49 +0100 From: Lukas Wunner To: Wayne Boyer Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter , stable@vger.kernel.org, "Goel, Akash" , Chris Wilson Subject: Re: [Intel-gfx] [PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Message-ID: <20151025145349.GA19581@wunner.de> References: <20151008090737.GJ27939@nuc-i3427.alporthouse.com> <1444337421-20784-1-git-send-email-wayne.boyer@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1444337421-20784-1-git-send-email-wayne.boyer@intel.com> Sender: stable-owner@vger.kernel.org List-ID: Another observation that occurred to me only after sending away my previous message (sorry): On Thu, Oct 08, 2015 at 01:50:21PM -0700, Wayne Boyer wrote: > From: Chris Wilson > > 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. Since this only concerns the case when the fb was inherited from BIOS, why don't you invoke i915_gem_obj_ggtt_pin() only in this particular case? It's discernible from the prealloc variable. I think then you also don't need to call i915_gem_object_ggtt_unpin() twice. > > v2: rebased on latest nightly (Wayne) > v3: changed i915_gem_object_ggtt_pin() to i915_gem_obj_ggtt_pin() based > on Chris' review. (Wayne) > > Signed-off-by: Chris Wilson > Cc: "Goel, Akash" > Cc: Daniel Vetter > Cc: Jesse Barnes > Cc: stable@vger.kernel.org > Reviewed-by: Deepak S > Signed-off-by: Wayne Boyer > --- > drivers/gpu/drm/i915/intel_fbdev.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 6532912..0ad46521 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -215,6 +215,16 @@ static int intelfb_create(struct drm_fb_helper *helper, > obj = intel_fb->obj; > size = obj->base.size; > > + /* The fb constructor will have already pinned us (or inherited a > + * GGTT region from the BIOS) suitable for a scanout, so > + * this should just be a no-op and increment the pin count for the > + * fbdev mmapping. It does have a useful side-effect of validating > + * the pin for fbdev's use via a GGTT mmapping. > + */ > + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); > + if (ret) > + goto out_unlock; > + > info = drm_fb_helper_alloc_fbi(helper); > if (IS_ERR(info)) { > ret = PTR_ERR(info); > @@ -274,6 +284,9 @@ static int intelfb_create(struct drm_fb_helper *helper, > out_destroy_fbi: > drm_fb_helper_release_fbi(helper); > out_unpin: > + /* Once for info->screen_base mmaping... */ > + i915_gem_object_ggtt_unpin(obj); > + /* ...and once for the intel_fb */ > i915_gem_object_ggtt_unpin(obj); > drm_gem_object_unreference(&obj->base); > out_unlock: > @@ -514,6 +527,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); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx