[PATCH v3] drm/radeon: Fix EEH during kexec

[PATCH v3] drm/radeon: Fix EEH during kexec

[KyleMahlkuch]

From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>

During kexec some adapters hit an EEH since they are not properly
shut down in the radeon_pci_shutdown() function. Adding
radeon_suspend_kms() fixes this issue.
Enabled only on PPC because this patch causes issues on some other
boards.

Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 9e55076..4528f4d 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 static void
 radeon_pci_shutdown(struct pci_dev *pdev)
 {
+#ifdef CONFIG_PPC64
+	struct drm_device *ddev = pci_get_drvdata(pdev);
+#endif
+
 	/* if we are running in a VM, make sure the device
 	 * torn down properly on reboot/shutdown
 	 */
 	if (radeon_device_is_virtual())
 		radeon_pci_remove(pdev);
+
+#ifdef CONFIG_PPC64
+	/* Some adapters need to be suspended before a
+	 * shutdown occurs in order to prevent an error
+	 * during kexec.
+	 * Make this power specific becauase it breaks
+	 * some non-power boards.
+	 */
+	radeon_suspend_kms(ddev, true, true, false);
+#endif
 }
 
 static int radeon_pmops_suspend(struct device *dev)
-- 
1.8.3.1

[PATCH v3] drm/radeon: Fix EEH during kexec

[KyleMahlkuch]

From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>

During kexec some adapters hit an EEH since they are not properly
shut down in the radeon_pci_shutdown() function. Adding
radeon_suspend_kms() fixes this issue.
Enabled only on PPC because this patch causes issues on some other
boards.

Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 9e55076..4528f4d 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 static void
 radeon_pci_shutdown(struct pci_dev *pdev)
 {
+#ifdef CONFIG_PPC64
+	struct drm_device *ddev = pci_get_drvdata(pdev);
+#endif
+
 	/* if we are running in a VM, make sure the device
 	 * torn down properly on reboot/shutdown
 	 */
 	if (radeon_device_is_virtual())
 		radeon_pci_remove(pdev);
+
+#ifdef CONFIG_PPC64
+	/* Some adapters need to be suspended before a
+	 * shutdown occurs in order to prevent an error
+	 * during kexec.
+	 * Make this power specific becauase it breaks
+	 * some non-power boards.
+	 */
+	radeon_suspend_kms(ddev, true, true, false);
+#endif
 }
 
 static int radeon_pmops_suspend(struct device *dev)
-- 
1.8.3.1

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Greg KH]

On Fri, Oct 25, 2019 at 03:40:50PM -0500, KyleMahlkuch wrote:
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
> 
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.
> 
> Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Greg KH]

On Fri, Oct 25, 2019 at 03:40:50PM -0500, KyleMahlkuch wrote:
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
> 
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.
> 
> Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Alex Deucher]

On Fri, Oct 25, 2019 at 4:44 PM KyleMahlkuch
<kmahlkuc@linux.vnet.ibm.com> wrote:
>
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.
>
> Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> +       struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif
> +
>         /* if we are running in a VM, make sure the device
>          * torn down properly on reboot/shutdown
>          */
>         if (radeon_device_is_virtual())
>                 radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> +       /* Some adapters need to be suspended before a
> +        * shutdown occurs in order to prevent an error
> +        * during kexec.
> +        * Make this power specific becauase it breaks
> +        * some non-power boards.
> +        */
> +       radeon_suspend_kms(ddev, true, true, false);
> +#endif
>  }
>
>  static int radeon_pmops_suspend(struct device *dev)
> --
> 1.8.3.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Alex Deucher]

On Fri, Oct 25, 2019 at 4:44 PM KyleMahlkuch
<kmahlkuc@linux.vnet.ibm.com> wrote:
>
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.
>
> Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> +       struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif
> +
>         /* if we are running in a VM, make sure the device
>          * torn down properly on reboot/shutdown
>          */
>         if (radeon_device_is_virtual())
>                 radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> +       /* Some adapters need to be suspended before a
> +        * shutdown occurs in order to prevent an error
> +        * during kexec.
> +        * Make this power specific becauase it breaks
> +        * some non-power boards.
> +        */
> +       radeon_suspend_kms(ddev, true, true, false);
> +#endif
>  }
>
>  static int radeon_pmops_suspend(struct device *dev)
> --
> 1.8.3.1
>
> _______________________________________________
> 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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Michael Ellerman]

Hi Kyle,

KyleMahlkuch <kmahlkuc@linux.vnet.ibm.com> writes:
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.

Which adapters hit the issues?

