On 01/12/2025 10:18, Ard Biesheuvel wrote:
On Mon, 1 Dec 2025 at 11:03, Tvrtko Ursulin <[email protected]> wrote:
On 01/12/2025 09:39, Thomas Zimmermann wrote:
Hi
Am 01.12.25 um 10:20 schrieb Tvrtko Ursulin:
On 01/12/2025 07:32, Thomas Zimmermann wrote:
Hi
Am 29.11.25 um 11:44 schrieb Tvrtko Ursulin:
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.
These are correct values.
But the stream deck is this device: [1], right? It uses landscape-
mode orientation. Why does it require rotation at all?
[1] https://de.wikipedia.org/wiki/Steam_Deck#/media/
Datei:Steam_Deck_(front).png
That's the device yes. For the user the screen is landscape, but the
actual panel is 800x1280 portrait. Left edge is top of the display.
(Hence the pre-existing entry in drm_get_panel_orientation_quirk.)
I see. So the EFI display settings are configured as if this was a
landscape panel.
What happens if you leave the EFI settings as-is and simply remove the
panel-orientation quirk?
That would create effectively the same situation as without my patch
because the panel-orientation quirk does not trigger unless detected
screen is 800x1280. Result is corrupted console since fbcon thinks it is
a landscape 1280x800 screen.
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.
Yeah, we better not do that.
You mean declare it a firmware bug and live with the corrupt console
until the final fb driver takes over?
I only mean that we should not use more video memory than provided by EFI.
Right, but that information is not available in the GOP, right? Ie. as I
wrote above it appears assumed:
si->lfb_size = si->lfb_linelength * si->lfb_height;
Do we have any other options apart from corruption or assume firmware
configured GOP screen info incorrectly?
How does it make sense to recalculate the line length? Those invisible
pixels at the end of the scanline are not going to be transposed to
the other dimension, right?
Not sure what you meant here. The line above is from gop.c and the
context is that GOP screen info appears to not carry the frame buffer
size in bytes so it is implied.
Elsewhere in the patch I quirk the pitch to the correct value so rotated
rendering is correct.
But the corrected pitch also means that in principle we need to adjust
the frame buffer size, since it is larger than the size implied with the
incorrect pitch.
Regards,
Tvrtko
