Hi,
thanks for the bug report
Am 28.11.25 um 16:04 schrieb Tvrtko Ursulin:
I am not sure how is simpledrmfb on top of EFI supposed to work, but at
least at the moment it appears there is a missing link in the "discovery"
of frame buffer parameters.
What I can see is that EFI GOP reads some parameters from the firmware and
infers the other, such as in this case problematic pitch, or stride.
The pitch/stride value comes from the firmware via pixels_per_scanline [1].
Can you verify that this value is really 800 instead of 832 (eq 3328
bytes) ?
[1]
https://elixir.bootlin.com/linux/v6.17.9/source/drivers/firmware/efi/libstub/gop.c#L493
One could be easily excused in thinking that pitch cannot be reliably
inferred, since different display hardware has differing alignment
requirements, so it is unclear how is hardware agnostic solution supposed
to work.
In the specific case of the Steam Deck hardware we have a 800x1280 native
panel which is also installed rotated 90 degrees counter clockwise.
Firmware appears to set up the pitch as 3328, while GOP assumes 3200,
based of a width * bpp calculation.
832 is a multiple of 64, while 800 is not. I've seen this
over-allocation in other context as well.
When this incorrect pitch propagates through (rather complicated) fbcon
and DRM call paths, the end result is corrupted rendering all until the
amdgpu takes over the fbdev.
Simplistic solution in this patch is to add a DMI quirk to the EFI
frame buffer setup code.
Apart from the incorrect pitch, the quirk also does the swapping of the
width and height. Apart from setting the correct fbcon dimensions this
one also allows the quirk from drm_get_panel_orientation_quirk() to
report the correct orientation.
Nice
Signed-off-by: Tvrtko Ursulin <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: "Christian König" <[email protected]>
Cc: Melissa Wen <[email protected]>
Cc: Rodrigo Siqueira <[email protected]>
Cc: Mario Limonciello <[email protected]>
Cc: [email protected]
---
drivers/firmware/efi/sysfb_efi.c | 48 ++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 1e509595ac03..84d9049bb2cb 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -231,6 +231,18 @@ static const struct dmi_system_id efifb_dmi_system_table[]
__initconst = {
{},
};
+struct efifb_mode_fixup {
+ unsigned int width;
+ unsigned int height;
+ unsigned int pitch;
+};
+
+static const struct efifb_mode_fixup efifb_steamdeck_mode_fixup = {
+ .width = 1280,
+ .height = 800,
+ .pitch = 3328,
+};
+
/*
* Some devices have a portrait LCD but advertise a landscape resolution (and
* pitch). We simply swap width and height for these devices so that we can
@@ -281,6 +293,24 @@ static const struct dmi_system_id
efifb_dmi_swap_width_height[] __initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X91"),
},
},
+ {
+ /* Valve Steam Deck (Jupiter) */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Valve"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Jupiter"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "1"),
+ },
+ .driver_data = (void *)&efifb_steamdeck_mode_fixup,
+ },
+ {
+ /* Valve Steam Deck (Galileo) */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Valve"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Galileo"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "1"),
+ },
+ .driver_data = (void *)&efifb_steamdeck_mode_fixup,
+ },
{},
};
@@ -351,17 +381,31 @@ static struct fwnode_handle efifb_fwnode;
__init void sysfb_apply_efi_quirks(void)
{
+ const struct dmi_system_id *match;
+
if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI ||
!(screen_info.capabilities & VIDEO_CAPABILITY_SKIP_QUIRKS))
dmi_check_system(efifb_dmi_system_table);
- if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI &&
- dmi_check_system(efifb_dmi_swap_width_height)) {
Could we keep swap_width_height a separate list?
The for loop would then be a full override-display list.
+ if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+ return;
+
+ for (match = dmi_first_match(efifb_dmi_swap_width_height);
+ match;
+ match = dmi_first_match(match + 1)) {
+ const struct efifb_mode_fixup *data = match->driver_data;
u16 temp = screen_info.lfb_width;
screen_info.lfb_width = screen_info.lfb_height;
screen_info.lfb_height = temp;
screen_info.lfb_linelength = 4 * screen_info.lfb_width;
FTR: This calculation has always been wrong. We need to take the
lfb_depth into account. No need to fix this here; just so you know.
+
+ if (data && data->pitch &&
+ data->width == screen_info.lfb_height &&
+ data->height == screen_info.lfb_width) {
+ screen_info.lfb_linelength = data->pitch;
+ screen_info.lfb_size = data->pitch * data->width;
+ }
If we have a separate fix-up list, we can make this much nicer:
if (data->width)
// assign data->width
if (data->height)
// assign data->width
if (data->data)
// assign data->pitch
and then recompute linelength and size
Best regards
Thomas
}
}
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
