<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<pre>
</pre>
<div class="moz-cite-prefix">
<pre>On 10/30/19 5:35 AM, Michael Ellerman wrote:</pre>
</div>
<blockquote type="cite" cite="mid:87o8xyfrmp.fsf@mpe.ellerman.id.au">
<pre class="moz-quote-pre" wrap="">Hi Kyle,
KyleMahlkuch <a class="moz-txt-link-rfc2396E" href="mailto:kmahlkuc@linux.vnet.ibm.com"><kmahlkuc@linux.vnet.ibm.com></a> writes:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">From: Kyle Mahlkuch <a class="moz-txt-link-rfc2396E" href="mailto:kmahlkuc@linux.vnet.ibm.com"><kmahlkuc@linux.vnet.ibm.com></a>
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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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?</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">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.
</pre>
<blockquote type="cite" cite="mid:87o8xyfrmp.fsf@mpe.ellerman.id.au">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This local serves no real purpose and could be avoided, which would also
avoid this ifdef.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> /* 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
AFAIK drm uses normal kernel comment style, so this should be:
/*
* Some adapters need to be suspended before a
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ * 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);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
ie, instead do:
radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I agree, this is a cleaner way to write this patch. I'll update the comment as
well. Thanks for the help.
</pre>
<blockquote type="cite" cite="mid:87o8xyfrmp.fsf@mpe.ellerman.id.au">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+#endif
}
static int radeon_pmops_suspend(struct device *dev)
--
1.8.3.1
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
cheers
</pre>
<pre>
</pre>
</blockquote>
</body>
</html>