All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH v2 0/2] fbdev fixes (need review)
Date: Sun, 8 Nov 2015 13:56:53 +0100	[thread overview]
Message-ID: <cover.1446987413.git.lukas@wunner.de> (raw)
In-Reply-To: <20151030182305.GQ16848@phenom.ffwll.local>

On Fri, Oct 30, 2015 at 07:23:45PM +0100, Daniel Vetter wrote:
> I don't think there's a leak here really. We always assign ifbdev->fb in
> intelfb_alloc, which means the fbdev teardown code will take care of it.
> The correct approach is probably to not unref anything at all, or if we
> unref then we have to clear ifbdev->fb (since it's the reference that very
> pointer is holding that we're clearing up).

Oh, right, and ifbdev->helper.fb would have to be set to NULL as well.

However it turns out the fbdev teardown code can't cope with an fbdev
that's only partially initialized, it lacks proper null pointer checks.
Patch 1 in this series fixes that.

Patch 2 is another attempt at handling failure of intelfb_create():
On failure, ifbdev->helper.fbdev will be released by intelfb_create()
but the intel_framebuffer and the gem_object are NOT unrefed. To make
up for this, intel_fbdev_initial_config() will call intel_fbdev_fini(),
which not only unrefs the intel_framebuffer and the gem_object but also
frees the ifbdev. It's as if CONFIG_DRM_FBDEV_EMULATION wasn't set.

Not sure if this is the right way to go but seems more sensible to me
than keeping an fbdev around that's only halfway initialized. We've
kicked out the VGA console at this point but failed to initialize an
fbdev, the console will thus be unusable. If X11 manages to start up
without errors, it will at least be able to use the memory that would
otherwise be hogged by the unusable fbdev.

Note that if you find patch 2 acceptable, the modifications in patch 1
to the following functions would actually become optional since they
already check if ifbdev is a null pointer (let me know if you want these
dropped): intel_fbdev_set_suspend(), intel_fbdev_output_poll_changed(),
intel_fbdev_restore_mode().

The patches are also browsable on GitHub:
https://github.com/l1k/linux/commits/intel_fbdev_fixes

Best regards,

Lukas


Lukas Wunner (2):
  drm/i915: Fix oops caused by fbdev initialization failure
  drm/i915: Tear down fbdev if initialization fails

 drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++-----------
 drivers/gpu/drm/i915/i915_dma.c     |  1 +
 drivers/gpu/drm/i915/intel_dp_mst.c | 10 ++++++++--
 drivers/gpu/drm/i915/intel_fbdev.c  | 25 +++++++++++++++----------
 4 files changed, 37 insertions(+), 23 deletions(-)

-- 
1.8.5.2 (Apple Git-48)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-11-08 16:16 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-30  9:06 [PATCH v5 1/2] drm/i915: Fix failure paths around initial fbdev allocation Lukas Wunner
2015-07-04  9:50 ` [PATCH v5 2/2] drm/i915: On fb alloc failure, unref gem object where it gets refed Lukas Wunner
2015-10-13 15:39   ` Ville Syrjälä
2015-10-13 15:04 ` [PATCH v5 1/2] drm/i915: Fix failure paths around initial fbdev allocation Ville Syrjälä
2015-10-14  9:35   ` Chris Wilson
2015-10-15 17:14   ` Lukas Wunner
2015-10-15 17:22     ` Daniel Vetter
2015-10-15 17:34     ` Ville Syrjälä
2015-10-18 18:03       ` Lukas Wunner
2015-10-25 11:14       ` [PATCH v6 0/4] fbdev deadlock & failure path fixes Lukas Wunner
2015-06-30  9:06         ` [PATCH v6 3/4] drm/i915: Fix failure paths around initial fbdev allocation Lukas Wunner
2015-10-30 18:28           ` Daniel Vetter
2015-11-07 10:41             ` [PATCH v7 0/3] fbdev fixes (reviewed) Lukas Wunner
2015-06-30  9:06               ` [PATCH v7 3/3] drm/i915: Fix failure paths around initial fbdev allocation Lukas Wunner
2015-07-04  9:50               ` [PATCH v7 1/3] drm/i915: On fb alloc failure, unref gem object where it gets refed Lukas Wunner
2015-10-22 11:37               ` [PATCH v7 2/3] drm/i915: Fix double unref in intelfb_alloc failure path Lukas Wunner
2015-11-09 14:23               ` [PATCH v7 0/3] fbdev fixes (reviewed) Jani Nikula
2015-11-12 19:20                 ` Lukas Wunner
2015-11-13  7:06                   ` Jani Nikula
2015-11-17 13:44                     ` Daniel Vetter
2015-07-04  9:50         ` [PATCH v6 1/4] drm/i915: On fb alloc failure, unref gem object where it gets refed Lukas Wunner
2015-10-22 11:37         ` [PATCH v6 2/4] drm/i915: Fix double unref in intelfb_alloc failure path Lukas Wunner
2015-10-30 18:17           ` Daniel Vetter
2015-10-23 22:27         ` [PATCH v6 4/4] drm/i915: Fix error handling in intelfb_create Lukas Wunner
2015-10-30 18:23           ` Daniel Vetter
2015-11-08 12:56             ` Lukas Wunner [this message]
2015-11-03  7:00               ` [PATCH v2 2/2] drm/i915: Tear down fbdev if initialization fails Lukas Wunner
2015-11-05  9:42               ` [PATCH v2 1/2] drm/i915: Fix oops caused by fbdev initialization failure Lukas Wunner
2015-11-17 13:51               ` [PATCH v2 0/2] fbdev fixes (need review) Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1446987413.git.lukas@wunner.de \
    --to=lukas@wunner.de \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.