All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
@ 2016-11-23  1:22 Peter Wu
       [not found] ` <20161123012225.11586-1-peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Wu @ 2016-11-23  1:22 UTC (permalink / raw)
  To: amd-gfx list; +Cc: Alex Deucher, Nayan Deshmukh

When PCIe port PM is not enabled (system BIOS is pre-2015 or the
pcie_port_pm=off parameter is set), legacy ATPX PM should still be
marked as supported. Otherwise the GPU can fail to power on after
runtime suspend. This affected a Dell Inspiron 5548.

Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
where hybrid graphics platforms using power resources was introduced),
but that seems more risky at this point and would not solve the
pcie_port_pm=off issue.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
Reported-and-tested-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
Hi,

This patch is already three weeks old. One alternative idea was lowering BIOS
date in PCI core, but as pcie_port_pm=force did not have the desired effect, I
do not think that this would help though.

I have also not contacted linux-pci or Mika about lowering the year due to the
lack of a good reason.  Might do it later though once ACPICA bug 1333 is sorted
out such that lowering the year actually has benefits for a Nvidia laptop (or if
some amdgpu problem can be solved by this).

Both patches should probably be Cc stable (4.8+), fixing 2f5af82eeab2 and
b8c9fd5ad4b4 ("track whether if this is a hybrid graphics platform"). There have
been some ifdef 0's and reverts in between, so I was not sure if adding the
Fixes tag is appropriate.

Kind regards,
Peter
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 10b5ddf..951addf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -33,6 +33,7 @@ struct amdgpu_atpx {
 
 static struct amdgpu_atpx_priv {
 	bool atpx_detected;
+	bool bridge_pm_usable;
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	acpi_handle other_handle;
@@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
 	atpx->is_hybrid = false;
 	if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
 		printk("ATPX Hybrid Graphics\n");
-		atpx->functions.power_cntl = false;
+		/*
+		 * Disable legacy PM methods only when pcie port PM is usable,
+		 * otherwise the device might fail to power off or power on.
+		 */
+		atpx->functions.power_cntl = !amdgpu_atpx_priv.bridge_pm_usable;
 		atpx->is_hybrid = true;
 	}
 
@@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum vga_switcheroo_client_id id,
  */
 static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
+	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
 	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
@@ -485,6 +491,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
 	}
 	amdgpu_atpx_priv.dhandle = dhandle;
 	amdgpu_atpx_priv.atpx.handle = atpx_handle;
+	amdgpu_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
 	return true;
 }
 
-- 
2.10.2

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

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

* [PATCH 2/2] drm/radeon: fix power state when port pm is unavailable
       [not found] ` <20161123012225.11586-1-peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
@ 2016-11-23  1:22   ` Peter Wu
  2016-11-23  7:53   ` [PATCH 1/2] drm/amdgpu: " Christian König
  2016-11-23 17:01   ` Alex Deucher
  2 siblings, 0 replies; 17+ messages in thread
From: Peter Wu @ 2016-11-23  1:22 UTC (permalink / raw)
  To: amd-gfx list; +Cc: Alex Deucher, Nayan Deshmukh

When PCIe port PM is not enabled (system BIOS is pre-2015 or the
pcie_port_pm=off parameter is set), legacy ATPX PM should still be
marked as supported. Otherwise the GPU can fail to power on after
runtime suspend. This affected a Dell Inspiron 5548.

Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
where hybrid graphics platforms using power resources was introduced),
but that seems more risky at this point and would not solve the
pcie_port_pm=off issue.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
 drivers/gpu/drm/radeon/radeon_atpx_handler.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index ddef0d4..298949c 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -33,6 +33,7 @@ struct radeon_atpx {
 
 static struct radeon_atpx_priv {
 	bool atpx_detected;
+	bool bridge_pm_usable;
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	struct radeon_atpx atpx;
@@ -199,7 +200,11 @@ static int radeon_atpx_validate(struct radeon_atpx *atpx)
 	if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
 		printk("ATPX Hybrid Graphics\n");
 		atpx->functions.power_cntl = false;
-		atpx->is_hybrid = true;
+		/*
+		 * Disable legacy PM methods only when pcie port PM is usable,
+		 * otherwise the device might fail to power off or power on.
+		 */
+		atpx->functions.power_cntl = !radeon_atpx_priv.bridge_pm_usable;
 	}
 
 	return 0;
@@ -469,6 +474,7 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
  */
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
+	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
 	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
@@ -482,6 +488,7 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx.handle = atpx_handle;
+	radeon_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
 	return true;
 }
 
-- 
2.10.2

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found] ` <20161123012225.11586-1-peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
  2016-11-23  1:22   ` [PATCH 2/2] drm/radeon: " Peter Wu
@ 2016-11-23  7:53   ` Christian König
  2016-11-23 17:01   ` Alex Deucher
  2 siblings, 0 replies; 17+ messages in thread
From: Christian König @ 2016-11-23  7:53 UTC (permalink / raw)
  To: Peter Wu, amd-gfx list; +Cc: Alex Deucher, Nayan Deshmukh

Am 23.11.2016 um 02:22 schrieb Peter Wu:
> When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> marked as supported. Otherwise the GPU can fail to power on after
> runtime suspend. This affected a Dell Inspiron 5548.
>
> Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
> where hybrid graphics platforms using power resources was introduced),
> but that seems more risky at this point and would not solve the
> pcie_port_pm=off issue.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> Reported-and-tested-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Even when lowering the BIOS date this sounds reasonable to me in the 
case of pcie_port_pm=off.

But since I'm not very deep into power management the patch is only 
Acked-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
> Hi,
>
> This patch is already three weeks old. One alternative idea was lowering BIOS
> date in PCI core, but as pcie_port_pm=force did not have the desired effect, I
> do not think that this would help though.
>
> I have also not contacted linux-pci or Mika about lowering the year due to the
> lack of a good reason.  Might do it later though once ACPICA bug 1333 is sorted
> out such that lowering the year actually has benefits for a Nvidia laptop (or if
> some amdgpu problem can be solved by this).
>
> Both patches should probably be Cc stable (4.8+), fixing 2f5af82eeab2 and
> b8c9fd5ad4b4 ("track whether if this is a hybrid graphics platform"). There have
> been some ifdef 0's and reverts in between, so I was not sure if adding the
> Fixes tag is appropriate.
>
> Kind regards,
> Peter
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index 10b5ddf..951addf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -33,6 +33,7 @@ struct amdgpu_atpx {
>   
>   static struct amdgpu_atpx_priv {
>   	bool atpx_detected;
> +	bool bridge_pm_usable;
>   	/* handle for device - and atpx */
>   	acpi_handle dhandle;
>   	acpi_handle other_handle;
> @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
>   	atpx->is_hybrid = false;
>   	if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
>   		printk("ATPX Hybrid Graphics\n");
> -		atpx->functions.power_cntl = false;
> +		/*
> +		 * Disable legacy PM methods only when pcie port PM is usable,
> +		 * otherwise the device might fail to power off or power on.
> +		 */
> +		atpx->functions.power_cntl = !amdgpu_atpx_priv.bridge_pm_usable;
>   		atpx->is_hybrid = true;
>   	}
>   
> @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum vga_switcheroo_client_id id,
>    */
>   static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
>   {
> +	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
>   	acpi_handle dhandle, atpx_handle;
>   	acpi_status status;
>   
> @@ -485,6 +491,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
>   	}
>   	amdgpu_atpx_priv.dhandle = dhandle;
>   	amdgpu_atpx_priv.atpx.handle = atpx_handle;
> +	amdgpu_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
>   	return true;
>   }
>   


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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found] ` <20161123012225.11586-1-peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
  2016-11-23  1:22   ` [PATCH 2/2] drm/radeon: " Peter Wu
  2016-11-23  7:53   ` [PATCH 1/2] drm/amdgpu: " Christian König
@ 2016-11-23 17:01   ` Alex Deucher
       [not found]     ` <CADnq5_On72XmLrE3wjD0u1=tmwghv6GofzfEGM5mhGoVtJav+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2 siblings, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2016-11-23 17:01 UTC (permalink / raw)
  To: Peter Wu; +Cc: Alex Deucher, Nayan Deshmukh, amd-gfx list

 On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> marked as supported. Otherwise the GPU can fail to power on after
> runtime suspend. This affected a Dell Inspiron 5548.
>
> Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
> where hybrid graphics platforms using power resources was introduced),
> but that seems more risky at this point and would not solve the
> pcie_port_pm=off issue.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> Reported-and-tested-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> Hi,
>
> This patch is already three weeks old. One alternative idea was lowering BIOS
> date in PCI core, but as pcie_port_pm=force did not have the desired effect, I
> do not think that this would help though.

Thanks for doing this.

>
> I have also not contacted linux-pci or Mika about lowering the year due to the
> lack of a good reason.  Might do it later though once ACPICA bug 1333 is sorted
> out such that lowering the year actually has benefits for a Nvidia laptop (or if
> some amdgpu problem can be solved by this).
>
> Both patches should probably be Cc stable (4.8+), fixing 2f5af82eeab2 and
> b8c9fd5ad4b4 ("track whether if this is a hybrid graphics platform"). There have
> been some ifdef 0's and reverts in between, so I was not sure if adding the
> Fixes tag is appropriate.

I don't think we need to cc stable.  In kernel 4.8 we don't attempt to
do d3cold at all.  4.8 and older kernels have:
c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around lack
of upstream ACPI support for D3cold)
which was reverted for 4.9.

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


>
> Kind regards,
> Peter
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index 10b5ddf..951addf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -33,6 +33,7 @@ struct amdgpu_atpx {
>
>  static struct amdgpu_atpx_priv {
>         bool atpx_detected;
> +       bool bridge_pm_usable;
>         /* handle for device - and atpx */
>         acpi_handle dhandle;
>         acpi_handle other_handle;
> @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
>         atpx->is_hybrid = false;
>         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
>                 printk("ATPX Hybrid Graphics\n");
> -               atpx->functions.power_cntl = false;
> +               /*
> +                * Disable legacy PM methods only when pcie port PM is usable,
> +                * otherwise the device might fail to power off or power on.
> +                */
> +               atpx->functions.power_cntl = !amdgpu_atpx_priv.bridge_pm_usable;
>                 atpx->is_hybrid = true;
>         }
>
> @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum vga_switcheroo_client_id id,
>   */
>  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
>  {
> +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
>         acpi_handle dhandle, atpx_handle;
>         acpi_status status;
>
> @@ -485,6 +491,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
>         }
>         amdgpu_atpx_priv.dhandle = dhandle;
>         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
>         return true;
>  }
>
> --
> 2.10.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]     ` <CADnq5_On72XmLrE3wjD0u1=tmwghv6GofzfEGM5mhGoVtJav+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-23 17:12       ` Peter Wu
  2016-11-23 17:16         ` Deucher, Alexander
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Wu @ 2016-11-23 17:12 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, Nayan Deshmukh, amd-gfx list

On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
>  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> > marked as supported. Otherwise the GPU can fail to power on after
> > runtime suspend. This affected a Dell Inspiron 5548.
> >
> > Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
> > where hybrid graphics platforms using power resources was introduced),
> > but that seems more risky at this point and would not solve the
> > pcie_port_pm=off issue.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > Reported-and-tested-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> > Hi,
> >
> > This patch is already three weeks old. One alternative idea was lowering BIOS
> > date in PCI core, but as pcie_port_pm=force did not have the desired effect, I
> > do not think that this would help though.
> 
> Thanks for doing this.
> 
> >
> > I have also not contacted linux-pci or Mika about lowering the year due to the
> > lack of a good reason.  Might do it later though once ACPICA bug 1333 is sorted
> > out such that lowering the year actually has benefits for a Nvidia laptop (or if
> > some amdgpu problem can be solved by this).
> >
> > Both patches should probably be Cc stable (4.8+), fixing 2f5af82eeab2 and
> > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics platform"). There have
> > been some ifdef 0's and reverts in between, so I was not sure if adding the
> > Fixes tag is appropriate.
> 
> I don't think we need to cc stable.  In kernel 4.8 we don't attempt to
> do d3cold at all.  4.8 and older kernels have:
> c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around lack
> of upstream ACPI support for D3cold)
> which was reverted for 4.9.

That patch was already reverted in 4.8 as far as I can see:

$ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep v4.8
v4.8

Can you double-check? v4.8 was the first kernel with D3cold support in
PCI core.

> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks!

Kind regards,
Peter

> 
> >
> > Kind regards,
> > Peter
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > index 10b5ddf..951addf 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> >
> >  static struct amdgpu_atpx_priv {
> >         bool atpx_detected;
> > +       bool bridge_pm_usable;
> >         /* handle for device - and atpx */
> >         acpi_handle dhandle;
> >         acpi_handle other_handle;
> > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
> >         atpx->is_hybrid = false;
> >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> >                 printk("ATPX Hybrid Graphics\n");
> > -               atpx->functions.power_cntl = false;
> > +               /*
> > +                * Disable legacy PM methods only when pcie port PM is usable,
> > +                * otherwise the device might fail to power off or power on.
> > +                */
> > +               atpx->functions.power_cntl = !amdgpu_atpx_priv.bridge_pm_usable;
> >                 atpx->is_hybrid = true;
> >         }
> >
> > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum vga_switcheroo_client_id id,
> >   */
> >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> >  {
> > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> >         acpi_handle dhandle, atpx_handle;
> >         acpi_status status;
> >
> > @@ -485,6 +491,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> >         }
> >         amdgpu_atpx_priv.dhandle = dhandle;
> >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
> >         return true;
> >  }
> >
> > --
> > 2.10.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* RE: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
  2016-11-23 17:12       ` Peter Wu
@ 2016-11-23 17:16         ` Deucher, Alexander
       [not found]           ` <MWHPR12MB16949832E9DB44C53995493AF7B70-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Deucher, Alexander @ 2016-11-23 17:16 UTC (permalink / raw)
  To: 'Peter Wu', Alex Deucher; +Cc: Nayan Deshmukh, amd-gfx list

> -----Original Message-----
> From: Peter Wu [mailto:peter@lekensteyn.nl]
> Sent: Wednesday, November 23, 2016 12:13 PM
> To: Alex Deucher
> Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> unavailable
> 
> On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> > > marked as supported. Otherwise the GPU can fail to power on after
> > > runtime suspend. This affected a Dell Inspiron 5548.
> > >
> > > Ideally the BIOS date in the PCI core is lowered to 2013 (the first year
> > > where hybrid graphics platforms using power resources was introduced),
> > > but that seems more risky at this point and would not solve the
> > > pcie_port_pm=off issue.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > Reported-and-tested-by: Nayan Deshmukh
> <nayan26deshmukh@gmail.com>
> > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > > ---
> > > Hi,
> > >
> > > This patch is already three weeks old. One alternative idea was lowering
> BIOS
> > > date in PCI core, but as pcie_port_pm=force did not have the desired
> effect, I
> > > do not think that this would help though.
> >
> > Thanks for doing this.
> >
> > >
> > > I have also not contacted linux-pci or Mika about lowering the year due to
> the
> > > lack of a good reason.  Might do it later though once ACPICA bug 1333 is
> sorted
> > > out such that lowering the year actually has benefits for a Nvidia laptop
> (or if
> > > some amdgpu problem can be solved by this).
> > >
> > > Both patches should probably be Cc stable (4.8+), fixing 2f5af82eeab2 and
> > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics platform"). There
> have
> > > been some ifdef 0's and reverts in between, so I was not sure if adding
> the
> > > Fixes tag is appropriate.
> >
> > I don't think we need to cc stable.  In kernel 4.8 we don't attempt to
> > do d3cold at all.  4.8 and older kernels have:
> > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> lack
> > of upstream ACPI support for D3cold)
> > which was reverted for 4.9.
> 
> That patch was already reverted in 4.8 as far as I can see:
> 
> $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep v4.8
> v4.8
> 
> Can you double-check? v4.8 was the first kernel with D3cold support in
> PCI core.

You are right.  I was thinking d3cold went upstream in 4.9, so yes, we should apply this to 4.8.

Alex

> 
> > Series is:
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Thanks!
> 
> Kind regards,
> Peter
> 
> >
> > >
> > > Kind regards,
> > > Peter
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > index 10b5ddf..951addf 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > >
> > >  static struct amdgpu_atpx_priv {
> > >         bool atpx_detected;
> > > +       bool bridge_pm_usable;
> > >         /* handle for device - and atpx */
> > >         acpi_handle dhandle;
> > >         acpi_handle other_handle;
> > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> amdgpu_atpx *atpx)
> > >         atpx->is_hybrid = false;
> > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > >                 printk("ATPX Hybrid Graphics\n");
> > > -               atpx->functions.power_cntl = false;
> > > +               /*
> > > +                * Disable legacy PM methods only when pcie port PM is usable,
> > > +                * otherwise the device might fail to power off or power on.
> > > +                */
> > > +               atpx->functions.power_cntl =
> !amdgpu_atpx_priv.bridge_pm_usable;
> > >                 atpx->is_hybrid = true;
> > >         }
> > >
> > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> vga_switcheroo_client_id id,
> > >   */
> > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > >  {
> > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > >         acpi_handle dhandle, atpx_handle;
> > >         acpi_status status;
> > >
> > > @@ -485,6 +491,7 @@ static bool
> amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > >         }
> > >         amdgpu_atpx_priv.dhandle = dhandle;
> > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> parent_pdev->bridge_d3;
> > >         return true;
> > >  }
> > >
> > > --
> > > 2.10.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]           ` <MWHPR12MB16949832E9DB44C53995493AF7B70-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-11-26  0:36             ` Mike Lothian
       [not found]               ` <CAHbf0-FgRU=6tYe2SYcm8E0ZvJuRFoqPpk77_DZdQKXdxZSsGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Mike Lothian @ 2016-11-26  0:36 UTC (permalink / raw)
  To: Deucher, Alexander, Peter Wu, Alex Deucher; +Cc: Nayan Deshmukh, amd-gfx list


[-- Attachment #1.1: Type: text/plain, Size: 5903 bytes --]

This patch is preventing my laptop from booting, I'm getting D3 error
messages and atom bios stuck messages

I mistakenly saw the v2 patch and didn't realise it was for radeon not
amdgpu - this branch and Linus's tree are currently not booting for me

On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
wrote:

> > -----Original Message-----
> > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > Sent: Wednesday, November 23, 2016 12:13 PM
> > To: Alex Deucher
> > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> > unavailable
> >
> > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> wrote:
> > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > > pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> > > > marked as supported. Otherwise the GPU can fail to power on after
> > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > >
> > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the first
> year
> > > > where hybrid graphics platforms using power resources was
> introduced),
> > > > but that seems more risky at this point and would not solve the
> > > > pcie_port_pm=off issue.
> > > >
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > Reported-and-tested-by: Nayan Deshmukh
> > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > ---
> > > > Hi,
> > > >
> > > > This patch is already three weeks old. One alternative idea was
> lowering
> > BIOS
> > > > date in PCI core, but as pcie_port_pm=force did not have the desired
> > effect, I
> > > > do not think that this would help though.
> > >
> > > Thanks for doing this.
> > >
> > > >
> > > > I have also not contacted linux-pci or Mika about lowering the year
> due to
> > the
> > > > lack of a good reason.  Might do it later though once ACPICA bug
> 1333 is
> > sorted
> > > > out such that lowering the year actually has benefits for a Nvidia
> laptop
> > (or if
> > > > some amdgpu problem can be solved by this).
> > > >
> > > > Both patches should probably be Cc stable (4.8+), fixing
> 2f5af82eeab2 and
> > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> platform"). There
> > have
> > > > been some ifdef 0's and reverts in between, so I was not sure if
> adding
> > the
> > > > Fixes tag is appropriate.
> > >
> > > I don't think we need to cc stable.  In kernel 4.8 we don't attempt to
> > > do d3cold at all.  4.8 and older kernels have:
> > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> > lack
> > > of upstream ACPI support for D3cold)
> > > which was reverted for 4.9.
> >
> > That patch was already reverted in 4.8 as far as I can see:
> >
> > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep v4.8
> > v4.8
> >
> > Can you double-check? v4.8 was the first kernel with D3cold support in
> > PCI core.
>
> You are right.  I was thinking d3cold went upstream in 4.9, so yes, we
> should apply this to 4.8.
>
> Alex
>
> >
> > > Series is:
> > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> >
> > Thanks!
> >
> > Kind regards,
> > Peter
> >
> > >
> > > >
> > > > Kind regards,
> > > > Peter
> > > > ---
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > index 10b5ddf..951addf 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > >
> > > >  static struct amdgpu_atpx_priv {
> > > >         bool atpx_detected;
> > > > +       bool bridge_pm_usable;
> > > >         /* handle for device - and atpx */
> > > >         acpi_handle dhandle;
> > > >         acpi_handle other_handle;
> > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > amdgpu_atpx *atpx)
> > > >         atpx->is_hybrid = false;
> > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > >                 printk("ATPX Hybrid Graphics\n");
> > > > -               atpx->functions.power_cntl = false;
> > > > +               /*
> > > > +                * Disable legacy PM methods only when pcie port PM
> is usable,
> > > > +                * otherwise the device might fail to power off or
> power on.
> > > > +                */
> > > > +               atpx->functions.power_cntl =
> > !amdgpu_atpx_priv.bridge_pm_usable;
> > > >                 atpx->is_hybrid = true;
> > > >         }
> > > >
> > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > vga_switcheroo_client_id id,
> > > >   */
> > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > >  {
> > > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > > >         acpi_handle dhandle, atpx_handle;
> > > >         acpi_status status;
> > > >
> > > > @@ -485,6 +491,7 @@ static bool
> > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > >         }
> > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > parent_pdev->bridge_d3;
> > > >         return true;
> > > >  }
> > > >
> > > > --
> > > > 2.10.2
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 11038 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]               ` <CAHbf0-FgRU=6tYe2SYcm8E0ZvJuRFoqPpk77_DZdQKXdxZSsGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-26 10:19                 ` Peter Wu
  2016-11-26 11:00                   ` Mike Lothian
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Wu @ 2016-11-26 10:19 UTC (permalink / raw)
  To: Mike Lothian
  Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh

Hi Mike,

On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> This patch is preventing my laptop from booting, I'm getting D3 error
> messages and atom bios stuck messages

Can you post a dmesg and acpidump? What was the previous working kernel?
I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
Does booting with runpm=0 work?

> I mistakenly saw the v2 patch and didn't realise it was for radeon not
> amdgpu - this branch and Linus's tree are currently not booting for me

What do you mean by this?

Kind regards,
Peter

> On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <Alexander.Deucher@amd.com>
> wrote:
> 
> > > -----Original Message-----
> > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > To: Alex Deucher
> > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> > > unavailable
> > >
> > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@lekensteyn.nl>
> > wrote:
> > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should still be
> > > > > marked as supported. Otherwise the GPU can fail to power on after
> > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > >
> > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the first
> > year
> > > > > where hybrid graphics platforms using power resources was
> > introduced),
> > > > > but that seems more risky at this point and would not solve the
> > > > > pcie_port_pm=off issue.
> > > > >
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > Reported-and-tested-by: Nayan Deshmukh
> > > <nayan26deshmukh@gmail.com>
> > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > > > > ---
> > > > > Hi,
> > > > >
> > > > > This patch is already three weeks old. One alternative idea was
> > lowering
> > > BIOS
> > > > > date in PCI core, but as pcie_port_pm=force did not have the desired
> > > effect, I
> > > > > do not think that this would help though.
> > > >
> > > > Thanks for doing this.
> > > >
> > > > >
> > > > > I have also not contacted linux-pci or Mika about lowering the year
> > due to
> > > the
> > > > > lack of a good reason.  Might do it later though once ACPICA bug
> > 1333 is
> > > sorted
> > > > > out such that lowering the year actually has benefits for a Nvidia
> > laptop
> > > (or if
> > > > > some amdgpu problem can be solved by this).
> > > > >
> > > > > Both patches should probably be Cc stable (4.8+), fixing
> > 2f5af82eeab2 and
> > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > platform"). There
> > > have
> > > > > been some ifdef 0's and reverts in between, so I was not sure if
> > adding
> > > the
> > > > > Fixes tag is appropriate.
> > > >
> > > > I don't think we need to cc stable.  In kernel 4.8 we don't attempt to
> > > > do d3cold at all.  4.8 and older kernels have:
> > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> > > lack
> > > > of upstream ACPI support for D3cold)
> > > > which was reverted for 4.9.
> > >
> > > That patch was already reverted in 4.8 as far as I can see:
> > >
> > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep v4.8
> > > v4.8
> > >
> > > Can you double-check? v4.8 was the first kernel with D3cold support in
> > > PCI core.
> >
> > You are right.  I was thinking d3cold went upstream in 4.9, so yes, we
> > should apply this to 4.8.
> >
> > Alex
> >
> > >
> > > > Series is:
> > > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > >
> > > Thanks!
> > >
> > > Kind regards,
> > > Peter
> > >
> > > >
> > > > >
> > > > > Kind regards,
> > > > > Peter
> > > > > ---
> > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > index 10b5ddf..951addf 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > >
> > > > >  static struct amdgpu_atpx_priv {
> > > > >         bool atpx_detected;
> > > > > +       bool bridge_pm_usable;
> > > > >         /* handle for device - and atpx */
> > > > >         acpi_handle dhandle;
> > > > >         acpi_handle other_handle;
> > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > amdgpu_atpx *atpx)
> > > > >         atpx->is_hybrid = false;
> > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > -               atpx->functions.power_cntl = false;
> > > > > +               /*
> > > > > +                * Disable legacy PM methods only when pcie port PM
> > is usable,
> > > > > +                * otherwise the device might fail to power off or
> > power on.
> > > > > +                */
> > > > > +               atpx->functions.power_cntl =
> > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > >                 atpx->is_hybrid = true;
> > > > >         }
> > > > >
> > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > vga_switcheroo_client_id id,
> > > > >   */
> > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > >  {
> > > > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > > > >         acpi_handle dhandle, atpx_handle;
> > > > >         acpi_status status;
> > > > >
> > > > > @@ -485,6 +491,7 @@ static bool
> > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > >         }
> > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > parent_pdev->bridge_d3;
> > > > >         return true;
> > > > >  }
> > > > >
> > > > > --
> > > > > 2.10.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
  2016-11-26 10:19                 ` Peter Wu
@ 2016-11-26 11:00                   ` Mike Lothian
       [not found]                     ` <CAHbf0-HfHGt5HvAosxgVf=rgevPK3KqBCQt4cJqjDiEaQWM1pQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Mike Lothian @ 2016-11-26 11:00 UTC (permalink / raw)
  To: Peter Wu; +Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh


[-- Attachment #1.1: Type: text/plain, Size: 7092 bytes --]

Hi

The previous working kernel, was the patch before this - reverting fixes it

My bios date is quite new 08/05/2016

Most of the messages fly straight off the screen, the journal isn't kept
and the wifi driver also complains, will see if I can get a slow motion
capture of the errors and screen shot them

Regards

Mike

On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:

> Hi Mike,
>
> On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > This patch is preventing my laptop from booting, I'm getting D3 error
> > messages and atom bios stuck messages
>
> Can you post a dmesg and acpidump? What was the previous working kernel?
> I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> Does booting with runpm=0 work?
>
> > I mistakenly saw the v2 patch and didn't realise it was for radeon not
> > amdgpu - this branch and Linus's tree are currently not booting for me
>
> What do you mean by this?
>
> Kind regards,
> Peter
>
> > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > wrote:
> >
> > > > -----Original Message-----
> > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > To: Alex Deucher
> > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> > > > unavailable
> > > >
> > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > wrote:
> > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should still
> be
> > > > > > marked as supported. Otherwise the GPU can fail to power on after
> > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > >
> > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> first
> > > year
> > > > > > where hybrid graphics platforms using power resources was
> > > introduced),
> > > > > > but that seems more risky at this point and would not solve the
> > > > > > pcie_port_pm=off issue.
> > > > > >
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > ---
> > > > > > Hi,
> > > > > >
> > > > > > This patch is already three weeks old. One alternative idea was
> > > lowering
> > > > BIOS
> > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> desired
> > > > effect, I
> > > > > > do not think that this would help though.
> > > > >
> > > > > Thanks for doing this.
> > > > >
> > > > > >
> > > > > > I have also not contacted linux-pci or Mika about lowering the
> year
> > > due to
> > > > the
> > > > > > lack of a good reason.  Might do it later though once ACPICA bug
> > > 1333 is
> > > > sorted
> > > > > > out such that lowering the year actually has benefits for a
> Nvidia
> > > laptop
> > > > (or if
> > > > > > some amdgpu problem can be solved by this).
> > > > > >
> > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > 2f5af82eeab2 and
> > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > platform"). There
> > > > have
> > > > > > been some ifdef 0's and reverts in between, so I was not sure if
> > > adding
> > > > the
> > > > > > Fixes tag is appropriate.
> > > > >
> > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> attempt to
> > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> > > > lack
> > > > > of upstream ACPI support for D3cold)
> > > > > which was reverted for 4.9.
> > > >
> > > > That patch was already reverted in 4.8 as far as I can see:
> > > >
> > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep
> v4.8
> > > > v4.8
> > > >
> > > > Can you double-check? v4.8 was the first kernel with D3cold support
> in
> > > > PCI core.
> > >
> > > You are right.  I was thinking d3cold went upstream in 4.9, so yes, we
> > > should apply this to 4.8.
> > >
> > > Alex
> > >
> > > >
> > > > > Series is:
> > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > >
> > > > Thanks!
> > > >
> > > > Kind regards,
> > > > Peter
> > > >
> > > > >
> > > > > >
> > > > > > Kind regards,
> > > > > > Peter
> > > > > > ---
> > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > index 10b5ddf..951addf 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > >
> > > > > >  static struct amdgpu_atpx_priv {
> > > > > >         bool atpx_detected;
> > > > > > +       bool bridge_pm_usable;
> > > > > >         /* handle for device - and atpx */
> > > > > >         acpi_handle dhandle;
> > > > > >         acpi_handle other_handle;
> > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > amdgpu_atpx *atpx)
> > > > > >         atpx->is_hybrid = false;
> > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > -               atpx->functions.power_cntl = false;
> > > > > > +               /*
> > > > > > +                * Disable legacy PM methods only when pcie port
> PM
> > > is usable,
> > > > > > +                * otherwise the device might fail to power off
> or
> > > power on.
> > > > > > +                */
> > > > > > +               atpx->functions.power_cntl =
> > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > >                 atpx->is_hybrid = true;
> > > > > >         }
> > > > > >
> > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > vga_switcheroo_client_id id,
> > > > > >   */
> > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > >  {
> > > > > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > >         acpi_status status;
> > > > > >
> > > > > > @@ -485,6 +491,7 @@ static bool
> > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > >         }
> > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > parent_pdev->bridge_d3;
> > > > > >         return true;
> > > > > >  }
> > > > > >
> > > > > > --
> > > > > > 2.10.2
>

[-- Attachment #1.2: Type: text/html, Size: 13849 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]                     ` <CAHbf0-HfHGt5HvAosxgVf=rgevPK3KqBCQt4cJqjDiEaQWM1pQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-26 11:20                       ` Mike Lothian
       [not found]                         ` <CAHbf0-HHFFthD3Li0wM8x5rciRreeWoE0SkQOvsENT9+ZRFEcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Mike Lothian @ 2016-11-26 11:20 UTC (permalink / raw)
  To: Peter Wu; +Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh


[-- Attachment #1.1: Type: text/plain, Size: 7324 bytes --]

He's some screen shots of the booting screens, apologies they're not that
clear

https://imgur.com/a/V7mRA

On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org> wrote:

> Hi
>
> The previous working kernel, was the patch before this - reverting fixes it
>
> My bios date is quite new 08/05/2016
>
> Most of the messages fly straight off the screen, the journal isn't kept
> and the wifi driver also complains, will see if I can get a slow motion
> capture of the errors and screen shot them
>
> Regards
>
> Mike
>
> On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
>
> Hi Mike,
>
> On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > This patch is preventing my laptop from booting, I'm getting D3 error
> > messages and atom bios stuck messages
>
> Can you post a dmesg and acpidump? What was the previous working kernel?
> I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> Does booting with runpm=0 work?
>
> > I mistakenly saw the v2 patch and didn't realise it was for radeon not
> > amdgpu - this branch and Linus's tree are currently not booting for me
>
> What do you mean by this?
>
> Kind regards,
> Peter
>
> > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > wrote:
> >
> > > > -----Original Message-----
> > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > To: Alex Deucher
> > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> > > > unavailable
> > > >
> > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > wrote:
> > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should still
> be
> > > > > > marked as supported. Otherwise the GPU can fail to power on after
> > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > >
> > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> first
> > > year
> > > > > > where hybrid graphics platforms using power resources was
> > > introduced),
> > > > > > but that seems more risky at this point and would not solve the
> > > > > > pcie_port_pm=off issue.
> > > > > >
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > ---
> > > > > > Hi,
> > > > > >
> > > > > > This patch is already three weeks old. One alternative idea was
> > > lowering
> > > > BIOS
> > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> desired
> > > > effect, I
> > > > > > do not think that this would help though.
> > > > >
> > > > > Thanks for doing this.
> > > > >
> > > > > >
> > > > > > I have also not contacted linux-pci or Mika about lowering the
> year
> > > due to
> > > > the
> > > > > > lack of a good reason.  Might do it later though once ACPICA bug
> > > 1333 is
> > > > sorted
> > > > > > out such that lowering the year actually has benefits for a
> Nvidia
> > > laptop
> > > > (or if
> > > > > > some amdgpu problem can be solved by this).
> > > > > >
> > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > 2f5af82eeab2 and
> > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > platform"). There
> > > > have
> > > > > > been some ifdef 0's and reverts in between, so I was not sure if
> > > adding
> > > > the
> > > > > > Fixes tag is appropriate.
> > > > >
> > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> attempt to
> > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> > > > lack
> > > > > of upstream ACPI support for D3cold)
> > > > > which was reverted for 4.9.
> > > >
> > > > That patch was already reverted in 4.8 as far as I can see:
> > > >
> > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep
> v4.8
> > > > v4.8
> > > >
> > > > Can you double-check? v4.8 was the first kernel with D3cold support
> in
> > > > PCI core.
> > >
> > > You are right.  I was thinking d3cold went upstream in 4.9, so yes, we
> > > should apply this to 4.8.
> > >
> > > Alex
> > >
> > > >
> > > > > Series is:
> > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > >
> > > > Thanks!
> > > >
> > > > Kind regards,
> > > > Peter
> > > >
> > > > >
> > > > > >
> > > > > > Kind regards,
> > > > > > Peter
> > > > > > ---
> > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > index 10b5ddf..951addf 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > >
> > > > > >  static struct amdgpu_atpx_priv {
> > > > > >         bool atpx_detected;
> > > > > > +       bool bridge_pm_usable;
> > > > > >         /* handle for device - and atpx */
> > > > > >         acpi_handle dhandle;
> > > > > >         acpi_handle other_handle;
> > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > amdgpu_atpx *atpx)
> > > > > >         atpx->is_hybrid = false;
> > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > -               atpx->functions.power_cntl = false;
> > > > > > +               /*
> > > > > > +                * Disable legacy PM methods only when pcie port
> PM
> > > is usable,
> > > > > > +                * otherwise the device might fail to power off
> or
> > > power on.
> > > > > > +                */
> > > > > > +               atpx->functions.power_cntl =
> > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > >                 atpx->is_hybrid = true;
> > > > > >         }
> > > > > >
> > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > vga_switcheroo_client_id id,
> > > > > >   */
> > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > >  {
> > > > > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > >         acpi_status status;
> > > > > >
> > > > > > @@ -485,6 +491,7 @@ static bool
> > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > >         }
> > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > parent_pdev->bridge_d3;
> > > > > >         return true;
> > > > > >  }
> > > > > >
> > > > > > --
> > > > > > 2.10.2
>
>