And do we know why they're not shut down correctly in
radeon_pci_shutdown()? That seems like the root cause no?


> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> +	struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif

This local serves no real purpose and could be avoided, which would also
avoid this ifdef.

>  	/* if we are running in a VM, make sure the device
>  	 * torn down properly on reboot/shutdown
>  	 */
>  	if (radeon_device_is_virtual())
>  		radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> +	/* Some adapters need to be suspended before a

AFAIK drm uses normal kernel comment style, so this should be:

	/*
	 * Some adapters need to be suspended before a
> +	 * shutdown occurs in order to prevent an error
> +	 * during kexec.
> +	 * Make this power specific becauase it breaks
> +	 * some non-power boards.
> +	 */
> +	radeon_suspend_kms(ddev, true, true, false);

ie, instead do:

	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

> +#endif
>  }
>  
>  static int radeon_pmops_suspend(struct device *dev)
> -- 
> 1.8.3.1

cheers

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Michael Ellerman]

Hi Kyle,

KyleMahlkuch <kmahlkuc@linux.vnet.ibm.com> writes:
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.

Which adapters hit the issues?

And do we know why they're not shut down correctly in
radeon_pci_shutdown()? That seems like the root cause no?


> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> +	struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif

This local serves no real purpose and could be avoided, which would also
avoid this ifdef.

>  	/* if we are running in a VM, make sure the device
>  	 * torn down properly on reboot/shutdown
>  	 */
>  	if (radeon_device_is_virtual())
>  		radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> +	/* Some adapters need to be suspended before a

AFAIK drm uses normal kernel comment style, so this should be:

	/*
	 * Some adapters need to be suspended before a
> +	 * shutdown occurs in order to prevent an error
> +	 * during kexec.
> +	 * Make this power specific becauase it breaks
> +	 * some non-power boards.
> +	 */
> +	radeon_suspend_kms(ddev, true, true, false);

ie, instead do:

	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

> +#endif
>  }
>  
>  static int radeon_pmops_suspend(struct device *dev)
> -- 
> 1.8.3.1

cheers

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Michael Ellerman]

Hi Kyle,

KyleMahlkuch <kmahlkuc@linux.vnet.ibm.com> writes:
> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.

Which adapters hit the issues?

And do we know why they're not shut down correctly in
radeon_pci_shutdown()? That seems like the root cause no?


> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> +	struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif

This local serves no real purpose and could be avoided, which would also
avoid this ifdef.

>  	/* if we are running in a VM, make sure the device
>  	 * torn down properly on reboot/shutdown
>  	 */
>  	if (radeon_device_is_virtual())
>  		radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> +	/* Some adapters need to be suspended before a

AFAIK drm uses normal kernel comment style, so this should be:

	/*
	 * Some adapters need to be suspended before a
> +	 * shutdown occurs in order to prevent an error
> +	 * during kexec.
> +	 * Make this power specific becauase it breaks
> +	 * some non-power boards.
> +	 */
> +	radeon_suspend_kms(ddev, true, true, false);

ie, instead do:

	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

> +#endif
>  }
>  
>  static int radeon_pmops_suspend(struct device *dev)
> -- 
> 1.8.3.1

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Kyle Mahlkuch]

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

On 10/30/19 5:35 AM, Michael Ellerman wrote:

> Hi Kyle,
>
> KyleMahlkuch <kmahlkuc@linux.vnet.ibm.com> writes:
>> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>>
>> During kexec some adapters hit an EEH since they are not properly
>> shut down in the radeon_pci_shutdown() function. Adding
>> radeon_suspend_kms() fixes this issue.
>> Enabled only on PPC because this patch causes issues on some other
>> boards.
> Which adapters hit the issues?
>
> And do we know why they're not shut down correctly in
> radeon_pci_shutdown()? That seems like the root cause no?

Hi Michael,
This is hit by the Caicos (edwards2) adapter that I have on ppc. It is not hit
on the Cedar (FirePro) adapter - though I haven't tested this one recently. I'm
not able to test any other adapters. As far as "why", I'm unsure. During
initialization after the kexec we hit an EEH. There could be another point in
the shutdown / start up process where something doesn't get reset correctly.
I'm open to other ideas if you have any.

