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.
The quirk in this patch therefore proposes to correct back the mode to
the default native.
You are indeed right that the criteria needs to be tweaked. In v2 I've
fixed and it now looks like this:
...
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;
if (!data ||
(data->width == screen_info.lfb_width &&
data->height == screen_info.lfb_height)) {
screen_info.lfb_width = screen_info.lfb_height;
screen_info.lfb_height = temp;
if (data && data->pitch) {
screen_info.lfb_linelength = data->pitch;
screen_info.lfb_size = data->pitch *
data->width;
} else {
screen_info.lfb_linelength = 4 *
screen_info.lfb_width;
}
}
}
...
Ie. only swap width<->height for the pre-existing quirks and the new
quirk *if* it is in 1280x800.
Regards,
Tvrtko
