From patchwork Wed Dec 18 11:48:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 3369751 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5887E9F32E for ; Wed, 18 Dec 2013 11:48:35 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B1D5C20443 for ; Wed, 18 Dec 2013 11:48:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 407D12015F for ; Wed, 18 Dec 2013 11:48:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753324Ab3LRLsb (ORCPT ); Wed, 18 Dec 2013 06:48:31 -0500 Received: from mail-ee0-f41.google.com ([74.125.83.41]:65161 "EHLO mail-ee0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753321Ab3LRLsa (ORCPT ); Wed, 18 Dec 2013 06:48:30 -0500 Received: by mail-ee0-f41.google.com with SMTP id t10so3498115eei.14 for ; Wed, 18 Dec 2013 03:48:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=XXJO7UViZhRGi+s72mVnBzjTYj0DLNMWNw3H6MmvIl8=; b=K9hTTSJQJkfTPpEtPNWOOUakRlGsLHyiWXNs/VoHaLNKSwnGWjaIvDe7/yoV16cUaE /wnLEoLABPa/Pk+Fm2REAIT4QWHQPMn+exK5zvFEdUl14gjJFgxoRk2K7XrAd1wnTdqJ Y4fSmpLiwgN78yEQttDyOiBfP8RENACtdbNuzSXYW/VRC5Fy77NmIRoTC7IxpCvUV/Y/ i3N1U6iSjZ21yxXGSZF1LDFfV1w4FiDXkS/WzxFFMtChS00X5mlxWyjdMnIz/J8DkCSH /eFF9EEGC77Znoiu6VDsAoZaHlaa2FZh3svMfR7b8k9LDpnN2osY4rpxKeWxvsdfJJgQ EZ+g== X-Received: by 10.14.223.198 with SMTP id v46mr28200930eep.20.1387367308903; Wed, 18 Dec 2013 03:48:28 -0800 (PST) Received: from david-ub.localdomain (stgt-5f703ec4.pool.mediaWays.net. [95.112.62.196]) by mx.google.com with ESMTPSA id b41sm61756288eef.16.2013.12.18.03.48.26 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 18 Dec 2013 03:48:27 -0800 (PST) From: David Herrmann To: linux-kernel@vger.kernel.org Cc: Takashi Iwai , Stephen Warren , the arch/x86 maintainers , linux-fbdev@vger.kernel.org, Geert Uytterhoeven , Ingo Molnar , David Herrmann Subject: [RFC] x86: sysfb: remove sysfb when probing real hw Date: Wed, 18 Dec 2013 12:48:03 +0100 Message-Id: <1387367283-20078-1-git-send-email-dh.herrmann@gmail.com> X-Mailer: git-send-email 1.8.5.1 In-Reply-To: References: Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If we probe a real hw driver for graphics devices, we need to unload any generic fallback driver like efifb/vesafb/simplefb on the system framebuffer. This is currently done via remove_conflicting_framebuffers() in fbmem.c. However, this only removes the fbdev driver, not the fake platform devices underneath. This hasn't been a problem so far, as efifb and vesafb didn't store any resources there. However, with simplefb this changed. To correctly release the IORESOURCE_MEM resources of simple-framebuffer platform devices, we need to unregister the underlying platform device *before* probing any new hw driver. This patch adds sysfb_unregister() for that purpose. It can be called from any context (except from the platform-device ->remove callback path) and synchronously unloads any global sysfb and prevents new sysfbs from getting registered. Thus, you can call it even before any sysfb has been loaded. This also changes remove_conflicting_framebuffer() to call this helper *before* trying it's fbdev heuristic to remove conflicting drivers. Signed-off-by: David Herrmann --- Hi This is imho the clean version of Takashi's fix. However, it gets pretty huge. I wouldn't object to marking CONFIG_X86_SYSFB broken in the stable series and get this in for 3.14. Your call.. This patch basically simulates an unplug event for system-framebuffers when loading real hardware drivers. To trigger it, call sysfb_unregister(). You can optionally pass an aperture-struct and primary-flag similar to remove_conflicting_framebuffers(). If they're not passed, we remove it unconditionally. Untested, but my kernel compiles are already running. If my tests succeed and nobody has objections, I can resend it as proper PATCH and marked for stable. And maybe split the fbmem and sysfb changes into two patches.. Thanks David arch/x86/include/asm/sysfb.h | 10 ++++ arch/x86/kernel/sysfb.c | 103 +++++++++++++++++++++++++++++++++++++-- arch/x86/kernel/sysfb_simplefb.c | 5 +- drivers/video/fbmem.c | 16 ++++++ 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/sysfb.h b/arch/x86/include/asm/sysfb.h index 2aeb3e2..713bc17 100644 --- a/arch/x86/include/asm/sysfb.h +++ b/arch/x86/include/asm/sysfb.h @@ -11,6 +11,7 @@ * any later version. */ +#include #include #include #include @@ -59,6 +60,15 @@ struct efifb_dmi_info { int flags; }; +__init struct platform_device *sysfb_alloc(const char *name, + int id, + const struct resource *res, + unsigned int res_num, + const void *data, + size_t data_size); +__init int sysfb_register(struct platform_device *dev); +void sysfb_unregister(const struct apertures_struct *apert, bool primary); + #ifdef CONFIG_EFI extern struct efifb_dmi_info efifb_dmi_list[]; diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c index 193ec2c..3d4554e 100644 --- a/arch/x86/kernel/sysfb.c +++ b/arch/x86/kernel/sysfb.c @@ -33,11 +33,106 @@ #include #include #include +#include #include #include #include #include +static DEFINE_MUTEX(sysfb_lock); +static struct platform_device *sysfb_dev; + +/* register @dev as sysfb; takes ownership over @dev */ +__init int sysfb_register(struct platform_device *dev) +{ + int r = 0; + + mutex_lock(&sysfb_lock); + if (!sysfb_dev) { + r = platform_device_add(dev); + if (r < 0) + put_device(&dev->dev); + else + sysfb_dev = dev; + } else { + /* if there already is/was a sysfb, destroy @pd but return 0 */ + platform_device_put(dev); + } + mutex_unlock(&sysfb_lock); + + return r; +} + +static bool sysfb_match(const struct apertures_struct *apert, bool primary) +{ + struct screen_info *si = &screen_info; + unsigned int i; + const struct aperture *a; + + if (!apert || primary) + return true; + + for (i = 0; i < apert->count; ++i) { + a = &apert->ranges[i]; + if (a->base >= si->lfb_base && + a->base < si->lfb_base + ((u64)si->lfb_size << 16)) + return true; + if (si->lfb_base >= a->base && + si->lfb_base < a->base + a->size) + return true; + } + + return false; +} + +/* unregister the sysfb and prevent new sysfbs from getting registered */ +void sysfb_unregister(const struct apertures_struct *apert, bool primary) +{ + + mutex_lock(&sysfb_lock); + if (!IS_ERR(sysfb_dev)) { + if (sysfb_dev) { + if (sysfb_match(apert, primary)) { + platform_device_del(sysfb_dev); + platform_device_put(sysfb_dev); + sysfb_dev = ERR_PTR(-EALREADY); + } + } else { + sysfb_dev = ERR_PTR(-EALREADY); + } + } + mutex_unlock(&sysfb_lock); +} + +__init struct platform_device *sysfb_alloc(const char *name, + int id, + const struct resource *res, + unsigned int res_num, + const void *data, + size_t data_size) +{ + struct platform_device *pd; + int ret; + + pd = platform_device_alloc(name, id); + if (!pd) + return ERR_PTR(-ENOMEM); + + ret = platform_device_add_resources(pd, res, res_num); + if (ret) + goto err; + + ret = platform_device_add_data(pd, data, data_size); + if (ret) + goto err; + + return pd; + +err: + platform_device_put(pd); + return ERR_PTR(ret); +} + static __init int sysfb_init(void) { struct screen_info *si = &screen_info; @@ -65,9 +160,11 @@ static __init int sysfb_init(void) else name = "platform-framebuffer"; - pd = platform_device_register_resndata(NULL, name, 0, - NULL, 0, si, sizeof(*si)); - return IS_ERR(pd) ? PTR_ERR(pd) : 0; + pd = sysfb_alloc(name, 0, NULL, 0, si, sizeof(*si)); + if (IS_ERR(pd)) + return PTR_ERR(pd); + + return sysfb_register(pd); } /* must execute after PCI subsystem for EFI quirks */ diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c index 86179d4..8e7bd23 100644 --- a/arch/x86/kernel/sysfb_simplefb.c +++ b/arch/x86/kernel/sysfb_simplefb.c @@ -86,10 +86,9 @@ __init int create_simplefb(const struct screen_info *si, if (res.end <= res.start) return -EINVAL; - pd = platform_device_register_resndata(NULL, "simple-framebuffer", 0, - &res, 1, mode, sizeof(*mode)); + pd = sysfb_alloc("simple-framebuffer", 0, &res, 1, mode, sizeof(*mode)); if (IS_ERR(pd)) return PTR_ERR(pd); - return 0; + return sysfb_register(pd); } diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 010d191..53e3894 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -35,6 +35,9 @@ #include +#if IS_ENABLED(CONFIG_X86_SYSFB) +#include +#endif /* * Frame buffer device initialization and setup routines @@ -1604,6 +1607,14 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, } } +static void remove_conflicting_sysfb(const struct apertures_struct *apert, + bool primary) +{ +#if IS_ENABLED(CONFIG_X86_SYSFB) + sysfb_unregister(apert, primary); +#endif +} + static int do_register_framebuffer(struct fb_info *fb_info) { int i; @@ -1742,6 +1753,8 @@ EXPORT_SYMBOL(unlink_framebuffer); void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name, bool primary) { + remove_conflicting_sysfb(a, primary); + mutex_lock(®istration_lock); do_remove_conflicting_framebuffers(a, name, primary); mutex_unlock(®istration_lock); @@ -1762,6 +1775,9 @@ register_framebuffer(struct fb_info *fb_info) { int ret; + remove_conflicting_sysfb(fb_info->apertures, + fb_is_primary_device(fb_info)); + mutex_lock(®istration_lock); ret = do_register_framebuffer(fb_info); mutex_unlock(®istration_lock);