Skip to contentSkip to conversation

load fbcon from drm_kms_helper

PATCH In review David Fries 3 participants 5 messages Dec 12, 2010 – Jan 12, 2011

Subsystems (2)
DRM drivers DRM misc
lore
Share & tools
lore

David Fries

author
#0 lore raw

PATCH 1/1 load fbcon from drm_kms_helper

Kconfig says fbcon is required by drm_kms_helper. If radeon, fbcon, and drm_kms_helper are all modules, radeon is auto loaded (by PCI id?), drm_kms_helper is loaded because of the module dependency, but fbcon isn't loaded leaving the console unusable. Since fbcon is required and there isn't an explicit module dependency, request the module to be loaded from drm_kms_helper.

Signed-off-by David Fries <[email protected]>
Cc David Airlie <[email protected]>
Cc [email protected]

---
The last patch had a typo 'namue', mental reminder, test again after running checkpatch.pl.

This solves compiling CONFIG_FB=m and being left with a blank screen because the radeon module is automatically loaded, but fbcon isn't. If radeon had to be manually loaded, then it would be the user's fault for
not loading fbcon as well, but as radeon is being loaded automatically,
there isn't much a user can do from console to even fix it. More bug details from here,
https://bugzilla.kernel.org/show_bug.cgi?id=16221

Does this look like a reasonable solution? Should it bother checking find_module first?

 drivers/gpu/drm/drm_fb_helper.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
drivers/gpu/drm/drm_fb_helper.c +21 -0
index d2849e4..41e3c3c 100644
@@ -1530,3 +1530,24 @@ bool drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 
+/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EMBEDDED)
+ * but the module doesn't depend on any fb console symbols.  At least
+ * attempt to load fbcon to avoid leaving the system without a usable console.
+ */
+#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EMBEDDED)
+static int __init drm_fb_helper_modinit(void)
+{
+	const char *name = "fbcon";
+	struct module *fbcon;
+
+	mutex_lock(&module_mutex);
+	fbcon = find_module(name);
+	mutex_unlock(&module_mutex);
+
+	if (!fbcon)
+		request_module_nowait(name);
+	return 0;
+}
+
+module_init(drm_fb_helper_modinit);
+#endif
Signature · 1 line
1.7.2.3
#1 lore raw
Quoted 14 lines · On Sun, 12 Dec 2010 12:39:22 -0600 David Fries <[email protected]> wrote:

Kconfig says fbcon is required by drm_kms_helper. If radeon, fbcon, and drm_kms_helper are all modules, radeon is auto loaded (by PCI id?), drm_kms_helper is loaded because of the module dependency, but fbcon isn't loaded leaving the console unusable. Since fbcon is required and there isn't an explicit module dependency, request the module to be loaded from drm_kms_helper.

Signed-off-by David Fries <[email protected]>
Cc David Airlie <[email protected]>
Cc [email protected]

---
The last patch had a typo 'namue', mental reminder, test again after running checkpatch.pl.

Quoted 9 lines

This solves compiling CONFIG_FB=m and being left with a blank screen because the radeon module is automatically loaded, but fbcon isn't. If radeon had to be manually loaded, then it would be the user's fault for
not loading fbcon as well, but as radeon is being loaded automatically,
there isn't much a user can do from console to even fix it. More bug details from here,
https://bugzilla.kernel.org/show_bug.cgi?id=16221

I guess this is reasonable. Maybe _if_ there actually is a usecase for a drm driver without fbcon, the drm could provide a parameter to skip loading fbcon?

But also the drm Kconfig seems to be bogus? SELECT is not transitiv. So selecting DRM_KMS_HELPER is not enough, as it will not select FB and FRAMEBUFFER_CONSOLE. Maybe the drm drivers that currently select DRM_KMS_HELPER should instead depend on it.

Sincerely,
Flo

author
#2 lore raw
Quoted 27 lines · Florian Mickler
Quoted 14 lines · On Sun, 12 Dec 2010 12:39:22 -0600 David Fries <[email protected]> wrote:

Kconfig says fbcon is required by drm_kms_helper. If radeon, fbcon, and drm_kms_helper are all modules, radeon is auto loaded (by PCI id?), drm_kms_helper is loaded because of the module dependency, but fbcon isn't loaded leaving the console unusable. Since fbcon is required and there isn't an explicit module dependency, request the module to be loaded from drm_kms_helper.

Signed-off-by David Fries <[email protected]>
Cc David Airlie <[email protected]>
Cc [email protected]

---
The last patch had a typo 'namue', mental reminder, test again after running checkpatch.pl.

Quoted 9 lines

This solves compiling CONFIG_FB=m and being left with a blank screen because the radeon module is automatically loaded, but fbcon isn't. If radeon had to be manually loaded, then it would be the user's fault for
not loading fbcon as well, but as radeon is being loaded automatically,
there isn't much a user can do from console to even fix it. More bug details from here,
https://bugzilla.kernel.org/show_bug.cgi?id=16221

Quoted 3 lines

I guess this is reasonable. Maybe _if_ there actually is a usecase for a drm driver without fbcon, the drm could provide a parameter to skip loading fbcon?

My question was more, is there a usecase for a drm driver with the fbcon module available, but not wanting it loaded?

Quoted 4 lines

But also the drm Kconfig seems to be bogus? SELECT is not transitiv. So selecting DRM_KMS_HELPER is not enough, as it will not select FB and FRAMEBUFFER_CONSOLE. Maybe the drm drivers that currently select DRM_KMS_HELPER should instead depend on it.

