Re: [Intel-gfx] [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 17 Dec 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Dec 10, 2015 at 05:41:30PM +0100, Takashi Iwai wrote:
>> On Thu, 10 Dec 2015 17:36:04 +0100,
>> Ville Syrjälä wrote:
>> > 
>> > 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 <chris@chris-wilson.co.uk>
>> > > Cc: "Goel, Akash" <akash.goel@intel.com>
>> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > > Cc: Lukas Wunner <lukas@wunner.de>
>> > > 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ä <ville.syrjala@linux.intel.com>
>> > 
>> > Takashi, don't know if you already found this patch, but it's definitely
>> > something you should try as well.
>> 
>> Great, I'll give this a try.  Thanks!
>
> Pulled both patches into dinq. Jani, can you please cherry-pick?

Picked the first, but I don't have the time to fix the conflicts on the
second one.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center