diff mbox series

[1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"

Message ID 20190814213118.28473-2-kherbst@redhat.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Adding a proper workaround for fixing RTD3 issues with Nouveau | expand

Commit Message

Karol Herbst Aug. 14, 2019, 9:31 p.m. UTC
This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.

The original commit message didn't even make sense. AMD _does_ support it and
it works with Nouveau as well.

Also what was the issue being solved here? No references to any bugs and not
even explaining any issue at all isn't the way we do things.

And even if it means a muxed design, then the fix is to make it work inside the
driver, not adding some hacky workaround through ACPI tricks.

And what out of tree drivers do or do not support we don't care one bit anyway.

Signed-off-by: Karol Herbst <kherbst@redhat.com>
CC: Alex Hung <alex.hung@canonical.com>
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Dave Airlie <airlied@redhat.com>
CC: Lyude Paul <lyude@redhat.com>
CC: Ben Skeggs <bskeggs@redhat.com>
---
 drivers/acpi/osi.c | 7 -------
 1 file changed, 7 deletions(-)

Comments

Alex Hung Aug. 14, 2019, 9:49 p.m. UTC | #1
Thanks for the series of fixes. I will check whether these fixes work
on the original intended systems.

On Wed, Aug 14, 2019 at 3:31 PM Karol Herbst <kherbst@redhat.com> wrote:
>
> This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
>
> The original commit message didn't even make sense. AMD _does_ support it and
> it works with Nouveau as well.
>
> Also what was the issue being solved here? No references to any bugs and not
> even explaining any issue at all isn't the way we do things.
>
> And even if it means a muxed design, then the fix is to make it work inside the
> driver, not adding some hacky workaround through ACPI tricks.
>
> And what out of tree drivers do or do not support we don't care one bit anyway.
>
> Signed-off-by: Karol Herbst <kherbst@redhat.com>
> CC: Alex Hung <alex.hung@canonical.com>
> CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> CC: Dave Airlie <airlied@redhat.com>
> CC: Lyude Paul <lyude@redhat.com>
> CC: Ben Skeggs <bskeggs@redhat.com>
> ---
>  drivers/acpi/osi.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index bec0bebc7f52..9b20ac4d79a0 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -61,13 +61,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
>          * a BIOS workaround.
>          */
>         {"Linux-Lenovo-NV-HDMI-Audio", true},
> -       /*
> -        * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
> -        * output video directly to external monitors on HP Inc. mobile
> -        * workstations as Nvidia and AMD VGA drivers provide limited
> -        * hybrid graphics supports.
> -        */
> -       {"Linux-HPI-Hybrid-Graphics", true},
>  };
>
>  static u32 acpi_osi_handler(acpi_string interface, u32 supported)
> --
> 2.21.0
>
Dave Airlie Aug. 14, 2019, 10:47 p.m. UTC | #2
On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
>
> This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
>
> The original commit message didn't even make sense. AMD _does_ support it and
> it works with Nouveau as well.
>
> Also what was the issue being solved here? No references to any bugs and not
> even explaining any issue at all isn't the way we do things.
>
> And even if it means a muxed design, then the fix is to make it work inside the
> driver, not adding some hacky workaround through ACPI tricks.
>
> And what out of tree drivers do or do not support we don't care one bit anyway.
>

I think the reverts should be merged via Rafael's tree as the original
patches went in via there, and we should get them in asap.

Acked-by: Dave Airlie <airlied@redhat.com>
Dave.
Limonciello, Mario Aug. 15, 2019, 1:55 p.m. UTC | #3
> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org <linux-acpi-owner@vger.kernel.org> On
> Behalf Of Dave Airlie
> Sent: Wednesday, August 14, 2019 5:48 PM
> To: Karol Herbst
> Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben
> Skeggs; Dave Airlie
> Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> enable dGPU direct output"
> 
> On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> >
> > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> >
> > The original commit message didn't even make sense. AMD _does_ support it and
> > it works with Nouveau as well.
> >
> > Also what was the issue being solved here? No references to any bugs and not
> > even explaining any issue at all isn't the way we do things.
> >
> > And even if it means a muxed design, then the fix is to make it work inside the
> > driver, not adding some hacky workaround through ACPI tricks.
> >
> > And what out of tree drivers do or do not support we don't care one bit anyway.
> >
> 
> I think the reverts should be merged via Rafael's tree as the original
> patches went in via there, and we should get them in asap.
> 
> Acked-by: Dave Airlie <airlied@redhat.com>
> Dave.

There are definitely going to be regressions on machines in the field with the
in tree drivers by reverting this.  I think we should have an answer for all of those
before this revert is accepted.

Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect
some information on the impact of reverting this.

When this is used on a system with Intel+AMD the ASL configures AMD GPU to use
"Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics"
when on Linux.

I feel we need a knob and/or DMI detection to affect the changes that the ASL
normally performs.
Karol Herbst Aug. 15, 2019, 2:04 p.m. UTC | #4
On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org <linux-acpi-owner@vger.kernel.org> On
> > Behalf Of Dave Airlie
> > Sent: Wednesday, August 14, 2019 5:48 PM
> > To: Karol Herbst
> > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben
> > Skeggs; Dave Airlie
> > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > enable dGPU direct output"
> >
> > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > >
> > > The original commit message didn't even make sense. AMD _does_ support it and
> > > it works with Nouveau as well.
> > >
> > > Also what was the issue being solved here? No references to any bugs and not
> > > even explaining any issue at all isn't the way we do things.
> > >
> > > And even if it means a muxed design, then the fix is to make it work inside the
> > > driver, not adding some hacky workaround through ACPI tricks.
> > >
> > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > >
> >
> > I think the reverts should be merged via Rafael's tree as the original
> > patches went in via there, and we should get them in asap.
> >
> > Acked-by: Dave Airlie <airlied@redhat.com>
> > Dave.
>
> There are definitely going to be regressions on machines in the field with the
> in tree drivers by reverting this.  I think we should have an answer for all of those
> before this revert is accepted.
>
> Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect
> some information on the impact of reverting this.
>
> When this is used on a system with Intel+AMD the ASL configures AMD GPU to use
> "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics"
> when on Linux.

and what's exactly the difference between those? And what's the actual
issue here?

We already have the PRIME offloading in place and if that's not
enough, we should work on extending it, not adding some ACPI based
workarounds, because that's exactly how that looks like.

Also, was this discussed with anybody involved in the drm subsystem?

>
> I feel we need a knob and/or DMI detection to affect the changes that the ASL
> normally performs.

Why do we have to do that on a firmware level at all?
Daniel Vetter Aug. 15, 2019, 2:11 p.m. UTC | #5
On Thu, Aug 15, 2019 at 12:47 AM Dave Airlie <airlied@gmail.com> wrote:
>
> On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> >
> > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> >
> > The original commit message didn't even make sense. AMD _does_ support it and
> > it works with Nouveau as well.
> >
> > Also what was the issue being solved here? No references to any bugs and not
> > even explaining any issue at all isn't the way we do things.
> >
> > And even if it means a muxed design, then the fix is to make it work inside the
> > driver, not adding some hacky workaround through ACPI tricks.
> >
> > And what out of tree drivers do or do not support we don't care one bit anyway.
> >
>
> I think the reverts should be merged via Rafael's tree as the original
> patches went in via there, and we should get them in asap.

+1

