* [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
@ 2025-06-26 9:49 oushixiong1025
2025-06-26 10:31 ` Thomas Zimmermann
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: oushixiong1025 @ 2025-06-26 9:49 UTC (permalink / raw)
To: Helge Deller
Cc: Peter Jones, Thomas Zimmermann, linux-fbdev, dri-devel,
linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
[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.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
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)
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
@ 2025-06-26 10:31 ` Thomas Zimmermann
2025-06-27 8:07 ` Shixiong Ou
2025-06-26 21:17 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Thomas Zimmermann @ 2025-06-26 10:31 UTC (permalink / raw)
To: oushixiong1025, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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
The patch at
https://patchwork.freedesktop.org/series/148057/
is supposed to fix the problem. It has been merged with v6.16-rc1 as
commit 2f29b5c23101 ("video: screen_info: Relocate framebuffers behind
PCI bridges"). It is in your tree?
Best regards
Thomas
> 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.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> 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)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
2025-06-26 10:31 ` Thomas Zimmermann
@ 2025-06-26 21:17 ` kernel test robot
2025-06-26 21:27 ` kernel test robot
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-06-26 21:17 UTC (permalink / raw)
To: oushixiong1025, Helge Deller
Cc: oe-kbuild-all, Peter Jones, Thomas Zimmermann, linux-fbdev,
dri-devel, linux-kernel, Shixiong Ou
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.16-rc3 next-20250626]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/oushixiong1025-163-com/fbdev-efifb-do-not-load-efifb-if-PCI-BAR-has-changed-but-not-fixuped/20250626-175111
base: linus/master
patch link: https://lore.kernel.org/r/20250626094937.515552-1-oushixiong1025%40163.com
patch subject: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
config: arm64-allnoconfig (https://download.01.org/0day-ci/archive/20250627/202506270544.iEP67RCm-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250627/202506270544.iEP67RCm-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506270544.iEP67RCm-lkp@intel.com/
All errors (new ones prefixed by >>):
aarch64-linux-ld: drivers/video/console/dummycon.o: in function `screen_info_is_useful':
>> dummycon.c:(.text+0x90): multiple definition of `screen_info_is_useful'; arch/arm64/kernel/setup.o:setup.c:(.text+0x0): first defined here
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
2025-06-26 10:31 ` Thomas Zimmermann
2025-06-26 21:17 ` kernel test robot
@ 2025-06-26 21:27 ` kernel test robot
2025-06-27 9:10 ` Thomas Zimmermann
2025-06-27 9:13 ` Thomas Zimmermann
4 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-06-26 21:27 UTC (permalink / raw)
To: oushixiong1025, Helge Deller
Cc: llvm, oe-kbuild-all, Peter Jones, Thomas Zimmermann, linux-fbdev,
dri-devel, linux-kernel, Shixiong Ou
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.16-rc3 next-20250626]
[cannot apply to drm-misc/drm-misc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/oushixiong1025-163-com/fbdev-efifb-do-not-load-efifb-if-PCI-BAR-has-changed-but-not-fixuped/20250626-175111
base: linus/master
patch link: https://lore.kernel.org/r/20250626094937.515552-1-oushixiong1025%40163.com
patch subject: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20250627/202506270558.awnEnyeN-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project e04c938cc08a90ae60440ce22d072ebc69d67ee8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250627/202506270558.awnEnyeN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506270558.awnEnyeN-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from init/main.c:110:
In file included from arch/arm/include/asm/setup.h:14:
>> include/linux/screen_info.h:145:6: warning: no previous prototype for function 'screen_info_is_useful' [-Wmissing-prototypes]
145 | bool screen_info_is_useful(void)
| ^
include/linux/screen_info.h:145:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
145 | bool screen_info_is_useful(void)
| ^
| static
1 warning generated.
vim +/screen_info_is_useful +145 include/linux/screen_info.h
139
140 #if defined(CONFIG_PCI)
141 bool screen_info_is_useful(void);
142 void screen_info_apply_fixups(void);
143 struct pci_dev *screen_info_pci_dev(const struct screen_info *si);
144 #else
> 145 bool screen_info_is_useful(void)
146 {
147 return true;
148 }
149 static inline void screen_info_apply_fixups(void)
150 { }
151 static inline struct pci_dev *screen_info_pci_dev(const struct screen_info *si)
152 {
153 return NULL;
154 }
155 #endif
156
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 10:31 ` Thomas Zimmermann
@ 2025-06-27 8:07 ` Shixiong Ou
2025-06-27 8:12 ` Thomas Zimmermann
0 siblings, 1 reply; 13+ messages in thread
From: Shixiong Ou @ 2025-06-27 8:07 UTC (permalink / raw)
To: Thomas Zimmermann, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
在 2025/6/26 18:31, Thomas Zimmermann 写道:
> Hi
>
> Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> [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
>
> The patch at
>
> https://patchwork.freedesktop.org/series/148057/
>
> is supposed to fix the problem. It has been merged with v6.16-rc1 as
> commit 2f29b5c23101 ("video: screen_info: Relocate framebuffers behind
> PCI bridges"). It is in your tree?
>
> Best regards
> Thomas
>
yeah, this patch is in my tree. but do not fix the problem.
this is some message:
kylin@kylin-pc:~$ dmesg | grep BAR
[ 0.688192] pci 0000:00:03.0: BAR 15: assigned [mem
0x1000000000-0x101fffffff 64bit pref]
[ 0.688200] pci 0000:00:00.0: BAR 0: assigned [mem
0x1020000000-0x10200fffff 64bit pref]
[ 0.688205] pci 0000:00:00.0: BAR 14: assigned [mem
0x58000000-0x580fffff]
[ 0.688210] pci 0000:00:01.0: BAR 0: assigned [mem
0x1020100000-0x10201fffff 64bit pref]
[ 0.688215] pci 0000:00:02.0: BAR 0: assigned [mem
0x1020200000-0x10202fffff 64bit pref]
[ 0.688221] pci 0000:00:02.0: BAR 14: assigned [mem
0x58100000-0x581fffff]
[ 0.688225] pci 0000:00:03.0: BAR 0: assigned [mem
0x1020300000-0x10203fffff 64bit pref]
[ 0.688231] pci 0000:00:03.0: BAR 14: assigned [mem
0x58200000-0x585fffff]
[ 0.688237] pci 0000:00:04.0: BAR 0: assigned [mem
0x1020400000-0x10204fffff 64bit pref]
[ 0.688243] pci 0000:00:05.0: BAR 0: assigned [mem
0x1020500000-0x10205fffff 64bit pref]
[ 0.688249] pci 0000:00:05.0: BAR 14: assigned [mem
0x58600000-0x586fffff]
[ 0.688253] pci 0000:01:00.0: BAR 0: assigned [mem
0x58000000-0x58003fff 64bit]
[ 0.688290] pci 0000:03:00.0: BAR 6: assigned [mem
0x58100000-0x5817ffff pref]
[ 0.688296] pci 0000:03:00.0: BAR 0: assigned [mem 0x58180000-0x58181fff]
[ 0.688303] pci 0000:03:00.0: BAR 5: assigned [mem 0x58182000-0x58183fff]
[ 0.688317] pci 0000:04:00.0: BAR 1: assigned [mem
0x1000000000-0x101fffffff 64bit pref]
[ 0.688326] pci 0000:04:00.0: BAR 0: assigned [mem 0x58200000-0x583fffff]
[ 0.688332] pci 0000:04:00.0: BAR 6: assigned [mem
0x58400000-0x584fffff pref]
[ 0.688336] pci 0000:04:00.1: BAR 0: assigned [mem 0x58500000-0x58503fff]
[ 0.688360] pci 0000:06:00.0: BAR 0: assigned [mem
0x58600000-0x58601fff 64bit]
kylin@kylin-pc:~$ dmesg | grep framebuffer
[ 1.137536] efifb: framebuffer at 0x1020000000, using 3072k, total 3072k
the efifb base address is still at 0x1020000000 after calling
pcibios_bus_to_resource().
>> 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.
>>
>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>> ---
>> 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");
And should something be done after __screen_info_relocation_is_valid()
return false?
Best regards
Shixiong.
>> }
>> }
>> + } 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)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-27 8:07 ` Shixiong Ou
@ 2025-06-27 8:12 ` Thomas Zimmermann
[not found] ` <bbfebeac-a5b4-4350-a4e8-3da8a5f0efad@163.com>
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Zimmermann @ 2025-06-27 8:12 UTC (permalink / raw)
To: Shixiong Ou, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 27.06.25 um 10:07 schrieb Shixiong Ou:
>
> 在 2025/6/26 18:31, Thomas Zimmermann 写道:
>> Hi
>>
>> Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
>>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>>
>>> [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
>>
>> The patch at
>>
>> https://patchwork.freedesktop.org/series/148057/
>>
>> is supposed to fix the problem. It has been merged with v6.16-rc1 as
>> commit 2f29b5c23101 ("video: screen_info: Relocate framebuffers
>> behind PCI bridges"). It is in your tree?
>>
>> Best regards
>> Thomas
>>
> yeah, this patch is in my tree. but do not fix the problem.
The patch's final revision had a rough development. Just for testing
purposes, could you revert the commit and instead apply the patch's
earlier revision from
https://patchwork.freedesktop.org/patch/649527/?series=148057&rev=1
?
Thanks a lot.
Best regards
Thomas
>
> this is some message:
>
> kylin@kylin-pc:~$ dmesg | grep BAR
> [ 0.688192] pci 0000:00:03.0: BAR 15: assigned [mem
> 0x1000000000-0x101fffffff 64bit pref]
> [ 0.688200] pci 0000:00:00.0: BAR 0: assigned [mem
> 0x1020000000-0x10200fffff 64bit pref]
> [ 0.688205] pci 0000:00:00.0: BAR 14: assigned [mem
> 0x58000000-0x580fffff]
> [ 0.688210] pci 0000:00:01.0: BAR 0: assigned [mem
> 0x1020100000-0x10201fffff 64bit pref]
> [ 0.688215] pci 0000:00:02.0: BAR 0: assigned [mem
> 0x1020200000-0x10202fffff 64bit pref]
> [ 0.688221] pci 0000:00:02.0: BAR 14: assigned [mem
> 0x58100000-0x581fffff]
> [ 0.688225] pci 0000:00:03.0: BAR 0: assigned [mem
> 0x1020300000-0x10203fffff 64bit pref]
> [ 0.688231] pci 0000:00:03.0: BAR 14: assigned [mem
> 0x58200000-0x585fffff]
> [ 0.688237] pci 0000:00:04.0: BAR 0: assigned [mem
> 0x1020400000-0x10204fffff 64bit pref]
> [ 0.688243] pci 0000:00:05.0: BAR 0: assigned [mem
> 0x1020500000-0x10205fffff 64bit pref]
> [ 0.688249] pci 0000:00:05.0: BAR 14: assigned [mem
> 0x58600000-0x586fffff]
> [ 0.688253] pci 0000:01:00.0: BAR 0: assigned [mem
> 0x58000000-0x58003fff 64bit]
> [ 0.688290] pci 0000:03:00.0: BAR 6: assigned [mem
> 0x58100000-0x5817ffff pref]
> [ 0.688296] pci 0000:03:00.0: BAR 0: assigned [mem
> 0x58180000-0x58181fff]
> [ 0.688303] pci 0000:03:00.0: BAR 5: assigned [mem
> 0x58182000-0x58183fff]
> [ 0.688317] pci 0000:04:00.0: BAR 1: assigned [mem
> 0x1000000000-0x101fffffff 64bit pref]
> [ 0.688326] pci 0000:04:00.0: BAR 0: assigned [mem
> 0x58200000-0x583fffff]
> [ 0.688332] pci 0000:04:00.0: BAR 6: assigned [mem
> 0x58400000-0x584fffff pref]
> [ 0.688336] pci 0000:04:00.1: BAR 0: assigned [mem
> 0x58500000-0x58503fff]
> [ 0.688360] pci 0000:06:00.0: BAR 0: assigned [mem
> 0x58600000-0x58601fff 64bit]
> kylin@kylin-pc:~$ dmesg | grep framebuffer
> [ 1.137536] efifb: framebuffer at 0x1020000000, using 3072k, total
> 3072k
>
> the efifb base address is still at 0x1020000000 after calling
> pcibios_bus_to_resource().
>
>
>>> 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.
>>>
>>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>>> ---
>>> 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");
>
> And should something be done after __screen_info_relocation_is_valid()
> return false?
>
> Best regards
> Shixiong.
>
>>> }
>>> }
>>> + } 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)
>>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
[not found] ` <bbfebeac-a5b4-4350-a4e8-3da8a5f0efad@163.com>
@ 2025-06-27 9:04 ` Thomas Zimmermann
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Zimmermann @ 2025-06-27 9:04 UTC (permalink / raw)
To: Shixiong Ou, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 27.06.25 um 10:48 schrieb Shixiong Ou:
>
>
> 在 2025/6/27 16:12, Thomas Zimmermann 写道:
>> Hi
>>
>> Am 27.06.25 um 10:07 schrieb Shixiong Ou:
>>>
>>> 在 2025/6/26 18:31, Thomas Zimmermann 写道:
>>>> Hi
>>>>
>>>> Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
>>>>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>>>>
>>>>> [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
>>>>
>>>> The patch at
>>>>
>>>> https://patchwork.freedesktop.org/series/148057/
>>>>
>>>> is supposed to fix the problem. It has been merged with v6.16-rc1
>>>> as commit 2f29b5c23101 ("video: screen_info: Relocate framebuffers
>>>> behind PCI bridges"). It is in your tree?
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>> yeah, this patch is in my tree. but do not fix the problem.
>>
>> The patch's final revision had a rough development. Just for testing
>> purposes, could you revert the commit and instead apply the patch's
>> earlier revision from
>>
>> https://patchwork.freedesktop.org/patch/649527/?series=148057&rev=1
>>
>> ?
>>
>> Thanks a lot.
>>
>> Best regards
>> Thomas
>>
> I have revert the commit and applied this patch, and added some prints as:
>
> + printk("pcibios_bus_to_resource orginal: start = %llx,
> end = %llx",
> + r->start, r->end);
> + pcibios_bus_to_resource(pdev->bus, r, &bus_region);
> + printk("pcibios_bus_to_resource finished: start =
> %llx, end = %llx",
> + r->start, r->end);
> +
>
> and the kernel message as follow:
>
> kylin@kylin-pc:~$ dmesg | grep pcibios_bus_to_resource
> [ 0.684698] pcibios_bus_to_resource orginal: start = 1020000000,
> end = 10202fffff
> [ 0.684702] pcibios_bus_to_resource finished: start = 1020000000,
> end = 10202fffff
>
> The address doesn't seem to have been modified.
Thanks for confirming.
> Best regards
> Shixiong.
>
>>>
>>> this is some message:
>>>
>>> kylin@kylin-pc:~$ dmesg | grep BAR
>>> [ 0.688192] pci 0000:00:03.0: BAR 15: assigned [mem
>>> 0x1000000000-0x101fffffff 64bit pref]
>>> [ 0.688200] pci 0000:00:00.0: BAR 0: assigned [mem
>>> 0x1020000000-0x10200fffff 64bit pref]
>>> [ 0.688205] pci 0000:00:00.0: BAR 14: assigned [mem
>>> 0x58000000-0x580fffff]
>>> [ 0.688210] pci 0000:00:01.0: BAR 0: assigned [mem
>>> 0x1020100000-0x10201fffff 64bit pref]
>>> [ 0.688215] pci 0000:00:02.0: BAR 0: assigned [mem
>>> 0x1020200000-0x10202fffff 64bit pref]
>>> [ 0.688221] pci 0000:00:02.0: BAR 14: assigned [mem
>>> 0x58100000-0x581fffff]
>>> [ 0.688225] pci 0000:00:03.0: BAR 0: assigned [mem
>>> 0x1020300000-0x10203fffff 64bit pref]
>>> [ 0.688231] pci 0000:00:03.0: BAR 14: assigned [mem
>>> 0x58200000-0x585fffff]
>>> [ 0.688237] pci 0000:00:04.0: BAR 0: assigned [mem
>>> 0x1020400000-0x10204fffff 64bit pref]
>>> [ 0.688243] pci 0000:00:05.0: BAR 0: assigned [mem
>>> 0x1020500000-0x10205fffff 64bit pref]
>>> [ 0.688249] pci 0000:00:05.0: BAR 14: assigned [mem
>>> 0x58600000-0x586fffff]
>>> [ 0.688253] pci 0000:01:00.0: BAR 0: assigned [mem
>>> 0x58000000-0x58003fff 64bit]
>>> [ 0.688290] pci 0000:03:00.0: BAR 6: assigned [mem
>>> 0x58100000-0x5817ffff pref]
>>> [ 0.688296] pci 0000:03:00.0: BAR 0: assigned [mem
>>> 0x58180000-0x58181fff]
>>> [ 0.688303] pci 0000:03:00.0: BAR 5: assigned [mem
>>> 0x58182000-0x58183fff]
>>> [ 0.688317] pci 0000:04:00.0: BAR 1: assigned [mem
>>> 0x1000000000-0x101fffffff 64bit pref]
>>> [ 0.688326] pci 0000:04:00.0: BAR 0: assigned [mem
>>> 0x58200000-0x583fffff]
>>> [ 0.688332] pci 0000:04:00.0: BAR 6: assigned [mem
>>> 0x58400000-0x584fffff pref]
>>> [ 0.688336] pci 0000:04:00.1: BAR 0: assigned [mem
>>> 0x58500000-0x58503fff]
>>> [ 0.688360] pci 0000:06:00.0: BAR 0: assigned [mem
>>> 0x58600000-0x58601fff 64bit]
>>> kylin@kylin-pc:~$ dmesg | grep framebuffer
>>> [ 1.137536] efifb: framebuffer at 0x1020000000, using 3072k,
>>> total 3072k
>>>
>>> the efifb base address is still at 0x1020000000 after calling
>>> pcibios_bus_to_resource().
>>>
>>>
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>>>>> ---
>>>>> 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");
>>>
>>> And should something be done
>>> after __screen_info_relocation_is_valid() return false?
>>>
>>> Best regards
>>> Shixiong.
>>>
>>>>> }
>>>>> }
>>>>> + } 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)
>>>>
>>>
>>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
` (2 preceding siblings ...)
2025-06-26 21:27 ` kernel test robot
@ 2025-06-27 9:10 ` Thomas Zimmermann
2025-06-27 9:13 ` Thomas Zimmermann
4 siblings, 0 replies; 13+ messages in thread
From: Thomas Zimmermann @ 2025-06-27 9:10 UTC (permalink / raw)
To: oushixiong1025, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> 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");
Here it says to disable the framebuffer, but does not actually disable
anything. Instead of adding new interfaces, simply do
screen_info.orig_video_isVGA = 0;
in this branch. Further kernel code will then ignore the framebuffer.
Also works with VIDEO_TYPE_VLFB.
Best regards
Thomas
> }
> }
> + } 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)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
` (3 preceding siblings ...)
2025-06-27 9:10 ` Thomas Zimmermann
@ 2025-06-27 9:13 ` Thomas Zimmermann
2025-07-07 9:24 ` Shixiong Ou
4 siblings, 1 reply; 13+ messages in thread
From: Thomas Zimmermann @ 2025-06-27 9:13 UTC (permalink / raw)
To: oushixiong1025, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> 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)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-06-27 9:13 ` Thomas Zimmermann
@ 2025-07-07 9:24 ` Shixiong Ou
2025-07-07 9:28 ` Thomas Zimmermann
0 siblings, 1 reply; 13+ messages in thread
From: Shixiong Ou @ 2025-07-07 9:24 UTC (permalink / raw)
To: Thomas Zimmermann, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
在 2025/6/27 17:13, Thomas Zimmermann 写道:
> Hi
>
> Am 26.06.25 um 11:49 schrieb oushixiong1025@163.com:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> [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.
Best regards
Shixiong Ou
>>
>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>> ---
>> 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)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-07-07 9:24 ` Shixiong Ou
@ 2025-07-07 9:28 ` Thomas Zimmermann
2025-07-07 10:02 ` Shixiong Ou
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Zimmermann @ 2025-07-07 9:28 UTC (permalink / raw)
To: Shixiong Ou, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
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 <oushixiong@kylinos.cn>
>>>
>>> [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
>
> Best regards
> Shixiong Ou
>
>>>
>>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>>> ---
>>> 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)
>>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-07-07 9:28 ` Thomas Zimmermann
@ 2025-07-07 10:02 ` Shixiong Ou
2025-07-07 10:17 ` Thomas Zimmermann
0 siblings, 1 reply; 13+ messages in thread
From: Shixiong Ou @ 2025-07-07 10:02 UTC (permalink / raw)
To: Thomas Zimmermann, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
在 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 <oushixiong@kylinos.cn>
>>>>
>>>> [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 <oushixiong@kylinos.cn>
>>>> ---
>>>> 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)
>>>
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
2025-07-07 10:02 ` Shixiong Ou
@ 2025-07-07 10:17 ` Thomas Zimmermann
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Zimmermann @ 2025-07-07 10:17 UTC (permalink / raw)
To: Shixiong Ou, Helge Deller
Cc: Peter Jones, linux-fbdev, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 07.07.25 um 12:02 schrieb Shixiong Ou:
>
> 在 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 <oushixiong@kylinos.cn>
>>>>>
>>>>> [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?
No. This being NULL is an entirely valid state.
Best regards
Thomas
>
> 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 <oushixiong@kylinos.cn>
>>>>> ---
>>>>> 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)
>>>>
>>>
>>
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-07-07 10:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-26 9:49 [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped oushixiong1025
2025-06-26 10:31 ` Thomas Zimmermann
2025-06-27 8:07 ` Shixiong Ou
2025-06-27 8:12 ` Thomas Zimmermann
[not found] ` <bbfebeac-a5b4-4350-a4e8-3da8a5f0efad@163.com>
2025-06-27 9:04 ` Thomas Zimmermann
2025-06-26 21:17 ` kernel test robot
2025-06-26 21:27 ` kernel test robot
2025-06-27 9:10 ` Thomas Zimmermann
2025-06-27 9:13 ` Thomas Zimmermann
2025-07-07 9:24 ` Shixiong Ou
2025-07-07 9:28 ` Thomas Zimmermann
2025-07-07 10:02 ` Shixiong Ou
2025-07-07 10:17 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).