Message-ID: <06314c8dc4adeb69cd7801f9621c831f75a37c89.camel@ljones.dev>
Date:   Sun, 04 Jun 2023 16:52:28 +1200
From:   Luke Jones <luke@...nes.dev>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     platform-driver-x86@...r.kernel.org,
        Barnabás Pőcze <pobrn@...tonmail.com>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, acpi4asus-user@...ts.sourceforge.net,
        corentin.chary@...il.com, markgross@...nel.org, jdelvare@...e.com,
        linux@...ck-us.net
Subject: Re: [PATCH v2 0/1] platform/x86: asus-wmi: add support for ASUS
 screenpad

On Thu, 2023-05-25 at 13:09 +0200, Hans de Goede wrote:
> Hi Luke,
> 
> On 5/16/23 00:34, Luke Jones wrote:
> > On Mon, May 15 2023 at 14:39:10 +0200, Hans de Goede
> > <hdegoede@...hat.com> wrote:
> 
> <snip>
> 
> > > >  Thank you on your work for this.
> > > > 
> > > >  Unfortunately I did not get a chance to react to the v1
> > > > posting and
> > > >  the remarks to switch to using /sys/class/backlight there
> > > > before you
> > > >  posted this v2.
> > > > 
> > > >  Technically the remark to use /sys/class/backlight for this is
> > > >  completely correct. But due to the way how userspace uses
> > > >  /sys/class/backlight this is a problematic.
> > > > 
> > > >  Userspace basically always assumes there is only 1 LCD panel
> > > >  and it then looks at /sys/class/backlight and picks 1
> > > >  /sys/class/backlight entry and uses that for the brightness
> > > >  slider in the desktop-environment UI / system-menu as well
> > > >  as to handle brightness up/down keyboard hotkey presses.
> > > > 
> > 
> > IMO, desktops need to adjust this expectation and start offering
> > controls for all possible screens. This would open up the
> > possibility of setting brightness of modern external screens also.
> > And then they should use the "Primary display" brightness controls,
> > or at least offer an option to set which is controlled.
> 
> Right this is what the proposal at:
> 
> https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/
> 
> is about. ATM userspace cannot reliably determine which
> /sys/class/backlight device belongs to which screen /
> video-output . So before desktops can offer this functionality
> we first need to fix the kernel <-> userspace APIs for this.
> 
> <snip>
> 
> > > >  So now we end up with 2 "raw" type backlight devices and if
> > > >  e.g. gnome-settings-daemon picks the right one now sort of
> > > >  is left to luck.
> > > > 
> > 
> > In a test KDE at least picked the right one.
> 
> That is good to know I'm still not entirely convinced using
> /sys/class/backlight for this is a good idea though. 
> 
> See below.
> 
> > > >  Well that is not entirely true, at least gnome-settings-daemon
> > > >  prefers raw backlight devices where the parent has an
> > > > "enabled"
> > > >  sysfs attribute (it expects the parent to be a drm_connector
> > > >  object) and where that enabled attribute reads as "enabled".
> > > > 
> > 
> > Ah I see. Parent for screenpad is "platform", while the main is on
> > igpu with parent "pci". I will paste some udev info at the end of
> > this reply.
> 
> Actually for the backlight-device on the iGPU the parent
> should be a drm-connector for the eDP (and the parent of
> that is the iGPU PCI device). Note I did not check
> the udev dump.
> 
> <snip>
> 
> > > >  Luke, question how does the second/exta panel look
> > > >  from an outputting video to it pov ?  Does it show
> > > >  up as an extra screen connected to a drm_connector
> > > >  on one of the GPUs. IOW can it be used with standard
> > > >  kernel-modesetting APIs ?
> > > 
> > 
> > Hi Hans, sorry about delay in response, just been tied up with
> > work.
> > 
> > As I don't actually have this kind of laptop I can't easily get
> > info, but I can ask a few people in my discord for information. Is
> > there anything in particular you would need to know? From my basic
> > understanding some of the points are:
> > 
> > 1. It does show as an actual additional screen
> > 2. Internal wiring is unclear, when dispaly MUX is switched to dgpu
> > the screen is still detected but not drawn to
> > 3. Point 2 is actually more uncertain as it seems only wayland had
> > this issue? I will get more info.
> 
> Right, so I think we first need to better understand the interactions
> between the WMI calls you are making and the drm/kms interface.
> 
> Question 1:
> 
> If you turn the second screen off through WMI, does it get seen as
> disconnected by the drm/kms driver then. Or does the drm/kms driver
> just go on treating it as an extra connected display, still drawing
> now no longer visible content to it ?

It is most certainly viewed as disconnected.

> IOW does the desktop environment's monitor-config panel no longer
> show the extra display after disabling it through WMI?

That is correct, it is no-longer a connected and visible display

> 
> The best way to check this is look under /sys/class/drm and find out
> which /sys/class/drm/card#-<conn-type>-# entry belongs to the extra
> panel. Step 1 check for all card#-<conn-type>-# entries
> where status returns connected, e.g. :
> 
> [hans@...lem ~]$ cat /sys/class/drm/card1-DP-1/status 
> connected
> 

On disable the status does change to:
cat /sys/class/drm/card1-DP-5/enabled
disabled