[-- Attachment #1.2: Type: text/html, Size: 14847 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]                         ` <CAHbf0-HHFFthD3Li0wM8x5rciRreeWoE0SkQOvsENT9+ZRFEcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-26 11:38                           ` Peter Wu
  2016-11-26 13:43                             ` Mike Lothian
  2016-11-26 13:49                             ` Mike Lothian
  0 siblings, 2 replies; 17+ messages in thread
From: Peter Wu @ 2016-11-26 11:38 UTC (permalink / raw)
  To: Mike Lothian
  Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh

Hi Mike,

To grab console output, you could also try to blacklist amdgpu at
startup and load it later.

Can you provide your acpidump (and lspci -nn, especially if you do not
have a dmesg).

This is the call chain:

+ amdgpu_atpx_detect
  + amdgpu_atpx_pci_probe_handle
    + return if ATPX ACPI handle cannot be found
    + set amdgpu_atpx_priv.bridge_pm_usable
  + amdgpu_atpx_init
    + amdgpu_atpx_validate
      + use amdgpu_atpx_priv.bridge_pm_usable

I wonder whether newer devices remove ATPX completely. If that is the
case (please provide acpidump!), can you try this hack:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 951addf..5c23382 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
 	status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
 	if (ACPI_FAILURE(status)) {
 		amdgpu_atpx_priv.other_handle = dhandle;
+		amdgpu_atpx_priv.bridge_pm_usable = true;
 		return false;
 	}
 	amdgpu_atpx_priv.dhandle = dhandle;

This is not intended for merging, but just testing a hypothesis.

Kind regards,
Peter

On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> He's some screen shots of the booting screens, apologies they're not that
> clear
> 
> https://imgur.com/a/V7mRA
> 
> On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike@fireburn.co.uk> wrote:
> 
> > Hi
> >
> > The previous working kernel, was the patch before this - reverting fixes it
> >
> > My bios date is quite new 08/05/2016
> >
> > Most of the messages fly straight off the screen, the journal isn't kept
> > and the wifi driver also complains, will see if I can get a slow motion
> > capture of the errors and screen shot them
> >
> > Regards
> >
> > Mike
> >
> > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter@lekensteyn.nl> wrote:
> >
> > Hi Mike,
> >
> > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > > This patch is preventing my laptop from booting, I'm getting D3 error
> > > messages and atom bios stuck messages
> >
> > Can you post a dmesg and acpidump? What was the previous working kernel?
> > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> > Does booting with runpm=0 work?
> >
> > > I mistakenly saw the v2 patch and didn't realise it was for radeon not
> > > amdgpu - this branch and Linus's tree are currently not booting for me
> >
> > What do you mean by this?
> >
> > Kind regards,
> > Peter
> >
> > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> > Alexander.Deucher@amd.com>
> > > wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > > To: Alex Deucher
> > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is
> > > > > unavailable
> > > > >
> > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@lekensteyn.nl>
> > > > wrote:
> > > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or the
> > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should still
> > be
> > > > > > > marked as supported. Otherwise the GPU can fail to power on after
> > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > > >
> > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> > first
> > > > year
> > > > > > > where hybrid graphics platforms using power resources was
> > > > introduced),
> > > > > > > but that seems more risky at this point and would not solve the
> > > > > > > pcie_port_pm=off issue.
> > > > > > >
> > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > > <nayan26deshmukh@gmail.com>
> > > > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > > > > > > ---
> > > > > > > Hi,
> > > > > > >
> > > > > > > This patch is already three weeks old. One alternative idea was
> > > > lowering
> > > > > BIOS
> > > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> > desired
> > > > > effect, I
> > > > > > > do not think that this would help though.
> > > > > >
> > > > > > Thanks for doing this.
> > > > > >
> > > > > > >
> > > > > > > I have also not contacted linux-pci or Mika about lowering the
> > year
> > > > due to
> > > > > the
> > > > > > > lack of a good reason.  Might do it later though once ACPICA bug
> > > > 1333 is
> > > > > sorted
> > > > > > > out such that lowering the year actually has benefits for a
> > Nvidia
> > > > laptop
> > > > > (or if
> > > > > > > some amdgpu problem can be solved by this).
> > > > > > >
> > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > > 2f5af82eeab2 and
> > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > > platform"). There
> > > > > have
> > > > > > > been some ifdef 0's and reverts in between, so I was not sure if
> > > > adding
> > > > > the
> > > > > > > Fixes tag is appropriate.
> > > > > >
> > > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> > attempt to
> > > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work around
> > > > > lack
> > > > > > of upstream ACPI support for D3cold)
> > > > > > which was reverted for 4.9.
> > > > >
> > > > > That patch was already reverted in 4.8 as far as I can see:
> > > > >
> > > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f | grep
> > v4.8
> > > > > v4.8
> > > > >
> > > > > Can you double-check? v4.8 was the first kernel with D3cold support
> > in
> > > > > PCI core.
> > > >
> > > > You are right.  I was thinking d3cold went upstream in 4.9, so yes, we
> > > > should apply this to 4.8.
> > > >
> > > > Alex
> > > >
> > > > >
> > > > > > Series is:
> > > > > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > > > >
> > > > > Thanks!
> > > > >
> > > > > Kind regards,
> > > > > Peter
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Kind regards,
> > > > > > > Peter
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++-
> > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > index 10b5ddf..951addf 100644
> > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > > >
> > > > > > >  static struct amdgpu_atpx_priv {
> > > > > > >         bool atpx_detected;
> > > > > > > +       bool bridge_pm_usable;
> > > > > > >         /* handle for device - and atpx */
> > > > > > >         acpi_handle dhandle;
> > > > > > >         acpi_handle other_handle;
> > > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > > amdgpu_atpx *atpx)
> > > > > > >         atpx->is_hybrid = false;
> > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > > -               atpx->functions.power_cntl = false;
> > > > > > > +               /*
> > > > > > > +                * Disable legacy PM methods only when pcie port
> > PM
> > > > is usable,
> > > > > > > +                * otherwise the device might fail to power off
> > or
> > > > power on.
> > > > > > > +                */
> > > > > > > +               atpx->functions.power_cntl =
> > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > > >                 atpx->is_hybrid = true;
> > > > > > >         }
> > > > > > >
> > > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > > vga_switcheroo_client_id id,
> > > > > > >   */
> > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > >  {
> > > > > > > +       struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > > >         acpi_status status;
> > > > > > >
> > > > > > > @@ -485,6 +491,7 @@ static bool
> > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > >         }
> > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > > parent_pdev->bridge_d3;
> > > > > > >         return true;
> > > > > > >  }
> > > > > > >
> > > > > > > --
> > > > > > > 2.10.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
  2016-11-26 11:38                           ` Peter Wu
@ 2016-11-26 13:43                             ` Mike Lothian
  2016-11-26 13:49                             ` Mike Lothian
  1 sibling, 0 replies; 17+ messages in thread
From: Mike Lothian @ 2016-11-26 13:43 UTC (permalink / raw)
  To: Peter Wu; +Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh


[-- Attachment #1.1: Type: text/plain, Size: 11775 bytes --]

Hi

axion fireburn # acpidump > acpidump
Wrong checksum for FADT!

ACPI Dump attached

axion fireburn # lspci -nn
00:00.0 Host bridge [0600]: Intel Corporation Skylake Host Bridge/DRAM
Registers [8086:1910] (rev 07)
00:01.0 PCI bridge [0604]: Intel Corporation Skylake PCIe Controller (x16)
[8086:1901] (rev 07)
00:02.0 VGA compatible controller [0300]: Intel Corporation HD Graphics 530
[8086:191b] (rev 06)
00:04.0 Signal processing controller [1180]: Intel Corporation Skylake
Processor Thermal Subsystem [8086:1903] (rev 07)
00:14.0 USB controller [0c03]: Intel Corporation Sunrise Point-H USB 3.0
xHCI Controller [8086:a12f] (rev 31)
00:14.2 Signal processing controller [1180]: Intel Corporation Sunrise
Point-H Thermal subsystem [8086:a131] (rev 31)
00:16.0 Communication controller [0780]: Intel Corporation Sunrise Point-H
CSME HECI #1 [8086:a13a] (rev 31)
00:17.0 SATA controller [0106]: Intel Corporation Sunrise Point-H SATA
Controller [AHCI mode] [8086:a103] (rev 31)
00:1c.0 PCI bridge [0604]: Intel Corporation Sunrise Point-H PCI Express
Root Port #1 [8086:a110] (rev f1)
00:1c.4 PCI bridge [0604]: Intel Corporation Sunrise Point-H PCI Express
Root Port #5 [8086:a114] (rev f1)
00:1c.5 PCI bridge [0604]: Intel Corporation Sunrise Point-H PCI Express
Root Port #6 [8086:a115] (rev f1)
00:1c.6 PCI bridge [0604]: Intel Corporation Sunrise Point-H PCI Express
Root Port #7 [8086:a116] (rev f1)
00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-H PCI Express
Root Port #9 [8086:a118] (rev f1)
00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point-H LPC Controller
[8086:a14e] (rev 31)
00:1f.2 Memory controller [0580]: Intel Corporation Sunrise Point-H PMC
[8086:a121] (rev 31)
00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-H HD Audio
[8086:a170] (rev 31)
00:1f.4 SMBus [0c05]: Intel Corporation Sunrise Point-H SMBus [8086:a123]
(rev 31)
01:00.0 Display controller [0380]: Advanced Micro Devices, Inc. [AMD/ATI]
Amethyst XT [Radeon R9 M295X] [1002:6921]
3b:00.0 Ethernet controller [0200]: Qualcomm Atheros Killer E2400 Gigabit
Ethernet Controller [1969:e0a1] (rev 10)
3c:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac
Wireless Network Adapter [168c:003e] (rev 32)
3d:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS5227
PCI Express Card Reader [10ec:5227] (rev 01)
3e:00.0 Non-Volatile memory controller [0108]: Samsung Electronics Co Ltd
NVMe SSD Controller [144d:a802] (rev 01)

I expected an instant lock up when insmodding the modules however that
didn't happen

Please also find the dmesg attached

On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:

Hi Mike,

To grab console output, you could also try to blacklist amdgpu at
startup and load it later.

Can you provide your acpidump (and lspci -nn, especially if you do not
have a dmesg).

This is the call chain:

+ amdgpu_atpx_detect
  + amdgpu_atpx_pci_probe_handle
    + return if ATPX ACPI handle cannot be found
    + set amdgpu_atpx_priv.bridge_pm_usable
  + amdgpu_atpx_init
    + amdgpu_atpx_validate
      + use amdgpu_atpx_priv.bridge_pm_usable

I wonder whether newer devices remove ATPX completely. If that is the
case (please provide acpidump!), can you try this hack:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 951addf..5c23382 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
*pdev)
        status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
        if (ACPI_FAILURE(status)) {
                amdgpu_atpx_priv.other_handle = dhandle;
+               amdgpu_atpx_priv.bridge_pm_usable = true;
                return false;
        }
        amdgpu_atpx_priv.dhandle = dhandle;

This is not intended for merging, but just testing a hypothesis.

Kind regards,
Peter

On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> He's some screen shots of the booting screens, apologies they're not that
> clear
>
> https://imgur.com/a/V7mRA
>
> On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org> wrote:
>
> > Hi
> >
> > The previous working kernel, was the patch before this - reverting
fixes it
> >
> > My bios date is quite new 08/05/2016
> >
> > Most of the messages fly straight off the screen, the journal isn't kept
> > and the wifi driver also complains, will see if I can get a slow motion
> > capture of the errors and screen shot them
> >
> > Regards
> >
> > Mike
> >
> > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> >
> > Hi Mike,
> >
> > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > > This patch is preventing my laptop from booting, I'm getting D3 error
> > > messages and atom bios stuck messages
> >
> > Can you post a dmesg and acpidump? What was the previous working kernel?
> > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> > Does booting with runpm=0 work?
> >
> > > I mistakenly saw the v2 patch and didn't realise it was for radeon not
> > > amdgpu - this branch and Linus's tree are currently not booting for me
> >
> > What do you mean by this?
> >
> > Kind regards,
> > Peter
> >
> > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> > Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > > wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > > To: Alex Deucher
> > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm
is
> > > > > unavailable
> > > > >
> > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > wrote:
> > > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or
the
> > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should
still
> > be
> > > > > > > marked as supported. Otherwise the GPU can fail to power on
after
> > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > > >
> > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> > first
> > > > year
> > > > > > > where hybrid graphics platforms using power resources was
> > > > introduced),
> > > > > > > but that seems more risky at this point and would not solve
the
> > > > > > > pcie_port_pm=off issue.
> > > > > > >
> > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > > ---
> > > > > > > Hi,
> > > > > > >
> > > > > > > This patch is already three weeks old. One alternative idea
was
> > > > lowering
> > > > > BIOS
> > > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> > desired
> > > > > effect, I
> > > > > > > do not think that this would help though.
> > > > > >
> > > > > > Thanks for doing this.
> > > > > >
> > > > > > >
> > > > > > > I have also not contacted linux-pci or Mika about lowering the
> > year
> > > > due to
> > > > > the
> > > > > > > lack of a good reason.  Might do it later though once ACPICA
bug
> > > > 1333 is
> > > > > sorted
> > > > > > > out such that lowering the year actually has benefits for a
> > Nvidia
> > > > laptop
> > > > > (or if
> > > > > > > some amdgpu problem can be solved by this).
> > > > > > >
> > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > > 2f5af82eeab2 and
> > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > > platform"). There
> > > > > have
> > > > > > > been some ifdef 0's and reverts in between, so I was not sure
if
> > > > adding
> > > > > the
> > > > > > > Fixes tag is appropriate.
> > > > > >
> > > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> > attempt to
> > > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
around
> > > > > lack
> > > > > > of upstream ACPI support for D3cold)
> > > > > > which was reverted for 4.9.
> > > > >
> > > > > That patch was already reverted in 4.8 as far as I can see:
> > > > >
> > > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f |
grep
> > v4.8
> > > > > v4.8
> > > > >
> > > > > Can you double-check? v4.8 was the first kernel with D3cold
support
> > in
> > > > > PCI core.
> > > >
> > > > You are right.  I was thinking d3cold went upstream in 4.9, so yes,
we
> > > > should apply this to 4.8.
> > > >
> > > > Alex
> > > >
> > > > >
> > > > > > Series is:
> > > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > > >
> > > > > Thanks!
> > > > >
> > > > > Kind regards,
> > > > > Peter
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Kind regards,
> > > > > > > Peter
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
++++++++-
> > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > index 10b5ddf..951addf 100644
> > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > > >
> > > > > > >  static struct amdgpu_atpx_priv {
> > > > > > >         bool atpx_detected;
> > > > > > > +       bool bridge_pm_usable;
> > > > > > >         /* handle for device - and atpx */
> > > > > > >         acpi_handle dhandle;
> > > > > > >         acpi_handle other_handle;
> > > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > > amdgpu_atpx *atpx)
> > > > > > >         atpx->is_hybrid = false;
> > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > > -               atpx->functions.power_cntl = false;
> > > > > > > +               /*
> > > > > > > +                * Disable legacy PM methods only when pcie
port
> > PM
> > > > is usable,
> > > > > > > +                * otherwise the device might fail to power
off
> > or
> > > > power on.
> > > > > > > +                */
> > > > > > > +               atpx->functions.power_cntl =
> > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > > >                 atpx->is_hybrid = true;
> > > > > > >         }
> > > > > > >
> > > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > > vga_switcheroo_client_id id,
> > > > > > >   */
> > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
*pdev)
> > > > > > >  {
> > > > > > > +       struct pci_dev *parent_pdev =
pci_upstream_bridge(pdev);
> > > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > > >         acpi_status status;
> > > > > > >
> > > > > > > @@ -485,6 +491,7 @@ static bool
> > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > >         }
> > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > > parent_pdev->bridge_d3;
> > > > > > >         return true;
> > > > > > >  }
> > > > > > >
> > > > > > > --
> > > > > > > 2.10.2

[-- Attachment #1.2: Type: text/html, Size: 23819 bytes --]

[-- Attachment #2: acpidump.xz --]
[-- Type: application/octet-stream, Size: 153088 bytes --]

[-- Attachment #3: dmesg.amdgpu.xz --]
[-- Type: application/x-xz, Size: 14956 bytes --]

[-- Attachment #4: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
  2016-11-26 11:38                           ` Peter Wu
  2016-11-26 13:43                             ` Mike Lothian
@ 2016-11-26 13:49                             ` Mike Lothian
       [not found]                               ` <CAHbf0-GjBMb2GfFoTADxYjfuLU=+hkJ8AcZCbh70zrpn56cD8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Mike Lothian @ 2016-11-26 13:49 UTC (permalink / raw)
  To: Peter Wu; +Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh


[-- Attachment #1.1: Type: text/plain, Size: 9781 bytes --]

I can confirm the patch allows my machine to boot again

On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:

> Hi Mike,
>
> To grab console output, you could also try to blacklist amdgpu at
> startup and load it later.
>
> Can you provide your acpidump (and lspci -nn, especially if you do not
> have a dmesg).
>
> This is the call chain:
>
> + amdgpu_atpx_detect
>   + amdgpu_atpx_pci_probe_handle
>     + return if ATPX ACPI handle cannot be found
>     + set amdgpu_atpx_priv.bridge_pm_usable
>   + amdgpu_atpx_init
>     + amdgpu_atpx_validate
>       + use amdgpu_atpx_priv.bridge_pm_usable
>
> I wonder whether newer devices remove ATPX completely. If that is the
> case (please provide acpidump!), can you try this hack:
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index 951addf..5c23382 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct
> pci_dev *pdev)
>         status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
>         if (ACPI_FAILURE(status)) {
>                 amdgpu_atpx_priv.other_handle = dhandle;
> +               amdgpu_atpx_priv.bridge_pm_usable = true;
>                 return false;
>         }
>         amdgpu_atpx_priv.dhandle = dhandle;
>
> This is not intended for merging, but just testing a hypothesis.
>
> Kind regards,
> Peter
>
> On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> > He's some screen shots of the booting screens, apologies they're not that
> > clear
> >
> > https://imgur.com/a/V7mRA
> >
> > On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org> wrote:
> >
> > > Hi
> > >
> > > The previous working kernel, was the patch before this - reverting
> fixes it
> > >
> > > My bios date is quite new 08/05/2016
> > >
> > > Most of the messages fly straight off the screen, the journal isn't
> kept
> > > and the wifi driver also complains, will see if I can get a slow motion
> > > capture of the errors and screen shot them
> > >
> > > Regards
> > >
> > > Mike
> > >
> > > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> > >
> > > Hi Mike,
> > >
> > > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > > > This patch is preventing my laptop from booting, I'm getting D3 error
> > > > messages and atom bios stuck messages
> > >
> > > Can you post a dmesg and acpidump? What was the previous working
> kernel?
> > > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> > > Does booting with runpm=0 work?
> > >
> > > > I mistakenly saw the v2 patch and didn't realise it was for radeon
> not
> > > > amdgpu - this branch and Linus's tree are currently not booting for
> me
> > >
> > > What do you mean by this?
> > >
> > > Kind regards,
> > > Peter
> > >
> > > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> > > Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > > > wrote:
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > > > To: Alex Deucher
> > > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port
> pm is
> > > > > > unavailable
> > > > > >
> > > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <
> peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > wrote:
> > > > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or
> the
> > > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should
> still
> > > be
> > > > > > > > marked as supported. Otherwise the GPU can fail to power on
> after
> > > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > > > >
> > > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> > > first
> > > > > year
> > > > > > > > where hybrid graphics platforms using power resources was
> > > > > introduced),
> > > > > > > > but that seems more risky at this point and would not solve
> the
> > > > > > > > pcie_port_pm=off issue.
> > > > > > > >
> > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > > > ---
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > This patch is already three weeks old. One alternative idea
> was
> > > > > lowering
> > > > > > BIOS
> > > > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> > > desired
> > > > > > effect, I
> > > > > > > > do not think that this would help though.
> > > > > > >
> > > > > > > Thanks for doing this.
> > > > > > >
> > > > > > > >
> > > > > > > > I have also not contacted linux-pci or Mika about lowering
> the
> > > year
> > > > > due to
> > > > > > the
> > > > > > > > lack of a good reason.  Might do it later though once ACPICA
> bug
> > > > > 1333 is
> > > > > > sorted
> > > > > > > > out such that lowering the year actually has benefits for a
> > > Nvidia
> > > > > laptop
> > > > > > (or if
> > > > > > > > some amdgpu problem can be solved by this).
> > > > > > > >
> > > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > > > 2f5af82eeab2 and
> > > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > > > platform"). There
> > > > > > have
> > > > > > > > been some ifdef 0's and reverts in between, so I was not
> sure if
> > > > > adding
> > > > > > the
> > > > > > > > Fixes tag is appropriate.
> > > > > > >
> > > > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> > > attempt to
> > > > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
> around
> > > > > > lack
> > > > > > > of upstream ACPI support for D3cold)
> > > > > > > which was reverted for 4.9.
> > > > > >
> > > > > > That patch was already reverted in 4.8 as far as I can see:
> > > > > >
> > > > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f |
> grep
> > > v4.8
> > > > > > v4.8
> > > > > >
> > > > > > Can you double-check? v4.8 was the first kernel with D3cold
> support
> > > in
> > > > > > PCI core.
> > > > >
> > > > > You are right.  I was thinking d3cold went upstream in 4.9, so
> yes, we
> > > > > should apply this to 4.8.
> > > > >
> > > > > Alex
> > > > >
> > > > > >
> > > > > > > Series is:
> > > > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > > > >
> > > > > > Thanks!
> > > > > >
> > > > > > Kind regards,
> > > > > > Peter
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Kind regards,
> > > > > > > > Peter
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
> ++++++++-
> > > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > index 10b5ddf..951addf 100644
> > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > > > >
> > > > > > > >  static struct amdgpu_atpx_priv {
> > > > > > > >         bool atpx_detected;
> > > > > > > > +       bool bridge_pm_usable;
> > > > > > > >         /* handle for device - and atpx */
> > > > > > > >         acpi_handle dhandle;
> > > > > > > >         acpi_handle other_handle;
> > > > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > > > amdgpu_atpx *atpx)
> > > > > > > >         atpx->is_hybrid = false;
> > > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > > > -               atpx->functions.power_cntl = false;
> > > > > > > > +               /*
> > > > > > > > +                * Disable legacy PM methods only when pcie
> port
> > > PM
> > > > > is usable,
> > > > > > > > +                * otherwise the device might fail to power
> off
> > > or
> > > > > power on.
> > > > > > > > +                */
> > > > > > > > +               atpx->functions.power_cntl =
> > > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > > > >                 atpx->is_hybrid = true;
> > > > > > > >         }
> > > > > > > >
> > > > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > > > vga_switcheroo_client_id id,
> > > > > > > >   */
> > > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
> *pdev)
> > > > > > > >  {
> > > > > > > > +       struct pci_dev *parent_pdev =
> pci_upstream_bridge(pdev);
> > > > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > > > >         acpi_status status;
> > > > > > > >
> > > > > > > > @@ -485,6 +491,7 @@ static bool
> > > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > > >         }
> > > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > > > parent_pdev->bridge_d3;
> > > > > > > >         return true;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > --
> > > > > > > > 2.10.2
>

[-- Attachment #1.2: Type: text/html, Size: 19423 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]                               ` <CAHbf0-GjBMb2GfFoTADxYjfuLU=+hkJ8AcZCbh70zrpn56cD8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-26 14:14                                 ` Peter Wu
  2016-11-28  5:48                                   ` Mike Lothian
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Wu @ 2016-11-26 14:14 UTC (permalink / raw)
  To: Mike Lothian
  Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh

[-- Attachment #1: Type: text/plain, Size: 10860 bytes --]

Found the problem.

amdgpu_atpx_pci_probe_handle is used to detect the ATPX handle, but this
does not necessarily have to be on the dGPU. In fact, on your machine it
is located on the Intel iGPU (00:02.0):

[  134.760509] vga_switcheroo: detected switching method \_SB_.PCI0.GFX0.ATPX handle

The parent device of the iGPU (00:00.0) is the root port which has no
PCIe capability, hence bridge_d3 evaluated to false.

Can you try the attached patch?

Kind regards,
Peter

On Sat, Nov 26, 2016 at 01:49:08PM +0000, Mike Lothian wrote:
> I can confirm the patch allows my machine to boot again
> 
> On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> 
> > Hi Mike,
> >
> > To grab console output, you could also try to blacklist amdgpu at
> > startup and load it later.
> >
> > Can you provide your acpidump (and lspci -nn, especially if you do not
> > have a dmesg).
> >
> > This is the call chain:
> >
> > + amdgpu_atpx_detect
> >   + amdgpu_atpx_pci_probe_handle
> >     + return if ATPX ACPI handle cannot be found
> >     + set amdgpu_atpx_priv.bridge_pm_usable
> >   + amdgpu_atpx_init
> >     + amdgpu_atpx_validate
> >       + use amdgpu_atpx_priv.bridge_pm_usable
> >
> > I wonder whether newer devices remove ATPX completely. If that is the
> > case (please provide acpidump!), can you try this hack:
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > index 951addf..5c23382 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > @@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct
> > pci_dev *pdev)
> >         status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
> >         if (ACPI_FAILURE(status)) {
> >                 amdgpu_atpx_priv.other_handle = dhandle;
> > +               amdgpu_atpx_priv.bridge_pm_usable = true;
> >                 return false;
> >         }
> >         amdgpu_atpx_priv.dhandle = dhandle;
> >
> > This is not intended for merging, but just testing a hypothesis.
> >
> > Kind regards,
> > Peter
> >
> > On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> > > He's some screen shots of the booting screens, apologies they're not that
> > > clear
> > >
> > > https://imgur.com/a/V7mRA
> > >
> > > On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org> wrote:
> > >
> > > > Hi
> > > >
> > > > The previous working kernel, was the patch before this - reverting
> > fixes it
> > > >
> > > > My bios date is quite new 08/05/2016
> > > >
> > > > Most of the messages fly straight off the screen, the journal isn't
> > kept
> > > > and the wifi driver also complains, will see if I can get a slow motion
> > > > capture of the errors and screen shot them
> > > >
> > > > Regards
> > > >
> > > > Mike
> > > >
> > > > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> > > >
> > > > Hi Mike,
> > > >
> > > > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > > > > This patch is preventing my laptop from booting, I'm getting D3 error
> > > > > messages and atom bios stuck messages
> > > >
> > > > Can you post a dmesg and acpidump? What was the previous working
> > kernel?
> > > > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> > > > Does booting with runpm=0 work?
> > > >
> > > > > I mistakenly saw the v2 patch and didn't realise it was for radeon
> > not
> > > > > amdgpu - this branch and Linus's tree are currently not booting for
> > me
> > > >
> > > > What do you mean by this?
> > > >
> > > > Kind regards,
> > > > Peter
> > > >
> > > > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> > > > Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > > > > wrote:
> > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > > > > To: Alex Deucher
> > > > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port
> > pm is
> > > > > > > unavailable
> > > > > > >
> > > > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <
> > peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > wrote:
> > > > > > > > > When PCIe port PM is not enabled (system BIOS is pre-2015 or
> > the
> > > > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should
> > still
> > > > be
> > > > > > > > > marked as supported. Otherwise the GPU can fail to power on
> > after
> > > > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > > > > >
> > > > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013 (the
> > > > first
> > > > > > year
> > > > > > > > > where hybrid graphics platforms using power resources was
> > > > > > introduced),
> > > > > > > > > but that seems more risky at this point and would not solve
> > the
> > > > > > > > > pcie_port_pm=off issue.
> > > > > > > > >
> > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > > > > ---
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > This patch is already three weeks old. One alternative idea
> > was
> > > > > > lowering
> > > > > > > BIOS
> > > > > > > > > date in PCI core, but as pcie_port_pm=force did not have the
> > > > desired
> > > > > > > effect, I
> > > > > > > > > do not think that this would help though.
> > > > > > > >
> > > > > > > > Thanks for doing this.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > I have also not contacted linux-pci or Mika about lowering
> > the
> > > > year
> > > > > > due to
> > > > > > > the
> > > > > > > > > lack of a good reason.  Might do it later though once ACPICA
> > bug
> > > > > > 1333 is
> > > > > > > sorted
> > > > > > > > > out such that lowering the year actually has benefits for a
> > > > Nvidia
> > > > > > laptop
> > > > > > > (or if
> > > > > > > > > some amdgpu problem can be solved by this).
> > > > > > > > >
> > > > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > > > > 2f5af82eeab2 and
> > > > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > > > > platform"). There
> > > > > > > have
> > > > > > > > > been some ifdef 0's and reverts in between, so I was not
> > sure if
> > > > > > adding
> > > > > > > the
> > > > > > > > > Fixes tag is appropriate.
> > > > > > > >
> > > > > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> > > > attempt to
> > > > > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
> > around
> > > > > > > lack
> > > > > > > > of upstream ACPI support for D3cold)
> > > > > > > > which was reverted for 4.9.
> > > > > > >
> > > > > > > That patch was already reverted in 4.8 as far as I can see:
> > > > > > >
> > > > > > > $ git tag --contains c39b487f195b93235ee76384427467786f7bf29f |
> > grep
> > > > v4.8
> > > > > > > v4.8
> > > > > > >
> > > > > > > Can you double-check? v4.8 was the first kernel with D3cold
> > support
> > > > in
> > > > > > > PCI core.
> > > > > >
> > > > > > You are right.  I was thinking d3cold went upstream in 4.9, so
> > yes, we
> > > > > > should apply this to 4.8.
> > > > > >
> > > > > > Alex
> > > > > >
> > > > > > >
> > > > > > > > Series is:
> > > > > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > > > > >
> > > > > > > Thanks!
> > > > > > >
> > > > > > > Kind regards,
> > > > > > > Peter
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Kind regards,
> > > > > > > > > Peter
> > > > > > > > > ---
> > > > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
> > ++++++++-
> > > > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > index 10b5ddf..951addf 100644
> > > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > > > > >
> > > > > > > > >  static struct amdgpu_atpx_priv {
> > > > > > > > >         bool atpx_detected;
> > > > > > > > > +       bool bridge_pm_usable;
> > > > > > > > >         /* handle for device - and atpx */
> > > > > > > > >         acpi_handle dhandle;
> > > > > > > > >         acpi_handle other_handle;
> > > > > > > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct
> > > > > > > amdgpu_atpx *atpx)
> > > > > > > > >         atpx->is_hybrid = false;
> > > > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > > > > -               atpx->functions.power_cntl = false;
> > > > > > > > > +               /*
> > > > > > > > > +                * Disable legacy PM methods only when pcie
> > port
> > > > PM
> > > > > > is usable,
> > > > > > > > > +                * otherwise the device might fail to power
> > off
> > > > or
> > > > > > power on.
> > > > > > > > > +                */
> > > > > > > > > +               atpx->functions.power_cntl =
> > > > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > > > > >                 atpx->is_hybrid = true;
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum
> > > > > > > vga_switcheroo_client_id id,
> > > > > > > > >   */
> > > > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
> > *pdev)
> > > > > > > > >  {
> > > > > > > > > +       struct pci_dev *parent_pdev =
> > pci_upstream_bridge(pdev);
> > > > > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > > > > >         acpi_status status;
> > > > > > > > >
> > > > > > > > > @@ -485,6 +491,7 @@ static bool
> > > > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > > > >         }
> > > > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev &&
> > > > > > > parent_pdev->bridge_d3;
> > > > > > > > >         return true;
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > 2.10.2

[-- Attachment #2: 0001-drm-amdgpu-fix-check-for-port-PM-availability.patch --]
[-- Type: text/x-diff, Size: 2743 bytes --]

>From 483607ff5c5dc6932e2f854d205cf41f9fbf26c8 Mon Sep 17 00:00:00 2001
From: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
Date: Sat, 26 Nov 2016 15:05:01 +0100
Subject: [PATCH] drm/amdgpu: fix check for port PM availability

The ATPX method does not always exist on the dGPU, it may be located at
the iGPU. The parent device of the iGPU is the root port for which
bridge_d3 is false. This accidentally enables the legacy PM method which
conflicts with port PM and prevented the dGPU from powering on.

Fixes: 1db4496f167b ("drm/amdgpu: fix power state when port pm is unavailable")
Reported-by: Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org>
Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 951addf..1ed085f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -476,7 +476,6 @@ static int amdgpu_atpx_power_state(enum vga_switcheroo_client_id id,
  */
 static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
 	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
@@ -491,7 +490,6 @@ static bool amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
 	}
 	amdgpu_atpx_priv.dhandle = dhandle;
 	amdgpu_atpx_priv.atpx.handle = atpx_handle;
-	amdgpu_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
 	return true;
 }
 
@@ -553,17 +551,25 @@ static bool amdgpu_atpx_detect(void)
 	struct pci_dev *pdev = NULL;
 	bool has_atpx = false;
 	int vga_count = 0;
+	bool d3_supported = false;
+	struct pci_dev *parent_pdev;
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
 		vga_count++;
 
 		has_atpx |= (amdgpu_atpx_pci_probe_handle(pdev) == true);
+
+		parent_pdev = pci_upstream_bridge(pdev);
+		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_OTHER << 8, pdev)) != NULL) {
 		vga_count++;
 
 		has_atpx |= (amdgpu_atpx_pci_probe_handle(pdev) == true);
+
+		parent_pdev = pci_upstream_bridge(pdev);
+		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
 	}
 
 	if (has_atpx && vga_count == 2) {
@@ -571,6 +577,7 @@ static bool amdgpu_atpx_detect(void)
 		printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
 		       acpi_method_name);
 		amdgpu_atpx_priv.atpx_detected = true;
+		amdgpu_atpx_priv.bridge_pm_usable = d3_supported;
 		amdgpu_atpx_init();
 		return true;
 	}
-- 
2.10.2


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
  2016-11-26 14:14                                 ` Peter Wu
@ 2016-11-28  5:48                                   ` Mike Lothian
       [not found]                                     ` <CAHbf0-HzBPXOqS_XLctTu-eTuxUUwP62_TcODVPwKb8T2PFrFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Mike Lothian @ 2016-11-28  5:48 UTC (permalink / raw)
  To: Peter Wu; +Cc: Deucher, Alexander, Alex Deucher, amd-gfx list, Nayan Deshmukh


[-- Attachment #1.1: Type: text/plain, Size: 11645 bytes --]

The attached patch allows my machine to boot also

Thanks

Mike

On Sat, 26 Nov 2016 at 14:14 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:

> Found the problem.
>
> amdgpu_atpx_pci_probe_handle is used to detect the ATPX handle, but this
> does not necessarily have to be on the dGPU. In fact, on your machine it
> is located on the Intel iGPU (00:02.0):
>
> [  134.760509] vga_switcheroo: detected switching method
> \_SB_.PCI0.GFX0.ATPX handle
>
> The parent device of the iGPU (00:00.0) is the root port which has no
> PCIe capability, hence bridge_d3 evaluated to false.
>
> Can you try the attached patch?
>
> Kind regards,
> Peter
>
> On Sat, Nov 26, 2016 at 01:49:08PM +0000, Mike Lothian wrote:
> > I can confirm the patch allows my machine to boot again
> >
> > On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> >
> > > Hi Mike,
> > >
> > > To grab console output, you could also try to blacklist amdgpu at
> > > startup and load it later.
> > >
> > > Can you provide your acpidump (and lspci -nn, especially if you do not
> > > have a dmesg).
> > >
> > > This is the call chain:
> > >
> > > + amdgpu_atpx_detect
> > >   + amdgpu_atpx_pci_probe_handle
> > >     + return if ATPX ACPI handle cannot be found
> > >     + set amdgpu_atpx_priv.bridge_pm_usable
> > >   + amdgpu_atpx_init
> > >     + amdgpu_atpx_validate
> > >       + use amdgpu_atpx_priv.bridge_pm_usable
> > >
> > > I wonder whether newer devices remove ATPX completely. If that is the
> > > case (please provide acpidump!), can you try this hack:
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > index 951addf..5c23382 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > @@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct
> > > pci_dev *pdev)
> > >         status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
> > >         if (ACPI_FAILURE(status)) {
> > >                 amdgpu_atpx_priv.other_handle = dhandle;
> > > +               amdgpu_atpx_priv.bridge_pm_usable = true;
> > >                 return false;
> > >         }
> > >         amdgpu_atpx_priv.dhandle = dhandle;
> > >
> > > This is not intended for merging, but just testing a hypothesis.
> > >
> > > Kind regards,
> > > Peter
> > >
> > > On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> > > > He's some screen shots of the booting screens, apologies they're not
> that
> > > > clear
> > > >
> > > > https://imgur.com/a/V7mRA
> > > >
> > > > On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org>
> wrote:
> > > >
> > > > > Hi
> > > > >
> > > > > The previous working kernel, was the patch before this - reverting
> > > fixes it
> > > > >
> > > > > My bios date is quite new 08/05/2016
> > > > >
> > > > > Most of the messages fly straight off the screen, the journal isn't
> > > kept
> > > > > and the wifi driver also complains, will see if I can get a slow
> motion
> > > > > capture of the errors and screen shot them
> > > > >
> > > > > Regards
> > > > >
> > > > > Mike
> > > > >
> > > > > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
> > > > >
> > > > > Hi Mike,
> > > > >
> > > > > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> > > > > > This patch is preventing my laptop from booting, I'm getting D3
> error
> > > > > > messages and atom bios stuck messages
> > > > >
> > > > > Can you post a dmesg and acpidump? What was the previous working
> > > kernel?
> > > > > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is pre-2015?
> > > > > Does booting with runpm=0 work?
> > > > >
> > > > > > I mistakenly saw the v2 patch and didn't realise it was for
> radeon
> > > not
> > > > > > amdgpu - this branch and Linus's tree are currently not booting
> for
> > > me
> > > > >
> > > > > What do you mean by this?
> > > > >
> > > > > Kind regards,
> > > > > Peter
> > > > >
> > > > > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> > > > > Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
> > > > > > wrote:
> > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
> > > > > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> > > > > > > > To: Alex Deucher
> > > > > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> > > > > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when
> port
> > > pm is
> > > > > > > > unavailable
> > > > > > > >
> > > > > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher wrote:
> > > > > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <
> > > peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > > wrote:
> > > > > > > > > > When PCIe port PM is not enabled (system BIOS is
> pre-2015 or
> > > the
> > > > > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM should
> > > still
> > > > > be
> > > > > > > > > > marked as supported. Otherwise the GPU can fail to power
> on
> > > after
> > > > > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> > > > > > > > > >
> > > > > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013
> (the
> > > > > first
> > > > > > > year
> > > > > > > > > > where hybrid graphics platforms using power resources was
> > > > > > > introduced),
> > > > > > > > > > but that seems more risky at this point and would not
> solve
> > > the
> > > > > > > > > > pcie_port_pm=off issue.
> > > > > > > > > >
> > > > > > > > > > Bugzilla:
> https://bugs.freedesktop.org/show_bug.cgi?id=98505
> > > > > > > > > > Reported-and-tested-by: Nayan Deshmukh
> > > > > > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
> > > > > > > > > > ---
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > This patch is already three weeks old. One alternative
> idea
> > > was
> > > > > > > lowering
> > > > > > > > BIOS
> > > > > > > > > > date in PCI core, but as pcie_port_pm=force did not have
> the
> > > > > desired
> > > > > > > > effect, I
> > > > > > > > > > do not think that this would help though.
> > > > > > > > >
> > > > > > > > > Thanks for doing this.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I have also not contacted linux-pci or Mika about
> lowering
> > > the
> > > > > year
> > > > > > > due to
> > > > > > > > the
> > > > > > > > > > lack of a good reason.  Might do it later though once
> ACPICA
> > > bug
> > > > > > > 1333 is
> > > > > > > > sorted
> > > > > > > > > > out such that lowering the year actually has benefits
> for a
> > > > > Nvidia
> > > > > > > laptop
> > > > > > > > (or if
> > > > > > > > > > some amdgpu problem can be solved by this).
> > > > > > > > > >
> > > > > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> > > > > > > 2f5af82eeab2 and
> > > > > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphics
> > > > > > > platform"). There
> > > > > > > > have
> > > > > > > > > > been some ifdef 0's and reverts in between, so I was not
> > > sure if
> > > > > > > adding
> > > > > > > > the
> > > > > > > > > > Fixes tag is appropriate.
> > > > > > > > >
> > > > > > > > > I don't think we need to cc stable.  In kernel 4.8 we don't
> > > > > attempt to
> > > > > > > > > do d3cold at all.  4.8 and older kernels have:
> > > > > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
> > > around
> > > > > > > > lack
> > > > > > > > > of upstream ACPI support for D3cold)
> > > > > > > > > which was reverted for 4.9.
> > > > > > > >
> > > > > > > > That patch was already reverted in 4.8 as far as I can see:
> > > > > > > >
> > > > > > > > $ git tag --contains
> c39b487f195b93235ee76384427467786f7bf29f |
> > > grep
> > > > > v4.8
> > > > > > > > v4.8
> > > > > > > >
> > > > > > > > Can you double-check? v4.8 was the first kernel with D3cold
> > > support
> > > > > in
> > > > > > > > PCI core.
> > > > > > >
> > > > > > > You are right.  I was thinking d3cold went upstream in 4.9, so
> > > yes, we
> > > > > > > should apply this to 4.8.
> > > > > > >
> > > > > > > Alex
> > > > > > >
> > > > > > > >
> > > > > > > > > Series is:
> > > > > > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> > > > > > > >
> > > > > > > > Thanks!
> > > > > > > >
> > > > > > > > Kind regards,
> > > > > > > > Peter
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Kind regards,
> > > > > > > > > > Peter
> > > > > > > > > > ---
> > > > > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
> > > ++++++++-
> > > > > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > > > > > >
> > > > > > > > > > diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > > index 10b5ddf..951addf 100644
> > > > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> > > > > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> > > > > > > > > >
> > > > > > > > > >  static struct amdgpu_atpx_priv {
> > > > > > > > > >         bool atpx_detected;
> > > > > > > > > > +       bool bridge_pm_usable;
> > > > > > > > > >         /* handle for device - and atpx */
> > > > > > > > > >         acpi_handle dhandle;
> > > > > > > > > >         acpi_handle other_handle;
> > > > > > > > > > @@ -200,7 +201,11 @@ static int
> amdgpu_atpx_validate(struct
> > > > > > > > amdgpu_atpx *atpx)
> > > > > > > > > >         atpx->is_hybrid = false;
> > > > > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> > > > > > > > > >                 printk("ATPX Hybrid Graphics\n");
> > > > > > > > > > -               atpx->functions.power_cntl = false;
> > > > > > > > > > +               /*
> > > > > > > > > > +                * Disable legacy PM methods only when
> pcie
> > > port
> > > > > PM
> > > > > > > is usable,
> > > > > > > > > > +                * otherwise the device might fail to
> power
> > > off
> > > > > or
> > > > > > > power on.
> > > > > > > > > > +                */
> > > > > > > > > > +               atpx->functions.power_cntl =
> > > > > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> > > > > > > > > >                 atpx->is_hybrid = true;
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > > @@ -471,6 +476,7 @@ static int
> amdgpu_atpx_power_state(enum
> > > > > > > > vga_switcheroo_client_id id,
> > > > > > > > > >   */
> > > > > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
> > > *pdev)
> > > > > > > > > >  {
> > > > > > > > > > +       struct pci_dev *parent_pdev =
> > > pci_upstream_bridge(pdev);
> > > > > > > > > >         acpi_handle dhandle, atpx_handle;
> > > > > > > > > >         acpi_status status;
> > > > > > > > > >
> > > > > > > > > > @@ -485,6 +491,7 @@ static bool
> > > > > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > > > > > > > > >         }
> > > > > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> > > > > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> > > > > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev
> &&
> > > > > > > > parent_pdev->bridge_d3;
> > > > > > > > > >         return true;
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > 2.10.2
>

[-- Attachment #1.2: Type: text/html, Size: 24040 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]                                     ` <CAHbf0-HzBPXOqS_XLctTu-eTuxUUwP62_TcODVPwKb8T2PFrFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-28 22:28                                       ` Alex Deucher
       [not found]                                         ` <CADnq5_MfLP+Ym+beD+Oak+CTxvyMVYfb9+DQDZuR9pbBWqwQtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2016-11-28 22:28 UTC (permalink / raw)
  To: Mike Lothian; +Cc: Deucher, Alexander, Nayan Deshmukh, Peter Wu, amd-gfx list

[-- Attachment #1: Type: text/plain, Size: 12622 bytes --]

On Mon, Nov 28, 2016 at 12:48 AM, Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org> wrote:
> The attached patch allows my machine to boot also
>
> Thanks
>
> Mike
>
> On Sat, 26 Nov 2016 at 14:14 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
>>
>> Found the problem.
>>
>> amdgpu_atpx_pci_probe_handle is used to detect the ATPX handle, but this
>> does not necessarily have to be on the dGPU. In fact, on your machine it
>> is located on the Intel iGPU (00:02.0):
>>
>> [  134.760509] vga_switcheroo: detected switching method
>> \_SB_.PCI0.GFX0.ATPX handle
>>
>> The parent device of the iGPU (00:00.0) is the root port which has no
>> PCIe capability, hence bridge_d3 evaluated to false.
>>
>> Can you try the attached patch?

Looks good to me.  Patch for radeon included as well.

Alex

>>
>> Kind regards,
>> Peter
>>
>> On Sat, Nov 26, 2016 at 01:49:08PM +0000, Mike Lothian wrote:
>> > I can confirm the patch allows my machine to boot again
>> >
>> > On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
>> >
>> > > Hi Mike,
>> > >
>> > > To grab console output, you could also try to blacklist amdgpu at
>> > > startup and load it later.
>> > >
>> > > Can you provide your acpidump (and lspci -nn, especially if you do not
>> > > have a dmesg).
>> > >
>> > > This is the call chain:
>> > >
>> > > + amdgpu_atpx_detect
>> > >   + amdgpu_atpx_pci_probe_handle
>> > >     + return if ATPX ACPI handle cannot be found
>> > >     + set amdgpu_atpx_priv.bridge_pm_usable
>> > >   + amdgpu_atpx_init
>> > >     + amdgpu_atpx_validate
>> > >       + use amdgpu_atpx_priv.bridge_pm_usable
>> > >
>> > > I wonder whether newer devices remove ATPX completely. If that is the
>> > > case (please provide acpidump!), can you try this hack:
>> > >
>> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > index 951addf..5c23382 100644
>> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > @@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct
>> > > pci_dev *pdev)
>> > >         status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
>> > >         if (ACPI_FAILURE(status)) {
>> > >                 amdgpu_atpx_priv.other_handle = dhandle;
>> > > +               amdgpu_atpx_priv.bridge_pm_usable = true;
>> > >                 return false;
>> > >         }
>> > >         amdgpu_atpx_priv.dhandle = dhandle;
>> > >
>> > > This is not intended for merging, but just testing a hypothesis.
>> > >
>> > > Kind regards,
>> > > Peter
>> > >
>> > > On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
>> > > > He's some screen shots of the booting screens, apologies they're not
>> > > > that
>> > > > clear
>> > > >
>> > > > https://imgur.com/a/V7mRA
>> > > >
>> > > > On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike-4+n8WJKc9ve9FHfhHBbuYA@public.gmane.org>
>> > > > wrote:
>> > > >
>> > > > > Hi
>> > > > >
>> > > > > The previous working kernel, was the patch before this - reverting
>> > > fixes it
>> > > > >
>> > > > > My bios date is quite new 08/05/2016
>> > > > >
>> > > > > Most of the messages fly straight off the screen, the journal
>> > > > > isn't
>> > > kept
>> > > > > and the wifi driver also complains, will see if I can get a slow
>> > > > > motion
>> > > > > capture of the errors and screen shot them
>> > > > >
>> > > > > Regards
>> > > > >
>> > > > > Mike
>> > > > >
>> > > > > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org> wrote:
>> > > > >
>> > > > > Hi Mike,
>> > > > >
>> > > > > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
>> > > > > > This patch is preventing my laptop from booting, I'm getting D3
>> > > > > > error
>> > > > > > messages and atom bios stuck messages
>> > > > >
>> > > > > Can you post a dmesg and acpidump? What was the previous working
>> > > kernel?
>> > > > > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is
>> > > > > pre-2015?
>> > > > > Does booting with runpm=0 work?
>> > > > >
>> > > > > > I mistakenly saw the v2 patch and didn't realise it was for
>> > > > > > radeon
>> > > not
>> > > > > > amdgpu - this branch and Linus's tree are currently not booting
>> > > > > > for
>> > > me
>> > > > >
>> > > > > What do you mean by this?
>> > > > >
>> > > > > Kind regards,
>> > > > > Peter
>> > > > >
>> > > > > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
>> > > > > Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > > -----Original Message-----
>> > > > > > > > From: Peter Wu [mailto:peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org]
>> > > > > > > > Sent: Wednesday, November 23, 2016 12:13 PM
>> > > > > > > > To: Alex Deucher
>> > > > > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
>> > > > > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when
>> > > > > > > > port
>> > > pm is
>> > > > > > > > unavailable
>> > > > > > > >
>> > > > > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher
>> > > > > > > > wrote:
>> > > > > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <
>> > > peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
>> > > > > > > wrote:
>> > > > > > > > > > When PCIe port PM is not enabled (system BIOS is
>> > > > > > > > > > pre-2015 or
>> > > the
>> > > > > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM
>> > > > > > > > > > should
>> > > still
>> > > > > be
>> > > > > > > > > > marked as supported. Otherwise the GPU can fail to power
>> > > > > > > > > > on
>> > > after
>> > > > > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
>> > > > > > > > > >
>> > > > > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013
>> > > > > > > > > > (the
>> > > > > first
>> > > > > > > year
>> > > > > > > > > > where hybrid graphics platforms using power resources
>> > > > > > > > > > was
>> > > > > > > introduced),
>> > > > > > > > > > but that seems more risky at this point and would not
>> > > > > > > > > > solve
>> > > the
>> > > > > > > > > > pcie_port_pm=off issue.
>> > > > > > > > > >
>> > > > > > > > > > Bugzilla:
>> > > > > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=98505
>> > > > > > > > > > Reported-and-tested-by: Nayan Deshmukh
>> > > > > > > > <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > > > > > > > > > Signed-off-by: Peter Wu <peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
>> > > > > > > > > > ---
>> > > > > > > > > > Hi,
>> > > > > > > > > >
>> > > > > > > > > > This patch is already three weeks old. One alternative
>> > > > > > > > > > idea
>> > > was
>> > > > > > > lowering
>> > > > > > > > BIOS
>> > > > > > > > > > date in PCI core, but as pcie_port_pm=force did not have
>> > > > > > > > > > the
>> > > > > desired
>> > > > > > > > effect, I
>> > > > > > > > > > do not think that this would help though.
>> > > > > > > > >
>> > > > > > > > > Thanks for doing this.
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > I have also not contacted linux-pci or Mika about
>> > > > > > > > > > lowering
>> > > the
>> > > > > year
>> > > > > > > due to
>> > > > > > > > the
>> > > > > > > > > > lack of a good reason.  Might do it later though once
>> > > > > > > > > > ACPICA
>> > > bug
>> > > > > > > 1333 is
>> > > > > > > > sorted
>> > > > > > > > > > out such that lowering the year actually has benefits
>> > > > > > > > > > for a
>> > > > > Nvidia
>> > > > > > > laptop
>> > > > > > > > (or if
>> > > > > > > > > > some amdgpu problem can be solved by this).
>> > > > > > > > > >
>> > > > > > > > > > Both patches should probably be Cc stable (4.8+), fixing
>> > > > > > > 2f5af82eeab2 and
>> > > > > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid
>> > > > > > > > > > graphics
>> > > > > > > platform"). There
>> > > > > > > > have
>> > > > > > > > > > been some ifdef 0's and reverts in between, so I was not
>> > > sure if
>> > > > > > > adding
>> > > > > > > > the
>> > > > > > > > > > Fixes tag is appropriate.
>> > > > > > > > >
>> > > > > > > > > I don't think we need to cc stable.  In kernel 4.8 we
>> > > > > > > > > don't
>> > > > > attempt to
>> > > > > > > > > do d3cold at all.  4.8 and older kernels have:
>> > > > > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
>> > > around
>> > > > > > > > lack
>> > > > > > > > > of upstream ACPI support for D3cold)
>> > > > > > > > > which was reverted for 4.9.
>> > > > > > > >
>> > > > > > > > That patch was already reverted in 4.8 as far as I can see:
>> > > > > > > >
>> > > > > > > > $ git tag --contains
>> > > > > > > > c39b487f195b93235ee76384427467786f7bf29f |
>> > > grep
>> > > > > v4.8
>> > > > > > > > v4.8
>> > > > > > > >
>> > > > > > > > Can you double-check? v4.8 was the first kernel with D3cold
>> > > support
>> > > > > in
>> > > > > > > > PCI core.
>> > > > > > >
>> > > > > > > You are right.  I was thinking d3cold went upstream in 4.9, so
>> > > yes, we
>> > > > > > > should apply this to 4.8.
>> > > > > > >
>> > > > > > > Alex
>> > > > > > >
>> > > > > > > >
>> > > > > > > > > Series is:
>> > > > > > > > > Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
>> > > > > > > >
>> > > > > > > > Thanks!
>> > > > > > > >
>> > > > > > > > Kind regards,
>> > > > > > > > Peter
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Kind regards,
>> > > > > > > > > > Peter
>> > > > > > > > > > ---
>> > > > > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
>> > > ++++++++-
>> > > > > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
>> > > > > > > > > >
>> > > > > > > > > > diff --git
>> > > > > > > > > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > > > > > > > > index 10b5ddf..951addf 100644
>> > > > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
>> > > > > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
>> > > > > > > > > >
>> > > > > > > > > >  static struct amdgpu_atpx_priv {
>> > > > > > > > > >         bool atpx_detected;
>> > > > > > > > > > +       bool bridge_pm_usable;
>> > > > > > > > > >         /* handle for device - and atpx */
>> > > > > > > > > >         acpi_handle dhandle;
>> > > > > > > > > >         acpi_handle other_handle;
>> > > > > > > > > > @@ -200,7 +201,11 @@ static int
>> > > > > > > > > > amdgpu_atpx_validate(struct
>> > > > > > > > amdgpu_atpx *atpx)
>> > > > > > > > > >         atpx->is_hybrid = false;
>> > > > > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
>> > > > > > > > > >                 printk("ATPX Hybrid Graphics\n");
>> > > > > > > > > > -               atpx->functions.power_cntl = false;
>> > > > > > > > > > +               /*
>> > > > > > > > > > +                * Disable legacy PM methods only when
>> > > > > > > > > > pcie
>> > > port
>> > > > > PM
>> > > > > > > is usable,
>> > > > > > > > > > +                * otherwise the device might fail to
>> > > > > > > > > > power
>> > > off
>> > > > > or
>> > > > > > > power on.
>> > > > > > > > > > +                */
>> > > > > > > > > > +               atpx->functions.power_cntl =
>> > > > > > > > !amdgpu_atpx_priv.bridge_pm_usable;
>> > > > > > > > > >                 atpx->is_hybrid = true;
>> > > > > > > > > >         }
>> > > > > > > > > >
>> > > > > > > > > > @@ -471,6 +476,7 @@ static int
>> > > > > > > > > > amdgpu_atpx_power_state(enum
>> > > > > > > > vga_switcheroo_client_id id,
>> > > > > > > > > >   */
>> > > > > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
>> > > *pdev)
>> > > > > > > > > >  {
>> > > > > > > > > > +       struct pci_dev *parent_pdev =
>> > > pci_upstream_bridge(pdev);
>> > > > > > > > > >         acpi_handle dhandle, atpx_handle;
>> > > > > > > > > >         acpi_status status;
>> > > > > > > > > >
>> > > > > > > > > > @@ -485,6 +491,7 @@ static bool
>> > > > > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
>> > > > > > > > > >         }
>> > > > > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
>> > > > > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
>> > > > > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev
>> > > > > > > > > > &&
>> > > > > > > > parent_pdev->bridge_d3;
>> > > > > > > > > >         return true;
>> > > > > > > > > >  }
>> > > > > > > > > >
>> > > > > > > > > > --
>> > > > > > > > > > 2.10.2

[-- Attachment #2: 0001-drm-radeon-fix-check-for-port-PM-availability.patch --]
[-- Type: text/x-patch, Size: 2833 bytes --]

From cd21b5055cca49b30b0caaf1107a9aaeb60a447f Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 28 Nov 2016 17:23:40 -0500
Subject: [PATCH] drm/radeon: fix check for port PM availability

The ATPX method does not always exist on the dGPU, it may be located at
the iGPU. The parent device of the iGPU is the root port for which
bridge_d3 is false. This accidentally enables the legacy PM method which
conflicts with port PM and prevented the dGPU from powering on.

Ported from amdgpu commit:
drm/amdgpu: fix check for port PM availability
from Peter Wu.

Fixes: d3ac31f3b4bf9fad (drm/radeon: fix power state when port pm is unavailable (v2))
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Peter Wu <peter@lekensteyn.nl>
Cc: <stable@vger.kernel.org> # 4.8+
---
 drivers/gpu/drm/radeon/radeon_atpx_handler.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 4129b12..0ae13cd2 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -479,7 +479,6 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
  */
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
 	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
@@ -493,7 +492,6 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx.handle = atpx_handle;
-	radeon_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
 	return true;
 }
 
@@ -555,11 +553,16 @@ static bool radeon_atpx_detect(void)
 	struct pci_dev *pdev = NULL;
 	bool has_atpx = false;
 	int vga_count = 0;
+	bool d3_supported = false;
+	struct pci_dev *parent_pdev;
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
 		vga_count++;
 
 		has_atpx |= (radeon_atpx_pci_probe_handle(pdev) == true);
+
+		parent_pdev = pci_upstream_bridge(pdev);
+		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
 	}
 
 	/* some newer PX laptops mark the dGPU as a non-VGA display device */
@@ -567,6 +570,9 @@ static bool radeon_atpx_detect(void)
 		vga_count++;
 
 		has_atpx |= (radeon_atpx_pci_probe_handle(pdev) == true);
+
+		parent_pdev = pci_upstream_bridge(pdev);
+		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
 	}
 
 	if (has_atpx && vga_count == 2) {
@@ -574,6 +580,7 @@ static bool radeon_atpx_detect(void)
 		printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
 		       acpi_method_name);
 		radeon_atpx_priv.atpx_detected = true;
+		radeon_atpx_priv.bridge_pm_usable = d3_supported;
 		radeon_atpx_init();
 		return true;
 	}
-- 
2.5.5


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable
       [not found]                                         ` <CADnq5_MfLP+Ym+beD+Oak+CTxvyMVYfb9+DQDZuR9pbBWqwQtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-11-29  0:58                                           ` Peter Wu
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Wu @ 2016-11-29  0:58 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Mike Lothian, Deucher, Alexander, amd-gfx list, Nayan Deshmukh

On Mon, Nov 28, 2016 at 05:28:00PM -0500, Alex Deucher wrote:
> On Mon, Nov 28, 2016 at 12:48 AM, Mike Lothian <mike@fireburn.co.uk> wrote:
> > The attached patch allows my machine to boot also
> >
> > Thanks
> >
> > Mike
> >
> > On Sat, 26 Nov 2016 at 14:14 Peter Wu <peter@lekensteyn.nl> wrote:
> >>
> >> Found the problem.
> >>
> >> amdgpu_atpx_pci_probe_handle is used to detect the ATPX handle, but this
> >> does not necessarily have to be on the dGPU. In fact, on your machine it
> >> is located on the Intel iGPU (00:02.0):
> >>
> >> [  134.760509] vga_switcheroo: detected switching method
> >> \_SB_.PCI0.GFX0.ATPX handle
> >>
> >> The parent device of the iGPU (00:00.0) is the root port which has no
> >> PCIe capability, hence bridge_d3 evaluated to false.
> >>
> >> Can you try the attached patch?
> 
> Looks good to me.  Patch for radeon included as well.
> 
> Alex

Thanks, the port looks good to me. (If still possible, I think you can
change Reported-by also to Reported-and-tested-by for the amdgpu patch.)

Kind regards,
Peter

