First three patches are removing ACPI workarounds which should have never landed.
The last four are adding a workaround to nouveau which seem to help quite a lot with the RTD3 issues with Nouveau, so let's discuss and get wider testing of those and see if there is any fallout or laptops where the issues don't get fixed.
Karol Herbst (7): Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output" Revert "ACPI / OSI: Add OEM _OSI string to enable NVidia HDMI audio" Revert "ACPI / OSI: Add OEM _OSI strings to disable NVidia RTD3" drm/nouveau/pci: enable pcie link changes for pascal drm/nouveau/pci: add nvkm_pcie_get_speed drm/nouveau/pci: save the boot pcie link speed and restore it on fini drm/nouveau: abort runtime suspend if we hit an error
drivers/acpi/osi.c | 24 ---------- .../drm/nouveau/include/nvkm/core/device.h | 2 + .../gpu/drm/nouveau/include/nvkm/subdev/pci.h | 9 ++-- drivers/gpu/drm/nouveau/nouveau_drm.c | 6 +++ .../gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +- .../gpu/drm/nouveau/nvkm/subdev/pci/base.c | 9 +++- .../gpu/drm/nouveau/nvkm/subdev/pci/gk104.c | 8 ++-- .../gpu/drm/nouveau/nvkm/subdev/pci/gp100.c | 10 ++++ .../gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 46 +++++++++++++++++-- .../gpu/drm/nouveau/nvkm/subdev/pci/priv.h | 7 +++ 10 files changed, 84 insertions(+), 39 deletions(-)
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)
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
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.
-----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.
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?
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
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
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
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.
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.
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.
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.
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?
-----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?
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?
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
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=a...
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
-----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=a... 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?
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=a... 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
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=a... 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
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
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
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
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
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
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
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
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).
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).
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).
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
This reverts commit 887532ca7ca59fcf0547a79211756791128030a3.
We have a better solution for this: b516ea586d717
And same as with the last commit: "NVidia Linux driver" that's Nouveau, any out of tree driver does _not_ matter. And with Nouveau all of this works even though it required a proper fix first, but we have that now.
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 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index 9b20ac4d79a0..56cc95b6b724 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -53,14 +53,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = { * be removed if both new and old graphics cards are supported. */ {"Linux-Dell-Video", true}, - /* - * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI - * audio device which is turned off for power-saving in Windows OS. - * This power management feature observed on some Lenovo Thinkpad - * systems which will not be able to output audio via HDMI without - * a BIOS workaround. - */ - {"Linux-Lenovo-NV-HDMI-Audio", true}, };
static u32 acpi_osi_handler(acpi_string interface, u32 supported)
Hi, bumping the discussion again here because we just ran into this again, as we just had a laptop OEM try to fix another issue by adding an OSI check for these strings instead of helping us fix the issue in nouveau. Note as well the issue I'm referring to is entirely independent of runtime D3, so we just got runtime D3 broken again because of these hacks.
Why hasn't this patch been accepted into the kernel? These were added without any sort of discussion with nouveau developers, and nouveau is the canonical nvidia driver in the Linux kernel. These strings shouldn't be here without discussion with the relevant driver maintainers, and need to be removed.
On Wed, 2019-08-14 at 23:31 +0200, Karol Herbst wrote:
This reverts commit 887532ca7ca59fcf0547a79211756791128030a3.
We have a better solution for this: b516ea586d717
And same as with the last commit: "NVidia Linux driver" that's Nouveau, any out of tree driver does _not_ matter. And with Nouveau all of this works even though it required a proper fix first, but we have that now.
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 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index 9b20ac4d79a0..56cc95b6b724 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -53,14 +53,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = { * be removed if both new and old graphics cards are supported. */ {"Linux-Dell-Video", true}, - /* - * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI - * audio device which is turned off for power-saving in Windows OS. - * This power management feature observed on some Lenovo Thinkpad - * systems which will not be able to output audio via HDMI without - * a BIOS workaround. - */ - {"Linux-Lenovo-NV-HDMI-Audio", true}, }; static u32 acpi_osi_handler(acpi_string interface, u32 supported)
This reverts commit 9251a71db62ca9cc7e7cf364218610b0f018c291.
This was never discussed with anybody Nouveau related and we would have NACKed this change immediately.
We have a better workaround, which makes it actually work with Nouveau. No idea why the comment mentions the Nvidia driver and assumes it gives any weight to the reasoning.... we don't care about out of tree drivers.
Nouveau does support RTD3, but we had some issues with that. And we even have a better fix for this issue. Also, can we _please_ do it in a way worthy of an upstream community the next time?
If some distribution feels like they have to please companies not wanting to be part of the linux community, please do so downstream and don't try to push something like this upstream.
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 | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index 56cc95b6b724..f5d559a2ff14 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -44,15 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = { {"Processor Device", true}, {"3.0 _SCP Extensions", true}, {"Processor Aggregator Device", true}, - /* - * Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia graphics - * cards as RTD3 is not supported by drivers now. Systems with NVidia - * cards will hang without RTD3 disabled. - * - * Once NVidia drivers officially support RTD3, this _OSI strings can - * be removed if both new and old graphics cards are supported. - */ - {"Linux-Dell-Video", true}, };
static u32 acpi_osi_handler(acpi_string interface, u32 supported)
On Wed, Aug 14, 2019 at 3:31 PM Karol Herbst kherbst@redhat.com wrote:
This reverts commit 9251a71db62ca9cc7e7cf364218610b0f018c291.
This was never discussed with anybody Nouveau related and we would have NACKed this change immediately.
We have a better workaround, which makes it actually work with Nouveau. No idea why the comment mentions the Nvidia driver and assumes it gives any weight to the reasoning.... we don't care about out of tree drivers.
Nouveau does support RTD3, but we had some issues with that. And we even have a better fix for this issue. Also, can we _please_ do it in a way worthy of an upstream community the next time?
If some distribution feels like they have to please companies not wanting to be part of the linux community, please do so downstream and don't try to push something like this upstream.
Hi Karol,
A lot of appreciation for your hard-work on this issue, but unfriendly comments aren't necessary. At the time this was discussed with hardware vendors and platform vendors and it worked for many systems and benefit for many people buying these platforms. Last but not least, I do appreciate better fixes and want to retire the hacks too.
I am going to notify hardware owners to test these patches on the original intended systems, and will report whether there are regressions.
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 | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index 56cc95b6b724..f5d559a2ff14 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -44,15 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = { {"Processor Device", true}, {"3.0 _SCP Extensions", true}, {"Processor Aggregator Device", true},
/** Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia graphics* cards as RTD3 is not supported by drivers now. Systems with NVidia* cards will hang without RTD3 disabled.** Once NVidia drivers officially support RTD3, this _OSI strings can* be removed if both new and old graphics cards are supported.*/{"Linux-Dell-Video", true},};
static u32 acpi_osi_handler(acpi_string interface, u32 supported)
2.21.0
-- Cheers, Alex Hung
On Thu, Aug 15, 2019 at 1:35 AM Alex Hung alex.hung@canonical.com wrote:
On Wed, Aug 14, 2019 at 3:31 PM Karol Herbst kherbst@redhat.com wrote:
This reverts commit 9251a71db62ca9cc7e7cf364218610b0f018c291.
This was never discussed with anybody Nouveau related and we would have NACKed this change immediately.
We have a better workaround, which makes it actually work with Nouveau. No idea why the comment mentions the Nvidia driver and assumes it gives any weight to the reasoning.... we don't care about out of tree drivers.
Nouveau does support RTD3, but we had some issues with that. And we even have a better fix for this issue. Also, can we _please_ do it in a way worthy of an upstream community the next time?
If some distribution feels like they have to please companies not wanting to be part of the linux community, please do so downstream and don't try to push something like this upstream.
Hi Karol,
A lot of appreciation for your hard-work on this issue, but unfriendly comments aren't necessary. At the time this was discussed with hardware vendors and platform vendors and it worked for many systems and benefit for many people buying these platforms. Last but not least, I do appreciate better fixes and want to retire the hacks too.
sorry if that sounded too harsh, but the issue is not that it helped, the issue is, that nobody talked with us about that. And those three commits also sounded like they were mainly targeted against the Nvidia driver (and please correct me if I am wrong here).
The situation would be completely different if we would have talked about all this earlier (and I wouldn't get annoyed by all of this in the first place).
I am well aware this is a super painful issue for a lot of users, but even then disabling RTD3 (for a limited amount of hardware) inside nouveau would be the only acceptable way to disable it.
I don't see how pushing vendors to add some firmware code to disable certain features helps in any way, when there is a fast and easy way to disable it and wouldn't draw resources away from actually fixing it.
I am going to notify hardware owners to test these patches on the original intended systems, and will report whether there are regressions.
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 | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index 56cc95b6b724..f5d559a2ff14 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -44,15 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = { {"Processor Device", true}, {"3.0 _SCP Extensions", true}, {"Processor Aggregator Device", true},
/** Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia graphics* cards as RTD3 is not supported by drivers now. Systems with NVidia* cards will hang without RTD3 disabled.** Once NVidia drivers officially support RTD3, this _OSI strings can* be removed if both new and old graphics cards are supported.*/{"Linux-Dell-Video", true},};
static u32 acpi_osi_handler(acpi_string interface, u32 supported)
2.21.0
-- Cheers, Alex Hung
Signed-off-by: Karol Herbst kherbst@redhat.com Reviewed-by: Lyude Paul lyude@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/gpu/drm/nouveau/nvkm/subdev/pci/gk104.c | 8 ++++---- drivers/gpu/drm/nouveau/nvkm/subdev/pci/gp100.c | 10 ++++++++++ drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h | 5 +++++ 3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gk104.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gk104.c index e68030507d88..664890185e15 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gk104.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gk104.c @@ -23,7 +23,7 @@ */ #include "priv.h"
-static int +int gk104_pcie_version_supported(struct nvkm_pci *pci) { return (nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x4) == 0x4 ? 2 : 1; @@ -108,7 +108,7 @@ gk104_pcie_lnkctl_speed(struct nvkm_pci *pci) return -1; }
-static enum nvkm_pcie_speed +enum nvkm_pcie_speed gk104_pcie_max_speed(struct nvkm_pci *pci) { u32 max_speed = nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x300000; @@ -146,7 +146,7 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum nvkm_pcie_speed speed) nvkm_mask(device, 0x8c040, 0x1, 0x1); }
-static int +int gk104_pcie_init(struct nvkm_pci * pci) { enum nvkm_pcie_speed lnkctl_speed, max_speed, cap_speed; @@ -178,7 +178,7 @@ gk104_pcie_init(struct nvkm_pci * pci) return 0; }
-static int +int gk104_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width) { struct nvkm_subdev *subdev = &pci->subdev; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gp100.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gp100.c index 82c5234a06ff..eb19c7a44561 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gp100.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/gp100.c @@ -35,6 +35,16 @@ gp100_pci_func = { .wr08 = nv40_pci_wr08, .wr32 = nv40_pci_wr32, .msi_rearm = gp100_pci_msi_rearm, + + .pcie.init = gk104_pcie_init, + .pcie.set_link = gk104_pcie_set_link, + + .pcie.max_speed = gk104_pcie_max_speed, + .pcie.cur_speed = g84_pcie_cur_speed, + + .pcie.set_version = gf100_pcie_set_version, + .pcie.version = gf100_pcie_version, + .pcie.version_supported = gk104_pcie_version_supported, };
int diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h index 7009aad86b6e..162ed5dc6fc3 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h @@ -54,6 +54,11 @@ int gf100_pcie_cap_speed(struct nvkm_pci *); int gf100_pcie_init(struct nvkm_pci *); int gf100_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8);
+int gk104_pcie_init(struct nvkm_pci *); +int gk104_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width); +enum nvkm_pcie_speed gk104_pcie_max_speed(struct nvkm_pci *); +int gk104_pcie_version_supported(struct nvkm_pci *); + int nvkm_pcie_oneinit(struct nvkm_pci *); int nvkm_pcie_init(struct nvkm_pci *); #endif
v2: fixed compilation error
Signed-off-by: Karol Herbst kherbst@redhat.com Reviewed-by: Lyude Paul lyude@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/gpu/drm/nouveau/include/nvkm/subdev/pci.h | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 8 ++++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h index 4803a4fad4a2..b29101e485f9 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h @@ -53,4 +53,5 @@ int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
/* pcie functions */ int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width); +enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *); #endif diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c index d71e5db5028a..03be24930c0e 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c @@ -163,3 +163,11 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
return ret; } + +enum nvkm_pcie_speed +nvkm_pcie_get_speed(struct nvkm_pci *pci) +{ + if (!pci || !pci_is_pcie(pci->pdev) || !pci->func->pcie.cur_speed) + return -ENODEV; + return pci->func->pcie.cur_speed(pci); +}
Apperantly things go south if we suspend the device with a different PCIE link speed set than it got booted with. Fixes runtime suspend on my gp107.
This all looks like some bug inside the pci subsystem and I would prefer a fix there instead of nouveau, but maybe there is no real nice way of doing that outside of drivers?
v2: squashed together patch 4 and 5 v3: only restore pcie speed on machines with runpm add NvRunpmWorkaround config option to disable the workaround
Signed-off-by: Karol Herbst kherbst@redhat.com Reviewed-by: Lyude Paul lyude@redhat.com (v2) 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 --- .../drm/nouveau/include/nvkm/core/device.h | 2 + .../gpu/drm/nouveau/include/nvkm/subdev/pci.h | 8 ++-- drivers/gpu/drm/nouveau/nouveau_drm.c | 1 + .../gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +- .../gpu/drm/nouveau/nvkm/subdev/pci/base.c | 9 ++++- .../gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 38 ++++++++++++++++--- .../gpu/drm/nouveau/nvkm/subdev/pci/priv.h | 2 + 7 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/device.h b/drivers/gpu/drm/nouveau/include/nvkm/core/device.h index 6d55cd0476aa..4fb3f972ff97 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/core/device.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/device.h @@ -125,6 +125,8 @@ struct nvkm_device { u8 chiprev; u32 crystal;
+ bool has_runpm; + struct { struct notifier_block nb; } acpi; diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h index b29101e485f9..56e73412fe4a 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h @@ -26,8 +26,9 @@ struct nvkm_pci { } agp;
struct { - enum nvkm_pcie_speed speed; - u8 width; + enum nvkm_pcie_speed cur_speed; + enum nvkm_pcie_speed def_speed; + u8 cur_width; } pcie;
bool msi; @@ -52,6 +53,7 @@ int gk104_pci_new(struct nvkm_device *, int, struct nvkm_pci **); int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
/* pcie functions */ -int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width); +int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width, + bool save); enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *); #endif diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 7c2fcaba42d6..16441c5bf29c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -675,6 +675,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
if (nouveau_atomic) driver_pci.driver_features |= DRIVER_ATOMIC; + device->has_runpm = nouveau_pmops_runtime();
drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev); if (IS_ERR(drm_dev)) { diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c index 40e564524b7a..1a524ffa77a3 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c @@ -278,7 +278,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei) nvkm_debug(subdev, "setting performance state %d\n", pstatei); clk->pstate = pstatei;
- nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width); + nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width, true);
if (fb && fb->ram && fb->ram->func->calc) { struct nvkm_ram *ram = fb->ram; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c index ee2431a7804e..d9fb5a83f7d2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c @@ -90,6 +90,8 @@ nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend)
if (pci->agp.bridge) nvkm_agp_fini(pci); + else if (pci_is_pcie(pci->pdev)) + nvkm_pcie_fini(pci);
return 0; } @@ -100,6 +102,8 @@ nvkm_pci_preinit(struct nvkm_subdev *subdev) struct nvkm_pci *pci = nvkm_pci(subdev); if (pci->agp.bridge) nvkm_agp_preinit(pci); + else if (pci_is_pcie(pci->pdev)) + nvkm_pcie_preinit(pci); return 0; }
@@ -193,8 +197,9 @@ nvkm_pci_new_(const struct nvkm_pci_func *func, struct nvkm_device *device, pci->func = func; pci->pdev = device->func->pci(device)->pdev; pci->irq = -1; - pci->pcie.speed = -1; - pci->pcie.width = -1; + pci->pcie.cur_speed = -1; + pci->pcie.def_speed = -1; + pci->pcie.cur_width = -1;
if (device->type == NVKM_DEVICE_AGP) nvkm_agp_ctor(pci); diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c index 03be24930c0e..d6b7ad6a8d0d 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c @@ -23,6 +23,8 @@ */ #include "priv.h"
+#include <core/option.h> + static char *nvkm_pcie_speeds[] = { "2.5GT/s", "5.0GT/s", @@ -85,6 +87,13 @@ nvkm_pcie_oneinit(struct nvkm_pci *pci) return 0; }
+int +nvkm_pcie_preinit(struct nvkm_pci *pci) +{ + pci->pcie.def_speed = nvkm_pcie_get_speed(pci); + return 0; +} + int nvkm_pcie_init(struct nvkm_pci *pci) { @@ -105,14 +114,31 @@ nvkm_pcie_init(struct nvkm_pci *pci) if (pci->func->pcie.init) pci->func->pcie.init(pci);
- if (pci->pcie.speed != -1) - nvkm_pcie_set_link(pci, pci->pcie.speed, pci->pcie.width); + if (pci->pcie.cur_speed != -1) + nvkm_pcie_set_link(pci, pci->pcie.cur_speed, + pci->pcie.cur_width, false); + + return 0; +} + +int +nvkm_pcie_fini(struct nvkm_pci *pci) +{ + struct nvkm_device *device = pci->subdev.device; + if (!device->has_runpm) + return 0; + + if (!nvkm_boolopt(device->cfgopt, "NvRunpmWorkaround", true)) + return 0;
+ if (!IS_ERR_VALUE(pci->pcie.def_speed)) + return nvkm_pcie_set_link(pci, pci->pcie.def_speed, 16, false); return 0; }
int -nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width) +nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width, + bool save) { struct nvkm_subdev *subdev = &pci->subdev; enum nvkm_pcie_speed cur_speed, max_speed; @@ -146,8 +172,10 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width) speed = max_speed; }
- pci->pcie.speed = speed; - pci->pcie.width = width; + if (save) { + pci->pcie.cur_speed = speed; + pci->pcie.cur_width = width; + }
if (speed == cur_speed) { nvkm_debug(subdev, "requested matches current speed\n"); diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h index 162ed5dc6fc3..85be0594f11b 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/priv.h @@ -60,5 +60,7 @@ enum nvkm_pcie_speed gk104_pcie_max_speed(struct nvkm_pci *); int gk104_pcie_version_supported(struct nvkm_pci *);
int nvkm_pcie_oneinit(struct nvkm_pci *); +int nvkm_pcie_preinit(struct nvkm_pci *); int nvkm_pcie_init(struct nvkm_pci *); +int nvkm_pcie_fini(struct nvkm_pci *); #endif
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/gpu/drm/nouveau/nouveau_drm.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 16441c5bf29c..b16157a9c736 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -910,6 +910,7 @@ nouveau_pmops_runtime_suspend(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); struct drm_device *drm_dev = pci_get_drvdata(pdev); + struct nouveau_drm *drm = nouveau_drm(drm_dev); int ret;
if (!nouveau_pmops_runtime()) { @@ -919,6 +920,10 @@ nouveau_pmops_runtime_suspend(struct device *dev)
nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); + if (ret) { + NV_ERROR(drm, "suspend failed with: %d\n", ret); + return ret; + } pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev);
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Alex Hung -
Daniel Vetter -
Dave Airlie -
Karol Herbst -
Lyude Paul -
Mario.Limonciello＠dell.com -
Rafael J. Wysocki -
Rafael J. Wysocki -
Takashi Iwai