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?
