* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Luca Weiss @ 2025-06-27 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Javier Martinez Canillas,
Helge Deller, linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <20250627-mysterious-optimistic-bird-acaafb@krzk-bin>
Hi Krzysztof,
On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote:
> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>> Document the interconnects property which is a list of interconnect
>> paths that is used by the framebuffer and therefore needs to be kept
>> alive when the framebuffer is being used.
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> @@ -79,6 +79,9 @@ properties:
>> power-domains:
>> description: List of power domains used by the framebuffer.
>>
>> + interconnects:
>> + description: List of interconnect paths used by the framebuffer.
>> +
>
> maxItems: 1, or this is not a simple FB anymore. Anything which needs
> some sort of resources in unknown way is not simple anymore. You need
> device specific bindings.
The bindings support an arbitrary number of clocks, regulators,
power-domains. Why should I artificially limit the interconnects to only
one?
The driver code also has that support added in this series.
Regards
Luca
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <20250626094937.515552-1-oushixiong1025@163.com>
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
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <20250626094937.515552-1-oushixiong1025@163.com>
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
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <bbfebeac-a5b4-4350-a4e8-3da8a5f0efad@163.com>
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
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <24f53098-710a-43f9-8d1c-d809fb5354eb@163.com>
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
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Krzysztof Kozlowski @ 2025-06-27 8:08 UTC (permalink / raw)
To: Luca Weiss
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Javier Martinez Canillas,
Helge Deller, linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <20250623-simple-drm-fb-icc-v2-1-f69b86cd3d7d@fairphone.com>
On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
> Document the interconnects property which is a list of interconnect
> paths that is used by the framebuffer and therefore needs to be kept
> alive when the framebuffer is being used.
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> @@ -79,6 +79,9 @@ properties:
> power-domains:
> description: List of power domains used by the framebuffer.
>
> + interconnects:
> + description: List of interconnect paths used by the framebuffer.
> +
maxItems: 1, or this is not a simple FB anymore. Anything which needs
some sort of resources in unknown way is not simple anymore. You need
device specific bindings.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <ecf7f260-4c5f-45fc-be8d-0361b00af6a3@suse.de>
在 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
* Re: [PATCH v2 5/5] fbdev/simplefb: Add support for interconnect paths
From: Javier Martinez Canillas @ 2025-06-27 7:56 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20250623-simple-drm-fb-icc-v2-5-f69b86cd3d7d@fairphone.com>
Luca Weiss <luca.weiss@fairphone.com> writes:
> Some devices might require keeping an interconnect path alive so that
> the framebuffer continues working. Add support for that by setting the
> bandwidth requirements appropriately for all provided interconnect
> paths.
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
[...]
> +static void simplefb_detach_icc(void *res)
> +{
> + struct simplefb_par *par = res;
> + int i;
> +
> + for (i = par->icc_count - 1; i >= 0; i--) {
> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
> + icc_put(par->icc_paths[i]);
> + }
> +}
> +
> +static int simplefb_attach_icc(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + int ret, count, i;
> +
> + count = of_count_phandle_with_args(dev->of_node, "interconnects",
> + "#interconnect-cells");
> + if (count < 0)
> + return 0;
> +
> + /* An interconnect path consists of two elements */
> + if (count % 2) {
> + dev_err(dev, "invalid interconnects value\n");
> + return -EINVAL;
> + }
> + par->icc_count = count / 2;
> +
> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
> + sizeof(*par->icc_paths),
> + GFP_KERNEL);
> + if (!par->icc_paths)
> + return -ENOMEM;
> +
> + for (i = 0; i < par->icc_count; i++) {
> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
> + ret = PTR_ERR(par->icc_paths[i]);
> + if (ret == -EPROBE_DEFER)
> + goto err;
> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
> + continue;
> + }
> +
> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
> + if (ret) {
> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
> + continue;
> + }
> + }
> +
> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
> +
> +err:
> + while (i) {
> + --i;
> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
> + icc_put(par->icc_paths[i]);
> + }
> + return ret;
> +}
> +#else
These two functions contain the same logic that you are using in the
simpledrm driver. I wonder if could be made helpers so that the code
isn't duplicated in both drivers.
But in any case it could be a follow-up of your series I think.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 4/5] fbdev/simplefb: Sort headers correctly
From: Javier Martinez Canillas @ 2025-06-27 7:52 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20250623-simple-drm-fb-icc-v2-4-f69b86cd3d7d@fairphone.com>
Luca Weiss <luca.weiss@fairphone.com> writes:
> Make sure the headers are sorted alphabetically to ensure consistent
> code.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 3/5] drm/sysfb: simpledrm: Add support for interconnect paths
From: Javier Martinez Canillas @ 2025-06-27 7:51 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20250623-simple-drm-fb-icc-v2-3-f69b86cd3d7d@fairphone.com>
Luca Weiss <luca.weiss@fairphone.com> writes:
> Some devices might require keeping an interconnect path alive so that
> the framebuffer continues working. Add support for that by setting the
> bandwidth requirements appropriately for all provided interconnect
> paths.
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> drivers/gpu/drm/sysfb/simpledrm.c | 83 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
> index 349219330314e3421a6bb26ad5cf39a679a5cb7a..47d213e20cab1dd1e19528674a95edea00f4bb30 100644
> --- a/drivers/gpu/drm/sysfb/simpledrm.c
> +++ b/drivers/gpu/drm/sysfb/simpledrm.c
> @@ -2,6 +2,7 @@
>
> #include <linux/aperture.h>
> #include <linux/clk.h>
> +#include <linux/interconnect.h>
> #include <linux/minmax.h>
> #include <linux/of_address.h>
> #include <linux/of_clk.h>
> @@ -225,6 +226,10 @@ struct simpledrm_device {
> struct device **pwr_dom_devs;
> struct device_link **pwr_dom_links;
> #endif
Can you add a /* interconnects */ comment here? Similarly how there is one
for clocks, regulators, power domains, etc.
> +#if defined CONFIG_OF && defined CONFIG_INTERCONNECT
> + unsigned int icc_count;
> + struct icc_path **icc_paths;
> +#endif
>
...
> +static int simpledrm_device_attach_icc(struct simpledrm_device *sdev)
> +{
> + struct device *dev = sdev->sysfb.dev.dev;
> + int ret, count, i;
> +
> + count = of_count_phandle_with_args(dev->of_node, "interconnects",
> + "#interconnect-cells");
> + if (count < 0)
> + return 0;
> +
> + /* An interconnect path consists of two elements */
> + if (count % 2) {
> + drm_err(&sdev->sysfb.dev,
> + "invalid interconnects value\n");
> + return -EINVAL;
> + }
> + sdev->icc_count = count / 2;
> +
> + sdev->icc_paths = devm_kcalloc(dev, sdev->icc_count,
> + sizeof(*sdev->icc_paths),
> + GFP_KERNEL);
> + if (!sdev->icc_paths)
> + return -ENOMEM;
> +
> + for (i = 0; i < sdev->icc_count; i++) {
> + sdev->icc_paths[i] = of_icc_get_by_index(dev, i);
> + if (IS_ERR_OR_NULL(sdev->icc_paths[i])) {
> + ret = PTR_ERR(sdev->icc_paths[i]);
> + if (ret == -EPROBE_DEFER)
> + goto err;
> + drm_err(&sdev->sysfb.dev, "failed to get interconnect path %u: %d\n",
> + i, ret);
You could use dev_err_probe() instead that already handles the -EPROBE_DEFER
case and also will get this message in the /sys/kernel/debug/devices_deferred
debugfs entry, as the reason why the probe deferral happened.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 4/5] fbdev/simplefb: Sort headers correctly
From: Thomas Zimmermann @ 2025-06-27 7:43 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
David Airlie, Simona Vetter, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Javier Martinez Canillas, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <20250623-simple-drm-fb-icc-v2-4-f69b86cd3d7d@fairphone.com>
Am 23.06.25 um 08:44 schrieb Luca Weiss:
> Make sure the headers are sorted alphabetically to ensure consistent
> code.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/simplefb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index be95fcddce4c8ca794826b805cd7dad2985bd637..db27d51046af5cc3c46a0bc81ad9d9ed9a0783cc 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -13,18 +13,18 @@
> */
>
> #include <linux/aperture.h>
> +#include <linux/clk.h>
> #include <linux/errno.h>
> #include <linux/fb.h>
> #include <linux/io.h>
> #include <linux/module.h>
> -#include <linux/platform_data/simplefb.h>
> -#include <linux/platform_device.h>
> -#include <linux/clk.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_clk.h>
> #include <linux/of_platform.h>
> #include <linux/parser.h>
> +#include <linux/platform_data/simplefb.h>
> +#include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/regulator/consumer.h>
>
>
--
--
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
* Re: [PATCH v2 2/5] drm/sysfb: simpledrm: Sort headers correctly
From: Thomas Zimmermann @ 2025-06-27 7:41 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
David Airlie, Simona Vetter, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Javier Martinez Canillas, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <20250623-simple-drm-fb-icc-v2-2-f69b86cd3d7d@fairphone.com>
Am 23.06.25 um 08:44 schrieb Luca Weiss:
> Make sure the headers are sorted alphabetically to ensure consistent
> code.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/sysfb/simpledrm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
> index a1c3119330deffc9e122b83941f3697e5b87f277..349219330314e3421a6bb26ad5cf39a679a5cb7a 100644
> --- a/drivers/gpu/drm/sysfb/simpledrm.c
> +++ b/drivers/gpu/drm/sysfb/simpledrm.c
> @@ -2,9 +2,9 @@
>
> #include <linux/aperture.h>
> #include <linux/clk.h>
> -#include <linux/of_clk.h>
> #include <linux/minmax.h>
> #include <linux/of_address.h>
> +#include <linux/of_clk.h>
> #include <linux/platform_data/simplefb.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
>
--
--
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
* Re: [PATCH v2 2/5] drm/sysfb: simpledrm: Sort headers correctly
From: Javier Martinez Canillas @ 2025-06-27 7:41 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20250623-simple-drm-fb-icc-v2-2-f69b86cd3d7d@fairphone.com>
Luca Weiss <luca.weiss@fairphone.com> writes:
> Make sure the headers are sorted alphabetically to ensure consistent
> code.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Javier Martinez Canillas @ 2025-06-27 7:40 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20250623-simple-drm-fb-icc-v2-1-f69b86cd3d7d@fairphone.com>
Luca Weiss <luca.weiss@fairphone.com> writes:
Hello Luca,
> Document the interconnects property which is a list of interconnect
> paths that is used by the framebuffer and therefore needs to be kept
> alive when the framebuffer is being used.
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Andy Shevchenko @ 2025-06-26 21:59 UTC (permalink / raw)
To: Dan Carpenter
Cc: Andy Shevchenko, Abdun Nihaal, andy, gregkh, lorenzo.stoakes,
tzimmermann, riyandhiman14, willy, notro, thomas.petazzoni,
dri-devel, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <0327da98-8a7c-4db8-8bcd-4179b87a9486@suswa.mountain>
Thu, Jun 26, 2025 at 11:11:39PM +0300, Dan Carpenter kirjoitti:
> On Thu, Jun 26, 2025 at 08:50:43PM +0300, Andy Shevchenko wrote:
> > On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
...
> > > release_framebuf:
> > > + fb_deferred_io_cleanup(info);
> > > framebuffer_release(info);
> >
> > While the fix sounds good, there are still problems in the driver in this area:
> >
> > 1) managed resources allocation is mixed up with plain allocations
> > (as you discovery hints);
> >
> > 2) the order in fbtft_framebuffer_release() is asymmetrical to what
> > we have in fbtft_framebuffer_alloc().
> >
> > I would recommend to study this code a bit more and provide the following
> > patches as a result:
> >
> > 1) fixing the order in fbtft_framebuffer_release();
> >
> > 2) moving vmem allocation closer to when it's needed, i.e. just after
> > successful allocation of the info; at the same time move txbuf allocation
> > from managed to unmanaged (drop devm, add respective kfree() calls where
> > it's required);
>
> Symetrical in this sense means that the cleanup in
> fbtft_framebuffer_release() and in fbtft_framebuffer_alloc() are
> similar:
>
> fb_deferred_io_cleanup();
> vfree();
> framebuffer_release();
>
> I feel like number 1 and 2 are sort of opposite approaches to making the
> order symmetrical. #1 is changing fbtft_framebuffer_release() and #2 is
> changing fbtft_framebuffer_alloc(). #2 is the less awkward approach.
>
> > 3) this patch.
> >
> > All three should have the respective Fixes tags and hence may be backported.
>
> Changing the order isn't a bug fix so it wouldn't get a Fixes tag.
> I agree with Andy that the code isn't beautiful. But I think it's
> easier to just fix the bug, and do the cleanup later as an optional
> patch 2/2. I would also have been fine with a larger patch that does
> the cleanup and the bug fix in one patch but I think other people
> won't like that.
Ah, you have a point. Yes, the moving vmem allocation will solve the ordering
issue.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <20250626094937.515552-1-oushixiong1025@163.com>
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
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <20250626094937.515552-1-oushixiong1025@163.com>
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
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Dan Carpenter @ 2025-06-26 20:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Abdun Nihaal, andy, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <aF2Ic8BP0zWS6R19@smile.fi.intel.com>
On Thu, Jun 26, 2025 at 08:50:43PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
> > In the error paths after fb_info structure is successfully allocated,
> > the memory allocated in fb_deferred_io_init() for info->pagerefs is not
> > freed. Fix that by adding the cleanup function on the error path.
>
> Thanks for the report and the fix! My comments below.
>
> ...
>
> > release_framebuf:
> > + fb_deferred_io_cleanup(info);
> > framebuffer_release(info);
>
> While the fix sounds good, there are still problems in the driver in this area:
>
> 1) managed resources allocation is mixed up with plain allocations
> (as you discovery hints);
>
> 2) the order in fbtft_framebuffer_release() is asymmetrical to what
> we have in fbtft_framebuffer_alloc().
>
> I would recommend to study this code a bit more and provide the following
> patches as a result:
>
> 1) fixing the order in fbtft_framebuffer_release();
>
> 2) moving vmem allocation closer to when it's needed, i.e. just after
> successful allocation of the info; at the same time move txbuf allocation
> from managed to unmanaged (drop devm, add respective kfree() calls where
> it's required);
>
Symetrical in this sense means that the cleanup in
fbtft_framebuffer_release() and in fbtft_framebuffer_alloc() are
similar:
fb_deferred_io_cleanup();
vfree();
framebuffer_release();
I feel like number 1 and 2 are sort of opposite approaches to making the
order symmetrical. #1 is changing fbtft_framebuffer_release() and #2 is
changing fbtft_framebuffer_alloc(). #2 is the less awkward approach.
> 3) this patch.
>
> All three should have the respective Fixes tags and hence may be backported.
Changing the order isn't a bug fix so it wouldn't get a Fixes tag.
I agree with Andy that the code isn't beautiful. But I think it's
easier to just fix the bug, and do the cleanup later as an optional
patch 2/2. I would also have been fine with a larger patch that does
the cleanup and the bug fix in one patch but I think other people
won't like that.
Diff below. Except, oops, this doesn't compile. Change the other
goto alloc_fail places to "return NULL;" I guess that means you
get authorship credit if you fix that.
So if you want you could resend your patch and you could send these
changes I've suggested as a patch 2/2 and then I think everyone will
be happy.
regards,
dan carpenter
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index da9c64152a60..abfd7b1157df 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -568,11 +568,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
height = display->height;
}
- vmem_size = display->width * display->height * bpp / 8;
- vmem = vzalloc(vmem_size);
- if (!vmem)
- goto alloc_fail;
-
fbdefio = devm_kzalloc(dev, sizeof(struct fb_deferred_io), GFP_KERNEL);
if (!fbdefio)
goto alloc_fail;
@@ -595,6 +590,11 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (!info)
goto alloc_fail;
+ vmem_size = display->width * display->height * bpp / 8;
+ vmem = vzalloc(vmem_size);
+ if (!vmem)
+ goto release_framebuf;
+
info->screen_buffer = vmem;
info->fbops = &fbtft_ops;
info->fbdefio = fbdefio;
@@ -652,7 +652,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (par->gamma.curves && gamma) {
if (fbtft_gamma_parse_str(par, par->gamma.curves, gamma,
strlen(gamma)))
- goto release_framebuf;
+ goto cleanup_deferred;
}
/* Transmit buffer */
@@ -669,7 +669,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (txbuflen > 0) {
txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
if (!txbuf)
- goto release_framebuf;
+ goto cleanup_deferred;
par->txbuf.buf = txbuf;
par->txbuf.len = txbuflen;
}
@@ -691,12 +691,12 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
return info;
+cleanup_deferred:
+ fb_deferred_io_cleanup(info);
+ vfree(info->screen_buffer);
release_framebuf:
framebuffer_release(info);
-alloc_fail:
- vfree(vmem);
-
return NULL;
}
EXPORT_SYMBOL(fbtft_framebuffer_alloc);
^ permalink raw reply related
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Dan Carpenter @ 2025-06-26 18:02 UTC (permalink / raw)
To: Abdun Nihaal
Cc: andy, gregkh, lorenzo.stoakes, tzimmermann, riyandhiman14, willy,
notro, thomas.petazzoni, dri-devel, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250626172412.18355-1-abdun.nihaal@gmail.com>
On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
> In the error paths after fb_info structure is successfully allocated,
> the memory allocated in fb_deferred_io_init() for info->pagerefs is not
> freed. Fix that by adding the cleanup function on the error path.
>
> Fixes: c296d5f9957c ("staging: fbtft: core support")
> Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
> ---
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Andy Shevchenko @ 2025-06-26 17:50 UTC (permalink / raw)
To: Abdun Nihaal
Cc: andy, gregkh, lorenzo.stoakes, tzimmermann, riyandhiman14, willy,
notro, thomas.petazzoni, dri-devel, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250626172412.18355-1-abdun.nihaal@gmail.com>
On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
> In the error paths after fb_info structure is successfully allocated,
> the memory allocated in fb_deferred_io_init() for info->pagerefs is not
> freed. Fix that by adding the cleanup function on the error path.
Thanks for the report and the fix! My comments below.
...
> release_framebuf:
> + fb_deferred_io_cleanup(info);
> framebuffer_release(info);
While the fix sounds good, there are still problems in the driver in this area:
1) managed resources allocation is mixed up with plain allocations
(as you discovery hints);
2) the order in fbtft_framebuffer_release() is asymmetrical to what
we have in fbtft_framebuffer_alloc().
I would recommend to study this code a bit more and provide the following
patches as a result:
1) fixing the order in fbtft_framebuffer_release();
2) moving vmem allocation closer to when it's needed, i.e. just after
successful allocation of the info; at the same time move txbuf allocation
from managed to unmanaged (drop devm, add respective kfree() calls where
it's required);
3) this patch.
All three should have the respective Fixes tags and hence may be backported.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-26 17:24 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, gregkh, lorenzo.stoakes, tzimmermann, riyandhiman14,
willy, notro, thomas.petazzoni, dri-devel, linux-fbdev,
linux-staging, linux-kernel
In the error paths after fb_info structure is successfully allocated,
the memory allocated in fb_deferred_io_init() for info->pagerefs is not
freed. Fix that by adding the cleanup function on the error path.
Fixes: c296d5f9957c ("staging: fbtft: core support")
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
This patch is compile tested only. Not tested on real hardware.
Bug was found using our prototype static analysis tool.
drivers/staging/fbtft/fbtft-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index da9c64152a60..39bced400065 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -692,6 +692,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
return info;
release_framebuf:
+ fb_deferred_io_cleanup(info);
framebuffer_release(info);
alloc_fail:
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
In-Reply-To: <20250626094937.515552-1-oushixiong1025@163.com>
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
* [PATCH] fbdev: efifb: do not load efifb if PCI BAR has changed but not fixuped
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
* Re: [PATCH 1/2] fbdev: remove fb_notify support
From: Helge Deller @ 2025-06-25 21:01 UTC (permalink / raw)
To: Sam Ravnborg, Arnd Bergmann
Cc: Thomas Zimmermann, Simona Vetter, Arnd Bergmann, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Javier Martinez Canillas,
linux-arm-kernel, linux-kernel, linux-fbdev, dri-devel
In-Reply-To: <20250625152033.GA183878@ravnborg.org>
Hi Arnd,
On 6/25/25 17:20, Sam Ravnborg wrote:
> Hi Arnd.
>
> I remember I stared at this code before, good to see it gone.
> There is a bit more tidiying up you can do.
>
> Also, I suggest to split it in two patches, it itches me to see the
> driver specific part mixed up with the fb_notify removal.
I assume you will send a v2 patch series.
If so, I'm happy to take it then.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH 1/2] fbdev: remove fb_notify support
From: Sam Ravnborg @ 2025-06-25 15:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Thomas Zimmermann, Simona Vetter, Helge Deller, Arnd Bergmann,
Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Javier Martinez Canillas, linux-arm-kernel, linux-kernel,
linux-fbdev, dri-devel
In-Reply-To: <20250625131511.3366522-1-arnd@kernel.org>
Hi Arnd.
I remember I stared at this code before, good to see it gone.
There is a bit more tidiying up you can do.
Also, I suggest to split it in two patches, it itches me to see the
driver specific part mixed up with the fb_notify removal.
Sam
On Wed, Jun 25, 2025 at 03:12:22PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Commit dc2139c0aa32 ("leds: backlight trigger: Replace fb events with a
> dedicated function call") removed the FB_EVENT_BLANK notifier, and now
> the only remaining user of the FB notifier is the metronomefb driver on
> the PXA/AM200EPD board.
>
> This was introduced in commit 922613436ae5 ("[ARM] 5200/1: am200epd: use
> fb notifiers and gpio api"), which converted it from an earlier version,
> but as far as I can tell this can never have worked because the notifier
> is called after the data it passes down is accessed.
>
> Commit 867187821e5e ("fbdev/metronomefb: Use struct fb_info.screen_buffer")
> broke this further, and there are likely other parts of the driver that
> no longer work.
>
> The am200epd board support itself should have also been removed long ago,
> as there are no users and it was never converted to devicetree format.
>
> Mark the board as broken to prevent build failures and remove the now
> unused notifiers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-pxa/Kconfig | 1 +
This is mixing things up a bit. I suggest
splitting the "depends on BROKEN" out in a dedicated patch.
> drivers/video/fbdev/core/Makefile | 1 -
> drivers/video/fbdev/core/fb_notify.c | 54 ----------------------------
> drivers/video/fbdev/core/fbmem.c | 15 --------
> include/linux/fb.h | 21 -----------
> 5 files changed, 1 insertion(+), 91 deletions(-)
> delete mode 100644 drivers/video/fbdev/core/fb_notify.c
>
> diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> index 10e472f4fa43..edefc953e4f9 100644
> --- a/arch/arm/mach-pxa/Kconfig
> +++ b/arch/arm/mach-pxa/Kconfig
> @@ -69,6 +69,7 @@ choice
>
> config GUMSTIX_AM200EPD
> bool "Enable AM200EPD board support"
> + depends on BROKEN
>
> config GUMSTIX_AM300EPD
> bool "Enable AM300EPD board support"
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index d15974759086..ac8036209501 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,5 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_FB_NOTIFY) += fb_notify.o
> obj-$(CONFIG_FB_CORE) += fb.o
> fb-y := fb_info.o \
> fbmem.o fbcmap.o \
> diff --git a/drivers/video/fbdev/core/fb_notify.c b/drivers/video/fbdev/core/fb_notify.c
> deleted file mode 100644
> index 10e3b9a74adc..000000000000
> --- a/drivers/video/fbdev/core/fb_notify.c
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -/*
> - * linux/drivers/video/fb_notify.c
> - *
> - * Copyright (C) 2006 Antonino Daplas <adaplas@pol.net>
> - *
> - * 2001 - Documented with DocBook
> - * - Brad Douglas <brad@neruo.com>
> - *
> - * This file is subject to the terms and conditions of the GNU General Public
> - * License. See the file COPYING in the main directory of this archive
> - * for more details.
> - */
> -#include <linux/fb.h>
> -#include <linux/notifier.h>
> -#include <linux/export.h>
> -
> -static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
> -
> -/**
> - * fb_register_client - register a client notifier
> - * @nb: notifier block to callback on events
> - *
> - * Return: 0 on success, negative error code on failure.
> - */
> -int fb_register_client(struct notifier_block *nb)
> -{
> - return blocking_notifier_chain_register(&fb_notifier_list, nb);
> -}
> -EXPORT_SYMBOL(fb_register_client);
> -
> -/**
> - * fb_unregister_client - unregister a client notifier
> - * @nb: notifier block to callback on events
> - *
> - * Return: 0 on success, negative error code on failure.
> - */
> -int fb_unregister_client(struct notifier_block *nb)
> -{
> - return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
> -}
> -EXPORT_SYMBOL(fb_unregister_client);
> -
> -/**
> - * fb_notifier_call_chain - notify clients of fb_events
> - * @val: value passed to callback
> - * @v: pointer passed to callback
> - *
> - * Return: The return value of the last notifier function
> - */
> -int fb_notifier_call_chain(unsigned long val, void *v)
> -{
> - return blocking_notifier_call_chain(&fb_notifier_list, val, v);
> -}
> -EXPORT_SYMBOL_GPL(fb_notifier_call_chain);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index dfcf5e4d1d4c..82ec7351e7da 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -498,14 +498,6 @@ static int do_register_framebuffer(struct fb_info *fb_info)
> num_registered_fb++;
> registered_fb[i] = fb_info;
>
> -#ifdef CONFIG_GUMSTIX_AM200EPD
> - {
> - struct fb_event event;
Drop the fb_event definition, it is no longer used.
> - event.info = fb_info;
> - fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
Drop the define for FB_EVENT_FB_REGISTERED
> - }
> -#endif
> -
> return fbcon_fb_registered(fb_info);
> }
>
> @@ -544,13 +536,6 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
> fb_destroy_modelist(&fb_info->modelist);
> registered_fb[fb_info->node] = NULL;
> num_registered_fb--;
> -#ifdef CONFIG_GUMSTIX_AM200EPD
> - {
> - struct fb_event event;
> - event.info = fb_info;
> - fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
Drop the define for FB_EVENT_FB_UNREGISTERED
> - }
> -#endif
> fbcon_fb_unregistered(fb_info);
>
> /* this may free fb info */
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 05cc251035da..520ad870b8b2 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -151,27 +151,6 @@ struct fb_blit_caps {
> u32 flags;
> };
>
> -#ifdef CONFIG_FB_NOTIFY
The Kconfig symbol FB_NOTIFY should be dropped as well.
> -extern int fb_register_client(struct notifier_block *nb);
Drop forward for notifier_block, last user in the file is gone.
> -extern int fb_unregister_client(struct notifier_block *nb);
> -extern int fb_notifier_call_chain(unsigned long val, void *v);
> -#else
> -static inline int fb_register_client(struct notifier_block *nb)
> -{
> - return 0;
> -};
> -
> -static inline int fb_unregister_client(struct notifier_block *nb)
> -{
> - return 0;
> -};
> -
> -static inline int fb_notifier_call_chain(unsigned long val, void *v)
> -{
> - return 0;
> -};
> -#endif
> -
> /*
> * Pixmap structure definition
> *
> --
> 2.39.5
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox