Bug 201553 - GPD Pocket 2 - broken rotation (i915 framebuffer)
Summary: GPD Pocket 2 - broken rotation (i915 framebuffer)
Status: NEW
Alias: None
Product: Platform Specific/Hardware
Classification: Unclassified
Component: x86-64 (show other bugs)
Hardware: Intel Linux
: P1 normal
Assignee: platform_ia-64
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-29 06:34 UTC by chrisaw
Modified: 2018-10-31 10:25 UTC (History)
1 user (show)

See Also:
Kernel Version: 4.18.14
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Hacky patch to force gpd pocket 2 rotation (efifb only) (1.73 KB, patch)
2018-10-29 06:34 UTC, chrisaw
Details | Diff
[2] GPD Rotation patch (2.01 KB, patch)
2018-10-30 16:27 UTC, chrisaw
Details | Diff

Description chrisaw 2018-10-29 06:34:04 UTC
Created attachment 279237 [details]
Hacky patch to force gpd pocket 2 rotation (efifb only)

I've just got the GPD Pocket 2 and it's been fiddly to try and fix the display rotation in the kernel.

So far I've written a hacky patch which does successfully force rotation to work by disabling most of the checks used for the original GPD Pocket quirk written by Hans de Goede (thanks!)

The problem I'm facing is - with the initial efifb framebuffer - this works and rotates the display from portrait to landscape as it should. The issue occurs once the i915 module loads and the handover to inteldrmfb occurs - the displasy then rotates back to portrait.

As such - it seems that i915 / inteldrmfb ignores the rotation quirk provided by Hans's work.
Comment 1 chrisaw 2018-10-29 06:46:43 UTC
Just to add to this - the dmesg output for this handover is:

[root@pocket:~]# dmesg |grep fb
[    0.000000] Command line: initrd=\efi\nixos\y8h0aadb1brffb7v16fxgcbxpdkzffl1-initrd-initrd.efi systemConfig=/nix/store/wc5bzk9yaghkc1c7s1zycxg0yic6bq7q-nixos-system-pocket-19.03pre155751.45a419ab5a2 init=/nix/store/wc5bzk9yaghkc1c7s1zycxg0yic6bq7q-nixos-system-pocket-19.03pre155751.45a419ab5a2/init intel_pstate=passive loglevel=4
[    0.000000] Kernel command line: initrd=\efi\nixos\y8h0aadb1brffb7v16fxgcbxpdkzffl1-initrd-initrd.efi systemConfig=/nix/store/wc5bzk9yaghkc1c7s1zycxg0yic6bq7q-nixos-system-pocket-19.03pre155751.45a419ab5a2 init=/nix/store/wc5bzk9yaghkc1c7s1zycxg0yic6bq7q-nixos-system-pocket-19.03pre155751.45a419ab5a2/init intel_pstate=passive loglevel=4
[    0.187061] pci 0000:00:02.0: BAR 2: assigned to efifb
[    0.639831] efifb: probing for efifb
[    0.639848] efifb: framebuffer at 0xc0000000, using 1900k, total 1900k
[    0.639849] efifb: mode is 600x800x32, linelength=2432, pages=1
[    0.639850] efifb: scrolling: redraw
[    0.639853] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0
[    0.646094] fb0: EFI VGA frame buffer device
[    1.166299] fb: switching to inteldrmfb from EFI VGA
[    1.293633] fbcon: inteldrmfb (fb0) is primary device
[    2.453447] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
Comment 2 chrisaw 2018-10-29 11:48:29 UTC
Further discovery - using boot option "i915.modeset=0" disables the inteldrmfb switchover and keeps the framebuffer with EFIFB which then retains the rotation properly.

So it seems like we're dealing with 2 problems here:

1.) How to uniquely identify the GPD Pocket 2 for use in the quirks code.
2.) How to retain rotation between EFIFB and inteldrmfb.

I'll keep working on this as time permits and try to address at least #1 - not sure if I'm best suited to resolving issue #2 but I'll certainly try.
Comment 3 Hans de Goede 2018-10-29 13:25:42 UTC
Hi,

The only thing I can think of which is causing your patch to not work without removing the extra checks is the resolution not matching, what is the output of:

cat /sys/class/drm/card0-DSI*/modes

?

Regards,

Hans
Comment 4 chrisaw 2018-10-29 13:27:25 UTC
(In reply to Hans de Goede from comment #3)
> Hi,
> 
> The only thing I can think of which is causing your patch to not work
> without removing the extra checks is the resolution not matching, what is
> the output of:
> 
> cat /sys/class/drm/card0-DSI*/modes
> 
> ?
> 
> Regards,
> 
> Hans

I did figure out the reason for this - EFIFB runs at 600x800 (portrait way around version of 800x600) so the resolutions don't match indeed.
Comment 5 Hans de Goede 2018-10-29 13:33:23 UTC
(In reply to chrisaw from comment #4)
> I did figure out the reason for this - EFIFB runs at 600x800 (portrait way
> around version of 800x600) so the resolutions don't match indeed.

Then the quirk should still work for the i915 driver (when not passing modesetting=0) we should first get the i915 driver to work and then later on we can see if we can also do something for the efifb.

Note if GPD is doing weird hacks with the efifb we may not be able to also get the efifb to work.
Comment 6 chrisaw 2018-10-29 15:16:55 UTC
Sounds sensible - just to confirm though:

- efifb does rotate properly with my patch in place. I'm not suggesting this gets merged obviously, it's a total hack but just confirming.

- I haven't yet tried compiling i915 in to the kernel and *not* efifb (if that's possible) - will try that a bit later on and report back. If we start with i915 framebuffer driver - we'll never have a handover (efifb->inteldrmfb) to perform.

- If I boot the system with fbcon=rotate:1 this issue is not present - including the handover from efifb->inteldrmfb. With that in mind - it seems to me like the i915 driver isn't acting on the rotation data from your code at all.

I'll have another tinker with this when I get home from work shortly and report back any progress I make. I have potential workarounds for a lot of this stuff but I would rather handle the rotation properly in the kernel rather than passing it fbcon bootargs.
Comment 7 chrisaw 2018-10-29 20:21:07 UTC
I (In reply to Hans de Goede from comment #3)
> Hi,
> 
> The only thing I can think of which is causing your patch to not work
> without removing the extra checks is the resolution not matching, what is
> the output of:
> 
> cat /sys/class/drm/card0-DSI*/modes
> 
> ?
> 
> Regards,
> 
> Hans

Did a bit of poking about here and it looks like they're using eDP instead of DSI in the Pocket 2 as you can see below:

[root@pocket:~]# cat /sys/class/drm/card0-eDP-1/modes 
1200x1920

Currently building a kernel with CONFIG_FB_EFI=n set so the only fb driver initialised should be when i915 module is loaded.

I'll report back on the results but I just wanted to feedback about the DSI->eDP switch in case it's important - though from a cursory glance at your code I don't see anything DSI specific.
Comment 8 Hans de Goede 2018-10-29 21:12:45 UTC
If the panel is eDP you also need to add this patch to the kernel:

https://github.com/jwrdegoede/linux-sunxi/commit/6502e6ae97cbf5b3c88ef90743d493048f4764aa

This is queued for merging into 4.20.

As for building in i915 that tends to not work well and even then the efifb driver will load first. We may need to have 2 entries for the GPD pocket 2 in this case, one for the actual screen resolution and one for the mode in which efifb comes up.
Comment 9 chrisaw 2018-10-29 22:11:36 UTC
(In reply to Hans de Goede from comment #8)
> If the panel is eDP you also need to add this patch to the kernel:
> 
> https://github.com/jwrdegoede/linux-sunxi/commit/
> 6502e6ae97cbf5b3c88ef90743d493048f4764aa
> 
> This is queued for merging into 4.20.
> 
> As for building in i915 that tends to not work well and even then the efifb
> driver will load first. We may need to have 2 entries for the GPD pocket 2
> in this case, one for the actual screen resolution and one for the mode in
> which efifb comes up.

Very useful about the eDP patch, thanks!

I've added that patch to my kernel build and I've got it building again now.

I'll poke around and see if I can find a more elegant solution than the two entries and will report back.
Comment 10 chrisaw 2018-10-30 16:27:09 UTC
Created attachment 279253 [details]
[2] GPD Rotation patch
Comment 11 chrisaw 2018-10-30 16:29:07 UTC
Ok lots of progress on this one:

- Using the eDP patch the intel fb rotation works perfectly.

- I've created a new patch to bypass resolution checking on the GPD Pocket 2 only (by using 0x0 as resolution.)

- I've verified the new patch works for both EFIFB and i915-fb support since it essentially ignores the resolution check and relies on the BIOS date.

What're your opinions on this change Hans? Happy to change it - this is just the cleanest way I could think of implementing this without larger changes or duplicating the gpd pocket 2 entry.
Comment 12 Hans de Goede 2018-10-30 18:17:23 UTC
(In reply to chrisaw from comment #11)
> Ok lots of progress on this one:
> 
> - Using the eDP patch the intel fb rotation works perfectly.
> 
> - I've created a new patch to bypass resolution checking on the GPD Pocket 2
> only (by using 0x0 as resolution.)

The problem with that is that it then will also get applied when booting with an external monitor. So I'm not really in favor of this.

> - I've verified the new patch works for both EFIFB and i915-fb support since
> it essentially ignores the resolution check and relies on the BIOS date.
> 
> What're your opinions on this change Hans? Happy to change it - this is just
> the cleanest way I could think of implementing this without larger changes
> or duplicating the gpd pocket 2 entry.

I would prefer to just have an entry with the native resolution for now. If you boot with "quiet" on the kernel commandline you should not get any kernel messages before the i915 driver loads, so the rotation should not be a problem.

Also have you looked if there is maybe a BIOS option which does make the efifb come up in the native resolution? Typically enabling fastboot (or disabling it) may influence this.

Are you perhaps booting Linux from an external media using the EFI bootmenu? 

If yes, then what happens if you change the boot order so you boot from Linux by default, without going through any firmware menus, does that make the efifb come up in its native resolution?
Comment 13 chrisaw 2018-10-31 09:34:36 UTC
(In reply to Hans de Goede from comment #12)
> (In reply to chrisaw from comment #11)
> > Ok lots of progress on this one:
> > 
> > - Using the eDP patch the intel fb rotation works perfectly.
> > 
> > - I've created a new patch to bypass resolution checking on the GPD Pocket
> 2
> > only (by using 0x0 as resolution.)
> 
> The problem with that is that it then will also get applied when booting
> with an external monitor. So I'm not really in favor of this.

Ah, good point! Hadn't thought of that - yep that makes complete sense.

> 
> > - I've verified the new patch works for both EFIFB and i915-fb support
> since
> > it essentially ignores the resolution check and relies on the BIOS date.
> > 
> > What're your opinions on this change Hans? Happy to change it - this is
> just
> > the cleanest way I could think of implementing this without larger changes
> > or duplicating the gpd pocket 2 entry.
> 
> I would prefer to just have an entry with the native resolution for now. If
> you boot with "quiet" on the kernel commandline you should not get any
> kernel messages before the i915 driver loads, so the rotation should not be
> a problem.
> 

I'm not going with the "quiet" option because I like to know what's going on at boot time. The better alternative to this (in my opinion) is to use: "video=efifb:off" which will present a blank screen until the i915 module loads (which for me is the very first module I load within the initrd stage so about 1sec or so of screen off, then up at the native resolution.

> Also have you looked if there is maybe a BIOS option which does make the
> efifb come up in the native resolution? Typically enabling fastboot (or
> disabling it) may influence this.

The BIOS is locked down again - hoping they release a nice open version like they did on the GPD Pocket original. In the meantime, however, the only option relating to GPU usage is to use the EFI driver or Auto - if I use Auto - it just selects EFI driver if I boot an EFI capable OS (according to its BIOS description.)

I know about the fastboot stuff but sadly there is no option for this present at all yet. Interestingly there is a BIOS option for how the rotation is handled but it just lets you select which type of orientation you want (really just which angle you want but it gives text descriptions of them.)

The only one thing I did think of is - I think GRUB2 can force resolutions but sadly systemd-boot (aka. gummiboot) cannot and just uses whatever the BIOS/EFI presents as its native resolution.

> 
> Are you perhaps booting Linux from an external media using the EFI bootmenu? 

Nope, booting from internal mmc storage (the only built in storage on the GPDs as you know.)

> 
> If yes, then what happens if you change the boot order so you boot from
> Linux by default, without going through any firmware menus, does that make
> the efifb come up in its native resolution?

N/A
Comment 14 Hans de Goede 2018-10-31 10:25:22 UTC
> The BIOS is locked down again

Bummer.

Ok, so I think the best thing to do here is really to go with 2 entries for the 2 different resolutions.

Can you split this over 2 patches, explain why we also have the 600x800 entry in the commit message of the second patch and then submit both upstream with me in the Cc?

Note You need to log in before you can comment on or make changes to this bug.