On 28/11/2025 17:07, Thomas Zimmermann wrote:
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
I actually got confused a bit in following the flow so thank you for
asking me to double check.
GOP actually reports 1280x800 with a stride of 5120. So it kind of
reports a rotated view already, kind of.
Only when the rotation quirk from efifb_dmi_swap_width_height triggers
the stride gets incorrectly recalculated:
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;
So this is where things go wrong, well, they actually go wrong a little
bit even earlier, in gop.c:
si->lfb_size = si->lfb_linelength * si->lfb_height;
Which potentially underestimates the fb size. If GOP was forward looking
enough to give us the size we could derive the pitch based on size..
Anyway, as it stands it looks a quirk in sysfb_apply_efi_quirks looks it
is required to fix it all up.
I am a bit uneasy about declaring the fb size larger than what was
implied by firmware provided pitch * height * depth but limited to a
specific DMI match and if it looks visually okay I think it is a safe
assumption the quirked size is actually correct and safe.
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.
I can add a patch to replace with screen_info.lfb_depth / BITS_PER_BYTE?
+
+ 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
I thought the approach of looking at the multiple parameters at the same
time, similar how drm_get_panel_orientation_quirk() does it, is safer.
Ie. we don't want to edit unless the whole mode matches the expectation.
Regards,
Tvrtko