> Acked-by: Dave Airlie <airlied@redhat.com>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also fully agreeing with Karol's reply further down, if this doesn't
work we need to improve the drivers, not pile stuff on top in some
ACPI hacks.
-Daniel
Alex Deucher Aug. 15, 2019, 2:13 p.m. UTC | #6
On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> >
> > > -----Original Message-----
> > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-owner@vger.kernel.org> On
> > > Behalf Of Dave Airlie
> > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > To: Karol Herbst
> > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben
> > > Skeggs; Dave Airlie
> > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > > enable dGPU direct output"
> > >
> > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > >
> > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > it works with Nouveau as well.
> > > >
> > > > Also what was the issue being solved here? No references to any bugs and not
> > > > even explaining any issue at all isn't the way we do things.
> > > >
> > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > driver, not adding some hacky workaround through ACPI tricks.
> > > >
> > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > >
> > >
> > > I think the reverts should be merged via Rafael's tree as the original
> > > patches went in via there, and we should get them in asap.
> > >
> > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > Dave.
> >
> > There are definitely going to be regressions on machines in the field with the
> > in tree drivers by reverting this.  I think we should have an answer for all of those
> > before this revert is accepted.
> >
> > Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect
> > some information on the impact of reverting this.
> >
> > When this is used on a system with Intel+AMD the ASL configures AMD GPU to use
> > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics"
> > when on Linux.
>
> and what's exactly the difference between those? And what's the actual
> issue here?

Hybrid Graphics is the new "standard" way of handling these laptops.
It uses the standard _PR3 APCI method to handle dGPU power.  Support
for this was added to Linux relatively later compared to when the
laptops were launched.  "Power Express" used the other AMD specific
ATPX ACPI method to handle dGPU power.  The driver supports both so
either method will work.

Alex

>
> We already have the PRIME offloading in place and if that's not
> enough, we should work on extending it, not adding some ACPI based
> workarounds, because that's exactly how that looks like.
>
> Also, was this discussed with anybody involved in the drm subsystem?
>
> >
> > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > normally performs.
>
> Why do we have to do that on a firmware level at all?
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Karol Herbst Aug. 15, 2019, 2:15 p.m. UTC | #7
On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-owner@vger.kernel.org> On
> > > > Behalf Of Dave Airlie
> > > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > > To: Karol Herbst
> > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben
> > > > Skeggs; Dave Airlie
> > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > > > enable dGPU direct output"
> > > >
> > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > > >
> > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > >
> > > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > > it works with Nouveau as well.
> > > > >
> > > > > Also what was the issue being solved here? No references to any bugs and not
> > > > > even explaining any issue at all isn't the way we do things.
> > > > >
> > > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > >
> > > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > > >
> > > >
> > > > I think the reverts should be merged via Rafael's tree as the original
> > > > patches went in via there, and we should get them in asap.
> > > >
> > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > > Dave.
> > >
> > > There are definitely going to be regressions on machines in the field with the
> > > in tree drivers by reverting this.  I think we should have an answer for all of those
> > > before this revert is accepted.
> > >
> > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect
> > > some information on the impact of reverting this.
> > >
> > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to use
> > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics"
> > > when on Linux.
> >
> > and what's exactly the difference between those? And what's the actual
> > issue here?
>
> Hybrid Graphics is the new "standard" way of handling these laptops.
> It uses the standard _PR3 APCI method to handle dGPU power.  Support
> for this was added to Linux relatively later compared to when the
> laptops were launched.  "Power Express" used the other AMD specific
> ATPX ACPI method to handle dGPU power.  The driver supports both so
> either method will work.
>
> Alex
>

thanks for clarifying. But that still means that we won't need such
workarounds for AMD users, right? amdgpu handles hybrid graphics just
fine, right?

> >
> > We already have the PRIME offloading in place and if that's not
> > enough, we should work on extending it, not adding some ACPI based
> > workarounds, because that's exactly how that looks like.
> >
> > Also, was this discussed with anybody involved in the drm subsystem?
> >
> > >
> > > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > > normally performs.
> >
> > Why do we have to do that on a firmware level at all?
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex Deucher Aug. 15, 2019, 2:17 p.m. UTC | #8
On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-owner@vger.kernel.org> On
> > > > > Behalf Of Dave Airlie
> > > > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > > > To: Karol Herbst
> > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben
> > > > > Skeggs; Dave Airlie
> > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > > > > enable dGPU direct output"
> > > > >
> > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > > > >
> > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > > >
> > > > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > > > it works with Nouveau as well.
> > > > > >
> > > > > > Also what was the issue being solved here? No references to any bugs and not
> > > > > > even explaining any issue at all isn't the way we do things.
> > > > > >
> > > > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > > >
> > > > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > > > >
> > > > >
> > > > > I think the reverts should be merged via Rafael's tree as the original
> > > > > patches went in via there, and we should get them in asap.
> > > > >
> > > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > > > Dave.
> > > >
> > > > There are definitely going to be regressions on machines in the field with the
> > > > in tree drivers by reverting this.  I think we should have an answer for all of those
> > > > before this revert is accepted.
> > > >
> > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect
> > > > some information on the impact of reverting this.
> > > >
> > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to use
> > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics"
> > > > when on Linux.
> > >
> > > and what's exactly the difference between those? And what's the actual
> > > issue here?
> >
> > Hybrid Graphics is the new "standard" way of handling these laptops.
> > It uses the standard _PR3 APCI method to handle dGPU power.  Support
> > for this was added to Linux relatively later compared to when the
> > laptops were launched.  "Power Express" used the other AMD specific
> > ATPX ACPI method to handle dGPU power.  The driver supports both so
> > either method will work.
> >
> > Alex
> >
>
> thanks for clarifying. But that still means that we won't need such
> workarounds for AMD users, right? amdgpu handles hybrid graphics just
> fine, right?

Yeah it should, assuming you have a new enough kernel which supports
HG, which has been several years at this point IIRC.

Alex

>
> > >
> > > We already have the PRIME offloading in place and if that's not
> > > enough, we should work on extending it, not adding some ACPI based
> > > workarounds, because that's exactly how that looks like.
> > >
> > > Also, was this discussed with anybody involved in the drm subsystem?
> > >
> > > >
> > > > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > > > normally performs.
> > >
> > > Why do we have to do that on a firmware level at all?
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Limonciello, Mario Aug. 15, 2019, 2:19 p.m. UTC | #9
> > There are definitely going to be regressions on machines in the field with the
> > in tree drivers by reverting this.  I think we should have an answer for all of
> those
> > before this revert is accepted.
> >
> > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> collect
> > some information on the impact of reverting this.
> >
> > When this is used on a system with Intel+AMD the ASL configures AMD GPU to
> use
> > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> Graphics"
> > when on Linux.
> 
> and what's exactly the difference between those? And what's the actual
> issue here?

DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
events.

> 
> We already have the PRIME offloading in place and if that's not
> enough, we should work on extending it, not adding some ACPI based
> workarounds, because that's exactly how that looks like.
> 
> Also, was this discussed with anybody involved in the drm subsystem?
> 
> >
> > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > normally performs.
> 
> Why do we have to do that on a firmware level at all?

