| Message ID | 1461093565-18631-1-git-send-email-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Eric, On 04/19/2016 03:19 PM, Eric Anholt wrote: > With simplefb and vc4 both enabled, simplefb would probe first and be > fb0, displaying for a moment before vc4 probed fb1 and reconfigured > the graphics hardware so that only its fbdev worked. > > Cc: javier@osg.samsung.com > > Signed-off-by: Eric Anholt <eric@anholt.net> > --- > > Ccing Javier, since it looks like Exynos has also had this trouble, > and may want to get some compatible string in the list. > That's correct, we had the same issue on some Exynos5250 Chromebooks (Snow and Spring) where the vendor provided u-boot binaries that had simplefb support, so users were relying on simplefb and things broke when proper DRM support for the platform was introduced in mainline. > drivers/video/fbdev/simplefb.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c > index e9cf19977285..a2971aa686f5 100644 > --- a/drivers/video/fbdev/simplefb.c > +++ b/drivers/video/fbdev/simplefb.c > @@ -512,11 +512,44 @@ static struct platform_driver simplefb_driver = { > .remove = simplefb_remove, > }; > > +/* Returns true if a simplefb node that's present should be ignored. > + * > + * The U-Boot bootloader, and possibly others, may add a simplefb > + * device node to the existing device tree based on the video > + * configuration initialized by the firmware. If we have a native > + * driver for graphics, then simplefb would just be writing into > + * memory that's no longer being scanned out. > + */ > +static bool > +simplefb_superseded(void) > +{ > + static const char *compats[] __initconst = { > +#ifdef CONFIG_DRM_VC4 I think this should be #if IS_ENABLED(CONFIG_DRM_VC4) since you will have the same issue if the VC4 DRM driver is built as a module, once the module is loaded. Your current patch only will works if built-in. > + "brcm,bcm2835-vc4", > +#endif > + }; > + struct device_node *node; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(compats); i++) { > + node = of_find_compatible_node(NULL, NULL, compats[i]); > + if (node) { > + of_node_put(node); > + return true; > + } > + } > + > + return false; > +} > + > static int __init simplefb_init(void) > { > int ret; > struct device_node *np; > > + if (simplefb_superseded()) > + return 0; > + I wonder if this is the correct approach though, for example in the module case I mentioned before, the user won't have display working if something happens that prevents the module to be loaded (i.e: wrong module install). It would be better if there is a way to do a hand-off between the drivers, something like what happens with earlyprintk and the real serial console. > ret = platform_driver_register(&simplefb_driver); > if (ret) > return ret; > Best regards,
Hi Eric, On 04/19/2016 09:19 PM, Eric Anholt wrote: > With simplefb and vc4 both enabled, simplefb would probe first and be > fb0, displaying for a moment before vc4 probed fb1 and reconfigured > the graphics hardware so that only its fbdev worked. The way this is supposed to work (theoretically you're the first one to hit this on ARM) is that you call remove_conflicting_framebuffers() from the probe of your driver. simplefb can only be built-in and uses fs_initcall() to register itself, so that it becomes available early on, so it should always be probed before the vc4 driver. When we discussed how all of this should fit together (again theoretically) at ELCE 2014, we decided to follow what the x86 kms driver do here, the idea was that u-boot would set up a console (so that the user can interact with u-boot / see error messages without hooking up a serial console) and then simplefb would pick up the u-boot console asap, so that the kernel can show error msg. This is esp. important for the Debian / Fedora ARM images where things like the vc4 driver will be a module and we want the user to see errors from the initrd if things go wrong before loading e.g. the vc4 module. So the boot sequence would be: 1) u-boot configures a framebuffer, shows msgs there 2) kernel brings on builtin simplefb early, shows msgs that way 3) vc4 driver loads calls remove_conflicting_framebuffers which should disable/remove the simplefb framebuffer 4) vc4 driver register its own framebuffer, kernel msgs get displayed there I hope this makes sense, let me know if you've any questions. Regards, Hans > > Cc: javier@osg.samsung.com > > Signed-off-by: Eric Anholt <eric@anholt.net> > --- > > Ccing Javier, since it looks like Exynos has also had this trouble, > and may want to get some compatible string in the list. > > drivers/video/fbdev/simplefb.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c > index e9cf19977285..a2971aa686f5 100644 > --- a/drivers/video/fbdev/simplefb.c > +++ b/drivers/video/fbdev/simplefb.c > @@ -512,11 +512,44 @@ static struct platform_driver simplefb_driver = { > .remove = simplefb_remove, > }; > > +/* Returns true if a simplefb node that's present should be ignored. > + * > + * The U-Boot bootloader, and possibly others, may add a simplefb > + * device node to the existing device tree based on the video > + * configuration initialized by the firmware. If we have a native > + * driver for graphics, then simplefb would just be writing into > + * memory that's no longer being scanned out. > + */ > +static bool > +simplefb_superseded(void) > +{ > + static const char *compats[] __initconst = { > +#ifdef CONFIG_DRM_VC4 > + "brcm,bcm2835-vc4", > +#endif > + }; > + struct device_node *node; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(compats); i++) { > + node = of_find_compatible_node(NULL, NULL, compats[i]); > + if (node) { > + of_node_put(node); > + return true; > + } > + } > + > + return false; > +} > + > static int __init simplefb_init(void) > { > int ret; > struct device_node *np; > > + if (simplefb_superseded()) > + return 0; > + > ret = platform_driver_register(&simplefb_driver); > if (ret) > return ret; > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede <hdegoede@redhat.com> writes: > Hi Eric, > > On 04/19/2016 09:19 PM, Eric Anholt wrote: >> With simplefb and vc4 both enabled, simplefb would probe first and be >> fb0, displaying for a moment before vc4 probed fb1 and reconfigured >> the graphics hardware so that only its fbdev worked. > > The way this is supposed to work (theoretically you're the first > one to hit this on ARM) is that you call remove_conflicting_framebuffers() > from the probe of your driver. > > simplefb can only be built-in and uses fs_initcall() to register itself, > so that it becomes available early on, so it should always be probed > before the vc4 driver. > > When we discussed how all of this should fit together (again theoretically) > at ELCE 2014, we decided to follow what the x86 kms driver do here, the > idea was that u-boot would set up a console (so that the user can interact > with u-boot / see error messages without hooking up a serial console) and > then simplefb would pick up the u-boot console asap, so that the kernel > can show error msg. This is esp. important for the Debian / Fedora ARM > images where things like the vc4 driver will be a module and we want > the user to see errors from the initrd if things go wrong before loading > e.g. the vc4 module. > > So the boot sequence would be: > > 1) u-boot configures a framebuffer, shows msgs there > 2) kernel brings on builtin simplefb early, shows msgs that way > 3) vc4 driver loads calls remove_conflicting_framebuffers which should > disable/remove the simplefb framebuffer > 4) vc4 driver register its own framebuffer, kernel msgs get displayed there > > I hope this makes sense, let me know if you've any questions. I hadn't seen this in the other ARM drivers, so I hadn't found that solution. Looks like it works fine, the only funny part being that I have to use 0 to ~0 for my aperture because it's a UMA system.
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index e9cf19977285..a2971aa686f5 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -512,11 +512,44 @@ static struct platform_driver simplefb_driver = { .remove = simplefb_remove, }; +/* Returns true if a simplefb node that's present should be ignored. + * + * The U-Boot bootloader, and possibly others, may add a simplefb + * device node to the existing device tree based on the video + * configuration initialized by the firmware. If we have a native + * driver for graphics, then simplefb would just be writing into + * memory that's no longer being scanned out. + */ +static bool +simplefb_superseded(void) +{ + static const char *compats[] __initconst = { +#ifdef CONFIG_DRM_VC4 + "brcm,bcm2835-vc4", +#endif + }; + struct device_node *node; + int i; + + for (i = 0; i < ARRAY_SIZE(compats); i++) { + node = of_find_compatible_node(NULL, NULL, compats[i]); + if (node) { + of_node_put(node); + return true; + } + } + + return false; +} + static int __init simplefb_init(void) { int ret; struct device_node *np; + if (simplefb_superseded()) + return 0; + ret = platform_driver_register(&simplefb_driver); if (ret) return ret;
With simplefb and vc4 both enabled, simplefb would probe first and be fb0, displaying for a moment before vc4 probed fb1 and reconfigured the graphics hardware so that only its fbdev worked. Cc: javier@osg.samsung.com Signed-off-by: Eric Anholt <eric@anholt.net> --- Ccing Javier, since it looks like Exynos has also had this trouble, and may want to get some compatible string in the list. drivers/video/fbdev/simplefb.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)