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:34:16 +0100 Message-ID: <20151025143416.GA19554@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 mailout1.hostsharing.net (mailout1.hostsharing.net [83.223.95.204]) by gabe.freedesktop.org (Postfix) with ESMTPS id 30CDA89EA3 for ; Sun, 25 Oct 2015 07:34:20 -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 SGksCgpPbiBUaHUsIE9jdCAwOCwgMjAxNSBhdCAwMTo1MDoyMVBNIC0wNzAwLCBXYXluZSBCb3ll ciB3cm90ZToKPiBGcm9tOiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdpbHNvbi5jby51az4K PiAKPiBBIGxvbmcgdGltZSBhZ28gKGJlZm9yZSAzLjE0KSB3ZSByZWxpZWQgb24gYSBwZXJtYW5l bnQgcGlubmluZyBvZiB0aGUKPiBpZmJkZXYgdG8gbG9jayB0aGUgZmIgaW4gcGxhY2UgaW5zaWRl IHRoZSBHR1RULiBIb3dldmVyLCB0aGUKPiBpbnRyb2R1Y3Rpb24gb2Ygc3RlYWxpbmcgdGhlIEJJ T1MgZnJhbWVidWZmZXIgYW5kIHJldXNpbmcgaXRzIGFkZHJlc3MgaW4KPiB0aGUgR0dUVCBmb3Ig dGhlIGZiZGV2IGhhcyBtdWRkaWVkIHdhdGVycyBhbmQgd2UgdXNlIGFuIGluaGVyaXRlZCBmYi4K PiBIb3dldmVyLCB0aGUgaW5oZXJpdGVkIGZiIGlzIG9ubHkgcGlubmVkIHdoaWxzdCBpdCBpcyBh Y3RpdmUgYW5kIHdlIG5vCj4gbG9uZ2VyIGhhdmUgYW4gZXhwbGljaXQgcGluIGZvciB0aGUgaW5m by0+c3lzdGVtX2Jhc2UgbW1hcHBpbmcgdXNlZCBieQo+IHRoZSBmYmRldi4gVGhlIHJlc3VsdCBp cyB0aGF0IGFmdGVyIHNvbWUgYXBlcnR1cmUgcHJlc3N1cmUgdGhlIGZiZGV2IG1heQo+IGJlIGV2 aWN0ZWQsIGJ1dCB3ZSBjb250aW51ZSB0byB3cml0ZSB0aGUgZmJjb24gaW50byB0aGUgc2FtZSBH R1RUCj4gYWRkcmVzcyAtIG92ZXJ3cml0aW5nIGFueXRoaW5nIGVsc2UgdGhhdCBtYXkgYmUgcHV0 IGludG8gdGhhdCBvZmZzZXQuCj4gVGhlIGVmZmVjdCBpcyBtb3N0IHByb25vdW5jZWQgYWNyb3Nz IHN1c3BlbmQvcmVzdW1lIGFzCj4gaW50ZWxfZmJkZXZfc2V0X3N1c3BlbmQoKSBkb2VzIGEgZnVs bCBjbGVhciBvdmVyIHRoZSB3aG9sZSBzY2Fub3V0Lgo+IAo+IHYyOiByZWJhc2VkIG9uIGxhdGVz dCBuaWdodGx5IChXYXluZSkKPiB2MzogY2hhbmdlZCBpOTE1X2dlbV9vYmplY3RfZ2d0dF9waW4o KSB0byBpOTE1X2dlbV9vYmpfZ2d0dF9waW4oKSBiYXNlZAo+IG9uIENocmlzJyByZXZpZXcuIChX YXluZSkKPiAKPiBTaWduZWQtb2ZmLWJ5OiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdpbHNv bi5jby51az4KPiBDYzogIkdvZWwsIEFrYXNoIiA8YWthc2guZ29lbEBpbnRlbC5jb20+Cj4gQ2M6 IERhbmllbCBWZXR0ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gQ2M6IEplc3NlIEJhcm5l cyA8amJhcm5lc0B2aXJ0dW91c2dlZWsub3JnPgo+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3Jn Cj4gUmV2aWV3ZWQtYnk6IERlZXBhayBTIDxkZWVwYWsuc0BsaW51eC5pbnRlbC5jb20+Cj4gU2ln bmVkLW9mZi1ieTogV2F5bmUgQm95ZXIgPHdheW5lLmJveWVyQGludGVsLmNvbT4KPiAtLS0KPiAg ZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZmJkZXYuYyB8IDE1ICsrKysrKysrKysrKysrKwo+ ICAxIGZpbGUgY2hhbmdlZCwgMTUgaW5zZXJ0aW9ucygrKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2 ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9mYmRldi5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50 ZWxfZmJkZXYuYwo+IGluZGV4IDY1MzI5MTIuLjBhZDQ2NTIxIDEwMDY0NAo+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX2ZiZGV2LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9mYmRldi5jCj4gQEAgLTIxNSw2ICsyMTUsMTYgQEAgc3RhdGljIGludCBpbnRlbGZi X2NyZWF0ZShzdHJ1Y3QgZHJtX2ZiX2hlbHBlciAqaGVscGVyLAo+ICAJb2JqID0gaW50ZWxfZmIt Pm9iajsKPiAgCXNpemUgPSBvYmotPmJhc2Uuc2l6ZTsKPiAgCj4gKwkvKiBUaGUgZmIgY29uc3Ry dWN0b3Igd2lsbCBoYXZlIGFscmVhZHkgcGlubmVkIHVzIChvciBpbmhlcml0ZWQgYQo+ICsJICog R0dUVCByZWdpb24gZnJvbSB0aGUgQklPUykgc3VpdGFibGUgZm9yIGEgc2Nhbm91dCwgc28KPiAr CSAqIHRoaXMgc2hvdWxkIGp1c3QgYmUgYSBuby1vcCBhbmQgaW5jcmVtZW50IHRoZSBwaW4gY291 bnQgZm9yIHRoZQo+ICsJICogZmJkZXYgbW1hcHBpbmcuIEl0IGRvZXMgaGF2ZSBhIHVzZWZ1bCBz aWRlLWVmZmVjdCBvZiB2YWxpZGF0aW5nCj4gKwkgKiB0aGUgcGluIGZvciBmYmRldidzIHVzZSB2 aWEgYSBHR1RUIG1tYXBwaW5nLgo+ICsJICovCj4gKwlyZXQgPSBpOTE1X2dlbV9vYmpfZ2d0dF9w aW4ob2JqLCAwLCBQSU5fTUFQUEFCTEUpOwo+ICsJaWYgKHJldCkKPiArCQlnb3RvIG91dF91bmxv Y2s7CgpJIHRoaW5rIGEgRFJNX0VSUk9SKCkgc2hvdWxkIGJlIGFkZGVkIGhlcmUsIG90aGVyd2lz ZSBpZiB0aGlzIGNoZWNrIGZhaWxzLAp0aGUgdXNlciB3aWxsIG5ldmVyIGJlIG5vdGlmaWVkIHRo ZXJlb2YuIEl0J3Mgbm90IHN1ZmZpY2llbnQgdG8ganVzdApyZXR1cm4gYSBuZWdhdGl2ZSBpbnQg YmVjYXVzZSB0aGlzIGp1c3QgZ2V0cyBwYXNzZWQgdXAgdGhlIGNhbGwgc3RhY2sgdG8KaW50ZWxf ZmJkZXZfaW5pdGlhbF9jb25maWcoKSB3aGljaCBpZ25vcmVzIGl0LgoKSSdtIGFkZGluZyBEUk1f RVJST1IoKSBpbnZvY2F0aW9ucyB0byB0aGUgdHdvIGV4aXN0aW5nIGVycm9yIGNvbmRpdGlvbnMg aW4KdGhpcyBmdW5jdGlvbiB3aXRoIHBhdGNoIDQgb2YgdGhlIHNlcmllcyBJIGp1c3QgcG9zdGVk IHRvIGludGVsLWdmeDoKCkxpbms6IGh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvYXJjaGl2 ZXMvaW50ZWwtZ2Z4LzIwMTUtT2N0b2Jlci8wNzg4MzcuaHRtbApNZXNzYWdlLUlkOiA8ZTA4YzEx M2MwNDE5MWVjZDY2OTA0NDIzZGE0ZjE5MDhmNDhiNWVmNi4xNDQ1NzcxNjkzLmdpdC5sdWthc0B3 dW5uZXIuZGU+CgpCZXN0IHJlZ2FyZHMsCgpMdWthcwoKCj4gKwo+ICAJaW5mbyA9IGRybV9mYl9o ZWxwZXJfYWxsb2NfZmJpKGhlbHBlcik7Cj4gIAlpZiAoSVNfRVJSKGluZm8pKSB7Cj4gIAkJcmV0 ID0gUFRSX0VSUihpbmZvKTsKPiBAQCAtMjc0LDYgKzI4NCw5IEBAIHN0YXRpYyBpbnQgaW50ZWxm Yl9jcmVhdGUoc3RydWN0IGRybV9mYl9oZWxwZXIgKmhlbHBlciwKPiAgb3V0X2Rlc3Ryb3lfZmJp Ogo+ICAJZHJtX2ZiX2hlbHBlcl9yZWxlYXNlX2ZiaShoZWxwZXIpOwo+ICBvdXRfdW5waW46Cj4g KwkvKiBPbmNlIGZvciBpbmZvLT5zY3JlZW5fYmFzZSBtbWFwaW5nLi4uICovCj4gKwlpOTE1X2dl bV9vYmplY3RfZ2d0dF91bnBpbihvYmopOwo+ICsJLyogLi4uYW5kIG9uY2UgZm9yIHRoZSBpbnRl bF9mYiAqLwo+ICAJaTkxNV9nZW1fb2JqZWN0X2dndHRfdW5waW4ob2JqKTsKPiAgCWRybV9nZW1f b2JqZWN0X3VucmVmZXJlbmNlKCZvYmotPmJhc2UpOwo+ICBvdXRfdW5sb2NrOgo+IEBAIC01MTQs NiArNTI3LDggQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fZmJfaGVscGVyX2Z1bmNzIGludGVs X2ZiX2hlbHBlcl9mdW5jcyA9IHsKPiAgc3RhdGljIHZvaWQgaW50ZWxfZmJkZXZfZGVzdHJveShz dHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ICAJCQkJc3RydWN0IGludGVsX2ZiZGV2ICppZmJkZXYp Cj4gIHsKPiArCS8qIFJlbGVhc2UgdGhlIHBpbm5pbmcgZm9yIHRoZSBpbmZvLT5zY3JlZW5fYmFz ZSBtbWFwaW5nLiAqLwo+ICsJaTkxNV9nZW1fb2JqZWN0X2dndHRfdW5waW4oaWZiZGV2LT5mYi0+ b2JqKTsKPiAgCj4gIAlkcm1fZmJfaGVscGVyX3VucmVnaXN0ZXJfZmJpKCZpZmJkZXYtPmhlbHBl cik7Cj4gIAlkcm1fZmJfaGVscGVyX3JlbGVhc2VfZmJpKCZpZmJkZXYtPmhlbHBlcik7Cj4gLS0g Cj4gMS45LjEKPiAKPiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwo+IEludGVsLWdmeCBtYWlsaW5nIGxpc3QKPiBJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCj4gaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2lu dGVsLWdmeApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJ bnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.hostsharing.net ([83.223.95.204]:59105 "EHLO mailout1.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751357AbbJYOnu (ORCPT ); Sun, 25 Oct 2015 10:43:50 -0400 Date: Sun, 25 Oct 2015 15:34:16 +0100 From: Lukas Wunner To: Wayne Boyer Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter , stable@vger.kernel.org, "Goel, Akash" Subject: Re: [Intel-gfx] [PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping Message-ID: <20151025143416.GA19554@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: Hi, 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. > > 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; I think a DRM_ERROR() should be added here, otherwise if this check fails, the user will never be notified thereof. It's not sufficient to just return a negative int because this just gets passed up the call stack to intel_fbdev_initial_config() which ignores it. I'm adding DRM_ERROR() invocations to the two existing error conditions in this function with patch 4 of the series I just posted to intel-gfx: Link: http://lists.freedesktop.org/archives/intel-gfx/2015-October/078837.html Message-Id: Best regards, Lukas > + > 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