Folks from AMD Graphics team recommended this approach.  From their perspective
it's not a workaround.  They view this as a different architecture for AMD graphics driver on
Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths used for
each.
Limonciello, Mario Aug. 15, 2019, 2:23 p.m. UTC | #10
> On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher <alexdeucher@gmail.com>
> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-
> owner@vger.kernel.org> On
> > > > > > Behalf Of Dave Airlie
> > > > > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > > > > To: Karol Herbst
> > > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung;
> Ben
> > > > > > Skeggs; Dave Airlie
> > > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI
> string to
> > > > > > enable dGPU direct output"
> > > > > >
> > > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com>
> wrote:
> > > > > > >
> > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > > > >
> > > > > > > The original commit message didn't even make sense. AMD _does_
> support it and
> > > > > > > it works with Nouveau as well.
> > > > > > >
> > > > > > > Also what was the issue being solved here? No references to any bugs
> and not
> > > > > > > even explaining any issue at all isn't the way we do things.
> > > > > > >
> > > > > > > And even if it means a muxed design, then the fix is to make it work
> inside the
> > > > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > > > >
> > > > > > > And what out of tree drivers do or do not support we don't care one
> bit anyway.
> > > > > > >
> > > > > >
> > > > > > I think the reverts should be merged via Rafael's tree as the original
> > > > > > patches went in via there, and we should get them in asap.
> > > > > >
> > > > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > > > > Dave.
> > > > >
> > > > > There are definitely going to be regressions on machines in the field with
> the
> > > > > in tree drivers by reverting this.  I think we should have an answer for all
> of those
> > > > > before this revert is accepted.
> > > > >
> > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> collect
> > > > > some information on the impact of reverting this.
> > > > >
> > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> GPU to use
> > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> "Switchable Graphics"
> > > > > when on Linux.
> > > >
> > > > and what's exactly the difference between those? And what's the actual
> > > > issue here?
> > >
> > > Hybrid Graphics is the new "standard" way of handling these laptops.
> > > It uses the standard _PR3 APCI method to handle dGPU power.  Support
> > > for this was added to Linux relatively later compared to when the
> > > laptops were launched.  "Power Express" used the other AMD specific
> > > ATPX ACPI method to handle dGPU power.  The driver supports both so
> > > either method will work.
> > >
> > > Alex
> > >
> >
> > thanks for clarifying. But that still means that we won't need such
> > workarounds for AMD users, right? amdgpu handles hybrid graphics just
> > fine, right?
> 
> Yeah it should, assuming you have a new enough kernel which supports
> HG, which has been several years at this point IIRC.
> 
> Alex
> 

Can you define how new of a kernel is a new enough kernel?

Looking on my side these problems were on new hardware (Precision 7740) and
are checked as recently as start of this summer, w/ kernel 4.15.

We can arrange to have it checked again on 5.3rcX w/ the OSI disabled.
Karol Herbst Aug. 15, 2019, 2:25 p.m. UTC | #11
On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
>
> > > There are definitely going to be regressions on machines in the field with the
> > > in tree drivers by reverting this.  I think we should have an answer for all of
> > those
> > > before this revert is accepted.
> > >
> > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > collect
> > > some information on the impact of reverting this.
> > >
> > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to
> > use
> > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > Graphics"
> > > when on Linux.
> >
> > and what's exactly the difference between those? And what's the actual
> > issue here?
>
> DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> events.
>

afaik Lyude was working on fixing all that, at least for some drivers.
If there is something wrong, we still should fix the drivers, not
adding ACPI workarounds.

Alex: do you know if there are remaining issues regarding that with amdgpu?

> >
> > We already have the PRIME offloading in place and if that's not
> > enough, we should work on extending it, not adding some ACPI based
> > workarounds, because that's exactly how that looks like.
> >
> > Also, was this discussed with anybody involved in the drm subsystem?
> >
> > >
> > > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > > normally performs.
> >
> > Why do we have to do that on a firmware level at all?
>
> Folks from AMD Graphics team recommended this approach.  From their perspective
> it's not a workaround.  They view this as a different architecture for AMD graphics driver on
> Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths used for
> each.

@alex: is this true?
Limonciello, Mario Aug. 15, 2019, 2:34 p.m. UTC | #12
> -----Original Message-----
> From: Karol Herbst <kherbst@redhat.com>
> Sent: Thursday, August 15, 2019 9:25 AM
> To: Limonciello, Mario
> Cc: Dave Airlie; LKML; Linux ACPI Mailing List; dri-devel; nouveau; Rafael J .
> Wysocki; Alex Hung; Ben Skeggs; David Airlie
> Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> enable dGPU direct output"
> 
> 
> [EXTERNAL EMAIL]
> 
> On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> >
> > > > There are definitely going to be regressions on machines in the field with
> the
> > > > in tree drivers by reverting this.  I think we should have an answer for all of
> > > those
> > > > before this revert is accepted.
> > > >
> > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > > collect
> > > > some information on the impact of reverting this.
> > > >
> > > > When this is used on a system with Intel+AMD the ASL configures AMD
> GPU to
> > > use
> > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > > Graphics"
> > > > when on Linux.
> > >
> > > and what's exactly the difference between those? And what's the actual
> > > issue here?
> >
> > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> > events.
> >
> 
> afaik Lyude was working on fixing all that, at least for some drivers.
> If there is something wrong, we still should fix the drivers, not
> adding ACPI workarounds.

I don't disagree, but timing is frequently a limitation if you want the hardware to
work when you put it on the market.

The whole idea behind the OSI string was that it could be reverted when the time
was right.  From this discussion it very well may be for systems with AMD GPU, but
it needs to be checked again.

> 
> Alex: do you know if there are remaining issues regarding that with amdgpu?
> 
> > >
> > > We already have the PRIME offloading in place and if that's not
> > > enough, we should work on extending it, not adding some ACPI based
> > > workarounds, because that's exactly how that looks like.
> > >
> > > Also, was this discussed with anybody involved in the drm subsystem?
> > >
> > > >
> > > > I feel we need a knob and/or DMI detection to affect the changes that the
> ASL
> > > > normally performs.
> > >
> > > Why do we have to do that on a firmware level at all?
> >
> > Folks from AMD Graphics team recommended this approach.  

I should clarify this is from the folks on AMD graphics team that interact to OEMs
like Dell which are not necessarily the same folks who work on the drivers directly.

> From their perspective
> > it's not a workaround.  They view this as a different architecture for AMD
> graphics driver on
> > Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths
> used for
> > each.
> 
> @alex: is this true?
Karol Herbst Aug. 15, 2019, 2:35 p.m. UTC | #13
On Thu, Aug 15, 2019 at 4:30 PM <Mario.Limonciello@dell.com> wrote:
>
> > On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher <alexdeucher@gmail.com>
> > wrote:
> > > >
> > > > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > > >
> > > > > On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-
> > owner@vger.kernel.org> On
> > > > > > > Behalf Of Dave Airlie
> > > > > > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > > > > > To: Karol Herbst
> > > > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung;
> > Ben
> > > > > > > Skeggs; Dave Airlie
> > > > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI
> > string to
> > > > > > > enable dGPU direct output"
> > > > > > >
> > > > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com>
> > wrote:
> > > > > > > >
> > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > > > > >
> > > > > > > > The original commit message didn't even make sense. AMD _does_
> > support it and
> > > > > > > > it works with Nouveau as well.
> > > > > > > >
> > > > > > > > Also what was the issue being solved here? No references to any bugs
> > and not
> > > > > > > > even explaining any issue at all isn't the way we do things.
> > > > > > > >
> > > > > > > > And even if it means a muxed design, then the fix is to make it work
> > inside the
> > > > > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > > > > >
> > > > > > > > And what out of tree drivers do or do not support we don't care one
> > bit anyway.
> > > > > > > >
> > > > > > >
> > > > > > > I think the reverts should be merged via Rafael's tree as the original
> > > > > > > patches went in via there, and we should get them in asap.
> > > > > > >
> > > > > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > > > > > Dave.
> > > > > >
> > > > > > There are definitely going to be regressions on machines in the field with
> > the
> > > > > > in tree drivers by reverting this.  I think we should have an answer for all
> > of those
> > > > > > before this revert is accepted.
> > > > > >
> > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > collect
> > > > > > some information on the impact of reverting this.
> > > > > >
> > > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> > GPU to use
> > > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> > "Switchable Graphics"
> > > > > > when on Linux.
> > > > >
> > > > > and what's exactly the difference between those? And what's the actual
> > > > > issue here?
> > > >
> > > > Hybrid Graphics is the new "standard" way of handling these laptops.
> > > > It uses the standard _PR3 APCI method to handle dGPU power.  Support
> > > > for this was added to Linux relatively later compared to when the
> > > > laptops were launched.  "Power Express" used the other AMD specific
> > > > ATPX ACPI method to handle dGPU power.  The driver supports both so
> > > > either method will work.
> > > >
> > > > Alex
> > > >
> > >
> > > thanks for clarifying. But that still means that we won't need such
> > > workarounds for AMD users, right? amdgpu handles hybrid graphics just
> > > fine, right?
> >
> > Yeah it should, assuming you have a new enough kernel which supports
> > HG, which has been several years at this point IIRC.
> >
> > Alex
> >
>
> Can you define how new of a kernel is a new enough kernel?
>
> Looking on my side these problems were on new hardware (Precision 7740) and
> are checked as recently as start of this summer, w/ kernel 4.15.

That's not even a long term one. And it shouldn't get any fixes. I
just checked, last update was 16 months ago.

>
> We can arrange to have it checked again on 5.3rcX w/ the OSI disabled.

yeah, please do. If there are any issues, we (as in drm developers)
are happy to fix the issues inside the drivers.
Karol Herbst Aug. 15, 2019, 2:36 p.m. UTC | #14
On Thu, Aug 15, 2019 at 4:34 PM <Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: Karol Herbst <kherbst@redhat.com>
> > Sent: Thursday, August 15, 2019 9:25 AM
> > To: Limonciello, Mario
> > Cc: Dave Airlie; LKML; Linux ACPI Mailing List; dri-devel; nouveau; Rafael J .
> > Wysocki; Alex Hung; Ben Skeggs; David Airlie
> > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > enable dGPU direct output"
> >
> >
> > [EXTERNAL EMAIL]
> >
> > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > >
> > > > > There are definitely going to be regressions on machines in the field with
> > the
> > > > > in tree drivers by reverting this.  I think we should have an answer for all of
> > > > those
> > > > > before this revert is accepted.
> > > > >
> > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > > > collect
> > > > > some information on the impact of reverting this.
> > > > >
> > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> > GPU to
> > > > use
> > > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > > > Graphics"
> > > > > when on Linux.
> > > >
> > > > and what's exactly the difference between those? And what's the actual
> > > > issue here?
> > >
> > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> > > events.
> > >
> >
> > afaik Lyude was working on fixing all that, at least for some drivers.
> > If there is something wrong, we still should fix the drivers, not
> > adding ACPI workarounds.
>
> I don't disagree, but timing is frequently a limitation if you want the hardware to
> work when you put it on the market.
>
> The whole idea behind the OSI string was that it could be reverted when the time
> was right.  From this discussion it very well may be for systems with AMD GPU, but
> it needs to be checked again.
>
> >
> > Alex: do you know if there are remaining issues regarding that with amdgpu?
> >
> > > >
> > > > We already have the PRIME offloading in place and if that's not
> > > > enough, we should work on extending it, not adding some ACPI based
> > > > workarounds, because that's exactly how that looks like.
> > > >
> > > > Also, was this discussed with anybody involved in the drm subsystem?
> > > >
> > > > >
> > > > > I feel we need a knob and/or DMI detection to affect the changes that the
> > ASL
> > > > > normally performs.
> > > >
> > > > Why do we have to do that on a firmware level at all?
> > >
> > > Folks from AMD Graphics team recommended this approach.
>
> I should clarify this is from the folks on AMD graphics team that interact to OEMs
> like Dell which are not necessarily the same folks who work on the drivers directly.
>

yeah, I understand the pressure, and it might have been fine if we
would have this discussion upstream, if it's about pushing things
upstream. The commits itself didn't even make the impression that
anybody with the drm drivers involved even looked at those patches and
this is the worst part of those commits.

> > From their perspective
> > > it's not a workaround.  They view this as a different architecture for AMD
> > graphics driver on
> > > Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths
> > used for
> > > each.
> >
> > @alex: is this true?
Alex Deucher Aug. 15, 2019, 2:37 p.m. UTC | #15
On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> >
> > > > There are definitely going to be regressions on machines in the field with the
> > > > in tree drivers by reverting this.  I think we should have an answer for all of
> > > those
> > > > before this revert is accepted.
> > > >
> > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > > collect
> > > > some information on the impact of reverting this.
> > > >
> > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to
> > > use
> > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > > Graphics"
> > > > when on Linux.
> > >
> > > and what's exactly the difference between those? And what's the actual
> > > issue here?
> >
> > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> > events.
> >
>
> afaik Lyude was working on fixing all that, at least for some drivers.
> If there is something wrong, we still should fix the drivers, not
> adding ACPI workarounds.
>
> Alex: do you know if there are remaining issues regarding that with amdgpu?

There was an issue with hpd events not making it to the audio side
when things were powered down that was fixed with this patch set:
https://patchwork.freedesktop.org/patch/316793/
Those patches depended on a bunch of alsa changes as well which may
have not been available in the distro used for a particular OEM
program.

>
> > >
> > > We already have the PRIME offloading in place and if that's not
> > > enough, we should work on extending it, not adding some ACPI based
> > > workarounds, because that's exactly how that looks like.
> > >
> > > Also, was this discussed with anybody involved in the drm subsystem?
> > >
> > > >
> > > > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > > > normally performs.
> > >
> > > Why do we have to do that on a firmware level at all?
> >
> > Folks from AMD Graphics team recommended this approach.  From their perspective
> > it's not a workaround.  They view this as a different architecture for AMD graphics driver on
> > Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths used for
> > each.
>
> @alex: is this true?

I'm not familiar with this patches in particular, but I know we've
done things with OEM programs to support Linux on platforms where
Linux support is lacking for in new features for the target distros.
E.g., when the first hybrid graphics laptops were coming out, Linux
didn't support it too well or at all depending on the timing, so the
bios exposed power express which was working well at the time if the
OS told ACPI it was Linux.

Alex
Alex Deucher Aug. 15, 2019, 2:41 p.m. UTC | #16
On Thu, Aug 15, 2019 at 10:30 AM <Mario.Limonciello@dell.com> wrote:
>
> > On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher <alexdeucher@gmail.com>
> > wrote:
> > > >
> > > > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > > >
> > > > > On Thu, Aug 15, 2019 at 3:56 PM <Mario.Limonciello@dell.com> wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: linux-acpi-owner@vger.kernel.org <linux-acpi-
> > owner@vger.kernel.org> On
> > > > > > > Behalf Of Dave Airlie
> > > > > > > Sent: Wednesday, August 14, 2019 5:48 PM
> > > > > > > To: Karol Herbst
> > > > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung;
> > Ben
> > > > > > > Skeggs; Dave Airlie
> > > > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI
> > string to
> > > > > > > enable dGPU direct output"
> > > > > > >
> > > > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com>
> > wrote:
> > > > > > > >
> > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > > > > >
> > > > > > > > The original commit message didn't even make sense. AMD _does_
> > support it and
> > > > > > > > it works with Nouveau as well.
> > > > > > > >
> > > > > > > > Also what was the issue being solved here? No references to any bugs
> > and not
> > > > > > > > even explaining any issue at all isn't the way we do things.
> > > > > > > >
> > > > > > > > And even if it means a muxed design, then the fix is to make it work
> > inside the
> > > > > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > > > > >
> > > > > > > > And what out of tree drivers do or do not support we don't care one
> > bit anyway.
> > > > > > > >
> > > > > > >
> > > > > > > I think the reverts should be merged via Rafael's tree as the original
> > > > > > > patches went in via there, and we should get them in asap.
> > > > > > >
> > > > > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > > > > > > Dave.
> > > > > >
> > > > > > There are definitely going to be regressions on machines in the field with
> > the
> > > > > > in tree drivers by reverting this.  I think we should have an answer for all
> > of those
> > > > > > before this revert is accepted.
> > > > > >
> > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > collect
> > > > > > some information on the impact of reverting this.
> > > > > >
> > > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> > GPU to use
> > > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> > "Switchable Graphics"
> > > > > > when on Linux.
> > > > >
> > > > > and what's exactly the difference between those? And what's the actual
> > > > > issue here?
> > > >
> > > > Hybrid Graphics is the new "standard" way of handling these laptops.
> > > > It uses the standard _PR3 APCI method to handle dGPU power.  Support
> > > > for this was added to Linux relatively later compared to when the
> > > > laptops were launched.  "Power Express" used the other AMD specific
> > > > ATPX ACPI method to handle dGPU power.  The driver supports both so
> > > > either method will work.
> > > >
> > > > Alex
> > > >
> > >
> > > thanks for clarifying. But that still means that we won't need such
> > > workarounds for AMD users, right? amdgpu handles hybrid graphics just
> > > fine, right?
> >
> > Yeah it should, assuming you have a new enough kernel which supports
> > HG, which has been several years at this point IIRC.
> >
> > Alex
> >
>
> Can you define how new of a kernel is a new enough kernel?
>
> Looking on my side these problems were on new hardware (Precision 7740) and
> are checked as recently as start of this summer, w/ kernel 4.15.

I don't remember off hand.  It was before the _PR3 support in the pci
subsystem was upstream.  What was the problem you were seeing with
this system?

Alex

>
> We can arrange to have it checked again on 5.3rcX w/ the OSI disabled.
Takashi Iwai Aug. 15, 2019, 2:56 p.m. UTC | #17
On Thu, 15 Aug 2019 16:37:05 +0200,
Alex Deucher wrote:
> 
> On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > >
> > > > > There are definitely going to be regressions on machines in the field with the
> > > > > in tree drivers by reverting this.  I think we should have an answer for all of
> > > > those
> > > > > before this revert is accepted.
> > > > >
> > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > > > collect
> > > > > some information on the impact of reverting this.
> > > > >
> > > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to
> > > > use
> > > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > > > Graphics"
> > > > > when on Linux.
> > > >
> > > > and what's exactly the difference between those? And what's the actual
> > > > issue here?
> > >
> > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> > > events.
> > >
> >
> > afaik Lyude was working on fixing all that, at least for some drivers.
> > If there is something wrong, we still should fix the drivers, not
> > adding ACPI workarounds.
> >
> > Alex: do you know if there are remaining issues regarding that with amdgpu?
> 
> There was an issue with hpd events not making it to the audio side
> when things were powered down that was fixed with this patch set:
> https://patchwork.freedesktop.org/patch/316793/
> Those patches depended on a bunch of alsa changes as well which may
> have not been available in the distro used for a particular OEM
> program.

FYI, the corresponding commit for ALSA part is destined for 5.4
kernel: 
  https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade49db337a9d44ac5835cfce1ee873549011b27

BTW, Nouveau should suffer from the same problem.  The patch to add
the audio component support is found at:
  https://patchwork.freedesktop.org/patch/319131/


thanks,

Takashi
Alex Deucher Aug. 15, 2019, 2:59 p.m. UTC | #18
On Thu, Aug 15, 2019 at 10:37 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > >
> > > > > There are definitely going to be regressions on machines in the field with the
> > > > > in tree drivers by reverting this.  I think we should have an answer for all of
> > > > those
> > > > > before this revert is accepted.
> > > > >
> > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to
> > > > collect
> > > > > some information on the impact of reverting this.
> > > > >
> > > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to
> > > > use
> > > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable
> > > > Graphics"
> > > > > when on Linux.
> > > >
> > > > and what's exactly the difference between those? And what's the actual
> > > > issue here?
> > >
> > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing HPD
> > > events.
> > >
> >
> > afaik Lyude was working on fixing all that, at least for some drivers.
> > If there is something wrong, we still should fix the drivers, not
> > adding ACPI workarounds.
> >
> > Alex: do you know if there are remaining issues regarding that with amdgpu?
>
> There was an issue with hpd events not making it to the audio side
> when things were powered down that was fixed with this patch set:
> https://patchwork.freedesktop.org/patch/316793/
> Those patches depended on a bunch of alsa changes as well which may
> have not been available in the distro used for a particular OEM
> program.
>
> >
> > > >
> > > > We already have the PRIME offloading in place and if that's not
> > > > enough, we should work on extending it, not adding some ACPI based
> > > > workarounds, because that's exactly how that looks like.
> > > >
> > > > Also, was this discussed with anybody involved in the drm subsystem?
> > > >
> > > > >
> > > > > I feel we need a knob and/or DMI detection to affect the changes that the ASL
> > > > > normally performs.
> > > >
> > > > Why do we have to do that on a firmware level at all?
> > >
> > > Folks from AMD Graphics team recommended this approach.  From their perspective
> > > it's not a workaround.  They view this as a different architecture for AMD graphics driver on
> > > Windows and AMD graphics w/ amdgpu driver.  They have different ASL paths used for
> > > each.
> >
> > @alex: is this true?
>
> I'm not familiar with this patches in particular, but I know we've
> done things with OEM programs to support Linux on platforms where
> Linux support is lacking for in new features for the target distros.
> E.g., when the first hybrid graphics laptops were coming out, Linux
> didn't support it too well or at all depending on the timing, so the
> bios exposed power express which was working well at the time if the
> OS told ACPI it was Linux.

FWIW, windows does something similar.  I don't think windows 7
supports hybrid graphics either so if the OS tells ACPI it's windows
7, it gets power express instead of hybrid graphics as well.  At least
on laptops that support windows 7 in the first place.

Alex

>
> Alex
Limonciello, Mario Aug. 15, 2019, 4:19 p.m. UTC | #19
> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Thursday, August 15, 2019 9:57 AM
> To: Alex Deucher
> Cc: Karol Herbst; Limonciello, Mario; nouveau; Rafael J . Wysocki; LKML; dri-devel;
> Linux ACPI Mailing List; Alex Hung; Ben Skeggs; David Airlie
> Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> enable dGPU direct output"
> 
> 
> [EXTERNAL EMAIL]
> 
> On Thu, 15 Aug 2019 16:37:05 +0200,
> Alex Deucher wrote:
> >
> > On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > > >
> > > > > > There are definitely going to be regressions on machines in the field
> with the
> > > > > > in tree drivers by reverting this.  I think we should have an answer for all
> of
> > > > > those
> > > > > > before this revert is accepted.
> > > > > >
> > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners
> to
> > > > > collect
> > > > > > some information on the impact of reverting this.
> > > > > >
> > > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> GPU to
> > > > > use
> > > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> "Switchable
> > > > > Graphics"
> > > > > > when on Linux.
> > > > >
> > > > > and what's exactly the difference between those? And what's the actual
> > > > > issue here?
> > > >
> > > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing
> HPD
> > > > events.
> > > >
> > >
> > > afaik Lyude was working on fixing all that, at least for some drivers.
> > > If there is something wrong, we still should fix the drivers, not
> > > adding ACPI workarounds.
> > >
> > > Alex: do you know if there are remaining issues regarding that with amdgpu?
> >
> > There was an issue with hpd events not making it to the audio side
> > when things were powered down that was fixed with this patch set:
> > https://patchwork.freedesktop.org/patch/316793/
> > Those patches depended on a bunch of alsa changes as well which may
> > have not been available in the distro used for a particular OEM
> > program.
> 
> FYI, the corresponding commit for ALSA part is destined for 5.4
> kernel:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade
> 49db337a9d44ac5835cfce1ee873549011b27
> 
> BTW, Nouveau should suffer from the same problem.  The patch to add
> the audio component support is found at:
>   https://patchwork.freedesktop.org/patch/319131/
> 
> 

It sounds like 5.3rcX won't be a useful check then.

So am I correct to understand that everything related to the AMD failures
described in this thread should be in linux-next at this point?
Takashi Iwai Aug. 15, 2019, 5:43 p.m. UTC | #20
On Thu, 15 Aug 2019 18:19:52 +0200,
<Mario.Limonciello@dell.com> wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: Thursday, August 15, 2019 9:57 AM
> > To: Alex Deucher
> > Cc: Karol Herbst; Limonciello, Mario; nouveau; Rafael J . Wysocki; LKML; dri-devel;
> > Linux ACPI Mailing List; Alex Hung; Ben Skeggs; David Airlie
> > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > enable dGPU direct output"
> > 
> > 
> > [EXTERNAL EMAIL]
> > 
> > On Thu, 15 Aug 2019 16:37:05 +0200,
> > Alex Deucher wrote:
> > >
> > > On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > > > >
> > > > > > > There are definitely going to be regressions on machines in the field
> > with the
> > > > > > > in tree drivers by reverting this.  I think we should have an answer for all
> > of
> > > > > > those
> > > > > > > before this revert is accepted.
> > > > > > >
> > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners
> > to
> > > > > > collect
> > > > > > > some information on the impact of reverting this.
> > > > > > >
> > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> > GPU to
> > > > > > use
> > > > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> > "Switchable
> > > > > > Graphics"
> > > > > > > when on Linux.
> > > > > >
> > > > > > and what's exactly the difference between those? And what's the actual
> > > > > > issue here?
> > > > >
> > > > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing
> > HPD
> > > > > events.
> > > > >
> > > >
> > > > afaik Lyude was working on fixing all that, at least for some drivers.
> > > > If there is something wrong, we still should fix the drivers, not
> > > > adding ACPI workarounds.
> > > >
> > > > Alex: do you know if there are remaining issues regarding that with amdgpu?
> > >
> > > There was an issue with hpd events not making it to the audio side
> > > when things were powered down that was fixed with this patch set:
> > > https://patchwork.freedesktop.org/patch/316793/
> > > Those patches depended on a bunch of alsa changes as well which may
> > > have not been available in the distro used for a particular OEM
> > > program.
> > 
> > FYI, the corresponding commit for ALSA part is destined for 5.4
> > kernel:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade
> > 49db337a9d44ac5835cfce1ee873549011b27
> > 
> > BTW, Nouveau should suffer from the same problem.  The patch to add
> > the audio component support is found at:
> >   https://patchwork.freedesktop.org/patch/319131/
> > 
> > 
> 
> It sounds like 5.3rcX won't be a useful check then.

For HDMI/DP audio, right, the main pieces are still missing in 5.3.
But there is no regression in this regard, OTOH.

> So am I correct to understand that everything related to the AMD failures
> described in this thread should be in linux-next at this point?

I'm not sure what you're referring as "AMD failures", so leave this
question to AMD people :)


Takashi
Alex Deucher Aug. 15, 2019, 6:34 p.m. UTC | #21
On Thu, Aug 15, 2019 at 12:19 PM <Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: Thursday, August 15, 2019 9:57 AM
> > To: Alex Deucher
> > Cc: Karol Herbst; Limonciello, Mario; nouveau; Rafael J . Wysocki; LKML; dri-devel;
> > Linux ACPI Mailing List; Alex Hung; Ben Skeggs; David Airlie
> > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to
> > enable dGPU direct output"
> >
> >
> > [EXTERNAL EMAIL]
> >
> > On Thu, 15 Aug 2019 16:37:05 +0200,
> > Alex Deucher wrote:
> > >
> > > On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > On Thu, Aug 15, 2019 at 4:20 PM <Mario.Limonciello@dell.com> wrote:
> > > > >
> > > > > > > There are definitely going to be regressions on machines in the field
> > with the
> > > > > > > in tree drivers by reverting this.  I think we should have an answer for all
> > of
> > > > > > those
> > > > > > > before this revert is accepted.
> > > > > > >
> > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners
> > to
> > > > > > collect
> > > > > > > some information on the impact of reverting this.
> > > > > > >
> > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD
> > GPU to
> > > > > > use
> > > > > > > "Hybrid Graphics" when on Windows and "Power Express" and
> > "Switchable
> > > > > > Graphics"
> > > > > > > when on Linux.
> > > > > >
> > > > > > and what's exactly the difference between those? And what's the actual
> > > > > > issue here?
> > > > >
> > > > > DP/HDMI is not detected unless plugged in at bootup.  It's due to missing
> > HPD
> > > > > events.
> > > > >
> > > >
> > > > afaik Lyude was working on fixing all that, at least for some drivers.
> > > > If there is something wrong, we still should fix the drivers, not
> > > > adding ACPI workarounds.
> > > >
> > > > Alex: do you know if there are remaining issues regarding that with amdgpu?
> > >
> > > There was an issue with hpd events not making it to the audio side
> > > when things were powered down that was fixed with this patch set:
> > > https://patchwork.freedesktop.org/patch/316793/
> > > Those patches depended on a bunch of alsa changes as well which may
> > > have not been available in the distro used for a particular OEM
> > > program.
> >
> > FYI, the corresponding commit for ALSA part is destined for 5.4
> > kernel:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade
> > 49db337a9d44ac5835cfce1ee873549011b27
> >
> > BTW, Nouveau should suffer from the same problem.  The patch to add
> > the audio component support is found at:
> >   https://patchwork.freedesktop.org/patch/319131/
> >
> >
>
> It sounds like 5.3rcX won't be a useful check then.
>
> So am I correct to understand that everything related to the AMD failures
> described in this thread should be in linux-next at this point?
>

Assuming you mean the missing audio hotplug events, then yes.

Alex
Rafael J. Wysocki Aug. 19, 2019, 9:52 a.m. UTC | #22
On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> >
> > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> >
> > The original commit message didn't even make sense. AMD _does_ support it and
> > it works with Nouveau as well.
> >
> > Also what was the issue being solved here? No references to any bugs and not
> > even explaining any issue at all isn't the way we do things.
> >
> > And even if it means a muxed design, then the fix is to make it work inside the
> > driver, not adding some hacky workaround through ACPI tricks.
> >
> > And what out of tree drivers do or do not support we don't care one bit anyway.
> >
> 
> I think the reverts should be merged via Rafael's tree as the original
> patches went in via there, and we should get them in asap.
> 
> Acked-by: Dave Airlie <airlied@redhat.com>

The _OSI strings are to be dropped when all of the needed support is there in
drivers, so they should go away along with the requisite driver changes.

I'm all for dropping then when that's the case, so please feel free to add ACKs
from me to the patches in question at that point.

Cheers,
Rafael
Karol Herbst Sept. 5, 2019, 3:51 p.m. UTC | #23
is there any update on the testing with my patches? On the hardware I
had access to those patches helped, but I can't know if it also helped
on the hardware for which those workarounds where actually added.

