[v2,0/1] platform/x86: asus-wmi: add support for ASUS screenpad

  Adds support for the screenpad(-plus) found on a few ASUS laptops that have a main 16:9 or 16:10 screen and a shorter screen below the main but above the keyboard.
The support consists of:
- On off control
- Setting brightness from 0-255

There are some small quirks with this device when considering only the raw WMI methods:
1. The Off method can only switch the device off
2. Changing the brightness turns the device back on
3. To turn the device back on the brightness must be > 1
4. When the device is off the brightness can't be changed (so it is stored by the driver if device is off).
5. Booting with a value of 0 brightness (retained by bios) means the bios will set a value of > 0, < 15 which is far too dim and was unexpected by testers. The compromise was to set the brightness to 60 which is a usable brightness if the module init brightness was under 15.
6. When the device is off it is "unplugged"

All of the above points are addressed within the patch to create a good user experience and keep within user expectations.

Changelog:
- V2
  - Complete refactor to use as a backlight device

Luke D. Jones (1):
  platform/x86: asus-wmi: add support for ASUS screenpad

 .../ABI/testing/sysfs-platform-asus-wmi       |   2 +-
 drivers/platform/x86/asus-wmi.c               | 132 ++++++++++++++++++
 drivers/platform/x86/asus-wmi.h               |   1 +
 include/linux/platform_data/x86/asus-wmi.h    |   4 +
 4 files changed, 138 insertions(+), 1 deletion(-)
  

Hi Luke,

On 5/5/23 06:30, Luke D. Jones wrote:
> Adds support for the screenpad(-plus) found on a few ASUS laptops that have a main 16:9 or 16:10 screen and a shorter screen below the main but above the keyboard.
> The support consists of:
> - On off control
> - Setting brightness from 0-255
> 
> There are some small quirks with this device when considering only the raw WMI methods:
> 1. The Off method can only switch the device off
> 2. Changing the brightness turns the device back on
> 3. To turn the device back on the brightness must be > 1
> 4. When the device is off the brightness can't be changed (so it is stored by the driver if device is off).
> 5. Booting with a value of 0 brightness (retained by bios) means the bios will set a value of > 0, < 15 which is far too dim and was unexpected by testers. The compromise was to set the brightness to 60 which is a usable brightness if the module init brightness was under 15.
> 6. When the device is off it is "unplugged"
> 
> All of the above points are addressed within the patch to create a good user experience and keep within user expectations.
> 
> Changelog:
> - V2
>   - Complete refactor to use as a backlight device

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.

In the (recent) past the kernel used to register e.g.
both /sys/class/backlight/acpi_video0 and
/sys/class/backlight/intel_backlight

For ACPI resp. direct hw control of the LCD panel backlight
(so both control the same backlight, sometimes both work
sometimes only 1 works).

Userspace uses the backlight-type to determine which backlight
class to use, using (for GNOME, but I believe everywhere) the
following preference order:

1. First look for "firmware" type backlight devices (like acpi_video0)
2. Then try "platform" type backlight devices
3. Last try "raw" type backlight devices

And to make things work the kernel has been hiding the "acpi_video0"
entry in cases where it is known that we need the "raw" aka native
type backlight.

Luke you seem to already be partly aware of this, because the patch
now has this:

	props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked */

but almost all modern laptops exclusively use the raw/native type
for backlight control of the main LCD panel.

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.

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".

This is done for hybrid-gfx laptops where there already may
be 2 raw backlight-class devices, 1 for each GPU but only
1 of the 2 drm_connectors going to the main LCD panel should
actually show as enabled.

So typing all this out I guess we could go ahead with using
the backlight class for this after all, but this relies
on userspace preferring raw backlight-class devices
with a drm_connector-object parent which show as being
enabled.

Any userspace code which does not do the parent has
an enabled attr reading "enabled" or a similar check
will end up picking a random backlight class device
as control for the main panel brightness which will not
always end well. So this all is a bit fragile ...

And I'm not sure what is the best thing to do here.

Barnabás, Ilpo, Guenter, any comments on this ?

Regards,

Hans


p.s.

Note I'm working on allowing brightness control for
multiple screens in a sane way, see:

https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/

The last few kernel-cycles I have landed a refactor/cleanup
of the existing backlight code so that we only ever
register 1 /sys/class/backlight entry for the main LCD
panel, instead of having e.g. both acpi_video0 + intel_backlight
and relying on userspace preferring acpi_video0 in that case.

And when I can find time for it I plan to implement
the API in the linked RFC, which allows properly
dealing with all this.

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,

On 5/6/23 13:52, Hans de Goede wrote:
> Hi Luke,
> 
> On 5/5/23 06:30, Luke D. Jones wrote:
>> Adds support for the screenpad(-plus) found on a few ASUS laptops that have a main 16:9 or 16:10 screen and a shorter screen below the main but above the keyboard.
>> The support consists of:
>> - On off control
>> - Setting brightness from 0-255
>>
>> There are some small quirks with this device when considering only the raw WMI methods:
>> 1. The Off method can only switch the device off
>> 2. Changing the brightness turns the device back on
>> 3. To turn the device back on the brightness must be > 1
>> 4. When the device is off the brightness can't be changed (so it is stored by the driver if device is off).
>> 5. Booting with a value of 0 brightness (retained by bios) means the bios will set a value of > 0, < 15 which is far too dim and was unexpected by testers. The compromise was to set the brightness to 60 which is a usable brightness if the module init brightness was under 15.
>> 6. When the device is off it is "unplugged"
>>
>> All of the above points are addressed within the patch to create a good user experience and keep within user expectations.
>>
>> Changelog:
>> - V2
>>   - Complete refactor to use as a backlight device
> 
> 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.
> 
> In the (recent) past the kernel used to register e.g.
> both /sys/class/backlight/acpi_video0 and
> /sys/class/backlight/intel_backlight
> 
> For ACPI resp. direct hw control of the LCD panel backlight
> (so both control the same backlight, sometimes both work
> sometimes only 1 works).
> 
> Userspace uses the backlight-type to determine which backlight
> class to use, using (for GNOME, but I believe everywhere) the
> following preference order:
> 
> 1. First look for "firmware" type backlight devices (like acpi_video0)
> 2. Then try "platform" type backlight devices
> 3. Last try "raw" type backlight devices
> 
> And to make things work the kernel has been hiding the "acpi_video0"
> entry in cases where it is known that we need the "raw" aka native
> type backlight.
> 
> Luke you seem to already be partly aware of this, because the patch
> now has this:
> 
> 	props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked */
> 
> but almost all modern laptops exclusively use the raw/native type
> for backlight control of the main LCD panel.
> 
> 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.
> 
> 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".
> 
> This is done for hybrid-gfx laptops where there already may
> be 2 raw backlight-class devices, 1 for each GPU but only
> 1 of the 2 drm_connectors going to the main LCD panel should
> actually show as enabled.
> 
> So typing all this out I guess we could go ahead with using
> the backlight class for this after all, but this relies
> on userspace preferring raw backlight-class devices
> with a drm_connector-object parent which show as being
> enabled.
> 
> Any userspace code which does not do the parent has
> an enabled attr reading "enabled" or a similar check
> will end up picking a random backlight class device
> as control for the main panel brightness which will not
> always end well. So this all is a bit fragile ...
> 
> And I'm not sure what is the best thing to do here.
> 
> Barnabás, Ilpo, Guenter, any comments on this ?

Hmm, no comments from anyone on the potential problems of using
/sys/class/backlight for this causing potential userspace confusion
since normally /sys/class/backlight devices control the main LCD
brightness ?

Luke do you have any thoughts on this yourself ?

And can you answer this question please ?  :

> 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 ?

Regards,

Hans
  