>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
>> index 9e55076..4528f4d 100644
>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>>   static void
>>   radeon_pci_shutdown(struct pci_dev *pdev)
>>   {
>> +#ifdef CONFIG_PPC64
>> +	struct drm_device *ddev = pci_get_drvdata(pdev);
>> +#endif
> This local serves no real purpose and could be avoided, which would also
> avoid this ifdef.
>
>>   	/* if we are running in a VM, make sure the device
>>   	 * torn down properly on reboot/shutdown
>>   	 */
>>   	if (radeon_device_is_virtual())
>>   		radeon_pci_remove(pdev);
>> +
>> +#ifdef CONFIG_PPC64
>> +	/* Some adapters need to be suspended before a
> AFAIK drm uses normal kernel comment style, so this should be:
>
> 	/*
> 	 * Some adapters need to be suspended before a
>> +	 * shutdown occurs in order to prevent an error
>> +	 * during kexec.
>> +	 * Make this power specific becauase it breaks
>> +	 * some non-power boards.
>> +	 */
>> +	radeon_suspend_kms(ddev, true, true, false);
> ie, instead do:
>
> 	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

I agree, this is a cleaner way to write this patch. I'll update the comment as
well. Thanks for the help.

>> +#endif
>>   }
>>   
>>   static int radeon_pmops_suspend(struct device *dev)
>> -- 
>> 1.8.3.1
> cheers
>

[-- Attachment #2: Type: text/html, Size: 3897 bytes --]

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Kyle Mahlkuch]

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

On 10/30/19 5:35 AM, Michael Ellerman wrote:

> Hi Kyle,
>
> KyleMahlkuch <kmahlkuc-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> writes:
>> From: Kyle Mahlkuch <kmahlkuc-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>> During kexec some adapters hit an EEH since they are not properly
>> shut down in the radeon_pci_shutdown() function. Adding
>> radeon_suspend_kms() fixes this issue.
>> Enabled only on PPC because this patch causes issues on some other
>> boards.
> Which adapters hit the issues?
>
> And do we know why they're not shut down correctly in
> radeon_pci_shutdown()? That seems like the root cause no?

Hi Michael,
This is hit by the Caicos (edwards2) adapter that I have on ppc. It is not hit
on the Cedar (FirePro) adapter - though I haven't tested this one recently. I'm
not able to test any other adapters. As far as "why", I'm unsure. During
initialization after the kexec we hit an EEH. There could be another point in
the shutdown / start up process where something doesn't get reset correctly.
I'm open to other ideas if you have any.

>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
>> index 9e55076..4528f4d 100644
>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>>   static void
>>   radeon_pci_shutdown(struct pci_dev *pdev)
>>   {
>> +#ifdef CONFIG_PPC64
>> +	struct drm_device *ddev = pci_get_drvdata(pdev);
>> +#endif
> This local serves no real purpose and could be avoided, which would also
> avoid this ifdef.
>
>>   	/* if we are running in a VM, make sure the device
>>   	 * torn down properly on reboot/shutdown
>>   	 */
>>   	if (radeon_device_is_virtual())
>>   		radeon_pci_remove(pdev);
>> +
>> +#ifdef CONFIG_PPC64
>> +	/* Some adapters need to be suspended before a
> AFAIK drm uses normal kernel comment style, so this should be:
>
> 	/*
> 	 * Some adapters need to be suspended before a
>> +	 * shutdown occurs in order to prevent an error
>> +	 * during kexec.
>> +	 * Make this power specific becauase it breaks
>> +	 * some non-power boards.
>> +	 */
>> +	radeon_suspend_kms(ddev, true, true, false);
> ie, instead do:
>
> 	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

I agree, this is a cleaner way to write this patch. I'll update the comment as
well. Thanks for the help.

>> +#endif
>>   }
>>   
>>   static int radeon_pmops_suspend(struct device *dev)
>> -- 
>> 1.8.3.1
> cheers
>

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

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

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Kyle Mahlkuch]

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

On 10/30/19 5:35 AM, Michael Ellerman wrote:

> Hi Kyle,
>
> KyleMahlkuch <kmahlkuc@linux.vnet.ibm.com> writes:
>> From: Kyle Mahlkuch <kmahlkuc@linux.vnet.ibm.com>
>>
>> During kexec some adapters hit an EEH since they are not properly
>> shut down in the radeon_pci_shutdown() function. Adding
>> radeon_suspend_kms() fixes this issue.
>> Enabled only on PPC because this patch causes issues on some other
>> boards.
> Which adapters hit the issues?
>
> And do we know why they're not shut down correctly in
> radeon_pci_shutdown()? That seems like the root cause no?

Hi Michael,
This is hit by the Caicos (edwards2) adapter that I have on ppc. It is not hit
on the Cedar (FirePro) adapter - though I haven't tested this one recently. I'm
not able to test any other adapters. As far as "why", I'm unsure. During
initialization after the kexec we hit an EEH. There could be another point in
the shutdown / start up process where something doesn't get reset correctly.
I'm open to other ideas if you have any.

>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
>> index 9e55076..4528f4d 100644
>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>>   static void
>>   radeon_pci_shutdown(struct pci_dev *pdev)
>>   {
>> +#ifdef CONFIG_PPC64
>> +	struct drm_device *ddev = pci_get_drvdata(pdev);
>> +#endif
> This local serves no real purpose and could be avoided, which would also
> avoid this ifdef.
>
>>   	/* if we are running in a VM, make sure the device
>>   	 * torn down properly on reboot/shutdown
>>   	 */
>>   	if (radeon_device_is_virtual())
>>   		radeon_pci_remove(pdev);
>> +
>> +#ifdef CONFIG_PPC64
>> +	/* Some adapters need to be suspended before a
> AFAIK drm uses normal kernel comment style, so this should be:
>
> 	/*
> 	 * Some adapters need to be suspended before a
>> +	 * shutdown occurs in order to prevent an error
>> +	 * during kexec.
>> +	 * Make this power specific becauase it breaks
>> +	 * some non-power boards.
>> +	 */
>> +	radeon_suspend_kms(ddev, true, true, false);
> ie, instead do:
>
> 	radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

I agree, this is a cleaner way to write this patch. I'll update the comment as
well. Thanks for the help.

>> +#endif
>>   }
>>   
>>   static int radeon_pmops_suspend(struct device *dev)
>> -- 
>> 1.8.3.1
> cheers
>

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

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

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Kyle Mahlkuch]

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


On 10/10/19 11:27 PM, Greg KH wrote:

> On Thu, Oct 10, 2019 at 02:44:29PM -0500, KyleMahlkuch wrote:
>> During kexec some adapters hit an EEH since they are not properly
>> shut down in the radeon_pci_shutdown() function. Adding
>> radeon_suspend_kms() fixes this issue.
>> Enabled only on PPC because this patch causes issues on some other
>> boards.
>>
>> Signed-off-by: Kyle Mahlkuch <Kyle.Mahlkuch at ibm.com>
> Real email address please, with a '@' sign.
>
> And your "From:" line did not match up with this :(

Greg, thanks for your help, I've resubmitted my patch.

>> ---
>>   drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
>
> </formletter>

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

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

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Kyle Mahlkuch]

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


On 10/10/19 11:27 PM, Greg KH wrote:

> On Thu, Oct 10, 2019 at 02:44:29PM -0500, KyleMahlkuch wrote:
>> During kexec some adapters hit an EEH since they are not properly
>> shut down in the radeon_pci_shutdown() function. Adding
>> radeon_suspend_kms() fixes this issue.
>> Enabled only on PPC because this patch causes issues on some other
>> boards.
>>
>> Signed-off-by: Kyle Mahlkuch <Kyle.Mahlkuch at ibm.com>
> Real email address please, with a '@' sign.
>
> And your "From:" line did not match up with this :(

Greg, thanks for your help, I've resubmitted my patch.

>> ---
>>   drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
>
> </formletter>

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

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

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Greg KH]

On Thu, Oct 10, 2019 at 02:44:29PM -0500, KyleMahlkuch wrote:
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.
> 
> Signed-off-by: Kyle Mahlkuch <Kyle.Mahlkuch at ibm.com>

Real email address please, with a '@' sign.

And your "From:" line did not match up with this :(

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

[PATCH v3] drm/radeon: Fix EEH during kexec

[KyleMahlkuch]

During kexec some adapters hit an EEH since they are not properly
shut down in the radeon_pci_shutdown() function. Adding
radeon_suspend_kms() fixes this issue.
Enabled only on PPC because this patch causes issues on some other
boards.

Signed-off-by: Kyle Mahlkuch <Kyle.Mahlkuch at ibm.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 9e55076..4528f4d 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 static void
 radeon_pci_shutdown(struct pci_dev *pdev)
 {
+#ifdef CONFIG_PPC64
+	struct drm_device *ddev = pci_get_drvdata(pdev);
+#endif
+
 	/* if we are running in a VM, make sure the device
 	 * torn down properly on reboot/shutdown
 	 */
 	if (radeon_device_is_virtual())
 		radeon_pci_remove(pdev);
+
+#ifdef CONFIG_PPC64
+	/* Some adapters need to be suspended before a
+	 * shutdown occurs in order to prevent an error
+	 * during kexec.
+	 * Make this power specific becauase it breaks
+	 * some non-power boards.
+	 */
+	radeon_suspend_kms(ddev, true, true, false);
+#endif
 }
 
 static int radeon_pmops_suspend(struct device *dev)
-- 
1.8.3.1