From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Lothian Subject: Re: [PATCH 1/2] drm/amdgpu: fix power state when port pm is unavailable Date: Sat, 26 Nov 2016 00:36:27 +0000 Message-ID: References: <20161123012225.11586-1-peter@lekensteyn.nl> <20161123171255.GA18537@al> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0015194520==" Return-path: In-Reply-To: List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: "Deucher, Alexander" , Peter Wu , Alex Deucher Cc: Nayan Deshmukh , amd-gfx list --===============0015194520== Content-Type: multipart/alternative; boundary=001a1130cf06813f790542296ed1 --001a1130cf06813f790542296ed1 Content-Type: text/plain; charset=UTF-8 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 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 > 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 > > > > > > Signed-off-by: Peter Wu > > > > --- > > > > 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 > > > > 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 > --001a1130cf06813f790542296ed1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
This patch is preventing my laptop from booting, I'm g= etting D3 error messages and atom bios stuck messages

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

On Wed, 23 Nov 201= 6 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:
> >=C2=A0 On Tue, Nov 22, 2016 at 8:22 PM, Peter Wu <peter@leken= steyn.nl> wrote:
> > > When PCIe port PM is not enabled (system BIOS is pre-2015 or= the
> > > pcie_port_pm=3Doff 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 (th= e first year
> > > where hybrid graphics platforms using power resources was in= troduced),
> > > but that seems more risky at this point and would not solve = the
> > > pcie_port_pm=3Doff issue.
> > >
> > > Bugzilla: htt= ps://bugs.freedesktop.org/show_bug.cgi?id=3D98505
> > > 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=3Dforce did not have t= he 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 t= he year due to
> the
> > > lack of a good reason.=C2=A0 Might do it later though once A= CPICA 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 2f5= af82eeab2 and
> > > b8c9fd5ad4b4 ("track whether if this is a hybrid graphi= cs 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.=C2=A0 In kernel 4.8 we do= n't attempt to
> > do d3cold at all.=C2=A0 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 v= 4.8
> v4.8
>
> Can you double-check? v4.8 was the first kernel with D3cold support in=
> PCI core.

You are right.=C2=A0 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
> > > ---
> > >=C2=A0 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 += +++++++-
> > >=C2=A0 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 {
> > >
> > >=C2=A0 static struct amdgpu_atpx_priv {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0bool atpx_detected;
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0bool bridge_pm_usable;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* handle for device - and = atpx */
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0acpi_handle dhandle;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0acpi_handle other_handle; > > > @@ -200,7 +201,11 @@ static int amdgpu_atpx_validate(struct<= br class=3D"gmail_msg"> > amdgpu_atpx *atpx)
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atpx->is_hybrid =3D fals= e;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (valid_bits & ATPX_M= S_HYBRID_GFX_SUPPORTED) {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0printk("ATPX Hybrid Graphics\n");
> > > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atpx= ->functions.power_cntl =3D false;
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * D= isable legacy PM methods only when pcie port PM is usable,
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * o= therwise the device might fail to power off or power on.
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 */<= br class=3D"gmail_msg"> > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atpx= ->functions.power_cntl =3D
> !amdgpu_atpx_priv.bridge_pm_usable;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0atpx->is_hybrid =3D true;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> > >
> > > @@ -471,6 +476,7 @@ static int amdgpu_atpx_power_state(enum<= br class=3D"gmail_msg"> > vga_switcheroo_client_id id,
> > >=C2=A0 =C2=A0*/
> > >=C2=A0 static bool amdgpu_atpx_pci_probe_handle(struct pci_de= v *pdev)
> > >=C2=A0 {
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0struct pci_dev *parent_pdev =3D = pci_upstream_bridge(pdev);
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0acpi_handle dhandle, atpx_h= andle;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0acpi_status status;
> > >
> > > @@ -485,6 +491,7 @@ static bool
> amdgpu_atpx_pci_probe_handle(struct pci_dev *pdev)
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0amdgpu_atpx_priv.dhandle = =3D dhandle;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0amdgpu_atpx_priv.atpx.handl= e =3D atpx_handle;
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0amdgpu_atpx_priv.bridge_pm_usabl= e =3D parent_pdev &&
> parent_pdev->bridge_d3;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return true;
> > >=C2=A0 }
> > >
> > > --
> > > 2.10.2
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.= org/mailman/listinfo/amd-gfx
--001a1130cf06813f790542296ed1-- --===============0015194520== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4Cg== --===============0015194520==--