On Mon, May 15 2023 at 14:39:10 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> Hi,
> 
> On 5/6/23 13:52, Hans de Goede wrote:
>>  Hi Luke,
>> 
>>  On 5/5/23 06:30, Luke D. Jones wrote:
>>>  Adds support for the screenpad(-plus) found on a few ASUS laptops 
>>> that have a main 16:9 or 16:10 screen and a shorter screen below 
>>> the main but above the keyboard.
>>>  The support consists of:
>>>  - On off control
>>>  - Setting brightness from 0-255
>>> 
>>>  There are some small quirks with this device when considering only 
>>> the raw WMI methods:
>>>  1. The Off method can only switch the device off
>>>  2. Changing the brightness turns the device back on
>>>  3. To turn the device back on the brightness must be > 1
>>>  4. When the device is off the brightness can't be changed (so it 
>>> is stored by the driver if device is off).
>>>  5. Booting with a value of 0 brightness (retained by bios) means 
>>> the bios will set a value of > 0, < 15 which is far too dim and was 
>>> unexpected by testers. The compromise was to set the brightness to 
>>> 60 which is a usable brightness if the module init brightness was 
>>> under 15.
>>>  6. When the device is off it is "unplugged"
>>> 
>>>  All of the above points are addressed within the patch to create a 
>>> good user experience and keep within user expectations.
>>> 
>>>  Changelog:
>>>  - V2
>>>    - Complete refactor to use as a backlight device
>> 
>>  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.

>> 
>>  In the (recent) past the kernel used to register e.g.
>>  both /sys/class/backlight/acpi_video0 and
>>  /sys/class/backlight/intel_backlight
>> 
>>  For ACPI resp. direct hw control of the LCD panel backlight
>>  (so both control the same backlight, sometimes both work
>>  sometimes only 1 works).
>> 
>>  Userspace uses the backlight-type to determine which backlight
>>  class to use, using (for GNOME, but I believe everywhere) the
>>  following preference order:
>> 
>>  1. First look for "firmware" type backlight devices (like 
>> acpi_video0)
>>  2. Then try "platform" type backlight devices
>>  3. Last try "raw" type backlight devices
>> 
>>  And to make things work the kernel has been hiding the "acpi_video0"
>>  entry in cases where it is known that we need the "raw" aka native
>>  type backlight.
>> 
>>  Luke you seem to already be partly aware of this, because the patch
>>  now has this:
>> 
>>  	props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked 
>> */
>> 
>>  but almost all modern laptops exclusively use the raw/native type
>>  for backlight control of the main LCD panel.
>> 
>>  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.

>> 
>>  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.

>> 
>>  This is done for hybrid-gfx laptops where there already may
>>  be 2 raw backlight-class devices, 1 for each GPU but only
>>  1 of the 2 drm_connectors going to the main LCD panel should
>>  actually show as enabled.
>> 
>>  So typing all this out I guess we could go ahead with using
>>  the backlight class for this after all, but this relies
>>  on userspace preferring raw backlight-class devices
>>  with a drm_connector-object parent which show as being
>>  enabled.
>> 
>>  Any userspace code which does not do the parent has
>>  an enabled attr reading "enabled" or a similar check
>>  will end up picking a random backlight class device
>>  as control for the main panel brightness which will not
>>  always end well. So this all is a bit fragile ...
>> 
>>  And I'm not sure what is the best thing to do here.
>> 
>>  Barnabás, Ilpo, Guenter, any comments on this ?
> 
> Hmm, no comments from anyone on the potential problems of using
> /sys/class/backlight for this causing potential userspace confusion
> since normally /sys/class/backlight devices control the main LCD
> brightness ?
> 
> Luke do you have any thoughts on this yourself ?
> 
> And can you answer this question please ?  :
> 
>>  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.

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?

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?

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.



UDEV info dumps:

