Hi Chris,
On Fri, Nov 20, 2015 at 04:29:52PM +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.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: "Goel, Akash" <akash.goel@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 7ccde58f8c98..79f02e72da8a 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,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);
If the call to intelfb_alloc() failed, ifbdev->fb will be NULL and
intelfb_create() will return a non-zero value. Further up in the call
stack, intel_fbdev_initial_config() will then clobber the fbdev by
calling intel_fbdev_destroy(). This will oops because you dereference
ifbdev->fb here. So you need to add:
if (ifbdev->fb)
If intel_pin_and_fence_fb_obj() failed, intelfb_create() will likewise
return a non-zero value and the fbdev gets clobbered. This will WARN
because you're calling i915_gem_object_ggtt_unpin() even though the
pin_count is 0.
Best regards,
Lukas
>
> drm_fb_helper_unregister_fbi(&ifbdev->helper);
> drm_fb_helper_release_fbi(&ifbdev->helper);
> --
> 2.6.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
