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 53402C8303C for ; Mon, 7 Jul 2025 10:02:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A688210E337; Mon, 7 Jul 2025 10:02:49 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (1024-bit key; unprotected) header.d=163.com header.i=@163.com header.b="n8lMu26K"; dkim-atps=neutral Received: from m16.mail.163.com (m16.mail.163.com [220.197.31.5]) by gabe.freedesktop.org (Postfix) with ESMTP id 0295410E337 for ; Mon, 7 Jul 2025 10:02:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:To:From: Content-Type; bh=JKlpTevxu85gp6f6igNYxYtQcdo+vMwHhI23b2Bpl/U=; b=n8lMu26KcKMwR2qks3e+G25Tf7fLeySn28QSabCIcoGWDfYAqRFAT8oVYmEwiL PcqxKaa/n0CmUkq9DQArZAhyApzTQvLMfj7ovRkehozIolYHm5+BiqPKRY/pfHLk Z/T1x1pbbmeGQyyLx/Fc7UGzrNDGbagUEOwm++sZfsjTE= Received: from [10.42.20.80] (unknown []) by gzga-smtp-mtada-g0-2 (Coremail) with SMTP id _____wAHHCs+m2toxqqODA--.34329S2; Mon, 07 Jul 2025 18:02:39 +0800 (CST) Message-ID: <1567a84b-3f78-4f0a-9549-bfe9fd4aa96b@163.com> Date: Mon, 7 Jul 2025 18:02:38 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped To: Thomas Zimmermann , Helge Deller Cc: Peter Jones , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Shixiong Ou References: <20250626094937.515552-1-oushixiong1025@163.com> <3b3feb03-c417-4569-b7b0-44565d7cce4f@suse.de> <1687bb52-e724-46a8-af75-26b486634c20@suse.de> From: Shixiong Ou In-Reply-To: <1687bb52-e724-46a8-af75-26b486634c20@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID: _____wAHHCs+m2toxqqODA--.34329S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxKrWxGr43XF4kAFW8JF15CFg_yoWxWFyxpF WfGFW3CF48Xrn7Grs0g3Wqyrn3tr4kWFyq9FsxKw1UJFyqyF1avrnruryDurykZr4kGF1I qr4jy34akF15CFJanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07Ua0PhUUUUU= X-Originating-IP: [116.128.244.169] X-CM-SenderInfo: xrxvxxx0lr0wirqskqqrwthudrp/xtbBYwCDD2hrkI3wHAAAsd 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 在 2025/7/7 17:28, Thomas Zimmermann 写道: > Hi > > Am 07.07.25 um 11:24 schrieb Shixiong Ou: >> >> 在 2025/6/27 17:13, Thomas Zimmermann 写道: >>> Hi >>> >>> Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com: >>>> From: Shixiong Ou >>>> >>>> [WHY] >>>> On an ARM machine, the following log is present: >>>> [    0.900884] efifb: framebuffer at 0x1020000000, using 3072k, >>>> total 3072k >>>> [    2.297884] amdgpu 0000:04:00.0: >>>> remove_conflicting_pci_framebuffers: bar 0: 0x1000000000 -> >>>> 0x100fffffff >>>> [    2.297886] amdgpu 0000:04:00.0: >>>> remove_conflicting_pci_framebuffers: bar 2: 0x1010000000 -> >>>> 0x10101fffff >>>> [    2.297888] amdgpu 0000:04:00.0: >>>> remove_conflicting_pci_framebuffers: bar 5: 0x58200000 -> 0x5823ffff >>>> >>>> It show that the efifb framebuffer base is out of PCI BAR, and this >>>> results in both efi-framebuffer and amdgpudrmfb co-existing. >>>> >>>> The fbcon will be bound to efi-framebuffer by default and cannot be >>>> used. >>>> >>>> [HOW] >>>> Do not load efifb driver if PCI BAR has changed but not fixuped. >>>> In the following cases: >>>>     1. screen_info_lfb_pdev is NULL. >>>>     2. __screen_info_relocation_is_valid return false. >>> >>> Apart from ruling out invalid screen_info, did you figure out why >>> the relocation tracking didn't work? It would be good to fix this if >>> possible. >>> >>> Best regards >>> Thomas >>> >> I haven’t figure out the root cause yet. >> >> This issue is quite rare and might be related to the EFI firmware. >> However, I wonder if we could add some handling when no PCI resources >> are found in screen_info_fixup_lfb(), as a temporary workaround for >> the problem I mentioned earlier. > > As I said elsewhere in the thread, you can clear the screen_info's > video type in the branch at [1] to disable it entirely. We should have > probably done this anyway. Knowing the cause of the issue would still > be nice though. > > Best regards > Thomas > > [1] > https://elixir.bootlin.com/linux/v6.15.5/source/drivers/video/screen_info_pci.c#L44 > > thanks for you suggestion, while there are two cases:     1. screen_info_lfb_pdev is NULL.     2. __screen_info_relocation_is_valid return false. should we do it at [1] too? Best regards Shixiong Ou [1] https://elixir.bootlin.com/linux/v6.15.5/source/drivers/video/screen_info_pci.c#L47 >> >> Best regards >> Shixiong Ou >> >>>> >>>> Signed-off-by: Shixiong Ou >>>> --- >>>>   drivers/video/fbdev/efifb.c     |  4 ++++ >>>>   drivers/video/screen_info_pci.c | 24 ++++++++++++++++++++++++ >>>>   include/linux/screen_info.h     |  5 +++++ >>>>   3 files changed, 33 insertions(+) >>>> >>>> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c >>>> index 0e1bd3dba255..de8d016c9a66 100644 >>>> --- a/drivers/video/fbdev/efifb.c >>>> +++ b/drivers/video/fbdev/efifb.c >>>> @@ -303,6 +303,10 @@ static void efifb_setup(struct screen_info >>>> *si, char *options) >>>>     static inline bool fb_base_is_valid(struct screen_info *si) >>>>   { >>>> +    /* check whether fb_base has changed but not fixuped */ >>>> +    if (!screen_info_is_useful()) >>>> +        return false; >>>> + >>>>       if (si->lfb_base) >>>>           return true; >>>>   diff --git a/drivers/video/screen_info_pci.c >>>> b/drivers/video/screen_info_pci.c >>>> index 66bfc1d0a6dc..ac57dcaf0cac 100644 >>>> --- a/drivers/video/screen_info_pci.c >>>> +++ b/drivers/video/screen_info_pci.c >>>> @@ -9,6 +9,8 @@ static struct pci_dev *screen_info_lfb_pdev; >>>>   static size_t screen_info_lfb_bar; >>>>   static resource_size_t screen_info_lfb_res_start; // original >>>> start of resource >>>>   static resource_size_t screen_info_lfb_offset; // framebuffer >>>> offset within resource >>>> +static bool screen_info_changed; >>>> +static bool screen_info_fixuped; >>>>     static bool __screen_info_relocation_is_valid(const struct >>>> screen_info *si, struct resource *pr) >>>>   { >>>> @@ -24,6 +26,24 @@ static bool >>>> __screen_info_relocation_is_valid(const struct screen_info *si, stru >>>>       return true; >>>>   } >>>>   +bool screen_info_is_useful(void) >>>> +{ >>>> +    unsigned int type; >>>> +    const struct screen_info *si = &screen_info; >>>> + >>>> +    type = screen_info_video_type(si); >>>> +    if (type != VIDEO_TYPE_EFI) >>>> +        return true; >>>> + >>>> +    if (screen_info_changed && !screen_info_fixuped) { >>>> +        pr_warn("The screen_info has changed but not fixuped"); >>>> +        return false; >>>> +    } >>>> + >>>> +    pr_info("The screen_info is useful"); >>>> +    return true; >>>> +} >>>> + >>>>   void screen_info_apply_fixups(void) >>>>   { >>>>       struct screen_info *si = &screen_info; >>>> @@ -32,18 +52,22 @@ void screen_info_apply_fixups(void) >>>>           struct resource *pr = >>>> &screen_info_lfb_pdev->resource[screen_info_lfb_bar]; >>>>             if (pr->start != screen_info_lfb_res_start) { >>>> +            screen_info_changed = true; >>>>               if (__screen_info_relocation_is_valid(si, pr)) { >>>>                   /* >>>>                    * Only update base if we have an actual >>>>                    * relocation to a valid I/O range. >>>>                    */ >>>>                   __screen_info_set_lfb_base(si, pr->start + >>>> screen_info_lfb_offset); >>>> +                screen_info_fixuped = true; >>>>                   pr_info("Relocating firmware framebuffer to >>>> offset %pa[d] within %pr\n", >>>>                       &screen_info_lfb_offset, pr); >>>>               } else { >>>>                   pr_warn("Invalid relocating, disabling firmware >>>> framebuffer\n"); >>>>               } >>>>           } >>>> +    } else { >>>> +        screen_info_changed = true; >>>>       } >>>>   } >>>>   diff --git a/include/linux/screen_info.h >>>> b/include/linux/screen_info.h >>>> index 923d68e07679..632cdbb1adbe 100644 >>>> --- a/include/linux/screen_info.h >>>> +++ b/include/linux/screen_info.h >>>> @@ -138,9 +138,14 @@ ssize_t screen_info_resources(const struct >>>> screen_info *si, struct resource *r, >>>>   u32 __screen_info_lfb_bits_per_pixel(const struct screen_info *si); >>>>     #if defined(CONFIG_PCI) >>>> +bool screen_info_is_useful(void); >>>>   void screen_info_apply_fixups(void); >>>>   struct pci_dev *screen_info_pci_dev(const struct screen_info *si); >>>>   #else >>>> +bool screen_info_is_useful(void) >>>> +{ >>>> +    return true; >>>> +} >>>>   static inline void screen_info_apply_fixups(void) >>>>   { } >>>>   static inline struct pci_dev *screen_info_pci_dev(const struct >>>> screen_info *si) >>> >> >