Device {
    initialized: true,
    device_major_minor_number: None,
    system_path: 
"/sys/devices/pci0000:00/0000:00:08.1/0000:09:00.0/backlight/amdgpu_bl1",
    device_path: 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0/backlight/amdgpu_bl1",
    device_node: None,
    subsystem_name: Some(
        "backlight",
    ),
    system_name: "amdgpu_bl1",
    instance_number: Some(
        1,
    ),
    device_type: None,
    driver: None,
    action: None,
    parent: Some(
        Device {
            initialized: true,
            device_major_minor_number: None,
            system_path: 
"/sys/devices/pci0000:00/0000:00:08.1/0000:09:00.0",
            device_path: 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0",
            device_node: None,
            subsystem_name: Some(
                "pci",
            ),
            system_name: "0000:09:00.0",
            instance_number: Some(
                0,
            ),
            device_type: None,
            driver: Some(
                "amdgpu",
            ),
            action: None,
            parent: Some(
                Device {
                    initialized: true,
                    device_major_minor_number: None,
                    system_path: "/sys/devices/pci0000:00/0000:00:08.1",
                    device_path: "/devices/pci0000:00/0000:00:08.1",
                    device_node: None,
                    subsystem_name: Some(
                        "pci",
                    ),
                    system_name: "0000:00:08.1",
                    instance_number: Some(
                        1,
                    ),
                    device_type: None,
                    driver: Some(
                        "pcieport",
                    ),
                    action: None,
                    parent: Some(
                        Device {
                            initialized: false,
                            device_major_minor_number: None,
                            system_path: "/sys/devices/pci0000:00",
                            device_path: "/devices/pci0000:00",
                            device_node: None,
                            subsystem_name: None,
                            system_name: "pci0000:00",
                            instance_number: Some(
                                0,
                            ),
                            device_type: None,
                            driver: None,
                            action: None,
                            parent: None,
                        },
                    ),
                },
            ),
        },
    ),
}
  [properties]
    - "CURRENT_TAGS" ":systemd:seat:"
    - "DEVPATH" 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0/backlight/amdgpu_bl1"
    - "DMI_FAMILY" "ROG Zephyrus Duo 16"
    - "DMI_VENDOR" "ASUSTeK COMPUTER INC."
    - "ID_FOR_SEAT" "backlight-pci-0000_09_00_0"
    - "ID_PATH" "pci-0000:09:00.0"
    - "ID_PATH_TAG" "pci-0000_09_00_0"
    - "NVME_HOST_IFACE" "none"
    - "SUBSYSTEM" "backlight"
    - "SYSTEMD_WANTS" "systemd-backlight@backlight:amdgpu_bl1.service"
    - "TAGS" ":systemd:seat:"
    - "USEC_INITIALIZED" "6189554"
  [attributes]
    - "actual_brightness" ""
    - "bl_power" ""
    - "brightness" ""
    - "device" ""
    - "max_brightness" ""
    - "power/autosuspend_delay_ms" ""
    - "power/control" ""
    - "power/runtime_active_time" ""
    - "power/runtime_status" ""
    - "power/runtime_suspended_time" ""
    - "scale" ""
    - "subsystem" ""
    - "type" ""
    - "uevent" ""

Device {
    initialized: true,
    device_major_minor_number: None,
    system_path: 
"/sys/devices/platform/asus-nb-wmi/backlight/asus_screenpad",
    device_path: 
"/devices/platform/asus-nb-wmi/backlight/asus_screenpad",
    device_node: None,
    subsystem_name: Some(
        "backlight",
    ),
    system_name: "asus_screenpad",
    instance_number: None,
    device_type: None,
    driver: None,
    action: None,
    parent: Some(
        Device {
            initialized: true,
            device_major_minor_number: None,
            system_path: "/sys/devices/platform/asus-nb-wmi",
            device_path: "/devices/platform/asus-nb-wmi",
            device_node: None,
            subsystem_name: Some(
                "platform",
            ),
            system_name: "asus-nb-wmi",
            instance_number: None,
            device_type: None,
            driver: Some(
                "asus-nb-wmi",
            ),
            action: None,
            parent: Some(
                Device {
                    initialized: false,
                    device_major_minor_number: None,
                    system_path: "/sys/devices/platform",
                    device_path: "/devices/platform",
                    device_node: None,
                    subsystem_name: None,
                    system_name: "platform",
                    instance_number: None,
                    device_type: None,
                    driver: None,
                    action: None,
                    parent: None,
                },
            ),
        },
    ),
}
  [properties]
    - "CURRENT_TAGS" ":systemd:seat:"
    - "DEVPATH" "/devices/platform/asus-nb-wmi/backlight/asus_screenpad"
    - "DMI_FAMILY" "ROG Zephyrus Duo 16"
    - "DMI_VENDOR" "ASUSTeK COMPUTER INC."
    - "ID_FOR_SEAT" "backlight-platform-asus-nb-wmi"
    - "ID_PATH" "platform-asus-nb-wmi"
    - "ID_PATH_TAG" "platform-asus-nb-wmi"
    - "NVME_HOST_IFACE" "none"
    - "SUBSYSTEM" "backlight"
    - "SYSTEMD_WANTS" 
