From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D3B5BC433EF for ; Tue, 5 Apr 2022 10:34:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D9CF810ED6B; Tue, 5 Apr 2022 10:34:45 +0000 (UTC) Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3EB1D10ED6B for ; Tue, 5 Apr 2022 10:34:44 +0000 (UTC) Received: by mail-oi1-x22f.google.com with SMTP id j83so12933598oih.6 for ; Tue, 05 Apr 2022 03:34:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=Ckyr/H4Efk9eg4/2J/KBC2fSKKkO3KzksdjLcY3RBSzT9NMCmfFcmXQP/tFtKO/470 cq+ZnczUI9w3TuhLKpo/lVk0VJYzTyel84GgUrfYKCT9mv2hijbapBsI0e1FG7FrRJmb fLucT+sdtvHXDNR/H3K7C3fno1IoUuh9gjqUQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=Y3gHOp5CCvqkASGjy02k13nAnczMAN199EOm0pCpkZrVetBytJ6rvUBWERPKrb64WS KlH7c550s4JUzzuRbpZ+bDVAmiYgGiRfYRptVxcMViO7hQ5VZhnVLpOotfKToSt+CpJP CusO8JERTshPT80uM6yYxBFPiupytfrPzJJh8HYbi0+shhSBiXQqlHrzg7yBOl5NYMwF Mf3XzPji8sjxjdvX7yXsUsr0n/HReOmBOc7OTYAKjHyLj/U3wcPYqW5WguuTyguNsZ9b TZhBmvr8CO6puW/iyKmGC0mhSuoRCeM4IIFBo66wYDhgvHjMmAp5CYMhEQXDwCVvCGvh gQWg== X-Gm-Message-State: AOAM531P25vz30YfT0GtLIutmikK4wYMadJ0TPUg0sWVn7fhYXSTgc79 hlVmehF+n7DYE9DK6/La9DrHpoCFCsOL7jTgLzZ77A== X-Google-Smtp-Source: ABdhPJzqoYVSx47i8TUU2SwXKC01iWiGjmQ1+pUIxIWzSlGlgOjCn7FBVo4L4bnb1FUmBmYZzaS59mRu1JbpVJ6Lf+k= X-Received: by 2002:a05:6808:1596:b0:2f7:5d89:eec7 with SMTP id t22-20020a056808159600b002f75d89eec7mr1125899oiw.228.1649154883395; Tue, 05 Apr 2022 03:34:43 -0700 (PDT) MIME-Version: 1.0 References: <20220208210824.2238981-1-daniel.vetter@ffwll.ch> <20220208210824.2238981-19-daniel.vetter@ffwll.ch> <4ae20b63-f452-fdb4-ced6-d4968a8d69f0@redhat.com> <408ffe9b-f09f-dc7e-7f5e-a93b311a06fa@redhat.com> In-Reply-To: From: Daniel Vetter Date: Tue, 5 Apr 2022 12:34:31 +0200 Message-ID: Subject: Re: [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered" To: Javier Martinez Canillas , Greg KH Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, Intel Graphics Development , LKML , DRI Development , Hans de Goede , Peter Jones , Thomas Zimmermann , Ilya Trukhanov , Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, 5 Apr 2022 at 11:52, Javier Martinez Canillas wrote: > > On 4/5/22 11:24, Daniel Vetter wrote: > > On Tue, 5 Apr 2022 at 11:19, Javier Martinez Canillas > > [snip] > > >> > >> This is how I think that work, please let me know if you see something > >> wrong in my logic: > >> > >> 1) A PCI device of OF device is registered for the GPU, this attempt to > >> match a registered driver but no driver was registered that match yet. > >> > >> 2) The efifb driver is built-in, will be initialized according to the link > >> order of the objects under drivers/video and the fbdev driver is registered. > >> > >> There is no platform device or PCI/OF device registered that matches. > >> > >> 3) The DRM driver is built-in, will be initialized according to the link > >> order of the objects under drivers/gpu and the DRM driver is registered. > >> > >> This matches the device registered in (1) and the DRM driver probes. > >> > >> 4) The DRM driver .probe kicks out any conflicting DRM drivers and pdev > >> before registering the DRM device. > >> > >> There are no conflicting drivers or platform device at this point. > >> > >> 5) Latter at some point the drivers/firmware/sysfb.c init function is > >> executed, and this registers a platform device for the generic fb. > >> > >> This device matches the efifb driver registered in (2) and the fbdev > >> driver probes. > >> > >> Since that happens *after* the DRM driver already matched, probed > >> and registered the DRM device, that is a bug and what the reverted > >> patch worked around. > >> > >> So we need to prevent (5) if (1) and (3) already happened. Having a flag > >> set in the fbdev core somewhere when remove_conflicting_framebuffers() > >> is called could be a solution indeed. > >> > >> That is, the fbdev core needs to know that a DRM driver already probed > >> and make register_framebuffer() fail if info->flag & FBINFO_MISC_FIRMWARE > >> > >> I can attempt to write a patch for that. > > > > Ah yeah that could be an issue. I think the right fix is to replace > > the platform dev unregister with a sysfb_unregister() function in > > sysfb.c, which is synced with a common lock with the sysfb_init > > function and a small boolean. I think I can type that up quickly for > > v3. > > It's more complicated than that since sysfb is just *one* of the several > places where platform devices can be registered for video devices. > > For instance, the vga16fb driver registers its own platform device in > its module_init() function so that can also happen after the conflicting > framebuffers (and associated devices) were removed by a DRM driver probe. > > I tried to minimize the issue for that particular driver with commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0499f419b76f > > But the point stands, it all boils down to the fact that you have two > different subsystems registering video drivers and they don't know all > about each other to take a proper decision. > > Right now the drm_aperture_remove_conflicting_framebuffers() call signals > in one direction from DRM to fbdev but there isn't a communication in the > other direction, from fbdev to DRM. > > I believe the correct fix would be for the fbdev core to keep a list of > the apertures struct that are passed to remove_conflicting_framebuffers(), > that way it will know what apertures are not available anymore and prevent > to register any fbdev framebuffer that conflicts with one already present. Hm that still feels like reinventing a driver model, badly. I think there's two cleaner solutions: - move all the firmware driver platform_dev into sysfb.c, and then just bind the special cases against that (e.g. offb, vga16fb and all these). Then we'd have one sysfb_try_unregister(struct device *dev) interface that fbmem.c uses. - let fbmem.c call into each of these firmware device providers, which means some loops most likely (like we can't call into vga16fb), so probably need to move that into fbmem.c and it all gets a bit messy. > Let me know if you think that makes sense and I can attempt to write a fix. I still think unregistering the platform_dev properly makes the most sense, and feels like the most proper linux device model solution instead of hacks on top - if the firmware fb is unuseable because a native driver has taken over, we should nuke that. And also the firmware fb driver would then just bind to that platform_dev if it exists, and only if it exists. Also I think it should be the responsibility of whichever piece of code that registers these platform devices to ensure that platform_dev actually still exists. That's why I think pushing all that code into sysfb.c is probably the cleanest solution. fbdev predates all that stuff by a lot, hence the hand-rolling. But maybe Greg has some more thoughts here too? -Daniel > > -- > Best regards, > > Javier Martinez Canillas > Linux Engineering > Red Hat > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0469DC433F5 for ; Tue, 5 Apr 2022 10:34:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8C4E510ED5B; Tue, 5 Apr 2022 10:34:44 +0000 (UTC) Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3887110ED5B for ; Tue, 5 Apr 2022 10:34:44 +0000 (UTC) Received: by mail-oi1-x236.google.com with SMTP id q129so12944083oif.4 for ; Tue, 05 Apr 2022 03:34:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=Ckyr/H4Efk9eg4/2J/KBC2fSKKkO3KzksdjLcY3RBSzT9NMCmfFcmXQP/tFtKO/470 cq+ZnczUI9w3TuhLKpo/lVk0VJYzTyel84GgUrfYKCT9mv2hijbapBsI0e1FG7FrRJmb fLucT+sdtvHXDNR/H3K7C3fno1IoUuh9gjqUQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=vag+alTInQEO9956vybkBwSrmwYlcLinf4qOZRDot7m3IKdwIqecRdCNdfmjTuQ5G+ JIqlvviy4PlxFBwntLOAiHCYechgJ6kwbhBnEk20CQ3h3SV3OiPt0ix8ZwseQ184BRUl oW0s6Lc8a3t3EbDcB1N5FSQaxv07UnZ9mI/8+aYppd/l4uX4dmJazSX6TKjGktZrhh3S S3JepxTzn/zbnkgfYQiFWcEyj7kGHETtnv/U9PYvj+U2sHqRPTUBiUpVbRXWr/RDjaae B/LL1u9sLTSNNSCH7DqPPtFyNbsXhM4ZrZ0WfYE7PrjPLWm5TrBma5g+dLKivhKfdXRR ZUCg== X-Gm-Message-State: AOAM531EREo0id5+CNVWoUkpPyQZelvZFnxgLu0WCPJljCsp5dOy2OLC I5F41/i/HEPCzlqCfdihT1ilE/eckl/Gkncr8fyDqA== X-Google-Smtp-Source: ABdhPJzqoYVSx47i8TUU2SwXKC01iWiGjmQ1+pUIxIWzSlGlgOjCn7FBVo4L4bnb1FUmBmYZzaS59mRu1JbpVJ6Lf+k= X-Received: by 2002:a05:6808:1596:b0:2f7:5d89:eec7 with SMTP id t22-20020a056808159600b002f75d89eec7mr1125899oiw.228.1649154883395; Tue, 05 Apr 2022 03:34:43 -0700 (PDT) MIME-Version: 1.0 References: <20220208210824.2238981-1-daniel.vetter@ffwll.ch> <20220208210824.2238981-19-daniel.vetter@ffwll.ch> <4ae20b63-f452-fdb4-ced6-d4968a8d69f0@redhat.com> <408ffe9b-f09f-dc7e-7f5e-a93b311a06fa@redhat.com> In-Reply-To: From: Daniel Vetter Date: Tue, 5 Apr 2022 12:34:31 +0200 Message-ID: To: Javier Martinez Canillas , Greg KH Content-Type: text/plain; charset="UTF-8" Subject: Re: [Intel-gfx] [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered" X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, Intel Graphics Development , LKML , DRI Development , Peter Jones , Thomas Zimmermann , Ilya Trukhanov , Daniel Vetter , Zack Rusin Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 5 Apr 2022 at 11:52, Javier Martinez Canillas wrote: > > On 4/5/22 11:24, Daniel Vetter wrote: > > On Tue, 5 Apr 2022 at 11:19, Javier Martinez Canillas > > [snip] > > >> > >> This is how I think that work, please let me know if you see something > >> wrong in my logic: > >> > >> 1) A PCI device of OF device is registered for the GPU, this attempt to > >> match a registered driver but no driver was registered that match yet. > >> > >> 2) The efifb driver is built-in, will be initialized according to the link > >> order of the objects under drivers/video and the fbdev driver is registered. > >> > >> There is no platform device or PCI/OF device registered that matches. > >> > >> 3) The DRM driver is built-in, will be initialized according to the link > >> order of the objects under drivers/gpu and the DRM driver is registered. > >> > >> This matches the device registered in (1) and the DRM driver probes. > >> > >> 4) The DRM driver .probe kicks out any conflicting DRM drivers and pdev > >> before registering the DRM device. > >> > >> There are no conflicting drivers or platform device at this point. > >> > >> 5) Latter at some point the drivers/firmware/sysfb.c init function is > >> executed, and this registers a platform device for the generic fb. > >> > >> This device matches the efifb driver registered in (2) and the fbdev > >> driver probes. > >> > >> Since that happens *after* the DRM driver already matched, probed > >> and registered the DRM device, that is a bug and what the reverted > >> patch worked around. > >> > >> So we need to prevent (5) if (1) and (3) already happened. Having a flag > >> set in the fbdev core somewhere when remove_conflicting_framebuffers() > >> is called could be a solution indeed. > >> > >> That is, the fbdev core needs to know that a DRM driver already probed > >> and make register_framebuffer() fail if info->flag & FBINFO_MISC_FIRMWARE > >> > >> I can attempt to write a patch for that. > > > > Ah yeah that could be an issue. I think the right fix is to replace > > the platform dev unregister with a sysfb_unregister() function in > > sysfb.c, which is synced with a common lock with the sysfb_init > > function and a small boolean. I think I can type that up quickly for > > v3. > > It's more complicated than that since sysfb is just *one* of the several > places where platform devices can be registered for video devices. > > For instance, the vga16fb driver registers its own platform device in > its module_init() function so that can also happen after the conflicting > framebuffers (and associated devices) were removed by a DRM driver probe. > > I tried to minimize the issue for that particular driver with commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0499f419b76f > > But the point stands, it all boils down to the fact that you have two > different subsystems registering video drivers and they don't know all > about each other to take a proper decision. > > Right now the drm_aperture_remove_conflicting_framebuffers() call signals > in one direction from DRM to fbdev but there isn't a communication in the > other direction, from fbdev to DRM. > > I believe the correct fix would be for the fbdev core to keep a list of > the apertures struct that are passed to remove_conflicting_framebuffers(), > that way it will know what apertures are not available anymore and prevent > to register any fbdev framebuffer that conflicts with one already present. Hm that still feels like reinventing a driver model, badly. I think there's two cleaner solutions: - move all the firmware driver platform_dev into sysfb.c, and then just bind the special cases against that (e.g. offb, vga16fb and all these). Then we'd have one sysfb_try_unregister(struct device *dev) interface that fbmem.c uses. - let fbmem.c call into each of these firmware device providers, which means some loops most likely (like we can't call into vga16fb), so probably need to move that into fbmem.c and it all gets a bit messy. > Let me know if you think that makes sense and I can attempt to write a fix. I still think unregistering the platform_dev properly makes the most sense, and feels like the most proper linux device model solution instead of hacks on top - if the firmware fb is unuseable because a native driver has taken over, we should nuke that. And also the firmware fb driver would then just bind to that platform_dev if it exists, and only if it exists. Also I think it should be the responsibility of whichever piece of code that registers these platform devices to ensure that platform_dev actually still exists. That's why I think pushing all that code into sysfb.c is probably the cleanest solution. fbdev predates all that stuff by a lot, hence the hand-rolling. But maybe Greg has some more thoughts here too? -Daniel > > -- > Best regards, > > Javier Martinez Canillas > Linux Engineering > Red Hat > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29739C43219 for ; Tue, 5 Apr 2022 21:18:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232069AbiDEVQy (ORCPT ); Tue, 5 Apr 2022 17:16:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50854 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1355436AbiDELOr (ORCPT ); Tue, 5 Apr 2022 07:14:47 -0400 Received: from mail-oi1-x22a.google.com (mail-oi1-x22a.google.com [IPv6:2607:f8b0:4864:20::22a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 84F194D62A for ; Tue, 5 Apr 2022 03:34:44 -0700 (PDT) Received: by mail-oi1-x22a.google.com with SMTP id z8so12937958oix.3 for ; Tue, 05 Apr 2022 03:34:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=Ckyr/H4Efk9eg4/2J/KBC2fSKKkO3KzksdjLcY3RBSzT9NMCmfFcmXQP/tFtKO/470 cq+ZnczUI9w3TuhLKpo/lVk0VJYzTyel84GgUrfYKCT9mv2hijbapBsI0e1FG7FrRJmb fLucT+sdtvHXDNR/H3K7C3fno1IoUuh9gjqUQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tzmhcp5RxadQONeu9KvciYKiNgXbRJejI1iC090AVWg=; b=RmAjORmcNhejd4HCi9U9MkW++uipkhHohUZ0DOqWhcMWp1PpRJ6QLXXpIcgk+dNvLW APvLyl4gBLIuAchy1dP7aTu1lojV1CmC52GqPv+6WvnDz75nih5j2GbFojX3UnoewFZa PkTJNI8wQYxS8ih1+nXj7TTgNZw8A78U14uUzunNKWxaXnU1b5apo958lXsLiZtr8fd4 Ui0Z4jTn40XxT01p/xR/TeU9LuN3MDhv9ZexdooJQrVqyNnpjIu1I1u1EnbQz5DinpyX /I7Twby5DfMPk4/F1nZPpoehm9JWwl9wGr/qgXt6KGAWsGbTVqVJrVkoAHaEMU7noSkQ BmKQ== X-Gm-Message-State: AOAM53171uJSW0SDI7AjSplq7lK/1thR5Yy0GAby7+FftrwjYIUki2fp jNw8ThULr1QUrcoOSWvMv8ComtqdU6cuvogMLtyP9Q== X-Google-Smtp-Source: ABdhPJzqoYVSx47i8TUU2SwXKC01iWiGjmQ1+pUIxIWzSlGlgOjCn7FBVo4L4bnb1FUmBmYZzaS59mRu1JbpVJ6Lf+k= X-Received: by 2002:a05:6808:1596:b0:2f7:5d89:eec7 with SMTP id t22-20020a056808159600b002f75d89eec7mr1125899oiw.228.1649154883395; Tue, 05 Apr 2022 03:34:43 -0700 (PDT) MIME-Version: 1.0 References: <20220208210824.2238981-1-daniel.vetter@ffwll.ch> <20220208210824.2238981-19-daniel.vetter@ffwll.ch> <4ae20b63-f452-fdb4-ced6-d4968a8d69f0@redhat.com> <408ffe9b-f09f-dc7e-7f5e-a93b311a06fa@redhat.com> In-Reply-To: From: Daniel Vetter Date: Tue, 5 Apr 2022 12:34:31 +0200 Message-ID: Subject: Re: [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered" To: Javier Martinez Canillas , Greg KH Cc: DRI Development , Intel Graphics Development , linux-fbdev@vger.kernel.org, LKML , Thomas Zimmermann , Zack Rusin , Hans de Goede , Ilya Trukhanov , Daniel Vetter , Peter Jones Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 5 Apr 2022 at 11:52, Javier Martinez Canillas wrote: > > On 4/5/22 11:24, Daniel Vetter wrote: > > On Tue, 5 Apr 2022 at 11:19, Javier Martinez Canillas > > [snip] > > >> > >> This is how I think that work, please let me know if you see something > >> wrong in my logic: > >> > >> 1) A PCI device of OF device is registered for the GPU, this attempt to > >> match a registered driver but no driver was registered that match yet. > >> > >> 2) The efifb driver is built-in, will be initialized according to the link > >> order of the objects under drivers/video and the fbdev driver is registered. > >> > >> There is no platform device or PCI/OF device registered that matches. > >> > >> 3) The DRM driver is built-in, will be initialized according to the link > >> order of the objects under drivers/gpu and the DRM driver is registered. > >> > >> This matches the device registered in (1) and the DRM driver probes. > >> > >> 4) The DRM driver .probe kicks out any conflicting DRM drivers and pdev > >> before registering the DRM device. > >> > >> There are no conflicting drivers or platform device at this point. > >> > >> 5) Latter at some point the drivers/firmware/sysfb.c init function is > >> executed, and this registers a platform device for the generic fb. > >> > >> This device matches the efifb driver registered in (2) and the fbdev > >> driver probes. > >> > >> Since that happens *after* the DRM driver already matched, probed > >> and registered the DRM device, that is a bug and what the reverted > >> patch worked around. > >> > >> So we need to prevent (5) if (1) and (3) already happened. Having a flag > >> set in the fbdev core somewhere when remove_conflicting_framebuffers() > >> is called could be a solution indeed. > >> > >> That is, the fbdev core needs to know that a DRM driver already probed > >> and make register_framebuffer() fail if info->flag & FBINFO_MISC_FIRMWARE > >> > >> I can attempt to write a patch for that. > > > > Ah yeah that could be an issue. I think the right fix is to replace > > the platform dev unregister with a sysfb_unregister() function in > > sysfb.c, which is synced with a common lock with the sysfb_init > > function and a small boolean. I think I can type that up quickly for > > v3. > > It's more complicated than that since sysfb is just *one* of the several > places where platform devices can be registered for video devices. > > For instance, the vga16fb driver registers its own platform device in > its module_init() function so that can also happen after the conflicting > framebuffers (and associated devices) were removed by a DRM driver probe. > > I tried to minimize the issue for that particular driver with commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0499f419b76f > > But the point stands, it all boils down to the fact that you have two > different subsystems registering video drivers and they don't know all > about each other to take a proper decision. > > Right now the drm_aperture_remove_conflicting_framebuffers() call signals > in one direction from DRM to fbdev but there isn't a communication in the > other direction, from fbdev to DRM. > > I believe the correct fix would be for the fbdev core to keep a list of > the apertures struct that are passed to remove_conflicting_framebuffers(), > that way it will know what apertures are not available anymore and prevent > to register any fbdev framebuffer that conflicts with one already present. Hm that still feels like reinventing a driver model, badly. I think there's two cleaner solutions: - move all the firmware driver platform_dev into sysfb.c, and then just bind the special cases against that (e.g. offb, vga16fb and all these). Then we'd have one sysfb_try_unregister(struct device *dev) interface that fbmem.c uses. - let fbmem.c call into each of these firmware device providers, which means some loops most likely (like we can't call into vga16fb), so probably need to move that into fbmem.c and it all gets a bit messy. > Let me know if you think that makes sense and I can attempt to write a fix. I still think unregistering the platform_dev properly makes the most sense, and feels like the most proper linux device model solution instead of hacks on top - if the firmware fb is unuseable because a native driver has taken over, we should nuke that. And also the firmware fb driver would then just bind to that platform_dev if it exists, and only if it exists. Also I think it should be the responsibility of whichever piece of code that registers these platform devices to ensure that platform_dev actually still exists. That's why I think pushing all that code into sysfb.c is probably the cleanest solution. fbdev predates all that stuff by a lot, hence the hand-rolling. But maybe Greg has some more thoughts here too? -Daniel > > -- > Best regards, > > Javier Martinez Canillas > Linux Engineering > Red Hat > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch