radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should

[Hans de Goede]

Hi,

On 1/6/21 6:07 PM, Alex Deucher wrote:
> On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede at redhat.com> wrote:
>>
>> Hi All,
>>
>> I get Cc-ed on all Fedora kernel bugs and this one stood out to me:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1911763
>>
>> Since I've done a lot of work on the acpi-video code I thought I should
>> take a look. I've managed to help the user with a kernel-commandline
>> option which stops video.ko (the acpi-video kernel module) from emitting
>> key-press events for ACPI_VIDEO_NOTIFY_PROBE events.
>>
>> This is on a Dell Vostro laptop with i915/radeon hybrid gfx.
>>
>> I was thinking about adding a DMI quirk for this, but from the brief time
>> that I worked on nouveau (and specifically hybrid gfx setups) I know that
>> these events get fired on hybrid gfx setups when the discrete GPU is
>> powered down and something happens which requires the discrete GPUs drivers
>> attention, like an external monitor being plugged into a connector handled
>> by the dGPU (note that is not the case here).
>>
>> So I took a quick look at the radeon code and the radeon_atif_handler()
>> function from drivers/gpu/drm/radeon/radeon_acpi.c. When successful that
>> returns NOTIFY_BAD which suppresses the key-press.
>>
>> But in various cases it returns NOTIFY_DONE instead which does not
>> suppress the key-press event. So I think that the spurious key-press events
>> which the user is seeing should be avoided by this function returning
>> NOTIFY_BAD.
>>
>> Specifically I'm wondering if we should not return
>> NOTIFY_BAD when count == 0?   I guess this can cause problems if there
>> are multiple GPUs, but we could check if the acpi-event is for the
>> pci-device the radeon driver is bound to. This would require changing the
>> acpi-notify code to also pass the acpi_device pointer as part of the
>> acpi_bus_event but that should not be a problem.
>>
> 
> For A+A PX/HG systems, we'd want the notifications for both the dGPU
> and the APU since some of the events are relevant to one or the other.
> ATIF_DGPU_DISPLAY_EVENT is only relevant to the dGPU, while
> ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST would be possibly relevant to
> both (if there was a mux), but mainly the APU.
> ATIF_SYSTEM_POWER_SOURCE_CHANGE_REQUEST would be relevant to both.
> The other events have extended bits to determine which GPU the event
> is targeted at.

Right, but AFAIK on hybrid systems there are 2 ACPI video-bus devices,
one for each of the iGPU and dGPU which is why I suggested passing 
the video-bus acpi_device as extra data in acpi_bus_event and then
radeon_atif_handler() could check if the acpi_device is the companion
device of the GPU. This assumes that events for GPU# will also
originate from (through an ACPI ASL notify call) the ACPI video-bus
which belongs to that GPU.

This all assumes though that the problem is that radeon_atif_handler() 
does not return NOTIFY_BAD when the event count being 0 (in other words
a spurious event). It is also possibly that one of the earlier checks in
radeon_atif_handler() is failing...

I guess that a first step in debugging this would be to ask the reporter
to run a kernel with some debugging printk-s added to radeon_atif_handler(),
to see which code-path in radeon_atif_handler we end up in
(assuming that radeon_atif_handler() gets called at all).

Any suggestions for other debugging printk-s, before I prepare a Fedora
kernel for the reporter to test?

Regards,

Hans