> Step 2: for the connected ones cat the modes, e.g.:
> 
> [hans@...lem ~]$ cat /sys/class/drm/card1-DP-1/modes
> 1920x1080
> 1600x1200
> ...
> 
> And find the one which matches with the resolution of the extra panel
> (the one which does not match with the resolution of the main panel).
> 
> Then turn the extra panel of through WMI and cat the status attribute
> again. If that still reads connected then that means the desktop
> environment keeps seeing an extra display output which is not ideal.
> This will e.g. cause any windows which were on the extra panel to
> stay there, even though they are no longer visible.
> 
> 
> Question 2:
> 
> If you turn the second screen off through drm/kms, using the desktop
> environments monitor config panel does this also turn off the
> backlight ?

The screen is dark but there is still some backlight coming out of it.
I think this means I need to add a small pre-off to the patch to ensure
backlight is fully off when display is turned off.

> 
> After disabling the screen in the desktop environments monitor config
> check that the enabled attribute, e.g. cat /sys/class/drm/card1-DP-
> 1/enabled shows disabled and after verifying this look at the extra
> screen in a dark room, do you see any backlight bleed indicating the
> backlight is still  on?
> 

Shows as disabled

> 
> We really want the backlight on/off state and the drm-connector
> enabled state to match. My proposal from above will allow this once
> implemented. Until we can hook this all up nicely I think it might be
> better to just go with the custom sysfs attributes from your v1 patch
> rather then adding a /sys/class/backlight device for this.
> 

I would like to go with the backlight patch as it seems more likely I
can adjust this without breaking userspace if required in future. The
WMI controls behave as expected to.

> 
> > So I think now is probably a good time to raise a particular issue
> > I've encountered with the last two years: the display MUX.
> > 
> > As I understand it now, there are two types of new MUX - the manual
> > switch, and the newer "Advanced Optimus" automatic switch. The
> > issues I have are with the manual switch since I've not encountered
> > the advanced optimus yet.
> > 
> > When the switch is. uh. switched. the dgpu drives the internal
> > display, and I expect that since the display is now detected
> > through the dgpu, this is why the dgpu is kept awake to drive it.
> > But, the igpu is also still active, and because of this the initial
> > boot from grub to display-manager is a black screen including tty.
> > This means anyone with an encrypted drive will never see the prompt
> > and they believe they have a failed boot. I don't know what to do
> > about this?
> 
> Is this with EFI booting or with classic BIOS boot? With EFI booting
> the EFIFB should be put on the right GPU by the firmware. So I
> suspect this is with classic BIOS boot?

No this is with EFI boot always. I'm not even sure these machines still
have the ability to boot oldschool bios style.

> 
> I think the best thing to do here is to just use EFI on machines like
> this. That or put grub in text mode so that it makes BIOS calls to
> display text. Using GRUB_TERMINAL_OUTPUT=gfxterm combined with
> classic BIOS booting will make grub try to directly drive the gfx
> card itself and I'm not surprised that it gets that wrong in this
> case.
> Note I think that just using EFI is prefered over switching grub to
> GRUB_TERMINAL_OUTPUT=console. I would expect
> GRUB_TERMINAL_OUTPUT=console to also work but I'm not sure. I don't
> think that the classic BIOS boot stuff is still tested by laptop
> vendors and esp. not tested with non standard BIOS settings ...

The grub gfx mode is GRUB_TERMINAL_OUTPUT="console", fedora default in
all cases here. Grub itself shows fine when the MUX mode is in dgpu
mode (aka, internal display connected to dgpu).

> > What I would love is somehow to either disable the igpu in kernel
> > if the MUX is toggled, or to change which device is the primary. Do
> > you have any thoughts on where I should start on this?
> 
> Not really, on the hybrid gfx devices which I have when optimus is
> disabled in the BIOS, muxing the main LCD to the Nvidia GPU the iGPU
> gets disabled by the BIOS.
> 
> If the iGPU in these models does not get disabled then I guess it is
> still needed for some connectors, e.g. maybe for displays connected
> to one of the thunderbolt ports ?

Yeah seems likely. I'll have to actually check this (I have TB4
connected monitor).

> Or maybe it is left on now a days for things like Intel quicksync ?
> 
> > An additional problem: `boot_vga` property of display adaptors.
> > I've been using this as a first-stage check in supergfxctl to
> > determine if there was a switch, but it is never ever reliable -
> > sometimes it changes, sometimes it is entirely blank (using udev to
> > fetch properties). And then I need to use a combination of checks
> > to determine state. So this `boot_vga` seems to always be available
> > but is practically unusable.
> 
> I'm afraid I cannot really help here. I think most of these laptops
> ship in Optimus mode (so both GPUs enabled) by default. And the non
> optimus mode is likely only tested under certain circumstances so
> there are going to be firmware bugs here. And Linux' code for
> detecting this likely also has issues of its own. Combine the 2 and
> you get lots of "fun".
> 
> I guess this is also what trips up grub. I wonder if this is better /
> more reliable under EFI mode though? Esp. since that is what vendors
> actually QA/test.

As mentioned, the machines are all booting in EFI mode right from the
start - the old mode (CMA?) doesn't seem to exist at all.

I really would like to find a solution here as it is tripping up a lot
of new users.

Cheers,
Luke.

P.S> I will go ahead with the screen patch as it looks like this
behaves as expected minus the need to turn off BL before turn off
display. I will submit revision later in the week.

Powered by blists - more mailing lists