[PATCH] PCI: Mark AMD Navi14 GPU 0x7341 rev 0x00 ATS as broken

[PATCH] PCI: Mark AMD Navi14 GPU 0x7341 rev 0x00 ATS as broken

[Evan Quan]

Unexpected GPU hang was observed during runpm stress test
on 0x7341 rev 0x00. Further debugging shows broken ATS is
related. Thus as a followup of commit 5e89cd303e3a ("PCI:
Mark AMD Navi14 GPU rev 0xc5 ATS as broken"), we disable
the ATS for the specific SKU also.

Change-Id: I3d9d570bd473762e3bfbb251cf8abaf5af38ced9
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/pci/quirks.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b7e19bbb901a..70803ad6d2ac 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5176,7 +5176,8 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0422, quirk_no_ext_tags);
 static void quirk_amd_harvest_no_ats(struct pci_dev *pdev)
 {
 	if ((pdev->device == 0x7312 && pdev->revision != 0x00) ||
-	    (pdev->device == 0x7340 && pdev->revision != 0xc5))
+	    (pdev->device == 0x7340 && pdev->revision != 0xc5) ||
+	    (pdev->device == 0x7341 && pdev->revision != 0x00))
 		return;
 
 	if (pdev->device == 0x15d8) {
@@ -5203,6 +5204,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6900, quirk_amd_harvest_no_ats);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7312, quirk_amd_harvest_no_ats);
 /* AMD Navi14 dGPU */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7340, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7341, quirk_amd_harvest_no_ats);
 /* AMD Raven platform iGPU */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8, quirk_amd_harvest_no_ats);
 #endif /* CONFIG_PCI_ATS */
-- 
2.29.0

Re: [PATCH] PCI: Mark AMD Navi14 GPU 0x7341 rev 0x00 ATS as broken

[Krzysztof Wilczyński]

Hi Evan,

Thank you for sending the patch over!

A small nitpick: the subject line could just say:

  Add quirk for AMD Navi 14 to disable ATS support

Or something along these lines, as I am not sure how useful the ID and
revision are in the subject, especially since the commit message
explains in details what hardware is affected, etc.

> Unexpected GPU hang was observed during runpm stress test
> on 0x7341 rev 0x00. Further debugging shows broken ATS is
> related. Thus as a followup of commit 5e89cd303e3a ("PCI:
> Mark AMD Navi14 GPU rev 0xc5 ATS as broken"), we disable
> the ATS for the specific SKU also.

As this might be a candidate for a back-port to current stable and
long-term kernels, does it have any "Fixes" tag we could include here
for reference?  If not, then it's OK.

> Change-Id: I3d9d570bd473762e3bfbb251cf8abaf5af38ced9

I assume this is from some code review service like Gerrit?  We usually
as people to drop these when sending patches to be included in the
kernel (so when sending patches upstream).

	Krzysztof

RE: [PATCH] PCI: Mark AMD Navi14 GPU 0x7341 rev 0x00 ATS as broken

[Quan, Evan]

[AMD Official Use Only]

Thanks Krzysztof. Comment inline

> -----Original Message-----
> From: Krzysztof Wilczyński <kw@linux.com>
> Sent: Monday, May 31, 2021 10:41 PM
> To: Quan, Evan <Evan.Quan@amd.com>
> Cc: linux-pci@vger.kernel.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH] PCI: Mark AMD Navi14 GPU 0x7341 rev 0x00 ATS as
> broken
> 
> Hi Evan,
> 
> Thank you for sending the patch over!
> 
> A small nitpick: the subject line could just say:
> 
>   Add quirk for AMD Navi 14 to disable ATS support
> 
> Or something along these lines, as I am not sure how useful the ID and
> revision are in the subject, especially since the commit message explains in
> details what hardware is affected, etc.
> 
[Quan, Evan] Will update it in V2.
> > Unexpected GPU hang was observed during runpm stress test on 0x7341
> > rev 0x00. Further debugging shows broken ATS is related. Thus as a
> > followup of commit 5e89cd303e3a ("PCI:
> > Mark AMD Navi14 GPU rev 0xc5 ATS as broken"), we disable the ATS for
> > the specific SKU also.
> 
> As this might be a candidate for a back-port to current stable and long-term
> kernels, does it have any "Fixes" tag we could include here for reference?  If
> not, then it's OK.
[Quan, Evan] We have an internal ticket/link for this. But that is not accessible outside AMD.
So, I would say there is no "Fixes" tag available for this.
> 
> > Change-Id: I3d9d570bd473762e3bfbb251cf8abaf5af38ced9
> 
> I assume this is from some code review service like Gerrit?  We usually as
> people to drop these when sending patches to be included in the kernel (so
> when sending patches upstream).
> 
> 	Krzysztof
[Quan, Evan] Thanks! Will drop it in V2.

BR
Evan