On Tue, 2 Dec 2025 at 11:44, Tvrtko Ursulin <[email protected]> wrote: > > > On 02/12/2025 07:34, Thomas Zimmermann wrote: > > Hi > > > > Am 01.12.25 um 16:43 schrieb Tvrtko Ursulin: > >> > >> On 01/12/2025 15:00, Ard Biesheuvel wrote: > >>> 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. > >> > >> Right, the whole 1280x800 mode I *think* could be "bogus", that is, > >> some kind of a software rotated mode implemented by the firmware. > >> > >> Default mode is 800x1280 (pitch 832), while this second native > >> resolution mode is 1280x800 (pitch 1280). > >> > >> If default mode is left then both simpledrmfb and efidrmfb work fine. > >> The existing panel orientation quirk will trigger on 800x1280 and tell > >> fbcon to rotate. > >> > >> But if someone, like for example grub2, changed the mode to this > >> software rotated one then the existing DRM quirk will not work. > > > > So this is a bug in grub? Should it supply the original mode? > > > > > > Apologies for only asking dump questions here. I find this very confusing. > > Not at all, it is complicated and open whether it is worth improving. > > I don't think it is a grub bug. To me it seems like an unfortunate > consequence of protocol limitations. > > > In the correct mode 800x1280, the first native pixel should be on the > > lower left corner. and the second pixel should be 'up form it'. And > > because it's marked as rotated CCW, fbcon adapts correctly. > > > > If the display is in the bogus mode 1280x800, in which direction does it > > draw by default? The framebuffer's first pixel should still be in one > > of the corners. And the second pixel is nearby. In which direction does > > it advance? > > I am not a grub expert, but I had a brief look at its codebase, and > AFAICT it draws by software rendering into a shadow frame buffer and > calls GOP->Blit to update the screen. > > As such my assumption is that with the fake 1280x800 mode, firmware > implements the blit to rotate under the hood. > > So for grub everything is fine. It sets 1280x800, sees 1280x800, renders > the menu in there, and courtesy of firmware blit the menu is presented > in the human friendly orientation. >
Indeed - GRUB never draws into the EFI GOP linear frame buffer directly, and this seems to be what makes it work in this case. Could you boot with video=efifb:list on the kernel command line to see how the GOP modes are reported? I suppose the bug here is that the mode in question is not reported with PixelFormat==PixelBltOnly as it should be.