"systemd-backlight@backlight:asus_screenpad.service"
    - "TAGS" ":systemd:seat:"
    - "USEC_INITIALIZED" "6323833"
  [attributes]
    - "actual_brightness" ""
    - "bl_power" ""
    - "brightness" ""
    - "device" ""
    - "max_brightness" ""
    - "power/autosuspend_delay_ms" ""
    - "power/control" ""
    - "power/runtime_active_time" ""
    - "power/runtime_status" ""
    - "power/runtime_suspended_time" ""
    - "scale" ""
    - "subsystem" ""
    - "type" ""
    - "uevent" ""


Device {
    initialized: true,
    device_major_minor_number: None,
    system_path: 
"/sys/devices/pci0000:00/0000:00:08.1/0000:09:00.0/drm/card1/card1-DP-1",
    device_path: 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0/drm/card1/card1-DP-1",
    device_node: None,
    subsystem_name: Some(
        "drm",
    ),
    system_name: "card1-DP-1",
    instance_number: Some(
        1,
    ),
    device_type: Some(
        "drm_connector",
    ),
    driver: None,
    action: None,
    parent: Some(
        Device {
            initialized: true,
            device_major_minor_number: Some(
                57857,
            ),
            system_path: 
"/sys/devices/pci0000:00/0000:00:08.1/0000:09:00.0/drm/card1",
            device_path: 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0/drm/card1",
            device_node: Some(
                "/dev/dri/card1",
            ),
            subsystem_name: Some(
                "drm",
            ),
            system_name: "card1",
            instance_number: Some(
                1,
            ),
            device_type: Some(
                "drm_minor",
            ),
            driver: None,
            action: None,
            parent: Some(
                Device {
                    initialized: true,
                    device_major_minor_number: None,
                    system_path: 
"/sys/devices/pci0000:00/0000:00:08.1/0000:09:00.0",
                    device_path: 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0",
                    device_node: None,
                    subsystem_name: Some(
                        "pci",
                    ),
                    system_name: "0000:09:00.0",
                    instance_number: Some(
                        0,
                    ),
                    device_type: None,
                    driver: Some(
                        "amdgpu",
                    ),
                    action: None,
                    parent: Some(
                        Device {
                            initialized: true,
                            device_major_minor_number: None,
                            system_path: 
"/sys/devices/pci0000:00/0000:00:08.1",
                            device_path: 
"/devices/pci0000:00/0000:00:08.1",
                            device_node: None,
                            subsystem_name: Some(
                                "pci",
                            ),
                            system_name: "0000:00:08.1",
                            instance_number: Some(
                                1,
                            ),
                            device_type: None,
                            driver: Some(
                                "pcieport",
                            ),
                            action: None,
                            parent: Some(
                                Device {
                                    initialized: false,
                                    device_major_minor_number: None,
                                    system_path: 
"/sys/devices/pci0000:00",
                                    device_path: "/devices/pci0000:00",
                                    device_node: None,
                                    subsystem_name: None,
                                    system_name: "pci0000:00",
                                    instance_number: Some(
                                        0,
                                    ),
                                    device_type: None,
                                    driver: None,
                                    action: None,
                                    parent: None,
                                },
                            ),
                        },
                    ),
                },
            ),
        },
    ),
}
  [properties]
    - "CURRENT_TAGS" ":master-of-seat:seat:"
    - "DEVPATH" 