> >>
> >> Kind regards,
> >> Peter
> >>
> >> On Sat, Nov 26, 2016 at 01:49:08PM +0000, Mike Lothian wrote:
> >> > I can confirm the patch allows my machine to boot again
> >> >
> >> > On Sat, 26 Nov 2016 at 11:39 Peter Wu <peter@lekensteyn.nl> wrote:
> >> >
> >> > > Hi Mike,
> >> > >
> >> > > To grab console output, you could also try to blacklist amdgpu at
> >> > > startup and load it later.
> >> > >
> >> > > Can you provide your acpidump (and lspci -nn, especially if you do not
> >> > > have a dmesg).
> >> > >
> >> > > This is the call chain:
> >> > >
> >> > > + amdgpu_atpx_detect
> >> > >   + amdgpu_atpx_pci_probe_handle
> >> > >     + return if ATPX ACPI handle cannot be found
> >> > >     + set amdgpu_atpx_priv.bridge_pm_usable
> >> > >   + amdgpu_atpx_init
> >> > >     + amdgpu_atpx_validate
> >> > >       + use amdgpu_atpx_priv.bridge_pm_usable
> >> > >
> >> > > I wonder whether newer devices remove ATPX completely. If that is the
> >> > > case (please provide acpidump!), can you try this hack:
> >> > >
> >> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > index 951addf..5c23382 100644
> >> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > @@ -487,6 +487,7 @@ static bool amdgpu_atpx_pci_probe_handle(struct
> >> > > pci_dev *pdev)
> >> > >         status = acpi_get_handle(dhandle, "ATPX", &atpx_handle);
> >> > >         if (ACPI_FAILURE(status)) {
> >> > >                 amdgpu_atpx_priv.other_handle = dhandle;
> >> > > +               amdgpu_atpx_priv.bridge_pm_usable = true;
> >> > >                 return false;
> >> > >         }
> >> > >         amdgpu_atpx_priv.dhandle = dhandle;
> >> > >
> >> > > This is not intended for merging, but just testing a hypothesis.
> >> > >
> >> > > Kind regards,
> >> > > Peter
> >> > >
> >> > > On Sat, Nov 26, 2016 at 11:20:28AM +0000, Mike Lothian wrote:
> >> > > > He's some screen shots of the booting screens, apologies they're not
> >> > > > that
> >> > > > clear
> >> > > >
> >> > > > https://imgur.com/a/V7mRA
> >> > > >
> >> > > > On Sat, 26 Nov 2016 at 11:00 Mike Lothian <mike@fireburn.co.uk>
> >> > > > wrote:
> >> > > >
> >> > > > > Hi
> >> > > > >
> >> > > > > The previous working kernel, was the patch before this - reverting
> >> > > fixes it
> >> > > > >
> >> > > > > My bios date is quite new 08/05/2016
> >> > > > >
> >> > > > > Most of the messages fly straight off the screen, the journal
> >> > > > > isn't
> >> > > kept
> >> > > > > and the wifi driver also complains, will see if I can get a slow
> >> > > > > motion
> >> > > > > capture of the errors and screen shot them
> >> > > > >
> >> > > > > Regards
> >> > > > >
> >> > > > > Mike
> >> > > > >
> >> > > > > On Sat, 26 Nov 2016 at 10:20 Peter Wu <peter@lekensteyn.nl> wrote:
> >> > > > >
> >> > > > > Hi Mike,
> >> > > > >
> >> > > > > On Sat, Nov 26, 2016 at 12:36:27AM +0000, Mike Lothian wrote:
> >> > > > > > This patch is preventing my laptop from booting, I'm getting D3
> >> > > > > > error
> >> > > > > > messages and atom bios stuck messages
> >> > > > >
> >> > > > > Can you post a dmesg and acpidump? What was the previous working
> >> > > kernel?
> >> > > > > I suppose your BIOS date (/sys/class/dmi/id/bios_date) is
> >> > > > > pre-2015?
> >> > > > > Does booting with runpm=0 work?
> >> > > > >
> >> > > > > > I mistakenly saw the v2 patch and didn't realise it was for
> >> > > > > > radeon
> >> > > not
> >> > > > > > amdgpu - this branch and Linus's tree are currently not booting
> >> > > > > > for
> >> > > me
> >> > > > >
> >> > > > > What do you mean by this?
> >> > > > >
> >> > > > > Kind regards,
> >> > > > > Peter
> >> > > > >
> >> > > > > > On Wed, 23 Nov 2016 at 17:16 Deucher, Alexander <
> >> > > > > Alexander.Deucher@amd.com>
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > > > > -----Original Message-----
> >> > > > > > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> >> > > > > > > > Sent: Wednesday, November 23, 2016 12:13 PM
> >> > > > > > > > To: Alex Deucher
> >> > > > > > > > Cc: amd-gfx list; Deucher, Alexander; Nayan Deshmukh
> >> > > > > > > > Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when
> >> > > > > > > > port
> >> > > pm is
> >> > > > > > > > unavailable
> >> > > > > > > >
> >> > > > > > > > On Wed, Nov 23, 2016 at 12:01:55PM -0500, Alex Deucher
> >> > > > > > > > wrote:
> >> > > > > > > > >  On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <
> >> > > peter@lekensteyn.nl>
> >> > > > > > > wrote:
> >> > > > > > > > > > When PCIe port PM is not enabled (system BIOS is
> >> > > > > > > > > > pre-2015 or
> >> > > the
> >> > > > > > > > > > pcie_port_pm=off parameter is set), legacy ATPX PM
> >> > > > > > > > > > should
> >> > > still
> >> > > > > be
> >> > > > > > > > > > marked as supported. Otherwise the GPU can fail to power
> >> > > > > > > > > > on
> >> > > after
> >> > > > > > > > > > runtime suspend. This affected a Dell Inspiron 5548.
> >> > > > > > > > > >
> >> > > > > > > > > > Ideally the BIOS date in the PCI core is lowered to 2013
> >> > > > > > > > > > (the
> >> > > > > first
> >> > > > > > > year
> >> > > > > > > > > > where hybrid graphics platforms using power resources
> >> > > > > > > > > > was
> >> > > > > > > introduced),
> >> > > > > > > > > > but that seems more risky at this point and would not
> >> > > > > > > > > > solve
> >> > > the
> >> > > > > > > > > > pcie_port_pm=off issue.
> >> > > > > > > > > >
> >> > > > > > > > > > Bugzilla:
> >> > > > > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=98505
> >> > > > > > > > > > Reported-and-tested-by: Nayan Deshmukh
> >> > > > > > > > <nayan26deshmukh@gmail.com>
> >> > > > > > > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> >> > > > > > > > > > ---
> >> > > > > > > > > > Hi,
> >> > > > > > > > > >
> >> > > > > > > > > > This patch is already three weeks old. One alternative
> >> > > > > > > > > > idea
> >> > > was
> >> > > > > > > lowering
> >> > > > > > > > BIOS
> >> > > > > > > > > > date in PCI core, but as pcie_port_pm=force did not have
> >> > > > > > > > > > the
> >> > > > > desired
> >> > > > > > > > effect, I
> >> > > > > > > > > > do not think that this would help though.
> >> > > > > > > > >
> >> > > > > > > > > Thanks for doing this.
> >> > > > > > > > >
> >> > > > > > > > > >
> >> > > > > > > > > > I have also not contacted linux-pci or Mika about
> >> > > > > > > > > > lowering
> >> > > the
> >> > > > > year
> >> > > > > > > due to
> >> > > > > > > > the
> >> > > > > > > > > > lack of a good reason.  Might do it later though once
> >> > > > > > > > > > ACPICA
> >> > > bug
> >> > > > > > > 1333 is
> >> > > > > > > > sorted
> >> > > > > > > > > > out such that lowering the year actually has benefits
> >> > > > > > > > > > for a
> >> > > > > Nvidia
> >> > > > > > > laptop
> >> > > > > > > > (or if
> >> > > > > > > > > > some amdgpu problem can be solved by this).
> >> > > > > > > > > >
> >> > > > > > > > > > Both patches should probably be Cc stable (4.8+), fixing
> >> > > > > > > 2f5af82eeab2 and
> >> > > > > > > > > > b8c9fd5ad4b4 ("track whether if this is a hybrid
> >> > > > > > > > > > graphics
> >> > > > > > > platform"). There
> >> > > > > > > > have
> >> > > > > > > > > > been some ifdef 0's and reverts in between, so I was not
> >> > > sure if
> >> > > > > > > adding
> >> > > > > > > > the
> >> > > > > > > > > > Fixes tag is appropriate.
> >> > > > > > > > >
> >> > > > > > > > > I don't think we need to cc stable.  In kernel 4.8 we
> >> > > > > > > > > don't
> >> > > > > attempt to
> >> > > > > > > > > do d3cold at all.  4.8 and older kernels have:
> >> > > > > > > > > c63695cc5e5f685e924e25a8f9555f6e846f1fc6 (drm/amdgpu: work
> >> > > around
> >> > > > > > > > lack
> >> > > > > > > > > of upstream ACPI support for D3cold)
> >> > > > > > > > > which was reverted for 4.9.
> >> > > > > > > >
> >> > > > > > > > That patch was already reverted in 4.8 as far as I can see:
> >> > > > > > > >
> >> > > > > > > > $ git tag --contains
> >> > > > > > > > c39b487f195b93235ee76384427467786f7bf29f |
> >> > > grep
> >> > > > > v4.8
> >> > > > > > > > v4.8
> >> > > > > > > >
> >> > > > > > > > Can you double-check? v4.8 was the first kernel with D3cold
> >> > > support
> >> > > > > in
> >> > > > > > > > PCI core.
> >> > > > > > >
> >> > > > > > > You are right.  I was thinking d3cold went upstream in 4.9, so
> >> > > yes, we
> >> > > > > > > should apply this to 4.8.
> >> > > > > > >
> >> > > > > > > Alex
> >> > > > > > >
> >> > > > > > > >
> >> > > > > > > > > Series is:
> >> > > > > > > > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >> > > > > > > >
> >> > > > > > > > Thanks!
> >> > > > > > > >
> >> > > > > > > > Kind regards,
> >> > > > > > > > Peter
> >> > > > > > > >
> >> > > > > > > > >
> >> > > > > > > > > >
> >> > > > > > > > > > Kind regards,
> >> > > > > > > > > > Peter
> >> > > > > > > > > > ---
> >> > > > > > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9
> >> > > ++++++++-
> >> > > > > > > > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> >> > > > > > > > > >
> >> > > > > > > > > > diff --git
> >> > > > > > > > > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > > > > > > > > index 10b5ddf..951addf 100644
> >> > > > > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > > > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> >> > > > > > > > > > @@ -33,6 +33,7 @@ struct amdgpu_atpx {
> >> > > > > > > > > >
> >> > > > > > > > > >  static struct amdgpu_atpx_priv {
> >> > > > > > > > > >         bool atpx_detected;
> >> > > > > > > > > > +       bool bridge_pm_usable;
> >> > > > > > > > > >         /* handle for device - and atpx */
> >> > > > > > > > > >         acpi_handle dhandle;
> >> > > > > > > > > >         acpi_handle other_handle;
> >> > > > > > > > > > @@ -200,7 +201,11 @@ static int
> >> > > > > > > > > > amdgpu_atpx_validate(struct
> >> > > > > > > > amdgpu_atpx *atpx)
> >> > > > > > > > > >         atpx->is_hybrid = false;
> >> > > > > > > > > >         if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> >> > > > > > > > > >                 printk("ATPX Hybrid Graphics\n");
> >> > > > > > > > > > -               atpx->functions.power_cntl = false;
> >> > > > > > > > > > +               /*
> >> > > > > > > > > > +                * Disable legacy PM methods only when
> >> > > > > > > > > > pcie
> >> > > port
> >> > > > > PM
> >> > > > > > > is usable,
> >> > > > > > > > > > +                * otherwise the device might fail to
> >> > > > > > > > > > power
> >> > > off
> >> > > > > or
> >> > > > > > > power on.
> >> > > > > > > > > > +                */
> >> > > > > > > > > > +               atpx->functions.power_cntl =
> >> > > > > > > > !amdgpu_atpx_priv.bridge_pm_usable;
> >> > > > > > > > > >                 atpx->is_hybrid = true;
> >> > > > > > > > > >         }
> >> > > > > > > > > >
> >> > > > > > > > > > @@ -471,6 +476,7 @@ static int
> >> > > > > > > > > > amdgpu_atpx_power_state(enum
> >> > > > > > > > vga_switcheroo_client_id id,
> >> > > > > > > > > >   */
> >> > > > > > > > > >  static bool amdgpu_atpx_pci_probe_handle(struct pci_dev
> >> > > *pdev)
> >> > > > > > > > > >  {
> >> > > > > > > > > > +       struct pci_dev *parent_pdev =
> >> > > pci_upstream_bridge(pdev);
> >> > > > > > > > > >         acpi_handle dhandle, atpx_handle;
> >> > > > > > > > > >         acpi_status status;
> >> > > > > > > > > >
> >> > > > > > > > > > @@ -485,6 +491,7 @@ static bool
> >> > > > > > > > amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> >> > > > > > > > > >         }
> >> > > > > > > > > >         amdgpu_atpx_priv.dhandle = dhandle;
> >> > > > > > > > > >         amdgpu_atpx_priv.atpx.handle = atpx_handle;
> >> > > > > > > > > > +       amdgpu_atpx_priv.bridge_pm_usable = parent_pdev
> >> > > > > > > > > > &&
> >> > > > > > > > parent_pdev->bridge_d3;
> >> > > > > > > > > >         return true;
> >> > > > > > > > > >  }
> >> > > > > > > > > >
> >> > > > > > > > > > --
> >> > > > > > > > > > 2.10.2

> From cd21b5055cca49b30b0caaf1107a9aaeb60a447f Mon Sep 17 00:00:00 2001
> From: Alex Deucher <alexander.deucher@amd.com>
> Date: Mon, 28 Nov 2016 17:23:40 -0500
> Subject: [PATCH] drm/radeon: fix check for port PM availability
> 
> The ATPX method does not always exist on the dGPU, it may be located at
> the iGPU. The parent device of the iGPU is the root port for which
> bridge_d3 is false. This accidentally enables the legacy PM method which
> conflicts with port PM and prevented the dGPU from powering on.
> 
> Ported from amdgpu commit:
> drm/amdgpu: fix check for port PM availability
> from Peter Wu.
> 
> Fixes: d3ac31f3b4bf9fad (drm/radeon: fix power state when port pm is unavailable (v2))
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Peter Wu <peter@lekensteyn.nl>
> Cc: <stable@vger.kernel.org> # 4.8+
> ---
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
> index 4129b12..0ae13cd2 100644
> --- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
> +++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
> @@ -479,7 +479,6 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
>   */
>  static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
>  {
> -	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
>  	acpi_handle dhandle, atpx_handle;
>  	acpi_status status;
>  
> @@ -493,7 +492,6 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
>  
>  	radeon_atpx_priv.dhandle = dhandle;
>  	radeon_atpx_priv.atpx.handle = atpx_handle;
> -	radeon_atpx_priv.bridge_pm_usable = parent_pdev && parent_pdev->bridge_d3;
>  	return true;
>  }
>  
> @@ -555,11 +553,16 @@ static bool radeon_atpx_detect(void)
>  	struct pci_dev *pdev = NULL;
>  	bool has_atpx = false;
>  	int vga_count = 0;
> +	bool d3_supported = false;
> +	struct pci_dev *parent_pdev;
>  
>  	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
>  		vga_count++;
>  
>  		has_atpx |= (radeon_atpx_pci_probe_handle(pdev) == true);
> +
> +		parent_pdev = pci_upstream_bridge(pdev);
> +		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
>  	}
>  
>  	/* some newer PX laptops mark the dGPU as a non-VGA display device */
> @@ -567,6 +570,9 @@ static bool radeon_atpx_detect(void)
>  		vga_count++;
>  
>  		has_atpx |= (radeon_atpx_pci_probe_handle(pdev) == true);
> +
> +		parent_pdev = pci_upstream_bridge(pdev);
> +		d3_supported |= parent_pdev && parent_pdev->bridge_d3;
>  	}
>  
>  	if (has_atpx && vga_count == 2) {
> @@ -574,6 +580,7 @@ static bool radeon_atpx_detect(void)
>  		printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
>  		       acpi_method_name);
>  		radeon_atpx_priv.atpx_detected = true;
> +		radeon_atpx_priv.bridge_pm_usable = d3_supported;
>  		radeon_atpx_init();
>  		return true;
>  	}
> -- 
> 2.5.5
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

end of thread, other threads:[~2016-11-29  0:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23  1:22 [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable Peter Wu
     [not found] ` <20161123012225.11586-1-peter-VTkQYDcBqhK7DlmcbJSQ7g@public.gmane.org>
2016-11-23  1:22   ` [PATCH 2/2] drm/radeon: " Peter Wu
2016-11-23  7:53   ` [PATCH 1/2] drm/amdgpu: " Christian König
2016-11-23 17:01   ` Alex Deucher
     [not found]     ` <CADnq5_On72XmLrE3wjD0u1=tmwghv6GofzfEGM5mhGoVtJav+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-23 17:12       ` Peter Wu
2016-11-23 17:16         ` Deucher, Alexander
     [not found]           ` <MWHPR12MB16949832E9DB44C53995493AF7B70-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-11-26  0:36             ` Mike Lothian
     [not found]               ` <CAHbf0-FgRU=6tYe2SYcm8E0ZvJuRFoqPpk77_DZdQKXdxZSsGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-26 10:19                 ` Peter Wu
2016-11-26 11:00                   ` Mike Lothian
     [not found]                     ` <CAHbf0-HfHGt5HvAosxgVf=rgevPK3KqBCQt4cJqjDiEaQWM1pQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-26 11:20                       ` Mike Lothian
     [not found]                         ` <CAHbf0-HHFFthD3Li0wM8x5rciRreeWoE0SkQOvsENT9+ZRFEcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-26 11:38                           ` Peter Wu
2016-11-26 13:43                             ` Mike Lothian
2016-11-26 13:49                             ` Mike Lothian
     [not found]                               ` <CAHbf0-GjBMb2GfFoTADxYjfuLU=+hkJ8AcZCbh70zrpn56cD8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-26 14:14                                 ` Peter Wu
2016-11-28  5:48                                   ` Mike Lothian
     [not found]                                     ` <CAHbf0-HzBPXOqS_XLctTu-eTuxUUwP62_TcODVPwKb8T2PFrFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-28 22:28                                       ` Alex Deucher
     [not found]                                         ` <CADnq5_MfLP+Ym+beD+Oak+CTxvyMVYfb9+DQDZuR9pbBWqwQtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-29  0:58                                           ` Peter Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.