Hello,
The patches in this series contain mostly changes suggested by Daniel Vetter
Thomas Zimmermann. They aim to fix existing races between the Generic System
Framebuffer (sysfb) infrastructure and the fbdev and DRM device registration.
For example, it is currently possible for sysfb to register a platform
device after a real DRM driver was registered and requested to remove the
conflicting framebuffers. Or is possible for a simple{fb,drm} to match with
a device previously registered by sysfb, even after a real driver is present.
A symptom of this issue, was worked around with the commit fb561bf9abde
("fbdev: Prevent probing generic drivers if a FB is already registered")
but that's really a hack and should be reverted instead.
This series attempt to fix it more correctly and revert the mentioned hack.
That will also allow to make the num_registered_fb variable not visible to
drivers anymore, since that's internal to fbdev core.
Pach 1 is just a simple cleanup in preparation for later patches.
Patch 2 add a sysfb_disable() helper to allow disabling sysfb and unregister
devices registered by sysfb.
Patch 3 fixes the race that exists between sysfb devices registration and
fbdev framebuffer devices registration, by disabling the sysfb when a DRM
or fbdev driver requests to remove conflicting framebuffers.
Patch 4 is the revert patch that was posted by Daniel before but dropped
from his set and finally patch 5 is the one that makes num_registered_fb
private to fbmem.c, to not allow drivers to use it anymore.
The patches were tested on a rpi4 with the vc4, simpledrm and simplefb
drivers, using different combinations of built-in and as a module.
Best regards,
Javier
Changes in v6:
- Drop sysfb_try_unregister() helper since is no longer needed.
- Move the sysfb_disable() before the remove conflicting framebuffers
loop (Daniel Vetter).
- Drop patch "fbdev: Make sysfb to unregister its own registered devices"
since was no longer needed.
Changes in v5:
- Move the sysfb_disable() call at conflicting framebuffers again to
avoid the need of a DRIVER_FIRMWARE capability flag.
- Add Daniel Vetter's Reviewed-by tag again since reverted to the old
patch that he already reviewed in v2.
- Drop patches that added a DRM_FIRMWARE capability and use them
since the case those prevented could be ignored (Daniel Vetter).
Changes in v4:
- Make sysfb_disable() to also attempt to unregister a device.
- Add patch to make registered_fb[] private.
- Add patches that introduce the DRM_FIRMWARE capability and usage.
Changes in v3:
- Add Thomas Zimmermann's Reviewed-by tag to patch #1.
- Call sysfb_disable() when a DRM dev and a fbdev are registered rather
than when conflicting framebuffers are removed (Thomas Zimmermann).
- Call sysfb_disable() when a fbdev framebuffer is registered rather
than when conflicting framebuffers are removed (Thomas Zimmermann).
- Drop Daniel Vetter's Reviewed-by tag since patch changed a lot.
- Rebase on top of latest drm-misc-next branch.
Changes in v2:
- Rebase on top of latest drm-misc-next and fix conflicts (Daniel Vetter).
- Add kernel-doc comments and include in other_interfaces.rst (Daniel Vetter).
- Explain in the commit message that fbmem has to unregister the device
as fallback if a driver registered the device itself (Daniel Vetter).
- Also explain that fallback in a comment in the code (Daniel Vetter).
- Don't encode in fbmem the assumption that sysfb will always register
platform devices (Daniel Vetter).
- Add a FIXME comment about drivers registering devices (Daniel Vetter).
- Drop RFC prefix since patches were already reviewed by Daniel Vetter.
- Add Daniel Reviewed-by tags to the patches.
Daniel Vetter (2):
Revert "fbdev: Prevent probing generic drivers if a FB is already
registered"
fbdev: Make registered_fb[] private to fbmem.c
Javier Martinez Canillas (3):
firmware: sysfb: Make sysfb_create_simplefb() return a pdev pointer
firmware: sysfb: Add sysfb_disable() helper function
fbdev: Disable sysfb device registration when removing conflicting FBs
.../driver-api/firmware/other_interfaces.rst | 6 ++
drivers/firmware/sysfb.c | 58 ++++++++++++++++---
drivers/firmware/sysfb_simplefb.c | 16 ++---
drivers/video/fbdev/core/fbmem.c | 20 ++++++-
drivers/video/fbdev/efifb.c | 11 ----
drivers/video/fbdev/simplefb.c | 11 ----
include/linux/fb.h | 7 +--
include/linux/sysfb.h | 23 ++++++--
8 files changed, 103 insertions(+), 49 deletions(-)
--
2.36.1
The platform devices registered by sysfb match with firmware-based DRM or
fbdev drivers, that are used to have early graphics using a framebuffer
provided by the system firmware.
DRM or fbdev drivers later are probed and remove all conflicting framebuffers,
leading to these platform devices for generic drivers to be unregistered.
But the current solution has a race, since the sysfb_init() function could
be called after a DRM or fbdev driver is probed and request to unregister
the devices for drivers with conflicting framebuffes.
To prevent this, disable any future sysfb platform device registration by
calling sysfb_disable(), if a driver requests to remove the conflicting
framebuffers.
Suggested-by: Daniel Vetter <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
---
Changes in v6:
- Move the sysfb_disable() before the remove conflicting framebuffers
loop (Daniel Vetter).
Changes in v5:
- Move the sysfb_disable() call at conflicting framebuffers again to
avoid the need of a DRIVER_FIRMWARE capability flag.
- Add Daniel Vetter's Reviewed-by tag again since reverted to the old
patch that he already reviewed in v2.
Changes in v3:
- Call sysfb_disable() when a DRM dev and a fbdev are registered rather
than when conflicting framebuffers are removed (Thomas Zimmermann).
- Call sysfb_disable() when a fbdev framebuffer is registered rather
than when conflicting framebuffers are removed (Thomas Zimmermann).
- Drop Daniel Vetter's Reviewed-by tag since patch changed a lot.
Changes in v2:
- Explain in the commit message that fbmem has to unregister the device
as fallback if a driver registered the device itself (Daniel Vetter).
- Also explain that fallback in a comment in the code (Daniel Vetter).
- Don't encode in fbmem the assumption that sysfb will always register
platform devices (Daniel Vetter).
- Add a FIXME comment about drivers registering devices (Daniel Vetter).
drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 2fda5917c212..e0720fef0ee6 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/major.h>
#include <linux/slab.h>
+#include <linux/sysfb.h>
#include <linux/mm.h>
#include <linux/mman.h>
#include <linux/vt.h>
@@ -1764,6 +1765,17 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
do_free = true;
}
+ /*
+ * If a driver asked to unregister a platform device registered by
+ * sysfb, then can be assumed that this is a driver for a display
+ * that is set up by the system firmware and has a generic driver.
+ *
+ * Drivers for devices that don't have a generic driver will never
+ * ask for this, so let's assume that a real driver for the display
+ * was already probed and prevent sysfb to register devices later.
+ */
+ sysfb_disable();
+
mutex_lock(®istration_lock);
do_remove_conflicting_framebuffers(a, name, primary);
mutex_unlock(®istration_lock);
--
2.36.1
This function just returned 0 on success or an errno code on error, but it
could be useful for sysfb_init() callers to have a pointer to the device.
Signed-off-by: Javier Martinez Canillas <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
---
(no changes since v3)
Changes in v3:
- Add Thomas Zimmermann's Reviewed-by tag to patch #1.
Changes in v2:
- Rebase on top of latest drm-misc-next and fix conflicts (Daniel Vetter).
drivers/firmware/sysfb.c | 4 ++--
drivers/firmware/sysfb_simplefb.c | 16 ++++++++--------
include/linux/sysfb.h | 10 +++++-----
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 2bfbb05f7d89..b032f40a92de 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -46,8 +46,8 @@ static __init int sysfb_init(void)
/* try to create a simple-framebuffer device */
compatible = sysfb_parse_mode(si, &mode);
if (compatible) {
- ret = sysfb_create_simplefb(si, &mode);
- if (!ret)
+ pd = sysfb_create_simplefb(si, &mode);
+ if (!IS_ERR(pd))
return 0;
}
diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index bda8712bfd8c..a353e27f83f5 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -57,8 +57,8 @@ __init bool sysfb_parse_mode(const struct screen_info *si,
return false;
}
-__init int sysfb_create_simplefb(const struct screen_info *si,
- const struct simplefb_platform_data *mode)
+__init struct platform_device *sysfb_create_simplefb(const struct screen_info *si,
+ const struct simplefb_platform_data *mode)
{
struct platform_device *pd;
struct resource res;
@@ -76,7 +76,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
base |= (u64)si->ext_lfb_base << 32;
if (!base || (u64)(resource_size_t)base != base) {
printk(KERN_DEBUG "sysfb: inaccessible VRAM base\n");
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
/*
@@ -93,7 +93,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
length = mode->height * mode->stride;
if (length > size) {
printk(KERN_WARNING "sysfb: VRAM smaller than advertised\n");
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
length = PAGE_ALIGN(length);
@@ -104,11 +104,11 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
res.start = base;
res.end = res.start + length - 1;
if (res.end <= res.start)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
pd = platform_device_alloc("simple-framebuffer", 0);
if (!pd)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
sysfb_apply_efi_quirks(pd);
@@ -124,10 +124,10 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
if (ret)
goto err_put_device;
- return 0;
+ return pd;
err_put_device:
platform_device_put(pd);
- return ret;
+ return ERR_PTR(ret);
}
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index b0dcfa26d07b..708152e9037b 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -72,8 +72,8 @@ static inline void sysfb_apply_efi_quirks(struct platform_device *pd)
bool sysfb_parse_mode(const struct screen_info *si,
struct simplefb_platform_data *mode);
-int sysfb_create_simplefb(const struct screen_info *si,
- const struct simplefb_platform_data *mode);
+struct platform_device *sysfb_create_simplefb(const struct screen_info *si,
+ const struct simplefb_platform_data *mode);
#else /* CONFIG_SYSFB_SIMPLE */
@@ -83,10 +83,10 @@ static inline bool sysfb_parse_mode(const struct screen_info *si,
return false;
}
-static inline int sysfb_create_simplefb(const struct screen_info *si,
- const struct simplefb_platform_data *mode)
+static inline struct platform_device *sysfb_create_simplefb(const struct screen_info *si,
+ const struct simplefb_platform_data *mode)
{
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
#endif /* CONFIG_SYSFB_SIMPLE */
--
2.36.1
From: Daniel Vetter <[email protected]>
Well except when the olpc dcon fbdev driver is enabled, that thing
digs around in there in rather unfixable ways.
Cc oldc_dcon maintainers as fyi.
v2: I typoed the config name (0day)
Cc: kernel test robot <[email protected]>
Cc: Jens Frederich <[email protected]>
Cc: Jon Nettleton <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: Daniel Vetter <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Reviewed-by: Javier Martinez Canillas <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Tetsuo Handa <[email protected]>
Cc: Zhen Lei <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Xiyu Yang <[email protected]>
Cc: [email protected]
Cc: Zheyu Ma <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
(no changes since v1)
drivers/video/fbdev/core/fbmem.c | 8 ++++++--
include/linux/fb.h | 7 +++----
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index e0720fef0ee6..bdb08b665b43 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -50,10 +50,14 @@
static DEFINE_MUTEX(registration_lock);
struct fb_info *registered_fb[FB_MAX] __read_mostly;
-EXPORT_SYMBOL(registered_fb);
-
int num_registered_fb __read_mostly;
+#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
+EXPORT_SYMBOL(registered_fb);
EXPORT_SYMBOL(num_registered_fb);
+#endif
+#define for_each_registered_fb(i) \
+ for (i = 0; i < FB_MAX; i++) \
+ if (!registered_fb[i]) {} else
bool fb_center_logo __read_mostly;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index bbe1e4571899..c563e24b6293 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -632,16 +632,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var,
extern int fb_get_options(const char *name, char **option);
extern int fb_new_modelist(struct fb_info *info);
+#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
extern struct fb_info *registered_fb[FB_MAX];
+
extern int num_registered_fb;
+#endif
extern bool fb_center_logo;
extern int fb_logo_count;
extern struct class *fb_class;
-#define for_each_registered_fb(i) \
- for (i = 0; i < FB_MAX; i++) \
- if (!registered_fb[i]) {} else
-
static inline void lock_fb_info(struct fb_info *info)
{
mutex_lock(&info->lock);
--
2.36.1
From: Daniel Vetter <[email protected]>
This reverts commit fb561bf9abde49f7e00fdbf9ed2ccf2d86cac8ee.
With
commit 27599aacbaefcbf2af7b06b0029459bbf682000d
Author: Thomas Zimmermann <[email protected]>
Date: Tue Jan 25 10:12:18 2022 +0100
fbdev: Hot-unplug firmware fb devices on forced removal
this should be fixed properly and we can remove this somewhat hackish
check here (e.g. this won't catch drm drivers if fbdev emulation isn't
enabled).
Cc: Thomas Zimmermann <[email protected]>
Cc: Zack Rusin <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Zack Rusin <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Ilya Trukhanov <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Reviewed-by: Javier Martinez Canillas <[email protected]>
Cc: Peter Jones <[email protected]>
Cc: [email protected]
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
(no changes since v1)
drivers/video/fbdev/efifb.c | 11 -----------
drivers/video/fbdev/simplefb.c | 11 -----------
2 files changed, 22 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index ea42ba6445b2..edca3703b964 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -351,17 +351,6 @@ static int efifb_probe(struct platform_device *dev)
char *option = NULL;
efi_memory_desc_t md;
- /*
- * Generic drivers must not be registered if a framebuffer exists.
- * If a native driver was probed, the display hardware was already
- * taken and attempting to use the system framebuffer is dangerous.
- */
- if (num_registered_fb > 0) {
- dev_err(&dev->dev,
- "efifb: a framebuffer is already registered\n");
- return -EINVAL;
- }
-
if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
return -ENODEV;
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 94fc9c6d0411..0ef41173325a 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -413,17 +413,6 @@ static int simplefb_probe(struct platform_device *pdev)
struct simplefb_par *par;
struct resource *res, *mem;
- /*
- * Generic drivers must not be registered if a framebuffer exists.
- * If a native driver was probed, the display hardware was already
- * taken and attempting to use the system framebuffer is dangerous.
- */
- if (num_registered_fb > 0) {
- dev_err(&pdev->dev,
- "simplefb: a framebuffer is already registered\n");
- return -EINVAL;
- }
-
if (fb_get_options("simplefb", NULL))
return -ENODEV;
--
2.36.1
Hi Javier
Am 07.06.22 um 20:23 schrieb Javier Martinez Canillas:
> From: Daniel Vetter <[email protected]>
>
> Well except when the olpc dcon fbdev driver is enabled, that thing
> digs around in there in rather unfixable ways.
There is fb_client_register() to set up a 'client' on top of an fbdev.
The client would then get messages about modesetting, blanks, removals,
etc. But you'd probably need an OLPC to convert dcon, and the mechanism
itself is somewhat unloved these days.
Your patch complicates the fbdev code AFAICT. So I'd either drop it or,
even better, build a nicer interface for dcon.
The dcon driver appears to look only at the first entry. Maybe add
fb_info_get_by_index() and fb_info_put() and export those. They would be
trivial wrappers somewhere in fbmem.c:
#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
struct fb_info *fb_info_get_by_index(unsigned int index)
{
return get_fb_info(index);
}
EXPORT_SYMBOL()
void fb_info_put(struct fb_info *fb_info)
{
put_fb_info(fb_info);
}
EXPORT_SYMBOL()
#endif
In dcon itself, using the new interfaces will actually acquire a
reference to keep the display alive. The code at [1] could be replaced.
And a call to fb_info_put() needs to go into dcon_remove(). [2]
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v5.18.2/source/drivers/staging/olpc_dcon/olpc_dcon.c#L605
[2]
https://elixir.bootlin.com/linux/v5.18.2/source/drivers/staging/olpc_dcon/olpc_dcon.c#L688
>
> Cc oldc_dcon maintainers as fyi.
>
> v2: I typoed the config name (0day)
>
> Cc: kernel test robot <[email protected]>
> Cc: Jens Frederich <[email protected]>
> Cc: Jon Nettleton <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Daniel Vetter <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> Reviewed-by: Javier Martinez Canillas <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> Cc: Tetsuo Handa <[email protected]>
> Cc: Zhen Lei <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Xiyu Yang <[email protected]>
> Cc: [email protected]
> Cc: Zheyu Ma <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> (no changes since v1)
>
> drivers/video/fbdev/core/fbmem.c | 8 ++++++--
> include/linux/fb.h | 7 +++----
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index e0720fef0ee6..bdb08b665b43 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -50,10 +50,14 @@
> static DEFINE_MUTEX(registration_lock);
>
> struct fb_info *registered_fb[FB_MAX] __read_mostly;
> -EXPORT_SYMBOL(registered_fb);
> -
> int num_registered_fb __read_mostly;
> +#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
> +EXPORT_SYMBOL(registered_fb);
> EXPORT_SYMBOL(num_registered_fb);
> +#endif
> +#define for_each_registered_fb(i) \
> + for (i = 0; i < FB_MAX; i++) \
> + if (!registered_fb[i]) {} else
>
> bool fb_center_logo __read_mostly;
>
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index bbe1e4571899..c563e24b6293 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -632,16 +632,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var,
> extern int fb_get_options(const char *name, char **option);
> extern int fb_new_modelist(struct fb_info *info);
>
> +#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
> extern struct fb_info *registered_fb[FB_MAX];
> +
> extern int num_registered_fb;
> +#endif
> extern bool fb_center_logo;
> extern int fb_logo_count;
> extern struct class *fb_class;
>
> -#define for_each_registered_fb(i) \
> - for (i = 0; i < FB_MAX; i++) \
> - if (!registered_fb[i]) {} else
> -
> static inline void lock_fb_info(struct fb_info *info)
> {
> mutex_lock(&info->lock);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
Hello Thomas,
On 6/9/22 13:49, Thomas Zimmermann wrote:
> Hi Javier
>
> Am 07.06.22 um 20:23 schrieb Javier Martinez Canillas:
>> From: Daniel Vetter <[email protected]>
>>
>> Well except when the olpc dcon fbdev driver is enabled, that thing
>> digs around in there in rather unfixable ways.
>
> There is fb_client_register() to set up a 'client' on top of an fbdev.
> The client would then get messages about modesetting, blanks, removals,
> etc. But you'd probably need an OLPC to convert dcon, and the mechanism
> itself is somewhat unloved these days.
>
> Your patch complicates the fbdev code AFAICT. So I'd either drop it or,
> even better, build a nicer interface for dcon.
>
> The dcon driver appears to look only at the first entry. Maybe add
> fb_info_get_by_index() and fb_info_put() and export those. They would be
> trivial wrappers somewhere in fbmem.c:
>
> #if IS_ENABLED(CONFIG_FB_OLPC_DCON)
> struct fb_info *fb_info_get_by_index(unsigned int index)
> {
> return get_fb_info(index);
> }
> EXPORT_SYMBOL()
> void fb_info_put(struct fb_info *fb_info)
> {
> put_fb_info(fb_info);
> }
> EXPORT_SYMBOL()
> #endif
>
> In dcon itself, using the new interfaces will actually acquire a
> reference to keep the display alive. The code at [1] could be replaced.
> And a call to fb_info_put() needs to go into dcon_remove(). [2]
>
Thanks for your suggestions, that makes sense to me. I'll drop this
patch from the set and post as a follow-up a different approach as
you suggested.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On 6/7/22 20:23, Javier Martinez Canillas wrote:
> The patches in this series contain mostly changes suggested by Daniel Vetter
> Thomas Zimmermann. They aim to fix existing races between the Generic System
> Framebuffer (sysfb) infrastructure and the fbdev and DRM device registration.
>
> For example, it is currently possible for sysfb to register a platform
> device after a real DRM driver was registered and requested to remove the
> conflicting framebuffers. Or is possible for a simple{fb,drm} to match with
> a device previously registered by sysfb, even after a real driver is present.
>
> A symptom of this issue, was worked around with the commit fb561bf9abde
> ("fbdev: Prevent probing generic drivers if a FB is already registered")
> but that's really a hack and should be reverted instead.
>
> This series attempt to fix it more correctly and revert the mentioned hack.
> That will also allow to make the num_registered_fb variable not visible to
> drivers anymore, since that's internal to fbdev core.
>
Pushed patches 1-4 to drm-misc (drm-misc-next). Thanks!
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Hi Javier.
On Thu, Jun 09, 2022 at 03:09:21PM +0200, Javier Martinez Canillas wrote:
> Hello Thomas,
>
> On 6/9/22 13:49, Thomas Zimmermann wrote:
> > Hi Javier
> >
> > Am 07.06.22 um 20:23 schrieb Javier Martinez Canillas:
> >> From: Daniel Vetter <[email protected]>
> >>
> >> Well except when the olpc dcon fbdev driver is enabled, that thing
> >> digs around in there in rather unfixable ways.
> >
> > There is fb_client_register() to set up a 'client' on top of an fbdev.
> > The client would then get messages about modesetting, blanks, removals,
> > etc. But you'd probably need an OLPC to convert dcon, and the mechanism
> > itself is somewhat unloved these days.
> >
> > Your patch complicates the fbdev code AFAICT. So I'd either drop it or,
> > even better, build a nicer interface for dcon.
> >
> > The dcon driver appears to look only at the first entry. Maybe add
> > fb_info_get_by_index() and fb_info_put() and export those. They would be
> > trivial wrappers somewhere in fbmem.c:
> >
> > #if IS_ENABLED(CONFIG_FB_OLPC_DCON)
> > struct fb_info *fb_info_get_by_index(unsigned int index)
> > {
> > return get_fb_info(index);
> > }
> > EXPORT_SYMBOL()
> > void fb_info_put(struct fb_info *fb_info)
> > {
> > put_fb_info(fb_info);
> > }
> > EXPORT_SYMBOL()
> > #endif
> >
> > In dcon itself, using the new interfaces will actually acquire a
> > reference to keep the display alive. The code at [1] could be replaced.
> > And a call to fb_info_put() needs to go into dcon_remove(). [2]
> >
>
> Thanks for your suggestions, that makes sense to me. I'll drop this
> patch from the set and post as a follow-up a different approach as
> you suggested.
To repeat myself from irc.
olpc_dcon is a staging driver and we should avoid inventing anything in
core code for to make staging drivers works.
Geert suggested EXPORT_SYMPBOL_NS_GPL() that could work and narrow it
down to olpc_dcon.
The better approach is to mark said driver BROKEN and then someone can
fix it it there is anyone who cares.
Last commit to olpc_dcon was in 2019: e40219d5e4b2177bfd4d885e7b64e3b236af40ac
and maybe Jerry Lin cares enough to fix it.
Added Jerry and Greg to the mail.
Sam
Hello Sam,
On 6/9/22 19:23, Sam Ravnborg wrote:
[snip]
>
> To repeat myself from irc.
> olpc_dcon is a staging driver and we should avoid inventing anything in
> core code for to make staging drivers works.
> Geert suggested EXPORT_SYMPBOL_NS_GPL() that could work and narrow it
> down to olpc_dcon.
> The better approach is to mark said driver BROKEN and then someone can
> fix it it there is anyone who cares.
> Last commit to olpc_dcon was in 2019: e40219d5e4b2177bfd4d885e7b64e3b236af40ac
> and maybe Jerry Lin cares enough to fix it.
>
> Added Jerry and Greg to the mail.
>
> Sam
>
That does sound like the best approach indeed. And if the driver is kept
BROKEN for a few releases then it can just remove it from the kernel. If
someone still uses/cares about the driver, they can fix it as you said,
and it could even be ported to DRM if is something that's still useful.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On Tue, 2022-06-07 at 20:23 +0200, Javier Martinez Canillas wrote:
> The platform devices registered by sysfb match with firmware-based DRM or
> fbdev drivers, that are used to have early graphics using a framebuffer
> provided by the system firmware.
>
> DRM or fbdev drivers later are probed and remove all conflicting framebuffers,
> leading to these platform devices for generic drivers to be unregistered.
>
> But the current solution has a race, since the sysfb_init() function could
> be called after a DRM or fbdev driver is probed and request to unregister
> the devices for drivers with conflicting framebuffes.
>
> To prevent this, disable any future sysfb platform device registration by
> calling sysfb_disable(), if a driver requests to remove the conflicting
> framebuffers.
>
> Suggested-by: Daniel Vetter <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> Reviewed-by: Daniel Vetter <[email protected]>
> ---
>
> Changes in v6:
> - Move the sysfb_disable() before the remove conflicting framebuffers
> loop (Daniel Vetter).
>
> Changes in v5:
> - Move the sysfb_disable() call at conflicting framebuffers again to
> avoid the need of a DRIVER_FIRMWARE capability flag.
> - Add Daniel Vetter's Reviewed-by tag again since reverted to the old
> patch that he already reviewed in v2.
>
> Changes in v3:
> - Call sysfb_disable() when a DRM dev and a fbdev are registered rather
> than when conflicting framebuffers are removed (Thomas Zimmermann).
> - Call sysfb_disable() when a fbdev framebuffer is registered rather
> than when conflicting framebuffers are removed (Thomas Zimmermann).
> - Drop Daniel Vetter's Reviewed-by tag since patch changed a lot.
>
> Changes in v2:
> - Explain in the commit message that fbmem has to unregister the device
> as fallback if a driver registered the device itself (Daniel Vetter).
> - Also explain that fallback in a comment in the code (Daniel Vetter).
> - Don't encode in fbmem the assumption that sysfb will always register
> platform devices (Daniel Vetter).
> - Add a FIXME comment about drivers registering devices (Daniel Vetter).
>
> drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 2fda5917c212..e0720fef0ee6 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/major.h>
> #include <linux/slab.h>
> +#include <linux/sysfb.h>
> #include <linux/mm.h>
> #include <linux/mman.h>
> #include <linux/vt.h>
> @@ -1764,6 +1765,17 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
> do_free = true;
> }
>
> + /*
> + * If a driver asked to unregister a platform device registered by
> + * sysfb, then can be assumed that this is a driver for a display
> + * that is set up by the system firmware and has a generic driver.
> + *
> + * Drivers for devices that don't have a generic driver will never
> + * ask for this, so let's assume that a real driver for the display
> + * was already probed and prevent sysfb to register devices later.
> + */
> + sysfb_disable();
> +
> mutex_lock(®istration_lock);
> do_remove_conflicting_framebuffers(a, name, primary);
> mutex_unlock(®istration_lock);
Hi, Javier.
This change broke arm64 with vmwgfx. We get a kernel oops at boot (let me know if
you'd like .config or just have us test something directly for you):
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000008
Mem abort info:
ESR = 0x96000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000001787ee000
[0000000000000008] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 96000004 [#1] SMP
Modules linked in: vmwgfx(+) e1000e(+) nvme ahci(+) xhci_pci drm_ttm_helper ttm
sha256_arm64 sha1_ce nvme_core xhci_pci_renesas aes_neon_bs aes_neon_blk aes>
CPU: 3 PID: 215 Comm: systemd-udevd Tainted: G U 5.18.0-rc5-vmwgfx
#12
Hardware name: VMware, Inc. VBSA/VBSA, BIOS VEFI 12/31/2020
pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : kernfs_find_and_get_ns+0x2c/0x80
lr : sysfs_unmerge_group+0x30/0x80
sp : ffff80000b78b3f0
x29: ffff80000b78b3f0 x28: ffff0000f7970910 x27: 0000000000000002
x26: ffff0000f7970900 x25: ffff80000abca358 x24: ffff80000936cfa0
x23: ffff80000ac8f458 x22: 0000000000000000 x21: ffff800009431938
x20: ffff800009431800 x19: 0000000000000000 x18: 0000000000000000
x17: 42555300302e7265 x16: 66667562656d6172 x15: 4d006d726f667461
x14: 6c703d4d45545359 x13: 595342555300302e x12: 726566667562656d
x11: 00323137323d4d55 x10: 4e51455300726566 x9 : ffff8000085f7400
x8 : ffff0000f7970980 x7 : 0000000000000000 x6 : 0000000077ffffff
x5 : 0000000070000000 x4 : ffff80000b78b548 x3 : ffff8000088b0420
x2 : 0000000000000000 x1 : ffff800009431938 x0 : 0000000000000000
Call trace:
kernfs_find_and_get_ns+0x2c/0x80
sysfs_unmerge_group+0x30/0x80
dpm_sysfs_remove+0x3c/0x17c
device_del+0xb0/0x3a0
platform_device_del.part.0+0x24/0xb0
platform_device_unregister+0x30/0x50
sysfb_disable+0x4c/0x80
remove_conflicting_framebuffers+0x40/0x100
remove_conflicting_pci_framebuffers+0x128/0x240
drm_aperture_remove_conflicting_pci_framebuffers+0xb8/0x170
vmw_probe+0x50/0xd30 [vmwgfx]
local_pci_probe+0x4c/0xc0
pci_device_probe+0x1e8/0x230
really_probe+0x18c/0x3f0
__driver_probe_device+0x124/0x1c0
driver_probe_device+0x44/0x140
__driver_attach+0xe0/0x234
bus_for_each_dev+0x7c/0xe0
driver_attach+0x30/0x40
bus_add_driver+0x158/0x250
driver_register+0x84/0x140
__pci_register_driver+0x50/0x5c
vmw_pci_driver_init+0x44/0x1000 [vmwgfx]
do_one_initcall+0x50/0x250
do_init_module+0x50/0x260
load_module+0x23e4/0x27c0
__do_sys_finit_module+0xac/0x12c
__arm64_sys_finit_module+0x2c/0x40
invoke_syscall+0x78/0x100
el0_svc_common.constprop.0+0x54/0x184
do_el0_svc+0x34/0x9c
el0_svc+0x54/0x1e0
el0t_64_sync_handler+0xa4/0x130
el0t_64_sync+0x1a0/0x1a4
Code: aa0003f3 a9025bf5 aa0103f5 aa0203f6 (f9400400)
---[ end trace 0000000000000000 ]---
Hello Zack,
On 6/16/22 21:29, Zack Rusin wrote:
> On Tue, 2022-06-07 at 20:23 +0200, Javier Martinez Canillas wrote:
>> The platform devices registered by sysfb match with firmware-based DRM or
>> fbdev drivers, that are used to have early graphics using a framebuffer
>> provided by the system firmware.
>>
[snip]
>
> Hi, Javier.
>
> This change broke arm64 with vmwgfx. We get a kernel oops at boot (let me know if
> you'd like .config or just have us test something directly for you):
>
Yes please share your .config and I'll try to reproduce on an arm64 machine.
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000008
> Mem abort info:
> ESR = 0x96000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004
> CM = 0, WnR = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001787ee000
> [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 96000004 [#1] SMP
> Modules linked in: vmwgfx(+) e1000e(+) nvme ahci(+) xhci_pci drm_ttm_helper ttm
> sha256_arm64 sha1_ce nvme_core xhci_pci_renesas aes_neon_bs aes_neon_blk aes>
> CPU: 3 PID: 215 Comm: systemd-udevd Tainted: G U 5.18.0-rc5-vmwgfx
> #12
I'm confused, your kernel version seems to be 5.18.0-rc5 but this patch
is only in drm-misc-next now and will land in 5.20...
Did you backport it? Can you please try to reproduce with latest drm-tip ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On Thu, 2022-06-16 at 21:55 +0200, Javier Martinez Canillas wrote:
> Hello Zack,
>
> On 6/16/22 21:29, Zack Rusin wrote:
> > On Tue, 2022-06-07 at 20:23 +0200, Javier Martinez Canillas wrote:
> > > The platform devices registered by sysfb match with firmware-based DRM or
> > > fbdev drivers, that are used to have early graphics using a framebuffer
> > > provided by the system firmware.
> > >
>
> [snip]
>
> >
> > Hi, Javier.
> >
> > This change broke arm64 with vmwgfx. We get a kernel oops at boot (let me know if
> > you'd like .config or just have us test something directly for you):
> >
>
> Yes please share your .config and I'll try to reproduce on an arm64 machine.
Attached. It might be a little hard to reproduce unless you have an arm64 machine
with a dedicated gpu. You'll need a system that actually transitions from a generic
fb driver (e.g. efifb) to the dedicated one.
> >
> > Unable to handle kernel NULL pointer dereference at virtual address
> > 0000000000000008
> > Mem abort info:
> > ESR = 0x96000004
> > EC = 0x25: DABT (current EL), IL = 32 bits
> > SET = 0, FnV = 0
> > EA = 0, S1PTW = 0
> > FSC = 0x04: level 0 translation fault
> > Data abort info:
> > ISV = 0, ISS = 0x00000004
> > CM = 0, WnR = 0
> > user pgtable: 4k pages, 48-bit VAs, pgdp=00000001787ee000
> > [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> > Internal error: Oops: 96000004 [#1] SMP
> > Modules linked in: vmwgfx(+) e1000e(+) nvme ahci(+) xhci_pci drm_ttm_helper ttm
> > sha256_arm64 sha1_ce nvme_core xhci_pci_renesas aes_neon_bs aes_neon_blk aes>
> > CPU: 3 PID: 215 Comm: systemd-udevd Tainted: G U 5.18.0-rc5-vmwgfx
> > #12
>
> I'm confused, your kernel version seems to be 5.18.0-rc5 but this patch
> is only in drm-misc-next now and will land in 5.20...
>
> Did you backport it? Can you please try to reproduce with latest drm-tip ?
No, this is drm-misc-next as of yesterday. drm-misc-next was still on 5.18.0-rc5
yesterday.
z
On 6/16/22 23:03, Zack Rusin wrote:
> On Thu, 2022-06-16 at 21:55 +0200, Javier Martinez Canillas wrote:
>> Hello Zack,
>>
>> On 6/16/22 21:29, Zack Rusin wrote:
>>> On Tue, 2022-06-07 at 20:23 +0200, Javier Martinez Canillas wrote:
>>>> The platform devices registered by sysfb match with firmware-based DRM or
>>>> fbdev drivers, that are used to have early graphics using a framebuffer
>>>> provided by the system firmware.
>>>>
>>
>> [snip]
>>
>>>
>>> Hi, Javier.
>>>
>>> This change broke arm64 with vmwgfx. We get a kernel oops at boot (let me know if
>>> you'd like .config or just have us test something directly for you):
>>>
>>
>> Yes please share your .config and I'll try to reproduce on an arm64 machine.
>
> Attached. It might be a little hard to reproduce unless you have an arm64 machine
> with a dedicated gpu. You'll need a system that actually transitions from a generic
> fb driver (e.g. efifb) to the dedicated one.
>
Yes, all my testing for this was done with a rpi4 so I should be able to reproduce
that case. I'm confused though because I tested efifb -> vc4, simplefb -> vc4 and
simpledrm -> vc4.
>>>
>>> Unable to handle kernel NULL pointer dereference at virtual address
>>> 0000000000000008
>>> Mem abort info:
>>> ESR = 0x96000004
>>> EC = 0x25: DABT (current EL), IL = 32 bits
>>> SET = 0, FnV = 0
>>> EA = 0, S1PTW = 0
>>> FSC = 0x04: level 0 translation fault
>>> Data abort info:
>>> ISV = 0, ISS = 0x00000004
>>> CM = 0, WnR = 0
>>> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001787ee000
>>> [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
>>> Internal error: Oops: 96000004 [#1] SMP
>>> Modules linked in: vmwgfx(+) e1000e(+) nvme ahci(+) xhci_pci drm_ttm_helper ttm
>>> sha256_arm64 sha1_ce nvme_core xhci_pci_renesas aes_neon_bs aes_neon_blk aes>
>>> CPU: 3 PID: 215 Comm: systemd-udevd Tainted: G U 5.18.0-rc5-vmwgfx
>>> #12
>>
>> I'm confused, your kernel version seems to be 5.18.0-rc5 but this patch
>> is only in drm-misc-next now and will land in 5.20...
>>
>> Did you backport it? Can you please try to reproduce with latest drm-tip ?
>
> No, this is drm-misc-next as of yesterday. drm-misc-next was still on 5.18.0-rc5
> yesterday.
>
Right! I looked at the base for drm-tip but forgot that drm-misc was still on 5.18.
I'll look at this tomorrow but in the meantime, could you please look if the following
commits on top of drm-misc-next help ?
d258d00fb9c7 fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove
1b5853dfab7f fbdev: efifb: Fix a use-after-free due early fb_info cleanup
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On 6/17/22 00:18, Javier Martinez Canillas wrote:
> On 6/16/22 23:03, Zack Rusin wrote:
[snip]
>
> I'll look at this tomorrow but in the meantime, could you please look if the following
> commits on top of drm-misc-next help ?
>
> d258d00fb9c7 fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove
> 1b5853dfab7f fbdev: efifb: Fix a use-after-free due early fb_info cleanup
>
Scratch that. I see in your config now that you are not using efifb but instead
simpledrm: CONFIG_DRM_SIMPLEDRM=y, CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_VMWGFX.
Since you mentioned efifb I misunderstood that you are using it. Anyways, as
said I'll investigate this tomorrow.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On Fri, 2022-06-17 at 01:21 +0200, Javier Martinez Canillas wrote:
> On 6/17/22 00:18, Javier Martinez Canillas wrote:
> > On 6/16/22 23:03, Zack Rusin wrote:
>
> [snip]
>
> >
> > I'll look at this tomorrow but in the meantime, could you please look if the following
> > commits on top of drm-misc-next help ?
> >
> > d258d00fb9c7 fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove
> > 1b5853dfab7f fbdev: efifb: Fix a use-after-free due early fb_info cleanup
> >
>
> Scratch that. I see in your config now that you are not using efifb but instead
> simpledrm: CONFIG_DRM_SIMPLEDRM=y, CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_VMWGFX.
>
> Since you mentioned efifb I misunderstood that you are using it. Anyways, as
> said I'll investigate this tomorrow.
Sounds good. Let me know if you'd like me to try it without SIMPLEFB.
z
Hello Zack,
On 6/17/22 03:35, Zack Rusin wrote:
> On Fri, 2022-06-17 at 01:21 +0200, Javier Martinez Canillas wrote:
>> On 6/17/22 00:18, Javier Martinez Canillas wrote:
>>> On 6/16/22 23:03, Zack Rusin wrote:
>>
>> [snip]
>>
>>>
>>> I'll look at this tomorrow but in the meantime, could you please look if the following
>>> commits on top of drm-misc-next help ?
>>>
>>> d258d00fb9c7 fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove
>>> 1b5853dfab7f fbdev: efifb: Fix a use-after-free due early fb_info cleanup
>>>
>>
>> Scratch that. I see in your config now that you are not using efifb but instead
>> simpledrm: CONFIG_DRM_SIMPLEDRM=y, CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_VMWGFX.
>>
>> Since you mentioned efifb I misunderstood that you are using it. Anyways, as
>> said I'll investigate this tomorrow.
>
> Sounds good. Let me know if you'd like me to try it without SIMPLEFB.
>
Yes, please do. Either with CONFIG_SYSFB_SIMPLEFB disabled and CONFIG_FB_EFI
enabled (so that "efi-framebuffer" is registered and efifb probed) or with
CONFIG_SYSFB_SIMPLEFB but CONFIG_FB_SIMPLE enabled (so "simple-framebuffer
is used too but with simplefb instead of simpledrm).
I'm not able to reproduce, it would be useful to have another data point.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On Fri, 2022-06-17 at 08:46 +0200, Javier Martinez Canillas wrote:
> Hello Zack,
>
> On 6/17/22 03:35, Zack Rusin wrote:
> > On Fri, 2022-06-17 at 01:21 +0200, Javier Martinez Canillas wrote:
> > > On 6/17/22 00:18, Javier Martinez Canillas wrote:
> > > > On 6/16/22 23:03, Zack Rusin wrote:
> > >
> > > [snip]
> > >
> > > >
> > > > I'll look at this tomorrow but in the meantime, could you please look if the following
> > > > commits on top of drm-misc-next help ?
> > > >
> > > > d258d00fb9c7 fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove
> > > > 1b5853dfab7f fbdev: efifb: Fix a use-after-free due early fb_info cleanup
> > > >
> > >
> > > Scratch that. I see in your config now that you are not using efifb but instead
> > > simpledrm: CONFIG_DRM_SIMPLEDRM=y, CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_VMWGFX.
> > >
> > > Since you mentioned efifb I misunderstood that you are using it. Anyways, as
> > > said I'll investigate this tomorrow.
> >
> > Sounds good. Let me know if you'd like me to try it without SIMPLEFB.
> >
>
> Yes, please do. Either with CONFIG_SYSFB_SIMPLEFB disabled and CONFIG_FB_EFI
> enabled (so that "efi-framebuffer" is registered and efifb probed) or with
> CONFIG_SYSFB_SIMPLEFB but CONFIG_FB_SIMPLE enabled (so "simple-framebuffer
> is used too but with simplefb instead of simpledrm).
>
> I'm not able to reproduce, it would be useful to have another data point.
Also happening for me with CONFIG_SYSFB_SIMPLEFB, on a Intel Core i7-
1065G7 (with iGPU).
Reverting this commit on top of 5.19-rc5 "fixes" the issue.
On Mon, 2022-07-04 at 17:36 +0800, Xi Ruoyao wrote:
> > Yes, please do. Either with CONFIG_SYSFB_SIMPLEFB disabled and CONFIG_FB_EFI
> > enabled (so that "efi-framebuffer" is registered and efifb probed) or with
> > CONFIG_SYSFB_SIMPLEFB but CONFIG_FB_SIMPLE enabled (so "simple-framebuffer
> > is used too but with simplefb instead of simpledrm).
> >
> > I'm not able to reproduce, it would be useful to have another data point.
>
> Also happening for me with CONFIG_SYSFB_SIMPLEFB, on a Intel Core i7-
> 1065G7 (with iGPU).
>
> Reverting this commit on top of 5.19-rc5 "fixes" the issue.
With CONFIG_SYSFB_SIMPLEFB and CONFIG_FB_SIMPLE enabled, there is no
issue.
I guess it's something going wrong on a "drm -> drm" pass over. For now
I'll continue to use simpledrm with this commit reverted.
Hello Xi,
On 7/4/22 12:29, Xi Ruoyao wrote:
> On Mon, 2022-07-04 at 17:36 +0800, Xi Ruoyao wrote:
>
>>> Yes, please do. Either with CONFIG_SYSFB_SIMPLEFB disabled and CONFIG_FB_EFI
>>> enabled (so that "efi-framebuffer" is registered and efifb probed) or with
>>> CONFIG_SYSFB_SIMPLEFB but CONFIG_FB_SIMPLE enabled (so "simple-framebuffer
>>> is used too but with simplefb instead of simpledrm).
>>>
>>> I'm not able to reproduce, it would be useful to have another data point.
>>
>> Also happening for me with CONFIG_SYSFB_SIMPLEFB, on a Intel Core i7-
>> 1065G7 (with iGPU).
>>
>> Reverting this commit on top of 5.19-rc5 "fixes" the issue.
>
> With CONFIG_SYSFB_SIMPLEFB and CONFIG_FB_SIMPLE enabled, there is no
> issue.
>
> I guess it's something going wrong on a "drm -> drm" pass over. For now
> I'll continue to use simpledrm with this commit reverted.
>
Yes, we need to also cherry-pick b84efa28a48 ("drm/aperture: Run fbdev
removal before internal helpers") now that the sysfb_disable() patches
are in v5.19-rc5.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On 7/4/22 14:11, Xi Ruoyao wrote:
> On Mon, 2022-07-04 at 13:04 +0200, Javier Martinez Canillas wrote:
>> Hello Xi,
>>>
>>> With CONFIG_SYSFB_SIMPLEFB and CONFIG_FB_SIMPLE enabled, there is no
>>> issue.
>>>
>>> I guess it's something going wrong on a "drm -> drm" pass over. For now
>>> I'll continue to use simpledrm with this commit reverted.
>>>
>>
>> Yes, we need to also cherry-pick b84efa28a48 ("drm/aperture: Run fbdev
>> removal before internal helpers") now that the sysfb_disable() patches
>> are in v5.19-rc5.
>
> I confirm that cherry-picking b84efa28a48 fixes the issue for v5.19-rc5.
>
Thanks for testing it! Thomas is getting that patch through the drm-fixes
path, so it should make it to the v5.19-rc cycle at some point.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Hi
Am 04.07.22 um 14:11 schrieb Xi Ruoyao:
> On Mon, 2022-07-04 at 13:04 +0200, Javier Martinez Canillas wrote:
>> Hello Xi,
>>>
>>> With CONFIG_SYSFB_SIMPLEFB and CONFIG_FB_SIMPLE enabled, there is no
>>> issue.
>>>
>>> I guess it's something going wrong on a "drm -> drm" pass over. For now
>>> I'll continue to use simpledrm with this commit reverted.
>>>
>>
>> Yes, we need to also cherry-pick b84efa28a48 ("drm/aperture: Run fbdev
>> removal before internal helpers") now that the sysfb_disable() patches
>> are in v5.19-rc5.
>
> I confirm that cherry-picking b84efa28a48 fixes the issue for v5.19-rc5.
I've cherry-picked the commit into drm-misc-fixes. It will show up in
mainline soon.
Best regards
Thomas
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
On Mon, 2022-07-04 at 13:04 +0200, Javier Martinez Canillas wrote:
> Hello Xi,
> >
> > With CONFIG_SYSFB_SIMPLEFB and CONFIG_FB_SIMPLE enabled, there is no
> > issue.
> >
> > I guess it's something going wrong on a "drm -> drm" pass over. For now
> > I'll continue to use simpledrm with this commit reverted.
> >
>
> Yes, we need to also cherry-pick b84efa28a48 ("drm/aperture: Run fbdev
> removal before internal helpers") now that the sysfb_disable() patches
> are in v5.19-rc5.
I confirm that cherry-picking b84efa28a48 fixes the issue for v5.19-rc5.