"/devices/pci0000:00/0000:00:08.1/0000:09:00.0/drm/card1/card1-DP-1"
    - "DEVTYPE" "drm_connector"
    - "DMI_FAMILY" "ROG Zephyrus Duo 16"
    - "DMI_VENDOR" "ASUSTeK COMPUTER INC."
    - "ID_FOR_SEAT" "drm-pci-0000_09_00_0"
    - "ID_PATH" "pci-0000:09:00.0"
    - "ID_PATH_TAG" "pci-0000_09_00_0"
    - "SUBSYSTEM" "drm"
    - "TAGS" ":master-of-seat:seat:"
    - "USEC_INITIALIZED" "6196005"
  [attributes]
    - "ddc" ""
    - "device" ""
    - "dpms" ""
    - "edid" ""
    - "enabled" ""
    - "modes" ""
    - "power/autosuspend_delay_ms" ""
    - "power/control" ""
    - "power/runtime_active_time" ""
    - "power/runtime_status" ""
    - "power/runtime_suspended_time" ""
    - "status" ""
    - "subsystem" ""
    - "uevent" ""
>
  

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@redhat.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 ?

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

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@shalem ~]$ cat /sys/class/drm/card1-DP-1/status 
connected

Step 2: for the connected ones cat the modes, e.g.:

[hans@shalem ~]$ 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 ?

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?


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.


> 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?

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 ...


> 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 ?

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.

Regards,

Hans
  

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@redhat.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@shalem ~]$ 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@shalem ~]$ 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.
  

Hi Luke,

On 6/4/23 06:52, Luke Jones wrote:
> On Thu, 2023-05-25 at 13:09 +0200, Hans de Goede wrote:

<snip>

>> 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.

Ok, that is good.

>> 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

Ack.

>> 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@shalem ~]$ 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

Ok, that is good.


>> Step 2: for the connected ones cat the modes, e.g.:
>>
>> [hans@shalem ~]$ 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.

I'm afraid that this is not going to be easy to fix at the kernel level,
we first need to tie backlight control to drm-connectors as I proposed
(and plan to implement when I can make time):

https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/

Once that is in place we can simply make the drm-code call out to
the backlight driver and have it turn the backlight off when disabling
the output through the drm/kms interface.

>> 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.

Ok, lets go with the v2 which adds /sys/class/backlight support then.

I must warn you though if this does turn out to cause issues I'll have
no choice but to revert it.

I must admit I've lost track a bit of the state of v2 during this
discussion.  Can I pick up v2 as is, or were there (other) remarks
which need addressing and should I expect a v3 ?

####### Switch to (off-topic) GPU mux discussion ########

>>> 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.

Hmm, weird with EFI grub is just using the EFI text output protocol
so what is happening here at the grub level is actually a bug in
the firmware of the laptop(s)...

>> 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).

Ah ok, so I misunderstood and the problem only happens *after* grub?

Have I understood that correctly?

And this is on Fedora with the nvidia binary driver ?

The problem then likely is that the nvidia binary driver is not in
the initrd (which is by design since it may need to be rebuild on
a driver update while the kernel is kept at the same version,
so the initrd won't be rebuild).

So during the initrd there then is no kms driver to drive the
internal display.

Normally plymouth falls back to using the efifb (through simpledrm)
but that is after a pretty large timeout and I think this may not
be happening here because plymouth maybe thinks there is a kms
capable display connected to the iGPU for some reason
(maybe the extra screenpad panel ?)

As a workaround you can tell plymouth to use the simpledrm kms
device as soon as it becomes available instead of waiting for
a native kms driver. To do this add "plymouth.use-simpledrm" to
the kernel commandline. I think that this will work around
the problem.

> 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.

Ok, so that answers my question from above and I should wait for a v3 :)

Regards,

Hans
  

**snip**
> >> 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.
> 
> I'm afraid that this is not going to be easy to fix at the kernel level,
> we first need to tie backlight control to drm-connectors as I proposed
> (and plan to implement when I can make time):
> 
> https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redha
> t.com/
> 
> Once that is in place we can simply make the drm-code call out to
> the backlight driver and have it turn the backlight off when disabling
> the output through the drm/kms interface.

