On 12/7/2022 15:04, Hans de Goede wrote:
Hi All,
Mario, thank you for working on this.
Sure
<snip>
Note that the problem of the creating a non functional acpi_video0
device happened before the overhaul too.
The difference is that now we have the in kernel GPU drivers
all call acpi_video_register_backlight() when they detect an
internal-panel for which they don't provide native-backlight
control themselves (to avoid the acpi_video0 backlight registering
before the native backlight gets a chance to register).
The timeout is only there in case no drivers ever call
acpi_video_register_backlight(). nomodeset is one case, but
loosing backlight control in the nomodeset case would be fine
IMHO. The bigger worry why we have the timeout is because of
the nvidia binary driver, for devices which use that driver +
rely on apci_video# for backlight control.
Back to the issue at hand of the unwanted (non functional)
apci_video# on various AMD APU using desktops.
Thanks for explaining.
The native drivers now all calling acpi_video_register_backlight()
gives us a chance to actually do something about it, so in that
sense the 6.1 backlight refactor is relevant.
To avoid this situation from happening add support for video drivers
to notify the ACPI video detection code that no panel was detected.
To reduce the risk of regressions on multi-GPU systems:
* only use this logic when the system is reported as a desktop enclosure.
* in the amdgpu code only report into this for APUs.
I'm afraid that there still is a potential issue for dual
GPU machines. The chassistype is not 100% reliable.
Have you ever seen an A+N machine with unreliable chassis type?
Given Windows HLK certification and knowing that these are to
be based off reference BIOS of laptops, I would be really surprised
if this was wrong on an A+N laptop.
Lets say we have a machine with the wrong chassis-type with
an AMD APU + nvidia GPU which relies on acpi_video0 for
backlight control.
Then if the LCD is connected to the nvidia GPU only, the
amdgpu code will call the new acpi_video_report_nolcd()
function.
+ Dan Dadap
Dan - the context is this series:
https://patchwork.freedesktop.org/series/111745/
Do you know if this is real or just conceptual?
And then even if the nvidia binary driver is patched
by nvidia to call the new acpi_video_register_backlight()
when it does see a panel, then acpi_video_should_register_backlight()
will still return false.
Basically the problem is that we only want to not try
and register the acpi_video0 backlight on dual GPU
machines if the output detection on *both* GPUs has not
found any builtin LCD panel.
But this series disables acpi_video0 backlight registration
as soon as *one* of the *two* GPUs has not found an internal
LCD panel.
As discussed above, after the backlight refactor,
GPU(KMS) drivers are expected to call
acpi_video_register_backlight() when necessary for any
internal panels connected to the GPU they are driving.
This mostly fixes the issue of having an acpi_video0 on
desktop APUs, except that the timeout thingie which was
added to avoid regressions still causes the acpi_video0
backlight to get registered.
Note that this timeout is already configurable through
the register_backlight_delay module option; and setting
that option to 0 disables the timeout based fallback
completely.
So another fix for this might be to just change the
default value of register_backlight_delay to 0 for
kernel 6.2 . This is a change which I want to make
eventually anyways; so we might just as well do this
now to fix the spurious acpi_video0 on desktop APUs
issue. And if this does cause issues for nvidia
binary driver users, they can easily work around this
by setting the module option.
Or alternatively we could go with this series,
reworked so that calling acpi_video_report_nolcd()
only cancels the timeout. This way drivers for another
GPU can still get the acpi_video0 if necessary by
explicitly calling acpi_video_register_backlight().
Personally I have a small preference for just changing
the default of register_backlight_delay to 0, disabling
the timeout based fallback starting with 6.2 .
How about we do both? Then you can always restore
register_backlight_delay without risk of introducing
regression of acpi_video0 coming back to desktop APU's.
I did not do this for 6.1 because there were already
many other backlight changes in 6.1, so I wanted to
have the fallback behavior there as a safeguard
against things not working as planned.
If you're open to my idea of both since I'm already
touching all this anyway I am fine to roll that change
into another patch for default of 0 too in a v2.