On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > >
> > > The original commit message didn't even make sense. AMD _does_ support it and
> > > it works with Nouveau as well.
> > >
> > > Also what was the issue being solved here? No references to any bugs and not
> > > even explaining any issue at all isn't the way we do things.
> > >
> > > And even if it means a muxed design, then the fix is to make it work inside the
> > > driver, not adding some hacky workaround through ACPI tricks.
> > >
> > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > >
> >
> > I think the reverts should be merged via Rafael's tree as the original
> > patches went in via there, and we should get them in asap.
> >
> > Acked-by: Dave Airlie <airlied@redhat.com>
>
> The _OSI strings are to be dropped when all of the needed support is there in
> drivers, so they should go away along with the requisite driver changes.
>

that goes beside the point. firmware level workarounds for GPU driver
issues were pushed without consulting with upstream GPU developers.
That's something which shouldn't have happened in the first place. And
yes, I am personally annoyed by the fact, that people know about
issues, but instead of contacting the proper persons and working on a
proper fix, we end up with stupid firmware level workarounds. I can't
see why we ever would have wanted such workarounds in the first place.

And I would be much happier if the next time something like that comes
up, that the drm mailing list will be contacted as well or somebody
involved.

We could have also just disable the feature inside the driver (and
probably we should have done that a long time ago, so that is
essentially our fault, but still....)

> I'm all for dropping then when that's the case, so please feel free to add ACKs
> from me to the patches in question at that point.
>
> Cheers,
> Rafael
>
>
>
Rafael J. Wysocki Sept. 5, 2019, 4:26 p.m. UTC | #24
On Thursday, September 5, 2019 5:51:23 PM CEST Karol Herbst wrote:
> is there any update on the testing with my patches? On the hardware I
> had access to those patches helped, but I can't know if it also helped
> on the hardware for which those workarounds where actually added.

Alex Hung and Mario need to answer this question I think.

> On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > >
> > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > it works with Nouveau as well.
> > > >
> > > > Also what was the issue being solved here? No references to any bugs and not
> > > > even explaining any issue at all isn't the way we do things.
> > > >
> > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > driver, not adding some hacky workaround through ACPI tricks.
> > > >
> > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > >
> > >
> > > I think the reverts should be merged via Rafael's tree as the original
> > > patches went in via there, and we should get them in asap.
> > >
> > > Acked-by: Dave Airlie <airlied@redhat.com>
> >
> > The _OSI strings are to be dropped when all of the needed support is there in
> > drivers, so they should go away along with the requisite driver changes.
> >
> 
> that goes beside the point. firmware level workarounds for GPU driver
> issues were pushed without consulting with upstream GPU developers.
> That's something which shouldn't have happened in the first place. And
> yes, I am personally annoyed by the fact, that people know about
> issues, but instead of contacting the proper persons and working on a
> proper fix, we end up with stupid firmware level workarounds. I can't
> see why we ever would have wanted such workarounds in the first place.
> 
> And I would be much happier if the next time something like that comes
> up, that the drm mailing list will be contacted as well or somebody
> involved.
> 
> We could have also just disable the feature inside the driver (and
> probably we should have done that a long time ago, so that is
> essentially our fault, but still....)
> 
> > I'm all for dropping then when that's the case, so please feel free to add ACKs
> > from me to the patches in question at that point.
> >
> > Cheers,
> > Rafael
> >
> >
> >
>
Alex Deucher Sept. 5, 2019, 4:35 p.m. UTC | #25
On Thu, Sep 5, 2019 at 11:51 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> is there any update on the testing with my patches? On the hardware I
> had access to those patches helped, but I can't know if it also helped
> on the hardware for which those workarounds where actually added.
>
> On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > >
> > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > it works with Nouveau as well.
> > > >
> > > > Also what was the issue being solved here? No references to any bugs and not
> > > > even explaining any issue at all isn't the way we do things.
> > > >
> > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > driver, not adding some hacky workaround through ACPI tricks.
> > > >
> > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > >
> > >
> > > I think the reverts should be merged via Rafael's tree as the original
> > > patches went in via there, and we should get them in asap.
> > >
> > > Acked-by: Dave Airlie <airlied@redhat.com>
> >
> > The _OSI strings are to be dropped when all of the needed support is there in
> > drivers, so they should go away along with the requisite driver changes.
> >
>
> that goes beside the point. firmware level workarounds for GPU driver
> issues were pushed without consulting with upstream GPU developers.
> That's something which shouldn't have happened in the first place. And
> yes, I am personally annoyed by the fact, that people know about
> issues, but instead of contacting the proper persons and working on a
> proper fix, we end up with stupid firmware level workarounds. I can't
> see why we ever would have wanted such workarounds in the first place.
>
> And I would be much happier if the next time something like that comes
> up, that the drm mailing list will be contacted as well or somebody
> involved.
>
> We could have also just disable the feature inside the driver (and
> probably we should have done that a long time ago, so that is
> essentially our fault, but still....)

Generally these conversations happen between the OEM, the relevant
distro, and hw vendor prior to production so they can't always be
discussed in public.  These programs have power, feature, and distro
targets and not all of those align.  Sometimes fixing this at the
firmware level is the best way to make the product work well at launch
given the state of Linux at a particular time.  Windows already does
similar stuff so that older versions of windows will work properly on
newer hardware.  I agree that we should all strive to fix stuff
properly, but that's not always possible.

Alex

>
> > I'm all for dropping then when that's the case, so please feel free to add ACKs
> > from me to the patches in question at that point.
> >
> > Cheers,
> > Rafael
> >
> >
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex Hung Sept. 9, 2019, 9:07 p.m. UTC | #26
On Thu, Sep 5, 2019 at 5:26 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Thursday, September 5, 2019 5:51:23 PM CEST Karol Herbst wrote:
> > is there any update on the testing with my patches? On the hardware I
> > had access to those patches helped, but I can't know if it also helped
> > on the hardware for which those workarounds where actually added.
>
> Alex Hung and Mario need to answer this question I think.

Sorry for taking a long time. I don't have full testing results yet
but we found at least a regression occurred with _OSI string removed -
it is not on nVidia hardware but on AMD PX one.

I will try to collect and share more details.

>
> > On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > >
> > > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > > >
> > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > >
> > > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > > it works with Nouveau as well.
> > > > >
> > > > > Also what was the issue being solved here? No references to any bugs and not
> > > > > even explaining any issue at all isn't the way we do things.
> > > > >
> > > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > >
> > > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > > >
> > > >
> > > > I think the reverts should be merged via Rafael's tree as the original
> > > > patches went in via there, and we should get them in asap.
> > > >
> > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > >
> > > The _OSI strings are to be dropped when all of the needed support is there in
> > > drivers, so they should go away along with the requisite driver changes.
> > >
> >
> > that goes beside the point. firmware level workarounds for GPU driver
> > issues were pushed without consulting with upstream GPU developers.
> > That's something which shouldn't have happened in the first place. And
> > yes, I am personally annoyed by the fact, that people know about
> > issues, but instead of contacting the proper persons and working on a
> > proper fix, we end up with stupid firmware level workarounds. I can't
> > see why we ever would have wanted such workarounds in the first place.
> >
> > And I would be much happier if the next time something like that comes
> > up, that the drm mailing list will be contacted as well or somebody
> > involved.
> >
> > We could have also just disable the feature inside the driver (and
> > probably we should have done that a long time ago, so that is
> > essentially our fault, but still....)
> >
> > > I'm all for dropping then when that's the case, so please feel free to add ACKs
> > > from me to the patches in question at that point.
> > >
> > > Cheers,
> > > Rafael
> > >
> > >
> > >
> >
>
>
>
>
Alex Hung Oct. 21, 2019, 2:14 a.m. UTC | #27
We have done some tests on three of Intel + nVidia configuration
systems with OEM _OSI strings removed - while some bugs are still
observed, ex. one out of three has suspend/resume issues, no system
crashes were observed - the biggest issue that worries us.