Okay cool. But until then I can set the screenpad to turn brightness off when 
it does the call to:
err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 0, NULL);
here I can also do before that call:
err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT,ctrl_param, NULL);

Then when the patch you mention is done this can be removed.

**snip** 
> > 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.
> 
> Ok, lets go with the v2 which adds /sys/class/backlight support then.
> 
> I must warn you though if this does turn out to cause issues I'll have
> no choice but to revert it.
> 
> I must admit I've lost track a bit of the state of v2 during this
> discussion.  Can I pick up v2 as is, or were there (other) remarks
> which need addressing and should I expect a v3 ?

There will be a V3. I don't anticipate any issues at all with this, and some 
folks have been using this patch with Gnome and KDE since V2 was submitted.

> 
> ####### Switch to (off-topic) GPU mux discussion ########
**snip**
> >> 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).
> 
> Ah ok, so I misunderstood and the problem only happens *after* grub?
> 
> Have I understood that correctly?
> 
> And this is on Fedora with the nvidia binary driver ?
> 
> The problem then likely is that the nvidia binary driver is not in
> the initrd (which is by design since it may need to be rebuild on
> a driver update while the kernel is kept at the same version,
> so the initrd won't be rebuild).

That was indeed the issue. It also creates new problems for when a user wants 
to use iGPU only via the plain (and frankly not adequate or good) method of 
simply removing the dGPU from the device tree.

Currently I maintain the supergfxd tool which is a lot more advanced than the 
other gpu switchers around, and I expose both the above method, and also PCI 
hotplug, and the ASUS WMI method (which I now think is used by other vendors 
also). Hotplug and Asus method can force the device off, and it can't be 
brought back by a PCI rescan - but to do so safely the Nvidia drivers must be 
unused and unloaded. I guess I'll need to tweak the boot process of supergfxd 
and block things until this is done.

Maybe we can move this to a new topic, because there looks to be a few things 
to discuss in relation to hybrid laptops, and specifically Nvidia with MUX, 
and Advanced Optimus.

Cheers,
Luke.
  

Hi,

On 6/7/23 00:45, Luke Jones wrote:

<snip>

>> I must admit I've lost track a bit of the state of v2 during this
>> discussion.  Can I pick up v2 as is, or were there (other) remarks
>> which need addressing and should I expect a v3 ?
> 
> There will be a V3. I don't anticipate any issues at all with this, and some 
> folks have been using this patch with Gnome and KDE since V2 was submitted.

Ok.


>> ####### Switch to (off-topic) GPU mux discussion ########
> **snip**
>>>> 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).
>>
>> Ah ok, so I misunderstood and the problem only happens *after* grub?
>>
>> Have I understood that correctly?
>>
>> And this is on Fedora with the nvidia binary driver ?
>>
>> The problem then likely is that the nvidia binary driver is not in
>> the initrd (which is by design since it may need to be rebuild on
>> a driver update while the kernel is kept at the same version,
>> so the initrd won't be rebuild).
> 
> That was indeed the issue. It also creates new problems for when a user wants 
> to use iGPU only via the plain (and frankly not adequate or good) method of 
> simply removing the dGPU from the device tree.
> 
> Currently I maintain the supergfxd tool which is a lot more advanced than the 
> other gpu switchers around, and I expose both the above method, and also PCI 
> hotplug, and the ASUS WMI method (which I now think is used by other vendors 
> also). Hotplug and Asus method can force the device off, and it can't be 
> brought back by a PCI rescan - but to do so safely the Nvidia drivers must be 
> unused and unloaded. I guess I'll need to tweak the boot process of supergfxd 
> and block things until this is done.
> 
> Maybe we can move this to a new topic, because there looks to be a few things 
> to discuss in relation to hybrid laptops, and specifically Nvidia with MUX, 
> and Advanced Optimus.

Yes if you want to discuss this further please start a new mailinglist
thread for this and also please put the dri-devel list on the Cc:

dri-devel@lists.freedesktop.org

Regards,

Hans