But the "select FB" and "select FRAMEBUFFER_CONSOLE if !EMBEDDED" Kconfig for DRM_KMS_HELPER is the mechanism that cause FB and FRAMEBUFFER_CONSOLE to be compiled. The was the other part of my reasoning, if there is a case for drm without fbcon, DRM_KMS_HELPER shouldn't force fbcon to be compiled. But as it does require fbcon to be compiled, and things are clearly broken if a drm driver is loaded (radeon) but fbcon isn't loaded (no usable console) DRM_KMS_HELPER should try to load it, hence the patch.

Just for a check I used make xconfig to verify they are automatically selected.
set FB, DRM_RADEON, DRM etc to =n so FRAMEBUFFER_CONSOLE can also be set to =n set DRM=m (FRAMEBUFFER_CONSOLE still =n)
set DRM_RADEON=m (DRM_KMS_HELPER, FB, and FRAMEBUFFER_CONSOLE become =m) That's because DRM_RADEON sets DRM_KMS_HELPER, and DRM_KMS_HELPER selects FB and FRAMEBUFFER_CONSOLE
config DRM_RADEON
select DRM_KMS_HELPER
config DRM_KMS_HELPER
select FB
select FRAMEBUFFER_CONSOLE if !EMBEDDED
and FRAMEBUFFER_CONSOLE is forced to =m

Signature · 2 lines
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
#3 lore raw
Quoted 28 lines · On Sun, 12 Dec 2010 18:34:11 -0600 David Fries <[email protected]> wrote:
Quoted 27 lines · Florian Mickler
Quoted 23 lines · On Sun, 12 Dec 2010 12:39:22 -0600 David Fries <[email protected]> wrote:

Kconfig says fbcon is required by drm_kms_helper. If radeon, fbcon, and drm_kms_helper are all modules, radeon is auto loaded (by PCI id?), drm_kms_helper is loaded because of the module dependency, but fbcon isn't loaded leaving the console unusable. Since fbcon is required and there isn't an explicit module dependency, request the module to be loaded from drm_kms_helper.

Signed-off-by David Fries <[email protected]>
Cc David Airlie <[email protected]>
Cc [email protected]

---
The last patch had a typo 'namue', mental reminder, test again after running checkpatch.pl.

This solves compiling CONFIG_FB=m and being left with a blank screen because the radeon module is automatically loaded, but fbcon isn't. If radeon had to be manually loaded, then it would be the user's fault for
not loading fbcon as well, but as radeon is being loaded automatically,
there isn't much a user can do from console to even fix it. More bug details from here,
https://bugzilla.kernel.org/show_bug.cgi?id=16221

Quoted 6 lines
Quoted 3 lines

I guess this is reasonable. Maybe _if_ there actually is a usecase for a drm driver without fbcon, the drm could provide a parameter to skip loading fbcon?

My question was more, is there a usecase for a drm driver with the fbcon module available, but not wanting it loaded?

Isn't that the same question? I intended it to be in any case. :)

Quoted 15 lines
Quoted 5 lines

But also the drm Kconfig seems to be bogus? SELECT is not transitiv. So selecting DRM_KMS_HELPER is not enough, as it will not select FB and FRAMEBUFFER_CONSOLE. Maybe the drm drivers that currently select DRM_KMS_HELPER should instead depend on it.

But the "select FB" and "select FRAMEBUFFER_CONSOLE if !EMBEDDED" Kconfig for DRM_KMS_HELPER is the mechanism that cause FB and FRAMEBUFFER_CONSOLE to be compiled. The was the other part of my reasoning, if there is a case for drm without fbcon, DRM_KMS_HELPER shouldn't force fbcon to be compiled. But as it does require fbcon to be compiled, and things are clearly broken if a drm driver is loaded (radeon) but fbcon isn't loaded (no usable console) DRM_KMS_HELPER should try to load it, hence the patch.

Quoted 14 lines

Just for a check I used make xconfig to verify they are automatically selected.
set FB, DRM_RADEON, DRM etc to =n so FRAMEBUFFER_CONSOLE can also be set to =n set DRM=m (FRAMEBUFFER_CONSOLE still =n)
set DRM_RADEON=m (DRM_KMS_HELPER, FB, and FRAMEBUFFER_CONSOLE become =m) That's because DRM_RADEON sets DRM_KMS_HELPER, and DRM_KMS_HELPER selects FB and FRAMEBUFFER_CONSOLE
config DRM_RADEON
select DRM_KMS_HELPER
config DRM_KMS_HELPER
select FB
select FRAMEBUFFER_CONSOLE if !EMBEDDED
and FRAMEBUFFER_CONSOLE is forced to =m

You are right. Sorry for the confusion. select is indeed transitive. (I could have sworn... bah..)

#4 lore raw
In reply to David Fries

Hi David!

Did you see this patch?
I was wondering what you think about it...

Quoted 10 lines · On Sun, 12 Dec 2010 12:39:22 -0600 David Fries <[email protected]> wrote:

Kconfig says fbcon is required by drm_kms_helper. If radeon, fbcon, and drm_kms_helper are all modules, radeon is auto loaded (by PCI id?), drm_kms_helper is loaded because of the module dependency, but fbcon isn't loaded leaving the console unusable. Since fbcon is required and there isn't an explicit module dependency, request the module to be loaded from drm_kms_helper.

Signed-off-by David Fries <[email protected]>
Cc David Airlie <[email protected]>
Cc [email protected]

Regards,
Flo