On Mon, 1 Dec 2025 at 11:33, Tvrtko Ursulin <[email protected]> wrote: > > > 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. >
OK, so if I understand all of the above correctly, you have a 800x1280 panel with 832 pixels per scanline, right? And the 5120 pitch is simply bogus, but needed to maintain the fiction that the panel is 1280 pixels wide, and so the resulting lfb_size is bogus too? Since we know that the PixelsPerScanline value is incorrect, I don't think there is any point in attempting to cross reference this against other firmware provided data. But it would make sense imho to apply the quirk only if the exact combination of incorrect values (i.e., 1280x800/5120) is encountered.