The positive results give us confident to ack the removal of the OEM
_OSI strings. While our tests were not able to cover all possible I+N
systems, we are sure we can fix issues along the way. If there aren't
systems that cannot be fixed without these OEM _OSI strings, these
strings should probably enable with DMI quirks (possible future
patches) so they won't affect others.

Acked-by: Alex Hung <alex.hung@canonical.com>




On Thu, Sep 5, 2019 at 10:26 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Thursday, September 5, 2019 5:51:23 PM CEST Karol Herbst wrote:
> > is there any update on the testing with my patches? On the hardware I
> > had access to those patches helped, but I can't know if it also helped
> > on the hardware for which those workarounds where actually added.
>
> Alex Hung and Mario need to answer this question I think.
>
> > On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > >
> > > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote:
> > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst <kherbst@redhat.com> wrote:
> > > > >
> > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
> > > > >
> > > > > The original commit message didn't even make sense. AMD _does_ support it and
> > > > > it works with Nouveau as well.
> > > > >
> > > > > Also what was the issue being solved here? No references to any bugs and not
> > > > > even explaining any issue at all isn't the way we do things.
> > > > >
> > > > > And even if it means a muxed design, then the fix is to make it work inside the
> > > > > driver, not adding some hacky workaround through ACPI tricks.
> > > > >
> > > > > And what out of tree drivers do or do not support we don't care one bit anyway.
> > > > >
> > > >
> > > > I think the reverts should be merged via Rafael's tree as the original
> > > > patches went in via there, and we should get them in asap.
> > > >
> > > > Acked-by: Dave Airlie <airlied@redhat.com>
> > >
> > > The _OSI strings are to be dropped when all of the needed support is there in
> > > drivers, so they should go away along with the requisite driver changes.
> > >
> >
> > that goes beside the point. firmware level workarounds for GPU driver
> > issues were pushed without consulting with upstream GPU developers.
> > That's something which shouldn't have happened in the first place. And
> > yes, I am personally annoyed by the fact, that people know about
> > issues, but instead of contacting the proper persons and working on a
> > proper fix, we end up with stupid firmware level workarounds. I can't
> > see why we ever would have wanted such workarounds in the first place.
> >
> > And I would be much happier if the next time something like that comes
> > up, that the drm mailing list will be contacted as well or somebody
> > involved.
> >
> > We could have also just disable the feature inside the driver (and
> > probably we should have done that a long time ago, so that is
> > essentially our fault, but still....)
> >
> > > I'm all for dropping then when that's the case, so please feel free to add ACKs
> > > from me to the patches in question at that point.
> > >
> > > Cheers,
> > > Rafael
> > >
> > >
> > >
> >
>
>
>
>
Rafael J. Wysocki Oct. 21, 2019, 8:14 a.m. UTC | #28
On Mon, Oct 21, 2019 at 4:14 AM Alex Hung <alex.hung@canonical.com> wrote:
>
> We have done some tests on three of Intel + nVidia configuration
> systems with OEM _OSI strings removed - while some bugs are still
> observed, ex. one out of three has suspend/resume issues, no system
> crashes were observed - the biggest issue that worries us.
>
> The positive results give us confident to ack the removal of the OEM
> _OSI strings. While our tests were not able to cover all possible I+N
> systems, we are sure we can fix issues along the way. If there aren't
> systems that cannot be fixed without these OEM _OSI strings, these
> strings should probably enable with DMI quirks (possible future
> patches) so they won't affect others.
>
> Acked-by: Alex Hung <alex.hung@canonical.com>

OK, thanks!

I can queue this up or if it's better to route it through the DRM
tree, please do that (and let me know).
Karol Herbst Oct. 21, 2019, 8:48 a.m. UTC | #29
fyi: I decided to go for a different workaround to fix the runpm
issues observed with nvidia gpus with nouveau in the "pci: prevent
putting nvidia GPUs into lower device states on certain intel bridges"
thread

that's on the pci and pm mailing list. Maybe it makes sense to wait
for that to land before actually removing the ACPI workarounds here?
The workaround I had in this series didn't seem to be reliable enough,
so I ditched that approached.

On Mon, Oct 21, 2019 at 10:14 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Oct 21, 2019 at 4:14 AM Alex Hung <alex.hung@canonical.com> wrote:
> >
> > We have done some tests on three of Intel + nVidia configuration
> > systems with OEM _OSI strings removed - while some bugs are still
> > observed, ex. one out of three has suspend/resume issues, no system
> > crashes were observed - the biggest issue that worries us.
> >
> > The positive results give us confident to ack the removal of the OEM
> > _OSI strings. While our tests were not able to cover all possible I+N
> > systems, we are sure we can fix issues along the way. If there aren't
> > systems that cannot be fixed without these OEM _OSI strings, these
> > strings should probably enable with DMI quirks (possible future
> > patches) so they won't affect others.
> >
> > Acked-by: Alex Hung <alex.hung@canonical.com>
>
> OK, thanks!
>
> I can queue this up or if it's better to route it through the DRM
> tree, please do that (and let me know).
Rafael J. Wysocki Oct. 21, 2019, 8:54 a.m. UTC | #30
On Mon, Oct 21, 2019 at 10:48 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> fyi: I decided to go for a different workaround to fix the runpm
> issues observed with nvidia gpus with nouveau in the "pci: prevent
> putting nvidia GPUs into lower device states on certain intel bridges"
> thread

OK, I've seen that.

> that's on the pci and pm mailing list. Maybe it makes sense to wait
> for that to land before actually removing the ACPI workarounds here?

Sounds reasonable.

> The workaround I had in this series didn't seem to be reliable enough,
> so I ditched that approached.

OK, please let me know when the _OSI string in question can be dropped.

> On Mon, Oct 21, 2019 at 10:14 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Mon, Oct 21, 2019 at 4:14 AM Alex Hung <alex.hung@canonical.com> wrote:
> > >
> > > We have done some tests on three of Intel + nVidia configuration
> > > systems with OEM _OSI strings removed - while some bugs are still
> > > observed, ex. one out of three has suspend/resume issues, no system
> > > crashes were observed - the biggest issue that worries us.
> > >
> > > The positive results give us confident to ack the removal of the OEM
> > > _OSI strings. While our tests were not able to cover all possible I+N
> > > systems, we are sure we can fix issues along the way. If there aren't
> > > systems that cannot be fixed without these OEM _OSI strings, these
> > > strings should probably enable with DMI quirks (possible future
> > > patches) so they won't affect others.
> > >
> > > Acked-by: Alex Hung <alex.hung@canonical.com>
> >
> > OK, thanks!
> >
> > I can queue this up or if it's better to route it through the DRM
> > tree, please do that (and let me know).
diff mbox series

Patch

diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index bec0bebc7f52..9b20ac4d79a0 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -61,13 +61,6 @@  osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
 	 * a BIOS workaround.
 	 */
 	{"Linux-Lenovo-NV-HDMI-Audio", true},
-	/*
-	 * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
-	 * output video directly to external monitors on HP Inc. mobile
-	 * workstations as Nvidia and AMD VGA drivers provide limited
-	 * hybrid graphics supports.
-	 */
-	{"Linux-HPI-Hybrid-Graphics", true},
 };
 
 static u32 acpi_osi_handler(acpi_string interface, u32 supported)