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