fbdev fixes (need review)
Get series with b4
b4 am [email protected]Versions newest first
- v2 Latest2 patches 1 replyIn review
Patches in v2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | drm/i915: Fix oops caused by fbdev initialization failuredrivers/ drivers/ drivers/ | +31 | 0 replies | |
| 2/2 | drm/i915: Tear down fbdev if initialization failsdrivers/ drivers/ | +6 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.
Cover letter [PATCH v2 0/2] fbdev fixes (need review)
Quoted 5 lines · Daniel Vetter
I don't think there's a leak here really. We always assign
ifbdev->fbinintelfb_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 clearifbdev->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(-)
Signature · 6 lines
1.8.5.2 (Apple Git-48) _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx