AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
@ 2021-01-06 15:58 Hans de Goede
  2021-01-06 17:07 ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-01-06 15:58 UTC (permalink / raw)
  To: amd-gfx; +Cc: Michel Dänzer

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.

Anyways I'm hoping you all have some ideas. If necessary I can build
a Fedora test-kernel with some patches for the reporter to test.

Regards,

Hans

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 15:58 radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should Hans de Goede
@ 2021-01-06 17:07 ` Alex Deucher
  2021-01-06 18:10   ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2021-01-06 17:07 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Michel Dänzer, amd-gfx list

On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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.

Alex


> Anyways I'm hoping you all have some ideas. If necessary I can build
> a Fedora test-kernel with some patches for the reporter to test.
>
> Regards,
>
> Hans
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 17:07 ` Alex Deucher
@ 2021-01-06 18:10   ` Hans de Goede
  2021-01-06 19:33     ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-01-06 18:10 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Michel Dänzer, amd-gfx list

Hi,

On 1/6/21 6:07 PM, Alex Deucher wrote:
> On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 18:10   ` Hans de Goede
@ 2021-01-06 19:33     ` Alex Deucher
  2021-01-06 20:04       ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2021-01-06 19:33 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Michel Dänzer, amd-gfx list

On Wed, Jan 6, 2021 at 1:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 1/6/21 6:07 PM, Alex Deucher wrote:
> > On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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.

That's not the case.  For PX/HG systems, ATIF is in the iGPU's
namespace, on dGPU only systems, ATIF is in the dGPU's namespace.

Alex

>
> 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
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 19:33     ` Alex Deucher
@ 2021-01-06 20:04       ` Hans de Goede
  2021-01-06 20:38         ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-01-06 20:04 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Michel Dänzer, amd-gfx list

Hi,

On 1/6/21 8:33 PM, Alex Deucher wrote:
> On Wed, Jan 6, 2021 at 1:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 1/6/21 6:07 PM, Alex Deucher wrote:
>>> On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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.
> 
> That's not the case.  For PX/HG systems, ATIF is in the iGPU's
> namespace, on dGPU only systems, ATIF is in the dGPU's namespace.

That assumes and AMD iGPU + AMD dGPU I believe ?  The system on
which the spurious ACPI_VIDEO_NOTIFY_PROBE events lead to spurious
KEY_SWITCHVIDEOMODE key-presses being reported uses an Intel iGPU
with an AMD dGPU. I don't have any hybrid gfx systems available for
testing atm, but I believe that in this case there will be 2 ACPI
video-busses, one for each GPU.

Note I'm not saying that that means that checking the originating
ACPI device is the companion of the GPUs PCI-device is the solution
here. But so far all I've heard from you is that that is not the
solution, without you offering any alternative ideas / possible
solutions to try for filtering out these spurious key-presses.

Regards,

Hans

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 20:04       ` Hans de Goede
@ 2021-01-06 20:38         ` Alex Deucher
  2021-01-06 22:32           ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2021-01-06 20:38 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Michel Dänzer, amd-gfx list

On Wed, Jan 6, 2021 at 3:04 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 1/6/21 8:33 PM, Alex Deucher wrote:
> > On Wed, Jan 6, 2021 at 1:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 1/6/21 6:07 PM, Alex Deucher wrote:
> >>> On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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.
> >
> > That's not the case.  For PX/HG systems, ATIF is in the iGPU's
> > namespace, on dGPU only systems, ATIF is in the dGPU's namespace.
>
> That assumes and AMD iGPU + AMD dGPU I believe ?  The system on
> which the spurious ACPI_VIDEO_NOTIFY_PROBE events lead to spurious
> KEY_SWITCHVIDEOMODE key-presses being reported uses an Intel iGPU
> with an AMD dGPU. I don't have any hybrid gfx systems available for
> testing atm, but I believe that in this case there will be 2 ACPI
> video-busses, one for each GPU.

I think the ATIF method will be on the iGPU regardless of whether it's
intel or AMD.

>
> Note I'm not saying that that means that checking the originating
> ACPI device is the companion of the GPUs PCI-device is the solution
> here. But so far all I've heard from you is that that is not the
> solution, without you offering any alternative ideas / possible
> solutions to try for filtering out these spurious key-presses.

Sorry, I'm not really an ACPI expert.  I think returning NOTIFY_BAD is
fine for this specific case, but I don't know if it will break other
platforms.  That said, I don't recall seeing any other similar bugs,
so maybe this is something specific to this particular laptop.

Alex
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should
  2021-01-06 20:38         ` Alex Deucher
@ 2021-01-06 22:32           ` Hans de Goede
  0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2021-01-06 22:32 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Michel Dänzer, amd-gfx list

Hi,

On 1/6/21 9:38 PM, Alex Deucher wrote:
> On Wed, Jan 6, 2021 at 3:04 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 1/6/21 8:33 PM, Alex Deucher wrote:
>>> On Wed, Jan 6, 2021 at 1:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 1/6/21 6:07 PM, Alex Deucher wrote:
>>>>> On Wed, Jan 6, 2021 at 11:25 AM Hans de Goede <hdegoede@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.
>>>
>>> That's not the case.  For PX/HG systems, ATIF is in the iGPU's
>>> namespace, on dGPU only systems, ATIF is in the dGPU's namespace.
>>
>> That assumes and AMD iGPU + AMD dGPU I believe ?  The system on
>> which the spurious ACPI_VIDEO_NOTIFY_PROBE events lead to spurious
>> KEY_SWITCHVIDEOMODE key-presses being reported uses an Intel iGPU
>> with an AMD dGPU. I don't have any hybrid gfx systems available for
>> testing atm, but I believe that in this case there will be 2 ACPI
>> video-busses, one for each GPU.
> 
> I think the ATIF method will be on the iGPU regardless of whether it's
> intel or AMD.

Ok.

>> Note I'm not saying that that means that checking the originating
>> ACPI device is the companion of the GPUs PCI-device is the solution
>> here. But so far all I've heard from you is that that is not the
>> solution, without you offering any alternative ideas / possible
>> solutions to try for filtering out these spurious key-presses.
> 
> Sorry, I'm not really an ACPI expert.  I think returning NOTIFY_BAD is
> fine for this specific case, but I don't know if it will break other
> platforms.

Yes, I'm worried too that it might break other platforms, so that
option is of the table then.

> That said, I don't recall seeing any other similar bugs,
> so maybe this is something specific to this particular laptop.

Ok, the acpi_video.c code already has the option to suppress
key-press reporting based on either a cmdline option, or
a DMI quirk and the reporter of the issue has already confirmed that
the kernel cmdline option works around this. So I will submit a patch
for acpi_video.c to add a DMI quirk for this then. This seems more
of a workaround then a real solution, but it looks like this is
the best which we can do atm.

Regards,

Hans

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-01-07  3:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-06 15:58 radeon kernel driver not suppressing ACPI_VIDEO_NOTIFY_PROBE events when it should Hans de Goede
2021-01-06 17:07 ` Alex Deucher
2021-01-06 18:10   ` Hans de Goede
2021-01-06 19:33     ` Alex Deucher
2021-01-06 20:04       ` Hans de Goede
2021-01-06 20:38         ` Alex Deucher
2021-01-06 22:32           ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox